oauth: always set token cookies in response regardless of forward_bearer_token option

[derekargueta]

Commit Message: oauth: always set token cookies in response regardless of forward_bearer_token option
Additional Description: Unconditionally set the BearerToken, IdToken, and RefreshToken cookies in the response. The documentation of forward_bearer_token states "Forward the OAuth token as a Bearer to upstream web service." It's confusing for this behavior to affect response cookies as well and I can't think of a benefit that's being achieved here. This brings the behavior of this filter more aligned with what the documentation describes.
Risk Level: Low
Testing: Included
Docs Changes: N/A
Release Notes: Included
Platform Specific Features: N/A
Fixes: #32566 #15489

[rgshenoy]

Any ETA on this PR? We'd love to use OAuth2 filter in prod - but we hit this bug and it is preventing us. Thank you!

[denniskniep]

Hi @derekargueta,

is there any chance to land this into the next release? If there is help needed, I can take something over.

Cheers,
Dennis

[zetaab]

@derekargueta ping

[zetaab]

@denniskniep thinking that could you just make separate PR as @derekargueta is not answering/pr not progressing? At least we need this feature

[derekargueta]

Was out on PTO. One reason I had for closing this was consideration of implementing encrypted cookies first (#23508), as this change would set the raw bearer token in the client browser which is undesirable by some.

[denniskniep]

Does it make sense to apply this functionality only, if a certain knob is set?

Edit: I ment introducing a new knob which controls if the cookies are set or not

[zetaab]

so how we should proceed here to get this forward?

[arkodg]

hey @derekargueta afaik this PR should bring back the previous behavior that existed with this filter
#23508 could be added as an enhancement with a new field in the filter

[denniskniep]

@derekargueta does it make sense to introduce further properties to disable single cookies disable_XYZ_set_cookie? Like it was done here: #33825

Adding following properties:

• disable_access_token_set_cookie
• disable_refresh_token_set_cookie

Then

1. we don't need to wait for OAuth2: tokens set in the cookies are not encrypted #23508
2. everyone can precisely control which cookies should be set or not, even if encryption is implemented.
3. forward_bearer_token no longer influences the cookie set behavior

What do you think?

[denniskniep]

@derekargueta I created a PR for my proposal here:
#35839

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[github-actions]

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!