Skip to content

fix: implement custom Cognito logout to use correct parameters #299

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.

Sign up for GitHub

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

Open
TamarW0 wants to merge 16 commits into
base: ga-release
Choose a base branch
from
Open

fix: implement custom Cognito logout to use correct parameters #299

Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e62fe1a
fix: implement custom Cognito logout to use correct parameters
Aug 6, 2026
79db702
fix: correct Cognito domain construction in logout URL
Aug 6, 2026
9f1bc01
fix: redirect to application root after Cognito logout, not API base
Aug 6, 2026
ac656cd
fix: prevent crash on non-Cognito OIDC logout
Aug 8, 2026
c2c7d0f
docs: add note that Cognito behind reverse proxy is not supported
Aug 9, 2026
e81e1c4
refactor: extract common logout response builder method
Aug 9, 2026
00ac90e
fix: properly invalidate session and show logout success page
Aug 9, 2026
2e57396
minimize changes
Aug 9, 2026
98b46b1
try fix
Aug 9, 2026
2fcd758
minimize changes
Aug 9, 2026
3f296f7
remove not relevant
Aug 9, 2026
3e7f952
fix: address PR feedback on Cognito logout implementation
Aug 9, 2026
45bcc55
test: add logout endpoint tests for Cognito and non-Cognito flows
Aug 9, 2026
decf74d
fix: address all code review comments from zvigrinberg
Aug 10, 2026
bece8b3
chore: revert unrelated package-lock.json changes
Aug 10, 2026
f00f862
fix: add URI.create exception handling and URL-encode client_id
Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions src/main/java/com/redhat/ecosystemappeng/exploitiq/rest/TokenResource.java
View file
Open in desktop
Comment thread
zvigrinberg marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,32 @@
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriInfo;

import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.eclipse.microprofile.openapi.annotations.Operation;
import com.redhat.ecosystemappeng.exploitiq.service.UserService;

import java.net.URI;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Optional;

@Path("/user")
public class TokenResource {

@Inject
UserService userService;

@ConfigProperty(name = "quarkus.oidc.auth-server-url")
Optional<String> authServerUrl;
Comment thread
zvigrinberg marked this conversation as resolved.
Outdated

@ConfigProperty(name = "quarkus.oidc.client-id")
Optional<String> clientId;

@GET
@Produces("application/json")
@Operation(hidden = true)
Expand All @@ -41,16 +55,51 @@ public String getUserName() {
}

/**
* Performs a local logout using the standard 'Clear-Site-Data' header.
* This feature is available only in secure contexts (HTTPS)
* https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Clear-Site-Data
* Logout endpoint with AWS Cognito support.
* For Cognito (when auth-server-url contains cognito-idp), redirects to Cognito's logout endpoint.
* For other OIDC providers, performs local logout with Clear-Site-Data header.
*/
@POST
@Path("/logout")
@Produces(MediaType.TEXT_HTML)
@Operation(hidden = true)
@PermitAll
public Response logout() {
public Response logout(@Context UriInfo uriInfo) {
// Check if we're using AWS Cognito
boolean isCognito = authServerUrl.isPresent() && authServerUrl.get().contains("cognito-idp");

@tmihalac tmihalac Aug 9, 2026

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.

Redundant Cognito detection with inconsistent dot check (line 73)

Line 73 checks authServerUrl.contains("cognito-idp") (no dot), then line 79
checks authServerUrl.indexOf("cognito-idp.") (with dot). The outer check
is strictly looser. A URL containing cognito-idp but not cognito-idp.
enters the Cognito branch, fails the inner check, and silently falls back
to local logout. Not a runtime bug, but confusing — the isCognito flag is
true while the code takes the non-Cognito path.

@tmihalac tmihalac Aug 9, 2026

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.

Cognito URL prefix recomputed on every request (line 76)

The region, domain prefix, and isCognito flag are derived from
@ConfigProperty values that never change at runtime. All the string parsing
(indexOf, substring, String.format, toLowerCase, replace) could be done
once in a @PostConstruct method and stored as final fields. Only
uriInfo.getBaseUri() varies per request. Not a bug, just wasted work on
every logout.


if (isCognito && clientId.isPresent()) {
// Build Cognito logout URL
// Extract Cognito domain from auth server URL
// auth-server-url format: https://cognito-idp.{region}.amazonaws.com/{user-pool-id}
// user-pool-id format: {region}_{random-string}, e.g., eu-north-1_rIW9qmUNl
// Cognito domain format: https://{region-lowercase}{random-string-lowercase}.auth.{region}.amazoncognito.com
// Example: eu-north-1_rIW9qmUNl -> eu-north-1riw9qmunl.auth.eu-north-1.amazoncognito.com
String authUrl = authServerUrl.get();
String region = authUrl.substring(authUrl.indexOf("cognito-idp.") + 12, authUrl.indexOf(".amazonaws.com"));
Comment thread
zvigrinberg marked this conversation as resolved.
Outdated
String userPoolId = authUrl.substring(authUrl.lastIndexOf("/") + 1);

@tmihalac tmihalac Aug 9, 2026

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.

Trailing slash produces empty userPoolId (line 83)

The code extracts the user pool ID via:
String userPoolId = authServerUrl.substring(authServerUrl.lastIndexOf("/")

  • 1);

If someone configures QUARKUS_OIDC_AUTH_SERVER_URL with a trailing slash
(e.g. .../eu-north-1_rIW9qmUNl/), lastIndexOf("/") matches the trailing
slash, and userPoolId becomes an empty string. The Cognito domain becomes
https://.auth.eu-north-1.amazoncognito.com — a broken URL that fails on
every logout.


// Build Cognito domain: replace underscore with nothing, keep hyphens, convert to lowercase
String cognitoDomain = String.format("https://%s.auth.%s.amazoncognito.com",

@tmihalac tmihalac Aug 9, 2026

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.

Cognito domain derivation from pool ID is wrong

The code constructs the Cognito logout URL domain by transforming the user
pool ID:
String cognitoDomain =
String.format("https://%s.auth.%s.amazoncognito.com",
userPoolId.replace("_", "").toLowerCase(), region);

This assumes the Cognito hosted-UI domain prefix equals the user pool ID
with underscores removed. But Cognito domain prefixes are manually chosen
by the admin in the AWS Console (e.g. my-app, prod-login, anything). They
have no relationship to the pool ID. So the constructed URL points to a
domain that doesn't exist, and every logout redirects to a dead page while
the Cognito session stays alive.

userPoolId.replace("_", "").toLowerCase(), region);

// Build logout redirect URI (application root, not API base)
// uriInfo.getBaseUri() returns https://host/api/v1/, we need https://host/
URI baseUri = uriInfo.getBaseUri();
String logoutRedirectUri = baseUri.getScheme() + "://" + baseUri.getAuthority() + "/";
Comment thread
zvigrinberg marked this conversation as resolved.
Outdated

// Build Cognito logout URL with required parameters
String cognitoLogoutUrl = String.format("%s/logout?client_id=%s&logout_uri=%s",
cognitoDomain,
clientId.get(),

@zvigrinberg zvigrinberg Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@TamarW0 No check on clientId if it's present, could return null and break the logout URL.

@TamarW0 TamarW0 Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the check is done in line 78: if (cognitoDomain.isPresent() && clientId.isPresent()) {

URLEncoder.encode(logoutRedirectUri, StandardCharsets.UTF_8));

return Response.seeOther(URI.create(cognitoLogoutUrl))
.header("Clear-Site-Data", "\"cookies\", \"storage\"")
.build();
}

// For non-Cognito providers, perform local logout
return Response.ok(LOGGED_OUT_HTML)
.header("Clear-Site-Data", "\"cookies\", \"storage\"")
.build();
Comment thread
zvigrinberg marked this conversation as resolved.
Outdated
Expand Down