idp_oidc: display authorization request errors (#40851)

src/authentic2_idp_oidc/views.py

@@ -46,6 +46,8 @@ from django_rbac.utils import get_ou_model
 
 from . import app_settings, models, utils
 
+logger = logging.getLogger(__name__)
+
 
 @setting_enabled('ENABLE', settings=app_settings)
 def openid_configuration(request, *args, **kwargs):
@@ -78,7 +80,6 @@ def certs(request, *args, **kwargs):
 
 def authorization_error(request, redirect_uri, error, error_description=None, error_uri=None,
                         state=None, fragment=False):
-    logger = logging.getLogger(__name__)
     params = {
         'error': error,
     }
@@ -112,24 +113,28 @@ def is_scopes_allowed(scopes, client):
     return scopes <= set(allowed_scopes(client))
 
 
+def log_invalid_request(request, debug_info):
+    logger.warning('idp_oidc: authorization request error, %s', debug_info)
+    error_message = _('Authorization request is invalid')
+    if settings.DEBUG:
+        error_message += ' (%s)' % debug_info
+    messages.warning(request, error_message)
+
+
 @setting_enabled('ENABLE', settings=app_settings)
 def authorize(request, *args, **kwargs):
-    logger = logging.getLogger(__name__)
     start = now()
 
     try:
         client_id = request.GET['client_id']
         redirect_uri = request.GET['redirect_uri']
     except KeyError as k:
-        messages.warning(request, _('Authorization request is invalid'))
-        logger.warning(u'idp_oidc: authorization request error, missing %s', k.args[0])
+        log_invalid_request(request, 'missing %s' % k.args[0])
         return redirect(request, 'auth_homepage')
     try:
         client = models.OIDCClient.objects.get(client_id=client_id)
     except models.OIDCClient.DoesNotExist:
-        messages.warning(request, _('Authorization request is invalid'))
-        logger.warning(u'idp_oidc: authorization request error, unknown client_id redirect_uri=%r client_id=%r',
-                       redirect_uri, client_id)
+        log_invalid_request(request, 'unknown client_id redirect_uri=%r client_id=%r' % (redirect_uri, client_id))
         return redirect(request, 'auth_homepage')
 
     if client.authorization_flow == client.FLOW_RESOURCE_OWNER_CRED:
@@ -142,9 +147,7 @@ def authorize(request, *args, **kwargs):
     try:
         client.validate_redirect_uri(redirect_uri)
     except ValueError as e:
-        messages.warning(request, _('Authorization request is invalid: %s') % e)
-        logger.warning(u'idp_oidc: authorization request error, invalid redirect_uri %r (client_id=%r): %s',
-                       redirect_uri, client_id, e)
+        log_invalid_request(request, 'invalid redirect_uri redirect_uri=%r client_id=%r (%s)' % (redirect_uri, client_id, e))
         return redirect(request, 'auth_homepage')
 
     fragment = client.authorization_flow == client.FLOW_IMPLICIT

tests/test_idp_oidc.py

@@ -28,6 +28,7 @@ from . import utils
 from django import VERSION as DJ_VERSION
 from django.core.exceptions import ValidationError
 from django.core.files import File
+from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils.encoding import force_text
 from django.utils.timezone import now
@@ -457,6 +458,13 @@ def test_invalid_request(caplog, oidc_settings, oidc_client, simple_user, app):
     assert urlparse.urlparse(response['Location']).path == '/'
     response = response.maybe_follow()
     assert 'Authorization request is invalid' in response
+    assert not 'invalid redirect_uri' in response
+
+    with override_settings(DEBUG=True):
+        response = app.get(authorize_url, status=302)
+    assert urlparse.urlparse(response['Location']).path  == '/'
+    response = response.maybe_follow()
+    assert 'invalid redirect_uri' in response
 
     # missing response_type
     authorize_url = make_url('oidc-authorize', params={