From cbf4c9275509e483c8c40c4a3eddaec43fc4f8a4 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sat, 28 Jul 2018 12:05:19 +0100 Subject: [PATCH 1/6] an inactive user shouldn't be able to be a collaborator --- options/locale/locale_en-US.ini | 1 + routers/repo/setting.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 96d450b5ca8d1..2a91db127948d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1014,6 +1014,7 @@ settings.transfer_succeed = The repository has been transferred. settings.confirm_delete = Delete Repository settings.add_collaborator = Add Collaborator settings.add_collaborator_success = The collaborator has been added. +settings.add_collaborator_inactive_user = Cannot add an inactive user as a collaborator. 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/repo/setting.go b/routers/repo/setting.go index fa3bd434d5598..048b500228646 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -381,6 +381,12 @@ func CollaborationPost(ctx *context.Context) { return } + if !u.IsActive { + ctx.Flash.Error("cannot add an inactive user as a collaborator") + ctx.Redirect(setting.AppSubURL + ctx.Req.URL.Path) + return + } + // Organization is not allowed to be added as a collaborator. if u.IsOrganization() { ctx.Flash.Error(ctx.Tr("repo.settings.org_not_allowed_to_be_collaborator")) From bf402c710675d1202d0944813105e7918cb49301 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sat, 28 Jul 2018 12:14:27 +0100 Subject: [PATCH 2/6] use translated error message --- routers/repo/setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 048b500228646..e855e5916bdb2 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -382,7 +382,7 @@ func CollaborationPost(ctx *context.Context) { } if !u.IsActive { - ctx.Flash.Error("cannot add an inactive user as a collaborator") + ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_inactive_user")) ctx.Redirect(setting.AppSubURL + ctx.Req.URL.Path) return } From 9d6f15c81554f1446ece7b265726cc02256cdb1e Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sun, 29 Jul 2018 10:27:34 +0100 Subject: [PATCH 3/6] add active user check when adding a new collaborator via the api --- routers/api/v1/repo/collaborators.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index 963b7c53ae32d..c52472b0f8724 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -5,6 +5,8 @@ package repo import ( + "errors" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" @@ -145,6 +147,11 @@ func AddCollaborator(ctx *context.APIContext, form api.AddCollaboratorOption) { return } + if !collaborator.IsActive { + ctx.Error(500, "InactiveCollaborator", errors.New("collaborator's account is inactive")) + return + } + if err := ctx.Repo.Repository.AddCollaborator(collaborator); err != nil { ctx.Error(500, "AddCollaborator", err) return From ace2e37c9a555f061a7810e80dfe7d937c456802 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sun, 29 Jul 2018 12:36:08 +0100 Subject: [PATCH 4/6] fix translation text --- options/locale/locale_en-US.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 2a91db127948d..d905ad3784b6f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1014,7 +1014,7 @@ settings.transfer_succeed = The repository has been transferred. settings.confirm_delete = Delete Repository settings.add_collaborator = Add Collaborator 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 = Can not add an inactive user as a collaborator. settings.delete_collaborator = Remove settings.collaborator_deletion = Remove Collaborator settings.collaborator_deletion_desc = Removing a collaborator will revoke their access to this repository. Continue? From 47278adb2338b0f57c6181e62166fcb29d795c1a Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sun, 29 Jul 2018 18:59:33 +0100 Subject: [PATCH 5/6] added collaborator test --- routers/repo/settings_test.go | 62 +++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/routers/repo/settings_test.go b/routers/repo/settings_test.go index 392c05f773a77..1a0bbccbc7f5e 100644 --- a/routers/repo/settings_test.go +++ b/routers/repo/settings_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" + "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" @@ -59,3 +60,64 @@ func TestAddReadWriteOnlyDeployKey(t *testing.T) { Mode: models.AccessModeWrite, }) } + +func TestCollaborationPost(t *testing.T) { + + models.PrepareTestEnv(t) + ctx := test.MockContext(t, "user2/repo1/issues/labels") + test.LoadUser(t, ctx, 2) + test.LoadUser(t, ctx, 4) + test.LoadRepo(t, ctx, 1) + + ctx.Req.Form.Set("collaborator", "user4") + + u := &models.User{ + LowerName: "user2", + Type: models.UserTypeIndividual, + } + + re := &models.Repository{ + ID: 2, + Owner: u, + } + + repo := &context.Repository{ + Owner: u, + Repository: re, + } + + ctx.Repo = repo + + CollaborationPost(ctx) + + assert.EqualValues(t, http.StatusFound, ctx.Resp.Status()) + + exists, err := re.IsCollaborator(4) + assert.NoError(t, err) + assert.True(t, exists) +} + +func TestCollaborationPost_InactiveUser(t *testing.T) { + + models.PrepareTestEnv(t) + ctx := test.MockContext(t, "user2/repo1/issues/labels") + test.LoadUser(t, ctx, 2) + test.LoadUser(t, ctx, 9) + test.LoadRepo(t, ctx, 1) + + ctx.Req.Form.Set("collaborator", "user9") + + repo := &context.Repository{ + Owner: &models.User{ + LowerName: "user2", + }, + } + + ctx.Repo = repo + + CollaborationPost(ctx) + + assert.EqualValues(t, http.StatusFound, ctx.Resp.Status()) + assert.NotEmpty(t, ctx.Flash.ErrorMsg) + +} From 7decd5101f924bcb585f5b91a4b46f2c2b1c8bb3 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sun, 29 Jul 2018 23:55:38 +0100 Subject: [PATCH 6/6] improvee testcases --- routers/repo/settings_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/routers/repo/settings_test.go b/routers/repo/settings_test.go index 1a0bbccbc7f5e..8df272898c381 100644 --- a/routers/repo/settings_test.go +++ b/routers/repo/settings_test.go @@ -119,5 +119,27 @@ func TestCollaborationPost_InactiveUser(t *testing.T) { assert.EqualValues(t, http.StatusFound, ctx.Resp.Status()) assert.NotEmpty(t, ctx.Flash.ErrorMsg) +} + +func TestCollaborationPost_NonExistentUser(t *testing.T) { + + models.PrepareTestEnv(t) + ctx := test.MockContext(t, "user2/repo1/issues/labels") + test.LoadUser(t, ctx, 2) + test.LoadRepo(t, ctx, 1) + + ctx.Req.Form.Set("collaborator", "user34") + + repo := &context.Repository{ + Owner: &models.User{ + LowerName: "user2", + }, + } + + ctx.Repo = repo + + CollaborationPost(ctx) + assert.EqualValues(t, http.StatusFound, ctx.Resp.Status()) + assert.NotEmpty(t, ctx.Flash.ErrorMsg) }