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

fix: Check user permissions on autoplan #3742

Merged
merged 12 commits into from
Nov 15, 2023
4 changes: 2 additions & 2 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers

conftextExec := policy.NewConfTestExecutorWorkflow(logger, binDir, &NoopTFDownloader{})

// swapping out version cache to something that always returns local contest
// swapping out version cache to something that always returns local conftest
// binary
conftextExec.VersionCache = &LocalConftestCache{}

Expand Down Expand Up @@ -1763,7 +1763,7 @@ func mkSubDirs(t *testing.T) (string, string, string) {

// Will fail test if conftest isn't in path
func ensureRunningConftest(t *testing.T) {
// use `conftest` command instead `contest$version`, so tests may fail on the environment cause the output logs may become change by version.
// use `conftest` command instead `conftest$version`, so tests may fail on the environment cause the output logs may become change by version.
t.Logf("conftest check may fail depends on conftest version. please use latest stable conftest.")
_, err := exec.LookPath(conftestCommand)
if err != nil {
Expand Down
16 changes: 13 additions & 3 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,16 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
timer := scope.Timer(metrics.ExecutionTimeMetric).Start()
defer timer.Stop()

// Check if the user who triggered the autoplan has permissions to run 'plan'.
ok, err := c.checkUserPermissions(baseRepo, user, "plan")
if err != nil {
c.Logger.Err("Unable to check user permissions: %s", err)
return
}
if !ok {
return
}

ctx := &command.Context{
User: user,
Log: log,
Expand Down Expand Up @@ -227,7 +237,7 @@ func (c *DefaultCommandRunner) commentUserDoesNotHavePermissions(baseRepo models
}

// checkUserPermissions checks if the user has permissions to execute the command
func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user models.User, cmd *CommentCommand) (bool, error) {
func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user models.User, cmdName string) (bool, error) {
if c.TeamAllowlistChecker == nil || !c.TeamAllowlistChecker.HasRules() {
// allowlist restriction is not enabled
return true, nil
Expand All @@ -236,7 +246,7 @@ func (c *DefaultCommandRunner) checkUserPermissions(repo models.Repo, user model
if err != nil {
return false, err
}
ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(teams, cmd.Name.String())
ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(teams, cmdName)
if !ok {
return false, nil
}
Expand Down Expand Up @@ -278,7 +288,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
defer timer.Stop()

// Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands
ok, err := c.checkUserPermissions(baseRepo, user, cmd)
ok, err := c.checkUserPermissions(baseRepo, user, cmd.Name.String())
if err != nil {
c.Logger.Err("Unable to check user permissions: %s", err)
return
Expand Down
8 changes: 4 additions & 4 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,15 +271,15 @@ func TestRunCommentCommand_GithubPullErr(t *testing.T) {
t.Log("if getting the github pull request fails an error should be logged")
vcsClient := setup(t)
When(githubGetter.GetPullRequest(testdata.GithubRepo, testdata.Pull.Num)).ThenReturn(nil, errors.New("err"))
ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, nil)
ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalledOnce().CreateComment(testdata.GithubRepo, testdata.Pull.Num, "`Error: making pull request API call to GitHub: err`", "")
}

func TestRunCommentCommand_GitlabMergeRequestErr(t *testing.T) {
t.Log("if getting the gitlab merge request fails an error should be logged")
vcsClient := setup(t)
When(gitlabGetter.GetMergeRequest(testdata.GitlabRepo.FullName, testdata.Pull.Num)).ThenReturn(nil, errors.New("err"))
ch.RunCommentCommand(testdata.GitlabRepo, &testdata.GitlabRepo, nil, testdata.User, testdata.Pull.Num, nil)
ch.RunCommentCommand(testdata.GitlabRepo, &testdata.GitlabRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalledOnce().CreateComment(testdata.GitlabRepo, testdata.Pull.Num, "`Error: making merge request API call to GitLab: err`", "")
}

Expand All @@ -290,7 +290,7 @@ func TestRunCommentCommand_GithubPullParseErr(t *testing.T) {
When(githubGetter.GetPullRequest(testdata.GithubRepo, testdata.Pull.Num)).ThenReturn(&pull, nil)
When(eventParsing.ParseGithubPull(&pull)).ThenReturn(testdata.Pull, testdata.GithubRepo, testdata.GitlabRepo, errors.New("err"))

ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, nil)
ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalledOnce().CreateComment(testdata.GithubRepo, testdata.Pull.Num, "`Error: extracting required fields from comment data: err`", "")
}

Expand Down Expand Up @@ -1188,7 +1188,7 @@ func TestRunCommentCommand_DrainNotOngoing(t *testing.T) {
t.Log("if drain is not ongoing then remove ongoing operation must be called even if panic occurred")
setup(t)
When(githubGetter.GetPullRequest(testdata.GithubRepo, testdata.Pull.Num)).ThenPanic("panic test - if you're seeing this in a test failure this isn't the failing test")
ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, nil)
ch.RunCommentCommand(testdata.GithubRepo, &testdata.GithubRepo, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
githubGetter.VerifyWasCalledOnce().GetPullRequest(testdata.GithubRepo, testdata.Pull.Num)
Equals(t, 0, drainer.GetStatus().InProgressOps)
}
Expand Down