From 27184df690f7f0c26c1de17b5250447b6990b6fc Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Fri, 12 Jun 2020 19:37:06 +0200 Subject: [PATCH 1/6] OldCASAccount model and migration --- authens/migrations/0002_old_cas_account.py | 33 ++++++++++++++++ authens/models.py | 45 +++++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 authens/migrations/0002_old_cas_account.py diff --git a/authens/migrations/0002_old_cas_account.py b/authens/migrations/0002_old_cas_account.py new file mode 100644 index 0000000..b1efde5 --- /dev/null +++ b/authens/migrations/0002_old_cas_account.py @@ -0,0 +1,33 @@ +# Generated by Django 3.0.6 on 2020-06-12 17:26 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('authens', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='OldCASAccount', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('cas_login', models.CharField(max_length=1023, verbose_name='ancien login CAS')), + ('entrance_year', models.SmallIntegerField(verbose_name='année de création du compte CAS')), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='old_cas_account', to=settings.AUTH_USER_MODEL, verbose_name='utilisateurice')), + ], + options={ + 'verbose_name': 'Ancien compte CAS', + 'verbose_name_plural': 'Anciens comptes CAS', + }, + ), + migrations.AddConstraint( + model_name='oldcasaccount', + constraint=models.UniqueConstraint(fields=('cas_login', 'entrance_year'), name='clipper_year_uniqueness'), + ), + ] diff --git a/authens/models.py b/authens/models.py index 90b107c..93a5d61 100644 --- a/authens/models.py +++ b/authens/models.py @@ -8,7 +8,7 @@ User = get_user_model() class CASAccount(models.Model): """Information about CAS accounts. - A user is given an instance of this model iff she has a CAS account. + A user is given an instance of this model iff they have a CAS account. Instances of this model should only be created by the `ENSCASBackend` authentication backend. @@ -37,3 +37,46 @@ class CASAccount(models.Model): "entrance_year": self.entrance_year, "user": self.user.username, } + + +class OldCASAccount(models.Model): + """Information about expired CAS accounts + + A² user is given an instance of this model iff they had a CAS account that expired. + + Instances of this model should not be created with a new user. + """ + + user = models.OneToOneField( + User, + verbose_name=_("utilisateurice"), + on_delete=models.CASCADE, + related_name="old_cas_account", + ) + + cas_login = models.CharField( + verbose_name=_("ancien login CAS"), max_length=1023, blank=False + ) + + entrance_year = models.SmallIntegerField( + verbose_name=_("année de création du compte CAS"), blank=False, null=False + ) + + class Meta: + # `unique_together` to be deprecated soon : we use `constraints` + constraints = [ + models.UniqueConstraint( + fields=["cas_login", "entrance_year"], name="clipper_year_uniqueness", + ) + ] + verbose_name = _("Ancien compte CAS") + verbose_name_plural = _("Anciens comptes CAS") + + def __str__(self): + return _( + "Ancien compte CAS %(cas_login) (promo %(entrance_year)s) lié à %(user)s" + ) % { + "cas_login": self.cas_login, + "entrance_year": self.entrance_year, + "user": self.user.username, + } From 6a84639afe14473e23ce0f206d464c9f48d7c0d3 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Fri, 12 Jun 2020 19:37:20 +0200 Subject: [PATCH 2/6] Auth backend --- authens/backends.py | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/authens/backends.py b/authens/backends.py index fcfd57a..92df47e 100644 --- a/authens/backends.py +++ b/authens/backends.py @@ -1,7 +1,7 @@ from django.contrib.auth import get_user_model from django.db import transaction -from authens.models import CASAccount +from authens.models import CASAccount, OldCASAccount from authens.utils import get_cas_client UserModel = get_user_model() @@ -101,8 +101,8 @@ class ENSCASBackend: """Handles account retrieval, creation and invalidation as described above. - If no CAS account exists, create one; - - If a CAS account exists, but with the wrong entrance year, remove it and - create a new one; + - If a CAS account exists, but with the wrong entrance year, deprecate it + into an OldCASAccount; - If a matching CAS account exists, retrieve it. """ @@ -113,6 +113,11 @@ class ENSCASBackend: try: user = UserModel.objects.get(cas_account__cas_login=cas_login) if user.cas_account.entrance_year != entrance_year: + OldCASAccount.objects.create( + user=user, + entrance_year=user.cas_account.entrance_year, + cas_login=cas_login, + ) user.cas_account.delete() user = None except UserModel.DoesNotExist: @@ -132,3 +137,27 @@ class ENSCASBackend: return UserModel.objects.get(pk=user_id) except UserModel.DoesNotExist: return None + + +class OldCASBackend: + def authenticate(self, request, cas_login=None, password=None, entrance_year=None): + if cas_login is None or password is None or entrance_year is None: + return + + try: + old_cas_acc = OldCASAccount.objects.get( + cas_login=cas_login, entrance_year=entrance_year + ) + user = old_cas_acc.user + except OldCASAccount.DoesNotExist: + # As in Django's ModelBackend, we run the password hasher once + # to avoid timing attacks + UserModel().set_password(password) + else: + if user.check_password(password) and self.user_can_authenticate(user): + return user + + def user_can_authenticate(self, user): + # Taken from Django's ModelBackend + is_active = getattr(user, "is_active", None) + return is_active or is_active is None From 35eb6f983365156947cbf4afa7ea5cd026fa29f1 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Fri, 12 Jun 2020 20:39:09 +0200 Subject: [PATCH 3/6] Tests for backend --- authens/tests/test_backend.py | 27 +++++++++++++++++++++++++-- tests/settings.py | 15 ++++++++------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/authens/tests/test_backend.py b/authens/tests/test_backend.py index 6c77c6d..486db62 100644 --- a/authens/tests/test_backend.py +++ b/authens/tests/test_backend.py @@ -4,7 +4,7 @@ from django.contrib.auth import authenticate, get_user_model from django.test import TestCase from authens.backends import ENSCASBackend -from authens.models import CASAccount +from authens.models import CASAccount, OldCASAccount from authens.tests.cas_utils import FakeCASClient UserModel = get_user_model() @@ -94,8 +94,31 @@ class TestCASBackend(TestCase): # Log the new 'johndoe' in. new_user = authenticate(None, ticket="dummy ticket") new_clipper = new_user.cas_account - # Check that it gets a fresh user and a fresh clipper account. + + # Check that it gets a fresh user and a fresh clipper account self.assertNotEqual(old_user, new_user) self.assertNotEqual(old_clipper, new_clipper) + + # Check that the created CAS account matches the old one self.assertEqual(new_clipper.cas_login, fake_cas_client.cas_login) self.assertEqual(new_clipper.entrance_year, fake_cas_client.entrance_year) + + # Check deprecation of the old CAS account + old_user.refresh_from_db() + self.assertFalse(hasattr(old_user, "cas_account")) + self.assertTrue(hasattr(old_user, "old_cas_account")) + old_cas = old_user.old_cas_account + self.assertEqual(old_cas.cas_login, old_clipper.cas_login) + self.assertEqual(old_cas.entrance_year, old_clipper.entrance_year) + + +class TestOldCASBackend(TestCase): + def test_simple_auth(self): + user = UserModel.objects.create_user(username="johndoe31", password="password") + wrong_user = UserModel.objects.create_user("johndoe", "password") + OldCASAccount.objects.create(user=user, cas_login="johndoe", entrance_year=2019) + + auth_user = authenticate( + None, cas_login="johndoe", entrance_year=2019, password="password" + ) + self.assertEqual(auth_user, user) diff --git a/tests/settings.py b/tests/settings.py index 82c5d96..1bcad09 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -16,16 +16,17 @@ INSTALLED_APPS = [ AUTHENTICATION_BACKENDS = [ "django.contrib.auth.backends.ModelBackend", "authens.backends.ENSCASBackend", + "authens.backends.OldCASBackend", ] 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', + "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 = [ From bb3aac63b9b7f0ae6393cada2901b23969c94446 Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Fri, 12 Jun 2020 20:40:20 +0200 Subject: [PATCH 4/6] typo --- authens/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authens/models.py b/authens/models.py index 93a5d61..9408167 100644 --- a/authens/models.py +++ b/authens/models.py @@ -42,7 +42,7 @@ class CASAccount(models.Model): class OldCASAccount(models.Model): """Information about expired CAS accounts - A² user is given an instance of this model iff they had a CAS account that expired. + A user is given an instance of this model iff they had a CAS account that expired. Instances of this model should not be created with a new user. """ From 0d4f848fe59c48deffdbe74b4c813c6b0d7affcc Mon Sep 17 00:00:00 2001 From: Ludovic Stephan Date: Sat, 13 Jun 2020 17:34:02 +0200 Subject: [PATCH 5/6] Better doc --- authens/backends.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/authens/backends.py b/authens/backends.py index 92df47e..4b489fa 100644 --- a/authens/backends.py +++ b/authens/backends.py @@ -101,8 +101,8 @@ class ENSCASBackend: """Handles account retrieval, creation and invalidation as described above. - If no CAS account exists, create one; - - If a CAS account exists, but with the wrong entrance year, deprecate it - into an OldCASAccount; + - If a CAS account exists, but with the wrong entrance year, convert it to + an OldCASAccount instance, and create a fresh CAS Account with the correct year. - If a matching CAS account exists, retrieve it. """ @@ -140,6 +140,13 @@ class ENSCASBackend: class OldCASBackend: + """Authentication backend for old CAS accounts. + + Given a CAS login, an entrance year and a password, first finds the matching + OldCASAccount instance (if it exists), then checks the given password with + the user associated to this account. + """ + def authenticate(self, request, cas_login=None, password=None, entrance_year=None): if cas_login is None or password is None or entrance_year is None: return @@ -151,7 +158,7 @@ class OldCASBackend: user = old_cas_acc.user except OldCASAccount.DoesNotExist: # As in Django's ModelBackend, we run the password hasher once - # to avoid timing attacks + # to mitigate timing attacks UserModel().set_password(password) else: if user.check_password(password) and self.user_can_authenticate(user): From 5047ee14de894d4b4cd5fa5dfa5508b3dae60753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sat, 13 Jun 2020 18:17:51 +0200 Subject: [PATCH 6/6] Black --- authens/migrations/0002_old_cas_account.py | 46 +++++++++++++++++----- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/authens/migrations/0002_old_cas_account.py b/authens/migrations/0002_old_cas_account.py index b1efde5..defdac9 100644 --- a/authens/migrations/0002_old_cas_account.py +++ b/authens/migrations/0002_old_cas_account.py @@ -9,25 +9,51 @@ class Migration(migrations.Migration): dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('authens', '0001_initial'), + ("authens", "0001_initial"), ] operations = [ migrations.CreateModel( - name='OldCASAccount', + name="OldCASAccount", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('cas_login', models.CharField(max_length=1023, verbose_name='ancien login CAS')), - ('entrance_year', models.SmallIntegerField(verbose_name='année de création du compte CAS')), - ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='old_cas_account', to=settings.AUTH_USER_MODEL, verbose_name='utilisateurice')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "cas_login", + models.CharField(max_length=1023, verbose_name="ancien login CAS"), + ), + ( + "entrance_year", + models.SmallIntegerField( + verbose_name="année de création du compte CAS" + ), + ), + ( + "user", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + related_name="old_cas_account", + to=settings.AUTH_USER_MODEL, + verbose_name="utilisateurice", + ), + ), ], options={ - 'verbose_name': 'Ancien compte CAS', - 'verbose_name_plural': 'Anciens comptes CAS', + "verbose_name": "Ancien compte CAS", + "verbose_name_plural": "Anciens comptes CAS", }, ), migrations.AddConstraint( - model_name='oldcasaccount', - constraint=models.UniqueConstraint(fields=('cas_login', 'entrance_year'), name='clipper_year_uniqueness'), + model_name="oldcasaccount", + constraint=models.UniqueConstraint( + fields=("cas_login", "entrance_year"), name="clipper_year_uniqueness" + ), ), ]