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

Userset Rewrites with Intersections #466

Closed
jon-whit opened this issue Mar 5, 2021 · 10 comments
Closed

Userset Rewrites with Intersections #466

jon-whit opened this issue Mar 5, 2021 · 10 comments
Labels
feat New feature or request. rfc A request for comments to discuss and share ideas.

Comments

@jon-whit
Copy link

jon-whit commented Mar 5, 2021

The Zanzibar paper mentions that the userset expression language specification supports sub-expressions such as union, intersection, and exclusion.

One use case that I immediately have is to be able to express relation tuples over disjoint sets. In my model I have permission boundaries (analogous to AWS IAM Permission Boundaries) and shared/inherited permissions. Permission boundaries specify the maximum level of permissions that a user can have, but it does not specify what permissions the user has. Shared/inherited permissions define the permissions granted to the user either via ABAC policies or explicitly shared rights, but do not fully define the effective permissions the user has.

An access evaluation decision looks like this:

boundaries = {"projects.list", "projects.create"}
permissions = {"projects.list"}

 // The effective permissions is the set intersection of the two.
effectivePermissions = boundaries & permissions

effectivePermissions --> {"projects.list"}

I'd like to collaborate with you guys on coming up with the namespace configuration expression language that will allow us to express any arbitrary combo of sub-expressions involving unions, intersections, and exclusions (negations).

I've started with the work from mishudark/zanzibar. He's done a pretty good job at laying a foundation down for userset rewrites with unions, but it lacks support for intersection and exclusion. I also don't think that the yaml modeling that he's defined will expand very well once we allow abitrarily nested sub-expressions, so it's probably worth revisiting and going back to the drawing board.

@zepatrik
Copy link
Member

zepatrik commented Mar 5, 2021

What about a config similar to this:

IMG_20210305_174831

This is just interesting from a content perspective, not syntax.

@jon-whit
Copy link
Author

jon-whit commented Mar 5, 2021

It's less around the configuration syntax and more around the evaluation of such rules.

I've been looking at Google's Common Expression Language and their go implementation google/cel-go. The question is:

How do we both define the syntax for rewrites and also evaluate arbitrary expressions composing those rewrites for Check and Expand evaluations.

My thinking around using CEL is that our userset rewrite configuration could just translate into a CEL expression, each operand of the expression can be evaluated/obtained using the Graph traversal and relationtuple lookups, and then the operands can be fed into the CEL evaluation engine for an outcome.

@jon-whit
Copy link
Author

jon-whit commented Mar 5, 2021

From CEL's documentation:

Ideal applications are ones in which the configuration is executed often and modified relatively infrequently.

This fits in with userset rewrite configurations as well. The expressions that result from the userset rewrites shouldn't change that frequently, but every Check or Expand RPC would have to evaluate such expressions. So CEL would potentially be a good candidate for such a use case.

@zepatrik
Copy link
Member

zepatrik commented Mar 5, 2021

CEL does look really promising, it might be useful elsewhere as well. Did you look at #468 already? I think it should be possible to implement it like that. Basically I modify the graph traversal algorithm to return on different conditions based on the rewrite rules.
But I will definitely dig deeper into CEL.

@jon-whit
Copy link
Author

jon-whit commented Mar 5, 2021

@zepatrik oh sorry I haven't take a look yet. Just throwing out ideas (sorry for the spam, haha).

@zepatrik
Copy link
Member

zepatrik commented Mar 5, 2021

No, CEL is definitely an awesome pointer, it seems even suitable as a config language from a first glance.

@aeneasr
Copy link
Member

aeneasr commented Mar 8, 2021

CEL looks really interesting!

@jon-whit
Copy link
Author

jon-whit commented Mar 10, 2021

@zepatrik I wanted to describe my use case a little better... The example above is non ideal for a few reasons that I outline below.

We have a data model where each user has an access level, and each access level has one or more permissions that are included in it.

type User struct {
    ID string
    AccessLevelID string
}

type AccessLevel struct {
    ID string
}

type AccessLevelPermissions struct {
    AccessLevelID string // joined on AccessLevel.ID
    IncludedPermissions []string
}

What I'd like to be able to do is write a userset rewrite that says "If the user has an access level that includes a certain (required) permission AND has any one of any number of shared permissions, then they are granted access". Additionally, when an AccessLevel changes such that the permissions included in it change, that should minimally impact the number of relation tuples stored. If we denormalize the AccessLevelPermissions, then any time the AccessLevel changes every single relation tuple referencing it would have to change, and that would really suck.

Do you have any thoughts?

@zepatrik zepatrik added the corp/m6 Up for M6 at Ory Corp. label Mar 29, 2021
@github-actions
Copy link

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan on resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneous you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label Mar 30, 2022
@zepatrik zepatrik added feat New feature or request. rfc A request for comments to discuss and share ideas. and removed stale Feedback from one or more authors is required to proceed. corp/m6 Up for M6 at Ory Corp. labels Mar 30, 2022
@zepatrik
Copy link
Member

Actually implemented with #984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request. rfc A request for comments to discuss and share ideas.
Projects
None yet
Development

No branches or pull requests

3 participants