From f7d1451ca3c5046f85013d3ac56d7f42e994d683 Mon Sep 17 00:00:00 2001 From: Anbraten Date: Mon, 3 Jan 2022 15:33:36 +0100 Subject: [PATCH] pass repo to remote perm func (#653) * pass repo to remote perm func * go:generate mockery Co-authored-by: 6543 <6543@obermui.de> --- server/remote/bitbucket/bitbucket.go | 4 ++-- server/remote/bitbucket/bitbucket_test.go | 8 +++---- .../remote/bitbucketserver/bitbucketserver.go | 4 ++-- server/remote/coding/coding.go | 4 ++-- server/remote/coding/coding_test.go | 10 ++++---- server/remote/gitea/gitea.go | 4 ++-- server/remote/gitea/gitea_test.go | 4 ++-- server/remote/github/github.go | 4 ++-- server/remote/github/github_test.go | 4 ++-- server/remote/gitlab/gitlab.go | 4 ++-- server/remote/gitlab/gitlab_test.go | 12 +++++++--- server/remote/gogs/gogs.go | 4 ++-- server/remote/gogs/gogs_test.go | 4 ++-- server/remote/mocks/remote.go | 23 ++++++++++--------- server/remote/remote.go | 2 +- server/router/middleware/session/repo.go | 2 +- server/shared/userSyncer.go | 2 +- 17 files changed, 53 insertions(+), 46 deletions(-) diff --git a/server/remote/bitbucket/bitbucket.go b/server/remote/bitbucket/bitbucket.go index 443cf7682..4f3a9b4a0 100644 --- a/server/remote/bitbucket/bitbucket.go +++ b/server/remote/bitbucket/bitbucket.go @@ -180,11 +180,11 @@ func (c *config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error // does not have an endpoint to access user permissions, we attempt to fetch // the repository hook list, which is restricted to administrators to calculate // administrative access to a repository. -func (c *config) Perm(ctx context.Context, u *model.User, owner, name string) (*model.Perm, error) { +func (c *config) Perm(ctx context.Context, u *model.User, r *model.Repo) (*model.Perm, error) { client := c.newClient(ctx, u) perms := new(model.Perm) - repo, err := client.FindRepo(owner, name) + repo, err := client.FindRepo(r.Owner, r.Name) if err != nil { return perms, err } diff --git a/server/remote/bitbucket/bitbucket_test.go b/server/remote/bitbucket/bitbucket_test.go index 5e392bfa0..7ed57df0d 100644 --- a/server/remote/bitbucket/bitbucket_test.go +++ b/server/remote/bitbucket/bitbucket_test.go @@ -138,25 +138,25 @@ func Test_bitbucket(t *testing.T) { g.Describe("When requesting repository permissions", func() { g.It("Should handle not found errors", func() { - _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name) + _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound) g.Assert(err).IsNotNil() }) g.It("Should authorize read access", func() { - perm, err := c.Perm(ctx, fakeUser, fakeRepoReadOnly.Owner, fakeRepoReadOnly.Name) + perm, err := c.Perm(ctx, fakeUser, fakeRepoReadOnly) g.Assert(err).IsNil() 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(ctx, fakeUser, fakeRepoWriteOnly.Owner, fakeRepoWriteOnly.Name) + perm, err := c.Perm(ctx, fakeUser, fakeRepoWriteOnly) g.Assert(err).IsNil() 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(ctx, fakeUser, fakeRepoAdmin.Owner, fakeRepoAdmin.Name) + perm, err := c.Perm(ctx, fakeUser, fakeRepoAdmin) g.Assert(err).IsNil() g.Assert(perm.Pull).IsTrue() g.Assert(perm.Push).IsTrue() diff --git a/server/remote/bitbucketserver/bitbucketserver.go b/server/remote/bitbucketserver/bitbucketserver.go index d909b7b02..9d4b6cff3 100644 --- a/server/remote/bitbucketserver/bitbucketserver.go +++ b/server/remote/bitbucketserver/bitbucketserver.go @@ -169,10 +169,10 @@ func (c *Config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error return all, nil } -func (c *Config) Perm(ctx context.Context, u *model.User, owner, repo string) (*model.Perm, error) { +func (c *Config) Perm(ctx context.Context, u *model.User, repo *model.Repo) (*model.Perm, error) { client := internal.NewClientWithToken(ctx, c.URL, c.Consumer, u.Token) - return client.FindRepoPerms(owner, repo) + return client.FindRepoPerms(repo.Owner, repo.Name) } func (c *Config) File(ctx context.Context, u *model.User, r *model.Repo, b *model.Build, f string) ([]byte, error) { diff --git a/server/remote/coding/coding.go b/server/remote/coding/coding.go index 4d80950f6..c251f69ef 100644 --- a/server/remote/coding/coding.go +++ b/server/remote/coding/coding.go @@ -213,8 +213,8 @@ func (c *Coding) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error // Perm fetches the named repository permissions from // the remote system for the specified user. -func (c *Coding) Perm(ctx context.Context, u *model.User, owner, repo string) (*model.Perm, error) { - project, err := c.newClient(ctx, u).GetProject(owner, repo) +func (c *Coding) Perm(ctx context.Context, u *model.User, repo *model.Repo) (*model.Perm, error) { + project, err := c.newClient(ctx, u).GetProject(repo.Owner, repo.Name) if err != nil { return nil, err } diff --git a/server/remote/coding/coding_test.go b/server/remote/coding/coding_test.go index dae813858..3034f2fbc 100644 --- a/server/remote/coding/coding_test.go +++ b/server/remote/coding/coding_test.go @@ -128,35 +128,35 @@ func Test_coding(t *testing.T) { g.Describe("When requesting repository permissions", func() { g.It("Should authorize admin access for project owner", func() { - perm, err := c.Perm(ctx, fakeUser, "demo1", "perm_owner") + perm, err := c.Perm(ctx, fakeUser, &model.Repo{Owner: "demo1", Name: "perm_owner"}) g.Assert(err).IsNil() g.Assert(perm.Pull).IsTrue() g.Assert(perm.Push).IsTrue() g.Assert(perm.Admin).IsTrue() }) g.It("Should authorize admin access for project admin", func() { - perm, err := c.Perm(ctx, fakeUser, "demo1", "perm_admin") + perm, err := c.Perm(ctx, fakeUser, &model.Repo{Owner: "demo1", Name: "perm_admin"}) g.Assert(err).IsNil() g.Assert(perm.Pull).IsTrue() g.Assert(perm.Push).IsTrue() g.Assert(perm.Admin).IsTrue() }) g.It("Should authorize read access for project member", func() { - perm, err := c.Perm(ctx, fakeUser, "demo1", "perm_member") + perm, err := c.Perm(ctx, fakeUser, &model.Repo{Owner: "demo1", Name: "perm_member"}) g.Assert(err).IsNil() g.Assert(perm.Pull).IsTrue() g.Assert(perm.Push).IsTrue() g.Assert(perm.Admin).IsFalse() }) g.It("Should authorize no access for project guest", func() { - perm, err := c.Perm(ctx, fakeUser, "demo1", "perm_guest") + perm, err := c.Perm(ctx, fakeUser, &model.Repo{Owner: "demo1", Name: "perm_guest"}) g.Assert(err).IsNil() g.Assert(perm.Pull).IsFalse() g.Assert(perm.Push).IsFalse() g.Assert(perm.Admin).IsFalse() }) g.It("Should handle not found errors", func() { - _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name) + _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound) g.Assert(err).IsNotNil() }) }) diff --git a/server/remote/gitea/gitea.go b/server/remote/gitea/gitea.go index 89e5ff5f6..c49f3b9fe 100644 --- a/server/remote/gitea/gitea.go +++ b/server/remote/gitea/gitea.go @@ -279,13 +279,13 @@ func (c *Gitea) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error) } // Perm returns the user permissions for the named Gitea repository. -func (c *Gitea) Perm(ctx context.Context, u *model.User, owner, name string) (*model.Perm, error) { +func (c *Gitea) Perm(ctx context.Context, u *model.User, r *model.Repo) (*model.Perm, error) { client, err := c.newClientToken(ctx, u.Token) if err != nil { return nil, err } - repo, _, err := client.GetRepo(owner, name) + repo, _, err := client.GetRepo(r.Owner, r.Name) if err != nil { return nil, err } diff --git a/server/remote/gitea/gitea_test.go b/server/remote/gitea/gitea_test.go index 63f66a9e6..86f4c5fd3 100644 --- a/server/remote/gitea/gitea_test.go +++ b/server/remote/gitea/gitea_test.go @@ -108,14 +108,14 @@ func Test_gitea(t *testing.T) { g.Describe("Requesting repository permissions", func() { g.It("Should return the permission details", func() { - perm, err := c.Perm(ctx, fakeUser, fakeRepo.Owner, fakeRepo.Name) + perm, err := c.Perm(ctx, fakeUser, fakeRepo) g.Assert(err).IsNil() g.Assert(perm.Admin).IsTrue() g.Assert(perm.Push).IsTrue() g.Assert(perm.Pull).IsTrue() }) g.It("Should handle a not found error", func() { - _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name) + _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound) g.Assert(err).IsNotNil() }) }) diff --git a/server/remote/github/github.go b/server/remote/github/github.go index 809e5ec17..ad4cbe859 100644 --- a/server/remote/github/github.go +++ b/server/remote/github/github.go @@ -213,9 +213,9 @@ func (c *client) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error } // Perm returns the user permissions for the named GitHub repository. -func (c *client) Perm(ctx context.Context, u *model.User, owner, name string) (*model.Perm, error) { +func (c *client) Perm(ctx context.Context, u *model.User, r *model.Repo) (*model.Perm, error) { client := c.newClientToken(ctx, u.Token) - repo, _, err := client.Repositories.Get(ctx, owner, name) + repo, _, err := client.Repositories.Get(ctx, r.Owner, r.Name) if err != nil { return nil, err } diff --git a/server/remote/github/github_test.go b/server/remote/github/github_test.go index fa4099ed7..e09bbc911 100644 --- a/server/remote/github/github_test.go +++ b/server/remote/github/github_test.go @@ -113,14 +113,14 @@ func Test_github(t *testing.T) { g.Describe("Requesting repository permissions", func() { g.It("Should return the permission details", func() { - perm, err := c.Perm(ctx, fakeUser, fakeRepo.Owner, fakeRepo.Name) + perm, err := c.Perm(ctx, fakeUser, fakeRepo) g.Assert(err).IsNil() g.Assert(perm.Admin).IsTrue() g.Assert(perm.Push).IsTrue() g.Assert(perm.Pull).IsTrue() }) g.It("Should handle a not found error", func() { - _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name) + _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound) g.Assert(err).IsNotNil() }) }) diff --git a/server/remote/gitlab/gitlab.go b/server/remote/gitlab/gitlab.go index c1385c2d4..14fd871a9 100644 --- a/server/remote/gitlab/gitlab.go +++ b/server/remote/gitlab/gitlab.go @@ -261,12 +261,12 @@ func (g *Gitlab) Repos(ctx context.Context, user *model.User) ([]*model.Repo, er } // Perm fetches the named repository from the remote system. -func (g *Gitlab) Perm(ctx context.Context, user *model.User, owner, name string) (*model.Perm, error) { +func (g *Gitlab) Perm(ctx context.Context, user *model.User, r *model.Repo) (*model.Perm, error) { client, err := newClient(g.URL, user.Token, g.SkipVerify) if err != nil { return nil, err } - repo, err := g.getProject(ctx, client, owner, name) + repo, err := g.getProject(ctx, client, r.Owner, r.Name) if err != nil { return nil, err } diff --git a/server/remote/gitlab/gitlab_test.go b/server/remote/gitlab/gitlab_test.go index 8e972cd19..d15acae05 100644 --- a/server/remote/gitlab/gitlab_test.go +++ b/server/remote/gitlab/gitlab_test.go @@ -109,21 +109,27 @@ func Test_Gitlab(t *testing.T) { // Test permissions method g.Describe("Perm", func() { g.It("Should return repo permissions", func() { - perm, err := client.Perm(ctx, &user, "diaspora", "diaspora-client") + perm, err := client.Perm(ctx, &user, &repo) assert.NoError(t, err) assert.True(t, perm.Admin) assert.True(t, perm.Pull) assert.True(t, perm.Push) }) g.It("Should return repo permissions when user is admin", func() { - perm, err := client.Perm(ctx, &user, "brightbox", "puppet") + perm, err := client.Perm(ctx, &user, &model.Repo{ + Owner: "brightbox", + Name: "puppet", + }) assert.NoError(t, err) g.Assert(perm.Admin).Equal(true) g.Assert(perm.Pull).Equal(true) g.Assert(perm.Push).Equal(true) }) g.It("Should return error, when repo is not exist", func() { - _, err := client.Perm(ctx, &user, "not-existed", "not-existed") + _, err := client.Perm(ctx, &user, &model.Repo{ + Owner: "not-existed", + Name: "not-existed", + }) g.Assert(err).IsNotNil() }) diff --git a/server/remote/gogs/gogs.go b/server/remote/gogs/gogs.go index 43fbcc10c..97f018228 100644 --- a/server/remote/gogs/gogs.go +++ b/server/remote/gogs/gogs.go @@ -171,9 +171,9 @@ func (c *client) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error } // Perm returns the user permissions for the named Gogs repository. -func (c *client) Perm(ctx context.Context, u *model.User, owner, name string) (*model.Perm, error) { +func (c *client) Perm(ctx context.Context, u *model.User, r *model.Repo) (*model.Perm, error) { client := c.newClientToken(u.Token) - repo, err := client.GetRepo(owner, name) + repo, err := client.GetRepo(r.Owner, r.Name) if err != nil { return nil, err } diff --git a/server/remote/gogs/gogs_test.go b/server/remote/gogs/gogs_test.go index 831fdc281..2b9d5b015 100644 --- a/server/remote/gogs/gogs_test.go +++ b/server/remote/gogs/gogs_test.go @@ -106,14 +106,14 @@ func Test_gogs(t *testing.T) { g.Describe("Requesting repository permissions", func() { g.It("Should return the permission details", func() { - perm, err := c.Perm(ctx, fakeUser, fakeRepo.Owner, fakeRepo.Name) + perm, err := c.Perm(ctx, fakeUser, fakeRepo) g.Assert(err).IsNil() g.Assert(perm.Admin).IsTrue() g.Assert(perm.Push).IsTrue() g.Assert(perm.Pull).IsTrue() }) g.It("Should handle a not found error", func() { - _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound.Owner, fakeRepoNotFound.Name) + _, err := c.Perm(ctx, fakeUser, fakeRepoNotFound) g.Assert(err).IsNotNil() }) }) diff --git a/server/remote/mocks/remote.go b/server/remote/mocks/remote.go index 3eec64db2..c56bedeec 100644 --- a/server/remote/mocks/remote.go +++ b/server/remote/mocks/remote.go @@ -9,6 +9,7 @@ import ( mock "github.com/stretchr/testify/mock" model "github.com/woodpecker-ci/woodpecker/server/model" + remote "github.com/woodpecker-ci/woodpecker/server/remote" ) @@ -213,13 +214,13 @@ func (_m *Remote) Netrc(u *model.User, r *model.Repo) (*model.Netrc, error) { return r0, r1 } -// Perm provides a mock function with given fields: ctx, u, owner, repo -func (_m *Remote) Perm(ctx context.Context, u *model.User, owner string, repo string) (*model.Perm, error) { - ret := _m.Called(ctx, u, owner, repo) +// Perm provides a mock function with given fields: ctx, u, r +func (_m *Remote) Perm(ctx context.Context, u *model.User, r *model.Repo) (*model.Perm, error) { + ret := _m.Called(ctx, u, r) var r0 *model.Perm - if rf, ok := ret.Get(0).(func(context.Context, *model.User, string, string) *model.Perm); ok { - r0 = rf(ctx, u, owner, repo) + if rf, ok := ret.Get(0).(func(context.Context, *model.User, *model.Repo) *model.Perm); ok { + r0 = rf(ctx, u, r) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Perm) @@ -227,8 +228,8 @@ func (_m *Remote) Perm(ctx context.Context, u *model.User, owner string, repo st } var r1 error - if rf, ok := ret.Get(1).(func(context.Context, *model.User, string, string) error); ok { - r1 = rf(ctx, u, owner, repo) + if rf, ok := ret.Get(1).(func(context.Context, *model.User, *model.Repo) error); ok { + r1 = rf(ctx, u, r) } else { r1 = ret.Error(1) } @@ -282,13 +283,13 @@ func (_m *Remote) Repos(ctx context.Context, u *model.User) ([]*model.Repo, erro return r0, r1 } -// Status provides a mock function with given fields: ctx, u, r, b, link, proc -func (_m *Remote) Status(ctx context.Context, u *model.User, r *model.Repo, b *model.Build, proc *model.Proc) error { - ret := _m.Called(ctx, u, r, b, proc) +// Status provides a mock function with given fields: ctx, u, r, b, p +func (_m *Remote) Status(ctx context.Context, u *model.User, r *model.Repo, b *model.Build, p *model.Proc) error { + ret := _m.Called(ctx, u, r, b, p) var r0 error if rf, ok := ret.Get(0).(func(context.Context, *model.User, *model.Repo, *model.Build, *model.Proc) error); ok { - r0 = rf(ctx, u, r, b, proc) + r0 = rf(ctx, u, r, b, p) } else { r0 = ret.Error(0) } diff --git a/server/remote/remote.go b/server/remote/remote.go index 68a047db1..78a567f2a 100644 --- a/server/remote/remote.go +++ b/server/remote/remote.go @@ -46,7 +46,7 @@ type Remote interface { // Perm fetches the named repository permissions from // the remote system for the specified user. - Perm(ctx context.Context, u *model.User, owner, repo string) (*model.Perm, error) + Perm(ctx context.Context, u *model.User, r *model.Repo) (*model.Perm, error) // File fetches a file from the remote repository and returns in string // format. diff --git a/server/router/middleware/session/repo.go b/server/router/middleware/session/repo.go index 33ae27be5..773d1baec 100644 --- a/server/router/middleware/session/repo.go +++ b/server/router/middleware/session/repo.go @@ -97,7 +97,7 @@ func SetPerm() gin.HandlerFunc { user.Login, repo.FullName, err) } if time.Unix(perm.Synced, 0).Add(time.Hour).Before(time.Now()) { - perm, err = server.Config.Services.Remote.Perm(c, user, repo.Owner, repo.Name) + perm, err = server.Config.Services.Remote.Perm(c, user, repo) if err == nil { log.Debug().Msgf("Synced user permission for %s %s", user.Login, repo.FullName) perm.Repo = repo.FullName diff --git a/server/shared/userSyncer.go b/server/shared/userSyncer.go index 5cbef9f07..537cbc055 100644 --- a/server/shared/userSyncer.go +++ b/server/shared/userSyncer.go @@ -84,7 +84,7 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo repo.Perm.Admin = true } } else { - remotePerm, err := s.Remote.Perm(ctx, user, repo.Owner, repo.Name) + remotePerm, err := s.Remote.Perm(ctx, user, repo) if err != nil { return fmt.Errorf("could not fetch permission of repo '%s': %v", repo.FullName, err) }