From 7296dc08c6be11ab158e0640ea22bd1581165478 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 13 Oct 2026 17:55:24 +0800 Subject: [PATCH] feat(customers): hashed single-use verification codes and guarded order alerts Every SMS code the storefront sends is issued hashed (HMAC on the row, 10 minute expiry, plain code only in the message), and create, verify-code, confirm-account-closure and verify-phone-number check the newest live code for the identity or user with core's check() and use it up with consume(), so a code verifies once and a leaked table gives nothing away. StorefrontOrderCreated sends its text through core's SmsServiceChannel so order alerts are budgeted and ledgered. Everything falls back to the old behaviour on a core without these APIs. --- .../Controllers/v1/CustomerController.php | 81 +++++++++++++++---- .../Notifications/StorefrontOrderCreated.php | 21 ++++- .../NotificationContractsTest.php | 6 +- 3 files changed, 91 insertions(+), 17 deletions(-) diff --git a/server/src/Http/Controllers/v1/CustomerController.php b/server/src/Http/Controllers/v1/CustomerController.php index 8cabd30b..edf76871 100644 --- a/server/src/Http/Controllers/v1/CustomerController.php +++ b/server/src/Http/Controllers/v1/CustomerController.php @@ -48,6 +48,11 @@ class CustomerController extends Controller public const CODE_ATTEMPT_LIMIT = 10; public const CODE_ATTEMPT_WINDOW = 900; + /** + * Wrong guesses a single code tolerates before it locks itself (core's check()). + */ + public const CODE_CHECK_ATTEMPTS = 5; + protected function authenticatedCustomerForRequest(Request $request): ?Contact { return Storefront::getCustomerFromToken(); @@ -287,6 +292,7 @@ public function requestCustomerCreationCode(VerifyCreateCustomerRequest $request ]); } else { VerificationCode::generateSmsVerificationFor($customer, 'storefront_create_customer', [ + 'hashed' => true, 'messageCallback' => function ($verification) use ($about) { return "Your {$about->name} verification code is {$verification->code}"; }, @@ -336,9 +342,8 @@ public function create(CreateCustomerRequest $request) return response()->apiError('Too many attempts. Try again later.', 429); } - // verify code - $verificationCode = VerificationCode::where(['code' => $code, 'for' => 'storefront_create_customer', 'meta->identity' => $identity])->exists(); - if (!$verificationCode) { + // verify code: the newest live code for this identity, checked and used up + if (!$this->verifyIdentityCode('storefront_create_customer', $identity, $code)) { $this->recordFailedCodeAttempt('create|' . $identity); return response()->apiError('Invalid verification code provided!'); @@ -687,6 +692,7 @@ public function loginWithPhone() try { VerificationCode::generateSmsVerificationFor($user, 'storefront_login', [ + 'hashed' => true, 'messageCallback' => $messageCallback, ]); @@ -1071,8 +1077,8 @@ public function verifyCode(Request $request) return response()->apiError('Unable to verify code.'); } - // find and verify code - $verificationCode = VerificationCode::where(['subject_uuid' => $user->uuid, 'code' => $code, 'for' => $for])->exists(); + // find and verify code: the newest live code for this user, checked and used up + $verificationCode = $this->verifySubjectCode($for, $user->uuid, $code); if (!$verificationCode && !static::isReviewAccountBypass($identity, $code)) { $this->recordFailedCodeAttempt('verify|' . $identity); @@ -1229,6 +1235,52 @@ protected static function sendFailureResponse(?\Throwable $failure, string $mess return response()->apiError($message); } + /** + * Check a code filed under an identity and use it up. The newest live code is the one + * that counts; a wrong guess counts against it until it locks. A core without + * findActiveFor() (before 1.6.70) falls back to the plain lookup it always had. + */ + protected function verifyIdentityCode(string $for, string $identity, mixed $code): bool + { + if (!method_exists(VerificationCode::class, 'findActiveFor')) { + return VerificationCode::where(['code' => $code, 'for' => $for, 'meta->identity' => $identity])->exists(); + } + + $verificationCode = VerificationCode::findActiveFor($for, $identity); + + return $verificationCode !== null && $this->checkAndConsume($verificationCode, $code); + } + + /** + * The live code for a subject once the submitted code matched it (and used it up), else null. + */ + protected function findSubjectCode(string $for, string $subjectUuid, mixed $code): ?VerificationCode + { + if (!method_exists(VerificationCode::class, 'findActiveForSubject')) { + return VerificationCode::where(['subject_uuid' => $subjectUuid, 'code' => $code, 'for' => $for])->first(); + } + + $verificationCode = VerificationCode::findActiveForSubject($for, $subjectUuid); + + return $verificationCode !== null && $this->checkAndConsume($verificationCode, $code) ? $verificationCode : null; + } + + protected function verifySubjectCode(string $for, string $subjectUuid, mixed $code): bool + { + return $this->findSubjectCode($for, $subjectUuid, $code) !== null; + } + + protected function checkAndConsume(VerificationCode $verificationCode, mixed $code): bool + { + if ($verificationCode->check((string) $code, static::CODE_CHECK_ATTEMPTS) !== VerificationCode::CHECK_VALID) { + return false; + } + + $verificationCode->consume(); + + return true; + } + protected function codeAttemptsExceeded(string $key): bool { $limiter = $this->attemptLimiter(); @@ -1379,6 +1431,7 @@ public function startAccountClosure(Request $request) if ($user->phone) { try { VerificationCode::generateSmsVerificationFor($user, 'storefront_account_closure', [ + 'hashed' => true, 'messageCallback' => $messageCallback, 'meta' => ['identity' => $identity], ]); @@ -1441,7 +1494,7 @@ public function confirmAccountClosure(Request $request) } // verify account closure code - $verificationCode = VerificationCode::where(['code' => $code, 'for' => 'storefront_account_closure', 'meta->identity' => $identity])->exists(); + $verificationCode = $this->verifyIdentityCode('storefront_account_closure', $identity, $code); if (!$verificationCode && !static::isReviewAccountBypass($identity, $code)) { $this->recordFailedCodeAttempt('closure|' . $customer->uuid); @@ -1517,6 +1570,7 @@ public function requestPhoneVerification(Request $request) try { VerificationCode::generateSmsVerificationFor($user, 'storefront_verify_phone', [ + 'hashed' => true, 'messageCallback' => function ($verification) use ($about) { return "Your {$about->name} verification code is {$verification->code}"; }, @@ -1574,12 +1628,8 @@ public function verifyPhoneNumber(Request $request) return response()->apiError('Too many attempts. Try again later.', 429); } - // Find the verification code - $verificationCode = VerificationCode::where([ - 'subject_uuid' => $user->uuid, - 'code' => $code, - 'for' => 'storefront_verify_phone', - ])->first(); + // Find the verification code: the newest live one for this user, checked and used up + $verificationCode = $this->findSubjectCode('storefront_verify_phone', $user->uuid, $code); // The bypass leaves no code row to read the phone back from, so it comes from the // request — which is what the caller is asking to verify in the first place. @@ -1612,8 +1662,11 @@ public function verifyPhoneNumber(Request $request) $user->update(['phone' => $phone, 'phone_verified_at' => now()]); $customer->update(['phone' => $phone]); - // Invalidate the verification code - $verificationCode->delete(); + // The code was used up by findSubjectCode(); older cores without consume() still + // need the row removed so it cannot verify twice. + if (!method_exists($verificationCode, 'consume')) { + $verificationCode->delete(); + } return new Customer($customer->fresh()); } diff --git a/server/src/Notifications/StorefrontOrderCreated.php b/server/src/Notifications/StorefrontOrderCreated.php index 1dd5439d..22f38780 100644 --- a/server/src/Notifications/StorefrontOrderCreated.php +++ b/server/src/Notifications/StorefrontOrderCreated.php @@ -8,10 +8,10 @@ use Fleetbase\Storefront\Support\Storefront; use Fleetbase\Support\Utils; use Illuminate\Bus\Queueable; -use Illuminate\Queue\SerializesModels; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; +use Illuminate\Queue\SerializesModels; use NotificationChannels\Twilio\TwilioChannel; use NotificationChannels\Twilio\TwilioSmsMessage; @@ -53,12 +53,29 @@ public function __construct(Order $order) $this->notificationId = uniqid('notification_'); } + /** + * What the SMS guard files the text under. + */ + public string $smsPurpose = 'storefront_order_alert'; + /** * Get the notification's delivery channels. + * + * The text goes through core's SmsServiceChannel, so it passes the SMS guard and lands + * on the send ledger like every other message. A core without that channel (before + * 1.6.70) keeps the Twilio notification channel. */ public function via($notifiable): array { - return ['mail', TwilioChannel::class]; + return ['mail', class_exists(\Fleetbase\Notifications\Channels\SmsServiceChannel::class) ? \Fleetbase\Notifications\Channels\SmsServiceChannel::class : TwilioChannel::class]; + } + + /** + * The text message, for SmsServiceChannel. + */ + public function toSms($notifiable): string + { + return (string) $this->toTwilio($notifiable)->content; } /** diff --git a/server/tests/Unit/Notifications/NotificationContractsTest.php b/server/tests/Unit/Notifications/NotificationContractsTest.php index f7166f32..ad4f57a2 100644 --- a/server/tests/Unit/Notifications/NotificationContractsTest.php +++ b/server/tests/Unit/Notifications/NotificationContractsTest.php @@ -237,7 +237,11 @@ function notificationOrder(array $meta = []): Order $store = new Store(['name' => 'Corner Store']); $notification = notificationWithoutConstructor(StorefrontOrderCreated::class, $order, $store); - expect($notification->via(null))->toBe(['mail', TwilioChannel::class]); + // Through core's guarded channel when the installed core has it, else Twilio's. + $smsChannel = class_exists(Fleetbase\Notifications\Channels\SmsServiceChannel::class) ? Fleetbase\Notifications\Channels\SmsServiceChannel::class : TwilioChannel::class; + expect($notification->via(null))->toBe(['mail', $smsChannel]) + ->and($notification->smsPurpose)->toBe('storefront_order_alert') + ->and($notification->toSms(null))->toContain('A new pickup order was just created!'); $sms = $notification->toTwilio(null); $mail = $notification->toMail(null);