Security review fixes

server/command.go

@@ -44,12 +44,13 @@ const (
 		"* `/confluence install cloud` - Connect Mattermost to a Confluence Cloud instance.\n" +
 		"* `/confluence install server` - Connect Mattermost to a Confluence Server or Data Center instance.\n"
 
-	invalidCommand          = "Invalid command."
-	installOnlySystemAdmin  = "`/confluence install` can only be run by a system administrator."
-	commandsOnlySystemAdmin = "`/confluence` commands can only be run by a system administrator."
-	disconnectedUser        = "User not connected. Please use `/confluence connect`."
-	errorExecutingCommand   = "Error executing the command, please retry."
-	oauth2ConnectPath       = "%s/oauth2/connect"
+	invalidCommand              = "Invalid command."
+	installOnlySystemAdmin      = "`/confluence install` can only be run by a system administrator."
+	commandsOnlySystemAdmin     = "`/confluence` commands can only be run by a system administrator."
+	errorUserLacksChannelAccess = "Cannot perform operation: user does not have access to the channel."
+	disconnectedUser            = "User not connected. Please use `/confluence connect`."
+	errorExecutingCommand       = "Error executing the command, please retry."
+	oauth2ConnectPath           = "%s/oauth2/connect"
 )
 
 const (
@@ -248,9 +249,12 @@ func showInstallServerHelp(p *Plugin, context *model.CommandArgs, _ ...string) *
 }
 
 func deleteSubscription(p *Plugin, context *model.CommandArgs, args ...string) *model.CommandResponse {
+	userID := context.UserId
+	channelID := context.ChannelId
+
 	pluginConfig := config.GetConfig()
 	if pluginConfig.ServerVersionGreaterthan9 {
-		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, context.UserId)
+		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, userID)
 		if err != nil {
 			if strings.Contains(err.Error(), "not found") {
 				postCommandResponse(context, disconnectedUser)
@@ -266,7 +270,7 @@ func deleteSubscription(p *Plugin, context *model.CommandArgs, args ...string) *
 			postCommandResponse(context, disconnectedUser)
 			return &model.CommandResponse{}
 		}
-	} else if !util.IsSystemAdmin(context.UserId) {
+	} else if !util.IsSystemAdmin(userID) {
 		postCommandResponse(context, commandsOnlySystemAdmin)
 		return &model.CommandResponse{}
 	}
@@ -275,12 +279,19 @@ func deleteSubscription(p *Plugin, context *model.CommandArgs, args ...string) *
 		postCommandResponse(context, specifyAlias)
 		return &model.CommandResponse{}
 	}
+
+	if !p.hasChannelAccess(userID, channelID) {
+		postCommandResponse(context, errorUserLacksChannelAccess)
+		return &model.CommandResponse{}
+	}
+
 	alias := strings.Join(args, " ")
-	if err := service.DeleteSubscription(context.ChannelId, alias); err != nil {
+	if err := service.DeleteSubscription(channelID, alias); err != nil {
 		p.client.Log.Error("Error deleting the subscription", "subscription alias", alias, "error", err.Error())
 		postCommandResponse(context, err.Error())
 		return &model.CommandResponse{}
 	}
+
 	postCommandResponse(context, fmt.Sprintf(subscriptionDeleteSuccess, alias))
 	return &model.CommandResponse{}
 }

server/confluence_cloud.go

@@ -26,7 +26,13 @@ func handleConfluenceCloudWebhook(w http.ResponseWriter, r *http.Request, p *Plu
 	}
 
 	params := mux.Vars(r)
-	event := serializer.ConfluenceCloudEventFromJSON(r.Body)
+	event, err := serializer.ConfluenceCloudEventFromJSON(r.Body)
+	if err != nil {
+		p.client.Log.Error("Error occurred while unmarshalling Confluence cloud webhook payload", "error", err)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+
 	go service.SendConfluenceNotifications(event, params["event"])
 
 	w.Header().Set("Content-Type", "application/json")

server/confluence_server.go

@@ -148,6 +148,7 @@ func (p *Plugin) GetClientFromUserKey(instanceID, eventUserKey string) (Client,
 		p.client.Log.Error("Error getting Mattermost User ID from Confluence ID", "InstanceID", instanceID, "Confluence Account ID", eventUserKey, "error", err.Error())
 		return nil, nil, err
 	}
+
 	connection, err := store.LoadConnection(instanceID, *mmUserID)
 	if err != nil {
 		p.client.Log.Error("Error loading the connection", "UserID", *mmUserID, "InstanceURL", instanceID, "error", err.Error())

server/edit_subscription.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"net/http"
+	"strings"
 
 	"github.com/gorilla/mux"
 
@@ -10,6 +11,8 @@ import (
 	"github.com/mattermost/mattermost-plugin-confluence/server/config"
 	"github.com/mattermost/mattermost-plugin-confluence/server/serializer"
 	"github.com/mattermost/mattermost-plugin-confluence/server/service"
+	"github.com/mattermost/mattermost-plugin-confluence/server/store"
+	"github.com/mattermost/mattermost-plugin-confluence/server/util/types"
 )
 
 var editChannelSubscription = &Endpoint{
@@ -20,39 +23,69 @@ var editChannelSubscription = &Endpoint{
 
 const subscriptionEditSuccess = "Your subscription has been edited successfully."
 
-func handleEditChannelSubscription(w http.ResponseWriter, r *http.Request, _ *Plugin) {
+func handleEditChannelSubscription(w http.ResponseWriter, r *http.Request, p *Plugin) {
 	params := mux.Vars(r)
 	channelID := params["channelID"]
 	subscriptionType := params["type"]
 	userID := r.Header.Get(config.HeaderMattermostUserID)
 	var subscription serializer.Subscription
 	var err error
-	switch subscriptionType {
-	case serializer.SubscriptionTypeSpace:
-		subscription, err = serializer.SpaceSubscriptionFromJSON(r.Body)
+
+	if !p.hasChannelAccess(userID, channelID) {
+		p.client.Log.Error("User does not have access to edit subscription for this channel", "UserID", userID, "ChannelID", channelID)
+		http.Error(w, "user does not have access to this channel", http.StatusForbidden)
+		return
+	}
+
+	pluginConfig := config.GetConfig()
+	if pluginConfig.ServerVersionGreaterthan9 {
+		var conn *types.Connection
+		conn, err = store.LoadConnection(pluginConfig.ConfluenceURL, userID)
 		if err != nil {
-			config.Mattermost.LogError("Error decoding request body.", "Error", err.Error())
-			http.Error(w, "Could not decode request body", http.StatusBadRequest)
+			if strings.Contains(err.Error(), "not found") {
+				http.Error(w, "User not connected to Confluence", http.StatusUnauthorized)
+				return
+			}
+
+			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
-	case serializer.SubscriptionTypePage:
-		subscription, err = serializer.PageSubscriptionFromJSON(r.Body)
-		if err != nil {
-			config.Mattermost.LogError("Error decoding request body.", "Error", err.Error())
-			http.Error(w, "Could not decode request body", http.StatusBadRequest)
+
+		if len(conn.ConfluenceAccountID()) == 0 {
+			http.Error(w, "User not connected to Confluence", http.StatusUnauthorized)
 			return
 		}
 	}
-	if err := service.EditSubscription(subscription); err != nil {
-		config.Mattermost.LogError(err.Error())
-		http.Error(w, "Failed to edit subscription", http.StatusBadRequest)
+
+	switch subscriptionType {
+	case serializer.SubscriptionTypeSpace:
+		subscription, err = serializer.SpaceSubscriptionFromJSON(r.Body)
+	case serializer.SubscriptionTypePage:
+		subscription, err = serializer.PageSubscriptionFromJSON(r.Body)
+	default:
+		p.client.Log.Error("Error updating channel subscription", "Subscription Type", subscriptionType, "error", "Invalid subscription type")
+		http.Error(w, "Invalid subscription type", http.StatusBadRequest)
+		return
+	}
+
+	if err != nil {
+		config.Mattermost.LogError("Error decoding request body.", "Error", err.Error())
+		http.Error(w, "Could not decode request body", http.StatusBadRequest)
+		return
+	}
+
+	if nErr := service.EditSubscription(subscription); nErr != nil {
+		config.Mattermost.LogError(nErr.Error())
+		http.Error(w, nErr.Error(), http.StatusBadRequest)
 		return
 	}
+
 	post := &model.Post{
 		UserId:    config.BotUserID,
 		ChannelId: channelID,
 		Message:   subscriptionEditSuccess,
 	}
+
 	_ = config.Mattermost.SendEphemeralPost(userID, post)
 	ReturnStatusOK(w)
 }

server/get_subscription.go

@@ -3,10 +3,13 @@ package main
 import (
 	"encoding/json"
 	"net/http"
+	"strings"
 
 	"github.com/gorilla/mux"
 
+	"github.com/mattermost/mattermost-plugin-confluence/server/config"
 	"github.com/mattermost/mattermost-plugin-confluence/server/service"
+	"github.com/mattermost/mattermost-plugin-confluence/server/store"
 )
 
 var getChannelSubscription = &Endpoint{
@@ -18,7 +21,34 @@ var getChannelSubscription = &Endpoint{
 func handleGetChannelSubscription(w http.ResponseWriter, r *http.Request, p *Plugin) {
 	params := mux.Vars(r)
 	channelID := params["channelID"]
+	userID := params["userID"]
 	alias := r.FormValue("alias")
+
+	if !p.hasChannelAccess(userID, channelID) {
+		p.client.Log.Error("User does not have access to get subscription for this channel", "UserID", userID, "ChannelID", channelID)
+		http.Error(w, "user does not have access to this channel", http.StatusForbidden)
+		return
+	}
+
+	pluginConfig := config.GetConfig()
+	if pluginConfig.ServerVersionGreaterthan9 {
+		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, userID)
+		if err != nil {
+			if strings.Contains(err.Error(), "not found") {
+				http.Error(w, "User not connected to Confluence", http.StatusUnauthorized)
+				return
+			}
+
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+
+		if len(conn.ConfluenceAccountID()) == 0 {
+			http.Error(w, "User not connected to Confluence", http.StatusUnauthorized)
+			return
+		}
+	}
+
 	subscription, errCode, err := service.GetChannelSubscription(channelID, alias)
 	if err != nil {
 		p.client.Log.Error("Error getting subscription for the channel", "ChannelID", channelID, "Subscription Alias", alias, "error", err.Error())

server/get_subscriptions.go

@@ -54,6 +54,11 @@ func handleGetChannelSubscriptions(w http.ResponseWriter, r *http.Request, p *Pl
 	}
 
 	channelID := r.FormValue("channel_id")
+	if _, err := p.API.GetChannel(channelID); err != nil {
+		http.Error(w, "invalid channel ID", http.StatusBadRequest)
+		return
+	}
+
 	subscriptions, err := service.GetSubscriptionsByChannelID(channelID)
 	if err != nil {
 		p.client.Log.Error("Error getting subscriptions for the channel", "ChannelID", channelID, "error", err.Error())

server/save_subscription.go

@@ -2,12 +2,14 @@ package main
 
 import (
 	"net/http"
+	"strings"
 
 	"github.com/gorilla/mux"
 
 	"github.com/mattermost/mattermost-plugin-confluence/server/config"
 	"github.com/mattermost/mattermost-plugin-confluence/server/serializer"
 	"github.com/mattermost/mattermost-plugin-confluence/server/service"
+	"github.com/mattermost/mattermost-plugin-confluence/server/store"
 
 	"github.com/mattermost/mattermost/server/public/model"
 )
@@ -20,39 +22,68 @@ var saveChannelSubscription = &Endpoint{
 	Execute: handleSaveSubscription,
 }
 
-func handleSaveSubscription(w http.ResponseWriter, r *http.Request, _ *Plugin) {
+func handleSaveSubscription(w http.ResponseWriter, r *http.Request, p *Plugin) {
 	params := mux.Vars(r)
 	channelID := params["channelID"]
 	subscriptionType := params["type"]
 	userID := r.Header.Get(config.HeaderMattermostUserID)
 	var subscription serializer.Subscription
 	var err error
+
+	if !p.hasChannelAccess(userID, channelID) {
+		p.client.Log.Error("User does not have access to create subscription for this channel", "UserID", userID, "ChannelID", channelID)
+		http.Error(w, "user does not have access to this channel", http.StatusForbidden)
+		return
+	}
+
 	switch subscriptionType {
 	case serializer.SubscriptionTypeSpace:
 		subscription, err = serializer.SpaceSubscriptionFromJSON(r.Body)
-		if err != nil {
-			config.Mattermost.LogError("Error decoding request body.", "Error", err.Error())
-			http.Error(w, "Could not decode request body", http.StatusBadRequest)
-			return
-		}
 	case serializer.SubscriptionTypePage:
 		subscription, err = serializer.PageSubscriptionFromJSON(r.Body)
+	default:
+		p.client.Log.Error("Invalid subscription type", "Subscription Type", subscriptionType)
+		http.Error(w, "Invalid subscription type", http.StatusBadRequest)
+		return
+	}
+
+	if err != nil {
+		config.Mattermost.LogError("Error decoding request body.", "Error", err.Error())
+		http.Error(w, "Could not decode request body", http.StatusBadRequest)
+		return
+	}
+
+	pluginConfig := config.GetConfig()
+	if pluginConfig.ServerVersionGreaterthan9 {
+		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, userID)
 		if err != nil {
-			config.Mattermost.LogError("Error decoding request body.", "Error", err.Error())
-			http.Error(w, "Could not decode request body", http.StatusBadRequest)
+			if strings.Contains(err.Error(), "not found") {
+				http.Error(w, "User not connected to Confluence", http.StatusUnauthorized)
+				return
+			}
+
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+
+		if len(conn.ConfluenceAccountID()) == 0 {
+			http.Error(w, "User not connected to Confluence", http.StatusUnauthorized)
 			return
 		}
 	}
+
 	if statusCode, sErr := service.SaveSubscription(subscription); sErr != nil {
 		config.Mattermost.LogError("Error occurred while saving subscription", "Subscription Name", subscription.Name(), "error", sErr.Error())
 		http.Error(w, "Failed to save subscription", statusCode)
 		return
 	}
+
 	post := &model.Post{
 		UserId:    config.BotUserID,
 		ChannelId: channelID,
 		Message:   subscriptionSaveSuccess,
 	}
+
 	_ = config.Mattermost.SendEphemeralPost(userID, post)
 	ReturnStatusOK(w)
 }

server/serializer/confluence_cloud.go

@@ -60,12 +60,13 @@ type ParentComment struct {
 	ID string `json:"id"`
 }
 
-func ConfluenceCloudEventFromJSON(data io.Reader) *ConfluenceCloudEvent {
+func ConfluenceCloudEventFromJSON(data io.Reader) (*ConfluenceCloudEvent, error) {
 	var confluenceCloudEvent ConfluenceCloudEvent
 	if err := json.NewDecoder(data).Decode(&confluenceCloudEvent); err != nil {
 		config.Mattermost.LogError("Unable to decode JSON for ConfluenceServerEvent.", "Error", err.Error())
+		return nil, err
 	}
-	return &confluenceCloudEvent
+	return &confluenceCloudEvent, nil
 }
 
 func (e ConfluenceCloudEvent) GetNotificationPost(eventType string) *model.Post {

server/service/delete_subscription.go

@@ -18,18 +18,21 @@ func DeleteSubscription(channelID, alias string) error {
 	if gErr != nil {
 		return fmt.Errorf(generalDeleteError, alias)
 	}
+
 	if channelSubscriptions, valid := subs.ByChannelID[channelID]; valid {
 		if subscription, ok := channelSubscriptions.GetInsensitiveCase(alias); ok {
 			aErr := store.AtomicModify(store.GetSubscriptionKey(), func(initialBytes []byte) ([]byte, error) {
 				subscriptions, err := serializer.SubscriptionsFromJSON(initialBytes)
 				if err != nil {
 					return nil, err
 				}
+
 				subscription.Remove(subscriptions)
 				modifiedBytes, marshalErr := json.Marshal(subscriptions)
 				if marshalErr != nil {
 					return nil, marshalErr
 				}
+
 				return modifiedBytes, nil
 			})
 			return aErr

server/user.go

@@ -392,3 +392,8 @@ func httpGetUserInfo(w http.ResponseWriter, r *http.Request, p *Plugin) {
 	w.Header().Set("Content-Type", "application/json")
 	_, _ = w.Write([]byte(string(b)))
 }
+
+func (p *Plugin) hasChannelAccess(userID, channelID string) bool {
+	_, err := p.API.GetChannelMember(channelID, userID)
+	return err == nil
+}