Session cookie modified before session is truly started

[barrelmaker97]

Right now, after a user logs out, I am redirecting them back to the application so they end up back at the login screen. However, if the user has a second browser tab open, and makes another request from that page (pressing a button for example), then when lua-resty-openidc's authenticate method is called for that request, a new session is started. This means that if the user goes back to the login page from the first tab and tries to log in, they will get the "state from argument does not match state restored from session" error because the session cookie was modified by the request from the second tab. Is there a way to prevent the creation of a new session before a user is actually authenticated? It seems that a session cookie should not be created/modified before the user is actually authenticated.

Environment

• lua-resty-openidc version: 1.7.2
• OpenID Connect provider: Red Hat Single-Sign-On (Keycloak)

Expected behaviour

User should be able to log in with no errors, even if they try to make changes after logging out.

Actual behaviour

The user sees a session mismatch error on login.

Minimized example

Login to application, open application again in another tab. Log out of application in tab one, then try to make changes in tab 2. Then try to log in again in tab one.

Configuration and NGINX server log files

Using the Kong OIDC plugin here: https://github.com/nokia/kong-oidc
Relevant code section here:
https://github.com/nokia/kong-oidc/blob/master/kong/plugins/oidc/handler.lua#L54

[bungle]

What I have done on my OpenID Connect Plugin implementation on Kong is that I use two cookies: one for authentication and another for session. I also detect that if authentication cookie exists. If it does I will reuse it when redirecting to authorization endpoint. I have also heard people encoding stuff inside state parameter, but I feel like it is wrong use of it.

[mcg1969]

@bungle is there any chance you can point us to an example of this code pattern? It sounds like a great idea. I can have a single session for the application, and then I really don't have to be concerned about the session behavior of the openidc work.

[SAGV]

We are hitting the same issue using a bare lua-resty-openidc and with a very similar reproducing algorithm:

• Log out
• Open the first tab, it will redirect to login. But do not log in there. Keep login page open
• Open the second tab, which will redirect to login, too. Do not log in there
• Return to the first tab and log in there
• Boom, a error like state from argument: xxxxx does not match state restored from session: yyyyy is raised

We store session in cookies. Opening the second tab overwrites first tab's session and breaks login flow in the first tab.

Not yet sure about the actual fix so ideas are welcome

[claesenm]

We are experiencing similar issues with multiple tabs. Have you ever found an adequate solution?

Surprisingly, in our case, we don't even need to explicitly logout in any tab. The state mismatch error appears to also be triggered when a server side API refresh occurs. I will circle back once we get some more info, either in this issue or a new one.

[SAGV]

We decided to phase out the lua-resty solution in general because of this and some other issues.

However, just before that we managed to implement some fix to keep the session state between tabs and it kinda worked in most cases:
#350

[omasseau]

After reading this, I understand that it is not recommended to allow multi-tabs if using lua-resty-openidc, right ?

[alexdowad]

I am occasionally experiencing a similar issue even when only one browser tab is used; sometimes when trying to load the main page of my site, the browser sends 2 requests rather than 1, and the 2nd one overwrites the session state stored by the 1st one. Then when the browser is redirected back to the site after authenticating, the state stored in the session doesn't match that which is passed back from the OAuth provider.

I know it's always a bad idea to assume that someone else's code is buggy (usually it's your own which is causing problems), but this almost seems like a browser bug. So far, all occurrences have happened with the same web browser.

[mateodelnorte]

Is there a recommended fix for this? not recommended to allow multi-tabs if using lua-resty-openidc does not seem like a reasonable constraint.

Looks like a near fix was implemented in #350, but not merged. Is there a recommendation for how that PR could be improved to allow a merge?

[bodewig]

please see the discussion inside of #350 to understand its problems. If two tabs were trying to log in at the same time and the PR was applied then login might succeed in both tabs but bot would end up on the same URI - which may not be what your users would expect.

The problem with several authentications flows happening at the same time is one that is not easily solved as there is no way for the server to know it is dealing with the same browser without using Cookies to start with. And coordinating multiple requests of the same client requires server-side state, you can't rely on setting things inside Cookies when many requests come in more or less at the same time. You need locking at the server.

Something that has worked for me in the past most of the time - and it requires quite a bit of custom Lua code in addition to what lua-resty-openidc provides:

• use a server side session store - Redis in our case as we needed Redis for other things as well
• if a request comes in that doesn't have a session attached, start a session and respond with a redirect to the exact same URI that the first request wanted to visit before ever talking to lua-resty-openidc. This way there never is an anonymous request and you "only" need to deal with the problem of a single client using the same session for all its requests
• keep track of all sessions that are currently taking part on an authorization flow - Redis as store and lua-resty-openidc lifecycle hooks help. If a request comes in unauthenticated for a session that is currently on a flow for a different request - wait a little and redirect back to the same URI it wanted to access. I.e. you hope the other flow has finished by the time the browser follows the redirect and the request will be authenticated next time around
• never refresh tokens via the frontend, always use server side token refreshs or you are just making things worse
• if you've got a JavaScript client performing requests during the lifetime of the session you may want to add some sort of heartbeat request that basically only serves as a way to keep the session alive and refresh tokens. This way your important requests are more likely to not run into token refresh scenarios.

we may have done even more things than that, it's been a few years.

Basically I don't believe it is possible to support parallel requests with lua-resty-openidc in a Cookie-only world. And I've thought long and hard back when we implemented the scheme I described and couldn't come up with something without problems that wouldn't require quite a bit of extra code.

"support multi-tabs" only sounds simple until you start thinking through the corner cases :-)

[mcg1969]

What a wonderfully detailed response. Thank you!

[mateodelnorte]

please see the discussion inside of #350 to understand its problems. If two tabs were trying to log in at the same time and the PR was applied then login might succeed in both tabs but bot would end up on the same URI - which may not be what your users would expect.

The problem with several authentications flows happening at the same time is one that is not easily solved as there is no way for the server to know it is dealing with the same browser without using Cookies to start with. And coordinating multiple requests of the same client requires server-side state, you can't rely on setting things inside Cookies when many requests come in more or less at the same time. You need locking at the server.

Something that has worked for me in the past most of the time - and it requires quite a bit of custom Lua code in addition to what lua-resty-openidc provides:

• use a server side session store - Redis in our case as we needed Redis for other things as well
• if a request comes in that doesn't have a session attached, start a session and respond with a redirect to the exact same URI that the first request wanted to visit before ever talking to lua-resty-openidc. This way there never is an anonymous request and you "only" need to deal with the problem of a single client using the same session for all its requests
• keep track of all sessions that are currently taking part on an authorization flow - Redis as store and lua-resty-openidc lifecycle hooks help. If a request comes in unauthenticated for a session that is currently on a flow for a different request - wait a little and redirect back to the same URI it wanted to access. I.e. you hope the other flow has finished by the time the browser follows the redirect and the request will be authenticated next time around
• never refresh tokens via the frontend, always use server side token refreshs or you are just making things worse
• if you've got a JavaScript client performing requests during the lifetime of the session you may want to add some sort of heartbeat request that basically only serves as a way to keep the session alive and refresh tokens. This way your important requests are more likely to not run into token refresh scenarios.

we may have done even more things than that, it's been a few years.

Basically I don't believe it is possible to support parallel requests with lua-resty-openidc in a Cookie-only world. And I've thought long and hard back when we implemented the scheme I described and couldn't come up with something without problems that wouldn't require quite a bit of extra code.

"support multi-tabs" only sounds simple until you start thinking through the corner cases :-)

@bodewig are there any free open source implementations of this? Does kong-oidc have the same limitations as lua-resty-openidc, since their plugin is based on this one?

I've had success with lua-resty-openidc, but the need to support legacy clients with poor request patterns makes this a showstopper.

Is there, otherwise, a fully free oidc compatible api-gateway you recommend?

[bodewig]

Not sure whether there is an open source implementation of what I described (or anything similar). I know I have never created one (what I described happened during working hours for a customer). And to be honest I don't know much about Kong (which may add stuff on top of lua-resty-openidc) and haven't got a complete enough view on the market to recommend anything.

[Clivern]

We are hitting the same issue using a bare lua-resty-openidc and with a very similar reproducing algorithm:

• Log out
• Open the first tab, it will redirect to login. But do not log in there. Keep login page open
• Open the second tab, which will redirect to login, too. Do not log in there
• Return to the first tab and log in there
• Boom, a error like state from argument: xxxxx does not match state restored from session: yyyyy is raised

We store session in cookies. Opening the second tab overwrites first tab's session and breaks login flow in the first tab.

Not yet sure about the actual fix so ideas are welcome

I think this behaviour is common. I mean when you open a protected tab and then open a different protected tab to override the state cookie then login from the first tab. You will get a state mismatch.

I tested this behaviour with grafana trying to see if they gone an extra mile trying to address this but i got the same error.

login.OAuthLogin(state mismatch)

[bodewig]

I don't believe it is possible to deal with multiple login flows in a server based solution without server side state at all. You can manage it with a solution that purely lives on the client side (and deal with all the security issues such a solution brings - not lua-resty-openidc's world), or you need server side state plus additional logic not part of lua-resty-openidc. The details of "additional logic" tend to depend on the system you design, I don't really see a way to easily extract it. In either case it would be beyond the scope of this library IMHO.