From 9d55f08803cad8ab7684646ef03bcfbe8453a0c2 Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore <83682223+sachscode@users.noreply.github.com> Date: Wed, 20 Oct 2021 08:56:26 -0700 Subject: [PATCH] (bug-fix/reconcile-headless) remove groupList from userInputs, fix single-group, deploy errors (#8501) Co-authored-by: Sachin Panemangalore --- .../schemas/S3/S3UserInputs.schema.json | 15 ++----- .../s3-walkthrough.test.ts | 4 -- .../cdk-stack-builder/s3-stack-builder.ts | 17 +++++--- .../default-values/s3-defaults.ts | 3 +- .../s3-user-input-types.ts | 6 +-- .../service-walkthroughs/s3-auth-api.ts | 9 ---- .../service-walkthroughs/s3-questions.ts | 18 ++++---- .../s3-user-input-state.ts | 41 ------------------- 8 files changed, 27 insertions(+), 86 deletions(-) diff --git a/packages/amplify-category-storage/resources/schemas/S3/S3UserInputs.schema.json b/packages/amplify-category-storage/resources/schemas/S3/S3UserInputs.schema.json index f05465fa221..93043c36951 100644 --- a/packages/amplify-category-storage/resources/schemas/S3/S3UserInputs.schema.json +++ b/packages/amplify-category-storage/resources/schemas/S3/S3UserInputs.schema.json @@ -43,28 +43,19 @@ "type": "string" }, "groupAccess": { - "$ref": "#/definitions/Record" - }, - "groupList": { - "type": "array", - "items": { - "type": "string" - } + "$ref": "#/definitions/GroupAccessType" } }, "required": [ "authAccess", "bucketName", - "groupAccess", - "groupList", "guestAccess", "policyUUID", "resourceName", - "storageAccess", - "triggerFunction" + "storageAccess" ], "definitions": { - "Record": { + "GroupAccessType": { "type": "object" } }, diff --git a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts index f58840a1307..fa1cf9031a9 100644 --- a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts +++ b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts @@ -848,7 +848,6 @@ class S3MockDataBuilder { guestAccess: [], authAccess: this.defaultAuthPerms, groupAccess: {}, - groupList: [], triggerFunction: 'NONE', }; cliInputs: S3UserInputs = { @@ -860,7 +859,6 @@ class S3MockDataBuilder { authAccess: [], triggerFunction: undefined, groupAccess: undefined, - groupList: undefined, }; constructor(startCliInputState: S3UserInputs | undefined) { @@ -937,13 +935,11 @@ class S3MockDataBuilder { addGroupAccess(): S3MockDataBuilder { this.cliInputs.groupAccess = this.mockGroupAccess; - this.cliInputs.groupList = Object.keys(this.mockGroupAccess); return this; } removeGroupAccess(): S3MockDataBuilder { this.cliInputs.groupAccess = undefined; - this.cliInputs.groupList = undefined; return this; } diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-builder.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-builder.ts index d6ba809653e..a130efd6da5 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-builder.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-builder.ts @@ -53,6 +53,11 @@ export class AmplifyS3ResourceCfnStack extends AmplifyResourceCfnStack implement this.s3DependsOnResources = []; } + private getGroupListFromProps(): Array|undefined { + const groupList = (this._props.groupAccess)? Object.keys(this._props.groupAccess) : []; + return groupList; + } + public getS3ResourceFriendlyName(): string { return this.id; } @@ -90,12 +95,13 @@ export class AmplifyS3ResourceCfnStack extends AmplifyResourceCfnStack implement this.createAndSetIAMPolicies(); //4. Configure Cognito User pool policies - if (this._props.groupList && this._props.groupList.length > 0) { + const groupList = this.getGroupListFromProps(); + if (groupList && groupList.length > 0) { const authResourceName: string = await s3AuthAPI.getAuthResourceARN(context); this.authResourceName = authResourceName; this.s3GroupPolicyList = this.createS3AmplifyGroupPolicies( authResourceName, - this._props.groupList as Array, + groupList, this._props.groupAccess as GroupAccessType, ); @@ -108,7 +114,7 @@ export class AmplifyS3ResourceCfnStack extends AmplifyResourceCfnStack implement //1. UserPoolID this.s3DependsOnResources.push(this.buildS3DependsOnUserPoolIdCfn(authResourceName as string)); //2. User Pool Group List - const userPoollGroupList = this.buildS3DependsOnUserPoolGroupRoleListCfn(this._props.groupList); + const userPoollGroupList = this.buildS3DependsOnUserPoolGroupRoleListCfn(groupList); this.s3DependsOnResources = this.s3DependsOnResources.concat(userPoollGroupList); } @@ -119,7 +125,8 @@ export class AmplifyS3ResourceCfnStack extends AmplifyResourceCfnStack implement } public addGroupParams(authResourceName: string): AmplifyS3CfnParameters | undefined { - if (this._props.groupList) { + const groupList = this.getGroupListFromProps(); + if (groupList && groupList.length > 0 ) { let s3CfnParams: Array = [ { params: [`auth${authResourceName}UserPoolId`], @@ -128,7 +135,7 @@ export class AmplifyS3ResourceCfnStack extends AmplifyResourceCfnStack implement }, ]; - for (const groupName of this._props.groupList) { + for (const groupName of groupList) { s3CfnParams.push({ params: [`authuserPoolGroups${this.buildGroupRoleName(groupName)}`], paramType: 'String', diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/default-values/s3-defaults.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/default-values/s3-defaults.ts index 27975396884..9038ade622c 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/default-values/s3-defaults.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/default-values/s3-defaults.ts @@ -13,8 +13,7 @@ export const getAllDefaults = (project: $TSAny, shortId : string) : S3UserInputs guestAccess: [], authAccess: [], triggerFunction : undefined, - groupAccess : {}, - groupList : [] + groupAccess : {} }; return defaults; diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthrough-types/s3-user-input-types.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthrough-types/s3-user-input-types.ts index f281806228e..fdc2cbca32c 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthrough-types/s3-user-input-types.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthrough-types/s3-user-input-types.ts @@ -46,9 +46,8 @@ export interface S3UserInputs { storageAccess : S3AccessType|undefined, guestAccess: S3PermissionType[], authAccess : S3PermissionType[], - triggerFunction: string|undefined, - groupAccess : GroupAccessType|undefined, //{ "admingroup": [create, read, delete, list], "secondgroup" :[...''...] } - groupList : Array|undefined //Keys of group policy Map + triggerFunction?: string|undefined, + groupAccess? : GroupAccessType|undefined, //{ "admingroup": [create, read, delete, list], "secondgroup" :[...''...] } } export function defaultS3UserInputs() :S3UserInputs { @@ -61,7 +60,6 @@ export function defaultS3UserInputs() :S3UserInputs { authAccess : [], triggerFunction:undefined, groupAccess: undefined, - groupList: undefined }; return defaultS3UserInputValues; } diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-auth-api.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-auth-api.ts index b3ff5cbf7db..ca3af603323 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-auth-api.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-auth-api.ts @@ -4,15 +4,6 @@ import { S3InputState } from "./s3-user-input-state"; /* This file contains all functions interacting with AUTH category */ -//DOWNSTREAM API: function to be called from Auth or Auth event handler -export async function saveUserPoolGroupsInUserInput( resourceName : string, userPoolGroups : Array ): Promise{ - const cliInputsState = new S3InputState(resourceName, undefined); - cliInputsState.setPoolGroupList(userPoolGroups); - const userInput : S3UserInputs = cliInputsState.getUserInput(); - cliInputsState.saveCliInputPayload(userInput); - return userInput; -} - //UPSTREAM API: function to be called from Storage to fetch or update Auth resources export async function getAuthResourceARN( context : $TSContext ) : Promise { let authResources = (await context.amplify.getResourceStatus('auth')).allResources; diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts index a466deb2a0b..ef917762453 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts @@ -142,10 +142,15 @@ export async function askUserPoolGroupSelectionQuestion( const message = 'Select groups:'; const choices = userPoolGroupList; const selectedChoices = defaultValues.groupAccess ? Object.keys(defaultValues.groupAccess) : []; - const selectedIndexes = defaultValues.groupList ? getIndexArray(choices, selectedChoices) : undefined; + const selectedIndexes = selectedChoices ? getIndexArray(choices, selectedChoices) : undefined; const userPoolGroups = await prompter.pick<'many', string>(message, choices, { returnSize: 'many', initial: selectedIndexes }); - //Selected user-pool groups - return userPoolGroups as string[]; + //prompter pick-many returns string if returnsize is 1, and array otherwise. + if ( Array.isArray(userPoolGroups) ){ + return userPoolGroups as string[]; + } else { + //Type is string + return [userPoolGroups]; + } } export async function askUserPoolGroupPermissionSelectionQuestion(): Promise { @@ -248,7 +253,6 @@ export async function askGroupOrIndividualAccessFlow( //Reset group-permissions if using auth/guest if (permissionSelected === UserPermissionTypeOptions.AUTH_GUEST_USERS) { - cliInputs.groupList = []; cliInputs.groupAccess = undefined; } } @@ -259,20 +263,16 @@ export async function askGroupOrIndividualAccessFlow( cliInputs.authAccess = []; cliInputs.guestAccess = []; } - //Update the groupList to select from + //Update the group to select from const selectedUserPoolGroupList = await askUserPoolGroupSelectionQuestion(userPoolGroupList, context, cliInputs); //Ask Group Permissions and assign to cliInputs if (!cliInputs.groupAccess) { cliInputs.groupAccess = {}; } - if (!cliInputs.groupList) { - cliInputs.groupList = []; - } if (selectedUserPoolGroupList && selectedUserPoolGroupList.length > 0) { for (const selectedGroup of selectedUserPoolGroupList) { cliInputs.groupAccess[selectedGroup] = await askGroupPermissionQuestion(selectedGroup, context, cliInputs); } - cliInputs.groupList = selectedUserPoolGroupList; } } } diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts index 32652a63fff..8a91f58f1c6 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts @@ -60,37 +60,6 @@ export type GroupCFNAccessType = Record; export type GroupStorageParamsAccessType = Record; -export type S3CLIWalkthroughParams = { - resourceName: string, - bucketName: string, - authPolicyName: string, - unauthPolicyName: string, - authRoleName: ResourcRefType, - unauthRoleName: ResourcRefType, - storageAccess: S3AccessType, - selectedGuestPermissions: S3CFNPermissionType[], - selectedAuthenticatedPermissions: S3CFNPermissionType[], - s3PermissionsAuthenticatedPublic: string, - s3PublicPolicy: string, - s3PermissionsAuthenticatedUploads: string, - s3UploadsPolicy: string, - s3PermissionsAuthenticatedProtected: string, - s3ProtectedPolicy: string, - s3PermissionsAuthenticatedPrivate: string, - s3PrivatePolicy: string, - AuthenticatedAllowList: string, - s3ReadPolicy: string, - s3PermissionsGuestPublic: string, - s3PermissionsGuestUploads: string, - GuestAllowList: string, - triggerFunction: string, - service: string, - providerPlugin: string, - dependsOn: S3CFNDependsOn[], - policyUUID: string, - groupPolicyMap: GroupCFNAccessType, - groupList: string[] //group-names from group policy Map -} //Data generated by amplify which should not be overridden by the user export type S3FeatureMetadata = { @@ -176,7 +145,6 @@ export class S3InputState { authAccess: [], triggerFunction: "NONE", groupAccess: undefined, - groupList: [] } if ( oldParams.triggerFunction ){ userInputs.triggerFunction = oldParams.triggerFunction; @@ -200,7 +168,6 @@ export class S3InputState { if (storageParams && storageParams.hasOwnProperty("groupPermissionMap")){ userInputs.groupAccess = S3InputState.getPolicyMapFromStorageParamPolicyMap( storageParams.groupPermissionMap ); - userInputs.groupList = (userInputs.groupAccess)?Object.keys(userInputs.groupAccess) : []; } return userInputs; @@ -244,14 +211,6 @@ export class S3InputState { return this._inputPayload; } - public setPoolGroupList(poolGroupList: Array) { - if (this._inputPayload) { - this._inputPayload.groupList = poolGroupList; - } else { - throw Error("ERROR: Pool Group is updated in Add Storage "); - } - } - public async isCLIInputsValid(cliInputs?: S3UserInputs) { if (!cliInputs) { cliInputs = this.getCliInputPayload();