From 680a58140db48d8776e979096747e25e8e099a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20K=C3=A4ldstr=C3=B6m?= Date: Sun, 13 Sep 2026 23:05:43 +0300 Subject: [PATCH 1/3] =?UTF-8?q?Plan=205:=20query=20=E2=80=94=20an=20unknow?= =?UTF-8?q?n=20principal=20and/or=20resource,=20one=20row=20per=20candidat?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01JSRPugoPu8zLyykBCW6QJh --- docs/plans/5-query.md | 45 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/plans/5-query.md diff --git a/docs/plans/5-query.md b/docs/plans/5-query.md new file mode 100644 index 0000000..e77a2c5 --- /dev/null +++ b/docs/plans/5-query.md @@ -0,0 +1,45 @@ +# Plan 5 — query: an unknown principal and/or resource, one row per candidate + +## Goal + +The README's second use: a request whose `principal` and/or `resource` is unknown returns, from the same +query, one row per candidate entity (every row of the type's table) or per pair, each with the decision, the +determining policies and the erroring policies of the concrete request it stands for. + +## Design + +- **Unknown roots** (`src/compile.rs`): `Compiler::ensure_unknown_roots` registers a root CTE for every + request variable without an id, so the query enumerates the candidates even when no policy refers to them + (a policy set that partial evaluation decided outright still yields one row per candidate, all with the + folded decision). Unknown roots are `CROSS JOIN`ed, as before; a `Var` in a residual reads the root's id. +- **Rows** (`src/authorizer.rs`): `CompiledAuthorization::unknown_roots` lists the enumerated variables in + order, and `row` decodes their ids into `EntityUid`s (the types come from the request) before the policy + columns; `SqlAuthorizer::query` runs the query and returns `QueryRow { principal, resource, response }` + per row; `is_authorized` rejects partial requests and `query` concrete ones. `partial_request` builds the + `PartialRequest` from a concrete `Request` with the chosen ids dropped (`concrete_request` is the special + case), and now rejects a request without a context, which `cedar-policy` treats as unknown, not empty. +- **Review fixes of branch 3**: `hasTag` on an entity type without tags is a guarded `FALSE` (the validator + types it as `false`); JSON-stored contents are checked against their Cedar types when loading + (`load::conforms`), since the compiled queries compare entity references inside sets and records by id + under their static type; the connection sets `standard_conforming_strings = on`, which the literal + quoting assumes. + +## Files + +`src/{compile,authorizer,load,backend/postgres}.rs`, `tests/{query_pg,hierarchy_pg,authorize_pg,load_pg}.rs`, +`tests/entities/kitchen_sink.json`, `docs/plans/5-query.md`. + +## Verification + +`cargo test --all-features` with `CEDAR_SQL_PG_URL` set: `tests/query_pg.rs` runs partial requests with the +principal, the resource and both unknown, and checks every row against `cedar_policy::Authorizer` on the +concrete candidate, the row set against the entities of the unknown types, and the allowed set against +`PolicySet::query_resource`; `tests/hierarchy_pg.rs` runs the recursive ancestors CTE over a table holding +only direct edges, with a cycle; `tests/authorize_pg.rs` gains the review's probes (quoted and over-long +literal ids, `like` over values with `%`, `_`, `\` and a newline, same-type elements on the right of `in`, +`x in x`, a join and an ancestors check on one path, `hasTag` on a tagless type). Then clippy and fmt. + +## History + +New; the fifth `cedar-sql` branch. The fixes for the review of branch 3 landed here, since branch 4 had +rewritten the compiler in the meantime. From 2d62adbf5c63651d0f1e2ad5308f36abee451fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20K=C3=A4ldstr=C3=B6m?= Date: Sun, 13 Sep 2026 23:05:43 +0300 Subject: [PATCH 2/3] Query partial requests: one row per candidate; branch 3 review fixes `SqlAuthorizer::query` enumerates the candidates of an unknown principal and/or resource (the roots are registered even when no policy refers to them) and returns each row's decision, determining and erroring policies. From the review of branch 3: `hasTag` on a tagless type is a guarded `FALSE`, the loader checks JSON-stored contents against their Cedar types, a request without a context is rejected as unknown, and connections set `standard_conforming_strings`. Tests: partial requests against brute-force concrete authorization and `query_resource`, the recursive ancestors CTE over direct edges with a cycle, and the review's probes. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01JSRPugoPu8zLyykBCW6QJh --- Cargo.toml | 1 + src/authorizer.rs | 142 ++++++++++++-- src/backend/postgres.rs | 8 +- src/compile.rs | 25 ++- src/load.rs | 64 +++++- tests/authorize_pg.rs | 65 ++++++ tests/entities/kitchen_sink.json | 3 + tests/hierarchy_pg.rs | 144 ++++++++++++++ tests/load_pg.rs | 33 +++- tests/query_pg.rs | 327 +++++++++++++++++++++++++++++++ 10 files changed, 783 insertions(+), 29 deletions(-) create mode 100644 tests/hierarchy_pg.rs create mode 100644 tests/query_pg.rs diff --git a/Cargo.toml b/Cargo.toml index d2467b5..6c8a024 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/luxas/cedar-sql" cedar-policy = { version = "4.12.0", features = ["tpe"] } cedar-policy-core = { version = "4.12.0", features = ["tpe"] } indexmap = { version = "2", features = ["serde"] } +ref-cast = "1" postgres = { version = "0.19", features = ["with-serde_json-1"] } serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/src/authorizer.rs b/src/authorizer.rs index 7374cd6..38b6453 100644 --- a/src/authorizer.rs +++ b/src/authorizer.rs @@ -11,19 +11,20 @@ //! policies are the satisfied forbids if any else the satisfied permits, and //! the erroring policies are reported but do not affect the decision. -use std::collections::{BTreeMap, BTreeSet}; -use std::sync::Arc; +use std::collections::BTreeSet; use cedar_policy::{ - Decision, Entities, PolicyId, PolicySet, Request, Schema, ValidationMode, Validator, + Decision, Entities, EntityId, EntityTypeName, EntityUid, PolicyId, PolicySet, Request, Schema, + ValidationMode, Validator, }; use cedar_policy_core::ast::{Context, Effect, Entity, EntityUIDEntry}; use cedar_policy_core::tpe; use cedar_policy_core::tpe::entities::{PartialEntities, PartialEntity}; use cedar_policy_core::tpe::request::{PartialEntityUID, PartialRequest}; +use ref_cast::RefCast; use crate::backend::{Backend, Row, SqlValue}; -use crate::compile::Compiler; +use crate::compile::{Compiler, RootKey}; use crate::config::DatabaseConfiguration; use crate::dialect::Dialect; use crate::{Error, Result}; @@ -39,36 +40,85 @@ pub struct Response { pub errors: BTreeSet, } +/// One row of a partial-request query: a candidate for each unknown +/// variable, and the response for that candidate. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct QueryRow { + /// The principal, when it was unknown. + pub principal: Option, + /// The resource, when it was unknown. + pub resource: Option, + /// The response for this candidate. + pub response: Response, +} + /// The outcome of partial evaluation and compilation for one request. #[derive(Clone, Debug)] pub struct CompiledAuthorization { - /// The query, when some policies are residual; `None` when partial - /// evaluation decided every policy. + /// The query, when some policies are residual or a request variable is + /// unknown; `None` when partial evaluation decided every policy of a + /// concrete request. pub sql: Option, /// The residual policies, in the order of the query's policy columns. pub residuals: Vec<(PolicyId, Effect)>, - /// The unknown request variables whose ids the query selects first. - pub unknown_roots: usize, + /// The unknown request variables whose ids the query selects first, in + /// order (`principal` before `resource`). + pub unknown_roots: Vec, + principal_type: EntityTypeName, + resource_type: EntityTypeName, true_permits: BTreeSet, true_forbids: BTreeSet, errors: BTreeSet, } impl CompiledAuthorization { + /// The row's candidates and response. + pub fn row(&self, row: &Row) -> Result { + let mut principal = None; + let mut resource = None; + for (root, value) in self.unknown_roots.iter().zip(row) { + let SqlValue::Text(id) = value else { + return Err(Error::Query(format!("a root id column holds {value:?}"))); + }; + let id = EntityId::new(id); + match root { + RootKey::Principal => { + principal = Some(EntityUid::from_type_name_and_id( + self.principal_type.clone(), + id, + )); + } + RootKey::Resource => { + resource = Some(EntityUid::from_type_name_and_id( + self.resource_type.clone(), + id, + )); + } + RootKey::Literal(_) => unreachable!("only variables are unknown roots"), + } + } + Ok(QueryRow { + principal, + resource, + response: self.response(Some(row))?, + }) + } + /// The response for one result row (`None` when there is no query). pub fn response(&self, row: Option<&Row>) -> Result { let mut permits = self.true_permits.clone(); let mut forbids = self.true_forbids.clone(); let mut errors = self.errors.clone(); if let Some(row) = row { - if row.len() != self.unknown_roots + self.residuals.len() { + let roots = self.unknown_roots.len(); + if row.len() != roots + self.residuals.len() { return Err(Error::Query(format!( "expected {} columns, got {}", - self.unknown_roots + self.residuals.len(), + roots + self.residuals.len(), row.len() ))); } - for ((id, effect), value) in self.residuals.iter().zip(&row[self.unknown_roots..]) { + for ((id, effect), value) in self.residuals.iter().zip(&row[roots..]) { match value { SqlValue::Bool(true) => { match effect { @@ -165,17 +215,22 @@ impl<'a> SqlAuthorizer<'a> { .chain(response.residual_forbids()) .collect(); residuals.sort_by_key(|p| p.get_policy_id().to_string()); - if residuals.is_empty() { + let principal_type = EntityTypeName::ref_cast(request.principal_type()).clone(); + let resource_type = EntityTypeName::ref_cast(request.resource_type()).clone(); + let mut compiler = Compiler::new(self.config, schema, self.dialect, request); + compiler.ensure_unknown_roots(); + if residuals.is_empty() && compiler.unknown_roots().is_empty() { return Ok(CompiledAuthorization { sql: None, residuals: Vec::new(), - unknown_roots: 0, + unknown_roots: Vec::new(), + principal_type, + resource_type, true_permits, true_forbids, errors, }); } - let mut compiler = Compiler::new(self.config, schema, self.dialect, request); let mut columns = Vec::new(); let mut order = Vec::new(); for (i, policy) in residuals.iter().enumerate() { @@ -187,13 +242,38 @@ impl<'a> SqlAuthorizer<'a> { Ok(CompiledAuthorization { sql: Some(sql), residuals: order, - unknown_roots: compiler.unknown_roots().len(), + unknown_roots: compiler.unknown_roots(), + principal_type, + resource_type, true_permits, true_forbids, errors, }) } + /// Authorizes a partial `request` — an unknown principal and/or + /// resource — over the database behind `db` and the action entities + /// `entities`: one row per candidate entity of the unknown type(s) (every + /// row of its table), or per pair when both are unknown. + pub fn query( + &self, + db: &mut dyn Backend, + request: &PartialRequest, + entities: &PartialEntities, + ) -> Result> { + let compiled = self.compile(request, entities)?; + if compiled.unknown_roots.is_empty() { + return Err(Error::Request( + "the request has no unknown principal or resource; use is_authorized".into(), + )); + } + let sql = compiled + .sql + .as_ref() + .expect("an unknown root always renders a query"); + db.query(sql)?.iter().map(|row| compiled.row(row)).collect() + } + /// Authorizes a concrete `request`, with the entity data in the database /// behind `db` and the action entities in `actions`. pub fn is_authorized( @@ -205,6 +285,11 @@ impl<'a> SqlAuthorizer<'a> { let request = concrete_request(request, self.schema)?; let entities = action_entities(actions, self.schema)?; let compiled = self.compile(&request, &entities)?; + if !compiled.unknown_roots.is_empty() { + return Err(Error::Request( + "the request has an unknown principal or resource; use query".into(), + )); + } let Some(sql) = &compiled.sql else { return compiled.response(None); }; @@ -221,16 +306,31 @@ impl<'a> SqlAuthorizer<'a> { /// The partial request with everything known. pub fn concrete_request(request: &Request, schema: &Schema) -> Result { + partial_request(request, false, false, schema) +} + +/// The partial request of `request` with the principal and/or the resource +/// id dropped (their types stay known), for [`SqlAuthorizer::query`]. +pub fn partial_request( + request: &Request, + unknown_principal: bool, + unknown_resource: bool, + schema: &Schema, +) -> Result { let core = request.as_ref(); - let known = |entry: &EntityUIDEntry, what: &str| match entry { + let known = |entry: &EntityUIDEntry, what: &str, unknown: bool| match entry { EntityUIDEntry::Known { euid, .. } => Ok(PartialEntityUID { ty: euid.entity_type().clone(), - eid: Some(euid.eid().clone()), + eid: if unknown { + None + } else { + Some(euid.eid().clone()) + }, }), EntityUIDEntry::Unknown { .. } => Err(Error::Request(format!("the {what} is unknown"))), }; - let principal = known(core.principal(), "principal")?; - let resource = known(core.resource(), "resource")?; + let principal = known(core.principal(), "principal", unknown_principal)?; + let resource = known(core.resource(), "resource", unknown_resource)?; let action = match core.action() { EntityUIDEntry::Known { euid, .. } => euid.as_ref().clone(), EntityUIDEntry::Unknown { .. } => { @@ -242,7 +342,9 @@ pub fn concrete_request(request: &Request, schema: &Schema) -> Result { return Err(Error::Unsupported("a partially unknown context")); } - None => Some(Arc::new(BTreeMap::new())), + // A request without a context is one with an *unknown* context, which + // `cedar-policy` evaluates as an unknown; it is not the empty record. + None => return Err(Error::Request("the context is unknown".into())), }; PartialRequest::new(principal, action, resource, context, schema.as_ref()) .map_err(|e| Error::Request(e.to_string())) diff --git a/src/backend/postgres.rs b/src/backend/postgres.rs index 3840800..66c6cdf 100644 --- a/src/backend/postgres.rs +++ b/src/backend/postgres.rs @@ -14,9 +14,11 @@ pub struct PgBackend { impl PgBackend { /// Connects to `url` (a `postgres://` URL or a key-value connection string). pub fn connect(url: &str) -> Result { - Ok(Self { - client: Client::connect(url, NoTls)?, - }) + let mut client = Client::connect(url, NoTls)?; + // The generated SQL quotes strings with `'` doubled and treats `\` + // literally, which is only right with standard conforming strings. + client.batch_execute("SET standard_conforming_strings = on")?; + Ok(Self { client }) } /// The underlying client. diff --git a/src/compile.rs b/src/compile.rs index 4ad956d..d959da7 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -174,6 +174,19 @@ impl<'a> Compiler<'a> { } } + /// Registers a root for every unknown request variable, so that the query + /// enumerates the candidates even when no policy refers to them. + pub fn ensure_unknown_roots(&mut self) { + if self.request.principal().eid.is_none() { + let ety = self.request.principal_type().clone(); + self.ensure_root(&RootKey::Principal, &ety); + } + if self.request.resource().eid.is_none() { + let ety = self.request.resource_type().clone(); + self.ensure_root(&RootKey::Resource, &ety); + } + } + /// Compiles a residual to a boolean SQL expression. pub fn condition(&mut self, r: &Residual) -> Result { let compiled = self.expr(r)?; @@ -645,7 +658,17 @@ impl<'a> Compiler<'a> { return Err(Error::Unsupported("tags of an entity type without a table")); }; let Some(tags) = table.tags.clone() else { - return Err(Error::Unsupported("tags of an entity type without tags")); + if op == BinaryOp::HasTag { + // The validator types `hasTag` on a tagless type as `false`. + return Ok(plain( + format!( + "(CASE WHEN {} IS NULL OR {} IS NULL THEN NULL ELSE FALSE END)", + a.sql, b.sql + ), + repr, + )); + } + return Err(Error::Unsupported("getTag on an entity type without tags")); }; let (a, b) = (self.share(a), self.share(b)); let lookup = format!( diff --git a/src/load.rs b/src/load.rs index 804e00c..5c5eaee 100644 --- a/src/load.rs +++ b/src/load.rs @@ -17,6 +17,7 @@ use cedar_policy_core::ast::{ Entity, EntityType, EntityUID, Literal, PartialValue, Value, ValueKind, }; use cedar_policy_core::validator::ValidatorSchema; +use cedar_policy_core::validator::types::{EntityKind, OpenTag, Type}; use serde_json::json; use crate::config::{ @@ -86,7 +87,13 @@ pub fn entities_to_sql( "required attribute {attr} of {uid} is missing" ))); } - Some(PartialValue::Value(v)) => literal(v, cc, dialect)?, + Some(PartialValue::Value(v)) => { + if let Some(attr_type) = vet.attr(attr) { + conforms(v, &attr_type.attr_type) + .map_err(|e| Error::Load(format!("attribute {attr} of {uid}: {e}")))?; + } + literal(v, cc, dialect)? + } Some(PartialValue::Residual(_)) => { return Err(Error::Unsupported("entities with unknown attribute values")); } @@ -134,6 +141,10 @@ pub fn entities_to_sql( let PartialValue::Value(value) = value else { return Err(Error::Unsupported("entities with unknown tag values")); }; + if let Some(tag_type) = vet.tag_type() { + conforms(value, tag_type) + .map_err(|e| Error::Load(format!("tag {tag} of {uid}: {e}")))?; + } load.statements.push(format!( "INSERT INTO {} (\"{TAGS_ENTITY_ID_COLUMN}\", \"{TAGS_TAG_COLUMN}\", \"{TAGS_VALUE_COLUMN}\") VALUES ({eid}, {}, {})", tags_table.table, @@ -145,6 +156,57 @@ pub fn entities_to_sql( Ok(load) } +/// Whether `value` conforms to the validator type `ty`: the compiled queries +/// trust the static types of JSON-stored contents (entity references inside +/// sets and records are compared by id), so the loader checks them. +pub fn conforms(value: &Value, ty: &Type) -> std::result::Result<(), String> { + let mismatch = || format!("the value {value} does not conform to the type {ty}"); + match (ty, value.value_kind()) { + (Type::Bool(_), ValueKind::Lit(Literal::Bool(_))) + | (Type::Long, ValueKind::Lit(Literal::Long(_))) + | (Type::String, ValueKind::Lit(Literal::String(_))) + | (Type::Entity(EntityKind::AnyEntity), ValueKind::Lit(Literal::EntityUID(_))) + | (Type::ExtensionType { .. }, ValueKind::ExtensionValue(_)) => Ok(()), + (Type::Entity(EntityKind::Entity(lub)), ValueKind::Lit(Literal::EntityUID(uid))) => { + match lub.get_single_entity() { + Some(ety) if ety == uid.entity_type() => Ok(()), + Some(_) => Err(mismatch()), + None => Err("union entity types are not supported".to_owned()), + } + } + (Type::Set { element_type }, ValueKind::Set(set)) => match element_type { + Some(element) => set.iter().try_for_each(|v| conforms(v, element)), + None => Ok(()), + }, + ( + Type::Record { + attrs, + open_attributes, + }, + ValueKind::Record(record), + ) => { + for (key, attr) in attrs.iter() { + match record.get(key) { + Some(v) => conforms(v, &attr.attr_type)?, + None if attr.is_required => { + return Err(format!( + "the required attribute {key} is missing in {value}" + )); + } + None => {} + } + } + if *open_attributes == OpenTag::ClosedAttributes + && let Some(extra) = record.keys().find(|k| attrs.get_attr(k).is_none()) + { + return Err(format!("the attribute {extra} of {value} is not declared")); + } + Ok(()) + } + _ => Err(mismatch()), + } +} + /// `value` as a SQL literal for `column`: strings and entity references of /// the referenced type in text columns, integers, booleans, and the canonical /// JSON of sets and records. diff --git a/tests/authorize_pg.rs b/tests/authorize_pg.rs index 449b115..cae560b 100644 --- a/tests/authorize_pg.rs +++ b/tests/authorize_pg.rs @@ -105,6 +105,7 @@ const ALICE: &str = "User::\"alice\""; const BOB: &str = "User::\"bob\""; const CAROL: &str = "User::\"carol\""; const NOBODY: &str = "User::\"nobody\""; +const WEIRD: &str = "User::\"weird\\\"q\\\\s\""; const D1: &str = "Doc::\"d1\""; const GHOST_NAME: &str = "User::\"ghost\".name == \"x\""; @@ -740,3 +741,67 @@ fn sharing_keeps_queries_linear() { "{sizes:?}" ); } + +#[test] +fn review_probes() { + for closed in [true, false] { + let mut fx = Fixture::new(closed); + let allow = |fx: &mut Fixture, p: &str, principal: &str| { + assert_eq!( + fx.clean(&permit(p), principal, D1).decision, + Decision::Allow, + "{p} {principal}" + ); + }; + let deny = |fx: &mut Fixture, p: &str, principal: &str| { + assert_eq!( + fx.clean(&permit(p), principal, D1).decision, + Decision::Deny, + "{p} {principal}" + ); + }; + // Literal roots whose ids need quoting or shortening. + allow(&mut fx, &format!("{WEIRD}.name like \"50*\""), ALICE); + let long = format!("User::\"{}\"", "x".repeat(200)); + fx.errored(&permit(&format!("{long}.admin")), ALICE, D1); + deny(&mut fx, &format!("{long} in Group::\"admins\""), ALICE); + // `like` over values holding `%`, `_`, `\\` and a newline. + allow(&mut fx, "principal.name like \"50%_x\\\\y*\"", WEIRD); + deny(&mut fx, "principal.name like \"50*_x\"", WEIRD); + allow(&mut fx, "principal.name like \"*z\"", WEIRD); + allow(&mut fx, "principal.name like \"50%_x\\\\y\nz\"", WEIRD); + deny(&mut fx, "principal.name like \"50_*\"", WEIRD); + // Same-type elements in a set on the right of `in`, and `x in x`. + allow(&mut fx, "principal in [User::\"x\", User::\"bob\"]", BOB); + deny(&mut fx, "principal in [User::\"x\", User::\"bob\"]", ALICE); + allow(&mut fx, "resource.owner in resource.owner", ALICE); + allow( + &mut fx, + "principal has friend && principal.friend in principal.friend", + CAROL, + ); + // A join and an ancestors check on the same path. + allow( + &mut fx, + "principal has friend && principal.friend.name == \"Bob\" && !(principal.friend in Group::\"admins\")", + ALICE, + ); + allow(&mut fx, "principal in [principal]", ALICE); + } +} + +#[test] +fn has_tag_on_a_tagless_type() { + let mut fx = Fixture::new(true); + // `Group` declares no tags: `hasTag` is always false, an error stays one. + assert_eq!( + fx.clean(&permit("!(Group::\"admins\".hasTag(\"k\"))"), ALICE, D1) + .decision, + Decision::Allow + ); + fx.errored( + &permit("Group::\"admins\".hasTag(User::\"ghost\".name)"), + ALICE, + D1, + ); +} diff --git a/tests/entities/kitchen_sink.json b/tests/entities/kitchen_sink.json index ddea2e1..68d452c 100644 --- a/tests/entities/kitchen_sink.json +++ b/tests/entities/kitchen_sink.json @@ -17,6 +17,9 @@ "attrs": {"name": "Carol", "admin": false, "groups": ["x"], "profile": {"city": "Oslo", "pets": [7]}, "friend": {"__entity": {"type": "User", "id": "ghost"}}, "friends": []}, "parents": []}, + {"uid": {"type": "User", "id": "weird\"q\\s"}, + "attrs": {"name": "50%_x\\y\nz", "admin": false, "groups": [], "profile": {"city": "", "pets": []}, "friends": []}, + "parents": []}, {"uid": {"type": "Doc", "id": "d1"}, "attrs": {"owner": {"__entity": {"type": "User", "id": "alice"}}}, "parents": [{"type": "Group", "id": "admins"}], diff --git a/tests/hierarchy_pg.rs b/tests/hierarchy_pg.rs new file mode 100644 index 0000000..27284e9 --- /dev/null +++ b/tests/hierarchy_pg.rs @@ -0,0 +1,144 @@ +//! The recursive ancestors CTE over a hierarchy table holding only direct +//! edges — with a cycle — and the closed table holding the closure. + +use std::str::FromStr; + +use cedar_policy::{Authorizer, Context, Decision, Entities, EntityUid, PolicySet, Request}; +use cedar_sql::authorizer::SqlAuthorizer; +use cedar_sql::backend::Backend; +use cedar_sql::config::DatabaseConfiguration; +use cedar_sql::ddl::create_tables; +use cedar_sql::dialect::Postgres; +use cedar_sql::testing::SharedPostgres; + +const SCHEMA: &str = r#" + entity G in [G]; + entity U in [G] = { boss?: U }; + action a appliesTo { principal: [U], resource: [G] }; +"#; + +/// Direct edges: u1 -> g1 -> g2 -> g3, g3 -> g1 (a cycle), u2 -> g3, u1.boss = u2. +const EDGES: &[(&str, &str, &str, &str)] = &[ + ("U", "u1", "G", "g1"), + ("G", "g1", "G", "g2"), + ("G", "g2", "G", "g3"), + ("G", "g3", "G", "g1"), + ("U", "u2", "G", "g3"), +]; + +/// The same data as Cedar entities, with the closure computed by Cedar. +const ENTITIES: &str = r#"[ + {"uid": {"type": "G", "id": "g1"}, "attrs": {}, "parents": [{"type": "G", "id": "g2"}]}, + {"uid": {"type": "G", "id": "g2"}, "attrs": {}, "parents": [{"type": "G", "id": "g3"}]}, + {"uid": {"type": "G", "id": "g3"}, "attrs": {}, "parents": [{"type": "G", "id": "g1"}]}, + {"uid": {"type": "U", "id": "u1"}, "attrs": {"boss": {"__entity": {"type": "U", "id": "u2"}}}, "parents": [{"type": "G", "id": "g1"}]}, + {"uid": {"type": "U", "id": "u2"}, "attrs": {}, "parents": [{"type": "G", "id": "g3"}]}, + {"uid": {"type": "Action", "id": "a"}, "attrs": {}, "parents": []} +]"#; + +#[test] +fn direct_edges_with_a_cycle() { + let (mut config, schema) = DatabaseConfiguration::from_cedarschema_str(SCHEMA).unwrap(); + config.emit_foreign_keys = false; + config.hierarchy_closed = false; + // Cedar rejects cycles when computing the closure, so the oracle uses a + // hierarchy without the closing edge; the SQL side gets the cycle. + let entities = Entities::from_json_str( + &ENTITIES.replace( + r#"[{"type": "G", "id": "g1"}]}, + {"uid": {"type": "U", "id": "u1"}"#, + r#"[]}, + {"uid": {"type": "U", "id": "u1"}"#, + ), + Some(&schema), + ) + .unwrap(); + let mut db = SharedPostgres::get().unwrap().connect().unwrap(); + db.begin().unwrap(); + for statement in create_tables(&config, &Postgres).unwrap() { + db.execute_batch(&statement).unwrap(); + } + for id in ["g1", "g2", "g3"] { + db.execute_batch(&format!( + "INSERT INTO \"G\" (\"__entity_id\") VALUES ('{id}')" + )) + .unwrap(); + } + db.execute_batch( + "INSERT INTO \"U\" (\"__entity_id\", \"boss\") VALUES ('u1', 'u2'), ('u2', NULL)", + ) + .unwrap(); + for (dt, di, at, ai) in EDGES { + db.execute_batch(&format!( + "INSERT INTO \"cedar_entity_hierarchy\" VALUES ('{dt}', '{di}', '{at}', '{ai}')" + )) + .unwrap(); + } + // (condition, principal, resource, expected, whether Cedar's cycle-free + // closure gives the same answer) + let cases = [ + ("principal in G::\"g1\"", "u1", "g1", Decision::Allow, true), + ("principal in G::\"g3\"", "u1", "g1", Decision::Allow, true), + ("principal in G::\"g1\"", "u2", "g1", Decision::Allow, false), + ("principal in G::\"nope\"", "u1", "g1", Decision::Deny, true), + ("resource in G::\"g1\"", "u1", "g2", Decision::Allow, false), + ("resource in resource", "u1", "g2", Decision::Allow, true), + ( + "principal has boss && principal.boss in G::\"g2\"", + "u1", + "g1", + Decision::Allow, + false, + ), + ( + "principal has boss && principal.boss in G::\"g2\"", + "u2", + "g1", + Decision::Deny, + true, + ), + ( + "principal in [G::\"x\", G::\"g2\"]", + "u1", + "g1", + Decision::Allow, + true, + ), + ("U::\"u2\" in G::\"g2\"", "u1", "g1", Decision::Allow, false), + ( + "U::\"nobody\" in G::\"g2\"", + "u1", + "g1", + Decision::Deny, + true, + ), + ]; + for (condition, principal, resource, expected, cedar_agrees) in cases { + let policies = PolicySet::from_str(&format!( + "permit(principal, action, resource) when {{ {condition} }};" + )) + .unwrap(); + let request = Request::new( + EntityUid::from_str(&format!("U::\"{principal}\"")).unwrap(), + EntityUid::from_str("Action::\"a\"").unwrap(), + EntityUid::from_str(&format!("G::\"{resource}\"")).unwrap(), + Context::empty(), + Some(&schema), + ) + .unwrap(); + let response = SqlAuthorizer::new(&schema, &config, &Postgres, &policies) + .unwrap() + .is_authorized(&mut db, &request, &entities) + .unwrap_or_else(|e| panic!("{condition}: {e}")); + assert_eq!( + response.decision, expected, + "{condition} {principal} {resource}" + ); + assert!(response.errors.is_empty(), "{condition}"); + if cedar_agrees { + let cedar = Authorizer::new().is_authorized(&request, &policies, &entities); + assert_eq!(cedar.decision(), expected, "cedar {condition} {principal}"); + } + } + db.rollback().unwrap(); +} diff --git a/tests/load_pg.rs b/tests/load_pg.rs index 41fe01d..981f4ae 100644 --- a/tests/load_pg.rs +++ b/tests/load_pg.rs @@ -95,6 +95,17 @@ fn round_trip() { text("ghost"), SqlValue::Json(json!([])), ], + vec![ + text("weird\"q\\s"), + text("User"), + text("50%_x\\y\nz"), + SqlValue::Null, + SqlValue::Bool(false), + SqlValue::Json(json!([])), + SqlValue::Json(json!({"r": {"city": "", "pets": []}})), + SqlValue::Null, + SqlValue::Json(json!([])), + ], ] ); let hierarchy = db @@ -231,21 +242,35 @@ fn rejects_bad_entities() { // do not mix. assert!( case(r#"[{"uid": {"type": "Doc", "id": "d"}, "attrs": {"owner": {"__entity": {"type": "Group", "id": "g"}}}, "parents": []}]"#) - .contains("referencing User") + .contains("does not conform") ); assert!( case( r#"[{"uid": {"type": "Doc", "id": "d"}, "attrs": {"owner": "alice"}, "parents": []}]"# ) - .contains("referencing User") + .contains("does not conform") ); assert!( case(r#"[{"uid": {"type": "Group", "id": "g"}, "attrs": {}, "parents": [], "tags": {}}, {"uid": {"type": "User", "id": "u"}, "attrs": {"name": {"__entity": {"type": "User", "id": "x"}}, "admin": true, "groups": [], "profile": {"city": "", "pets": []}, "friends": []}, "parents": []}]"#) - .contains("cannot be stored in a column of type Text") + .contains("does not conform") ); assert!( case(r#"[{"uid": {"type": "Doc", "id": "d"}, "attrs": {"owner": {"__entity": {"type": "User", "id": "u"}}}, "parents": [], "tags": {"t": {"__entity": {"type": "Group", "id": "g"}}}}]"#) - .contains("referencing User") + .contains("does not conform") + ); + // JSON-stored contents must conform to their type: an entity of the + // wrong type inside a record or a set, an undeclared record attribute. + assert!( + case(r#"[{"uid": {"type": "User", "id": "u"}, "attrs": {"name": "x", "admin": true, "groups": [], "profile": {"city": "", "pets": [], "boss": {"__entity": {"type": "Group", "id": "g"}}}, "friends": []}, "parents": []}]"#) + .contains("does not conform") + ); + assert!( + case(r#"[{"uid": {"type": "User", "id": "u"}, "attrs": {"name": "x", "admin": true, "groups": [], "profile": {"city": "", "pets": []}, "friends": [{"__entity": {"type": "Group", "id": "g"}}]}, "parents": []}]"#) + .contains("does not conform") + ); + assert!( + case(r#"[{"uid": {"type": "User", "id": "u"}, "attrs": {"name": "x", "admin": true, "groups": [], "profile": {"city": "", "pets": [], "extra": 1}, "friends": []}, "parents": []}]"#) + .contains("not declared") ); // A NUL character, which Postgres cannot store, is rejected up front. let nul = Entity::new_no_attrs( diff --git a/tests/query_pg.rs b/tests/query_pg.rs new file mode 100644 index 0000000..634c3b1 --- /dev/null +++ b/tests/query_pg.rs @@ -0,0 +1,327 @@ +//! Partial requests: one row per candidate, against brute-force concrete +//! authorization of every candidate and against `PolicySet::query_resource`. + +use std::collections::{BTreeMap, BTreeSet}; +use std::str::FromStr; + +use cedar_policy::{ + Authorizer, Context, Decision, Entities, EntityUid, PolicyId, PolicySet, Request, + ResourceQueryRequest, Validator, +}; +use cedar_sql::authorizer::{QueryRow, Response, SqlAuthorizer, action_entities, partial_request}; +use cedar_sql::backend::Backend; +use cedar_sql::backend::postgres::PgBackend; +use cedar_sql::config::DatabaseConfiguration; +use cedar_sql::ddl::create_tables; +use cedar_sql::dialect::Postgres; +use cedar_sql::load::entities_to_sql; +use cedar_sql::testing::SharedPostgres; + +struct Fixture { + schema: cedar_policy::Schema, + config: DatabaseConfiguration, + entities: Entities, + db: PgBackend, +} + +impl Fixture { + fn new(closed: bool) -> Self { + let src = std::fs::read_to_string("tests/schemas/kitchen_sink.cedarschema").unwrap(); + let (mut config, schema) = DatabaseConfiguration::from_cedarschema_str(&src).unwrap(); + config.emit_foreign_keys = false; + config.hierarchy_closed = closed; + let json = std::fs::read_to_string("tests/entities/kitchen_sink.json").unwrap(); + let entities = Entities::from_json_str(&json, Some(&schema)).unwrap(); + let mut db = SharedPostgres::get().unwrap().connect().unwrap(); + db.begin().unwrap(); + for statement in create_tables(&config, &Postgres).unwrap() { + db.execute_batch(&statement).unwrap(); + } + let load = entities_to_sql(&entities, &schema, &config, &Postgres).unwrap(); + for statement in &load.statements { + db.execute_batch(statement).unwrap(); + } + Self { + schema, + config, + entities, + db, + } + } + + fn request(&self, principal: &str, resource: &str) -> Request { + Request::new( + EntityUid::from_str(principal).unwrap(), + EntityUid::from_str("Action::\"view\"").unwrap(), + EntityUid::from_str(resource).unwrap(), + Context::from_json_str(r#"{"ip": "1.2.3.4"}"#, None).unwrap(), + Some(&self.schema), + ) + .unwrap() + } + + fn expected(&self, request: &Request, policies: &PolicySet) -> Response { + let response = Authorizer::new().is_authorized(request, policies, &self.entities); + Response { + decision: response.decision(), + reason: response.diagnostics().reason().cloned().collect(), + errors: response + .diagnostics() + .errors() + .map(|e| match e { + cedar_policy::AuthorizationError::PolicyEvaluationError(e) => { + e.policy_id().clone() + } + }) + .collect(), + } + } + + /// Runs the partial query and checks every row against the concrete + /// authorizer, and the row set against the entities of the unknown types. + fn check( + &mut self, + policies: &str, + principal: &str, + resource: &str, + unknown_principal: bool, + unknown_resource: bool, + ) -> Vec { + let policies = PolicySet::from_str(policies).unwrap(); + let request = self.request(principal, resource); + let partial = + partial_request(&request, unknown_principal, unknown_resource, &self.schema).unwrap(); + let actions = action_entities(&self.entities, &self.schema).unwrap(); + let rows = SqlAuthorizer::new(&self.schema, &self.config, &Postgres, &policies) + .unwrap() + .query(&mut self.db, &partial, &actions) + .unwrap_or_else(|e| panic!("{policies}\n{e}")); + let candidates = |unknown: bool, uid: &EntityUid| -> Vec { + if unknown { + self.entities + .iter() + .map(|e| e.uid()) + .filter(|u| u.type_name() == uid.type_name()) + .collect() + } else { + vec![uid.clone()] + } + }; + let principals = candidates(unknown_principal, request.principal().unwrap()); + let resources = candidates(unknown_resource, request.resource().unwrap()); + let mut expected = BTreeMap::new(); + for p in &principals { + for r in &resources { + let concrete = Request::new( + p.clone(), + request.action().unwrap().clone(), + r.clone(), + request.context().unwrap().clone(), + None, + ) + .unwrap(); + expected.insert( + (p.to_string(), r.to_string()), + self.expected(&concrete, &policies), + ); + } + } + let actual: BTreeMap<(String, String), Response> = rows + .iter() + .map(|row| { + let p = row + .principal + .clone() + .unwrap_or_else(|| request.principal().unwrap().clone()); + let r = row + .resource + .clone() + .unwrap_or_else(|| request.resource().unwrap().clone()); + ((p.to_string(), r.to_string()), row.response.clone()) + }) + .collect(); + assert_eq!(actual.len(), rows.len(), "duplicate rows for {policies}"); + assert_eq!(actual, expected, "{policies}"); + rows + } +} + +const ALICE: &str = "User::\"alice\""; +const D1: &str = "Doc::\"d1\""; + +fn permit(condition: &str) -> String { + format!("permit(principal, action, resource) when {{ {condition} }};") +} + +fn allowed(rows: &[QueryRow]) -> BTreeSet { + rows.iter() + .filter(|r| r.response.decision == Decision::Allow) + .map(|r| { + r.principal + .clone() + .or_else(|| r.resource.clone()) + .unwrap() + .to_string() + }) + .collect() +} + +#[test] +fn unknown_principal() { + for closed in [true, false] { + let mut fx = Fixture::new(closed); + let rows = fx.check( + &permit("principal.name like \"A*\""), + ALICE, + D1, + true, + false, + ); + assert_eq!(rows.len(), 4); + assert_eq!( + allowed(&rows), + BTreeSet::from(["User::\"alice\"".to_owned()]) + ); + let rows = fx.check( + &permit("principal in Group::\"admins\""), + ALICE, + D1, + true, + false, + ); + assert_eq!( + allowed(&rows), + BTreeSet::from(["User::\"alice\"".to_owned()]) + ); + // Errors per row: carol's friend does not exist. + let rows = fx.check( + &permit("principal has friend && principal.friend.name == \"Bob\""), + ALICE, + D1, + true, + false, + ); + let errors: BTreeSet = rows + .iter() + .filter(|r| !r.response.errors.is_empty()) + .map(|r| r.principal.clone().unwrap().to_string()) + .collect(); + assert_eq!(errors, BTreeSet::from(["User::\"carol\"".to_owned()])); + assert_eq!( + allowed(&rows), + BTreeSet::from(["User::\"alice\"".to_owned()]) + ); + // Sets, tags and computed entities under an unknown root. + fx.check( + &permit("principal.groups.contains(\"a\")"), + ALICE, + D1, + true, + false, + ); + fx.check( + &permit("principal.hasTag(\"k\") && principal.getTag(\"k\") == \"v\""), + ALICE, + D1, + true, + false, + ); + fx.check( + &permit("principal in resource.owner || resource.owner in principal"), + ALICE, + D1, + true, + false, + ); + fx.check( + &permit("principal has friend && principal.friend in Group::\"admins\""), + ALICE, + D1, + true, + false, + ); + } +} + +#[test] +fn unknown_resource_and_both() { + let mut fx = Fixture::new(true); + let rows = fx.check( + &permit("resource in Group::\"admins\""), + ALICE, + D1, + false, + true, + ); + assert_eq!(rows.len(), 1); + // No residual at all: every candidate gets the folded decision. + let rows = fx.check( + "permit(principal, action, resource);", + ALICE, + D1, + false, + true, + ); + assert_eq!(allowed(&rows).len(), 1); + let rows = fx.check( + "permit(principal == User::\"zzz\", action, resource);", + ALICE, + D1, + false, + true, + ); + assert!(allowed(&rows).is_empty() && rows.len() == 1); + // Both unknown: one row per pair. + let rows = fx.check( + &permit("principal.name == \"Alice\" && resource.owner == principal"), + ALICE, + D1, + true, + true, + ); + assert_eq!(rows.len(), 4); + let rows = fx.check( + "permit(principal, action, resource);", + ALICE, + D1, + true, + true, + ); + assert_eq!(rows.len(), 4); + // A forbid over both. + fx.check( + &format!( + "{}\nforbid(principal, action, resource) when {{ principal.name == \"Bob\" || resource.owner == principal }};", + permit("true") + ), + ALICE, + D1, + true, + true, + ); +} + +#[test] +fn agrees_with_query_resource() { + let mut fx = Fixture::new(true); + let policies = permit("resource.owner == principal || resource in Group::\"admins\""); + let rows = fx.check(&policies, ALICE, D1, false, true); + let policy_set = PolicySet::from_str(&policies).unwrap(); + let validator = Validator::new(fx.schema.clone()); + let request = fx.request(ALICE, D1); + let query = ResourceQueryRequest::new( + request.principal().unwrap().clone(), + request.action().unwrap().clone(), + request.resource().unwrap().type_name().clone(), + request.context().unwrap().clone(), + validator.schema(), + ) + .unwrap(); + let expected: BTreeSet = policy_set + .query_resource(&query, &fx.entities, validator.schema()) + .unwrap() + .map(|u| u.to_string()) + .collect(); + assert_eq!(allowed(&rows), expected); + let _ = PolicyId::new("x"); +} From a4e3a7e8c094c40788b0c1fcf2d1ece9cb6721f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20K=C3=A4ldstr=C3=B6m?= Date: Sun, 13 Sep 2026 23:27:07 +0300 Subject: [PATCH 3/3] Follow-ups for branch 5: query (review) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01JSRPugoPu8zLyykBCW6QJh --- docs/followups/5-query.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 docs/followups/5-query.md diff --git a/docs/followups/5-query.md b/docs/followups/5-query.md new file mode 100644 index 0000000..15d6fbf --- /dev/null +++ b/docs/followups/5-query.md @@ -0,0 +1,20 @@ +# Follow-ups for branch 5 — query (review) + +## PR description + +Partial requests: an unknown principal and/or resource yields one row per candidate, each with the decision, +determining and erroring policies of the concrete request it stands for (`docs/plans/5-query.md`), plus the +fixes of the branch 3 review. + +## Review findings + +No semantic bug: the row decoding, the unknown roots, `partial_request`, the `conforms` check and the zero-row +case all matched `cedar-policy` and `PolicySet::query_resource`. Findings, fixed in branch 6: the test suite +lacked a same-type principal and resource both unknown, a candidate type without rows, an enum type and +attribute names with dots; the crate signals NUL strings with a message the harness matched by substring. +Design note: the candidates of an enum entity type are its table rows, not its declared values, as for +`PolicySet::query_resource`. + +## Suggested follow-ups + +- Partial context (out of scope per the README) is the remaining unknown a request could carry.