From d19ac19469f2b773d1e49e88312cdd197fe72dc2 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Wed, 23 Nov 2022 11:55:45 +0100
Subject: [PATCH] auth_fc: close FranceConnect session when linking fails
 (#71607)

---
 src/authentic2_auth_fc/views.py | 25 +++++++++++++++----------
 tests/auth_fc/test_auth_fc.py   |  6 ++++++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/authentic2_auth_fc/views.py b/src/authentic2_auth_fc/views.py
index 8514b7dc8..e9755a086 100644
--- a/src/authentic2_auth_fc/views.py
+++ b/src/authentic2_auth_fc/views.py
@@ -124,6 +124,15 @@ class LoginOrLinkView(View):
     def redirect(self):
         return utils_misc.redirect(self.request, self.next_url)
 
+    def logout_and_redirect(self):
+        url = utils.build_logout_url(self.request, self.authenticator.logout_url, next_url=self.next_url)
+        if url:
+            clean_fc_session(self.request.session)
+            response = utils_misc.redirect(self.request, url, resolve=False)
+            response.display_message = False
+            return response
+        return self.redirect()
+
     @property
     def fc_display_name(self):
         '''Human representation of the current FC account'''
@@ -228,6 +237,10 @@ class LoginOrLinkView(View):
         # clear FranceConnect down status
         cache.delete('fc_is_down')
 
+        # keep id_token around for logout
+        request.session['fc_id_token'] = self.id_token
+        request.session['fc_id_token_raw'] = self.token['id_token']
+
         if request.user.is_authenticated:
             return self.link(request)
         else:
@@ -334,10 +347,6 @@ class LoginOrLinkView(View):
 
     def link(self, request):
         '''Request an access grant code and associate it to the current user'''
-        # keep id_token around for logout
-        request.session['fc_id_token'] = self.id_token
-        request.session['fc_id_token_raw'] = self.token['id_token']
-
         try:
             self.fc_account, created = models.FcAccount.objects.get_or_create(
                 sub=self.sub,
@@ -377,7 +386,7 @@ class LoginOrLinkView(View):
             created = False
 
         if not user:
-            return self.redirect()
+            return self.logout_and_redirect()
 
         return self.finish_login(request, user, self.user_info, created)
 
@@ -386,10 +395,6 @@ class LoginOrLinkView(View):
         utils_views.check_cookie_works(request)
         utils_misc.login(request, user, 'france-connect')
 
-        # keep id_token around for logout
-        request.session['fc_id_token'] = self.id_token
-        request.session['fc_id_token_raw'] = self.token['id_token']
-
         # set session expiration policy to EXPIRE_AT_BROWSER_CLOSE
         request.session.set_expiry(0)
 
@@ -517,7 +522,7 @@ class LoginOrLinkView(View):
                     self.fc_display_name
                 ),
             )
-        return self.redirect()
+        return self.logout_and_redirect()
 
     def update_user_info(self, user, user_info):
         # always handle given_name and family_name
diff --git a/tests/auth_fc/test_auth_fc.py b/tests/auth_fc/test_auth_fc.py
index 37ed0194d..a3852306d 100644
--- a/tests/auth_fc/test_auth_fc.py
+++ b/tests/auth_fc/test_auth_fc.py
@@ -262,6 +262,8 @@ def test_login_email_is_unique_and_already_linked(settings, app, franceconnect,
         cookie = cookie[0].message
     assert 'is already used' in cookie
     assert '_auth_user_id' not in app.session
+    response = franceconnect.handle_logout(app, response.location)
+    assert response.location == '/idp/'
 
 
 def test_requests_proxies_support(settings, app, monkeypatch):
@@ -466,6 +468,8 @@ def test_multiple_accounts_with_same_email(settings, app, franceconnect):
     User.objects.create(email=franceconnect.user_info['email'], ou=ou)
 
     response = franceconnect.login_with_fc(app, path='/accounts/')
+    response = franceconnect.handle_logout(app, response.location)
+    assert response.location == '/accounts/'
 
     response = response.maybe_follow()
     assert 'is already used by another' in response
@@ -657,6 +661,8 @@ def test_same_email_different_sub(app, franceconnect):
 
     resp = franceconnect.login_with_fc_fixed_params(app)
 
+    resp = franceconnect.handle_logout(app, resp.location)
+
     resp = resp.maybe_follow()
     # email collision, sub is different, no new user created
     assert User.objects.count() == 1