Generalized in operator
#18
Conversation
cdisselkoen
added
the
pending
text/0018-generalized-in.md
Outdated
| ### Alternative 5: `in` always means set-membership when RHS is a set | ||
|
|
||
| This would be a breaking change that changes the current meaning of `principal.manager in [Group::"A", Group::"B"]`. The resulting semantics would be simpler as a result, and Examples 1 and 2 would still look the same as in the main proposal. | ||
|
|
||
| In this alternative, users wouldn't have any way to get the current "deep" semantics. Some ways to address that: | ||
| * We could add a new keyword that has the current "deep" semantics -- say, `inAny` | ||
| * If we eventually introduce a more general feature to map operators over sets, users could get the "deep" semantics by mapping `in` over a set | ||
| * If the RHS is a literal, users could just write out `a in G1 || a in G2 || ...`. This doesn't help for the case where the RHS is not a literal, e.g. today's `User::"Henry" in resource.allowedGroups` | ||
|
|
||
| Reasons not to take this alternative: | ||
| * It would be a breaking change for some existing Cedar policies | ||
| * If we add a new keyword like `inAny`, the distinction between `in` and `inAny` could cause confusion | ||
| * At least one user has opined that it's unintuitive that `John in [SalesTeam::"California", SalesTeam::"Oregon"]` would have different semantics than `John in SalesTeam::"WestCoast"` (assuming the natural group memberships there) |
There was a problem hiding this comment.
I have a slight preference for this option, in conjunction with Alternative 2 (removing contains). It feels cleaner to me to have a consistent behavior when the RHS of in is a set (i.e., always shallow). I also like having a separate inAny operation for when you want deep membership.
There was a problem hiding this comment.
IMO this change is a bad idea without alternative 5. Alternative 5 is a large enough breaking change that I oppose this RFC.
| Arguably, this is the "intuitive" semantics that users want for `in` -- it blends "deep" `in` for entity references while also providing shallow set-membership for sets. | ||
|
|
There was a problem hiding this comment.
Is "intuitiveness" something you can test? What if we did a Quiz on the Cedar Slack to ask people's expectations for various examples?
| 1. This is simply awkward in some use cases, such as the two examples above. Subjectively, the `.contains()` syntax is hard to read in these cases, because the set is required to be on the LHS and the value you're actually interested in is required to be on the RHS. In these examples, having the set on the RHS instead of the LHS reads more intuitively. | ||
| 2. Anecdotally, many users intuitively expect that `in` can be used for set membership and not just hierarchy membership, and are confused when this results in Cedar type errors. See, for example, [this thread](https://cedar-policy.slack.com/archives/C0547KH7R19/p1689253188448809) on the Cedar Slack. | ||
|
|
||
| ## Detailed design |
There was a problem hiding this comment.
What is the impact of this change on validation?
text/0018-generalized-in.md
Outdated
|
|
||
| ## Drawbacks | ||
|
|
||
| - The new semantics is more complicated than Cedar's current semantics for `in`. This could lead to additional confusion. |
There was a problem hiding this comment.
One drawback not listed here is that it is harder to do static analysis with this in, which may make implementing features (especially optimizing evaluation, analysis of evaluation using smt, and compiling expressions to other languages) more difficult in the future. (I'm coming from the point of view of currently working on compiling cedar expressions to e.g. SQL). While it's somewhat tempting to make cedar more "dynamic" by combining operators like "in" and "contains" and essentially deciding at runtime which behavior to use based on the runtime types, I think this will make it harder to analyze.
|
I've updated the RFC to note its current status, which we're terming "archived" -- it doesn't currently have much support, but also we're open to resuming the discussion on it at any time. Feel free to leave a comment and resume the discussion. But in absence of any activity or popular demand, the Cedar team does not plan to take any action on this RFC in the near future. |
cdisselkoen
added
archived
Rendered
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.