Skip to content

Handle ldap blocking when no servers exist

Vijay Hawoldar requested to merge vij-saml-ldap-provider-fix into master

What does this MR do and why?

In #358175 (closed) it was reported that when a SM instance stops using LDAP and removes the servers, SAML logins would cause a 500 error.

It was established that this is because a change in !81272 (merged) meant that if there is no matching LDAP servers defined for the user's Identity, the ::Gitlab::Auth::Ldap::Config instantiation would raise an error, where as before, it would have returned early.

This MR addresses the problem by returning early if no LDAP servers are defined (i.e. LDAP is unused) and the user is an LDAP one.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

  1. Checkout master
  2. Setup and enable LDAP locally as per https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/main/doc/howto/ldap.md
  3. Setup and enable SAML locally as per https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/main/doc/howto/saml.md (ensure auto_link_ldap_user is enabled)
  4. Sign-in via LDAP
  5. Sign-out
  6. Disable LDAP in config/gitlab.yml and remove the servers listed in the appropriate section
  7. Restart GDK
  8. Sign-in via SAML
  9. View error as described in the issue
  10. Checkout this branch - vij-saml-ldap-provider-fix
  11. Sign-in via SAML
  12. Successful sign-in 🎉

Notes: to get this to work locally myself, I had to manually modify the LDAP user email address to match that of a SAML user, because I wasn't quite sure how to modify the simple-saml users:

user = User.find_by_username('john')
user.email = "user_1@example.com"
user.save

I then used user1 / user1pass for my SAML sign-in, which matched against the LDAP user john.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Vijay Hawoldar

Merge request reports

Loading