Implement memberof with direct/indirect tracking and testcases. (#48)

* Implement memberof with direct/indirect tracking and testcases.

Cargo.toml

@@ -1,10 +1,10 @@
-cargo-features = ["default-run"]
+# cargo-features = ["default-run"]
 
 [package]
 name = "rsidm"
 version = "0.1.0"
 authors = ["William Brown <william@blackhats.net.au>"]
-default-run = "rsidm_core"
+# default-run = "rsidm_core"
 edition = "2018"
 
 
@@ -33,6 +33,7 @@ env_logger = "0.5"
 reqwest = "0.9"
 
 chrono = "0.4"
+cookie = "0.11"
 regex = "1"
 lazy_static = "1.2.0"
 

designs/memberof.rst

@@ -153,7 +153,13 @@ changes occur, we have completed the operation.
 Considerations
 --------------
 
-* Preventing recursion: As of course, we are 
+* Preventing recursion: As of course, we are using a recursive algo, it has to end. The base case
+is "is there no groups with differences" which causes us to NO-OP and return.
 
-* Replication
+* Replication; Because each server has MO, then content of the member of should be consistent. However
+what should be considered is the changelog items to ensure that the member changes are accurately
+reflected inside of the members.
+
+* Fixup: Simply apply a modify of "purged: *memberof*", and that should cause
+recalculation. (testing needed).
 

src/lib/be/mod.rs

@@ -474,7 +474,7 @@ impl BackendWriteTransaction {
         })
     }
 
-    pub fn backup(&self, audit: &mut AuditScope, dstPath: &str) -> Result<(), OperationError> {
+    pub fn backup(&self, audit: &mut AuditScope, dst_path: &str) -> Result<(), OperationError> {
         // load all entries into RAM, may need to change this later
         // if the size of the database compared to RAM is an issue
         let mut raw_entries: Vec<IdEntry> = Vec::new();
@@ -512,16 +512,16 @@ impl BackendWriteTransaction {
 
         let entries = entries?;
 
-        let mut serializedEntries = serde_json::to_string_pretty(&entries);
+        let serialized_entries = serde_json::to_string_pretty(&entries);
 
-        let serializedEntriesStr = try_audit!(
+        let serialized_entries_str = try_audit!(
             audit,
-            serializedEntries,
+            serialized_entries,
             "serde error {:?}",
             OperationError::SerdeJsonError
         );
 
-        let result = fs::write(dstPath, serializedEntriesStr);
+        let result = fs::write(dst_path, serialized_entries_str);
 
         try_audit!(
             audit,
@@ -545,28 +545,26 @@ impl BackendWriteTransaction {
         Ok(())
     }
 
-    pub fn restore(&self, audit: &mut AuditScope, srcPath: &str) -> Result<(), OperationError> {
+    pub fn restore(&self, audit: &mut AuditScope, src_path: &str) -> Result<(), OperationError> {
         // load all entries into RAM, may need to change this later
         // if the size of the database compared to RAM is an issue
-        let mut serializedStringOption = fs::read_to_string(srcPath);
+        let serialized_string_option = fs::read_to_string(src_path);
 
-        let mut serializedString = try_audit!(
+        let serialized_string = try_audit!(
             audit,
-            serializedStringOption,
+            serialized_string_option,
             "fs::read_to_string {:?}",
             OperationError::FsError
         );
 
-        unsafe {
+        try_audit!(audit, unsafe { self.purge(audit) });
-            self.purge(audit);
-        }
 
-        let entriesOption: Result<Vec<DbEntry>, serde_json::Error> =
+        let entries_option: Result<Vec<DbEntry>, serde_json::Error> =
-            serde_json::from_str(&serializedString);
+            serde_json::from_str(&serialized_string);
 
         let entries = try_audit!(
             audit,
-            entriesOption,
+            entries_option,
             "serde_json error {:?}",
             OperationError::SerdeJsonError
         );

src/lib/constants.rs

@@ -61,6 +61,7 @@ pub static UUID_SCHEMA_ATTR_MEMBEROF: &'static str = "2ff1abc8-2f64-4f41-9e3d-33
 pub static UUID_SCHEMA_ATTR_SSH_PUBLICKEY: &'static str = "52f2f13f-d35c-4cca-9f43-90a12c968f72";
 pub static UUID_SCHEMA_ATTR_PASSWORD: &'static str = "a5121082-be54-4624-a307-383839b0366b";
 pub static UUID_SCHEMA_ATTR_MEMBER: &'static str = "cbb7cb55-1d48-4b89-8da7-8d570e755b47";
+pub static UUID_SCHEMA_ATTR_DIRECTMEMBEROF: &'static str = "63f6a766-3838-48e3-bd78-0fb1152b862f";
 pub static UUID_SCHEMA_ATTR_VERSION: &'static str = "896d5095-b3ae-451e-a91f-4314165b5395";
 pub static UUID_SCHEMA_ATTR_DOMAIN: &'static str = "c9926716-eaaa-4c83-a1ab-1ed4372a7491";
 

src/lib/core.rs

@@ -250,9 +250,9 @@ pub fn create_server_core(config: Configuration) {
             // be generated (probably stored in DB for cross-host access)
             session::CookieSessionBackend::signed(&[0; 32])
                 .path("/")
-                //.max_age() duration of the token life
+                //.max_age() duration of the token life TODO make this proper!
-                // .domain()
+                .domain("localhost")
-                //.same_site() constraunt to the domain
+                .same_site(cookie::SameSite::Strict) // constrain to the domain
                 // Disallow from js
                 .http_only(true)
                 .name("rsidm-session")

src/lib/entry.rs

@@ -143,6 +143,12 @@ pub struct Entry<VALID, STATE> {
     attrs: BTreeMap<String, Vec<String>>,
 }
 
+impl<STATE> std::fmt::Display for Entry<EntryValid, STATE> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "{}", self.get_uuid())
+    }
+}
+
 impl Entry<EntryInvalid, EntryNew> {
     #[cfg(test)]
     pub fn new() -> Self {
@@ -475,6 +481,7 @@ impl<STATE> Entry<EntryValid, STATE> {
         }
     }
 
+    /*
     pub fn seal(self) -> Entry<EntryValid, EntryCommitted> {
         Entry {
             valid: self.valid,
@@ -484,6 +491,16 @@ impl<STATE> Entry<EntryValid, STATE> {
             attrs: self.attrs,
         }
     }
+    */
+
+    pub fn get_uuid(&self) -> &String {
+        // TODO: Make this not unwrap!!!
+        self.attrs
+            .get("uuid")
+            .expect("UUID ATTR NOT PRESENT, INVALID ENTRY STATE!!!")
+            .first()
+            .expect("UUID VALUE NOT PRESENT, INVALID ENTRY STATE!!!")
+    }
 
     // Assert if this filter matches the entry (no index)
     pub fn entry_match_no_index(&self, filter: &Filter<FilterValid>) -> bool {
@@ -612,6 +629,7 @@ impl<STATE> Entry<EntryValid, STATE> {
 }
 
 impl<VALID, STATE> Entry<VALID, STATE> {
+    /* WARNING: Should these TODO move to EntryValid only? */
     pub fn get_ava(&self, attr: &String) -> Option<&Vec<String>> {
         self.attrs.get(attr)
     }
@@ -621,6 +639,16 @@ impl<VALID, STATE> Entry<VALID, STATE> {
         self.attrs.contains_key(attr)
     }
 
+    pub fn attribute_value_pres(&self, attr: &str, value: &str) -> bool {
+        match self.attrs.get(attr) {
+            Some(v_list) => match v_list.binary_search(&value.to_string()) {
+                Ok(_) => true,
+                Err(_) => false,
+            },
+            None => false,
+        }
+    }
+
     pub fn attribute_equality(&self, attr: &str, value: &str) -> bool {
         // we assume based on schema normalisation on the way in
         // that the equality here of the raw values MUST be correct.
@@ -725,31 +753,19 @@ where
     }
 
     // Should this be schemaless, relying on checks of the modlist, and the entry validate after?
-    pub fn apply_modlist(
+    pub fn apply_modlist(&mut self, modlist: &ModifyList<ModifyValid>) {
-        &self,
+        // -> Result<Entry<EntryInvalid, STATE>, OperationError> {
-        modlist: &ModifyList<ModifyValid>,
-    ) -> Result<Entry<EntryInvalid, STATE>, OperationError> {
         // Apply a modlist, generating a new entry that conforms to the changes.
         // This is effectively clone-and-transform
 
-        // clone the entry
-        let mut ne: Entry<EntryInvalid, STATE> = Entry {
-            valid: self.valid,
-            state: self.state,
-            attrs: self.attrs.clone(),
-        };
-
         // mutate
         for modify in modlist {
             match modify {
-                Modify::Present(a, v) => ne.add_ava(a.clone(), v.clone()),
+                Modify::Present(a, v) => self.add_ava(a.clone(), v.clone()),
-                Modify::Removed(a, v) => ne.remove_ava(a, v),
+                Modify::Removed(a, v) => self.remove_ava(a, v),
-                Modify::Purged(a) => ne.purge_ava(a),
+                Modify::Purged(a) => self.purge_ava(a),
             }
         }
-
-        // return it
-        Ok(ne)
     }
 }
 
@@ -798,7 +814,7 @@ struct User {
 mod tests {
     use crate::entry::{Entry, EntryInvalid, EntryNew};
     use crate::modify::{Modify, ModifyList};
-    use serde_json;
+    // use serde_json;
 
     #[test]
     fn test_entry_basic() {
@@ -859,10 +875,10 @@ mod tests {
             )])
         };
 
-        let ne = e.apply_modlist(&mods).expect("Failed to apply modlist");
+        e.apply_modlist(&mods);
 
         // Assert the changes are there
-        assert!(ne.attribute_equality("attr", "value"));
+        assert!(e.attribute_equality("attr", "value"));
 
         // Assert present for multivalue
         // Assert purge on single/multi/empty value

src/lib/error.rs

@@ -43,4 +43,5 @@ pub enum ConsistencyError {
     UuidIndexCorrupt(String),
     UuidNotUnique(String),
     RefintNotUpheld(u64),
+    MemberOfInvalid(u64),
 }

src/lib/lib.rs

@@ -12,6 +12,7 @@ extern crate uuid;
 
 extern crate bytes;
 extern crate chrono;
+extern crate cookie;
 extern crate env_logger;
 
 extern crate regex;

src/lib/plugins/base.rs

@@ -243,12 +243,12 @@ impl Plugin for Base {
 
 #[cfg(test)]
 mod tests {
-    #[macro_use]
+    // #[macro_use]
-    use crate::plugins::Plugin;
+    // use crate::plugins::Plugin;
     use crate::entry::{Entry, EntryInvalid, EntryNew};
     use crate::error::OperationError;
     use crate::filter::Filter;
-    use crate::modify::{Modify, ModifyInvalid, ModifyList};
+    use crate::modify::{Modify, ModifyList};
     use crate::server::QueryServerReadTransaction;
     use crate::server::QueryServerWriteTransaction;
 

src/lib/plugins/macros.rs

@@ -60,9 +60,14 @@ macro_rules! run_create_test {
                 let r = qs_write.create(&mut au_test, &ce);
                 assert!(r == $expect);
                 $check(&mut au_test, &qs_write);
-                r.map(|_| {
+                match r {
-                    assert!(qs_write.commit(&mut au_test).is_ok());
+                    Ok(_) => {
-                });
+                        qs_write.commit(&mut au_test).expect("commit failure!");
+                    }
+                    Err(e) => {
+                        audit_log!(&mut au_test, "Rolling back => {:?}", e);
+                    }
+                }
             }
             // Make sure there are no errors.
             assert!(qs.verify(&mut au_test).len() == 0);
@@ -107,9 +112,14 @@ macro_rules! run_modify_test {
                 let r = qs_write.modify(&mut au_test, &me);
                 $check(&mut au_test, &qs_write);
                 assert!(r == $expect);
-                r.map(|_| {
+                match r {
-                    assert!(qs_write.commit(&mut au_test).is_ok());
+                    Ok(_) => {
-                });
+                        qs_write.commit(&mut au_test).expect("commit failure!");
+                    }
+                    Err(e) => {
+                        audit_log!(&mut au_test, "Rolling back => {:?}", e);
+                    }
+                }
             }
             // Make sure there are no errors.
             assert!(qs.verify(&mut au_test).len() == 0);
@@ -153,9 +163,14 @@ macro_rules! run_delete_test {
                 let r = qs_write.delete(&mut au_test, &de);
                 $check(&mut au_test, &qs_write);
                 assert!(r == $expect);
-                r.map(|_| {
+                match r {
-                    assert!(qs_write.commit(&mut au_test).is_ok());
+                    Ok(_) => {
-                });
+                        qs_write.commit(&mut au_test).expect("commit failure!");
+                    }
+                    Err(e) => {
+                        audit_log!(&mut au_test, "Rolling back => {:?}", e);
+                    }
+                }
             }
             // Make sure there are no errors.
             assert!(qs.verify(&mut au_test).len() == 0);

src/lib/plugins/mod.rs

@@ -10,6 +10,7 @@ mod macros;
 
 mod base;
 mod failure;
+mod memberof;
 mod protected;
 mod recycle;
 mod refint;
@@ -64,6 +65,7 @@ trait Plugin {
         _au: &mut AuditScope,
         _qs: &QueryServerWriteTransaction,
         // List of what we modified that was valid?
+        _pre_cand: &Vec<Entry<EntryValid, EntryCommitted>>,
         _cand: &Vec<Entry<EntryValid, EntryCommitted>>,
         _ce: &ModifyEvent,
         _modlist: &ModifyList<ModifyValid>,
@@ -190,6 +192,7 @@ macro_rules! run_post_modify_plugin {
     (
         $au:ident,
         $qs:ident,
+        $pre_cand:ident,
         $cand:ident,
         $ce:ident,
         $ml:ident,
@@ -199,6 +202,7 @@ macro_rules! run_post_modify_plugin {
         let r = audit_segment!(audit_scope, || <($target_plugin)>::post_modify(
             &mut audit_scope,
             $qs,
+            $pre_cand,
             $cand,
             $ce,
             $ml
@@ -308,6 +312,7 @@ impl Plugins {
         audit_segment!(au, || {
             let res = run_post_create_plugin!(au, qs, cand, ce, base::Base).and_then(|_| {
                 run_post_create_plugin!(au, qs, cand, ce, refint::ReferentialIntegrity)
+                    .and_then(|_| run_post_create_plugin!(au, qs, cand, ce, memberof::MemberOf))
             });
 
             res
@@ -334,14 +339,34 @@ impl Plugins {
     pub fn run_post_modify(
         au: &mut AuditScope,
         qs: &QueryServerWriteTransaction,
+        pre_cand: &Vec<Entry<EntryValid, EntryCommitted>>,
         cand: &Vec<Entry<EntryValid, EntryCommitted>>,
         me: &ModifyEvent,
         modlist: &ModifyList<ModifyValid>,
     ) -> Result<(), OperationError> {
         audit_segment!(au, || {
-            let res =
+            let res = run_post_modify_plugin!(au, qs, pre_cand, cand, me, modlist, base::Base)
-                run_post_modify_plugin!(au, qs, cand, me, modlist, base::Base).and_then(|_| {
+                .and_then(|_| {
-                    run_post_modify_plugin!(au, qs, cand, me, modlist, refint::ReferentialIntegrity)
+                    run_post_modify_plugin!(
+                        au,
+                        qs,
+                        pre_cand,
+                        cand,
+                        me,
+                        modlist,
+                        refint::ReferentialIntegrity
+                    )
+                    .and_then(|_| {
+                        run_post_modify_plugin!(
+                            au,
+                            qs,
+                            pre_cand,
+                            cand,
+                            me,
+                            modlist,
+                            memberof::MemberOf
+                        )
+                    })
                 });
 
             res
@@ -372,6 +397,7 @@ impl Plugins {
         audit_segment!(au, || {
             let res = run_post_delete_plugin!(au, qs, cand, de, base::Base).and_then(|_| {
                 run_post_delete_plugin!(au, qs, cand, de, refint::ReferentialIntegrity)
+                    .and_then(|_| run_post_delete_plugin!(au, qs, cand, de, memberof::MemberOf))
             });
 
             res
@@ -385,6 +411,7 @@ impl Plugins {
         let mut results = Vec::new();
         run_verify_plugin!(au, qs, &mut results, base::Base);
         run_verify_plugin!(au, qs, &mut results, refint::ReferentialIntegrity);
+        run_verify_plugin!(au, qs, &mut results, memberof::MemberOf);
         results
     }
 }

src/lib/plugins/refint.rs

@@ -12,7 +12,7 @@
 use std::collections::HashMap;
 
 use crate::audit::AuditScope;
-use crate::entry::{Entry, EntryCommitted, EntryInvalid, EntryNew, EntryValid};
+use crate::entry::{Entry, EntryCommitted, EntryNew, EntryValid};
 use crate::error::{ConsistencyError, OperationError};
 use crate::event::{CreateEvent, DeleteEvent, ModifyEvent};
 use crate::filter::{Filter, FilterInvalid};
@@ -30,12 +30,13 @@ impl ReferentialIntegrity {
     fn check_uuid_exists(
         au: &mut AuditScope,
         qs: &QueryServerWriteTransaction,
+        rtype: &String,
         uuid: &String,
     ) -> Result<(), OperationError> {
         let mut au_qs = AuditScope::new("qs_exist");
         let filt_in: Filter<FilterInvalid> =
             Filter::new_ignore_hidden(Filter::Eq("uuid".to_string(), uuid.clone()));
-        let r = qs.internal_exists(au, filt_in);
+        let r = qs.internal_exists(&mut au_qs, filt_in);
         au.append_scope(au_qs);
 
         let b = try_audit!(au, r);
@@ -43,6 +44,12 @@ impl ReferentialIntegrity {
         if b {
             Ok(())
         } else {
+            audit_log!(
+                au,
+                "{:?}:{:?} UUID reference not found in database",
+                rtype,
+                uuid
+            );
             Err(OperationError::Plugin)
         }
     }
@@ -85,7 +92,7 @@ impl Plugin for ReferentialIntegrity {
                     Some(vs) => {
                         // For each value in the set.
                         for v in vs {
-                            Self::check_uuid_exists(au, qs, v)?
+                            Self::check_uuid_exists(au, qs, &rtype.name, v)?
                         }
                     }
                     None => {}
@@ -98,8 +105,9 @@ impl Plugin for ReferentialIntegrity {
     fn post_modify(
         au: &mut AuditScope,
         qs: &QueryServerWriteTransaction,
+        _pre_cand: &Vec<Entry<EntryValid, EntryCommitted>>,
         _cand: &Vec<Entry<EntryValid, EntryCommitted>>,
-        me: &ModifyEvent,
+        _me: &ModifyEvent,
         modlist: &ModifyList<ModifyValid>,
     ) -> Result<(), OperationError> {
         let schema = qs.get_schema();
@@ -113,7 +121,7 @@ impl Plugin for ReferentialIntegrity {
                     match ref_types.get(a) {
                         Some(a_type) => {
                             // So it is a reference type, now check it.
-                            Self::check_uuid_exists(au, qs, v)?
+                            Self::check_uuid_exists(au, qs, &a_type.name, v)?
                         }
                         None => {}
                     }
@@ -148,7 +156,8 @@ impl Plugin for ReferentialIntegrity {
             .collect();
 
         // Generate a filter which is the set of all schema reference types
-        // as EQ to all uuid of all entries in delete.
+        // as EQ to all uuid of all entries in delete. - this INCLUDES recycled
+        // types too!
         let filt: Filter<FilterInvalid> = Filter::Or(
             uuids
                 .iter()
@@ -256,8 +265,8 @@ impl Plugin for ReferentialIntegrity {
 
 #[cfg(test)]
 mod tests {
-    #[macro_use]
+    // #[macro_use]
-    use crate::plugins::Plugin;
+    // use crate::plugins::Plugin;
     use crate::entry::{Entry, EntryInvalid, EntryNew};
     use crate::error::OperationError;
     use crate::filter::Filter;
@@ -338,7 +347,7 @@ mod tests {
                         Filter::Eq("name".to_string(), "testgroup_b".to_string()),
                     )
                     .expect("Internal search failure");
-                let ue = cands.first().expect("No cand");
+                let _ue = cands.first().expect("No cand");
             }
         );
     }
@@ -374,7 +383,7 @@ mod tests {
                 let cands = qs
                     .internal_search(au, Filter::Eq("name".to_string(), "testgroup".to_string()))
                     .expect("Internal search failure");
-                let ue = cands.first().expect("No cand");
+                let _ue = cands.first().expect("No cand");
             }
         );
     }
@@ -615,7 +624,7 @@ mod tests {
             preload,
             Filter::Eq("name".to_string(), "testgroup_a".to_string()),
             false,
-            |au: &mut AuditScope, qs: &QueryServerWriteTransaction| {}
+            |_au: &mut AuditScope, _qs: &QueryServerWriteTransaction| {}
         );
     }
 
@@ -663,7 +672,7 @@ mod tests {
             preload,
             Filter::Eq("name".to_string(), "testgroup_b".to_string()),
             false,
-            |au: &mut AuditScope, qs: &QueryServerWriteTransaction| {}
+            |_au: &mut AuditScope, _qs: &QueryServerWriteTransaction| {}
         );
     }
 
@@ -692,7 +701,7 @@ mod tests {
             preload,
             Filter::Eq("name".to_string(), "testgroup_b".to_string()),
             false,
-            |au: &mut AuditScope, qs: &QueryServerWriteTransaction| {}
+            |_au: &mut AuditScope, _qs: &QueryServerWriteTransaction| {}
         );
     }
 }

src/lib/proto_v1_actors.rs

@@ -6,8 +6,8 @@ use crate::be::Backend;
 
 use crate::error::OperationError;
 use crate::event::{
-    CreateEvent, DeleteEvent, ModifyEvent, OpResult, PurgeRecycledEvent, PurgeTombstoneEvent,
+    CreateEvent, DeleteEvent, ModifyEvent, PurgeRecycledEvent, PurgeTombstoneEvent, SearchEvent,
-    SearchEvent, SearchResult,
+    SearchResult,
 };
 use crate::log::EventLog;
 use crate::schema::{Schema, SchemaReadTransaction};
@@ -275,7 +275,7 @@ impl Handler<PurgeTombstoneEvent> for QueryServerV1 {
 
             let res = qs_write
                 .purge_tombstones(&mut audit)
-                .map(|_| qs_write.commit(&mut audit).map(|_| OpResult {}));
+                .and_then(|_| qs_write.commit(&mut audit));
             audit_log!(audit, "Purge tombstones result: {:?}", res);
             res.expect("Invalid Server State");
         });
@@ -296,7 +296,7 @@ impl Handler<PurgeRecycledEvent> for QueryServerV1 {
 
             let res = qs_write
                 .purge_recycled(&mut audit)
-                .map(|_| qs_write.commit(&mut audit).map(|_| OpResult {}));
+                .and_then(|_| qs_write.commit(&mut audit));
             audit_log!(audit, "Purge recycled result: {:?}", res);
             res.expect("Invalid Server State");
         });

src/lib/schema.rs

@@ -733,6 +733,20 @@ impl SchemaInner {
                     syntax: SyntaxType::REFERENCE_UUID,
                 },
             );
+            self.attributes.insert(
+                String::from("directmemberof"),
+                SchemaAttribute {
+                    name: String::from("directmemberof"),
+                    uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_DIRECTMEMBEROF)
+                        .expect("unable to parse static uuid"),
+                    description: String::from("reverse direct group membership of the object"),
+                    system: true,
+                    secret: false,
+                    multivalue: true,
+                    index: vec![IndexType::EQUALITY],
+                    syntax: SyntaxType::REFERENCE_UUID,
+                },
+            );
             // ssh_publickey // multi
             self.attributes.insert(
                 String::from("ssh_publickey"),

src/lib/server.rs

@@ -609,12 +609,14 @@ impl<'a> QueryServerWriteTransaction<'a> {
             Err(e) => return Err(OperationError::SchemaViolation(e)),
         };
 
-        let mut candidates: Result<Vec<Entry<EntryInvalid, EntryCommitted>>, _> = pre_candidates
+        let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
-            .into_iter()
+            .iter()
-            .map(|er| er.invalidate().apply_modlist(&modlist))
+            .map(|er| er.clone().invalidate())
             .collect();
 
-        let mut candidates = try_audit!(au, candidates);
+        candidates
+            .iter_mut()
+            .for_each(|er| er.apply_modlist(&modlist));
 
         audit_log!(au, "delete: candidates -> {:?}", candidates);
 
@@ -807,12 +809,16 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // Clone a set of writeables.
         // Apply the modlist -> Remember, we have a set of origs
         // and the new modified ents.
-        let mut candidates: Result<Vec<Entry<EntryInvalid, EntryCommitted>>, _> = pre_candidates
+        let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
-            .into_iter()
+            .iter()
-            .map(|er| er.invalidate().apply_modlist(&modlist))
+            .map(|er| er.clone().invalidate())
             .collect();
 
-        let mut candidates = try_audit!(au, candidates);
+        candidates
+            .iter_mut()
+            .for_each(|er| er.apply_modlist(&modlist));
+
+        // let mut candidates = try_audit!(au, candidates);
 
         audit_log!(au, "modify: candidates -> {:?}", candidates);
 
@@ -833,6 +839,9 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // optimisations, this could be premature - so we for now, just
         // do the CORRECT thing and recommit as we may find later we always
         // want to add CSN's or other.
+        //
+        // memberOf actually wants the pre cand list and the norm_cand list to see what
+        // changed. Could be optimised, but this is correct still ...
 
         let res: Result<Vec<Entry<EntryValid, EntryCommitted>>, SchemaError> = candidates
             .into_iter()
@@ -860,8 +869,14 @@ impl<'a> QueryServerWriteTransaction<'a> {
 
         // Post Plugins
         let mut audit_plugin_post = AuditScope::new("plugin_post_modify");
-        let plug_post_res =
+        let plug_post_res = Plugins::run_post_modify(
-            Plugins::run_post_modify(&mut audit_plugin_post, &self, &norm_cand, me, &modlist);
+            &mut audit_plugin_post,
+            &self,
+            &pre_candidates,
+            &norm_cand,
+            me,
+            &modlist,
+        );
         au.append_scope(audit_plugin_post);
 
         if plug_post_res.is_err() {
@@ -1127,9 +1142,7 @@ mod tests {
     use crate::proto_v1::Filter as ProtoFilter;
     use crate::proto_v1::Modify as ProtoModify;
     use crate::proto_v1::ModifyList as ProtoModifyList;
-    use crate::proto_v1::{
+    use crate::proto_v1::{DeleteRequest, ModifyRequest, ReviveRecycledRequest};
-        DeleteRequest, ModifyRequest, ReviveRecycledRequest, SearchRecycledRequest, SearchRequest,
-    };
     use crate::schema::Schema;
     use crate::server::{QueryServer, QueryServerReadTransaction};
 

src/server/main.rs

@@ -1,6 +1,8 @@
 extern crate actix;
 
 extern crate rsidm;
+
+
 use rsidm::config::Configuration;
 use rsidm::core::create_server_core;
 

tests/proto_v1_test.rs

@@ -4,13 +4,13 @@ use actix::prelude::*;
 extern crate rsidm;
 use rsidm::config::Configuration;
 use rsidm::core::create_server_core;
-use rsidm::proto_v1::{CreateRequest, Entry, OperationResponse, SearchRequest, SearchResponse};
+use rsidm::proto_v1::{CreateRequest, Entry, OperationResponse};
 
 extern crate reqwest;
 
 extern crate futures;
-use futures::future;
+// use futures::future;
-use futures::future::Future;
+// use futures::future::Future;
 
 use std::sync::mpsc;
 use std::thread;