SecurityUtils .getCurrentUserLogin in a microservice returns "null" with OAuth2 and Okta

[Falydoor]

Overview of the issue

Calling SecurityUtils.getCurrentUserLogin() in a microservice returns null instead of the user login

Motivation for or Use Case

It should be possible to retrieve the user from the security context

Reproduce the error

Use SecurityUtils.getCurrentUserLogin() in a microservice using Okta for the authentication

Related issues

9613

Suggest a Fix

} else if (authentication instanceof JwtAuthenticationToken) {
    return (String) ((JwtAuthenticationToken)authentication).getToken().getClaims().get("preferred_username");
} else if (authentication.getPrincipal() instanceof DefaultOidcUser) {

preferred_username doesn't exist when using Okta, is it specific to Keycloak?

JHipster Version(s)

store@0.0.0 /Users/theo/Documents/perso/oauth2/store
└── generator-jhipster@6.5.1

JDL for the Entity configuration(s) entityName.json files generated in the .jhipster directory

JDL entity definitions

application {
  config {
    baseName gateway,
    packageName com.jhipster.gateway,
    applicationType gateway,
    authenticationType oauth2,
    prodDatabaseType postgresql,
    serviceDiscoveryType eureka
  }
  entities Product
}

application {
  config {
    baseName store,
    packageName com.jhipster.store,
    applicationType microservice,
    authenticationType oauth2,
    prodDatabaseType postgresql,
    serviceDiscoveryType eureka
    serverPort 8081
  }
  entities Product
}

entity Product {
  title String
}

microservice Product with store

Environment and Tools

openjdk version "11.0.4" 2019-07-16 OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.4+11) OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.4+11, mixed mode)

git version 2.23.0

node: v10.17.0

npm: 6.11.3

yeoman: 3.1.1

Docker version 19.03.5, build 633a0ea

docker-compose version 1.24.1, build 4667896b

Browsers and Operating System

• [x] Checking this box is mandatory (this is just to show you read everything)

[pascalgrimaud]

As we have a use case with Okta, can you have a look at this ticket plz @mraible ?

[mraible]

The preferred_username claim is a default claim from OpenID Connect (OIDC) that's returned when you send in a scope of openid profile. See https://developer.okta.com/blog/2017/07/25/oidc-primer-part-1 for more information.

In a microservice app, communication is secured using access tokens, not ID tokens, so OIDC is not used. In the default Keycloak configuration, there are protocol mappers configured to return a number of default OIDC claims in access tokens. I don't think this is a good idea, but I didn't create the jhipster-realm.json we use.

I ran into this when setting up the Ionic for JHipster module. Because I wanted to save user information in the database (via user syncing), the workaround I used was to have the user set up claims that get included in the access token.

To add a preferred_username claim to your access tokens in Okta, you should be able to follow similar instructions, using user.login as the Expression value.

To summarize: I don't think this is a bug in our OAuth implementation, I think it's a bug in how we've configured Keycloak by default.

[mraible]

We could improve things by making it so user information is retrieved from the /userinfo endpoint on the authorization server (with an access token), rather than from claims. This would solve this issue for all OAuth providers, and reduce the configuration needed for Ionic and other clients that access JHipster with an access token.

[stevemac79]

I have added the Claim as suggested above. However I am now running one of my APIs via the Registry swagger interface for dev/testing. The SecurityUtils.getCurrentUserLogin() still comes back with anonymousUser in this context. My registry application is configured to use Okta (as are the gateway and microservice apps). Is there any way to ensure that when running from the Swagger UI, getCurrentUserLogin returns the correct preferred_username (user logged in to the registry) so I can continue to test my APIs in this way?

[mraible]

@stevemac79 Does this issue happen when you're not using the Swagger UI? I haven't used it; just trying to isolate where the problem is coming from.

[stevemac79]

@mraible When I am using the Gateway application to initiate the request I can see the user name by configuring the preferred_username Claim as you explained above.

[mraible]

Thanks for clarifying @stevemac79. I've added a "swagger" label to this issue and a bug bounty to fix. I haven't done much with the Swagger code, but hopefully someone else has.

[Falydoor]

Hey @mraible, I decided to tackle this issue but I need some directions from you.

I was able to consume the /userinfo endpoint with both Keycloak/Okta but I need more details on where/when to perform this operation. It can be done in SecurityUtils.getCurrentUserLogin but I don't really like the idea of doing a call to the authorization server every time getCurrentUserLogin is called.

Also a quick fix would be to just use the correct Okta claim field if preferred_username is null.

[mraible]

I'd do it in AccountResource#getAccount():

https://github.com/jhipster/generator-jhipster/blob/master/generators/server/templates/src/main/java/package/web/rest/AccountResource.java.ejs

Or maybe do it in the UserService method it calls?

I think it only needs to be done when there's a missing "given_name" claim.

On Sep 16, 2020, at 18:11, Théo LEBRUN notifications@github.com wrote:

 Hey @mraible, I decided to tackle this issue but I need some directions from you.

I was able to consume the /userinfo endpoint with both Keycloak/Okta but I need more details on where/when to perform this operation. It can be done in SecurityUtils.getCurrentUserLogin but I don't really like the idea of doing a call to the authorization server every time getCurrentUserLogin is called.

Also a quick fix would be to just use the correct Okta claim field if preferred_username is null.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[Falydoor]

SecurityUtils.getCurrentUserLogin only uses the Authentication from the security context so the two locations you suggested don't really work.

The fix that I suggest is to use authentication.getName() for both Okta/Keycloak instead of ((JwtAuthenticationToken)authentication).getToken().getClaims().get("preferred_username").

However I noticed that authentication.getName() doesn't return the login for Keycloak, it returns the user ID because the sub field from the token is the user ID.

For Keycloak to work, we just need a mapper that put the username in the sub field (commit to all the changes):

{
    "id": "f4a23a8e-f892-11ea-adc1-0242ac120002",
    "name": "UsernameInSubject",
    "protocol": "openid-connect",
    "protocolMapper": "oidc-usermodel-property-mapper",
    "consentRequired": false,
    "config": {
        "userinfo.token.claim": "false",
        "user.attribute": "username",
        "id.token.claim": "false",
        "access.token.claim": "true",
        "claim.name": "sub",
        "jsonType.label": "String"
    }
}

Let me know what you think!

[mraible]

Sounds good!

On Sep 16, 2020, at 22:45, Théo LEBRUN notifications@github.com wrote:

 SecurityUtils.getCurrentUserLogin only uses the Authentication from the security context so the two locations you suggested don't really work.

The fix that I suggest is to use authentication.getName() for both Okta/Keycloak instead of ((JwtAuthenticationToken)authentication).getToken().getClaims().get("preferred_username").

However I noticed that authentication.getName() doesn't return the login for Keycloak, it returns the user ID because the sub field from the token is the user ID.

For Keycloak to work, we just need a mapper that put the username in the sub field (commit to all the changes):

{ "id": "f4a23a8e-f892-11ea-adc1-0242ac120002", "name": "UsernameInSubject", "protocol": "openid-connect", "protocolMapper": "oidc-usermodel-property-mapper", "consentRequired": false, "config": { "userinfo.token.claim": "false", "user.attribute": "username", "id.token.claim": "false", "access.token.claim": "true", "claim.name": "sub", "jsonType.label": "String" } } Let me know what you think!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[vishal423]

Instead of a custom mapper, I think you just need to correctly configure like https://github.com/vishal423/ngx-hipster/blob/6e634c8a4a36b2d0fb9599006e2b2d276e34af48/demo/ngx-hipster-sample-api/src/main/resources/application-oidc.yml#L12

[Falydoor]

That would be cool to just have a property but I don't think that can work because the field preferred_username doesn't exist in an access token with Okta by default.

[mraible]

The problem I hope to solve with this issue is eliminate the need to setup custom claims for Okta when you're using JHipster as a resource server. Currently, with Ionic for JHipster (and like @ruddell's Ignite JHipster), you have to add a number of claims to the access token. If we could trigger a lookup of the user's information when these claims aren't available, it'd solve my problem.

Keycloak doesn't have this issue since we seem to have it configured to return these claims in the access token.

[Falydoor]

A claim converter could be used like defined here https://docs.spring.io/spring-security/site/docs/5.4.1-SNAPSHOT/reference/html5/#oauth2resourceserver-jwt-claimsetmapping but unfortunately the lookup to /userinfo will not work since the converter doesn't have access to the token.

[Falydoor]

@mraible I was able to have something working using the claim converter! Here is the commit if you're curious https://github.com/Falydoor/jhipster-oauth2/commit/daeb4e1a8c70b430ce8ebfd841d641ba2d8982ce

There are two things that I don't like:

• The userinfo endpoint is not the same for Okta/Keycloak

• The userinfo endpoint is called for every requests which add some delay

[mraible]

Nice work @Falydoor! I think there's a couple things we could improve:

1. Look up the user info endpoint, like we do for the Logout URL
2. Improve performance by adding caching or doing a one-time lookup somehow

[Falydoor]

I refactored to lookup the userinfo endpoint and have a "simple" caching system.

You can check the implementation here https://github.com/Falydoor/jhipster-oauth2/blob/master/src/main/java/com/okta/developer/blog/security/oauth2/CustomClaimConverter.java

Let me know if everything works as expected with Ionic!

[Falydoor]

@mraible did you have a chance to look at it?

[mraible]

Not yet. I had a snafu on my laptop last week and had to restore from a backup over the weekend. I'll do my best to try it out this week.

Thanks,

Matt

On Sep 28, 2020, at 7:03 AM, Théo LEBRUN notifications@github.com wrote:

 @mraible did you have a chance to look at it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[mraible]

@Falydoor I tried it this evening and changed the "preferred_username" part to match what Ionic expects:

// Add custom claims
convertedClaims.put("groups", user.get("groups").asText());
convertedClaims.put("given_name", user.get("given_name").asText());
convertedClaims.put("family_name", user.get("family_name").asText());

After registering it in SecurityConfiguration.java:

jwtDecoder.setJwtValidator(withAudience);
jwtDecoder.setClaimSetConverter(new CustomClaimConverter(clientRegistrationRepository.findByRegistrationId("oidc")));

I found that it fails when I log in. IntelliJ also seems a bit concerned about user.get() w/o a null check.

2020-09-29 22:35:03.345  INFO 94575 --- [  XNIO-1 task-4] o.j.d.b.s.oauth2.CustomClaimConverter    : USER INFO -> {"sub":"00ueblyxoowgcboNP356","name":"Okta Demo","locale":"en-US","preferred_username":"demo@okta.com","given_name":"Okta","family_name":"Demo","zoneinfo":"America/Los_Angeles","updated_at":1600905244,"groups":["Everyone","ROLE_USER","user","ROLE_OTHER"]}
2020-09-29 22:35:03.352 ERROR 94575 --- [  XNIO-1 task-4] io.undertow.request                      : UT005023: Exception handling request to /api/account

java.lang.ClassCastException: class java.lang.String cannot be cast to class java.util.Collection (java.lang.String and java.util.Collection are in module java.base of loader 'bootstrap')
    at org.jhipster.demo.blog.security.SecurityUtils.getRolesFromClaims(SecurityUtils.java:93)
    at org.jhipster.demo.blog.security.SecurityUtils.extractAuthorityFromClaims(SecurityUtils.java:88)
    at org.jhipster.demo.blog.security.oauth2.JwtGrantedAuthorityConverter.convert(JwtGrantedAuthorityConverter.java:20)
    at org.jhipster.demo.blog.security.oauth2.JwtGrantedAuthorityConverter.convert(JwtGrantedAuthorityConverter.java:11)
    at org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter.extractAuthorities(JwtAuthenticationConverter.java:53)
    at org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter.convert(JwtAuthenticationConverter.java:38)
    at org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter.convert(JwtAuthenticationConverter.java:32)
    at org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationProvider.authenticate(JwtAuthenticationProvider.java:95)
    at org.springframework.security.authentication.ProviderManager.authenticate(ProviderManager.java:175)

If I remove the .asText() from each line, I get:

java.lang.ClassCastException: class com.fasterxml.jackson.databind.node.ArrayNode cannot be cast to class java.util.Collection (com.fasterxml.jackson.databind.node.ArrayNode is in unnamed module of loader 'app'; java.util.Collection is in module java.base of loader 'bootstrap')
    at org.jhipster.demo.blog.security.SecurityUtils.getRolesFromClaims(SecurityUtils.java:93)
    at org.jhipster.demo.blog.security.SecurityUtils.extractAuthorityFromClaims(SecurityUtils.java:88

[Falydoor]

@mraible I think it's because groups is an ArrayNode so asText() doesn't work.

I did a new commit https://github.com/Falydoor/jhipster-oauth2/commit/16bf5c4df1279c6ce076b58cea011d7026313f63, maybe that's more clear!

[mraible]

Sweet! That works!! Can you please create a PR with this change and add a test too?

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted. We are accepting PRs :smiley:. Comment or this will be closed in 7 days

[mraible]

We're still actively working on this issue in https://github.com/jhipster/generator-jhipster/pull/12609.

[Falydoor]

Bounty claimed https://opencollective.com/generator-jhipster/expenses/31614

[pascalgrimaud]

@Falydoor : just approved. Anyway, can you take care of my comment in your PR plz?