Amplify-js: Redirect to callback URL occurs twice when using `federatedSignIn()`

[JacksonGariety]

Describe the bug
When using federatedSignIn(), the cognito redirects the browser to the URL specific in the cognito app configuration (the "sign in callback url") at the point in the authorization process where the ?code=blah-blah-blah is passed to the client.

But the redirect occurs again on the frontend on this exact page, because when Amplify finishes exchanging the code for the token, there's another redirect here: https://github.com/aws-amplify/amplify-js/blob/cf8738ac5e7961a163ce158a7383bc27f693475f/packages/auth/src/Auth.ts#L1886

What's more, this redirect occurs _after_ the sign in methods are dispatched to Hub, so that if you want to redirect again after sign in, you can't, because you'll be redirected back to the callback URL a second time.

To Reproduce

To reproduce, just try to do a front-end redirect after completing a social sign in:

// inside of a react-router component
Hub.listen("auth", ({ payload: { event, data } }) => {
  switch (event) {
  case "cognitoHostedUI":
  case "signIn":
    props.history.replace("/");
  }
});

It does work, however, if you use a setTimeout:

setTimeout(() => props.history.replace("/"))

Doing this allows you to do your own redirect from the callback URL (i.e. to a homepage) without being redirected back to the callback page. But, obviously, setTimeout is not at all ideal.

Expected behavior
Redirect should only happen once, or an event should be dispatched after the second redirect so that we can use the history API after signing in.

Environment

 System:
    OS: macOS Mojave 10.14.5
    CPU: (4) x64 Intel(R) Core(TM) i5-5350U CPU @ 1.80GHz
    Memory: 69.27 MB / 8.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 10.17.0 - ~/.nvm/versions/node/v10.17.0/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.11.3 - ~/.nvm/versions/node/v10.17.0/bin/npm
  Browsers:
    Chrome: 79.0.3945.130
    Safari: 12.1.1
  npmPackages:
    @babel/core: 7.7.4 => 7.7.4
    @svgr/webpack: 4.3.3 => 4.3.3
    @typescript-eslint/eslint-plugin: ^2.8.0 => 2.10.0
    @typescript-eslint/parser: ^2.8.0 => 2.10.0
    aws-amplify: ^2.2.2 => 2.2.2
    babel-eslint: 10.0.3 => 10.0.3
    babel-jest: ^24.9.0 => 24.9.0
    babel-loader: 8.0.6 => 8.0.6
    babel-plugin-named-asset-import: ^0.3.5 => 0.3.5
    babel-preset-react-app: ^9.1.0 => 9.1.0
    camelcase: ^5.3.1 => 5.3.1
    case-sensitive-paths-webpack-plugin: 2.2.0 => 2.2.0
    css-loader: 3.2.0 => 3.2.0
    dotenv: 8.2.0 => 8.2.0
    dotenv-expand: 5.1.0 => 5.1.0
    eslint: ^6.6.0 => 6.7.2
    eslint-config-react-app: ^5.1.0 => 5.1.0
    eslint-loader: 3.0.2 => 3.0.2
    eslint-plugin-flowtype: 3.13.0 => 3.13.0
    eslint-plugin-import: 2.18.2 => 2.18.2
    eslint-plugin-jsx-a11y: 6.2.3 => 6.2.3
    eslint-plugin-react: 7.16.0 => 7.16.0
    eslint-plugin-react-hooks: ^1.6.1 => 1.7.0
    esri-loader: ^2.13.0 => 2.13.0
    file-loader: 4.3.0 => 4.3.0
    fs-extra: ^8.1.0 => 8.1.0
    html-webpack-plugin: 4.0.0-beta.5 => 4.0.0-beta.5
    identity-obj-proxy: 3.0.0 => 3.0.0
    jest: 24.9.0 => 24.9.0
    jest-environment-jsdom-fourteen: 0.1.0 => 0.1.0
    jest-resolve: 24.9.0 => 24.9.0
    jest-watch-typeahead: 0.4.2 => 0.4.2
    mini-css-extract-plugin: 0.8.0 => 0.8.0
    optimize-css-assets-webpack-plugin: 5.0.3 => 5.0.3
    pnp-webpack-plugin: 1.5.0 => 1.5.0
    postcss-flexbugs-fixes: 4.1.0 => 4.1.0
    postcss-loader: 3.0.0 => 3.0.0
    postcss-normalize: 8.0.1 => 8.0.1
    postcss-preset-env: 6.7.0 => 6.7.0
    postcss-safe-parser: 4.0.1 => 4.0.1
    react: ^16.12.0 => 16.12.0
    react-app-polyfill: ^1.0.5 => 1.0.5
    react-dev-utils: ^10.0.0 => 10.0.0
    react-dom: ^16.12.0 => 16.12.0
    react-router-dom: ^5.1.2 => 5.1.2
    react-scripts: ^3.3.0 => 3.3.0
    resolve: 1.12.2 => 1.12.2
    resolve-url-loader: 3.1.1 => 3.1.1
    sass-loader: 8.0.0 => 8.0.0
    semver: 6.3.0 => 6.3.0
    style-loader: 1.0.0 => 1.0.0
    terser-webpack-plugin: 2.2.1 => 2.2.1
    ts-pnp: 1.1.5 => 1.1.5
    url-loader: 2.3.0 => 2.3.0
    webpack: 4.41.2 => 4.41.2
    webpack-dev-server: 3.9.0 => 3.9.0
    webpack-manifest-plugin: 2.2.0 => 2.2.0
    workbox-webpack-plugin: 4.3.1 => 4.3.1
  npmGlobalPackages:
    @sitespeed.io/throttle: 0.5.4
    ngrok: 3.2.5
    npm: 6.11.3

[ericclemmons]

@JacksonGariety Thanks for bringing this to our attention. This totally makes sense, and it seems that (since history.replace is synchronous), what we could do to fix it is move our replaceState call _above_ the signIn event:

https://github.com/aws-amplify/amplify-js/blob/cf8738ac5e7961a163ce158a7383bc27f693475f/packages/auth/src/Auth.ts#L1857-L1861

In the meantime, check out this unit test:
https://github.com/aws-amplify/amplify-js/blob/510f86ef39bae3fba9a17deb630237a64c9dc7dc/packages/auth/__tests__/auth-unit-test.ts#L2716-L2761

You should be able to set oauth.redirectSignIn when you configure for this to happen automatically.

Do you have that set currently, but it's not working? Or is it not being set for other reasons?

[ooridoori]

This also happened to me as well - can someone get around to this issue ASAP?

[camin-mccluskey]

Also struggling with this - I want to respond to the sign in event just once but everything I have tried (debouncing, using lodash once etc) has failed to discern between the repeated events.

[JacksonGariety]

@ericclemmons I do have redirectSignIn set to my homepage, but I'd like to redirect the user to different places dynamically after they log in.

[LaszloDev]

Same issue here, the user is redirect back to my application with the ?code=123 param added and I would like to redirect when the authentication is successful.

My suggestion would be to add an option param in the federatedSign() function call to not replace the url and let the us implement our own logic for it.