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>
This commit is contained in:
Leonard Gram 2019-11-01 15:42:22 +01:00 committed by GitHub
parent 992b4b8adf
commit 730bedf36f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 31 additions and 13 deletions

View File

@ -278,28 +278,33 @@ func (server *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response {
orgRoles := []LDAPRoleDTO{} orgRoles := []LDAPRoleDTO{}
// First, let's find the groupDN that we did match by inspecting the assigned user OrgRoles. // Need to iterate based on the config groups as only the first match for an org is used
for _, group := range serverConfig.Groups { // We are showing all matches as that should help in understanding why one match wins out
orgRole, ok := user.OrgRoles[group.OrgId] // over another.
for _, configGroup := range serverConfig.Groups {
if ok && orgRole == group.OrgRole { for _, userGroup := range user.Groups {
r := &LDAPRoleDTO{GroupDN: group.GroupDN, OrgId: group.OrgId, OrgRole: group.OrgRole} if configGroup.GroupDN == userGroup {
orgRoles = append(orgRoles, *r) 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 // Then, we find what we did not match by inspecting the list of groups returned from
// LDAP against what we have already matched above. // LDAP against what we have already matched above.
for _, userGroup := range user.Groups { for _, userGroup := range user.Groups {
var matches int var matched bool
for _, orgRole := range orgRoles { for _, orgRole := range orgRoles {
if orgRole.GroupDN == userGroup { // we already matched it if orgRole.GroupDN == userGroup { // we already matched it
matches++ matched = true
break
} }
} }
if matches < 1 { if !matched {
r := &LDAPRoleDTO{GroupDN: userGroup} r := &LDAPRoleDTO{GroupDN: userGroup}
orgRoles = append(orgRoles, *r) orgRoles = append(orgRoles, *r)
} }

View File

@ -122,7 +122,7 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) {
OrgRole: models.ROLE_ADMIN, OrgRole: models.ROLE_ADMIN,
}, },
{ {
GroupDN: "cn=admins,ou=groups,dc=grafana2,dc=org", GroupDN: "cn=admins,ou=groups,dc=grafana,dc=org",
OrgId: 2, OrgId: 2,
OrgRole: models.ROLE_VIEWER, OrgRole: models.ROLE_VIEWER,
}, },
@ -148,7 +148,7 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) {
sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe") sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe")
require.Equal(t, sc.resp.Code, http.StatusBadRequest) require.Equal(t, http.StatusBadRequest, sc.resp.Code)
expected := ` expected := `
{ {
@ -183,6 +183,11 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) {
OrgId: 1, OrgId: 1,
OrgRole: models.ROLE_ADMIN, 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", Name: "John Doe",
Email: "john.doe@example.com", Email: "john.doe@example.com",
Login: "johndoe", Login: "johndoe",
Groups: []string{"cn=admins,ou=groups,dc=grafana,dc=org"},
OrgRoles: map[int64]models.RoleType{1: models.ROLE_ADMIN}, OrgRoles: map[int64]models.RoleType{1: models.ROLE_ADMIN},
IsGrafanaAdmin: &isAdmin, IsGrafanaAdmin: &isAdmin,
} }

View File

@ -17,7 +17,14 @@ export const LdapUserGroups: FC<Props> = ({ groups, showAttributeMapping }) => {
<thead> <thead>
<tr> <tr>
{showAttributeMapping && <th>LDAP Group</th>} {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> <th>Role</th>
</tr> </tr>
</thead> </thead>