From b0417e5bd7ecd990602a70f7804f7da9ded2b733 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Wed, 3 Nov 2021 15:22:06 -0700 Subject: [PATCH] Finish pulling out the AdLdap2-based LDAP remnants that were still in the system --- .../Controllers/Api/SettingsController.php | 208 ++++++++---------- app/Http/Controllers/Auth/LoginController.php | 58 +++-- app/Models/Ldap.php | 54 +++-- routes/api.php | 4 +- 4 files changed, 175 insertions(+), 149 deletions(-) diff --git a/app/Http/Controllers/Api/SettingsController.php b/app/Http/Controllers/Api/SettingsController.php index 59236731e4..813bca5939 100644 --- a/app/Http/Controllers/Api/SettingsController.php +++ b/app/Http/Controllers/Api/SettingsController.php @@ -2,163 +2,136 @@ namespace App\Http\Controllers\Api; +use Illuminate\Http\Request; use App\Http\Controllers\Controller; -use App\Http\Transformers\LoginAttemptsTransformer; use App\Models\Ldap; +use Validator; use App\Models\Setting; +use Mail; +use App\Notifications\SlackTest; +use Notification; use App\Notifications\MailTest; use GuzzleHttp\Client; -use Illuminate\Http\JsonResponse; -use Illuminate\Http\Request; -use Illuminate\Http\Response; -use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Notification; -use Illuminate\Support\Facades\Storage; -use Illuminate\Support\Facades\Validator; // forward-port of v4 LDAP model for Sync class SettingsController extends Controller { - /** - * Test the ldap settings - * - * @author Wes Hulette - * - * @since 5.0.0 - * - * @param App\Models\LdapAd $ldap - * - * @return \Illuminate\Http\JsonResponse - */ - public function ldapAdSettingsTest(LdapAd $ldap): JsonResponse - { - if (! $ldap->init()) { - Log::info('LDAP is not enabled so we cannot test.'); + + public function ldaptest() + { + $settings = Setting::getSettings(); + + if ($settings->ldap_enabled!='1') { + \Log::debug('LDAP is not enabled cannot test.'); return response()->json(['message' => 'LDAP is not enabled, cannot test.'], 400); } - // The connect, bind and resulting users message - $message = []; + \Log::debug('Preparing to test LDAP connection'); - // This is all kinda fucked right now. The connection test doesn't actually do what you think, - // // and the way we parse the errors - // on the JS side is horrible. - Log::info('Preparing to test LDAP user login'); - // Test user can connect to the LDAP server + $message = []; //where we collect together test messages try { - $ldap->testLdapAdUserConnection(); - $message['login'] = [ - 'message' => 'Successfully connected to LDAP server.', - ]; - } catch (\Exception $ex) { - \Log::debug('Connection to LDAP server '.Setting::getSettings()->ldap_server.' failed. Please check your LDAP settings and try again. Server Responded with error: '.$ex->getMessage()); - - return response()->json( - ['message' => 'Connection to LDAP server '.Setting::getSettings()->ldap_server." failed. Verify that the LDAP hostname is entered correctly and that it can be reached from this web server. \n\nServer Responded with error: ".$ex->getMessage(), - - ], 400); - } - - Log::info('Preparing to test LDAP bind connection'); - // Test user can bind to the LDAP server - try { - Log::info('Testing Bind'); - $ldap->testLdapAdBindConnection(); - $message['bind'] = [ - 'message' => 'Successfully bound to LDAP server.', - ]; - } catch (\Exception $ex) { - Log::info('LDAP Bind failed'); - - return response()->json(['message' => 'Connection to LDAP successful, but we were unable to Bind the LDAP user '.Setting::getSettings()->ldap_uname.". Verify your that your LDAP Bind username and password are correct. \n\nServer Responded with error: ".$ex->getMessage(), - ], 400); - } - - Log::info('Preparing to get sample user set from LDAP directory'); - // Get a sample of 10 users so user can verify the data is correct - $settings = Setting::getSettings(); - try { - Log::info('Testing LDAP sync'); - error_reporting(E_ALL & ~E_DEPRECATED); // workaround for php7.4, which deprecates ldap_control_paged_result - // $users = $ldap->testUserImportSync(); // from AdLdap2 from v5, disabling and falling back to v4's sync code - $users = collect(Ldap::findLdapUsers())->slice(0, 11)->filter(function ($value, $key) { //choosing ELEVEN because one is going to be the count, which we're about to filter out in the next line - return is_int($key); - })->map(function ($item) use ($settings) { - return (object) [ - 'username' => $item[$settings['ldap_username_field']][0] ?? null, - 'employee_number' => $item[$settings['ldap_emp_num']][0] ?? null, - 'lastname' => $item[$settings['ldap_lname_field']][0] ?? null, - 'firstname' => $item[$settings['ldap_fname_field']][0] ?? null, - 'email' => $item[$settings['ldap_email']][0] ?? null, - ]; - }); - if ($users->count() > 0) { - $message['user_sync'] = [ - 'users' => $users, - ]; - } else { - $message['user_sync'] = [ - 'message' => 'Connection to LDAP was successful, however there were no users returned from your query. You should confirm the Base Bind DN above.', + $connection = Ldap::connectToLdap(); + try { + $message['bind'] = ['message' => 'Successfully bound to LDAP server.']; + \Log::debug('attempting to bind to LDAP for LDAP test'); + Ldap::bindAdminToLdap($connection); + $message['login'] = [ + 'message' => 'Successfully connected to LDAP server.', ]; - return response()->json($message, 400); + $users = collect(Ldap::findLdapUsers(null,10))->filter(function ($value, $key) { + return is_int($key); + })->slice(0, 10)->map(function ($item) use ($settings) { + return (object) [ + 'username' => $item[$settings['ldap_username_field']][0] ?? null, + 'employee_number' => $item[$settings['ldap_emp_num']][0] ?? null, + 'lastname' => $item[$settings['ldap_lname_field']][0] ?? null, + 'firstname' => $item[$settings['ldap_fname_field']][0] ?? null, + 'email' => $item[$settings['ldap_email']][0] ?? null, + ]; + }); + if ($users->count() > 0) { + $message['user_sync'] = [ + 'users' => $users, + ]; + } else { + $message['user_sync'] = [ + 'message' => 'Connection to LDAP was successful, however there were no users returned from your query. You should confirm the Base Bind DN above.', + ]; + + return response()->json($message, 400); + } + + return response()->json($message, 200); + } catch (\Exception $e) { + \Log::debug('Bind failed'); + \Log::debug("Exception was: ".$e->getMessage()); + return response()->json(['message' => $e->getMessage()], 400); + //return response()->json(['message' => $e->getMessage()], 500); } - } catch (\Exception $ex) { - Log::info('LDAP sync failed'); - $message['user_sync'] = [ - 'message' => 'Error getting users from LDAP directory, error: '.$ex->getMessage(), - ]; - - return response()->json($message, 400); + } catch (\Exception $e) { + \Log::debug('Connection failed but we cannot debug it any further on our end.'); + return response()->json(['message' => $e->getMessage()], 500); } - return response()->json($message, 200); + } - public function ldaptestlogin(Request $request, LdapAd $ldap) + public function ldaptestlogin(Request $request) { - if (Setting::getSettings()->ldap_enabled != '1') { - \Log::debug('LDAP is not enabled. Cannot test.'); + if (Setting::getSettings()->ldap_enabled!='1') { + \Log::debug('LDAP is not enabled. Cannot test.'); return response()->json(['message' => 'LDAP is not enabled, cannot test.'], 400); } - $rules = [ + + $rules = array( 'ldaptest_user' => 'required', - 'ldaptest_password' => 'required', - ]; + 'ldaptest_password' => 'required' + ); $validator = Validator::make($request->all(), $rules); if ($validator->fails()) { \Log::debug('LDAP Validation test failed.'); - $validation_errors = implode(' ', $validator->errors()->all()); - + $validation_errors = implode(' ',$validator->errors()->all()); return response()->json(['message' => $validator->errors()->all()], 400); } + \Log::debug('Preparing to test LDAP login'); try { - DB::beginTransaction(); //this was the easiest way to invoke a full test of an LDAP login without adding new users to the DB (which may not be desired) + $connection = Ldap::connectToLdap(); + try { + Ldap::bindAdminToLdap($connection); + \Log::debug('Attempting to bind to LDAP for LDAP test'); + try { + $ldap_user = Ldap::findAndBindUserLdap($request->input('ldaptest_user'), $request->input('ldaptest_password')); + if ($ldap_user) { + \Log::debug('It worked! '. $request->input('ldaptest_user').' successfully binded to LDAP.'); + return response()->json(['message' => 'It worked! '. $request->input('ldaptest_user').' successfully binded to LDAP.'], 200); + } + return response()->json(['message' => 'Login Failed. '. $request->input('ldaptest_user').' did not successfully bind to LDAP.'], 400); - // $results = $ldap->ldap->auth()->attempt($request->input('ldaptest_username'), $request->input('ldaptest_password'), true); - // can't do this because that's a protected property. + } catch (\Exception $e) { + \Log::debug('LDAP login failed'); + return response()->json(['message' => $e->getMessage()], 400); + } - $results = $ldap->ldapLogin($request->input('ldaptest_user'), $request->input('ldaptest_password')); // this would normally create a user on success (if they didn't already exist), but for the transaction - if ($results) { - return response()->json(['message' => 'It worked! '.$request->input('ldaptest_user').' successfully binded to LDAP.'], 200); - } else { - return response()->json(['message' => 'Login Failed. '.$request->input('ldaptest_user').' did not successfully bind to LDAP.'], 400); + } catch (\Exception $e) { + \Log::debug('Bind failed'); + return response()->json(['message' => $e->getMessage()], 400); + //return response()->json(['message' => $e->getMessage()], 500); } } catch (\Exception $e) { \Log::debug('Connection failed'); - - return response()->json(['message' => $e->getMessage()], 400); - } finally { - DB::rollBack(); // ALWAYS rollback, whether success or failure + return response()->json(['message' => $e->getMessage()], 500); } + + } + public function slacktest(Request $request) { $slack = new Client([ @@ -187,6 +160,7 @@ class SettingsController extends Controller return response()->json(['message' => 'Something went wrong :( '], 400); } + /** * Test the email configuration * @@ -196,19 +170,19 @@ class SettingsController extends Controller */ public function ajaxTestEmail() { - if (! config('app.lock_passwords')) { + if (!config('app.lock_passwords')) { try { Notification::send(Setting::first(), new MailTest()); - return response()->json(['message' => 'Mail sent to '.config('mail.reply_to.address')], 200); } catch (\Exception $e) { return response()->json(['message' => $e->getMessage()], 500); } } - return response()->json(['message' => 'Mail would have been sent, but this application is in demo mode! '], 200); + } + /** * Delete server-cached barcodes * @@ -266,4 +240,4 @@ class SettingsController extends Controller return (new LoginAttemptsTransformer)->transformLoginAttempts($login_attempt_results, $total); } -} +} \ No newline at end of file diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 2c94cc70b2..3f37efb2b5 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -5,7 +5,7 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; use App\Models\Setting; use App\Models\User; -use App\Services\LdapAd; +use App\Models\Ldap; use App\Services\Saml; use Com\Tecnick\Barcode\Barcode; use Google2FA; @@ -39,11 +39,6 @@ class LoginController extends Controller */ protected $redirectTo = '/'; - /** - * @var LdapAd - */ - protected $ldap; - /** * @var Saml */ @@ -52,12 +47,11 @@ class LoginController extends Controller /** * Create a new authentication controller instance. * - * @param LdapAd $ldap * @param Saml $saml * * @return void */ - public function __construct(/*LdapAd $ldap, */ Saml $saml) + public function __construct(Saml $saml) { parent::__construct(); $this->middleware('guest', ['except' => ['logout', 'postTwoFactorAuth', 'getTwoFactorAuth', 'getTwoFactorEnroll']]); @@ -141,13 +135,47 @@ class LoginController extends Controller */ private function loginViaLdap(Request $request): User { - $ldap = \App::make(LdapAd::class); - try { - return $ldap->ldapLogin($request->input('username'), $request->input('password')); - } catch (\Exception $ex) { - LOG::debug('LDAP user login: '.$ex->getMessage()); - throw new \Exception($ex->getMessage()); - } + Log::debug("Binding user to LDAP."); + $ldap_user = Ldap::findAndBindUserLdap($request->input('username'), $request->input('password')); + if (!$ldap_user) { + Log::debug("LDAP user ".$request->input('username')." not found in LDAP or could not bind"); + throw new \Exception("Could not find user in LDAP directory"); + } else { + Log::debug("LDAP user ".$request->input('username')." successfully bound to LDAP"); + } + + // Check if the user already exists in the database and was imported via LDAP + $user = User::where('username', '=', $request->input('username'))->whereNull('deleted_at')->where('ldap_import', '=', 1)->where('activated', '=', '1')->first(); + Log::debug("Local auth lookup complete"); + + // The user does not exist in the database. Try to get them from LDAP. + // If user does not exist and authenticates successfully with LDAP we + // will create it on the fly and sign in with default permissions + if (!$user) { + Log::debug("Local user ".$request->input('username')." does not exist"); + Log::debug("Creating local user ".$request->input('username')); + + if ($user = Ldap::createUserFromLdap($ldap_user)) { //this handles passwords on its own + Log::debug("Local user created."); + } else { + Log::debug("Could not create local user."); + throw new \Exception("Could not create local user"); + } + // If the user exists and they were imported from LDAP already + } else { + Log::debug("Local user ".$request->input('username')." exists in database. Updating existing user against LDAP."); + + $ldap_attr = Ldap::parseAndMapLdapAttributes($ldap_user); + + if (Setting::getSettings()->ldap_pw_sync=='1') { + $user->password = bcrypt($request->input('password')); + } + $user->email = $ldap_attr['email']; + $user->first_name = $ldap_attr['firstname']; + $user->last_name = $ldap_attr['lastname']; //FIXME (or TODO?) - do we need to map additional fields that we now support? E.g. country, phone, etc. + $user->save(); + } // End if(!user) + return $user; } private function loginViaRemoteUser(Request $request) diff --git a/app/Models/Ldap.php b/app/Models/Ldap.php index 6b23fd54ec..b574820279 100644 --- a/app/Models/Ldap.php +++ b/app/Models/Ldap.php @@ -86,22 +86,22 @@ class Ldap extends Model // If they are, we can skip building the UPN to authenticate against AD if ($ldap_username_field == 'userprincipalname') { $userDn = $username; - } else { + } else { // FIXME - we have to respect the new 'append AD domain to username' setting (which sucks.) // In case they haven't added an AD domain $userDn = ($settings->ad_domain != '') ? $username.'@'.$settings->ad_domain : $username.'@'.$settings->email_domain; } } - \Log::debug('Attempting to login using distinguished name:'.$userDn); - $filterQuery = $settings->ldap_auth_filter_query.$username; - $filter = Setting::getSettings()->ldap_filter; + $filter = Setting::getSettings()->ldap_filter; //TODO - this *does* respect the ldap filter, but I believe that AdLdap2 did *not*. $filterQuery = "({$filter}({$filterQuery}))"; \Log::debug('Filter query: '.$filterQuery); if (! $ldapbind = @ldap_bind($connection, $userDn, $password)) { - if (! $ldapbind = self::bindAdminToLdap($connection)) { + \Log::debug("Status of binding user: $userDn to directory: (directly!) ".($ldapbind ? "success" : "FAILURE")); + if (! $ldapbind = self::bindAdminToLdap($connection)) { // TODO uh, this seems...dangerous? Why would we just switch over to the admin connection? That's too loose, I feel. + \Log::debug("Status of binding Admin user: $userDn to directory instead: ".($ldapbind ? "success" : "FAILURE")); return false; } } @@ -233,11 +233,11 @@ class Ldap extends Model * * @author [A. Gianotto] [] * @since [v3.0] - * @param $ldapatttibutes * @param $base_dn + * @param $count * @return array|bool */ - public static function findLdapUsers($base_dn = null) + public static function findLdapUsers($base_dn = null, $count = -1) { $ldapconn = self::connectToLdap(); $ldap_bind = self::bindAdminToLdap($ldapconn); @@ -256,10 +256,10 @@ class Ldap extends Model // Perform the search do { - // Paginate (non-critical, if not supported by server) - if (! $ldap_paging = @ldap_control_paged_result($ldapconn, $page_size, false, $cookie)) { - throw new Exception('Problem with your LDAP connection. Try checking the Use TLS setting in Admin > Settings. '); - } + // // Paginate (non-critical, if not supported by server) + // if (! $ldap_paging = ldap_search($ldapconn, $page_size, false, $cookie)) { //FIXME! This command doesn't exist anymore? I don't know what to replace it with. maybe nothing? + // throw new Exception('Problem with your LDAP connection. Try checking the Use TLS setting in Admin > Settings. '); + // } if ($filter != '' && substr($filter, 0, 1) != '(') { // wrap parens around NON-EMPTY filters that DON'T have them, for back-compatibility with AdLdap2-based filters $filter = "($filter)"; @@ -267,12 +267,36 @@ class Ldap extends Model $filter = '(cn=*)'; } - $search_results = ldap_search($ldapconn, $base_dn, $filter); - + // HUGE thanks to this article: https://stackoverflow.com/questions/68275972/how-to-get-paged-ldap-queries-in-php-8-and-read-more-than-1000-entries + // which helped me wrap my head around paged results! + \Log::info("ldap conn is: ".$ldapconn." basedn is: $base_dn, filter is: $filter - count is: $count. page size is: $page_size"); + // if a $count is set and it's smaller than $page_size then use that as the page size + $ldap_controls = []; + if($count == -1) { //count is -1 means we have to employ paging to query the entire directory + $ldap_controls = [['oid' => LDAP_CONTROL_PAGEDRESULTS, 'iscritical' => false, 'value' => ['size'=> $page_size, 'cookie' => $cookie]]]; + } + $search_results = @ldap_search($ldapconn, $base_dn, $filter, [], 0, /* $page_size*/ -1, -1, LDAP_DEREF_NEVER, $ldap_controls); + \Log::info("did the search run? I guess so if you got here!"); if (! $search_results) { return redirect()->route('users.index')->with('error', trans('admin/users/message.error.ldap_could_not_search').ldap_error($ldapconn)); // FIXME this is never called in any routed context - only from the Artisan command. So this redirect will never work. } + $errcode = null; + $matcheddn = null; + $errmsg = null; + $referrals = null; + $controls = []; + ldap_parse_result($ldapconn, $search_results, $errcode , $matcheddn , $errmsg , $referrals, $controls); + if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { + // You need to pass the cookie from the last call to the next one + $cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; + \Log::info("okay, at least one more page to go!!!"); + } else { + \Log::info("okay, we're out of pages - no cookie (or empty cookie) was passed"); + $cookie = ''; + } + // Empty cookie means last page + // Get results from page $results = ldap_get_entries($ldapconn, $search_results); if (! $results) { @@ -282,14 +306,14 @@ class Ldap extends Model // Add results to result set $global_count += $results['count']; $result_set = array_merge($result_set, $results); + \Log::info("Total count is: $global_count"); - @ldap_control_paged_result_response($ldapconn, $search_results, $cookie); + // ldap_search($ldapconn, $search_results, $cookie); // FIXME - this function is removed in PHP8 } while ($cookie !== null && $cookie != ''); // Clean up after search $result_set['count'] = $global_count; $results = $result_set; - @ldap_control_paged_result($ldapconn, 0); return $results; } diff --git a/routes/api.php b/routes/api.php index 85895ec85e..175d17587c 100644 --- a/routes/api.php +++ b/routes/api.php @@ -702,10 +702,10 @@ Route::group(['prefix' => 'v1', 'middleware' => 'api'], function () { */ Route::group(['middleware'=> ['auth', 'authorize:superuser'], 'prefix' => 'settings'], function () { - Route::post('ldaptest', + Route::get('ldaptest', [ Api\SettingsController::class, - 'ldapAdSettingsTest' + 'ldaptest' ] )->name('api.settings.ldaptest');