From 916e70f820c0a12c7bfa72712def3efc535415a5 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 25 Sep 2026 16:07:35 +0800 Subject: [PATCH] fix(settings): send test SMS with the credentials entered in the console "Test SMS Provider" failed with "Credentials are required to create a Client" even with a Twilio SID and token filled in (fleetbase/fleetbase#680). The test endpoints apply the entered credentials to config, but the Twilio manager copies its settings when it is built and is cached twice: as a container singleton and in the facade's static cache. Under Octane that facade cache outlives the request, and the provider's singleton closure reads the worker's base config rather than the request's copy. So a test send used whatever the worker first built: empty credentials when none were saved (the reported error), or the saved ones instead of those just entered. - After applying the entered credentials, both test endpoints rebuild the manager from this request's config and clear the facade cache. A stand-in bound in place of the real manager is left alone. - Once the test send is done the facade cache is cleared again, so the credentials under test are not reused by later requests in the worker. Fixes fleetbase/fleetbase#680 --- .../Internal/v1/SettingController.php | 54 ++++++++- .../SettingControllerExternalProbesTest.php | 109 +++++++++++++++++- 2 files changed, 161 insertions(+), 2 deletions(-) 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'); +});