From 4cc401eb336bb9757637f4663dc6ad1f918f5cbc Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 26 Sep 2026 14:21:04 +0800 Subject: [PATCH 01/10] fix(2fa): start two-factor sessions only after the password is checked - two-fa/check no longer starts a session from the identity alone, which let the emailed/SMS code stand in for the password and revealed which accounts have 2FA on. It now always answers {twoFaSession: null, isTwoFaEnabled: false} so older consoles fall through to auth/login, which already starts the session after the password check. createTwoFaSessionIfEnabled is deprecated. - Store 2FA sessions with a 600 second TTL. EX was being given an absolute timestamp, so sessions lived for decades. - Invalidate a 2FA session after 5 wrong codes, counted per session so resending a code does not reset the count, and compare codes with hash_equals. - Generate verification codes with random_int instead of mt_rand. --- .../Internal/v1/TwoFaController.php | 15 ++-- src/Models/VerificationCode.php | 2 +- src/Support/TwoFactorAuth.php | 43 ++++++++++- .../Http/AuthControllerLoginBootstrapTest.php | 27 +++++++ tests/Unit/Http/OAuthControllerTest.php | 11 ++- tests/Unit/Http/TwoFaControllerTest.php | 73 +++++++++++++++---- tests/Unit/Support/TwoFactorAuthTest.php | 69 +++++++++++++++++- 7 files changed, 209 insertions(+), 31 deletions(-) diff --git a/src/Http/Controllers/Internal/v1/TwoFaController.php b/src/Http/Controllers/Internal/v1/TwoFaController.php index f81c7d58..15d7a2c9 100644 --- a/src/Http/Controllers/Internal/v1/TwoFaController.php +++ b/src/Http/Controllers/Internal/v1/TwoFaController.php @@ -44,19 +44,20 @@ public function getSystemConfig() } /** - * Check Two-Factor Authentication status for a given user identity. + * Retained for older consoles, which call this before submitting the password. + * + * It used to start a 2FA session from the identity alone, which let the emailed/SMS + * code stand in for the password and revealed which accounts have 2FA enabled. A 2FA + * session is now only started by `auth/login` once the password checks out, so this + * always reports 2FA as off and older consoles continue to the password login. * * @return \Illuminate\Http\Response */ public function checkTwoFactor(Request $request) { - $identity = $request->input('identity'); - $twoFaSession = TwoFactorAuth::createTwoFaSessionIfEnabled($identity); - $isTwoFaEnabled = $twoFaSession !== null; - return response()->json([ - 'twoFaSession' => $twoFaSession, - 'isTwoFaEnabled' => $isTwoFaEnabled, + 'twoFaSession' => null, + 'isTwoFaEnabled' => false, ]); } diff --git a/src/Models/VerificationCode.php b/src/Models/VerificationCode.php index b3ae6efb..c7b45b63 100644 --- a/src/Models/VerificationCode.php +++ b/src/Models/VerificationCode.php @@ -65,7 +65,7 @@ public static function boot() { parent::boot(); static::creating(function ($model) { - $model->code = mt_rand(100000, 999999); + $model->code = random_int(100000, 999999); }); } diff --git a/src/Support/TwoFactorAuth.php b/src/Support/TwoFactorAuth.php index 7eed6c74..649b316d 100644 --- a/src/Support/TwoFactorAuth.php +++ b/src/Support/TwoFactorAuth.php @@ -17,6 +17,16 @@ */ class TwoFactorAuth { + /** + * Lifetime of a 2FA session in seconds. + */ + public const SESSION_TTL = 600; + + /** + * Failed code attempts allowed per 2FA session before it is invalidated. + */ + public const MAX_VERIFY_ATTEMPTS = 5; + /** * Save Two-Factor Authentication settings for System wide usage. * @@ -313,6 +323,10 @@ public static function sendVerificationCode(User $user, int $expiresAfter = 61): /** * Create a Two-Factor Authentication session if enabled. * + * @deprecated a 2FA session must only be started once the user's first factor (password or + * OAuth provider) has been verified, otherwise the code alone is enough to sign in. + * Use start() with the authenticated user instead. + * * @param string $identity the user identity * * @return string|null the Two-Factor Authentication session key, or null if not enabled @@ -470,10 +484,10 @@ public static function verifyCode(string $code, string $token, string $clientTok } // Check if verification code matches user provided code - $verificationCodeMatches = $verificationCode->code === $code; + $verificationCodeMatches = hash_equals((string) $verificationCode->code, $code); if ($verificationCodeMatches) { // Kill the two fa session - Redis::del($twoFaSessionKey); + Redis::del($twoFaSessionKey, static::attemptsKey($twoFaSessionKey)); // Authenticate the user $token = $user->createToken($user->uuid); @@ -481,6 +495,18 @@ public static function verifyCode(string $code, string $token, string $clientTok return $token->plainTextToken; } + // Count the failure against the 2FA session, which outlives any single + // code, so resending codes does not reset the budget. + $attemptsKey = static::attemptsKey($twoFaSessionKey); + $attempts = (int) Redis::incr($attemptsKey); + Redis::expire($attemptsKey, static::SESSION_TTL); + + if ($attempts >= static::MAX_VERIFY_ATTEMPTS) { + Redis::del($twoFaSessionKey, $attemptsKey); + + throw new \Exception('Too many failed verification attempts. Please sign in again.'); + } + throw new \Exception('Verification code does not match.'); } } @@ -594,17 +620,26 @@ public static function forgetTwoFaSession(string $token, string $identity): bool * * @return string the Two-Factor Authentication session key */ - private static function createTwoFaSessionKey(User $user, string $token, bool $storeInCache = true, int $expiresAfter = 600): string + private static function createTwoFaSessionKey(User $user, string $token, bool $storeInCache = true, int $expiresAfter = self::SESSION_TTL): string { $twoFaSessionKey = 'two_fa_session:' . $user->uuid . ':' . $token; if ($storeInCache) { - Redis::set($twoFaSessionKey, $user->uuid, 'EX', now()->addSeconds($expiresAfter)->timestamp); + // EX takes a TTL in seconds, not an absolute timestamp. + Redis::set($twoFaSessionKey, $user->uuid, 'EX', $expiresAfter); } return $twoFaSessionKey; } + /** + * Redis key holding the failed verification attempts for a 2FA session. + */ + private static function attemptsKey(string $twoFaSessionKey): string + { + return $twoFaSessionKey . ':attempts'; + } + /** * Get a user based on the provided identity (email or phone). * diff --git a/tests/Unit/Http/AuthControllerLoginBootstrapTest.php b/tests/Unit/Http/AuthControllerLoginBootstrapTest.php index 1008904d..50dcd298 100644 --- a/tests/Unit/Http/AuthControllerLoginBootstrapTest.php +++ b/tests/Unit/Http/AuthControllerLoginBootstrapTest.php @@ -6,6 +6,7 @@ use Fleetbase\Http\Requests\JoinOrganizationRequest; use Fleetbase\Http\Requests\LoginRequest; use Fleetbase\Models\User; +use Fleetbase\Support\TwoFactorAuth; use Illuminate\Database\Capsule\Manager as Capsule; use Illuminate\Database\Eloquent\Model as EloquentModel; use Illuminate\Events\Dispatcher; @@ -717,9 +718,35 @@ function auth_controller_insert_join_invite(Capsule $capsule, string $companyUui ->and($redis->sets)->toHaveCount(1) ->and($redis->sets[0]['key'])->toStartWith('two_fa_session:11111111-1111-4111-8111-111111111111:') ->and($redis->sets[0]['value'])->toBe('11111111-1111-4111-8111-111111111111') + // A relative TTL in seconds, not an absolute timestamp. + ->and($redis->sets[0]['options'])->toBe(['EX', TwoFactorAuth::SESSION_TTL]) ->and($capsule->getConnection('mysql')->table('personal_access_tokens')->count())->toBe(0); }); +test('login does not start two factor authentication or reveal it when the password is wrong', function () { + $capsule = auth_controller_login_bootstrap_database(); + auth_controller_login_insert_user($capsule, [ + 'type' => 'dispatcher', + 'email_verified_at' => '2026-07-18 10:00:00', + ]); + $capsule->getConnection('mysql')->table('settings')->insert([ + 'key' => 'user.11111111-1111-4111-8111-111111111111.2fa', + 'value' => json_encode(['enabled' => true, 'method' => 'email']), + ]); + + $response = (new AuthController())->login(auth_controller_login_request([ + 'identity' => 'auth@example.test', + 'password' => 'not-the-password', + ])); + $payload = $response->getData(true); + + expect($response->getStatusCode())->toBe(401) + ->and($payload['code'])->toBe('invalid_credentials') + ->and($payload)->not->toHaveKey('twoFaSession') + ->and($payload)->not->toHaveKey('isEnabled') + ->and(app('redis')->sets)->toBe([]); +}); + test('bootstrap returns cached session and organization response contracts', function () { $capsule = auth_controller_login_bootstrap_database(); auth_controller_login_insert_user($capsule, [ diff --git a/tests/Unit/Http/OAuthControllerTest.php b/tests/Unit/Http/OAuthControllerTest.php index 555b3037..21314980 100644 --- a/tests/Unit/Http/OAuthControllerTest.php +++ b/tests/Unit/Http/OAuthControllerTest.php @@ -16,6 +16,7 @@ use Fleetbase\Services\OAuth\OAuthFlowService; use Fleetbase\Services\OAuth\OAuthIdentityService; use Fleetbase\Services\OAuth\OAuthStateService; +use Fleetbase\Support\TwoFactorAuth; use Illuminate\Contracts\Encryption\Encrypter; use Illuminate\Database\Capsule\Manager as Capsule; use Illuminate\Database\Eloquent\Model as EloquentModel; @@ -174,9 +175,12 @@ class OAuthControllerRedisFake { public array $values = []; + public array $sets = []; + public function set(string $key, mixed $value, mixed ...$options): bool { $this->values[$key] = $value; + $this->sets[] = compact('key', 'value', 'options'); return true; } @@ -1046,7 +1050,12 @@ function oauth_controller_handoff(OAuthController $controller, array $redirectQu expect($data)->not->toHaveKey('token') ->and($data['twoFaSession'])->toBeString() ->and($data['linked'])->toBe('fakeprovider') - ->and($capsule->getConnection('mysql')->table('personal_access_tokens')->count())->toBe(0); + ->and($capsule->getConnection('mysql')->table('personal_access_tokens')->count())->toBe(0) + // The 2FA session is started only after the provider has vouched for the user, + // and lives for a relative TTL in seconds rather than an absolute timestamp. + ->and(app('redis')->sets)->toHaveCount(1) + ->and(app('redis')->sets[0]['key'])->toStartWith('two_fa_session:user-1:') + ->and(app('redis')->sets[0]['options'])->toBe(['EX', TwoFactorAuth::SESSION_TTL]); }); it('asks the user to link by hand when automatic linking does not apply', function (array $account, array $config, ?Closure $before = null) { diff --git a/tests/Unit/Http/TwoFaControllerTest.php b/tests/Unit/Http/TwoFaControllerTest.php index f9e77ba7..7ea726cb 100644 --- a/tests/Unit/Http/TwoFaControllerTest.php +++ b/tests/Unit/Http/TwoFaControllerTest.php @@ -31,14 +31,28 @@ public function exists(string $key): bool return array_key_exists($key, $this->values); } - public function del(?string $key): bool + public function del(?string ...$keys): bool { - $this->deleted[] = $key; - unset($this->values[$key]); + foreach ($keys as $key) { + $this->deleted[] = $key; + unset($this->values[$key]); + } return true; } + public function incr(string $key): int + { + $this->values[$key] = (int) ($this->values[$key] ?? 0) + 1; + + return $this->values[$key]; + } + + public function expire(string $key, int $seconds): bool + { + return array_key_exists($key, $this->values); + } + public function connection(): self { return $this; @@ -328,27 +342,58 @@ function two_fa_controller_verification_code(User $user, Carbon $expiresAt, stri ->and(Setting::where('key', 'system.2fa')->value('value'))->not->toBeNull(); }); -test('two fa controller reports enabled sessions only when user level two factor is enabled', function () { - two_fa_controller_database(); +test('two fa controller check never starts a session or reveals whether two factor is enabled', function () { + $redis = two_fa_controller_database(); $user = two_fa_controller_user(); $controller = two_fa_controller(); - $disabled = $controller->checkTwoFactor(Request::create('/int/v1/two-fa/check', 'POST', [ + $disabled = $controller->checkTwoFactor(Request::create('/int/v1/two-fa/check', 'GET', [ 'identity' => $user->email, ])); TwoFactorAuth::saveTwoFaSettingsForUser($user, ['enabled' => true, 'method' => 'email']); - $enabled = $controller->checkTwoFactor(Request::create('/int/v1/two-fa/check', 'POST', [ + $enabled = $controller->checkTwoFactor(Request::create('/int/v1/two-fa/check', 'GET', [ 'identity' => $user->email, ])); + $missing = $controller->checkTwoFactor(Request::create('/int/v1/two-fa/check', 'GET', [ + 'identity' => 'missing@example.test', + ])); - expect($disabled->getData(true))->toBe([ - 'twoFaSession' => null, - 'isTwoFaEnabled' => false, - ]) - ->and($enabled->getData(true)['isTwoFaEnabled'])->toBeTrue() - ->and($enabled->getData(true)['twoFaSession'])->toBeString() - ->and($enabled->getData(true)['twoFaSession'])->not->toContain('two_fa_session'); + // A 2FA session is only started by auth/login once the password has been checked. + $expected = ['twoFaSession' => null, 'isTwoFaEnabled' => false]; + + expect($disabled->getData(true))->toBe($expected) + ->and($enabled->getData(true))->toBe($expected) + ->and($missing->getData(true))->toBe($expected) + ->and($redis->values)->toBe([]); +}); + +test('two fa controller verify invalidates the session after too many wrong codes', function () { + $redis = two_fa_controller_database(); + $user = two_fa_controller_user(); + TwoFactorAuth::saveTwoFaSettingsForUser($user, ['enabled' => true, 'method' => 'email']); + $token = TwoFactorAuth::start($user->email, 10); + $verificationCode = two_fa_controller_verification_code($user, Carbon::now()->addMinutes(5)); + $clientToken = TwoFactorAuth::createClientSessionToken($verificationCode); + + $verify = fn (string $code) => two_fa_controller()->verifyCode(Request::create('/int/v1/two-fa/verify', 'POST', [ + 'code' => $code, + 'token' => $token, + 'clientToken' => $clientToken, + ])); + + $failures = []; + for ($i = 0; $i < TwoFactorAuth::MAX_VERIFY_ATTEMPTS; $i++) { + $failures[] = $verify('000000')->getData(true); + } + $afterLockout = $verify('123456'); + + expect(array_slice($failures, 0, -1))->each->toBe(['errors' => ['Verification code does not match.']]) + ->and(end($failures))->toBe(['errors' => ['Too many failed verification attempts. Please sign in again.']]) + ->and($afterLockout->getStatusCode())->toBe(400) + ->and($afterLockout->getData(true))->toBe(['errors' => ['Verification code is invalid.']]) + ->and($redis->values)->toBe([]) + ->and(app('db')->table('personal_access_tokens')->count())->toBe(0); }); test('two fa controller validates sessions returning existing client tokens expired states and errors', function () { diff --git a/tests/Unit/Support/TwoFactorAuthTest.php b/tests/Unit/Support/TwoFactorAuthTest.php index 7f025861..8c075ed7 100644 --- a/tests/Unit/Support/TwoFactorAuthTest.php +++ b/tests/Unit/Support/TwoFactorAuthTest.php @@ -44,14 +44,28 @@ public function exists(string $key): bool return array_key_exists($key, $this->values); } - public function del(?string $key): bool + public function del(?string ...$keys): bool { - $this->deleted[] = $key; - unset($this->values[$key]); + foreach ($keys as $key) { + $this->deleted[] = $key; + unset($this->values[$key]); + } return true; } + public function incr(string $key): int + { + $this->values[$key] = (int) ($this->values[$key] ?? 0) + 1; + + return $this->values[$key]; + } + + public function expire(string $key, int $seconds): bool + { + return array_key_exists($key, $this->values); + } + public function connection(): self { return $this; @@ -384,6 +398,7 @@ function two_factor_auth_verification_code(User $user, Carbon $expiresAt): Verif ->and($redis->sets)->toHaveCount(1) ->and($redis->sets[0]['key'])->toStartWith('two_fa_session:' . $user->uuid . ':') ->and($redis->sets[0]['value'])->toBe($user->uuid) + ->and($redis->sets[0]['options'])->toBe(['EX', TwoFactorAuth::SESSION_TTL]) ->and(TwoFactorAuth::validateSessionToken($token, $user->email))->toBeTrue() ->and(TwoFactorAuth::validateSessionToken($token, '+19999999999'))->toBeFalse() ->and(TwoFactorAuth::createTwoFaSessionIfEnabled('missing@example.com'))->toBeNull(); @@ -583,7 +598,53 @@ function two_factor_auth_verification_code(User $user, Carbon $expiresAt): Verif expect($accessToken)->toContain('|') ->and(app('db')->table('personal_access_tokens')->where('tokenable_id', $user->uuid)->count())->toBe(1) - ->and($redis->deleted)->toBe([$redis->sets[0]['key']]); + ->and($redis->deleted)->toBe([$redis->sets[0]['key'], $redis->sets[0]['key'] . ':attempts']); +}); + +test('two factor auth invalidates the session after too many failed codes', function () { + [$user, , $redis] = two_factor_auth_fixtures(); + TwoFactorAuth::saveTwoFaSettingsForUser($user, ['enabled' => true, 'method' => 'email']); + + $token = TwoFactorAuth::start($user->email, 10); + $sessionKey = $redis->sets[0]['key']; + $verificationCode = two_factor_auth_verification_code($user, Carbon::now()->addMinutes(5)); + $clientToken = TwoFactorAuth::createClientSessionToken($verificationCode); + + for ($i = 1; $i < TwoFactorAuth::MAX_VERIFY_ATTEMPTS; $i++) { + expect(fn () => TwoFactorAuth::verifyCode('000000', $token, $clientToken)) + ->toThrow(Exception::class, 'Verification code does not match.'); + } + + expect($redis->values[$sessionKey . ':attempts'])->toBe(TwoFactorAuth::MAX_VERIFY_ATTEMPTS - 1) + ->and(fn () => TwoFactorAuth::verifyCode('000000', $token, $clientToken)) + ->toThrow(Exception::class, 'Too many failed verification attempts. Please sign in again.') + ->and($redis->exists($sessionKey))->toBeFalse() + ->and($redis->exists($sessionKey . ':attempts'))->toBeFalse() + // The correct code no longer works once the session is gone. + ->and(fn () => TwoFactorAuth::verifyCode('123456', $token, $clientToken)) + ->toThrow(Exception::class, 'Verification code is invalid.') + ->and(app('db')->table('personal_access_tokens')->count())->toBe(0); +}); + +test('two factor auth keeps counting failed codes across resent codes', function () { + [$user, , $redis] = two_factor_auth_fixtures(); + TwoFactorAuth::saveTwoFaSettingsForUser($user, ['enabled' => true, 'method' => 'email']); + + $token = TwoFactorAuth::start($user->email, 10); + $sessionKey = $redis->sets[0]['key']; + + for ($i = 1; $i < TwoFactorAuth::MAX_VERIFY_ATTEMPTS; $i++) { + $clientToken = TwoFactorAuth::resendCode($user->email, $token); + + expect(fn () => TwoFactorAuth::verifyCode('not-the-code', $token, $clientToken)) + ->toThrow(Exception::class, 'Verification code does not match.'); + } + + $clientToken = TwoFactorAuth::resendCode($user->email, $token); + + expect(fn () => TwoFactorAuth::verifyCode('not-the-code', $token, $clientToken)) + ->toThrow(Exception::class, 'Too many failed verification attempts. Please sign in again.') + ->and($redis->exists($sessionKey))->toBeFalse(); }); test('two factor auth verify code rejects invalid and mismatched codes', function () { From 68b1938850f8e742dc921d5f87ad570aef413d57 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 26 Sep 2026 12:17:01 +0800 Subject: [PATCH 02/10] fix(push): send FCM order notifications with android high priority Without an explicit android.priority, Android held order pushes while the device was in deep Doze and delivered them only when the phone was unlocked or charged. Closes #268 (cherry picked from commit 5bd2ba9232c4f7fced8d502f07ab0906418d3368) --- src/Support/PushNotification.php | 2 ++ tests/Unit/NotificationsAndMailTest.php | 1 + tests/Unit/Support/PushNotificationTest.php | 1 + 3 files changed, 4 insertions(+) diff --git a/src/Support/PushNotification.php b/src/Support/PushNotification.php index a48516df..21bac77a 100644 --- a/src/Support/PushNotification.php +++ b/src/Support/PushNotification.php @@ -52,6 +52,8 @@ public static function createFcmMessage(string $title, string $body, array $data ->data($data) ->custom([ 'android' => [ + // High priority so the push is delivered while the device is in Doze + 'priority' => 'high', 'notification' => [ 'color' => '#4391EA', 'sound' => 'default', diff --git a/tests/Unit/NotificationsAndMailTest.php b/tests/Unit/NotificationsAndMailTest.php index 35e1e286..5787b8c8 100644 --- a/tests/Unit/NotificationsAndMailTest.php +++ b/tests/Unit/NotificationsAndMailTest.php @@ -375,6 +375,7 @@ function notification_chat_channel(array $attributes = []): ChatChannel 'body' => 'Test body', ], 'android' => [ + 'priority' => 'high', 'notification' => [ 'color' => '#4391EA', 'sound' => 'default', diff --git a/tests/Unit/Support/PushNotificationTest.php b/tests/Unit/Support/PushNotificationTest.php index dd8cc6d2..1de7e857 100644 --- a/tests/Unit/Support/PushNotificationTest.php +++ b/tests/Unit/Support/PushNotificationTest.php @@ -350,6 +350,7 @@ protected static function getFcmMessagingClient(): Messaging 'screen' => 'orders.show', ], 'android' => [ + 'priority' => 'high', 'notification' => [ 'color' => '#4391EA', 'sound' => 'default', From 17462b16ecd78de6e00807d9454b7a359529f3fd Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 26 Sep 2026 12:19:32 +0800 Subject: [PATCH 03/10] fix(notifications): resolve notification settings from the notified company NotificationRegistry::notify() read company settings from the session, but it is called from queued listeners and console commands, which have no session. Settings -> Notifications was therefore ignored whenever the queue ran in its own worker. notifyUsingDefinitionName() read the global notification_settings key, which the console never writes. Both now take the company from the notification parameters (a company, or a model with a company_uuid) and fall back to the session. Adds Setting::lookupForCompany() for looking up a company setting without a session. Closes #262 (cherry picked from commit 9c24673e7677c53d8d6adf01eb6b5ae98c40fb2c) --- src/Models/Setting.php | 21 ++- src/Support/NotificationRegistry.php | 35 +++- tests/Unit/Models/SettingModelTest.php | 13 ++ .../Unit/Support/NotificationRegistryTest.php | 160 ++++++++++++++++-- 4 files changed, 213 insertions(+), 16 deletions(-) diff --git a/src/Models/Setting.php b/src/Models/Setting.php index 7c29cacb..9eda4995 100644 --- a/src/Models/Setting.php +++ b/src/Models/Setting.php @@ -279,7 +279,26 @@ public static function lookupFromCompany(string $key, $defaultValue = null) return $defaultValue; } - return static::lookup('company.' . session('company') . '.' . $key, $defaultValue); + return static::lookupForCompany(session('company'), $key, $defaultValue); + } + + /** + * Retrieves a setting for the given company. Use this where there is no company session, + * such as queued jobs and console commands. + * + * @param string|null $companyUuid the company the setting belongs to + * @param string $key the setting key associated with the company + * @param mixed|null $defaultValue the default value to return if the setting or company is not found + * + * @return mixed returns the value of the setting if found, or the default value if not + */ + public static function lookupForCompany(?string $companyUuid, string $key, $defaultValue = null) + { + if (!$companyUuid) { + return $defaultValue; + } + + return static::lookup('company.' . $companyUuid . '.' . $key, $defaultValue); } /** diff --git a/src/Support/NotificationRegistry.php b/src/Support/NotificationRegistry.php index ac55ae02..2714dfe0 100644 --- a/src/Support/NotificationRegistry.php +++ b/src/Support/NotificationRegistry.php @@ -2,6 +2,7 @@ namespace Fleetbase\Support; +use Fleetbase\Models\Company; use Fleetbase\Models\Setting; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\Schema; @@ -28,7 +29,7 @@ class NotificationRegistry \Fleetbase\Models\User::class, \Fleetbase\Models\Group::class, \Fleetbase\Models\Role::class, - \Fleetbase\Models\Company::class, + Company::class, ]; /** @@ -251,7 +252,7 @@ public static function notify($notificationClass, ...$params): void } // resolve settings for notification - $notificationSettings = Setting::lookupCompany('notification_settings'); + $notificationSettings = static::resolveNotificationSettings(...$params); // Get the notification class definition $definition = static::findNotificationRegistrationByDefinition($notificationClass); @@ -324,7 +325,7 @@ public static function notifyUsingDefinitionName($notificationClass, $notificati } // resolve settings for notification - $notificationSettings = Setting::lookup('notification_settings'); + $notificationSettings = static::resolveNotificationSettings(...$params); // iterate the properties to find the notifications key starting with the class $notificationSettingsKey = Str::camel(str_replace('\\', '', $notificationClass)) . '__' . Str::camel($notificationName); @@ -362,6 +363,34 @@ public static function notifyUsingDefinitionName($notificationClass, $notificati } } + /** + * Resolve the notification settings for the company the notification is about. + * + * The company is taken from the notification parameters (a company, or a model + * with a `company_uuid`) because notifications are often sent from queued jobs + * and console commands, where there is no company session. + * + * @param mixed ...$params the parameters passed to the notification class + */ + protected static function resolveNotificationSettings(...$params): mixed + { + $companyUuid = null; + + foreach ($params as $param) { + if ($param instanceof Company) { + $companyUuid = $param->uuid; + break; + } + + if ($param instanceof Model && $param->getAttribute('company_uuid')) { + $companyUuid = $param->getAttribute('company_uuid'); + break; + } + } + + return Setting::lookupForCompany($companyUuid ?? session('company'), 'notification_settings', []); + } + /** * Resolve a notifiable object to an Eloquent model. * diff --git a/tests/Unit/Models/SettingModelTest.php b/tests/Unit/Models/SettingModelTest.php index e6df5fab..4199abcb 100644 --- a/tests/Unit/Models/SettingModelTest.php +++ b/tests/Unit/Models/SettingModelTest.php @@ -329,6 +329,19 @@ function setting_model_database(): array ->and(Setting::getByKey('company.' . $companyUuid . '.dispatch.enabled'))->toBeInstanceOf(Setting::class); }); +it('looks up company settings by company uuid without a session', function () { + setting_model_database(); + session()->flush(); + + $companyUuid = '8b5cc964-2d67-4d9f-8b5d-0aa3070a5b5d'; + Setting::configure('company.' . $companyUuid . '.dispatch.enabled', true); + + expect(Setting::lookupForCompany($companyUuid, 'dispatch.enabled', false))->toBeTrue() + ->and(Setting::lookupForCompany($companyUuid, 'missing.key', 'fallback'))->toBe('fallback') + ->and(Setting::lookupForCompany(null, 'dispatch.enabled', 'fallback'))->toBe('fallback') + ->and(Setting::lookupCompany('dispatch.enabled', 'default'))->toBe('default'); +}); + it('exposes JSON value helpers and database connection checks', function () { setting_model_database(); diff --git a/tests/Unit/Support/NotificationRegistryTest.php b/tests/Unit/Support/NotificationRegistryTest.php index 665debb9..4bfd0d96 100644 --- a/tests/Unit/Support/NotificationRegistryTest.php +++ b/tests/Unit/Support/NotificationRegistryTest.php @@ -381,7 +381,7 @@ function notification_registry_dispatch_database(): Capsule notification_registry_dispatch_database(); Fleetbase\Models\Setting::query()->create([ - 'key' => 'notification_settings', + 'key' => 'company.company-1.notification_settings', 'value' => [ 'notificationRegistryDispatchNotification__manualNotice' => [ 'notifiables' => [ @@ -400,8 +400,9 @@ function notification_registry_dispatch_database(): Capsule ], ]); - $subject = new NotificationRegistryDispatchSubject(); - $subject->uuid = 'subject-2'; + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-2'; + $subject->company_uuid = 'company-1'; NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'manual'); @@ -425,7 +426,7 @@ function notification_registry_dispatch_database(): Capsule notification_registry_dispatch_database(); Fleetbase\Models\Setting::query()->create([ - 'key' => 'notification_settings', + 'key' => 'company.company-1.notification_settings', 'value' => [ 'notificationRegistryDispatchNotification__manualNotice' => [ 'notifiables' => [ @@ -439,8 +440,9 @@ function notification_registry_dispatch_database(): Capsule ], ]); - $subject = new NotificationRegistryDispatchPropertySubject(); - $subject->uuid = 'subject-3'; + $subject = new NotificationRegistryDispatchPropertySubject(); + $subject->uuid = 'subject-3'; + $subject->company_uuid = 'company-1'; NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'property'); @@ -505,7 +507,7 @@ function notification_registry_dispatch_database(): Capsule notification_registry_dispatch_database(); Fleetbase\Models\Setting::query()->create([ - 'key' => 'notification_settings', + 'key' => 'company.company-1.notification_settings', 'value' => [ 'notificationRegistryDispatchNotification__manualNotice' => [ 'notifiables' => [ @@ -519,8 +521,9 @@ function notification_registry_dispatch_database(): Capsule ], ]); - $subject = new NotificationRegistryDispatchPropertySubject(); - $subject->uuid = 'subject-5'; + $subject = new NotificationRegistryDispatchPropertySubject(); + $subject->uuid = 'subject-5'; + $subject->company_uuid = 'company-1'; $subject->setRelation('assignee', NotificationRegistryDispatchTarget::query()->find('target-2')); NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'property'); @@ -539,7 +542,7 @@ function notification_registry_dispatch_database(): Capsule notification_registry_dispatch_database(); Fleetbase\Models\Setting::query()->create([ - 'key' => 'notification_settings', + 'key' => 'company.company-1.notification_settings', 'value' => [ 'notificationRegistryDispatchNotification__manualNotice' => [ 'notifiables' => [ @@ -553,10 +556,143 @@ function notification_registry_dispatch_database(): Capsule ], ]); - $subject = new NotificationRegistryDispatchSubject(); - $subject->uuid = 'subject-6'; + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-6'; + $subject->company_uuid = 'company-1'; NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'ignored'); expect(NotificationRegistryDispatchTarget::$sent)->toBe([]); }); + +function notification_registry_company_settings(string $companyUuid, string $settingsKey, string $targetUuid = 'target-1'): void +{ + Fleetbase\Models\Setting::query()->create([ + 'key' => 'company.' . $companyUuid . '.notification_settings', + 'value' => [ + $settingsKey => [ + 'notifiables' => [ + [ + 'definition' => NotificationRegistryDispatchTarget::class, + 'primaryKey' => 'uuid', + 'key' => $targetUuid, + ], + ], + ], + ], + ]); +} + +test('notification registry resolves company settings from the subject when there is no session', function () { + notification_registry_dispatch_database(); + NotificationRegistry::register(NotificationRegistryDispatchNotification::class); + notification_registry_company_settings('company-1', 'notificationRegistryDispatchNotification__dispatchNotice'); + + // Queued listeners and console commands run without a company session + expect(session()->missing('company'))->toBeTrue(); + + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-queued'; + $subject->company_uuid = 'company-1'; + + NotificationRegistry::notify(NotificationRegistryDispatchNotification::class, $subject, 'queued'); + + expect(NotificationRegistryDispatchTarget::$sent)->toBe([ + [ + 'target' => 'target-1', + 'notification' => NotificationRegistryDispatchNotification::class, + 'subject' => 'subject-queued', + 'label' => 'queued', + ], + ]); +}); + +test('notification registry prefers the subject company over the session company', function () { + notification_registry_dispatch_database(); + session(['company' => 'company-2']); + NotificationRegistry::register(NotificationRegistryDispatchNotification::class); + notification_registry_company_settings('company-1', 'notificationRegistryDispatchNotification__dispatchNotice', 'target-1'); + notification_registry_company_settings('company-2', 'notificationRegistryDispatchNotification__dispatchNotice', 'target-2'); + + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-company'; + $subject->company_uuid = 'company-1'; + + NotificationRegistry::notify(NotificationRegistryDispatchNotification::class, $subject, 'subject-company'); + + expect(array_column(NotificationRegistryDispatchTarget::$sent, 'target'))->toBe(['target-1']); +}); + +test('notification registry resolves company settings from a company parameter', function () { + notification_registry_dispatch_database(); + NotificationRegistry::register(NotificationRegistryDispatchNotification::class); + notification_registry_company_settings('company-1', 'notificationRegistryDispatchNotification__dispatchNotice'); + + $company = new Fleetbase\Models\Company(); + $company->uuid = 'company-1'; + + NotificationRegistry::notify(NotificationRegistryDispatchNotification::class, $company, 'company'); + + expect(NotificationRegistryDispatchTarget::$sent)->toBe([ + [ + 'target' => 'target-1', + 'notification' => NotificationRegistryDispatchNotification::class, + 'subject' => 'company-1', + 'label' => 'company', + ], + ]); +}); + +test('notification registry falls back to the session company when the subject has none', function () { + notification_registry_dispatch_database(); + session(['company' => 'company-1']); + NotificationRegistry::register(NotificationRegistryDispatchNotification::class); + notification_registry_company_settings('company-1', 'notificationRegistryDispatchNotification__dispatchNotice'); + + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-session'; + + NotificationRegistry::notify(NotificationRegistryDispatchNotification::class, $subject, 'session'); + + expect(array_column(NotificationRegistryDispatchTarget::$sent, 'target'))->toBe(['target-1']); +}); + +test('notification registry notifies nobody when no company can be resolved', function () { + notification_registry_dispatch_database(); + NotificationRegistry::register(NotificationRegistryDispatchNotification::class); + notification_registry_company_settings('company-1', 'notificationRegistryDispatchNotification__dispatchNotice'); + + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-orphan'; + + NotificationRegistry::notify(NotificationRegistryDispatchNotification::class, $subject, 'orphan'); + NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Dispatch Notice', $subject, 'orphan'); + + expect(NotificationRegistryDispatchTarget::$sent)->toBe([]); +}); + +test('notification registry no longer reads the global notification settings key', function () { + notification_registry_dispatch_database(); + Fleetbase\Models\Setting::query()->create([ + 'key' => 'notification_settings', + 'value' => [ + 'notificationRegistryDispatchNotification__manualNotice' => [ + 'notifiables' => [ + [ + 'definition' => NotificationRegistryDispatchTarget::class, + 'primaryKey' => 'uuid', + 'key' => 'target-1', + ], + ], + ], + ], + ]); + + $subject = new NotificationRegistryDispatchSubject(); + $subject->uuid = 'subject-global'; + $subject->company_uuid = 'company-1'; + + NotificationRegistry::notifyUsingDefinitionName(NotificationRegistryDispatchNotification::class, 'Manual Notice', $subject, 'global'); + + expect(NotificationRegistryDispatchTarget::$sent)->toBe([]); +}); From a496779dad2a69168f173c7c055a9958933549ce Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 26 Sep 2026 12:39:52 +0800 Subject: [PATCH 04/10] fix(iam): enforce the change-password permission and let invited users set a password POST users/set-password, validate-password and change-password had no SkipAuthorizationCheck, so the generic check required `iam create user`. Invited non-admin users could not set their first password (#263), and non-admins could not change their own password. Password endpoints now follow an AWS-style model: - set-password needs no IAM permission, but works only once, within 24h of accepting an invite (a server-side allowance set by acceptCompanyInvite). It used to accept a new password at any time without the current one. - change-password requires the current password in the same request, and is allowed for admins and Administrators, when the organization allows users to change their own password (new setting, default on), or with the `iam change-password` permission (already seeded, never enforced until now). - validate-password and change-password are throttled per user. - New GET users/password-policy and GET/POST companies/auth-settings. - Password and auth-setting changes are written to the `auth` activity log. Closes #263 (cherry picked from commit 58ab1578513c0473bf4ff0df5bd16c9e3e32b6c4) --- .../Internal/v1/CompanyController.php | 59 ++++ .../Internal/v1/UserController.php | 60 ++++- .../Internal/ChangeCurrentPasswordRequest.php | 77 ++++++ src/Support/Auth.php | 66 +++++ src/routes.php | 7 +- tests/Unit/Http/CompanyControllerTest.php | 42 +++ tests/Unit/Http/UserControllerTest.php | 254 +++++++++++++++++- 7 files changed, 546 insertions(+), 19 deletions(-) create mode 100644 src/Http/Requests/Internal/ChangeCurrentPasswordRequest.php diff --git a/src/Http/Controllers/Internal/v1/CompanyController.php b/src/Http/Controllers/Internal/v1/CompanyController.php index 14f42b4d..b9688074 100644 --- a/src/Http/Controllers/Internal/v1/CompanyController.php +++ b/src/Http/Controllers/Internal/v1/CompanyController.php @@ -2,6 +2,7 @@ namespace Fleetbase\Http\Controllers\Internal\v1; +use Fleetbase\Attributes\SkipAuthorizationCheck; use Fleetbase\Events\UserRemovedFromCompany; use Fleetbase\Exceptions\FleetbaseRequestValidationException; use Fleetbase\Exports\CompanyExport; @@ -14,6 +15,7 @@ use Fleetbase\Models\CompanyUser; use Fleetbase\Models\ExtensionInstall; use Fleetbase\Models\Invite; +use Fleetbase\Models\Setting; use Fleetbase\Models\User; use Fleetbase\Support\Auth; use Fleetbase\Support\TwoFactorAuth; @@ -165,6 +167,63 @@ public function saveTwoFactorSettings(Request $request) return response()->json(['message' => 'Two-Factor Authentication saved successfully']); } + /** + * Get the current organization's authentication settings. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function getAuthSettings() + { + $company = Auth::getCompany(); + + if (!$company) { + return response()->error('No company session found', 401); + } + + return response()->json(Auth::getCompanyAuthSettings($company->uuid)); + } + + /** + * Save the current organization's authentication settings. Only admins and users + * holding the Administrator role may change them. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function saveAuthSettings(Request $request) + { + $user = $request->user(); + $company = Auth::getCompany(); + + if (!$company) { + return response()->error('No company session found', 401); + } + + if (!$user || !($user->isAdmin() || $user->hasRole('Administrator'))) { + return response()->error('Only administrators can change authentication settings.', 403); + } + + if (!$request->has('allow_users_change_password')) { + return response()->error('No authentication settings provided.', 422); + } + + $settings = array_merge(Auth::getCompanyAuthSettings($company->uuid), [ + 'allow_users_change_password' => $request->boolean('allow_users_change_password'), + ]); + + Setting::configure('company.' . $company->uuid . '.auth', $settings); + + activity('auth') + ->causedBy($user) + ->performedOn($company) + ->withProperties($settings) + ->event('auth_settings_updated') + ->log('Authentication settings updated'); + + return response()->json($settings); + } + /** * Get all users for a company. * diff --git a/src/Http/Controllers/Internal/v1/UserController.php b/src/Http/Controllers/Internal/v1/UserController.php index 2dc94412..27bea66f 100644 --- a/src/Http/Controllers/Internal/v1/UserController.php +++ b/src/Http/Controllers/Internal/v1/UserController.php @@ -10,6 +10,7 @@ use Fleetbase\Http\Requests\CreateUserRequest; use Fleetbase\Http\Requests\ExportRequest; use Fleetbase\Http\Requests\Internal\AcceptCompanyInvite; +use Fleetbase\Http\Requests\Internal\ChangeCurrentPasswordRequest; use Fleetbase\Http\Requests\Internal\ChangeCurrentUserEmailRequest; use Fleetbase\Http\Requests\Internal\ChangeUserEmailRequest; use Fleetbase\Http\Requests\Internal\InviteUserRequest; @@ -1026,6 +1027,11 @@ public function acceptCompanyInvite(AcceptCompanyInvite $request) $user->activate(); } + // allow the user to set their first password without a current password + if ($needsPassword) { + Auth::markPasswordSetupPending($user); + } + // create authentication token for user $token = $user->createToken($invite->code); @@ -1365,10 +1371,15 @@ public function removeFromCompany($id) } /** - * Updates the current users password. + * Sets the current user's first password, e.g. right after accepting an invite. + * + * No IAM permission is required because the user cannot skip this step, but it is + * only allowed once, while the password setup allowance is pending. Afterwards the + * password must be changed with `changeUserPassword`. * * @return \Illuminate\Http\Response */ + #[SkipAuthorizationCheck] public function setCurrentUserPassword(UpdatePasswordRequest $request) { $password = $request->input('password'); @@ -1379,7 +1390,14 @@ public function setCurrentUserPassword(UpdatePasswordRequest $request) return response()->error('User not authenticated'); } + if (!Auth::isPasswordSetupPending($user)) { + return response()->error('Your password has already been set. Use change password instead.', 403); + } + $user->changePassword($password); + Auth::clearPasswordSetupPending($user); + + activity('auth')->causedBy($user)->performedOn($user)->event('password_set')->log('Password set'); return response()->json(['status' => 'ok']); } @@ -1417,33 +1435,57 @@ public function export(ExportRequest $request) /** * Validate the user's current password. * + * Only checks the user's own password, so no IAM permission is required. + * * @return \Illuminate\Http\Response */ + #[SkipAuthorizationCheck] public function validatePassword(ValidatePasswordRequest $request) { return response()->json(['status' => 'ok']); } /** - * Change the user's password. + * Change the current user's password. + * + * Requires the current password, and that the user may change their own password + * (see `Auth::canChangeOwnPassword`). The generic resource check is skipped because + * it would require the `create user` permission for this POST. * * @return \Illuminate\Http\Response */ - public function changeUserPassword(UpdatePasswordRequest $request) + #[SkipAuthorizationCheck] + public function changeUserPassword(ChangeCurrentPasswordRequest $request) { - $user = $request->user(); - $newPassword = $request->input('password'); - $newConfirmPassword = $request->input('password_confirmation'); + $user = $request->user(); - if ($newPassword !== $newConfirmPassword) { - return response()->error('Password is not matching'); + if (!Auth::canChangeOwnPassword($user)) { + return response()->error('You are not allowed to change your password. Ask an administrator.', 403); } - $user->changePassword($newPassword); + $user->changePassword($request->input('password')); + Auth::clearPasswordSetupPending($user); + + activity('auth')->causedBy($user)->performedOn($user)->event('password_changed')->log('Password changed'); return response()->json(['status' => 'ok']); } + /** + * Get the current user's password policy. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function getPasswordPolicy(Request $request) + { + $user = $request->user(); + + return response()->json([ + 'can_change_password' => Auth::canChangeOwnPassword($user), + ]); + } + /** * Save the user selected locale. * diff --git a/src/Http/Requests/Internal/ChangeCurrentPasswordRequest.php b/src/Http/Requests/Internal/ChangeCurrentPasswordRequest.php new file mode 100644 index 00000000..1316ff50 --- /dev/null +++ b/src/Http/Requests/Internal/ChangeCurrentPasswordRequest.php @@ -0,0 +1,77 @@ + [ + 'required', + 'string', + function ($attribute, $value, $fail) { + $user = $this->user(); + if (!$user || !$user->checkPassword($value)) { + $fail('The current password provided is invalid.'); + } + }, + ], + 'password' => [ + 'required', + 'string', + 'confirmed', + 'different:current_password', + Password::min(8) + ->mixedCase() + ->letters() + ->numbers() + ->symbols() + ->uncompromised(), + ], + 'password_confirmation' => ['required', 'string'], + ]; + } + + /** + * Get the error messages for the defined validation rules. + * + * @return array + */ + public function messages() + { + return [ + 'current_password.required' => 'The current password is required.', + 'password.required' => 'A new password is required.', + 'password.confirmed' => 'Passwords do not match.', + 'password.different' => 'The new password must be different from the current password.', + 'password.min' => 'Password must be at least 8 characters.', + 'password.mixed' => 'Password must contain both uppercase and lowercase letters.', + 'password.letters' => 'Password must contain at least one letter.', + 'password.numbers' => 'Password must contain at least one number.', + 'password.symbols' => 'Password must contain at least one symbol.', + 'password.uncompromised' => 'This password has appeared in a data breach. Please choose a different one.', + ]; + } +} diff --git a/src/Support/Auth.php b/src/Support/Auth.php index f5a21a4f..9b3a8878 100644 --- a/src/Support/Auth.php +++ b/src/Support/Auth.php @@ -10,8 +10,10 @@ use Fleetbase\Models\Permission; use Fleetbase\Models\Policy; use Fleetbase\Models\Role; +use Fleetbase\Models\Setting; use Fleetbase\Models\User; use Illuminate\Http\Request; +use Illuminate\Support\Carbon; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Auth as Authentication; use Illuminate\Support\Facades\Hash; @@ -94,6 +96,70 @@ public static function canGrantRole(Role $role, ?User $actor): bool return $actor instanceof User && ($actor->isAdmin() || $actor->hasRole('Administrator')); } + /** + * Get the organization's authentication settings, with defaults applied. + */ + public static function getCompanyAuthSettings(?string $companyUuid): array + { + $settings = Setting::lookupForCompany($companyUuid, 'auth', []); + + return [ + 'allow_users_change_password' => Utils::castBoolean(data_get($settings, 'allow_users_change_password', true)), + ]; + } + + /** + * Whether the user may change their own password. Admins and users holding the + * Administrator role always can. Everyone else can when the organization allows + * users to change their own password, or when they hold the + * `iam change-password` permission. + */ + public static function canChangeOwnPassword(User $user): bool + { + if ($user->isAdmin() || $user->hasRole('Administrator')) { + return true; + } + + $settings = static::getCompanyAuthSettings(session('company', $user->company_uuid)); + if ($settings['allow_users_change_password']) { + return true; + } + + $permissions = Permission::findByNames(['iam change-password', 'iam *']); + + return $permissions->isNotEmpty() && $user->companyUser && $user->hasPermissions($permissions); + } + + /** + * Allow the user to set their first password without the current password, for + * example right after accepting an invite. The allowance expires after the given + * number of hours and is used up once the password is set. + */ + public static function markPasswordSetupPending(User $user, int $expiresInHours = 24): void + { + Setting::configure('user.' . $user->uuid . '.password_setup_pending', [ + 'expires_at' => now()->addHours($expiresInHours)->toIso8601String(), + ]); + } + + /** + * Whether the user may still set their first password. + */ + public static function isPasswordSetupPending(User $user): bool + { + $expiresAt = data_get(Setting::lookup('user.' . $user->uuid . '.password_setup_pending'), 'expires_at'); + + return $expiresAt && now()->lt(Carbon::parse($expiresAt)); + } + + /** + * Use up the user's allowance to set their first password. + */ + public static function clearPasswordSetupPending(User $user): void + { + Setting::where('key', 'user.' . $user->uuid . '.password_setup_pending')->delete(); + } + /** * Set session variables for user. * diff --git a/src/routes.php b/src/routes.php index 8aa671d7..b2456af3 100644 --- a/src/routes.php +++ b/src/routes.php @@ -281,6 +281,8 @@ function ($router, $controller) { $router->fleetbaseRoutes('companies', null, [], function ($router, $controller) { $router->get('two-fa', $controller('getTwoFactorSettings')); $router->post('two-fa', $controller('saveTwoFactorSettings')); + $router->get('auth-settings', $controller('getAuthSettings')); + $router->post('auth-settings', $controller('saveAuthSettings')); $router->post('transfer-ownership', $controller('transferOwnership')); $router->post('leave', $controller('leaveOrganization')); $router->match(['get', 'post'], 'export', $controller('export')); @@ -307,8 +309,9 @@ function ($router, $controller) { $router->post('invite-user', $controller('inviteUser')); $router->post('resend-invite', $controller('resendInvitation')); $router->post('set-password', $controller('setCurrentUserPassword')); - $router->post('validate-password', $controller('validatePassword')); - $router->post('change-password', $controller('changeUserPassword')); + $router->post('validate-password', $controller('validatePassword'))->middleware(Illuminate\Routing\Middleware\ThrottleRequests::class . ':10,1'); + $router->post('change-password', $controller('changeUserPassword'))->middleware(Illuminate\Routing\Middleware\ThrottleRequests::class . ':10,1'); + $router->get('password-policy', $controller('getPasswordPolicy')); $router->post('two-fa', $controller('saveTwoFactorSettings')); $router->get('two-fa', $controller('getTwoFactorSettings')); $router->post('locale', $controller('setUserLocale')); diff --git a/tests/Unit/Http/CompanyControllerTest.php b/tests/Unit/Http/CompanyControllerTest.php index 0ecf7ef1..edc341b2 100644 --- a/tests/Unit/Http/CompanyControllerTest.php +++ b/tests/Unit/Http/CompanyControllerTest.php @@ -1427,3 +1427,45 @@ function company_controller_bind_activity(): CompanyControllerActivityFake ->and($notMember->getStatusCode())->toBe(400) ->and($notMember->getData(true))->toBe(['errors' => ['User selected to leave organization is not a member of this organization.']]); }); + +test('company controller reads and saves organization authentication settings', function () { + $capsule = company_controller_fixtures(); + $activity = company_controller_bind_activity(); + $capsule->getConnection('mysql')->table('model_has_roles')->insert([ + 'role_id' => 'Administrator', 'model_type' => Fleetbase\Models\CompanyUser::class, 'model_uuid' => 'pivot-owner-1', + ]); + + $defaults = company_controller()->getAuthSettings(); + + expect($defaults->getStatusCode())->toBe(200) + ->and($defaults->getData(true))->toBe(['allow_users_change_password' => true]); + + $saved = company_controller()->saveAuthSettings(company_controller_request('POST', [ + 'allow_users_change_password' => false, + ], company_controller_user('owner-1'))); + + expect($saved->getStatusCode())->toBe(200) + ->and($saved->getData(true))->toBe(['allow_users_change_password' => false]) + ->and(json_decode($capsule->getConnection('mysql')->table('settings')->where('key', 'company.company-1.auth')->value('value'), true))->toBe(['allow_users_change_password' => false]) + ->and(company_controller()->getAuthSettings()->getData(true))->toBe(['allow_users_change_password' => false]) + ->and(array_column($activity->entries, 'event'))->toBe(['auth_settings_updated']); +}); + +test('company controller only lets administrators change organization authentication settings', function () { + $capsule = company_controller_fixtures(); + company_controller_bind_activity(); + + $member = company_controller()->saveAuthSettings(company_controller_request('POST', [ + 'allow_users_change_password' => false, + ], company_controller_user('member-1'))); + $admin = company_controller()->saveAuthSettings(company_controller_request('POST', [ + 'allow_users_change_password' => false, + ], company_controller_user('admin-1'))); + + $missing = company_controller()->saveAuthSettings(company_controller_request('POST', [], company_controller_user('admin-1'))); + + expect($member->getStatusCode())->toBe(403) + ->and($admin->getStatusCode())->toBe(200) + ->and($missing->getStatusCode())->toBe(422) + ->and(json_decode($capsule->getConnection('mysql')->table('settings')->where('key', 'company.company-1.auth')->value('value'), true))->toBe(['allow_users_change_password' => false]); +}); diff --git a/tests/Unit/Http/UserControllerTest.php b/tests/Unit/Http/UserControllerTest.php index bdc128c2..175c627f 100644 --- a/tests/Unit/Http/UserControllerTest.php +++ b/tests/Unit/Http/UserControllerTest.php @@ -5,6 +5,7 @@ use Fleetbase\Http\Controllers\Internal\v1\UserController; use Fleetbase\Http\Requests\ExportRequest; use Fleetbase\Http\Requests\Internal\AcceptCompanyInvite; +use Fleetbase\Http\Requests\Internal\ChangeCurrentPasswordRequest; use Fleetbase\Http\Requests\Internal\ChangeCurrentUserEmailRequest; use Fleetbase\Http\Requests\Internal\ChangeUserEmailRequest; use Fleetbase\Http\Requests\Internal\InviteUserRequest; @@ -13,6 +14,7 @@ use Fleetbase\Http\Requests\Internal\ValidatePasswordRequest; use Fleetbase\Models\Role; use Fleetbase\Models\User; +use Fleetbase\Support\Auth; use Illuminate\Container\Container; use Illuminate\Database\Capsule\Manager as Capsule; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; @@ -24,6 +26,8 @@ use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Facade; use PHPUnit\Framework\Assert; +use Spatie\Activitylog\ActivityLogger; +use Spatie\Activitylog\PendingActivityLog; if (!function_exists('base_path')) { function base_path(string $path = ''): string @@ -324,6 +328,73 @@ public function getSingularName(): string } } +class UserControllerActivityLoggerFake extends ActivityLogger +{ + public static array $logged = []; + + private array $entry = []; + + public function __construct(private ?string $logName = null) + { + } + + public function causedBy(EloquentModel|int|string|null $modelOrId): static + { + $this->entry['causer'] = $modelOrId instanceof EloquentModel ? $modelOrId->getKey() : $modelOrId; + + return $this; + } + + public function performedOn(EloquentModel $model): static + { + $this->entry['subject'] = $model->getKey(); + + return $this; + } + + public function event(string $event): static + { + $this->entry['event'] = $event; + + return $this; + } + + public function withProperties(mixed $properties): static + { + $this->entry['properties'] = $properties; + + return $this; + } + + public function log(string $description): ?Spatie\Activitylog\Contracts\Activity + { + static::$logged[] = array_merge(['log' => $this->logName, 'description' => $description], $this->entry); + + return null; + } +} + +class UserControllerPendingActivityLogFake extends PendingActivityLog +{ + private ?string $logName = null; + + public function __construct() + { + } + + public function useLog(?string $logName): static + { + $this->logName = $logName; + + return $this; + } + + public function logger(): ActivityLogger + { + return new UserControllerActivityLoggerFake($this->logName); + } +} + function user_controller_database(): Capsule { EloquentModel::clearBootedModels(); @@ -602,6 +673,9 @@ public function clear(): void ['id' => 'Administrator', 'company_uuid' => null, 'name' => 'Administrator', 'guard_name' => 'sanctum', 'created_at' => $now, 'updated_at' => $now], ]); + UserControllerActivityLoggerFake::$logged = []; + app()->instance(PendingActivityLog::class, new UserControllerPendingActivityLogFake()); + return $capsule; } @@ -1355,22 +1429,17 @@ function user_controller_assert_created_user_response(mixed $response): object|a $missingPassword = user_controller()->setCurrentUserPassword(user_controller_request('POST', [ 'password' => 'new-password', ], null, 'setCurrentUserPassword', UpdatePasswordRequest::class)); - $passwordMismatch = user_controller()->changeUserPassword(user_controller_request('POST', [ - 'password' => 'new-password', - 'password_confirmation' => 'different-password', - ], $user, 'changeUserPassword', UpdatePasswordRequest::class)); expect($missingCurrent->getStatusCode())->toBe(401) ->and($missingCurrent->getData(true))->toBe(['errors' => ['No user session found']]) ->and($missingPassword->getStatusCode())->toBe(400) - ->and($missingPassword->getData(true))->toBe(['errors' => ['User not authenticated']]) - ->and($passwordMismatch->getStatusCode())->toBe(400) - ->and($passwordMismatch->getData(true))->toBe(['errors' => ['Password is not matching']]); + ->and($missingPassword->getData(true))->toBe(['errors' => ['User not authenticated']]); $changedPassword = user_controller()->changeUserPassword(user_controller_request('POST', [ + 'current_password' => 'old-password', 'password' => 'new-password', 'password_confirmation' => 'new-password', - ], $user, 'changeUserPassword', UpdatePasswordRequest::class)); + ], $user, 'changeUserPassword', ChangeCurrentPasswordRequest::class)); $setLocale = user_controller()->setUserLocale(user_controller_request('POST', [ 'locale' => 'fr-fr', ], $user)); @@ -1573,6 +1642,8 @@ function user_controller_assert_created_user_response(mixed $response): object|a 'model_uuid' => 'pivot-owner-1', ]); + Auth::markPasswordSetupPending($user); + $setPassword = user_controller()->setCurrentUserPassword(user_controller_request('POST', [ 'password' => 'current-new-password', ], $user, 'setCurrentUserPassword', UpdatePasswordRequest::class)); @@ -1798,6 +1869,7 @@ function user_controller_assert_created_user_response(mixed $response): object|a expect($accepted->getStatusCode())->toBe(200) ->and($accepted->getData(true)['status'])->toBe('ok') ->and($accepted->getData(true)['needs_password'])->toBeTrue() + ->and(Auth::isPasswordSetupPending($pending))->toBeTrue() ->and($accepted->getData(true)['token'])->toContain('|') ->and($capsule->getConnection('mysql')->table('company_users')->where('company_uuid', 'company-1')->where('user_uuid', 'pending-1')->exists())->toBeTrue() ->and($capsule->getConnection('mysql')->table('users')->where('uuid', 'pending-1')->value('company_uuid'))->toBe('company-1') @@ -2214,3 +2286,169 @@ function user_controller_managed_account(Capsule $capsule, array $attributes = [ 'a driver account' => [['type' => 'driver'], null], 'an account promoted by an operator' => [['type' => 'user'], ['promoted_from' => 'contact']], ]); + +function user_controller_disallow_changing_own_password(Capsule $capsule, string $companyUuid = 'company-1'): void +{ + $capsule->getConnection('mysql')->table('settings')->insert([ + 'key' => 'company.' . $companyUuid . '.auth', + 'value' => json_encode(['allow_users_change_password' => false]), + ]); +} + +function user_controller_grant_permission(Capsule $capsule, string $companyUserUuid, string $permission): void +{ + $id = 'permission-' . Illuminate\Support\Str::slug($permission); + if ($capsule->getConnection('mysql')->table('permissions')->where('id', $id)->doesntExist()) { + $capsule->getConnection('mysql')->table('permissions')->insert([ + 'id' => $id, + 'name' => $permission, + 'guard_name' => 'sanctum', + 'created_at' => now(), + 'updated_at' => now(), + ]); + } + + $capsule->getConnection('mysql')->table('model_has_permissions')->insert([ + 'permission_id' => $id, + 'model_type' => Fleetbase\Models\CompanyUser::class, + 'model_uuid' => $companyUserUuid, + ]); +} + +test('user controller self-service password endpoints skip the generic resource permission check', function (string $method) { + user_controller_database(); + + $attributes = (new ReflectionMethod(UserController::class, $method))->getAttributes(Fleetbase\Attributes\SkipAuthorizationCheck::class); + + expect($attributes)->toHaveCount(1); +})->with(['setCurrentUserPassword', 'validatePassword', 'changeUserPassword', 'getPasswordPolicy']); + +test('user controller lets an invited non-admin set their first password only once', function () { + $capsule = user_controller_database(); + session(['company' => 'company-1']); + $member = user_controller_user('member-1'); + + $request = fn (string $password) => user_controller_request('POST', [ + 'password' => $password, + 'password_confirmation' => $password, + ], $member, 'setCurrentUserPassword', UpdatePasswordRequest::class); + + $withoutAllowance = user_controller()->setCurrentUserPassword($request('First-password-1!')); + + Auth::markPasswordSetupPending($member); + $firstPassword = user_controller()->setCurrentUserPassword($request('First-password-1!')); + $secondAttempt = user_controller()->setCurrentUserPassword($request('Second-password-1!')); + + expect($withoutAllowance->getStatusCode())->toBe(403) + ->and($firstPassword->getStatusCode())->toBe(200) + ->and($firstPassword->getData(true))->toBe(['status' => 'ok']) + ->and($secondAttempt->getStatusCode())->toBe(403) + ->and(Auth::isPasswordSetupPending($member))->toBeFalse() + ->and(UserControllerActivityLoggerFake::$logged)->toBe([ + ['log' => 'auth', 'description' => 'Password set', 'causer' => 'member-1', 'subject' => 'member-1', 'event' => 'password_set'], + ]) + ->and(password_verify('First-password-1!', $capsule->getConnection('mysql')->table('users')->where('uuid', 'member-1')->value('password')))->toBeTrue(); +}); + +test('user controller password setup allowance expires', function () { + user_controller_database(); + $member = user_controller_user('member-1'); + + Carbon::setTestNow('2026-09-26 10:00:00'); + Auth::markPasswordSetupPending($member, 24); + + Carbon::setTestNow('2026-09-27 09:59:00'); + expect(Auth::isPasswordSetupPending($member))->toBeTrue(); + + Carbon::setTestNow('2026-09-27 10:01:00'); + $expired = user_controller()->setCurrentUserPassword(user_controller_request('POST', [ + 'password' => 'Late-password-1!', + ], $member, 'setCurrentUserPassword', UpdatePasswordRequest::class)); + + expect($expired->getStatusCode())->toBe(403); +}); + +test('user controller lets non-admins change their own password when the organization allows it', function () { + $capsule = user_controller_database(); + session(['company' => 'company-1']); + $member = user_controller_user('member-1'); + + $policy = user_controller()->getPasswordPolicy(user_controller_request('GET', [], $member, 'getPasswordPolicy')); + $changed = user_controller()->changeUserPassword(user_controller_request('POST', [ + 'current_password' => 'old-password', + 'password' => 'Changed-password-1!', + 'password_confirmation' => 'Changed-password-1!', + ], $member, 'changeUserPassword', ChangeCurrentPasswordRequest::class)); + + expect($policy->getData(true))->toBe(['can_change_password' => true]) + ->and($changed->getStatusCode())->toBe(200) + ->and(array_column(UserControllerActivityLoggerFake::$logged, 'event'))->toBe(['password_changed']) + ->and(password_verify('Changed-password-1!', $capsule->getConnection('mysql')->table('users')->where('uuid', 'member-1')->value('password')))->toBeTrue(); +}); + +test('user controller requires the change-password permission when the organization disallows changing own password', function () { + $capsule = user_controller_database(); + session(['company' => 'company-1']); + user_controller_disallow_changing_own_password($capsule); + $member = user_controller_user('member-1'); + + $request = fn () => user_controller_request('POST', [ + 'current_password' => 'old-password', + 'password' => 'Changed-password-1!', + 'password_confirmation' => 'Changed-password-1!', + ], $member, 'changeUserPassword', ChangeCurrentPasswordRequest::class); + + $denied = user_controller()->changeUserPassword($request()); + $policy = user_controller()->getPasswordPolicy(user_controller_request('GET', [], $member, 'getPasswordPolicy')); + + expect($denied->getStatusCode())->toBe(403) + ->and($policy->getData(true))->toBe(['can_change_password' => false]) + ->and(password_verify('old-password', $capsule->getConnection('mysql')->table('users')->where('uuid', 'member-1')->value('password')))->toBeTrue(); + + user_controller_grant_permission($capsule, 'pivot-member-1', 'iam change-password'); + $member = user_controller_user('member-1'); + $allowed = user_controller()->changeUserPassword($request()); + + expect($allowed->getStatusCode())->toBe(200); +}); + +test('user controller always lets admins and administrators change their own password', function () { + $capsule = user_controller_database(); + session(['company' => 'company-1']); + user_controller_disallow_changing_own_password($capsule); + user_controller_owner_is_administrator($capsule); + + expect(Auth::canChangeOwnPassword(user_controller_user('owner-1')))->toBeTrue() + ->and(Auth::canChangeOwnPassword(user_controller_user('admin-1')))->toBeTrue() + ->and(Auth::canChangeOwnPassword(user_controller_user('member-1')))->toBeFalse(); +}); + +test('user controller accepts the iam wildcard permission for changing own password', function () { + $capsule = user_controller_database(); + session(['company' => 'company-1']); + user_controller_disallow_changing_own_password($capsule); + user_controller_grant_permission($capsule, 'pivot-member-1', 'iam *'); + + expect(Auth::canChangeOwnPassword(user_controller_user('member-1')))->toBeTrue(); +}); + +test('change current password request checks the current password in the same request', function () { + user_controller_database(); + $member = user_controller_user('member-1'); + $request = ChangeCurrentPasswordRequest::create('/int/v1/users/change-password', 'POST'); + $request->setUserResolver(fn () => $member); + + $rules = ['current_password' => $request->rules()['current_password']]; + + $translator = new Illuminate\Translation\Translator(new Illuminate\Translation\ArrayLoader(), 'en'); + $validate = fn (array $input) => new Illuminate\Validation\Validator($translator, $input, $rules); + + $wrong = $validate(['current_password' => 'not-my-password']); + $right = $validate(['current_password' => 'old-password']); + $none = $validate([]); + + expect($wrong->fails())->toBeTrue() + ->and($wrong->errors()->first('current_password'))->toBe('The current password provided is invalid.') + ->and($right->fails())->toBeFalse() + ->and($none->fails())->toBeTrue(); +}); From 5ded44765c3969f6c6f56a740af617e5b2c1d4be Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sat, 26 Sep 2026 16:00:33 +0800 Subject: [PATCH 05/10] feat: record app and APNs environment on user devices Add optional app_identifier, environment and last_seen_at columns to user_devices so push senders can pick the credentials of the app a token was registered from and the APNs environment that issued it, instead of guessing. All columns are nullable and guarded, and callers that do not set them keep working. --- ...dd_push_metadata_to_user_devices_table.php | 46 +++++++++++++++++++ src/Models/UserDevice.php | 11 ++++- 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 migrations/2026_09_26_000000_add_push_metadata_to_user_devices_table.php diff --git a/migrations/2026_09_26_000000_add_push_metadata_to_user_devices_table.php b/migrations/2026_09_26_000000_add_push_metadata_to_user_devices_table.php new file mode 100644 index 00000000..e1e00600 --- /dev/null +++ b/migrations/2026_09_26_000000_add_push_metadata_to_user_devices_table.php @@ -0,0 +1,46 @@ +string('app_identifier', 191)->nullable()->index()->after('platform'); + } + + if (!Schema::hasColumn('user_devices', 'environment')) { + $table->string('environment', 32)->nullable()->after('app_identifier'); + } + + if (!Schema::hasColumn('user_devices', 'last_seen_at')) { + $table->timestamp('last_seen_at')->nullable()->after('status'); + } + }); + } + + public function down(): void + { + Schema::table('user_devices', function (Blueprint $table) { + foreach (['app_identifier', 'environment', 'last_seen_at'] as $column) { + if (Schema::hasColumn('user_devices', $column)) { + $table->dropColumn($column); + } + } + }); + } +}; diff --git a/src/Models/UserDevice.php b/src/Models/UserDevice.php index 88d3b9d4..05b8d8c2 100644 --- a/src/Models/UserDevice.php +++ b/src/Models/UserDevice.php @@ -31,7 +31,16 @@ class UserDevice extends Model * * @var array */ - protected $fillable = ['user_uuid', 'platform', 'token', 'status']; + protected $fillable = ['user_uuid', 'platform', 'app_identifier', 'environment', 'token', 'status', 'last_seen_at']; + + /** + * The attributes that should be cast to native types. + * + * @var array + */ + protected $casts = [ + 'last_seen_at' => 'datetime', + ]; /** * Dynamic attributes that are appended to object. From 969242f76816fadd7ef743fca6e874d565a479eb Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sun, 27 Sep 2026 13:26:47 +0800 Subject: [PATCH 06/10] feat(2fa): sign in with an authenticator app, with recovery codes Adds authenticator apps (TOTP, RFC 6238) as a 2FA method, next to email and SMS. Works with Google Authenticator, Authy, 1Password and similar apps. - Setup: POST users/two-fa/authenticator/setup (current password) returns a secret, an otpauth:// URL and a QR code. confirm checks a code from the app, makes it the user's 2FA method and returns 8 one-time recovery codes. disable and recovery-codes also need the current password. - Sign-in: when the method is authenticator_app, two-fa/validate sends nothing and returns method "authenticator_app". two-fa/verify accepts a code from the app (one time step either side, each code usable once) or a recovery code. Wrong codes count towards the existing 5-attempt lockout. two-fa/resend sends a code by email (SMS without an email) as a fallback. - Storage: the secret is encrypted with the app key; recovery codes are stored as keyed SHA-256 hashes. Neither is ever returned again or logged. - Enable, disable, recovery code use, successful sign-ins and lockouts are written to the `auth` activity log. - saveTwoFactorSettings refuses authenticator_app until an app is set up. - New Fleetbase\Support\Barcode helper, which owns milon/barcode. QR codes are drawn as a compact SVG on a white background with a quiet zone, so they scan on dark themes too. - New dependencies: pragmarx/google2fa ^8.0 and milon/barcode ^10.0 (already installed everywhere through Fleet-Ops). - Tests: the shared test container now provides a recording activity logger and a test encrypter. Closes #163 --- composer.json | 2 + .../Internal/v1/TwoFaController.php | 2 + .../Internal/v1/UserController.php | 102 ++++ src/Support/Barcode.php | 107 +++++ src/Support/TwoFactorAuth.php | 443 +++++++++++++++++- src/routes.php | 5 + tests/Pest.php | 126 +++++ tests/Unit/Http/TwoFaControllerTest.php | 26 + tests/Unit/Http/UserControllerTest.php | 66 +++ tests/Unit/Support/BarcodeTest.php | 59 +++ tests/Unit/Support/TwoFactorAuthTest.php | 196 ++++++++ 11 files changed, 1123 insertions(+), 11 deletions(-) create mode 100644 src/Support/Barcode.php create mode 100644 tests/Unit/Support/BarcodeTest.php diff --git a/composer.json b/composer.json index 5ef971d4..e43f6294 100644 --- a/composer.json +++ b/composer.json @@ -46,9 +46,11 @@ "lcobucci/clock": "3.3.1", "lcobucci/jwt": "^5.4", "maatwebsite/excel": "^3.1", + "milon/barcode": "^10.0", "mossadal/math-parser": "^1.3", "phpoffice/phpspreadsheet": "^1.28", "phrity/websocket": "^1.7", + "pragmarx/google2fa": "^8.0", "rlanvin/php-rrule": "^2.4", "sentry/sentry-laravel": "*", "spatie/laravel-activitylog": "^4.7", diff --git a/src/Http/Controllers/Internal/v1/TwoFaController.php b/src/Http/Controllers/Internal/v1/TwoFaController.php index 15d7a2c9..7eb7da3b 100644 --- a/src/Http/Controllers/Internal/v1/TwoFaController.php +++ b/src/Http/Controllers/Internal/v1/TwoFaController.php @@ -77,6 +77,7 @@ public function validateSession(TwoFaValidationRequest $request) return response()->json([ 'clientToken' => $validClientToken, + 'method' => TwoFactorAuth::getChallengeMethod($identity, $validClientToken), 'expired' => false, ]); } catch (\Exception $e) { @@ -138,6 +139,7 @@ public function resendCode(Request $request) return response()->json([ 'clientToken' => $clientToken, + 'method' => TwoFactorAuth::getChallengeMethod($identity, $clientToken), ]); } catch (\Exception $e) { return response()->error($e->getMessage()); diff --git a/src/Http/Controllers/Internal/v1/UserController.php b/src/Http/Controllers/Internal/v1/UserController.php index 2dc94412..7e6a5bf6 100644 --- a/src/Http/Controllers/Internal/v1/UserController.php +++ b/src/Http/Controllers/Internal/v1/UserController.php @@ -711,11 +711,113 @@ public function saveTwoFactorSettings(Request $request) return response()->error('No user session found', 401); } + if (($twoFaSettings['method'] ?? null) === TwoFactorAuth::METHOD_AUTHENTICATOR_APP && !TwoFactorAuth::hasAuthenticatorApp($user)) { + return response()->error('Set up your authenticator app before choosing it as your two-factor method.', 422); + } + $twoFaSettings = TwoFactorAuth::saveTwoFaSettingsForUser($user, $twoFaSettings); return response()->json($twoFaSettings->value); } + /** + * Get the current user's authenticator app status. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function getAuthenticatorApp(Request $request) + { + return response()->json(TwoFactorAuth::getAuthenticatorStatus($request->user())); + } + + /** + * Start setting up an authenticator app for the current user. Requires the current + * password, since it changes how the user signs in. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function setupAuthenticatorApp(Request $request) + { + $user = $request->user(); + if (!$user->checkPassword((string) $request->input('password'))) { + return response()->error('The current password provided is invalid.', 422); + } + + return response()->json(TwoFactorAuth::beginAuthenticatorEnrollment($user)); + } + + /** + * Confirm the current user's new authenticator app with a code from it. Returns the + * recovery codes, which are only shown this once. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function confirmAuthenticatorApp(Request $request) + { + $user = $request->user(); + + try { + $recoveryCodes = TwoFactorAuth::confirmAuthenticatorEnrollment($user, (string) $request->input('code')); + } catch (\Exception $e) { + return response()->error($e->getMessage(), 422); + } + + return response()->json([ + 'recovery_codes' => $recoveryCodes, + 'status' => TwoFactorAuth::getAuthenticatorStatus($user), + 'settings' => TwoFactorAuth::getTwoFaSettingsForUser($user)->value, + ]); + } + + /** + * Remove the current user's authenticator app. Requires the current password. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function disableAuthenticatorApp(Request $request) + { + $user = $request->user(); + if (!$user->checkPassword((string) $request->input('password'))) { + return response()->error('The current password provided is invalid.', 422); + } + + TwoFactorAuth::disableAuthenticatorApp($user); + + return response()->json([ + 'status' => TwoFactorAuth::getAuthenticatorStatus($user), + 'settings' => TwoFactorAuth::getTwoFaSettingsForUser($user)->value, + ]); + } + + /** + * Replace the current user's recovery codes. Requires the current password. + * + * @return \Illuminate\Http\Response + */ + #[SkipAuthorizationCheck] + public function regenerateRecoveryCodes(Request $request) + { + $user = $request->user(); + if (!$user->checkPassword((string) $request->input('password'))) { + return response()->error('The current password provided is invalid.', 422); + } + + try { + $recoveryCodes = TwoFactorAuth::regenerateRecoveryCodes($user); + } catch (\Exception $e) { + return response()->error($e->getMessage(), 422); + } + + return response()->json([ + 'recovery_codes' => $recoveryCodes, + 'status' => TwoFactorAuth::getAuthenticatorStatus($user), + ]); + } + /** * Invite a user (new or existing) to join the current organisation. * diff --git a/src/Support/Barcode.php b/src/Support/Barcode.php new file mode 100644 index 00000000..5841bc60 --- /dev/null +++ b/src/Support/Barcode.php @@ -0,0 +1,107 @@ +' + . '' + . '' + . ''; + } + + /** + * Render a QR code as an SVG data URI, ready for an ``. + * + * @param string $data the content to encode + * @param string $errorCorrection L, M, Q or H + */ + public static function qrCodeDataUri(string $data, string $errorCorrection = 'M'): string + { + return 'data:image/svg+xml;base64,' . base64_encode(static::qrCodeSvg($data, $errorCorrection)); + } + + /** + * Render a 2D barcode as a base64-encoded PNG, with a transparent background. + * + * @param string $data the content to encode + * @param string $type a `milon/barcode` 2D type, e.g. `QRCODE`, `QRCODE,H`, `PDF417` or `DATAMATRIX` + * @param int $w the width of one module, in pixels + * @param int $h the height of one module, in pixels + * + * @return string|false the PNG, or false when no image library is available + */ + public static function png(string $data, string $type = 'QRCODE', int $w = 3, int $h = 3): string|false + { + return (new DNS2D())->setStorPath(sys_get_temp_dir())->getBarcodePNG($data, $type, $w, $h); + } + + /** + * Get the module matrix for a QR code. + * + * @return array with `num_rows`, `num_cols` and the `bcode` module rows + */ + protected static function qrCodeMatrix(string $data, string $errorCorrection): array + { + $errorCorrection = strtoupper($errorCorrection); + if (!in_array($errorCorrection, ['L', 'M', 'Q', 'H'], true)) { + throw new \InvalidArgumentException('QR code error correction must be L, M, Q or H.'); + } + + $matrix = $data === '' ? false : (new QRcode($data, $errorCorrection))->getBarcodeArray(); + if (!is_array($matrix) || empty($matrix['num_rows'])) { + throw new \InvalidArgumentException('The data could not be encoded as a QR code.'); + } + + return $matrix; + } +} diff --git a/src/Support/TwoFactorAuth.php b/src/Support/TwoFactorAuth.php index 649b316d..02e8efe0 100644 --- a/src/Support/TwoFactorAuth.php +++ b/src/Support/TwoFactorAuth.php @@ -9,8 +9,10 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Arr; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Redis; use Illuminate\Support\Str; +use PragmaRX\Google2FA\Google2FA; /** * Class TwoFactorAuth. @@ -27,6 +29,31 @@ class TwoFactorAuth */ public const MAX_VERIFY_ATTEMPTS = 5; + /** + * The 2FA method that uses codes from an authenticator app (TOTP, RFC 6238). + */ + public const METHOD_AUTHENTICATOR_APP = 'authenticator_app'; + + /** + * Seconds a user has to confirm a new authenticator app with its first code. + */ + public const AUTHENTICATOR_ENROLLMENT_TTL = 900; + + /** + * Number of one-time recovery codes issued with an authenticator app. + */ + public const RECOVERY_CODE_COUNT = 8; + + /** + * Authenticator codes accepted either side of the current 30 second step, to allow for clock drift. + */ + public const AUTHENTICATOR_WINDOW = 1; + + /** + * Marks a client token for an authenticator app challenge, instead of a sent code. + */ + private const AUTHENTICATOR_CLIENT_TOKEN = 'authenticator'; + /** * Save Two-Factor Authentication settings for System wide usage. * @@ -173,6 +200,17 @@ public static function getClientSessionTokenFromTwoFaSession(string $token, stri throw new \Exception('2FA Authentication is not enabled.'); } + // An authenticator app challenge stays valid for as long as the 2FA session does + if ($clientToken && static::isAuthenticatorClientToken($clientToken)) { + if (static::getUserFromAuthenticatorClientToken($clientToken)?->uuid === $user->uuid + && static::isTwoFaSessionKeyValid(static::decryptSessionKey($token, $user->uuid), $user)) { + return $clientToken; + } + + static::forgetTwoFaSession($token, $identity); + throw new \Exception('2FA Verification session has expired.'); + } + // If a client session token is provided validate by fetching the verification code // If a verification code exists then we just return the current valid client session if ($clientToken) { @@ -200,6 +238,11 @@ public static function getClientSessionTokenFromTwoFaSession(string $token, stri // Validate session key that it is valid and exists if (static::isTwoFaSessionKeyValid($twoFaSessionKey, $user)) { + // Authenticator app users type the code from their app, so nothing is sent + if (static::usesAuthenticatorApp($user)) { + return static::createAuthenticatorClientToken($user); + } + // Send the verification code then create a client session for the verification code and user $verificationCode = static::sendVerificationCode($user); $clientToken = static::createClientSessionToken($verificationCode); @@ -210,6 +253,26 @@ public static function getClientSessionTokenFromTwoFaSession(string $token, stri throw new \Exception('2FA Authentication session is invalid'); } + /** + * Get how the user answers a 2FA challenge: `authenticator_app` when the client token + * is for an authenticator app, otherwise the method the code was sent with. + * + * @param string $identity the user identity + * @param string $clientToken the client session token + */ + public static function getChallengeMethod(string $identity, string $clientToken): string + { + if (static::isAuthenticatorClientToken($clientToken)) { + return static::METHOD_AUTHENTICATOR_APP; + } + + $user = static::getUserFromIdentity($identity); + $method = $user ? (string) static::getTwoFaSettingsForUser($user)->getValue('method', 'email') : 'email'; + + // Authenticator app users only get a sent code as a fallback, see sendVerificationCode() + return $method === static::METHOD_AUTHENTICATOR_APP && $user ? static::fallbackMethod($user) : $method; + } + /** * Validate a Two-Factor Authentication session token. * @@ -232,6 +295,11 @@ public static function validateSessionToken(string $token, string $identity, ?st return false; } + if ($clientToken && static::isAuthenticatorClientToken($clientToken)) { + return static::getUserFromAuthenticatorClientToken($clientToken)?->uuid === $user->uuid + && static::isTwoFaSessionKeyValid(static::decryptSessionKey($token, $user->uuid), $user); + } + // If a client session token is provided validate by fetching the verification code // If a verification code exists then we just return the current valid client session if ($clientToken) { @@ -283,6 +351,12 @@ public static function sendVerificationCode(User $user, int $expiresAfter = 61): $method = $twoFaSettings->getValue('method', 'email'); $expiresAfter = Carbon::now()->addSeconds($expiresAfter); + // A code can still be sent to authenticator app users, as a fallback when they + // cannot use their app. + if ($method === static::METHOD_AUTHENTICATOR_APP) { + $method = static::fallbackMethod($user); + } + // Create SMS and Email message callback $messageCallback = function ($verificationCode) { return $verificationCode->code . ' is your ' . config('app.name') . ' 2FA Code'; @@ -447,6 +521,10 @@ public static function start(string|User $identity, int $tokenLength = 40): ?str */ public static function verifyCode(string $code, string $token, string $clientToken): string { + if (static::isAuthenticatorClientToken($clientToken)) { + return static::verifyAuthenticatorChallenge($code, $token, $clientToken); + } + // Get verification code from the client token $verificationCode = static::getVerificationCodeFromClientToken($clientToken); @@ -488,6 +566,7 @@ public static function verifyCode(string $code, string $token, string $clientTok if ($verificationCodeMatches) { // Kill the two fa session Redis::del($twoFaSessionKey, static::attemptsKey($twoFaSessionKey)); + static::logActivity($user, 'two_factor_verified', 'Two-factor sign-in verified', ['method' => 'code']); // Authenticate the user $token = $user->createToken($user->uuid); @@ -495,17 +574,7 @@ public static function verifyCode(string $code, string $token, string $clientTok return $token->plainTextToken; } - // Count the failure against the 2FA session, which outlives any single - // code, so resending codes does not reset the budget. - $attemptsKey = static::attemptsKey($twoFaSessionKey); - $attempts = (int) Redis::incr($attemptsKey); - Redis::expire($attemptsKey, static::SESSION_TTL); - - if ($attempts >= static::MAX_VERIFY_ATTEMPTS) { - Redis::del($twoFaSessionKey, $attemptsKey); - - throw new \Exception('Too many failed verification attempts. Please sign in again.'); - } + static::recordFailedAttempt($twoFaSessionKey, $user); throw new \Exception('Verification code does not match.'); } @@ -610,6 +679,358 @@ public static function forgetTwoFaSession(string $token, string $identity): bool return Redis::del($twoFaSessionKey); } + /** + * Whether the user signs in with an authenticator app they have confirmed. + */ + public static function usesAuthenticatorApp(User $user): bool + { + return static::hasAuthenticatorApp($user) + && static::getTwoFaSettingsForUser($user)->getValue('method') === static::METHOD_AUTHENTICATOR_APP; + } + + /** + * Whether the user has confirmed an authenticator app. + */ + public static function hasAuthenticatorApp(User $user): bool + { + return !empty(static::getAuthenticatorRecord($user)['confirmed_at']); + } + + /** + * Describe the user's authenticator app, without any secrets. + * + * @return array{enabled: bool, confirmed_at: string|null, recovery_codes_remaining: int} + */ + public static function getAuthenticatorStatus(User $user): array + { + $record = static::getAuthenticatorRecord($user); + + return [ + 'enabled' => !empty($record['confirmed_at']), + 'confirmed_at' => $record['confirmed_at'] ?? null, + 'recovery_codes_remaining' => count($record['recovery_codes'] ?? []), + ]; + } + + /** + * Start setting up an authenticator app. The new secret is kept aside until the user + * confirms it with a code, so an app that is already set up keeps working until then. + * + * @return array{secret: string, otpauth_url: string, qr_code: string, expires_at: string} + */ + public static function beginAuthenticatorEnrollment(User $user): array + { + $google2fa = static::google2fa(); + $secret = $google2fa->generateSecretKey(32); + $expiresAt = Carbon::now()->addSeconds(static::AUTHENTICATOR_ENROLLMENT_TTL); + $issuer = (string) config('app.name', 'Fleetbase'); + $account = $user->email ?: ($user->phone ?: ($user->username ?: $user->uuid)); + $url = $google2fa->getQRCodeUrl($issuer, $account, $secret); + + static::saveAuthenticatorRecord($user, array_merge(static::getAuthenticatorRecord($user), [ + 'pending_secret' => Crypt::encryptString($secret), + 'pending_expires_at' => $expiresAt->toIso8601String(), + ])); + + return [ + 'secret' => $secret, + 'otpauth_url' => $url, + 'qr_code' => Barcode::qrCodeDataUri($url), + 'expires_at' => $expiresAt->toIso8601String(), + ]; + } + + /** + * Confirm a new authenticator app with a code from it, make it the user's 2FA method, + * and issue new recovery codes. + * + * @return array the recovery codes, which are only ever shown here + * + * @throws \Exception if there is no pending setup, it expired, or the code is wrong + */ + public static function confirmAuthenticatorEnrollment(User $user, string $code): array + { + $record = static::getAuthenticatorRecord($user); + $expiresAt = $record['pending_expires_at'] ?? null; + + if (empty($record['pending_secret']) || !$expiresAt || Carbon::now()->gte(Carbon::parse($expiresAt))) { + throw new \Exception('Authenticator setup has expired. Start again.'); + } + + // verifyKeyNewer() returns the matched time step only when given a previous one + $secret = Crypt::decryptString($record['pending_secret']); + $timestep = static::google2fa()->verifyKeyNewer($secret, static::normalizeOtp($code), 0, static::AUTHENTICATOR_WINDOW); + if ($timestep === false) { + throw new \Exception('The code from your authenticator app is not correct.'); + } + + [$recoveryCodes, $hashes] = static::generateRecoveryCodes(); + + static::saveAuthenticatorRecord($user, [ + 'secret' => Crypt::encryptString($secret), + 'confirmed_at' => Carbon::now()->toIso8601String(), + 'last_timestep' => $timestep, + 'recovery_codes' => $hashes, + ]); + static::saveTwoFaSettingsForUser($user, array_merge(static::getTwoFaSettingsForUser($user)->value ?? [], [ + 'enabled' => true, + 'method' => static::METHOD_AUTHENTICATOR_APP, + ])); + static::logActivity($user, 'authenticator_enabled', 'Authenticator app enabled'); + + return $recoveryCodes; + } + + /** + * Remove the user's authenticator app. If it was their 2FA method, 2FA is turned off + * so they are not locked out; they can turn it on again with another method. + */ + public static function disableAuthenticatorApp(User $user): void + { + Setting::where('key', static::authenticatorKey($user))->delete(); + + $settings = static::getTwoFaSettingsForUser($user)->value ?? []; + if (($settings['method'] ?? null) === static::METHOD_AUTHENTICATOR_APP) { + static::saveTwoFaSettingsForUser($user, array_merge($settings, ['enabled' => false, 'method' => 'email'])); + } + + static::logActivity($user, 'authenticator_disabled', 'Authenticator app disabled'); + } + + /** + * Replace the user's recovery codes. + * + * @return array the new recovery codes, which are only ever shown here + * + * @throws \Exception if the user has no authenticator app + */ + public static function regenerateRecoveryCodes(User $user): array + { + $record = static::getAuthenticatorRecord($user); + if (empty($record['confirmed_at'])) { + throw new \Exception('Set up an authenticator app first.'); + } + + [$recoveryCodes, $hashes] = static::generateRecoveryCodes(); + $record['recovery_codes'] = $hashes; + static::saveAuthenticatorRecord($user, $record); + static::logActivity($user, 'recovery_codes_regenerated', 'Two-factor recovery codes regenerated'); + + return $recoveryCodes; + } + + /** + * Check a code from the user's authenticator app, or one of their recovery codes. + * Each authenticator code and each recovery code can only be used once. + * + * @return string|null `authenticator_app` or `recovery_code` for the kind of code that matched, or null + */ + public static function verifyAuthenticatorCode(User $user, string $code): ?string + { + $record = static::getAuthenticatorRecord($user); + if (empty($record['secret'])) { + return null; + } + + $otp = static::normalizeOtp($code); + if (preg_match('/^\d{6}$/', $otp)) { + $timestep = static::google2fa()->verifyKeyNewer( + Crypt::decryptString($record['secret']), + $otp, + (int) ($record['last_timestep'] ?? 0), + static::AUTHENTICATOR_WINDOW + ); + + if ($timestep === false) { + return null; + } + + $record['last_timestep'] = $timestep; + static::saveAuthenticatorRecord($user, $record); + + return static::METHOD_AUTHENTICATOR_APP; + } + + $hash = static::hashRecoveryCode($code); + $codes = $record['recovery_codes'] ?? []; + foreach ($codes as $index => $storedHash) { + if (hash_equals((string) $storedHash, $hash)) { + unset($codes[$index]); + $record['recovery_codes'] = array_values($codes); + static::saveAuthenticatorRecord($user, $record); + static::logActivity($user, 'recovery_code_used', 'Two-factor recovery code used', ['remaining' => count($codes)]); + + return 'recovery_code'; + } + } + + return null; + } + + /** + * Verify an authenticator app challenge and return a user token. + * + * @throws \Exception if the challenge is invalid or the code is wrong + */ + private static function verifyAuthenticatorChallenge(string $code, string $token, string $clientToken): string + { + $user = static::getUserFromAuthenticatorClientToken($clientToken); + if (!$user) { + throw new \Exception('Verification code is invalid.'); + } + + $twoFaSessionKey = static::decryptSessionKey($token, $user->uuid); + if (!static::isTwoFaSessionKeyValid($twoFaSessionKey, $user)) { + throw new \Exception('Verification code is invalid.'); + } + + $matched = static::verifyAuthenticatorCode($user, $code); + if (!$matched) { + static::recordFailedAttempt($twoFaSessionKey, $user); + + throw new \Exception('Verification code does not match.'); + } + + Redis::del($twoFaSessionKey, static::attemptsKey($twoFaSessionKey), static::authenticatorClientKey($clientToken)); + static::logActivity($user, 'two_factor_verified', 'Two-factor sign-in verified', ['method' => $matched]); + + return $user->createToken($user->uuid)->plainTextToken; + } + + /** + * Count a failed code against the 2FA session, which outlives any single code, so + * resending codes does not reset the budget. Ends the session after too many failures. + * + * @throws \Exception when the session has been ended + */ + private static function recordFailedAttempt(string $twoFaSessionKey, User $user): void + { + $attemptsKey = static::attemptsKey($twoFaSessionKey); + $attempts = (int) Redis::incr($attemptsKey); + Redis::expire($attemptsKey, static::SESSION_TTL); + + if ($attempts >= static::MAX_VERIFY_ATTEMPTS) { + Redis::del($twoFaSessionKey, $attemptsKey); + static::logActivity($user, 'two_factor_locked', 'Two-factor sign-in stopped after too many wrong codes', ['attempts' => $attempts]); + + throw new \Exception('Too many failed verification attempts. Please sign in again.'); + } + } + + /** + * The method used to send a code to an authenticator app user who cannot use their app. + */ + private static function fallbackMethod(User $user): string + { + return $user->email ? 'email' : 'sms'; + } + + /** + * Create a client token for an authenticator app challenge. It points at the user + * server side, so the token itself does not reveal who it is for. + */ + private static function createAuthenticatorClientToken(User $user): string + { + $reference = Str::random(40); + $clientToken = base64_encode(Carbon::now()->addSeconds(static::SESSION_TTL) . '|' . static::AUTHENTICATOR_CLIENT_TOKEN . '|' . $reference); + + Redis::set(static::authenticatorClientKey($clientToken), $user->uuid, 'EX', static::SESSION_TTL); + + return $clientToken; + } + + private static function isAuthenticatorClientToken(string $clientToken): bool + { + return (static::decodeClientToken($clientToken)[1] ?? null) === static::AUTHENTICATOR_CLIENT_TOKEN; + } + + private static function getUserFromAuthenticatorClientToken(string $clientToken): ?User + { + $userUuid = Redis::get(static::authenticatorClientKey($clientToken)); + + return $userUuid ? User::where('uuid', $userUuid)->first() : null; + } + + private static function authenticatorClientKey(string $clientToken): string + { + return 'two_fa_client:' . (static::decodeClientToken($clientToken)[2] ?? ''); + } + + /** + * Get the user's stored authenticator app record: the encrypted secret, when it was + * confirmed, the last used time step, and hashed recovery codes. + */ + private static function getAuthenticatorRecord(User $user): array + { + $record = Setting::lookup(static::authenticatorKey($user), []); + + return is_array($record) ? $record : []; + } + + private static function saveAuthenticatorRecord(User $user, array $record): void + { + Setting::configure(static::authenticatorKey($user), $record); + } + + private static function authenticatorKey(User $user): string + { + return 'user.' . $user->uuid . '.2fa_authenticator'; + } + + /** + * Generate recovery codes like `k7d2m-9xq4p`, and the hashes that are stored. + * + * @return array{0: array, 1: array} + */ + private static function generateRecoveryCodes(): array + { + $alphabet = 'abcdefghjkmnpqrstuvwxyz23456789'; + $codes = []; + + for ($i = 0; $i < static::RECOVERY_CODE_COUNT; $i++) { + $code = ''; + for ($j = 0; $j < 10; $j++) { + $code .= $alphabet[random_int(0, strlen($alphabet) - 1)]; + } + $codes[] = substr($code, 0, 5) . '-' . substr($code, 5); + } + + return [$codes, array_map([static::class, 'hashRecoveryCode'], $codes)]; + } + + /** + * Recovery codes are random and single use, so a keyed SHA-256 hash is enough to store them. + */ + private static function hashRecoveryCode(string $code): string + { + $normalized = strtolower((string) preg_replace('/[^a-z0-9]/i', '', $code)); + + return hash_hmac('sha256', $normalized, (string) config('app.key', '')); + } + + private static function normalizeOtp(string $code): string + { + return (string) preg_replace('/\s+/', '', $code); + } + + private static function google2fa(): Google2FA + { + return new Google2FA(); + } + + /** + * Record a 2FA event in the `auth` activity log. Codes and secrets are never logged. + */ + private static function logActivity(User $user, string $event, string $description, array $properties = []): void + { + activity('auth') + ->causedBy($user) + ->performedOn($user) + ->withProperties($properties) + ->event($event) + ->log($description); + } + /** * Create a Two-Factor Authentication session key. * diff --git a/src/routes.php b/src/routes.php index 8aa671d7..230a5e0e 100644 --- a/src/routes.php +++ b/src/routes.php @@ -311,6 +311,11 @@ function ($router, $controller) { $router->post('change-password', $controller('changeUserPassword')); $router->post('two-fa', $controller('saveTwoFactorSettings')); $router->get('two-fa', $controller('getTwoFactorSettings')); + $router->get('two-fa/authenticator', $controller('getAuthenticatorApp')); + $router->post('two-fa/authenticator/setup', $controller('setupAuthenticatorApp'))->middleware(Illuminate\Routing\Middleware\ThrottleRequests::class . ':10,1'); + $router->post('two-fa/authenticator/confirm', $controller('confirmAuthenticatorApp'))->middleware(Illuminate\Routing\Middleware\ThrottleRequests::class . ':10,1'); + $router->post('two-fa/authenticator/disable', $controller('disableAuthenticatorApp'))->middleware(Illuminate\Routing\Middleware\ThrottleRequests::class . ':10,1'); + $router->post('two-fa/recovery-codes', $controller('regenerateRecoveryCodes'))->middleware(Illuminate\Routing\Middleware\ThrottleRequests::class . ':10,1'); $router->post('locale', $controller('setUserLocale')); $router->get('locale', $controller('getUserLocale')); } diff --git a/tests/Pest.php b/tests/Pest.php index 77acb87e..cae14328 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -403,6 +403,124 @@ public function runningUnitTests(): bool } } + /** + * A reversible stand-in for the host app's encrypter (core-api does not depend on + * illuminate/encryption). Encrypted values are prefixed so tests can tell them apart. + */ + class TestEncrypter implements Illuminate\Contracts\Encryption\Encrypter, Illuminate\Contracts\Encryption\StringEncrypter + { + public function encrypt(#[SensitiveParameter] $value, $serialize = true) + { + return 'encrypted:' . base64_encode($serialize ? serialize($value) : $value); + } + + public function decrypt($payload, $unserialize = true) + { + if (!str_starts_with((string) $payload, 'encrypted:')) { + throw new Illuminate\Contracts\Encryption\DecryptException('The payload is invalid.'); + } + + $value = base64_decode(substr($payload, 10)); + + return $unserialize ? unserialize($value) : $value; + } + + public function encryptString(#[SensitiveParameter] $value) + { + return $this->encrypt($value, false); + } + + public function decryptString($payload) + { + return $this->decrypt($payload, false); + } + + public function getKey() + { + return 'test-key'; + } + + public function getAllKeys() + { + return ['test-key']; + } + + public function getPreviousKeys() + { + return []; + } + } + + /** + * Records `activity()` calls made in tests. Read TestActivityLogger::$logged. + */ + class TestActivityLogger extends Spatie\Activitylog\ActivityLogger + { + public static array $logged = []; + + private array $entry = []; + + public function __construct(private ?string $logName = null) + { + } + + public function causedBy(Illuminate\Database\Eloquent\Model|int|string|null $modelOrId): static + { + $this->entry['causer'] = $modelOrId instanceof Illuminate\Database\Eloquent\Model ? $modelOrId->getKey() : $modelOrId; + + return $this; + } + + public function performedOn(Illuminate\Database\Eloquent\Model $model): static + { + $this->entry['subject'] = $model->getKey(); + + return $this; + } + + public function event(string $event): static + { + $this->entry['event'] = $event; + + return $this; + } + + public function withProperties(mixed $properties): static + { + $this->entry['properties'] = $properties; + + return $this; + } + + public function log(string $description): ?Spatie\Activitylog\Contracts\Activity + { + static::$logged[] = array_merge(['log' => $this->logName, 'description' => $description], $this->entry); + + return null; + } + } + + class TestPendingActivityLog extends Spatie\Activitylog\PendingActivityLog + { + private ?string $logName = null; + + public function __construct() + { + } + + public function useLog(?string $logName): static + { + $this->logName = $logName; + + return $this; + } + + public function logger(): Spatie\Activitylog\ActivityLogger + { + return new TestActivityLogger($this->logName); + } + } + function bind_test_container(array $config = []): Container { if (!Container::getInstance() instanceof FleetbaseTestContainer) { @@ -422,6 +540,14 @@ function bind_test_container(array $config = []): Container $container->instance('request', Request::create('/int/v1/test', 'GET')); + // activity() works in every test; files that assert on it bind their own fake + TestActivityLogger::$logged = []; + $container->instance(Spatie\Activitylog\PendingActivityLog::class, new TestPendingActivityLog()); + + if (!$container->bound('encrypter')) { + $container->instance('encrypter', new TestEncrypter()); + } + $container->instance('log', new class { public array $entries = []; diff --git a/tests/Unit/Http/TwoFaControllerTest.php b/tests/Unit/Http/TwoFaControllerTest.php index 7ea726cb..c3e77466 100644 --- a/tests/Unit/Http/TwoFaControllerTest.php +++ b/tests/Unit/Http/TwoFaControllerTest.php @@ -31,6 +31,11 @@ public function exists(string $key): bool return array_key_exists($key, $this->values); } + public function get(string $key): mixed + { + return $this->values[$key] ?? null; + } + public function del(?string ...$keys): bool { foreach ($keys as $key) { @@ -423,6 +428,7 @@ function two_fa_controller_verification_code(User $user, Carbon $expiresAt, stri expect($valid->getData(true))->toBe([ 'clientToken' => $clientToken, + 'method' => 'email', 'expired' => false, ]) ->and($expired->getData(true))->toBe(['expired' => true]) @@ -514,3 +520,23 @@ function two_fa_controller_verification_code(User $user, Carbon $expiresAt, stri ->and($response->getData(true)['code'])->toBe('console_access_not_allowed') ->and(app('db')->table('personal_access_tokens')->where('tokenable_id', $user->uuid)->count())->toBe(0); }); + +test('two fa controller tells the console when to ask for a code from the authenticator app', function () { + two_fa_controller_database(); + $user = two_fa_controller_user(); + $enrollment = @TwoFactorAuth::beginAuthenticatorEnrollment($user); + TwoFactorAuth::confirmAuthenticatorEnrollment($user, (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($enrollment['secret'])); + $token = TwoFactorAuth::start($user); + + $challenge = two_fa_controller()->validateSession(two_fa_controller_validation_request([ + 'token' => $token, + 'identity' => $user->email, + ])); + $fallback = two_fa_controller()->resendCode(Request::create('/int/v1/two-fa/resend', 'POST', [ + 'identity' => $user->email, + 'token' => $token, + ])); + + expect($challenge->getData(true))->toMatchArray(['method' => 'authenticator_app', 'expired' => false]) + ->and($fallback->getData(true)['method'])->toBe('email'); +}); diff --git a/tests/Unit/Http/UserControllerTest.php b/tests/Unit/Http/UserControllerTest.php index bdc128c2..f91f1288 100644 --- a/tests/Unit/Http/UserControllerTest.php +++ b/tests/Unit/Http/UserControllerTest.php @@ -13,6 +13,7 @@ use Fleetbase\Http\Requests\Internal\ValidatePasswordRequest; use Fleetbase\Models\Role; use Fleetbase\Models\User; +use Fleetbase\Support\TwoFactorAuth; use Illuminate\Container\Container; use Illuminate\Database\Capsule\Manager as Capsule; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; @@ -2214,3 +2215,68 @@ function user_controller_managed_account(Capsule $capsule, array $attributes = [ 'a driver account' => [['type' => 'driver'], null], 'an account promoted by an operator' => [['type' => 'user'], ['promoted_from' => 'contact']], ]); + +test('user controller authenticator app endpoints skip the generic resource permission check', function (string $method) { + user_controller_database(); + + expect((new ReflectionMethod(UserController::class, $method))->getAttributes(Fleetbase\Attributes\SkipAuthorizationCheck::class))->toHaveCount(1); +})->with(['getAuthenticatorApp', 'setupAuthenticatorApp', 'confirmAuthenticatorApp', 'disableAuthenticatorApp', 'regenerateRecoveryCodes']); + +test('user controller sets up an authenticator app after checking the current password', function () { + user_controller_database(); + $user = user_controller_user('owner-1'); + + $wrongPassword = user_controller()->setupAuthenticatorApp(user_controller_request('POST', ['password' => 'nope'], $user, 'setupAuthenticatorApp')); + $setup = @user_controller()->setupAuthenticatorApp(user_controller_request('POST', ['password' => 'old-password'], $user, 'setupAuthenticatorApp')); + $secret = $setup->getData(true)['secret']; + $wrongCode = user_controller()->confirmAuthenticatorApp(user_controller_request('POST', ['code' => '000000'], $user, 'confirmAuthenticatorApp')); + $confirmed = user_controller()->confirmAuthenticatorApp(user_controller_request('POST', [ + 'code' => (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($secret), + ], $user, 'confirmAuthenticatorApp')); + + expect($wrongPassword->getStatusCode())->toBe(422) + ->and($setup->getStatusCode())->toBe(200) + ->and(array_keys($setup->getData(true)))->toBe(['secret', 'otpauth_url', 'qr_code', 'expires_at']) + ->and($wrongCode->getStatusCode())->toBe(422) + ->and($wrongCode->getData(true))->toBe(['errors' => ['The code from your authenticator app is not correct.']]) + ->and($confirmed->getStatusCode())->toBe(200) + ->and($confirmed->getData(true)['recovery_codes'])->toHaveCount(8) + ->and($confirmed->getData(true)['status'])->toMatchArray(['enabled' => true, 'recovery_codes_remaining' => 8]) + ->and($confirmed->getData(true)['settings'])->toBe(['enabled' => true, 'method' => 'authenticator_app']) + ->and(user_controller()->getAuthenticatorApp(user_controller_request('GET', [], $user, 'getAuthenticatorApp'))->getData(true)['enabled'])->toBeTrue(); +}); + +test('user controller only accepts the authenticator app as the two factor method once it is set up', function () { + user_controller_database(); + $user = user_controller_user('owner-1'); + + $rejected = user_controller()->saveTwoFactorSettings(user_controller_request('POST', [ + 'twoFaSettings' => ['enabled' => true, 'method' => 'authenticator_app'], + ], $user, 'saveTwoFactorSettings')); + + expect($rejected->getStatusCode())->toBe(422) + ->and(TwoFactorAuth::getTwoFaSettingsForUser($user)->value)->toBe(['enabled' => false, 'method' => 'email']); +}); + +test('user controller disables the authenticator app and replaces recovery codes only with the current password', function () { + user_controller_database(); + $user = user_controller_user('owner-1'); + $enrollment = @TwoFactorAuth::beginAuthenticatorEnrollment($user); + TwoFactorAuth::confirmAuthenticatorEnrollment($user, (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($enrollment['secret'])); + + $codesDenied = user_controller()->regenerateRecoveryCodes(user_controller_request('POST', ['password' => 'nope'], $user, 'regenerateRecoveryCodes')); + $codes = user_controller()->regenerateRecoveryCodes(user_controller_request('POST', ['password' => 'old-password'], $user, 'regenerateRecoveryCodes')); + $denied = user_controller()->disableAuthenticatorApp(user_controller_request('POST', ['password' => 'nope'], $user, 'disableAuthenticatorApp')); + $disabled = user_controller()->disableAuthenticatorApp(user_controller_request('POST', ['password' => 'old-password'], $user, 'disableAuthenticatorApp')); + $noApp = user_controller()->regenerateRecoveryCodes(user_controller_request('POST', ['password' => 'old-password'], $user, 'regenerateRecoveryCodes')); + + expect($codesDenied->getStatusCode())->toBe(422) + ->and($codes->getData(true)['recovery_codes'])->toHaveCount(8) + ->and($denied->getStatusCode())->toBe(422) + ->and(TestActivityLogger::$logged)->not->toBeEmpty() + ->and($disabled->getData(true))->toBe([ + 'status' => ['enabled' => false, 'confirmed_at' => null, 'recovery_codes_remaining' => 0], + 'settings' => ['enabled' => false, 'method' => 'email'], + ]) + ->and($noApp->getStatusCode())->toBe(422); +}); diff --git a/tests/Unit/Support/BarcodeTest.php b/tests/Unit/Support/BarcodeTest.php new file mode 100644 index 00000000..f3419176 --- /dev/null +++ b/tests/Unit/Support/BarcodeTest.php @@ -0,0 +1,59 @@ + 3, + 'num_cols' => 3, + 'bcode' => [ + [1, 1, 0], + [0, 0, 0], + [1, 0, 1], + ], + ]; + } +} + +test('barcode renders qr modules as merged runs on a white background with a quiet zone', function () { + expect(BarcodeFixedMatrix::qrCodeSvg('anything', 'M', 1))->toBe( + '' + . '' + . '' + . '' + ); +}); + +test('barcode encodes real qr codes with the standard quiet zone', function () { + $url = 'otpauth://totp/Fleetbase:user%40example.com?secret=JBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP&issuer=Fleetbase'; + $svg = @Barcode::qrCodeSvg($url); + + preg_match('/viewBox="0 0 (\d+) (\d+)"/', $svg, $size); + + // A QR code is 21 + 4n modules square, plus 4 blank modules on each side + expect((int) $size[1])->toBe((int) $size[2]) + ->and(((int) $size[1] - 8 - 21) % 4)->toBe(0) + ->and($svg)->toContain('fill="#ffffff"') + ->and($svg)->toContain(' Barcode::qrCodeSvg(''))->toThrow(InvalidArgumentException::class, 'The data could not be encoded as a QR code.') + ->and(fn () => Barcode::qrCodeSvg('data', 'X'))->toThrow(InvalidArgumentException::class, 'QR code error correction must be L, M, Q or H.'); +}); + +test('barcode renders png barcodes through milon', function () { + $png = @Barcode::png('order-123', 'QRCODE'); + + expect($png)->toBeString() + ->and(substr(base64_decode($png), 1, 3))->toBe('PNG'); +})->skip(!function_exists('imagecreate') && !extension_loaded('imagick'), 'needs GD or Imagick'); diff --git a/tests/Unit/Support/TwoFactorAuthTest.php b/tests/Unit/Support/TwoFactorAuthTest.php index 8c075ed7..ebce0c8c 100644 --- a/tests/Unit/Support/TwoFactorAuthTest.php +++ b/tests/Unit/Support/TwoFactorAuthTest.php @@ -44,6 +44,11 @@ public function exists(string $key): bool return array_key_exists($key, $this->values); } + public function get(string $key): mixed + { + return $this->values[$key] ?? null; + } + public function del(?string ...$keys): bool { foreach ($keys as $key) { @@ -676,3 +681,194 @@ function two_factor_auth_verification_code(User $user, Carbon $expiresAt): Verif ->and(fn () => TwoFactorAuth::resendCode('missing@example.com', $token))->toThrow(Exception::class, 'No user found using the provided identity') ->and(fn () => TwoFactorAuth::resendCode($user->email, 'invalid-token'))->toThrow(Exception::class, '2FA session is invalid.'); }); + +/** + * Enroll the user in an authenticator app and return its secret and recovery codes. + * + * @return array{0: string, 1: array} + */ +function two_factor_auth_enroll_authenticator(User $user): array +{ + $enrollment = TwoFactorAuth::beginAuthenticatorEnrollment($user); + $recoveryCodes = TwoFactorAuth::confirmAuthenticatorEnrollment($user, (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($enrollment['secret'])); + + return [$enrollment['secret'], $recoveryCodes]; +} + +function two_factor_auth_authenticator_record(User $user): array +{ + return json_decode(app('db')->table('settings')->where('key', 'user.' . $user->uuid . '.2fa_authenticator')->value('value') ?? '[]', true) ?? []; +} + +test('two factor auth sets up an authenticator app only once confirmed with a code from it', function () { + [$user] = two_factor_auth_fixtures(); + TwoFactorAuth::saveTwoFaSettingsForUser($user, ['enabled' => true, 'method' => 'email']); + + $enrollment = TwoFactorAuth::beginAuthenticatorEnrollment($user); + $pending = two_factor_auth_authenticator_record($user); + + expect($enrollment['secret'])->toMatch('/^[A-Z2-7]{32}$/') + ->and($enrollment['otpauth_url'])->toStartWith('otpauth://totp/Fleetbase:user%40example.com?secret=' . $enrollment['secret']) + ->and($enrollment['qr_code'])->toStartWith('data:image/svg+xml;base64,') + ->and($pending['pending_secret'])->toStartWith('encrypted:') + ->and(json_encode($pending))->not->toContain($enrollment['secret']) + ->and(TwoFactorAuth::hasAuthenticatorApp($user))->toBeFalse() + ->and(fn () => TwoFactorAuth::confirmAuthenticatorEnrollment($user, '000000'))->toThrow(Exception::class, 'The code from your authenticator app is not correct.'); + + $recoveryCodes = TwoFactorAuth::confirmAuthenticatorEnrollment($user, (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($enrollment['secret'])); + $record = two_factor_auth_authenticator_record($user); + + expect($recoveryCodes)->toHaveCount(TwoFactorAuth::RECOVERY_CODE_COUNT) + ->and($recoveryCodes[0])->toMatch('/^[a-z2-9]{5}-[a-z2-9]{5}$/') + ->and($record)->not->toHaveKey('pending_secret') + ->and($record['secret'])->toStartWith('encrypted:') + ->and(json_encode($record))->not->toContain($recoveryCodes[0]) + ->and(TwoFactorAuth::usesAuthenticatorApp($user))->toBeTrue() + ->and(TwoFactorAuth::getTwoFaSettingsForUser($user)->value)->toBe(['enabled' => true, 'method' => 'authenticator_app']) + ->and(TwoFactorAuth::getAuthenticatorStatus($user))->toMatchArray(['enabled' => true, 'recovery_codes_remaining' => 8]) + ->and(array_column(TestActivityLogger::$logged, 'event'))->toBe(['authenticator_enabled']); +}); + +test('two factor auth rejects an authenticator setup that has expired', function () { + [$user] = two_factor_auth_fixtures(); + + $enrollment = TwoFactorAuth::beginAuthenticatorEnrollment($user); + Carbon::setTestNow(Carbon::now()->addSeconds(TwoFactorAuth::AUTHENTICATOR_ENROLLMENT_TTL + 1)); + + expect(fn () => TwoFactorAuth::confirmAuthenticatorEnrollment($user, (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($enrollment['secret']))) + ->toThrow(Exception::class, 'Authenticator setup has expired. Start again.') + ->and(TwoFactorAuth::hasAuthenticatorApp($user))->toBeFalse(); +}); + +test('two factor auth signs in authenticator app users with a code from the app and sends nothing', function () { + [$user, , $redis] = two_factor_auth_fixtures(); + [$secret] = two_factor_auth_enroll_authenticator($user); + TestActivityLogger::$logged = []; + + $token = TwoFactorAuth::start($user); + $clientToken = TwoFactorAuth::getClientSessionTokenFromTwoFaSession($token, $user->email); + + expect(app('db')->table('verification_codes')->count())->toBe(0) + ->and(app('mail.manager')->sent)->toBe([]) + ->and(base64_decode($clientToken))->not->toContain($user->uuid) + ->and(TwoFactorAuth::getChallengeMethod($user->email, $clientToken))->toBe('authenticator_app') + ->and(TwoFactorAuth::getClientSessionTokenFromTwoFaSession($token, $user->email, $clientToken))->toBe($clientToken) + ->and(TwoFactorAuth::validateSessionToken($token, $user->email, $clientToken))->toBeTrue(); + + // Enrolling used the current time step, so sign in with the next one, which the + // clock drift window still accepts + $google2fa = new PragmaRX\Google2FA\Google2FA(); + $nextCode = $google2fa->oathTotp($secret, two_factor_auth_authenticator_record($user)['last_timestep'] + 1); + $accessToken = TwoFactorAuth::verifyCode($nextCode, $token, $clientToken); + + expect($accessToken)->toContain('|') + ->and(app('db')->table('personal_access_tokens')->where('tokenable_id', $user->uuid)->count())->toBe(1) + ->and(array_keys($redis->values))->toBe([]) + ->and(TestActivityLogger::$logged)->toBe([ + ['log' => 'auth', 'description' => 'Two-factor sign-in verified', 'causer' => $user->uuid, 'subject' => $user->uuid, 'properties' => ['method' => 'authenticator_app'], 'event' => 'two_factor_verified'], + ]); +}); + +test('two factor auth does not accept the same authenticator code twice', function () { + [$user] = two_factor_auth_fixtures(); + [$secret] = two_factor_auth_enroll_authenticator($user); + + // The enrollment code is the current one, so it cannot be used again to sign in + $code = (new PragmaRX\Google2FA\Google2FA())->getCurrentOtp($secret); + $token = TwoFactorAuth::start($user); + $clientToken = TwoFactorAuth::getClientSessionTokenFromTwoFaSession($token, $user->email); + + expect(fn () => TwoFactorAuth::verifyCode($code, $token, $clientToken))->toThrow(Exception::class, 'Verification code does not match.') + ->and(TwoFactorAuth::verifyAuthenticatorCode($user, $code))->toBeNull(); +}); + +test('two factor auth accepts each recovery code once', function () { + [$user, , $redis] = two_factor_auth_fixtures(); + [, $recoveryCodes] = two_factor_auth_enroll_authenticator($user); + TestActivityLogger::$logged = []; + + $token = TwoFactorAuth::start($user); + $clientToken = TwoFactorAuth::getClientSessionTokenFromTwoFaSession($token, $user->email); + $accessToken = TwoFactorAuth::verifyCode(strtoupper(str_replace('-', ' ', $recoveryCodes[0])), $token, $clientToken); + + expect($accessToken)->toContain('|') + ->and(TwoFactorAuth::getAuthenticatorStatus($user)['recovery_codes_remaining'])->toBe(7) + ->and(array_column(TestActivityLogger::$logged, 'event'))->toBe(['recovery_code_used', 'two_factor_verified']) + ->and(TestActivityLogger::$logged[1]['properties'])->toBe(['method' => 'recovery_code']) + ->and(TwoFactorAuth::verifyAuthenticatorCode($user, $recoveryCodes[0]))->toBeNull() + ->and(TwoFactorAuth::verifyAuthenticatorCode($user, $recoveryCodes[1]))->toBe('recovery_code'); +}); + +test('two factor auth counts wrong authenticator codes towards the lockout', function () { + [$user, , $redis] = two_factor_auth_fixtures(); + two_factor_auth_enroll_authenticator($user); + + $token = TwoFactorAuth::start($user); + $clientToken = TwoFactorAuth::getClientSessionTokenFromTwoFaSession($token, $user->email); + + for ($i = 1; $i < TwoFactorAuth::MAX_VERIFY_ATTEMPTS; $i++) { + expect(fn () => TwoFactorAuth::verifyCode('abcde-fghjk', $token, $clientToken))->toThrow(Exception::class, 'Verification code does not match.'); + } + + expect(fn () => TwoFactorAuth::verifyCode('000000', $token, $clientToken))->toThrow(Exception::class, 'Too many failed verification attempts. Please sign in again.') + ->and(TwoFactorAuth::validateSessionToken($token, $user->email, $clientToken))->toBeFalse() + ->and(array_column(TestActivityLogger::$logged, 'event'))->toContain('two_factor_locked'); +}); + +test('two factor auth rejects an authenticator challenge for another user', function () { + [$user] = two_factor_auth_fixtures(); + two_factor_auth_enroll_authenticator($user); + + $other = new User(['uuid' => '44444444-4444-4444-8444-444444444444', 'email' => 'other@example.com', 'name' => 'Other']); + app('db')->table('users')->insert(['uuid' => $other->uuid, 'email' => $other->email, 'name' => $other->name, 'created_at' => now(), 'updated_at' => now()]); + TwoFactorAuth::saveTwoFaSettingsForUser($other, ['enabled' => true, 'method' => 'email']); + + $clientToken = TwoFactorAuth::getClientSessionTokenFromTwoFaSession(TwoFactorAuth::start($user), $user->email); + $otherToken = TwoFactorAuth::start($other); + + expect(TwoFactorAuth::validateSessionToken($otherToken, $other->email, $clientToken))->toBeFalse() + ->and(fn () => TwoFactorAuth::getClientSessionTokenFromTwoFaSession($otherToken, $other->email, $clientToken))->toThrow(Exception::class, '2FA Verification session has expired.') + ->and(fn () => TwoFactorAuth::verifyCode('123456', $otherToken, $clientToken))->toThrow(Exception::class, 'Verification code is invalid.'); +}); + +test('two factor auth sends authenticator app users a code by email as a fallback', function () { + [$user] = two_factor_auth_fixtures(); + two_factor_auth_enroll_authenticator($user); + + $token = TwoFactorAuth::start($user); + $clientToken = TwoFactorAuth::resendCode($user->email, $token); + + expect(app('db')->table('verification_codes')->count())->toBe(1) + ->and(app('mail.manager')->sent)->toHaveCount(1) + ->and(TwoFactorAuth::getChallengeMethod($user->email, $clientToken))->toBe('email') + ->and(TwoFactorAuth::verifyCode(app('db')->table('verification_codes')->value('code'), $token, $clientToken))->toContain('|'); +}); + +test('two factor auth turns two factor off when the authenticator app it used is removed', function () { + [$user] = two_factor_auth_fixtures(); + [, $recoveryCodes] = two_factor_auth_enroll_authenticator($user); + + TwoFactorAuth::disableAuthenticatorApp($user); + + expect(TwoFactorAuth::hasAuthenticatorApp($user))->toBeFalse() + ->and(two_factor_auth_authenticator_record($user))->toBe([]) + ->and(TwoFactorAuth::getTwoFaSettingsForUser($user)->value)->toBe(['enabled' => false, 'method' => 'email']) + ->and(TwoFactorAuth::verifyAuthenticatorCode($user, $recoveryCodes[0]))->toBeNull() + ->and(array_column(TestActivityLogger::$logged, 'event'))->toBe(['authenticator_enabled', 'authenticator_disabled']); +}); + +test('two factor auth replaces recovery codes', function () { + [$user] = two_factor_auth_fixtures(); + [, $oldCodes] = two_factor_auth_enroll_authenticator($user); + + $newCodes = TwoFactorAuth::regenerateRecoveryCodes($user); + + expect($newCodes)->toHaveCount(8) + ->and(array_intersect($oldCodes, $newCodes))->toBe([]) + ->and(TwoFactorAuth::verifyAuthenticatorCode($user, $oldCodes[0]))->toBeNull() + ->and(TwoFactorAuth::verifyAuthenticatorCode($user, $newCodes[0]))->toBe('recovery_code'); + + TwoFactorAuth::disableAuthenticatorApp($user); + + expect(fn () => TwoFactorAuth::regenerateRecoveryCodes($user))->toThrow(Exception::class, 'Set up an authenticator app first.'); +}); From fb3d405042143d79756dbcbc856eaece92ba2297 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Sun, 27 Sep 2026 14:32:20 +0800 Subject: [PATCH 07/10] fix(iam): close authorization gaps in core controllers - Organization settings: any member could update the company, including owner_uuid (take ownership) and billing/lifecycle fields, and change the organization 2FA policy. Updates and the 2FA policy now require the owner, the Administrator role or a system admin; owner, Stripe ids, plan, status, trial and type are ignored for non-admin updates (ownership keeps its transfer endpoint). - System-wide 2FA policy save is restricted to system admins. - Admin platform metrics are restricted to system admins; IAM and developer metrics require iam list user / developers list api-key. - Reports resolved to the "fleetbase" service, so no permission ever matched and every report endpoint was open. ReportController now uses the iam service, and direct query execution/export/download require iam execute/export report. - API credentials, webhooks, API events and request logs resolved to resource names (api-credential, webhook-endpoint, ...) that no permission uses, leaving them unguarded. Controllers can now declare $permissionResource, used by Auth when resolving permissions; these map to the Developers schema resources (api-key, webhook, event, log). - Auth::cannotUnlessAdmin() for explicit checks outside AuthorizationGuard. --- .../Internal/v1/AdminMetricsController.php | 13 ++++ .../Internal/v1/ApiCredentialController.php | 5 ++ .../Internal/v1/ApiEventController.php | 5 ++ .../Internal/v1/ApiRequestLogController.php | 5 ++ .../Internal/v1/CompanyController.php | 46 +++++++++++++++ .../v1/DeveloperMetricsController.php | 13 ++++ .../Internal/v1/IamMetricsController.php | 13 ++++ .../Internal/v1/ReportController.php | 23 ++++++++ .../Internal/v1/TwoFaController.php | 12 ++++ .../Internal/v1/WebhookEndpointController.php | 5 ++ .../v1/WebhookRequestLogController.php | 5 ++ src/Support/Auth.php | 38 +++++++++++- src/Traits/HasApiControllerBehavior.php | 16 +++++ tests/Unit/Http/CompanyControllerTest.php | 41 +++++++++++-- tests/Unit/Http/ReportControllerTest.php | 18 ++++++ tests/Unit/Http/TwoFaControllerTest.php | 14 +++++ tests/Unit/Support/AuthSupportTest.php | 59 +++++++++++++++++++ 17 files changed, 325 insertions(+), 6 deletions(-) diff --git a/src/Http/Controllers/Internal/v1/AdminMetricsController.php b/src/Http/Controllers/Internal/v1/AdminMetricsController.php index 65404844..0dfafd78 100644 --- a/src/Http/Controllers/Internal/v1/AdminMetricsController.php +++ b/src/Http/Controllers/Internal/v1/AdminMetricsController.php @@ -6,6 +6,7 @@ use Fleetbase\Models\Activity; use Fleetbase\Models\Company; use Fleetbase\Models\User; +use Fleetbase\Support\Auth; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Carbon; @@ -14,6 +15,18 @@ class AdminMetricsController extends Controller { + public function __construct() + { + // Platform-wide figures: system administrators only. + $this->middleware(function ($request, $next) { + if (!Auth::getUserFromSession($request)?->isAdmin()) { + return response()->error('Only system administrators can view platform metrics.', 401); + } + + return $next($request); + }); + } + public function kpi(Request $request, string $slug): JsonResponse { [$currentPeriodStart, $previousPeriodStart] = $this->periodBoundaries(); diff --git a/src/Http/Controllers/Internal/v1/ApiCredentialController.php b/src/Http/Controllers/Internal/v1/ApiCredentialController.php index 4d482114..9e9c64b4 100644 --- a/src/Http/Controllers/Internal/v1/ApiCredentialController.php +++ b/src/Http/Controllers/Internal/v1/ApiCredentialController.php @@ -33,6 +33,11 @@ class ApiCredentialController extends FleetbaseController */ public $service = 'developers'; + /** + * The IAM schema resource this controller's permissions use. + */ + public string $permissionResource = 'api-key'; + /** * Create a new API credential record. * diff --git a/src/Http/Controllers/Internal/v1/ApiEventController.php b/src/Http/Controllers/Internal/v1/ApiEventController.php index fcf4eb51..b73ffcb0 100644 --- a/src/Http/Controllers/Internal/v1/ApiEventController.php +++ b/src/Http/Controllers/Internal/v1/ApiEventController.php @@ -19,4 +19,9 @@ class ApiEventController extends FleetbaseController * @var string */ public $service = 'developers'; + + /** + * The IAM schema resource this controller's permissions use. + */ + public string $permissionResource = 'event'; } diff --git a/src/Http/Controllers/Internal/v1/ApiRequestLogController.php b/src/Http/Controllers/Internal/v1/ApiRequestLogController.php index bb3ef65f..a2f85852 100644 --- a/src/Http/Controllers/Internal/v1/ApiRequestLogController.php +++ b/src/Http/Controllers/Internal/v1/ApiRequestLogController.php @@ -19,4 +19,9 @@ class ApiRequestLogController extends FleetbaseController * @var string */ public $service = 'developers'; + + /** + * The IAM schema resource this controller's permissions use. + */ + public string $permissionResource = 'log'; } diff --git a/src/Http/Controllers/Internal/v1/CompanyController.php b/src/Http/Controllers/Internal/v1/CompanyController.php index 14f42b4d..9a9a5cfd 100644 --- a/src/Http/Controllers/Internal/v1/CompanyController.php +++ b/src/Http/Controllers/Internal/v1/CompanyController.php @@ -32,6 +32,26 @@ class CompanyController extends FleetbaseController */ public $resource = 'company'; + /** + * Company attributes only platform administrators may change through updateRecord(). + */ + private const PLATFORM_MANAGED_FIELDS = ['owner_uuid', 'stripe_customer_id', 'stripe_connect_id', 'plan', 'status', 'trial_ends_at', 'type']; + + public function __construct() + { + parent::__construct(); + + // Organization settings and the organization's 2FA policy: owner, Administrator role or system admin. + $this->middleware(function ($request, $next) { + $company = Company::where('uuid', session('company'))->first(); + if (!$company || !$this->currentUserManagesOrganization($company)) { + return response()->error('Only the organization owner or an Administrator can change organization settings.', 401); + } + + return $next($request); + })->only(['updateRecord', 'saveTwoFactorSettings']); + } + /** * Find an organization visible to the current session company. * @@ -63,6 +83,11 @@ public function updateRecord(Request $request, string $id) try { $input = $this->model->getApiPayloadFromRequest($request); + + // Ownership moves through transferOwnership(); billing and lifecycle fields are platform-managed. + if (!$request->user()?->isAdmin()) { + $input = Arr::except($input, self::PLATFORM_MANAGED_FIELDS); + } $input = $this->model->fillSessionAttributes($input, [], ['updated_by_uuid']); if ($this->model->isColumn('slug')) { @@ -157,6 +182,7 @@ public function saveTwoFactorSettings(Request $request) if (!$company) { return response()->error('No company session found', 401); } + if (isset($twoFaSettings['enabled']) && $twoFaSettings['enabled'] === false) { $twoFaSettings['enforced'] = false; } @@ -261,6 +287,26 @@ private function resolveVisibleCompanyForUsers(string $id, Request $request): ?C ->first(); } + /** + * Whether the session user may manage the organization: platform admins, the owner, + * and members holding the Administrator role in it. + */ + private function currentUserManagesOrganization(Company $company): bool + { + $user = Auth::getUserFromSession(); + if (!$user) { + return false; + } + + if ($user->isAdmin() || $company->owner_uuid === $user->uuid) { + return true; + } + + $companyUser = CompanyUser::where('company_uuid', $company->uuid)->where('user_uuid', $user->uuid)->first(); + + return $companyUser !== null && $companyUser->roles()->where('name', 'Administrator')->exists(); + } + private function resolveVisibleCompany(string $id): ?Company { $sessionCompany = session('company'); diff --git a/src/Http/Controllers/Internal/v1/DeveloperMetricsController.php b/src/Http/Controllers/Internal/v1/DeveloperMetricsController.php index 6add50a9..4eb3baf1 100644 --- a/src/Http/Controllers/Internal/v1/DeveloperMetricsController.php +++ b/src/Http/Controllers/Internal/v1/DeveloperMetricsController.php @@ -8,12 +8,25 @@ use Fleetbase\Models\ApiRequestLog; use Fleetbase\Models\WebhookEndpoint; use Fleetbase\Models\WebhookRequestLog; +use Fleetbase\Support\Auth; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Carbon; class DeveloperMetricsController extends Controller { + public function __construct() + { + // Not a resource controller, so AuthorizationGuard cannot resolve a permission for it. + $this->middleware(function ($request, $next) { + if (Auth::cannotUnlessAdmin('developers list api-key')) { + return response()->error('User is not authorized to list api-key', 401); + } + + return $next($request); + }); + } + public function kpis(Request $request): JsonResponse { [$start, $end, $previousStart, $previousEnd] = $this->periods($request); diff --git a/src/Http/Controllers/Internal/v1/IamMetricsController.php b/src/Http/Controllers/Internal/v1/IamMetricsController.php index 27a75a9d..e8208812 100644 --- a/src/Http/Controllers/Internal/v1/IamMetricsController.php +++ b/src/Http/Controllers/Internal/v1/IamMetricsController.php @@ -11,6 +11,7 @@ use Fleetbase\Models\Role; use Fleetbase\Models\Setting; use Fleetbase\Models\User; +use Fleetbase\Support\Auth; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Carbon; @@ -21,6 +22,18 @@ class IamMetricsController extends Controller { private const DORMANT_DAYS = 90; + public function __construct() + { + // Not a resource controller, so AuthorizationGuard cannot resolve a permission for it. + $this->middleware(function ($request, $next) { + if (Auth::cannotUnlessAdmin('iam list user')) { + return response()->error('User is not authorized to list user', 401); + } + + return $next($request); + }); + } + public function kpis(Request $request): JsonResponse { $companyUuid = session('company'); diff --git a/src/Http/Controllers/Internal/v1/ReportController.php b/src/Http/Controllers/Internal/v1/ReportController.php index 7cb7f29d..1693292c 100644 --- a/src/Http/Controllers/Internal/v1/ReportController.php +++ b/src/Http/Controllers/Internal/v1/ReportController.php @@ -2,8 +2,10 @@ namespace Fleetbase\Http\Controllers\Internal\v1; +use Fleetbase\Attributes\SkipAuthorizationCheck; use Fleetbase\Http\Controllers\FleetbaseController; use Fleetbase\Models\Report; +use Fleetbase\Support\Auth; use Fleetbase\Support\Reporting\ComputedColumnValidator; use Fleetbase\Support\Reporting\ReportQueryConverter; use Fleetbase\Support\Reporting\ReportQueryErrorHandler; @@ -22,12 +24,30 @@ class ReportController extends FleetbaseController */ public $resource = 'report'; + /** + * The IAM schema service for report permissions (`iam list report`, `iam execute report`, ...). + * + * @var string + */ + public $service = 'iam'; + protected ReportQueryValidator $queryValidator; protected ReportQueryErrorHandler $errorHandler; public function __construct() { parent::__construct(); + + // These endpoints skip AuthorizationGuard (their method names do not map to a schema action). + foreach (['execute' => ['executeQuery'], 'export' => ['exportQuery', 'download']] as $action => $methods) { + $this->middleware(function ($request, $next) use ($action) { + if (Auth::cannotUnlessAdmin("iam {$action} report")) { + return response()->error("User is not authorized to {$action} report", 401); + } + + return $next($request); + })->only($methods); + } $this->queryValidator = new ReportQueryValidator(app(ReportSchemaRegistry::class)); $this->errorHandler = new ReportQueryErrorHandler(); } @@ -238,6 +258,7 @@ public function execute(Request $request, string $id): JsonResponse /** * Execute a query directly without saving as report. */ + #[SkipAuthorizationCheck] public function executeQuery(Request $request): JsonResponse { try { @@ -368,6 +389,7 @@ public function export(Request $request, string $id): JsonResponse /** * Export query results directly without saving as report. */ + #[SkipAuthorizationCheck] public function exportQuery(Request $request): JsonResponse { try { @@ -431,6 +453,7 @@ public function exportQuery(Request $request): JsonResponse /** * Download exported file. */ + #[SkipAuthorizationCheck] public function download(Request $request, string $filename) { try { diff --git a/src/Http/Controllers/Internal/v1/TwoFaController.php b/src/Http/Controllers/Internal/v1/TwoFaController.php index f81c7d58..158aa1f9 100644 --- a/src/Http/Controllers/Internal/v1/TwoFaController.php +++ b/src/Http/Controllers/Internal/v1/TwoFaController.php @@ -15,6 +15,18 @@ */ class TwoFaController extends Controller { + public function __construct() + { + // The system-wide 2FA policy applies to every organization: system administrators only. + $this->middleware(function ($request, $next) { + if (!Auth::getUserFromSession($request)?->isAdmin()) { + return response()->error('Only system administrators can change the system two-factor policy.', 401); + } + + return $next($request); + })->only('saveSystemConfig'); + } + /** * Save Two-Factor Authentication system wide settings. * diff --git a/src/Http/Controllers/Internal/v1/WebhookEndpointController.php b/src/Http/Controllers/Internal/v1/WebhookEndpointController.php index 46731192..869eb880 100644 --- a/src/Http/Controllers/Internal/v1/WebhookEndpointController.php +++ b/src/Http/Controllers/Internal/v1/WebhookEndpointController.php @@ -37,6 +37,11 @@ class WebhookEndpointController extends FleetbaseController */ public $service = 'developers'; + /** + * The IAM schema resource this controller's permissions use. + */ + public string $permissionResource = 'webhook'; + /** * Enables a webhook endpoint. * diff --git a/src/Http/Controllers/Internal/v1/WebhookRequestLogController.php b/src/Http/Controllers/Internal/v1/WebhookRequestLogController.php index f172c7a7..9d4253db 100644 --- a/src/Http/Controllers/Internal/v1/WebhookRequestLogController.php +++ b/src/Http/Controllers/Internal/v1/WebhookRequestLogController.php @@ -19,4 +19,9 @@ class WebhookRequestLogController extends FleetbaseController * @var string */ public $service = 'developers'; + + /** + * The IAM schema resource this controller's permissions use. + */ + public string $permissionResource = 'log'; } diff --git a/src/Support/Auth.php b/src/Support/Auth.php index f5a21a4f..01d81466 100644 --- a/src/Support/Auth.php +++ b/src/Support/Auth.php @@ -395,7 +395,7 @@ public static function resolvePermissionsFromRequest(Request $request): Collecti } $service = $controller->getService(); - $resource = str_replace('_', '-', $controller->getResourceSingularName()); + $resource = static::getPermissionResourceFromController($controller); $action = ActionMapper::resolve($request, $resource); // If the resource is not guarded at all @@ -522,12 +522,24 @@ public static function applyDirectivesToQuery($builder, ?Request $request = null public static function getRequiredPermissionNameFromRequest(Request $request): string { $controller = $request->getController(); - $resource = str_replace('_', '-', $controller->getResourceSingularName()); + $resource = static::getPermissionResourceFromController($controller); $action = ActionMapper::resolve($request, $resource); return implode(' ', [$action, $resource]); } + /** + * Resolves the permission resource name for a resource controller. + */ + public static function getPermissionResourceFromController($controller): string + { + if (method_exists($controller, 'getPermissionResourceName')) { + return $controller->getPermissionResourceName(); + } + + return str_replace('_', '-', $controller->getResourceSingularName()); + } + /** * Checks if a resource is guarded by any permissions. * @@ -569,6 +581,28 @@ public static function can(string $permission): bool }); } + /** + * Determines if the current user lacks the specified permission, treating platform + * administrators (and a missing session user) the way AuthorizationGuard does. + * + * For explicit checks in controllers that the guard cannot resolve on its own. + * + * @param string $permission the permission string in the format '{service} {action} {resource}' + */ + public static function cannotUnlessAdmin(string $permission): bool + { + $user = static::getUserFromSession(); + if (!$user) { + return true; + } + + if ($user->isAdmin()) { + return false; + } + + return static::cannot($permission); + } + /** * Determines if the current user lacks the specified permission. * diff --git a/src/Traits/HasApiControllerBehavior.php b/src/Traits/HasApiControllerBehavior.php index 589f35aa..d3395472 100644 --- a/src/Traits/HasApiControllerBehavior.php +++ b/src/Traits/HasApiControllerBehavior.php @@ -271,6 +271,22 @@ public function getResourceSingularName(): string return $this->resourceSingularlName; } + /** + * Gets the resource name used in permission names, e.g. "user" in "iam list user". + * + * Defaults to the kebab-cased singular resource name. A controller whose model name + * differs from its IAM schema resource declares `public string $permissionResource` + * (for example ApiCredentialController uses the schema resource "api-key"). + */ + public function getPermissionResourceName(): string + { + if (property_exists($this, 'permissionResource') && !empty($this->permissionResource)) { + return $this->permissionResource; + } + + return str_replace('_', '-', $this->getResourceSingularName()); + } + /** * Gets the service associated with the controller. * diff --git a/tests/Unit/Http/CompanyControllerTest.php b/tests/Unit/Http/CompanyControllerTest.php index 0ecf7ef1..1445168e 100644 --- a/tests/Unit/Http/CompanyControllerTest.php +++ b/tests/Unit/Http/CompanyControllerTest.php @@ -762,17 +762,22 @@ function company_controller_bind_activity(): CompanyControllerActivityFake expect($foreign->getStatusCode())->toBe(404) ->and($foreign->getData(true))->toBe(['errors' => ['Organization not found.']]); + $before = $capsule->getConnection('mysql')->table('companies')->where('uuid', 'company-1')->first(); + $updated = company_controller()->updateRecord(company_controller_request('PUT', [ - 'name' => 'Acme Updated', - 'slug' => 'attempted-slug-change', - 'status' => 'suspended', + 'name' => 'Acme Updated', + 'slug' => 'attempted-slug-change', + 'status' => 'suspended', + 'owner_uuid' => 'attempted-owner-takeover', ]), 'company_public_1'); $record = $capsule->getConnection('mysql')->table('companies')->where('uuid', 'company-1')->first(); + // Ownership and lifecycle status are platform-managed and ignored for non-admin updates. expect($updated['company']->resource->name)->toBe('Acme Updated') ->and($record->name)->toBe('Acme Updated') - ->and($record->status)->toBe('suspended') + ->and($record->status)->toBe($before->status) + ->and($record->owner_uuid)->toBe($before->owner_uuid) ->and($record->slug)->toBe('acme-logistics'); $deleted = company_controller()->deleteRecord('company_public_1', company_controller_request('DELETE')); @@ -1427,3 +1432,31 @@ function company_controller_bind_activity(): CompanyControllerActivityFake ->and($notMember->getStatusCode())->toBe(400) ->and($notMember->getData(true))->toBe(['errors' => ['User selected to leave organization is not a member of this organization.']]); }); + +test('company controller only lets organization managers update settings or the organization 2fa policy', function () { + $capsule = company_controller_fixtures(); + + $registered = collect(company_controller()->getMiddleware()) + ->first(fn ($entry) => ($entry['options']['only'] ?? null) === ['updateRecord', 'saveTwoFactorSettings']); + expect($registered)->not->toBeNull(); + + $run = function (string $user) use ($registered) { + session(['company' => 'company-1', 'user' => $user]); + + return ($registered['middleware'])(company_controller_request('PUT'), fn () => 'allowed'); + }; + + // A plain member (the dispatcher case) is refused. + $refused = $run('member-1'); + expect($refused->getStatusCode())->toBe(401) + ->and($refused->getData(true))->toBe(['errors' => ['Only the organization owner or an Administrator can change organization settings.']]); + + // The owner, a member holding the Administrator role, and system admins are allowed. + expect($run('owner-1'))->toBe('allowed') + ->and($run('admin-1'))->toBe('allowed'); + + $capsule->getConnection('mysql')->table('model_has_roles')->insert([ + 'role_id' => 'Administrator', 'model_type' => Fleetbase\Models\CompanyUser::class, 'model_uuid' => 'pivot-member-1', + ]); + expect($run('member-1'))->toBe('allowed'); +}); diff --git a/tests/Unit/Http/ReportControllerTest.php b/tests/Unit/Http/ReportControllerTest.php index 7c961ccd..ea410ddf 100644 --- a/tests/Unit/Http/ReportControllerTest.php +++ b/tests/Unit/Http/ReportControllerTest.php @@ -916,3 +916,21 @@ function report_controller_use_query_validator(ReportController $controller, Rep ->and($current->company_uuid)->toBe('company-1') ->and($other)->toBeNull(); }); + +test('report controller maps direct query execution and exports to the iam report permissions', function () { + report_controller_bind(); + + $controller = new ReportController(); + $scoped = collect($controller->getMiddleware())->mapWithKeys(fn ($entry) => [implode(',', $entry['options']['only'] ?? []) => $entry['middleware']]); + + expect($scoped->keys()->all())->toBe(['executeQuery', 'exportQuery,download']) + ->and($controller->getService())->toBe('iam'); + + // No session user: both are refused before reaching the query engine. + session()->flush(); + $execute = ($scoped['executeQuery'])(Request::create('/int/v1/reports/execute-query', 'POST'), fn () => 'allowed'); + $export = ($scoped['exportQuery,download'])(Request::create('/int/v1/reports/export-query', 'POST'), fn () => 'allowed'); + + expect($execute->getData(true))->toBe(['errors' => ['User is not authorized to execute report']]) + ->and($export->getData(true))->toBe(['errors' => ['User is not authorized to export report']]); +}); diff --git a/tests/Unit/Http/TwoFaControllerTest.php b/tests/Unit/Http/TwoFaControllerTest.php index f9e77ba7..176e30f4 100644 --- a/tests/Unit/Http/TwoFaControllerTest.php +++ b/tests/Unit/Http/TwoFaControllerTest.php @@ -469,3 +469,17 @@ function two_fa_controller_verification_code(User $user, Carbon $expiresAt, stri ->and($response->getData(true)['code'])->toBe('console_access_not_allowed') ->and(app('db')->table('personal_access_tokens')->where('tokenable_id', $user->uuid)->count())->toBe(0); }); + +test('two fa controller restricts the system-wide policy save to system administrators', function () { + two_fa_controller_database(); + + $middleware = two_fa_controller()->getMiddleware(); + expect($middleware)->toHaveCount(1) + ->and($middleware[0]['options']['only'])->toBe(['saveSystemConfig']); + + session()->flush(); + $refused = ($middleware[0]['middleware'])(Request::create('/int/v1/two-fa/config', 'POST'), fn () => 'allowed'); + + expect($refused->getStatusCode())->toBe(401) + ->and($refused->getData(true))->toBe(['errors' => ['Only system administrators can change the system two-factor policy.']]); +}); diff --git a/tests/Unit/Support/AuthSupportTest.php b/tests/Unit/Support/AuthSupportTest.php index 4aa4c7cd..cd425cff 100644 --- a/tests/Unit/Support/AuthSupportTest.php +++ b/tests/Unit/Support/AuthSupportTest.php @@ -221,6 +221,30 @@ public function queryRecord(): void } } +class AuthSupportAliasedResourceController +{ + public string $permissionResource = 'api-key'; + + public function getService(): string + { + return 'developers'; + } + + public function getResourceSingularName(): string + { + return 'api_credential'; + } + + public function getPermissionResourceName(): string + { + return $this->permissionResource; + } + + public function queryRecord(): void + { + } +} + class AuthSupportSkipController { #[SkipAuthorizationCheck] @@ -787,6 +811,41 @@ function auth_support_request(string $method = 'GET', ?string $controllerClass = expect(Auth::resolvePermissionsFromRequest(auth_support_request('GET', AuthSupportSkipController::class))->isEmpty())->toBeTrue(); }); +test('auth support resolves permissions against a controller permission resource alias', function () { + [$admin] = auth_support_fixtures(); + session(['user' => $admin->uuid]); + + app('db')->table('permissions')->insert([ + ['id' => 'permission-list-api-key', 'name' => 'developers list api-key', 'guard_name' => 'sanctum', 'service' => 'developers', 'created_at' => now(), 'updated_at' => now()], + ]); + + $request = auth_support_request('GET', AuthSupportAliasedResourceController::class); + + // Without the alias the model name "api-credential" matches no permission and the guard lets everyone through. + expect(Auth::isResourceGuarded('api-credential'))->toBeFalse() + ->and(Auth::getRequiredPermissionNameFromRequest($request))->toBe('list api-key') + ->and(Auth::resolvePermissionsFromRequest($request)->pluck('id')->all())->toBe(['permission-list-api-key']) + ->and(Auth::getPermissionResourceFromController(app(AuthSupportResourceController::class)))->toBe('user'); +}); + +test('auth support cannotUnlessAdmin lets platform admins through and checks everyone else', function () { + [$admin] = auth_support_fixtures(); + + app('db')->table('permissions')->insert([ + ['id' => 'permission-list-user-admin-check', 'name' => 'iam list user', 'guard_name' => 'sanctum', 'service' => 'iam', 'created_at' => now(), 'updated_at' => now()], + ]); + + session(['user' => $admin->uuid]); + $admin->forceFill(['type' => 'admin'])->save(); + expect(Auth::cannotUnlessAdmin('iam list user'))->toBeFalse(); + + $admin->forceFill(['type' => 'user'])->save(); + expect(Auth::cannotUnlessAdmin('iam list user'))->toBeTrue(); + + session(['user' => null]); + expect(Auth::cannotUnlessAdmin('iam list user'))->toBeTrue(); +}); + test('auth support filters and applies directives for assigned role and policy subjects', function () { [$admin] = auth_support_fixtures(); session(['user' => $admin->uuid]); From da9043015fca082fb3b77c4a171679afa8dede16 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 28 Sep 2026 11:27:40 +0800 Subject: [PATCH 08/10] chore(release): v1.6.65 --- RELEASE.md | 46 ++++++++++++++++++++++++++++------------------ composer.json | 2 +- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 86343070..0ef370f9 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,31 +1,41 @@ -# v1.6.64 — Order reporting and test SMS with the entered credentials +# v1.6.65 — Authenticator-app 2FA, sign-in hardening and IAM permission fixes -## Improvements for reporting +## Improvements -- Declare row-level expression columns with `Column::expression($name, $sql, $type)`. Bare names resolve against the table or relationship that declares it, so `JSON_EXTRACT(meta, '$.quantity')` on `payload.entities` reads the joined entity's `meta`. An expression column can be selected, filtered, sorted, grouped by and aggregated. -- Summary columns (`Column::count/sum/avg`) are flagged `aggregate` and resolve to their computation. Without grouping they return a single summary row; with grouping they sit beside the group keys. -- `Table::softDeletes()` and `Relationship::softDeletes()` leave out soft-deleted rows. On joins the filter goes in the `ON` clause, so LEFT joins keep the parent row. -- Computed columns can be group keys, conditions and sort columns, and a grouped report can be sorted by an aggregate's alias. A new `count_distinct` aggregate is available. -- Custom expressions accept the JSON functions, `DATE()`, `CAST(… AS DECIMAL(15,2))` and the other cast types, `DISTINCT`, `IN`, `GROUP_CONCAT(… ORDER BY … SEPARATOR …)`, `->`/`->>` and `INTERVAL n UNIT`. -- `public_id` and `internal_id` are no longer hidden as foreign keys, so ID columns appear in the column picker. -- Relationship columns are labelled with the whole relationship name ("Order Config Namespace", not "Order Namespace"), and aggregate labels use the column label ("Sum (Quantity)"). -- `_key` and `_import_id` are never listed or selectable, whatever a schema declares. +- Sign in with an authenticator app (TOTP, RFC 6238), next to email and SMS 2FA (#163). It works with Authy, Google Authenticator, Microsoft Authenticator and 1Password. + - New `users/two-fa/authenticator` endpoints to set up, confirm, disable and inspect the app. Setup, disable and regenerating recovery codes need the current password. + - Confirming the app returns 8 single-use recovery codes. `two-fa/verify` accepts an app code (±1 step for clock drift, each code once) or a recovery code. + - `two-fa/resend` falls back to an emailed (or SMS) code and returns the new `method`. + - The secret is encrypted with the app key, and recovery codes are stored as keyed hashes. + - New `Fleetbase\Support\Barcode` helper with a compact `qrCodeSvg()`. +- Record the app, APNs environment and last-seen time on user devices. New nullable `user_devices` columns: `app_identifier` (indexed), `environment` and `last_seen_at`. +- New organization setting to let users change their own password (default on), at `GET/POST companies/auth-settings`. `GET users/password-policy` returns `{can_change_password}`. ## Fixes -- Test SMS Provider and Test Twilio in Admin › System Config › Services use the credentials entered in the form (fleetbase/fleetbase#680). Under Octane the Twilio client was built once per worker, so a test failed with "Credentials are required to create a Client" or reported success for the saved account. The endpoints now rebuild the client from the request's config and release it after the send. +- Settings → Notifications applies when notifications are sent from the queue or a console command (#262). `NotificationRegistry` takes the company from the notification's subject instead of the session. `notifyUsingDefinitionName()` reads the company-scoped settings instead of a global key nothing writes. New `Setting::lookupForCompany()`. +- Invited users can set their first password, and non-admins can change their own (#263). The password endpoints no longer fall through to `iam create user`. +- FCM order notifications are sent with Android `priority: high`, so drivers' phones in Doze get them immediately (#268). ## Security -- Every computed column is validated up front, including in grouped reports. Names must be safe identifiers, and `SELECT` is forbidden. -- Schema-declared columns always take their SQL from the registry, never from the request. Group keys, aggregate columns, sort columns and condition fields must be allowed or computed columns. -- Sort direction is normalised to `asc`/`desc`, and grouped reports validate `aggregateBy.computation`. +- A 2FA session starts only after the password is checked. `GET two-fa/check` used to start one from the identity alone, so an email plus the emailed code was enough to sign in. It now always returns `{twoFaSession: null, isTwoFaEnabled: false}` and no longer reveals whether an account uses 2FA. +- 2FA sessions expire after 10 minutes; they used to live about 56 years. The 5th wrong code deletes the session, and resending doesn't reset the count. Codes are compared in constant time and generated with `random_int`. +- `users/change-password` requires `current_password` in the same request, and `iam change-password` is enforced. `users/set-password` works only once, within 24 hours of accepting an invite. `validate-password` and `change-password` are limited to 10 requests per minute. +- Close authorization gaps where `AuthorizationGuard` resolved to permission names that don't exist: + - Updating the organization and its 2FA policy is limited to the owner, the Administrator role and system admins. Non-admin updates ignore `owner_uuid`, Stripe ids, `plan`, `status`, `trial_ends_at` and `type`. + - `POST two-fa/config` (system 2FA policy) and admin platform metrics are limited to system admins. + - IAM and developer metrics need `iam list user` / `developers list api-key`. + - Reports use the `iam` service: `iam execute report` for direct queries, `iam export report` for exports. + - API credentials, webhooks, API events and request logs check the `api-key`, `webhook`, `event` and `log` permissions. +- Password, auth-setting and authenticator changes are written to the `auth` activity log. ## Behaviour changes -- In a grouped report, a selected column that is neither a group key nor aggregated is now an error instead of being dropped. -- Invalid report shapes fail with a clear message instead of an SQL error. +- Consoles need the companion fleetbase/fleetbase changes: the 2FA sign-in flow (`fix/2fa-login-hardening`), `current_password` on change-password (fleetbase/fleetbase#685) and the authenticator-app UI (fleetbase/fleetbase#686). With an older console, users with 2FA can't sign in and self-service password changes fail. +- Users need `iam … report` permissions to use reports, and `developers …` permissions for API keys, webhooks, events and logs. Fleet-Ops' report screens check the same names in fleetbase/fleetops#345. +- A user who accepted an invite before this release but never set a password should use **Forgot password**. -A database migration is not required. No configuration change is needed. The FleetOps order report schema ships in fleetbase/fleetops v0.6.70, and the report builder changes in fleetbase/ember-ui v0.4.4. +Run the migrations for the new `user_devices` columns. Run `composer update` to install `pragmarx/google2fa`. -Changes: [#269](https://github.com/fleetbase/core-api/pull/269), [#270](https://github.com/fleetbase/core-api/pull/270). +Changes: [#272](https://github.com/fleetbase/core-api/pull/272), [#273](https://github.com/fleetbase/core-api/pull/273), [#274](https://github.com/fleetbase/core-api/pull/274), [#275](https://github.com/fleetbase/core-api/pull/275), [#276](https://github.com/fleetbase/core-api/pull/276), [#277](https://github.com/fleetbase/core-api/pull/277), [#278](https://github.com/fleetbase/core-api/pull/278). diff --git a/composer.json b/composer.json index 5ef971d4..8d61f2bd 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "fleetbase/core-api", - "version": "1.6.64", + "version": "1.6.65", "description": "Core Framework and Resources for Fleetbase API", "keywords": [ "fleetbase", From c8296aaa05a53e4226c42294f75bca575b7753df Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 28 Sep 2026 16:42:21 +0800 Subject: [PATCH 09/10] feat: enhance admin organization queries and usage summaries --- .../Internal/v1/CompanyController.php | 21 ++++- src/Http/Filter/CompanyFilter.php | 70 +++++++++++++- src/Http/Resources/Organization.php | 3 +- src/Http/Resources/User.php | 4 + src/Models/Company.php | 2 +- src/Support/OrganizationAdminSummary.php | 71 ++++++++++++++ src/routes.php | 1 + tests/Unit/Http/CompanyControllerTest.php | 94 +++++++++++++++++++ .../Unit/Http/ConcreteFilterContractsTest.php | 41 ++++++++ 9 files changed, 301 insertions(+), 6 deletions(-) create mode 100644 src/Support/OrganizationAdminSummary.php diff --git a/src/Http/Controllers/Internal/v1/CompanyController.php b/src/Http/Controllers/Internal/v1/CompanyController.php index 4c96814b..f9baaffa 100644 --- a/src/Http/Controllers/Internal/v1/CompanyController.php +++ b/src/Http/Controllers/Internal/v1/CompanyController.php @@ -18,6 +18,7 @@ use Fleetbase\Models\Setting; use Fleetbase\Models\User; use Fleetbase\Support\Auth; +use Fleetbase\Support\OrganizationAdminSummary; use Fleetbase\Support\TwoFactorAuth; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -61,7 +62,7 @@ public function __construct() */ public function findRecord(Request $request, $id) { - $company = $this->resolveVisibleCompany($id); + $company = $this->resolveVisibleCompanyForUsers($id, $request); if (!$company) { return response()->error('Organization not found.', 404); @@ -298,6 +299,10 @@ public function users(string $id, Request $request) // replace in pagination $users->setCollection($transformedItems); + if ($request->user()?->isAdmin()) { + OrganizationAdminSummary::attachAuthentication($transformedItems); + } + return response()->json([ 'users' => UserResource::collection($users->getCollection()), 'meta' => [ @@ -322,9 +327,23 @@ public function users(string $id, Request $request) return $companyUser->user; }); + if ($request->user()?->isAdmin()) { + OrganizationAdminSummary::attachAuthentication($users); + } + return UserResource::collection($users); } + public function usage(string $id, AdminRequest $request): JsonResponse + { + $company = $this->resolveAdminCompany($id); + if (!$company) { + return response()->json(['error' => 'Organization not found.'], 404); + } + + return response()->json(['usage' => OrganizationAdminSummary::usage($company)]); + } + private function resolveVisibleCompanyForUsers(string $id, Request $request): ?Company { $user = $request->user(); diff --git a/src/Http/Filter/CompanyFilter.php b/src/Http/Filter/CompanyFilter.php index 47db8b1b..15dadfcb 100644 --- a/src/Http/Filter/CompanyFilter.php +++ b/src/Http/Filter/CompanyFilter.php @@ -29,7 +29,18 @@ function ($query) { public function query(?string $searchQuery) { - $this->builder->searchWhere('name', $searchQuery); + $this->builder->where(function ($query) use ($searchQuery) { + foreach (['name', 'description', 'phone', 'website_url', 'public_id', 'slug', 'country', 'timezone'] as $column) { + $query->orWhereRaw('LOWER(companies.' . $column . ') LIKE ?', ['%' . mb_strtolower(trim($searchQuery ?? '')) . '%']); + } + $query->orWhereHas('owner', function ($owner) use ($searchQuery) { + $owner->where(function ($query) use ($searchQuery) { + foreach (['name', 'email', 'phone', 'ip_address'] as $column) { + $query->orWhereRaw('LOWER(users.' . $column . ') LIKE ?', ['%' . mb_strtolower(trim($searchQuery ?? '')) . '%']); + } + }); + }); + }); } public function name(?string $name) @@ -39,12 +50,45 @@ public function name(?string $name) public function country(?string $country) { - $this->builder->searchWhere('country', $country); + $this->builder->where('country', strtoupper($country ?? '')); } public function status(?string $status) { - $this->builder->searchWhere('status', $status); + if ($status === 'active') { + $this->builder->where(function ($query) { + $query->whereNull('status')->orWhere('status', 'active'); + }); + + return; + } + + $this->builder->where('status', $status); + } + + public function timezone(?string $timezone) + { + $this->builder->where('timezone', $timezone); + } + + public function type(?string $type) + { + $this->builder->where('type', $type); + } + + public function ipAddress(?string $ipAddress) + { + $this->builder->whereHas('owner', fn ($query) => $query->where('ip_address', $ipAddress)); + } + + public function ownerName(?string $name) + { + $this->builder->whereHas('owner', fn ($query) => $query->searchWhere('name', $name)); + } + + public function ownerPhone(?string $phone) + { + $this->builder->whereHas('owner', fn ($query) => $query->searchWhere('phone', $phone)); } public function ownerEmail(?string $email) @@ -121,4 +165,24 @@ public function createdAt(?string $date) $this->builder->whereDate('created_at', $date); } + + public function createdAtBetween(?string $from, ?string $to) + { + $this->dateRange('created_at', $from, $to); + } + + public function updatedAtBetween(?string $from, ?string $to) + { + $this->dateRange('updated_at', $from, $to); + } + + private function dateRange(string $column, ?string $from, ?string $to): void + { + if ($from) { + $this->builder->whereDate($column, '>=', $from); + } + if ($to) { + $this->builder->whereDate($column, '<=', $to); + } + } } diff --git a/src/Http/Resources/Organization.php b/src/Http/Resources/Organization.php index b5a095a9..1960a49c 100644 --- a/src/Http/Resources/Organization.php +++ b/src/Http/Resources/Organization.php @@ -37,8 +37,9 @@ public function toArray($request) 'name' => $this->name, 'description' => $this->description, 'phone' => $this->phone, + 'website_url' => $this->when(Http::isInternalRequest(), $this->website_url), 'type' => $this->when(Http::isInternalRequest(), $this->type), - 'users_count' => $this->when(Http::isInternalRequest(), $this->companyUsers()->count()), + 'users_count' => $this->when(Http::isInternalRequest(), fn () => $this->resource->users_count ?? $this->users()->count()), 'timezone' => $this->timezone, 'country' => $this->country, 'currency' => $this->currency, diff --git a/src/Http/Resources/User.php b/src/Http/Resources/User.php index f01b108d..0b70e4cd 100644 --- a/src/Http/Resources/User.php +++ b/src/Http/Resources/User.php @@ -60,6 +60,10 @@ public function toArray($request) 'created_at' => $this->created_at, ]; + if (Http::isInternalRequest() && $request->user()?->isAdmin()) { + $data = array_merge($data, $this->resource->getAttribute('admin_authentication') ?? []); + } + return ResourceTransformerRegistry::transform($this->resource, $data); } diff --git a/src/Models/Company.php b/src/Models/Company.php index 0e202cce..882bfcb6 100644 --- a/src/Models/Company.php +++ b/src/Models/Company.php @@ -186,7 +186,7 @@ public function users(): BelongsToMany 'user_uuid', 'uuid', 'uuid' - ); + )->wherePivotNull('deleted_at'); } public function companyUsers(): HasManyThrough diff --git a/src/Support/OrganizationAdminSummary.php b/src/Support/OrganizationAdminSummary.php new file mode 100644 index 00000000..a0d16c70 --- /dev/null +++ b/src/Support/OrganizationAdminSummary.php @@ -0,0 +1,71 @@ +getConnection(); + $schema = $connection->getSchemaBuilder(); + $usage = ['users_count' => $company->users()->count()]; + + foreach ([ + 'drivers_count' => 'drivers', + 'customers_count' => 'contacts', + 'orders_count' => 'orders', + 'api_requests_count' => 'api_request_logs', + 'webhook_callbacks_count' => 'webhook_request_logs', + ] as $key => $table) { + // Fleet-Ops is optional. An unavailable module is distinct from zero records. + if (!$schema->hasTable($table)) { + $usage[$key] = null; + continue; + } + + $query = $connection->table($table)->where('company_uuid', $company->uuid); + if ($schema->hasColumn($table, 'deleted_at')) { + $query->whereNull('deleted_at'); + } + if ($table === 'contacts') { + $query->where('type', 'customer'); + } + $usage[$key] = $query->count(); + } + + return $usage; + } + + /** + * Attach only non-secret authentication facts, using two queries for the entire page. + * Reading the admin view must not create default 2FA settings for other users. + */ + public static function attachAuthentication(Collection $users): void + { + if ($users->isEmpty()) { + return; + } + + $keys = $users->map(fn ($user) => 'user.' . $user->uuid . '.2fa')->all(); + $settings = Setting::whereIn('key', $keys)->get(['key', 'value'])->keyBy('key'); + $identities = OAuthIdentity::whereIn('user_uuid', $users->pluck('uuid'))->get(['user_uuid', 'provider'])->groupBy('user_uuid'); + + foreach ($users as $user) { + $settingsForUser = $settings->get('user.' . $user->uuid . '.2fa'); + $enabled = filter_var(data_get($settingsForUser, 'value.enabled', false), FILTER_VALIDATE_BOOLEAN); + $user->setAttribute('admin_authentication', [ + 'two_factor_enabled' => $enabled, + 'two_factor_method' => $enabled ? data_get($settingsForUser, 'value.method', 'email') : null, + 'oauth_providers' => $identities->get($user->uuid, collect())->pluck('provider')->unique()->sort()->values()->all(), + ]); + } + } +} diff --git a/src/routes.php b/src/routes.php index a9ed3443..e60690fc 100644 --- a/src/routes.php +++ b/src/routes.php @@ -287,6 +287,7 @@ function ($router, $controller) { $router->post('leave', $controller('leaveOrganization')); $router->match(['get', 'post'], 'export', $controller('export')); $router->get('{id}/extensions', $controller('extensions')); + $router->get('{id}/usage', $controller('usage')); $router->patch('{id}/status', $controller('setAdminStatus')); $router->patch('{id}/onboarding', $controller('setAdminOnboarding')); $router->post('{id}/transfer-ownership', $controller('transferOwnershipAdmin')); diff --git a/tests/Unit/Http/CompanyControllerTest.php b/tests/Unit/Http/CompanyControllerTest.php index f0978dbf..a28240d1 100644 --- a/tests/Unit/Http/CompanyControllerTest.php +++ b/tests/Unit/Http/CompanyControllerTest.php @@ -461,6 +461,7 @@ function company_controller_fixtures(): Capsule $table->string('timezone')->nullable(); $table->string('country')->nullable(); $table->string('currency')->nullable(); + $table->string('website_url')->nullable(); $table->timestamp('onboarding_completed_at')->nullable(); $table->string('onboarding_completed_by_uuid')->nullable(); $table->timestamp('deleted_at')->nullable(); @@ -495,6 +496,13 @@ function company_controller_fixtures(): Capsule $table->string('key')->nullable()->index(); $table->text('value')->nullable(); }); + $schema->create('oauth_identities', function ($table) { + $table->string('uuid')->primary(); + $table->string('user_uuid')->index(); + $table->string('provider'); + $table->string('provider_user_id')->nullable(); + $table->text('meta')->nullable(); + }); $schema->create('invites', function ($table) { $table->string('uuid')->primary(); $table->string('public_id')->nullable(); @@ -1502,3 +1510,89 @@ function company_controller_bind_activity(): CompanyControllerActivityFake ->and($missing->getStatusCode())->toBe(422) ->and(json_decode($capsule->getConnection('mysql')->table('settings')->where('key', 'company.company-1.auth')->value('value'), true))->toBe(['allow_users_change_password' => false]); }); + +it('allows platform administrators to open a foreign organization and returns scoped usage', function () { + $database = company_controller_fixtures()->getConnection('mysql'); + $request = company_controller_admin_request('GET', [], company_controller_user('admin-1')); + $controller = company_controller(); + $found = $controller->findRecord($request, 'company_public_2'); + expect($found['company']->resource->uuid)->toBe('company-2'); + + foreach (['drivers', 'contacts', 'orders', 'api_request_logs', 'webhook_request_logs'] as $tableName) { + $database->getSchemaBuilder()->create($tableName, function ($table) use ($tableName) { + $table->string('company_uuid'); + $table->string('type')->nullable(); + if ($tableName !== 'api_request_logs') { + $table->softDeletes(); + } + }); + $database->table($tableName)->insert([ + ['company_uuid' => 'company-1', 'type' => 'customer'], + ['company_uuid' => 'company-2', 'type' => 'customer'], + ['company_uuid' => 'company-2', 'type' => 'customer'], + ]); + if ($tableName !== 'api_request_logs') { + $database->table($tableName)->insert(['company_uuid' => 'company-2', 'type' => 'customer', 'deleted_at' => '2026-09-01']); + } + } + $database->table('contacts')->insert(['company_uuid' => 'company-2', 'type' => 'contact']); + $database->table('company_users')->insert(['uuid' => 'removed', 'company_uuid' => 'company-2', 'user_uuid' => 'owner-1', 'deleted_at' => '2026-09-01']); + + expect($controller->usage('company_public_2', $request)->getData(true)['usage'])->toBe([ + 'users_count' => 1, + 'drivers_count' => 2, + 'customers_count' => 2, + 'orders_count' => 2, + 'api_requests_count' => 2, + 'webhook_callbacks_count' => 2, + ])->and($controller->usage('missing', $request)->getStatusCode())->toBe(404); + + $database->getSchemaBuilder()->drop('drivers'); + expect($controller->usage('company_public_2', $request)->getData(true)['usage']['drivers_count'])->toBeNull(); +}); + +it('returns non-secret authentication metadata only to organization platform administrators', function () { + $database = company_controller_fixtures()->getConnection('mysql'); + $database->table('settings')->insert([ + 'key' => 'user.owner-1.2fa', + 'value' => json_encode(['enabled' => true, 'method' => 'totp', 'secret' => 'must-not-leak', 'recovery_codes' => ['private']]), + ]); + $database->table('oauth_identities')->insert([ + ['uuid' => 'oauth-1', 'user_uuid' => 'owner-1', 'provider' => 'google', 'provider_user_id' => 'private-google-id', 'meta' => '{"access_token":"private"}'], + ['uuid' => 'oauth-2', 'user_uuid' => 'owner-1', 'provider' => 'github', 'provider_user_id' => 'private-github-id', 'meta' => '{}'], + ['uuid' => 'oauth-3', 'user_uuid' => 'foreign-1', 'provider' => 'microsoft', 'provider_user_id' => 'private-foreign-id', 'meta' => '{}'], + ]); + $controller = company_controller(); + $request = company_controller_request('GET', [], company_controller_user('admin-1')); + $users = $controller->users('company_public_1', $request)->resolve($request); + $owner = collect($users)->firstWhere('uuid', 'owner-1'); + $member = collect($users)->firstWhere('uuid', 'member-1'); + expect($owner)->toMatchArray(['two_factor_enabled' => true, 'two_factor_method' => 'totp', 'oauth_providers' => ['github', 'google']]) + ->and($member)->toMatchArray(['two_factor_enabled' => false, 'two_factor_method' => null, 'oauth_providers' => []]) + ->and(json_encode($users))->not->toContain('must-not-leak', 'recovery_codes', 'access_token', 'provider_user_id', 'microsoft') + ->and($database->table('settings')->where('key', 'user.member-1.2fa')->exists())->toBeFalse(); + + $paginated = $controller->users('company_public_1', company_controller_request('GET', ['paginate' => true], company_controller_user('admin-1')))->getData(true); + expect(collect($paginated['users'])->firstWhere('uuid', 'owner-1')['oauth_providers'])->toBe(['github', 'google']); + + $request = company_controller_request('GET', [], company_controller_user('owner-1')); + $regularUsers = $controller->users('company_public_1', $request)->resolve($request); + expect(collect($regularUsers)->firstWhere('uuid', 'owner-1'))->not->toHaveKey('two_factor_enabled'); + $database->table('company_users')->where('company_uuid', 'company-2')->delete(); + $request = company_controller_request('GET', [], company_controller_user('admin-1')); + expect($controller->users('company_public_2', $request)->resolve($request))->toBe([]); +}); + +it('sorts organizations by current membership count without counting removed users', function () { + $database = company_controller_fixtures()->getConnection('mysql'); + $database->table('companies')->where('uuid', 'company-2')->update(['website_url' => 'https://organization.example.test']); + $database->table('company_users')->insert([ + 'uuid' => 'removed-count', 'company_uuid' => 'company-2', 'user_uuid' => 'owner-1', 'deleted_at' => '2026-09-01', + ]); + $companies = Company::whereIn('uuid', ['company-1', 'company-2'])->withCount('users')->orderBy('users_count', 'desc')->get(); + expect($companies->pluck('uuid')->all())->toBe(['company-1', 'company-2']) + ->and($companies->pluck('users_count')->all())->toBe([2, 1]); + $request = company_controller_request('GET', [], company_controller_user('admin-1')); + $resource = new Fleetbase\Http\Resources\Organization($companies->last()); + expect($resource->resolve($request))->toMatchArray(['users_count' => 1, 'website_url' => 'https://organization.example.test']); +}); diff --git a/tests/Unit/Http/ConcreteFilterContractsTest.php b/tests/Unit/Http/ConcreteFilterContractsTest.php index 9256868f..96ab6ab3 100644 --- a/tests/Unit/Http/ConcreteFilterContractsTest.php +++ b/tests/Unit/Http/ConcreteFilterContractsTest.php @@ -208,6 +208,12 @@ function concrete_filter_database(): Capsule $table->string('public_id')->nullable(); $table->string('name')->nullable(); $table->string('owner_uuid')->nullable(); + $table->string('description')->nullable(); + $table->string('phone')->nullable(); + $table->string('website_url')->nullable(); + $table->string('slug')->nullable(); + $table->string('timezone')->nullable(); + $table->string('type')->nullable(); $table->string('country')->nullable(); $table->string('status')->nullable(); $table->string('plan')->nullable(); @@ -223,6 +229,7 @@ function concrete_filter_database(): Capsule $table->string('email')->nullable(); $table->string('phone')->nullable(); $table->string('type')->nullable(); + $table->string('ip_address')->nullable(); $table->softDeletes(); $table->timestamps(); }); @@ -1205,3 +1212,37 @@ public function isAdmin(): bool ->and(concrete_filter_uuids(UserFilter::class, User::class, ['country' => 'MN'], 'int/v1/users'))->toBe(['user-unverified']) ->and(concrete_filter_uuids(UserFilter::class, User::class, ['timezone' => 'Asia/Singapore'], 'int/v1/users'))->toBe(['user-verified']); }); + +it('searches organization identity and owner details without escaping the tenant scope', function () { + $database = concrete_filter_database()->getConnection('mysql'); + $database->table('users')->insert([ + ['uuid' => 'user-1', 'name' => 'Alice Owner', 'email' => 'alice@example.test', 'phone' => '+14155550123', 'ip_address' => '198.51.100.1'], + ['uuid' => 'other', 'name' => 'Other Owner', 'email' => 'other@foreign.test', 'phone' => '+441234', 'ip_address' => '198.51.100.2'], + ]); + $database->table('companies')->insert([ + ['uuid' => 'owned', 'owner_uuid' => 'user-1', 'name' => 'Fleet Example', 'public_id' => 'company_example', 'phone' => '+14155550199', 'description' => 'Regional delivery', 'website_url' => 'https://example.test', 'slug' => 'fleet-example', 'country' => 'US', 'timezone' => 'America/New_York', 'type' => 'logistics', 'status' => null, 'created_at' => '2026-09-01 23:59:59', 'updated_at' => '2026-09-28 23:59:59'], + ['uuid' => 'foreign', 'owner_uuid' => 'other', 'name' => 'Foreign', 'public_id' => 'company_foreign', 'phone' => null, 'description' => 'Regional delivery', 'website_url' => null, 'slug' => null, 'country' => 'GB', 'timezone' => 'Europe/London', 'type' => 'retail', 'status' => 'suspended', 'created_at' => '2026-08-01 00:00:00', 'updated_at' => '2026-08-02 00:00:00'], + ]); + + foreach (['Alice', 'alice@example.test', '4155550123', '198.51.100.1', 'company_example', '4155550199', 'example.test', 'fleet-example', 'America/New_York'] as $query) { + expect(concrete_filter_admin_uuids(CompanyFilter::class, Company::class, ['query' => $query]))->toBe(['owned']); + } + foreach ([ + ['owner_name' => 'Alice'], ['owner_email' => 'alice@'], ['owner_phone' => '4155550123'], + ['ip_address' => '198.51.100.1'], ['country' => 'us'], ['timezone' => 'America/New_York'], + ['type' => 'logistics'], ['status' => 'active'], + ['created_at_after' => '2026-09-01', 'created_at_before' => '2026-09-01'], + ['updated_at_after' => '2026-09-28'], ['updated_at_before' => '2026-09-28', 'country' => 'US'], + ] as $filters) { + expect(concrete_filter_admin_uuids(CompanyFilter::class, Company::class, $filters))->toBe(['owned']); + } + expect(concrete_filter_admin_uuids(CompanyFilter::class, Company::class, ['created_at_before' => '2026-08-31']))->toBe(['foreign']); + $request = concrete_filter_request(['query' => 'Regional']); + $request->setUserResolver(fn () => new class { + public function isAdmin(): bool + { + return false; + } + }); + expect((new CompanyFilter($request))->apply(Company::query())->pluck('uuid')->all())->toBe(['owned']); +}); From ad39f60307734ac8be83d35b0700881eeec884ce Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 28 Sep 2026 17:08:54 +0800 Subject: [PATCH 10/10] fix: restore core authentication CI coverage --- .github/workflows/ci.yml | 3 + RELEASE.md | 1 + tests/Unit/Http/CompanyControllerTest.php | 14 +++++ .../Unit/Http/ConcreteFilterContractsTest.php | 4 +- tests/Unit/Http/UserControllerTest.php | 63 ++++++++++++++++++- tests/Unit/Support/TwoFactorAuthTest.php | 10 +++ 6 files changed, 91 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73dd8d44..eb6ecd72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,9 @@ jobs: run: composer test:unit - name: Generate Coverage Baseline + timeout-minutes: 30 + env: + COMPOSER_PROCESS_TIMEOUT: '0' run: composer coverage:baseline - name: Upload Coverage Baseline diff --git a/RELEASE.md b/RELEASE.md index 0ef370f9..43a03b15 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -2,6 +2,7 @@ ## Improvements +- Organization administration supports company and owner identity search, country/timezone/owner-IP and registration/update-date filters, and sorting by current user count. Admins can open organizations outside their own memberships, inspect organization-scoped usage totals, and see members' 2FA methods and linked OAuth providers without exposing authentication secrets. - Sign in with an authenticator app (TOTP, RFC 6238), next to email and SMS 2FA (#163). It works with Authy, Google Authenticator, Microsoft Authenticator and 1Password. - New `users/two-fa/authenticator` endpoints to set up, confirm, disable and inspect the app. Setup, disable and regenerating recovery codes need the current password. - Confirming the app returns 8 single-use recovery codes. `two-fa/verify` accepts an app code (±1 step for clock drift, each code once) or a recovery code. diff --git a/tests/Unit/Http/CompanyControllerTest.php b/tests/Unit/Http/CompanyControllerTest.php index a28240d1..74b65700 100644 --- a/tests/Unit/Http/CompanyControllerTest.php +++ b/tests/Unit/Http/CompanyControllerTest.php @@ -1596,3 +1596,17 @@ function company_controller_bind_activity(): CompanyControllerActivityFake $resource = new Fleetbase\Http\Resources\Organization($companies->last()); expect($resource->resolve($request))->toMatchArray(['users_count' => 1, 'website_url' => 'https://organization.example.test']); }); + +it('rejects missing sessions before reading or changing organization authentication settings', function () { + company_controller_fixtures(); + session()->remove('company'); + $controller = company_controller(); + expect($controller->getAuthSettings()->getStatusCode())->toBe(401) + ->and($controller->saveAuthSettings(company_controller_request('POST'))->getStatusCode())->toBe(401) + ->and($controller->updateRecord(company_controller_request('PATCH'), 'company_public_1')->getStatusCode())->toBe(404); + + session(['company' => 'company-1']); + session()->remove('user'); + $middleware = collect($controller->getMiddleware())->first(fn ($entry) => ($entry['options']['only'] ?? null) === ['updateRecord', 'saveTwoFactorSettings'])['middleware']; + expect($middleware(company_controller_request('PATCH'), fn () => 'allowed')->getStatusCode())->toBe(401); +}); diff --git a/tests/Unit/Http/ConcreteFilterContractsTest.php b/tests/Unit/Http/ConcreteFilterContractsTest.php index 96ab6ab3..72f85db6 100644 --- a/tests/Unit/Http/ConcreteFilterContractsTest.php +++ b/tests/Unit/Http/ConcreteFilterContractsTest.php @@ -1220,7 +1220,7 @@ public function isAdmin(): bool ['uuid' => 'other', 'name' => 'Other Owner', 'email' => 'other@foreign.test', 'phone' => '+441234', 'ip_address' => '198.51.100.2'], ]); $database->table('companies')->insert([ - ['uuid' => 'owned', 'owner_uuid' => 'user-1', 'name' => 'Fleet Example', 'public_id' => 'company_example', 'phone' => '+14155550199', 'description' => 'Regional delivery', 'website_url' => 'https://example.test', 'slug' => 'fleet-example', 'country' => 'US', 'timezone' => 'America/New_York', 'type' => 'logistics', 'status' => null, 'created_at' => '2026-09-01 23:59:59', 'updated_at' => '2026-09-28 23:59:59'], + ['uuid' => 'owned', 'owner_uuid' => 'user-1', 'name' => 'Fleet Example', 'public_id' => 'company_example', 'phone' => '+14155550199', 'description' => 'Regional delivery', 'website_url' => 'https://example.test', 'slug' => 'fleet-example', 'country' => 'US', 'timezone' => 'America/New_York', 'type' => 'logistics', 'status' => null, 'created_at' => '2026-09-01 23:59:59', 'updated_at' => '2026-09-27 23:59:59'], ['uuid' => 'foreign', 'owner_uuid' => 'other', 'name' => 'Foreign', 'public_id' => 'company_foreign', 'phone' => null, 'description' => 'Regional delivery', 'website_url' => null, 'slug' => null, 'country' => 'GB', 'timezone' => 'Europe/London', 'type' => 'retail', 'status' => 'suspended', 'created_at' => '2026-08-01 00:00:00', 'updated_at' => '2026-08-02 00:00:00'], ]); @@ -1232,7 +1232,7 @@ public function isAdmin(): bool ['ip_address' => '198.51.100.1'], ['country' => 'us'], ['timezone' => 'America/New_York'], ['type' => 'logistics'], ['status' => 'active'], ['created_at_after' => '2026-09-01', 'created_at_before' => '2026-09-01'], - ['updated_at_after' => '2026-09-28'], ['updated_at_before' => '2026-09-28', 'country' => 'US'], + ['updated_at_after' => '2026-09-27'], ['updated_at_before' => '2026-09-27', 'country' => 'US'], ] as $filters) { expect(concrete_filter_admin_uuids(CompanyFilter::class, Company::class, $filters))->toBe(['owned']); } diff --git a/tests/Unit/Http/UserControllerTest.php b/tests/Unit/Http/UserControllerTest.php index 2413ee4a..44fce37e 100644 --- a/tests/Unit/Http/UserControllerTest.php +++ b/tests/Unit/Http/UserControllerTest.php @@ -2448,10 +2448,18 @@ function user_controller_grant_permission(Capsule $capsule, string $companyUserU $right = $validate(['current_password' => 'old-password']); $none = $validate([]); - expect($wrong->fails())->toBeTrue() + $missing = new Illuminate\Validation\Validator($translator, [], $rules, $request->messages()); + + expect($request->authorize())->toBeTrue() + ->and($missing->fails())->toBeTrue() + ->and($missing->errors()->first('current_password'))->toBe('The current password is required.') + ->and($request->messages()['password.uncompromised'])->toContain('data breach') + ->and($wrong->fails())->toBeTrue() ->and($wrong->errors()->first('current_password'))->toBe('The current password provided is invalid.') ->and($right->fails())->toBeFalse() ->and($none->fails())->toBeTrue(); +}); + test('user controller authenticator app endpoints skip the generic resource permission check', function (string $method) { user_controller_database(); @@ -2509,10 +2517,61 @@ function user_controller_grant_permission(Capsule $capsule, string $companyUserU expect($codesDenied->getStatusCode())->toBe(422) ->and($codes->getData(true)['recovery_codes'])->toHaveCount(8) ->and($denied->getStatusCode())->toBe(422) - ->and(TestActivityLogger::$logged)->not->toBeEmpty() + ->and(array_column(UserControllerActivityLoggerFake::$logged, 'event'))->toBe(['authenticator_enabled', 'recovery_codes_regenerated', 'authenticator_disabled']) ->and($disabled->getData(true))->toBe([ 'status' => ['enabled' => false, 'confirmed_at' => null, 'recovery_codes_remaining' => 0], 'settings' => ['enabled' => false, 'method' => 'email'], ]) ->and($noApp->getStatusCode())->toBe(422); }); + +it('enforces metrics and system two-factor permissions before reaching their handlers', function (string $controllerClass, ?string $permission) { + $database = user_controller_database(); + $controller = new $controllerClass(); + $middleware = collect($controller->getMiddleware())->first(fn ($entry) => $entry['middleware'] instanceof Closure)['middleware']; + $run = function (?string $actor) use ($middleware) { + session()->remove('user'); + if ($actor !== null) { + session(['user' => $actor]); + } + $request = user_controller_request('GET', [], $actor ? user_controller_user($actor) : null); + + return $middleware($request, fn () => 'allowed'); + }; + + expect($run(null)->getStatusCode())->toBe(401) + ->and($run('member-1')->getStatusCode())->toBe(401) + ->and($run('admin-1'))->toBe('allowed'); + + if ($permission !== null) { + user_controller_grant_permission($database, 'pivot-member-1', $permission); + expect($run('member-1'))->toBe('allowed'); + } +})->with([ + 'platform metrics' => [Fleetbase\Http\Controllers\Internal\v1\AdminMetricsController::class, null], + 'developer metrics' => [Fleetbase\Http\Controllers\Internal\v1\DeveloperMetricsController::class, 'developers list api-key'], + 'IAM metrics' => [Fleetbase\Http\Controllers\Internal\v1\IamMetricsController::class, 'iam list user'], + 'system two-factor policy' => [Fleetbase\Http\Controllers\Internal\v1\TwoFaController::class, null], +]); + +it('allows report execution and export only after the matching permission check', function (string $action, string $method) { + $database = user_controller_database(); + $controller = new Fleetbase\Http\Controllers\Internal\v1\ReportController(); + $middleware = collect($controller->getMiddleware())->first(fn ($entry) => in_array($method, $entry['options']['only'] ?? [], true))['middleware']; + $run = function (string $actor) use ($middleware, $method) { + session(['user' => $actor]); + $request = user_controller_request('POST', [], user_controller_user($actor), $method); + + return $middleware($request, fn () => 'allowed'); + }; + + expect($run('member-1')->getStatusCode())->toBe(401) + ->and($run('admin-1'))->toBe('allowed'); + user_controller_grant_permission($database, 'pivot-member-1', 'iam ' . $action . ' report'); + expect($run('member-1'))->toBe('allowed'); +})->with([['execute', 'executeQuery'], ['export', 'exportQuery'], ['export', 'download']]); + +it('uses the API key schema name when authorizing credential operations', function () { + user_controller_database(); + expect((new Fleetbase\Http\Controllers\Internal\v1\ApiCredentialController())->getPermissionResourceName())->toBe('api-key'); +}); diff --git a/tests/Unit/Support/TwoFactorAuthTest.php b/tests/Unit/Support/TwoFactorAuthTest.php index ebce0c8c..cb64d7a7 100644 --- a/tests/Unit/Support/TwoFactorAuthTest.php +++ b/tests/Unit/Support/TwoFactorAuthTest.php @@ -872,3 +872,13 @@ function two_factor_auth_authenticator_record(User $user): array expect(fn () => TwoFactorAuth::regenerateRecoveryCodes($user))->toThrow(Exception::class, 'Set up an authenticator app first.'); }); + +test('two factor auth rejects a pending authenticator challenge after its user is removed', function () { + [$user] = two_factor_auth_fixtures(); + two_factor_auth_enroll_authenticator($user); + $token = TwoFactorAuth::start($user); + $clientToken = TwoFactorAuth::getClientSessionTokenFromTwoFaSession($token, $user->email); + app('db')->table('users')->where('uuid', $user->uuid)->delete(); + + expect(fn () => TwoFactorAuth::verifyCode('123456', $token, $clientToken))->toThrow(Exception::class, 'Verification code is invalid.'); +});