From 3b71e8e5814cff93b84ba36f386449971625c946 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] 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';