# ~~Roles~~ RBAC Task Force Meeting (RTFM)

## Action Items

- ~~@gerrod to revise <https://github.com/pulp/pulpcore/pull/1676/>~~
- ~~@gerrod adjust for the api change in pulp_cli and find reviewers~~
- ~~@all review/merge <https://github.com/pulp/pulpcore/pull/1766>~~
- @all clean up this document
- @all look at CodeArtifact

## Oct 4 Update
- Added tests to pulpcore & pulp_file
- Needed to customize openapi-generator: https://github.com/pulp/pulp-openapi-generator/pull/80
- Needed to modify gen_object_with_cleanup: https://github.com/pulp/pulp-smash/pull/1306
- Need to rework some tests that use hard-coded paths
- Wrote down current development summary of domains:
- https://hackmd.io/n-OGwj3xQYapMsipzkgqbA?view
- Look at https://rsinger86.github.io/drf-access-policy/policy_reuse/
- A new feature added in DRF-access-policy that could be interesting for us.

## Isolation on the domain level

Pulp would gain the conept of a domain (similar to a namespace).
All Pulp models except User Group and Role would gain a foreign key to a domain, and add it to all their uniqueness constraints.
Roles can now be assigned on the global, domain and object level.
Roles like a domain admin can also be assigned to the domain as an object.
Storage would be configured on the domain level.
Two different domains can now store the same artifact twice, each in their own storage bucket.
The foreign key to Domain will possibly be `None` to make this an opt in feature.

Idea for using django-storages on a domain level:
* Have the domain object build the storage object based on its saved configuration
* Have `ArtifactFileField` storage point to its domain's storage
* Replace all usage of `default_storage` with new domain storage method
* e.g. https://gist.github.com/gerrod3/37466aa0e775b81cf2ad24cba03c3540

Models-domain relations: https://hackmd.io/X8IWzy95SCWnuTV4RPK9JA
Going to have domain name in the URL
Discourse post: https://discourse.pulpproject.org/t/new-multi-tenancy-feature-domains/635

## Use Cases for Content Isolation

1. As a user I see can see only orphaned content and that content I have repo read access to. Same stands for rest of the users.
* /pulp/api/v3/content/rpm/rpm/
* /pulp/api/v3/content/rpm/rpm/<content_id>/
* /pulp/api/v3/content/rpm/rpm/?repository_version=repo_version_href
* Container example https://github.com/pulp/pulp_container/blob/main/pulp_container/app/viewsets.py#L189
2. As a user I am sure my content won't be stolen during upload
* orphaned content is visible to every authed user
* take into account simultaneous uploads of the same content and same payload. Pulp might need to ask for 'proof' of incoming bits instead of deduplicating - ask for sha256 in the upload request or always re-upload bits regardless whether there is already such content record in the DB.
* take into account simultaneous uploads of the same content but different payload. Not every every plugin manages such situation - pulp_rpm has PkgId among uniqueness https://github.com/pulp/pulp_rpm/blob/main/pulp_rpm/app/models/package.py#L263. Also repo_keys_fields are adjusted https://github.com/pulp/pulp_rpm/blob/main/pulp_rpm/app/models/package.py#L236
3. As a user, I can add a Content unit to Pulp with the same natural key that is already in Pulp that has *the same* artifacts as the existing content unit
4. As a user, I can add a Content unit to Pulp with the same natural key that is already in Pulp that has *a different* artifacts as the existing content unit
* This should not be possible as such, but instead, some unique digest should be part of the natural key.
5. Requirements I (gerrod) came up with originaly https://discourse.pulpproject.org/t/multi-tennancy-and-the-question-of-owning-content/237/2

### Recap of Usecases
1. Artifacts will be listed/retrieved through the Repository Content view permissions
* artifacts-->content-->repos
2. [DONE] Content will be listed/retrieved through the Repository view permisions
* content-->repos
#### Summary:
a) pulpcore add queryset_scoping attribute to https://github.com/pulp/pulpcore/blob/main/pulpcore/app/models/access_policy.py#L40 which will be using whether the default content queryset scoping or disable it completely
b) plugins will add content queryset scoping with hardcoded 'view repo permission'
c) if users will lack more queryset scoping customization we will hear from them and implement alternative methods to choose from
3. [DONE] Orphaned content is not visible by default? (Yes [mdellweg], maybe add a content_admin role)
* unless you're admin you don't see orphan content
* https://github.com/pulp/pulpcore/blob/main/pulpcore/app/viewsets/content.py#L117
4. [DONE] Plugins should implement one-shot-upload so orphaned content is not produced but added to a destination repo in one go
* Audit pulpcore upload serializers and make sure that repository field is required
* Add is_admin check, otherwise make repository field required
* Add new global access policy helper method for one-shot upload serializer that has the is_admin or require repository field check
* Require plugin writers to use this method on the upload action of their content access policy
5. [DONE] One-shot-upload needs to be extended to "use" chunked Uploads.
- https://github.com/pulp/pulpcore/issues/2786
6. [DONE] Chunked Uploads need RBAC Protection
- https://github.com/pulp/pulpcore/issues/2362
7. [IN PROGRESS] On Content upload, payload is expected to always be sent, even if there is already a Content record in the DB with same artifacts - this is a way to circumvemt content stealing problem. Payload will be saved, and digest will be calculated
* unless user can see the content( which is present in the DB) through repo view permissions. In that case it is not necessary to calculate digest of the payload/save it.
8. [IN PROGRESS] Plugins should add digest to the Content natural key, to enable usecase when there can co-exist two same content units but with different artifacts. Repo_key_fields should be extented too.

## In the queue

* https://github.com/pulp/pulpcore/blob/55887c49799c14ebe0d8da0838693d13e99b583a/pulpcore/app/global_access_conditions.py#L150
* we should check if 'remote' parameter is present in `request` (as create empty repository without remote perm is forbidden even when no remote used)
* Are you sure about that? The `validated_data.get("remote")` should return `None` if no remote is present in the request.
* https://github.com/pulp/pulp_container/blob/main/pulp_container/app/viewsets.py#L900
* double `destroy`?
* Probably related to this issue: https://github.com/pulp/pulpcore/issues/1964
* Can be fixed with `condition_expression`

## Documentation Strategy (from bmbouter)

Goal: have short enough RBAC docs for users that they can understand what is done
Goal: while having docs that have really clear examples for each type of RBAC case we've implemented

Idea: Have the docs be short. The goal of these docs is to explain what Pulp does in simple use case language. Then for each use case, link to the test as the concrete example of the various assertions Pulp complies with. This avoids us having to basically "write the concrete examples in-detail in the docs".

## Discussion complex queryset filtering conditions

current (PR) state: `"filtering_permissions": ["core.view_task"]`

"filtering_permissions": ["core.view_task"]


{
    "function": "scope_my_queryset",
    "parameters": [
        "filtering_permissions": "core.view_task",
    ],
}

{
    "function": "scope_my_queryset_by_attribute",
    "parameters": [
				"attribute": "task_group",
        "filtering_permissions": "core.view_task_group",
    ],
},

"filtering_conditions": ["file.view_filerepository", "has_remote_param_model:file.view_fileremote"]

## Problem: how to disable Authorization entirely (Is this done?)

Crazy Idea: Push **everything** RBAC into the `AccessPolicyFromDB`

https://discourse.pulpproject.org/t/how-should-pulp-handle-different-default-permission-classes/301

1. Some endpoints override the permission classes
* The status endpoint
* a few pulp_container endpoints
2. Django-Live APIs are different enough to pulp APIs to not use NamedViewSet and not have the machinery for access_policies
* container registry api
* the galaxy API in pulp_ansible
3. access policies are installed regardless of any configuration
* maybe not a problem
4. creation-hooks are always active
* https://github.com/pulp/pulpcore/issues/2116 <- ready for work
5. Queryset scoping currently never turns off, which is AuthZ driven
* scoping is hard coded matching the default access policy
* https://github.com/pulp/pulpcore/issues/2114 <- needs more discussion
* https://github.com/pulp/pulpcore/issues/2115 <- needs more discussion

## Techical Debt

- Deadlock issue: https://github.com/pulp/pulp_container/runs/5551325332#step:14:956
traceback below
- RBAC on more pulpcore viewsets
- Default permissions: https://github.com/pulp/pulpcore/issues/2156
- Tests for reset access policy
- <https://github.com/pulp/pulpcore/pull/1774>
- Least important because currently covered by a CLI test currently
- Does not use generated bindings
- Tests that perform api calls as specific users
- [DONE] We should provide useful helpers there
- pytest fixture factories come to mind
- <https://pytest.org/en/latest/how-to/tmp_path.html#tmp-path-factory-example>
- Documentation for adopting RBAC in plugin
- RBAC user documentation
- [DONE?] Queryset scoping (revisiting)
- Queryset scoping is hard coded (should be part of the access policy)
- Developers should never use querysets not scoped by permissions
- permission filtering on querysets should be made easily accessible (ideas below)
- Schema validation on the access policy

## CLI user interface to object role assignments (DONE)

<https://github.com/pulp/pulp-cli/pull/439>

`pulp task role list --href /pulp/api/v3/tasks/.../` <- that's what i want to introduce
`pulp group role list --group test` <- current state needs to move away
`pulp group-role list --group test`
`pulp role group-assignment list`
`pulp role -t group add`

## Idea: make finding objects for user chainable

Current implementation needs calls like:
`get_objects_for_user(current_user, "core.delete_task", qs=qs)`

We should augment the Managers/QuerySets to be able to:

- `qs.with_perm("core.task_delete")`
- `qs.with_perm("core.task_delete", "core.task_view")`
- `qs.with_perms(["core.task_delete", "core.task_view"])`
- `qs.with_perm("core.task_delete").with_perm( "core.task_view")`
- `.filter(permission="permission")` # Let's not mess with Django filtering
- `.filter(permission__in=["list of permissions"])`

## Idea: Raise an exception for querysets not user-filtered

Enable an object to emit an exception when a queryset has been evaluated and there was no call to `with_perm()`. This would be an opt-in, model-by-model safety feature. Then allow a developer to silence that warning.

- `qs.no_perms_warning()` <-- any Queryset with this will not emit the call, e.g.
- `Task.objects.no_perms_warning().all()`
- Context manager to avoid verbose filters.

What about

- `qs.all() | qs.with_perm("core.task_show")` -> unsafe
- `qs.none() | qs.with_perm("core.task_show")` -> safe
- `qs.none() & qs.with_perm("core.task_show")` ??
- `qs.all() & qs.with_perm("core.task_show")` -> safe

## Galaxy subclassing Group (settled)
When galaxy subclasses Group (proxy model) the creation hooks trigger and try to assign non matching roles.
Options:
- Tell Galaxy to adjust the default access_policy for groups
- Tedious, but probably the right thing to do
- Bring forward https://github.com/pulp/pulpcore/pull/2198 and make use of a different PermissionClass (THIS IS WHAT WE'LL DO)
- Make django.auth.Group work in role permission checks
- May work, but produce a big Awww in the future
- Make creation hooks fault tolerant
- Will make some failures silent
- Find a way to bind creation hooks to a specific model
- Way too hacky
- Reconsider not subclassing Group at all

## deadlock issue stacktrace

pulp [d9e662bf6a87404ba564a4b12e593e33]: django.request:ERROR: Internal Server Error: /rerouted/djnd/api/v3/users/14/
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.DeadlockDetected: deadlock detected
DETAIL:  Process 3555 waits for ShareLock on transaction 5218; blocked by process 3556.
Process 3556 waits for ShareLock on transaction 5221; blocked by process 3555.
HINT:  See server log for query details.
CONTEXT:  while deleting tuple (1,21) in relation "core_userrole"


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/viewsets.py", line 125, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/mixins.py", line 91, in destroy
    self.perform_destroy(instance)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/mixins.py", line 95, in perform_destroy
    instance.delete()
  File "/usr/local/lib/python3.8/site-packages/django/db/models/base.py", line 967, in delete
    return collector.delete()
  File "/usr/local/lib/python3.8/site-packages/django/db/models/deletion.py", line 429, in delete
    count = query.delete_batch(pk_list, self.using)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/sql/subqueries.py", line 43, in delete_batch
    num_deleted += self.do_query(self.get_meta().db_table, self.where, using=using)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/sql/subqueries.py", line 23, in do_query
    cursor = self.get_compiler(using).execute_sql(CURSOR)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1175, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.8/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.OperationalError: deadlock detected
DETAIL:  Process 3555 waits for ShareLock on transaction 5218; blocked by process 3556.
Process 3556 waits for ShareLock on transaction 5221; blocked by process 3555.
HINT:  See server log for query details.
CONTEXT:  while deleting tuple (1,21) in relation "core_userrole"

###### tags: `Minutes`