mirror of
				https://codeberg.org/forgejo/forgejo.git
				synced 2025-10-31 06:21:11 +00:00 
			
		
		
		
	[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 commit8a3caac330) (cherry picked from commita92b4cfeb6) (cherry picked from commitacaaaf07d9) (cherry picked from commit735818863c) (cherry picked from commitf50fa43b32) (cherry picked from commite166836433) (cherry picked from commite0187b21fe) (cherry picked from commit697a492686)
This commit is contained in:
		
					parent
					
						
							
								3c6e5fca84
							
						
					
				
			
			
				commit
				
					
						beb4a8a452
					
				
			
		
					 11 changed files with 111 additions and 12 deletions
				
			
		|  | @ -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 | ||||
|  |  | |||
|  | @ -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) | ||||
| } | ||||
|  |  | |||
|  | @ -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]) | ||||
| 	} | ||||
| } | ||||
|  |  | |||
|  | @ -2072,6 +2072,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. | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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 | ||||
| 	} | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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() | ||||
| } | ||||
|  |  | |||
|  | @ -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) | ||||
| 	}) | ||||
| } | ||||
|  |  | |||
|  | @ -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"), | ||||
| 		) | ||||
| 	}) | ||||
| } | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue