Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strictly validated policies should be able to compare records with optional attributes #1303

Open
john-h-kastner-aws opened this issue Nov 4, 2024 · 0 comments
Labels
feature-request This issue requets a substantial new feature

Comments

@john-h-kastner-aws
Copy link
Contributor

john-h-kastner-aws commented Nov 4, 2024

Category

Cedar validation features/changes

Describe the feature you'd like to request

Given a type representing an address, and an entity with an associated address,

type Addr = {
  "housenumber": String,
  "street": String, 
  "unit"?: String
};
entity User {
  "addr": Addr
};

We would like to be able to write an expression like this, but it is rejected by the validator:

User::"BigBird".address == {"housenumber": "123", "street": "Sesame Street"}
  × policy set validation failed
  ╰─▶ the types {housenumber: String,street: String,} and {housenumber: String,street: String,unit?: String,} are not compatible
   ╭─[2:3]
 1 │ permit(principal, action, resource) when {
 2 │   User::"BigBird".addr  == {"housenumber": "123", "street": "Sesame Street"}
   ·   ──────────────────────────────────────────────────────────────────────────
 3 │ };
   ╰────
  help: for policy `policy0`, both operands to a `==` expression must have compatible types. Compatible record types must have exactly the same attributes

This happens because the declared type of addr has an extra, optional, attribute unit. The validator assumes the type of the record literal is {"housenumber": String, "street": String} even though it could soundly infer a type with any number of other optional attributes.

Worse, this even won't work if the literal contains a unit attribute. The validator then infers the type {"housenumber": String, "street": String, "unit": String}, i.e., with unit as a required attribute. We can't compare records if the qualifiers on any of their attributes differ even if all the types are the same. The validator could soundly infer that unit should be considered optional in the literal, but it does not.

 × the types {housenumber: String,street: String,unit?: String,} and {housenumber: String,street: String,unit: String,} are not compatible
   ╭─[6:3]
 5 │ permit(principal, action, resource) when {
 6 │   User::"BigBird".addr  == {"housenumber": "123", "street": "Sesame Street", "unit": "4"}
   ·   ───────────────────────────────────────────────────────────────────────────────────────
 7 │ };
   ╰────
  help: for policy `policy1`, both operands to a `==` expression must have compatible types. Corresponding attributes of compatible record types must have the same optionality, either both being required or both being optional

jkastner@88665a03aa97 test % 

This issue also applies to testing sets with contains, containsAll, and containsAny.

Workaround 1

We can avoid this issue by not using optional attributes and instead picking a default value to represent an absent attribute. For example, it doesn't make sense to have an empty string for the unit, so we can reserve "" for representing the case where there is no unit. The Addr type would then be {"housenumber": String, "street": String, "unit": String}, and we would be able to test for {"housenumber": "123", "street": "Sesame Street", "unit": ""} and {"housenumber": "123", "street": "Sesame Street", "unit": "4"}.

This unfortunately relies on knowing a value which won't ever occur as the value of the attribute. If this is a problem, the attribute could be wrapped in another record so that we have "unit": {"value": String, "is_present": Bool}, but this is even more verbose.

This workaround also changes how has works for the attribute. principal.addr has unit will now always return true, which could silently break policies that were written before the schema was modified to apply this workaround.

Workaround 2

Alternatively, we can also avoid the problem by encoding record literals in typed attributes in the context. We would have an action

action Act appliesTo {
  princpal: User,
  resource: R,
  context: {addr_to_test_for: Addr}
};

The expression would then assume that the context is populated with the correct address and be written User::"BigBird".addr == context.addr_to_test_for. This works because we statically know based on the schema that both the attributes have exactly the same type.

This workaround isn't ideal since it depends on preparing the context with the correct constant values for every request that might need it.

Workaround 3

Don't use strict validation. This problem is unique to strict validation, so unvalidated policies and policies written using permissive validation can do these comparisons.

@john-h-kastner-aws john-h-kastner-aws added pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. feature-request This issue requets a substantial new feature and removed pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. labels Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requets a substantial new feature
Projects
None yet
Development

No branches or pull requests

1 participant