Fix MM-55314 (#26595)

server/channels/api4/group.go

@@ -5,6 +5,7 @@ package api4
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -14,6 +15,7 @@ import (
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/v8/channels/app"
 	"github.com/mattermost/mattermost/server/v8/channels/audit"
+	"github.com/mattermost/mattermost/server/v8/channels/store"
 )
 
 func (api *API) InitGroup() {
@@ -332,17 +334,6 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil)
-	if appErr != nil {
-		c.Err = appErr
-		return
-	}
-
-	if group.Source != model.GroupSourceLdap {
-		c.Err = model.NewAppError("Api4.linkGroupSyncable", "app.group.crud_permission", nil, "", http.StatusBadRequest)
-		return
-	}
-
 	auditRec := c.MakeAuditRecord("linkGroupSyncable", audit.Fail)
 	defer c.LogAuditRec(auditRec)
 	audit.AddEventParameter(auditRec, "group_id", c.Params.GroupId)
@@ -363,8 +354,9 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	appErr = verifyLinkUnlinkPermission(c, syncableType, syncableID)
+	appErr := verifyLinkUnlinkPermission(c, syncableType, syncableID)
 	if appErr != nil {
+		appErr.Where = "Api4.linkGroupSyncable"
 		c.Err = appErr
 		return
 	}
@@ -541,6 +533,7 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	appErr := verifyLinkUnlinkPermission(c, syncableType, syncableID)
 	if appErr != nil {
+		appErr.Where = "Api4.patchGroupSyncable"
 		c.Err = appErr
 		return
 	}
@@ -611,6 +604,7 @@ func unlinkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	appErr := verifyLinkUnlinkPermission(c, syncableType, syncableID)
 	if appErr != nil {
+		appErr.Where = "Api4.unlinkGroupSyncable"
 		c.Err = appErr
 		return
 	}
@@ -631,15 +625,48 @@ func unlinkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) {
 }
 
 func verifyLinkUnlinkPermission(c *Context, syncableType model.GroupSyncableType, syncableID string) *model.AppError {
+	group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil)
+	if appErr != nil {
+		return appErr
+	}
+
+	if group.Source != model.GroupSourceLdap {
+		return model.NewAppError("Api4.linkGroupSyncable", "app.group.crud_permission", nil, "", http.StatusBadRequest)
+	}
+
+	// If AllowReference is disabled, limit who can link the group.
+	// This voids leaking the list of group members.
+	// See https://mattermost.atlassian.net/browse/MM-55314 for more details.
+	if !group.AllowReference {
+		if !c.App.SessionHasPermissionToGroup(*c.AppContext.Session(), c.Params.GroupId, model.PermissionSysconsoleReadUserManagementGroups) {
+			return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionSysconsoleReadUserManagementGroups})
+		}
+	}
+
 	switch syncableType {
 	case model.GroupSyncableTypeTeam:
-		if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionManageTeam) {
+		if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionInviteUser) {
-			return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageTeam})
+			return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionInviteUser})
 		}
 	case model.GroupSyncableTypeChannel:
-		channel, err := c.App.GetChannel(c.AppContext, syncableID)
+		channel, appErr := c.App.GetChannel(c.AppContext, syncableID)
-		if err != nil {
+		if appErr != nil {
-			return err
+			return appErr
+		}
+
+		// If it's the first time that the syncable gets linked to the team (i.e. no current sync to the team or to a team's channel),
+		// check that the user has the permission to manage the team.
+		_, appErr = c.App.GetGroupSyncable(c.Params.GroupId, channel.TeamId, model.GroupSyncableTypeTeam)
+		if appErr != nil {
+			var nfErr *store.ErrNotFound
+			switch {
+			case errors.As(appErr, &nfErr):
+				if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionInviteUser) {
+					return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionInviteUser})
+				}
+			default:
+				return appErr
+			}
 		}
 
 		var permission *model.Permission

server/channels/api4/group_test.go

@@ -367,44 +367,94 @@ func TestLinkGroupTeam(t *testing.T) {
 	})
 	assert.Nil(t, appErr)
 
+	id = model.NewId()
+	gRef, 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()),
+		AllowReference: true,
+	})
+	assert.Nil(t, appErr)
+
 	patch := &model.GroupSyncablePatch{
 		AutoAdd: model.NewBool(true),
 	}
 
-	_, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	t.Run("Error if no license is installed", func(t *testing.T) {
-	require.Error(t, err)
+		groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
-	CheckNotImplementedStatus(t, response)
+		require.Error(t, err)
+		CheckNotImplementedStatus(t, response)
+		assert.Nil(t, groupSyncable)
 
-	_, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+		groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
-	require.Error(t, err)
+		require.Error(t, err)
-	CheckNotImplementedStatus(t, response)
+		CheckNotImplementedStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
 
 	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
 
-	_, _, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	t.Run("Normal users are not allowed to link", func(t *testing.T) {
-	assert.Error(t, err)
+		groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
 
 	th.UpdateUserToTeamAdmin(th.BasicUser, th.BasicTeam)
-	th.Client.Logout(context.Background())
+	response, err := th.Client.Logout(context.Background())
-	th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+	_, response, err = th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
 
-	groupTeam, response, _ := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	var groupSyncable *model.GroupSyncable
-	assert.Equal(t, http.StatusCreated, response.StatusCode)
+	t.Run("Team admins are not allowed to link", func(t *testing.T) {
-	assert.NotNil(t, groupTeam)
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
-
+		require.Error(t, err)
-	gid := model.NewId()
+		CheckForbiddenStatus(t, response)
-	g2, app2Err := th.App.CreateGroup(&model.Group{
+		assert.Nil(t, groupSyncable)
-		DisplayName: "dn_" + gid,
-		Name:        model.NewString("name" + gid),
-		Source:      model.GroupSourceCustom,
-		Description: "description_" + gid,
-		RemoteId:    model.NewString(model.NewId()),
 	})
-	assert.Nil(t, app2Err)
 
-	_, response, err = th.Client.LinkGroupSyncable(context.Background(), g2.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	t.Run("Team admins are allowed to link if AllowReference is enabled", func(t *testing.T) {
-	require.Error(t, err)
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
-	CheckBadRequestStatus(t, response)
+		require.NoError(t, err)
+		CheckCreatedStatus(t, response)
+		assert.NotNil(t, groupSyncable)
+
+		t.Cleanup(func() {
+			response, err = th.Client.UnlinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+			require.NoError(t, err)
+			CheckOKStatus(t, response)
+		})
+	})
+
+	t.Run("System admins are allowed to link", func(t *testing.T) {
+		groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+		require.NoError(t, err)
+		CheckCreatedStatus(t, response)
+		assert.NotNil(t, groupSyncable)
+	})
+
+	t.Run("Custom groups can't be linked", func(t *testing.T) {
+		gid := model.NewId()
+		gCustom, appErr := th.App.CreateGroup(&model.Group{
+			DisplayName: "dn_" + gid,
+			Name:        model.NewString("name" + gid),
+			Source:      model.GroupSourceCustom,
+			Description: "description_" + gid,
+			RemoteId:    model.NewString(model.NewId()),
+		})
+		assert.Nil(t, appErr)
+
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), gCustom.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+		require.Error(t, err)
+		CheckBadRequestStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
 }
 
 func TestLinkGroupChannel(t *testing.T) {
@@ -421,46 +471,106 @@ func TestLinkGroupChannel(t *testing.T) {
 	})
 	assert.Nil(t, appErr)
 
+	id = model.NewId()
+	gRef, 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()),
+		AllowReference: true,
+	})
+	assert.Nil(t, appErr)
+
 	patch := &model.GroupSyncablePatch{
 		AutoAdd: model.NewBool(true),
 	}
 
-	_, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+	t.Run("Error if no license is installed", func(t *testing.T) {
-	require.Error(t, err)
+		groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
-	CheckNotImplementedStatus(t, response)
+		require.Error(t, err)
+		CheckNotImplementedStatus(t, response)
+		assert.Nil(t, groupSyncable)
 
-	_, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+		groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
-	require.Error(t, err)
+		require.Error(t, err)
-	CheckNotImplementedStatus(t, response)
+		CheckNotImplementedStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
 
 	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
 
-	groupTeam, response, _ := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+	t.Run("Normal users are not allowed to link", func(t *testing.T) {
-	assert.Equal(t, http.StatusCreated, response.StatusCode)
+		groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
-	assert.Equal(t, th.BasicChannel.TeamId, groupTeam.TeamID)
+		require.Error(t, err)
-	assert.NotNil(t, groupTeam)
+		CheckForbiddenStatus(t, response)
-
+		assert.Nil(t, groupSyncable)
-	_, err = th.SystemAdminClient.UpdateChannelRoles(context.Background(), th.BasicChannel.Id, th.BasicUser.Id, "")
-	require.NoError(t, err)
-	th.Client.Logout(context.Background())
-	th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
-
-	_, _, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
-	assert.Error(t, err)
-
-	gid := model.NewId()
-	g2, app2Err := th.App.CreateGroup(&model.Group{
-		DisplayName: "dn_" + gid,
-		Name:        model.NewString("name" + gid),
-		Source:      model.GroupSourceCustom,
-		Description: "description_" + gid,
-		RemoteId:    model.NewString(model.NewId()),
 	})
-	assert.Nil(t, app2Err)
 
-	_, response, err = th.Client.LinkGroupSyncable(context.Background(), g2.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+	th.MakeUserChannelAdmin(th.BasicUser, th.BasicChannel)
-	require.Error(t, err)
+	response, err := th.Client.Logout(context.Background())
-	CheckBadRequestStatus(t, response)
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+	_, response, err = th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+
+	var groupSyncable *model.GroupSyncable
+	t.Run("Channel admins are not allowed to link", func(t *testing.T) {
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
+
+	t.Run("Channel admins are not allowed to link if AllowReference is enabled, but not team syncable exists", func(t *testing.T) {
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
+
+	groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	require.NoError(t, err)
+	CheckCreatedStatus(t, response)
+	assert.NotNil(t, groupSyncable)
+
+	t.Run("Channel admins are allowed to link if AllowReference is enabled and a team syncable exists", func(t *testing.T) {
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+		require.NoError(t, err)
+		CheckCreatedStatus(t, response)
+		assert.NotNil(t, groupSyncable)
+
+		t.Cleanup(func() {
+			response, err = th.Client.UnlinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+			require.NoError(t, err)
+			CheckOKStatus(t, response)
+		})
+	})
+
+	t.Run("System admins are allowed to link", func(t *testing.T) {
+		groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+		require.NoError(t, err)
+		CheckCreatedStatus(t, response)
+		assert.NotNil(t, groupSyncable)
+	})
+
+	t.Run("Custom groups can't be linked", func(t *testing.T) {
+		gid := model.NewId()
+		g2, appErr := th.App.CreateGroup(&model.Group{
+			DisplayName: "dn_" + gid,
+			Name:        model.NewString("name" + gid),
+			Source:      model.GroupSourceCustom,
+			Description: "description_" + gid,
+			RemoteId:    model.NewString(model.NewId()),
+		})
+		assert.Nil(t, appErr)
+
+		groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), g2.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+		require.Error(t, err)
+		CheckBadRequestStatus(t, response)
+		assert.Nil(t, groupSyncable)
+	})
 }
 
 func TestUnlinkGroupTeam(t *testing.T) {
@@ -477,30 +587,53 @@ func TestUnlinkGroupTeam(t *testing.T) {
 	})
 	assert.Nil(t, appErr)
 
+	id = model.NewId()
+	gRef, 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()),
+		AllowReference: true,
+	})
+	assert.Nil(t, appErr)
+
 	patch := &model.GroupSyncablePatch{
 		AutoAdd: model.NewBool(true),
 	}
 
 	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
 
-	_, response, _ := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	groupSyncable, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
-	assert.Equal(t, http.StatusCreated, response.StatusCode)
+	require.NoError(t, err)
+	CheckCreatedStatus(t, response)
+	assert.NotNil(t, groupSyncable)
 
-	th.App.Srv().SetLicense(nil)
+	groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+	require.NoError(t, err)
+	CheckCreatedStatus(t, response)
+	assert.NotNil(t, groupSyncable)
 
-	response, err := th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+	t.Run("Error if no license is installed", func(t *testing.T) {
-	require.Error(t, err)
+		th.App.Srv().SetLicense(nil)
-	CheckNotImplementedStatus(t, response)
+		t.Cleanup(func() { th.App.Srv().SetLicense(model.NewTestLicense("ldap")) })
 
-	response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
-	require.Error(t, err)
+		require.Error(t, err)
-	CheckNotImplementedStatus(t, response)
+		CheckNotImplementedStatus(t, response)
 
-	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
+		response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+		require.Error(t, err)
+		CheckNotImplementedStatus(t, response)
+	})
 
-	_, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+	t.Run("Normal users are not allowed to unlink", func(t *testing.T) {
-	assert.Error(t, err)
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
-	time.Sleep(2 * time.Second) // A hack to let "go c.App.SyncRolesAndMembership" finish before moving on.
+		assert.Error(t, err)
+		CheckForbiddenStatus(t, response)
+	})
+
+	time.Sleep(4 * time.Second) // A hack to let "go c.App.SyncRolesAndMembership" finish before moving on.
 	th.UpdateUserToTeamAdmin(th.BasicUser, th.BasicTeam)
 	response, err = th.Client.Logout(context.Background())
 	require.NoError(t, err)
@@ -509,9 +642,46 @@ func TestUnlinkGroupTeam(t *testing.T) {
 	require.NoError(t, err)
 	CheckOKStatus(t, response)
 
-	response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+	t.Run("Team admins are not allowed to link", func(t *testing.T) {
-	require.NoError(t, err)
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
-	CheckOKStatus(t, response)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, response)
+	})
+
+	t.Run("Team admins are allowed to unlink if AllowReference is enabled", func(t *testing.T) {
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+		require.NoError(t, err)
+		CheckOKStatus(t, response)
+
+		t.Cleanup(func() {
+			groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch)
+			require.NoError(t, err)
+			CheckCreatedStatus(t, response)
+			assert.NotNil(t, groupSyncable)
+		})
+	})
+
+	t.Run("System admins are allowed to unlink", func(t *testing.T) {
+		response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), gRef.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+		require.NoError(t, err)
+		CheckOKStatus(t, response)
+	})
+
+	t.Run("Custom groups can't get unlinked", func(t *testing.T) {
+		gid := model.NewId()
+		g2, appErr := th.App.CreateGroup(&model.Group{
+			DisplayName: "dn_" + gid,
+			Name:        model.NewString("name" + gid),
+			Source:      model.GroupSourceCustom,
+			Description: "description_" + gid,
+			RemoteId:    model.NewString(model.NewId()),
+		})
+		assert.Nil(t, appErr)
+
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g2.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
+		require.Error(t, err)
+		CheckBadRequestStatus(t, response)
+	})
 }
 
 func TestUnlinkGroupChannel(t *testing.T) {
@@ -528,42 +698,101 @@ func TestUnlinkGroupChannel(t *testing.T) {
 	})
 	assert.Nil(t, appErr)
 
+	id = model.NewId()
+	gRef, 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()),
+		AllowReference: true,
+	})
+	assert.Nil(t, appErr)
+
 	patch := &model.GroupSyncablePatch{
 		AutoAdd: model.NewBool(true),
 	}
 
 	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
 
-	_, response, _ := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+	groupSyncable, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
-	assert.Equal(t, http.StatusCreated, response.StatusCode)
-
-	th.App.Srv().SetLicense(nil)
-
-	response, err := th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
-	require.Error(t, err)
-	CheckNotImplementedStatus(t, response)
-
-	response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
-	require.Error(t, err)
-	CheckNotImplementedStatus(t, response)
-
-	th.App.Srv().SetLicense(model.NewTestLicense("ldap"))
-
-	_, err = th.SystemAdminClient.UpdateChannelRoles(context.Background(), th.BasicChannel.Id, th.BasicUser.Id, "")
 	require.NoError(t, err)
-	th.Client.Logout(context.Background())
+	CheckCreatedStatus(t, response)
-	th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+	assert.NotNil(t, groupSyncable)
 
-	_, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+	groupSyncable, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
-	assert.Error(t, err)
-
-	_, err = th.SystemAdminClient.UpdateChannelRoles(context.Background(), th.BasicChannel.Id, th.BasicUser.Id, "channel_admin channel_user")
 	require.NoError(t, err)
-	th.Client.Logout(context.Background())
+	CheckCreatedStatus(t, response)
-	th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+	assert.NotNil(t, groupSyncable)
 
-	_, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+	t.Run("Error if no license is installed", func(t *testing.T) {
-	assert.NoError(t, err)
+		th.App.Srv().SetLicense(nil)
+		t.Cleanup(func() { th.App.Srv().SetLicense(model.NewTestLicense("ldap")) })
+
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		require.Error(t, err)
+		CheckNotImplementedStatus(t, response)
+
+		response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		require.Error(t, err)
+		CheckNotImplementedStatus(t, response)
+	})
+
+	t.Run("Normal users are not allowed to unlink", func(t *testing.T) {
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		assert.Error(t, err)
+		CheckForbiddenStatus(t, response)
+	})
+
+	th.MakeUserChannelAdmin(th.BasicUser, th.BasicChannel)
+
+	response, err = th.Client.Logout(context.Background())
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+	_, response, err = th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
+	require.NoError(t, err)
+	CheckOKStatus(t, response)
+
+	t.Run("Team admins are not allowed to link", func(t *testing.T) {
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		require.Error(t, err)
+		CheckForbiddenStatus(t, response)
+	})
+
+	t.Run("Team admins are allowed to unlink if AllowReference is enabled", func(t *testing.T) {
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		require.NoError(t, err)
+		CheckOKStatus(t, response)
+
+		t.Cleanup(func() {
+			groupSyncable, response, err = th.Client.LinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch)
+			require.NoError(t, err)
+			CheckCreatedStatus(t, response)
+			assert.NotNil(t, groupSyncable)
+		})
+	})
+
+	t.Run("System admins are allowed to unlink", func(t *testing.T) {
+		response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), gRef.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		require.NoError(t, err)
+		CheckOKStatus(t, response)
+	})
+
+	t.Run("Custom groups can't get unlinked", func(t *testing.T) {
+		gid := model.NewId()
+		g2, appErr := th.App.CreateGroup(&model.Group{
+			DisplayName: "dn_" + gid,
+			Name:        model.NewString("name" + gid),
+			Source:      model.GroupSourceCustom,
+			Description: "description_" + gid,
+			RemoteId:    model.NewString(model.NewId()),
+		})
+		assert.Nil(t, appErr)
+
+		response, err = th.Client.UnlinkGroupSyncable(context.Background(), g2.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel)
+		require.Error(t, err)
+		CheckBadRequestStatus(t, response)
+	})
 }
 
 func TestGetGroupTeam(t *testing.T) {