diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index 66c41262d54fe..7d28b86bd9817 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -54,8 +54,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 0bd267cbd87ce..1cb90be98fa9f 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -134,8 +134,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]; } @@ -177,17 +176,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; } @@ -221,4 +220,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[] = $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 67e7f6af675b9..4b4d8d0fa062d 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -333,7 +333,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 = $this->invokePrivate($this->auth, 'validateUserPass', ['username', 'password']); diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php index ab6db8c9c75c6..80def826a1c9d 100644 --- a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php +++ b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php @@ -89,9 +89,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 = []; + } + + $authenticated = in_array($share->getId(), $allowedShareIds) + || $this->shareManager->checkPassword($share, $password); + $storedPassword = $share->getPassword(); - $authenticated = $this->session->get(PublicAuth::DAV_AUTHENTICATED) === $share->getId() || - $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 ad8023ba6bb35..46cba2e7da575 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -196,7 +196,12 @@ 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 = []; + } + + $this->session->set(PublicAuth::DAV_AUTHENTICATED, array_merge($allowedShareIds, [$this->share->getId()])); } 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 458606455d165..9bab261d7bd5f 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 d6e0321023ee6..c22d09d6cf1e4 100644 --- a/tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php +++ b/tests/lib/AppFramework/Controller/AuthPublicShareControllerTest.php @@ -109,17 +109,15 @@ public function testAuthenticateValidPassword(): void { $this->session->method('get') ->willReturnMap(['public_link_authenticate_redirect', ['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; @@ -131,7 +129,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 f8430d42ef1d3..bfc59e46aced6 100644 --- a/tests/lib/AppFramework/Controller/PublicShareControllerTest.php +++ b/tests/lib/AppFramework/Controller/PublicShareControllerTest.php @@ -77,10 +77,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);