diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index b5f9d91bf4a..de87c128a89 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -34,7 +34,7 @@ type LDAPAttribute struct { } // RoleDTO is a serializer for mapped roles from LDAP -type RoleDTO struct { +type LDAPRoleDTO struct { OrgId int64 `json:"orgId"` OrgName string `json:"orgName"` OrgRole models.RoleType `json:"orgRole"` @@ -49,7 +49,7 @@ type LDAPUserDTO struct { Username *LDAPAttribute `json:"login"` IsGrafanaAdmin *bool `json:"isGrafanaAdmin"` IsDisabled bool `json:"isDisabled"` - OrgRoles []RoleDTO `json:"roles"` + OrgRoles []LDAPRoleDTO `json:"roles"` Teams []models.TeamOrgGroupDTO `json:"teams"` } @@ -90,6 +90,10 @@ func (user *LDAPUserDTO) FetchOrgs() error { } for i, orgDTO := range user.OrgRoles { + if orgDTO.OrgId < 1 { + continue + } + orgName := orgNamesById[orgDTO.OrgId] if orgName != "" { @@ -256,7 +260,7 @@ func (server *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { user, serverConfig, err := ldap.User(username) if user == nil { - return Error(http.StatusNotFound, "No user was found on the LDAP server(s)", err) + return Error(http.StatusNotFound, "No user was found in the LDAP server(s) with that username", err) } logger.Debug("user found", "user", user) @@ -272,22 +276,32 @@ func (server *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { IsDisabled: user.IsDisabled, } - orgRoles := []RoleDTO{} + orgRoles := []LDAPRoleDTO{} - for _, g := range serverConfig.Groups { - role := &RoleDTO{} + // 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 isMatchToLDAPGroup(user, g) { - role.OrgId = g.OrgID - role.OrgRole = user.OrgRoles[g.OrgID] - role.GroupDN = g.GroupDN + if ok && orgRole == group.OrgRole { + r := &LDAPRoleDTO{GroupDN: group.GroupDN, OrgId: group.OrgId, OrgRole: group.OrgRole} + orgRoles = append(orgRoles, *r) + } + } - orgRoles = append(orgRoles, *role) - } else { - role.OrgId = g.OrgID - role.GroupDN = g.GroupDN + // 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 - orgRoles = append(orgRoles, *role) + for _, orgRole := range orgRoles { + if orgRole.GroupDN == userGroup { // we already matched it + matches++ + } + } + + if matches < 1 { + r := &LDAPRoleDTO{GroupDN: userGroup} + orgRoles = append(orgRoles, *r) } } @@ -312,12 +326,6 @@ func (server *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { return JSON(200, u) } -// isMatchToLDAPGroup determines if we were able to match an LDAP group to an organization+role. -// Since we allow one role per organization. If it's set, we were able to match it. -func isMatchToLDAPGroup(user *models.ExternalUserInfo, groupConfig *ldap.GroupToOrgRole) bool { - return user.OrgRoles[groupConfig.OrgID] == groupConfig.OrgRole -} - // splitName receives the full name of a user and splits it into two parts: A name and a surname. func splitName(name string) (string, string) { names := util.SplitString(name) diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go index 938c3b1b228..b5815503f61 100644 --- a/pkg/api/ldap_debug_test.go +++ b/pkg/api/ldap_debug_test.go @@ -94,7 +94,7 @@ func TestGetUserFromLDAPApiEndpoint_UserNotFound(t *testing.T) { sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist") require.Equal(t, sc.resp.Code, http.StatusNotFound) - assert.JSONEq(t, "{\"message\":\"No user was found on the LDAP server(s)\"}", sc.resp.Body.String()) + assert.JSONEq(t, "{\"message\":\"No user was found in the LDAP server(s) with that username\"}", sc.resp.Body.String()) } func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) { @@ -103,6 +103,7 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(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, 2: models.ROLE_VIEWER}, IsGrafanaAdmin: &isAdmin, } @@ -117,12 +118,12 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) { Groups: []*ldap.GroupToOrgRole{ { GroupDN: "cn=admins,ou=groups,dc=grafana,dc=org", - OrgID: 1, + OrgId: 1, OrgRole: models.ROLE_ADMIN, }, { GroupDN: "cn=admins,ou=groups,dc=grafana2,dc=org", - OrgID: 2, + OrgId: 2, OrgRole: models.ROLE_VIEWER, }, }, @@ -164,6 +165,7 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) { Name: "John Doe", Email: "john.doe@example.com", Login: "johndoe", + Groups: []string{"cn=admins,ou=groups,dc=grafana,dc=org", "another-group-not-matched"}, OrgRoles: map[int64]models.RoleType{1: models.ROLE_ADMIN}, IsGrafanaAdmin: &isAdmin, } @@ -178,7 +180,7 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) { Groups: []*ldap.GroupToOrgRole{ { GroupDN: "cn=admins,ou=groups,dc=grafana,dc=org", - OrgID: 1, + OrgId: 1, OrgRole: models.ROLE_ADMIN, }, }, @@ -203,7 +205,7 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) { sc := getUserFromLDAPContext(t, "/api/admin/ldap/johndoe") - require.Equal(t, sc.resp.Code, http.StatusOK) + assert.Equal(t, sc.resp.Code, http.StatusOK) expected := ` { @@ -222,7 +224,8 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) { "isGrafanaAdmin": true, "isDisabled": false, "roles": [ - { "orgId": 1, "orgRole": "Admin", "orgName": "Main Org.", "groupDN": "cn=admins,ou=groups,dc=grafana,dc=org" } + { "orgId": 1, "orgRole": "Admin", "orgName": "Main Org.", "groupDN": "cn=admins,ou=groups,dc=grafana,dc=org" }, + { "orgId": 0, "orgRole": "", "orgName": "", "groupDN": "another-group-not-matched" } ], "teams": null } @@ -251,7 +254,7 @@ func TestGetUserFromLDAPApiEndpoint_WithTeamHandler(t *testing.T) { Groups: []*ldap.GroupToOrgRole{ { GroupDN: "cn=admins,ou=groups,dc=grafana,dc=org", - OrgID: 1, + OrgId: 1, OrgRole: models.ROLE_ADMIN, }, }, diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index d079d632f25..a6718ce69d9 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -408,12 +408,12 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn for _, group := range server.Config.Groups { // only use the first match for each org - if extUser.OrgRoles[group.OrgID] != "" { + if extUser.OrgRoles[group.OrgId] != "" { continue } if isMemberOf(memberOf, group.GroupDN) { - extUser.OrgRoles[group.OrgID] = group.OrgRole + extUser.OrgRoles[group.OrgId] = group.OrgRole if extUser.IsGrafanaAdmin == nil || !*extUser.IsGrafanaAdmin { extUser.IsGrafanaAdmin = group.IsGrafanaAdmin } diff --git a/pkg/services/ldap/ldap_private_test.go b/pkg/services/ldap/ldap_private_test.go index ff8bc430ed9..0382cdc5987 100644 --- a/pkg/services/ldap/ldap_private_test.go +++ b/pkg/services/ldap/ldap_private_test.go @@ -3,11 +3,10 @@ package ldap import ( "testing" - . "github.com/smartystreets/goconvey/convey" - "gopkg.in/ldap.v3" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" + "gopkg.in/ldap.v3" ) func TestLDAPPrivateMethods(t *testing.T) { @@ -124,7 +123,7 @@ func TestLDAPPrivateMethods(t *testing.T) { Config: &ServerConfig{ Groups: []*GroupToOrgRole{ { - OrgID: 1, + OrgId: 1, }, }, }, @@ -162,7 +161,7 @@ func TestLDAPPrivateMethods(t *testing.T) { Config: &ServerConfig{ Groups: []*GroupToOrgRole{ { - OrgID: 1, + OrgId: 1, }, }, }, diff --git a/pkg/services/ldap/settings.go b/pkg/services/ldap/settings.go index beba4c76c19..720180716f6 100644 --- a/pkg/services/ldap/settings.go +++ b/pkg/services/ldap/settings.go @@ -55,7 +55,7 @@ type AttributeMap struct { // config "group_mappings" setting type GroupToOrgRole struct { GroupDN string `toml:"group_dn"` - OrgID int64 `toml:"org_id"` + OrgId int64 `toml:"org_id"` // This pointer specifies if setting was set (for backwards compatibility) IsGrafanaAdmin *bool `toml:"grafana_admin"` @@ -139,8 +139,8 @@ func readConfig(configFile string) (*Config, error) { } for _, groupMap := range server.Groups { - if groupMap.OrgID == 0 { - groupMap.OrgID = 1 + if groupMap.OrgId == 0 { + groupMap.OrgId = 1 } } } diff --git a/public/app/features/admin/ldap/LdapPage.tsx b/public/app/features/admin/ldap/LdapPage.tsx index 57b865badc3..9f65470ff74 100644 --- a/public/app/features/admin/ldap/LdapPage.tsx +++ b/public/app/features/admin/ldap/LdapPage.tsx @@ -89,12 +89,12 @@ export class LdapPage extends PureComponent { {config.buildInfo.isEnterprise && ldapSyncInfo && } -

User mapping

+

Test user mapping

- +
diff --git a/public/app/features/admin/ldap/LdapUserGroups.tsx b/public/app/features/admin/ldap/LdapUserGroups.tsx index d38fbb956cc..a21cd4da876 100644 --- a/public/app/features/admin/ldap/LdapUserGroups.tsx +++ b/public/app/features/admin/ldap/LdapUserGroups.tsx @@ -9,7 +9,6 @@ interface Props { export const LdapUserGroups: FC = ({ groups, showAttributeMapping }) => { const items = showAttributeMapping ? groups : groups.filter(item => item.orgRole); - const roleColumnClass = showAttributeMapping && 'width-14'; return (
@@ -17,32 +16,39 @@ export const LdapUserGroups: FC = ({ groups, showAttributeMapping }) => { + {showAttributeMapping && } - {showAttributeMapping && } {items.map((group, index) => { return ( - - {showAttributeMapping && ( <> - + {!group.orgRole && ( + <> + + + )} + + )} + {group.orgName && ( + <> + + )} diff --git a/public/app/features/admin/ldap/LdapUserInfo.tsx b/public/app/features/admin/ldap/LdapUserInfo.tsx index 3aed200c4df..c1c3298ccb1 100644 --- a/public/app/features/admin/ldap/LdapUserInfo.tsx +++ b/public/app/features/admin/ldap/LdapUserInfo.tsx @@ -18,8 +18,21 @@ export const LdapUserInfo: FC = ({ ldapUser, showAttributeMapping }) => { {ldapUser.roles && ldapUser.roles.length > 0 && ( )} - {ldapUser.teams && ldapUser.teams.length > 0 && ( + + {ldapUser.teams && ldapUser.teams.length > 0 ? ( + ) : ( +
+
+
LDAP GroupOrganisation RoleLDAP Group
{group.orgName}{group.orgRole}{group.groupDN} - {!group.orgRole && ( - - No match - -
- -
- - - )} - 		+ + + No match + + + + + + + {group.orgName}{group.orgRole}
+ + + + + +
No teams found via LDAP
+
+ )} ); diff --git a/public/app/features/admin/ldap/LdapUserTeams.tsx b/public/app/features/admin/ldap/LdapUserTeams.tsx index 30ac81b5231..e8a87d2bfaa 100644 --- a/public/app/features/admin/ldap/LdapUserTeams.tsx +++ b/public/app/features/admin/ldap/LdapUserTeams.tsx @@ -1,5 +1,4 @@ import React, { FC } from 'react'; -import { css } from 'emotion'; import { Tooltip } from '@grafana/ui'; import { LdapTeam } from 'app/types'; @@ -10,10 +9,6 @@ interface Props { export const LdapUserTeams: FC = ({ teams, showAttributeMapping }) => { const items = showAttributeMapping ? teams : teams.filter(item => item.teamName); - const teamColumnClass = showAttributeMapping && 'width-14'; - const noMatchPlaceholderStyle = css` - display: flex; - `; return (
@@ -21,29 +16,41 @@ export const LdapUserTeams: FC = ({ teams, showAttributeMapping }) => { + {showAttributeMapping && } - {showAttributeMapping && } {items.map((team, index) => { return ( - - - {showAttributeMapping && } + {showAttributeMapping && ( + <> + + {!team.orgName && ( + <> + + + )} + + )} + {team.orgName && ( + <> + + + + )} ); })} diff --git a/public/sass/layout/_page.scss b/public/sass/layout/_page.scss index 6b2fa95730e..66b6c47356a 100644 --- a/public/sass/layout/_page.scss +++ b/public/sass/layout/_page.scss @@ -99,7 +99,7 @@ .page-heading { font-size: $font-size-h4; margin-top: 0; - margin-bottom: $spacer * 0.7; + margin-bottom: $spacer; } .page-action-bar {
LDAP GroupOrganisation TeamLDAP
- {team.orgName || ( -
- No match - -
- -
- -
- )} - 		{team.teamName}{team.groupDN}{team.groupDN} + +
+ No match + + + + + +
+ 		{team.orgName}{team.teamName}