Skip to content

Commit

Permalink
Make ApplicationVerifier params optional in Phone Auth APIs (#8366)
Browse files Browse the repository at this point in the history
* Make ApplicationVerifier params optional in Phone APIs

* Add more unit tests for when ApplicationVerifier is not available
  • Loading branch information
NhienLam authored Jul 31, 2024
1 parent 56f904c commit 1dee30b
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 21 deletions.
8 changes: 4 additions & 4 deletions common/api-review/auth.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ export function isSignInWithEmailLink(auth: Auth, emailLink: string): boolean;
export function linkWithCredential(user: User, credential: AuthCredential): Promise<UserCredential>;

// @public
export function linkWithPhoneNumber(user: User, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
export function linkWithPhoneNumber(user: User, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;

// @public
export function linkWithPopup(user: User, provider: AuthProvider, resolver?: PopupRedirectResolver): Promise<UserCredential>;
Expand Down Expand Up @@ -625,7 +625,7 @@ export class PhoneAuthProvider {
static readonly PHONE_SIGN_IN_METHOD: 'phone';
static readonly PROVIDER_ID: 'phone';
readonly providerId: "phone";
verifyPhoneNumber(phoneOptions: PhoneInfoOptions | string, applicationVerifier: ApplicationVerifier): Promise<string>;
verifyPhoneNumber(phoneOptions: PhoneInfoOptions | string, applicationVerifier?: ApplicationVerifier): Promise<string>;
}

// @public
Expand Down Expand Up @@ -692,7 +692,7 @@ export interface ReactNativeAsyncStorage {
export function reauthenticateWithCredential(user: User, credential: AuthCredential): Promise<UserCredential>;

// @public
export function reauthenticateWithPhoneNumber(user: User, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
export function reauthenticateWithPhoneNumber(user: User, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;

// @public
export function reauthenticateWithPopup(user: User, provider: AuthProvider, resolver?: PopupRedirectResolver): Promise<UserCredential>;
Expand Down Expand Up @@ -778,7 +778,7 @@ export function signInWithEmailAndPassword(auth: Auth, email: string, password:
export function signInWithEmailLink(auth: Auth, email: string, emailLink?: string): Promise<UserCredential>;

// @public
export function signInWithPhoneNumber(auth: Auth, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
export function signInWithPhoneNumber(auth: Auth, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;

// @public
export function signInWithPopup(auth: Auth, provider: AuthProvider, resolver?: PopupRedirectResolver): Promise<UserCredential>;
Expand Down
6 changes: 3 additions & 3 deletions docs-devsite/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ This method does not work in a Node.js environment or with [Auth](./auth.auth.md
<b>Signature:</b>

```typescript
export declare function signInWithPhoneNumber(auth: Auth, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
export declare function signInWithPhoneNumber(auth: Auth, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;
```

#### Parameters
Expand Down Expand Up @@ -1304,7 +1304,7 @@ This method does not work in a Node.js environment.
<b>Signature:</b>

```typescript
export declare function linkWithPhoneNumber(user: User, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
export declare function linkWithPhoneNumber(user: User, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;
```

#### Parameters
Expand Down Expand Up @@ -1457,7 +1457,7 @@ This method does not work in a Node.js environment or on any [User](./auth.user.
<b>Signature:</b>

```typescript
export declare function reauthenticateWithPhoneNumber(user: User, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
export declare function reauthenticateWithPhoneNumber(user: User, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;
```

#### Parameters
Expand Down
2 changes: 1 addition & 1 deletion docs-devsite/auth.phoneauthprovider.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ Starts a phone number authentication flow by sending a verification code to the
<b>Signature:</b>

```typescript
verifyPhoneNumber(phoneOptions: PhoneInfoOptions | string, applicationVerifier: ApplicationVerifier): Promise<string>;
verifyPhoneNumber(phoneOptions: PhoneInfoOptions | string, applicationVerifier?: ApplicationVerifier): Promise<string>;
```

#### Parameters
Expand Down
2 changes: 1 addition & 1 deletion packages/auth/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const enum RecaptchaActionName {
SIGN_UP_PASSWORD = 'signUpPassword',
SEND_VERIFICATION_CODE = 'sendVerificationCode',
MFA_SMS_ENROLLMENT = 'mfaSmsEnrollment',
MFA_SMS_SIGNIN = 'mfaSmsSignin'
MFA_SMS_SIGNIN = 'mfaSmsSignIn'
}

export const enum EnforcementState {
Expand Down
84 changes: 83 additions & 1 deletion packages/auth/src/platform_browser/providers/phone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
* limitations under the License.
*/

import { expect } from 'chai';
import { expect, use } from 'chai';
import chaiAsPromised from 'chai-as-promised';
import * as sinon from 'sinon';

import { FirebaseError } from '@firebase/util';

import {
mockEndpoint,
mockEndpointWithParams
Expand All @@ -37,6 +40,8 @@ import { FAKE_TOKEN } from '../recaptcha/recaptcha_enterprise_verifier';
import { MockGreCAPTCHATopLevel } from '../recaptcha/recaptcha_mock';
import { ApplicationVerifierInternal } from '../../model/application_verifier';

use(chaiAsPromised);

describe('platform_browser/providers/phone', () => {
let auth: TestAuth;
let v2Verifier: ApplicationVerifierInternal;
Expand Down Expand Up @@ -104,6 +109,83 @@ describe('platform_browser/providers/phone', () => {
});
});

it('throws an error if verify without appVerifier when recaptcha enterprise is disabled', async () => {
const recaptchaConfigResponseOff = {
recaptchaKey: 'foo/bar/to/site-key',
recaptchaEnforcementState: [
{
provider: RecaptchaAuthProvider.PHONE_PROVIDER,
enforcementState: EnforcementState.OFF
}
]
};
const recaptcha = new MockGreCAPTCHATopLevel();
if (typeof window === 'undefined') {
return;
}
window.grecaptcha = recaptcha;
sinon
.stub(recaptcha.enterprise, 'execute')
.returns(Promise.resolve('enterprise-token'));

mockEndpointWithParams(
Endpoint.GET_RECAPTCHA_CONFIG,
{
clientType: RecaptchaClientType.WEB,
version: RecaptchaVersion.ENTERPRISE
},
recaptchaConfigResponseOff
);

const provider = new PhoneAuthProvider(auth);
await expect(
provider.verifyPhoneNumber('+15105550000')
).to.be.rejectedWith(FirebaseError, 'auth/argument-error');
});

it('calls the server without appVerifier when recaptcha enterprise is enabled', async () => {
const recaptchaConfigResponseEnforce = {
recaptchaKey: 'foo/bar/to/site-key',
recaptchaEnforcementState: [
{
provider: RecaptchaAuthProvider.PHONE_PROVIDER,
enforcementState: EnforcementState.ENFORCE
}
]
};
const recaptcha = new MockGreCAPTCHATopLevel();
if (typeof window === 'undefined') {
return;
}
window.grecaptcha = recaptcha;
sinon
.stub(recaptcha.enterprise, 'execute')
.returns(Promise.resolve('enterprise-token'));

mockEndpointWithParams(
Endpoint.GET_RECAPTCHA_CONFIG,
{
clientType: RecaptchaClientType.WEB,
version: RecaptchaVersion.ENTERPRISE
},
recaptchaConfigResponseEnforce
);

const route = mockEndpoint(Endpoint.SEND_VERIFICATION_CODE, {
sessionInfo: 'verification-id'
});

const provider = new PhoneAuthProvider(auth);
const result = await provider.verifyPhoneNumber('+15105550000');
expect(result).to.eq('verification-id');
expect(route.calls[0].request).to.eql({
phoneNumber: '+15105550000',
captchaResponse: 'enterprise-token',
clientType: RecaptchaClientType.WEB,
recaptchaVersion: RecaptchaVersion.ENTERPRISE
});
});

it('calls the server when recaptcha enterprise is enabled', async () => {
const recaptchaConfigResponseEnforce = {
recaptchaKey: 'foo/bar/to/site-key',
Expand Down
2 changes: 1 addition & 1 deletion packages/auth/src/platform_browser/providers/phone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class PhoneAuthProvider {
*/
verifyPhoneNumber(
phoneOptions: PhoneInfoOptions | string,
applicationVerifier: ApplicationVerifier
applicationVerifier?: ApplicationVerifier
): Promise<string> {
return _verifyPhoneNumber(
this.auth,
Expand Down
53 changes: 53 additions & 0 deletions packages/auth/src/platform_browser/strategies/phone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,32 @@ describe('platform_browser/strategies/phone', () => {
});
});

it('calls verify phone number without a v2 RecaptchaVerifier when recaptcha enterprise is enabled', async () => {
if (typeof window === 'undefined') {
return;
}
mockRecaptchaEnterpriseEnablement(EnforcementState.ENFORCE);
await signInWithPhoneNumber(auth, '+15105550000');

expect(sendCodeEndpoint.calls[0].request).to.eql({
phoneNumber: '+15105550000',
captchaResponse: RECAPTCHA_ENTERPRISE_TOKEN,
clientType: RecaptchaClientType.WEB,
recaptchaVersion: RecaptchaVersion.ENTERPRISE
});
});

it('throws an error if verify phone number without a v2 RecaptchaVerifier when recaptcha enterprise is disabled', async () => {
if (typeof window === 'undefined') {
return;
}
mockRecaptchaEnterpriseEnablement(EnforcementState.OFF);

await expect(
signInWithPhoneNumber(auth, '+15105550000')
).to.be.rejectedWith(FirebaseError, 'auth/argument-error');
});

context('ConfirmationResult', () => {
it('result contains verification id baked in', async () => {
if (typeof window === 'undefined') {
Expand Down Expand Up @@ -504,6 +530,33 @@ describe('platform_browser/strategies/phone', () => {
});
});

it('works without v2 RecaptchaVerifier when recaptcha enterprise is enabled', async () => {
if (typeof window === 'undefined') {
return;
}
mockRecaptchaEnterpriseEnablement(EnforcementState.ENFORCE);
const sessionInfo = await _verifyPhoneNumber(auth, 'number');
expect(sessionInfo).to.eq('session-info');
expect(sendCodeEndpoint.calls[0].request).to.eql({
phoneNumber: 'number',
captchaResponse: RECAPTCHA_ENTERPRISE_TOKEN,
clientType: RecaptchaClientType.WEB,
recaptchaVersion: RecaptchaVersion.ENTERPRISE
});
});

it('throws error if calls verify phone number without v2 RecaptchaVerifier when recaptcha enterprise is disabled', async () => {
if (typeof window === 'undefined') {
return;
}
mockRecaptchaEnterpriseEnablement(EnforcementState.OFF);

await expect(_verifyPhoneNumber(auth, 'number')).to.be.rejectedWith(
FirebaseError,
'auth/argument-error'
);
});

it('calls fallback to recaptcha v2 flow when receiving MISSING_RECAPTCHA_TOKEN error in recaptcha enterprise audit mode', async () => {
if (typeof window === 'undefined') {
return;
Expand Down
20 changes: 10 additions & 10 deletions packages/auth/src/platform_browser/strategies/phone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class ConfirmationResultImpl implements ConfirmationResult {
export async function signInWithPhoneNumber(
auth: Auth,
phoneNumber: string,
appVerifier: ApplicationVerifier
appVerifier?: ApplicationVerifier
): Promise<ConfirmationResult> {
if (_isFirebaseServerApp(auth.app)) {
return Promise.reject(
Expand Down Expand Up @@ -162,7 +162,7 @@ export async function signInWithPhoneNumber(
export async function linkWithPhoneNumber(
user: User,
phoneNumber: string,
appVerifier: ApplicationVerifier
appVerifier?: ApplicationVerifier
): Promise<ConfirmationResult> {
const userInternal = getModularInstance(user) as UserInternal;
await _assertLinkedStatus(false, userInternal, ProviderId.PHONE);
Expand Down Expand Up @@ -194,7 +194,7 @@ export async function linkWithPhoneNumber(
export async function reauthenticateWithPhoneNumber(
user: User,
phoneNumber: string,
appVerifier: ApplicationVerifier
appVerifier?: ApplicationVerifier
): Promise<ConfirmationResult> {
const userInternal = getModularInstance(user) as UserInternal;
if (_isFirebaseServerApp(userInternal.auth.app)) {
Expand Down Expand Up @@ -224,7 +224,7 @@ type PhoneApiCaller<TRequest, TResponse> = (
export async function _verifyPhoneNumber(
auth: AuthInternal,
options: PhoneInfoOptions | string,
verifier: ApplicationVerifierInternal
verifier?: ApplicationVerifierInternal
): Promise<string> {
if (!auth._getRecaptchaConfig()) {
const enterpriseVerifier = new RecaptchaEnterpriseVerifier(auth);
Expand Down Expand Up @@ -274,7 +274,7 @@ export async function _verifyPhoneNumber(
request.phoneEnrollmentInfo.captchaResponse === FAKE_TOKEN
) {
_assert(
verifier.type === RECAPTCHA_VERIFIER_TYPE,
verifier?.type === RECAPTCHA_VERIFIER_TYPE,
authInstance,
AuthErrorCode.ARGUMENT_ERROR
);
Expand Down Expand Up @@ -329,14 +329,14 @@ export async function _verifyPhoneNumber(
authInstance: AuthInternal,
request: StartPhoneMfaSignInRequest
) => {
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch v2 token and inject into request.
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch reCAPTCHA v2 token and inject into request.
if (
!request.phoneSignInInfo.captchaResponse ||
request.phoneSignInInfo.captchaResponse.length === 0 ||
request.phoneSignInInfo.captchaResponse === FAKE_TOKEN
) {
_assert(
verifier.type === RECAPTCHA_VERIFIER_TYPE,
verifier?.type === RECAPTCHA_VERIFIER_TYPE,
authInstance,
AuthErrorCode.ARGUMENT_ERROR
);
Expand Down Expand Up @@ -380,14 +380,14 @@ export async function _verifyPhoneNumber(
authInstance: AuthInternal,
request: SendPhoneVerificationCodeRequest
) => {
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch v2 token and inject into request.
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch reCAPTCHA v2 token and inject into request.
if (
!request.captchaResponse ||
request.captchaResponse.length === 0 ||
request.captchaResponse === FAKE_TOKEN
) {
_assert(
verifier.type === RECAPTCHA_VERIFIER_TYPE,
verifier?.type === RECAPTCHA_VERIFIER_TYPE,
authInstance,
AuthErrorCode.ARGUMENT_ERROR
);
Expand Down Expand Up @@ -421,7 +421,7 @@ export async function _verifyPhoneNumber(
return response.sessionInfo;
}
} finally {
verifier._reset();
verifier?._reset();
}
}

Expand Down

0 comments on commit 1dee30b

Please sign in to comment.