From 3898c61d04659a47e28d53e6138aa45fd2d2f46a Mon Sep 17 00:00:00 2001 From: Spitap Date: Tue, 28 Apr 2026 19:06:55 +0200 Subject: [PATCH 1/5] Improved logging for failed BC logout refurbished $throttleMetadata not used since 9b5d6c6 Signed-off-by: Spitap --- lib/Controller/LoginController.php | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index e5e67d13..fff29791 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -863,7 +863,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'provider not found', 'The provider was not found in Nextcloud', - ['provider_not_found' => $providerIdentifier] + ['severity' => 'warning', 'extra_context' => 'Got provider identifier: ' . $providerIdentifier] ); } @@ -882,7 +882,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'invalid audience', 'The audience of the logout token does not match the provider', - ['invalid_audience' => $logoutTokenPayload->aud] + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] ); } @@ -891,7 +891,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'invalid event', 'The backchannel-logout event was not found in the logout token', - ['invalid_event' => true] + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] ); } @@ -900,7 +900,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'invalid nonce', 'The logout token should not contain a nonce attribute', - ['nonce_should_not_be_set' => true] + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] ); } @@ -908,7 +908,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'invalid iss', 'The logout token should contain an iss attribute', - ['iss_should_be_set' => true] + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] ); } $iss = $logoutTokenPayload->iss; @@ -917,7 +917,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'invalid sid+sub', 'The logout token should contain sid or sub or both', - ['no_sid_no_sub' => true] + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] ); } @@ -941,7 +941,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( $sub === null ? 'invalid SID or ISS' : 'invalid SID, SUB or ISS', $sub === null ? 'Multiple sessions were found with this (sid,iss)' : 'Multiple sessions were found with this (sid,sub,iss)', - ['multiple_sessions_found' => $sid] + ['severity' => 'error', 'extra_context' => 'Probably is a Nextcloud side issue'] ); } $oidcSessionsToKill[] = $oidcSession; @@ -954,7 +954,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok return $this->getBackchannelLogoutErrorResponse( 'error with sub+iss', 'Failed to retrieve session with sub+iss', - ['sub_iss_error' => true] + ['exception' => $e] ); } @@ -998,15 +998,21 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok * * @param string $error * @param string $description - * @param array $throttleMetadata + * @param array $metadata * @return JSONResponse */ private function getBackchannelLogoutErrorResponse( string $error, string $description, - array $throttleMetadata = [], + array $metadata, ): JSONResponse { - $this->logger->debug('Backchannel logout error. ' . $error . ' ; ' . $description); + $logSeverity = 'debug'; + if (isset($metadata["severity"])) { + $logSeverity = $metadata["severity"]; + unset($metadata["severity"]); + } + $this->logger->log($logSeverity, 'Backchannel logout error. ' . $error . ' ; ' . $description, + $metadata); return new JSONResponse( [ 'error' => $error, From a5720bf3587a95efc0187ea9b64d95eebc8a1e60 Mon Sep 17 00:00:00 2001 From: Spitap Date: Wed, 29 Apr 2026 13:12:44 +0200 Subject: [PATCH 2/5] Fixed DB exception and clearer logging Signed-off-by: Spitap --- lib/Controller/LoginController.php | 33 +++++++++++------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index fff29791..52782e3e 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -846,6 +846,9 @@ public function singleLogoutService() { * Endpoint called by the IdP (OP) when end_session_endpoint is called by another client * The logout token contains the sid for which we know the sessionId * which leads to the auth token that we can invalidate + * Note : in a RP-initiated logout scenario + * the invalidation step should not be required since it would have been cleared + * in singleLogoutService() * Implemented according to https://openid.net/specs/openid-connect-backchannel-1_0.html * * @param string $providerIdentifier @@ -929,42 +932,30 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok $sub = $logoutTokenPayload->sub ?? null; try { $oidcSession = $this->sessionMapper->findSessionBySid($sid, $sub, $iss); + $oidcSessionsToKill[] = $oidcSession; } catch (DoesNotExistException $e) { // Already-logged-out is a success per OIDC Backchannel Logout 1.0 §2.6. // https://openid.net/specs/openid-connect-backchannel-1_0.html#BCActions - $this->logger->debug( - '[BackchannelLogout] no RP session for (sid,iss) — treating as already-logged-out', - ['sid' => $sid, 'sub_present' => $sub !== null] - ); - return new JSONResponse([], Http::STATUS_OK); + $this->logger->debug('[BackchannelLogout] OIDC session not found with sid+sub+iss (expected for a RP-initiated logout)'); } catch (MultipleObjectsReturnedException $e) { - return $this->getBackchannelLogoutErrorResponse( - $sub === null ? 'invalid SID or ISS' : 'invalid SID, SUB or ISS', - $sub === null ? 'Multiple sessions were found with this (sid,iss)' : 'Multiple sessions were found with this (sid,sub,iss)', - ['severity' => 'error', 'extra_context' => 'Probably is a Nextcloud side issue'] + $this->logger->warning('[BackchannelLogout] Multiple OIDC sessions retrieved (sid+sub+iss). ' + . 'This should not happen. Please check that you have created your DB indexes' ); } - $oidcSessionsToKill[] = $oidcSession; } else { // here we know the sid is not set so the sub is set $sub = $logoutTokenPayload->sub; try { $oidcSessionsToKill = $this->sessionMapper->findSessionsBySubAndIss($sub, $iss); - } catch (\OCP\Db\Exception $e) { - return $this->getBackchannelLogoutErrorResponse( - 'error with sub+iss', - 'Failed to retrieve session with sub+iss', - ['exception' => $e] - ); + } catch (\OCP\DB\Exception $e) { + $this->logger->error( + '[BackchannelLogout] Database failure while trying to retrieve user session (sub+iss)' + . ['exception' => $e]); } if (empty($oidcSessionsToKill)) { // Already-logged-out is a success per OIDC Backchannel Logout 1.0 §2.6. - $this->logger->debug( - '[BackchannelLogout] no RP sessions for (sub,iss) — treating as already-logged-out', - ['sub' => $sub] - ); - return new JSONResponse([], Http::STATUS_OK); + $this->logger->debug('[BackchannelLogout] OIDC session not found with sub+iss (expected for a RP-initiated logout)'); } } From df30c88a68895d30129bc05d714c1ff7975bb707 Mon Sep 17 00:00:00 2001 From: Spitap Date: Tue, 28 Apr 2026 20:19:07 +0200 Subject: [PATCH 3/5] tell idp not to cache BC logout response Signed-off-by: Spitap --- lib/Controller/LoginController.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 52782e3e..28449b3b 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -980,7 +980,12 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok $this->sessionMapper->delete($oidcSession); } - return new JSONResponse([], Http::STATUS_OK); + // Tell the Idp not to cache the response + // Per RFC : https://openid.net/specs/openid-connect-backchannel-1_0.html#BCResponse + $response = new JSONResponse([], Http::STATUS_OK); + $response->cacheFor(0); + + return $response; } /** @@ -1004,13 +1009,18 @@ private function getBackchannelLogoutErrorResponse( } $this->logger->log($logSeverity, 'Backchannel logout error. ' . $error . ' ; ' . $description, $metadata); - return new JSONResponse( + + $response = new JSONResponse( [ 'error' => $error, 'error_description' => $description, ], Http::STATUS_BAD_REQUEST, ); + // Tell the Idp not to cache the response + // Per RFC : https://openid.net/specs/openid-connect-backchannel-1_0.html#BCResponse + $response->cacheFor(0); + return $response; } private function toCodeChallenge(string $data): string { From a31cb16e410d904584b25d7d5685a3b1208ceaf3 Mon Sep 17 00:00:00 2001 From: Spitap Date: Fri, 1 May 2026 12:13:36 +0200 Subject: [PATCH 4/5] Check for the required claims Signed-off-by: Spitap --- lib/Controller/LoginController.php | 33 ++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 28449b3b..bbe220ad 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -877,6 +877,27 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok $this->logger->debug('Parsed the logout JWT payload: ' . json_encode($logoutTokenPayload, JSON_THROW_ON_ERROR)); + // REQUIRED claims check step + // https://openid.net/specs/openid-connect-backchannel-1_0.html#LogoutToken + $requiredClaims = ['iss', 'aud', 'iat', 'exp', 'jti', 'events']; + $missingClaims = []; + $logoutTokenArray = (array) $logoutTokenPayload; + foreach ($requiredClaims as $claim) { + if (!in_array($claim, $logoutTokenArray)) { + $missingClaims[] = $claim; + } + } + if (!empty($missingClaims)) { + return $this->getBackchannelLogoutErrorResponse( + 'missing one or more claims', + 'missing the following claim(s) : ' . implode(', ', $missingClaims), + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] + ); + } + + // Logout token validation step + // https://openid.net/specs/openid-connect-backchannel-1_0.html#Validation + // check the audience $aud = $logoutTokenPayload->aud; $clientId = $provider->getClientId(); @@ -890,7 +911,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok } // check the event attr - if (!isset($logoutTokenPayload->events->{'http://schemas.openid.net/event/backchannel-logout'})) { + if (!$logoutTokenPayload->events->{'http://schemas.openid.net/event/backchannel-logout'}) { return $this->getBackchannelLogoutErrorResponse( 'invalid event', 'The backchannel-logout event was not found in the logout token', @@ -907,13 +928,6 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok ); } - if (!isset($logoutTokenPayload->iss)) { - return $this->getBackchannelLogoutErrorResponse( - 'invalid iss', - 'The logout token should contain an iss attribute', - ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] - ); - } $iss = $logoutTokenPayload->iss; if (!isset($logoutTokenPayload->sid) && !isset($logoutTokenPayload->sub)) { @@ -950,7 +964,8 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok } catch (\OCP\DB\Exception $e) { $this->logger->error( '[BackchannelLogout] Database failure while trying to retrieve user session (sub+iss)' - . ['exception' => $e]); + . ['exception' => $e] + ); } if (empty($oidcSessionsToKill)) { From f209fb602855aef639c8a279bebe6f167d16ab83 Mon Sep 17 00:00:00 2001 From: Spitap Date: Wed, 29 Apr 2026 17:34:23 +0200 Subject: [PATCH 5/5] added verification for exp and iss in BC-LO Signed-off-by: Spitap --- lib/Controller/LoginController.php | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index bbe220ad..7377ab3b 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -870,6 +870,17 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok ); } + try { + $discovery = $this->discoveryService->obtainDiscovery($provider); + } catch (\Exception $e) { + $this->logger->error('Could not reach the provider at URL ' . $provider->getDiscoveryEndpoint(), ['exception' => $e]); + return $this->getBackchannelLogoutErrorResponse( + 'could not reach provider endpoint', + 'URL: ' . $provider->getDiscoveryEndpoint() . 'was not reachable', + ['severity' => 'error'] + ); + } + // decrypt the logout token $jwks = $this->discoveryService->obtainJWK($provider, $logout_token); JWT::$leeway = 60; @@ -929,6 +940,21 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok } $iss = $logoutTokenPayload->iss; + if ($iss !== $discovery['issuer']) { + return $this->getBackchannelLogoutErrorResponse( + 'invalid iss', + 'The iss of the logout token does not match the issuer', + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] + ); + } + + if (!isset($logoutTokenPayload->exp) || $logoutTokenPayload->exp < $this->timeFactory->getTime()) { + return $this->getBackchannelLogoutErrorResponse( + 'invalid exp', + 'The logout token is expired', + ['severity' => 'warning', 'extra_context' => 'Probably is an IdP side issue'] + ); + } if (!isset($logoutTokenPayload->sid) && !isset($logoutTokenPayload->sub)) { return $this->getBackchannelLogoutErrorResponse(