diff --git a/server/command.go b/server/command.go index 9d3f3b536..c2ad6fbdd 100644 --- a/server/command.go +++ b/server/command.go @@ -24,6 +24,7 @@ const commandHelp = `* |/gitlab connect| - Connect your Mattermost account to yo * |/gitlab subscriptions add owner[/repo] [features]| - Subscribe the current channel to receive notifications about opened merge requests and issues for a group or repository * |features| is a comma-delimited list of one or more the following: * issues - includes new and closed issues + * confidential_issues - includes new and closed confidential issues * jobs - includes jobs status updates * merges - includes new and closed merge requests * pushes - includes pushes @@ -574,6 +575,7 @@ func (p *Plugin) subscriptionsListCommand(channelID string) string { if err != nil { return err.Error() } + if len(subs) == 0 { txt = "Currently there are no subscriptions in this channel" } else { @@ -610,6 +612,12 @@ func (p *Plugin) subscriptionsAddCommand(ctx context.Context, info *gitlab.UserI return err.Error() } + if hasPermission := p.permissionToProject(ctx, info.UserID, namespace, project); !hasPermission { + msg := "You don't have the permissions to create subscriptions for this project." + p.client.Log.Warn(msg) + return msg + } + updatedSubscriptions, subscribeErr := p.Subscribe(info, namespace, project, channelID, features) if subscribeErr != nil { p.client.Log.Warn( @@ -783,7 +791,7 @@ func getAutocompleteData(config *configuration) *model.AutocompleteData { subscriptionsAdd := model.NewAutocompleteData(commandAdd, "owner[/repo] [features]", "Subscribe the current channel to receive notifications from a project") subscriptionsAdd.AddTextArgument("Project path: includes user or group name with optional slash project name", "owner[/repo]", "") - subscriptionsAdd.AddTextArgument("comma-delimited list of features to subscribe to: issues, merges, pushes, issue_comments, merge_request_comments, pipeline, tag, pull_reviews, label:", "[features] (optional)", `/[^,-\s]+(,[^,-\s]+)*/`) + subscriptionsAdd.AddTextArgument("comma-delimited list of features to subscribe to: issues, confidential_issues, merges, pushes, issue_comments, merge_request_comments, pipeline, tag, pull_reviews, label:", "[features] (optional)", `/[^,-\s]+(,[^,-\s]+)*/`) subscriptions.AddCommand(subscriptionsAdd) subscriptionsDelete := model.NewAutocompleteData(commandDelete, "owner[/repo]", "Unsubscribe the current channel from a repository") diff --git a/server/command_test.go b/server/command_test.go index eeeb2180d..4dfbcbbd6 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "testing" "github.com/golang/mock/gomock" @@ -11,6 +12,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" gitLabAPI "github.com/xanzy/go-gitlab" "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" @@ -23,7 +25,9 @@ type subscribeCommandTest struct { want string webhookInfo []*gitlab.WebhookInfo mattermostURL string + noAccess bool projectHookErr error + getProjectErr error mockGitlab bool } @@ -37,6 +41,25 @@ var subscribeCommandTests = []subscribeCommandTest{ parameters: []string{"list"}, want: "Currently there are no subscriptions in this channel", }, + { + testName: "No Repository permissions", + parameters: []string{"add", "group/project"}, + mockGitlab: true, + want: "You don't have the permissions to create subscriptions for this project.", + webhookInfo: []*gitlab.WebhookInfo{{URL: "example.com/somewebhookURL"}}, + noAccess: true, + mattermostURL: "example.com", + getProjectErr: errors.New("unable to get project"), + }, + { + testName: "Guest permissions only", + parameters: []string{"add", "group/project"}, + mockGitlab: true, + want: "You don't have the permissions to create subscriptions for this project.", + webhookInfo: []*gitlab.WebhookInfo{{URL: "example.com/somewebhookURL"}}, + noAccess: true, + mattermostURL: "example.com", + }, { testName: "Hook Found", parameters: []string{"add", "group/project"}, @@ -78,9 +101,11 @@ func TestSubscribeCommand(t *testing.T) { mockCtrl := gomock.NewController(t) channelID := "12345" - userInfo := &gitlab.UserInfo{} + userInfo := &gitlab.UserInfo{ + UserID: "user_id", + } - p := getTestPlugin(t, mockCtrl, test.webhookInfo, test.mattermostURL, test.projectHookErr, test.mockGitlab) + p := getTestPlugin(t, mockCtrl, test.webhookInfo, test.mattermostURL, test.projectHookErr, test.getProjectErr, test.mockGitlab, test.noAccess) subscribeMessage := p.subscribeCommand(context.Background(), test.parameters, channelID, &configuration{}, userInfo) assert.Equal(t, test.want, subscribeMessage, "Subscribe command message should be the same.") @@ -224,15 +249,34 @@ func TestListWebhookCommand(t *testing.T) { } } -func getTestPlugin(t *testing.T, mockCtrl *gomock.Controller, hooks []*gitlab.WebhookInfo, mattermostURL string, projectHookErr error, mockGitlab bool) *Plugin { +func getTestPlugin(t *testing.T, mockCtrl *gomock.Controller, hooks []*gitlab.WebhookInfo, mattermostURL string, projectHookErr error, getProjectErr error, mockGitlab, noAccess bool) *Plugin { p := new(Plugin) + accessLevel := gitLabAPI.OwnerPermission + if noAccess { + accessLevel = gitLabAPI.GuestPermissions + } + mockedClient := mocks.NewMockGitlab(mockCtrl) if mockGitlab { mockedClient.EXPECT().ResolveNamespaceAndProject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("group", "project", nil) - mockedClient.EXPECT().GetProjectHooks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr) - if projectHookErr == nil { - mockedClient.EXPECT().GetGroupHooks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr) + if getProjectErr != nil { + mockedClient.EXPECT().GetProject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, getProjectErr) + } else { + mockedClient.EXPECT().GetProject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&gitLabAPI.Project{ + Permissions: &gitLabAPI.Permissions{ + ProjectAccess: &gitLabAPI.ProjectAccess{ + AccessLevel: accessLevel, + }, + }, + }, nil) + } + + if !noAccess { + mockedClient.EXPECT().GetProjectHooks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr) + if projectHookErr == nil { + mockedClient.EXPECT().GetGroupHooks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr) + } } } @@ -247,15 +291,32 @@ func getTestPlugin(t *testing.T, mockCtrl *gomock.Controller, hooks []*gitlab.We EncryptionKey: testEncryptionKey, } + info := gitlab.UserInfo{ + UserID: "user_id", + GitlabUsername: "gitlab_username", + } + + jsonInfo, err := json.Marshal(info) + require.NoError(t, err) + var subVal []byte api := &plugintest.API{} api.On("GetConfig", mock.Anything).Return(conf) - api.On("KVGet", "_usertoken").Return([]byte(encryptedToken), nil) - api.On("KVGet", mock.Anything).Return(subVal, nil) + api.On("KVGet", "user_id_usertoken").Return([]byte(encryptedToken), nil) + api.On("KVGet", "user_id_userinfo").Return(subVal, nil).Once() + api.On("KVGet", "user_id_gitlabtoken").Return(jsonInfo, nil).Once() + api.On("KVGet", "subscriptions").Return(subVal, nil) + api.On("KVSet", mock.Anything, mock.Anything).Return(nil) api.On("KVSetWithOptions", mock.AnythingOfType("string"), mock.Anything, mock.AnythingOfType("model.PluginKVSetOptions")).Return(true, nil) api.On("PublishWebSocketEvent", mock.Anything, mock.Anything, mock.Anything).Return(nil) + api.On("LogWarn", + mock.AnythingOfTypeArgument("string"), + mock.AnythingOfTypeArgument("string"), + mock.AnythingOfTypeArgument("string"), + mock.AnythingOfTypeArgument("string"), + mock.AnythingOfTypeArgument("string")) p.SetAPI(api) p.client = pluginapi.NewClient(api, p.Driver) diff --git a/server/subscription/subscription.go b/server/subscription/subscription.go index 6291cca3f..ce121c842 100644 --- a/server/subscription/subscription.go +++ b/server/subscription/subscription.go @@ -16,7 +16,8 @@ var allFeatures = map[string]bool{ "pipeline": true, "tag": true, "pull_reviews": true, - // "label:": true,//particular case for label:XXX + "confidential_issues": true, + // "label:": true,//particular case for label:XXX } type Subscription struct { @@ -63,6 +64,10 @@ func (s *Subscription) Issues() bool { return strings.Contains(s.Features, "issues") } +func (s *Subscription) ConfidentialIssues() bool { + return strings.Contains(s.Features, "confidential_issues") +} + func (s *Subscription) Pushes() bool { return strings.Contains(s.Features, "pushes") } diff --git a/server/webhook.go b/server/webhook.go index 64a43aac0..9994e82f2 100644 --- a/server/webhook.go +++ b/server/webhook.go @@ -73,7 +73,8 @@ func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) { return } - event, err := gitlabLib.ParseWebhook(gitlabLib.WebhookEventType(r), body) + eventType := gitlabLib.WebhookEventType(r) + event, err := gitlabLib.ParseWebhook(eventType, body) if err != nil { p.client.Log.Debug("Can't parse webhook", "err", err.Error(), "header", r.Header.Get("X-Gitlab-Event"), "event", string(body)) http.Error(w, "Unable to handle request", http.StatusBadRequest) @@ -99,7 +100,7 @@ func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) { repoPrivate = event.Project.Visibility == gitlabLib.PrivateVisibility pathWithNamespace = event.Project.PathWithNamespace fromUser = event.User.Username - handlers, errHandler = p.WebhookHandler.HandleIssue(ctx, event) + handlers, errHandler = p.WebhookHandler.HandleIssue(ctx, event, eventType) case *gitlabLib.IssueCommentEvent: repoPrivate = event.Project.Visibility == gitlabLib.PrivateVisibility pathWithNamespace = event.Project.PathWithNamespace @@ -224,10 +225,16 @@ func (p *Plugin) permissionToProject(ctx context.Context, userID, namespace, pro }) if result == nil || err != nil { if err != nil { - p.client.Log.Warn("can't get project in webhook", "err", err.Error(), "project", namespace+"/"+project) + p.client.Log.Warn("Can't get project in webhook", "err", err.Error(), "project", namespace+"/"+project) } return false } + + // Check for guest level permissions + if result.Permissions.ProjectAccess == nil || result.Permissions.ProjectAccess.AccessLevel == gitlabLib.GuestPermissions { + return false + } + return true } diff --git a/server/webhook/issue.go b/server/webhook/issue.go index 3943f3691..7c377fed7 100644 --- a/server/webhook/issue.go +++ b/server/webhook/issue.go @@ -7,12 +7,12 @@ import ( "github.com/xanzy/go-gitlab" ) -func (w *webhook) HandleIssue(ctx context.Context, event *gitlab.IssueEvent) ([]*HandleWebhook, error) { +func (w *webhook) HandleIssue(ctx context.Context, event *gitlab.IssueEvent, eventType gitlab.EventType) ([]*HandleWebhook, error) { handlers, err := w.handleDMIssue(event) if err != nil { return nil, err } - handlers2, err := w.handleChannelIssue(ctx, event) + handlers2, err := w.handleChannelIssue(ctx, event, eventType) if err != nil { return nil, err } @@ -65,7 +65,7 @@ func (w *webhook) handleDMIssue(event *gitlab.IssueEvent) ([]*HandleWebhook, err return []*HandleWebhook{}, nil } -func (w *webhook) handleChannelIssue(ctx context.Context, event *gitlab.IssueEvent) ([]*HandleWebhook, error) { +func (w *webhook) handleChannelIssue(ctx context.Context, event *gitlab.IssueEvent, eventType gitlab.EventType) ([]*HandleWebhook, error) { issue := event.ObjectAttributes senderGitlabUsername := event.User.Username repo := event.Project @@ -98,6 +98,10 @@ func (w *webhook) handleChannelIssue(ctx context.Context, event *gitlab.IssueEve continue } + if eventType == gitlab.EventConfidentialIssue && !sub.ConfidentialIssues() { + continue + } + if sub.Label() != "" && !containsLabel(event.Labels, sub.Label()) { continue } diff --git a/server/webhook/issue_test.go b/server/webhook/issue_test.go index fcd80a02c..2af26d44e 100644 --- a/server/webhook/issue_test.go +++ b/server/webhook/issue_test.go @@ -111,7 +111,7 @@ func TestIssueWebhook(t *testing.T) { if err := json.Unmarshal([]byte(test.fixture), issueEvent); err != nil { assert.Fail(t, "can't unmarshal fixture") } - res, err := w.HandleIssue(context.Background(), issueEvent) + res, err := w.HandleIssue(context.Background(), issueEvent, gitlab.EventTypeIssue) assert.Empty(t, err) assert.Equal(t, len(test.res), len(res)) for index := range res { diff --git a/server/webhook/webhook.go b/server/webhook/webhook.go index 56f4f1ac0..06015bd20 100644 --- a/server/webhook/webhook.go +++ b/server/webhook/webhook.go @@ -37,7 +37,7 @@ type HandleWebhook struct { } type Webhook interface { - HandleIssue(ctx context.Context, event *gitlab.IssueEvent) ([]*HandleWebhook, error) + HandleIssue(ctx context.Context, event *gitlab.IssueEvent, eventType gitlab.EventType) ([]*HandleWebhook, error) HandleMergeRequest(ctx context.Context, event *gitlab.MergeEvent) ([]*HandleWebhook, error) HandleIssueComment(ctx context.Context, event *gitlab.IssueCommentEvent) ([]*HandleWebhook, error) HandleMergeRequestComment(ctx context.Context, event *gitlab.MergeCommentEvent) ([]*HandleWebhook, error) diff --git a/server/webhook_test.go b/server/webhook_test.go index de18739c2..9922397ae 100644 --- a/server/webhook_test.go +++ b/server/webhook_test.go @@ -18,7 +18,7 @@ import ( type fakeWebhookHandler struct{} -func (fakeWebhookHandler) HandleIssue(_ context.Context, _ *gitlabLib.IssueEvent) ([]*webhook.HandleWebhook, error) { +func (fakeWebhookHandler) HandleIssue(_ context.Context, _ *gitlabLib.IssueEvent, _ gitlabLib.EventType) ([]*webhook.HandleWebhook, error) { return []*webhook.HandleWebhook{{ Message: "hello", From: "test", diff --git a/webapp/package-lock.json b/webapp/package-lock.json index cdd39089e..ebb8ef9b0 100644 --- a/webapp/package-lock.json +++ b/webapp/package-lock.json @@ -11110,7 +11110,7 @@ "react-custom-scrollbars": { "version": "4.2.1", "resolved": "https://registry.npmjs.org/react-custom-scrollbars/-/react-custom-scrollbars-4.2.1.tgz", - "integrity": "sha512-VtJTUvZ7kPh/auZWIbBRceGPkE30XBYe+HktFxuMWBR2eVQQ+Ur6yFJMoaYcNpyGq22uYJ9Wx4UAEcC0K+LNPQ==", + "integrity": "sha1-gw/ZUCkn6X6KeMIIaBOJmyqLZts=", "requires": { "dom-css": "^2.0.0", "prop-types": "^15.5.10", @@ -12939,7 +12939,7 @@ "to-camel-case": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/to-camel-case/-/to-camel-case-1.0.0.tgz", - "integrity": "sha512-nD8pQi5H34kyu1QDMFjzEIYqk0xa9Alt6ZfrdEMuHCFOfTLhDG5pgTu/aAM9Wt9lXILwlXmWP43b8sav0GNE8Q==", + "integrity": "sha1-GlYFSy+daWKYzmamCJcyK29CPkY=", "requires": { "to-space-case": "^1.0.0" } @@ -12952,7 +12952,7 @@ "to-no-case": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/to-no-case/-/to-no-case-1.0.2.tgz", - "integrity": "sha512-Z3g735FxuZY8rodxV4gH7LxClE4H0hTIyHNIHdk+vpQxjLm0cwnKXq/OFVZ76SOQmto7txVcwSCwkU5kqp+FKg==" + "integrity": "sha1-xyKQcWTvaxeBMsjmmTAhLRtKoWo=" }, "to-object-path": { "version": "0.3.0", @@ -12995,7 +12995,7 @@ "to-space-case": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/to-space-case/-/to-space-case-1.0.0.tgz", - "integrity": "sha512-rLdvwXZ39VOn1IxGL3V6ZstoTbwLRckQmn/U8ZDLuWwIXNpuZDhQ3AiRUlhTbOXFVE9C+dR51wM0CBDhk31VcA==", + "integrity": "sha1-sFLar7Gysp3HcM6gFj5ewOvJ/Bc=", "requires": { "to-no-case": "^1.0.0" }