Merge pull request #4804 from BookStackApp/oidc_pkce

Add OIDC PKCE functionality
This commit is contained in:
Dan Brown 2024-01-27 18:11:19 +00:00 committed by GitHub
commit 415663a9bc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 51 additions and 20 deletions

View file

@ -83,15 +83,9 @@ class OidcOAuthProvider extends AbstractProvider
/**
* Checks a provider response for errors.
*
* @param ResponseInterface $response
* @param array|string $data Parsed response data
*
* @throws IdentityProviderException
*
* @return void
*/
protected function checkResponse(ResponseInterface $response, $data)
protected function checkResponse(ResponseInterface $response, $data): void
{
if ($response->getStatusCode() >= 400 || isset($data['error'])) {
throw new IdentityProviderException(
@ -105,13 +99,8 @@ class OidcOAuthProvider extends AbstractProvider
/**
* Generates a resource owner object from a successful resource owner
* details request.
*
* @param array $response
* @param AccessToken $token
*
* @return ResourceOwnerInterface
*/
protected function createResourceOwner(array $response, AccessToken $token)
protected function createResourceOwner(array $response, AccessToken $token): ResourceOwnerInterface
{
return new GenericResourceOwner($response, '');
}
@ -121,14 +110,18 @@ class OidcOAuthProvider extends AbstractProvider
*
* The grant that was used to fetch the response can be used to provide
* additional context.
*
* @param array $response
* @param AbstractGrant $grant
*
* @return OidcAccessToken
*/
protected function createAccessToken(array $response, AbstractGrant $grant)
protected function createAccessToken(array $response, AbstractGrant $grant): OidcAccessToken
{
return new OidcAccessToken($response);
}
/**
* Get the method used for PKCE code verifier hashing, which is passed
* in the "code_challenge_method" parameter in the authorization request.
*/
protected function getPkceMethod(): string
{
return static::PKCE_METHOD_S256;
}
}

View file

@ -33,6 +33,8 @@ class OidcService
/**
* Initiate an authorization flow.
* Provides back an authorize redirect URL, in addition to other
* details which may be required for the auth flow.
*
* @throws OidcException
*
@ -42,8 +44,12 @@ class OidcService
{
$settings = $this->getProviderSettings();
$provider = $this->getProvider($settings);
$url = $provider->getAuthorizationUrl();
session()->put('oidc_pkce_code', $provider->getPkceCode() ?? '');
return [
'url' => $provider->getAuthorizationUrl(),
'url' => $url,
'state' => $provider->getState(),
];
}
@ -63,6 +69,10 @@ class OidcService
$settings = $this->getProviderSettings();
$provider = $this->getProvider($settings);
// Set PKCE code flashed at login
$pkceCode = session()->pull('oidc_pkce_code', '');
$provider->setPkceCode($pkceCode);
// Try to exchange authorization code for access token
$accessToken = $provider->getAccessToken('authorization_code', [
'code' => $authorizationCode,

View file

@ -655,6 +655,34 @@ class OidcTest extends TestCase
]);
}
public function test_pkce_used_on_authorize_and_access()
{
// Start auth
$resp = $this->post('/oidc/login');
$state = session()->get('oidc_state');
$pkceCode = session()->get('oidc_pkce_code');
$this->assertGreaterThan(30, strlen($pkceCode));
$expectedCodeChallenge = trim(strtr(base64_encode(hash('sha256', $pkceCode, true)), '+/', '-_'), '=');
$redirect = $resp->headers->get('Location');
$redirectParams = [];
parse_str(parse_url($redirect, PHP_URL_QUERY), $redirectParams);
$this->assertEquals($expectedCodeChallenge, $redirectParams['code_challenge']);
$this->assertEquals('S256', $redirectParams['code_challenge_method']);
$transactions = $this->mockHttpClient([$this->getMockAuthorizationResponse([
'email' => 'benny@example.com',
'sub' => 'benny1010101',
])]);
$this->get('/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=' . $state);
$tokenRequest = $transactions->latestRequest();
$bodyParams = [];
parse_str($tokenRequest->getBody(), $bodyParams);
$this->assertEquals($pkceCode, $bodyParams['code_verifier']);
}
protected function withAutodiscovery()
{
config()->set([