diff --git a/src/Http/Controllers/Internal/v1/SettingController.php b/src/Http/Controllers/Internal/v1/SettingController.php index 2cc01cac..ac994e1c 100644 --- a/src/Http/Controllers/Internal/v1/SettingController.php +++ b/src/Http/Controllers/Internal/v1/SettingController.php @@ -21,6 +21,9 @@ use Fleetbase\Services\SmsService; use Fleetbase\Support\PlatformApi; use Fleetbase\Support\Utils; +use Fleetbase\Twilio\Manager as TwilioManager; +use Fleetbase\Twilio\Support\Laravel\Facade as TwilioFacade; +use Fleetbase\Twilio\TwilioInterface; use Illuminate\Http\Request; use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Arr; @@ -594,6 +597,8 @@ public function testSmsProviderConfig(AdminRequest $request) } catch (\Throwable $e) { $responseMessage = $e->getMessage(); $status = 'error'; + } finally { + $this->releaseTwilioClient(); } return response()->json([ @@ -843,12 +848,13 @@ public function testTwilioConfig(AdminRequest $request) // Set config from request config(['twilio.twilio.connections.twilio.sid' => $sid, 'twilio.twilio.connections.twilio.token' => $token, 'twilio.twilio.connections.twilio.from' => $from]); + $this->refreshTwilioClient(); $message = 'Twilio configuration is successful, SMS sent to ' . $phone . '.'; $status = 'success'; try { - \Fleetbase\Twilio\Support\Laravel\Facade::message($phone, 'This is a Twilio test from Fleetbase'); + TwilioFacade::message($phone, 'This is a Twilio test from Fleetbase'); } catch (\Twilio\Exceptions\RestException $e) { $message = $e->getMessage(); $status = 'error'; @@ -861,6 +867,8 @@ public function testTwilioConfig(AdminRequest $request) } catch (\Error $e) { $message = $e->getMessage(); $status = 'error'; + } finally { + $this->releaseTwilioClient(); } return response()->json(['status' => $status, 'message' => $message]); @@ -899,6 +907,7 @@ protected function setTemporarySmsProviderConfig(string $provider, array $provid 'services.twilio' => array_replace_recursive(config('services.twilio', []), $providerConfig), 'twilio.twilio.connections.twilio' => array_replace_recursive(config('twilio.twilio.connections.twilio', []), $providerConfig), ]); + $this->refreshTwilioClient(); } if ($provider === SmsService::PROVIDER_CALLPRO) { @@ -908,6 +917,49 @@ protected function setTemporarySmsProviderConfig(string $provider, array $provid } } + /** + * Rebuild the Twilio client from the config just applied. + * + * The Twilio manager copies its connection settings when it is built, and both the + * container singleton and the facade's static cache keep the built manager. Under + * Octane the facade cache outlives the request, so a test send kept using the + * credentials the worker first saw: it failed with "Credentials are required to create + * a Client" when none were saved, or quietly used the saved ones instead of those just + * entered. A stand-in bound in place of the real manager is left alone. + */ + protected function refreshTwilioClient(): void + { + TwilioFacade::clearResolvedInstance('twilio'); + + if (!app()->bound('twilio')) { + return; + } + + $current = app()->resolved('twilio') ? app('twilio') : null; + if ($current !== null && !($current instanceof TwilioManager)) { + return; + } + + // Build it here from this request's config: the provider's singleton closure reads + // the config of the application it was registered on, which under Octane is the + // worker's base application, not the copy this request just changed. + $manager = $current ? get_class($current) : TwilioManager::class; + $config = config('twilio.twilio', []); + + app()->instance('twilio', new $manager($config['default'] ?? 'twilio', $config['connections'] ?? [])); + app()->forgetInstance(TwilioInterface::class); + } + + /** + * Forget the facade's cached Twilio client once a test send is done, so the credentials + * under test don't outlive this request in a long-running worker and get used for real + * messages (verification codes, notifications) sent by later requests. + */ + protected function releaseTwilioClient(): void + { + TwilioFacade::clearResolvedInstance('twilio'); + } + /** * Sends a test exception to Sentry. * diff --git a/tests/Unit/Http/SettingControllerExternalProbesTest.php b/tests/Unit/Http/SettingControllerExternalProbesTest.php index daedf6aa..4f6c04ee 100644 --- a/tests/Unit/Http/SettingControllerExternalProbesTest.php +++ b/tests/Unit/Http/SettingControllerExternalProbesTest.php @@ -47,7 +47,9 @@ function setting_controller_external_probe_request(array $input = []): AdminRequ } afterEach(function () { - app()->forgetInstance('twilio'); + // Unbind rather than just forget the instance: a binding left behind by these tests + // would satisfy later files that expect twilio to be unbound. + app()->offsetUnset('twilio'); Facade::clearResolvedInstances(); }); @@ -211,3 +213,108 @@ function setting_controller_external_probe_request(array $input = []): AdminRequ ]) ->and(config('sentry.dsn'))->toBeNull(); }); + +/** + * A real Twilio manager that records which credentials a send would use instead of + * calling Twilio. + */ +class SettingControllerRecordingTwilioManager extends Fleetbase\Twilio\Manager +{ + public static array $sent = []; + + public function message(string $to, string $message, array $mediaUrls = [], array $params = []): Twilio\Rest\Api\V2010\Account\MessageInstance + { + $connection = (new ReflectionProperty(Fleetbase\Twilio\Manager::class, 'settings'))->getValue($this)['twilio']; + static::$sent[] = ['to' => $to, 'sid' => $connection['sid'], 'token' => $connection['token'], 'from' => $connection['from']]; + + throw new RuntimeException('recorded'); + } +} + +function setting_controller_bind_recording_twilio(): void +{ + SettingControllerRecordingTwilioManager::$sent = []; + + // Bound the way the Twilio service provider binds the real manager. Under Octane its + // closure reads the config of the worker's base application, not the request's copy, + // so it is modelled here with the config captured when the binding was registered. + $bootConfig = config('twilio.twilio'); + app()->singleton('twilio', fn () => new SettingControllerRecordingTwilioManager($bootConfig['default'] ?? 'twilio', $bootConfig['connections'])); + + // Resolved before the test runs with the saved credentials, as happens at boot or in an + // earlier request handled by the same Octane worker. + app('twilio'); + Fleetbase\Twilio\Support\Laravel\Facade::getFacadeRoot(); +} + +function setting_controller_twilio_facade_is_cached(): bool +{ + $resolved = (new ReflectionProperty(Facade::class, 'resolvedInstance'))->getValue(); + + return isset($resolved['twilio']); +} + +test('test twilio config sends with the credentials entered, not those the client was built with', function () { + setting_controller_external_probe_fixtures(['twilio.twilio.default' => 'twilio']); + setting_controller_bind_recording_twilio(); + + $response = (new SettingController())->testTwilioConfig(setting_controller_external_probe_request([ + 'sid' => 'entered-sid', + 'token' => 'entered-token', + 'from' => '+15555550999', + 'phone' => '+15555550123', + ])); + + expect($response->getData(true)['message'])->toBe('recorded') + ->and(SettingControllerRecordingTwilioManager::$sent)->toBe([ + ['to' => '+15555550123', 'sid' => 'entered-sid', 'token' => 'entered-token', 'from' => '+15555550999'], + ]) + ->and(setting_controller_twilio_facade_is_cached())->toBeFalse('the credentials under test do not outlive the request'); +}); + +test('test sms provider config sends through twilio with the credentials entered', function () { + setting_controller_external_probe_fixtures(['twilio.twilio.default' => 'twilio']); + setting_controller_bind_recording_twilio(); + + $response = (new SettingController())->testSmsProviderConfig(setting_controller_external_probe_request([ + 'provider' => 'twilio', + 'phone' => '+15555550123', + 'config' => ['sid' => 'entered-sid', 'token' => 'entered-token', 'from' => '+15555550999'], + ])); + + expect($response->getData(true))->toMatchArray(['status' => 'error', 'message' => 'recorded']) + ->and(SettingControllerRecordingTwilioManager::$sent)->toBe([ + ['to' => '+15555550123', 'sid' => 'entered-sid', 'token' => 'entered-token', 'from' => '+15555550999'], + ]) + ->and(setting_controller_twilio_facade_is_cached())->toBeFalse(); +}); + +test('a stand-in bound in place of the twilio manager is left in place', function () { + setting_controller_external_probe_fixtures(); + $twilio = new SettingControllerTwilioFake(); + app()->instance('twilio', $twilio); + + (new SettingController())->testTwilioConfig(setting_controller_external_probe_request([ + 'sid' => 'entered-sid', + 'token' => 'entered-token', + 'from' => '+15555550999', + 'phone' => '+15555550123', + ])); + + expect(app('twilio'))->toBe($twilio) + ->and($twilio->messages)->toBe([['+15555550123', 'This is a Twilio test from Fleetbase']]); +}); + +test('a twilio manager bound but not yet built is built from the config just applied', function () { + setting_controller_external_probe_fixtures(['twilio.twilio.default' => 'twilio']); + $bootConfig = config('twilio.twilio'); + app()->singleton('twilio', fn () => new SettingControllerRecordingTwilioManager($bootConfig['default'], $bootConfig['connections'])); + config(['twilio.twilio.connections.twilio.sid' => 'entered-sid']); + + $refresh = new ReflectionMethod(SettingController::class, 'refreshTwilioClient'); + $refresh->invoke(new SettingController()); + + $manager = app('twilio'); + expect(get_class($manager))->toBe(Fleetbase\Twilio\Manager::class) + ->and((new ReflectionProperty(Fleetbase\Twilio\Manager::class, 'settings'))->getValue($manager)['twilio']['sid'])->toBe('entered-sid'); +});