From e8b2082a5b57612d8c7c31aaec999a004735898b Mon Sep 17 00:00:00 2001 From: ernolf Date: Wed, 19 Aug 2026 19:01:17 +0200 Subject: [PATCH 1/2] fix(dav): return the auth failure instead of throwing it - the throw leaves Auth\Plugin::beforeMethod() before challenge(), so the 401 has no WWW-Authenticate header - ajax callers keep the throw to avoid the browser password prompt Fixes #59391 Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: ernolf --- apps/dav/lib/Connector/Sabre/PublicAuth.php | 7 ++++- .../unit/Connector/Sabre/PublicAuthTest.php | 27 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/PublicAuth.php b/apps/dav/lib/Connector/Sabre/PublicAuth.php index 59a7bc178176b..5a08e27ad6160 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -144,7 +144,12 @@ private function checkToken(): array { // If the share is protected but user is not authenticated if ($share->isPasswordProtected()) { $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); - throw new NotAuthenticated(); + if (in_array('XMLHttpRequest', explode(',', $this->request->getHeader('X-Requested-With')))) { + // do not challenge over ajax, it would trigger the browser password prompt + throw new NotAuthenticated(); + } + // only a returned failure reaches Auth\Plugin::challenge() + return [false, 'No password supplied for password protected share']; } return [true, $this->principalPrefix . $token]; diff --git a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php index aa843fd0cfa35..3e881e4c8f64a 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -144,6 +144,28 @@ public function testCheckTokenAlreadyAuthenticated(): void { public function testCheckTokenPasswordNotAuthenticated(): void { $this->request->method('getPathInfo') ->willReturn('/dav/files/GX9HSGQrGE'); + $this->request->method('getHeader')->with('X-Requested-With')->willReturn(''); + + $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()) + ->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(false); + + $result = self::invokePrivate($this->auth, 'checkToken'); + $this->assertSame([false, 'No password supplied for password protected share'], $result); + } + + public function testCheckTokenPasswordNotAuthenticatedAjax(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + $this->request->method('getHeader')->with('X-Requested-With')->willReturn('XMLHttpRequest'); $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); @@ -164,6 +186,7 @@ public function testCheckTokenPasswordNotAuthenticated(): void { public function testCheckTokenPasswordAuthenticatedWrongShare(): void { $this->request->method('getPathInfo') ->willReturn('/dav/files/GX9HSGQrGE'); + $this->request->method('getHeader')->with('X-Requested-With')->willReturn(''); $share = $this->createMock(IShare::class); $share->method('getPassword')->willReturn('password'); @@ -178,8 +201,8 @@ public function testCheckTokenPasswordAuthenticatedWrongShare(): void { $this->session->method('exists')->with('public_link_authenticated')->willReturn(false); $this->session->method('get')->with('public_link_authenticated')->willReturn('43'); - $this->expectException(\Sabre\DAV\Exception\NotAuthenticated::class); - self::invokePrivate($this->auth, 'checkToken'); + $result = self::invokePrivate($this->auth, 'checkToken'); + $this->assertSame([false, 'No password supplied for password protected share'], $result); } public function testNoShare(): void { From 0eae4f0cddf757368a640be20e2e23c0c66d4cf8 Mon Sep 17 00:00:00 2001 From: ernolf Date: Wed, 19 Aug 2026 19:03:00 +0200 Subject: [PATCH 2/2] fix(dav): skip the strict cookie check when credentials are supplied - clients that cannot hold __Host- cookies get a redirect to /s/ and loop between the endpoints - the check targets ambient session replay, which a request carrying its own credentials is not Fixes #63418 Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: ernolf --- apps/dav/lib/Connector/Sabre/PublicAuth.php | 10 +-- .../unit/Connector/Sabre/PublicAuthTest.php | 63 +++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/PublicAuth.php b/apps/dav/lib/Connector/Sabre/PublicAuth.php index 5a08e27ad6160..45116c9197b71 100644 --- a/apps/dav/lib/Connector/Sabre/PublicAuth.php +++ b/apps/dav/lib/Connector/Sabre/PublicAuth.php @@ -63,10 +63,6 @@ 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()->isPasswordProtected()) { - throw new PreconditionFailed('Strict cookie check failed'); - } - $auth = new HTTP\Auth\Basic( $this->realm, $request, @@ -74,6 +70,12 @@ public function check(RequestInterface $request, ResponseInterface $response): a ); $userpass = $auth->getCredentials(); + + // the check targets ambient session replay, supplied credentials are not ambient + if ($userpass === null && count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->isPasswordProtected()) { + throw new PreconditionFailed('Strict cookie check failed'); + } + // If authentication provided, checking its validity if ($userpass && !$this->validateUserPass($userpass[0], $userpass[1])) { return [false, 'Username or password was incorrect']; diff --git a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php index 3e881e4c8f64a..faba957967487 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php @@ -205,6 +205,69 @@ public function testCheckTokenPasswordAuthenticatedWrongShare(): void { $this->assertSame([false, 'No password supplied for password protected share'], $result); } + public function testCheckWithoutCredentialsRunsStrictCookieCheck(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + $this->request->method('passesStrictCookieCheck')->willReturn(false); + + $share = $this->createMock(IShare::class); + $share->method('isPasswordProtected')->willReturn(true); + + $this->shareManager->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + + $this->urlGenerator->method('linkToRoute')->willReturn('/s/GX9HSGQrGE'); + + $cookies = $_COOKIE; + $_COOKIE = ['nc_session_id' => 'irrelevant']; + + try { + $this->expectException(\Sabre\DAV\Exception\PreconditionFailed::class); + $this->auth->check( + new \Sabre\HTTP\Request('PROPFIND', '/public.php/dav/files/GX9HSGQrGE'), + new \Sabre\HTTP\Response(), + ); + } finally { + $_COOKIE = $cookies; + } + } + + public function testCheckWithCredentialsSkipsStrictCookieCheck(): void { + $this->request->method('getPathInfo') + ->willReturn('/dav/files/GX9HSGQrGE'); + $this->request->method('passesStrictCookieCheck')->willReturn(false); + + $share = $this->createMock(IShare::class); + $share->method('isPasswordProtected')->willReturn(true); + $share->method('getShareType')->willReturn(IShare::TYPE_LINK); + $share->method('getId')->willReturn('42'); + + $this->shareManager->method('getShareByToken') + ->with('GX9HSGQrGE') + ->willReturn($share); + $this->shareManager->method('checkPassword') + ->with($share, 'password') + ->willReturn(true); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn(['42']); + + $request = new \Sabre\HTTP\Request('PROPFIND', '/public.php/dav/files/GX9HSGQrGE'); + $request->addHeader('Authorization', 'Basic ' . base64_encode('GX9HSGQrGE:password')); + + $cookies = $_COOKIE; + $_COOKIE = ['nc_session_id' => 'irrelevant']; + + try { + $result = $this->auth->check($request, new \Sabre\HTTP\Response()); + } finally { + $_COOKIE = $cookies; + } + + $this->assertSame([true, 'principals/GX9HSGQrGE'], $result); + } + public function testNoShare(): void { $this->request->method('getPathInfo') ->willReturn('/dav/files/GX9HSGQrGE');