From f33441324cb71ae8d29a1c612443fc48afeaac8a Mon Sep 17 00:00:00 2001 From: Eric <34304046+EricTRL@users.noreply.github.com> Date: Sun, 17 Sep 2023 20:04:46 +0200 Subject: [PATCH] Moved code around + bugfix Fixed a bug with dispatch order where successful links were redirected to the fail page. Ensured that last_updated_by is set when an account is linked. Moved some code to the utils module. --- core/views.py | 47 +++++++++ membership_file/forms.py | 62 ++---------- membership_file/views.py | 209 +++------------------------------------ utils/forms.py | 53 +++++++++- utils/tokens.py | 143 +++++++++++++++++++++++++++ 5 files changed, 263 insertions(+), 251 deletions(-) create mode 100644 utils/tokens.py diff --git a/core/views.py b/core/views.py index 375b29d0..a98c1fc8 100644 --- a/core/views.py +++ b/core/views.py @@ -62,6 +62,53 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: return context +class LinkedLoginView(LoginView): + """ + A variant of the standard LoginView that shows that some data will be linked to + the Squire account when logging in. This does not actually link any data itself; + subclasses should implement that sort of behaviour. + """ + + image_source = None + image_alt = None + link_title = None + link_description = None + link_extra = None + + def get_image_source(self): + """The image for the data to be linked. Defaults to Squire's logo.""" + return self.image_source + + def get_image_alt(self): + """Alt text for the image.""" + return self.image_alt + + def get_link_title(self): + """Title for the data to be linked.""" + return self.link_title + + def get_link_description(self): + """A description of the data to be linked.""" + return self.link_description + + def get_link_extra(self): + """Any extra information for the data to be linked.""" + return self.link_extra + + def get_context_data(self, **kwargs: Any) -> dict[str, Any]: + context = super().get_context_data(**kwargs) + context.update( + { + "image_source": self.get_image_source(), + "image_alt": self.get_image_alt(), + "link_title": self.get_link_title(), + "link_description": self.get_link_description(), + "link_extra": self.get_link_extra(), + } + ) + return context + + class LogoutSuccessView(TemplateView): """View that is displayed when a user is logged out.""" diff --git a/membership_file/forms.py b/membership_file/forms.py index 833d81c4..b19b323c 100644 --- a/membership_file/forms.py +++ b/membership_file/forms.py @@ -1,13 +1,9 @@ -import copy from typing import Any, Dict from django import forms -from django.conf import settings -from django.contrib import messages from django.contrib.admin.widgets import FilteredSelectMultiple from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ImproperlyConfigured, ValidationError -from django.core.mail import EmailMultiAlternatives, send_mail -from django.forms.models import ModelFormMetaclass +from django.core.mail import EmailMultiAlternatives from django.http import HttpRequest from django.template import loader from django.utils.encoding import force_bytes @@ -18,7 +14,7 @@ from core.forms import LoginForm, RegisterForm from membership_file.models import Member, Room, MemberYear, Membership from membership_file.util import LinkAccountTokenGenerator -from utils.forms import UpdatingUserFormMixin +from utils.forms import FieldsetAdminFormMixin, UpdatingUserFormMixin from utils.widgets import OtherRadioSelect ################################################################################## @@ -116,51 +112,6 @@ def save(self): ) -class FieldsetModelFormMetaclass(ModelFormMetaclass): - """Sets the `_meta.fieldsets` attribute that is required by the admin panel.""" - - def __new__(mcs, name, bases, attrs): - new_class = super().__new__(mcs, name, bases, attrs) - new_class._meta.fieldsets = None - meta_class = getattr(new_class, "Meta", None) - if meta_class is not None: - new_class._meta.fieldsets = getattr(meta_class, "fieldsets", None) - return new_class - - -class FieldsetAdminFormMixin(metaclass=FieldsetModelFormMetaclass): - """ - This mixin allows a form to be used in the admin panel. Notably allows using fieldsets - and default admin widgets (e.g. the datetime picker) - """ - - required_css_class = "required" - - # ModelAdmin media - @property - def media(self): - extra = "" if settings.DEBUG else ".min" - js = [ - "vendor/jquery/jquery%s.js" % extra, - "jquery.init.js", - "core.js", - "admin/RelatedObjectLookups.js", - "actions.js", - "urlify.js", - "prepopulate.js", - "vendor/xregexp/xregexp%s.js" % extra, - ] - return forms.Media(js=["admin/js/%s" % url for url in js]) + super().media - - def get_fieldsets(self, request, obj=None): - """ - Hook for specifying fieldsets. - """ - if self._meta.fieldsets: - return copy.deepcopy(self._meta.fieldsets) - return [(None, {"fields": self.fields})] - - class RegisterMemberForm(UpdatingUserFormMixin, FieldsetAdminFormMixin, forms.ModelForm): """ Registers a member in the membership file, and optionally sends them an email to link or register a Squire account. @@ -404,9 +355,7 @@ def send_mail( class ConfirmLinkMembershipRegisterForm(RegisterForm): - """ - TODO: RequestingUserMixin - """ + """A RegisterForm that, when saved, also links a predetermined member to the newly registered user.""" def __init__(self, member: Member, *args, **kwargs): # Member should not already have an attached user @@ -418,12 +367,14 @@ def _save_m2m(self): super()._save_m2m() # Attach new user to predetermined member self.member.user = self.instance + self.member.last_updated_by = self.instance self.member.save() class ConfirmLinkMembershipLoginForm(LoginForm): """ - TODO: RequestingUserMixin + A LoginForm that, when saved, also links a predetermined member to the logged in user. + Also sets the user's email and name to match that of the linked member. """ def __init__(self, member: Member, *args, **kwargs): @@ -436,6 +387,7 @@ def save(self): # Attach new user to predetermined member user = self.get_user() self.member.user = user + self.member.last_updated_by = user self.member.save() # Update user email and real name diff --git a/membership_file/views.py b/membership_file/views.py index 77b28eef..9684db15 100644 --- a/membership_file/views.py +++ b/membership_file/views.py @@ -1,28 +1,20 @@ -from functools import partial -from typing import Any, Dict, Optional, Type -from django import forms +from typing import Any, Dict, Optional from django.contrib import messages -from django.contrib.auth import get_user_model, login as auth_login -from django.contrib.auth.tokens import PasswordResetTokenGenerator +from django.contrib.auth import get_user_model, login as auth_login, logout as auth_logout from django.contrib.admin import helpers, ModelAdmin from django.core.exceptions import PermissionDenied -from django.db.models import Model -from django.forms import ValidationError from django.forms.models import BaseModelForm from django.http import HttpResponse, HttpResponseRedirect from django.shortcuts import render from django.views import View -from django.views.decorators.cache import never_cache -from django.views.decorators.debug import sensitive_post_parameters from django.views.generic.edit import CreateView from django.views.generic import TemplateView, FormView from django.template.response import TemplateResponse from django.urls import reverse, reverse_lazy -from django.utils.decorators import method_decorator from django.utils.html import format_html -from django.utils.http import urlsafe_base64_decode from dynamic_preferences.registries import global_preferences_registry -from core.views import LoginView, RegisterUserView +from core.views import LinkedLoginView, LoginView, RegisterUserView +from utils.tokens import SessionTokenMixin, UrlTokenMixin UserModel = get_user_model() @@ -211,136 +203,6 @@ def get_success_url(self) -> str: return reverse(f"admin:membership_file_member_actions", args=("register_new_member",)) -class TokenMixinBase: - """ - The basis for a more general implementation of token verification in Django's `PasswordResetConfirmView`. - This base class is used for both `UrlTokenMixin` and `SessionTokenMixin`; refer there for details. - - `fail_template_name`: Name of the template that is rendered whenever token verification fails. - `session_token_name`: Name of the session token stored inside `django_session` - `token_generator`: Token generator used to create and validate tokens. This should be different for each type - of token (e.g. password reset, link account, etc.) - `object_id_kwarg_name`: The url kwarg used to pass the base64-encoded id of the object - `object_class`: The model class that objects passed to this view belong to. E.g. User for Django's password resets - """ - - # Subclasses should override this - fail_template_name = "fail_template_name: TokenMixin fail placeholder" - session_token_name: str = None - token_generator: PasswordResetTokenGenerator = None - object_id_kwarg_name = "uidb64" - object_class: Type[Model] = UserModel - - def get_url_object(self, uidb64: str): - """ - Decodes the base64-encoded id for an object of type `object_class` from the URL. If - no object can be decoded, then `None` is returned instead. - Equivalent to `PasswordResetConfirmView.get_user` - """ - try: - # urlsafe_base64_decode() decodes to bytestring - uid = urlsafe_base64_decode(uidb64).decode() - url_object = self.object_class._default_manager.get(pk=uid) - except (TypeError, ValueError, OverflowError, self.object_class.DoesNotExist, ValidationError): - url_object = None - return url_object - - def delete_token(self): - """Deletes the token from the session data""" - del self.request.session[self.session_token_name] - - def dispatch(self, validlink, *args, **kwargs): - # Render fail template if the URL is invalid - if not validlink: - return render(self.request, self.fail_template_name) - return super().dispatch(*args, **kwargs) - - -class UrlTokenMixin(TokenMixinBase): - """ - Stores a token passed through a URL in the session data. This allows it to be reused later, and - avoids the possibility of leaking the token in the HTTP Referer header. - - This class generalizes Django's `PasswordResetConfirmView` token handling. The behaviour of - `dispatch` is pretty much identical, but instead plugs in overridable methods and class variables - to allow this mixin to be reused. - - Views using this mixin have a URL formatted like `foo//`, where the - token is a long hash generated by the token generator. This hash contains various properties of the passed - object that must have changed once the link/token should become invalid. In order to not leak the token, - the token is first removed from the URL and replaced by `url_token_name`. The token is also saved in the - session data so it can actually be used and verified later on. - - `url_token_name`: The value the token is set to in the URL once the token is stored in the session data. - Identical behaviour to `reset_url_token` in Django's `PasswordResetConfirmView` - `token_kwarg_name`: The url kwarg used to pass the token - """ - - # Subclasses should override this - url_token_name: str = None - token_kwarg_name = "token" - - @method_decorator(sensitive_post_parameters()) - @method_decorator(never_cache) - def dispatch(self, *args, **kwargs): - assert self.object_id_kwarg_name in kwargs and self.token_kwarg_name in kwargs - - self.validlink = False - self.url_object = self.get_url_object(kwargs[self.object_id_kwarg_name]) - - # View is only valid if a url object was passed - if self.url_object is not None: - token = kwargs[self.token_kwarg_name] - if token == self.url_token_name: - session_token = self.request.session.get(self.session_token_name) - if self.token_generator.check_token(self.url_object, session_token): - # If the token is valid, display the link account form. - self.validlink = True - return super().dispatch(self.validlink, *args, **kwargs) - else: - if self.token_generator.check_token(self.url_object, token): - # Store the token in the session and redirect to the - # link account form at a URL without the token. That - # avoids the possibility of leaking the token in the - # HTTP Referer header. - self.request.session[self.session_token_name] = token - redirect_url = self.request.path.replace(token, self.url_token_name) - return HttpResponseRedirect(redirect_url) - - # Token was invalid - return super().dispatch(self.validlink, *args, **kwargs) - - -class SessionTokenMixin(TokenMixinBase): - """ - A more simplistic mixin that allows verifying a token already present in the session data. - This mixin is thus only useful in combination with `UrlTokenMixin`. Specifically, only when a - view using `UrlTokenMixin` redirects to a view using this mixin. - - This class generalizes a fraction of Django's `PasswordResetConfirmView` token handling. The behaviour of - `dispatch` mimics the session token verification, but instead plugs in overridable methods and class variables - to allow this mixin to be reused. - """ - - @method_decorator(sensitive_post_parameters()) - @method_decorator(never_cache) - def dispatch(self, *args, **kwargs): - assert self.object_id_kwarg_name in kwargs - - self.validlink = False - self.url_object = self.get_url_object(kwargs[self.object_id_kwarg_name]) - - # View is only valid if a url object was passed - if self.url_object is not None: - session_token = self.request.session.get(self.session_token_name) - if self.token_generator.check_token(self.url_object, session_token): - # If the token is valid, display the link account form. - self.validlink = True - return super().dispatch(self.validlink, *args, **kwargs) - - return super().dispatch(self.validlink, *args, **kwargs) - - class LinkMembershipViewTokenMixin: """ A mixin to be used in combination with `UrlTokenMixin` or `SessionTokenMixin`. @@ -365,11 +227,10 @@ def get_url_object(self, uidb64: str): return member def dispatch(self, *args, **kwargs): - res = super().dispatch(*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 res + return super().dispatch(*args, **kwargs) class LinkMembershipConfirmView(LinkMembershipViewTokenMixin, UrlTokenMixin, View): @@ -408,11 +269,10 @@ class LinkMembershipRegisterView(LinkMembershipViewTokenMixin, SessionTokenMixin success_url = reverse_lazy("account:membership:view") def dispatch(self, *args, **kwargs): - res = super().dispatch(*args, **kwargs) # Fail if a user is logged in if self.request.user.is_authenticated: return render(self.request, self.fail_template_name) - return res + return super().dispatch(*args, **kwargs) def get_login_url(self): return reverse("membership:link_account/login", args=(self.kwargs[self.object_id_kwarg_name],)) @@ -440,59 +300,11 @@ def form_valid(self, form: ConfirmLinkMembershipRegisterForm): return super().form_valid(form) -class LinkedLoginView(LoginView): - """ - A variant of the standard LoginView that shows that some data will be linked to - the Squire account when logging in. This does not actually link any data itself; - subclasses should implement that sort of behaviour. - """ - - image_source = None - image_alt = None - link_title = None - link_description = None - link_extra = None - - def get_image_source(self): - """The image for the data to be linked. Defaults to Squire's logo.""" - return self.image_source - - def get_image_alt(self): - """Alt text for the image.""" - return self.image_alt - - def get_link_title(self): - """Title for the data to be linked.""" - return self.link_title - - def get_link_description(self): - """A description of the data to be linked.""" - return self.link_description - - def get_link_extra(self): - """Any extra information for the data to be linked.""" - return self.link_extra - - def get_context_data(self, **kwargs: Any) -> dict[str, Any]: - context = super().get_context_data(**kwargs) - context.update( - { - "image_source": self.get_image_source(), - "image_alt": self.get_image_alt(), - "link_title": self.get_link_title(), - "link_description": self.get_link_description(), - "link_extra": self.get_link_extra(), - } - ) - return context - - class LinkMembershipLoginView(LinkMembershipViewTokenMixin, SessionTokenMixin, LinkedLoginView): """ Shows a login form which, when filled, attached a predetermined member to the user that was logged in. If a user is already logged in, they should still enter their credentials. TODO: Skip asking for login credentials if the user had already logged in very recently (e.g. 5 minutes ago) - TODO: Additional cleaning: A user that successfully logs in should not already have an associated member """ authentication_form = ConfirmLinkMembershipLoginForm @@ -525,6 +337,15 @@ def get_form_kwargs(self): return kwargs def form_valid(self, form: ConfirmLinkMembershipLoginForm): + # If the logged in user already has an attached member, abort + if hasattr(form.get_user(), "member"): + messages.error(self.request, "This account already has a linked member.") + # Logout user, but keep the token (session data is flushed) + token = self.request.session[self.session_token_name] + auth_logout(self.request) + self.request.session[self.session_token_name] = token + return self.render_to_response(self.get_context_data()) + form.save() self.delete_token() messages.success(self.request, "Membership data linked successfully!") diff --git a/utils/forms.py b/utils/forms.py index b7333cef..1fc838d6 100644 --- a/utils/forms.py +++ b/utils/forms.py @@ -1,5 +1,8 @@ +import copy from django import forms +from django.conf import settings from django.core.exceptions import ObjectDoesNotExist +from django.forms.models import ModelFormMetaclass def get_basic_filter_by_field_form(field_name): @@ -57,7 +60,7 @@ def save(self, commit=True): class RequestUserToFormModelAdminMixin: """ - Mixin that passes the requeesting user as a kwarg to + Mixin that passes the requesting user as a kwarg to the ModelAdmin's Form. """ @@ -74,7 +77,8 @@ def __new__(cls, *args, **kwargs): class FormGroup: - """A formgroup functions as a shell to a form and any number of formsets. This is ideal when editing an + """ + A formgroup functions as a shell to a form and any number of formsets. This is ideal when editing an object along with some directly related objects. Can be used on FormViews form_class: The class of the main form formset_class: The class of the formset @@ -146,3 +150,48 @@ def save(self): for formset in self.formsets: if hasattr(formset, "save"): formset.save() + + +class FieldsetModelFormMetaclass(ModelFormMetaclass): + """Sets the `_meta.fieldsets` attribute that is required by the admin panel.""" + + def __new__(mcs, name, bases, attrs): + new_class = super().__new__(mcs, name, bases, attrs) + new_class._meta.fieldsets = None + meta_class = getattr(new_class, "Meta", None) + if meta_class is not None: + new_class._meta.fieldsets = getattr(meta_class, "fieldsets", None) + return new_class + + +class FieldsetAdminFormMixin(metaclass=FieldsetModelFormMetaclass): + """ + This mixin allows a form to be used in the admin panel. Notably allows using fieldsets + and default admin widgets (e.g. the datetime picker) + """ + + required_css_class = "required" + + # ModelAdmin media + @property + def media(self): + extra = "" if settings.DEBUG else ".min" + js = [ + "vendor/jquery/jquery%s.js" % extra, + "jquery.init.js", + "core.js", + "admin/RelatedObjectLookups.js", + "actions.js", + "urlify.js", + "prepopulate.js", + "vendor/xregexp/xregexp%s.js" % extra, + ] + return forms.Media(js=["admin/js/%s" % url for url in js]) + super().media + + def get_fieldsets(self, request, obj=None): + """ + Hook for specifying fieldsets. + """ + if self._meta.fieldsets: + return copy.deepcopy(self._meta.fieldsets) + return [(None, {"fields": self.fields})] diff --git a/utils/tokens.py b/utils/tokens.py new file mode 100644 index 00000000..be4f4ebb --- /dev/null +++ b/utils/tokens.py @@ -0,0 +1,143 @@ +from typing import Type +from django.contrib.auth import get_user_model +from django.contrib.auth.tokens import PasswordResetTokenGenerator +from django.db.models import Model +from django.forms import ValidationError +from django.http import HttpResponseRedirect +from django.shortcuts import render +from django.utils.decorators import method_decorator +from django.utils.http import urlsafe_base64_decode +from django.views.decorators.cache import never_cache +from django.views.decorators.debug import sensitive_post_parameters + +UserModel = get_user_model() + + +class TokenMixinBase: + """ + The basis for a more general implementation of token verification in Django's `PasswordResetConfirmView`. + This base class is used for both `UrlTokenMixin` and `SessionTokenMixin`; refer there for details. + + `fail_template_name`: Name of the template that is rendered whenever token verification fails. + `session_token_name`: Name of the session token stored inside `django_session` + `token_generator`: Token generator used to create and validate tokens. This should be different for each type + of token (e.g. password reset, link account, etc.) + `object_id_kwarg_name`: The url kwarg used to pass the base64-encoded id of the object + `object_class`: The model class that objects passed to this view belong to. E.g. User for Django's password resets + """ + + # Subclasses should override this + fail_template_name = "fail_template_name: TokenMixin fail placeholder" + session_token_name: str = None + token_generator: PasswordResetTokenGenerator = None + object_id_kwarg_name = "uidb64" + object_class: Type[Model] = UserModel + + def get_url_object(self, uidb64: str): + """ + Decodes the base64-encoded id for an object of type `object_class` from the URL. If + no object can be decoded, then `None` is returned instead. + Equivalent to `PasswordResetConfirmView.get_user` + """ + try: + # urlsafe_base64_decode() decodes to bytestring + uid = urlsafe_base64_decode(uidb64).decode() + url_object = self.object_class._default_manager.get(pk=uid) + except (TypeError, ValueError, OverflowError, self.object_class.DoesNotExist, ValidationError): + url_object = None + return url_object + + def delete_token(self): + """Deletes the token from the session data""" + del self.request.session[self.session_token_name] + + def dispatch(self, validlink, *args, **kwargs): + # Render fail template if the URL is invalid + if not validlink: + return render(self.request, self.fail_template_name) + return super().dispatch(*args, **kwargs) + + +class UrlTokenMixin(TokenMixinBase): + """ + Stores a token passed through a URL in the session data. This allows it to be reused later, and + avoids the possibility of leaking the token in the HTTP Referer header. + + This class generalizes Django's `PasswordResetConfirmView` token handling. The behaviour of + `dispatch` is pretty much identical, but instead plugs in overridable methods and class variables + to allow this mixin to be reused. + + Views using this mixin have a URL formatted like `foo//`, where the + token is a long hash generated by the token generator. This hash contains various properties of the passed + object that must have changed once the link/token should become invalid. In order to not leak the token, + the token is first removed from the URL and replaced by `url_token_name`. The token is also saved in the + session data so it can actually be used and verified later on. + + `url_token_name`: The value the token is set to in the URL once the token is stored in the session data. + Identical behaviour to `reset_url_token` in Django's `PasswordResetConfirmView` + `token_kwarg_name`: The url kwarg used to pass the token + """ + + # Subclasses should override this + url_token_name: str = None + token_kwarg_name = "token" + + @method_decorator(sensitive_post_parameters()) + @method_decorator(never_cache) + def dispatch(self, *args, **kwargs): + assert self.object_id_kwarg_name in kwargs and self.token_kwarg_name in kwargs + + self.validlink = False + self.url_object = self.get_url_object(kwargs[self.object_id_kwarg_name]) + + # View is only valid if a url object was passed + if self.url_object is not None: + token = kwargs[self.token_kwarg_name] + if token == self.url_token_name: + session_token = self.request.session.get(self.session_token_name) + if self.token_generator.check_token(self.url_object, session_token): + # If the token is valid, display the link account form. + self.validlink = True + return super().dispatch(self.validlink, *args, **kwargs) + else: + if self.token_generator.check_token(self.url_object, token): + # Store the token in the session and redirect to the + # link account form at a URL without the token. That + # avoids the possibility of leaking the token in the + # HTTP Referer header. + self.request.session[self.session_token_name] = token + redirect_url = self.request.path.replace(token, self.url_token_name) + return HttpResponseRedirect(redirect_url) + + # Token was invalid + return super().dispatch(self.validlink, *args, **kwargs) + + +class SessionTokenMixin(TokenMixinBase): + """ + A more simplistic mixin that allows verifying a token already present in the session data. + This mixin is thus only useful in combination with `UrlTokenMixin`. Specifically, only when a + view using `UrlTokenMixin` redirects to a view using this mixin. + + This class generalizes a fraction of Django's `PasswordResetConfirmView` token handling. The behaviour of + `dispatch` mimics the session token verification, but instead plugs in overridable methods and class variables + to allow this mixin to be reused. + """ + + @method_decorator(sensitive_post_parameters()) + @method_decorator(never_cache) + def dispatch(self, *args, **kwargs): + assert self.object_id_kwarg_name in kwargs + + self.validlink = False + self.url_object = self.get_url_object(kwargs[self.object_id_kwarg_name]) + + # View is only valid if a url object was passed + if self.url_object is not None: + session_token = self.request.session.get(self.session_token_name) + if self.token_generator.check_token(self.url_object, session_token): + # If the token is valid, display the link account form. + self.validlink = True + return super().dispatch(self.validlink, *args, **kwargs) + + return super().dispatch(self.validlink, *args, **kwargs)