From 8fa473b07a70a9f41074bb6fc29c7bb0874f0487 Mon Sep 17 00:00:00 2001 From: Dan Carley Date: Thu, 15 Jan 2015 14:39:44 +0000 Subject: [PATCH] Support org whitelists for GitHub+GHE remotes Allow the GitHub and GitHub Enterprise remotes to restrict who can login based on a user's organisation membership. This can be used as a safe addition to open registration and also ensures that access is revoked when a user is subsequently removed from the org. The default is not to restrict at all. --- README.md | 2 ++ packaging/root/etc/drone/drone.toml | 2 ++ plugin/remote/github/github.go | 18 +++++++++++++++--- plugin/remote/github/github_test.go | 28 ++++++++++++++++++++++++---- plugin/remote/github/helper.go | 22 ++++++++++++++++++++++ plugin/remote/github/helper_test.go | 15 +++++++++++++++ plugin/remote/github/register.go | 5 ++++- 7 files changed, 84 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index fef19f00..3cd213cb 100644 --- a/README.md +++ b/README.md @@ -100,12 +100,14 @@ open=true [github] client="" secret="" +orgs=[] [github_enterprise] client="" secret="" api="" url="" +orgs=[] private_mode=false [bitbucket] diff --git a/packaging/root/etc/drone/drone.toml b/packaging/root/etc/drone/drone.toml index 29dfbe42..3f74c6bc 100644 --- a/packaging/root/etc/drone/drone.toml +++ b/packaging/root/etc/drone/drone.toml @@ -37,12 +37,14 @@ datasource="/var/lib/drone/drone.sqlite" # [github] # client="" # secret="" +# orgs=[] # [github_enterprise] # client="" # secret="" # api="" # url="" +# orgs=[] # private_mode=false # [bitbucket] diff --git a/plugin/remote/github/github.go b/plugin/remote/github/github.go index 31ff84e5..840d493f 100644 --- a/plugin/remote/github/github.go +++ b/plugin/remote/github/github.go @@ -27,9 +27,10 @@ type GitHub struct { Secret string Private bool SkipVerify bool + Orgs []string } -func New(url, api, client, secret string, private, skipVerify bool) *GitHub { +func New(url, api, client, secret string, private, skipVerify bool, orgs []string) *GitHub { var github = GitHub{ URL: url, API: api, @@ -37,6 +38,7 @@ func New(url, api, client, secret string, private, skipVerify bool) *GitHub { Secret: secret, Private: private, SkipVerify: skipVerify, + Orgs: orgs, } // the API must have a trailing slash if !strings.HasSuffix(github.API, "/") { @@ -49,8 +51,8 @@ func New(url, api, client, secret string, private, skipVerify bool) *GitHub { return &github } -func NewDefault(client, secret string) *GitHub { - return New(DefaultURL, DefaultAPI, client, secret, false, false) +func NewDefault(client, secret string, orgs []string) *GitHub { + return New(DefaultURL, DefaultAPI, client, secret, false, false, orgs) } // Authorize handles GitHub API Authorization. @@ -92,6 +94,16 @@ func (r *GitHub) Authorize(res http.ResponseWriter, req *http.Request) (*model.L return nil, fmt.Errorf("Error retrieving user or verified email. %s", errr) } + if len(r.Orgs) > 0 { + allowedOrg, err := UserBelongsToOrg(client, r.Orgs) + if err != nil { + return nil, fmt.Errorf("Could not check org membership. %s", err) + } + if !allowedOrg { + return nil, fmt.Errorf("User does not belong to correct org") + } + } + var login = new(model.Login) login.ID = int64(*useremail.ID) login.Access = token.AccessToken diff --git a/plugin/remote/github/github_test.go b/plugin/remote/github/github_test.go index 209b0bb5..c46aede9 100644 --- a/plugin/remote/github/github_test.go +++ b/plugin/remote/github/github_test.go @@ -94,7 +94,11 @@ func Test_Github(t *testing.T) { g.It("Should parse a pull request hook") - g.It("Should authorize a valid user", func() { + g.Describe("Authorize", func() { + g.AfterEach(func() { + github.Orgs = []string{} + }) + var resp = httptest.NewRecorder() var state = "validstate" var req, _ = http.NewRequest( @@ -104,9 +108,25 @@ func Test_Github(t *testing.T) { ) req.AddCookie(&http.Cookie{Name: "github_state", Value: state}) - var login, err = github.Authorize(resp, req) - g.Assert(err == nil).IsTrue() - g.Assert(login == nil).IsFalse() + g.It("Should authorize a valid user with no org restrictions", func() { + var login, err = github.Authorize(resp, req) + g.Assert(err == nil).IsTrue() + g.Assert(login == nil).IsFalse() + }) + + g.It("Should authorize a valid user in the correct org", func() { + github.Orgs = []string{"octocats-inc"} + var login, err = github.Authorize(resp, req) + g.Assert(err == nil).IsTrue() + g.Assert(login == nil).IsFalse() + }) + + g.It("Should not authorize a valid user in the wrong org", func() { + github.Orgs = []string{"acme"} + var login, err = github.Authorize(resp, req) + g.Assert(err != nil).IsTrue() + g.Assert(login == nil).IsTrue() + }) }) }) } diff --git a/plugin/remote/github/helper.go b/plugin/remote/github/helper.go index 5bafccd8..16dd1f02 100644 --- a/plugin/remote/github/helper.go +++ b/plugin/remote/github/helper.go @@ -270,3 +270,25 @@ func GetPayload(req *http.Request) []byte { } return []byte(payload) } + +// UserBelongsToOrg returns true if the currently authenticated user is a +// member of any of the organizations provided. +func UserBelongsToOrg(client *github.Client, permittedOrgs []string) (bool, error) { + userOrgs, err := GetOrgs(client) + if err != nil { + return false, err + } + + userOrgSet := make(map[string]struct{}, len(userOrgs)) + for _, org := range userOrgs { + userOrgSet[*org.Login] = struct{}{} + } + + for _, org := range permittedOrgs { + if _, ok := userOrgSet[org]; ok { + return true, nil + } + } + + return false, nil +} diff --git a/plugin/remote/github/helper_test.go b/plugin/remote/github/helper_test.go index d5e6add3..3155e61e 100644 --- a/plugin/remote/github/helper_test.go +++ b/plugin/remote/github/helper_test.go @@ -39,5 +39,20 @@ func Test_Helper(t *testing.T) { g.It("Should Create or Update a Repo Hook") g.It("Should Get a Repo File") + g.Describe("UserBelongsToOrg", func() { + g.It("Should confirm user does belong to 'octocats-inc' org", func() { + var requiredOrgs = []string{"one", "octocats-inc", "two"} + var member, err = UserBelongsToOrg(client, requiredOrgs) + g.Assert(err == nil).IsTrue() + g.Assert(member).IsTrue() + }) + + g.It("Should confirm user not does belong to 'octocats-inc' org", func() { + var requiredOrgs = []string{"one", "two"} + var member, err = UserBelongsToOrg(client, requiredOrgs) + g.Assert(err == nil).IsTrue() + g.Assert(member).IsFalse() + }) + }) }) } diff --git a/plugin/remote/github/register.go b/plugin/remote/github/register.go index eb71403f..52c72838 100644 --- a/plugin/remote/github/register.go +++ b/plugin/remote/github/register.go @@ -9,6 +9,7 @@ var ( // GitHub cloud configuration details githubClient = config.String("github-client", "") githubSecret = config.String("github-secret", "") + githubOrgs = config.Strings("github-orgs") // GitHub Enterprise configuration details githubEnterpriseURL = config.String("github-enterprise-url", "") @@ -17,6 +18,7 @@ var ( githubEnterpriseSecret = config.String("github-enterprise-secret", "") githubEnterprisePrivate = config.Bool("github-enterprise-private-mode", true) githubEnterpriseSkipVerify = config.Bool("github-enterprise-skip-verify", false) + githubEnterpriseOrgs = config.Strings("github-enterprise-orgs") ) // Registers the GitHub plugins using the default @@ -33,7 +35,7 @@ func registerGitHub() { return } remote.Register( - NewDefault(*githubClient, *githubSecret), + NewDefault(*githubClient, *githubSecret, *githubOrgs), ) } @@ -53,6 +55,7 @@ func registerGitHubEnterprise() { *githubEnterpriseSecret, *githubEnterprisePrivate, *githubEnterpriseSkipVerify, + *githubEnterpriseOrgs, ), ) }