[GH-326] Added subscription flag for filtering out confidential issues, and disallow guests from creating subscriptions (#376)

* [MI-3045]: Added checks for confidential issues at the time of making subscriptions (#37)

* [MI-3045]:Added checks for confidential issues

* [MI-3045]:Added unit test cases

* [MI-3045]:Fixed self review comments

* [MI-3045]:Fixed unit test cases

* [MI-3045]:Fixed lint errors

* [MI-3045]:Fixed review comments

* [MI-3045]:Fixed context deadline error

* [MI-3045]:Fixed review comments

* [MI-3045]:Fixed statements

* [MI-3060]:Fixed review comments of PR #376 on gitlab plugin (#38)

* [MI-3060]:Fixed review comments of PR #376 on gitlab plugin

* [MI-3060]:Fixed test cases

* [MI-3060]:Updated the msg

* [MI-3060]:Fixed the test cases

* [MM-326]:fixed test cases

* [MM-326]:Fixed CI error

* [MM-326]:Fixed review comments

* [MM-326]:Fixed review comments

---------

Co-authored-by: ayusht2810 <ayush.thakur@brightscout.com>

Author: Kshitij-Katiyar

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
@@ -578,6 +579,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 {
@@ -614,6 +616,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(
@@ -807,7 +815,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:<labelName>", "[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:<labelName>", "[features] (optional)", `/[^,-\s]+(,[^,-\s]+)*/`)
 	subscriptions.AddCommand(subscriptionsAdd)
 
 	subscriptionsDelete := model.NewAutocompleteData(commandDelete, "owner[/repo]", "Unsubscribe the current channel from a repository")

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)

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")
 }

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
 }
 

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
 			}

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 {

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)

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",