Skip to content

Commit

Permalink
Allow JWT to not contain a "kid" value (#55)
Browse files Browse the repository at this point in the history
* Allow JWT to not contain a "kid" value when the JWKS endpoint only returns a single key

* Remove `kid` value from test

* Add test for no `kid` with a single key returned from jwks endpoint in passports.tests.js
  • Loading branch information
dejan9393 authored Apr 8, 2020
1 parent 398c05e commit a3ba52e
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 12 deletions.
13 changes: 8 additions & 5 deletions src/JwksClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,9 @@ export class JwksClient {
if(key.kty !== 'RSA') {
return false;
}
if(!key.kid) {
return false;
}
if(key.hasOwnProperty('use') && key.use !== 'sig') {
return false;
}
}
return ((key.x5c && key.x5c.length) || (key.n && key.e));
})
.map(key => {
Expand Down Expand Up @@ -111,7 +108,13 @@ export class JwksClient {
return cb(err);
}

const key = keys.find(k => k.kid === kid);
const kidDefined = kid !== undefined && kid !== null;
if (!kidDefined && keys.length > 1) {
this.logger('No KID specified and JWKS endpoint returned more than 1 key');
return cb(new SigningKeyNotFoundError('No KID specified and JWKS endpoint returned more than 1 key'));
}

const key = keys.find(k => !kidDefined || k.kid === kid);
if (key) {
return cb(null, key);
} else {
Expand Down
26 changes: 24 additions & 2 deletions tests/express.es5.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ describe('expressJwtSecret', () => {
});
});

it('should not provide a key if token is RS256 and no KID was provided', (done) => {
it('should not provide a key if JWKS endpoint returned multiple keys and no KID was provided', (done) => {
const middleware = expressJwt({
secret: jwksRsa.expressJwtSecret({
jwksUri: 'http://localhost/.well-known/jwks.json'
})
});

jwksEndpoint('http://localhost', [ { pub: publicKey, kid: '123' } ]);
jwksEndpoint('http://localhost', [ { pub: publicKey, kid: '123' }, { pub: publicKey, kid: '456' } ]);

const token = createToken(privateKey, null, { sub: 'john' });
middleware({ headers: { authorization: `Bearer ${token}` } }, { }, function(err) {
Expand Down Expand Up @@ -152,5 +152,27 @@ describe('expressJwtSecret', () => {
});
});

it('should work if the JWKS endpoint returns a single key and no KID is provided', (done) => {
const middleware = expressJwt({
secret: jwksRsa.expressJwtSecret({
jwksUri: 'http://localhost/.well-known/jwks.json',
handleSigningKeyError: (err, cb) => {
if (err instanceof jwksRsa.SigningKeyNotFoundError) {
cb(new Error('This is bad'));
}
}
})
});

jwksEndpoint('http://localhost', [ { pub: publicKey } ]);

const token = createToken(privateKey, undefined, { sub: 'john' });
const req = { headers: { authorization: `Bearer ${token}` } };
middleware(req, { }, function(err) {
expect(err).to.be.undefined;
expect(req.user.sub).to.equal('john');
done();
});
});
});
});
32 changes: 29 additions & 3 deletions tests/koa.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('koaJwtSecret', () => {
});
});

it('should not provide a key if token is RS256 and no KID was provided', (done) => {
it('should not provide a key if JWKS endpoint returned multiple keys and no KID was provided', (done) => {
const app = new Koa();
app.use(koaJwt({
debug: true,
Expand All @@ -83,14 +83,14 @@ describe('koaJwtSecret', () => {
}));

const token = createToken(privateKey, null, { sub: 'john' });
jwksEndpoint('http://localhost', [ { pub: publicKey, kid: '123' } ]);
jwksEndpoint('http://localhost', [ { pub: publicKey, kid: '123' }, { pub: publicKey, kid: '456' } ]);

request(app.listen())
.get('/')
.set('Authorization', `Bearer ${ token }`)
.expect(401)
.end((err, res) => {
expect(res.text).to.equal('Unable to find a signing key that matches \'null\'');
expect(res.text).to.equal('No KID specified and JWKS endpoint returned more than 1 key');
done();
});
});
Expand Down Expand Up @@ -196,4 +196,30 @@ describe('koaJwtSecret', () => {
});
});

it('should work if the JWKS endpoint returns a single key and no KID is provided', (done) => {
const app = new Koa();
app.use(koaJwt({
debug: true,
secret: jwksRsa.koaJwtSecret({
jwksUri: 'http://localhost/.well-known/jwks.json'
})
}));
app.use((ctx) => {
ctx.body = ctx.state.user;
ctx.status = 200;
});

const token = createToken(privateKey, undefined, { sub: 'john' });
jwksEndpoint('http://localhost', [ { pub: publicKey } ]);

request(app.listen())
.get('/')
.set('Authorization', `Bearer ${ token }`)
.expect(200)
.end((err, res) => {
expect(res.body.sub).to.equal('john');
done();
});
});

});
40 changes: 38 additions & 2 deletions tests/passport.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('passportJwtSecret', () => {
});
});

it('should not provide a key if token is RS256 and no KID was provided', done => {
it('should not provide a key if JWKS endpoint returned multiple keys and no KID was provided', done => {
const app = new Express();
passport.use(
new JwtStrategy(
Expand Down Expand Up @@ -144,7 +144,7 @@ describe('passportJwtSecret', () => {
);

const token = createToken(privateKey, null, { sub: 'john' });
jwksEndpoint('http://localhost', [ { pub: publicKey, kid: '123' } ]);
jwksEndpoint('http://localhost', [ { pub: publicKey, kid: '123' }, { pub: publicKey, kid: '456' } ]);

request(app.listen())
.get('/')
Expand Down Expand Up @@ -323,4 +323,40 @@ describe('passportJwtSecret', () => {
done();
});
});

it('should work if the JWKS endpoint returns a single key and no KID is provided', done => {
const app = new Express();
passport.use(
new JwtStrategy(
{
secretOrKeyProvider: jwksRsa.passportJwtSecret({
jwksUri: 'http://localhost/.well-known/jwks.json'
}),
jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(),
algorithms: [ 'RS256' ]
},
(jwt_payload, done) => done(null, jwt_payload)
)
);

app.get(
'/',
passport.authenticate('jwt', { session: false }),
(req, res) => {
res.json(req.user);
}
);

const token = createToken(privateKey, null, { sub: 'john' });
jwksEndpoint('http://localhost', [ { pub: publicKey } ]);

request(app.listen())
.get('/')
.set('Authorization', `Bearer ${token}`)
.expect(200)
.end((err, res) => {
expect(res.body.sub).to.equal('john');
done();
});
});
});

0 comments on commit a3ba52e

Please sign in to comment.