diff --git a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.test.ts b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.test.ts new file mode 100644 index 00000000000..e912ca335bc --- /dev/null +++ b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.test.ts @@ -0,0 +1,93 @@ +import { $TSContext } from '@aws-amplify/amplify-cli-core'; +import { AmplifyS3ResourceStackTransform } from '../../../../provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform'; +import { S3UserInputs } from '../../../../provider-utils/awscloudformation/service-walkthrough-types/s3-user-input-types'; +import { S3InputState } from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state'; + +jest.mock('@aws-amplify/amplify-cli-core', () => ({ + pathManager: { + getBackendDirPath: jest.fn().mockReturnValue('mockbackendpath'), + }, + AmplifyError: class AmplifyError extends Error { + constructor(code: string, options: { message: string }) { + super(options.message); + this.name = code; + } + }, +})); + +jest.mock('../../../../provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state'); + +const userInput: S3UserInputs = { + resourceName: 'storage', + bucketName: 'bucket', + policyUUID: 'abc123', + storageAccess: undefined, + guestAccess: [], + authAccess: [], +}; + +const importedAuthMeta = { auth: { cognitoauth: { service: 'Cognito', serviceType: 'imported' } } }; +const managedAuthMeta = { auth: { cognitoauth: { service: 'Cognito', serviceType: 'managed' } } }; + +const buildContext = (meta: unknown, envName: string): $TSContext => + ({ + amplify: { + getProjectMeta: jest.fn().mockReturnValue(meta), + getEnvInfo: jest.fn().mockReturnValue({ envName }), + }, + } as unknown as $TSContext); + +const stubInputState = (): void => { + jest.spyOn(S3InputState.prototype, 'getCliInputPayload').mockReturnValue(userInput); + jest.spyOn(S3InputState.prototype, 'getUserInput').mockReturnValue(userInput); + jest.spyOn(S3InputState, 'getCfnPermissionsFromInputPermissions').mockReturnValue([]); +}; + +describe('AmplifyS3ResourceStackTransform', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('appends the environment name to IAM policy names when auth is imported', () => { + stubInputState(); + const transform = new AmplifyS3ResourceStackTransform('storage', buildContext(importedAuthMeta, 'prod')); + transform.generateCfnInputParameters(); + + expect(transform.getCFNInputParams()).toMatchObject({ + s3PrivatePolicy: 'Private_policy_abc123_prod', + s3ProtectedPolicy: 'Protected_policy_abc123_prod', + s3PublicPolicy: 'Public_policy_abc123_prod', + s3ReadPolicy: 'read_policy_abc123_prod', + s3UploadsPolicy: 'Uploads_policy_abc123_prod', + authPolicyName: 's3_amplify_abc123_prod', + unauthPolicyName: 's3_amplify_abc123_prod', + }); + }); + + it('keeps the legacy (env-agnostic) IAM policy names when auth is not imported', () => { + // Managed auth gives each environment its own roles, so names never collide. Keeping the + // legacy name avoids forcing a policy replacement on every existing storage app on upgrade. + stubInputState(); + const transform = new AmplifyS3ResourceStackTransform('storage', buildContext(managedAuthMeta, 'prod')); + transform.generateCfnInputParameters(); + + expect(transform.getCFNInputParams()).toMatchObject({ + s3PrivatePolicy: 'Private_policy_abc123', + s3ProtectedPolicy: 'Protected_policy_abc123', + s3PublicPolicy: 'Public_policy_abc123', + s3ReadPolicy: 'read_policy_abc123', + s3UploadsPolicy: 'Uploads_policy_abc123', + authPolicyName: 's3_amplify_abc123', + unauthPolicyName: 's3_amplify_abc123', + }); + }); + + it('throws when auth is imported but the current environment name cannot be determined', () => { + // Imported auth needs the env suffix; a blank envName would otherwise degrade to a + // shared "_undefined" name and re-collide, so we fail fast instead. + stubInputState(); + const transform = new AmplifyS3ResourceStackTransform('storage', buildContext(importedAuthMeta, '')); + + expect(() => transform.generateCfnInputParameters()).toThrow(/environment name/); + }); +}); diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.ts index 36d666baf35..baeb7bff3b9 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform.ts @@ -112,13 +112,46 @@ export class AmplifyS3ResourceStackTransform { if (userInput.adminTriggerFunction?.triggerFunction && userInput.adminTriggerFunction.triggerFunction !== 'NONE') { this.cfnInputParams.adminTriggerFunction = userInput.adminTriggerFunction.triggerFunction; } - this.cfnInputParams.s3PrivatePolicy = `Private_policy_${userInput.policyUUID}`; - this.cfnInputParams.s3ProtectedPolicy = `Protected_policy_${userInput.policyUUID}`; - this.cfnInputParams.s3PublicPolicy = `Public_policy_${userInput.policyUUID}`; - this.cfnInputParams.s3ReadPolicy = `read_policy_${userInput.policyUUID}`; - this.cfnInputParams.s3UploadsPolicy = `Uploads_policy_${userInput.policyUUID}`; - this.cfnInputParams.authPolicyName = `s3_amplify_${userInput.policyUUID}`; - this.cfnInputParams.unauthPolicyName = `s3_amplify_${userInput.policyUUID}`; + // Policy names carry the app-wide policyUUID. That alone is only insufficient when the + // authenticated/unauthenticated IAM roles are SHARED across environments, which happens + // when auth is an imported Cognito Identity Pool: two environments' storage stacks then + // try to manage identically named inline policies on the same role, and CloudFormation's + // single-stack ownership enforcement rejects the deploy ("Policy resource was already + // managed by another stack"). Appending the environment name keeps them distinct. + // + // For the default (managed) auth case each environment owns its own auth roles, so the + // identical policy names land on different roles and never collide. We deliberately keep + // the legacy `${policyUUID}` name there: a PolicyName change is replace-on-update for + // AWS::IAM::Policy, so suffixing unconditionally would force a policy replacement on + // EVERY existing storage app's next push -- churn (and risk) far beyond the shared-role + // bug this fixes. Gating on imported auth limits the rename to the environments that + // actually need it (and whose deploys are already failing). + const authResources = this.context.amplify.getProjectMeta?.()?.auth ?? {}; + const hasImportedAuth = Object.values(authResources).some( + (authResource: $TSAny) => authResource?.service === 'Cognito' && authResource?.serviceType === 'imported', + ); + let policyNameSuffix = userInput.policyUUID; + if (hasImportedAuth) { + // Imported auth shares roles across environments, so the env name is required to keep + // policy names distinct. generateCfnInputParameters only runs for a checked-out + // environment (push / add / update / override / export), so envName is expected here; + // fail fast with a clear error rather than silently producing a shared "_undefined". + const envName = this.context.amplify.getEnvInfo()?.envName; + if (typeof envName !== 'string' || envName.length === 0) { + throw new AmplifyError('EnvironmentNotInitializedError', { + message: 'Cannot determine the current Amplify environment name while generating S3 IAM policy names for imported auth.', + resolution: `Run 'amplify init' or 'amplify env checkout ' in the root of your app directory to select an environment, then try again.`, + }); + } + policyNameSuffix = `${userInput.policyUUID}_${envName}`; + } + this.cfnInputParams.s3PrivatePolicy = `Private_policy_${policyNameSuffix}`; + this.cfnInputParams.s3ProtectedPolicy = `Protected_policy_${policyNameSuffix}`; + this.cfnInputParams.s3PublicPolicy = `Public_policy_${policyNameSuffix}`; + this.cfnInputParams.s3ReadPolicy = `read_policy_${policyNameSuffix}`; + this.cfnInputParams.s3UploadsPolicy = `Uploads_policy_${policyNameSuffix}`; + this.cfnInputParams.authPolicyName = `s3_amplify_${policyNameSuffix}`; + this.cfnInputParams.unauthPolicyName = `s3_amplify_${policyNameSuffix}`; this.cfnInputParams.AuthenticatedAllowList = this._getAuthGuestListPermission(S3PermissionType.READ, userInput.authAccess); this.cfnInputParams.GuestAllowList = this._getAuthGuestListPermission(S3PermissionType.READ, userInput.guestAccess); this.cfnInputParams.s3PermissionsAuthenticatedPrivate = this._getPublicPrivatePermissions(