Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion etc/firebase-admin.phone-number-verification.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function getPhoneNumberVerification(app?: App): PhoneNumberVerification;
// @public
export class PhoneNumberVerification {
get app(): App;
verifyToken(jwt: string): Promise<PhoneNumberVerificationToken>;
verifyToken(jwt: string, options?: VerifyPhoneNumberTokenOptions): Promise<PhoneNumberVerificationToken>;
}

// @public
Expand All @@ -46,6 +46,13 @@ export interface PhoneNumberVerificationToken {
nonce: string;
phoneNumber: string;
sub: string;
test_number?: boolean;
testNumber?: boolean;
}

// @public
export interface VerifyPhoneNumberTokenOptions {
acceptTestTokens?: boolean;
}

```
1 change: 1 addition & 0 deletions src/phone-number-verification/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export {

export {
PhoneNumberVerificationToken,
VerifyPhoneNumberTokenOptions,
} from './phone-number-verification-api'

/**
Expand Down
30 changes: 30 additions & 0 deletions src/phone-number-verification/phone-number-verification-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,42 @@ export interface PhoneNumberVerificationToken {
*/
phoneNumber: string;

/**
* Indicates whether the token was issued while the app was in test mode.
* Present and set to `true` for test-mode tokens.
* As a convenience, a boolean value is also copied over to the
* {@link PhoneNumberVerificationToken.testNumber | testNumber} property.
*/
test_number?: boolean;

/**
* Indicates whether the token was issued while the app was in test mode.
* This value is not actually one of the JWT token claims. It is added as a
* convenience, and is set to `true` when the {@link PhoneNumberVerificationToken.test_number}
* claim is `true`, or `false` otherwise.
*/
testNumber?: boolean;

/**
* Other arbitrary claims included in the token.
*/
[key: string]: any;
}

/**
* Interface representing options for the {@link PhoneNumberVerification.verifyToken} method.
*/
export interface VerifyPhoneNumberTokenOptions {
/**
* To accept test-mode tokens (tokens containing the claim `"test_number": true`),
* set this option to `true`. By default, {@link PhoneNumberVerification.verifyToken}
* rejects test-mode tokens.
*
* Enable this option only during development and testing.
*/
acceptTestTokens?: boolean;
}

export {
PhoneNumberVerificationErrorCode,
FirebasePhoneNumberVerificationError,
Expand Down
40 changes: 36 additions & 4 deletions src/phone-number-verification/phone-number-verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
*/

import { App } from '../app';
import { PhoneNumberVerificationToken } from './phone-number-verification-api';
import * as validator from '../utils/validator';
import {
FirebasePhoneNumberVerificationError,
PhoneNumberVerificationToken,
VerifyPhoneNumberTokenOptions,
} from './phone-number-verification-api';
import { FPNV_ERROR_CODE_MAPPING } from './error';
import { PhoneNumberTokenVerifier } from './token-verifier';
import { JWKS_URL, FPNV_TOKEN_INFO } from './phone-number-verification-api-client-internal';

Expand Down Expand Up @@ -53,13 +59,39 @@ export class PhoneNumberVerification {
}

/**
* Verifies a Firebase Phone Number Verification token.
* Verifies a Firebase Phone Number Verification token. By default, tokens issued in
* test mode (containing `"test_number": true`) are rejected unless
* `options.acceptTestTokens` is set to `true`.
*
* @param jwt - A string containing the Firebase Phone Number Verification JWT.
* @param options - Optional {@link VerifyPhoneNumberTokenOptions} object when verifying a
* Phone Number Verification token.
* @returns A promise that resolves with the decoded token.
*/
public verifyToken(jwt: string): Promise<PhoneNumberVerificationToken> {
return this.phoneNumberVerificationVerifier.verifyJWT(jwt);
public verifyToken(
jwt: string,
options?: VerifyPhoneNumberTokenOptions
): Promise<PhoneNumberVerificationToken> {
this.validateVerifyPhoneNumberTokenOptions(options);
return this.phoneNumberVerificationVerifier.verifyJWT(jwt, options);
}

private validateVerifyPhoneNumberTokenOptions(options?: VerifyPhoneNumberTokenOptions): void {
if (typeof options === 'undefined') {
return;
}
if (!validator.isNonNullObject(options)) {
throw new FirebasePhoneNumberVerificationError({
code: FPNV_ERROR_CODE_MAPPING.INVALID_ARGUMENT,
message: 'VerifyPhoneNumberTokenOptions must be a non-null object.',
});
}
if (typeof options.acceptTestTokens !== 'undefined' && !validator.isBoolean(options.acceptTestTokens)) {
throw new FirebasePhoneNumberVerificationError({
code: FPNV_ERROR_CODE_MAPPING.INVALID_ARGUMENT,
message: 'VerifyPhoneNumberTokenOptions.acceptTestTokens must be a boolean.',
});
}
}
Comment on lines +79 to 95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential runtime issues and ensure robust type safety, we should validate that acceptTestTokens is a boolean if it is provided in the options object. This prevents developers from accidentally passing non-boolean truthy values (like strings or numbers) which might otherwise bypass the test token rejection logic.

  private validateVerifyPhoneNumberTokenOptions(options?: VerifyPhoneNumberTokenOptions): void {
    if (typeof options === 'undefined') {
      return;
    }
    if (!validator.isNonNullObject(options)) {
      throw new FirebasePhoneNumberVerificationError({
        code: FPNV_ERROR_CODE_MAPPING.INVALID_ARGUMENT,
        message: 'VerifyPhoneNumberTokenOptions must be a non-null object.',
      });
    }
    if (typeof options.acceptTestTokens !== 'undefined' && !validator.isBoolean(options.acceptTestTokens)) {
      throw new FirebasePhoneNumberVerificationError({
        code: FPNV_ERROR_CODE_MAPPING.INVALID_ARGUMENT,
        message: 'acceptTestTokens option must be a boolean.',
      });
    }
  }

}

19 changes: 17 additions & 2 deletions src/phone-number-verification/token-verifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
*/

import { App } from '../app';
import { FirebasePhoneNumberVerificationError, PhoneNumberVerificationToken } from './phone-number-verification-api';
import {
FirebasePhoneNumberVerificationError,
PhoneNumberVerificationToken,
VerifyPhoneNumberTokenOptions,
} from './phone-number-verification-api';
import * as util from '../utils/index';
import * as validator from '../utils/validator';
import {
Expand Down Expand Up @@ -81,7 +85,10 @@ export class PhoneNumberTokenVerifier {
// Project ID is validated in the verification call.
}

public async verifyJWT(jwtToken: string): Promise<PhoneNumberVerificationToken> {
public async verifyJWT(
jwtToken: string,
options?: VerifyPhoneNumberTokenOptions
): Promise<PhoneNumberVerificationToken> {
if (!validator.isString(jwtToken)) {
throw new FirebasePhoneNumberVerificationError({
code: FPNV_ERROR_CODE_MAPPING.INVALID_TOKEN,
Expand All @@ -92,7 +99,15 @@ export class PhoneNumberTokenVerifier {
const projectId = await this.ensureProjectId();
const decoded = await this.decodeAndVerify(jwtToken, projectId);
const decodedIdToken = decoded.payload as PhoneNumberVerificationToken;
if (decodedIdToken.test_number === true && options?.acceptTestTokens !== true) {
throw new FirebasePhoneNumberVerificationError({
code: FPNV_ERROR_CODE_MAPPING.INVALID_TOKEN,
message: `${this.tokenInfo.jwtName} is a test token. To accept test tokens during ` +
`development, pass { acceptTestTokens: true } to ${this.tokenInfo.verifyApiName}.`,
});
}
decodedIdToken.phoneNumber = decodedIdToken.sub;
decodedIdToken.testNumber = Boolean(decodedIdToken.test_number);
return decodedIdToken;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,35 @@ describe('PhoneNumberVerification Service', () => {
await phoneNumberVerificationService.verifyToken(mockTokenString);

expect(verifyJwtStub.calledOnce).to.be.true;
expect(verifyJwtStub.calledWith(mockTokenString)).to.be.true;
expect(verifyJwtStub.calledWith(mockTokenString, undefined)).to.be.true;
});

it('should pass VerifyPhoneNumberTokenOptions to the internal verifier', async () => {
verifyJwtStub.resolves(mockDecodedToken);

await phoneNumberVerificationService.verifyToken(mockTokenString, { acceptTestTokens: true });

expect(verifyJwtStub.calledOnce).to.be.true;
expect(verifyJwtStub.calledWith(mockTokenString, { acceptTestTokens: true })).to.be.true;
});

it('should throw given invalid VerifyPhoneNumberTokenOptions', () => {
[null, 100, -100, 'abc', [], true].forEach((invalidOptions) => {
expect(() => {
return phoneNumberVerificationService.verifyToken(mockTokenString, invalidOptions as any);
}).to.throw('VerifyPhoneNumberTokenOptions must be a non-null object.');
});
});
Comment on lines +89 to 95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add test cases to verify that passing a non-boolean value for acceptTestTokens correctly throws an error, matching the new validation logic.

    it('should throw given invalid VerifyPhoneNumberTokenOptions', () => {
      [null, 100, -100, 'abc', [], true].forEach((invalidOptions) => {
        expect(() => {
          return phoneNumberVerificationService.verifyToken(mockTokenString, invalidOptions as any);
        }).to.throw('VerifyPhoneNumberTokenOptions must be a non-null object.');
      });

      [123, 'true', [], null].forEach((invalidAcceptTestTokens) => {
        expect(() => {
          return phoneNumberVerificationService.verifyToken(mockTokenString, { acceptTestTokens: invalidAcceptTestTokens as any });
        }).to.throw('acceptTestTokens option must be a boolean.');
      });
    }


it('should throw given non-boolean acceptTestTokens option', () => {
['false', 'true', 0, 1, null, {}, []].forEach((invalidAcceptTestTokens) => {
expect(() => {
return phoneNumberVerificationService.verifyToken(
mockTokenString,
{ acceptTestTokens: invalidAcceptTestTokens as any }
);
}).to.throw('VerifyPhoneNumberTokenOptions.acceptTestTokens must be a boolean.');
});
});

it('should return the decoded token object on success', async () => {
Expand Down
37 changes: 35 additions & 2 deletions test/unit/phone-number-verification/token-verifier.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ describe('PhoneNumberTokenVerifier', () => {
});

describe('Success', () => {
it('should return the token with phoneNumber property appended', async () => {
it('should return the token with phoneNumber and testNumber properties appended', async () => {
decodeJwtStub.resolves({ header: VALID_HEADER, payload: VALID_PAYLOAD });
signatureVerifierStub.verify.resolves();

Expand All @@ -316,9 +316,42 @@ describe('PhoneNumberTokenVerifier', () => {
expect(result.sub).to.equal(VALID_PAYLOAD.sub);
expect(result.aud).to.deep.equal(VALID_PAYLOAD.aud);

// Check the property
// Check the properties
expect(result).to.have.property('phoneNumber');
expect(result.phoneNumber).to.equal(VALID_PAYLOAD.sub);
expect(result.testNumber).to.be.false;
});
});

describe('Test Token Handling', () => {
beforeEach(() => {
decodeJwtStub.resolves({
header: VALID_HEADER,
payload: { ...VALID_PAYLOAD, test_number: true },
});
signatureVerifierStub.verify.resolves();
});

it('should reject a test token by default with INVALID_TOKEN', async () => {
await expect(verifier.verifyJWT('test-token'))
.to.be.rejectedWith(/is a test token/)
.and.eventually.have.property('code', `${MOCK_FPNV_PREFIX}/${FPNV_ERROR_CODE_MAPPING.INVALID_TOKEN}`);

await expect(verifier.verifyJWT('test-token', { acceptTestTokens: false }))
.to.be.rejectedWith(/is a test token/)
.and.eventually.have.property('code', `${MOCK_FPNV_PREFIX}/${FPNV_ERROR_CODE_MAPPING.INVALID_TOKEN}`);

await expect(verifier.verifyJWT('test-token', { acceptTestTokens: 'false' as any }))
.to.be.rejectedWith(/is a test token/)
.and.eventually.have.property('code', `${MOCK_FPNV_PREFIX}/${FPNV_ERROR_CODE_MAPPING.INVALID_TOKEN}`);
});

it('should accept a test token when acceptTestTokens is true and populate testNumber', async () => {
const result = await verifier.verifyJWT('test-token', { acceptTestTokens: true });

expect(result.test_number).to.be.true;
expect(result.testNumber).to.be.true;
expect(result.phoneNumber).to.equal(VALID_PAYLOAD.sub);
});
});

Expand Down
Loading