FFI nits

cedar-policy-core/src/authorizer.rs

@@ -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,

cedar-policy/CHANGELOG.md

@@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - for the `partial-eval` experimental feature: `PartialResponse` api has changed significantly
 - Moved `<PolicyId as FromStr>::Err` to `Infallible` (#588, resolving #551)
 - Overhauled the FFI interface in the `frontend` module, and renamed it to
-  `ffi`; see #757. (#760, #793, #794, #800, #824, more coming)
+  `ffi`; see #757. (#760, #793, #794, #800, #824, #837, more coming)
 - Much richer error information in the FFI interface (#800)
 - Deprecated the integration testing harness code. It will be removed from the
   `cedar-policy` crate in the next major version.

cedar-policy/src/api.rs

@@ -50,6 +50,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::str::FromStr;
@@ -1060,7 +1061,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]
 pub enum ValidationMode {
     /// Validate that policies do not contain any type errors, and additionally

cedar-policy/src/ffi/is_authorized.rs

@@ -22,7 +22,6 @@ 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};
@@ -154,10 +153,10 @@ 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>,
+    #[cfg_attr(feature = "wasm", tsify(type = "Set<String>"))] // REVIEW: still needed?
+    reason: HashSet<SmolStr>,
     /// Set of errors that occurred
     errors: HashSet<AuthorizationError>,
 }
@@ -166,7 +165,7 @@ impl Response {
     /// Construct a `Response`
     pub fn new(
         decision: Decision,
-        reason: HashSet<PolicyId>,
+        reason: HashSet<SmolStr>,
         errors: HashSet<AuthorizationError>,
     ) -> Self {
         Self {
@@ -191,7 +190,7 @@ impl From<crate::Response> for Response {
         let (reason, errors) = response.diagnostics.into_components();
         Self::new(
             response.decision,
-            reason.collect(),
+            reason.map(|id| id.to_smolstr()).collect(),
             errors.map(Into::into).collect(),
         )
     }
@@ -208,7 +207,7 @@ impl TryFrom<crate::PartialResponse> for Response {
 
 impl Diagnostics {
     /// Get the policies that contributed to the decision
-    pub fn reason(&self) -> impl Iterator<Item = &PolicyId> {
+    pub fn reason(&self) -> impl Iterator<Item = &SmolStr> {
         self.reason.iter()
     }
 
@@ -254,7 +253,7 @@ impl From<crate::AuthorizationError> for AuthorizationError {
     fn from(e: crate::AuthorizationError) -> Self {
         match e {
             crate::AuthorizationError::PolicyEvaluationError { id, error } => {
-                AuthorizationError::new(id.to_smolstr(), error)
+                Self::new(id.to_smolstr(), error)
             }
         }
     }
@@ -276,12 +275,12 @@ impl From<cedar_policy_core::authorizer::AuthorizationError> for AuthorizationEr
 #[serde(rename_all = "camelCase")]
 pub struct ResidualResponse {
     decision: Option<Decision>,
-    satisfied: HashSet<PolicyId>,
-    errored: HashSet<PolicyId>,
-    may_be_determining: HashSet<PolicyId>,
-    must_be_determining: HashSet<PolicyId>,
-    residuals: HashMap<PolicyId, serde_json::Value>,
-    nontrivial_residuals: HashSet<PolicyId>,
+    satisfied: HashSet<SmolStr>,
+    errored: HashSet<SmolStr>,
+    may_be_determining: HashSet<SmolStr>,
+    must_be_determining: HashSet<SmolStr>,
+    residuals: HashMap<SmolStr, serde_json::Value>,
+    nontrivial_residuals: HashSet<SmolStr>,
 }
 
 #[cfg(feature = "partial-eval")]
@@ -292,22 +291,22 @@ impl ResidualResponse {
     }
 
     /// Set of all satisfied policy Ids
-    pub fn satisfied(&self) -> impl Iterator<Item = &PolicyId> {
+    pub fn satisfied(&self) -> impl Iterator<Item = &SmolStr> {
         self.satisfied.iter()
     }
 
     /// Set of all policy ids for policies that errored
-    pub fn errored(&self) -> impl Iterator<Item = &PolicyId> {
+    pub fn errored(&self) -> impl Iterator<Item = &SmolStr> {
         self.errored.iter()
     }
 
     /// Over approximation of policies that determine the auth decision
-    pub fn may_be_determining(&self) -> impl Iterator<Item = &PolicyId> {
+    pub fn may_be_determining(&self) -> impl Iterator<Item = &SmolStr> {
         self.may_be_determining.iter()
     }
 
     /// Under approximation of policies that determine the auth decision
-    pub fn must_be_determining(&self) -> impl Iterator<Item = &PolicyId> {
+    pub fn must_be_determining(&self) -> impl Iterator<Item = &SmolStr> {
         self.must_be_determining.iter()
     }
 
@@ -321,8 +320,8 @@ impl ResidualResponse {
         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: &SmolStr) -> Option<&serde_json::Value> {
         self.residuals.get(p)
     }
 
@@ -338,7 +337,7 @@ impl ResidualResponse {
     }
 
     ///  Iterator over the set of non-trivial residual policy ids
-    pub fn nontrivial_residual_ids(&self) -> impl Iterator<Item = &PolicyId> {
+    pub fn nontrivial_residual_ids(&self) -> impl Iterator<Item = &SmolStr> {
         self.nontrivial_residuals.iter()
     }
 }
@@ -352,24 +351,27 @@ impl TryFrom<crate::PartialResponse> for ResidualResponse {
             decision: partial_response.decision(),
             satisfied: partial_response
                 .definitely_satisfied()
-                .map(|p| p.id().clone())
+                .map(|p| p.id().to_smolstr())
+                .collect(),
+            errored: partial_response
+                .definitely_errored()
+                .map(ToSmolStr::to_smolstr)
                 .collect(),
-            errored: partial_response.definitely_errored().cloned().collect(),
             may_be_determining: partial_response
                 .may_be_determining()
-                .map(|p| p.id().clone())
+                .map(|p| p.id().to_smolstr())
                 .collect(),
             must_be_determining: partial_response
                 .must_be_determining()
-                .map(|p| p.id().clone())
+                .map(|p| p.id().to_smolstr())
                 .collect(),
             nontrivial_residuals: partial_response
                 .nontrivial_residuals()
-                .map(|p| p.id().clone())
+                .map(|p| p.id().to_smolstr())
                 .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().to_smolstr(), json)))
                 .collect::<Result<_, _>>()?,
         })
     }
@@ -444,18 +446,18 @@ pub enum PartialAuthorizationAnswer {
 pub struct AuthorizationCall {
     /// The principal taking action
     #[cfg_attr(feature = "wasm", tsify(type = "string|{type: string, id: string}"))]
-    principal: Option<JsonValueWithNoDuplicateKeys>,
+    principal: Option<serde_json::Value>,
     /// The action the principal is taking
     #[cfg_attr(feature = "wasm", tsify(type = "string|{type: string, id: string}"))]
-    action: JsonValueWithNoDuplicateKeys,
+    action: serde_json::Value,
     /// The resource being acted on by the principal
     #[cfg_attr(feature = "wasm", tsify(type = "string|{type: string, id: string}"))]
-    resource: Option<JsonValueWithNoDuplicateKeys>,
+    resource: Option<serde_json::Value>,
     /// The context details specific to the request
     #[serde_as(as = "MapPreventDuplicates<_, _>")]
     #[cfg_attr(feature = "wasm", tsify(type = "Record<string, CedarValueJson>"))]
     /// The context details specific to the request
-    context: HashMap<String, JsonValueWithNoDuplicateKeys>,
+    context: HashMap<String, serde_json::Value>,
     /// Optional schema.
     /// If present, this will inform the parsing: for instance, it will allow
     /// `__entity` and `__extn` escapes to be implicit, and it will error if
@@ -479,11 +481,11 @@ fn constant_true() -> bool {
 /// Parses the given JSON into an [`EntityUid`], or if it fails, adds an
 /// appropriate error to `errs` and returns `None`.
 fn parse_entity_uid(
-    entity_uid_json: JsonValueWithNoDuplicateKeys,
+    entity_uid_json: serde_json::Value,
     category: &str,
     errs: &mut Vec<miette::Report>,
 ) -> Option<EntityUid> {
-    match EntityUid::from_json(entity_uid_json.into())
+    match EntityUid::from_json(entity_uid_json)
         .wrap_err_with(|| format!("Failed to parse {category}"))
     {
         Ok(euid) => Some(euid),
@@ -497,7 +499,7 @@ fn parse_entity_uid(
 /// Parses the given JSON context into a [`Context`], or if it fails, adds
 /// appropriate error(s) to `errs` and returns an empty context.
 fn parse_context(
-    context_map: HashMap<String, JsonValueWithNoDuplicateKeys>,
+    context_map: HashMap<String, serde_json::Value>,
     schema_ref: Option<&crate::Schema>,
     action_ref: Option<&EntityUid>,
     errs: &mut Vec<miette::Report>,
@@ -749,7 +751,7 @@ struct RecvdSlice {
     /// JSON object containing the entities data, in "natural JSON" form -- same
     /// format as expected by EntityJsonParser
     #[cfg_attr(feature = "wasm", tsify(type = "Array<EntityJson>"))]
-    entities: JsonValueWithNoDuplicateKeys,
+    entities: serde_json::Value,
 
     /// Optional template policies.
     #[serde_as(as = "Option<MapPreventDuplicates<_, _>>")]
@@ -816,7 +818,7 @@ impl RecvdSlice {
                 crate::PolicySet::new()
             }
         };
-        let entities = match Entities::from_json_value(entities.into(), schema) {
+        let entities = match Entities::from_json_value(entities, schema) {
             Ok(entities) => entities,
             Err(e) => {
                 errs.push(miette::Report::new(e));
@@ -1906,12 +1908,12 @@ mod partial_test {
             assert_eq!(response.decision(), None);
             let errors: Vec<_> = response.errored().collect();
             assert_eq!(errors.len(), 0, "{errors:?}");
-            let actual_residuals: HashSet<_> = response.nontrivial_residual_ids().collect();
+            let actual_residuals: HashSet<&str> = response.nontrivial_residual_ids().map(|id| id.as_str()).collect();
             for id in &expected_residuals {
-                assert!(actual_residuals.contains(&PolicyId::from_str(id).ok().unwrap()), "expected nontrivial residual for {id}, but it's missing")
+                assert!(actual_residuals.contains(id), "expected nontrivial residual for {id}, but it's missing")
             }
             for id in &actual_residuals {
-                assert!(expected_residuals.contains(id.to_string().as_str()),"found unexpected nontrivial residual for {id}")
+                assert!(expected_residuals.contains(id),"found unexpected nontrivial residual for {id}")
             }
         });
     }

cedar-policy/src/ffi/utils.rs

@@ -16,7 +16,6 @@
 
 //! 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};
@@ -131,12 +130,10 @@ impl<'a, E: miette::Diagnostic + ?Sized> From<&'a E> for DetailedError {
             severity: diag.severity().map(Into::into),
             source_locations: diag
                 .labels()
-                .map(|labels| labels.map(Into::into).collect())
-                .unwrap_or_else(|| vec![]),
+                .map_or_else(Vec::new, |labels| labels.map(Into::into).collect()),
             related: diag
                 .related()
-                .map(|errs| errs.map(|e| e.into()).collect())
-                .unwrap_or_else(|| vec![]),
+                .map_or_else(Vec::new, |errs| errs.map(Into::into).collect()),
         }
     }
 }
@@ -237,11 +234,9 @@ 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),
+    Json(serde_json::Value),
 }
 
 impl Schema {
@@ -257,7 +252,7 @@ impl Schema {
                     )
                 })
                 .map_err(miette::Report::new),
-            Self::Json(val) => crate::Schema::from_json_value(val.into())
+            Self::Json(val) => crate::Schema::from_json_value(val)
                 .map(|sch| {
                     (
                         sch,