From ae5e5b76d638ad7be40a383859ca35b3faaeeadc Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Tue, 12 Nov 2024 09:40:19 +0100 Subject: [PATCH 1/5] Fix backchannel logout changes --- lib/Controller/LoginController.php | 43 +++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 970ef16e..49f11f9f 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -503,7 +503,8 @@ public function code(string $state = '', string $code = '', string $scope = '', try { $authToken = $this->authTokenProvider->getToken($this->session->getId()); $this->sessionMapper->createSession( - $idTokenPayload->sid ?? 'fallback-sid', + //$idTokenPayload->sid ?? 'fallback-sid', + $idTokenPayload->{'urn:telekom.com:session_token'} ?? 'fallback-sid', $idTokenPayload->sub ?? 'fallback-sub', $idTokenPayload->iss ?? 'fallback-iss', $authToken->getId(), @@ -578,7 +579,9 @@ public function singleLogoutService() { } // cleanup related oidc session - $this->sessionMapper->deleteFromNcSessionId($this->session->getId()); + // it is not a good idea to remove the session early as some IDM send a backchannel logout also to the initiating system. + // This will falsely fail if already deleted. So rely always on backchannel cleanup or make this an option? + // $this->sessionMapper->deleteFromNcSessionId($this->session->getId()); $this->userSession->logout(); @@ -665,8 +668,10 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok ); } - $sub = $logoutTokenPayload->sub; - if ($oidcSession->getSub() !== $sub) { + // $sub = $logoutTokenPayload->sub; + // if ($oidcSession->getSub() !== $sub) { + // handle sub only if it is available; session is enough to identify a logout + if (isset($logoutTokenPayload->sub) && ($oidcSession->getSub() !== $logoutTokenPayload->sub)) { return $this->getBackchannelLogoutErrorResponse( 'invalid SUB', 'The sub does not match the one from the login ID token', @@ -691,17 +696,19 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok $userId = $authToken->getUID(); $this->authTokenProvider->invalidateTokenById($userId, $authToken->getId()); } catch (InvalidTokenException $e) { - return $this->getBackchannelLogoutErrorResponse( - 'nc session not found', - 'The authentication session was not found in Nextcloud', - ['nc_auth_session_not_found' => $authTokenId] - ); + // it is not a problem if the auth token is already deleted, so no error + // return $this->getBackchannelLogoutErrorResponse( + // 'nc session not found', + // 'The authentication session was not found in Nextcloud', + // ['nc_auth_session_not_found' => $authTokenId] + // ); } // cleanup $this->sessionMapper->delete($oidcSession); - return new JSONResponse([], Http::STATUS_OK); + // return new JSONResponse([], Http::STATUS_OK); + return new JSONResponse(); } /** @@ -735,4 +742,20 @@ private function toCodeChallenge(string $data): string { $s = str_replace('/', '_', $s); // 63rd char of encoding return $s; } + + /** + * Backward compatible function for MagentaCLOUD to smoothly transition to new config + * + * @PublicPage + * @NoCSRFRequired + * @BruteForceProtection(action=userOidcBackchannelLogout) + * + * @param string $logout_token + * @return JSONResponse + * @throws Exception + * @throws \JsonException + */ + public function telekomBackChannelLogout(string $logout_token = '') { + return $this->backChannelLogout('Telekom', $logout_token); + } } From e6d118eca0b3b7cbd795bcda1604b431c15f2bcd Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Tue, 12 Nov 2024 09:52:53 +0100 Subject: [PATCH 2/5] Fix OIDC state mismatch --- lib/Controller/LoginController.php | 34 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 49f11f9f..629cdcc3 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -162,12 +162,26 @@ public function login(int $providerId, ?string $redirectUrl = null) { return $this->buildErrorTemplateResponse($message, Http::STATUS_NOT_FOUND, ['reason' => 'provider unreachable']); } - $state = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); - $this->session->set(self::STATE, $state); - $this->session->set(self::REDIRECT_AFTER_LOGIN, $redirectUrl); + // $state = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); + // $this->session->set(self::STATE, $state); + // $this->session->set(self::REDIRECT_AFTER_LOGIN, $redirectUrl); - $nonce = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); - $this->session->set(self::NONCE, $nonce); + // $nonce = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); + // $this->session->set(self::NONCE, $nonce); + + // check if oidc state is present in session data + if ($this->session->exists(self::STATE)) { + $state = $this->session->get(self::STATE); + $nonce = $this->session->get(self::NONCE); + } else { + $state = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); + $this->session->set(self::STATE, $state); + $this->session->set(self::REDIRECT_AFTER_LOGIN, $redirectUrl); + + $nonce = $this->random->generate(32, ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_UPPER); + $this->session->set(self::NONCE, $nonce); + $this->session->set(self::PROVIDERID, $providerId); + } $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); $isPkceSupported = in_array('S256', $discovery['code_challenge_methods_supported'] ?? [], true); @@ -179,7 +193,7 @@ public function login(int $providerId, ?string $redirectUrl = null) { $this->session->set(self::CODE_VERIFIER, $code_verifier); } - $this->session->set(self::PROVIDERID, $providerId); + // $this->session->set(self::PROVIDERID, $providerId); $this->session->close(); // get attribute mapping settings @@ -496,6 +510,10 @@ public function code(string $state = '', string $code = '', string $scope = '', $this->userSession->createRememberMeToken($user); } + // remove code login session values + $this->session->remove(self::STATE); + $this->session->remove(self::NONCE); + // Set last password confirm to the future as we don't have passwords to confirm against with SSO $this->session->set('last-password-confirm', strtotime('+4 year', time())); @@ -668,10 +686,10 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok ); } - // $sub = $logoutTokenPayload->sub; + $sub = $logoutTokenPayload->sub; // if ($oidcSession->getSub() !== $sub) { // handle sub only if it is available; session is enough to identify a logout - if (isset($logoutTokenPayload->sub) && ($oidcSession->getSub() !== $logoutTokenPayload->sub)) { + if (isset($logoutTokenPayload->sub) && ($oidcSession->getSub() !== $sub)) { return $this->getBackchannelLogoutErrorResponse( 'invalid SUB', 'The sub does not match the one from the login ID token', From d1cfd565946d793b0a26b0bd97e67fb4f59c5d18 Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Thu, 14 Nov 2024 10:23:11 +0100 Subject: [PATCH 3/5] fixed code style --- lib/Controller/LoginController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 629cdcc3..aab0a213 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -597,7 +597,7 @@ public function singleLogoutService() { } // cleanup related oidc session - // it is not a good idea to remove the session early as some IDM send a backchannel logout also to the initiating system. + // it is not a good idea to remove the session early as some IDM send a backchannel logout also to the initiating system. // This will falsely fail if already deleted. So rely always on backchannel cleanup or make this an option? // $this->sessionMapper->deleteFromNcSessionId($this->session->getId()); @@ -716,9 +716,9 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok } catch (InvalidTokenException $e) { // it is not a problem if the auth token is already deleted, so no error // return $this->getBackchannelLogoutErrorResponse( - // 'nc session not found', - // 'The authentication session was not found in Nextcloud', - // ['nc_auth_session_not_found' => $authTokenId] + // 'nc session not found', + // 'The authentication session was not found in Nextcloud', + // ['nc_auth_session_not_found' => $authTokenId] // ); } From d67ca045ca577f4950aad824016e0895bebfc2f6 Mon Sep 17 00:00:00 2001 From: memurats Date: Tue, 4 Feb 2025 09:03:47 +0100 Subject: [PATCH 4/5] fix errors --- lib/Controller/LoginController.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 4eb2d68d..8eebbc21 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -538,12 +538,12 @@ public function code(string $state = '', string $code = '', string $scope = '', // $tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true); // if ($tokenExchangeEnabled) { - // store all token information for potential token exchange requests - // $tokenData = array_merge( - // $data, - // ['provider_id' => $providerId], - // ); - // $this->tokenService->storeToken($tokenData); + // store all token information for potential token exchange requests + // $tokenData = array_merge( + // $data, + // ['provider_id' => $providerId], + // ); + // $this->tokenService->storeToken($tokenData); // } // $this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1'); From f20568598746b4d7f747d014e8bfc4b4a7fae01e Mon Sep 17 00:00:00 2001 From: memurats Date: Thu, 6 Feb 2025 09:34:55 +0100 Subject: [PATCH 5/5] fix merge error --- lib/Controller/LoginController.php | 32 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 8eebbc21..9adef78d 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -780,6 +780,22 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return new JSONResponse(); } + /** + * Backward compatible function for MagentaCLOUD to smoothly transition to new config + * + * @PublicPage + * @NoCSRFRequired + * @BruteForceProtection(action=userOidcBackchannelLogout) + * + * @param string $logout_token + * @return JSONResponse + * @throws Exception + * @throws \JsonException + */ + public function telekomBackChannelLogout(string $logout_token = '') { + return $this->backChannelLogout('Telekom', $logout_token); + } + /** * Generate an error response according to the OIDC standard * Log the error @@ -811,20 +827,4 @@ private function toCodeChallenge(string $data): string { $s = str_replace('/', '_', $s); // 63rd char of encoding return $s; } - - /** - * Backward compatible function for MagentaCLOUD to smoothly transition to new config - * - * @PublicPage - * @NoCSRFRequired - * @BruteForceProtection(action=userOidcBackchannelLogout) - * - * @param string $logout_token - * @return JSONResponse - * @throws Exception - * @throws \JsonException - */ - public function telekomBackChannelLogout(string $logout_token = '') { - return $this->backChannelLogout('Telekom', $logout_token); - } }