Skip to content

Commit d9a94e5

Browse files
hsubox76hsubox76
committed
Address PR comments
1 parent 5e63c0e commit d9a94e5

File tree

6 files changed

+130
-92
lines changed

6 files changed

+130
-92
lines changed

‎packages/app-check/src/api.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ describe('api', () => {
186186
});
187187

188188
it('initialize reCAPTCHA when a ReCaptchaV3Provider is provided', () => {
189-
const initReCAPTCHAStub = stub(reCAPTCHA, 'initialize').returns(
189+
const initReCAPTCHAStub = stub(reCAPTCHA, 'initializeV3').returns(
190190
Promise.resolve({} as any)
191191
);
192192
initializeAppCheck(app, {
@@ -199,16 +199,15 @@ describe('api', () => {
199199
});
200200

201201
it('initialize reCAPTCHA when a ReCaptchaEnterpriseProvider is provided', () => {
202-
const initReCAPTCHAStub = stub(reCAPTCHA, 'initialize').returns(
202+
const initReCAPTCHAStub = stub(reCAPTCHA, 'initializeEnterprise').returns(
203203
Promise.resolve({} as any)
204204
);
205205
initializeAppCheck(app, {
206206
provider: new ReCaptchaEnterpriseProvider(FAKE_SITE_KEY)
207207
});
208208
expect(initReCAPTCHAStub).to.have.been.calledWithExactly(
209209
app,
210-
FAKE_SITE_KEY,
211-
true
210+
FAKE_SITE_KEY
212211
);
213212
});
214213

‎packages/app-check/src/client.test.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { expect } from 'chai';
2020
import { stub, SinonStub, useFakeTimers } from 'sinon';
2121
import { FirebaseApp } from '@firebase/app';
2222
import { getFakeApp, getFakePlatformLoggingProvider } from '../test/util';
23-
import { getExchangeRecaptchaTokenRequest, exchangeToken } from './client';
23+
import { getExchangeRecaptchaV3TokenRequest, exchangeToken, getExchangeRecaptchaEnterpriseTokenRequest } from './client';
2424
import { FirebaseError } from '@firebase/util';
2525
import { ERROR_FACTORY, AppCheckError } from './errors';
2626
import { BASE_ENDPOINT } from './constants';
@@ -36,7 +36,7 @@ describe('client', () => {
3636
});
3737

3838
it('creates exchange recaptcha token request correctly', () => {
39-
const request = getExchangeRecaptchaTokenRequest(
39+
const request = getExchangeRecaptchaV3TokenRequest(
4040
app,
4141
'fake-recaptcha-token'
4242
);
@@ -52,10 +52,9 @@ describe('client', () => {
5252
});
5353

5454
it('creates exchange recaptcha enterprise token request correctly', () => {
55-
const request = getExchangeRecaptchaTokenRequest(
55+
const request = getExchangeRecaptchaEnterpriseTokenRequest(
5656
app,
57-
'fake-recaptcha-token',
58-
true
57+
'fake-recaptcha-token'
5958
);
6059
const { projectId, appId, apiKey } = app.options;
6160

@@ -82,7 +81,7 @@ describe('client', () => {
8281
);
8382

8483
const response = await exchangeToken(
85-
getExchangeRecaptchaTokenRequest(app, 'fake-custom-token'),
84+
getExchangeRecaptchaV3TokenRequest(app, 'fake-custom-token'),
8685
getFakePlatformLoggingProvider('a/1.2.3 fire-app-check/2.3.4')
8786
);
8887

@@ -110,7 +109,7 @@ describe('client', () => {
110109

111110
try {
112111
await exchangeToken(
113-
getExchangeRecaptchaTokenRequest(app, 'fake-custom-token'),
112+
getExchangeRecaptchaV3TokenRequest(app, 'fake-custom-token'),
114113
getFakePlatformLoggingProvider()
115114
);
116115
} catch (e) {
@@ -139,7 +138,7 @@ describe('client', () => {
139138

140139
try {
141140
await exchangeToken(
142-
getExchangeRecaptchaTokenRequest(app, 'fake-custom-token'),
141+
getExchangeRecaptchaV3TokenRequest(app, 'fake-custom-token'),
143142
getFakePlatformLoggingProvider()
144143
);
145144
} catch (e) {
@@ -167,7 +166,7 @@ describe('client', () => {
167166

168167
try {
169168
await exchangeToken(
170-
getExchangeRecaptchaTokenRequest(app, 'fake-custom-token'),
169+
getExchangeRecaptchaV3TokenRequest(app, 'fake-custom-token'),
171170
getFakePlatformLoggingProvider()
172171
);
173172
} catch (e) {
@@ -201,7 +200,7 @@ describe('client', () => {
201200

202201
try {
203202
await exchangeToken(
204-
getExchangeRecaptchaTokenRequest(app, 'fake-custom-token'),
203+
getExchangeRecaptchaV3TokenRequest(app, 'fake-custom-token'),
205204
getFakePlatformLoggingProvider()
206205
);
207206
} catch (e) {

‎packages/app-check/src/client.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,23 +104,30 @@ export async function exchangeToken(
104104
};
105105
}
106106

107-
export function getExchangeRecaptchaTokenRequest(
107+
export function getExchangeRecaptchaV3TokenRequest(
108108
app: FirebaseApp,
109-
reCAPTCHAToken: string,
110-
isEnterprise: boolean = false
109+
reCAPTCHAToken: string
111110
): AppCheckRequest {
112111
const { projectId, appId, apiKey } = app.options;
113-
const fieldName = isEnterprise
114-
? 'recaptcha_enterprise_token'
115-
: 'recaptcha_token';
116-
const exchangeMethod = isEnterprise
117-
? EXCHANGE_RECAPTCHA_ENTERPRISE_TOKEN_METHOD
118-
: EXCHANGE_RECAPTCHA_TOKEN_METHOD;
119112

120113
return {
121-
url: `${BASE_ENDPOINT}/projects/${projectId}/apps/${appId}:${exchangeMethod}?key=${apiKey}`,
114+
url: `${BASE_ENDPOINT}/projects/${projectId}/apps/${appId}:${EXCHANGE_RECAPTCHA_TOKEN_METHOD}?key=${apiKey}`,
122115
body: {
123-
[fieldName]: reCAPTCHAToken
116+
'recaptcha_token': reCAPTCHAToken
117+
}
118+
};
119+
}
120+
121+
export function getExchangeRecaptchaEnterpriseTokenRequest(
122+
app: FirebaseApp,
123+
reCAPTCHAToken: string
124+
): AppCheckRequest {
125+
const { projectId, appId, apiKey } = app.options;
126+
127+
return {
128+
url: `${BASE_ENDPOINT}/projects/${projectId}/apps/${appId}:${EXCHANGE_RECAPTCHA_ENTERPRISE_TOKEN_METHOD}?key=${apiKey}`,
129+
body: {
130+
'recaptcha_enterprise_token': reCAPTCHAToken
124131
}
125132
};
126133
}

‎packages/app-check/src/providers.ts

Lines changed: 15 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818
import { FirebaseApp, _getProvider } from '@firebase/app';
1919
import { Provider } from '@firebase/component';
2020
import { issuedAtTime } from '@firebase/util';
21-
import { exchangeToken, getExchangeRecaptchaTokenRequest } from './client';
21+
import { exchangeToken, getExchangeRecaptchaEnterpriseTokenRequest, getExchangeRecaptchaV3TokenRequest } from './client';
2222
import { AppCheckError, ERROR_FACTORY } from './errors';
2323
import { CustomProviderOptions } from './public-types';
2424
import {
2525
getToken as getReCAPTCHAToken,
26-
initialize as initializeRecaptcha
26+
initializeV3 as initializeRecaptchaV3,
27+
initializeEnterprise as initializeRecaptchaEnterprise
2728
} from './recaptcha';
2829
import { AppCheckProvider, AppCheckTokenInternal } from './types';
2930

@@ -47,21 +48,15 @@ export class ReCaptchaV3Provider implements AppCheckProvider {
4748
* @internal
4849
*/
4950
async getToken(): Promise<AppCheckTokenInternal> {
50-
if (!this._app || !this._platformLoggerProvider) {
51-
// This should only occur if user has not called initializeAppCheck().
52-
// We don't have an appName to provide if so.
53-
// This should already be caught in the top level `getToken()` function.
54-
throw ERROR_FACTORY.create(AppCheckError.USE_BEFORE_ACTIVATION, {
55-
appName: ''
56-
});
57-
}
58-
const attestedClaimsToken = await getReCAPTCHAToken(this._app).catch(_e => {
51+
// Top-level `getToken()` has already checked that App Check is initialized
52+
// and therefore this._app and this._platformLoggerProvider are available.
53+
const attestedClaimsToken = await getReCAPTCHAToken(this._app!).catch(_e => {
5954
// reCaptcha.execute() throws null which is not very descriptive.
6055
throw ERROR_FACTORY.create(AppCheckError.RECAPTCHA_ERROR);
6156
});
6257
return exchangeToken(
63-
getExchangeRecaptchaTokenRequest(this._app, attestedClaimsToken),
64-
this._platformLoggerProvider
58+
getExchangeRecaptchaV3TokenRequest(this._app!, attestedClaimsToken),
59+
this._platformLoggerProvider!
6560
);
6661
}
6762

@@ -71,7 +66,7 @@ export class ReCaptchaV3Provider implements AppCheckProvider {
7166
initialize(app: FirebaseApp): void {
7267
this._app = app;
7368
this._platformLoggerProvider = _getProvider(app, 'platform-logger');
74-
initializeRecaptcha(app, this._siteKey).catch(() => {
69+
initializeRecaptchaV3(app, this._siteKey).catch(() => {
7570
/* we don't care about the initialization result */
7671
});
7772
}
@@ -108,21 +103,15 @@ export class ReCaptchaEnterpriseProvider implements AppCheckProvider {
108103
* @internal
109104
*/
110105
async getToken(): Promise<AppCheckTokenInternal> {
111-
if (!this._app || !this._platformLoggerProvider) {
112-
// This should only occur if user has not called initializeAppCheck().
113-
// We don't have an appName to provide if so.
114-
// This should already be caught in the top level `getToken()` function.
115-
throw ERROR_FACTORY.create(AppCheckError.USE_BEFORE_ACTIVATION, {
116-
appName: ''
117-
});
118-
}
119-
const attestedClaimsToken = await getReCAPTCHAToken(this._app).catch(_e => {
106+
// Top-level `getToken()` has already checked that App Check is initialized
107+
// and therefore this._app and this._platformLoggerProvider are available.
108+
const attestedClaimsToken = await getReCAPTCHAToken(this._app!).catch(_e => {
120109
// reCaptcha.execute() throws null which is not very descriptive.
121110
throw ERROR_FACTORY.create(AppCheckError.RECAPTCHA_ERROR);
122111
});
123112
return exchangeToken(
124-
getExchangeRecaptchaTokenRequest(this._app, attestedClaimsToken, true),
125-
this._platformLoggerProvider
113+
getExchangeRecaptchaEnterpriseTokenRequest(this._app!, attestedClaimsToken),
114+
this._platformLoggerProvider!
126115
);
127116
}
128117

@@ -132,7 +121,7 @@ export class ReCaptchaEnterpriseProvider implements AppCheckProvider {
132121
initialize(app: FirebaseApp): void {
133122
this._app = app;
134123
this._platformLoggerProvider = _getProvider(app, 'platform-logger');
135-
initializeRecaptcha(app, this._siteKey, true).catch(() => {
124+
initializeRecaptchaEnterprise(app, this._siteKey).catch(() => {
136125
/* we don't care about the initialization result */
137126
});
138127
}
@@ -162,14 +151,6 @@ export class CustomProvider implements AppCheckProvider {
162151
* @internal
163152
*/
164153
async getToken(): Promise<AppCheckTokenInternal> {
165-
if (!this._app) {
166-
// This should only occur if user has not called initializeAppCheck().
167-
// We don't have an appName to provide if so.
168-
// This should already be caught in the top level `getToken()` function.
169-
throw ERROR_FACTORY.create(AppCheckError.USE_BEFORE_ACTIVATION, {
170-
appName: ''
171-
});
172-
}
173154
// custom provider
174155
const customToken = await this._customProviderOptions.getToken();
175156
// Try to extract IAT from custom token, in case this token is not

‎packages/app-check/src/recaptcha.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
findgreCAPTCHAScriptsOnPage,
2727
FAKE_SITE_KEY
2828
} from '../test/util';
29-
import { initialize, getToken, GreCAPTCHATopLevel } from './recaptcha';
29+
import { initializeV3, initializeEnterprise, getToken, GreCAPTCHATopLevel } from './recaptcha';
3030
import * as utils from './util';
3131
import { getState } from './state';
3232
import { Deferred } from '@firebase/util';
@@ -49,7 +49,7 @@ describe('recaptcha', () => {
4949
it('sets reCAPTCHAState', async () => {
5050
self.grecaptcha = getFakeGreCAPTCHA() as GreCAPTCHATopLevel;
5151
expect(getState(app).reCAPTCHAState).to.equal(undefined);
52-
await initialize(app, FAKE_SITE_KEY);
52+
await initializeV3(app, FAKE_SITE_KEY);
5353
expect(getState(app).reCAPTCHAState?.initialized).to.be.instanceof(
5454
Deferred
5555
);
@@ -68,7 +68,7 @@ describe('recaptcha', () => {
6868
});
6969

7070
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0);
71-
await initialize(app, FAKE_SITE_KEY);
71+
await initializeV3(app, FAKE_SITE_KEY);
7272
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1);
7373
});
7474

@@ -77,7 +77,7 @@ describe('recaptcha', () => {
7777
const renderStub = stub(grecaptchaFake, 'render').callThrough();
7878
self.grecaptcha = grecaptchaFake as GreCAPTCHATopLevel;
7979

80-
await initialize(app, FAKE_SITE_KEY);
80+
await initializeV3(app, FAKE_SITE_KEY);
8181

8282
expect(renderStub).to.be.calledWith(`fire_app_check_${app.name}`, {
8383
sitekey: FAKE_SITE_KEY,
@@ -92,7 +92,7 @@ describe('recaptcha', () => {
9292
it('sets reCAPTCHAState', async () => {
9393
self.grecaptcha = getFakeGreCAPTCHA() as GreCAPTCHATopLevel;
9494
expect(getState(app).reCAPTCHAState).to.equal(undefined);
95-
await initialize(app, FAKE_SITE_KEY, true);
95+
await initializeEnterprise(app, FAKE_SITE_KEY);
9696
expect(getState(app).reCAPTCHAState?.initialized).to.be.instanceof(
9797
Deferred
9898
);
@@ -111,7 +111,7 @@ describe('recaptcha', () => {
111111
});
112112

113113
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(0);
114-
await initialize(app, FAKE_SITE_KEY, true);
114+
await initializeEnterprise(app, FAKE_SITE_KEY);
115115
expect(findgreCAPTCHAScriptsOnPage().length).to.equal(1);
116116
});
117117

@@ -123,7 +123,7 @@ describe('recaptcha', () => {
123123
).callThrough();
124124
self.grecaptcha = grecaptchaFake;
125125

126-
await initialize(app, FAKE_SITE_KEY, true);
126+
await initializeEnterprise(app, FAKE_SITE_KEY);
127127

128128
expect(renderStub).to.be.calledWith(`fire_app_check_${app.name}`, {
129129
sitekey: FAKE_SITE_KEY,

0 commit comments

Comments
 (0)