diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index 04b77cb831c..75fd60b7e9d 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -456,6 +456,7 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*login.ExternalUserInf return extUser, nil } + isGrafanaAdmin := false for _, group := range server.Config.Groups { // only use the first match for each org if extUser.OrgRoles[group.OrgId] != "" { @@ -467,11 +468,12 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*login.ExternalUserInf extUser.OrgRoles[group.OrgId] = group.OrgRole } - if extUser.IsGrafanaAdmin == nil || !*extUser.IsGrafanaAdmin { - extUser.IsGrafanaAdmin = group.IsGrafanaAdmin + if !isGrafanaAdmin && (group.IsGrafanaAdmin != nil && *group.IsGrafanaAdmin) { + isGrafanaAdmin = true } } } + extUser.IsGrafanaAdmin = &isGrafanaAdmin // If there are group org mappings configured, but no matching mappings, // the user will not be able to login and will be disabled diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 8e37c456e2e..27f0563f065 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -243,6 +243,12 @@ func TestServer_Users(t *testing.T) { {Name: "username", Values: []string{"groot"}}, {Name: "name", Values: []string{"I am Groot"}}, }}}} + babyGrootDN := "dn=babygroot," + usersOU + babyGrootSearch := ldap.SearchResult{Entries: []*ldap.Entry{{DN: grootDN, + Attributes: []*ldap.EntryAttribute{ + {Name: "username", Values: []string{"babygroot"}}, + {Name: "name", Values: []string{"I am baby Groot"}}, + }}}} peterDN := "dn=peter," + usersOU peterSearch := ldap.SearchResult{Entries: []*ldap.Entry{{DN: peterDN, Attributes: []*ldap.EntryAttribute{ @@ -256,6 +262,12 @@ func TestServer_Users(t *testing.T) { {Name: "member", Values: []string{grootDN}}, }}}, } + babyCreaturesDN := "dn=babycreatures," + groupsOU + babyGrootGroups := ldap.SearchResult{Entries: []*ldap.Entry{{DN: babyCreaturesDN, + Attributes: []*ldap.EntryAttribute{ + {Name: "member", Values: []string{babyGrootDN}}, + }}}, + } humansDN := "dn=humans," + groupsOU peterGroups := ldap.SearchResult{Entries: []*ldap.Entry{{DN: humansDN, Attributes: []*ldap.EntryAttribute{ @@ -269,6 +281,8 @@ func TestServer_Users(t *testing.T) { switch request.Filter { case "(|(username=groot))": return &grootSearch, nil + case "(|(username=babygroot))": + return &babyGrootSearch, nil case "(|(username=peter))": return &peterSearch, nil default: @@ -278,6 +292,8 @@ func TestServer_Users(t *testing.T) { switch request.Filter { case "(member=groot)": return &grootGroups, nil + case "(member=babygroot)": + return &babyGrootGroups, nil case "(member=peter)": return &peterGroups, nil default: @@ -288,6 +304,7 @@ func TestServer_Users(t *testing.T) { } }) + isGrafanaAdmin := true cfg := setting.NewCfg() cfg.LDAPAuthEnabled = true @@ -306,9 +323,14 @@ func TestServer_Users(t *testing.T) { { GroupDN: creaturesDN, OrgId: 2, - IsGrafanaAdmin: new(bool), + IsGrafanaAdmin: &isGrafanaAdmin, OrgRole: "Admin", }, + { + GroupDN: babyCreaturesDN, + OrgId: 2, + OrgRole: "Editor", + }, }, }, Connection: conn, @@ -347,6 +369,22 @@ func TestServer_Users(t *testing.T) { require.True(t, mappingExist) require.Equal(t, roletype.RoleAdmin, role) require.False(t, res[0].IsDisabled) + require.NotNil(t, res[0].IsGrafanaAdmin) + assert.True(t, *res[0].IsGrafanaAdmin) + }) + t.Run("set Grafana Admin to false by default", func(t *testing.T) { + res, err := server.Users([]string{"babygroot"}) + require.NoError(t, err) + require.Len(t, res, 1) + require.Equal(t, "I am baby Groot", res[0].Name) + require.ElementsMatch(t, res[0].Groups, []string{babyCreaturesDN}) + require.Len(t, res[0].OrgRoles, 1) + role, mappingExist := res[0].OrgRoles[2] + require.True(t, mappingExist) + require.Equal(t, roletype.RoleEditor, role) + require.False(t, res[0].IsDisabled) + require.NotNil(t, res[0].IsGrafanaAdmin) + assert.False(t, *res[0].IsGrafanaAdmin) }) }) }