Keep non default protocol in cloud id

When there is no protocol on the cloud id, we assume it's https://
But this means that when an http:// server currently sends an OCM
invite to another server, the protocol is striped and the remote
instance will try to talk back to https:// which might not be available.

Signed-off-by: Joas Schilling <coding@schilljs.com>

2 files changed, 59 insertions, 60 deletions

diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index de9af35f541..c5306d1df27 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -555,15 +555,9 @@ class User implements IUser {
 		return $uid . '@' . $server;
 	}
 
-	/**
-	 * @param string $url
-	 * @return string
-	 */
-	private function removeProtocolFromUrl($url) {
+	private function removeProtocolFromUrl(string $url): string {
 		if (strpos($url, 'https://') === 0) {
 			return substr($url, strlen('https://'));
-		} elseif (strpos($url, 'http://') === 0) {
-			return substr($url, strlen('http://'));
 		}
 
 		return $url;
diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
index 7fab7ececca..cc65fbf3356 100644
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -16,6 +16,7 @@ use OC\User\User;
 use OCP\Comments\ICommentsManager;
 use OCP\Files\Storage\IStorageFactory;
 use OCP\IConfig;
+use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Notification\INotification;
@@ -43,7 +44,7 @@ class UserTest extends TestCase {
 
 	public function testDisplayName() {
 		/**
-		 * @var \OC\User\Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var \OC\User\Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Backend::class);
 		$backend->expects($this->once())
@@ -65,7 +66,7 @@ class UserTest extends TestCase {
 	 */
 	public function testDisplayNameEmpty() {
 		/**
-		 * @var \OC\User\Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var \OC\User\Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Backend::class);
 		$backend->expects($this->once())
@@ -84,7 +85,7 @@ class UserTest extends TestCase {
 
 	public function testDisplayNameNotSupported() {
 		/**
-		 * @var \OC\User\Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var \OC\User\Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Backend::class);
 		$backend->expects($this->never())
@@ -101,7 +102,7 @@ class UserTest extends TestCase {
 
 	public function testSetPassword() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@@ -124,7 +125,7 @@ class UserTest extends TestCase {
 
 	public function testSetPasswordNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->never())
@@ -140,7 +141,7 @@ class UserTest extends TestCase {
 
 	public function testChangeAvatarSupportedYes() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(AvatarUserDummy::class);
 		$backend->expects($this->once())
@@ -164,7 +165,7 @@ class UserTest extends TestCase {
 
 	public function testChangeAvatarSupportedNo() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(AvatarUserDummy::class);
 		$backend->expects($this->once())
@@ -188,7 +189,7 @@ class UserTest extends TestCase {
 
 	public function testChangeAvatarNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(AvatarUserDummy::class);
 		$backend->expects($this->never())
@@ -204,7 +205,7 @@ class UserTest extends TestCase {
 
 	public function testDelete() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@@ -227,11 +228,11 @@ class UserTest extends TestCase {
 		}
 
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
-		$backend->expects($this->at(0))
+		$backend->expects($this->once())
 			->method('implementsActions')
 			->willReturnCallback(function ($actions) {
 				if ($actions === \OC\User\Backend::GET_HOME) {
@@ -244,12 +245,12 @@ class UserTest extends TestCase {
 		// important: getHome MUST be called before deleteUser because
 		// once the user is deleted, getHome implementations might not
 		// return anything
-		$backend->expects($this->at(1))
+		$backend->expects($this->once())
 			->method('getHome')
 			->with($this->equalTo('foo'))
 			->willReturn('/home/foo');
 
-		$backend->expects($this->at(2))
+		$backend->expects($this->once())
 			->method('deleteUser')
 			->with($this->equalTo('foo'));
 
@@ -259,7 +260,7 @@ class UserTest extends TestCase {
 
 	public function testGetHome() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@@ -290,7 +291,7 @@ class UserTest extends TestCase {
 
 	public function testGetHomeNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->never())
@@ -317,7 +318,7 @@ class UserTest extends TestCase {
 
 	public function testCanChangePassword() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -337,7 +338,7 @@ class UserTest extends TestCase {
 
 	public function testCanChangePasswordNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -351,7 +352,7 @@ class UserTest extends TestCase {
 
 	public function testCanChangeDisplayName() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -371,7 +372,7 @@ class UserTest extends TestCase {
 
 	public function testCanChangeDisplayNameNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -385,7 +386,7 @@ class UserTest extends TestCase {
 
 	public function testSetDisplayNameSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Database::class);
 
@@ -414,7 +415,7 @@ class UserTest extends TestCase {
 	 */
 	public function testSetDisplayNameEmpty() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Database::class);
 
@@ -435,7 +436,7 @@ class UserTest extends TestCase {
 
 	public function testSetDisplayNameNotSupported() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\OC\User\Database::class);
 
@@ -456,7 +457,7 @@ class UserTest extends TestCase {
 		$test = $this;
 
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@@ -509,7 +510,7 @@ class UserTest extends TestCase {
 		$test = $this;
 
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())
@@ -587,26 +588,30 @@ class UserTest extends TestCase {
 		$this->assertEquals($expectedHooks, $hooksCalled);
 	}
 
-	public function testGetCloudId() {
-		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
-		 */
+	public function dataGetCloudId(): array {
+		return [
+			['https://localhost:8888/nextcloud', 'foo@localhost:8888/nextcloud'],
+			['http://localhost:8888/nextcloud', 'foo@http://localhost:8888/nextcloud'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetCloudId
+	 */
+	public function testGetCloudId(string $absoluteUrl, string $cloudId): void {
+		/** @var Backend|MockObject $backend */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
-		$urlGenerator = $this->getMockBuilder('\OC\URLGenerator')
-				->setMethods(['getAbsoluteURL'])
-				->disableOriginalConstructor()->getMock();
-		$urlGenerator
-				->expects($this->any())
-				->method('getAbsoluteURL')
-				->withAnyParameters()
-				->willReturn('http://localhost:8888/owncloud');
+		$urlGenerator = $this->createMock(IURLGenerator::class);
+		$urlGenerator->method('getAbsoluteURL')
+			->withAnyParameters()
+			->willReturn($absoluteUrl);
 		$user = new User('foo', $backend, $this->dispatcher, null, null, $urlGenerator);
-		$this->assertEquals('foo@localhost:8888/owncloud', $user->getCloudId());
+		$this->assertEquals($cloudId, $user->getCloudId());
 	}
 
 	public function testSetEMailAddressEmpty() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -642,7 +647,7 @@ class UserTest extends TestCase {
 
 	public function testSetEMailAddress() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -679,11 +684,11 @@ class UserTest extends TestCase {
 
 	public function testSetEMailAddressNoChange() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@@ -704,7 +709,7 @@ class UserTest extends TestCase {
 
 	public function testSetQuota() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -741,11 +746,11 @@ class UserTest extends TestCase {
 
 	public function testGetDefaultUnlimitedQuota() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@@ -772,11 +777,11 @@ class UserTest extends TestCase {
 
 	public function testGetDefaultUnlimitedQuotaForbidden() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@@ -806,11 +811,11 @@ class UserTest extends TestCase {
 
 	public function testSetQuotaAddressNoChange() {
 		/**
-		 * @var UserInterface | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var UserInterface | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
-		/** @var PublicEmitter|\PHPUnit\Framework\MockObject\MockObject $emitter */
+		/** @var PublicEmitter|MockObject $emitter */
 		$emitter = $this->createMock(PublicEmitter::class);
 		$emitter->expects($this->never())
 			->method('emit');
@@ -828,7 +833,7 @@ class UserTest extends TestCase {
 
 	public function testGetLastLogin() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -848,7 +853,7 @@ class UserTest extends TestCase {
 
 	public function testSetEnabled() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -868,7 +873,7 @@ class UserTest extends TestCase {
 
 	public function testSetDisabled() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -908,7 +913,7 @@ class UserTest extends TestCase {
 
 	public function testSetDisabledAlreadyDisabled() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 
@@ -938,7 +943,7 @@ class UserTest extends TestCase {
 
 	public function testGetEMailAddress() {
 		/**
-		 * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend
+		 * @var Backend | MockObject $backend
 		 */
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);