diff --git a/jobbergate-api/jobbergate_api/apps/services.py b/jobbergate-api/jobbergate_api/apps/services.py index dd67c4b1..89366a2b 100644 --- a/jobbergate-api/jobbergate_api/apps/services.py +++ b/jobbergate-api/jobbergate_api/apps/services.py @@ -7,7 +7,7 @@ from typing import Any, Generic, Protocol, TypeVar from botocore.response import StreamingBody -from buzz import check_expressions, enforce_defined, handle_errors, require_condition +from buzz import enforce_defined, handle_errors, require_condition from fastapi import HTTPException, UploadFile, status from fastapi_pagination import Page from fastapi_pagination.ext.sqlalchemy import paginate @@ -345,16 +345,10 @@ def ensure_attribute(self, instance: CrudModel, **attributes) -> None: Raises HTTPException if the instance does not have the specified values. """ - with check_expressions( - main_message="Request not allowed on {} by id={} due to mismatch on attribute(s)".format( - self.name, instance.id - ), - raise_exc_class=ServiceError, - raise_kwargs=dict(status_code=status.HTTP_403_FORBIDDEN), - ) as check: - for attr_name, expected_value in attributes.items(): - actual_value = getattr(instance, attr_name) - check(actual_value == expected_value, message=attr_name) + if mismatched := {k for k, v in attributes.items() if getattr(instance, k) != v}: + message = f"Mismatch on attribute(s): {', '.join(mismatched)}" + logger.debug("Access to {} id={} is forbidden due to {}", self.name, instance.id, message) + raise ServiceError(message, status_code=status.HTTP_403_FORBIDDEN) @property def name(self): diff --git a/jobbergate-cli/CHANGELOG.md b/jobbergate-cli/CHANGELOG.md index 4373ed3a..fdc2b52c 100644 --- a/jobbergate-cli/CHANGELOG.md +++ b/jobbergate-cli/CHANGELOG.md @@ -3,7 +3,7 @@ This file keeps track of all notable changes to jobbergate-cli ## Unreleased - +- Enhance error message when request returns 403 Forbidden [ASP-5433] ## 5.3.0a4 -- 2024-08-23 - Enabled authentication by OIDC client secret [ASP-5244] diff --git a/jobbergate-cli/jobbergate_cli/requests.py b/jobbergate-cli/jobbergate_cli/requests.py index 4517342b..120136b9 100644 --- a/jobbergate-cli/jobbergate_cli/requests.py +++ b/jobbergate-cli/jobbergate_cli/requests.py @@ -15,25 +15,27 @@ from jobbergate_cli.text_tools import dedent, unwrap -def get_possible_solution_to_error(error_code: int) -> str: +def get_possible_solution_to_error(response: httpx.Response) -> str: """ Get a possible solution to an error code. """ - if httpx.codes.is_client_error(error_code): + if response.is_client_error: default_solution = "Please check the data on your request and try again" else: default_solution = "Please try again and contact support if the problem persists" custom_solutions: dict[int, str] = { # client errors - httpx.codes.UNAUTHORIZED: "Please verify your credentials to perform this action with system admin", - httpx.codes.FORBIDDEN: "Please notice only the owner of the resource can perform this action", + httpx.codes.UNAUTHORIZED: "Please login and try again", + httpx.codes.FORBIDDEN: "Unable to modify an entry owned by someone else, please contact the resource owner" + if "mismatch on attribute" in response.text.lower() + else "Please verify your credentials to perform this action with a system admin", httpx.codes.NOT_FOUND: "Please check the id number or identifier and try again", httpx.codes.REQUEST_TIMEOUT: "Please try again and contact support if the problem persists", # server errors # ... } - return custom_solutions.get(error_code, default_solution) + return custom_solutions.get(response.status_code, default_solution) def format_response_error(response: httpx.Response, default_text) -> str: @@ -45,7 +47,7 @@ def format_response_error(response: httpx.Response, default_text) -> str: message.append(response.json()["detail"]) except Exception: pass - message.append(get_possible_solution_to_error(response.status_code)) + message.append(get_possible_solution_to_error(response)) return " -- ".join(message) diff --git a/jobbergate-cli/tests/test_requests.py b/jobbergate-cli/tests/test_requests.py index 3e39b9f9..85486178 100644 --- a/jobbergate-cli/tests/test_requests.py +++ b/jobbergate-cli/tests/test_requests.py @@ -165,7 +165,7 @@ def test_make_request__raises_Abort_with_ownership_message_for_403_for_non_owner with pytest.raises( Abort, - match="There was a big problem -- This jabroni does not own this whingding -- Please notice only the owner of", + match="There was a big problem -- This jabroni does not own this whingding -- Please verify your credentials", ) as err_info: make_request( client,