LDAP fixes (#6533)

* Add iCheck png files to webpack config (inconsistency for css <> png) and blue.png to public folder * php 7.3 collect() fix (undefined variable) * Fix travis ci * Add iCheck png files to webpack config (inconsistency for css <> png) and blue.png to public folder * php 7.3 collect() fix (undefined variable) * change LDAP implementation from model to (singleton) service * Re-apply check for content in ldap_server variable before parsing * Update LDAP implementation * Switch iCheck to minimal as referenced in js * Don't init on load but on first access via init (returns ldap enabled status) * Re-Enable notifications * Re-add missing test target php versions * Only init() once (singleton class, so ldap variable is already set)
snipe · Jan 10, 2019 · 1de9087 · 1de9087
1 parent c23cdb0
commit 1de9087
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 61 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -29,6 +29,7 @@ before_script:
   - mysql -e 'CREATE USER "travis'@'localhost";'
   - mysql -e 'GRANT ALL PRIVILEGES ON * . * TO "travis'@'localhost";'
   - mysql -e 'FLUSH PRIVILEGES;'
+  - cp .env.testing-ci .env
   - composer self-update
   - composer install -n --prefer-source
   - chmod -R 777 storage

diff --git a/app/Console/Commands/LdapSync.php b/app/Console/Commands/LdapSync.php
@@ -4,10 +4,9 @@
 
 namespace App\Console\Commands;
 
-use Log;
+use App\Services\LdapAd;
+use Illuminate\Support\Facades\Log;
 use Exception;
-use App\Models\User;
-use App\Models\LdapAd;
 use App\Models\Location;
 use Illuminate\Console\Command;
 use Adldap\Models\User as AdldapUser;
@@ -48,13 +47,6 @@ class LdapSync extends Command
      */
     private $ldap;
 
-    /**
-     * LDAP settings collection.
-     *
-     * @var \Illuminate\Support\Collection
-     */
-    private $settings = null;
-
     /**
      * A default location collection.
      *
@@ -92,13 +84,16 @@ class LdapSync extends Command
 
     /**
      * Create a new command instance.
+     *
+     * @param LdapAd $ldap
      */
     public function __construct(LdapAd $ldap)
     {
+
         parent::__construct();
-        $this->ldap     = $ldap;
-        $this->settings = $this->ldap->ldapSettings;
         $this->summary  = collect();
+
+        $this->ldap = $ldap;
     }
 
     /**
@@ -333,7 +328,7 @@ private function getUserDefaultLocation(): void
      */
     private function checkIfLdapIsEnabled(): void
     {
-        if (false === $this->settings['ldap_enabled']) {
+        if (!$this->ldap->init()) {
             $msg = 'LDAP intergration is not enabled. Exiting sync process.';
             $this->info($msg);
             Log::info($msg);

diff --git a/app/Http/Controllers/Api/SettingsController.php b/app/Http/Controllers/Api/SettingsController.php
@@ -2,12 +2,9 @@
 
 namespace App\Http\Controllers\Api;
 
-use DB;
-use Mail;
-use Validator;
-use Notification;
-use App\Models\Ldap;
-use App\Models\LdapAd;
+use App\Services\LdapAd;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Notification;
 use App\Models\Setting;
 use Illuminate\Http\Request;
 use App\Notifications\MailTest;
@@ -32,8 +29,8 @@ class SettingsController extends Controller
      * @return \Illuminate\Http\JsonResponse
      */
     public function ldapAdSettingsTest(LdapAd $ldap): JsonResponse
-    {   
-        if($ldap->ldapSettings['ldap_enabled'] === false) {
+    {
+        if(!$ldap->init()) {
             Log::info('LDAP is not enabled cannot test.');
             return response()->json(['message' => 'LDAP is not enabled, cannot test.'], 400);
         }

diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
@@ -2,21 +2,19 @@
 
 namespace App\Http\Controllers\Auth;
 
-use Validator;
+use App\Services\LdapAd;
+use Illuminate\Support\Carbon;
+use Illuminate\Support\Facades\Session;
+use Illuminate\Support\Facades\Validator;
 use App\Http\Controllers\Controller;
 use Illuminate\Foundation\Auth\ThrottlesLogins;
 use App\Models\Setting;
-use App\Models\Ldap;
 use App\Models\User;
-use Auth;
-use Config;
+use Illuminate\Support\Facades\Auth;
 use Illuminate\Http\Request;
-use Input;
+use Illuminate\Support\Facades\Input;
 use Redirect;
-use Log;
-use View;
-use PragmaRX\Google2FA\Google2FA;
-use App\Models\LdapAd;
+use Illuminate\Support\Facades\Log;
 
 /**
  * This controller handles authentication for the user, including local
@@ -41,23 +39,23 @@ class LoginController extends Controller
     protected $redirectTo = '/';
 
     /**
-     * An LdapAd instance
-     *
-     * @var \App\Models\LdapAd
+     * @var LdapAd
      */
-    protected $ldapAd;
+    protected $ldap;
 
     /**
      * Create a new authentication controller instance.
      *
+     * @param LdapAd $ldap
+     *
      * @return void
      */
-    public function __construct(LdapAd $ldapAd)
+    public function __construct(LdapAd $ldap)
     {
+        parent::__construct();
         $this->middleware('guest', ['except' => ['logout','postTwoFactorAuth','getTwoFactorAuth','getTwoFactorEnroll']]);
-        \Session::put('backUrl', \URL::previous());
-
-        $this->ldapAd = $ldapAd;
+        Session::put('backUrl', \URL::previous());
+        $this->ldap = $ldap;
     }
 
     function showLoginForm(Request $request)
@@ -85,12 +83,12 @@ function showLoginForm(Request $request)
      * 
      * @return User
      * 
-     * @throws Exception
+     * @throws \Exception
      */
     private function loginViaLdap(Request $request): User
     {
         try {
-            return $this->ldapAd->ldapLogin($request->input('username'), $request->input('password'));
+            return $this->ldap->ldapLogin($request->input('username'), $request->input('password'));
         } catch (\Exception $ex) {
             LOG::debug("LDAP user login: " . $ex->getMessage());
             throw new \Exception($ex->getMessage());
@@ -146,7 +144,7 @@ public function login(Request $request)
         $user = null;
 
         // Should we even check for LDAP users?
-        if (Setting::getSettings()->ldap_enabled=='1') {
+        if ($this->ldap->init()) {
             LOG::debug("LDAP is enabled.");
             try {
                 LOG::debug("Attempting to log user in by LDAP authentication.");
@@ -179,8 +177,8 @@ public function login(Request $request)
         }
 
         if ($user = Auth::user()) {
-            $user->last_login = \Carbon::now();
-            \Log::debug('Last login:'.$user->last_login);
+            $user->last_login = Carbon::now();
+            Log::debug('Last login:'.$user->last_login);
             $user->save();
         }
         // Redirect to the users page
@@ -233,6 +231,8 @@ public function getTwoFactorAuth()
     /**
      * Two factor code submission
      *
+     * @param Request $request
+     *
      * @return Redirect
      */
     public function postTwoFactorAuth(Request $request)
@@ -263,6 +263,8 @@ public function postTwoFactorAuth(Request $request)
     /**
      * Logout page.
      *
+     * @param Request $request
+     *
      * @return Redirect
      */
     public function logout(Request $request)
@@ -327,7 +329,7 @@ protected function sendLockoutResponse(Request $request)
     * Override the lockout time and duration
     *
     * @param  \Illuminate\Http\Request  $request
-    * @return \Illuminate\Http\RedirectResponse
+    * @return  bool
     */
     protected function hasTooManyLoginAttempts(Request $request)
     {

diff --git a/app/Http/Controllers/Users/LDAPImportController.php b/app/Http/Controllers/Users/LDAPImportController.php
@@ -3,10 +3,10 @@
 namespace App\Http\Controllers\Users;
 
 use App\Models\Ldap;
+use App\Services\LdapAd;
 use Illuminate\Http\Request;
 use App\Http\Controllers\Controller;
 use Illuminate\Support\Facades\Artisan;
-use App\Models\LdapAd;
 
 class LDAPImportController extends Controller
 {
@@ -24,6 +24,7 @@ class LDAPImportController extends Controller
      */
     public function __construct(LdapAd $ldap)
     {
+        parent::__construct();
         $this->ldap = $ldap;
     }
 

diff --git a/app/Http/Controllers/ViewAssetsController.php b/app/Http/Controllers/ViewAssetsController.php
@@ -63,7 +63,7 @@ public function getRequestableIndex()
         $assets = Asset::with('model', 'defaultLoc', 'location', 'assignedTo', 'requests')->Hardware()->RequestableAssets()->get();
         $models = AssetModel::with('category', 'requests', 'assets')->RequestableModels()->get();
 
-        return view('account/requestable-assets', compact('user', 'assets', 'models'));
+        return view('account/requestable-assets', compact('assets', 'models'));
     }
 
 

diff --git a/app/Providers/LdapServiceProvider.php b/app/Providers/LdapServiceProvider.php
@@ -0,0 +1,29 @@
+<?php namespace App\Providers;
+
+use App\Services\LdapAd;
+use Illuminate\Support\ServiceProvider;
+
+class LdapServiceProvider extends ServiceProvider
+{
+
+    /**
+     * Bootstrap the application services.
+     *
+     * @return void
+     */
+    public function boot()
+    {
+        $this->app->singleton(LdapAd::class, LdapAd::class);
+    }
+
+
+    /**
+     * Register any application services.
+     *
+     * @return void
+     */
+    public function register()
+    {
+
+    }
+}
diff --git a/app/Models/LdapAd.php → app/Services/LdapAd.php b/app/Models/LdapAd.php → app/Services/LdapAd.php
@@ -2,11 +2,11 @@
 
 declare(strict_types=1);
 
-namespace App\Models;
+namespace App\Services;
 
+use App\Models\User;
 use Exception;
 use Adldap\Adldap;
-use App\Traits\UserTrait;
 use Adldap\Query\Paginator;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Log;
@@ -22,8 +22,6 @@
  */
 class LdapAd extends LdapAdConfiguration
 {
-    use UserTrait;
-
     /**
      * @see https://wdmsb.wordpress.com/2014/12/03/descriptions-of-active-directory-useraccountcontrol-value/
      */
@@ -49,18 +47,29 @@ class LdapAd extends LdapAdConfiguration
     protected $ldap;
 
     /**
-     * __construct.
+     * Initialize LDAP from user settings
+     *
+     * @since 5.0.0
+     *
+     * @return bool
      */
-    public function __construct()
+    public function init() : bool
     {
+        // Already initialized
+        if($this->ldap) {
+            return true;
+        }
+
+        parent::init();
         if($this->isLdapEnabled()) {
-            parent::__construct();
             $this->ldap = new Adldap();
             $this->ldap->addProvider($this->ldapConfig);
+            return true;
         }
+        return false;
     }
 
-    /**
+        /**
      * Create a user if they successfully login to the LDAP server.
      *
      * @author Wes Hulette <jwhulette@gmail.com>
@@ -250,7 +259,7 @@ private function getActiveStatus(AdldapUser $user): int
      *
      * @since 5.0.0
      *
-     * @param Adldap\Models\User $user
+     * @param \Adldap\Models\User $user
      * @param Collection|null    $defaultLocation
      * @param Collection|null    $mappedLocations
      *

diff --git a/app/Models/LdapAdConfiguration.php → app/Services/LdapAdConfiguration.php b/app/Models/LdapAdConfiguration.php → app/Services/LdapAdConfiguration.php
@@ -2,8 +2,9 @@
 
 declare(strict_types=1);
 
-namespace App\Models;
+namespace App\Services;
 
+use App\Models\Setting;
 use Exception;
 use Illuminate\Support\Collection;
 
@@ -38,10 +39,11 @@ class LdapAdConfiguration
     public $ldapConfig;
 
     /**
-     * __construct.
+     * Initialize LDAP from user settings
+     *
+     * @since 5.0.0
      */
-    public function __construct()
-    {
+    public function init() {
         $this->ldapSettings = $this->getSnipeItLdapSettings();
         if ($this->isLdapEnabled()) {
             $this->setSnipeItConfig();
@@ -92,7 +94,7 @@ private function getSnipeItLdapSettings(): Collection
                     }
                 }
 
-                if (($item) && ('ldap_server' === $key)) {
+                if ($item && 'ldap_server' === $key) {
                     return collect(parse_url($item));
                 }
 
@@ -246,7 +248,7 @@ private function getServerUrlBase(): array
      *
      * @return bool
      */
-    protected function isLdapEnabled(): bool
+    public function isLdapEnabled(): bool
     {
         return $this->ldapSettings && $this->ldapSettings->get('ldap_enabled');
     }

diff --git a/config/app.php b/config/app.php
@@ -277,6 +277,7 @@
         * Custom service provider
         */
         App\Providers\MacroServiceProvider::class,
+        App\Providers\LdapServiceProvider::class,
 
 
     ],

diff --git a/public/css/blue.png b/public/css/blue.png
diff --git a/webpack.mix.js b/webpack.mix.js
@@ -29,6 +29,9 @@ mix
     "./public/css/all.css"
   );
 
+mix.copy(["./node_modules/icheck/skins/minimal/blue.png",
+          "./node_modules/icheck/skins/minimal/blue@2x.png"], "./public/css");
+
 /**
  * Copy, minify and version skins
  */
-Original file line number
+Diff line change
@@ Expand Up / @@ -277,6 +277,7 @@ @@
             * Custom service provider
             */
             App\Providers\MacroServiceProvider::class,
+            App\Providers\LdapServiceProvider::class,
         ],
@@ Expand Down @@