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.42k stars 2.12k forks source link

Auth.signIn() does not gracefully handle incorrect username/password #9686

Closed mgithubmessier closed 2 years ago

mgithubmessier commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

``` # Put output below this line System: OS: macOS 12.2.1 CPU: (10) arm64 Apple M1 Pro Memory: 130.28 MB / 16.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 16.13.1 - /usr/local/bin/node Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.1/bin/yarn npm: 8.3.0 - ~/.nvm/versions/node/v16.13.1/bin/npm Watchman: 2022.01.31.00 - /opt/homebrew/bin/watchman Browsers: Chrome: 99.0.4844.51 Safari: 15.3 npmPackages: @aws-amplify/auth: ^4.4.2 => 4.4.2 @aws-amplify/ui-react: ^2.9.0 => 2.9.0 @aws-amplify/ui-react-internal: undefined () @aws-amplify/ui-react-legacy: undefined () @dispatch/prettier-config: ^1.3.1 => 1.3.1 @emotion/babel-preset-css-prop: ^11.2.0 => 11.2.0 @emotion/css: ^11.7.1 => 11.7.1 @emotion/react: ^11.8.1 => 11.8.1 @emotion/styled: ^11.8.1 => 11.8.1 @mui/icons-material: ^5.4.4 => 5.4.4 @mui/lab: 5.0.0-alpha.71 => 5.0.0-alpha.71 @mui/material: ^5.4.4 => 5.4.4 @mui/types: ^7.1.2 => 7.1.2 @testing-library/jest-dom: ^4.2.4 => 4.2.4 @testing-library/react: ^9.5.0 => 9.5.0 @testing-library/user-event: ^7.2.1 => 7.2.1 @types/d3: ^7.1.0 => 7.1.0 @types/jest: ^24.9.1 => 24.9.1 @types/lodash: ^4.14.159 => 4.14.179 @types/node: ^12.12.53 => 12.12.54 @types/pluralize: ^0.0.29 => 0.0.29 @types/react: ^17.0.39 => 17.0.39 @types/react-dom: ^17.0.12 => 17.0.12 @types/react-redux: ^7.1.9 => 7.1.23 @types/react-router: ^5.1.8 => 5.1.8 @types/react-router-dom: ^5.1.5 => 5.1.5 @types/react-speech-recognition: ^3.9.0 => 3.9.0 @types/yup: ^0.29.4 => 0.29.4 babel-plugin-import: ^1.13.0 => 1.13.0 customize-cra: ^1.0.0 => 1.0.0 d3: ^7.3.0 => 7.3.0 husky: ^4.2.5 => 4.2.5 lint-staged: ^10.2.11 => 10.2.11 lodash: ^4.17.19 => 4.17.21 pluralize: ^8.0.0 => 8.0.0 prettier: 2.0.5 => 2.0.5 query-string: ^6.13.1 => 6.13.1 (4.3.4) react: ^17.0.2 => 17.0.2 react-app-rewired: ^2.2.1 => 2.2.1 react-dom: ^17.0.2 => 17.0.2 react-hook-form: ^7.27.1 => 7.27.1 react-query: ^3.34.16 => 3.34.16 react-redux: ^7.2.6 => 7.2.6 react-router-dom: ^5.2.0 => 5.3.0 react-scripts: 3.4.1 => 3.4.1 react-speech-recognition: ^3.9.0 => 3.9.0 redux: ^4.0.5 => 4.0.5 redux-devtools-extension: ^2.13.8 => 2.13.8 typescript: ^4.6.2 => 4.6.2 whatwg-fetch: ^3.4.0 => 3.4.0 yup: ^0.32.11 => 0.32.11 npmGlobalPackages: @dispatch/auth: 0.1.2 @dispatch/availability-settings: 0.1.1 @dispatch/entities: 0.2.0 @dispatch/hooks: 0.0.4 @dispatch/job-filters: 1.0.5 @dispatch/messaging: 1.0.0 @dispatch/porcelain: 2.32.5 @nrwl/cli: 13.4.3 corepack: 0.10.0 dispatch: 0.0.0 jest-cli: 27.4.7 npm: 8.3.0 react-devtools-core: 4.22.1 react-devtools: 4.22.1 yarn: 1.22.17 ```

Describe the bug

I've run into a problem using Auth.signIn() with an incorrect username/password where instead of only reporting an invalid username/password error gracefully, there is instead an undefined access error handling the AuthEvent. Seems to me like this could be solved with some optional chaining on the setUser function.

Screen Shot 2022-03-10 at 9 45 47 AM

Expected behavior

After supplying Auth.signIn() with invalid credentials, ONLY the NotAuthorizedException should be thrown.

Reproduction steps

  1. Install "@aws-amplify/auth": "^4.4.2" and "@aws-amplify/ui-react": "^2.9.0"
  2. Wrap your component in the
  3. Call Auth.signIn with an invalid username and password

Code Snippet

<AmplifyProvider>
    <Authenticator
      services={{
        handleSignIn: async ({ username, password }) => {
          let cognitoUser;
          try {
            cognitoUser = await Auth.signIn(
              username,
              password,
            )
          } catch (error) {
            console.error('Failed to sign in with amplify', error);
            return;
          }
        },
      }}
    />
</AmplifyProvider>

Log output

``` // Put your logs below this line index.js:1 Failed to sign in with amplify NotAuthorizedException: Incorrect username or password. at http://localhost:3000/static/js/0.chunk.js:713721:19 console. @ index.js:1 handleSignIn @ App.tsx:149 await in handleSignIn (async) signIn @ signIn.ts:442 Interpreter.exec @ interpreter.js:846 Interpreter.execute @ interpreter.js:201 Interpreter.update @ interpreter.js:227 (anonymous) @ interpreter.js:107 Scheduler.process @ scheduler.js:65 Scheduler.schedule @ scheduler.js:44 Interpreter.send @ interpreter.js:101 Interpreter.sendTo @ interpreter.js:133 Interpreter.exec @ interpreter.js:790 Interpreter.execute @ interpreter.js:201 Interpreter.update @ interpreter.js:227 (anonymous) @ interpreter.js:107 Scheduler.process @ scheduler.js:65 Scheduler.schedule @ scheduler.js:44 Interpreter.send @ interpreter.js:101 (anonymous) @ facade.ts:30 handleSubmit @ index.js:1 callCallback @ react-dom.development.js:3945 invokeGuardedCallbackDev @ react-dom.development.js:3994 invokeGuardedCallback @ react-dom.development.js:4056 invokeGuardedCallbackAndCatchFirstError @ react-dom.development.js:4070 executeDispatch @ react-dom.development.js:8243 processDispatchQueueItemsInOrder @ react-dom.development.js:8275 processDispatchQueue @ react-dom.development.js:8288 dispatchEventsForPlugins @ react-dom.development.js:8299 (anonymous) @ react-dom.development.js:8508 batchedEventUpdates$1 @ react-dom.development.js:22396 batchedEventUpdates @ react-dom.development.js:3745 dispatchEventForPluginEventSystem @ react-dom.development.js:8507 attemptToDispatchEvent @ react-dom.development.js:6005 dispatchEvent @ react-dom.development.js:5924 unstable_runWithPriority @ scheduler.development.js:468 runWithPriority$1 @ react-dom.development.js:11276 discreteUpdates$1 @ react-dom.development.js:22413 discreteUpdates @ react-dom.development.js:3756 dispatchDiscreteEvent @ react-dom.development.js:5889 Show 9 more frames actions.ts:104 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'user') at user (actions.ts:104:1) at utils.js:368:1 at Array.reduce () at updateContext (utils.js:350:1) at resolveActions (actions.js:415:1) at StateNode.resolveTransition (StateNode.js:890:1) at StateNode.transition (StateNode.js:826:1) at interpreter.js:703:1 at provide (serviceScope.js:8:1) at Interpreter.nextState (interpreter.js:702:1) at interpreter.js:105:1 at Scheduler.process (scheduler.js:65:1) at Scheduler.schedule (scheduler.js:44:1) at Interpreter.send (interpreter.js:101:1) at interpreter.js:1007:1 user @ actions.ts:104 (anonymous) @ utils.js:368 updateContext @ utils.js:350 resolveActions @ actions.js:415 StateNode.resolveTransition @ StateNode.js:890 StateNode.transition @ StateNode.js:826 (anonymous) @ interpreter.js:703 provide @ serviceScope.js:8 Interpreter.nextState @ interpreter.js:702 (anonymous) @ interpreter.js:105 Scheduler.process @ scheduler.js:65 Scheduler.schedule @ scheduler.js:44 Interpreter.send @ interpreter.js:101 (anonymous) @ interpreter.js:1007 Promise.then (async) Interpreter.spawnPromise @ interpreter.js:1001 Interpreter.spawn @ interpreter.js:929 Interpreter.exec @ interpreter.js:866 Interpreter.execute @ interpreter.js:201 Interpreter.update @ interpreter.js:227 (anonymous) @ interpreter.js:107 Scheduler.process @ scheduler.js:65 Scheduler.schedule @ scheduler.js:44 Interpreter.send @ interpreter.js:101 Interpreter.sendTo @ interpreter.js:133 Interpreter.exec @ interpreter.js:790 Interpreter.execute @ interpreter.js:201 Interpreter.update @ interpreter.js:227 (anonymous) @ interpreter.js:107 Scheduler.process @ scheduler.js:65 Scheduler.schedule @ scheduler.js:44 Interpreter.send @ interpreter.js:101 (anonymous) @ facade.ts:30 handleSubmit @ index.js:1 callCallback @ react-dom.development.js:3945 invokeGuardedCallbackDev @ react-dom.development.js:3994 invokeGuardedCallback @ react-dom.development.js:4056 invokeGuardedCallbackAndCatchFirstError @ react-dom.development.js:4070 executeDispatch @ react-dom.development.js:8243 processDispatchQueueItemsInOrder @ react-dom.development.js:8275 processDispatchQueue @ react-dom.development.js:8288 dispatchEventsForPlugins @ react-dom.development.js:8299 (anonymous) @ react-dom.development.js:8508 batchedEventUpdates$1 @ react-dom.development.js:22396 batchedEventUpdates @ react-dom.development.js:3745 dispatchEventForPluginEventSystem @ react-dom.development.js:8507 attemptToDispatchEvent @ react-dom.development.js:6005 dispatchEvent @ react-dom.development.js:5924 unstable_runWithPriority @ scheduler.development.js:468 runWithPriority$1 @ react-dom.development.js:11276 discreteUpdates$1 @ react-dom.development.js:22413 discreteUpdates @ react-dom.development.js:3756 dispatchDiscreteEvent @ react-dom.development.js:5889 Show 22 more frames ```

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

mgithubmessier commented 2 years ago

I will say that I just ended up solving this by removing the handleSignIn callback:

       handleSignIn: async ({ username, password }) => {
          let cognitoUser;
          try {
            cognitoUser = await Auth.signIn(
              username,
              password,
            )
          } catch (error) {
            console.error('Failed to sign in with amplify', error);
            return;
          }
        },

And instead making use of the userAuthenticator hook to pickup on when a user is signed in. It's great when you solve your own problems! Haha

chrisbonifacio commented 2 years ago

@mgithubmessier awesome! well done 😄

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server amplify-help forum.