Add setting to force LDAP UUID to upper/lower case user- and group names

[svenseeberg]

• Resolves: Add force lower/uppercase setting for LDAP UUIDs #44486

Summary

Add a new expert setting for the user_ldap app to convert the LDAP user and group UUID attributes to lower or upper case.

Before I continue with writing tests, I would first like collect feedback for the proposed solution.

Screen before the change:

Screen after the change:

TODO

• Translations
• Tests

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[svenseeberg]

As soon as I get some kind of heads up that this feature is wanted this way, I can open a PR for documentation that updates https://github.com/nextcloud/documentation/blob/master/admin_manual/configuration_user/user_auth_ldap.rst#expert-settings . And for the tests I would be happy to get some kind of input what should be tested. A unit test that validates the different return values for the function should be easy enough.

[come-nc]

Can you explain the usecase?
In the UI you put the option in the internal username but in the code you apply it to the UUID.

UUID and internal user name are two different things, even if by default UUID is used for internal user name.

[svenseeberg]

Can you explain the usecase?

Sure.

Nextcloud usernames are case sensitive. Per default, the user_ldap app uses the ObjectGUID and similar attributes as the username.

Per RFC4122 UUIDs are not case sensitive. This results in unexpected behavior which has already been reported, for example when combining LDAP with OIDC or SAML ( nextcloud/user_saml#406 nextcloud/user_saml#563 ).

Confusing the situation even more: the different handling of upper and lower case UUIDs by different LDAP servers. For example, LDAP via Samba returns uppercase ObjectGUIDs while OpenLDAP (for example via UCS) uses lowercase UUIDs. This makes migrating from one directory service to another, while keeping users in Nextcloud, very difficult. Dumping the database, replacing the UUIDs with their upper or lower case alternative, inserting the dump again and moving the user data directories in theory does work.

However, du to the fact that Nextcloud user_ldap uses UUIDs for mapping case sensitive usernames and UUIDs per definition are not case sensitive, the app should provide some kind of mechanism to deal with the issue.

To sum up the use cases:

• compatibility with different LDAP server implementation when migrating
• compatibility with OIDC or SAML login

In the UI you put the option in the internal username but in the code you apply it to the UUID.

Yes, I'd expect that the LDAP server should not care if the UUIDs are upper or lower case if it is needed for other queries. But it will transform the UUID into upper or lower case and therefore will apply to the Nextcloud username.

UUID and internal user name are two different things, even if by default UUID is used for internal user name.

Sure, but the username is generated from the UUID by default. My reasoning for putting it in this place is that the UUID comes from this function. If it is converted to lower or upper case right in the beginning, it should be used like that everywhere else.

But to be honest: I'm not sure if the place of the code is the best or correct position. In my tests it yielded the expected result (as in "worked").

[come-nc]

From what I see in the code even for UUID the casing from LDAP server is always used.
I’m not sure about this, adding an option for this scares me, it’s yet another option that people will not understand and misuse.

[svenseeberg]

From what I see in the code even for UUID the casing from LDAP server is always used.

Can you be a bit more specific why this is the case? This is the only place I can see which actually fetches the UUID from the LDAP server. The fact that ldapExpertUUIDUserAttr is only used here basically guarantees this AFAICT. If the UUID is converted in this function, the converted case is used everywhere else. I actually tested this with a Nextcloud connected to a Samba LDAP. With the current stable user_ldap version, all users where created with upper case UUIDs as usernames. After patching and switching this setting, new users were created with lower case UUIDs as usernames.

I’m not sure about this, adding an option for this scares me, it’s yet another option that people will not understand and misuse.

This setting is definitely highly dangerous. As are all other settings in the Expert tab. With the default setting, nothing will change for existing Nextcloud installations.

Changing any of the LDAP settings on a running production server, even non-expert, can definitely destroy a lot of data. However, the design choice was made that Nextcloud uses case sensitive usernames. And also the design choice was made that UUIDs from LDAP servers are not normalized to lower or upper case by default. If that would have happened in the past, this would not be necessary.

Also, it does generally not harm to normalize uppercase UUIDs to lowercase or vice versa as long as it is done from the beginning. The UUID can be used in both cases as LDAP servers ignore the case for queries. The following queries will return the same object on all LDAP implementation I'm aware of. Two examples:

Samba:

ldbsearch -H private/sam.ldb "(objectGUID=46D6D630-44C6-4D97-A0DD-DE1E3AF97D35)"
ldbsearch -H private/sam.ldb "(objectGUID=46d6d630-44c6-4d97-a0dd-de1e3af97d35)"

OpenLDAP:

slapcat -a "(entryUUID=46D6D630-44C6-4D97-A0DD-DE1E3AF97D35)"
slapcat -a "(entryUUID=46D6d630-44c6-4d97-a0dd-de1e3af97d35)"

But this setting is the only sane way to get any of these setups to work in a good way (IMHO the OIDC and SAML apps should have a similar setting). And yes, this is a problem that Nextcloud as a software should solve, because it ignored RFC 4122.

SSO with Keycloak via OIDC & Synchronization of Users and Groups with Samba

• Samba uses upper case UUIDs
• Keycloak imports the UUIDs in lower case

When you log in with OIDC, a second user object will be created in Nextcloud because the upper and lower case UUID based usernames do not match.

With this setting, it would be possible to force lower case usernames via LDAP and therefore the user object from LDAP and OIDC will be the same.

Migration from Samba to UCS

• All user objects are created with upper case UUIDs as usernames because Samba returns the UUIDs in upper case
• When migrating to UCS, OpenLDAP will usually return lower case UUIDs

When a user logs in after the migration, a second account is created instead of the old one being reused.

In this case, switching the setting from "accept" to "uppercase" will convert the UUIDs from OpenLDAP to upper case and the existing user accounts continue to be used.

Migrating from UCS to Samba

Basically the inverse procedure to "Migration from Samba to UCS".

[svenseeberg]

I think I have to revise the use cases: migrating between different LDAP servers might be possible with occ ldap:update-uuid. However, the use case with OIDC still seems very valid. But even update-uuid looks a bit odd to me, as it seems to use the (not so permanent) DN to update the UUID (which is supposed to not change in most LDAP setups I know).

[svenseeberg]

From what I see in the code even for UUID the casing from LDAP server is always used.

Okay, now I got it. The case is not applied to https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Access.php#L544-L550. I see 4 options for tackling the issue:

• Rename the new setting to "Force case for username attribute" and apply always. (Currently my preferred option)
• Check if "Internal Username Attribute" is the same as the "UUID Attribute for Users". If yes, apply the case.
• Check if $username[0] does actually contain a string that is a UUID.
• Add a checkbox to apply the case setting to the username attribute regardless of the attribute chosen.

[come-nc]

I think I have to revise the use cases: migrating between different LDAP servers might be possible with occ ldap:update-uuid. However, the use case with OIDC still seems very valid. But even update-uuid looks a bit odd to me, as it seems to use the (not so permanent) DN to update the UUID (which is supposed to not change in most LDAP setups I know).

The normal behavior is to update the DN from the UUID, indeed. This command allows to do the other way around to help with situation of LDAP migration from one server to another, where the UUIDs are changed. It also helps in situations where the configuration of which field is used for UUID is changed.

From what I see in the code even for UUID the casing from LDAP server is always used.

Okay, now I got it. The case is not applied to https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Access.php#L544-L550. I see 4 options for tackling the issue:

• Rename the new setting to "Force case for username attribute" and apply always. (Currently my preferred option)
• Check if "Internal Username Attribute" is the same as the "UUID Attribute for Users". If yes, apply the case.
• Check if $username[0] does actually contain a string that is a UUID.
• Add a checkbox to apply the case setting to the username attribute regardless of the attribute chosen.

I’m not sure I understand the 4 options. If you add a checkbox to force a case on the internal name it should always apply to internal name and leave UUID alone.

[svenseeberg]

I’m not sure I understand the 4 options.

I'll elaborate a bit:

• Rename the new setting to "Force case for username attribute" and apply always. (Currently my preferred option)

We can allow the admin to convert the string returned from the ldapExpertUsernameAttr in https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Access.php#L544-L555. It would be up to the admin to decide for which attributes this setting is useful and for which it isn't.

• Advantages: very generic setting
• Disadvantages: none really (maybe it could accidentally applied to a cn based username)

• Check if "Internal Username Attribute" is the same as the "UUID Attribute for Users". If yes, apply the case.

Instead of letting the admin choose the case for the ldapExpertUsernameAttr, we let the code automatically decide. It will decide based on 2 factors: a) upper or lower case is enforced b) the attribute matches ldapExpertUUIDUserAttr (or the automatically detected value).

• Advantages: the setting would only apply to UUIDs and usernames derived from UUIDs but not to usernames based on the cn or other attributes
• Disadvantages: I think there is too much magic happening here which is hard to explain

• Check if $username[0] does actually contain a string that is a UUID.

If $username in https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Access.php#L550-L553 actually contains a UUIDv4 (validate string with a new function), we apply the setting there.

• Advantages: the setting will only apply to UUIDv4 values
• Disadvantages: needs a function to parse a string to detect if it is a UUIDv4

• Add a checkbox to apply the case setting to the username attribute regardless of the attribute chosen.

Basically an extension to the first option to make it even more explicit. But I think we can skip that. Its not really useful.

If you add a checkbox to force a case on the internal name it should always apply to internal name and leave UUID alone.

Not touching the UUID at all is definitely an option. As it should not matter whether it is upper or lower case, I thought it would be beneficial to have it as the same case as the username. I think there are good reasons for applying and for not applying the setting to the UUID stored in the database.

[blizzz]

In most IdPs like keycloak you can modify the original value before submitting it to the SP. In Keycloak you can have a Javascript Mapper that would uppercase the UUID and you'd be done.

It would also not be an issue once we implement the user lookup from SAML to LDAP and get rid of the SAML-ID to Nextcloud User ID mapping.

I am not a fan of having the configuration there. Either this can be made an automatic lower-casing once when the value is identified as valid UUID – which still bears a risk. Or resort to the options just stated ^

[svenseeberg]

In most IdPs like keycloak you can modify the original value before submitting it to the SP. In Keycloak you can have a Javascript Mapper that would uppercase the UUID and you'd be done.

Yes, I implemented that. However the problem is that new users will receive an upper case UUID username after migrating to Samba while old users have a lower case UUID as username. As a temporary fix, I modified the plugin to convert all UUIDs from LDAP to lower case.

The only workaround here is to replace all user UUIDs in a database dump and insert the DB again and then move all data directories.

The problem with an automatic detection is that it will only solve problems for new Nextcloud installations in the future and not help with all existing servers that already have either uppercase or lowercase usernames.

Of the previously discussed options I think it would be best to rename the setting "Force case for username" and insert the conversion between the following lines: https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Access.php#L555-L556

If you think that such a migration (Samba to UCS or vice versa) is such a rare use case then I will start spending my time on converting the database instead of pushing for this setting ;-)

[blizzz]

It is best dealt with by resolving the hard dependency between the SAML provided id and the Nextcloud user id by a look up mechanism. But, cannot say when it arrives, so for the short term i suppose the dump modification is better for you, but contains its own risks of course.

[svenseeberg]

It is best dealt with by resolving the hard dependency between the SAML provided id and the Nextcloud user id by a look up mechanism.

But wouldn't you have to implement the exact same (or an even more complicated) logic there? The problem is that you would have to implement some logic there as the usernames / lookup IDs are not normalized.

[blizzz]

It is best dealt with by resolving the hard dependency between the SAML provided id and the Nextcloud user id by a look up mechanism.

But wouldn't you have to implement the exact same (or an even more complicated) logic there? The is that you would have to implement some logic there as the usernames / lookup IDs are not normalized.

Not the same, not really complicated, and the setting would not be potentially damaging.

[svenseeberg]

I still think that user_ldap should provide a mechanism to deal with changing LDAP servers w/o having to manually change usernames in the database, but if the maintainers think that it is not worth the effort I can live with that.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

[cluck]

In case you're using user_saml to enable Kerberos/NEGOTIATE login via Apache, mod_auth_gssapi and php-fpm like me, you may be able to convert the lowercase uniqueID retrieved via LookupUserAttr to uppercase using ProxyFCGISetEnvIf:

<Location "/apps/user_saml/saml/login">
    <IfModule mod_auth_gssapi.c>
       LookupUserAttr uniqueID REMOTE_USER_USERID " "
   </IfModule>
</Location>
ProxyFCGISetEnvIf "toupper(reqenv('REDIRECT_REMOTE_USER_USERID')) =~ m|(.+)|" REDIRECT_REMOTE_USER_USERID_UCASE "$1"

Then you'll need to point user_saml to read REDIRECT_REMOTE_USER_USERID_UCASE as "uid".

Same nextcloud/user_saml#406 (comment) here.