Skip to content

Commit

Permalink
Resolve admins and collaborators on pull requests as approval actors
Browse files Browse the repository at this point in the history
Fixes #15
  • Loading branch information
asvoboda committed Jan 21, 2019
1 parent 19530c2 commit aa4fb71
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 1 deletion.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,14 @@ requires:
users: ["user1", "user2"]
organizations: ["org1", "org2"]
teams: ["org1/team1", "org2/team2"]

# "github" resolves the membership with respect to github settings for the repo
# using the API for both organization and repository settings
github:
# allows approval by admins of the org or repository
admin: true
# allows approval by users who have write on the repository
write_collaborator: true
```

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

// Github specific interpolation options
Github *Github `yaml:"github"`
}

type Github struct {
Admin bool `yaml:"admin"`
WriteCollaborator bool `yaml:"write_collaborator"`
}

// 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)
return a == nil || (len(a.Users) == 0 && len(a.Teams) == 0 && len(a.Organizations) == 0 && a.Github == nil)
}

// IsActor returns true if the given user satisfies at least one of the
Expand Down Expand Up @@ -65,5 +73,28 @@ func (a *Actors) IsActor(ctx context.Context, prctx pull.Context, user string) (
}
}

if a.Github != nil {
if a.Github.Admin {
prctx.Locator()
isAdmin, err := prctx.IsCollaborator("admin", prctx.Owner(), prctx.Repo(), user)
if err != nil {
return false, errors.Wrap(err, "failed to get admin collaborator status")
}
if isAdmin {
return true, nil
}
}

if a.Github.WriteCollaborator {
isWrite, err := prctx.IsCollaborator("push", prctx.Owner(), prctx.Repo(), user)
if err != nil {
return false, errors.Wrap(err, "failed to get write collaborator status")
}
if isWrite {
return true, nil
}
}
}

return false, nil
}
28 changes: 28 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": {"admin", "push"},
},
}

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

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

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

t.Run("push", func(t *testing.T) {
a := &Actors{
Github: &Github{
WriteCollaborator: true,
},
}

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

func TestIsEmpty(t *testing.T) {
Expand All @@ -88,6 +113,9 @@ func TestIsEmpty(t *testing.T) {
a = &Actors{Organizations: []string{"org"}}
assert.False(t, a.IsEmpty(), "Actors struct was empty")

a = &Actors{Github: &Github{Admin: true}}
assert.False(t, a.IsEmpty(), "Actors struct was empty")

a = nil
assert.True(t, a.IsEmpty(), "nil struct was not empty")
}
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(desiredPerm, org, repo, user 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

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

// Repo returns the repo that the pull request targets.
Repo() 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(desiredPerm, org, repo, user string) (bool, error) {
return ghc.mbrCtx.IsCollaborator(desiredPerm, org, repo, user)
}

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

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

func (ghc *GitHubContext) Repo() 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(desiredPerm, org, repo, user string) (bool, error) {
perm, _, err := mc.client.Repositories.GetPermissionLevel(mc.ctx, org, repo, user)
if err != nil {
return false, errors.Wrap(err, "failed to get repo admin permission")
}

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) Owner() string {
if c.OwnerValue != "" {
return c.OwnerValue
}
return "pulltest"
}

func (c *Context) Repo() 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(desiredPerm, org, repo, user 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(desiredPerm, org, repo, user string) (bool, error) {
mbrCtx, err := c.getCtxForOrg(org)
if err != nil {
return false, err
}
return mbrCtx.IsCollaborator(desiredPerm, org, repo, user)
}

0 comments on commit aa4fb71

Please sign in to comment.