From b40795059f61f07a78406b4e98af75869a1c1d84 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 13 Oct 2026 18:25:52 +0800 Subject: [PATCH] feat(auth): hashed single-use verification codes, no stub users before verification Phase 1 of the SMS guard on the FleetOps side (needs core-api with the hashed-code APIs, fleetbase/core-api#307): - The customer API no longer persists a User for an identity that merely asked for a creation code; the code's subject is the existing user or an unsaved one. A users row per unverified identity was a way to fill the table from a leaked credential. - Every SMS code the driver and customer APIs send is issued hashed (HMAC on the row, 10 minute expiry, plain code only in the message). - One verify-and-consume path: create, verify-code, and the password resets check the submitted code against the live codes for the identity or user, newest first, lock a code after five wrong guesses, and mark the matching one used. Plaintext rows written before this verify the same way. - The public tracking page consumes a verified code instead of deleting it, so the row stays for the ledger. --- .../Controllers/Api/v1/CustomerController.php | 79 +++++++++++++++---- .../Controllers/Api/v1/DriverController.php | 51 +++++++++--- .../Support/TrackingPage/TrackingAccess.php | 20 ++++- .../ApiCustomerControllerContractsTest.php | 10 +-- .../tests/ApiDriverPasswordContractsTest.php | 19 +---- .../Api/CustomerControllerHelperSeamsTest.php | 24 ++++++ .../TrackingPage/TrackingAccessTest.php | 4 +- 7 files changed, 155 insertions(+), 52 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/CustomerController.php b/server/src/Http/Controllers/Api/v1/CustomerController.php index 204dcd85b..108f11482 100644 --- a/server/src/Http/Controllers/Api/v1/CustomerController.php +++ b/server/src/Http/Controllers/Api/v1/CustomerController.php @@ -46,6 +46,11 @@ class CustomerController extends Controller public const DEACTIVATED_LOGIN_MESSAGE = 'This customer login has been deactivated.'; + /** + * Wrong guesses a single code tolerates before it locks itself (core's check()). + */ + public const CODE_CHECK_ATTEMPTS = 5; + /* ============================================================ | Public auth flows (API credential only, no Customer-Token) * ============================================================ */ @@ -80,23 +85,21 @@ public function requestCreationCode(VerifyCreateCustomerRequest $request) $providedName = trim((string) $request->input('name', '')); $providedPhone = $request->filled('phone') ? static::phone($request->input('phone')) : null; - // The verification code needs a persisted subject so the mail renderer - // can resolve the polymorphic `subject` relation (the verification blade - // template references `$user->name`). Look up — or create — the User - // before sending. `create()` later backfills password + remaining fields - // on this same row when the customer confirms the code. + // The code's subject is the person signing up: an existing user (a prior + // incomplete signup, or an account in another organization) as it stands, else + // an unsaved User carrying the name and contact details the message needs. + // Nothing is persisted until create() verifies the code — a users row per + // unverified identity was a way to fill the table from a leaked credential. $subject = $this->findActiveUserByIdentity($identity, $isEmail ? 'email' : 'phone'); if (!$subject) { - // `password` and `type` are guarded on User; assign type after create - // via setUserType (saves the row). - $subject = $this->createUser([ + $subject = new User(); + $subject->forceFill([ 'company_uuid' => $sessionCompany, 'name' => $providedName !== '' ? $providedName : $identity, 'email' => $isEmail ? $identity : null, 'phone' => $isEmail ? $providedPhone : $identity, ]); - $subject->setUserType('customer'); } elseif ($providedName !== '' && (!$subject->name || $subject->name === $subject->email)) { // Existing stub user from a prior incomplete signup — refresh the // greeting name if the client supplied one. @@ -118,6 +121,7 @@ public function requestCreationCode(VerifyCreateCustomerRequest $request) ]); } else { $this->generateSmsVerification($subject, 'fleetops_create_customer', [ + 'hashed' => true, 'messageCallback' => fn ($verification) => 'Your ' . config('app.name') . ' verification code is ' . $verification->code, 'meta' => $meta, ]); @@ -393,6 +397,7 @@ public function loginWithPhone(Request $request) try { $this->generateSmsVerification($user, 'fleetops_customer_login', [ + 'hashed' => true, 'messageCallback' => fn ($verification) => 'Your ' . config('app.name') . ' verification code is ' . $verification->code, ]); @@ -494,6 +499,7 @@ public function forgotPassword(Request $request) ]); } else { $this->generateSmsVerification($user, 'fleetops_customer_password_reset', [ + 'hashed' => true, 'messageCallback' => fn ($v) => 'Your ' . config('app.name') . ' password reset code is ' . $v->code, 'meta' => $meta, ]); @@ -542,12 +548,9 @@ public function resetPassword(Request $request) // setPasswordAttribute mutator hashes plaintext; don't pre-hash here. $user->password = $password; $user->save(); - // Invalidate all existing sessions for this user after a password reset. + // Invalidate all existing sessions for this user after a password reset. The + // code itself was used up when it verified (findVerificationCode). $this->deleteUserTokens($user); - // Null on the testing-bypass path — there is no row to consume. - if ($verificationCode) { - $verificationCode->delete(); - } return response()->json(['status' => 'ok']); } @@ -978,14 +981,58 @@ protected function verificationBypassMatches(?string $identity, ?string $code): ); } + /** + * Whether the submitted code verifies against a live code for the purpose and + * subject or identity in `$attributes`, using the code up when it does. + */ protected function verificationCodeExists(array $attributes): bool { - return VerificationCode::where($attributes)->exists(); + return $this->findVerificationCode($attributes) !== null; } + /** + * The live code the submitted code verified against, used up so it cannot verify + * again, or null. `$attributes` carries `code`, `for` and either `subject_uuid` or + * `meta->identity`. Live codes are checked newest first; a wrong guess counts against + * each until it locks (core's check()). Plaintext rows written before codes were hashed + * verify the same way. + */ protected function findVerificationCode(array $attributes): mixed { - return VerificationCode::where($attributes)->first(); + $code = (string) ($attributes['code'] ?? ''); + + foreach ($this->liveVerificationCodes($attributes) as $verificationCode) { + if ($verificationCode->check($code, static::CODE_CHECK_ATTEMPTS) === VerificationCode::CHECK_VALID) { + $verificationCode->consume(); + + return $verificationCode; + } + } + + return null; + } + + /** + * Unexpired, unused, unlocked codes for the purpose and subject or identity in + * `$attributes`, newest first. + * + * @return \Illuminate\Support\Collection + */ + protected function liveVerificationCodes(array $attributes) + { + $query = VerificationCode::query() + ->where('for', (string) ($attributes['for'] ?? '')) + ->where(function ($query) { + $query->whereNull('status')->orWhereNotIn('status', [VerificationCode::STATUS_USED, VerificationCode::STATUS_LOCKED]); + }); + + foreach (['subject_uuid', 'meta->identity'] as $column) { + if (array_key_exists($column, $attributes)) { + $query->where($column, $attributes[$column]); + } + } + + return $query->orderByDesc('created_at')->get(); } protected function findFileByPublicId(string $publicId): mixed diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index 124373721..7171cf6b4 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -56,6 +56,11 @@ class DriverController extends Controller */ public const DEACTIVATED_LOGIN_MESSAGE = 'This driver login has been deactivated.'; + /** + * Wrong guesses a single code tolerates before it locks itself (core's check()). + */ + public const CODE_CHECK_ATTEMPTS = 5; + /** * Public expansion name => Eloquent relation name. * @@ -588,6 +593,7 @@ public function loginWithPhone() // generate verification token try { VerificationCode::generateSmsVerificationFor($user, 'driver_login', [ + 'hashed' => true, 'company_uuid' => $company->uuid, 'messageCallback' => function ($verification) use ($company) { return 'Your ' . data_get($company, 'name', config('app.name')) . ' verification code is ' . $verification->code; @@ -652,8 +658,8 @@ public function verifyCode(Request $request) return response()->apiError(static::DEACTIVATED_LOGIN_MESSAGE, 403); } - // find and verify code - $verificationCode = VerificationCode::where(['subject_uuid' => $user->uuid, 'code' => $code, 'for' => $for])->exists(); + // find and verify code: the user's newest live code for this purpose, used up + $verificationCode = static::spendCode($user, $for, (string) $code); if (!$verificationCode && !static::verificationBypassMatches($identity, $code)) { return response()->apiError('Invalid verification code!'); } @@ -1387,7 +1393,7 @@ public function resetPassword(Request $request) $user->password = $password; $user->save(); - $verification->delete(); + // The code was used up when it verified (findResetCode); every session ends. $user->tokens()->delete(); return response()->json(['status' => 'ok']); @@ -1422,14 +1428,40 @@ protected static function passwordMatches(User $user, string $plain): bool return Hash::check($plain, $user->password); } - /** The unexpired reset code for this user, if the one supplied matches. */ + /** + * The live reset code the supplied one verified against, used up so it cannot + * verify again, or null. + */ protected static function findResetCode(User $user, string $code) { - return VerificationCode::where([ - 'subject_uuid' => $user->uuid, - 'code' => $code, - 'for' => 'driver_password_reset', - ])->where('expires_at', '>', now())->first(); + return static::spendCode($user, 'driver_password_reset', $code); + } + + /** + * Verify a code against the user's live codes for a purpose and use it up. Codes are + * checked newest first; a wrong guess counts against each until it locks (core's + * check()). Plaintext rows written before codes were hashed verify the same way. + */ + protected static function spendCode(User $user, string $for, string $code): ?VerificationCode + { + $live = VerificationCode::query() + ->where('for', $for) + ->where('subject_uuid', $user->uuid) + ->where(function ($query) { + $query->whereNull('status')->orWhereNotIn('status', [VerificationCode::STATUS_USED, VerificationCode::STATUS_LOCKED]); + }) + ->orderByDesc('created_at') + ->get(); + + foreach ($live as $verificationCode) { + if ($verificationCode->check($code, static::CODE_CHECK_ATTEMPTS) === VerificationCode::CHECK_VALID) { + $verificationCode->consume(); + + return $verificationCode; + } + } + + return null; } /** Sends the reset code by whichever channel the identity names. */ @@ -1444,6 +1476,7 @@ protected static function sendResetCode(User $user, string $identity): void ]); } else { VerificationCode::generateSmsVerificationFor($user, 'driver_password_reset', [ + 'hashed' => true, 'messageCallback' => fn ($v) => 'Your ' . config('app.name') . ' password reset code is ' . $v->code, ]); } diff --git a/server/src/Support/TrackingPage/TrackingAccess.php b/server/src/Support/TrackingPage/TrackingAccess.php index f6872871a..bc2cc7dde 100644 --- a/server/src/Support/TrackingPage/TrackingAccess.php +++ b/server/src/Support/TrackingPage/TrackingAccess.php @@ -141,7 +141,7 @@ public function checkCode(TrackingScope $scope, string $code): array $result = $verificationCode->check($code, self::MAX_ATTEMPTS); if ($result === VerificationCode::CHECK_VALID) { $channel = $verificationCode->getMeta('channel'); - $this->deleteCode($verificationCode); + $this->consumeCode($verificationCode); return ['status' => $result, 'channel' => $channel]; } @@ -290,14 +290,26 @@ protected function createCode(mixed $recipient, array $options): VerificationCod return VerificationCode::issue($recipient, self::CODE_PURPOSE, $options); } + /** + * The scope's newest code that has not been used. Read without the expiry scope so an + * expired code is reported as such rather than as missing; a used code stays on the + * table for the ledger but never verifies again. + */ protected function latestCode(TrackingScope $scope): ?VerificationCode { - return VerificationCode::withoutGlobalScope(ExpiryScope::class)->where('for', self::CODE_PURPOSE)->where('meta->scope', $scope->key())->latest()->first(); + return VerificationCode::withoutGlobalScope(ExpiryScope::class) + ->where('for', self::CODE_PURPOSE) + ->where('meta->scope', $scope->key()) + ->where(function ($query) { + $query->whereNull('status')->orWhere('status', '!=', VerificationCode::STATUS_USED); + }) + ->latest() + ->first(); } - protected function deleteCode(VerificationCode $verificationCode): void + protected function consumeCode(VerificationCode $verificationCode): void { - $verificationCode->delete(); + $verificationCode->consume(); } protected function findSession(string $tokenHash): ?TrackingSession diff --git a/server/tests/ApiCustomerControllerContractsTest.php b/server/tests/ApiCustomerControllerContractsTest.php index 49131be51..1140e1460 100644 --- a/server/tests/ApiCustomerControllerContractsTest.php +++ b/server/tests/ApiCustomerControllerContractsTest.php @@ -692,12 +692,9 @@ function fleetopsApiCustomerJson($response): array ])); expect($created)->toBe(['status' => 'ok']) - ->and($controller->createdUsers[0])->toMatchArray([ - 'company_uuid' => 'company-uuid', - 'name' => 'Jane', - 'email' => 'jane@example.test', - 'phone' => '+15551234567', - ]) + // Nothing is persisted for an unverified identity: the code's subject is an unsaved user. + ->and($controller->createdUsers)->toBe([]) + ->and($controller->emailVerifications[0][0])->toBeNull() ->and($controller->emailVerifications[0][1])->toBe('fleetops_create_customer') ->and($updated)->toBe(['status' => 'ok']) ->and($stub->name)->toBe('Stub Name') @@ -844,7 +841,6 @@ function fleetopsApiCustomerJson($response): array ->and(fleetopsApiCustomerJson($reset))->toBe(['status' => 'ok']) ->and($controller->genericUser->savedForTest)->toBeTrue() ->and($controller->genericUser->tokensDeleted)->toBeTrue() - ->and($controller->verificationCode->deleted)->toBeTrue() ->and(fleetopsApiCustomerJson($unknown->forgotPassword(Request::create('/v1/customers/forgot-password', 'POST', [ 'identity' => 'missing@example.test', ]))))->toBe(['status' => 'ok']) diff --git a/server/tests/ApiDriverPasswordContractsTest.php b/server/tests/ApiDriverPasswordContractsTest.php index 8d7a44414..3716031c8 100644 --- a/server/tests/ApiDriverPasswordContractsTest.php +++ b/server/tests/ApiDriverPasswordContractsTest.php @@ -362,23 +362,13 @@ function fleetopsPasswordUser(string $plainPassword = 'correct-horse'): FleetOps ->and($unknownIdentity->getData())->toBe($badCode->getData()); }); -test('reset password sets the password, spends the code, and ends every session', function () { +test('reset password sets the password and ends every session once the code verified', function () { $user = fleetopsPasswordUser(); FleetOpsPasswordControllerProbe::$identityUser = $user; - $deleted = false; - FleetOpsPasswordControllerProbe::$resetCode = new class($deleted) { - public function __construct(public bool &$deleted) - { - } - - public function delete(): bool - { - $this->deleted = true; - - return true; - } - }; + // findResetCode() hands back the code it verified and used up; nothing more is done + // to it here. + FleetOpsPasswordControllerProbe::$resetCode = (object) ['uuid' => 'reset-code']; $response = (new FleetOpsPasswordControllerProbe())->resetPassword( new Request(['identity' => 'driver@example.test', 'code' => '123456', 'password' => 'newpassword']) @@ -386,7 +376,6 @@ public function delete(): bool expect($response->getStatusCode())->toBe(200) ->and($user->saved)->toHaveCount(1) - ->and($deleted)->toBeTrue() // A reset is a recovery from losing control; nothing keeps working. ->and($user->tokensDeleted)->toBeTrue(); }); diff --git a/server/tests/Feature/Http/Api/CustomerControllerHelperSeamsTest.php b/server/tests/Feature/Http/Api/CustomerControllerHelperSeamsTest.php index 6477520a2..bd034d946 100644 --- a/server/tests/Feature/Http/Api/CustomerControllerHelperSeamsTest.php +++ b/server/tests/Feature/Http/Api/CustomerControllerHelperSeamsTest.php @@ -829,3 +829,27 @@ protected function createFileFromBase64(string $contents, string $path): mixed expect($connection->table('contacts')->where('uuid', '88888888-8888-4888-8888-888888888820')->value('photo_uuid'))->toBe('file-b64-1'); app()->forgetInstance(CustomerAuth::APP_BINDING); }); + +test('a wrong code counts against the live code, a right one uses it up, and a used code never verifies again', function () { + $connection = fleetopsCustomerHelperBoot(); + $controller = new CustomerController(); + + $connection->table('verification_codes')->insert([ + ['uuid' => 'vc-older', 'code' => '101010', 'for' => 'fleetops_create_customer', 'meta' => json_encode(['identity' => 'spend@example.com']), 'status' => 'active', 'created_at' => '2026-10-13 09:00:00'], + ['uuid' => 'vc-newer', 'code' => '202020', 'for' => 'fleetops_create_customer', 'meta' => json_encode(['identity' => 'spend@example.com']), 'status' => 'active', 'created_at' => '2026-10-13 09:05:00'], + ]); + + $wrong = $controller->create(CreateCustomerRequest::create('/v1/customers', 'POST', ['code' => '999999', 'identity' => 'spend@example.com', 'name' => 'Spender', 'password' => 'secret'])); + expect($wrong->getData(true)['error'])->toContain('Invalid verification code') + ->and(json_decode($connection->table('verification_codes')->where('uuid', 'vc-newer')->value('meta'), true)['attempts'])->toBe(1) + ->and(json_decode($connection->table('verification_codes')->where('uuid', 'vc-older')->value('meta'), true)['attempts'])->toBe(1); + + // The older code still verifies while it is live, and is used up by doing so. + $controller->create(CreateCustomerRequest::create('/v1/customers', 'POST', ['code' => '101010', 'identity' => 'spend@example.com', 'name' => 'Spender', 'password' => 'secret'])); + expect($connection->table('contacts')->where('email', 'spend@example.com')->count())->toBe(1) + ->and($connection->table('verification_codes')->where('uuid', 'vc-older')->value('status'))->toBe('used') + ->and($connection->table('verification_codes')->where('uuid', 'vc-newer')->value('status'))->toBe('active'); + + $again = $controller->create(CreateCustomerRequest::create('/v1/customers', 'POST', ['code' => '101010', 'identity' => 'spend@example.com', 'name' => 'Spender', 'password' => 'secret'])); + expect($again->getData(true)['error'])->toContain('Invalid verification code'); +}); diff --git a/server/tests/Unit/Support/TrackingPage/TrackingAccessTest.php b/server/tests/Unit/Support/TrackingPage/TrackingAccessTest.php index 70ae31558..c3376434d 100644 --- a/server/tests/Unit/Support/TrackingPage/TrackingAccessTest.php +++ b/server/tests/Unit/Support/TrackingPage/TrackingAccessTest.php @@ -126,8 +126,10 @@ function trackingRequest(array $cookies = [], string $ip = '203.0.113.7'): Reque expect($access->checkCode($scope, $wrong))->toBe(['status' => VerificationCode::CHECK_INVALID, 'attempts_left' => 2]) ->and($access->pausedFor($scope))->toBe(0); + // A used code stays on the table for the ledger, marked used, and never verifies again. expect($access->checkCode($scope, $second->plainCode))->toBe(['status' => VerificationCode::CHECK_VALID, 'channel' => 'email']) - ->and(VerificationCode::withoutGlobalScope(ExpiryScope::class)->count())->toBe(0) + ->and(VerificationCode::withoutGlobalScope(ExpiryScope::class)->count())->toBe(1) + ->and(VerificationCode::withoutGlobalScope(ExpiryScope::class)->value('status'))->toBe(VerificationCode::STATUS_USED) ->and($access->checkCode($scope, $second->plainCode))->toBe(['status' => VerificationCode::CHECK_EXPIRED]); $third = $access->issueCode($scope, null, 'sms');