From 9bf9bb02734e9db88249d62dd895c81f8a8c5315 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel.odegaard@gmail.com>
Date: Sat, 18 Jul 2015 09:55:21 +0200
Subject: [PATCH] fix(ldap): fixed issue with ldap group mappings to org roles
 sync, #1450

---
 pkg/login/ldap.go      | 19 ++++++++++++-------
 pkg/login/ldap_test.go | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/pkg/login/ldap.go b/pkg/login/ldap.go
index 479a599fbe6..abc3c6a587a 100644
--- a/pkg/login/ldap.go
+++ b/pkg/login/ldap.go
@@ -130,14 +130,17 @@ func (a *ldapAuther) syncOrgRoles(user *m.User, ldapUser *ldapUserInfo) error {
 		return err
 	}
 
-	// remove or update org roles
+	// update or remove org roles
 	for _, org := range orgsQuery.Result {
+		match := false
+
 		for _, group := range a.server.LdapGroups {
 			if org.OrgId != group.OrgId {
 				continue
 			}
 
 			if ldapUser.isMemberOf(group.GroupDN) {
+				match = true
 				if org.Role != group.OrgRole {
 					// update role
 					cmd := m.UpdateOrgUserCommand{OrgId: org.OrgId, UserId: user.Id, Role: group.OrgRole}
@@ -147,12 +150,14 @@ func (a *ldapAuther) syncOrgRoles(user *m.User, ldapUser *ldapUserInfo) error {
 				}
 				// ignore subsequent ldap group mapping matches
 				break
-			} else {
-				// remove role
-				cmd := m.RemoveOrgUserCommand{OrgId: org.OrgId, UserId: user.Id}
-				if err := bus.Dispatch(&cmd); err != nil {
-					return err
-				}
+			}
+		}
+
+		// remove role if no mappings match
+		if !match {
+			cmd := m.RemoveOrgUserCommand{OrgId: org.OrgId, UserId: user.Id}
+			if err := bus.Dispatch(&cmd); err != nil {
+				return err
 			}
 		}
 	}
diff --git a/pkg/login/ldap_test.go b/pkg/login/ldap_test.go
index 3d19115f878..6713d0ca3fe 100644
--- a/pkg/login/ldap_test.go
+++ b/pkg/login/ldap_test.go
@@ -139,6 +139,26 @@ func TestLdapAuther(t *testing.T) {
 			})
 		})
 
+		ldapAutherScenario("given org role is updated in config", func(sc *scenarioContext) {
+			ldapAuther := NewLdapAuthenticator(&LdapServerConf{
+				LdapGroups: []*LdapGroupToOrgRole{
+					{GroupDN: "cn=admin", OrgId: 1, OrgRole: "Admin"},
+					{GroupDN: "cn=users", OrgId: 1, OrgRole: "Viewer"},
+				},
+			})
+
+			sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_EDITOR}})
+			err := ldapAuther.syncOrgRoles(&m.User{}, &ldapUserInfo{
+				MemberOf: []string{"cn=users"},
+			})
+
+			Convey("Should update org role", func() {
+				So(err, ShouldBeNil)
+				So(sc.removeOrgUserCmd, ShouldBeNil)
+				So(sc.updateOrgUserCmd, ShouldNotBeNil)
+			})
+		})
+
 		ldapAutherScenario("given multiple matching ldap groups", func(sc *scenarioContext) {
 			ldapAuther := NewLdapAuthenticator(&LdapServerConf{
 				LdapGroups: []*LdapGroupToOrgRole{