From b7b28b807a00f9a61830403f6738dad4ea3f8049 Mon Sep 17 00:00:00 2001 From: Nish Krishnan Date: Tue, 4 Jan 2022 14:20:29 -0500 Subject: [PATCH] fix: default permissions for gh-team-allowlist. (#1974) * Fix default permissions for gh-team-allowlist. * Fix broken links. --- cmd/server.go | 5 --- runatlantis.io/docs/troubleshooting-https.md | 4 +-- server/events/command_runner.go | 2 +- server/events/command_runner_test.go | 36 ++++++++++++++++++++ server/events/team_allowlist_checker.go | 4 +++ server/events/team_allowlist_checker_test.go | 7 ++++ 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index b7ca3c6745..dc24de2acc 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -117,7 +117,6 @@ const ( DefaultBitbucketBaseURL = bitbucketcloud.BaseURL DefaultDataDir = "~/.atlantis" DefaultGHHostname = "github.com" - DefaultGHTeamAllowlist = "" DefaultGitlabHostname = "gitlab.com" DefaultLogLevel = "info" DefaultParallelPoolSize = 15 @@ -211,7 +210,6 @@ var stringFlags = map[string]stringFlag{ "and allows the 'devops' team to perform any operation. If this argument is not provided, the default value (*:*) " + "will be used and the default behavior will be to not check permissions " + "and to allow users from any team to perform any operation.", - defaultValue: DefaultGHTeamAllowlist, }, GHUserFlag: { description: "GitHub username of API user.", @@ -663,9 +661,6 @@ func (s *ServerCmd) setDefaults(c *server.UserConfig) { if c.VCSStatusName == "" { c.VCSStatusName = DefaultVCSStatusName } - if c.GithubTeamAllowlist == "" { - c.GithubTeamAllowlist = DefaultGHTeamAllowlist - } if c.TFEHostname == "" { c.TFEHostname = DefaultTFEHostname } diff --git a/runatlantis.io/docs/troubleshooting-https.md b/runatlantis.io/docs/troubleshooting-https.md index 9d46e6813b..52ae0ffb9f 100644 --- a/runatlantis.io/docs/troubleshooting-https.md +++ b/runatlantis.io/docs/troubleshooting-https.md @@ -4,9 +4,9 @@ When using a self-signed certificate for Atlantis (with flags `--ssl-cert-file` there are a few considerations. Atlantis uses the web server from the standard Go library, -the method name is [ListenAndServeTLS](https://golang.org/pkg/net/http/#ListenAndServeTLS). +the method name is [ListenAndServeTLS](https://pkg.go.dev/net/http#ListenAndServeTLS). -`ListenAndServeTLS` acts identically to [ListenAndServe](https://golang.org/pkg/net/http/#ListenAndServe), +`ListenAndServeTLS` acts identically to [ListenAndServe](https://pkg.go.dev/net/http#ListenAndServe), except that it expects HTTPS connections. Additionally, files containing a certificate and matching private key for the server must be provided. If the certificate is signed by a certificate authority, diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 106e392061..7f4f5f6304 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -174,7 +174,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) { - if c.TeamAllowlistChecker == nil || len(c.TeamAllowlistChecker.rules) == 0 { + if c.TeamAllowlistChecker == nil || !c.TeamAllowlistChecker.HasRules() { // allowlist restriction is not enabled return true, nil } diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 26abd4e9e8..644f06a450 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -242,6 +242,42 @@ func TestRunCommentCommand_GithubPullParseErr(t *testing.T) { vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "`Error: extracting required fields from comment data: err`", "") } +func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) { + t.Run("nil checker", func(t *testing.T) { + vcsClient := setup(t) + // by default these are false so don't need to reset + ch.TeamAllowlistChecker = nil + var pull github.PullRequest + modelPull := models.PullRequest{ + BaseRepo: fixtures.GithubRepo, + State: models.OpenPullState, + } + When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(&pull, nil) + When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil) + + ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.PlanCommand}) + vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(fixtures.GithubRepo, fixtures.User) + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, "Ran Plan for 0 projects:\n\n\n\n", "plan") + }) + + t.Run("no rules", func(t *testing.T) { + vcsClient := setup(t) + // by default these are false so don't need to reset + ch.TeamAllowlistChecker = &events.TeamAllowlistChecker{} + var pull github.PullRequest + modelPull := models.PullRequest{ + BaseRepo: fixtures.GithubRepo, + State: models.OpenPullState, + } + When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(&pull, nil) + When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil) + + ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.PlanCommand}) + vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(fixtures.GithubRepo, fixtures.User) + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, "Ran Plan for 0 projects:\n\n\n\n", "plan") + }) +} + func TestRunCommentCommand_ForkPRDisabled(t *testing.T) { t.Log("if a command is run on a forked pull request and this is disabled atlantis should" + " comment saying that this is not allowed") diff --git a/server/events/team_allowlist_checker.go b/server/events/team_allowlist_checker.go index 5d0cc49f03..01e7bed73c 100644 --- a/server/events/team_allowlist_checker.go +++ b/server/events/team_allowlist_checker.go @@ -34,6 +34,10 @@ func NewTeamAllowlistChecker(allowlist string) (*TeamAllowlistChecker, error) { }, nil } +func (checker *TeamAllowlistChecker) HasRules() bool { + return len(checker.rules) > 0 +} + // IsCommandAllowedForTeam returns true if the team is allowed to execute the command // and false otherwise. func (checker *TeamAllowlistChecker) IsCommandAllowedForTeam(team string, command string) bool { diff --git a/server/events/team_allowlist_checker_test.go b/server/events/team_allowlist_checker_test.go index 59087cf141..b389b49ae0 100644 --- a/server/events/team_allowlist_checker_test.go +++ b/server/events/team_allowlist_checker_test.go @@ -13,6 +13,13 @@ func TestNewTeamAllowListChecker(t *testing.T) { Ok(t, err) } +func TestNewTeamAllowListCheckerEmpty(t *testing.T) { + allowlist := `` + checker, err := events.NewTeamAllowlistChecker(allowlist) + Ok(t, err) + Equals(t, false, checker.HasRules()) +} + func TestIsCommandAllowedForTeam(t *testing.T) { allowlist := `bob:plan, dave:apply, connie:plan, connie:apply` checker, err := events.NewTeamAllowlistChecker(allowlist)