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);