diff --git a/addon_service/common/aiohttp_session.py b/addon_service/common/aiohttp_session.py
index 6f83274f..9cf356d1 100644
--- a/addon_service/common/aiohttp_session.py
+++ b/addon_service/common/aiohttp_session.py
@@ -14,6 +14,10 @@
"close_singleton_client_session__blocking",
)
+# an identifying user-agent: some providers' CDNs (e.g. Mendeley, behind Cloudflare)
+# challenge or block requests sent with aiohttp's default `Python/x.y aiohttp/z` user-agent
+USER_AGENT = "GravyValet/1.0"
+
__SINGLETON_CLIENT_SESSION_STORE: MutableMapping[threading.Thread, ClientSession] = (
weakref.WeakKeyDictionary()
)
@@ -30,6 +34,7 @@ async def get_singleton_client_session() -> aiohttp.ClientSession:
if thread_id not in __SINGLETON_CLIENT_SESSION_STORE:
__SINGLETON_CLIENT_SESSION_STORE[thread_id] = aiohttp.ClientSession(
cookie_jar=aiohttp.DummyCookieJar(), # ignore all cookies
+ headers={"User-Agent": USER_AGENT},
)
return __SINGLETON_CLIENT_SESSION_STORE[thread_id]
diff --git a/addon_service/oauth2/views.py b/addon_service/oauth2/views.py
index 4c6d30ff..9650c0e6 100644
--- a/addon_service/oauth2/views.py
+++ b/addon_service/oauth2/views.py
@@ -1,4 +1,5 @@
import asyncio
+import logging
from http import HTTPStatus
from asgiref.sync import sync_to_async
@@ -13,6 +14,22 @@
from addon_service.oauth2.utils import get_initial_access_token
+_logger = logging.getLogger(__name__)
+
+# https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1
+_KNOWN_ERROR_CODES = frozenset(
+ (
+ "invalid_request",
+ "unauthorized_client",
+ "access_denied",
+ "unsupported_response_type",
+ "invalid_scope",
+ "server_error",
+ "temporarily_unavailable",
+ )
+)
+
+
# Exclude oAuth views from openapi schema as they are from internal use only
@extend_schema(exclude=True)
@transaction.non_atomic_requests # async views and ATOMIC_REQUESTS do not mix
@@ -23,7 +40,20 @@ async def oauth2_callback_view(request):
see https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2
"""
- # TODO: handle error: https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1
+ if "error" in request.GET:
+ # query params are attacker-controlled: never reflect them, and never as html
+ _error = request.GET["error"]
+ _logger.warning(
+ "oauth2 callback error=%r error_description=%r",
+ _error[:100],
+ request.GET.get("error_description", "")[:200],
+ )
+ return HttpResponse(
+ "oauth2 authorization failed"
+ + (f": {_error}" if _error in _KNOWN_ERROR_CODES else ""),
+ status=HTTPStatus.BAD_REQUEST,
+ content_type="text/plain; charset=utf-8",
+ )
_state_token = request.GET["state"]
_authorization_code = request.GET["code"]
_token_metadata, _oauth_client_config = await _resolve_state_token(_state_token)
diff --git a/addon_service/tests/test_oauth2_callback_view.py b/addon_service/tests/test_oauth2_callback_view.py
new file mode 100644
index 00000000..e3d17396
--- /dev/null
+++ b/addon_service/tests/test_oauth2_callback_view.py
@@ -0,0 +1,42 @@
+from http import HTTPStatus
+from unittest import IsolatedAsyncioTestCase
+
+from django.test import SimpleTestCase
+from django.urls import reverse
+
+from addon_service.common.aiohttp_session import (
+ USER_AGENT,
+ close_singleton_client_session,
+ get_singleton_client_session,
+)
+
+
+class TestOAuth2CallbackViewErrors(SimpleTestCase):
+ def _get_error(self, **params):
+ return self.client.get(reverse("oauth2-callback"), {"state": "1.n", **params})
+
+ def test_known_error_code(self):
+ _resp = self._get_error(error="access_denied", error_description="no thanks")
+ self.assertEqual(_resp.status_code, HTTPStatus.BAD_REQUEST)
+ self.assertIn("access_denied", _resp.content.decode())
+ self.assertNotIn("no thanks", _resp.content.decode())
+
+ def test_error_params_are_not_reflected(self):
+ with self.assertLogs("addon_service.oauth2.views", "WARNING"):
+ _resp = self._get_error(
+ error="",
+ error_description="
",
+ )
+ self.assertEqual(_resp.status_code, HTTPStatus.BAD_REQUEST)
+ self.assertTrue(_resp["Content-Type"].startswith("text/plain"))
+ self.assertNotIn("