diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 34a6a7af..80036cd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,5 +24,5 @@ jobs: needs: get-branch-name uses: ./.github/workflows/run_cedar_java_reusable.yml with: - cedar_policy_ref: "refs/heads/main" # use the latest commit on main + cedar_policy_ref: ${{ needs.get-branch-name.outputs.branch_name }} # use the same branch of cedar-policy cedar_java_ref: "${{ github.href }}" # use the current PR's commit diff --git a/CedarJava/CHANGELOG.md b/CedarJava/CHANGELOG.md index 81d7c07c..5d1490ae 100644 --- a/CedarJava/CHANGELOG.md +++ b/CedarJava/CHANGELOG.md @@ -1,6 +1,23 @@ # Changelog -## Unreleased +## 4.11.0 +* Pinned to `cedar-policy 4.11.2` +* Added `PolicyParseException`, thrown by `PolicySet.parsePolicies` when policy text fails to parse. It is a subclass of `InternalException`, so existing `catch` blocks and `getMessage()` are unaffected, and adds `getDetailedErrors()` returning Cedar's structured diagnostics - source span, expected tokens, and help text - for each error. `getErrors()` now carries one entry per parse error rather than a single entry for the whole document [#367](https://github.com/cedar-policy/cedar-java/pull/367) +* Adds Type-Aware Partial Evaluation (TPE) support [#372](https://github.com/cedar-policy/cedar-java/pull/372). This is an experimental feature in cedar-policy and can change behavior in future releases (See [experimental-features](https://github.com/cedar-policy/rfcs/blob/main/README.md#experimental-features)). + +## 4.10.0 +* Pinned to `cedar-policy 4.10.0` + +## 4.9.0 +* Pinned to `cedar-policy 4.9.1` +* Improve toCedarExpr() correctness and add CedarStrings utility [#351](https://github.com/cedar-policy/cedar-java/pull/351) +* Add PolicySet and Schema caching [#350](https://github.com/cedar-policy/cedar-java/pull/350) +* Optimize JNI performance and fix Decimal validation [#349](https://github.com/cedar-policy/cedar-java/pull/349) +* * Updated Java source and target compatibility from 1.8 to 17 aligning with the pre-requisites in README [#350](https://github.com/cedar-policy/cedar-java/pull/350) +* Bugfix: EntityIdentifier equals implementation [#352](https://github.com/cedar-policy/cedar-java/pull/352) + + +## 4.8.0 * Added Schema conversion APIs [#325](https://github.com/cedar-policy/cedar-java/pull/325) * Added Level Validation [#327](https://github.com/cedar-policy/cedar-java/pull/327) * Added DateTime extension support [#328](https://github.com/cedar-policy/cedar-java/pull/328) @@ -8,7 +25,6 @@ * Added Offset function support [#331](https://github.com/cedar-policy/cedar-java/pull/331) * Added PolicySet to JSON conversion API [#329](https://github.com/cedar-policy/cedar-java/pull/329) * Added Cedar Schema support for Entity Validation [#332](https://github.com/cedar-policy/cedar-java/pull/332) -* Added `PolicyParseException`, thrown by `PolicySet.parsePolicies` when policy text fails to parse. It is a subclass of `InternalException`, so existing `catch` blocks and `getMessage()` are unaffected, and adds `getDetailedErrors()` returning Cedar's structured diagnostics - source span, expected tokens, and help text - for each error. `getErrors()` now carries one entry per parse error rather than a single entry for the whole document [#367](https://github.com/cedar-policy/cedar-java/pull/367) ## 4.3.1 ### Added diff --git a/CedarJava/build.gradle b/CedarJava/build.gradle index 45f0b000..28167ba2 100644 --- a/CedarJava/build.gradle +++ b/CedarJava/build.gradle @@ -230,12 +230,17 @@ tasks.register('uberJar', Jar) { from(layout.buildDirectory.dir(compiledLibDir)) } +// The cedar-integration-tests branch to test against; should match the cedar-policy branch in CedarJavaFFI/Cargo.toml. +def IntegrationTestsBranch = 'release/4.11.x' +// GitHub names the zip's top-level folder after the branch, with `/` replaced by `-`. +def IntegrationTestsZipRoot = 'cedar-integration-tests-' + IntegrationTestsBranch.replace('/', '-') + tasks.register('downloadIntegrationTests', Download) { group 'Build' description 'Downloads Cedar repository with integration tests.' - src 'https://codeload.github.com/cedar-policy/cedar-integration-tests/zip/main' - dest layout.buildDirectory.file('cedar-integration-tests-main.zip') + src "https://codeload.github.com/cedar-policy/cedar-integration-tests/zip/${IntegrationTestsBranch}" + dest layout.buildDirectory.file("${IntegrationTestsZipRoot}.zip") overwrite false } @@ -245,8 +250,11 @@ tasks.register('extractIntegrationTests', Copy) { dependsOn('checkstyleTest') dependsOn('downloadIntegrationTests') - from zipTree(layout.buildDirectory.file('cedar-integration-tests-main.zip')) + from zipTree(layout.buildDirectory.file("${IntegrationTestsZipRoot}.zip")) into layout.buildDirectory.dir('resources/test') + // Extract under a fixed folder name so the tests need not know which branch was downloaded. + eachFile { it.path = it.path.replaceFirst("^${IntegrationTestsZipRoot}/", 'cedar-integration-tests/') } + includeEmptyDirs = false } tasks.register('extractCorpusTests', Copy) { @@ -255,8 +263,8 @@ tasks.register('extractCorpusTests', Copy) { dependsOn('extractIntegrationTests') dependsOn('processTestResources') - from tarTree(layout.buildDirectory.file('resources/test/cedar-integration-tests-main/corpus-tests.tar.gz')) - into layout.buildDirectory.dir('resources/test/cedar-integration-tests-main') + from tarTree(layout.buildDirectory.file('resources/test/cedar-integration-tests/corpus-tests.tar.gz')) + into layout.buildDirectory.dir('resources/test/cedar-integration-tests') } tasks.named('test') { @@ -324,7 +332,7 @@ publishing { from components.java groupId = 'com.cedarpolicy' artifactId = 'cedar-java' - version = '3.1.2' + version = '4.11.0' artifacts { jar diff --git a/CedarJava/src/test/java/com/cedarpolicy/PartialEntitiesTests.java b/CedarJava/src/test/java/com/cedarpolicy/PartialEntitiesTests.java index 10f1fff2..67f84e83 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/PartialEntitiesTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/PartialEntitiesTests.java @@ -39,6 +39,7 @@ import static com.cedarpolicy.TestUtil.assertMessageContains; import static com.cedarpolicy.TestUtil.buildEuidObject; import static com.cedarpolicy.TestUtil.buildUidObject; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -58,13 +59,11 @@ public void testChecksAgainstCedarFormatSchema() throws InternalException { new PartialEntity(alice, Optional.empty(), Optional.empty(), Optional.empty(), TPE_SCHEMA_CEDAR)); assertEquals(1, new PartialEntities(entities, TPE_SCHEMA_CEDAR).getEntities().size()); - // A collection-level rule, which only the collection validator can catch. + // Accepted, as with the JSON-format schema: see `testAcceptsDuplicateUid`. var duplicated = Set.of( new PartialEntity(alice, Optional.empty(), Optional.empty(), Optional.empty(), TPE_SCHEMA_CEDAR), new PartialEntity(alice, Optional.of(Map.of()), Optional.empty(), Optional.empty(), TPE_SCHEMA_CEDAR)); - InternalException e = - assertThrows(InternalException.class, () -> new PartialEntities(duplicated, TPE_SCHEMA_CEDAR)); - assertMessageContains(e, "duplicate entity entry", "User::\"alice\""); + assertDoesNotThrow(() -> new PartialEntities(duplicated, TPE_SCHEMA_CEDAR)); } @Test @@ -77,7 +76,7 @@ public void testRejectsParentWithUnknownParents() throws InternalException { new PartialEntity(admins, Optional.empty(), Optional.empty(), Optional.empty(), TPE_SCHEMA)); InternalException e = assertThrows(InternalException.class, () -> new PartialEntities(entities, TPE_SCHEMA)); - assertMessageContains(e, "ancestor `Group::\"admins\"`", "of `User::\"alice\"`", "has unknown ancestors"); + assertMessageContains(e, "User::\"alice\"", "ancestor `Group::\"admins\"`", "has unknown ancestors"); } @Test @@ -97,15 +96,14 @@ public void testAllowsParentAbsentFromTheCollection() throws InternalException { assertEquals(2, withParent.getEntities().size()); } + /** Cedar 4.11 does not reject a repeated uid in partial entities, and CedarJava follows Cedar. */ @Test - public void testRejectsDuplicateUid() throws InternalException { + public void testAcceptsDuplicateUid() throws InternalException { var alice = new EntityUID(EntityTypeName.parse("User").get(), "alice"); var entities = Set.of( new PartialEntity(alice, Optional.empty(), Optional.empty(), Optional.empty(), TPE_SCHEMA), new PartialEntity(alice, Optional.of(Map.of()), Optional.empty(), Optional.empty(), TPE_SCHEMA)); - InternalException e = - assertThrows(InternalException.class, () -> new PartialEntities(entities, TPE_SCHEMA)); - assertMessageContains(e, "duplicate entity entry", "User::\"alice\""); + assertDoesNotThrow(() -> new PartialEntities(entities, TPE_SCHEMA)); } @Test @@ -222,8 +220,8 @@ public void testFromJsonRoundTrip() throws InternalException { } @Test - public void testFromJsonRejectsDuplicateUid() { - // The same rule as `testRejectsDuplicateUid`, reached through the JSON encoding. Worth covering separately + public void testFromJsonAcceptsDuplicateUid() { + // The same rule as `testAcceptsDuplicateUid`, reached through the JSON encoding. Worth covering separately // because a JSON array can carry two entries with the same uid, whereas a `Set` relies on the elements // differing to keep both. ObjectNode first = JsonNodeFactory.instance.objectNode(); @@ -233,9 +231,7 @@ public void testFromJsonRejectsDuplicateUid() { second.set("attrs", JsonNodeFactory.instance.objectNode()); ArrayNode json = JsonNodeFactory.instance.arrayNode().add(first).add(second); - InternalException e = - assertThrows(InternalException.class, () -> PartialEntities.fromJson(json, TPE_SCHEMA)); - assertMessageContains(e, "duplicate entity entry", "User::\"alice\""); + assertDoesNotThrow(() -> PartialEntities.fromJson(json, TPE_SCHEMA)); } @Test diff --git a/CedarJava/src/test/java/com/cedarpolicy/SharedIntegrationTests.java b/CedarJava/src/test/java/com/cedarpolicy/SharedIntegrationTests.java index 4810160f..68ccc9bd 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/SharedIntegrationTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/SharedIntegrationTests.java @@ -83,7 +83,7 @@ private Path resolveIntegrationTestPath(String path) { if (resolved.isAbsolute()) { return resolved; } else { - final URL integrationTestsLocation = getClass().getResource("/cedar-integration-tests-main"); + final URL integrationTestsLocation = getClass().getResource("/cedar-integration-tests"); return integrationTestsLocation == null ? resolved : Paths.get(integrationTestsLocation.getPath(), path); } } diff --git a/CedarJava/src/test/java/com/cedarpolicy/TypeAwarePartialAuthorizationEngineTests.java b/CedarJava/src/test/java/com/cedarpolicy/TypeAwarePartialAuthorizationEngineTests.java index 6e89f76a..e87c1967 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/TypeAwarePartialAuthorizationEngineTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/TypeAwarePartialAuthorizationEngineTests.java @@ -109,7 +109,7 @@ public void tpeUnknownPrincipalIdYieldsAResidual() { assertEquals(Set.of("p0"), TestUtil.ids(success.getNontrivialResiduals())); // The principal id is unknown, so `principal` survives as a variable. assertEquals("permit(principal, action, resource) when " - + "{ (principal has department) && (principal.department == \"eng\") };", + + "{ (principal has department) && ((principal.department) == \"eng\") };", success.getResiduals().iterator().next().getSource()); }); } @@ -145,7 +145,7 @@ public void tpeResidualTextShowsTheFoldedCondition() { assertEquals("p0", residual.getID()); assertEquals("@owner(\"team-a\")\npermit(principal, action, resource) when " - + "{ (User::\"alice\" has department) && (User::\"alice\".department == \"eng\") };", + + "{ (User::\"alice\" has department) && ((User::\"alice\".department) == \"eng\") };", residual.getSource()); // The annotation survives, and the residual is still a usable Policy because nothing errored. assertEquals(Map.of("owner", "team-a"), residual.getAnnotations()); @@ -209,7 +209,7 @@ public void tpeUnknownAttrsYieldsAResidual() { // The principal id is known here, unlike the test above, so `principal` is substituted with the // entity — which is why a residual must not be evaluated for a different principal. assertEquals("permit(principal, action, resource) when " - + "{ (User::\"alice\" has department) && (User::\"alice\".department == \"eng\") };", + + "{ (User::\"alice\" has department) && ((User::\"alice\".department) == \"eng\") };", success.getResiduals().iterator().next().getSource()); }); } @@ -590,7 +590,7 @@ public void tpeNestedErrorIsBothNontrivialAndErrored() { assertEquals(Set.of("p0"), success.getErroredResidualIds(), "a nested error must still be reported, which error_permits() alone would not do"); assertEquals("permit(principal, action, resource) when " - + "{ (User::\"alice\" has department) || error() };", + + "{ (User::\"alice\" has department) || (error()) };", success.getResiduals().iterator().next().getSource()); }); } @@ -638,35 +638,34 @@ record Mismatch(String label, AuthorizationRequest request, Entities entities, S new AuthorizationRequest(bob, TPE_VIEW, TPE_DOOR, new HashMap()), new Entities(Set.of(aliceEng, new Entity(bob, Map.of("department", new PrimString("eng")), Set.of(), Map.of()))), - "partial request principal id `alice` does not match concrete request principal id `bob`"), + "Principal eid `alice` and `bob` do not match"), new Mismatch("resource id", new AuthorizationRequest(TPE_ALICE, TPE_VIEW, window, new HashMap()), new Entities(Set.of(aliceEng)), - "partial request resource id `door` does not match concrete request resource id `window`"), + "Resource eid `door` and `window` do not match"), new Mismatch("action", new AuthorizationRequest(TPE_ALICE, edit, TPE_DOOR, new HashMap()), new Entities(Set.of(aliceEng)), - "does not match concrete request action"), + "Actions `Action::\"view\"` and `Action::\"edit\"` do not match"), new Mismatch("context", new AuthorizationRequest(TPE_ALICE, TPE_VIEW, TPE_DOOR, new HashMap<>(Map.of("authenticated", new PrimBool(true)))), new Entities(Set.of(aliceEng)), - "the partial and concrete request contexts do not match"), + "Contexts are inconsistent"), new Mismatch("entity missing from the concrete store", new AuthorizationRequest(TPE_ALICE, TPE_VIEW, TPE_DOOR, new HashMap()), new Entities(Set.of()), - "entity `User::\"alice\"` is present in the partial entities but missing from the " - + "concrete entities"), + "Concrete entities does not include `User::\"alice\"`"), new Mismatch("parents", new AuthorizationRequest(TPE_ALICE, TPE_VIEW, TPE_DOOR, new HashMap()), new Entities(Set.of(new Entity(TPE_ALICE, Map.of("department", new PrimString("eng")), Set.of(group), Map.of()))), - "concrete entity `User::\"alice\"` has ancestors that do not match the partial entity"), + "Entity `User::\"alice\"`'s ancestors do not match"), new Mismatch("tags", new AuthorizationRequest(TPE_ALICE, TPE_VIEW, TPE_DOOR, new HashMap()), new Entities(Set.of(new Entity(TPE_ALICE, Map.of("department", new PrimString("eng")), Set.of(), Map.of("clearance", new PrimString("high"))))), - "concrete entity `User::\"alice\"` has tag values that do not match the partial entity")); + "Entity `User::\"alice\"`'s tags do not match")); for (var m : mismatches) { var response = success.reauthorize(m.request(), m.entities()); @@ -708,7 +707,7 @@ public void tpeReauthorizeRejectsAChangedAttributeValue() { new Entity(TPE_ALICE, Map.of("department", new PrimString("sales")), Set.of(), Map.of())))); assertEquals(SuccessOrFailure.Failure, changed.type); assertTrue(changed.errors.orElseThrow().get(0).message.contains( - "concrete entity `User::\"alice\"` has attribute values that do not match the partial entity"), + "Entity `User::\"alice\"`'s attributes do not match"), "unexpected message: " + changed.errors.orElseThrow().get(0).message); }); } @@ -750,7 +749,7 @@ public void tpeReauthorizeRejectsAChangedPrincipalTypeWhenTheIdWasUnknown() { new Entity(root, Map.of("department", new PrimString("eng")), Set.of(), Map.of())))); assertEquals(SuccessOrFailure.Failure, asAdmin.type); assertTrue(asAdmin.errors.orElseThrow().get(0).message.contains( - "partial request principal type `User` does not match concrete request principal type `Admin`"), + "Principal types `User` and `Admin` do not match"), "unexpected message: " + asAdmin.errors.orElseThrow().get(0).message); }); } diff --git a/CedarJava/src/test/java/com/cedarpolicy/ValidationTests.java b/CedarJava/src/test/java/com/cedarpolicy/ValidationTests.java index fc60bcca..d3943c70 100644 --- a/CedarJava/src/test/java/com/cedarpolicy/ValidationTests.java +++ b/CedarJava/src/test/java/com/cedarpolicy/ValidationTests.java @@ -80,7 +80,7 @@ public void givenExampleSchemaAndCorrectPolicyReturnsValid() { /** Test. */ @Test - public void givenExampleSchemaAndImpossiblePolicyReturnsWarning() { + public void givenExampleSchemaAndIncorrectPolicyReturnsValid() { givenSchema(PHOTOFLASH_SCHEMA); givenPolicy( "policy0", @@ -90,9 +90,9 @@ public void givenExampleSchemaAndImpossiblePolicyReturnsWarning() { + " resource == User::\"bob\"" + ");"); ValidationResponse response = whenValidated(); - thenHasWarnings(response); + thenIsNotValid(response); ValidationResponse levelResponse = whenLevelValidated(1); - thenHasWarnings(levelResponse); + thenIsNotValid(levelResponse); } /** Test. */ @@ -169,16 +169,16 @@ public void validateTemplateLinkedPolicyFailsWhenExpected() { thenValidationFailed(levelResponse3); - // validation warns if we provide a link with the wrong type + // validation returns an error if we provide a link with the wrong type LinkValue badLink1 = new LinkValue("?resource", EntityUID.parse("Library::User::\"Victor\"").get()); this.policies = new PolicySet(new HashSet<>(), templates, List.of(new TemplateLink("template1", "policy", List.of(badLink1)))); ValidationResponse response4 = whenValidated(); - thenHasWarnings(response4); + thenIsNotValid(response4); ValidationResponse levelResponse4 = whenLevelValidated(1); - thenHasWarnings(levelResponse4); + thenIsNotValid(levelResponse4); - // validation returns an error if we provide a link with an unrecognized type + // validation returns an error if we provide a link with an invalid type LinkValue badLink2 = new LinkValue("?resource", EntityUID.parse("Library::BOOK::\"The black Swan\"").get()); this.policies = new PolicySet(new HashSet<>(), templates, List.of(new TemplateLink("template1", "policy", List.of(badLink2)))); @@ -284,22 +284,6 @@ private void thenIsNotValid(ValidationResponse response) { }); } - private void thenHasWarnings(ValidationResponse response) { - assertEquals(response.type, SuccessOrFailure.Success); - final ValidationSuccessResponse success = assertDoesNotThrow(() -> response.success.get()); - assertTrue( - success.validationErrors.isEmpty(), - () -> { - String errors = response.success.get().validationErrors.stream() - .map(note -> String.format("in policy %s: %s", note.getPolicyId(), note.getError())) - .collect(Collectors.joining("\n")); - return "Expected no validation errors but got:\n" + errors; - }); - assertFalse( - success.validationWarnings.isEmpty(), - () -> "Expected validation warnings but did not find any"); - } - private void thenValidationFailed(ValidationResponse response) { assertEquals(response.type, SuccessOrFailure.Failure); final List errors = assertDoesNotThrow(() -> response.errors.get()); diff --git a/CedarJavaFFI/Cargo.toml b/CedarJavaFFI/Cargo.toml index aea20ba3..8639fc0f 100644 --- a/CedarJavaFFI/Cargo.toml +++ b/CedarJavaFFI/Cargo.toml @@ -6,7 +6,7 @@ description = "Java FFI for Cedar (from the cedar-policy crate)." edition = "2021" -version = "4.0.0" +version = "4.11.0" [dependencies] serde = { version = "1.0", features = ["derive", "rc"] } @@ -33,11 +33,11 @@ jni = { version = "0.21.1", features = ["invocation"] } crate_type = ["cdylib"] [dependencies.cedar-policy] -version = "4.0.0" +version = "4.11.2" git = "https://github.com/cedar-policy/cedar" -branch = "main" +branch = "release/4.11.x" [dependencies.cedar-policy-formatter] -version = "4.0.0" +version = "4.11.2" git = "https://github.com/cedar-policy/cedar" -branch = "main" +branch = "release/4.11.x" diff --git a/CedarJavaFFI/src/interface.rs b/CedarJavaFFI/src/interface.rs index 3c511839..8c10e8b5 100644 --- a/CedarJavaFFI/src/interface.rs +++ b/CedarJavaFFI/src/interface.rs @@ -1560,6 +1560,7 @@ pub(crate) mod jvm_based_tests { assert!(!env.exception_check().unwrap()); } + /// Cedar 4.11 accepts a repeated uid (the later entry wins), and so does this crate. #[test] fn validate_partial_entities_internal_duplicate_uid() { let mut env = JVM.attach_current_thread().unwrap(); @@ -1570,10 +1571,10 @@ pub(crate) mod jvm_based_tests { let entities_jstr = env.new_string(entities.to_string()).unwrap(); let schema_jstr = env.new_string(schema_cedar_src()).unwrap(); - assert_err_contains( - validate_partial_entities_internal(&mut env, entities_jstr, schema_jstr), - &["duplicate entity entry", "User::\"alice\""], - ); + let result = + validate_partial_entities_internal(&mut env, entities_jstr, schema_jstr).unwrap(); + assert_eq!(success_string(&mut env, result), "success"); + assert!(!env.exception_check().unwrap()); } #[test] @@ -1653,10 +1654,10 @@ pub(crate) mod jvm_based_tests { ]); let duplicated_jstr = env.new_string(duplicated.to_string()).unwrap(); let schema_jstr = env.new_string(schema_json()).unwrap(); - assert_err_contains( - validate_partial_entities_internal(&mut env, duplicated_jstr, schema_jstr), - &["duplicate entity entry", "User::\"alice\""], - ); + let result = + validate_partial_entities_internal(&mut env, duplicated_jstr, schema_jstr).unwrap(); + assert_eq!(success_string(&mut env, result), "success"); + assert!(!env.exception_check().unwrap()); } #[test] diff --git a/CedarJavaFFI/src/tests.rs b/CedarJavaFFI/src/tests.rs index 91da11cb..0073623f 100644 --- a/CedarJavaFFI/src/tests.rs +++ b/CedarJavaFFI/src/tests.rs @@ -1184,8 +1184,8 @@ mod tpe_validation_tests { assert_err_contains( validate_partial_entities(&entities.to_string(), &schema_cedar_src()), &[ + "User::\"alice\"", "ancestor `Group::\"admins\"`", - "of `User::\"alice\"`", "has unknown ancestors", ], ); @@ -1206,8 +1206,10 @@ mod tpe_validation_tests { ); } + /// Cedar 4.11's `PartialEntities::from_json_value` does not reject a repeated uid — the later entry + /// replaces the earlier one — and this crate follows Cedar rather than adding its own check. #[test] - fn validate_partial_entities_with_duplicate_uid_fails() { + fn validate_partial_entities_with_duplicate_uid_succeeds() { let entities = json!([ { "uid": { "type": "User", "id": "alice" }, @@ -1220,9 +1222,9 @@ mod tpe_validation_tests { "parents": [] } ]); - assert_err_contains( + assert_matches!( validate_partial_entities(&entities.to_string(), &schema_cedar_src()), - &["duplicate entity entry", "User::\"alice\""], + Ok(()) ); } @@ -1815,9 +1817,10 @@ mod tpe_validation_tests { { "uid": { "type": "User", "id": "alice" }, "attrs": { "isAdmin": false }, "parents": [] }, { "uid": { "type": "User", "id": "alice" }, "attrs": { "isAdmin": true }, "parents": [] } ]); - assert_err_contains( + // Accepted, as with the Cedar-format schema: see `validate_partial_entities_with_duplicate_uid_succeeds`. + assert_matches!( validate_partial_entities(&duplicated.to_string(), &schema_json()), - &["duplicate entity entry", "User::\"alice\""], + Ok(()) ); } diff --git a/CedarJavaFFI/src/tpe.rs b/CedarJavaFFI/src/tpe.rs index aea781c5..ebf50e5b 100644 --- a/CedarJavaFFI/src/tpe.rs +++ b/CedarJavaFFI/src/tpe.rs @@ -256,6 +256,22 @@ fn plain(message: String) -> DetailedError { } } +/// Cedar 4.11's `TpeError` is not a `miette::Diagnostic`, and its `Validation` variant displays as a +/// `{:#?}` dump. Rebuild the shape later Cedar versions produce — one "failed to validate" error with +/// each validation error, help text and source span intact, under `related` — so callers see the same +/// failure on either Cedar line. The other variants carry only a message. +#[cfg(feature = "tpe")] +fn tpe_error_details(e: cedar_policy::tpe_err::TpeError) -> Vec { + use cedar_policy::tpe_err::TpeError; + match e { + TpeError::Validation(errs) => vec![DetailedError { + related: errs.into_iter().map(detailed).collect(), + ..plain("policy failed to validate against the schema".to_string()) + }], + e => vec![plain(e.to_string())], + } +} + #[cfg(feature = "tpe")] fn tpe_failure(message: String) -> TypeAwarePartialAuthAnswer { tpe_failure_detailed(vec![plain(message)]) @@ -306,8 +322,9 @@ impl TypeAwarePartialAuthCall { .map(|c| Context::from_json_value(c, Some((&schema, &action)))) .transpose() .map_err(|e| vec![detailed(e)])?; + // Cedar 4.11's `tpe_err::EntitiesError` is not a `miette::Diagnostic`. let entities = PartialEntities::from_json_value(self.entities, &schema) - .map_err(|e| vec![detailed(e)])?; + .map_err(|e| vec![plain(e.to_string())])?; let request = PartialRequest::new(principal, action, resource, context, &schema) .map_err(|e| vec![detailed(e)])?; Ok(ParsedCall { @@ -361,12 +378,13 @@ fn type_aware_partial_is_authorized(input: &str) -> TypeAwarePartialAuthAnswer { }; let response = match policies.tpe(&request, &entities, &schema) { Ok(response) => response, - Err(e) => return tpe_failure_detailed(vec![detailed(e)]), + Err(e) => return tpe_failure_detailed(tpe_error_details(e)), }; let mut residuals = HashMap::new(); let mut errored_residuals = HashSet::new(); - for p in response.policies() { + // In Cedar 4.11 `residual_policies` yields every residual, trivial or not. + for p in response.residual_policies() { let id = p.id().to_string(); if or_failure!(residual_errors(&p)) { errored_residuals.insert(id.clone()); @@ -448,7 +466,10 @@ fn type_aware_partial_reauthorize(input: &str) -> cedar_policy::ffi::Authorizati Ok(parsed) => parsed, Err(errors) => return auth_failure(errors), }; - let response = or_failure!(policies.tpe(&request, &entities, &schema)); + let response = match policies.tpe(&request, &entities, &schema) { + Ok(response) => response, + Err(e) => return auth_failure(tpe_error_details(e)), + }; let concrete = call.concrete_request; let action = or_failure!(EntityUid::from_json(concrete.action));