From ebdbc207405b373ab8b67b335e14dc3e388a80b5 Mon Sep 17 00:00:00 2001 From: snipe Date: Mon, 6 Dec 2021 11:40:24 -0800 Subject: [PATCH] Adds stricter validation for slack endpoints Signed-off-by: snipe --- .../Controllers/Api/SettingsController.php | 43 +++++++++++-------- app/Http/Controllers/SettingsController.php | 10 ----- app/Models/Setting.php | 4 +- resources/lang/en/validation.php | 1 + resources/views/settings/slack.blade.php | 16 +++++-- 5 files changed, 40 insertions(+), 34 deletions(-) diff --git a/app/Http/Controllers/Api/SettingsController.php b/app/Http/Controllers/Api/SettingsController.php index 852530c20702..e02715b5dae1 100644 --- a/app/Http/Controllers/Api/SettingsController.php +++ b/app/Http/Controllers/Api/SettingsController.php @@ -162,27 +162,34 @@ public function ldaptestlogin(Request $request, LdapAd $ldap) public function slacktest(Request $request) { - $slack = new Client([ - 'base_url' => e($request->input('slack_endpoint')), - 'defaults' => [ - 'exceptions' => false, - ], - ]); - - $payload = json_encode( - [ - 'channel' => e($request->input('slack_channel')), - 'text' => trans('general.slack_test_msg'), - 'username' => e($request->input('slack_botname')), - 'icon_emoji' => ':heart:', + + // Only attempt the slack request if the validation passes + if ($request->validate([ + 'slack_endpoint' => 'url|required_with:slack_channel|starts_with:https://hooks.slack.com|nullable', + 'slack_channel' => 'required_with:slack_endpoint|starts_with:#|nullable', + ])) { + $slack = new Client([ + 'base_url' => e($request->input('slack_endpoint')), + 'defaults' => [ + 'exceptions' => false, + ], ]); - try { - $slack->post($request->input('slack_endpoint'), ['body' => $payload]); + $payload = json_encode( + [ + 'channel' => e($request->input('slack_channel')), + 'text' => trans('general.slack_test_msg'), + 'username' => e($request->input('slack_botname')), + 'icon_emoji' => ':heart:', + ]); - return response()->json(['message' => 'Success'], 200); - } catch (\Exception $e) { - return response()->json(['message' => 'Oops! Please check the channel name and webhook endpoint URL. Slack responded with: '.$e->getMessage()], 400); + try { + $slack->post($request->input('slack_endpoint'), ['body' => $payload]); + + return response()->json(['message' => 'Success'], 200); + } catch (\Exception $e) { + return response()->json(['message' => 'Oops! Please check the channel name and webhook endpoint URL. Slack responded with: '.$e->getMessage()], 400); + } } return response()->json(['message' => 'Something went wrong :( '], 400); diff --git a/app/Http/Controllers/SettingsController.php b/app/Http/Controllers/SettingsController.php index bf6ad53334c3..41f0a3ba7466 100755 --- a/app/Http/Controllers/SettingsController.php +++ b/app/Http/Controllers/SettingsController.php @@ -665,16 +665,6 @@ public function postSlack(Request $request) return redirect()->to('admin')->with('error', trans('admin/settings/message.update.error')); } - $validatedData = $request->validate([ - 'slack_channel' => 'regex:/(?slack_endpoint = $request->input('slack_endpoint'); - $setting->slack_channel = $request->input('slack_channel'); - $setting->slack_botname = $request->input('slack_botname'); - } - if ($setting->save()) { return redirect()->route('settings.index') ->with('success', trans('admin/settings/message.update.success')); diff --git a/app/Models/Setting.php b/app/Models/Setting.php index 91e349f9fd6a..b594374384ab 100755 --- a/app/Models/Setting.php +++ b/app/Models/Setting.php @@ -54,9 +54,9 @@ class Setting extends Model 'admin_cc_email' => 'email|nullable', 'default_currency' => 'required', 'locale' => 'required', - 'slack_endpoint' => 'url|required_with:slack_channel|nullable', + 'slack_endpoint' => 'url|required_with:slack_channel|nullable|starts_with:https://hooks.slack.com', 'labels_per_page' => 'numeric', - 'slack_channel' => 'regex:/^[\#\@]?\w+/|required_with:slack_endpoint|nullable', + 'slack_channel' => 'required_with:slack_endpoint|starts_with:#|nullable', 'slack_botname' => 'string|nullable', 'labels_width' => 'numeric', 'labels_height' => 'numeric', diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index afec9934b354..72b465f2119d 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -64,6 +64,7 @@ 'string' => 'The :attribute must be at least :min characters.', 'array' => 'The :attribute must have at least :min items.', ], + 'starts_with' => 'The :attribute must start with one of the following: :values.', 'not_in' => 'The selected :attribute is invalid.', 'numeric' => 'The :attribute must be a number.', 'present' => 'The :attribute field must be present.', diff --git a/resources/views/settings/slack.blade.php b/resources/views/settings/slack.blade.php index 31586397827d..7b754e77ec2f 100644 --- a/resources/views/settings/slack.blade.php +++ b/resources/views/settings/slack.blade.php @@ -194,9 +194,11 @@ if (data.responseJSON) { - var errors = data.responseJSON.message; + var errors = data.responseJSON.errors; + var error_msg = data.responseJSON.message; } else { var errors; + var error_msg = 'Something went wrong.'; } var error_text = ''; @@ -204,15 +206,20 @@ $('#save_slack').attr("disabled", true); $("#slacktesticon").html(''); $("#slackteststatus").addClass('text-danger'); - $("#slacktesticon").html(''); + $("#slacktesticon").html('' + error_msg+ ''); + if (data.status == 500) { $('#slackteststatus').html('500 Server Error'); - } else if (data.status == 400) { + } else if ((data.status == 400) || (data.status == 422)) { + console.log('Type of errors is '+ typeof errors); + console.log('Data status was 400 or 422'); if (typeof errors != 'string') { + + console.log(errors.length); - for (i = 0; i < errors.length; i++) { + for (i in errors) { if (errors[i]) { error_text += '
  • Error: ' + errors[i]; } @@ -220,6 +227,7 @@ } } else { + error_text = errors; }