Implements all refresh token revocation APIs.#149
Conversation
This includes: admin.auth.UserRecord.prototype.tokensValidAfterTime:?string (readonly) admin.auth.Auth.prototype.verifyIdToken(idToken:string, checkRevoked:boolean=}:!Promise<!admin.auth.DecodedIdToken> admin.auth.Auth.prototype.revokeRefreshTokens(uid: string): Promise<void>
hiranya911
left a comment
hiranya911
left a comment
There was a problem hiding this comment.
Looks pretty solid. Just a a few minor nits, and then I can LGTM.
There was a problem hiding this comment.
Minor readability nit: Consider using 5000 as the offset here, as in the previous test case.
There was a problem hiding this comment.
Consider adding the line:
let invalidUid: any = {'localId': uid};
And then pass invalidUid to the method being tested.
| return this.tokenGenerator_.verifyIdToken(idToken); | ||
| return this.tokenGenerator_.verifyIdToken(idToken) | ||
| .then((decodedIdToken: DecodedIdToken) => { | ||
| // Whether to check if the token was revoked. |
There was a problem hiding this comment.
Do a check for !checkRevoked at the top, and remove the large if block:
if (!checkRevoked) {
return decodedIdToken;
}
return this.getUser(decodedIdToken.sub)
...
test/unit/auth/auth.spec.ts
Outdated
There was a problem hiding this comment.
Can we declare the return type to be DecodedIdToken in the signature?
| return auth.verifyIdToken(mockIdToken).then(() => { | ||
| return auth.verifyIdToken(mockIdToken).then((result) => { | ||
| expect(result).to.deep.equal(decodedIdToken); | ||
| expect(stub).to.have.been.calledOnce.and.calledWith(mockIdToken); |
There was a problem hiding this comment.
Perhaps also include a test to check that getUser() is never called.
There was a problem hiding this comment.
Good idea. Done.
src/auth/user-record.ts
Outdated
There was a problem hiding this comment.
It took a while for me to realize what's going on here. Can we expand this a bit?
let validAfterTime: string = null;
if (typeof response.validSince !== 'undefined') {
validAfterTime = parseDate(...);
}
utils.addReadOnlyGetter(this, 'tokensValidAfterTime', validAfterTime);
|
Thanks for the prompt review. I think I addressed all the comments. Let me know if I missed anything. |
2 participants
This includes:
admin.auth.UserRecord.prototype.tokensValidAfterTime:?string (readonly)
admin.auth.Auth.prototype.verifyIdToken(idToken:string, checkRevoked:boolean=):!Promise<!admin.auth.DecodedIdToken>
admin.auth.Auth.prototype.revokeRefreshTokens(uid: string): Promise
Unit and integration tests also included.