template groups RFC

[mwhicks1]

RFC for template groups, which fixes cedar-policy/cedar#106

Rendered

[cdisselkoen]

I like the direction and motivation of this RFC; I think it would be great to have a mechanism to group and instantiate templates together, for all the reasons described in the Motivation section. I do have one important concern with the proposed solution.

Currently, no attributes are "special" in Cedar, they are all completely up to the user to interpret. This RFC would change that, and that's a large change in our philosophy, which is not currently addressed in the text of the RFC (IMO it's worth a mention in "Drawbacks".) For this reason, I would lean toward preferring the first option under "Alternatives", namely, adding a dedicated syntax for this feature. The syntax currently suggested under "Alternatives" might bear unfortunate resemblance to policy annotations, while actually being nothing of the kind. So instead I propose this new syntax for discussion:

templategroup "role1" {
    @id("policy1")
    permit(...)

    @id("policy2")
    permit(...)
}

In the future, we could even extend this to support attributes on template groups themselves:

@foo("bar")
templategroup "role1" {
    ...
}

but we don't have to do that, and I certainly am not proposing that allowing attributes on template groups be included in this RFC. Just keeping the door open.

[mwhicks1]

I have mixed feelings about baking in named template groups. It feels incongruous with the explicit non naming of individual policies. That's why I wanted a similar mechanism. But I like the built-in syntax change.

[cdisselkoen]

I agree that it's a little incongruous with the fact that policy names are not explicit in Cedar.

One more tiny question that might be worth addressing in the RFC -- should the new group IDs be part of the same ID-space as policy/template IDs? I.e., is it allowed or not to have a template group with the same ID as an individual template (or policy)?

[cdisselkoen]

If the explicit name is a turn-off, we could still have the syntax

templategroup {
    @id("policy1")
    permit(...)

    @id("policy2")
    permit(...)
}

In this case, to indicate the name of the template group in the CLI, we might actually need to support annotations on template groups.

[mwhicks1]

should the new group IDs be part of the same ID-space as policy/template IDs?

Good question -- they should be part of the same namespace so that there's no ambiguity when you are linking whether you mean to link by Policy ID or by Group ID.

[mwhicks1]

If the explicit name is a turn-off, we could still have the syntax

That's a good idea. What do others think?

[emina]

I vote for making the grouping mechanism general enough to allow template groups to share templates. For example, suppose that you define 3 templates T1, T2, and T3. Then, you want some users to have permissions T1 and T2, and others to have permissions T2 and T3. In this case, we'd like to create two template groups that reuse these templates, so { T1, T2 } and { T2, T3 }. This argues against inlining template definitions into the group definitions, and instead having the groups reference the names of the included templates.

I've kept the description of the above use case abstract, but it comes from a real application.

[shaobo-he-aws]

I vote for making the grouping mechanism general enough to allow template groups to share templates. For example, suppose that you define 3 templates T1, T2, and T3. Then, you want some users to have permissions T1 and T2, and others to have permissions T2 and T3. In this case, we'd like to create two template groups that reuse these templates, so { T1, T2 } and { T2, T3 }. This argues against inlining template definitions into the group definitions, and instead having the groups reference the names of the included templates.

I've kept the description of the above use case abstract, but it comes from a real application.

Would this use case motivate the approach where the users implement this functionality using existing APIs (e.g., filter policy set by template group annotations)?

[emina]

Would this use case motivate the approach where the users implement this functionality using existing APIs (e.g., filter policy set by template group annotations)?

If I’m understanding the suggestion correctly, users would manage template instantiations in this case manually, by searching for policies with specific annotations? That’s certainly possible, but then template groups as a syntactic construct aren’t as useful as they could be. Also, to achieve the desired effect through annotations, the user would need to employ some clumsy workarounds because we can’t bind more than one value to an annotation key. For example, they’d pack the names of all template groups to which a template belongs into a string, something like @templategroup(“G1, G2”) and then parse the string.

[cdisselkoen]

@emina's use case seems to want a syntax such as

@id("policy1")
permit(...)

@id("policy2")
permit(...)

@id("policy3")
permit(...)

templategroup "role1" { "policy1", "policy2" };
templategroup "role2" { "policy2", "policy3" };

Unfortunately this still runs afoul of the concern farther up this thread that suddenly particular attributes (id) would be interpreted specially by Cedar. Makes me think that not having explicit policy names was a mistake. Imagine if we could have

template "policy1":
permit(...)

template "policy2":
permit(...)

template "policy3":
permit(...)

policy "some static policy unrelated to this example":
permit(...)

templategroup "role1" { "policy1", "policy2" };
templategroup "role2" { "policy2", "policy3" };

I know Verified Permissions needs to assign its own UUIDs to each policy, but there's no reason they couldn't identify policies by their own UUIDs even if Cedar had syntax for explicit names.

[emina]

Yes, I agree that it’s undesirable to have attributes with special meaning to Cedar.

Perhaps sharing templates is a rare enough use case that it’s okay not to support it syntactically. Though it’s also unsatisfying for the syntactic solution to not cover this use case too.

I wonder if a cleaner alternative is to not add extra syntax, but provide dedicated APIs that make instantiation of template groups easy? Another feature that could help with this is to extend the annotation mechanism to take not only strings as values but also sets of strings (to allow annotation keys to be bound to multiple values).

[D-McAdams]

The RFC should explain the customer input or end-goal that led to this proposal. What use case is this enabling that can't be achieved today? More specifically, imagine we were writing the user guide for this feature. How would we explain the situations in which to use it? Something along the lines of "You may want to deliver a permissions management experience like X. You could do so by doing Y, but it has drawback Z. These drawbacks can be avoided by using template groups in the following manner..." (In fairness, I recall Mike explained it to me in person, but I've already forgotten the details.)

[mwhicks1]

The RFC should explain the customer input or end-goal that led to this proposal. What use case is this enabling that can't be achieved today?

The motivation for this feature comes form cedar-policy/cedar#81. There, @WanderingStar gave two templates that together describe a single conceptual policy rule (which requires using a feature proposed in RFC #3)

@id("AdminAdmin")
permit(
  principal == ?principal,
  action, 
  resource in ?resource);     // take any action in the scope that you're admin on;

@id("AdminNavigate")
permit(
  principal == ?principal,
  action == Action::"Navigate", 
  ?resource in resource); // allow access up the tree for navigation

They said that they'd prefer a single template because with two "it would be possible to get into strange states where someone had AdminAdmin applied, but not AdminNavigate". The single template is more complex:

@id("Admin")
permit(
  principal == ?principal,
  action, 
  resource)
when {
    // take any action in the scope that you're admin on
    resource in ?resource

    // allow access up the tree for navigation
    || (action == Action::"Navigate" && ?resource in resource)
};

The proposal of this RFC aims for the best of both worlds: easier-to-read templates, but grouped together to ensure they are always linked together, and can be analyzed together.

[mwhicks1]

Since the motivating use-case is relatively simple and generalizing it leads to questions with somewhat unsatisfying answers, I propose to simplify the RFC to

1. make no changes to Cedar or its APIs
2. add code to the CLI to implement the simple grouping mechanism. Customers who are happy with that can lift the code into their own apps. Or they can generalize that code to support more complex grouping mechanisms.

Unless others have other ideas that do require Cedar language or API changes, I’ll cancel this RFC and post an issue focusing on the CLI.

[cdisselkoen]

We could have (might want?) Cedar API support for template groups even if we don't provide Cedar syntax indicating which policies are grouped together. For instance (and note I haven't thought about this proposal very hard yet)

impl PolicySet {
    pub fn mark_as_template_group(&self, group_id: PolicyId, ids: impl IntoIterator<Item = PolicyId>) -> Result<(), some_error_type>;
}

and then Cedar could at least allow instantiating the template group at once, using the specified group_id (see link_group below); and maybe even, prohibit instantiating the individual policies that were incorporated into the template group.

impl PolicySet {
    pub fn link_group(&mut self, group_id: PolicyId, new_id: PolicyId, vals: HashMap<SlotId, EntityUid>) -> Result<(), PolicySetError>;
}

[mwhicks1]

We could have (might want?) Cedar API support for template groups even if we don't provide Cedar syntax

Indeed. While this might be useful, I think a library-level extension to PolicySet that implements would work, and would just layer on top of the functionality already there. People who want groups can just lift that code from the CLI into their own code, no?

I worry about fixing our notion of a group to PolicyID, and having to fix that particular policies have group IDs, in the API. Doing it all in the CLI means the whole concept of group, and that you can be member of just one (say), is not baked into Cedar.

[cdisselkoen]

Good point. This addresses my concern.

We could also do the implementation as its own crate in the cedar repo, and have the CLI depend on that. This might be more reusable than pointing people to copy code from the CLI repo. But that would be more work, and at any rate we can discuss that on a PR and not on this RFC.

[mwhicks1]

I'm canceling this RFC and reopening the original issue but applied to just to the CLI rather than the Cedar language and APIs (and hence it no longer needs to be an RFC).