From c0322841a449d12a4ec102df2568a2e6fa9b1134 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 20 Aug 2025 16:47:13 +0200 Subject: [PATCH 1/5] feat(revoke-token): react to a user session being revoked, call the IdP's end_session_endpoint to make sure it's not possible to login again from the invalidated browser session Signed-off-by: Julien Veyssier --- appinfo/info.xml | 2 +- lib/AppInfo/Application.php | 5 + lib/Controller/LoginController.php | 15 ++- lib/Db/Provider.php | 26 ++-- lib/Db/Session.php | 66 ++++++---- lib/Db/SessionMapper.php | 48 +++++++- lib/Helper/HttpClientHelper.php | 6 +- lib/Listener/TokenInvalidatedListener.php | 116 ++++++++++++++++++ .../Version070400Date20250820141709.php | 67 ++++++++++ psalm.xml | 1 + tests/stubs/ocp_token_invalidated_event.php | 13 ++ 11 files changed, 315 insertions(+), 50 deletions(-) create mode 100644 lib/Listener/TokenInvalidatedListener.php create mode 100644 lib/Migration/Version070400Date20250820141709.php create mode 100644 tests/stubs/ocp_token_invalidated_event.php diff --git a/appinfo/info.xml b/appinfo/info.xml index df4bdf9d1..8f0be565a 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -8,7 +8,7 @@ OpenID Connect user backend Use an OpenID Connect backend to login to your Nextcloud Allows flexible configuration of an OIDC server as Nextcloud login user backend. - 7.3.2 + 7.4.0 agpl Roeland Jago Douma Julius Härtl diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index b6b06fde7..a036c09a8 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -19,6 +19,7 @@ use OCA\UserOIDC\Listener\ExternalTokenRequestedListener; use OCA\UserOIDC\Listener\InternalTokenRequestedListener; use OCA\UserOIDC\Listener\TimezoneHandlingListener; +use OCA\UserOIDC\Listener\TokenInvalidatedListener; use OCA\UserOIDC\Service\ID4MeService; use OCA\UserOIDC\Service\SettingsService; use OCA\UserOIDC\Service\TokenService; @@ -59,6 +60,10 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(ExchangedTokenRequestedEvent::class, ExchangedTokenRequestedListener::class); $context->registerEventListener(ExternalTokenRequestedEvent::class, ExternalTokenRequestedListener::class); $context->registerEventListener(InternalTokenRequestedEvent::class, InternalTokenRequestedListener::class); + + if (class_exists(\OCP\Authentication\Events\TokenInvalidatedEvent::class)) { + $context->registerEventListener(\OCP\Authentication\Events\TokenInvalidatedEvent::class, TokenInvalidatedListener::class); + } } public function boot(IBootContext $context): void { diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 97c9c4247..3be170d40 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -615,7 +615,10 @@ public function code(string $state = '', string $code = '', string $scope = '', $idTokenPayload->sub ?? 'fallback-sub', $idTokenPayload->iss ?? 'fallback-iss', $authToken->getId(), - $this->session->getId() + $this->session->getId(), + $idTokenRaw, + $user->getUID(), + $providerId, ); } catch (InvalidTokenException $e) { $this->logger->debug('Auth token not found after login'); @@ -687,7 +690,7 @@ public function singleLogoutService() { return $this->buildErrorTemplateResponse($message, Http::STATUS_NOT_FOUND, ['provider_id' => $providerId]); } - // Check if a custom end_session_endpoint is deposited otherwise use the default one provided by the openid-configuration + // Check if a custom end_session_endpoint is set in the provider otherwise use the default one provided by the openid-configuration $discoveryData = $this->discoveryService->obtainDiscovery($provider); $defaultEndSessionEndpoint = $discoveryData['end_session_endpoint'] ?? null; $customEndSessionEndpoint = $provider->getEndSessionEndpoint(); @@ -842,8 +845,12 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok } foreach ($oidcSessionsToKill as $oidcSession) { - // i don't know why but the cast is necessary - $authTokenId = (int)$oidcSession->getAuthtokenId(); + // we know the IdP session is closed + // we need this to prevent requesting the end_session_endpoint when we catch the TokenInvalidatedEvent + $oidcSession->setIdpSessionClosed(1); + $this->sessionMapper->update($oidcSession); + + $authTokenId = $oidcSession->getAuthtokenId(); try { $authToken = $this->authTokenProvider->getTokenById($authTokenId); // we could also get the auth token by nc session ID diff --git a/lib/Db/Provider.php b/lib/Db/Provider.php index 4399b8f34..9db25fa0c 100644 --- a/lib/Db/Provider.php +++ b/lib/Db/Provider.php @@ -11,17 +11,17 @@ use OCP\AppFramework\Db\Entity; /** - * @method string getIdentifier() - * @method void setIdentifier(string $identifier) - * @method string getClientId() - * @method void setClientId(string $clientId) - * @method string getClientSecret() - * @method void setClientSecret(string $clientSecret) - * @method string getDiscoveryEndpoint() - * @method void setDiscoveryEndpoint(string $discoveryEndpoint) - * @method string getEndSessionEndpoint() - * @method void setEndSessionEndpoint(string $endSessionEndpoint) - * @method void setScope(string $scope) + * @method \string getIdentifier() + * @method \void setIdentifier(string $identifier) + * @method \string getClientId() + * @method \void setClientId(string $clientId) + * @method \string getClientSecret() + * @method \void setClientSecret(string $clientSecret) + * @method \string|\null getDiscoveryEndpoint() + * @method \void setDiscoveryEndpoint(?string $discoveryEndpoint) + * @method \string|\null getEndSessionEndpoint() + * @method \void setEndSessionEndpoint(?string $endSessionEndpoint) + * @method \void setScope(string $scope) */ class Provider extends Entity implements \JsonSerializable { @@ -34,10 +34,10 @@ class Provider extends Entity implements \JsonSerializable { /** @var string */ protected $clientSecret; - /** @var string */ + /** @var ?string */ protected $discoveryEndpoint; - /** @var string */ + /** @var ?string */ protected $endSessionEndpoint; /** @var string */ diff --git a/lib/Db/Session.php b/lib/Db/Session.php index b75e3a515..943427563 100644 --- a/lib/Db/Session.php +++ b/lib/Db/Session.php @@ -11,38 +11,49 @@ use OCP\AppFramework\Db\Entity; /** - * @method string getSid() - * @method void setSid(string $sid) - * @method string getSub() - * @method void setSub(string $sub) - * @method string getIss() - * @method void setIss(string $iss) - * @method int getAuthtokenId() - * @method void setAuthtokenId(int $authtokenId) - * @method string getNcSessionId() - * @method void setNcSessionId(string $ncSessionId) - * @method int getCreatedAt() - * @method void setCreatedAt(int $createdAt) + * @method \string getSid() + * @method \void setSid(string $sid) + * @method \string getSub() + * @method \void setSub(string $sub) + * @method \string getIss() + * @method \void setIss(string $iss) + * @method \int getAuthtokenId() + * @method \void setAuthtokenId(int $authtokenId) + * @method \string getNcSessionId() + * @method \void setNcSessionId(string $ncSessionId) + * @method \int getCreatedAt() + * @method \void setCreatedAt(int $createdAt) + * @method \string|\null getIdToken() + * @method \void setIdToken(?string $idToken) + * @method \string|\null getUserId() + * @method \void setUserId(?string $userId) + * @method \int getProviderId() + * @method \void setProviderId(int $providerId) + * @method \int getIdpSessionClosed() + * @method \void setIdpSessionClosed(int $idpSessionClosed) */ class Session extends Entity implements \JsonSerializable { /** @var string */ protected $sid; - /** @var string */ protected $sub; - /** @var string */ protected $iss; - /** @var int */ protected $authtokenId; - /** @var string */ protected $ncSessionId; - /** @var int */ protected $createdAt; + /** @var ?string */ + protected $idToken; + /** @var ?string */ + protected $userId; + /** @var int */ + protected $providerId; + /** @var int */ + protected $idpSessionClosed; public function __construct() { $this->addType('sid', 'string'); @@ -51,18 +62,25 @@ public function __construct() { $this->addType('authtoken_id', 'integer'); $this->addType('nc_session_id', 'string'); $this->addType('created_at', 'integer'); + $this->addType('id_token', 'string'); + $this->addType('user_id', 'string'); + $this->addType('provider_id', 'integer'); + $this->addType('idp_session_closed', 'integer'); } #[\ReturnTypeWillChange] public function jsonSerialize() { return [ - 'id' => $this->id, - 'sid' => $this->sid, - 'sub' => $this->sub, - 'iss' => $this->iss, - 'authtoken_id' => $this->authtokenId, - 'nc_session_id' => $this->ncSessionId, - 'created_at' => $this->createdAt, + 'id' => $this->getId(), + 'sid' => $this->getSid(), + 'sub' => $this->getSub(), + 'iss' => $this->getIss(), + 'authtoken_id' => $this->getAuthtokenId(), + 'nc_session_id' => $this->getNcSessionId(), + 'created_at' => $this->getCreatedAt(), + 'user_id' => $this->getUserId(), + 'provider_id' => $this->getProviderId(), + 'idp_session_closed' => $this->getIdpSessionClosed() !== 0, ]; } } diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index 5089732e1..f2e8c120f 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -28,8 +28,9 @@ public function __construct(IDBConnection $db) { /** * @param int $id * @return Session - * @throws \OCP\AppFramework\Db\DoesNotExistException - * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws DoesNotExistException + * @throws Exception + * @throws MultipleObjectsReturnedException */ public function getSession(int $id): Session { $qb = $this->db->getQueryBuilder(); @@ -99,10 +100,33 @@ public function findSessionBySid(string $sid, ?string $sub = null, ?string $iss return $this->findEntity($qb); } + /** + * @param int $authTokenId + * @param string $userId + * @return Session + * @throws DoesNotExistException + * @throws Exception + * @throws MultipleObjectsReturnedException + */ + public function getSessionByAuthTokenAndUid(int $authTokenId, string $userId): Session { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('authtoken_id', $qb->createNamedParameter($authTokenId, IQueryBuilder::PARAM_INT)) + ) + ->andWhere( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + ); + + return $this->findEntity($qb); + } + /** * @param string $ncSessionId * @return int - * @throws \OCP\DB\Exception + * @throws Exception */ public function deleteFromNcSessionId(string $ncSessionId): int { $qb = $this->db->getQueryBuilder(); @@ -116,7 +140,7 @@ public function deleteFromNcSessionId(string $ncSessionId): int { /** * @param int $minCreationTimestamp - * @throws \OCP\DB\Exception + * @throws Exception */ public function cleanupSessions(int $minCreationTimestamp): void { $qb = $this->db->getQueryBuilder(); @@ -136,9 +160,17 @@ public function cleanupSessions(int $minCreationTimestamp): void { * @param string $iss * @param int $authtokenId * @param string $ncSessionid - * @return mixed|Session|\OCP\AppFramework\Db\Entity + * @param string $idToken + * @param string $userId + * @param int $providerId + * @param bool $idpSessionClosed + * @return Session|null + * @throws Exception */ - public function createSession(string $sid, string $sub, string $iss, int $authtokenId, string $ncSessionid) { + public function createSession( + string $sid, string $sub, string $iss, int $authtokenId, string $ncSessionid, + string $idToken, string $userId, int $providerId, bool $idpSessionClosed = false, + ): ?Session { try { // do not create if one with same sid already exists (which should not happen) return $this->findSessionBySid($sid); @@ -157,6 +189,10 @@ public function createSession(string $sid, string $sub, string $iss, int $authto $session->setAuthtokenId($authtokenId); $session->setNcSessionId($ncSessionid); $session->setCreatedAt($createdAt); + $session->setIdToken($idToken); + $session->setUserId($userId); + $session->setProviderId($providerId); + $session->setIdpSessionClosed($idpSessionClosed ? 1 : 0); return $this->insert($session); } } diff --git a/lib/Helper/HttpClientHelper.php b/lib/Helper/HttpClientHelper.php index 86197881d..4811041a2 100644 --- a/lib/Helper/HttpClientHelper.php +++ b/lib/Helper/HttpClientHelper.php @@ -27,8 +27,10 @@ public function get($url, array $headers = [], array $options = []) { $client = $this->clientService->newClient(); - if (isset($oidcConfig['httpclient.allowselfsigned']) - && !in_array($oidcConfig['httpclient.allowselfsigned'], [false, 'false', 0, '0'], true)) { + $debugModeEnabled = $this->config->getSystemValueBool('debug', false); + if ($debugModeEnabled + || (isset($oidcConfig['httpclient.allowselfsigned']) + && !in_array($oidcConfig['httpclient.allowselfsigned'], [false, 'false', 0, '0'], true))) { $options['verify'] = false; } diff --git a/lib/Listener/TokenInvalidatedListener.php b/lib/Listener/TokenInvalidatedListener.php new file mode 100644 index 000000000..8459309bd --- /dev/null +++ b/lib/Listener/TokenInvalidatedListener.php @@ -0,0 +1,116 @@ + + */ +class TokenInvalidatedListener implements IEventListener { + + public function __construct( + private LoggerInterface $logger, + private SessionMapper $sessionMapper, + private ProviderMapper $providerMapper, + private DiscoveryService $discoveryService, + private IURLGenerator $urlGenerator, + private HttpClientHelper $httpClientHelper, + ) { + } + + public function handle(Event $event): void { + if (!$event instanceof TokenInvalidatedEvent) { + return; + } + + $this->logger->debug('[TokenInvalidatedListener] received event', [ + 'token_id' => $event->getTokenId(), + 'user_id' => $event->getUserId(), + ]); + + try { + $oidcSession = $this->sessionMapper->getSessionByAuthTokenAndUid($event->getTokenId(), $event->getUserId()); + } catch (Exception|DoesNotExistException|MultipleObjectsReturnedException $e) { + $this->logger->warning('[TokenInvalidatedListener] Could not find the OIDC session related with an invalidated token', [ + 'token_id' => $event->getTokenId(), + 'user_id' => $event->getUserId(), + 'exception' => $e, + ]); + return; + } + // we have nothing to do if we know the idp session is already closed + if ($oidcSession->getIdpSessionClosed() !== 0) { + $this->logger->warning('[TokenInvalidatedListener] The session is already closed on the IdP side', [ + 'token_id' => $event->getTokenId(), + 'user_id' => $event->getUserId(), + ]); + return; + } + + // now we call the end_session_endpoint + try { + $provider = $this->providerMapper->getProvider($oidcSession->getProviderId()); + } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { + $this->logger->warning('[TokenInvalidatedListener] Could not find the OIDC provider of a session related with an invalidated token', [ + 'token_id' => $event->getTokenId(), + 'user_id' => $event->getUserId(), + 'provider_id' => $oidcSession->getProviderId(), + 'exception' => $e, + ]); + return; + } + + // Check if a custom end_session_endpoint is set in the provider otherwise use the default one provided by the openid-configuration + $discoveryData = $this->discoveryService->obtainDiscovery($provider); + $defaultEndSessionEndpoint = $discoveryData['end_session_endpoint'] ?? null; + $customEndSessionEndpoint = $provider->getEndSessionEndpoint(); + $endSessionEndpoint = $customEndSessionEndpoint ?: $defaultEndSessionEndpoint; + + if ($endSessionEndpoint === null || $endSessionEndpoint === '') { + $this->logger->warning('[TokenInvalidatedListener] Could not find the end_session_endpoint of the OIDC provider of a session related with an invalidated token', [ + 'token_id' => $event->getTokenId(), + 'user_id' => $event->getUserId(), + 'provider_id' => $oidcSession->getProviderId(), + ]); + return; + } + + $endSessionEndpoint .= '?post_logout_redirect_uri=' . $this->urlGenerator->getAbsoluteURL('/'); + $endSessionEndpoint .= '&client_id=' . $provider->getClientId(); + $endSessionEndpoint .= '&id_token_hint=' . $oidcSession->getIdToken(); + + $this->logger->warning('[TokenInvalidatedListener] requesting ' . $endSessionEndpoint); + try { + $this->httpClientHelper->get($endSessionEndpoint); + } catch (ClientException|ServerException $e) { + $response = $e->getResponse(); + $body = (string)$response->getBody(); + $this->logger->debug('[TokenInvalidatedListener] Failed to request the end_session_endpoint, client or server error', [ + 'status_code' => $response->getStatusCode(), + 'body' => $body, + 'exception' => $e, + ]); + } catch (\Exception $e) { + $this->logger->debug('[TokenInvalidatedListener] Failed to request the end_session_endpoint', ['exception' => $e]); + } + } +} diff --git a/lib/Migration/Version070400Date20250820141709.php b/lib/Migration/Version070400Date20250820141709.php new file mode 100644 index 000000000..ec997c6bf --- /dev/null +++ b/lib/Migration/Version070400Date20250820141709.php @@ -0,0 +1,67 @@ +hasTable('user_oidc_sessions')) { + $table = $schema->getTable('user_oidc_sessions'); + if (!$table->hasColumn('id_token')) { + $table->addColumn('id_token', Types::TEXT, [ + 'notnull' => false, + ]); + $schemaChanged = true; + } + if (!$table->hasColumn('user_id')) { + $table->addColumn('user_id', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + 'default' => null, + ]); + $schemaChanged = true; + } + if (!$table->hasColumn('provider_id')) { + $table->addColumn('provider_id', Types::BIGINT, [ + 'notnull' => true, + 'default' => 0, + 'unsigned' => true, + ]); + $schemaChanged = true; + } + if (!$table->hasColumn('idp_session_closed')) { + $table->addColumn('idp_session_closed', Types::SMALLINT, [ + 'notnull' => true, + 'default' => 0, + 'unsigned' => true, + ]); + $schemaChanged = true; + } + } + + return $schemaChanged ? $schema : null; + } +} diff --git a/psalm.xml b/psalm.xml index 1d059fd4a..749664361 100644 --- a/psalm.xml +++ b/psalm.xml @@ -74,5 +74,6 @@ + diff --git a/tests/stubs/ocp_token_invalidated_event.php b/tests/stubs/ocp_token_invalidated_event.php new file mode 100644 index 000000000..bfc1ddd70 --- /dev/null +++ b/tests/stubs/ocp_token_invalidated_event.php @@ -0,0 +1,13 @@ + Date: Wed, 20 Aug 2025 17:23:58 +0200 Subject: [PATCH 2/5] feat(revoke-token): add a timeout when requesting the end_session_endpoint, cleanup our own oidc session when a token has been invalidated Signed-off-by: Julien Veyssier --- lib/Listener/TokenInvalidatedListener.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Listener/TokenInvalidatedListener.php b/lib/Listener/TokenInvalidatedListener.php index 8459309bd..5e0703c20 100644 --- a/lib/Listener/TokenInvalidatedListener.php +++ b/lib/Listener/TokenInvalidatedListener.php @@ -100,7 +100,7 @@ public function handle(Event $event): void { $this->logger->warning('[TokenInvalidatedListener] requesting ' . $endSessionEndpoint); try { - $this->httpClientHelper->get($endSessionEndpoint); + $this->httpClientHelper->get($endSessionEndpoint, [], ['timeout' => 5]); } catch (ClientException|ServerException $e) { $response = $e->getResponse(); $body = (string)$response->getBody(); @@ -112,5 +112,7 @@ public function handle(Event $event): void { } catch (\Exception $e) { $this->logger->debug('[TokenInvalidatedListener] Failed to request the end_session_endpoint', ['exception' => $e]); } + // we know this oidc session is not useful anymore, we can delete it + $this->sessionMapper->delete($oidcSession); } } From af788fdd128c4fe3c9870acc218f6cb59d70e478 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Thu, 21 Aug 2025 12:54:11 +0200 Subject: [PATCH 3/5] feat(revoke-token): adjust to changes in the event Signed-off-by: Julien Veyssier --- lib/Listener/TokenInvalidatedListener.php | 26 ++++++++++++--------- tests/stubs/ocp_token_invalidated_event.php | 3 +-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/Listener/TokenInvalidatedListener.php b/lib/Listener/TokenInvalidatedListener.php index 5e0703c20..ce8a81592 100644 --- a/lib/Listener/TokenInvalidatedListener.php +++ b/lib/Listener/TokenInvalidatedListener.php @@ -42,17 +42,21 @@ public function handle(Event $event): void { return; } + $eventToken = $event->getToken(); + $eventTokenId = $eventToken->getId(); + $eventTokenUserId = $eventToken->getUID(); + $this->logger->debug('[TokenInvalidatedListener] received event', [ - 'token_id' => $event->getTokenId(), - 'user_id' => $event->getUserId(), + 'token_id' => $eventTokenId, + 'user_id' => $eventTokenUserId, ]); try { - $oidcSession = $this->sessionMapper->getSessionByAuthTokenAndUid($event->getTokenId(), $event->getUserId()); + $oidcSession = $this->sessionMapper->getSessionByAuthTokenAndUid($eventTokenId, $eventTokenUserId); } catch (Exception|DoesNotExistException|MultipleObjectsReturnedException $e) { $this->logger->warning('[TokenInvalidatedListener] Could not find the OIDC session related with an invalidated token', [ - 'token_id' => $event->getTokenId(), - 'user_id' => $event->getUserId(), + 'token_id' => $eventTokenId, + 'user_id' => $eventTokenUserId, 'exception' => $e, ]); return; @@ -60,8 +64,8 @@ public function handle(Event $event): void { // we have nothing to do if we know the idp session is already closed if ($oidcSession->getIdpSessionClosed() !== 0) { $this->logger->warning('[TokenInvalidatedListener] The session is already closed on the IdP side', [ - 'token_id' => $event->getTokenId(), - 'user_id' => $event->getUserId(), + 'token_id' => $eventTokenId, + 'user_id' => $eventTokenUserId, ]); return; } @@ -71,8 +75,8 @@ public function handle(Event $event): void { $provider = $this->providerMapper->getProvider($oidcSession->getProviderId()); } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { $this->logger->warning('[TokenInvalidatedListener] Could not find the OIDC provider of a session related with an invalidated token', [ - 'token_id' => $event->getTokenId(), - 'user_id' => $event->getUserId(), + 'token_id' => $eventTokenId, + 'user_id' => $eventTokenUserId, 'provider_id' => $oidcSession->getProviderId(), 'exception' => $e, ]); @@ -87,8 +91,8 @@ public function handle(Event $event): void { if ($endSessionEndpoint === null || $endSessionEndpoint === '') { $this->logger->warning('[TokenInvalidatedListener] Could not find the end_session_endpoint of the OIDC provider of a session related with an invalidated token', [ - 'token_id' => $event->getTokenId(), - 'user_id' => $event->getUserId(), + 'token_id' => $eventTokenId, + 'user_id' => $eventTokenUserId, 'provider_id' => $oidcSession->getProviderId(), ]); return; diff --git a/tests/stubs/ocp_token_invalidated_event.php b/tests/stubs/ocp_token_invalidated_event.php index bfc1ddd70..6e6d962e0 100644 --- a/tests/stubs/ocp_token_invalidated_event.php +++ b/tests/stubs/ocp_token_invalidated_event.php @@ -7,7 +7,6 @@ namespace OCP\Authentication\Events { interface TokenInvalidatedEvent extends \OCP\EventDispatcher\Event { - public function getUserId(): string; - public function getTokenId(): int; + public function getToken(): \OCP\Authentication\Token\IToken; } } From 55c3f6ed442bf785c77be4fd350db588ba01c7a7 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Thu, 21 Aug 2025 16:26:44 +0200 Subject: [PATCH 4/5] feat(revoke-token): encrypt login id token in session db row Signed-off-by: Julien Veyssier --- lib/Db/SessionMapper.php | 8 ++++++-- lib/Listener/TokenInvalidatedListener.php | 10 +++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index f2e8c120f..6f6d9183d 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -16,12 +16,16 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; +use OCP\Security\ICrypto; /** * @extends QBMapper */ class SessionMapper extends QBMapper { - public function __construct(IDBConnection $db) { + public function __construct( + IDBConnection $db, + private ICrypto $crypto, + ) { parent::__construct($db, 'user_oidc_sessions', Session::class); } @@ -189,7 +193,7 @@ public function createSession( $session->setAuthtokenId($authtokenId); $session->setNcSessionId($ncSessionid); $session->setCreatedAt($createdAt); - $session->setIdToken($idToken); + $session->setIdToken($this->crypto->encrypt($idToken)); $session->setUserId($userId); $session->setProviderId($providerId); $session->setIdpSessionClosed($idpSessionClosed ? 1 : 0); diff --git a/lib/Listener/TokenInvalidatedListener.php b/lib/Listener/TokenInvalidatedListener.php index ce8a81592..7622dac75 100644 --- a/lib/Listener/TokenInvalidatedListener.php +++ b/lib/Listener/TokenInvalidatedListener.php @@ -20,6 +20,7 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\IURLGenerator; +use OCP\Security\ICrypto; use Psr\Log\LoggerInterface; /** @@ -34,6 +35,7 @@ public function __construct( private DiscoveryService $discoveryService, private IURLGenerator $urlGenerator, private HttpClientHelper $httpClientHelper, + private ICrypto $crypto, ) { } @@ -98,9 +100,15 @@ public function handle(Event $event): void { return; } + try { + $decryptedIdToken = $this->crypto->decrypt($oidcSession->getIdToken()); + } catch (\Exception $e) { + $this->logger->warning('[TokenInvalidatedListener] Could not decrpyt the login id token of a session related with an invalidated token', ['exception' => $e]); + return; + } $endSessionEndpoint .= '?post_logout_redirect_uri=' . $this->urlGenerator->getAbsoluteURL('/'); $endSessionEndpoint .= '&client_id=' . $provider->getClientId(); - $endSessionEndpoint .= '&id_token_hint=' . $oidcSession->getIdToken(); + $endSessionEndpoint .= '&id_token_hint=' . $decryptedIdToken; $this->logger->warning('[TokenInvalidatedListener] requesting ' . $endSessionEndpoint); try { From 97950464feab88b39738a136d2246fdf5ee6e708 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Thu, 28 Aug 2025 11:17:12 +0200 Subject: [PATCH 5/5] feat(revoke-token): on login, if there already is a oc_user_oidc_sessions row with the SID, update it Signed-off-by: Julien Veyssier --- lib/Controller/LoginController.php | 2 +- lib/Db/SessionMapper.php | 32 ++++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 3be170d40..652b4267b 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -610,7 +610,7 @@ public function code(string $state = '', string $code = '', string $scope = '', // for backchannel logout try { $authToken = $this->authTokenProvider->getToken($this->session->getId()); - $this->sessionMapper->createSession( + $this->sessionMapper->createOrUpdateSession( $idTokenPayload->sid ?? 'fallback-sid', $idTokenPayload->sub ?? 'fallback-sub', $idTokenPayload->iss ?? 'fallback-iss', diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index 6f6d9183d..c7e3c1922 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -157,13 +157,19 @@ public function cleanupSessions(int $minCreationTimestamp): void { } /** - * Create a session + * Create or update a Nextcloud Oidc session + * + * We have a unique constraint on the "sid" column because there cannot be multiple Nextcloud Oidc sessions for the same IdP session (sid) + * So if we log in with an IdP session that was already used in a previous Nextcloud Oidc session, we can safely assume + * the related real Nextcloud session does not exist anymore. So we update the row for this "sid". + * + * In short: If there are multiple Nextcloud logins using the same IdP session, we only store the last one * * @param string $sid * @param string $sub * @param string $iss * @param int $authtokenId - * @param string $ncSessionid + * @param string $ncSessionId * @param string $idToken * @param string $userId * @param int $providerId @@ -171,27 +177,37 @@ public function cleanupSessions(int $minCreationTimestamp): void { * @return Session|null * @throws Exception */ - public function createSession( - string $sid, string $sub, string $iss, int $authtokenId, string $ncSessionid, + public function createOrUpdateSession( + string $sid, string $sub, string $iss, int $authtokenId, string $ncSessionId, string $idToken, string $userId, int $providerId, bool $idpSessionClosed = false, ): ?Session { + $createdAt = (new DateTime())->getTimestamp(); + try { // do not create if one with same sid already exists (which should not happen) - return $this->findSessionBySid($sid); + $existingSession = $this->findSessionBySid($sid); + $existingSession->setSub($sub); + $existingSession->setIss($iss); + $existingSession->setAuthtokenId($authtokenId); + $existingSession->setNcSessionId($ncSessionId); + $existingSession->setCreatedAt($createdAt); + $existingSession->setIdToken($this->crypto->encrypt($idToken)); + $existingSession->setUserId($userId); + $existingSession->setProviderId($providerId); + $existingSession->setIdpSessionClosed($idpSessionClosed ? 1 : 0); + return $this->update($existingSession); } catch (MultipleObjectsReturnedException $e) { // this can't happen return null; } catch (DoesNotExistException $e) { } - $createdAt = (new DateTime())->getTimestamp(); - $session = new Session(); $session->setSid($sid); $session->setSub($sub); $session->setIss($iss); $session->setAuthtokenId($authtokenId); - $session->setNcSessionId($ncSessionid); + $session->setNcSessionId($ncSessionId); $session->setCreatedAt($createdAt); $session->setIdToken($this->crypto->encrypt($idToken)); $session->setUserId($userId);