Skip to content

Commit

Permalink
feat: add support for GitLab groups
Browse files Browse the repository at this point in the history
  • Loading branch information
peikk0 committed Dec 13, 2023
1 parent 27475fb commit 3123763
Show file tree
Hide file tree
Showing 24 changed files with 256 additions and 41 deletions.
12 changes: 12 additions & 0 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const (
GHOrganizationFlag = "gh-org"
GHWebhookSecretFlag = "gh-webhook-secret" // nolint: gosec
GHAllowMergeableBypassApply = "gh-allow-mergeable-bypass-apply" // nolint: gosec
GitlabGroupAllowlistFlag = "gitlab-group-allowlist"
GitlabHostnameFlag = "gitlab-hostname"
GitlabTokenFlag = "gitlab-token"
GitlabUserFlag = "gitlab-user"
Expand Down Expand Up @@ -319,6 +320,17 @@ var stringFlags = map[string]stringFlag{
"This means that an attacker could spoof calls to Atlantis and cause it to perform malicious actions. " +
"Should be specified via the ATLANTIS_GH_WEBHOOK_SECRET environment variable.",
},
GitlabGroupAllowlistFlag: {
description: "Comma separated list of key-value pairs representing the GitLab groups and the operations that " +
"the members of a particular group are allowed to perform. " +
"The format is {group}:{command},{group}:{command}. " +
"Valid values for 'command' are 'plan', 'apply' and '*', e.g. 'myorg/dev:plan,myorg/ops:apply,myorg/devops:*'" +
"This example gives the users from the 'myorg/dev' GitLab group the permissions to execute the 'plan' command, " +
"the 'myorg/ops' group the permissions to execute the 'apply' command, " +
"and allows the 'myorg/devops' group 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 group to perform any operation.",
},
GitlabHostnameFlag: {
description: "Hostname of your GitLab Enterprise installation. If using gitlab.com, no need to set.",
defaultValue: DefaultGitlabHostname,
Expand Down
14 changes: 14 additions & 0 deletions runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,20 @@ and set `--autoplan-modules` to `false`.
This means that an attacker could spoof calls to Atlantis and cause it to perform malicious actions.
:::

### `--gitlab-group-allowlist`
```bash
atlantis server --gitlab-group-allowlist="myorg/mygroup:plan, myorg/secteam:apply, myorg/devops:apply, myorg/devops:import"
# or
ATLANTIS_GITLAB_GROUP_ALLOWLIST="myorg/mygroup:plan, myorg/secteam:apply, myorg/devops:apply, myorg/devops:import"
```
Comma-separated list of GitLab groups and permission pairs.

By default, any group can plan and apply.

::: warning NOTE
Atlantis needs to be able to view the listed group members, inaccessible or non-existent groups are silently ignored.
:::

### `--gitlab-hostname`
```bash
atlantis server --gitlab-hostname="my.gitlab.enterprise.com"
Expand Down
20 changes: 10 additions & 10 deletions runatlantis.io/docs/server-side-repo-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -536,18 +536,18 @@ If you set a workflow with the key `default`, it will override this.

### Policies

| Key | Type | Default | Required | Description |
|------------------------|-----------------|---------|-----------|----------------------------------------------------------|
| conftest_version | string | none | no | conftest version to run all policy sets |
| owners | Owners(#Owners) | none | yes | owners that can approve failing policies |
| approve_count | int | 1 | no | number of approvals required to bypass failing policies. |
| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output |
| Key | Type | Default | Required | Description |
|------------------------|-----------------|---------|-----------|---------------------------------------------------------|
| conftest_version | string | none | no | conftest version to run all policy sets |
| owners | Owners(#Owners) | none | yes | owners that can approve failing policies |
| approve_count | int | 1 | no | number of approvals required to bypass failing policies |
| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output |

### Owners
| Key | Type | Default | Required | Description |
|-------------|-------------------|---------|------------|---------------------------------------------------------|
| users | []string | none | no | list of github users that can approve failing policies |
| teams | []string | none | no | list of github teams that can approve failing policies |
| Key | Type | Default | Required | Description |
|-------------|-------------------|---------|------------|-------------------------------------------------------------------------|
| users | []string | none | no | list of GitHub or GitLab users that can approve failing policies |
| teams | []string | none | no | list of GitHub teams or GitLab groups that can approve failing policies |

### PolicySet

Expand Down
14 changes: 14 additions & 0 deletions server/core/config/valid/policies.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package valid

import (
"slices"
"strings"

version "github.com/hashicorp/go-version"
Expand Down Expand Up @@ -66,3 +67,16 @@ func (o *PolicyOwners) IsOwner(username string, userTeams []string) bool {

return false
}

// Return all owner teams from all policy sets
func (p *PolicySets) AllTeams() []string {
teams := p.Owners.Teams
for _, policySet := range p.PolicySets {
for _, team := range policySet.Owners.Teams {
if !slices.Contains(teams, team) {
teams = append(teams, team)
}
}
}
return teams
}
63 changes: 63 additions & 0 deletions server/core/config/valid/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,66 @@ func TestPoliciesConfig_IsOwners(t *testing.T) {
})
}
}

func TestPoliciesConfig_AllTeams(t *testing.T) {
cases := []struct {
description string
input valid.PolicySets
expResult []string
}{
{
description: "has only top-level team owner",
input: valid.PolicySets{
Owners: valid.PolicyOwners{
Teams: []string{
"team1",
},
},
},
expResult: []string{"team1"},
},
{
description: "has only policy-level team owner",
input: valid.PolicySets{
PolicySets: []valid.PolicySet{
{
Name: "policy1",
Owners: valid.PolicyOwners{
Teams: []string{
"team2",
},
},
},
},
},
expResult: []string{"team2"},
},
{
description: "has both top-level and policy-level team owners",
input: valid.PolicySets{
Owners: valid.PolicyOwners{
Teams: []string{
"team1",
},
},
PolicySets: []valid.PolicySet{
{
Name: "policy1",
Owners: valid.PolicyOwners{
Teams: []string{
"team2",
},
},
},
},
},
expResult: []string{"team1", "team2"},
},
}
for _, c := range cases {
t.Run(c.description, func(t *testing.T) {
result := c.input.AllTeams()
Equals(t, c.expResult, result)
})
}
}
21 changes: 17 additions & 4 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ type DefaultCommandRunner struct {
PreWorkflowHooksCommandRunner PreWorkflowHooksCommandRunner
PostWorkflowHooksCommandRunner PostWorkflowHooksCommandRunner
PullStatusFetcher PullStatusFetcher
TeamAllowlistChecker *TeamAllowlistChecker
GitHubTeamAllowlistChecker *TeamAllowlistChecker
GitLabGroupAllowlistChecker *TeamAllowlistChecker
VarFileAllowlistChecker *VarFileAllowlistChecker
CommitStatusUpdater CommitStatusUpdater
}
Expand Down Expand Up @@ -238,15 +239,27 @@ 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, cmdName string) (bool, error) {
if c.TeamAllowlistChecker == nil || !c.TeamAllowlistChecker.HasRules() {
var teamAllowListChecker *TeamAllowlistChecker

switch repo.VCSHost.Type {
case models.Github:
teamAllowListChecker = c.GitHubTeamAllowlistChecker
case models.Gitlab:
teamAllowListChecker = c.GitLabGroupAllowlistChecker
default:
// allowlist restriction is not supported
return true, nil
}

if teamAllowListChecker == nil || !teamAllowListChecker.HasRules() {
// allowlist restriction is not enabled
return true, nil
}
teams, err := c.VCSClient.GetTeamNamesForUser(repo, user)
teams, err := c.VCSClient.GetTeamNamesForUser(repo, user, teamAllowListChecker.AllTeamsForCommand(cmdName))
if err != nil {
return false, err
}
ok := c.TeamAllowlistChecker.IsCommandAllowedForAnyTeam(teams, cmdName)
ok := teamAllowListChecker.IsCommandAllowedForAnyTeam(teams, cmdName)
if !ok {
return false, nil
}
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 @@ -298,7 +298,7 @@ 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
ch.GitHubTeamAllowlistChecker = nil
var pull github.PullRequest
modelPull := models.PullRequest{
BaseRepo: testdata.GithubRepo,
Expand All @@ -308,14 +308,14 @@ func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) {
When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)

ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(testdata.GithubRepo, testdata.User)
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(testdata.GithubRepo, testdata.User, []string{})
vcsClient.VerifyWasCalledOnce().CreateComment(testdata.GithubRepo, modelPull.Num, "Ran Plan for 0 projects:", "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{}
ch.GitHubTeamAllowlistChecker = &events.TeamAllowlistChecker{}
var pull github.PullRequest
modelPull := models.PullRequest{
BaseRepo: testdata.GithubRepo,
Expand All @@ -325,7 +325,7 @@ func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) {
When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)

ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(testdata.GithubRepo, testdata.User)
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(testdata.GithubRepo, testdata.User, []string{})
vcsClient.VerifyWasCalledOnce().CreateComment(testdata.GithubRepo, modelPull.Num, "Ran Plan for 0 projects:", "plan")
})
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/project_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte
// Only query the users team membership if any teams have been configured as owners on any policy set(s).
if policySetCfg.HasTeamOwners() {
// A convenient way to access vcsClient. Not sure if best way.
userTeams, err := p.VcsClient.GetTeamNamesForUser(ctx.Pull.BaseRepo, ctx.User)
userTeams, err := p.VcsClient.GetTeamNamesForUser(ctx.Pull.BaseRepo, ctx.User, policySetCfg.AllTeams())
if err != nil {
ctx.Log.Err("unable to get team membership for user: %s", err)
return nil, "", err
Expand Down
2 changes: 1 addition & 1 deletion server/events/project_command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) {
}

modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num}
When(runner.VcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User)).ThenReturn(c.userTeams, nil)
When(runner.VcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User, c.policySetCfg.AllTeams())).ThenReturn(c.userTeams, nil)
ctx := command.ProjectContext{
User: testdata.User,
Log: logging.NewNoopLogger(t),
Expand Down
13 changes: 13 additions & 0 deletions server/events/team_allowlist_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,16 @@ func (checker *TeamAllowlistChecker) IsCommandAllowedForAnyTeam(teams []string,
}
return false
}

// AllTeams returns all teams listed in the rule for a command
func (checker *TeamAllowlistChecker) AllTeamsForCommand(command string) []string {
var teamNames []string
for _, rule := range checker.rules {
for key, value := range rule {
if strings.EqualFold(value, command) {
teamNames = append(teamNames, key)
}
}
}
return teamNames
}
8 changes: 8 additions & 0 deletions server/events/team_allowlist_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,11 @@ func TestIsCommandAllowedForAnyTeam(t *testing.T) {
Equals(t, true, checker.IsCommandAllowedForAnyTeam(teams, `unlock`))
Equals(t, false, checker.IsCommandAllowedForAnyTeam(teams, `noop`))
}

func TestAllTeamsForCommand(t *testing.T) {
allowlist := `bob:plan, dave:apply, connie:plan, connie:apply`
checker, err := events.NewTeamAllowlistChecker(allowlist)
Ok(t, err)
Equals(t, []string{"bob", "connie"}, checker.AllTeamsForCommand(`plan`))
Equals(t, []string{"dave", "connie"}, checker.AllTeamsForCommand(`apply`))
}
2 changes: 1 addition & 1 deletion server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func SplitAzureDevopsRepoFullName(repoFullName string) (owner string, project st
}

// 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) { //nolint: revive
func (g *AzureDevopsClient) GetTeamNamesForUser(repo models.Repo, user models.User, configuredTeams []string) ([]string, error) { //nolint: revive
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketcloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
}

// 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(_ models.Repo, _ models.User) ([]string, error) {
func (b *Client) GetTeamNamesForUser(_ models.Repo, _ models.User, _ []string) ([]string, error) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
}

// 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(_ models.Repo, _ models.User) ([]string, error) {
func (b *Client) GetTeamNamesForUser(_ models.Repo, _ models.User, _ []string) ([]string, error) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type Client interface {
DiscardReviews(repo models.Repo, pull models.PullRequest) error
MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error
MarkdownPullLink(pull models.PullRequest) (string, error)
GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error)
GetTeamNamesForUser(repo models.Repo, user models.User, configuredTeams []string) ([]string, error)

// GetFileContent a repository file content from VCS (which support fetch a single file from repository)
// The first return value indicates whether the repo contains a file or not
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ func (g *GithubClient) MarkdownPullLink(pull models.PullRequest) (string, error)

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
// https://docs.github.com/en/graphql/reference/objects#organization
func (g *GithubClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
func (g *GithubClient) GetTeamNamesForUser(repo models.Repo, user models.User, _ []string) ([]string, error) {
orgName := repo.Owner
variables := map[string]interface{}{
"orgName": githubv4.String(orgName),
Expand Down
3 changes: 2 additions & 1 deletion server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,8 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) {
Owner: "testrepo",
}, models.User{
Username: "testuser",
})
},
[]string{})
Ok(t, err)
Equals(t, []string{"Frontend Developers", "frontend-developers", "Employees", "employees"}, teams)
}
Expand Down
35 changes: 32 additions & 3 deletions server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,38 @@ 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(_ models.Repo, _ models.User) ([]string, error) {
return nil, nil
// GetTeamNamesForUser returns the names of the GitLab groups that the user belongs to.
// The user membership is checked in each group from configuredTeams, groups
// that the Atlantis user doesn't have access to are silently ignored.
func (g *GitlabClient) GetTeamNamesForUser(repo models.Repo, user models.User, configuredTeams []string) ([]string, error) {
var teamNames []string

users, resp, err := g.Client.Users.ListUsers(&gitlab.ListUsersOptions{Username: &user.Username})
if resp.StatusCode == http.StatusNotFound {
return teamNames, nil
}
if err != nil {
return nil, errors.Wrapf(err, "GET /users returned: %d", resp.StatusCode)
} else if len(users) == 0 {
return nil, errors.Wrap(err, "GET /users returned no user")
} else if len(users) > 1 {
// Theoretically impossible, just being extra safe
return nil, errors.Wrap(err, "GET /users returned more than 1 user")
}
userID := users[0].ID
for _, groupName := range configuredTeams {
membership, resp, err := g.Client.GroupMembers.GetGroupMember(groupName, userID)
if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusForbidden {
continue
}
if err != nil {
return nil, errors.Wrapf(err, "GET /groups/%s/members/%d returned: %d", groupName, userID, resp.StatusCode)
}
if resp.StatusCode == http.StatusOK && membership.State == "active" {
teamNames = append(teamNames, groupName)
}
}
return teamNames, nil
}

// GetFileContent a repository file content from VCS (which support fetch a single file from repository)
Expand Down
Loading

0 comments on commit 3123763

Please sign in to comment.