From 764c36f736ebafd236ece1a423e5b60a3db3cc48 Mon Sep 17 00:00:00 2001 From: folex <0xdxdy@gmail.com> Date: Fri, 27 Apr 2018 18:59:36 +0300 Subject: [PATCH 1/4] Use user/permissions bitbucket API instead of hooks --- .gitignore | 1 + remote/bitbucket/bitbucket.go | 14 +++++++---- remote/bitbucket/internal/client.go | 37 ++++++++++++++++++++++------- remote/bitbucket/internal/types.go | 10 ++++++++ 4 files changed, 49 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index c07e1ad4..ff661245 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ extras/ release/ server/swagger/files/*.json +.idea/ diff --git a/remote/bitbucket/bitbucket.go b/remote/bitbucket/bitbucket.go index c76d3821..1dcaa068 100644 --- a/remote/bitbucket/bitbucket.go +++ b/remote/bitbucket/bitbucket.go @@ -176,15 +176,21 @@ func (c *config) Perm(u *model.User, owner, name string) (*model.Perm, error) { client := c.newClient(u) perms := new(model.Perm) - _, err := client.FindRepo(owner, name) + repo, err := client.FindRepo(owner, name) if err != nil { return perms, err } - _, err = client.ListHooks(owner, name, &internal.ListOpts{}) + perm, err := client.GetPermission(repo.FullName) + if err == nil { - perms.Push = true - perms.Admin = true + switch perm.Permission { + case "admin": + perms.Push = true + perms.Admin = true + case "write": + perms.Push = true + } } perms.Pull = true return perms, nil diff --git a/remote/bitbucket/internal/client.go b/remote/bitbucket/internal/client.go index 9787255a..1b98dec9 100644 --- a/remote/bitbucket/internal/client.go +++ b/remote/bitbucket/internal/client.go @@ -22,6 +22,7 @@ import ( "net/http" "net/url" + "github.com/pkg/errors" "golang.org/x/oauth2" "golang.org/x/oauth2/bitbucket" ) @@ -34,15 +35,16 @@ const ( ) const ( - pathUser = "%s/2.0/user/" - pathEmails = "%s/2.0/user/emails" - pathTeams = "%s/2.0/teams/?%s" - pathRepo = "%s/2.0/repositories/%s/%s" - pathRepos = "%s/2.0/repositories/%s?%s" - pathHook = "%s/2.0/repositories/%s/%s/hooks/%s" - pathHooks = "%s/2.0/repositories/%s/%s/hooks?%s" - pathSource = "%s/1.0/repositories/%s/%s/src/%s/%s" - pathStatus = "%s/2.0/repositories/%s/%s/commit/%s/statuses/build" + pathUser = "%s/2.0/user/" + pathEmails = "%s/2.0/user/emails" + pathPermissions = "%s/2.0/user/permissions/repositories?q=repository.full_name=\"%s\"" + pathTeams = "%s/2.0/teams/?%s" + pathRepo = "%s/2.0/repositories/%s/%s" + pathRepos = "%s/2.0/repositories/%s?%s" + pathHook = "%s/2.0/repositories/%s/%s/hooks/%s" + pathHooks = "%s/2.0/repositories/%s/%s/hooks?%s" + pathSource = "%s/1.0/repositories/%s/%s/src/%s/%s" + pathStatus = "%s/2.0/repositories/%s/%s/commit/%s/statuses/build" ) type Client struct { @@ -152,6 +154,23 @@ func (c *Client) CreateStatus(owner, name, revision string, status *BuildStatus) return c.do(uri, post, status, nil) } +func (c *Client) GetPermission(full_name string) (*RepoPerm, error) { + out := new(RepoPermResp) + uri := fmt.Sprintf(pathPermissions, c.base, full_name) + err := c.do(uri, get, nil, out) + + if err != nil { + return nil, err + } + + if len(out.Values) == 0 { + err = errors.New(fmt.Sprint("no permissions in repository ", full_name)) + return nil, err + } else { + return out.Values[0], nil + } +} + func (c *Client) do(rawurl, method string, in, out interface{}) error { uri, err := url.Parse(rawurl) diff --git a/remote/bitbucket/internal/types.go b/remote/bitbucket/internal/types.go index 18c5e0ca..652b037c 100644 --- a/remote/bitbucket/internal/types.go +++ b/remote/bitbucket/internal/types.go @@ -224,3 +224,13 @@ type Error struct { func (e Error) Error() string { return e.Body.Message } + +type RepoPermResp struct { + Page int `json:"page"` + Pages int `json:"pagelen"` + Values []*RepoPerm `json:"values"` +} + +type RepoPerm struct { + Permission string `json:"permission"` +} From 0b73e5489b6312534eb4e0830bc116fc1d085ea8 Mon Sep 17 00:00:00 2001 From: folex <0xdxdy@gmail.com> Date: Fri, 27 Apr 2018 20:48:54 +0300 Subject: [PATCH 2/4] Add tests: - read access - write access - admin access --- remote/bitbucket/bitbucket.go | 24 +++++++++------- remote/bitbucket/bitbucket_test.go | 37 +++++++++++++++++++++--- remote/bitbucket/fixtures/handler.go | 42 ++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 14 deletions(-) diff --git a/remote/bitbucket/bitbucket.go b/remote/bitbucket/bitbucket.go index 1dcaa068..35ec84f2 100644 --- a/remote/bitbucket/bitbucket.go +++ b/remote/bitbucket/bitbucket.go @@ -182,17 +182,21 @@ func (c *config) Perm(u *model.User, owner, name string) (*model.Perm, error) { } perm, err := client.GetPermission(repo.FullName) - - if err == nil { - switch perm.Permission { - case "admin": - perms.Push = true - perms.Admin = true - case "write": - perms.Push = true - } + if err != nil { + return perms, err } - perms.Pull = true + + switch perm.Permission { + case "admin": + perms.Admin = true + fallthrough + case "write": + perms.Push = true + fallthrough + default: + perms.Pull = true + } + return perms, nil } diff --git a/remote/bitbucket/bitbucket_test.go b/remote/bitbucket/bitbucket_test.go index c22d0e79..91172321 100644 --- a/remote/bitbucket/bitbucket_test.go +++ b/remote/bitbucket/bitbucket_test.go @@ -161,19 +161,30 @@ func Test_bitbucket(t *testing.T) { g.It("Should authorize read access", func() { perm, err := c.Perm( fakeUser, - fakeRepoNoHooks.Owner, - fakeRepoNoHooks.Name, + fakeRepoReadOnly.Owner, + fakeRepoReadOnly.Name, ) g.Assert(err == nil).IsTrue() g.Assert(perm.Pull).IsTrue() g.Assert(perm.Push).IsFalse() g.Assert(perm.Admin).IsFalse() }) + g.It("Should authorize write access", func() { + perm, err := c.Perm( + fakeUser, + fakeRepoWriteOnly.Owner, + fakeRepoWriteOnly.Name, + ) + g.Assert(err == nil).IsTrue() + g.Assert(perm.Pull).IsTrue() + g.Assert(perm.Push).IsTrue() + g.Assert(perm.Admin).IsFalse() + }) g.It("Should authorize admin access", func() { perm, err := c.Perm( fakeUser, - fakeRepo.Owner, - fakeRepo.Name, + fakeRepoAdmin.Owner, + fakeRepoAdmin.Name, ) g.Assert(err == nil).IsTrue() g.Assert(perm.Pull).IsTrue() @@ -350,6 +361,24 @@ var ( FullName: "test_name/hook_empty", } + fakeRepoReadOnly = &model.Repo{ + Owner: "test_name", + Name: "permission_read", + FullName: "test_name/permission_read", + } + + fakeRepoWriteOnly = &model.Repo{ + Owner: "test_name", + Name: "permission_write", + FullName: "test_name/permission_write", + } + + fakeRepoAdmin = &model.Repo{ + Owner: "test_name", + Name: "permission_admin", + FullName: "test_name/permission_admin", + } + fakeBuild = &model.Build{ Commit: "9ecad50", } diff --git a/remote/bitbucket/fixtures/handler.go b/remote/bitbucket/fixtures/handler.go index ec855a60..f0565995 100644 --- a/remote/bitbucket/fixtures/handler.go +++ b/remote/bitbucket/fixtures/handler.go @@ -15,6 +15,7 @@ package fixtures import ( + "fmt" "net/http" "github.com/gin-gonic/gin" @@ -36,6 +37,7 @@ func Handler() http.Handler { e.GET("/2.0/repositories/:owner", getUserRepos) e.GET("/2.0/teams/", getUserTeams) e.GET("/2.0/user/", getUser) + e.GET("/2.0/user/permissions/repositories", getPermissions) return e } @@ -70,6 +72,8 @@ func getRepo(c *gin.Context) { switch c.Param("name") { case "not_found", "repo_unknown", "repo_not_found": c.String(404, "") + case "permission_read", "permission_write", "permission_admin": + c.String(200, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) default: c.String(200, repoPayload) } @@ -144,6 +148,24 @@ func getUserRepos(c *gin.Context) { } } +func permission(p string) string { + return fmt.Sprintf(permissionPayload, p) +} + +func getPermissions(c *gin.Context) { + query := c.Request.URL.Query()["q"][0] + switch query { + case `repository.full_name="test_name/permission_read"`: + c.String(200, permission("read")) + case `repository.full_name="test_name/permission_write"`: + c.String(200, permission("write")) + case `repository.full_name="test_name/permission_admin"`: + c.String(200, permission("admin")) + default: + c.String(200, permission("read")) + } +} + const tokenPayload = ` { "access_token":"2YotnFZFEjr1zCsicMWpAA", @@ -170,6 +192,14 @@ const repoPayload = ` } ` +const permissionRepoPayload = ` +{ + "full_name": "test_name/%s", + "scm": "git", + "is_private": true +} +` + const repoHookPayload = ` { "pagelen": 10, @@ -238,3 +268,15 @@ const userTeamPayload = ` ] } ` + +const permissionPayload = ` +{ + "pagelen": 1, + "values": [ + { + "permission": "%s" + } + ], + "page": 1 +} +` From b1dfa4a5a9a248419a776c43217504d4d5605a1b Mon Sep 17 00:00:00 2001 From: folex <0xdxdy@gmail.com> Date: Sat, 28 Apr 2018 10:25:30 +0300 Subject: [PATCH 3/4] Fix PR comments full_name -> fullName "%s" -> %q errors.New -> fmt.Errorf --- remote/bitbucket/internal/client.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/remote/bitbucket/internal/client.go b/remote/bitbucket/internal/client.go index 1b98dec9..374e280d 100644 --- a/remote/bitbucket/internal/client.go +++ b/remote/bitbucket/internal/client.go @@ -22,7 +22,6 @@ import ( "net/http" "net/url" - "github.com/pkg/errors" "golang.org/x/oauth2" "golang.org/x/oauth2/bitbucket" ) @@ -37,7 +36,7 @@ const ( const ( pathUser = "%s/2.0/user/" pathEmails = "%s/2.0/user/emails" - pathPermissions = "%s/2.0/user/permissions/repositories?q=repository.full_name=\"%s\"" + pathPermissions = "%s/2.0/user/permissions/repositories?q=repository.full_name=%q" pathTeams = "%s/2.0/teams/?%s" pathRepo = "%s/2.0/repositories/%s/%s" pathRepos = "%s/2.0/repositories/%s?%s" @@ -154,9 +153,9 @@ func (c *Client) CreateStatus(owner, name, revision string, status *BuildStatus) return c.do(uri, post, status, nil) } -func (c *Client) GetPermission(full_name string) (*RepoPerm, error) { +func (c *Client) GetPermission(fullName string) (*RepoPerm, error) { out := new(RepoPermResp) - uri := fmt.Sprintf(pathPermissions, c.base, full_name) + uri := fmt.Sprintf(pathPermissions, c.base, fullName) err := c.do(uri, get, nil, out) if err != nil { @@ -164,8 +163,7 @@ func (c *Client) GetPermission(full_name string) (*RepoPerm, error) { } if len(out.Values) == 0 { - err = errors.New(fmt.Sprint("no permissions in repository ", full_name)) - return nil, err + return nil, fmt.Errorf("no permissions in repository ", fullName) } else { return out.Values[0], nil } From 0f737fd82c8d7b7fe006f239104cb55d331c8489 Mon Sep 17 00:00:00 2001 From: folex <0xdxdy@gmail.com> Date: Mon, 30 Apr 2018 11:50:49 +0300 Subject: [PATCH 4/4] Fix fmt.ErrorF usage --- remote/bitbucket/internal/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote/bitbucket/internal/client.go b/remote/bitbucket/internal/client.go index 374e280d..71ea12a7 100644 --- a/remote/bitbucket/internal/client.go +++ b/remote/bitbucket/internal/client.go @@ -163,7 +163,7 @@ func (c *Client) GetPermission(fullName string) (*RepoPerm, error) { } if len(out.Values) == 0 { - return nil, fmt.Errorf("no permissions in repository ", fullName) + return nil, fmt.Errorf("no permissions in repository %s", fullName) } else { return out.Values[0], nil }