github-mirror/BookStack
2
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2024-11-25 09:42:10 +08:00
Code Issues Actions 24 Packages Projects Releases Wiki Activity

Fixed OIDC handling when no JWKS 'use' prop exists

Browse Source 
Now assume, based on OIDC discovery spec, that keys without 'use' are
'sig' keys. Should not affect existing use-cases since existance of such
keys would have throw exceptions in prev. versions of bookstack.

For #3869
This commit is contained in:
Dan Brown 2022-11-23 11:50:59 +00:00
parent 85b7b10c01
commit e20c944350
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
4 changed files with 45 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/Auth/Access/Oidc/OidcJwtSigningKey.php
View File

@ -67,11 +67,10 @@ class OidcJwtSigningKey
throw new OidcInvalidKeyException("Only RS256 keys are currently supported. Found key using {$alg}");
}
if (empty($jwk['use'])) {
throw new OidcInvalidKeyException('A "use" parameter on the provided key is expected');
}
if ($jwk['use'] !== 'sig') {
// 'use' is optional for a JWK but we assume 'sig' where no value exists since that's what
// the OIDC discovery spec infers since 'sig' MUST be set if encryption keys come into play.
$use = $jwk['use'] ?? 'sig';
if ($use !== 'sig') {
throw new OidcInvalidKeyException("Only signature keys are currently supported. Found key for use {$jwk['use']}");
}

42
app/Auth/Access/Oidc/OidcProviderSettings.php
View File

@ -15,40 +15,17 @@ use Psr\Http\Client\ClientInterface;
*/
class OidcProviderSettings
{
/**
* @var string
*/
public $issuer;
/**
* @var string
*/
public $clientId;
/**
* @var string
*/
public $clientSecret;
/**
* @var string
*/
public $redirectUri;
/**
* @var string
*/
public $authorizationEndpoint;
/**
* @var string
*/
public $tokenEndpoint;
public string $issuer;
public string $clientId;
public string $clientSecret;
public ?string $redirectUri;
public ?string $authorizationEndpoint;
public ?string $tokenEndpoint;
/**
* @var string[]|array[]
*/
public $keys = [];
public ?array $keys = [];
public function __construct(array $settings)
{
@ -164,9 +141,10 @@ class OidcProviderSettings
protected function filterKeys(array $keys): array
{
return array_filter($keys, function (array $key) {
$alg = $key['alg'] ?? null;
$alg = $key['alg'] ?? 'RS256';
$use = $key['use'] ?? 'sig';
return $key['kty'] === 'RSA' && $key['use'] === 'sig' && (is_null($alg) || $alg === 'RS256');
return $key['kty'] === 'RSA' && $use === 'sig' && $alg === 'RS256';
});
}

1
app/Auth/Access/Oidc/OidcService.php
View File

@ -52,7 +52,6 @@ class OidcService
{
$settings = $this->getProviderSettings();
$provider = $this->getProvider($settings);
return [
'url' => $provider->getAuthorizationUrl(),
'state' => $provider->getState(),

31
tests/Auth/OidcTest.php
View File

@ -360,6 +360,37 @@ class OidcTest extends TestCase
$this->assertTrue(auth()->check());
}
public function test_auth_login_with_autodiscovery_with_keys_that_do_not_have_use_property()
{
// Based on reading the OIDC discovery spec:
// > This contains the signing key(s) the RP uses to validate signatures from the OP. The JWK Set MAY also
// > contain the Server's encryption key(s), which are used by RPs to encrypt requests to the Server. When
// > both signing and encryption keys are made available, a use (Key Use) parameter value is REQUIRED for all
// > keys in the referenced JWK Set to indicate each key's intended usage.
// We can assume that keys without use are intended for signing.
$this->withAutodiscovery();
$keyArray = OidcJwtHelper::publicJwkKeyArray();
unset($keyArray['use']);
$this->mockHttpClient([
$this->getAutoDiscoveryResponse(),
new Response(200, [
'Content-Type' => 'application/json',
'Cache-Control' => 'no-cache, no-store',
'Pragma' => 'no-cache',
], json_encode([
'keys' => [
$keyArray,
],
])),
]);
$this->assertFalse(auth()->check());
$this->runLogin();
$this->assertTrue(auth()->check());
}
public function test_login_group_sync()
{
config()->set([