From 09ad5b6657a5cc0a5d935b8a8a6cb93bde652146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 13 May 2020 13:57:26 +0200 Subject: [PATCH 1/5] redirect CAS-users to CAS_URL/logout at logout --- authens/backends.py | 5 +++- .../0002_casaccount_connected_to_cas.py | 18 +++++++++++++ authens/models.py | 5 ++++ authens/urls.py | 3 +-- authens/views.py | 26 ++++++++++++++++++- 5 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 authens/migrations/0002_casaccount_connected_to_cas.py diff --git a/authens/backends.py b/authens/backends.py index 6033aad..40080ac 100644 --- a/authens/backends.py +++ b/authens/backends.py @@ -60,7 +60,10 @@ class ENSCASBackend: cas_login = self.clean_cas_login(cas_login) year = get_entrance_year(attributes) - return self._get_or_create(cas_login, year) + user = self._get_or_create(cas_login, year) + user.cas_account.connected_to_cas = True + user.cas_account.save() + return user def clean_cas_login(self, cas_login): return cas_login.strip().lower() diff --git a/authens/migrations/0002_casaccount_connected_to_cas.py b/authens/migrations/0002_casaccount_connected_to_cas.py new file mode 100644 index 0000000..84910a3 --- /dev/null +++ b/authens/migrations/0002_casaccount_connected_to_cas.py @@ -0,0 +1,18 @@ +# Generated by Django 3.0.6 on 2020-05-17 12:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('authens', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='casaccount', + name='connected_to_cas', + field=models.BooleanField(default=False, editable=False), + ), + ] diff --git a/authens/models.py b/authens/models.py index 90b107c..01699fd 100644 --- a/authens/models.py +++ b/authens/models.py @@ -27,6 +27,11 @@ class CASAccount(models.Model): verbose_name=_("année de création du compte CAS"), blank=False, null=False ) + # This is True if and only if the user is connected via CAS (and not e.g. by + # password). This is used to decide whether to redirect to user to the CAS logout + # page or not when the user disconnects. + connected_to_cas = models.BooleanField(default=False, editable=False) + class Meta: verbose_name = _("Compte CAS") verbose_name_plural = _("Comptes CAS") diff --git a/authens/urls.py b/authens/urls.py index f28100f..7b13310 100644 --- a/authens/urls.py +++ b/authens/urls.py @@ -1,4 +1,3 @@ -from django.contrib.auth import views as auth_views from django.urls import path from authens import views @@ -8,5 +7,5 @@ urlpatterns = [ path("login/choose", views.LoginSwitchView.as_view(), name="login"), path("login/cas", views.CASLoginView.as_view(), name="login.cas"), path("login/pwd", views.PasswordLoginView.as_view(), name="login.pwd"), - path("logout", auth_views.LogoutView.as_view(), name="logout"), + path("logout", views.LogoutView.as_view(), name="logout"), ] diff --git a/authens/views.py b/authens/views.py index 06b80d5..58152a0 100644 --- a/authens/views.py +++ b/authens/views.py @@ -1,10 +1,12 @@ from django.conf import settings from django.contrib import auth +from django.contrib.auth import views as auth_views from django.core.exceptions import PermissionDenied from django.views.generic import TemplateView, View from django.shortcuts import redirect from django.utils.translation import gettext_lazy as _ +from authens.models import CASAccount from authens.utils import get_cas_client @@ -72,5 +74,27 @@ class CASLoginView(NextPageMixin, View): return redirect(self.get_next_url()) -class PasswordLoginView(auth.views.LoginView): +class PasswordLoginView(auth_views.LoginView): template_name = "authens/pwd_login.html" + + +class LogoutView(auth_views.LogoutView): + """Logout view of AuthENS. + + Tell Django to log the user out, then redirect to the CAS logout page if the user + logged in via CAS. + """ + + def setup(self, *args, **kwargs): + super().setup(*args, **kwargs) + cas_account = CASAccount.objects.filter(user=self.request.user) + self.cas_account = cas_account.get() if cas_account.exists() else None + + def get_next_page(self): + if self.cas_account and self.cas_account.connected_to_cas: + cas_client = get_cas_client(self.request) + self.cas_account.connected_to_cas = False + self.cas_account.save() + return cas_client.get_logout_url() + else: + return super().get_next_page() From ead851893ad10aae01f4918faa565837c88ade9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sun, 17 May 2020 17:36:15 +0200 Subject: [PATCH 2/5] Test the logout view --- .../0002_casaccount_connected_to_cas.py | 6 +- authens/tests/test_views.py | 63 +++++++++++++++++++ tests/settings.py | 29 +++++++++ tests/urls.py | 4 ++ 4 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 authens/tests/test_views.py create mode 100644 tests/urls.py diff --git a/authens/migrations/0002_casaccount_connected_to_cas.py b/authens/migrations/0002_casaccount_connected_to_cas.py index 84910a3..9559757 100644 --- a/authens/migrations/0002_casaccount_connected_to_cas.py +++ b/authens/migrations/0002_casaccount_connected_to_cas.py @@ -6,13 +6,13 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('authens', '0001_initial'), + ("authens", "0001_initial"), ] operations = [ migrations.AddField( - model_name='casaccount', - name='connected_to_cas', + model_name="casaccount", + name="connected_to_cas", field=models.BooleanField(default=False, editable=False), ), ] diff --git a/authens/tests/test_views.py b/authens/tests/test_views.py new file mode 100644 index 0000000..4ad5176 --- /dev/null +++ b/authens/tests/test_views.py @@ -0,0 +1,63 @@ +from unittest.mock import patch + +from django.conf import settings +from django.contrib.auth import get_user_model +from django.contrib.sessions.models import Session +from django.test import Client, TestCase + +from authens.models import CASAccount +from authens.tests.cas_utils import FakeCASClient + +UserModel = get_user_model() + + +class TestLogoutView(TestCase): + def test_regular_logout(self): + # Regular user (without a CAS account) + user = UserModel.objects.create_user(username="johndoe") + + # Log the user in + client = Client() + client.force_login(user) + + self.assertEqual(Session.objects.count(), 1) + response = client.get("/authens/logout") + self.assertEqual(Session.objects.count(), 0) # User is actually logged out. + self.assertRedirects(response, settings.LOGOUT_REDIRECT_URL) + + @patch("authens.backends.get_cas_client") + def test_cas_logout(self, mock_cas_client): + # Make `get_cas_client` return a dummy CAS client that skips ticket verification + # and always log in a user with CAS login 'johndoe'. + # This is only used for login. + mock_cas_client.return_value = FakeCASClient("johndoe", 2019) + + # CAS user + user = UserModel.objects.create_user(username="johndoe") + CASAccount.objects.create(user=user, cas_login="johndoe", entrance_year=2019) + + # Log the user in via CAS + client = Client() + client.login(ticket="dummy ticket") + + self.assertEqual(Session.objects.count(), 1) + response = client.get("/authens/logout") + self.assertEqual(Session.objects.count(), 0) # User is logged out… + self.assertRedirects( # … and redirected to the CAS logout page. + response, "https://cas.eleves.ens.fr/logout", fetch_redirect_response=False + ) + + def test_regular_logout_on_cas_account(self): + # CAS user + user = UserModel.objects.create_user(username="johndoe", password="p4ssw0rd") + CASAccount.objects.create(user=user, cas_login="johndoe", entrance_year=2019) + + # Log the user in by password and *not* via CAS + client = Client() + client.login(username="johndoe", password="p4ssw0rd") + + self.assertEqual(Session.objects.count(), 1) + response = client.get("/authens/logout") + self.assertEqual(Session.objects.count(), 0) # User is logged out… + # … and not redirected to the CAS logout page. + self.assertRedirects(response, settings.LOGOUT_REDIRECT_URL) diff --git a/tests/settings.py b/tests/settings.py index 999e477..04168fd 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -8,6 +8,7 @@ SECRET_KEY = "dummy" INSTALLED_APPS = [ "django.contrib.contenttypes", "django.contrib.auth", + "django.contrib.sessions", "authens", "tests", ] @@ -17,6 +18,34 @@ AUTHENTICATION_BACKENDS = [ "authens.backends.ENSCASBackend", ] +MIDDLEWARE = [ + 'django.middleware.security.SecurityMiddleware', + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.middleware.common.CommonMiddleware', + 'django.middleware.csrf.CsrfViewMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'django.contrib.messages.middleware.MessageMiddleware', + 'django.middleware.clickjacking.XFrameOptionsMiddleware', +] + +TEMPLATES = [ + { + "BACKEND": "django.template.backends.django.DjangoTemplates", + "DIRS": [], + "APP_DIRS": True, + "OPTIONS": { + "context_processors": [ + "django.template.context_processors.debug", + "django.template.context_processors.request", + "django.contrib.auth.context_processors.auth", + "django.contrib.messages.context_processors.messages", + ], + }, + }, +] + DATABASES = {"default": {"ENGINE": "django.db.backends.sqlite3"}} +ROOT_URLCONF = "tests.urls" LOGIN_URL = reverse_lazy("authens:login") +LOGOUT_REDIRECT_URL = reverse_lazy("authens:login") diff --git a/tests/urls.py b/tests/urls.py new file mode 100644 index 0000000..2b6ab10 --- /dev/null +++ b/tests/urls.py @@ -0,0 +1,4 @@ +from django.urls import include, path + + +urlpatterns = [path("authens/", include("authens.urls"))] From 6a75f78541fdd5028d076e7d3edd45d17148a44a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sun, 17 May 2020 18:50:54 +0200 Subject: [PATCH 3/5] Store the connexion method in a session variable --- authens/backends.py | 7 +++---- .../0002_casaccount_connected_to_cas.py | 18 ------------------ authens/models.py | 5 ----- authens/tests/test_views.py | 2 +- authens/views.py | 16 ++++++++-------- 5 files changed, 12 insertions(+), 36 deletions(-) delete mode 100644 authens/migrations/0002_casaccount_connected_to_cas.py diff --git a/authens/backends.py b/authens/backends.py index 40080ac..d5d1b74 100644 --- a/authens/backends.py +++ b/authens/backends.py @@ -60,10 +60,9 @@ class ENSCASBackend: cas_login = self.clean_cas_login(cas_login) year = get_entrance_year(attributes) - user = self._get_or_create(cas_login, year) - user.cas_account.connected_to_cas = True - user.cas_account.save() - return user + if request: + request.session["CASCONNECTED"] = True + return self._get_or_create(cas_login, year) def clean_cas_login(self, cas_login): return cas_login.strip().lower() diff --git a/authens/migrations/0002_casaccount_connected_to_cas.py b/authens/migrations/0002_casaccount_connected_to_cas.py deleted file mode 100644 index 9559757..0000000 --- a/authens/migrations/0002_casaccount_connected_to_cas.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 3.0.6 on 2020-05-17 12:23 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("authens", "0001_initial"), - ] - - operations = [ - migrations.AddField( - model_name="casaccount", - name="connected_to_cas", - field=models.BooleanField(default=False, editable=False), - ), - ] diff --git a/authens/models.py b/authens/models.py index 01699fd..90b107c 100644 --- a/authens/models.py +++ b/authens/models.py @@ -27,11 +27,6 @@ class CASAccount(models.Model): verbose_name=_("année de création du compte CAS"), blank=False, null=False ) - # This is True if and only if the user is connected via CAS (and not e.g. by - # password). This is used to decide whether to redirect to user to the CAS logout - # page or not when the user disconnects. - connected_to_cas = models.BooleanField(default=False, editable=False) - class Meta: verbose_name = _("Compte CAS") verbose_name_plural = _("Comptes CAS") diff --git a/authens/tests/test_views.py b/authens/tests/test_views.py index 4ad5176..25dddb8 100644 --- a/authens/tests/test_views.py +++ b/authens/tests/test_views.py @@ -38,7 +38,7 @@ class TestLogoutView(TestCase): # Log the user in via CAS client = Client() - client.login(ticket="dummy ticket") + client.get("/authens/login/cas?ticket=dummy-ticket") self.assertEqual(Session.objects.count(), 1) response = client.get("/authens/logout") diff --git a/authens/views.py b/authens/views.py index 58152a0..da17c48 100644 --- a/authens/views.py +++ b/authens/views.py @@ -6,7 +6,6 @@ from django.views.generic import TemplateView, View from django.shortcuts import redirect from django.utils.translation import gettext_lazy as _ -from authens.models import CASAccount from authens.utils import get_cas_client @@ -85,16 +84,17 @@ class LogoutView(auth_views.LogoutView): logged in via CAS. """ - def setup(self, *args, **kwargs): - super().setup(*args, **kwargs) - cas_account = CASAccount.objects.filter(user=self.request.user) - self.cas_account = cas_account.get() if cas_account.exists() else None + def setup(self, request): + super().setup(request) + if "CASCONNECTED" in request.session: + del request.session["CASCONNECTED"] + self.cas_connected = True + else: + self.cas_connected = False def get_next_page(self): - if self.cas_account and self.cas_account.connected_to_cas: + if self.cas_connected: cas_client = get_cas_client(self.request) - self.cas_account.connected_to_cas = False - self.cas_account.save() return cas_client.get_logout_url() else: return super().get_next_page() From 8e95a0164722c625843fd5a7c1dea3e9477f1679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 10 Jun 2020 22:22:13 +0200 Subject: [PATCH 4/5] Redirect after a CAS logout --- authens/tests/test_views.py | 8 +++++++- authens/views.py | 11 ++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/authens/tests/test_views.py b/authens/tests/test_views.py index 25dddb8..94d27c2 100644 --- a/authens/tests/test_views.py +++ b/authens/tests/test_views.py @@ -1,9 +1,11 @@ from unittest.mock import patch +from urllib.parse import quote as urlquote from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.sessions.models import Session from django.test import Client, TestCase +from django.urls import reverse from authens.models import CASAccount from authens.tests.cas_utils import FakeCASClient @@ -44,7 +46,11 @@ class TestLogoutView(TestCase): response = client.get("/authens/logout") self.assertEqual(Session.objects.count(), 0) # User is logged out… self.assertRedirects( # … and redirected to the CAS logout page. - response, "https://cas.eleves.ens.fr/logout", fetch_redirect_response=False + response, + "https://cas.eleves.ens.fr/logout?service={}".format( + urlquote("http://testserver" + reverse("authens:login")) + ), + fetch_redirect_response=False, ) def test_regular_logout_on_cas_account(self): diff --git a/authens/views.py b/authens/views.py index da17c48..194814f 100644 --- a/authens/views.py +++ b/authens/views.py @@ -1,3 +1,5 @@ +from urllib.parse import urlunparse + from django.conf import settings from django.contrib import auth from django.contrib.auth import views as auth_views @@ -93,8 +95,11 @@ class LogoutView(auth_views.LogoutView): self.cas_connected = False def get_next_page(self): + next_page = super().get_next_page() if self.cas_connected: cas_client = get_cas_client(self.request) - return cas_client.get_logout_url() - else: - return super().get_next_page() + redirect_url = urlunparse( + (self.request.scheme, self.request.get_host(), next_page, "", "", "") + ) + next_page = cas_client.get_logout_url(redirect_url=redirect_url) + return next_page From 352fedb8b2047e914d973335eabe5d7abaf002c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Wed, 10 Jun 2020 23:23:33 +0200 Subject: [PATCH 5/5] Logout next url can be local or absolute --- authens/views.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/authens/views.py b/authens/views.py index 194814f..14ead4e 100644 --- a/authens/views.py +++ b/authens/views.py @@ -1,4 +1,4 @@ -from urllib.parse import urlunparse +from urllib.parse import urlparse, urlunparse from django.conf import settings from django.contrib import auth @@ -98,8 +98,14 @@ class LogoutView(auth_views.LogoutView): next_page = super().get_next_page() if self.cas_connected: cas_client = get_cas_client(self.request) - redirect_url = urlunparse( - (self.request.scheme, self.request.get_host(), next_page, "", "", "") - ) - next_page = cas_client.get_logout_url(redirect_url=redirect_url) + + # If the next_url is local (no hostname), make it absolute so that the user + # is correctly redirected from CAS. + if not urlparse(next_page).netloc: + request = self.request + next_page = urlunparse( + (request.scheme, request.get_host(), next_page, "", "", "") + ) + + next_page = cas_client.get_logout_url(redirect_url=next_page) return next_page