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

BadRequestError: checks.state argument is missing when routing between pages using browser's back button #625

Open
6 tasks done
link01153113 opened this issue Jul 5, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@link01153113
Copy link

Checklist

Description

I have multiple apps using the same domain with the same login system (Cognito). When navigating directly to a page, there are no issues. However, if I navigate to the first page, then go to the second page, and finally click the browser's back button to return to the first page, I always encounter a BadRequestError.

BadRequestError: checks.state argument is missing
    at ResponseContext.callback (/home/node_modules/express-openid-connect/lib/context.js:354:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

It only happens on v2 express-openid-connect, but not on v1.
App using v2 is able to return to app using v1 without issue using browser's back button. Somehow it only happens in v2.
Is it the desired behavior or am I overlooking something? Thank you for your time.

Reproduction

  1. Login to the portal
  2. Route to the first page
  3. Route to the second page
  4. Click browser's back button to return to the first page.
  5. Error shows up

One of the apps setup:

server.use(
        auth({
            authRequired: true,
            issuerBaseURL: `https://cognito-idp.us-east-1.amazonaws.com/${process.env.USER_POOL_ID}`,
            baseURL,
            clientID: `${process.env.COGNITO_CLIENT_ID}`,
            clientSecret: `${process.env.COGNITO_CLIENT_SECRET}`,
            secret,
            authorizationParams: {
                response_type: "code",
                scope: "openid profile",
            },
            routes: {
                postLogoutRedirect: `https://${process.env.COGNITO_USER_POOL_DOMAIN}/logout?client_id=${process.env.COGNITO_CLIENT_ID}&logout_uri=${baseUrl}`,
            },
            afterCallback: async function (req, res, session, decodedState) {
                try {
                    return session;
                } catch (err) {
                    console.error(
                        "There was a problem after auth callback: ",
                        err
                    );
                    return session;
                }
            },
        })
    );

Additional context

No response

express-openid-connect version

2.17.1

Express version

4.18.2

Node.js version

16

@link01153113 link01153113 added the bug Something isn't working label Jul 5, 2024
@madaster97
Copy link
Contributor

madaster97 commented Jul 6, 2024

Update: Just to be clear, I think the issue is that your apps are overwriting each other's cookies (specifically the auth_verification cookie). The steps below are to segregate each app's cookies onto a specific server path.

Since your 2 apps are on the same domain, does that mean they're each hosted on different paths?

I had to do something similar before, and it was a pain to deal with and requires you to re-mount your routes based on how your reverse proxy forwards requests. A couple things I remember doing:

  1. Configure each app to inject a Path cookie attribute so they get stored separately in the browser (example below). I can't remember if I needed different cookie names as well, but worth testing
  2. Re-mount my app on the /<path> I had just configured. This will depend on how your proxy forwards the requests. Is it stripping out the first path before forwarding to your node server?

Add cookie path config:

server.use(
        auth({
            ...,
            session:{
                name: '<Different name for each app, to avoid cookie name collision>',
                cookie: {
                    path: '<Different path for each app>'
                }
            }
        })
    );

@natewaddoups
Copy link

natewaddoups commented Jul 25, 2024

There might be fewer requests for help with this error message if it was changed from:

"checks.state argument is missing"
to
"checks.state is not present in the auth_verification cookie"

The current error message probably made sense to whoever was writing the function that was invoked with a "checks" object that was missing a "state" property, but it's meaningless to those of us who are using this library, since we didn't call that function ourselves.

I'd submit a PR to fix this, but I can't figure out where the phrase "argument is missing" is coming from. I searched the repo, but apparently it only appears in test cases.

edit: this is now a feature request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants