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

filters/auth: add grant cookie encoder #2953

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

AlexanderYastrebov
Copy link
Member

@AlexanderYastrebov AlexanderYastrebov commented Feb 22, 2024

Add CookerEncoder interface to allow custom implementation of grant cookie encoding.

For example custom implementation may store token value in some permanent key-value storage and encode key into the cookie.

Another implementation may encode token value into multiple cookies.

@@ -12,6 +12,33 @@ import (
"golang.org/x/oauth2"
)

type CookieEncoder interface {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want cookie encoder or a generic token encoder that works with http.Response and may encode/extract token into/from cookie or e.g. headers?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing Oauth2 approach here is designed to work with a cookie. Cookie-specific is fine for us unless you think it can be used somewhere else

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AtorVdm I've updated CookieEncoder interface, please check if its suitable for the implementation you have in mind.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a discussion with @AlexanderYastrebov. This works for us! If this is left as-is - we will implement our own compression on top. Another option is if you can split the cookie into multiple ones based on cookie max size param (can be defined). Either way this PR would be a prerequisite for this so I think it's good to get it merged.

filters/auth/grantcookie.go Outdated Show resolved Hide resolved
@AlexanderYastrebov AlexanderYastrebov force-pushed the filters/auth/grant-cookie-encoder branch from 8e67ae3 to 5f8ce1a Compare March 1, 2024 17:34
@AlexanderYastrebov AlexanderYastrebov added the major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs label Mar 1, 2024
@AlexanderYastrebov AlexanderYastrebov force-pushed the filters/auth/grant-cookie-encoder branch 5 times, most recently from e42a740 to 72b043f Compare March 18, 2024 14:34
@AlexanderYastrebov AlexanderYastrebov marked this pull request as ready for review March 18, 2024 14:34
@AlexanderYastrebov AlexanderYastrebov force-pushed the filters/auth/grant-cookie-encoder branch from 72b043f to af10778 Compare March 19, 2024 15:08
@AlexanderYastrebov AlexanderYastrebov marked this pull request as draft March 20, 2024 09:34
@AlexanderYastrebov AlexanderYastrebov force-pushed the filters/auth/grant-cookie-encoder branch 2 times, most recently from 88d4663 to 2a860d2 Compare March 20, 2024 15:38
Add CookerEncoder interface to allow custom implementation of grant
cookie encoding.

For example custom implementation may store token value in some
permanent key-value storage and encode key into the cookie.

Another implementation may encode token value into multiple cookies.

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Export EncryptedCookieEncoder and decouple it from OAuthConfig.

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@AlexanderYastrebov AlexanderYastrebov force-pushed the filters/auth/grant-cookie-encoder branch from 2a860d2 to 54a3e3d Compare March 22, 2024 14:57
@AlexanderYastrebov AlexanderYastrebov marked this pull request as ready for review March 22, 2024 14:59
@MustafaSaber
Copy link
Member

👍

@RomanZavodskikh
Copy link
Member

👍

@AlexanderYastrebov AlexanderYastrebov merged commit 281c3df into master Apr 11, 2024
14 checks passed
@AlexanderYastrebov AlexanderYastrebov deleted the filters/auth/grant-cookie-encoder branch April 11, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants