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

When Authorize should state check in session? #531

Closed
mengxzh opened this issue Jan 8, 2025 · 4 comments
Closed

When Authorize should state check in session? #531

mengxzh opened this issue Jan 8, 2025 · 4 comments

Comments

@mengxzh
Copy link

mengxzh commented Jan 8, 2025

  • lua-resty-openidc version 1.8.0
  • OpenID Connect provider Spring Authotrization Server
    2025-01-08_101047

Question

When state will be set in session, should check session has already exists state

Expected

When state has already in session, use it
When state not in session, generate a new and set in session

@mengxzh
Copy link
Author

mengxzh commented Mar 7, 2025

in 363, when concurrent oauth2_authorize and callback hasent been processed yet, state will be cover in next request.
should 321 generating state check state has already in session?

@oldium
Copy link
Collaborator

oldium commented Mar 7, 2025

(I am writing this from memory, sorry for not being precise in the terminology.)

There are two types of requests:

  1. Full-auth requests. Those will be redirected to OAuth2 provider requiring user to interact with UI and there is not much you can do with regards to parallel requests but see below.
  2. Token refresh requests. There can be multiple refresh requests active in parallel, it does not actually matter which one “wins” at the end and is stored into the session cookie, because the tokens will have plus minus the same lifetime. Note: The refresh token reuse protection must be inactive on the provider side if there is any, because the same refresh token is used in the parallel requests.

Regarding the full-auth refresh using the state:

When state has already in session, use it
When state not in session, generate a new and set in session

This will not help, for parallel requests from the same browser you initially have no session (or existing session) and at the end you have several conflicting session updates coming to the browser together with the auth redirection. You cannot prevent them unless you want to introduce some kind of thread locks on the Nginx side. Also, each request to the OAuth2 provider should be unique (together with unique nonce and code_verifier) to prevent replay attacks.

But there is still a chance (a hack actually). You need to introduce a separate state-dependent custom session storage and have one “classic” session storage (containing the tokens):

  1. Pass your own custom session object simulating the lua-resty-session v4 API. You can pass it as fourth argument to openidc.authenticate.
  2. You need to implement the following methods in the custom session object (used by lua-resty-openidc code): save, get_data (returning nil when session does not exist yet), set, get.
  3. When the save is called on the custom session object, take auth-specific session keys (state, nonce, code_verifier, original_url, last_authenticated) and store them in Redis with state value as a key and serialized key-value object as a value (all auth-specific session keys). Before saving the data into the “classic” session, ensure that the auth-specific data is removed.
  4. The custom session object needs to be initialized before the call to openidc.authenticate in the following way:
    • If the state key is not in the request URL query parameters, simply load whatever “classic” session has.
    • If the state key is in the request URL query parameters, load whatever “classic” session has and load the key-value object from Redis from the state value as a Redis key. Merge the keys into the custom session object.

Thats it. I am using this hack and it somehow works 😉

Edit: updated description

@mengxzh
Copy link
Author

mengxzh commented Mar 10, 2025

Oh, got, You mean the state could store in session attribute(or other durable session method), rather than in request attribute. When state has already in session attribute, load it. When session has not state, generate it? @oldium

@oldium
Copy link
Collaborator

oldium commented Mar 10, 2025

Oh, got, You mean the state could store in session attribute(or other durable session method), rather than in request attribute. When state has already in session attribute, load it. When session has not state, generate it?

I do not know what you mean by “session attribute”. I know about sessionStorage in browser (not accessible by lua-resty-openidc), I know about session cookie handled by lua-resty-session and I know about the session parameter to the openidc_authorize method call.

And I am not talking about generating missing data. I am talking about a storage of auth-specific session keys into a separate storage (like into Redis, but it could be stored in a cookie with per-state-value name).

And please note that this hack is very advanced and uses internals of lua-resty-openidc and lua-resty-session, so use it at your own risk - it might break with any release of lua-resty-openidc or lua-resty-session.

@mengxzh mengxzh closed this as completed Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants