Fix missing procedure for delete membership & "better" failure handling

After an object has been updated, membership information must be
propagated to other object. Such operations may fail when calling consul
but if they do we don't return fail immediatly returning an error code
any more.  Instead we just print all the errors to our logs and try to
process the remaining updates.

1 files changed, 72 insertions, 20 deletions

diff --git a/write.go b/write.go
index 33779c4..f86423a 100644
--- a/write.go
+++ b/write.go
@@ -60,11 +60,10 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 		return ldap.LDAPResultEntryAlreadyExists, nil
 	}
 
-	// Add object
-
 	// If adding a group, track of who the members will be so that their memberOf field can be updated later
 	var members []string = nil
 
+	// Check attributes
 	entry := Entry{}
 	for _, attribute := range r.Attributes() {
 		key := string(attribute.Type_())
@@ -79,7 +78,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 			return ldap.LDAPResultObjectClassViolation, err
 		}
 		// If they are writing a member key, we have to check they are adding valid members
-		if strings.EqualFold(key, "member") {
+		if strings.EqualFold(key, ATTR_MEMBER) {
 			members = vals_str
 			for _, member := range members {
 				_, err := server.checkSuffix(member, false)
@@ -105,16 +104,21 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 	entry[ATTR_ENTRYUUID] = []string{genUuid()}
 	entry[dnSplit[0].Type] = []string{dnSplit[0].Value}
 
+	// Add our intem in the DB
 	err = server.addElements(dn, entry)
 	if err != nil {
 		return ldap.LDAPResultOperationsError, err
 	}
+	// ~~ from this point on, our operation succeeded ~~
+	// ~~ 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 {
-				return ldap.LDAPResultOperationsError, err
+				server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, member, err)
+				continue
 			}
 			if memberGroups == nil {
 				memberGroups = []string{}
@@ -136,7 +140,7 @@ func (server *Server) handleAddInternal(state *State, r *message.AddRequest) (in
 					ATTR_MEMBEROF: memberGroups,
 				})
 				if err != nil {
-					return ldap.LDAPResultOperationsError, err
+					server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, member, err)
 				}
 			}
 		}
@@ -209,19 +213,26 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest)
 	if err != nil {
 		return ldap.LDAPResultOperationsError, err
 	}
+	memberList, err := server.getAttribute(dn, ATTR_MEMBER)
+	if err != nil {
+		return ldap.LDAPResultOperationsError, err
+	}
 
 	// Delete the LDAP entry
 	_, err = server.kv.DeleteTree(path+"/", nil)
 	if err != nil {
 		return ldap.LDAPResultOperationsError, err
 	}
+	// ~~ from this point on, our operation succeeded ~~
+	// ~~ future errors cause inconsistencies in the DB and are logged ~~
 
 	// Delete it from the member list of all the groups it was a member of
 	if memberOf != nil {
 		for _, group := range memberOf {
-			groupMembers, err := server.getAttribute(dn, "member")
+			groupMembers, err := server.getAttribute(group, ATTR_MEMBER)
 			if err != nil {
-				return ldap.LDAPResultOperationsError, err
+				server.logger.Printf("Could not remove %s from members of %s: %s", dn, group, err)
+				continue
 			}
 
 			newMembers := []string{}
@@ -232,10 +243,35 @@ func (server *Server) handleDeleteInternal(state *State, r *message.DelRequest)
 			}
 
 			err = server.addElements(group, Entry{
-				"member": newMembers,
+				ATTR_MEMBER: newMembers,
 			})
 			if err != nil {
-				return ldap.LDAPResultOperationsError, err
+				server.logger.Printf("Could not remove %s from members of %s: %s", dn, group, err)
+			}
+		}
+	}
+
+	// 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.Printf("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.Printf("Could not remove %s from memberOf of %s: %s", dn, member, err)
 			}
 		}
 	}
@@ -347,7 +383,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 				}
 				if !present {
 					newEntry[attr] = append(newEntry[attr], string(val))
-					if strings.EqualFold(attr, "member") {
+					if strings.EqualFold(attr, ATTR_MEMBER) {
 						addMembers = append(addMembers, string(val))
 					}
 				}
@@ -356,7 +392,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 			if len(values) == 0 {
 				// Delete everything
 				newEntry[attr] = []string{}
-				if strings.EqualFold(attr, "member") {
+				if strings.EqualFold(attr, ATTR_MEMBER) {
 					delMembers = append(delMembers, prevEntry[attr]...)
 				}
 			} else {
@@ -373,7 +409,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 					if keep {
 						newEntry[attr] = append(newEntry[attr], prevVal)
 					} else {
-						if strings.EqualFold(attr, "member") {
+						if strings.EqualFold(attr, ATTR_MEMBER) {
 							delMembers = append(delMembers, prevVal)
 						}
 					}
@@ -384,7 +420,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 			for _, newVal := range values {
 				newEntry[attr] = append(newEntry[attr], string(newVal))
 			}
-			if strings.EqualFold(attr, "member") {
+			if strings.EqualFold(attr, ATTR_MEMBER) {
 				for _, newMem := range newEntry[attr] {
 					mustAdd := true
 					for _, prevMem := range prevEntry[attr] {
@@ -430,27 +466,43 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 
 	// Save the edited values
 	server.addElements(dn, newEntry)
+	// ~~ 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 {
-			return ldap.LDAPResultOperationsError, err
+			server.logger.Printf("Could not add %s to memberOf of %s: %s", dn, addMem, err)
+			continue
 		}
 		if memberOf == nil {
 			memberOf = []string{}
 		}
-		memberOf = append(memberOf, dn)
-		err = server.addElements(addMem, Entry{ATTR_MEMBEROF: memberOf})
-		if err != nil {
-			return ldap.LDAPResultOperationsError, err
+
+		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.Printf("Could not add %s to memberOf of %s: %s", dn, addMem, err)
+			}
 		}
 	}
 
 	for _, delMem := range delMembers {
 		memberOf, err := server.getAttribute(delMem, ATTR_MEMBEROF)
 		if err != nil {
-			return ldap.LDAPResultOperationsError, err
+			server.logger.Printf("Could not remove %s from memberOf of %s: %s", dn, delMem, err)
+			continue
 		}
 		if memberOf == nil {
 			memberOf = []string{}
@@ -464,7 +516,7 @@ func (server *Server) handleModifyInternal(state *State, r *message.ModifyReques
 
 		err = server.addElements(delMem, Entry{ATTR_MEMBEROF: newMemberOf})
 		if err != nil {
-			return ldap.LDAPResultOperationsError, err
+			server.logger.Printf("Could not remove %s from memberOf of %s: %s", dn, delMem, err)
 		}
 	}