Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ApplicationVerifier params optional in Phone Auth APIs #8366

Merged
merged 2 commits into from
Jul 31, 2024
Merged
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
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
Loading