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

feat: introduce hierarchy for can_relations #419

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

BarcoMasile
Copy link
Contributor

Description

This PR introduces a hierarchy for all can_* relations in the OpenFGA authorization model.
The hierarchy is as follows

  • if a user can delete an object (top permission for can_* permission) then the user can also edit it
  • if a user can edit an object, then the user can also view it
  • if a user can view an object, the user doesn't inherit additional permissions from this relation only

So: can_delete >> can_edit >> can_view
Can create is not touched by this since it gets special treatment in the implementation.

Why

This is done with the intention of simplifying both user experience and developer life when it comes to reasoning about user authorization based on entitlements.
This way we can rely on some simple implications to treat entitlements. Also, this allows us to not crowd the OpenFGA store with "same user" tuples.

nsklikas
nsklikas previously approved these changes Sep 19, 2024
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

No problem with this, but I have a question:
What is the upgrade path for this? This is a question for the charm, but I am curious to know

@BarcoMasile
Copy link
Contributor Author

No problem with this, but I have a question: What is the upgrade path for this? This is a question for the charm, but I am curious to know

@nsklikas it should suffice to just apply the new authorization model. It should not cause conflicts (OpenFGA handles multiple authorization model weird for what I remember).
We could technically dump the database tuples content, and re-apply the content after the upgrade, since the previous tuples would be 100% valid even after the change.
But this should be investigated, thanks for raising this!

@shipperizer
Copy link
Contributor

No problem with this, but I have a question: What is the upgrade path for this? This is a question for the charm, but I am curious to know

afaik the upgrade of the model brings the tuples with it, we would have to trigger the right relationship refresh to communicate the app(s) that the model id has changed

in our specific case, this would trigger a clash as we do the model validation at runtime, so it's not something that can be orchestrated without scheduled downtime

question is: do we validate the model consistency or we rely on openfga blindly?

it goes
`can_delete` >> `can_edit` >> `can_view`
can create is not touched by this since it gets special treatment
@shipperizer
Copy link
Contributor

shipperizer commented Sep 20, 2024

side note, when we create a resource what tuples do we assign ?is it can view and can edit?if yes we can drop the first

https://github.com/canonical/identity-platform-admin-ui/blob/main/internal%2Fauthorization%2Fresource_manager.go#L23

@BarcoMasile
Copy link
Contributor Author

side note, when we create a resource what tuples do we assign ?is it can view and can edit?if yes we can drop the first

https://github.com/canonical/identity-platform-admin-ui/blob/main/internal%2Fauthorization%2Fresource_manager.go#L23

@shipperizer for groups and roles we only add can_view for the newly created object

@shipperizer
Copy link
Contributor

side note, when we create a resource what tuples do we assign ?is it can view and can edit?if yes we can drop the first

https://github.com/canonical/identity-platform-admin-ui/blob/main/internal%2Fauthorization%2Fresource_manager.go#L23

@shipperizer for groups and roles we only add can_view for the newly created object

was more concerned about the other non OpenFGA backed resources, but, for roles and groups are we ok with can view only?

@BarcoMasile BarcoMasile merged commit 09682b6 into main Sep 20, 2024
10 checks passed
@BarcoMasile BarcoMasile deleted the feat/ofga-authmodel-hierarchy branch September 20, 2024 10:49
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.

3 participants