fix: add cognito custom logout params#930
Merged
carlosthe19916 merged 2 commits intoguacsec:mainfrom Mar 4, 2026
Merged
Conversation
Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com>
Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com>
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a shared OIDC sign-out configuration that includes Cognito-specific logout query parameters and wires it into all signoutRedirect() call sites to prevent 400 errors when logging out via AWS Cognito while remaining compatible with Keycloak. Sequence diagram for updated OIDC logout with Cognito and KeycloaksequenceDiagram
actor User
participant FrontendApp
participant OidcUserManager
participant OIDCProviderCognito
participant OIDCProviderKeycloak
User->>FrontendApp: Click Logout
FrontendApp->>OidcUserManager: signoutRedirect(oidcSignoutArgs)
Note over FrontendApp,OidcUserManager: oidcSignoutArgs.extraQueryParams
OidcUserManager->>OidcUserManager: Build logout URL
OidcUserManager->>User: Redirect to logout URL
alt Using Cognito
User->>OIDCProviderCognito: GET /logout
Note over OIDCProviderCognito: Reads client_id and logout_uri
OIDCProviderCognito-->>User: 302 redirect to logout_uri
else Using Keycloak
User->>OIDCProviderKeycloak: GET /protocol/openid-connect/logout
Note over OIDCProviderKeycloak: Reads id_token_hint and post_logout_redirect_uri
OIDCProviderKeycloak-->>User: 302 redirect to post_logout_redirect_uri
end
User-->>FrontendApp: Returns to app after logout
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Using
window.location.originat module top-level inoidcSignoutArgscan break in non-browser/SSR/test environments wherewindowis undefined; consider deriving this lazily (inside the logout flow) or guarding with atypeof window !== 'undefined'check. - Hardcoding
logout_uritowindow.location.originmay be too coarse for deployments where the frontend is served under a sub-path; consider making the logout redirect base configurable (e.g., via env) rather than assuming the origin root.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `window.location.origin` at module top-level in `oidcSignoutArgs` can break in non-browser/SSR/test environments where `window` is undefined; consider deriving this lazily (inside the logout flow) or guarding with a `typeof window !== 'undefined'` check.
- Hardcoding `logout_uri` to `window.location.origin` may be too coarse for deployments where the frontend is served under a sub-path; consider making the logout redirect base configurable (e.g., via env) rather than assuming the origin root.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #930 +/- ##
==========================================
- Coverage 65.07% 65.02% -0.05%
==========================================
Files 195 195
Lines 3341 3342 +1
Branches 753 753
==========================================
- Hits 2174 2173 -1
- Misses 868 872 +4
+ Partials 299 297 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
stanislavsemeniuk
approved these changes
Mar 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: https://issues.redhat.com/browse/TC-3265
Summary
Added
client_idandlogout_uriasextraQueryParamsto allsignoutRedirect()calls to fix AWS Cognito logout returning HTTP 400.Context
When using AWS Cognito as the OIDC provider, clicking "Logout" resulted in an error. The
oidc-client-tslibrary follows the OIDC RP-Initiated Logout spec, sendingid_token_hint+post_logout_redirect_uri. Keycloak understands these standard parameters, but Cognito's hosted UI predates the spec and requires its own proprietary parameters:client_idandlogout_uri.By adding both as
extraQueryParams, the signout URL now contains all four parameters. Each provider reads the ones it understands and ignores the rest:id_token_hintpost_logout_redirect_uriclient_idlogout_uriSummary by Sourcery
Bug Fixes: