From edef62e69ec2a54a870530a81b20a3cc7ca71dbd Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 25 Feb 2021 08:49:27 +0100 Subject: [PATCH] Backport: Repo Transfer permission checks (#14792) (#14794) * Backport: Repo Transfer permission checks (#14792) * update tests --- integrations/api_repo_test.go | 2 +- routers/api/v1/repo/transfer.go | 7 ++++++- routers/repo/setting.go | 5 ++++- services/repository/transfer.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 1aa45984e0..7cabd51add 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -445,7 +445,7 @@ func TestAPIRepoTransfer(t *testing.T) { expectedStatus int }{ {ctxUserID: 1, newOwner: "user2", teams: nil, expectedStatus: http.StatusAccepted}, - {ctxUserID: 2, newOwner: "user1", teams: nil, expectedStatus: http.StatusAccepted}, + {ctxUserID: 2, newOwner: "user1", teams: nil, expectedStatus: http.StatusForbidden}, {ctxUserID: 2, newOwner: "user6", teams: nil, expectedStatus: http.StatusForbidden}, {ctxUserID: 1, newOwner: "user2", teams: &[]int64{2}, expectedStatus: http.StatusUnprocessableEntity}, {ctxUserID: 1, newOwner: "user3", teams: &[]int64{5}, expectedStatus: http.StatusForbidden}, diff --git a/routers/api/v1/repo/transfer.go b/routers/api/v1/repo/transfer.go index b1271b7721..b52f16f906 100644 --- a/routers/api/v1/repo/transfer.go +++ b/routers/api/v1/repo/transfer.go @@ -93,7 +93,12 @@ func Transfer(ctx *context.APIContext, opts api.TransferRepoOption) { } } - if err = repo_service.TransferOwnership(ctx.User, newOwner, ctx.Repo.Repository, teams); err != nil { + if err = repo_service.StartRepositoryTransfer(ctx.User, newOwner, ctx.Repo.Repository, teams); err != nil { + if models.IsErrCancelled(err) { + ctx.Error(http.StatusForbidden, "transfer", "user has no right to create repo for new owner") + return + } + ctx.InternalServerError(err) return } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index e4f8adc38f..e722cd8c28 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -475,9 +475,12 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { ctx.Repo.GitRepo.Close() ctx.Repo.GitRepo = nil } - if err = repo_service.TransferOwnership(ctx.User, newOwner, repo, nil); err != nil { + if err = repo_service.StartRepositoryTransfer(ctx.User, newOwner, repo, nil); err != nil { if models.IsErrRepoAlreadyExist(err) { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil) + } else if models.IsErrCancelled(err) { + // this err msg is not translated, since it was introduced in a backport + ctx.RenderWithErr("user has no right to create repo for new owner", tplSettingsOptions, nil) } else { ctx.ServerError("TransferOwnership", err) } diff --git a/services/repository/transfer.go b/services/repository/transfer.go index d34c812b86..a80f8635bb 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -72,3 +72,31 @@ func ChangeRepositoryName(doer *models.User, repo *models.Repository, newRepoNam return nil } + +// StartRepositoryTransfer transfer a repo from one owner to a new one. +// it make repository into pending transfer state, if doer can not create repo for new owner. +func StartRepositoryTransfer(doer, newOwner *models.User, repo *models.Repository, teams []*models.Team) error { + if repo.Status != models.RepositoryReady { + return fmt.Errorf("repository is not ready for transfer") + } + + // Admin is always allowed to transfer + if doer.IsAdmin { + return TransferOwnership(doer, newOwner, repo, teams) + } + + // If new owner is an org and user can create repos he can transfer directly too + if newOwner.IsOrganization() { + allowed, err := models.CanCreateOrgRepo(newOwner.ID, doer.ID) + if err != nil { + return err + } + if allowed { + return TransferOwnership(doer, newOwner, repo, teams) + } + } + + // Block Transfer, new feature will come in v1.14.0 + // https://github.com/go-gitea/gitea/pull/14792 + return models.ErrCancelled{} +}