search

aws-amplify / amplify-backend

Home to all tools related to Amplify's code-first DX (Gen 2) for building fullstack apps on AWS
Apache License 2.0
158 stars 54 forks source link

Password policy changes aren't reflected in amplify_outputs.json #1891

Closed bcoates closed 2 weeks ago

bcoates commented 3 weeks ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

Amplify CLI

Environment information

``` # Put output below this line System: OS: macOS 14.6.1 CPU: (8) arm64 Apple M1 Memory: 73.13 MB / 8.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 20.16.0 - /usr/local/bin/node npm: 10.8.1 - /usr/local/bin/npm Browsers: Safari: 17.6 npmPackages: %name%: 0.1.0 @aws-amplify/backend: ^1.0.4 => 1.0.4 @aws-amplify/backend-cli: ^1.1.1 => 1.1.1 @aws-amplify/ui-react: ^6.1.13 => 6.1.13 @aws-amplify/ui-react-internal: undefined () @types/react: ^18.2.66 => 18.3.3 @types/react-dom: ^18.2.22 => 18.3.0 @typescript-eslint/eslint-plugin: ^7.2.0 => 7.16.1 @typescript-eslint/parser: ^7.2.0 => 7.16.1 @vitejs/plugin-react: ^4.2.1 => 4.3.1 aws-amplify: ^6.4.0 => 6.4.0 aws-amplify/adapter-core: undefined () aws-amplify/analytics: undefined () aws-amplify/analytics/kinesis: undefined () aws-amplify/analytics/kinesis-firehose: undefined () aws-amplify/analytics/personalize: undefined () aws-amplify/analytics/pinpoint: undefined () aws-amplify/api: undefined () aws-amplify/api/server: undefined () aws-amplify/auth: undefined () aws-amplify/auth/cognito: undefined () aws-amplify/auth/cognito/server: undefined () aws-amplify/auth/enable-oauth-listener: undefined () aws-amplify/auth/server: undefined () aws-amplify/data: undefined () aws-amplify/data/server: undefined () aws-amplify/datastore: undefined () aws-amplify/in-app-messaging: undefined () aws-amplify/in-app-messaging/pinpoint: undefined () aws-amplify/push-notifications: undefined () aws-amplify/push-notifications/pinpoint: undefined () aws-amplify/storage: undefined () aws-amplify/storage/s3: undefined () aws-amplify/storage/s3/server: undefined () aws-amplify/storage/server: undefined () aws-amplify/utils: undefined () aws-cdk: ^2.138.0 => 2.149.0 aws-cdk-lib: ^2.138.0 => 2.149.0 constructs: ^10.3.0 => 10.3.0 esbuild: ^0.20.2 => 0.20.2 (0.21.5) eslint: ^8.57.0 => 8.57.0 eslint-plugin-react-hooks: ^4.6.0 => 4.6.2 eslint-plugin-react-refresh: ^0.4.6 => 0.4.8 react: ^18.2.0 => 18.3.1 react-dom: ^18.2.0 => 18.3.1 tsx: ^4.7.2 => 4.16.2 typescript: ^5.4.5 => 5.5.3 (4.4.4, 4.9.5) vite: ^5.2.0 => 5.3.4 npmGlobalPackages: corepack: 0.28.2 npm: 10.8.1 ```

Describe the bug

Starting from the tutorial at https://docs.amplify.aws/react/start/quickstart/ Following the directions at https://docs.amplify.aws/react/build-a-backend/auth/modify-resources-with-cdk/ I override the default password policy:

(backend.ts)

  'Policies',
  {
    PasswordPolicy: {
      MinimumLength: 7,
      RequireLowercase: false,
      RequireNumbers: false,
      RequireSymbols: false,
      RequireUppercase: false
    }
  }
);

When deployed this correctly configures Cognito with this setting. However, the produced amplify_outputs.json still contains the defaults:

{
  "auth": {
 ///...snip
   "mfa_configuration": "NONE",
    "password_policy": {
      "min_length": 8,
      "require_numbers": true,
      "require_lowercase": true,
      "require_uppercase": true,
      "require_symbols": true
    },
  },

This confuses the UI components and causes them to still enforce the old rules on the client-side. If there is a way to override this behavior as well, I can't find it in the documentation anywhere.

Expected behavior

I'd expect the amplify_outputs.json to be consistent with the configuration pushed to Cognito.

Reproduction steps

  1. use the quickstart tutorial at https://docs.amplify.aws/react/start/quickstart/
  2. update backend.ts
  3. deploy
  4. verify that the Cognito User Pool created has the modified policy
  5. using the deployed app, try to create a new user with a password that violates the old policy (8 letters, 1 lowercase, 1 uppercase, 1 number, 1 symbol) but obeys the new one
  6. Get a UI error
  7. confirm that amplify_output.json still has the original policy

Code Snippet

// backend.ts
import { defineBackend } from '@aws-amplify/backend';
import { auth } from './auth/resource';
import { data } from './data/resource';

const backend = defineBackend({
  auth,
  data,
});

// override user pool password policies
backend.auth.resources.cfnResources.cfnUserPool.addPropertyOverride(
  'Policies',
  {
    PasswordPolicy: {
      MinimumLength: 7,
      RequireLowercase: false,
      RequireNumbers: false,
      RequireSymbols: false,
      RequireUppercase: false
    }
  }
);

Log output

``` // Put your logs below this line ```

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

cwomack commented 3 weeks ago

Hello, @bcoates 👋. I believe this might be fixed in the latest version of @aws-amplify/backend after this fix was merged in. Can you see if upgrading resolves the issue for you?

I'll also transfer this issue to our amplify-backend repo for further assistance (if upgrading doesn't resolve this).

josefaidt commented 3 weeks ago

Hey @bcoates :wave: thanks for raising this! The issue you are experiencing is due to how addPropertyOverride works and how it applies changes compared to mutating the initialized constructs directly. Can you try to modify the policies instead? https://docs.amplify.aws/react/build-a-backend/auth/modify-resources-with-cdk/#override-cognito-userpool-password-policies

bcoates commented 3 weeks ago

Hi @josefaidt,

If I do this:

const { cfnUserPool } = backend.auth.resources.cfnResources;
// modify cfnUserPool policies directly
cfnUserPool.policies = {
  passwordPolicy: {
    minimumLength: 6,
    requireLowercase: false,
    requireNumbers: true,
    requireSymbols: true,
    requireUppercase: true,
  },
};

it works under npx ampx sandbox locally, but it produces this in the amplify_outputs.json:

"password_policy": {
      "min_length": 6,
      "require_numbers": true,
      "require_uppercase": true,
      "require_symbols": true
    },

the false "requireLowercase" causes it to be omitted entirely. This causes an error on deployment outside the sandbox:

2024-08-20T20:54:31.823Z [INFO]: # Completed phase: build
## Completed Backend Build
2024-08-20T20:54:31.828Z [INFO]: ## Starting Frontend Build
# Starting phase: build
# Executing command: npm run build
2024-08-20T20:54:32.116Z [INFO]: > amplify-vite-react-template@0.0.0 build
> tsc && vite build
2024-08-20T20:54:37.473Z [INFO]: src/main.tsx(8,19): error TS2345: Argument of type '{ auth: { user_pool_id: string; aws_region: string; user_pool_client_id: string; identity_pool_id: string; mfa_methods: never[]; standard_required_attributes: string[]; username_attributes: string[]; user_verification_types: string[]; mfa_configuration: string; password_policy: { ...; }; unauthenticated_identities_e...' is not assignable to parameter of type 'ResourcesConfig | LegacyConfig | AmplifyOutputs'.
Type '{ auth: { user_pool_id: string; aws_region: string; user_pool_client_id: string; identity_pool_id: string; mfa_methods: never[]; standard_required_attributes: string[]; username_attributes: string[]; user_verification_types: string[]; mfa_configuration: string; password_policy: { ...; }; unauthenticated_identities_e...' is not assignable to type 'AmplifyOutputs'.
The types of 'auth.password_policy' are incompatible between these types.
Property 'require_lowercase' is missing in type '{ min_length: number; require_numbers: boolean; require_uppercase: boolean; require_symbols: boolean; }' but required in type '{ min_length: number; require_numbers: boolean; require_lowercase: boolean; require_uppercase: boolean; require_symbols: boolean; }'.
2024-08-20T20:54:37.574Z [ERROR]: !!! Build failed
2024-08-20T20:54:37.575Z [ERROR]: !!! Error: Command failed with exit code 2
ykethan commented 3 weeks ago

Hey @bcoates, tried reproducing this but did not observe this behavior

backend.ts

const { cfnUserPool } = backend.auth.resources.cfnResources;
cfnUserPool.policies = {
  passwordPolicy: {
    minimumLength: 6,
    requireLowercase: false,
    requireNumbers: true,
    requireSymbols: true,
    requireUppercase: true,
  },
};

amplify_outputs.json

"password_policy": {
      "min_length": 6,
      "require_lowercase": false,
      "require_numbers": true,
      "require_symbols": true,
      "require_uppercase": true
    },

additionally, tried some additional modification and observed the changes were reflected on the outputs

Could you try upgrading the backend packages to the latest? as i tried reproducing this on the latest versions

    "@aws-amplify/backend": "^1.1.1",
    "@aws-amplify/backend-cli": "^1.2.3",
bcoates commented 3 weeks ago

Hey @ykethan

I Upgraded the quickstart:

   "devDependencies": {
-    "@aws-amplify/backend": "^1.0.4",
-    "@aws-amplify/backend-cli": "^1.1.1",
+    "@aws-amplify/backend": "^1.1.1",
+    "@aws-amplify/backend-cli": "^1.2.3",

when I run locally with ampx sandbox I get the same results you do. However when I push to the AWS hosted Amplify it results in an amplity_outputs.json that still has false values as missing -- however the deployment doesn't fail and works as expcted (with missing treated as false). Not sure how versioning on the package.json interacts with the AWS hosted version but it does seem to be working both ways with the updated version

Jay2113 commented 3 weeks ago

Hi @bcoates, thanks for reporting it! We are upgrading the @aws-amplify packages to latest for the quickstart guide via this PR: https://github.com/aws-samples/amplify-vite-react-template/pull/13.

ykethan commented 2 weeks ago

Hey @bcoates, this behavior on amplify_outputs.json is currently being tracked on https://github.com/aws-amplify/amplify-backend/issues/1914. Closing this issue, do reach out of if you are still experiencing any issues.