Skip to content

Commit

Permalink
Namespace vieset_name in AccessPolicy
Browse files Browse the repository at this point in the history
To avoid naming conflicts with similarly named viewsets in plugins, a
scoping by the plugin app_id is needed.

fixes #7845
https://pulp.plan.io/issues/7845
  • Loading branch information
mdellweg committed Nov 26, 2020
1 parent ad98ad0 commit 7f738e0
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGES/7845.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Properly namespaced the `viewset_name` in `AccessPolicy` to avoid naming conflicts in plugins.
10 changes: 10 additions & 0 deletions CHANGES/plugin_api/7845.deprecation
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Using the ViewSet's classname to identify its AccessPolicy has been deprecated and is slated for removal in 3.10.
Instead the urlpattern is supposed to be used.

Plugins with existing AccessPolicies should add a data migration to rename their AccessPolicies:

::
access_policy = AccessPolicy.get(viewset_name="MyViewSet")
access_policy.viewset_name = "objectclass/myplugin/myclass"
access_policy.save()

2 changes: 1 addition & 1 deletion docs/plugins/plugin-writer/concepts/rbac/access_policy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ an example of code to create an instance, which would be contained in a data mig
]
AccessPolicy.objects.create(
viewset_name="FileRemoteViewSet",
viewset_name="remotes/file/file",
statements=FILE_REMOTE_STATEMENTS,
permissions_assignment=FILE_REMOTE_PERMISSIONS_ASSIGNMENT
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ this:
]
AccessPolicy.objects.create(
viewset_name="FileRemoteViewSet",
viewset_name="remotes/file/file",
statements=FILE_REMOTE_STATEMENTS,
permissions_assignment=FILE_REMOTE_PERMISSIONS_ASSIGNMENT
)
Expand Down
2 changes: 1 addition & 1 deletion docs/plugins/plugin-writer/concepts/rbac/permissions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ By default, each model receives four permissions:
* The “view” permission limits the ability to view an object.

The Model-level permissions are created automatically by Django, and receive a name like:
``<app_name>_<action>_<model_name>``. For example to change file remote the permission is named
``<app_name>.<action>_<model_name>``. For example to change file remote the permission is named
``file.change_fileremote``. You can view the Permissions on a system via the Django ORM with:
``Permission.objects.all()``. See the `Django Permissions Docs <https://docs.djangoproject.com/en/
2.2/topics/auth/default/#permissions-and-authorization>`_ for more information on working with
Expand Down
14 changes: 13 additions & 1 deletion pulpcore/app/access_policy.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import warnings

from rest_access_policy import AccessPolicy

from pulpcore.app.models import AccessPolicy as AccessPolicyModel
Expand Down Expand Up @@ -29,5 +31,15 @@ def get_policy_statements(self, request, view):
Returns:
The access policy statements in drf-access-policy policy structure.
"""
access_policy_obj = AccessPolicyModel.objects.get(viewset_name=view.__class__.__name__)
try:
access_policy_obj = AccessPolicyModel.objects.get(
viewset_name=view.__class__.urlpattern()
)
except AccessPolicyModel.NotFound:
access_policy_obj = AccessPolicyModel.objects.get(viewset_name=view.__class__.__name__)
warnings.warn(
"Addressing AccessPolicy via the viewset's classname is deprecated"
"and will be removed in pulpcore==3.10; use the viewset's urlpattern().",
warnings.DeprecationWarning,
)
return access_policy_obj.statements
28 changes: 28 additions & 0 deletions pulpcore/app/migrations/0050_namespace_access_policies.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Generated by Django 2.2.17 on 2020-11-18 09:41

from django.db import migrations

from pulpcore.app.models import AccessPolicy


def namespace_access_policies_up(apps, schema_editor):
task_policy = AccessPolicy.objects.get(viewset_name="TaskViewSet")
task_policy.viewset_name = "tasks"
task_policy.save()


def namespace_access_policies_down(apps, schema_editor):
AccessPolicy.objects.get(viewset_name="tasks").delete()
task_policy.viewset_name = "TaskViewSet"
task_policy.save()


class Migration(migrations.Migration):

dependencies = [
('core', '0049_add_file_field_to_uploadchunk'),
]

operations = [
migrations.RunPython(namespace_access_policies_up, reverse_code=namespace_access_policies_down),
]
2 changes: 1 addition & 1 deletion pulpcore/app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ class Task(BaseModel, AutoDeleteObjPermsMixin, AutoAddObjPermsMixin):
# as it has a separate job ID that is not the task ID.
_resource_job_id = models.UUIDField()

ACCESS_POLICY_VIEWSET_NAME = "TaskViewSet"
ACCESS_POLICY_VIEWSET_NAME = "tasks"

@staticmethod
def current():
Expand Down

0 comments on commit 7f738e0

Please sign in to comment.