From 59b10e66f757441e8c74532f30740bf4a96e9ac1 Mon Sep 17 00:00:00 2001
From: Lanre Adelowo <adelowomailbox@gmail.com>
Date: Tue, 7 Aug 2018 11:01:06 +0100
Subject: [PATCH] An inactive user shouldn't be able to be added as a
 collaborator (#4535)

* an inactive user shouldn't be able to be a collaborator

* use translated error message

* add active user check when adding a new collaborator via the api

* fix translation text

* added collaborator test

* improvee testcases
---
 options/locale/locale_en-US.ini      |  3 ++-
 routers/api/v1/repo/collaborators.go |  7 +++++++
 routers/repo/setting.go              |  6 ++++++
 routers/repo/settings_test.go        | 24 ++++++++++++++++++++++++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index bd13288b46..85178d3417 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1025,7 +1025,8 @@ 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_duplicate =The collaborator is already added to this repository.
+settings.add_collaborator_inactive_user = Can not add an inactive user as a collaborator.
+settings.add_collaborator_duplicate = The collaborator is already added to this repository.
 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 963b7c53ae..c52472b0f8 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
diff --git a/routers/repo/setting.go b/routers/repo/setting.go
index 835ba0a751..b1f50d5387 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(ctx.Tr("repo.settings.add_collaborator_inactive_user"))
+		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"))
diff --git a/routers/repo/settings_test.go b/routers/repo/settings_test.go
index a8cc645075..cf7ed840a8 100644
--- a/routers/repo/settings_test.go
+++ b/routers/repo/settings_test.go
@@ -97,6 +97,30 @@ func TestCollaborationPost(t *testing.T) {
 	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)
+}
+
 func TestCollaborationPost_AddCollaboratorTwice(t *testing.T) {
 
 	models.PrepareTestEnv(t)