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