From 730bedf36f5c003abc5ea1b19e406b06f7eb8069 Mon Sep 17 00:00:00 2001
From: Leonard Gram <leo@xlson.com>
Date: Fri, 1 Nov 2019 15:42:22 +0100
Subject: [PATCH] LDAP Debug: No longer shows incorrectly matching groups based
 on role (#20018)

* LDAP Debug: No longer shows incorrectly matching groups based on role

Org Role was used as a shortcut to figure out what groups were matching
and which weren't. That lead to too all groups matching a specific role
to show up for a user if that user got that role.

* LDAP Debug: Fixes ordering of matches

The order of groups in the ldap.toml file is important, only the first
match for an organisation will be used. This means we have to iterate
based on the config and stop matching when a match is found.

We might want to think about showing further matches as potential
matches that are shadowed by the first match. That would possibly make
it easier to understand why one match is used instead of another one.

* LDAP Debug: never display more than one match for the same LDAP group/mapping.

* LDAP Debug: show all matches, even if they aren't used

* Update public/app/features/admin/ldap/LdapUserGroups.tsx

Co-Authored-By: gotjosh <josue.abreu@gmail.com>

* Update public/app/features/admin/ldap/LdapUserGroups.tsx

Co-Authored-By: gotjosh <josue.abreu@gmail.com>
---
 pkg/api/ldap_debug.go                         | 25 +++++++++++--------
 pkg/api/ldap_debug_test.go                    | 10 ++++++--
 .../features/admin/ldap/LdapUserGroups.tsx    |  9 ++++++-
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go
index de87c128a89..b9064e9bb20 100644
--- a/pkg/api/ldap_debug.go
+++ b/pkg/api/ldap_debug.go
@@ -278,28 +278,33 @@ func (server *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response {
 
 	orgRoles := []LDAPRoleDTO{}
 
-	// First, let's find the groupDN that we did match by inspecting the assigned user OrgRoles.
-	for _, group := range serverConfig.Groups {
-		orgRole, ok := user.OrgRoles[group.OrgId]
-
-		if ok && orgRole == group.OrgRole {
-			r := &LDAPRoleDTO{GroupDN: group.GroupDN, OrgId: group.OrgId, OrgRole: group.OrgRole}
-			orgRoles = append(orgRoles, *r)
+	// 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 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.
 	for _, userGroup := range user.Groups {
-		var matches int
+		var matched bool
 
 		for _, orgRole := range orgRoles {
 			if orgRole.GroupDN == userGroup { // we already matched it
-				matches++
+				matched = true
+				break
 			}
 		}
 
-		if matches < 1 {
+		if !matched {
 			r := &LDAPRoleDTO{GroupDN: userGroup}
 			orgRoles = append(orgRoles, *r)
 		}
diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go
index b5815503f61..ce930b455e7 100644
--- a/pkg/api/ldap_debug_test.go
+++ b/pkg/api/ldap_debug_test.go
@@ -122,7 +122,7 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) {
 				OrgRole: models.ROLE_ADMIN,
 			},
 			{
-				GroupDN: "cn=admins,ou=groups,dc=grafana2,dc=org",
+				GroupDN: "cn=admins,ou=groups,dc=grafana,dc=org",
 				OrgId:   2,
 				OrgRole: models.ROLE_VIEWER,
 			},
@@ -148,7 +148,7 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) {
 
 	sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe")
 
-	require.Equal(t, sc.resp.Code, http.StatusBadRequest)
+	require.Equal(t, http.StatusBadRequest, sc.resp.Code)
 
 	expected := `
 	{
@@ -183,6 +183,11 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) {
 				OrgId:   1,
 				OrgRole: models.ROLE_ADMIN,
 			},
+			{
+				GroupDN: "cn=admins2,ou=groups,dc=grafana,dc=org",
+				OrgId:   1,
+				OrgRole: models.ROLE_ADMIN,
+			},
 		},
 	}
 
@@ -240,6 +245,7 @@ func TestGetUserFromLDAPApiEndpoint_WithTeamHandler(t *testing.T) {
 		Name:           "John Doe",
 		Email:          "john.doe@example.com",
 		Login:          "johndoe",
+		Groups:         []string{"cn=admins,ou=groups,dc=grafana,dc=org"},
 		OrgRoles:       map[int64]models.RoleType{1: models.ROLE_ADMIN},
 		IsGrafanaAdmin: &isAdmin,
 	}
diff --git a/public/app/features/admin/ldap/LdapUserGroups.tsx b/public/app/features/admin/ldap/LdapUserGroups.tsx
index a21cd4da876..be74a1cb3f7 100644
--- a/public/app/features/admin/ldap/LdapUserGroups.tsx
+++ b/public/app/features/admin/ldap/LdapUserGroups.tsx
@@ -17,7 +17,14 @@ export const LdapUserGroups: FC<Props> = ({ groups, showAttributeMapping }) => {
           <thead>
             <tr>
               {showAttributeMapping && <th>LDAP Group</th>}
-              <th>Organisation</th>
+              <th>
+                Organization
+                <Tooltip placement="top" content="Only the first match for an Organization will be used" theme={'info'}>
+                  <span className="gf-form-help-icon">
+                    <i className="fa fa-info-circle" />
+                  </span>
+                </Tooltip>
+              </th>
               <th>Role</th>
             </tr>
           </thead>