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

Introduce quarkus-oidc-redis-token-state-manager #44382

Closed
sberyozkin opened this issue Nov 7, 2024 · 7 comments · Fixed by #44546
Closed

Introduce quarkus-oidc-redis-token-state-manager #44382

sberyozkin opened this issue Nov 7, 2024 · 7 comments · Fixed by #44546
Assignees
Labels
Milestone

Comments

@sberyozkin
Copy link
Member

sberyozkin commented Nov 7, 2024

Description

By default, Quarkus OIDC is stateless. It keeps tokens in the encrypted session cookies.

However, it is totally customizable with custom TokenStateManager.
In fact, we ship quarkus-oidc-db-token-state-manager, nicely done by @michalvavrik, which makes it super easy to make Quarkus OIDC stateful and have tokens stored in a DB of choice.

However, shipping a quality cache based TokenStateManager out of the box can help some users with making a migration decision. For example, Vert.x supports Redis cache as well. Also, I'm aware of at least a single custom TokenStateManager which uses Redis. This is why I suggest to start with Redis and we can do Infinispan later as well.

Implementation ideas

Start with using Redis directly, similar to how we do it for storing and managing tokens in DB.
Then later, if Ladislav @Ladicek finds the time to complete his nice Vert.x based session support PR for Redis and Infinispan, we can just make quarkus-oidc-redis-token-state-manager call the Vertx Session API, and the whole extension will have a few lines of code only.

CC @michalvavrik @cescoffier

@sberyozkin sberyozkin added the kind/enhancement New feature or request label Nov 7, 2024
Copy link

quarkus-bot bot commented Nov 7, 2024

/cc @Ladicek (redis), @cescoffier (redis), @machi1990 (redis), @pedroigor (oidc)

@michalvavrik
Copy link
Member

Yes, that is very interesting! I am in.

@geoand
Copy link
Contributor

geoand commented Nov 8, 2024

Very interesting indeed

@michalvavrik
Copy link
Member

michalvavrik commented Nov 8, 2024

I thought about it bit and I can start working on it early next week. In case someone else want to work on it instead, shout! (especially Redis experts @Ladicek :-)).

@sberyozkin
Copy link
Member Author

sberyozkin commented Nov 8, 2024

Hi @michalvavrik, I believe Ladislav had a nearly completed Vertx Session API PR, with only a few questions like how to remove entries, or something like that, remaining, with the Vertx Session API possibly not supporting such actions. I'm hoping it would be completed at some point because it was looking very good, simplicity wise.
But in meantime, please have a look when you get a chance, take your time please... thanks

@michalvavrik michalvavrik self-assigned this Nov 8, 2024
@Ladicek
Copy link
Contributor

Ladicek commented Nov 11, 2024

Hi @sberyozkin, the issue with Vert.x Web sessions was that there's no way to prevent concurrent session access. Which is OK for read-only requests, but read-write requests, not so much. In case there are multiple concurrent read-write requests, only one of them will be able to write to the session; modifications from the others are lost. That's not good, and I don't have a good solution for that (yet).

@sberyozkin
Copy link
Member Author

Hi @Ladicek Thanks for the clarification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants