Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
May 4, 2023
14 changes: 13 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,16 @@ 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("don't have the permission to access the project")
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
return "Don't have the permission to access the project"
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
}

if guestUser := p.checkForGuestUser(ctx, info.UserID, namespace, project); guestUser {
p.API.LogWarn("guest user cannot create a subscription")
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
return "Guest user cannot create a subscription"
}

updatedSubscriptions, subscribeErr := p.Subscribe(info, namespace, project, channelID, features)
if subscribeErr != nil {
p.client.Log.Warn(
Expand Down Expand Up @@ -710,7 +722,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
102 changes: 92 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,7 +28,9 @@ type subscribeCommandTest struct {
want string
webhookInfo []*gitlab.WebhookInfo
mattermostURL string
noAccess bool
projectHookErr error
getProjectErr error
mockGitlab bool
}

Expand All @@ -35,6 +42,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: "Don't have the permission to access the 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: "Guest user cannot create a subscription",
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 +102,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 +239,87 @@ 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)
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
} else {
mockedClient.EXPECT().GetProject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&gitLabAPI.Project{
Permissions: &gitLabAPI.Permissions{
ProjectAccess: &gitLabAPI.ProjectAccess{
AccessLevel: accessLevel,
},
},
}, nil).Times(2)
}

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 := configuration{
GitlabURL: "https://example.com",
GitlabOAuthClientID: "client_id",
GitlabOAuthClientSecret: "secret",
EncryptionKey: "aaaaaaaaaaaaaaaa",
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
}

p.configuration = &config
p.initializeAPI()

token := oauth2.Token{
AccessToken: "access_token",
Expiry: time.Now().Add(1 * time.Hour),
}
info := gitlab.UserInfo{
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
UserID: "user_id",
Token: &token,
GitlabUsername: "gitlab_username",
GitlabUserID: 0,
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
}
encryptedToken, err := encrypt([]byte(config.EncryptionKey), info.Token.AccessToken)
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
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")).Return(nil)
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved

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
27 changes: 25 additions & 2 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 @@ -216,6 +217,28 @@ func (p *Plugin) permissionToProject(ctx context.Context, userID, namespace, pro
return true
}

func (p *Plugin) checkForGuestUser(ctx context.Context, userID, namespace, project string) bool {
info, apiErr := p.getGitlabUserInfoByMattermostID(userID)
if apiErr != nil {
return false
}

result, err := p.GitlabClient.GetProject(ctx, info, namespace, project)
if result == nil || err != nil {
if err != nil {
p.API.LogWarn("can't get project in webhook", "err", err.Error(), "project", namespace+"/"+project)
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
}
return false
}

// Check for guest level permissions
if result.Permissions.ProjectAccess != nil && result.Permissions.ProjectAccess.AccessLevel == gitlabLib.GuestPermissions {
return true
}

return false
}

func CreateHook(ctx context.Context, gitlabClient gitlab.Gitlab, info *gitlab.UserInfo, group, project string, hookOptions *gitlab.AddWebhookOptions) (*gitlab.WebhookInfo, error) {
// If project scope
if project != "" {
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