From 29cbc24eb961e46236eb6be7ba032396ee6437e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean=20Charles=20Del=C3=A9pine?= Date: Wed, 3 Jun 2026 21:55:12 +0200 Subject: [PATCH 1/2] refactor: delegate logout handling from login.php to LoginService login.php handled logout logic directly (CSRF check, clearAuth, session setup, redirect). LoginService.performLogout() already encapsulates all of this. Delegate to it so logout behaviour is consistent whether the request comes from login.php or from the modern routing stack (ResponsiveLogoutController). Pre-logout handlers registered in LoginService now run on all logout paths, not just those going through the responsive stack. --- login.php | 33 +++++++++++++-------------------- src/Service/LoginService.php | 30 ++++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/login.php b/login.php index 6f4d420d..25ae7cd9 100644 --- a/login.php +++ b/login.php @@ -204,6 +204,7 @@ function _collectAppLoginParams($injector, $registry, $logger = null, $collectPo } if ($logout_reason) { + $postLogoutData = ['redirect' => null]; if ($is_auth) { try { $tokenService = $injector->getInstance(Token::class); @@ -223,36 +224,28 @@ function _collectAppLoginParams($injector, $registry, $logger = null, $collectPo require HORDE_BASE . '/index.php'; exit; } - $is_auth = null; - $logger->notice(sprintf( - 'User %s logged out of Horde (%s)%s', - $registry->getAuth(), - $_SERVER['REMOTE_ADDR'], - empty($_SERVER['HTTP_X_FORWARDED_FOR']) ? '' : ' (forwarded for [' . $_SERVER['HTTP_X_FORWARDED_FOR'] . '])' + $loginService = $injector->getInstance(\Horde\Horde\Service\LoginService::class); + $postLogoutData = $loginService->performLogout(new \Horde\Horde\ValueObject\LogoutRequest( + csrfToken: null, // already verified above + reason: $logout_reason, + remoteAddr: $_SERVER['REMOTE_ADDR'], + forwardedFor: $_SERVER['HTTP_X_FORWARDED_FOR'] ?? null, )); + $is_auth = null; + } else { + $registry->clearAuth(); + $session->setup(); } - $registry->clearAuth(); - - /* Reset notification handler now, since it may still be using a status - * handler that is no longer valid. */ - $notification->detach('status'); - $notification->attach('status'); - - /* Redirect the user on logout if redirection is enabled and this is an - * an intended logout. */ - if (($logout_reason == Horde_Auth::REASON_LOGOUT) - && !empty($conf['auth']['redirect_on_logout'])) { - $logout_url = new Horde_Url($conf['auth']['redirect_on_logout'], true); + if (!empty($postLogoutData['redirect'])) { + $logout_url = new Horde_Url($postLogoutData['redirect'], true); if (!isset($_COOKIE[session_name()])) { $logout_url->add(session_name(), session_id()); } _addAnchor($logout_url, 'url', $vars, $url_anchor)->redirect(); } - $session->setup(); - /* Explicitly set language in un-authenticated session. */ $registry->setLanguage($GLOBALS['language']); diff --git a/src/Service/LoginService.php b/src/Service/LoginService.php index 23b406da..b38755f1 100644 --- a/src/Service/LoginService.php +++ b/src/Service/LoginService.php @@ -444,6 +444,24 @@ public function performLogout(LogoutRequest $request): array ); } + // Run pre-logout handlers before the session is destroyed. + // A failing handler must never prevent the logout from completing. + $postLogoutRedirect = null; + if ($currentUser) { + foreach ($this->preLogoutHandlers as $handler) { + try { + $data = $handler->onBeforeLogout($currentUser, $request->reason); + if (!empty($data['redirect']) && $postLogoutRedirect === null) { + $postLogoutRedirect = $data['redirect']; + } + } catch (Throwable $e) { + $this->logger->warning( + 'PreLogoutHandler ' . $handler::class . ' failed: ' . $e->getMessage() + ); + } + } + } + // Clear authentication $this->registry->clearAuth(); @@ -474,11 +492,15 @@ public function performLogout(LogoutRequest $request): array // Ignore - theme will use system default } - // Check redirect_on_logout config - if ($request->reason === Horde_Auth::REASON_LOGOUT + // Handler redirect takes priority over redirect_on_logout config + if ($postLogoutRedirect === null + && $request->reason === Horde_Auth::REASON_LOGOUT && !empty($this->conf['auth']['redirect_on_logout'])) { - $logoutUrl = $this->conf['auth']['redirect_on_logout']; - return ['redirect' => $logoutUrl, 'reason' => $request->reason]; + $postLogoutRedirect = $this->conf['auth']['redirect_on_logout']; + } + + if ($postLogoutRedirect !== null) { + return ['redirect' => $postLogoutRedirect, 'reason' => $request->reason]; } // Default redirect to login page From 635c51cb1fb312845b927d49ebbbc0a67ce33854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean=20Charles=20Del=C3=A9pine?= Date: Tue, 26 May 2026 20:44:29 +0200 Subject: [PATCH 2/2] feat: OIDC/OAuth2 authentication integration for Horde base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires OIDC/OAuth2 support into the Horde base application, independent of the Horde auth driver (LDAP, SQL, application, etc. all supported): - OAuthLoginController: accept GET in addition to POST to support direct links/buttons to the IdP. - LoginServiceFactory: register OidcPreLogoutHandler whenever OAuth/OIDC providers are configured — a user authenticating via LDAP/SQL/etc. may still hold OAuth tokens (e.g. for XOAUTH2 IMAP access) that need revocation/SLO on logout. No dependency on auth.driver. - LoginService::performLogout(): aggregate redirect URLs from pre-logout handlers; handler redirect takes priority over redirect_on_logout config. - OAuthAccountController: resolve a human-readable Horde username from OIDC userinfo claims (preferred_username, uid, login, email); store tokens for XOAUTH2 hooks; filter login.php as post-login redirect destination. - OAuthTokenRepositoryFactory: SQL-backed token repository. - Admin UI: OIDC-specific provider fields (logout strategy, SLO endpoints, XOAUTH2 domain). - conf.xml: OAuth/OIDC token storage tab under Authentication. - Migration 3: add OIDC-specific columns to horde_oauth_providers. - hooks.php.dist: smtp_credentials and backchannel logout route. An earlier revision of this branch also auto-redirected to the IdP when auth.driver was set to a dedicated 'oidc' value. That driver no longer exists — this integration works alongside any existing auth driver. An admin wanting to skip the login page entirely can already do so via the pre-existing generic alternate_login config. Depends on: - horde/Core: OIDC integration (PR #160) - horde/base refactor/login-logout-delegate-to-service (PR #109) --- config/conf.xml | 25 +++++++ config/hooks.php.dist | 51 ++++++++++++++ config/routes.php | 15 ++++- .../3_horde_oauth_providers_oidc_fields.php | 49 ++++++++++++++ src/Admin/OAuthProviderController.php | 6 ++ src/Auth/OAuthLoginController.php | 6 +- src/Factory/LoginServiceFactory.php | 17 +++++ src/Factory/OAuthTokenRepositoryFactory.php | 39 +++++++++++ src/Service/LoginService.php | 2 + src/Settings/OAuthAccountController.php | 66 ++++++++++++++++++- .../admin/oauthprovider/edit-oauth2.html.php | 48 ++++++++++++++ 11 files changed, 319 insertions(+), 5 deletions(-) create mode 100644 migration/3_horde_oauth_providers_oidc_fields.php create mode 100644 src/Factory/OAuthTokenRepositoryFactory.php diff --git a/config/conf.xml b/config/conf.xml index 772e5e46..1f0b89a1 100644 --- a/config/conf.xml +++ b/config/conf.xml @@ -2716,5 +2716,30 @@ cookie policy. See session configuration options.">$_SERVER['SERVER_NAME'] ?? $_ + + + + + OAuth / OIDC Token Storage + Configure where OAuth2 access and refresh tokens are + stored server-side. These tokens are required when Horde authenticates + users via OIDC (auth driver = oidc) so that applications such as IMP + can connect to IMAP and SMTP servers using XOAUTH2 without storing the + user password in the Horde session. Must be set to SQL for any + multi-worker or production deployment. + + null + + + + + + + + + + diff --git a/config/hooks.php.dist b/config/hooks.php.dist index 587150a4..d857ef1e 100644 --- a/config/hooks.php.dist +++ b/config/hooks.php.dist @@ -104,6 +104,19 @@ * credentials. * (array) - Replace the credentials with the given value. * + * smtp pre-authentication. This hook allows + * modification of credentials immediately before authentication to the + * SMTP server. This gives the chance to modify the username and + * have it affect ONLY the IMAP authentication process. I.e., the modified + * username is NOT visible to Horde or any application. This is useful e.g. + * when the IMAP server requires prepending the user's IP address to the + * username for audit purposes. + * + * @param array $credentials An array containing authentication credentials + * - userId: (string) The authentication username. + * + * @return array The possibly modified authentication credentials. + * * authusername * ------------ * This hook is used to dynamically convert between an authentication username @@ -1302,5 +1315,43 @@ class Horde_Hooks // return $map[$deviceId] ?? null; // } + /** + * smtp_credentials + * + * Called by Horde_Core_Factory_Mail when creating the SMTP transport. + * + * @param string $username Authenticated Horde username + * @return array Credentials with 'xoauth2_token' and 'username' keys + * @throws Horde_Exception_HookNotSet If XOAUTH2 not applicable + */ +// public function smtp_credentials(string $username): array +// { +// global $injector; +// +// $tokenService = $injector->getInstance(\Horde\Core\Service\OAuthTokenService::class); +// $providerConfig = $injector->getInstance(\Horde\Core\Service\OAuthProviderConfigRepository::class); +// +// $row = \Horde\Core\Service\OidcHookHelper::findProviderForUser( +// $username, $tokenService, $providerConfig +// ); +// if ($row === null) { +// throw new Horde_Exception_HookNotSet(); +// } +// +// $accessToken = \Horde\Core\Service\OidcHookHelper::getValidAccessToken( +// $username, $row, $tokenService, $injector +// ); +// if ($accessToken === null) { +// throw new Horde_Exception_HookNotSet(); +// } +// +// $xoauth2User = \Horde\Core\Service\OidcHookHelper::xoauth2Username($username, $row); +// +// return [ +// 'xoauth2_token' => new Horde_Smtp_Password_Xoauth2($xoauth2User, $accessToken), +// 'username' => $xoauth2User, +// ]; +// } + } diff --git a/config/routes.php b/config/routes.php index 70484b9c..14cf9c4b 100644 --- a/config/routes.php +++ b/config/routes.php @@ -50,7 +50,7 @@ ->withController(Auth\OAuthLoginController::class) ->withDefaults(['HordeAuthType' => 'NONE']) ->withMiddleware([AuthHordeSession::class]) - ->withMethods(['POST']) + ->withMethods(['POST', 'GET']) ->add(); // Responsive UI Routes - Phase 1: Portal @@ -635,3 +635,16 @@ ]) ->withMethods(['GET']) ->add(); + +// OIDC Back-Channel Logout (RFC 9470) +// Receives a signed logout_token JWT from the identity provider. +// POST only, no Horde session required (the IdP has no session with us). +// Always returns 200 to prevent provider retries. +// Configure in your IdP (Apereo CAS): logoutType=BACK_CHANNEL, logoutUrl= +$mapper->buildRoute(uri: '/auth/oidc/backchannel-logout', name: 'OidcBackchannelLogout') + ->withController(\Horde\Core\Auth\OidcBackchannelLogoutController::class) + ->withDefaults(['HordeAuthType' => 'NONE']) + ->withMiddleware([HordeCore::class, ErrorFilter::class]) + ->withMethods(['POST']) + ->add(); + diff --git a/migration/3_horde_oauth_providers_oidc_fields.php b/migration/3_horde_oauth_providers_oidc_fields.php new file mode 100644 index 00000000..5fef790b --- /dev/null +++ b/migration/3_horde_oauth_providers_oidc_fields.php @@ -0,0 +1,49 @@ +columns('horde_oauth_providers'), + null, + 'name' + ); + + if (!isset($cols['logout_type'])) { + $this->addColumn('horde_oauth_providers', 'logout_type', + 'string', ['limit' => 20, 'default' => 'local']); + } + if (!isset($cols['end_session_endpoint'])) { + $this->addColumn('horde_oauth_providers', 'end_session_endpoint', + 'string', ['limit' => 1024]); + } + if (!isset($cols['post_logout_redirect_uri'])) { + $this->addColumn('horde_oauth_providers', 'post_logout_redirect_uri', + 'string', ['limit' => 1024]); + } + if (!isset($cols['backchannel_username_claim'])) { + $this->addColumn('horde_oauth_providers', 'backchannel_username_claim', + 'string', ['limit' => 64, 'default' => 'sub']); + } + if (!isset($cols['xoauth2_use_email'])) { + $this->addColumn('horde_oauth_providers', 'xoauth2_use_email', + 'integer', ['default' => 0]); + } + if (!isset($cols['xoauth2_domain'])) { + $this->addColumn('horde_oauth_providers', 'xoauth2_domain', + 'string', ['limit' => 255]); + } + } + + public function down() + { + foreach ([ + 'logout_type', 'end_session_endpoint', 'post_logout_redirect_uri', + 'backchannel_username_claim', 'xoauth2_use_email', 'xoauth2_domain', + ] as $col) { + $this->removeColumn('horde_oauth_providers', $col); + } + } +} diff --git a/src/Admin/OAuthProviderController.php b/src/Admin/OAuthProviderController.php index 054bce02..614e3f66 100644 --- a/src/Admin/OAuthProviderController.php +++ b/src/Admin/OAuthProviderController.php @@ -279,6 +279,8 @@ private function extractUpdateData(array $existing, array $body): array 'revocation_endpoint', 'introspection_endpoint', 'default_scopes', 'app_identifier', 'installation_id', + 'logout_type', 'end_session_endpoint', 'post_logout_redirect_uri', + 'backchannel_username_claim', 'xoauth2_use_email', 'xoauth2_domain', ]; $data = []; @@ -288,6 +290,10 @@ private function extractUpdateData(array $existing, array $body): array } } + if (isset($data['xoauth2_use_email'])) { + $data['xoauth2_use_email'] = (int) ($data['xoauth2_use_email'] ?? 0); + } + if ($data['enabled'] ?? null !== null) { $data['enabled'] = (int) ($data['enabled'] ?? 1); } diff --git a/src/Auth/OAuthLoginController.php b/src/Auth/OAuthLoginController.php index 91654e29..fa8e6f0f 100644 --- a/src/Auth/OAuthLoginController.php +++ b/src/Auth/OAuthLoginController.php @@ -68,8 +68,10 @@ public function handle(ServerRequestInterface $request): ResponseInterface return $this->redirect($loginUrl . '?error=failed'); } - $body = $request->getParsedBody() ?? []; - $redirectUrl = is_string($body['url'] ?? null) ? $body['url'] : ''; + $params = $request->getMethod() === 'GET' + ? $request->getQueryParams() + : ($request->getParsedBody() ?? []); + $redirectUrl = is_string($params['url'] ?? null) ? $params['url'] : ''; $verifier = PkceGenerator::generateVerifier(); $challenge = PkceGenerator::computeChallenge($verifier); diff --git a/src/Factory/LoginServiceFactory.php b/src/Factory/LoginServiceFactory.php index 54ae55df..da692330 100644 --- a/src/Factory/LoginServiceFactory.php +++ b/src/Factory/LoginServiceFactory.php @@ -18,6 +18,8 @@ use Exception; use Horde\Core\Config\ConfigLoader; use Horde\Core\Service\OAuthProviderConfigRepository; +use Horde\Core\Service\OidcPreLogoutHandler; +use Horde\Core\Session\HordeSession; use Horde\Core\Session\SessionAccess; use Horde\Core\Session\SessionConfig; use Horde\Core\Session\SessionLifecycle; @@ -69,6 +71,20 @@ public function create(Injector $injector): LoginService // ConfigLoader is unavailable (test fixtures, partial DI setups). $conf = $this->resolveConf($injector); + // Register OidcPreLogoutHandler whenever OAuth/OIDC providers are + // configured, independently of the Horde auth driver — a user may + // authenticate via LDAP/SQL/etc. while still holding OAuth tokens + // (e.g. for XOAUTH2 IMAP access) that should be revoked/SLO'd on + // logout. + $preLogoutHandlers = []; + try { + if ($providerConfig->listEnabled() !== []) { + $preLogoutHandlers[] = $injector->getInstance(OidcPreLogoutHandler::class); + } + } catch (Exception $e) { + $logger->warning('Could not resolve OidcPreLogoutHandler: ' . $e->getMessage()); + } + return new LoginService( $registry, $logger, @@ -83,6 +99,7 @@ public function create(Injector $injector): LoginService $sessionConfig, $notification, $conf, + $preLogoutHandlers, ); } diff --git a/src/Factory/OAuthTokenRepositoryFactory.php b/src/Factory/OAuthTokenRepositoryFactory.php new file mode 100644 index 00000000..e00b4377 --- /dev/null +++ b/src/Factory/OAuthTokenRepositoryFactory.php @@ -0,0 +1,39 @@ + + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + */ + +namespace Horde\Horde\Factory; + +use Horde\Core\Service\OAuthTokenRepository; +use Horde\Db\Adapter; +use Horde\Horde\Service\SqlOAuthTokenRepository; +use Horde\Injector\Injector; +use Horde\Secret\SecretManager; + +/** + * Factory for OAuthTokenRepository. + * + * Returns SqlOAuthTokenRepository backed by the Horde DB adapter. + */ +class OAuthTokenRepositoryFactory +{ + public function create(Injector $injector): OAuthTokenRepository + { + return new SqlOAuthTokenRepository( + db: $injector->getInstance(Adapter::class), + secret: $injector->getInstance(SecretManager::class), + ); + } +} diff --git a/src/Service/LoginService.php b/src/Service/LoginService.php index b38755f1..2b6b67a9 100644 --- a/src/Service/LoginService.php +++ b/src/Service/LoginService.php @@ -26,6 +26,7 @@ use Horde\Core\Assets\ResponsiveAssets; use Horde\Core\Config\RegistryState; use Horde\Core\Service\OAuthProviderConfigRepository; +use Horde\Core\Service\PreLogoutHandlerInterface; use Horde\Core\Session\HordeSession; use Horde\Core\Session\SessionAccess; use Horde\Exception\HordeThrowable; @@ -74,6 +75,7 @@ public function __construct( private readonly SessionConfig $sessionConfig, private readonly Horde_Notification_Handler $notification, private readonly array $conf, + private readonly array $preLogoutHandlers = [], ) {} /** diff --git a/src/Settings/OAuthAccountController.php b/src/Settings/OAuthAccountController.php index 7d031883..9d476cc0 100644 --- a/src/Settings/OAuthAccountController.php +++ b/src/Settings/OAuthAccountController.php @@ -274,6 +274,12 @@ private function handleLoginCallback( $email = $userinfo['email'] ?? null; $displayName = $userinfo['name'] ?? $userinfo['display_name'] ?? $userinfo['login'] ?? null; + // Resolve a local Horde username from the userinfo claims. + // Priority: preferred_username → uid → login → left part of email. + // This ensures a human-readable Horde username instead of the + // opaque identity UUID, regardless of whether a local link exists. + $preferredUsername = $this->resolveLocalUsername($userinfo); + if ($externalId === '') { return $this->redirect($loginUrl . '?error=failed'); } @@ -296,7 +302,19 @@ private function handleLoginCallback( } } + // No local link yet: create one from the preferred username so that + // subsequent logins resolve to a human-readable Horde username + // instead of the identity UUID. + if ($localUsername === null && $preferredUsername !== null) { + $this->identityLinkService->coupleLocalUser($identity->id, $preferredUsername); + $localUsername = $preferredUsername; + } + + // Store tokens so IMP/Ingo hooks can retrieve them via OAuthTokenService. + // handleLoginCallback does not link an account but still needs tokens + // available for XOAUTH2 authentication in mail clients. $authId = $localUsername ?? $identity->id; + $this->tokenService->store($authId, $providerId, $tokenSet); $this->registry->setAuth($authId, []); if ($this->identityService->getAll($authId) === []) { @@ -307,11 +325,17 @@ private function handleLoginCallback( ]); } + // Filter login.php as redirect destination — it loops back to the portal + if ($redirectUrl !== '' && str_contains($redirectUrl, '/login.php')) { + $redirectUrl = ''; + } + if ($redirectUrl !== '') { return $this->redirect($redirectUrl); } - return $this->redirect((string) $this->registry->getServiceLink('portal')); + return $this->redirect(rtrim($this->registry->get('webroot', 'horde'), '/') . '/index.php'); + } catch (Throwable $e) { error_log('OAuthLoginCallback failed: ' . $e->getMessage() . ' in ' . $e->getFile() . ':' . $e->getLine()); return $this->redirect($loginUrl . '?error=failed'); @@ -401,7 +425,10 @@ private function handleAccountLinkCallback( } if ($flowData->redirectUrl !== '') { - return $this->redirect($flowData->redirectUrl); + if (str_contains($flowData->redirectUrl, '/login.php')) { + $initialPage = $this->registry->getInitialPage('horde'); + return $this->redirect($initialPage ?? (string) $this->registry->getServiceLink('portal')); + } } return $this->redirect($baseUrl . '/'); @@ -488,4 +515,39 @@ private function getBaseUrl(): string { return rtrim($this->registry->get('webroot', 'horde'), '/') . '/settings/oauth'; } + + + /** + * Resolve a local Horde username from OIDC userinfo claims. + * + * Priority: + * 1. preferred_username (standard OIDC claim, used by CAS, Keycloak…) + * 2. uid (LDAP-style claim, common in university IdPs) + * 3. login (GitHub, GitLab) + * 4. left part of email (last resort, strips domain) + * + * Returns null if none of the above are available, in which case the + * caller falls back to the identity UUID. + */ + private function resolveLocalUsername(array $userinfo): ?string + { + foreach (['preferred_username', 'uid', 'login', 'sub', 'id'] as $claim) { + $value = trim((string) ($userinfo[$claim] ?? '')); + if ($value !== '') { + // Strip domain suffix if present (e.g. "jdoe@example.com" → "jdoe") + return strstr($value, '@', true) ?: $value; + } + } + + // Last resort: left part of email + $email = trim((string) ($userinfo['email'] ?? '')); + if ($email !== '') { + $local = strstr($email, '@', true); + if ($local !== false && $local !== '') { + return $local; + } + } + + return null; + } } diff --git a/templates/admin/oauthprovider/edit-oauth2.html.php b/templates/admin/oauthprovider/edit-oauth2.html.php index f0b96ef2..2e807afd 100644 --- a/templates/admin/oauthprovider/edit-oauth2.html.php +++ b/templates/admin/oauthprovider/edit-oauth2.html.php @@ -126,6 +126,54 @@ +provider['type'] === 'oidc'): ?> +

+ +
+ + + +
+ +
+ + + +
+ +
+ + + +
+ +
+ + + +
+ +

+ +
+ + +
+ +
+ + + +
+ +