[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 8a3caac330)
(cherry picked from commit a92b4cfeb6)
(cherry picked from commit acaaaf07d9)
(cherry picked from commit 735818863c)
(cherry picked from commit f50fa43b32)
This commit is contained in:
Gusted 2023-09-13 20:11:32 +02:00 committed by Earl Warren
parent 634c5604d4
commit e166836433
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
11 changed files with 111 additions and 12 deletions

View file

@ -83,7 +83,7 @@
is_empty: false is_empty: false
is_archived: false is_archived: false
is_mirror: false is_mirror: false
status: 0 status: 2
is_fork: false is_fork: false
fork_id: 0 fork_id: 0
is_template: false is_template: false

View file

@ -417,3 +417,13 @@ func TransferOwnership(ctx context.Context, doer *user_model.User, newOwnerName
return committer.Commit() 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)
}

View file

@ -55,3 +55,16 @@ func TestRepositoryTransfer(t *testing.T) {
// Cancel transfer // Cancel transfer
assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, repo)) 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])
}
}

View file

@ -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.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.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_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 = Convert to Regular Repository
settings.convert_desc = You can convert this mirror into a regular repository. This cannot be undone. 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. settings.convert_notices_1 = This operation will convert the mirror into a regular repository and cannot be undone.

View file

@ -4,6 +4,7 @@
package repo package repo
import ( import (
"errors"
"fmt" "fmt"
"net/http" "net/http"
@ -107,6 +108,11 @@ func Transfer(ctx *context.APIContext) {
oldFullname := ctx.Repo.Repository.FullName() oldFullname := ctx.Repo.Repository.FullName()
if err := repo_service.StartRepositoryTransfer(ctx, ctx.Doer, newOwner, ctx.Repo.Repository, teams); err != nil { 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) { if models.IsErrRepoTransferInProgress(err) {
ctx.Error(http.StatusConflict, "StartRepositoryTransfer", err) ctx.Error(http.StatusConflict, "StartRepositoryTransfer", err)
return return

View file

@ -5,6 +5,7 @@
package setting package setting
import ( import (
"errors"
"fmt" "fmt"
"net/http" "net/http"
"strconv" "strconv"
@ -775,7 +776,9 @@ func SettingsPost(ctx *context.Context) {
} }
if err := repo_service.StartRepositoryTransfer(ctx, ctx.Doer, newOwner, repo, nil); err != nil { 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) ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil)
} else if models.IsErrRepoTransferInProgress(err) { } else if models.IsErrRepoTransferInProgress(err) {
ctx.RenderWithErr(ctx.Tr("repo.settings.transfer_in_progress"), tplSettingsOptions, nil) ctx.RenderWithErr(ctx.Tr("repo.settings.transfer_in_progress"), tplSettingsOptions, nil)

View file

@ -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. // 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. // 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 { 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 { if err := models.TestRepositoryReadyForTransfer(repo.Status); err != nil {
return err return err
} }

View file

@ -63,7 +63,7 @@ func TestStartRepositoryTransferSetPermission(t *testing.T) {
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
recipient := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) 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}) repo.Owner = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
hasAccess, err := access_model.HasAccess(db.DefaultContext, recipient.ID, repo) hasAccess, err := access_model.HasAccess(db.DefaultContext, recipient.ID, repo)

View file

@ -5,9 +5,12 @@ package user
import ( import (
"context" "context"
model "code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"xorm.io/builder"
) )
// BlockUser adds a blocked user entry for userID to block blockID. // 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 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() return committer.Commit()
} }

View file

@ -6,6 +6,7 @@ package user
import ( import (
"testing" "testing"
model "code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
@ -70,4 +71,21 @@ func TestBlockUser(t *testing.T) {
assert.False(t, isBlockedUserCollab(repo1)) assert.False(t, isBlockedUserCollab(repo1))
assert.False(t, isBlockedUserCollab(repo2)) 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)
})
} }

View file

@ -162,7 +162,9 @@ func TestBlockActions(t *testing.T) {
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) 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}) 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}) issue4 := unittest.AssertExistsAndLoadBean(t, &issue_model.Issue{ID: 4, RepoID: repo2.ID})
issue4URL := fmt.Sprintf("/%s/issues/%d", repo2.FullName(), issue4.Index) issue4URL := fmt.Sprintf("/%s/issues/%d", repo2.FullName(), issue4.Index)
// NOTE: Sessions shouldn't be shared, because in some situations flash // NOTE: Sessions shouldn't be shared, because in some situations flash
@ -170,6 +172,7 @@ func TestBlockActions(t *testing.T) {
// results. // results.
BlockUser(t, doer, blockedUser) BlockUser(t, doer, blockedUser)
BlockUser(t, doer, blockedUser2)
// Ensures that issue creation on doer's ownen repositories are blocked. // Ensures that issue creation on doer's ownen repositories are blocked.
t.Run("Issue creation", func(t *testing.T) { 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. // Ensures that the doer and blocked user cannot add each each other as collaborators.
t.Run("Add collaborator", func(t *testing.T) { 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) { t.Run("Doer Add BlockedUser", func(t *testing.T) {
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()
@ -338,7 +337,7 @@ func TestBlockActions(t *testing.T) {
req := NewRequestWithValues(t, "POST", link, map[string]string{ req := NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": GetCSRF(t, session, link), "_csrf": GetCSRF(t, session, link),
"collaborator": blockedUser.Name, "collaborator": blockedUser2.Name,
}) })
session.MakeRequest(t, req, http.StatusSeeOther) session.MakeRequest(t, req, http.StatusSeeOther)
@ -350,10 +349,8 @@ func TestBlockActions(t *testing.T) {
t.Run("BlockedUser Add doer", func(t *testing.T) { t.Run("BlockedUser Add doer", func(t *testing.T) {
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: blockedUser.ID}) session := loginUser(t, blockedUser2.Name)
link := fmt.Sprintf("/%s/settings/collaboration", repo7.FullName())
session := loginUser(t, blockedUser.Name)
link := fmt.Sprintf("/%s/settings/collaboration", repo.FullName())
req := NewRequestWithValues(t, "POST", link, map[string]string{ req := NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": GetCSRF(t, session, link), "_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) 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"),
)
})
} }