Working on some major refactoring.

stevebauman · stevebauman · commit ead560ccc405 · 2016-05-21T18:37:55.000-04:00
diff --git a/src/AdldapAuthUserProvider.php b/src/AdldapAuthUserProvider.php
@@ -38,36 +38,15 @@ public function retrieveByToken($identifier, $token)
      */
     public function retrieveByCredentials(array $credentials)
     {
-        // Get the search query for users only.
-        $query = $this->newAdldapUserQuery();
+        $user = $this->authenticateWithCredentials($credentials);
 
-        // Make sure the connection is bound
-        // before we try to utilize it.
-        if ($query->getConnection()->isBound()) {
-            // Get the username input attributes.
-            $attributes = $this->getUsernameAttribute();
-
-            // Get the input key.
-            $key = key($attributes);
-
-            // Filter the query by the username attribute and retrieve the first user result.
-            $user = $query->where([$attributes[$key] => $credentials[$key]])->first();
-
-            // If the user is an Adldap User model instance.
-            if ($user instanceof User) {
-                // Retrieve the users login attribute.
-                $username = $this->getUsernameFromUser($user);
-
-                // Retrieve the password from the submitted credentials.
-                $password = $this->getPasswordFromCredentials($credentials);
+        // If the user is an Adldap User model instance.
+        if ($user instanceof User) {
+            // Retrieve the password from the submitted credentials.
+            $password = $this->getPasswordFromCredentials($credentials);
 
-                // Try to log the user in.
-                if (!is_null($password) && $this->authenticate($username, $password)) {
-                    // Login was successful, we'll create a new
-                    // Laravel model with the Adldap user.
-                    return $this->getModelFromAdldap($user, $password);
-                }
-            }
+            // Construct / retrieve the eloquent model from our Adldap user.
+            return $this->getModelFromAdldap($user, $password);
         }
 
         if ($this->getLoginFallback()) {
@@ -82,17 +61,21 @@ public function retrieveByCredentials(array $credentials)
      */
     public function validateCredentials(Authenticatable $user, array $credentials)
     {
-        if ($this->getPasswordSync()) {
-            // If password syncing is enabled. We can hit our
-            // local database to check the hashed password.
+        if ($this->authenticateWithCredentials($credentials)) {
+            // We've authenticated successfully, we'll finally
+            // save the user to our local database.
+            $this->saveModel($user);
+
+            return true;
+        }
+
+        if ($this->getLoginFallback() && $user->exists) {
+            // If the user exists in our local database already and fallback is
+            // enabled, we'll perform standard eloquent authentication.
             return parent::validateCredentials($user, $credentials);
         }
 
-        // We've already performed LDAP authentication on the user
-        // and password synchronization is disabled, therefore
-        // we can't validate the submitted password in our
-        // local database. We'll return true here.
-        return true;
+        return false;
     }
 
     /**
@@ -122,7 +105,17 @@ protected function discoverAdldapFromModel($model)
     }
 
     /**
-     * Authenticates a user against Active Directory.
+     * Checks if we're currently connected to our configured LDAP server.
+     *
+     * @return bool
+     */
+    protected function isConnected()
+    {
+        return $this->getAdldap()->getConnection()->isBound();
+    }
+
+    /**
+     * Authenticates a user against our LDAP connection.
      *
      * @param string $username
      * @param string $password
@@ -135,23 +128,49 @@ protected function authenticate($username, $password)
     }
 
     /**
-     * Returns the configured username from the specified AD user.
+     * Authenticates against Active Directory using the specified credentials.
      *
-     * @param User $user
+     * @param array $credentials
      *
-     * @return string
+     * @return User|false
      */
-    protected function getUsernameFromUser(User $user)
+    protected function authenticateWithCredentials(array $credentials = [])
     {
-        $username = $user->{$this->getLoginAttribute()};
+        // Make sure we're connected to our LDAP server before we run any operations.
+        if ($this->isConnected()) {
+            // Retrieve the Adldap user.
+            $user = $this->newAdldapUserQuery()->where([
+                $this->getUsernameValue() => $this->getUsernameFromCredentials($credentials)
+            ])->first();
+
+            if ($user instanceof User) {
+                // Retrieve the authentication username for the AD user.
+                $username = $this->getUsernameFromAdUser($user);
 
-        if (is_array($username)) {
-            // We'll make sure we retrieve the users first username
-            // attribute if it's contained in an array.
-            $username = Arr::get($username, 0);
+                // Retrieve the users password.
+                $password = $this->getPasswordFromCredentials($credentials);
+
+                // Perform LDAP authentication.
+                if ($this->authenticate($username, $password)) {
+                    // Passed, return the user instance.
+                    return $user;
+                }
+            }
         }
 
-        return $username;
+        return false;
+    }
+
+    /**
+     * Returns the username from the specified credentials.
+     *
+     * @param array $credentials
+     *
+     * @return string
+     */
+    protected function getUsernameFromCredentials(array $credentials = [])
+    {
+        return Arr::get($credentials, $this->getUsernameKey());
     }
 
     /**
diff --git a/src/Middleware/WindowsAuthenticate.php b/src/Middleware/WindowsAuthenticate.php
@@ -68,15 +68,18 @@ public function handle(Request $request, Closure $next)
                 // Double check that we have the correct AD user instance.
                 if ($user instanceof User) {
                     // Retrieve the Eloquent user model from our AD user instance.
+                    // We'll assign the user a random password since we don't
+                    // have access to it through SSO auth.
                     $model = $this->getModelFromAdldap($user, str_random());
 
-                    if ($model instanceof Model) {
-                        // If we've been given the correct object instance, we'll log the user in.
-                        $this->auth->login($model);
+                    // Save model in case of changes.
+                    $this->saveModel($model);
 
-                        // Perform any further operations on the authenticated user model.
-                        $this->handleAuthenticatedUser($model);
-                    }
+                    // Manually log the user in.
+                    $this->auth->login($model);
+
+                    // Perform any further operations on the authenticated user model.
+                    $this->handleAuthenticatedUser($model);
                 }
             }
         }
diff --git a/src/Traits/ImportsUsers.php b/src/Traits/ImportsUsers.php
@@ -4,7 +4,6 @@
 
 use Adldap\Laravel\Facades\Adldap;
 use Adldap\Models\User;
-use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\Config;
@@ -17,7 +16,7 @@ trait ImportsUsers
     abstract public function createModel();
 
     /**
-     * Creates a local User from Active Directory.
+     * Returns an existing or new Eloquent user from the specified Adldap user instance.
      *
      * @param User   $user
      * @param string $password
@@ -26,7 +25,7 @@ abstract public function createModel();
      */
     protected function getModelFromAdldap(User $user, $password)
     {
-        // Get the username attributes.
+        // Get the model key.
         $attributes = $this->getUsernameAttribute();
 
         // Get the model key.
@@ -64,12 +63,12 @@ protected function getModelFromAdldap(User $user, $password)
      * Binds the Adldap User instance to the Eloquent model instance
      * by setting its `adldapUser` public property.
      *
-     * @param User            $user
-     * @param Authenticatable $model
+     * @param User  $user
+     * @param Model $model
      *
-     * @return Authenticatable
+     * @return Model
      */
-    protected function bindAdldapToModel(User $user, Authenticatable $model)
+    protected function bindAdldapToModel(User $user, Model $model)
     {
         $model->adldapUser = $user;
 
@@ -79,12 +78,12 @@ protected function bindAdldapToModel(User $user, Authenticatable $model)
     /**
      * Fills a models attributes by the specified Users attributes.
      *
-     * @param User            $user
-     * @param Authenticatable $model
+     * @param User  $user
+     * @param Model $model
      *
-     * @return Authenticatable
+     * @return Model
      */
-    protected function syncModelFromAdldap(User $user, Authenticatable $model)
+    protected function syncModelFromAdldap(User $user, Model $model)
     {
         foreach ($this->getSyncAttributes() as $modelField => $adField) {
             if ($this->isAttributeCallback($adField)) {
@@ -96,20 +95,18 @@ protected function syncModelFromAdldap(User $user, Authenticatable $model)
             $model->{$modelField} = $value;
         }
 
-        $model->save();
-
         return $model;
     }
 
     /**
      * Syncs the models password with the specified password.
      *
-     * @param Authenticatable $model
-     * @param string          $password
+     * @param Model  $model
+     * @param string $password
      *
-     * @return Authenticatable
+     * @return Model
      */
-    protected function syncModelPassword(Authenticatable $model, $password)
+    protected function syncModelPassword(Model $model, $password)
     {
         // If the developer doesn't want to synchronize AD passwords,
         // we'll set the password to a random 16 character string.
@@ -124,6 +121,18 @@ protected function syncModelPassword(Authenticatable $model, $password)
         return $model;
     }
 
+    /**
+     * Saves the specified model.
+     *
+     * @param Model $model
+     *
+     * @return bool
+     */
+    protected function saveModel(Model $model)
+    {
+        return $model->save();
+    }
+
     /**
      * Returns true / false if the specified string
      * is a callback for an attribute handler.
@@ -230,6 +239,50 @@ protected function getAdldap($provider = null)
         return $ad->getManager()->get($provider);
     }
 
+    /**
+     * Returns the configured username from the specified AD user.
+     *
+     * @param User $user
+     *
+     * @return string
+     */
+    protected function getUsernameFromAdUser(User $user)
+    {
+        $username = $user->{$this->getLoginAttribute()};
+
+        if (is_array($username)) {
+            // We'll make sure we retrieve the users first username
+            // attribute if it's contained in an array.
+            $username = Arr::get($username, 0);
+        }
+
+        return $username;
+    }
+
+    /**
+     * Returns the configured username key.
+     *
+     * For example: 'email' or 'username'.
+     *
+     * @return string
+     */
+    protected function getUsernameKey()
+    {
+        return key($this->getUsernameAttribute());
+    }
+
+    /**
+     * Returns the configured username value.
+     *
+     * For example: 'samaccountname' or 'mail'.
+     *
+     * @return string
+     */
+    protected function getUsernameValue()
+    {
+        return Arr::get($this->getUsernameAttribute(), $this->getUsernameKey());
+    }
+
     /**
      * Returns the configured select attributes when performing
      * queries for authentication and binding for users.
