Deny unknown fields in FFI. (#815)

cedar-policy/CHANGELOG.md

@@ -22,8 +22,8 @@ Cedar Language Version: 4.0
 
 ### Changed
 
-- The API around `Request::new` has changed to remove the `Option`s 
-  around the entity type arguments.
+- The API around `Request::new` has changed to remove the `Option`s
+  around the entity type arguments. See [RFC 55](https://github.com/cedar-policy/rfcs/blob/main/text/0055-remove-unspecified.md).
 - Significantly reworked all public-facing error types to address some issues
   and improve consistency. See issue #745.
 - Finalized the `ffi` module which was preview-released in 3.2.0.
@@ -35,6 +35,7 @@ Cedar Language Version: 4.0
 - Changed JSON schema parser so that `Set`, `Entity`, `Record`, and `Extension`
   can be common type names; updated the error message when common type names
   conflict with built-in primitive type names (#974, partially resolving #973)
+- Changed the FFI to error on typos or unexpected fields in the input JSON.
 
 ### Removed
 

cedar-policy/src/ffi/is_authorized.rs

@@ -156,6 +156,7 @@ pub fn is_authorized_partial_json_str(json: &str) -> Result<String, serde_json::
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct Response {
     /// Authorization decision
     decision: Decision,
@@ -169,6 +170,7 @@ pub struct Response {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct Diagnostics {
     /// Ids of the policies that contributed to the decision.
     /// If no policies applied to the request, this set will be empty.
@@ -239,6 +241,7 @@ impl Diagnostics {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct AuthorizationError {
     /// Id of the policy where the error (or warning) occurred
     #[cfg_attr(feature = "wasm", tsify(type = "string"))]
@@ -291,6 +294,7 @@ impl From<cedar_policy_core::authorizer::AuthorizationError> for AuthorizationEr
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct ResidualResponse {
     decision: Option<Decision>,
     satisfied: HashSet<PolicyId>,
@@ -458,6 +462,7 @@ pub enum PartialAuthorizationAnswer {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct AuthorizationCall {
     /// The principal taking action
     principal: EntityUid,
@@ -492,6 +497,7 @@ pub struct AuthorizationCall {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct PartialAuthorizationCall {
     /// The principal taking action. If this field is empty, then the principal is unknown.
     principal: Option<EntityUid>,
@@ -594,7 +600,7 @@ impl AuthorizationCall {
             },
             _ => {
                 // At least one of the `errs.push(e)` statements above must have been reached
-                return build_error(errs, warnings);
+                build_error(errs, warnings)
             }
         }
     }
@@ -680,7 +686,7 @@ impl PartialAuthorizationCall {
             },
             _ => {
                 // At least one of the `errs.push(e)` statements above must have been reached
-                return build_error(errs, warnings);
+                build_error(errs, warnings)
             }
         }
     }
@@ -1642,8 +1648,7 @@ mod partial_test {
                     "ID1": "permit(principal == User::\"alice\", action, resource);"
                 }
             },
-            "entities": [],
-            "partial_evaluation": true
+            "entities": []
         });
 
         assert_is_authorized_json_partial(call);
@@ -1666,8 +1671,7 @@ mod partial_test {
                     "ID1": "permit(principal == User::\"alice\", action, resource);"
                 }
             },
-            "entities": [],
-            "partial_evaluation": true
+            "entities": []
         });
 
         assert_is_not_authorized_json_partial(call);
@@ -1690,8 +1694,7 @@ mod partial_test {
                     "ID1": "permit(principal == User::\"alice\", action, resource);"
                 }
             },
-            "entities": [],
-            "partial_evaluation": true
+            "entities": []
         });
 
         assert_is_residual(call, &HashSet::from(["ID1"]));
@@ -1714,8 +1717,7 @@ mod partial_test {
                     "ID1": "permit(principal, action, resource) when { principal == User::\"alice\" };"
                 }
             },
-            "entities": [],
-            "partial_evaluation": true
+            "entities": []
         });
 
         assert_is_residual(call, &HashSet::from(["ID1"]));
@@ -1739,8 +1741,7 @@ mod partial_test {
                     "ID2": "forbid(principal, action, resource) unless { resource == Photo::\"door\" };"
                 }
             },
-            "entities": [],
-            "partial_evaluation": true
+            "entities": []
         });
 
         assert_is_residual(call, &HashSet::from(["ID1"]));

cedar-policy/src/ffi/mod.rs

@@ -22,3 +22,4 @@ mod utils;
 pub use utils::*;
 mod validate;
 pub use validate::*;
+mod tests;

cedar-policy/src/ffi/tests.rs

@@ -0,0 +1,144 @@
+#[cfg(test)]
+mod ffi_tests {
+    #[cfg(feature = "partial-eval")]
+    use crate::ffi::is_authorized_partial_json;
+    use crate::ffi::{is_authorized_json, validate_json};
+    use cool_asserts::assert_matches;
+
+    #[test]
+    fn test_fail_unknown_field_policy_slice() {
+        let json = serde_json::json!({
+            "principal": {
+             "type": "User",
+             "id": "alice"
+            },
+            "action": {
+             "type": "Photo",
+             "id": "view"
+            },
+            "resource": {
+             "type": "Photo",
+             "id": "door"
+            },
+            "context": {},
+            "slice": {
+             "policies": {},
+             "templatePolicies": {},
+             "entities": []
+            }
+        });
+
+        assert_matches!(is_authorized_json(json), Err(e) => {
+            assert_eq!(e.to_string(), "unknown field `slice`, expected one of `principal`, `action`, `resource`, `context`, `schema`, `validateRequest`, `policies`, `entities`");
+        });
+    }
+
+    #[test]
+    fn test_fail_unknown_field_enable_request_validation() {
+        let json = serde_json::json!({
+            "principal": {
+             "type": "User",
+             "id": "alice"
+            },
+            "action": {
+             "type": "Photo",
+             "id": "view"
+            },
+            "resource": {
+             "type": "Photo",
+             "id": "door"
+            },
+            "context": {},
+            "policies": {},
+            "entities": [],
+            "enableRequestValidation": true,
+        });
+
+        assert_matches!(is_authorized_json(json), Err(e) => {
+            assert_eq!(e.to_string(), "unknown field `enableRequestValidation`, expected one of `principal`, `action`, `resource`, `context`, `schema`, `validateRequest`, `policies`, `entities`");
+        });
+    }
+
+    #[test]
+    fn test_fail_unknown_field_policies() {
+        let json = serde_json::json!({
+            "principal": {
+             "type": "User",
+             "id": "alice"
+            },
+            "action": {
+             "type": "Photo",
+             "id": "view"
+            },
+            "resource": {
+             "type": "Photo",
+             "id": "door"
+            },
+            "context": {},
+            "policies": {
+              "policies": {}
+            },
+            "entities": []
+        });
+
+        assert_matches!(is_authorized_json(json), Err(e) => {
+            assert_eq!(e.to_string(), "unknown field `policies`, expected one of `staticPolicies`, `templates`, `templateLinks`");
+        });
+    }
+
+    #[cfg(feature = "partial-eval")]
+    #[test]
+    fn test_fail_unknown_field_partial_evaluation() {
+        let json = serde_json::json!({
+            "principal": {
+                "type": "User",
+                "id": "alice"
+            },
+            "action": {
+                "type": "Photo",
+                "id": "view"
+            },
+            "context": {},
+            "policies": {
+                "staticPolicies": {
+                    "ID1": "permit(principal == User::\"alice\", action, resource);"
+                }
+            },
+            "entities": [],
+            "partial_evaluation": true
+        });
+
+        assert_matches!(is_authorized_partial_json(json), Err(e) => {
+            assert_eq!(e.to_string(), "unknown field `partial_evaluation`, expected one of `principal`, `action`, `resource`, `context`, `schema`, `validateRequest`, `policies`, `entities`");
+        });
+    }
+
+    #[test]
+    fn test_fail_unknown_field_validation() {
+        let json = serde_json::json!({
+          "schema": { "json": { "": {
+            "entityTypes": {
+              "User": {
+                "memberOfTypes": [ ]
+              },
+              "Photo": {
+                "memberOfTypes": [ ]
+              }
+            },
+            "actions": {
+              "viewPhoto": {
+                "appliesTo": {
+                  "resourceTypes": [ "Photo" ],
+                  "principalTypes": [ "User" ]
+                }
+              }
+            }
+          }}},
+          "Policies": "forbid(principal, action, resource);permit(principal == Photo::\"photo.jpg\", action == Action::\"viewPhoto\", resource == User::\"alice\");"
+        });
+
+        assert_matches!(validate_json(json), Err(e) => {
+            assert_eq!(e.to_string(), "unknown field `Policies`, expected one of `validationSettings`, `schema`, `policies`");
+        });
+    }
+}

cedar-policy/src/ffi/utils.rs

@@ -33,6 +33,7 @@ extern crate tsify;
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct DetailedError {
     /// Main error message, including both the `miette` "message" and the
     /// `miette` "causes" (uses `miette`'s default `Display` output)
@@ -84,6 +85,7 @@ impl From<miette::Severity> for Severity {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct SourceLabel {
     /// Text of the label (if any)
     pub label: Option<String>,
@@ -97,6 +99,7 @@ pub struct SourceLabel {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct SourceLocation {
     /// Start of the source location (in bytes)
     pub start: usize,
@@ -408,6 +411,7 @@ impl Default for StaticPolicySet {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct TemplateLink {
     /// Id of the template to link against
     template_id: PolicyId,
@@ -441,6 +445,7 @@ impl TemplateLink {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct PolicySet {
     /// static policies
     #[serde(default)]

cedar-policy/src/ffi/validate.rs

@@ -104,6 +104,7 @@ pub fn validate_json_str(json: &str) -> Result<String, serde_json::Error> {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct ValidationCall {
     /// Validation settings
     #[serde(default)]
@@ -154,6 +155,7 @@ impl ValidationCall {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct ValidationSettings {
     /// Whether validation is enabled. If this flag is set to `false`, then
     /// only parsing is performed. The default value is `true`.
@@ -176,6 +178,7 @@ impl Default for ValidationSettings {
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
 #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
 #[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields)]
 pub struct ValidationError {
     /// Id of the policy where the error (or warning) occurred
     #[cfg_attr(feature = "wasm", tsify(type = "string"))]