From e3dc28daf3c99b0e2761ee31a755dfe9a30e6ca2 Mon Sep 17 00:00:00 2001 From: kshitij katiyar <90389917+Kshitij-Katiyar@users.noreply.github.com> Date: Tue, 18 Jul 2023 19:55:51 +0530 Subject: [PATCH] [MI-3276]:Added recovery handler and CheckAuth middleware (#2) * [MI-3276]:Added recovery handler and CheckAuth middleware * [MI-3276]:Fixed review comments --- go.mod | 1 + go.sum | 2 + server/plugin.go | 127 ++++++++++++++++++------------------------ webapp/src/actions.js | 2 +- 4 files changed, 58 insertions(+), 74 deletions(-) diff --git a/go.mod b/go.mod index 7eff6657..831e558c 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index d0273d3e..78965c40 100644 --- a/go.sum +++ b/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= diff --git a/server/plugin.go b/server/plugin.go index 63b13402..e5b64f42 100644 --- a/server/plugin.go +++ b/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) @@ -152,7 +179,6 @@ func (p *Plugin) handleTelemetry(w http.ResponseWriter, r *http.Request) { p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err) return } - r.Body.Close() if telemetryRequest == nil { p.API.LogError("Invalid request body") @@ -174,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) @@ -187,7 +209,6 @@ func (p *Plugin) handleAdd(w http.ResponseWriter, r *http.Request) { p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err) return } - r.Body.Close() senderName := p.listManager.GetUserName(userID) @@ -280,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 @@ -349,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) @@ -361,7 +374,6 @@ 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") @@ -401,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) @@ -413,7 +421,6 @@ 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") @@ -463,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) @@ -475,7 +478,6 @@ func (p *Plugin) handleAccept(w http.ResponseWriter, r *http.Request) { p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err) return } - r.Body.Close() if acceptRequest == nil { p.API.LogError("Invalid request body") @@ -506,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) @@ -518,7 +516,6 @@ func (p *Plugin) handleComplete(w http.ResponseWriter, r *http.Request) { p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err) return } - r.Body.Close() if completeRequest == nil { p.API.LogError("Invalid request body") @@ -557,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) @@ -570,7 +563,6 @@ func (p *Plugin) handleRemove(w http.ResponseWriter, r *http.Request) { p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err) return } - r.Body.Close() if removeRequest == nil { p.API.LogError("Invalid request body") @@ -615,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) @@ -628,7 +616,6 @@ func (p *Plugin) handleBump(w http.ResponseWriter, r *http.Request) { p.handleErrorWithCode(w, http.StatusBadRequest, "Unable to decode JSON", err) return } - r.Body.Close() if bumpRequest == nil { p.API.LogError("Invalid request body") @@ -658,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 diff --git a/webapp/src/actions.js b/webapp/src/actions.js index 257f3742..6aaa5432 100644 --- a/webapp/src/actions.js +++ b/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}), })); };