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

Cookies set by oauthGrant filter are occasionally too big #3211

Open
oabdoun opened this issue Aug 28, 2024 · 4 comments
Open

Cookies set by oauthGrant filter are occasionally too big #3211

oabdoun opened this issue Aug 28, 2024 · 4 comments

Comments

@oabdoun
Copy link

oabdoun commented Aug 28, 2024

Describe the bug
The OAuth flow as implemented by the oauthGrant() filter sets the encrypted, base64-encoded OAuth access token as a cookie. Depending of the OAuth authentication service (AWS Cognito in our case), this can result in a Cookie larger than 4096 bytes, at which point the browser (Chrome and Firefox at least) "refuse" the cookie and don't send it back to Skipper in the following requests, resulting in an apparent authentication failure.

This is similar to the already fixed #1089 issue.

To Reproduce
This depends on the OAuth server used for testing, but in AWS Cognito case, if you add the user to a lot of user groups, this list of groups is part of the access token, making it possible to grow it at will and reproduce the issue...

Expected behavior
The "OAuth2 Token Cookie" should be whether less than 4096 bytes or chunked into separate cookies if larger. Compression before or after encryption of the access token would improve the situation in the short term, but chunking (with or without compression) would be the long term solution.

Observed behavior
The oauthGrant() filter can return a "OAuth2 Token Cookie" larger than the accepted limit of 4096.

@AlexanderYastrebov
Copy link
Member

For applications based on Skipper it is possible to customize cookie encoding, see #2953

@AlexanderYastrebov
Copy link
Member

The tricky part about cookie cutting (I think not implemented by #1289) is that updating cookie to a smaller value should also delete last chunks of the old cookie.
E.g. if existing cookie had chunk-01, chunk-02 and chunk-03 and the new cookie only requires chunk-01 and chunk-02 - update logic must issue deletion cookie for chunk-03.

@oabdoun
Copy link
Author

oabdoun commented Aug 29, 2024

In the case of an OAuth chunked cookie size getting a smaller value, couldn't the Expires attribute be used to check if all the chunks belong to the same batch ? If the OAuth access_token has been updated, it is very likely that it's expiration date (mirrored in the cookie) was also updated ?!

@AlexanderYastrebov
Copy link
Member

couldn't the Expires attribute be used to check if all the chunks belong to the same batch ?

I think this is user-agent attribute provided via Set-Cookie header but request's Cookie header contains only name=value pairs.

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

No branches or pull requests

2 participants