From 10c080dad16c3e65e880a8000a9e290f37f2eb72 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Wed, 12 Oct 2022 13:33:33 +0200 Subject: [PATCH] LDAP: Add `skip_org_role_sync` configuration option (#56679) * LDAP: Add skip_org_role_sync option * Document the new config option * Nit on docs * Update docs/sources/setup-grafana/configure-security/configure-authentication/ldap.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Docs suggestions Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Jguer * Add test, Fix disabled user when no role Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Jguer --- conf/defaults.ini | 1 + conf/sample.ini | 2 + .../configure-authentication/ldap.md | 27 +++- packages/grafana-data/src/types/config.ts | 1 + pkg/api/frontendsettings.go | 1 + pkg/services/ldap/ldap.go | 8 +- pkg/services/ldap/ldap_test.go | 118 ++++++++++++++++++ pkg/services/ldap/settings.go | 4 + pkg/setting/setting.go | 8 +- public/app/features/admin/UserAdminPage.tsx | 17 ++- 10 files changed, 175 insertions(+), 12 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index e5c19f5e4f7..1a682053900 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -632,6 +632,7 @@ allow_assign_grafana_admin = false enabled = false config_file = /etc/grafana/ldap.toml allow_sign_up = true +skip_org_role_sync = false # LDAP background sync (Enterprise only) # At 1 am every day diff --git a/conf/sample.ini b/conf/sample.ini index 22921822f1d..a79f44d5c0e 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -624,6 +624,8 @@ ;enabled = false ;config_file = /etc/grafana/ldap.toml ;allow_sign_up = true +# prevent synchronizing ldap users organization roles +;skip_org_role_sync = false # LDAP background sync (Enterprise only) # At 1 am every day diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/ldap.md b/docs/sources/setup-grafana/configure-security/configure-authentication/ldap.md index 6ba7db02aff..0ef6fe4ce9a 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/ldap.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/ldap.md @@ -28,7 +28,7 @@ This means that you should be able to configure LDAP integration using any compl In order to use LDAP integration you'll first need to enable LDAP in the [main config file]({{< relref "../../configure-grafana/" >}}) as well as specify the path to the LDAP specific configuration file (default: `/etc/grafana/ldap.toml`). -```bash +```ini [auth.ldap] # Set to `true` to enable LDAP integration (default: `false`) enabled = true @@ -36,11 +36,32 @@ enabled = true # Path to the LDAP specific configuration file (default: `/etc/grafana/ldap.toml`) config_file = /etc/grafana/ldap.toml -# Allow sign up should almost always be true (default) to allow new Grafana users to be created (if LDAP authentication is ok). If set to -# false only pre-existing Grafana users will be able to login (if LDAP authentication is ok). +# Allow sign-up should be `true` (default) to allow Grafana to create users on successful LDAP authentication. +# If set to `false` only already existing Grafana users will be able to login. allow_sign_up = true ``` +## Disable org role synchronization + +If you use LDAP to authenticate users but don't use role mapping, and prefer to manually assign organizations +and roles, you can use the `skip_org_role_sync` configuration option. + +```ini +[auth.ldap] +# Set to `true` to enable LDAP integration (default: `false`) +enabled = true + +# Path to the LDAP specific configuration file (default: `/etc/grafana/ldap.toml`) +config_file = /etc/grafana/ldap.toml + +# Allow sign-up should be `true` (default) to allow Grafana to create users on successful LDAP authentication. +# If set to `false` only already existing Grafana users will be able to login. +allow_sign_up = true + +# Prevent synchronizing ldap users organization roles +skip_org_role_sync = true +``` + ## Grafana LDAP Configuration Depending on which LDAP server you're using and how that's configured your Grafana LDAP configuration may vary. diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index 7fa8ec72e86..06cbc24da04 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -220,5 +220,6 @@ export interface GrafanaConfig { export interface AuthSettings { OAuthSkipOrgRoleUpdateSync?: boolean; SAMLSkipOrgRoleSync?: boolean; + LDAPSkipOrgRoleSync?: boolean; DisableSyncLock?: boolean; } diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 817d0cd8912..3ff4e29913e 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -146,6 +146,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i "auth": map[string]interface{}{ "OAuthSkipOrgRoleUpdateSync": hs.Cfg.OAuthSkipOrgRoleUpdateSync, "SAMLSkipOrgRoleSync": hs.Cfg.SectionWithEnvOverrides("auth.saml").Key("skip_org_role_sync").MustBool(false), + "LDAPSkipOrgRoleSync": hs.Cfg.LDAPSkipOrgRoleSync, "DisableSyncLock": hs.Cfg.DisableSyncLock, }, "buildInfo": map[string]interface{}{ diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index 4120d89a5fd..d2048dd5c69 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -363,7 +363,8 @@ func (server *Server) users(logins []string) ( // If there are no ldap group mappings access is true // otherwise a single group must match func (server *Server) validateGrafanaUser(user *models.ExternalUserInfo) error { - if len(server.Config.Groups) > 0 && (len(user.OrgRoles) == 0 && (user.IsGrafanaAdmin == nil || !*user.IsGrafanaAdmin)) { + if !SkipOrgRoleSync() && len(server.Config.Groups) > 0 && + (len(user.OrgRoles) == 0 && (user.IsGrafanaAdmin == nil || !*user.IsGrafanaAdmin)) { server.log.Error( "User does not belong in any of the specified LDAP groups", "username", user.Login, @@ -446,6 +447,11 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn OrgRoles: map[int64]org.RoleType{}, } + // Skipping org role sync + if SkipOrgRoleSync() { + return extUser, nil + } + for _, group := range server.Config.Groups { // only use the first match for each org if extUser.OrgRoles[group.OrgId] != "" { diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 042ac045506..22d0b8b9b9d 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -10,6 +10,8 @@ import ( "gopkg.in/ldap.v3" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/models/roletype" + "github.com/grafana/grafana/pkg/setting" ) func TestNew(t *testing.T) { @@ -221,6 +223,122 @@ func TestServer_Users(t *testing.T) { require.Len(t, res, 1) assert.Equal(t, "Grot the First", res[0].Name) }) + + t.Run("org role mapping", func(t *testing.T) { + conn := &MockConnection{} + + usersOU := "ou=users,dc=example,dc=org" + grootDN := "dn=groot," + usersOU + grootSearch := ldap.SearchResult{Entries: []*ldap.Entry{{DN: grootDN, + Attributes: []*ldap.EntryAttribute{ + {Name: "username", Values: []string{"groot"}}, + {Name: "name", Values: []string{"I am Groot"}}, + }}}} + peterDN := "dn=peter," + usersOU + peterSearch := ldap.SearchResult{Entries: []*ldap.Entry{{DN: peterDN, + Attributes: []*ldap.EntryAttribute{ + {Name: "username", Values: []string{"peter"}}, + {Name: "name", Values: []string{"Peter"}}, + }}}} + groupsOU := "ou=groups,dc=example,dc=org" + creaturesDN := "dn=creatures," + groupsOU + grootGroups := ldap.SearchResult{Entries: []*ldap.Entry{{DN: creaturesDN, + Attributes: []*ldap.EntryAttribute{ + {Name: "member", Values: []string{grootDN}}, + }}}, + } + humansDN := "dn=humans," + groupsOU + peterGroups := ldap.SearchResult{Entries: []*ldap.Entry{{DN: humansDN, + Attributes: []*ldap.EntryAttribute{ + {Name: "member", Values: []string{peterDN}}, + }}}, + } + + conn.setSearchFunc(func(request *ldap.SearchRequest) (*ldap.SearchResult, error) { + switch request.BaseDN { + case usersOU: + switch request.Filter { + case "(|(username=groot))": + return &grootSearch, nil + case "(|(username=peter))": + return &peterSearch, nil + default: + return nil, fmt.Errorf("test case not defined for user filter: '%s'", request.Filter) + } + case groupsOU: + switch request.Filter { + case "(member=groot)": + return &grootGroups, nil + case "(member=peter)": + return &peterGroups, nil + default: + return nil, fmt.Errorf("test case not defined for group filter: '%s'", request.Filter) + } + default: + return nil, fmt.Errorf("test case not defined for baseDN: '%s'", request.BaseDN) + } + }) + + server := &Server{ + Config: &ServerConfig{ + Attr: AttributeMap{ + Username: "username", + Name: "name", + }, + SearchBaseDNs: []string{usersOU}, + SearchFilter: "(username=%s)", + GroupSearchFilter: "(member=%s)", + GroupSearchBaseDNs: []string{groupsOU}, + Groups: []*GroupToOrgRole{ + { + GroupDN: creaturesDN, + OrgId: 2, + IsGrafanaAdmin: new(bool), + OrgRole: "Admin", + }, + }, + }, + Connection: conn, + log: log.New("test-logger"), + } + + t.Run("disable user with no mapping", func(t *testing.T) { + res, err := server.Users([]string{"peter"}) + require.NoError(t, err) + require.Len(t, res, 1) + require.Equal(t, "Peter", res[0].Name) + require.ElementsMatch(t, res[0].Groups, []string{humansDN}) + require.Empty(t, res[0].OrgRoles) + require.True(t, res[0].IsDisabled) + }) + t.Run("skip org role sync", func(t *testing.T) { + backup := setting.LDAPSkipOrgRoleSync + defer func() { + setting.LDAPSkipOrgRoleSync = backup + }() + setting.LDAPSkipOrgRoleSync = true + + res, err := server.Users([]string{"groot"}) + require.NoError(t, err) + require.Len(t, res, 1) + require.Equal(t, "I am Groot", res[0].Name) + require.ElementsMatch(t, res[0].Groups, []string{creaturesDN}) + require.Empty(t, res[0].OrgRoles) + require.False(t, res[0].IsDisabled) + }) + t.Run("sync org role", func(t *testing.T) { + res, err := server.Users([]string{"groot"}) + require.NoError(t, err) + require.Len(t, res, 1) + require.Equal(t, "I am Groot", res[0].Name) + require.ElementsMatch(t, res[0].Groups, []string{creaturesDN}) + require.Len(t, res[0].OrgRoles, 1) + role, mappingExist := res[0].OrgRoles[2] + require.True(t, mappingExist) + require.Equal(t, roletype.RoleAdmin, role) + require.False(t, res[0].IsDisabled) + }) + }) } func TestServer_UserBind(t *testing.T) { diff --git a/pkg/services/ldap/settings.go b/pkg/services/ldap/settings.go index dbada844bec..450b43b078a 100644 --- a/pkg/services/ldap/settings.go +++ b/pkg/services/ldap/settings.go @@ -76,6 +76,10 @@ func IsEnabled() bool { return setting.LDAPEnabled } +func SkipOrgRoleSync() bool { + return setting.LDAPSkipOrgRoleSync +} + // ReloadConfig reads the config from the disk and caches it. func ReloadConfig() error { if !IsEnabled() { diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index c4d182eb8ff..84fb4d1e4e2 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -147,6 +147,7 @@ var ( // LDAP LDAPEnabled bool + LDAPSkipOrgRoleSync bool LDAPConfigFile string LDAPSyncCron string LDAPAllowSignup bool @@ -413,8 +414,9 @@ type Cfg struct { FeedbackLinksEnabled bool // LDAP - LDAPEnabled bool - LDAPAllowSignup bool + LDAPEnabled bool + LDAPSkipOrgRoleSync bool + LDAPAllowSignup bool Quota QuotaSettings @@ -1131,6 +1133,8 @@ func (cfg *Cfg) readLDAPConfig() { LDAPSyncCron = ldapSec.Key("sync_cron").String() LDAPEnabled = ldapSec.Key("enabled").MustBool(false) cfg.LDAPEnabled = LDAPEnabled + LDAPSkipOrgRoleSync = ldapSec.Key("skip_org_role_sync").MustBool(false) + cfg.LDAPSkipOrgRoleSync = LDAPSkipOrgRoleSync LDAPActiveSyncEnabled = ldapSec.Key("active_sync_enabled").MustBool(false) LDAPAllowSignup = ldapSec.Key("allow_sign_up").MustBool(true) cfg.LDAPAllowSignup = LDAPAllowSignup diff --git a/public/app/features/admin/UserAdminPage.tsx b/public/app/features/admin/UserAdminPage.tsx index b3087552d04..a1c26e08a1a 100644 --- a/public/app/features/admin/UserAdminPage.tsx +++ b/public/app/features/admin/UserAdminPage.tsx @@ -105,7 +105,7 @@ export class UserAdminPage extends PureComponent { render() { const { user, orgs, sessions, ldapSyncInfo, isLoading } = this.props; - const isLDAPUser = user && user.isExternal && user.authLabels && user.authLabels.includes('LDAP'); + const isLDAPUser = user?.isExternal && user?.authLabels?.includes('LDAP'); const canReadSessions = contextSrv.hasPermission(AccessControlAction.UsersAuthTokenList); const canReadLDAPStatus = contextSrv.hasPermission(AccessControlAction.LDAPStatusRead); const isOAuthUserWithSkippableSync = @@ -113,9 +113,10 @@ export class UserAdminPage extends PureComponent { const isSAMLUser = user?.isExternal && user?.authLabels?.includes('SAML'); const isUserSynced = !config.auth.DisableSyncLock && - ((user?.isExternal && !(isOAuthUserWithSkippableSync || isSAMLUser)) || + ((user?.isExternal && !(isOAuthUserWithSkippableSync || isSAMLUser || isLDAPUser)) || (!config.auth.OAuthSkipOrgRoleUpdateSync && isOAuthUserWithSkippableSync) || - (!config.auth.SAMLSkipOrgRoleSync && isSAMLUser)); + (!config.auth.SAMLSkipOrgRoleSync && isSAMLUser) || + (!config.auth.LDAPSkipOrgRoleSync && isLDAPUser)); const pageNav: NavModelItem = { text: user?.login ?? '', @@ -137,9 +138,13 @@ export class UserAdminPage extends PureComponent { onUserEnable={this.onUserEnable} onPasswordChange={this.onPasswordChange} /> - {isLDAPUser && featureEnabled('ldapsync') && ldapSyncInfo && canReadLDAPStatus && ( - - )} + {!config.auth.LDAPSkipOrgRoleSync && + isLDAPUser && + featureEnabled('ldapsync') && + ldapSyncInfo && + canReadLDAPStatus && ( + + )} )}