diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index a174920946a32..5ef33958c9b0a 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -55,8 +55,11 @@ public function __construct( * @see https://github.com/owncloud/core/issues/13245 */ public function isDavAuthenticated(string $username): bool { - return !is_null($this->session->get(self::DAV_AUTHENTICATED)) - && $this->session->get(self::DAV_AUTHENTICATED) === $username; + if (is_null($this->session->get(self::DAV_AUTHENTICATED))) { + return false; + } + + return $this->session->get(self::DAV_AUTHENTICATED) === $username; } /** diff --git a/apps/dav/lib/Connector/Sabre/PublicAuth.php b/apps/dav/lib/Connector/Sabre/PublicAuth.php index 2ca1c25e2f69a..b39babb85b459 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -137,8 +137,7 @@ private function checkToken(): array { \OC_User::setIncognitoMode(true); // If already authenticated - if ($this->session->exists(self::DAV_AUTHENTICATED) - && $this->session->get(self::DAV_AUTHENTICATED) === $share->getId()) { + if ($this->isShareInSession($share)) { return [true, $this->principalPrefix . $token]; } @@ -180,17 +179,17 @@ protected function validateUserPass($username, $password) { if ($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL || $share->getShareType() === IShare::TYPE_CIRCLE) { + // Validate password if provided if ($this->shareManager->checkPassword($share, $password)) { // If not set, set authenticated session cookie - if (!$this->session->exists(self::DAV_AUTHENTICATED) - || $this->session->get(self::DAV_AUTHENTICATED) !== $share->getId()) { - $this->session->set(self::DAV_AUTHENTICATED, $share->getId()); + if (!$this->isShareInSession($share)) { + $this->addShareToSession($share); } return true; } - if ($this->session->exists(PublicAuth::DAV_AUTHENTICATED) - && $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId()) { + // We are already authenticated for this share in the session + if ($this->isShareInSession($share)) { return true; } @@ -224,4 +223,27 @@ public function getShare(): IShare { return $this->share; } + + private function addShareToSession(IShare $share): void { + $allowedShareIds = $this->session->get(self::DAV_AUTHENTICATED) ?? []; + if (!is_array($allowedShareIds)) { + $allowedShareIds = []; + } + + $allowedShareIds[] = (string)$share->getId(); + $this->session->set(self::DAV_AUTHENTICATED, $allowedShareIds); + } + + private function isShareInSession(IShare $share): bool { + if (!$this->session->exists(self::DAV_AUTHENTICATED)) { + return false; + } + + $allowedShareIds = $this->session->get(self::DAV_AUTHENTICATED); + if (!is_array($allowedShareIds)) { + return false; + } + + return in_array($share->getId(), $allowedShareIds); + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php index fef62b51c6750..26d0ef541f2f3 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -316,7 +316,7 @@ public function testInvalidSharePasswordLinkValidSession(): void { )->willReturn(false); $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); - $this->session->method('get')->with('public_link_authenticated')->willReturn('42'); + $this->session->method('get')->with('public_link_authenticated')->willReturn(['42']); $result = self::invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php index b8d2090713b8a..d0aa108cba635 100644 --- a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php +++ b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php @@ -90,9 +90,15 @@ public function createFederatedShare($shareWith, $token, $password = '') { } // make sure that user is authenticated in case of a password protected link + $allowedShareIds = $this->session->get(PublicAuth::DAV_AUTHENTICATED); + if (!is_array($allowedShareIds)) { + $allowedShareIds = []; + } + $storedPassword = $share->getPassword(); - $authenticated = $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId() + $authenticated = in_array($share->getId(), $allowedShareIds) || $this->shareManager->checkPassword($share, $password); + if (!empty($storedPassword) && !$authenticated) { $response = new JSONResponse( ['message' => 'No permission to access the share'], diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 5a776379fce84..6a3b2cc91312e 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -197,7 +197,13 @@ protected function authSucceeded() { } // For share this was always set so it is still used in other apps - $this->session->set(PublicAuth::DAV_AUTHENTICATED, $this->share->getId()); + $allowedShareIds = $this->session->get(PublicAuth::DAV_AUTHENTICATED); + if (!is_array($allowedShareIds)) { + $allowedShareIds = []; + } + + $allowedShareIds[] = $this->share->getId(); + $this->session->set(PublicAuth::DAV_AUTHENTICATED, $allowedShareIds); } protected function authFailed() { diff --git a/lib/public/AppFramework/AuthPublicShareController.php b/lib/public/AppFramework/AuthPublicShareController.php index 28a92fedcc9a1..4113f95770b12 100644 --- a/lib/public/AppFramework/AuthPublicShareController.php +++ b/lib/public/AppFramework/AuthPublicShareController.php @@ -158,8 +158,7 @@ final public function authenticate(string $password = '', string $passwordReques $this->session->regenerateId(true, true); $response = $this->getRedirect(); - $this->session->set('public_link_authenticated_token', $this->getToken()); - $this->session->set('public_link_authenticated_password_hash', $this->getPasswordHash()); + $this->storeTokenSession($this->getToken(), $this->getPasswordHash()); $this->authSucceeded(); diff --git a/lib/public/AppFramework/PublicShareController.php b/lib/public/AppFramework/PublicShareController.php index 999b3827565ff..b0f7284131475 100644 --- a/lib/public/AppFramework/PublicShareController.php +++ b/lib/public/AppFramework/PublicShareController.php @@ -25,8 +25,8 @@ * @since 14.0.0 */ abstract class PublicShareController extends Controller { - /** @var ISession */ - protected $session; + + public const DAV_AUTHENTICATED_FRONTEND = 'public_link_authenticated_frontend'; /** @var string */ private $token; @@ -34,12 +34,12 @@ abstract class PublicShareController extends Controller { /** * @since 14.0.0 */ - public function __construct(string $appName, + public function __construct( + string $appName, IRequest $request, - ISession $session) { + protected ISession $session, + ) { parent::__construct($appName, $request); - - $this->session = $session; } /** @@ -98,8 +98,7 @@ public function isAuthenticated(): bool { } // If we are authenticated properly - if ($this->session->get('public_link_authenticated_token') === $this->getToken() - && $this->session->get('public_link_authenticated_password_hash') === $this->getPasswordHash()) { + if ($this->validateTokenSession($this->getToken(), $this->getPasswordHash())) { return true; } @@ -116,4 +115,31 @@ public function isAuthenticated(): bool { */ public function shareNotFound() { } + + /** + * Validate the token and password hash stored in session + */ + protected function validateTokenSession(string $token, string $passwordHash): bool { + $allowedTokensJSON = $this->session->get(self::DAV_AUTHENTICATED_FRONTEND) ?? '[]'; + $allowedTokens = json_decode($allowedTokensJSON, true); + if (!is_array($allowedTokens)) { + $allowedTokens = []; + } + + return ($allowedTokens[$token] ?? '') == $passwordHash; + } + + /** + * Store the token and password hash in session + */ + protected function storeTokenSession(string $token, string $passwordHash = ''): void { + $allowedTokensJSON = $this->session->get(self::DAV_AUTHENTICATED_FRONTEND) ?? '[]'; + $allowedTokens = json_decode($allowedTokensJSON, true); + if (!is_array($allowedTokens)) { + $allowedTokens = []; + } + + $allowedTokens[$token] = $passwordHash; + $this->session->set(self::DAV_AUTHENTICATED_FRONTEND, json_encode($allowedTokens)); + } } diff --git a/tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php b/tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php index f692333cc9e33..5975c2e10d602 100644 --- a/tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php +++ b/tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php @@ -112,17 +112,15 @@ public function testAuthenticateValidPassword(): void { ['public_link_authenticate_redirect', json_encode(['foo' => 'bar'])], ]); - $tokenSet = false; - $hashSet = false; + $tokenStored = false; $this->session ->method('set') - ->willReturnCallback(function ($key, $value) use (&$tokenSet, &$hashSet) { - if ($key === 'public_link_authenticated_token' && $value === 'token') { - $tokenSet = true; - return true; - } - if ($key === 'public_link_authenticated_password_hash' && $value === 'hash') { - $hashSet = true; + ->willReturnCallback(function ($key, $value) use (&$tokenStored) { + if ($key === AuthPublicShareController::DAV_AUTHENTICATED_FRONTEND) { + $decoded = json_decode($value, true); + if (isset($decoded['token']) && $decoded['token'] === 'hash') { + $tokenStored = true; + } return true; } return false; @@ -134,7 +132,6 @@ public function testAuthenticateValidPassword(): void { $result = $this->controller->authenticate('password'); $this->assertInstanceOf(RedirectResponse::class, $result); $this->assertSame('myLink!', $result->getRedirectURL()); - $this->assertTrue($tokenSet); - $this->assertTrue($hashSet); + $this->assertTrue($tokenStored); } } diff --git a/tests/lib/AppFramework/Controller/PublicShareControllerTest.php b/tests/lib/AppFramework/Controller/PublicShareControllerTest.php index 2615bfd13f0dd..7f41a30e55a0b 100644 --- a/tests/lib/AppFramework/Controller/PublicShareControllerTest.php +++ b/tests/lib/AppFramework/Controller/PublicShareControllerTest.php @@ -74,10 +74,8 @@ public function testIsAuthenticatedNotPasswordProtected(bool $protected, string $controller = new TestController('app', $this->request, $this->session, $hash2, $protected); $this->session->method('get') - ->willReturnMap([ - ['public_link_authenticated_token', $token1], - ['public_link_authenticated_password_hash', $hash1], - ]); + ->with(PublicShareController::DAV_AUTHENTICATED_FRONTEND) + ->willReturn("{\"$token1\":\"$hash1\"}"); $controller->setToken($token2);