From 9913560787a9591f7db5035d10728ec2d5fc5b34 Mon Sep 17 00:00:00 2001 From: Bubka <858858+Bubka@users.noreply.github.com> Date: Thu, 16 Mar 2023 13:25:43 +0100 Subject: [PATCH] Enhance logging during authentication (#163) --- app/Api/v1/Controllers/UserController.php | 3 +++ app/Extensions/WebauthnCredentialBroker.php | 6 ++++++ app/Http/Controllers/Auth/LoginController.php | 18 +++++++++++++++--- .../Auth/WebAuthnManageController.php | 6 +++--- .../Auth/WebAuthnRecoveryController.php | 2 ++ .../Auth/WebAuthnRegisterController.php | 3 +++ app/Services/SettingService.php | 4 ++-- .../Auth/WebAuthnRegisterControllerTest.php | 6 +++++- 8 files changed, 39 insertions(+), 9 deletions(-) diff --git a/app/Api/v1/Controllers/UserController.php b/app/Api/v1/Controllers/UserController.php index ffd5d7f5..2a5b074d 100644 --- a/app/Api/v1/Controllers/UserController.php +++ b/app/Api/v1/Controllers/UserController.php @@ -7,6 +7,7 @@ use App\Api\v1\Resources\UserResource; use App\Http\Controllers\Controller; use Illuminate\Http\Request; use Illuminate\Support\Arr; +use Illuminate\Support\Facades\Log; class UserController extends Controller { @@ -76,6 +77,8 @@ class UserController extends Controller $request->user()['preferences->' . $preferenceName] = $validated['value']; $request->user()->save(); + + Log::info(sprintf('User ID #%s changed its preference %s to %s', $request->user()->id, var_export($preferenceName, true), var_export($validated['value'], true))); return response()->json([ 'key' => $preferenceName, diff --git a/app/Extensions/WebauthnCredentialBroker.php b/app/Extensions/WebauthnCredentialBroker.php index 4b1cc83d..641f844b 100644 --- a/app/Extensions/WebauthnCredentialBroker.php +++ b/app/Extensions/WebauthnCredentialBroker.php @@ -6,6 +6,7 @@ use App\Models\WebAuthnAuthenticatable; use Closure; use Illuminate\Auth\Passwords\PasswordBroker; use Illuminate\Contracts\Auth\CanResetPassword as CanResetPasswordContract; +use Illuminate\Support\Facades\Log; class WebauthnCredentialBroker extends PasswordBroker { @@ -18,6 +19,9 @@ class WebauthnCredentialBroker extends PasswordBroker */ public function sendResetLink(array $credentials, Closure $callback = null) : string { + /** + * @var \App\Models\User + */ $user = $this->getUser($credentials); if (! $user instanceof WebAuthnAuthenticatable) { @@ -36,6 +40,8 @@ class WebauthnCredentialBroker extends PasswordBroker $user->sendWebauthnRecoveryNotification($token); } + Log::notice(sprintf('Webauthn recovery email sent to user ID #%s', $user->id)); + return static::RESET_LINK_SENT; } diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 246ed4e4..c4a26796 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -37,7 +37,7 @@ class LoginController extends Controller */ public function login(LoginRequest $request) { - Log::info('User login requested'); + Log::info(sprintf('User login requested by %s from %s', var_export($request['email'], true), $request->ip())); // If the class is using the ThrottlesLogins trait, we can automatically throttle // the login attempts for this application. We'll key this by the username and @@ -46,6 +46,12 @@ class LoginController extends Controller $this->hasTooManyLoginAttempts($request)) { $this->fireLockoutEvent($request); + Log::notice(sprintf( + '%s from %s locked-out, too many failed login attempts (using email+password)', + var_export($request['email'], true), + $request->ip() + )); + return $this->sendLockoutResponse($request); } @@ -58,7 +64,13 @@ class LoginController extends Controller // user surpasses their maximum number of attempts they will get locked out. $this->incrementLoginAttempts($request); - Log::info('User login failed'); + Log::notice(sprintf( + 'Failed login for %s from %s - Attemp %d/%d (using email+password)', + var_export($request['email'], true), + $request->ip(), + $this->limiter()->attempts($this->throttleKey($request)), + $this->maxAttempts() + )); return $this->sendFailedLoginResponse($request); } @@ -154,6 +166,6 @@ class LoginController extends Controller $user->last_seen_at = Carbon::now()->format('Y-m-d H:i:s'); $user->save(); - Log::info(sprintf('User ID #%s authenticated using login & pwd', $user->id)); + Log::info(sprintf('User ID #%s authenticated (using email+password)', $user->id)); } } diff --git a/app/Http/Controllers/Auth/WebAuthnManageController.php b/app/Http/Controllers/Auth/WebAuthnManageController.php index 31ef7e3f..d1a933ee 100644 --- a/app/Http/Controllers/Auth/WebAuthnManageController.php +++ b/app/Http/Controllers/Auth/WebAuthnManageController.php @@ -57,12 +57,12 @@ class WebAuthnManageController extends Controller // no more registered device exists. // See #110 if (blank($user->webAuthnCredentials()->WhereEnabled()->get())) { - Log::notice('No Webauthn credential enabled, Webauthn settings reset to default'); $request->user()->preferences['useWebauthnOnly'] = false; $request->user()->save(); + Log::notice(sprintf('No more Webauthn credential for user ID #%s, user Webauthn options reset to default', $user->id)); } - - Log::info('Security device deleted'); + + Log::info(sprintf('User ID #%s revoked a security device', $user->id)); return response()->json(null, 204); } diff --git a/app/Http/Controllers/Auth/WebAuthnRecoveryController.php b/app/Http/Controllers/Auth/WebAuthnRecoveryController.php index ba6f9e32..4921a39c 100644 --- a/app/Http/Controllers/Auth/WebAuthnRecoveryController.php +++ b/app/Http/Controllers/Auth/WebAuthnRecoveryController.php @@ -10,6 +10,7 @@ use Illuminate\Foundation\Auth\ResetsPasswords; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Password; use Illuminate\Validation\ValidationException; @@ -53,6 +54,7 @@ class WebAuthnRecoveryController extends Controller } $user->preferences['useWebauthnOnly'] = false; $user->save(); + Log::notice(sprintf('Legacy login restored for user ID #%s', $user->id)); } else { throw new AuthenticationException(); } diff --git a/app/Http/Controllers/Auth/WebAuthnRegisterController.php b/app/Http/Controllers/Auth/WebAuthnRegisterController.php index 5c014ef8..de448162 100644 --- a/app/Http/Controllers/Auth/WebAuthnRegisterController.php +++ b/app/Http/Controllers/Auth/WebAuthnRegisterController.php @@ -5,6 +5,7 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; use Illuminate\Contracts\Support\Responsable; use Illuminate\Http\Response; +use Illuminate\Support\Facades\Log; use Laragear\WebAuthn\Http\Requests\AttestationRequest; use Laragear\WebAuthn\Http\Requests\AttestedRequest; use Laragear\WebAuthn\WebAuthn; @@ -43,6 +44,8 @@ class WebAuthnRegisterController extends Controller public function register(AttestedRequest $request) : Response { $request->save(); + + Log::info(sprintf('User ID #%s registered a new security device', $request->user()->id)); return response()->noContent(); } diff --git a/app/Services/SettingService.php b/app/Services/SettingService.php index 41c13eca..a357f570 100644 --- a/app/Services/SettingService.php +++ b/app/Services/SettingService.php @@ -86,7 +86,7 @@ class SettingService foreach ($settings as $setting => $value) { Option::updateOrCreate(['key' => $setting], ['value' => $value]); - Log::info(sprintf('Setting %s is now %s', var_export($setting, true), var_export($this->restoreType($value), true))); + Log::notice(sprintf('App setting %s set to %s', var_export($setting, true), var_export($this->restoreType($value), true))); } self::buildAndCache(); @@ -100,7 +100,7 @@ class SettingService public function delete(string $name) : void { Option::where('key', $name)->delete(); - Log::info(sprintf('Setting %s deleted', var_export($name, true))); + Log::notice(sprintf('App setting %s reset to default', var_export($name, true))); self::buildAndCache(); } diff --git a/tests/Feature/Http/Auth/WebAuthnRegisterControllerTest.php b/tests/Feature/Http/Auth/WebAuthnRegisterControllerTest.php index d6b8059e..25f7d279 100644 --- a/tests/Feature/Http/Auth/WebAuthnRegisterControllerTest.php +++ b/tests/Feature/Http/Auth/WebAuthnRegisterControllerTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Http\Auth; use App\Models\User; use Illuminate\Support\Facades\Config; +use Illuminate\Support\Facades\Log; use Laragear\WebAuthn\Http\Requests\AttestationRequest; use Laragear\WebAuthn\Http\Requests\AttestedRequest; use Laragear\WebAuthn\JsonTransport; @@ -69,7 +70,10 @@ class WebAuthnRegisterControllerTest extends FeatureTestCase */ public function test_register_uses_attested_request() : void { - $this->mock(AttestedRequest::class)->expects('save')->andReturn(); + $request = $this->mock(AttestedRequest::class); + + $request->expects('save')->andReturn(); + $request->expects('user')->andReturn($this->user); $this->actingAs($this->user, 'web-guard') ->json('POST', '/webauthn/register')