diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go index f5cdc35045..4099a178f2 100644 --- a/modules/repository/collaborator.go +++ b/modules/repository/collaborator.go @@ -14,6 +14,10 @@ import ( ) 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 { collaboration := &repo_model.Collaboration{ RepoID: repo.ID, diff --git a/modules/repository/collaborator_test.go b/modules/repository/collaborator_test.go index c772806819..b051aa2ac7 100644 --- a/modules/repository/collaborator_test.go +++ b/modules/repository/collaborator_test.go @@ -33,6 +33,33 @@ func TestRepository_AddCollaborator(t *testing.T) { 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) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 997169632a..6bc9b5b7d3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -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_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_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. 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_owner = Cannot add an owner as a collaborator. 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.collaborator_deletion = Remove Collaborator settings.collaborator_deletion_desc = Removing a collaborator will revoke their access to this repository. Continue? diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index 942d4c799f..cfa45e9995 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -156,6 +156,8 @@ func AddCollaborator(ctx *context.APIContext) { // "$ref": "#/responses/empty" // "422": // "$ref": "#/responses/validationError" + // "403": + // "$ref": "#/responses/forbidden" 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 { - 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 } diff --git a/routers/web/repo/setting/collaboration.go b/routers/web/repo/setting/collaboration.go index 8f2d306862..a9dc47693d 100644 --- a/routers/web/repo/setting/collaboration.go +++ b/routers/web/repo/setting/collaboration.go @@ -4,6 +4,7 @@ package setting import ( + "errors" "net/http" "strings" @@ -102,7 +103,18 @@ func CollaborationPost(ctx *context.Context) { } 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 } diff --git a/routers/web/repo/setting/settings_test.go b/routers/web/repo/setting/settings_test.go index 6f7c844ce7..46180ac372 100644 --- a/routers/web/repo/setting/settings_test.go +++ b/routers/web/repo/setting/settings_test.go @@ -102,13 +102,15 @@ func TestCollaborationPost(t *testing.T) { ctx.Req.Form.Set("collaborator", "user4") u := &user_model.User{ + ID: 2, LowerName: "user2", Type: user_model.UserTypeIndividual, } re := &repo_model.Repository{ - ID: 2, - Owner: u, + ID: 2, + Owner: u, + OwnerID: u.ID, } repo := &context.Repository{ @@ -160,13 +162,15 @@ func TestCollaborationPost_AddCollaboratorTwice(t *testing.T) { ctx.Req.Form.Set("collaborator", "user4") u := &user_model.User{ + ID: 2, LowerName: "user2", Type: user_model.UserTypeIndividual, } re := &repo_model.Repository{ - ID: 2, - Owner: u, + ID: 2, + Owner: u, + OwnerID: u.ID, } repo := &context.Repository{ diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6f27cd9c76..eefe3b2c3d 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3948,6 +3948,9 @@ "204": { "$ref": "#/responses/empty" }, + "403": { + "$ref": "#/responses/forbidden" + }, "422": { "$ref": "#/responses/validationError" } diff --git a/templates/user/profile.tmpl b/templates/user/profile.tmpl index 6a2f719a34..05e95a1a34 100644 --- a/templates/user/profile.tmpl +++ b/templates/user/profile.tmpl @@ -52,6 +52,7 @@ {{template "base/modal_actions_confirm" .}} diff --git a/tests/integration/api_block_test.go b/tests/integration/api_block_test.go index 2118080c7a..b07dd2455a 100644 --- a/tests/integration/api_block_test.go +++ b/tests/integration/api_block_test.go @@ -9,6 +9,7 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" 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}) }) } + +// 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) + }) +} diff --git a/tests/integration/block_test.go b/tests/integration/block_test.go index b8001f4968..be1097f0f8 100644 --- a/tests/integration/block_test.go +++ b/tests/integration/block_test.go @@ -16,6 +16,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" 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/tests" @@ -210,3 +211,47 @@ func TestBlockUserFromOrganization(t *testing.T) { session.MakeRequest(t, req, http.StatusSeeOther) 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) + }) +}