search

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

Creating user with federatedSignIn then deleting that user locks you out of logging in with that idP account for an hour #11543

Closed mtourj closed 1 year ago

mtourj commented 1 year ago

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

``` # Put output below this line System: OS: macOS 13.1 CPU: (10) arm64 Apple M1 Pro Memory: 64.77 MB / 16.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 18.12.1 - /usr/local/bin/node npm: 8.19.2 - /usr/local/bin/npm Watchman: 2023.03.06.00 - /opt/homebrew/bin/watchman Browsers: Chrome: 114.0.5735.133 Safari: 16.2 npmPackages: @babel/core: ^7.20.0 => 7.21.4 @babel/eslint-parser: ^7.19.1 => 7.21.3 @babel/plugin-proposal-export-namespace-from: ^7.18.9 => 7.18.9 @babel/preset-typescript: ^7.16.7 => 7.21.4 @config-plugins/android-jsc-intl: ^4.0.0 => 4.0.0 @config-plugins/react-native-blob-util: ^4.0.0 => 4.0.0 @config-plugins/react-native-pdf: ^4.0.0 => 4.0.0 @date-io/date-fns: ^2.14.0 => 2.16.0 @emotion/react: ^11.9.0 => 11.10.6 @emotion/styled: ^11.8.1 => 11.10.6 @everapi/currencyapi-js: ^1.0.6 => 1.0.6 @expo/config-plugins: ^6.0.1 => 6.0.1 (4.1.1) @expo/vector-icons: ^13.0.0 => 13.0.0 @expo/webpack-config: 18.0.4 => 18.0.4 @faker-js/faker: ^7.6.0 => 7.6.0 @formatjs/intl-displaynames: ^6.2.4 => 6.3.1 @formatjs/intl-getcanonicallocales: ^2.0.5 => 2.1.0 @formatjs/intl-locale: ^3.0.11 => 3.2.1 @graphql-codegen/cli: 2.13.5 => 2.13.5 @graphql-codegen/typescript: 2.4.9 => 2.4.9 @graphql-codegen/typescript-operations: 2.3.6 => 2.3.6 @graphql-codegen/typescript-urql: ^3.5.8 => 3.7.3 @mtourj/react-native-action-sheet: ^4.0.0 => 4.0.0 @mtourj/react-native-context-menu-view: ^1.10.6 => 1.10.6 @mtourj/react-native-image-crop: ^2.0.11 => 2.0.11 @mtourj/react-native-keyboard-aware-scroll-view: ^1.0.9 => 1.0.9 @mtourj/react-native-side-menu: ^1.3.3 => 1.3.3 @mtourj/react-native-skeleton-content: ^1.0.30 => 1.0.30 @mui/material: ^5.6.2 => 5.12.2 @mui/styles: ^5.6.2 => 5.12.0 @mui/x-data-grid: ^5.9.0 => 5.17.26 @mui/x-date-pickers: ^5.0.0-beta.4 => 5.0.20 @react-native-async-storage/async-storage: 1.17.11 => 1.17.11 @react-native-community/datetimepicker: 6.7.3 => 6.7.3 @react-native-community/eslint-config: ^3.2.0 => 3.2.0 @react-native-community/netinfo: 9.3.7 => 9.3.7 @react-native-community/slider: 4.4.2 => 4.4.2 @react-native-picker/picker: 2.4.8 => 2.4.8 @react-native-segmented-control/segmented-control: 2.4.0 => 2.4.0 @react-navigation/bottom-tabs: ^6.4.1 => 6.5.7 @react-navigation/elements: ^1.3.7 => 1.3.17 @react-navigation/native: ^6.0.14 => 6.1.6 @react-navigation/stack: ^6.3.5 => 6.3.16 @rneui/base: ^4.0.0-rc.6 => 4.0.0-rc.7 @rneui/themed: ^4.0.0-rc.6 => 4.0.0-rc.7 @sentry/react-native: 4.15.2 => 4.15.2 @shopify/flash-list: 1.4.0 => 1.4.0 @skele/components: ^1.0.0-alpha.40 => 1.0.0-alpha.40 @testing-library/jest-native: ^4.0.11 => 4.0.13 @testing-library/react-native: ^11.0.0 => 11.5.4 @types/color: ^3.0.3 => 3.0.3 @types/jest: ^29.0.1 => 29.5.1 @types/lodash: ^4.14.189 => 4.14.194 @types/react: ~18.0.27 => 18.0.38 (17.0.58) @types/react-dom: ~18.0.10 => 18.0.11 @types/react-helmet: ^6.1.5 => 6.1.6 @types/react-native-phone-input: ^0.2.2 => 0.2.2 @types/react-test-renderer: ^18.0.0 => 18.0.0 @types/uuid: ^8.3.4 => 8.3.4 @typescript-eslint/eslint-plugin: ^5.49.0 => 5.59.1 @typescript-eslint/parser: ^5.52.0 => 5.59.1 @urql/exchange-auth: ^0.1.7 => 0.1.7 @urql/exchange-graphcache: ^4.4.0 => 4.4.3 @urql/exchange-request-policy: ^1.0.0 => 1.0.2 @urql/exchange-retry: ^1.1.1 => 1.1.1 (0.3.0) HelloWorld: 0.0.1 amazon-cognito-identity-js: ^5.2.9 => 5.2.14 (6.2.0) aws-amplify: ^5.0.8 => 5.1.4 aws-amplify-react-native: ^6.0.4 => 6.0.8 babel-eslint: ^10.1.0 => 10.1.0 babel-plugin-inline-import: ^3.0.0 => 3.0.0 babel-plugin-module-resolver: ^4.1.0 => 4.1.0 babel-preset-expo: ^9.3.0 => 9.3.2 braces: ^3.0.2 => 3.0.2 (2.3.2) color: ^4.2.3 => 4.2.3 (3.2.1) country-currency-map: ^2.1.7 => 2.1.7 currency-decimal-places: ^0.0.5 => 0.0.5 currency-symbol-map: ^5.1.0 => 5.1.0 date-fns: ^2.29.1 => 2.29.3 dayjs: ^1.11.6 => 1.11.7 eslint: ^8.13.0 => 8.39.0 eslint-config-airbnb-typescript: ^17.0.0 => 17.0.0 eslint-config-prettier: ^8.6.0 => 8.8.0 eslint-config-universe: ^11.1.1 => 11.2.0 eslint-import-resolver-babel-module: ^5.1.2 => 5.3.2 eslint-plugin-import: ^2.22.0 => 2.27.5 eslint-plugin-import-helpers: ^1.3.1 => 1.3.1 eslint-plugin-keyconservation: file:eslint-rules => 1.0.0 eslint-plugin-prettier: ^4.2.1 => 4.2.1 eslint-plugin-react: ^7.29.4 => 7.32.2 eslint-plugin-react-hooks: ^4.4.0 => 4.6.0 eslint-utils: ^3.0.0 => 3.0.0 (2.1.0) example: 0.0.1 expo: ^48.0.0 => 48.0.15 expo-application: ~5.1.1 => 5.1.1 expo-asset: ~8.9.1 => 8.9.1 expo-av: ~13.2.1 => 13.2.1 expo-blur: ~12.2.2 => 12.2.2 expo-checkbox: ~2.3.1 => 2.3.1 expo-clipboard: ~4.1.2 => 4.1.2 expo-constants: ~14.2.1 => 14.2.1 expo-dev-client: ~2.2.1 => 2.2.1 expo-device: ~5.2.1 => 5.2.1 expo-document-picker: ~11.2.2 => 11.2.2 expo-font: ~11.1.1 => 11.1.1 expo-haptics: ~12.2.1 => 12.2.1 expo-image: ~1.0.1 => 1.0.1 expo-image-manipulator: ~11.1.1 => 11.1.1 expo-image-picker: ~14.1.1 => 14.1.1 expo-linear-gradient: ~12.1.2 => 12.1.2 expo-linking: ~4.0.1 => 4.0.1 expo-localization: ~14.1.1 => 14.1.1 expo-location: ~15.1.1 => 15.1.1 expo-media-library: ~15.2.3 => 15.2.3 expo-notifications: ~0.18.1 => 0.18.1 expo-secure-store: ~12.1.1 => 12.1.1 expo-splash-screen: ~0.18.2 => 0.18.2 expo-status-bar: ~1.4.4 => 1.4.4 expo-updates: ~0.16.4 => 0.16.4 expo-video-thumbnails: ~7.2.1 => 7.2.1 expo-web-browser: ~12.1.1 => 12.1.1 for-each: ^0.3.3 => 0.3.3 fuse.js: ^6.6.2 => 6.6.2 (3.4.5, 3.6.1) global: ^4.4.0 => 4.4.0 graphql: ^14.7.0 => 14.7.0 (15.8.0) graphql-tag: ^2.12.6 => 2.12.6 graphql-ws: ^5.11.3 => 5.12.1 hls.js: ^1.4.1 => 1.4.1 hyphenation.en-us: ^0.2.1 => 0.2.1 hypher: ^0.2.5 => 0.2.5 intl: ^1.2.5 => 1.2.5 jest: ^29.2.1 => 29.5.0 jest-expo: ^48.0.0 => 48.0.2 jest-fail-on-console: ^3.0.1 => 3.1.1 lodash: ^4.17.21 => 4.17.21 lottie-react-native: 5.1.4 => 5.1.4 mime-types: ^2.1.35 => 2.1.35 minimist: ^1.2.5 => 1.2.8 node-fetch: ^3.2.10 => 3.3.1 (2.6.9, 2.6.7, 2.6.11, 1.7.3) patch-package: ^6.4.7 => 6.5.1 prettier: 2.8.4 => 2.8.4 react: 18.2.0 => 18.2.0 react-dom: 18.2.0 => 18.2.0 react-dropzone: ^14.2.3 => 14.2.3 react-geocode: ^0.2.1 => 0.2.3 react-helmet: ^6.1.0 => 6.1.0 react-lifecycles-compat: ^3.0.4 => 3.0.4 react-native: 0.71.8 => 0.71.8 react-native-animatable: ^1.3.2 => 1.3.3 react-native-blob-util: ^0.17.0 => 0.17.3 react-native-chart-kit: ^6.12.0 => 6.12.0 react-native-circular-progress: ^1.3.7 => 1.3.8 react-native-compressor: ^1.5.2 => 1.6.1 react-native-country-picker-modal: ^1.10.0 => 1.11.0 react-native-css-vh-vw: ^1.0.5 => 1.0.5 react-native-custom-slider: ^1.0.2 => 1.0.2 react-native-element-dropdown: ^2.6.0 => 2.9.0 react-native-gesture-handler: ~2.9.0 => 2.9.0 react-native-get-random-values: ~1.8.0 => 1.8.0 react-native-maps: 1.3.2 => 1.3.2 react-native-material-dropdown: ^0.11.1 => 0.11.1 react-native-mime-types: ^2.3.0 => 2.4.0 react-native-pager-view: 6.1.2 => 6.1.2 react-native-pdf: ^6.6.2 => 6.6.2 react-native-phone-input: ^1.3.3 => 1.3.6 react-native-popover-view: ^2.0.5 => 2.1.3 react-native-popup-menu: ^0.15.6 => 0.15.13 react-native-reanimated: ~2.14.4 => 2.14.4 react-native-refresh-control: ^2.1.0 => 2.1.0 react-native-responsive-dimensions: ^3.0.0 => 3.1.1 react-native-safe-area-context: 4.5.0 => 4.5.0 react-native-screens: ~3.20.0 => 3.20.0 react-native-search-filter: ^0.1.4 => 0.1.5 react-native-slick: ^1.0.2 => 1.6.0 react-native-status-bar-height: ^2.6.0 => 2.6.0 react-native-svg: 13.4.0 => 13.4.0 react-native-tab-view: ^3.1.1 => 3.5.1 react-native-vector-icons: ^7.0.0 => 7.1.0 react-native-web: ~0.18.11 => 0.18.12 react-native-web-maps: ^0.3.0 => 0.3.0 react-native-webview: 11.26.0 => 11.26.0 react-test-renderer: ^18.0.0 => 18.2.0 rn-responsive-styles: ^1.1.1 => 1.1.2 sentry-expo: ~6.2.0 => 6.2.1 setimmediate: ^1.0.5 => 1.0.5 string.prototype.trim: ^1.2.1 => 1.2.7 stripe: ^11.14.0 => 11.18.0 tiny-emitter: ^2.1.0 => 2.1.0 typescript: ^4.9.4 => 4.9.5 urql: ^2.2.0 => 2.2.3 urql-core: undefined () urql-exchange-graphcache-default-storage: 0.0.0 urql-exchange-graphcache-extras: 0.0.0 uuid: ^7.0.3 => 7.0.3 (3.4.0, 8.3.2) validator: ^13.9.0 => 13.9.0 webpack-bundle-analyzer: ^4.7.0 => 4.8.0 wonka: ^6.1.0 => 6.3.2 (4.0.15) ws: ^8.12.1 => 8.13.0 (7.5.9, 6.2.2) xml2js: ^0.5.0 => 0.5.0 (0.4.23) npmGlobalPackages: @aws-amplify/cli: 10.6.1 corepack: 0.14.2 eas-cli: 3.13.3 expo-cli: 6.3.7 npm: 8.19.2 react-devtools: 4.27.4 ```

Describe the bug

Hello. I am running into a rather specific issue when trying to implement the following workflow:

This works well. However, if I sign out and try to sign in again using my idP account, I get a invalid_grant after being redirected to my app with the code and state in the URL.

If I wait an hour, I am able to log in using my idP account just fine. This is, of course, not acceptable as we'd be banking on the user not trying to log in again using their idP within an hour of linking it to an existing account.

It seems that a token is being mismanaged in some way after a Cognito user is deleted, and so if within an hour I try to log in again with the same idP account after it has been linked to an existing user, it fails.

Another way of doing this would have been via the PreSignup trigger. This way, the linking step would occur before a user had to be created then deleted, and no invalid tokens would be lingering anywhere. However since there is no way for me to pass any variables to PreSignup (#5522), there is no way for me to branch PreSignup's behavior in a way that allows me to give users the choice to link accounts or create new ones using their idP.

I have found two instances of the same issue in this repository:

Similarly to the author of #6041, I too am using Expo, and my application is deployed to iOS, Android and the Web. My return URLs setup is similar. This might be relevant as return URLs seemed affect the behavior for him, although I have not tested this.

Expected behavior

When an idP-linked user is deleted from Cognito, it should not interfere with the future ability to create/link another user using the same external idP account, even if it is within an hour.

Reproduction steps

  1. Sign up using Auth.federeatedSignIn({ provider })
  2. Delete the user that was created using the AdminDeleteUser command.
  3. Link the idP account to an existing native (username/password) Cognito user using the AdminLinkProviderForUser command.
  4. Attempt to log in again via Auth.federatedSignIn({ provider }) using the same external idP account.
  5. The fourth step must be done within an hour of the first.

Code Snippet

// Put your code below this line.

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

nadetastic commented 1 year ago

Hi @mtourj thank you for opening this issue. From the flow you have, it sounds like you have provide to option to link the user instead of auto linking with a pre-sign up lambda trigger for example. Additionally, it also sounds like you explicitly delete the copy of user that was create through social sign in, once the two users are linked. In my experience, explicitly deleting shouldn't be necessary, and have a sample function defined here that auto links users using the pre-sign up trigger.

Could you take a look and let me know if you have similar logic for when you are linking users? I'd like to reproduce your scenario but hoping to get as much info as possible.

mtourj commented 1 year ago

Hello @nadetastic, yes the flow you describe is correct.

As far as the sample function you provided, the difference is that after finding the existing user with matching email, we set an attribute on the on the newly created user called "custom:linkable_sub" to the sub of the existing user, and this happens in the post-confirmation trigger.

Then if the user chooses to link accounts, the client makes a call to our backend application which deletes the "newly created" user then links the idP with the existing account via AdminLinkProviderForUser; in that order.

I did originally try to just run AdminLinkProviderForUser without deleting the "newly created" user, but the existence of that Cognito user seems to override the link created and the "identities" attribute on the native user, and as a result it does not log you into the intended account.

nadetastic commented 1 year ago

@mtourj I'm wondering if maybe the order is what is causing this issue - specifically, instead of deleting the "newly created" user then, linking the idP with the existing account via AdminLinkProviderForUser, you do the opposite instead - where you first link the idP with the existing account then delete the new user after linking.

nadetastic commented 1 year ago

Hi @mtourj following up here, were you able to get some head way with this is?

nadetastic commented 1 year ago

@mtourj closing out this issue for now as there hasn't been a response in a while. If you still have questions, let me know.