[MODERATION] Block adding collaborators
- Ensure that the doer and blocked user cannot add each other as collaborators to repositories. - The Web UI gets an detailed message of the specific situation, the API gets an generic Forbidden code. - Unit tests has been added. - Integration testing for Web and API has been added. - This commit doesn't introduce removing each other as collaborators on the block action, due to the complexity of database calls that needs to be figured out. That deserves its own commit and test code.
This commit is contained in:
parent
cfa539e590
commit
747be949a1
10 changed files with 151 additions and 6 deletions
|
@ -14,6 +14,10 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User) error {
|
func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User) error {
|
||||||
|
if user_model.IsBlocked(ctx, repo.OwnerID, u.ID) || user_model.IsBlocked(ctx, u.ID, repo.OwnerID) {
|
||||||
|
return user_model.ErrBlockedByUser
|
||||||
|
}
|
||||||
|
|
||||||
return db.WithTx(ctx, func(ctx context.Context) error {
|
return db.WithTx(ctx, func(ctx context.Context) error {
|
||||||
collaboration := &repo_model.Collaboration{
|
collaboration := &repo_model.Collaboration{
|
||||||
RepoID: repo.ID,
|
RepoID: repo.ID,
|
||||||
|
|
|
@ -33,6 +33,33 @@ func TestRepository_AddCollaborator(t *testing.T) {
|
||||||
testSuccess(3, 4)
|
testSuccess(3, 4)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRepository_AddCollaborator_IsBlocked(t *testing.T) {
|
||||||
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||||
|
|
||||||
|
testSuccess := func(repoID, userID int64) {
|
||||||
|
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID})
|
||||||
|
assert.NoError(t, repo.LoadOwner(db.DefaultContext))
|
||||||
|
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID})
|
||||||
|
|
||||||
|
// Owner blocked user.
|
||||||
|
unittest.AssertSuccessfulInsert(t, &user_model.BlockedUser{UserID: repo.OwnerID, BlockID: userID})
|
||||||
|
assert.ErrorIs(t, AddCollaborator(db.DefaultContext, repo, user), user_model.ErrBlockedByUser)
|
||||||
|
unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repoID}, &user_model.User{ID: userID})
|
||||||
|
_, err := db.DeleteByBean(db.DefaultContext, &user_model.BlockedUser{UserID: repo.OwnerID, BlockID: userID})
|
||||||
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
// User has owner blocked.
|
||||||
|
unittest.AssertSuccessfulInsert(t, &user_model.BlockedUser{UserID: userID, BlockID: repo.OwnerID})
|
||||||
|
assert.ErrorIs(t, AddCollaborator(db.DefaultContext, repo, user), user_model.ErrBlockedByUser)
|
||||||
|
unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repoID}, &user_model.User{ID: userID})
|
||||||
|
}
|
||||||
|
// Ensure idempotency (public repository).
|
||||||
|
testSuccess(1, 4)
|
||||||
|
testSuccess(1, 4)
|
||||||
|
// Add collaborator to private repository.
|
||||||
|
testSuccess(3, 4)
|
||||||
|
}
|
||||||
|
|
||||||
func TestRepoPermissionPublicNonOrgRepo(t *testing.T) {
|
func TestRepoPermissionPublicNonOrgRepo(t *testing.T) {
|
||||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||||
|
|
||||||
|
|
|
@ -607,6 +607,7 @@ block_user = Block User
|
||||||
block_user.detail = Please understand that if you block this user, other actions will be taken. Such as:
|
block_user.detail = Please understand that if you block this user, other actions will be taken. Such as:
|
||||||
block_user.detail_1 = You are being unfollowed from this user.
|
block_user.detail_1 = You are being unfollowed from this user.
|
||||||
block_user.detail_2 = This user cannot interact with your repositories, created issues and comments.
|
block_user.detail_2 = This user cannot interact with your repositories, created issues and comments.
|
||||||
|
block_user.detail_3 = This user cannot add you as a collaborator, nor can you add them as a collaborator.
|
||||||
follow_blocked_user = You cannot follow this user because you have blocked this user or this user has blocked you.
|
follow_blocked_user = You cannot follow this user because you have blocked this user or this user has blocked you.
|
||||||
|
|
||||||
form.name_reserved = The username "%s" is reserved.
|
form.name_reserved = The username "%s" is reserved.
|
||||||
|
@ -2080,6 +2081,8 @@ settings.add_collaborator_success = The collaborator has been added.
|
||||||
settings.add_collaborator_inactive_user = Cannot add an inactive user as a collaborator.
|
settings.add_collaborator_inactive_user = Cannot add an inactive user as a collaborator.
|
||||||
settings.add_collaborator_owner = Cannot add an owner as a collaborator.
|
settings.add_collaborator_owner = Cannot add an owner as a collaborator.
|
||||||
settings.add_collaborator_duplicate = The collaborator is already added to this repository.
|
settings.add_collaborator_duplicate = The collaborator is already added to this repository.
|
||||||
|
settings.add_collaborator_blocked_our = Cannot add the collaborator, because the repository owner has blocked them.
|
||||||
|
settings.add_collaborator_blocked_them = Cannot add the collaborator, because they have blocked the repository owner.
|
||||||
settings.delete_collaborator = Remove
|
settings.delete_collaborator = Remove
|
||||||
settings.collaborator_deletion = Remove Collaborator
|
settings.collaborator_deletion = Remove Collaborator
|
||||||
settings.collaborator_deletion_desc = Removing a collaborator will revoke their access to this repository. Continue?
|
settings.collaborator_deletion_desc = Removing a collaborator will revoke their access to this repository. Continue?
|
||||||
|
|
|
@ -156,6 +156,8 @@ func AddCollaborator(ctx *context.APIContext) {
|
||||||
// "$ref": "#/responses/empty"
|
// "$ref": "#/responses/empty"
|
||||||
// "422":
|
// "422":
|
||||||
// "$ref": "#/responses/validationError"
|
// "$ref": "#/responses/validationError"
|
||||||
|
// "403":
|
||||||
|
// "$ref": "#/responses/forbidden"
|
||||||
|
|
||||||
form := web.GetForm(ctx).(*api.AddCollaboratorOption)
|
form := web.GetForm(ctx).(*api.AddCollaboratorOption)
|
||||||
|
|
||||||
|
@ -175,7 +177,11 @@ func AddCollaborator(ctx *context.APIContext) {
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := repo_module.AddCollaborator(ctx, ctx.Repo.Repository, collaborator); err != nil {
|
if err := repo_module.AddCollaborator(ctx, ctx.Repo.Repository, collaborator); err != nil {
|
||||||
ctx.Error(http.StatusInternalServerError, "AddCollaborator", err)
|
if errors.Is(err, user_model.ErrBlockedByUser) {
|
||||||
|
ctx.Error(http.StatusForbidden, "AddCollaborator", err)
|
||||||
|
} else {
|
||||||
|
ctx.Error(http.StatusInternalServerError, "AddCollaborator", err)
|
||||||
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -4,6 +4,7 @@
|
||||||
package setting
|
package setting
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
@ -102,7 +103,18 @@ func CollaborationPost(ctx *context.Context) {
|
||||||
}
|
}
|
||||||
|
|
||||||
if err = repo_module.AddCollaborator(ctx, ctx.Repo.Repository, u); err != nil {
|
if err = repo_module.AddCollaborator(ctx, ctx.Repo.Repository, u); err != nil {
|
||||||
ctx.ServerError("AddCollaborator", err)
|
if !errors.Is(err, user_model.ErrBlockedByUser) {
|
||||||
|
ctx.ServerError("AddCollaborator", err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// To give an good error message, be precise on who has blocked who.
|
||||||
|
if blockedOurs := user_model.IsBlocked(ctx, ctx.Repo.Repository.OwnerID, u.ID); blockedOurs {
|
||||||
|
ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_blocked_our"))
|
||||||
|
} else {
|
||||||
|
ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_blocked_them"))
|
||||||
|
}
|
||||||
|
ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -102,13 +102,15 @@ func TestCollaborationPost(t *testing.T) {
|
||||||
ctx.Req.Form.Set("collaborator", "user4")
|
ctx.Req.Form.Set("collaborator", "user4")
|
||||||
|
|
||||||
u := &user_model.User{
|
u := &user_model.User{
|
||||||
|
ID: 2,
|
||||||
LowerName: "user2",
|
LowerName: "user2",
|
||||||
Type: user_model.UserTypeIndividual,
|
Type: user_model.UserTypeIndividual,
|
||||||
}
|
}
|
||||||
|
|
||||||
re := &repo_model.Repository{
|
re := &repo_model.Repository{
|
||||||
ID: 2,
|
ID: 2,
|
||||||
Owner: u,
|
Owner: u,
|
||||||
|
OwnerID: u.ID,
|
||||||
}
|
}
|
||||||
|
|
||||||
repo := &context.Repository{
|
repo := &context.Repository{
|
||||||
|
@ -160,13 +162,15 @@ func TestCollaborationPost_AddCollaboratorTwice(t *testing.T) {
|
||||||
ctx.Req.Form.Set("collaborator", "user4")
|
ctx.Req.Form.Set("collaborator", "user4")
|
||||||
|
|
||||||
u := &user_model.User{
|
u := &user_model.User{
|
||||||
|
ID: 2,
|
||||||
LowerName: "user2",
|
LowerName: "user2",
|
||||||
Type: user_model.UserTypeIndividual,
|
Type: user_model.UserTypeIndividual,
|
||||||
}
|
}
|
||||||
|
|
||||||
re := &repo_model.Repository{
|
re := &repo_model.Repository{
|
||||||
ID: 2,
|
ID: 2,
|
||||||
Owner: u,
|
Owner: u,
|
||||||
|
OwnerID: u.ID,
|
||||||
}
|
}
|
||||||
|
|
||||||
repo := &context.Repository{
|
repo := &context.Repository{
|
||||||
|
|
3
templates/swagger/v1_json.tmpl
generated
3
templates/swagger/v1_json.tmpl
generated
|
@ -3948,6 +3948,9 @@
|
||||||
"204": {
|
"204": {
|
||||||
"$ref": "#/responses/empty"
|
"$ref": "#/responses/empty"
|
||||||
},
|
},
|
||||||
|
"403": {
|
||||||
|
"$ref": "#/responses/forbidden"
|
||||||
|
},
|
||||||
"422": {
|
"422": {
|
||||||
"$ref": "#/responses/validationError"
|
"$ref": "#/responses/validationError"
|
||||||
}
|
}
|
||||||
|
|
|
@ -52,6 +52,7 @@
|
||||||
<ul>
|
<ul>
|
||||||
<li>{{$.locale.Tr "user.block_user.detail_1"}}</li>
|
<li>{{$.locale.Tr "user.block_user.detail_1"}}</li>
|
||||||
<li>{{$.locale.Tr "user.block_user.detail_2"}}</li>
|
<li>{{$.locale.Tr "user.block_user.detail_2"}}</li>
|
||||||
|
<li>{{$.locale.Tr "user.block_user.detail_3"}}</li>
|
||||||
</ul>
|
</ul>
|
||||||
</div>
|
</div>
|
||||||
{{template "base/modal_actions_confirm" .}}
|
{{template "base/modal_actions_confirm" .}}
|
||||||
|
|
|
@ -9,6 +9,7 @@ import (
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
auth_model "code.gitea.io/gitea/models/auth"
|
auth_model "code.gitea.io/gitea/models/auth"
|
||||||
|
repo_model "code.gitea.io/gitea/models/repo"
|
||||||
"code.gitea.io/gitea/models/unittest"
|
"code.gitea.io/gitea/models/unittest"
|
||||||
user_model "code.gitea.io/gitea/models/user"
|
user_model "code.gitea.io/gitea/models/user"
|
||||||
api "code.gitea.io/gitea/modules/structs"
|
api "code.gitea.io/gitea/modules/structs"
|
||||||
|
@ -99,3 +100,42 @@ func TestAPIOrgBlock(t *testing.T) {
|
||||||
unittest.AssertNotExistsBean(t, &user_model.BlockedUser{UserID: 6, BlockID: 2})
|
unittest.AssertNotExistsBean(t, &user_model.BlockedUser{UserID: 6, BlockID: 2})
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestAPIBlock_AddCollaborator ensures that the doer and blocked user cannot
|
||||||
|
// add each others as collaborators via the API.
|
||||||
|
func TestAPIBlock_AddCollaborator(t *testing.T) {
|
||||||
|
defer tests.PrepareTestEnv(t)()
|
||||||
|
|
||||||
|
user1 := "user10"
|
||||||
|
user2 := "user2"
|
||||||
|
perm := "write"
|
||||||
|
collabOption := &api.AddCollaboratorOption{Permission: &perm}
|
||||||
|
|
||||||
|
// User1 blocks User2.
|
||||||
|
session := loginUser(t, user1)
|
||||||
|
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository)
|
||||||
|
|
||||||
|
req := NewRequest(t, "PUT", fmt.Sprintf("/api/v1/user/block/%s?token=%s", user2, token))
|
||||||
|
MakeRequest(t, req, http.StatusNoContent)
|
||||||
|
unittest.AssertExistsAndLoadBean(t, &user_model.BlockedUser{UserID: 10, BlockID: 2})
|
||||||
|
|
||||||
|
t.Run("BlockedUser Add Doer", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: 2})
|
||||||
|
session := loginUser(t, user2)
|
||||||
|
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||||
|
|
||||||
|
req := NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/collaborators/%s?token=%s", user2, repo.Name, user1, token), collabOption)
|
||||||
|
session.MakeRequest(t, req, http.StatusForbidden)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Doer Add BlockedUser", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: 10})
|
||||||
|
session := loginUser(t, user1)
|
||||||
|
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||||
|
|
||||||
|
req := NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/collaborators/%s?token=%s", user1, repo.Name, user2, token), collabOption)
|
||||||
|
session.MakeRequest(t, req, http.StatusForbidden)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
|
@ -16,6 +16,7 @@ import (
|
||||||
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"
|
||||||
user_model "code.gitea.io/gitea/models/user"
|
user_model "code.gitea.io/gitea/models/user"
|
||||||
|
forgejo_context "code.gitea.io/gitea/modules/context"
|
||||||
"code.gitea.io/gitea/modules/translation"
|
"code.gitea.io/gitea/modules/translation"
|
||||||
"code.gitea.io/gitea/tests"
|
"code.gitea.io/gitea/tests"
|
||||||
|
|
||||||
|
@ -210,3 +211,47 @@ func TestBlockUserFromOrganization(t *testing.T) {
|
||||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||||
unittest.AssertNotExistsBean(t, &user_model.BlockedUser{BlockID: blockedUser.ID, UserID: org.ID})
|
unittest.AssertNotExistsBean(t, &user_model.BlockedUser{BlockID: blockedUser.ID, UserID: org.ID})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestBlockAddCollaborator ensures that the doer and blocked user cannot add each each other as collaborators.
|
||||||
|
func TestBlockAddCollaborator(t *testing.T) {
|
||||||
|
defer tests.PrepareTestEnv(t)()
|
||||||
|
|
||||||
|
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10})
|
||||||
|
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||||
|
|
||||||
|
BlockUser(t, user1, user2)
|
||||||
|
|
||||||
|
t.Run("BlockedUser Add Doer", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
|
||||||
|
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: user2.ID})
|
||||||
|
|
||||||
|
session := loginUser(t, user2.Name)
|
||||||
|
req := NewRequestWithValues(t, "POST", path.Join(repo.Link(), "/settings/collaboration"), map[string]string{
|
||||||
|
"_csrf": GetCSRF(t, session, path.Join(repo.Link(), "/settings/collaboration")),
|
||||||
|
"collaborator": user1.Name,
|
||||||
|
})
|
||||||
|
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||||
|
|
||||||
|
flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
|
||||||
|
assert.NotNil(t, flashCookie)
|
||||||
|
assert.EqualValues(t, "error%3DCannot%2Badd%2Bthe%2Bcollaborator%252C%2Bbecause%2Bthey%2Bhave%2Bblocked%2Bthe%2Brepository%2Bowner.", flashCookie.Value)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Doer Add BlockedUser", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
|
||||||
|
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 7, OwnerID: user1.ID})
|
||||||
|
|
||||||
|
session := loginUser(t, user1.Name)
|
||||||
|
req := NewRequestWithValues(t, "POST", path.Join(repo.Link(), "/settings/collaboration"), map[string]string{
|
||||||
|
"_csrf": GetCSRF(t, session, path.Join(repo.Link(), "/settings/collaboration")),
|
||||||
|
"collaborator": user2.Name,
|
||||||
|
})
|
||||||
|
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||||
|
|
||||||
|
flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
|
||||||
|
assert.NotNil(t, flashCookie)
|
||||||
|
assert.EqualValues(t, "error%3DCannot%2Badd%2Bthe%2Bcollaborator%252C%2Bbecause%2Bthe%2Brepository%2Bowner%2Bhas%2Bblocked%2Bthem.", flashCookie.Value)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue