Flexibilize matching of external user ids and Nextcloud users (SAML, OIDC to LDAP)

[blizzz]

The scenario is: When user A logs in via SSO/SAML the external identity provider submits a user id. An LDAP server is often connected to Nextcloud and is backing the logged in user. So when the login happens we have to exactly match the external uid with the Nextcloud internal user id. When a UUID is being used, there is typically no problem. It is not unlikely though that in some setups different values are being used (like last names, email addresses) or UUIDs change due to LDAP server migrations (not all products allow to specify an UUID). We do see cases about this regularly. Recently, the OIDC backend faces the same situation.

The suggestion to resolve the issue is the following:

• The existing ILDAPProvider is extended with a "findOneUser" method. It shall execute the search for one user based on either a provided LDAP filter, or a provided combination of an attribute and a search term. When exactly one match is found, the IUser object will be returned. If none is found, the return value is null. If more a found a specific exception is being thrown.
• The LDAP backend's implementation of the ILDAPProvider will obviously be equipped with the concrete implementation of above.
• Other login backends that may rely on LDAP, like the SAML/SSO or OIDC backend, will get an additional configuration option to specify an attribute to which the received user id is being compared. When an admin configured it, and an LDAPProvider is available, upon login the backend will try to resolve the user with the afore mentioned method. If no user was found the regular procedure continues, if more than one user were found (exception case) it will be logged accordingly, and the Not Provided error will be shown (background: prevent data leakage).

Performance impact: The scenario is on login only, but even there the impact is not negative. On the contrary it can have a positive impact. This is because on login, we (SAML, OIDC) execute a user search to serve the case that potential new users are being imported to Nextcloud so that a login will not fail. With this new approach this search will not be necessary anymore (the direct lookup will import the user, too), for it is being replaced by a slimmer and more direct search filter: it will contain only one search attribute and no trailing wildcard.
There is further potential to not always look the user, but save the resolved user ids at least for some time after reading them once.

Related: nextcloud/user_saml#563

Discussed previously with @come-nc and @julien-nc

[julien-nc]

Makes sense.

The scenario is on login only, but even there the impact is not negative. On the contrary it can have a positive impact. This is because on login, we (SAML, OIDC) execute a user search to serve the case that potential new users are being imported to Nextcloud so that a login will not fail. With this new approach this search will not be necessary anymore

Not sure I get that. If I understand correctly, on login, when looking for a local user, we would add an extra step that would run first. Something like:

if ldap is enabled:
    localLdapUser = ldapUserProxy.newUserSearchMethod(filter, attribute, searchTerm)

If something is found, great, we saved an LDAP query with a trailing wildcard. We have a matching user, login can succeed.

If nothing was found, we continue as usual with $existingUser = $this->userManager->get($userId); which will trigger an LDAP query with a trailing wildcard.

Did I get it?

With this new approach this search will not be necessary anymore

How do you prevent the $this->userManager->get($userId) call to trigger an LDAP search?

[blizzz]

@julien-nc Yes! $existingUser = $this->userManager->get($userId); does not trigger an LDAP search, it will check only whether an LDAP user mapped to this user id is existing.

[julien-nc]

Ah ok, so we would either run

• localLdapUser = ldapUserProxy.newUserSearchMethod(filter, attribute, searchTerm)
• or what we actually run now: $this->userManager->search($userId, 1, 0); $this->ldapService->syncUser($userId);

See https://github.com/nextcloud/user_oidc/blob/53e7b15b81c159a7c3741c76af98e6505ef19195/lib/Controller/LoginController.php#L544-L551

[blizzz]

Ah ok, so we would either run

* `localLdapUser = ldapUserProxy.newUserSearchMethod(filter, attribute, searchTerm)`

* or what we actually run now: `$this->userManager->search($userId, 1, 0); $this->ldapService->syncUser($userId);`

See https://github.com/nextcloud/user_oidc/blob/53e7b15b81c159a7c3741c76af98e6505ef19195/lib/Controller/LoginController.php#L544-L551

I'd to the second only if there is no configuration given to exactly localize that user, but LDAP enabled (might argue about potential other user backends that may work on discovery, i am not aware of any though).

[thpiron]

Hello, I'd like to contribute to this!

I opened a PR to implement the two first points (on user_ldap app).