From 5cc9134a4cd37899010c155d3026c525b89cefb5 Mon Sep 17 00:00:00 2001 From: Deon George Date: Sat, 26 Apr 2025 18:01:32 +1000 Subject: [PATCH] Change we now store logged in user details in session, instead of cookies. This is so when the session expires, the logged in user details are expired as well, which wasnt happening with cookies. --- app/Classes/LDAP/Server.php | 16 +---- app/Http/Controllers/Auth/LoginController.php | 65 +++++++++---------- app/Http/Middleware/SwapinAuthUser.php | 15 +---- app/Ldap/Guard.php | 20 ++---- bootstrap/providers.php | 2 +- config/logging.php | 2 +- 6 files changed, 46 insertions(+), 74 deletions(-) diff --git a/app/Classes/LDAP/Server.php b/app/Classes/LDAP/Server.php index f6b89cff..239e8d9f 100644 --- a/app/Classes/LDAP/Server.php +++ b/app/Classes/LDAP/Server.php @@ -8,9 +8,7 @@ use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Config; -use Illuminate\Support\Facades\Cookie; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Session; use LdapRecord\LdapRecordException; use LdapRecord\Models\Model; use LdapRecord\Query\Collection as LDAPCollection; @@ -173,16 +171,6 @@ final class Server } catch (LdapRecordException $e) { switch ($e->getDetailedError()?->getErrorCode()) { case 49: - // Since we failed authentication, we should delete our auth cookie - if (Cookie::has('password_encrypt')) { - Log::alert('Clearing user credentials and logging out'); - - Cookie::queue(Cookie::forget('password_encrypt')); - Cookie::queue(Cookie::forget('username_encrypt')); - - Session::invalidate(); - } - abort(401,$e->getDetailedError()->getErrorMessage()); default: @@ -196,8 +184,8 @@ final class Server /** * @note While we are caching our baseDNs, it seems if we have more than 1, * our caching doesnt generate a hit on a subsequent call to this function (before the cache expires). - * IE: If we have 5 baseDNs, it takes 5 calls to this function to case them all. - * @todo Possibly a bug wtih ldaprecord, so need to investigate + * IE: If we have 5 baseDNs, it takes 5 calls to this function to cache them all. + * @todo Possibly a bug with ldaprecord, so need to investigate */ $result = collect(); foreach ($base->namingcontexts as $dn) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 2111e8ba..a85785d1 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -5,41 +5,43 @@ namespace App\Http\Controllers\Auth; use Illuminate\Foundation\Auth\AuthenticatesUsers; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Cookie; +use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\Log; use App\Http\Controllers\Controller; class LoginController extends Controller { - /* - |-------------------------------------------------------------------------- - | Login Controller - |-------------------------------------------------------------------------- - | - | This controller handles authenticating users for the application and - | redirecting them to your home screen. The controller uses a trait - | to conveniently provide its functionality to your applications. - | - */ + /* + |-------------------------------------------------------------------------- + | Login Controller + |-------------------------------------------------------------------------- + | + | This controller handles authenticating users for the application and + | redirecting them to your home screen. The controller uses a trait + | to conveniently provide its functionality to your applications. + | + */ - use AuthenticatesUsers; + use AuthenticatesUsers; - /** - * Where to redirect users after login. - * - * @var string - */ - protected $redirectTo = '/'; + /** + * Where to redirect users after login. + * + * @var string + */ + protected $redirectTo = '/'; - /** - * Create a new controller instance. - * - * @return void - */ - public function __construct() - { - $this->middleware('guest')->except('logout'); - } + /** + * Create a new controller instance. + * + * @return void + */ + public function __construct() + { + $this->middleware('guest') + ->except('logout'); + } protected function credentials(Request $request): array { @@ -58,17 +60,14 @@ class LoginController extends Controller */ public function logout(Request $request) { - // Delete our LDAP authentication cookies - Cookie::queue(Cookie::forget('username_encrypt')); - Cookie::queue(Cookie::forget('password_encrypt')); + $user = Auth::user(); $this->guard()->logout(); - $request->session()->invalidate(); - $request->session()->regenerateToken(); if ($response = $this->loggedOut($request)) { + Log::info(sprintf('Logged out [%s]',$user->dn)); return $response; } @@ -100,4 +99,4 @@ class LoginController extends Controller { return login_attr_name(); } -} +} \ No newline at end of file diff --git a/app/Http/Middleware/SwapinAuthUser.php b/app/Http/Middleware/SwapinAuthUser.php index aef7e95f..e7c24e06 100644 --- a/app/Http/Middleware/SwapinAuthUser.php +++ b/app/Http/Middleware/SwapinAuthUser.php @@ -5,8 +5,9 @@ namespace App\Http\Middleware; use Closure; use Illuminate\Http\Request; use Illuminate\Support\Facades\Config; -use Illuminate\Support\Facades\Cookie; +use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Session; use LdapRecord\Container; use App\Ldap\Connection; @@ -28,21 +29,11 @@ class SwapinAuthUser if (! array_key_exists($key,config('ldap.connections'))) abort(599,sprintf('LDAP default server [%s] configuration doesnt exist?',$key)); - /* - // Rebuild our connection with the authenticated user. if (Session::has('username_encrypt') && Session::has('password_encrypt')) { Config::set('ldap.connections.'.$key.'.username',Crypt::decryptString(Session::get('username_encrypt'))); Config::set('ldap.connections.'.$key.'.password',Crypt::decryptString(Session::get('password_encrypt'))); - } else - */ - - // @todo it seems sometimes we have cookies that show the logged in user, but Auth::user() has expired? - if (Cookie::has('username_encrypt') && Cookie::has('password_encrypt')) { - Config::set('ldap.connections.'.$key.'.username',Cookie::get('username_encrypt')); - Config::set('ldap.connections.'.$key.'.password',Cookie::get('password_encrypt')); - - Log::debug('Swapping out configured LDAP credentials with the user\'s cookie.',['key'=>$key,'user'=>Cookie::get('username_encrypt')]); + Log::debug('Swapping out configured LDAP credentials with the user\'s session.',['key'=>$key]); } // We need to override our Connection object so that we can store and retrieve the logged in user and swap out the credentials to use them. diff --git a/app/Ldap/Guard.php b/app/Ldap/Guard.php index e7ae64f7..4874f68f 100644 --- a/app/Ldap/Guard.php +++ b/app/Ldap/Guard.php @@ -2,26 +2,20 @@ namespace App\Ldap; -use Illuminate\Support\Facades\Cookie; -// use Illuminate\Support\Facades\Crypt; +use Illuminate\Support\Facades\Crypt; +use Illuminate\Support\Facades\Log; use LdapRecord\Auth\Guard as GuardBase; class Guard extends GuardBase { public function attempt(string $username, string $password, bool $stayBound = false): bool { - if ($result = parent::attempt($username,$password,$stayBound)) { - /* - * We can either use our session or cookies to store this. If using session, then Http/Kernel needs to be - * updated to start a session for API calls. - // We need to store our password so that we can swap in the user in during SwapinAuthUser::class middleware - request()->session()->put('username_encrypt',Crypt::encryptString($username)); - request()->session()->put('password_encrypt',Crypt::encryptString($password)); - */ + Log::info(sprintf('Attempting login for [%s] with password [%s]',$username,($password ? str_repeat('*',16) : str_repeat('?',16)))); - // For our API calls, we store the cookie - which our cookies are already encrypted - Cookie::queue('username_encrypt',$username); - Cookie::queue('password_encrypt',$password); + if ($result = parent::attempt($username,$password,$stayBound)) { + // Store user details so we can swap in auth details in SwapinAuthUser + session()->put('username_encrypt',Crypt::encryptString($username)); + session()->put('password_encrypt',Crypt::encryptString($password)); } return $result; diff --git a/bootstrap/providers.php b/bootstrap/providers.php index 38b258d1..daf50626 100644 --- a/bootstrap/providers.php +++ b/bootstrap/providers.php @@ -1,5 +1,5 @@ [ 'driver' => 'daily', 'path' => storage_path('logs/laravel.log'), - 'level' => env('LOG_LEVEL', 'debug'), + 'level' => env('LOG_LEVEL', 'info'), 'days' => env('LOG_DAILY_DAYS', 14), 'replace_placeholders' => true, ],