Skip to content

Commit

Permalink
fix: Check user permissions on autoplan (#3742)
Browse files Browse the repository at this point in the history
* Check user permissions on autoplan

---------

Co-authored-by: PePe Amengual <[email protected]>
  • Loading branch information
meringu and jamengual authored Nov 15, 2023
1 parent 896c394 commit ce2b992
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
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

0 comments on commit ce2b992

Please sign in to comment.