[MM-53425]: Added additional checks for POST type APIs

go.mod

@@ -3,6 +3,7 @@ module github.com/mattermost/mattermost-plugin-todo
 go 1.13
 
 require (
+	github.com/gorilla/mux v1.8.0
 	github.com/mattermost/mattermost-plugin-api v0.0.11
 	github.com/mattermost/mattermost-server/v5 v5.3.2-0.20200804063212-d4dac31b042a
 	github.com/pkg/errors v0.9.1

go.sum

@@ -219,6 +219,8 @@ github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51
 github.com/gorilla/handlers v1.4.2/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ=
 github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/gorilla/mux v1.7.4/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
+github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
+github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
 github.com/gorilla/schema v1.1.0/go.mod h1:kgLaKoK1FELgZqMAVxx/5cbj0kT+57qxUrAlIO2eleU=
 github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=

server/plugin.go

@@ -4,10 +4,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"runtime/debug"
 	"strconv"
 	"sync"
 	"time"
 
+	"github.com/gorilla/mux"
 	"github.com/mattermost/mattermost-plugin-api/experimental/telemetry"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/plugin"
@@ -57,6 +59,8 @@ type Plugin struct {
 	// configurationLock synchronizes access to the configuration.
 	configurationLock sync.RWMutex
 
+	router *mux.Router
+
 	// configuration is the active plugin configuration. Consult getConfiguration and
 	// setConfiguration for usage.
 	configuration *configuration
@@ -85,6 +89,8 @@ func (p *Plugin) OnActivate() error {
 
 	p.listManager = NewListManager(p.API)
 
+	p.initializeAPI()
+
 	p.telemetryClient, err = telemetry.NewRudderClient()
 	if err != nil {
 		p.API.LogWarn("telemetry client not started", "error", err.Error())
@@ -104,31 +110,56 @@ func (p *Plugin) OnDeactivate() error {
 	return nil
 }
 
+func (p *Plugin) initializeAPI() {
+	p.router = mux.NewRouter()
+	p.router.Use(p.withRecovery)
+
+	p.router.HandleFunc("/add", p.checkAuth(p.handleAdd)).Methods(http.MethodPost)
+	p.router.HandleFunc("/list", p.checkAuth(p.handleList)).Methods(http.MethodGet)
+	p.router.HandleFunc("/remove", p.checkAuth(p.handleRemove)).Methods(http.MethodPost)
+	p.router.HandleFunc("/complete", p.checkAuth(p.handleComplete)).Methods(http.MethodPost)
+	p.router.HandleFunc("/accept", p.checkAuth(p.handleAccept)).Methods(http.MethodPost)
+	p.router.HandleFunc("/bump", p.checkAuth(p.handleBump)).Methods(http.MethodPost)
+	p.router.HandleFunc("/telemetry", p.checkAuth(p.handleTelemetry)).Methods(http.MethodPost)
+	p.router.HandleFunc("/config", p.checkAuth(p.handleConfig)).Methods(http.MethodGet)
+	p.router.HandleFunc("/edit", p.checkAuth(p.handleEdit)).Methods(http.MethodPut)
+	p.router.HandleFunc("/change_assignment", p.checkAuth(p.handleChangeAssignment)).Methods(http.MethodPost)
+
+	// 404 handler
+	p.router.Handle("{anything:.*}", http.NotFoundHandler())
+}
+
 // ServeHTTP demonstrates a plugin that handles HTTP requests by greeting the world.
 func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) {
-	switch r.URL.Path {
-	case "/add":
-		p.handleAdd(w, r)
-	case "/list":
-		p.handleList(w, r)
-	case "/remove":
-		p.handleRemove(w, r)
-	case "/complete":
-		p.handleComplete(w, r)
-	case "/accept":
-		p.handleAccept(w, r)
-	case "/bump":
-		p.handleBump(w, r)
-	case "/telemetry":
-		p.handleTelemetry(w, r)
-	case "/config":
-		p.handleConfig(w, r)
-	case "/edit":
-		p.handleEdit(w, r)
-	case "/change_assignment":
-		p.handleChangeAssignment(w, r)
-	default:
-		http.NotFound(w, r)
+	w.Header().Set("Content-Type", "application/json")
+
+	p.router.ServeHTTP(w, r)
+}
+
+func (p *Plugin) withRecovery(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer func() {
+			if x := recover(); x != nil {
+				p.API.LogWarn("Recovered from a panic",
+					"url", r.URL.String(),
+					"error", x,
+					"stack", string(debug.Stack()))
+			}
+		}()
+
+		next.ServeHTTP(w, r)
+	})
+}
+
+func (p *Plugin) checkAuth(handler http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		userID := r.Header.Get("Mattermost-User-ID")
+		if userID == "" {
+			http.Error(w, "Not authorized", http.StatusUnauthorized)
+			return
+		}
+
+		handler(w, r)
 	}
 }
 
@@ -139,10 +170,6 @@ type telemetryAPIRequest struct {
 
 func (p *Plugin) handleTelemetry(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var telemetryRequest *telemetryAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -153,6 +180,12 @@ func (p *Plugin) handleTelemetry(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if telemetryRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
+
 	if telemetryRequest.Event != "" {
 		p.trackFrontend(userID, telemetryRequest.Event, telemetryRequest.Properties)
 	}
@@ -167,10 +200,6 @@ type addAPIRequest struct {
 
 func (p *Plugin) handleAdd(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var addRequest *addAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -183,6 +212,12 @@ func (p *Plugin) handleAdd(w http.ResponseWriter, r *http.Request) {
 
 	senderName := p.listManager.GetUserName(userID)
 
+	if addRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
+
 	if addRequest.SendTo == "" {
 		_, err = p.listManager.AddIssue(userID, addRequest.Message, addRequest.Description, addRequest.PostID)
 		if err != nil {
@@ -266,10 +301,6 @@ func (p *Plugin) postReplyIfNeeded(postID, message, todo string) {
 
 func (p *Plugin) handleList(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	listInput := r.URL.Query().Get("list")
 	listID := MyListKey
@@ -335,10 +366,6 @@ type editAPIRequest struct {
 
 func (p *Plugin) handleEdit(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var editRequest *editAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -347,7 +374,12 @@ func (p *Plugin) handleEdit(w http.ResponseWriter, r *http.Request) {
 		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err)
 		return
 	}
-	r.Body.Close()
+
+	if editRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
 
 	foreignUserID, list, oldMessage, err := p.listManager.EditIssue(userID, editRequest.ID, editRequest.Message, editRequest.Description)
 	if err != nil {
@@ -381,10 +413,6 @@ type changeAssignmentAPIRequest struct {
 
 func (p *Plugin) handleChangeAssignment(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var changeRequest *changeAssignmentAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -393,7 +421,12 @@ func (p *Plugin) handleChangeAssignment(w http.ResponseWriter, r *http.Request)
 		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err)
 		return
 	}
-	r.Body.Close()
+
+	if changeRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
 
 	if changeRequest.SendTo == "" {
 		http.Error(w, "No user specified", http.StatusBadRequest)
@@ -437,10 +470,6 @@ type acceptAPIRequest struct {
 
 func (p *Plugin) handleAccept(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var acceptRequest *acceptAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -450,6 +479,12 @@ func (p *Plugin) handleAccept(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if acceptRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
+
 	todoMessage, sender, err := p.listManager.AcceptIssue(userID, acceptRequest.ID)
 	if err != nil {
 		p.API.LogError("Unable to accept issue err=" + err.Error())
@@ -473,10 +508,6 @@ type completeAPIRequest struct {
 
 func (p *Plugin) handleComplete(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var completeRequest *completeAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -486,6 +517,12 @@ func (p *Plugin) handleComplete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if completeRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
+
 	issue, foreignID, listToUpdate, err := p.listManager.CompleteIssue(userID, completeRequest.ID)
 	if err != nil {
 		p.API.LogError("Unable to complete issue err=" + err.Error())
@@ -517,10 +554,6 @@ type removeAPIRequest struct {
 
 func (p *Plugin) handleRemove(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var removeRequest *removeAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -531,6 +564,12 @@ func (p *Plugin) handleRemove(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if removeRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
+
 	issue, foreignID, isSender, listToUpdate, err := p.listManager.RemoveIssue(userID, removeRequest.ID)
 	if err != nil {
 		p.API.LogError("Unable to remove issue, err=" + err.Error())
@@ -568,10 +607,6 @@ type bumpAPIRequest struct {
 
 func (p *Plugin) handleBump(w http.ResponseWriter, r *http.Request) {
 	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
 
 	var bumpRequest *bumpAPIRequest
 	decoder := json.NewDecoder(r.Body)
@@ -582,6 +617,12 @@ func (p *Plugin) handleBump(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if bumpRequest == nil {
+		p.API.LogError("Invalid request body")
+		p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", errors.New("invalid request body"))
+		return
+	}
+
 	todoMessage, foreignUser, foreignIssueID, err := p.listManager.BumpIssue(userID, bumpRequest.ID)
 	if err != nil {
 		p.API.LogError("Unable to bump issue, err=" + err.Error())
@@ -604,12 +645,6 @@ func (p *Plugin) handleBump(w http.ResponseWriter, r *http.Request) {
 
 // API endpoint to retrieve plugin configurations
 func (p *Plugin) handleConfig(w http.ResponseWriter, r *http.Request) {
-	userID := r.Header.Get("Mattermost-User-ID")
-	if userID == "" {
-		http.Error(w, "Not authorized", http.StatusUnauthorized)
-		return
-	}
-
 	if r.Method != http.MethodGet {
 		http.Error(w, "Invalid request method", http.StatusMethodNotAllowed)
 		return

webapp/src/actions.js

@@ -128,7 +128,7 @@ export const add = (message, description, sendTo, postID) => async (dispatch, ge
 
 export const editIssue = (postID, message, description) => async (dispatch, getState) => {
     await fetch(getPluginServerRoute(getState()) + '/edit', Client4.getOptions({
-        method: 'post',
+        method: 'put',
         body: JSON.stringify({id: postID, message, description}),
     }));
 };