Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query strings always consumed by Amplify Auth after invoking getCurrentUser() #13096

Closed
3 tasks done
shavez-btl opened this issue Mar 7, 2024 · 7 comments
Closed
3 tasks done
Assignees
Labels
Auth Related to Auth components/category question General question

Comments

@shavez-btl
Copy link

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

Amplify CLI

Environment information

# Put output below this line

System:
    OS: Linux 6.7 Arch Linux
    CPU: (12) x64 AMD Ryzen 5 5500U with Radeon Graphics
    Memory: 3.20 GB / 14.98 GB
    Container: Yes
    Shell: 5.9 - /usr/bin/bash
  Binaries:
    Node: 16.20.1 - ~/.nvm/versions/node/v16.20.1/bin/node
    Yarn: 1.22.21 - /usr/bin/yarn
    npm: 8.19.4 - ~/.nvm/versions/node/v16.20.1/bin/npm
  Browsers:
    Chromium: 122.0.6261.94
  npmPackages:
    @ckeditor/ckeditor5-build-classic: ^32.0.0 => 32.0.0 
    @ckeditor/ckeditor5-react: ^4.0.0 => 4.0.1 
    @emotion/styled: ^11.10.6 => 11.10.6 
    @fullcalendar/bootstrap: ^5.11.0 => 5.11.0 
    @fullcalendar/daygrid: ^5.11.0 => 5.11.0 
    @fullcalendar/interaction: ^5.11.0 => 5.11.0 
    @fullcalendar/list: ^5.11.0 => 5.11.0 
    @fullcalendar/react: ^5.11.1 => 5.11.1 
    @lourenci/react-kanban: ^2.1.0 => 2.1.0 
    @mui/material: ^5.12.0 => 5.12.0 
    @react-oauth/google: ^0.12.1 => 0.12.1 
    @simonwep/pickr: ^1.8.2 => 1.8.2 
    @stripe/react-stripe-js: ^2.4.0 => 2.4.0 
    @stripe/stripe-js: ^2.2.0 => 2.2.0 
    @tanstack/react-table: ^8.7.9 => 8.7.9 
    @testing-library/jest-dom: ^5.16.2 => 5.16.4 
    @testing-library/react: ^12.1.3 => 12.1.5 
    @testing-library/user-event: ^13.5.0 => 13.5.0 
    @vtaits/react-color-picker: ^0.1.2 => 0.1.2 
    amazon-cognito-identity-js: ^5.2.9 => 5.2.9 
    aos: ^2.3.4 => 2.3.4 
    apexcharts: ^3.33.1 => 3.35.3 
    aws-amplify: 6.0.18 => 6.0.18 
    aws-amplify/adapter-core:  undefined ()
    aws-amplify/analytics:  undefined ()
    aws-amplify/analytics/kinesis:  undefined ()
    aws-amplify/analytics/kinesis-firehose:  undefined ()
    aws-amplify/analytics/personalize:  undefined ()
    aws-amplify/analytics/pinpoint:  undefined ()
    aws-amplify/api:  undefined ()
    aws-amplify/api/server:  undefined ()
    aws-amplify/auth:  undefined ()
    aws-amplify/auth/cognito:  undefined ()
    aws-amplify/auth/cognito/server:  undefined ()
    aws-amplify/auth/enable-oauth-listener:  undefined ()
    aws-amplify/auth/server:  undefined ()
    aws-amplify/datastore:  undefined ()
    aws-amplify/in-app-messaging:  undefined ()
    aws-amplify/in-app-messaging/pinpoint:  undefined ()
    aws-amplify/push-notifications:  undefined ()
    aws-amplify/push-notifications/pinpoint:  undefined ()
    aws-amplify/storage:  undefined ()
    aws-amplify/storage/s3:  undefined ()
    aws-amplify/storage/s3/server:  undefined ()
    aws-amplify/storage/server:  undefined ()
    aws-amplify/utils:  undefined ()
    axios: ^1.6.7 => 1.6.7 
    axios-mock-adapter: ^1.20.0 => 1.21.1 
    babel-plugin-prismjs: ^2.1.0 => 2.1.0 
    bootstrap: ^5.1.3 => 5.1.3 
    chart.js: ^2.9.4 => 2.9.4 
    cleave.js: ^1.6.0 => 1.6.0 
    country-list: ^2.3.0 => 2.3.0 
    currency-symbol-map: ^5.1.0 => 5.1.0 
    echarts: ^5.3.0 => 5.3.3 
    echarts-for-react: ^3.0.2 => 3.0.2 
    eslint: ^8.9.0 => 8.18.0 
    feather-icons-react: ^0.5.0 => 0.5.0 
    filepond: ^4.30.3 => 4.30.4 
    filepond-plugin-image-exif-orientation: ^1.0.11 => 1.0.11 
    filepond-plugin-image-preview: ^4.6.10 => 4.6.11 
    firebase: ^10.8.1 => 10.8.1 
    firebase/analytics:  undefined ()
    firebase/app:  undefined ()
    firebase/app-check:  undefined ()
    firebase/auth:  undefined ()
    firebase/auth/cordova:  undefined ()
    firebase/auth/web-extension:  undefined ()
    firebase/compat:  undefined ()
    firebase/compat/analytics:  undefined ()
    firebase/compat/app:  undefined ()
    firebase/compat/app-check:  undefined ()
    firebase/compat/auth:  undefined ()
    firebase/compat/database:  undefined ()
    firebase/compat/firestore:  undefined ()
    firebase/compat/functions:  undefined ()
    firebase/compat/installations:  undefined ()
    firebase/compat/messaging:  undefined ()
    firebase/compat/performance:  undefined ()
    firebase/compat/remote-config:  undefined ()
    firebase/compat/storage:  undefined ()
    firebase/database:  undefined ()
    firebase/firestore:  undefined ()
    firebase/firestore/lite:  undefined ()
    firebase/functions:  undefined ()
    firebase/installations:  undefined ()
    firebase/messaging:  undefined ()
    firebase/messaging/sw:  undefined ()
    firebase/performance:  undefined ()
    firebase/remote-config:  undefined ()
    firebase/storage:  undefined ()
    formik: ^2.2.9 => 2.2.9 
    google-maps-react: ^2.0.6 => 2.0.6 
    gridjs: ^5.0.2 => 5.0.2 
    gridjs-react: ^5.0.2 => 5.0.2 
    i18next: ^21.6.11 => 21.8.11 
    i18next-browser-languagedetector: ^6.1.3 => 6.1.4 
    js-cookie: ^3.0.1 => 3.0.1 (3.0.5, 2.2.1)
    js-sha256: ^0.10.1 => 0.10.1 
    leaflet: ^1.7.1 => 1.8.0 
    list.js: ^2.3.1 => 2.3.1 
    lord-icon-element: ^3.3.3 => 3.4.2 
    lottie-web: ^5.8.1 => 5.9.5 
    material-ui-slider: ^3.0.8 => 3.0.8 
    mdb-react-ui-kit: ^5.1.0 => 5.1.0 
    node-sass: ^9.0.0 => 9.0.0 
    nouislider-react: ^3.4.1 => 3.4.1 
    prismjs: ^1.26.0 => 1.28.0 
    quill: ^1.3.7 => 1.3.7 
    react: ^18.1.0 => 18.2.0 
    react-apexcharts: ^1.4.0 => 1.4.0 
    react-chartjs-2: ^2.11.1 => 2.11.2 
    react-color: ^2.19.3 => 2.19.3 
    react-countdown: ^2.3.2 => 2.3.2 
    react-countup: ^6.1.1 => 6.3.0 
    react-data-table-component: ^7.5.2 => 7.5.2 
    react-dom: ^18.0.0 => 18.2.0 
    react-drag-listview: ^0.1.9 => 0.1.9 
    react-dragula: ^1.1.17 => 1.1.17 
    react-dropzone: ^12.0.1 => 12.1.0 
    react-dual-listbox: ^2.2.0 => 2.2.0 
    react-facebook-login: ^4.1.1 => 4.1.1 
    react-filepond: ^7.1.1 => 7.1.2 
    react-flatpickr: ^3.10.7 => 3.10.13 
    react-hot-toast: ^2.2.0 => 2.2.0 
    react-i18next: ^11.15.3 => 11.17.3 
    react-image-lightbox: ^5.1.4 => 5.1.4 
    react-jvectormap: ^0.0.16 => 0.0.16 
    react-leaflet: ^3.2.5 => 3.2.5 
    react-masonry-component: ^6.3.0 => 6.3.0 
    react-masonry-css: ^1.0.16 => 1.0.16 
    react-meta-tags: ^1.0.1 => 1.0.1 
    react-perfect-scrollbar: ^1.5.8 => 1.5.8 
    react-phone-input-2: ^2.15.0 => 2.15.0 
    react-phone-number-input: ^3.2.3 => 3.2.3 
    react-phone-number-input/commonjs:  undefined ()
    react-phone-number-input/core:  undefined ()
    react-phone-number-input/flags:  undefined ()
    react-phone-number-input/input-core:  undefined ()
    react-phone-number-input/input-max:  undefined ()
    react-phone-number-input/input-min:  undefined ()
    react-phone-number-input/input-mobile:  undefined ()
    react-phone-number-input/max:  undefined ()
    react-phone-number-input/min:  undefined ()
    react-phone-number-input/mobile:  undefined ()
    react-phone-number-input/react-hook-form:  undefined ()
    react-phone-number-input/react-hook-form-core:  undefined ()
    react-phone-number-input/react-hook-form-input:  undefined ()
    react-phone-number-input/react-hook-form-input-core:  undefined ()
    react-phone-number-input/react-native-input:  undefined ()
    react-quilljs: ^1.2.17 => 1.3.0 
    react-rangeslider: ^2.2.0 => 2.2.0 
    react-rating: ^2.0.5 => 2.0.5 
    react-redux: ^7.2.8 => 7.2.8 
    react-responsive-carousel: ^3.2.22 => 3.2.23 
    react-router: ^6.17.0 => 6.17.0 
    react-router-dom: ^6.17.0 => 6.17.0 
    react-scripts: 5.0.0 => 5.0.0 
    react-scrollspy: ^3.4.3 => 3.4.3 
    react-select: ^5.2.2 => 5.3.2 
    react-shepherd: ^3.3.6 => 3.3.7 
    react-switch: ^6.0.0 => 6.1.0 
    react-table: ^7.8.0 => 7.8.0 
    react-tag-input: ^6.8.1 => 6.8.1 
    react-toastify: ^8.1.1 => 8.2.0 
    react-tsparticles: ^1.40.0 => 1.43.1 
    reactstrap: ^9.0.1 => 9.1.1 
    redux: ^4.1.2 => 4.2.0 
    redux-saga: ^1.1.3 => 1.1.3 
    redux-saga/effects:  undefined ()
    remixicon-react: ^1.0.0 => 1.0.0 
    save: ^2.4.0 => 2.5.0 
    semantic-ui-react: ^2.1.4 => 2.1.4 
    simplebar-react: ^2.3.6 => 2.4.1 
    styled-components: ^6.1.8 => 6.1.8 
    styled-components/native:  undefined ()
    swiper: ^8.0.6 => 8.2.5 
    swiper_angular:  0.0.1 
    validator: ^13.7.0 => 13.7.0 
    web-vitals: ^2.1.4 => 2.1.4 
    yup: ^0.32.11 => 0.32.11 
  npmGlobalPackages:
    corepack: 0.17.0
    npm: 8.19.4

Describe the bug

I am trying to implement Outh2 Authorization Code flow for getting user consent to access Microsoft account. When requesting the Authorization Code, microsoft redirects to my app with the query string "code". This query string is caught by aws-amplify/auth.

Adding any query string to the url is consumed by Amplify.

If I remove the getCurrentUser() function which is invoked whenever auth protected routes are opened, the issue goes away. So it seems like getCurrentUser() is triggering some code that then listens for all query parameters.

Also, pages not protected by auth don't have this issue at all.

It is important to note that I am not using signInWithRedirect(), so there is no reason for Amplify to listen for query strings.

Expected behavior

Amplify shouldn't consume query strings

Reproduction steps

  1. Setup a React application with Amplify username, password auth.
  2. Setup auth protected routes using react-router-dom and use getCurrentUser() function from aws-amplify/auth.
  3. Open a protected route and then append query string to the URL

Code Snippet

// This is the code I use to check for Authentication before allowing a user to navigate to an auth protected route.

import { getCurrentUser } from "aws-amplify/auth";

const AuthProtected = (props) => {

  let [authenticated, setAuthenticated] = useState(null);

  // If authenticated is null, then request amplify sdk to provide authentication state
  if (authenticated == null) {
    getCurrentUser()
      .then((user) => {
        setAuthenticated(true);
      })
      .catch((e) => setAuthenticated(false));

    return <p>Loading...</p>;
  }

 // Go to login page because user not authenticated
  if (!authenticated) {
    return (
      <Navigate to={{ pathname: "/login", state: { from: props.location } }} />
    );
  }

  // Go to the requested page
  return <>{props.children}</>;
};

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

@shavez-btl shavez-btl added the pending-triage Issue is pending triage label Mar 7, 2024
@nadetastic nadetastic self-assigned this Mar 7, 2024
@nadetastic nadetastic added Auth Related to Auth components/category investigating This issue is being investigated labels Mar 7, 2024
@nadetastic
Copy link
Member

HI @shavez-btl thank you for opening this issue, I've been doing some testing here to try and reproduce this issue however the ?code=foobar param remains in my url even when calling getCurrentUser.

Can you provide a bit more context, specifically on how your router is structured and when/how you are calling <AuthProtected /> ?

Also have you tested by directly going to a route and manually appending the code?=...?

@shavez-btl
Copy link
Author

shavez-btl commented Mar 8, 2024

Can you provide a bit more context, specifically on how your router is structured and when/how you are calling?

My application is quite simple and it goes like this:

<App>

  <Routes>
        //  Unauthenticated routes
        <Route>
          {publicRoutes.map((route, idx) => (
            <Route
              path={route.path}
              element={
                 <NonAuthLayout>
                      {route.component}
                 </NonAuthLayout>
              }
              key={idx}
              exact={true}
            />
          ))}
        </Route>

        // Authenticated routes
        <Route>
          {authProtectedRoutes.map((route, idx) => (
            <Route
              path={route.path}
              element={
                <AuthProtected>
                  <VerticalLayout showFooter={route.showFooter ?? true}>
                    {route.component}
                  </VerticalLayout>
                </AuthProtected>
              }
              key={idx}
              exact={true}
            />
          ))}
        </Route>

  </Routes>

</App>

In this publicRoutes and authProtectedRoutes are just a simple array of maps to the respective components. For eg:

const publicRoutes = [
  { path: "/logout", component: <Logout /> },
  { path: "/login", component: <Login /> },
  { path: "/forgot-password", component: <ForgetPasswordPage /> },
];
const authProtectedRoutes = [
  { path: "/dashboard", component: <DashboardEcommerce /> },
  { path: "/subscription", component: <Subscription /> },
  { path: "/recommendations", component: <RecommendationPage /> },
  //Error
  { path: "*", component: <Error404 /> },

  {
    path: "/",
    exact: true,
    component: <Navigate to="/dashboard" />,
  }
]

Also have you tested by directly going to a route and manually appending the code?=...?

Yes, I have tested and amplify consumes the query string. In fact this is valid for any query string and not just code.

@timharsch
Copy link

I'd like to suggest that once a fix is found for this issue that it be backported to v5 if possible. Our team, and I assume perhaps others, can't easily upgrade to v6 due to the webpack version change.

@shavez-btl
Copy link
Author

Hi @timharsch, would it be possible for you to test if the issue goes away if you remove getCurrentUser()? For me the issue is present on all auth protected routes and removing call to getCurrentUser() fixes the issue.

@chrisbonifacio
Copy link
Member

chrisbonifacio commented Mar 12, 2024

@shavez-btl Looking at the source code for getCurrentUser, it doesn't seem to do anything that would remove query strings from the url 🤔

@chrisbonifacio
Copy link
Member

chrisbonifacio commented Mar 12, 2024

You mentioned removing the getCurrentUser call fixed the issue for you but I'm curious if the issue was getCurrentUser or the re-rendering it was triggering of the nested component when changing the state of authenticated.

Have you tried testing to see if setting state in AuthProtected without calling getCurrentUser causes the behavior as nested components re-render?

How are you navigating the user to protected pages with query strings? Is it from one page to another, updating the url from the same page, etc?

@shavez-btl
Copy link
Author

shavez-btl commented Mar 13, 2024

Fixed it! Turns out getCurrentUser() was always ending in error the first time as there was an issue in how we were configuring Amplify.

Since it was ending in error, the app was redirecting user to /login resulting in the query string getting lost. In the login page getCurrentUser() was called again but this time it worked because Amplify was configured correctly now.

I think some of us are facing this issue because there was a change in how Amplify is configured in v6.

I am closing this issue for now.

@cwomack cwomack added question General question and removed investigating This issue is being investigated pending-triage Issue is pending triage labels Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auth Related to Auth components/category question General question
Projects
None yet
Development

No branches or pull requests

5 participants