From 61335a1ae59b9c1b668b8e2de3c0dff36ebac2b8 Mon Sep 17 00:00:00 2001 From: William Brown Date: Tue, 5 Mar 2019 15:03:24 +1000 Subject: [PATCH] Implemented name to uuid --- src/lib/error.rs | 1 + src/lib/event.rs | 34 ++---------- src/lib/filter.rs | 23 ++++++++- src/lib/schema.rs | 13 ++--- src/lib/server.rs | 128 +++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 154 insertions(+), 45 deletions(-) diff --git a/src/lib/error.rs b/src/lib/error.rs index 3c1fd0d88..81187ed8a 100644 --- a/src/lib/error.rs +++ b/src/lib/error.rs @@ -24,5 +24,6 @@ pub enum OperationError { InvalidDBState, InvalidRequestState, InvalidState, + InvalidEntryState, BackendEngine, } diff --git a/src/lib/event.rs b/src/lib/event.rs index 11694d3ad..4aa7391aa 100644 --- a/src/lib/event.rs +++ b/src/lib/event.rs @@ -69,13 +69,7 @@ impl SearchEvent { pub fn from_request(request: SearchRequest) -> Self { SearchEvent { internal: false, - filter: Filter::And(vec![ - Filter::AndNot(Box::new(Filter::Or(vec![ - Filter::Eq("class".to_string(), "tombstone".to_string()), - Filter::Eq("class".to_string(), "recycled".to_string()), - ]))), - Filter::from(&request.filter), - ]), + filter: Filter::new_ignore_hidden(Filter::from(&request.filter)), class: (), } } @@ -90,10 +84,7 @@ impl SearchEvent { pub fn from_rec_request(request: SearchRecycledRequest) -> Self { SearchEvent { - filter: Filter::And(vec![ - Filter::Eq("class".to_string(), "recycled".to_string()), - Filter::from(&request.filter), - ]), + filter: Filter::new_recycled(Filter::from(&request.filter)), internal: false, class: (), } @@ -174,13 +165,7 @@ pub struct DeleteEvent { impl DeleteEvent { pub fn from_request(request: DeleteRequest) -> Self { DeleteEvent { - filter: Filter::And(vec![ - Filter::AndNot(Box::new(Filter::Or(vec![ - Filter::Eq("class".to_string(), "tombstone".to_string()), - Filter::Eq("class".to_string(), "recycled".to_string()), - ]))), - Filter::from(&request.filter), - ]), + filter: Filter::new_ignore_hidden(Filter::from(&request.filter)), internal: false, } } @@ -211,13 +196,7 @@ pub struct ModifyEvent { impl ModifyEvent { pub fn from_request(request: ModifyRequest) -> Self { ModifyEvent { - filter: Filter::And(vec![ - Filter::AndNot(Box::new(Filter::Or(vec![ - Filter::Eq("class".to_string(), "tombstone".to_string()), - Filter::Eq("class".to_string(), "recycled".to_string()), - ]))), - Filter::from(&request.filter), - ]), + filter: Filter::new_ignore_hidden(Filter::from(&request.filter)), modlist: ModifyList::from(&request.modlist), internal: false, } @@ -301,10 +280,7 @@ impl Message for ReviveRecycledEvent { impl ReviveRecycledEvent { pub fn from_request(request: ReviveRecycledRequest) -> Self { ReviveRecycledEvent { - filter: Filter::And(vec![ - Filter::Eq("class".to_string(), "recycled".to_string()), - Filter::from(&request.filter), - ]), + filter: Filter::new_recycled(Filter::from(&request.filter)), internal: false, } } diff --git a/src/lib/filter.rs b/src/lib/filter.rs index fea392ea4..04e90d3db 100644 --- a/src/lib/filter.rs +++ b/src/lib/filter.rs @@ -68,6 +68,25 @@ impl Filter { } impl Filter { + pub fn new_ignore_hidden(inner: Filter) -> Self { + // Create a new filter, that ignores hidden entries. + Filter::And(vec![ + Filter::AndNot(Box::new(Filter::Or(vec![ + Filter::Eq("class".to_string(), "tombstone".to_string()), + Filter::Eq("class".to_string(), "recycled".to_string()), + ]))), + inner, + ]) + } + + pub fn new_recycled(inner: Filter) -> Self { + // Create a filter that searches recycled items only. + Filter::And(vec![ + Filter::Eq("class".to_string(), "recycled".to_string()), + inner, + ]) + } + pub fn validate( &self, schema: &SchemaReadTransaction, @@ -96,7 +115,7 @@ impl Filter { Some(schema_a) => { let value_norm = schema_a.normalise_value(value); schema_a - .validate_value(value) + .validate_value(&value_norm) // Okay, it worked, transform to a filter component .map(|_| Filter::Eq(attr_norm, value_norm)) // On error, pass the error back out. @@ -112,7 +131,7 @@ impl Filter { Some(schema_a) => { let value_norm = schema_a.normalise_value(value); schema_a - .validate_value(value) + .validate_value(&value_norm) // Okay, it worked, transform to a filter component .map(|_| Filter::Sub(attr_norm, value_norm)) // On error, pass the error back out. diff --git a/src/lib/schema.rs b/src/lib/schema.rs index 239461637..068516d87 100644 --- a/src/lib/schema.rs +++ b/src/lib/schema.rs @@ -1479,29 +1479,27 @@ mod tests { let f_insense = Filter::Eq("class".to_string(), "AttributeType".to_string()); assert_eq!( f_insense.validate(&schema), - Err(SchemaError::InvalidAttributeSyntax) + Ok(Filter::Eq("class".to_string(), "attributetype".to_string())) ); // Test the recursive structures validate let f_or_empty = Filter::Or(Vec::new()); assert_eq!(f_or_empty.validate(&schema), Err(SchemaError::EmptyFilter)); - let f_or = Filter::Or(vec![Filter::Eq( - "class".to_string(), - "AttributeType".to_string(), - )]); + let f_or = Filter::Or(vec![Filter::Eq("secret".to_string(), "zzzz".to_string())]); assert_eq!( f_or.validate(&schema), Err(SchemaError::InvalidAttributeSyntax) ); let f_or_mult = Filter::And(vec![ Filter::Eq("class".to_string(), "attributetype".to_string()), - Filter::Eq("class".to_string(), "AttributeType".to_string()), + Filter::Eq("secret".to_string(), "zzzzzzz".to_string()), ]); assert_eq!( f_or_mult.validate(&schema), Err(SchemaError::InvalidAttributeSyntax) ); + // Test mixed case attr name - this is a pass, due to normalisation let f_or_ok = Filter::AndNot(Box::new(Filter::And(vec![ - Filter::Eq("class".to_string(), "attributetype".to_string()), + Filter::Eq("Class".to_string(), "AttributeType".to_string()), Filter::Sub("class".to_string(), "classtype".to_string()), Filter::Pres("class".to_string()), ]))); @@ -1513,7 +1511,6 @@ mod tests { Filter::Pres("class".to_string()), ])))) ); - // Test mixed case attr name - this is a pass, due to normalisation println!("{}", audit); } diff --git a/src/lib/server.rs b/src/lib/server.rs index 8e08c2f51..60187e5d3 100644 --- a/src/lib/server.rs +++ b/src/lib/server.rs @@ -36,22 +36,22 @@ pub trait QueryServerReadTransaction { au: &mut AuditScope, se: &SearchEvent, ) -> Result>, OperationError> { - // How to get schema? + audit_log!(au, "search: filter -> {:?}", se.filter); + // This is an important security step because it prevents us from // performing un-indexed searches on attr's that don't exist in the // server. This is why ExtensibleObject can only take schema that // exists in the server, not arbitrary attr names. - audit_log!(au, "search: filter -> {:?}", se.filter); - - // TODO: Normalise the filter - - // TODO: Validate the filter + // + // This normalises and validates in a single step. let vf = match se.filter.validate(self.get_schema()) { Ok(f) => f, // TODO: Do something with this error Err(e) => return Err(OperationError::SchemaViolation(e)), }; + audit_log!(au, "search: valid filter -> {:?}", vf); + // TODO: Assert access control allows the filter and requested attrs. // TODO: Pre-search plugins @@ -91,6 +91,83 @@ pub trait QueryServerReadTransaction { res } + // TODO: Should this actually be names_to_uuids and we do batches? + // In the initial design "no", we can always write a batched + // interface later. + // + // The main question is if we need association between the name and + // the request uuid - if we do, we need singular. If we don't, we can + // just do the batching. + // + // Filter conversion likely needs 1:1, due to and/or conversions + // but create/mod likely doesn't due to the nature of the attributes. + // + // In the end, singular is the simple and correct option, so lets do + // that first, and we can add batched (and cache!) later. + // + // Remember, we don't care if the name is invalid, because search + // will validate/normalise the filter we construct for us. COOL! + fn name_to_uuid( + &self, + audit: &mut AuditScope, + name: &String, + ) -> Result { + // For now this just constructs a filter and searches, but later + // we could actually improve this to contact the backend and do + // index searches, completely bypassing id2entry. + + // construct the filter + let filt = Filter::new_ignore_hidden(Filter::Eq("name".to_string(), name.clone())); + audit_log!(audit, "name_to_uuid: name -> {:?}", name); + + // Internal search - DO NOT SEARCH TOMBSTONES AND RECYCLE + let res = match self.internal_search(audit, filt) { + Ok(e) => e, + Err(e) => return Err(e), + }; + + audit_log!(audit, "name_to_uuid: results -- {:?}", res); + + if res.len() == 0 { + // If result len == 0, error no such result + return Err(OperationError::NoMatchingEntries); + } else if res.len() >= 2 { + // if result len >= 2, error, invaid entry state. + return Err(OperationError::InvalidDBState); + } + + // TODO: Is there a better solution here than this? + // Perhaps we could res.first, then unwrap the some + // for 0/1 case, but check len for >= 2 to eliminate that case. + let e = res.first().unwrap(); + // Get the uuid from the entry. Again, check it exists, and only one. + let uuid_res = match e.get_ava(&String::from("uuid")) { + Some(vas) => match vas.first() { + Some(u) => u.clone(), + None => return Err(OperationError::InvalidEntryState), + }, + None => return Err(OperationError::InvalidEntryState), + }; + + audit_log!(audit, "name_to_uuid: uuid <- {:?}", uuid_res); + + Ok(uuid_res) + } + + fn uuid_to_name(&self, audit: &mut AuditScope, &String) -> Result { + // Construct filter + + // Internal search - DO NOT SEARCH TOMBSTONES AND RECYCLE + + // If result len == 0, error no such result + // if result len >= 2, error, invaid entry state. + + // Get the name + + // Return it. + unimplemented!(); + } + // From internal, generate an exists event and dispatch fn internal_exists( &self, @@ -1497,4 +1574,43 @@ mod tests { assert!(server_txn.commit(audit).is_ok()); }) } + + #[test] + fn test_qs_name_to_uuid() { + run_test!(|mut server: QueryServer, audit: &mut AuditScope| { + let mut 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()); + + // Name doesn't exist + let r1 = server_txn.name_to_uuid(audit, &String::from("testpers")); + assert!(r1.is_err()); + // Name doesn't exist (not syntax normalised) + let r2 = server_txn.name_to_uuid(audit, &String::from("tEsTpErS")); + assert!(r2.is_err()); + // Name does exist + let r3 = server_txn.name_to_uuid(audit, &String::from("testperson1")); + assert!(r3.is_ok()); + // Name is not syntax normalised (but exists) + let r4 = server_txn.name_to_uuid(audit, &String::from("tEsTpErSoN1")); + println!("{:?}", r4); + assert!(r4.is_ok()); + }) + } }