From 61e56dd32ce3d285773f68e05ac56ac04bc3e13c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Wed, 4 Jul 2018 23:40:09 +0300 Subject: [PATCH 1/3] Repositories can only migrated to own user or organizations --- integrations/api_repo_test.go | 25 +++++++++++++++++++++++++ routers/api/v1/repo/repo.go | 5 +++++ 2 files changed, 30 insertions(+) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 12429c88a871e..db5ba16f74dfd 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -235,3 +235,28 @@ func TestAPIGetRepoByIDUnauthorized(t *testing.T) { req := NewRequestf(t, "GET", "/api/v1/repositories/2") sess.MakeRequest(t, req, http.StatusNotFound) } + +func TestAPIRepoMigrate(t *testing.T) { + testCases := []struct { + ctxUserID, userID int64 + cloneURL, repoName string + expectedStatus int + }{ + {ctxUserID: 2, userID: 2, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git", expectedStatus: http.StatusCreated}, + {ctxUserID: 2, userID: 1, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-bad", expectedStatus: http.StatusForbidden}, + {ctxUserID: 2, userID: 3, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-org", expectedStatus: http.StatusCreated}, + } + + prepareTestEnv(t) + for _, testCase := range testCases { + user := models.AssertExistsAndLoadBean(t, &models.User{ID: testCase.ctxUserID}).(*models.User) + session := loginUser(t, user.Name) + + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/migrate", &api.MigrateRepoOption{ + CloneAddr: testCase.cloneURL, + UID: int(testCase.userID), + RepoName: testCase.repoName, + }) + session.MakeRequest(t, req, testCase.expectedStatus) + } +} diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index ccfe0440c852a..67e8d91bc095f 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -306,6 +306,11 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { return } + if !ctxUser.IsOrganization() && ctx.User.ID != ctxUser.ID { + ctx.Error(403, "", "Given user is not an organization.") + return + } + if ctxUser.IsOrganization() && !ctx.User.IsAdmin { // Check ownership of organization. isOwner, err := ctxUser.IsOwnedBy(ctx.User.ID) From e0ffe0b58d665bd525cf2010e00034a2b37a31d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Thu, 5 Jul 2018 00:06:02 +0300 Subject: [PATCH 2/3] Add check for organization that user does not belog to --- integrations/api_repo_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index db5ba16f74dfd..79cc216189048 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -245,6 +245,7 @@ func TestAPIRepoMigrate(t *testing.T) { {ctxUserID: 2, userID: 2, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git", expectedStatus: http.StatusCreated}, {ctxUserID: 2, userID: 1, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-bad", expectedStatus: http.StatusForbidden}, {ctxUserID: 2, userID: 3, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-org", expectedStatus: http.StatusCreated}, + {ctxUserID: 2, userID: 6, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-bad-org", expectedStatus: http.StatusForbidden}, } prepareTestEnv(t) From 0cf6c338e7f978c502df63cc47444c4249801fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Thu, 5 Jul 2018 00:31:24 +0300 Subject: [PATCH 3/3] Allow admin to migrate repositories for other users --- integrations/api_repo_test.go | 3 ++- routers/api/v1/repo/repo.go | 28 +++++++++++++++------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 79cc216189048..c789cc9ee4661 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -242,7 +242,8 @@ func TestAPIRepoMigrate(t *testing.T) { cloneURL, repoName string expectedStatus int }{ - {ctxUserID: 2, userID: 2, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git", expectedStatus: http.StatusCreated}, + {ctxUserID: 1, userID: 2, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-admin", expectedStatus: http.StatusCreated}, + {ctxUserID: 2, userID: 2, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-own", expectedStatus: http.StatusCreated}, {ctxUserID: 2, userID: 1, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-bad", expectedStatus: http.StatusForbidden}, {ctxUserID: 2, userID: 3, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-org", expectedStatus: http.StatusCreated}, {ctxUserID: 2, userID: 6, cloneURL: "https://github.com/go-gitea/git.git", repoName: "git-bad-org", expectedStatus: http.StatusForbidden}, diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 67e8d91bc095f..c6c5d4aec3d2b 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -306,21 +306,23 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { return } - if !ctxUser.IsOrganization() && ctx.User.ID != ctxUser.ID { - ctx.Error(403, "", "Given user is not an organization.") - return - } - - if ctxUser.IsOrganization() && !ctx.User.IsAdmin { - // Check ownership of organization. - isOwner, err := ctxUser.IsOwnedBy(ctx.User.ID) - if err != nil { - ctx.Error(500, "IsOwnedBy", err) - return - } else if !isOwner { - ctx.Error(403, "", "Given user is not owner of organization.") + if !ctx.User.IsAdmin { + if !ctxUser.IsOrganization() && ctx.User.ID != ctxUser.ID { + ctx.Error(403, "", "Given user is not an organization.") return } + + if ctxUser.IsOrganization() { + // Check ownership of organization. + isOwner, err := ctxUser.IsOwnedBy(ctx.User.ID) + if err != nil { + ctx.Error(500, "IsOwnedBy", err) + return + } else if !isOwner { + ctx.Error(403, "", "Given user is not owner of organization.") + return + } + } } remoteAddr, err := form.ParseRemoteAddr(ctx.User)