Ext overrides3 fix auth import s3 add (aws-amplify#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 <sachinrp@amazon.com>

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(),

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();
 });

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);
+      }
     }
   }
 };

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;
+    }
+  }
+}
+
+

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