High severity8.8NVD Advisory· Published Jun 12, 2026
CVE-2026-7387
CVE-2026-7387
Description
Mattermost fails to enforce role-management authorization on group syncable endpoints, allowing privilege escalation to team or channel admin.
AI Insight
LLM-synthesized narrative grounded in this CVE's description and references.
Mattermost fails to enforce role-management authorization on group syncable endpoints, allowing privilege escalation to team or channel admin.
Vulnerability
Mattermost versions 11.6.0 through 11.6.1, 11.5.0 through 11.5.4, 10.11.0 through 10.11.15, and 10.11.16 fail to require role-management authorization when setting the scheme_admin flag on group syncable link and patch endpoints. This allows a user with group-link permissions to craft API requests that escalate themselves and group members to team or channel admin privileges. The advisory ID is MMSA-2026-00665 [1].
Exploitation
An attacker needs to be an authenticated user with group-link permissions in Mattermost. The attacker can send a crafted API request to the affected endpoints, setting the scheme_admin flag without proper authorization checks. No additional user interaction or race condition is required [1].
Impact
Successful exploitation leads to unauthorized privilege escalation. The attacker can grant themselves or other group members team or channel admin roles, gaining elevated permissions such as modifying channel settings, managing members, and accessing sensitive information in those teams or channels [1].
Mitigation
Mattermost has released security updates. Users should upgrade to versions 11.6.2, 11.5.5, or 10.11.17 as applicable [1]. No workarounds are documented in the available references.
References
AI Insight generated on Jun 12, 2026. Synthesized from this CVE's description and the cited reference URLs; citations are validated against the source bundle.
Affected products
1- Range: <=11.6.1, <=11.5.4, <=10.11.16
Patches
3d5f29c8ebbebMM-68547: Tighten authorization on group syncable link and patch endpoints (#36316) (#36432)
8 files changed · +1006 −23
e2e-tests/cypress/tests/integration/channels/enterprise/system_console/group_configuration_spec.js+6 −2 modified@@ -394,8 +394,10 @@ describe('group configuration', () => { // # Save settings savePage(); - // * Check the groupteam via the API to ensure its role wasn't updated + // * Check the groupteam via the API to ensure the team was + // removed (delete_at != 0) and its role wasn't updated. cy.apiGetGroupTeam(groupID, testTeam.id).then(({body}) => { + expect(body.delete_at).to.not.eq(0); expect(body.scheme_admin).to.eq(false); }); }); @@ -519,8 +521,10 @@ describe('group configuration', () => { // # Save settings savePage(); - // * Check the groupteam via the API to ensure its role wasn't updated + // * Check the groupteam via the API to ensure the channel was + // removed (delete_at != 0) and its role wasn't updated. cy.apiGetGroupChannel(groupID, testChannel.id).then(({body}) => { + expect(body.delete_at).to.not.eq(0); expect(body.scheme_admin).to.eq(false); }); });
server/channels/api4/group.go+68 −6 modified@@ -372,10 +372,35 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } - groupSyncable := &model.GroupSyncable{ - GroupId: c.Params.GroupId, - SyncableId: syncableID, - Type: syncableType, + appErr = verifySchemeAdminAssignmentPermission(c, syncableType, syncableID, patch) + if appErr != nil { + appErr.Where = "Api4.linkGroupSyncable" + c.Err = appErr + return + } + + // Upsert onto the existing row only when it is currently active so + // unspecified fields are preserved. A fresh link, or a re-link of a + // soft-deleted row, starts from a zero-value struct so that fields + // the caller did not (or was not authorized to) set are not carried + // over from the previous incarnation. The downstream upsert clears + // DeleteAt when re-activating. + existing, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) + if appErr != nil && appErr.StatusCode != http.StatusNotFound { + appErr.Where = "Api4.linkGroupSyncable" + c.Err = appErr + return + } + + var groupSyncable *model.GroupSyncable + if existing != nil && existing.DeleteAt == 0 { + groupSyncable = existing + } else { + groupSyncable = &model.GroupSyncable{ + GroupId: c.Params.GroupId, + SyncableId: syncableID, + Type: syncableType, + } } groupSyncable.Patch(patch) groupSyncable, appErr = c.App.UpsertGroupSyncable(groupSyncable) @@ -387,8 +412,9 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventResultState(groupSyncable) auditRec.AddEventObjectType("group_syncable") + syncRoles := patch.SchemeAdmin != nil c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId) + c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId, syncRoles) }) w.WriteHeader(http.StatusCreated) @@ -555,6 +581,13 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } + appErr = verifySchemeAdminAssignmentPermission(c, syncableType, syncableID, patch) + if appErr != nil { + appErr.Where = "Api4.patchGroupSyncable" + c.Err = appErr + return + } + groupSyncable, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) if appErr != nil { c.Err = appErr @@ -572,8 +605,9 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventResultState(groupSyncable) auditRec.AddEventObjectType("group_syncable") + syncRoles := patch.SchemeAdmin != nil c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId) + c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId, syncRoles) }) b, err := json.Marshal(groupSyncable) @@ -705,6 +739,34 @@ func verifyLinkUnlinkPermission(c *Context, syncableType model.GroupSyncableType return nil } +// verifySchemeAdminAssignmentPermission requires the caller to hold the +// role-management permission for the target syncable +// (manage_team_roles / manage_channel_roles), or the sysconsole groups +// write permission, before an explicit SchemeAdmin value in the patch is +// accepted. A nil patch.SchemeAdmin is a no-op. +func verifySchemeAdminAssignmentPermission(c *Context, syncableType model.GroupSyncableType, syncableID string, patch *model.GroupSyncablePatch) *model.AppError { + if patch == nil || patch.SchemeAdmin == nil { + return nil + } + + if c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementGroups) { + return nil + } + + switch syncableType { + case model.GroupSyncableTypeTeam: + if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionManageTeamRoles) { + return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageTeamRoles}) + } + case model.GroupSyncableTypeChannel: + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), syncableID, model.PermissionManageChannelRoles); !ok { + return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageChannelRoles}) + } + } + + return nil +} + func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { permissionErr := requireLicense(c) if permissionErr != nil {
server/channels/api4/group_test.go+733 −0 modified@@ -3002,3 +3002,736 @@ func TestDeleteMembersFromGroup(t *testing.T) { CheckBadRequestStatus(t, response) }) } + +// newSchemeAdminTestLdapGroup creates a fresh LDAP-source group with +// AllowReference=true. +func newSchemeAdminTestLdapGroup(t *testing.T, th *TestHelper) *model.Group { + t.Helper() + id := model.NewId() + g, appErr := th.App.CreateGroup(&model.Group{ + DisplayName: "dn_" + id, + Name: model.NewPointer("name" + id), + Source: model.GroupSourceLdap, + Description: "description_" + id, + RemoteId: model.NewPointer(model.NewId()), + AllowReference: true, + }) + require.Nil(t, appErr) + return g +} + +// findPersistedGroupSyncable returns the persisted GroupSyncable for a +// given (groupID, syncableID, syncableType) tuple, including SchemeAdmin. +func findPersistedGroupSyncable(t *testing.T, th *TestHelper, groupID, syncableID string, syncableType model.GroupSyncableType) *model.GroupSyncable { + t.Helper() + syncables, appErr := th.App.GetGroupSyncables(groupID, syncableType) + require.Nil(t, appErr) + for _, s := range syncables { + if s.SyncableId == syncableID { + return s + } + } + return nil +} + +func TestLinkGroupTeam_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + schemeAdminTrue := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + } + + t.Run("regular team user with invite_user must NOT be able to set scheme_admin: true", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + assert.Nil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) + + t.Run("system admin can still set scheme_admin: true", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user can still link with scheme_admin omitted", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.NotNil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user must NOT be able to link with scheme_admin: false explicitly", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + 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) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) +} + +func TestLinkGroupChannel_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // A regular user can only link a channel syncable when the group is + // already linked to the parent team, so seed the team link as sysadmin. + mkLinkedGroup := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + } + + t.Run("regular channel user with manage_*_channel_members must NOT be able to set scheme_admin: true", func(t *testing.T) { + g := mkLinkedGroup(t) + + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + assert.Nil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) + + t.Run("system admin can still set scheme_admin: true", func(t *testing.T) { + g := mkLinkedGroup(t) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user can still link with scheme_admin omitted", func(t *testing.T) { + g := mkLinkedGroup(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.NotNil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user must NOT be able to link with scheme_admin: false explicitly", func(t *testing.T) { + g := mkLinkedGroup(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + 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) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) +} + +func TestPatchGroupTeam_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // schemeAdmin controls the seeded SchemeAdmin value on the team syncable. + setupLinkedGroup := func(t *testing.T, schemeAdmin bool) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(schemeAdmin), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(true), + } + + schemeAdminFalse := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(false), + } + + t.Run("regular team user with invite_user must NOT be able to patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user can still patch other fields with scheme_admin omitted", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.AutoAdd) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user must NOT be able to patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("sysconsole_write_user_management_groups holder can patch scheme_admin in either direction", func(t *testing.T) { + // system_manager bundles sysconsole_write_user_management_groups, + // the override honoured by verifySchemeAdminAssignmentPermission. + th.LoginSystemManager(t) + + gPromote := setupLinkedGroup(t, false) + _, response, err := th.SystemManagerClient.PatchGroupSyncable(context.Background(), gPromote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedPromote := findPersistedGroupSyncable(t, th, gPromote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persistedPromote) + assert.True(t, persistedPromote.SchemeAdmin) + + gDemote := setupLinkedGroup(t, true) + _, response, err = th.SystemManagerClient.PatchGroupSyncable(context.Background(), gDemote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedDemote := findPersistedGroupSyncable(t, th, gDemote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persistedDemote) + assert.False(t, persistedDemote.SchemeAdmin) + }) +} + +func TestPatchGroupChannel_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // schemeAdmin controls the seeded SchemeAdmin value on the channel + // syncable. The team syncable is seeded so the channel link succeeds. + setupLinkedGroup := func(t *testing.T, schemeAdmin bool) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(schemeAdmin), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(true), + } + + schemeAdminFalse := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(false), + } + + t.Run("regular channel user with manage_*_channel_members must NOT be able to patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user can still patch other fields with scheme_admin omitted", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.AutoAdd) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user must NOT be able to patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("sysconsole_write_user_management_groups holder can patch scheme_admin in either direction", func(t *testing.T) { + // system_manager bundles sysconsole_write_user_management_groups, + // the override honoured by verifySchemeAdminAssignmentPermission. + th.LoginSystemManager(t) + + gPromote := setupLinkedGroup(t, false) + _, response, err := th.SystemManagerClient.PatchGroupSyncable(context.Background(), gPromote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedPromote := findPersistedGroupSyncable(t, th, gPromote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persistedPromote) + assert.True(t, persistedPromote.SchemeAdmin) + + gDemote := setupLinkedGroup(t, true) + _, response, err = th.SystemManagerClient.PatchGroupSyncable(context.Background(), gDemote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedDemote := findPersistedGroupSyncable(t, th, gDemote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persistedDemote) + assert.False(t, persistedDemote.SchemeAdmin) + }) +} + +func TestLinkGroupTeam_LinkOnExistingPreservesSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + seedSchemeAdminTrue := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + require.True(t, persisted.SchemeAdmin) + return g + } + + t.Run("regular team user calling LINK with scheme_admin omitted must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user calling LINK with scheme_admin: false must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) +} + +func TestLinkGroupChannel_LinkOnExistingPreservesSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + seedSchemeAdminTrue := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + require.True(t, persisted.SchemeAdmin) + return g + } + + t.Run("regular channel user calling LINK with scheme_admin omitted must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user calling LINK with scheme_admin: false must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) +} + +func TestLinkGroupTeam_LinkOnSoftDeletedDoesNotPreserveSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + t.Run("regular team user re-linking a soft-deleted syncable with scheme_admin omitted must persist scheme_admin: false", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NoError(t, err) + CheckOKStatus(t, response) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) +} + +func TestPatchGroupTeam_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.UpdateUserToTeamAdmin(t, th.BasicUser2, th.BasicTeam) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err = th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestPatchGroupChannel_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.MakeUserChannelAdmin(t, th.BasicUser2, th.BasicChannel) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err = th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + time.Sleep(2 * time.Second) + + cm, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) +} + +func TestLinkGroupTeam_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + th.UpdateUserToTeamAdmin(t, th.BasicUser2, th.BasicTeam) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestLinkGroupChannel_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.MakeUserChannelAdmin(t, th.BasicUser2, th.BasicChannel) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + cm, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) +} + +func TestLinkGroupTeam_SchemeAdminTruePromotesGroupMembers(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, appErr := th.App.UpsertGroupMember(g.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestLinkGroupTeam_AutoAddOnlyAddsGroupMembers(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + newUser := th.CreateUser(t) + _, appErr := th.App.UpsertGroupMember(g.Id, newUser.Id) + require.Nil(t, appErr) + + _, appErr = th.App.GetTeamMember(th.Context, th.BasicTeam.Id, newUser.Id) + require.NotNil(t, appErr) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, newUser.Id) + require.Nil(t, appErr) + assert.Equal(t, newUser.Id, tm.UserId) +}
server/channels/app/syncables.go+8 −6 modified@@ -268,18 +268,20 @@ func (a *App) SyncSyncableRoles(rctx request.CTX, syncableID string, syncableTyp return nil } -// SyncRolesAndMembership updates the SchemeAdmin status and membership of all of the members of the given -// syncable. -func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, groupID string) { +// SyncRolesAndMembership updates the membership of the given syncable and, +// when syncRoles is true, also reconciles SchemeAdmin status for its members. +func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, groupID string, syncRoles bool) { group, appErr := a.GetGroup(groupID, nil, nil) if appErr != nil { rctx.Logger().Warn("Error getting group", mlog.Err(appErr)) return } - appErr = a.SyncSyncableRoles(rctx, syncableID, syncableType) - if appErr != nil { - rctx.Logger().Warn("Error syncing syncable roles", mlog.Err(appErr)) + if syncRoles { + appErr = a.SyncSyncableRoles(rctx, syncableID, syncableType) + if appErr != nil { + rctx.Logger().Warn("Error syncing syncable roles", mlog.Err(appErr)) + } } var since int64
server/channels/app/syncables_test.go+144 −0 modified@@ -674,3 +674,147 @@ func TestSyncSyncableRoles(t *testing.T) { require.True(t, cm.SchemeAdmin) } } + +func TestSyncRolesAndMembership_RoleSyncGate(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + setup := func(t *testing.T) (*model.Team, *model.Channel, *model.Group, *model.User) { + t.Helper() + + team := th.CreateTeam(t) + channel := th.CreateChannel(t, team) + group := th.CreateGroup(t) + + _, err := th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: team.Id, + Type: model.GroupSyncableTypeTeam, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: channel.Id, + Type: model.GroupSyncableTypeChannel, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + directAdmin := th.CreateUser(t) + _, appErr := th.App.AddTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, directAdmin, channel, false) + require.Nil(t, appErr) + + tm, storeErr := th.App.Srv().Store().Team().GetMember(th.Context, team.Id, directAdmin.Id) + require.NoError(t, storeErr) + tm.SchemeAdmin = true + _, storeErr = th.App.Srv().Store().Team().UpdateMember(th.Context, tm) + require.NoError(t, storeErr) + + cm, storeErr := th.App.Srv().Store().Channel().GetMember(th.Context, channel.Id, directAdmin.Id) + require.NoError(t, storeErr) + cm.SchemeAdmin = true + _, storeErr = th.App.Srv().Store().Channel().UpdateMember(th.Context, cm) + require.NoError(t, storeErr) + + return team, channel, group, directAdmin + } + + t.Run("syncRoles=false preserves the existing SchemeAdmin on team members", func(t *testing.T) { + team, _, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, false) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) + }) + + t.Run("syncRoles=false preserves the existing SchemeAdmin on channel members", func(t *testing.T) { + _, channel, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, false) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) + }) + + t.Run("syncRoles=true reconciles team SchemeAdmin against PermittedSyncableAdmins", func(t *testing.T) { + team, _, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, true) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.False(t, tm.SchemeAdmin) + }) + + t.Run("syncRoles=true reconciles channel SchemeAdmin against PermittedSyncableAdmins", func(t *testing.T) { + _, channel, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, true) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.False(t, cm.SchemeAdmin) + }) +} + +func TestSyncRolesAndMembership_AlwaysSyncsMembership(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + setup := func(t *testing.T) (*model.Team, *model.Channel, *model.Group, *model.User) { + t.Helper() + + team := th.CreateTeam(t) + channel := th.CreateChannel(t, team) + group := th.CreateGroup(t) + + _, err := th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: team.Id, + Type: model.GroupSyncableTypeTeam, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: channel.Id, + Type: model.GroupSyncableTypeChannel, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + groupMember := th.CreateUser(t) + _, err = th.App.UpsertGroupMember(group.Id, groupMember.Id) + require.Nil(t, err) + + return team, channel, group, groupMember + } + + t.Run("syncRoles=false still adds group members to the team", func(t *testing.T) { + team, _, group, groupMember := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, false) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, groupMember.Id) + require.Nil(t, appErr) + assert.Equal(t, groupMember.Id, tm.UserId) + }) + + t.Run("syncRoles=false still adds group members to the channel", func(t *testing.T) { + _, channel, group, groupMember := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, false) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, groupMember.Id) + require.Nil(t, appErr) + assert.Equal(t, groupMember.Id, cm.UserId) + }) +}
server/channels/store/sqlstore/group_store.go+3 −1 modified@@ -791,6 +791,7 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync groupSyncable.DeleteAt = groupTeam.DeleteAt groupSyncable.UpdateAt = groupTeam.UpdateAt groupSyncable.Type = syncableType + groupSyncable.SchemeAdmin = groupTeam.SchemeAdmin case model.GroupSyncableTypeChannel: groupChannel := result.(*groupChannel) groupSyncable.SyncableId = groupChannel.ChannelId @@ -800,6 +801,7 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync groupSyncable.DeleteAt = groupChannel.DeleteAt groupSyncable.UpdateAt = groupChannel.UpdateAt groupSyncable.Type = syncableType + groupSyncable.SchemeAdmin = groupChannel.SchemeAdmin default: return nil, fmt.Errorf("unable to convert syncableType: %s", syncableType.String()) } @@ -1829,7 +1831,7 @@ func (s *SqlGroupStore) AdminRoleGroupsForSyncableMember(userID, syncableID stri func (s *SqlGroupStore) PermittedSyncableAdmins(syncableID string, syncableType model.GroupSyncableType) ([]string, error) { builder := s.getQueryBuilder().Select("UserId"). From(fmt.Sprintf("Group%ss", syncableType)). - Join(fmt.Sprintf("GroupMembers ON GroupMembers.GroupId = Group%ss.GroupId AND Group%[1]ss.SchemeAdmin = TRUE AND GroupMembers.DeleteAt = 0", syncableType.String())).Where(fmt.Sprintf("Group%[1]ss.%[1]sId = ?", syncableType.String()), syncableID) + Join(fmt.Sprintf("GroupMembers ON GroupMembers.GroupId = Group%ss.GroupId AND Group%[1]ss.SchemeAdmin = TRUE AND Group%[1]ss.DeleteAt = 0 AND GroupMembers.DeleteAt = 0", syncableType.String())).Where(fmt.Sprintf("Group%[1]ss.%[1]sId = ?", syncableType.String()), syncableID) var userIDs []string if err := s.GetMaster().SelectBuilder(&userIDs, builder); err != nil {
server/channels/store/storetest/group_store.go+28 −0 modified@@ -1616,9 +1616,19 @@ func testGetGroupSyncable(t *testing.T, rctx request.CTX, ss store.Store) { require.Equal(t, gt1.GroupId, dgt.GroupId) require.Equal(t, gt1.SyncableId, dgt.SyncableId) require.Equal(t, gt1.AutoAdd, dgt.AutoAdd) + require.Equal(t, gt1.SchemeAdmin, dgt.SchemeAdmin) require.NotZero(t, gt1.CreateAt) require.NotZero(t, gt1.UpdateAt) require.Zero(t, gt1.DeleteAt) + + // Round-trip SchemeAdmin: true through UpdateGroupSyncable and re-fetch. + dgt.SchemeAdmin = true + _, err = ss.Group().UpdateGroupSyncable(dgt) + require.NoError(t, err) + + dgt, err = ss.Group().GetGroupSyncable(groupTeam.GroupId, groupTeam.SyncableId, model.GroupSyncableTypeTeam) + require.NoError(t, err) + require.True(t, dgt.SchemeAdmin, "GetGroupSyncable must populate SchemeAdmin from the persisted row") } func testGetGroupSyncableErrors(t *testing.T, rctx request.CTX, ss store.Store) { @@ -4979,6 +4989,15 @@ func groupTestPermittedSyncableAdminsTeam(t *testing.T, rctx request.CTX, ss sto // deleted group syncable no longer includes group members _, err = ss.Group().DeleteGroupSyncable(group1.Id, team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) + + // The persisted row must still carry SchemeAdmin=true after soft-delete; + // PermittedSyncableAdmins excludes it via the DeleteAt = 0 predicate, not + // via the field having been silently cleared. + deletedSyncable, err := ss.Group().GetGroupSyncable(group1.Id, team.Id, model.GroupSyncableTypeTeam) + require.NoError(t, err) + require.True(t, deletedSyncable.SchemeAdmin) + require.NotZero(t, deletedSyncable.DeleteAt) + actualUserIDs, err = ss.Group().PermittedSyncableAdmins(team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) require.ElementsMatch(t, []string{user3.Id}, actualUserIDs) @@ -5086,6 +5105,15 @@ func groupTestPermittedSyncableAdminsChannel(t *testing.T, rctx request.CTX, ss // deleted group syncable no longer includes group members _, err = ss.Group().DeleteGroupSyncable(group1.Id, channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) + + // The persisted row must still carry SchemeAdmin=true after soft-delete; + // PermittedSyncableAdmins excludes it via the DeleteAt = 0 predicate, not + // via the field having been silently cleared. + deletedSyncable, err := ss.Group().GetGroupSyncable(group1.Id, channel.Id, model.GroupSyncableTypeChannel) + require.NoError(t, err) + require.True(t, deletedSyncable.SchemeAdmin) + require.NotZero(t, deletedSyncable.DeleteAt) + actualUserIDs, err = ss.Group().PermittedSyncableAdmins(channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) require.ElementsMatch(t, []string{user3.Id}, actualUserIDs)
webapp/channels/src/components/admin_console/group_settings/group_details/group_details.tsx+16 −8 modified@@ -417,17 +417,25 @@ class GroupDetails extends React.PureComponent<Props, State> { roleChangeKey = (groupTeamOrChannel: { type?: SyncableType; + id?: string; team_id?: string; channel_id?: string; }) => { - let id; - if ( - this.syncableTypeFromEntryType(groupTeamOrChannel.type) === - SyncableType.Team - ) { - id = groupTeamOrChannel.team_id; - } else { - id = groupTeamOrChannel.channel_id; + // Items in itemsToRemove use a generic `id`, while items coming from + // teamsToAdd/channelsToAdd use `team_id`/`channel_id`. The key must + // be identical regardless of source so the dedup in + // handleRemovedTeamsAndChannels and handleAddedTeamsAndChannels + // matches the key produced by onChangeRoles. + let id = groupTeamOrChannel.id; + if (!id) { + if ( + this.syncableTypeFromEntryType(groupTeamOrChannel.type) === + SyncableType.Team + ) { + id = groupTeamOrChannel.team_id; + } else { + id = groupTeamOrChannel.channel_id; + } } return `${id}/${groupTeamOrChannel.type}`; };
1ce2484a00c9MM-68547: Tighten authorization on group syncable link and patch endpoints (#36316) (#36431)
8 files changed · +1006 −23
e2e-tests/cypress/tests/integration/channels/enterprise/system_console/group_configuration_spec.js+6 −2 modified@@ -394,8 +394,10 @@ describe('group configuration', () => { // # Save settings savePage(); - // * Check the groupteam via the API to ensure its role wasn't updated + // * Check the groupteam via the API to ensure the team was + // removed (delete_at != 0) and its role wasn't updated. cy.apiGetGroupTeam(groupID, testTeam.id).then(({body}) => { + expect(body.delete_at).to.not.eq(0); expect(body.scheme_admin).to.eq(false); }); }); @@ -519,8 +521,10 @@ describe('group configuration', () => { // # Save settings savePage(); - // * Check the groupteam via the API to ensure its role wasn't updated + // * Check the groupteam via the API to ensure the channel was + // removed (delete_at != 0) and its role wasn't updated. cy.apiGetGroupChannel(groupID, testChannel.id).then(({body}) => { + expect(body.delete_at).to.not.eq(0); expect(body.scheme_admin).to.eq(false); }); });
server/channels/api4/group.go+68 −6 modified@@ -372,10 +372,35 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } - groupSyncable := &model.GroupSyncable{ - GroupId: c.Params.GroupId, - SyncableId: syncableID, - Type: syncableType, + appErr = verifySchemeAdminAssignmentPermission(c, syncableType, syncableID, patch) + if appErr != nil { + appErr.Where = "Api4.linkGroupSyncable" + c.Err = appErr + return + } + + // Upsert onto the existing row only when it is currently active so + // unspecified fields are preserved. A fresh link, or a re-link of a + // soft-deleted row, starts from a zero-value struct so that fields + // the caller did not (or was not authorized to) set are not carried + // over from the previous incarnation. The downstream upsert clears + // DeleteAt when re-activating. + existing, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) + if appErr != nil && appErr.StatusCode != http.StatusNotFound { + appErr.Where = "Api4.linkGroupSyncable" + c.Err = appErr + return + } + + var groupSyncable *model.GroupSyncable + if existing != nil && existing.DeleteAt == 0 { + groupSyncable = existing + } else { + groupSyncable = &model.GroupSyncable{ + GroupId: c.Params.GroupId, + SyncableId: syncableID, + Type: syncableType, + } } groupSyncable.Patch(patch) groupSyncable, appErr = c.App.UpsertGroupSyncable(groupSyncable) @@ -387,8 +412,9 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventResultState(groupSyncable) auditRec.AddEventObjectType("group_syncable") + syncRoles := patch.SchemeAdmin != nil c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId) + c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId, syncRoles) }) w.WriteHeader(http.StatusCreated) @@ -555,6 +581,13 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } + appErr = verifySchemeAdminAssignmentPermission(c, syncableType, syncableID, patch) + if appErr != nil { + appErr.Where = "Api4.patchGroupSyncable" + c.Err = appErr + return + } + groupSyncable, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) if appErr != nil { c.Err = appErr @@ -572,8 +605,9 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventResultState(groupSyncable) auditRec.AddEventObjectType("group_syncable") + syncRoles := patch.SchemeAdmin != nil c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId) + c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId, syncRoles) }) b, err := json.Marshal(groupSyncable) @@ -705,6 +739,34 @@ func verifyLinkUnlinkPermission(c *Context, syncableType model.GroupSyncableType return nil } +// verifySchemeAdminAssignmentPermission requires the caller to hold the +// role-management permission for the target syncable +// (manage_team_roles / manage_channel_roles), or the sysconsole groups +// write permission, before an explicit SchemeAdmin value in the patch is +// accepted. A nil patch.SchemeAdmin is a no-op. +func verifySchemeAdminAssignmentPermission(c *Context, syncableType model.GroupSyncableType, syncableID string, patch *model.GroupSyncablePatch) *model.AppError { + if patch == nil || patch.SchemeAdmin == nil { + return nil + } + + if c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementGroups) { + return nil + } + + switch syncableType { + case model.GroupSyncableTypeTeam: + if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionManageTeamRoles) { + return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageTeamRoles}) + } + case model.GroupSyncableTypeChannel: + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), syncableID, model.PermissionManageChannelRoles); !ok { + return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageChannelRoles}) + } + } + + return nil +} + func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { permissionErr := requireLicense(c) if permissionErr != nil {
server/channels/api4/group_test.go+733 −0 modified@@ -3002,3 +3002,736 @@ func TestDeleteMembersFromGroup(t *testing.T) { CheckBadRequestStatus(t, response) }) } + +// newSchemeAdminTestLdapGroup creates a fresh LDAP-source group with +// AllowReference=true. +func newSchemeAdminTestLdapGroup(t *testing.T, th *TestHelper) *model.Group { + t.Helper() + id := model.NewId() + g, appErr := th.App.CreateGroup(&model.Group{ + DisplayName: "dn_" + id, + Name: model.NewPointer("name" + id), + Source: model.GroupSourceLdap, + Description: "description_" + id, + RemoteId: model.NewPointer(model.NewId()), + AllowReference: true, + }) + require.Nil(t, appErr) + return g +} + +// findPersistedGroupSyncable returns the persisted GroupSyncable for a +// given (groupID, syncableID, syncableType) tuple, including SchemeAdmin. +func findPersistedGroupSyncable(t *testing.T, th *TestHelper, groupID, syncableID string, syncableType model.GroupSyncableType) *model.GroupSyncable { + t.Helper() + syncables, appErr := th.App.GetGroupSyncables(groupID, syncableType) + require.Nil(t, appErr) + for _, s := range syncables { + if s.SyncableId == syncableID { + return s + } + } + return nil +} + +func TestLinkGroupTeam_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + schemeAdminTrue := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + } + + t.Run("regular team user with invite_user must NOT be able to set scheme_admin: true", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + assert.Nil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) + + t.Run("system admin can still set scheme_admin: true", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user can still link with scheme_admin omitted", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.NotNil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user must NOT be able to link with scheme_admin: false explicitly", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + 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) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) +} + +func TestLinkGroupChannel_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // A regular user can only link a channel syncable when the group is + // already linked to the parent team, so seed the team link as sysadmin. + mkLinkedGroup := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + } + + t.Run("regular channel user with manage_*_channel_members must NOT be able to set scheme_admin: true", func(t *testing.T) { + g := mkLinkedGroup(t) + + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + assert.Nil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) + + t.Run("system admin can still set scheme_admin: true", func(t *testing.T) { + g := mkLinkedGroup(t) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user can still link with scheme_admin omitted", func(t *testing.T) { + g := mkLinkedGroup(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.NotNil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user must NOT be able to link with scheme_admin: false explicitly", func(t *testing.T) { + g := mkLinkedGroup(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + 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) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) +} + +func TestPatchGroupTeam_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // schemeAdmin controls the seeded SchemeAdmin value on the team syncable. + setupLinkedGroup := func(t *testing.T, schemeAdmin bool) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(schemeAdmin), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(true), + } + + schemeAdminFalse := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(false), + } + + t.Run("regular team user with invite_user must NOT be able to patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user can still patch other fields with scheme_admin omitted", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.AutoAdd) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user must NOT be able to patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("sysconsole_write_user_management_groups holder can patch scheme_admin in either direction", func(t *testing.T) { + // system_manager bundles sysconsole_write_user_management_groups, + // the override honoured by verifySchemeAdminAssignmentPermission. + th.LoginSystemManager(t) + + gPromote := setupLinkedGroup(t, false) + _, response, err := th.SystemManagerClient.PatchGroupSyncable(context.Background(), gPromote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedPromote := findPersistedGroupSyncable(t, th, gPromote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persistedPromote) + assert.True(t, persistedPromote.SchemeAdmin) + + gDemote := setupLinkedGroup(t, true) + _, response, err = th.SystemManagerClient.PatchGroupSyncable(context.Background(), gDemote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedDemote := findPersistedGroupSyncable(t, th, gDemote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persistedDemote) + assert.False(t, persistedDemote.SchemeAdmin) + }) +} + +func TestPatchGroupChannel_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // schemeAdmin controls the seeded SchemeAdmin value on the channel + // syncable. The team syncable is seeded so the channel link succeeds. + setupLinkedGroup := func(t *testing.T, schemeAdmin bool) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(schemeAdmin), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(true), + } + + schemeAdminFalse := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(false), + } + + t.Run("regular channel user with manage_*_channel_members must NOT be able to patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user can still patch other fields with scheme_admin omitted", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.AutoAdd) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user must NOT be able to patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("sysconsole_write_user_management_groups holder can patch scheme_admin in either direction", func(t *testing.T) { + // system_manager bundles sysconsole_write_user_management_groups, + // the override honoured by verifySchemeAdminAssignmentPermission. + th.LoginSystemManager(t) + + gPromote := setupLinkedGroup(t, false) + _, response, err := th.SystemManagerClient.PatchGroupSyncable(context.Background(), gPromote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedPromote := findPersistedGroupSyncable(t, th, gPromote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persistedPromote) + assert.True(t, persistedPromote.SchemeAdmin) + + gDemote := setupLinkedGroup(t, true) + _, response, err = th.SystemManagerClient.PatchGroupSyncable(context.Background(), gDemote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedDemote := findPersistedGroupSyncable(t, th, gDemote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persistedDemote) + assert.False(t, persistedDemote.SchemeAdmin) + }) +} + +func TestLinkGroupTeam_LinkOnExistingPreservesSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + seedSchemeAdminTrue := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + require.True(t, persisted.SchemeAdmin) + return g + } + + t.Run("regular team user calling LINK with scheme_admin omitted must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user calling LINK with scheme_admin: false must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) +} + +func TestLinkGroupChannel_LinkOnExistingPreservesSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + seedSchemeAdminTrue := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + require.True(t, persisted.SchemeAdmin) + return g + } + + t.Run("regular channel user calling LINK with scheme_admin omitted must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user calling LINK with scheme_admin: false must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) +} + +func TestLinkGroupTeam_LinkOnSoftDeletedDoesNotPreserveSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + t.Run("regular team user re-linking a soft-deleted syncable with scheme_admin omitted must persist scheme_admin: false", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NoError(t, err) + CheckOKStatus(t, response) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) +} + +func TestPatchGroupTeam_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.UpdateUserToTeamAdmin(t, th.BasicUser2, th.BasicTeam) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err = th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestPatchGroupChannel_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.MakeUserChannelAdmin(t, th.BasicUser2, th.BasicChannel) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err = th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + time.Sleep(2 * time.Second) + + cm, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) +} + +func TestLinkGroupTeam_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + th.UpdateUserToTeamAdmin(t, th.BasicUser2, th.BasicTeam) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestLinkGroupChannel_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.MakeUserChannelAdmin(t, th.BasicUser2, th.BasicChannel) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + cm, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) +} + +func TestLinkGroupTeam_SchemeAdminTruePromotesGroupMembers(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, appErr := th.App.UpsertGroupMember(g.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestLinkGroupTeam_AutoAddOnlyAddsGroupMembers(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + newUser := th.CreateUser(t) + _, appErr := th.App.UpsertGroupMember(g.Id, newUser.Id) + require.Nil(t, appErr) + + _, appErr = th.App.GetTeamMember(th.Context, th.BasicTeam.Id, newUser.Id) + require.NotNil(t, appErr) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, newUser.Id) + require.Nil(t, appErr) + assert.Equal(t, newUser.Id, tm.UserId) +}
server/channels/app/syncables.go+8 −6 modified@@ -268,18 +268,20 @@ func (a *App) SyncSyncableRoles(rctx request.CTX, syncableID string, syncableTyp return nil } -// SyncRolesAndMembership updates the SchemeAdmin status and membership of all of the members of the given -// syncable. -func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, groupID string) { +// SyncRolesAndMembership updates the membership of the given syncable and, +// when syncRoles is true, also reconciles SchemeAdmin status for its members. +func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, groupID string, syncRoles bool) { group, appErr := a.GetGroup(groupID, nil, nil) if appErr != nil { rctx.Logger().Warn("Error getting group", mlog.Err(appErr)) return } - appErr = a.SyncSyncableRoles(rctx, syncableID, syncableType) - if appErr != nil { - rctx.Logger().Warn("Error syncing syncable roles", mlog.Err(appErr)) + if syncRoles { + appErr = a.SyncSyncableRoles(rctx, syncableID, syncableType) + if appErr != nil { + rctx.Logger().Warn("Error syncing syncable roles", mlog.Err(appErr)) + } } var since int64
server/channels/app/syncables_test.go+144 −0 modified@@ -674,3 +674,147 @@ func TestSyncSyncableRoles(t *testing.T) { require.True(t, cm.SchemeAdmin) } } + +func TestSyncRolesAndMembership_RoleSyncGate(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + setup := func(t *testing.T) (*model.Team, *model.Channel, *model.Group, *model.User) { + t.Helper() + + team := th.CreateTeam(t) + channel := th.CreateChannel(t, team) + group := th.CreateGroup(t) + + _, err := th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: team.Id, + Type: model.GroupSyncableTypeTeam, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: channel.Id, + Type: model.GroupSyncableTypeChannel, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + directAdmin := th.CreateUser(t) + _, appErr := th.App.AddTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, directAdmin, channel, false) + require.Nil(t, appErr) + + tm, storeErr := th.App.Srv().Store().Team().GetMember(th.Context, team.Id, directAdmin.Id) + require.NoError(t, storeErr) + tm.SchemeAdmin = true + _, storeErr = th.App.Srv().Store().Team().UpdateMember(th.Context, tm) + require.NoError(t, storeErr) + + cm, storeErr := th.App.Srv().Store().Channel().GetMember(th.Context, channel.Id, directAdmin.Id) + require.NoError(t, storeErr) + cm.SchemeAdmin = true + _, storeErr = th.App.Srv().Store().Channel().UpdateMember(th.Context, cm) + require.NoError(t, storeErr) + + return team, channel, group, directAdmin + } + + t.Run("syncRoles=false preserves the existing SchemeAdmin on team members", func(t *testing.T) { + team, _, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, false) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) + }) + + t.Run("syncRoles=false preserves the existing SchemeAdmin on channel members", func(t *testing.T) { + _, channel, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, false) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) + }) + + t.Run("syncRoles=true reconciles team SchemeAdmin against PermittedSyncableAdmins", func(t *testing.T) { + team, _, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, true) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.False(t, tm.SchemeAdmin) + }) + + t.Run("syncRoles=true reconciles channel SchemeAdmin against PermittedSyncableAdmins", func(t *testing.T) { + _, channel, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, true) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.False(t, cm.SchemeAdmin) + }) +} + +func TestSyncRolesAndMembership_AlwaysSyncsMembership(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + setup := func(t *testing.T) (*model.Team, *model.Channel, *model.Group, *model.User) { + t.Helper() + + team := th.CreateTeam(t) + channel := th.CreateChannel(t, team) + group := th.CreateGroup(t) + + _, err := th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: team.Id, + Type: model.GroupSyncableTypeTeam, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: channel.Id, + Type: model.GroupSyncableTypeChannel, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + groupMember := th.CreateUser(t) + _, err = th.App.UpsertGroupMember(group.Id, groupMember.Id) + require.Nil(t, err) + + return team, channel, group, groupMember + } + + t.Run("syncRoles=false still adds group members to the team", func(t *testing.T) { + team, _, group, groupMember := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, false) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, groupMember.Id) + require.Nil(t, appErr) + assert.Equal(t, groupMember.Id, tm.UserId) + }) + + t.Run("syncRoles=false still adds group members to the channel", func(t *testing.T) { + _, channel, group, groupMember := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, false) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, groupMember.Id) + require.Nil(t, appErr) + assert.Equal(t, groupMember.Id, cm.UserId) + }) +}
server/channels/store/sqlstore/group_store.go+3 −1 modified@@ -798,6 +798,7 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync groupSyncable.DeleteAt = groupTeam.DeleteAt groupSyncable.UpdateAt = groupTeam.UpdateAt groupSyncable.Type = syncableType + groupSyncable.SchemeAdmin = groupTeam.SchemeAdmin case model.GroupSyncableTypeChannel: groupChannel := result.(*groupChannel) groupSyncable.SyncableId = groupChannel.ChannelId @@ -807,6 +808,7 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync groupSyncable.DeleteAt = groupChannel.DeleteAt groupSyncable.UpdateAt = groupChannel.UpdateAt groupSyncable.Type = syncableType + groupSyncable.SchemeAdmin = groupChannel.SchemeAdmin default: return nil, fmt.Errorf("unable to convert syncableType: %s", syncableType.String()) } @@ -1836,7 +1838,7 @@ func (s *SqlGroupStore) AdminRoleGroupsForSyncableMember(userID, syncableID stri func (s *SqlGroupStore) PermittedSyncableAdmins(syncableID string, syncableType model.GroupSyncableType) ([]string, error) { builder := s.getQueryBuilder().Select("UserId"). From(fmt.Sprintf("Group%ss", syncableType)). - Join(fmt.Sprintf("GroupMembers ON GroupMembers.GroupId = Group%ss.GroupId AND Group%[1]ss.SchemeAdmin = TRUE AND GroupMembers.DeleteAt = 0", syncableType.String())).Where(fmt.Sprintf("Group%[1]ss.%[1]sId = ?", syncableType.String()), syncableID) + Join(fmt.Sprintf("GroupMembers ON GroupMembers.GroupId = Group%ss.GroupId AND Group%[1]ss.SchemeAdmin = TRUE AND Group%[1]ss.DeleteAt = 0 AND GroupMembers.DeleteAt = 0", syncableType.String())).Where(fmt.Sprintf("Group%[1]ss.%[1]sId = ?", syncableType.String()), syncableID) var userIDs []string if err := s.GetMaster().SelectBuilder(&userIDs, builder); err != nil {
server/channels/store/storetest/group_store.go+28 −0 modified@@ -1616,9 +1616,19 @@ func testGetGroupSyncable(t *testing.T, rctx request.CTX, ss store.Store) { require.Equal(t, gt1.GroupId, dgt.GroupId) require.Equal(t, gt1.SyncableId, dgt.SyncableId) require.Equal(t, gt1.AutoAdd, dgt.AutoAdd) + require.Equal(t, gt1.SchemeAdmin, dgt.SchemeAdmin) require.NotZero(t, gt1.CreateAt) require.NotZero(t, gt1.UpdateAt) require.Zero(t, gt1.DeleteAt) + + // Round-trip SchemeAdmin: true through UpdateGroupSyncable and re-fetch. + dgt.SchemeAdmin = true + _, err = ss.Group().UpdateGroupSyncable(dgt) + require.NoError(t, err) + + dgt, err = ss.Group().GetGroupSyncable(groupTeam.GroupId, groupTeam.SyncableId, model.GroupSyncableTypeTeam) + require.NoError(t, err) + require.True(t, dgt.SchemeAdmin, "GetGroupSyncable must populate SchemeAdmin from the persisted row") } func testGetGroupSyncableErrors(t *testing.T, rctx request.CTX, ss store.Store) { @@ -4979,6 +4989,15 @@ func groupTestPermittedSyncableAdminsTeam(t *testing.T, rctx request.CTX, ss sto // deleted group syncable no longer includes group members _, err = ss.Group().DeleteGroupSyncable(group1.Id, team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) + + // The persisted row must still carry SchemeAdmin=true after soft-delete; + // PermittedSyncableAdmins excludes it via the DeleteAt = 0 predicate, not + // via the field having been silently cleared. + deletedSyncable, err := ss.Group().GetGroupSyncable(group1.Id, team.Id, model.GroupSyncableTypeTeam) + require.NoError(t, err) + require.True(t, deletedSyncable.SchemeAdmin) + require.NotZero(t, deletedSyncable.DeleteAt) + actualUserIDs, err = ss.Group().PermittedSyncableAdmins(team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) require.ElementsMatch(t, []string{user3.Id}, actualUserIDs) @@ -5086,6 +5105,15 @@ func groupTestPermittedSyncableAdminsChannel(t *testing.T, rctx request.CTX, ss // deleted group syncable no longer includes group members _, err = ss.Group().DeleteGroupSyncable(group1.Id, channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) + + // The persisted row must still carry SchemeAdmin=true after soft-delete; + // PermittedSyncableAdmins excludes it via the DeleteAt = 0 predicate, not + // via the field having been silently cleared. + deletedSyncable, err := ss.Group().GetGroupSyncable(group1.Id, channel.Id, model.GroupSyncableTypeChannel) + require.NoError(t, err) + require.True(t, deletedSyncable.SchemeAdmin) + require.NotZero(t, deletedSyncable.DeleteAt) + actualUserIDs, err = ss.Group().PermittedSyncableAdmins(channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) require.ElementsMatch(t, []string{user3.Id}, actualUserIDs)
webapp/channels/src/components/admin_console/group_settings/group_details/group_details.tsx+16 −8 modified@@ -417,17 +417,25 @@ class GroupDetails extends React.PureComponent<Props, State> { roleChangeKey = (groupTeamOrChannel: { type?: SyncableType; + id?: string; team_id?: string; channel_id?: string; }) => { - let id; - if ( - this.syncableTypeFromEntryType(groupTeamOrChannel.type) === - SyncableType.Team - ) { - id = groupTeamOrChannel.team_id; - } else { - id = groupTeamOrChannel.channel_id; + // Items in itemsToRemove use a generic `id`, while items coming from + // teamsToAdd/channelsToAdd use `team_id`/`channel_id`. The key must + // be identical regardless of source so the dedup in + // handleRemovedTeamsAndChannels and handleAddedTeamsAndChannels + // matches the key produced by onChangeRoles. + let id = groupTeamOrChannel.id; + if (!id) { + if ( + this.syncableTypeFromEntryType(groupTeamOrChannel.type) === + SyncableType.Team + ) { + id = groupTeamOrChannel.team_id; + } else { + id = groupTeamOrChannel.channel_id; + } } return `${id}/${groupTeamOrChannel.type}`; };
202d125afa87[release-10.11] MM-68547: Tighten authorization on group syncable link and patch endpoints (#36434)
8 files changed · +1006 −23
e2e-tests/cypress/tests/integration/channels/enterprise/system_console/group_configuration_spec.js+6 −2 modified@@ -394,8 +394,10 @@ describe('group configuration', () => { // # Save settings savePage(); - // * Check the groupteam via the API to ensure its role wasn't updated + // * Check the groupteam via the API to ensure the team was + // removed (delete_at != 0) and its role wasn't updated. cy.apiGetGroupTeam(groupID, testTeam.id).then(({body}) => { + expect(body.delete_at).to.not.eq(0); expect(body.scheme_admin).to.eq(false); }); }); @@ -519,8 +521,10 @@ describe('group configuration', () => { // # Save settings savePage(); - // * Check the groupteam via the API to ensure its role wasn't updated + // * Check the groupteam via the API to ensure the channel was + // removed (delete_at != 0) and its role wasn't updated. cy.apiGetGroupChannel(groupID, testChannel.id).then(({body}) => { + expect(body.delete_at).to.not.eq(0); expect(body.scheme_admin).to.eq(false); }); });
server/channels/api4/group.go+68 −6 modified@@ -377,10 +377,35 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } - groupSyncable := &model.GroupSyncable{ - GroupId: c.Params.GroupId, - SyncableId: syncableID, - Type: syncableType, + appErr = verifySchemeAdminAssignmentPermission(c, syncableType, syncableID, patch) + if appErr != nil { + appErr.Where = "Api4.linkGroupSyncable" + c.Err = appErr + return + } + + // Upsert onto the existing row only when it is currently active so + // unspecified fields are preserved. A fresh link, or a re-link of a + // soft-deleted row, starts from a zero-value struct so that fields + // the caller did not (or was not authorized to) set are not carried + // over from the previous incarnation. The downstream upsert clears + // DeleteAt when re-activating. + existing, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) + if appErr != nil && appErr.StatusCode != http.StatusNotFound { + appErr.Where = "Api4.linkGroupSyncable" + c.Err = appErr + return + } + + var groupSyncable *model.GroupSyncable + if existing != nil && existing.DeleteAt == 0 { + groupSyncable = existing + } else { + groupSyncable = &model.GroupSyncable{ + GroupId: c.Params.GroupId, + SyncableId: syncableID, + Type: syncableType, + } } groupSyncable.Patch(patch) groupSyncable, appErr = c.App.UpsertGroupSyncable(groupSyncable) @@ -392,8 +417,9 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventResultState(groupSyncable) auditRec.AddEventObjectType("group_syncable") + syncRoles := patch.SchemeAdmin != nil c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId) + c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId, syncRoles) }) w.WriteHeader(http.StatusCreated) @@ -560,6 +586,13 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } + appErr = verifySchemeAdminAssignmentPermission(c, syncableType, syncableID, patch) + if appErr != nil { + appErr.Where = "Api4.patchGroupSyncable" + c.Err = appErr + return + } + groupSyncable, appErr := c.App.GetGroupSyncable(c.Params.GroupId, syncableID, syncableType) if appErr != nil { c.Err = appErr @@ -577,8 +610,9 @@ func patchGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventResultState(groupSyncable) auditRec.AddEventObjectType("group_syncable") + syncRoles := patch.SchemeAdmin != nil c.App.Srv().Go(func() { - c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId) + c.App.SyncRolesAndMembership(c.AppContext, syncableID, syncableType, c.Params.GroupId, syncRoles) }) b, err := json.Marshal(groupSyncable) @@ -710,6 +744,34 @@ func verifyLinkUnlinkPermission(c *Context, syncableType model.GroupSyncableType return nil } +// verifySchemeAdminAssignmentPermission requires the caller to hold the +// role-management permission for the target syncable +// (manage_team_roles / manage_channel_roles), or the sysconsole groups +// write permission, before an explicit SchemeAdmin value in the patch is +// accepted. A nil patch.SchemeAdmin is a no-op. +func verifySchemeAdminAssignmentPermission(c *Context, syncableType model.GroupSyncableType, syncableID string, patch *model.GroupSyncablePatch) *model.AppError { + if patch == nil || patch.SchemeAdmin == nil { + return nil + } + + if c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementGroups) { + return nil + } + + switch syncableType { + case model.GroupSyncableTypeTeam: + if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), syncableID, model.PermissionManageTeamRoles) { + return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageTeamRoles}) + } + case model.GroupSyncableTypeChannel: + if ok, _ := c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), syncableID, model.PermissionManageChannelRoles); !ok { + return model.MakePermissionError(c.AppContext.Session(), []*model.Permission{model.PermissionManageChannelRoles}) + } + } + + return nil +} + func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { permissionErr := requireLicense(c) if permissionErr != nil {
server/channels/api4/group_test.go+733 −0 modified@@ -2847,3 +2847,736 @@ func TestDeleteMembersFromGroup(t *testing.T) { CheckBadRequestStatus(t, response) }) } + +// newSchemeAdminTestLdapGroup creates a fresh LDAP-source group with +// AllowReference=true. +func newSchemeAdminTestLdapGroup(t *testing.T, th *TestHelper) *model.Group { + t.Helper() + id := model.NewId() + g, appErr := th.App.CreateGroup(&model.Group{ + DisplayName: "dn_" + id, + Name: model.NewPointer("name" + id), + Source: model.GroupSourceLdap, + Description: "description_" + id, + RemoteId: model.NewPointer(model.NewId()), + AllowReference: true, + }) + require.Nil(t, appErr) + return g +} + +// findPersistedGroupSyncable returns the persisted GroupSyncable for a +// given (groupID, syncableID, syncableType) tuple, including SchemeAdmin. +func findPersistedGroupSyncable(t *testing.T, th *TestHelper, groupID, syncableID string, syncableType model.GroupSyncableType) *model.GroupSyncable { + t.Helper() + syncables, appErr := th.App.GetGroupSyncables(groupID, syncableType) + require.Nil(t, appErr) + for _, s := range syncables { + if s.SyncableId == syncableID { + return s + } + } + return nil +} + +func TestLinkGroupTeam_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + schemeAdminTrue := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + } + + t.Run("regular team user with invite_user must NOT be able to set scheme_admin: true", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + assert.Nil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) + + t.Run("system admin can still set scheme_admin: true", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user can still link with scheme_admin omitted", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.NotNil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user must NOT be able to link with scheme_admin: false explicitly", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + 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) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) +} + +func TestLinkGroupChannel_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // A regular user can only link a channel syncable when the group is + // already linked to the parent team, so seed the team link as sysadmin. + mkLinkedGroup := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + } + + t.Run("regular channel user with manage_*_channel_members must NOT be able to set scheme_admin: true", func(t *testing.T) { + g := mkLinkedGroup(t) + + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + assert.Nil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) + + t.Run("system admin can still set scheme_admin: true", func(t *testing.T) { + g := mkLinkedGroup(t) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user can still link with scheme_admin omitted", func(t *testing.T) { + g := mkLinkedGroup(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + groupSyncable, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.NotNil(t, groupSyncable) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user must NOT be able to link with scheme_admin: false explicitly", func(t *testing.T) { + g := mkLinkedGroup(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + 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) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + if persisted != nil { + assert.False(t, persisted.SchemeAdmin) + } + }) +} + +func TestPatchGroupTeam_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // schemeAdmin controls the seeded SchemeAdmin value on the team syncable. + setupLinkedGroup := func(t *testing.T, schemeAdmin bool) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(schemeAdmin), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(true), + } + + schemeAdminFalse := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(false), + } + + t.Run("regular team user with invite_user must NOT be able to patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user can still patch other fields with scheme_admin omitted", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.AutoAdd) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user must NOT be able to patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("sysconsole_write_user_management_groups holder can patch scheme_admin in either direction", func(t *testing.T) { + // system_manager bundles sysconsole_write_user_management_groups, + // the override honoured by verifySchemeAdminAssignmentPermission. + th.LoginSystemManager() + + gPromote := setupLinkedGroup(t, false) + _, response, err := th.SystemManagerClient.PatchGroupSyncable(context.Background(), gPromote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedPromote := findPersistedGroupSyncable(t, th, gPromote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persistedPromote) + assert.True(t, persistedPromote.SchemeAdmin) + + gDemote := setupLinkedGroup(t, true) + _, response, err = th.SystemManagerClient.PatchGroupSyncable(context.Background(), gDemote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedDemote := findPersistedGroupSyncable(t, th, gDemote.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persistedDemote) + assert.False(t, persistedDemote.SchemeAdmin) + }) +} + +func TestPatchGroupChannel_SchemeAdminRequiresElevatedPermission(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + // schemeAdmin controls the seeded SchemeAdmin value on the channel + // syncable. The team syncable is seeded so the channel link succeeds. + setupLinkedGroup := func(t *testing.T, schemeAdmin bool) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(schemeAdmin), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + return g + } + + schemeAdminTrue := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(true), + } + + schemeAdminFalse := &model.GroupSyncablePatch{ + SchemeAdmin: model.NewPointer(false), + } + + t.Run("regular channel user with manage_*_channel_members must NOT be able to patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: true", func(t *testing.T) { + g := setupLinkedGroup(t, false) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user can still patch other fields with scheme_admin omitted", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.AutoAdd) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user must NOT be able to patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.Error(t, err) + CheckForbiddenStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("system admin can still patch scheme_admin: false", func(t *testing.T) { + g := setupLinkedGroup(t, true) + + _, response, err := th.SystemAdminClient.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) + + t.Run("sysconsole_write_user_management_groups holder can patch scheme_admin in either direction", func(t *testing.T) { + // system_manager bundles sysconsole_write_user_management_groups, + // the override honoured by verifySchemeAdminAssignmentPermission. + th.LoginSystemManager() + + gPromote := setupLinkedGroup(t, false) + _, response, err := th.SystemManagerClient.PatchGroupSyncable(context.Background(), gPromote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminTrue) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedPromote := findPersistedGroupSyncable(t, th, gPromote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persistedPromote) + assert.True(t, persistedPromote.SchemeAdmin) + + gDemote := setupLinkedGroup(t, true) + _, response, err = th.SystemManagerClient.PatchGroupSyncable(context.Background(), gDemote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, schemeAdminFalse) + require.NoError(t, err) + CheckOKStatus(t, response) + persistedDemote := findPersistedGroupSyncable(t, th, gDemote.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persistedDemote) + assert.False(t, persistedDemote.SchemeAdmin) + }) +} + +func TestLinkGroupTeam_LinkOnExistingPreservesSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + seedSchemeAdminTrue := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + require.True(t, persisted.SchemeAdmin) + return g + } + + t.Run("regular team user calling LINK with scheme_admin omitted must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular team user calling LINK with scheme_admin: false must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) +} + +func TestLinkGroupChannel_LinkOnExistingPreservesSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + seedSchemeAdminTrue := func(t *testing.T) *model.Group { + t.Helper() + g := newSchemeAdminTestLdapGroup(t, th) + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + require.True(t, persisted.SchemeAdmin) + return g + } + + t.Run("regular channel user calling LINK with scheme_admin omitted must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) + + t.Run("regular channel user calling LINK with scheme_admin: false must not change persisted scheme_admin", func(t *testing.T) { + g := seedSchemeAdminTrue(t) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(false), + } + _, _, _ = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel) + require.NotNil(t, persisted) + assert.True(t, persisted.SchemeAdmin) + }) +} + +func TestLinkGroupTeam_LinkOnSoftDeletedDoesNotPreserveSchemeAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + t.Run("regular team user re-linking a soft-deleted syncable with scheme_admin omitted must persist scheme_admin: false", func(t *testing.T) { + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + response, err = th.SystemAdminClient.UnlinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NoError(t, err) + CheckOKStatus(t, response) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + persisted := findPersistedGroupSyncable(t, th, g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam) + require.NotNil(t, persisted) + assert.False(t, persisted.SchemeAdmin) + }) +} + +func TestPatchGroupTeam_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.UpdateUserToTeamAdmin(th.BasicUser2, th.BasicTeam) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err = th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestPatchGroupChannel_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + _, response, err = th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.MakeUserChannelAdmin(th.BasicUser2, th.BasicChannel) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(false), + } + _, response, err = th.Client.PatchGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + time.Sleep(2 * time.Second) + + cm, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) +} + +func TestLinkGroupTeam_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + th.UpdateUserToTeamAdmin(th.BasicUser2, th.BasicTeam) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err := th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestLinkGroupChannel_OmittedSchemeAdminDoesNotDemoteDirectAdmin(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + th.MakeUserChannelAdmin(th.BasicUser2, th.BasicChannel) + + patch := &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + } + _, response, err = th.Client.LinkGroupSyncable(context.Background(), g.Id, th.BasicChannel.Id, model.GroupSyncableTypeChannel, patch) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + cm, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) +} + +func TestLinkGroupTeam_SchemeAdminTruePromotesGroupMembers(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + _, appErr := th.App.UpsertGroupMember(g.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + SchemeAdmin: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) +} + +func TestLinkGroupTeam_AutoAddOnlyAddsGroupMembers(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + g := newSchemeAdminTestLdapGroup(t, th) + + newUser := th.CreateUser() + _, appErr := th.App.UpsertGroupMember(g.Id, newUser.Id) + require.Nil(t, appErr) + + _, appErr = th.App.GetTeamMember(th.Context, th.BasicTeam.Id, newUser.Id) + require.NotNil(t, appErr) + + _, response, err := th.SystemAdminClient.LinkGroupSyncable(context.Background(), g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam, &model.GroupSyncablePatch{ + AutoAdd: model.NewPointer(true), + }) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + time.Sleep(2 * time.Second) + + tm, appErr := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, newUser.Id) + require.Nil(t, appErr) + assert.Equal(t, newUser.Id, tm.UserId) +}
server/channels/app/syncables.go+8 −6 modified@@ -268,18 +268,20 @@ func (a *App) SyncSyncableRoles(rctx request.CTX, syncableID string, syncableTyp return nil } -// SyncRolesAndMembership updates the SchemeAdmin status and membership of all of the members of the given -// syncable. -func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, groupID string) { +// SyncRolesAndMembership updates the membership of the given syncable and, +// when syncRoles is true, also reconciles SchemeAdmin status for its members. +func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, groupID string, syncRoles bool) { group, appErr := a.GetGroup(groupID, nil, nil) if appErr != nil { rctx.Logger().Warn("Error getting group", mlog.Err(appErr)) return } - appErr = a.SyncSyncableRoles(rctx, syncableID, syncableType) - if appErr != nil { - rctx.Logger().Warn("Error syncing syncable roles", mlog.Err(appErr)) + if syncRoles { + appErr = a.SyncSyncableRoles(rctx, syncableID, syncableType) + if appErr != nil { + rctx.Logger().Warn("Error syncing syncable roles", mlog.Err(appErr)) + } } var since int64
server/channels/app/syncables_test.go+144 −0 modified@@ -677,3 +677,147 @@ func TestSyncSyncableRoles(t *testing.T) { require.True(t, cm.SchemeAdmin) } } + +func TestSyncRolesAndMembership_RoleSyncGate(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + setup := func(t *testing.T) (*model.Team, *model.Channel, *model.Group, *model.User) { + t.Helper() + + team := th.CreateTeam() + channel := th.CreateChannel(th.Context, team) + group := th.CreateGroup() + + _, err := th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: team.Id, + Type: model.GroupSyncableTypeTeam, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: channel.Id, + Type: model.GroupSyncableTypeChannel, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + directAdmin := th.CreateUser() + _, appErr := th.App.AddTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, directAdmin, channel, false) + require.Nil(t, appErr) + + tm, storeErr := th.App.Srv().Store().Team().GetMember(th.Context, team.Id, directAdmin.Id) + require.NoError(t, storeErr) + tm.SchemeAdmin = true + _, storeErr = th.App.Srv().Store().Team().UpdateMember(th.Context, tm) + require.NoError(t, storeErr) + + cm, storeErr := th.App.Srv().Store().Channel().GetMember(th.Context.Context(), channel.Id, directAdmin.Id) + require.NoError(t, storeErr) + cm.SchemeAdmin = true + _, storeErr = th.App.Srv().Store().Channel().UpdateMember(th.Context, cm) + require.NoError(t, storeErr) + + return team, channel, group, directAdmin + } + + t.Run("syncRoles=false preserves the existing SchemeAdmin on team members", func(t *testing.T) { + team, _, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, false) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.True(t, tm.SchemeAdmin) + }) + + t.Run("syncRoles=false preserves the existing SchemeAdmin on channel members", func(t *testing.T) { + _, channel, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, false) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.True(t, cm.SchemeAdmin) + }) + + t.Run("syncRoles=true reconciles team SchemeAdmin against PermittedSyncableAdmins", func(t *testing.T) { + team, _, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, true) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.False(t, tm.SchemeAdmin) + }) + + t.Run("syncRoles=true reconciles channel SchemeAdmin against PermittedSyncableAdmins", func(t *testing.T) { + _, channel, group, directAdmin := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, true) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, directAdmin.Id) + require.Nil(t, appErr) + assert.False(t, cm.SchemeAdmin) + }) +} + +func TestSyncRolesAndMembership_AlwaysSyncsMembership(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic() + + setup := func(t *testing.T) (*model.Team, *model.Channel, *model.Group, *model.User) { + t.Helper() + + team := th.CreateTeam() + channel := th.CreateChannel(th.Context, team) + group := th.CreateGroup() + + _, err := th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: team.Id, + Type: model.GroupSyncableTypeTeam, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ + SyncableId: channel.Id, + Type: model.GroupSyncableTypeChannel, + GroupId: group.Id, + AutoAdd: true, + }) + require.Nil(t, err) + + groupMember := th.CreateUser() + _, err = th.App.UpsertGroupMember(group.Id, groupMember.Id) + require.Nil(t, err) + + return team, channel, group, groupMember + } + + t.Run("syncRoles=false still adds group members to the team", func(t *testing.T) { + team, _, group, groupMember := setup(t) + + th.App.SyncRolesAndMembership(th.Context, team.Id, model.GroupSyncableTypeTeam, group.Id, false) + + tm, appErr := th.App.GetTeamMember(th.Context, team.Id, groupMember.Id) + require.Nil(t, appErr) + assert.Equal(t, groupMember.Id, tm.UserId) + }) + + t.Run("syncRoles=false still adds group members to the channel", func(t *testing.T) { + _, channel, group, groupMember := setup(t) + + th.App.SyncRolesAndMembership(th.Context, channel.Id, model.GroupSyncableTypeChannel, group.Id, false) + + cm, appErr := th.App.GetChannelMember(th.Context, channel.Id, groupMember.Id) + require.Nil(t, appErr) + assert.Equal(t, groupMember.Id, cm.UserId) + }) +}
server/channels/store/sqlstore/group_store.go+3 −1 modified@@ -777,6 +777,7 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync groupSyncable.DeleteAt = groupTeam.DeleteAt groupSyncable.UpdateAt = groupTeam.UpdateAt groupSyncable.Type = syncableType + groupSyncable.SchemeAdmin = groupTeam.SchemeAdmin case model.GroupSyncableTypeChannel: groupChannel := result.(*groupChannel) groupSyncable.SyncableId = groupChannel.ChannelId @@ -786,6 +787,7 @@ func (s *SqlGroupStore) getGroupSyncable(groupID string, syncableID string, sync groupSyncable.DeleteAt = groupChannel.DeleteAt groupSyncable.UpdateAt = groupChannel.UpdateAt groupSyncable.Type = syncableType + groupSyncable.SchemeAdmin = groupChannel.SchemeAdmin default: return nil, fmt.Errorf("unable to convert syncableType: %s", syncableType.String()) } @@ -1834,7 +1836,7 @@ func (s *SqlGroupStore) AdminRoleGroupsForSyncableMember(userID, syncableID stri func (s *SqlGroupStore) PermittedSyncableAdmins(syncableID string, syncableType model.GroupSyncableType) ([]string, error) { builder := s.getQueryBuilder().Select("UserId"). From(fmt.Sprintf("Group%ss", syncableType)). - Join(fmt.Sprintf("GroupMembers ON GroupMembers.GroupId = Group%ss.GroupId AND Group%[1]ss.SchemeAdmin = TRUE AND GroupMembers.DeleteAt = 0", syncableType.String())).Where(fmt.Sprintf("Group%[1]ss.%[1]sId = ?", syncableType.String()), syncableID) + Join(fmt.Sprintf("GroupMembers ON GroupMembers.GroupId = Group%ss.GroupId AND Group%[1]ss.SchemeAdmin = TRUE AND Group%[1]ss.DeleteAt = 0 AND GroupMembers.DeleteAt = 0", syncableType.String())).Where(fmt.Sprintf("Group%[1]ss.%[1]sId = ?", syncableType.String()), syncableID) var userIDs []string if err := s.GetMaster().SelectBuilder(&userIDs, builder); err != nil {
server/channels/store/storetest/group_store.go+28 −0 modified@@ -1616,9 +1616,19 @@ func testGetGroupSyncable(t *testing.T, rctx request.CTX, ss store.Store) { require.Equal(t, gt1.GroupId, dgt.GroupId) require.Equal(t, gt1.SyncableId, dgt.SyncableId) require.Equal(t, gt1.AutoAdd, dgt.AutoAdd) + require.Equal(t, gt1.SchemeAdmin, dgt.SchemeAdmin) require.NotZero(t, gt1.CreateAt) require.NotZero(t, gt1.UpdateAt) require.Zero(t, gt1.DeleteAt) + + // Round-trip SchemeAdmin: true through UpdateGroupSyncable and re-fetch. + dgt.SchemeAdmin = true + _, err = ss.Group().UpdateGroupSyncable(dgt) + require.NoError(t, err) + + dgt, err = ss.Group().GetGroupSyncable(groupTeam.GroupId, groupTeam.SyncableId, model.GroupSyncableTypeTeam) + require.NoError(t, err) + require.True(t, dgt.SchemeAdmin, "GetGroupSyncable must populate SchemeAdmin from the persisted row") } func testGetGroupSyncableErrors(t *testing.T, rctx request.CTX, ss store.Store) { @@ -4989,6 +4999,15 @@ func groupTestPermittedSyncableAdminsTeam(t *testing.T, rctx request.CTX, ss sto // deleted group syncable no longer includes group members _, err = ss.Group().DeleteGroupSyncable(group1.Id, team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) + + // The persisted row must still carry SchemeAdmin=true after soft-delete; + // PermittedSyncableAdmins excludes it via the DeleteAt = 0 predicate, not + // via the field having been silently cleared. + deletedSyncable, err := ss.Group().GetGroupSyncable(group1.Id, team.Id, model.GroupSyncableTypeTeam) + require.NoError(t, err) + require.True(t, deletedSyncable.SchemeAdmin) + require.NotZero(t, deletedSyncable.DeleteAt) + actualUserIDs, err = ss.Group().PermittedSyncableAdmins(team.Id, model.GroupSyncableTypeTeam) require.NoError(t, err) require.ElementsMatch(t, []string{user3.Id}, actualUserIDs) @@ -5096,6 +5115,15 @@ func groupTestPermittedSyncableAdminsChannel(t *testing.T, rctx request.CTX, ss // deleted group syncable no longer includes group members _, err = ss.Group().DeleteGroupSyncable(group1.Id, channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) + + // The persisted row must still carry SchemeAdmin=true after soft-delete; + // PermittedSyncableAdmins excludes it via the DeleteAt = 0 predicate, not + // via the field having been silently cleared. + deletedSyncable, err := ss.Group().GetGroupSyncable(group1.Id, channel.Id, model.GroupSyncableTypeChannel) + require.NoError(t, err) + require.True(t, deletedSyncable.SchemeAdmin) + require.NotZero(t, deletedSyncable.DeleteAt) + actualUserIDs, err = ss.Group().PermittedSyncableAdmins(channel.Id, model.GroupSyncableTypeChannel) require.NoError(t, err) require.ElementsMatch(t, []string{user3.Id}, actualUserIDs)
webapp/channels/src/components/admin_console/group_settings/group_details/group_details.tsx+16 −8 modified@@ -417,17 +417,25 @@ class GroupDetails extends React.PureComponent<Props, State> { roleChangeKey = (groupTeamOrChannel: { type?: SyncableType; + id?: string; team_id?: string; channel_id?: string; }) => { - let id; - if ( - this.syncableTypeFromEntryType(groupTeamOrChannel.type) === - SyncableType.Team - ) { - id = groupTeamOrChannel.team_id; - } else { - id = groupTeamOrChannel.channel_id; + // Items in itemsToRemove use a generic `id`, while items coming from + // teamsToAdd/channelsToAdd use `team_id`/`channel_id`. The key must + // be identical regardless of source so the dedup in + // handleRemovedTeamsAndChannels and handleAddedTeamsAndChannels + // matches the key produced by onChangeRoles. + let id = groupTeamOrChannel.id; + if (!id) { + if ( + this.syncableTypeFromEntryType(groupTeamOrChannel.type) === + SyncableType.Team + ) { + id = groupTeamOrChannel.team_id; + } else { + id = groupTeamOrChannel.channel_id; + } } return `${id}/${groupTeamOrChannel.type}`; };
Vulnerability mechanics
Root cause
"Missing authorization check on the `SchemeAdmin` field in group syncable link and patch endpoints allows privilege escalation."
Attack vector
An authenticated attacker with group-link permissions (e.g., a regular team user with `invite_user` or a channel user with `manage_*_channel_members`) can craft API requests to the group syncable link or patch endpoints that include a `SchemeAdmin` field set to `true`. Because the endpoints previously did not verify that the caller had role-management authorization, the attacker could escalate themselves or other group members to team or channel admin. The attack is network-accessible, requires low-privilege authentication, and needs no special configuration beyond an LDAP license.
Affected code
The vulnerability exists in the `LinkGroupSyncable` and `PatchGroupSyncable` API endpoints in `server/channels/api4/group.go`. These endpoints failed to require role-management authorization when setting the `SchemeAdmin` flag on group syncable links, allowing users with only group-link permissions to escalate privileges. The patch adds a new authorization check via `verifySchemeAdminAssignmentPermission` that gates any request that includes a non-nil `SchemeAdmin` field in the patch payload.
What the fix does
The patch introduces a new authorization helper `verifySchemeAdminAssignmentPermission` that checks whether the requesting user holds the `sysconsole_write_user_management_groups` permission (or is a system admin) before allowing any `SchemeAdmin` field to be written. The `LinkGroupSyncable` and `PatchGroupSyncable` handlers now inspect the incoming patch for a non-nil `SchemeAdmin` pointer and, if present, invoke this check. If the caller lacks the required permission, the request is rejected with a Forbidden status. This ensures that only users with role-management authorization can promote or demote group members to admin via group syncable links.
Preconditions
- configThe Mattermost server must have an LDAP license enabled.
- authThe attacker must be an authenticated user with group-link permissions (e.g., a regular team member or channel member).
- networkThe attacker must be able to send crafted API requests to the group syncable link or patch endpoints.
- inputThe request payload must include a non-nil `SchemeAdmin` field.
Generated on Jun 12, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
1News mentions
0No linked articles in our index yet.