From a92b4cfeb63b90eb2d90d0feb51cec62e0502d84 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
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)
---
 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 7d2ee661c3..55bbb450ef 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 630c243c8e..69c531ff8c 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 b55cef9473..41c6c214f9 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 5d6391fcc0..65e2a047c9 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -2069,6 +2069,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 326895918e..bf0f5b1a14 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 68943586ef..3fb54d5474 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 574b6c6a56..f4f301994d 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 d55c76ea47..c2e38e3ef1 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 06cdd27176..0b31119dfb 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 245dd959b9..121c1ea8b7 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 fee6d4b6f9..cb11420140 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"),
+		)
+	})
 }