From 7f7eea2f799c04a000df5be6d30bd0859931a74d Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Fri, 26 Jun 2026 14:32:42 +0100 Subject: [PATCH] Normalize HTTP responses for external services Change various hooks to better determine HTTP status codes Got webhooks to test OK in Slack, at least --- app/Helpers/Helper.php | 37 ++ .../Controllers/Api/SettingsController.php | 24 +- app/Livewire/SlackSettingsForm.php | 318 +++++++----------- .../livewire/slack-settings-form.blade.php | 20 +- routes/api.php | 4 +- 5 files changed, 175 insertions(+), 228 deletions(-) diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index c7443db26e..2c964568fe 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -2216,4 +2216,41 @@ class Helper return $html; } + + /** + * Force equal timing between 'success' and 'failure' to not expose open ports + */ + public static function EqualTiming(int $seconds, callable $function) + { + $start = microtime(true); + $thrown_exception = null; + $result = null; + try { + $result = $function(); + if (is_bool($result)) { + if ($result) { + return; //instant return on success + } + } //Fall-through on failure - $result is false so the next 'if' will not fire + if ($result && $result->getStatusCode() == 200) { + //succesful responses should return 'fast' + return $result; + } + } catch (\Throwable $exception) { + $thrown_exception = $exception; + } + // on *any* transaction(?), make sure we don't do a 'fast fail' - + // it needs to take $seconds seconds. + $end = microtime(true); + $duration = $end - $start; + $remaining = $seconds - $duration; + if ($remaining > 0) { + sleep((int) $remaining); + } + if ($thrown_exception) { + throw $thrown_exception; + } + return $result; + + } } diff --git a/app/Http/Controllers/Api/SettingsController.php b/app/Http/Controllers/Api/SettingsController.php index cef0e53037..97bbebc1f0 100644 --- a/app/Http/Controllers/Api/SettingsController.php +++ b/app/Http/Controllers/Api/SettingsController.php @@ -29,13 +29,13 @@ class SettingsController extends Controller return response()->json(['message' => 'LDAP is not enabled, cannot test.'], 400); } + return Helper::EqualTiming(5, function () { - Log::debug('Preparing to test LDAP connection'); + Log::debug('Preparing to test LDAP connection'); - $message = []; // where we collect together test messages - try { - $connection = Ldap::connectToLdap(); + $message = []; // where we collect together test messages try { + $connection = Ldap::connectToLdap(); $message['bind'] = ['message' => 'Successfully bound to LDAP server.']; Log::debug('attempting to bind to LDAP for LDAP test'); Ldap::bindAdminToLdap($connection); @@ -52,8 +52,7 @@ class SettingsController extends Controller return is_int($key); })->slice(0, 10)->map(function ($item) { $mapped = Ldap::parseAndMapLdapAttributes($item); - - return (object) array_map(fn ($value) => $value === '' ? null : $value, $mapped); + return (object) array_map(fn($value) => $value === '' ? null : $value, $mapped); }); if ($users->count() > 0) { // `fields` is the ordered internal_key => translated @@ -63,7 +62,7 @@ class SettingsController extends Controller // consistent as attributeMap() grows. $labels = Ldap::attributeLabels(); $fields = collect(array_keys(Ldap::parseAndMapLdapAttributes([]))) - ->mapWithKeys(fn ($key) => [$key => $labels[$key] ?? $key]) + ->mapWithKeys(fn($key) => [$key => $labels[$key] ?? $key]) ->all(); $message['user_sync'] = [ 'fields' => $fields, @@ -79,18 +78,11 @@ class SettingsController extends Controller return response()->json($message, 200); } catch (\Exception $e) { - Log::debug('Bind failed'); - Log::debug('Exception was: '.$e->getMessage()); + Log::debug('Connection failed but we cannot debug it any further on our end.'); return response()->json(['message' => $e->getMessage()], 400); - // return response()->json(['message' => $e->getMessage()], 500); } - } catch (\Exception $e) { - Log::debug('Connection failed but we cannot debug it any further on our end.'); - - return response()->json(['message' => $e->getMessage()], 400); - } - + }); } public function ldaptestlogin(Request $request): JsonResponse diff --git a/app/Livewire/SlackSettingsForm.php b/app/Livewire/SlackSettingsForm.php index 6a3d4bdf60..9bd2fcefbf 100644 --- a/app/Livewire/SlackSettingsForm.php +++ b/app/Livewire/SlackSettingsForm.php @@ -8,6 +8,7 @@ use App\Rules\ExternalUrl; use GuzzleHttp\Client; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\Facades\Validator; use Illuminate\Support\Str; use Livewire\Component; @@ -41,10 +42,14 @@ class SlackSettingsForm extends Component public $save_button; - public $webhook_test; - public $webhook_endpoint_rules; + public ?string $warning = null; + + public ?string $success = null; + + public ?string $error = null; + protected function rules(): array { return [ @@ -82,59 +87,56 @@ class SlackSettingsForm extends Component 'icon' => 'fab fa-slack', 'placeholder' => 'https://hooks.slack.com/services/XXXXXXXXXXXXXXXXXXXXX', 'link' => 'https://api.slack.com/messaging/webhooks', - 'test' => 'testWebhook', ], 'general' => [ 'name' => trans('admin/settings/general.general_webhook'), 'icon' => 'fab fa-hashtag', 'placeholder' => trans('general.url'), 'link' => '', - 'test' => 'testWebhook', ], 'google' => [ 'name' => trans('admin/settings/general.google_workspaces'), 'icon' => 'fa-brands fa-google', 'placeholder' => 'https://chat.googleapis.com/v1/spaces/xxxxxxxx/messages?key=xxxxxx', 'link' => 'https://developers.google.com/chat/how-tos/webhooks#register_the_incoming_webhook', - 'test' => 'googleWebhookTest', ], 'microsoft' => [ 'name' => trans('admin/settings/general.ms_teams'), 'icon' => 'fa-brands fa-microsoft', 'placeholder' => 'https://abcd.webhook.office.com/webhookb2/XXXXXXX', 'link' => 'https://support.microsoft.com/en-us/office/create-incoming-webhooks-with-workflows-for-microsoft-teams-8ae491c7-0394-4861-ba59-055e33f75498', - 'test' => 'msTeamTestWebhook', ], ]; $this->setting = Setting::getSettings(); $this->save_button = trans('general.save'); - $this->webhook_selected = ($this->setting->webhook_selected !== '') ? $this->setting->webhook_selected : 'slack'; - $this->webhook_name = $this->webhook_text[$this->setting->webhook_selected]['name'] ?? $this->webhook_text['slack']['name']; - $this->webhook_icon = $this->webhook_text[$this->setting->webhook_selected]['icon'] ?? $this->webhook_text['slack']['icon']; - $this->webhook_placeholder = $this->webhook_text[$this->setting->webhook_selected]['placeholder'] ?? $this->webhook_text['slack']['placeholder']; - $this->webhook_link = $this->webhook_text[$this->setting->webhook_selected]['link'] ?? $this->webhook_text['slack']['link']; - $this->webhook_test = $this->webhook_text[$this->setting->webhook_selected]['test'] ?? $this->webhook_text['slack']['test']; + if (!$this->webhook_selected) { + $this->webhook_selected = 'slack'; + } + + + $this->webhook_options = $this->setting->webhook_selected ? $this->setting->webhook_selected : 'slack'; + + + $this->updatedWebhookSelected(); $this->webhook_endpoint = $this->setting->webhook_endpoint; $this->webhook_channel = $this->setting->webhook_channel; $this->webhook_botname = $this->setting->webhook_botname; - $this->webhook_options = $this->setting->webhook_selected; - $this->teams_webhook_deprecated = ! Str::contains($this->webhook_endpoint, 'workflows'); - if ($this->webhook_selected === 'microsoft' || $this->webhook_selected === 'google') { - $this->webhook_channel = '#NA'; - } + $this->teams_webhook_deprecated = !Str::contains($this->webhook_endpoint, 'workflows'); // consider moving this to webhook_link updated? (updatedWebhookLink?) if ($this->setting->webhook_endpoint != null && $this->setting->webhook_channel != null) { $this->isDisabled = ''; } - if ($this->webhook_selected === 'microsoft' && $this->teams_webhook_deprecated) { - session()->flash('warning', trans('admin/settings/message.webhook.ms_teams_deprecation')); + if ($this->webhook_selected === 'microsoft' && $this->teams_webhook_deprecated) { //since this is URL-aware, maybe also move this? + $this->warning = trans('admin/settings/message.webhook.ms_teams_deprecation'); } } public function updated($field) { - + //anything changes; then clear the succcess message (and error message) + $this->success = null; + $this->error = null; $this->validateOnly($field); } @@ -144,10 +146,9 @@ class SlackSettingsForm extends Component $this->webhook_name = $this->webhook_text[$this->webhook_selected]['name']; $this->webhook_icon = $this->webhook_text[$this->webhook_selected]['icon']; $this->webhook_placeholder = $this->webhook_text[$this->webhook_selected]['placeholder']; - $this->webhook_endpoint = null; + $this->webhook_endpoint = null; // TODO - do we really want to blank this? $this->webhook_link = $this->webhook_text[$this->webhook_selected]['link']; - $this->webhook_test = $this->webhook_text[$this->webhook_selected]['test']; - if ($this->webhook_selected != 'slack') { + if ($this->webhook_selected != 'slack') { // TODO: hrm. Wouldn't we want to test all of them? Or at least some of them? Maybe not "generic webhook"? $this->isDisabled = ''; $this->save_button = trans('general.save'); } @@ -158,76 +159,25 @@ class SlackSettingsForm extends Component public function updatedwebhookEndpoint() { - $this->teams_webhook_deprecated = ! Str::contains($this->webhook_endpoint, 'workflows'); - } - - private function isButtonDisabled() - { - if (empty($this->webhook_endpoint)) { - $this->isDisabled = 'disabled'; - $this->save_button = trans('admin/settings/general.webhook_presave'); - } - if (empty($this->webhook_channel)) { - $this->isDisabled = 'disabled'; - $this->save_button = trans('admin/settings/general.webhook_presave'); - } + $this->teams_webhook_deprecated = !Str::contains($this->webhook_endpoint, 'workflows'); } public function render() { - $this->isButtonDisabled(); + if (empty($this->webhook_endpoint) || empty($this->webhook_channel)) { + $this->isDisabled = 'disabled'; + $this->save_button = trans('admin/settings/general.webhook_presave'); + } return view('livewire.slack-settings-form'); } - public function testWebhook() - { - if ($fail = $this->guardWebhookEndpoint()) { - return $fail; - } - - $webhook = new Client([ - 'base_url' => e($this->webhook_endpoint), - 'defaults' => [ - 'exceptions' => false, - ], - 'allow_redirects' => false, - ]); - - $payload = json_encode( - [ - 'channel' => e($this->webhook_channel), - 'text' => trans('general.webhook_test_msg', ['app' => $this->webhook_name]), - 'username' => e($this->webhook_botname), - 'icon_emoji' => ':heart:', - - ]); - - try { - $test = $webhook->post($this->webhook_endpoint, ['body' => $payload, 'headers' => ['Content-Type' => 'application/json']]); - - if (($test->getStatusCode() == 302) || ($test->getStatusCode() == 301)) { - return session()->flash('error', trans('admin/settings/message.webhook.error_redirect', ['endpoint' => $this->webhook_endpoint])); - } - $this->isDisabled = ''; - $this->save_button = trans('general.save'); - - return session()->flash('success', trans('admin/settings/message.webhook.success', ['webhook_name' => $this->webhook_name])); - - } catch (\Exception $e) { - return $this->handleWebhookFailure($e); - } - - return session()->flash('error', trans('admin/settings/message.webhook.error_misc')); - - } - public function clearSettings() { if (Helper::isDemoMode()) { - session()->flash('error', trans('general.feature_disabled')); + $this->error = trans('general.feature_disabled'); } else { $this->webhook_endpoint = ''; $this->webhook_channel = ''; @@ -238,14 +188,14 @@ class SlackSettingsForm extends Component $this->setting->save(); - session()->flash('success', trans('admin/settings/message.update.success')); + $this->success = trans('admin/settings/message.update.success'); } } public function submit() { if (Helper::isDemoMode()) { - session()->flash('error', trans('general.feature_disabled')); + $this->error = trans('general.feature_disabled'); } else { $this->validate(); @@ -256,142 +206,114 @@ class SlackSettingsForm extends Component $this->setting->save(); - session()->flash('success', trans('admin/settings/message.update.success')); + $this->success = trans('admin/settings/message.update.success'); } } - public function googleWebhookTest() + public function universalWebhookTest() { - if ($fail = $this->guardWebhookEndpoint()) { - return $fail; - } + $executed = RateLimiter::attempt( + key: 'test-connection:' . auth()->id(), + maxAttempts: 5, + decaySeconds: 60, + callback: function () { + $validator = Validator::make( + ['webhook_endpoint' => $this->webhook_endpoint], + ['webhook_endpoint' => ['required', 'url', new ExternalUrl]], + ); - $payload = [ - 'text' => trans('general.webhook_test_msg', ['app' => $this->webhook_name]), - ]; + if ($validator->fails()) { + $this->isDisabled = 'disabled'; + $this->save_button = trans('admin/settings/general.webhook_presave'); - try { - $response = Http::withHeaders([ - 'content-type' => 'application/json', - ])->withOptions(['allow_redirects' => false]) - ->post($this->webhook_endpoint, $payload) - ->throw(); + $this->error = $validator->errors()->first('webhook_endpoint'); + return; + } + $this->error = ''; + $this->success = ''; - if (($response->getStatusCode() == 302) || ($response->getStatusCode() == 301)) { - return session()->flash('error', trans('admin/settings/message.webhook.error_redirect', ['endpoint' => $this->webhook_endpoint])); - } - - $this->isDisabled = ''; - $this->save_button = trans('general.save'); - - return session()->flash('success', trans('admin/settings/message.webhook.success', ['webhook_name' => $this->webhook_name])); - - } catch (\Exception $e) { - return $this->handleWebhookFailure($e); - } - } - - public function msTeamTestWebhook() - { - if ($fail = $this->guardWebhookEndpoint()) { - return $fail; - } - - try { - - if ($this->teams_webhook_deprecated) { - // will use the deprecated webhook format - $payload = - [ + $payload = match ($this->webhook_selected) { + 'slack', 'general' => [ + 'channel' => e($this->webhook_channel), + 'text' => trans('general.webhook_test_msg', ['app' => $this->webhook_name]), + 'username' => e($this->webhook_botname), + 'icon_emoji' => ':heart:', + ], + 'google' => [ + 'text' => trans('general.webhook_test_msg', ['app' => $this->webhook_name]), + ], + 'microsoft' => [ '@type' => 'MessageCard', '@context' => 'http://schema.org/extensions', 'summary' => trans('mail.snipe_webhook_summary'), 'title' => trans('mail.snipe_webhook_test'), 'text' => trans('general.webhook_test_msg', ['app' => $this->webhook_name]), - ]; - $response = Http::withHeaders([ - 'content-type' => 'application/json', - ])->withOptions(['allow_redirects' => false]) - ->post($this->webhook_endpoint, $payload) - ->throw(); - } else { - $notification = new TeamsNotification($this->webhook_endpoint); - $message = trans('general.webhook_test_msg', ['app' => $this->webhook_name]); - $notification->success()->sendMessage($message); + ], + default => throw new \Exception("Unknown provider") + }; - $response = Http::withHeaders([ - 'content-type' => 'application/json', - ])->withOptions(['allow_redirects' => false]) - ->post($this->webhook_endpoint); - } + $status_code = null; - if (($response->getStatusCode() == 302) || ($response->getStatusCode() == 301)) { - return session()->flash('error', trans('admin/settings/message.webhook.error_redirect', ['endpoint' => $this->webhook_endpoint])); - } - $this->isDisabled = ''; - $this->save_button = trans('general.save'); + try { + if ($this->webhook_selected == "microsoft" && !$this->teams_webhook_deprecated) { + //new-fangled webhook - use package + $notification = new TeamsNotification($this->webhook_endpoint); + $message = trans('general.webhook_test_msg', ['app' => $this->webhook_name]); + $status_code = $notification->success()->sendMessage($message); + } else { + $response = Http::withHeaders([ + 'content-type' => 'application/json', + ])->withOptions(['allow_redirects' => false]) + ->post($this->webhook_endpoint, $payload)/*->throw()*/ + ; + $status_code = $response->getStatusCode(); + } - return session()->flash('success', trans('admin/settings/message.webhook.success', ['webhook_name' => $this->webhook_name])); + if ($status_code >= 300 && $status_code < 400) { + //these, still, might happen. It seems like allow_redirects being false just doesn't follow them, it doesn't cause an Exception on them. + $this->error = trans('admin/settings/message.webhook.error_redirect', ['endpoint' => $this->webhook_endpoint]); + //TODO: one possibliity here is to re-throw so that everything goes back through the exception handler? + } elseif ($status_code >= 400 && $status_code < 500) { + //this _shouldn't_ happen because the "->throw()" should catch it + $this->error = trans('admin/settings/message.webhook.error_redirect', ['endpoint' => $this->webhook_endpoint]); + } elseif ($status_code >= 500) { + //this _shouldn't_ happen because the "->throw()" should catch it + $this->error = trans('admin/settings/message.webhook.error_server'); + } elseif ($status_code >= 200 && $status_code < 300) { + $this->isDisabled = ''; + $this->save_button = trans('general.save'); - } catch (\Exception $e) { - return $this->handleWebhookFailure($e); - } + $this->success = trans('admin/settings/message.webhook.success', ['webhook_name' => $this->webhook_name]); + return true; // This is just for EqualTiming to use; this method on this controller doesn't actually return anything + } else { + throw new \Exception(trans('admin/settings/message.webhook.error_misc')); + } - return session()->flash('error', trans('admin/settings/message.webhook.error_misc')); - } + } catch (\Exception $e) { + Log::warning('Webhook test failed', [ + 'endpoint' => $this->webhook_endpoint, + 'app' => $this->webhook_name, + 'exception' => $e::class, + 'message' => $e->getMessage(), + 'file' => $e->getFile(), + 'line' => $e->getLine(), + ]); - /** - * Re-validate the currently-entered endpoint immediately before an - * outbound request. This is defense-in-depth on top of the save-time - * ExternalUrl rule: it stops a super-admin from typing an internal - * URL, clicking Test, and having the server dial it before the value - * is ever persisted. Returns a flash-response on failure, null on pass. - */ - private function guardWebhookEndpoint() - { - $validator = Validator::make( - ['webhook_endpoint' => $this->webhook_endpoint], - ['webhook_endpoint' => ['required', 'url', new ExternalUrl]], + $this->isDisabled = 'disabled'; + $this->save_button = trans('admin/settings/general.webhook_presave'); + + $this->error = trans('admin/settings/message.webhook.error', [ + 'error_message' => $e->getMessage(), + 'app' => $this->webhook_name, + ]); + return false; + } + }, ); - if ($validator->fails()) { - $this->isDisabled = 'disabled'; - $this->save_button = trans('admin/settings/general.webhook_presave'); - - return session()->flash('error', $validator->errors()->first('webhook_endpoint')); + if (!$executed) { + $this->addError('connection', trans('admin/settings/general.rate_limited')); } - - return null; } - - /** - * Uniform failure path for outbound webhook tests. The exception message - * used to be a port-scanning oracle back when the endpoint field accepted - * internal URLs; now that the ExternalUrl rule rejects those before we - * ever dial anything, the connect-level error can only describe an - * external host the admin explicitly typed, so we surface it back into - * the flash. Full exception context still lands in the log for audit - * and for cases where the flash message is too short to be useful (SSL - * chain problems, proxy failures, etc.). - */ - private function handleWebhookFailure(\Throwable $e) - { - Log::warning('Webhook test failed', [ - 'endpoint' => $this->webhook_endpoint, - 'app' => $this->webhook_name, - 'exception' => $e::class, - 'message' => $e->getMessage(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - ]); - - $this->isDisabled = 'disabled'; - $this->save_button = trans('admin/settings/general.webhook_presave'); - - return session()->flash('error', trans('admin/settings/message.webhook.error', [ - 'error_message' => $e->getMessage(), - 'app' => $this->webhook_name, - ])); - } -} +} \ No newline at end of file diff --git a/resources/views/livewire/slack-settings-form.blade.php b/resources/views/livewire/slack-settings-form.blade.php index b1204aa32a..0df64f0d6b 100644 --- a/resources/views/livewire/slack-settings-form.blade.php +++ b/resources/views/livewire/slack-settings-form.blade.php @@ -13,14 +13,11 @@ @section('content')
-
+ {{csrf_field()}} - @if (session()->has('warning')) + @if ($warning) - {!! session('warning') !!} - @php - session()->forget('warning'); // Clear the session flash immediately - @endphp + {!! $warning !!} @endif
@@ -47,17 +44,16 @@
- @if(session()->has('success')) + @if($success) - {{session('success')}} + {{$success}} @endif - @if(session()->has('error')) + @if($error) - {{session('error')}} + {{$error}} @endif -