Skip to content

Commit

Permalink
[MI-3045]: Added checks for confidential issues at the time of making…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
Kshitij-Katiyar authored May 4, 2023
1 parent de004a9 commit ddd4074
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 22 deletions.
9 changes: 8 additions & 1 deletion server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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
Expand Down Expand Up @@ -514,6 +515,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 {
Expand Down Expand Up @@ -544,6 +546,11 @@ func (p *Plugin) subscriptionsAddCommand(ctx context.Context, info *gitlab.UserI
return err.Error()
}

if hasPermission := p.permissionToProject(ctx, info.UserID, namespace, project); !hasPermission {
p.API.LogWarn("You don't have the permission to create subscription for this project")
return "You don't have the permission to create subscription for this project."
}

updatedSubscriptions, subscribeErr := p.Subscribe(info, namespace, project, channelID, features)
if subscribeErr != nil {
p.client.Log.Warn(
Expand Down Expand Up @@ -710,7 +717,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")
Expand Down
106 changes: 96 additions & 10 deletions server/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,20 @@ package main

import (
"context"
"encoding/json"
"testing"
"time"

"github.com/golang/mock/gomock"
pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/plugin/plugintest"
"golang.org/x/oauth2"

"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"
Expand All @@ -23,10 +28,21 @@ type subscribeCommandTest struct {
want string
webhookInfo []*gitlab.WebhookInfo
mattermostURL string
noAccess bool
projectHookErr error
getProjectErr error
mockGitlab bool
}

func getTestConfig() *configuration {
return &configuration{
GitlabURL: "https://example.com",
GitlabOAuthClientID: "client_id",
GitlabOAuthClientSecret: "secret",
EncryptionKey: "encryption___key",
}
}

const subscribeSuccessMessage = "Successfully subscribed to group/project.\nA Webhook is needed, run ```/gitlab webhook add group/project``` to create one now."

var subscribeCommandTests = []subscribeCommandTest{
Expand All @@ -35,6 +51,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 permission to create subscription 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 permission to create subscription for this project.",
webhookInfo: []*gitlab.WebhookInfo{{URL: "example.com/somewebhookURL"}},
noAccess: true,
mattermostURL: "example.com",
},
{
testName: "Hook Found",
parameters: []string{"add", "group/project"},
Expand Down Expand Up @@ -76,9 +111,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.")
Expand Down Expand Up @@ -211,33 +248,82 @@ 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()).Return("group", "project", nil)
mockedClient.EXPECT().GetProjectHooks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr)
if projectHookErr == nil {
mockedClient.EXPECT().GetGroupHooks(gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr)
if getProjectErr != nil {
mockedClient.EXPECT().GetProject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, getProjectErr)
} else {
mockedClient.EXPECT().GetProject(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()).Return(hooks, projectHookErr)
if projectHookErr == nil {
mockedClient.EXPECT().GetGroupHooks(gomock.Any(), gomock.Any(), gomock.Any()).Return(hooks, projectHookErr)
}
}
}

p.GitlabClient = mockedClient

api := &plugintest.API{}
p.SetAPI(api)

conf := &model.Config{}
conf.ServiceSettings.SiteURL = &mattermostURL
api.On("GetConfig", mock.Anything).Return(conf)

var subVal []byte
api.On("KVGet", mock.Anything).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)

p.SetAPI(api)
config := getTestConfig()
p.configuration = config
p.initializeAPI()

token := oauth2.Token{
AccessToken: "access_token",
Expiry: time.Now().Add(1 * time.Hour),
}

info := gitlab.UserInfo{
UserID: "user_id",
Token: &token,
GitlabUsername: "gitlab_username",
}

encryptedToken, err := encrypt([]byte(config.EncryptionKey), info.Token.AccessToken)
require.NoError(t, err)

info.Token.AccessToken = encryptedToken

jsonInfo, err := json.Marshal(info)
require.NoError(t, err)

api.On("KVGet", "user_id_gitlabtoken").Return(jsonInfo, nil)
var subVal []byte
api.On("KVGet", "subscriptions").Return(subVal, nil)
api.On("LogWarn",
mock.AnythingOfTypeArgument("string"),
mock.AnythingOfTypeArgument("string"),
mock.AnythingOfTypeArgument("string"),
mock.AnythingOfTypeArgument("string"),
mock.AnythingOfTypeArgument("string"))

p.client = pluginapi.NewClient(api, p.Driver)

return p
Expand Down
7 changes: 6 additions & 1 deletion server/subscription/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}
Expand Down
16 changes: 12 additions & 4 deletions server/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,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)
Expand All @@ -97,7 +98,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
Expand Down Expand Up @@ -207,12 +208,19 @@ func (p *Plugin) permissionToProject(ctx context.Context, userID, namespace, pro
return false
}

if result, err := p.GitlabClient.GetProject(ctx, info, namespace, project); result == nil || err != nil {
result, err := p.GitlabClient.GetProject(ctx, info, namespace, project)
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.API.LogWarn("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
}

Expand Down
10 changes: 7 additions & 3 deletions server/webhook/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion server/webhook/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion server/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion server/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit ddd4074

Please sign in to comment.