From 94fd03f44fee3f32a3ba9a88c59a441011ab81ea Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Fri, 22 Apr 2022 15:45:54 +0200 Subject: [PATCH] LDAP: Fix debug view to display the actual computed mapping in ldap.go (#48103) * LDAP debug fix with Org role inheritance Co-authored-by: Jguer * ldap debug coherent with ldap.go Co-authored-by: Jguer Co-authored-by: Jguer --- pkg/api/ldap_debug.go | 54 ++++++++++---------------- pkg/services/ldap/helpers.go | 2 +- pkg/services/ldap/ldap.go | 2 +- pkg/services/ldap/ldap_helpers_test.go | 2 +- 4 files changed, 23 insertions(+), 37 deletions(-) diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index c7a978e0234..f62daaa0407 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -241,7 +241,7 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration", err) } - ldap := newLDAP(ldapConfig.Servers) + multiLDAP := newLDAP(ldapConfig.Servers) username := web.Params(c.Req)[":username"] @@ -249,9 +249,8 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "Validation error. You must specify an username", nil) } - user, serverConfig, err := ldap.User(username) - - if user == nil { + user, serverConfig, err := multiLDAP.User(username) + if user == nil || err != nil { return response.Error(http.StatusNotFound, "No user was found in the LDAP server(s) with that username", err) } @@ -268,45 +267,32 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) response.Response { IsDisabled: user.IsDisabled, } - orgRoles := []LDAPRoleDTO{} - - // Need to iterate based on the config groups as only the first match for an org is used - // We are showing all matches as that should help in understanding why one match wins out - // over another. - for _, configGroup := range serverConfig.Groups { - for _, userGroup := range user.Groups { - if strings.EqualFold(configGroup.GroupDN, userGroup) { - r := &LDAPRoleDTO{GroupDN: configGroup.GroupDN, OrgId: configGroup.OrgId, OrgRole: configGroup.OrgRole} - orgRoles = append(orgRoles, *r) - break - } - } - //} - } - - // Then, we find what we did not match by inspecting the list of groups returned from - // LDAP against what we have already matched above. + unmappedUserGroups := map[string]struct{}{} for _, userGroup := range user.Groups { - var matched bool + unmappedUserGroups[strings.ToLower(userGroup)] = struct{}{} + } - for _, orgRole := range orgRoles { - if strings.EqualFold(orgRole.GroupDN, userGroup) { // we already matched it - matched = true - break - } + orgRolesMap := map[int64]models.RoleType{} + for _, group := range serverConfig.Groups { + // only use the first match for each org + if orgRolesMap[group.OrgId] != "" { + continue } - if !matched { - r := &LDAPRoleDTO{GroupDN: userGroup} - orgRoles = append(orgRoles, *r) + if ldap.IsMemberOf(user.Groups, group.GroupDN) { + orgRolesMap[group.OrgId] = group.OrgRole + u.OrgRoles = append(u.OrgRoles, LDAPRoleDTO{GroupDN: group.GroupDN, + OrgId: group.OrgId, OrgRole: group.OrgRole}) + delete(unmappedUserGroups, strings.ToLower(group.GroupDN)) } } - u.OrgRoles = orgRoles + for userGroup := range unmappedUserGroups { + u.OrgRoles = append(u.OrgRoles, LDAPRoleDTO{GroupDN: userGroup}) + } ldapLogger.Debug("mapping org roles", "orgsRoles", u.OrgRoles) - err = u.FetchOrgs(c.Req.Context(), hs.SQLStore) - if err != nil { + if err := u.FetchOrgs(c.Req.Context(), hs.SQLStore); err != nil { return response.Error(http.StatusBadRequest, "An organization was not found - Please verify your LDAP configuration", err) } diff --git a/pkg/services/ldap/helpers.go b/pkg/services/ldap/helpers.go index 12438f23f6c..851725f2d19 100644 --- a/pkg/services/ldap/helpers.go +++ b/pkg/services/ldap/helpers.go @@ -6,7 +6,7 @@ import ( "gopkg.in/ldap.v3" ) -func isMemberOf(memberOf []string, group string) bool { +func IsMemberOf(memberOf []string, group string) bool { if group == "*" { return true } diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index 1ad3fa15905..02f9e2b2f13 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -422,7 +422,7 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn continue } - if isMemberOf(memberOf, group.GroupDN) { + if IsMemberOf(memberOf, group.GroupDN) { extUser.OrgRoles[group.OrgId] = group.OrgRole if extUser.IsGrafanaAdmin == nil || !*extUser.IsGrafanaAdmin { extUser.IsGrafanaAdmin = group.IsGrafanaAdmin diff --git a/pkg/services/ldap/ldap_helpers_test.go b/pkg/services/ldap/ldap_helpers_test.go index e276917c973..887a2a0a5df 100644 --- a/pkg/services/ldap/ldap_helpers_test.go +++ b/pkg/services/ldap/ldap_helpers_test.go @@ -21,7 +21,7 @@ func TestIsMemberOf(t *testing.T) { for _, tc := range tests { t.Run(fmt.Sprintf("isMemberOf(%v, \"%s\") = %v", tc.memberOf, tc.group, tc.expected), func(t *testing.T) { - assert.Equal(t, tc.expected, isMemberOf(tc.memberOf, tc.group)) + assert.Equal(t, tc.expected, IsMemberOf(tc.memberOf, tc.group)) }) } }