From 1964118eaa84db0e86c44a6d08955c64eea105ac Mon Sep 17 00:00:00 2001 From: David Newswanger Date: Mon, 15 Nov 2021 14:38:15 -0500 Subject: [PATCH 1/6] - Fix role assignment - Replace get_users_with_perms - Replace get_objects_for_user Issue: AAH-1093 --- galaxy_ng/app/access_control/fields.py | 36 ++++++++----------- galaxy_ng/app/access_control/mixins.py | 30 ++++++++++++---- .../ui/serializers/execution_environment.py | 2 +- galaxy_ng/app/api/ui/viewsets/distribution.py | 2 +- .../api/ui/viewsets/execution_environment.py | 4 +-- galaxy_ng/app/api/ui/viewsets/my_namespace.py | 4 +-- galaxy_ng/app/api/ui/viewsets/my_synclist.py | 7 ++-- galaxy_ng/app/auth/auth.py | 17 +++++---- 8 files changed, 57 insertions(+), 45 deletions(-) diff --git a/galaxy_ng/app/access_control/fields.py b/galaxy_ng/app/access_control/fields.py index b72b17ed8b..ebb9832489 100644 --- a/galaxy_ng/app/access_control/fields.py +++ b/galaxy_ng/app/access_control/fields.py @@ -1,44 +1,38 @@ -from django.contrib.auth.models import Permission -from django.db.models import Q from django.utils.translation import gettext_lazy as _ -from guardian.shortcuts import get_perms_for_model - from rest_framework import serializers from rest_framework.exceptions import ValidationError +from pulpcore.app.models.role import Role + +from pulpcore.app.role_util import get_perms_for_model + from galaxy_ng.app.models import auth as auth_models class GroupPermissionField(serializers.Field): def _validate_group(self, group_data): - if 'object_permissions' not in group_data: + if 'object_roles' not in group_data: raise ValidationError(detail={ - 'groups': _('object_permissions field is required')}) + 'groups': _('object_roles field is required')}) if 'id' not in group_data and 'name' not in group_data: raise ValidationError(detail={ 'groups': _('id or name field is required')}) - perms = group_data['object_permissions'] + roles = group_data['object_roles'] - if not isinstance(perms, list): + if not isinstance(roles, list): raise ValidationError(detail={ - 'groups': _('object_permissions must be a list of strings')}) + 'groups': _('object_roles must be a list of strings')}) # validate that the permissions exist - for perm in perms: - if '.' in perm: - app_label, codename = perm.split('.', maxsplit=1) - filter_q = Q(content_type__app_label=app_label) & Q(codename=codename) - else: - filter_q = Q(codename=perm) - + for role in roles: # TODO(newswangerd): Figure out how to make this one SQL query instead of # performing N queries for each permission - if not Permission.objects.filter(filter_q).exists(): + if not Role.objects.filter(name=role).exists(): raise ValidationError(detail={ - 'groups': _('Permission {} does not exist').format(perm)}) + 'groups': _('Role {} does not exist').format(role)}) def to_representation(self, value): rep = [] @@ -46,7 +40,7 @@ def to_representation(self, value): rep.append({ 'id': group.id, 'name': group.name, - 'object_permissions': value[group] + 'object_roles': value[group] }) return rep @@ -65,7 +59,7 @@ def to_internal_value(self, data): group_filter[field] = group_data[field] try: group = auth_models.Group.objects.get(**group_filter) - internal[group] = group_data['object_permissions'] + internal[group] = group_data['object_roles'] except auth_models.Group.DoesNotExist: raise ValidationError(detail={ 'groups': _("Group name=%s, id=%s does not exist") % ( @@ -84,8 +78,6 @@ def to_representation(self, obj): return [] user = request.user - # guardian's get_perms(user, obj) method only returns user permissions, - # not all permissions a user has. my_perms = [] for perm in get_perms_for_model(type(obj)).all(): codename = "{}.{}".format(perm.content_type.app_label, perm.codename) diff --git a/galaxy_ng/app/access_control/mixins.py b/galaxy_ng/app/access_control/mixins.py index e366e51e8d..266d7be5db 100644 --- a/galaxy_ng/app/access_control/mixins.py +++ b/galaxy_ng/app/access_control/mixins.py @@ -1,6 +1,16 @@ from django.conf import settings from django.db import transaction -from guardian.shortcuts import get_groups_with_perms, assign_perm, remove_perm +from django.core.exceptions import BadRequest +from django.utils.translation import gettext_lazy as _ + +from rest_framework.exceptions import ValidationError + +from pulpcore.app.role_util import ( + assign_role, + remove_role, + get_groups_with_perms_attached_roles +) + from django_lifecycle import hook @@ -9,7 +19,7 @@ class GroupModelPermissionsMixin: @property def groups(self): - return get_groups_with_perms(self, attach_perms=True) + return get_groups_with_perms_attached_roles(self) @groups.setter def groups(self, groups): @@ -17,20 +27,26 @@ def groups(self, groups): @transaction.atomic def _set_groups(self, groups): - # guardian doesn't allow adding permissions to objects that haven't been + # Can't add permissions to objects that haven't been # saved. When creating new objects, save group data to _groups where it # can be picked up by the post save hook. if self._state.adding: self._groups = groups else: - current_groups = get_groups_with_perms(self, attach_perms=True) + current_groups = get_groups_with_perms_attached_roles(self) for group in current_groups: for perm in current_groups[group]: - remove_perm(perm, group, self) + remove_role(perm, group, self) for group in groups: - for perm in groups[group]: - assign_perm(perm, group, self) + for role in groups[group]: + try: + assign_role(role, group, self) + except BadRequest: + raise ValidationError( + detail={'groups': _(f'Role {role} does not exist or does not ' + 'have any permissions related to this object.')} + ) @hook('after_save') def set_object_groups(self): diff --git a/galaxy_ng/app/api/ui/serializers/execution_environment.py b/galaxy_ng/app/api/ui/serializers/execution_environment.py index 239e38f055..7b7ba6492a 100644 --- a/galaxy_ng/app/api/ui/serializers/execution_environment.py +++ b/galaxy_ng/app/api/ui/serializers/execution_environment.py @@ -8,7 +8,7 @@ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import extend_schema_field -from guardian.shortcuts import get_users_with_perms +from pulpcore.app.role_util import get_users_with_perms from pulp_container.app import models as container_models from pulp_container.app import serializers as container_serializers diff --git a/galaxy_ng/app/api/ui/viewsets/distribution.py b/galaxy_ng/app/api/ui/viewsets/distribution.py index 69f6d86878..d2448d935b 100644 --- a/galaxy_ng/app/api/ui/viewsets/distribution.py +++ b/galaxy_ng/app/api/ui/viewsets/distribution.py @@ -1,6 +1,6 @@ from rest_framework import mixins from pulp_ansible.app import models as pulp_models -from guardian.shortcuts import get_objects_for_user +from pulpcore.app.role_util import get_objects_for_user from galaxy_ng.app.access_control import access_policy from galaxy_ng.app.api.ui import serializers, versioning diff --git a/galaxy_ng/app/api/ui/viewsets/execution_environment.py b/galaxy_ng/app/api/ui/viewsets/execution_environment.py index 05fa502125..cb1de8ee6c 100644 --- a/galaxy_ng/app/api/ui/viewsets/execution_environment.py +++ b/galaxy_ng/app/api/ui/viewsets/execution_environment.py @@ -6,7 +6,7 @@ from django_filters import filters from django_filters.rest_framework import DjangoFilterBackend, filterset from drf_spectacular.utils import extend_schema -from guardian.shortcuts import get_objects_for_user +from pulpcore.app.role_util import get_objects_for_user from pulp_container.app import models as container_models from pulpcore.plugin import models as core_models from pulpcore.plugin.serializers import AsyncOperationResponseSerializer @@ -47,7 +47,7 @@ class Meta: def has_permissions(self, queryset, name, value): perms = self.request.query_params.getlist(name) namespaces = get_objects_for_user( - self.request.user, perms, klass=container_models.ContainerNamespace) + self.request.user, perms, qs=container_models.ContainerNamespace.objects.all()) return self.queryset.filter(namespace__in=namespaces) diff --git a/galaxy_ng/app/api/ui/viewsets/my_namespace.py b/galaxy_ng/app/api/ui/viewsets/my_namespace.py index e9bd3b6706..7d8cc27f31 100644 --- a/galaxy_ng/app/api/ui/viewsets/my_namespace.py +++ b/galaxy_ng/app/api/ui/viewsets/my_namespace.py @@ -1,5 +1,5 @@ from galaxy_ng.app import models -from guardian.shortcuts import get_objects_for_user +from pulpcore.app.role_util import get_objects_for_user from .namespace import NamespaceViewSet @@ -10,5 +10,5 @@ def get_queryset(self): self.request.user, ('galaxy.change_namespace', 'galaxy.upload_to_namespace'), any_perm=True, - klass=models.Namespace + qs=models.Namespace.objects.all() ) diff --git a/galaxy_ng/app/api/ui/viewsets/my_synclist.py b/galaxy_ng/app/api/ui/viewsets/my_synclist.py index b6a9bf7a0d..2fbb10d298 100644 --- a/galaxy_ng/app/api/ui/viewsets/my_synclist.py +++ b/galaxy_ng/app/api/ui/viewsets/my_synclist.py @@ -1,7 +1,7 @@ import logging from django.shortcuts import get_object_or_404 -from guardian.shortcuts import get_objects_for_user +from pulpcore.app.role_util import get_objects_for_user from rest_framework.decorators import action @@ -30,9 +30,8 @@ def get_queryset(self): return get_objects_for_user( self.request.user, "galaxy.change_synclist", - any_perm=True, - accept_global_perms=False, - klass=models.SyncList, + # any_perm=True, + qs=models.SyncList.objects.all(), ) @action(detail=True, methods=["post"]) diff --git a/galaxy_ng/app/auth/auth.py b/galaxy_ng/app/auth/auth.py index 3f7651e024..acf24cf0fb 100644 --- a/galaxy_ng/app/auth/auth.py +++ b/galaxy_ng/app/auth/auth.py @@ -4,7 +4,9 @@ from django.conf import settings from django.db import transaction -from guardian import shortcuts + +from pulpcore.app.role_util import get_objects_for_group + from pulp_ansible.app.models import AnsibleDistribution, AnsibleRepository from rest_framework.authentication import BaseAuthentication from rest_framework.exceptions import AuthenticationFailed @@ -79,11 +81,9 @@ def _ensure_group(self, account_scope, account): def _ensure_synclists(self, group): with transaction.atomic(): # check for existing synclists - perms = ['galaxy.view_synclist'] synclists_owned_by_group = \ - shortcuts.get_objects_for_group(group, perms, klass=SyncList, - any_perm=False, accept_global_perms=True) + get_objects_for_group(group, 'galaxy.view_synclist', SyncList.objects.all()) if synclists_owned_by_group: return synclists_owned_by_group @@ -105,8 +105,13 @@ def _ensure_synclists(self, group): }, ) - default_synclist.groups = {group: ['galaxy.view_synclist', 'galaxy.add_synclist', - 'galaxy.delete_synclist', 'galaxy.change_synclist']} + + + + + + # TODO need to create role for synclist owners + default_synclist.groups = {group: ['galaxy.synclist_owner']} default_synclist.save() return default_synclist From e9c6bbaf3ab4b441479e279ccd75c1b8b966c3a4 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Thu, 13 Jan 2022 09:35:18 -0500 Subject: [PATCH 2/6] - Use the plugin api - Rename object_roles back to object_permissions for compatibility - Updated tests - Import Roles model from the plugin api - Remove unused settings import - subclass core.Group for Group model - Viewset shim required by pulpcore - Fix gettext CI error - Add locked roles galaxy.group_admin, galaxy.user_admin and updated galaxy.synclist_owner Issue: AAH-1093 --- CHANGES/1093.misc | 1 + galaxy_ng/app/access_control/fields.py | 9 ++-- galaxy_ng/app/access_control/mixins.py | 7 +-- .../app/access_control/statements/roles.py | 13 ++++++ .../ui/serializers/execution_environment.py | 2 +- galaxy_ng/app/api/ui/viewsets/distribution.py | 4 +- .../api/ui/viewsets/execution_environment.py | 2 +- galaxy_ng/app/api/ui/viewsets/my_namespace.py | 2 +- galaxy_ng/app/api/ui/viewsets/my_synclist.py | 2 +- galaxy_ng/app/auth/auth.py | 8 +--- galaxy_ng/app/models/auth.py | 4 +- galaxy_ng/app/viewsets.py | 9 ++++ .../integration/api/test_locked_roles.py | 4 +- galaxy_ng/tests/unit/api/base.py | 44 ++++++------------- galaxy_ng/tests/unit/api/synclist_base.py | 18 +++----- .../unit/api/test_api_ui_container_remote.py | 9 +--- .../unit/api/test_api_ui_distributions.py | 26 +++++++---- .../unit/api/test_api_ui_my_synclists.py | 12 ++--- .../unit/api/test_api_ui_user_viewsets.py | 41 ++++++----------- .../api/test_api_v3_namespace_viewsets.py | 9 ++-- 20 files changed, 109 insertions(+), 117 deletions(-) create mode 100644 CHANGES/1093.misc diff --git a/CHANGES/1093.misc b/CHANGES/1093.misc new file mode 100644 index 0000000000..b90a363290 --- /dev/null +++ b/CHANGES/1093.misc @@ -0,0 +1 @@ +Removing django-guardian and migrating to RBAC Roles \ No newline at end of file diff --git a/galaxy_ng/app/access_control/fields.py b/galaxy_ng/app/access_control/fields.py index ebb9832489..60aee52ce3 100644 --- a/galaxy_ng/app/access_control/fields.py +++ b/galaxy_ng/app/access_control/fields.py @@ -3,9 +3,9 @@ from rest_framework import serializers from rest_framework.exceptions import ValidationError -from pulpcore.app.models.role import Role +from pulpcore.plugin.models.role import Role -from pulpcore.app.role_util import get_perms_for_model +from pulpcore.plugin.util import get_perms_for_model from galaxy_ng.app.models import auth as auth_models @@ -59,7 +59,10 @@ def to_internal_value(self, data): group_filter[field] = group_data[field] try: group = auth_models.Group.objects.get(**group_filter) - internal[group] = group_data['object_roles'] + if 'object_permissions' in group_data: + internal[group] = group_data['object_permissions'] + if 'object_roles' in group_data: + internal[group] = group_data['object_roles'] except auth_models.Group.DoesNotExist: raise ValidationError(detail={ 'groups': _("Group name=%s, id=%s does not exist") % ( diff --git a/galaxy_ng/app/access_control/mixins.py b/galaxy_ng/app/access_control/mixins.py index 266d7be5db..0784e28d88 100644 --- a/galaxy_ng/app/access_control/mixins.py +++ b/galaxy_ng/app/access_control/mixins.py @@ -5,7 +5,7 @@ from rest_framework.exceptions import ValidationError -from pulpcore.app.role_util import ( +from pulpcore.plugin.util import ( assign_role, remove_role, get_groups_with_perms_attached_roles @@ -44,8 +44,9 @@ def _set_groups(self, groups): assign_role(role, group, self) except BadRequest: raise ValidationError( - detail={'groups': _(f'Role {role} does not exist or does not ' - 'have any permissions related to this object.')} + detail={'groups': _('Role {role} does not exist or does not ' + 'have any permissions related to this object.' + ).format(role=role)} ) @hook('after_save') diff --git a/galaxy_ng/app/access_control/statements/roles.py b/galaxy_ng/app/access_control/statements/roles.py index 5337408bd0..b1cb6f8b54 100644 --- a/galaxy_ng/app/access_control/statements/roles.py +++ b/galaxy_ng/app/access_control/statements/roles.py @@ -36,6 +36,18 @@ "galaxy.upload_to_namespace", "ansible.delete_collection", ], + "galaxy.group_admin": [ + "galaxy.view_group", + "galaxy.delete_group", + "galaxy.add_group", + "galaxy.change_group", + ], + "galaxy.user_admin": [ + "galaxy.view_user", + "galaxy.delete_user", + "galaxy.add_user", + "galaxy.change_user", + ], }, }, "SyncListViewSet": { @@ -45,6 +57,7 @@ "galaxy.change_synclist", "galaxy.delete_synclist", "galaxy.view_synclist", + "ansible.change_collectionremote", ], } }, diff --git a/galaxy_ng/app/api/ui/serializers/execution_environment.py b/galaxy_ng/app/api/ui/serializers/execution_environment.py index 7b7ba6492a..22d8cd52f7 100644 --- a/galaxy_ng/app/api/ui/serializers/execution_environment.py +++ b/galaxy_ng/app/api/ui/serializers/execution_environment.py @@ -8,7 +8,7 @@ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import extend_schema_field -from pulpcore.app.role_util import get_users_with_perms +from pulpcore.plugin.util import get_users_with_perms from pulp_container.app import models as container_models from pulp_container.app import serializers as container_serializers diff --git a/galaxy_ng/app/api/ui/viewsets/distribution.py b/galaxy_ng/app/api/ui/viewsets/distribution.py index d2448d935b..05a926a262 100644 --- a/galaxy_ng/app/api/ui/viewsets/distribution.py +++ b/galaxy_ng/app/api/ui/viewsets/distribution.py @@ -1,6 +1,6 @@ from rest_framework import mixins from pulp_ansible.app import models as pulp_models -from pulpcore.app.role_util import get_objects_for_user +from pulpcore.plugin.util import get_objects_for_user from galaxy_ng.app.access_control import access_policy from galaxy_ng.app.api.ui import serializers, versioning @@ -31,7 +31,7 @@ def get_queryset(self): 'galaxy.change_synclist', any_perm=True, accept_global_perms=False, - klass=models.SyncList + qs=models.SyncList.objects.all() ) # TODO: find a better way query this data diff --git a/galaxy_ng/app/api/ui/viewsets/execution_environment.py b/galaxy_ng/app/api/ui/viewsets/execution_environment.py index cb1de8ee6c..1bd4ed57e6 100644 --- a/galaxy_ng/app/api/ui/viewsets/execution_environment.py +++ b/galaxy_ng/app/api/ui/viewsets/execution_environment.py @@ -6,7 +6,7 @@ from django_filters import filters from django_filters.rest_framework import DjangoFilterBackend, filterset from drf_spectacular.utils import extend_schema -from pulpcore.app.role_util import get_objects_for_user +from pulpcore.plugin.util import get_objects_for_user from pulp_container.app import models as container_models from pulpcore.plugin import models as core_models from pulpcore.plugin.serializers import AsyncOperationResponseSerializer diff --git a/galaxy_ng/app/api/ui/viewsets/my_namespace.py b/galaxy_ng/app/api/ui/viewsets/my_namespace.py index 7d8cc27f31..8e96d4973c 100644 --- a/galaxy_ng/app/api/ui/viewsets/my_namespace.py +++ b/galaxy_ng/app/api/ui/viewsets/my_namespace.py @@ -1,5 +1,5 @@ from galaxy_ng.app import models -from pulpcore.app.role_util import get_objects_for_user +from pulpcore.plugin.util import get_objects_for_user from .namespace import NamespaceViewSet diff --git a/galaxy_ng/app/api/ui/viewsets/my_synclist.py b/galaxy_ng/app/api/ui/viewsets/my_synclist.py index 2fbb10d298..f064fd1afd 100644 --- a/galaxy_ng/app/api/ui/viewsets/my_synclist.py +++ b/galaxy_ng/app/api/ui/viewsets/my_synclist.py @@ -1,7 +1,7 @@ import logging from django.shortcuts import get_object_or_404 -from pulpcore.app.role_util import get_objects_for_user +from pulpcore.plugin.util import get_objects_for_user from rest_framework.decorators import action diff --git a/galaxy_ng/app/auth/auth.py b/galaxy_ng/app/auth/auth.py index acf24cf0fb..93c7d9e935 100644 --- a/galaxy_ng/app/auth/auth.py +++ b/galaxy_ng/app/auth/auth.py @@ -5,7 +5,7 @@ from django.conf import settings from django.db import transaction -from pulpcore.app.role_util import get_objects_for_group +from pulpcore.plugin.util import get_objects_for_group from pulp_ansible.app.models import AnsibleDistribution, AnsibleRepository from rest_framework.authentication import BaseAuthentication @@ -105,12 +105,6 @@ def _ensure_synclists(self, group): }, ) - - - - - - # TODO need to create role for synclist owners default_synclist.groups = {group: ['galaxy.synclist_owner']} default_synclist.save() return default_synclist diff --git a/galaxy_ng/app/models/auth.py b/galaxy_ng/app/models/auth.py index 89813a9cc1..0933c2b54a 100644 --- a/galaxy_ng/app/models/auth.py +++ b/galaxy_ng/app/models/auth.py @@ -2,6 +2,8 @@ from django.contrib.auth import models as auth_models +from pulpcore.plugin.models import Group as PulpGroup + log = logging.getLogger(__name__) __all__ = ( @@ -36,7 +38,7 @@ def _make_name(scope, name): return f"{scope}:{name}" -class Group(auth_models.Group): +class Group(PulpGroup): objects = GroupManager() class Meta: diff --git a/galaxy_ng/app/viewsets.py b/galaxy_ng/app/viewsets.py index 0d15f21f2b..786c88f9a7 100644 --- a/galaxy_ng/app/viewsets.py +++ b/galaxy_ng/app/viewsets.py @@ -46,3 +46,12 @@ class ContentRedirectContentGuardViewSet( ): queryset = models.ContentRedirectContentGuard.objects.all() endpoint_name = "contentgaurd" + + +class AuthViewSet( + pulp_viewsets.NamedModelViewSet, + mixins.RetrieveModelMixin, + mixins.DestroyModelMixin, +): + queryset = models.auth.Group.objects.all() + endpoint_name = "auth" diff --git a/galaxy_ng/tests/integration/api/test_locked_roles.py b/galaxy_ng/tests/integration/api/test_locked_roles.py index 62480e8188..df5e6ab645 100644 --- a/galaxy_ng/tests/integration/api/test_locked_roles.py +++ b/galaxy_ng/tests/integration/api/test_locked_roles.py @@ -13,8 +13,10 @@ def test_locked_roles_exist(ansible_config): galaxy_locked_roles = [ "galaxy.collection_admin", "galaxy.execution_environment_admin", - "galaxy.namespace_owner", + "galaxy.group_admin", "galaxy.publisher", + "galaxy.user_admin", + "galaxy.namespace_owner", "galaxy.synclist_owner", ] diff --git a/galaxy_ng/tests/unit/api/base.py b/galaxy_ng/tests/unit/api/base.py index 2c2ff493cc..8db613057f 100644 --- a/galaxy_ng/tests/unit/api/base.py +++ b/galaxy_ng/tests/unit/api/base.py @@ -8,7 +8,7 @@ from galaxy_ng.app import models from galaxy_ng.app.access_control import access_policy from galaxy_ng.app.models import auth as auth_models -from guardian.shortcuts import assign_perm +from pulpcore.plugin.util import assign_role from galaxy_ng.app import constants @@ -76,13 +76,13 @@ def _create_user(username): return auth_models.User.objects.create(username=username) @staticmethod - def _create_group(scope, name, users=None, perms=[]): + def _create_group(scope, name, users=None, roles=[]): group, _ = auth_models.Group.objects.get_or_create_identity(scope, name) if isinstance(users, auth_models.User): users = [users] group.user_set.add(*users) - for p in perms: - assign_perm(p, group) + for r in roles: + assign_role(r, group) return group @staticmethod @@ -95,51 +95,33 @@ def _create_namespace(name, groups=None): groups_to_add = {} for group in groups: groups_to_add[group] = [ - 'galaxy.upload_to_namespace', - 'galaxy.change_namespace', - 'galaxy.delete_namespace' + 'galaxy.namespace_owner', ] namespace.groups = groups_to_add return namespace @staticmethod def _create_partner_engineer_group(): - pe_perms = [ + pe_roles = [ # namespaces - 'galaxy.add_namespace', - 'galaxy.change_namespace', - 'galaxy.upload_to_namespace', - 'galaxy.delete_namespace', + 'galaxy.namespace_owner', # collections - 'ansible.modify_ansible_repo_content', - 'ansible.delete_collection', + 'galaxy.collection_admin', # users - 'galaxy.view_user', - 'galaxy.delete_user', - 'galaxy.add_user', - 'galaxy.change_user', + 'galaxy.user_admin', # groups - 'galaxy.view_group', - 'galaxy.delete_group', - 'galaxy.add_group', - 'galaxy.change_group', + 'galaxy.group_admin', # synclists - 'galaxy.delete_synclist', - 'galaxy.change_synclist', - 'galaxy.view_synclist', - 'galaxy.add_synclist', - - # sync config - 'ansible.change_collectionremote', + 'galaxy.synclist_owner', ] pe_group = auth_models.Group.objects.create( name='partner-engineers') - for perm in pe_perms: - assign_perm(perm, pe_group) + for role in pe_roles: + assign_role(role, pe_group) return pe_group diff --git a/galaxy_ng/tests/unit/api/synclist_base.py b/galaxy_ng/tests/unit/api/synclist_base.py index cbdf333df2..ffec78d6df 100644 --- a/galaxy_ng/tests/unit/api/synclist_base.py +++ b/galaxy_ng/tests/unit/api/synclist_base.py @@ -2,8 +2,8 @@ from unittest import mock from django.conf import settings -from guardian import shortcuts +from pulpcore.plugin.util import assign_role from pulp_ansible.app import models as pulp_ansible_models from galaxy_ng.app import models as galaxy_models @@ -15,12 +15,7 @@ ACCOUNT_SCOPE = "rh-identity-account" -SYNCLIST_PERMS = [ - "add_synclist", - "view_synclist", - "delete_synclist", - "change_synclist", -] +SYNCLIST_ROLES = ["galaxy.synclist_owner"] log.info("settings.FIXTURE_DIRS(module scope): %s", settings.FIXTURE_DIRS) @@ -28,7 +23,7 @@ class BaseSyncListViewSet(base.BaseTestCase): url_name = "galaxy:api:v3:ui:synclists-list" - default_owner_permissions = SYNCLIST_PERMS + default_owner_roles = SYNCLIST_ROLES def setUp(self): super().setUp() @@ -59,9 +54,8 @@ def _create_group_with_synclist_perms(scope, name, users=None): if isinstance(users, auth_models.User): users = [users] group.user_set.add(*users) - - for perm in SYNCLIST_PERMS: - shortcuts.assign_perm(f"galaxy.{perm}", group) + for role in SYNCLIST_ROLES: + assign_role(role, group) return group def _create_repository(self, name): @@ -87,7 +81,7 @@ def _create_synclist( groups_to_add = {} for group in groups: - groups_to_add[group] = self.default_owner_permissions + groups_to_add[group] = self.default_owner_roles synclist, _ = galaxy_models.SyncList.objects.get_or_create( name=name, repository=repository, upstream_repository=upstream_repository, diff --git a/galaxy_ng/tests/unit/api/test_api_ui_container_remote.py b/galaxy_ng/tests/unit/api/test_api_ui_container_remote.py index 1568779828..111f9c8e26 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_container_remote.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_container_remote.py @@ -17,12 +17,7 @@ class TestContainerRemote(BaseTestCase): def setUp(self): super().setUp() - permissions = [ - # change containers - 'container.namespace_change_containerdistribution', - # create containers - 'container.add_containernamespace', - ] + roles = ['galaxy.execution_environment_admin'] self.container_user = auth_models.User.objects.create(username='container_user') self.admin = auth_models.User.objects.create(username='admin', is_superuser=True) self.regular_user = auth_models.User.objects.create(username='hacker') @@ -43,7 +38,7 @@ def setUp(self): "", "container_group", users=[self.container_user, ], - perms=permissions + roles=roles ) def _create_remote(self, user, name, registry_pk): diff --git a/galaxy_ng/tests/unit/api/test_api_ui_distributions.py b/galaxy_ng/tests/unit/api/test_api_ui_distributions.py index 5e9eb2457d..c95f62b67e 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_distributions.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_distributions.py @@ -1,7 +1,9 @@ import logging +# from django.contrib.auth import default_app_config from rest_framework import status as http_code +from pulpcore.plugin.util import assign_role from pulp_ansible.app import models as pulp_ansible_models from galaxy_ng.app.constants import DeploymentMode @@ -10,16 +12,14 @@ from .base import BaseTestCase, get_current_ui_url +from .synclist_base import ACCOUNT_SCOPE + log = logging.getLogger(__name__) logging.getLogger().setLevel(logging.DEBUG) class TestUIDistributions(BaseTestCase): - default_owner_permissions = [ - 'change_synclist', - 'view_synclist', - 'delete_synclist' - ] + default_owner_roles = ['galaxy.synclist_owner'] def setUp(self): super().setUp() @@ -27,8 +27,9 @@ def setUp(self): self.distro_url = get_current_ui_url('distributions-list') self.my_distro_url = get_current_ui_url('my-distributions-list') - self.group = auth_models.Group.objects.create(name='test1_group') - self.user.groups.add(self.group) + self.group = self._create_group_with_synclist_perms( + ACCOUNT_SCOPE, "test1_group", users=[self.user] + ) self.synclist_repo = self._create_repository('123-synclist') self.repo2 = self._create_repository('other-repo') @@ -54,6 +55,15 @@ def _create_repository(self, name): repo = pulp_ansible_models.AnsibleRepository.objects.create(name=name) return repo + def _create_group_with_synclist_perms(self, scope, name, users=None): + group, _ = auth_models.Group.objects.get_or_create_identity(scope, name) + if isinstance(users, auth_models.User): + users = [users] + group.user_set.add(*users) + for role in self.default_owner_roles: + assign_role(role, group) + return group + def _create_synclist( self, name, repository, upstream_repository, collections=None, namespaces=None, policy=None, groups=None, @@ -63,7 +73,7 @@ def _create_synclist( if groups: groups_to_add = {} for group in groups: - groups_to_add[group] = self.default_owner_permissions + groups_to_add[group] = self.default_owner_roles synclist.groups = groups_to_add return synclist diff --git a/galaxy_ng/tests/unit/api/test_api_ui_my_synclists.py b/galaxy_ng/tests/unit/api/test_api_ui_my_synclists.py index bc9cc36bdd..be5f8cdce5 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_my_synclists.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_my_synclists.py @@ -59,7 +59,7 @@ def test_my_synclist_create(self): { "id": self.group.id, "name": self.group.name, - "object_permissions": self.default_owner_permissions, + "object_roles": self.default_owner_roles, }, ], } @@ -89,7 +89,7 @@ def test_my_synclist_update(self): { "id": self.group.id, "name": self.group.name, - "object_permissions": self.default_owner_permissions, + "object_roles": self.default_owner_roles, }, ], } @@ -109,16 +109,16 @@ def test_my_synclist_update(self): self.assertEqual(response.data["name"], self.synclist_name) self.assertEqual(response.data["policy"], "include") - # Sort permission list for comparison - response.data["groups"][0]["object_permissions"].sort() - self.default_owner_permissions.sort() + # Sort role list for comparison + response.data["groups"][0]["object_roles"].sort() + self.default_owner_roles.sort() self.assertEqual( response.data["groups"], [ { "name": self.group.name, "id": self.group.id, - "object_permissions": self.default_owner_permissions, + "object_roles": self.default_owner_roles } ], ) diff --git a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py index 4cc9e5bead..2d8a989fbd 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_user_viewsets.py @@ -26,11 +26,8 @@ def setUp(self): def test_super_user(self): with self.settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value): user = auth_models.User.objects.create(username='haxor') - self._create_group('', 'test_group1', users=[user], perms=[ - 'galaxy.view_user', - 'galaxy.delete_user', - 'galaxy.add_user', - 'galaxy.change_user', + self._create_group('', 'test_group1', users=[user], roles=[ + 'galaxy.user_admin', ]) self.client.force_authenticate(user=user) new_user_data = { @@ -72,11 +69,8 @@ def test_super_user(self): def test_user_can_only_create_users_with_their_groups(self): user = auth_models.User.objects.create(username='haxor') - group = self._create_group('', 'test_group1', users=[user], perms=[ - 'galaxy.view_user', - 'galaxy.delete_user', - 'galaxy.add_user', - 'galaxy.change_user', + group = self._create_group('', 'test_group1', users=[user], roles=[ + 'galaxy.user_admin', ]) self.client.force_authenticate(user=user) @@ -109,12 +103,9 @@ def test_user_can_only_create_users_with_their_groups(self): def test_user_can_create_users_with_right_perms(self): user = auth_models.User.objects.create(username='haxor') - self._create_group('', 'test_group1', users=[user], perms=[ - 'galaxy.view_user', - 'galaxy.delete_user', - 'galaxy.add_user', - 'galaxy.change_user', - 'galaxy.change_group' + self._create_group('', 'test_group1', users=[user], roles=[ + 'galaxy.user_admin', + 'galaxy.group_admin', ]) self.client.force_authenticate(user=user) @@ -313,12 +304,9 @@ def test_me_delete(self): group = self._create_group('', 'people_that_can_delete_users', users=[user], - perms=[ - 'galaxy.view_user', - 'galaxy.delete_user', - 'galaxy.add_user', - 'galaxy.change_user', - 'galaxy.change_group' + roles=[ + 'galaxy.user_admin', + 'galaxy.group_admin' ]) self.client.force_authenticate(user=user) @@ -361,12 +349,9 @@ def test_superuser_can_not_be_deleted(self): group = self._create_group('', 'people_that_can_delete_users', users=[user], - perms=[ - 'galaxy.view_user', - 'galaxy.delete_user', - 'galaxy.add_user', - 'galaxy.change_user', - 'galaxy.change_group' + roles=[ + 'galaxy.user_admin', + 'galaxy.group_admin' ]) new_user_data = { diff --git a/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py b/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py index e1abd2b8f5..a7fe96e9db 100644 --- a/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py @@ -39,6 +39,8 @@ def setUp(self): def test_namespace_validation(self): ns_name = "unittestnamespace" + print(ns_name) + print(self.pe_group) ns1 = self._create_namespace(ns_name, groups=[self.pe_group]) ns_detail_url = reverse('galaxy:api:v3:namespaces-detail', kwargs={"name": ns1.name}) @@ -186,16 +188,15 @@ def test_namespace_api_creates_deletes_inbound_repo(self): { "id": self.pe_group.id, "name": self.pe_group.name, - "object_permissions": [ - 'galaxy.upload_to_namespace', - 'galaxy.change_namespace', - 'galaxy.delete_namespace', + "object_roles": [ + 'galaxy.namespace_owner', ] }, ], }, format='json', ) + print(f"\n\n response: {response} \n\n") self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(1, len(AnsibleRepository.objects.filter(name=repo_name))) self.assertEqual(1, len(AnsibleDistribution.objects.filter(name=repo_name))) From d12372335323cd444f981c91430bf1d08826ae03 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Tue, 29 Mar 2022 11:29:19 -0400 Subject: [PATCH 3/6] Update pe group management command Issue: AAH-1093 --- .../management/commands/maintain-pe-group.py | 59 ++++++++++++------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/galaxy_ng/app/management/commands/maintain-pe-group.py b/galaxy_ng/app/management/commands/maintain-pe-group.py index afc6d95bf8..4abfbc58a8 100644 --- a/galaxy_ng/app/management/commands/maintain-pe-group.py +++ b/galaxy_ng/app/management/commands/maintain-pe-group.py @@ -1,9 +1,12 @@ +from django.contrib.auth.models import Permission from django.core.management import BaseCommand -from guardian.shortcuts import assign_perm +from pulpcore.plugin.models.role import Role +from pulpcore.plugin.util import assign_role from galaxy_ng.app.models.auth import Group PE_GROUP_NAME = "system:partner-engineers" +PE_ROLE_NAME = "system:partner-engineers" class Command(BaseCommand): @@ -18,34 +21,50 @@ class Command(BaseCommand): help = "Creates/updates partner engineering group with permissions" def handle(self, *args, **options): - pe_group, created = Group.objects.get_or_create(name=PE_GROUP_NAME) - if created: + pe_group, group_created = Group.objects.get_or_create(name=PE_GROUP_NAME) + if group_created: self.stdout.write(f"Created group '{PE_GROUP_NAME}'") else: self.stdout.write(f"Group '{PE_GROUP_NAME}' already exists") + pe_role, role_created = Role.objects.get_or_create(name=PE_ROLE_NAME) + if role_created: + self.stdout.write(f"Created role '{PE_ROLE_NAME}'") + else: + self.stdout.write(f"Role '{PE_ROLE_NAME}' already exists") + pe_perms = [ # groups - "galaxy.view_group", - "galaxy.delete_group", - "galaxy.add_group", - "galaxy.change_group", + ("galaxy", "view_group"), + ("galaxy", "delete_group"), + ("galaxy", "add_group"), + ("galaxy", "change_group"), # users - "galaxy.view_user", - "galaxy.delete_user", - "galaxy.add_user", - "galaxy.change_user", + ("galaxy", "view_user"), + ("galaxy", "delete_user"), + ("galaxy", "add_user"), + ("galaxy", "change_user"), # collections - "ansible.modify_ansible_repo_content", - "ansible.delete_collection", + ("ansible", "modify_ansible_repo_content"), + ("ansible", "delete_collection"), # namespaces - "galaxy.add_namespace", - "galaxy.change_namespace", - "galaxy.upload_to_namespace", - "galaxy.delete_namespace", + ("galaxy", "add_namespace"), + ("galaxy", "change_namespace"), + ("galaxy", "upload_to_namespace"), + ("galaxy", "delete_namespace"), ] + for app_label, codename in pe_perms: + perm = Permission.objects.filter( + content_type__app_label=app_label, + codename=codename, + ).first() + if perm: + pe_role.permissions.add(perm) + else: + self.stdout.write(f"Permission {app_label}.{codename} not found.") - for perm in pe_perms: - assign_perm(perm, pe_group) + assign_role(pe_role, pe_group, self) - self.stdout.write(f"Permissions assigned to '{PE_GROUP_NAME}'") + self.stdout.write( + f"Permissions assigned to '{PE_ROLE_NAME}', Role assigned to '{PE_GROUP_NAME}'" + ) From 2cb3e6c6c482d379c95bcd13e479ef7e0a00729d Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Wed, 30 Mar 2022 15:45:45 -0400 Subject: [PATCH 4/6] - Updated pe group management command - Added galaxy.content_admin locked role - Updated integration test Issue: AAH-1093 --- .../app/access_control/statements/roles.py | 3 ++ .../management/commands/maintain-pe-group.py | 53 +++++-------------- .../integration/api/test_locked_roles.py | 1 + 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/galaxy_ng/app/access_control/statements/roles.py b/galaxy_ng/app/access_control/statements/roles.py index b1cb6f8b54..327ea7fcd9 100644 --- a/galaxy_ng/app/access_control/statements/roles.py +++ b/galaxy_ng/app/access_control/statements/roles.py @@ -24,6 +24,9 @@ }, "NamespaceViewSet": { "LOCKED_ROLES": { + "galaxy.content_admin": [ + "ansible.modify_ansible_repo_content", + ], "galaxy.namespace_owner": [ "galaxy.add_namespace", "galaxy.change_namespace", diff --git a/galaxy_ng/app/management/commands/maintain-pe-group.py b/galaxy_ng/app/management/commands/maintain-pe-group.py index 4abfbc58a8..44092bd60a 100644 --- a/galaxy_ng/app/management/commands/maintain-pe-group.py +++ b/galaxy_ng/app/management/commands/maintain-pe-group.py @@ -1,19 +1,17 @@ -from django.contrib.auth.models import Permission from django.core.management import BaseCommand -from pulpcore.plugin.models.role import Role + from pulpcore.plugin.util import assign_role from galaxy_ng.app.models.auth import Group PE_GROUP_NAME = "system:partner-engineers" -PE_ROLE_NAME = "system:partner-engineers" class Command(BaseCommand): """ This command creates or updates a partner engineering group - with a standard set of permissions. Intended to be used for - settings.GALAXY_DEPLOYMENT_MODE==insights. + with a standard set of permissions via Galaxy locked roles. + Intended to be used for settings.GALAXY_DEPLOYMENT_MODE==insights. $ django-admin maintain-pe-group """ @@ -27,44 +25,17 @@ def handle(self, *args, **options): else: self.stdout.write(f"Group '{PE_GROUP_NAME}' already exists") - pe_role, role_created = Role.objects.get_or_create(name=PE_ROLE_NAME) - if role_created: - self.stdout.write(f"Created role '{PE_ROLE_NAME}'") - else: - self.stdout.write(f"Role '{PE_ROLE_NAME}' already exists") - - pe_perms = [ - # groups - ("galaxy", "view_group"), - ("galaxy", "delete_group"), - ("galaxy", "add_group"), - ("galaxy", "change_group"), - # users - ("galaxy", "view_user"), - ("galaxy", "delete_user"), - ("galaxy", "add_user"), - ("galaxy", "change_user"), - # collections - ("ansible", "modify_ansible_repo_content"), - ("ansible", "delete_collection"), - # namespaces - ("galaxy", "add_namespace"), - ("galaxy", "change_namespace"), - ("galaxy", "upload_to_namespace"), - ("galaxy", "delete_namespace"), + pe_roles = [ + 'galaxy.group_admin', + 'galaxy.user_admin', + 'galaxy.collection_admin', + 'galaxy.namespace_owner', + 'galaxy.content_admin', ] - for app_label, codename in pe_perms: - perm = Permission.objects.filter( - content_type__app_label=app_label, - codename=codename, - ).first() - if perm: - pe_role.permissions.add(perm) - else: - self.stdout.write(f"Permission {app_label}.{codename} not found.") - assign_role(pe_role, pe_group, self) + for role in pe_roles: + assign_role(rolename=role, entity=pe_group) self.stdout.write( - f"Permissions assigned to '{PE_ROLE_NAME}', Role assigned to '{PE_GROUP_NAME}'" + f"Roles assigned to '{PE_GROUP_NAME}'" ) diff --git a/galaxy_ng/tests/integration/api/test_locked_roles.py b/galaxy_ng/tests/integration/api/test_locked_roles.py index df5e6ab645..2ec30960a4 100644 --- a/galaxy_ng/tests/integration/api/test_locked_roles.py +++ b/galaxy_ng/tests/integration/api/test_locked_roles.py @@ -18,6 +18,7 @@ def test_locked_roles_exist(ansible_config): "galaxy.user_admin", "galaxy.namespace_owner", "galaxy.synclist_owner", + "galaxy.content_admin" ] config = ansible_config("ansible_partner") From f2097caa3e125f5d069a27808a67bb5bf28dac4e Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Thu, 31 Mar 2022 09:59:06 -0400 Subject: [PATCH 5/6] Make unit test pe_group consistent with management command maintain-pe-group pe_group - updated tests as needed Issue: AAH-1093 --- galaxy_ng/tests/unit/api/base.py | 13 +++---------- galaxy_ng/tests/unit/api/test_api_ui_sync_config.py | 11 +++++++---- galaxy_ng/tests/unit/api/test_api_v3_tasks.py | 11 +++++++---- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/galaxy_ng/tests/unit/api/base.py b/galaxy_ng/tests/unit/api/base.py index 8db613057f..1ca5b85e05 100644 --- a/galaxy_ng/tests/unit/api/base.py +++ b/galaxy_ng/tests/unit/api/base.py @@ -102,21 +102,14 @@ def _create_namespace(name, groups=None): @staticmethod def _create_partner_engineer_group(): + # Maintain PE Group consistency with + # galaxy_ng/app/management/commands/maintain-pe-group.py:28 pe_roles = [ - # namespaces 'galaxy.namespace_owner', - - # collections 'galaxy.collection_admin', - - # users 'galaxy.user_admin', - - # groups 'galaxy.group_admin', - - # synclists - 'galaxy.synclist_owner', + 'galaxy.content_admin', ] pe_group = auth_models.Group.objects.create( name='partner-engineers') diff --git a/galaxy_ng/tests/unit/api/test_api_ui_sync_config.py b/galaxy_ng/tests/unit/api/test_api_ui_sync_config.py index ecbcde2867..2060868be6 100644 --- a/galaxy_ng/tests/unit/api/test_api_ui_sync_config.py +++ b/galaxy_ng/tests/unit/api/test_api_ui_sync_config.py @@ -8,7 +8,7 @@ CollectionRemote ) from galaxy_ng.app.constants import DeploymentMode -from .base import BaseTestCase +from .synclist_base import BaseSyncListViewSet log = logging.getLogger(__name__) @@ -26,13 +26,16 @@ def _create_remote(name, url, **kwargs): @override_settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value) -class TestUiSyncConfigViewSet(BaseTestCase): +class TestUiSyncConfigViewSet(BaseSyncListViewSet): def setUp(self): super().setUp() self.admin_user = self._create_user("admin") - self.pe_group = self._create_partner_engineer_group() - self.admin_user.groups.add(self.pe_group) + self.sync_group = self._create_group_with_synclist_perms( + scope=None, + name="sync_group", + users=self.admin_user + ) self.admin_user.save() # Remotes are created by data migration diff --git a/galaxy_ng/tests/unit/api/test_api_v3_tasks.py b/galaxy_ng/tests/unit/api/test_api_v3_tasks.py index 839c9e801c..7094d7d56c 100644 --- a/galaxy_ng/tests/unit/api/test_api_v3_tasks.py +++ b/galaxy_ng/tests/unit/api/test_api_v3_tasks.py @@ -4,19 +4,22 @@ from django.test import override_settings from pulp_ansible.app.models import CollectionRemote from galaxy_ng.app.constants import DeploymentMode -from .base import BaseTestCase +from .synclist_base import BaseSyncListViewSet log = logging.getLogger(__name__) @override_settings(GALAXY_DEPLOYMENT_MODE=DeploymentMode.STANDALONE.value) -class TestUiTaskListViewSet(BaseTestCase): +class TestUiTaskListViewSet(BaseSyncListViewSet): def setUp(self): super().setUp() self.admin_user = self._create_user("admin") - self.pe_group = self._create_partner_engineer_group() - self.admin_user.groups.add(self.pe_group) + self.sync_group = self._create_group_with_synclist_perms( + scope=None, + name="sync_group", + users=self.admin_user + ) self.admin_user.save() self.certified_remote = CollectionRemote.objects.get(name='rh-certified') From a307740431a7c89a1dccb1d97675fb30e8ac0cd9 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Thu, 31 Mar 2022 14:41:36 -0400 Subject: [PATCH 6/6] Remove print statements Issue: AAH-1093 --- galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py b/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py index a7fe96e9db..a91a03f21a 100644 --- a/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py @@ -39,8 +39,6 @@ def setUp(self): def test_namespace_validation(self): ns_name = "unittestnamespace" - print(ns_name) - print(self.pe_group) ns1 = self._create_namespace(ns_name, groups=[self.pe_group]) ns_detail_url = reverse('galaxy:api:v3:namespaces-detail', kwargs={"name": ns1.name})