Skip to content

Move to enforceAppCheck option #1145

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

Merged
merged 1 commit into from
Jun 24, 2022
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
66 changes: 64 additions & 2 deletions spec/common/providers/https.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,10 @@ describe('onCallHandler', () => {
const appCheckToken = generateUnsignedAppCheckToken(projectId, appId);
await runCallableTest({
httpRequest: mockRequest(null, 'application/json', { appCheckToken }),
callableOption: {
cors: { origin: true, methods: 'POST' },
enforceAppCheck: true,
},
expectedData: null,
callableFunction: (data, context) => {
return;
Expand All @@ -390,7 +394,7 @@ describe('onCallHandler', () => {
});
});

it('should handle bad AppCheck token with callable option', async () => {
it('should handle bad AppCheck token with enforcement disabled', async () => {
await runCallableTest({
httpRequest: mockRequest(null, 'application/json', {
appCheckToken: 'FAKE',
Expand All @@ -404,7 +408,7 @@ describe('onCallHandler', () => {
},
callableOption: {
cors: { origin: true, methods: 'POST' },
allowInvalidAppCheckToken: true,
enforceAppCheck: false,
},
expectedHttpResponse: {
status: 200,
Expand All @@ -414,6 +418,64 @@ describe('onCallHandler', () => {
});
});

it('should handle bad AppCheck token with enforcement enabled', async () => {
await runCallableTest({
httpRequest: mockRequest(null, 'application/json', {
appCheckToken: 'FAKE',
}),
expectedData: null,
callableFunction: (data, context) => {
return;
},
callableFunction2: (request) => {
return;
},
callableOption: {
cors: { origin: true, methods: 'POST' },
enforceAppCheck: true,
},
expectedHttpResponse: {
status: 401,
headers: expectedResponseHeaders,
body: {
error: {
message: 'Unauthenticated',
status: 'UNAUTHENTICATED',
},
},
},
});
});

it('should handle no AppCheck token with enforcement enabled', async () => {
await runCallableTest({
httpRequest: mockRequest(null, 'application/json', {
appCheckToken: 'MISSING',
}),
expectedData: null,
callableFunction: (data, context) => {
return;
},
callableFunction2: (request) => {
return;
},
callableOption: {
cors: { origin: true, methods: 'POST' },
enforceAppCheck: true,
},
expectedHttpResponse: {
status: 401,
headers: expectedResponseHeaders,
body: {
error: {
message: 'Unauthenticated',
status: 'UNAUTHENTICATED',
},
},
},
});
});

it('should handle instance id', async () => {
await runCallableTest({
httpRequest: mockRequest(null, 'application/json', {
Expand Down
13 changes: 11 additions & 2 deletions src/common/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ type v2CallableHandler<Req, Res> = (request: CallableRequest<Req>) => Res;
/** @internal **/
export interface CallableOptions {
cors: cors.CorsOptions;
allowInvalidAppCheckToken?: boolean;
enforceAppCheck?: boolean;
}

/** @internal */
Expand Down Expand Up @@ -679,7 +679,16 @@ function wrapOnCallHandler<Req = any, Res = any>(
if (tokenStatus.auth === 'INVALID') {
throw new HttpsError('unauthenticated', 'Unauthenticated');
}
if (tokenStatus.app === 'INVALID' && !options.allowInvalidAppCheckToken) {
if (tokenStatus.app === 'INVALID') {
if (options.enforceAppCheck) {
throw new HttpsError('unauthenticated', 'Unauthenticated');
} else {
logger.warn(
'Allowing request with invalid AppCheck token because enforcement is disabled'
);
}
}
if (tokenStatus.app === 'MISSING' && options.enforceAppCheck) {
throw new HttpsError('unauthenticated', 'Unauthenticated');
}

Expand Down
7 changes: 7 additions & 0 deletions src/function-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,13 @@ function assertRuntimeOptionsValid(runtimeOptions: RuntimeOptions): boolean {
}
}

if ('allowInvalidAppCheckToken' in runtimeOptions) {
throw new Error(
'runWith option "allowInvalidAppCheckToken" has been inverted and ' +
'renamed "enforceAppCheck"'
);
}

return true;
}

Expand Down
13 changes: 8 additions & 5 deletions src/function-configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,18 @@ export interface RuntimeOptions {
*/
invoker?: 'public' | 'private' | string | string[];

/*
* Allow requests with invalid App Check tokens on callable functions.
*/
allowInvalidAppCheckToken?: boolean;

/*
* Secrets to bind to a function instance.
*/
secrets?: string[];

/**
* Determines whether Firebase AppCheck is enforced.
* When true, requests with invalid tokens autorespond with a 401
* (Unauthorized) error.
* When false, requests with invalid tokens set context.app to undefiend.
*/
enforceAppCheck?: boolean;
}

export interface DeploymentOptions extends RuntimeOptions {
Expand Down
2 changes: 1 addition & 1 deletion src/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export function _onCallWithOptions(
handler(data, context);
const func: any = onCallHandler(
{
allowInvalidAppCheckToken: options.allowInvalidAppCheckToken,
enforceAppCheck: options.enforceAppCheck,
cors: { origin: true, methods: 'POST' },
},
fixedLen
Expand Down
11 changes: 10 additions & 1 deletion src/v2/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,14 @@ export interface GlobalOptions {
* Secrets to bind to a function.
*/
secrets?: string[];

/**
* Determines whether Firebase AppCheck is enforced.
* When true, requests with invalid tokens autorespond with a 401
* (Unauthorized) error.
* When false, requests with invalid tokens set event.app to undefiend.
*/
enforceAppCheck?: boolean;
}

let globalOptions: GlobalOptions | undefined;
Expand Down Expand Up @@ -216,7 +224,8 @@ export function getGlobalOptions(): GlobalOptions {
/**
* Additional fields that can be set on any event-handling Cloud Function.
*/
export interface EventHandlerOptions extends GlobalOptions {
export interface EventHandlerOptions
extends Omit<GlobalOptions, 'enforceAppCheck'> {
/** Whether failed executions should be delivered again. */
retry?: boolean;
}
Expand Down
29 changes: 23 additions & 6 deletions src/v2/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import { GlobalOptions, SupportedRegion } from '../options';
export { Request, CallableRequest, FunctionsErrorCode, HttpsError };

/**
* Options that can be set on an individual HTTPS function.
* Options that can be set on an onRequest HTTPS function.
*/
export interface HttpsOptions extends Omit<GlobalOptions, 'region'> {
/** HTTP functions can override global options and can specify multiple regions to deploy to. */
Expand Down Expand Up @@ -150,6 +150,19 @@ export interface HttpsOptions extends Omit<GlobalOptions, 'region'> {
retry?: boolean;
}

/**
* Options that can be set on a callable HTTPS function.
*/
export interface CallableOptions extends HttpsOptions {
/**
* Determines whether Firebase AppCheck is enforced.
* When true, requests with invalid tokens autorespond with a 401
* (Unauthorized) error.
* When false, requests with invalid tokens set event.app to undefiend.
*/
enforceAppCheck?: boolean;
}

/**
* Handles HTTPS requests.
*/
Expand Down Expand Up @@ -298,7 +311,7 @@ export function onRequest(
* @returns A function that you can export and deploy.
*/
export function onCall<T = any, Return = any | Promise<any>>(
opts: HttpsOptions,
opts: CallableOptions,
handler: (request: CallableRequest<T>) => Return
): CallableFunction<T, Return>;
/**
Expand All @@ -310,15 +323,15 @@ export function onCall<T = any, Return = any | Promise<any>>(
handler: (request: CallableRequest<T>) => Return
): CallableFunction<T, Return>;
export function onCall<T = any, Return = any | Promise<any>>(
optsOrHandler: HttpsOptions | ((request: CallableRequest<T>) => Return),
optsOrHandler: CallableOptions | ((request: CallableRequest<T>) => Return),
handler?: (request: CallableRequest<T>) => Return
): CallableFunction<T, Return> {
let opts: HttpsOptions;
let opts: CallableOptions;
if (arguments.length == 1) {
opts = {};
handler = optsOrHandler as (request: CallableRequest<T>) => Return;
} else {
opts = optsOrHandler as HttpsOptions;
opts = optsOrHandler as CallableOptions;
}

const origin = isDebugFeatureEnabled('enableCors')
Expand All @@ -331,7 +344,11 @@ export function onCall<T = any, Return = any | Promise<any>>(
// fix the length to prevent api versions from being mismatched.
const fixedLen = (req: CallableRequest<T>) => handler(req);
const func: any = onCallHandler(
{ cors: { origin, methods: 'POST' } },
{
cors: { origin, methods: 'POST' },
enforceAppCheck:
opts.enforceAppCheck ?? options.getGlobalOptions().enforceAppCheck,
},
fixedLen
);

Expand Down