fix(dav): allow multiple link shares token in session

Signed-off-by: skjnldsv <[email protected]>

[skip ci]

Author: skjnldsv

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,11 +176,11 @@ 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;
 				}
@@ -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);
+	}
 }

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']);
 

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() {

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();
 

lib/public/AppFramework/PublicShareController.php

@@ -25,21 +25,21 @@
  * @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;
 
 	/**
 	 * @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;
 	}
 
 	/**
@@ -116,4 +116,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));
+	}
 }

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);
 	}
 }

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);