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

Fix: oauth2 state encoding #37473

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

zhaohuabing
Copy link
Member

@zhaohuabing zhaohuabing commented Dec 3, 2024

Commit Message:

  • Changes the state pramater to a json object to store the user state before redirecting the user request to the auth server. Currently, the json object has two fields: the original url and a nonce for csrf prevention.
  • Changes the state econding to Base64URL to fix AWS cognito doesn't support url encoded state value

Example:
original state: {"url":"https://localhost:8080/login","nonce":"12345678"}
base64url encoded state: eyJ1cmwiOiJodHRwczovL2xvY2FsaG9zdDo4MDgwL2xvZ2luIiwibm9uY2UiOiIxMjM0NTY3OCJ9

Additional Description: The nonce in the state parameter is used for csrf prevention and is applicable for both oauth2 and oidc. Please note that the OIDC spec defines a seperate nonce parameter ,which is specifically designed to prevent replay attacks and is unique to OIDC.

More discussion about state and nonce parameters and be found in this comment: #37050 (comment)

Risk Level:
Testing: Unit test and integration test
Docs Changes:
Release Notes: Yes
Platform Specific Features:
[Optional Runtime guard:] A runtime gurad "envoy.reloadable_features.oauth2_enable_state_nonce" has been added for the new nonce in the state parameter.
[Optional Fixes #37049]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Related Envoy Gateway issue: #36871

cc @missBerg @arkodg

Copy link

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #37473 was synchronize by zhaohuabing.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Dec 4, 2024
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #37473 was synchronize by zhaohuabing.

see: more, trace.

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
@zhaohuabing zhaohuabing marked this pull request as ready for review December 4, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option to disable nonce in the OAuth2 filter
2 participants