envoy
envoy copied to clipboard
oauth2 filter: encrypt tokens
Commit Message: This PR encrypts the access, ID, and refresh tokens for the OAuth2 filter.
Risk Level: low Testing: Yes Docs Changes: No Release Notes: Yes Implements #23508
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!
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!
I am fine to this change. But I think it deserve a runtime flag (prefer this one) or a bool flag in the API for this behavior change.
I am fine to this change. But I think it deserve a runtime flag (prefer this one) or a bool flag in the API for this behavior change.
This behavior change is internal to the OAuth2 implementation only and not exposed to users/applications.
- The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.
- The OAuth2 filters decrypt the tokens in the cookies before forwarding them to the backend services.
So there is no behavior change from the users and applications's perspective, do we still need a runtime flag in this case?
- The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.
From proxy/client's perspective, some set-cookie values format are changed (no backward compatibility), I would think it's a minor behavior change. And a flag with default value true should be harmless.
pr ci just broke, apologies, working on a fix
- The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.
From proxy/client's perspective, some set-cookie values format are changed (no backward compatibility), I would think it's a minor behavior change. And a flag with default value
trueshould be harmless.
Sounds good. I'm adding a runtime guard.
CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).
How could I fix this? @wbpcode @phlax
==================== Test output for //tools/code:check_test:
CodeChecker ERROR [runtime_guards] Missing from changelogs: envoy_reloadable_features_oauth2_encrypt_tokens
CodeChecker ERROR [runtime_guards] Check failed
CodeChecker ERROR ERRORS Summary [runtime_guards]:
--------------------------------------------------------------------------------
Missing from changelogs: envoy_reloadable_features_oauth2_encrypt_tokens
I did mention this runtime guard in the change log:
- area: oauth2
change: |
The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. This behavior can
be reverted by setting the runtime guard ``"envoy.reloadable_features.oauth2_encrypt_tokens"`` to ``false``.
~its in the wrong section - possibly you want 2 changelog entries~
see below
/retest
/retest