fix(ldap): fixed issue with ldap group mappings to org roles sync, #1450

pkg/login/ldap.go

@@ -130,14 +130,17 @@ func (a *ldapAuther) syncOrgRoles(user *m.User, ldapUser *ldapUserInfo) error {
 		return err
 		return err
 	}
 	}
 
 
-	// remove or update org roles
+	// update or remove org roles
 	for _, org := range orgsQuery.Result {
 	for _, org := range orgsQuery.Result {
+		match := false
+
 		for _, group := range a.server.LdapGroups {
 		for _, group := range a.server.LdapGroups {
 			if org.OrgId != group.OrgId {
 			if org.OrgId != group.OrgId {
 				continue
 				continue
 			}
 			}
 
 
 			if ldapUser.isMemberOf(group.GroupDN) {
 			if ldapUser.isMemberOf(group.GroupDN) {
+				match = true
 				if org.Role != group.OrgRole {
 				if org.Role != group.OrgRole {
 					// update role
 					// update role
 					cmd := m.UpdateOrgUserCommand{OrgId: org.OrgId, UserId: user.Id, Role: group.OrgRole}
 					cmd := m.UpdateOrgUserCommand{OrgId: org.OrgId, UserId: user.Id, Role: group.OrgRole}
@@ -147,15 +150,17 @@ func (a *ldapAuther) syncOrgRoles(user *m.User, ldapUser *ldapUserInfo) error {
 				}
 				}
 				// ignore subsequent ldap group mapping matches
 				// ignore subsequent ldap group mapping matches
 				break
 				break
-			} else {
-				// remove role
+			}
+		}
+
+		// remove role if no mappings match
+		if !match {
 			cmd := m.RemoveOrgUserCommand{OrgId: org.OrgId, UserId: user.Id}
 			cmd := m.RemoveOrgUserCommand{OrgId: org.OrgId, UserId: user.Id}
 			if err := bus.Dispatch(&cmd); err != nil {
 			if err := bus.Dispatch(&cmd); err != nil {
 				return err
 				return err
 			}
 			}
 		}
 		}
 	}
 	}
-	}
 
 
 	// add missing org roles
 	// add missing org roles
 	for _, group := range a.server.LdapGroups {
 	for _, group := range a.server.LdapGroups {

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) {
 		ldapAutherScenario("given multiple matching ldap groups", func(sc *scenarioContext) {
 			ldapAuther := NewLdapAuthenticator(&LdapServerConf{
 			ldapAuther := NewLdapAuthenticator(&LdapServerConf{
 				LdapGroups: []*LdapGroupToOrgRole{
 				LdapGroups: []*LdapGroupToOrgRole{