From b015cff8bd0ca0d388db86fae288c5487fded14b Mon Sep 17 00:00:00 2001
From: Brady Wetherington <uberbrady@gmail.com>
Date: Tue, 6 Oct 2020 18:31:06 -0700
Subject: [PATCH] Modify Snipe-IT v5 AD handling to use the same ldap_host, and
 improve OU handling

---
 app/Console/Commands/LdapSync.php    | 8 ++++----
 app/Services/LdapAd.php              | 8 +++++---
 app/Services/LdapAdConfiguration.php | 7 +++++--
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/app/Console/Commands/LdapSync.php b/app/Console/Commands/LdapSync.php
index 9b0365f630..89a6208e5f 100644
--- a/app/Console/Commands/LdapSync.php
+++ b/app/Console/Commands/LdapSync.php
@@ -197,14 +197,14 @@ class LdapSync extends Command
             } else {
                 $errors = '';
                 foreach ($user->getErrors()->getMessages() as  $error) {
-                    $errors .= $error[0];
+                    $errors .= implode(", ",$error);
                 }
-                $summary['note']   = $userMsg.' was not imported. REASON: '.$errors;
+                $summary['note']   = $snipeUser->getDN().' was not imported. REASON: '.$errors;
                 $summary['status'] = 'ERROR';
             }
         }
 
-        $summary['note'] = ($user->getOriginal('username') ? 'UPDATED' : 'CREATED');
+        // $summary['note'] = ($user->getOriginal('username') ? 'UPDATED' : 'CREATED'); // this seems, kinda, like, superfluous, relative to the $summary['note'] thing above, yeah?
         $this->summary->push($summary);
     }
 
@@ -265,7 +265,7 @@ class LdapSync extends Command
                 $this->info($msg);
             }
 
-            $this->mappedLocations = $locations->pluck('ldap_ou', 'id');
+            $this->mappedLocations = $locations->pluck('ldap_ou', 'id'); // TODO: this seems ok-ish, but the key-> value is going location_id -> OU name, and the primary action here is the opposite of that - going from OU's to location ID's.
         }
     }
 
diff --git a/app/Services/LdapAd.php b/app/Services/LdapAd.php
index 608bf829eb..50e8aec667 100644
--- a/app/Services/LdapAd.php
+++ b/app/Services/LdapAd.php
@@ -286,13 +286,15 @@ class LdapAd extends LdapAdConfiguration
         // Check to see if the user is in a mapped location
         if ($mappedLocations) {
             $location = $mappedLocations->filter(function ($value, $key) use ($user) {
-                if ($user->inOu($value)) {
-                    return $key;
+                //if ($user->inOu($value)) { // <----- *THIS* seems not to be working, and it seems more 'intelligent' - but it's literally just a strpos() call, and it doesn't work quite right against plain strings
+                $user_ou = substr($user->getDn(), -strlen($value)); // get the LAST chars of the user's DN, the count of those chars being the length of the thing we're checking against
+                if(strcasecmp($user_ou, $value) === 0) { // case *IN*sensitive comparision - some people say OU=blah, some say ou=blah. returns 0 when strings are identical (which is a little odd, yeah)
+                    return $key; // WARNING: we are doing a 'filter' - not a regular for-loop. So the answer(s) get "return"ed into the $location array
                 }
             });
 
             if ($location->count() > 0) {
-                $locationId = $location->keys()->first();
+                $locationId = $location->keys()->first(); // from the returned $location array from the ->filter() method above, we return the first match - there should be only one
             }
         }
 
diff --git a/app/Services/LdapAdConfiguration.php b/app/Services/LdapAdConfiguration.php
index 8c912edb53..e9b7e96d42 100644
--- a/app/Services/LdapAdConfiguration.php
+++ b/app/Services/LdapAdConfiguration.php
@@ -248,11 +248,14 @@ class LdapAdConfiguration
      */
     private function getServerUrlBase(): array
     {
-        if ($this->ldapSettings['is_ad']) {
+        /* if ($this->ldapSettings['is_ad']) {
             return collect(explode(',', $this->ldapSettings['ad_domain']))->map(function ($item) {
                 return trim($item);
             })->toArray();
-        }
+        } */ // <- this was the *original* intent of the PR for AdLdap2, but we've been moving away from having
+             // two separate fields - one for "ldap_host" and one for "ad_domain" - towards just using "ldap_host"
+             // ad_domain for us just means "append this domain to your usernames for login, if you click that checkbox"
+             // that's all, nothing more (I hope).
 
         $url = $this->getLdapServerData('host');
         return $url ? [$url] : [];