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
This commit is contained in:
		
							parent
							
								
									c7a6ee5c0b
								
							
						
					
					
						commit
						59b10e66f7
					
				|  | @ -1025,7 +1025,8 @@ settings.transfer_succeed = The repository has been transferred. | ||||||
| settings.confirm_delete = Delete Repository | settings.confirm_delete = Delete Repository | ||||||
| settings.add_collaborator = Add Collaborator | settings.add_collaborator = Add Collaborator | ||||||
| settings.add_collaborator_success = The collaborator has been added. | 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.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? | ||||||
|  |  | ||||||
|  | @ -5,6 +5,8 @@ | ||||||
| package repo | package repo | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
|  | 	"errors" | ||||||
|  | 
 | ||||||
| 	"code.gitea.io/gitea/models" | 	"code.gitea.io/gitea/models" | ||||||
| 	"code.gitea.io/gitea/modules/context" | 	"code.gitea.io/gitea/modules/context" | ||||||
| 
 | 
 | ||||||
|  | @ -145,6 +147,11 @@ func AddCollaborator(ctx *context.APIContext, form api.AddCollaboratorOption) { | ||||||
| 		return | 		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 { | 	if err := ctx.Repo.Repository.AddCollaborator(collaborator); err != nil { | ||||||
| 		ctx.Error(500, "AddCollaborator", err) | 		ctx.Error(500, "AddCollaborator", err) | ||||||
| 		return | 		return | ||||||
|  |  | ||||||
|  | @ -381,6 +381,12 @@ func CollaborationPost(ctx *context.Context) { | ||||||
| 		return | 		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.
 | 	// Organization is not allowed to be added as a collaborator.
 | ||||||
| 	if u.IsOrganization() { | 	if u.IsOrganization() { | ||||||
| 		ctx.Flash.Error(ctx.Tr("repo.settings.org_not_allowed_to_be_collaborator")) | 		ctx.Flash.Error(ctx.Tr("repo.settings.org_not_allowed_to_be_collaborator")) | ||||||
|  |  | ||||||
|  | @ -97,6 +97,30 @@ func TestCollaborationPost(t *testing.T) { | ||||||
| 	assert.True(t, exists) | 	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) { | func TestCollaborationPost_AddCollaboratorTwice(t *testing.T) { | ||||||
| 
 | 
 | ||||||
| 	models.PrepareTestEnv(t) | 	models.PrepareTestEnv(t) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue