From 4d4d52ff548341936b7ddbfb1fb70b41028842c9 Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore <83682223+sachscode@users.noreply.github.com> Date: Sat, 6 Nov 2021 16:22:39 -0700 Subject: [PATCH] Ext overrides3 fix auth import s3 add (#8704) * (fix) add missing logic to validate auth requirements in storage * (fix) check for import auth * (fix) auth unit-tests couldnt find getImportedAuthProperties Co-authored-by: Sachin Panemangalore --- .../src/__tests__/commands/update.test.ts | 1 + .../utils/check-for-auth-migration.test.ts | 15 ++++- .../utils/check-for-auth-migration.ts | 26 +++++---- .../service-walkthroughs/s3-auth-api.ts | 55 +++++++++++++++++++ .../service-walkthroughs/s3-walkthrough.ts | 35 +++++++----- 5 files changed, 106 insertions(+), 26 deletions(-) diff --git a/packages/amplify-category-auth/src/__tests__/commands/update.test.ts b/packages/amplify-category-auth/src/__tests__/commands/update.test.ts index cb41a4eb4d4..c34db693a87 100644 --- a/packages/amplify-category-auth/src/__tests__/commands/update.test.ts +++ b/packages/amplify-category-auth/src/__tests__/commands/update.test.ts @@ -72,6 +72,7 @@ describe('auth update: ', () => { getProjectDetails: mockGetProjectDetails, serviceSelectionPrompt: mockSelectionPrompt, getPluginInstance: jest.fn().mockReturnValue(mockPluginInstance), + getImportedAuthProperties : jest.fn().mockReturnValue({ imported : false }), readJsonFile: jest.fn(path => JSON.parse(fs.readFileSync(path, 'utf-8'))), pathManager: { getBackendDirPath: jest.fn(), diff --git a/packages/amplify-category-auth/src/__tests__/provider-utils/awscloudformation/utils/check-for-auth-migration.test.ts b/packages/amplify-category-auth/src/__tests__/provider-utils/awscloudformation/utils/check-for-auth-migration.test.ts index e764f0c2d73..9c4dedc834d 100644 --- a/packages/amplify-category-auth/src/__tests__/provider-utils/awscloudformation/utils/check-for-auth-migration.test.ts +++ b/packages/amplify-category-auth/src/__tests__/provider-utils/awscloudformation/utils/check-for-auth-migration.test.ts @@ -63,7 +63,13 @@ test('migration gets called when cli-inputs doesnt exist', async () => { }, }); - await checkAuthResourceMigration({} as unknown as $TSContext, 'mockResource'); + const mockContext = { + amplify : { + getImportedAuthProperties : jest.fn().mockReturnValue({ imported : false }) + } + } + + await checkAuthResourceMigration(mockContext as unknown as $TSContext, 'mockResource'); expect(migrateResourceToSupportOverride).toBeCalled(); expect(generateAuthStackTemplate).toBeCalled(); }); @@ -72,8 +78,13 @@ test('migration doesnt called when cli-inputs exist', async () => { jest.clearAllMocks(); jest.spyOn(AuthInputState.prototype, 'cliInputFileExists').mockImplementation(() => true); jest.spyOn(AuthInputState.prototype, 'getCLIInputPayload'); + const mockContext = { + amplify : { + getImportedAuthProperties : jest.fn().mockReturnValue({ imported : false }) + } + } - await checkAuthResourceMigration({} as unknown as $TSContext, 'mockResource'); + await checkAuthResourceMigration(mockContext as unknown as $TSContext, 'mockResource'); expect(migrateResourceToSupportOverride).not.toBeCalled(); expect(generateAuthStackTemplate).not.toBeCalled(); }); diff --git a/packages/amplify-category-auth/src/provider-utils/awscloudformation/utils/check-for-auth-migration.ts b/packages/amplify-category-auth/src/provider-utils/awscloudformation/utils/check-for-auth-migration.ts index 36828a85608..5aa4b966b10 100644 --- a/packages/amplify-category-auth/src/provider-utils/awscloudformation/utils/check-for-auth-migration.ts +++ b/packages/amplify-category-auth/src/provider-utils/awscloudformation/utils/check-for-auth-migration.ts @@ -5,17 +5,21 @@ import { generateAuthStackTemplate } from './generate-auth-stack-template'; import { migrateResourceToSupportOverride } from './migrate-override-resource'; export const checkAuthResourceMigration = async (context: $TSContext, authName: string) => { - const cliState = new AuthInputState(authName); - if (!cliState.cliInputFileExists()) { - printer.debug('Cli-inputs.json doesnt exist'); - // put spinner here - const isMigrate = await prompter.yesOrNo(`Do you want to migrate this ${authName} to support overrides?`, true); - if (isMigrate) { - // generate cli-inputs for migration from parameters.json - await migrateResourceToSupportOverride(authName); - // fetch cli Inputs again - const cliInputs = cliState.getCLIInputPayload(); - await generateAuthStackTemplate(context, cliInputs.cognitoConfig.resourceName); + // check if its imported auth + const { imported} = context.amplify.getImportedAuthProperties(context); + if(!imported){ + const cliState = new AuthInputState(authName); + if (!cliState.cliInputFileExists()) { + printer.debug('Cli-inputs.json doesnt exist'); + // put spinner here + const isMigrate = await prompter.yesOrNo(`Do you want to migrate this ${authName} to support overrides?`, true); + if (isMigrate) { + // generate cli-inputs for migration from parameters.json + await migrateResourceToSupportOverride(authName); + // fetch cli Inputs again + const cliInputs = cliState.getCLIInputPayload(); + await generateAuthStackTemplate(context, cliInputs.cognitoConfig.resourceName); + } } } }; 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 294f8838548..2b659bd80b0 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 @@ -1,6 +1,8 @@ import { $TSAny, $TSContext } from 'amplify-cli-core'; import { printer } from 'amplify-prompts'; import { AmplifyCategories } from 'amplify-cli-core'; +import os from 'os'; +import { S3AccessType } from '../service-walkthrough-types/s3-user-input-types'; /* This file contains all functions interacting with AUTH category */ @@ -43,3 +45,56 @@ export async function migrateAuthDependencyResource(context: $TSContext) { } } } + +/** + * Check if storage authentication requirements are satisfied by the configured storage + * @param context + * @param storageResourceName + * @param allowUnauthenticatedIdentities + */ +export async function checkStorageAuthenticationRequirements( + context: $TSContext, + storageResourceName: string, + allowUnauthenticatedIdentities: boolean, +) { + const storageRequirements = { authSelections: 'identityPoolAndUserPool', allowUnauthenticatedIdentities }; + + const checkResult: $TSAny = await context.amplify.invokePluginMethod(context, AmplifyCategories.AUTH, undefined, 'checkRequirements', [ + storageRequirements, + context, + 'storage', + storageResourceName, + ]); + + // If auth is imported and configured, we have to throw the error instead of printing since there is no way to adjust the auth + // configuration. + if (checkResult.authImported === true && checkResult.errors && checkResult.errors.length > 0) { + throw new Error(checkResult.errors.join(os.EOL)); + } + + if (checkResult.errors && checkResult.errors.length > 0) { + printer.warn(checkResult.errors.join(os.EOL)); + } + + // If auth is not imported and there were errors, adjust or enable auth configuration + if (!checkResult.authEnabled || !checkResult.requirementsMet) { + try { + // If this is not set as requirement, then explicitly configure it to disabled. + if (storageRequirements.allowUnauthenticatedIdentities === undefined) { + storageRequirements.allowUnauthenticatedIdentities = false; + } + + await context.amplify.invokePluginMethod(context, AmplifyCategories.AUTH, undefined, 'externalAuthEnable', [ + context, + AmplifyCategories.STORAGE, + storageResourceName, + storageRequirements, + ]); + } catch (error) { + printer.error(error as string); + throw error; + } + } +} + + diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts index e96b1a12fce..719f202ec14 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts @@ -33,7 +33,7 @@ import { } from './s3-questions'; import { printErrorAlreadyCreated, printErrorAuthResourceMigrationFailed, printErrorNoResourcesToUpdate } from './s3-errors'; import { getAllDefaults } from '../default-values/s3-defaults'; -import { migrateAuthDependencyResource } from './s3-auth-api'; +import { checkStorageAuthenticationRequirements, migrateAuthDependencyResource } from './s3-auth-api'; import { s3GetAdminTriggerFunctionName } from './s3-resource-api'; /** @@ -66,14 +66,13 @@ export async function addWalkthrough(context: $TSContext, defaultValuesFilename: exitOnNextTick(0); } else { //Ask S3 walkthrough questions - const policyID = buildShortUUID(); //prefix/suffix for all resources. const defaultValues = getAllDefaults(amplify.getProjectDetails(), policyID); - const resourceName = await askResourceNameQuestion(context, defaultValues); //Cannot be changed once added - const bucketName = await askBucketNameQuestion(context, defaultValues, resourceName); //Cannot be changed once added + const storageResourceName = await askResourceNameQuestion(context, defaultValues); //Cannot be changed once added + const bucketName = await askBucketNameQuestion(context, defaultValues, storageResourceName); //Cannot be changed once added let cliInputs: S3UserInputs = Object.assign({}, defaultValues); cliInputs.policyUUID = policyID; - cliInputs.resourceName = resourceName; + cliInputs.resourceName = storageResourceName; cliInputs.bucketName = bucketName; //Check if user-pools are already created const userPoolGroupList = context.amplify.getUserPoolGroupList(); @@ -87,9 +86,14 @@ export async function addWalkthrough(context: $TSContext, defaultValuesFilename: cliInputs.authAccess = await askAuthPermissionQuestion(context, defaultValues); cliInputs.guestAccess = await await conditionallyAskGuestPermissionQuestion(cliInputs.storageAccess, context, defaultValues); } - const triggerFunction = await startAddTriggerFunctionFlow(context, resourceName, policyID, undefined); + const triggerFunction = await startAddTriggerFunctionFlow(context, storageResourceName, policyID, undefined); cliInputs.triggerFunction = triggerFunction ? triggerFunction : 'NONE'; + //Validate Authentication requirements + //e.g if storage is added after import auth, + const allowUnauthenticatedIdentities = ( cliInputs.guestAccess && (cliInputs.guestAccess.length > 0 ) ); + await checkStorageAuthenticationRequirements( context, storageResourceName, allowUnauthenticatedIdentities ); + //Save CLI Inputs payload const cliInputsState = new S3InputState(cliInputs.resourceName as string, cliInputs); await cliInputsState.saveCliInputPayload(cliInputs); @@ -115,25 +119,25 @@ export async function addWalkthrough(context: $TSContext, defaultValuesFilename: */ export async function updateWalkthrough(context: $TSContext) { const amplifyMeta = stateManager.getMeta(); - const resourceName: string | undefined = await getS3ResourceNameFromMeta(amplifyMeta); - if (resourceName === undefined) { + const storageResourceName: string | undefined = await getS3ResourceNameFromMeta(amplifyMeta); + if (storageResourceName === undefined) { await printErrorNoResourcesToUpdate(context); exitOnNextTick(0); } else { // For better DX check if the storage is imported - if (amplifyMeta[AmplifyCategories.STORAGE][resourceName].serviceType === 'imported') { + if (amplifyMeta[AmplifyCategories.STORAGE][storageResourceName].serviceType === 'imported') { printer.error('Updating of an imported storage resource is not supported.'); return; } //load existing cliInputs - let cliInputsState = new S3InputState(resourceName, undefined); + let cliInputsState = new S3InputState(storageResourceName, undefined); //Check if migration is required if (!cliInputsState.cliInputFileExists()) { if (context.exeInfo?.forcePush || (await prompter.confirmContinue('File migration required to continue. Do you want to continue?'))) { //migrate auth and storage await cliInputsState.migrate(context); - const stackGenerator = new AmplifyS3ResourceStackTransform(resourceName, context); + const stackGenerator = new AmplifyS3ResourceStackTransform(storageResourceName, context); await stackGenerator.transform(CLISubCommandType.UPDATE); //generates cloudformation } else { return; @@ -158,19 +162,24 @@ export async function updateWalkthrough(context: $TSContext) { if (previousUserInput.triggerFunction && previousUserInput.triggerFunction != 'NONE') { cliInputs.triggerFunction = await startUpdateTriggerFunctionFlow( context, - resourceName, + storageResourceName, previousUserInput.policyUUID as string, previousUserInput.triggerFunction, ); } else { cliInputs.triggerFunction = await startAddTriggerFunctionFlow( context, - resourceName, + storageResourceName, previousUserInput.policyUUID as string, undefined, ); } + //Validate Authentication requirements + //e.g if storage is added after import auth, + const allowUnauthenticatedIdentities = ( cliInputs.guestAccess && (cliInputs.guestAccess.length > 0 ) ); + await checkStorageAuthenticationRequirements( context, storageResourceName, allowUnauthenticatedIdentities ); + //Save CLI Inputs payload await cliInputsState.saveCliInputPayload(cliInputs); //Generate Cloudformation