From e2b30862786b6dcfbbf61c884f2bede79fd68e5d Mon Sep 17 00:00:00 2001 From: William Brown Date: Wed, 20 Mar 2019 15:30:34 +1000 Subject: [PATCH] Add UUID resolution to clone_val0ue --- src/lib/entry.rs | 11 +++- src/lib/event.rs | 21 +++++--- src/lib/filter.rs | 28 ++++++---- src/lib/modify.rs | 14 +++-- src/lib/proto_v1_actors.rs | 8 +-- src/lib/server.rs | 108 ++++++++++++++++++++++++++++++++----- 6 files changed, 148 insertions(+), 42 deletions(-) diff --git a/src/lib/entry.rs b/src/lib/entry.rs index de5d32e44..7be9862e6 100644 --- a/src/lib/entry.rs +++ b/src/lib/entry.rs @@ -1,4 +1,5 @@ // use serde_json::{Error, Value}; +use audit::AuditScope; use super::proto_v1::Entry as ProtoEntry; use error::{OperationError, SchemaError}; use filter::{Filter, FilterValid}; @@ -153,7 +154,11 @@ impl Entry { } // FIXME: Can we consume protoentry? - pub fn from(e: &ProtoEntry, qs: &QueryServerWriteTransaction) -> Result { + pub fn from( + audit: &mut AuditScope, + e: &ProtoEntry, + qs: &QueryServerWriteTransaction + ) -> Result { // Why not the trait? In the future we may want to extend // this with server aware functions for changes of the // incoming data. @@ -165,7 +170,7 @@ impl Entry { .attrs .iter() .map(|(k, v)| { - let nv: Result, _> = v.iter().map(|vr| qs.clone_value(&k, vr)).collect(); + let nv: Result, _> = v.iter().map(|vr| qs.clone_value(audit, &k, vr)).collect(); match nv { Ok(mut nvi) => { nvi.sort_unstable(); @@ -452,6 +457,7 @@ impl Entry { self.attribute_pres(attr.as_str()) } Filter::Or(l) => l.iter().fold(false, |acc, f| { + // Check with ftweedal about or filter zero len correctness. if acc { acc } else { @@ -459,6 +465,7 @@ impl Entry { } }), Filter::And(l) => l.iter().fold(true, |acc, f| { + // Check with ftweedal about and filter zero len correctness. if acc { self.entry_match_no_index(f) } else { diff --git a/src/lib/event.rs b/src/lib/event.rs index 09502af84..f42d60fb5 100644 --- a/src/lib/event.rs +++ b/src/lib/event.rs @@ -1,3 +1,4 @@ +use audit::AuditScope; use super::filter::{Filter, FilterInvalid}; use super::proto_v1::Entry as ProtoEntry; use super::proto_v1::{ @@ -67,10 +68,11 @@ pub struct SearchEvent { impl SearchEvent { pub fn from_request( + audit: &mut AuditScope, request: SearchRequest, qs: &QueryServerTransaction, ) -> Result { - match Filter::from_ro(&request.filter, qs) { + match Filter::from_ro(audit, &request.filter, qs) { Ok(f) => Ok(SearchEvent { internal: false, filter: Filter::new_ignore_hidden(f), @@ -88,10 +90,11 @@ impl SearchEvent { } pub fn from_rec_request( + audit: &mut AuditScope, request: SearchRecycledRequest, qs: &QueryServerTransaction, ) -> Result { - match Filter::from_ro(&request.filter, qs) { + match Filter::from_ro(audit, &request.filter, qs) { Ok(f) => Ok(SearchEvent { filter: Filter::new_recycled(f), internal: false, @@ -139,11 +142,12 @@ pub struct CreateEvent { // FIXME: Should this actually be in createEvent handler? impl CreateEvent { pub fn from_request( + audit: &mut AuditScope, request: CreateRequest, qs: &QueryServerWriteTransaction, ) -> Result { let rentries: Result, _> = - request.entries.iter().map(|e| Entry::from(e, qs)).collect(); + request.entries.iter().map(|e| Entry::from(audit, e, qs)).collect(); match rentries { Ok(entries) => Ok(CreateEvent { // From ProtoEntry -> Entry @@ -196,10 +200,11 @@ pub struct DeleteEvent { impl DeleteEvent { pub fn from_request( + audit: &mut AuditScope, request: DeleteRequest, qs: &QueryServerWriteTransaction, ) -> Result { - match Filter::from_rw(&request.filter, qs) { + match Filter::from_rw(audit, &request.filter, qs) { Ok(f) => Ok(DeleteEvent { filter: Filter::new_ignore_hidden(f), internal: false, @@ -233,11 +238,12 @@ pub struct ModifyEvent { impl ModifyEvent { pub fn from_request( + audit: &mut AuditScope, request: ModifyRequest, qs: &QueryServerWriteTransaction, ) -> Result { - match Filter::from_rw(&request.filter, qs) { - Ok(f) => match ModifyList::from(&request.modlist, qs) { + match Filter::from_rw(audit, &request.filter, qs) { + Ok(f) => match ModifyList::from(audit, &request.modlist, qs) { Ok(m) => Ok(ModifyEvent { filter: Filter::new_ignore_hidden(f), modlist: m, @@ -327,10 +333,11 @@ impl Message for ReviveRecycledEvent { impl ReviveRecycledEvent { pub fn from_request( + audit: &mut AuditScope, request: ReviveRecycledRequest, qs: &QueryServerWriteTransaction, ) -> Result { - match Filter::from_rw(&request.filter, qs) { + match Filter::from_rw(audit, &request.filter, qs) { Ok(f) => Ok(ReviveRecycledEvent { filter: Filter::new_recycled(f), internal: false, diff --git a/src/lib/filter.rs b/src/lib/filter.rs index 9ac7ce1c6..d1eb551a0 100644 --- a/src/lib/filter.rs +++ b/src/lib/filter.rs @@ -2,6 +2,7 @@ // in parallel map/reduce style, or directly on a single // entry to assert it matches. +use audit::AuditScope; use be::BackendReadTransaction; use error::{OperationError, SchemaError}; use proto_v1::Filter as ProtoFilter; @@ -195,44 +196,49 @@ impl Filter { // TODO: This has to have two versions to account for ro/rw traits, because RS can't // monomorphise on the trait to call clone_value. An option is to make a fn that // takes "clone_value(t, a, v) instead, but that may have a similar issue. - pub fn from_ro(f: &ProtoFilter, qs: &QueryServerTransaction) -> Result { + pub fn from_ro( + audit: &mut AuditScope, + f: &ProtoFilter, + qs: &QueryServerTransaction + ) -> Result { Ok(match f { - ProtoFilter::Eq(a, v) => Filter::Eq(a.clone(), qs.clone_value(a, v)?), - ProtoFilter::Sub(a, v) => Filter::Sub(a.clone(), qs.clone_value(a, v)?), + ProtoFilter::Eq(a, v) => Filter::Eq(a.clone(), qs.clone_value(audit, a, v)?), + ProtoFilter::Sub(a, v) => Filter::Sub(a.clone(), qs.clone_value(audit, a, v)?), ProtoFilter::Pres(a) => Filter::Pres(a.clone()), ProtoFilter::Or(l) => Filter::Or( l.iter() - .map(|f| Self::from_ro(f, qs)) + .map(|f| Self::from_ro(audit, f, qs)) .collect::, _>>()?, ), ProtoFilter::And(l) => Filter::And( l.iter() - .map(|f| Self::from_ro(f, qs)) + .map(|f| Self::from_ro(audit, f, qs)) .collect::, _>>()?, ), - ProtoFilter::AndNot(l) => Filter::AndNot(Box::new(Self::from_ro(l, qs)?)), + ProtoFilter::AndNot(l) => Filter::AndNot(Box::new(Self::from_ro(audit, l, qs)?)), }) } pub fn from_rw( + audit: &mut AuditScope, f: &ProtoFilter, qs: &QueryServerWriteTransaction, ) -> Result { Ok(match f { - ProtoFilter::Eq(a, v) => Filter::Eq(a.clone(), qs.clone_value(a, v)?), - ProtoFilter::Sub(a, v) => Filter::Sub(a.clone(), qs.clone_value(a, v)?), + ProtoFilter::Eq(a, v) => Filter::Eq(a.clone(), qs.clone_value(audit, a, v)?), + ProtoFilter::Sub(a, v) => Filter::Sub(a.clone(), qs.clone_value(audit, a, v)?), ProtoFilter::Pres(a) => Filter::Pres(a.clone()), ProtoFilter::Or(l) => Filter::Or( l.iter() - .map(|f| Self::from_rw(f, qs)) + .map(|f| Self::from_rw(audit, f, qs)) .collect::, _>>()?, ), ProtoFilter::And(l) => Filter::And( l.iter() - .map(|f| Self::from_rw(f, qs)) + .map(|f| Self::from_rw(audit, f, qs)) .collect::, _>>()?, ), - ProtoFilter::AndNot(l) => Filter::AndNot(Box::new(Self::from_rw(l, qs)?)), + ProtoFilter::AndNot(l) => Filter::AndNot(Box::new(Self::from_rw(audit, l, qs)?)), }) } } diff --git a/src/lib/modify.rs b/src/lib/modify.rs index f733324b9..5e8f19be4 100644 --- a/src/lib/modify.rs +++ b/src/lib/modify.rs @@ -1,3 +1,4 @@ +use audit::AuditScope; use proto_v1::Modify as ProtoModify; use proto_v1::ModifyList as ProtoModifyList; @@ -24,10 +25,14 @@ pub enum Modify { } impl Modify { - pub fn from(m: &ProtoModify, qs: &QueryServerWriteTransaction) -> Result { + pub fn from( + audit: &mut AuditScope, + m: &ProtoModify, + qs: &QueryServerWriteTransaction + ) -> Result { Ok(match m { - ProtoModify::Present(a, v) => Modify::Present(a.clone(), qs.clone_value(a, v)?), - ProtoModify::Removed(a, v) => Modify::Removed(a.clone(), qs.clone_value(a, v)?), + ProtoModify::Present(a, v) => Modify::Present(a.clone(), qs.clone_value(audit, a, v)?), + ProtoModify::Removed(a, v) => Modify::Removed(a.clone(), qs.clone_value(audit, a, v)?), ProtoModify::Purged(a) => Modify::Purged(a.clone()), }) } @@ -72,11 +77,12 @@ impl ModifyList { } pub fn from( + audit: &mut AuditScope, ml: &ProtoModifyList, qs: &QueryServerWriteTransaction, ) -> Result { // For each ProtoModify, do a from. - let inner: Result, _> = ml.mods.iter().map(|pm| Modify::from(pm, qs)).collect(); + let inner: Result, _> = ml.mods.iter().map(|pm| Modify::from(audit, pm, qs)).collect(); match inner { Ok(m) => Ok(ModifyList { valid: ModifyInvalid, diff --git a/src/lib/proto_v1_actors.rs b/src/lib/proto_v1_actors.rs index a72b26e49..ddf38eca0 100644 --- a/src/lib/proto_v1_actors.rs +++ b/src/lib/proto_v1_actors.rs @@ -136,7 +136,7 @@ impl Handler for QueryServerV1 { let qs_read = self.qs.read(); // Make an event from the request - let srch = match SearchEvent::from_request(msg, &qs_read) { + let srch = match SearchEvent::from_request(&mut audit, msg, &qs_read) { Ok(s) => s, Err(e) => { audit_log!(audit, "Failed to begin search: {:?}", e); @@ -169,7 +169,7 @@ impl Handler for QueryServerV1 { let res = audit_segment!(&mut audit, || { let qs_write = self.qs.write(); - let crt = match CreateEvent::from_request(msg, &qs_write) { + let crt = match CreateEvent::from_request(&mut audit, msg, &qs_write) { Ok(c) => c, Err(e) => { audit_log!(audit, "Failed to begin create: {:?}", e); @@ -196,7 +196,7 @@ impl Handler for QueryServerV1 { let mut audit = AuditScope::new("modify"); let res = audit_segment!(&mut audit, || { let qs_write = self.qs.write(); - let mdf = match ModifyEvent::from_request(msg, &qs_write) { + let mdf = match ModifyEvent::from_request(&mut audit, msg, &qs_write) { Ok(m) => m, Err(e) => { audit_log!(audit, "Failed to begin modify: {:?}", e); @@ -223,7 +223,7 @@ impl Handler for QueryServerV1 { let res = audit_segment!(&mut audit, || { let qs_write = self.qs.write(); - let del = match DeleteEvent::from_request(msg, &qs_write) { + let del = match DeleteEvent::from_request(&mut audit, msg, &qs_write) { Ok(d) => d, Err(e) => { audit_log!(audit, "Failed to begin delete: {:?}", e); diff --git a/src/lib/server.rs b/src/lib/server.rs index 3c87057a1..3f9fa1a07 100644 --- a/src/lib/server.rs +++ b/src/lib/server.rs @@ -250,24 +250,39 @@ pub trait QueryServerReadTransaction { // TODO: It could be argued that we should have a proper "Value" type, so that we can // take care of this a bit cleaner, and do the checks in that, but I think for // now this is good enough. - fn clone_value(&self, attr: &String, value: &String) -> Result { + fn clone_value(&self, + audit: &mut AuditScope, + attr: &String, + value: &String + ) -> Result { let schema = self.get_schema(); + // TODO: Normalise the attr, else lookup with fail .... + let schema_name = schema.get_attributes().get("name").expect("Schema corrupted"); + + // TODO: Should we return the normalise attr? + let temp_a = schema_name.normalise_value(attr); + // Lookup the attr - match schema.get_attributes().get(attr) { + match schema.get_attributes().get(&temp_a) { Some(schema_a) => { // Now check the type of the attribute ... match schema_a.syntax { SyntaxType::REFERENCE_UUID => { - // So, if possible, resolve the value - // to a concrete uuid. - // Is it already a UUID? - // Yes, move along ... - // No, attempt to resolve - // self.name_to_uuid() - // if resolve works, great - // if resolve fails, we need to fail. - - unimplemented!() + match schema_a.validate_value(value) { + // So, if possible, resolve the value + // to a concrete uuid. + Ok(_) => { + // TODO: Should this check existance? + Ok(value.clone()) + } + Err(_) => { + // it's not a uuid, try to resolve it. + // TODO: If this errors, should we actually pass + // back a place holder "no such uuid" and then + // fail an exists check later? + Ok(self.name_to_uuid(audit, value)?) + } + } } _ => { // Probs okay. @@ -1420,6 +1435,7 @@ mod tests { // Create fake external requests. Probably from admin later // Should we do this with impersonate instead of using the external let me_ts = ModifyEvent::from_request( + audit, ModifyRequest::new( filt_ts.clone(), ProtoModifyList::new_list(vec![ProtoModify::Present( @@ -1430,7 +1446,7 @@ mod tests { &server_txn, ) .unwrap(); - let de_ts = DeleteEvent::from_request(DeleteRequest::new(filt_ts.clone()), &server_txn) + let de_ts = DeleteEvent::from_request(audit, DeleteRequest::new(filt_ts.clone()), &server_txn) .unwrap(); let se_ts = SearchEvent::new_ext_impersonate(filt_i_ts.clone()); @@ -1497,6 +1513,7 @@ mod tests { // Create fake external requests. Probably from admin later let me_rc = ModifyEvent::from_request( + audit, ModifyRequest::new( filt_rc.clone(), ProtoModifyList::new_list(vec![ProtoModify::Present( @@ -1507,13 +1524,14 @@ mod tests { &server_txn, ) .unwrap(); - let de_rc = DeleteEvent::from_request(DeleteRequest::new(filt_rc.clone()), &server_txn) + let de_rc = DeleteEvent::from_request(audit, DeleteRequest::new(filt_rc.clone()), &server_txn) .unwrap(); let se_rc = SearchEvent::new_ext_impersonate(filt_i_rc.clone()); let sre_rc = SearchEvent::new_rec_impersonate(filt_i_rc.clone()); let rre_rc = ReviveRecycledEvent::from_request( + audit, ReviveRecycledRequest::new(ProtoFilter::Eq( "name".to_string(), "testperson1".to_string(), @@ -1733,4 +1751,66 @@ mod tests { assert!(r4.is_ok()); }) } + + #[test] + fn test_qs_clone_value() { + run_test!(|server: QueryServer, audit: &mut AuditScope| { + let server_txn = server.write(); + let e1: Entry = serde_json::from_str( + r#"{ + "valid": null, + "state": null, + "attrs": { + "class": ["object", "person"], + "name": ["testperson1"], + "uuid": ["cc8e95b4-c24f-4d68-ba54-8bed76f63930"], + "description": ["testperson"], + "displayname": ["testperson1"] + } + }"#, + ) + .unwrap(); + let ce = CreateEvent::from_vec(vec![e1]); + let cr = server_txn.create(audit, &ce); + assert!(cr.is_ok()); + + // test attr not exist + let r1 = server_txn.clone_value( + audit, + &"tausau".to_string(), + &"naoeutnhaou".to_string(), + ); + + assert!(r1 == Ok("naoeutnhaou".to_string())); + + // test attr not-normalised + // test attr not-reference + let r2 = server_txn.clone_value( + audit, + &"NaMe".to_string(), + &"NaMe".to_string(), + ); + + assert!(r2 == Ok("NaMe".to_string())); + + // test attr reference + let r3 = server_txn.clone_value( + audit, + &"member".to_string(), + &"testperson1".to_string(), + ); + + assert!(r3 == Ok("cc8e95b4-c24f-4d68-ba54-8bed76f63930".to_string())); + + // test attr reference already resolved. + let r4 = server_txn.clone_value( + audit, + &"member".to_string(), + &"cc8e95b4-c24f-4d68-ba54-8bed76f63930".to_string(), + ); + + println!("{:?}", r4); + assert!(r4 == Ok("cc8e95b4-c24f-4d68-ba54-8bed76f63930".to_string())); + }) + } }