diff --git a/ansible_base/rbac/validators.py b/ansible_base/rbac/validators.py index 07014aa8b..cd94b0bb5 100644 --- a/ansible_base/rbac/validators.py +++ b/ansible_base/rbac/validators.py @@ -34,7 +34,7 @@ def permissions_allowed_for_system_role() -> dict[type, list[str]]: return permissions_by_model -def permissions_allowed_for_role(cls) -> dict[type, list[str]]: +def permissions_allowed_for_role(cls: Union[Model, Type[Model], None]) -> dict[type, list[str]]: "Permission codenames valid for a RoleDefinition of given class, organized by permission class" if cls is None: return permissions_allowed_for_system_role() @@ -42,8 +42,13 @@ def permissions_allowed_for_role(cls) -> dict[type, list[str]]: if not permission_registry.is_registered(cls): raise ValidationError(f'Django-ansible-base RBAC does not track permissions for model {cls._meta.model_name}') - # Include direct model permissions (except for add permission) permissions_by_model = defaultdict(list) + + info = permission_registry.get_info(cls) + if not info.allow_object_roles: + return permissions_by_model + + # Include direct model permissions (except for add permission) permissions_by_model[cls] = [codename for codename in codenames_for_cls(cls) if not is_add_perm(codename)] # Include model permissions for all child models, including the add permission @@ -100,6 +105,10 @@ def validate_permissions_for_model(permissions, content_type: Optional[Model], m role_model = content_type.model_class() permissions_by_model = permissions_allowed_for_role(role_model) + if not permissions_by_model: + print_model = role_model._meta.verbose_name if role_model else 'global roles' + raise ValidationError({'content_type': f'Creating roles for the {print_model} model is disabled'}) + invalid_codenames = codename_list - combine_values(permissions_by_model) if invalid_codenames: print_codenames = ', '.join(f'"{codename}"' for codename in invalid_codenames) diff --git a/test_app/migrations/0001_initial.py b/test_app/migrations/0001_initial.py index 53adf83b5..83e029f18 100644 --- a/test_app/migrations/0001_initial.py +++ b/test_app/migrations/0001_initial.py @@ -284,4 +284,20 @@ class Migration(migrations.Migration): }, bases=('test_app.original2',), ), + migrations.CreateModel( + name='MemberGuide', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('modified', models.DateTimeField(auto_now=True, help_text='The date/time this resource was created')), + ('created', models.DateTimeField(auto_now_add=True, help_text='The date/time this resource was created')), + ('name', models.CharField(help_text='The name of this resource', max_length=512)), + ('article', models.TextField(default='-- Help article stub --')), + ('created_by', models.ForeignKey(default=None, editable=False, help_text='The user who created this resource', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_created+', to=settings.AUTH_USER_MODEL)), + ('modified_by', models.ForeignKey(default=None, editable=False, help_text='The user who last modified this resource', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_modified+', to=settings.AUTH_USER_MODEL)), + ('organization', models.ForeignKey(help_text='Docs for all org members', on_delete=django.db.models.deletion.CASCADE, related_name='member_guides', to='test_app.organization')), + ], + options={ + 'abstract': False, + }, + ), ] diff --git a/test_app/models.py b/test_app/models.py index a7951168e..6c66ec182 100644 --- a/test_app/models.py +++ b/test_app/models.py @@ -190,6 +190,20 @@ class UUIDModel(models.Model): organization = models.ForeignKey(Organization, on_delete=models.CASCADE, related_name='uuidmodels') +class MemberGuide(NamedCommonModel): + """Demonstrates use case for hypothetical model with no object-level roles + + The pretend use-case is that this saves articles for documentation, which is intended to + be available for all members of the organization. + Since this tracks so closely with organization member permission, object-level roles + would be overkill and confusing. + Our intent is that permissions are only delegated at the organization level. + """ + + organization = models.ForeignKey(Organization, on_delete=models.CASCADE, related_name='member_guides', help_text='Docs for all org members') + article = models.TextField(default='-- Help article stub --') + + class ImmutableTask(models.Model): "Hypothetical immutable task-like thing, can be created and canceled but not edited" @@ -262,6 +276,7 @@ class Meta: permission_registry.register(ParentName, parent_field_name='my_organization') permission_registry.register(CollectionImport, parent_field_name='namespace') permission_registry.register(InstanceGroup, ImmutableTask, parent_field_name=None) +permission_registry.register(MemberGuide, allow_object_roles=False) # NOTE(cutwater): Using hard coded role names instead of ones defined in ReconcileUser class, # to avoid circular dependency between models and claims modules. This is a temporary workarond, diff --git a/test_app/router.py b/test_app/router.py index f5edf67b6..159bf1a42 100644 --- a/test_app/router.py +++ b/test_app/router.py @@ -60,6 +60,7 @@ def filter_associate_queryset(self, qs): 'parentnames': (views.ParentNameViewSet, 'parentnames'), 'positionmodels': (views.PositionModelViewSet, 'positionmodels'), 'weirdperms': (views.WeirdPermViewSet, 'weirdperms'), + 'member_guides': (views.MemberGuideViewSet, 'member_guides'), }, ) diff --git a/test_app/serializers.py b/test_app/serializers.py index 64347c397..97eac68f7 100644 --- a/test_app/serializers.py +++ b/test_app/serializers.py @@ -110,6 +110,12 @@ class Meta: fields = '__all__' +class MemberGuideSerializer(RelatedAccessMixin, ModelSerializer): + class Meta: + model = models.MemberGuide + fields = '__all__' + + class ImmutableLogEntrySerializer(ImmutableCommonModelSerializer): class Meta: model = models.ImmutableLogEntry diff --git a/test_app/tests/rbac/features/test_object_roles_disabled.py b/test_app/tests/rbac/features/test_object_roles_disabled.py new file mode 100644 index 000000000..8be7a8bd5 --- /dev/null +++ b/test_app/tests/rbac/features/test_object_roles_disabled.py @@ -0,0 +1,67 @@ +import pytest +from django.contrib.contenttypes.models import ContentType +from rest_framework.exceptions import ValidationError +from rest_framework.reverse import reverse + +from ansible_base.rbac.models import DABPermission, RoleDefinition +from ansible_base.rbac.validators import permissions_allowed_for_role, validate_permissions_for_model +from test_app.models import MemberGuide + + +@pytest.fixture +def member_guide(organization): + return MemberGuide.objects.create(name='Beginner stuff', article='This is where you file a ticket: https://foo.invalid', organization=organization) + + +@pytest.mark.django_db +def test_org_admin_access(rando, organization, member_guide): + assert not rando.has_obj_perm(member_guide, 'change') + RoleDefinition.objects.managed.org_admin.give_permission(rando, organization) + assert rando.has_obj_perm(member_guide, 'change') + + +@pytest.mark.django_db +def test_no_permissions_allowed_for_model(): + assert permissions_allowed_for_role(MemberGuide) == {} + + +@pytest.mark.django_db +def test_role_definition_validation_error(): + mg_ct = ContentType.objects.get_for_model(MemberGuide) + permissions = [DABPermission.objects.get(codename='view_memberguide')] + with pytest.raises(ValidationError) as exc: + validate_permissions_for_model(permissions, mg_ct) + assert 'Creating roles for the member guide model is disabled' in str(exc) + + +@pytest.mark.django_db +def test_custom_role_denied_elegantly(admin_api_client): + url = reverse('roledefinition-list') + data = {'name': 'MemberGuide object role', 'permissions': ['local.view_memberguide'], 'content_type': 'local.memberguide'} + response = admin_api_client.post(url, data=data, format='json') + assert response.status_code == 400, response.data + assert 'Creating roles for the member guide model is disabled' in str(response.data['content_type']) + + +@pytest.mark.django_db +def test_role_metadata_without_object_roles(user_api_client): + url = reverse('role-metadata') + response = user_api_client.get(url) + assert 'allowed_permissions' in response.data + allowed_permissions = response.data['allowed_permissions'] + assert 'shared.organization' in allowed_permissions.keys() # sanity + assert 'memberguide' not in str(allowed_permissions.keys()) + assert 'aap.change_memberguide' in allowed_permissions['shared.organization'] + + +@pytest.mark.django_db +def test_custom_role_for_organization(admin_api_client, rando, member_guide, organization): + url = reverse('roledefinition-list') + data = {'name': 'MemberGuide view', 'permissions': ['local.view_memberguide'], 'content_type': 'local.organization'} + response = admin_api_client.post(url, data=data, format='json') + assert response.status_code == 201, response.data + + assert not rando.has_obj_perm(member_guide, 'view') + rd = RoleDefinition.objects.get(id=response.data['id']) + rd.give_permission(rando, organization) + assert rando.has_obj_perm(member_guide, 'view') diff --git a/test_app/views.py b/test_app/views.py index a15d34f9e..7f0bef2a5 100644 --- a/test_app/views.py +++ b/test_app/views.py @@ -176,6 +176,10 @@ def api_root(request, format=None): return Response(list_endpoints) +class MemberGuideViewSet(TestAppViewSet): + serializer_class = serializers.MemberGuideSerializer + + class MultipleFieldsViewSet(TestAppViewSet): serializer_class = serializers.MultipleFieldsModelSerializer