diff --git a/apps/dav/lib/Connector/LegacyPublicAuth.php b/apps/dav/lib/Connector/LegacyPublicAuth.php index 37695229f0f63..0aa77993a7785 100644 --- a/apps/dav/lib/Connector/LegacyPublicAuth.php +++ b/apps/dav/lib/Connector/LegacyPublicAuth.php @@ -67,7 +67,7 @@ protected function validateUserPass($username, $password) { \OC_User::setIncognitoMode(true); // check if the share is password protected - if ($share->getPassword() !== null) { + if ($share->isPasswordProtected()) { if ($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL || $share->getShareType() === IShare::TYPE_CIRCLE) { diff --git a/apps/dav/lib/Connector/Sabre/PublicAuth.php b/apps/dav/lib/Connector/Sabre/PublicAuth.php index 4f41b95c220af..9548050374f5d 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -63,7 +63,7 @@ public function check(RequestInterface $request, ResponseInterface $response): a try { $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), self::BRUTEFORCE_ACTION); - if (count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->getPassword() !== null) { + if (count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->isPasswordProtected()) { throw new PreconditionFailed('Strict cookie check failed'); } @@ -142,7 +142,7 @@ private function checkToken(): array { } // If the share is protected but user is not authenticated - if ($share->getPassword() !== null) { + if ($share->isPasswordProtected()) { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); throw new NotAuthenticated(); } @@ -176,7 +176,7 @@ protected function validateUserPass($username, $password) { \OC_User::setIncognitoMode(true); // check if the share is password protected - if ($share->getPassword() !== null) { + if ($share->isPasswordProtected()) { if ($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL || $share->getShareType() === IShare::TYPE_CIRCLE) { diff --git a/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php b/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php index f72869015fc3a..57cd8bc96a27e 100644 --- a/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php @@ -73,6 +73,7 @@ public function testNoShare(): void { public function testShareNoPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn(null); + $share->method('isPasswordProtected')->willReturn(false); $this->shareManager->expects($this->once()) ->method('getShareByToken') @@ -86,6 +87,7 @@ public function testShareNoPassword(): void { public function testSharePasswordFancyShareType(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(42); $this->shareManager->expects($this->once()) @@ -100,6 +102,7 @@ public function testSharePasswordFancyShareType(): void { public function testSharePasswordRemote(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_REMOTE); $this->shareManager->expects($this->once()) @@ -114,6 +117,7 @@ public function testSharePasswordRemote(): void { public function testSharePasswordLinkValidPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $this->shareManager->expects($this->once()) @@ -134,6 +138,7 @@ public function testSharePasswordLinkValidPassword(): void { public function testSharePasswordMailValidPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_EMAIL); $this->shareManager->expects($this->once()) @@ -154,6 +159,7 @@ public function testSharePasswordMailValidPassword(): void { public function testInvalidSharePasswordLinkValidSession(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $share->method('getId')->willReturn('42'); @@ -178,6 +184,7 @@ public function testInvalidSharePasswordLinkValidSession(): void { public function testSharePasswordLinkInvalidSession(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $share->method('getId')->willReturn('42'); @@ -202,6 +209,7 @@ public function testSharePasswordLinkInvalidSession(): void { public function testSharePasswordMailInvalidSession(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_EMAIL); $share->method('getId')->willReturn('42'); diff --git a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php index b853358569c7e..aa843fd0cfa35 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -97,6 +97,7 @@ public function testCheckTokenValidShare(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn(null); + $share->method('isPasswordProtected')->willReturn(false); $this->shareManager->expects($this->once()) ->method('getShareByToken') @@ -146,6 +147,7 @@ public function testCheckTokenPasswordNotAuthenticated(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(42); $this->shareManager->expects($this->once()) @@ -165,6 +167,7 @@ public function testCheckTokenPasswordAuthenticatedWrongShare(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(42); $this->shareManager->expects($this->once()) @@ -199,6 +202,7 @@ public function testShareNoPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn(null); + $share->method('isPasswordProtected')->willReturn(false); $this->shareManager->expects($this->once()) ->method('getShareByToken') @@ -216,6 +220,7 @@ public function testSharePasswordFancyShareType(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(42); $this->shareManager->expects($this->once()) @@ -234,6 +239,7 @@ public function testSharePasswordRemote(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_REMOTE); $this->shareManager->expects($this->once()) @@ -252,6 +258,7 @@ public function testSharePasswordLinkValidPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $this->shareManager->expects($this->once()) @@ -276,6 +283,7 @@ public function testSharePasswordMailValidPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_EMAIL); $this->shareManager->expects($this->once()) @@ -300,6 +308,7 @@ public function testInvalidSharePasswordLinkValidSession(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $share->method('getId')->willReturn('42'); @@ -329,6 +338,7 @@ public function testSharePasswordLinkInvalidSession(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $share->method('getId')->willReturn('42'); @@ -358,6 +368,7 @@ public function testSharePasswordMailInvalidSession(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getShareType')->willReturn(IShare::TYPE_EMAIL); $share->method('getId')->willReturn('42'); diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php index ceac2388f7b24..ac5b5f4ec055f 100644 --- a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php +++ b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php @@ -98,8 +98,7 @@ public function createFederatedShare($shareWith, $token, $password = '') { $authenticated = in_array($share->getId(), $allowedShareIds) || $this->shareManager->checkPassword($share, $password); - $storedPassword = $share->getPassword(); - if (!empty($storedPassword) && !$authenticated) { + if ($share->isPasswordProtected() && !$authenticated) { $response = new JSONResponse( ['message' => 'No permission to access the share'], Http::STATUS_BAD_REQUEST diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index 24a537e110d1d..f4015a02f4a43 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -1,7 +1,7 @@ share->getPassword() !== null; + return $this->share->isPasswordProtected(); } /** @@ -181,7 +181,7 @@ public function directLink(string $token) { } // Password protected shares have no direct link! - if ($share->getPassword() !== null) { + if ($share->isPasswordProtected()) { return new DataResponse([], Http::STATUS_FORBIDDEN); } diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 0cc7f1a21248e..18e8b32b7a014 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -1,7 +1,7 @@ share->getPassword() !== null; + return $this->share->isPasswordProtected(); } #[\Override] diff --git a/apps/files_sharing/lib/Controller/ShareInfoController.php b/apps/files_sharing/lib/Controller/ShareInfoController.php index b2e577e75702f..1265256d7a6c2 100644 --- a/apps/files_sharing/lib/Controller/ShareInfoController.php +++ b/apps/files_sharing/lib/Controller/ShareInfoController.php @@ -1,7 +1,7 @@ getPassword() && !$this->shareManager->checkPassword($share, $password)) { + if ($share->isPasswordProtected() && !$this->shareManager->checkPassword($share, $password)) { $response = new JSONResponse([], Http::STATUS_FORBIDDEN); $response->throttle(['token' => $t]); return $response; diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 6d684a8c74d30..e275782ccbbe3 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -638,6 +638,7 @@ public function createShare( $share->method('getMailSend')->willReturn($mail_send); $share->method('getToken')->willReturn($token); $share->method('getPassword')->willReturn($password); + $share->method('isPasswordProtected')->willReturn(!empty($password)); if ($shareType === IShare::TYPE_USER || $shareType === IShare::TYPE_GROUP diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index 8bec64ef42143..22b2f8d29cf16 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -702,6 +702,7 @@ public function testShowShareInvalid(): void { public function testDownloadShareWithCreateOnlyShare(): void { $share = $this->getMockBuilder(IShare::class)->getMock(); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share ->expects($this->once()) ->method('getPermissions') @@ -728,6 +729,7 @@ public function testDownloadShareWithoutDownloadPermission(): void { $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $share->expects(self::once()) ->method('getPermissions') ->willReturn(Constants::PERMISSION_READ); diff --git a/apps/files_sharing/tests/Controller/ShareInfoControllerTest.php b/apps/files_sharing/tests/Controller/ShareInfoControllerTest.php index e795ccba92316..288682c7f896a 100644 --- a/apps/files_sharing/tests/Controller/ShareInfoControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareInfoControllerTest.php @@ -51,6 +51,7 @@ public function testWrongPassword(): void { $share = $this->createMock(IShare::class); $share->method('getPassword') ->willReturn('sharePass'); + $share->method('isPasswordProtected')->willReturn(true); $this->shareManager->method('getShareByToken') ->with('token') @@ -68,6 +69,7 @@ public function testNoReadPermissions(): void { $share = $this->createMock(IShare::class); $share->method('getPassword') ->willReturn('sharePass'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getPermissions') ->willReturn(Constants::PERMISSION_CREATE); @@ -109,6 +111,7 @@ public function testInfoFile(): void { $share = $this->createMock(IShare::class); $share->method('getPassword') ->willReturn('sharePass'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getPermissions') ->willReturn(Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE); $share->method('getNode') @@ -141,6 +144,7 @@ public function testInfoFileRO(): void { $share = $this->createMock(IShare::class); $share->method('getPassword') ->willReturn('sharePass'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getPermissions') ->willReturn(Constants::PERMISSION_READ); $share->method('getNode') @@ -222,6 +226,7 @@ public function testInfoFolder(): void { $share = $this->createMock(IShare::class); $share->method('getPassword') ->willReturn('sharePass'); + $share->method('isPasswordProtected')->willReturn(true); $share->method('getPermissions') ->willReturn(Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE); $share->method('getNode') diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 524f121bb9297..8a296d3641f37 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1,7 +1,7 @@ getPassword() === null) { + // if the share is not password protected or the password to check is empty, there is nothing to check + if ($password === null || !$share->isPasswordProtected()) { return false; } diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index a89cbc458a343..3f8d3d97b21ef 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -1,7 +1,7 @@ password; } + /** + * @inheritdoc + */ + #[\Override] + public function isPasswordProtected(): bool { + return $this->password !== '' && $this->password !== null; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 16fcd3f9f0010..1323299bc5137 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -1,7 +1,7 @@ method('getAttributes')->willReturn($attributes); $share->method('getExpirationDate')->willReturn($expireDate); $share->method('getPassword')->willReturn($password); + $share->method('isPasswordProtected')->willReturn(!empty($password)); return $share; } @@ -4126,6 +4127,7 @@ public function testCheckPasswordNoPassword(): void { $this->assertFalse($this->manager->checkPassword($share, 'password')); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $this->assertFalse($this->manager->checkPassword($share, null)); } @@ -4133,6 +4135,7 @@ public function testCheckPasswordInvalidPassword(): void { $share = $this->createMock(IShare::class); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $share->method('getPassword')->willReturn('password'); + $share->method('isPasswordProtected')->willReturn(true); $this->hasher->method('verify')->with('invalidpassword', 'password', '')->willReturn(false); @@ -4143,6 +4146,7 @@ public function testCheckPasswordValidPassword(): void { $share = $this->createMock(IShare::class); $share->method('getShareType')->willReturn(IShare::TYPE_LINK); $share->method('getPassword')->willReturn('passwordHash'); + $share->method('isPasswordProtected')->willReturn(true); $this->hasher->method('verify')->with('password', 'passwordHash', '')->willReturn(true);