Skip to content

Commit 1dee30b

Browse files
NhienLamNhienLam
authored
Make ApplicationVerifier params optional in Phone Auth APIs (#8366)
* Make ApplicationVerifier params optional in Phone APIs * Add more unit tests for when ApplicationVerifier is not available
1 parent 56f904c commit 1dee30b

File tree

8 files changed

+156
-21
lines changed

8 files changed

+156
-21
lines changed

‎common/api-review/auth.api.md

+4-4
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ export function isSignInWithEmailLink(auth: Auth, emailLink: string): boolean;
445445
export function linkWithCredential(user: User, credential: AuthCredential): Promise<UserCredential>;
446446

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

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

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

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

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

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

783783
// @public
784784
export function signInWithPopup(auth: Auth, provider: AuthProvider, resolver?: PopupRedirectResolver): Promise<UserCredential>;

‎docs-devsite/auth.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ This method does not work in a Node.js environment or with [Auth](./auth.auth.md
930930
<b>Signature:</b>
931931

932932
```typescript
933-
export declare function signInWithPhoneNumber(auth: Auth, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
933+
export declare function signInWithPhoneNumber(auth: Auth, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;
934934
```
935935

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

13061306
```typescript
1307-
export declare function linkWithPhoneNumber(user: User, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
1307+
export declare function linkWithPhoneNumber(user: User, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;
13081308
```
13091309

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

14591459
```typescript
1460-
export declare function reauthenticateWithPhoneNumber(user: User, phoneNumber: string, appVerifier: ApplicationVerifier): Promise<ConfirmationResult>;
1460+
export declare function reauthenticateWithPhoneNumber(user: User, phoneNumber: string, appVerifier?: ApplicationVerifier): Promise<ConfirmationResult>;
14611461
```
14621462

14631463
#### Parameters

‎docs-devsite/auth.phoneauthprovider.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ Starts a phone number authentication flow by sending a verification code to the
203203
<b>Signature:</b>
204204

205205
```typescript
206-
verifyPhoneNumber(phoneOptions: PhoneInfoOptions | string, applicationVerifier: ApplicationVerifier): Promise<string>;
206+
verifyPhoneNumber(phoneOptions: PhoneInfoOptions | string, applicationVerifier?: ApplicationVerifier): Promise<string>;
207207
```
208208

209209
#### Parameters

‎packages/auth/src/api/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export const enum RecaptchaActionName {
8989
SIGN_UP_PASSWORD = 'signUpPassword',
9090
SEND_VERIFICATION_CODE = 'sendVerificationCode',
9191
MFA_SMS_ENROLLMENT = 'mfaSmsEnrollment',
92-
MFA_SMS_SIGNIN = 'mfaSmsSignin'
92+
MFA_SMS_SIGNIN = 'mfaSmsSignIn'
9393
}
9494

9595
export const enum EnforcementState {

‎packages/auth/src/platform_browser/providers/phone.test.ts

+83-1
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { expect } from 'chai';
18+
import { expect, use } from 'chai';
19+
import chaiAsPromised from 'chai-as-promised';
1920
import * as sinon from 'sinon';
2021

22+
import { FirebaseError } from '@firebase/util';
23+
2124
import {
2225
mockEndpoint,
2326
mockEndpointWithParams
@@ -37,6 +40,8 @@ import { FAKE_TOKEN } from '../recaptcha/recaptcha_enterprise_verifier';
3740
import { MockGreCAPTCHATopLevel } from '../recaptcha/recaptcha_mock';
3841
import { ApplicationVerifierInternal } from '../../model/application_verifier';
3942

43+
use(chaiAsPromised);
44+
4045
describe('platform_browser/providers/phone', () => {
4146
let auth: TestAuth;
4247
let v2Verifier: ApplicationVerifierInternal;
@@ -104,6 +109,83 @@ describe('platform_browser/providers/phone', () => {
104109
});
105110
});
106111

112+
it('throws an error if verify without appVerifier when recaptcha enterprise is disabled', async () => {
113+
const recaptchaConfigResponseOff = {
114+
recaptchaKey: 'foo/bar/to/site-key',
115+
recaptchaEnforcementState: [
116+
{
117+
provider: RecaptchaAuthProvider.PHONE_PROVIDER,
118+
enforcementState: EnforcementState.OFF
119+
}
120+
]
121+
};
122+
const recaptcha = new MockGreCAPTCHATopLevel();
123+
if (typeof window === 'undefined') {
124+
return;
125+
}
126+
window.grecaptcha = recaptcha;
127+
sinon
128+
.stub(recaptcha.enterprise, 'execute')
129+
.returns(Promise.resolve('enterprise-token'));
130+
131+
mockEndpointWithParams(
132+
Endpoint.GET_RECAPTCHA_CONFIG,
133+
{
134+
clientType: RecaptchaClientType.WEB,
135+
version: RecaptchaVersion.ENTERPRISE
136+
},
137+
recaptchaConfigResponseOff
138+
);
139+
140+
const provider = new PhoneAuthProvider(auth);
141+
await expect(
142+
provider.verifyPhoneNumber('+15105550000')
143+
).to.be.rejectedWith(FirebaseError, 'auth/argument-error');
144+
});
145+
146+
it('calls the server without appVerifier when recaptcha enterprise is enabled', async () => {
147+
const recaptchaConfigResponseEnforce = {
148+
recaptchaKey: 'foo/bar/to/site-key',
149+
recaptchaEnforcementState: [
150+
{
151+
provider: RecaptchaAuthProvider.PHONE_PROVIDER,
152+
enforcementState: EnforcementState.ENFORCE
153+
}
154+
]
155+
};
156+
const recaptcha = new MockGreCAPTCHATopLevel();
157+
if (typeof window === 'undefined') {
158+
return;
159+
}
160+
window.grecaptcha = recaptcha;
161+
sinon
162+
.stub(recaptcha.enterprise, 'execute')
163+
.returns(Promise.resolve('enterprise-token'));
164+
165+
mockEndpointWithParams(
166+
Endpoint.GET_RECAPTCHA_CONFIG,
167+
{
168+
clientType: RecaptchaClientType.WEB,
169+
version: RecaptchaVersion.ENTERPRISE
170+
},
171+
recaptchaConfigResponseEnforce
172+
);
173+
174+
const route = mockEndpoint(Endpoint.SEND_VERIFICATION_CODE, {
175+
sessionInfo: 'verification-id'
176+
});
177+
178+
const provider = new PhoneAuthProvider(auth);
179+
const result = await provider.verifyPhoneNumber('+15105550000');
180+
expect(result).to.eq('verification-id');
181+
expect(route.calls[0].request).to.eql({
182+
phoneNumber: '+15105550000',
183+
captchaResponse: 'enterprise-token',
184+
clientType: RecaptchaClientType.WEB,
185+
recaptchaVersion: RecaptchaVersion.ENTERPRISE
186+
});
187+
});
188+
107189
it('calls the server when recaptcha enterprise is enabled', async () => {
108190
const recaptchaConfigResponseEnforce = {
109191
recaptchaKey: 'foo/bar/to/site-key',

‎packages/auth/src/platform_browser/providers/phone.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class PhoneAuthProvider {
104104
*/
105105
verifyPhoneNumber(
106106
phoneOptions: PhoneInfoOptions | string,
107-
applicationVerifier: ApplicationVerifier
107+
applicationVerifier?: ApplicationVerifier
108108
): Promise<string> {
109109
return _verifyPhoneNumber(
110110
this.auth,

‎packages/auth/src/platform_browser/strategies/phone.test.ts

+53
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,32 @@ describe('platform_browser/strategies/phone', () => {
181181
});
182182
});
183183

184+
it('calls verify phone number without a v2 RecaptchaVerifier when recaptcha enterprise is enabled', async () => {
185+
if (typeof window === 'undefined') {
186+
return;
187+
}
188+
mockRecaptchaEnterpriseEnablement(EnforcementState.ENFORCE);
189+
await signInWithPhoneNumber(auth, '+15105550000');
190+
191+
expect(sendCodeEndpoint.calls[0].request).to.eql({
192+
phoneNumber: '+15105550000',
193+
captchaResponse: RECAPTCHA_ENTERPRISE_TOKEN,
194+
clientType: RecaptchaClientType.WEB,
195+
recaptchaVersion: RecaptchaVersion.ENTERPRISE
196+
});
197+
});
198+
199+
it('throws an error if verify phone number without a v2 RecaptchaVerifier when recaptcha enterprise is disabled', async () => {
200+
if (typeof window === 'undefined') {
201+
return;
202+
}
203+
mockRecaptchaEnterpriseEnablement(EnforcementState.OFF);
204+
205+
await expect(
206+
signInWithPhoneNumber(auth, '+15105550000')
207+
).to.be.rejectedWith(FirebaseError, 'auth/argument-error');
208+
});
209+
184210
context('ConfirmationResult', () => {
185211
it('result contains verification id baked in', async () => {
186212
if (typeof window === 'undefined') {
@@ -504,6 +530,33 @@ describe('platform_browser/strategies/phone', () => {
504530
});
505531
});
506532

533+
it('works without v2 RecaptchaVerifier when recaptcha enterprise is enabled', async () => {
534+
if (typeof window === 'undefined') {
535+
return;
536+
}
537+
mockRecaptchaEnterpriseEnablement(EnforcementState.ENFORCE);
538+
const sessionInfo = await _verifyPhoneNumber(auth, 'number');
539+
expect(sessionInfo).to.eq('session-info');
540+
expect(sendCodeEndpoint.calls[0].request).to.eql({
541+
phoneNumber: 'number',
542+
captchaResponse: RECAPTCHA_ENTERPRISE_TOKEN,
543+
clientType: RecaptchaClientType.WEB,
544+
recaptchaVersion: RecaptchaVersion.ENTERPRISE
545+
});
546+
});
547+
548+
it('throws error if calls verify phone number without v2 RecaptchaVerifier when recaptcha enterprise is disabled', async () => {
549+
if (typeof window === 'undefined') {
550+
return;
551+
}
552+
mockRecaptchaEnterpriseEnablement(EnforcementState.OFF);
553+
554+
await expect(_verifyPhoneNumber(auth, 'number')).to.be.rejectedWith(
555+
FirebaseError,
556+
'auth/argument-error'
557+
);
558+
});
559+
507560
it('calls fallback to recaptcha v2 flow when receiving MISSING_RECAPTCHA_TOKEN error in recaptcha enterprise audit mode', async () => {
508561
if (typeof window === 'undefined') {
509562
return;

‎packages/auth/src/platform_browser/strategies/phone.ts

+10-10
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class ConfirmationResultImpl implements ConfirmationResult {
129129
export async function signInWithPhoneNumber(
130130
auth: Auth,
131131
phoneNumber: string,
132-
appVerifier: ApplicationVerifier
132+
appVerifier?: ApplicationVerifier
133133
): Promise<ConfirmationResult> {
134134
if (_isFirebaseServerApp(auth.app)) {
135135
return Promise.reject(
@@ -162,7 +162,7 @@ export async function signInWithPhoneNumber(
162162
export async function linkWithPhoneNumber(
163163
user: User,
164164
phoneNumber: string,
165-
appVerifier: ApplicationVerifier
165+
appVerifier?: ApplicationVerifier
166166
): Promise<ConfirmationResult> {
167167
const userInternal = getModularInstance(user) as UserInternal;
168168
await _assertLinkedStatus(false, userInternal, ProviderId.PHONE);
@@ -194,7 +194,7 @@ export async function linkWithPhoneNumber(
194194
export async function reauthenticateWithPhoneNumber(
195195
user: User,
196196
phoneNumber: string,
197-
appVerifier: ApplicationVerifier
197+
appVerifier?: ApplicationVerifier
198198
): Promise<ConfirmationResult> {
199199
const userInternal = getModularInstance(user) as UserInternal;
200200
if (_isFirebaseServerApp(userInternal.auth.app)) {
@@ -224,7 +224,7 @@ type PhoneApiCaller<TRequest, TResponse> = (
224224
export async function _verifyPhoneNumber(
225225
auth: AuthInternal,
226226
options: PhoneInfoOptions | string,
227-
verifier: ApplicationVerifierInternal
227+
verifier?: ApplicationVerifierInternal
228228
): Promise<string> {
229229
if (!auth._getRecaptchaConfig()) {
230230
const enterpriseVerifier = new RecaptchaEnterpriseVerifier(auth);
@@ -274,7 +274,7 @@ export async function _verifyPhoneNumber(
274274
request.phoneEnrollmentInfo.captchaResponse === FAKE_TOKEN
275275
) {
276276
_assert(
277-
verifier.type === RECAPTCHA_VERIFIER_TYPE,
277+
verifier?.type === RECAPTCHA_VERIFIER_TYPE,
278278
authInstance,
279279
AuthErrorCode.ARGUMENT_ERROR
280280
);
@@ -329,14 +329,14 @@ export async function _verifyPhoneNumber(
329329
authInstance: AuthInternal,
330330
request: StartPhoneMfaSignInRequest
331331
) => {
332-
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch v2 token and inject into request.
332+
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch reCAPTCHA v2 token and inject into request.
333333
if (
334334
!request.phoneSignInInfo.captchaResponse ||
335335
request.phoneSignInInfo.captchaResponse.length === 0 ||
336336
request.phoneSignInInfo.captchaResponse === FAKE_TOKEN
337337
) {
338338
_assert(
339-
verifier.type === RECAPTCHA_VERIFIER_TYPE,
339+
verifier?.type === RECAPTCHA_VERIFIER_TYPE,
340340
authInstance,
341341
AuthErrorCode.ARGUMENT_ERROR
342342
);
@@ -380,14 +380,14 @@ export async function _verifyPhoneNumber(
380380
authInstance: AuthInternal,
381381
request: SendPhoneVerificationCodeRequest
382382
) => {
383-
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch v2 token and inject into request.
383+
// If reCAPTCHA Enterprise token is empty or "NO_RECAPTCHA", fetch reCAPTCHA v2 token and inject into request.
384384
if (
385385
!request.captchaResponse ||
386386
request.captchaResponse.length === 0 ||
387387
request.captchaResponse === FAKE_TOKEN
388388
) {
389389
_assert(
390-
verifier.type === RECAPTCHA_VERIFIER_TYPE,
390+
verifier?.type === RECAPTCHA_VERIFIER_TYPE,
391391
authInstance,
392392
AuthErrorCode.ARGUMENT_ERROR
393393
);
@@ -421,7 +421,7 @@ export async function _verifyPhoneNumber(
421421
return response.sessionInfo;
422422
}
423423
} finally {
424-
verifier._reset();
424+
verifier?._reset();
425425
}
426426
}
427427

0 commit comments

Comments
 (0)