Skip to content

fix: keycloak logout not revoking the session.#5913

Open
lamoboos223 wants to merge 3 commits into
flipt-io:v2from
lamoboos223:feat/oidc-logout
Open

fix: keycloak logout not revoking the session.#5913
lamoboos223 wants to merge 3 commits into
flipt-io:v2from
lamoboos223:feat/oidc-logout

Conversation

@lamoboos223

Copy link
Copy Markdown

Enhance OIDC support by adding issuer, client ID, and ID token to metadata. Update logout flow to include ID token and client ID in the logout URL. Modify user data structure to accommodate new fields.

@lamoboos223 lamoboos223 requested a review from a team as a code owner May 25, 2026 23:16
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi @lamoboos223! Thanks for your contribution to this project.

It looks like one or more of your commits are missing a DCO (Developer Certificate of Origin) sign-off. The DCO is a simple way for you to certify that you have the right to submit this code under the project's license.

How to fix this:

# For future commits, use the -s flag
git commit -s -m "Your commit message"

# To sign off on existing commits in this PR
git rebase HEAD~$(git rev-list --count origin/v2..HEAD) --signoff
git push --force-with-lease

The -s flag adds this line to your commit message:
Signed-off-by: Your Name <your.email@example.com>

📋 View the failing DCO check for more details

For more information about the DCO, visit: https://developercertificate.org/

Enhance OIDC support by adding issuer, client ID, and ID token to metadata. Update logout flow to include ID token and client ID in the logout URL. Modify user data structure to accommodate new fields.

Signed-off-by: Lama Alosaimi <lamo.boos@hotmail.com>
@erka

erka commented May 26, 2026

Copy link
Copy Markdown
Contributor

Hey @lamoboos223

Thanks for your PR, but while it may work with Keycloak, it breaks support for other OIDC providers such as Azure, Google, Gitea, etc. Although OIDC defines end_session_endpoint, in practice it is not supported by many providers.

@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.05%. Comparing base (e7645db) to head (e31905d).

Additional details and impacted files
@@           Coverage Diff           @@
##               v2    #5913   +/-   ##
=======================================
  Coverage   61.05%   61.05%           
=======================================
  Files         141      141           
  Lines       14211    14214    +3     
=======================================
+ Hits         8676     8679    +3     
  Misses       4809     4809           
  Partials      726      726           
Flag Coverage Δ
integrationtests 34.21% <0.00%> (-0.01%) ⬇️
unittests 52.39% <100.00%> (+0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@lamoboos223

Copy link
Copy Markdown
Author

@erka
Thanks for the note, Roman!
I will make this flow specifically for keycloak only so it doesn’t break other oidc providers

@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 PR improves OIDC metadata and logout support, but the new logout handling is Keycloak-specific and will break logout for other OIDC providers. Restrict the new logic to Keycloak or ensure full OIDC compatibility. Changes otherwise conform to project standards and are adequately tested.

ui/src/components/NavUser.tsx

  • major (L38): The logout flow in NavUser.tsx and related UI updates now always assume Keycloak's openid-connect/logout endpoint, and add id_token, client_id, and redirect URI parameters for all OIDC providers. This breaks compatibility with other OIDC providers (Azure, Google, Gitea, etc.), many of which either do not support this endpoint or require a different mechanism. The logout logic should be restricted to Keycloak, or rewritten to detect and handle provider-specific logout flows so as not to break non-Keycloak users.

🤖 Automated review by the Flue PR review agent.

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

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants