Skip to content

Commit 5b837bf

Browse files
strobelpierrestrobelpierre
committed
fix(jwks): filter unsupported key types before returning from fixJwksAlg
When an OIDC provider's JWKS endpoint returns keys with key types that Firebase JWT cannot parse (e.g. EC P-521, certain OKP subtypes), JWK::parseKeySet() throws a DomainException instead of skipping them. fixJwksAlg() already identifies the matching key by type, but returns the entire JWKS including incompatible keys. This causes a crash in parseKeySet() on the first unsupported key it encounters. Filter the JWKS to only include keys matching the expected kty before returning, so parseKeySet() never sees unsupported key types. Fixes #823 See googleapis/php-jwt#561 Signed-off-by: Strobel Pierre <strobelpierre@gmail.com>
1 parent a302bdf commit 5b837bf

File tree

2 files changed

+113
-0
lines changed

2 files changed

+113
-0
lines changed

‎lib/Service/DiscoveryService.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,13 @@ public function fixJwksAlg(array $jwks, string $jwt): array {
247247
$jwks['keys'][$matchingIndex]['alg'] = $alg;
248248
}
249249

250+
// Filter out keys with incompatible key types to prevent
251+
// Firebase JWT from failing on unsupported curves (e.g. P-521, Ed448).
252+
// See https://github.com/firebase/php-jwt/issues/561
253+
$jwks['keys'] = array_values(array_filter($jwks['keys'], function ($key) use ($expectedKty) {
254+
return ($key['kty'] ?? null) === $expectedKty;
255+
}));
256+
250257
return $jwks;
251258
}
252259
}

‎tests/unit/Service/DiscoveryServiceTest.php‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,112 @@ public function setUp(): void {
4343
$this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->config, $this->cacheFactory);
4444
}
4545

46+
/**
47+
* Test that fixJwksAlg filters out keys with unsupported key types.
48+
* This prevents Firebase JWT from crashing on P-521 or OKP keys.
49+
* See https://github.com/firebase/php-jwt/issues/561
50+
*/
51+
public function testFixJwksAlgFiltersUnsupportedKeyTypes() {
52+
// Build a fake JWT with RS256 alg and a known kid
53+
$header = json_encode(['alg' => 'RS256', 'kid' => 'rsa-key-1', 'typ' => 'JWT']);
54+
$payload = json_encode(['sub' => '1234']);
55+
$fakeJwt = rtrim(strtr(base64_encode($header), '+/', '-_'), '=')
56+
. '.' . rtrim(strtr(base64_encode($payload), '+/', '-_'), '=')
57+
. '.fake-signature';
58+
59+
// JWKS with mixed key types: RSA (matching), EC P-521, and OKP
60+
$jwks = [
61+
'keys' => [
62+
[
63+
'kty' => 'EC',
64+
'crv' => 'P-521',
65+
'kid' => 'ec-p521-key',
66+
'use' => 'sig',
67+
'x' => 'AekpBQ8ST8a8VcfVOTNl353vSrDCLL-Jmn1TZOFz5EhU',
68+
'y' => 'ADSmRA43Z1DSNx_RvcLI87cdL07l6jQyyBXMoxVg_l2T',
69+
],
70+
[
71+
'kty' => 'RSA',
72+
'kid' => 'rsa-key-1',
73+
'use' => 'sig',
74+
'n' => str_repeat('A', 342), // Fake 2048-bit modulus (256 bytes base64)
75+
'e' => 'AQAB',
76+
],
77+
[
78+
'kty' => 'OKP',
79+
'crv' => 'Ed25519',
80+
'kid' => 'okp-key',
81+
'use' => 'sig',
82+
'x' => 'some-value',
83+
],
84+
],
85+
];
86+
87+
// Mock config to disable key strength validation (we use fake key material)
88+
$this->config->method('getSystemValue')
89+
->with('user_oidc', [])
90+
->willReturn(['validate_jwk_strength' => false]);
91+
92+
$result = $this->discoveryService->fixJwksAlg($jwks, $fakeJwt);
93+
94+
// Only the RSA key should remain
95+
Assert::assertCount(1, $result['keys']);
96+
Assert::assertEquals('RSA', $result['keys'][0]['kty']);
97+
Assert::assertEquals('rsa-key-1', $result['keys'][0]['kid']);
98+
Assert::assertEquals('RS256', $result['keys'][0]['alg']);
99+
}
100+
101+
/**
102+
* Test that fixJwksAlg works with EC keys when JWT uses ES256.
103+
*/
104+
public function testFixJwksAlgKeepsCompatibleEcKeys() {
105+
$header = json_encode(['alg' => 'ES256', 'kid' => 'ec-key-1', 'typ' => 'JWT']);
106+
$payload = json_encode(['sub' => '1234']);
107+
$fakeJwt = rtrim(strtr(base64_encode($header), '+/', '-_'), '=')
108+
. '.' . rtrim(strtr(base64_encode($payload), '+/', '-_'), '=')
109+
. '.fake-signature';
110+
111+
$jwks = [
112+
'keys' => [
113+
[
114+
'kty' => 'RSA',
115+
'kid' => 'rsa-key-1',
116+
'use' => 'sig',
117+
'n' => str_repeat('A', 342),
118+
'e' => 'AQAB',
119+
],
120+
[
121+
'kty' => 'EC',
122+
'crv' => 'P-256',
123+
'kid' => 'ec-key-1',
124+
'use' => 'sig',
125+
'x' => 'AekpBQ8ST8a8VcfVOTNl353vSrDCLL-Jmn1TZOFz5EhU',
126+
'y' => 'ADSmRA43Z1DSNx_RvcLI87cdL07l6jQyyBXMoxVg_l2T',
127+
],
128+
[
129+
'kty' => 'EC',
130+
'crv' => 'P-521',
131+
'kid' => 'ec-p521-key',
132+
'use' => 'sig',
133+
'x' => 'AekpBQ8ST8a8VcfVOTNl353vSrDCLL-Jmn1TZOFz5EhU',
134+
'y' => 'ADSmRA43Z1DSNx_RvcLI87cdL07l6jQyyBXMoxVg_l2T',
135+
],
136+
],
137+
];
138+
139+
$this->config->method('getSystemValue')
140+
->with('user_oidc', [])
141+
->willReturn(['validate_jwk_strength' => false]);
142+
143+
$result = $this->discoveryService->fixJwksAlg($jwks, $fakeJwt);
144+
145+
// Both EC keys should remain (same kty), RSA filtered out
146+
Assert::assertCount(2, $result['keys']);
147+
Assert::assertEquals('EC', $result['keys'][0]['kty']);
148+
Assert::assertEquals('ec-key-1', $result['keys'][0]['kid']);
149+
Assert::assertEquals('EC', $result['keys'][1]['kty']);
150+
}
151+
46152
public function testBuildAuthorizationUrl() {
47153
$xss1 = '\'"http-equiv=><svg/onload=alert(1)>';
48154
$cleanedXss1 = '&#039;&quot;http-equiv=&gt;&lt;svg/onload=alert(1)&gt;';

0 commit comments

Comments
 (0)