diff --git a/server/client.go b/server/client.go index 87b3dd27..b939baf0 100644 --- a/server/client.go +++ b/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 { diff --git a/server/client_cloud.go b/server/client_cloud.go index 6a987cf7..29cab00f 100644 --- a/server/client_cloud.go +++ b/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") } diff --git a/server/client_cloud_test.go b/server/client_cloud_test.go new file mode 100644 index 00000000..871e7dfa --- /dev/null +++ b/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) +} diff --git a/server/client_server.go b/server/client_server.go index 9efa842b..58f987f8 100644 --- a/server/client_server.go +++ b/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 diff --git a/server/command.go b/server/command.go index 99168534..3ec3ee35 100644 --- a/server/command.go +++ b/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{} } diff --git a/server/config/main.go b/server/config/main.go index 63109987..9b35d3ca 100644 --- a/server/config/main.go +++ b/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 != "") } diff --git a/server/edit_subscription.go b/server/edit_subscription.go index f348a746..4841c012 100644 --- a/server/edit_subscription.go +++ b/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()) diff --git a/server/get_subscription.go b/server/get_subscription.go index fa6f7da4..c46547dd 100644 --- a/server/get_subscription.go +++ b/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()) diff --git a/server/get_subscriptions.go b/server/get_subscriptions.go index acc70035..c79c930d 100644 --- a/server/get_subscriptions.go +++ b/server/get_subscriptions.go @@ -3,12 +3,9 @@ package main import ( "encoding/json" "net/http" - "strings" "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" "github.com/mattermost/mattermost/server/public/model" ) @@ -23,38 +20,15 @@ var autocompleteGetChannelSubscriptions = &Endpoint{ func handleGetChannelSubscriptions(w http.ResponseWriter, r *http.Request, p *Plugin) { mattermostUserID := r.Header.Get(config.HeaderMattermostUserID) - if !util.IsSystemAdmin(mattermostUserID) { - p.client.Log.Error("Non admin user does not have access to fetch subscription list", "UserID", mattermostUserID) - http.Error(w, "only system admin can fetch subscription list", http.StatusForbidden) + // Autocomplete cannot surface an error, so denied users get no suggestions. + if access := p.checkSubscriptionAccess(mattermostUserID); !access.Allowed { + p.client.Log.Debug("User does not have access to fetch subscription list", "UserID", mattermostUserID, "reason", access.Reason) + b, _ := json.Marshal([]model.AutocompleteListItem{}) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(b) return } - pluginConfig := config.GetConfig() - if pluginConfig.ServerVersionGreaterthan9 { - conn, err := store.LoadConnection(pluginConfig.ConfluenceURL, mattermostUserID) - if err != nil { - if strings.Contains(err.Error(), "not found") { - out := []model.AutocompleteListItem{} - b, _ := json.Marshal(out) - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(b) - return - } - - p.client.Log.Error("Error loading Confluence connection.", "UserID", mattermostUserID, "Error", err.Error()) - http.Error(w, "Unable to fetch user's Confluence connection.", http.StatusInternalServerError) - return - } - - if len(conn.ConfluenceAccountID()) == 0 { - out := []model.AutocompleteListItem{} - b, _ := json.Marshal(out) - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(b) - return - } - } - channelID := r.FormValue("channel_id") if _, err := p.API.GetChannel(channelID); err != nil { p.client.Log.Error("Invalid channel ID. ChannelID: %s. Error: %s", channelID, err.Error()) diff --git a/server/instance_cloud.go b/server/instance_cloud.go index 334ccf1f..58d904c3 100644 --- a/server/instance_cloud.go +++ b/server/instance_cloud.go @@ -14,6 +14,7 @@ import ( "golang.org/x/oauth2" "github.com/mattermost/mattermost-plugin-confluence/server/config" + "github.com/mattermost/mattermost-plugin-confluence/server/store" "github.com/mattermost/mattermost-plugin-confluence/server/util" "github.com/mattermost/mattermost-plugin-confluence/server/util/types" ) @@ -126,3 +127,54 @@ func (p *Plugin) GetCloudClient(instanceURL, cloudID string, connection *types.C return newCloudClient(fmt.Sprintf(cloudAPIBaseFmt, cloudID), httpClient), nil } + +func (p *Plugin) GetClient(instanceID, mattermostUserID string, connection *types.Connection) (Client, error) { + if !config.GetConfig().IsCloud { + return p.GetServerClient(instanceID, connection) + } + + cloudID, err := p.resolveCloudID(instanceID, mattermostUserID, connection) + if err != nil { + return nil, err + } + + return p.GetCloudClient(instanceID, cloudID, connection) +} + +// Backfills connections stored before the cloudId was persisted, so existing +// users do not have to reconnect. +func (p *Plugin) resolveCloudID(instanceID, mattermostUserID string, connection *types.Connection) (string, error) { + if connection.CloudID != "" { + return connection.CloudID, nil + } + + oconf, err := p.GetCloudOAuth2Config(connection.IsAdmin) + if err != nil { + return "", err + } + + token, err := p.refreshAndStoreToken(connection, instanceID, oconf) + if err != nil { + return "", err + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + resources, err := p.GetCloudAccessibleResources(ctx, token) + if err != nil { + return "", errors.Wrap(err, "cloud accessible-resources lookup failed") + } + + cloudID := matchCloudResource(resources, instanceID) + if cloudID == "" { + return "", errors.Errorf("authenticated user has no access to %s", instanceID) + } + + connection.CloudID = cloudID + if err := store.StoreConnection(instanceID, mattermostUserID, connection); err != nil { + p.client.Log.Warn("Failed to persist the resolved Confluence cloud ID", "UserID", mattermostUserID, "error", err.Error()) + } + + return cloudID, nil +} diff --git a/server/save_subscription.go b/server/save_subscription.go index 0b01de76..dd20f554 100644 --- a/server/save_subscription.go +++ b/server/save_subscription.go @@ -9,7 +9,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" "github.com/mattermost/mattermost/server/public/model" ) @@ -30,9 +29,9 @@ func handleSaveSubscription(w http.ResponseWriter, r *http.Request, p *Plugin) { userID := r.Header.Get(config.HeaderMattermostUserID) var subscription serializer.Subscription - if !util.IsSystemAdmin(userID) { - p.client.Log.Error("Non admin user does not have access to create subscription for this channel", "UserID", userID, "ChannelID", channelID) - http.Error(w, "only system admin can save a subscription", http.StatusForbidden) + if access := p.checkSubscriptionAccess(userID); !access.Allowed { + p.client.Log.Error("User does not have access to create subscription for this channel", "UserID", userID, "ChannelID", channelID, "reason", access.Reason) + http.Error(w, access.Message, access.StatusCode) return } @@ -60,7 +59,7 @@ func handleSaveSubscription(w http.ResponseWriter, r *http.Request, p *Plugin) { } pluginConfig := config.GetConfig() - if pluginConfig.ServerVersionGreaterthan9 { + if pluginConfig.HasPerUserConfluenceAuth() { 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()) http.Error(w, err.Error(), statusCode) // safe to return the error string directly, as this function ensures all returned errors are user-friendly diff --git a/server/user.go b/server/user.go index bd631e0d..fa6fd45d 100644 --- a/server/user.go +++ b/server/user.go @@ -234,6 +234,7 @@ func (p *Plugin) completeCloudOAuth2(mmuser *model.User, mattermostUserID, code, OAuth2Token: encryptedToken, IsAdmin: isAdmin, MattermostUserID: mattermostUserID, + CloudID: cloudID, } client, err := p.GetCloudClient(instanceID, cloudID, connection) @@ -414,61 +415,95 @@ func (p *Plugin) refreshAndStoreToken(connection *types.Connection, instanceID s } type UserConnectionInfo struct { - CanRunSubscribeCommand bool `json:"can_run_subscribe_command"` - ServerVersionGreaterthan9 bool `json:"server_version_greater_than_9"` + CanRunSubscribeCommand bool `json:"can_run_subscribe_command"` + SubscribeDeniedReason string `json:"subscribe_denied_reason,omitempty"` } -func httpGetUserInfo(w http.ResponseWriter, r *http.Request, p *Plugin) { - if r.Method != http.MethodGet { - err := errors.New("method " + r.Method + " is not allowed, must be GET") - p.client.Log.Error("Invalid HTTP method used in GetUserInfo. Error: %s", err.Error()) - _, _ = respondErr(w, http.StatusMethodNotAllowed, err) - return - } +// Mirrored in webapp/src/constants. +const ( + subscribeDeniedAdminOnly = "admin_only" + subscribeDeniedNotConnected = "not_connected" + subscribeDeniedError = "error" +) - mattermostUserID := r.Header.Get(config.HeaderMattermostUserID) - serverVersionGreaterThan9 := config.GetConfig().ServerVersionGreaterthan9 +type subscriptionAccess struct { + Allowed bool + Reason string + Message string + StatusCode int +} - if !serverVersionGreaterThan9 { - info := &UserConnectionInfo{ - CanRunSubscribeCommand: util.IsSystemAdmin(mattermostUserID), - ServerVersionGreaterthan9: serverVersionGreaterThan9, +// Admins always pass. Where each user authenticates against Confluence +// individually, so does any connected user, with their Confluence permissions +// enforced per-subscription by validateUserConfluenceAccess. +func (p *Plugin) checkSubscriptionAccess(mattermostUserID string) subscriptionAccess { + if util.IsSystemAdmin(mattermostUserID) { + return subscriptionAccess{Allowed: true, StatusCode: http.StatusOK} + } + + pluginConfig := config.GetConfig() + if !pluginConfig.HasPerUserConfluenceAuth() { + return subscriptionAccess{ + Reason: subscribeDeniedAdminOnly, + Message: commandsOnlySystemAdmin, + StatusCode: http.StatusForbidden, } - b, _ := json.Marshal(info) - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(b) - return } - instanceURL := config.GetConfig().GetConfluenceBaseURL() + internalError := subscriptionAccess{ + Reason: subscribeDeniedError, + Message: errorExecutingCommand, + StatusCode: http.StatusInternalServerError, + } + + instanceURL := pluginConfig.GetConfluenceBaseURL() if instanceURL == "" { - err := errors.New("missing Confluence base URL") - p.client.Log.Error("Confluence base URL is not configured. Error: %s", err.Error()) - http.Error(w, "Confluence is not properly configured. Please contact the system administrator.", http.StatusInternalServerError) - return + p.client.Log.Error("Confluence base URL is not configured") + return internalError + } + + notConnected := subscriptionAccess{ + Reason: subscribeDeniedNotConnected, + Message: disconnectedUser, + StatusCode: http.StatusUnauthorized, } connection, err := store.LoadConnection(instanceURL, mattermostUserID) if err != nil { if strings.Contains(err.Error(), "not found") { - info := &UserConnectionInfo{ - CanRunSubscribeCommand: false, - ServerVersionGreaterthan9: serverVersionGreaterThan9, - } - b, _ := json.Marshal(info) - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(b) - return + return notConnected } - p.client.Log.Error("Failed to load user Confluence connection. MattermostUserID: %s. Error: %s", mattermostUserID, err.Error()) + p.client.Log.Error("Error loading the Confluence connection for the user", "UserID", mattermostUserID, "error", err.Error()) + return internalError + } + + if connection.ConfluenceAccountID() == "" { + return notConnected + } + + return subscriptionAccess{Allowed: true, StatusCode: http.StatusOK} +} + +func httpGetUserInfo(w http.ResponseWriter, r *http.Request, p *Plugin) { + if r.Method != http.MethodGet { + err := errors.New("method " + r.Method + " is not allowed, must be GET") + p.client.Log.Error("Invalid HTTP method used in GetUserInfo. Error: %s", err.Error()) + _, _ = respondErr(w, http.StatusMethodNotAllowed, err) + return + } + + mattermostUserID := r.Header.Get(config.HeaderMattermostUserID) + access := p.checkSubscriptionAccess(mattermostUserID) + + if access.StatusCode == http.StatusInternalServerError { http.Error(w, "Failed to retrieve user connection status. Please retry after some time.", http.StatusInternalServerError) return } info := &UserConnectionInfo{ - CanRunSubscribeCommand: len(connection.ConfluenceAccountID()) != 0, - ServerVersionGreaterthan9: serverVersionGreaterThan9, + CanRunSubscribeCommand: access.Allowed, + SubscribeDeniedReason: access.Reason, } b, _ := json.Marshal(info) @@ -481,6 +516,23 @@ func (p *Plugin) hasChannelAccess(userID, channelID string) bool { return err == nil } +func classifyConfluenceAccessError(err error, resource string) (int, error) { + unreachable := errors.Errorf("Confluence could not be reached to verify your access to this %s. Please try again later", resource) + + var apiErr *APIError + switch { + case !errors.As(err, &apiErr): + // Nothing came back from Confluence, so access is unproven, not refused. + return http.StatusBadGateway, unreachable + case apiErr.IsAccessDenied(): + return http.StatusForbidden, errors.Errorf("User does not have an access to this Confluence %s", resource) + case apiErr.StatusCode == http.StatusTooManyRequests: + return http.StatusTooManyRequests, errors.New("Confluence is rate limiting requests. Please try again in a few minutes") + default: + return http.StatusBadGateway, unreachable + } +} + func (p *Plugin) validateUserConfluenceAccess(userID, confluenceURL, subscriptionType string, subscription serializer.Subscription) (int, error) { conn, err := store.LoadConnection(confluenceURL, userID) if err != nil { @@ -495,18 +547,12 @@ func (p *Plugin) validateUserConfluenceAccess(userID, confluenceURL, subscriptio return http.StatusUnauthorized, errors.New("User needs to connect their Confluence account") } - client, err := p.GetServerClient(confluenceURL, conn) + client, err := p.GetClient(confluenceURL, userID, conn) if err != nil { p.client.Log.Error("Error getting Confluence client. UserID: %s. Error: %s", userID, err.Error()) return http.StatusInternalServerError, errors.New("An error occurred while connecting to Confluence. Please try again later") } - serverClient, ok := client.(*confluenceServerClient) - if !ok { - p.client.Log.Error("Invalid Confluence server client type while validating user's Confluence access. UserID: %s", userID) - return http.StatusInternalServerError, errors.New("an unexpected error occurred. Please try again later") - } - switch subscriptionType { case serializer.SubscriptionTypeSpace: spaceSub, ok := subscription.(serializer.SpaceSubscription) @@ -514,9 +560,9 @@ func (p *Plugin) validateUserConfluenceAccess(userID, confluenceURL, subscriptio p.client.Log.Error("Failed to parse space subscription. UserID: %s", userID) return http.StatusBadRequest, errors.New("invalid space subscription details provided") } - if _, err = serverClient.GetSpaceData(spaceSub.SpaceKey); err != nil { - p.client.Log.Error("User does not have access to the space. UserID: %s, SpaceKey: %s. Error: %s", userID, spaceSub.SpaceKey, err.Error()) - return http.StatusForbidden, errors.New("User does not have an access to this Confluence space") + if _, err = client.GetSpaceData(spaceSub.SpaceKey); err != nil { + p.client.Log.Error("Unable to confirm the user's access to the space. UserID: %s, SpaceKey: %s. Error: %s", userID, spaceSub.SpaceKey, err.Error()) + return classifyConfluenceAccessError(err, "space") } case serializer.SubscriptionTypePage: @@ -531,9 +577,9 @@ func (p *Plugin) validateUserConfluenceAccess(userID, confluenceURL, subscriptio return http.StatusInternalServerError, errors.New("an error occurred while processing the page details. Please try again later") } - if _, err := serverClient.GetPageData(pageID); err != nil { - p.client.Log.Error("User does not have access to the page. UserID: %s, PageID: %d. Error: %s", userID, pageID, err.Error()) - return http.StatusForbidden, errors.New("User does not have an access to this Confluence page") + if _, err := client.GetPageData(pageID); err != nil { + p.client.Log.Error("Unable to confirm the user's access to the page. UserID: %s, PageID: %d. Error: %s", userID, pageID, err.Error()) + return classifyConfluenceAccessError(err, "page") } default: diff --git a/server/user_test.go b/server/user_test.go new file mode 100644 index 00000000..44ecd926 --- /dev/null +++ b/server/user_test.go @@ -0,0 +1,140 @@ +package main + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/pkg/errors" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin/plugintest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-plugin-confluence/server/config" + "github.com/mattermost/mattermost-plugin-confluence/server/util/types" +) + +const testConfluenceURL = "https://test.atlassian.net" + +func marshalConnection(t *testing.T, connection *types.Connection) []byte { + t.Helper() + b, err := json.Marshal(connection) + require.NoError(t, err) + return b +} + +func TestClassifyConfluenceAccessError(t *testing.T) { + for name, tc := range map[string]struct { + err error + expectedStatusCode int + }{ + "not found is treated as hidden from the user": { + err: &APIError{StatusCode: http.StatusNotFound, Path: "/space/MM"}, + expectedStatusCode: http.StatusForbidden, + }, + "forbidden denies access": { + err: &APIError{StatusCode: http.StatusForbidden, Path: "/space/MM"}, + expectedStatusCode: http.StatusForbidden, + }, + "rate limit is retryable, not a denial": { + err: &APIError{StatusCode: http.StatusTooManyRequests, Path: "/space/MM"}, + expectedStatusCode: http.StatusTooManyRequests, + }, + "upstream outage is not a denial": { + err: &APIError{StatusCode: http.StatusServiceUnavailable, Path: "/space/MM"}, + expectedStatusCode: http.StatusBadGateway, + }, + "wrapped status is still recognized": { + err: errors.Wrap(&APIError{StatusCode: http.StatusBadGateway, Path: "/space/MM"}, "upstream failed"), + expectedStatusCode: http.StatusBadGateway, + }, + "transport failure without a response is not a denial": { + err: errors.New("dial tcp: connection refused"), + expectedStatusCode: http.StatusBadGateway, + }, + } { + t.Run(name, func(t *testing.T) { + statusCode, err := classifyConfluenceAccessError(tc.err, "space") + + assert.Equal(t, tc.expectedStatusCode, statusCode) + assert.Error(t, err) + }) + } +} + +func TestCheckSubscriptionAccess(t *testing.T) { + connected := marshalConnection(t, &types.Connection{ConfluenceUser: types.ConfluenceUser{AccountID: "confluence-account-id"}}) + connectedWithoutAccount := marshalConnection(t, &types.Connection{}) + + cloudConfig := &config.Configuration{ConfluenceURL: testConfluenceURL, IsCloud: true} + serverV9Config := &config.Configuration{ConfluenceURL: testConfluenceURL, ServerVersionGreaterthan9: true} + legacyServerConfig := &config.Configuration{ConfluenceURL: testConfluenceURL} + + for name, tc := range map[string]struct { + pluginConfig *config.Configuration + roles string + connection []byte + expectedAllowed bool + expectedReason string + }{ + "cloud, system admin without a connection": { + pluginConfig: cloudConfig, + roles: model.SystemAdminRoleId, + expectedAllowed: true, + }, + "cloud, connected non-admin": { + pluginConfig: cloudConfig, + roles: model.SystemUserRoleId, + connection: connected, + expectedAllowed: true, + }, + "cloud, disconnected non-admin": { + pluginConfig: cloudConfig, + roles: model.SystemUserRoleId, + expectedReason: subscribeDeniedNotConnected, + }, + "cloud, non-admin whose connection has no Confluence account": { + pluginConfig: cloudConfig, + roles: model.SystemUserRoleId, + connection: connectedWithoutAccount, + expectedReason: subscribeDeniedNotConnected, + }, + "server 9+, connected non-admin": { + pluginConfig: serverV9Config, + roles: model.SystemUserRoleId, + connection: connected, + expectedAllowed: true, + }, + "server below 9, connected non-admin stays admin-only": { + pluginConfig: legacyServerConfig, + roles: model.SystemUserRoleId, + connection: connected, + expectedReason: subscribeDeniedAdminOnly, + }, + "server below 9, system admin": { + pluginConfig: legacyServerConfig, + roles: model.SystemAdminRoleId, + expectedAllowed: true, + }, + } { + t.Run(name, func(t *testing.T) { + mockAPI := &plugintest.API{} + config.Mattermost = mockAPI + config.SetConfig(tc.pluginConfig) + + mockAPI.On("GetUser", mock.AnythingOfType("string")).Return(&model.User{Roles: tc.roles}, nil) + mockAPI.On("KVGet", mock.AnythingOfType("string")).Return(tc.connection, nil) + + access := (&Plugin{}).checkSubscriptionAccess("user-id") + + assert.Equal(t, tc.expectedAllowed, access.Allowed) + assert.Equal(t, tc.expectedReason, access.Reason) + if !tc.expectedAllowed { + assert.NotEmpty(t, access.Message) + } + }) + } +} diff --git a/server/util/types/connection.go b/server/util/types/connection.go index a6b43319..28dc4939 100644 --- a/server/util/types/connection.go +++ b/server/util/types/connection.go @@ -25,6 +25,7 @@ type Connection struct { DefaultProjectKey string `json:"default_project_key,omitempty"` IsAdmin bool `json:"is_admin,omitempty"` MattermostUserID string `json:"mattermost_user_id,omitempty"` + CloudID string `json:"cloud_id,omitempty"` } func (c *Connection) ConfluenceAccountID() string { diff --git a/webapp/src/constants/index.js b/webapp/src/constants/index.js index c891d021..f9162550 100644 --- a/webapp/src/constants/index.js +++ b/webapp/src/constants/index.js @@ -58,6 +58,11 @@ const SYSTEM_ADMIN_ROLE = 'system_admin'; const DISCONNECTED_USER = 'User not connected. Please use `/confluence connect`.'; const ERROR_EXECUTING_COMMAND = 'An error occurred while executing the command. Please try again later.'; +const SUBSCRIBE_DENIED_REASON = { + ADMIN_ONLY: 'admin_only', + NOT_CONNECTED: 'not_connected', +}; + export default { ACTION_TYPES, CONFLUENCE_EVENTS, @@ -70,4 +75,5 @@ export default { SUBSCRIPTION_TYPE, DISCONNECTED_USER, ERROR_EXECUTING_COMMAND, + SUBSCRIBE_DENIED_REASON, }; diff --git a/webapp/src/hooks/index.js b/webapp/src/hooks/index.js index 4955000d..96f287af 100644 --- a/webapp/src/hooks/index.js +++ b/webapp/src/hooks/index.js @@ -6,6 +6,15 @@ import {splitArgs} from '../utils'; import {getSubscriptionAccess, sendEphemeralPost} from '../actions/subscription_modal'; import Constants from '../constants'; +const SUBSCRIBE_DENIED_MESSAGES = { + [Constants.SUBSCRIBE_DENIED_REASON.ADMIN_ONLY]: Constants.COMMAND_ADMIN_ONLY, + [Constants.SUBSCRIBE_DENIED_REASON.NOT_CONNECTED]: Constants.DISCONNECTED_USER, +}; + +const subscribeDeniedMessage = (subscriptionAccessData) => ( + SUBSCRIBE_DENIED_MESSAGES[subscriptionAccessData?.subscribe_denied_reason] || Constants.ERROR_EXECUTING_COMMAND +); + export default class Hooks { constructor(store) { this.store = store; @@ -36,8 +45,7 @@ export default class Hooks { } if (!subscriptionAccessData?.can_run_subscribe_command) { - const errorMsg = subscriptionAccessData?.server_version_greater_than_9 ? Constants.DISCONNECTED_USER : Constants.COMMAND_ADMIN_ONLY; - this.store.dispatch(sendEphemeralPost(errorMsg, contextArgs.channel_id, user.id)); + this.store.dispatch(sendEphemeralPost(subscribeDeniedMessage(subscriptionAccessData), contextArgs.channel_id, user.id)); return Promise.resolve({}); } @@ -52,8 +60,7 @@ export default class Hooks { } if (!subscriptionAccessData?.can_run_subscribe_command) { - const errorMsg = subscriptionAccessData?.server_version_greater_than_9 ? Constants.DISCONNECTED_USER : Constants.COMMAND_ADMIN_ONLY; - this.store.dispatch(sendEphemeralPost(errorMsg, contextArgs.channel_id, user.id)); + this.store.dispatch(sendEphemeralPost(subscribeDeniedMessage(subscriptionAccessData), contextArgs.channel_id, user.id)); return Promise.resolve({}); }