-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add IAP and additional claims support #268
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM w/ question.
src/auth/jwtclient.ts
Outdated
// TODO: There is some duplication between the code in google/node-gtoken | ||
// and the code in JWTAccess. This particular code path needs to be | ||
// further rationalized. | ||
if (!this.access) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/auth/jwtaccess.ts
Outdated
}; | ||
const payload = Object.assign( | ||
{iss: this.email, sub: this.email, aud: authURI, exp, iat}, | ||
additionalClaims); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #268 +/- ##
==========================================
+ Coverage 93.91% 94.08% +0.16%
==========================================
Files 13 13
Lines 838 845 +7
Branches 182 186 +4
==========================================
+ Hits 787 795 +8
+ Misses 51 50 -1
Continue to review full report at Codecov.
|
src/auth/jwtaccess.ts
Outdated
secret: this.key | ||
}; | ||
// NOTE: Users can also override the values for iss / sub / aud / etc | ||
// with additionalClaims. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
FYI @matthewg |
@ofrobots ping :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM w/ nit.
src/auth/jwtaccess.ts
Outdated
// if additionalClaims are provided, ensure they do not collide with | ||
// other required claims. | ||
if (additionalClaims) { | ||
['iss', 'sub', 'aud', 'exp', 'iat'].forEach(claim => { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Adds support for additionalClaims to JWT, and an example of using it to work with the Identity Aware Proxy (IAP). Resolves #134 🤘