Skip to content

fix: Extract username from JWT token claims instead of UserInfo - #291

Merged
TamarW0 merged 3 commits into
ga-releasefrom
APPENG-5865-01
Aug 4, 2026
Merged

fix: Extract username from JWT token claims instead of UserInfo#291
TamarW0 merged 3 commits into
ga-releasefrom
APPENG-5865-01

Conversation

@TamarW0

@TamarW0 TamarW0 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

rhartuv and others added 2 commits August 3, 2026 15:54
* feat: support AWS Cognito as an OIDC provider for external-idp

* docs: add authentication.md file

* chore: add logs and test for role is not in targetRoles
@vbelouso

vbelouso commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@TamarW0
TamarW0 marked this pull request as ready for review August 4, 2026 07:38
@TamarW0
TamarW0 requested a review from rhartuv August 4, 2026 07:39

@zvigrinberg zvigrinberg left a comment

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 Hi Tamar,

2 comments.

Thank you.

Comment thread src/main/java/com/redhat/ecosystemappeng/exploitiq/service/UserService.java Outdated
Comment on lines 57 to 123
public String getUserName() {
// First try to get username from JWT token directly
if (securityIdentity != null && securityIdentity.getPrincipal() instanceof JsonWebToken) {
JsonWebToken jwt = (JsonWebToken) securityIdentity.getPrincipal();

// Try email claim (Cognito ID tokens, common in OIDC)
String name = jwt.getClaim("email");
if (Objects.nonNull(name) && !name.isBlank()) {
return name;
}

// Try cognito:username (Cognito-specific)
name = jwt.getClaim("cognito:username");
if (Objects.nonNull(name) && !name.isBlank()) {
return name;
}

// Try username claim
name = jwt.getClaim("username");
if (Objects.nonNull(name) && !name.isBlank()) {
return name;
}

// Try upn (user principal name - common in enterprise)
name = jwt.getClaim("upn");
if (Objects.nonNull(name) && !name.isBlank()) {
return name;
}

// Try preferred_username (standard OIDC claim)
name = jwt.getClaim("preferred_username");
if (Objects.nonNull(name) && !name.isBlank()) {
return name;
}

// Try sub (subject - always present but may be UUID)
name = jwt.getClaim("sub");
if (Objects.nonNull(name) && !name.isBlank()) {
return name;
}
}

// Fallback to UserInfo if JWT extraction didn't work
if (Objects.nonNull(userInfo)) {
var name = userInfo.getString("email");
if (Objects.nonNull(name)) {
return name;
}
name = userInfo.getString("upn");
if (Objects.nonNull(name)) {
return name;
}
var metadata = userInfo.getObject("metadata");
if (Objects.nonNull(metadata)) {
name = metadata.getString("name");
if (Objects.nonNull(name)) {
return name;
}
}
name = userInfo.getString("preferred_username");
if (Objects.nonNull(name)) {
return name;
}
name = userInfo.getString("sub");
if (Objects.nonNull(name)) {
return name;
}

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 Tests will be good here, to see that it gives precedence to jwt claims, and if the expected claims are not present in the JWT, it will fallback to the userinfo object as a source.
If you don't have time, as it's urgent, please take it off to a future task. ( the most important things is that you'll verify these behavior for production/external-idp' keycloak that we don't have regressions there).

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.

it is under testing

…rService.java

Co-authored-by: Zvi Grinberg <75700623+zvigrinberg@users.noreply.github.com>

@zvigrinberg zvigrinberg left a comment

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 LGTM Approved with conditions:

  1. Please open a follow-up task on you to complete the related tests of the added logic here.
  2. Also don't forget to change the target branch from main to ga-release and then cherry-pick to main.

@TamarW0
TamarW0 changed the base branch from main to ga-release August 4, 2026 12:46
@TamarW0
TamarW0 merged commit 7254edc into ga-release Aug 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants