Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 63 additions & 10 deletions CedarJava/src/main/java/com/cedarpolicy/model/policy/Policy.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
import com.cedarpolicy.model.Effect;

import java.util.concurrent.atomic.AtomicInteger;

import java.util.HashMap;
import java.util.Map;

/** Policies in the Cedar language. */
public class Policy {
Expand All @@ -36,18 +37,18 @@ public class Policy {
public final String policySrc;
/** Policy ID. */
public final String policyID;
/** Annotations */
private Map<String, String> annotations;

/**
* Creates a Cedar policy object.
*
* @param policy String containing the source code of a Cedar policy in the Cedar policy
* language.
* @param policyID The id of this policy. Must be unique. Note: We may flip the order of the
* arguments here for idiomatic reasons.
* @param policy String containing the source code of a Cedar policy in the Cedar policy language.
* @param policyID The id of this policy. Must be unique. Note: We may flip the order of the arguments here for
* idiomatic reasons.
*/
@SuppressFBWarnings("CT_CONSTRUCTOR_THROW")
public Policy(
@JsonProperty("policySrc") String policy, @JsonProperty("policyID") String policyID)
public Policy(@JsonProperty("policySrc") String policy, @JsonProperty("policyID") String policyID)
throws NullPointerException {

if (policy == null) {
Expand Down Expand Up @@ -82,8 +83,8 @@ public String toString() {
/**
* 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.
* 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
Expand Down Expand Up @@ -114,17 +115,69 @@ public static Policy parseStaticPolicy(String policyStr) throws InternalExceptio
return new Policy(policyText, null);
}

public static Policy parsePolicyTemplate(String templateStr) throws InternalException, NullPointerException {
public static Policy parsePolicyTemplate(String templateStr) throws InternalException, NullPointerException {
String templateText = parsePolicyTemplateJni(templateStr);
return new Policy(templateText, null);
}

/**
* Gets a copy of the policy annotations map. Annotations are loaded lazily when this method is first called. Works
* for both static policies and templates.
*
* @return A new HashMap containing the policy's annotations. For annotations without explicit values, an empty
* string ("") is used as the value
*/
public Map<String, String> getAnnotations() throws InternalException {
ensureAnnotationsLoaded();
return new HashMap<>(this.annotations);
}

/**
* Gets the value of a specific annotation by its key.
*
* @param key The annotation key to look up
* @return The value associated with the annotation key, or null if the key doesn't exist
* @throws InternalException if there is an error loading or parsing the annotations
*/
public String getAnnotation(String key) throws InternalException {
ensureAnnotationsLoaded();
return this.annotations.getOrDefault(key, null);
}

/**
* Ensures that the annotations map is loaded for this policy. If annotations haven't been loaded yet, attempts to
* load them first from static policy, then falls back to template if needed.
*
* @throws InternalException if there is an error loading or parsing the annotations
*/
private void ensureAnnotationsLoaded() throws InternalException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason we can't include this snippet of code that should be run once per instance in the constructor?

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 snippet throws an InternalException, but the current constructor doesn't declare this exception. Moving this code into the constructor would require us to either add throws InternalException to the constructor signature, which would be a breaking change for existing users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good reason for this major version at least 👍

if (annotations == null) {
try {
this.annotations = getPolicyAnnotationsJni(this.policySrc);
} catch (InternalException e) {
if (e.getMessage().contains("expected a static policy")) {
this.annotations = getTemplateAnnotationsJni(this.policySrc);
} else {
throw e;
}
}
}
}

private static native String parsePolicyJni(String policyStr) throws InternalException, NullPointerException;

private static native String parsePolicyTemplateJni(String policyTemplateStr)
throws InternalException, NullPointerException;

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;

private static native Map<String, String> getPolicyAnnotationsJni(String policyStr) throws InternalException;

private static native Map<String, String> getTemplateAnnotationsJni(String policyStr) throws InternalException;
}
89 changes: 88 additions & 1 deletion CedarJava/src/test/java/com/cedarpolicy/PolicyTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,18 @@
import com.cedarpolicy.model.exception.InternalException;
import com.cedarpolicy.model.policy.Policy;
import com.cedarpolicy.model.Effect;
import org.junit.jupiter.api.Test;

import org.junit.jupiter.api.Test;
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;
import static org.junit.jupiter.api.Assertions.fail;

import java.util.Map;
import java.util.HashMap;

public class PolicyTests {
@Test
public void parseStaticPolicyTests() {
Expand Down Expand Up @@ -140,4 +143,88 @@ public void policyEffectTest() throws InternalException {
assertEquals(forbidTemplate.effect(), Effect.FORBID);

}

@Test
public void givenStaticPolicyGetAnnotationsReturns() throws InternalException {
Policy staticPolicy = Policy.parseStaticPolicy("""
@id("policyID1")
@annotation("myAnnotation")
@emptyAnnotation
permit(principal, action, resource);
""");

Map<String, String> expectedMap = new HashMap<>();
expectedMap.put("id", "policyID1");
expectedMap.put("annotation", "myAnnotation");
expectedMap.put("emptyAnnotation", "");

assertEquals(staticPolicy.getAnnotations(), expectedMap);

Policy staticPolicyNoAnnotations = Policy.parseStaticPolicy("""
permit(principal, action, resource);
""");

assertEquals(staticPolicyNoAnnotations.getAnnotations(), new HashMap<>());
}

@Test
public void givenStaticPolicyGetAnnotationReturns() throws InternalException {
Policy staticPolicy = Policy.parseStaticPolicy("""
@id("policyID1")
@annotation("myAnnotation")
@emptyAnnotation
permit(principal, action, resource);
""");

assertEquals(staticPolicy.getAnnotation("annotation"), "myAnnotation");
assertEquals(staticPolicy.getAnnotation("emptyAnnotation"), "");

Policy staticPolicyNoAnnotations = Policy.parseStaticPolicy("""
permit(principal, action, resource);
""");

assertEquals(staticPolicyNoAnnotations.getAnnotation("invalidAnnotation"), null);
}

@Test
public void givenTemplatePolicyGetAnnotationsReturns() throws InternalException {
Policy templatePolicy = Policy.parsePolicyTemplate("""
@id("policyID1")
@annotation("myAnnotation")
@emptyAnnotation
permit(principal == ?principal, action, resource);
""");

Map<String, String> expectedMap = new HashMap<>();
expectedMap.put("id", "policyID1");
expectedMap.put("annotation", "myAnnotation");
expectedMap.put("emptyAnnotation", "");

assertEquals(templatePolicy.getAnnotations(), expectedMap);

Policy templatePolicyNoAnnotations = Policy.parsePolicyTemplate("""
permit(principal == ?principal, action, resource);
""");

assertEquals(templatePolicyNoAnnotations.getAnnotations(), new HashMap<>());
}

@Test
public void givenTemplatePolicyGetAnnotationReturns() throws InternalException {
Policy templatePolicy = Policy.parsePolicyTemplate("""
@id("policyID1")
@annotation("myAnnotation")
@emptyAnnotation
permit(principal == ?principal, action, resource);
""");

assertEquals(templatePolicy.getAnnotation("annotation"), "myAnnotation");
assertEquals(templatePolicy.getAnnotation("emptyAnnotation"), "");

Policy templatePolicyNoAnnotations = Policy.parsePolicyTemplate("""
permit(principal == ?principal, action, resource);
""");

assertEquals(templatePolicyNoAnnotations.getAnnotation("invalidAnnotation"), null);
}
}
Loading
Loading