From c8137f46028393d880dd9a8060c398ddb8d7bd59 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 9 Oct 2023 10:37:17 +0100 Subject: [PATCH] Validate user login when creating repositories Signed-off-by: Somtochi Onyekwere --- gitea/client_repositories_user.go | 24 +++++++++++++++++++++ gitea/integration_repositories_user_test.go | 17 +++++++++------ github/client_repositories_user.go | 24 +++++++++++++++++++++ github/githubclient.go | 9 ++++++++ github/integration_test.go | 17 +++++++++------ gitlab/client_repositories_user.go | 24 +++++++++++++++++++++ gitlab/gitlabclient.go | 9 ++++++++ gitlab/integration_test.go | 17 +++++++++------ gitprovider/client.go | 3 +++ gitprovider/errors.go | 18 ++++++++++++++++ stash/client_repositories_user.go | 9 ++++++++ 11 files changed, 153 insertions(+), 18 deletions(-) diff --git a/gitea/client_repositories_user.go b/gitea/client_repositories_user.go index 229968b2..439bd56b 100644 --- a/gitea/client_repositories_user.go +++ b/gitea/client_repositories_user.go @@ -95,6 +95,19 @@ func (c *UserRepositoriesClient) listUserRepos(username string) ([]*gitea.Reposi return validateRepositoryObjects(apiObjs) } +// GetUserLogin returns the authenticated user +func (c *UserRepositoriesClient) GetUserLogin(ctx context.Context) (gitprovider.IdentityRef, error) { + // GET /user + user, _, err := c.c.GetMyUserInfo() + if err != nil { + return nil, err + } + return gitprovider.UserRef{ + Domain: c.domain, + UserLogin: user.UserName, + }, nil +} + // Create creates a repository for the given organization, with the data and options // // ErrAlreadyExists will be returned if the resource already exists. @@ -108,6 +121,17 @@ func (c *UserRepositoriesClient) Create(ctx context.Context, return nil, err } + // extra validation to ensure we don't create a project when the wrong owner + // is passed in + idRef, err := c.GetUserLogin(ctx) + if err != nil { + return nil, fmt.Errorf("unable to get owner from API") + } + + if ref.GetIdentity() != idRef.GetIdentity() { + return nil, gitprovider.NewErrIncorrectUser(ref.GetIdentity()) + } + apiObj, err := createRepository(ctx, c.c, ref, "", req, opts...) if err != nil { return nil, err diff --git a/gitea/integration_repositories_user_test.go b/gitea/integration_repositories_user_test.go index f9d91c1b..3aa7a8c3 100644 --- a/gitea/integration_repositories_user_test.go +++ b/gitea/integration_repositories_user_test.go @@ -58,6 +58,13 @@ var _ = Describe("Gitea Provider", func() { Expect(*info.DefaultBranch).To(Equal(defaultBranch)) } + It("should get the current user", func() { + user, err := c.UserRepositories().GetUserLogin(ctx) + Expect(err).ToNot(HaveOccurred()) + + Expect(user.GetIdentity()).To(Equal(giteaUser)) + }) + It("should be possible to create a user repository", func() { // First, check what repositories are available repos, err := c.UserRepositories().List(ctx, newUserRef(giteaUser)) @@ -100,17 +107,15 @@ var _ = Describe("Gitea Provider", func() { Expect(getSpec.Equals(postSpec)).To(BeTrue()) }) - It("should return correct repo info when creating a repository with wrong UserLogin", func() { + It("should fail when creating a repository with wrong UserLogin", func() { repoName := fmt.Sprintf("test-user-repo-creation-%03d", rand.Intn(1000)) repoRef := newUserRepoRef(repoName) repoRef.UserLogin = "yadda-yadda-yada" - repo, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) + _, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) - Expect(err).To(BeNil()) - Expect( - repo.Repository().GetCloneURL(gitprovider.TransportTypeHTTPS)). - To(Equal(fmt.Sprintf("%s/%s/%s.git", giteaBaseUrl, giteaUser, repoName))) + expectedErr := gitprovider.NewErrIncorrectUser(repoRef.UserLogin) + Expect(err).To(MatchError(expectedErr)) }) It("should error at creation time if the repo already does exist", func() { diff --git a/github/client_repositories_user.go b/github/client_repositories_user.go index 572bc58c..be30fd42 100644 --- a/github/client_repositories_user.go +++ b/github/client_repositories_user.go @@ -32,6 +32,19 @@ type UserRepositoriesClient struct { *clientContext } +// GetUserLogin returns the current authenticated user. +func (c *UserRepositoriesClient) GetUserLogin(ctx context.Context) (gitprovider.IdentityRef, error) { + // GET /user + user, err := c.c.GetUser(ctx) + if err != nil { + return nil, err + } + return gitprovider.UserRef{ + Domain: c.domain, + UserLogin: user.GetLogin(), + }, nil +} + // Get returns the repository at the given path. // // ErrNotFound is returned if the resource does not exist. @@ -88,6 +101,17 @@ func (c *UserRepositoriesClient) Create(ctx context.Context, return nil, err } + // extra validation to ensure we don't create a project when the wrong owner + // is passed in + idRef, err := c.GetUserLogin(ctx) + if err != nil { + return nil, fmt.Errorf("unable to get owner from API") + } + + if ref.GetIdentity() != idRef.GetIdentity() { + return nil, gitprovider.NewErrIncorrectUser(ref.GetIdentity()) + } + apiObj, err := createRepository(ctx, c.c, ref, "", req, opts...) if err != nil { return nil, err diff --git a/github/githubclient.go b/github/githubclient.go index 5edb032c..0abc78a1 100644 --- a/github/githubclient.go +++ b/github/githubclient.go @@ -67,6 +67,9 @@ type githubClient interface { // DANGEROUS COMMAND: In order to use this, you must set destructiveActions to true. DeleteRepo(ctx context.Context, owner, repo string) error + // GetUser is a wrapper for "GET /user" + GetUser(ctx context.Context) (*github.User, error) + // ListKeys is a wrapper for "GET /repos/{owner}/{repo}/keys". // This function handles pagination, HTTP error wrapping, and validates the server result. ListKeys(ctx context.Context, owner, repo string) ([]*github.Key, error) @@ -297,6 +300,12 @@ func (c *githubClientImpl) ListKeys(ctx context.Context, owner, repo string) ([] return apiObjs, nil } +func (c *githubClientImpl) GetUser(ctx context.Context) (*github.User, error) { + // GET /user + user, _, err := c.c.Users.Get(ctx, "") + return user, err +} + func (c *githubClientImpl) ListCommitsPage(ctx context.Context, owner, repo, branch string, perPage int, page int) ([]*github.Commit, error) { apiObjs := make([]*github.Commit, 0) lcOpts := &github.CommitsListOptions{ diff --git a/github/integration_test.go b/github/integration_test.go index ea0acc9d..98ca4e67 100644 --- a/github/integration_test.go +++ b/github/integration_test.go @@ -202,6 +202,13 @@ var _ = Describe("GitHub Provider", func() { } } + It("should get the current user", func() { + user, err := c.UserRepositories().GetUserLogin(ctx) + Expect(err).ToNot(HaveOccurred()) + + Expect(user.GetIdentity()).To(Equal(testUser)) + }) + It("should list the available organizations the user has access to", func() { // Get a list of all organizations the user is part of orgs, err := c.Organizations().List(ctx) @@ -345,16 +352,14 @@ var _ = Describe("GitHub Provider", func() { Expect(getSpec.Equals(postSpec)).To(BeTrue()) }) - It("should return correct repo info when creating a repository with wrong UserLogin", func() { + It("should fail when creating a repository with wrong UserLogin", func() { repoName := fmt.Sprintf("test-user-repo-creation-%03d", rand.Intn(1000)) repoRef := newUserRepoRef("yadda-yadda-yada", repoName) - repo, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) + _, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) - Expect(err).To(BeNil()) - Expect( - repo.Repository().GetCloneURL(gitprovider.TransportTypeHTTPS)). - To(Equal(fmt.Sprintf("https://%s/%s/%s.git", githubDomain, testUser, repoName))) + expectedErr := gitprovider.NewErrIncorrectUser(repoRef.GetIdentity()) + Expect(err).To(MatchError(expectedErr)) }) It("should error at creation time if the repo already does exist", func() { diff --git a/gitlab/client_repositories_user.go b/gitlab/client_repositories_user.go index 0f8ffc85..461092c8 100644 --- a/gitlab/client_repositories_user.go +++ b/gitlab/client_repositories_user.go @@ -32,6 +32,19 @@ type UserRepositoriesClient struct { *clientContext } +// GetUserLogin returns the current authenticated user. +func (c *UserRepositoriesClient) GetUserLogin(ctx context.Context) (gitprovider.IdentityRef, error) { + // GET /user + user, err := c.c.GetUser(ctx) + if err != nil { + return nil, err + } + return &gitprovider.UserRef{ + Domain: c.domain, + UserLogin: user.Username, + }, nil +} + // Get returns the repository at the given path. // // ErrNotFound is returned if the resource does not exist. @@ -89,6 +102,17 @@ func (c *UserRepositoriesClient) Create(ctx context.Context, return nil, err } + // extra validation to ensure we don't create a project when the wrong owner + // is passed in + idRef, err := c.GetUserLogin(ctx) + if err != nil { + return nil, fmt.Errorf("unable to get owner from API") + } + + if ref.GetIdentity() != idRef.GetIdentity() { + return nil, gitprovider.NewErrIncorrectUser(ref.GetIdentity()) + } + apiObj, err := createProject(ctx, c.c, ref, "", req, opts...) if err != nil { return nil, err diff --git a/gitlab/gitlabclient.go b/gitlab/gitlabclient.go index a02a4e6f..25b34bf7 100644 --- a/gitlab/gitlabclient.go +++ b/gitlab/gitlabclient.go @@ -76,6 +76,9 @@ type gitlabClient interface { // DANGEROUS COMMAND: In order to use this, you must set destructiveActions to true. DeleteProject(ctx context.Context, projectName string) error + // GetUser is a wrapper for "GET /user" + GetUser(ctx context.Context) (*gitlab.User, error) + // Deploy key methods // ListKeys is a wrapper for "GET /projects/{project}/deploy_keys". @@ -341,6 +344,12 @@ func (c *gitlabClientImpl) DeleteProject(ctx context.Context, projectName string return err } +func (c *gitlabClientImpl) GetUser(ctx context.Context) (*gitlab.User, error) { + // GET /user + proj, _, err := c.c.Users.CurrentUser(gitlab.WithContext(ctx)) + return proj, err +} + func (c *gitlabClientImpl) ListKeys(projectName string) ([]*gitlab.ProjectDeployKey, error) { apiObjs := []*gitlab.ProjectDeployKey{} opts := &gitlab.ListProjectDeployKeysOptions{} diff --git a/gitlab/integration_test.go b/gitlab/integration_test.go index bda91f36..bb9512ea 100644 --- a/gitlab/integration_test.go +++ b/gitlab/integration_test.go @@ -318,6 +318,13 @@ var _ = Describe("GitLab Provider", func() { } } + It("should get the current user", func() { + user, err := c.UserRepositories().GetUserLogin(ctx) + Expect(err).ToNot(HaveOccurred()) + + Expect(user.GetIdentity()).To(Equal(testUserName)) + }) + It("should list the available organizations the user has access to", func() { // Get a list of all organizations the user is part of orgs, err := c.Organizations().List(ctx) @@ -796,16 +803,14 @@ var _ = Describe("GitLab Provider", func() { Expect(errors.Is(err, gitprovider.ErrAlreadyExists)).To(BeTrue()) }) - It("should return correct repo info when creating a repository with wrong UserLogin", func() { + It("should fail info when creating a repository with wrong UserLogin", func() { repoName := fmt.Sprintf("test-user-repo-creation-%03d", rand.Intn(1000)) repoRef := newUserRepoRef(testBaseUrl, "yadda-yadda-yada", repoName) - repo, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) + _, err := c.UserRepositories().Create(ctx, repoRef, gitprovider.RepositoryInfo{}) - Expect(err).To(BeNil()) - Expect( - repo.Repository().GetCloneURL(gitprovider.TransportTypeHTTPS)). - To(Equal(fmt.Sprintf("%s/%s/%s.git", testBaseUrl, testUserName, repoName))) + expectedErr := gitprovider.NewErrIncorrectUser(repoRef.GetIdentity()) + Expect(err).To(MatchError(expectedErr)) }) It("should update if the user repo already exists when reconciling", func() { diff --git a/gitprovider/client.go b/gitprovider/client.go index ffff12a2..21fc6be4 100644 --- a/gitprovider/client.go +++ b/gitprovider/client.go @@ -123,6 +123,9 @@ type UserRepositoriesClient interface { // ErrAlreadyExists will be returned if the resource already exists. Create(ctx context.Context, r UserRepositoryRef, req RepositoryInfo, opts ...RepositoryCreateOption) (UserRepository, error) + // GetUserLogin returns the current authenticated user. + GetUserLogin(ctx context.Context) (IdentityRef, error) + // Reconcile makes sure the given desired state (req) becomes the actual state in the backing Git provider. // // If req doesn't exist under the hood, it is created (actionTaken == true). diff --git a/gitprovider/errors.go b/gitprovider/errors.go index 0383212f..ef360dac 100644 --- a/gitprovider/errors.go +++ b/gitprovider/errors.go @@ -18,6 +18,7 @@ package gitprovider import ( "errors" + "fmt" "net/http" "time" ) @@ -131,3 +132,20 @@ type InvalidCredentialsError struct { // InvalidCredentialsError extends HTTPError. HTTPError `json:",inline"` } + +// ErrIncorrectUser describes that the user provided was incorrect +// +// It is returned by `UserRepositories().Create` when an incorrect UserLogin is passed in +type ErrIncorrectUser struct { + user string +} + +// Error implements the error interface. +func NewErrIncorrectUser(user string) *ErrIncorrectUser { + return &ErrIncorrectUser{user} +} + +// Error implements the error interface. +func (e *ErrIncorrectUser) Error() string { + return fmt.Sprintf("incorrect user '%s' provided", e.user) +} diff --git a/stash/client_repositories_user.go b/stash/client_repositories_user.go index 4b7693dd..f3dbc06c 100644 --- a/stash/client_repositories_user.go +++ b/stash/client_repositories_user.go @@ -34,6 +34,15 @@ type UserRepositoriesClient struct { *clientContext } +// GetUserLogin returns the authenticated user. +// +// Stash currently doesn't have an endpoint for this, so this +// is mostly to implement the interface. +func (c *UserRepositoriesClient) GetUserLogin(ctx context.Context) (gitprovider.IdentityRef, error) { + // TODO: call API for stash + return gitprovider.UserRef{}, nil +} + // Get returns the repository at the given path. // ErrNotFound is returned if the resource does not exist. func (c *UserRepositoriesClient) Get(ctx context.Context, ref gitprovider.UserRepositoryRef) (gitprovider.UserRepository, error) {