From 30d010624b24e0539d60faab7307fc530152fe55 Mon Sep 17 00:00:00 2001 From: Mudit Chaudhary Date: Thu, 9 Jan 2025 20:08:39 +0000 Subject: [PATCH 1/5] exposes policy effect for static policies and template Signed-off-by: Mudit Chaudhary --- .../com/cedarpolicy/model/policy/Policy.java | 20 ++++++++ CedarJavaFFI/src/interface.rs | 46 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java b/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java index 2f58c762..5d1836e0 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java @@ -78,6 +78,24 @@ public String toString() { return "// Policy ID: " + policyID + "\n" + policySrc; } + /** + * Returns the effect of a policy. + * + * Determines the policy effect by attempting static policy first, then template. + * In future, it will only support static policies once new class is introduced for Template. + * + * @return The effect of the policy, either "permit" or "forbid" + * @throws InternalException + * @throws NullPointerException + */ + public String effect() throws InternalException, NullPointerException { + try { + return policyEffectJni(policySrc); // Get effect for static policy + } catch (InternalException e) { + return templateEffectJni(policySrc); // Get effect for template + } + } + /** * Get the JSON representation of the policy. Currently only supports static policies. */ @@ -106,4 +124,6 @@ private static native String parsePolicyTemplateJni(String policyTemplateStr) private native String toJsonJni(String policyStr) throws InternalException, NullPointerException; private static native String fromJsonJni(String policyJsonStr) throws InternalException, NullPointerException; + private native String policyEffectJni(String policyStr) throws InternalException, NullPointerException; + private native String templateEffectJni(String policyStr) throws InternalException, NullPointerException; } diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 419d9af0..18717936 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -387,6 +387,52 @@ fn to_json_internal<'a>(env: &mut JNIEnv<'a>, policy_jstr: JString<'a>) -> Resul } } +#[jni_fn("com.cedarpolicy.model.policy.Policy")] +pub fn policyEffectJni<'a>(mut env: JNIEnv<'a>, _: JClass, policy_jstr: JString<'a>) -> jvalue { + match policy_effect_jni_internal(&mut env, policy_jstr) { + Err(e) => jni_failed(&mut env, e.as_ref()), + Ok(effect) => effect.as_jni(), + } +} + +fn policy_effect_jni_internal<'a>( + env: &mut JNIEnv<'a>, + policy_jstr: JString<'a>, +) -> Result> { + if policy_jstr.is_null() { + raise_npe(env) + } else { + let policy_jstring = env.get_string(&policy_jstr)?; + let policy_string = String::from(policy_jstring); + let policy = Policy::from_str(&policy_string)?; + let policy_effect = serde_json::to_string(&policy.effect())?; + Ok(JValueGen::Object(env.new_string(&policy_effect)?.into())) + } +} + +#[jni_fn("com.cedarpolicy.model.policy.Policy")] +pub fn templateEffectJni<'a>(mut env: JNIEnv<'a>, _: JClass, policy_jstr: JString<'a>) -> jvalue { + match template_effect_jni_internal(&mut env, policy_jstr) { + Err(e) => jni_failed(&mut env, e.as_ref()), + Ok(effect) => effect.as_jni(), + } +} + +fn template_effect_jni_internal<'a>( + env: &mut JNIEnv<'a>, + policy_jstr: JString<'a>, +) -> Result> { + if policy_jstr.is_null() { + raise_npe(env) + } else { + let policy_jstring = env.get_string(&policy_jstr)?; + let policy_string = String::from(policy_jstring); + let policy = Template::from_str(&policy_string)?; + let policy_effect = serde_json::to_string(&policy.effect())?; + Ok(JValueGen::Object(env.new_string(&policy_effect)?.into())) + } +} + #[jni_fn("com.cedarpolicy.model.policy.Policy")] pub fn fromJsonJni<'a>(mut env: JNIEnv<'a>, _: JClass, policy_json_jstr: JString<'a>) -> jvalue { match from_json_internal(&mut env, policy_json_jstr) { From 477c56d4058aab607436f615955f675f5b99c735 Mon Sep 17 00:00:00 2001 From: Mudit Chaudhary Date: Thu, 9 Jan 2025 20:09:35 +0000 Subject: [PATCH 2/5] adds unit tests for get policy effect Signed-off-by: Mudit Chaudhary --- .../java/com/cedarpolicy/PolicyTests.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java index ab293957..4438913c 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java @@ -109,4 +109,36 @@ public void policyFromJsonTest() throws InternalException { String actualJson = p.toJson(); assertEquals(validJson, actualJson); } + + @Test void policyEffectTest() throws InternalException { + + assertThrows(NullPointerException.class, () -> { + Policy p = new Policy(null, null); + p.toJson(); + }); + + // For effects not in {permit, forbid} + assertThrows(InternalException.class, () -> { + Policy p = new Policy("perm(principal == ?principal, action, resource in ?resource);", null); + p.toJson(); + }); + + String actualPermitValue = "\"permit\""; + String actualForbidValue = "\"forbid\""; + + // Tests for static policies + Policy permitPolicy = new Policy("permit(principal, action, resource);", null); + assertEquals(permitPolicy.effect(), actualPermitValue); + + Policy forbidPolicy = new Policy("forbid(principal, action, resource);", null); + assertEquals(forbidPolicy.effect(), actualForbidValue); + + // Tests for templates + Policy permitTemplate = new Policy("permit(principal == ?principal, action, resource == ?resource);", null); + assertEquals(permitTemplate.effect(), actualPermitValue); + + Policy forbidTemplate = new Policy("forbid(principal == ?principal, action, resource == ?resource);", null); + assertEquals(forbidTemplate.effect(), actualForbidValue); + + } } From 0dbed4a226e02effe6ecec93d5434b79ef767361 Mon Sep 17 00:00:00 2001 From: Mudit Chaudhary Date: Fri, 10 Jan 2025 15:37:08 +0000 Subject: [PATCH 3/5] removes enclosing quotes from policy effect Signed-off-by: Mudit Chaudhary --- .../src/test/java/com/cedarpolicy/PolicyTests.java | 4 ++-- CedarJavaFFI/src/interface.rs | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java index 4438913c..f4db7070 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java @@ -123,8 +123,8 @@ public void policyFromJsonTest() throws InternalException { p.toJson(); }); - String actualPermitValue = "\"permit\""; - String actualForbidValue = "\"forbid\""; + String actualPermitValue = "permit"; + String actualForbidValue = "forbid"; // Tests for static policies Policy permitPolicy = new Policy("permit(principal, action, resource);", null); diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 18717936..08c25776 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -406,7 +406,10 @@ fn policy_effect_jni_internal<'a>( let policy_string = String::from(policy_jstring); let policy = Policy::from_str(&policy_string)?; let policy_effect = serde_json::to_string(&policy.effect())?; - Ok(JValueGen::Object(env.new_string(&policy_effect)?.into())) + let policy_effect_unquoted: &str = serde_json::from_str(&policy_effect)?; + Ok(JValueGen::Object( + env.new_string(&policy_effect_unquoted)?.into(), + )) } } @@ -429,7 +432,10 @@ fn template_effect_jni_internal<'a>( let policy_string = String::from(policy_jstring); let policy = Template::from_str(&policy_string)?; let policy_effect = serde_json::to_string(&policy.effect())?; - Ok(JValueGen::Object(env.new_string(&policy_effect)?.into())) + let policy_effect_unquoted: &str = serde_json::from_str(&policy_effect)?; + Ok(JValueGen::Object( + env.new_string(&policy_effect_unquoted)?.into(), + )) } } From d0967d3c48c5dc6ea93991a5644cbc3f3eda1abe Mon Sep 17 00:00:00 2001 From: Mudit Chaudhary Date: Fri, 10 Jan 2025 16:30:54 +0000 Subject: [PATCH 4/5] adds Effect enum; refactors code and tests to use Effect enum Signed-off-by: Mudit Chaudhary --- .../java/com/cedarpolicy/model/Effect.java | 34 +++++++++++++++++++ .../com/cedarpolicy/model/policy/Policy.java | 15 ++++---- .../java/com/cedarpolicy/PolicyTests.java | 16 ++++----- 3 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 CedarJava/src/main/java/com/cedarpolicy/model/Effect.java diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/Effect.java b/CedarJava/src/main/java/com/cedarpolicy/model/Effect.java new file mode 100644 index 00000000..a8ee4995 --- /dev/null +++ b/CedarJava/src/main/java/com/cedarpolicy/model/Effect.java @@ -0,0 +1,34 @@ +package com.cedarpolicy.model; + +/** + * Represents the effect of a Cedar policy. + */ +public enum Effect { + PERMIT, + FORBID; + + /** + * Converts a string to an Effect enum value, case-insensitive. + * + * @param effectString the string value to convert + * @return the corresponding Effect enum value + * @throws NullPointerException if the effectString is null + * @throws IllegalArgumentException if the effectString doesn't match any Effect in {permit, forbid} + */ + public static Effect fromString(String effectString) { + + if (effectString == null) { + throw new NullPointerException(); + } + + switch (effectString.toLowerCase()) { + case "permit": + return PERMIT; + case "forbid": + return FORBID; + default: + throw new IllegalArgumentException("Invalid Effect: " + effectString + ". Expected 'permit' or 'forbid'"); + } + } +} + diff --git a/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java b/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java index 5d1836e0..5f1a7a93 100644 --- a/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java +++ b/CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java @@ -20,6 +20,7 @@ import com.cedarpolicy.model.exception.InternalException; import com.fasterxml.jackson.annotation.JsonProperty; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import com.cedarpolicy.model.Effect; import java.util.concurrent.atomic.AtomicInteger; @@ -79,20 +80,20 @@ public String toString() { } /** - * Returns the effect of a policy. - * - * Determines the policy effect by attempting static policy first, then template. + * Returns the effect of a policy. + * + * Determines the policy effect by attempting static policy first, then template. * In future, it will only support static policies once new class is introduced for Template. - * + * * @return The effect of the policy, either "permit" or "forbid" * @throws InternalException * @throws NullPointerException */ - public String effect() throws InternalException, NullPointerException { + public Effect effect() throws InternalException, NullPointerException { try { - return policyEffectJni(policySrc); // Get effect for static policy + return Effect.fromString(policyEffectJni(policySrc)); // Get effect for static policy } catch (InternalException e) { - return templateEffectJni(policySrc); // Get effect for template + return Effect.fromString(templateEffectJni(policySrc)); // Get effect for template } } diff --git a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java index f4db7070..a4a5de5b 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java @@ -18,6 +18,7 @@ import com.cedarpolicy.model.exception.InternalException; import com.cedarpolicy.model.policy.Policy; +import com.cedarpolicy.model.Effect; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -114,31 +115,28 @@ public void policyFromJsonTest() throws InternalException { assertThrows(NullPointerException.class, () -> { Policy p = new Policy(null, null); - p.toJson(); + p.effect(); }); // For effects not in {permit, forbid} assertThrows(InternalException.class, () -> { Policy p = new Policy("perm(principal == ?principal, action, resource in ?resource);", null); - p.toJson(); + p.effect(); }); - String actualPermitValue = "permit"; - String actualForbidValue = "forbid"; - // Tests for static policies Policy permitPolicy = new Policy("permit(principal, action, resource);", null); - assertEquals(permitPolicy.effect(), actualPermitValue); + assertEquals(permitPolicy.effect(), Effect.PERMIT); Policy forbidPolicy = new Policy("forbid(principal, action, resource);", null); - assertEquals(forbidPolicy.effect(), actualForbidValue); + assertEquals(forbidPolicy.effect(), Effect.FORBID); // Tests for templates Policy permitTemplate = new Policy("permit(principal == ?principal, action, resource == ?resource);", null); - assertEquals(permitTemplate.effect(), actualPermitValue); + assertEquals(permitTemplate.effect(), Effect.PERMIT); Policy forbidTemplate = new Policy("forbid(principal == ?principal, action, resource == ?resource);", null); - assertEquals(forbidTemplate.effect(), actualForbidValue); + assertEquals(forbidTemplate.effect(), Effect.FORBID); } } From b6fd4d0b5f506aaa24b0057627bf22639615d48e Mon Sep 17 00:00:00 2001 From: Mudit Chaudhary Date: Fri, 10 Jan 2025 17:07:16 +0000 Subject: [PATCH 5/5] simplifies policy effect to string conversion in CedarJavaFFI Signed-off-by: Mudit Chaudhary --- CedarJavaFFI/src/interface.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 08c25776..94e30491 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -405,11 +405,8 @@ fn policy_effect_jni_internal<'a>( let policy_jstring = env.get_string(&policy_jstr)?; let policy_string = String::from(policy_jstring); let policy = Policy::from_str(&policy_string)?; - let policy_effect = serde_json::to_string(&policy.effect())?; - let policy_effect_unquoted: &str = serde_json::from_str(&policy_effect)?; - Ok(JValueGen::Object( - env.new_string(&policy_effect_unquoted)?.into(), - )) + let policy_effect = policy.effect().to_string(); + Ok(JValueGen::Object(env.new_string(&policy_effect)?.into())) } } @@ -431,11 +428,8 @@ fn template_effect_jni_internal<'a>( let policy_jstring = env.get_string(&policy_jstr)?; let policy_string = String::from(policy_jstring); let policy = Template::from_str(&policy_string)?; - let policy_effect = serde_json::to_string(&policy.effect())?; - let policy_effect_unquoted: &str = serde_json::from_str(&policy_effect)?; - Ok(JValueGen::Object( - env.new_string(&policy_effect_unquoted)?.into(), - )) + let policy_effect = policy.effect().to_string(); + Ok(JValueGen::Object(env.new_string(&policy_effect)?.into())) } }