Skip to content

Commit

Permalink
Invalid render + configuration errors + Util tests
Browse files Browse the repository at this point in the history
Added TokenMixinBase.token_invalid, which is called whenever token verification fails. This method returns a TemplateResponse with a 40x HTTP status code.

Added several configuration errors to TokenMixinBase and related classes. Not setting these values can lead to insecurities (or errors).

Added tests for OtherRadioSelect (widget), ModelAdminFormViewMixin, FieldsetAdminFormMixin, and the various TokenMixin views.
  • Loading branch information
EricTRL committed Sep 23, 2023
1 parent 9e805e1 commit 431e27a
Show file tree
Hide file tree
Showing 12 changed files with 531 additions and 77 deletions.
7 changes: 3 additions & 4 deletions membership_file/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,9 @@ def __init__(self, request: HttpRequest, token_generator: LinkAccountTokenGenera
super().__init__(*args, **kwargs)

# Make more fields required
# TODO
# req_fields = ('street', 'house_number', 'postal_code', 'city', 'country', 'date_of_birth')
# for field in req_fields:
# self.fields[field].required = True
req_fields = ('street', 'house_number', 'postal_code', 'city', 'country', 'date_of_birth')
for field in req_fields:
self.fields[field].required = True

# Add field to automatically create memberships in one or more active years
choices = [(year.id, year.name) for year in MemberYear.objects.filter(is_active=True)]
Expand Down
2 changes: 1 addition & 1 deletion membership_file/processor.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
def member_context(request):
return {
"member": request.member,
"member": getattr(request, "member", None),
}
2 changes: 1 addition & 1 deletion membership_file/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_successful_get(self):
def test_succesful_post(self):
response = self.client.post(self.get_base_url(), data={}, follow=True)
self.assertRedirects(response, reverse("membership:continue_success"))
msg = "Succesfully extended Knights membership into {year}".format(year=MemberYear.objects.get(id=3))
msg = "Successfully extended Knights membership into {year}".format(year=MemberYear.objects.get(id=3))
self.assertHasMessage(response, level=messages.SUCCESS, text=msg)

@suppress_warnings
Expand Down
70 changes: 4 additions & 66 deletions membership_file/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from dynamic_preferences.registries import global_preferences_registry
from core.views import LinkedLoginView, LoginView, RegisterUserView
from utils.tokens import SessionTokenMixin, UrlTokenMixin
from utils.views import ModelAdminFormViewMixin

UserModel = get_user_model()

Expand Down Expand Up @@ -86,7 +87,7 @@ def get_form_kwargs(self):

def form_valid(self, form):
form.save()
msg = f"Succesfully extended Knights membership into {self.year}"
msg = f"Successfully extended Knights membership into {self.year}"
messages.success(self.request, msg)
return super(ExtendMembershipView, self).form_valid(form)

Expand All @@ -95,69 +96,6 @@ class ExtendMembershipSuccessView(MemberMixin, UpdateMemberYearMixin, TemplateVi
template_name = "membership_file/extend_membership_successpage.html"


class ModelAdminFormViewMixin:
"""
A Mixin that allows a ModelForm (e.g in a CreateView) to be rendered
inside a ModelAdmin in the admin panel using features normally available there.
This includes default widgets and styling (e.g. for datetime) and formsets.
The `form_class` must also inherit `membership_file.forms.FieldsetAdminFormMixin`
in order for this to work.
Furthermore, a `model_admin` should be passed in order to instantiate this view.
"""

# Class variable needed as we need to be able to pass this through as_view(..)
model_admin: ModelAdmin = None

def __init__(self, *args, model_admin: ModelAdmin = None, **kwargs) -> None:
assert model_admin is not None
self.model_admin = model_admin
super().__init__(*args, **kwargs)

def get_form(self, form_class: Optional[type[BaseModelForm]] = None) -> BaseModelForm:
# This method should return a form instance
if form_class is None:
form_class = self.get_form_class()

# Use this form_class's excludes instead of those from the ModelAdmin's form_class
exclude = form_class._meta.exclude or ()

# This constructs a form class
# NB: More defaults can be passed into the **kwargs of ModelAdmin.get_form
form_class = self.model_admin.get_form(
self.request,
None,
change=False,
# Fields are defined in the form
fields=None,
# Override standard ModelAdmin form and ignore its exclude list
form=form_class,
exclude=exclude,
)

# Use the newly constructed form class to create a form
return super().get_form(form_class)

def get_context_data(self, **kwargs: Any) -> Dict[str, Any]:
context = super().get_context_data(**kwargs)
form = context.pop("form")
adminForm = helpers.AdminForm(
form, list(form.get_fieldsets(self.request, self.object)), {}, model_admin=self.model_admin
)

context.update(
{
"adminform": adminForm,
"is_nav_sidebar_enabled": True,
"opts": Member._meta,
"title": "Register new member",
}
)

return context


LINK_TOKEN_GENERATOR = LinkAccountTokenGenerator()


Expand Down Expand Up @@ -229,7 +167,7 @@ def get_url_object(self, uidb64: str):
def dispatch(self, *args, **kwargs):
# Fail if the requesting user already has a member
if hasattr(self.request.user, "member"):
return render(self.request, self.fail_template_name)
return self.token_invalid()
return super().dispatch(*args, **kwargs)


Expand Down Expand Up @@ -271,7 +209,7 @@ class LinkMembershipRegisterView(LinkMembershipViewTokenMixin, SessionTokenMixin
def dispatch(self, *args, **kwargs):
# Fail if a user is logged in
if self.request.user.is_authenticated:
return render(self.request, self.fail_template_name)
return self.token_invalid(status=403)
return super().dispatch(*args, **kwargs)

def get_login_url(self):
Expand Down
1 change: 1 addition & 0 deletions utils/templates/utils/testing/token_fail_template.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
TOKEN FAILURE!
75 changes: 75 additions & 0 deletions utils/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from unittest.mock import MagicMock

from utils.forms import (
FieldsetAdminFormMixin,
RequestUserToFormModelAdminMixin,
UpdatingUserFormMixin,
get_basic_filter_by_field_form,
Expand Down Expand Up @@ -200,3 +201,77 @@ def test_formset_prefix(self):
self.assertEqual(form_group.formset_class.call_args.kwargs["prefix"], "formset")
form_group = self._construct_form_group(prefix="Group")
self.assertEqual(form_group.formset_class.call_args.kwargs["prefix"], "Group-formset")


class FieldsetAdminUserForm(FieldsetAdminFormMixin, forms.ModelForm):
"""ModelForm that includes fieldsets."""

class Meta:
model = User
fields = ("username", "first_name", "last_name", "email", "last_login")

fieldsets = [
(None, {"fields": [("username",), "email", "last_login"]}),
("Name", {"fields": [("first_name", "last_name")]}),
]


class FieldsetAdminFormMixinTestCase(TestCase):
"""Tests for forms utilising FieldsetAdminFormMixin"""

class FieldAdminForm(FieldsetAdminFormMixin, forms.ModelForm):
"""ModelForm without fieldsets."""

class Meta:
model = User
fields = ("username", "first_name", "last_name", "email")

class Media:
css = {"all": ("extra-css-file.css",)}
js = ("extra-js-file.js",)

class NoMetaForm(FieldAdminForm, forms.ModelForm):
"""ModelForm without a Meta class"""

def test_media(self):
"""Tests if form media is added and merged correctly"""
media = FieldsetAdminFormMixinTestCase.FieldAdminForm().media.render()
# Additional media is there
self.assertIn("extra-js-file.js", media)
self.assertIn("extra-css-file.css", media)
# Parent media is there
self.assertIn("admin/js/admin/RelatedObjectLookups.js", media)

def test_fieldsets(self):
"""Tests whether fieldsets are properly created"""
# Meta has fieldsets
form = FieldsetAdminUserForm()
fieldsets = form.get_fieldsets(None)
self.assertEqual(len(fieldsets), 2)
self.assertEqual(fieldsets[1][0], "Name")

# Meta has no fieldsets (all fields are added to a single fieldset)
form = FieldsetAdminFormMixinTestCase.FieldAdminForm()
fieldsets = form.get_fieldsets(None)
self.assertEqual(len(fieldsets), 1)
name, attrs = fieldsets[0]
self.assertIsNone(name)
self.assertIn("fields", attrs)
self.assertDictEqual(attrs["fields"], form.fields)

def test_meta(self):
"""Tests whether the _meta.fieldsets attribute is set"""
# No Meta class
form = FieldsetAdminFormMixinTestCase.NoMetaForm()
self.assertTrue(hasattr(form._meta, "fieldsets"))
self.assertIsNone(form._meta.fieldsets)

# Meta class (no fieldset attribute)
form = FieldsetAdminFormMixinTestCase.FieldAdminForm()
self.assertTrue(hasattr(form._meta, "fieldsets"))
self.assertIsNone(form._meta.fieldsets)

# Meta class (fieldset attribute)
form = FieldsetAdminUserForm()
self.assertTrue(hasattr(form._meta, "fieldsets"))
self.assertIsNotNone(form._meta.fieldsets)
Loading

0 comments on commit 431e27a

Please sign in to comment.