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>
| }; | ||
| const requestData = { | ||
| localId: uid, | ||
| validSince: Math.ceil((now.getTime() + 2000) / 1000), |
There was a problem hiding this comment.
Minor readability nit: Consider using 5000 as the offset here, as in the previous test case.
| const expectedError = new FirebaseAuthError(AuthClientErrorCode.INVALID_UID); | ||
|
|
||
| const requestHandler = new FirebaseAuthRequestHandler(mockApp); | ||
| return requestHandler.revokeRefreshTokens({'localId': uid} as any) |
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
| * @param {Date} authTime The authentication time of the ID token. | ||
| * @return {DecodedIdToken} The generated decoded ID token. | ||
| */ | ||
| function getDecodedIdToken(uid: string, authTime: Date) { |
There was a problem hiding this comment.
Can we declare the return type to be DecodedIdToken in the signature?
test/unit/auth/auth.spec.ts
Outdated
| @@ -229,7 +268,8 @@ describe('Auth', () => { | |||
| }); | |||
|
|
|||
| it('should forward on the call to the token generator\'s verifyIdToken() method', () => { | |||
| 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
| @@ -180,6 +181,9 @@ export class UserRecord { | |||
| // Ignore error. | |||
| utils.addReadonlyGetter(this, 'customClaims', undefined); | |||
| } | |||
| // Convert validSince first to UTC milliseconds and then to UTC date string. | |||
| utils.addReadonlyGetter( | |||
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. |
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.