Custom groups (#18839)

* WIP

* adding initial creategroup endpoint

* fetching by group source

* fixing startup error

* updating create endpoint to take an array of user_ids, this will allow us to create the group with one request

* adding delete group endpoint and appropriate test

* adding source param for getGroups

* adding add members and delete members endpoints

* locking down crud endpoints to only be allowed for custom groups

* user search stuff

* allowing remoteid be null by changing field to pointer

* code cleanup and store level tests

* adding new tests and removing unused endpoint

* resolving conflicts

* Adds authz check for group.

* Adds authz checks to groups APIs.

* Updated create group authz tests.

* Updates delete group tests.

* Tests create group.

* Adds some tests and validations.

* adding new parameter so I can get users not in a group

* Fixed all lint warnings.

* Fix type.

* fixing search users not in group

* Fixes some lint errors.

* Moves entry in JSON array.

* Fixed SQL query.

* Fixes permission migration test.

* Fixes migration test.

* Fixes some group store tests.

* Fix test.

* Fix test.

* Revert lint change.

* Migrated CreateWithUserIds to sqlx.

* Adds tests for GetMember; migrates implementation to sqlx.

* Tests GetNonMemberUsersPage and hanles wrong group id.

* Fixes test.

* Switches GetMaster to GetMasterX.

* Switches GetReplica to GetReplicaX.

* Fixes logic.

* Fixes shadow declaration.

* Adds include_member_count to get group API endpoint.

* Adds filter_has_member param to getGroups.

* Fixes.

* Removes array of group sources.

* fixing error

* Testing reverting CreateWithUserIds back to gorp.

* Added websocket event for CreateGroupWithUserIds.

* Changed a few response status codes. Switched to correct permission.

* Added member count to ws payload for group when updating or creating.

* Adds feature flag checks for custom groups.

* Added middleware function to require license. Added config to disable custom groups.

* Change for function signature change of executePossiblyEmptyQuery.

* Lint fixes.

* Adds telemetry none comment.

* Adds translations.

* Migrated to sqlx.

* Temp. removal of translation.

* Fixed typo.

* Added an intermediary model to query with a field that is now ignored by sqlx on read queries.

* Re-used existing store struct.

* Inludes member count.

* Fix for merge error.'

* Require license for group endpoints.

* Updates translations.

* Fix shadow declaration.

* Renames permissions. Switches to new method to retrieve remoteid.

* Added WS events for upsert and delete member(s).

* Added new store error type ErrUniqueConstraint.

* Added EnableCustonGroups to the client config.

* Sanitized some user records.

* Added parameter to include_total_count for listing groups.

* Added translations.

* adding deleteAt field to getByUsers query

* Revert sanitize.

* Added uniqueness constraint error to UpdateGroup.

* Removed the FutureFeatures flag so that the feature is not enabled on old Enterprise licenses.

* Renamed function.

* Updates authz check for user search related to groups.

* Removed debug statement.

* Removed unused app method.

* Added telemetry for enable_custom_groups.

* Returns early from nil license.

* Updates test.

* Returned early to avoid nesting in (*SqlGroupStore).checkUserExist. Switched to reading from replica in (*SqlGroupStore).GetMember. Handled JSON marshal error in (*Client4).UpsertGroupMembers

* Switched to SanitizeProfile.

* Switched to model.NewInt.

* Switched from status NotImplemented to Forbidden for missing license.

* Removed deactivated users from 'exists' set.

* Revert gotool update.

* Ignored lint error that I think is invalid.

* Added the approprate access tag for disabling custom groups.

* Revert change to response status.

* Fixed refactor mistake.

* Limited the group member WS events to individual users.

* Removed WS event of deleted groups.

* Updated license check for searchUsers endpoint.

* Switched from license feature to license sku.

* Update app/group.go

Co-authored-by: Claudio Costa <cstcld91@gmail.com>

* Update app/group.go

Co-authored-by: Claudio Costa <cstcld91@gmail.com>

* Remove linter ignore comment.

* Added function to create sku-specific license.

* Fixed typo. Removed comment.

* Fixed for wrong type.

* Added missing param to client. Removed unnecessary props setting. Added test for retrieving groups by source.

* Updated some tests now that we're validating group membership not created for deactivated user.

* Fix for groups endpoint returning all group types by default.

* Changes constant names. Adds migration for all users to manage custom group members.

* Removes requirement for manage_system permission to filter user search by group.

* Added migration mock.

* Removes default permissions from custom_group_user role.

* Fixes migration.

* Fixes emoji migration test.

* fixing issue with member counts

* fixing search issue for deleted members

Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MacBook-Pro.local>
Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MBP.ht.home>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MacBook-Pro.fritz.box>
Co-authored-by: Claudio Costa <cstcld91@gmail.com>

api4/group_test.go

@@ -26,7 +26,7 @@ func TestGetGroup(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -65,7 +65,135 @@ func TestGetGroup(t *testing.T) {
 	require.Error(t, err)
 	CheckUnauthorizedStatus(t, response)
 }
+func TestCreateGroup(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
 
+	id := model.NewId()
+	g := &model.Group{
+		DisplayName:    "dn_" + id,
+		Name:           model.NewString("name" + id),
+		Source:         model.GroupSourceCustom,
+		Description:    "description_" + id,
+		AllowReference: true,
+	}
+
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional, "ldap"))
+
+	group, _, err := th.SystemAdminClient.CreateGroup(g)
+	require.NoError(t, err)
+
+	assert.Equal(t, g.DisplayName, group.DisplayName)
+	assert.Equal(t, g.Name, group.Name)
+	assert.Equal(t, g.Source, group.Source)
+	assert.Equal(t, g.Description, group.Description)
+	assert.Equal(t, g.RemoteId, group.RemoteId)
+
+	gbroken := &model.Group{
+		DisplayName: "dn_" + id,
+		Name:        model.NewString("name" + id),
+		Source:      "rrrr",
+		Description: "description_" + id,
+	}
+
+	_, response, err := th.SystemAdminClient.CreateGroup(gbroken)
+	require.Error(t, err)
+	CheckNotImplementedStatus(t, response)
+
+	validGroup := &model.Group{
+		DisplayName:    "dn_" + model.NewId(),
+		Name:           model.NewString("name" + model.NewId()),
+		Source:         model.GroupSourceCustom,
+		AllowReference: true,
+	}
+
+	th.RemovePermissionFromRole(model.PermissionCreateCustomGroup.Id, model.SystemAdminRoleId)
+	th.RemovePermissionFromRole(model.PermissionCreateCustomGroup.Id, model.SystemUserRoleId)
+	defer th.AddPermissionToRole(model.PermissionCreateCustomGroup.Id, model.SystemUserRoleId)
+	_, response, err = th.SystemAdminClient.CreateGroup(validGroup)
+	require.Error(t, err)
+	CheckForbiddenStatus(t, response)
+
+	th.AddPermissionToRole(model.PermissionCreateCustomGroup.Id, model.SystemAdminRoleId)
+	_, response, err = th.SystemAdminClient.CreateGroup(validGroup)
+	require.NoError(t, err)
+	CheckCreatedStatus(t, response)
+
+	unReferenceableCustomGroup := &model.Group{
+		DisplayName:    "dn_" + model.NewId(),
+		Name:           model.NewString("name" + model.NewId()),
+		Source:         model.GroupSourceCustom,
+		AllowReference: false,
+	}
+	_, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup)
+	require.Error(t, err)
+	CheckNotImplementedStatus(t, response)
+	unReferenceableCustomGroup.AllowReference = true
+	_, response, err = th.SystemAdminClient.CreateGroup(unReferenceableCustomGroup)
+	require.NoError(t, err)
+	CheckCreatedStatus(t, response)
+
+	customGroupWithRemoteID := &model.Group{
+		DisplayName:    "dn_" + model.NewId(),
+		Name:           model.NewString("name" + model.NewId()),
+		Source:         model.GroupSourceCustom,
+		AllowReference: true,
+		RemoteId:       model.NewString(model.NewId()),
+	}
+	_, response, err = th.SystemAdminClient.CreateGroup(customGroupWithRemoteID)
+	require.Error(t, err)
+	CheckNotImplementedStatus(t, response)
+
+	th.SystemAdminClient.Logout()
+	_, response, err = th.SystemAdminClient.CreateGroup(g)
+	require.Error(t, err)
+	CheckUnauthorizedStatus(t, response)
+}
+
+func TestDeleteGroup(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	id := model.NewId()
+	g, appErr := th.App.CreateGroup(&model.Group{
+		DisplayName: "dn_" + id,
+		Name:        model.NewString("name" + id),
+		Source:      model.GroupSourceLdap,
+		Description: "description_" + id,
+		RemoteId:    model.NewString(model.NewId()),
+	})
+	assert.Nil(t, appErr)
+
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional))
+
+	_, response, err := th.Client.DeleteGroup(g.Id)
+	require.Error(t, err)
+	CheckNotImplementedStatus(t, response)
+
+	th.AddPermissionToRole(model.PermissionDeleteCustomGroup.Id, model.SystemUserRoleId)
+	_, response, err = th.Client.DeleteGroup(g.Id)
+	require.Error(t, err)
+	CheckNotImplementedStatus(t, response)
+
+	_, response, err = th.Client.DeleteGroup(g.Id)
+	require.Error(t, err)
+	CheckNotImplementedStatus(t, response)
+
+	_, response, err = th.Client.DeleteGroup("wertyuijhbgvfcde")
+	require.Error(t, err)
+	CheckBadRequestStatus(t, response)
+
+	validGroup, appErr := th.App.CreateGroup(&model.Group{
+		DisplayName: "dn_" + model.NewId(),
+		Name:        model.NewString("name" + model.NewId()),
+		Source:      model.GroupSourceCustom,
+	})
+	assert.Nil(t, appErr)
+
+	_, response, err = th.Client.DeleteGroup(validGroup.Id)
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+}
 func TestPatchGroup(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()
@@ -76,7 +204,15 @@ func TestPatchGroup(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
+	})
+	assert.Nil(t, appErr)
+
+	g2, appErr := th.App.CreateGroup(&model.Group{
+		DisplayName:    "dn_" + model.NewId(),
+		Name:           model.NewString("name" + model.NewId()),
+		Source:         model.GroupSourceCustom,
+		AllowReference: true,
 	})
 	assert.Nil(t, appErr)
 
@@ -100,7 +236,7 @@ func TestPatchGroup(t *testing.T) {
 	require.Error(t, err)
 	CheckNotImplementedStatus(t, response)
 
-	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional, "ldap"))
 
 	group2, response, err := th.SystemAdminClient.PatchGroup(g.Id, gp)
 	require.NoError(t, err)
@@ -131,6 +267,23 @@ func TestPatchGroup(t *testing.T) {
 	require.Error(t, err)
 	CheckNotFoundStatus(t, response)
 
+	_, response, err = th.SystemAdminClient.PatchGroup(g2.Id, &model.GroupPatch{
+		Name:           model.NewString(model.NewId()),
+		DisplayName:    model.NewString("foo"),
+		AllowReference: model.NewBool(false),
+	})
+	require.Error(t, err)
+	CheckBadRequestStatus(t, response)
+
+	// ensure that omitting the AllowReference field from the patch doesn't patch it to false
+	patchedG2, response, err := th.SystemAdminClient.PatchGroup(g2.Id, &model.GroupPatch{
+		Name:        model.NewString(model.NewId()),
+		DisplayName: model.NewString("foo"),
+	})
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+	require.Equal(t, true, patchedG2.AllowReference)
+
 	th.SystemAdminClient.Logout()
 	_, response, err = th.SystemAdminClient.PatchGroup(group.Id, gp)
 	require.Error(t, err)
@@ -147,7 +300,7 @@ func TestLinkGroupTeam(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -187,7 +340,7 @@ func TestLinkGroupChannel(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -229,7 +382,7 @@ func TestUnlinkGroupTeam(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -280,7 +433,7 @@ func TestUnlinkGroupChannel(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -332,7 +485,7 @@ func TestGetGroupTeam(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -394,7 +547,7 @@ func TestGetGroupChannel(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -456,7 +609,7 @@ func TestGetGroupTeams(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -509,7 +662,7 @@ func TestGetGroupChannels(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -561,7 +714,7 @@ func TestPatchGroupTeam(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -633,7 +786,7 @@ func TestPatchGroupChannel(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -716,7 +869,7 @@ func TestGetGroupsByChannel(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -735,6 +888,8 @@ func TestGetGroupsByChannel(t *testing.T) {
 		},
 	}
 
+	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
+
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
 		_, _, response, err := client.GetGroupsByChannel("asdfasdf", opts)
 		require.Error(t, err)
@@ -795,7 +950,7 @@ func TestGetGroupsAssociatedToChannelsByTeam(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -814,6 +969,8 @@ func TestGetGroupsAssociatedToChannelsByTeam(t *testing.T) {
 		},
 	}
 
+	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
+
 	_, response, err := th.SystemAdminClient.GetGroupsAssociatedToChannelsByTeam("asdfasdf", opts)
 	require.Error(t, err)
 	CheckBadRequestStatus(t, response)
@@ -871,7 +1028,7 @@ func TestGetGroupsByTeam(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, err)
 
@@ -890,13 +1047,15 @@ func TestGetGroupsByTeam(t *testing.T) {
 		},
 	}
 
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional))
+
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
 		_, _, response, err := client.GetGroupsByTeam("asdfasdf", opts)
 		require.Error(t, err)
 		CheckBadRequestStatus(t, response)
 	})
 
-	th.App.Srv().SetLicense(nil)
+	th.App.Srv().RemoveLicense()
 
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
 		_, _, response, err := client.GetGroupsByTeam(th.BasicTeam.Id, opts)
@@ -945,27 +1104,32 @@ func TestGetGroups(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 	start := group.UpdateAt - 1
 
+	id2 := model.NewId()
+	group2, appErr := th.App.CreateGroup(&model.Group{
+		DisplayName: "dn-foo_" + id2,
+		Name:        model.NewString("name" + id2),
+		Source:      model.GroupSourceCustom,
+		Description: "description_" + id2,
+		RemoteId:    model.NewString(model.NewId()),
+	})
+	assert.Nil(t, appErr)
+
 	opts := model.GroupSearchOpts{
+		Source: model.GroupSourceLdap,
 		PageOpts: &model.PageOpts{
 			Page:    0,
 			PerPage: 60,
 		},
 	}
 
-	th.App.Srv().SetLicense(nil)
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional))
 
-	_, response, err := th.SystemAdminClient.GetGroups(opts)
-	require.Error(t, err)
-	CheckNotImplementedStatus(t, response)
-
-	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
-
-	_, _, err = th.SystemAdminClient.GetGroups(opts)
+	_, _, err := th.SystemAdminClient.GetGroups(opts)
 	require.NoError(t, err)
 
 	_, err = th.SystemAdminClient.UpdateChannelRoles(th.BasicChannel.Id, th.BasicUser.Id, "")
@@ -1031,6 +1195,12 @@ func TestGetGroups(t *testing.T) {
 	assert.Len(t, groups, 1)
 	// make sure it returned th.Group,not group
 	assert.Equal(t, groups[0].Id, th.Group.Id)
+
+	opts.Source = model.GroupSourceCustom
+	groups, _, err = th.Client.GetGroups(opts)
+	assert.NoError(t, err)
+	assert.Len(t, groups, 1)
+	assert.Equal(t, groups[0].Id, group2.Id)
 }
 
 func TestGetGroupsByUserId(t *testing.T) {
@@ -1043,7 +1213,7 @@ func TestGetGroupsByUserId(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -1059,7 +1229,7 @@ func TestGetGroupsByUserId(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -1109,7 +1279,7 @@ func TestGetGroupStats(t *testing.T) {
 		Name:        model.NewString("name" + id),
 		Source:      model.GroupSourceLdap,
 		Description: "description_" + id,
-		RemoteId:    model.NewId(),
+		RemoteId:    model.NewString(model.NewId()),
 	})
 	assert.Nil(t, appErr)
 
@@ -1129,7 +1299,8 @@ func TestGetGroupStats(t *testing.T) {
 	})
 
 	t.Run("Returns stats for a group with no members", func(t *testing.T) {
-		stats, _, _ := th.SystemAdminClient.GetGroupStats(group.Id)
+		stats, _, err := th.SystemAdminClient.GetGroupStats(group.Id)
+		require.NoError(t, err)
 		assert.Equal(t, stats.GroupID, group.Id)
 		assert.Equal(t, stats.TotalMemberCount, int64(0))
 	})
@@ -1160,7 +1331,7 @@ func TestGetGroupsGroupConstrainedParentTeam(t *testing.T) {
 			Name:        model.NewString("name" + id),
 			Source:      model.GroupSourceLdap,
 			Description: "description_" + id,
-			RemoteId:    model.NewId(),
+			RemoteId:    model.NewString(model.NewId()),
 		})
 		require.Nil(t, err)
 		groups = append(groups, group)
@@ -1228,3 +1399,149 @@ func TestGetGroupsGroupConstrainedParentTeam(t *testing.T) {
 	require.NotContains(t, apiGroups, groups[0])
 	require.Contains(t, apiGroups, groups[2])
 }
+
+func TestAddMembersToGroup(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	// 1. Test with custom source
+	id := model.NewId()
+	group, err := th.App.CreateGroup(&model.Group{
+		DisplayName: "dn_" + id,
+		Name:        model.NewString("name" + id),
+		Source:      model.GroupSourceCustom,
+		Description: "description_" + id,
+	})
+	assert.Nil(t, err)
+
+	user1, appErr := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user1", Password: "test-password-1", Username: "test-user-1", Roles: model.SystemUserRoleId})
+	assert.Nil(t, appErr)
+
+	user2, appErr := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user2", Password: "test-password-2", Username: "test-user-2", Roles: model.SystemUserRoleId})
+	assert.Nil(t, appErr)
+
+	members := &model.GroupModifyMembers{
+		UserIds: []string{user1.Id, user2.Id},
+	}
+
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional))
+
+	groupMembers, response, upsertErr := th.SystemAdminClient.UpsertGroupMembers(group.Id, members)
+	require.NoError(t, upsertErr)
+	CheckOKStatus(t, response)
+
+	assert.Len(t, groupMembers, 2)
+
+	count, countErr := th.App.GetGroupMemberCount(group.Id)
+	assert.Nil(t, countErr)
+
+	assert.Equal(t, count, int64(2))
+
+	// 2. Test invalid group ID
+	_, response, upsertErr = th.Client.UpsertGroupMembers("abc123", members)
+	require.Error(t, upsertErr)
+	CheckBadRequestStatus(t, response)
+
+	// 3. Test invalid user ID
+	invalidMembers := &model.GroupModifyMembers{
+		UserIds: []string{"abc123"},
+	}
+
+	_, response, upsertErr = th.SystemAdminClient.UpsertGroupMembers(group.Id, invalidMembers)
+	require.Error(t, upsertErr)
+	CheckInternalErrorStatus(t, response)
+
+	// 4. Test with ldap source
+	ldapId := model.NewId()
+	ldapGroup, err := th.App.CreateGroup(&model.Group{
+		DisplayName: "dn_" + ldapId,
+		Name:        model.NewString("name" + ldapId),
+		Source:      model.GroupSourceLdap,
+		Description: "description_" + ldapId,
+		RemoteId:    model.NewString(model.NewId()),
+	})
+	assert.Nil(t, err)
+
+	_, response, upsertErr = th.SystemAdminClient.UpsertGroupMembers(ldapGroup.Id, members)
+
+	require.Error(t, upsertErr)
+	CheckNotImplementedStatus(t, response)
+}
+
+func TestDeleteMembersFromGroup(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	// 1. Test with custom source
+	user1, appErr := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user1", Password: "test-password-1", Username: "test-user-1", Roles: model.SystemUserRoleId})
+	assert.Nil(t, appErr)
+
+	user2, appErr := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user2", Password: "test-password-2", Username: "test-user-2", Roles: model.SystemUserRoleId})
+	assert.Nil(t, appErr)
+
+	id := model.NewId()
+	g := &model.Group{
+		DisplayName: "dn_" + id,
+		Name:        model.NewString("name" + id),
+		Source:      model.GroupSourceCustom,
+		Description: "description_" + id,
+	}
+	group, err := th.App.CreateGroupWithUserIds(&model.GroupWithUserIds{
+		Group:   *g,
+		UserIds: []string{user1.Id, user2.Id},
+	})
+	assert.Nil(t, err)
+
+	members := &model.GroupModifyMembers{
+		UserIds: []string{user1.Id},
+	}
+
+	th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional))
+
+	groupMembers, response, deleteErr := th.SystemAdminClient.DeleteGroupMembers(group.Id, members)
+	require.NoError(t, deleteErr)
+	CheckOKStatus(t, response)
+
+	assert.Len(t, groupMembers, 1)
+	assert.Equal(t, groupMembers[0].UserId, user1.Id)
+
+	users, usersErr := th.App.GetGroupMemberUsers(group.Id)
+	assert.Nil(t, usersErr)
+
+	assert.Len(t, users, 1)
+	assert.Equal(t, users[0].Id, user2.Id)
+
+	// 2. Test invalid group ID
+	_, response, deleteErr = th.Client.DeleteGroupMembers("abc123", members)
+	require.Error(t, deleteErr)
+	CheckBadRequestStatus(t, response)
+
+	// 3. Test invalid user ID
+	invalidMembers := &model.GroupModifyMembers{
+		UserIds: []string{"abc123"},
+	}
+
+	_, response, deleteErr = th.SystemAdminClient.DeleteGroupMembers(group.Id, invalidMembers)
+	require.Error(t, deleteErr)
+	CheckInternalErrorStatus(t, response)
+
+	// 4. Test with ldap source
+	ldapId := model.NewId()
+	g1 := &model.Group{
+		DisplayName: "dn_" + ldapId,
+		Name:        model.NewString("name" + ldapId),
+		Source:      model.GroupSourceLdap,
+		Description: "description_" + ldapId,
+		RemoteId:    model.NewString(model.NewId()),
+	}
+	ldapGroup, err := th.App.CreateGroupWithUserIds(&model.GroupWithUserIds{
+		Group:   *g1,
+		UserIds: []string{user1.Id, user2.Id},
+	})
+	assert.Nil(t, err)
+
+	_, response, deleteErr = th.SystemAdminClient.DeleteGroupMembers(ldapGroup.Id, members)
+
+	require.Error(t, deleteErr)
+	CheckNotImplementedStatus(t, response)
+}