SAML: change the config option for making SAML UI accessible to org Admins (#67399)

* change from role grant overrides to SAML UI specific config option

* update permissions needed to access SAML UI

* PR feedback: change config name, change required perms to write, add a comment

pkg/api/api.go

@@ -225,7 +225,8 @@ func (hs *HTTPServer) registerRoutes() {
 	}
 
 	if hs.License.FeatureEnabled("saml") && hs.Features.IsEnabled(featuremgmt.FlagAuthenticationConfigUI) {
-		r.Get("/admin/authentication/", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionSettingsRead, ac.ScopeSettingsAll)), hs.Index)
+		// TODO change the scope when we extend the auth UI to more providers
+		r.Get("/admin/authentication/", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionSettingsWrite, ac.ScopeSettingsSAML)), hs.Index)
 	}
 
 	// authed api

pkg/services/accesscontrol/acimpl/service.go

@@ -36,7 +36,7 @@ func ProvideService(cfg *setting.Cfg, store db.DB, routeRegister routing.RouteRe
 
 	if !accesscontrol.IsDisabled(cfg) {
 		api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints()
-		if err := accesscontrol.DeclareFixedRoles(service); err != nil {
+		if err := accesscontrol.DeclareFixedRoles(service, cfg); err != nil {
 			return nil, err
 		}
 	}
@@ -166,12 +166,7 @@ func (s *Service) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistrat
 		return nil
 	}
 
-	for i := range registrations {
+	for _, r := range registrations {
-		r := registrations[i]
-		if r.AllowGrantsOverride {
-			s.handleGrantOverrides(&r)
-		}
-
 		err := accesscontrol.ValidateFixedRole(r.Role)
 		if err != nil {
 			return err
@@ -188,15 +183,6 @@ func (s *Service) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistrat
 	return nil
 }
 
-func (s *Service) handleGrantOverrides(r *accesscontrol.RoleRegistration) {
-	// Replace ":" and "." with "_" to match the key in the config
-	key := strings.ReplaceAll(strings.ReplaceAll(r.Role.Name, ":", "_"), ".", "_")
-	if overrides, ok := s.cfg.RBACGrantOverrides[key]; ok {
-		r.Grants = overrides
-		s.log.Info("Overriding grants for role", "role", r.Role.Name, "overrides", overrides)
-	}
-}
-
 // RegisterFixedRoles registers all declared roles in RAM
 func (s *Service) RegisterFixedRoles(ctx context.Context) error {
 	// If accesscontrol is disabled no need to register roles

pkg/services/accesscontrol/acimpl/service_test.go

@@ -161,77 +161,6 @@ func TestService_DeclareFixedRoles(t *testing.T) {
 	}
 }
 
-func TestService_DeclareFixedRoles_Overrides(t *testing.T) {
-	tests := []struct {
-		name         string
-		registration accesscontrol.RoleRegistration
-		overrides    map[string][]string
-		wantGrants   []string
-		wantErr      bool
-	}{
-		{
-			name: "no grant override",
-			registration: accesscontrol.RoleRegistration{
-				Role:                accesscontrol.RoleDTO{Name: "fixed:test:test"},
-				Grants:              []string{"Admin"},
-				AllowGrantsOverride: true,
-			},
-			wantGrants: []string{"Admin"},
-			wantErr:    false,
-		},
-		{
-			name: "should account for grant overrides",
-			registration: accesscontrol.RoleRegistration{
-				Role:                accesscontrol.RoleDTO{Name: "fixed:test:test"},
-				Grants:              []string{"Admin"},
-				AllowGrantsOverride: true,
-			},
-			overrides:  map[string][]string{"fixed_test_test": {"Viewer", "Grafana Admin"}},
-			wantGrants: []string{"Viewer", "Grafana Admin"},
-			wantErr:    false,
-		},
-		{
-			name: "should not account for grant overrides",
-			registration: accesscontrol.RoleRegistration{
-				Role:                accesscontrol.RoleDTO{Name: "fixed:test:test"},
-				Grants:              []string{"Admin"},
-				AllowGrantsOverride: false,
-			},
-			overrides:  map[string][]string{"fixed_test_test": {"Viewer", "Grafana Admin"}},
-			wantGrants: []string{"Admin"},
-			wantErr:    false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			ac := setupTestEnv(t)
-
-			// Reset the registations
-			ac.registrations = accesscontrol.RegistrationList{}
-			ac.cfg.RBACGrantOverrides = tt.overrides
-
-			// Test
-			err := ac.DeclareFixedRoles(tt.registration)
-			if tt.wantErr {
-				require.Error(t, err)
-				return
-			}
-			require.NoError(t, err)
-
-			registrationCnt := 0
-			grants := []string{}
-			ac.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
-				registrationCnt++
-				grants = registration.Grants
-				return true
-			})
-			require.Equal(t, 1, registrationCnt,
-				"expected service registration list to contain the registration")
-			require.ElementsMatch(t, tt.wantGrants, grants)
-		})
-	}
-}
-
 func TestService_DeclarePluginRoles(t *testing.T) {
 	tests := []struct {
 		name          string

pkg/services/accesscontrol/models.go

@@ -16,9 +16,8 @@ var ErrInternal = errutil.NewBase(errutil.StatusInternal, "accesscontrol.interna
 // RoleRegistration stores a role and its assignments to built-in roles
 // (Viewer, Editor, Admin, Grafana Admin)
 type RoleRegistration struct {
-	Role                RoleDTO
+	Role   RoleDTO
-	Grants              []string
+	Grants []string
-	AllowGrantsOverride bool
 }
 
 // Role is the model for Role in RBAC.

pkg/services/accesscontrol/roles.go

@@ -6,6 +6,7 @@ import (
 	"sync"
 
 	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 // Roles definition
@@ -191,7 +192,7 @@ var (
 )
 
 // Declare OSS roles to the accesscontrol service
-func DeclareFixedRoles(service Service) error {
+func DeclareFixedRoles(service Service, cfg *setting.Cfg) error {
 	ldapReader := RoleRegistration{
 		Role:   ldapReaderRole,
 		Grants: []string{RoleGrafanaAdmin},
@@ -224,10 +225,15 @@ func DeclareFixedRoles(service Service) error {
 		Role:   usersWriterRole,
 		Grants: []string{RoleGrafanaAdmin},
 	}
+
+	// TODO: Move to own service when implemented
 	authenticationConfigWriter := RoleRegistration{
-		Role:                authenticationConfigWriterRole,
+		Role:   authenticationConfigWriterRole,
-		Grants:              []string{RoleGrafanaAdmin},
+		Grants: []string{RoleGrafanaAdmin},
-		AllowGrantsOverride: true,
+	}
+
+	if cfg.AuthConfigUIAdminAccess {
+		authenticationConfigWriter.Grants = append(authenticationConfigWriter.Grants, string(org.RoleAdmin))
 	}
 
 	return service.DeclareFixedRoles(ldapReader, ldapWriter, orgUsersReader, orgUsersWriter,

pkg/services/navtree/navtreeimpl/admin.go

@@ -163,9 +163,7 @@ func enableServiceAccount(s *ServiceImpl, c *contextmodel.ReqContext) bool {
 
 func evalAuthenticationSettings() ac.Evaluator {
 	return ac.EvalAll(
-		ac.EvalPermission(ac.ActionSettingsWrite, ac.ScopeSettingsAuth),
 		ac.EvalPermission(ac.ActionSettingsWrite, ac.ScopeSettingsSAML),
-		ac.EvalPermission(ac.ActionSettingsRead, ac.ScopeSettingsAuth),
 		ac.EvalPermission(ac.ActionSettingsRead, ac.ScopeSettingsSAML),
 	)
 }

pkg/setting/setting.go

@@ -269,6 +269,9 @@ type Cfg struct {
 	AdminEmail                   string
 	DisableSyncLock              bool
 	DisableLoginForm             bool
+	// Not documented & not supported
+	// stand in until a more complete solution is implemented
+	AuthConfigUIAdminAccess bool
 
 	// AWS Plugin Auth
 	AWSAllowedAuthProviders []string
@@ -509,8 +512,6 @@ type Cfg struct {
 	RBACPermissionValidationEnabled bool
 	// Reset basic roles permissions on start-up
 	RBACResetBasicRoles bool
-	// Override default fixed role assignments
-	RBACGrantOverrides map[string][]string
 
 	// GRPC Server.
 	GRPCServerNetwork   string
@@ -1459,7 +1460,8 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {
 
 	// Debug setting unlocking frontend auth sync lock. Users will still be reset on their next login.
 	cfg.DisableSyncLock = auth.Key("disable_sync_lock").MustBool(false)
-
+	// Do not use
+	cfg.AuthConfigUIAdminAccess = auth.Key("config_ui_admin_access").MustBool(false)
 	cfg.DisableLoginForm = auth.Key("disable_login_form").MustBool(false)
 	DisableSignoutMenu = auth.Key("disable_signout_menu").MustBool(false)
 
@@ -1567,17 +1569,6 @@ func readAccessControlSettings(iniFile *ini.File, cfg *Cfg) {
 	cfg.RBACPermissionCache = rbac.Key("permission_cache").MustBool(true)
 	cfg.RBACPermissionValidationEnabled = rbac.Key("permission_validation_enabled").MustBool(false)
 	cfg.RBACResetBasicRoles = rbac.Key("reset_basic_roles").MustBool(false)
-
-	rbacOverrides := iniFile.Section("rbac.overrides")
-	cfg.RBACGrantOverrides = map[string][]string{}
-	for _, key := range rbacOverrides.Keys() {
-		value := key.MustString("")
-		grants := strings.Split(value, ",")
-		for i, grant := range grants {
-			grants[i] = strings.TrimSpace(grant)
-		}
-		cfg.RBACGrantOverrides[key.Name()] = grants
-	}
 }
 
 func readUserSettings(iniFile *ini.File, cfg *Cfg) error {

public/app/routes/routes.tsx

@@ -309,6 +309,7 @@ export function getAppRoutes(): RouteDescriptor[] {
     // ADMIN
     {
       path: '/admin/authentication',
+      roles: () => contextSrv.evaluatePermission(() => ['Admin', 'ServerAdmin'], [AccessControlAction.SettingsWrite]),
       component:
         config.featureToggles.authenticationConfigUI && config.licenseInfo.enabledFeatures?.saml
           ? SafeDynamicImport(