Skip to content

oauth2: add cookie expiration margin#45810

Open
ftaboadac wants to merge 3 commits into
envoyproxy:mainfrom
ftaboadac:oauth2-cookie-expiration-margin
Open

oauth2: add cookie expiration margin#45810
ftaboadac wants to merge 3 commits into
envoyproxy:mainfrom
ftaboadac:oauth2-cookie-expiration-margin

Conversation

@ftaboadac

@ftaboadac ftaboadac commented Jun 23, 2026

Copy link
Copy Markdown

Commit Message:
oauth2: add cookie expiration margin

Additional Description:
Adds cookie_expiration_margin to the OAuth2 HTTP filter. When configured, the filter subtracts this margin from the lifetime of OAuth2 auth-related cookies whose expiration is derived from token/session expiry, including the bearer token, ID token, refresh token, OAuth expiry, and HMAC cookies.

This lets Envoy proactively refresh or re-authenticate before forwarding a request with a token that is close to expiration, matching the motivation in #45749.

The default is unchanged when cookie_expiration_margin is unset or zero.

This PR was prepared with assistance from OpenAI Codex. I reviewed and understand the submitted code and tests, and take responsibility for the change.

Risk Level:
Low. The new field is optional and the default behavior is unchanged.

Testing:
Added OAuth2 filter unit coverage for:

  • configured cookie expiration margin subtraction
  • margin equal to token lifetime, clamping affected cookie Max-Age values to 0 rather than underflowing

Also ran:

  • git diff --check
  • scoped Envoy format check: PASS
  • npx -y @bazel/bazelisk test //test/extensions/filters/http/oauth2:filter_test --test_filter=OAuth2Test.OAuthAccessTokenSucessWithCookieExpirationMargin:OAuth2Test.OAuthAccessTokenSucessWithCookieExpirationMarginEqualToTokenLifetime --config=macos --macos_minimum_os=10.15 --jobs=4 --test_output=errors: PASS

Docs Changes:
Added inline API documentation for cookie_expiration_margin and updated the OAuth2 filter documentation with a short description and example configuration.

Release Notes:
Added changelog fragment under changelogs/current/new_features.

Platform Specific Features:
N/A

[Optional Runtime guard:]
N/A

[Optional Fixes #Issue]
Fixes #45749

[Optional Fixes commit #PR or SHA]
N/A

[Optional Deprecated:]
N/A

[Optional API Considerations:]
Adds a new optional field to the existing v3 OAuth2 filter config. The default is unset/0, preserving existing behavior.

@repokitteh-read-only

Copy link
Copy Markdown

Hi @ftaboadac, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #45810 was opened by ftaboadac.

see: more, trace.

@repokitteh-read-only

Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #45810 was opened by ftaboadac.

see: more, trace.

@mattklein123 mattklein123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/wait

std::chrono::seconds(600));
}

TEST_F(OAuth2Test, OAuthAccessTokenSucessWithCookieExpirationMarginEqualToTokenLifetime) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would explicitly test the overflow case as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, added an explicit cookie_expiration_margin > token lifetime test case

Signed-off-by: Facundo Taboada <ftaboadacuria@gmail.com>
@ftaboadac ftaboadac force-pushed the oauth2-cookie-expiration-margin branch from 93b3ce7 to 495e3f6 Compare June 24, 2026 13:08
@ftaboadac ftaboadac had a problem deploying to external-contributors June 24, 2026 13:08 — with GitHub Actions Error
Signed-off-by: Facundo Taboada <ftaboadacuria@gmail.com>
@ftaboadac ftaboadac temporarily deployed to external-contributors June 24, 2026 13:14 — with GitHub Actions Inactive
mattklein123
mattklein123 previously approved these changes Jun 24, 2026
@ftaboadac

Copy link
Copy Markdown
Author

@mattklein123 Thanks for the review. It looks like CI is still blocked on external contributor approval. Could someone approve the workflow run when available?

Signed-off-by: Facundo Taboada <ftaboadacuria@gmail.com>
@ftaboadac

ftaboadac commented Jun 26, 2026

Copy link
Copy Markdown
Author

Pushed a follow-up fix for the CI failure. The issue was a proto field number conflict after merging main: forward_id_token now uses field 31, so I moved cookie_expiration_margin to field 32 and bumped the next-free-field marker to 33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

oauth2: configurable safety margin to expire cookies before the contained tokens expire

2 participants