prevent redirection loop on artifact resolution errors (fixes #14810)

Signature of method sso_failure() is changed to match the name name of
the context variable in template mellon/authentication_failed.html
(idp_message => reason).

mellon/locale/fr/LC_MESSAGES/django.po

@@ -86,10 +86,5 @@ msgstr "Aucun utilisateur trouvé pour l'identifiant SAML %(name_id)s"
 msgid "IdP is temporarily down, please try again later."
 msgstr "Le fournisseur d'identités est temporairement inaccessible, veuillez réessayer plus tard."
 
-#~ msgid ""
+msgid "There were too many redirections with the identity provider."
-#~ "The authentication has failed, you can return to\n"
+msgstr "Il y a eu trop de redirections avec le fournisseur d'identité."
-#~ "    the <a href=\"%(next_url)s\">last page</a> you where.\n"
-#~ "    "
-#~ msgstr ""
-#~ "L'authentification a échoué, vous pouvez retourner à <a href="
-#~ "\"%(next_url)s\">la dernière page</a> atteinte."

mellon/templates/mellon/authentication_failed.html

@@ -12,7 +12,7 @@
   <h2 class="mellon-message-header">{% trans "Authentication failed" %}</h2>
   <p class="mellon-message-body">
     {% blocktrans %}The authentication has failed.{% endblocktrans %}
-    {% if idp_message %}<p class="mellon-idp-message">{% trans "Reason" %}&nbsp;: {{ idp_message }}</p>{% endif %}
+    {% if reason %}<p class="mellon-reason">{% trans "Reason" %}&nbsp;: {{ reason }}</p>{% endif %}
   </p>
   <p class="mellon-message-continue">
     <a class="mellon-link" href="{{ next_url }}">{% trans "Continue" %}</a>

mellon/views.py

@@ -21,6 +21,7 @@ from django.utils.translation import ugettext as _
 
 from . import app_settings, utils
 
+RETRY_LOGIN_COOKIE = 'MELLON_RETRY_LOGIN'
 
 lasso.setFlag('thin-sessions')
 
@@ -133,24 +134,24 @@ class LoginView(ProfileMixin, LogMixin, View):
             if 'RelayState' in request.POST and utils.is_nonnull(request.POST['RelayState']):
                 login.msgRelayState = request.POST['RelayState']
             return self.sso_success(request, login)
-        return self.sso_failure(request, login, idp_message, status_codes)
+        return self.sso_failure(request, reason=idp_message, status_codes=status_codes)
 
-    def sso_failure(self, request, login, idp_message, status_codes):
+    def sso_failure(self, request, reason='', status_codes=()):
         '''show error message to user after a login failure'''
+        login = self.profile
         idp = utils.get_idp(login.remoteProviderId)
         error_url = utils.get_setting(idp, 'ERROR_URL')
         error_redirect_after_timeout = utils.get_setting(idp, 'ERROR_REDIRECT_AFTER_TIMEOUT')
         if error_url:
             error_url = resolve_url(error_url)
-        next_url = error_url or resolve_url(settings.LOGIN_REDIRECT_URL)
+        next_url = error_url or self.get_next_url(default=resolve_url(settings.LOGIN_REDIRECT_URL))
         return render(request, 'mellon/authentication_failed.html',
                       {
                           'debug': settings.DEBUG,
-                          'idp_message': idp_message,
+                          'reason': reason,
                           'status_codes': status_codes,
                           'issuer': login.remoteProviderId,
                           'next_url': next_url,
-                          'error_url': error_url,
                           'relaystate': login.msgRelayState,
                           'error_redirect_after_timeout': error_redirect_after_timeout,
                       })
@@ -193,7 +194,9 @@ class LoginView(ProfileMixin, LogMixin, View):
                 attributes['authn_context_class_ref'] = \
                     authn_context.authnContextClassRef
         self.log.debug('trying to authenticate with attributes %r', attributes)
-        return self.authenticate(request, login, attributes)
+        response = self.authenticate(request, login, attributes)
+        response.delete_cookie(RETRY_LOGIN_COOKIE)
+        return response
 
     def authenticate(self, request, login, attributes):
         user = auth.authenticate(saml_attributes=attributes)
@@ -224,12 +227,23 @@ class LoginView(ProfileMixin, LogMixin, View):
         return HttpResponseRedirect(next_url)
 
     def retry_login(self):
-        '''Retry login if it failed for a temporary error'''
+        '''Retry login if it failed for a temporary error.
+
+           Use a cookie to prevent looping forever.
+        '''
+        if RETRY_LOGIN_COOKIE in self.request.COOKIES:
+            response = self.sso_failure(
+                self.request,
+                reason=_('There were too many redirections with the identity provider.'))
+            response.delete_cookie(RETRY_LOGIN_COOKIE)
+            return response
         url = reverse('mellon_login')
         next_url = self.get_next_url()
         if next_url:
             url = '%s?%s' % (url, urlencode({REDIRECT_FIELD_NAME: next_url}))
-        return HttpResponseRedirect(url)
+        response = HttpResponseRedirect(url)
+        response.set_cookie(RETRY_LOGIN_COOKIE, value='1', max_age=None)
+        return response
 
     def continue_sso_artifact(self, request, method):
         idp_message = None
@@ -271,12 +285,13 @@ class LoginView(ProfileMixin, LogMixin, View):
                                    verify=verify_ssl_certificate)
         except RequestException as e:
             self.log.warning('unable to reach %r: %s', login.msgUrl, e)
-            return self.sso_failure(request, login, _('IdP is temporarily down, please try again '
+            return self.sso_failure(request,
-                                                      'later.'), status_codes)
+                                    reason=_('IdP is temporarily down, please try again ' 'later.'),
+                                    status_codes=status_codes)
         if result.status_code != 200:
             self.log.warning('SAML authentication failed: IdP returned %s when given artifact: %r',
                              result.status_code, result.content)
-            return self.sso_failure(request, login, idp_message, status_codes)
+            return self.sso_failure(request, reason=idp_message, status_codes=status_codes)
 
         self.log.info('Got SAML Artifact Response', extra={'saml_response': result.content})
         result.encoding = utils.get_xml_encoding(result.content)
@@ -318,7 +333,7 @@ class LoginView(ProfileMixin, LogMixin, View):
             return HttpResponseBadRequest('error processing the authentication response: %r' % e)
         else:
             return self.sso_success(request, login)
-        return self.sso_failure(request, login, idp_message, status_codes)
+        return self.sso_failure(request, login, reason=idp_message, status_codes=status_codes)
 
     def request_discovery_service(self, request, is_passive=False):
         self_url = request.build_absolute_uri(request.path)

tests/test_sso_slo.py

@@ -1,3 +1,4 @@
+import re
 import base64
 import zlib
 
@@ -129,7 +130,7 @@ def test_sso_slo(db, app, idp, caplog, sp_settings):
     response = app.post(reverse('mellon_login'), params={'SAMLResponse': body, 'RelayState': relay_state})
     assert 'created new user' in caplog.text
     assert 'logged in using SAML' in caplog.text
-    assert response['Location'].endswith('/whatever/')
+    assert urlparse.urlparse(response['Location']).path == '/whatever/'
 
 
 def test_sso(db, app, idp, caplog, sp_settings):
@@ -140,7 +141,7 @@ def test_sso(db, app, idp, caplog, sp_settings):
     response = app.post(reverse('mellon_login'), params={'SAMLResponse': body, 'RelayState': relay_state})
     assert 'created new user' in caplog.text
     assert 'logged in using SAML' in caplog.text
-    assert response['Location'].endswith(sp_settings.LOGIN_REDIRECT_URL)
+    assert urlparse.urlparse(response['Location']).path == sp_settings.LOGIN_REDIRECT_URL
 
 
 def test_sso_request_denied(db, app, idp, caplog, sp_settings):
@@ -173,16 +174,16 @@ def test_sso_artifact(db, app, caplog, sp_settings, idp_metadata, idp_private_ke
         response = app.get(acs_artifact_url, params={'RelayState': relay_state})
     assert 'created new user' in caplog.text
     assert 'logged in using SAML' in caplog.text
-    assert response['Location'].endswith('/whatever/')
+    assert urlparse.urlparse(response['Location']).path == '/whatever/'
-    # force delog
+    # force delog, but keep session information for relaystate handling
+    assert app.session
     del app.session['_auth_user_id']
     assert 'dead artifact' not in caplog.text
     with HTTMock(idp.mock_artifact_resolver()):
         response = app.get(acs_artifact_url, params={'RelayState': relay_state})
     # verify retry login was asked
     assert 'dead artifact' in caplog.text
-    assert response.status_code == 302
+    assert urlparse.urlparse(response['Location']).path == reverse('mellon_login')
-    assert reverse('mellon_login') in url
     response = response.follow()
     url, body, relay_state = idp.process_authn_request_redirect(response['Location'])
     assert relay_state
@@ -197,7 +198,7 @@ def test_sso_artifact(db, app, caplog, sp_settings, idp_metadata, idp_private_ke
         response = app.get(acs_artifact_url, params={'RelayState': relay_state})
     assert 'created new user' in caplog.text
     assert 'logged in using SAML' in caplog.text
-    assert response['Location'].endswith('/whatever/')
+    assert urlparse.urlparse(response['Location']).path == '/whatever/'
 
 
 def test_sso_slo_pass_next_url(db, app, idp, caplog, sp_settings):
@@ -210,3 +211,53 @@ def test_sso_slo_pass_next_url(db, app, idp, caplog, sp_settings):
     assert 'created new user' in caplog.text
     assert 'logged in using SAML' in caplog.text
     assert response['Location'].endswith('/whatever/')
+
+
+def test_sso_artifact_no_loop(db, app, caplog, sp_settings, idp_metadata, idp_private_key, rf):
+    sp_settings.MELLON_DEFAULT_ASSERTION_CONSUMER_BINDING = 'artifact'
+    request = rf.get('/')
+    sp_metadata = create_metadata(request)
+    idp = MockIdp(idp_metadata, idp_private_key, sp_metadata)
+    response = app.get(reverse('mellon_login'))
+    url, body, relay_state = idp.process_authn_request_redirect(response['Location'])
+    assert body is None
+    assert reverse('mellon_login') in url
+    assert 'SAMLart' in url
+    acs_artifact_url = url.split('testserver', 1)[1]
+
+    # forget the artifact
+    idp.artifact = ''
+
+    with HTTMock(idp.mock_artifact_resolver()):
+        response = app.get(acs_artifact_url)
+    assert 'MELLON_RETRY_LOGIN=1;' in response['Set-Cookie']
+
+    # first error, we retry
+    assert urlparse.urlparse(response['Location']).path == reverse('mellon_login')
+
+    # check we are not logged
+    assert not app.session
+
+    # redo
+    response = app.get(reverse('mellon_login'))
+    url, body, relay_state = idp.process_authn_request_redirect(response['Location'])
+    assert body is None
+    assert reverse('mellon_login') in url
+    assert 'SAMLart' in url
+    acs_artifact_url = url.split('testserver', 1)[1]
+
+    # forget the artifact
+    idp.artifact = ''
+    with HTTMock(idp.mock_artifact_resolver()):
+        response = app.get(acs_artifact_url)
+
+    # check cookie is deleted after failed retry
+    # Py3-Dj111 variation
+    assert re.match(r'.*MELLON_RETRY_LOGIN=("")?;', response['Set-Cookie'])
+    assert 'Location' not in response
+
+    # check we are still not logged
+    assert not app.session
+
+    # check return url is in page
+    assert '"%s"' % sp_settings.LOGIN_REDIRECT_URL in response.text