From 37b42370f1c83f35227c3616d2959f608e490756 Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Mon, 26 Jul 2021 15:44:49 -0700 Subject: [PATCH 1/7] add invoker option to RunWith --- src/v1/function-configuration.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/v1/function-configuration.ts b/src/v1/function-configuration.ts index b1ed51623..76d3665b1 100644 --- a/src/v1/function-configuration.ts +++ b/src/v1/function-configuration.ts @@ -98,6 +98,11 @@ export const DEFAULT_FAILURE_POLICY: FailurePolicy = { export const MAX_NUMBER_USER_LABELS = 58; +/** + * Invoker access control type for https functions. + */ +export type Invoker = "public" | "private" | string; + export interface RuntimeOptions { /** * Which platform should host the backend. Valid options are "gcfv1" @@ -156,6 +161,11 @@ export interface RuntimeOptions { * User labels to set on the function. */ labels?: Record; + + /** + * Invoker to set access control on https functions. + */ + invoker?: Invoker | Invoker[]; } export interface DeploymentOptions extends RuntimeOptions { From 46efa887e2a7607b185329cdacdba8e61957c6d2 Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Mon, 26 Jul 2021 15:58:13 -0700 Subject: [PATCH 2/7] fixing lint issue --- src/v1/function-configuration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/v1/function-configuration.ts b/src/v1/function-configuration.ts index 76d3665b1..f7acef18e 100644 --- a/src/v1/function-configuration.ts +++ b/src/v1/function-configuration.ts @@ -101,7 +101,7 @@ export const MAX_NUMBER_USER_LABELS = 58; /** * Invoker access control type for https functions. */ -export type Invoker = "public" | "private" | string; +export type Invoker = 'public' | 'private' | string; export interface RuntimeOptions { /** From 5afe165c27c292541bd2c49d5f1dcba77bcebaab Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Tue, 27 Jul 2021 16:24:52 -0700 Subject: [PATCH 3/7] adding invoker checking logic --- src/v1/function-builder.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/v1/function-builder.ts b/src/v1/function-builder.ts index b421668d4..34546d119 100644 --- a/src/v1/function-builder.ts +++ b/src/v1/function-builder.ts @@ -44,6 +44,10 @@ import * as remoteConfig from './providers/remoteConfig'; import * as storage from './providers/storage'; import * as testLab from './providers/testLab'; +function validServiceAccount(serviceAccount: string): boolean { + return true; +} + /** * Assert that the runtime options passed in are valid. * @param runtimeOptions object containing memory and timeout information. @@ -192,6 +196,28 @@ function assertRuntimeOptionsValid(runtimeOptions: RuntimeOptions): boolean { ); } } + + if (runtimeOptions.invoker) { + if ( + typeof runtimeOptions.invoker === 'string' && + runtimeOptions.invoker !== 'public' && + runtimeOptions.invoker !== 'private' && + !validServiceAccount(runtimeOptions.invoker) + ) { + throw new Error( + `Invalid invoker service account, must be of the form '{serviceAccountName}@'` + ); + } else if (Array.isArray(runtimeOptions.invoker)) { + for (const serviceAccount of runtimeOptions.invoker) { + if (!validServiceAccount(serviceAccount)) { + throw new Error( + `Invalid invoker service account, must be of the form '{serviceAccountName}@'` + ); + } + } + } + } + return true; } From 85c5969f199cd24ba3b8f14e169886053c99c68b Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Wed, 28 Jul 2021 10:23:48 -0700 Subject: [PATCH 4/7] adding tests --- spec/v1/function-builder.spec.ts | 24 +++++++++++++++++++ src/v1/function-builder.ts | 41 ++++++++++++++++---------------- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/spec/v1/function-builder.spec.ts b/spec/v1/function-builder.spec.ts index bfb9e99fc..541d3a8fa 100644 --- a/spec/v1/function-builder.spec.ts +++ b/spec/v1/function-builder.spec.ts @@ -439,4 +439,28 @@ describe('FunctionBuilder', () => { }) ).to.throw(); }); + + it('should throw an error if invoker is an empty string', () => { + expect(() => + functions.runWith({ + invoker: '', + }) + ).to.throw(); + }); + + it('should throw an error if invoker is an empty array', () => { + expect(() => + functions.runWith({ + invoker: [''], + }) + ).to.throw(); + }); + + it('should throw an error if invoker has an empty string', () => { + expect(() => + functions.runWith({ + invoker: ['service-account1', '', 'service-account2'], + }) + ).to.throw(); + }); }); diff --git a/src/v1/function-builder.ts b/src/v1/function-builder.ts index 34546d119..ee5470d44 100644 --- a/src/v1/function-builder.ts +++ b/src/v1/function-builder.ts @@ -44,10 +44,6 @@ import * as remoteConfig from './providers/remoteConfig'; import * as storage from './providers/storage'; import * as testLab from './providers/testLab'; -function validServiceAccount(serviceAccount: string): boolean { - return true; -} - /** * Assert that the runtime options passed in are valid. * @param runtimeOptions object containing memory and timeout information. @@ -197,24 +193,29 @@ function assertRuntimeOptionsValid(runtimeOptions: RuntimeOptions): boolean { } } - if (runtimeOptions.invoker) { - if ( - typeof runtimeOptions.invoker === 'string' && - runtimeOptions.invoker !== 'public' && - runtimeOptions.invoker !== 'private' && - !validServiceAccount(runtimeOptions.invoker) - ) { + if ( + runtimeOptions.invoker !== undefined && + typeof runtimeOptions.invoker === 'string' && + runtimeOptions.invoker.length == 0 + ) { + throw new Error( + 'Invalid service account for function invoker, must be a non-empty string' + ); + } + if ( + runtimeOptions.invoker !== undefined && + Array.isArray(runtimeOptions.invoker) + ) { + if (runtimeOptions.invoker.length == 0) { throw new Error( - `Invalid invoker service account, must be of the form '{serviceAccountName}@'` + 'Invalid invoker array, must contain at least 1 service account entry' ); - } else if (Array.isArray(runtimeOptions.invoker)) { - for (const serviceAccount of runtimeOptions.invoker) { - if (!validServiceAccount(serviceAccount)) { - throw new Error( - `Invalid invoker service account, must be of the form '{serviceAccountName}@'` - ); - } - } + } + for (const serviceAccount of runtimeOptions.invoker) { + if (serviceAccount.length == 0) + throw new Error( + 'Invalid invoker array, a service account must be a non-empty string' + ); } } From ac2ec0056a01bcfc0cb4e29c8852f5dd6e994892 Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Wed, 28 Jul 2021 16:20:36 -0700 Subject: [PATCH 5/7] find invoker in the trigger options during cli deployment --- src/v1/cloud-functions.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/v1/cloud-functions.ts b/src/v1/cloud-functions.ts index ef7b8debe..8fe6be318 100644 --- a/src/v1/cloud-functions.ts +++ b/src/v1/cloud-functions.ts @@ -28,6 +28,7 @@ import { DeploymentOptions, FailurePolicy, Schedule, + Invoker, } from './function-configuration'; export { Request, Response }; import { @@ -278,6 +279,7 @@ export interface TriggerAnnotated { vpcConnectorEgressSettings?: string; serviceAccountEmail?: string; ingressSettings?: string; + invoker?: Invoker | Invoker[]; }; } @@ -497,7 +499,8 @@ export function optionsToTrigger(options: DeploymentOptions) { 'ingressSettings', 'vpcConnectorEgressSettings', 'vpcConnector', - 'labels' + 'labels', + 'invoker', ); convertIfPresent( trigger, From 9f6fe55b63fa65d4a98e45e2de0a885afedb02aa Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Wed, 28 Jul 2021 16:23:51 -0700 Subject: [PATCH 6/7] fix lint/format --- src/v1/cloud-functions.ts | 4 ++-- src/v1/function-builder.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/v1/cloud-functions.ts b/src/v1/cloud-functions.ts index 8fe6be318..fc8f1406a 100644 --- a/src/v1/cloud-functions.ts +++ b/src/v1/cloud-functions.ts @@ -27,8 +27,8 @@ import { DEFAULT_FAILURE_POLICY, DeploymentOptions, FailurePolicy, - Schedule, Invoker, + Schedule, } from './function-configuration'; export { Request, Response }; import { @@ -500,7 +500,7 @@ export function optionsToTrigger(options: DeploymentOptions) { 'vpcConnectorEgressSettings', 'vpcConnector', 'labels', - 'invoker', + 'invoker' ); convertIfPresent( trigger, diff --git a/src/v1/function-builder.ts b/src/v1/function-builder.ts index ee5470d44..da7548538 100644 --- a/src/v1/function-builder.ts +++ b/src/v1/function-builder.ts @@ -212,10 +212,11 @@ function assertRuntimeOptionsValid(runtimeOptions: RuntimeOptions): boolean { ); } for (const serviceAccount of runtimeOptions.invoker) { - if (serviceAccount.length == 0) + if (serviceAccount.length == 0) { throw new Error( 'Invalid invoker array, a service account must be a non-empty string' ); + } } } From f09abc55760ef891e882c4070ea0e60811f01e9e Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Fri, 30 Jul 2021 13:45:10 -0400 Subject: [PATCH 7/7] add checks for public/private in invoker arrray --- spec/v1/function-builder.spec.ts | 16 ++++++++++++++++ src/v1/function-builder.ts | 17 +++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/spec/v1/function-builder.spec.ts b/spec/v1/function-builder.spec.ts index 541d3a8fa..86a441c67 100644 --- a/spec/v1/function-builder.spec.ts +++ b/spec/v1/function-builder.spec.ts @@ -463,4 +463,20 @@ describe('FunctionBuilder', () => { }) ).to.throw(); }); + + it('should throw an error if public identifier is in the invoker array', () => { + expect(() => + functions.runWith({ + invoker: ['service-account1', 'public', 'service-account2'], + }) + ).to.throw(); + }); + + it('', () => { + expect(() => + functions.runWith({ + invoker: ['service-account1', 'private', 'service-account2'], + }) + ).to.throw(); + }); }); diff --git a/src/v1/function-builder.ts b/src/v1/function-builder.ts index da7548538..ada3b724c 100644 --- a/src/v1/function-builder.ts +++ b/src/v1/function-builder.ts @@ -194,9 +194,8 @@ function assertRuntimeOptionsValid(runtimeOptions: RuntimeOptions): boolean { } if ( - runtimeOptions.invoker !== undefined && typeof runtimeOptions.invoker === 'string' && - runtimeOptions.invoker.length == 0 + runtimeOptions.invoker.length === 0 ) { throw new Error( 'Invalid service account for function invoker, must be a non-empty string' @@ -206,17 +205,27 @@ function assertRuntimeOptionsValid(runtimeOptions: RuntimeOptions): boolean { runtimeOptions.invoker !== undefined && Array.isArray(runtimeOptions.invoker) ) { - if (runtimeOptions.invoker.length == 0) { + if (runtimeOptions.invoker.length === 0) { throw new Error( 'Invalid invoker array, must contain at least 1 service account entry' ); } for (const serviceAccount of runtimeOptions.invoker) { - if (serviceAccount.length == 0) { + if (serviceAccount.length === 0) { throw new Error( 'Invalid invoker array, a service account must be a non-empty string' ); } + if (serviceAccount === 'public') { + throw new Error( + "Invalid invoker array, a service account cannot be set to the 'public' identifier" + ); + } + if (serviceAccount === 'private') { + throw new Error( + "Invalid invoker array, a service account cannot be set to the 'private' identifier" + ); + } } }