Skip to content

Commit

Permalink
[ASP-5433] Enhance error message when user is Forbidden to perform ac…
Browse files Browse the repository at this point in the history
…tion (#608)

* feat(cli): enhance message when request return 403 forbidden

* code review
  • Loading branch information
fschuch authored Aug 30, 2024
1 parent e368126 commit 675aa9e
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 19 deletions.
16 changes: 5 additions & 11 deletions jobbergate-api/jobbergate_api/apps/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion jobbergate-cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
14 changes: 8 additions & 6 deletions jobbergate-cli/jobbergate_cli/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)


Expand Down
2 changes: 1 addition & 1 deletion jobbergate-cli/tests/test_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 675aa9e

Please sign in to comment.