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

Resolve admins and collaborators on pull requests as approval actors #48

Merged
merged 4 commits into from
Jan 23, 2019
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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ requires:
users: ["user1", "user2"]
organizations: ["org1", "org2"]
teams: ["org1/team1", "org2/team2"]

# allows approval by admins of the org or repository
admins: true
# allows approval by users who have write on the repository
write_collaborators: true
```

### Approval Policies
Expand Down
29 changes: 29 additions & 0 deletions policy/common/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,17 @@ type Actors struct {
Users []string `yaml:"users"`
Teams []string `yaml:"teams"`
Organizations []string `yaml:"organizations"`

// Github repository specific interpolation options
Admins bool `yaml:"admins"`
WriteCollaborators bool `yaml:"write_collaborators"`
}

const (
GithubWritePermission = "write"
GithubAdminPermission = "admin"
)

// IsEmpty returns true if no conditions for actors are defined.
func (a *Actors) IsEmpty() bool {
return a == nil || (len(a.Users) == 0 && len(a.Teams) == 0 && len(a.Organizations) == 0)
Expand Down Expand Up @@ -65,5 +74,25 @@ func (a *Actors) IsActor(ctx context.Context, prctx pull.Context, user string) (
}
}

if a.Admins {
isAdmin, err := prctx.IsCollaborator(prctx.RepositoryOwner(), prctx.RepositoryName(), user, GithubAdminPermission)
if err != nil {
return false, errors.Wrap(err, "failed to get admin collaborator status")
}
if isAdmin {
return true, nil
}
}

if a.WriteCollaborators {
isWrite, err := prctx.IsCollaborator(prctx.RepositoryOwner(), prctx.RepositoryName(), user, GithubWritePermission)
if err != nil {
return false, errors.Wrap(err, "failed to get write collaborator status")
}
if isWrite {
return true, nil
}
}

return false, nil
}
17 changes: 17 additions & 0 deletions policy/common/actor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ func TestIsActor(t *testing.T) {
OrgMemberships: map[string][]string{
"mhaypenny": {"cool-org", "regular-org"},
},
CollaboratorMemberships: map[string][]string{
"mhaypenny": {GithubAdminPermission, GithubWritePermission},
},
}

assertActor := func(t *testing.T, a *Actors, user string) {
Expand Down Expand Up @@ -73,6 +76,20 @@ func TestIsActor(t *testing.T) {
assertActor(t, a, "mhaypenny")
assertNotActor(t, a, "ttest")
})

t.Run("admins", func(t *testing.T) {
a := &Actors{Admins: true}

assertActor(t, a, "mhaypenny")
assertNotActor(t, a, "ttest")
})

t.Run("write", func(t *testing.T) {
a := &Actors{WriteCollaborators: true}

assertActor(t, a, "mhaypenny")
assertNotActor(t, a, "ttest")
})
}

func TestIsEmpty(t *testing.T) {
Expand Down
9 changes: 9 additions & 0 deletions pull/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ type MembershipContext interface {

// IsOrgMember returns true if the user is a member of the given organzation.
IsOrgMember(org, user string) (bool, error)

// IsCollaborator returns true if the user meets the desiredPerm of the given organzation's repository.
IsCollaborator(org, repo, user, desiredPerm string) (bool, error)
}

// Context is the context for a pull request. It defines methods to get
Expand All @@ -42,6 +45,12 @@ type Context interface {
// string is formated as "<owner>/<repository>#<number>"
Locator() string

// RepositoryOwner returns the owner of the repo that the pull request targets.
RepositoryOwner() string

// RepositoryName returns the repo that the pull request targets.
RepositoryName() string

// Author returns the username of the user who opened the pull request.
Author() (string, error)

Expand Down
12 changes: 12 additions & 0 deletions pull/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,22 @@ func (ghc *GitHubContext) IsOrgMember(org, user string) (bool, error) {
return ghc.mbrCtx.IsOrgMember(org, user)
}

func (ghc *GitHubContext) IsCollaborator(org, repo, user, desiredPerm string) (bool, error) {
return ghc.mbrCtx.IsCollaborator(org, repo, user, desiredPerm)
}

func (ghc *GitHubContext) Locator() string {
return fmt.Sprintf("%s/%s#%d", ghc.owner, ghc.repo, ghc.number)
}

func (ghc *GitHubContext) RepositoryOwner() string {
return ghc.owner
}

func (ghc *GitHubContext) RepositoryName() string {
return ghc.repo
}

func (ghc *GitHubContext) Author() (string, error) {
return ghc.pr.GetUser().GetLogin(), nil
}
Expand Down
9 changes: 9 additions & 0 deletions pull/github_membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,12 @@ func (mc *GitHubMembershipContext) IsOrgMember(org, user string) (bool, error) {
mc.membership[key] = isMember
return isMember, nil
}

func (mc *GitHubMembershipContext) IsCollaborator(org, repo, user, desiredPerm string) (bool, error) {
perm, _, err := mc.client.Repositories.GetPermissionLevel(mc.ctx, org, repo, user)
if err != nil {
return false, errors.Wrapf(err, "failed to get repo %s permission", desiredPerm)
}

return perm.GetPermission() == desiredPerm, nil
}
32 changes: 32 additions & 0 deletions pull/pulltest/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (

type Context struct {
LocatorValue string
OwnerValue string
RepoValue string

AuthorValue string
AuthorError error
Expand All @@ -42,6 +44,9 @@ type Context struct {
OrgMemberships map[string][]string
OrgMembershipError error

CollaboratorMemberships map[string][]string
CollaboratorMembershipError error

BranchBaseName string
BranchHeadName string
BranchesError error
Expand All @@ -57,6 +62,20 @@ func (c *Context) Locator() string {
return "pulltest/context#1"
}

func (c *Context) RepositoryOwner() string {
if c.OwnerValue != "" {
return c.OwnerValue
}
return "pulltest"
}

func (c *Context) RepositoryName() string {
if c.RepoValue != "" {
return c.RepoValue
}
return "context"
}

func (c *Context) Author() (string, error) {
return c.AuthorValue, c.AuthorError
}
Expand Down Expand Up @@ -95,6 +114,19 @@ func (c *Context) IsOrgMember(org, user string) (bool, error) {
return false, nil
}

func (c *Context) IsCollaborator(org, repo, user, desiredPerm string) (bool, error) {
if c.CollaboratorMembershipError != nil {
return false, c.CollaboratorMembershipError
}

for _, c := range c.CollaboratorMemberships[user] {
if c == desiredPerm {
return true, nil
}
}
return false, nil
}

func (c *Context) Comments() ([]*pull.Comment, error) {
return c.CommentsValue, c.CommentsError
}
Expand Down
8 changes: 8 additions & 0 deletions server/handler/cross_org.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,11 @@ func (c *CrossOrgMembershipContext) IsOrgMember(org, user string) (bool, error)
}
return mbrCtx.IsOrgMember(org, user)
}

func (c *CrossOrgMembershipContext) IsCollaborator(org, repo, user, desiredPerm string) (bool, error) {
mbrCtx, err := c.getCtxForOrg(org)
if err != nil {
return false, err
}
return mbrCtx.IsCollaborator(org, repo, user, desiredPerm)
}