Skip to content

Commit

Permalink
chore: Excluding certain event_types from processing uid (#2370)
Browse files Browse the repository at this point in the history
* adding optional param to token verifier

* update _verifyAuthBlockingToken method

* fix lint

* adding public definitions

* add AuthBlockingEventType type

* use decoded payload instead

* remove debug statement

* formatting

* add unit tests

* unit test

* unit test

* add comment per code review

* Apply suggestions from code review

Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>

* fix unit test messages

* fix lint

---------

Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Lahiru Maramba <llahiru@gmail.com>
  • Loading branch information
3 people committed Apr 15, 2024
1 parent a833f4e commit 19c74dc
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 9 deletions.
20 changes: 13 additions & 7 deletions src/auth/token-verifier.ts
Expand Up @@ -529,13 +529,19 @@ export class FirebaseTokenVerifier {
errorMessage = `${this.tokenInfo.jwtName} has incorrect "iss" (issuer) claim. Expected ` +
`"${this.issuer}` + projectId + '" but got "' +
payload.iss + '".' + projectIdMatchMessage + verifyJwtTokenDocsMessage;
} else if (typeof payload.sub !== 'string') {
errorMessage = `${this.tokenInfo.jwtName} has no "sub" (subject) claim.` + verifyJwtTokenDocsMessage;
} else if (payload.sub === '') {
errorMessage = `${this.tokenInfo.jwtName} has an empty string "sub" (subject) claim.` + verifyJwtTokenDocsMessage;
} else if (payload.sub.length > 128) {
errorMessage = `${this.tokenInfo.jwtName} has "sub" (subject) claim longer than 128 characters.` +
verifyJwtTokenDocsMessage;
} else if (!(payload.event_type !== undefined &&
(payload.event_type === 'beforeSendSms' || payload.event_type === 'beforeSendEmail'))) {
// excluding `beforeSendSms` and `beforeSendEmail` from processing `sub` as there is no user record available.
// `sub` is the same as `uid` which is part of the user record.
if (typeof payload.sub !== 'string') {
errorMessage = `${this.tokenInfo.jwtName} has no "sub" (subject) claim.` + verifyJwtTokenDocsMessage;
} else if (payload.sub === '') {
errorMessage = `${this.tokenInfo.jwtName} has an empty "sub" (subject) claim.` +
verifyJwtTokenDocsMessage;
} else if (payload.sub.length > 128) {
errorMessage = `${this.tokenInfo.jwtName} has a "sub" (subject) claim longer than 128 characters.` +
verifyJwtTokenDocsMessage;
}
}
if (errorMessage) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_ARGUMENT, errorMessage);
Expand Down
41 changes: 39 additions & 2 deletions test/unit/auth/token-verifier.spec.ts
Expand Up @@ -341,7 +341,8 @@ describe('FirebaseTokenVerifier', () => {
});

return tokenVerifier.verifyJWT(mockIdToken)
.should.eventually.be.rejectedWith('Firebase ID token has "sub" (subject) claim longer than 128 characters');
.should.eventually.be.rejectedWith('Firebase ID token has a "sub" (subject) claim longer than 128 ' +
'characters');
});
});

Expand Down Expand Up @@ -659,7 +660,7 @@ describe('FirebaseTokenVerifier', () => {

return authBlockingTokenVerifier._verifyAuthBlockingToken(mockAuthBlockingToken, false, undefined)
.should.eventually.be.rejectedWith(
'Firebase Auth Blocking token has "sub" (subject) claim longer than 128 characters');
'Firebase Auth Blocking token has a "sub" (subject) claim longer than 128 characters');
});
});

Expand Down Expand Up @@ -780,5 +781,41 @@ describe('FirebaseTokenVerifier', () => {
await authBlockingTokenVerifier._verifyAuthBlockingToken(idTokenNoHeader, false, undefined)
.should.eventually.be.rejectedWith('Firebase Auth Blocking token has no "kid" claim.');
});

const eventTypesWithoutUid = ['beforeSendSms', 'beforeSendEmail'];
eventTypesWithoutUid.forEach((eventType) => {
it('should not throw error on invalid `sub` when event_type is "' + eventType + '"' , async () => {
const verifierStub = sinon.stub(PublicKeySignatureVerifier.prototype, 'verify')
.resolves();
stubs.push(verifierStub);

const mockAuthBlockingToken = mocks.generateAuthBlockingToken({
subject: ''
}, {
event_type: eventType,
});
return authBlockingTokenVerifier._verifyAuthBlockingToken(mockAuthBlockingToken, false, undefined)
.should.eventually.be.fulfilled;
});
});

const eventTypesWithUid = ['beforeCreate', 'beforeSignIn', undefined];
eventTypesWithUid.forEach((eventType) => {
it('should not throw error on invalid `sub` when event_type is "' + eventType + '"', async () => {
const verifierStub = sinon.stub(PublicKeySignatureVerifier.prototype, 'verify')
.resolves();
stubs.push(verifierStub);

const mockAuthBlockingToken = mocks.generateAuthBlockingToken({
subject: ''
}, {
event_type: eventType,
});
return authBlockingTokenVerifier._verifyAuthBlockingToken(mockAuthBlockingToken, false, undefined)
.should.eventually.be.rejectedWith('Firebase Auth Blocking token has an empty "sub" (subject) claim.' +
' See https://cloud.google.com/identity-platform/docs/blocking-functions for details on how to retrieve an' +
' Auth Blocking token.');
});
});
});
});

0 comments on commit 19c74dc

Please sign in to comment.