From 6076c95b54dda897e7988c34aafbacceadaa3cc9 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Sat, 24 Oct 2020 18:15:06 +0200
Subject: [PATCH 4/5] idp_oidc: implement correct error reporting in user_info
 (#47900)

* error and error_description are reported in a status 401 HTTP response,
  inside the WWW-Authenticate and inside the JSON body of the response.
---
 src/authentic2_idp_oidc/views.py | 34 +++++++++------
 tests/test_idp_oidc.py           | 73 ++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/src/authentic2_idp_oidc/views.py b/src/authentic2_idp_oidc/views.py
index d6a0cdc2..a7216eaf 100644
--- a/src/authentic2_idp_oidc/views.py
+++ b/src/authentic2_idp_oidc/views.py
@@ -116,6 +116,10 @@ class InvalidRequest(OIDCException):
     error_code = 'invalid_request'
 
 
+class InvalidToken(OIDCException):
+    error_code = 'invalid_token'
+
+
 class MissingParameter(InvalidRequest):
     def __init__(self, parameter):
         super().__init__(error_description=_('Missing parameter "%s"') % parameter)
@@ -705,30 +709,36 @@ def token(request, *args, **kwargs):
 
 def authenticate_access_token(request):
     if 'HTTP_AUTHORIZATION' not in request.META:
-        return None
+        raise InvalidRequest(_('Bearer authentication is mandatory'), status=401)
     authorization = request.META['HTTP_AUTHORIZATION'].split()
     if authorization[0] != 'Bearer' or len(authorization) != 2:
-        return None
+        raise InvalidRequest(_('Invalid Bearer authentication'), status=401)
     try:
         access_token = models.OIDCAccessToken.objects.select_related().get(uuid=authorization[1])
     except models.OIDCAccessToken.DoesNotExist:
-        return None
+        raise InvalidToken(_('Token is unknown'), status=401)
     if not access_token.is_valid():
-        return None
+        raise InvalidToken(_('Token is expired or user is disconnected'), status=401)
     return access_token
 
 
 @setting_enabled('ENABLE', settings=app_settings)
 @csrf_exempt
 def user_info(request, *args, **kwargs):
-    access_token = authenticate_access_token(request)
-    if access_token is None:
-        return HttpResponse('unauthenticated', status=401)
-    user_info = utils.create_user_info(request,
-                                       access_token.client,
-                                       access_token.user,
-                                       access_token.scope_set())
-    return JsonResponse(user_info)
+    try:
+        access_token = authenticate_access_token(request)
+        user_info = utils.create_user_info(request,
+                                           access_token.client,
+                                           access_token.user,
+                                           access_token.scope_set())
+        return JsonResponse(user_info)
+    except OIDCException as e:
+        error_response = JsonResponse(
+            {'error': e.error_code, 'error_description': e.error_description}, status=e.status)
+        if e.status == 401:
+            error_response['WWW-Authenticate'] = 'Bearer error="%s", error_description="%s"' % (
+                e.error_code, e.error_description)
+        return error_response
 
 
 @setting_enabled('ENABLE', settings=app_settings)
diff --git a/tests/test_idp_oidc.py b/tests/test_idp_oidc.py
index 093405d8..a03c0ae5 100644
--- a/tests/test_idp_oidc.py
+++ b/tests/test_idp_oidc.py
@@ -1754,3 +1754,76 @@ def test_oidc_good_next_url_hook(app, oidc_client):
     rf = RequestFactory()
     request = rf.get('/')
     assert good_next_url(request, 'https://example.com/')
+
+
+@pytest.fixture
+def access_token(client, simple_user):
+    return OIDCAccessToken.objects.create(
+        client=client,
+        user=simple_user,
+        scopes='openid profile email',
+        expired=now() + datetime.timedelta(seconds=3600))
+
+
+def test_user_info(app, client, access_token, freezer):
+    def get_user_info(**kwargs):
+        return app.get(
+            '/idp/oidc/user_info/',
+            headers=bearer_authentication_headers(access_token.uuid),
+            **kwargs)
+
+    response = app.get('/idp/oidc/user_info/', status=401)
+    assert (
+        response['WWW-Authenticate']
+        == 'Bearer error="invalid_request", error_description="Bearer authentication is mandatory"'
+    )
+
+    response = app.get('/idp/oidc/user_info/',
+                       headers={'Authorization': 'Bearer'}, status=401)
+    assert (
+        response['WWW-Authenticate']
+        == 'Bearer error="invalid_request", error_description="Invalid Bearer authentication"'
+    )
+
+    response = get_user_info(status=200)
+    assert dict(response.json, sub='') == {
+        'email': 'user@example.net',
+        'email_verified': False,
+        'family_name': 'Dôe',
+        'family_name_verified': True,
+        'given_name': 'Jôhn',
+        'given_name_verified': True,
+        'preferred_username': 'user',
+        'sub': '',
+    }
+
+    # token is expired
+    access_token.expired = now() - datetime.timedelta(seconds=1)
+    access_token.save()
+    response = get_user_info(status=401)
+    assert (
+        response['WWW-Authenticate']
+        == 'Bearer error="invalid_token", error_description="Token is expired or user is disconnected"'
+    )
+
+    # token is unknown
+    access_token.delete()
+    response = get_user_info(status=401)
+    assert (
+        response['WWW-Authenticate']
+        == 'Bearer error="invalid_token", error_description="Token is unknown"'
+    )
+
+    utils.login(app, access_token.user)
+    access_token.expired = now() + datetime.timedelta(seconds=1)
+    access_token.session_key = app.session.session_key
+    access_token.save()
+
+    get_user_info(status=200)
+
+    app.session.flush()
+    response = get_user_info(status=401)
+    assert (
+        response['WWW-Authenticate']
+        == 'Bearer error="invalid_token", error_description="Token is expired or user is disconnected"'
+    )
-- 
2.29.2