diff --git a/api4/role.go b/api4/role.go index 842acae4c4..944b290dbb 100644 --- a/api4/role.go +++ b/api4/role.go @@ -10,6 +10,24 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) +var allowedPermissions = []string{ + model.PERMISSION_CREATE_TEAM.Id, + model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, + model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, + model.PERMISSION_MANAGE_SLASH_COMMANDS.Id, + model.PERMISSION_MANAGE_OAUTH.Id, + model.PERMISSION_MANAGE_SYSTEM_WIDE_OAUTH.Id, + model.PERMISSION_CREATE_EMOJIS.Id, + model.PERMISSION_DELETE_EMOJIS.Id, + model.PERMISSION_EDIT_OTHERS_POSTS.Id, +} + +var notAllowedPermissions = []string{ + model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES.Id, + model.PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_SYSTEM_ROLES.Id, + model.PERMISSION_MANAGE_ROLES.Id, +} + func (api *API) InitRole() { api.BaseRoutes.Roles.Handle("/{role_id:[A-Za-z0-9]+}", api.ApiSessionRequiredTrustRequester(getRole)).Methods("GET") api.BaseRoutes.Roles.Handle("/name/{role_name:[a-z0-9_]+}", api.ApiSessionRequiredTrustRequester(getRoleByName)).Methods("GET") @@ -92,22 +110,17 @@ func patchRole(c *Context, w http.ResponseWriter, r *http.Request) { } auditRec.AddMeta("role", oldRole) + if oldRole.Name == model.SYSTEM_ADMIN_ROLE_ID { + c.Err = model.NewAppError("Api4.PatchRoles", "api.roles.patch_roles.admin_role.error", nil, "", http.StatusNotImplemented) + return + } + + isGuest := oldRole.Name == model.SYSTEM_GUEST_ROLE_ID || oldRole.Name == model.TEAM_GUEST_ROLE_ID || oldRole.Name == model.CHANNEL_GUEST_ROLE_ID if c.App.Srv().License() == nil && patch.Permissions != nil { - if oldRole.Name == "system_guest" || oldRole.Name == "team_guest" || oldRole.Name == "channel_guest" { + if isGuest { c.Err = model.NewAppError("Api4.PatchRoles", "api.roles.patch_roles.license.error", nil, "", http.StatusNotImplemented) return } - allowedPermissions := []string{ - model.PERMISSION_CREATE_TEAM.Id, - model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, - model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, - model.PERMISSION_MANAGE_SLASH_COMMANDS.Id, - model.PERMISSION_MANAGE_OAUTH.Id, - model.PERMISSION_MANAGE_SYSTEM_WIDE_OAUTH.Id, - model.PERMISSION_CREATE_EMOJIS.Id, - model.PERMISSION_DELETE_EMOJIS.Id, - model.PERMISSION_EDIT_OTHERS_POSTS.Id, - } changedPermissions := model.PermissionsChangedByPatch(oldRole, patch) for _, permission := range changedPermissions { @@ -125,14 +138,39 @@ func patchRole(c *Context, w http.ResponseWriter, r *http.Request) { } } - if c.App.Srv().License() != nil && (oldRole.Name == "system_guest" || oldRole.Name == "team_guest" || oldRole.Name == "channel_guest") && !*c.App.Srv().License().Features.GuestAccountsPermissions { + if patch.Permissions != nil { + deltaPermissions := model.PermissionsChangedByPatch(oldRole, patch) + + for _, permission := range deltaPermissions { + notAllowed := false + for _, notAllowedPermission := range notAllowedPermissions { + if permission == notAllowedPermission { + notAllowed = true + } + } + + if notAllowed { + c.Err = model.NewAppError("Api4.PatchRoles", "api.roles.patch_roles.not_allowed_permission.error", nil, "Cannot add or remove permission: "+permission, http.StatusNotImplemented) + return + } + } + } + + if c.App.Srv().License() != nil && isGuest && !*c.App.Srv().License().Features.GuestAccountsPermissions { c.Err = model.NewAppError("Api4.PatchRoles", "api.roles.patch_roles.license.error", nil, "", http.StatusNotImplemented) return } - if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_PERMISSIONS) { - c.SetPermissionError(model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_PERMISSIONS) - return + if oldRole.Name == model.TEAM_ADMIN_ROLE_ID || oldRole.Name == model.CHANNEL_ADMIN_ROLE_ID || oldRole.Name == model.SYSTEM_USER_ROLE_ID || oldRole.Name == model.TEAM_USER_ROLE_ID || oldRole.Name == model.CHANNEL_USER_ROLE_ID || oldRole.Name == model.SYSTEM_GUEST_ROLE_ID || oldRole.Name == model.TEAM_GUEST_ROLE_ID || oldRole.Name == model.CHANNEL_GUEST_ROLE_ID { + if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_PERMISSIONS) { + c.SetPermissionError(model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_PERMISSIONS) + return + } + } else { + if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES) { + c.SetPermissionError(model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES) + return + } } role, err := c.App.PatchRole(oldRole, patch) diff --git a/api4/role_test.go b/api4/role_test.go index 53f0b40c6e..e4e2dbeba5 100644 --- a/api4/role_test.go +++ b/api4/role_test.go @@ -177,6 +177,43 @@ func TestPatchRole(t *testing.T) { Permissions: &[]string{"manage_system", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"}, } + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + + // Cannot edit a system admin + adminRole, err := th.App.Srv().Store.Role().GetByName("system_admin") + assert.Nil(t, err) + defer th.App.Srv().Store.Job().Delete(adminRole.Id) + + _, resp := client.PatchRole(adminRole.Id, patch) + CheckNotImplementedStatus(t, resp) + + // Cannot give other roles read / write to system roles or manage roles because only system admin can do these actions + systemManager, err := th.App.Srv().Store.Role().GetByName("system_manager") + assert.Nil(t, err) + defer th.App.Srv().Store.Job().Delete(systemManager.Id) + + patchWriteSystemRoles := &model.RolePatch{ + Permissions: &[]string{model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES.Id}, + } + + _, resp = client.PatchRole(systemManager.Id, patchWriteSystemRoles) + CheckNotImplementedStatus(t, resp) + + patchReadSystemRoles := &model.RolePatch{ + Permissions: &[]string{model.PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_SYSTEM_ROLES.Id}, + } + + _, resp = client.PatchRole(systemManager.Id, patchReadSystemRoles) + CheckNotImplementedStatus(t, resp) + + patchManageRoles := &model.RolePatch{ + Permissions: &[]string{model.PERMISSION_MANAGE_ROLES.Id}, + } + + _, resp = client.PatchRole(systemManager.Id, patchManageRoles) + CheckNotImplementedStatus(t, resp) + }) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { received, resp := client.PatchRole(role.Id, patch) CheckNoError(t, resp) diff --git a/api4/user.go b/api4/user.go index 219c3a3b62..7c4eb190ff 100644 --- a/api4/user.go +++ b/api4/user.go @@ -585,8 +585,8 @@ func getFilteredUsersStats(c *Context, w http.ResponseWriter, r *http.Request) { TeamRoles: teamRoles, } - if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) { - c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) + if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_USERS) { + c.SetPermissionError(model.PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_USERS) return } diff --git a/app/permissions_migrations.go b/app/permissions_migrations.go index 1c12ae78be..a1b91b852e 100644 --- a/app/permissions_migrations.go +++ b/app/permissions_migrations.go @@ -502,6 +502,15 @@ func (a *App) getAddConvertChannelPermissionsMigration() (permissionsMap, error) }, nil } +func (a *App) getSystemRolesPermissionsMigration() (permissionsMap, error) { + return permissionsMap{ + permissionTransformation{ + On: isRole(model.SYSTEM_ADMIN_ROLE_ID), + Add: []string{model.PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_SYSTEM_ROLES.Id, model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES.Id}, + }, + }, nil +} + func (a *App) getAddManageSharedChannelsPermissionsMigration() (permissionsMap, error) { return permissionsMap{ permissionTransformation{ @@ -531,6 +540,7 @@ func (a *App) DoPermissionsMigrations() error { {Key: model.MIGRATION_KEY_ADD_SYSTEM_CONSOLE_PERMISSIONS, Migration: a.getAddSystemConsolePermissionsMigration}, {Key: model.MIGRATION_KEY_ADD_CONVERT_CHANNEL_PERMISSIONS, Migration: a.getAddConvertChannelPermissionsMigration}, {Key: model.MIGRATION_KEY_ADD_MANAGE_SHARED_CHANNEL_PERMISSIONS, Migration: a.getAddManageSharedChannelsPermissionsMigration}, + {Key: model.MIGRATION_KEY_ADD_SYSTEM_ROLES_PERMISSIONS, Migration: a.getSystemRolesPermissionsMigration}, } for _, migration := range PermissionsMigrations { diff --git a/i18n/en.json b/i18n/en.json index 3caf13f4e3..6f58e74dc7 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1954,10 +1954,18 @@ "id": "api.restricted_system_admin", "translation": "This action is forbidden to a restricted system admin." }, + { + "id": "api.roles.patch_roles.admin_role.error", + "translation": "System Admin role cannot be edited or changed" + }, { "id": "api.roles.patch_roles.license.error", "translation": "Your license does not support advanced permissions." }, + { + "id": "api.roles.patch_roles.not_allowed_permission.error", + "translation": "One or more of the following permissions that you are trying to add or remove is not allowed" + }, { "id": "api.scheme.create_scheme.license.error", "translation": "Your license does not support creating permissions schemes." diff --git a/model/migration.go b/model/migration.go index 25bc4a27be..f552d16872 100644 --- a/model/migration.go +++ b/model/migration.go @@ -21,5 +21,6 @@ const ( MIGRATION_KEY_ADD_SYSTEM_CONSOLE_PERMISSIONS = "add_system_console_permissions" MIGRATION_KEY_SIDEBAR_CATEGORIES_PHASE_2 = "migration_sidebar_categories_phase_2" MIGRATION_KEY_ADD_CONVERT_CHANNEL_PERMISSIONS = "add_convert_channel_permissions" + MIGRATION_KEY_ADD_SYSTEM_ROLES_PERMISSIONS = "add_system_roles_permissions" MIGRATION_KEY_ADD_MANAGE_SHARED_CHANNEL_PERMISSIONS = "manage_shared_channel_permissions" ) diff --git a/model/permission.go b/model/permission.go index 63e4a322af..d982962fd2 100644 --- a/model/permission.go +++ b/model/permission.go @@ -122,6 +122,9 @@ var PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_CHANNELS *Permission var PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_PERMISSIONS *Permission var PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_PERMISSIONS *Permission +var PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_SYSTEM_ROLES *Permission +var PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES *Permission + var PERMISSION_SYSCONSOLE_READ_ENVIRONMENT *Permission var PERMISSION_SYSCONSOLE_WRITE_ENVIRONMENT *Permission @@ -757,6 +760,18 @@ func initializePermissions() { "authentication.permissions.use_group_mentions.description", PermissionScopeSystem, } + PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_SYSTEM_ROLES = &Permission{ + "sysconsole_read_user_management_system_roles", + "authentication.permissions.use_group_mentions.name", + "authentication.permissions.use_group_mentions.description", + PermissionScopeSystem, + } + PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES = &Permission{ + "sysconsole_write_user_management_system_roles", + "authentication.permissions.use_group_mentions.name", + "authentication.permissions.use_group_mentions.description", + PermissionScopeSystem, + } PERMISSION_SYSCONSOLE_READ_ENVIRONMENT = &Permission{ "sysconsole_read_environment", "authentication.permissions.use_group_mentions.name", @@ -862,6 +877,7 @@ func initializePermissions() { PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_TEAMS, PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_CHANNELS, PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_PERMISSIONS, + PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_SYSTEM_ROLES, PERMISSION_SYSCONSOLE_READ_ENVIRONMENT, PERMISSION_SYSCONSOLE_READ_SITE, PERMISSION_SYSCONSOLE_READ_AUTHENTICATION, @@ -879,6 +895,7 @@ func initializePermissions() { PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_TEAMS, PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_CHANNELS, PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_PERMISSIONS, + PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_SYSTEM_ROLES, PERMISSION_SYSCONSOLE_WRITE_ENVIRONMENT, PERMISSION_SYSCONSOLE_WRITE_SITE, PERMISSION_SYSCONSOLE_WRITE_AUTHENTICATION, diff --git a/model/role.go b/model/role.go index fe3bdb0a3c..271e295be6 100644 --- a/model/role.go +++ b/model/role.go @@ -133,6 +133,7 @@ func init() { PERMISSION_SYSCONSOLE_READ_SITE.Id, PERMISSION_SYSCONSOLE_READ_AUTHENTICATION.Id, PERMISSION_SYSCONSOLE_READ_PLUGINS.Id, + PERMISSION_SYSCONSOLE_READ_COMPLIANCE.Id, PERMISSION_SYSCONSOLE_READ_INTEGRATIONS.Id, PERMISSION_SYSCONSOLE_READ_EXPERIMENTAL.Id, } diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 1297ed7357..512baae8bc 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -412,6 +412,7 @@ func (us SqlUserStore) GetAllProfiles(options *model.UserGetOptions) ([]*model.U query = applyViewRestrictionsFilter(query, options.ViewRestrictions, true) query = applyRoleFilter(query, options.Role, isPostgreSQL) + query = applyMultiRoleFilters(query, options.Roles, []string{}, []string{}, isPostgreSQL) if options.Inactive { query = query.Where("u.DeleteAt != 0") @@ -451,119 +452,75 @@ func applyRoleFilter(query sq.SelectBuilder, role string, isPostgreSQL bool) sq. return query.Where("u.Roles LIKE ? ESCAPE '*'", roleParam) } -func applyMultiRoleFilters(query sq.SelectBuilder, roles []string, teamRoles []string, channelRoles []string) sq.SelectBuilder { - queryString := "" - if len(roles) > 0 && roles[0] != "" { - schemeGuest := false - schemeAdmin := false - schemeUser := false +func applyMultiRoleFilters(query sq.SelectBuilder, systemRoles []string, teamRoles []string, channelRoles []string, isPostgreSQL bool) sq.SelectBuilder { + sqOr := sq.Or{} - for _, role := range roles { + if len(systemRoles) > 0 && systemRoles[0] != "" { + for _, role := range systemRoles { + queryRole := wildcardSearchTerm(role) switch role { - case model.SYSTEM_ADMIN_ROLE_ID: - schemeAdmin = true case model.SYSTEM_USER_ROLE_ID: - schemeUser = true - case model.SYSTEM_GUEST_ROLE_ID: - schemeGuest = true - } - } - - if schemeAdmin || schemeUser || schemeGuest { - if schemeAdmin && schemeUser { - queryString += `(u.Roles LIKE '%system_user%' OR u.Roles LIKE '%system_admin%') ` - } else if schemeAdmin { - queryString += `(u.Roles LIKE '%system_admin%') ` - } else if schemeUser { - queryString += `(u.Roles LIKE '%system_user%' AND u.Roles NOT LIKE '%system_admin%') ` - } - - if schemeGuest { - if queryString != "" { - queryString += "OR " + // If querying for a `system_user` ensure that the user is only a system_user. + sqOr = append(sqOr, sq.Eq{"u.Roles": role}) + case model.SYSTEM_GUEST_ROLE_ID, model.SYSTEM_ADMIN_ROLE_ID, model.SYSTEM_USER_MANAGER_ROLE_ID, model.SYSTEM_READ_ONLY_ADMIN_ROLE_ID, model.SYSTEM_MANAGER_ROLE_ID: + // If querying for any other roles search using a wildcard. + if isPostgreSQL { + sqOr = append(sqOr, sq.ILike{"u.Roles": queryRole}) + } else { + sqOr = append(sqOr, sq.Like{"u.Roles": queryRole}) } - queryString += `(u.Roles LIKE '%system_guest%') ` } + } } if len(channelRoles) > 0 && channelRoles[0] != "" { - schemeGuest := false - schemeAdmin := false - schemeUser := false for _, channelRole := range channelRoles { switch channelRole { case model.CHANNEL_ADMIN_ROLE_ID: - schemeAdmin = true - case model.CHANNEL_USER_ROLE_ID: - schemeUser = true - case model.CHANNEL_GUEST_ROLE_ID: - schemeGuest = true - } - } - - if schemeAdmin || schemeUser || schemeGuest { - if queryString != "" { - queryString += "OR " - } - if schemeAdmin && schemeUser { - queryString += `(cm.SchemeUser = true AND u.Roles = 'system_user')` - } else if schemeAdmin { - queryString += `(cm.SchemeAdmin = true AND u.Roles = 'system_user')` - } else if schemeUser { - queryString += `(cm.SchemeUser = true AND cm.SchemeAdmin = false AND u.Roles = 'system_user')` - } - - if schemeGuest { - if queryString != "" && queryString[len(queryString)-3:] != "OR " { - queryString += "OR " + if isPostgreSQL { + sqOr = append(sqOr, sq.And{sq.Eq{"cm.SchemeAdmin": true}, sq.NotILike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) + } else { + sqOr = append(sqOr, sq.And{sq.Eq{"cm.SchemeAdmin": true}, sq.NotLike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) } - queryString += `(cm.SchemeGuest = true AND u.Roles = 'system_guest')` + case model.CHANNEL_USER_ROLE_ID: + if isPostgreSQL { + sqOr = append(sqOr, sq.And{sq.Eq{"cm.SchemeUser": true}, sq.Eq{"cm.SchemeAdmin": false}, sq.NotILike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) + } else { + sqOr = append(sqOr, sq.And{sq.Eq{"cm.SchemeUser": true}, sq.Eq{"cm.SchemeAdmin": false}, sq.NotLike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) + } + case model.CHANNEL_GUEST_ROLE_ID: + sqOr = append(sqOr, sq.Eq{"cm.SchemeGuest": true}) } } } if len(teamRoles) > 0 && teamRoles[0] != "" { - schemeAdmin := false - schemeUser := false - schemeGuest := false for _, teamRole := range teamRoles { switch teamRole { case model.TEAM_ADMIN_ROLE_ID: - schemeAdmin = true - case model.TEAM_USER_ROLE_ID: - schemeUser = true - case model.TEAM_GUEST_ROLE_ID: - schemeGuest = true - } - } - - if schemeAdmin || schemeUser || schemeGuest { - if queryString != "" { - queryString += "OR " - } - if schemeAdmin && schemeUser { - queryString += `(tm.SchemeUser = true AND u.Roles = 'system_user')` - } else if schemeAdmin { - queryString += `(tm.SchemeAdmin = true AND u.Roles = 'system_user')` - } else if schemeUser { - queryString += `(tm.SchemeUser = true AND tm.SchemeAdmin = false AND u.Roles = 'system_user')` - } - - if schemeGuest { - if queryString != "" && queryString[len(queryString)-3:] != "OR " { - queryString += "OR " + if isPostgreSQL { + sqOr = append(sqOr, sq.And{sq.Eq{"tm.SchemeAdmin": true}, sq.NotILike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) + } else { + sqOr = append(sqOr, sq.And{sq.Eq{"tm.SchemeAdmin": true}, sq.NotLike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) } - queryString += `(tm.SchemeGuest = true AND u.Roles = 'system_guest')` + case model.TEAM_USER_ROLE_ID: + if isPostgreSQL { + sqOr = append(sqOr, sq.And{sq.Eq{"tm.SchemeUser": true}, sq.Eq{"tm.SchemeAdmin": false}, sq.NotILike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) + } else { + sqOr = append(sqOr, sq.And{sq.Eq{"tm.SchemeUser": true}, sq.Eq{"tm.SchemeAdmin": false}, sq.NotLike{"u.Roles": wildcardSearchTerm(model.SYSTEM_ADMIN_ROLE_ID)}}) + } + case model.TEAM_GUEST_ROLE_ID: + sqOr = append(sqOr, sq.Eq{"tm.SchemeGuest": true}) } } } - if queryString != "" { - query = query.Where("(" + queryString + ")") + if len(sqOr) > 0 { + return query.Where(sqOr) + } else { + return query } - - return query } func applyChannelGroupConstrainedFilter(query sq.SelectBuilder, channelId string) sq.SelectBuilder { @@ -633,7 +590,7 @@ func (us SqlUserStore) GetProfiles(options *model.UserGetOptions) ([]*model.User query = applyViewRestrictionsFilter(query, options.ViewRestrictions, true) query = applyRoleFilter(query, options.Role, isPostgreSQL) - query = applyMultiRoleFilters(query, options.Roles, options.TeamRoles, options.ChannelRoles) + query = applyMultiRoleFilters(query, options.Roles, options.TeamRoles, options.ChannelRoles, isPostgreSQL) if options.Inactive { query = query.Where("u.DeleteAt != 0") @@ -1228,7 +1185,7 @@ func (us SqlUserStore) Count(options model.UserCountOptions) (int64, error) { query = query.LeftJoin("ChannelMembers AS cm ON u.Id = cm.UserId").Where("cm.ChannelId = ?", options.ChannelId) } query = applyViewRestrictionsFilter(query, options.ViewRestrictions, false) - query = applyMultiRoleFilters(query, options.Roles, options.TeamRoles, options.ChannelRoles) + query = applyMultiRoleFilters(query, options.Roles, options.TeamRoles, options.ChannelRoles, isPostgreSQL) if isPostgreSQL { query = query.PlaceholderFormat(sq.Dollar) @@ -1461,7 +1418,7 @@ func (us SqlUserStore) performSearch(query sq.SelectBuilder, term string, option isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES query = applyRoleFilter(query, options.Role, isPostgreSQL) - query = applyMultiRoleFilters(query, options.Roles, options.TeamRoles, options.ChannelRoles) + query = applyMultiRoleFilters(query, options.Roles, options.TeamRoles, options.ChannelRoles, isPostgreSQL) if !options.AllowInactive { query = query.Where("u.DeleteAt = 0") diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index fd8c2ec152..35837418f7 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -368,6 +368,7 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { u1, err := ss.User().Save(&model.User{ Email: MakeEmail(), Username: "u1" + model.NewId(), + Roles: model.SYSTEM_USER_ROLE_ID, }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u1.Id)) }() @@ -375,6 +376,7 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { u2, err := ss.User().Save(&model.User{ Email: MakeEmail(), Username: "u2" + model.NewId(), + Roles: model.SYSTEM_USER_ROLE_ID, }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u2.Id)) }() @@ -423,14 +425,15 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { Email: MakeEmail(), Username: "u7" + model.NewId(), DeleteAt: model.GetMillis(), + Roles: model.SYSTEM_USER_ROLE_ID, }) require.Nil(t, err) defer func() { require.Nil(t, ss.User().PermanentDelete(u7.Id)) }() t.Run("get offset 0, limit 100", func(t *testing.T) { options := &model.UserGetOptions{Page: 0, PerPage: 100} - actual, err := ss.User().GetAllProfiles(options) - require.Nil(t, err) + actual, userErr := ss.User().GetAllProfiles(options) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u1), @@ -444,19 +447,19 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("get offset 0, limit 1", func(t *testing.T) { - actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 1, }) - require.Nil(t, err) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u1), }, actual) }) t.Run("get all", func(t *testing.T) { - actual, err := ss.User().GetAll() - require.Nil(t, err) + actual, userErr := ss.User().GetAll() + require.Nil(t, userErr) require.Equal(t, []*model.User{ u1, @@ -474,8 +477,8 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { uNew := &model.User{} uNew.Email = MakeEmail() - _, err := ss.User().Save(uNew) - require.Nil(t, err) + _, userErr := ss.User().Save(uNew) + require.Nil(t, userErr) defer func() { require.Nil(t, ss.User().PermanentDelete(uNew.Id)) }() updatedEtag := ss.User().GetEtagForAllProfiles() @@ -483,12 +486,12 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("filter to system_admin role", func(t *testing.T) { - actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Role: "system_admin", }) - require.Nil(t, err) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u5), sanitized(u6), @@ -496,25 +499,25 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("filter to system_admin role, inactive", func(t *testing.T) { - actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Role: "system_admin", Inactive: true, }) - require.Nil(t, err) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u6), }, actual) }) t.Run("filter to inactive", func(t *testing.T) { - actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Inactive: true, }) - require.Nil(t, err) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u6), sanitized(u7), @@ -522,12 +525,12 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("filter to active", func(t *testing.T) { - actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Active: true, }) - require.Nil(t, err) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u1), sanitized(u2), @@ -538,18 +541,87 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { }) t.Run("try to filter to active and inactive", func(t *testing.T) { - actual, err := ss.User().GetAllProfiles(&model.UserGetOptions{ + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ Page: 0, PerPage: 10, Inactive: true, Active: true, }) - require.Nil(t, err) + require.Nil(t, userErr) require.Equal(t, []*model.User{ sanitized(u6), sanitized(u7), }, actual) }) + + u8, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "u8" + model.NewId(), + DeleteAt: model.GetMillis(), + Roles: "system_user_manager system_user", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(u8.Id)) }() + + u9, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "u9" + model.NewId(), + DeleteAt: model.GetMillis(), + Roles: "system_manager system_user", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(u9.Id)) }() + + u10, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "u10" + model.NewId(), + DeleteAt: model.GetMillis(), + Roles: "system_read_only_admin system_user", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(u10.Id)) }() + + t.Run("filter by system_user_manager role", func(t *testing.T) { + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ + Page: 0, + PerPage: 10, + Roles: []string{"system_user_manager"}, + }) + require.Nil(t, userErr) + require.Equal(t, []*model.User{ + sanitized(u8), + }, actual) + }) + + t.Run("filter by multiple system roles", func(t *testing.T) { + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ + Page: 0, + PerPage: 10, + Roles: []string{"system_manager", "system_user_manager", "system_read_only_admin", "system_admin"}, + }) + require.Nil(t, userErr) + require.Equal(t, []*model.User{ + sanitized(u10), + sanitized(u5), + sanitized(u6), + sanitized(u8), + sanitized(u9), + }, actual) + }) + + t.Run("filter by system_user only", func(t *testing.T) { + actual, userErr := ss.User().GetAllProfiles(&model.UserGetOptions{ + Page: 0, + PerPage: 10, + Roles: []string{"system_user"}, + }) + require.Nil(t, userErr) + require.Equal(t, []*model.User{ + sanitized(u1), + sanitized(u2), + sanitized(u7), + }, actual) + }) } func testUserStoreGetProfiles(t *testing.T, ss store.Store) { @@ -2437,7 +2509,7 @@ func testUserStoreSearch(t *testing.T, ss store.Store) { u2 := &model.User{ Username: "jim2-bobby" + model.NewId(), Email: MakeEmail(), - Roles: "system_user", + Roles: "system_user system_user_manager", } _, err = ss.User().Save(u2) require.Nil(t, err) diff --git a/testlib/store.go b/testlib/store.go index 53b3669e7f..9f622d93d9 100644 --- a/testlib/store.go +++ b/testlib/store.go @@ -46,6 +46,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", model.MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION).Return(&model.System{Name: model.MIGRATION_KEY_ADD_USE_GROUP_MENTIONS_PERMISSION, Value: "true"}, nil) systemStore.On("GetByName", model.MIGRATION_KEY_ADD_SYSTEM_CONSOLE_PERMISSIONS).Return(&model.System{Name: model.MIGRATION_KEY_ADD_SYSTEM_CONSOLE_PERMISSIONS, Value: "true"}, nil) systemStore.On("GetByName", model.MIGRATION_KEY_ADD_CONVERT_CHANNEL_PERMISSIONS).Return(&model.System{Name: model.MIGRATION_KEY_ADD_CONVERT_CHANNEL_PERMISSIONS, Value: "true"}, nil) + systemStore.On("GetByName", model.MIGRATION_KEY_ADD_SYSTEM_ROLES_PERMISSIONS).Return(&model.System{Name: model.MIGRATION_KEY_ADD_SYSTEM_ROLES_PERMISSIONS, Value: "true"}, nil) systemStore.On("GetByName", model.MIGRATION_KEY_ADD_MANAGE_SHARED_CHANNEL_PERMISSIONS).Return(&model.System{Name: model.MIGRATION_KEY_ADD_MANAGE_SHARED_CHANNEL_PERMISSIONS, Value: "true"}, nil) systemStore.On("Get").Return(make(model.StringMap), nil) systemStore.On("Save", mock.AnythingOfType("*model.System")).Return(nil)