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

Refactor the RBAC documentation for the plugin #811

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

lubosmj
Copy link
Member

@lubosmj lubosmj commented May 31, 2022

closes #641

@lubosmj lubosmj marked this pull request as ready for review May 31, 2022 13:57
@lubosmj lubosmj force-pushed the rbac-up-docs-641 branch from ce4a634 to 4d015ed Compare May 31, 2022 14:01
Role-based Access Control
=========================

Role-based access control (RBAC) **restricts** access to content based on a user's role within an
Copy link
Member

Choose a reason for hiding this comment

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

Are you talking specifically about "content" here (i think not). Maybe use entities instead. In the context of Pulp, "content" already has quite a distinct meaning.

=========================

Role-based access control (RBAC) **restricts** access to content based on a user's role within an
organization. The role consists of one or more permissions. Users having a proper set of roles can
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
organization. The role consists of one or more permissions. Users having a proper set of roles can
organization. A role consists of one or more permissions. Users having a proper set of roles can

organization. The role consists of one or more permissions. Users having a proper set of roles can
view, modify, or delete content hosted on different endpoints.

By default, all container repositories are accessible by anyone, unless the opposite is *explicitly*
Copy link
Member

Choose a reason for hiding this comment

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

I think you are referring to registry pull access here. Maybe something like ... repositories content is accessible via podman pull by anyone...

Comment on lines 6 to 7
A role is defined by one or more permissions. In this section, there are discussed details of
permissions used within the container plugin.
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
A role is defined by one or more permissions. In this section, there are discussed details of
permissions used within the container plugin.
A role is defined by one or more permissions. In this section, details of
permissions used within the container plugin are discussed.

Default Roles
-------------

For each endpoint, there is defined a different set of roles. The roles can be assigned at the model
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
For each endpoint, there is defined a different set of roles. The roles can be assigned at the model
For each endpoint, a different set of roles is defined. The roles can be assigned at the model

Comment on lines 146 to 147
The *Consumer* role contains only the ``view`` and ``pull`` permissions. Below is showcased a list
of associated permissions for distributions:
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
The *Consumer* role contains only the ``view`` and ``pull`` permissions. Below is showcased a list
of associated permissions for distributions:
The *Consumer* role contains only the ``view`` and ``pull`` permissions. Below, a list
of associated permissions for distributions is showcased:

}

Having the ``view`` and ``pull`` permissions allows a user to see and pull content from the Pulp
Registry. Assigning this role only at the object level allows administrators to select what the
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
Registry. Assigning this role only at the object level allows administrators to select what the
Registry. Assigning this role only at the object level allows administrators and owners to select what the

~~~~~~~~~~~~~~~~~

The *Collaborator* role represents a set of permissions that a co-worker working within a same user-space
should have. In comparison to the *Consumer* role, users with the *Collaborator* role are allowed
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
should have. In comparison to the *Consumer* role, users with the *Collaborator* role are allowed
should have. In addition to the *Consumer* role, users with the *Collaborator* role are allowed

Comment on lines +251 to +266
.. code-block:: bash

pulp role create --name "container.containerrepository_syncer" \
--permission "container.view_containerrepository" \
--permission "container.view_containerremote" \
--permission "container.change_containerrepository" \
--permission "container.modify_content_containerrepository" \
--permission "container.sync_containerrepository"

pulp user role-assignment add --username "alice" --role "container.containerrepository_syncer" object ""
Copy link
Member

Choose a reason for hiding this comment

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

Should this be an example about changing the AP?

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course. I provide an example of how to create, assign, and hook a role.

For the sake of completeness, I will add an example of assigning default permissions to a user in one of the descriptions of the default roles.

Copy link
Member

@mdellweg mdellweg Jun 17, 2022

Choose a reason for hiding this comment

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

I would still consider these two commands as an example to create and use a "custom role". "changing the access policy" to me is the next level of customization. My suggestion: Make this example its own thing, and move the headline for AP below it. Also maybe add an example for changing the statements in the AP.

Copy link
Member Author

Choose a reason for hiding this comment

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

I divided the section into two parts: Customizing Roles and Customizing Access Policies. It should be now better readable.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good!

docs/tech-preview.rst Show resolved Hide resolved
@lubosmj lubosmj force-pushed the rbac-up-docs-641 branch 2 times, most recently from 38d9d9e to f3a0e24 Compare June 7, 2022 12:57
@ipanova ipanova requested review from ipanova and mdellweg June 16, 2022 15:25
@lubosmj lubosmj force-pushed the rbac-up-docs-641 branch from f3a0e24 to 82fe503 Compare June 21, 2022 19:31
Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

One nit-pick if you agree.

Customizing Roles
-----------------

In Pulp, users are allowed to create or update their roles. To create a role with permissions
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is admins...

@lubosmj lubosmj force-pushed the rbac-up-docs-641 branch from 82fe503 to d358452 Compare June 22, 2022 11:30

Role-based access control (RBAC) **restricts** access to entities based on a user's role within an
organization. A role consists of one or more permissions. Users having a proper set of roles can
view, modify, or delete content hosted on different endpoints.
Copy link
Member

Choose a reason for hiding this comment

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

s/content/resources. There is rbac not just for content.

Role-based access control (RBAC) **restricts** access to entities based on a user's role within an
organization. A role consists of one or more permissions. Users having a proper set of roles can
view, modify, or delete content hosted on different endpoints.

Copy link
Member

@ipanova ipanova Jun 23, 2022

Choose a reason for hiding this comment

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

maybe worth adding a section called 'Content and Repository access'

Copy link
Member

Choose a reason for hiding this comment

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

can you also add information about the queryset scoping?

Copy link
Member Author

@lubosmj lubosmj Jun 24, 2022

Choose a reason for hiding this comment

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

Not sure how the information about the queryset scoping will be useful to users. It tells plugin-writers that Pulp restricts access to objects based on users' permissions (which is generally known and expected): https://github.com/pulp/pulpcore/blob/8ab3fdfba539b81fa25b67876dde7327992a6ce4/docs/plugins/plugin-writer/concepts/rbac/queryset_scoping.rst

=====================

As of release 2.11.0, the plugin started to support roles instead of separate groups and
permissions. Permission classes provided by Pulp are **automatically** migrated when upgrading
Copy link
Member

Choose a reason for hiding this comment

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

Default permissions classes

done
Similarly, execute an adjusted script for other repository objects that were asserted under
the permissions' scope.
Copy link
Member

Choose a reason for hiding this comment

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

Can you mention the dump-permission management command Matthias has added?
Also please add a note that with pulp-container 2.13 this would be the only way to process unmigrated permissions because of removed django-guardian form the stack.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a note about the version and the dump-permission command.

@ipanova ipanova self-requested a review June 23, 2022 17:03
@lubosmj lubosmj force-pushed the rbac-up-docs-641 branch from d358452 to 781a1a0 Compare June 24, 2022 09:07
@ipanova ipanova merged commit ab9d72f into pulp:main Jun 24, 2022
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.

Add docs that facilitate manual steps from permissions to roles
4 participants