Having ?code= on the query string seems to always be caught by some Cognito code

[PeteDuncanson]

Before opening, please confirm:

• I have searched for duplicate or closed issues and discussions.
• I have read the guide for submitting bug reports.
• I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

# Put output below this line
  System:
    OS: Windows 10 10.0.19041
    CPU: (16) x64 Intel(R) Core(TM) i7-10875H CPU @ 2.30GHz
    Memory: 11.16 GB / 31.84 GB
  Binaries:
    Node: 14.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.11 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 94.0.4606.81
    Edge: Spartan (44.19041.1266.0), Chromium (95.0.1020.44)
    Internet Explorer: 11.0.19041.1202
  npmPackages:
    @asseinfo/react-kanban: ^2.1.0 => 2.1.0
    @aws-amplify/ui-react: ^1.2.21 => 1.2.21
    @brainhubeu/react-file-input: 0.7.21 => 0.7.21
    @headlessui/react: ^1.2.0 => 1.2.0
    @ideal-postcodes/api-typings: ^2.0.1 => 2.0.1
    @ideal-postcodes/core-browser: ^1.5.2 => 1.5.2
    @sentry/react: ^6.3.0 => 6.3.0
    @sentry/tracing: ^6.2.4 => 6.2.4
    @tailwindcss/aspect-ratio: ^0.2.0 => 0.2.0
    @tailwindcss/forms: ^0.2.1 => 0.2.1
    @tailwindcss/typography: ^0.4.0 => 0.4.0 (0.2.0)
    @tailwindcss/ui: ^0.6.2 => 0.6.2
    @tailwindui/react: ^0.1.1 => 0.1.1
    @testing-library/jest-dom: ^4.2.4 => 4.2.4
    @testing-library/react: ^9.3.2 => 9.5.0
    @testing-library/user-event: ^7.1.2 => 7.2.1
    @types/javascript-time-ago: ^2.0.2 => 2.0.2
    @types/jest: ^27.0.2 => 27.0.2
    @types/node: ^14.14.33 => 14.14.33 (13.9.2, 14.11.2)
    @types/reach__router: ^1.3.7 => 1.3.7
    @types/react: ^16.14.5 => 16.14.5 (16.9.49)
    @types/react-dom: ^16.9.11 => 16.9.11 (16.9.8)
    @types/yup: ^0.29.11 => 0.29.11
    @typescript-eslint/eslint-plugin: ^4.17.0 => 4.17.0 (4.2.0)
    @typescript-eslint/parser: ^4.17.0 => 4.17.0 (4.2.0)
    apollo-boost: ^0.4.9 => 0.4.9
    autoprefixer: ^10.2.5 => 10.2.5 (7.2.6, 6.7.7, 9.7.4)
    aws: ^0.0.3-2 => 0.0.3-2
    aws-amplify: ^4.3.3 => 4.3.3
    aws-amplify-react: ^5.1.4 => 5.1.4
    aws-sdk: ^2.860.0 => 2.860.0
    axios: ^0.21.1 => 0.21.1 (0.21.4, 0.19.0)
    babel-eslint: ^10.1.0 => 10.1.0
    chart.js: ^3.2.0 => 3.2.0
    chart.js-auto:  undefined ()
    chart.js-helpers:  undefined ()
    eslint: ^7.21.0 => 7.21.0 (7.32.0)
    eslint-config-react-app: ^6.0.0 => 6.0.0
    file-saver: ^2.0.5 => 2.0.5
    formik: ^2.2.6 => 2.2.6
    google-map-react: ^2.1.10 => 2.1.10
    graphiql: ^1.4.0 => 1.4.0
    graphql: 15.5.1 => 15.5.1 (14.0.0, 14.7.0)
    graphql-auto-transformer: ^1.3.1 => 1.3.1
    graphql-tag: ^2.12.5 => 2.12.5 (2.11.0)
    history: 5 => 5.0.0 (4.10.1)
    postcode: ^5.1.0 => 5.1.0
    postcss: ^8.2.8 => 8.2.8 (6.0.23, 7.0.27, 5.2.18, 8.2.4, 8.3.11, 7.0.39, 7.0.36)
    react: ^17.0.2 => 17.0.2 (16.14.0)
    react-apollo: ^3.1.5 => 3.1.5
    react-chartjs-2: ^3.0.2 => 3.0.2
    react-datepicker: ^3.4.1 => 3.4.1
    react-dom: ^16.14.0 => 16.14.0
    react-icons: ^4.2.0 => 4.2.0
    react-pagination-hook: ^0.0.1 => 0.0.1
    react-router-dom: ^6.0.0-beta.8 => 6.0.0-beta.8 (4.3.1)
    react-scripts: 4.0.3 => 4.0.3
    react-tag-autocomplete: ^6.1.0 => 6.1.0
    react-three-state-checkbox: ^1.3.4 => 1.3.4
    react-time-ago: ^6.2.2 => 6.2.2
    react-use: ^15.3.8 => 15.3.8
    rimraf: ^3.0.2 => 3.0.2 (2.7.1)
    smoothscroll-polyfill: ^0.4.4 => 0.4.4
    tailwind: ^4.0.0 => 4.0.0
    tailwindcss: ^2.0.3 => 2.0.3
    typescript: ^4.2.3 => 4.2.3
    vest: ^2.2.3 => 2.2.3
    yup: ^0.29.3 => 0.29.3
  npmGlobalPackages:
    @aws-amplify/cli: 6.4.0
    nodemon: 2.0.7
    ts-node: 9.1.1
    yarn: 1.22.10

Describe the bug

This is an odd one.

We had a callback that we needed for Xero (3rd party accountancy software) authentication which wants to send us a code on the querystring such as /mycallbackurl/?code=123454&secret=56789

Trouble is our App would always redirect to the root url and our code to handle the callback would never fire! After some testing we found that its just the querystring name of "code" that is causing the issue. We suspect that our Cognito Authentication is being too eager and hoovering up anything with "code" in the querystring regardless of the url structure.

To work around it and prove the point I added this to the top of my index.js in the root of the App before anything else can run:

// The xero hack, for some reason if we pass ?code= on the query string it causes a redirect to the root url /
// This breaks the xero callback url. After many hours searching we came up with this hack, we intercept it and rename it on the querystring!
const qs = window.location.search;
console.info("QS", qs);
if (window.location.pathname.indexOf("xero") > -1 && qs.indexOf("code=") > -1) {
  window.location.search = qs.replace("code", "xeroCode");
}
console.info(
  "Updated querystring to avoid the xero code quirk",
  window.location.search
);

Its not pretty, but it renames the code querystring param to something else if we know its for Xero. Then in our xero callback handling code we look for the new value. This now works! But its damn weird and something we thought would catch out others. I'll include our UserProvider at the bottom so you can see how we are using it for the Authentication if it helps. The above code though is the only bit of "quirky" stuff we have in there.

Expected behavior

I'd have thought that the Cognito code would only kick in on whatever callback url it wants to you (yes this could even be the root url) rather than on everything. Seems a bit too much of a over-reach of the querystring for me.

Reproduction steps

This is hard to unpick, if you have an authed site already then you could wrap our UserProvider around it and see if you can get that running.

Then navigate to a sub folder (ie /myfolder/) and try adding ?code=1234 to the end. The site should redirect/reload at the root url and your querystring will have disappeared. Yet if you go back to the same folder but add ?code2=1234 to the end it won't redirect and will instead just render and leave the querystring on the url.

Code Snippet

// Put your code below this line.
import { CognitoUser } from "@aws-amplify/auth";
import { CognitoHostedUIIdentityProvider } from "@aws-amplify/auth/lib/types";
import { CognitoUserSession } from "amazon-cognito-identity-js";
import Amplify, { Auth } from "aws-amplify";
import React, { useCallback } from "react";


export type UserContextType = {
  /** This is the user model from a successful Login */
  cognitoUser: CognitoUser;
  /** The global user ID used for BOTH cognito pool and the DynamoDB User.id field */
  globalUserID: string;
  isCognitoLoading: boolean;

  cognitoUserSession: CognitoUserSession | null;

  login: Function;
  googleLogin: Function;
  logout: Function;
  isAdmin: Function;
};

// Create a context that will hold the values that we are going to expose to our components.
export const UserContext = React.createContext<UserContextType | undefined>(
  undefined
);

// Create a "controller" component that will calculate all the data that we need to give to our components bellow via the `UserContext.Provider` component.
// This is where the Amplify will be mapped to a different interface, the one that we are going to expose to the rest of the app.
export const UserProvider = ({ children }) => {
  const [cognitoUser, setCognitoUser] = React.useState(null) as
    | CognitoUser
    | any;
  const [cognitoUserSession, setCognitoUserSession] =
    React.useState<CognitoUserSession | null>(null);
  const [isCognitoLoading, setIsCognitoLoading] = React.useState<boolean>(true);

  // console.count("UserProvider");

  React.useEffect(() => {
    // Configure the keys needed for the Auth module
    Auth.configure({});

    // attempt to fetch the info of the user that was already logged in
    Auth.currentAuthenticatedUser()
      .then((user) => {
        setCognitoUser(user);
        Auth.currentSession().then((session) => {
          setCognitoUserSession(session);
        });
      })
      .catch(() => {
        setCognitoUser(null);
      })
      .finally(() => setIsCognitoLoading(false));
  }, [setCognitoUser]);

  // We make sure to handle the user update here, but return the resolve value in order for our components to be  able to chain additional `.then()` logic.
  // Additionally, we `.catch` the error and "enhance it" by providing a message that our React components can use.
  const login = useCallback(
    (usernameOrEmail: string, password: string) => {
      Auth.signIn(usernameOrEmail, password)
        .then((cognitoUser) => {
          setCognitoUser(cognitoUser);
          Auth.currentSession().then((session) => {
            setCognitoUserSession(session);
          });
          return cognitoUser;
        })
        .catch((err) => {
          if (err.code === "UserNotFoundException") {
            err.message = "Invalid username or password";
          }
          throw err;
        })
        .finally(() => setIsCognitoLoading(false));
    },
    [setCognitoUser]
  );

  const googleLogin = useCallback(() => {
    Auth.federatedSignIn({ provider: CognitoHostedUIIdentityProvider.Google })
      .then((cognitoUser) => {
        setCognitoUser(cognitoUser);
        Auth.currentSession().then((session) => {
          setCognitoUserSession(session);
        });
        return cognitoUser;
      })
      .catch((err) => {
        if (err.code === "UserNotFoundException") {
          err.message = "Invalid username or password";
        }
        throw err;
      })
      .finally(() => setIsCognitoLoading(false));
  }, [setCognitoUser]);

  const logout = useCallback(() => {
    Auth.signOut()
      .then((data) => {
        setCognitoUser(null);
        setCognitoUserSession(null);
        // We should clear out the DataStore incase we have different users using the same machine
        Amplify.DataStore.clear();
        return data;
      })
      .finally(() => setIsCognitoLoading(false));
  }, [setCognitoUser, setCognitoUserSession, setIsCognitoLoading]);

  const isAdmin = useCallback(() => {
    let isAdmin = false;
    if (cognitoUserSession) {
      const groups =
        cognitoUserSession.getAccessToken().payload["cognito:groups"];
      if (groups.includes("admin")) {
        isAdmin = true;
      }
    }

    return isAdmin;
  }, [cognitoUserSession]);

  /** Helper field to wrap up the logic for getting the current users ID easier */
  const globalUserID: string = cognitoUser?.attributes?.sub ?? "";

  // Make sure to not force a re-render on the components that are reading these values, unless the `user` value has changed.
  // This is an optimisation that is mostly needed in cases where the parent of the current component re-renders and thus the current component is forced to re-render as well.
  // If it does, we want to make sure to give the `UserContext.Provider` the same value as long as the user data is the same.
  // If you have multiple other "controller" components or Providers above this component, then this will be a performance booster.
  const values = React.useMemo(
    () =>
      ({
        cognitoUser,
        globalUserID,
        isCognitoLoading,
        cognitoUserSession,
        login,
        googleLogin,
        logout,
        isAdmin,
      } as UserContextType),
    [
      cognitoUser,
      globalUserID,
      isCognitoLoading,
      cognitoUserSession,
      googleLogin,
      isAdmin,
      logout,
      login,
    ]
  );

  // Finally, return the interface that we want to expose to our other components
  return <UserContext.Provider value={values}>{children}</UserContext.Provider>;
};

// We also create a simple custom hook to read these values from.
// We want our React components to know as little as possible on how everything is handled
// so we are not only abtracting them from the fact that we are using React's context, but we also skip some imports.
export const useCognitoUser = () => {
  const context = React.useContext(UserContext);

  if (context === undefined) {
    throw new Error(
      "`useCognitoUser` hook must be used within a `UserProvider` component"
    );
  }
  return context;
};

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

[ashika01]

@PeteDuncanson thanks for bringing this up. We will get on triaging this. While we do that, I just wanted to make a callout, the reason we might be looking for ?code instead of what you are asking for in expected behavior would be for 3rd party /federated auth use cases. We gotta look deeper on this since we understand weirdness in implementation you are doing to get around.

[thejuan]

Having the same issue with gsuite auth enabled. This is a regression of a previous issue that was fixed, can't find it currently but will add if i do.

[kieran-bellew]

Having the same issue with bitbucket oauth. I downgraded to latest-3 in the meantime. Hopefully this is fixed soon.

[ashika01]

@thejuan @kieran-bellew When did u start seeing the issue ? Could you point what upgrade triggered this?

[colehendo]

@ashika01 (I'm working with @kieran-bellew) we first saw it after upgrading aws-amplify/auth from 1.6.3 -> 4.3.16 and aws-amplify/core from 1.3.3 -> 4.3.8. After rolling back both packages to 3.4.34 and 3.8.24 respectively we stopped seeing the issue. The issue we encountered was not with ?code=foo in our path, but rather #auth_token=foo. It caused the same behavior described by @PeteDuncanson. Oddly enough, when switched to #auth-token=foo we no longer saw the issue.

[ashika01]

@colehendo Thank you for the info. Let me look into it and get back to you.

Have you tried any version 4.x.x to see if it works?

[ashika01]

Also, @colehendo if u have a sample running out in github, point it out here. Helps speed up debugging :)

[colehendo]

@ashika01 I don't have the code in github, but I can describe our process.

We have a component on our site which requires Bitbucket authentication to access. The user clicks on the link in our site (built using Angular), and get rerouted to https://bitbucket.org/site/oauth2/authorize?client_id=<CLIENT_ID>&response_type=token.

That url then automatically routes them back to https://my-site.com/bitbucket-redirect#auth_token=12345. On that redirect back to our site, the url with the /bitbucket-redirect path is displayed in the browser. Before the Angular component associated with the bitbucket-redirect path can even construct (which pulls and stores the value associated with auth_token), the user is redirected to the root path at https://my-site.com/

[hkjpotato]

@PeteDuncanson I am trying to reproduce the issue, I would like to share somethings I found so far:

1. It seems it depends on how we setup the routing. If I use react-router hash, I do not see the "code" handler triggered for this URL http://localhost:8080/#callback?code=100, but if there is an attempt to reroute /callback back to the index.html. Then the code handler will be triggered.
2. Several comments mentioned older version of Amplify does not have this issue. I will try to see what is the diff in between. I don't have full context, but the change from 3.8.24 to the current one seems necessary for react-native use case (e.g. 8cc5587). The concern is about not being able to uniquely identify the "correct" callback URL that should trigger the cognito code handler.
3. I will keep updating here, but you can also find me on amplify discord as hkjpotato.

[hkjpotato]

Potentially related to #8166, but overall this working code flow needs careful thinking as we also need to support react native use case. Will do more study on this.

[filol]

Hello @hkjpotato any news about this issue ?

[zenpi-me]

Hello,
we are having a very similar issue. For us it's happening when we have a ?error=anything_here in the url. I also tried ?code= and also got the redirect.
We are using aws-amplify 4.3.12.

Any news on this? For now I would tackle the error and try to solve it.

[pkalisiewicz]

hey, any updates on this?

[mfranceschit]

Still broken on aws-amplify@4.3.27

[ghost]

We got the same issue with the lastest version.

Anyone can help to merge this PR?
#9370

Thanks

[timharsch]

It's worth mentioning that this issue causes problems for apps that use Cognito and SMART on FHIR together. e.g. the library https://github.com/smart-on-fhir/client-js, which relies heavily on the code parameter.

I believe this to be the commit that causes the issue:
d4f8fe6

[ghost]

I'm facing the same problem, any news on where this #9370 will be merge?

[abdallahshaban557]

Just sent a followup to the PR creator to see if they can make updates based on recommendations from our team.

[menendezjaume]

Same problem here.

Since cognito doesn't allow me to choose the scopes from Google, I ended up scrapping third party auth altogether. However, once you are in the app, there is the option to "sync your calendar". The Google auth workflow returns the code keyword and cognito picks it up, despite the fact I don't have third party auth enabled.

[timharsch]

@abdallahshaban557 How long to wait before it's safe to assume the original PR creator will not see it through? It's a pretty serious issue, is there someone on the core team that can start the ball rolling again? This bug has been in affect for more than a year now.. (since this commit d4f8fe6)

[abdallahshaban557]

Hi @timharsch - I am asking our team to check what needs to be done to fix this further. We will get back to you when we have more information. FYI @nadetastic @tannerabread @cwomack

[wjureczka]

it is a kinda big deal for us, how to prevent Cognito from changing URLs that have a code query param?

[timharsch]

For reference, this is the commit responsible for the issue. d4f8fe6

I tried raising this issue with our AWS account manager, who had a solutions architect try to get the status from the core team. I was told that the response was that the issue is slated to be fixed in V6 release. I don't put a lot of faith in that response though because I also asked that someone from the team update this issue with a milestone label, and update the community.

[abdallahshaban557]

Hi @timharsch - we have this issue high on our tracking list for v6, we understand the change needed, but do not want to break existing customers. We will update this issue when we have an ETA on our release! We are taking the concerns of our community to heart on resolving this pain! Please let us know if you have any further questions/concerns.

[alexkates]

Here is my work around in the meanwhile.

/**
 * This is a hack to fix the issue with the Amplify library treating all incoming urls as OAuth responses.
 * https://github.com/aws-amplify/amplify-js/issues/9208#issuecomment-1309890756*
 */
// @ts-ignore
const _handleAuthResponse = Auth._handleAuthResponse.bind(Auth);
// @ts-ignore
Auth._handleAuthResponse = (url: string) => {
  const configuration = Auth.configure();
  // @ts-ignore
  if (!url.includes(configuration.oauth?.redirectSignIn)) return;
  return _handleAuthResponse(url);
};

Amplify.configure(awsExports);

[ConorFl]

Any updates on this? This is still a pretty big issue.

Also, FYI, if you're using the workaround proposed by @alexkates, the latest version it works on is 5.3.6, afterwards Auth.ts seems to have been completely refactor so it doesn't work.

[alexkates]

I'm still on version 5.0.24

[nadetastic]

The developer preview for v6 of Amplify has officially been released with improvements Authentication and much more! Please check out our announcement and updated documentation to see what has changed.

This issue should be resolved within the dev preview and upcoming General Availability for Amplify v6, but let us know with a comment if there are further issues.

Please note that the method has changed from Auth.federatedSignIn() to signInWithRedirect()

[PeteDuncanson]

For those following, I'd recommend checking the upgrade docs https://docs.amplify.aws/javascript/build-a-backend/troubleshooting/migrate-from-javascript-v5-to-v6/

[Bar-Security]

I have migrated to v6 and this bug is still happening.
All query params are being removed, not just ?code=.
Because the new way functions are imported in v6 (i.e. import { } from 'aws-amplify/auth') I can't use the hacks provided here to override Auth._handleAuthResponse

[nadetastic]

HI @Bar-Security do you mind opening a new issue with details and context so I may be able to better assist you?

[timharsch]

Our team has found that upgrading to v6 is a non-trivial task for us. We first need to make our whole app webpack 5 compatible before we can use v6, since v6 now has a dependency on webpack 5 (

amplify-js/package.json

Line 119 in b092100

"webpack": "^5.75.0",

). Is it possible to take the fix from v6 and backport it to v5? Can you point to the code, and/or tests, or maybe the PR, in v6 that solves the issue?

[aprilmintacpineda-ive]

Issue already closed but fix still not released. Why?

[timharsch]

@nadetastic please reopen the issue. The problem persists..

[cwomack]

Reopening this issue as we investigate this further on both v5 and v6.

[nadetastic]

Hi @timharsch, cc @Bar-Security

We are still unable to reproduce this issue with v6 of aws-amplify - here are the reproduction steps we have followed:

1. Begin signIn flow using signInWithRedirect()
2. Authenticate with google successfully and get redirected back to the app and establish a user session
3. Manually add /?code=1234 to app's url which remains and isn't swallowed

Since you are still seeing this issue with v6, do you mind opening a new issue with additional context and reproduction steps in order to better assist you?

[shavez-btl]

I could also reproduce the behaviour mentioned by @Bar-Security and have opened a fresh issue as suggested by @nadetastic with more details: #13096

[ashwinkumar6]

Hi @timharsch,
Wanted to check if you're still blocked on upgrading to V6 to tackle this issue. Back porting the fix from V6 to V5 would result in a breaking change.

"webpack": "^5.75.0" is a devDependency in V6. Could you please elaborate more on the blocker you're facing

[timharsch]

Hi @timharsch, Wanted to check if you're still blocked on upgrading to V6 to tackle this issue. Back porting the fix from V6 to V5 would result in a breaking change.

"webpack": "^5.75.0" is a devDependency in V6. Could you please elaborate more on the blocker you're facing

For our app, we have other packages that would not be happy if we try to change the webpack version, which, currently is preventing us from upgrading to v6 of Amplify. Right now, my dev that needs to upgrade us to v6 has been on other critical tasks, I'm hoping to get some his time for this in next couple of weeks.

We did not try nate's test sequence unfortunately, because we are not on v6, but looking at those steps I'm not sure how they could validate that it works. What good would it do to add /code=1234 manually after the redirect? It's in the redirect process that the param is dropped isn't it? Do you have an automated test that could validate the expectations?

[ashwinkumar6]

For our app, we have other packages that would not be happy if we try to change the webpack version, which, currently is preventing us from upgrading to v6 of Amplify. Right now, my dev that needs to upgrade us to v6 has been on other critical tasks, I'm hoping to get some his time for this in next couple of weeks.
We did not try nate's test sequence unfortunately, because we are not on v6, but looking at those steps I'm not sure how they could validate that it works. What good would it do to add /code=1234 manually after the redirect? It's in the redirect process that the param is dropped isn't it? Do you have an automated test that could validate the expectations?

V5 supports a global urlListener where, when oauth is configured and the url contains code, error or access_token query params it assumes it's redirected from HostedUI and proceeds to fetch tokens and credentials.
(even if there isn't an oauth in flight)

To test this flow in V5

1. Configure oauth (with Amplify.configure())
2. Manually update the url to contains code, error or access_token query params
3. Notice these params consumed

This reason this was designed in this manner was to support:

• ServiceProvider-initiated (initiated from with-in the app example Auth.federatedSignIn())
• IdentityProvider-initiated (initiated externally by identity provider)

However V6 architecture is different, it does not contain a global urlListener and hence this issue does not exist