diff --git a/app/Models/Ldap.php b/app/Models/Ldap.php index c40ad60f16a1..18011d6365aa 100644 --- a/app/Models/Ldap.php +++ b/app/Models/Ldap.php @@ -102,6 +102,78 @@ public static function connectToLdap() return $connection; } + /** + * Finds user via Admin search *first*, and _then_ try to bind as that user, returning the user attributes on success, + * or false on failure. This enables login when the DN is harder to programmatically 'guess' due to having users in + * various different OU's or other LDAP entities. + */ + public static function findAndBindMultiOU(string $baseDn, string $filterQuery, string $password, int $slow_failure = 3): array|false + { + /** + * If you *don't* set the slow_failure variable, do note that we might permit timing attacks in here - if + * your find results come back 'slow' when a user *does* exist, but fast if they *don't* exist, then you + * can use this to enumerate users. + * + * Even if that's *not* true, we still might have an issue: if we don't find the user, then we don't even _try_ + * to bind as them. Again, that could permit a timing attack. + * + * Instead of checking every little thing, we just wrap everything in a try/catch in order to unify the + * 'slow_failure' treatment. All failures are re-raised as exceptions so that all failures exit from the + * same place. + */ + $connection = null; + $admin_conn = null; + try { + /** + * First we get an 'admin' connection, which will need search permissions. That was already a requirement + * here, so that's not a big lift. But it _is_ possible to configure LDAP to only login, and *not* to be + * able to import lists of users. In that case, this function *will not work* - and you should use the + * legacy 'findAndBindUserLdap' method, below. Otherwise, it looks like this would attempt an anonymous + * bind - which you might want, but you probably don't. + * + **/ + $admin_conn = self::connectToLdap(); + self::bindAdminToLdap($admin_conn); + $results = ldap_search($admin_conn, $baseDn, $filterQuery); + $entry_count = ldap_count_entries($admin_conn, $results); + if ($entry_count != 1) { + throw new \Exception('Wrong number of entries found: ' . $entry_count); + } + $entry = ldap_first_entry($admin_conn, $results); + $user = ldap_get_attributes($admin_conn, $entry); + $userDn = ldap_get_dn($admin_conn, $entry); + if (!$userDn) { + throw new \Exception("No user DN found"); + } + \Log::debug("FOUND DN IS: $userDn"); + // The temptation now is to do ldap_unbind on the $admin_conn, but that gets handled in the 'finally' below. + // I don't know if that means a separate 'connection' is maintained to the LDAP server or not, and would + // definitely prefer to not do that if we can avoid it. But I don't know enough about the LDAP protocol to + // be certain that that happens. + + //now we try to log in (bind) as that found user + $connection = self::connectToLdap(); + $bind_results = ldap_bind($connection, $userDn, $password); + if (!$bind_results) { + throw new \Exception("Unable to bind as user"); + } + return array_change_key_case($user); + } catch (\Exception $e) { + \Log::debug("Exception on fast find-and-bind: " . $e->getMessage()); + if ($slow_failure) { + sleep($slow_failure); + } + return false; //TODO - make this null instead for a slightly nicer type signature + } finally { + if ($admin_conn) { + ldap_unbind($admin_conn); + } + if ($connection) { + ldap_unbind($connection); + } + } + } + /** * Binds/authenticates the user to LDAP, and returns their attributes. @@ -145,6 +217,17 @@ public static function findAndBindUserLdap($username, $password) Log::debug('Filter query: '.$filterQuery); + // only try this if we have an Admin username set; otherwise use the 'legacy' method + if ($settings->ldap_uname) { + // in the fallowing call, we pick a slow-failure of 0 because we might need to fall through to 'legacy' + $fast_bind = self::findAndBindMultiOU($baseDn, $filterQuery, $password, 0); + if ($fast_bind) { + \Log::debug("Fast bind worked"); + return $fast_bind; + } + \Log::debug("Fast bind failed; falling through to legacy bind"); + } + if (! $ldapbind = @ldap_bind($connection, $userDn, $password)) { Log::debug("Status of binding user: $userDn to directory: (directly!) ".($ldapbind ? "success" : "FAILURE")); if (! $ldapbind = self::bindAdminToLdap($connection)) { @@ -152,11 +235,11 @@ public static function findAndBindUserLdap($username, $password) * TODO PLEASE: * * this isn't very clear, so it's important to note: the $ldapbind value is never correctly returned - we never 'return true' from self::bindAdminToLdap() (the function - * just "falls off the end" without ever explictly returning 'true') + * just "falls off the end" without ever explicitly returning 'true') * * but it *does* have an interesting side-effect of checking for the LDAP password being incorrectly encrypted with the wrong APP_KEY, so I'm leaving it in for now. * - * If it *did* correctly return 'true' on a succesful bind, it would _probably_ allow users to log in with an incorrect password. Which would be horrible! + * If it *did* correctly return 'true' on a successful bind, it would _probably_ allow users to log in with an incorrect password. Which would be horrible! * * Let's definitely fix this at the next refactor!!!! *