Skip to content

Commit

Permalink
fix: do not implicitly calculate key ids for Client instances
Browse files Browse the repository at this point in the history
closes #379
  • Loading branch information
panva committed Oct 13, 2021
1 parent 3ae206d commit 46e44e7
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 2 deletions.
19 changes: 18 additions & 1 deletion lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,17 @@ async function claimJWT(label, jwt) {
}

function getKeystore(jwks) {
if (!isPlainObject(jwks) || !Array.isArray(jwks.keys) || jwks.keys.some((k) => !isPlainObject(k) || !('kty' in k))) {
throw new TypeError('jwks must be a JSON Web Key Set formatted object');
}

// eslint-disable-next-line no-restricted-syntax
for (const jwk of jwks.keys) {
if (jwk.kid === undefined) {
jwk.kid = `DONOTUSE.${random()}`;
}
}

const keystore = jose.JWKS.asKeyStore(jwks);
if (keystore.all().some((key) => key.type !== 'private')) {
throw new TypeError('jwks must only contain private keys');
Expand Down Expand Up @@ -1426,6 +1437,12 @@ module.exports = (issuer, aadIssValidation = false) => class Client extends Base
if (jwks !== undefined && !(metadata.jwks || metadata.jwks_uri)) {
const keystore = getKeystore.call(this, jwks);
metadata.jwks = keystore.toJWKS(false);
// eslint-disable-next-line no-restricted-syntax
for (const jwk of metadata.jwks.keys) {
if (jwk.kid.startsWith('DONOTUSE.')) {
delete jwk.kid;
}
}
}

const response = await request.call(this, {
Expand Down Expand Up @@ -1525,7 +1542,7 @@ module.exports = (issuer, aadIssValidation = false) => class Client extends Base

signed = jose.JWS.sign(payload, key, {
...header,
kid: symmetric ? undefined : key.kid,
kid: symmetric || key.kid.startsWith('DONOTUSE.') ? undefined : key.kid,
});
}

Expand Down
2 changes: 1 addition & 1 deletion lib/helpers/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async function clientAssertion(endpoint, payload) {
if (!key) {
throw new TypeError(`no key found in client jwks to sign a client assertion with using alg ${alg}`);
}
return jose.JWS.sign(payload, key, { alg, typ: 'JWT', kid: key.kid });
return jose.JWS.sign(payload, key, { alg, typ: 'JWT', kid: key.kid.startsWith('DONOTUSE.') ? undefined : key.kid });
}

async function authFor(endpoint, { clientAssertionPayload } = {}) {
Expand Down
76 changes: 76 additions & 0 deletions test/client/implicit_kid.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
const { expect } = require('chai');
const jose = require('jose');
const nock = require('nock');

const Issuer = require('../../lib/issuer');
const clientInternal = require('../../lib/helpers/client');

async function noKidJWKS() {
const store = new jose.JWKS.KeyStore();
await store.generate('EC');
const jwks = store.toJWKS(true);
delete jwks.keys[0].kid;
expect(jwks.keys[0].kid).to.be.undefined;
return jwks;
}

describe('no implicit Key IDs (kid)', function () {
afterEach(nock.cleanAll);

it('is not added to client assertions', async () => {
const issuer = new Issuer();
const jwks = await noKidJWKS();
const client = new issuer.Client({
client_id: 'identifier',
token_endpoint_auth_method: 'private_key_jwt',
token_endpoint_auth_signing_alg: 'ES256',
}, jwks);

const { form: { client_assertion: jwt } } = await clientInternal.authFor.call(client, 'token');

const header = JSON.parse(Buffer.from(jwt.split('.')[0], 'base64'));
expect(header).to.have.property('alg', 'ES256');
expect(header).not.to.have.property('kid');
});

it('is not added to request objects', async () => {
const issuer = new Issuer();
const jwks = await noKidJWKS();
const client = new issuer.Client({
client_id: 'identifier',
request_object_signing_alg: 'ES256',
}, jwks);

const jwt = await client.requestObject();

const header = JSON.parse(Buffer.from(jwt.split('.')[0], 'base64'));
expect(header).to.have.property('alg', 'ES256');
expect(header).not.to.have.property('kid');
});

it('is not added to dynamic registration requests', async () => {
const issuer = new Issuer({
registration_endpoint: 'https://op.example.com/client/registration',
});

nock('https://op.example.com')
.filteringRequestBody(function (body) {
const payload = JSON.parse(body);
expect(payload).to.have.nested.property('jwks.keys').that.is.an('array');
expect(payload.jwks.keys[0]).to.be.an('object').with.property('kty');
expect(payload.jwks.keys[0]).not.to.have.property('kid');
})
.post('/client/registration', () => true) // to make sure filteringRequestBody works
.reply(201, {
client_id: 'identifier',
token_endpoint_auth_method: 'private_key_jwt',
jwks: {}, // ignore
});

await issuer.Client.register({
token_endpoint_auth_method: 'private_key_jwt',
}, { jwks: await noKidJWKS() });

expect(nock.isDone()).to.be.true;
});
});

0 comments on commit 46e44e7

Please sign in to comment.