From 415d9da44efd9f3241bcd6ee4f478e8da88081b9 Mon Sep 17 00:00:00 2001 From: Nivesh Mittapally Date: Mon, 3 Jul 2023 21:35:39 +0530 Subject: [PATCH 1/2] fix: error handling for view functions --- Access/accessrequest_helper.py | 71 +++--- Access/decorators.py | 10 +- Access/group_helper.py | 26 +-- Access/views.py | 207 ++++++++---------- enigma_automation/settings.py | 2 + templates/EnigmaOps/dashboard.html | 24 ++ .../EnigmaOps/groupAccessRequestForm.html | 52 +++-- 7 files changed, 192 insertions(+), 200 deletions(-) diff --git a/Access/accessrequest_helper.py b/Access/accessrequest_helper.py index 1c15e93d..891233a9 100644 --- a/Access/accessrequest_helper.py +++ b/Access/accessrequest_helper.py @@ -117,44 +117,33 @@ def __init__(self): def get_request_access(request): """ Get list of all accesses for requesting access to """ context = {} - try: - for each_tag, each_module in \ - helpers.get_available_access_modules().items(): - if each_tag in request.GET.getlist("accesses"): - if "accesses" not in context: - context["accesses"] = [] - context["genericForm"] = True - try: - extra_fields = each_module.get_extra_fields() - except Exception: - extra_fields = [] - try: - notice = each_module.get_notice() - - except Exception: - notice = "" - context["accesses"].append({ - "formDesc": each_module.access_desc(), - "accessTag": each_tag, - "accessTypes": each_module.access_types(), - "accessRequestData": each_module.access_request_data( - request, is_group=False - ), - "extraFields": extra_fields, - "notice": notice, - "accessRequestPath": - each_module.fetch_access_request_form_path(), - }) - except Exception as exception: - logger.exception(exception) - context = {} - context["status"] = { - "title": "Error Occured", - "msg": ( - "There was an error in getting the requested access" - " resources. Please contact Admin" - ), - } + for each_tag, each_module in \ + helpers.get_available_access_modules().items(): + if each_tag in request.GET.getlist("accesses"): + if "accesses" not in context: + context["accesses"] = [] + context["genericForm"] = True + try: + extra_fields = each_module.get_extra_fields() + except Exception: + extra_fields = [] + try: + notice = each_module.get_notice() + + except Exception: + notice = "" + context["accesses"].append({ + "formDesc": each_module.access_desc(), + "accessTag": each_tag, + "accessTypes": each_module.access_types(), + "accessRequestData": each_module.access_request_data( + request, is_group=False + ), + "extraFields": extra_fields, + "notice": notice, + "accessRequestPath": + each_module.fetch_access_request_form_path(), + }) return context @@ -441,6 +430,9 @@ def create_request(auth_user, access_request_form): access_module = helpers.get_available_access_module_from_tag(access_tag) module_access_labels = access_module.validate_request(access_request_form, auth_user.user) + if access_module.can_auto_approve(): + raise ImplementationPendingException() + for _, access_label in enumerate(module_access_labels): request_id = ( auth_user.username @@ -458,9 +450,6 @@ def create_request(auth_user, access_request_form): logger.info("Duplicate request found: %s", access_create_error) continue - if access_module.can_auto_approve(): - raise ImplementationPendingException() - json_response["status"] = { "title": REQUEST_SUCCESS_MSG["title"].format( access_tag=access_tag diff --git a/Access/decorators.py b/Access/decorators.py index 84cec0aa..671f7e8e 100644 --- a/Access/decorators.py +++ b/Access/decorators.py @@ -1,6 +1,7 @@ from django.core.exceptions import PermissionDenied from Access.helpers import get_possible_approver_permissions from django.core.paginator import Paginator +from django.template.response import TemplateResponse def user_admin_or_ops(function): @@ -54,7 +55,14 @@ def wrap(request, *args, **kwargs): def paginated_search(view_function): def wrap(request, *args, **kwargs): - template, context = view_function(request, *args, **kwargs) + response = view_function(request, *args, **kwargs) + if type(response) is not tuple: + return response + template, context = response + if not context: + if type(template) == TemplateResponse: + return template.render() + return template if context.get("error"): template.context_data = context return template.render() diff --git a/Access/group_helper.py b/Access/group_helper.py index 2abffc59..0490a843 100644 --- a/Access/group_helper.py +++ b/Access/group_helper.py @@ -26,6 +26,11 @@ "msg": "Error Occured while loading the page. Please contact admin", } +INVALID_REQUEST_ERROR = { + "error_msg": "Invalid Request", + "msg": "Error in request parameters, please check and request." +} + USER_UNAUTHORIZED_MESSAGE = "User unauthorised to perform the action." GROUP_ACCESS_MAPPING_NOT_FOUND = "Group Access Mapping not found in the database." @@ -128,10 +133,8 @@ def create_group(request): except Exception as e: logger.exception(e) logger.error("Error in Create New Group request.") - context = {} - context["error"] = { - "error_msg": INTERNAL_ERROR_MESSAGE["error_msg"], - "msg": INTERNAL_ERROR_MESSAGE["msg"], + context = { + "error": INVALID_REQUEST_ERROR } return context @@ -215,15 +218,6 @@ def get_group_access_list(auth_user, group_name): return context auth_user = auth_user - if not auth_user.user.is_allowed_admin_actions_on_group(group): - logger.debug("Permission denied, requester is non owner") - context = { - "error": { - "error_msg": NON_OWNER_PERMISSION_DENIED_ERROR["error_msg"], - "msg": NON_OWNER_PERMISSION_DENIED_ERROR["msg"], - } - } - return context group_members = group.get_all_members().filter(status="Approved") group_members = [ @@ -638,8 +632,8 @@ def accept_member(auth_user, requestId, shouldRender=True): def get_group_access(form_data, auth_user): - data = dict(form_data.lists()) - group_name = data["groupName"][0] + # data = dict(form_data.lists()) + group_name = form_data.get("groupName") #data["groupName"][0] context = {} context["accesses"] = [] @@ -651,7 +645,7 @@ def get_group_access(form_data, auth_user): context["status"] = validation_error return context - access_module_list = data["accessList"] + access_module_list = form_data.getlist("accessList") for module_value in access_module_list: module = helper.get_available_access_modules()[module_value] try: diff --git a/Access/views.py b/Access/views.py index c3769711..eca59473 100644 --- a/Access/views.py +++ b/Access/views.py @@ -8,8 +8,9 @@ from django.core.exceptions import ValidationError from django.core.paginator import Paginator from django.contrib.auth.models import User as djangoUser -from django.http import JsonResponse -from django.shortcuts import render +from django.http import HttpResponseServerError, JsonResponse, HttpResponseNotFound +from django.contrib import messages +from django.shortcuts import redirect, render from django.template.response import TemplateResponse from django.db.models import Q @@ -58,27 +59,31 @@ @login_required +@api_view(["GET"]) @paginated_search def show_access_history(request): """Show access request history for a User.""" - if request.method == "POST": - return render_error_message( - request, - "POST for showAccessHistory not supported", - "Invalid Request", - "Error request not found OR Invalid request type", - ) - - try: - access_user = User.objects.get(email=request.user.email) - except Exception as ex: - return render_error_message( - request, - f"Access user with email {request.user.email} not found. Error: {str(ex)}", - "Invalid Request", - "Please login again", - ) - + # if request.method == "POST": + # logger.error("POST for showAccessHistory not supported") + # return HttpResponseNotFound("Error request not found OR Invalid request type"), {} + # return render_error_message( + # request, + # "POST for showAccessHistory not supported", + # "Invalid Request", + # "Error request not found OR Invalid request type", + # ) + + # try: + # access_user = User.objects.get(email=request.user.email) + # except Exception as ex: + # return render_error_message( + # request, + # f"Access user with email {request.user.email} not found. Error: {str(ex)}", + # "Invalid Request", + # "Please login again", + # ) + + access_user = request.user.user selected_status = request.GET.getlist("status") selected_access_modules = request.GET.getlist("access_desc") @@ -108,25 +113,12 @@ def show_access_history(request): @login_required +@api_view(["GET"]) def new_access_request(request): """ new access request """ - if request.method == "POST": - return render_error_message( - request, - "POST for this endpoint is not supported", - "Invalid Request", - "Error request not found OR Invalid request type", - ) - - try: - User.objects.get(email=request.user.email) - except Exception as exc: - return render_error_message( - request, - f"Access user with email {request.user.email} not found. Error: {str(exc)}", - "Invalid Request", - "Please login again", - ) + # if request.method == "POST": + # logger.error("POST for this endpoint is not supported") + # return HttpResponseNotFound("Error request not found OR Invalid request type") return render( request, @@ -135,6 +127,7 @@ def new_access_request(request): @login_required +@api_view(["GET"]) @user_admin_or_ops def failure_requests(request): """ Request failed to process by celery """ @@ -151,15 +144,18 @@ def failure_requests(request): logger.error( "Error in request not found OR Invalid request type, Error: %s", str(ex) ) - json_response = {} - json_response["error"] = {"error_msg": str(ex), "msg": INVALID_REQUEST_MESSAGE} - return render(request, "EnigmaOps/accessStatus.html", json_response) + # json_response = {} + # json_response["error"] = {"error_msg": str(ex), "msg": INVALID_REQUEST_MESSAGE} + # return render(request, "EnigmaOps/accessStatus.html", json_response) + return HttpResponseServerError("Internal Server Error, Something went wrong while fetching this page") @login_required +@api_view(["GET"]) def update_user_info(request): """Templates to capture indentity information, with already saved Identity information for access modules""" + context = get_identity_templates(request.user) return render(request, "updateUser.html", context) @@ -190,31 +186,29 @@ def save_identity(request): "title": IDENTITY_UNCHANGED_ERROR_MESSAGE["title"], "msg": IDENTITY_UNCHANGED_ERROR_MESSAGE["msg"].format(modulename=modname), } - return JsonResponse(json.dumps(context), safe=False, status=200) + return JsonResponse(json.dumps(context), safe=False, status=400) except Exception: context["error"] = { "title": NEW_IDENTITY_CREATE_ERROR_MESSAGE["title"], "msg": NEW_IDENTITY_CREATE_ERROR_MESSAGE["msg"].format(modulename=modname), } - return JsonResponse(json.dumps(context), safe=False, status=200) - context["error"] = { - "title": "Bad Request", - "msg": "Invalid Request.", - } - return JsonResponse(json.dumps(context), safe=False, status=400) + return JsonResponse(json.dumps(context), safe=False, status=500) @login_required +@api_view(["GET", "POST"]) def create_new_group(request): """Template to capture new group info, or status of save request.""" if request.POST: - context = group_helper.create_group(request) - if "error" in context: - return JsonResponse(context, status=400) - if "status" in context: + try: + context = group_helper.create_group(request) + if "error" in context: + return JsonResponse(context, status=400) return JsonResponse(context, status=200) - return JsonResponse({}) + except Exception as exception: + logger.error("Internal Server error occured while creating new group %s", str(exception)) + return HttpResponseServerError("Something went wrong while processing request") return render(request, "EnigmaOps/createNewGroup.html") @@ -249,6 +243,7 @@ def user_offboarding(request): @login_required +@api_view(["POST", "GET"]) def request_access(request): """Request access to a module. @@ -284,38 +279,28 @@ def request_access(request): } return JsonResponse(context, status=status) - context = get_request_access(request) - return render(request, "EnigmaOps/accessRequestForm.html", context) + try: + context = get_request_access(request) + return render(request, "EnigmaOps/accessRequestForm.html", context) + except Exception as exp: + logger.exception("Error while getting access request forms: %s", str(exp)) + + return HttpResponseServerError("Something went wrong while processing request") @login_required +@api_view(["GET"]) def new_group_access_request(request): """Group Request Access""" - if request.method == "POST": - return render_error_message( - request, - "POST for this endpoint is not supported", - "Invalid Request", - "Error request not found OR Invalid request type", - ) - - try: - User.objects.get(email=request.user.email) - except Exception as ex: - return render_error_message( - request, - f"Access user with email {request.user.email} not found. Error: {str(ex)}", - "Invalid Request", - "Please login again", - ) context = { - "groupName": dict(request.GET.lists())["groupName"][0] + "groupName": request.GET.get("groupName") } return render(request, "EnigmaOps/newAccessGroupRequest.html", context) @login_required +@api_view(["GET", "POST"]) def group_access(request): """Request access to a group. @@ -339,19 +324,27 @@ def group_access(request): } return JsonResponse(context, status=status) + if not "groupName" in request.GET: + messages.error(request, "GroupName is missing for the request") + return redirect("/") context = group_helper.get_group_access(request.GET, request.user) return render(request, "EnigmaOps/groupAccessRequestForm.html", context) @login_required +@api_view(["GET"]) @paginated_search def group_access_list(request): """lists the accesses for a group.""" try: group_name = request.GET.get('group_name') + if not group_name: + messages.error(request, "Group name is required for listing group details") + return redirect("/") group_detail = group_helper.get_group_access_list(request.user, group_name) if "error" in group_detail: - return TemplateResponse(request, "EnigmaOps/accessStatus.html"), group_detail + messages.error(request, group_access["msg"]) + return redirect("/") group_detail = group_helper.get_role_based_on_membership(group_detail) context = { @@ -401,12 +394,7 @@ def group_access_list(request): logger.exception( "Error in Group Access List, Error: %s", str(ex) ) - json_response = {} - json_response["error"] = { - "error_msg": INVALID_REQUEST_MESSAGE, - "msg": INVALID_REQUEST_MESSAGE, - } - return TemplateResponse(request, "EnigmaOps/accessStatus.html"), json_response + return HttpResponseServerError("Something went wrong while processing request") @login_required @@ -435,49 +423,37 @@ def update_group_owners(request, group_name): return JsonResponse(json_response, status=400) +@api_view(["GET"]) @login_required @paginated_search def group_dashboard(request): """ view group dashboard """ - if request.method == "POST": - return render_error_message( - request, - "POST for groupHistory not supported", - "Invalid Request", - "Error request not found OR Invalid request type", - ) - try: access_user = request.user.user - except Exception as exc: - return render_error_message( - request, - f"Access user with email {request.user.email} not found. Error: {str(exc)}", - "Invalid Request", - "Please login again", - ) - - selected_role = request.GET.getlist("role") - selected_status = request.GET.getlist("status") + selected_role = request.GET.getlist("role") + selected_status = request.GET.getlist("status") - context = { - "search_data_key": "dataList", - "search_rows": ["name"], - "dataList": access_user.get_groups_history(), - "statusFilter": { - "selected": selected_status, - "notSelected": group_helper.get_group_status_list(selected_status), - }, - "roleFilter": { - "selected": selected_role, - "notSelected": group_helper.get_group_member_role_list(selected_role) - }, - "search_value": request.GET.get("search"), - "filter_rows": ["role", "status"] - } - return TemplateResponse( - request, - "EnigmaOps/showGroupHistory.html"), context + context = { + "search_data_key": "dataList", + "search_rows": ["name"], + "dataList": access_user.get_groups_history(), + "statusFilter": { + "selected": selected_status, + "notSelected": group_helper.get_group_status_list(selected_status), + }, + "roleFilter": { + "selected": selected_role, + "notSelected": group_helper.get_group_member_role_list(selected_role) + }, + "search_value": request.GET.get("search"), + "filter_rows": ["role", "status"] + } + return TemplateResponse( + request, + "EnigmaOps/showGroupHistory.html"), context + except Exception as e: + logger.exception("Error while listing all groups: %s", str(e)) + return HttpResponseServerError("Something went wrong while processing request") def approve_new_group(request, group_id): @@ -494,6 +470,7 @@ def approve_new_group(request, group_id): @login_required +@api_view(["GET", "POST"]) def add_user_to_group(request, group_name): """Add one or more users to a group. diff --git a/enigma_automation/settings.py b/enigma_automation/settings.py index 7012f1be..5e2e3203 100644 --- a/enigma_automation/settings.py +++ b/enigma_automation/settings.py @@ -153,6 +153,8 @@ def cid_generator(): }, ] +MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' + WSGI_APPLICATION = "enigma_automation.wsgi.application" diff --git a/templates/EnigmaOps/dashboard.html b/templates/EnigmaOps/dashboard.html index d3883cca..38987305 100644 --- a/templates/EnigmaOps/dashboard.html +++ b/templates/EnigmaOps/dashboard.html @@ -7,6 +7,30 @@ {% block content_body %}
+ {% for message in messages %} +
+
+
+ +
+
+

{{message}}

+
+
+
+ +
+
+
+
+ {% endfor %}

Profile Overview

diff --git a/templates/EnigmaOps/groupAccessRequestForm.html b/templates/EnigmaOps/groupAccessRequestForm.html index 573f07c1..3d3c4573 100644 --- a/templates/EnigmaOps/groupAccessRequestForm.html +++ b/templates/EnigmaOps/groupAccessRequestForm.html @@ -1,4 +1,5 @@ {% extends 'global_layout.html' %} +{% load reusable_components %} {% load static %} {% block head_body %} @@ -52,11 +53,7 @@ {% block content_body %}
- - - {% if genericForm %} - - {% if accesses|length > 1 %} + {% if accesses|length > 1 %}
@@ -101,30 +98,31 @@

Module details

{% endfor %}
- {% else %} - {% for each_access in accesses %} -
-
-

{{ each_access.formDesc }} Module Details

-
+ {% elif accesses|length == 0 %} + {% get_error_panel_with_message "No modules matched" "Please select proper modules to raise access" %} + {% else %} + {% for each_access in accesses %} +
+
+

{{ each_access.formDesc }} Module Details

-
- {% csrf_token %} - {% with each_access.accessRequestPath as includePath %} - {% include includePath %} - {% endwith %} -
- -
- -
- - +
+ + {% csrf_token %} + {% with each_access.accessRequestPath as includePath %} + {% include includePath %} + {% endwith %} +
+ +
+
- - - {% endfor %} - {% endif %} + + +
+ + + {% endfor %} {% endif %}