From 9d6548170771b017443559d2d6e3fe92895c433e Mon Sep 17 00:00:00 2001 From: Antoine LAURENT Date: Mon, 2 Sep 2024 15:08:55 +0200 Subject: [PATCH] perms: Force logout in midleware instead of redirecting to logout page --- itou/utils/perms/middleware.py | 7 ++++-- .../openid_connect/inclusion_connect/tests.py | 17 ++++++++++---- tests/utils/tests.py | 6 ++--- .../www/companies_views/test_members_views.py | 6 +++-- tests/www/dashboard/test_dashboard.py | 7 +++--- .../invitations_views/test_company_accept.py | 22 +++++++++++++++---- 6 files changed, 47 insertions(+), 18 deletions(-) diff --git a/itou/utils/perms/middleware.py b/itou/utils/perms/middleware.py index b56269f2a2..94d911a5b2 100644 --- a/itou/utils/perms/middleware.py +++ b/itou/utils/perms/middleware.py @@ -1,7 +1,7 @@ +from allauth.account.adapter import get_adapter from django.conf import settings from django.contrib import messages from django.http import HttpResponseRedirect -from django.shortcuts import redirect from django.urls import reverse from django.utils.safestring import mark_safe @@ -165,6 +165,9 @@ def __call__(self, request): if redirect_message is not None: messages.warning(request, redirect_message) - return redirect("account_logout") + adapter = get_adapter(request) + redirect_url = adapter.get_logout_redirect_url(request) + adapter.logout(request) + return HttpResponseRedirect(redirect_url) return self.get_response(request) diff --git a/tests/openid_connect/inclusion_connect/tests.py b/tests/openid_connect/inclusion_connect/tests.py index 15904ace1b..ab803c715a 100644 --- a/tests/openid_connect/inclusion_connect/tests.py +++ b/tests/openid_connect/inclusion_connect/tests.py @@ -2,11 +2,12 @@ import json from operator import itemgetter from unittest import mock -from urllib.parse import quote, urlencode +from urllib.parse import parse_qs, quote, urlencode, urlparse import httpx import pytest import respx +from django.conf import settings from django.contrib import auth, messages from django.contrib.auth import get_user from django.contrib.messages import Message @@ -97,9 +98,6 @@ def mock_oauth_dance( else: assert response.url.startswith(constants.INCLUSION_CONNECT_ENDPOINT_AUTHORIZE) - # User is logged out from IC when an error happens during the oauth dance. - respx.get(constants.INCLUSION_CONNECT_ENDPOINT_LOGOUT).respond(200) - token_json = {"access_token": "access_token", "token_type": "Bearer", "expires_in": 60, "id_token": "123456"} respx.post(constants.INCLUSION_CONNECT_ENDPOINT_TOKEN).mock(return_value=httpx.Response(200, json=token_json)) @@ -119,6 +117,17 @@ def mock_oauth_dance( return response +def assert_and_mock_forced_logout(client, response, expected_redirect_url=reverse("search:employers_home")): + response = client.get(response.url) + assertRedirects(response, reverse("inclusion_connect:logout") + "?token=123456", fetch_redirect_response=False) + response = client.get(response.url) + assert response.url.startswith(constants.INCLUSION_CONNECT_ENDPOINT_LOGOUT) + post_logout_redirect_uri = parse_qs(urlparse(response.url).query)["post_logout_redirect_uri"][0] + local_url = post_logout_redirect_uri.split(settings.ITOU_FQDN)[1] + assert local_url == expected_redirect_url + return response + + class InclusionConnectModelTest(InclusionConnectBaseTestCase): def test_state_delete(self): state = InclusionConnectState.objects.create(state="foo") diff --git a/tests/utils/tests.py b/tests/utils/tests.py index 2effefb23c..eff1ed2a1c 100644 --- a/tests/utils/tests.py +++ b/tests/utils/tests.py @@ -159,7 +159,7 @@ def test_siae_no_member(self, mocked_get_response_for_middlewaremixin): with assertNumQueries(1): # Retrieve user memberships response = ItouCurrentOrganizationMiddleware(get_response_for_middlewaremixin)(request) assert mocked_get_response_for_middlewaremixin.call_count == 0 - assertRedirects(response, reverse("account_logout"), fetch_redirect_response=False) + assertRedirects(response, reverse("search:employers_home"), fetch_redirect_response=False) assert list(messages.get_messages(request)) == [ messages.Message( messages.WARNING, @@ -237,7 +237,7 @@ def test_employer_of_inactive_siae(self, mocked_get_response_for_middlewaremixin ): response = ItouCurrentOrganizationMiddleware(mocked_get_response_for_middlewaremixin)(request) assert mocked_get_response_for_middlewaremixin.call_count == 0 - assertRedirects(response, reverse("account_logout"), fetch_redirect_response=False) + assertRedirects(response, reverse("search:employers_home"), fetch_redirect_response=False) assert list(messages.get_messages(request)) == [ messages.Message( messages.WARNING, @@ -433,7 +433,7 @@ def test_labor_inspector_no_member(self, mocked_get_response_for_middlewaremixin with assertNumQueries(1): # retrieve user memberships response = ItouCurrentOrganizationMiddleware(mocked_get_response_for_middlewaremixin)(request) assert mocked_get_response_for_middlewaremixin.call_count == 0 - assertRedirects(response, reverse("account_logout"), fetch_redirect_response=False) + assertRedirects(response, reverse("search:employers_home"), fetch_redirect_response=False) assert list(messages.get_messages(request)) == [ messages.Message( messages.WARNING, diff --git a/tests/www/companies_views/test_members_views.py b/tests/www/companies_views/test_members_views.py index 4db6521cb2..b5e184e711 100644 --- a/tests/www/companies_views/test_members_views.py +++ b/tests/www/companies_views/test_members_views.py @@ -1,4 +1,5 @@ import pytest +from django.contrib.auth import get_user from django.core import mail from django.urls import reverse from django.urls.exceptions import NoReverseMatch @@ -159,9 +160,10 @@ def test_user_with_no_company_left(self): url = reverse("dashboard:index") response = self.client.get(url) - # should be redirected to logout + # should be redirected to home page and logged out assert response.status_code == 302 - assert response.url == reverse("account_logout") + assert response.url == reverse("search:employers_home") + assert not get_user(self.client).is_authenticated def test_structure_selector(self): """ diff --git a/tests/www/dashboard/test_dashboard.py b/tests/www/dashboard/test_dashboard.py index 4528200b71..5502409ea7 100644 --- a/tests/www/dashboard/test_dashboard.py +++ b/tests/www/dashboard/test_dashboard.py @@ -4,6 +4,7 @@ import pytest from dateutil.relativedelta import relativedelta from django.conf import settings +from django.contrib.auth import get_user from django.test import override_settings from django.urls import reverse from django.utils import timezone @@ -103,10 +104,10 @@ def test_user_with_inactive_company_cannot_login_after_grace_period(self): url = reverse("dashboard:index") response = self.client.get(url, follow=True) + # Should be redirected to home page and logged out + self.assertRedirects(response, reverse("search:employers_home")) assert response.status_code == 200 - last_url = response.redirect_chain[-1][0] - assert last_url == reverse("account_logout") - + assert not get_user(self.client).is_authenticated expected_message = "votre compte n'est malheureusement plus actif" self.assertContains(response, expected_message) diff --git a/tests/www/invitations_views/test_company_accept.py b/tests/www/invitations_views/test_company_accept.py index 4e71be8aff..330c9eb5f5 100644 --- a/tests/www/invitations_views/test_company_accept.py +++ b/tests/www/invitations_views/test_company_accept.py @@ -3,6 +3,7 @@ import respx from django.conf import settings from django.contrib import messages +from django.contrib.auth import get_user from django.contrib.messages.test import MessagesTestMixin from django.core import mail from django.shortcuts import reverse @@ -16,7 +17,7 @@ from tests.companies.factories import CompanyFactory from tests.invitations.factories import ExpiredEmployerInvitationFactory, SentEmployerInvitationFactory from tests.openid_connect.inclusion_connect.test import InclusionConnectBaseTestCase -from tests.openid_connect.inclusion_connect.tests import OIDC_USERINFO, mock_oauth_dance +from tests.openid_connect.inclusion_connect.tests import OIDC_USERINFO, assert_and_mock_forced_logout, mock_oauth_dance from tests.prescribers.factories import PrescriberOrganizationWithMembershipFactory from tests.users.factories import EmployerFactory from tests.utils.test import ItouClient @@ -337,9 +338,22 @@ def test_accept_new_user_to_inactive_siae(self): previous_url=previous_url, next_url=next_url, ) - # Follow the redirection. - response = self.client.get(response.url, follow=True) - self.assertContains(response, escape("Cette structure n'est plus active.")) + # response redirects to /invitation/uid/join-company + response = self.client.get(response.url) + # company is inactive so the user will be logged out + response = assert_and_mock_forced_logout(self.client, response) + assert not get_user(self.client).is_authenticated + self.assertMessages( + response, + [ + messages.Message(messages.ERROR, "Cette structure n'est plus active."), + messages.Message( + messages.WARNING, + "Nous sommes désolés, votre compte n'est actuellement rattaché à aucune structure." + "
Nous espérons cependant avoir l'occasion de vous accueillir de nouveau.", + ), + ], + ) user = User.objects.get(email=invitation.email) assert user.company_set.count() == 0