Giter VIP home page Giter VIP logo

Comments (6)

mraible avatar mraible commented on September 24, 2024

I don't think anything has changed in JHipster's OAuth/OIDC implementations since v7. I'm not sure I understand your question. Can you please rephrase it?

from generator-jhipster.

mshima avatar mshima commented on September 24, 2024

I confirm the bug.
OAuth2 token is not correctly mapped to the spring security authentication without syncUserWithIdp option.

This option is required in ionic blueprint:
https://github.com/jhipster/generator-jhipster-ionic/blob/c4c2a30002fe3fd5f2ef6a6f402dc21fc2d0ba04/.blueprint/generate-sample/templates/samples/reactive-oauth2.jdl#L12
Otherwise this test fails:
https://github.com/jhipster/generator-jhipster-ionic/blob/c4c2a30002fe3fd5f2ef6a6f402dc21fc2d0ba04/generators/ionic/resources/oauth2/cypress/e2e/login.cy.ts#L22

from generator-jhipster.

dwarakaprasad avatar dwarakaprasad commented on September 24, 2024

@mshima I couldn't fully understand the bug here, I am willing to contribute to this one. Let me start with what i understand,

  1. With this pr #24632, oauth2 option will by default not generate user related code unless one of this is set gnerateUserManagement or syncUserWithIdp or an entity relating to User built in entity.

  2. Currently irrespective of the 'syncUserWithIdp' feature, the endpoint to return account exists (pulls data from the IDP and constructs userVM). This should go away if 'syncUserWithIdp' is not requested.

Also, on the blueprint side, user related code needs to be conditionally generated based on the 'syncUserWithIdp', which is why ionic blueprint started failing when migrated to v8.4.0.

I can default the 'syncUserWithIdpP' similar to nodejs for now until its implemented.

Please let me know.

from generator-jhipster.

mshima avatar mshima commented on September 24, 2024

@dwarakaprasad
If syncUserWithIdp is true, this api/account is used:

generator-jhipster/generators/server/templates/src/main/java/_package_/web/rest/AccountResource_oauth2.java.ejs

Lines 75 to 80 in 9c885f1

public <% if (reactive) { %>Mono<<%= user.adminUserDto %>><% } else { %><%= user.adminUserDto %><% } %> getAccount(Principal principal) {
if (principal instanceof AbstractAuthenticationToken) {
return userService.getUserFromAuthentication((AbstractAuthenticationToken) principal);
} else {
throw new AccountResourceException("User could not be found");
}

As stated in the issue description UserService does not exists without syncUserWithIdp so this other api/account is used:

generator-jhipster/generators/server/templates/src/main/java/_package_/web/rest/AccountResource_skipUserManagement.java.ejs

Lines 119 to 125 in 9c885f1

public UserVM getAccount(Principal principal) {
if (principal instanceof AbstractAuthenticationToken) {
return getUserFromAuthentication((AbstractAuthenticationToken) principal);
} else {
throw new AccountResourceException("User could not be found");
}
}

UserVM has these attributes:

generator-jhipster/generators/server/templates/src/main/java/_package_/web/rest/AccountResource_skipUserManagement.java.ejs

Lines 158 to 161 in 9c885f1

private static class UserVM {
private String login;
private Set<String> authorities;

While User has much more info:

generator-jhipster/generators/spring-boot/templates/src/main/java/_package_/_entityPackage_/service/UserService.java.ejs

Lines 1029 to 1088 in 9c885f1

private static <%= databaseTypeNo ? user.adminUserDto : user.persistClass %> getUser(Map<String, Object> details) {
<%= databaseTypeNo ? user.adminUserDto : user.persistClass %> user = new <%= databaseTypeNo ? user.adminUserDto : user.persistClass %>();
Boolean activated = Boolean.TRUE;
String sub = String.valueOf(details.get("sub"));
String username = null;
if (details.get("preferred_username") != null) {
username = ((String) details.get("preferred_username")).toLowerCase();
}
// handle resource server JWT, where sub claim is email and uid is ID
if (details.get("uid") != null) {
user.setId((String) details.get("uid"));
user.setLogin(sub);
} else {
user.setId(sub);
}
if (username != null) {
user.setLogin(username);
} else if (user.getLogin() == null) {
user.setLogin(user.getId());
}
if (details.get("given_name") != null) {
user.setFirstName((String) details.get("given_name"));
} else if (details.get("name") != null) {
user.setFirstName((String) details.get("name"));
}
if (details.get("family_name") != null) {
user.setLastName((String) details.get("family_name"));
}
if (details.get("email_verified") != null) {
activated = (Boolean) details.get("email_verified");
}
if (details.get("email") != null) {
user.setEmail(((String) details.get("email")).toLowerCase());
} else if (sub.contains("|") && (username != null && username.contains("@"))) {
// special handling for Auth0
user.setEmail(username);
} else {
user.setEmail(sub);
}
if (details.get("langKey") != null) {
user.setLangKey((String) details.get("langKey"));
} else if (details.get("locale") != null) {
// trim off country code if it exists
String locale = (String) details.get("locale");
if (locale.contains("_")) {
locale = locale.substring(0, locale.indexOf('_'));
} else if (locale.contains("-")) {
locale = locale.substring(0, locale.indexOf('-'));
}
user.setLangKey(locale.toLowerCase());
} else {
// set langKey to default if not specified by IdP
user.setLangKey(Constants.DEFAULT_LANGUAGE);
}
if (details.get("picture") != null) {
user.setImageUrl((String) details.get("picture"));
}
user.setActivated(activated);
return user;
}

from generator-jhipster.

ndywicki avatar ndywicki commented on September 24, 2024

@mshima This is my point.
By default with OAuth2, only basic claims feed api/account response via UserVM.
To have other attributs from IDP (give_name, imageUrl, etc), we need pass the extra --sync-user-with-idp parameter option to use the AccountResource_oauth2.java.ejs that call userService.getUserFromAuthentication() and which of course contain the user sync mechanism.

I couldn't find in the history if there was a reason not to include userservice.getUserFromAuthentication() by default and move the syncUserWithIdp condition here:

generator-jhipster/generators/spring-boot/templates/src/main/java/_package_/_entityPackage_/service/UserService.java.ejs

Line 1024 in 9c885f1

return new <%= user.adminUserDto %>(syncUserWithIdP(attributes, user));

This is not really a problem for me, we noticed it because we use a blueprint which takes into account the user's avatar when the IDP is Entra ID (via MS Graph endpoint) and this has changed compared to the version v7 of JHipster.

from generator-jhipster.

mshima avatar mshima commented on September 24, 2024

Entire UserService is implemented for syncUserWithIdp.
It checks and saves current User at each page visit.
So IMO:

  • syncUserWithIdp should be done only once per session.
  • Account should be retrieved from authentication token even with syncUserWithIdp instead of retrieving from database.

from generator-jhipster.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.