Config: Remove setting editors_can_admin (#101607)

* Remove editors can admin

* delete file

* add ac_test back

* Add linting ignore

* more static ignore
This commit is contained in:
Eric Leijonmarck
2025-03-12 16:25:23 +00:00
committed by GitHub
parent 21b9d45ca6
commit 94020aa9cd
13 changed files with 41 additions and 411 deletions
@@ -195,7 +195,6 @@ export interface GrafanaConfig {
passwordHint: string;
loginError?: string;
viewersCanEdit: boolean;
editorsCanAdmin: boolean;
disableSanitizeHtml: boolean;
trustedTypesDefaultPolicyEnabled: boolean;
cspReportOnlyEnabled: boolean;
-2
View File
@@ -103,7 +103,6 @@ export class GrafanaBootConfig implements GrafanaConfig {
passwordHint = '';
loginError: string | undefined = undefined;
viewersCanEdit = false;
editorsCanAdmin = false;
disableSanitizeHtml = false;
trustedTypesDefaultPolicyEnabled = false;
cspReportOnlyEnabled = false;
@@ -229,7 +228,6 @@ export class GrafanaBootConfig implements GrafanaConfig {
env: 'production',
},
viewersCanEdit: false,
editorsCanAdmin: false,
disableSanitizeHtml: false,
};
+2 -3
View File
@@ -71,6 +71,7 @@ func (hs *HTTPServer) declareFixedRoles() error {
Grants: []string{string(org.RoleEditor)},
}
//nolint:staticcheck // ViewersCanEdit is deprecated but still used for backward compatibility
if hs.Cfg.ViewersCanEdit {
datasourcesExplorerRole.Grants = append(datasourcesExplorerRole.Grants, string(org.RoleViewer))
}
@@ -256,9 +257,7 @@ func (hs *HTTPServer) declareFixedRoles() error {
}
teamCreatorGrants := []string{string(org.RoleAdmin)}
if hs.Cfg.EditorsCanAdmin {
teamCreatorGrants = append(teamCreatorGrants, string(org.RoleEditor))
}
teamsCreatorRole := ac.RoleRegistration{
Role: ac.RoleDTO{
Name: "fixed:teams:creator",
-1
View File
@@ -207,7 +207,6 @@ type FrontendSettingsDTO struct {
ExternalUserMngAnalyticsParams string `json:"externalUserMngAnalyticsParams"`
ViewersCanEdit bool `json:"viewersCanEdit"`
AngularSupportEnabled bool `json:"angularSupportEnabled"`
EditorsCanAdmin bool `json:"editorsCanAdmin"`
DisableSanitizeHtml bool `json:"disableSanitizeHtml"`
TrustedTypesDefaultPolicyEnabled bool `json:"trustedTypesDefaultPolicyEnabled"`
CSPReportOnlyEnabled bool `json:"cspReportOnlyEnabled"`
+20 -20
View File
@@ -228,26 +228,26 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro
ExternalUserMngLinkName: hs.Cfg.ExternalUserMngLinkName,
ExternalUserMngAnalytics: hs.Cfg.ExternalUserMngAnalytics,
ExternalUserMngAnalyticsParams: hs.Cfg.ExternalUserMngAnalyticsParams,
ViewersCanEdit: hs.Cfg.ViewersCanEdit,
AngularSupportEnabled: hs.Cfg.AngularSupportEnabled,
EditorsCanAdmin: hs.Cfg.EditorsCanAdmin,
DisableSanitizeHtml: hs.Cfg.DisableSanitizeHtml,
TrustedTypesDefaultPolicyEnabled: trustedTypesDefaultPolicyEnabled,
CSPReportOnlyEnabled: hs.Cfg.CSPReportOnlyEnabled,
DateFormats: hs.Cfg.DateFormats,
SecureSocksDSProxyEnabled: hs.Cfg.SecureSocksDSProxy.Enabled && hs.Cfg.SecureSocksDSProxy.ShowUI,
EnableFrontendSandboxForPlugins: hs.Cfg.EnableFrontendSandboxForPlugins,
PublicDashboardAccessToken: c.PublicDashboardAccessToken,
PublicDashboardsEnabled: hs.Cfg.PublicDashboardsEnabled,
CloudMigrationIsTarget: isCloudMigrationTarget,
CloudMigrationFeedbackURL: hs.Cfg.CloudMigration.FeedbackURL,
CloudMigrationPollIntervalMs: int(hs.Cfg.CloudMigration.FrontendPollInterval.Milliseconds()),
SharedWithMeFolderUID: folder.SharedWithMeFolderUID,
RootFolderUID: accesscontrol.GeneralFolderUID,
LocalFileSystemAvailable: hs.Cfg.LocalFileSystemAvailable,
ReportingStaticContext: hs.Cfg.ReportingStaticContext,
ExploreDefaultTimeOffset: hs.Cfg.ExploreDefaultTimeOffset,
ExploreHideLogsDownload: hs.Cfg.ExploreHideLogsDownload,
//nolint:staticcheck // ViewersCanEdit is deprecated but still used for backward compatibility
ViewersCanEdit: hs.Cfg.ViewersCanEdit,
AngularSupportEnabled: hs.Cfg.AngularSupportEnabled,
DisableSanitizeHtml: hs.Cfg.DisableSanitizeHtml,
TrustedTypesDefaultPolicyEnabled: trustedTypesDefaultPolicyEnabled,
CSPReportOnlyEnabled: hs.Cfg.CSPReportOnlyEnabled,
DateFormats: hs.Cfg.DateFormats,
SecureSocksDSProxyEnabled: hs.Cfg.SecureSocksDSProxy.Enabled && hs.Cfg.SecureSocksDSProxy.ShowUI,
EnableFrontendSandboxForPlugins: hs.Cfg.EnableFrontendSandboxForPlugins,
PublicDashboardAccessToken: c.PublicDashboardAccessToken,
PublicDashboardsEnabled: hs.Cfg.PublicDashboardsEnabled,
CloudMigrationIsTarget: isCloudMigrationTarget,
CloudMigrationFeedbackURL: hs.Cfg.CloudMigration.FeedbackURL,
CloudMigrationPollIntervalMs: int(hs.Cfg.CloudMigration.FrontendPollInterval.Milliseconds()),
SharedWithMeFolderUID: folder.SharedWithMeFolderUID,
RootFolderUID: accesscontrol.GeneralFolderUID,
LocalFileSystemAvailable: hs.Cfg.LocalFileSystemAvailable,
ReportingStaticContext: hs.Cfg.ReportingStaticContext,
ExploreDefaultTimeOffset: hs.Cfg.ExploreDefaultTimeOffset,
ExploreHideLogsDownload: hs.Cfg.ExploreHideLogsDownload,
DefaultDatasourceManageAlertsUIToggle: hs.Cfg.DefaultDatasourceManageAlertsUIToggle,
+1 -5
View File
@@ -31,15 +31,11 @@ func (s *Service) getUsageStats(ctx context.Context) (map[string]any, error) {
// FIXME: Move this to accesscontrol OSS.
// FIXME: Access Control OSS usage stats is currently disabled if Enterprise is enabled.
m["stats.authz.viewers_can_edit.count"] = 0
//nolint:staticcheck // ViewersCanEdit is deprecated but still used for backward compatibility
if s.cfg.ViewersCanEdit {
m["stats.authz.viewers_can_edit.count"] = 1
}
m["stats.authz.editors_can_admin.count"] = 0
if s.cfg.EditorsCanAdmin {
m["stats.authz.editors_can_admin.count"] = 1
}
for _, client := range s.clients {
if usac, ok := client.(authn.UsageStatClient); ok {
clientStats, err := usac.UsageStatFn(ctx)
@@ -23,7 +23,7 @@ func TestService_getUsageStats(t *testing.T) {
svc.cfg.AuthProxy.Enabled = true
svc.cfg.JWTAuth.Enabled = true
svc.cfg.LDAPAuthEnabled = true
svc.cfg.EditorsCanAdmin = true
//nolint:staticcheck
svc.cfg.ViewersCanEdit = true
got, err := svc.getUsageStats(context.Background())
@@ -35,7 +35,6 @@ func TestService_getUsageStats(t *testing.T) {
"stats.auth_enabled.jwt.count": 1,
"stats.auth_enabled.ldap.count": 1,
"stats.auth_enabled.login_form.count": 1,
"stats.authz.editors_can_admin.count": 1,
"stats.authz.viewers_can_edit.count": 1,
"stats.test.enabled.count": 1,
}
@@ -221,6 +221,7 @@ func (a *accessControlDashboardGuardian) CanEdit() (bool, error) {
return false, ErrGuardianDashboardNotFound.Errorf("failed to check edit permissions for dashboard")
}
//nolint:staticcheck // ViewersCanEdit is deprecated but still used for backward compatibility
if a.cfg.ViewersCanEdit {
return a.CanView()
}
@@ -235,6 +236,7 @@ func (a *accessControlFolderGuardian) CanEdit() (bool, error) {
return false, ErrGuardianFolderNotFound.Errorf("failed to check edit permissions for folder")
}
//nolint:staticcheck // ViewersCanEdit is deprecated but still used for backward compatibility
if a.cfg.ViewersCanEdit {
return a.CanView()
}
@@ -341,6 +341,7 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
cfg := setting.NewCfg()
//nolint:staticcheck
cfg.ViewersCanEdit = tt.viewersCanEdit
guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, cfg)
+2
View File
@@ -30,6 +30,8 @@ type CallbackHandler func(c *contextmodel.ReqContext) response.Response
func (s *QueryHistoryService) permissionsMiddleware(handler CallbackHandler, errorMessage string) CallbackHandler {
return func(c *contextmodel.ReqContext) response.Response {
hasAccess := ac.HasAccess(s.accessControl, c)
// ViewersCanEdit is deprecated but still used for backward compatibility
//nolint:staticcheck
if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit && !hasAccess(ac.EvalPermission(ac.ActionDatasourcesExplore)) {
return response.Error(http.StatusUnauthorized, errorMessage, nil)
}
@@ -18,14 +18,13 @@ const (
)
func AddTeamMembershipMigrations(mg *migrator.Migrator) {
mg.AddMigration(TeamsMigrationID, &teamPermissionMigrator{editorsCanAdmin: mg.Cfg.EditorsCanAdmin})
mg.AddMigration(TeamsMigrationID, &teamPermissionMigrator{})
}
var _ migrator.CodeMigration = new(teamPermissionMigrator)
type teamPermissionMigrator struct {
permissionMigrator
editorsCanAdmin bool
}
func (p *teamPermissionMigrator) SQL(dialect migrator.Dialect) string {
@@ -210,7 +209,7 @@ func (p *teamPermissionMigrator) generateAssociatedPermissions(teamMemberships [
// only admins or editors (when editorsCanAdmin option is enabled)
// can access team administration endpoints
if m.Permission == team.PermissionTypeAdmin {
if userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleViewer) || (userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleEditor) && !p.editorsCanAdmin) {
if userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleViewer) || (userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleEditor)) {
m.Permission = 0
if _, err := p.sess.Cols("permission").Where("org_id=? and team_id=? and user_id=?", m.OrgID, m.TeamID, m.UserID).Update(m); err != nil {
@@ -5,22 +5,15 @@ import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
"gopkg.in/ini.v1"
"xorm.io/xorm"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
acmig "github.com/grafana/grafana/pkg/services/sqlstore/migrations/accesscontrol"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
type rawPermission struct {
@@ -40,59 +33,6 @@ func (rp *rawPermission) toPermission(roleID int64, ts time.Time) accesscontrol.
// Setup users
var (
now = time.Now()
users = []user.User{
{
ID: 1,
UID: "u1",
Email: "viewer1@example.org",
Name: "viewer1",
Login: "viewer1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 2,
UID: "u2",
Email: "viewer2@example.org",
Name: "viewer2",
Login: "viewer2",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 3,
UID: "u3",
Email: "editor1@example.org",
Name: "editor1",
Login: "editor1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 4,
UID: "u4",
Email: "admin1@example.org",
Name: "admin1",
Login: "admin1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 5,
UID: "u5",
Email: "editor2@example.org",
Name: "editor2",
Login: "editor2",
OrgID: 2,
Created: now,
Updated: now,
},
}
)
func convertToRawPermissions(permissions []accesscontrol.Permission) []rawPermission {
@@ -103,121 +43,6 @@ func convertToRawPermissions(permissions []accesscontrol.Permission) []rawPermis
return raw
}
func TestMigrations(t *testing.T) {
// Run initial migration to have a working DB
x := setupTestDB(t)
// Populate users and teams
setupTeams(t, x)
// Create managed user roles with teams permissions (ex: teams:read and teams.permissions:read)
setupUnecessaryRBACPermissions(t, x)
team1Scope := accesscontrol.Scope("teams", "id", "1")
team2Scope := accesscontrol.Scope("teams", "id", "2")
type teamMigrationTestCase struct {
desc string
config *setting.Cfg
expectedRolePerms map[string][]rawPermission
}
testCases := []teamMigrationTestCase{
{
desc: "with editors can admin",
config: &setting.Cfg{
EditorsCanAdmin: true,
Raw: ini.Empty(),
},
expectedRolePerms: map[string][]rawPermission{
"managed:users:1:permissions": {{Action: "teams:read", Scope: team1Scope}},
"managed:users:2:permissions": {{Action: "teams:read", Scope: team1Scope}},
"managed:users:3:permissions": {
{Action: "teams:read", Scope: team1Scope},
{Action: "teams:delete", Scope: team1Scope},
{Action: "teams:write", Scope: team1Scope},
{Action: "teams.permissions:read", Scope: team1Scope},
{Action: "teams.permissions:write", Scope: team1Scope},
},
"managed:users:4:permissions": {
{Action: "teams:read", Scope: team1Scope},
{Action: "teams:delete", Scope: team1Scope},
{Action: "teams:write", Scope: team1Scope},
{Action: "teams.permissions:read", Scope: team1Scope},
{Action: "teams.permissions:write", Scope: team1Scope},
},
"managed:users:5:permissions": {
{Action: "teams:read", Scope: team2Scope},
{Action: "users:read", Scope: "users:*"},
},
},
},
{
desc: "without editors can admin",
// nolint:staticcheck
config: setting.NewCfgWithFeatures(featuremgmt.WithFeatures("accesscontrol").IsEnabledGlobally),
expectedRolePerms: map[string][]rawPermission{
"managed:users:1:permissions": {{Action: "teams:read", Scope: team1Scope}},
"managed:users:2:permissions": {{Action: "teams:read", Scope: team1Scope}},
"managed:users:3:permissions": {{Action: "teams:read", Scope: team1Scope}},
"managed:users:4:permissions": {
{Action: "teams:read", Scope: team1Scope},
{Action: "teams:delete", Scope: team1Scope},
{Action: "teams:write", Scope: team1Scope},
{Action: "teams.permissions:read", Scope: team1Scope},
{Action: "teams.permissions:write", Scope: team1Scope},
},
"managed:users:5:permissions": {
{Action: "teams:read", Scope: team2Scope},
{Action: "users:read", Scope: "users:*"},
},
},
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
// Remove migration
_, errDeleteMig := x.Exec("DELETE FROM migration_log WHERE migration_id = ?", acmig.TeamsMigrationID)
require.NoError(t, errDeleteMig)
// Run accesscontrol migration (permissions insertion should not have conflicted)
acmigrator := migrator.NewMigrator(x, tc.config)
acmig.AddTeamMembershipMigrations(acmigrator)
errRunningMig := acmigrator.Start(false, 0)
require.NoError(t, errRunningMig)
for _, user := range users {
// Check managed roles exist
roleName := fmt.Sprintf("managed:users:%d:permissions", user.ID)
role := accesscontrol.Role{}
hasRole, errManagedRoleSearch := x.Table("role").Where("org_id = ? AND name = ?", user.OrgID, roleName).Get(&role)
require.NoError(t, errManagedRoleSearch)
assert.True(t, hasRole, "expected role to be granted to user", user, roleName)
// Check permissions associated with each role
perms := []accesscontrol.Permission{}
countUserPermissions, errManagedPermsSearch := x.Table("permission").Where("role_id = ?", role.ID).FindAndCount(&perms)
require.NoError(t, errManagedPermsSearch)
assert.Equal(t, int64(len(tc.expectedRolePerms[roleName])), countUserPermissions, "expected role to be tied to permissions", user, role)
rawPerms := convertToRawPermissions(perms)
for _, perm := range rawPerms {
assert.Contains(t, tc.expectedRolePerms[roleName], perm)
}
// Check assignment of the roles
assign := accesscontrol.UserRole{}
has, errAssignmentSearch := x.Table("user_role").Where("role_id = ? AND user_id = ?", role.ID, user.ID).Get(&assign)
require.NoError(t, errAssignmentSearch)
assert.True(t, has, "expected assignment of role to user", role, user)
}
})
}
}
func setupTestDB(t *testing.T) *xorm.Engine {
t.Helper()
dbType := sqlutil.GetTestDBType()
@@ -250,188 +75,3 @@ func setupTestDB(t *testing.T) *xorm.Engine {
return x
}
func setupTeams(t *testing.T, x *xorm.Engine) {
t.Helper()
usersCount, errInsertUsers := x.Insert(users)
require.NoError(t, errInsertUsers)
require.Equal(t, int64(5), usersCount, "needed 5 users for this test to run")
orgUsers := []org.OrgUser{
{
OrgID: 1,
UserID: 1,
Role: org.RoleViewer,
Created: now,
Updated: now,
},
{
OrgID: 1,
UserID: 2,
Role: org.RoleViewer,
Created: now,
Updated: now,
},
{
OrgID: 1,
UserID: 3,
Role: org.RoleEditor,
Created: now,
Updated: now,
},
{
OrgID: 1,
UserID: 4,
Role: org.RoleAdmin,
Created: now,
Updated: now,
},
{
OrgID: 2,
UserID: 5,
Role: org.RoleEditor,
Created: now,
Updated: now,
},
}
orgUsersCount, errInsertOrgUsers := x.Insert(orgUsers)
require.NoError(t, errInsertOrgUsers)
require.Equal(t, int64(5), orgUsersCount, "needed 5 users for this test to run")
// Setup teams (and members)
teams := []team.Team{
{
OrgID: 1,
Name: "teamOrg1",
Email: "teamorg1@example.org",
Created: now,
Updated: now,
},
{
OrgID: 2,
Name: "teamOrg2",
Email: "teamorg2@example.org",
Created: now,
Updated: now,
},
}
teamCount, errInsertTeams := x.Insert(teams)
require.NoError(t, errInsertTeams)
require.Equal(t, int64(2), teamCount, "needed 2 teams for this test to run")
members := []team.TeamMember{
{
// Can have viewer permissions
OrgID: 1,
TeamID: 1,
UserID: 1,
External: false,
Permission: team.PermissionTypeMember,
Created: now,
Updated: now,
},
{
// Cannot have admin permissions
OrgID: 1,
TeamID: 1,
UserID: 2,
External: false,
Permission: team.PermissionTypeAdmin,
Created: now,
Updated: now,
},
{
// Can have admin permissions
OrgID: 1,
TeamID: 1,
UserID: 3,
External: false,
Permission: team.PermissionTypeAdmin,
Created: now,
Updated: now,
},
{
// Can have admin permissions
OrgID: 1,
TeamID: 1,
UserID: 4,
External: false,
Permission: team.PermissionTypeAdmin,
Created: now,
Updated: now,
},
{
// Can have viewer permissions
OrgID: 2,
TeamID: 2,
UserID: 5,
External: false,
Permission: team.PermissionTypeMember,
Created: now,
Updated: now,
},
}
membersCount, err := x.Insert(members)
require.NoError(t, err)
require.Equal(t, int64(5), membersCount, "needed 5 members for this test to run")
}
func setupUnecessaryRBACPermissions(t *testing.T, x *xorm.Engine) {
t.Helper()
now := time.Now()
role := accesscontrol.Role{
// ID: 1, Not specifying this for pgsql to correctly increment sequence
OrgID: 2,
Version: 1,
UID: "user5managedpermissions",
Name: "managed:users:5:permissions",
Updated: now,
Created: now,
}
rolesCount, err := x.Insert(role)
require.NoError(t, err)
require.Equal(t, int64(1), rolesCount, "needed 1 role for this test to run")
userRole := accesscontrol.UserRole{
OrgID: 2,
RoleID: 1,
UserID: 5,
Created: now,
}
userRoleCount, err := x.Insert(userRole)
require.NoError(t, err)
require.Equal(t, int64(1), userRoleCount, "needed 1 assignment for this test to run")
permissions := []accesscontrol.Permission{
{
// Permission that shouldn't be removed
RoleID: 1,
Action: "users:read",
Scope: "users:*",
Updated: now,
Created: now,
},
{
// Permission that should be recreated
RoleID: 1,
Action: "teams:read",
Scope: "teams:*",
Updated: now,
Created: now,
},
{
// Permission that should be removed
RoleID: 1,
Action: "teams.permissions:read",
Scope: "teams:*",
Updated: now,
Created: now,
},
}
permissionsCount, err := x.Insert(permissions)
require.NoError(t, err)
require.Equal(t, int64(3), permissionsCount, "needed 3 permissions for this test to run")
}
+4 -8
View File
@@ -294,8 +294,8 @@ type Cfg struct {
// DistributedCache
RemoteCacheOptions *RemoteCacheSettings
ViewersCanEdit bool // Deprecated: no longer used
EditorsCanAdmin bool // Deprecated: no longer used
// Deprecated: no longer used
ViewersCanEdit bool
ApiKeyMaxSecondsToLive int64
@@ -1733,16 +1733,12 @@ func readUserSettings(iniFile *ini.File, cfg *Cfg) error {
cfg.ExternalUserMngAnalytics = users.Key("external_manage_analytics").MustBool(false)
cfg.ExternalUserMngAnalyticsParams = valueAsString(users, "external_manage_analytics_params", "")
// Deprecated
//nolint:staticcheck
cfg.ViewersCanEdit = users.Key("viewers_can_edit").MustBool(false)
//nolint:staticcheck
if cfg.ViewersCanEdit {
cfg.Logger.Warn("[Deprecated] The viewers_can_edit configuration setting is deprecated. Please upgrade viewers to editors.")
}
// Deprecated
cfg.EditorsCanAdmin = users.Key("editors_can_admin").MustBool(false)
if cfg.EditorsCanAdmin {
cfg.Logger.Warn("[Deprecated] The editors_can_admin configuration setting is deprecated. Please upgrade editors to admin.")
}
userInviteMaxLifetimeVal := valueAsString(users, "user_invite_max_lifetime_duration", "24h")
userInviteMaxLifetimeDuration, err := gtime.ParseDuration(userInviteMaxLifetimeVal)