diff --git a/models/repo/transfer.go b/models/repo/transfer.go index 43e15b33bcf..b669145d68a 100644 --- a/models/repo/transfer.go +++ b/models/repo/transfer.go @@ -68,6 +68,7 @@ type RepoTransfer struct { //nolint RecipientID int64 Recipient *user_model.User `xorm:"-"` RepoID int64 + Repo *Repository `xorm:"-"` TeamIDs []int64 Teams []*organization.Team `xorm:"-"` @@ -79,48 +80,65 @@ func init() { db.RegisterModel(new(RepoTransfer)) } -// LoadAttributes fetches the transfer recipient from the database -func (r *RepoTransfer) LoadAttributes(ctx context.Context) error { +func (r *RepoTransfer) LoadRecipient(ctx context.Context) error { if r.Recipient == nil { u, err := user_model.GetUserByID(ctx, r.RecipientID) if err != nil { return err } - r.Recipient = u } - if r.Recipient.IsOrganization() && len(r.TeamIDs) != len(r.Teams) { - for _, v := range r.TeamIDs { - team, err := organization.GetTeamByID(ctx, v) - if err != nil { - return err - } + return nil +} - if team.OrgID != r.Recipient.ID { - return fmt.Errorf("team %d belongs not to org %d", v, r.Recipient.ID) - } +func (r *RepoTransfer) LoadRepo(ctx context.Context) error { + if r.Repo == nil { + repo, err := GetRepositoryByID(ctx, r.RepoID) + if err != nil { + return err + } + r.Repo = repo + } + + return nil +} + +// LoadAttributes fetches the transfer recipient from the database +func (r *RepoTransfer) LoadAttributes(ctx context.Context) error { + if err := r.LoadRecipient(ctx); err != nil { + return err + } + if r.Recipient.IsOrganization() && r.Teams == nil { + teamsMap, err := organization.GetTeamsByIDs(ctx, r.TeamIDs) + if err != nil { + return err + } + for _, team := range teamsMap { r.Teams = append(r.Teams, team) } } + if err := r.LoadRepo(ctx); err != nil { + return err + } + if r.Doer == nil { u, err := user_model.GetUserByID(ctx, r.DoerID) if err != nil { return err } - r.Doer = u } return nil } -// CanUserAcceptTransfer checks if the user has the rights to accept/decline a repo transfer. +// CanUserAcceptOrRejectTransfer checks if the user has the rights to accept/decline a repo transfer. // For user, it checks if it's himself // For organizations, it checks if the user is able to create repos -func (r *RepoTransfer) CanUserAcceptTransfer(ctx context.Context, u *user_model.User) bool { +func (r *RepoTransfer) CanUserAcceptOrRejectTransfer(ctx context.Context, u *user_model.User) bool { if err := r.LoadAttributes(ctx); err != nil { log.Error("LoadAttributes: %v", err) return false @@ -166,6 +184,10 @@ func GetPendingRepositoryTransfers(ctx context.Context, opts *PendingRepositoryT Find(&transfers) } +func IsRepositoryTransferExist(ctx context.Context, repoID int64) (bool, error) { + return db.GetEngine(ctx).Where("repo_id = ?", repoID).Exist(new(RepoTransfer)) +} + // GetPendingRepositoryTransfer fetches the most recent and ongoing transfer // process for the repository func GetPendingRepositoryTransfer(ctx context.Context, repo *Repository) (*RepoTransfer, error) { @@ -206,11 +228,26 @@ func CreatePendingRepositoryTransfer(ctx context.Context, doer, newOwner *user_m return err } + if _, err := user_model.GetUserByID(ctx, newOwner.ID); err != nil { + return err + } + // Make sure repo is ready to transfer if err := TestRepositoryReadyForTransfer(repo.Status); err != nil { return err } + exist, err := IsRepositoryTransferExist(ctx, repo.ID) + if err != nil { + return err + } + if exist { + return ErrRepoTransferInProgress{ + Uname: repo.Owner.LowerName, + Name: repo.Name, + } + } + repo.Status = RepositoryPendingTransfer if err := UpdateRepositoryCols(ctx, repo, "status"); err != nil { return err diff --git a/routers/api/v1/repo/transfer.go b/routers/api/v1/repo/transfer.go index b2090cac41c..bb666f6487e 100644 --- a/routers/api/v1/repo/transfer.go +++ b/routers/api/v1/repo/transfer.go @@ -15,6 +15,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" @@ -161,12 +162,16 @@ func AcceptTransfer(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - err := acceptOrRejectRepoTransfer(ctx, true) - if ctx.Written() { - return - } + err := repo_service.AcceptTransferOwnership(ctx, ctx.Repo.Repository, ctx.Doer) if err != nil { - ctx.Error(http.StatusInternalServerError, "acceptOrRejectRepoTransfer", err) + switch { + case repo_model.IsErrNoPendingTransfer(err): + ctx.Error(http.StatusNotFound, "AcceptTransferOwnership", err) + case errors.Is(err, util.ErrPermissionDenied): + ctx.Error(http.StatusForbidden, "AcceptTransferOwnership", err) + default: + ctx.Error(http.StatusInternalServerError, "AcceptTransferOwnership", err) + } return } @@ -199,40 +204,18 @@ func RejectTransfer(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - err := acceptOrRejectRepoTransfer(ctx, false) - if ctx.Written() { - return - } + err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer) if err != nil { - ctx.Error(http.StatusInternalServerError, "acceptOrRejectRepoTransfer", err) + switch { + case repo_model.IsErrNoPendingTransfer(err): + ctx.Error(http.StatusNotFound, "RejectRepositoryTransfer", err) + case errors.Is(err, util.ErrPermissionDenied): + ctx.Error(http.StatusForbidden, "RejectRepositoryTransfer", err) + default: + ctx.Error(http.StatusInternalServerError, "RejectRepositoryTransfer", err) + } return } ctx.JSON(http.StatusOK, convert.ToRepo(ctx, ctx.Repo.Repository, ctx.Repo.Permission)) } - -func acceptOrRejectRepoTransfer(ctx *context.APIContext, accept bool) error { - repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository) - if err != nil { - if repo_model.IsErrNoPendingTransfer(err) { - ctx.NotFound() - return nil - } - return err - } - - if err := repoTransfer.LoadAttributes(ctx); err != nil { - return err - } - - if !repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer) { - ctx.Error(http.StatusForbidden, "CanUserAcceptTransfer", nil) - return fmt.Errorf("user does not have permissions to do this") - } - - if accept { - return repo_service.TransferOwnership(ctx, repoTransfer.Doer, repoTransfer.Recipient, ctx.Repo.Repository, repoTransfer.Teams) - } - - return repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository) -} diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 6c2bc74e690..8ebf5bcf390 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -309,6 +309,36 @@ const ( tplStarUnstar templates.TplName = "repo/star_unstar" ) +func acceptTransfer(ctx *context.Context) { + err := repo_service.AcceptTransferOwnership(ctx, ctx.Repo.Repository, ctx.Doer) + if err == nil { + ctx.Flash.Success(ctx.Tr("repo.settings.transfer.success")) + ctx.Redirect(ctx.Repo.Repository.Link()) + return + } + handleActionError(ctx, err) +} + +func rejectTransfer(ctx *context.Context) { + err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer) + if err == nil { + ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected")) + ctx.Redirect(ctx.Repo.Repository.Link()) + return + } + handleActionError(ctx, err) +} + +func handleActionError(ctx *context.Context, err error) { + if errors.Is(err, user_model.ErrBlockedUser) { + ctx.Flash.Error(ctx.Tr("repo.action.blocked_user")) + } else if errors.Is(err, util.ErrPermissionDenied) { + ctx.Error(http.StatusNotFound) + } else { + ctx.ServerError(fmt.Sprintf("Action (%s)", ctx.PathParam("action")), err) + } +} + // Action response for actions to a repository func Action(ctx *context.Context) { var err error @@ -322,9 +352,11 @@ func Action(ctx *context.Context) { case "unstar": err = repo_model.StarRepo(ctx, ctx.Doer, ctx.Repo.Repository, false) case "accept_transfer": - err = acceptOrRejectRepoTransfer(ctx, true) + acceptTransfer(ctx) + return case "reject_transfer": - err = acceptOrRejectRepoTransfer(ctx, false) + rejectTransfer(ctx) + return case "desc": // FIXME: this is not used if !ctx.Repo.IsOwner() { ctx.Error(http.StatusNotFound) @@ -337,12 +369,8 @@ func Action(ctx *context.Context) { } if err != nil { - if errors.Is(err, user_model.ErrBlockedUser) { - ctx.Flash.Error(ctx.Tr("repo.action.blocked_user")) - } else { - ctx.ServerError(fmt.Sprintf("Action (%s)", ctx.PathParam("action")), err) - return - } + handleActionError(ctx, err) + return } switch ctx.PathParam("action") { @@ -377,41 +405,6 @@ func Action(ctx *context.Context) { ctx.RedirectToCurrentSite(ctx.FormString("redirect_to"), ctx.Repo.RepoLink) } -func acceptOrRejectRepoTransfer(ctx *context.Context, accept bool) error { - repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository) - if err != nil { - return err - } - - if err := repoTransfer.LoadAttributes(ctx); err != nil { - return err - } - - if !repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer) { - return errors.New("user does not have enough permissions") - } - - if accept { - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - ctx.Repo.GitRepo = nil - } - - if err := repo_service.TransferOwnership(ctx, repoTransfer.Doer, repoTransfer.Recipient, ctx.Repo.Repository, repoTransfer.Teams); err != nil { - return err - } - ctx.Flash.Success(ctx.Tr("repo.settings.transfer.success")) - } else { - if err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository); err != nil { - return err - } - ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected")) - } - - ctx.Redirect(ctx.Repo.Repository.Link()) - return nil -} - // RedirectDownload return a file based on the following infos: func RedirectDownload(ctx *context.Context) { var ( diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 5b34fc60da5..be37d0a9c45 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -832,16 +832,10 @@ func SettingsPost(ctx *context.Context) { } else { ctx.ServerError("GetPendingRepositoryTransfer", err) } - - return - } - - if err := repoTransfer.LoadAttributes(ctx); err != nil { - ctx.ServerError("LoadRecipient", err) return } - if err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository); err != nil { + if err := repo_service.CancelRepositoryTransfer(ctx, repoTransfer, ctx.Doer); err != nil { ctx.ServerError("CancelRepositoryTransfer", err) return } diff --git a/services/context/repo.go b/services/context/repo.go index 32f8aaed2e6..b0cfd78cf55 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -653,7 +653,7 @@ func RepoAssignment(ctx *Context) { ctx.Data["RepoTransfer"] = repoTransfer if ctx.Doer != nil { - ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer) + ctx.Data["CanUserAcceptOrRejectTransfer"] = repoTransfer.CanUserAcceptOrRejectTransfer(ctx, ctx.Doer) } } diff --git a/services/notify/notify.go b/services/notify/notify.go index 3b5f24340bb..c97d0fcbaf0 100644 --- a/services/notify/notify.go +++ b/services/notify/notify.go @@ -272,9 +272,9 @@ func MigrateRepository(ctx context.Context, doer, u *user_model.User, repo *repo } // TransferRepository notifies create repository to notifiers -func TransferRepository(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, newOwnerName string) { +func TransferRepository(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, oldOwnerName string) { for _, notifier := range notifiers { - notifier.TransferRepository(ctx, doer, repo, newOwnerName) + notifier.TransferRepository(ctx, doer, repo, oldOwnerName) } } diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 9ef28ddeb9f..bd3bf326b4f 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -27,19 +27,8 @@ func getRepoWorkingLockKey(repoID int64) string { return fmt.Sprintf("repo_working_%d", repoID) } -// TransferOwnership transfers all corresponding setting from old user to new one. -func TransferOwnership(ctx context.Context, doer, newOwner *user_model.User, repo *repo_model.Repository, teams []*organization.Team) error { - if err := repo.LoadOwner(ctx); err != nil { - return err - } - for _, team := range teams { - if newOwner.ID != team.OrgID { - return fmt.Errorf("team %d does not belong to organization", team.ID) - } - } - - oldOwner := repo.Owner - +// AcceptTransferOwnership transfers all corresponding setting from old user to new one. +func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error { releaser, err := globallock.Lock(ctx, getRepoWorkingLockKey(repo.ID)) if err != nil { log.Error("lock.Lock(): %v", err) @@ -47,29 +36,44 @@ func TransferOwnership(ctx context.Context, doer, newOwner *user_model.User, rep } defer releaser() - if err := transferOwnership(ctx, doer, newOwner.Name, repo); err != nil { - return err - } - releaser() - - newRepo, err := repo_model.GetRepositoryByID(ctx, repo.ID) + repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, repo) if err != nil { return err } - for _, team := range teams { - if err := addRepositoryToTeam(ctx, team, newRepo); err != nil { + oldOwnerName := repo.OwnerName + + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := repoTransfer.LoadAttributes(ctx); err != nil { + return err + } + + if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) { + return util.ErrPermissionDenied + } + + if err := repo.LoadOwner(ctx); err != nil { return err } + for _, team := range repoTransfer.Teams { + if repoTransfer.Recipient.ID != team.OrgID { + return fmt.Errorf("team %d does not belong to organization", team.ID) + } + } + + return transferOwnership(ctx, repoTransfer.Doer, repoTransfer.Recipient.Name, repo, repoTransfer.Teams) + }); err != nil { + return err } + releaser() - notify_service.TransferRepository(ctx, doer, repo, oldOwner.Name) + notify_service.TransferRepository(ctx, doer, repo, oldOwnerName) return nil } // transferOwnership transfers all corresponding repository items from old user to new one. -func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName string, repo *repo_model.Repository) (err error) { +func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName string, repo *repo_model.Repository, teams []*organization.Team) (err error) { repoRenamed := false wikiRenamed := false oldOwnerName := doer.Name @@ -138,7 +142,7 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName repo.OwnerName = newOwner.Name // Update repository. - if _, err := sess.ID(repo.ID).Update(repo); err != nil { + if err := repo_model.UpdateRepositoryCols(ctx, repo, "owner_id", "owner_name"); err != nil { return fmt.Errorf("update owner: %w", err) } @@ -174,15 +178,13 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName collaboration.UserID = 0 } - // Remove old team-repository relations. if oldOwner.IsOrganization() { + // Remove old team-repository relations. if err := organization.RemoveOrgRepo(ctx, oldOwner.ID, repo.ID); err != nil { return fmt.Errorf("removeOrgRepo: %w", err) } - } - // Remove project's issues that belong to old organization's projects - if oldOwner.IsOrganization() { + // Remove project's issues that belong to old organization's projects projects, err := project_model.GetAllProjectsIDsByOwnerIDAndType(ctx, oldOwner.ID, project_model.TypeOrganization) if err != nil { return fmt.Errorf("Unable to find old org projects: %w", err) @@ -225,15 +227,13 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName return fmt.Errorf("watchRepo: %w", err) } - // Remove watch for organization. if oldOwner.IsOrganization() { + // Remove watch for organization. if err := repo_model.WatchRepo(ctx, oldOwner, repo, false); err != nil { return fmt.Errorf("watchRepo [false]: %w", err) } - } - // Delete labels that belong to the old organization and comments that added these labels - if oldOwner.IsOrganization() { + // Delete labels that belong to the old organization and comments that added these labels if _, err := sess.Exec(`DELETE FROM issue_label WHERE issue_label.id IN ( SELECT il_too.id FROM ( SELECT il_too_too.id @@ -261,7 +261,6 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName // Rename remote repository to new path and delete local copy. dir := user_model.UserPath(newOwner.Name) - if err := os.MkdirAll(dir, os.ModePerm); err != nil { return fmt.Errorf("Failed to create dir %s: %w", dir, err) } @@ -273,7 +272,6 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName // Rename remote wiki repository to new path and delete local copy. wikiPath := repo_model.WikiPath(oldOwner.Name, repo.Name) - if isExist, err := util.IsExist(wikiPath); err != nil { log.Error("Unable to check if %s exists. Error: %v", wikiPath, err) return err @@ -301,6 +299,17 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName return fmt.Errorf("repo_model.NewRedirect: %w", err) } + newRepo, err := repo_model.GetRepositoryByID(ctx, repo.ID) + if err != nil { + return err + } + + for _, team := range teams { + if err := addRepositoryToTeam(ctx, team, newRepo); err != nil { + return err + } + } + return committer.Commit() } @@ -343,17 +352,9 @@ func changeRepositoryName(ctx context.Context, repo *repo_model.Repository, newR } } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := repo_model.NewRedirect(ctx, repo.Owner.ID, repo.ID, oldRepoName, newRepoName); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return repo_model.NewRedirect(ctx, repo.Owner.ID, repo.ID, oldRepoName, newRepoName) + }) } // ChangeRepositoryName changes all corresponding setting from old repository name to new one. @@ -387,70 +388,142 @@ func ChangeRepositoryName(ctx context.Context, doer *user_model.User, repo *repo // 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(ctx context.Context, doer, newOwner *user_model.User, repo *repo_model.Repository, teams []*organization.Team) error { + releaser, err := globallock.Lock(ctx, getRepoWorkingLockKey(repo.ID)) + if err != nil { + return fmt.Errorf("lock.Lock: %w", err) + } + defer releaser() + if err := repo_model.TestRepositoryReadyForTransfer(repo.Status); err != nil { return err } - // Admin is always allowed to transfer || user transfer repo back to his account - if doer.IsAdmin || doer.ID == newOwner.ID { - return TransferOwnership(ctx, doer, newOwner, repo, teams) - } + var isDirectTransfer bool + oldOwnerName := repo.OwnerName - if user_model.IsUserBlockedBy(ctx, doer, newOwner.ID) { - return user_model.ErrBlockedUser - } + if err := db.WithTx(ctx, func(ctx context.Context) error { + // Admin is always allowed to transfer || user transfer repo back to his account, + // then it will transfer directly without acceptance. + if doer.IsAdmin || doer.ID == newOwner.ID { + isDirectTransfer = true + return transferOwnership(ctx, doer, newOwner.Name, repo, teams) + } - // If new owner is an org and user can create repos he can transfer directly too - if newOwner.IsOrganization() { - allowed, err := organization.CanCreateOrgRepo(ctx, newOwner.ID, doer.ID) - if err != nil { - return err + if user_model.IsUserBlockedBy(ctx, doer, newOwner.ID) { + return user_model.ErrBlockedUser } - if allowed { - return TransferOwnership(ctx, 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 := organization.CanCreateOrgRepo(ctx, newOwner.ID, doer.ID) + if err != nil { + return err + } + if allowed { + isDirectTransfer = true + return transferOwnership(ctx, doer, newOwner.Name, repo, teams) + } } - } - // In case the new owner would not have sufficient access to the repo, give access rights for read - hasAccess, err := access_model.HasAnyUnitAccess(ctx, newOwner.ID, repo) - if err != nil { - return err - } - if !hasAccess { - if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil { + // In case the new owner would not have sufficient access to the repo, give access rights for read + hasAccess, err := access_model.HasAnyUnitAccess(ctx, newOwner.ID, repo) + if err != nil { return err } - } + if !hasAccess { + if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil { + return err + } + } - // Make repo as pending for transfer - repo.Status = repo_model.RepositoryPendingTransfer - if err := repo_model.CreatePendingRepositoryTransfer(ctx, doer, newOwner, repo.ID, teams); err != nil { + // Make repo as pending for transfer + repo.Status = repo_model.RepositoryPendingTransfer + return repo_model.CreatePendingRepositoryTransfer(ctx, doer, newOwner, repo.ID, teams) + }); err != nil { return err } - // notify users who are able to accept / reject transfer - notify_service.RepoPendingTransfer(ctx, doer, newOwner, repo) + if isDirectTransfer { + notify_service.TransferRepository(ctx, doer, repo, oldOwnerName) + } else { + // notify users who are able to accept / reject transfer + notify_service.RepoPendingTransfer(ctx, doer, newOwner, repo) + } return nil } -// CancelRepositoryTransfer marks the repository as ready and remove pending transfer entry, +// RejectRepositoryTransfer marks the repository as ready and remove pending transfer entry, // thus cancel the transfer process. -func CancelRepositoryTransfer(ctx context.Context, repo *repo_model.Repository) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err +// The accepter can reject the transfer. +func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error { + return db.WithTx(ctx, func(ctx context.Context) error { + repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, repo) + if err != nil { + return err + } + + if err := repoTransfer.LoadAttributes(ctx); err != nil { + return err + } + + if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) { + return util.ErrPermissionDenied + } + + repo.Status = repo_model.RepositoryReady + if err := repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil { + return err + } + + return repo_model.DeleteRepositoryTransfer(ctx, repo.ID) + }) +} + +func canUserCancelTransfer(ctx context.Context, r *repo_model.RepoTransfer, u *user_model.User) bool { + if u.IsAdmin || u.ID == r.DoerID { + return true } - defer committer.Close() - repo.Status = repo_model.RepositoryReady - if err := repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil { - return err + if err := r.LoadAttributes(ctx); err != nil { + log.Error("LoadAttributes: %v", err) + return false } - if err := repo_model.DeleteRepositoryTransfer(ctx, repo.ID); err != nil { - return err + if err := r.Repo.LoadOwner(ctx); err != nil { + log.Error("LoadOwner: %v", err) + return false } - return committer.Commit() + if !r.Repo.Owner.IsOrganization() { + return r.Repo.OwnerID == u.ID + } + + perm, err := access_model.GetUserRepoPermission(ctx, r.Repo, u) + if err != nil { + log.Error("GetUserRepoPermission: %v", err) + return false + } + return perm.IsOwner() +} + +// CancelRepositoryTransfer cancels the repository transfer process. The sender or +// the users who have admin permission of the original repository can cancel the transfer +func CancelRepositoryTransfer(ctx context.Context, repoTransfer *repo_model.RepoTransfer, doer *user_model.User) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if err := repoTransfer.LoadAttributes(ctx); err != nil { + return err + } + + if !canUserCancelTransfer(ctx, repoTransfer, doer) { + return util.ErrPermissionDenied + } + + repoTransfer.Repo.Status = repo_model.RepositoryReady + if err := repo_model.UpdateRepositoryCols(ctx, repoTransfer.Repo, "status"); err != nil { + return err + } + + return repo_model.DeleteRepositoryTransfer(ctx, repoTransfer.RepoID) + }) } diff --git a/services/repository/transfer_test.go b/services/repository/transfer_test.go index 91722fb8ae1..16a8fb6e1ee 100644 --- a/services/repository/transfer_test.go +++ b/services/repository/transfer_test.go @@ -34,23 +34,26 @@ func TestTransferOwnership(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - repo.Owner = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) - assert.NoError(t, TransferOwnership(db.DefaultContext, doer, doer, repo, nil)) + assert.NoError(t, repo.LoadOwner(db.DefaultContext)) + repoTransfer := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoTransfer{ID: 1}) + assert.NoError(t, repoTransfer.LoadAttributes(db.DefaultContext)) + assert.NoError(t, AcceptTransferOwnership(db.DefaultContext, repo, doer)) transferredRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - assert.EqualValues(t, 2, transferredRepo.OwnerID) + assert.EqualValues(t, 1, transferredRepo.OwnerID) // repo_transfer.yml id=1 + unittest.AssertNotExistsBean(t, &repo_model.RepoTransfer{ID: 1}) exist, err := util.IsExist(repo_model.RepoPath("org3", "repo3")) assert.NoError(t, err) assert.False(t, exist) - exist, err = util.IsExist(repo_model.RepoPath("user2", "repo3")) + exist, err = util.IsExist(repo_model.RepoPath("user1", "repo3")) assert.NoError(t, err) assert.True(t, exist) unittest.AssertExistsAndLoadBean(t, &activities_model.Action{ OpType: activities_model.ActionTransferRepo, - ActUserID: 2, + ActUserID: 1, RepoID: 3, Content: "org3/repo3", }) @@ -61,10 +64,10 @@ func TestTransferOwnership(t *testing.T) { func TestStartRepositoryTransferSetPermission(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) recipient := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - repo.Owner = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + assert.NoError(t, repo.LoadOwner(db.DefaultContext)) hasAccess, err := access_model.HasAnyUnitAccess(db.DefaultContext, recipient.ID, repo) assert.NoError(t, err) @@ -82,7 +85,7 @@ func TestStartRepositoryTransferSetPermission(t *testing.T) { func TestRepositoryTransfer(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) transfer, err := repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo) @@ -90,7 +93,7 @@ func TestRepositoryTransfer(t *testing.T) { assert.NotNil(t, transfer) // Cancel transfer - assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, repo)) + assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, transfer, doer)) transfer, err = repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo) assert.Error(t, err) @@ -113,10 +116,12 @@ func TestRepositoryTransfer(t *testing.T) { assert.Error(t, err) assert.True(t, repo_model.IsErrRepoTransferInProgress(err)) - // Unknown user - err = repo_model.CreatePendingRepositoryTransfer(db.DefaultContext, doer, &user_model.User{ID: 1000, LowerName: "user1000"}, repo.ID, nil) + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + // Unknown user, transfer non-existent transfer repo id = 2 + err = repo_model.CreatePendingRepositoryTransfer(db.DefaultContext, doer, &user_model.User{ID: 1000, LowerName: "user1000"}, repo2.ID, nil) assert.Error(t, err) - // Cancel transfer - assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, repo)) + // Reject transfer + err = RejectRepositoryTransfer(db.DefaultContext, repo2, doer) + assert.True(t, repo_model.IsErrNoPendingTransfer(err)) } diff --git a/services/user/block.go b/services/user/block.go index c24ce5273ca..7727780dfc9 100644 --- a/services/user/block.go +++ b/services/user/block.go @@ -117,10 +117,10 @@ func BlockUser(ctx context.Context, doer, blocker, blockee *user_model.User, not } // cancel each other repository transfers - if err := cancelRepositoryTransfers(ctx, blocker, blockee); err != nil { + if err := cancelRepositoryTransfers(ctx, doer, blocker, blockee); err != nil { return err } - if err := cancelRepositoryTransfers(ctx, blockee, blocker); err != nil { + if err := cancelRepositoryTransfers(ctx, doer, blockee, blocker); err != nil { return err } @@ -192,7 +192,7 @@ func unwatchRepos(ctx context.Context, watcher, repoOwner *user_model.User) erro } } -func cancelRepositoryTransfers(ctx context.Context, sender, recipient *user_model.User) error { +func cancelRepositoryTransfers(ctx context.Context, doer, sender, recipient *user_model.User) error { transfers, err := repo_model.GetPendingRepositoryTransfers(ctx, &repo_model.PendingRepositoryTransferOptions{ SenderID: sender.ID, RecipientID: recipient.ID, @@ -202,12 +202,7 @@ func cancelRepositoryTransfers(ctx context.Context, sender, recipient *user_mode } for _, transfer := range transfers { - repo, err := repo_model.GetRepositoryByID(ctx, transfer.RepoID) - if err != nil { - return err - } - - if err := repo_service.CancelRepositoryTransfer(ctx, repo); err != nil { + if err := repo_service.CancelRepositoryTransfer(ctx, transfer, doer); err != nil { return err } } diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 3e7214f48cf..8c2a0da8d03 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -39,16 +39,16 @@ {{if $.RepoTransfer}}
{{$.CsrfTokenHtml}} -
-
{{$.CsrfTokenHtml}} -
-