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

Allow disabling object-level permissions for a given model in the RBAC registry #424

Open
AlanCoding opened this issue May 31, 2024 · 4 comments
Labels
app:rbac enhancement New feature or request

Comments

@AlanCoding
Copy link
Member

AlanCoding commented May 31, 2024

This is a "model registration flexibility" issue similar to #365

The ask here is to allow disabling of using object-level roles for a particular model. Spitball of the contract for this:

permission_registry.register(Instance, allow_object_roles=False)

We would have allow_object_roles default to True, as this is how it is always used today. With this change, the /api/v1/role_metadata/ endpoint would not show the content type for the Instance model (see later notes). However, importantly, the permissions related to the Instance model would still be shown as possible for system-wide roles. This is a valid use-case for this feature, because you may want to allow someone to manage instances in AWX without making them a superuser, but you still don't want to build out a UI for giving permissions to single instances, because you just don't want that.

The AWX Instance model does not have a related organization, but you could repeat for an organization-scoped model.

Our use case for this second version (org-scoped) mode would be the OAuth2 Application model. In AWX organization admins got permissions to this model. Implementation here would be that the model's permissions are listed in the organization admin roles. Optionally, someone could create custom organization-level roles that include OAuth2 Application permissions. But a user would not be able to create custom roles that give permission to individual applications, simply because we do not want to build a UI for this.

@AlanCoding AlanCoding added enhancement New feature or request app:rbac labels May 31, 2024
relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 4, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 5, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 6, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
@AlanCoding
Copy link
Member Author

@relrod my only hangup for doing a fast 30-minute version of this is where this data could live. Right now the permission_registry keeps some simple dictionaries and sets that were introduced for specific problems.

  • We could introduce another... set on permission_registry that lists model names that don't allow object roles? That would get the job done.
  • Another option to ninja this in would be that we could take the model class itself and do something like setattr(cls, '_allow_object_roles', True), and then use that flag on the objects themselves in the serializer.

Thinking it over, I'm okay with either approach.

relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 6, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 7, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 7, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
relrod added a commit to relrod/django-ansible-base that referenced this issue Jun 7, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

Signed-off-by: Rick Elrod <[email protected]>
relrod added a commit that referenced this issue Jun 7, 2024
Doing this properly with RBAC depends on #424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

This also fixes a problem in the activity stream tests where INSTALLED_APPS
got permanently modified which made other tests unpredictable.

It also fixes a bug where token scope choices were previously limited to
only one option.

Signed-off-by: Rick Elrod <[email protected]>
Co-authored-by: John Westcott IV <[email protected]>
thedoubl3j pushed a commit to thedoubl3j/django-ansible-base that referenced this issue Jun 18, 2024
Doing this properly with RBAC depends on ansible#424.

For now, we limit Application views to superusers. We limit Token
views using a custom DRF permission class based on what was specified
in AWX's access.py

This also fixes a problem in the activity stream tests where INSTALLED_APPS
got permanently modified which made other tests unpredictable.

It also fixes a bug where token scope choices were previously limited to
only one option.

Signed-off-by: Rick Elrod <[email protected]>
Co-authored-by: John Westcott IV <[email protected]>
@AlanCoding
Copy link
Member Author

This has a problem with the need for "creator" roles after someone creates a new object and does not have a role which would give implicit access to that object. Options for ways around that are:

  1. For this type of model, require that the "add" permission is coupled with all other permissions. You can't have a role with "add" permission by itself.
  2. Prohibit the "add" permission for models that have no object-level roles and use "change", "delete", or "view" permissions as proxy. This is complex and confusing.
  3. Still allow system object-level roles but prevent custom object-level roles. But how would we communicate this to clients?

None of these options are slam dunks. If I were to choose one, I would lean towards (1). However that still leaves unanswered questions.

Also, a half-way option that looks like option (3) will be possible with #490. Drawback is that it blocks the action late in the user experience, as opposed to giving advance notice to clients. On the other hand, it can offer a detailed rationale which needs to be filled in by the specific model permission policies.

Those together make this a pretty solid won't-fix.

@AlanCoding AlanCoding closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2024
@AlanCoding
Copy link
Member Author

This is now coming back up for galaxy_ng, and we will be getting a model list for this for those models.

@AlanCoding AlanCoding reopened this Aug 19, 2024
@AlanCoding
Copy link
Member Author

Link prior work for this #475

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app:rbac enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant