Merge pull request #10394 from snipe/fixes/add_stricter_validation_fo…

…r_slack_hooks Adds stricter validation for slack hooks
snipe · Dec 6, 2021 · 4612b9e · 4612b9e
2 parents cae62fd + ebdbc20
commit 4612b9e
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 34 deletions.
diff --git 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
@@ -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:/(?<!\w)#\w+/|required_with:slack_endpoint|nullable',
-        ]);
-
-        if ($validatedData) {
-            $setting->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
@@ -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
@@ -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
@@ -194,32 +194,40 @@
 
 
                     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 = '';
 
                     $('#save_slack').attr("disabled", true);
                     $("#slacktesticon").html('');
                     $("#slackteststatus").addClass('text-danger');
-                    $("#slacktesticon").html('<i class="fas fa-exclamation-triangle text-danger"></i>');
+                    $("#slacktesticon").html('<i class="fas fa-exclamation-triangle text-danger"></i><span class="text-danger">' + error_msg+ '</span>');
 
+                    
                     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 += '<li>Error: ' + errors[i];
                                 }
 
                             }
 
                         } else {
+
                             error_text = errors;
                         }