Let connected non-admins manage channel subscriptions on Confluence

server/client.go

@@ -1,6 +1,38 @@
 package main
 
-import "github.com/mattermost/mattermost-plugin-confluence/server/util/types"
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/pkg/errors"
+
+	"github.com/mattermost/mattermost-plugin-confluence/server/util/types"
+)
+
+type APIError struct {
+	StatusCode int
+	Path       string
+}
+
+func (e *APIError) Error() string {
+	return fmt.Sprintf("confluence request for %s returned %d", e.Path, e.StatusCode)
+}
+
+func (e *APIError) IsAccessDenied() bool {
+	switch e.StatusCode {
+	case http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound:
+		return true
+	default:
+		return false
+	}
+}
+
+func withAPIStatus(err error, path string, statusCode int) error {
+	if err == nil || statusCode == 0 {
+		return err
+	}
+	return errors.Wrap(&APIError{StatusCode: statusCode, Path: path}, err.Error())
+}
 
 // Client is the combined interface for all upstream APIs and convenience methods.
 type Client interface {

server/client_cloud.go

@@ -5,10 +5,13 @@ package main
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
+	"net/url"
 
 	"github.com/pkg/errors"
 
+	"github.com/mattermost/mattermost-plugin-confluence/server/util"
 	"github.com/mattermost/mattermost-plugin-confluence/server/util/types"
 )
 
@@ -30,29 +33,44 @@ type cloudCurrentUser struct {
 	DisplayName string `json:"displayName"`
 }
 
-// GetSelf returns the authenticated user via the Atlassian platform "me"
-// endpoint. The Confluence v2 user endpoints require a known accountId; "me"
-// is the canonical way to discover that with just an OAuth token.
-func (c *confluenceCloudClient) GetSelf() (*types.ConfluenceUser, error) {
-	req, err := http.NewRequest(http.MethodGet, c.APIBase+"/wiki/rest/api/user/current", nil)
+// v1 REST paths: the plugin's classic OAuth scopes do not authorize the v2 API,
+// which would need a re-consent from every existing install.
+const cloudAPIPrefix = "/wiki"
+
+// getJSON GETs path relative to the Cloud API base and decodes the JSON
+// response body into out. A non-200 response yields an *APIError so callers can
+// act on the upstream status.
+func (c *confluenceCloudClient) getJSON(path string, out interface{}) error {
+	req, err := http.NewRequest(http.MethodGet, c.APIBase+cloudAPIPrefix+path, nil)
 	if err != nil {
-		return nil, errors.Wrap(err, "build cloud GetSelf request")
+		return errors.Wrapf(err, "build cloud request for %s", path)
 	}
 	req.Header.Set("Accept", "application/json")
 
 	resp, err := c.HTTPClient.Do(req)
 	if err != nil {
-		return nil, errors.Wrap(err, "cloud GetSelf request failed")
+		return errors.Wrapf(err, "cloud request for %s failed", path)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, errors.Errorf("cloud GetSelf returned %d", resp.StatusCode)
+		return &APIError{StatusCode: resp.StatusCode, Path: path}
 	}
 
+	if err := json.NewDecoder(resp.Body).Decode(out); err != nil {
+		return errors.Wrapf(err, "decode cloud response for %s", path)
+	}
+
+	return nil
+}
+
+// GetSelf returns the authenticated user via the Atlassian platform "me"
+// endpoint. The Confluence v2 user endpoints require a known accountId; "me"
+// is the canonical way to discover that with just an OAuth token.
+func (c *confluenceCloudClient) GetSelf() (*types.ConfluenceUser, error) {
 	var u cloudCurrentUser
-	if err := json.NewDecoder(resp.Body).Decode(&u); err != nil {
-		return nil, errors.Wrap(err, "decode cloud GetSelf response")
+	if err := c.getJSON(PathCurrentUser, &u); err != nil {
+		return nil, errors.Wrap(err, "Confluence GetSelf. Error getting the current user")
 	}
 
 	name := u.PublicName
@@ -66,20 +84,27 @@ func (c *confluenceCloudClient) GetSelf() (*types.ConfluenceUser, error) {
 	}, nil
 }
 
-// GetSpaceData / GetPageData / GetSpaceKeyFromSpaceID are only invoked by the
-// ServerVersionGreaterthan9 subscription-validation path (see
-// validateUserConfluenceAccess in user.go) which does not fire for Cloud
-// installs. They stay stubbed until Cloud subscriptions need server-side
-// validation against Confluence Cloud REST v2.
+func (c *confluenceCloudClient) GetSpaceData(spaceKey string) (*SpaceResponse, error) {
+	spaceResponse := &SpaceResponse{}
+	if err := c.getJSON(fmt.Sprintf("%s%s?status=any", PathSpaceData, url.PathEscape(spaceKey)), spaceResponse); err != nil {
+		return nil, err
+	}
 
-func (c *confluenceCloudClient) GetSpaceData(string) (*SpaceResponse, error) {
-	return nil, errors.New("GetSpaceData is not implemented for Confluence Cloud")
+	return spaceResponse, nil
 }
 
-func (c *confluenceCloudClient) GetPageData(int) (*PageResponse, error) {
-	return nil, errors.New("GetPageData is not implemented for Confluence Cloud")
+func (c *confluenceCloudClient) GetPageData(pageID int) (*PageResponse, error) {
+	pageResponse := &PageResponse{}
+	if err := c.getJSON(fmt.Sprintf("%s%d?status=any&expand=body.view,space,history", PathContentData, pageID), pageResponse); err != nil {
+		return nil, err
+	}
+
+	pageResponse.Body.View.Value = util.GetBodyForExcerpt(pageResponse.Body.View.Value)
+
+	return pageResponse, nil
 }
 
+// Cloud events arrive through the Forge bridge already carrying the space key.
 func (c *confluenceCloudClient) GetSpaceKeyFromSpaceID(int64) (string, error) {
 	return "", errors.New("GetSpaceKeyFromSpaceID is not implemented for Confluence Cloud")
 }

server/client_cloud_test.go

@@ -0,0 +1,55 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCloudClientGetSpaceData(t *testing.T) {
+	var requestedPath string
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requestedPath = r.URL.RequestURI()
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"id":1,"key":"MM","name":"Mattermost"}`))
+	}))
+	defer ts.Close()
+
+	client := newCloudClient(ts.URL, ts.Client())
+	space, err := client.GetSpaceData("MM")
+
+	require.NoError(t, err)
+	assert.Equal(t, "MM", space.Key)
+	assert.Equal(t, "/wiki/rest/api/space/MM?status=any", requestedPath)
+}
+
+func TestCloudClientGetSpaceDataForbidden(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+	}))
+	defer ts.Close()
+
+	_, err := newCloudClient(ts.URL, ts.Client()).GetSpaceData("MM")
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "403")
+}
+
+func TestCloudClientGetPageData(t *testing.T) {
+	var requestedPath string
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requestedPath = r.URL.Path
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"id":"12345","title":"Release notes"}`))
+	}))
+	defer ts.Close()
+
+	page, err := newCloudClient(ts.URL, ts.Client()).GetPageData(12345)
+
+	require.NoError(t, err)
+	assert.Equal(t, "Release notes", page.Title)
+	assert.Equal(t, "/wiki/rest/api/content/12345", requestedPath)
+}

server/client_server.go

@@ -170,8 +170,9 @@ func (csc *confluenceServerClient) GetCommentData(webhookPayload *serializer.Con
 
 func (csc *confluenceServerClient) GetPageData(pageID int) (*PageResponse, error) {
 	pageResponse := &PageResponse{}
-	if _, _, err := service.CallJSONWithURL(csc.URL, fmt.Sprintf("%s%s?status=any&expand=body.view,container,space,history", PathContentData, strconv.Itoa(pageID)), http.MethodGet, nil, pageResponse, csc.HTTPClient); err != nil {
-		return nil, err
+	path := fmt.Sprintf("%s%s?status=any&expand=body.view,container,space,history", PathContentData, strconv.Itoa(pageID))
+	if _, statusCode, err := service.CallJSONWithURL(csc.URL, path, http.MethodGet, nil, pageResponse, csc.HTTPClient); err != nil {
+		return nil, withAPIStatus(err, path, statusCode)
 	}
 
 	pageResponse.Body.View.Value = util.GetBodyForExcerpt(pageResponse.Body.View.Value)
@@ -181,8 +182,9 @@ func (csc *confluenceServerClient) GetPageData(pageID int) (*PageResponse, error
 
 func (csc *confluenceServerClient) GetSpaceData(spaceKey string) (*SpaceResponse, error) {
 	spaceResponse := &SpaceResponse{}
-	if _, _, err := service.CallJSONWithURL(csc.URL, fmt.Sprintf("%s%s?status=any", PathSpaceData, spaceKey), http.MethodGet, nil, spaceResponse, csc.HTTPClient); err != nil {
-		return nil, err
+	path := fmt.Sprintf("%s%s?status=any", PathSpaceData, spaceKey)
+	if _, statusCode, err := service.CallJSONWithURL(csc.URL, path, http.MethodGet, nil, spaceResponse, csc.HTTPClient); err != nil {
+		return nil, withAPIStatus(err, path, statusCode)
 	}
 
 	return spaceResponse, nil

server/command.go

@@ -287,31 +287,11 @@ func deleteSubscription(p *Plugin, context *model.CommandArgs, args ...string) *
 	userID := context.UserId
 	channelID := context.ChannelId
 
-	if !util.IsSystemAdmin(userID) {
-		postCommandResponse(context, commandsOnlySystemAdmin)
+	if access := p.checkSubscriptionAccess(userID); !access.Allowed {
+		postCommandResponse(context, access.Message)
 		return &model.CommandResponse{}
 	}
 
-	pluginConfig := config.GetConfig()
-	if pluginConfig.ServerVersionGreaterthan9 {
-		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, userID)
-		if err != nil {
-			if strings.Contains(err.Error(), "not found") {
-				postCommandResponse(context, disconnectedUser)
-				return &model.CommandResponse{}
-			}
-
-			p.client.Log.Error("Error loading the connection for the user", "UserID", context.UserId, "error", err.Error())
-			postCommandResponse(context, errorExecutingCommand)
-			return &model.CommandResponse{}
-		}
-
-		if len(conn.ConfluenceAccountID()) == 0 {
-			postCommandResponse(context, disconnectedUser)
-			return &model.CommandResponse{}
-		}
-	}
-
 	if len(args) == 0 {
 		postCommandResponse(context, specifyAlias)
 		return &model.CommandResponse{}
@@ -334,26 +314,8 @@ func deleteSubscription(p *Plugin, context *model.CommandArgs, args ...string) *
 }
 
 func listChannelSubscription(p *Plugin, context *model.CommandArgs, _ ...string) *model.CommandResponse {
-	pluginConfig := config.GetConfig()
-	if pluginConfig.ServerVersionGreaterthan9 {
-		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, context.UserId)
-		if err != nil {
-			if strings.Contains(err.Error(), "not found") {
-				postCommandResponse(context, disconnectedUser)
-				return &model.CommandResponse{}
-			}
-
-			p.client.Log.Error("Error loading the connection for the user", "UserID", context.UserId, "error", err.Error())
-			postCommandResponse(context, errorExecutingCommand)
-			return &model.CommandResponse{}
-		}
-
-		if len(conn.ConfluenceAccountID()) == 0 {
-			postCommandResponse(context, disconnectedUser)
-			return &model.CommandResponse{}
-		}
-	} else if !util.IsSystemAdmin(context.UserId) {
-		postCommandResponse(context, commandsOnlySystemAdmin)
+	if access := p.checkSubscriptionAccess(context.UserId); !access.Allowed {
+		postCommandResponse(context, access.Message)
 		return &model.CommandResponse{}
 	}
 

server/config/main.go

@@ -80,6 +80,10 @@ func (c *Configuration) Sanitize() {
 	c.ConfluenceOAuthClientSecret = strings.TrimSpace(c.ConfluenceOAuthClientSecret)
 }
 
+func (c *Configuration) HasPerUserConfluenceAuth() bool {
+	return c.IsCloud || c.ServerVersionGreaterthan9
+}
+
 func (c *Configuration) IsOAuthConfigured() bool {
 	return (c.ConfluenceOAuthClientID != "" && c.ConfluenceOAuthClientSecret != "")
 }

server/edit_subscription.go

@@ -11,7 +11,6 @@ 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/util"
 )
 
 var editChannelSubscription = &Endpoint{
@@ -31,9 +30,9 @@ func handleEditChannelSubscription(w http.ResponseWriter, r *http.Request, p *Pl
 	var subscription serializer.Subscription
 	var err error
 
-	if !util.IsSystemAdmin(userID) {
-		p.client.Log.Error("Non admin user does not have access to edit subscription for this channel", "UserID", userID, "ChannelID", channelID)
-		http.Error(w, "only system admin can edit a subscription", http.StatusForbidden)
+	if access := p.checkSubscriptionAccess(userID); !access.Allowed {
+		p.client.Log.Error("User does not have access to edit subscription for this channel", "UserID", userID, "ChannelID", channelID, "reason", access.Reason)
+		http.Error(w, access.Message, access.StatusCode)
 		return
 	}
 
@@ -61,7 +60,7 @@ func handleEditChannelSubscription(w http.ResponseWriter, r *http.Request, p *Pl
 	}
 
 	pluginConfig := config.GetConfig()
-	if pluginConfig.ServerVersionGreaterthan9 {
+	if pluginConfig.HasPerUserConfluenceAuth() {
 		var statusCode int
 		if statusCode, err = p.validateUserConfluenceAccess(userID, pluginConfig.ConfluenceURL, subscriptionType, subscription); err != nil {
 			p.client.Log.Error("Error validating the user's Confluence access", "Error", err.Error())

server/get_subscription.go

@@ -3,14 +3,11 @@ 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"
-	"github.com/mattermost/mattermost-plugin-confluence/server/util"
 )
 
 var getChannelSubscription = &Endpoint{
@@ -26,9 +23,9 @@ func handleGetChannelSubscription(w http.ResponseWriter, r *http.Request, p *Plu
 	userID := r.Header.Get(config.HeaderMattermostUserID)
 	alias := r.FormValue("alias")
 
-	if !util.IsSystemAdmin(userID) {
-		p.client.Log.Error("Non admin user does not have access to fetch subscription for this channel", "UserID", userID, "ChannelID", channelID)
-		http.Error(w, "only system admin can fetch a subscription", http.StatusForbidden)
+	if access := p.checkSubscriptionAccess(userID); !access.Allowed {
+		p.client.Log.Error("User does not have access to fetch subscription for this channel", "UserID", userID, "ChannelID", channelID, "reason", access.Reason)
+		http.Error(w, access.Message, access.StatusCode)
 		return
 	}
 
@@ -38,27 +35,6 @@ func handleGetChannelSubscription(w http.ResponseWriter, r *http.Request, p *Plu
 		return
 	}
 
-	pluginConfig := config.GetConfig()
-	if pluginConfig.ServerVersionGreaterthan9 {
-		conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, userID)
-		if err != nil {
-			if strings.Contains(err.Error(), "not found") {
-				p.client.Log.Info("User not connected to Confluence. UserID: %s. Error: %s", userID, err.Error())
-				http.Error(w, "User not connected to Confluence.", http.StatusUnauthorized)
-				return
-			}
-			p.client.Log.Error("Error loading Confluence connection. UserID: %s. Error: %s", userID, err.Error())
-			http.Error(w, "An error occurred while verifying user's Confluence connection.", http.StatusInternalServerError)
-			return
-		}
-
-		if len(conn.ConfluenceAccountID()) == 0 {
-			p.client.Log.Error("User not connected to Confluence. UserID: %s", userID)
-			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: %s, Alias: %s. Error: %s", channelID, alias, err.Error())