feat(github): support team hierarchy in GH_TEAM_ALLOWLIST#6365
Open
hussein-mimi wants to merge 1 commit intorunatlantis:mainfrom
Open
feat(github): support team hierarchy in GH_TEAM_ALLOWLIST#6365hussein-mimi wants to merge 1 commit intorunatlantis:mainfrom
hussein-mimi wants to merge 1 commit intorunatlantis:mainfrom
Conversation
When a parent team is added to ATLANTIS_GH_TEAM_ALLOWLIST, users who are members of any descendant (child/grandchild) team are now correctly authorized, instead of being rejected. The fix adds GetChildTeams to the GitHub client, which fetches direct child teams via GraphQL. In checkUserPermissions, after the fast-path direct-membership check fails, each allowlisted team is expanded to all its descendants (up to 20 levels deep) using recursive GetChildTeams calls. The user's direct teams are then checked against this expanded set. Non-GitHub VCS clients are unaffected. Fixes runatlantis#6107 Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
dosubot
bot
added
feature
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for GitHub team hierarchy in the Atlantis allowlist. When a parent team is added to ATLANTIS_GH_TEAM_ALLOWLIST, users who are members of descendant (child/grandchild/etc.) teams are now correctly authorized. The implementation uses a duck-typed interface to support team hierarchies for VCS clients that support them (like GitHub), while remaining compatible with VCS clients that don't support this feature.
Changes:
- Added
GetChildTeams()method to the GitHub client that queries the GraphQL API for direct child teams with pagination support - Added
childTeamFetcherinterface for VCS clients supporting team hierarchies - Added
fetchDescendantTeams()recursive function with depth limiting to expand teams to all descendants - Updated
checkUserPermissions()with a two-path approach: fast path for direct membership, slow path for hierarchical expansion - Added test for
GetChildTeams()method
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/events/vcs/github/client.go | Added GetChildTeams() method to query GitHub's GraphQL API for direct child teams with pagination |
| server/events/vcs/github/client_test.go | Added TestClient_GetChildTeams() to verify the method correctly parses GraphQL responses |
| server/events/command_runner.go | Added childTeamFetcher interface, fetchDescendantTeams() function, and updated checkUserPermissions() to support team hierarchy expansion with a two-path authorization check |
Comment on lines
+319
to
+347
| // Slow path: check if the user belongs to a child team of any allowlisted team. | ||
| // Only available when the VCS client supports fetching child teams (e.g. GitHub). | ||
| fetcher, ok := c.VCSClient.(childTeamFetcher) | ||
| if !ok { | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
|
|
||
| const maxHierarchyDepth = 20 | ||
| for _, allowedTeam := range c.TeamAllowlistChecker.AllTeams() { | ||
| if allowedTeam == "*" { | ||
| continue | ||
| } | ||
| // Only expand teams that actually grant permission for this command. | ||
| if !c.TeamAllowlistChecker.IsCommandAllowedForTeam(ctx, allowedTeam, cmdName) { | ||
| continue | ||
| } | ||
| descendants, err := fetchDescendantTeams(fetcher, c.Logger, repo, allowedTeam, maxHierarchyDepth) | ||
| if err != nil { | ||
| c.Logger.Warn("Could not fetch child teams for '%s': %s", allowedTeam, err) | ||
| continue | ||
| } | ||
| for _, userTeam := range user.Teams { | ||
| for _, desc := range descendants { | ||
| if strings.EqualFold(userTeam, desc) { | ||
| return true, nil | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new team hierarchy expansion logic in checkUserPermissions (lines 319-347) lacks test coverage. This includes the fetchDescendantTeams function and the logic that recursively expands allowlisted parent teams to check if a user belongs to any descendant team. While the GetChildTeams method has a test, the integration of this feature into the permission checking flow should be tested to ensure it works correctly end-to-end.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #6107
When a parent team is added to
ATLANTIS_GH_TEAM_ALLOWLIST, users who are members of any descendant (child/grandchild/etc.) team are now correctly authorized, instead of being rejected.Before: User in
child-team→ allowlist hasparent-team→ ❌ rejectedAfter: User in
child-team→ allowlist hasparent-team→ ✅ authorizedHow it works
GetChildTeams(logger, repo, teamSlug)to the GitHub client — queries the GitHub GraphQL API for direct child teams of a given team slug (with pagination).checkUserPermissions, after the fast path (direct team membership, zero extra API calls) fails, a slow path kicks in:GetChildTeams(up to 20 levels deep to prevent infinite loops).childTeamFetcherinterface, so no changes to the sharedClientinterface or any other VCS provider.Test plan
TestClient_GetTeamNamesForUser— existing test, still passes unchangedTestClient_GetChildTeams— new test verifyingGetChildTeamsreturns direct children from a mocked GraphQL responsego build ./server/events/...— clean buildATLANTIS_GH_TEAM_ALLOWLIST, comment as a user who is only in a child team, verify they are now authorized🤖 Generated with Claude Code