Attempt to link SAML (all OAuth) users to LDAP by email

Zendesk: https://gitlab.zendesk.com/agent/tickets/78679

Currently, during the OAuth process, GitLab will attempt to link an LDAP user - first by querying LDAP for the OAuth 'UID' by LDAP UID and then by DN. This is because the OAuth process, especially in the case of SAML, may contain either the LDAP UID or DN. In this customer's case, the SAML response actually contains the user's email address rather than UID or DN. Currently, that means that the LDAP link fails and the user's groups aren't properly synced via LDAP group sync.

At a minimum we could add a third lookup so we had first lookup by UID, then DN, then email. Unfortunately, this means up to 3 queries each and everything time a user signs in. This logic is in Gitlab::OAuth::User#ldap_person. There are a couple of ways I think we can reduce this:

1. Turn the individual queries into a compound filter query. Instead of (uid=foo) and then (dn=foo), we could build an OR query like (|(uid=foo)(dn=foo)(email=foo)) and take the result. We may need to check if there are multiple values returned and have some order of precedence, but it is probably more performant.
2. Swap the order in the Gitlab::SAML::User#gl_user method. Currently, Gitlab::SAML::User#gl_user first tries to link an LDAP user by querying LDAP, then looks internally for a user with a matching email address. We should look up the local user by email address first and then query LDAP only if the user we find doesn't have an LDAP identity. This will eliminate unnecessary queries to LDAP.

@mydigitalself support would like to see this linking get scheduled soon. Can you chime in and let us know your thoughts?

mentioned in issue #26496 (moved)

changed milestone to %Next 3-6 months

@lbot we are completely full up in 9.5 & 9.6 at the moment, so it's going to be a little while until we can address this.

@mydigitalself Can we target this for 10.0 or 10.1? (Since there will be no 9.6)

@dstanley 9.6 was renamed to 10.0, so it's still (overly!) full I'm afraid :(

We're going to start doing some LDAP work in 10.1 & 10.2 so will look at this for either one of those releases. I'm going temporarily put it at 10.1 and when we do planning over this and next week, will have a better picture of when it can be delivered.

changed milestone to %10.1

added customer label

added backend label

added Deliverable label

assigned to @tiagonbotelho

@dblessing how can I check:

We should look up the local user by email address first and then query LDAP only if the user we find doesn't have an LDAP identity. This will eliminate unnecessary queries to LDAP.

More specifically the part where the user we find does not have an LDAP identity. Do we just check ldap_user?

mentioned in merge request !14216 (merged)

@tiagonbotelho Pretty much, yeah. In Gitlab::Saml::User#gl_user, find_or_create_ldap_user is relatively expensive, so we should find_by_email first, and if it has a result that already has an LDAP identity, we can skip find_or_create_ldap_user.

@DouweM the thing is that https://gitlab.com/gitlab-org/gitlab-ce/blob/13a63ec8ac11689d7955e04824118604245ae09a/lib/gitlab/saml/user.rb#L45 creates a new identity which will break some specs because of this line right here: https://gitlab.com/gitlab-org/gitlab-ce/blob/13a63ec8ac11689d7955e04824118604245ae09a/lib/gitlab/saml/user.rb#L52

@tiagonbotelho Can we skip that line if an identity already exists?

mentioned in issue #2177 (moved)

mentioned in issue #38320

added In review label

closed via commit b40192a9

closed via merge request !14216 (merged)

mentioned in commit b40192a9