Skip to content

Commit 4a3e72b

Browse files
julien-ncjulien-nc
committed
fix: only redirect to the login flow when the request comes from a 'navigation' context
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
1 parent a84c1ab commit 4a3e72b

4 files changed

Lines changed: 142 additions & 1 deletion

File tree

‎lib/AppInfo/Application.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OCA\UserOIDC\Listener\TimezoneHandlingListener;
2323
use OCA\UserOIDC\Listener\TokenInvalidatedListener;
2424
use OCA\UserOIDC\Service\ID4MeService;
25+
use OCA\UserOIDC\Service\RequestClassificationService;
2526
use OCA\UserOIDC\Service\SettingsService;
2627
use OCA\UserOIDC\Service\TokenService;
2728
use OCA\UserOIDC\User\Backend;
@@ -114,7 +115,10 @@ private function registerRedirect(IRequest $request, IURLGenerator $urlGenerator
114115
} catch (Exception $e) {
115116
// in case any errors happen when checking for the path do not apply redirect logic as it is only needed for the login
116117
}
117-
if ($isDefaultLogin && !$settings->getAllowMultipleUserBackEnds()) {
118+
if ($isDefaultLogin
119+
&& RequestClassificationService::isTopLevelHtmlNavigation($request)
120+
&& !$settings->getAllowMultipleUserBackEnds()
121+
) {
118122
$providers = $this->getCachedProviders($providerMapper);
119123
if (count($providers) === 1) {
120124
$targetUrl = $urlGenerator->linkToRoute(self::APP_ID . '.login.login', [

‎lib/Service/RequestClassificationService.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\UserOIDC\Service;
11+
12+
use OCP\IRequest;
13+
14+
class RequestClassificationService {
15+
public static function isTopLevelHtmlNavigation(IRequest $request): bool {
16+
if (strtoupper($request->getMethod()) !== 'GET') {
17+
return false;
18+
}
19+
20+
$accept = strtolower($request->getHeader('Accept'));
21+
if ($accept !== '' && strpos($accept, 'text/html') === false) {
22+
return false;
23+
}
24+
25+
if ($request->getHeader('X-Requested-With') === 'XMLHttpRequest') {
26+
return false;
27+
}
28+
29+
$secFetchMode = strtolower($request->getHeader('Sec-Fetch-Mode'));
30+
if ($secFetchMode !== '' && $secFetchMode !== 'navigate') {
31+
return false;
32+
}
33+
34+
$secFetchDest = strtolower($request->getHeader('Sec-Fetch-Dest'));
35+
if ($secFetchDest !== '' && $secFetchDest !== 'document') {
36+
return false;
37+
}
38+
39+
return true;
40+
}
41+
}

‎lib/Service/TokenService.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,15 @@ public function checkLoginToken(): void {
185185
}
186186

187187
public function reauthenticate(int $providerId) {
188+
if (!RequestClassificationService::isTopLevelHtmlNavigation($this->request)) {
189+
$this->userSession->logout();
190+
$this->logger->debug('[TokenService] reauthenticate skipped: request is not a top-level HTML navigation', [
191+
'provider_id' => $providerId,
192+
'request_uri' => $this->request->getRequestUri(),
193+
]);
194+
return;
195+
}
196+
188197
// Logout the user and redirect to the oidc login flow to gather a fresh token
189198
$this->userSession->logout();
190199
$redirectUrl = $this->urlGenerator->linkToRouteAbsolute(Application::APP_ID . '.login.login', [

‎tests/unit/Service/RequestClassificationServiceTest.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
use OCA\UserOIDC\Service\RequestClassificationService;
11+
use OCP\IRequest;
12+
use PHPUnit\Framework\Attributes\DataProvider;
13+
use PHPUnit\Framework\TestCase;
14+
15+
class RequestClassificationServiceTest extends TestCase {
16+
#[DataProvider('topLevelHtmlNavigationProvider')]
17+
public function testIsTopLevelHtmlNavigation(string $method, array $headers, bool $expected): void {
18+
$request = $this->createMock(IRequest::class);
19+
$request->method('getMethod')
20+
->willReturn($method);
21+
$request->method('getHeader')
22+
->willReturnCallback(static function (string $name) use ($headers): string {
23+
return $headers[$name] ?? '';
24+
});
25+
26+
self::assertSame($expected, RequestClassificationService::isTopLevelHtmlNavigation($request));
27+
}
28+
29+
public static function topLevelHtmlNavigationProvider(): array {
30+
return [
31+
'top level navigation with html accept' => [
32+
'GET',
33+
[
34+
'Accept' => 'text/html,application/xhtml+xml',
35+
'Sec-Fetch-Mode' => 'navigate',
36+
'Sec-Fetch-Dest' => 'document',
37+
],
38+
true,
39+
],
40+
'html accept without fetch metadata' => [
41+
'GET',
42+
[
43+
'Accept' => 'text/html',
44+
],
45+
true,
46+
],
47+
'xhr request' => [
48+
'GET',
49+
[
50+
'Accept' => 'text/html',
51+
'X-Requested-With' => 'XMLHttpRequest',
52+
],
53+
false,
54+
],
55+
'json request' => [
56+
'GET',
57+
[
58+
'Accept' => 'application/json',
59+
],
60+
false,
61+
],
62+
'non navigate fetch mode' => [
63+
'GET',
64+
[
65+
'Accept' => 'text/html',
66+
'Sec-Fetch-Mode' => 'cors',
67+
],
68+
false,
69+
],
70+
'non document destination' => [
71+
'GET',
72+
[
73+
'Accept' => 'text/html',
74+
'Sec-Fetch-Dest' => 'empty',
75+
],
76+
false,
77+
],
78+
'non get request' => [
79+
'POST',
80+
[
81+
'Accept' => 'text/html',
82+
],
83+
false,
84+
],
85+
];
86+
}
87+
}

0 commit comments

Comments
 (0)