Change the semantics of appliesTo fields in the schema

[khieta]

Rendered

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[max2me]

Kesha @khieta, thank you for converting issue to the RFC, it looks great!

[khieta]

@max2me Thanks for filing a clear issue that was easy to convert 😄

[mwhicks1]

Looks great!

[cdisselkoen]

Approving as-is, but I do have a question about an alternative that's not currently discussed in the RFC.

[john-h-kastner-aws]

The new semantics sound good.

[khieta]

⚠️ Major revisions in the latest commit! ⚠️

Offline discussion on backwards compatibility revealed that this RFC was more backwards compatible then we originally thought: omitting the appliesTo field (or subfields) means that an action applies to the unspecified entity (not any entity) in all released versions of Cedar. This means that the expression principal == User::"alice" will be typed as False (not Bool) in cases where principal is unspecified. Thanks @mwhicks1 for pointing this out.

[mwhicks1]

Some nits.

[mwhicks1]

Looks great. I'm not bothered by the backward incompatibility problem given here but if that's a showstopper then I'll wait to see what the proposed alternative might be.

Dismissing my approval (for now) pending resolution of the backwards compatibility / migration-related issues brought up in this thread and this thread. Whatever decision is reached should be reflected in the RFC text.

[khieta]

I've added two new alternatives + an unresolved question to summarize our discussions so far 🎉 Starting fresh threads for more targeted discussion.

[khieta]

Update: re-starting discussion on this RFC 🎉 I've just pushed a major update that switches the proposal to what was originally "Alternative A". I feel that this is a more acceptable solution for users that currently have appliesTo.principalTypes: [] and/or appliesTo.resourceTypes: [] in their schemas.

[Swolebrain]

I don't see why this is inconsistent with RFC24. Maybe there's something I'm missing but I don't see why RFC24 needs to map exactly 1:1 to json format. There could be syntactic sugar to it. For instance, both of the following:

• action ActionGroup;
• action ActionGroup appliesTo { principalTypes: [], resourceTypes: [], context: {} };

Could transpile to the same thing in json:

"ActionGroup": {
    "appliesTo": {
        "principalTypes": [],
        "resourceTypes": [],
        "context": {}
    }
},

could they not?

[khieta]

It's true that RFC24 doesn't need to map 1:1 to the JSON format: we can easily write a translator between the two syntaxes for the current proposal.

In RFC24, action ActionGroup appliesTo { principalTypes: [], resourceTypes: [], context: {} }; would not be valid syntax, since empty lists are not allowed.

[cdisselkoen]

I don't see why this is inconsistent with RFC24. Maybe there's something I'm missing but I don't see why RFC24 needs to map exactly 1:1 to json format.

I think we're all in agreement -- it is "inconsistent" which might be a poor UX / confusing for any folks dealing with both the JSON and RFC24 formats. But "inconsistent" does not mean "incompatible", and we can easily translate between the two even if they do not use precisely the same syntax for some edge cases.

[cdisselkoen]

Instead of "the current implementation", we could more precisely state "Cedar 2.x and 3.0" or even "Cedar 2.x and 3.x" (here and in other places in this RFC). The tradeoff is that as this RFC text lives to posterity, we may not keep updating this text with which Cedar versions have the described behavior. Nonetheless, the additional clarity might be worth it.

[khieta]

After much debate, we've decided to abandon this RFC for the following reasons:

1. The current proposal is a breaking change without a large benefit: it just disallows up a corner case in the schema format. We'd prefer a less invasive change, such as returning a warning rather than failing to parse previously parseable inputs. Cedar doesn't currently allow warnings during schema construction, but may be extended with this in the future.
2. This RFC only affects the JSON syntax. In general, we will recommend that users use the new schema syntax from RFC24, which will be released as part of Cedar v3.1.0.
3. The breaking change that we really want is RFC55, which makes the current RFC irrelevant. There is still discussion to be had on whether this new breaking change is worth it, but that can be discussed on new RFC.