diff --git a/server/channels/app/role.go b/server/channels/app/role.go index dfcc8a1dab1..44de9f594ed 100644 --- a/server/channels/app/role.go +++ b/server/channels/app/role.go @@ -7,12 +7,14 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "reflect" "slices" "strings" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/public/shared/request" "github.com/mattermost/mattermost/server/v8/channels/store" @@ -276,13 +278,93 @@ func (a *App) CheckRolesExist(roleNames []string) *model.AppError { } func (a *App) sendUpdatedRoleEvent(role *model.Role) *model.AppError { - message := model.NewWebSocketEvent(model.WebsocketEventRoleUpdated, "", "", "", nil, "") roleJSON, jsonErr := json.Marshal(role) if jsonErr != nil { return model.NewAppError("sendUpdatedRoleEvent", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(jsonErr) } - message.Add("role", string(roleJSON)) - a.Publish(message) + + publishEvent := func(teamID, channelID string) { + message := model.NewWebSocketEvent(model.WebsocketEventRoleUpdated, teamID, channelID, "", nil, "") + message.Add("role", string(roleJSON)) + a.Publish(message) + } + + // Built-in system roles apply to all users; broadcast globally without a DB lookup. + if role.BuiltIn { + publishEvent("", "") + return nil + } + + // Scheme-managed roles: use SchemeId to look up the owning scheme. + if role.SchemeId == nil { + // No owning scheme — treat as global (e.g. custom non-scheme role). + publishEvent("", "") + return nil + } + scheme, err := a.Srv().Store().Scheme().Get(*role.SchemeId) + if err != nil { + a.Log().Error("Failed to look up scheme for role event; skipping broadcast", + mlog.String("role_id", role.Id), + mlog.String("scheme_id", *role.SchemeId), + mlog.Err(err)) + return nil + } + + const pageSize = 1000 + const maxBroadcasts = 100000 + switch scheme.Scope { + case model.SchemeScopeTeam: + totalBroadcasts := 0 + offset := 0 + for { + teams, storeErr := a.Srv().Store().Team().GetTeamsByScheme(scheme.Id, offset, pageSize) + if storeErr != nil { + return model.NewAppError("sendUpdatedRoleEvent", "app.role.send_updated_role_event.app_error", nil, "", http.StatusInternalServerError).Wrap(storeErr) + } + for _, team := range teams { + publishEvent(team.Id, "") + } + totalBroadcasts += len(teams) + if len(teams) < pageSize { + break + } + if totalBroadcasts >= maxBroadcasts { + a.Log().Error("sendUpdatedRoleEvent: hit broadcast limit for team scheme", + mlog.String("scheme_id", scheme.Id), + mlog.Int("totalBroadcasts", totalBroadcasts)) + break + } + offset += pageSize + } + case model.SchemeScopeChannel: + totalBroadcasts := 0 + offset := 0 + for { + channels, storeErr := a.Srv().Store().Channel().GetChannelsByScheme(scheme.Id, offset, pageSize) + if storeErr != nil { + return model.NewAppError("sendUpdatedRoleEvent", "app.role.send_updated_role_event.app_error", nil, "", http.StatusInternalServerError).Wrap(storeErr) + } + for _, channel := range channels { + publishEvent("", channel.Id) + } + totalBroadcasts += len(channels) + if len(channels) < pageSize { + break + } + if totalBroadcasts >= maxBroadcasts { + a.Log().Error("sendUpdatedRoleEvent: hit broadcast limit for channel scheme", + mlog.String("scheme_id", scheme.Id), + mlog.Int("totalBroadcasts", totalBroadcasts)) + break + } + offset += pageSize + } + case model.SchemeScopePlaybook, model.SchemeScopeRun: + // Playbook/run schemes don't map to teams or channels; broadcast globally. + publishEvent("", "") + default: + return model.NewAppError("sendUpdatedRoleEvent", "app.role.send_updated_role_event.unknown_scope", nil, fmt.Sprintf("unknown scheme scope: %s", scheme.Scope), http.StatusInternalServerError) + } return nil } diff --git a/server/channels/app/role_test.go b/server/channels/app/role_test.go index e2762cb0ed6..d89d6379b60 100644 --- a/server/channels/app/role_test.go +++ b/server/channels/app/role_test.go @@ -5,6 +5,7 @@ package app import ( "encoding/csv" + "errors" "io" "os" "slices" @@ -12,9 +13,11 @@ import ( "strings" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks" ) type permissionInheritanceTestData struct { @@ -259,3 +262,225 @@ func testPermissionInheritance(t *testing.T, testCallback func(t *testing.T, th // test 24 combinations where the higher-scoped scheme is a TEAM scheme test(teamScheme.DefaultChannelGuestRole, teamScheme.DefaultChannelUserRole, teamScheme.DefaultChannelAdminRole) } + +func TestSendUpdatedRoleEvent(t *testing.T) { + t.Run("BuiltIn role broadcasts globally without a DB lookup", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockStore.On("Scheme").Return(&mockSchemeStore) + + role := &model.Role{Name: model.TeamAdminRoleId, BuiltIn: true} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockSchemeStore.AssertNotCalled(t, "Get", mock.Anything) + }) + + t.Run("Team scheme role calls GetTeamsByScheme and emits per-team events", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + roleName := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopeTeam} + teams := []*model.Team{{Id: model.NewId()}, {Id: model.NewId()}} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockTeamStore := mocks.TeamStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockTeamStore.On("GetTeamsByScheme", schemeID, 0, 1000).Return(teams, nil) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Team").Return(&mockTeamStore) + + role := &model.Role{Name: roleName, BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockSchemeStore.AssertCalled(t, "Get", schemeID) + mockTeamStore.AssertCalled(t, "GetTeamsByScheme", schemeID, 0, 1000) + }) + + t.Run("Channel scheme role calls GetChannelsByScheme and emits per-channel events", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + roleName := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopeChannel} + channels := model.ChannelList{{Id: model.NewId()}} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockChannelStore := mocks.ChannelStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockChannelStore.On("GetChannelsByScheme", schemeID, 0, 1000).Return(channels, nil) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Channel").Return(&mockChannelStore) + + role := &model.Role{Name: roleName, BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockSchemeStore.AssertCalled(t, "Get", schemeID) + mockChannelStore.AssertCalled(t, "GetChannelsByScheme", schemeID, 0, 1000) + }) + + t.Run("Role not in any scheme broadcasts globally", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockTeamStore := mocks.TeamStore{} + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Team").Return(&mockTeamStore) + + role := &model.Role{Name: model.NewId(), BuiltIn: false, SchemeId: nil} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockSchemeStore.AssertNotCalled(t, "Get", mock.Anything) + mockTeamStore.AssertNotCalled(t, "GetTeamsByScheme", mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("Playbook scope falls back to global broadcast without querying teams or channels", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + roleName := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopePlaybook} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockTeamStore := mocks.TeamStore{} + mockChannelStore := mocks.ChannelStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Team").Return(&mockTeamStore) + mockStore.On("Channel").Return(&mockChannelStore) + + role := &model.Role{Name: roleName, BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockTeamStore.AssertNotCalled(t, "GetTeamsByScheme", mock.Anything, mock.Anything, mock.Anything) + mockChannelStore.AssertNotCalled(t, "GetChannelsByScheme", mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("Scheme store error is logged and skips broadcast", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + roleName := model.NewId() + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockSchemeStore.On("Get", schemeID).Return(nil, errors.New("db error")) + mockStore.On("Scheme").Return(&mockSchemeStore) + + role := &model.Role{Name: roleName, BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + }) + + t.Run("GetTeamsByScheme store error propagates as AppError", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + roleName := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopeTeam} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockTeamStore := mocks.TeamStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockTeamStore.On("GetTeamsByScheme", schemeID, 0, 1000).Return(nil, errors.New("db error")) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Team").Return(&mockTeamStore) + + role := &model.Role{Name: roleName, BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.NotNil(t, appErr) + }) + + t.Run("Team scheme paginates across multiple pages", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopeTeam} + + // Build a full first page (1000 teams) and a partial second page (2 teams). + page1 := make([]*model.Team, 1000) + for i := range page1 { + page1[i] = &model.Team{Id: model.NewId()} + } + page2 := []*model.Team{{Id: model.NewId()}, {Id: model.NewId()}} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockTeamStore := mocks.TeamStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockTeamStore.On("GetTeamsByScheme", schemeID, 0, 1000).Return(page1, nil) + mockTeamStore.On("GetTeamsByScheme", schemeID, 1000, 1000).Return(page2, nil) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Team").Return(&mockTeamStore) + + role := &model.Role{Name: model.NewId(), BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockTeamStore.AssertCalled(t, "GetTeamsByScheme", schemeID, 0, 1000) + mockTeamStore.AssertCalled(t, "GetTeamsByScheme", schemeID, 1000, 1000) + }) + + t.Run("Channel scheme paginates across multiple pages", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopeChannel} + + page1 := make(model.ChannelList, 1000) + for i := range page1 { + page1[i] = &model.Channel{Id: model.NewId()} + } + page2 := model.ChannelList{{Id: model.NewId()}, {Id: model.NewId()}, {Id: model.NewId()}} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockChannelStore := mocks.ChannelStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockChannelStore.On("GetChannelsByScheme", schemeID, 0, 1000).Return(page1, nil) + mockChannelStore.On("GetChannelsByScheme", schemeID, 1000, 1000).Return(page2, nil) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Channel").Return(&mockChannelStore) + + role := &model.Role{Name: model.NewId(), BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.Nil(t, appErr) + mockChannelStore.AssertCalled(t, "GetChannelsByScheme", schemeID, 0, 1000) + mockChannelStore.AssertCalled(t, "GetChannelsByScheme", schemeID, 1000, 1000) + }) + + t.Run("GetChannelsByScheme store error propagates as AppError", func(t *testing.T) { + mainHelper.Parallel(t) + th := SetupWithStoreMock(t) + + schemeID := model.NewId() + roleName := model.NewId() + scheme := &model.Scheme{Id: schemeID, Scope: model.SchemeScopeChannel} + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockSchemeStore := mocks.SchemeStore{} + mockChannelStore := mocks.ChannelStore{} + mockSchemeStore.On("Get", schemeID).Return(scheme, nil) + mockChannelStore.On("GetChannelsByScheme", schemeID, 0, 1000).Return(nil, errors.New("db error")) + mockStore.On("Scheme").Return(&mockSchemeStore) + mockStore.On("Channel").Return(&mockChannelStore) + + role := &model.Role{Name: roleName, BuiltIn: false, SchemeId: &schemeID} + appErr := th.App.sendUpdatedRoleEvent(role) + require.NotNil(t, appErr) + }) +} diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 66fdb3db6e5..132aeb4650f 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -307,3 +307,9 @@ channels/db/migrations/postgres/000154_drop_translation_updateat_index.down.sql channels/db/migrations/postgres/000154_drop_translation_updateat_index.up.sql channels/db/migrations/postgres/000155_create_translation_channel_updateat_index.down.sql channels/db/migrations/postgres/000155_create_translation_channel_updateat_index.up.sql +channels/db/migrations/postgres/000156_add_schemeid_to_roles.down.sql +channels/db/migrations/postgres/000156_add_schemeid_to_roles.up.sql +channels/db/migrations/postgres/000157_backfill_roles_schemeid.down.sql +channels/db/migrations/postgres/000157_backfill_roles_schemeid.up.sql +channels/db/migrations/postgres/000158_add_roles_schemeid_index.down.sql +channels/db/migrations/postgres/000158_add_roles_schemeid_index.up.sql diff --git a/server/channels/db/migrations/postgres/000156_add_schemeid_to_roles.down.sql b/server/channels/db/migrations/postgres/000156_add_schemeid_to_roles.down.sql new file mode 100644 index 00000000000..ac12dd12e4b --- /dev/null +++ b/server/channels/db/migrations/postgres/000156_add_schemeid_to_roles.down.sql @@ -0,0 +1 @@ +ALTER TABLE roles DROP COLUMN IF EXISTS schemeid; diff --git a/server/channels/db/migrations/postgres/000156_add_schemeid_to_roles.up.sql b/server/channels/db/migrations/postgres/000156_add_schemeid_to_roles.up.sql new file mode 100644 index 00000000000..6c3612417ea --- /dev/null +++ b/server/channels/db/migrations/postgres/000156_add_schemeid_to_roles.up.sql @@ -0,0 +1 @@ +ALTER TABLE roles ADD COLUMN IF NOT EXISTS schemeid VARCHAR(26); diff --git a/server/channels/db/migrations/postgres/000157_backfill_roles_schemeid.down.sql b/server/channels/db/migrations/postgres/000157_backfill_roles_schemeid.down.sql new file mode 100644 index 00000000000..efb09e806d4 --- /dev/null +++ b/server/channels/db/migrations/postgres/000157_backfill_roles_schemeid.down.sql @@ -0,0 +1 @@ +UPDATE roles SET schemeid = NULL; diff --git a/server/channels/db/migrations/postgres/000157_backfill_roles_schemeid.up.sql b/server/channels/db/migrations/postgres/000157_backfill_roles_schemeid.up.sql new file mode 100644 index 00000000000..1c4acba08bc --- /dev/null +++ b/server/channels/db/migrations/postgres/000157_backfill_roles_schemeid.up.sql @@ -0,0 +1,23 @@ +UPDATE roles +SET schemeid = match.scheme_id +FROM ( + SELECT DISTINCT ON (role_name) id AS scheme_id, role_name + FROM ( + SELECT id, unnest(ARRAY[ + defaultteamadminrole, + defaultteamuserrole, + defaultteamguestrole, + defaultchanneladminrole, + defaultchanneluserrole, + defaultchannelguestrole, + defaultplaybookadminrole, + defaultplaybookmemberrole, + defaultrunadminrole, + defaultrunmemberrole + ]) AS role_name + FROM schemes + ) expanded + WHERE role_name IS NOT NULL AND role_name <> '' + ORDER BY role_name, scheme_id +) match +WHERE roles.name = match.role_name; diff --git a/server/channels/db/migrations/postgres/000158_add_roles_schemeid_index.down.sql b/server/channels/db/migrations/postgres/000158_add_roles_schemeid_index.down.sql new file mode 100644 index 00000000000..61b69a26135 --- /dev/null +++ b/server/channels/db/migrations/postgres/000158_add_roles_schemeid_index.down.sql @@ -0,0 +1,2 @@ +-- morph:nontransactional +DROP INDEX CONCURRENTLY IF EXISTS idx_roles_scheme_id; diff --git a/server/channels/db/migrations/postgres/000158_add_roles_schemeid_index.up.sql b/server/channels/db/migrations/postgres/000158_add_roles_schemeid_index.up.sql new file mode 100644 index 00000000000..16b6d4198a1 --- /dev/null +++ b/server/channels/db/migrations/postgres/000158_add_roles_schemeid_index.up.sql @@ -0,0 +1,2 @@ +-- morph:nontransactional +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_roles_scheme_id ON roles(schemeid); diff --git a/server/channels/store/sqlstore/role_store.go b/server/channels/store/sqlstore/role_store.go index 809d4982ced..7a48391e94a 100644 --- a/server/channels/store/sqlstore/role_store.go +++ b/server/channels/store/sqlstore/role_store.go @@ -33,6 +33,7 @@ type Role struct { Permissions string SchemeManaged bool BuiltIn bool + SchemeId *string } type channelRolesPermissions struct { @@ -66,6 +67,7 @@ func NewRoleFromModel(role *model.Role) *Role { Permissions: permissions.String(), SchemeManaged: role.SchemeManaged, BuiltIn: role.BuiltIn, + SchemeId: role.SchemeId, } } @@ -81,6 +83,7 @@ func (role Role) ToModel() *model.Role { Permissions: strings.Fields(role.Permissions), SchemeManaged: role.SchemeManaged, BuiltIn: role.BuiltIn, + SchemeId: role.SchemeId, } } @@ -90,7 +93,7 @@ func newSqlRoleStore(sqlStore *SqlStore) store.RoleStore { } s.tableSelectQuery = s.getQueryBuilder(). - Select("Id", "Name", "DisplayName", "Description", "CreateAt", "UpdateAt", "DeleteAt", "Permissions", "SchemeManaged", "BuiltIn"). + Select("Id", "Name", "DisplayName", "Description", "CreateAt", "UpdateAt", "DeleteAt", "Permissions", "SchemeManaged", "BuiltIn", "SchemeId"). From("Roles") return &s @@ -122,9 +125,10 @@ func (s *SqlRoleStore) Save(role *model.Role) (_ *model.Role, err error) { dbRole.UpdateAt = model.GetMillis() res, err := s.GetMaster().NamedExec(`UPDATE Roles - SET UpdateAt=:UpdateAt, DeleteAt=:DeleteAt, CreateAt=:CreateAt, Name=:Name, DisplayName=:DisplayName, - Description=:Description, Permissions=:Permissions, SchemeManaged=:SchemeManaged, BuiltIn=:BuiltIn - WHERE Id=:Id`, &dbRole) + SET UpdateAt=:UpdateAt, DeleteAt=:DeleteAt, CreateAt=:CreateAt, Name=:Name, DisplayName=:DisplayName, + Description=:Description, Permissions=:Permissions, SchemeManaged=:SchemeManaged, BuiltIn=:BuiltIn, + SchemeId=:SchemeId + WHERE Id=:Id`, &dbRole) if err != nil { return nil, errors.Wrap(err, "failed to update Role") @@ -155,9 +159,9 @@ func (s *SqlRoleStore) createRole(role *model.Role, transaction *sqlxTxWrapper) dbRole.UpdateAt = dbRole.CreateAt if _, err := transaction.NamedExec(`INSERT INTO Roles - (Id, Name, DisplayName, Description, Permissions, CreateAt, UpdateAt, DeleteAt, SchemeManaged, BuiltIn) + (Id, Name, DisplayName, Description, Permissions, CreateAt, UpdateAt, DeleteAt, SchemeManaged, BuiltIn, SchemeId) VALUES - (:Id, :Name, :DisplayName, :Description, :Permissions, :CreateAt, :UpdateAt, :DeleteAt, :SchemeManaged, :BuiltIn)`, dbRole); err != nil { + (:Id, :Name, :DisplayName, :Description, :Permissions, :CreateAt, :UpdateAt, :DeleteAt, :SchemeManaged, :BuiltIn, :SchemeId)`, dbRole); err != nil { return nil, errors.Wrap(err, "failed to save Role") } @@ -230,7 +234,7 @@ func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, error) { err = rows.Scan( &role.Id, &role.Name, &role.DisplayName, &role.Description, &role.CreateAt, &role.UpdateAt, &role.DeleteAt, &role.Permissions, - &role.SchemeManaged, &role.BuiltIn) + &role.SchemeManaged, &role.BuiltIn, &role.SchemeId) if err != nil { return nil, errors.Wrap(err, "failed to scan values") } @@ -394,9 +398,10 @@ func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) { "Roles.Permissions", "Roles.SchemeManaged", "Roles.BuiltIn", + "Roles.SchemeId", ). - From("Schemes"). - Join("Roles ON Schemes.DefaultChannelGuestRole = Roles.Name OR Schemes.DefaultChannelUserRole = Roles.Name OR Schemes.DefaultChannelAdminRole = Roles.Name"). + From("Roles"). + Join("Schemes ON Roles.SchemeId = Schemes.Id"). Where(sq.Eq{"Schemes.Scope": model.SchemeScopeChannel}). Where(sq.Eq{"Roles.DeleteAt": 0}). Where(sq.Eq{"Schemes.DeleteAt": 0}) @@ -433,13 +438,14 @@ func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role "ChannelSchemeRoles.Permissions", "ChannelSchemeRoles.SchemeManaged", "ChannelSchemeRoles.BuiltIn", + "ChannelSchemeRoles.SchemeId", ). From("Roles AS HigherScopedRoles"). Join("Schemes AS HigherScopedSchemes ON (HigherScopedRoles.Name = HigherScopedSchemes.DefaultChannelGuestRole OR HigherScopedRoles.Name = HigherScopedSchemes.DefaultChannelUserRole OR HigherScopedRoles.Name = HigherScopedSchemes.DefaultChannelAdminRole)"). Join("Teams ON Teams.SchemeId = HigherScopedSchemes.Id"). Join("Channels ON Channels.TeamId = Teams.Id"). Join("Schemes AS ChannelSchemes ON Channels.SchemeId = ChannelSchemes.Id"). - Join("Roles AS ChannelSchemeRoles ON (ChannelSchemeRoles.Name = ChannelSchemes.DefaultChannelGuestRole OR ChannelSchemeRoles.Name = ChannelSchemes.DefaultChannelUserRole OR ChannelSchemeRoles.Name = ChannelSchemes.DefaultChannelAdminRole)"). + Join("Roles AS ChannelSchemeRoles ON ChannelSchemeRoles.SchemeId = ChannelSchemes.Id"). Where(sq.Eq{"HigherScopedSchemes.Scope": model.SchemeScopeTeam}). Where(sq.Eq{"HigherScopedRoles.Name": roleName}). Where(sq.Eq{"HigherScopedRoles.DeleteAt": 0}). diff --git a/server/channels/store/sqlstore/scheme_store.go b/server/channels/store/sqlstore/scheme_store.go index 182f7739015..c42962fd82c 100644 --- a/server/channels/store/sqlstore/scheme_store.go +++ b/server/channels/store/sqlstore/scheme_store.go @@ -100,6 +100,12 @@ func (s *SqlSchemeStore) Save(scheme *model.Scheme) (_ *model.Scheme, err error) } func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxWrapper) (*model.Scheme, error) { + // Generate the scheme ID up front so it can be recorded on each created role. + scheme.Id = model.NewId() + if scheme.Name == "" { + scheme.Name = model.NewId() + } + // Fetch the default system scheme roles to populate default permissions. defaultRoleNames := []string{ model.TeamAdminRoleId, @@ -135,6 +141,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNameTeamAdmin, scheme.Name), Permissions: defaultRoles[model.TeamAdminRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err := s.SqlStore.Role().(*SqlRoleStore).createRole(teamAdminRole, transaction) @@ -149,6 +156,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNameTeamUser, scheme.Name), Permissions: defaultRoles[model.TeamUserRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err = s.SqlStore.Role().(*SqlRoleStore).createRole(teamUserRole, transaction) @@ -163,6 +171,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNameTeamGuest, scheme.Name), Permissions: defaultRoles[model.TeamGuestRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err = s.SqlStore.Role().(*SqlRoleStore).createRole(teamGuestRole, transaction) @@ -177,6 +186,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNamePlaybookAdmin, scheme.Name), Permissions: defaultRoles[model.PlaybookAdminRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err = s.SqlStore.Role().(*SqlRoleStore).createRole(playbookAdminRole, transaction) if err != nil { @@ -190,6 +200,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNamePlaybookMember, scheme.Name), Permissions: defaultRoles[model.PlaybookMemberRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err = s.SqlStore.Role().(*SqlRoleStore).createRole(playbookMemberRole, transaction) if err != nil { @@ -203,6 +214,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNameRunAdmin, scheme.Name), Permissions: defaultRoles[model.RunAdminRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err = s.SqlStore.Role().(*SqlRoleStore).createRole(runAdminRole, transaction) if err != nil { @@ -216,6 +228,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("%s %s", SchemeRoleDisplayNameRunMember, scheme.Name), Permissions: defaultRoles[model.RunMemberRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } savedRole, err = s.SqlStore.Role().(*SqlRoleStore).createRole(runMemberRole, transaction) if err != nil { @@ -231,6 +244,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("Channel Admin Role for Scheme %s", scheme.Name), Permissions: defaultRoles[model.ChannelAdminRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } if scheme.Scope == model.SchemeScopeChannel { @@ -249,6 +263,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("Channel User Role for Scheme %s", scheme.Name), Permissions: defaultRoles[model.ChannelUserRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } if scheme.Scope == model.SchemeScopeChannel { @@ -267,6 +282,7 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW DisplayName: fmt.Sprintf("Channel Guest Role for Scheme %s", scheme.Name), Permissions: defaultRoles[model.ChannelGuestRoleId].Permissions, SchemeManaged: true, + SchemeId: &scheme.Id, } if scheme.Scope == model.SchemeScopeChannel { @@ -280,10 +296,6 @@ func (s *SqlSchemeStore) createScheme(scheme *model.Scheme, transaction *sqlxTxW scheme.DefaultChannelGuestRole = savedRole.Name } - scheme.Id = model.NewId() - if scheme.Name == "" { - scheme.Name = model.NewId() - } scheme.CreateAt = model.GetMillis() scheme.UpdateAt = scheme.CreateAt @@ -363,23 +375,11 @@ func (s *SqlSchemeStore) Delete(schemeId string) (*model.Scheme, error) { s.Channel().ClearCaches() // Delete the roles belonging to the scheme. - roleNames := []string{scheme.DefaultChannelGuestRole, scheme.DefaultChannelUserRole, scheme.DefaultChannelAdminRole} - if scheme.Scope == model.SchemeScopeTeam { - roleNames = append(roleNames, scheme.DefaultTeamGuestRole, scheme.DefaultTeamUserRole, scheme.DefaultTeamAdminRole) - } - if scheme.Scope == model.SchemeScopePlaybook { - roleNames = append(roleNames, scheme.DefaultPlaybookAdminRole, scheme.DefaultPlaybookMemberRole) - } - - if scheme.Scope == model.SchemeScopeRun { - roleNames = append(roleNames, scheme.DefaultRunAdminRole, scheme.DefaultRunMemberRole) - } - time := model.GetMillis() updateQuery, args, err := s.getQueryBuilder(). Update("Roles"). - Where(sq.Eq{"Name": roleNames}). + Where(sq.Eq{"SchemeId": schemeId}). Set("UpdateAt", time). Set("DeleteAt", time). ToSql() @@ -388,7 +388,7 @@ func (s *SqlSchemeStore) Delete(schemeId string) (*model.Scheme, error) { } if _, err = s.GetMaster().Exec(updateQuery, args...); err != nil { - return nil, errors.Wrapf(err, "failed to update Roles with name in (%s)", roleNames) + return nil, errors.Wrapf(err, "failed to update Roles with SchemeId=%s", schemeId) } // Delete the scheme itself. diff --git a/server/channels/store/storetest/role_store.go b/server/channels/store/storetest/role_store.go index d7dcb8bab7f..2e33d98b90a 100644 --- a/server/channels/store/storetest/role_store.go +++ b/server/channels/store/storetest/role_store.go @@ -456,39 +456,45 @@ func testRoleStoreLowerScopedChannelSchemeRoles(t *testing.T, rctx request.CTX, actualRoles, err := ss.Role().ChannelRolesUnderTeamRole(teamScheme1.DefaultChannelGuestRole) require.NoError(t, err) - var actualRoleNames []string + roleByName := make(map[string]*model.Role, len(actualRoles)) for _, role := range actualRoles { - actualRoleNames = append(actualRoleNames, role.Name) + roleByName[role.Name] = role } - require.Contains(t, actualRoleNames, channelScheme1.DefaultChannelGuestRole) - require.NotContains(t, actualRoleNames, channelScheme2.DefaultChannelGuestRole) + require.Contains(t, roleByName, channelScheme1.DefaultChannelGuestRole) + require.NotContains(t, roleByName, channelScheme2.DefaultChannelGuestRole) + require.NotNil(t, roleByName[channelScheme1.DefaultChannelGuestRole].SchemeId) + assert.Equal(t, channelScheme1.Id, *roleByName[channelScheme1.DefaultChannelGuestRole].SchemeId) }) t.Run("user role for the right team's channels are returned", func(t *testing.T) { actualRoles, err := ss.Role().ChannelRolesUnderTeamRole(teamScheme1.DefaultChannelUserRole) require.NoError(t, err) - var actualRoleNames []string + roleByName := make(map[string]*model.Role, len(actualRoles)) for _, role := range actualRoles { - actualRoleNames = append(actualRoleNames, role.Name) + roleByName[role.Name] = role } - require.Contains(t, actualRoleNames, channelScheme1.DefaultChannelUserRole) - require.NotContains(t, actualRoleNames, channelScheme2.DefaultChannelUserRole) + require.Contains(t, roleByName, channelScheme1.DefaultChannelUserRole) + require.NotContains(t, roleByName, channelScheme2.DefaultChannelUserRole) + require.NotNil(t, roleByName[channelScheme1.DefaultChannelUserRole].SchemeId) + assert.Equal(t, channelScheme1.Id, *roleByName[channelScheme1.DefaultChannelUserRole].SchemeId) }) t.Run("admin role for the right team's channels are returned", func(t *testing.T) { actualRoles, err := ss.Role().ChannelRolesUnderTeamRole(teamScheme1.DefaultChannelAdminRole) require.NoError(t, err) - var actualRoleNames []string + roleByName := make(map[string]*model.Role, len(actualRoles)) for _, role := range actualRoles { - actualRoleNames = append(actualRoleNames, role.Name) + roleByName[role.Name] = role } - require.Contains(t, actualRoleNames, channelScheme1.DefaultChannelAdminRole) - require.NotContains(t, actualRoleNames, channelScheme2.DefaultChannelAdminRole) + require.Contains(t, roleByName, channelScheme1.DefaultChannelAdminRole) + require.NotContains(t, roleByName, channelScheme2.DefaultChannelAdminRole) + require.NotNil(t, roleByName[channelScheme1.DefaultChannelAdminRole].SchemeId) + assert.Equal(t, channelScheme1.Id, *roleByName[channelScheme1.DefaultChannelAdminRole].SchemeId) }) }) @@ -497,9 +503,9 @@ func testRoleStoreLowerScopedChannelSchemeRoles(t *testing.T, rctx request.CTX, actualRoles, err := ss.Role().AllChannelSchemeRoles() require.NoError(t, err) - var actualRoleNames []string + roleByName := make(map[string]*model.Role, len(actualRoles)) for _, role := range actualRoles { - actualRoleNames = append(actualRoleNames, role.Name) + roleByName[role.Name] = role } allRoleNames := []string{ @@ -514,7 +520,27 @@ func testRoleStoreLowerScopedChannelSchemeRoles(t *testing.T, rctx request.CTX, } for _, roleName := range allRoleNames { - require.Contains(t, actualRoleNames, roleName) + require.Contains(t, roleByName, roleName) + } + + // Roles for channelScheme1 must carry channelScheme1's ID. + for _, roleName := range []string{ + channelScheme1.DefaultChannelGuestRole, + channelScheme1.DefaultChannelUserRole, + channelScheme1.DefaultChannelAdminRole, + } { + require.NotNil(t, roleByName[roleName].SchemeId) + assert.Equal(t, channelScheme1.Id, *roleByName[roleName].SchemeId) + } + + // Roles for channelScheme2 must carry channelScheme2's ID. + for _, roleName := range []string{ + channelScheme2.DefaultChannelGuestRole, + channelScheme2.DefaultChannelUserRole, + channelScheme2.DefaultChannelAdminRole, + } { + require.NotNil(t, roleByName[roleName].SchemeId) + assert.Equal(t, channelScheme2.Id, *roleByName[roleName].SchemeId) } }) }) diff --git a/server/channels/store/storetest/scheme_store.go b/server/channels/store/storetest/scheme_store.go index 386fb86dcac..5e8f0962fc2 100644 --- a/server/channels/store/storetest/scheme_store.go +++ b/server/channels/store/storetest/scheme_store.go @@ -174,6 +174,28 @@ func testSchemeStoreSave(t *testing.T, rctx request.CTX, ss store.Store) { assert.Equal(t, role6.Permissions, []string{"read_channel", "read_channel_content", "create_post"}) assert.True(t, role6.SchemeManaged) + role7, err := ss.Role().GetByName(context.Background(), d1.DefaultPlaybookAdminRole) + assert.NoError(t, err) + assert.True(t, role7.SchemeManaged) + + role8, err := ss.Role().GetByName(context.Background(), d1.DefaultPlaybookMemberRole) + assert.NoError(t, err) + assert.True(t, role8.SchemeManaged) + + role9, err := ss.Role().GetByName(context.Background(), d1.DefaultRunAdminRole) + assert.NoError(t, err) + assert.True(t, role9.SchemeManaged) + + role10, err := ss.Role().GetByName(context.Background(), d1.DefaultRunMemberRole) + assert.NoError(t, err) + assert.True(t, role10.SchemeManaged) + + // Every role created for a scheme must carry the scheme's ID. + for _, role := range []*model.Role{role1, role2, role3, role4, role5, role6, role7, role8, role9, role10} { + require.NotNil(t, role.SchemeId) + assert.Equal(t, d1.Id, *role.SchemeId) + } + // Change the scheme description and update. d1.Description = model.NewId() diff --git a/server/i18n/en.json b/server/i18n/en.json index 4c6e2950cb6..4b32b304944 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -7674,6 +7674,14 @@ "id": "app.role.save.invalid_role.app_error", "translation": "The role was not valid." }, + { + "id": "app.role.send_updated_role_event.app_error", + "translation": "An error occurred while broadcasting the role update." + }, + { + "id": "app.role.send_updated_role_event.unknown_scope", + "translation": "An error occurred while broadcasting the role update: unknown scheme scope." + }, { "id": "app.save_config.app_error", "translation": "An error occurred saving the configuration." diff --git a/server/public/model/role.go b/server/public/model/role.go index a29ea8f0e37..3f8d48565f5 100644 --- a/server/public/model/role.go +++ b/server/public/model/role.go @@ -432,6 +432,7 @@ type Role struct { Permissions []string `json:"permissions"` SchemeManaged bool `json:"scheme_managed"` BuiltIn bool `json:"built_in"` + SchemeId *string `json:"scheme_id"` } func (r *Role) Auditable() map[string]any { @@ -446,6 +447,7 @@ func (r *Role) Auditable() map[string]any { "permissions": r.Permissions, "scheme_managed": r.SchemeManaged, "built_in": r.BuiltIn, + "scheme_id": r.SchemeId, } } @@ -466,6 +468,7 @@ func (r *Role) MarshalYAML() (any, error) { Permissions []string `yaml:"permissions"` SchemeManaged bool `yaml:"scheme_managed"` BuiltIn bool `yaml:"built_in"` + SchemeId *string `yaml:"scheme_id"` }{ Id: r.Id, Name: r.Name, @@ -477,6 +480,7 @@ func (r *Role) MarshalYAML() (any, error) { Permissions: r.Permissions, SchemeManaged: r.SchemeManaged, BuiltIn: r.BuiltIn, + SchemeId: r.SchemeId, }, nil } @@ -492,6 +496,7 @@ func (r *Role) UnmarshalYAML(unmarshal func(any) error) error { Permissions []string `yaml:"permissions"` SchemeManaged bool `yaml:"scheme_managed"` BuiltIn bool `yaml:"built_in"` + SchemeId *string `yaml:"scheme_id"` }{} err := unmarshal(&out) @@ -523,6 +528,7 @@ func (r *Role) UnmarshalYAML(unmarshal func(any) error) error { Permissions: out.Permissions, SchemeManaged: out.SchemeManaged, BuiltIn: out.BuiltIn, + SchemeId: out.SchemeId, } return nil }