From bc0c72262e3dfa76c6c4b4b990335e6ace8c15c6 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 13 Oct 2026 18:53:22 +0800 Subject: [PATCH] fix(api): answer an SMS guard refusal with its status The customer and driver login, creation-code and forgot-password endpoints wrap their SMS send in a catch-all, so a refusal from the guard (a RuntimeException) was flattened into the generic 400 send error, or into the email fallback, and an app could neither explain it nor back off. The refusal now reaches the caller with the status it carries: 422 for a number that can never be texted, 429 with Retry-After for a cooldown or budget, 423 while texting is paused. Login still falls back to email when the account has an address; the refusal is the answer only when nothing else can carry the code. --- .../Controllers/Api/v1/CustomerController.php | 47 ++++++++++++++++- .../Controllers/Api/v1/DriverController.php | 43 +++++++++++++++- .../ApiCustomerControllerContractsTest.php | 50 +++++++++++++++++++ .../tests/ApiDriverPasswordContractsTest.php | 27 +++++++--- .../Api/DriverControllerAuthFlowsTest.php | 23 +++++++++ 5 files changed, 181 insertions(+), 9 deletions(-) diff --git a/server/src/Http/Controllers/Api/v1/CustomerController.php b/server/src/Http/Controllers/Api/v1/CustomerController.php index 108f11482..0fc7479c8 100644 --- a/server/src/Http/Controllers/Api/v1/CustomerController.php +++ b/server/src/Http/Controllers/Api/v1/CustomerController.php @@ -2,6 +2,7 @@ namespace Fleetbase\FleetOps\Http\Controllers\Api\v1; +use Fleetbase\Exceptions\SmsBlockedException; use Fleetbase\FleetOps\Exceptions\UserAlreadyExistsException; use Fleetbase\FleetOps\Http\Requests\CreateCustomerOrderRequest; use Fleetbase\FleetOps\Http\Requests\CreateCustomerRequest; @@ -28,6 +29,7 @@ use Illuminate\Http\Request; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; +use Symfony\Component\HttpFoundation\Response; /** * Customer-facing Fleet-Ops API. @@ -126,6 +128,8 @@ public function requestCreationCode(VerifyCreateCustomerRequest $request) 'meta' => $meta, ]); } + } catch (SmsBlockedException $e) { + return static::smsBlockedResponse($e); } catch (\Twilio\Exceptions\RestException $e) { return response()->apiError($e->getMessage()); } catch (\Exception $e) { @@ -395,6 +399,11 @@ public function loginWithPhone(Request $request) return response()->apiError(static::DEACTIVATED_LOGIN_MESSAGE, 403); } + // A guard refusal (paused, over budget, unusable number) falls back to email like + // a provider failure does; with no email to fall back to, the refusal itself is the + // answer, with its status, not the generic error. + $smsFailure = null; + try { $this->generateSmsVerification($user, 'fleetops_customer_login', [ 'hashed' => true, @@ -403,6 +412,8 @@ public function loginWithPhone(Request $request) return response()->json(['status' => 'ok', 'method' => 'sms']); } catch (\Throwable $e) { + $smsFailure = $e; + if ($user->email) { try { $this->generateEmailVerification($user, 'fleetops_customer_login', [ @@ -412,12 +423,12 @@ public function loginWithPhone(Request $request) return response()->json(['status' => 'ok', 'method' => 'email']); } catch (\Throwable $inner) { - return response()->apiError('Unable to send verification code.'); + return static::sendFailureResponse($smsFailure, 'Unable to send verification code.'); } } } - return response()->apiError('Unable to send verification code.'); + return static::sendFailureResponse($smsFailure, 'Unable to send verification code.'); } /** @@ -504,6 +515,8 @@ public function forgotPassword(Request $request) 'meta' => $meta, ]); } + } catch (SmsBlockedException $e) { + return static::smsBlockedResponse($e); } catch (\Throwable $e) { return response()->apiError(app()->hasDebugModeEnabled() ? $e->getMessage() : 'Unable to send reset code.'); } @@ -951,6 +964,36 @@ protected function generateSmsVerification(User $user, string $for, array $optio return VerificationCode::generateSmsVerificationFor($user, $for, $options); } + /** + * Answer a refusal from the SMS guard with the status it carries: 422 for a number that + * can never be texted, 429 with Retry-After for a cooldown or budget, 423 while texting + * is paused. The sends here sit inside catch-alls so a provider outage becomes an email + * fallback or a generic error; a refusal must not be flattened into that generic error, + * or an app can neither explain it to the person nor back off. + */ + protected static function smsBlockedResponse(SmsBlockedException $e) + { + $response = response()->apiError($e->getMessage(), $e->status()); + if ($response instanceof Response) { + $response->headers->add($e->headers()); + } + + return $response; + } + + /** + * The response when a code could not be sent: the guard's refusal when that is why, + * otherwise the flow's own generic message. + */ + protected static function sendFailureResponse(?\Throwable $failure, string $message) + { + if ($failure instanceof SmsBlockedException) { + return static::smsBlockedResponse($failure); + } + + return response()->apiError($message); + } + /** * Whether the supplied code matches the configured testing bypass code. * diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index 7171cf6b4..2deb0b7a8 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -2,6 +2,7 @@ namespace Fleetbase\FleetOps\Http\Controllers\Api\v1; +use Fleetbase\Exceptions\SmsBlockedException; use Fleetbase\FleetOps\Events\DriverLocationChanged; use Fleetbase\FleetOps\Events\GeofenceEntered; use Fleetbase\FleetOps\Events\GeofenceExited; @@ -44,6 +45,7 @@ use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; +use Symfony\Component\HttpFoundation\Response; class DriverController extends Controller { @@ -590,6 +592,11 @@ public function loginWithPhone() // Get the user's company for this driver profile $company = static::getDriverCompanyFromUser($user); + // A guard refusal (paused, over budget, unusable number) falls back to email like + // a provider failure does; with no email to fall back to, the refusal itself is the + // answer, with its status, not the generic error. + $smsFailure = null; + // generate verification token try { VerificationCode::generateSmsVerificationFor($user, 'driver_login', [ @@ -602,6 +609,8 @@ public function loginWithPhone() return response()->json(['status' => 'OK', 'method' => 'sms']); } catch (\Throwable $e) { + $smsFailure = $e; + if (app()->bound('sentry')) { app('sentry')->captureException($e); } @@ -625,7 +634,7 @@ public function loginWithPhone() } } - return response()->apiError('Unable to send SMS Verification code.'); + return static::sendFailureResponse($smsFailure, 'Unable to send SMS Verification code.'); } /** @@ -1353,6 +1362,8 @@ public function forgotPassword(Request $request) try { static::sendResetCode($user, $identity); + } catch (SmsBlockedException $e) { + return static::smsBlockedResponse($e); } catch (\Throwable $e) { return response()->apiError(static::debugEnabled() ? $e->getMessage() : 'Unable to send reset code.'); } @@ -1482,6 +1493,36 @@ protected static function sendResetCode(User $user, string $identity): void } } + /** + * Answer a refusal from the SMS guard with the status it carries: 422 for a number that + * can never be texted, 429 with Retry-After for a cooldown or budget, 423 while texting + * is paused. The sends here sit inside catch-alls so a provider outage becomes an email + * fallback or a generic error; a refusal must not be flattened into that generic error, + * or an app can neither explain it to the person nor back off. + */ + protected static function smsBlockedResponse(SmsBlockedException $e) + { + $response = response()->apiError($e->getMessage(), $e->status()); + if ($response instanceof Response) { + $response->headers->add($e->headers()); + } + + return $response; + } + + /** + * The response when a code could not be sent: the guard's refusal when that is why, + * otherwise the flow's own generic message. + */ + protected static function sendFailureResponse(?\Throwable $failure, string $message) + { + if ($failure instanceof SmsBlockedException) { + return static::smsBlockedResponse($failure); + } + + return response()->apiError($message); + } + /** Resolve a driver's user account from an email address or a phone number. */ protected static function findDriverUserByIdentity(string $identity): ?User { diff --git a/server/tests/ApiCustomerControllerContractsTest.php b/server/tests/ApiCustomerControllerContractsTest.php index 1140e1460..81edbd4f6 100644 --- a/server/tests/ApiCustomerControllerContractsTest.php +++ b/server/tests/ApiCustomerControllerContractsTest.php @@ -1,5 +1,6 @@ smsVerificationThrows) { + throw $this->smsVerificationThrows; + } if ($this->smsVerificationFails) { throw new RuntimeException('sms failed'); } @@ -978,6 +984,50 @@ function fleetopsApiCustomerJson($response): array ]))))->toBe(['error' => 'update failed']); }); +test('api customer controller answers an sms guard refusal with its status, not the generic error', function () { + // A paused organization: the sign-up code request answers 423 with the wait. + $paused = fleetopsApiCustomerController(); + $paused->activeUser = null; + $paused->smsVerificationThrows = SmsBlockedException::paused('company:company-uuid', 120); + $creation = $paused->requestCreationCode(new VerifyCreateCustomerRequest([ + 'mode' => 'sms', + 'identity' => '15550000000', + ])); + + // Over budget with no email on file: the 429 and its Retry-After reach the app. + $limited = fleetopsApiCustomerController(); + $limited->smsVerificationThrows = SmsBlockedException::limited('company_per_hour', 45); + $login = $limited->loginWithPhone(Request::create('/v1/customers/login/phone', 'POST', [ + 'phone' => '15551234567', + ])); + + // Over budget with an email on file: the code still goes by email. + $fallback = fleetopsApiCustomerController(); + $fallback->smsVerificationThrows = SmsBlockedException::limited('company_per_hour', 45); + $fallback->activeUser->email = 'jane@example.test'; + $byEmail = $fallback->loginWithPhone(Request::create('/v1/customers/login/phone', 'POST', [ + 'phone' => '15551234567', + ])); + + // A number that can never be texted: the reset request answers 422. + $invalid = fleetopsApiCustomerController(); + $invalid->smsVerificationThrows = SmsBlockedException::invalidDestination(PhoneValidator::REASON_TYPE_NOT_ALLOWED); + $forgot = $invalid->forgotPassword(Request::create('/v1/customers/forgot-password', 'POST', [ + 'identity' => '15551234567', + ])); + + expect($creation->getStatusCode())->toBe(423) + ->and($creation->headers->get('Retry-After'))->toBe('120') + ->and(fleetopsApiCustomerJson($creation))->toBe(['error' => 'Text messages are temporarily unavailable. Try again later.']) + ->and($login->getStatusCode())->toBe(429) + ->and($login->headers->get('Retry-After'))->toBe('45') + ->and(fleetopsApiCustomerJson($login))->toBe(['error' => 'Unable to send a verification code right now. Try again in a minute.']) + ->and(fleetopsApiCustomerJson($byEmail))->toBe(['status' => 'ok', 'method' => 'email']) + ->and($fallback->emailVerifications[0][1])->toBe('fleetops_customer_login') + ->and($forgot->getStatusCode())->toBe(422) + ->and(fleetopsApiCustomerJson($forgot))->toBe(['error' => 'The phone number provided cannot receive text messages.']); +}); + /** * Run a callback with an app container that supports environment(). * diff --git a/server/tests/ApiDriverPasswordContractsTest.php b/server/tests/ApiDriverPasswordContractsTest.php index 3716031c8..f9e580361 100644 --- a/server/tests/ApiDriverPasswordContractsTest.php +++ b/server/tests/ApiDriverPasswordContractsTest.php @@ -130,12 +130,13 @@ public function getUser(): ?User class FleetOpsPasswordControllerProbe extends DriverController { - public static ?Driver $driver = null; - public static bool $driverMissing = false; - public static ?User $identityUser = null; - public static mixed $resetCode = null; - public static array $sentCodes = []; - public static bool $sendThrows = false; + public static ?Driver $driver = null; + public static bool $driverMissing = false; + public static ?User $identityUser = null; + public static mixed $resetCode = null; + public static array $sentCodes = []; + public static bool $sendThrows = false; + public static ?Throwable $sendRefusal = null; public function findDriver(string $id, array $with = []): Driver { @@ -180,6 +181,9 @@ protected static function debugEnabled(): bool protected static function sendResetCode(User $user, string $identity): void { + if (static::$sendRefusal) { + throw static::$sendRefusal; + } if (static::$sendThrows) { throw new RuntimeException('sms gateway down'); } @@ -205,6 +209,7 @@ function fleetopsPasswordUser(string $plainPassword = 'correct-horse'): FleetOps FleetOpsPasswordControllerProbe::$resetCode = null; FleetOpsPasswordControllerProbe::$sentCodes = []; FleetOpsPasswordControllerProbe::$sendThrows = false; + FleetOpsPasswordControllerProbe::$sendRefusal = null; }); test('change password requires both the current and the new one', function () { @@ -331,6 +336,16 @@ function fleetopsPasswordUser(string $plainPassword = 'correct-horse'): FleetOps expect($response->getStatusCode())->toBe(400); }); +test('forgot password passes an sms guard refusal through with its status', function () { + FleetOpsPasswordControllerProbe::$identityUser = fleetopsPasswordUser(); + FleetOpsPasswordControllerProbe::$sendRefusal = Fleetbase\Exceptions\SmsBlockedException::limited('destination_cooldown', 30); + + $response = (new FleetOpsPasswordControllerProbe())->forgotPassword(new Request(['identity' => '+6581000001'])); + + expect($response->getStatusCode())->toBe(429) + ->and($response->getData())->toBe(['error' => 'Unable to send a verification code right now. Try again in a minute.']); +}); + test('reset password requires identity, code and a new password', function () { $response = (new FleetOpsPasswordControllerProbe())->resetPassword(new Request(['identity' => 'a@b.test'])); diff --git a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php index d2bf127af..f3b6bc9b3 100644 --- a/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php +++ b/server/tests/Feature/Http/Api/DriverControllerAuthFlowsTest.php @@ -304,6 +304,29 @@ public function __call($method, $arguments) ->and($connection->table('verification_codes')->where('for', 'driver_login')->count())->toBe(1); }); +test('phone login passes an sms guard refusal through once no email can carry the code', function () { + $connection = fleetopsDriverAuthBoot(); + app()->instance('twilio', new class { + public function message($to, $text, $mediaUrls = [], $params = []) + { + throw Fleetbase\Exceptions\SmsBlockedException::paused('platform', 300); + } + }); + Fleetbase\Twilio\Support\Laravel\Facade::clearResolvedInstances(); + + // With an email on file the refusal is handled like any send failure: by email. + app()->instance('request', Request::create('/x', 'POST', ['phone' => '6591234567'])); + $byEmail = (new DriverController())->loginWithPhone(); + expect($byEmail->getData(true))->toBe(['status' => 'OK', 'method' => 'email']); + + // Without one, the app gets the refusal itself: 423 and how long to wait. + $connection->table('users')->where('uuid', 'user-1')->update(['email' => null]); + $refused = (new DriverController())->loginWithPhone(); + expect($refused->getStatusCode())->toBe(423) + ->and($refused->headers->get('Retry-After'))->toBe('300') + ->and($refused->getData(true)['error'])->toBe('Text messages are temporarily unavailable. Try again later.'); +}); + test('code verification handles unknown users invalid codes bypass and success', function () { $connection = fleetopsDriverAuthBoot(); $controller = new DriverController();