Skip to content

Commit

Permalink
Add permissions check to valid_owner
Browse files Browse the repository at this point in the history
Previously, the valid_owner check used the Repositories List Teams API
endpoint to check if a team was valid. Due to an issue in the
GitHub API, that endpoint does not return child teams that would
inherit a parent team's permissions.

This commit changes the initial check for the validity of a team to use
the Teams List Teams API endpoint, which lists all teams in a org to
check if the team exists at all. If that check fails, it will clearly
state that the team does not exist.

As a follow up to the initial valid team check, this commit checks the
Teams IsTeamRepoBySlug endpoint, which returns an object containing the
actual permissions for the team being checked on the repo. By adding
this check, we can verify that a user can actually provide their review
on PRs that made the CODEOWNERS line.
  • Loading branch information
highb authored and mszostok committed Jan 21, 2021
1 parent cde24ed commit 9224144
Showing 1 changed file with 47 additions and 2 deletions.
49 changes: 47 additions & 2 deletions internal/check/valid_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (v *ValidOwner) initOrgListTeams(ctx context.Context) *validateError {
PerPage: 100,
}
for {
resultPage, resp, err := v.ghClient.Repositories.ListTeams(ctx, v.orgName, v.orgRepoName, req)
resultPage, resp, err := v.ghClient.Teams.ListTeams(ctx, v.orgName, req)
if err != nil { // TODO(mszostok): implement retry?
switch err := err.(type) {
case *github.ErrorResponse:
Expand Down Expand Up @@ -192,7 +192,52 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr
}

if !teamExists() {
return newValidateError("Team %q does not exist in organization %q or has no permissions associated with the repository.", team, org)
return newValidateError("Team %q does not exist in organization %q.", team, org)
}

// repo contains the permissions for the team slug given
repo, _, err := v.ghClient.Teams.IsTeamRepoBySlug(ctx, v.orgName, team, org, v.orgRepoName)
if err != nil { // TODO(mszostok): implement retry?
switch err := err.(type) {
case *github.ErrorResponse:
if err.Response.StatusCode == http.StatusUnauthorized {
return newValidateError("Team permissions information for %q/%q could not be queried. Requires GitHub authorization.", org, v.orgRepoName)
} else if err.Response.StatusCode == http.StatusNotFound {
return newValidateError("Team %q does not have permissions associated with the repository %q.", team, v.orgRepoName)
} else {
return newValidateError("HTTP error occurred while calling GitHub: %v", err)
}
case *github.RateLimitError:
return newValidateError("GitHub rate limit reached: %v", err.Message)
default:
return newValidateError("Unknown error occurred while calling GitHub: %v", err)
}
}

teamHasWritePermission := func() bool {
for k, v := range *repo.Permissions {
if v == false {
continue
}

switch k {
case
"admin",
"maintain",
"push",
"triage":
return true
case
"pull":
continue
}
}

return false
}

if !teamHasWritePermission() {
return newValidateError("Team %q cannot review PRs on %q.", team, v.orgRepoName)
}

return nil
Expand Down

0 comments on commit 9224144

Please sign in to comment.