Skip to content

Commit

Permalink
(bug-fix/reconcile-headless) remove groupList from userInputs, fix si…
Browse files Browse the repository at this point in the history
…ngle-group, deploy errors (aws-amplify#8501)

Co-authored-by: Sachin Panemangalore <sachinrp@amazon.com>
  • Loading branch information
sachscode and Sachin Panemangalore committed Nov 10, 2021
1 parent 5086efc commit a2caf63
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,19 @@
"type": "string"
},
"groupAccess": {
"$ref": "#/definitions/Record<string,S3PermissionType[]>"
},
"groupList": {
"type": "array",
"items": {
"type": "string"
}
"$ref": "#/definitions/GroupAccessType"
}
},
"required": [
"authAccess",
"bucketName",
"groupAccess",
"groupList",
"guestAccess",
"policyUUID",
"resourceName",
"storageAccess",
"triggerFunction"
"storageAccess"
],
"definitions": {
"Record<string,S3PermissionType[]>": {
"GroupAccessType": {
"type": "object"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,6 @@ class S3MockDataBuilder {
guestAccess: [],
authAccess: this.defaultAuthPerms,
groupAccess: {},
groupList: [],
triggerFunction: 'NONE',
};
cliInputs: S3UserInputs = {
Expand All @@ -860,7 +859,6 @@ class S3MockDataBuilder {
authAccess: [],
triggerFunction: undefined,
groupAccess: undefined,
groupList: undefined,
};

constructor(startCliInputState: S3UserInputs | undefined) {
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ export class AmplifyS3ResourceCfnStack extends AmplifyResourceCfnStack implement
this.s3DependsOnResources = [];
}

private getGroupListFromProps(): Array<string>|undefined {
const groupList = (this._props.groupAccess)? Object.keys(this._props.groupAccess) : [];
return groupList;
}

public getS3ResourceFriendlyName(): string {
return this.id;
}
Expand Down Expand Up @@ -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<string>,
groupList,
this._props.groupAccess as GroupAccessType,
);

Expand All @@ -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);
}

Expand All @@ -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<AmplifyCfnParamType> = [
{
params: [`auth${authResourceName}UserPoolId`],
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ export const getAllDefaults = (project: $TSAny, shortId : string) : S3UserInputs
guestAccess: [],
authAccess: [],
triggerFunction : undefined,
groupAccess : {},
groupList : []
groupAccess : {}
};

return defaults;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>|undefined //Keys of group policy Map
triggerFunction?: string|undefined,
groupAccess? : GroupAccessType|undefined, //{ "admingroup": [create, read, delete, list], "secondgroup" :[...''...] }
}

export function defaultS3UserInputs() :S3UserInputs {
Expand All @@ -61,7 +60,6 @@ export function defaultS3UserInputs() :S3UserInputs {
authAccess : [],
triggerFunction:undefined,
groupAccess: undefined,
groupList: undefined
};
return defaultS3UserInputValues;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> ): Promise<S3UserInputs>{
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<string> {
let authResources = (await context.amplify.getResourceStatus('auth')).allResources;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<UserPermissionTypeOptions> {
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,37 +60,6 @@ export type GroupCFNAccessType = Record<string, S3CFNPermissionType[]>;

export type GroupStorageParamsAccessType = Record<string, S3StorageParamsPermissionType[]>;

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 = {
Expand Down Expand Up @@ -176,7 +145,6 @@ export class S3InputState {
authAccess: [],
triggerFunction: "NONE",
groupAccess: undefined,
groupList: []
}
if ( oldParams.triggerFunction ){
userInputs.triggerFunction = oldParams.triggerFunction;
Expand All @@ -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;
Expand Down Expand Up @@ -244,14 +211,6 @@ export class S3InputState {
return this._inputPayload;
}

public setPoolGroupList(poolGroupList: Array<string>) {
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();
Expand Down

0 comments on commit a2caf63

Please sign in to comment.