mirror of
https://github.com/BookStackApp/BookStack.git
synced 2024-10-01 01:36:00 -04:00
OIDC RP Logout: Fixed issues during testing
- Disabled by default due to strict rejection by auth systems. - Fixed issue when autoloading logout URL, but not provided in autodiscovery response. - Added proper handling for if the logout URL contains a query string already. - Added extra tests to cover. - Forced config endpoint to be used, if set as a string, instead of autodiscovery endpoint.
This commit is contained in:
parent
a72e0fee70
commit
81d256aebd
@ -84,7 +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'],
|
'endSessionEndpoint' => is_string($config['end_session_endpoint']) ? $config['end_session_endpoint'] : null,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
// Use keys if configured
|
// Use keys if configured
|
||||||
@ -102,8 +102,11 @@ class OidcService
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Prevent use of RP-initiated logout if specifically disabled
|
// Prevent use of RP-initiated logout if specifically disabled
|
||||||
|
// Or force use of a URL if specifically set.
|
||||||
if ($config['end_session_endpoint'] === false) {
|
if ($config['end_session_endpoint'] === false) {
|
||||||
$settings->endSessionEndpoint = null;
|
$settings->endSessionEndpoint = null;
|
||||||
|
} else if (is_string($config['end_session_endpoint'])) {
|
||||||
|
$settings->endSessionEndpoint = $config['end_session_endpoint'];
|
||||||
}
|
}
|
||||||
|
|
||||||
$settings->validate();
|
$settings->validate();
|
||||||
@ -314,6 +317,8 @@ class OidcService
|
|||||||
'post_logout_redirect_uri' => $defaultLogoutUrl,
|
'post_logout_redirect_uri' => $defaultLogoutUrl,
|
||||||
];
|
];
|
||||||
|
|
||||||
return $oidcSettings->endSessionEndpoint . '?' . http_build_query($endpointParams);
|
$joiner = str_contains($oidcSettings->endSessionEndpoint, '?') ? '&' : '?';
|
||||||
|
|
||||||
|
return $oidcSettings->endSessionEndpoint . $joiner . http_build_query($endpointParams);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -37,9 +37,10 @@ return [
|
|||||||
'token_endpoint' => env('OIDC_TOKEN_ENDPOINT', null),
|
'token_endpoint' => env('OIDC_TOKEN_ENDPOINT', null),
|
||||||
|
|
||||||
// OIDC RP-Initiated Logout endpoint URL.
|
// 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 false value force-disables RP-Initiated Logout.
|
||||||
'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null),
|
// A true value gets the URL from discovery, if active.
|
||||||
|
// A string value is used as the URL.
|
||||||
|
'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', false),
|
||||||
|
|
||||||
// Add extra scopes, upon those required, to the OIDC authentication request
|
// Add extra scopes, upon those required, to the OIDC authentication request
|
||||||
// Multiple values can be provided comma seperated.
|
// Multiple values can be provided comma seperated.
|
||||||
|
@ -44,7 +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,
|
'oidc.end_session_endpoint' => false,
|
||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -486,8 +486,9 @@ class OidcTest extends TestCase
|
|||||||
$resp = $this->get('/');
|
$resp = $this->get('/');
|
||||||
$this->withHtml($resp)->assertElementExists('header form[action$="/oidc/logout"] button');
|
$this->withHtml($resp)->assertElementExists('header form[action$="/oidc/logout"] button');
|
||||||
}
|
}
|
||||||
public function test_logout_with_autodiscovery()
|
public function test_logout_with_autodiscovery_with_oidc_logout_enabled()
|
||||||
{
|
{
|
||||||
|
config()->set(['oidc.end_session_endpoint' => true]);
|
||||||
$this->withAutodiscovery();
|
$this->withAutodiscovery();
|
||||||
|
|
||||||
$transactions = $this->mockHttpClient([
|
$transactions = $this->mockHttpClient([
|
||||||
@ -499,9 +500,10 @@ class OidcTest extends TestCase
|
|||||||
$resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode(url('/')));
|
$resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode(url('/')));
|
||||||
|
|
||||||
$this->assertEquals(2, $transactions->requestCount());
|
$this->assertEquals(2, $transactions->requestCount());
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_logout_with_autodiscovery_but_oidc_logout_disabled()
|
public function test_logout_with_autodiscovery_with_oidc_logout_disabled()
|
||||||
{
|
{
|
||||||
$this->withAutodiscovery();
|
$this->withAutodiscovery();
|
||||||
config()->set(['oidc.end_session_endpoint' => false]);
|
config()->set(['oidc.end_session_endpoint' => false]);
|
||||||
@ -513,6 +515,7 @@ class OidcTest extends TestCase
|
|||||||
|
|
||||||
$resp = $this->asEditor()->post('/oidc/logout');
|
$resp = $this->asEditor()->post('/oidc/logout');
|
||||||
$resp->assertRedirect('/');
|
$resp->assertRedirect('/');
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_logout_without_autodiscovery_but_with_endpoint_configured()
|
public function test_logout_without_autodiscovery_but_with_endpoint_configured()
|
||||||
@ -521,6 +524,16 @@ class OidcTest extends TestCase
|
|||||||
|
|
||||||
$resp = $this->asEditor()->post('/oidc/logout');
|
$resp = $this->asEditor()->post('/oidc/logout');
|
||||||
$resp->assertRedirect('https://example.com/logout?post_logout_redirect_uri=' . urlencode(url('/')));
|
$resp->assertRedirect('https://example.com/logout?post_logout_redirect_uri=' . urlencode(url('/')));
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_logout_without_autodiscovery_with_configured_endpoint_adds_to_query_if_existing()
|
||||||
|
{
|
||||||
|
config()->set(['oidc.end_session_endpoint' => 'https://example.com/logout?a=b']);
|
||||||
|
|
||||||
|
$resp = $this->asEditor()->post('/oidc/logout');
|
||||||
|
$resp->assertRedirect('https://example.com/logout?a=b&post_logout_redirect_uri=' . urlencode(url('/')));
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_logout_with_autodiscovery_and_auto_initiate_returns_to_auto_prevented_login()
|
public function test_logout_with_autodiscovery_and_auto_initiate_returns_to_auto_prevented_login()
|
||||||
@ -530,6 +543,7 @@ class OidcTest extends TestCase
|
|||||||
'auth.auto_initiate' => true,
|
'auth.auto_initiate' => true,
|
||||||
'services.google.client_id' => false,
|
'services.google.client_id' => false,
|
||||||
'services.github.client_id' => false,
|
'services.github.client_id' => false,
|
||||||
|
'oidc.end_session_endpoint' => true,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$this->mockHttpClient([
|
$this->mockHttpClient([
|
||||||
@ -541,6 +555,39 @@ class OidcTest extends TestCase
|
|||||||
|
|
||||||
$redirectUrl = url('/login?prevent_auto_init=true');
|
$redirectUrl = url('/login?prevent_auto_init=true');
|
||||||
$resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode($redirectUrl));
|
$resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode($redirectUrl));
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_logout_endpoint_url_overrides_autodiscovery_endpoint()
|
||||||
|
{
|
||||||
|
config()->set(['oidc.end_session_endpoint' => 'https://a.example.com']);
|
||||||
|
$this->withAutodiscovery();
|
||||||
|
|
||||||
|
$transactions = $this->mockHttpClient([
|
||||||
|
$this->getAutoDiscoveryResponse(),
|
||||||
|
$this->getJwksResponse(),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$resp = $this->asEditor()->post('/oidc/logout');
|
||||||
|
$resp->assertRedirect('https://a.example.com?post_logout_redirect_uri=' . urlencode(url('/')));
|
||||||
|
|
||||||
|
$this->assertEquals(2, $transactions->requestCount());
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_logout_with_autodiscovery_does_not_use_rp_logout_if_no_url_via_autodiscovery()
|
||||||
|
{
|
||||||
|
config()->set(['oidc.end_session_endpoint' => true]);
|
||||||
|
$this->withAutodiscovery();
|
||||||
|
|
||||||
|
$this->mockHttpClient([
|
||||||
|
$this->getAutoDiscoveryResponse(['end_session_endpoint' => null]),
|
||||||
|
$this->getJwksResponse(),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$resp = $this->asEditor()->post('/oidc/logout');
|
||||||
|
$resp->assertRedirect('/');
|
||||||
|
$this->assertFalse(auth()->check());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_logout_redirect_contains_id_token_hint_if_existing()
|
public function test_logout_redirect_contains_id_token_hint_if_existing()
|
||||||
|
Loading…
Reference in New Issue
Block a user