Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

fix: Don't show detailed errors for anonymous users #956

Merged
merged 1 commit into from
Nov 5, 2024
Merged

fix: Don't show detailed errors for anonymous users #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion graphql_api/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ async def test_when_debug_is_false_and_exception_we_know(self):
assert data["errors"][0]["type"] == "Unauthorized"
assert data["errors"][0].get("extensions") is None

@override_settings(DEBUG=False)
@override_settings(DEBUG=True)
async def test_when_bad_query(self):
schema = generate_schema_that_raise_with(Unauthorized())
data = await self.do_query(schema, " { fieldThatDoesntExist }")
Expand All @@ -123,6 +123,13 @@ async def test_when_bad_query(self):
== "Cannot query field 'fieldThatDoesntExist' on type 'Query'."
)

@override_settings(DEBUG=False)
async def test_when_bad_query_and_anonymous(self):
schema = generate_schema_that_raise_with(Unauthorized())
data = await self.do_query(schema, " { fieldThatDoesntExist }")
assert data["errors"] is not None
assert data["errors"][0]["message"] == "INTERNAL SERVER ERROR"

@override_settings(DEBUG=False, GRAPHQL_QUERY_COST_THRESHOLD=1000)
@patch("logging.Logger.error")
async def test_when_costly_query(self, mock_error_logger):
Expand Down
6 changes: 5 additions & 1 deletion graphql_api/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ async def post(self, request, *args, **kwargs):

def context_value(self, request, *_):
request_body = json.loads(request.body.decode("utf-8")) if request.body else {}
self.request = request

return {
"request": request,
"service": request.resolver_match.kwargs["service"],
Expand All @@ -300,9 +302,11 @@ def context_value(self, request, *_):
}

def error_formatter(self, error, debug=False):
user = self.request.user
Copy link
Contributor

@ajay-sentry ajay-sentry Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if user is a "none" value wouldn't line 306 return a can't access on undefined error?

maybe we could do a self.request.get("user",{})

Copy link
Contributor Author

@RulaKhaled RulaKhaled Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really! request.user is automatically populated by the AuthenticationMiddleware middleware, it exists in all requests as an instance of User or AnonymousUser, and fwiw we're not checking for user anywhere else in the view

Copy link
Contributor

@ajay-sentry ajay-sentry Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it! Thanks for clarifying

is_anonymous = user.is_anonymous if user else True
# the only way to check for a malformed query
is_bad_query = "Cannot query field" in error.formatted["message"]
if debug or is_bad_query:
if debug or (not is_anonymous and is_bad_query):
return format_error(error, debug)
formatted = error.formatted
formatted["message"] = "INTERNAL SERVER ERROR"
Expand Down
Loading