From 658e409d9080ce6ef48775fd4e5834aad5a903be Mon Sep 17 00:00:00 2001 From: William Brown Date: Tue, 12 Mar 2019 15:20:08 +1000 Subject: [PATCH] Most changes for state validated modlist done, just need to write the validate! --- src/lib/entry.rs | 18 ++++++----- src/lib/event.rs | 8 ++--- src/lib/modify.rs | 76 ++++++++++++++++++++++++++++++++++++++++------- src/lib/server.rs | 21 +++++++++---- 4 files changed, 95 insertions(+), 28 deletions(-) diff --git a/src/lib/entry.rs b/src/lib/entry.rs index f103c9457..1bddce521 100644 --- a/src/lib/entry.rs +++ b/src/lib/entry.rs @@ -2,7 +2,7 @@ use super::proto_v1::Entry as ProtoEntry; use error::SchemaError; use filter::{Filter, FilterValid}; -use modify::{Modify, ModifyList}; +use modify::{Modify, ModifyList, ModifyValid, ModifyInvalid}; use schema::{SchemaAttribute, SchemaClass, SchemaReadTransaction}; use std::collections::btree_map::{Iter as BTreeIter, IterMut as BTreeIterMut}; use std::collections::BTreeMap; @@ -503,7 +503,7 @@ impl Entry { pub fn gen_modlist_assert( &self, schema: &SchemaReadTransaction, - ) -> Result { + ) -> Result, SchemaError> { // Create a modlist from this entry. We make this assuming we want the entry // to have this one as a subset of values. This means if we have single // values, we'll replace, if they are multivalue, we present them. @@ -645,7 +645,7 @@ where } // Should this be schemaless, relying on checks of the modlist, and the entry validate after? - pub fn apply_modlist(&self, modlist: &ModifyList) -> Result, ()> { + pub fn apply_modlist(&self, modlist: &ModifyList) -> Result, ()> { // Apply a modlist, generating a new entry that conforms to the changes. // This is effectively clone-and-transform @@ -658,7 +658,7 @@ where }; // mutate - for modify in modlist.mods.iter() { + for modify in modlist { match modify { Modify::Present(a, v) => ne.add_ava(a.clone(), v.clone()), Modify::Removed(a, v) => ne.remove_ava(a, v), @@ -770,10 +770,12 @@ mod tests { let mut e: Entry = Entry::new(); e.add_ava(String::from("userid"), String::from("william")); - let mods = ModifyList::new_list(vec![Modify::Present( - String::from("attr"), - String::from("value"), - )]); + let mods = unsafe { + ModifyList::new_valid_list(vec![Modify::Present( + String::from("attr"), + String::from("value"), + )]) + }; let ne = e.apply_modlist(&mods).unwrap(); diff --git a/src/lib/event.rs b/src/lib/event.rs index 4aa7391aa..9e0ad3025 100644 --- a/src/lib/event.rs +++ b/src/lib/event.rs @@ -6,7 +6,7 @@ use super::proto_v1::{ }; use entry::{Entry, EntryCommitted, EntryInvalid, EntryNew, EntryValid}; // use error::OperationError; -use modify::ModifyList; +use modify::{ModifyList, ModifyInvalid}; use actix::prelude::*; @@ -189,7 +189,7 @@ impl DeleteEvent { #[derive(Debug)] pub struct ModifyEvent { pub filter: Filter, - pub modlist: ModifyList, + pub modlist: ModifyList, pub internal: bool, } @@ -203,7 +203,7 @@ impl ModifyEvent { } #[cfg(test)] - pub fn from_filter(filter: Filter, modlist: ModifyList) -> Self { + pub fn from_filter(filter: Filter, modlist: ModifyList) -> Self { ModifyEvent { filter: filter, modlist: modlist, @@ -211,7 +211,7 @@ impl ModifyEvent { } } - pub fn new_internal(filter: Filter, modlist: ModifyList) -> Self { + pub fn new_internal(filter: Filter, modlist: ModifyList) -> Self { ModifyEvent { filter: filter, modlist: modlist, diff --git a/src/lib/modify.rs b/src/lib/modify.rs index 4cbb1e906..940eaed32 100644 --- a/src/lib/modify.rs +++ b/src/lib/modify.rs @@ -1,6 +1,17 @@ use proto_v1::Modify as ProtoModify; use proto_v1::ModifyList as ProtoModifyList; +use error::SchemaError; +use schema::{SchemaAttribute, SchemaReadTransaction}; + +// Should this be std? +use std::slice; + +#[derive(Serialize, Deserialize, Debug)] +pub struct ModifyValid; +#[derive(Serialize, Deserialize, Debug)] +pub struct ModifyInvalid; + #[derive(Serialize, Deserialize, Debug)] pub enum Modify { // This value *should* exist. @@ -22,36 +33,79 @@ impl Modify { } #[derive(Serialize, Deserialize, Debug)] -pub struct ModifyList { - // And ordered list of changes to apply. Should this be state based? - pub mods: Vec, +pub struct ModifyList { + valid: VALID, + // The order of this list matters. Each change must be done in order. + mods: Vec, } // TODO: ModifyList should be like filter and have valid/invalid to schema. // Or do we not care because the entry will be invalid at the end? -impl ModifyList { +impl<'a> IntoIterator for &'a ModifyList { + type Item = &'a Modify; + type IntoIter = slice::Iter<'a, Modify>; + + fn into_iter(self) -> Self::IntoIter { + self.mods.iter() + } +} + + +impl ModifyList { pub fn new() -> Self { - ModifyList { mods: Vec::new() } + ModifyList { + valid: ModifyInvalid, + mods: Vec::new() + } } pub fn new_list(mods: Vec) -> Self { - ModifyList { mods: mods } + ModifyList { + valid: ModifyInvalid, + mods: mods + } } pub fn push_mod(&mut self, modify: Modify) { self.mods.push(modify) } - pub fn len(&self) -> usize { - self.mods.len() - } - pub fn from(ml: &ProtoModifyList) -> Self { // For each ProtoModify, do a from. - ModifyList { + valid: ModifyInvalid, mods: ml.mods.iter().map(|pm| Modify::from(pm)).collect(), } } + + pub fn validate(&self, + schema: &SchemaReadTransaction, + ) -> Result, SchemaError> { + // Check that all attributes exist in the schema + + // Normalise them + + // Validate them + + // Return new ModifyList! + + unimplemented!() + } +} + +impl ModifyList { + #[cfg(test)] + pub unsafe fn new_valid_list(mods: Vec) -> Self { + ModifyList { + valid: ModifyValid, + mods: mods + } + } +} + +impl ModifyList { + pub fn len(&self) -> usize { + self.mods.len() + } } diff --git a/src/lib/server.rs b/src/lib/server.rs index e7e6d0295..af52348b7 100644 --- a/src/lib/server.rs +++ b/src/lib/server.rs @@ -13,7 +13,7 @@ use entry::{Entry, EntryCommitted, EntryInvalid, EntryNew, EntryValid}; use error::{OperationError, SchemaError}; use event::{CreateEvent, DeleteEvent, ExistsEvent, ModifyEvent, ReviveRecycledEvent, SearchEvent}; use filter::{Filter, FilterInvalid}; -use modify::{Modify, ModifyList}; +use modify::{Modify, ModifyList, ModifyInvalid}; use plugins::Plugins; use schema::{Schema, SchemaReadTransaction, SchemaTransaction, SchemaWriteTransaction}; @@ -448,11 +448,16 @@ impl<'a> QueryServerWriteTransaction<'a> { return Err(OperationError::NoMatchingEntries); }; - let modlist = ModifyList::new_list(vec![Modify::Present( + let modlist_inv = ModifyList::new_list(vec![Modify::Present( String::from("class"), String::from("recycled"), )]); + let modlist = match modlist_inv.validate(&self.schema) { + Ok(ml) => ml, + Err(e) => return Err(OperationError::SchemaViolation(e)), + }; + let candidates: Vec> = pre_candidates .into_iter() .map(|er| { @@ -647,6 +652,12 @@ impl<'a> QueryServerWriteTransaction<'a> { return Err(OperationError::EmptyRequest); } + // Is the modlist valid? + let modlist = match me.modlist.validate(&self.schema) { + Ok(ml) => ml, + Err(e) => return Err(OperationError::SchemaViolation(e)), + }; + // Is the filter invalid to schema? // WARNING! Check access controls here!!!! @@ -675,7 +686,7 @@ impl<'a> QueryServerWriteTransaction<'a> { .into_iter() .map(|er| { // TODO: Deal with this properly william - er.invalidate().apply_modlist(&me.modlist).unwrap() + er.invalidate().apply_modlist(&modlist).unwrap() }) .collect(); @@ -786,7 +797,7 @@ impl<'a> QueryServerWriteTransaction<'a> { &self, audit: &mut AuditScope, filter: Filter, - modlist: ModifyList, + modlist: ModifyList, ) -> Result<(), OperationError> { let mut audit_int = AuditScope::new("internal_modify"); let me = ModifyEvent::new_internal(filter, modlist); @@ -799,7 +810,7 @@ impl<'a> QueryServerWriteTransaction<'a> { &self, audit: &mut AuditScope, filter: Filter, - modlist: ModifyList, + modlist: ModifyList, ) -> Result<(), OperationError> { let mut audit_int = AuditScope::new("impersonate_modify"); let me = ModifyEvent::new_internal(filter, modlist);