Skip to content

Atrix 7.0.0.-alpha5#60

Open
ChrisHubinger wants to merge 5 commits into
masterfrom
v7
Open

Atrix 7.0.0.-alpha5#60
ChrisHubinger wants to merge 5 commits into
masterfrom
v7

Conversation

@ChrisHubinger

Copy link
Copy Markdown
Member

Features

  • Implement JWKS key fetching for JWT security strategy using jwks-rsa
  • Optional settings to validate issuer and audience with jwt security enabled

Breaking Changes

  • Upgrade to Hapi 21:
    Please refer to Hapi documentation to check for breaking changes that may affect you by using hapi internals.

@thomaspeklak thomaspeklak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation seems fine. Used the same library in other project. Only some minor issues.

Is there a place where the config is validated, have not seen it.

Comment on lines +253 to +257
const verifyOptions = {
audience: cfg.verifyOptions && cfg.verifyOptions.audience || undefined,
issuer: cfg.verifyOptions && cfg.verifyOptions.issuer || undefined,
algorithms: cfg.verifyOptions && cfg.verifyOptions.algorithms || [cfg.algorithm],
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verifyOptions are options from https://github.com/auth0/node-jsonwebtoken#jwtverifytoken-secretorpublickey-options-callback. You could also verify sub (subject) for instance.

const res = await svc.get('/data');
expect(res.statusCode).to.equal(401);
});
describe('Use JWKS to fetch keys', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only happy paths tests. e.g. no expired token, unsupported algorithm, validateUser returns false, ...

Comment thread README.md
rateLimit: true,

// max jwks requests
jwksRequestsPerMinute: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be a number, not boolean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants