Skip to content

MM-67398 Added checks for instance and webhooks commands #635

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.

Sign up for GitHub

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

Open
avasconcelos114 wants to merge 3 commits into
base: master
Choose a base branch
from
+179 −16
Open

MM-67398 Added checks for instance and webhooks commands #635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa699c5
Added sysadmin permission checks for instance and webhooks commands
avasconcelos114 Feb 2, 2026
8be51f6
Added test coverage for commands
avasconcelos114 Feb 2, 2026
68cc02f
PR Feedback
avasconcelos114 Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions server/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,15 @@ func (p *Plugin) handleConfigError(args *model.CommandArgs, err error) (*model.C
}

func (p *Plugin) handleInstance(args *model.CommandArgs, parameters []string) (*model.CommandResponse, *model.AppError) {
isSysAdmin, sysErr := p.isAuthorizedSysAdmin(args.UserId)
if sysErr != nil {
p.client.Log.Warn("Failed to check if user is System Admin", "error", sysErr.Error())
return p.getCommandResponse(args, "Error checking user's permissions", true), nil
}

if !isSysAdmin {
return p.getCommandResponse(args, "Only System Admins are allowed to manage instances.", true), nil
}
if len(parameters) < 1 {
return p.getCommandResponse(args, "Please specify the instance command.", true), nil
}
Expand All @@ -247,22 +256,9 @@ func (p *Plugin) handleInstance(args *model.CommandArgs, parameters []string) (*
}

func (p *Plugin) handleInstallInstance(args *model.CommandArgs, parameters []string) (*model.CommandResponse, *model.AppError) {
userID := args.UserId
isSysAdmin, err := p.isAuthorizedSysAdmin(userID)
err := p.flowManager.StartOauthWizard(args.UserId)
if err != nil {
p.client.Log.Warn("Failed to check if user is System Admin", "error", err.Error())
p.postCommandResponse(args, "Error checking user's permissions", true)
return &model.CommandResponse{}, nil
}

if !isSysAdmin {
p.postCommandResponse(args, "Only System Admins are allowed to set up the plugin.", true)
return &model.CommandResponse{}, nil
}

err = p.flowManager.StartOauthWizard(userID)
if err != nil {
p.postCommandResponse(args, err.Error(), true)
return p.getCommandResponse(args, err.Error(), true), nil
}

return &model.CommandResponse{}, nil
Expand All @@ -272,7 +268,6 @@ func (p *Plugin) handleUnInstallInstance(args *model.CommandArgs, parameters []s
if len(parameters) < 1 {
return p.getCommandResponse(args, "Please specify the instance name.", true), nil
}

instanceName := strings.TrimSpace(strings.Join(parameters, " "))

err := p.uninstallInstance(instanceName)
Expand Down Expand Up @@ -524,6 +519,15 @@ func (p *Plugin) handleSettings(ctx context.Context, args *model.CommandArgs, pa
}

func (p *Plugin) handleWebhookHandler(ctx context.Context, args *model.CommandArgs, parameters []string, info *gitlab.UserInfo) (*model.CommandResponse, *model.AppError) {
isSysAdmin, err := p.isAuthorizedSysAdmin(args.UserId)
if err != nil {
p.client.Log.Warn("Failed to check if user is System Admin", "error", err.Error())
return p.getCommandResponse(args, "Error checking user's permissions", true), nil
}
if !isSysAdmin {
return p.getCommandResponse(args, "Only System Admins are allowed to manage webhooks.", true), nil
}

config := p.getConfiguration()
message := p.webhookCommand(ctx, parameters, info, config.EnablePrivateRepo)
return p.getCommandResponse(args, message, true), nil
Expand Down Expand Up @@ -1067,6 +1071,7 @@ func (p *Plugin) getAutocompleteData(config *configuration) *model.AutocompleteD
gitlab.AddCommand(disconnect)

instance := model.NewAutocompleteData("instance", "[command]", "Install, Uninstall, List, Set-Default Instance")
instance.RoleID = model.SystemAdminRoleId

install := model.NewAutocompleteData("install", "", "Install GitLab Instance")
instance.AddCommand(install)
Expand Down Expand Up @@ -1140,6 +1145,7 @@ func (p *Plugin) getAutocompleteData(config *configuration) *model.AutocompleteD
gitlab.AddCommand(settings)

webhook := model.NewAutocompleteData("webhook", "[command]", "Available Commands: list, add")
webhook.RoleID = model.SystemAdminRoleId
webhookList := model.NewAutocompleteData(commandList, "owner/[repo]", "List existing project or group webhooks")
webhookList.AddTextArgument("Project path: includes user or group name with optional slash project name", "owner[/repo]", "")
webhook.AddCommand(webhookList)
Expand Down
157 changes: 157 additions & 0 deletions server/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,163 @@ func setupInstanceCommandTest(t *testing.T, instanceList []string, instanceConfi
return p, &capturedMessage, api
}

func TestAdminProtectedCommands(t *testing.T) {
t.Run("webhook command requires admin", func(t *testing.T) {
p := new(Plugin)
p.configuration = &configuration{EncryptionKey: testEncryptionKey}

// Mock a non-admin user
nonAdminUser := &model.User{
Id: "user_id",
Roles: "system_user",
}

api := &plugintest.API{}
api.On("GetUser", "user_id").Return(nonAdminUser, nil)

var capturedMessage string
api.On("SendEphemeralPost", mock.Anything, mock.MatchedBy(func(post *model.Post) bool {
capturedMessage = post.Message
return true
})).Return(&model.Post{})

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

args := &model.CommandArgs{UserId: "user_id", ChannelId: "channel_id"}
userInfo := &gitlab.UserInfo{UserID: "user_id"}

_, _ = p.handleWebhookHandler(context.Background(), args, []string{"list", "group/project"}, userInfo)

assert.Contains(t, capturedMessage, "Only System Admins are allowed to manage webhooks.")
})

t.Run("webhook command allowed for admin", func(t *testing.T) {
p := new(Plugin)

mockCtrl := gomock.NewController(t)
mockedClient := mocks.NewMockGitlab(mockCtrl)

// Mock admin user
adminUser := &model.User{
Id: "admin_id",
Roles: "system_admin system_user",
}

encryptedToken, _ := encrypt([]byte(testEncryptionKey), testGitlabToken)

p.configuration = &configuration{
EncryptionKey: testEncryptionKey,
EnablePrivateRepo: true,
}

mockedClient.EXPECT().ResolveNamespaceAndProject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), true).Return("group", "project", nil)
mockedClient.EXPECT().GetProjectHooks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]*gitlab.WebhookInfo{}, nil)
p.GitlabClient = mockedClient

api := &plugintest.API{}
api.On("GetUser", "admin_id").Return(adminUser, nil)
api.On("KVGet", "admin_id_usertoken").Return([]byte(encryptedToken), nil)

var capturedMessage string
api.On("SendEphemeralPost", mock.Anything, mock.MatchedBy(func(post *model.Post) bool {
capturedMessage = post.Message
return true
})).Return(&model.Post{})

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

args := &model.CommandArgs{UserId: "admin_id", ChannelId: "channel_id"}
userInfo := &gitlab.UserInfo{UserID: "admin_id"}

_, _ = p.handleWebhookHandler(context.Background(), args, []string{"list", "group/project"}, userInfo)

assert.Contains(t, capturedMessage, "No webhooks found")
})

t.Run("instance commands require admin", func(t *testing.T) {
testCases := []struct {
name string
parameters []string
}{
{"list", []string{"list"}},
{"install", []string{"install"}},
{"uninstall", []string{"uninstall", "test-instance"}},
{"set-default", []string{"set-default", "test-instance"}},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
p := new(Plugin)
p.configuration = &configuration{EncryptionKey: testEncryptionKey}

// Mock a non-admin user
nonAdminUser := &model.User{
Id: "user_id",
Roles: "system_user",
}

api := &plugintest.API{}
api.On("GetUser", "user_id").Return(nonAdminUser, nil)

var capturedMessage string
api.On("SendEphemeralPost", mock.Anything, mock.MatchedBy(func(post *model.Post) bool {
capturedMessage = post.Message
return true
})).Return(&model.Post{})

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

args := &model.CommandArgs{UserId: "user_id", ChannelId: "channel_id"}

_, _ = p.handleInstance(args, tc.parameters)

assert.Contains(t, capturedMessage, "Only System Admins are allowed to manage instances.")
})
}
})

t.Run("instance commands allowed for admin", func(t *testing.T) {
p := new(Plugin)
p.configuration = &configuration{EncryptionKey: testEncryptionKey}

// Mock admin user
adminUser := &model.User{
Id: "admin_id",
Roles: "system_admin system_user",
}

instanceList := []string{"test-instance"}
instanceConfig := map[string]InstanceConfiguration{"test-instance": {GitlabURL: "https://gitlab.example.com"}}
instanceListJSON, _ := json.Marshal(instanceList)
instanceConfigJSON, _ := json.Marshal(instanceConfig)

api := &plugintest.API{}
api.On("GetUser", "admin_id").Return(adminUser, nil)
api.On("KVGet", instanceConfigNameListKey).Return(instanceListJSON, nil)
api.On("KVGet", instanceConfigMapKey).Return(instanceConfigJSON, nil)

var capturedMessage string
api.On("SendEphemeralPost", mock.Anything, mock.MatchedBy(func(post *model.Post) bool {
capturedMessage = post.Message
return true
})).Return(&model.Post{})

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

args := &model.CommandArgs{UserId: "admin_id", ChannelId: "channel_id"}

_, _ = p.handleInstance(args, []string{"list"})

// Should show instance list, not an error
assert.Contains(t, capturedMessage, "test-instance")
assert.NotContains(t, capturedMessage, "Only System Admins")
})
}

func TestInstanceCommands(t *testing.T) {
args := &model.CommandArgs{UserId: "user_id", ChannelId: "channel_id"}

Expand Down
Loading