Merge pull request #1268 from mattermost/plt-514

PLT-514 Validate callback urls on the server and add help text to outgoing webhooks

model/outgoing_webhook.go

@@ -100,6 +100,12 @@ func (o *OutgoingWebhook) IsValid() *AppError {
 		return NewAppError("OutgoingWebhook.IsValid", "Invalid callback urls", "")
 	}
 
+	for _, callback := range o.CallbackURLs {
+		if !IsValidHttpUrl(callback) {
+			return NewAppError("OutgoingWebhook.IsValid", "Invalid callback URLs. Each must be a valid URL and start with http:// or https://", "")
+		}
+	}
+
 	return nil
 }
 

model/outgoing_webhook_test.go

@@ -80,6 +80,11 @@ func TestOutgoingWebhookIsValid(t *testing.T) {
 		t.Fatal("should be invalid")
 	}
 
+	o.CallbackURLs = []string{"nowhere.com/"}
+	if err := o.IsValid(); err == nil {
+		t.Fatal("should be invalid")
+	}
+
 	o.CallbackURLs = []string{"http://nowhere.com/"}
 	if err := o.IsValid(); err != nil {
 		t.Fatal(err)

model/utils.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"io"
 	"net/mail"
+	"net/url"
 	"regexp"
 	"strings"
 	"time"
@@ -301,3 +302,15 @@ var UrlRegex = regexp.MustCompile(`^((?:[a-z]+:\/\/)?(?:(?:[a-z0-9\-]+\.)+(?:[a-
 var PartialUrlRegex = regexp.MustCompile(`/([A-Za-z0-9]{26})/([A-Za-z0-9]{26})/((?:[A-Za-z0-9]{26})?.+(?:\.[A-Za-z0-9]{3,})?)`)
 
 var SplitRunes = map[rune]bool{',': true, ' ': true, '.': true, '!': true, '?': true, ':': true, ';': true, '\n': true, '<': true, '>': true, '(': true, ')': true, '{': true, '}': true, '[': true, ']': true, '+': true, '/': true, '\\': true}
+
+func IsValidHttpUrl(rawUrl string) bool {
+	if strings.Index(rawUrl, "http://") != 0 && strings.Index(rawUrl, "https://") != 0 {
+		return false
+	}
+
+	if _, err := url.ParseRequestURI(rawUrl); err != nil {
+		return false
+	}
+
+	return true
+}

web/react/components/user_settings/manage_outgoing_hooks.jsx

@@ -1,10 +1,12 @@
 // Copyright (c) 2015 Spinpunch, Inc. All Rights Reserved.
 // See License.txt for license information.
 
-var Client = require('../../utils/client.jsx');
+const LoadingScreen = require('../loading_screen.jsx');
-var Constants = require('../../utils/constants.jsx');
+
-var ChannelStore = require('../../stores/channel_store.jsx');
+const ChannelStore = require('../../stores/channel_store.jsx');
-var LoadingScreen = require('../loading_screen.jsx');
+
+const Client = require('../../utils/client.jsx');
+const Constants = require('../../utils/constants.jsx');
 
 export default class ManageOutgoingHooks extends React.Component {
     constructor() {
@@ -44,10 +46,10 @@ export default class ManageOutgoingHooks extends React.Component {
                     hooks = [];
                 }
                 hooks.push(data);
-                this.setState({hooks, serverError: null, channelId: '', triggerWords: '', callbackURLs: ''});
+                this.setState({hooks, addError: null, channelId: '', triggerWords: '', callbackURLs: ''});
             },
             (err) => {
-                this.setState({serverError: err});
+                this.setState({addError: err.message});
             }
         );
     }
@@ -74,7 +76,7 @@ export default class ManageOutgoingHooks extends React.Component {
                 this.setState({hooks});
             },
             (err) => {
-                this.setState({serverError: err});
+                this.setState({editError: err.message});
             }
         );
     }
@@ -93,10 +95,10 @@ export default class ManageOutgoingHooks extends React.Component {
                     }
                 }
 
-                this.setState({hooks, serverError: null});
+                this.setState({hooks, editError: null});
             },
             (err) => {
-                this.setState({serverError: err});
+                this.setState({editError: err.message});
             }
         );
     }
@@ -104,11 +106,11 @@ export default class ManageOutgoingHooks extends React.Component {
         Client.listOutgoingHooks(
             (data) => {
                 if (data) {
-                    this.setState({hooks: data, getHooksComplete: true, serverError: null});
+                    this.setState({hooks: data, getHooksComplete: true, editError: null});
                 }
             },
             (err) => {
-                this.setState({serverError: err});
+                this.setState({editError: err.message});
             }
         );
     }
@@ -122,9 +124,13 @@ export default class ManageOutgoingHooks extends React.Component {
         this.setState({callbackURLs: e.target.value});
     }
     render() {
-        let serverError;
+        let addError;
-        if (this.state.serverError) {
+        if (this.state.addError) {
-            serverError = <label className='has-error'>{this.state.serverError}</label>;
+            addError = <label className='has-error'>{this.state.addError}</label>;
+        }
+        let editError;
+        if (this.state.editError) {
+            addError = <label className='has-error'>{this.state.editError}</label>;
         }
 
         const channels = ChannelStore.getAll();
@@ -234,6 +240,7 @@ export default class ManageOutgoingHooks extends React.Component {
 
         return (
             <div key='addOutgoingHook'>
+                {'Create webhooks to send new message events to an external integration. Please see '}<a href='http://mattermost.org/webhooks'>{'http://mattermost.org/webhooks'}</a> {' to learn more.'}
                 <label className='control-label'>{'Add a new outgoing webhook'}</label>
                 <div className='padding-top divider-light'></div>
                 <div className='padding-top'>
@@ -274,10 +281,11 @@ export default class ManageOutgoingHooks extends React.Component {
                             resize={false}
                             rows={3}
                             onChange={this.updateCallbackURLs}
+                            placeholder='Each URL must start with http:// or https://'
                         />
                         </div>
                         <div className='padding-top'>{'New line separated URLs that will receive the HTTP POST event'}</div>
-                        {serverError}
+                        {addError}
                     </div>
                     <div className='padding-top padding-bottom'>
                         <a
@@ -291,6 +299,7 @@ export default class ManageOutgoingHooks extends React.Component {
                     </div>
                 </div>
                 {existingHooks}
+                {editError}
             </div>
         );
     }

web/react/components/user_settings/user_settings_integrations.jsx

@@ -56,7 +56,7 @@ export default class UserSettingsIntegrationsTab extends React.Component {
                     <SettingItemMin
                         title='Incoming Webhooks'
                         width='medium'
-                        describe='Manage your incoming webhooks (Developer feature)'
+                        describe='Manage your incoming webhooks'
                         updateSection={() => {
                             this.updateSection('incoming-hooks');
                         }}