diff --git a/profiles/default_rbac/default_roles.py b/profiles/default_rbac/default_roles.py index db54a67fa..9d3631d56 100644 --- a/profiles/default_rbac/default_roles.py +++ b/profiles/default_rbac/default_roles.py @@ -21,6 +21,8 @@ "service_catalog.view_instance", "service_catalog.archive_instance", "service_catalog.unarchive_instance", + "service_catalog.rename_instance", + "service_catalog.change_requester_on_instance", "service_catalog.view_request", "service_catalog.cancel_request", diff --git a/service_catalog/forms/instance_forms.py b/service_catalog/forms/instance_forms.py index 7eead3a89..9270cf6d5 100644 --- a/service_catalog/forms/instance_forms.py +++ b/service_catalog/forms/instance_forms.py @@ -6,6 +6,27 @@ class InstanceForm(SquestModelForm): + def __init__(self, *args, **kwargs): + self.user = kwargs.pop('user', None) + super(InstanceForm, self).__init__(*args, **kwargs) + class Meta: model = Instance fields = "__all__" + + +class InstanceFormRestricted(SquestModelForm): + class Meta: + model = Instance + fields = ["name", "requester"] + + def __init__(self, *args, **kwargs): + self.user = kwargs.pop('user', None) + super(InstanceFormRestricted, self).__init__(*args, **kwargs) + if not self.user.has_perm('service_catalog.rename_instance', self.instance): + self.fields.pop('name') + if not self.user.has_perm('service_catalog.change_requester_on_instance', self.instance): + self.fields.pop('requester') + else: + # Update the field to contains all user available in the scope + self.fields["requester"].queryset = self.instance.quota_scope.users diff --git a/service_catalog/migrations/0042_alter_instance_options.py b/service_catalog/migrations/0042_alter_instance_options.py new file mode 100644 index 000000000..8e0d9f9d8 --- /dev/null +++ b/service_catalog/migrations/0042_alter_instance_options.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.13 on 2024-05-21 13:46 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('service_catalog', '0041_operation_validators'), + ] + + operations = [ + migrations.AlterModelOptions( + name='instance', + options={'default_permissions': ('add', 'change', 'delete', 'view', 'list'), 'ordering': ['-last_updated'], 'permissions': [('archive_instance', 'Can archive instance'), ('unarchive_instance', 'Can unarchive instance'), ('request_on_instance', 'Can request a day2 operation on instance'), ('admin_request_on_instance', 'Can request an admin day2 operation on instance'), ('view_admin_spec_instance', 'Can view admin spec on instance'), ('change_admin_spec_instance', 'Can change admin spec on instance'), ('rename_instance', 'Can rename instance'), ('change_requester_on_instance', 'Can change owner of the instance')]}, + ), + ] diff --git a/service_catalog/models/instance.py b/service_catalog/models/instance.py index 5378dff20..b6ead00ff 100644 --- a/service_catalog/models/instance.py +++ b/service_catalog/models/instance.py @@ -30,6 +30,8 @@ class Meta: ("admin_request_on_instance", "Can request an admin day2 operation on instance"), ("view_admin_spec_instance", "Can view admin spec on instance"), ("change_admin_spec_instance", "Can change admin spec on instance"), + ("rename_instance", "Can rename instance"), + ("change_requester_on_instance", "Can change owner of the instance"), ] default_permissions = ('add', 'change', 'delete', 'view', 'list') diff --git a/service_catalog/views/instance.py b/service_catalog/views/instance.py index 51fbefc5d..62ab23588 100644 --- a/service_catalog/views/instance.py +++ b/service_catalog/views/instance.py @@ -14,7 +14,8 @@ from Squest.utils.squest_views import SquestListView, SquestDetailView, SquestUpdateView, SquestDeleteView, \ SquestPermissionDenied from service_catalog.filters.instance_filter import InstanceFilter, InstanceArchivedFilter -from service_catalog.forms import InstanceForm, OperationRequestForm, SupportRequestForm, SupportMessageForm +from service_catalog.forms import InstanceForm, OperationRequestForm, SupportRequestForm, SupportMessageForm, \ + InstanceFormRestricted from service_catalog.models.documentation import Doc from service_catalog.models.instance import Instance from service_catalog.models.instance_state import InstanceState @@ -147,7 +148,31 @@ def get_context_data(self, **kwargs): class InstanceEditView(SquestUpdateView): model = Instance - form_class = InstanceForm + form_class = None + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs.update({'user': self.request.user}) + return kwargs + + def has_permission(self): + try: + obj = self.get_object() + except AttributeError: + obj = None + if self.request.user.has_perm("service_catalog.change_instance", obj) or \ + self.request.user.has_perm("service_catalog.rename_instance", obj) or \ + self.request.user.has_perm("service_catalog.change_owner_instance", obj): + return True + return False + + def get_form_class(self): + if self.request.user.has_perm("service_catalog.change_instance", self.object): + return InstanceForm + elif (self.request.user.has_perm("service_catalog.rename_instance", self.object) or + self.request.user.has_perm("service_catalog.change_owner_instance", self.object)): + return InstanceFormRestricted + return None class InstanceDeleteView(SquestDeleteView): diff --git a/templates/service_catalog/buttons/instance_edit_button.html b/templates/service_catalog/buttons/instance_edit_button.html new file mode 100644 index 000000000..3c07f7ebd --- /dev/null +++ b/templates/service_catalog/buttons/instance_edit_button.html @@ -0,0 +1,10 @@ + +{% has_perm request.user "service_catalog.rename_instance" object as can_rename_instance %} +{% has_perm request.user "service_catalog.change_instance" object as can_change_requester_instance %} +{% has_perm request.user "service_catalog.change_owner_instance" object as can_change_instance %} +{% if can_rename_instance or can_change_requester_instance or can_change_instance %} + + + +{% endif %} diff --git a/templates/service_catalog/instance_detail.html b/templates/service_catalog/instance_detail.html index 371573e1c..0c904f498 100644 --- a/templates/service_catalog/instance_detail.html +++ b/templates/service_catalog/instance_detail.html @@ -9,7 +9,7 @@ {% endblock %} {% block header_button %} - {% include 'generics/buttons/edit_button.html' %} + {% include 'service_catalog/buttons/instance_edit_button.html' %} {% include 'generics/buttons/delete_button.html' %} {% endblock %} {% block custom_links %} @@ -26,7 +26,7 @@
-

{{ object.name }}

+

{{ object.name }}

diff --git a/tests/test_service_catalog/test_forms/test_instance_form.py b/tests/test_service_catalog/test_forms/test_instance_form.py new file mode 100644 index 000000000..d28f2367c --- /dev/null +++ b/tests/test_service_catalog/test_forms/test_instance_form.py @@ -0,0 +1,46 @@ +from profiles.models import Permission +from service_catalog.forms import InstanceFormRestricted +from tests.test_service_catalog.base_test_request import BaseTestRequest + + +class TestInstanceForm(BaseTestRequest): + + def setUp(self): + super(TestInstanceForm, self).setUp() + + def test_instance_form_restricted_with_admin(self): + parameters = { + 'instance': self.test_instance, + 'user': self.standard_user + } + data = { + 'name': 'test_instance_updated', + 'requester': self.standard_user_2, + } + form = InstanceFormRestricted(data, **parameters) + self.assertTrue(form.is_valid()) + self.assertFalse('name' in form.fields) + self.assertFalse('requester' in form.fields) + self.team_member_role.permissions.add( + Permission.objects.get_by_natural_key(codename="rename_instance", + app_label="service_catalog", + model="instance")) + form = InstanceFormRestricted(data, **parameters) + self.assertTrue(form.is_valid()) + self.assertTrue('name' in form.fields) + self.assertFalse('requester' in form.fields) + self.team_member_role.permissions.add( + Permission.objects.get_by_natural_key(codename="change_requester_on_instance", + app_label="service_catalog", + model="instance")) + form = InstanceFormRestricted(data, **parameters) + self.assertTrue('name' in form.fields) + self.assertTrue('requester' in form.fields) + # standard user 2 not part of the team yet so not a valid choice + self.assertFalse(form.is_valid()) + # add standard user 2 to the team + self.test_quota_scope.add_user_in_role(self.standard_user_2, self.team_member_role) + self.test_quota_scope_team.add_user_in_role(self.standard_user_2, self.team_member_role) + form = InstanceFormRestricted(data, **parameters) + self.assertIn(self.standard_user_2, list(form.fields["requester"].queryset)) + self.assertTrue(form.is_valid()) diff --git a/tests/test_service_catalog/test_views/test_admin/test_instance/test_instance_view.py b/tests/test_service_catalog/test_views/test_admin/test_instance/test_instance_view.py index eb8310016..4c8c11acf 100644 --- a/tests/test_service_catalog/test_views/test_admin/test_instance/test_instance_view.py +++ b/tests/test_service_catalog/test_views/test_admin/test_instance/test_instance_view.py @@ -3,6 +3,7 @@ from django.urls import reverse +from profiles.models import Permission from service_catalog.models import Support, Instance, Request, InstanceState from tests.test_service_catalog.base_test_request import BaseTestRequest @@ -67,6 +68,30 @@ def test_instance_edit(self): self.test_instance.refresh_from_db() self.assertEqual(self.test_instance.name, "new_instance_name") + def test_instance_edit_standard_user(self): + self.client.login(username=self.standard_user, password=self.common_password) + url = reverse('service_catalog:instance_edit', kwargs=self.args) + response = self.client.post(url, data=self.edit_instance_data) + # by default is not allowed + self.assertEqual(403, response.status_code) + # give permission to the team + self.team_member_role.permissions.add( + Permission.objects.get_by_natural_key(codename="rename_instance", + app_label="service_catalog", + model="instance"), + Permission.objects.get_by_natural_key(codename="change_requester_on_instance", + app_label="service_catalog", + model="instance") + ) + self.test_quota_scope.add_user_in_role(self.standard_user_2, self.team_member_role) + self.test_quota_scope_team.add_user_in_role(self.standard_user_2, self.team_member_role) + response = self.client.post(url, data=self.edit_instance_data) + # by default is not allowed + self.assertEqual(302, response.status_code) + self.test_instance.refresh_from_db() + self.assertEqual(self.test_instance.name, "new_instance_name") + self.assertEqual(self.test_instance.requester, self.standard_user_2) + def test_instance_edit_with_empty_spec(self): old_spec = copy(self.test_instance.spec) url = reverse('service_catalog:instance_edit', kwargs=self.args)