From 25886dce240b17cba8d09f9e654b6045fca762a0 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 | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/login.php b/login.php index 67f844bf..f3bfbe86 100644 --- a/login.php +++ b/login.php @@ -125,6 +125,7 @@ function _addAnchor($url, $type, $vars, $url_anchor = null) } if ($logout_reason) { + $postLogoutData = ['redirect' => null]; if ($is_auth) { try { $tokenService = $injector->getInstance(Token::class); @@ -144,36 +145,28 @@ function _addAnchor($url, $type, $vars, $url_anchor = null) 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 ae8a290e..dbe6cf1b 100644 --- a/src/Service/LoginService.php +++ b/src/Service/LoginService.php @@ -416,6 +416,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(); @@ -453,6 +471,17 @@ public function performLogout(LogoutRequest $request): array // Ignore - theme will use system default } + // 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'])) { + $postLogoutRedirect = $this->conf['auth']['redirect_on_logout']; + } + + if ($postLogoutRedirect !== null) { + return ['redirect' => $postLogoutRedirect, 'reason' => $request->reason]; + } + // Default redirect to login page $webroot = $this->registry->get('webroot', 'horde'); $redirect = $webroot . '/auth/login?logout_reason=logout'; From ab6cc1389cfacf2818c266183334fde153462c82 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 Wires the OIDC auth driver (horde/Core) into the Horde base application: - OAuthLoginController: accept GET in addition to POST to support automatic redirect from login.php to the IdP. - login.php: auto-redirect to the configured OIDC provider when auth.driver = oidc and auth.params.redirect_provider is set. Redirects on all logout reasons except explicit auth failures (REASON_BADLOGIN, REASON_FAILED, REASON_LOCKED). - LoginServiceFactory: register OidcPreLogoutHandler as a pre-logout handler when the auth driver is oidc. - 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: OIDC auth driver configuration and OAuth/OIDC token storage tab. - Migration 3: add OIDC-specific columns to horde_oauth_providers. - hooks.php.dist: smtp_credentials and backchannel logout route. Depends on: - horde/Core: OIDC integration (PR ##160) - horde/base refactor/login-logout-delegate-to-service (PR #109) --- config/conf.xml | 41 ++++++++++++ config/hooks.php.dist | 51 ++++++++++++++ config/routes.php | 14 +++- login.php | 16 +++++ .../3_horde_oauth_providers_oidc_fields.php | 49 ++++++++++++++ src/Admin/OAuthProviderController.php | 6 ++ src/Auth/OAuthLoginController.php | 6 +- src/Factory/LoginServiceFactory.php | 14 ++++ src/Factory/OAuthTokenRepositoryFactory.php | 39 +++++++++++ src/Service/LoginService.php | 9 +-- src/Settings/OAuthAccountController.php | 66 ++++++++++++++++++- .../admin/oauthprovider/edit-oauth2.html.php | 48 ++++++++++++++ 12 files changed, 347 insertions(+), 12 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 d1b07832..6abf4e1b 100644 --- a/config/conf.xml +++ b/config/conf.xml @@ -751,6 +751,22 @@ cookie policy. See session configuration options.">$_SERVER['SERVER_NAME'] ?? $_ + + + + + @@ -2605,5 +2621,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 75ccc499..3df6585b 100644 --- a/config/routes.php +++ b/config/routes.php @@ -46,7 +46,7 @@ ->withController(Auth\OAuthLoginController::class) ->withDefaults(['HordeAuthType' => 'NONE']) ->withMiddleware([AuthHordeSession::class]) - ->withMethods(['POST']) + ->withMethods(['POST', 'GET']) ->add(); // Responsive UI Routes - Phase 1: Portal @@ -583,3 +583,15 @@ ]) ->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/login.php b/login.php index f3bfbe86..016f8469 100644 --- a/login.php +++ b/login.php @@ -375,6 +375,22 @@ function _addAnchor($url, $type, $vars, $url_anchor = null) } } +// OIDC auth driver: auto-redirect to the configured identity provider. +// When auth.driver = oidc and auth.params.redirect_provider is set, +// build alternate_login automatically so login.php redirects to the +// provider without showing the username/password form. +if (empty($conf['auth']['alternate_login']) + && strcasecmp($conf['auth']['driver'] ?? '', 'oidc') === 0 + && !empty($conf['auth']['params']['redirect_provider']) + && (!($logout_reason === Horde_Auth::REASON_BADLOGIN + || $logout_reason === Horde_Auth::REASON_FAILED + || $logout_reason === Horde_Auth::REASON_LOCKED))) { + $conf['auth']['alternate_login'] = + rtrim($registry->get('webroot', 'horde'), '/') + . '/auth/oauth/login/' + . rawurlencode($conf['auth']['params']['redirect_provider']); +} + /* Redirect the user if an alternate login page has been specified. */ if (!empty($conf['auth']['alternate_login'])) { $url = new Horde_Url($conf['auth']['alternate_login'], true); 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 d1af5a9d..7b1e0a27 100644 --- a/src/Factory/LoginServiceFactory.php +++ b/src/Factory/LoginServiceFactory.php @@ -18,6 +18,7 @@ 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\SessionConfig; use Horde\Core\Session\SessionLifecycle; @@ -68,6 +69,18 @@ public function create(Injector $injector): LoginService // ConfigLoader is unavailable (test fixtures, partial DI setups). $conf = $this->resolveConf($injector); + // Collect pre-logout handlers. Handlers are registered conditionally + // based on the configured auth driver. Additional handlers can be + // added here as new auth drivers are introduced. + $preLogoutHandlers = []; + if (strcasecmp($conf['auth']['driver'] ?? '', 'oidc') === 0) { + try { + $preLogoutHandlers[] = $injector->getInstance(OidcPreLogoutHandler::class); + } catch (Exception $e) { + $logger->warning('Could not resolve OidcPreLogoutHandler: ' . $e->getMessage()); + } + } + return new LoginService( $registry, $logger, @@ -82,6 +95,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 dbe6cf1b..a27ed6c4 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\SessionConfig; use Horde\Core\Session\SessionLifecycle; @@ -71,6 +72,7 @@ public function __construct( private readonly SessionConfig $sessionConfig, private readonly Horde_Notification_Handler $notification, private readonly array $conf, + private readonly array $preLogoutHandlers = [], ) {} /** @@ -441,13 +443,6 @@ public function performLogout(LogoutRequest $request): array $this->notification->detach('status'); $this->notification->attach('status'); - // Check redirect_on_logout config - if ($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]; - } - // Setup fresh anonymous session via the modern lifecycle. // SessionLifecycle::setup() is idempotent and replaces the // legacy shim's $GLOBALS['session']->setup() path that this 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'): ?> +

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

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