No support for IdPs using multiple JWK certificates
Impacted package
Which packages do you think might be impacted by the bug ?
- [ ] solid-client-authn-browser
- [ ] solid-client-authn-node
- [x] solid-client-authn-core
- [ ] oidc-client-ext
- [ ] Other (please specify): ...
Bug description
It is not uncommon for OIDC IdPs to use multiple JWK keys, resulting in the array published at jwks_uri to have more than one element. For example:
- Google has 3 keys (JWKS);
- Connect2ID has 9 keys (JWKS);
- Keycloak realms have 3 keys by default;
- Okta has 2 keys (JWKS);
- Microsoft has 7 keys (JWKS);
- Ping deployments can use up to 16 keys (!)
(While Google, MS and Okta are of low relevance here, Ping, Keycloak and Connect2ID could potentially become Solid-OIDC compliant in the near future as they either already ship the necessary features like DPoP and PKCE or will start shipping them soon.)
Currently, this is not supported by the library as it would just pick the first key (IRedirectHandler.ts:73):
// The JWKS should only contain the current key for the issuer.
let jwk: JWK;
try {
jwk = (await jwksResponse.json()).keys[0] as JWK;
} catch (e) {
throw new Error(
`Malformed JWKS for [${issuerIri}] at [${jwksIri}]: ${
(e as WithMessage).message
}`
);
}
return jwk;
The statement about JWKS having to contain only the current key seems erroneous to me; I couldn't find any specification that would mandate the only key. Actually, the kid of the key should be matched against the corresponding header claim of the access/ID token.
@justinwb @jamiefiedler
Expected result
Successful validation of acess/ID tokens minted by an IdP using multiple keys.
Actual result
Validation would only succeed if the token was signed by the first key from the published JWKS set.
Environment
System:
OS: Linux 5.10 Mageia 8
CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Memory: 1.80 GB / 15.48 GB
Container: Yes
Shell: 5.1.4 - /bin/bash
Binaries:
Node: 14.17.6 - /usr/bin/node
npm: 6.14.15 - /usr/bin/npm
Browsers:
Chrome: 95.0.4638.69
Firefox: 91.3.0esr
npmPackages:
@babel/core: ^7.12.16 => 7.12.16
@babel/plugin-proposal-class-properties: ^7.12.13 => 7.12.13
@babel/preset-react: ^7.12.13 => 7.12.13
@datapunt/matomo-tracker-react: ^0.3.1 => 0.3.1
@date-io/date-fns: ^1.3.13 => 1.3.13
@inrupt/eslint-config-base: ^0.0.4 => 0.0.4
@inrupt/eslint-config-react: ^0.0.4 => 0.0.4
@inrupt/prism-react-components: ^0.13.7 => 0.13.7
@inrupt/solid-client: ^1.15.0 => 1.15.0
@inrupt/solid-client-access-grants: ^0.3.3-fix2258-verification-endpoint-discovery-1412767200-263-1635864188.0 => 0.3.3-fix2258-verification-endpoint-discovery-1412767200-263-1635864188.0
@inrupt/solid-client-authn-browser: ^1.8.2 => 1.8.2
@inrupt/solid-ui-react: ^2.3.1 => 2.3.1
@material-ui/core: ^4.11.3 => 4.11.3
@material-ui/icons: ^4.11.2 => 4.11.2
@material-ui/lab: ^4.0.0-alpha.57 => 4.0.0-alpha.57
@material-ui/pickers: ^3.3.10 => 3.3.10
@sentry/node: ^6.1.0 => 6.1.0
@sentry/react: ^6.1.0 => 6.1.0
@sentry/webpack-plugin: ^1.14.0 => 1.14.0
@solid/lit-prism-patterns: ^0.13.7 => 0.13.7
@solid/lit-prism-theme-sdk-default: ^0.13.7 => 0.13.7
@testing-library/dom: ^7.29.4 => 7.29.4
@testing-library/jest-dom: ^5.11.9 => 5.11.9
@testing-library/react: ^11.2.5 => 11.2.5
@testing-library/react-hooks: ^5.1.2 => 5.1.2
@testing-library/user-event: ^12.7.1 => 12.7.1
@types/jest: ^26.0.20 => 26.0.20
@types/react: ^17.0.2 => 17.0.2
@types/react-table: ^7.0.28 => 7.0.28
@typescript-eslint/eslint-plugin: ^3.10.1 => 3.10.1
@typescript-eslint/parser: ^3.10.1 => 3.10.1
babel-eslint: ^10.1.0 => 10.1.0
babel-jest: ^26.6.3 => 26.6.3
date-fns: ^2.23.0 => 2.23.0
encoding: ^0.1.13 => 0.1.13
eslint: ^7.20.0 => 7.20.0
eslint-config-airbnb: ^18.2.0 => 18.2.0
eslint-config-airbnb-base: ^14.2.0 => 14.2.0
eslint-config-next: ^11.0.1 => 11.0.1
eslint-config-prettier: ^6.15.0 => 6.15.0
eslint-plugin-babel: ^5.3.1 => 5.3.1
eslint-plugin-import: ^2.22.1 => 2.22.1
eslint-plugin-jest: ^23.17.1 => 23.20.0
eslint-plugin-jsx-a11y: ^6.4.1 => 6.4.1
eslint-plugin-license-header: ^0.2.0 => 0.2.0
eslint-plugin-prettier: ^3.1.4 => 3.1.4
eslint-plugin-react: ^7.21.5 => 7.21.5
eslint-plugin-react-hooks: ^4.2.0 => 4.2.0
http-link-header: ^1.0.3 => 1.0.3
husky: ^4.3.7 => 4.3.7
jest: ^26.6.3 => 26.6.3
jest-localstorage-mock: ^2.4.3 => 2.4.3
jest-mock-extended: ^1.0.10 => 1.0.10
jest-raw-loader: ^1.0.1 => 1.0.1
jsdom: ^16.4.0 => 16.4.0
jsdom-global: 3.0.2 => 3.0.2
jss: ^10.5.1 => 10.5.1
jss-preset-default: ^10.5.1 => 10.5.1
license-checker: ^25.0.1 => 25.0.1
next: ^11.0.1 => 11.0.1
next-runtime-dotenv: ^1.4.0 => 1.4.0
nock: ^13.1.1 => 13.1.1
node-mocks-http: ^1.10.1 => 1.10.1
prettier: ^2.2.1 => 2.2.1
prop-types: ^15.7.2 => 15.7.2
raw-loader: ^4.0.2 => 4.0.2
rdf-namespaces: ^1.9.2 => 1.9.2
react: ^17.0.2 => 17.0.2
react-dom: ^17.0.2 => 17.0.2
react-id-generator: ^3.0.1 => 3.0.1
react-jss: ^10.4.0 => 10.4.0
react-table: ^7.6.3 => 7.6.3
react-test-renderer: ^17.0.1 => 17.0.1
react-transition-group: ^4.4.1 => 4.4.1
swr: ^0.4.2 => 0.4.2
typescript: ^4.4.3 => 4.4.3
uuid: ^8.3.1 => 8.3.1
vercel: ^21.2.3 => 21.2.3
whatwg-fetch: ^3.5.0 => 3.5.0
npmGlobalPackages:
npm: 6.14.15
Worth noting that it is not sufficient to match only against the kid value. The kty value also needs to match.
I have encountered OPs in the wild with multiple keys in a JWKS that use the same kid but which have different kty values, and the JWK specification (RFC 7517) specifically allows this.
Hi @dteleguin ,
Thanks for your suggestion. This has been added to our backlog and will be considered as an improvement to be added in in the future.
All the best, Nick.-