Git access over HTTPS is broken with LDAP account on Geo secondary

Summary

When using an LDAP account, a user can not git clone over HTTPS from a secondary Geo node

Steps to reproduce

1. Log into primary Geo node with LDAP account
2. git clone a project from the Geo secondary over HTTPS
3. Fill in the correct password when prompted

What is the current bug behavior?

Access denied as if you typed the wrong password.

What is the expected correct behavior?

Cloning into ...

This has a significant impact

Because it completely blocks usage of Geo for a company that is standardizing on HTTPS Git access.

Customer being blocked by this https://gitlab.zendesk.com/agent/tickets/75619

@teemo @victorwu @mydigitalself we would need to find someone else to work on this while Gabriel is on vacation.

There is a chance they might have included LDAP config on their secondary, I am waiting to get this confirmed. If that is the case this could be resolved by fixing that, which would mean this is not a bug.

changed milestone to %9.4

@dewetblomerus resources for 9.3 are pretty planned and full at the moment, so will put to 9.4 as Deliverable.

added Deliverable label

@DouweM if we're able to get through this in 9.3 given it may be a regression wdyt?

@dewetblomerus We probably need to reproduce this locally, happy to help It also potentially affects a customer+ in https://gitlab.zendesk.com/agent/tickets/76039

@brodock Have you tried this before?

I haven't tried LDAP with https with git protocol. It may work with the API key. We can try to add support for LDAP authentication at the git https level, and skip it at the Web authentication. We need yo skip it for the web UI because of the Audiy and brute force protection. As we now have a writabble database for secondary nodes (DR) we may decide to write audit logs there. What you think @stanhu?

@brodock Could you clarify what you mean? I think we should first understand why this doesn't work and then fix it. :)

Just one puzzle piece to add, it does work with a personal access token but I know of at least one customer for whom this is not an acceptable solution.

added SP1 label

@stanhu Ok, the context was only on my mind. I have one assumption I still need to validate:

When we configure LDAP authentication in a secondary node, it will let you authenticate git http(s) requests, but will mess with the login/logout. So we need to double check the Web part and unplug anything related to LDAP when Gitlab::Geo.secondary?

Removing Platform as ~geo is currently effectively run outside of the Platform backlog.

@stanhu is this on your radar to fix?

assigned to @stanhu

@mydigitalself Yes, I'm taking a look at this. I just got LDAP working on my Geo nodes now.

Yeah, I'm not sure if this is really a bug. I think we may just need to update the documentation. Essentially, if LDAP isn't configured on the secondary, the user won't be able to authenticate against an LDAP server. The clone fails here because the secondary thinks LDAP is not enabled:

https://gitlab.com/gitlab-org/gitlab-ee/blob/v9.2.2-ee/lib/gitlab/auth.rb#L47-51

It seems to me that we do want the secondary to be configured to talk to an LDAP server, which can also be a read slave of the primary LDAP.

Consider the alternative: skipping the LDAP verification when Geo is enabled and assuming that if the user is valid if he/she is in the GitLab database. That is dangerous because if a user is removed from the LDAP server, then he/she will still be able to access the secondary Geo node.

@stanhu skipping the LDAP verification is a bad idea(tm).

Imagine a policy where an account is supposed to get locked after a certain number of (consecutive) password mismatches. Even auth using SSH keys is a bit tricky here, I didn't check what happens if that occurs.

Normal LDAP on the primary is currently fine in that respect, the user gets blocked in Gitlab and gets unblocked automatically when the LDAP account gets unlocked.

Your assumption about the local LDAP is correct - we have local AD controllers in the branch offices that we can use to speed up LDAP checks (that also implement the policy about account auto-locking...)

@ulja2852 Thank you for the feedback. I think the action item here is to update the documentation to encourage users to set up local LDAP servers to work with secondaries.

mentioned in commit 120140bd

mentioned in merge request !2098 (merged)

closed via commit 120140bd

mentioned in commit 5c6b66db