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

Auth: Generate entitlement definitions #13256

Merged
merged 14 commits into from
Apr 4, 2024

Conversation

markylaing
Copy link
Contributor

@markylaing markylaing commented Apr 3, 2024

This PR adds a small go program to read the OpenFGA model file and generate a go file containing entitlement definitions and a mapping of entity type to entitlement slice. This is used to validate entitlements when add a permission to a group, and when using the permissions API.

This can be taken further to generate documentation from the comments that are in the OpenFGA model file.

Generating entitlements directly from the model revealed a few discrepancies between the model and the entitlement to entity type map that was previously defined. These would have led to some quite obfuscated bugs. I've fixed them in this PR and generating entitlements from the model should prevent this happening in the future.

Partially addresses #12999
Prepares for #12928

@markylaing markylaing self-assigned this Apr 3, 2024
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing markylaing marked this pull request as ready for review April 4, 2024 08:22
@markylaing
Copy link
Contributor Author

@tomponline ready for review when you have time. Thanks

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

LGTM - just a couple of nits.

@tomponline
Copy link
Member

@markylaing should this be linked to any outstanding issue such that its closed when merged?

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
…tor`.

EntitlementProjectOperator was the string "operator". This was to
differentiate it from the server level operator but there is no benefit
to this now that we are generating the entitlements.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Granting a `can_delete` permission on an identity to a group
was possible via the API but this would have had no effect because
the relation was not present in the model.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Granting `can_view`, `can_edit`, or `can_delete` on a group was
possible via the API but this would have had no effect because
members of a group (group#member) could not be directly related to
the group type in the model via these relations.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Re-generate the entitlements map to reflect changes.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
…tificate.

This restriction was artificial and also incorrect. We should allow granting
permissions against certificates because this is used by the certificates API.
Since identities and certificates are different entity types, a permission
granted on an identity is not identical to a permission granted on a certificate,
so we need to allow this.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
This adds entitlements for identity provider groups that we previously
omitted incorrectly. Additionally, the `can_view` entitlement on server
cannot be granted because this is the type-bound public access that
allows all authenticated clients to call GET /1.0.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
Copy link
Contributor Author

All comments addressed I think. I've linked a couple of relevant issues but there is no specific issue for this.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

This is great thanks!

@tomponline tomponline merged commit ebd6897 into canonical:main Apr 4, 2024
29 checks passed
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