search

zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.09k stars 350 forks source link

Cookies set by oauthGrant filter are occasionally too big #3211

Open oabdoun opened 1 month ago

oabdoun commented 1 month ago

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 commented 1 month ago

For applications based on Skipper it is possible to customize cookie encoding, see https://github.com/zalando/skipper/pull/2953

AlexanderYastrebov commented 1 month ago

The tricky part about cookie cutting (I think not implemented by https://github.com/zalando/skipper/pull/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 commented 1 month ago

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 commented 1 month ago

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.