aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.41k stars 2.12k forks source link

bug with SOFTWARE_TOKEN_MFA if you use `CUSTOM_AUTH` flow #9592

Open Famin42 opened 2 years ago

Famin42 commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

``` System: OS: macOS 12.0.1 CPU: (8) x64 Apple M1 Memory: 25.52 MB / 8.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 14.18.0 - ~/.nvm/versions/node/v14.18.0/bin/node Yarn: 1.22.11 - /usr/local/bin/yarn npm: 6.14.15 - ~/.nvm/versions/node/v14.18.0/bin/npm Browsers: Chrome: 98.0.4758.80 Safari: 15.1 npmPackages: @aws-amplify/auth: ^4.4.1 => 4.4.1 @babel/core: 7.15.5 => 7.15.5 (7.16.0, 7.12.3, 7.13.8) @babel/node: ^7.0.0 => 7.16.0 @babel/plugin-proposal-class-properties: 7.14.5 => 7.14.5 (7.16.0, 7.12.1) @babel/preset-env: 7.15.6 => 7.15.6 (7.12.1) @babel/preset-react: ^7.0.0 => 7.16.0 (7.12.1) @fortawesome/fontawesome-svg-core: ^1.2.0-14 => 1.2.36 @fortawesome/free-solid-svg-icons: ^5.12.0 => 5.15.4 @fortawesome/react-fontawesome: 0.1.15 => 0.1.15 @freshes/jsdoc-template: ^1.0.0 => 1.0.0 @hookform/resolvers: ^2.8.5 => 2.8.5 @lingui/babel-preset-react: ^2.9.2 => 2.9.2 @lingui/cli: ^2.9.0 => 2.9.2 @lingui/core: ^2.9.0 => 2.9.2 @lingui/react: ^2.9.0 => 2.9.2 @testing-library/react: ^12.1.0 => 12.1.2 @wojtekmaj/enzyme-adapter-react-17: ^0.6.3 => 0.6.5 amazon-cognito-identity-js: ^5.1.0 => 5.2.3 (5.2.6) apollo-link: ^1.2.4 => 1.2.14 apollo-link-http: ^1.5.7 => 1.5.17 babel-core: ^7.0.0-bridge.0 => 7.0.0-bridge.0 babel-eslint: ^10.1.0 => 10.1.0 babel-jest: ^27.1.1 => 27.3.1 babel-loader: ^8.1.0 => 8.2.3 babel-plugin-dynamic-import-node: ^2.3.3 => 2.3.3 babel-plugin-lodash: ^3.3.2 => 3.3.4 babel-plugin-named-asset-import: ^0.3.7 => 0.3.7 babel-plugin-styled-components: ^1.11.1 => 1.13.3 babel-plugin-syntax-dynamic-import: 6.18.0 => 6.18.0 babel-preset-react-app: ^10.0.0 => 10.0.0 bfj: ^7.0.2 => 7.0.2 big.js: ^6.1.1 => 6.1.1 (5.2.2, 3.2.0) bip39: ^3.0.4 => 3.0.4 camelize: ^1.0.0 => 1.0.0 case-sensitive-paths-webpack-plugin: ^2.1.2 => 2.4.0 chokidar: ^3.5.2 => 3.5.2 (2.1.8) class-validator: 1.0.0 clipboard: ^2.0.1 => 2.0.8 computed-types: 1.0.0 copy-webpack-plugin: ^6.4.1 => 6.4.1 cross-env: ^7.0.3 => 7.0.3 cypress: ^8.3.1 => 8.7.0 decamelize: ^4.0.0 => 4.0.0 (1.2.0) docdash: 1.2.0 => 1.2.0 docs: 0.0.0 dotenv: ^10.0.0 => 10.0.0 dotenv-expand: ^5.1.0 => 5.1.0 effector: ^22.1.1 => 22.1.2 effector-logger: ^0.10.0 => 0.10.0 effector-react: ^22.0.4 => 22.0.4 enzyme: ^3.11.0 => 3.11.0 eslint: ^7.32.0 => 7.32.0 eslint-config-airbnb: ^18.2.1 => 18.2.1 eslint-config-prettier: ^8.3.0 => 8.3.0 eslint-config-react-app: ^6.0.0 => 6.0.0 eslint-config-standard: ^16.0.3 => 16.0.3 eslint-import-resolver-webpack: 0.13.1 => 0.13.1 eslint-loader: ^4.0.2 => 4.0.2 eslint-plugin-flowtype: ^5.9.2 => 5.10.0 eslint-plugin-import: ^2.14.0 => 2.25.3 eslint-plugin-jsx-a11y: ^6.1.1 => 6.5.1 eslint-plugin-node: ^11.1.0 => 11.1.0 eslint-plugin-prettier: ^4.0.0 => 4.0.0 eslint-plugin-promise: ^5.1.0 => 5.1.1 eslint-plugin-react: ^7.11.1 => 7.27.0 eslint-plugin-react-hooks: ^1.6.1 => 1.7.0 eslint-plugin-standard: ^5.0.0 => 5.0.0 file-loader: ^6.1.1 => 6.2.0 fs-extra: ^10.0.0 => 10.0.0 (9.1.0, 8.1.0, 7.0.1) graphql: ^15.5.3 => 15.7.2 graphql-tag: ^2.10.0 => 2.12.6 history: ^5.0.1 => 5.1.0 (4.10.1) html-webpack-plugin: ^4.5.2 => 4.5.2 (3.2.0) husky: ^3.1.0 => 3.1.0 io-ts: 1.0.0 isomorphic-unfetch: ^3.0.0 => 3.1.0 jest: ^27.1.1 => 27.3.1 jest-canvas-mock: ^2.2.0 => 2.3.1 joi: 1.0.0 jsdoc: ^3.5.5 => 3.6.7 jsdoc-babel: 0.5.0 => 0.5.0 jsdom: ^17.0.0 => 17.0.0 (16.7.0) lint-staged: ^11.1.2 => 11.2.6 live-server: ^1.2.0 => 1.2.1 lodash: ^4.17.21 => 4.17.21 markdown-jsx-loader: ^3.0.2 => 3.0.2 moment: ^2.21.0 => 2.29.1 nope: 1.0.0 numeral: ^2.0.6 => 2.0.6 opn: ^6.0.0 => 6.0.0 (5.5.0) path: 0.12.7 => 0.12.7 plop: ^2.7.4 => 2.7.6 plop-example: undefined () prettier: ^2.2.1 => 2.4.1 prop-types: ^15.6.0 => 15.7.2 qrcode.react: 1.0.1 => 1.0.1 query-string: ^7.0.1 => 7.0.1 (6.14.1, 5.1.1) rc-tooltip: ^5.1.1 => 5.1.1 react: ^17.0.2 => 17.0.2 react-dev-utils: ^11.0.3 => 11.0.4 react-document-title: ^2.0.3 => 2.0.3 react-dom: ^17.0.2 => 17.0.2 react-flip-move: ^3.0.4 => 3.0.4 react-google-recaptcha-v3: 1.9.7 => 1.9.7 react-hook-form: ^7.23.0 => 7.23.0 react-hot-loader: ^4.3.3 => 4.13.0 react-modal: ^3.4.5 => 3.14.4 react-on-screen: ^2.1.0 => 2.1.1 react-redux: ^7.2.5 => 7.2.6 react-router-dom: ^5.3.0 => 5.3.0 react-svg-loader: 3.0.3 => 3.0.3 react-transition-group: ^4.4.2 => 4.4.2 (2.9.0) react-transition-group/CSSTransition: undefined () react-transition-group/ReplaceTransition: undefined () react-transition-group/SwitchTransition: undefined () react-transition-group/Transition: undefined () react-transition-group/TransitionGroup: undefined () react-transition-group/TransitionGroupContext: undefined () react-transition-group/config: undefined () redux: ^4.1.1 => 4.1.2 redux-mock-store: ^1.5.1 => 1.5.4 redux-thunk: ^2.2.0 => 2.4.0 reselect: ^4.0.0 => 4.1.4 style-loader: ^3.2.1 => 3.3.1 styled-components: ^5.3.1 => 5.3.3 styled-components/macro: undefined () styled-components/native: undefined () styled-components/primitives: undefined () styled-normalize: ^8.0.7 => 8.0.7 stylelint: ^13.13.1 => 13.13.1 stylelint-config-standard: ^22.0.0 => 22.0.0 stylelint-config-styled-components: 0.1.1 => 0.1.1 stylelint-processor-styled-components: ^1.10.0 => 1.10.0 superstruct: 1.0.0 terser-webpack-plugin: ^4.2.3 => 4.2.3 (1.4.5) tinycolor2: ^1.4.1 => 1.4.2 tweetnacl: ^1.0.0 => 1.0.3 (0.14.5) tweetnacl-util: 0.15.1 => 0.15.1 typanion: 1.0.0 ua-parser-js: 0.7.28 => 0.7.28 uglifyjs-webpack-plugin: ^2.2.0 => 2.2.0 url-loader: ^4.1.1 => 4.1.1 url-polyfill: ^1.0.13 => 1.1.12 vest: 1.0.0 webpack: ^4.44.2 => 4.46.0 webpack-bundle-analyzer: ^4.4.2 => 4.5.0 webpack-cli: 4.2.0 => 4.2.0 webpack-dev-server: ^3.11.1 => 3.11.3 webpack-manifest-plugin: ^2.2.0 => 2.2.0 yup: ^0.32.11 => 0.32.11 (1.0.0) zod: 1.0.0 npmGlobalPackages: npm: 6.14.15 ```

Describe the bug

The main problem is in Custom Auth Challenge Lambda Triggers

On the project we use Amplify.js (Auth library) on frontend (react) and AWS Cognito User Pool (as a part of infrastructure) on the server side.

As you can see here at docs, AWS Cognito provides different ways how to use its authentication functional:

Auth Flows Configurations:

There was ALLOW_USER_SRP_AUTH by default, which provide users to use SOFTWARE MFA as the 2th authentication factor.

later we ran into a problem with bruteforcing our users accounts.

So at first we'd like to try setup Firewall for our Cognito User Pool (but we discovered that in our case it's impossible to setup custom domain for Cognito User Pool with CloudFlare (for example), or somehow override setup Cognito's default Firewall, cause we relize that default Cognito's Firewall is quite silly and fails to do its job). So, the idea with firewall seems not work.

Ok, next one what we decided to try was to add Google recaptcha v3. Frontend requests google for a recaptcha and then puts it to a metadata at Auth.signIn .On the server side recaptcha is processed at PreAuth Lambda trigger. It only worked for a couple of months, cause we got situation: when attackers try to bruteforce our app with thousands of request from hundreds IPs, recaptcha v3 goes crazy and returns very low score (from 0.1 to 0.3) for all requests (good users also get low score and can't login)

So, we decide to try to implement Custom Auth Challenge Lambda Triggers, cause this allows us to move recaptcha validation to a next steps and to implement our custom DDOS protection mechanism at PreAuth trigger.

So, the SignIn request flow will be like this:

  1. Frontend call Auth.signIn and send first request to Cognito with action AWSCognitoIdentityProviderService.InitiateAuth. Cognito PreAuth trigger validates this request with our custom DDOS protection mechanism, if everything is ok, request processing will be continue, otherwise auth flow will be stopped with error.

  2. After PreAuth trigger this request will be processed at DefineAuthChallenge trigger, which will recognize that this is an initial request (cause it has only challengeName === "SRP_A") and answer with a resposne:

response = {
  issueTokens: false,
  failAuthentication: false,
  challengeName: "PASSWORD_VERIFIER",
};
  1. Frontend will process this response under the hood with help of Amplify.auth and send next request to Cognito with action AWSCognitoIdentityProviderService.RespondToAuthChallenge with challengeName: "PASSWORD_VERIFIER". This request will be processed at DefineAuthChallenge trigger again, but will response with:
response = {
  issueTokens: false,
  failAuthentication: false,
  challengeName: "CUSTOM_CHALLENGE",
};

And then (cause we return challengeName: "CUSTOM_CHALLENGE" ) CreateAuthChallenge trigger will intercept this response to create our custom challenge.

  1. Frontend will process this response with challengeName: "CUSTOM_CHALLENGE , use recaptcha as an answer and send request to Cognito with action AWSCognitoIdentityProviderService.RespondToAuthChallenge with challengeName: "CUSTOM_CHALLENGE.

  2. This request will be processed at VerifyAuthChallengeResponse trigger (just recaptcha v3 server side validation based on score)

SO, the whole flow works fine if users use only one factor for authentication, in our case its login with password. BUT if user has a second authentication factor (in our case it's optional for users) such as SOFTWARE_TOKEN_MFA, the flow will be broken on step, when the client receives a response from Cognito! (even if the MFA validation was successful) with Error Cannot read properties of undefined (reading 'NewDeviceMetadata').

BTW: Some time ago, we received an advice from AWS Support about how to protect from bruteforcing: just use our Adding advanced security feature with additional charges and everything will be fine!

BUT:

Also there is a question on stackoverflow.

Expected behavior

Just to add that we're seeing exactly the same issue here.

Reproduction steps

CognitoUser.js:808 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'NewDeviceMetadata')
    at CognitoUser.js:808:49
    at Client.js:140:31

Code Snippet

DefineAuthChallenge trigger:

export const handler: Handler<DefineAuthChallengeTriggerEvent> = (
  event: DefineAuthChallengeTriggerEvent,
  context: Context,
  callback: Callback<DefineAuthChallengeTriggerEvent>,
) => {
  if (
    event.request.session.length == 1 &&
    event.request.session[0].challengeName == "SRP_A"
  ) {
    event.response.issueTokens = false;
    event.response.failAuthentication = false;
    event.response.challengeName = "PASSWORD_VERIFIER";
  } else if (
    event.request.session.length == 2 &&
    event.request.session[1].challengeName == "PASSWORD_VERIFIER" &&
    event.request.session[1].challengeResult == true
  ) {
    event.response.issueTokens = false;
    event.response.failAuthentication = false;
    event.response.challengeName = "CUSTOM_CHALLENGE";
  } else if (
    event.request.session.length == 3 &&
    event.request.session[2].challengeName == "CUSTOM_CHALLENGE" &&
    event.request.session[2].challengeResult == true
  ) {
    event.response.issueTokens = true;
    event.response.failAuthentication = false;
    // CUSTOM FOR MFA
  } else if (
    event.request.session.length === 3 &&
    event.request.session[2].challengeName === "SOFTWARE_TOKEN_MFA" &&
    event.request.session[2].challengeResult === true
  ) {
    event.response.issueTokens = false;
    event.response.failAuthentication = false;
    event.response.challengeName = "CUSTOM_CHALLENGE";
  } else if (
    event.request.session.length == 4 &&
    event.request.session[3].challengeName == "CUSTOM_CHALLENGE" &&
    event.request.session[3].challengeResult == true
  ) {
    event.response.issueTokens = true;
    event.response.failAuthentication = false;
  } else {
    event.response.issueTokens = false;
    event.response.failAuthentication = true;
  }

  callback(null, event);
};

CreateAuthChallenge trigger:

export const handler: CreateAuthChallengeTriggerHandler = (
  event: CreateAuthChallengeTriggerEvent,
  context: Context,
  callback: Callback<CreateAuthChallengeTriggerEvent>,
) => {
  if (event.request.challengeName == "CUSTOM_CHALLENGE") {
    event.response.publicChallengeParameters = {};
    event.response.publicChallengeParameters.captchaUrl = "url/123.jpg";
    event.response.privateChallengeParameters = {};
    event.response.privateChallengeParameters.answer = "5";

    event.response.challengeMetadata = "RECAPTCHA_CHALLENGE";
  }

  callback(null, event);
};

VerifyAuthChallengeResponse trigger:

export const handler: VerifyAuthChallengeResponseTriggerHandler = async (
  event: VerifyAuthChallengeResponseTriggerEvent,
  context: Context,
  callback: Callback<VerifyAuthChallengeResponseTriggerEvent>,
) => {

  try {
    const { recaptchaToken: token } = JSON.parse(
      event.request.challengeAnswer,
    );

   if(token !== "recaptchaToken") { throw new Error(); }

    event.response.answerCorrect = true;
  } catch (error) {
    logger.error(error);
    event.response.answerCorrect = false;
  } finally {
    callback(null, event);
  }
};

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

Famin42 commented 2 years ago

@sammartinez any news?

Famin42 commented 2 years ago

@sammartinez any news?

mohayad commented 2 years ago

@sammartinez @Fomin2402 @robbevan @calavera any news?

mohayad commented 2 years ago

Cognito's team is unreliable

Famin42 commented 2 years ago

@sammartinez any news? Bug/issue is still actual

Famin42 commented 2 years ago

@nickarocho @elorzafe @abdallahshaban557 Hi guys, are there any news?

nickarocho commented 2 years ago

Hi @Fomin2402 , thank you for your patience. We are currently working on reproducing the issue on our side to fully understand the scope of the problem and pinpoint the root cause.

I'll reach back out when I have a working sample representing the exact flow you've described here. Thanks!

Famin42 commented 2 years ago

@nickarocho 20 days passed from your last update. This issue is open from 15 Feb 2022.

Ok, as I can see there are no options for Custom Auth flow with SOFTWARE_TOKEN_MFA challenge due to this link [1].

BUT, as you can see here [2], Custom Auth Flow allows developers to use Custom Auth Flow with SRP. And if SOFTWARE_TOKEN_MFA is a necessary part of SRP Auth, why does it not available in Custom Auth Flow? For us, it's a real problem, cause we need to set up Custom Auth Flow on our production project with more than 100.000 users who can have an optional SOFTWARE MFA step.

Please tell us why this is so and what to do?

[1] Define Auth challenge Lambda trigger - Define Auth challenge request parameters

[2] Custom authentication flow and challenges - A custom authentication flow can also use a combination of built-in challenges, such as SRP

Famin42 commented 2 years ago

Yes, also this is the runtime error in Amplify lib

CognitoUser.js:808 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'NewDeviceMetadata')
    at CognitoUser.js:808:49
    at Client.js:140:31
nickarocho commented 2 years ago

Another member of our team was able to reproduce the problem. We have this issue triaged internally and will be working on a fix. Thanks again for bringing this to our attention.

Famin42 commented 2 years ago

@nickarocho any news?

abdallahshaban557 commented 2 years ago

Hi @Fomin2402 - we do not have an update on this from our end. We will reply back on this issue when we have updates!

Famin42 commented 1 year ago

Any news?

KurogamiLight3303 commented 1 year ago

Do anyone knows of a solution for this?

Amir-Inbar commented 1 year ago

There are any updates? @nickarocho

cwomack commented 1 year ago

We are looking into this issue to determine the root cause of where the bug comes from and will update this issue soon.

Vingtoft commented 1 year ago

Updates?

pf-joao-schmitt commented 1 year ago

Any news? This is a very important topic, I hope the team can decide to prioritize this fix ASAP. The difference is that instead of CAPTCHA I'm implementing a custom authentication flow with e-mail validation.

That's exactly the flow breaking for me:

import { Amplify, Auth } from 'aws-amplify'
import pkg from 'prompt-sync'

const prompt = pkg({ sigint: true })

const region = 'xx-xxxx-x'
const pool_id = 'xx-xxxx-x_xxxxxxxx'
const client_id = 'xxxxxxxxxxxxxxxxxxxxxxxx'
const username = 'xxxxx'
const password = 'xxxxx'

Amplify.configure({
    Auth: {
        region: region,
        userPoolId: pool_id,
        userPoolWebClientId: client_id,
        mandatorySignIn: false,
        authenticationFlowType: 'CUSTOM_AUTH'
    }
})

async function login() {

    var resp = await Auth.signIn(username, password)
    console.log(resp)

    if (resp?.challengeName === 'SOFTWARE_TOKEN_MFA') {
        const mfa_code = prompt("Enter MFA code: ")
        resp = await Auth.confirmSignIn(resp, mfa_code, 'SOFTWARE_TOKEN_MFA')
        console.log(resp)
    }

    if (resp?.challengeName === 'CUSTOM_CHALLENGE') {
        const email_code = prompt("Enter the EMAIL code: ")
        // breakes with the mentioned error: 
       // TypeError: Cannot read properties of undefined (reading 'NewDeviceMetadata')
        resp = await Auth.sendCustomChallengeAnswer(resp, email_code)
        console.log(resp)
    }

    console.log(`Access token: ${resp?.signInUserSession?.accessToken?.jwtToken}`)
}

await login()
rufusmai commented 1 year ago

I have the exact same problem with the amazon-coginto-identity-js library. The error occurs while calling cognitoUser.sendMFACode(). None of the callback methods are called (onSuccess/onError), but an error is thrown instead:

TypeError: Cannot read properties of undefined (reading 'NewDeviceMetadata')

This is breaking my login flow logic, where I need to use a CUSTOM_CHALLENGE after MFA. I also hope this issue will be prioritized.

aamirorbit commented 9 months ago

Hello, I'm getting the same issue while implementing the custom auth flow, is there any update on this one?

campbellborder commented 8 months ago

Hi, also getting the same issue when trying to implement CUSTOM_CHALLENGE -> SOFTWARE_TOKEN_MFA. I'm getting the error "Invalid code or auth state for the user" even though the MFA code is correct.

cwomack commented 5 months ago

For anyone following this issue, this issue has been replicated on v5 but is resolved in v6 of Amplify. Can anyone migrate to v6 and let us know if they still run into the issue?

JVaghela-Fintech commented 2 months ago

I'm trying to implement email 2FA with TOTP using Amplify v6, but the sign-in process always returns CONFIRM_SIGN_IN_WITH_TOTP_CODE even though the auth flow is set to CUSTOM_WITH_SRP. After disabling 2FA from the Cognito pool, it returns CONFIRM_SIGN_IN_WITH_CUSTOM_CHALLENGE. How can we enable 2FA with TOTP using the default flow and email 2FA with the custom flow?

const { nextStep } = await Auth.signIn({ username: email, password: password, options:{ authFlowType:'CUSTOM_WITH_SRP' } });

rwilczekTBI commented 2 months ago

I also have a need for this use case and have though of a possible workaround. Since I am already using custom auth for SMS and returning that answer via Amplify Auth.sendCustomChallengeAnswer(cognitoUser, code) which is received by my verifyAuthChallenge Lambda trigger and verified. Why not use the same Amplify method to send the TOTP code to the verifyAuthChallenge Lambda and have it call the Cognito API respondToAuthChallenge method to validate the code.

Doing a quick test with the following payload from Lambda successfully returned the AccessToken, RefreshToken and IdToken.

{ ChallengeName: "SOFTWARE_TOKEN_MFA", ChallengeResponses: { USERNAME: "Cognito User", SOFTWARE_TOKEN_MFA_CODE: "TOTP Code" }, ClientId: "Cognito Client Id", Session: "valid user session" }

Anyone see any issues with doing this?

gkphillips commented 1 week ago

For anyone following this issue, this issue has been replicated on v5 but is resolved in v6 of Amplify. Can anyone migrate to v6 and let us know if they still run into the issue?

I have updated to Amplify V6 and continue to experience this same issue.