From 9fb78da03c28eb34fc634e794bd06f645e12143d Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 12 Apr 2023 16:14:28 -0400 Subject: [PATCH 1/9] feat(appcheck): Appcheck improvements --- .../app-check-api-client-internal.ts | 37 +++++++++++++++++++ src/app-check/app-check-api.ts | 21 +++++++++++ src/app-check/app-check.ts | 31 +++++++++++++++- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/app-check/app-check-api-client-internal.ts b/src/app-check/app-check-api-client-internal.ts index aae736cf63..be876f122b 100644 --- a/src/app-check/app-check-api-client-internal.ts +++ b/src/app-check/app-check-api-client-internal.ts @@ -27,6 +27,7 @@ import { AppCheckToken } from './app-check-api' // App Check backend constants const FIREBASE_APP_CHECK_V1_API_URL_FORMAT = 'https://firebaseappcheck.googleapis.com/v1/projects/{projectId}/apps/{appId}:exchangeCustomToken'; +const ONE_TIME_USE_TOKEN_VERIFICATION_URL_FORMAT = 'https://firebaseappcheck.googleapis.com/v1beta/projects/{projectId}:verifyAppCheckToken'; const FIREBASE_APP_CHECK_CONFIG_HEADERS = { 'X-Firebase-Client': `fire-admin-node/${utils.getSdkVersion()}` @@ -86,6 +87,31 @@ export class AppCheckApiClient { }); } + public verifyOneTimeProtection(token: string): Promise { + if (!validator.isNonEmptyString(token)) { + throw new FirebaseAppCheckError( + 'invalid-argument', + '`token` must be a non-empty string.'); + } + return this.getVerifyTokenUrl() + .then((url) => { + const request: HttpRequestConfig = { + method: 'POST', + url, + headers: FIREBASE_APP_CHECK_CONFIG_HEADERS, + data: { token } + }; + return this.httpClient.send(request); + }) + // eslint-disable-next-line @typescript-eslint/no-unused-vars + .then((resp) => { + return true; + }) + .catch((err) => { + throw this.toFirebaseError(err); + }); + } + private getUrl(appId: string): Promise { return this.getProjectId() .then((projectId) => { @@ -98,6 +124,17 @@ export class AppCheckApiClient { }); } + private getVerifyTokenUrl(): Promise { + return this.getProjectId() + .then((projectId) => { + const urlParams = { + projectId + }; + const baseUrl = utils.formatString(ONE_TIME_USE_TOKEN_VERIFICATION_URL_FORMAT, urlParams); + return utils.formatString(baseUrl); + }); + } + private getProjectId(): Promise { if (this.projectId) { return Promise.resolve(this.projectId); diff --git a/src/app-check/app-check-api.ts b/src/app-check/app-check-api.ts index ab959af04d..2369957257 100644 --- a/src/app-check/app-check-api.ts +++ b/src/app-check/app-check-api.ts @@ -41,6 +41,18 @@ export interface AppCheckTokenOptions { ttlMillis?: number; } +/** + * Interface representing options for {@link AppCheck.verifyToken} method. + */ +export interface VerifyAppCheckTokenOptions { + /** + * Sets the one-time use tokens feature. + * When set to `true`, checks if this token has already been consumed. + * This feature requires an additional network call to the backend and could be slower when enabled. + */ + consume?: boolean; +} + /** * Interface representing a decoded Firebase App Check token, returned from the * {@link AppCheck.verifyToken} method. @@ -86,6 +98,15 @@ export interface DecodedAppCheckToken { * convenience, and is set as the value of the {@link DecodedAppCheckToken.sub | sub} property. */ app_id: string; + + /** + * Indicates weather this token was already consumed. + * If this is the first time {@link AppCheck.verifyToken} method has seen this token, + * this field will contain the value `false`. The given token will then be + * marked as `already_consumed` for all future invocations of this {@link AppCheck.verifyToken} + * method for this token. + */ + already_consumed?: boolean; [key: string]: any; } diff --git a/src/app-check/app-check.ts b/src/app-check/app-check.ts index 0785fd6621..e367926948 100644 --- a/src/app-check/app-check.ts +++ b/src/app-check/app-check.ts @@ -15,8 +15,10 @@ * limitations under the License. */ +import * as validator from '../utils/validator'; + import { App } from '../app'; -import { AppCheckApiClient } from './app-check-api-client-internal'; +import { AppCheckApiClient, FirebaseAppCheckError } from './app-check-api-client-internal'; import { appCheckErrorFromCryptoSignerError, AppCheckTokenGenerator, } from './token-generator'; @@ -26,6 +28,7 @@ import { cryptoSignerFromApp } from '../utils/crypto-signer'; import { AppCheckToken, AppCheckTokenOptions, + VerifyAppCheckTokenOptions, VerifyAppCheckTokenResponse, } from './app-check-api'; @@ -75,17 +78,41 @@ export class AppCheck { * rejected. * * @param appCheckToken - The App Check token to verify. + * @param options - Optional {@link VerifyAppCheckTokenOptions} object when verifying an App Check Token. * * @returns A promise fulfilled with the token's decoded claims * if the App Check token is valid; otherwise, a rejected promise. */ - public verifyToken(appCheckToken: string): Promise { + public verifyToken(appCheckToken: string, options?: VerifyAppCheckTokenOptions) + : Promise { + this.validateVerifyAppCheckTokenOptions(options); return this.appCheckTokenVerifier.verifyToken(appCheckToken) .then((decodedToken) => { + if (options?.consume) { + return this.client.verifyOneTimeProtection(appCheckToken) + .then((alreadyConsumed) => { + decodedToken.already_consumed = alreadyConsumed; + return { + appId: decodedToken.app_id, + token: decodedToken, + }; + }); + } return { appId: decodedToken.app_id, token: decodedToken, }; }); } + + private validateVerifyAppCheckTokenOptions(options?: VerifyAppCheckTokenOptions): void { + if (typeof options === 'undefined') { + return; + } + if (!validator.isNonNullObject(options)) { + throw new FirebaseAppCheckError( + 'invalid-argument', + 'VerifyAppCheckTokenOptions must be a non-null object.'); + } + } } From a7013e5f46c8bb39f1a97f70e3a7391f91cb5d25 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 12 Apr 2023 16:27:41 -0400 Subject: [PATCH 2/9] fix api extractor --- etc/firebase-admin.api.md | 4 ++++ etc/firebase-admin.app-check.api.md | 8 +++++++- src/app-check/app-check-namespace.ts | 3 +++ src/app-check/index.ts | 1 + 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/etc/firebase-admin.api.md b/etc/firebase-admin.api.md index e8c27b99bd..d1eeb4e2db 100644 --- a/etc/firebase-admin.api.md +++ b/etc/firebase-admin.api.md @@ -62,6 +62,10 @@ export namespace appCheck { export type AppCheckTokenOptions = AppCheckTokenOptions; // Warning: (ae-forgotten-export) The symbol "DecodedAppCheckToken" needs to be exported by the entry point default-namespace.d.ts export type DecodedAppCheckToken = DecodedAppCheckToken; + // Warning: (ae-forgotten-export) The symbol "VerifyAppCheckTokenOptions" needs to be exported by the entry point default-namespace.d.ts + // + // (undocumented) + export type VerifyAppCheckTokenOptions = VerifyAppCheckTokenOptions; // Warning: (ae-forgotten-export) The symbol "VerifyAppCheckTokenResponse" needs to be exported by the entry point default-namespace.d.ts export type VerifyAppCheckTokenResponse = VerifyAppCheckTokenResponse; } diff --git a/etc/firebase-admin.app-check.api.md b/etc/firebase-admin.app-check.api.md index fb4e10ff64..5c2516210b 100644 --- a/etc/firebase-admin.app-check.api.md +++ b/etc/firebase-admin.app-check.api.md @@ -15,7 +15,7 @@ export class AppCheck { // (undocumented) readonly app: App; createToken(appId: string, options?: AppCheckTokenOptions): Promise; - verifyToken(appCheckToken: string): Promise; + verifyToken(appCheckToken: string, options?: VerifyAppCheckTokenOptions): Promise; } // @public @@ -33,6 +33,7 @@ export interface AppCheckTokenOptions { export interface DecodedAppCheckToken { // (undocumented) [key: string]: any; + already_consumed?: boolean; app_id: string; aud: string[]; exp: number; @@ -44,6 +45,11 @@ export interface DecodedAppCheckToken { // @public export function getAppCheck(app?: App): AppCheck; +// @public +export interface VerifyAppCheckTokenOptions { + consume?: boolean; +} + // @public export interface VerifyAppCheckTokenResponse { appId: string; diff --git a/src/app-check/app-check-namespace.ts b/src/app-check/app-check-namespace.ts index 070fb7a751..cdc4049b3d 100644 --- a/src/app-check/app-check-namespace.ts +++ b/src/app-check/app-check-namespace.ts @@ -19,6 +19,7 @@ import { AppCheckToken as TAppCheckToken, AppCheckTokenOptions as TAppCheckTokenOptions, DecodedAppCheckToken as TDecodedAppCheckToken, + VerifyAppCheckTokenOptions as TVerifyAppCheckTokenOptions, VerifyAppCheckTokenResponse as TVerifyAppCheckTokenResponse, } from './app-check-api'; import { AppCheck as TAppCheck } from './app-check'; @@ -74,4 +75,6 @@ export namespace appCheck { export type VerifyAppCheckTokenResponse = TVerifyAppCheckTokenResponse; export type AppCheckTokenOptions = TAppCheckTokenOptions; + + export type VerifyAppCheckTokenOptions = TVerifyAppCheckTokenOptions; } diff --git a/src/app-check/index.ts b/src/app-check/index.ts index 72f4d54b00..54cd9291d8 100644 --- a/src/app-check/index.ts +++ b/src/app-check/index.ts @@ -29,6 +29,7 @@ export { AppCheckToken, AppCheckTokenOptions, DecodedAppCheckToken, + VerifyAppCheckTokenOptions, VerifyAppCheckTokenResponse, } from './app-check-api'; export { AppCheck } from './app-check'; From 3de33f772a66b79c2df825b20c11e25e72d97a32 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 14 Apr 2023 16:56:22 -0400 Subject: [PATCH 3/9] Update docs and move already_consume outside --- etc/firebase-admin.api.md | 4 -- etc/firebase-admin.app-check.api.md | 2 +- .../app-check-api-client-internal.ts | 3 +- src/app-check/app-check-api.ts | 37 +++++++++++++------ src/app-check/app-check-namespace.ts | 6 +++ src/app-check/app-check.ts | 2 +- 6 files changed, 34 insertions(+), 20 deletions(-) diff --git a/etc/firebase-admin.api.md b/etc/firebase-admin.api.md index d1eeb4e2db..e49af96cc9 100644 --- a/etc/firebase-admin.api.md +++ b/etc/firebase-admin.api.md @@ -57,14 +57,10 @@ export namespace appCheck { // Warning: (ae-forgotten-export) The symbol "AppCheckToken" needs to be exported by the entry point default-namespace.d.ts export type AppCheckToken = AppCheckToken; // Warning: (ae-forgotten-export) The symbol "AppCheckTokenOptions" needs to be exported by the entry point default-namespace.d.ts - // - // (undocumented) export type AppCheckTokenOptions = AppCheckTokenOptions; // Warning: (ae-forgotten-export) The symbol "DecodedAppCheckToken" needs to be exported by the entry point default-namespace.d.ts export type DecodedAppCheckToken = DecodedAppCheckToken; // Warning: (ae-forgotten-export) The symbol "VerifyAppCheckTokenOptions" needs to be exported by the entry point default-namespace.d.ts - // - // (undocumented) export type VerifyAppCheckTokenOptions = VerifyAppCheckTokenOptions; // Warning: (ae-forgotten-export) The symbol "VerifyAppCheckTokenResponse" needs to be exported by the entry point default-namespace.d.ts export type VerifyAppCheckTokenResponse = VerifyAppCheckTokenResponse; diff --git a/etc/firebase-admin.app-check.api.md b/etc/firebase-admin.app-check.api.md index 5c2516210b..7c883d5f38 100644 --- a/etc/firebase-admin.app-check.api.md +++ b/etc/firebase-admin.app-check.api.md @@ -33,7 +33,6 @@ export interface AppCheckTokenOptions { export interface DecodedAppCheckToken { // (undocumented) [key: string]: any; - already_consumed?: boolean; app_id: string; aud: string[]; exp: number; @@ -52,6 +51,7 @@ export interface VerifyAppCheckTokenOptions { // @public export interface VerifyAppCheckTokenResponse { + alreadyConsumed?: boolean; appId: string; token: DecodedAppCheckToken; } diff --git a/src/app-check/app-check-api-client-internal.ts b/src/app-check/app-check-api-client-internal.ts index be876f122b..59d064c858 100644 --- a/src/app-check/app-check-api-client-internal.ts +++ b/src/app-check/app-check-api-client-internal.ts @@ -103,9 +103,8 @@ export class AppCheckApiClient { }; return this.httpClient.send(request); }) - // eslint-disable-next-line @typescript-eslint/no-unused-vars .then((resp) => { - return true; + return resp.data.already_consumed; }) .catch((err) => { throw this.toFirebaseError(err); diff --git a/src/app-check/app-check-api.ts b/src/app-check/app-check-api.ts index 2369957257..acf4515871 100644 --- a/src/app-check/app-check-api.ts +++ b/src/app-check/app-check-api.ts @@ -46,9 +46,18 @@ export interface AppCheckTokenOptions { */ export interface VerifyAppCheckTokenOptions { /** - * Sets the one-time use tokens feature. - * When set to `true`, checks if this token has already been consumed. - * This feature requires an additional network call to the backend and could be slower when enabled. + * To use the replay protection feature, set this to true to mark the token as consumed. + * Tokens that are found to be already consumed will be marked as such in the response. + * + * Tokens are only considered to be consumed if it is sent to App Check backend by calling the + * {@link AppCheck.verifyToken} method with this field set to `true`; other uses of the token + * do not consume it. + * + * This replay protection feature requires an additional network call to the App Check backend + * and forces your clients to obtain a fresh attestation from your chosen attestation providers. + * This can therefore negatively impact performance and can potentially deplete your attestation + * providers' quotas faster. We recommend that you use this feature only for protecting + * low volume, security critical, or expensive operations. */ consume?: boolean; } @@ -98,15 +107,6 @@ export interface DecodedAppCheckToken { * convenience, and is set as the value of the {@link DecodedAppCheckToken.sub | sub} property. */ app_id: string; - - /** - * Indicates weather this token was already consumed. - * If this is the first time {@link AppCheck.verifyToken} method has seen this token, - * this field will contain the value `false`. The given token will then be - * marked as `already_consumed` for all future invocations of this {@link AppCheck.verifyToken} - * method for this token. - */ - already_consumed?: boolean; [key: string]: any; } @@ -123,4 +123,17 @@ export interface VerifyAppCheckTokenResponse { * The decoded Firebase App Check token. */ token: DecodedAppCheckToken; + + /** + * Indicates weather this token was already consumed. + * If this is the first time {@link AppCheck.verifyToken} method has seen this token, + * this field will contain the value `false`. The given token will then be + * marked as `already_consumed` for all future invocations of this {@link AppCheck.verifyToken} + * method for this token. + * + * When this field is `true`, the caller is attempting to reuse a previously consumed token. + * You should take precautions against such a caller; for example, you can take actions such as + * rejecting the request or ask the caller to pass additional layers of security checks. + */ + alreadyConsumed?: boolean; } diff --git a/src/app-check/app-check-namespace.ts b/src/app-check/app-check-namespace.ts index cdc4049b3d..13e5beb3a7 100644 --- a/src/app-check/app-check-namespace.ts +++ b/src/app-check/app-check-namespace.ts @@ -74,7 +74,13 @@ export namespace appCheck { */ export type VerifyAppCheckTokenResponse = TVerifyAppCheckTokenResponse; + /** + * Type alias to {@link firebase-admin.app-check#AppCheckTokenOptions}. + */ export type AppCheckTokenOptions = TAppCheckTokenOptions; + /** + * Type alias to {@link firebase-admin.app-check#VerifyAppCheckTokenOptions}. + */ export type VerifyAppCheckTokenOptions = TVerifyAppCheckTokenOptions; } diff --git a/src/app-check/app-check.ts b/src/app-check/app-check.ts index e367926948..ef57054d0a 100644 --- a/src/app-check/app-check.ts +++ b/src/app-check/app-check.ts @@ -91,8 +91,8 @@ export class AppCheck { if (options?.consume) { return this.client.verifyOneTimeProtection(appCheckToken) .then((alreadyConsumed) => { - decodedToken.already_consumed = alreadyConsumed; return { + alreadyConsumed, appId: decodedToken.app_id, token: decodedToken, }; From 06f0ec3e611a735c2afd85cb1f66bf4d6789a9b5 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 14 Apr 2023 18:09:39 -0400 Subject: [PATCH 4/9] add unit tests --- .../app-check-api-client-internal.ts | 2 +- src/app-check/app-check.ts | 2 +- .../app-check-api-client-internal.spec.ts | 3 + test/unit/app-check/app-check.spec.ts | 137 ++++++++++++++++++ 4 files changed, 142 insertions(+), 2 deletions(-) diff --git a/src/app-check/app-check-api-client-internal.ts b/src/app-check/app-check-api-client-internal.ts index 59d064c858..dc23738cfe 100644 --- a/src/app-check/app-check-api-client-internal.ts +++ b/src/app-check/app-check-api-client-internal.ts @@ -87,7 +87,7 @@ export class AppCheckApiClient { }); } - public verifyOneTimeProtection(token: string): Promise { + public verifyReplayProtection(token: string): Promise { if (!validator.isNonEmptyString(token)) { throw new FirebaseAppCheckError( 'invalid-argument', diff --git a/src/app-check/app-check.ts b/src/app-check/app-check.ts index ef57054d0a..c81a04acf5 100644 --- a/src/app-check/app-check.ts +++ b/src/app-check/app-check.ts @@ -89,7 +89,7 @@ export class AppCheck { return this.appCheckTokenVerifier.verifyToken(appCheckToken) .then((decodedToken) => { if (options?.consume) { - return this.client.verifyOneTimeProtection(appCheckToken) + return this.client.verifyReplayProtection(appCheckToken) .then((alreadyConsumed) => { return { alreadyConsumed, diff --git a/test/unit/app-check/app-check-api-client-internal.spec.ts b/test/unit/app-check/app-check-api-client-internal.spec.ts index 65bc64d69d..518612e6ca 100644 --- a/test/unit/app-check/app-check-api-client-internal.spec.ts +++ b/test/unit/app-check/app-check-api-client-internal.spec.ts @@ -235,4 +235,7 @@ describe('AppCheckApiClient', () => { }); }); }); + + //describe verifyReplayProtection + }); diff --git a/test/unit/app-check/app-check.spec.ts b/test/unit/app-check/app-check.spec.ts index 61b7d81bf8..6f214d4f93 100644 --- a/test/unit/app-check/app-check.spec.ts +++ b/test/unit/app-check/app-check.spec.ts @@ -197,6 +197,143 @@ describe('AppCheck', () => { .then((tokenResponse) => { expect(tokenResponse.appId).equals('app-id'); expect(tokenResponse.token).equals(response); + expect(tokenResponse.alreadyConsumed).equals(undefined); + }); + }); + + it('should throw given an invalid options', () => { + [null, 100, -100, 'abc', [], true].forEach((invalidOptions) => { + expect(() => { + return appCheck.verifyToken('token', invalidOptions as any) + }).to.throw( + 'VerifyAppCheckTokenOptions must be a non-null object.'); + }); + }); + + it('should call verifyReplayProtection when consume is set to true', () => { + const response = { + sub: 'app-id', + iss: 'https://firebaseappcheck.googleapis.com/123456', + app_id: 'app-id', + aud: ['123456', 'project-id'], + exp: 1617741496, + iat: 1516239022, + }; + const verifierStub = sinon + .stub(AppCheckTokenVerifier.prototype, 'verifyToken') + .resolves(response); + const replayStub = sinon + .stub(AppCheckApiClient.prototype, 'verifyReplayProtection') + .resolves(true); + stubs.push(verifierStub); + stubs.push(replayStub); + return appCheck.verifyToken('token', { consume: true }) + .then((tokenResponse) => { + expect(tokenResponse.appId).equals('app-id'); + expect(tokenResponse.token).equals(response); + expect(tokenResponse.alreadyConsumed).equals(true); + + expect(verifierStub).to.have.been.calledOnce.and.calledWith('token'); + expect(replayStub).to.have.been.calledOnce.and.calledWith('token'); + }); + }); + + it('should not call verifyReplayProtection when consume is set to false', () => { + const response = { + sub: 'app-id', + iss: 'https://firebaseappcheck.googleapis.com/123456', + app_id: 'app-id', + aud: ['123456', 'project-id'], + exp: 1617741496, + iat: 1516239022, + }; + const verifierStub = sinon + .stub(AppCheckTokenVerifier.prototype, 'verifyToken') + .resolves(response); + const replayStub = sinon + .stub(AppCheckApiClient.prototype, 'verifyReplayProtection') + .resolves(true); + stubs.push(verifierStub); + stubs.push(replayStub); + return appCheck.verifyToken('token', { consume: false }) + .then((tokenResponse) => { + expect(tokenResponse.appId).equals('app-id'); + expect(tokenResponse.token).equals(response); + expect(tokenResponse.alreadyConsumed).equals(undefined); + + expect(verifierStub).to.have.been.calledOnce.and.calledWith('token'); + expect(replayStub).to.not.have.been.called; + }); + }); + + it('should not call verifyReplayProtection when consume is set to undefined', () => { + const response = { + sub: 'app-id', + iss: 'https://firebaseappcheck.googleapis.com/123456', + app_id: 'app-id', + aud: ['123456', 'project-id'], + exp: 1617741496, + iat: 1516239022, + }; + const verifierStub = sinon + .stub(AppCheckTokenVerifier.prototype, 'verifyToken') + .resolves(response); + const replayStub = sinon + .stub(AppCheckApiClient.prototype, 'verifyReplayProtection') + .resolves(true); + stubs.push(verifierStub); + stubs.push(replayStub); + return appCheck.verifyToken('token', { consume: undefined }) + .then((tokenResponse) => { + expect(tokenResponse.appId).equals('app-id'); + expect(tokenResponse.token).equals(response); + expect(tokenResponse.alreadyConsumed).equals(undefined); + + expect(verifierStub).to.have.been.calledOnce.and.calledWith('token'); + expect(replayStub).to.not.have.been.called; + }); + }); + + it('should not call verifyReplayProtection for an invalid token when consume is set to true', () => { + const verifierStub = sinon + .stub(AppCheckTokenVerifier.prototype, 'verifyToken') + .rejects(INTERNAL_ERROR); + const replayStub = sinon + .stub(AppCheckApiClient.prototype, 'verifyReplayProtection') + .resolves(true); + stubs.push(verifierStub); + stubs.push(replayStub); + appCheck.verifyToken('token', { consume: true }) + .should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR); + expect(verifierStub).to.have.been.calledOnce.and.calledWith('token'); + return expect(replayStub).to.not.have.been.called; + }); + + it('should resolve with VerifyAppCheckTokenResponse on success with already_consumed set', () => { + const response = { + sub: 'app-id', + iss: 'https://firebaseappcheck.googleapis.com/123456', + app_id: 'app-id', + aud: ['123456', 'project-id'], + exp: 1617741496, + iat: 1516239022, + }; + const verifierStub = sinon + .stub(AppCheckTokenVerifier.prototype, 'verifyToken') + .resolves(response); + const replayStub = sinon + .stub(AppCheckApiClient.prototype, 'verifyReplayProtection') + .resolves(false); + stubs.push(verifierStub); + stubs.push(replayStub); + return appCheck.verifyToken('token', { consume: true }) + .then((tokenResponse) => { + expect(tokenResponse.appId).equals('app-id'); + expect(tokenResponse.token).equals(response); + expect(tokenResponse.alreadyConsumed).equals(false); + + expect(verifierStub).to.have.been.calledOnce.and.calledWith('token'); + expect(replayStub).to.have.been.calledOnce.and.calledWith('token'); }); }); }); From 1b9465be28cf3b58c0d6afc6117c9b0da6400112 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 17 Apr 2023 14:58:54 -0400 Subject: [PATCH 5/9] Added unit tests for verifyReplayProtection --- .../app-check-api-client-internal.ts | 4 + .../app-check-api-client-internal.spec.ts | 115 +++++++++++++++++- 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/app-check/app-check-api-client-internal.ts b/src/app-check/app-check-api-client-internal.ts index dc23738cfe..b3fab814e9 100644 --- a/src/app-check/app-check-api-client-internal.ts +++ b/src/app-check/app-check-api-client-internal.ts @@ -104,6 +104,10 @@ export class AppCheckApiClient { return this.httpClient.send(request); }) .then((resp) => { + if (!validator.isBoolean(resp.data?.already_consumed)) { + throw new FirebaseAppCheckError( + 'invalid-argument', '`already_consumed` must be a boolean value.'); + } return resp.data.already_consumed; }) .catch((err) => { diff --git a/test/unit/app-check/app-check-api-client-internal.spec.ts b/test/unit/app-check/app-check-api-client-internal.spec.ts index 518612e6ca..54623e3a27 100644 --- a/test/unit/app-check/app-check-api-client-internal.spec.ts +++ b/test/unit/app-check/app-check-api-client-internal.spec.ts @@ -236,6 +236,119 @@ describe('AppCheckApiClient', () => { }); }); - //describe verifyReplayProtection + describe('verifyReplayProtection', () => { + it('should reject when project id is not available', () => { + return clientWithoutProjectId.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .should.eventually.be.rejectedWith(noProjectId); + }); + + it('should throw given no token', () => { + expect(() => { + (apiClient as any).verifyReplayProtection(undefined); + }).to.throw('`token` must be a non-empty string.'); + }); + + [null, NaN, 0, 1, true, false, [], {}, { a: 1 }, _.noop].forEach((invalidToken) => { + it('should throw given a non-string token: ' + JSON.stringify(invalidToken), () => { + expect(() => { + apiClient.verifyReplayProtection(invalidToken as any); + }).to.throw('`token` must be a non-empty string.'); + }); + }); + + it('should throw given an empty string token', () => { + expect(() => { + apiClient.verifyReplayProtection(''); + }).to.throw('`token` must be a non-empty string.'); + }); + + it('should reject when a full platform error response is received', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom(ERROR_RESPONSE, 404)); + stubs.push(stub); + const expected = new FirebaseAppCheckError('not-found', 'Requested entity not found'); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .should.eventually.be.rejected.and.deep.include(expected); + }); + + it('should reject with unknown-error when error code is not present', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom({}, 404)); + stubs.push(stub); + const expected = new FirebaseAppCheckError('unknown-error', 'Unknown server error: {}'); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .should.eventually.be.rejected.and.deep.include(expected); + }); + + it('should reject with unknown-error for non-json response', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom('not json', 404)); + stubs.push(stub); + const expected = new FirebaseAppCheckError( + 'unknown-error', 'Unexpected response with status: 404 and body: not json'); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .should.eventually.be.rejected.and.deep.include(expected); + }); + + it('should reject when rejected with a FirebaseAppError', () => { + const expected = new FirebaseAppError('network-error', 'socket hang up'); + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(expected); + stubs.push(stub); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .should.eventually.be.rejected.and.deep.include(expected); + }); + + ['', 'abc', '3s2', 'sssa', '3.000000001', '3.2', null, NaN, [], {}, 100, 1.2, -200, -2.4] + .forEach((invalidAlreadyConsumed) => { + it(`should throw if the returned already_consumed value is: ${invalidAlreadyConsumed}`, () => { + const response = { already_consumed: invalidAlreadyConsumed }; + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom(response, 200)); + stubs.push(stub); + const expected = new FirebaseAppCheckError( + 'invalid-argument', '`already_consumed` must be a boolean value.'); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .should.eventually.be.rejected.and.deep.include(expected); + }); + }); + + it('should resolve with the already_consumed status on success', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom({ already_consumed: true }, 200)); + stubs.push(stub); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .then((alreadyConsumed) => { + expect(alreadyConsumed).to.equal(true); + expect(stub).to.have.been.calledOnce.and.calledWith({ + method: 'POST', + url: 'https://firebaseappcheck.googleapis.com/v1beta/projects/test-project:verifyAppCheckToken', + headers: EXPECTED_HEADERS, + data: { token: TEST_TOKEN_TO_EXCHANGE } + }); + }); + }); + + [true, false].forEach((expectedAlreadyConsumed) => { + it(`should resolve with alreadyConsumed as ${expectedAlreadyConsumed} when already_consumed + from server is: ${expectedAlreadyConsumed}`, () => { + const response = { already_consumed: expectedAlreadyConsumed }; + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom(response, 200)); + stubs.push(stub); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .then((alreadyConsumed) => { + expect(alreadyConsumed).to.equal(expectedAlreadyConsumed); + }); + }); + }); + }); }); From 26fa4733845b52b46c0c7a563fa331e3b0a47e32 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 19 Apr 2023 16:02:22 -0400 Subject: [PATCH 6/9] fix docstrings --- src/app-check/app-check-api.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app-check/app-check-api.ts b/src/app-check/app-check-api.ts index acf4515871..de44a5a854 100644 --- a/src/app-check/app-check-api.ts +++ b/src/app-check/app-check-api.ts @@ -42,11 +42,13 @@ export interface AppCheckTokenOptions { } /** - * Interface representing options for {@link AppCheck.verifyToken} method. + * Interface representing options for the {@link AppCheck.verifyToken} method. */ export interface VerifyAppCheckTokenOptions { /** - * To use the replay protection feature, set this to true to mark the token as consumed. + * To use the replay protection feature, set this to `true`. The {@link AppCheck.verifyToken} + * method will mark the token as consumed after verifying it. + * * Tokens that are found to be already consumed will be marked as such in the response. * * Tokens are only considered to be consumed if it is sent to App Check backend by calling the From ebc64a73930923d06bace918a5c88dcca6b0bc0f Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 26 Apr 2023 17:04:42 -0400 Subject: [PATCH 7/9] cleanup unit tests --- .../app-check-api-client-internal.ts | 7 ++--- .../app-check-api-client-internal.spec.ts | 27 ++++++++++++++----- test/unit/app-check/app-check.spec.ts | 2 +- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/app-check/app-check-api-client-internal.ts b/src/app-check/app-check-api-client-internal.ts index b3fab814e9..baa3fd55cd 100644 --- a/src/app-check/app-check-api-client-internal.ts +++ b/src/app-check/app-check-api-client-internal.ts @@ -104,11 +104,12 @@ export class AppCheckApiClient { return this.httpClient.send(request); }) .then((resp) => { - if (!validator.isBoolean(resp.data?.already_consumed)) { + if (typeof resp.data.alreadyConsumed !== 'undefined' + && !validator.isBoolean(resp.data?.alreadyConsumed)) { throw new FirebaseAppCheckError( - 'invalid-argument', '`already_consumed` must be a boolean value.'); + 'invalid-argument', '`alreadyConsumed` must be a boolean value.'); } - return resp.data.already_consumed; + return resp.data.alreadyConsumed || false; }) .catch((err) => { throw this.toFirebaseError(err); diff --git a/test/unit/app-check/app-check-api-client-internal.spec.ts b/test/unit/app-check/app-check-api-client-internal.spec.ts index 54623e3a27..be461d0434 100644 --- a/test/unit/app-check/app-check-api-client-internal.spec.ts +++ b/test/unit/app-check/app-check-api-client-internal.spec.ts @@ -305,23 +305,23 @@ describe('AppCheckApiClient', () => { ['', 'abc', '3s2', 'sssa', '3.000000001', '3.2', null, NaN, [], {}, 100, 1.2, -200, -2.4] .forEach((invalidAlreadyConsumed) => { - it(`should throw if the returned already_consumed value is: ${invalidAlreadyConsumed}`, () => { - const response = { already_consumed: invalidAlreadyConsumed }; + it(`should throw if the returned alreadyConsumed value is: ${invalidAlreadyConsumed}`, () => { + const response = { alreadyConsumed: invalidAlreadyConsumed }; const stub = sinon .stub(HttpClient.prototype, 'send') .resolves(utils.responseFrom(response, 200)); stubs.push(stub); const expected = new FirebaseAppCheckError( - 'invalid-argument', '`already_consumed` must be a boolean value.'); + 'invalid-argument', '`alreadyConsumed` must be a boolean value.'); return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) .should.eventually.be.rejected.and.deep.include(expected); }); }); - it('should resolve with the already_consumed status on success', () => { + it('should resolve with the alreadyConsumed status on success', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom({ already_consumed: true }, 200)); + .resolves(utils.responseFrom({ alreadyConsumed: true }, 200)); stubs.push(stub); return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) .then((alreadyConsumed) => { @@ -336,9 +336,9 @@ describe('AppCheckApiClient', () => { }); [true, false].forEach((expectedAlreadyConsumed) => { - it(`should resolve with alreadyConsumed as ${expectedAlreadyConsumed} when already_consumed + it(`should resolve with alreadyConsumed as ${expectedAlreadyConsumed} when alreadyConsumed from server is: ${expectedAlreadyConsumed}`, () => { - const response = { already_consumed: expectedAlreadyConsumed }; + const response = { alreadyConsumed: expectedAlreadyConsumed }; const stub = sinon .stub(HttpClient.prototype, 'send') .resolves(utils.responseFrom(response, 200)); @@ -349,6 +349,19 @@ describe('AppCheckApiClient', () => { }); }); }); + + it(`should resolve with alreadyConsumed as false when alreadyConsumed + from server is: undefined`, () => { + const response = { }; + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom(response, 200)); + stubs.push(stub); + return apiClient.verifyReplayProtection(TEST_TOKEN_TO_EXCHANGE) + .then((alreadyConsumed) => { + expect(alreadyConsumed).to.equal(false); + }); + }); }); }); diff --git a/test/unit/app-check/app-check.spec.ts b/test/unit/app-check/app-check.spec.ts index 6f214d4f93..62b8eeac64 100644 --- a/test/unit/app-check/app-check.spec.ts +++ b/test/unit/app-check/app-check.spec.ts @@ -309,7 +309,7 @@ describe('AppCheck', () => { return expect(replayStub).to.not.have.been.called; }); - it('should resolve with VerifyAppCheckTokenResponse on success with already_consumed set', () => { + it('should resolve with VerifyAppCheckTokenResponse on success with alreadyConsumed set', () => { const response = { sub: 'app-id', iss: 'https://firebaseappcheck.googleapis.com/123456', From 56a0b9874b04c26484da84314590aee2230368f6 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 1 May 2023 15:38:33 -0400 Subject: [PATCH 8/9] update json payload --- src/app-check/app-check-api-client-internal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app-check/app-check-api-client-internal.ts b/src/app-check/app-check-api-client-internal.ts index baa3fd55cd..6e99bc2c64 100644 --- a/src/app-check/app-check-api-client-internal.ts +++ b/src/app-check/app-check-api-client-internal.ts @@ -99,7 +99,7 @@ export class AppCheckApiClient { method: 'POST', url, headers: FIREBASE_APP_CHECK_CONFIG_HEADERS, - data: { token } + data: { app_check_token: token } }; return this.httpClient.send(request); }) From b893a3128a9efd58e178f2d75db8bd7642fe214e Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 1 May 2023 15:47:19 -0400 Subject: [PATCH 9/9] fix tests --- test/unit/app-check/app-check-api-client-internal.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/app-check/app-check-api-client-internal.spec.ts b/test/unit/app-check/app-check-api-client-internal.spec.ts index be461d0434..bd048bd9e4 100644 --- a/test/unit/app-check/app-check-api-client-internal.spec.ts +++ b/test/unit/app-check/app-check-api-client-internal.spec.ts @@ -330,7 +330,7 @@ describe('AppCheckApiClient', () => { method: 'POST', url: 'https://firebaseappcheck.googleapis.com/v1beta/projects/test-project:verifyAppCheckToken', headers: EXPECTED_HEADERS, - data: { token: TEST_TOKEN_TO_EXCHANGE } + data: { app_check_token: TEST_TOKEN_TO_EXCHANGE } }); }); });