diff --git a/bda/tests/test_views.py b/bda/tests/test_views.py index 6bfa3257..b4083c30 100644 --- a/bda/tests/test_views.py +++ b/bda/tests/test_views.py @@ -8,6 +8,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.core.management import call_command from django.test import Client, TestCase +from django.urls import reverse from django.utils import timezone from ..models import CategorieSpectacle, Salle, Spectacle, Tirage @@ -63,14 +64,10 @@ class BdATestHelpers: def craft_redirect_url(user): if redirect_url: return redirect_url - elif user is None: - # client is not logged in - login_url = "/login" - if url: - login_url += "?{}".format(urlencode({"next": url}, safe="/")) - return login_url - else: - return "/" + login_url = reverse("account_login") + if url: + login_url += "?{}".format(urlencode({"next": url}, safe="/")) + return login_url for (user, client) in self.client_matrix: resp = client.get(url, follow=True) @@ -82,12 +79,6 @@ class BdATestHelpers: class TestBdAViews(BdATestHelpers, TestCase): def setUp(self): - # Signals handlers on login/logout send messages. - # Due to the way the Django' test Client performs login, this raise an - # error. As workaround, we mock the Django' messages module. - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) # Set up the helpers super().setUp() # Some BdA stuff diff --git a/cof/settings/common.py b/cof/settings/common.py index ebc7fb2a..9209ae97 100644 --- a/cof/settings/common.py +++ b/cof/settings/common.py @@ -69,7 +69,6 @@ INSTALLED_APPS = [ "django.contrib.admindocs", "bda", "captcha", - "django_cas_ng", "bootstrapform", "kfet", "kfet.open", @@ -95,6 +94,12 @@ INSTALLED_APPS = [ "kfet.auth", "kfet.cms", "corsheaders", + "allauth_ens", + "allauth_cas", + "allauth", + "allauth.account", + "allauth.socialaccount", + "allauth_ens.providers.clipper", ] MIDDLEWARE = [ @@ -182,22 +187,26 @@ MAIL_DATA = { }, } -LOGIN_URL = "cof-login" -LOGIN_REDIRECT_URL = "home" -CAS_SERVER_URL = "https://cas.eleves.ens.fr/" -CAS_VERSION = "3" -CAS_LOGIN_MSG = None -CAS_IGNORE_REFERER = True -CAS_REDIRECT_URL = "/" -CAS_EMAIL_FORMAT = "%s@clipper.ens.fr" +# Authentication +# https://docs.djangoproject.com/en/1.11/ref/settings/#auth +# https://django-allauth.readthedocs.io/en/latest/index.html AUTHENTICATION_BACKENDS = ( - "django.contrib.auth.backends.ModelBackend", - "gestioncof.shared.COFCASBackend", + "allauth.account.auth_backends.AuthenticationBackend", "kfet.auth.backends.GenericBackend", ) +LOGIN_URL = "account_login" +LOGIN_REDIRECT_URL = "home" +ACCOUNT_LOGOUT_REDIRECT_URL = "home" + +ACCOUNT_ADAPTER = "shared.allauth_adapter.AccountAdapter" +ACCOUNT_AUTHENTICATED_LOGIN_REDIRECTS = False +ACCOUNT_HOME_URL = "home" +ACCOUNT_USER_DISPLAY = lambda u: u.get_short_name() or u.username +SOCIALACCOUNT_ADAPTER = "shared.allauth_adapter.SocialAccountAdapter" + # reCAPTCHA settings # https://github.com/praekelt/django-recaptcha diff --git a/cof/urls.py b/cof/urls.py index 7a0bee4c..b9429887 100644 --- a/cof/urls.py +++ b/cof/urls.py @@ -2,13 +2,12 @@ Fichier principal de configuration des urls du projet GestioCOF """ +from allauth_ens.views import capture_login, capture_logout from django.conf import settings from django.conf.urls import include, url from django.conf.urls.static import static from django.contrib import admin -from django.contrib.auth import views as django_views -from django.views.generic.base import TemplateView -from django_cas_ng import views as django_cas_views +from django.views.generic import RedirectView from wagtail.wagtailadmin import urls as wagtailadmin_urls from wagtail.wagtailcore import urls as wagtail_urls from wagtail.wagtaildocs import urls as wagtaildocs_urls @@ -26,6 +25,8 @@ from gestioncof.urls import ( admin.autodiscover() +redirect_to_home = RedirectView.as_view(pattern_name="home") + urlpatterns = [ # Page d'accueil url(r"^$", gestioncof_views.home, name="home"), @@ -43,30 +44,6 @@ urlpatterns = [ url(r"^calendar/", include(calendar_patterns)), # Clubs url(r"^clubs/", include(clubs_patterns)), - # Authentification - url( - r"^cof/denied$", - TemplateView.as_view(template_name="cof-denied.html"), - name="cof-denied", - ), - url(r"^cas/login$", django_cas_views.login, name="cas_login_view"), - url(r"^cas/logout$", django_cas_views.logout), - url(r"^outsider/login$", gestioncof_views.login_ext, name="ext_login_view"), - url(r"^outsider/logout$", django_views.logout, {"next_page": "home"}), - url(r"^login$", gestioncof_views.login, name="cof-login"), - url(r"^logout$", gestioncof_views.logout, name="cof-logout"), - # Infos persos - url(r"^profile$", gestioncof_views.profile, name="profile"), - url( - r"^outsider/password-change$", - django_views.password_change, - name="password_change", - ), - url( - r"^outsider/password-change-done$", - django_views.password_change_done, - name="password_change_done", - ), # Inscription d'un nouveau membre url(r"^registration$", gestioncof_views.registration, name="registration"), url( @@ -95,15 +72,6 @@ urlpatterns = [ gestioncof_views.user_autocomplete, name="cof-user-autocomplete", ), - # Interface admin - url(r"^admin/logout/", gestioncof_views.logout), - url(r"^admin/doc/", include("django.contrib.admindocs.urls")), - url( - r"^admin/(?P[\d\w]+)/(?P[\d\w]+)/csv/", - csv_views.admin_list_export, - {"fields": ["username"]}, - ), - url(r"^admin/", include(admin.site.urls)), # Liens utiles du COF et du BdA url(r"^utile_cof$", gestioncof_views.utile_cof, name="utile_cof"), url(r"^utile_bda$", gestioncof_views.utile_bda, name="utile_bda"), @@ -115,12 +83,40 @@ urlpatterns = [ name="ml_bda_revente", ), url(r"^k-fet/", include("kfet.urls")), - url(r"^cms/", include(wagtailadmin_urls)), - url(r"^documents/", include(wagtaildocs_urls)), # djconfig url(r"^config", gestioncof_views.ConfigUpdate.as_view(), name="config.edit"), ] +# Admin site + +admin_urls = [ + # Replace the login and logout views with allauth ones. + url(r"^login/", capture_login), + url(r"^logout/", capture_logout), + url(r"^doc/", include("django.contrib.admindocs.urls")), + url( + r"^(?P[\d\w]+)/(?P[\d\w]+)/csv/", + csv_views.admin_list_export, + {"fields": ["username"]}, + ), + url(r"^", include(admin.site.urls)), +] + +urlpatterns += [url(r"^admin/", include(admin_urls))] + +# Profile urls. +# https://django-allauth.readthedocs.io/en/latest/ + +profile_urls = [ + url(r"^edition/$", gestioncof_views.profile, name="profile.edit"), + # allauth urls + # Multiple emails management is unused. + url(r"^email/", redirect_to_home), + url(r"^", include("allauth.urls")), +] + +urlpatterns += [url(r"^profil/", include(profile_urls))] + if "debug_toolbar" in settings.INSTALLED_APPS: import debug_toolbar @@ -131,5 +127,15 @@ if settings.DEBUG: # Il faut dire à Django de servir MEDIA_ROOT lui-même en développement. urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) -# Wagtail for uncatched -urlpatterns += [url(r"", include(wagtail_urls))] +cms_urls = [ + # Replace Wagtail login and logout views with allauth ones. + url(r"^cms/login/", capture_login), + url(r"^cms/logout/", capture_logout), + # Wagtail admin. + url(r"^cms/", include(wagtailadmin_urls)), + url(r"^documents/", include(wagtaildocs_urls)), + # Wagtail serves all uncatched requests. + url(r"", include(wagtail_urls)), +] + +urlpatterns += cms_urls diff --git a/gestioncof/migrations/0015_unique_login_clipper.py b/gestioncof/migrations/0015_unique_login_clipper.py new file mode 100644 index 00000000..a6841be2 --- /dev/null +++ b/gestioncof/migrations/0015_unique_login_clipper.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + +import utils.models + + +def convert_empty_clipper(apps, schema_editor): + CofProfile = apps.get_model("gestioncof", "CofProfile") + CofProfile.objects.filter(login_clipper="").update(login_clipper=None) + + +class Migration(migrations.Migration): + + dependencies = [("gestioncof", "0014_cofprofile_mailing_unernestaparis")] + + operations = [ + migrations.AlterField( + model_name="cofprofile", + name="login_clipper", + field=models.CharField( + verbose_name="Login clipper", + max_length=32, + null=True, + blank=True, + default="", + ), + ), + migrations.RunPython(convert_empty_clipper), + migrations.AlterField( + model_name="cofprofile", + name="login_clipper", + field=utils.models.NullCharField( + verbose_name="Login clipper", max_length=32, default="", unique=True + ), + ), + ] diff --git a/gestioncof/migrations/0016_create_clipper_conns.py b/gestioncof/migrations/0016_create_clipper_conns.py new file mode 100644 index 00000000..1d547767 --- /dev/null +++ b/gestioncof/migrations/0016_create_clipper_conns.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from django.db import migrations + + +def create_clipper_connections(apps, schema_editor): + CofProfile = apps.get_model("gestioncof", "CofProfile") + SocialAccount = apps.get_model("socialaccount", "SocialAccount") + + profiles = CofProfile.objects.exclude(login_clipper="").values( + "user_id", "login_clipper" + ) + + SocialAccount.objects.bulk_create( + [ + SocialAccount( + provider="clipper", user_id=p["user_id"], uid=p["login_clipper"] + ) + for p in profiles + ] + ) + + +class Migration(migrations.Migration): + """ + As part of the allauth integration, this migration creates SocialAccount + instances for Clipper provider, based on the `CofProfile.login_clipper` + values. + """ + + dependencies = [ + ("gestioncof", "0015_unique_login_clipper"), + ("socialaccount", "0003_extra_data_default_dict"), + ] + + operations = [migrations.RunPython(create_clipper_connections)] diff --git a/gestioncof/models.py b/gestioncof/models.py index 227fa936..3f435a1f 100644 --- a/gestioncof/models.py +++ b/gestioncof/models.py @@ -1,3 +1,4 @@ +from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.db import models from django.db.models.signals import post_delete, post_save @@ -6,6 +7,7 @@ from django.utils.translation import ugettext_lazy as _ from bda.models import Spectacle from gestioncof.petits_cours_models import choices_length +from utils.models import NullCharField TYPE_COMMENT_FIELD = (("text", _("Texte long")), ("char", _("Texte court"))) @@ -46,7 +48,9 @@ class CofProfile(models.Model): ) user = models.OneToOneField(User, on_delete=models.CASCADE, related_name="profile") - login_clipper = models.CharField("Login clipper", max_length=32, blank=True) + login_clipper = NullCharField( + _("Login clipper"), max_length=32, blank=True, unique=True, default="" + ) is_cof = models.BooleanField("Membre du COF", default=False) phone = models.CharField("Téléphone", max_length=20, blank=True) occupation = models.CharField( @@ -86,6 +90,53 @@ class CofProfile(models.Model): def __str__(self): return self.user.username + def save(self, *args, **kwargs): + created = self.pk is None + res = super().save(*args, **kwargs) + self.sync_clipper_connections(created=created) + return res + + def sync_clipper_connections(self, created): + """ + Update the clipper connections of the user according to the value of + `login_clipper`. + + If empty, all clipper connections are removed. + + See also `sync_clipper…` signals handlers (in `signals` module). They + sync `login_clipper` from the clipper connections. + + Raises + IntegrityError: login_clipper is already used by another user. + """ + user, clipper = self.user, self.login_clipper + conns = user.socialaccount_set + clipper_conns = conns.filter(provider="clipper") + + if created and clipper: + conns.create(provider="clipper", uid=clipper) + return + + if clipper: + try: + # If a clipper connection already exists with the uid, call + # save to update last_login value (an auto_now field). + conn = clipper_conns.get(uid=clipper) + conn.save() + except SocialAccount.DoesNotExist: + # Nothing prevents the user from having multiple clipper + # connections. Let's update the most recently used with the + # given identifier. If none exists, create it. + try: + conn = clipper_conns.latest("last_login") + if conn.uid != clipper: + conn.uid = clipper + conn.save(update_fields=["uid"]) + except SocialAccount.DoesNotExist: + conns.create(provider="clipper", uid=clipper) + else: + clipper_conns.delete() + @receiver(post_save, sender=User) def create_user_profile(sender, instance, created, **kwargs): diff --git a/gestioncof/petits_cours_views.py b/gestioncof/petits_cours_views.py index d640981a..97449144 100644 --- a/gestioncof/petits_cours_views.py +++ b/gestioncof/petits_cours_views.py @@ -3,16 +3,15 @@ import json from custommail.shortcuts import render_custom_mail from django.conf import settings from django.contrib import messages -from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User from django.core import mail from django.db import transaction -from django.shortcuts import get_object_or_404, redirect, render +from django.shortcuts import get_object_or_404, render from django.utils import timezone from django.views.decorators.csrf import csrf_exempt from django.views.generic import DetailView, ListView -from gestioncof.decorators import buro_required +from gestioncof.decorators import buro_required, cof_required from gestioncof.models import CofProfile from gestioncof.petits_cours_forms import DemandeForm, MatieresFormSet from gestioncof.petits_cours_models import ( @@ -336,11 +335,9 @@ def _traitement_post(request, demande): ) -@login_required +@cof_required def inscription(request): profile, created = CofProfile.objects.get_or_create(user=request.user) - if not profile.is_cof: - return redirect("cof-denied") success = False if request.method == "POST": formset = MatieresFormSet(request.POST, instance=request.user) diff --git a/gestioncof/shared.py b/gestioncof/shared.py index 1a5bd32e..3600bae3 100644 --- a/gestioncof/shared.py +++ b/gestioncof/shared.py @@ -1,22 +1,4 @@ -from django.conf import settings from django.contrib.sites.models import Site -from django_cas_ng.backends import CASBackend - - -class COFCASBackend(CASBackend): - def clean_username(self, username): - # Le CAS de l'ENS accepte les logins avec des espaces au début - # et à la fin, ainsi qu’avec une casse variable. On normalise pour - # éviter les doublons. - return username.strip().lower() - - def configure_user(self, user): - clipper = user.username - user.profile.login_clipper = clipper - user.profile.save() - user.email = settings.CAS_EMAIL_FORMAT % clipper - user.save() - return user def context_processor(request): diff --git a/gestioncof/signals.py b/gestioncof/signals.py index 3614b1c8..806db2a2 100644 --- a/gestioncof/signals.py +++ b/gestioncof/signals.py @@ -1,22 +1,40 @@ -from django.contrib import messages -from django.contrib.auth.signals import user_logged_in +from allauth.socialaccount.models import SocialAccount +from django.db.models.signals import post_delete, post_save from django.dispatch import receiver -from django.utils.translation import ugettext_lazy as _ -from django_cas_ng.signals import cas_user_authenticated -@receiver(user_logged_in) -def messages_on_out_login(request, user, **kwargs): - if user.backend.startswith("django.contrib.auth"): - msg = _("Connexion à GestioCOF réussie. Bienvenue {}.").format( - user.get_short_name() - ) - messages.success(request, msg) +@receiver(post_save, sender=SocialAccount) +def sync_clipper_on_saving_connection(sender, instance, **kwargs): + if instance.provider != "clipper": + return + + # Saving instance makes it the most recently used clipper connection. + # Always update login_clipper, if value is not the used identifier. + + profile = instance.user.profile + + if profile.login_clipper != instance.uid: + profile.login_clipper = instance.uid + profile.save(update_fields=["login_clipper"]) -@receiver(cas_user_authenticated) -def mesagges_on_cas_login(request, user, **kwargs): - msg = _("Connexion à GestioCOF par CAS réussie. Bienvenue {}.").format( - user.get_short_name() - ) - messages.success(request, msg) +@receiver(post_delete, sender=SocialAccount) +def sync_clipper_on_deleting_connection(sender, instance, **kwargs): + if instance.provider != "clipper": + return + + profile = instance.user.profile + + try: + # Get the most recently used clipper connection. Set its identifier as + # login_clipper value of the related CofProfile. + conn = SocialAccount.objects.filter( + provider="clipper", user=profile.user + ).latest("last_login") + if profile.login_clipper != conn.uid: + profile.login_clipper = conn.uid + profile.save(update_fields=["login_clipper"]) + except SocialAccount.DoesNotExist: + # If none lefts, flush it. + profile.login_clipper = "" + profile.save(update_fields=["login_clipper"]) diff --git a/gestioncof/static/css/cof.css b/gestioncof/static/css/cof.css index 84c81d60..3beb08c7 100644 --- a/gestioncof/static/css/cof.css +++ b/gestioncof/static/css/cof.css @@ -299,24 +299,6 @@ fieldset legend { display: none; } -#main-login-container { - margin-top : 7em; - margin-bottom: 7em; -} - -#main-login-container .banner { - padding-right: 15px; - padding-left: 15px; -} - -#main-login { - background-color: #DE826B; -} - -#main-login .btn-primary { - background-color: #B56A59 -} - .title-link { float: none; display: block; diff --git a/gestioncof/templates/cof-denied.html b/gestioncof/templates/cof-denied.html deleted file mode 100644 index b2a12717..00000000 --- a/gestioncof/templates/cof-denied.html +++ /dev/null @@ -1,5 +0,0 @@ -{% extends "base_title.html" %} - -{% block realcontent %} -

Section réservée aux membres du COF -- merci de vous inscrire au COF ou de passer au COF/nous envoyer un mail si vous êtes déjà membre :)

-{% endblock %} diff --git a/gestioncof/templates/error.html b/gestioncof/templates/error.html deleted file mode 100644 index 082abcf0..00000000 --- a/gestioncof/templates/error.html +++ /dev/null @@ -1,14 +0,0 @@ -{% extends "base_title.html" %} - -{% block realcontent %} - {% if error_type == "use_clipper_login" %} -

Votre identifiant est lié à un compte clipper

-

Veuillez vous connecter à l'aide de votre compte clipper

- {% elif error_type == "no_password" %} -

Votre compte n'a pas de mot de passe associé

-

Veuillez nous contacter pour que nous en définissions un et que nous vous le transmettions !

- {% else %} -

{{ error_title }}

-

{{ error_description }}

- {% endif %} -{% endblock %} diff --git a/gestioncof/templates/gestioncof/base_header.html b/gestioncof/templates/gestioncof/base_header.html index e5f757a7..2fd32233 100644 --- a/gestioncof/templates/gestioncof/base_header.html +++ b/gestioncof/templates/gestioncof/base_header.html @@ -1,4 +1,5 @@ {% extends "base.html" %} +{% load i18n %} {% block content %}
@@ -11,7 +12,12 @@
  |  - Se déconnecter  + + + {% trans "Se déconnecter" %} + + +

{% if user.first_name %}{{ user.first_name }}{% else %}{{ user.username }}{% endif %}, {% if user.profile.is_cof %}au COF{% else %}non-COF{% endif %}

diff --git a/gestioncof/templates/home.html b/gestioncof/templates/home.html index b14c0a82..4689e7d3 100644 --- a/gestioncof/templates/home.html +++ b/gestioncof/templates/home.html @@ -1,4 +1,5 @@ {% extends "gestioncof/base_header.html" %} +{% load i18n %} {% load wagtailcore_tags %} {% block homelink %} @@ -65,16 +66,39 @@ {% endif %} + +

+ + {% trans "Mon compte" %} +

+ +
+ +
{% if user.profile.is_cof %}

Divers

{% endif %} diff --git a/gestioncof/tests/test_models.py b/gestioncof/tests/test_models.py new file mode 100644 index 00000000..eb591f16 --- /dev/null +++ b/gestioncof/tests/test_models.py @@ -0,0 +1,155 @@ +# -*- coding: utf-8 -*- +from allauth.socialaccount.models import SocialAccount +from django.db import IntegrityError, transaction +from django.test import TestCase + +from gestioncof.models import CofProfile, User + + +class UserTests(TestCase): + def test_has_profile(self): + """ + A User always has a related CofProfile and a CofProfile always has a + related User. + """ + u = User.objects.create_user("foo", "", "") + + # Creating a User creates a related CofProfile. + self.assertTrue(hasattr(u, "profile")) + + # there's no point in having a cofprofile without a user associated. + p = u.profile + + u.delete() + + with transaction.atomic(): + with self.assertRaises(CofProfile.DoesNotExist): + p.refresh_from_db() + + # there's no point in having a user without a cofprofile associated. + u = User.objects.create_user("foo", "", "") + + u.profile.delete() + + with transaction.atomic(): + with self.assertRaises(User.DoesNotExist): + u.refresh_from_db() + + +class CofProfileTests(TestCase): + def setUp(self): + self.u = User.objects.create_user("user", "", "") + self.p = self.u.profile + + def test_login_clipper_sync_clipper_connections(self): + """ + The value of `login_clipper` is sync with the identifier of the most + recently used clipper connection. + """ + + def socialaccount_transform(o): + """ + Function to compare QuerySet of `SocialAccount`, instead of the + default, `repr`, only depends on the involved user. + """ + return "{provider} {username} {uid}".format( + provider=o.provider, username=o.user.username, uid=o.uid + ) + + def assertClipperAccountQuerysetEquals(user, *args, **kwargs): + kwargs.setdefault("transform", socialaccount_transform) + qs = SocialAccount.objects.filter(provider="clipper", user=user).order_by( + "last_login" + ) + self.assertQuerysetEqual(qs, *args, **kwargs) + + # Not saved in the database. + self.assertEqual(CofProfile().login_clipper, "") + + # This CofProfile has been created without any value for login_clipper. + u, p = self.u, self.p + self.assertEqual(p.login_clipper, "") + + # Filling value for the first time triggers the creation of a clipper + # connection (SocialAccount) for the related user. + p.login_clipper = "theclipper" + p.save() + + self.assertEqual(p.login_clipper, "theclipper") + assertClipperAccountQuerysetEquals(u, ["clipper user theclipper"]) + + # Assigning a new value updates the existing connection. + p.login_clipper = "anotherclipper" + p.save() + + self.assertEqual(p.login_clipper, "anotherclipper") + assertClipperAccountQuerysetEquals(u, ["clipper user anotherclipper"]) + + # Creating a clipper connection, using SocialAccount, sets the used + # identifier as value. + conn = SocialAccount.objects.create(provider="clipper", user=u, uid="clip") + + self.assertEqual(p.login_clipper, "clip") + assertClipperAccountQuerysetEquals( + u, ["clipper user anotherclipper", "clipper user clip"] + ) + + # Removing a clipper connection sets the identifier most recently + # used one as value. + conn.delete() + + p.refresh_from_db() + self.assertEqual(p.login_clipper, "anotherclipper") + assertClipperAccountQuerysetEquals(u, ["clipper user anotherclipper"]) + + # If the deletion of SocialAccount(s) leaves no clipper connection, + # flush the value. + SocialAccount.objects.filter(provider="clipper", user=u).delete() + + p.refresh_from_db() + self.assertEqual(p.login_clipper, "") + + # Assigning an identifier already in use updates the related connection + # as the most recently used. + SocialAccount.objects.bulk_create( + [ + SocialAccount(provider="clipper", user=u, uid="theclipper"), + SocialAccount(provider="clipper", user=u, uid="clip"), + ] + ) + # Note 'clip' is the most recent one, and value is empty because + # bulk_create didn't trigger post_save signals. + + p.login_clipper = "theclipper" + p.save() + + self.assertEqual(p.login_clipper, "theclipper") + assertClipperAccountQuerysetEquals( + u, ["clipper user clip", "clipper user theclipper"] + ) + + # Flushing value removes all clipper connections, and set value to None + # to avoid failure due to the unicity constraint. + p.login_clipper = "" + p.save() + + self.assertEqual(p.login_clipper, "") + self.assertFalse( + SocialAccount.objects.filter(provider="clipper", user=u).exists() + ) + + # Value is unique among all CofProfile instances… + p.login_clipper = "theclipper" + p.save() + p2 = User.objects.create_user("user2", "", "").profile + p2.login_clipper = "theclipper" + + with transaction.atomic(): + with self.assertRaises(IntegrityError): + p2.save() + + # …except for '' (stored as NULL). + p.login_clipper = "" + p.save() + p2.login_clipper = "" + p2.save() diff --git a/gestioncof/tests/test_views.py b/gestioncof/tests/test_views.py index 43ea148c..b5c5454b 100644 --- a/gestioncof/tests/test_views.py +++ b/gestioncof/tests/test_views.py @@ -355,31 +355,31 @@ class HomeViewTests(ViewTestCaseMixin, TestCase): self.assertEqual(r.status_code, 200) -class ProfileViewTests(ViewTestCaseMixin, TestCase): - url_name = "profile" - url_expected = "/profile" +class ProfileEditViewTests(ViewTestCaseMixin, TestCase): + url_name = "profile.edit" + url_expected = "/profil/edition/" http_methods = ["GET", "POST"] - auth_user = "member" - auth_forbidden = [None, "user"] + auth_user = "user" + auth_forbidden = [None] def test_get(self): r = self.client.get(self.url) self.assertEqual(r.status_code, 200) def test_post(self): - u = self.users["member"] + u = self.users["user"] r = self.client.post( self.url, { "u-first_name": "First", "u-last_name": "Last", - "p-phone": "", - # 'mailing_cof': '1', - # 'mailing_bda': '1', - # 'mailing_bda_revente': '1', + "u-phone": "", + # "p-mailing_cof": "1", + # "p-mailing_bda": "1", + # "p-mailing_bda_revente': "1", }, ) diff --git a/gestioncof/views.py b/gestioncof/views.py index 618fb24a..4310c521 100644 --- a/gestioncof/views.py +++ b/gestioncof/views.py @@ -6,11 +6,7 @@ from custommail.shortcuts import send_custom_mail from django.contrib import messages from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User -from django.contrib.auth.views import ( - login as django_login_view, - logout as django_logout_view, - redirect_to_login, -) +from django.contrib.auth.views import redirect_to_login from django.contrib.sites.models import Site from django.core.urlresolvers import reverse_lazy from django.http import Http404, HttpResponse, HttpResponseForbidden @@ -18,7 +14,6 @@ from django.shortcuts import get_object_or_404, redirect, render from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from django.views.generic import FormView -from django_cas_ng.views import logout as cas_logout_view from icalendar import Calendar, Event as Vevent from bda.models import Spectacle, Tirage @@ -72,55 +67,6 @@ def home(request): return render(request, "home.html", data) -def login(request): - if request.user.is_authenticated: - return redirect("home") - context = {} - if request.method == "GET" and "next" in request.GET: - context["next"] = request.GET["next"] - return render(request, "login_switch.html", context) - - -def login_ext(request): - if request.method == "POST" and "username" in request.POST: - try: - user = User.objects.get(username=request.POST["username"]) - if not user.has_usable_password() or user.password in ("", "!"): - profile, created = CofProfile.objects.get_or_create(user=user) - if profile.login_clipper: - return render( - request, "error.html", {"error_type": "use_clipper_login"} - ) - else: - return render(request, "error.html", {"error_type": "no_password"}) - except User.DoesNotExist: - pass - context = {} - if request.method == "GET" and "next" in request.GET: - context["next"] = request.GET["next"] - if request.method == "POST" and "next" in request.POST: - context["next"] = request.POST["next"] - return django_login_view(request, template_name="login.html", extra_context=context) - - -@login_required -def logout(request, next_page=None): - if next_page is None: - next_page = request.GET.get("next", None) - - profile = getattr(request.user, "profile", None) - - if profile and profile.login_clipper: - msg = _("Déconnexion de GestioCOF et CAS réussie. À bientôt {}.") - logout_view = cas_logout_view - else: - msg = _("Déconnexion de GestioCOF réussie. À bientôt {}.") - logout_view = django_logout_view - - messages.success(request, msg.format(request.user.get_short_name())) - return logout_view(request, next_page=next_page) - - @login_required def survey(request, survey_id): survey = get_object_or_404( @@ -359,7 +305,7 @@ def survey_status(request, survey_id): ) -@cof_required +@login_required def profile(request): user = request.user data = request.POST if request.method == "POST" else None diff --git a/kfet/auth/tests.py b/kfet/auth/tests.py index e4330eef..a8c81360 100644 --- a/kfet/auth/tests.py +++ b/kfet/auth/tests.py @@ -123,10 +123,6 @@ class GenericBackendTests(TestCase): class GenericLoginViewTests(TestCase): def setUp(self): - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) - user_acc = Account(trigramme="000") user_acc.save({"username": "user"}) self.user = user_acc.user @@ -262,10 +258,6 @@ class GenericLoginViewTests(TestCase): class TemporaryAuthTests(TestCase): def setUp(self): - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) - self.factory = RequestFactory() self.middleware = TemporaryAuthMiddleware(mock.Mock()) diff --git a/kfet/auth/views.py b/kfet/auth/views.py index 35a6eedf..75c4c672 100644 --- a/kfet/auth/views.py +++ b/kfet/auth/views.py @@ -73,7 +73,7 @@ class GenericLoginView(View): here_qd["next"] = self.request.GET["next"] here_url += "?{}".format(here_qd.urlencode()) - logout_url = reverse("cof-logout") + logout_url = reverse("account_logout") logout_qd = QueryDict(mutable=True) logout_qd["next"] = here_url logout_url += "?{}".format(logout_qd.urlencode(safe="/")) diff --git a/kfet/open/tests.py b/kfet/open/tests.py index 75a9bf8a..e7bcd276 100644 --- a/kfet/open/tests.py +++ b/kfet/open/tests.py @@ -106,11 +106,6 @@ class OpenKfetViewsTest(ChannelTestCase): """OpenKfet views unit-tests suite.""" def setUp(self): - # Need this (and here) because of '.login' in setUp - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) - # get some permissions perms = { "kfet.is_team": Permission.objects.get(codename="is_team"), @@ -187,8 +182,7 @@ class OpenKfetConsumerTest(ChannelTestCase): OpenKfetConsumer.group_send("kfet.open.team", {"test": "plop"}) self.assertIsNone(c.receive()) - @mock.patch("gestioncof.signals.messages") - def test_team_user(self, mock_messages): + def test_team_user(self): """Team user is added to kfet.open.team group.""" # setup team user and its client t = User.objects.create_user("team", "", "team") @@ -217,10 +211,6 @@ class OpenKfetScenarioTest(ChannelTestCase): """OpenKfet functionnal tests suite.""" def setUp(self): - # Need this (and here) because of '.login' in setUp - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) # anonymous client (for views) self.c = Client() diff --git a/kfet/templates/kfet/base_nav.html b/kfet/templates/kfet/base_nav.html index 1cded20b..1b71d8ca 100644 --- a/kfet/templates/kfet/base_nav.html +++ b/kfet/templates/kfet/base_nav.html @@ -101,8 +101,8 @@ {% endif %}
  • - - Déconnexion + + {% trans "Déconnexion" %}
    • @@ -110,15 +110,14 @@ {% endif %} {% if user.is_authenticated and not perms.kfet.is_team %}
  • - +
    • {% elif not user.is_authenticated %}
  • - - Connexion + + {% trans "Connexion" %}
    • {% endif %} diff --git a/kfet/tests/test_statistic.py b/kfet/tests/test_statistic.py index f0ed7f74..ba1c2f64 100644 --- a/kfet/tests/test_statistic.py +++ b/kfet/tests/test_statistic.py @@ -1,5 +1,3 @@ -from unittest.mock import patch - from django.contrib.auth.models import Permission, User from django.test import Client, TestCase @@ -7,8 +5,7 @@ from kfet.models import Account, Article, ArticleCategory class TestStats(TestCase): - @patch("gestioncof.signals.messages") - def test_user_stats(self, mock_messages): + def test_user_stats(self): """ Checks that we can get the stat-related pages without any problem. """ @@ -68,5 +65,5 @@ class TestStats(TestCase): for url in articles_urls: resp = client.get(url) self.assertEqual(200, resp.status_code) - resp2 = client2.get(url, follow=True) - self.assertRedirects(resp2, "/") + resp2 = client2.get(url) + self.assertRedirects(resp2, "/profil/login/?next={}".format(url)) diff --git a/kfet/tests/testcases.py b/kfet/tests/testcases.py index 36a4ab65..206c2477 100644 --- a/kfet/tests/testcases.py +++ b/kfet/tests/testcases.py @@ -1,67 +1,14 @@ -from unittest import mock -from urllib.parse import parse_qs, urlparse - -from django.core.urlresolvers import reverse -from django.http import QueryDict -from django.test import Client -from django.utils import timezone -from django.utils.functional import cached_property +from shared.tests.testcases import ( + TestCaseMixin as BaseTestCaseMixin, + ViewTestCaseMixin as BaseViewTestCaseMixin, +) from .utils import create_root, create_team, create_user -class TestCaseMixin: +class TestCaseMixin(BaseTestCaseMixin): """Extends TestCase for kfet application tests.""" - def assertForbidden(self, response): - """ - Test that the response (retrieved with a Client) is a denial of access. - - The response should verify one of the following: - - its HTTP response code is 403, - - it redirects to the login page with a GET parameter named 'next' - whose value is the url of the requested page. - - """ - request = response.wsgi_request - - try: - try: - # Is this an HTTP Forbidden response ? - self.assertEqual(response.status_code, 403) - except AssertionError: - # A redirection to the login view is fine too. - - # Let's build the login url with the 'next' param on current - # page. - full_path = request.get_full_path() - - querystring = QueryDict(mutable=True) - querystring["next"] = full_path - - login_url = "/login?" + querystring.urlencode(safe="/") - - # We don't focus on what the login view does. - # So don't fetch the redirect. - self.assertRedirects(response, login_url, fetch_redirect_response=False) - except AssertionError: - raise AssertionError( - "%(http_method)s request at %(path)s should be forbidden for " - "%(username)s user.\n" - "Response isn't 403, nor a redirect to login view. Instead, " - "response code is %(code)d." - % { - "http_method": request.method, - "path": request.get_full_path(), - "username": ( - "'{}'".format(request.user) - if request.user.is_authenticated - else "anonymous" - ), - "code": response.status_code, - } - ) - def assertForbiddenKfet(self, response, form_ctx="form"): """ Test that a response (retrieved with a Client) contains error due to @@ -113,65 +60,13 @@ class TestCaseMixin: value = value() self.assertEqual(value, expected_value) - def assertUrlsEqual(self, actual, expected): - """ - Test that the url 'actual' is as 'expected'. - Arguments: - actual (str): Url to verify. - expected: Two forms are accepted. - * (str): Expected url. Strings equality is checked. - * (dict): Its keys must be attributes of 'urlparse(actual)'. - Equality is checked for each present key, except for - 'query' which must be a dict of the expected query string - parameters. - - """ - if type(expected) == dict: - parsed = urlparse(actual) - for part, expected_part in expected.items(): - if part == "query": - self.assertDictEqual( - parse_qs(parsed.query), expected.get("query", {}) - ) - else: - self.assertEqual(getattr(parsed, part), expected_part) - else: - self.assertEqual(actual, expected) - - -class ViewTestCaseMixin(TestCaseMixin): +class ViewTestCaseMixin(TestCaseMixin, BaseViewTestCaseMixin): """ TestCase extension to ease tests of kfet views. - - Urls concerns - ------------- - - # Basic usage - - Attributes: - url_name (str): Name of view under test, as given to 'reverse' - function. - url_args (list, optional): Will be given to 'reverse' call. - url_kwargs (dict, optional): Same. - url_expcted (str): What 'reverse' should return given previous - attributes. - - View url can then be accessed at the 'url' attribute. - - # Advanced usage - - If multiple combinations of url name, args, kwargs can be used for a view, - it is possible to define 'urls_conf' attribute. It must be a list whose - each item is a dict defining arguments for 'reverse' call ('name', 'args', - 'kwargs' keys) and its expected result ('expected' key). - - The reversed urls can be accessed at the 't_urls' attribute. - - - Users concerns - -------------- + Most information can be found in the base parent class doc. + This class performs some changes to users management, detailed below. During setup, three users are created with their kfet account: - 'user': a basic user without any permission, account trigramme: 000, @@ -181,78 +76,19 @@ class ViewTestCaseMixin(TestCaseMixin): trigramme: LIQ. Their password is their username. - One can create additionnal users with 'get_users_extra' method, or prevent - these 3 users to be created with 'get_users_base' method. See these two - methods for further informations. - By using 'register_user' method, these users can then be accessed at - 'users' attribute by their label. Similarly, their kfet account is + 'users' attribute by their label. Similarly, their kfet account, if any, is registered on 'accounts' attribute. - - A user label can be given to 'auth_user' attribute. The related user is - then authenticated on self.client during test setup. Its value defaults to - 'None', meaning no user is authenticated. - - - Automated tests - --------------- - - # Url reverse - - Based on url-related attributes/properties, the test 'test_urls' checks - that expected url is returned by 'reverse' (once with basic url usage and - each for advanced usage). - - # Forbidden responses - - The 'test_forbidden' test verifies that each user, from labels of - 'auth_forbidden' attribute, can't access the url(s), i.e. response should - be a 403, or a redirect to login view. - - Tested HTTP requests are given by 'http_methods' attribute. Additional data - can be given by defining an attribute '_data'. - """ - url_name = None - url_expected = None - - http_methods = ["GET"] - - auth_user = None - auth_forbidden = [] - with_liq = False def setUp(self): """ Warning: Do not forget to call super().setUp() in subclasses. """ - # Signals handlers on login/logout send messages. - # Due to the way the Django' test Client performs login, this raise an - # error. As workaround, we mock the Django' messages module. - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) - - # A test can mock 'django.utils.timezone.now' and give this as return - # value. E.g. it is useful if the test checks values of 'auto_now' or - # 'auto_now_add' fields. - self.now = timezone.now() - - # These attributes register users and accounts instances. - self.users = {} self.accounts = {} - - for label, user in dict(self.users_base, **self.users_extra).items(): - self.register_user(label, user) - - if self.auth_user: - self.client.force_login(self.users[self.auth_user]) - - def tearDown(self): - del self.users_base - del self.users_extra + super().setUp() def get_users_base(self): """ @@ -277,80 +113,7 @@ class ViewTestCaseMixin(TestCaseMixin): users_base["liq"] = create_user("liq", "LIQ") return users_base - @cached_property - def users_base(self): - return self.get_users_base() - - def get_users_extra(self): - """ - Dict of . - - Note: Don't access yourself this property. Use 'users_base' attribute - which cache the returned value from here. - It allows to give functions calls, which create users instances, as - values here. - - """ - return {} - - @cached_property - def users_extra(self): - return self.get_users_extra() - def register_user(self, label, user): - self.users[label] = user + super().register_user(label, user) if hasattr(user.profile, "account_kfet"): self.accounts[label] = user.profile.account_kfet - - def get_user(self, label): - if self.auth_user is not None: - return self.auth_user - return self.auth_user_mapping.get(label) - - @property - def urls_conf(self): - return [ - { - "name": self.url_name, - "args": getattr(self, "url_args", []), - "kwargs": getattr(self, "url_kwargs", {}), - "expected": self.url_expected, - } - ] - - @property - def t_urls(self): - return [ - reverse( - url_conf["name"], - args=url_conf.get("args", []), - kwargs=url_conf.get("kwargs", {}), - ) - for url_conf in self.urls_conf - ] - - @property - def url(self): - return self.t_urls[0] - - def test_urls(self): - for url, conf in zip(self.t_urls, self.urls_conf): - self.assertEqual(url, conf["expected"]) - - def test_forbidden(self): - for method in self.http_methods: - for user in self.auth_forbidden: - for url in self.t_urls: - self.check_forbidden(method, url, user) - - def check_forbidden(self, method, url, user=None): - method = method.lower() - client = Client() - if user is not None: - client.login(username=user, password=user) - - send_request = getattr(client, method) - data = getattr(self, "{}_data".format(method), {}) - - r = send_request(url, data) - self.assertForbidden(r) diff --git a/requirements.txt b/requirements.txt index 19b185c7..44047c46 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ configparser==3.5.0 Django==1.11.* django-autocomplete-light==3.1.3 +django-allauth-ens==1.1.* django-autoslug==1.9.3 -django-cas-ng==3.5.7 django-djconfig==0.5.3 django-recaptcha==1.4.0 django-redis-cache==1.7.1 diff --git a/shared/__init__.py b/shared/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/shared/allauth_adapter.py b/shared/allauth_adapter.py new file mode 100644 index 00000000..e3873fee --- /dev/null +++ b/shared/allauth_adapter.py @@ -0,0 +1,21 @@ +from allauth.account.adapter import DefaultAccountAdapter +from allauth.socialaccount.adapter import DefaultSocialAccountAdapter + + +class AccountAdapter(DefaultAccountAdapter): + def is_open_for_signup(self, request): + """ + Signup is not allowed by default. + """ + return False + + +class SocialAccountAdapter(DefaultSocialAccountAdapter): + def is_open_for_signup(self, request, sociallogin): + """ + Authorize user connecting via Clipper to get a GestioCOF account. + """ + provider = sociallogin.account.provider + if provider == "clipper": + return True + return super().is_open_for_signup(request, sociallogin) diff --git a/shared/tests/test_auth.py b/shared/tests/test_auth.py new file mode 100644 index 00000000..a9254fa2 --- /dev/null +++ b/shared/tests/test_auth.py @@ -0,0 +1,354 @@ +import re + +from allauth.socialaccount.models import SocialAccount +from allauth.socialaccount.providers import registry as providers_registry +from allauth_cas.test.testcases import CASViewTestCase +from django.contrib.auth import HASH_SESSION_KEY, get_user_model +from django.core import mail +from django.core.urlresolvers import reverse +from django.test import Client, TestCase + +from .testcases import ViewTestCaseMixin + +User = get_user_model() + + +def prevent_logout_pwd_change(client, user): + """ + Updating a user's password logs out all sessions for the user. + By calling this function this behavior will be prevented. + + See this link, and the source code of `update_session_auth_hash`: + https://docs.djangoproject.com/en/dev/topics/auth/default/#session-invalidation-on-password-change + """ + if hasattr(user, "get_session_auth_hash"): + session = client.session + session[HASH_SESSION_KEY] = user.get_session_auth_hash() + session.save() + + +def get_reset_password_link(email_msg): + m = re.search(r"http://testserver(/profil/password/reset/key/.*/)", email_msg.body) + return m.group(1) + + +class LoginViewTests(ViewTestCaseMixin, TestCase): + url_name = "account_login" + url_expected = "/profil/login/" + + http_methods = ["GET", "POST"] + + def test_get(self): + """ + Unauthenticated users can access the login form. + """ + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + def test_get_already_auth(self): + """ + Even already authenticated users can access the login form. + They may have been redirected due to a lack of authorizations. + """ + self.client.login(username="user", password="user") + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + def test_post(self): + """ + Users can log in. + """ + r = self.client.post(self.url, {"login": "user", "password": "user"}) + + self.assertRedirects(r, reverse("home")) + self.assertEqual(r.wsgi_request.user, self.users["user"]) + + def test_post_redirect(self): + """ + On login, user is redirected to the value of the `next` GET parameter. + """ + redirect_url = reverse("account_logout") + url = self.url + "?next=" + redirect_url + + r = self.client.post(url, {"login": "user", "password": "user"}) + + self.assertRedirects(r, redirect_url) + self.assertEqual(r.wsgi_request.user, self.users["user"]) + + def test_post_invalid_password(self): + """ + If credentials are incorrect, the page is displayed again. + """ + r = self.client.post(self.url, {"username": "user", "password": "bad password"}) + + self.assertEqual(r.status_code, 200) + self.assertTrue(r.wsgi_request.user.is_anonymous()) + + +class LogoutViewTests(ViewTestCaseMixin, TestCase): + url_name = "account_logout" + url_expected = "/profil/logout/" + + http_methods = ["GET", "POST"] + + auth_user = "user" + + def test_get(self): + """ + Using the HTTP method GET, only a confirmation is prompted to the + user. + """ + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + def test_post(self): + """ + With a POST request, user is logged out. + """ + r = self.client.post(self.url) + + self.assertRedirects(r, reverse("home"), fetch_redirect_response=False) + self.assertTrue(r.wsgi_request.user.is_anonymous()) + + def test_post_redirect(self): + """ + On logout, user is redirected to the value of the `next` GET parameter. + """ + redirect_url = reverse("account_set_password") + url = self.url + "?next=" + redirect_url + + r = self.client.post(url) + + self.assertRedirects(r, redirect_url, fetch_redirect_response=False) + self.assertTrue(r.wsgi_request.user.is_anonymous()) + + +class ChangePasswordViewTests(ViewTestCaseMixin, TestCase): + url_name = "account_change_password" + url_expected = "/profil/password/change/" + + http_methods = ["GET", "POST"] + + auth_user = "user" + auth_forbidden = [None] + + def test_get(self): + """ + Authenticated users can access the page to change their password. + """ + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + def test_get_no_password(self): + """ + Authenticated users who do not have a password are redirected to the + `account_set_password` view. + """ + user = self.users["user"] + user.set_unusable_password() + user.save() + prevent_logout_pwd_change(self.client, user) + + r = self.client.get(self.url) + + self.assertRedirects(r, reverse("account_set_password")) + + def test_post(self): + """ + Authenticated users can change their password. + """ + user = self.users["user"] + + r = self.client.post( + self.url, + {"oldpassword": "user", "password1": "usertruc", "password2": "usertruc"}, + ) + + self.assertRedirects(r, reverse("account_change_password")) + user.refresh_from_db() + self.assertTrue(user.check_password("usertruc")) + + +class SetPasswordViewTests(ViewTestCaseMixin, TestCase): + url_name = "account_set_password" + url_expected = "/profil/password/set/" + + http_methods = ["GET", "POST"] + + auth_user = "user_nopwd" + auth_forbidden = [None] + + def setUp(self): + super().setUp() + user_nopwd = self.users["user_nopwd"] + user_nopwd.set_unusable_password() + user_nopwd.save() + prevent_logout_pwd_change(self.client, user_nopwd) + + def get_users_extra(self): + # `user_nopwd` is created with a password to use the `login` method of + # the test client. The password is then removed. + return {"user_nopwd": User.objects.create_user("user_nopwd", "", "user_nopwd")} + + def test_get(self): + """ + Authenticated users who do not have a password can access the page. + """ + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + def test_get_has_password(self): + """ + Authenticated users who already have a password are redirected to the + `account_change_password` view. + """ + client = Client() + client.login(username="user", password="user") + r = client.get(self.url) + self.assertRedirects(r, reverse("account_change_password")) + + def test_post(self): + """ + Authenticated users can set their password. + """ + user = self.users["user_nopwd"] + + r = self.client.post( + self.url, {"password1": "plop2fois", "password2": "plop2fois"} + ) + + self.assertRedirects( + r, reverse("account_set_password"), fetch_redirect_response=False + ) + user.refresh_from_db() + self.assertTrue(user.check_password("plop2fois")) + + +class ResetPasswordViewTests(ViewTestCaseMixin, TestCase): + url_name = "account_reset_password" + url_expected = "/profil/password/reset/" + + http_methods = ["GET", "POST"] + + def setUp(self): + super().setUp() + user = self.users["user"] + user.email = "user@mail.net" + user.save() + + def test_get(self): + """ + Users can access the page. + """ + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + def test_post(self): + """ + Users can ask for a link to be sent to reset their password. + """ + r = self.client.post(self.url, {"email": "user@mail.net"}) + + self.assertRedirects(r, reverse("account_reset_password_done")) + get_reset_password_link(mail.outbox[0]) + + +class ResetPasswordKeyViewTests(TestCase): + def setUp(self): + self.user = User.objects.create_user("user", "user@mail.net", "user") + self.client.post(reverse("account_reset_password"), {"email": "user@mail.net"}) + self.reset_link = get_reset_password_link(mail.outbox[0]) + + def test_get(self): + """ + With valid link, users can access the form to reset their password. + """ + # Redirection happens in order to "hide" the reset key. + r = self.client.get(self.reset_link, follow=True) + + self.assertEqual(r.status_code, 200) + self.assertIn("form", r.context) + + def test_get_bad_token(self): + """ + """ + # Edit the key (remove the last slash, add some keys) + bad_link = self.reset_link[:-1] + "reallybad/" + + r = self.client.get(bad_link, follow=True) + + self.assertEqual(r.status_code, 200) + self.assertNotIn("form", r.context) + + def test_post(self): + """ + If the form is valid, user password is changed. + """ + r = self.client.get(self.reset_link, follow=True) + + r = self.client.post( + r.redirect_chain[-1][0], + {"password1": "thepassword", "password2": "thepassword"}, + ) + + self.assertRedirects(r, reverse("home")) + self.user.refresh_from_db() + self.assertTrue(self.user.check_password("thepassword")) + + +class ClipperAuthTests(CASViewTestCase): + def setUp(self): + self.user = User.objects.create_user("user", "", "user") + self.provider = providers_registry.by_id("clipper") + + # When the Clipper callback view verifies ticket with the CAS server, + # this ensures it will approve the ticket for the identifier + # 'theclipper'. + self.patch_cas_response(username="theclipper", valid_ticket="__all__") + self.callback_url = reverse("clipper_callback") + "?ticket=any" + + def test_login(self): + SocialAccount.objects.create( + provider="clipper", user=self.user, uid="theclipper" + ) + + self.client.get(reverse("clipper_login")) + r = self.client.get(self.callback_url) + + self.assertRedirects(r, reverse("home")) + self.assertEqual(r.wsgi_request.user, self.user) + + def test_autosignup(self): + """ + Connecting via Clipper automatically creates a User instance, if none + is already linked to the used clipper login. + This identifier is used as username (trimmed, lowercased). + """ + self.assertFalse(User.objects.filter(username="theclipper").exists()) + + self.client.get(reverse("clipper_login")) + r = self.client.get(self.callback_url) + + self.assertRedirects(r, reverse("home")) + user = User.objects.get(username="theclipper") + SocialAccount.objects.get(provider="clipper", user=user, uid="theclipper") + self.assertEqual(r.wsgi_request.user, user) + self.assertEqual(user.email, "theclipper@clipper.ens.fr") + + def test_autosignup_conflict_username(self): + """ + When creating User via Clipper auto-signup, if the username is not + available, a similar one is chosen. + """ + User.objects.create_user("theclipper", "", "") + previous_user_pks = list(User.objects.values_list("pk", flat=True)) + + self.client.get(reverse("clipper_login")) + r = self.client.get(self.callback_url) + + self.assertRedirects(r, reverse("home")) + user = User.objects.exclude(pk__in=previous_user_pks).get() + SocialAccount.objects.get(provider="clipper", user=user, uid="theclipper") + self.assertEqual(r.wsgi_request.user, user) + self.assertTrue(user.username.startswith("theclipper")) + self.assertEqual(user.email, "theclipper@clipper.ens.fr") diff --git a/shared/tests/testcases.py b/shared/tests/testcases.py index 19122322..1aa3ef58 100644 --- a/shared/tests/testcases.py +++ b/shared/tests/testcases.py @@ -41,7 +41,7 @@ class TestCaseMixin: querystring["next"] = full_path login_url = "{}?{}".format( - reverse("cof-login"), querystring.urlencode(safe="/") + reverse("account_login"), querystring.urlencode(safe="/") ) # We don't focus on what the login view does. @@ -239,13 +239,6 @@ class ViewTestCaseMixin(TestCaseMixin): """ Warning: Do not forget to call super().setUp() in subclasses. """ - # Signals handlers on login/logout send messages. - # Due to the way the Django' test Client performs login, this raise an - # error. As workaround, we mock the Django' messages module. - patcher_messages = mock.patch("gestioncof.signals.messages") - patcher_messages.start() - self.addCleanup(patcher_messages.stop) - # A test can mock 'django.utils.timezone.now' and give this as return # value. E.g. it is useful if the test checks values of 'auto_now' or # 'auto_now_add' fields. diff --git a/utils/models.py b/utils/models.py new file mode 100644 index 00000000..f4ddb060 --- /dev/null +++ b/utils/models.py @@ -0,0 +1,43 @@ +from django.core.exceptions import ImproperlyConfigured +from django.db import models + + +class NullCharField(models.CharField): + """ + CharField that stores '' as NULL and returns NULL as ''. + Use this for unique CharField. + + As long as this field is referenced in a migration, this definition must be + kept. + + When upgrading to Django >= 1.11, it is possible to drop the methods: + `from_db_value`, `to_python` and `get_prep_value`. + + Source: + https://github.com/django-oscar/django-oscar/blob/1a2e8279161a396e70e44339d7206f25355b33a3/src/oscar/models/fields/__init__.py#L94 + """ + + description = "CharField that stores '' as NULL and returns NULL as ''" + + def __init__(self, *args, **kwargs): + if not kwargs.get("null", True) or not kwargs.get("blank", True): + raise ImproperlyConfigured("NullCharField implies 'null == blank == True'.") + kwargs["null"] = kwargs["blank"] = True + super().__init__(*args, **kwargs) + + def from_db_value(self, value, *args, **kwargs): + return self.to_python(value) + + def to_python(self, *args, **kwargs): + val = super().to_python(*args, **kwargs) + return val if val is not None else "" + + def get_prep_value(self, *args, **kwargs): + prepped = super().get_prep_value(*args, **kwargs) + return prepped if prepped != "" else None + + def deconstruct(self): + name, path, args, kwargs = super().deconstruct() + del kwargs["null"] + del kwargs["blank"] + return name, path, args, kwargs