From 918da9ccdbc6da74bbeee06d8fdb227df7a9fc6e Mon Sep 17 00:00:00 2001 From: Yiannis Date: Thu, 24 Sep 2020 17:34:43 +0100 Subject: [PATCH 1/6] Implement HasTokenPermission for GitHub --- github/client.go | 29 +++++++++++++++++++++++++++++ gitprovider/client.go | 4 ++++ gitprovider/enums.go | 6 ++++++ gitprovider/errors.go | 2 ++ 4 files changed, 41 insertions(+) diff --git a/github/client.go b/github/client.go index 068775d0..279ba5d5 100644 --- a/github/client.go +++ b/github/client.go @@ -17,6 +17,9 @@ limitations under the License. package github import ( + "context" + "strings" + "github.com/google/go-github/v32/github" "github.com/fluxcd/go-git-providers/gitprovider" @@ -94,3 +97,29 @@ func (c *Client) OrgRepositories() gitprovider.OrgRepositoriesClient { func (c *Client) UserRepositories() gitprovider.UserRepositoriesClient { return c.userRepos } + +//nolint:gochecknoglobals +var permissionsToScopes = map[gitprovider.TokenPermission]string{ + gitprovider.TokenPermissionFullRepo: "repo", +} + +func (c *Client) HasTokenPermission(ctx context.Context, permission gitprovider.TokenPermission) (bool, error) { + // The X-OAuth-Scopes header is returned for any API calls, using Meta here to keep things simple. + _, res, err := c.c.Client().APIMeta(ctx) + if err != nil { + return false, err + } + + scopes := res.Header.Get("X-OAuth-Scopes") + if scopes == "" { + return false, gitprovider.ErrMissingHeader + } + + for _, scope := range strings.Split(scopes, ",") { + if permissionsToScopes[permission] == scope { + return true, nil + } + } + + return false, nil +} diff --git a/gitprovider/client.go b/gitprovider/client.go index 37d15d6d..ff596153 100644 --- a/gitprovider/client.go +++ b/gitprovider/client.go @@ -33,6 +33,10 @@ type Client interface { // This field is set at client creation time, and can't be changed. ProviderID() ProviderID + // HasTokenPermission returns a boolean indicating whether the supplied token has the requested + // permission. Permissions should be coarse-grained and applicable to *all* providers. + HasTokenPermission(ctx context.Context, permission TokenPermission) (bool, error) + // Raw returns the Go client used under the hood to access the Git provider. Raw() interface{} } diff --git a/gitprovider/enums.go b/gitprovider/enums.go index 639ac29c..1be9da49 100644 --- a/gitprovider/enums.go +++ b/gitprovider/enums.go @@ -161,3 +161,9 @@ func ValidateLicenseTemplate(t LicenseTemplate) error { func LicenseTemplateVar(t LicenseTemplate) *LicenseTemplate { return &t } + +type TokenPermission string + +const ( + TokenPermissionFullRepo = TokenPermission("full_repo") +) diff --git a/gitprovider/errors.go b/gitprovider/errors.go index 44cfb4b7..089dbfab 100644 --- a/gitprovider/errors.go +++ b/gitprovider/errors.go @@ -66,6 +66,8 @@ var ( // ErrInvalidPermissionLevel is the error returned when there is no mapping // from the given level to the gitprovider levels. ErrInvalidPermissionLevel = errors.New("invalid permission level") + // ErrMissingHeader is returned when an expected header is missing from the HTTP response. + ErrMissingHeader = errors.New("header is missing") ) // HTTPError is an error that contains context about the HTTP request/response that failed. From 221dbe31af855bbeb7a3ff4057c1a75e249567b9 Mon Sep 17 00:00:00 2001 From: Yiannis Date: Fri, 25 Sep 2020 14:57:38 +0100 Subject: [PATCH 2/6] Retry GET after POST to allow some time for the repo to be created --- github/integration_test.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/github/integration_test.go b/github/integration_test.go index a40743da..0c8843de 100644 --- a/github/integration_test.go +++ b/github/integration_test.go @@ -248,8 +248,12 @@ var _ = Describe("GitHub Provider", func() { Expect(err).ToNot(HaveOccurred()) validateRepo(repo, repoRef) - getRepo, err := c.OrgRepositories().Get(ctx, repoRef) - Expect(err).ToNot(HaveOccurred()) + var getRepo gitprovider.OrgRepository + Eventually(func() error { + getRepo, err = c.OrgRepositories().Get(ctx, repoRef) + return err + }, 3*time.Second, 1*time.Second).ShouldNot(HaveOccurred()) + // Expect the two responses (one from POST and one from GET to have equal "spec") getSpec := newGithubRepositorySpec(getRepo.APIObject().(*github.Repository)) postSpec := newGithubRepositorySpec(repo.APIObject().(*github.Repository)) @@ -292,8 +296,12 @@ var _ = Describe("GitHub Provider", func() { _, err = anonClient.UserRepositories().Get(ctx, userRepoRef) Expect(errors.Is(err, gitprovider.ErrNotFound)).To(BeTrue()) - getUserRepo, err := c.UserRepositories().Get(ctx, userRepoRef) - Expect(err).ToNot(HaveOccurred()) + var getUserRepo gitprovider.UserRepository + Eventually(func() error { + getUserRepo, err = c.UserRepositories().Get(ctx, userRepoRef) + return err + }, 3*time.Second, 1*time.Second).ShouldNot(HaveOccurred()) + // Expect the two responses (one from POST and one from GET to have equal "spec") getSpec := newGithubRepositorySpec(getUserRepo.APIObject().(*github.Repository)) postSpec := newGithubRepositorySpec(userRepo.APIObject().(*github.Repository)) From fdd22a0217823e67e515ac8996be1bcfec7cc0b1 Mon Sep 17 00:00:00 2001 From: Yiannis Date: Mon, 28 Sep 2020 09:49:21 +0100 Subject: [PATCH 3/6] Add test for GitHub --- github/client.go | 12 +++++++++--- github/integration_test.go | 10 ++++++++++ gitprovider/enums.go | 5 +++-- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/github/client.go b/github/client.go index 279ba5d5..affa2274 100644 --- a/github/client.go +++ b/github/client.go @@ -99,11 +99,16 @@ func (c *Client) UserRepositories() gitprovider.UserRepositoriesClient { } //nolint:gochecknoglobals -var permissionsToScopes = map[gitprovider.TokenPermission]string{ +var permissionScopes = map[gitprovider.TokenPermission]string{ gitprovider.TokenPermissionFullRepo: "repo", } func (c *Client) HasTokenPermission(ctx context.Context, permission gitprovider.TokenPermission) (bool, error) { + requestedScope, ok := permissionScopes[permission] + if !ok { + return false, gitprovider.ErrNoProviderSupport + } + // The X-OAuth-Scopes header is returned for any API calls, using Meta here to keep things simple. _, res, err := c.c.Client().APIMeta(ctx) if err != nil { @@ -115,8 +120,9 @@ func (c *Client) HasTokenPermission(ctx context.Context, permission gitprovider. return false, gitprovider.ErrMissingHeader } - for _, scope := range strings.Split(scopes, ",") { - if permissionsToScopes[permission] == scope { + for _, s := range strings.Split(scopes, ",") { + scope := strings.TrimSpace(s) + if scope == requestedScope { return true, nil } } diff --git a/github/integration_test.go b/github/integration_test.go index 0c8843de..3e559a46 100644 --- a/github/integration_test.go +++ b/github/integration_test.go @@ -365,6 +365,16 @@ var _ = Describe("GitHub Provider", func() { Expect(actionTaken).To(BeTrue()) }) + It("should validate that the token has the correct permissions", func() { + hasPermission, err := c.HasTokenPermission(ctx, 0) + Expect(err).To(Equal(gitprovider.ErrNoProviderSupport)) + Expect(hasPermission).To(Equal(false)) + + hasPermission, err = c.HasTokenPermission(ctx, gitprovider.TokenPermissionFullRepo) + Expect(err).ToNot(HaveOccurred()) + Expect(hasPermission).To(Equal(true)) + }) + AfterSuite(func() { // Don't do anything more if c wasn't created if c == nil { diff --git a/gitprovider/enums.go b/gitprovider/enums.go index 1be9da49..53ca05fb 100644 --- a/gitprovider/enums.go +++ b/gitprovider/enums.go @@ -162,8 +162,9 @@ func LicenseTemplateVar(t LicenseTemplate) *LicenseTemplate { return &t } -type TokenPermission string +type TokenPermission int const ( - TokenPermissionFullRepo = TokenPermission("full_repo") + // Read/Write permission for public/private repositories. + TokenPermissionFullRepo TokenPermission = iota + 1 ) From 384e4e3c18d316b3b87fe26a856576b31bff0ea6 Mon Sep 17 00:00:00 2001 From: Yiannis Date: Mon, 28 Sep 2020 11:40:28 +0100 Subject: [PATCH 4/6] Update gitprovider/enums.go Co-authored-by: Stefan Prodan --- gitprovider/enums.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitprovider/enums.go b/gitprovider/enums.go index 53ca05fb..ca80bc44 100644 --- a/gitprovider/enums.go +++ b/gitprovider/enums.go @@ -166,5 +166,5 @@ type TokenPermission int const ( // Read/Write permission for public/private repositories. - TokenPermissionFullRepo TokenPermission = iota + 1 + TokenPermissionRWRepository TokenPermission = iota + 1 ) From 5babcbfcef7b97f8fb2c9ec4bf3ec6546cc0fcf9 Mon Sep 17 00:00:00 2001 From: Yiannis Date: Mon, 28 Sep 2020 11:44:55 +0100 Subject: [PATCH 5/6] Rename permission --- github/client.go | 2 +- github/integration_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/github/client.go b/github/client.go index affa2274..8c0c56fd 100644 --- a/github/client.go +++ b/github/client.go @@ -100,7 +100,7 @@ func (c *Client) UserRepositories() gitprovider.UserRepositoriesClient { //nolint:gochecknoglobals var permissionScopes = map[gitprovider.TokenPermission]string{ - gitprovider.TokenPermissionFullRepo: "repo", + gitprovider.TokenPermissionRWRepository: "repo", } func (c *Client) HasTokenPermission(ctx context.Context, permission gitprovider.TokenPermission) (bool, error) { diff --git a/github/integration_test.go b/github/integration_test.go index 3e559a46..fd6b8b7a 100644 --- a/github/integration_test.go +++ b/github/integration_test.go @@ -370,7 +370,7 @@ var _ = Describe("GitHub Provider", func() { Expect(err).To(Equal(gitprovider.ErrNoProviderSupport)) Expect(hasPermission).To(Equal(false)) - hasPermission, err = c.HasTokenPermission(ctx, gitprovider.TokenPermissionFullRepo) + hasPermission, err = c.HasTokenPermission(ctx, gitprovider.TokenPermissionRWRepository) Expect(err).ToNot(HaveOccurred()) Expect(hasPermission).To(Equal(true)) }) From 17b7f5a2d2fc59f0f411dc78c71f825622438601 Mon Sep 17 00:00:00 2001 From: Yiannis Date: Tue, 29 Sep 2020 15:15:05 +0100 Subject: [PATCH 6/6] Implement HasTokenPermission for GitLab provider --- gitlab/client.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gitlab/client.go b/gitlab/client.go index 852bae30..8822ca71 100644 --- a/gitlab/client.go +++ b/gitlab/client.go @@ -17,6 +17,8 @@ limitations under the License. package gitlab import ( + "context" + "github.com/fluxcd/go-git-providers/gitprovider" "github.com/xanzy/go-gitlab" ) @@ -102,3 +104,7 @@ func (c *Client) OrgRepositories() gitprovider.OrgRepositoriesClient { func (c *Client) UserRepositories() gitprovider.UserRepositoriesClient { return c.userRepos } + +func (c *Client) HasTokenPermission(ctx context.Context, permission gitprovider.TokenPermission) (bool, error) { + return false, gitprovider.ErrNoProviderSupport +}