mirror of
				https://github.com/BookStackApp/BookStack.git
				synced 2025-10-29 16:09:29 +03:00 
			
		
		
		
	OIDC RP Logout: Added autodiscovery support and test cases
This commit is contained in:
		| @@ -10,27 +10,19 @@ use BookStack\Facades\Activity; | |||||||
| use BookStack\Http\Controller; | use BookStack\Http\Controller; | ||||||
| use Illuminate\Http\RedirectResponse; | use Illuminate\Http\RedirectResponse; | ||||||
| use Illuminate\Http\Request; | use Illuminate\Http\Request; | ||||||
| use Illuminate\Support\Facades\Auth; |  | ||||||
| use Illuminate\Validation\ValidationException; | use Illuminate\Validation\ValidationException; | ||||||
|  |  | ||||||
| class LoginController extends Controller | class LoginController extends Controller | ||||||
| { | { | ||||||
|     use ThrottlesLogins; |     use ThrottlesLogins; | ||||||
|  |  | ||||||
|     protected SocialDriverManager $socialDriverManager; |     public function __construct( | ||||||
|     protected LoginService $loginService; |         protected SocialDriverManager $socialDriverManager, | ||||||
|  |         protected LoginService $loginService, | ||||||
|     /** |     ) { | ||||||
|      * Create a new controller instance. |  | ||||||
|      */ |  | ||||||
|     public function __construct(SocialDriverManager $driverManager, LoginService $loginService) |  | ||||||
|     { |  | ||||||
|         $this->middleware('guest', ['only' => ['getLogin', 'login']]); |         $this->middleware('guest', ['only' => ['getLogin', 'login']]); | ||||||
|         $this->middleware('guard:standard,ldap', ['only' => ['login']]); |         $this->middleware('guard:standard,ldap', ['only' => ['login']]); | ||||||
|         $this->middleware('guard:standard,ldap,oidc', ['only' => ['logout']]); |         $this->middleware('guard:standard,ldap,oidc', ['only' => ['logout']]); | ||||||
|  |  | ||||||
|         $this->socialDriverManager = $driverManager; |  | ||||||
|         $this->loginService = $loginService; |  | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     /** |     /** | ||||||
| @@ -52,7 +44,7 @@ class LoginController extends Controller | |||||||
|         // Store the previous location for redirect after login |         // Store the previous location for redirect after login | ||||||
|         $this->updateIntendedFromPrevious(); |         $this->updateIntendedFromPrevious(); | ||||||
|  |  | ||||||
|         if (!$preventInitiation && $this->shouldAutoInitiate()) { |         if (!$preventInitiation && $this->loginService->shouldAutoInitiate()) { | ||||||
|             return view('auth.login-initiate', [ |             return view('auth.login-initiate', [ | ||||||
|                 'authMethod'    => $authMethod, |                 'authMethod'    => $authMethod, | ||||||
|             ]); |             ]); | ||||||
| @@ -194,7 +186,7 @@ class LoginController extends Controller | |||||||
|     { |     { | ||||||
|         // Store the previous location for redirect after login |         // Store the previous location for redirect after login | ||||||
|         $previous = url()->previous(''); |         $previous = url()->previous(''); | ||||||
|         $isPreviousFromInstance = (strpos($previous, url('/')) === 0); |         $isPreviousFromInstance = str_starts_with($previous, url('/')); | ||||||
|         if (!$previous || !setting('app-public') || !$isPreviousFromInstance) { |         if (!$previous || !setting('app-public') || !$isPreviousFromInstance) { | ||||||
|             return; |             return; | ||||||
|         } |         } | ||||||
| @@ -205,7 +197,7 @@ class LoginController extends Controller | |||||||
|         ]; |         ]; | ||||||
|  |  | ||||||
|         foreach ($ignorePrefixList as $ignorePrefix) { |         foreach ($ignorePrefixList as $ignorePrefix) { | ||||||
|             if (strpos($previous, url($ignorePrefix)) === 0) { |             if (str_starts_with($previous, url($ignorePrefix))) { | ||||||
|                 return; |                 return; | ||||||
|             } |             } | ||||||
|         } |         } | ||||||
|   | |||||||
| @@ -176,14 +176,18 @@ class LoginService | |||||||
|     } |     } | ||||||
|  |  | ||||||
|     /** |     /** | ||||||
|      * Check if login auto-initiate should be valid based upon authentication config. |      * Check if login auto-initiate should be active based upon authentication config. | ||||||
|      */ |      */ | ||||||
|     protected function shouldAutoInitiate(): bool |     public function shouldAutoInitiate(): bool | ||||||
|     { |     { | ||||||
|  |         $autoRedirect = config('auth.auto_initiate'); | ||||||
|  |         if (!$autoRedirect) { | ||||||
|  |             return false; | ||||||
|  |         } | ||||||
|  |  | ||||||
|         $socialDrivers = $this->socialDriverManager->getActive(); |         $socialDrivers = $this->socialDriverManager->getActive(); | ||||||
|         $authMethod = config('auth.method'); |         $authMethod = config('auth.method'); | ||||||
|         $autoRedirect = config('auth.auto_initiate'); |  | ||||||
|  |  | ||||||
|         return $autoRedirect && count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']); |         return count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']); | ||||||
|     } |     } | ||||||
| } | } | ||||||
|   | |||||||
| @@ -21,6 +21,7 @@ class OidcProviderSettings | |||||||
|     public ?string $redirectUri; |     public ?string $redirectUri; | ||||||
|     public ?string $authorizationEndpoint; |     public ?string $authorizationEndpoint; | ||||||
|     public ?string $tokenEndpoint; |     public ?string $tokenEndpoint; | ||||||
|  |     public ?string $endSessionEndpoint; | ||||||
|  |  | ||||||
|     /** |     /** | ||||||
|      * @var string[]|array[] |      * @var string[]|array[] | ||||||
| @@ -132,6 +133,10 @@ class OidcProviderSettings | |||||||
|             $discoveredSettings['keys'] = $this->filterKeys($keys); |             $discoveredSettings['keys'] = $this->filterKeys($keys); | ||||||
|         } |         } | ||||||
|  |  | ||||||
|  |         if (!empty($result['end_session_endpoint'])) { | ||||||
|  |             $discoveredSettings['endSessionEndpoint'] = $result['end_session_endpoint']; | ||||||
|  |         } | ||||||
|  |  | ||||||
|         return $discoveredSettings; |         return $discoveredSettings; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -84,6 +84,7 @@ class OidcService | |||||||
|             'redirectUri'           => url('/oidc/callback'), |             'redirectUri'           => url('/oidc/callback'), | ||||||
|             'authorizationEndpoint' => $config['authorization_endpoint'], |             'authorizationEndpoint' => $config['authorization_endpoint'], | ||||||
|             'tokenEndpoint'         => $config['token_endpoint'], |             'tokenEndpoint'         => $config['token_endpoint'], | ||||||
|  |             'endSessionEndpoint'    => $config['end_session_endpoint'], | ||||||
|         ]); |         ]); | ||||||
|  |  | ||||||
|         // Use keys if configured |         // Use keys if configured | ||||||
| @@ -100,6 +101,11 @@ class OidcService | |||||||
|             } |             } | ||||||
|         } |         } | ||||||
|  |  | ||||||
|  |         // Prevent use of RP-initiated logout if specifically disabled | ||||||
|  |         if ($config['end_session_endpoint'] === false) { | ||||||
|  |             $settings->endSessionEndpoint = null; | ||||||
|  |         } | ||||||
|  |  | ||||||
|         $settings->validate(); |         $settings->validate(); | ||||||
|  |  | ||||||
|         return $settings; |         return $settings; | ||||||
| @@ -291,20 +297,23 @@ class OidcService | |||||||
|      * Start the RP-initiated logout flow if active, otherwise start a standard logout flow. |      * Start the RP-initiated logout flow if active, otherwise start a standard logout flow. | ||||||
|      * Returns a post-app-logout redirect URL. |      * Returns a post-app-logout redirect URL. | ||||||
|      * Reference: https://openid.net/specs/openid-connect-rpinitiated-1_0.html |      * Reference: https://openid.net/specs/openid-connect-rpinitiated-1_0.html | ||||||
|  |      * @throws OidcException | ||||||
|      */ |      */ | ||||||
|     public function logout(): string |     public function logout(): string | ||||||
|     { |     { | ||||||
|         $endSessionEndpoint = $this->config()["end_session_endpoint"]; |  | ||||||
|  |  | ||||||
|         // TODO - Add autodiscovery and false/null config value support. |  | ||||||
|  |  | ||||||
|         $oidcToken = session()->pull("oidc_id_token"); |         $oidcToken = session()->pull("oidc_id_token"); | ||||||
|         $defaultLogoutUrl = url($this->loginService->logout()); |         $defaultLogoutUrl = url($this->loginService->logout()); | ||||||
|  |         $oidcSettings = $this->getProviderSettings(); | ||||||
|  |  | ||||||
|  |         if (!$oidcSettings->endSessionEndpoint) { | ||||||
|  |             return $defaultLogoutUrl; | ||||||
|  |         } | ||||||
|  |  | ||||||
|         $endpointParams = [ |         $endpointParams = [ | ||||||
|             'id_token_hint' => $oidcToken, |             'id_token_hint' => $oidcToken, | ||||||
|             'post_logout_redirect_uri' => $defaultLogoutUrl, |             'post_logout_redirect_uri' => $defaultLogoutUrl, | ||||||
|         ]; |         ]; | ||||||
|  |  | ||||||
|         return $endSessionEndpoint . '?' . http_build_query($endpointParams); |         return $oidcSettings->endSessionEndpoint . '?' . http_build_query($endpointParams); | ||||||
|     } |     } | ||||||
| } | } | ||||||
|   | |||||||
| @@ -36,10 +36,9 @@ return [ | |||||||
|     'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null), |     'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null), | ||||||
|     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null), |     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null), | ||||||
|  |  | ||||||
|     // OIDC RP-Initiated Logout endpoint |     // OIDC RP-Initiated Logout endpoint URL. | ||||||
|     // A null value gets the URL from discovery, if active. |     // A null value gets the URL from discovery, if active. | ||||||
|     // A false value force-disables RP-Initiated Logout. |     // 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), |     'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null), | ||||||
|  |  | ||||||
|     // Add extra scopes, upon those required, to the OIDC authentication request |     // Add extra scopes, upon those required, to the OIDC authentication request | ||||||
|   | |||||||
| @@ -44,6 +44,7 @@ class OidcTest extends TestCase | |||||||
|             'oidc.groups_claim'           => 'group', |             'oidc.groups_claim'           => 'group', | ||||||
|             'oidc.remove_from_groups'     => false, |             'oidc.remove_from_groups'     => false, | ||||||
|             'oidc.external_id_claim'      => 'sub', |             'oidc.external_id_claim'      => 'sub', | ||||||
|  |             'oidc.end_session_endpoint'   => null, | ||||||
|         ]); |         ]); | ||||||
|     } |     } | ||||||
|  |  | ||||||
| @@ -478,6 +479,81 @@ class OidcTest extends TestCase | |||||||
|         $this->assertTrue($user->hasRole($roleA->id)); |         $this->assertTrue($user->hasRole($roleA->id)); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  |     public function test_oidc_logout_form_active_when_oidc_active() | ||||||
|  |     { | ||||||
|  |         $this->runLogin(); | ||||||
|  |  | ||||||
|  |         $resp = $this->get('/'); | ||||||
|  |         $this->withHtml($resp)->assertElementExists('header form[action$="/oidc/logout"] button'); | ||||||
|  |     } | ||||||
|  |     public function test_logout_with_autodiscovery() | ||||||
|  |     { | ||||||
|  |         $this->withAutodiscovery(); | ||||||
|  |  | ||||||
|  |         $transactions = $this->mockHttpClient([ | ||||||
|  |             $this->getAutoDiscoveryResponse(), | ||||||
|  |             $this->getJwksResponse(), | ||||||
|  |         ]); | ||||||
|  |  | ||||||
|  |         $resp = $this->asEditor()->post('/oidc/logout'); | ||||||
|  |         $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode(url('/'))); | ||||||
|  |  | ||||||
|  |         $this->assertEquals(2, $transactions->requestCount()); | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     public function test_logout_with_autodiscovery_but_oidc_logout_disabled() | ||||||
|  |     { | ||||||
|  |         $this->withAutodiscovery(); | ||||||
|  |         config()->set(['oidc.end_session_endpoint' => false]); | ||||||
|  |  | ||||||
|  |         $this->mockHttpClient([ | ||||||
|  |             $this->getAutoDiscoveryResponse(), | ||||||
|  |             $this->getJwksResponse(), | ||||||
|  |         ]); | ||||||
|  |  | ||||||
|  |         $resp = $this->asEditor()->post('/oidc/logout'); | ||||||
|  |         $resp->assertRedirect('/'); | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     public function test_logout_without_autodiscovery_but_with_endpoint_configured() | ||||||
|  |     { | ||||||
|  |         config()->set(['oidc.end_session_endpoint' => 'https://example.com/logout']); | ||||||
|  |  | ||||||
|  |         $resp = $this->asEditor()->post('/oidc/logout'); | ||||||
|  |         $resp->assertRedirect('https://example.com/logout?post_logout_redirect_uri=' . urlencode(url('/'))); | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     public function test_logout_with_autodiscovery_and_auto_initiate_returns_to_auto_prevented_login() | ||||||
|  |     { | ||||||
|  |         $this->withAutodiscovery(); | ||||||
|  |         config()->set([ | ||||||
|  |             'auth.auto_initiate' => true, | ||||||
|  |             'services.google.client_id' => false, | ||||||
|  |             'services.github.client_id' => false, | ||||||
|  |         ]); | ||||||
|  |  | ||||||
|  |         $this->mockHttpClient([ | ||||||
|  |             $this->getAutoDiscoveryResponse(), | ||||||
|  |             $this->getJwksResponse(), | ||||||
|  |         ]); | ||||||
|  |  | ||||||
|  |         $resp = $this->asEditor()->post('/oidc/logout'); | ||||||
|  |  | ||||||
|  |         $redirectUrl = url('/login?prevent_auto_init=true'); | ||||||
|  |         $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode($redirectUrl)); | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     public function test_logout_redirect_contains_id_token_hint_if_existing() | ||||||
|  |     { | ||||||
|  |         config()->set(['oidc.end_session_endpoint' => 'https://example.com/logout']); | ||||||
|  |  | ||||||
|  |         $this->runLogin(); | ||||||
|  |  | ||||||
|  |         $resp = $this->asEditor()->post('/oidc/logout'); | ||||||
|  |         $query = 'id_token_hint=' . urlencode(OidcJwtHelper::idToken()) .  '&post_logout_redirect_uri=' . urlencode(url('/')); | ||||||
|  |         $resp->assertRedirect('https://example.com/logout?' . $query); | ||||||
|  |     } | ||||||
|  |  | ||||||
|     public function test_oidc_id_token_pre_validate_theme_event_without_return() |     public function test_oidc_id_token_pre_validate_theme_event_without_return() | ||||||
|     { |     { | ||||||
|         $args = []; |         $args = []; | ||||||
| @@ -563,6 +639,7 @@ class OidcTest extends TestCase | |||||||
|             'authorization_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/authorize', |             'authorization_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/authorize', | ||||||
|             'jwks_uri'               => OidcJwtHelper::defaultIssuer() . '/oidc/keys', |             'jwks_uri'               => OidcJwtHelper::defaultIssuer() . '/oidc/keys', | ||||||
|             'issuer'                 => OidcJwtHelper::defaultIssuer(), |             'issuer'                 => OidcJwtHelper::defaultIssuer(), | ||||||
|  |             'end_session_endpoint'   => OidcJwtHelper::defaultIssuer() . '/oidc/logout', | ||||||
|         ], $responseOverrides))); |         ], $responseOverrides))); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user