Merge pull request #55962 from nextcloud/backport/55955/stable31

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

apps/dav/lib/Connector/Sabre/Auth.php

@@ -54,8 +54,11 @@ class Auth extends AbstractBasic {
 	 * @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;
 	}
 
 	/**

apps/dav/lib/Connector/Sabre/PublicAuth.php

@@ -134,8 +134,7 @@ class PublicAuth extends AbstractBasic {
 		\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 @@ class PublicAuth extends AbstractBasic {
 			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 @@ class PublicAuth extends AbstractBasic {
 
 		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 @@ class PublicAuthTest extends \Test\TestCase {
 			)->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/federatedfilesharing/lib/Controller/MountPublicLinkController.php

@@ -89,9 +89,15 @@ class MountPublicLinkController extends Controller {
 		}
 
 		// 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'],

apps/files_sharing/lib/Controller/ShareController.php

@@ -196,7 +196,12 @@ class ShareController extends AuthPublicShareController {
 		}
 
 		// 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 @@ abstract class AuthPublicShareController extends PublicShareController {
 		$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,8 +25,8 @@ use OCP\ISession;
  * @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 @@ abstract class PublicShareController extends Controller {
 		}
 
 		// 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 @@ abstract class PublicShareController extends Controller {
 	 */
 	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 @@ class AuthPublicShareControllerTest extends \Test\TestCase {
 		$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 @@ class AuthPublicShareControllerTest extends \Test\TestCase {
 		$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 @@ class PublicShareControllerTest extends \Test\TestCase {
 		$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);