[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 commit82a0e4a381
)
This commit is contained in:
parent
32893dbab1
commit
ff233c19c4
11 changed files with 111 additions and 12 deletions
|
@ -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
|
||||||
|
|
|
@ -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)
|
||||||
|
}
|
||||||
|
|
|
@ -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])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -2079,6 +2079,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.
|
||||||
|
|
|
@ -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
|
||||||
|
|
|
@ -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)
|
||||||
|
|
|
@ -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
|
||||||
}
|
}
|
||||||
|
|
|
@ -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)
|
||||||
|
|
|
@ -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()
|
||||||
}
|
}
|
||||||
|
|
|
@ -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)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
|
@ -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"),
|
||||||
|
)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue