From b45fd52b3d4da435962b1706e187343293ffb68e Mon Sep 17 00:00:00 2001 From: Nicolas Marcq Date: Fri, 13 Dec 2024 16:23:18 +0100 Subject: [PATCH] commit review --- Squest/utils/squest_rbac.py | 6 ++---- ...alter_operation_options_operation_permission.py | 4 ++-- service_catalog/models/operations.py | 3 +-- service_catalog/views/instance.py | 2 -- tests/test_service_catalog/base.py | 2 +- .../test_request/test_operation_request_create.py | 2 +- .../test_api/test_urls/test_instance/test_crud.py | 9 ++------- .../test_urls/test_instance/test_crud.py | 12 +++--------- .../test_service_catalog/test_urls/test_service.py | 14 +++++--------- .../test_customer/test_catalog/test_catalog.py | 9 ++------- .../test_instance_request_operation.py | 9 ++------- 11 files changed, 21 insertions(+), 51 deletions(-) diff --git a/Squest/utils/squest_rbac.py b/Squest/utils/squest_rbac.py index 2b4b3dc85..836a592ac 100644 --- a/Squest/utils/squest_rbac.py +++ b/Squest/utils/squest_rbac.py @@ -28,10 +28,8 @@ def has_perm(self, user_obj, perm, obj=None): if not user_obj.is_authenticated: return False - try: - app_label, codename = perm.split(".") - except Exception as test: - print(test) + + app_label, codename = perm.split(".") if obj is None: key = f"{user_obj.id}_{perm}" diff --git a/service_catalog/migrations/0044_alter_operation_options_operation_permission.py b/service_catalog/migrations/0044_alter_operation_options_operation_permission.py index bad325d1c..6df8350c2 100644 --- a/service_catalog/migrations/0044_alter_operation_options_operation_permission.py +++ b/service_catalog/migrations/0044_alter_operation_options_operation_permission.py @@ -16,9 +16,9 @@ def set_perm_to_operations(apps, schema_editor): ContentType = apps.get_model('contenttypes', 'ContentType') operation_content_type = ContentType.objects.get_for_model(Operation) - is_admin_operation, _ = Permission.objects.get_or_create(codename="is_admin_operation", content_type=operation_content_type) + view_admin_operation, _ = Permission.objects.get_or_create(codename="view_admin_operation", content_type=operation_content_type) view_operation, _ = Permission.objects.get_or_create(codename="view_operation", content_type=operation_content_type) - Operation.objects.filter(is_admin_operation=True).update(permission=is_admin_operation) + Operation.objects.filter(is_admin_operation=True).update(permission=view_admin_operation) Operation.objects.filter(is_admin_operation=False).update(permission=view_operation) diff --git a/service_catalog/models/operations.py b/service_catalog/models/operations.py index 1f2ce9c62..070e73d95 100644 --- a/service_catalog/models/operations.py +++ b/service_catalog/models/operations.py @@ -1,6 +1,6 @@ from django.core.exceptions import ValidationError from django.db.models import CharField, ForeignKey, BooleanField, IntegerField, CASCADE, SET_NULL, JSONField, \ - SET_DEFAULT, Q, PROTECT + PROTECT from django.db.models.signals import post_save, pre_save, post_delete from django.dispatch import receiver from django.urls import reverse @@ -10,7 +10,6 @@ from Squest.utils.plugin_controller import PluginController from Squest.utils.squest_model import SquestModel from profiles.models import Permission - from service_catalog.models.job_templates import JobTemplate from service_catalog.models.operation_type import OperationType from service_catalog.models.services import Service diff --git a/service_catalog/views/instance.py b/service_catalog/views/instance.py index e3f368af7..30d96deb8 100644 --- a/service_catalog/views/instance.py +++ b/service_catalog/views/instance.py @@ -1,5 +1,4 @@ import logging -from gc import enable from django.contrib import messages from django.contrib.auth.decorators import login_required @@ -26,7 +25,6 @@ from service_catalog.models.operations import Operation from service_catalog.models.support import Support from service_catalog.tables.instance_tables import InstanceTable -from service_catalog.tables.operation_tables import OperationTableFromInstanceDetails from service_catalog.tables.request_tables import RequestTable from service_catalog.tables.support_tables import SupportTable diff --git a/tests/test_service_catalog/base.py b/tests/test_service_catalog/base.py index 9fe1353a4..aa15e704a 100644 --- a/tests/test_service_catalog/base.py +++ b/tests/test_service_catalog/base.py @@ -373,7 +373,7 @@ def setUp(self): operation_content_type = ContentType.objects.get_for_model(Operation) self.admin_operation, _ = Permission.objects.get_or_create( - content_type=operation_content_type, codename="is_admin_operation") + content_type=operation_content_type, codename="view_admin_operation") # --------------------------------------------------- # Small job template diff --git a/tests/test_service_catalog/test_api/test_request/test_operation_request_create.py b/tests/test_service_catalog/test_api/test_request/test_operation_request_create.py index 6f0a2b37e..ae285a6ea 100644 --- a/tests/test_service_catalog/test_api/test_request/test_operation_request_create.py +++ b/tests/test_service_catalog/test_api/test_request/test_operation_request_create.py @@ -42,7 +42,7 @@ def test_cannot_create_with_disabled_operation(self): def test_customer_cannot_create_an_admin_operation(self): self.client.force_login(self.standard_user) - self.update_operation_test.is_admin_operation = True + self.update_operation_test.permission = self.admin_operation self.update_operation_test.save() request_count = Request.objects.count() response = self.client.post(self.url, data=self.data, format='json') diff --git a/tests/test_service_catalog/test_api/test_urls/test_instance/test_crud.py b/tests/test_service_catalog/test_api/test_urls/test_instance/test_crud.py index 07500cb9b..0424cd5c6 100644 --- a/tests/test_service_catalog/test_api/test_urls/test_instance/test_crud.py +++ b/tests/test_service_catalog/test_api/test_urls/test_instance/test_crud.py @@ -1,7 +1,4 @@ -from django.contrib.contenttypes.models import ContentType - -from profiles.models import Permission -from service_catalog.models import CustomLink, InstanceState, Operation +from service_catalog.models import InstanceState from tests.permission_endpoint import TestPermissionEndpoint, TestingGetContextView, TestingPostContextView, \ TestingPatchContextView, TestingPutContextView, TestingDeleteContextView from tests.test_service_catalog.base_test_request import BaseTestRequestAPI @@ -15,9 +12,7 @@ def setUp(self): self.test_instance_2.service = self.update_operation_test_2.service self.test_instance.save() self.test_instance_2.save() - operation_content_type = ContentType.objects.get_for_model(Operation) - self.update_operation_test_2.permission, _ = Permission.objects.get_or_create(content_type=operation_content_type, - codename="is_admin_operation") + self.update_operation_test_2.permission = self.admin_operation self.update_operation_test_2.save() diff --git a/tests/test_service_catalog/test_urls/test_instance/test_crud.py b/tests/test_service_catalog/test_urls/test_instance/test_crud.py index 389697a59..f413d2907 100644 --- a/tests/test_service_catalog/test_urls/test_instance/test_crud.py +++ b/tests/test_service_catalog/test_urls/test_instance/test_crud.py @@ -1,11 +1,8 @@ import json -from django.contrib.contenttypes.models import ContentType - -from profiles.models import Permission -from service_catalog.models import InstanceState, Operation -from tests.test_service_catalog.base_test_request import BaseTestRequest +from service_catalog.models import InstanceState from tests.permission_endpoint import TestingGetContextView, TestingPostContextView, TestPermissionEndpoint +from tests.test_service_catalog.base_test_request import BaseTestRequest class TestServiceCatalogInstancePermissionsCRUDViews(BaseTestRequest, TestPermissionEndpoint): @@ -17,10 +14,7 @@ def setUp(self): self.test_instance.save() self.test_instance_2.save() - operation_content_type = ContentType.objects.get_for_model(Operation) - self.update_operation_test_2.permission, _ = Permission.objects.get_or_create( - content_type=operation_content_type, - codename="is_admin_operation") + self.update_operation_test_2.permission = self.admin_operation self.update_operation_test_2.save() def test_crud_views(self): diff --git a/tests/test_service_catalog/test_urls/test_service.py b/tests/test_service_catalog/test_urls/test_service.py index f7e9f9c05..1e8e802cb 100644 --- a/tests/test_service_catalog/test_urls/test_service.py +++ b/tests/test_service_catalog/test_urls/test_service.py @@ -1,17 +1,13 @@ -from django.contrib.contenttypes.models import ContentType - from profiles.models import Permission -from service_catalog.models import Operation -from tests.test_service_catalog.base_test_request import BaseTestRequest -from tests.permission_endpoint import TestingGetContextView, TestingPostContextView, TestPermissionEndpoint from service_catalog.forms.form_utils import FormUtils +from tests.permission_endpoint import TestingGetContextView, TestingPostContextView, TestPermissionEndpoint +from tests.test_service_catalog.base_test_request import BaseTestRequest + class TestServiceCatalogServicePermissionsViews(BaseTestRequest, TestPermissionEndpoint): def setUp(self): super().setUp() - operation_content_type = ContentType.objects.get_for_model(Operation) - self.update_operation_test_2.permission, _ = Permission.objects.get_or_create(content_type=operation_content_type, - codename="is_admin_operation") + self.update_operation_test_2.permission = self.admin_operation self.update_operation_test_2.save() def test_service_views(self): testing_view_list = [ @@ -68,7 +64,7 @@ def test_service_views(self): def test_request_service_views(self): self.empty_role.permissions.add( Permission.objects.get(content_type__app_label='profiles', codename='consume_quota_scope')) - self.create_operation_test_2.is_admin_operation = True + self.create_operation_test_2.permission = self.admin_operation self.create_operation_test_2.save() testing_view_list = [ TestingGetContextView( diff --git a/tests/test_service_catalog/test_views/test_customer/test_catalog/test_catalog.py b/tests/test_service_catalog/test_views/test_customer/test_catalog/test_catalog.py index 77698a1b7..7e8b7a748 100644 --- a/tests/test_service_catalog/test_views/test_customer/test_catalog/test_catalog.py +++ b/tests/test_service_catalog/test_views/test_customer/test_catalog/test_catalog.py @@ -1,7 +1,5 @@ -from django.contrib.contenttypes.models import ContentType from django.urls import reverse -from profiles.models import Permission from service_catalog.models import Request, Service, RequestMessage, Portfolio, Operation, OperationType from tests.test_service_catalog.base_test_request import BaseTestRequest @@ -12,9 +10,6 @@ def setUp(self): super(TestCustomerCatalogViews, self).setUp() self.client.login(username=self.standard_user, password=self.common_password) - operation_content_type = ContentType.objects.get_for_model(Operation) - self.is_admin_perm, _ = Permission.objects.get_or_create(content_type=operation_content_type, - codename="is_admin_operation") def test_customer_list_service_in_root(self): url = reverse('service_catalog:service_catalog_list') @@ -133,7 +128,7 @@ def test_customer_cannot_create_request_on_admin_only_operation(self): service=self.service_test, job_template=self.job_template_test, process_timeout_second=20, - permission=self.is_admin_perm) + permission=self.admin_operation) # add a second create operation to avoid being redirected directly to the unique operation form Operation.objects.create(name="create non admin", service=self.service_test, @@ -172,7 +167,7 @@ def test_user_cannot_list_admin_create_operation(self): Operation.objects.create(name="second create admin", service=self.service_test, job_template=self.job_template_test, - permission=self.is_admin_perm, + permission=self.admin_operation, type=OperationType.CREATE, process_timeout_second=20) Operation.objects.create(name="third create non admin", diff --git a/tests/test_service_catalog/test_views/test_customer/test_instance/test_instance_request_operation.py b/tests/test_service_catalog/test_views/test_customer/test_instance/test_instance_request_operation.py index fc366636e..1014e5c30 100644 --- a/tests/test_service_catalog/test_views/test_customer/test_instance/test_instance_request_operation.py +++ b/tests/test_service_catalog/test_views/test_customer/test_instance/test_instance_request_operation.py @@ -1,8 +1,6 @@ -from django.contrib.contenttypes.models import ContentType from django.urls import reverse -from profiles.models import Permission -from service_catalog.models import Request, RequestMessage, Doc, Operation +from service_catalog.models import Request, RequestMessage, Doc from service_catalog.models.instance import InstanceState from tests.test_service_catalog.base_test_request import BaseTestRequest @@ -57,10 +55,7 @@ def test_cannot_request_non_valid_operation(self): def test_customer_cannot_request_admin_operation(self): # set an operation to be admin only - operation_content_type = ContentType.objects.get_for_model(Operation) - self.update_operation_test.permission, _ = Permission.objects.get_or_create( - content_type=operation_content_type, - codename="is_admin_operation") + self.update_operation_test.permission = self.admin_operation self.update_operation_test.save() args = { 'instance_id': self.test_instance.id,