]> BookStack Code Mirror - bookstack/commitdiff
Fixed OIDC JWT key parsing in microsoft environments
authorDan Brown <redacted>
Fri, 28 Jan 2022 14:00:55 +0000 (14:00 +0000)
committerDan Brown <redacted>
Fri, 28 Jan 2022 14:00:55 +0000 (14:00 +0000)
Made existence of 'alg' optional when JWK array set so we instead infer
it as RSA256 if not existing.

Fixes #3206

app/Auth/Access/Oidc/OidcJwtSigningKey.php
app/Auth/Access/Oidc/OidcProviderSettings.php
tests/Auth/OidcTest.php

index a70f3b3c74568ca8d05ae8d39d4d53246cb33b3a..012a6cbf9c10c72a7a27a26883f8a23173e58f72 100644 (file)
@@ -60,8 +60,11 @@ class OidcJwtSigningKey
      */
     protected function loadFromJwkArray(array $jwk)
     {
-        if ($jwk['alg'] !== 'RS256') {
-            throw new OidcInvalidKeyException("Only RS256 keys are currently supported. Found key using {$jwk['alg']}");
+        // 'alg' is optional for a JWK, but we will still attempt to validate if
+        // it exists otherwise presume it will be compatible.
+        $alg = $jwk['alg'] ?? null;
+        if ($jwk['kty'] !== 'RSA' || !(is_null($alg) || $alg === 'RS256')) {
+            throw new OidcInvalidKeyException("Only RS256 keys are currently supported. Found key using {$alg}");
         }
 
         if (empty($jwk['use'])) {
index 32946d058cf807c51153c3ce9f2108ccd2c708d9..016d006d2c0af68d8751dcc5ef97b933c84dfa8a 100644 (file)
@@ -164,7 +164,8 @@ class OidcProviderSettings
     protected function filterKeys(array $keys): array
     {
         return array_filter($keys, function (array $key) {
-            return $key['kty'] === 'RSA' && $key['use'] === 'sig' && $key['alg'] === 'RS256';
+            $alg = $key['alg'] ?? null;
+            return $key['kty'] === 'RSA' && $key['use'] === 'sig' && (is_null($alg) || $alg === 'RS256');
         });
     }
 
index 0b033ea812593bf462e09891624ada9b882696f3..9fa4d0012a41c9460b0cf35f5d9b9e89fa832b58 100644 (file)
@@ -318,6 +318,31 @@ class OidcTest extends TestCase
         $this->assertCount(4, $transactions);
     }
 
+    public function test_auth_login_with_autodiscovery_with_keys_that_do_not_have_alg_property()
+    {
+        $this->withAutodiscovery();
+
+        $keyArray = OidcJwtHelper::publicJwkKeyArray();
+        unset($keyArray['alg']);
+
+        $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());
+    }
+
     protected function withAutodiscovery()
     {
         config()->set([