Refactor memberOf management logic

1 files changed, 22 insertions, 94 deletions

diff --git a/write.go b/write.go
index 57c642c..24fdc1a 100644
--- a/write.go
+++ b/write.go
@@ -58,8 +58,10 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 		return ldap.LDAPResultEntryAlreadyExists, nil
 	}
 
+	// TODO: check that parent object exists
+
 	// If adding a group, track of who the members will be so that their memberOf field can be updated later
-	var members []string = nil
+	members := []string{}
 
 	// Check attributes
 	entry := Entry{}
@@ -77,8 +79,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 		}
 		// If they are writing a member key, we have to check they are adding valid members
 		if strings.EqualFold(key, ATTR_MEMBER) {
-			members = vals_str
-			for _, member := range members {
+			for _, member := range vals_str {
 				member_canonical, err := server.checkDN(member, false)
 				if err != nil {
 					return ldap.LDAPResultInvalidDNSyntax, err
@@ -93,8 +94,13 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 						member_canonical)
 				}
 			}
+			members = append(members, vals_str...)
+		}
+		if prev, ok := entry[key]; ok {
+			entry[key] = append(prev, vals_str...)
+		} else {
+			entry[key] = vals_str
 		}
-		entry[key] = vals_str
 	}
 
 	entry[ATTR_CREATORSNAME] = []string{state.login.user}
@@ -111,34 +117,8 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 	// ~~ future errors cause inconsistencies in the DB and are logged ~~
 
 	// If our item has a member list, add it to all of its member's memberOf attribute
-	if members != nil {
-		for _, member := range members {
-			memberGroups, err := server.getAttribute(member, ATTR_MEMBEROF)
-			if err != nil {
-				server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, member, err)
-				continue
-			}
-
-			alreadyMember := false
-			for _, mb := range memberGroups {
-				if mb == dn {
-					alreadyMember = true
-					server.logger.Warnf("Warning: inconsistency detected, %s was memberOf %s at a time when it didn't exist!",
-						member, dn)
-					break
-				}
-			}
-
-			if !alreadyMember {
-				memberGroups = append(memberGroups, dn)
-				err = server.addElements(member, Entry{
-					ATTR_MEMBEROF: memberGroups,
-				})
-				if err != nil {
-					server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, member, err)
-				}
-			}
-		}
+	for _, member := range members {
+		server.memberOfAdd(member, dn)
 	}
 
 	return ldap.LDAPResultSuccess, nil
@@ -196,7 +176,7 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest)
 		}
 		if itemDN != dn {
 			return ldap.LDAPResultNotAllowedOnNonLeaf, fmt.Errorf(
-				"Cannot delete %d as it has children", dn)
+				"Cannot delete %s as it has children", dn)
 		}
 	}
 
@@ -244,28 +224,8 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest)
 	}
 
 	// Delete it from all of its member's memberOf info
-	if memberList != nil {
-		for _, member := range memberList {
-			memberOf, err := server.getAttribute(member, ATTR_MEMBEROF)
-			if err != nil || memberOf == nil {
-				server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, member, err)
-				continue
-			}
-
-			newMemberOf := []string{}
-			for _, group := range memberOf {
-				if group != dn {
-					newMemberOf = append(newMemberOf, group)
-				}
-			}
-
-			err = server.addElements(member, Entry{
-				ATTR_MEMBEROF: newMemberOf,
-			})
-			if err != nil {
-				server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, member, err)
-			}
-		}
+	for _, member := range memberList {
+		server.memberOfRemove(member, dn)
 	}
 
 	return ldap.LDAPResultSuccess, nil
@@ -455,57 +415,25 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 		addMembers[i] = addMem
 	}
 
+	// Now, the modification has been processed and accepted and we want to commit it
 	newEntry[ATTR_MODIFIERSNAME] = []string{state.login.user}
 	newEntry[ATTR_MODIFYTIMESTAMP] = []string{genTimestamp()}
 
 	// Save the edited values
-	server.addElements(dn, newEntry)
+	err = server.addElements(dn, newEntry)
+	if err != nil {
+		return ldap.LDAPResultOperationsError, err
+	}
 	// ~~ from this point on, our operation succeeded ~~
 	// ~~ future errors cause inconsistencies in the DB and are logged ~~
 
 	// Update memberOf for added members and deleted members
 	for _, addMem := range addMembers {
-		memberOf, err := server.getAttribute(addMem, ATTR_MEMBEROF)
-		if err != nil {
-			server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, addMem, err)
-			continue
-		}
-
-		alreadyMember := false
-		for _, mb := range memberOf {
-			if mb == dn {
-				alreadyMember = true
-				break
-			}
-		}
-		if !alreadyMember {
-			memberOf = append(memberOf, dn)
-			err = server.addElements(addMem, Entry{
-				ATTR_MEMBEROF: memberOf,
-			})
-			if err != nil {
-				server.logger.Warnf("Could not add %s to memberOf of %s: %s", dn, addMem, err)
-			}
-		}
+		server.memberOfAdd(addMem, dn)
 	}
 
 	for _, delMem := range delMembers {
-		memberOf, err := server.getAttribute(delMem, ATTR_MEMBEROF)
-		if err != nil {
-			server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, delMem, err)
-			continue
-		}
-		newMemberOf := []string{}
-		for _, g := range memberOf {
-			if g != dn {
-				newMemberOf = append(newMemberOf, g)
-			}
-		}
-
-		err = server.addElements(delMem, Entry{ATTR_MEMBEROF: newMemberOf})
-		if err != nil {
-			server.logger.Warnf("Could not remove %s from memberOf of %s: %s", dn, delMem, err)
-		}
+		server.memberOfRemove(delMem, dn)
 	}
 
 	return ldap.LDAPResultSuccess, nil