Skip to content

Commit abc10bf

Browse files
committed
fix(fpnv): validate JWT audience using project ID and improve safety checks
1 parent 59aafdc commit abc10bf

2 files changed

Lines changed: 28 additions & 9 deletions

File tree

src/fpnv/token-verifier.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export class FirebasePhoneNumberTokenVerifier {
7676

7777
this.signatureVerifier = PublicKeySignatureVerifier.withJwksUrl(jwksUrl, app.options.httpAgent);
7878

79-
// For backward compatibility, the project ID is validated in the verification call.
79+
// Project ID is validated in the verification call.
8080
}
8181

8282
public async verifyJWT(jwtToken: string): Promise<FpnvToken> {
@@ -87,10 +87,13 @@ export class FirebasePhoneNumberTokenVerifier {
8787
);
8888
}
8989

90-
await this.ensureProjectId();
91-
const decoded = await this.decodeAndVerify(jwtToken);
90+
const projectId = await this.ensureProjectId();
91+
const decoded = await this.decodeAndVerify(jwtToken, projectId);
9292
const decodedIdToken = decoded.payload as FpnvToken;
93-
decodedIdToken.getPhoneNumber = () => decodedIdToken.sub;
93+
Object.defineProperty(decodedIdToken, 'getPhoneNumber', {
94+
value: () => decodedIdToken.sub,
95+
enumerable: false,
96+
});
9497
return decodedIdToken;
9598
}
9699

@@ -107,9 +110,10 @@ export class FirebasePhoneNumberTokenVerifier {
107110

108111
private async decodeAndVerify(
109112
token: string,
113+
projectId: string
110114
): Promise<DecodedToken> {
111115
const decodedToken = await this.safeDecode(token);
112-
this.verifyContent(decodedToken);
116+
this.verifyContent(decodedToken, projectId);
113117
await this.verifySignature(token);
114118
return decodedToken;
115119
}
@@ -133,6 +137,7 @@ export class FirebasePhoneNumberTokenVerifier {
133137

134138
private verifyContent(
135139
fullDecodedToken: DecodedToken,
140+
projectId: string
136141
): void {
137142
const header = fullDecodedToken && fullDecodedToken.header;
138143
const payload = fullDecodedToken && fullDecodedToken.payload;
@@ -141,11 +146,12 @@ export class FirebasePhoneNumberTokenVerifier {
141146
'Firebase project as the service account used to authenticate this SDK.';
142147
const verifyJwtTokenDocsMessage = ` See ${this.tokenInfo.url} ` +
143148
`for details on how to retrieve ${this.shortNameArticle} ${this.tokenInfo.shortName}.`;
149+
const scopedProjectId = `${this.issuer}${projectId}`;
144150

145151
let errorMessage: string | undefined;
146152

147153
// JWT Header
148-
if (typeof header.kid === 'undefined') {
154+
if (!header || typeof header.kid === 'undefined') {
149155
errorMessage = `${this.tokenInfo.jwtName} has no "kid" claim.`;
150156
errorMessage += verifyJwtTokenDocsMessage;
151157
} else if (header.alg !== ALGORITHM_ES256) {
@@ -156,13 +162,15 @@ export class FirebasePhoneNumberTokenVerifier {
156162
`"${header.typ}". ${verifyJwtTokenDocsMessage}`;
157163
}
158164
// FPNV Token
159-
else if (!validator.isNonEmptyString(payload.iss)) {
165+
else if (!payload) {
166+
errorMessage = `${this.tokenInfo.jwtName} has no payload. ${verifyJwtTokenDocsMessage}`;
167+
} else if (typeof payload.iss !== 'string' || !payload.iss.startsWith(this.issuer)) {
160168
errorMessage = `${this.tokenInfo.jwtName} has incorrect "iss" (issuer) claim. Expected ` +
161169
`an issuer starting with "${this.issuer}" but got "${payload.iss}".`
162170
+ ` ${projectIdMatchMessage} ${verifyJwtTokenDocsMessage}`;
163-
} else if (!validator.isNonEmptyArray(payload.aud) || !payload.aud.includes(payload.iss)) {
171+
} else if (!validator.isNonEmptyArray(payload.aud) || !payload.aud.includes(scopedProjectId)) {
164172
errorMessage = `${this.tokenInfo.jwtName} has incorrect "aud" (audience) claim. Expected ` +
165-
`"${payload.iss}" to be one of "${payload.aud}". ${projectIdMatchMessage} ${verifyJwtTokenDocsMessage}`;
173+
`"${scopedProjectId}" to be one of "${payload.aud}". ${projectIdMatchMessage} ${verifyJwtTokenDocsMessage}`;
166174
} else if (typeof payload.sub !== 'string') {
167175
errorMessage = `${this.tokenInfo.jwtName} has no "sub" (subject) claim. ${verifyJwtTokenDocsMessage}`;
168176
} else if (payload.sub === '') {

test/unit/fpnv/token-verifier.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,17 @@ describe('FirebasePhoneNumberTokenVerifier', () => {
236236
setupDecode({}, { sub: '' });
237237
await expect(verifier.verifyJWT('token')).to.be.rejectedWith('empty "sub"');
238238
});
239+
240+
it('should throw if payload is missing', async () => {
241+
decodeJwtStub.resolves({ header: VALID_HEADER, payload: undefined });
242+
await expect(verifier.verifyJWT('token')).to.be.rejectedWith('has no payload');
243+
});
244+
245+
it('should throw if "aud" does not match the current app projectId', async () => {
246+
findProjectIdStub.resolves('some-other-project-id');
247+
setupDecode(); // uses VALID_PAYLOAD which has 'fpnv-team-test'
248+
await expect(verifier.verifyJWT('token')).to.be.rejectedWith('has incorrect "aud"');
249+
});
239250
});
240251

241252
describe('Signature Verification', () => {

0 commit comments

Comments
 (0)