From 820e0ab158e8d4ee3e97845e39a92f98c180bdbc Mon Sep 17 00:00:00 2001 From: Jacob McCann Date: Fri, 11 Nov 2016 12:56:48 -0600 Subject: [PATCH] Check remote for org secrets access Fully implemented for github remote --- cache/helper.go | 24 +++++- remote/bitbucket/bitbucket.go | 5 ++ remote/bitbucketserver/bitbucketserver.go | 5 ++ remote/gerrit/gerrit.go | 5 ++ remote/github/convert.go | 12 +++ remote/github/fixtures/handler.go | 95 +++++++++++++++++++++++ remote/github/github.go | 10 +++ remote/github/github_test.go | 17 ++++ remote/gitlab/gitlab.go | 5 ++ remote/gogs/gogs.go | 5 ++ remote/mock/remote.go | 23 ++++++ remote/remote.go | 10 +++ router/middleware/session/team.go | 70 ++++++++++++++--- router/middleware/session/team_test.go | 95 +++++++++++++++++++++++ 14 files changed, 371 insertions(+), 10 deletions(-) create mode 100644 router/middleware/session/team_test.go diff --git a/cache/helper.go b/cache/helper.go index f4def328e..c39fbd60a 100644 --- a/cache/helper.go +++ b/cache/helper.go @@ -8,7 +8,7 @@ import ( "golang.org/x/net/context" ) -// GetPerm returns the user permissions repositories from the cache +// GetPerms returns the user permissions repositories from the cache // associated with the current repository. func GetPerms(c context.Context, user *model.User, owner, name string) (*model.Perm, error) { key := fmt.Sprintf("perms:%s:%s/%s", @@ -31,6 +31,28 @@ func GetPerms(c context.Context, user *model.User, owner, name string) (*model.P return perm, nil } +// GetTeamPerms returns the user permissions from the cache +// associated with the current organization. +func GetTeamPerms(c context.Context, user *model.User, org string) (*model.Perm, error) { + key := fmt.Sprintf("perms:%s:%s", + user.Login, + org, + ) + // if we fetch from the cache we can return immediately + val, err := Get(c, key) + if err == nil { + return val.(*model.Perm), nil + } + // else we try to grab from the remote system and + // populate our cache. + perm, err := remote.TeamPerm(c, user, org) + if err != nil { + return nil, err + } + Set(c, key, perm) + return perm, nil +} + // GetRepos returns the list of user repositories from the cache // associated with the current context. func GetRepos(c context.Context, user *model.User) ([]*model.RepoLite, error) { diff --git a/remote/bitbucket/bitbucket.go b/remote/bitbucket/bitbucket.go index b3e51a97f..cff6bdfb7 100644 --- a/remote/bitbucket/bitbucket.go +++ b/remote/bitbucket/bitbucket.go @@ -104,6 +104,11 @@ func (c *config) Teams(u *model.User) ([]*model.Team, error) { return convertTeamList(resp.Values), nil } +// TeamPerm is not supported by the Bitbucket driver. +func (c *config) TeamPerm(u *model.User, org string) (*model.Perm, error) { + return nil, nil +} + // Repo returns the named Bitbucket repository. func (c *config) Repo(u *model.User, owner, name string) (*model.Repo, error) { repo, err := c.newClient(u).FindRepo(owner, name) diff --git a/remote/bitbucketserver/bitbucketserver.go b/remote/bitbucketserver/bitbucketserver.go index cb4e415f1..1da0c015f 100644 --- a/remote/bitbucketserver/bitbucketserver.go +++ b/remote/bitbucketserver/bitbucketserver.go @@ -115,6 +115,11 @@ func (*Config) Teams(u *model.User) ([]*model.Team, error) { return teams, nil } +// TeamPerm is not supported by the Stash driver. +func (*Config) TeamPerm(u *model.User, org string) (*model.Perm, error) { + return nil, nil +} + func (c *Config) Repo(u *model.User, owner, name string) (*model.Repo, error) { repo, err := internal.NewClientWithToken(c.URL, c.Consumer, u.Token).FindRepo(owner, name) if err != nil { diff --git a/remote/gerrit/gerrit.go b/remote/gerrit/gerrit.go index 33963ff0d..6b8be5162 100644 --- a/remote/gerrit/gerrit.go +++ b/remote/gerrit/gerrit.go @@ -69,6 +69,11 @@ func (c *client) Teams(u *model.User) ([]*model.Team, error) { return empty, nil } +// TeamPerm is not supported by the Gerrit driver. +func (c *client) TeamPerm(u *model.User, org string) (*model.Perm, error) { + return nil, nil +} + // Repo is not supported by the Gerrit driver. func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { return nil, nil diff --git a/remote/github/convert.go b/remote/github/convert.go index 0597b33d0..dc6ebb9ad 100644 --- a/remote/github/convert.go +++ b/remote/github/convert.go @@ -93,6 +93,18 @@ func convertPerm(from *github.Repository) *model.Perm { } } +// convertTeamPerm is a helper function used to convert a GitHub organization +// permissions to the common Drone permissions structure. +func convertTeamPerm(from *github.Membership) *model.Perm { + admin := false + if *from.Role == "admin" { + admin = true + } + return &model.Perm{ + Admin: admin, + } +} + // convertRepoList is a helper function used to convert a GitHub repository // list to the common Drone repository structure. func convertRepoList(from []github.Repository) []*model.RepoLite { diff --git a/remote/github/fixtures/handler.go b/remote/github/fixtures/handler.go index 8b0b270bf..ce4aa3d40 100644 --- a/remote/github/fixtures/handler.go +++ b/remote/github/fixtures/handler.go @@ -13,6 +13,8 @@ func Handler() http.Handler { e := gin.New() e.GET("/api/v3/repos/:owner/:name", getRepo) + e.GET("/api/v3/orgs/:org/memberships/:user", getMembership) + e.GET("/api/v3/user/memberships/orgs/:org", getMembership) return e } @@ -26,6 +28,17 @@ func getRepo(c *gin.Context) { } } +func getMembership(c *gin.Context) { + switch c.Param("org") { + case "org_not_found": + c.String(404, "") + case "github": + c.String(200, membershipIsMemberPayload) + default: + c.String(200, membershipIsOwnerPayload) + } +} + var repoPayload = ` { "owner": { @@ -45,3 +58,85 @@ var repoPayload = ` } } ` + +var membershipIsOwnerPayload = ` +{ + "url": "https://api.github.com/orgs/octocat/memberships/octocat", + "state": "active", + "role": "admin", + "organization_url": "https://api.github.com/orgs/octocat", + "user": { + "login": "octocat", + "id": 5555555, + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "organization": { + "login": "octocat", + "id": 5555556, + "url": "https://api.github.com/orgs/octocat", + "repos_url": "https://api.github.com/orgs/octocat/repos", + "events_url": "https://api.github.com/orgs/octocat/events", + "hooks_url": "https://api.github.com/orgs/octocat/hooks", + "issues_url": "https://api.github.com/orgs/octocat/issues", + "members_url": "https://api.github.com/orgs/octocat/members{/member}", + "public_members_url": "https://api.github.com/orgs/octocat/public_members{/member}", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "description": "" + } +} +` + +var membershipIsMemberPayload = ` +{ + "url": "https://api.github.com/orgs/github/memberships/octocat", + "state": "active", + "role": "member", + "organization_url": "https://api.github.com/orgs/github", + "user": { + "login": "octocat", + "id": 5555555, + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "organization": { + "login": "octocat", + "id": 5555557, + "url": "https://api.github.com/orgs/github", + "repos_url": "https://api.github.com/orgs/github/repos", + "events_url": "https://api.github.com/orgs/github/events", + "hooks_url": "https://api.github.com/orgs/github/hooks", + "issues_url": "https://api.github.com/orgs/github/issues", + "members_url": "https://api.github.com/orgs/github/members{/member}", + "public_members_url": "https://api.github.com/orgs/github/public_members{/member}", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "description": "" + } +} +` diff --git a/remote/github/github.go b/remote/github/github.go index abd05dc19..ba69cac56 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -158,6 +158,16 @@ func (c *client) Teams(u *model.User) ([]*model.Team, error) { return teams, nil } +// TeamPerm returns the user permissions for the named GitHub organization. +func (c *client) TeamPerm(u *model.User, org string) (*model.Perm, error) { + client := c.newClientToken(u.Token) + membership, _, err := client.Organizations.GetOrgMembership(u.Login, org) + if err != nil { + return nil, err + } + return convertTeamPerm(membership), nil +} + // Repo returns the named GitHub repository. func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { client := c.newClientToken(u.Token) diff --git a/remote/github/github_test.go b/remote/github/github_test.go index 28c030cf5..83e41f60d 100644 --- a/remote/github/github_test.go +++ b/remote/github/github_test.go @@ -110,6 +110,23 @@ func Test_github(t *testing.T) { }) }) + g.Describe("Requesting organization permissions", func() { + g.It("Should return the permission details of an admin", func() { + perm, err := c.TeamPerm(fakeUser, "octocat") + g.Assert(err == nil).IsTrue() + g.Assert(perm.Admin).IsTrue() + }) + g.It("Should return the permission details of a member", func() { + perm, err := c.TeamPerm(fakeUser, "github") + g.Assert(err == nil).IsTrue() + g.Assert(perm.Admin).IsFalse() + }) + g.It("Should handle a not found error", func() { + _, err := c.TeamPerm(fakeUser, "org_not_found") + g.Assert(err != nil).IsTrue() + }) + }) + g.It("Should return a user repository list") g.It("Should return a user team list") diff --git a/remote/gitlab/gitlab.go b/remote/gitlab/gitlab.go index 488dbd9d7..543b32bb8 100644 --- a/remote/gitlab/gitlab.go +++ b/remote/gitlab/gitlab.go @@ -194,6 +194,11 @@ func (g *Gitlab) Teams(u *model.User) ([]*model.Team, error) { return teams, nil } +// TeamPerm is not supported by the Gitlab driver. +func (g *Gitlab) TeamPerm(u *model.User, org string) (*model.Perm, error) { + return nil, nil +} + // Repo fetches the named repository from the remote system. func (g *Gitlab) Repo(u *model.User, owner, name string) (*model.Repo, error) { client := NewClient(g.URL, u.Token, g.SkipVerify) diff --git a/remote/gogs/gogs.go b/remote/gogs/gogs.go index 4ff607068..361ee9919 100644 --- a/remote/gogs/gogs.go +++ b/remote/gogs/gogs.go @@ -126,6 +126,11 @@ func (c *client) Teams(u *model.User) ([]*model.Team, error) { return teams, nil } +// TeamPerm is not supported by the Gogs driver. +func (c *client) TeamPerm(u *model.User, org string) (*model.Perm, error) { + return nil, nil +} + // Repo returns the named Gogs repository. func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { client := c.newClientToken(u.Token) diff --git a/remote/mock/remote.go b/remote/mock/remote.go index 917a85c50..d44194710 100644 --- a/remote/mock/remote.go +++ b/remote/mock/remote.go @@ -267,3 +267,26 @@ func (_m *Remote) Teams(u *model.User) ([]*model.Team, error) { return r0, r1 } + +// TeamPerm provides a mock function with given fields: u, org +func (_m *Remote) TeamPerm(u *model.User, org string) (*model.Perm, error) { + ret := _m.Called(u, org) + + var r0 *model.Perm + if rf, ok := ret.Get(0).(func(*model.User, string) *model.Perm); ok { + r0 = rf(u, org) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Perm) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.User, string) error); ok { + r1 = rf(u, org) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/remote/remote.go b/remote/remote.go index cfcdafdf0..d1ba29785 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -22,6 +22,10 @@ type Remote interface { // Teams fetches a list of team memberships from the remote system. Teams(u *model.User) ([]*model.Team, error) + // TeamPerm fetches the named organization permissions from + // the remote system for the specified user. + TeamPerm(u *model.User, org string) (*model.Perm, error) + // Repo fetches the named repository from the remote system. Repo(u *model.User, owner, repo string) (*model.Repo, error) @@ -80,6 +84,12 @@ func Teams(c context.Context, u *model.User) ([]*model.Team, error) { return FromContext(c).Teams(u) } +// TeamPerm fetches the named organization permissions from +// the remote system for the specified user. +func TeamPerm(c context.Context, u *model.User, org string) (*model.Perm, error) { + return FromContext(c).TeamPerm(u, org) +} + // Repo fetches the named repository from the remote system. func Repo(c context.Context, u *model.User, owner, repo string) (*model.Repo, error) { return FromContext(c).Repo(u, owner, repo) diff --git a/router/middleware/session/team.go b/router/middleware/session/team.go index c9fc7ce5f..c761d39b0 100644 --- a/router/middleware/session/team.go +++ b/router/middleware/session/team.go @@ -1,21 +1,73 @@ package session import ( + "github.com/drone/drone/cache" + "github.com/drone/drone/model" + + log "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" ) +func TeamPerm(c *gin.Context) *model.Perm { + user := User(c) + team := c.Param("team") + perm := &model.Perm{} + + switch { + // if the user is not authenticated + case user == nil: + perm.Admin = false + perm.Pull = false + perm.Push = false + + // if the user is a DRONE_ADMIN + case user.Admin: + perm.Admin = true + perm.Pull = true + perm.Push = true + + // otherwise if the user is authenticated we should + // check the remote system to get the users permissiosn. + default: + log.Debugf("Fetching team permission for %s %s", + user.Login, team) + + var err error + perm, err = cache.GetTeamPerms(c, user, team) + if err != nil { + // debug + log.Errorf("Error fetching team permission for %s %s", + user.Login, team) + + perm.Admin = false + perm.Pull = false + perm.Push = false + } + } + + if user != nil { + log.Debugf("%s granted %+v team permission to %s", + user.Login, perm, team) + } else { + log.Debugf("Guest granted %+v to %s", perm, team) + + perm.Admin = false + perm.Pull = false + perm.Push = false + } + + return perm +} + func MustTeamAdmin() gin.HandlerFunc { return func(c *gin.Context) { - user := User(c) - switch { - case user == nil: - c.String(401, "User not authorized") - c.Abort() - case user.Admin == false: - c.String(413, "User not authorized") - c.Abort() - default: + perm := TeamPerm(c) + + if perm.Admin { c.Next() } + + c.String(401, "User not authorized") + c.Abort() } } diff --git a/router/middleware/session/team_test.go b/router/middleware/session/team_test.go new file mode 100644 index 000000000..a212a520a --- /dev/null +++ b/router/middleware/session/team_test.go @@ -0,0 +1,95 @@ +package session + +import ( + "testing" + + "github.com/drone/drone/cache" + "github.com/drone/drone/model" + "github.com/franela/goblin" + "github.com/gin-gonic/gin" +) + +func TestTeamPerm(t *testing.T) { + g := goblin.Goblin(t) + + g.Describe("TeamPerm", func() { + + var c *gin.Context + g.BeforeEach(func() { + c = new(gin.Context) + cache.ToContext(c, cache.Default()) + }) + + g.It("Should set admin to false (user not logged in)", func() { + p := TeamPerm(c) + g.Assert(p.Admin).IsFalse("admin should be false") + }) + g.It("Should set admin to true (user is DRONE_ADMIN)", func() { + // Set DRONE_ADMIN user + c.Set("user", fakeUserAdmin) + + p := TeamPerm(c) + g.Assert(p.Admin).IsTrue("admin should be false") + }) + g.It("Should set admin to false (user logged in, not owner of org)", func() { + // Set fake org + params := gin.Params{ + gin.Param{ + Key: "team", + Value: "test_org", + }, + } + c.Params = params + + // Set cache to show user does not Owner/Admin + cache.Set(c, "perms:octocat:test_org", fakeTeamPerm) + + // Set User + c.Set("user", fakeUser) + + p := TeamPerm(c) + g.Assert(p.Admin).IsFalse("admin should be false") + }) + g.It("Should set admin to true (user logged in, owner of org)", func() { + // Set fake org + params := gin.Params{ + gin.Param{ + Key: "team", + Value: "test_org", + }, + } + c.Params = params + + // Set cache to show user is Owner/Admin + cache.Set(c, "perms:octocat:test_org", fakeTeamPermAdmin) + + // Set User + c.Set("user", fakeUser) + + p := TeamPerm(c) + g.Assert(p.Admin).IsTrue("admin should be true") + }) + }) +} + +var ( + fakeUserAdmin = &model.User{ + Login: "octocatAdmin", + Token: "cfcd2084", + Admin: true, + } + + fakeUser = &model.User{ + Login: "octocat", + Token: "cfcd2084", + Admin: false, + } + + fakeTeamPermAdmin = &model.Perm{ + Admin: true, + } + + fakeTeamPerm = &model.Perm{ + Admin: false, + } +)