From cf840958391cd4e1395222922117bd0f062f0705 Mon Sep 17 00:00:00 2001 From: Pierre Guinoiseau Date: Tue, 28 Nov 2023 17:05:37 +1300 Subject: [PATCH] feat: add support for GitLab groups Signed-off-by: Pierre Guinoiseau --- cmd/server.go | 12 ++++ cmd/server_test.go | 1 + runatlantis.io/docs/server-configuration.md | 16 +++++ .../docs/server-side-repo-config.md | 12 ++-- server/core/config/valid/policies.go | 14 +++++ server/core/config/valid/policies_test.go | 63 +++++++++++++++++++ .../events/command/team_allowlist_checker.go | 14 +++++ server/events/command_runner.go | 8 +-- server/events/command_runner_test.go | 4 +- .../events/external_team_allowlist_checker.go | 4 ++ server/events/mock_workingdir_test.go | 5 +- server/events/project_command_runner.go | 3 +- server/events/project_command_runner_test.go | 2 +- server/events/vcs/azuredevops_client.go | 2 +- server/events/vcs/bitbucketcloud/client.go | 2 +- server/events/vcs/bitbucketserver/client.go | 2 +- server/events/vcs/client.go | 2 +- server/events/vcs/gitea/client.go | 2 +- server/events/vcs/github_client.go | 3 +- server/events/vcs/github_client_test.go | 12 ++-- server/events/vcs/gitlab_client.go | 45 +++++++++++-- server/events/vcs/gitlab_client_test.go | 56 +++++++++++++++-- server/events/vcs/mocks/mock_client.go | 30 +++++---- .../events/vcs/not_configured_vcs_client.go | 2 +- server/events/vcs/proxy.go | 4 +- .../gitlab-group-membership-success.json | 22 +++++++ .../vcs/testdata/gitlab-user-success.json | 11 ++++ server/server.go | 17 ++++- server/user_config.go | 1 + 29 files changed, 317 insertions(+), 54 deletions(-) create mode 100644 server/events/vcs/testdata/gitlab-group-membership-success.json create mode 100644 server/events/vcs/testdata/gitlab-user-success.json diff --git a/cmd/server.go b/cmd/server.go index 5722b38cfa..3cec86fe14 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -106,6 +106,7 @@ const ( GiteaUserFlag = "gitea-user" GiteaWebhookSecretFlag = "gitea-webhook-secret" // nolint: gosec GiteaPageSizeFlag = "gitea-page-size" + GitlabGroupAllowlistFlag = "gitlab-group-allowlist" GitlabHostnameFlag = "gitlab-hostname" GitlabTokenFlag = "gitlab-token" GitlabUserFlag = "gitlab-user" @@ -358,6 +359,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_GITEA_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, diff --git a/cmd/server_test.go b/cmd/server_test.go index c14e43cdd6..02ce6f3ab0 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -100,6 +100,7 @@ var testFlags = map[string]interface{}{ GiteaUserFlag: "gitea-user", GiteaWebhookSecretFlag: "gitea-secret", GiteaPageSizeFlag: 30, + GitlabGroupAllowlistFlag: "", GitlabHostnameFlag: "gitlab-hostname", GitlabTokenFlag: "gitlab-token", GitlabUserFlag: "gitlab-user", diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 303e9df067..1692caf06a 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -770,6 +770,22 @@ based on the organization or user that triggered the webhook. 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 diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md index 2469eec4d7..bac3429ce8 100644 --- a/runatlantis.io/docs/server-side-repo-config.md +++ b/runatlantis.io/docs/server-side-repo-config.md @@ -592,12 +592,12 @@ mode: on_apply ### 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 diff --git a/server/core/config/valid/policies.go b/server/core/config/valid/policies.go index 6aee54179c..718338d05b 100644 --- a/server/core/config/valid/policies.go +++ b/server/core/config/valid/policies.go @@ -1,6 +1,7 @@ package valid import ( + "slices" "strings" version "github.com/hashicorp/go-version" @@ -67,3 +68,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 +} diff --git a/server/core/config/valid/policies_test.go b/server/core/config/valid/policies_test.go index c575a4585a..5147dd8686 100644 --- a/server/core/config/valid/policies_test.go +++ b/server/core/config/valid/policies_test.go @@ -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) + }) + } +} diff --git a/server/events/command/team_allowlist_checker.go b/server/events/command/team_allowlist_checker.go index 5c58873650..f71684a6d7 100644 --- a/server/events/command/team_allowlist_checker.go +++ b/server/events/command/team_allowlist_checker.go @@ -21,6 +21,9 @@ type TeamAllowlistChecker interface { // IsCommandAllowedForAnyTeam determines if any of the specified teams can perform the specified action IsCommandAllowedForAnyTeam(ctx models.TeamAllowlistCheckerContext, teams []string, command string) bool + + // AllTeams returns all teams configured in the allowlist + AllTeams() []string } // DefaultTeamAllowlistChecker implements checking the teams and the operations that the members @@ -84,3 +87,14 @@ func (checker *DefaultTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx model } return false } + +// AllTeams returns all teams configured in the allowlist +func (checker *DefaultTeamAllowlistChecker) AllTeams() []string { + var teamNames []string + for _, rule := range checker.rules { + for key := range rule { + teamNames = append(teamNames, key) + } + } + return teamNames +} diff --git a/server/events/command_runner.go b/server/events/command_runner.go index fdd4b39153..ac42d1e7aa 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -157,7 +157,7 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo // Check if the user who triggered the autoplan has permissions to run 'plan'. if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() { - err := c.fetchUserTeams(baseRepo, &user) + err := c.fetchUserTeams(log, baseRepo, &user) if err != nil { log.Err("Unable to fetch user teams: %s", err) return @@ -300,7 +300,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead // Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() { - err := c.fetchUserTeams(baseRepo, &user) + err := c.fetchUserTeams(log, baseRepo, &user) if err != nil { c.Logger.Err("Unable to fetch user teams: %s", err) return @@ -491,8 +491,8 @@ func (c *DefaultCommandRunner) ensureValidRepoMetadata( return } -func (c *DefaultCommandRunner) fetchUserTeams(repo models.Repo, user *models.User) error { - teams, err := c.VCSClient.GetTeamNamesForUser(repo, *user) +func (c *DefaultCommandRunner) fetchUserTeams(logger logging.SimpleLogging, repo models.Repo, user *models.User) error { + teams, err := c.VCSClient.GetTeamNamesForUser(logger, repo, *user) if err != nil { return err } diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 8764c08bea..46a812f4b3 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -313,7 +313,7 @@ func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) { When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(&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(ch.Logger, testdata.GithubRepo, testdata.User) vcsClient.VerifyWasCalledOnce().CreateComment( Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull.Num), Eq("Ran Plan for 0 projects:"), Eq("plan")) }) @@ -331,7 +331,7 @@ func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) { When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(&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(ch.Logger, testdata.GithubRepo, testdata.User) vcsClient.VerifyWasCalledOnce().CreateComment( Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull.Num), Eq("Ran Plan for 0 projects:"), Eq("plan")) }) diff --git a/server/events/external_team_allowlist_checker.go b/server/events/external_team_allowlist_checker.go index 9f3fe419ef..6592b6b181 100644 --- a/server/events/external_team_allowlist_checker.go +++ b/server/events/external_team_allowlist_checker.go @@ -39,6 +39,10 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx mode return checker.checkOutputResults(out) } +func (checker *ExternalTeamAllowlistChecker) AllTeams() []string { + return []string{} +} + func (checker *ExternalTeamAllowlistChecker) buildCommandString(ctx models.TeamAllowlistCheckerContext, teams []string, command string) string { // Build command string // Format is "$external_cmd $external_args $command $repo $teams" diff --git a/server/events/mock_workingdir_test.go b/server/events/mock_workingdir_test.go index c11b9e28bf..65d5fc00a7 100644 --- a/server/events/mock_workingdir_test.go +++ b/server/events/mock_workingdir_test.go @@ -4,12 +4,11 @@ package events import ( - "reflect" - "time" - pegomock "github.com/petergtz/pegomock/v4" models "github.com/runatlantis/atlantis/server/events/models" logging "github.com/runatlantis/atlantis/server/logging" + "reflect" + "time" ) type MockWorkingDir struct { diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index 8c5d810cca..150e5e8fef 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -225,6 +225,7 @@ type DefaultProjectCommandRunner struct { VcsClient vcs.Client Locker ProjectLocker LockURLGenerator LockURLGenerator + Logger logging.SimpleLogging InitStepRunner StepRunner PlanStepRunner StepRunner ShowStepRunner StepRunner @@ -367,7 +368,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(p.Logger, ctx.Pull.BaseRepo, ctx.User) if err != nil { ctx.Log.Err("unable to get team membership for user: %s", err) return nil, "", err diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index 68548efdd0..0e2baa2dfd 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -1276,7 +1276,7 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) { } modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num, Author: testdata.User.Username} - When(runner.VcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User)).ThenReturn(c.userTeams, nil) + When(runner.VcsClient.GetTeamNamesForUser(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.User))).ThenReturn(c.userTeams, nil) ctx := command.ProjectContext{ User: testdata.User, Log: logging.NewNoopLogger(t), diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 35c303bae2..77ccf948e5 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -393,7 +393,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(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) { //nolint: revive return nil, nil } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index b777030237..bc5cd95e97 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -266,7 +266,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(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) { return nil, nil } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 058b411100..47f61a526a 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -350,7 +350,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(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) { return nil, nil } diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 9e32981a82..3344ffa6e9 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -42,7 +42,7 @@ type Client interface { DiscardReviews(repo models.Repo, pull models.PullRequest) error MergePull(logger logging.SimpleLogging, pull models.PullRequest, pullOptions models.PullRequestOptions) error MarkdownPullLink(pull models.PullRequest) (string, error) - GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) + GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]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 diff --git a/server/events/vcs/gitea/client.go b/server/events/vcs/gitea/client.go index e971534288..e262d8d820 100644 --- a/server/events/vcs/gitea/client.go +++ b/server/events/vcs/gitea/client.go @@ -413,7 +413,7 @@ func (c *GiteaClient) 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). -func (c *GiteaClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) { +func (c *GiteaClient) GetTeamNamesForUser(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) { // TODO: implement return nil, errors.New("GetTeamNamesForUser not (yet) implemented for Gitea client") } diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 1000f73e07..17aad85646 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -1009,7 +1009,8 @@ 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(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]string, error) { + logger.Debug("Getting GitHub team names for user '%s'", user) orgName := repo.Owner variables := map[string]interface{}{ "orgName": githubv4.String(orgName), diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index ffd0e02e59..7a926f5223 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -1397,11 +1397,13 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) { Ok(t, err) defer disableSSLVerification()() - teams, err := client.GetTeamNamesForUser(models.Repo{ - Owner: "testrepo", - }, models.User{ - Username: "testuser", - }) + teams, err := client.GetTeamNamesForUser( + logger, + models.Repo{ + Owner: "testrepo", + }, models.User{ + Username: "testuser", + }) Ok(t, err) Equals(t, []string{"Frontend Developers", "frontend-developers", "Employees", "employees"}, teams) } diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index fffe8c63e9..7cea7c25b7 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -41,6 +41,8 @@ type GitlabClient struct { Client *gitlab.Client // Version is set to the server version. Version *version.Version + // All GitLab groups configured in allowlists and policies + ConfiguredGroups []string // PollingInterval is the time between successive polls, where applicable. PollingInterval time.Duration // PollingInterval is the total duration for which to poll, where applicable. @@ -56,11 +58,12 @@ var commonMarkSupported = MustConstraint(">=11.1") var gitlabClientUnderTest = false // NewGitlabClient returns a valid GitLab client. -func NewGitlabClient(hostname string, token string, logger logging.SimpleLogging) (*GitlabClient, error) { +func NewGitlabClient(hostname string, token string, configuredGroups []string, logger logging.SimpleLogging) (*GitlabClient, error) { logger.Debug("Creating new GitLab client for %s", hostname) client := &GitlabClient{ - PollingInterval: time.Second, - PollingTimeout: time.Second * 30, + ConfiguredGroups: configuredGroups, + PollingInterval: time.Second, + PollingTimeout: time.Second * 30, } // Create the client differently depending on the base URL. @@ -620,9 +623,39 @@ 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(logger logging.SimpleLogging, _ models.Repo, user models.User) ([]string, error) { + logger.Debug("Getting GitLab group names for user '%s'", user) + 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 g.ConfiguredGroups { + 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) diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 8aee1e865a..796e8a844a 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -94,7 +94,7 @@ func TestNewGitlabClient_BaseURL(t *testing.T) { for _, c := range cases { t.Run(c.Hostname, func(t *testing.T) { log := logging.NewNoopLogger(t) - client, err := NewGitlabClient(c.Hostname, "token", log) + client, err := NewGitlabClient(c.Hostname, "token", []string{}, log) Ok(t, err) Equals(t, c.ExpBaseURL, client.Client.BaseURL().String()) }) @@ -887,7 +887,7 @@ func TestGitlabClient_MarkdownPullLink(t *testing.T) { logger := logging.NewNoopLogger(t) gitlabClientUnderTest = true defer func() { gitlabClientUnderTest = false }() - client, err := NewGitlabClient("gitlab.com", "token", logger) + client, err := NewGitlabClient("gitlab.com", "token", []string{}, logger) Ok(t, err) pull := models.PullRequest{Num: 1} s, _ := client.MarkdownPullLink(pull) @@ -1039,7 +1039,7 @@ func TestGitlabClient_HideOldComments(t *testing.T) { } } -func TestGithubClient_GetPullLabels(t *testing.T) { +func TestGitlabClient_GetPullLabels(t *testing.T) { logger := logging.NewNoopLogger(t) mergeSuccessWithLabel, err := os.ReadFile("testdata/gitlab-merge-success-with-label.json") Ok(t, err) @@ -1076,7 +1076,7 @@ func TestGithubClient_GetPullLabels(t *testing.T) { Equals(t, []string{"work in progress"}, labels) } -func TestGithubClient_GetPullLabels_EmptyResponse(t *testing.T) { +func TestGitlabClient_GetPullLabels_EmptyResponse(t *testing.T) { logger := logging.NewNoopLogger(t) pipelineSuccess, err := os.ReadFile("testdata/gitlab-pipeline-success.json") Ok(t, err) @@ -1110,3 +1110,51 @@ func TestGithubClient_GetPullLabels_EmptyResponse(t *testing.T) { Ok(t, err) Equals(t, 0, len(labels)) } + +// GetTeamNamesForUser returns the names of the GitLab groups that the user belongs to. +func TestGitlabClient_GetTeamNamesForUser(t *testing.T) { + logger := logging.NewNoopLogger(t) + + groupMembershipSuccess, err := os.ReadFile("testdata/gitlab-group-membership-success.json") + Ok(t, err) + + userSuccess, err := os.ReadFile("testdata/gitlab-user-success.json") + Ok(t, err) + + configuredGroups := []string{"someorg/group1", "someorg/group2", "someorg/group3", "someorg/group4"} + testServer := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v4/users?username=testuser": + w.WriteHeader(http.StatusOK) + w.Write(userSuccess) // nolint: errcheck + case "/api/v4/groups/someorg%2Fgroup1/members/123", "/api/v4/groups/someorg%2Fgroup2/members/123": + w.WriteHeader(http.StatusOK) + w.Write(groupMembershipSuccess) // nolint: errcheck + case "/api/v4/groups/someorg%2Fgroup3/members/123": + http.Error(w, "forbidden", http.StatusForbidden) + case "/api/v4/groups/someorg%2Fgroup4/members/123": + http.Error(w, "not found", http.StatusNotFound) + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + } + })) + internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) + Ok(t, err) + client := &GitlabClient{ + Client: internalClient, + Version: nil, + ConfiguredGroups: configuredGroups, + } + + teams, err := client.GetTeamNamesForUser( + logger, + models.Repo{ + Owner: "someorg", + }, models.User{ + Username: "testuser", + }) + Ok(t, err) + Equals(t, []string{"someorg/group1", "someorg/group2"}, teams) +} diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index e798b8b79a..3b4bd7dbf7 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -136,11 +136,11 @@ func (mock *MockClient) GetPullLabels(logger logging.SimpleLogging, repo models. return _ret0, _ret1 } -func (mock *MockClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) { +func (mock *MockClient) GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } - _params := []pegomock.Param{repo, user} + _params := []pegomock.Param{logger, repo, user} _result := pegomock.GetGenericMockFrom(mock).Invoke("GetTeamNamesForUser", _params, []reflect.Type{reflect.TypeOf((*[]string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var _ret0 []string var _ret1 error @@ -576,8 +576,8 @@ func (c *MockClient_GetPullLabels_OngoingVerification) GetAllCapturedArguments() return } -func (verifier *VerifierMockClient) GetTeamNamesForUser(repo models.Repo, user models.User) *MockClient_GetTeamNamesForUser_OngoingVerification { - _params := []pegomock.Param{repo, user} +func (verifier *VerifierMockClient) GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) *MockClient_GetTeamNamesForUser_OngoingVerification { + _params := []pegomock.Param{logger, repo, user} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetTeamNamesForUser", _params, verifier.timeout) return &MockClient_GetTeamNamesForUser_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -587,24 +587,30 @@ type MockClient_GetTeamNamesForUser_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockClient_GetTeamNamesForUser_OngoingVerification) GetCapturedArguments() (models.Repo, models.User) { - repo, user := c.GetAllCapturedArguments() - return repo[len(repo)-1], user[len(user)-1] +func (c *MockClient_GetTeamNamesForUser_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, models.Repo, models.User) { + logger, repo, user := c.GetAllCapturedArguments() + return logger[len(logger)-1], repo[len(repo)-1], user[len(user)-1] } -func (c *MockClient_GetTeamNamesForUser_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.User) { +func (c *MockClient_GetTeamNamesForUser_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []models.Repo, _param2 []models.User) { _params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(_params) > 0 { if len(_params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) for u, param := range _params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(logging.SimpleLogging) } } if len(_params) > 1 { - _param1 = make([]models.User, len(c.methodInvocations)) + _param1 = make([]models.Repo, len(c.methodInvocations)) for u, param := range _params[1] { - _param1[u] = param.(models.User) + _param1[u] = param.(models.Repo) + } + } + if len(_params) > 2 { + _param2 = make([]models.User, len(c.methodInvocations)) + for u, param := range _params[2] { + _param2[u] = param.(models.User) } } } diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index 41b14ad2c6..0a48f210b6 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -60,7 +60,7 @@ func (a *NotConfiguredVCSClient) MarkdownPullLink(_ models.PullRequest) (string, func (a *NotConfiguredVCSClient) err() error { return fmt.Errorf("atlantis was not configured to support repos from %s", a.Host.String()) } -func (a *NotConfiguredVCSClient) GetTeamNamesForUser(_ models.Repo, _ models.User) ([]string, error) { +func (a *NotConfiguredVCSClient) GetTeamNamesForUser(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) { return nil, a.err() } diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 68aa45bf58..71d66116b1 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -97,8 +97,8 @@ 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) GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]string, error) { + return d.clients[repo.VCSHost.Type].GetTeamNamesForUser(logger, repo, user) } func (d *ClientProxy) GetFileContent(logger logging.SimpleLogging, pull models.PullRequest, fileName string) (bool, []byte, error) { diff --git a/server/events/vcs/testdata/gitlab-group-membership-success.json b/server/events/vcs/testdata/gitlab-group-membership-success.json new file mode 100644 index 0000000000..897c4438ad --- /dev/null +++ b/server/events/vcs/testdata/gitlab-group-membership-success.json @@ -0,0 +1,22 @@ +{ + "access_level": 50, + "created_at": "2023-11-28T01:23:45.789Z", + "created_by": { + "id": 456, + "username": "someone", + "name": "Someone", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/456/avatar.png", + "web_url": "https://gitlab.com/someone" + }, + "expires_at": null, + "id": 123, + "username": "testuser", + "name": "Test User", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/123/avatar.png", + "web_url": "https://gitlab.com/testuser", + "membership_state": "active" +} diff --git a/server/events/vcs/testdata/gitlab-user-success.json b/server/events/vcs/testdata/gitlab-user-success.json new file mode 100644 index 0000000000..0b87fc9e12 --- /dev/null +++ b/server/events/vcs/testdata/gitlab-user-success.json @@ -0,0 +1,11 @@ +[ + { + "id": 123, + "username": "testuser", + "name": "Test User", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/123/avatar.png", + "web_url": "https://gitlab.com/testuser" + } +] diff --git a/server/server.go b/server/server.go index 22f6db5498..2bfea19af3 100644 --- a/server/server.go +++ b/server/server.go @@ -28,6 +28,7 @@ import ( "os" "os/signal" "path/filepath" + "slices" "sort" "strings" "syscall" @@ -269,7 +270,15 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { if userConfig.GitlabUser != "" { supportedVCSHosts = append(supportedVCSHosts, models.Gitlab) var err error - gitlabClient, err = vcs.NewGitlabClient(userConfig.GitlabHostname, userConfig.GitlabToken, logger) + + gitlabGroupAllowlistChecker, err := command.NewTeamAllowlistChecker(userConfig.GitlabGroupAllowlist) + if err != nil { + return nil, err + } + + gitlabGroups := slices.Concat(gitlabGroupAllowlistChecker.AllTeams(), globalCfg.PolicySets.AllTeams()) + slices.Sort(gitlabGroups) + gitlabClient, err = vcs.NewGitlabClient(userConfig.GitlabHostname, userConfig.GitlabToken, slices.Compact(gitlabGroups), logger) if err != nil { return nil, err } @@ -671,6 +680,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { VcsClient: vcsClient, Locker: projectLocker, LockURLGenerator: router, + Logger: logger, InitStepRunner: &runtime.InitStepRunner{ TerraformExecutor: terraformClient, DefaultTFVersion: defaultTfVersion, @@ -834,6 +844,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { ExtraArgs: globalCfg.TeamAuthz.Args, ExternalTeamAllowlistRunner: &runtime.DefaultExternalTeamAllowlistRunner{}, } + } else if userConfig.GitlabUser != "" { + teamAllowlistChecker, err = command.NewTeamAllowlistChecker(userConfig.GitlabGroupAllowlist) + if err != nil { + return nil, err + } } else { teamAllowlistChecker, err = command.NewTeamAllowlistChecker(userConfig.GithubTeamAllowlist) if err != nil { diff --git a/server/user_config.go b/server/user_config.go index 10e6e6b9fc..5321e98051 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -66,6 +66,7 @@ type UserConfig struct { GiteaWebhookSecret string `mapstructure:"gitea-webhook-secret"` GiteaPageSize int `mapstructure:"gitea-page-size"` GitlabHostname string `mapstructure:"gitlab-hostname"` + GitlabGroupAllowlist string `mapstructure:"gitlab-group-allowlist"` GitlabToken string `mapstructure:"gitlab-token"` GitlabUser string `mapstructure:"gitlab-user"` GitlabWebhookSecret string `mapstructure:"gitlab-webhook-secret"`