diff --git a/app/Access/Controllers/LoginController.php b/app/Access/Controllers/LoginController.php
index 904736656..ce872ba88 100644
--- a/app/Access/Controllers/LoginController.php
+++ b/app/Access/Controllers/LoginController.php
@@ -10,27 +10,19 @@ use BookStack\Facades\Activity;
use BookStack\Http\Controller;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Auth;
use Illuminate\Validation\ValidationException;
class LoginController extends Controller
{
use ThrottlesLogins;
- protected SocialDriverManager $socialDriverManager;
- protected LoginService $loginService;
-
- /**
- * Create a new controller instance.
- */
- public function __construct(SocialDriverManager $driverManager, LoginService $loginService)
- {
+ public function __construct(
+ protected SocialDriverManager $socialDriverManager,
+ protected LoginService $loginService,
+ ) {
$this->middleware('guest', ['only' => ['getLogin', 'login']]);
$this->middleware('guard:standard,ldap', ['only' => ['login']]);
$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
$this->updateIntendedFromPrevious();
- if (!$preventInitiation && $this->shouldAutoInitiate()) {
+ if (!$preventInitiation && $this->loginService->shouldAutoInitiate()) {
return view('auth.login-initiate', [
'authMethod' => $authMethod,
]);
@@ -194,7 +186,7 @@ class LoginController extends Controller
{
// Store the previous location for redirect after login
$previous = url()->previous('');
- $isPreviousFromInstance = (strpos($previous, url('/')) === 0);
+ $isPreviousFromInstance = str_starts_with($previous, url('/'));
if (!$previous || !setting('app-public') || !$isPreviousFromInstance) {
return;
}
@@ -205,7 +197,7 @@ class LoginController extends Controller
];
foreach ($ignorePrefixList as $ignorePrefix) {
- if (strpos($previous, url($ignorePrefix)) === 0) {
+ if (str_starts_with($previous, url($ignorePrefix))) {
return;
}
}
diff --git a/app/Access/LoginService.php b/app/Access/LoginService.php
index f0f6ad4d3..cc48e0f9b 100644
--- a/app/Access/LoginService.php
+++ b/app/Access/LoginService.php
@@ -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();
$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']);
}
}
diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php
index fa3f579b1..bea6a523e 100644
--- a/app/Access/Oidc/OidcProviderSettings.php
+++ b/app/Access/Oidc/OidcProviderSettings.php
@@ -21,6 +21,7 @@ class OidcProviderSettings
public ?string $redirectUri;
public ?string $authorizationEndpoint;
public ?string $tokenEndpoint;
+ public ?string $endSessionEndpoint;
/**
* @var string[]|array[]
@@ -132,6 +133,10 @@ class OidcProviderSettings
$discoveredSettings['keys'] = $this->filterKeys($keys);
}
+ if (!empty($result['end_session_endpoint'])) {
+ $discoveredSettings['endSessionEndpoint'] = $result['end_session_endpoint'];
+ }
+
return $discoveredSettings;
}
diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index be869b179..3f9cd41b4 100644
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -84,6 +84,7 @@ class OidcService
'redirectUri' => url('/oidc/callback'),
'authorizationEndpoint' => $config['authorization_endpoint'],
'tokenEndpoint' => $config['token_endpoint'],
+ 'endSessionEndpoint' => $config['end_session_endpoint'],
]);
// 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();
return $settings;
@@ -291,20 +297,23 @@ class OidcService
* 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
+ * @throws OidcException
*/
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");
$defaultLogoutUrl = url($this->loginService->logout());
+ $oidcSettings = $this->getProviderSettings();
+
+ if (!$oidcSettings->endSessionEndpoint) {
+ return $defaultLogoutUrl;
+ }
+
$endpointParams = [
'id_token_hint' => $oidcToken,
'post_logout_redirect_uri' => $defaultLogoutUrl,
];
- return $endSessionEndpoint . '?' . http_build_query($endpointParams);
+ return $oidcSettings->endSessionEndpoint . '?' . http_build_query($endpointParams);
}
}
diff --git a/app/Config/oidc.php b/app/Config/oidc.php
index 5f61063f6..ed9302d10 100644
--- a/app/Config/oidc.php
+++ b/app/Config/oidc.php
@@ -36,10 +36,9 @@ return [
'authorization_endpoint' => env('OIDC_AUTH_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 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
diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 204a3bb5f..d10582d8c 100644
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -44,6 +44,7 @@ class OidcTest extends TestCase
'oidc.groups_claim' => 'group',
'oidc.remove_from_groups' => false,
'oidc.external_id_claim' => 'sub',
+ 'oidc.end_session_endpoint' => null,
]);
}
@@ -478,6 +479,81 @@ class OidcTest extends TestCase
$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()
{
$args = [];
@@ -563,6 +639,7 @@ class OidcTest extends TestCase
'authorization_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/authorize',
'jwks_uri' => OidcJwtHelper::defaultIssuer() . '/oidc/keys',
'issuer' => OidcJwtHelper::defaultIssuer(),
+ 'end_session_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/logout',
], $responseOverrides)));
}