Skip to content

feat(authn/oidc): add front-channel logout support#6164

Open
erka wants to merge 4 commits into
v2from
rd/v2/oidc-front
Open

feat(authn/oidc): add front-channel logout support#6164
erka wants to merge 4 commits into
v2from
rd/v2/oidc-front

Conversation

@erka

@erka erka commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
  • Add AllowFrontChannelLogout config option for OIDC providers
  • Route GET /auth/v1/method/oidc/{provider}/revoke to Revoke handler
  • Validate iss/sid query params on front-channel logout requests
  • Clear session cookies via ForwardRevokeOIDCResponseOption only when
    front-channel logout succeeds (gated by provider config)
  • Set SameSite=None on cleared cookies so browser discards them in
    cross-origin iframe context (required by OIDC front-channel logout)
  • Add proto fields iss/sid to RevokeOIDCRequest for front-channel params

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The latest Buf updates on your PR. Results from workflow Proto / proto-lint (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJul 15, 2026, 11:00 PM

@erka erka changed the title feat(authn/oidc): add front-channel logout support with cookie clearing feat(authn/oidc): add front-channel logout support Jul 9, 2026
@erka erka added the v2 Flipt v2 label Jul 9, 2026
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.28571% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.37%. Comparing base (555c557) to head (e45a25e).

Files with missing lines Patch % Lines
internal/server/authn/method/oidc/server.go 75.00% 8 Missing and 1 partial ⚠️
internal/cmd/authn.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v2    #6164      +/-   ##
==========================================
+ Coverage   62.27%   62.37%   +0.09%     
==========================================
  Files         145      145              
  Lines       14815    14867      +52     
==========================================
+ Hits         9226     9273      +47     
- Misses       4829     4833       +4     
- Partials      760      761       +1     
Flag Coverage Δ
integrationtests 33.38% <0.00%> (-0.13%) ⬇️
unittests 53.98% <84.28%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: request changes

The front-channel logout wiring is well-structured and the session.secure validation is solid, but the prior review’s major concern remains unaddressed: ForwardRevokeOIDCResponseOption unconditionally clears session cookies for every successful GET /auth/v1/method/oidc/{provider}/revoke—including no-op responses where sid/iss is missing or no matching session was found. This incorrectly logs out the currently authenticated user when the front-channel request targets a different (or absent) session.

internal/server/authn/middleware/http/middleware.go

  • major (L71): Clearing session cookies unconditionally for every successful GET /auth/v1/method/oidc/{provider}/revoke will force-logout the current browser user even when the Revoke handler returned a no-op because sid/iss was missing, mismatched, or no session was found. The handler should signal when sessions were actually terminated (e.g., by setting outgoing gRPC metadata such as x-oidc-session-cleared), and this middleware should check for that signal before calling clearAllCookies.

🤖 Automated review by the Flipt PR review agent.

@erka
erka force-pushed the rd/v2/oidc-front branch 6 times, most recently from 437177b to 0a51ae3 Compare July 10, 2026 09:23
@erka
erka marked this pull request as ready for review July 10, 2026 09:38
@erka
erka requested a review from a team as a code owner July 10, 2026 09:38
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 10, 2026
@erka

erka commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

I'll stop here. If anyone has security concerns about Front-Channel Logout, Back-Channel Logout is available as an alternative. Azure, please consider supporting Back-Channel Logout as well.

erka added 3 commits July 13, 2026 15:37
- Add AllowFrontChannelLogout config option for OIDC providers
- Route GET /auth/v1/method/oidc/{provider}/revoke to Revoke handler
- Validate iss/sid query params on front-channel logout requests
- Clear session cookies via ForwardRevokeOIDCResponseOption only when
  front-channel logout succeeds (gated by provider config)
- Set SameSite=None on cleared cookies so browser discards them in
  cross-origin iframe context (required by OIDC front-channel logout)
- Add proto fields iss/sid to RevokeOIDCRequest for front-channel params

Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
@erka
erka force-pushed the rd/v2/oidc-front branch from f03b9e3 to 0b0446f Compare July 13, 2026 14:37
@erka

erka commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

The front-channel logout wiring is well-structured and the session.secure validation is solid, but the prior review’s major concern remains unaddressed: ForwardRevokeOIDCResponseOption unconditionally clears session cookies for every successful GET /auth/v1/method/oidc/{provider}/revoke—including no-op responses where sid/iss is missing or no matching session was found. This incorrectly logs out the currently authenticated user when the front-channel request targets a different (or absent) session.

OIDC Front-Channel Logout specifies that sid and iss are optional. Because the logout URL is loaded in a hidden iframe, no cookies are sent to Flipt, so there is nothing to clean up on the server side without additional hints. If sid and iss are not provided, Flipt can only clear the browser cookies. Likewise, if no authentication record is found in Flipt, clearing the browser cookies is sufficient, as I see it.

@erka erka linked an issue Jul 20, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files. v2 Flipt v2

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[Feature]: Add logout URL for OIDC authentication

2 participants