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

feat: add GitHub team allowlist configuration option #1694

Merged
merged 5 commits into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const (
EnableRegExpCmdFlag = "enable-regexp-cmd"
EnableDiffMarkdownFormat = "enable-diff-markdown-format"
GHHostnameFlag = "gh-hostname"
GHTeamAllowlistFlag = "gh-team-allowlist"
GHTokenFlag = "gh-token"
GHUserFlag = "gh-user"
GHAppIDFlag = "gh-app-id"
Expand Down Expand Up @@ -116,6 +117,7 @@ const (
DefaultBitbucketBaseURL = bitbucketcloud.BaseURL
DefaultDataDir = "~/.atlantis"
DefaultGHHostname = "github.com"
DefaultGHTeamAllowlist = "*:plan,*:apply"
DefaultGitlabHostname = "gitlab.com"
DefaultLogLevel = "info"
DefaultParallelPoolSize = 15
Expand Down Expand Up @@ -199,6 +201,18 @@ var stringFlags = map[string]stringFlag{
description: "Hostname of your Github Enterprise installation. If using github.com, no need to set.",
defaultValue: DefaultGHHostname,
},
GHTeamAllowlistFlag: {
description: "Comma separated list of key-value pairs representing the GitHub teams and the operations that " +
"the members of a particular team are allowed to perform. " +
"The format is {team}:{command},{team}:{command}. " +
"Valid values for 'command' are 'plan', 'apply' and '*', e.g. 'dev:plan,ops:apply,devops:*'" +
"This example gives the users from the 'dev' GitHub team the permissions to execute the 'plan' command, " +
"the 'ops' team the permissions to execute the 'apply' command, " +
"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.",
defaultValue: "",
Expand Down Expand Up @@ -649,6 +663,9 @@ 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
}
Expand Down
6 changes: 6 additions & 0 deletions runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,12 @@ Values are chosen in this order:
::: warning SECURITY WARNING
The contents of the private key will be visible by anyone that can run `ps` or look at the shell history of the machine where Atlantis is running. Use `--gh-app-key-file` to mitigate that risk.
:::
-
- ### `--gh-team-allowlist`
```bash
atlantis server --gh-team-allowlist="myteam:plan, secteam:apply"
```
Comma-separated list of GitHub team and permission pairs. By default, any team can plan and apply.

* ### `--gitlab-hostname`
```bash
Expand Down
38 changes: 38 additions & 0 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ type DefaultCommandRunner struct {
Drainer *Drainer
PreWorkflowHooksCommandRunner PreWorkflowHooksCommandRunner
PullStatusFetcher PullStatusFetcher
TeamAllowlistChecker *TeamAllowlistChecker
}

// RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated.
Expand Down Expand Up @@ -162,6 +163,32 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
autoPlanRunner.Run(ctx, nil)
}

// commentUserDoesNotHavePermissions comments on the pull request that the user
// is not allowed to execute the command.
func (c *DefaultCommandRunner) commentUserDoesNotHavePermissions(baseRepo models.Repo, pullNum int, user models.User, cmd *CommentCommand) {
errMsg := fmt.Sprintf("```\nError: User @%s does not have permissions to execute '%s' command.\n```", user.Username, cmd.Name.String())
if err := c.VCSClient.CreateComment(baseRepo, pullNum, errMsg, ""); err != nil {
c.Logger.Err("unable to comment on pull request: %s", err)
}
}

// 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 {
// allowlist restriction is not enabled
return true, nil
}
teams, err := c.VCSClient.GetTeamNamesForUser(repo, user)
if err != nil {
return false, err
}
ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(teams, cmd.Name.String())
if !ok {
return false, nil
}
return true, nil
}

// RunCommentCommand executes the command.
// We take in a pointer for maybeHeadRepo because for some events there isn't
// enough data to construct the Repo model and callers might want to wait until
Expand All @@ -179,6 +206,17 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
log := c.buildLogger(baseRepo.FullName, pullNum)
defer c.logPanics(baseRepo, pullNum, log)

// Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands
ok, err := c.checkUserPermissions(baseRepo, user, cmd)
if err != nil {
c.Logger.Err("Unable to check user permissions: %s", err)
return
}
if !ok {
c.commentUserDoesNotHavePermissions(baseRepo, pullNum, user, cmd)
return
}

headRepo, pull, err := c.ensureValidRepoMetadata(baseRepo, maybeHeadRepo, maybePull, user, pullNum, log)
if err != nil {
return
Expand Down
69 changes: 69 additions & 0 deletions server/events/team_allowlist_checker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package events

import (
"strings"
)

// Wildcard matches all teams and all commands
const wildcard = "*"

// mapOfStrings is an alias for map[string]string
type mapOfStrings map[string]string

// TeamAllowlistChecker implements checking the teams and the operations that the members
// of a particular team are allowed to perform
type TeamAllowlistChecker struct {
rules []mapOfStrings
}

// NewTeamAllowlistChecker constructs a new checker
func NewTeamAllowlistChecker(allowlist string) (*TeamAllowlistChecker, error) {
var rules []mapOfStrings
pairs := strings.Split(allowlist, ",")
if pairs[0] != "" {
for _, pair := range pairs {
values := strings.Split(pair, ":")
team := strings.TrimSpace(values[0])
command := strings.TrimSpace(values[1])
m := mapOfStrings{team: command}
rules = append(rules, m)
}
}
return &TeamAllowlistChecker{
rules: rules,
}, nil
}

// IsCommandAllowedForTeam returns true if the team is allowed to execute the command
// and false otherwise.
func (checker *TeamAllowlistChecker) IsCommandAllowedForTeam(team string, command string) bool {
for _, rule := range checker.rules {
for key, value := range rule {
if (key == wildcard || strings.EqualFold(key, team)) && (value == wildcard || strings.EqualFold(value, command)) {
return true
}
}
}
return false
}

// IsCommandAllowedForAnyTeam returns true if any of the teams is allowed to execute the command
// and false otherwise.
func (checker *TeamAllowlistChecker) IsCommandAllowedForAnyTeam(teams []string, command string) bool {
if len(teams) == 0 {
for _, rule := range checker.rules {
for key, value := range rule {
if (key == wildcard) && (value == wildcard || strings.EqualFold(value, command)) {
return true
}
}
}
} else {
for _, t := range teams {
if checker.IsCommandAllowedForTeam(t, command) {
return true
}
}
}
return false
}
36 changes: 36 additions & 0 deletions server/events/team_allowlist_checker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package events_test

import (
"testing"

"github.com/runatlantis/atlantis/server/events"
. "github.com/runatlantis/atlantis/testing"
)

func TestNewTeamAllowListChecker(t *testing.T) {
allowlist := `bob:plan, dave:apply`
_, err := events.NewTeamAllowlistChecker(allowlist)
Ok(t, err)
}

func TestIsCommandAllowedForTeam(t *testing.T) {
allowlist := `bob:plan, dave:apply, connie:plan, connie:apply`
checker, err := events.NewTeamAllowlistChecker(allowlist)
Ok(t, err)
Equals(t, true, checker.IsCommandAllowedForTeam("connie", "plan"))
Equals(t, true, checker.IsCommandAllowedForTeam("connie", "apply"))
Equals(t, true, checker.IsCommandAllowedForTeam("dave", "apply"))
Equals(t, true, checker.IsCommandAllowedForTeam("bob", "plan"))
Equals(t, false, checker.IsCommandAllowedForTeam("bob", "apply"))
}

func TestIsCommandAllowedForAnyTeam(t *testing.T) {
allowlist := `alpha:plan,beta:release,*:unlock,nobody:*`
teams := []string{`alpha`, `beta`}
checker, err := events.NewTeamAllowlistChecker(allowlist)
Ok(t, err)
Equals(t, true, checker.IsCommandAllowedForAnyTeam(teams, `plan`))
Equals(t, true, checker.IsCommandAllowedForAnyTeam(teams, `release`))
Equals(t, true, checker.IsCommandAllowedForAnyTeam(teams, `unlock`))
Equals(t, false, checker.IsCommandAllowedForAnyTeam(teams, `noop`))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add some tests for wildcard as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 23c5cd9

5 changes: 5 additions & 0 deletions server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,11 @@ func SplitAzureDevopsRepoFullName(repoFullName string) (owner string, project st
return repoFullName[:lastSlashIdx], "", repoFullName[lastSlashIdx+1:]
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (g *AzureDevopsClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
return nil, nil
}

func (g *AzureDevopsClient) SupportsSingleFileDownload(repo models.Repo) bool {
return false
}
Expand Down
5 changes: 5 additions & 0 deletions server/events/vcs/bitbucketcloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,11 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
return respBody, nil
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (b *Client) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
return nil, nil
}

func (b *Client) SupportsSingleFileDownload(models.Repo) bool {
return false
}
Expand Down
5 changes: 5 additions & 0 deletions server/events/vcs/bitbucketserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,11 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
return respBody, nil
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (b *Client) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
return nil, nil
}

func (b *Client) SupportsSingleFileDownload(repo models.Repo) bool {
return false
}
Expand Down
1 change: 1 addition & 0 deletions server/events/vcs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Client interface {
UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error
MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error
MarkdownPullLink(pull models.PullRequest) (string, error)
GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error)

// DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository)
// The first return value indicate that repo contain atlantis.yaml or not
Expand Down
29 changes: 29 additions & 0 deletions server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,35 @@ func (g *GithubClient) MarkdownPullLink(pull models.PullRequest) (string, error)
return fmt.Sprintf("#%d", pull.Num), nil
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
// https://developer.github.com/v3/teams/members/#get-team-membership
func (g *GithubClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
var teamNames []string
opts := &github.ListOptions{}
org := repo.Owner
for {
teams, resp, err := g.client.Teams.ListTeams(g.ctx, org, opts)
if err != nil {
return nil, errors.Wrap(err, "retrieving GitHub teams")
}
for _, t := range teams {
membership, _, err := g.client.Teams.GetTeamMembershipBySlug(g.ctx, org, *t.Slug, user.Username)
Copy link
Contributor

@albertorm95 albertorm95 Jan 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the rate limiting? Wound't it be better to use the GraphQL so we could just pull the orgs teams of a user?

https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting

--- edit:
https://stackoverflow.com/questions/30466741/github-api-list-a-users-teams-within-an-organization/47538467#47538467 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I haven't worked with the GraphQL API, but it does seem like some calls could be saved that way

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulerickson Thanks for the answer I could try to switch this feature to use GraphQL :)

if err != nil {
g.logger.Err("Failed to get team membership from GitHub: %s", err)
} else if membership != nil {
if *membership.State == "active" && (*membership.Role == "member" || *membership.Role == "maintainer") {
teamNames = append(teamNames, t.GetName())
}
}
}
if resp.NextPage == 0 {
break
}
opts.Page = resp.NextPage
}
return teamNames, nil
}

// ExchangeCode returns a newly created app's info
func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, error) {
ctx := context.Background()
Expand Down
5 changes: 5 additions & 0 deletions server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,11 @@ func MustConstraint(constraint string) version.Constraints {
return c
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (g *GitlabClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
return nil, nil
}

// DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository)
// The first return value indicate that repo contain atlantis.yaml or not
// if BaseRepo had one repo config file, its content will placed on the second return value
Expand Down
30 changes: 30 additions & 0 deletions server/events/vcs/mocks/mock_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions server/events/vcs/not_configured_vcs_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ func (a *NotConfiguredVCSClient) MarkdownPullLink(pull models.PullRequest) (stri
func (a *NotConfiguredVCSClient) err() error {
return fmt.Errorf("atlantis was not configured to support repos from %s", a.Host.String())
}
func (a *NotConfiguredVCSClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
return nil, a.err()
}

func (a *NotConfiguredVCSClient) SupportsSingleFileDownload(repo models.Repo) bool {
return false
Expand Down
4 changes: 4 additions & 0 deletions server/events/vcs/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ func (d *ClientProxy) MarkdownPullLink(pull models.PullRequest) (string, error)
return d.clients[pull.BaseRepo.VCSHost.Type].MarkdownPullLink(pull)
}

func (d *ClientProxy) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
return d.clients[repo.VCSHost.Type].GetTeamNamesForUser(repo, user)
}

func (d *ClientProxy) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) {
return d.clients[pull.BaseRepo.VCSHost.Type].DownloadRepoConfigFile(pull)
}
Expand Down
Loading