RBAC Docs #2486

pavelpicka · 2022-04-11T17:47:59Z

RBAC documentation.

docs/rbac.rst

dralley · 2022-04-14T04:52:14Z

docs/rbac.rst

+
+.. important::
+
+    Using user ``admin`` will bypass all RBAC checks.


Does it actually bypass the checks or does the admin simply possess every permission? I thought it was the latter https://github.com/pulp/pulp_rpm/pull/2418/files#diff-37aef788d3a12e65873c374ff05bc8629a8b636cab51ac31c127f94d43f1314eR234

The end result is the same but we should properly communicate what's going on :)

rephrased to "admin gets all permissions"

The admin is assumed to have all the permissions (even before asking any permission backend). But the access policy can still disallow any access. So there is actually a difference in the end result.

dralley · 2022-04-14T04:53:39Z

docs/rbac.rst

+    Using user ``admin`` will bypass all RBAC checks.
+
+
+* `Permission` : Permission to do **one** action on one `viewset` as a `view` or a `create`.


Should "as a" be ", for example" or "e.g."? I'm not sure what the intention is here

This section should explain basic terms.

But I would remove it.

dralley · 2022-04-14T04:55:53Z

docs/rbac.rst

+
+* `Permission` : Permission to do **one** action on one `viewset` as a `view` or a `create`.
+
+  * `Model Permission` : a permission for a model type, it means you can do this action on any object of the model


Are these docs meant to be for the user or for the plugin writer? If they're for a user, I would avoid using terms like "model" and "viewset" and use more generic terms like object or API endpoint.

docs/rbac.rst

dralley · 2022-04-14T04:58:23Z

docs/rbac.rst

+Viewer Role
+^^^^^^^^^^^
+
+This role contains only ``view`` permission for a `viewset`. Using this permission you can control who can


As with above, most users won't know what a viewset is. Repeated a few times below.

docs/rbac.rst

mdellweg · 2022-04-24T06:23:00Z

docs/rbac.rst

+   If a role contains multiple permissions over different objects, then you can't assign this role to a specific object.
+   Trying to assign the role to a single object will fail.
+   For example, if a role contains ``add`` permission, you must use the empty ``--object`` param.


This is not true.
Having the add permission on an object does not lead anywhere, but it's possible.
Also assigning a role to an object is possible if at least one of it's permissions is applicable to that object.

mdellweg · 2022-04-24T06:24:08Z

docs/rbac.rst

+.. code-block:: shell
+   :caption: Change the access policy of the retrieve action without requiring any permission.
+
+    pulp access-policy update --viewset-name "remotes/rpm/rpm" --statements '[{"action": "retrieve", "effect": "allow"}]'


This is dangerous, as it deletes all but the retrieve action from this viewset.

Fixed and reflect this information to the doc as well.

dralley · 2022-04-25T14:01:15Z

docs/rbac.rst

+Viewer Role
+^^^^^^^^^^^
+
+This role contains only ``view`` permission for an `endpoint`. Using this permission you can control who can


Not sure if endpoint needs to be a keyword

dralley · 2022-04-25T14:02:27Z

docs/rbac.rst

+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+This role is like a pulp admin but only for the RPM plugin.
+You can perform every RPM workflow. If you need a more


This role is like a pulp admin but only for the RPM plugin - it can perform any RPM workflow with no restrictions.

docs/rbac.rst

dralley · 2022-04-25T14:05:27Z

docs/rbac.rst

+To find out which permissions you need for a new role, you can list an endpoints policy with the following command:
+
+.. code-block:: shell
+   :caption: List an access policy of the repository endpoint


If this lists many role names then it should be phrased as "List possible access policies for the repository endpoint"

Maybe you should include example output?

dralley · 2022-04-25T14:06:48Z

docs/rbac.rst

+   :caption: Change the access policy of all actions not requiring any permission.
+
+    pulp access-policy update \
+        --viewset-name "remotes/rpm/rpm" \


Is viewset-name already baked into the CLI or is this something new? It would be better to use generic terms here as well (even though it's not part of this PR)

viewset-name is already in the CLI and already used in a few more plugins so changing it wouldn't be so easy.

That is unfortunate. @mdellweg Do you think this would be a worthwhile change, or do you think the cost would outweigh the benefit? If we have to carry around the old commands indefinitely it's probably not worth it.

The thing that i was told cannot change is the parameter in the REST api. The cli is just following the bad naming for consistency and less confusion and i'd rather keep it that way.

dralley · 2022-04-25T14:07:46Z

docs/rbac.rst

+.. code-block:: shell
+   :caption: Update creation hook of a repository to creator gets another role than `repository_owner`
+
+    pulp access-policy update --href "repositories/rpm/rpm" \


Is --href "repositories/rpm/rpm" correct? What is the difference between --href and --viewset-name then?

--viewset-name is a better option in this place. --href is intended to use with the already created object.

Yes href would not work...

dralley

Good work @pavelpicka

Could you please squash these commits before the PR gets merged?

docs/index.rst

ipanova · 2022-04-29T13:09:53Z

docs/rbac.rst

+
+    User ``admin`` automatically gets all RBAC permissions.
+
+RPM plugin presents Role Based Access for these endpoints :


what about copy, compsxml and rest of the content?

Added note about the content and rpm copy.

ipanova · 2022-04-29T13:14:27Z

@pavelpicka please whether reference this issue you are going ti create #2418 (review) or file a separate doc issue

RBAC documentation. closes pulp#2506 pulp#2506

pavelpicka force-pushed the rpm_rbac_docs branch from 5b6685c to 3bca77b Compare April 11, 2022 18:15

dralley reviewed Apr 14, 2022

View reviewed changes