Skip to content

Comments

Adds Entities with backward compatibility#293

Merged
muditchaudhary merged 8 commits intocedar-policy:mainfrom
muditchaudhary:addsBackwardCompatEntities
Feb 25, 2025
Merged

Adds Entities with backward compatibility#293
muditchaudhary merged 8 commits intocedar-policy:mainfrom
muditchaudhary:addsBackwardCompatEntities

Conversation

@muditchaudhary
Copy link
Contributor

Description

This PR introduces the Entities class to CedarJava, aligning it more closely with CedarRust's implementation. Key changes include:

  1. New Entities class: Replacement for the current Set<Entity> representation for entity collections. This new class is equivalent to CedarRust's Entities.

  2. Parse Entities from JSON: The Entities class supports parsing entities from both JSON strings and files.

  3. Updated authorization methods:

    • Overloaded isAuthorized() and isAuthorizedPartial() methods to accept the new Entities type.
    • Maintained backward compatibility by retaining support for Set<Entity> in existing method signatures.
  4. Testing:

    • Added unit tests for the new Entities class.
    • Implemented tests to verify backward compatibility of authorization calls using both Entities and Set<Entity>.
    • Introduced org.skyscreamer:jsonassert:2.0-rc1 as a test dependency
      • Enables semantic comparison of JSON structures needed to test Entities using JSONAssertEquals().
      • Provides foundation for future JSON-related testing needs (e.g., .toJson() methods).

Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
…ward compatiblity

Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
@muditchaudhary muditchaudhary marked this pull request as ready for review February 12, 2025 19:05

@Override
public String toString() {
return String.join("\n", this.entities.stream().map(Entity::toString).toList());
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this style match the behavior in Rust where each is printed on a new line?

Copy link
Contributor Author

@muditchaudhary muditchaudhary Feb 24, 2025

Choose a reason for hiding this comment

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

No. The default behavior in Rust is to display the JSON representation of Entities and Entity. .toString() for Entities continues to follow the same display format which was implemented for Entity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like this format for displaying Entity was introduced in CedarJava 2.x. Maybe we can make the behavior similar to Rust in the next major version update as this will change the behavior of an existing public API

Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
Copy link

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Are there no plans to deprecate the methods using Set<Entity> in favor of these new ones? It's great to have these APIs but at the moment it's not clear why anyone would use one over the other.

@muditchaudhary
Copy link
Contributor Author

Are there no plans to deprecate the methods using Set<Entity> in favor of these new ones? It's great to have these APIs but at the moment it's not clear why anyone would use one over the other.

Currently, we have not decided the concrete version when we plan to deprecate the methods using Set<Entity>. The introduction of Entities serves two main purposes: First, it establishes the foundation for accessing CedarRust Entities APIs (such as is_ancestor_of, ancestors). Second, it simplifies the process of loading entities from files/strings, reducing the burden on users who currently need to create entities programmatically. While both approaches will coexist for now, the Entities implementation offers/will offer these additional capabilities that might be useful for the users over time.

Signed-off-by: Mudit Chaudhary <chmudit@amazon.com>
@muditchaudhary muditchaudhary merged commit ef341fd into cedar-policy:main Feb 25, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants