From 7dd7e75961f41716d34b1c968ca6952e1fb5a38f Mon Sep 17 00:00:00 2001 From: Mark Creamer Date: Thu, 27 Jun 2024 14:28:07 -0400 Subject: [PATCH 1/6] Adding PolicySet parsePolicies method Signed-off-by: Mark Creamer --- .../cedarpolicy/model/slice/PolicySet.java | 39 ++++++++++ .../java/com/cedarpolicy/PolicySetTests.java | 69 ++++++++++++++++++ .../java/com/cedarpolicy/PolicyTests.java | 5 +- .../test/resources/duplicate_policies.cedar | 13 ++++ .../test/resources/malformed_policy_set.cedar | 13 ++++ CedarJava/src/test/resources/policies.cedar | 13 ++++ CedarJava/src/test/resources/template.cedar | 19 +++++ CedarJavaFFI/src/interface.rs | 72 +++++++++++++++++++ 8 files changed, 241 insertions(+), 2 deletions(-) create mode 100644 CedarJava/src/test/java/com/cedarpolicy/PolicySetTests.java create mode 100644 CedarJava/src/test/resources/duplicate_policies.cedar create mode 100644 CedarJava/src/test/resources/malformed_policy_set.cedar create mode 100644 CedarJava/src/test/resources/policies.cedar create mode 100644 CedarJava/src/test/resources/template.cedar diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java b/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java index 38705d48..e8a6742e 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java @@ -16,12 +16,23 @@ package com.cedarpolicy.model.slice; +import com.cedarpolicy.loader.LibraryLoader; + +import com.cedarpolicy.model.exception.InternalException; + import java.util.Collections; import java.util.List; import java.util.Set; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + /** Policy Set containing policies in the Cedar language. */ public class PolicySet { + static { + LibraryLoader.loadLibrary(); + } /** Policy set. */ public Set policies; @@ -49,4 +60,32 @@ public PolicySet(Set policies, Set templates, List p.policyID).distinct().count()); + assertEquals(2, policySet.policies.size()); + assertEquals(0, policySet.templates.size()); + } + + @Test + public void parsePoliciesStringTests() throws InternalException { + PolicySet policySet = PolicySet.parsePolicies("permit(principal, action, resource);"); + PolicySet policySet2 = PolicySet.parsePolicies("permit(principal, action, resource) when { principal has x && principal.x == 5};"); + for (Policy p: policySet.policies) { + assertNotNull(p.policySrc); + } + assertEquals(1, policySet.policies.size()); + assertEquals(0, policySet.templates.size()); + for (Policy p: policySet2.policies) { + assertNotNull(p.policySrc); + } + assertEquals(1, policySet2.policies.size()); + assertEquals(0, policySet2.templates.size()); + } + + @Test + public void parseTemplatesTests() throws InternalException, IOException { + PolicySet policySet = PolicySet.parsePolicies(Path.of(TEST_RESOURCES_DIR + "template.cedar")); + for (Policy p: policySet.policies) { + assertNotNull(p.policySrc); + } + assertEquals(2, policySet.policies.size()); + + for (Policy p: policySet.templates) { + assertNotNull(p.policySrc); + } + assertEquals(1, policySet.templates.size()); + } + + @Test + public void parsePoliciesExceptionTests() throws InternalException, IOException { + assertThrows(IOException.class, () -> { + PolicySet.parsePolicies(Path.of("nonExistentFilePath.cedar")); + }); + assertThrows(InternalException.class, () -> { + PolicySet.parsePolicies(Path.of(TEST_RESOURCES_DIR + "malformed_policy_set.cedar")); + }); + } +} diff --git a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java index 4457eb39..16a069c0 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -16,7 +17,7 @@ public void parseStaticPolicyTests() { assertDoesNotThrow(() -> { var policy1 = Policy.parseStaticPolicy("permit(principal, action, resource);"); var policy2 = Policy.parseStaticPolicy("permit(principal, action, resource) when { principal has x && principal.x == 5};"); - assertNotEquals(policy1.policyID.equals(policy2.policyID), true); + assertNotEquals(policy1.policyID, policy2.policyID); }); assertThrows(InternalException.class, () -> { Policy.parseStaticPolicy("permit();"); @@ -31,7 +32,7 @@ public void parsePolicyTemplateTests() { assertDoesNotThrow(() -> { String tbody = "permit(principal == ?principal, action, resource in ?resource);"; var template = Policy.parsePolicyTemplate(tbody); - assertTrue(template.policySrc.equals(tbody)); + assertEquals(tbody, template.policySrc); }); assertThrows(InternalException.class, () -> { Policy.parsePolicyTemplate("permit(principal in ?resource, action, resource);"); diff --git a/CedarJava/src/test/resources/duplicate_policies.cedar b/CedarJava/src/test/resources/duplicate_policies.cedar new file mode 100644 index 00000000..0572c90b --- /dev/null +++ b/CedarJava/src/test/resources/duplicate_policies.cedar @@ -0,0 +1,13 @@ +@id("Policy") +permit ( + principal == User::"Mark", + action == Action::"view", + resource == Photo::"SomePhoto.jpg" +); + +@id("Policy") +permit ( + principal == User::"Mark", + action == Action::"view", + resource == Photo::"SomePhoto.jpg" +); \ No newline at end of file diff --git a/CedarJava/src/test/resources/malformed_policy_set.cedar b/CedarJava/src/test/resources/malformed_policy_set.cedar new file mode 100644 index 00000000..80972386 --- /dev/null +++ b/CedarJava/src/test/resources/malformed_policy_set.cedar @@ -0,0 +1,13 @@ +@id("Proper Policy") +permit ( + principal == User::"Matt", + action == Action::"view", + resource == Photo::"Husky.jpg" +); + +@id("Malformed Policy") +forbid ( + principal == User::"Liam", + action, + resource = Photo::"Husky.jpg" +); \ No newline at end of file diff --git a/CedarJava/src/test/resources/policies.cedar b/CedarJava/src/test/resources/policies.cedar new file mode 100644 index 00000000..b0ada812 --- /dev/null +++ b/CedarJava/src/test/resources/policies.cedar @@ -0,0 +1,13 @@ +@id("Policy #1") +permit ( + principal in UserGroup::"friends", + action == Action::"view", + resource == Photo::"Husky.jpg" +); + +@id("Policy #2") +forbid ( + principal == User::"Matt", + action, + resource == Photo::"Husky.jpg" +); \ No newline at end of file diff --git a/CedarJava/src/test/resources/template.cedar b/CedarJava/src/test/resources/template.cedar new file mode 100644 index 00000000..52d785d7 --- /dev/null +++ b/CedarJava/src/test/resources/template.cedar @@ -0,0 +1,19 @@ +@id("Policy #1") +permit ( + principal == User::"Aaron", + action == Action::"view", + resource == Photo::"SomePhoto.jpg" +); + +@id("Policy #2") +permit (principal == User::"Josh", action == Action::"comment", resource == Photo::"SomePhoto.jpg"); + +@id("Template #1") +permit ( + principal in ?principal, + action in [Action::"view", Action::"comment"], + resource in ?resource +) +unless { + resource.tag == "private" +}; \ No newline at end of file diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 039fadd4..0ccd6de0 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -219,6 +219,78 @@ fn parse_policy_internal<'a>( } } +#[jni_fn("com.cedarpolicy.model.slice.PolicySet")] +pub fn parsePoliciesJni<'a>(mut env: JNIEnv<'a>, _: JClass, policies_jstr: JString<'a>) -> jvalue { + match parse_policies_internal(&mut env, policies_jstr) { + Err(e) => jni_failed(&mut env, e.as_ref()), + Ok(policies_set) => policies_set.as_jni(), + } +} + +fn parse_policies_internal<'a>( + env: &mut JNIEnv<'a>, + policies_jstr: JString<'a>, +) -> Result> { + if policies_jstr.is_null() { + raise_npe(env) + } else { + // Parse the string into the Rust PolicySet + let policies_jstring = env.get_string(&policies_jstr)?; + let policies_string = String::from(policies_jstring); + let policy_set = PolicySet::from_str(&policies_string)?; + + // Enumerate over the parsed policies + let policies_java_hash_set = create_java_hash_set(env); + for policy in policy_set.policies() { + let policy_text = format!("{}", policy); + let java_policy_object = create_java_policy(env, &env.new_string(&policy_text)?.into()); + let add_result = env.call_method(&policies_java_hash_set, "add", "(Ljava/lang/Object;)Z", &[JValueGen::Object(&java_policy_object)]); + + match add_result { + Err(e) => return Err(Box::new(e)), + Ok(_) => {} + } + } + + let templates_java_hash_set = create_java_hash_set(env); + for template in policy_set.templates() { + let policy_text = format!("{}", template); + let java_policy_object = create_java_policy(env, &env.new_string(&policy_text)?.into()); + let add_result = env.call_method(&templates_java_hash_set, "add", "(Ljava/lang/Object;)Z", &[JValueGen::Object(&java_policy_object)]); + + match add_result { + Err(e) => return Err(Box::new(e)), + Ok(_) => {} + } + } + + let java_policy_set = create_java_policy_set(env, &policies_java_hash_set, &templates_java_hash_set); + + Ok(JValueGen::Object(java_policy_set.into())) + } +} + +fn create_java_hash_set<'a>(env: &mut JNIEnv<'a>) -> JObject<'a> { + env.new_object("java/util/HashSet", &"()V", &[]).expect("Failed to create new HashSet object") +} + +fn create_java_policy<'a>(env: &mut JNIEnv<'a>, policy_string: &JString) -> JObject<'a> { + env.new_object( + "com/cedarpolicy/model/slice/Policy", + &"(Ljava/lang/String;Ljava/lang/String;)V", + &[JValueGen::Object(&policy_string), JValueGen::Object(&JObject::null())], + ).expect("Failed to create new Policy object") +} + +fn create_java_policy_set<'a>(env: &mut JNIEnv<'a>, policies_java_hash_set: &JObject<'a>, templates_java_hash_set: &JObject<'a>) -> JObject<'a> { + env.new_object( + "com/cedarpolicy/model/slice/PolicySet", + &"(Ljava/util/Set;Ljava/util/Set;Ljava/util/List;)V", + &[JValueGen::Object(&policies_java_hash_set), JValueGen::Object(&templates_java_hash_set), JValueGen::Object(&JObject::null())], + ) + .expect("Failed to create new PolicySet object") +} + #[jni_fn("com.cedarpolicy.model.slice.Policy")] pub fn parsePolicyTemplateJni<'a>( mut env: JNIEnv<'a>, From 3ce1a7e6fabf6e3e833ab306a660563dd5497d86 Mon Sep 17 00:00:00 2001 From: Mark Creamer Date: Thu, 27 Jun 2024 15:04:00 -0400 Subject: [PATCH 2/6] Addressing Cargo fmt check Signed-off-by: Mark Creamer --- CedarJavaFFI/src/interface.rs | 40 ++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 0ccd6de0..5ce8b9e5 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -244,7 +244,12 @@ fn parse_policies_internal<'a>( for policy in policy_set.policies() { let policy_text = format!("{}", policy); let java_policy_object = create_java_policy(env, &env.new_string(&policy_text)?.into()); - let add_result = env.call_method(&policies_java_hash_set, "add", "(Ljava/lang/Object;)Z", &[JValueGen::Object(&java_policy_object)]); + let add_result = env.call_method( + &policies_java_hash_set, + "add", + "(Ljava/lang/Object;)Z", + &[JValueGen::Object(&java_policy_object)], + ); match add_result { Err(e) => return Err(Box::new(e)), @@ -256,7 +261,12 @@ fn parse_policies_internal<'a>( for template in policy_set.templates() { let policy_text = format!("{}", template); let java_policy_object = create_java_policy(env, &env.new_string(&policy_text)?.into()); - let add_result = env.call_method(&templates_java_hash_set, "add", "(Ljava/lang/Object;)Z", &[JValueGen::Object(&java_policy_object)]); + let add_result = env.call_method( + &templates_java_hash_set, + "add", + "(Ljava/lang/Object;)Z", + &[JValueGen::Object(&java_policy_object)], + ); match add_result { Err(e) => return Err(Box::new(e)), @@ -264,29 +274,43 @@ fn parse_policies_internal<'a>( } } - let java_policy_set = create_java_policy_set(env, &policies_java_hash_set, &templates_java_hash_set); + let java_policy_set = + create_java_policy_set(env, &policies_java_hash_set, &templates_java_hash_set); Ok(JValueGen::Object(java_policy_set.into())) } } fn create_java_hash_set<'a>(env: &mut JNIEnv<'a>) -> JObject<'a> { - env.new_object("java/util/HashSet", &"()V", &[]).expect("Failed to create new HashSet object") + env.new_object("java/util/HashSet", &"()V", &[]) + .expect("Failed to create new HashSet object") } fn create_java_policy<'a>(env: &mut JNIEnv<'a>, policy_string: &JString) -> JObject<'a> { env.new_object( "com/cedarpolicy/model/slice/Policy", &"(Ljava/lang/String;Ljava/lang/String;)V", - &[JValueGen::Object(&policy_string), JValueGen::Object(&JObject::null())], - ).expect("Failed to create new Policy object") + &[ + JValueGen::Object(&policy_string), + JValueGen::Object(&JObject::null()), + ], + ) + .expect("Failed to create new Policy object") } -fn create_java_policy_set<'a>(env: &mut JNIEnv<'a>, policies_java_hash_set: &JObject<'a>, templates_java_hash_set: &JObject<'a>) -> JObject<'a> { +fn create_java_policy_set<'a>( + env: &mut JNIEnv<'a>, + policies_java_hash_set: &JObject<'a>, + templates_java_hash_set: &JObject<'a>, +) -> JObject<'a> { env.new_object( "com/cedarpolicy/model/slice/PolicySet", &"(Ljava/util/Set;Ljava/util/Set;Ljava/util/List;)V", - &[JValueGen::Object(&policies_java_hash_set), JValueGen::Object(&templates_java_hash_set), JValueGen::Object(&JObject::null())], + &[ + JValueGen::Object(&policies_java_hash_set), + JValueGen::Object(&templates_java_hash_set), + JValueGen::Object(&JObject::null()), + ], ) .expect("Failed to create new PolicySet object") } From 9c94702f0a8706c8e325965a2cdaf877509212a3 Mon Sep 17 00:00:00 2001 From: Mark Creamer Date: Fri, 28 Jun 2024 14:11:32 -0400 Subject: [PATCH 3/6] Introducing jset wrapper Signed-off-by: Mark Creamer --- CedarJavaFFI/src/interface.rs | 59 ++++++++--------------------------- CedarJavaFFI/src/jset.rs | 46 +++++++++++++++++++++++++++ CedarJavaFFI/src/lib.rs | 1 + CedarJavaFFI/src/objects.rs | 37 ++++++++++++++++++++++ 4 files changed, 97 insertions(+), 46 deletions(-) create mode 100644 CedarJavaFFI/src/jset.rs diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 5ce8b9e5..ee6b7a39 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -31,7 +31,8 @@ use std::{collections::HashMap, error::Error, str::FromStr, thread}; use crate::{ answer::Answer, - objects::{JEntityId, JEntityTypeName, JEntityUID, Object}, + jset::Set, + objects::{JEntityId, JEntityTypeName, JEntityUID, JPolicy, Object}, utils::raise_npe, }; @@ -240,64 +241,30 @@ fn parse_policies_internal<'a>( let policy_set = PolicySet::from_str(&policies_string)?; // Enumerate over the parsed policies - let policies_java_hash_set = create_java_hash_set(env); + let mut policies_java_hash_set = Set::new(env)?; for policy in policy_set.policies() { let policy_text = format!("{}", policy); - let java_policy_object = create_java_policy(env, &env.new_string(&policy_text)?.into()); - let add_result = env.call_method( - &policies_java_hash_set, - "add", - "(Ljava/lang/Object;)Z", - &[JValueGen::Object(&java_policy_object)], - ); - - match add_result { - Err(e) => return Err(Box::new(e)), - Ok(_) => {} - } + let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &JObject::null().into())?; + let _ = policies_java_hash_set.add(env, java_policy_object); } - let templates_java_hash_set = create_java_hash_set(env); + let mut templates_java_hash_set = Set::new(env)?; for template in policy_set.templates() { let policy_text = format!("{}", template); - let java_policy_object = create_java_policy(env, &env.new_string(&policy_text)?.into()); - let add_result = env.call_method( - &templates_java_hash_set, - "add", - "(Ljava/lang/Object;)Z", - &[JValueGen::Object(&java_policy_object)], - ); - - match add_result { - Err(e) => return Err(Box::new(e)), - Ok(_) => {} - } + let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &JObject::null().into())?; + let _ = templates_java_hash_set.add(env, java_policy_object); } - let java_policy_set = - create_java_policy_set(env, &policies_java_hash_set, &templates_java_hash_set); + let java_policy_set = create_java_policy_set( + env, + policies_java_hash_set.as_ref(), + templates_java_hash_set.as_ref(), + ); Ok(JValueGen::Object(java_policy_set.into())) } } -fn create_java_hash_set<'a>(env: &mut JNIEnv<'a>) -> JObject<'a> { - env.new_object("java/util/HashSet", &"()V", &[]) - .expect("Failed to create new HashSet object") -} - -fn create_java_policy<'a>(env: &mut JNIEnv<'a>, policy_string: &JString) -> JObject<'a> { - env.new_object( - "com/cedarpolicy/model/slice/Policy", - &"(Ljava/lang/String;Ljava/lang/String;)V", - &[ - JValueGen::Object(&policy_string), - JValueGen::Object(&JObject::null()), - ], - ) - .expect("Failed to create new Policy object") -} - fn create_java_policy_set<'a>( env: &mut JNIEnv<'a>, policies_java_hash_set: &JObject<'a>, diff --git a/CedarJavaFFI/src/jset.rs b/CedarJavaFFI/src/jset.rs new file mode 100644 index 00000000..9b707ac8 --- /dev/null +++ b/CedarJavaFFI/src/jset.rs @@ -0,0 +1,46 @@ +use std::marker::PhantomData; + +use crate::{objects::Object, utils::Result}; +use jni::{ + objects::{JObject, JValueGen}, + JNIEnv, +}; + +/// Typed wrapper for Java sets +/// (java.util.Set) +#[derive(Debug)] +pub struct Set<'a, T> { + /// Underlying Java object + obj: JObject<'a>, + /// ZST for tracking type info + marker: PhantomData, + /// The size of this set + size: i32, +} + +impl<'a, T: Object<'a>> Set<'a, T> { + /// Construct an empty hash set, which will serve as a set + pub fn new(env: &mut JNIEnv<'a>) -> Result { + let obj = env.new_object("java/util/HashSet", "()V", &[])?; + + Ok(Self { + obj, + marker: PhantomData, + size: 0, + }) + } + + /// Add an item to the set + pub fn add(&mut self, env: &mut JNIEnv<'a>, v: T) -> Result<()> { + let value = JValueGen::Object(v.as_ref()); + env.call_method(&self.obj, "add", "(Ljava/lang/Object;)Z", &[value])?; + self.size += 1; + Ok(()) + } +} + +impl<'a, T> AsRef> for Set<'a, T> { + fn as_ref(&self) -> &JObject<'a> { + &self.obj + } +} diff --git a/CedarJavaFFI/src/lib.rs b/CedarJavaFFI/src/lib.rs index 98357a50..3c1266af 100644 --- a/CedarJavaFFI/src/lib.rs +++ b/CedarJavaFFI/src/lib.rs @@ -18,6 +18,7 @@ mod answer; mod interface; mod jlist; +mod jset; mod objects; mod tests; mod utils; diff --git a/CedarJavaFFI/src/objects.rs b/CedarJavaFFI/src/objects.rs index 69112af6..dc7133de 100644 --- a/CedarJavaFFI/src/objects.rs +++ b/CedarJavaFFI/src/objects.rs @@ -339,3 +339,40 @@ impl<'a> AsRef> for JEntityUID<'a> { &self.obj } } + +/// Typed wrapper for Policy objects +/// (com.cedarpolicy.model.slice.Policy) +pub struct JPolicy<'a> { + obj: JObject<'a> +} + +impl<'a> JPolicy<'a> { + /// Construct a new Policy object + pub fn new(env: &mut JNIEnv<'a>, policy_string: &JString, policy_id_string: &JString) -> Result { + let obj = env + .new_object( + "com/cedarpolicy/model/slice/Policy", + &"(Ljava/lang/String;Ljava/lang/String;)V", + &[ + JValueGen::Object(&policy_string), + JValueGen::Object(&policy_id_string), + ], + ) + .expect("Failed to create new Policy object"); + + Ok(Self { obj }) + } +} + +impl<'a> Object<'a> for JPolicy<'a> { + fn cast(env: &mut JNIEnv<'a>, obj: JObject<'a>) -> Result { + assert_is_class(env, &obj, "com/cedarpolicy/model/slice/Policy")?; + Ok(Self { obj }) + } +} + +impl<'a> AsRef> for JPolicy<'a> { + fn as_ref(&self) -> &JObject<'a> { + &self.obj + } +} From 3b174cf92ea8508dfee63da844a6205e7f44d18e Mon Sep 17 00:00:00 2001 From: Mark Creamer Date: Sun, 30 Jun 2024 15:28:33 -0400 Subject: [PATCH 4/6] Refactoring SharedIntegrationTests Also modified the FFI code to actually take the policy id from the Rust output instead of using the atomic policy ID counter from the Java Policy object --- .../cedarpolicy/model/slice/PolicySet.java | 12 +++- .../cedarpolicy/SharedIntegrationTests.java | 69 +++---------------- CedarJavaFFI/src/interface.rs | 11 +-- 3 files changed, 25 insertions(+), 67 deletions(-) diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java b/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java index e8a6742e..c22bbde3 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/slice/PolicySet.java @@ -55,6 +55,12 @@ public PolicySet(Set policies) { this.templateInstantiations = Collections.emptyList(); } + public PolicySet(Set policies, Set templates) { + this.policies = policies; + this.templates = templates; + this.templateInstantiations = Collections.emptyList(); + } + public PolicySet(Set policies, Set templates, List templateInstantiations) { this.policies = policies; this.templates = templates; @@ -62,14 +68,14 @@ public PolicySet(Set policies, Set templates, List integrationTestsFromJson() throws IOException { + public List integrationTestsFromJson() throws InternalException, IOException { List tests = new ArrayList<>(); // handwritten integration tests for (String testFile : JSON_TEST_FILES) { @@ -236,6 +237,8 @@ public List integrationTestsFromJson() throws IOException { // inside the forEach we can't throw checked exceptions, but we // can throw this unchecked exception throw new UncheckedIOException(e); + } catch (final InternalException e) { + throw new RuntimeException(e); } }); } @@ -247,14 +250,14 @@ public List integrationTestsFromJson() throws IOException { * test, and all the test in the json file are grouped into the returned container. */ @SuppressFBWarnings("NP_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD") - private DynamicContainer loadJsonTests(String jsonFile) throws IOException { + private DynamicContainer loadJsonTests(String jsonFile) throws InternalException, IOException { JsonTest test; try (InputStream jsonIn = new FileInputStream(resolveIntegrationTestPath(jsonFile).toFile())) { test = OBJECT_MAPPER.reader().readValue(jsonIn, JsonTest.class); } Set entities = loadEntities(test.entities); - Set policies = loadPolicies(test.policies); + PolicySet policySet = PolicySet.parsePolicies(resolveIntegrationTestPath(test.policies)); Schema schema = loadSchema(test.schema); return DynamicContainer.dynamicContainer( @@ -263,7 +266,7 @@ private DynamicContainer loadJsonTests(String jsonFile) throws IOException { Stream.of(DynamicTest.dynamicTest( jsonFile + ": validate", () -> - executeJsonValidationTest(policies, schema, test.shouldValidate))), + executeJsonValidationTest(policySet.policies, schema, test.shouldValidate))), test.requests.stream() .map( request -> @@ -271,61 +274,10 @@ private DynamicContainer loadJsonTests(String jsonFile) throws IOException { jsonFile + ": " + request.description, () -> executeJsonRequestTest( - entities, policies, request, + entities, policySet, request, schema))))); } - /** - * Load all policies from the policy file. The policy file path must be relative to the shared - * integration test root. This should be the case if the path was obtained from a JsonTest - * object. Extra processing is required because the test format does not include policy ids, and - * does not explicit separate policies in a file other than by semicolons. - */ - private Set loadPolicies(String policiesFile) throws IOException { - String policiesSrc = String.join("\n", Files.readAllLines(resolveIntegrationTestPath(policiesFile))); - - // Get a list of the policy sources for the individual policies in the - // file by splitting the full policy source on semicolons. This will - // break if a semicolon shows up in a string, eid, or comment. - String[] policyStrings = policiesSrc.split(";"); - // Some of the corpus tests contain semicolons in strings and/or eids. - // A simple way to check if the code above did the wrong thing in this case - // is to check for unmatched, unescaped quotes in the resulting policies. - for (String policyString : policyStrings) { - if (hasUnmatchedQuote(policyString)) { - policyStrings = null; - } - } - - Set policies = new HashSet<>(); - if (policyStrings == null) { - // This case will only be reached for corpus tests. - // The corpus tests all consist of a single policy, so it is fine to use - // the full policy source as a single policy. - policies.add(new Policy(policiesSrc, "policy0")); - } else { - for (int i = 0; i < policyStrings.length; i++) { - // The policy source doesn't include an explicit policy id, but the expected output - // implicitly assumes policies are numbered by their position in file. - String policyId = "policy" + i; - String policySrc = policyStrings[i]; - if (!policySrc.trim().isEmpty()) { - policies.add(new Policy(policySrc + ";", policyId)); - } - } - } - return policies; - } - - /** Check for unmatched quotes. */ - private Boolean hasUnmatchedQuote(String str) { - // Ignore escaped quotes, i.e. \" - // Note that backslashes in the regular expression have to be double escaped. - String newStr = str.replaceAll("\\\\\"", ""); - long count = newStr.chars().filter(ch -> ch == '\"').count(); - return (count % 2 == 1); - } - /** Load the schema file. */ private Schema loadSchema(String schemaFile) throws IOException { try (InputStream schemaStream = @@ -394,7 +346,7 @@ private void executeJsonValidationTest(Set policies, Schema schema, Bool * that the result is equal to the expected result. */ private void executeJsonRequestTest( - Set entities, Set policies, JsonRequest request, Schema schema) throws AuthException { + Set entities, PolicySet policySet, JsonRequest request, Schema schema) throws AuthException { AuthorizationEngine auth = new BasicAuthorizationEngine(); AuthorizationRequest authRequest = new AuthorizationRequest( @@ -404,7 +356,6 @@ private void executeJsonRequestTest( Optional.of(request.context), Optional.of(schema), request.validateRequest); - PolicySet policySet = new PolicySet(policies); try { final AuthorizationResponse response = auth.isAuthorized(authRequest, policySet, entities); diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index ee6b7a39..427e6d07 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -243,15 +243,17 @@ fn parse_policies_internal<'a>( // Enumerate over the parsed policies let mut policies_java_hash_set = Set::new(env)?; for policy in policy_set.policies() { + let policy_id = format!("{}", policy.id()); let policy_text = format!("{}", policy); - let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &JObject::null().into())?; + let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &env.new_string(&policy_id)?.into())?; let _ = policies_java_hash_set.add(env, java_policy_object); } let mut templates_java_hash_set = Set::new(env)?; for template in policy_set.templates() { + let policy_id = format!("{}", template.id()); let policy_text = format!("{}", template); - let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &JObject::null().into())?; + let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &env.new_string(&policy_id)?.into())?; let _ = templates_java_hash_set.add(env, java_policy_object); } @@ -272,11 +274,10 @@ fn create_java_policy_set<'a>( ) -> JObject<'a> { env.new_object( "com/cedarpolicy/model/slice/PolicySet", - &"(Ljava/util/Set;Ljava/util/Set;Ljava/util/List;)V", + &"(Ljava/util/Set;Ljava/util/Set;)V", &[ JValueGen::Object(&policies_java_hash_set), - JValueGen::Object(&templates_java_hash_set), - JValueGen::Object(&JObject::null()), + JValueGen::Object(&templates_java_hash_set) ], ) .expect("Failed to create new PolicySet object") From 0bc03a68ca227ba743bab35eac65cddc46f1c142 Mon Sep 17 00:00:00 2001 From: Mark Creamer Date: Sun, 30 Jun 2024 17:37:02 -0400 Subject: [PATCH 5/6] Fixing cargo fmt checks Signed-off-by: Mark Creamer --- CedarJavaFFI/src/interface.rs | 14 +++++++++++--- CedarJavaFFI/src/objects.rs | 8 ++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 427e6d07..188aa74a 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -245,7 +245,11 @@ fn parse_policies_internal<'a>( for policy in policy_set.policies() { let policy_id = format!("{}", policy.id()); let policy_text = format!("{}", policy); - let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &env.new_string(&policy_id)?.into())?; + let java_policy_object = JPolicy::new( + env, + &env.new_string(&policy_text)?.into(), + &env.new_string(&policy_id)?.into(), + )?; let _ = policies_java_hash_set.add(env, java_policy_object); } @@ -253,7 +257,11 @@ fn parse_policies_internal<'a>( for template in policy_set.templates() { let policy_id = format!("{}", template.id()); let policy_text = format!("{}", template); - let java_policy_object = JPolicy::new(env, &env.new_string(&policy_text)?.into(), &env.new_string(&policy_id)?.into())?; + let java_policy_object = JPolicy::new( + env, + &env.new_string(&policy_text)?.into(), + &env.new_string(&policy_id)?.into(), + )?; let _ = templates_java_hash_set.add(env, java_policy_object); } @@ -277,7 +285,7 @@ fn create_java_policy_set<'a>( &"(Ljava/util/Set;Ljava/util/Set;)V", &[ JValueGen::Object(&policies_java_hash_set), - JValueGen::Object(&templates_java_hash_set) + JValueGen::Object(&templates_java_hash_set), ], ) .expect("Failed to create new PolicySet object") diff --git a/CedarJavaFFI/src/objects.rs b/CedarJavaFFI/src/objects.rs index dc7133de..d01b83a4 100644 --- a/CedarJavaFFI/src/objects.rs +++ b/CedarJavaFFI/src/objects.rs @@ -343,12 +343,16 @@ impl<'a> AsRef> for JEntityUID<'a> { /// Typed wrapper for Policy objects /// (com.cedarpolicy.model.slice.Policy) pub struct JPolicy<'a> { - obj: JObject<'a> + obj: JObject<'a>, } impl<'a> JPolicy<'a> { /// Construct a new Policy object - pub fn new(env: &mut JNIEnv<'a>, policy_string: &JString, policy_id_string: &JString) -> Result { + pub fn new( + env: &mut JNIEnv<'a>, + policy_string: &JString, + policy_id_string: &JString, + ) -> Result { let obj = env .new_object( "com/cedarpolicy/model/slice/Policy", From 8423668e31cbef4f68ca861701807ee8bc84b0e6 Mon Sep 17 00:00:00 2001 From: Mark Creamer Date: Sun, 30 Jun 2024 18:00:25 -0400 Subject: [PATCH 6/6] Removing unused duplicate_policies test file Signed-off-by: Mark Creamer --- .../src/test/resources/duplicate_policies.cedar | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 CedarJava/src/test/resources/duplicate_policies.cedar diff --git a/CedarJava/src/test/resources/duplicate_policies.cedar b/CedarJava/src/test/resources/duplicate_policies.cedar deleted file mode 100644 index 0572c90b..00000000 --- a/CedarJava/src/test/resources/duplicate_policies.cedar +++ /dev/null @@ -1,13 +0,0 @@ -@id("Policy") -permit ( - principal == User::"Mark", - action == Action::"view", - resource == Photo::"SomePhoto.jpg" -); - -@id("Policy") -permit ( - principal == User::"Mark", - action == Action::"view", - resource == Photo::"SomePhoto.jpg" -); \ No newline at end of file