Improvements to behaviour (#4800)

core/src/doc/check.rs

@@ -124,27 +124,11 @@ impl Document {
 						// The id is a match, so don't error
 						v if rid.id.is(&v) => (),
 						// The in field does not match
-						v => match v.convert_to_record() {
+						v => {
-							// This is a value which matches the id
+							return Err(Error::IdMismatch {
-							Ok(v) if v.eq(&rid) => (),
+								value: v.to_string(),
-							// The value is a record but doesn't match
+							})
-							Ok(v) => {
+						}
-								return Err(Error::IdMismatch {
-									value: v.to_string(),
-								})
-							}
-							// The in field does not match at all
-							Err(Error::ConvertTo {
-								from,
-								..
-							}) => {
-								return Err(Error::IdMismatch {
-									value: from.to_string(),
-								})
-							}
-							// Return any other error
-							Err(e) => return Err(e),
-						},
 					}
 				}
 			}
@@ -161,27 +145,11 @@ impl Document {
 						// The in is a match, so don't error
 						v if l.id.is(&v) => (),
 						// The in field does not match
-						v => match v.convert_to_record() {
+						v => {
-							// This is a value which matches the id
+							return Err(Error::InMismatch {
-							Ok(v) if v.eq(l) => (),
+								value: v.to_string(),
-							// The value is a record but doesn't match
+							})
-							Ok(v) => {
+						}
-								return Err(Error::InMismatch {
-									value: v.to_string(),
-								})
-							}
-							// The in field does not match at all
-							Err(Error::ConvertTo {
-								from,
-								..
-							}) => {
-								return Err(Error::InMismatch {
-									value: from.to_string(),
-								})
-							}
-							// Return any other error
-							Err(e) => return Err(e),
-						},
 					}
 				}
 				// Check that the 'out' field matches
@@ -192,27 +160,11 @@ impl Document {
 						// The out is a match, so don't error
 						v if r.id.is(&v) => (),
 						// The in field does not match
-						v => match v.convert_to_record() {
+						v => {
-							// This is a value which matches the id
+							return Err(Error::OutMismatch {
-							Ok(v) if v.eq(r) => (),
+								value: v.to_string(),
-							// The value is a record but doesn't match
+							})
-							Ok(v) => {
+						}
-								return Err(Error::OutMismatch {
-									value: v.to_string(),
-								})
-							}
-							// The in field does not match at all
-							Err(Error::ConvertTo {
-								from,
-								..
-							}) => {
-								return Err(Error::OutMismatch {
-									value: from.to_string(),
-								})
-							}
-							// Return any other error
-							Err(e) => return Err(e),
-						},
 					}
 				}
 			}
@@ -225,27 +177,11 @@ impl Document {
 					// The in is a match, so don't error
 					v if l.id.is(&v) => (),
 					// The in field does not match
-					v => match v.convert_to_record() {
+					v => {
-						// This is a value which matches the id
+						return Err(Error::InMismatch {
-						Ok(v) if v.eq(l) => (),
+							value: v.to_string(),
-						// The value is a record but doesn't match
+						})
-						Ok(v) => {
+					}
-							return Err(Error::InMismatch {
-								value: v.to_string(),
-							})
-						}
-						// The in field does not match at all
-						Err(Error::ConvertTo {
-							from,
-							..
-						}) => {
-							return Err(Error::InMismatch {
-								value: from.to_string(),
-							})
-						}
-						// Return any other error
-						Err(e) => return Err(e),
-					},
 				}
 				// Check that the 'out' field matches
 				match data.pick(&*OUT).compute(stk, ctx, opt, Some(&self.current)).await? {
@@ -254,27 +190,11 @@ impl Document {
 					// The out is a match, so don't error
 					v if l.id.is(&v) => (),
 					// The out field does not match
-					v => match v.convert_to_record() {
+					v => {
-						// This is a value which matches the id
+						return Err(Error::OutMismatch {
-						Ok(v) if v.eq(l) => (),
+							value: v.to_string(),
-						// The value is a record but doesn't match
+						})
-						Ok(v) => {
+					}
-							return Err(Error::OutMismatch {
-								value: v.to_string(),
-							})
-						}
-						// The out field does not match at all
-						Err(Error::ConvertTo {
-							from,
-							..
-						}) => {
-							return Err(Error::OutMismatch {
-								value: from.to_string(),
-							})
-						}
-						// Return any other error
-						Err(e) => return Err(e),
-					},
 				}
 			}
 		}
@@ -287,7 +207,7 @@ impl Document {
 	/// a table, or from an index can be filtered out
 	/// before being included within the query output.
 	pub async fn check_where_condition(
-		&self,
+		&mut self,
 		stk: &mut Stk,
 		ctx: &Context,
 		opt: &Options,
@@ -295,8 +215,10 @@ impl Document {
 	) -> Result<(), Error> {
 		// Check where condition
 		if let Some(cond) = stm.conds() {
+			// Process the current permitted
+			self.process_permitted_current(stk, ctx, opt).await?;
 			// Check if the expression is truthy
-			if !cond.compute(stk, ctx, opt, Some(&self.current)).await?.is_truthy() {
+			if !cond.compute(stk, ctx, opt, Some(&self.current_permitted)).await?.is_truthy() {
 				// Ignore this document
 				return Err(Error::Ignore);
 			}

core/src/doc/document.rs

@@ -1,10 +1,12 @@
 use crate::ctx::Context;
+use crate::ctx::MutableContext;
 use crate::dbs::Options;
 use crate::dbs::Workable;
 use crate::err::Error;
 use crate::iam::Action;
 use crate::iam::ResourceKind;
 use crate::idx::planner::iterators::IteratorRecord;
+use crate::sql::permission::Permission;
 use crate::sql::statements::define::DefineEventStatement;
 use crate::sql::statements::define::DefineFieldStatement;
 use crate::sql::statements::define::DefineIndexStatement;
@@ -13,6 +15,7 @@ use crate::sql::statements::live::LiveStatement;
 use crate::sql::thing::Thing;
 use crate::sql::value::Value;
 use crate::sql::Base;
+use reblessive::tree::Stk;
 use std::fmt::{Debug, Formatter};
 use std::mem;
 use std::ops::Deref;
@@ -23,10 +26,12 @@ pub(crate) struct Document {
 	pub(super) extras: Workable,
 	pub(super) initial: CursorDoc,
 	pub(super) current: CursorDoc,
+	pub(super) initial_permitted: CursorDoc,
+	pub(super) current_permitted: CursorDoc,
 }
 
 #[non_exhaustive]
-#[cfg_attr(debug_assertions, derive(Debug))]
+#[derive(Clone, Debug)]
 pub struct CursorDoc {
 	pub(crate) rid: Option<Arc<Thing>>,
 	pub(crate) ir: Option<Arc<IteratorRecord>>,
@@ -34,8 +39,7 @@ pub struct CursorDoc {
 }
 
 #[non_exhaustive]
-#[cfg_attr(debug_assertions, derive(Debug))]
+#[derive(Clone, Debug)]
-#[derive(Clone)]
 pub(crate) struct CursorValue {
 	mutable: Value,
 	read_only: Option<Arc<Value>>,
@@ -165,7 +169,9 @@ impl Document {
 			id: id.clone(),
 			extras,
 			current: CursorDoc::new(id.clone(), ir.clone(), val.clone()),
-			initial: CursorDoc::new(id, ir, val),
+			initial: CursorDoc::new(id.clone(), ir.clone(), val.clone()),
+			current_permitted: CursorDoc::new(id.clone(), ir.clone(), val.clone()),
+			initial_permitted: CursorDoc::new(id.clone(), ir.clone(), val.clone()),
 		}
 	}
 
@@ -180,6 +186,118 @@ impl Document {
 	pub(crate) fn initial_doc(&self) -> &Value {
 		&self.initial.doc
 	}
+
+	pub(crate) async fn process_permitted_current(
+		&mut self,
+		stk: &mut Stk,
+		ctx: &Context,
+		opt: &Options,
+	) -> Result<(), Error> {
+		// Clone the fully permitted documents
+		self.current_permitted = self.current.clone();
+		// Check if this record exists
+		if self.id.is_some() {
+			// Should we run permissions checks?
+			if opt.check_perms(Action::View)? {
+				// Loop through all field statements
+				for fd in self.fd(ctx, opt).await?.iter() {
+					//
+					let mut out = self
+						.current
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current))
+						.await?;
+					// Loop over each field in document
+					for k in out.each(&fd.name).iter() {
+						// Process the field permissions
+						match &fd.permissions.select {
+							Permission::Full => (),
+							Permission::None => out.del(stk, ctx, opt, k).await?,
+							Permission::Specific(e) => {
+								// Disable permissions
+								let opt = &opt.new_with_perms(false);
+								// Get the current value
+								let val = Arc::new(self.current.doc.as_ref().pick(k));
+								// Configure the context
+								let mut ctx = MutableContext::new(ctx);
+								ctx.add_value("value", val);
+								let ctx = ctx.freeze();
+								// Process the PERMISSION clause
+								if !e
+									.compute(stk, &ctx, opt, Some(&self.current))
+									.await?
+									.is_truthy()
+								{
+									out.del(stk, &ctx, opt, k).await?
+								}
+							}
+						}
+					}
+					// Update the permitted document
+					self.current_permitted.doc = out.into();
+				}
+			}
+		}
+		// Carry on
+		Ok(())
+	}
+
+	pub(crate) async fn process_permitted_initial(
+		&mut self,
+		stk: &mut Stk,
+		ctx: &Context,
+		opt: &Options,
+	) -> Result<(), Error> {
+		// Clone the fully permitted documents
+		self.initial_permitted = self.initial.clone();
+		// Check if this record exists
+		if self.id.is_some() {
+			// Should we run permissions checks?
+			if opt.check_perms(Action::View)? {
+				// Loop through all field statements
+				for fd in self.fd(ctx, opt).await?.iter() {
+					//
+					let mut out = self
+						.initial
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.initial))
+						.await?;
+					// Loop over each field in document
+					for k in out.each(&fd.name).iter() {
+						// Process the field permissions
+						match &fd.permissions.select {
+							Permission::Full => (),
+							Permission::None => out.del(stk, ctx, opt, k).await?,
+							Permission::Specific(e) => {
+								// Disable permissions
+								let opt = &opt.new_with_perms(false);
+								// Get the current value
+								let val = Arc::new(self.initial.doc.as_ref().pick(k));
+								// Configure the context
+								let mut ctx = MutableContext::new(ctx);
+								ctx.add_value("value", val);
+								let ctx = ctx.freeze();
+								// Process the PERMISSION clause
+								if !e
+									.compute(stk, &ctx, opt, Some(&self.initial))
+									.await?
+									.is_truthy()
+								{
+									out.del(stk, &ctx, opt, k).await?
+								}
+							}
+						}
+					}
+					// Update the permitted document
+					self.initial_permitted.doc = out.into();
+				}
+			}
+		}
+		// Carry on
+		Ok(())
+	}
 }
 
 impl Document {

core/src/doc/pluck.rs

@@ -25,36 +25,91 @@ impl Document {
 	) -> Result<Value, Error> {
 		// Ensure futures are run
 		let opt = &opt.new_with_futures(true);
+		// Check if this record exists
+		if self.id.is_some() {
+			// Should we run permissions checks?
+			if opt.check_perms(Action::View)? {
+				// Get the table for this document
+				let table = self.tb(ctx, opt).await?;
+				// Get the permissions for this table
+				let perms = &table.permissions.select;
+				// Process the table permissions
+				match perms {
+					Permission::None => return Err(Error::Ignore),
+					Permission::Full => (),
+					Permission::Specific(e) => {
+						// Disable permissions
+						let opt = &opt.new_with_perms(false);
+						// Process the PERMISSION clause
+						if !e
+							.compute(
+								stk,
+								ctx,
+								opt,
+								Some(match stm.is_delete() {
+									true => &self.initial,
+									false => &self.current,
+								}),
+							)
+							.await?
+							.is_truthy()
+						{
+							return Err(Error::Ignore);
+						}
+					}
+				}
+			}
+		}
 		// Process the desired output
 		let mut out = match stm.output() {
 			Some(v) => match v {
 				Output::None => Err(Error::Ignore),
 				Output::Null => Ok(Value::Null),
 				Output::Diff => {
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the initial permitted
+					self.process_permitted_initial(stk, ctx, opt).await?;
 					// Output a DIFF of any changes applied to the document
 					Ok(self
-						.initial
+						.initial_permitted
 						.doc
 						.as_ref()
-						.diff(self.current.doc.as_ref(), Idiom::default())
+						.diff(self.current_permitted.doc.as_ref(), Idiom::default())
 						.into())
 				}
 				Output::After => {
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
 					// Output the full document after all changes were applied
-					self.current.doc.as_ref().compute(stk, ctx, opt, Some(&self.current)).await
+					self.current_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current_permitted))
+						.await
 				}
 				Output::Before => {
+					// Process the initial permitted
+					self.process_permitted_initial(stk, ctx, opt).await?;
 					// Output the full document before any changes were applied
-					self.initial.doc.as_ref().compute(stk, ctx, opt, Some(&self.initial)).await
+					self.initial_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.initial_permitted))
+						.await
 				}
 				Output::Fields(v) => {
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the initial permitted
+					self.process_permitted_initial(stk, ctx, opt).await?;
 					// Configure the context
 					let mut ctx = MutableContext::new(ctx);
-					ctx.add_value("after", self.current.doc.as_arc());
+					ctx.add_value("after", self.current_permitted.doc.as_arc());
-					ctx.add_value("before", self.initial.doc.as_arc());
+					ctx.add_value("before", self.initial_permitted.doc.as_arc());
 					let ctx = ctx.freeze();
 					// Output the specified fields
-					v.compute(stk, &ctx, opt, Some(&self.current), false).await
+					v.compute(stk, &ctx, opt, Some(&self.current_permitted), false).await
 				}
 			},
 			None => match stm {
@@ -68,22 +123,61 @@ impl Document {
 					_ => s.expr.compute(stk, ctx, opt, Some(&self.current), false).await,
 				},
 				Statement::Select(s) => {
-					s.expr.compute(stk, ctx, opt, Some(&self.current), s.group.is_some()).await
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					s.expr
+						.compute(stk, ctx, opt, Some(&self.current_permitted), s.group.is_some())
+						.await
 				}
 				Statement::Create(_) => {
-					self.current.doc.as_ref().compute(stk, ctx, opt, Some(&self.current)).await
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the document output
+					self.current_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current_permitted))
+						.await
 				}
 				Statement::Upsert(_) => {
-					self.current.doc.as_ref().compute(stk, ctx, opt, Some(&self.current)).await
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the document output
+					self.current_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current_permitted))
+						.await
 				}
 				Statement::Update(_) => {
-					self.current.doc.as_ref().compute(stk, ctx, opt, Some(&self.current)).await
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the document output
+					self.current_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current_permitted))
+						.await
 				}
 				Statement::Relate(_) => {
-					self.current.doc.as_ref().compute(stk, ctx, opt, Some(&self.current)).await
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the document output
+					self.current_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current_permitted))
+						.await
 				}
 				Statement::Insert(_) => {
-					self.current.doc.as_ref().compute(stk, ctx, opt, Some(&self.current)).await
+					// Process the current permitted
+					self.process_permitted_current(stk, ctx, opt).await?;
+					// Process the document output
+					self.current_permitted
+						.doc
+						.as_ref()
+						.compute(stk, ctx, opt, Some(&self.current_permitted))
+						.await
 				}
 				_ => Err(Error::Ignore),
 			},

sdk/tests/delete.rs

@@ -429,7 +429,7 @@ async fn delete_filtered_live_notification() -> Result<(), Error> {
 }
 
 #[tokio::test]
-async fn delete_with_permissions() -> Result<(), Error> {
+async fn delete_with_permissions_no_select() -> Result<(), Error> {
 	let sql = "
 		DEFINE TABLE friends_with PERMISSIONS FOR delete WHERE in = $auth;
 		CREATE user:john, user:mary;
@@ -462,6 +462,50 @@ async fn delete_with_permissions() -> Result<(), Error> {
 	assert_eq!(res.len(), 2);
 	//
 	let tmp = res.remove(0).result?;
+	let val = Value::parse("[]");
+	assert_eq!(tmp, val);
+	//
+	let tmp = res.remove(0).result?;
+	let val = Value::parse("[]");
+	assert_eq!(tmp, val);
+	//
+	Ok(())
+}
+
+#[tokio::test]
+async fn delete_with_permissions_with_select() -> Result<(), Error> {
+	let sql = "
+		DEFINE TABLE friends_with PERMISSIONS FOR select, delete WHERE in = $auth;
+		CREATE user:john, user:mary;
+		RELATE user:john->friends_with:1->user:mary;
+		RELATE user:mary->friends_with:2->user:john;
+	";
+	let dbs = new_ds().await?.with_auth_enabled(true);
+	let ses = Session::owner().with_ns("test").with_db("test");
+	let res = &mut dbs.execute(sql, &ses, None).await?;
+	assert_eq!(res.len(), 4);
+	//
+	let tmp = res.remove(0).result;
+	assert!(tmp.is_ok());
+	//
+	let tmp = res.remove(0).result;
+	assert!(tmp.is_ok());
+	//
+	let tmp = res.remove(0).result;
+	assert!(tmp.is_ok());
+	//
+	let tmp = res.remove(0).result;
+	assert!(tmp.is_ok());
+	//
+	let sql = "
+		DELETE friends_with:1 RETURN BEFORE;
+		DELETE friends_with:2 RETURN BEFORE;
+	";
+	let ses = Session::for_record("test", "test", "test", Thing::from(("user", "john")).into());
+	let res = &mut dbs.execute(sql, &ses, None).await?;
+	assert_eq!(res.len(), 2);
+	//
+	let tmp = res.remove(0).result?;
 	let val = Value::parse(
 		"[
 			{

sdk/tests/field.rs

@@ -844,7 +844,7 @@ async fn field_definition_edge_permissions() -> Result<(), Error> {
 		DEFINE TABLE user SCHEMAFULL;
 		DEFINE TABLE business SCHEMAFULL;
 		DEFINE FIELD owner ON TABLE business TYPE record<user>;
-		DEFINE TABLE contact TYPE RELATION SCHEMAFULL PERMISSIONS FOR create WHERE in.owner.id = $auth.id;
+		DEFINE TABLE contact TYPE RELATION SCHEMAFULL PERMISSIONS FOR select, create WHERE in.owner.id = $auth.id;
 		INSERT INTO user (id, name) VALUES (user:one, 'John'), (user:two, 'Lucy');
 		INSERT INTO business (id, owner) VALUES (business:one, user:one), (business:two, user:two);
 	";