refactor: replacing access type with access tag (#73)

removing unused parameters, using the new method for access grant.
moving the approval type check and status change code to models
adding the call to the accept_request in the accept access request function

Author: Kartikkumar-Shetty

Access/accessrequest_helper.py

@@ -16,8 +16,9 @@
     User,
     GroupV2,
     AccessV2,
+    ApprovalType,
 )
-from Access.background_task_manager import background_task
+from Access.background_task_manager import background_task, accept_request
 from . import helpers as helper
 
 logger = logging.getLogger(__name__)
@@ -548,10 +549,10 @@ def validate_access_labels(access_labels_json, access_type):
     return access_labels
 
 
-def _get_approver_permissions(access_type, access_label=None):
+def _get_approver_permissions(access_tag, access_label=None):
     json_response = {}
 
-    access_module = helper.get_available_access_module_from_tag(access_type)
+    access_module = helper.get_available_access_module_from_tag(access_tag)
     approver_permissions = []
     approver_permissions = access_module.fetch_approver_permissions(access_label)
 
@@ -573,7 +574,7 @@ def is_request_valid(request_id, access_mapping):
     return True
 
 
-def accept_user_access_requests(request, access_type, request_id):
+def accept_user_access_requests(auth_user, request_id):
     json_response = {}
     access_mapping = UserAccessMapping.get_access_request(request_id)
     if not is_request_valid(request_id, access_mapping):
@@ -583,54 +584,53 @@ def accept_user_access_requests(request, access_type, request_id):
         return json_response
 
     requester = access_mapping.user_identity.user.email
-    if request.user.username == requester:
+    if auth_user.username == requester:
         json_response["error"] = USER_REQUEST_PERMISSION_DENIED_ERR_MSG
         return json_response
 
     access_label = access_mapping.access.access_label
 
     try:
-        permissions = _get_approver_permissions(access_type, access_label)
+        permissions = _get_approver_permissions(access_mapping.access.access_tag, access_label)
         approver_permissions = permissions["approver_permissions"]
         if not helper.check_user_permissions(
-            request.user, list(approver_permissions.values())
+            auth_user, list(approver_permissions.values())
         ):
             logger.debug(USER_REQUEST_PERMISSION_DENIED_ERR_MSG)
             json_response["error"] = USER_REQUEST_PERMISSION_DENIED_ERR_MSG
             return json_response
 
         is_primary_approver = (
             access_mapping.is_pending()
-            and request.user.user.has_permission(approver_permissions["1"])
+            and auth_user.user.has_permission(approver_permissions["1"])
         )
         is_secondary_approver = (
             access_mapping.is_secondary_pending()
-            and request.user.user.has_permission(approver_permissions["2"])
+            and auth_user.user.has_permission(approver_permissions["2"])
         )
 
         if not (is_primary_approver or is_secondary_approver):
             logger.debug(USER_REQUEST_PERMISSION_DENIED_ERR_MSG)
             json_response["error"] = USER_REQUEST_PERMISSION_DENIED_ERR_MSG
             return json_response
         if is_primary_approver and "2" in approver_permissions:
-            access_mapping.approver_1 = request.user.user
+            access_mapping.approver_1 = auth_user.user
             access_mapping.update_access_status("SecondaryPending")
             json_response["msg"] = USER_REQUEST_SECONDARY_PENDING_MSG.format(
-                request_id=request_id, approved_by=request.user.username
+                request_id=request_id, approved_by=auth_user.username
             )
             logger.debug(
                 USER_REQUEST_SECONDARY_PENDING_MSG.format(
-                    request_id=request_id, approved_by=request.user.username
+                    request_id=request_id, approved_by=auth_user.username
                 )
             )
         else:
             json_response = run_accept_request_task(
                 is_primary_approver,
                 access_mapping,
-                request,
-                request_id,
-                access_type,
-                access_label,
+                auth_user=auth_user,
+                request_id=request_id,
+                access_label=access_label,
             )
     except Exception as e:
         return process_error_response(e)
@@ -639,24 +639,16 @@ def accept_user_access_requests(request, access_type, request_id):
 
 
 def run_accept_request_task(
-    is_primary_approver, access_mapping, request, request_id, access_type, access_label
+    is_primary_approver, access_mapping, auth_user, request_id, access_label
 ):
     json_response = {}
     json_response["status"] = []
-    if is_primary_approver:
-        access_mapping.approver_1 = request.user.user
-    else:
-        access_mapping.approver_2 = request.user.user
+    approval_type = ApprovalType.Primary if is_primary_approver else ApprovalType.Secondary
     json_response["msg"] = REQUEST_PROCESS_MSG.format(request_id=request_id)
 
     with transaction.atomic():
         try:
-            access_mapping.update_access_status("Processing")
-
-            background_task(
-                "run_accept_request",
-                json.dumps({"request_id": request_id, "access_type": access_type}),
-            )
+            accept_request(user_access_mapping=access_mapping, approval_type=approval_type, approver = auth_user.user)
         except Exception as e:
             logger.exception(e)
             raise Exception(
@@ -725,7 +717,7 @@ def decline_individual_access(request, access_type, request_id, reason):
     return json_response
 
 
-def accept_group_access(request, request_id):
+def accept_group_access(auth_user, request_id):
     json_response = {}
 
     group_mapping = GroupAccessMapping.get_by_request_id(request_id=request_id)
@@ -742,7 +734,7 @@ def accept_group_access(request, request_id):
         approver_permissions = permissions["approver_permissions"]
 
         if not helper.check_user_permissions(
-            request.user, list(approver_permissions.values())
+            auth_user, list(approver_permissions.values())
         ):
             logger.debug(USER_REQUEST_PERMISSION_DENIED_ERR_MSG)
             return create_error_response(
@@ -752,17 +744,17 @@ def accept_group_access(request, request_id):
         if not (group_mapping.is_pending() or group_mapping.is_secondary_pending()):
             logger.warning(
                 ALREADY_PROCESSED_REQUEST_MSG.format(
-                    request_id=request_id, user=request.user.username
+                    request_id=request_id, user=auth_user.username
                 )
             )
             return create_error_response(
                 error_msg=USER_REQUEST_IN_PROCESS_ERR_MSG.format(request_id=request_id)
             )
-        elif group_mapping.is_self_approval(approver=request.user.user):
+        elif group_mapping.is_self_approval(approver=auth_user.user):
             return create_error_response(error_msg=SELF_APPROVAL_ERROR_MSG)
         else:
             is_primary_approver, is_secondary_approver = is_valid_approver(
-                request=request,
+                auth_user=auth_user,
                 group_mapping=group_mapping,
                 approver_permissions=approver_permissions,
             )
@@ -772,21 +764,21 @@ def accept_group_access(request, request_id):
                     error_msg=USER_REQUEST_PERMISSION_DENIED_ERR_MSG
                 )
             if is_primary_approver and "2" in approver_permissions:
-                group_mapping.set_primary_approver(request.user.user)
+                group_mapping.set_primary_approver(auth_user.user)
                 json_response["msg"] = USER_REQUEST_SECONDARY_PENDING_MSG.format(
-                    request_id=request_id, approved_by=request.user.username
+                    request_id=request_id, approved_by=auth_user.username
                 )
                 group_mapping.update_access_status(current_status="SecondaryPending")
                 logger.debug(
                     USER_REQUEST_SECONDARY_PENDING_MSG.format(
-                        request_id=request_id, approved_by=request.user.username
+                        request_id=request_id, approved_by=auth_user.username
                     )
                 )
             else:
                 if is_primary_approver:
-                    group_mapping.set_primary_approver(request.user.user)
+                    group_mapping.set_primary_approver(auth_user.user)
                 else:
-                    group_mapping.set_secondary_approver(request.user.user)
+                    group_mapping.set_secondary_approver(auth_user.user)
                 json_response["msg"] = REQUEST_ACCESS_AUTO_APPROVED_MSG["title"].format(
                     request_id=request_id
                 )
@@ -799,7 +791,7 @@ def accept_group_access(request, request_id):
                 execute_group_access(userMappingsList)
                 logger.debug(
                     APPROVAL_PROCESS_STARTED_MSG.format(
-                        request_id=request_id, approver=request.user.username
+                        request_id=request_id, approver=auth_user.username
                     )
                 )
             return json_response
@@ -889,14 +881,14 @@ def create_error_response(error_msg):
     return json_response
 
 
-def is_valid_approver(request, group_mapping, approver_permissions):
+def is_valid_approver(auth_user, group_mapping, approver_permissions):
     is_primary_approver = (
         group_mapping.is_pending()
-        and request.user.user.has_permission(approver_permissions["1"])
+        and auth_user.user.has_permission(approver_permissions["1"])
     )
     is_secondary_approver = (
         group_mapping.is_secondary_pending()
-        and request.user.user.has_permission(approver_permissions["2"])
+        and auth_user.user.has_permission(approver_permissions["2"])
     )
     return is_primary_approver, is_secondary_approver
 

Access/background_task_manager.py

@@ -299,17 +299,12 @@ def run_accept_request(data):
     return {"status": False}
 
 
-def accept_request(user_access_mapping, approval_type="", approver=None):
+def accept_request(user_access_mapping, approval_type, approver):
     result = None
+    if approval_type != ApprovalType.Primary and approval_type != ApprovalType.Secondary:
+        raise Exception("Invalid Approval Type")
 
-    if approval_type == ApprovalType.Primary:
-        user_access_mapping.approver_1 = approver
-    elif approval_type == ApprovalType.Secondary:
-        user_access_mapping.approver_2 = approver
-    elif user_access_mapping.approver_1 or user_access_mapping.approver_2:
-        raise Exception("Request Not approved")
-
-    user_access_mapping.processing()
+    user_access_mapping.processing(approval_type = approval_type, approver=approver)
     try:
         result = run_access_grant.delay(user_access_mapping.request_id)
     except Exception:
@@ -319,7 +314,6 @@ def accept_request(user_access_mapping, approval_type="", approver=None):
         return True
     return False
 
-
 def revoke_request(user_access_mapping, revoker=None):
     result = None
     # change the status to revoke processing

Access/group_helper.py

@@ -172,7 +172,7 @@ def get_generic_access(group_mapping):
     return access_details
 
 
-def get_group_access_list(request, group_name):
+def get_group_access_list(auth_user, group_name):
     context = {}
     group = GroupV2.get_active_group_by_name(group_name)
     if not group:
@@ -186,7 +186,7 @@ def get_group_access_list(request, group_name):
         return context
 
     group_members = group.get_all_members().filter(status="Approved")
-    auth_user = request.user
+    auth_user = auth_user
 
     if not auth_user.user.is_allowed_admin_actions_on_group(group):
         logger.debug("Permission denied, requester is non owner")
@@ -304,7 +304,7 @@ def check_user_is_group_owner(user_name, group):
         return False
 
 
-def approve_new_group_request(request, group_id):
+def approve_new_group_request(auth_user, group_id):
     try:
         group = GroupV2.get_pending_group(group_id=group_id)
     except Exception as e:
@@ -316,7 +316,7 @@ def approve_new_group_request(request, group_id):
         context["error"] = REQUEST_NOT_FOUND_ERROR
         return context
     try:
-        if group.is_self_approval(approver=request.user.user):
+        if group.is_self_approval(approver=auth_user.user):
             context = {}
             context["error"] = SELF_APPROVAL_ERROR
             return context
@@ -325,8 +325,8 @@ def approve_new_group_request(request, group_id):
             context["msg"] = REQUEST_PROCESSING.format(requestId=group_id)
 
             with transaction.atomic():
-                group.approve(approved_by=request.user.user)
-                group.approve_all_pending_users(approved_by=request.user.user)
+                group.approve(approved_by=auth_user.user)
+                group.approve_all_pending_users(approved_by=auth_user.user)
             initial_members = group.get_all_members()
             initial_member_names = [user.user.name for user in initial_members]
             try:
@@ -344,7 +344,7 @@ def approve_new_group_request(request, group_id):
                 "Approved group creation for - "
                 + group_id
                 + " - Approver="
-                + request.user.username
+                + auth_user.username
             )
             if initial_members:
                 logger.debug(
@@ -502,7 +502,7 @@ def is_user_in_group(user_email, group_members_email):
     return user_email in group_members_email
 
 
-def accept_member(request, requestId, shouldRender=True):
+def accept_member(auth_user, requestId, shouldRender=True):
     try:
         membership = MembershipV2.get_membership(membership_id=requestId)
     except Exception as e:
@@ -514,22 +514,22 @@ def accept_member(request, requestId, shouldRender=True):
         if not membership.is_pending():
             logger.warning(
                 "An Already Approved/Declined/Processing Request was accessed by - "
-                + request.user.username
+                + auth_user.username
             )
             context = {}
             context["error"] = REQUEST_PROCESSED_BY.format(
                 requestId=requestId, user=membership.approver.user.username
             )
             return context
-        elif membership.is_self_approval(approver=request.user.user):
+        elif membership.is_self_approval(approver=auth_user.user):
             context = {}
             context["error"] = SELF_APPROVAL_ERROR
             return context
         else:
             context = {}
             context["msg"] = REQUEST_PROCESSING.format(requestId=requestId)
             with transaction.atomic():
-                membership.approve(request.user.user)
+                membership.approve(auth_user.user)
                 group = membership.group
                 user = membership.user
                 user_mappings_list = views_helper.generate_user_mappings(
@@ -545,7 +545,7 @@ def accept_member(request, requestId, shouldRender=True):
                 "Process has been started for the Approval of request - "
                 + requestId
                 + " - Approver="
-                + request.user.username
+                + auth_user.username
             )
             return context
     except Exception as e:

Access/models.py

@@ -843,7 +843,13 @@ def revoking(self, revoker):
         self.status = "ProcessingRevoke"
         self.save()        
 
-    def processing(self):
+    def processing(self, approval_type, approver):
+        if approval_type == ApprovalType.Primary:
+            self.approver_1 = approver
+        elif approval_type == ApprovalType.Secondary: 
+            self.approver_2 = approver
+        else:
+            raise Exception("Invalid ApprovalType")
         self.status = "Processing"
         self.save()
 

Access/userlist_helper.py

@@ -1,7 +1,7 @@
 import json
 from Access import helpers
 from Access.background_task_manager import background_task, accept_request, revoke_request
-from Access.models import MembershipV2, User
+from Access.models import User, ApprovalType
 import logging
 from . import helpers as helper
 from django.db import transaction
@@ -151,8 +151,14 @@ def __change_identity_and_transfer_access_mapping(
     existing_user_identity.decline_all_non_approved_access_mappings("Identity Updated")            
 
     for mapping in new_user_access_mapping:
-        if mapping.is_approved():
-            accept_request(user_access_mapping = mapping)
+        if mapping.is_approved() or mapping.is_grantfailed():
+            if mapping.approver_2:
+                accept_request(user_access_mapping = mapping, approval_type=ApprovalType.Secondary, approver = mapping.approver_2)
+            elif mapping.approver_1:
+                accept_request(user_access_mapping=mapping, approval_type=ApprovalType.Primary, approver = mapping.approver_1)
+            else:
+                logger.fatal("migration failed for request_id:%s mapping is approved but approvers are missing: %s", 
+                mapping.request_id)
 
 def getallUserList(request):
     try: