From f2cd34d0d897b5c349524f316e71197e45471526 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Fri, 10 Apr 2026 16:07:13 +0200 Subject: [PATCH 1/6] fix: only redirect to the login flow when the request comes from a 'navigation' context Signed-off-by: Julien Veyssier --- lib/AppInfo/Application.php | 6 +- lib/Service/RequestClassificationService.php | 41 +++++++++ lib/Service/TokenService.php | 9 ++ .../RequestClassificationServiceTest.php | 87 +++++++++++++++++++ 4 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 lib/Service/RequestClassificationService.php create mode 100644 tests/unit/Service/RequestClassificationServiceTest.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 0a421c6c..192220ca 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -22,6 +22,7 @@ use OCA\UserOIDC\Listener\TimezoneHandlingListener; use OCA\UserOIDC\Listener\TokenInvalidatedListener; use OCA\UserOIDC\Service\ID4MeService; +use OCA\UserOIDC\Service\RequestClassificationService; use OCA\UserOIDC\Service\SettingsService; use OCA\UserOIDC\Service\TokenService; use OCA\UserOIDC\User\Backend; @@ -114,7 +115,10 @@ private function registerRedirect(IRequest $request, IURLGenerator $urlGenerator } catch (Exception $e) { // in case any errors happen when checking for the path do not apply redirect logic as it is only needed for the login } - if ($isDefaultLogin && !$settings->getAllowMultipleUserBackEnds()) { + if ($isDefaultLogin + && RequestClassificationService::isTopLevelHtmlNavigation($request) + && !$settings->getAllowMultipleUserBackEnds() + ) { $providers = $this->getCachedProviders($providerMapper); if (count($providers) === 1) { $targetUrl = $urlGenerator->linkToRoute(self::APP_ID . '.login.login', [ diff --git a/lib/Service/RequestClassificationService.php b/lib/Service/RequestClassificationService.php new file mode 100644 index 00000000..bdd0f730 --- /dev/null +++ b/lib/Service/RequestClassificationService.php @@ -0,0 +1,41 @@ +getMethod()) !== 'GET') { + return false; + } + + $accept = strtolower($request->getHeader('Accept')); + if ($accept !== '' && strpos($accept, 'text/html') === false) { + return false; + } + + if ($request->getHeader('X-Requested-With') === 'XMLHttpRequest') { + return false; + } + + $secFetchMode = strtolower($request->getHeader('Sec-Fetch-Mode')); + if ($secFetchMode !== '' && $secFetchMode !== 'navigate') { + return false; + } + + $secFetchDest = strtolower($request->getHeader('Sec-Fetch-Dest')); + if ($secFetchDest !== '' && $secFetchDest !== 'document') { + return false; + } + + return true; + } +} diff --git a/lib/Service/TokenService.php b/lib/Service/TokenService.php index 4a0ddf85..9da79c6f 100644 --- a/lib/Service/TokenService.php +++ b/lib/Service/TokenService.php @@ -185,6 +185,15 @@ public function checkLoginToken(): void { } public function reauthenticate(int $providerId) { + if (!RequestClassificationService::isTopLevelHtmlNavigation($this->request)) { + $this->userSession->logout(); + $this->logger->debug('[TokenService] reauthenticate skipped: request is not a top-level HTML navigation', [ + 'provider_id' => $providerId, + 'request_uri' => $this->request->getRequestUri(), + ]); + return; + } + // Logout the user and redirect to the oidc login flow to gather a fresh token $this->userSession->logout(); $redirectUrl = $this->urlGenerator->linkToRouteAbsolute(Application::APP_ID . '.login.login', [ diff --git a/tests/unit/Service/RequestClassificationServiceTest.php b/tests/unit/Service/RequestClassificationServiceTest.php new file mode 100644 index 00000000..44dfaab0 --- /dev/null +++ b/tests/unit/Service/RequestClassificationServiceTest.php @@ -0,0 +1,87 @@ +createMock(IRequest::class); + $request->method('getMethod') + ->willReturn($method); + $request->method('getHeader') + ->willReturnCallback(static function (string $name) use ($headers): string { + return $headers[$name] ?? ''; + }); + + self::assertSame($expected, RequestClassificationService::isTopLevelHtmlNavigation($request)); + } + + public static function topLevelHtmlNavigationProvider(): array { + return [ + 'top level navigation with html accept' => [ + 'GET', + [ + 'Accept' => 'text/html,application/xhtml+xml', + 'Sec-Fetch-Mode' => 'navigate', + 'Sec-Fetch-Dest' => 'document', + ], + true, + ], + 'html accept without fetch metadata' => [ + 'GET', + [ + 'Accept' => 'text/html', + ], + true, + ], + 'xhr request' => [ + 'GET', + [ + 'Accept' => 'text/html', + 'X-Requested-With' => 'XMLHttpRequest', + ], + false, + ], + 'json request' => [ + 'GET', + [ + 'Accept' => 'application/json', + ], + false, + ], + 'non navigate fetch mode' => [ + 'GET', + [ + 'Accept' => 'text/html', + 'Sec-Fetch-Mode' => 'cors', + ], + false, + ], + 'non document destination' => [ + 'GET', + [ + 'Accept' => 'text/html', + 'Sec-Fetch-Dest' => 'empty', + ], + false, + ], + 'non get request' => [ + 'POST', + [ + 'Accept' => 'text/html', + ], + false, + ], + ]; + } +} From 2a7b8692fdcaf0a7a1b3a581e7b360fd6014978d Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Thu, 16 Apr 2026 17:01:20 +0200 Subject: [PATCH 2/6] make isTopLevelHtmlNavigation less restrictive Signed-off-by: Julien Veyssier --- lib/Service/RequestClassificationService.php | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/lib/Service/RequestClassificationService.php b/lib/Service/RequestClassificationService.php index bdd0f730..b9c39ac0 100644 --- a/lib/Service/RequestClassificationService.php +++ b/lib/Service/RequestClassificationService.php @@ -17,8 +17,7 @@ public static function isTopLevelHtmlNavigation(IRequest $request): bool { return false; } - $accept = strtolower($request->getHeader('Accept')); - if ($accept !== '' && strpos($accept, 'text/html') === false) { + if ($request->getHeader('OCS-apirequest') !== '') { return false; } @@ -26,16 +25,6 @@ public static function isTopLevelHtmlNavigation(IRequest $request): bool { return false; } - $secFetchMode = strtolower($request->getHeader('Sec-Fetch-Mode')); - if ($secFetchMode !== '' && $secFetchMode !== 'navigate') { - return false; - } - - $secFetchDest = strtolower($request->getHeader('Sec-Fetch-Dest')); - if ($secFetchDest !== '' && $secFetchDest !== 'document') { - return false; - } - return true; } } From 8a3cb0e3a7e004efbdb2bd5d22b7ff4c7df655bf Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Thu, 16 Apr 2026 17:53:12 +0200 Subject: [PATCH 3/6] allow storing multiple states, cleanup on failure is still missing Signed-off-by: Julien Veyssier --- lib/Controller/LoginController.php | 54 ++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 33e2e02a..79349da1 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -67,12 +67,24 @@ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class LoginController extends BaseOidcController { + // these keys (state, nonce, login_providerid, redirect, code_verifier, timestamp) + // are suffixed with the state value so they can be stored once per login flow private const STATE = 'oidc.state'; private const NONCE = 'oidc.nonce'; - public const PROVIDERID = 'oidc.providerid'; + // this is the provider ID we store during the login flow (set by login, get by code) + public const LOGIN_PROVIDERID = 'oidc.login.providerid'; public const REDIRECT_AFTER_LOGIN = 'oidc.redirect'; - private const ID_TOKEN = 'oidc.id_token'; private const CODE_VERIFIER = 'oidc.code_verifier'; + private const TIMESTAMP = 'oidc.timestamp'; + + // this is the provider ID we store once the authentication was successful + // it is used by the singleLogout endpoint and the user backend + public const PROVIDERID = 'oidc.providerid'; + // this id token is used to send id_token_hint to the IdP logout endpoint + private const ID_TOKEN = 'oidc.id_token'; + + // we consider that a login flow should complete within 5 minutes + private const LOGIN_FLOW_TIMEOUT = 300; public function __construct( IRequest $request, @@ -189,12 +201,15 @@ public function login(int $providerId, ?string $redirectUrl = null) { } $state = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); - $this->session->set(self::STATE, $state); + $sessionKeySuffix = '-' . $state; + $this->session->set(self::STATE . $sessionKeySuffix, $state); $this->logger->debug('Storing OIDC state', ['state' => $state]); - $this->session->set(self::REDIRECT_AFTER_LOGIN, $redirectUrl); + $timestamp = $this->timeFactory->getTime(); + $this->session->set(self::TIMESTAMP . $sessionKeySuffix, $timestamp); + $this->session->set(self::REDIRECT_AFTER_LOGIN . $sessionKeySuffix, $redirectUrl); $nonce = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); - $this->session->set(self::NONCE, $nonce); + $this->session->set(self::NONCE . $sessionKeySuffix, $nonce); $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); $isPkceSupported = in_array('S256', $discovery['code_challenge_methods_supported'] ?? [], true); @@ -203,10 +218,10 @@ public function login(int $providerId, ?string $redirectUrl = null) { if ($isPkceEnabled) { // PKCE code_challenge see https://datatracker.ietf.org/doc/html/rfc7636 $code_verifier = $this->random->generate(128, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_LOWER); - $this->session->set(self::CODE_VERIFIER, $code_verifier); + $this->session->set(self::CODE_VERIFIER . $sessionKeySuffix, $code_verifier); } - $this->session->set(self::PROVIDERID, $providerId); + $this->session->set(self::LOGIN_PROVIDERID . $sessionKeySuffix, $providerId); $this->session->close(); // get attribute mapping settings @@ -360,13 +375,22 @@ public function code(string $state = '', string $code = '', string $scope = '', return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, [], false); } - $storedState = $this->session->get(self::STATE); + $sessionKeySuffix = '-' . $state; + $storedState = $this->session->get(self::STATE . $sessionKeySuffix); + + $currentTimestamp = $this->timeFactory->getTime(); + $sessionTimestamp = $this->session->get(self::TIMESTAMP . $sessionKeySuffix); + if ($currentTimestamp - $sessionTimestamp > self::LOGIN_FLOW_TIMEOUT) { + // the state, nonce etc... were stored too long ago, the login flow has expired + $message = $this->l10n->t('The received state has expired.'); + return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); + } if ($storedState !== $state) { $this->logger->warning('state does not match', [ 'got' => $state, 'expected' => $storedState, - 'state_exists_in_session' => $this->session->exists(self::STATE), + 'state_exists_in_session' => $this->session->exists(self::STATE . $sessionKeySuffix), ]); $message = $this->l10n->t('The received state does not match the expected value.'); @@ -376,7 +400,7 @@ public function code(string $state = '', string $code = '', string $scope = '', 'error_description' => $message, 'got' => $state, 'expected' => $storedState, - 'state_exists_in_session' => $this->session->exists(self::STATE), + 'state_exists_in_session' => $this->session->exists(self::STATE . $sessionKeySuffix), ]; return new JSONResponse($responseData, Http::STATUS_FORBIDDEN); } @@ -384,7 +408,7 @@ public function code(string $state = '', string $code = '', string $scope = '', return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'state does not match'], true); } - $providerId = (int)$this->session->get(self::PROVIDERID); + $providerId = (int)$this->session->get(self::LOGIN_PROVIDERID . $sessionKeySuffix); $provider = $this->providerMapper->getProvider($providerId); try { $providerClientSecret = $this->crypto->decrypt($provider->getClientSecret()); @@ -409,7 +433,8 @@ public function code(string $state = '', string $code = '', string $scope = '', 'grant_type' => 'authorization_code', ]; if ($isPkceEnabled) { - $requestBody['code_verifier'] = $this->session->get(self::CODE_VERIFIER); // Set for the PKCE flow + // Set for the PKCE flow + $requestBody['code_verifier'] = $this->session->get(self::CODE_VERIFIER . $sessionKeySuffix); } $headers = []; @@ -558,7 +583,7 @@ public function code(string $state = '', string $code = '', string $scope = '', } } - if (isset($idTokenPayload->nonce) && $idTokenPayload->nonce !== $this->session->get(self::NONCE)) { + if (isset($idTokenPayload->nonce) && $idTokenPayload->nonce !== $this->session->get(self::NONCE . $sessionKeySuffix)) { $this->logger->debug('Nonce does not match'); $message = $this->l10n->t('The nonce does not match'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'invalid nonce']); @@ -627,6 +652,7 @@ public function code(string $state = '', string $code = '', string $scope = '', } $this->session->set(self::ID_TOKEN, $idTokenRaw); + $this->session->set(self::PROVIDERID, $providerId); $this->logger->debug('Logging user in'); @@ -692,7 +718,7 @@ public function code(string $state = '', string $code = '', string $scope = '', $this->logger->debug('Redirecting user'); - $redirectUrl = $this->session->get(self::REDIRECT_AFTER_LOGIN); + $redirectUrl = $this->session->get(self::REDIRECT_AFTER_LOGIN . $sessionKeySuffix); if ($redirectUrl) { return $this->getRedirectResponse($redirectUrl); } From f1b7775a15a0e605f96c093effc5d98a1c355544 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Fri, 17 Apr 2026 01:41:25 +0200 Subject: [PATCH 4/6] delete flow session values when the code endpoint fails or succeeds Signed-off-by: Julien Veyssier --- lib/Controller/LoginController.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 79349da1..40ad1121 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -382,6 +382,7 @@ public function code(string $state = '', string $code = '', string $scope = '', $sessionTimestamp = $this->session->get(self::TIMESTAMP . $sessionKeySuffix); if ($currentTimestamp - $sessionTimestamp > self::LOGIN_FLOW_TIMEOUT) { // the state, nonce etc... were stored too long ago, the login flow has expired + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The received state has expired.'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); } @@ -393,6 +394,7 @@ public function code(string $state = '', string $code = '', string $scope = '', 'state_exists_in_session' => $this->session->exists(self::STATE . $sessionKeySuffix), ]); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The received state does not match the expected value.'); if ($this->isDebugModeEnabled()) { $responseData = [ @@ -414,6 +416,7 @@ public function code(string $state = '', string $code = '', string $scope = '', $providerClientSecret = $this->crypto->decrypt($provider->getClientSecret()); } catch (\Exception $e) { $this->logger->error('Failed to decrypt the client secret', ['exception' => $e]); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('Failed to decrypt the OIDC provider client secret'); return $this->buildErrorTemplateResponse($message, Http::STATUS_BAD_REQUEST, [], false); } @@ -486,10 +489,12 @@ public function code(string $state = '', string $code = '', string $scope = '', $this->logger->debug('Failed to contact the OIDC provider token endpoint', ['exception' => $e]); $message = $this->l10n->t('Failed to contact the OIDC provider token endpoint'); } + $this->cleanupSessionState($sessionKeySuffix); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); } catch (\Exception $e) { $this->logger->debug('Failed to contact the OIDC provider token endpoint', ['exception' => $e]); $message = $this->l10n->t('Failed to contact the OIDC provider token endpoint'); + $this->cleanupSessionState($sessionKeySuffix); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); } @@ -501,12 +506,14 @@ public function code(string $state = '', string $code = '', string $scope = '', 'body' => $body, ]); $message = $this->l10n->t('Failed to contact the OIDC provider token endpoint'); + $this->cleanupSessionState($sessionKeySuffix); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); } if (!isset($data['id_token'])) { $this->logger->error('Missing id_token in IdP token response', ['keys' => array_keys($data)]); $message = $this->l10n->t('Failed to contact the OIDC provider token endpoint'); + $this->cleanupSessionState($sessionKeySuffix); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); } @@ -544,6 +551,7 @@ public function code(string $state = '', string $code = '', string $scope = '', if (!isset($idTokenPayload->exp) || $idTokenPayload->exp < $this->timeFactory->getTime()) { $this->logger->debug('Token expired'); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The received token is expired.'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'token expired']); } @@ -551,6 +559,7 @@ public function code(string $state = '', string $code = '', string $scope = '', // Verify issuer if (!isset($idTokenPayload->iss) || $idTokenPayload->iss !== $discovery['issuer']) { $this->logger->debug('This token is issued by the wrong issuer'); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The issuer does not match the one from the discovery endpoint'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_issuer' => $idTokenPayload->iss]); } @@ -566,6 +575,7 @@ public function code(string $state = '', string $code = '', string $scope = '', || (is_array($tokenAudience) && !in_array($providerClientId, $tokenAudience, true)) ) { $this->logger->debug('This token is not for us'); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The audience does not match ours'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_audience' => $idTokenPayload->aud]); } @@ -578,6 +588,7 @@ public function code(string $state = '', string $code = '', string $scope = '', // If the azp claim is present, it should be the client ID if (isset($idTokenPayload->azp) && $idTokenPayload->azp !== $provider->getClientId()) { $this->logger->debug('This token is not for us, authorized party (azp) is different than the client ID'); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The authorized party does not match ours'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['invalid_azp' => $idTokenPayload->azp]); } @@ -585,6 +596,7 @@ public function code(string $state = '', string $code = '', string $scope = '', if (isset($idTokenPayload->nonce) && $idTokenPayload->nonce !== $this->session->get(self::NONCE . $sessionKeySuffix)) { $this->logger->debug('Nonce does not match'); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('The nonce does not match'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'invalid nonce']); } @@ -594,6 +606,7 @@ public function code(string $state = '', string $code = '', string $scope = '', $userId = $this->provisioningService->getClaimValue($idTokenPayload, $uidAttribute, $providerId); if ($userId === null) { + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('Failed to provision the user'); return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'failed to provision user']); } @@ -605,6 +618,7 @@ public function code(string $state = '', string $code = '', string $scope = '', if ($syncGroups === null || count($syncGroups) === 0) { $this->logger->debug('Prevented user from login as user is not part of a whitelisted group'); + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('You do not have permission to log in to this instance. If you think this is an error, please contact an administrator.'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'user not in any whitelisted group']); } @@ -630,6 +644,7 @@ public function code(string $state = '', string $code = '', string $scope = '', if (!$softAutoProvisionAllowed && $existingUser !== null && $existingUser->getBackendClassName() !== Application::APP_ID) { // if soft auto-provisioning is disabled, // we refuse login for a user that already exists in another backend + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('User conflict'); return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); } @@ -647,6 +662,7 @@ public function code(string $state = '', string $code = '', string $scope = '', } if ($user === null) { + $this->cleanupSessionState($sessionKeySuffix); $message = $this->l10n->t('Failed to provision the user'); return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'failed to provision user']); } @@ -719,6 +735,7 @@ public function code(string $state = '', string $code = '', string $scope = '', $this->logger->debug('Redirecting user'); $redirectUrl = $this->session->get(self::REDIRECT_AFTER_LOGIN . $sessionKeySuffix); + $this->cleanupSessionState($sessionKeySuffix); if ($redirectUrl) { return $this->getRedirectResponse($redirectUrl); } @@ -998,4 +1015,16 @@ private function toCodeChallenge(string $data): string { $s = str_replace('/', '_', $s); // 63rd char of encoding return $s; } + + /** + * Clean up session values for a given state suffix + */ + private function cleanupSessionState(string $sessionKeySuffix): void { + $this->session->remove(self::STATE . $sessionKeySuffix); + $this->session->remove(self::NONCE . $sessionKeySuffix); + $this->session->remove(self::LOGIN_PROVIDERID . $sessionKeySuffix); + $this->session->remove(self::REDIRECT_AFTER_LOGIN . $sessionKeySuffix); + $this->session->remove(self::CODE_VERIFIER . $sessionKeySuffix); + $this->session->remove(self::TIMESTAMP . $sessionKeySuffix); + } } From 08a4855d5a3c3a6ea4bdef019bc0dfb32c0f35cd Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Fri, 17 Apr 2026 01:51:28 +0200 Subject: [PATCH 5/6] check if user is logged in at the beginning of the code endpoint. if so, redirect to the redirect URL and do nothing Signed-off-by: Julien Veyssier --- lib/Controller/LoginController.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 40ad1121..b44bab66 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -358,6 +358,12 @@ public function login(int $providerId, ?string $redirectUrl = null) { #[UseSession] #[BruteForceProtection(action: 'userOidcCode')] public function code(string $state = '', string $code = '', string $scope = '', string $error = '', string $error_description = '') { + if ($this->userSession->isLoggedIn()) { + $sessionKeySuffix = '-' . $state; + $redirectUrl = $this->session->get(self::REDIRECT_AFTER_LOGIN . $sessionKeySuffix); + $this->cleanupSessionState($sessionKeySuffix); + return $this->getRedirectResponse(!empty($redirectUrl) ? $redirectUrl : null); + } if (!$this->isSecure()) { return $this->buildProtocolErrorResponse(); } From ca67713568445ce307c69ffbb7f0bc37c722df49 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Fri, 17 Apr 2026 01:56:01 +0200 Subject: [PATCH 6/6] adjust isTopLevelHtmlNavigation tests Signed-off-by: Julien Veyssier --- .../RequestClassificationServiceTest.php | 40 +++---------------- 1 file changed, 5 insertions(+), 35 deletions(-) diff --git a/tests/unit/Service/RequestClassificationServiceTest.php b/tests/unit/Service/RequestClassificationServiceTest.php index 44dfaab0..82df5f04 100644 --- a/tests/unit/Service/RequestClassificationServiceTest.php +++ b/tests/unit/Service/RequestClassificationServiceTest.php @@ -28,58 +28,28 @@ public function testIsTopLevelHtmlNavigation(string $method, array $headers, boo public static function topLevelHtmlNavigationProvider(): array { return [ - 'top level navigation with html accept' => [ + 'top level navigation' => [ 'GET', - [ - 'Accept' => 'text/html,application/xhtml+xml', - 'Sec-Fetch-Mode' => 'navigate', - 'Sec-Fetch-Dest' => 'document', - ], - true, - ], - 'html accept without fetch metadata' => [ - 'GET', - [ - 'Accept' => 'text/html', - ], + [], true, ], 'xhr request' => [ 'GET', [ - 'Accept' => 'text/html', 'X-Requested-With' => 'XMLHttpRequest', ], false, ], - 'json request' => [ + 'ocs api request' => [ 'GET', [ - 'Accept' => 'application/json', - ], - false, - ], - 'non navigate fetch mode' => [ - 'GET', - [ - 'Accept' => 'text/html', - 'Sec-Fetch-Mode' => 'cors', - ], - false, - ], - 'non document destination' => [ - 'GET', - [ - 'Accept' => 'text/html', - 'Sec-Fetch-Dest' => 'empty', + 'OCS-apirequest' => 'true', ], false, ], 'non get request' => [ 'POST', - [ - 'Accept' => 'text/html', - ], + [], false, ], ];