diff --git a/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_e20_1_spec.js b/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_e20_1_spec.js index 10bd7234dbf..cc322a28898 100644 --- a/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_e20_1_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/system_console/site_configuration/link_customization_e20_1_spec.js @@ -130,7 +130,7 @@ describe('SupportSettings', () => { it('MM-T1038 - Customization App download link - Change to different', () => { // # Edit links in the support email field - const link = 'some_link'; + const link = 'https://github.com/mattermost/desktop/releases'; cy.findByTestId('NativeAppSettings.AppDownloadLinkinput').clear().type(link); // # Save setting then back to team view diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index 952c1d980c8..0f6b5db44d1 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -1046,6 +1046,12 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Users who can't create posts in a channel shouldn't be able to edit them either. + userCreatePostPermissionCheckWithContext(c, originalPost.ChannelId) + if c.Err != nil { + return + } + auditRec.AddEventPriorState(originalPost) auditRec.AddEventObjectType("post") @@ -1183,6 +1189,12 @@ func postPatchChecks(c *Context, auditRec *model.AuditRecord, message *string) b return false } + // Users who can't create posts in a channel shouldn't be able to edit them either. + userCreatePostPermissionCheckWithContext(c, originalPost.ChannelId) + if c.Err != nil { + return false + } + if *c.App.Config().ServiceSettings.PostEditTimeLimit != -1 && model.GetMillis() > originalPost.CreateAt+int64(*c.App.Config().ServiceSettings.PostEditTimeLimit*1000) && message != nil { c.Err = model.NewAppError("patchPost", "api.post.update_post.permissions_time_limit.app_error", map[string]any{"timeLimit": *c.App.Config().ServiceSettings.PostEditTimeLimit}, "", http.StatusBadRequest) return isMember diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index efff02abdcc..9697fde175d 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -1701,6 +1701,29 @@ func TestUpdatePost(t *testing.T) { assert.Contains(t, updatedPost.FileIds, fileId) }) + t.Run("should prevent editing when create_post permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + postToEdit, _, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "original message", + }, channel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + th.RemovePermissionFromRole(t, model.PermissionCreatePost.Id, model.ChannelUserRoleId) + defer th.AddPermissionToRole(t, model.PermissionCreatePost.Id, model.ChannelUserRoleId) + + updatePost := &model.Post{ + Id: postToEdit.Id, + ChannelId: channel.Id, + Message: "edited message", + } + _, resp, err := client.UpdatePost(context.Background(), postToEdit.Id, updatePost) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + t.Run("logged out", func(t *testing.T) { _, err := client.Logout(context.Background()) require.NoError(t, err) @@ -2085,6 +2108,27 @@ func TestPatchPost(t *testing.T) { require.NoError(t, err) }) + t.Run("should prevent patching when create_post permission is revoked", func(t *testing.T) { + th.LoginBasic(t) + + postToEdit, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "original message", + }) + require.NoError(t, err) + + defaultPerms := th.SaveDefaultRolePermissions(t) + defer th.RestoreDefaultRolePermissions(t, defaultPerms) + th.RemovePermissionFromRole(t, model.PermissionCreatePost.Id, model.ChannelUserRoleId) + + patch := &model.PostPatch{ + Message: model.NewPointer("edited message"), + } + _, resp, err := client.PatchPost(context.Background(), postToEdit.Id, patch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + t.Run("time limit expired", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.PostEditTimeLimit = 1 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/enterprise/elasticsearch/common/common.go b/server/enterprise/elasticsearch/common/common.go index 255d366ee1b..0ec56b0e9ca 100644 --- a/server/enterprise/elasticsearch/common/common.go +++ b/server/enterprise/elasticsearch/common/common.go @@ -133,7 +133,7 @@ func ESPostFromPostForIndexing(post *model.PostForIndexing) *ESPost { if !mOk { continue } - for _, key := range []string{"text", "title", "pretext", "fallback"} { + for _, key := range []string{"text", "title", "pretext", "fallback", "footer", "author_name"} { if s, _ := m[key].(string); s != "" { searchAttachments = append(searchAttachments, s) } @@ -165,7 +165,7 @@ func ESPostFromPostForIndexing(post *model.PostForIndexing) *ESPost { if attachment == nil { continue } - for _, s := range []string{attachment.Text, attachment.Title, attachment.Pretext, attachment.Fallback} { + for _, s := range []string{attachment.Text, attachment.Title, attachment.Pretext, attachment.Fallback, attachment.Footer, attachment.AuthorName} { if s != "" { searchAttachments = append(searchAttachments, s) } diff --git a/server/enterprise/elasticsearch/common/common_test.go b/server/enterprise/elasticsearch/common/common_test.go index 3715a4520b8..b112ce7de70 100644 --- a/server/enterprise/elasticsearch/common/common_test.go +++ b/server/enterprise/elasticsearch/common/common_test.go @@ -4,6 +4,7 @@ package common import ( + "encoding/json" "testing" "time" @@ -319,6 +320,64 @@ func TestESPostFromPostForIndexing(t *testing.T) { assert.Contains(t, espost.Attachments, "99.5") }) + t.Run("any form indexes footer and author_name", func(t *testing.T) { + post := model.PostForIndexing{ + TeamId: model.NewId(), + Post: model.Post{ + Id: model.NewId(), + ChannelId: model.NewId(), + UserId: model.NewId(), + CreateAt: model.GetMillis(), + Message: "", + Type: "slack_attachment", + Props: map[string]any{ + model.PostPropsAttachments: []any{ + map[string]any{ + "text": "body text", + "footer": "Opportunity #OPP-000035341 • United States", + "author_name": "Salesforce", + }, + }, + }, + }, + } + + espost := ESPostFromPostForIndexing(&post) + + assert.Contains(t, espost.Attachments, "body text") + assert.Contains(t, espost.Attachments, "Opportunity #OPP-000035341 • United States") + assert.Contains(t, espost.Attachments, "Salesforce") + }) + + t.Run("MessageAttachment form indexes footer and author_name", func(t *testing.T) { + post := model.PostForIndexing{ + TeamId: model.NewId(), + Post: model.Post{ + Id: model.NewId(), + ChannelId: model.NewId(), + UserId: model.NewId(), + CreateAt: model.GetMillis(), + Message: "", + Type: "slack_attachment", + Props: map[string]any{ + model.PostPropsAttachments: []*model.MessageAttachment{ + { + Text: "body text", + Footer: "Opportunity #OPP-000035341 • United States", + AuthorName: "Salesforce", + }, + }, + }, + }, + } + + espost := ESPostFromPostForIndexing(&post) + + assert.Contains(t, espost.Attachments, "body text") + assert.Contains(t, espost.Attachments, "Opportunity #OPP-000035341 • United States") + assert.Contains(t, espost.Attachments, "Salesforce") + }) + t.Run("multiple attachments are combined", func(t *testing.T) { post := model.PostForIndexing{ TeamId: model.NewId(), @@ -353,6 +412,121 @@ func TestESPostFromPostForIndexing(t *testing.T) { }) } +// TestESPostFromPost_CreatePostJSONRoundTrip simulates the exact flow that happens +// in production: App.CreatePost converts []*MessageAttachment to []any via JSON +// marshal/unmarshal (Post.Attachments in post.go), then the search layer calls +// ESPostFromPost which does ShallowCopy → ESPostFromPostForIndexing. +func TestESPostFromPost_CreatePostJSONRoundTrip(t *testing.T) { + // Step 1: Start with typed MessageAttachment (as a plugin/webhook would create) + original := &model.Post{ + Id: model.NewId(), + ChannelId: model.NewId(), + UserId: model.NewId(), + CreateAt: model.GetMillis(), + Message: "#closedwon #renewal", + Type: "slack_attachment", + } + original.AddProp(model.PostPropsAttachments, []*model.MessageAttachment{ + { + Title: "Account: Acme Corp / Widget Industries, LLC", + Text: "Renewal ARR: $49,140.00", + Pretext: "#closedwon #renewal", + Footer: "Opportunity #OPP-000035341 • United States", + AuthorName: "CRM Bot", + Fields: []*model.MessageAttachmentField{ + {Title: "Sales Rep", Value: "Jane Smith"}, + {Title: "Account Manager", Value: "John Doe"}, + {Title: "Opportunity", Value: "Acme Corp / Widget Industries, LLC - Enterprise - 350 Seats - '26 Renewal"}, + {Title: "Seats", Value: 350.0}, + }, + }, + }) + + // Step 2: Simulate CreatePost JSON round-trip (post.go:305-315) + if attachments, ok := original.GetProp(model.PostPropsAttachments).([]*model.MessageAttachment); ok { + jsonAttachments, err := json.Marshal(attachments) + require.NoError(t, err) + attachmentsInterface := []any{} + err = json.Unmarshal(jsonAttachments, &attachmentsInterface) + require.NoError(t, err) + original.AddProp(model.PostPropsAttachments, attachmentsInterface) + } else { + require.Fail(t, "expected []*MessageAttachment type assertion to succeed") + } + + // Step 3: Simulate ESPostFromPost (what the search layer calls) + teamId := model.NewId() + esPost, err := ESPostFromPost(original, teamId, "O") + require.NoError(t, err) + + // Step 4: Verify all attachment fields are indexed + assert.Contains(t, esPost.Attachments, "Account: Acme Corp / Widget Industries, LLC", "title should be indexed") + assert.Contains(t, esPost.Attachments, "Renewal ARR: $49,140.00", "text should be indexed") + assert.Contains(t, esPost.Attachments, "#closedwon #renewal", "pretext should be indexed") + + // Field titles and values + assert.Contains(t, esPost.Attachments, "Sales Rep", "field title should be indexed") + assert.Contains(t, esPost.Attachments, "Jane Smith", "field value should be indexed") + assert.Contains(t, esPost.Attachments, "Account Manager", "field title should be indexed") + assert.Contains(t, esPost.Attachments, "John Doe", "field value should be indexed") + assert.Contains(t, esPost.Attachments, "Opportunity", "field title should be indexed") + assert.Contains(t, esPost.Attachments, "Acme Corp / Widget Industries, LLC - Enterprise - 350 Seats - '26 Renewal", "field value should be indexed") + assert.Contains(t, esPost.Attachments, "350", "numeric field value should be indexed") + + // Footer and AuthorName + assert.Contains(t, esPost.Attachments, "Opportunity #OPP-000035341 • United States", "footer should be indexed") + assert.Contains(t, esPost.Attachments, "CRM Bot", "author_name should be indexed") + + t.Logf("Attachments field content: %s", esPost.Attachments) +} + +// TestESPostFromPost_APIJSONUnmarshal simulates a post created via the REST API +// where Props are deserialized from JSON (attachments arrive as []any directly). +func TestESPostFromPost_APIJSONUnmarshal(t *testing.T) { + // Simulate what happens when a bot POSTs JSON to /api/v4/posts + postJSON := `{ + "channel_id": "` + model.NewId() + `", + "message": "", + "props": { + "attachments": [{ + "title": "Account: Acme Corp", + "text": "Renewal ARR: $49,140.00", + "footer": "Opportunity #OPP-000035341", + "author_name": "CRM Bot", + "fields": [ + {"title": "Sales Rep", "value": "Jane Smith"}, + {"title": "Seats", "value": 350} + ] + }] + } + }` + + var post model.Post + err := json.Unmarshal([]byte(postJSON), &post) + require.NoError(t, err) + post.Id = model.NewId() + post.UserId = model.NewId() + post.CreateAt = model.GetMillis() + + // The CreatePost conversion at post.go:305-315 would NOT trigger here + // because the type is already []any, not []*MessageAttachment + _, typeOk := post.GetProp(model.PostPropsAttachments).([]*model.MessageAttachment) + assert.False(t, typeOk, "API-created post should have []any, not []*MessageAttachment") + + esPost, err := ESPostFromPost(&post, model.NewId(), "O") + require.NoError(t, err) + + assert.Contains(t, esPost.Attachments, "Account: Acme Corp", "title should be indexed") + assert.Contains(t, esPost.Attachments, "Renewal ARR: $49,140.00", "text should be indexed") + assert.Contains(t, esPost.Attachments, "Sales Rep", "field title should be indexed") + assert.Contains(t, esPost.Attachments, "Jane Smith", "field value should be indexed") + assert.Contains(t, esPost.Attachments, "350", "numeric field value should be indexed") + assert.Contains(t, esPost.Attachments, "Opportunity #OPP-000035341", "footer should be indexed") + assert.Contains(t, esPost.Attachments, "CRM Bot", "author_name should be indexed") + + t.Logf("Attachments field content: %s", esPost.Attachments) +} + func TestGetMatchesForHit(t *testing.T) { snippets := map[string][]string{ "message": { diff --git a/server/i18n/en.json b/server/i18n/en.json index 4c6e2950cb6..a64e3266e1d 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." @@ -10460,6 +10468,10 @@ "id": "model.config.is_valid.move_thread.domain_invalid.app_error", "translation": "Invalid domain for move thread settings" }, + { + "id": "model.config.is_valid.native_app_settings.download_link.app_error", + "translation": "One or more app download links are not valid URLs." + }, { "id": "model.config.is_valid.notification_settings.invalid_event", "translation": "Invalid flagging event specified." diff --git a/server/public/model/config.go b/server/public/model/config.go index 370e3863241..2f4eb7a0b01 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -3093,6 +3093,19 @@ func (s *NativeAppSettings) SetDefaults() { } } +func (s *NativeAppSettings) AreDownloadLinksValid() *AppError { + for _, link := range []*string{s.AppDownloadLink, s.AndroidAppDownloadLink, s.IosAppDownloadLink} { + if link == nil || *link == "" { + continue + } + u, err := url.ParseRequestURI(*link) + if err != nil || u.Scheme == "" || u.Hostname() == "" { + return NewAppError("NativeAppSettings.AreDownloadLinksValid", "model.config.is_valid.native_app_settings.download_link.app_error", nil, "", http.StatusBadRequest) + } + } + return nil +} + type ElasticsearchSettings struct { ConnectionURL *string `access:"environment_elasticsearch,write_restrictable,cloud_restrictable"` Backend *string `access:"environment_elasticsearch,write_restrictable,cloud_restrictable"` @@ -4164,6 +4177,10 @@ func (o *Config) IsValid() *AppError { return appErr } + if appErr := o.NativeAppSettings.AreDownloadLinksValid(); appErr != nil { + return appErr + } + // Cross-reference validation: IntuneSettings requires either Office365 or SAML to be enabled if o.IntuneSettings.Enable != nil && *o.IntuneSettings.Enable { if o.IntuneSettings.AuthService != nil && *o.IntuneSettings.AuthService != "" { diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index 112e38d30f0..0991f8a214a 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -2905,3 +2905,63 @@ func TestConfigAccessTagsMapToValidPermissions(t *testing.T) { checkStruct(t, reflect.TypeFor[Config](), "Config") } + +func TestNativeAppSettingsIsValid(t *testing.T) { + t.Run("defaults are valid", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + require.Nil(t, cfg.NativeAppSettings.AreDownloadLinksValid()) + }) + + t.Run("empty string is valid (hides the menu item)", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + *cfg.NativeAppSettings.AppDownloadLink = "" + *cfg.NativeAppSettings.AndroidAppDownloadLink = "" + *cfg.NativeAppSettings.IosAppDownloadLink = "" + require.Nil(t, cfg.NativeAppSettings.AreDownloadLinksValid()) + }) + + t.Run("valid https URLs are accepted", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + *cfg.NativeAppSettings.AppDownloadLink = "https://example.com/download" + *cfg.NativeAppSettings.AndroidAppDownloadLink = "https://play.google.com/store/apps/details?id=com.mattermost.rn" + *cfg.NativeAppSettings.IosAppDownloadLink = "https://apps.apple.com/us/app/mattermost/id1257222717" + require.Nil(t, cfg.NativeAppSettings.AreDownloadLinksValid()) + }) + + t.Run("custom scheme URLs are accepted for enterprise use cases", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + *cfg.NativeAppSettings.AppDownloadLink = "myapp://install/mattermost" + require.Nil(t, cfg.NativeAppSettings.AreDownloadLinksValid()) + }) + + t.Run("malformed AppDownloadLink is rejected", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + *cfg.NativeAppSettings.AppDownloadLink = "http://://mattermost.com" + appErr := cfg.NativeAppSettings.AreDownloadLinksValid() + require.NotNil(t, appErr) + require.Equal(t, "model.config.is_valid.native_app_settings.download_link.app_error", appErr.Id) + }) + + t.Run("malformed AndroidAppDownloadLink is rejected", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + *cfg.NativeAppSettings.AndroidAppDownloadLink = "not-a-url" + appErr := cfg.NativeAppSettings.AreDownloadLinksValid() + require.NotNil(t, appErr) + require.Equal(t, "model.config.is_valid.native_app_settings.download_link.app_error", appErr.Id) + }) + + t.Run("malformed IosAppDownloadLink is rejected", func(t *testing.T) { + cfg := Config{} + cfg.SetDefaults() + *cfg.NativeAppSettings.IosAppDownloadLink = "not-a-url" + appErr := cfg.NativeAppSettings.AreDownloadLinksValid() + require.NotNil(t, appErr) + require.Equal(t, "model.config.is_valid.native_app_settings.download_link.app_error", appErr.Id) + }) +} 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 } diff --git a/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.test.tsx b/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.test.tsx index d882a9f2c87..be739dfe697 100644 --- a/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.test.tsx +++ b/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.test.tsx @@ -64,6 +64,27 @@ describe('components/user_settings/display/user_settings_theme/user_settings_the expect(requiredProps.actions.saveTheme).toHaveBeenCalled(); }); + it('should show premade themes when custom themes are disabled', () => { + const props = { + ...requiredProps, + selected: true, + allowCustomThemes: false, + }; + + renderWithContext( + , + ); + + // Premade theme chooser should still be rendered + expect(screen.getByText('Save')).toBeInTheDocument(); + expect(screen.queryByLabelText('Premade Themes')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Custom Theme')).not.toBeInTheDocument(); + + // The premade themes should be visible (theme thumbnails are rendered) + const premadeThemes = document.querySelectorAll('.premade-themes'); + expect(premadeThemes.length).toBeGreaterThan(0); + }); + it('should deleteTeamSpecificThemes if applyToAllTeams is enabled', async () => { const props = { ...requiredProps, diff --git a/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.tsx b/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.tsx index 02927d1dad4..0a076ba9912 100644 --- a/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.tsx +++ b/webapp/channels/src/components/user_settings/display/user_settings_theme/user_settings_theme.tsx @@ -238,6 +238,8 @@ export default class ThemeSetting extends React.PureComponent { , ); + } else { + inputs.push(premade); } let allTeamsCheckbox = null;