From bba7dcce49394f478bd401486a2d8cba23ff79ac Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 6 Dec 2023 13:49:53 +0000 Subject: [PATCH] Auth: Refactored OIDC RP-logout PR code, Extracted logout Extracted logout to the login service so the logic can be shared instead of re-implemented at each stage. For this, the SocialAuthService was split so the driver management is in its own class, so it can be used elsewhere without use (or circular dependencies) of the SocialAuthService. During review of #4467 --- .env.example.complete | 3 - app/Access/Controllers/LoginController.php | 32 +--- app/Access/Controllers/OidcController.php | 13 +- app/Access/Controllers/RegisterController.php | 10 +- app/Access/Controllers/SocialController.php | 4 +- app/Access/LoginService.php | 37 +++- app/Access/Oidc/OidcService.php | 46 ++--- app/Access/Saml2Service.php | 14 +- app/Access/SocialAuthService.php | 169 ++---------------- app/Access/SocialDriverManager.php | 147 +++++++++++++++ app/App/Providers/AppServiceProvider.php | 4 +- app/Config/oidc.php | 13 +- app/Theming/ThemeService.php | 8 +- .../Controllers/UserAccountController.php | 6 +- app/Users/Controllers/UserController.php | 6 +- .../layouts/parts/header-user-menu.blade.php | 36 ++-- routes/web.php | 3 +- 17 files changed, 263 insertions(+), 288 deletions(-) create mode 100644 app/Access/SocialDriverManager.php diff --git a/.env.example.complete b/.env.example.complete index 0667cb75b..57d24eb5d 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -273,11 +273,8 @@ OIDC_USER_TO_GROUPS=false OIDC_GROUPS_CLAIM=groups OIDC_REMOVE_FROM_GROUPS=false OIDC_EXTERNAL_ID_CLAIM=sub - -# OIDC Logout Feature: Its value should be value of end_session_endpoint from /.well-known/openid-configuration OIDC_END_SESSION_ENDPOINT=null - # Disable default third-party services such as Gravatar and Draw.IO # Service-specific options will override this option DISABLE_EXTERNAL_SERVICES=false diff --git a/app/Access/Controllers/LoginController.php b/app/Access/Controllers/LoginController.php index 3b4f9b347..904736656 100644 --- a/app/Access/Controllers/LoginController.php +++ b/app/Access/Controllers/LoginController.php @@ -3,7 +3,7 @@ namespace BookStack\Access\Controllers; use BookStack\Access\LoginService; -use BookStack\Access\SocialAuthService; +use BookStack\Access\SocialDriverManager; use BookStack\Exceptions\LoginAttemptEmailNeededException; use BookStack\Exceptions\LoginAttemptException; use BookStack\Facades\Activity; @@ -17,19 +17,19 @@ class LoginController extends Controller { use ThrottlesLogins; - protected SocialAuthService $socialAuthService; + protected SocialDriverManager $socialDriverManager; protected LoginService $loginService; /** * Create a new controller instance. */ - public function __construct(SocialAuthService $socialAuthService, LoginService $loginService) + public function __construct(SocialDriverManager $driverManager, LoginService $loginService) { $this->middleware('guest', ['only' => ['getLogin', 'login']]); $this->middleware('guard:standard,ldap', ['only' => ['login']]); $this->middleware('guard:standard,ldap,oidc', ['only' => ['logout']]); - $this->socialAuthService = $socialAuthService; + $this->socialDriverManager = $driverManager; $this->loginService = $loginService; } @@ -38,7 +38,7 @@ class LoginController extends Controller */ public function getLogin(Request $request) { - $socialDrivers = $this->socialAuthService->getActiveDrivers(); + $socialDrivers = $this->socialDriverManager->getActive(); $authMethod = config('auth.method'); $preventInitiation = $request->get('prevent_auto_init') === 'true'; @@ -101,15 +101,9 @@ class LoginController extends Controller /** * Logout user and perform subsequent redirect. */ - public function logout(Request $request) + public function logout() { - Auth::guard()->logout(); - $request->session()->invalidate(); - $request->session()->regenerateToken(); - - $redirectUri = $this->shouldAutoInitiate() ? '/login?prevent_auto_init=true' : '/'; - - return redirect($redirectUri); + return redirect($this->loginService->logout()); } /** @@ -218,16 +212,4 @@ class LoginController extends Controller redirect()->setIntendedUrl($previous); } - - /** - * Check if login auto-initiate should be valid based upon authentication config. - */ - protected function shouldAutoInitiate(): bool - { - $socialDrivers = $this->socialAuthService->getActiveDrivers(); - $authMethod = config('auth.method'); - $autoRedirect = config('auth.auto_initiate'); - - return $autoRedirect && count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']); - } } diff --git a/app/Access/Controllers/OidcController.php b/app/Access/Controllers/OidcController.php index 083e83e35..055d4c140 100644 --- a/app/Access/Controllers/OidcController.php +++ b/app/Access/Controllers/OidcController.php @@ -11,9 +11,6 @@ class OidcController extends Controller { protected OidcService $oidcService; - /** - * OpenIdController constructor. - */ public function __construct(OidcService $oidcService) { $this->oidcService = $oidcService; @@ -65,16 +62,10 @@ class OidcController extends Controller } /** - * OIDC Logout Feature: Start the authorization logout flow via OIDC. + * Log the user out then start the OIDC RP-initiated logout process. */ public function logout() { - try { - return $this->oidcService->logout(); - } catch (OidcException $exception) { - $this->showErrorNotification($exception->getMessage()); - return redirect('/logout'); - } + return redirect($this->oidcService->logout()); } - } diff --git a/app/Access/Controllers/RegisterController.php b/app/Access/Controllers/RegisterController.php index 3c653a073..13b97f03c 100644 --- a/app/Access/Controllers/RegisterController.php +++ b/app/Access/Controllers/RegisterController.php @@ -4,7 +4,7 @@ namespace BookStack\Access\Controllers; use BookStack\Access\LoginService; use BookStack\Access\RegistrationService; -use BookStack\Access\SocialAuthService; +use BookStack\Access\SocialDriverManager; use BookStack\Exceptions\StoppedAuthenticationException; use BookStack\Exceptions\UserRegistrationException; use BookStack\Http\Controller; @@ -15,7 +15,7 @@ use Illuminate\Validation\Rules\Password; class RegisterController extends Controller { - protected SocialAuthService $socialAuthService; + protected SocialDriverManager $socialDriverManager; protected RegistrationService $registrationService; protected LoginService $loginService; @@ -23,14 +23,14 @@ class RegisterController extends Controller * Create a new controller instance. */ public function __construct( - SocialAuthService $socialAuthService, + SocialDriverManager $socialDriverManager, RegistrationService $registrationService, LoginService $loginService ) { $this->middleware('guest'); $this->middleware('guard:standard'); - $this->socialAuthService = $socialAuthService; + $this->socialDriverManager = $socialDriverManager; $this->registrationService = $registrationService; $this->loginService = $loginService; } @@ -43,7 +43,7 @@ class RegisterController extends Controller public function getRegister() { $this->registrationService->ensureRegistrationAllowed(); - $socialDrivers = $this->socialAuthService->getActiveDrivers(); + $socialDrivers = $this->socialDriverManager->getActive(); return view('auth.register', [ 'socialDrivers' => $socialDrivers, diff --git a/app/Access/Controllers/SocialController.php b/app/Access/Controllers/SocialController.php index ff6d5c2dd..bbdabb4ab 100644 --- a/app/Access/Controllers/SocialController.php +++ b/app/Access/Controllers/SocialController.php @@ -79,7 +79,7 @@ class SocialController extends Controller try { return $this->socialAuthService->handleLoginCallback($socialDriver, $socialUser); } catch (SocialSignInAccountNotUsed $exception) { - if ($this->socialAuthService->driverAutoRegisterEnabled($socialDriver)) { + if ($this->socialAuthService->drivers()->isAutoRegisterEnabled($socialDriver)) { return $this->socialRegisterCallback($socialDriver, $socialUser); } @@ -114,7 +114,7 @@ class SocialController extends Controller { $socialUser = $this->socialAuthService->handleRegistrationCallback($socialDriver, $socialUser); $socialAccount = $this->socialAuthService->newSocialAccount($socialDriver, $socialUser); - $emailVerified = $this->socialAuthService->driverAutoConfirmEmailEnabled($socialDriver); + $emailVerified = $this->socialAuthService->drivers()->isAutoConfirmEmailEnabled($socialDriver); // Create an array of the user data to create a new user instance $userData = [ diff --git a/app/Access/LoginService.php b/app/Access/LoginService.php index 27480ba21..f0f6ad4d3 100644 --- a/app/Access/LoginService.php +++ b/app/Access/LoginService.php @@ -16,13 +16,11 @@ class LoginService { protected const LAST_LOGIN_ATTEMPTED_SESSION_KEY = 'auth-login-last-attempted'; - protected $mfaSession; - protected $emailConfirmationService; - - public function __construct(MfaSession $mfaSession, EmailConfirmationService $emailConfirmationService) - { - $this->mfaSession = $mfaSession; - $this->emailConfirmationService = $emailConfirmationService; + public function __construct( + protected MfaSession $mfaSession, + protected EmailConfirmationService $emailConfirmationService, + protected SocialDriverManager $socialDriverManager, + ) { } /** @@ -163,4 +161,29 @@ class LoginService return $result; } + + /** + * Logs the current user out of the application. + * Returns an app post-redirect path. + */ + public function logout(): string + { + auth()->logout(); + session()->invalidate(); + session()->regenerateToken(); + + return $this->shouldAutoInitiate() ? '/login?prevent_auto_init=true' : '/'; + } + + /** + * Check if login auto-initiate should be valid based upon authentication config. + */ + protected function shouldAutoInitiate(): bool + { + $socialDrivers = $this->socialDriverManager->getActive(); + $authMethod = config('auth.method'); + $autoRedirect = config('auth.auto_initiate'); + + return $autoRedirect && count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']); + } } diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php index 1067b0832..be869b179 100644 --- a/app/Access/Oidc/OidcService.php +++ b/app/Access/Oidc/OidcService.php @@ -217,11 +217,7 @@ class OidcService $settings->keys, ); - // OIDC Logout Feature: Temporarily save token in session - $access_token_for_logout = $idTokenText; - session()->put("oidctoken", $access_token_for_logout); - - + session()->put("oidc_id_token", $idTokenText); $returnClaims = Theme::dispatch(ThemeEvents::OIDC_ID_TOKEN_PRE_VALIDATE, $idToken->getAllClaims(), [ 'access_token' => $accessToken->getToken(), @@ -291,36 +287,24 @@ class OidcService return $this->config()['user_to_groups'] !== false; } - /** - * OIDC Logout Feature: Initiate a logout flow. - * - * @throws OidcException - * - * @return string + * Start the RP-initiated logout flow if active, otherwise start a standard logout flow. + * Returns a post-app-logout redirect URL. + * Reference: https://openid.net/specs/openid-connect-rpinitiated-1_0.html */ - public function logout() { + public function logout(): string + { + $endSessionEndpoint = $this->config()["end_session_endpoint"]; - $config = $this->config(); - $app_url = env('APP_URL', ''); - $end_session_endpoint = $config["end_session_endpoint"]; - - $oidctoken = session()->get("oidctoken"); - session()->invalidate(); - - if (str_contains($app_url, 'https://')) { - $protocol = 'https://'; - } else { - $protocol = 'http://'; - } - - - - return redirect($end_session_endpoint.'?id_token_hint='.$oidctoken."&post_logout_redirect_uri=".$protocol.$_SERVER['HTTP_HOST']."/"); + // TODO - Add autodiscovery and false/null config value support. + $oidcToken = session()->pull("oidc_id_token"); + $defaultLogoutUrl = url($this->loginService->logout()); + $endpointParams = [ + 'id_token_hint' => $oidcToken, + 'post_logout_redirect_uri' => $defaultLogoutUrl, + ]; + return $endSessionEndpoint . '?' . http_build_query($endpointParams); } - - - } diff --git a/app/Access/Saml2Service.php b/app/Access/Saml2Service.php index 7f599762e..e7627f7e4 100644 --- a/app/Access/Saml2Service.php +++ b/app/Access/Saml2Service.php @@ -71,8 +71,7 @@ class Saml2Service throw $error; } - $this->actionLogout(); - $url = '/'; + $url = $this->loginService->logout(); $id = null; } @@ -140,20 +139,11 @@ class Saml2Service ); } - $this->actionLogout(); + $this->loginService->logout(); return $redirect; } - /** - * Do the required actions to log a user out. - */ - protected function actionLogout() - { - auth()->logout(); - session()->invalidate(); - } - /** * Get the metadata for this service provider. * diff --git a/app/Access/SocialAuthService.php b/app/Access/SocialAuthService.php index f0e0413f0..c3c20587d 100644 --- a/app/Access/SocialAuthService.php +++ b/app/Access/SocialAuthService.php @@ -2,69 +2,24 @@ namespace BookStack\Access; -use BookStack\Auth\Access\handler; use BookStack\Exceptions\SocialDriverNotConfigured; use BookStack\Exceptions\SocialSignInAccountNotUsed; use BookStack\Exceptions\UserRegistrationException; use BookStack\Users\Models\User; -use Illuminate\Support\Facades\Event; use Illuminate\Support\Str; use Laravel\Socialite\Contracts\Factory as Socialite; use Laravel\Socialite\Contracts\Provider; use Laravel\Socialite\Contracts\User as SocialUser; use Laravel\Socialite\Two\GoogleProvider; -use SocialiteProviders\Manager\SocialiteWasCalled; use Symfony\Component\HttpFoundation\RedirectResponse; class SocialAuthService { - /** - * The core socialite library used. - * - * @var Socialite - */ - protected $socialite; - - /** - * @var LoginService - */ - protected $loginService; - - /** - * The default built-in social drivers we support. - * - * @var string[] - */ - protected $validSocialDrivers = [ - 'google', - 'github', - 'facebook', - 'slack', - 'twitter', - 'azure', - 'okta', - 'gitlab', - 'twitch', - 'discord', - ]; - - /** - * Callbacks to run when configuring a social driver - * for an initial redirect action. - * Array is keyed by social driver name. - * Callbacks are passed an instance of the driver. - * - * @var array - */ - protected $configureForRedirectCallbacks = []; - - /** - * SocialAuthService constructor. - */ - public function __construct(Socialite $socialite, LoginService $loginService) - { - $this->socialite = $socialite; - $this->loginService = $loginService; + public function __construct( + protected Socialite $socialite, + protected LoginService $loginService, + protected SocialDriverManager $driverManager, + ) { } /** @@ -74,9 +29,10 @@ class SocialAuthService */ public function startLogIn(string $socialDriver): RedirectResponse { - $driver = $this->validateDriver($socialDriver); + $socialDriver = trim(strtolower($socialDriver)); + $this->driverManager->ensureDriverActive($socialDriver); - return $this->getDriverForRedirect($driver)->redirect(); + return $this->getDriverForRedirect($socialDriver)->redirect(); } /** @@ -86,9 +42,10 @@ class SocialAuthService */ public function startRegister(string $socialDriver): RedirectResponse { - $driver = $this->validateDriver($socialDriver); + $socialDriver = trim(strtolower($socialDriver)); + $this->driverManager->ensureDriverActive($socialDriver); - return $this->getDriverForRedirect($driver)->redirect(); + return $this->getDriverForRedirect($socialDriver)->redirect(); } /** @@ -119,9 +76,10 @@ class SocialAuthService */ public function getSocialUser(string $socialDriver): SocialUser { - $driver = $this->validateDriver($socialDriver); + $socialDriver = trim(strtolower($socialDriver)); + $this->driverManager->ensureDriverActive($socialDriver); - return $this->socialite->driver($driver)->user(); + return $this->socialite->driver($socialDriver)->user(); } /** @@ -131,6 +89,7 @@ class SocialAuthService */ public function handleLoginCallback(string $socialDriver, SocialUser $socialUser) { + $socialDriver = trim(strtolower($socialDriver)); $socialId = $socialUser->getId(); // Get any attached social accounts or users @@ -181,76 +140,11 @@ class SocialAuthService } /** - * Ensure the social driver is correct and supported. - * - * @throws SocialDriverNotConfigured + * Get the social driver manager used by this service. */ - protected function validateDriver(string $socialDriver): string + public function drivers(): SocialDriverManager { - $driver = trim(strtolower($socialDriver)); - - if (!in_array($driver, $this->validSocialDrivers)) { - abort(404, trans('errors.social_driver_not_found')); - } - - if (!$this->checkDriverConfigured($driver)) { - throw new SocialDriverNotConfigured(trans('errors.social_driver_not_configured', ['socialAccount' => Str::title($socialDriver)])); - } - - return $driver; - } - - /** - * Check a social driver has been configured correctly. - */ - protected function checkDriverConfigured(string $driver): bool - { - $lowerName = strtolower($driver); - $configPrefix = 'services.' . $lowerName . '.'; - $config = [config($configPrefix . 'client_id'), config($configPrefix . 'client_secret'), config('services.callback_url')]; - - return !in_array(false, $config) && !in_array(null, $config); - } - - /** - * Gets the names of the active social drivers. - * @returns array - */ - public function getActiveDrivers(): array - { - $activeDrivers = []; - - foreach ($this->validSocialDrivers as $driverKey) { - if ($this->checkDriverConfigured($driverKey)) { - $activeDrivers[$driverKey] = $this->getDriverName($driverKey); - } - } - - return $activeDrivers; - } - - /** - * Get the presentational name for a driver. - */ - public function getDriverName(string $driver): string - { - return config('services.' . strtolower($driver) . '.name'); - } - - /** - * Check if the current config for the given driver allows auto-registration. - */ - public function driverAutoRegisterEnabled(string $driver): bool - { - return config('services.' . strtolower($driver) . '.auto_register') === true; - } - - /** - * Check if the current config for the given driver allow email address auto-confirmation. - */ - public function driverAutoConfirmEmailEnabled(string $driver): bool - { - return config('services.' . strtolower($driver) . '.auto_confirm') === true; + return $this->driverManager; } /** @@ -284,33 +178,8 @@ class SocialAuthService $driver->with(['prompt' => 'select_account']); } - if (isset($this->configureForRedirectCallbacks[$driverName])) { - $this->configureForRedirectCallbacks[$driverName]($driver); - } + $this->driverManager->getConfigureForRedirectCallback($driverName)($driver); return $driver; } - - /** - * Add a custom socialite driver to be used. - * Driver name should be lower_snake_case. - * Config array should mirror the structure of a service - * within the `Config/services.php` file. - * Handler should be a Class@method handler to the SocialiteWasCalled event. - */ - public function addSocialDriver( - string $driverName, - array $config, - string $socialiteHandler, - callable $configureForRedirect = null - ) { - $this->validSocialDrivers[] = $driverName; - config()->set('services.' . $driverName, $config); - config()->set('services.' . $driverName . '.redirect', url('/login/service/' . $driverName . '/callback')); - config()->set('services.' . $driverName . '.name', $config['name'] ?? $driverName); - Event::listen(SocialiteWasCalled::class, $socialiteHandler); - if (!is_null($configureForRedirect)) { - $this->configureForRedirectCallbacks[$driverName] = $configureForRedirect; - } - } } diff --git a/app/Access/SocialDriverManager.php b/app/Access/SocialDriverManager.php new file mode 100644 index 000000000..536b2e63d --- /dev/null +++ b/app/Access/SocialDriverManager.php @@ -0,0 +1,147 @@ + + */ + protected array $configureForRedirectCallbacks = []; + + /** + * Check if the current config for the given driver allows auto-registration. + */ + public function isAutoRegisterEnabled(string $driver): bool + { + return $this->getDriverConfigProperty($driver, 'auto_register') === true; + } + + /** + * Check if the current config for the given driver allow email address auto-confirmation. + */ + public function isAutoConfirmEmailEnabled(string $driver): bool + { + return $this->getDriverConfigProperty($driver, 'auto_confirm') === true; + } + + /** + * Gets the names of the active social drivers, keyed by driver id. + * @returns array + */ + public function getActive(): array + { + $activeDrivers = []; + + foreach ($this->validDrivers as $driverKey) { + if ($this->checkDriverConfigured($driverKey)) { + $activeDrivers[$driverKey] = $this->getName($driverKey); + } + } + + return $activeDrivers; + } + + /** + * Get the configure-for-redirect callback for the given driver. + * This is a callable that allows modification of the driver at redirect time. + * Commonly used to perform custom dynamic configuration where required. + * The callback is passed a \Laravel\Socialite\Contracts\Provider instance. + */ + public function getConfigureForRedirectCallback(string $driver): callable + { + return $this->configureForRedirectCallbacks[$driver] ?? (fn() => true); + } + + /** + * Add a custom socialite driver to be used. + * Driver name should be lower_snake_case. + * Config array should mirror the structure of a service + * within the `Config/services.php` file. + * Handler should be a Class@method handler to the SocialiteWasCalled event. + */ + public function addSocialDriver( + string $driverName, + array $config, + string $socialiteHandler, + callable $configureForRedirect = null + ) { + $this->validDrivers[] = $driverName; + config()->set('services.' . $driverName, $config); + config()->set('services.' . $driverName . '.redirect', url('/login/service/' . $driverName . '/callback')); + config()->set('services.' . $driverName . '.name', $config['name'] ?? $driverName); + Event::listen(SocialiteWasCalled::class, $socialiteHandler); + if (!is_null($configureForRedirect)) { + $this->configureForRedirectCallbacks[$driverName] = $configureForRedirect; + } + } + + /** + * Get the presentational name for a driver. + */ + protected function getName(string $driver): string + { + return $this->getDriverConfigProperty($driver, 'name') ?? ''; + } + + protected function getDriverConfigProperty(string $driver, string $property): mixed + { + return config("services.{$driver}.{$property}"); + } + + /** + * Ensure the social driver is correct and supported. + * + * @throws SocialDriverNotConfigured + */ + public function ensureDriverActive(string $driverName): void + { + if (!in_array($driverName, $this->validDrivers)) { + abort(404, trans('errors.social_driver_not_found')); + } + + if (!$this->checkDriverConfigured($driverName)) { + throw new SocialDriverNotConfigured(trans('errors.social_driver_not_configured', ['socialAccount' => Str::title($driverName)])); + } + } + + /** + * Check a social driver has been configured correctly. + */ + protected function checkDriverConfigured(string $driver): bool + { + $lowerName = strtolower($driver); + $configPrefix = 'services.' . $lowerName . '.'; + $config = [config($configPrefix . 'client_id'), config($configPrefix . 'client_secret'), config('services.callback_url')]; + + return !in_array(false, $config) && !in_array(null, $config); + } +} diff --git a/app/App/Providers/AppServiceProvider.php b/app/App/Providers/AppServiceProvider.php index 0275a5489..0f4dc55dd 100644 --- a/app/App/Providers/AppServiceProvider.php +++ b/app/App/Providers/AppServiceProvider.php @@ -2,7 +2,7 @@ namespace BookStack\App\Providers; -use BookStack\Access\SocialAuthService; +use BookStack\Access\SocialDriverManager; use BookStack\Activity\Tools\ActivityLogger; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; @@ -36,7 +36,7 @@ class AppServiceProvider extends ServiceProvider public $singletons = [ 'activity' => ActivityLogger::class, SettingService::class => SettingService::class, - SocialAuthService::class => SocialAuthService::class, + SocialDriverManager::class => SocialDriverManager::class, CspService::class => CspService::class, HttpRequestService::class => HttpRequestService::class, ]; diff --git a/app/Config/oidc.php b/app/Config/oidc.php index 0410588b8..5f61063f6 100644 --- a/app/Config/oidc.php +++ b/app/Config/oidc.php @@ -36,6 +36,12 @@ return [ 'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null), 'token_endpoint' => env('OIDC_TOKEN_ENDPOINT', null), + // OIDC RP-Initiated Logout endpoint + // A null value gets the URL from discovery, if active. + // A false value force-disables RP-Initiated Logout. + // A string value forces the given URL to be used. + 'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null), + // Add extra scopes, upon those required, to the OIDC authentication request // Multiple values can be provided comma seperated. 'additional_scopes' => env('OIDC_ADDITIONAL_SCOPES', null), @@ -45,11 +51,6 @@ return [ 'user_to_groups' => env('OIDC_USER_TO_GROUPS', false), // Attribute, within a OIDC ID token, to find group names within 'groups_claim' => env('OIDC_GROUPS_CLAIM', 'groups'), - // When syncing groups, remove any groups that no longer match. Otherwise sync only adds new groups. + // When syncing groups, remove any groups that no longer match. Otherwise, sync only adds new groups. 'remove_from_groups' => env('OIDC_REMOVE_FROM_GROUPS', false), - - // OIDC Logout Feature: OAuth2 end_session_endpoint - 'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null), - ]; - diff --git a/app/Theming/ThemeService.php b/app/Theming/ThemeService.php index 0c2526536..94e471217 100644 --- a/app/Theming/ThemeService.php +++ b/app/Theming/ThemeService.php @@ -2,7 +2,7 @@ namespace BookStack\Theming; -use BookStack\Access\SocialAuthService; +use BookStack\Access\SocialDriverManager; use BookStack\Exceptions\ThemeException; use Illuminate\Console\Application; use Illuminate\Console\Application as Artisan; @@ -82,11 +82,11 @@ class ThemeService } /** - * @see SocialAuthService::addSocialDriver + * @see SocialDriverManager::addSocialDriver */ public function addSocialDriver(string $driverName, array $config, string $socialiteHandler, callable $configureForRedirect = null): void { - $socialAuthService = app()->make(SocialAuthService::class); - $socialAuthService->addSocialDriver($driverName, $config, $socialiteHandler, $configureForRedirect); + $driverManager = app()->make(SocialDriverManager::class); + $driverManager->addSocialDriver($driverName, $config, $socialiteHandler, $configureForRedirect); } } diff --git a/app/Users/Controllers/UserAccountController.php b/app/Users/Controllers/UserAccountController.php index 55776a7f6..708a91e9d 100644 --- a/app/Users/Controllers/UserAccountController.php +++ b/app/Users/Controllers/UserAccountController.php @@ -2,7 +2,7 @@ namespace BookStack\Users\Controllers; -use BookStack\Access\SocialAuthService; +use BookStack\Access\SocialDriverManager; use BookStack\Http\Controller; use BookStack\Permissions\PermissionApplicator; use BookStack\Settings\UserNotificationPreferences; @@ -161,7 +161,7 @@ class UserAccountController extends Controller /** * Show the view for the "Access & Security" account options. */ - public function showAuth(SocialAuthService $socialAuthService) + public function showAuth(SocialDriverManager $socialDriverManager) { $mfaMethods = user()->mfaValues()->get()->groupBy('method'); @@ -171,7 +171,7 @@ class UserAccountController extends Controller 'category' => 'auth', 'mfaMethods' => $mfaMethods, 'authMethod' => config('auth.method'), - 'activeSocialDrivers' => $socialAuthService->getActiveDrivers(), + 'activeSocialDrivers' => $socialDriverManager->getActive(), ]); } diff --git a/app/Users/Controllers/UserController.php b/app/Users/Controllers/UserController.php index 507c7cf06..185d6101c 100644 --- a/app/Users/Controllers/UserController.php +++ b/app/Users/Controllers/UserController.php @@ -2,7 +2,7 @@ namespace BookStack\Users\Controllers; -use BookStack\Access\SocialAuthService; +use BookStack\Access\SocialDriverManager; use BookStack\Exceptions\ImageUploadException; use BookStack\Exceptions\UserUpdateException; use BookStack\Http\Controller; @@ -101,7 +101,7 @@ class UserController extends Controller /** * Show the form for editing the specified user. */ - public function edit(int $id, SocialAuthService $socialAuthService) + public function edit(int $id, SocialDriverManager $socialDriverManager) { $this->checkPermission('users-manage'); @@ -109,7 +109,7 @@ class UserController extends Controller $user->load(['apiTokens', 'mfaValues']); $authMethod = ($user->system_name) ? 'system' : config('auth.method'); - $activeSocialDrivers = $socialAuthService->getActiveDrivers(); + $activeSocialDrivers = $socialDriverManager->getActive(); $mfaMethods = $user->mfaValues->groupBy('method'); $this->setPageTitle(trans('settings.user_profile')); $roles = Role::query()->orderBy('display_name', 'asc')->get(); diff --git a/resources/views/layouts/parts/header-user-menu.blade.php b/resources/views/layouts/parts/header-user-menu.blade.php index ff28f1cfb..db4820a4d 100644 --- a/resources/views/layouts/parts/header-user-menu.blade.php +++ b/resources/views/layouts/parts/header-user-menu.blade.php @@ -29,28 +29,20 @@
  • - -
    - - - - {{ csrf_field() }} - -
    + @php + $logoutPath = match (config('auth.method')) { + 'saml2' => '/saml2/logout', + 'oidc' => '/oidc/logout', + default => '/logout', + } + @endphp +
    + {{ csrf_field() }} + +
    • \ No newline at end of file diff --git a/routes/web.php b/routes/web.php index a02b19ca3..8fc90ee54 100644 --- a/routes/web.php +++ b/routes/web.php @@ -332,8 +332,7 @@ Route::get('/saml2/acs', [AccessControllers\Saml2Controller::class, 'processAcs' // OIDC routes Route::post('/oidc/login', [AccessControllers\OidcController::class, 'login']); Route::get('/oidc/callback', [AccessControllers\OidcController::class, 'callback']); -// OIDC Logout Feature: Added to cater OIDC logout -Route::get('/oidc/logout', [AccessControllers\OidcController::class, 'logout']); +Route::post('/oidc/logout', [AccessControllers\OidcController::class, 'logout']); // User invitation routes Route::get('/register/invite/{token}', [AccessControllers\UserInviteController::class, 'showSetPassword']);