Implement webhook validation (#279)

* move webhook handlers to its own file

* implement webhook validation

* lint

* return useful error message

* document usage of zoom's webhook secret

* PR feedback

* rename methods for readability

* add todo comment

---------

Co-authored-by: Mattermost Build <[email protected]>

docs/installation/zoom-configuration/webhook-configuration.md

@@ -4,17 +4,32 @@
 
 When a Zoom meeting ends, the original link shared in the channel can be changed to indicate the meeting has ended and how long it lasted. To enable this functionality, you can create a webhook subscription in Zoom that tells the Mattermost server every time a meeting ends. The Mattermost server then updates the original Zoom message.
 
-1. Select **Feature**.
-2. Enable **Event Subscriptions**.
-3. Select **Add New Event Subscription** and give it a name \(e.g. "Meeting Ended"\).
-4. Enter a valid **Event notification endpoint URL** \(`https://SITEURL/plugins/zoom/webhook?secret=WEBHOOKSECRET`\).
-   * `SITEURL` should be your Mattermost server URL.
-   * `WEBHOOKSECRET` is generated during [Mattermost Setup](../mattermost-setup.md).
+Select **Feature** in the left sidebar to begin setting up the webhook subscription.
 
-![Feature screen](../../.gitbook/assets/feature.png)
+### Configuring webhook authentication
+
+1. Copy the "Secret Token" value from Zoom's form.
+2. Paste this value into the Zoom plugin's settings in the Mattermost system console for the `Zoom Webhook Secret` field.
+
+![Mattermost Webhook Secret](../../.gitbook/assets/mattermost_webhook_secret.png)
+![Zoom Webhook Secret](../../.gitbook/assets/zoom_webhook_secret.png)
+
+3. In the Mattermost system console, generate and copy the `Webhook Secret`. We'll use this value in the next section.
+
+Zoom's webhook secret authentication system has been made required for all webhooks created or modified after the new system was rolled out. In order to maintain backwards compatibility with existing installations of the Zoom plugin, we still support (and also require) the original webhook secret system used by the plugin. So any new webhooks created will need to be configured with both secrets as mentioned in the steps above.
+
+### Configuring webhook events
+
+1. Enable **Event Subscriptions**.
+2. Select **Add New Event Subscription** and give it a name \(e.g. "Meeting Ended"\).
+3. Construct the following URL, where `SITEURL` is your Mattermost server URL, and `WEBHOOKSECRET` is the value in the system console labeled `Webhook Secret`.
+4. Enter in the **Event notification endpoint URL** field the constructed URL:
+
+`https://SITEURL/plugins/zoom/webhook?secret=WEBHOOKSECRET`
+
+![Webhook URL](../../.gitbook/assets/webhook_url.png)
 
 5. Select **Add events** and select the **End Meeting** event.
+6. Select **Done** and to save your app.
 
 ![Event types screen](../../.gitbook/assets/event_types.png)
-
-6. Select **Done** and then save your app.

plugin.json

@@ -54,7 +54,7 @@
                 "key": "AccountLevelApp",
                 "display_name": "OAuth by Account Level App (Beta):",
                 "type": "bool",
-                "help_text": "When true, only an account administrator has to log in. The rest of the users will use their email to log in.",
+                "help_text": "When true, only an account administrator has to log in. The rest of the users will automatically use their Mattermost email to authenticate when starting meetings.",
                 "placeholder": "",
                 "default": false
             },
@@ -109,6 +109,15 @@
                 "regenerate_help_text": "Regenerates the secret for the webhook URL endpoint. Regenerating the secret invalidates your existing Zoom plugin.",
                 "placeholder": "",
                 "default": null
+            },
+            {
+                "key": "ZoomWebhookSecret",
+                "display_name": "Zoom Webhook Secret:",
+                "type": "text",
+                "help_text": "`Secret Token` taken from Zoom's webhook configuration page",
+                "regenerate_help_text": "",
+                "placeholder": "",
+                "default": null
             }
         ]
     }

server/configuration.go

@@ -38,9 +38,13 @@ type configuration struct {
 	AccountLevelApp   bool
 	OAuthClientID     string
 	OAuthClientSecret string
-	OAuthRedirectURL  string
 	EncryptionKey     string
-	WebhookSecret     string
+
+	// WebhookSecret is generated in the Mattermost system console
+	WebhookSecret string
+
+	// ZoomWebhookSecret is the `Secret Token` taken from Zoom's webhook configuration page
+	ZoomWebhookSecret string
 }
 
 // Clone shallow copies the configuration. Your implementation may require a deep copy if

server/http.go

@@ -6,11 +6,8 @@ package main
 import (
 	"bytes"
 	"context"
-	"crypto/subtle"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
-	"math"
 	"net/http"
 	"strings"
 	"time"
@@ -203,109 +200,6 @@ func (p *Plugin) completeUserOAuthToZoom(w http.ResponseWriter, r *http.Request)
 	}
 }
 
-func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) {
-	if !p.verifyWebhookSecret(r) {
-		p.API.LogWarn("Could not verify webhook secreet")
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
-
-	if !strings.Contains(r.Header.Get("Content-Type"), "application/json") {
-		res := fmt.Sprintf("Expected Content-Type 'application/json' for webhook request, received '%s'.", r.Header.Get("Content-Type"))
-		p.API.LogWarn(res)
-		http.Error(w, res, http.StatusBadRequest)
-		return
-	}
-
-	b, err := ioutil.ReadAll(r.Body)
-	if err != nil {
-		p.API.LogWarn("Cannot read body from Webhook")
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
-	}
-
-	var webhook zoom.Webhook
-	if err = json.Unmarshal(b, &webhook); err != nil {
-		p.API.LogError("Error unmarshaling webhook", "err", err.Error())
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
-	}
-
-	if webhook.Event != zoom.EventTypeMeetingEnded {
-		w.WriteHeader(http.StatusOK)
-		return
-	}
-
-	var meetingWebhook zoom.MeetingWebhook
-	if err = json.Unmarshal(b, &meetingWebhook); err != nil {
-		p.API.LogError("Error unmarshaling meeting webhook", "err", err.Error())
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
-	}
-	p.handleMeetingEnded(w, r, &meetingWebhook)
-}
-
-func (p *Plugin) handleMeetingEnded(w http.ResponseWriter, r *http.Request, webhook *zoom.MeetingWebhook) {
-	meetingPostID := webhook.Payload.Object.ID
-	postID, appErr := p.fetchMeetingPostID(meetingPostID)
-	if appErr != nil {
-		http.Error(w, appErr.Error(), appErr.StatusCode)
-		return
-	}
-
-	post, appErr := p.API.GetPost(postID)
-	if appErr != nil {
-		p.API.LogWarn("Could not get meeting post by id", "err", appErr)
-		http.Error(w, appErr.Error(), appErr.StatusCode)
-		return
-	}
-
-	start := time.Unix(0, post.CreateAt*int64(time.Millisecond))
-	length := int(math.Ceil(float64((model.GetMillis()-post.CreateAt)/1000) / 60))
-	startText := start.Format("Mon Jan 2 15:04:05 -0700 MST 2006")
-	topic, ok := post.Props["meeting_topic"].(string)
-	if !ok {
-		topic = defaultMeetingTopic
-	}
-
-	meetingID, ok := post.Props["meeting_id"].(float64)
-	if !ok {
-		meetingID = 0
-	}
-
-	slackAttachment := model.SlackAttachment{
-		Fallback: fmt.Sprintf("Meeting %s has ended: started at %s, length: %d minute(s).", post.Props["meeting_id"], startText, length),
-		Title:    topic,
-		Text: fmt.Sprintf(
-			"Meeting ID: %d\n\n##### Meeting Summary\n\nDate: %s\n\nMeeting Length: %d minute(s)",
-			int(meetingID),
-			startText,
-			length,
-		),
-	}
-
-	post.Message = "The meeting has ended."
-	post.Props["meeting_status"] = zoom.WebhookStatusEnded
-	post.Props["attachments"] = []*model.SlackAttachment{&slackAttachment}
-
-	_, appErr = p.API.UpdatePost(post)
-	if appErr != nil {
-		p.API.LogWarn("Could not update the post", "err", appErr)
-		http.Error(w, appErr.Error(), appErr.StatusCode)
-		return
-	}
-
-	if appErr = p.deleteMeetingPostID(meetingPostID); appErr != nil {
-		p.API.LogWarn("failed to delete db entry", "error", appErr.Error())
-		return
-	}
-
-	w.Header().Set("Content-Type", "application/json")
-	if err := json.NewEncoder(w).Encode(post); err != nil {
-		p.API.LogWarn("failed to write response", "error", err.Error())
-	}
-}
-
 func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID string, rootID string, topic string) error {
 	meetingURL := p.getMeetingURL(creator, meetingID)
 
@@ -544,7 +438,8 @@ func getString(key string, props model.StringInterface) string {
 }
 
 func (p *Plugin) deauthorizeUser(w http.ResponseWriter, r *http.Request) {
-	if !p.verifyWebhookSecret(r) {
+	// TODO: Check if we need to perform Zoom's webhook verification here https://github.com/mattermost/mattermost-plugin-zoom/issues/291
+	if !p.verifyMattermostWebhookSecret(r) {
 		http.Error(w, "Not authorized", http.StatusUnauthorized)
 		return
 	}
@@ -578,11 +473,6 @@ func (p *Plugin) deauthorizeUser(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (p *Plugin) verifyWebhookSecret(r *http.Request) bool {
-	config := p.getConfiguration()
-	return subtle.ConstantTimeCompare([]byte(r.URL.Query().Get("secret")), []byte(config.WebhookSecret)) == 1
-}
-
 func (p *Plugin) completeCompliance(payload zoom.DeauthorizationPayload) error {
 	data := zoom.ComplianceRequest{
 		ClientID:                     payload.ClientID,

server/plugin_test.go

@@ -25,7 +25,6 @@ import (
 )
 
 func TestPlugin(t *testing.T) {
-	t.Skip("need to fix this test and use the new plugin-api lib")
 	// Mock zoom server
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path == "/users/theuseremail" {
@@ -111,6 +110,12 @@ func TestPlugin(t *testing.T) {
 
 			api := &plugintest.API{}
 
+			api.On("GetLicense").Return(nil)
+			api.On("GetServerVersion").Return("6.2.0")
+
+			api.On("KVGet", "mmi_botid").Return([]byte(botUserID), nil)
+			api.On("PatchBot", botUserID, mock.AnythingOfType("*model.BotPatch")).Return(nil, nil)
+
 			api.On("GetUser", "theuserid").Return(&model.User{
 				Id:    "theuserid",
 				Email: "theuseremail",
@@ -159,11 +164,6 @@ func TestPlugin(t *testing.T) {
 			p.SetAPI(api)
 			p.tracker = telemetry.NewTracker(nil, "", "", "", "", "", false)
 
-			// TODO: fixme
-			// helpers := &plugintest.Helpers{}
-			// helpers.On("EnsureBot", mock.AnythingOfType("*model.Bot")).Return(botUserID, nil)
-			// p.SetHelpers(helpers)
-
 			err = p.OnActivate()
 			require.Nil(t, err)