Tests + Registration permission fix

Fixed a bug where the member registration admin view as accessible to any staff member.

Added tests for MemberAdmin, RegisterNewMemberAdminView, LInkMembersihpConfirmView (and related classes), LinkAccountTokenGenerator, RegisterMemberForm, LinkedLoginView, LoginForm, RegisterForm,

core/fixtures/test_users.json

@@ -53,6 +53,24 @@
             "user_permissions": []
         }
     },
+    {
+        "model": "auth.user",
+        "pk": 4,
+        "fields": {
+            "password": "pbkdf2_sha256$150000$FQNl6kahiWF9$UMJi+YpgjsAidu6lhrQen5qPyHVa2JBSH6Ve2/WKy/k=",
+            "last_login": "1970-01-01T12:00:00Z",
+            "is_superuser": false,
+            "username": "test_staff",
+            "first_name": "",
+            "last_name": "",
+            "email": "",
+            "is_staff": true,
+            "is_active": true,
+            "date_joined": "1970-01-01T12:00:00Z",
+            "groups": [],
+            "user_permissions": []
+        }
+    },
     {
         "model": "auth.user",
         "pk": 100,

core/forms.py

@@ -110,6 +110,7 @@ def clean_email(self):
             )
         return cleaned_email
 
+
 # Adds the relevant bootstrap classes to the password change form
 class PasswordChangeForm(DjangoPasswordChangeForm):
     pass

core/tests/tests_forms.py

@@ -10,7 +10,7 @@
 from martor.widgets import MartorWidget
 
 from core.fields import MarkdownObject, MarkdownCharField, MarkdownTextField
-from core.forms import MarkdownForm
+from core.forms import LoginForm, MarkdownForm
 from core.models import MarkdownImage
 from core.widgets import ImageUploadMartorWidget
 
@@ -29,9 +29,7 @@ class Meta:
 
 
 class MarkdownFormTest(TestCase):
-    """
-    Tests for MarkdownForm
-    """
+    """Tests for MarkdownForm"""
 
     def setUp(self):
         # Use an Admin-panel LogEntry object to test (it has a FK to user)

core/tests/tests_frontend.py

@@ -17,36 +17,37 @@
 ##################################################################################
 
 
-# Tests whether front-end pages can be accessed
 class FrontEndTest(TestCase):
+    """Tests whether front-end pages can be accessed"""
+
     fixtures = TestAccountUser.get_fixtures()
 
-    # Tests if the login page can be accessed
     def test_login(self):
+        """Tests if the login page can be accessed"""
         check_http_response(self, settings.LOGIN_URL, "get", TestPublicUser)
 
-    # Tests if the logout-success page can be accessed
     def test_logout_success(self):
+        """Tests if the logout-success page can be accessed"""
         check_http_response(self, settings.LOGOUT_REDIRECT_URL, "get", TestPublicUser)
 
-    # Tests if the logout-success page can be accessed if logged in
     def test_logout_success_when_logged_in(self):
+        """Tests if the logout-success page can be accessed if logged in"""
         check_http_response(
             self, settings.LOGOUT_REDIRECT_URL, "get", TestAccountUser, redirect_url=settings.LOGOUT_REDIRECT_URL
         )
 
-    # Tests if the logout page can be accessed if not logged in
     def test_logout_redirect(self):
+        """Tests if the logout page can be accessed if not logged in"""
         check_http_response(
             self, settings.LOGOUT_URL, "get", TestPublicUser, redirect_url=settings.LOGOUT_REDIRECT_URL
         )
 
-    # Tests if the register-page can be accessed
     def test_register(self):
+        """Tests if the register-page can be accessed"""
         check_http_response(self, "/register", "get", TestPublicUser)
 
-    # Tests if the register-success page can be accessed
     def test_register_success(self):
+        """Tests if the register-success page can be accessed"""
         check_http_response(self, "/register/success", "get", TestPublicUser)
 
     def test_homepage_message(self):
@@ -106,16 +107,17 @@ def test_newsletter(self):
         self._test_global_preference_page_404("newsletter__share_link", reverse("core:newsletters"), TestMemberUser)
 
 
-# Tests the login form
 class LoginFormTest(TestCase):
+    """Tests the login form"""
+
     def setUp(self):
         # Called each time before a testcase runs
         # Set up data for each test.
         self.user = User.objects.create_user(username="its-a-me", password="mario")
         User.save(self.user)
 
-    # Test if a login is allowed if the username-password pair are correct
     def test_form_correct(self):
+        """Test if a login is allowed if the username-password pair are correct"""
         form_data = {
             "username": "its-a-me",
             "password": "mario",
@@ -124,8 +126,8 @@ def test_form_correct(self):
         # Data that was entered is correct
         self.assertTrue(form.is_valid())
 
-    # Test if a login is disallowed if the username-password pair are incorrect
     def test_form_incorrect(self):
+        """Test if a login is disallowed if the username-password pair are incorrect"""
         form_data = {
             "username": "its-a-me",
             "password": "luigi",
@@ -140,8 +142,8 @@ def test_form_incorrect(self):
         self.assertEqual(len(form.non_field_errors().as_data()), 1)
         self.assertEqual(form.non_field_errors().as_data()[0].code, "ERROR_INVALID_LOGIN")
 
-    # Test if a login is disallowed if the username is missing
     def test_form_username_missing(self):
+        """Test if a login is disallowed if the username is missing"""
         form_data = {
             "password": "wario",
         }
@@ -154,9 +156,15 @@ def test_form_username_missing(self):
         self.assertTrue(form.has_error("username"))
         self.assertEqual(len(form.errors.as_data()), 1)
 
+    def test_default_disable(self):
+        """Tests if the username field is disabled if initial data is passed"""
+        form = LoginForm(initial={"username": "user"})
+        self.assertTrue(form.fields["username"].disabled)
+
 
-# Tests the register form
 class RegisterFormTest(TestCase):
+    """Tests the register form"""
+
     def setUp(self):
         # Called each time before a testcase runs
         # Set up data for each test.
@@ -165,8 +173,8 @@ def setUp(self):
         )
         User.save(self.user)
 
-    # Test if the user can register if everything is correct
     def test_form_correct(self):
+        """Test if the user can register if everything is correct"""
         form_data = {
             "username": "schaduwkandi",
             "password1": "bestaatookniet",
@@ -197,8 +205,8 @@ def test_form_correct(self):
         self.assertTrue(user.check_password("bestaatookniet"))
         self.assertEqual(user.first_name, "Schaduw Kandi")
 
-    # Test if a registering fails if required fields are missing
     def test_form_fields_missing(self):
+        """Test if a registering fails if required fields are missing"""
         form_data = {
             "first_name": "empty",
         }
@@ -218,8 +226,8 @@ def test_form_fields_missing(self):
         self.assertEqual(len(form.errors.as_data()["email"]), 1)
         self.assertEqual(len(form.errors.as_data()), 4)
 
-    # Test if a registering fails if the two passwords do not match
     def test_form_nonmatching_password(self):
+        """Test if a registering fails if the two passwords do not match"""
         form_data = {
             "username": "schaduwkandi",
             "password1": "bestaatookniet",
@@ -236,8 +244,8 @@ def test_form_nonmatching_password(self):
         self.assertTrue(form.has_error("password2"))
         self.assertEqual(len(form.errors.as_data()["password2"]), 1)
 
-    # Test if a registering fails if username or email was already chosen by another user
     def test_form_duplicate_field(self):
+        """Test if a registering fails if username or email was already chosen by another user"""
         form_data = {
             "username": "schaduwbestuur",
             "password1": "secret",
@@ -256,13 +264,37 @@ def test_form_duplicate_field(self):
         self.assertTrue(form.has_error("email"))
         self.assertEqual(len(form.errors.as_data()["email"]), 1)
 
+    def test_default_disable(self):
+        """Tests if the first_name and email fields are disabled if initial data is passed"""
+        # Email
+        form = RegisterForm(initial={"email": "mail@example.com"})
+        self.assertFalse(form.fields["first_name"].disabled)
+        self.assertTrue(form.fields["email"].disabled)
+
+        # First name
+        form = RegisterForm(initial={"first_name": "Name"})
+        self.assertTrue(form.fields["first_name"].disabled)
+        self.assertFalse(form.fields["email"].disabled)
+
+        # Both
+        form = RegisterForm(initial={"first_name": "Name", "email": "mail@example.com"})
+        self.assertTrue(form.fields["first_name"].disabled)
+        self.assertTrue(form.fields["email"].disabled)
+
+    def test_fields_required(self):
+        """Tests if all form fields are required"""
+        form = RegisterForm()
+        self.assertTrue(form.fields["first_name"].required)
+        self.assertTrue(form.fields["email"].required)
+
 
-# Tests the registerForm view
 class RegisterFormViewTest(TestCase):
+    """Tests the registerForm view"""
+
     fixtures = TestAccountUser.get_fixtures()
 
-    # Tests if redirected when form data was entered correctly
     def test_success_redirect(self):
+        """Tests if redirected when form data was entered correctly"""
         form_data = {
             "username": "username",
             "password1": "thisactuallyneedstobeagoodpassword",
@@ -280,8 +312,8 @@ def test_success_redirect(self):
         self.assertTrue(user.check_password("thisactuallyneedstobeagoodpassword"))
         self.assertEqual(user.first_name, "My Real name")
 
-    # Tests if not redirected when form data was entered incorrectly
     def test_fail_form_enter_no_first_name(self):
+        """Tests if not redirected when form data was entered incorrectly"""
         form_data = {
             "username": "username",
             "password1": "thisactuallyneedstobeagoodpassword",  # Real name not passed
@@ -293,8 +325,8 @@ def test_fail_form_enter_no_first_name(self):
         user = User.objects.filter(username="username").first()
         self.assertIsNone(user)
 
-    # Tests if not redirected when form data was entered incorrectly
     def test_fail_form_enter(self):
+        """Tests if not redirected when form data was entered incorrectly"""
         form_data = {
             "username": "username",
             "password1": "password",  # Password too easy so should fail

core/tests/tests_views.py

@@ -0,0 +1,39 @@
+from django.test import TestCase, RequestFactory
+from core.views import LinkedLoginView
+
+class LinkedLoginTestCase(TestCase):
+    """Tests for LinkedLoginView"""
+
+    class DummyLinkedLoginView(LinkedLoginView):
+        """Linked login with some overrides"""
+        image_source = "image.png"
+        image_alt = "Image"
+        link_title = "Link account"
+        link_description = "Links account to something"
+        link_extra = "Some extra info"
+
+    view_class = DummyLinkedLoginView
+
+    def setUp(self) -> None:
+        self.view = self.view_class()
+        request_factory = RequestFactory()
+        self.view.setup(request_factory.get(f"/my_path/"))
+
+        return super().setUp()
+
+    def test_helper_methods(self):
+        """Tests helper methods to retrieve class variables"""
+        self.assertEqual(self.view.get_image_source(), self.view_class.image_source)
+        self.assertEqual(self.view.get_image_alt(), self.view_class.image_alt)
+        self.assertEqual(self.view.get_link_title(), self.view_class.link_title)
+        self.assertEqual(self.view.get_link_description(), self.view_class.link_description)
+        self.assertEqual(self.view.get_link_extra(), self.view_class.link_extra)
+
+    def test_context(self):
+        """Tests if context variables are passed"""
+        context = self.view.get_context_data()
+        self.assertIn("image_source", context)
+        self.assertIn("image_alt", context)
+        self.assertIn("link_title", context)
+        self.assertIn("link_description", context)
+        self.assertIn("link_extra", context)

core/views.py

@@ -68,6 +68,7 @@ class LinkedLoginView(LoginView):
     the Squire account when logging in. This does not actually link any data itself;
     subclasses should implement that sort of behaviour.
     """
+    template_name = "core/user_accounts/login_linked.html"
 
     image_source = None
     image_alt = None

membership_file/admin.py

@@ -1,12 +1,7 @@
 from datetime import datetime
-from typing import List
 
 from django.contrib import admin, messages
-from django.contrib.contenttypes.models import ContentType
-from django.http import HttpResponseRedirect
 from django_object_actions import DjangoObjectActions, action as object_action
-from django.urls import reverse, path
-from django.urls.resolvers import URLPattern
 from import_export.admin import ExportActionMixin
 from import_export.formats.base_formats import CSV, TSV, ODS, XLSX
 

membership_file/forms.py

@@ -280,7 +280,7 @@ def _save_m2m(self):
         # This is done in _save_m2m as the member-instance must exist. This is only the case if commit=True
         super()._save_m2m()
         # Create membership in selected active years
-        if self.cleaned_data["active_years"]:
+        if "active_years" in self.cleaned_data and self.cleaned_data["active_years"]:
             # No need to do this if there were no active years to begin with
             years = MemberYear.objects.filter(id__in=self.cleaned_data["active_years"])
 
@@ -290,8 +290,8 @@ def _save_m2m(self):
                 Membership.objects.create(member=self.instance, year=year, created_by=created_member)
 
         # Room access
-        if self.cleaned_data["room_access"]:
-            self.instance.accessible_rooms.add(self.cleaned_data["room_access"])
+        if "room_access" in self.cleaned_data and self.cleaned_data["room_access"]:
+            self.instance.accessible_rooms.add(*self.cleaned_data["room_access"])
 
         # Only send out an email once the member is actually saved
         if self.cleaned_data["send_registration_email"]:

membership_file/templates/membership_file/testing/registration_email.html

@@ -0,0 +1 @@
+HTML EMAIL!