Skip to content

Split OIDC session cookie if its size is more than 4KB #37891

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

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Dec 21, 2023

Fixes #37836.

This PR will probably be of most help to many users, as mentioned in #37836, quite often, now that the session cookie is encrypted by default, users will get a warning that the encrypted cookie (which typically has 3 tokens concatenated, ID, access and refresh tokens, with ID one taking anything up to 4K and sometimes more in a clear form) is more than 4K.
Next, the users have to choose what to do - we offer many options, but it really can be a bit annoying and sometimes difficult to decide upon - they may have to make a decision to disable the encryption or use the stateful approach with quarkus-oidc-db-token-state-manager, etc.

So this PR simply makes sure it always works by splitting a session cookie into chunks if it exceeds 4K, as for example, done by ASP.NET (as noted at https://stackoverflow.com/questions/77660268/quarkus-oidc-session-cookie-limit) and possibly by some other stacks. Users will still be able to use the other supported options if they prefer.

While the changes look quite extensive, no OIDC logic is affected at all, it is all about making sure that if a session cookie length is greater than 4K, it will be split into chunks and then correctly assembled into a single value, and correctly removed.

I've added a unit test and also fixed/updated a few integration tests which have already been operating with session cookies with the length > 4K, it is just that HtmlUnit does not enforce a 4K limit.

In reality, I believe, we'll ever get up to 2 chunks, cases where we can have cookies up to 20K or so would be extremely rare but nonetheless I've updated the code to deal with a random unsorted number of cookies.

If it can make it to 3.7 it would be great

Copy link

quarkus-bot bot commented Dec 21, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@sberyozkin
Copy link
Member Author

Thanks @gastaldi, I'll merge early in Jan, 3rd or 4th Jan, as I'll be off the week starting 8th Jan, Pedro should be back on 2nd Jan so there will be an option for him to comment during those 2 days.

Happy Holidays

@sberyozkin sberyozkin merged commit 620b4ec into quarkusio:main Jan 3, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jan 3, 2024
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Jan 3, 2024
@sberyozkin sberyozkin deleted the oidc_session_chunks branch January 3, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for splitting OIDC session cookie into chunks
2 participants