Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFI nits #837

Merged
merged 14 commits into from
Jun 4, 2024
1 change: 1 addition & 0 deletions cedar-policy-core/src/authorizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ impl Response {
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy)]
#[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
#[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
#[serde(rename_all = "camelCase")]
pub enum Decision {
/// The `Authorizer` determined that the request should be allowed
Allow,
Expand Down
4 changes: 4 additions & 0 deletions cedar-policy-core/src/jsonvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ use serde::{Deserialize, Serialize};
/// Wrapper around `serde_json::Value`, with a different `Deserialize`
/// implementation, such that duplicate keys in JSON objects (maps/records) are
/// not allowed (result in an error).
//
// CAUTION: this type is publicly exported in `cedar-policy`.
// Don't make fields `pub`, don't make breaking changes, and use caution
// when adding public methods.
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub struct JsonValueWithNoDuplicateKeys(serde_json::Value);

Expand Down
6 changes: 5 additions & 1 deletion cedar-policy/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use cedar_policy_validator::RequestValidationError; // this type is unsuitable f
use itertools::{Either, Itertools};
use miette::Diagnostic;
use ref_cast::RefCast;
use serde::{Deserialize, Serialize};
use smol_str::SmolStr;
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
use std::io::Read;
Expand Down Expand Up @@ -1165,7 +1166,10 @@ impl From<authorizer::Response> for Response {
}

/// Used to select how a policy will be validated.
#[derive(Default, Eq, PartialEq, Copy, Clone, Debug)]
#[derive(Default, Eq, PartialEq, Copy, Clone, Debug, Serialize, Deserialize)]
#[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
#[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
#[serde(rename_all = "camelCase")]
#[non_exhaustive]
Copy link
Contributor

@john-h-kastner-aws john-h-kastner-aws May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the experimental modes visible in the ffi api where they weren't before. That probably a good thing, but worth thinking about whether that could go wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't break anything since the WASM code directly uses the Rust FFI definitions. It should just silently add support for the new validation modes if the cedar_policy crate is compiled with the appropriate flags. What other complications were you thinking of?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they should be visible in the FFI.

pub enum ValidationMode {
/// Validate that policies do not contain any type errors, and additionally
Expand Down
36 changes: 17 additions & 19 deletions cedar-policy/src/ffi/is_authorized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,15 @@
//! This module contains the `is_authorized` entry points that other language
//! FFIs can call
#![allow(clippy::module_name_repetitions)]
use super::utils::{DetailedError, PolicySet, Schema, WithWarnings};
use super::utils::{DetailedError, JsonValueWithNoDuplicateKeys, PolicySet, Schema, WithWarnings};
use crate::{
Authorizer, Context, Decision, Entities, EntityId, EntityTypeName, EntityUid, PolicyId,
Request, SlotId,
};
use cedar_policy_core::jsonvalue::JsonValueWithNoDuplicateKeys;
use itertools::Itertools;
use miette::{miette, Diagnostic, WrapErr};
use serde::{Deserialize, Serialize};
use serde_with::{serde_as, MapPreventDuplicates};
use smol_str::{SmolStr, ToSmolStr};
use std::collections::{HashMap, HashSet};
#[cfg(feature = "partial-eval")]
use std::convert::Infallible;
Expand Down Expand Up @@ -154,7 +152,7 @@ pub struct Response {
#[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
#[serde(rename_all = "camelCase")]
pub struct Diagnostics {
/// `PolicyId`s of the policies that contributed to the decision.
/// Ids of the policies that contributed to the decision.
/// If no policies applied to the request, this set will be empty.
#[cfg_attr(feature = "wasm", tsify(type = "Set<String>"))]
reason: HashSet<PolicyId>,
Expand Down Expand Up @@ -225,7 +223,7 @@ impl Diagnostics {
#[serde(rename_all = "camelCase")]
pub struct AuthorizationError {
/// Id of the policy where the error (or warning) occurred
pub policy_id: SmolStr,
pub policy_id: PolicyId,
/// Error (or warning).
/// You can look at the `severity` field to see whether it is actually an
/// error or a warning.
Expand All @@ -235,14 +233,14 @@ pub struct AuthorizationError {
impl AuthorizationError {
/// Create an `AuthorizationError` from a policy ID and any `miette` error
pub fn new(
policy_id: impl Into<SmolStr>,
policy_id: impl Into<PolicyId>,
error: impl miette::Diagnostic + Send + Sync + 'static,
) -> Self {
Self::new_from_report(policy_id, miette::Report::new(error))
}

/// Create an `AuthorizationError` from a policy ID and a `miette::Report`
pub fn new_from_report(policy_id: impl Into<SmolStr>, report: miette::Report) -> Self {
pub fn new_from_report(policy_id: impl Into<PolicyId>, report: miette::Report) -> Self {
Self {
policy_id: policy_id.into(),
error: report.into(),
Expand All @@ -254,7 +252,7 @@ impl From<crate::AuthorizationError> for AuthorizationError {
fn from(e: crate::AuthorizationError) -> Self {
match e {
crate::AuthorizationError::PolicyEvaluationError(e) => {
Self::new(e.id().to_smolstr(), e.into_inner())
Self::new(e.id().clone(), e.into_inner())
}
}
}
Expand All @@ -280,7 +278,7 @@ pub struct ResidualResponse {
errored: HashSet<PolicyId>,
may_be_determining: HashSet<PolicyId>,
must_be_determining: HashSet<PolicyId>,
residuals: HashMap<PolicyId, serde_json::Value>,
residuals: HashMap<PolicyId, JsonValueWithNoDuplicateKeys>,
nontrivial_residuals: HashSet<PolicyId>,
}

Expand Down Expand Up @@ -312,22 +310,22 @@ impl ResidualResponse {
}

/// (Borrowed) Iterator over the set of residual policies
pub fn residuals(&self) -> impl Iterator<Item = &serde_json::Value> {
pub fn residuals(&self) -> impl Iterator<Item = &JsonValueWithNoDuplicateKeys> {
self.residuals.values()
}

/// (Owned) Iterator over the set of residual policies
pub fn into_residuals(self) -> impl Iterator<Item = serde_json::Value> {
pub fn into_residuals(self) -> impl Iterator<Item = JsonValueWithNoDuplicateKeys> {
self.residuals.into_values()
}

/// Get the residual policy for a specified [`PolicyId`] if it exists
pub fn residual(&self, p: &PolicyId) -> Option<&serde_json::Value> {
/// Get the residual policy for a specified id if it exists
pub fn residual(&self, p: &PolicyId) -> Option<&JsonValueWithNoDuplicateKeys> {
self.residuals.get(p)
}

/// (Borrowed) Iterator over the set of non-trivial residual policies
pub fn nontrivial_residuals(&self) -> impl Iterator<Item = &serde_json::Value> {
pub fn nontrivial_residuals(&self) -> impl Iterator<Item = &JsonValueWithNoDuplicateKeys> {
self.residuals.iter().filter_map(|(id, policy)| {
if self.nontrivial_residuals.contains(id) {
Some(policy)
Expand Down Expand Up @@ -369,7 +367,7 @@ impl TryFrom<crate::PartialResponse> for ResidualResponse {
.collect(),
residuals: partial_response
.all_residuals()
.map(|e| e.to_json().map(|json| (e.id().clone(), json)))
.map(|e| e.to_json().map(|json| (e.id().clone(), json.into())))
.collect::<Result<_, _>>()?,
})
}
Expand Down Expand Up @@ -467,7 +465,7 @@ pub struct AuthorizationCall {
/// parsing of `context`, and not for request validation.
/// If a schema is not provided, this option has no effect.
#[serde(default = "constant_true")]
enable_request_validation: bool,
validate_request: bool,
/// The slice containing entities and policies
slice: RecvdSlice,
}
Expand Down Expand Up @@ -556,7 +554,7 @@ impl AuthorizationCall {
action,
resource,
context,
if self.enable_request_validation {
if self.validate_request {
schema.as_ref()
} else {
None
Expand Down Expand Up @@ -625,7 +623,7 @@ impl AuthorizationCall {
b = b.resource(resource);
}
b = b.context(context);
let request = if self.enable_request_validation {
let request = if self.validate_request {
match schema.as_ref() {
Some(schema_ref) => match b.schema(schema_ref).build() {
Ok(req) => Some(req),
Expand Down Expand Up @@ -1857,7 +1855,7 @@ mod test {
"schema": {
"human": "entity User, Photo; action view appliesTo { principal: User, resource: Photo };"
},
"enableRequestValidation": false,
"validateRequest": false,
});

assert_is_authorized_json(good_call);
Expand Down
8 changes: 5 additions & 3 deletions cedar-policy/src/ffi/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@

//! Utility functions and types for JSON interface
use crate::{Policy, SchemaWarning, Template};
use cedar_policy_core::jsonvalue::JsonValueWithNoDuplicateKeys;
use miette::WrapErr;
use serde::{Deserialize, Serialize};
use std::{collections::HashMap, str::FromStr};

// Publicly expose the `JsonValueWithNoDuplicateKeys` type so that the
// `*_json_str` APIs will correctly error if the input JSON string contains
// duplicate keys.
pub use cedar_policy_core::jsonvalue::JsonValueWithNoDuplicateKeys;

#[cfg(feature = "wasm")]
extern crate tsify;

Expand Down Expand Up @@ -237,10 +241,8 @@ impl PolicySet {
#[serde(rename_all = "camelCase")]
pub enum Schema {
/// Schema in the Cedar schema format. See <https://docs.cedarpolicy.com/schema/human-readable-schema.html>
#[serde(rename = "human")]
Human(String),
/// Schema in Cedar's JSON schema format. See <https://docs.cedarpolicy.com/schema/json-schema.html>
#[serde(rename = "json")]
Json(JsonValueWithNoDuplicateKeys),
}

Expand Down
Loading
Loading