From 05eeb6a25cd606394e1577fd5f94ca2ebfe27e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Delobelle?= Date: Thu, 19 Oct 2017 01:12:52 +0200 Subject: [PATCH] core -- Install django-allauth-ens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refer to allauth doc for an accurate features list: http://django-allauth.readthedocs.io/en/latest/ Users can now change their password, ask for a password reset, or set one if they don't have one. In particular, it allows users whose account has been created via a clipper authentication to configure a password before losing their clipper. Even if they have already lost it, they are able to get one using the "Reset password" functionality. Allauth multiple emails management is deactivated. Requests to the related url redirect to the home page. All the login and logout views are replaced by the allauth' ones. It also concerns the Django and Wagtail admin sites. Note that users are no longer logged out of the clipper CAS server when they authenticated via this server. Instead a message suggests the user to disconnect. Clipper connections and `login_clipper` --------------------------------------- - Non-empty `login_clipper` are now unique among `CofProfile` instances. - They are created once for users with a non-empty 'login_clipper' (with the data migration 0014_create_clipper_connections). - The `login_clipper` of CofProfile instances are sync with their clipper connections: * `CofProfile.sync_clipper_connections` method updates the connections based on `login_clipper`. * Signals receivers `sync_clipper…` update `login_clipper` based on connections creations/updates/deletions. Misc ---- - Add NullCharField (model field) which allows to use `unique=True` on CharField (even with empty strings). - Parts of kfet mixins for TestCase are now in shared.tests.testcase, as they are used elsewhere than in the kfet app. --- bda/tests/test_views.py | 19 +- cof/settings/common.py | 31 +- cof/urls.py | 86 +++-- .../migrations/0015_unique_login_clipper.py | 38 ++ .../migrations/0016_create_clipper_conns.py | 35 ++ gestioncof/models.py | 53 ++- gestioncof/petits_cours_views.py | 9 +- gestioncof/shared.py | 18 - gestioncof/signals.py | 52 ++- gestioncof/static/css/cof.css | 18 - gestioncof/templates/cof-denied.html | 5 - gestioncof/templates/error.html | 14 - .../templates/gestioncof/base_header.html | 8 +- gestioncof/templates/home.html | 32 +- gestioncof/tests/test_models.py | 155 ++++++++ gestioncof/tests/test_views.py | 20 +- gestioncof/views.py | 58 +-- kfet/auth/tests.py | 8 - kfet/auth/views.py | 2 +- kfet/open/tests.py | 12 +- kfet/templates/kfet/base_nav.html | 11 +- kfet/tests/test_statistic.py | 9 +- kfet/tests/testcases.py | 259 +------------ requirements.txt | 2 +- shared/__init__.py | 0 shared/allauth_adapter.py | 21 ++ shared/tests/test_auth.py | 354 ++++++++++++++++++ shared/tests/testcases.py | 9 +- utils/models.py | 43 +++ 29 files changed, 877 insertions(+), 504 deletions(-) create mode 100644 gestioncof/migrations/0015_unique_login_clipper.py create mode 100644 gestioncof/migrations/0016_create_clipper_conns.py delete mode 100644 gestioncof/templates/cof-denied.html delete mode 100644 gestioncof/templates/error.html create mode 100644 gestioncof/tests/test_models.py create mode 100644 shared/__init__.py create mode 100644 shared/allauth_adapter.py create mode 100644 shared/tests/test_auth.py create mode 100644 utils/models.py 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