From 15cbadb2e319e20bb3e39879c5ef1d1db3e04597 Mon Sep 17 00:00:00 2001 From: Adams Pierre David <57180807+adamspd@users.noreply.github.com> Date: Sat, 10 Feb 2024 01:40:59 +0100 Subject: [PATCH 1/4] Only admin and appt owner can delete it --- appointment/views_admin.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/appointment/views_admin.py b/appointment/views_admin.py index 5fbfac3..21569eb 100644 --- a/appointment/views_admin.py +++ b/appointment/views_admin.py @@ -239,7 +239,7 @@ def fetch_service_list_for_staff(request): # Ensure the staff member is associated with this appointment if not Appointment.objects.filter(id=appointment_id, appointment_request__staff_member=staff_member).exists(): - return json_response(_("You do not have permission to access this appointment."), status_code=403) + return handle_unauthorized_response(request, _("You do not have permission to access this appointment."), response_type='html') else: # Fetch all services for the staff member (create mode) try: @@ -346,7 +346,7 @@ def update_personal_info(request, staff_user_id=None): 'email': user.email, }, user=user) - context = get_generic_context_with_extra(request=request, extra={'form': form}) + context = get_generic_context_with_extra(request=request, extra={'form': form, 'btn_text': _("Update")}) return render(request, 'administration/manage_staff_personal_info.html', context) @@ -386,7 +386,7 @@ def create_new_staff_member(request): return redirect('appointment:add_staff_member_personal_info') form = PersonalInformationForm() - context = get_generic_context_with_extra(request=request, extra={'form': form}) + context = get_generic_context_with_extra(request=request, extra={'form': form, 'btn_text': _("Create")}) return render(request, 'administration/manage_staff_personal_info.html', context=context) @@ -491,6 +491,9 @@ def get_service_list(request, response_type='html'): @require_staff_or_superuser def delete_appointment(request, appointment_id): appointment = get_object_or_404(Appointment, pk=appointment_id) + if not check_extensive_permissions(appointment.get_staff_member().user_id, request.user, appointment): + message = _("You can only delete your own appointments.") + return handle_unauthorized_response(request, message, 'html') appointment.delete() messages.success(request, _("Appointment deleted successfully!")) return redirect('appointment:get_user_appointments') @@ -502,6 +505,9 @@ def delete_appointment_ajax(request): data = json.loads(request.body) appointment_id = data.get("appointment_id") appointment = get_object_or_404(Appointment, pk=appointment_id) + if not check_extensive_permissions(appointment.get_staff_member().user_id, request.user, appointment): + message = _("You can only delete your own appointments.") + return json_response(message, status=403, success=False, error_code=ErrorCode.NOT_AUTHORIZED) appointment.delete() return json_response(_("Appointment deleted successfully.")) From 0062f3937229c9c21b18db0cef30b85a10069bb2 Mon Sep 17 00:00:00 2001 From: Adams Pierre David <57180807+adamspd@users.noreply.github.com> Date: Tue, 13 Feb 2024 01:43:11 +0100 Subject: [PATCH 2/4] Revert json response message --- appointment/views_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appointment/views_admin.py b/appointment/views_admin.py index 21569eb..38d9d88 100644 --- a/appointment/views_admin.py +++ b/appointment/views_admin.py @@ -239,7 +239,7 @@ def fetch_service_list_for_staff(request): # Ensure the staff member is associated with this appointment if not Appointment.objects.filter(id=appointment_id, appointment_request__staff_member=staff_member).exists(): - return handle_unauthorized_response(request, _("You do not have permission to access this appointment."), response_type='html') + return json_response(_("You do not have permission to access this appointment."), status_code=403) else: # Fetch all services for the staff member (create mode) try: From a8f8719d7b8b01b0c2872b5a55c95349e7347920 Mon Sep 17 00:00:00 2001 From: Adams Pierre David <57180807+adamspd@users.noreply.github.com> Date: Tue, 13 Feb 2024 01:44:06 +0100 Subject: [PATCH 3/4] Minor fixes --- appointment/static/js/appointments.js | 2 +- .../manage_staff_personal_info.html | 2 +- .../administration/manage_working_hours.html | 8 +- .../appointment/default_thank_you.html | 12 +- .../templates/error_pages/403_forbidden.html | 103 +++++++++--------- .../templates/error_pages/404_not_found.html | 2 +- appointment/utils/json_context.py | 3 +- 7 files changed, 66 insertions(+), 66 deletions(-) diff --git a/appointment/static/js/appointments.js b/appointment/static/js/appointments.js index 4c9dfdc..9e10e93 100644 --- a/appointment/static/js/appointments.js +++ b/appointment/static/js/appointments.js @@ -199,7 +199,7 @@ function getAvailableSlots(selectedDate, staffId = null) { console.log('No staff ID provided, displaying error message.'); const errorMessage = $('

'+ noStaffMemberSelectedTxt + '

'); errorMessageContainer.append(errorMessage); - // Optionally disable the submit button here + // Optionally disable the "submit" button here $('.btn-submit-appointment').attr('disabled', 'disabled'); return; // Exit the function early } diff --git a/appointment/templates/administration/manage_staff_personal_info.html b/appointment/templates/administration/manage_staff_personal_info.html index eddb4f5..b409a97 100644 --- a/appointment/templates/administration/manage_staff_personal_info.html +++ b/appointment/templates/administration/manage_staff_personal_info.html @@ -29,7 +29,7 @@

{% trans 'Staff Personal Information' %}

{{ form.email }} - +
{% if messages %} diff --git a/appointment/templates/administration/manage_working_hours.html b/appointment/templates/administration/manage_working_hours.html index 554b2c0..819f383 100644 --- a/appointment/templates/administration/manage_working_hours.html +++ b/appointment/templates/administration/manage_working_hours.html @@ -8,8 +8,8 @@ - {% trans "Manage Working Hours" %} - + - + +{% endblock %} diff --git a/appointment/templates/error_pages/404_not_found.html b/appointment/templates/error_pages/404_not_found.html index a201b92..fcc34c1 100644 --- a/appointment/templates/error_pages/404_not_found.html +++ b/appointment/templates/error_pages/404_not_found.html @@ -1,4 +1,4 @@ -{% extends ADMIN_BASE_TEMPLATE %} +{% extends BASE_TEMPLATE %} {% load i18n %} {% load static %} {% block site_description %}Designer: Adams Pierre David, Category: Resources not found, code 404{% endblock %} diff --git a/appointment/utils/json_context.py b/appointment/utils/json_context.py index 799ceaf..924edc5 100644 --- a/appointment/utils/json_context.py +++ b/appointment/utils/json_context.py @@ -78,7 +78,8 @@ def handle_unauthorized_response(request, message, response_type): # If not 'json', handle as HTML response by default. context = { 'message': message, - 'back_url': reverse('appointment:user_profile') + 'back_url': reverse('appointment:user_profile'), + 'BASE_TEMPLATE': APPOINTMENT_BASE_TEMPLATE, } # set return code to 403 return render(request, 'error_pages/403_forbidden.html', context=context, status=403) From a8d28a91705686f9d481606d7e2986c2d34d11da Mon Sep 17 00:00:00 2001 From: Adams Pierre David <57180807+adamspd@users.noreply.github.com> Date: Tue, 13 Feb 2024 11:09:52 +0100 Subject: [PATCH 4/4] Added tests --- appointment/tests/test_views.py | 35 ++++++++++++++++++++++++++++++++ appointment/utils/permissions.py | 8 ++++++++ appointment/views_admin.py | 9 +++++--- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/appointment/tests/test_views.py b/appointment/tests/test_views.py index eb76d66..ceeed53 100644 --- a/appointment/tests/test_views.py +++ b/appointment/tests/test_views.py @@ -224,6 +224,41 @@ def test_delete_appointment_ajax(self): appointment_exists = Appointment.objects.filter(pk=self.appointment.id).exists() self.assertFalse(appointment_exists, "Appointment should be deleted but still exists.") + def test_delete_appointment_without_permission(self): + """Test that deleting an appointment without permission fails.""" + self.need_staff_login() # Login as a regular staff user + + # Try to delete an appointment belonging to a different staff member + different_appointment = self.create_appointment_for_user2() + url = reverse('appointment:delete_appointment', args=[different_appointment.id]) + + response = self.client.post(url) + + # Check that the user is redirected due to lack of permissions + self.assertEqual(response.status_code, 403) + + # Verify that the appointment still exists in the database + self.assertTrue(Appointment.objects.filter(id=different_appointment.id).exists()) + + def test_delete_appointment_ajax_without_permission(self): + """Test that deleting an appointment via AJAX without permission fails.""" + self.need_staff_login() # Login as a regular staff user + + # Try to delete an appointment belonging to a different staff member + different_appointment = self.create_appointment_for_user2() + url = reverse('appointment:delete_appointment_ajax') + + response = self.client.post(url, {'appointment_id': different_appointment.id}, content_type='application/json') + + # Check that the response indicates failure due to lack of permissions + self.assertEqual(response.status_code, 403) + response_data = response.json() + self.assertEqual(response_data['message'], _("You can only delete your own appointments.")) + self.assertFalse(response_data['success']) + + # Verify that the appointment still exists in the database + self.assertTrue(Appointment.objects.filter(id=different_appointment.id).exists()) + def test_remove_staff_member(self): self.need_superuser_login() self.clean_staff_member_objects() diff --git a/appointment/utils/permissions.py b/appointment/utils/permissions.py index da281cc..e59486a 100644 --- a/appointment/utils/permissions.py +++ b/appointment/utils/permissions.py @@ -27,3 +27,11 @@ def check_permissions(staff_user_id, user): if (staff_user_id and int(staff_user_id) == user.pk) or user.is_superuser: return True return False + + +def has_permission_to_delete_appointment(user, appointment): + """ + Check if the user has permission to delete the given appointment. + Returns True if the user has permission, False otherwise. + """ + return check_extensive_permissions(appointment.get_staff_member().user_id, user, appointment) diff --git a/appointment/views_admin.py b/appointment/views_admin.py index 38d9d88..4660030 100644 --- a/appointment/views_admin.py +++ b/appointment/views_admin.py @@ -33,7 +33,8 @@ from appointment.utils.json_context import ( convert_appointment_to_json, get_generic_context, get_generic_context_with_extra, handle_unauthorized_response, json_response) -from appointment.utils.permissions import check_extensive_permissions, check_permissions +from appointment.utils.permissions import check_extensive_permissions, check_permissions, \ + has_permission_to_delete_appointment ############################################################### @@ -454,6 +455,8 @@ def delete_service(request, service_id): ############################################################### # Remove staff member +@require_user_authenticated +@require_superuser def remove_staff_member(request, staff_user_id): staff_member = get_object_or_404(StaffMember, user_id=staff_user_id) staff_member.delete() @@ -491,7 +494,7 @@ def get_service_list(request, response_type='html'): @require_staff_or_superuser def delete_appointment(request, appointment_id): appointment = get_object_or_404(Appointment, pk=appointment_id) - if not check_extensive_permissions(appointment.get_staff_member().user_id, request.user, appointment): + if not has_permission_to_delete_appointment(request.user, appointment): message = _("You can only delete your own appointments.") return handle_unauthorized_response(request, message, 'html') appointment.delete() @@ -505,7 +508,7 @@ def delete_appointment_ajax(request): data = json.loads(request.body) appointment_id = data.get("appointment_id") appointment = get_object_or_404(Appointment, pk=appointment_id) - if not check_extensive_permissions(appointment.get_staff_member().user_id, request.user, appointment): + if not has_permission_to_delete_appointment(request.user, appointment): message = _("You can only delete your own appointments.") return json_response(message, status=403, success=False, error_code=ErrorCode.NOT_AUTHORIZED) appointment.delete()