From fbb9bd6c735b2142443cd0ec8d43350ae1d905eb Mon Sep 17 00:00:00 2001 From: renkelvin Date: Mon, 24 Apr 2023 11:24:42 -0700 Subject: [PATCH 1/6] Fail open and send auth request to the GCIP backend if Recaptcha token fetch failed. --- .../recaptcha_enterprise_verifier.ts | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.ts b/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.ts index b051a05e88a..dd7e4225a68 100644 --- a/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.ts +++ b/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.ts @@ -33,6 +33,7 @@ const RECAPTCHA_ENTERPRISE_URL = 'https://www.google.com/recaptcha/enterprise.js?render='; export const RECAPTCHA_ENTERPRISE_VERIFIER_TYPE = 'recaptcha-enterprise'; +export const FAKE_TOKEN = 'NO_RECAPTCHA'; export class RecaptchaEnterpriseVerifier { /** @@ -105,18 +106,14 @@ export class RecaptchaEnterpriseVerifier { const grecaptcha = window.grecaptcha; if (isEnterprise(grecaptcha)) { grecaptcha.enterprise.ready(() => { - try { - grecaptcha.enterprise - .execute(siteKey, { action }) - .then(token => { - resolve(token); - }) - .catch(error => { - reject(error); - }); - } catch (error) { - reject(error); - } + grecaptcha.enterprise + .execute(siteKey, { action }) + .then(token => { + resolve(token); + }) + .catch(() => { + resolve(FAKE_TOKEN); + }); }); } else { reject(Error('No reCAPTCHA enterprise script loaded.')); From 98162adca53120f33efa7715d5f70c5f113618fb Mon Sep 17 00:00:00 2001 From: renkelvin Date: Mon, 24 Apr 2023 11:28:55 -0700 Subject: [PATCH 2/6] Update recaptcha_enterprise_verifier.test.ts --- .../recaptcha/recaptcha_enterprise_verifier.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts b/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts index 90e548cf98c..fe239c792d3 100644 --- a/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts +++ b/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts @@ -102,7 +102,7 @@ describe('platform_browser/recaptcha/recaptcha_enterprise_verifier', () => { ); }); - it('reject if error is thrown when retieve recaptcha token', async () => { + it('return fake recaptcha token if error is thrown when retieve recaptcha token', async () => { mockEndpointWithParams( Endpoint.GET_RECAPTCHA_CONFIG, request, @@ -111,10 +111,7 @@ describe('platform_browser/recaptcha/recaptcha_enterprise_verifier', () => { sinon .stub(recaptcha.enterprise, 'execute') .returns(Promise.reject(Error('retieve-recaptcha-token-error'))); - await expect(verifier.verify()).to.be.rejectedWith( - Error, - 'retieve-recaptcha-token-error' - ); + expect(await verifier.verify()).to.eq('NO_RECAPTCHA'); }); }); }); From 1874faafa8b212218824966e7ace47e67bdfd655 Mon Sep 17 00:00:00 2001 From: renkelvin Date: Mon, 24 Apr 2023 20:40:49 -0700 Subject: [PATCH 3/6] Update email.test.ts --- packages/auth/src/core/credentials/email.test.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/auth/src/core/credentials/email.test.ts b/packages/auth/src/core/credentials/email.test.ts index 5cdd1c01740..5c4a7438a12 100644 --- a/packages/auth/src/core/credentials/email.test.ts +++ b/packages/auth/src/core/credentials/email.test.ts @@ -197,17 +197,9 @@ describe('core/credentials/email', () => { const stub = sinon.stub(recaptcha.enterprise, 'execute'); // First verification should fail with 'wrong-site-key' - stub - .withArgs('wrong-site-key', { - action: RecaptchaActionName.SIGN_IN_WITH_PASSWORD - }) - .rejects(); // Second verifcation should succeed with site key refreshed - stub - .withArgs('site-key', { - action: RecaptchaActionName.SIGN_IN_WITH_PASSWORD - }) - .returns(Promise.resolve('recaptcha-response')); + stub.onCall(0).rejects(); + stub.onCall(1).returns(Promise.resolve('recaptcha-response')); mockEndpointWithParams( Endpoint.GET_RECAPTCHA_CONFIG, From 99c13e6914ab7a650ce293b039ca7dcec0a898c1 Mon Sep 17 00:00:00 2001 From: renkelvin Date: Mon, 24 Apr 2023 23:29:35 -0700 Subject: [PATCH 4/6] address pr issues --- packages/auth/src/core/credentials/email.test.ts | 4 ++-- .../recaptcha/recaptcha_enterprise_verifier.test.ts | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/auth/src/core/credentials/email.test.ts b/packages/auth/src/core/credentials/email.test.ts index 5c4a7438a12..a57440e0a89 100644 --- a/packages/auth/src/core/credentials/email.test.ts +++ b/packages/auth/src/core/credentials/email.test.ts @@ -196,7 +196,7 @@ describe('core/credentials/email', () => { window.grecaptcha = recaptcha; const stub = sinon.stub(recaptcha.enterprise, 'execute'); - // First verification should fail with 'wrong-site-key' + // Force first verification call to fail to simulate verification error // Second verifcation should succeed with site key refreshed stub.onCall(0).rejects(); stub.onCall(1).returns(Promise.resolve('recaptcha-response')); @@ -210,7 +210,7 @@ describe('core/credentials/email', () => { recaptchaConfigResponseEnforce ); await auth.initializeRecaptchaConfig(); - auth._agentRecaptchaConfig!.siteKey = 'wrong-site-key'; + auth._agentRecaptchaConfig!.siteKey = 'cached-site-key'; const idTokenResponse = await credential._getIdTokenResponse(auth); expect(idTokenResponse.idToken).to.eq('id-token'); diff --git a/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts b/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts index fe239c792d3..b1cbd959c4c 100644 --- a/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts +++ b/packages/auth/src/platform_browser/recaptcha/recaptcha_enterprise_verifier.test.ts @@ -27,7 +27,10 @@ import * as mockFetch from '../../../test/helpers/mock_fetch'; import { ServerError } from '../../api/errors'; import { MockGreCAPTCHATopLevel } from './recaptcha_mock'; -import { RecaptchaEnterpriseVerifier } from './recaptcha_enterprise_verifier'; +import { + RecaptchaEnterpriseVerifier, + FAKE_TOKEN +} from './recaptcha_enterprise_verifier'; use(chaiAsPromised); use(sinonChai); @@ -81,7 +84,7 @@ describe('platform_browser/recaptcha/recaptcha_enterprise_verifier', () => { expect(await verifier.verify()).to.eq('recaptcha-response'); }); - it('reject if error is thrown when retieve site key', async () => { + it('reject if error is thrown when retrieve site key', async () => { mockEndpointWithParams( Endpoint.GET_RECAPTCHA_CONFIG, request, @@ -102,7 +105,7 @@ describe('platform_browser/recaptcha/recaptcha_enterprise_verifier', () => { ); }); - it('return fake recaptcha token if error is thrown when retieve recaptcha token', async () => { + it('return fake recaptcha token if error is thrown when retrieve recaptcha token', async () => { mockEndpointWithParams( Endpoint.GET_RECAPTCHA_CONFIG, request, @@ -110,8 +113,8 @@ describe('platform_browser/recaptcha/recaptcha_enterprise_verifier', () => { ); sinon .stub(recaptcha.enterprise, 'execute') - .returns(Promise.reject(Error('retieve-recaptcha-token-error'))); - expect(await verifier.verify()).to.eq('NO_RECAPTCHA'); + .returns(Promise.reject(Error('retrieve-recaptcha-token-error'))); + expect(await verifier.verify()).to.eq(FAKE_TOKEN); }); }); }); From 40ba4a7ced82f3c8df47f38f25d88a43551d4f73 Mon Sep 17 00:00:00 2001 From: renkelvin Date: Tue, 25 Apr 2023 11:09:36 -0700 Subject: [PATCH 5/6] Update email.test.ts --- .../auth/src/core/credentials/email.test.ts | 33 +++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/packages/auth/src/core/credentials/email.test.ts b/packages/auth/src/core/credentials/email.test.ts index a57440e0a89..565bcfd56d9 100644 --- a/packages/auth/src/core/credentials/email.test.ts +++ b/packages/auth/src/core/credentials/email.test.ts @@ -184,24 +184,16 @@ describe('core/credentials/email', () => { }); }); - it('calls sign in with password with recaptcha forced refresh succeed', async () => { + it('calls sign in with password with recaptcha forced refresh', async () => { if (typeof window === 'undefined') { return; } - // Mock recaptcha js loading method and manually set window.recaptcha + // Mock recaptcha js loading method but not set window.recaptcha to simulate recaptcha token retrieval failure sinon .stub(jsHelpers, '_loadJS') .returns(Promise.resolve(new Event(''))); - const recaptcha = new MockGreCAPTCHATopLevel(); - window.grecaptcha = recaptcha; - const stub = sinon.stub(recaptcha.enterprise, 'execute'); - - // Force first verification call to fail to simulate verification error - // Second verifcation should succeed with site key refreshed - stub.onCall(0).rejects(); - stub.onCall(1).returns(Promise.resolve('recaptcha-response')); - mockEndpointWithParams( + const getRecaptchaConfigMock = mockEndpointWithParams( Endpoint.GET_RECAPTCHA_CONFIG, { clientType: RecaptchaClientType.WEB, @@ -212,19 +204,12 @@ describe('core/credentials/email', () => { await auth.initializeRecaptchaConfig(); auth._agentRecaptchaConfig!.siteKey = 'cached-site-key'; - const idTokenResponse = await credential._getIdTokenResponse(auth); - expect(idTokenResponse.idToken).to.eq('id-token'); - expect(idTokenResponse.refreshToken).to.eq('refresh-token'); - expect(idTokenResponse.expiresIn).to.eq('1234'); - expect(idTokenResponse.localId).to.eq(serverUser.localId); - expect(apiMock.calls[0].request).to.eql({ - captchaResponse: 'recaptcha-response', - clientType: RecaptchaClientType.WEB, - email: 'some-email', - password: 'some-password', - recaptchaVersion: RecaptchaVersion.ENTERPRISE, - returnSecureToken: true - }); + await expect(credential._getIdTokenResponse(auth)).to.be.rejectedWith( + 'No reCAPTCHA enterprise script loaded.' + ); + // Should call getRecaptchaConfig once to refresh the cached recaptcha config + expect(getRecaptchaConfigMock.calls.length).to.eq(2); + expect(auth._agentRecaptchaConfig?.siteKey).to.eq('site-key'); }); it('calls fallback to recaptcha flow when receiving MISSING_RECAPTCHA_TOKEN error', async () => { From 7fad65c51d4104bab0a8f2c23f7bb731b439a7d6 Mon Sep 17 00:00:00 2001 From: renkelvin Date: Tue, 25 Apr 2023 11:47:15 -0700 Subject: [PATCH 6/6] Update email.test.ts --- packages/auth/src/core/credentials/email.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/auth/src/core/credentials/email.test.ts b/packages/auth/src/core/credentials/email.test.ts index 565bcfd56d9..bcf14ca437a 100644 --- a/packages/auth/src/core/credentials/email.test.ts +++ b/packages/auth/src/core/credentials/email.test.ts @@ -192,6 +192,7 @@ describe('core/credentials/email', () => { sinon .stub(jsHelpers, '_loadJS') .returns(Promise.resolve(new Event(''))); + window.grecaptcha = undefined; const getRecaptchaConfigMock = mockEndpointWithParams( Endpoint.GET_RECAPTCHA_CONFIG,