From 8ad87d215f2b6adb978de77e53ba2bf7ea571430 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 13 Sep 2023 20:11:32 +0200 Subject: [PATCH] [MODERATION] Add repo transfers to blocked functionality (squash) - When someone gets blocked, remove all pending repository transfers from the blocked user to the doer. - Do not allow to start transferring repositories to the doer as blocked user. - Added unit testing. - Added integration testing. (cherry picked from commit 8a3caac33013482ddbee2fa51510c6918ba54466) (cherry picked from commit a92b4cfeb63b90eb2d90d0feb51cec62e0502d84) (cherry picked from commit acaaaf07d999974dbe5f9c5e792621c597bfb542) (cherry picked from commit 735818863c1793aa6f6983afedc4bd3b36026ca5) (cherry picked from commit f50fa43b32160d0d88eca1dbdca09b5f575fb62b) (cherry picked from commit e16683643388fb3c60ea478f1419a6af4f4aa283) (cherry picked from commit 82a0e4a3814a66ce44be6a031bdf08484586c61b) (cherry picked from commit ff233c19c4a5edcc2b99a6f41a2d19dbe8c08b3b) --- models/fixtures/repository.yml | 2 +- models/repo_transfer.go | 10 ++++++++ models/repo_transfer_test.go | 13 ++++++++++ options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/transfer.go | 6 +++++ routers/web/repo/setting/setting.go | 5 +++- services/repository/transfer.go | 4 +++ services/repository/transfer_test.go | 2 +- services/user/block.go | 25 +++++++++++++++++++ services/user/block_test.go | 18 ++++++++++++++ tests/integration/block_test.go | 37 +++++++++++++++++++++------- 11 files changed, 111 insertions(+), 12 deletions(-) diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 382d7c14a..c74fb716b 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -83,7 +83,7 @@ is_empty: false is_archived: false is_mirror: false - status: 0 + status: 2 is_fork: false fork_id: 0 is_template: false diff --git a/models/repo_transfer.go b/models/repo_transfer.go index 630c243c8..69c531ff8 100644 --- a/models/repo_transfer.go +++ b/models/repo_transfer.go @@ -417,3 +417,13 @@ func TransferOwnership(ctx context.Context, doer *user_model.User, newOwnerName return committer.Commit() } + +// GetPendingTransfers returns the pending transfers of recipient which were sent by by doer. +func GetPendingTransferIDs(ctx context.Context, reciepientID, doerID int64) ([]int64, error) { + pendingTransferIDs := make([]int64, 0, 8) + return pendingTransferIDs, db.GetEngine(ctx).Table("repo_transfer"). + Where("doer_id = ?", doerID). + And("recipient_id = ?", reciepientID). + Cols("id"). + Find(&pendingTransferIDs) +} diff --git a/models/repo_transfer_test.go b/models/repo_transfer_test.go index b55cef947..41c6c214f 100644 --- a/models/repo_transfer_test.go +++ b/models/repo_transfer_test.go @@ -55,3 +55,16 @@ func TestRepositoryTransfer(t *testing.T) { // Cancel transfer assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, repo)) } + +func TestGetPendingTransferIDs(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + reciepient := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + pendingTransfer := unittest.AssertExistsAndLoadBean(t, &RepoTransfer{RecipientID: reciepient.ID, DoerID: doer.ID}) + + pendingTransferIDs, err := GetPendingTransferIDs(db.DefaultContext, reciepient.ID, doer.ID) + assert.NoError(t, err) + if assert.Len(t, pendingTransferIDs, 1) { + assert.EqualValues(t, pendingTransfer.ID, pendingTransferIDs[0]) + } +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1c5f99cdd..a990ad3a2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2081,6 +2081,7 @@ settings.reindex_requested=Reindex Requested settings.admin_enable_close_issues_via_commit_in_any_branch = Close an issue via a commit made in a non default branch settings.danger_zone = Danger Zone settings.new_owner_has_same_repo = The new owner already has a repository with same name. Please choose another name. +settings.new_owner_blocked_doer = The new owner has blocked you. settings.convert = Convert to Regular Repository settings.convert_desc = You can convert this mirror into a regular repository. This cannot be undone. settings.convert_notices_1 = This operation will convert the mirror into a regular repository and cannot be undone. diff --git a/routers/api/v1/repo/transfer.go b/routers/api/v1/repo/transfer.go index b3120f4be..cc6d08a8f 100644 --- a/routers/api/v1/repo/transfer.go +++ b/routers/api/v1/repo/transfer.go @@ -4,6 +4,7 @@ package repo import ( + "errors" "fmt" "net/http" @@ -107,6 +108,11 @@ func Transfer(ctx *context.APIContext) { oldFullname := ctx.Repo.Repository.FullName() if err := repo_service.StartRepositoryTransfer(ctx, ctx.Doer, newOwner, ctx.Repo.Repository, teams); err != nil { + if errors.Is(err, user_model.ErrBlockedByUser) { + ctx.Error(http.StatusForbidden, "StartRepositoryTransfer", err) + return + } + if models.IsErrRepoTransferInProgress(err) { ctx.Error(http.StatusConflict, "StartRepositoryTransfer", err) return diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 69cbcdbf8..a5404725c 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -5,6 +5,7 @@ package setting import ( + "errors" "fmt" "net/http" "strconv" @@ -775,7 +776,9 @@ func SettingsPost(ctx *context.Context) { } if err := repo_service.StartRepositoryTransfer(ctx, ctx.Doer, newOwner, repo, nil); err != nil { - if repo_model.IsErrRepoAlreadyExist(err) { + if errors.Is(err, user_model.ErrBlockedByUser) { + ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_blocked_doer"), tplSettingsOptions, nil) + } else if repo_model.IsErrRepoAlreadyExist(err) { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil) } else if models.IsErrRepoTransferInProgress(err) { ctx.RenderWithErr(ctx.Tr("repo.settings.transfer_in_progress"), tplSettingsOptions, nil) diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 574b6c6a5..f4f301994 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -85,6 +85,10 @@ 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 { + if user_model.IsBlocked(ctx, newOwner.ID, doer.ID) { + return user_model.ErrBlockedByUser + } + if err := models.TestRepositoryReadyForTransfer(repo.Status); err != nil { return err } diff --git a/services/repository/transfer_test.go b/services/repository/transfer_test.go index d55c76ea4..c2e38e3ef 100644 --- a/services/repository/transfer_test.go +++ b/services/repository/transfer_test.go @@ -63,7 +63,7 @@ func TestStartRepositoryTransferSetPermission(t *testing.T) { doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) recipient := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 5}) repo.Owner = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) hasAccess, err := access_model.HasAccess(db.DefaultContext, recipient.ID, repo) diff --git a/services/user/block.go b/services/user/block.go index 06cdd2717..0b31119df 100644 --- a/services/user/block.go +++ b/services/user/block.go @@ -5,9 +5,12 @@ package user import ( "context" + model "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + + "xorm.io/builder" ) // BlockUser adds a blocked user entry for userID to block blockID. @@ -66,5 +69,27 @@ func BlockUser(ctx context.Context, userID, blockID int64) error { return err } + // Remove pending repository transfers, and set the status on those repository + // back to ready. + pendingTransfersIDs, err := model.GetPendingTransferIDs(ctx, userID, blockID) + if err != nil { + return err + } + + // Use a subquery instead of a JOIN, because not every database supports JOIN + // on a UPDATE query. + _, err = db.GetEngine(ctx).Table("repository"). + In("id", builder.Select("repo_id").From("repo_transfer").Where(builder.In("id", pendingTransfersIDs))). + Cols("status"). + Update(&repo_model.Repository{Status: repo_model.RepositoryReady}) + if err != nil { + return err + } + + _, err = db.GetEngine(ctx).In("id", pendingTransfersIDs).Delete(&model.RepoTransfer{}) + if err != nil { + return err + } + return committer.Commit() } diff --git a/services/user/block_test.go b/services/user/block_test.go index 245dd959b..121c1ea8b 100644 --- a/services/user/block_test.go +++ b/services/user/block_test.go @@ -6,6 +6,7 @@ package user import ( "testing" + model "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -70,4 +71,21 @@ func TestBlockUser(t *testing.T) { assert.False(t, isBlockedUserCollab(repo1)) assert.False(t, isBlockedUserCollab(repo2)) }) + + t.Run("Pending transfers", func(t *testing.T) { + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + defer user_model.UnblockUser(db.DefaultContext, doer.ID, blockedUser.ID) + + unittest.AssertExistsIf(t, true, &repo_model.Repository{ID: 3, OwnerID: blockedUser.ID, Status: repo_model.RepositoryPendingTransfer}) + unittest.AssertExistsIf(t, true, &model.RepoTransfer{ID: 1, RecipientID: doer.ID, DoerID: blockedUser.ID}) + + assert.NoError(t, BlockUser(db.DefaultContext, doer.ID, blockedUser.ID)) + + unittest.AssertExistsIf(t, false, &model.RepoTransfer{ID: 1, RecipientID: doer.ID, DoerID: blockedUser.ID}) + + // Don't use AssertExistsIf, as it doesn't include the zero values in the condition such as `repo_model.RepositoryReady`. + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3, OwnerID: blockedUser.ID}) + assert.Equal(t, repo_model.RepositoryReady, repo.Status) + }) } diff --git a/tests/integration/block_test.go b/tests/integration/block_test.go index fee6d4b6f..cb1142014 100644 --- a/tests/integration/block_test.go +++ b/tests/integration/block_test.go @@ -162,7 +162,9 @@ func TestBlockActions(t *testing.T) { doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + blockedUser2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10}) repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: doer.ID}) + repo7 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: blockedUser2.ID}) issue4 := unittest.AssertExistsAndLoadBean(t, &issue_model.Issue{ID: 4, RepoID: repo2.ID}) issue4URL := fmt.Sprintf("/%s/issues/%d", repo2.FullName(), issue4.Index) // NOTE: Sessions shouldn't be shared, because in some situations flash @@ -170,6 +172,7 @@ func TestBlockActions(t *testing.T) { // results. BlockUser(t, doer, blockedUser) + BlockUser(t, doer, blockedUser2) // Ensures that issue creation on doer's ownen repositories are blocked. t.Run("Issue creation", func(t *testing.T) { @@ -326,10 +329,6 @@ func TestBlockActions(t *testing.T) { // Ensures that the doer and blocked user cannot add each each other as collaborators. t.Run("Add collaborator", func(t *testing.T) { - blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10}) - - BlockUser(t, doer, blockedUser) - t.Run("Doer Add BlockedUser", func(t *testing.T) { defer tests.PrintCurrentTest(t)() @@ -338,7 +337,7 @@ func TestBlockActions(t *testing.T) { req := NewRequestWithValues(t, "POST", link, map[string]string{ "_csrf": GetCSRF(t, session, link), - "collaborator": blockedUser.Name, + "collaborator": blockedUser2.Name, }) session.MakeRequest(t, req, http.StatusSeeOther) @@ -350,10 +349,8 @@ func TestBlockActions(t *testing.T) { t.Run("BlockedUser Add doer", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: blockedUser.ID}) - - session := loginUser(t, blockedUser.Name) - link := fmt.Sprintf("/%s/settings/collaboration", repo.FullName()) + session := loginUser(t, blockedUser2.Name) + link := fmt.Sprintf("/%s/settings/collaboration", repo7.FullName()) req := NewRequestWithValues(t, "POST", link, map[string]string{ "_csrf": GetCSRF(t, session, link), @@ -366,4 +363,26 @@ func TestBlockActions(t *testing.T) { assert.EqualValues(t, "error%3DCannot%2Badd%2Bthe%2Bcollaborator%252C%2Bbecause%2Bthey%2Bhave%2Bblocked%2Bthe%2Brepository%2Bowner.", flashCookie.Value) }) }) + + // Ensures that the blocked user cannot transfer a repository to the doer. + t.Run("Repository transfer", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, blockedUser2.Name) + link := fmt.Sprintf("%s/settings", repo7.FullName()) + + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": GetCSRF(t, session, link), + "action": "transfer", + "repo_name": repo7.Name, + "new_owner_name": doer.Name, + }) + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + assert.Contains(t, + htmlDoc.doc.Find(".ui.negative.message").Text(), + translation.NewLocale("en-US").Tr("repo.settings.new_owner_blocked_doer"), + ) + }) }