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

ACL WIP #66

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

ACL WIP #66

wants to merge 2 commits into from

Conversation

Lory1990
Copy link

Using ACLs based data model with ROND

Please do not merge, I haven't tested yet. Check the wording, the concepts and the data models


has_read_permission_for_given_company {
companyId := input.request.query["companyId"][0]
user := input.user[_].resource == { resourceType: "company", resourceId: companyId }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredmaggiowski really not sure about that

@fredmaggiowski fredmaggiowski marked this pull request as draft February 21, 2023 11:13
@fredmaggiowski
Copy link
Member

Thank you @Lory1990 I'm reviewing it ASAP, in the meantime I've converted the PR to draft to be sure it won't get merged until it's ready

Copy link
Member

@fredmaggiowski fredmaggiowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also add some info about ACL support in this section

https://rond-authz.io/docs/getting-started#r%C3%B6nd-and-rbac

Maybe with a simple comparison between RBAC and ACL approaches, some resources and links to the documentation page you are preparing for ACL, what do you think about it?


## RBAC Based ACL Data Model

You can use a the [binding association collection](#bindings) to rapresent a ACL data model, in this case you could only use the following fields:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You can use a the [binding association collection](#bindings) to rapresent a ACL data model, in this case you could only use the following fields:
You can use a the [binding association collection](#bindings) to represent a ACL data model, in this case you could only use the following fields:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated code

_data/toc.yml Outdated
@@ -38,5 +38,9 @@
url: "docs/policy-integration#custom-built-ins"
- title: RBAC Data Model
url: "docs/policy-integration#rbac-data-model"
- title: RBAC Based ACL Data Model
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not present it as RBAC Based ACL 🤔 it's either RBAC or ACL.. It could be titled as "Implement ACL with Bindings"

Also what is the difference you expect between RBAC Based ACL Data Model and ACL Data Model 🤔

I'd collapse them in a single title.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG they are the same model 😱 so sorry, I am only a human with Cognitive bias

@@ -436,6 +436,10 @@ This collection contains all the bindings between users or groups of users and a
"roles": [
"TLRoleId"
],
"permissions": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep this example free of permissions to avoid confusion, do you find it more clear with it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me is essential to declare all the capabilities of RBAC so that there is no misunderstanding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants