diff --git a/backend/api/campaigns/resources.py b/backend/api/campaigns/resources.py index 466090ce00..08f3965912 100644 --- a/backend/api/campaigns/resources.py +++ b/backend/api/campaigns/resources.py @@ -1,6 +1,7 @@ from flask_restful import Resource, request, current_app from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.dtos.campaign_dto import CampaignDTO, NewCampaignDTO from backend.services.campaign_service import CampaignService from backend.services.organisation_service import OrganisationService @@ -115,15 +116,11 @@ def patch(self, campaign_id): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"CampaignsRestAPI PATCH: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") try: campaign_dto = CampaignDTO(request.get_json()) @@ -179,15 +176,11 @@ def delete(self, campaign_id): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"CampaignsRestAPI DELETE: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") campaign = CampaignService.get_campaign(campaign_id) CampaignService.delete_campaign(campaign.id) @@ -268,15 +261,11 @@ def post(self): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"CampaignsAllAPI POST: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") try: campaign_dto = NewCampaignDTO(request.get_json()) diff --git a/backend/api/comments/resources.py b/backend/api/comments/resources.py index 0e031360ef..2c6f0e2cbb 100644 --- a/backend/api/comments/resources.py +++ b/backend/api/comments/resources.py @@ -1,6 +1,7 @@ from flask_restful import Resource, request, current_app from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.dtos.message_dto import ChatMessageDTO from backend.models.dtos.mapping_dto import TaskCommentDTO from backend.services.messaging.chat_service import ChatService @@ -53,7 +54,7 @@ def post(self, project_id): """ authenticated_user_id = token_auth.current_user() if UserService.is_user_blocked(authenticated_user_id): - return {"Error": "User is on read only mode", "SubCode": "ReadOnly"}, 403 + raise Forbidden(sub_code="USER_BLOCKED") try: chat_dto = ChatMessageDTO(request.get_json()) @@ -67,13 +68,10 @@ def post(self, project_id): "SubCode": "InvalidData", }, 400 - try: - project_messages = ChatService.post_message( - chat_dto, project_id, authenticated_user_id - ) - return project_messages.to_primitive(), 201 - except ValueError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + project_messages = ChatService.post_message( + chat_dto, project_id, authenticated_user_id + ) + return project_messages.to_primitive(), 201 def get(self, project_id): """ @@ -155,13 +153,10 @@ def delete(self, project_id, comment_id): description: Internal Server Error """ authenticated_user_id = token_auth.current_user() - try: - ChatService.delete_project_chat_by_id( - project_id, comment_id, authenticated_user_id - ) - return {"Success": "Comment deleted"}, 200 - except ValueError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + ChatService.delete_project_chat_by_id( + project_id, comment_id, authenticated_user_id + ) + return {"Success": "Comment deleted"}, 200 class CommentsTasksRestAPI(Resource): @@ -222,7 +217,7 @@ def post(self, project_id, task_id): """ authenticated_user_id = token_auth.current_user() if UserService.is_user_blocked(authenticated_user_id): - return {"Error": "User is on read only mode", "SubCode": "ReadOnly"}, 403 + raise Forbidden(sub_code="USER_BLOCKED") try: task_comment = TaskCommentDTO(request.get_json()) @@ -234,11 +229,8 @@ def post(self, project_id, task_id): current_app.logger.error(f"Error validating request: {str(e)}") return {"Error": "Unable to add comment", "SubCode": "InvalidData"}, 400 - try: - task = MappingService.add_task_comment(task_comment) - return task.to_primitive(), 201 - except MappingServiceError: - return {"Error": "Task update failed"}, 403 + task = MappingService.add_task_comment(task_comment) + return task.to_primitive(), 201 def get(self, project_id, task_id): """ diff --git a/backend/api/interests/resources.py b/backend/api/interests/resources.py index d1c0a0e1c6..dbe39ce238 100644 --- a/backend/api/interests/resources.py +++ b/backend/api/interests/resources.py @@ -1,6 +1,7 @@ from flask_restful import Resource, current_app, request from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.dtos.interests_dto import InterestDTO from backend.services.interests_service import InterestService from backend.services.organisation_service import OrganisationService @@ -9,6 +10,7 @@ from sqlalchemy.exc import IntegrityError INTEREST_NOT_FOUND = "Interest Not Found" +# FLAGGED FOR PERMISSIONS REVIEW class InterestsAllAPI(Resource): @@ -49,15 +51,11 @@ def post(self): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"InterestsAllAPI POST: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") try: interest_dto = InterestDTO(request.get_json()) @@ -133,15 +131,12 @@ def get(self, interest_id): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"InterestsRestAPI GET: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + # FLAGGED + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") interest = InterestService.get(interest_id) return interest.to_primitive(), 200 @@ -191,15 +186,11 @@ def patch(self, interest_id): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"InterestsRestAPI PATCH: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") try: interest_dto = InterestDTO(request.get_json()) @@ -245,15 +236,11 @@ def delete(self, interest_id): 500: description: Internal Server Error """ - try: - orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( - token_auth.current_user() - ) - if len(orgs_dto.organisations) < 1: - raise ValueError("User not a Org Manager") - except ValueError as e: - error_msg = f"InterestsRestAPI DELETE: {str(e)}" - return {"Error": error_msg, "SubCode": "UserNotPermitted"}, 403 + orgs_dto = OrganisationService.get_organisations_managed_by_user_as_dto( + token_auth.current_user() + ) + if len(orgs_dto.organisations) < 1: + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") InterestService.delete(interest_id) return {"Success": "Interest deleted"}, 200 diff --git a/backend/api/notifications/resources.py b/backend/api/notifications/resources.py index 6575fad152..6618aeb749 100644 --- a/backend/api/notifications/resources.py +++ b/backend/api/notifications/resources.py @@ -1,8 +1,5 @@ from flask_restful import Resource, request -from backend.services.messaging.message_service import ( - MessageService, - MessageServiceError, -) +from backend.services.messaging.message_service import MessageService from backend.services.notification_service import NotificationService from backend.services.users.authentication_service import token_auth, tm @@ -41,13 +38,10 @@ def get(self, message_id): 500: description: Internal Server Error """ - try: - user_message = MessageService.get_message_as_dto( - message_id, token_auth.current_user() - ) - return user_message.to_primitive(), 200 - except MessageServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + user_message = MessageService.get_message_as_dto( + message_id, token_auth.current_user() + ) + return user_message.to_primitive(), 200 @tm.pm_only(False) @token_auth.login_required @@ -82,11 +76,8 @@ def delete(self, message_id): 500: description: Internal Server Error """ - try: - MessageService.delete_message(message_id, token_auth.current_user()) - return {"Success": "Message deleted"}, 200 - except MessageServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + MessageService.delete_message(message_id, token_auth.current_user()) + return {"Success": "Message deleted"}, 200 class NotificationsAllAPI(Resource): diff --git a/backend/api/organisations/campaigns.py b/backend/api/organisations/campaigns.py index 9abaf40440..5a83007fc6 100644 --- a/backend/api/organisations/campaigns.py +++ b/backend/api/organisations/campaigns.py @@ -1,5 +1,6 @@ from flask_restful import Resource +from backend.exceptions import Forbidden from backend.services.campaign_service import CampaignService from backend.services.organisation_service import OrganisationService from backend.services.users.authentication_service import token_auth @@ -63,10 +64,7 @@ def post(self, organisation_id, campaign_id): ) return {"Success": message}, 200 else: - return { - "Error": "User is not a manager of the organisation", - "SubCode": "UserNotPermitted", - }, 403 + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") def get(self, organisation_id): """ @@ -149,7 +147,4 @@ def delete(self, organisation_id, campaign_id): 200, ) else: - return { - "Error": "User is not a manager of the organisation", - "SubCode": "UserNotPermitted", - }, 403 + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") diff --git a/backend/api/organisations/resources.py b/backend/api/organisations/resources.py index 86e2badb0b..c198c32aa8 100644 --- a/backend/api/organisations/resources.py +++ b/backend/api/organisations/resources.py @@ -2,11 +2,12 @@ from flask_restful import Resource, request, current_app from schematics.exceptions import DataError +from backend.exceptions import Forbidden, Unauthorized from backend.models.dtos.organisation_dto import ( NewOrganisationDTO, UpdateOrganisationDTO, ) -from backend.models.postgis.user import User +from backend.models.postgis.user import User, UserRole from backend.services.organisation_service import ( OrganisationService, OrganisationServiceError, @@ -121,11 +122,8 @@ def post(self): description: Internal Server Error """ request_user = User.get_by_id(token_auth.current_user()) - if request_user.role != 1: - return { - "Error": "Only admin users can create organisations.", - "SubCode": "OnlyAdminAccess", - }, 403 + if request_user.role != UserRole.ADMIN.value: + raise Forbidden(sub_code="USER_NOT_ADMIN") try: organisation_dto = NewOrganisationDTO(request.get_json()) @@ -179,10 +177,7 @@ def delete(self, organisation_id): if not OrganisationService.can_user_manage_organisation( organisation_id, token_auth.current_user() ): - return { - "Error": "User is not an admin for the org", - "SubCode": "UserNotOrgAdmin", - }, 403 + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") try: OrganisationService.delete_organisation(organisation_id) return {"Success": "Organisation deleted"}, 200 @@ -190,7 +185,7 @@ def delete(self, organisation_id): return { "Error": "Organisation has some projects", "SubCode": "OrgHasProjects", - }, 403 + }, 409 @token_auth.login_required(optional=True) def get(self, organisation_id): @@ -303,15 +298,13 @@ def patch(self, organisation_id): if not OrganisationService.can_user_manage_organisation( organisation_id, token_auth.current_user() ): - return { - "Error": "User is not an admin for the org", - "SubCode": "UserNotOrgAdmin", - }, 403 + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") try: organisation_dto = UpdateOrganisationDTO(request.get_json()) organisation_dto.organisation_id = organisation_id + # Don't update organisation type and subscription_tier if request user is not an admin - if User.get_by_id(token_auth.current_user()).role != 1: + if User.get_by_id(token_auth.current_user()).role != UserRole.ADMIN.value: org = OrganisationService.get_organisation_by_id(organisation_id) organisation_dto.type = OrganisationType(org.type).name organisation_dto.subscription_tier = org.subscription_tier @@ -324,7 +317,7 @@ def patch(self, organisation_id): OrganisationService.update_organisation(organisation_dto) return {"Status": "Updated"}, 200 except OrganisationServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 402 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class OrganisationsStatsAPI(Resource): @@ -415,13 +408,7 @@ def get(self): manager_user_id = None if manager_user_id is not None and not authenticated_user_id: - return ( - { - "Error": "Unauthorized - Filter by manager_user_id is not allowed to unauthenticated requests", - "SubCode": "LoginToFilterManager", - }, - 403, - ) + raise Unauthorized(sub_code="UNAUTHORIZED_MANAGER_FILTER") # Validate abbreviated. omit_managers = bool(strtobool(request.args.get("omitManagerList", "false"))) diff --git a/backend/api/projects/actions.py b/backend/api/projects/actions.py index 23bcb73e6f..9fc3803d37 100644 --- a/backend/api/projects/actions.py +++ b/backend/api/projects/actions.py @@ -3,13 +3,11 @@ from flask_restful import Resource, request, current_app from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.dtos.message_dto import MessageDTO from backend.models.dtos.grid_dto import GridDTO from backend.services.project_service import ProjectService -from backend.services.project_admin_service import ( - ProjectAdminService, - ProjectAdminServiceError, -) +from backend.services.project_admin_service import ProjectAdminService from backend.services.grid.grid_service import GridService from backend.services.messaging.message_service import MessageService from backend.services.users.authentication_service import token_auth, tm @@ -65,14 +63,12 @@ def post(self, project_id): username = request.get_json()["username"] except Exception: return {"Error": "Username not provided", "SubCode": "InvalidData"}, 400 - try: - authenticated_user_id = token_auth.current_user() - ProjectAdminService.transfer_project_to( - project_id, authenticated_user_id, username - ) - return {"Success": "Project Transferred"}, 200 - except (ValueError, ProjectAdminServiceError) as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + + authenticated_user_id = token_auth.current_user() + ProjectAdminService.transfer_project_to( + project_id, authenticated_user_id, username + ) + return {"Success": "Project Transferred"}, 200 class ProjectsActionsMessageContributorsAPI(Resource): @@ -128,7 +124,9 @@ def post(self, project_id): message_dto.from_user_id = authenticated_user_id message_dto.validate() except DataError as e: - current_app.logger.error(f"Error validating request: {str(e)}") + current_app.logger.error( + f"Error validating request: {str(e)}" + ) # FLAGGED FOR LOG LEVEL return { "Error": "Unable to send message to mappers", "SubCode": "InvalidData", @@ -137,10 +135,11 @@ def post(self, project_id): if not ProjectAdminService.is_user_action_permitted_on_project( authenticated_user_id, project_id ): - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) threading.Thread( target=MessageService.send_message_to_all_contributors, args=(project_id, message_dto), @@ -183,23 +182,24 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) try: ProjectService.set_project_as_featured(project_id) return {"Success": True}, 200 except ValueError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return { + "Error": str(e).split("-")[1], + "SubCode": str(e).split("-")[0], + }, 409 class ProjectsActionsUnFeatureAPI(Resource): @@ -237,23 +237,24 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) try: ProjectService.unset_project_as_featured(project_id) return {"Success": True}, 200 except ValueError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return { + "Error": str(e).split("-")[1], + "SubCode": str(e).split("-")[0], + }, 403 class ProjectsActionsSetInterestsAPI(Resource): @@ -301,17 +302,15 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) data = request.get_json() project_interests = InterestService.create_or_update_project_interests( diff --git a/backend/api/projects/campaigns.py b/backend/api/projects/campaigns.py index 6d29087970..164d2a487a 100644 --- a/backend/api/projects/campaigns.py +++ b/backend/api/projects/campaigns.py @@ -1,6 +1,7 @@ from flask_restful import Resource, current_app from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.dtos.campaign_dto import CampaignProjectDTO from backend.services.campaign_service import CampaignService from backend.services.project_admin_service import ProjectAdminService @@ -52,10 +53,11 @@ def post(self, project_id, campaign_id): if not ProjectAdminService.is_user_action_permitted_on_project( authenticated_user_id, project_id ): - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) try: campaign_project_dto = CampaignProjectDTO() campaign_project_dto.campaign_id = campaign_id @@ -145,10 +147,11 @@ def delete(self, project_id, campaign_id): if not ProjectAdminService.is_user_action_permitted_on_project( authenticated_user_id, project_id ): - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) CampaignService.delete_project_campaign(project_id, campaign_id) return {"Success": "Campaigns Deleted"}, 200 diff --git a/backend/api/projects/resources.py b/backend/api/projects/resources.py index 3831ccb071..3abe82b346 100644 --- a/backend/api/projects/resources.py +++ b/backend/api/projects/resources.py @@ -1,9 +1,12 @@ import geojson import io +from distutils.util import strtobool + from flask import send_file from flask_restful import Resource, current_app, request from schematics.exceptions import DataError -from distutils.util import strtobool + +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.project_dto import ( DraftProjectDTO, ProjectDTO, @@ -15,11 +18,7 @@ ProjectSearchServiceError, BBoxTooBigError, ) -from backend.services.project_service import ( - ProjectService, - ProjectServiceError, - NotFound, -) +from backend.services.project_service import ProjectService from backend.services.users.user_service import UserService from backend.services.organisation_service import OrganisationService from backend.services.users.authentication_service import token_auth @@ -100,24 +99,15 @@ def get(self, project_id): abbreviated, ) - if project_dto: - project_dto = project_dto.to_primitive() - if as_file: - return send_file( - io.BytesIO(geojson.dumps(project_dto).encode("utf-8")), - mimetype="application/json", - as_attachment=True, - download_name=f"project_{str(project_id)}.json", - ) - - return project_dto, 200 - else: - return { - "Error": "User not permitted: Private Project", - "SubCode": "PrivateProject", - }, 403 - except ProjectServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + project_dto = project_dto.to_primitive() + if as_file: + return send_file( + io.BytesIO(geojson.dumps(project_dto).encode("utf-8")), + mimetype="application/json", + as_attachment=True, + download_name=f"project_{str(project_id)}.json", + ) + return project_dto, 200 finally: # this will try to unlock tasks that have been locked too long try: @@ -204,8 +194,6 @@ def post(self): draft_project_dto ) return {"projectId": draft_project_id}, 201 - except ProjectAdminServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 except (InvalidGeoJson, InvalidData) as e: return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 400 @@ -248,10 +236,11 @@ def head(self, project_id): token_auth.current_user(), project_id ) except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=token_auth.current_user(), + ) project_dto = ProjectAdminService.get_project_dto_for_admin(project_id) return project_dto.to_primitive(), 200 @@ -383,10 +372,11 @@ def patch(self, project_id): if not ProjectAdminService.is_user_action_permitted_on_project( authenticated_user_id, project_id ): - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) try: project_dto = ProjectDTO(request.get_json()) project_dto.project_id = project_id @@ -396,12 +386,15 @@ def patch(self, project_id): return {"Error": "Unable to update project", "SubCode": "InvalidData"}, 400 try: - ProjectAdminService.update_project(project_dto, authenticated_user_id) + ProjectAdminService.update_project(project_dto) return {"Status": "Updated"}, 200 except InvalidGeoJson as e: return {"Invalid GeoJson": str(e)}, 400 except ProjectAdminServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return { + "Error": str(e).split("-")[1], + "SubCode": str(e).split("-")[0], + }, 400 @token_auth.login_required def delete(self, project_id): @@ -437,23 +430,19 @@ def delete(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", user_id=authenticated_user_id + ) try: ProjectAdminService.delete_project(project_id, authenticated_user_id) return {"Success": "Project deleted"}, 200 except ProjectAdminServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class ProjectSearchBase(Resource): @@ -733,10 +722,9 @@ def get(self): authenticated_user_id ) if len(orgs_dto.organisations) < 1: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_ORG_MANAGER", user_id=authenticated_user_id + ) try: search_dto = ProjectSearchBBoxDTO() @@ -806,10 +794,9 @@ def get(self): authenticated_user_id ) if len(orgs_dto.organisations) < 1: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_ORG_MANAGER", user_id=authenticated_user_id + ) search_dto = self.setup_search_dto() admin_projects = ProjectAdminService.get_projects_for_admin( @@ -952,8 +939,6 @@ def get(self, project_id): ) return project_dto, 200 - except ProjectServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 finally: # this will try to unlock tasks that have been locked too long try: @@ -997,13 +982,15 @@ def get(self, project_id): 500: description: Internal Server Error """ + authenticated_user_id = token_auth.current_user() if not ProjectAdminService.is_user_action_permitted_on_project( - token_auth.current_user(), project_id + authenticated_user_id, project_id ): - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) project_dto = ProjectAdminService.get_project_dto_for_admin(project_id) return project_dto.to_primitive(), 200 @@ -1085,11 +1072,8 @@ def get(self, project_id): 500: description: Internal Server Error """ - try: - priority_areas = ProjectService.get_project_priority_areas(project_id) - return priority_areas, 200 - except ProjectServiceError: - return {"Error": "Unable to fetch project"}, 403 + priority_areas = ProjectService.get_project_priority_areas(project_id) + return priority_areas, 200 class ProjectsQueriesFeaturedAPI(Resource): diff --git a/backend/api/projects/teams.py b/backend/api/projects/teams.py index 3dbb12a337..fc88df55f1 100644 --- a/backend/api/projects/teams.py +++ b/backend/api/projects/teams.py @@ -1,7 +1,8 @@ from flask_restful import Resource, request, current_app from schematics.exceptions import DataError -from backend.services.team_service import TeamService, TeamServiceError +from backend.exceptions import Forbidden +from backend.services.team_service import TeamService from backend.services.project_admin_service import ProjectAdminService from backend.services.project_service import ProjectService from backend.services.users.authentication_service import token_auth @@ -32,7 +33,7 @@ def get(self, project_id): responses: 200: description: Teams listed successfully - 403: + 403: # TODO: Check if this is the correct error code description: Forbidden, if user is not authenticated 404: description: Not found @@ -91,11 +92,14 @@ def post(self, team_id, project_id): 500: description: Internal Server Error """ - if not TeamService.is_user_team_manager(team_id, token_auth.current_user()): - return { - "Error": "User is not an admin or a manager for the team", - "SubCode": "UserPermissionError", - }, 401 + authenticated_user = token_auth.current_user() + if not TeamService.is_user_team_manager(team_id, authenticated_user): + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + project_id=project_id, + team_id=team_id, + user_id=authenticated_user, + ) try: role = request.get_json(force=True)["role"] @@ -103,25 +107,25 @@ def post(self, team_id, project_id): current_app.logger.error(f"Error validating request: {str(e)}") return {"Error": str(e), "SubCode": "InvalidData"}, 400 - try: - if not ProjectAdminService.is_user_action_permitted_on_project( - token_auth.current_user, project_id - ): - raise ValueError() - TeamService.add_team_project(team_id, project_id, role) - return ( - { - "Success": "Team {} assigned to project {} with role {}".format( - team_id, project_id, role - ) - }, - 201, + if not ProjectAdminService.is_user_action_permitted_on_project( + token_auth.current_user, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + team_id=team_id, + user_id=authenticated_user, ) - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + + TeamService.add_team_project(team_id, project_id, role) + return ( + { + "Success": "Team {} assigned to project {} with role {}".format( + team_id, project_id, role + ) + }, + 201, + ) @token_auth.login_required def patch(self, team_id, project_id): @@ -162,9 +166,9 @@ def patch(self, team_id, project_id): 201: description: Team project assignment created 401: - description: Forbidden, if user is not a manager of the project + description: Unauthenticated user 403: - description: Forbidden, if user is not authenticated + description: Forbidden, if user is not a manager of the project 404: description: Not found 500: @@ -176,20 +180,13 @@ def patch(self, team_id, project_id): current_app.logger.error(f"Error validating request: {str(e)}") return {"Error": str(e), "SubCode": "InvalidData"}, 400 - try: - if not ProjectAdminService.is_user_action_permitted_on_project( - token_auth.current_user, project_id - ): - raise ValueError() - TeamService.change_team_role(team_id, project_id, role) - return {"Status": "Team role updated successfully."}, 200 - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 - except TeamServiceError as e: - return str(e), 402 + if not ProjectAdminService.is_user_action_permitted_on_project( + token_auth.current_user, project_id + ): + raise Forbidden(sub_code="USER_NOT_PROJECT_MANAGER") + + TeamService.change_team_role(team_id, project_id, role) + return {"Status": "Team role updated successfully."}, 200 @token_auth.login_required def delete(self, team_id, project_id): @@ -225,15 +222,15 @@ def delete(self, team_id, project_id): 500: description: Internal Server Error """ - try: - if not ProjectAdminService.is_user_action_permitted_on_project( - token_auth.current_user, project_id - ): - raise ValueError() - TeamService.delete_team_project(team_id, project_id) - return {"Success": True}, 200 - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + team_id=team_id, + user_id=authenticated_user_id, + ) + TeamService.delete_team_project(team_id, project_id) + return {"Success": True}, 200 diff --git a/backend/api/system/authentication.py b/backend/api/system/authentication.py index 58453ddad6..e88eba7aa0 100644 --- a/backend/api/system/authentication.py +++ b/backend/api/system/authentication.py @@ -161,4 +161,7 @@ def get(self): return {"Status": "OK"}, 200 except AuthServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return { + "Error": str(e).split("-")[1], + "SubCode": str(e).split("-")[0], + }, 400 diff --git a/backend/api/system/banner.py b/backend/api/system/banner.py index 272bc3e12d..2c8c982e5d 100644 --- a/backend/api/system/banner.py +++ b/backend/api/system/banner.py @@ -2,6 +2,7 @@ from flask_restful import Resource, request from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.postgis.banner import Banner from backend.models.dtos.banner_dto import BannerDTO from backend.models.postgis.statuses import UserRole @@ -83,10 +84,7 @@ def patch(self): authenticated_user_id = token_auth.current_user() authenticated_user = UserService.get_user_by_id(authenticated_user_id) if authenticated_user.role != UserRole.ADMIN.value: - return { - "Error": "Banner can only be updated by system admins", - "SubCode": "OnlyAdminAccess", - }, 403 + raise Forbidden(sub_code="USER_NOT_ADMIN", user_id=authenticated_user_id) banner_dto.message = Banner.to_html( banner_dto.message diff --git a/backend/api/tasks/actions.py b/backend/api/tasks/actions.py index 10878ecffc..16741a99d6 100644 --- a/backend/api/tasks/actions.py +++ b/backend/api/tasks/actions.py @@ -1,7 +1,7 @@ from flask_restful import Resource, current_app, request from schematics.exceptions import DataError -from backend.exceptions import NotFound +from backend.exceptions import Forbidden from backend.models.dtos.grid_dto import SplitTaskDTO from backend.models.postgis.utils import InvalidGeoJson from backend.services.grid.split_service import SplitService, SplitServiceError @@ -96,7 +96,7 @@ def post(self, project_id, task_id): task = MappingService.lock_task_for_mapping(lock_task_dto) return task.to_primitive(), 200 except MappingServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 except UserLicenseError: return { "Error": "User not accepted license terms", @@ -182,7 +182,7 @@ def post(self, project_id, task_id): task = MappingService.stop_mapping_task(stop_task) return task.to_primitive(), 200 except MappingServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class TasksActionsMappingUnlockAPI(Resource): @@ -261,16 +261,7 @@ def post(self, project_id, task_id): task = MappingService.unlock_task_after_mapping(mapped_task) return task.to_primitive(), 200 except MappingServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 - except NotFound as e: - return e.to_dict() - except Exception as e: - error_msg = f"Task Lock API - unhandled error: {str(e)}" - current_app.logger.critical(error_msg) - return { - "Error": "Task unlock failed", - "SubCode": "InternalServerError", - }, 500 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 finally: # Refresh mapper level after mapping UserService.check_and_update_mapper_level(authenticated_user_id) @@ -329,7 +320,7 @@ def post(self, project_id, task_id): ) return task.to_primitive(), 200 except MappingServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class TasksActionsValidationLockAPI(Resource): @@ -404,7 +395,7 @@ def post(self, project_id): tasks = ValidatorService.lock_tasks_for_validation(validator_dto) return tasks.to_primitive(), 200 except ValidatorServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 except UserLicenseError: return { "Error": "User not accepted license terms", @@ -482,7 +473,7 @@ def post(self, project_id): tasks = ValidatorService.stop_validating_tasks(validated_dto) return tasks.to_primitive(), 200 except ValidatorServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class TasksActionsValidationUnlockAPI(Resource): @@ -554,7 +545,7 @@ def post(self, project_id): tasks = ValidatorService.unlock_tasks_after_validation(validated_dto) return tasks.to_primitive(), 200 except ValidatorServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class TasksActionsMapAllAPI(Resource): @@ -590,17 +581,15 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) MappingService.map_all_tasks(project_id, authenticated_user_id) return {"Success": "All tasks mapped"}, 200 @@ -639,17 +628,15 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) ValidatorService.validate_all_tasks(project_id, authenticated_user_id) return {"Success": "All tasks validated"}, 200 @@ -688,17 +675,15 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) ValidatorService.invalidate_all_tasks(project_id, authenticated_user_id) return {"Success": "All tasks invalidated"}, 200 @@ -737,17 +722,15 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) MappingService.reset_all_badimagery(project_id, authenticated_user_id) return {"Success": "All bad imagery tasks marked ready for mapping"}, 200 @@ -786,18 +769,15 @@ def post(self, project_id): 500: description: Internal Server Error """ - try: - authenticated_user_id = token_auth.current_user() - authenticated_user_id = token_auth.current_user() - if not ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - raise ValueError() - except ValueError: - return { - "Error": "User is not a manager of the project", - "SubCode": "UserPermissionError", - }, 403 + authenticated_user_id = token_auth.current_user() + if not ProjectAdminService.is_user_action_permitted_on_project( + authenticated_user_id, project_id + ): + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=project_id, + user_id=authenticated_user_id, + ) ProjectAdminService.reset_all_tasks(project_id, authenticated_user_id) return {"Success": "All tasks reset"}, 200 @@ -869,9 +849,9 @@ def post(self, project_id, task_id): tasks = SplitService.split_task(split_task_dto) return tasks.to_primitive(), 200 except SplitServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 except InvalidGeoJson as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 400 class TasksActionsExtendAPI(Resource): @@ -946,7 +926,7 @@ def post(self, project_id): MappingService.extend_task_lock_time(extend_dto) return {"Success": "Successfully extended task expiry"}, 200 except MappingServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 409 class TasksActionsReverUserTaskstAPI(Resource): @@ -1017,8 +997,6 @@ def post(self, project_id): "Error": "Unable to revert tasks", "SubCode": "InvalidData", }, 400 - try: - ValidatorService.revert_user_tasks(revert_dto) - return {"Success": "Successfully reverted tasks"}, 200 - except ValidatorServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + + ValidatorService.revert_user_tasks(revert_dto) + return {"Success": "Successfully reverted tasks"}, 200 diff --git a/backend/api/tasks/resources.py b/backend/api/tasks/resources.py index 90a04905cd..1feaa6fa2b 100644 --- a/backend/api/tasks/resources.py +++ b/backend/api/tasks/resources.py @@ -5,13 +5,12 @@ from flask_restful import Resource, current_app, request from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.services.mapping_service import MappingService from backend.models.dtos.grid_dto import GridDTO - from backend.services.users.authentication_service import token_auth, tm from backend.services.users.user_service import UserService from backend.services.validator_service import ValidatorService - from backend.services.project_service import ProjectService, ProjectServiceError from backend.services.grid.grid_service import GridService from backend.models.postgis.statuses import UserRole @@ -96,28 +95,25 @@ def get(self, project_id): 500: description: Internal Server Error """ - try: - tasks = request.args.get("tasks") if request.args.get("tasks") else None - as_file = ( - strtobool(request.args.get("as_file")) - if request.args.get("as_file") - else True - ) + tasks = request.args.get("tasks") if request.args.get("tasks") else None + as_file = ( + strtobool(request.args.get("as_file")) + if request.args.get("as_file") + else True + ) - tasks_json = ProjectService.get_project_tasks(int(project_id), tasks) + tasks_json = ProjectService.get_project_tasks(int(project_id), tasks) - if as_file: - tasks_json = str(tasks_json).encode("utf-8") - return send_file( - io.BytesIO(tasks_json), - mimetype="application/json", - as_attachment=True, - download_name=f"{str(project_id)}-tasks.geojson", - ) + if as_file: + tasks_json = str(tasks_json).encode("utf-8") + return send_file( + io.BytesIO(tasks_json), + mimetype="application/json", + as_attachment=True, + download_name=f"{str(project_id)}-tasks.geojson", + ) - return tasks_json, 200 - except ProjectServiceError as e: - return {"Error": str(e)}, 403 + return tasks_json, 200 @token_auth.login_required def delete(self, project_id): @@ -167,10 +163,7 @@ def delete(self, project_id): user_id = token_auth.current_user() user = UserService.get_user_by_id(user_id) if user.role != UserRole.ADMIN.value: - return { - "Error": "This endpoint action is restricted to ADMIN users.", - "SubCode": "OnlyAdminAccess", - }, 403 + raise Forbidden(sub_code="USER_NOT_ADMIN", user_id=user_id) tasks_ids = request.get_json().get("tasks") if tasks_ids is None: diff --git a/backend/api/teams/actions.py b/backend/api/teams/actions.py index 27a5799afd..06e1b4edff 100644 --- a/backend/api/teams/actions.py +++ b/backend/api/teams/actions.py @@ -2,10 +2,10 @@ from schematics.exceptions import DataError import threading +from backend.exceptions import Forbidden from backend.models.dtos.message_dto import MessageDTO from backend.services.team_service import ( TeamService, - TeamJoinNotAllowed, TeamServiceError, ) from backend.services.users.authentication_service import token_auth, tm @@ -129,12 +129,10 @@ def patch(self, team_id): ) return {"Success": "True"}, 200 else: - return ( - { - "Error": "You don't have permissions to approve this join team request", - "SubCode": "ApproveJoinError", - }, - 403, + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + team_id=team_id, + user_id=authenticated_user_id, ) elif request_type == "invite-response": TeamService.accept_reject_invitation_request( @@ -199,12 +197,9 @@ def post(self, team_id): "SubCode": "InvalidData", }, 400 - try: - authenticated_user_id = token_auth.current_user() - TeamService.add_user_to_team(team_id, authenticated_user_id, username, role) - return {"Success": "User added to the team"}, 200 - except TeamJoinNotAllowed as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + authenticated_user_id = token_auth.current_user() + TeamService.add_user_to_team(team_id, authenticated_user_id, username, role) + return {"Success": "User added to the team"}, 200 class TeamsActionsLeaveAPI(Resource): @@ -261,14 +256,10 @@ def post(self, team_id): TeamService.leave_team(team_id, username) return {"Success": "User removed from the team"}, 200 else: - return ( - { - "Error": "You don't have permissions to remove {} from this team.".format( - username - ), - "SubCode": "RemoveUserError", - }, - 403, + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + team_id=team_id, + user_id=authenticated_user_id, ) @@ -329,7 +320,12 @@ def post(self, team_id): team_id, authenticated_user_id ) if not is_manager: - raise ValueError + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + team_id=team_id, + user_id=authenticated_user_id, + ) + message_dto.from_user_id = authenticated_user_id message_dto.validate() if not message_dto.message.strip() or not message_dto.subject.strip(): @@ -342,18 +338,10 @@ def post(self, team_id): "Error": "Request payload did not match validation", "SubCode": "InvalidData", }, 400 - except ValueError: - return { - "Error": "Unauthorised to send message to team members", - "SubCode": "UserNotPermitted", - }, 403 - try: - threading.Thread( - target=TeamService.send_message_to_all_team_members, - args=(team_id, team.name, message_dto), - ).start() + threading.Thread( + target=TeamService.send_message_to_all_team_members, + args=(team_id, team.name, message_dto), + ).start() - return {"Success": "Message sent successfully"}, 200 - except ValueError as e: - return {"Error": str(e)}, 403 + return {"Success": "Message sent successfully"}, 200 diff --git a/backend/api/teams/resources.py b/backend/api/teams/resources.py index 3f5c13b848..8718c86161 100644 --- a/backend/api/teams/resources.py +++ b/backend/api/teams/resources.py @@ -1,6 +1,7 @@ from flask_restful import Resource, request, current_app from schematics.exceptions import DataError +from backend.exceptions import Forbidden from backend.models.dtos.team_dto import ( NewTeamDTO, UpdateTeamDTO, @@ -86,10 +87,11 @@ def patch(self, team_id): ) and not OrganisationService.can_user_manage_organisation( team.organisation_id, authenticated_user_id ): - return { - "Error": "User is not a admin or a manager for the team", - "SubCode": "UserNotTeamManager", - }, 403 + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + team_id=team_id, + user_id=authenticated_user_id, + ) except DataError as e: current_app.logger.error(f"error validating request: {str(e)}") return {"Error": str(e), "SubCode": "InvalidData"}, 400 @@ -139,8 +141,6 @@ def get(self, team_id): team_dto = TeamService.get_team_as_dto(team_id, user_id, omit_members) return team_dto.to_primitive(), 200 - # TODO: Add delete API then do front end services and ui work - @token_auth.login_required def delete(self, team_id): """ @@ -175,11 +175,13 @@ def delete(self, team_id): 500: description: Internal Server Error """ - if not TeamService.is_user_team_manager(team_id, token_auth.current_user()): - return { - "Error": "User is not a manager for the team", - "SubCode": "UserNotTeamManager", - }, 401 + authenticated_user_id = token_auth.current_user() + if not TeamService.is_user_team_manager(team_id, authenticated_user_id): + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + team_id=team_id, + user_id=authenticated_user_id, + ) TeamService.delete_team(team_id) return {"Success": "Team deleted"}, 200 @@ -365,7 +367,6 @@ def post(self): team_id = TeamService.create_team(team_dto) return {"teamId": team_id}, 201 else: - error_msg = "User not permitted to create team for the Organisation" - return {"Error": error_msg, "SubCode": "CreateTeamNotPermitted"}, 403 + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER") except TeamServiceError as e: return str(e), 400 diff --git a/backend/api/users/actions.py b/backend/api/users/actions.py index 0bd77f8c9d..e97c3678ee 100644 --- a/backend/api/users/actions.py +++ b/backend/api/users/actions.py @@ -183,6 +183,8 @@ def patch(self, username, role): responses: 200: description: Role set + 400: + description: Bad Request - Client Error 401: description: Unauthorized - Invalid credentials 403: @@ -196,7 +198,7 @@ def patch(self, username, role): UserService.add_role_to_user(token_auth.current_user(), username, role) return {"Success": "Role Added"}, 200 except UserServiceError as e: - return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403 + return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 400 class UsersActionsSetExpertModeAPI(Resource): diff --git a/backend/api/users/resources.py b/backend/api/users/resources.py index ab08d78fd9..603209c002 100644 --- a/backend/api/users/resources.py +++ b/backend/api/users/resources.py @@ -372,8 +372,6 @@ def get(self, username): description: Recommended projects found 401: description: Unauthorized - Invalid credentials - 403: - description: Forbidden 404: description: No recommended projects found 500: diff --git a/backend/error_messages.json b/backend/error_messages.json index f2afe220cf..1a193c8b12 100644 --- a/backend/error_messages.json +++ b/backend/error_messages.json @@ -21,8 +21,21 @@ "USER_NOT_IN_TEAM": "The requested user is not a member of the team.", "USER_NOT_FOUND": "The requested user was not found on the server.", "BAD_REQUEST": "The request was invalid or cannot be otherwise served.", - "UNAUTHORIZED": "Authentication credentials were missing or incorrect.", + "UNAUTHORIZED": "You are not authorized to perform this action. Please login and try again.", + "UNAUTHORIZED_MANAGER_FILTER": "You need to be authenticated to filter organisations by manager.", "FORBIDDEN": "The request is understood, but it has been refused or access is not allowed.", + "USER_NOT_ADMIN": "This action is only allowed for system administrators.", + "USER_NOT_ORG_MANAGER": "This action is only allowed for organisation managers or system administrators.", + "USER_NOT_PROJECT_MANAGER": "This action is only allowed for users with project manage permissions.", + "USER_NOT_VALIDATOR": "This action is only allowed for users with validation permissions in the project.", + "USER_NOT_TEAM_MANAGER": "This action is only allowed for users with team manage permissions.", + "INVALID_NEW_PROJECT_OWNER": "New project owner must be project's organisation manager or system admin", + "USER_ACTION_NOT_PERMITTED": "This user does not have the required permissions to perform this action.", + "DRAFT_PROJECT_NOT_ALLOWED": "This is a draft project and you do not have the required permissions to perform this action.", + "PRIVATE_PROJECT_NOT_ALLOWED": "This is a private project and you do not have the required permissions to perform this action.", + "USER_BLOCKED": "Your account has been blocked and is currently in read-only mode. Please contact support for further assistance.", + "MESSAGE_NOT_FOR_USER": "The requested message doesn't belong to the requesting user thus the action is not permitted.", + "COMMENT_NOT_OF_USER": "The requested comment doesn't belong to the requesting user thus the action is not permitted.", "CONFLICT": "The request could not be completed due to a conflict with the current state of the resource.", "INTERNAL_SERVER_ERROR": "An unexpected error occurred while processing the request. Please contact the system administrator." } diff --git a/backend/models/postgis/statuses.py b/backend/models/postgis/statuses.py index ef78498948..5922ba7168 100644 --- a/backend/models/postgis/statuses.py +++ b/backend/models/postgis/statuses.py @@ -73,6 +73,7 @@ class MappingNotAllowed(Enum): USER_NOT_TEAM_MEMBER = 105 PROJECT_HAS_NO_TEAM = 106 NOT_A_MAPPING_TEAM = 107 + USER_IS_BLOCKED = 108 class ValidatingNotAllowed(Enum): @@ -87,6 +88,7 @@ class ValidatingNotAllowed(Enum): USER_NOT_TEAM_MEMBER = 106 PROJECT_HAS_NO_TEAM = 107 USER_ALREADY_HAS_TASK_LOCKED = 108 + USER_IS_BLOCKED = 109 class UserGender(Enum): diff --git a/backend/services/grid/split_service.py b/backend/services/grid/split_service.py index fe57b04015..ac99ac649e 100644 --- a/backend/services/grid/split_service.py +++ b/backend/services/grid/split_service.py @@ -186,17 +186,19 @@ def split_task(split_task_dto: SplitTaskDTO) -> TaskDTOs: if ( original_task.zoom and original_task.zoom >= 18 ) or original_task_area_m < 25000: - raise SplitServiceError("SmallToSplit- Task is too small to be split") + raise SplitServiceError( + "SmallToSplit- Task is too small to be split" + ) # FLAGGED STATUS CODE: 409 # check its locked for mapping by the current user if TaskStatus(original_task.task_status) != TaskStatus.LOCKED_FOR_MAPPING: raise SplitServiceError( - "LockToSplit- Status must be LOCKED_FOR_MAPPING to split" + "LockToSplit- Status must be LOCKED_FOR_MAPPING to split" # FLAGGED STATUS CODE: 409 ) if original_task.locked_by != split_task_dto.user_id: raise SplitServiceError( - "SplitOtherUserTask- Attempting to split a task owned by another user" + "SplitOtherUserTask- Attempting to split a task owned by another user" # FLAGGED STATUS CODE: 423 ) # create new geometries from the task geometry @@ -205,7 +207,9 @@ def split_task(split_task_dto: SplitTaskDTO) -> TaskDTOs: original_task.x, original_task.y, original_task.zoom, original_task ) except Exception as e: - raise SplitServiceError(f"Error splitting task{str(e)}") + raise SplitServiceError( + f"Error splitting task{str(e)}" + ) # FLAGGED STATUS CODE: 500 # create new tasks from the new geojson i = Task.get_max_task_id_for_project(split_task_dto.project_id) @@ -217,7 +221,7 @@ def split_task(split_task_dto: SplitTaskDTO) -> TaskDTOs: if not new_geometry.intersects(original_geometry): raise InvalidGeoJson( "SplitGeoJsonError- New split task does not intersect original task" - ) + ) # FLAGGED STATUS CODE: 400 # insert new tasks into database i = i + 1 diff --git a/backend/services/mapping_service.py b/backend/services/mapping_service.py index ca8c1a47bd..2a5c362c7c 100644 --- a/backend/services/mapping_service.py +++ b/backend/services/mapping_service.py @@ -4,7 +4,7 @@ from flask import current_app from geoalchemy2 import shape -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.mapping_dto import ( ExtendLockTimeDTO, TaskDTO, @@ -59,22 +59,31 @@ def get_task_as_dto( @staticmethod def _is_task_undoable(logged_in_user_id: int, task: Task) -> bool: """Determines if the current task status can be undone by the logged in user""" - # Test to see if user can undo status on this task - if logged_in_user_id and TaskStatus(task.task_status) not in [ + # Check if task status is in a state that can be undone + if TaskStatus(task.task_status) in [ TaskStatus.LOCKED_FOR_MAPPING, TaskStatus.LOCKED_FOR_VALIDATION, TaskStatus.READY, ]: - last_action = TaskHistory.get_last_action(task.project_id, task.id) - - # User requesting task made the last change, so they are allowed to undo it. - is_user_permitted, _ = ProjectService.is_user_permitted_to_validate( - task.project_id, logged_in_user_id + raise MappingServiceError( + "UndoNotAllowed- Task is in a state that cannot be undone" ) - if last_action.user_id == int(logged_in_user_id) or is_user_permitted: - return True + # Test to see if user can undo status on this task + last_action = TaskHistory.get_last_action(task.project_id, task.id) - return False + # User requesting task made the last change, so they are allowed to undo it. + is_user_permitted, _ = ProjectService.is_user_permitted_to_validate( + task.project_id, logged_in_user_id + ) + if last_action.user_id == int(logged_in_user_id) or is_user_permitted: + return True + else: + raise Forbidden( + sub_code="USER_NOT_VALIDATOR", + user_id=logged_in_user_id, + project_id=task.project_id, + task_id=task.id, + ) @staticmethod def lock_task_for_mapping(lock_task_dto: LockTaskDTO) -> TaskDTO: @@ -88,7 +97,7 @@ def lock_task_for_mapping(lock_task_dto: LockTaskDTO) -> TaskDTO: if task.locked_by != lock_task_dto.user_id: if not task.is_mappable(): - raise MappingServiceError( + raise MappingServiceError( # FLAGGED FOR STATUS CODE: 423 "InvalidTaskState- Task in invalid state for mapping" ) @@ -97,17 +106,26 @@ def lock_task_for_mapping(lock_task_dto: LockTaskDTO) -> TaskDTO: ) if not user_can_map: if error_reason == MappingNotAllowed.USER_NOT_ACCEPTED_LICENSE: - raise UserLicenseError("User must accept license to map this task") + raise UserLicenseError( + "User must accept license to map this task" + ) # FLAGGED FOR STATUS CODE: 409 + elif error_reason == MappingNotAllowed.USER_IS_BLOCKED: + raise Forbidden( + sub_code="USER_BLOCKED", user_id=lock_task_dto.user_id + ) elif error_reason == MappingNotAllowed.USER_NOT_ON_ALLOWED_LIST: - raise MappingServiceError( - "UserNotAllowed- User not on allowed list" + raise Forbidden( + sub_code="USER_ACTION_NOT_PERMITTED", + user_id=lock_task_dto.user_id, ) elif error_reason == MappingNotAllowed.PROJECT_NOT_PUBLISHED: - raise MappingServiceError( - "ProjectNotPublished- Project is not published" + raise Forbidden( + sub_code="DRAFT_PROJECT_NOT_ALLOWED", + project_id=lock_task_dto.project_id, + user_id=lock_task_dto.user_id, ) elif error_reason == MappingNotAllowed.USER_ALREADY_HAS_TASK_LOCKED: - raise MappingServiceError( + raise MappingServiceError( # FLAGGED FOR STATUS CODE: 409 "UserAlreadyHasTaskLocked- User already has task locked" ) else: @@ -134,7 +152,7 @@ def unlock_task_after_mapping(mapped_task: MappedTaskDTO) -> TaskDTO: ]: raise MappingServiceError( "InvalidUnlockState- Can only set status to MAPPED, BADIMAGERY, READY after mapping" - ) + ) # FLAGGED FOR STATUS CODE: 400 # Update stats around the change of state last_state = TaskHistory.get_last_status( @@ -180,19 +198,15 @@ def get_task_locked_by_user(project_id: int, task_id: int, user_id: int) -> Task :raises: MappingServiceError """ task = MappingService.get_task(task_id, project_id) - if task is None: - raise NotFound( - sub_code="TASK_NOT_FOUND", project_id=project_id, task_id=task_id - ) current_state = TaskStatus(task.task_status) if current_state != TaskStatus.LOCKED_FOR_MAPPING: raise MappingServiceError( "LockBeforeUnlocking- Status must be LOCKED_FOR_MAPPING to unlock" - ) + ) # FLAGGED FOR STATUS CODE: 409 if task.locked_by != user_id: raise MappingServiceError( "TaskNotOwned- Attempting to unlock a task owned by another user" - ) + ) # FLAGGED FOR STATUS CODE: 409 return task @staticmethod @@ -444,11 +458,11 @@ def lock_time_can_be_extended(project_id, task_id, user_id): ]: raise MappingServiceError( f"TaskStatusNotLocked- Task {task_id} status is not LOCKED_FOR_MAPPING or LOCKED_FOR_VALIDATION." - ) + ) # FLAGGED FOR STATUS CODE: 409 if task.locked_by != user_id: raise MappingServiceError( "LockedByAnotherUser- Task is locked by another user." - ) + ) # FLAGGED FOR STATUS CODE: 423 @staticmethod def extend_task_lock_time(extend_dto: ExtendLockTimeDTO): diff --git a/backend/services/messaging/chat_service.py b/backend/services/messaging/chat_service.py index 825be2123f..8b84101878 100644 --- a/backend/services/messaging/chat_service.py +++ b/backend/services/messaging/chat_service.py @@ -2,7 +2,7 @@ from flask import current_app from backend import db -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.message_dto import ChatMessageDTO, ProjectChatDTO from backend.models.postgis.project_chat import ProjectChat from backend.models.postgis.project_info import ProjectInfo @@ -26,6 +26,8 @@ def post_message( project_name = ProjectInfo.get_dto_for_locale( project_id, project.default_locale ).name + + # Check if user is allowed to post message is_allowed_user = True is_manager_permission = ProjectAdminService.is_user_action_permitted_on_project( authenticated_user_id, project_id @@ -37,7 +39,11 @@ def post_message( ProjectStatus(project.status) == ProjectStatus.DRAFT and not is_manager_permission ): - raise ValueError("UserNotPermitted- User not permitted to post Comment") + raise Forbidden( + sub_code="DRAFT_PROJECT_NOT_ALLOWED", + project_id=project_id, + user_id=authenticated_user_id, + ) if project.private: is_allowed_user = False @@ -77,7 +83,11 @@ def post_message( # Ensure we return latest messages after post return ProjectChat.get_messages(chat_dto.project_id, 1, 5) else: - raise ValueError("UserNotPermitted- User not permitted to post Comment") + raise Forbidden( + sub_code="PRIVATE_PROJECT_NOT_ALLOWED", + project_id=project_id, + user_id=authenticated_user_id, + ) @staticmethod def get_messages(project_id: int, page: int, per_page: int) -> ProjectChatDTO: @@ -112,12 +122,12 @@ def get_project_chat_by_id(project_id: int, comment_id: int) -> ProjectChat: def delete_project_chat_by_id(project_id: int, comment_id: int, user_id: int): """Deletes a message from a project chat ---------------------------------------- - :param project_id: The id of the project the message belongs to - :param message_id: The message id to delete :param user_id: The id of the requesting user + :param project_id: The id of the project the message belongs to + :param comment_id: The message id to delete ---------------------------------------- :raises NotFound: When the message is not found - :raises Unauthorized: When the user is not allowed to delete the message + :raises Forbidden: When the user is not allowed to delete the message ---------------------------------------- returns: None """ @@ -145,6 +155,8 @@ def delete_project_chat_by_id(project_id: int, comment_id: int, user_id: int): db.session.delete(chat_message) db.session.commit() else: - raise ValueError( - "DeletePermissionError- User not allowed to delete message" + raise Forbidden( + sub_code="COMMENT_NOT_OF_USER", + project_id=project_id, + comment_id=comment_id, ) diff --git a/backend/services/messaging/message_service.py b/backend/services/messaging/message_service.py index a74dba6167..3726b6c8e2 100644 --- a/backend/services/messaging/message_service.py +++ b/backend/services/messaging/message_service.py @@ -9,7 +9,7 @@ from markdown import markdown from backend import db, create_app -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.message_dto import MessageDTO, MessagesDTO from backend.models.dtos.stats_dto import Pagination from backend.models.postgis.message import Message, MessageType @@ -754,9 +754,8 @@ def get_message(message_id: int, user_id: int) -> Message: raise NotFound(sub_code="MESSAGE_NOT_FOUND", message_id=message_id) if message.to_user_id != int(user_id): - raise MessageServiceError( - "AccessOtherUserMessage- " - + f"User {user_id} attempting to access another users message {message_id}" + raise Forbidden( + sub_code="MESSAGE_NOT_FOR_USER", message_id=message_id, user_id=user_id ) return message diff --git a/backend/services/project_admin_service.py b/backend/services/project_admin_service.py index 13e34bb6fb..4623aeb975 100644 --- a/backend/services/project_admin_service.py +++ b/backend/services/project_admin_service.py @@ -3,7 +3,7 @@ import geojson from flask import current_app -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.project_dto import ( DraftProjectDTO, ProjectDTO, @@ -57,12 +57,7 @@ def create_draft_project(draft_project_dto: DraftProjectDTO) -> int: # First things first, we need to validate that the author_id is a PM. issue #1715 if not (is_admin or is_org_manager): - user = UserService.get_user_by_id(user_id) - raise ( - ProjectAdminServiceError( - f"NotPermittedToCreate- User {user.username} is not permitted to create project" - ) - ) + raise Forbidden(sub_code="USER_NOT_ORG_MANAGER", user_id=user_id) # If we're cloning we'll copy all the project details from the clone, otherwise create brand new project if draft_project_dto.cloneFromProjectId: @@ -119,7 +114,7 @@ def get_project_dto_for_admin(project_id: int) -> ProjectDTO: return project.as_dto_for_admin(project_id) @staticmethod - def update_project(project_dto: ProjectDTO, authenticated_user_id: int): + def update_project(project_dto: ProjectDTO): project_id = project_dto.project_id if project_dto.project_status == ProjectStatus.PUBLISHED.name: @@ -130,17 +125,8 @@ def update_project(project_dto: ProjectDTO, authenticated_user_id: int): if project_dto.license_id: ProjectAdminService._validate_imagery_licence(project_dto.license_id) - # To be handled before reaching this function - if ProjectAdminService.is_user_action_permitted_on_project( - authenticated_user_id, project_id - ): - project = ProjectAdminService._get_project_by_id(project_id) - project.update(project_dto) - else: - raise ValueError( - str(project_id) - + " :Project can only be updated by admins or by the owner" - ) + project = ProjectAdminService._get_project_by_id(project_id) + project.update(project_dto) return project @@ -150,7 +136,7 @@ def _validate_imagery_licence(license_id: int): try: LicenseService.get_license_as_dto(license_id) except NotFound: - raise ProjectAdminServiceError( + raise ProjectAdminServiceError( # FLAGGED WHY NOT NOTFOUND? f"RequireLicenseId- LicenseId {license_id} not found" ) @@ -159,22 +145,11 @@ def delete_project(project_id: int, authenticated_user_id: int): """Deletes project if it has no completed tasks""" project = ProjectAdminService._get_project_by_id(project_id) - is_admin = UserService.is_user_an_admin(authenticated_user_id) - user_orgs = OrganisationService.get_organisations_managed_by_user_as_dto( - authenticated_user_id - ) - is_org_manager = len(user_orgs.organisations) > 0 - - if is_admin or is_org_manager: - if project.can_be_deleted(): - project.delete() - else: - raise ProjectAdminServiceError( - "HasMappedTasks- Project has mapped tasks, cannot be deleted" - ) + if project.can_be_deleted(): + project.delete() else: raise ProjectAdminServiceError( - "DeletePermissionError- User does not have permissions to delete project" + "HasMappedTasks- Project has mapped tasks, cannot be deleted" ) @staticmethod @@ -257,7 +232,7 @@ def _validate_default_locale(default_locale, project_info_locales): break if default_info is None: - raise ProjectAdminServiceError( + raise ProjectAdminServiceError( # FLAGGED: SHOULD BE VALIDATED IN DTO? 400 "InfoForLocaleRequired- Project Info for Default Locale not provided" ) @@ -267,10 +242,10 @@ def _validate_default_locale(default_locale, project_info_locales): if not value: raise ( - ProjectAdminServiceError( + ProjectAdminServiceError( # FLAGGED: SHOULD BE VALIDATED IN DTO? f"MissingRequiredAttribute- {attr} not provided for Default Locale" ) - ) + ) # Flagged : for status code 400 return True # Indicates valid default locale for unit testing @@ -299,8 +274,8 @@ def transfer_project_to(project_id: int, transfering_user_id: int, username: str project.organisation_id, transfering_user_id ) if not (is_admin or is_author or is_org_manager): - raise ProjectAdminServiceError( - "TransferPermissionError- User does not have permissions to transfer project" + raise Forbidden( + sub_code="USER_NOT_ORG_MANAGER", user_id=transfering_user_id ) # Check permissions for the new owner - must be project's org manager @@ -309,12 +284,7 @@ def transfer_project_to(project_id: int, transfering_user_id: int, username: str ) is_new_owner_admin = UserService.is_user_an_admin(new_owner.id) if not (is_new_owner_org_manager or is_new_owner_admin): - error_message = ( - "InvalidNewOwner- New owner must be project's org manager or TM admin" - ) - if current_app: - current_app.logger.debug(error_message) - raise ValueError(error_message) + raise Forbidden(sub_code="INVALID_NEW_PROJECT_OWNER", user_id=new_owner.id) else: transferred_by = User.get_by_id(transfering_user_id).username project.author_id = new_owner.id diff --git a/backend/services/project_service.py b/backend/services/project_service.py index 7d6dd1f7e3..be05cecf76 100644 --- a/backend/services/project_service.py +++ b/backend/services/project_service.py @@ -2,7 +2,7 @@ from cachetools import TTLCache, cached from flask import current_app -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.mapping_dto import TaskDTOs from backend.models.dtos.project_dto import ( ProjectDTO, @@ -202,7 +202,7 @@ def get_project_dto_for_mapper( """ project = ProjectService.get_project_by_id(project_id) # if project is public and is not draft, we don't need to check permissions - if not project.private and not project.status == ProjectStatus.DRAFT.value: + if not project.private and project.status != ProjectStatus.DRAFT.value: return project.as_dto_for_mapping(current_user_id, locale, abbrev) is_allowed_user = True @@ -219,11 +219,14 @@ def get_project_dto_for_mapper( if project.status == ProjectStatus.DRAFT.value: if not is_manager_permission: is_allowed_user = False - raise ProjectServiceError("ProjectNotFetched- Unable to fetch project") + raise Forbidden( + sub_code="DRAFT_PROJECT_NOT_ALLOWED", + project_id=project_id, + user_id=current_user_id, + ) # Private Projects - allowed_users, admins, org admins & # assigned teams (mappers, validators, project managers), authors permitted - if project.private and not is_manager_permission: is_allowed_user = False if current_user_id: @@ -252,7 +255,11 @@ def get_project_dto_for_mapper( if is_allowed_user or is_manager_permission or is_team_member: return project.as_dto_for_mapping(current_user_id, locale, abbrev) else: - return None + raise Forbidden( + sub_code="PRIVATE_PROJECT_NOT_ALLOWED", + project_id=project_id, + user_id=current_user_id, + ) @staticmethod def get_project_tasks( @@ -324,7 +331,7 @@ def evaluate_mapping_permission( project_id, allowed_roles, user_id ) - # mapping_permission = 1(level),2(teams),3(teamsAndLevel) + # mapping_permission: 1(level),2(teams),3(teamsAndLevel) if mapping_permission == MappingPermission.TEAMS.value: if not is_team_member: return False, MappingNotAllowed.USER_NOT_TEAM_MEMBER @@ -343,7 +350,7 @@ def evaluate_mapping_permission( def is_user_permitted_to_map(project_id: int, user_id: int): """Check if the user is allowed to map the on the project in scope""" if UserService.is_user_blocked(user_id): - return False, MappingNotAllowed.USER_NOT_ON_ALLOWED_LIST + return False, MappingNotAllowed.USER_IS_BLOCKED project = ProjectService.get_project_by_id(project_id) if project.license_id: @@ -428,7 +435,7 @@ def evaluate_validation_permission( def is_user_permitted_to_validate(project_id, user_id): """Check if the user is allowed to validate on the project in scope""" if UserService.is_user_blocked(user_id): - return False, ValidatingNotAllowed.USER_NOT_ON_ALLOWED_LIST + return False, ValidatingNotAllowed.USER_IS_BLOCKED project = ProjectService.get_project_by_id(project_id) if project.license_id: diff --git a/backend/services/team_service.py b/backend/services/team_service.py index 292360e3b8..7e34c83d13 100644 --- a/backend/services/team_service.py +++ b/backend/services/team_service.py @@ -3,7 +3,7 @@ from markdown import markdown from backend import create_app, db -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.team_dto import ( TeamDTO, NewTeamDTO, @@ -39,14 +39,6 @@ def __init__(self, message): current_app.logger.debug(message) -class TeamJoinNotAllowed(Exception): - """Custom Exception to notify bad user level on joining team""" - - def __init__(self, message): - if current_app: - current_app.logger.debug(message) - - class TeamService: @staticmethod def request_to_join_team(team_id: int, user_id: int): @@ -92,7 +84,11 @@ def add_user_to_team( ): is_manager = TeamService.is_user_team_manager(team_id, requesting_user) if not is_manager: - raise TeamServiceError("User is not allowed to add member to the team") + raise Forbidden( + sub_code="USER_NOT_TEAM_MANAGER", + team_id=team_id, + user_id=requesting_user, + ) team = TeamService.get_team_by_id(team_id) from_user = UserService.get_user_by_id(requesting_user) to_user = UserService.get_user_by_username(username) @@ -467,7 +463,9 @@ def update_team(team_dto: TeamDTO) -> Team: return team @staticmethod - def assert_validate_organisation(org_id: int): + def assert_validate_organisation( + org_id: int, + ): # FLAGGED: IS THIS FUNCTION REQUIRED? """Makes sure an organisation exists""" try: OrganisationService.get_organisation_by_id(org_id) diff --git a/backend/services/users/user_service.py b/backend/services/users/user_service.py index a2c2550874..f4f7e00467 100644 --- a/backend/services/users/user_service.py +++ b/backend/services/users/user_service.py @@ -4,7 +4,7 @@ from sqlalchemy.sql.expression import literal from sqlalchemy import func, or_, desc, and_, distinct, cast, Time, column -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend import db from backend.models.dtos.project_dto import ProjectFavoritesDTO, ProjectSearchResultsDTO from backend.models.dtos.user_dto import ( @@ -679,7 +679,7 @@ def add_role_to_user(admin_user_id: int, username: str, role: str): :param role: The requested role :raises UserServiceError """ - try: + try: # FLAGGED CHECK IN DTO requested_role = UserRole[role.upper()] except KeyError: raise UserServiceError( @@ -691,10 +691,7 @@ def add_role_to_user(admin_user_id: int, username: str, role: str): admin_role = UserRole(admin.role) if admin_role != UserRole.ADMIN and requested_role == UserRole.ADMIN: - raise UserServiceError( - "NeedAdminRole- You must be an Admin to assign Admin role" - ) - + raise Forbidden(sub_code="USER_NOT_ADMIN", user_id=admin_user_id) user = UserService.get_user_by_username(username) user.set_user_role(requested_role) diff --git a/backend/services/validator_service.py b/backend/services/validator_service.py index 598839c993..5d620541da 100644 --- a/backend/services/validator_service.py +++ b/backend/services/validator_service.py @@ -1,7 +1,7 @@ from flask import current_app from sqlalchemy import text -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.dtos.mapping_dto import TaskDTOs from backend.models.dtos.stats_dto import Pagination from backend.models.dtos.validator_dto import ( @@ -67,7 +67,7 @@ def lock_tasks_for_validation(validation_dto: LockForValidationDTO) -> TaskDTOs: ]: raise ValidatorServiceError( f"NotReadyForValidation- Task {task_id} is not MAPPED, BADIMAGERY or INVALIDATED" - ) + ) # FLAGGED STATUS CODE: 409 user_can_validate = ValidatorService._user_can_validate_task( validation_dto.user_id, task.mapped_by ) @@ -75,7 +75,7 @@ def lock_tasks_for_validation(validation_dto: LockForValidationDTO) -> TaskDTOs: raise ValidatorServiceError( "CannotValidateMappedTask-" + "Tasks cannot be validated by the same user who marked task as mapped or badimagery" - ) + ) # FLAGGED STATUS CODE: 409 tasks_to_lock.append(task) @@ -86,20 +86,25 @@ def lock_tasks_for_validation(validation_dto: LockForValidationDTO) -> TaskDTOs: if not user_can_validate: if error_reason == ValidatingNotAllowed.USER_NOT_ACCEPTED_LICENSE: raise UserLicenseError("User must accept license to map this task") + elif error_reason == ValidatingNotAllowed.USER_IS_BLOCKED: + raise Forbidden(sub_code="USER_BLOCKED", user_id=validation_dto.user_id) elif error_reason == ValidatingNotAllowed.USER_NOT_ON_ALLOWED_LIST: - raise ValidatorServiceError( - "UserNotAllowed- Validation not allowed because: User not on allowed list" + raise Forbidden( + sub_code="USER_ACTION_NOT_PERMITTED", + user_id=validation_dto.user_id, ) elif error_reason == ValidatingNotAllowed.PROJECT_NOT_PUBLISHED: - raise ValidatorServiceError( - "ProjectNotPublished- Validation not allowed because: Project not published" + raise Forbidden( + sub_code="DRAFT_PROJECT_NOT_ALLOWED", + project_id=validation_dto.project_id, + user_id=validation_dto.user_id, ) elif error_reason == ValidatingNotAllowed.USER_ALREADY_HAS_TASK_LOCKED: user_tasks = Task.get_locked_tasks_for_user(validation_dto.user_id) if set(user_tasks.locked_tasks) != set(validation_dto.task_ids): raise ValidatorServiceError( "UserAlreadyHasTaskLocked- User already has a task locked" - ) + ) # FLAGGED FOR STATUS CODE: 409 else: raise ValidatorServiceError( f"ValidtionNotAllowed- Validation not allowed because: {error_reason}" @@ -269,12 +274,12 @@ def get_tasks_locked_by_user(project_id: int, unlock_tasks, user_id: int): if current_state != TaskStatus.LOCKED_FOR_VALIDATION: raise ValidatorServiceError( f"NotLockedForValidation- Task {unlock_task.task_id} is not LOCKED_FOR_VALIDATION" - ) + ) # FLAGGED STATUS CODE: 409 if task.locked_by != user_id: raise ValidatorServiceError( "TaskNotOwned- Attempting to unlock a task owned by another user" - ) + ) # FLAGGED STATUS CODE: 409 if hasattr(unlock_task, "status"): # we know what status we ate going to be setting to on unlock @@ -435,6 +440,8 @@ def revert_user_tasks(revert_dto: RevertUserTasksDTO): revert_dto.preferred_locale, ) else: - raise ValidatorServiceError( - "UserActionNotPermitted- User not permitted to revert tasks" + raise Forbidden( + sub_code="USER_NOT_PROJECT_MANAGER", + project_id=revert_dto.project_id, + user_id=revert_dto.action_by, ) diff --git a/frontend/src/network/tests/server-handlers.js b/frontend/src/network/tests/server-handlers.js index c565d4cfb7..fdad0844f1 100644 --- a/frontend/src/network/tests/server-handlers.js +++ b/frontend/src/network/tests/server-handlers.js @@ -372,7 +372,7 @@ const faultyHandlers = [ return res.once( ctx.status(403), ctx.json({ - SubCode: `PrivateProject`, + error:{sub_code: `PRIVATE_PROJECT_NOT_ALLOWED`} }), ); }), diff --git a/frontend/src/utils/promise.js b/frontend/src/utils/promise.js index a9af63507a..2ee009b416 100644 --- a/frontend/src/utils/promise.js +++ b/frontend/src/utils/promise.js @@ -24,7 +24,7 @@ export async function handleErrors(response) { .clone() .json() .then((res) => { - text = res.SubCode || response.statusText; + text = res.error?.sub_code || res.SubCode || response.statusText; }); throw Error(text); } diff --git a/frontend/src/views/project.js b/frontend/src/views/project.js index 321bbd8826..ac80dd3fb8 100644 --- a/frontend/src/views/project.js +++ b/frontend/src/views/project.js @@ -198,7 +198,7 @@ export const ProjectDetailPage = () => { )} {error && ( <> - {error.message === 'PrivateProject' ? ( + {error.message === 'PRIVATE_PROJECT_NOT_ALLOWED' ? ( ) : ( diff --git a/tests/backend/integration/api/campaigns/test_resources.py b/tests/backend/integration/api/campaigns/test_resources.py index 0e2cfbf154..13a774efda 100644 --- a/tests/backend/integration/api/campaigns/test_resources.py +++ b/tests/backend/integration/api/campaigns/test_resources.py @@ -85,12 +85,9 @@ def test_update_existent_campaign_by_non_admin_fails(self): }, headers={"Authorization": self.non_admin_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "CampaignsRestAPI PATCH: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_update_existent_campaign_by_unauthenticated_user_fails(self): """ @@ -165,12 +162,9 @@ def test_delete_campaign_by_non_admin_fails(self): f"{self.endpoint_url}{self.test_campaign.id}/", headers={"Authorization": self.non_admin_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "CampaignsRestAPI DELETE: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_delete_campaign_by_unauthenticated_user_fails(self): """ @@ -261,12 +255,9 @@ def test_create_new_campaign_by_non_admin_fails(self): }, headers={"Authorization": non_admin_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "CampaignsAllAPI POST: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_create_new_campaign_by_unauthenticated_user_fails(self): """ diff --git a/tests/backend/integration/api/comments/test_resources.py b/tests/backend/integration/api/comments/test_resources.py index 4bb5603b00..9076049c7a 100644 --- a/tests/backend/integration/api/comments/test_resources.py +++ b/tests/backend/integration/api/comments/test_resources.py @@ -53,10 +53,9 @@ def test_post_comment_to_project_chat_by_blocked_user_fails(self): headers={"Authorization": generate_encoded_token(self.test_user.id)}, json={"message": TEST_MESSAGE}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["Error"], "User is on read only mode") - self.assertEqual(response_body["SubCode"], "ReadOnly") + self.assertEqual(response_body["sub_code"], "USER_BLOCKED") def test_invalid_request_to_post_comment_to_project_chat_fails(self): """ @@ -191,10 +190,9 @@ def test_returns_403_if_user_not_allowed_to_delete_comment(self): self.endpoint_url, headers={"Authorization": self.test_user_token} ) # Assert - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["Error"], " User not allowed to delete message") - self.assertEqual(response_body["SubCode"], "DeletePermissionError") + self.assertEqual(response_body["sub_code"], "COMMENT_NOT_OF_USER") def test_comment_can_be_deleted_by_author(self): """Test that endpoint returns 200 when deleting a comment of a project by the comment author""" @@ -267,10 +265,9 @@ def test_post_comment_to_task_chat_by_blocked_user_fails(self): headers={"Authorization": generate_encoded_token(self.test_user.id)}, json={"comment": TEST_MESSAGE}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["Error"], "User is on read only mode") - self.assertEqual(response_body["SubCode"], "ReadOnly") + self.assertEqual(response_body["sub_code"], "USER_BLOCKED") def test_post_comment_to_task_chat_using_an_invalid_request_fails(self): """ diff --git a/tests/backend/integration/api/interests/test_resources.py b/tests/backend/integration/api/interests/test_resources.py index 2fd7c4d885..5e90670309 100644 --- a/tests/backend/integration/api/interests/test_resources.py +++ b/tests/backend/integration/api/interests/test_resources.py @@ -42,12 +42,9 @@ def test_create_a_new_interest_by_non_admin_fails(self): headers={"Authorization": self.session_token}, json={"name": NEW_INTEREST_NAME}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "InterestsAllAPI POST: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_create_a_new_interest_using_invalid_data_fails(self): """ @@ -152,12 +149,9 @@ def test_get_an_interest_by_non_admin_fails(self): self.endpoint_url, headers={"Authorization": self.session_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "InterestsRestAPI GET: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_get_a_non_existent_interest_fails(self): """ @@ -210,12 +204,9 @@ def test_update_an_interest_by_non_admin_fails(self): headers={"Authorization": self.session_token}, json={"name": NEW_INTEREST_NAME}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "InterestsRestAPI PATCH: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_update_a_non_existent_interest_fails(self): """ @@ -297,12 +288,9 @@ def test_delete_an_interest_by_non_admin_fails(self): self.endpoint_url, headers={"Authorization": self.session_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "InterestsRestAPI DELETE: User not a Org Manager" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_delete_a_non_existent_interest_fails(self): """ diff --git a/tests/backend/integration/api/notifications/test_resources.py b/tests/backend/integration/api/notifications/test_resources.py index 07f0d55417..0bccb57fce 100644 --- a/tests/backend/integration/api/notifications/test_resources.py +++ b/tests/backend/integration/api/notifications/test_resources.py @@ -55,9 +55,9 @@ def test_get_message_returns_403(self): self.url, headers={"Authorization": self.test_sender_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["SubCode"], "AccessOtherUserMessage") + self.assertEqual(response_body["sub_code"], "MESSAGE_NOT_FOR_USER") def test_get_message_returns_404(self): """ @@ -104,9 +104,9 @@ def test_delete_message_returns_403(self): self.url, headers={"Authorization": self.test_sender_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["SubCode"], "AccessOtherUserMessage") + self.assertEqual(response_body["sub_code"], "MESSAGE_NOT_FOR_USER") def test_delete_message_returns_404(self): """ diff --git a/tests/backend/integration/api/organisations/test_campaigns.py b/tests/backend/integration/api/organisations/test_campaigns.py index b0e1aeb861..337a503e94 100644 --- a/tests/backend/integration/api/organisations/test_campaigns.py +++ b/tests/backend/integration/api/organisations/test_campaigns.py @@ -96,12 +96,9 @@ def test_non_org_admin_assigns_new_campaign_to_org_fails(self): }, headers={"Authorization": self.test_user_session_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "User is not a manager of the organisation" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") # get def test_get_organisation_campaigns_passes(self): @@ -142,12 +139,9 @@ def test_delete_organisation_campaign_non_admin_fails(self): f"/api/v2/organisations/{self.test_org.id}/campaigns/{self.test_campaign.id}/", headers={"Authorization": self.test_user_session_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "User is not a manager of the organisation" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_delete_non_existent_organisation_campaign_fails(self): """Tests that the endpoint returns 404 when the org admin deletes a non existent campaign""" diff --git a/tests/backend/integration/api/organisations/test_resources.py b/tests/backend/integration/api/organisations/test_resources.py index 8aec9f9f65..c1e6a40c8f 100644 --- a/tests/backend/integration/api/organisations/test_resources.py +++ b/tests/backend/integration/api/organisations/test_resources.py @@ -74,13 +74,13 @@ def test_get_all_org_includes_managers_if_user_is_authenticated(self): response_body[0]["managers"][0]["username"], self.test_author.username ) - def test_get_all_org_raises_error_if_filter_by_manager_id__on_unauthenticated_request( + def test_get_all_org_raises_error_if_filter_by_manager_id_on_unauthenticated_request( self, ): "Test 403 is returned if filter by manager id on unauthenticated request" response = self.client.get(f"{self.endpoint_url}?manager_user_id=2") - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 401) def test_get_all_org_includes_stats_if_omit_stats_set_false(self): """Test stats are not returned is omitOrgStats is set to False""" @@ -208,12 +208,9 @@ def test_create_org_with_non_admin_fails(self): "managers": [TEST_USERNAME], }, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "Only admin users can create organisations." - ) - self.assertEqual(response_body["SubCode"], "OnlyAdminAccess") + self.assertEqual(response_body["sub_code"], "USER_NOT_ADMIN") # get organisation def test_get_org_when_omitManagerList_is_false_passes(self): @@ -271,10 +268,9 @@ def test_delete_item_by_non_admin_user_fails(self): f"/api/v2/organisations/{self.test_org.id}/", headers={"Authorization": non_admin_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["Error"], "User is not an admin for the org") - self.assertEqual(response_body["SubCode"], "UserNotOrgAdmin") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_delete_org_with_projects_fails(self): """ @@ -287,7 +283,7 @@ def test_delete_org_with_projects_fails(self): headers={"Authorization": self.session_token}, ) response_body = response.get_json() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response_body["Error"], "Organisation has some projects") self.assertEqual(response_body["SubCode"], "OrgHasProjects") @@ -327,10 +323,9 @@ def test_update_org_details_by_non_admin_fails(self): "managers": [TEST_USERNAME], }, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual(response_body["Error"], "User is not an admin for the org") - self.assertEqual(response_body["SubCode"], "UserNotOrgAdmin") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_update_org_details_with_invalid_data_fails(self): """ diff --git a/tests/backend/integration/api/projects/test_resources.py b/tests/backend/integration/api/projects/test_resources.py index 5cbf1b267d..3fb801d20a 100644 --- a/tests/backend/integration/api/projects/test_resources.py +++ b/tests/backend/integration/api/projects/test_resources.py @@ -89,7 +89,7 @@ def test_delete_project_returns_403_on_if_request_by_unauthorized_user(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") def test_project_with_mapped_tasks_cannot_be_deleted(self): "Test project with mapped tasks cannot be deleted" @@ -101,7 +101,7 @@ def test_project_with_mapped_tasks_cannot_be_deleted(self): self.url, headers={"Authorization": self.author_session_token} ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "HasMappedTasks") def test_org_manager_can_delete_project(self): @@ -187,7 +187,7 @@ def test_create_project_returns_403_on_if_request_by_unauthorized_user(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "NotPermittedToCreate") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_ORG_MANAGER") def test_create_project_returns_400_if_invalid_json(self): "Test returns 400 if invalid json" @@ -369,7 +369,9 @@ def test_draft_project_can_only_be_accessed_by_user_with_PM_permissions(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "ProjectNotFetched") + self.assertEqual( + response.json["error"]["sub_code"], "DRAFT_PROJECT_NOT_ALLOWED" + ) # Authenticated user with that is a admin self.test_user.role = UserRole.ADMIN.value @@ -404,7 +406,9 @@ def test_private_project_requires_atleast_PM_permissions(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "PrivateProject") + self.assertEqual( + response.json["error"]["sub_code"], "PRIVATE_PROJECT_NOT_ALLOWED" + ) # Authenticated user with that is a admin # Arrange @@ -539,7 +543,7 @@ def test_patch_project_returns_403_if_user_is_not_project_manager(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") def test_patch_project_returns_400_if_invalid_body(self): "Test patch project returns 400 if invalid body." diff --git a/tests/backend/integration/api/system/test_authentication.py b/tests/backend/integration/api/system/test_authentication.py index 62ff06316c..95fbbf5f4c 100644 --- a/tests/backend/integration/api/system/test_authentication.py +++ b/tests/backend/integration/api/system/test_authentication.py @@ -210,7 +210,7 @@ def test_returns_404_if_user_does_not_exist(self): self.assertEqual(error_details["sub_code"], USER_NOT_FOUND_SUB_CODE) self.assertEqual(error_details["message"], USER_NOT_FOUND_MESSAGE) - def test_returns_403_if_invalid_email_token(self): + def test_returns_400_if_invalid_email_token(self): # Arrange user = return_canned_user(USERNAME, USER_ID) user.create() @@ -219,7 +219,7 @@ def test_returns_403_if_invalid_email_token(self): self.url, query_string={"token": "1234", "username": USERNAME} ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 400) self.assertEqual(response.json["SubCode"], "BadSignature") self.assertEqual(response.json["Error"], " Bad Token Signature") @@ -239,11 +239,11 @@ def test_returns_403_if_expired_email_token(self, mock_is_valid_token): self.url, query_string={"token": token, "username": USERNAME} ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 400) self.assertEqual(response.json["SubCode"], "ExpiredToken") self.assertEqual(response.json["Error"], " Token has expired") - def test_returns_403_if_token_and_email_mismatch(self): + def test_returns_400_if_token_and_email_mismatch(self): # Arrange user = return_canned_user(USERNAME, USER_ID) user.email_address = "test_email_2" @@ -257,7 +257,7 @@ def test_returns_403_if_token_and_email_mismatch(self): self.url, query_string={"token": token, "username": USERNAME} ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 400) self.assertEqual(response.json["SubCode"], "InvalidEmail") self.assertEqual(response.json["Error"], " Email address does not match token") diff --git a/tests/backend/integration/api/system/test_banner.py b/tests/backend/integration/api/system/test_banner.py index 426397de5c..1deace790b 100644 --- a/tests/backend/integration/api/system/test_banner.py +++ b/tests/backend/integration/api/system/test_banner.py @@ -48,7 +48,7 @@ def test_patch_banner(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "OnlyAdminAccess") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_ADMIN") # Test returns 200 OK for admin users test_user.role = 1 diff --git a/tests/backend/integration/api/tasks/test_actions.py b/tests/backend/integration/api/tasks/test_actions.py index 4d7bb034aa..13885e1254 100644 --- a/tests/backend/integration/api/tasks/test_actions.py +++ b/tests/backend/integration/api/tasks/test_actions.py @@ -58,7 +58,7 @@ def test_map_all_tasks_returns_403_for_non_PM_role_request(self, mock_pm_role): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") def test_map_all_tasks_is_allowed_for_user_with_pm_role(self, mock_pm_role): @@ -116,7 +116,7 @@ def test_validate_all_tasks_returns_403_for_non_PM_role_request(self, mock_pm_ro ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") def test_validate_all_tasks_is_allowed_for_user_with_pm_role(self, mock_pm_role): @@ -173,7 +173,7 @@ def test_invalidate_all_tasks_returns_403_for_non_PM_role_request( ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") def test_invalidate_all_tasks_is_allowed_for_user_with_pm_role(self, mock_pm_role): @@ -233,7 +233,7 @@ def test_reset_all_badimagery_tasks_returns_403_for_non_PM_role_request( ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") def test_reset_all_badimagery_tasks_is_allowed_for_user_with_pm_role( @@ -287,7 +287,7 @@ def test_reset_all_tasks_returns_403_for_non_PM_role_request(self, mock_pm_role) ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") def test_reset_all_tasks_is_allowed_for_user_with_pm_role(self, mock_pm_role): @@ -381,13 +381,15 @@ def test_mapping_lock_returns_403_for_if_user_not_allowed_to_map( ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "ProjectNotPublished") + self.assertEqual( + response.json["error"]["sub_code"], "DRAFT_PROJECT_NOT_ALLOWED" + ) @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") - def test_mapping_lock_returns_403_if_task_in_invalid_state_for_mapping( + def test_mapping_lock_returns_409_if_task_in_invalid_state_for_mapping( self, mock_pm_role ): - """Test returns 403 if task is in invalid state for mapping. i.e. not in READY or INVALIDATED state.""" + """Test returns 409 if task is in invalid state for mapping. i.e. not in READY or INVALIDATED state.""" # Arrange mock_pm_role.return_value = True # Act @@ -396,7 +398,7 @@ def test_mapping_lock_returns_403_if_task_in_invalid_state_for_mapping( ) # Assert # As Task 1 is in MAPPED state, it should not be allowed to be locked for mapping. - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "InvalidTaskState") # Arrange @@ -409,7 +411,7 @@ def test_mapping_lock_returns_403_if_task_in_invalid_state_for_mapping( ) # Assert # As Task 1 is in VALIDATED state, it should not be allowed to be locked for mapping. - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "InvalidTaskState") @patch.object(UserService, "has_user_accepted_license") @@ -511,8 +513,8 @@ def test_mapping_unlock_returns_404_for_invalid_task_id(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_mapping_unlock_returns_403_if_task_not_locked_for_mapping(self): - """Test returns 403 if task is not locked for mapping.""" + def test_mapping_unlock_returns_409_if_task_not_locked_for_mapping(self): + """Test returns 409 if task is not locked for mapping.""" # Act response = self.client.post( self.url, @@ -520,11 +522,11 @@ def test_mapping_unlock_returns_403_if_task_not_locked_for_mapping(self): json={"status": "MAPPED"}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "LockBeforeUnlocking") - def test_mapping_unlock_returns_403_if_task_locked_by_other_user(self): - """Test returns 403 if task is locked by other user.""" + def test_mapping_unlock_returns_409_if_task_locked_by_other_user(self): + """Test returns 409 if task is locked by other user.""" # Arrange task = Task.get(1, self.test_project.id) task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value @@ -537,11 +539,11 @@ def test_mapping_unlock_returns_403_if_task_locked_by_other_user(self): json={"status": "MAPPED"}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "TaskNotOwned") - def test_returns_403_if_invalid_new_state_passed(self): - """Test returns 403 if invalid new state passed as new task state should be READY, MAPPED or BADIMAGERY.""" + def test_returns_409_if_invalid_new_state_passed(self): + """Test returns 409 if invalid new state passed as new task state should be READY, MAPPED or BADIMAGERY.""" # Arrange task = Task.get(1, self.test_project.id) task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value @@ -554,7 +556,7 @@ def test_returns_403_if_invalid_new_state_passed(self): json={"status": "INVALIDATED"}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "InvalidUnlockState") def test_mapping_unlock_returns_200_on_success(self): @@ -662,11 +664,11 @@ def test_mapping_stop_returns_403_if_task_not_locked_for_mapping(self): headers={"Authorization": self.user_session_token}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "LockBeforeUnlocking") - def test_mapping_stop_returns_403_if_task_locked_by_other_user(self): - """Test returns 403 if task locked by other user.""" + def test_mapping_stop_returns_409_if_task_locked_by_other_user(self): + """Test returns 409 if task locked by other user.""" # Arrange task = Task.get(1, self.test_project.id) task.task_status = TaskStatus.LOCKED_FOR_MAPPING.value @@ -678,7 +680,7 @@ def test_mapping_stop_returns_403_if_task_locked_by_other_user(self): headers={"Authorization": self.user_session_token}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "TaskNotOwned") def test_mapping_stop_returns_200_on_success(self): @@ -778,8 +780,8 @@ def test_validation_lock_returns_404_for_invalid_task_id(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_validation_lock_returns_403_if_task_not_ready_for_validation(self): - """Test returns 403 if task not ready for validation.""" + def test_validation_lock_returns_409_if_task_not_ready_for_validation(self): + """Test returns 409 if task not ready for validation.""" # Arrange task = Task.get(1, self.test_project.id) task.task_status = TaskStatus.READY.value # not ready for validation @@ -791,13 +793,13 @@ def test_validation_lock_returns_403_if_task_not_ready_for_validation(self): json={"taskIds": [1]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "NotReadyForValidation") - def test_validation_lock_returns_403_if_mapped_by_same_user_and_user_not_admin( + def test_validation_lock_returns_409_if_mapped_by_same_user_and_user_not_admin( self, ): - """Test returns 403 if mapped by same user.""" + """Test returns 409 if mapped by same user.""" # Arrange task = Task.get(1, self.test_project.id) task.task_status = TaskStatus.MAPPED.value @@ -810,7 +812,7 @@ def test_validation_lock_returns_403_if_mapped_by_same_user_and_user_not_admin( json={"taskIds": [1]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "CannotValidateMappedTask") def test_validation_lock_returns_403_if_user_not_permitted_to_validate(self): @@ -823,7 +825,9 @@ def test_validation_lock_returns_403_if_user_not_permitted_to_validate(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "ProjectNotPublished") + self.assertEqual( + response.json["error"]["sub_code"], "DRAFT_PROJECT_NOT_ALLOWED" + ) def test_validation_lock_returns_409_if_user_hasnt_accepted_license(self): """Test returns 409 if user hasn't accepted license.""" @@ -867,10 +871,36 @@ def test_validation_lock_returns_403_if_user_not_on_allowed_list( ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserNotAllowed") + self.assertEqual( + response.json["error"]["sub_code"], "USER_ACTION_NOT_PERMITTED" + ) + + @patch.object(ProjectService, "is_user_permitted_to_validate") + def test_validation_lock_returns_403_if_user_is_blocked( + self, mock_validate_permitted + ): + """Test returns 403 if user not on allowed list.""" + # Arrange + mock_validate_permitted.return_value = ( + False, + ValidatingNotAllowed.USER_IS_BLOCKED, + ) + task = Task.get(1, self.test_project.id) + task.task_status = TaskStatus.MAPPED.value + task.mapped_by = self.test_author.id + task.update() + # Act + response = self.client.post( + self.url, + headers={"Authorization": self.user_session_token}, + json={"taskIds": [1]}, + ) + # Assert + self.assertEqual(response.status_code, 403) + self.assertEqual(response.json["error"]["sub_code"], "USER_BLOCKED") - def test_validation_lock_returns_403_if_user_has_already_locked_other_task(self): - """Test returns 403 if user has already locked other task.""" + def test_validation_lock_returns_409_if_user_has_already_locked_other_task(self): + """Test returns 409 if user has already locked other task.""" # Arrange self.test_project.status = ProjectStatus.PUBLISHED.value self.test_project.save() @@ -885,7 +915,7 @@ def test_validation_lock_returns_403_if_user_has_already_locked_other_task(self) json={"taskIds": [1]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "UserAlreadyHasTaskLocked") @patch.object(ProjectService, "is_user_permitted_to_validate") @@ -976,8 +1006,8 @@ def test_validation_unlock_returns_404_if_task_not_found(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_validation_unlock_returns_403_if_task_not_locked_for_validation(self): - """Test returns 403 if task not locked for validation.""" + def test_validation_unlock_returns_409_if_task_not_locked_for_validation(self): + """Test returns 409 if task not locked for validation.""" # Act response = self.client.post( self.url, @@ -985,7 +1015,7 @@ def test_validation_unlock_returns_403_if_task_not_locked_for_validation(self): json={"validatedTasks": [{"taskId": 1, "status": "VALIDATED"}]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "NotLockedForValidation") @staticmethod @@ -998,8 +1028,8 @@ def lock_task_for_validation(task_id, project_id, user_id, mapped_by=None): task.mapped_by = mapped_by task.update() - def test_validation_unlock_returns_403_if_task_locked_by_other_user(self): - """Test returns 403 if task locked by other user.""" + def test_validation_unlock_returns_409_if_task_locked_by_other_user(self): + """Test returns 409 if task locked by other user.""" # Arrange TestTasksActionsValidationUnlockAPI.lock_task_for_validation( 1, self.test_project.id, self.test_author.id @@ -1011,7 +1041,7 @@ def test_validation_unlock_returns_403_if_task_locked_by_other_user(self): json={"validatedTasks": [{"taskId": 1, "status": "VALIDATED"}]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "TaskNotOwned") def test_validation_unlock_returns_400_if_invalid_state_passsed(self): @@ -1160,8 +1190,8 @@ def test_validation_stop_returns_404_if_task_not_found(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_validation_stop_returns_403_if_task_not_locked_for_validation(self): - """Test returns 403 if task not locked for validation.""" + def test_validation_stop_returns_409_if_task_not_locked_for_validation(self): + """Test returns 409 if task not locked for validation.""" # Arrange TestTasksActionsValidationUnlockAPI.lock_task_for_validation( 1, self.test_project.id, self.test_user.id @@ -1174,11 +1204,11 @@ def test_validation_stop_returns_403_if_task_not_locked_for_validation(self): ) # Assert # Since task 2 is not locked for validation, we should get a 403 even though task 1 is locked for validation - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "NotLockedForValidation") - def test_validation_stop_returns_403_if_task_locked_by_other_user(self): - """Test returns 403 if task locked by other user.""" + def test_validation_stop_returns_409_if_task_locked_by_other_user(self): + """Test returns 409 if task locked by other user.""" # Arrange TestTasksActionsValidationUnlockAPI.lock_task_for_validation( 1, self.test_project.id, self.test_author.id, self.test_author.id @@ -1190,7 +1220,7 @@ def test_validation_stop_returns_403_if_task_locked_by_other_user(self): json={"resetTasks": [{"taskId": 1}]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "TaskNotOwned") def test_validation_stop_returns_200_if_task_locked_by_user(self): @@ -1283,8 +1313,8 @@ def test_returns_404_if_task_not_found(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_returns_403_if_task_too_small_to_split(self): - """Test returns 403 if task too small to split.""" + def test_returns_409_if_task_too_small_to_split(self): + """Test returns 409 if task too small to split.""" # Arrange task = Task.get(1, self.test_project.id) task.zoom = 18 @@ -1295,21 +1325,21 @@ def test_returns_403_if_task_too_small_to_split(self): headers={"Authorization": self.author_session_token}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "SmallToSplit") - def test_returns_403_if_task_not_locked_for_mapping(self): - """Test returns 403 if task not locked for mapping.""" + def test_returns_409_if_task_not_locked_for_mapping(self): + """Test returns 409 if task not locked for mapping.""" # Since task should be locked for mapping to split, we should get a 403 response = self.client.post( self.url, headers={"Authorization": self.author_session_token}, ) - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "LockToSplit") - def test_returns_403_if_task_locked_by_other_user(self): - """Test returns 403 if task locked by other user.""" + def test_returns_409_if_task_locked_by_other_user(self): + """Test returns 409 if task locked by other user.""" # Arrange test_user = return_canned_user("test user", 1111111) test_user.create() @@ -1321,7 +1351,7 @@ def test_returns_403_if_task_locked_by_other_user(self): headers={"Authorization": self.author_session_token}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "SplitOtherUserTask") def test_returns_200_if_task_locked_by_user(self): @@ -1384,10 +1414,10 @@ def test_returns_404_if_task_not_found(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_returns_403_if_task_in_invalid_state_for_undo(self): - """Test returns 403 if task in invalid state for undo.""" + def test_returns_409_if_task_in_invalid_state_for_undo(self): + """Test returns 409 if task in invalid state for undo.""" # Since task cannot be in READY, LOCKED_FOR_VALIDATION or LOCKED_FOR_MAPPING state for undo, - # we should get a 403 + # we should get a 409 # Arrange task = Task.get(1, self.test_project.id) task.task_status = TaskStatus.READY.value @@ -1398,8 +1428,8 @@ def test_returns_403_if_task_in_invalid_state_for_undo(self): headers={"Authorization": self.user_session_token}, ) # Assert - self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UndoPermissionError") + self.assertEqual(response.status_code, 409) + self.assertEqual(response.json["SubCode"], "UndoNotAllowed") @staticmethod def validate_task(task_id, project_id, user_id): @@ -1424,7 +1454,7 @@ def test_returns_403_if_user_not_permitted_for_undo(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UndoPermissionError") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_VALIDATOR") @staticmethod def assert_undo_response(response, project_id, last_status, username, new_status): @@ -1545,8 +1575,8 @@ def test_returns_404_if_task_not_found(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json["error"]["sub_code"], TASK_NOT_FOUND_SUB_CODE) - def test_returns_403_if_task_not_locked(self): - """Test returns 403 if task not locked.""" + def test_returns_409_if_task_not_locked(self): + """Test returns 409 if task not locked.""" # Task should be locked for mapping or validation to extend # Act response = self.client.post( @@ -1555,11 +1585,11 @@ def test_returns_403_if_task_not_locked(self): json={"taskIds": [1]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "TaskStatusNotLocked") - def test_returns_403_if_task_is_not_locked_by_requesting_user(self): - """Test returns 403 if task is not locked by requesting user.""" + def test_returns_409_if_task_is_not_locked_by_requesting_user(self): + """Test returns 409 if task is not locked by requesting user.""" # Task should be locked for mapping or validation to extend # Arrange task = Task.get(1, self.test_project.id) @@ -1571,7 +1601,7 @@ def test_returns_403_if_task_is_not_locked_by_requesting_user(self): json={"taskIds": [1]}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) self.assertEqual(response.json["SubCode"], "LockedByAnotherUser") def test_returns_200_if_task_locked_by_requesting_user(self): @@ -1660,7 +1690,7 @@ def test_returns_403_if_user_doesnot_have_PM_permission(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "UserActionNotPermitted") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_PROJECT_MANAGER") def set_task_status(self, task, status, user_id): """Set task status.""" diff --git a/tests/backend/integration/api/teams/test_actions.py b/tests/backend/integration/api/teams/test_actions.py index 931ba0412c..00d3ce3cde 100644 --- a/tests/backend/integration/api/teams/test_actions.py +++ b/tests/backend/integration/api/teams/test_actions.py @@ -136,13 +136,9 @@ def test_handle_join_request_by_non_admin_fails(self): "username": self.test_user.username, }, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], - "You don't have permissions to approve this join team request", - ) - self.assertEqual(response_body["SubCode"], "ApproveJoinError") + self.assertEqual(response_body["sub_code"], "USER_NOT_TEAM_MANAGER") def test_handle_join_request_to_non_existent_team_fails(self): """ @@ -238,12 +234,9 @@ def test_add_members_to_team_by_non_admin_fails(self): headers={"Authorization": self.session_token}, ) response_body = response.get_json() - self.assertEqual(response.status_code, 500) + self.assertEqual(response.status_code, 403) error_resp = response_body["error"] - self.assertEqual(error_resp["sub_code"], "INTERNAL_SERVER_ERROR") - self.assertTrue( - "User is not allowed to add member to the team" in error_resp["message"] - ) + self.assertEqual(error_resp["sub_code"], "USER_NOT_TEAM_MANAGER") def test_add_non_existent_members_to_team_fails(self): """ @@ -342,13 +335,9 @@ def test_remove_members_from_team_by_non_admin_fails(self): }, headers={"Authorization": self.session_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], - f"You don't have permissions to remove {TEST_ADMIN_USERNAME} from this team.", - ) - self.assertEqual(response_body["SubCode"], "RemoveUserError") + self.assertEqual(response_body["sub_code"], "USER_NOT_TEAM_MANAGER") def test_remove_non_existent_members_from_team_fails(self): """ @@ -436,12 +425,9 @@ def test_message_team_members_by_non_admin_fails(self): json={"message": TEST_MESSAGE, "subject": TEST_SUBJECT}, headers={"Authorization": self.session_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "Unauthorised to send message to team members" - ) - self.assertEqual(response_body["SubCode"], "UserNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_TEAM_MANAGER") def test_message_team_members_with_invalid_data_fails(self): """ diff --git a/tests/backend/integration/api/teams/test_resources.py b/tests/backend/integration/api/teams/test_resources.py index 5ff3baeaa5..e2c772e514 100644 --- a/tests/backend/integration/api/teams/test_resources.py +++ b/tests/backend/integration/api/teams/test_resources.py @@ -93,12 +93,9 @@ def test_update_team_by_non_admin_fails(self): headers={"Authorization": self.test_user_token}, json={"name": NEW_TEAM_NAME}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], "User is not a admin or a manager for the team" - ) - self.assertEqual(response_body["SubCode"], "UserNotTeamManager") + self.assertEqual(response_body["sub_code"], "USER_NOT_TEAM_MANAGER") def test_update_team_with_invalid_data_fails(self): """ @@ -140,10 +137,9 @@ def test_delete_team_by_non_admin_fails(self): response = self.client.delete( self.endpoint_url, headers={"Authorization": self.test_user_token} ) - response_body = response.get_json() - self.assertEqual(response.status_code, 401) - self.assertEqual(response_body["Error"], "User is not a manager for the team") - self.assertEqual(response_body["SubCode"], "UserNotTeamManager") + response_body = response.get_json()["error"] + self.assertEqual(response.status_code, 403) + self.assertEqual(response_body["sub_code"], "USER_NOT_TEAM_MANAGER") def test_delete_non_existent_team_by_id_fails(self): """ @@ -209,13 +205,9 @@ def test_create_new_team_non_admin_fails(self): }, headers={"Authorization": self.non_admin_token}, ) - response_body = response.get_json() + response_body = response.get_json()["error"] self.assertEqual(response.status_code, 403) - self.assertEqual( - response_body["Error"], - "User not permitted to create team for the Organisation", - ) - self.assertEqual(response_body["SubCode"], "CreateTeamNotPermitted") + self.assertEqual(response_body["sub_code"], "USER_NOT_ORG_MANAGER") def test_create_new_team_existent_org_passes(self): """ diff --git a/tests/backend/integration/api/users/test_actions.py b/tests/backend/integration/api/users/test_actions.py index 0dcc76cfba..99def8c212 100644 --- a/tests/backend/integration/api/users/test_actions.py +++ b/tests/backend/integration/api/users/test_actions.py @@ -257,7 +257,7 @@ def test_returns_403_if_user_not_admin(self): ) # Assert self.assertEqual(response.status_code, 403) - self.assertEqual(response.json["SubCode"], "NeedAdminRole") + self.assertEqual(response.json["error"]["sub_code"], "USER_NOT_ADMIN") def test_returns_403_if_unknown_user_role(self): """Test API returns 403 if unknown user role is provided""" @@ -267,7 +267,7 @@ def test_returns_403_if_unknown_user_role(self): headers={"Authorization": self.admin_session_token}, ) # Assert - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 400) self.assertEqual(response.json["SubCode"], "UnknownAddRole") def test_returns_404_if_user_not_found(self): diff --git a/tests/backend/integration/services/messaging/test_chat_service.py b/tests/backend/integration/services/messaging/test_chat_service.py index 1a52724718..a787640f55 100644 --- a/tests/backend/integration/services/messaging/test_chat_service.py +++ b/tests/backend/integration/services/messaging/test_chat_service.py @@ -1,7 +1,7 @@ import threading from unittest.mock import patch -from backend.exceptions import NotFound +from backend.exceptions import NotFound, Forbidden from backend.models.postgis.statuses import ProjectStatus from backend.models.postgis.team import TeamRoles, TeamMemberFunctions from tests.backend.base import BaseTestCase @@ -44,7 +44,7 @@ def test_post_message_raises_error_if_user_not_manager_in_draft_project(self): self.chat_dto.user_id = self.test_user.id self.chat_dto.username = self.test_user.username # Act/Assert - with self.assertRaises(ValueError): + with self.assertRaises(Forbidden): ChatService.post_message( self.chat_dto, self.canned_project.id, self.test_user.id ) @@ -81,7 +81,7 @@ def test_post_message_raises_error_if_user_not_member_of_allowed_team_in_private self.canned_project.status = ProjectStatus.PUBLISHED.value self.canned_project.private = True # Act/Assert - with self.assertRaises(ValueError): + with self.assertRaises(Forbidden): ChatService.post_message( self.chat_dto, self.canned_project.id, self.test_user.id ) @@ -143,7 +143,7 @@ def test_delete_project_chat_by_id_raises_error_if_user_is_not_comment_author_an ) comment = comments.chat[0] # Act/Assert - with self.assertRaises(ValueError): + with self.assertRaises(Forbidden): ChatService.delete_project_chat_by_id( self.canned_project.id, comment.id, self.test_user.id ) diff --git a/tests/backend/integration/services/test_project_admin_service.py b/tests/backend/integration/services/test_project_admin_service.py index e5515a6704..fc95d71664 100644 --- a/tests/backend/integration/services/test_project_admin_service.py +++ b/tests/backend/integration/services/test_project_admin_service.py @@ -2,7 +2,8 @@ import json from flask import current_app -from backend.models.postgis.project import Project, User, NotFound, ProjectPriority +from backend.exceptions import NotFound, Forbidden +from backend.models.postgis.project import Project, User, ProjectPriority from backend.models.postgis.statuses import UserRole, ProjectDifficulty from backend.services.messaging.message_service import MessageService from backend.services.team_service import TeamService @@ -48,7 +49,7 @@ def test_create_draft_project_raises_error_if_user_not_admin_or_org_manager( mock_user_get.return_value = return_canned_user() # Act/Assert - with self.assertRaises(ProjectAdminServiceError): + with self.assertRaises(Forbidden): ProjectAdminService.create_draft_project(draft_project_dto) @patch.object(UserService, "is_user_an_admin") @@ -286,7 +287,7 @@ def test_update_published_project_with_incomplete_default_locale_raises_error( mock_user.return_value = stub_admin_user # Act / Assert with self.assertRaises(ProjectAdminServiceError): - ProjectAdminService.update_project(dto, mock_user.id) + ProjectAdminService.update_project(dto) @patch.object(User, "get_by_id") @patch.object(Project, "update") @@ -309,7 +310,7 @@ def test_updating_a_private_project_with_no_allowed_users_raises_error( # Act try: - ProjectAdminService.update_project(dto, mock_user.id) + ProjectAdminService.update_project(dto) # Assert except ProjectAdminServiceError: self.fail("update_project raised an exception when setting it as private") @@ -348,44 +349,7 @@ def test_update_project_with_non_existant_license_raises_error( mock_user.return_value = stub_admin_user # Act / Assert with self.assertRaises(ProjectAdminServiceError): - ProjectAdminService.update_project(dto, mock_user.id) - - @patch.object(User, "get_by_id") - @patch.object(Project, "update") - @patch.object(Project, "get") - def test_updating_a_project_with_different_roles_raises_error( - self, mock_project, mock_project2, mock_user - ): - # Arrange - stub_project = Project() - stub_project.status = ProjectStatus.DRAFT.value - - mock_project.return_value = stub_project - - locales = [] - info = ProjectInfoDTO() - info.locale = "en" - info.name = "Test" - locales.append(info) - - dto = ProjectDTO() - dto.project_id = 1 - dto.default_locale = "en" - dto.project_status = ProjectStatus.DRAFT.name - dto.project_priority = ProjectPriority.LOW.name - dto.difficulty = ProjectDifficulty.EASY.name - dto.mapping_types = ["ROADS"] - dto.mapping_editors = ["ID"] - dto.validation_editors = ["ID"] - dto.project_info_locales = locales - - stub_user = User() - stub_user.username = "mapper" - stub_user.role = UserRole.MAPPER.value - mock_user.return_value = stub_user - # Act/Assert - with self.assertRaises(ValueError): - ProjectAdminService.update_project(dto, mock_user.id) + ProjectAdminService.update_project(dto) def test_updating_a_project_with_valid_project_info(self): locales = [] @@ -397,7 +361,7 @@ def test_updating_a_project_with_valid_project_info(self): info.instructions = "Test instructions" locales.append(info) - test_project, test_user = create_canned_project() + test_project, _ = create_canned_project() dto = ProjectDTO() dto.project_id = test_project.id @@ -410,7 +374,7 @@ def test_updating_a_project_with_valid_project_info(self): dto.validation_editors = ["ID"] dto.project_info_locales = locales # Act - updated_project = ProjectAdminService.update_project(dto, test_user.id) + updated_project = ProjectAdminService.update_project(dto) # Assert self.assertEqual( updated_project.difficulty, ProjectDifficulty[dto.difficulty.upper()].value @@ -438,7 +402,7 @@ def test_project_transfer_to_(self, mock_send_message): current_app.logger.debug( "Testing error is raised if initiating user is not permitted to transfer project" ) - with self.assertRaises(ProjectAdminServiceError): + with self.assertRaises(Forbidden): ProjectAdminService.transfer_project_to( test_project.id, test_user.id, test_manager.username ) @@ -447,7 +411,7 @@ def test_project_transfer_to_(self, mock_send_message): current_app.logger.debug( "Testing error is raised if transferred to user who is not a manager of the organisation" ) - with self.assertRaises(ValueError): + with self.assertRaises(Forbidden): ProjectAdminService.transfer_project_to( test_project.id, test_manager.id, test_user.username ) diff --git a/tests/backend/integration/services/test_project_service.py b/tests/backend/integration/services/test_project_service.py index 615b532d5c..e671d1bc53 100644 --- a/tests/backend/integration/services/test_project_service.py +++ b/tests/backend/integration/services/test_project_service.py @@ -1,8 +1,9 @@ from unittest.mock import patch +from backend.exceptions import Forbidden from backend.models.postgis.statuses import ProjectStatus, UserRole from backend.services.project_admin_service import ProjectAdminService -from backend.services.project_service import ProjectService, ProjectServiceError +from backend.services.project_service import ProjectService from backend.services.team_service import TeamService from tests.backend.base import BaseTestCase from tests.backend.helpers.test_helpers import create_canned_project, return_canned_user @@ -31,7 +32,7 @@ def test_get_project_dto_for_mapper_returns_project_dto(self): def test_get_project_dto_for_mapper_raises_error_if_draft_project(self): # Project status is already set as draft while creating test project so no need to change it's status # Act/Assert - with self.assertRaises(ProjectServiceError): + with self.assertRaises(Forbidden): ProjectService.get_project_dto_for_mapper( self.test_project.id, self.test_mapper.id ) @@ -42,11 +43,10 @@ def test_get_project_dto_for_mapper_returns_none_if_private_project(self): self.test_project.status = ProjectStatus.PUBLISHED.value self.test_project.private = True # Act - project_dto = ProjectService.get_project_dto_for_mapper( - self.test_project.id, self.test_mapper.id - ) - # Assert - self.assertIsNone(project_dto) + with self.assertRaises(Forbidden): + ProjectService.get_project_dto_for_mapper( + self.test_project.id, self.test_mapper.id + ) def test_get_project_dto_for_mapper_returns_private_and_draft_project_dto_for_adimn( self, diff --git a/tests/backend/integration/services/test_validation_service.py b/tests/backend/integration/services/test_validation_service.py index dc8edd1452..2645fd558c 100644 --- a/tests/backend/integration/services/test_validation_service.py +++ b/tests/backend/integration/services/test_validation_service.py @@ -1,6 +1,6 @@ +from backend.exceptions import Forbidden from backend.services.validator_service import ( ValidatorService, - ValidatorServiceError, TaskStatus, Task, ) @@ -70,7 +70,7 @@ def test_revert_user_tasks_requires_user_with_PM_permission_for_successful_opera revert_dto.action_by = self.test_user.id revert_dto.action = "VALIDATED" # Act/Assert - with self.assertRaises(ValidatorServiceError): + with self.assertRaises(Forbidden): ValidatorService.revert_user_tasks(revert_dto) def test_revert_user_tasks_revert_validated_task_to_mapped_status(self): diff --git a/tests/backend/unit/services/test_mapping_service.py b/tests/backend/unit/services/test_mapping_service.py index 21700db801..ac6e3d3e05 100644 --- a/tests/backend/unit/services/test_mapping_service.py +++ b/tests/backend/unit/services/test_mapping_service.py @@ -1,4 +1,6 @@ from unittest.mock import patch, MagicMock + +from backend.exceptions import Forbidden from backend.services.mapping_service import ( MappingService, Task, @@ -231,7 +233,7 @@ def test_task_is_not_undoable_if_last_change_not_made_by_you( # Act mock_project.return_value = (False, None) - is_undoable = MappingService._is_task_undoable(1, task) # Assert - self.assertFalse(is_undoable) + with self.assertRaises(Forbidden): + MappingService._is_task_undoable(1, task) diff --git a/tests/backend/unit/services/test_project_service.py b/tests/backend/unit/services/test_project_service.py index f986f61258..d931d17f0f 100644 --- a/tests/backend/unit/services/test_project_service.py +++ b/tests/backend/unit/services/test_project_service.py @@ -119,7 +119,7 @@ def test_user_allowed_to_map( mock_user_blocked.return_value = True allowed, reason = ProjectService.is_user_permitted_to_map(1, 1) self.assertFalse(allowed) - self.assertEqual(reason, MappingNotAllowed.USER_NOT_ON_ALLOWED_LIST) + self.assertEqual(reason, MappingNotAllowed.USER_IS_BLOCKED) @patch.object(ProjectAdminService, "is_user_action_permitted_on_project") @patch.object(UserService, "is_user_blocked") @@ -169,7 +169,7 @@ def test_user_permitted_to_validate( mock_user_blocked.return_value = True allowed, reason = ProjectService.is_user_permitted_to_validate(1, 1) self.assertFalse(allowed) - self.assertEqual(reason, ValidatingNotAllowed.USER_NOT_ON_ALLOWED_LIST) + self.assertEqual(reason, ValidatingNotAllowed.USER_IS_BLOCKED) # Unpublished project stub_project.status = ProjectStatus.DRAFT.value