Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
20 changes: 20 additions & 0 deletions docs/followups/5-query.md
Original file line number Diff line number Diff line change
@@ -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.
45 changes: 45 additions & 0 deletions docs/plans/5-query.md
Original file line number Diff line number Diff line change
@@ -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.
142 changes: 122 additions & 20 deletions src/authorizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -39,36 +40,85 @@ pub struct Response {
pub errors: BTreeSet<PolicyId>,
}

/// 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<EntityUid>,
/// The resource, when it was unknown.
pub resource: Option<EntityUid>,
/// 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<String>,
/// 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<RootKey>,
principal_type: EntityTypeName,
resource_type: EntityTypeName,
true_permits: BTreeSet<PolicyId>,
true_forbids: BTreeSet<PolicyId>,
errors: BTreeSet<PolicyId>,
}

impl CompiledAuthorization {
/// The row's candidates and response.
pub fn row(&self, row: &Row) -> Result<QueryRow> {
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<Response> {
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 {
Expand Down Expand Up @@ -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() {
Expand All @@ -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<Vec<QueryRow>> {
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(
Expand All @@ -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);
};
Expand All @@ -221,16 +306,31 @@ impl<'a> SqlAuthorizer<'a> {

/// The partial request with everything known.
pub fn concrete_request(request: &Request, schema: &Schema) -> Result<PartialRequest> {
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<PartialRequest> {
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 { .. } => {
Expand All @@ -242,7 +342,9 @@ pub fn concrete_request(request: &Request, schema: &Schema) -> Result<PartialReq
Some(Context::RestrictedResidual(_)) => {
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()))
Expand Down
8 changes: 5 additions & 3 deletions src/backend/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self> {
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.
Expand Down
25 changes: 24 additions & 1 deletion src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {
let compiled = self.expr(r)?;
Expand Down Expand Up @@ -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!(
Expand Down
Loading