Oauth2 OIDC - claims - BuiltInUserEntity

[ndywicki]

Hi,

In the last Jhipster versions v8, I noticed on monolithic applications with the OAuth2 OIDC option, this is the template AccountResource_skipUserManagement.java.ejs which is used by default (ie. with the UserVM: login and authorities)

And no AccountResource_oauth2.java.ejs which involves the use of UserService and in particular all the claims attributes mapped from the token (ie. with UserDTO)

By searching in the code I saw that the generation of AccountResource_oauth2.java.ejs is conditionned by generateBuiltInUserEntity and depend of syncUserWithIdp option:

generator-jhipster/generators/bootstrap-application-base/generator.ts

Line 193 in 1e17884

generateBuiltInUserEntity: ({ generateUserManagement, syncUserWithIdp }) => generateUserManagement || syncUserWithIdp,

The command-line help states:
--sync-user-with-idp Allow relationships with User for oauth2 applications

For me it is not very clear:
When I choose the Oauth2 OIDC option I think I have complete recovery of IDP claims independently of synchronization in database and/or the possibility of creating relationships between Users and other entities.

Is this the behavior that was intended?

Note: The documentation Command-line options is no longer up to date with the new options 😉

Thanks

[mraible]

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?

[mshima]

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

[dwarakaprasad]

@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 add syncUserWithIdp option #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.

[mshima]

@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;
	}

[ndywicki]

@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.

[mshima]

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.