cedar-policy / cedar

Implementation of the Cedar Policy Language
https://www.cedarpolicy.com
Apache License 2.0
889 stars 80 forks source link

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

Open john-h-kastner-aws opened 2 weeks ago

john-h-kastner-aws commented 2 weeks ago

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.