search

zorn-v / nextcloud-social-login

GNU Affero General Public License v3.0
198 stars 137 forks source link

(OAuth2) Display name search order differs if User Info URL is specified #290

Closed philpem closed 2 years ago

philpem commented 2 years ago

I've been investigating an issue where the "Full Name" field in Nextcloud is being populated with the username instead of the user's preferred name. I have the "User Info" URL set in the Social Login plugin's OAuth configuration.

In lib/Provider/CustomOpenIDConnect.php (public function getUserProfile()) there are two code paths to set the user name:

First the display name is set as either the name field from the OAuth data, or the preferred_username field if user is unset:

        $userProfile->displayName = $data->get('name') ?: $data->get('preferred_username');

But if the User Info URL is specified, the plugin makes a request to the User Info endpoint and does this:

            $userProfile->displayName = $profile->get('preferred_username') ?: $profile->get('nickname') ?: $profile->get('name');

This means that if you have a User Info endpoint set, the display name will default to the preferred_username, but fall back to the nickname and then the name. When authenticating against Keycloak, this means the user's display name will always be their Keycloak username.

If the User Info endpoint is removed from the configuration, the plugin will use name (for which Keycloak sends the user's full name) and fall back to preferred_username -- which seems closer to the correct (or at least what I'd prefer) behaviour.

Edit: Disabling the User Info URL didn't work as a workaround. This causes an error when logging in with Keycloak, that the user has no group mappings. I expect this is because Keycloak by default only sends the group information as part of the user information endpoint response.

zorn-v commented 2 years ago

Yep, I made some experiments with it. Inconsistency as I see ) Many providers, many rules... Seems "Display name claim" is needed in options, which get priority if specified.

zorn-v commented 2 years ago

Set claims priority in token and user endpoint the same, and added "Display name claim" in v4.8.0

manning-ncsa commented 2 years ago

The coincidence of this update is amazing. I only just started exploring how to configure Keycloak with Nextcloud yesterday, and I was stuck on this very issue. When I discovered this improvement all I had to do was update the app and my problem was solved. Thank you and keep up the good work!