Don't allow to create new users when using external authentication systems like KeyCloak or OpenID

[josegar74]

• Tested with local authentication --> New user option is available (OK)
• Tested with Keycloak authentication --> New user option is not available (OK)

Checklist

• I have read the contribution guidelines
• Pull request provided for main branch, backports managed with label
• Good housekeeping of code, cleaning up comments, tests, and documentation
• Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
• Clean commit messages, longer verbose messages are encouraged
• API Changes are identified in commit messages
• Testing provided for features or enhancements using automatic tests
• User documentation provided for new features or enhancements in manual
• Build documentation provided for development instructions in README.md files
• Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

[fxprunayre]

Small note about this. In some setup we sill have the need of some functional account (eg. for scripts or third party tools like FME) even if using external auth providers. But we can disable this change if needed. Maybe a good alternative would be to support some kind of API keys at some point.

[ianwallen]

In the next condition there is check to skip if the user is an administrator. This allows administrators to pre-create the users before the user logs in. I suspect that there should be a check here as well to skip the check for administrators?

We used this logic during a migration exercise to be able to populate the metadata and the ownership via api. In this case the users never logged in so it was nice to be able to pre-create the users and then use the account for metadata ownership before the user ever logged in. In this case, only the email was specified in the user account as the email/username. The future OIDC login updated the user account with the rest of the information. Otherwise we would have had to do all this logic via SQL statements and it could have been more prone to errors.

[josegar74]

@ianwallen I have updated the code, but the original code you point:

core-geonetwork/services/src/main/java/org/fao/geonet/api/users/UsersApi.java

Lines 464 to 469 in c7be4e4

	// Allow administrator to modify the user profile as they may need to manually pre-create users via api in certain cases (i.e. migration)
	if (securityProviderConfiguration != null &&
	!securityProviderConfiguration.isUserProfileUpdateEnabled() &&
	!Profile.Administrator.equals(myProfile)) {
	return new ResponseEntity<>(messages.getString("security_provider_unsupported_functionality"), HttpStatus.PRECONDITION_FAILED);
	}

The comment is about updating a user profile, but the method is to create a new user, not to update a user, that seems confusing. Maybe the previous code should not have that check? Or do I miss something?

[ianwallen]

isUserProfileUpdateEnabled means are we allowed to make changes to the profile. This implies creation and updates since creations are modifying the profile and so is updating. I'm not understanding why we would want to allow someone to create profiles but not update them? Or vice versa.

The goal for this flag was to disallow the modification of the user profile since the profile is managed by the OIDC.
This is why if the external authentication is updating the profile then we use the opposite for this flag.

core-geonetwork/core/src/main/java/org/fao/geonet/kernel/security/openidconnect/OIDCConfiguration.java

Line 226 in c7be4e4

return !updateProfile;

I guess if the external authentication system would only copy the user profile during account creation and then going forward it was only used to authentication then I guess disallowing create and allowing updates would be acceptable? But I don't think this is what you are trying to fix.

To be honest, I'm not understanding why we need a variable for creation and another one for updates? Since they are for the same purpose. And if the system is only allowing create but not update and a user is created with a minor bug (i.e. incorrect email) then update would not be allowed to fix the issue via the api? - Based you the description of this PR there is no information on what issue we are trying to address? In our environment, we use keycloak via openidconnect configuration and we cannot create/update users via the ui. We have used the api as an administrator (which we used during a migration to pre-create users.)

I have no objection with refactoring the variable if you think it would clarify the meaning.