From 9e1cf5cf2a7fd36159a30d8734ee88d38ad2760f Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Aug 2024 23:49:21 +0200 Subject: [PATCH 1/9] Add django-ratelimit as dependency --- pyproject.toml | 1 + requirements.txt | 3 +++ requirements/requirements-dev.txt | 3 +++ 3 files changed, 7 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index a14bfbc..cd9b9bb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,7 @@ dependencies = [ "django-registries==0.0.3", "django-view-decorator==0.0.4", "django-oauth-toolkit~=2.4", + "django-ratelimit~=4.1", "django_stubs_ext~=5.0", "stripe~=10.5", ] diff --git a/requirements.txt b/requirements.txt index 418b0bc..1ea4268 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ # - django-allauth~=0.63 # - django-money~=3.5 # - django-oauth-toolkit~=2.4 +# - django-ratelimit~=4.1 # - django-registries==0.0.3 # - django-stubs-ext~=5.0 # - django-view-decorator==0.0.4 @@ -53,6 +54,8 @@ django-money==3.5.3 # via hatch.envs.default django-oauth-toolkit==2.4.0 # via hatch.envs.default +django-ratelimit==4.1.0 + # via hatch.envs.default django-registries==0.0.3 # via hatch.envs.default django-stubs-ext==5.0.4 diff --git a/requirements/requirements-dev.txt b/requirements/requirements-dev.txt index 0617412..ff3a26a 100644 --- a/requirements/requirements-dev.txt +++ b/requirements/requirements-dev.txt @@ -14,6 +14,7 @@ # - django-allauth~=0.63 # - django-money~=3.5 # - django-oauth-toolkit~=2.4 +# - django-ratelimit~=4.1 # - django-registries==0.0.3 # - django-stubs-ext~=5.0 # - django-view-decorator==0.0.4 @@ -81,6 +82,8 @@ django-money==3.5.3 # via hatch.envs.dev django-oauth-toolkit==2.4.0 # via hatch.envs.dev +django-ratelimit==4.1.0 + # via hatch.envs.dev django-registries==0.0.3 # via hatch.envs.dev django-stubs==1.16.0 -- 2.43.4 From 93e80284130b461874f8b0599a93fed854baab1f Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Wed, 7 Aug 2024 14:33:15 +0200 Subject: [PATCH 2/9] WIP: Add generic email functionality, extending for sending out invite emails and order links... --- pyproject.toml | 2 + src/membership/emails.py | 88 +++++++++++++++++++ .../templates/membership/emails/base.txt | 9 ++ .../membership/emails/invitation.txt | 11 +++ src/membership/views.py | 25 ++++++ 5 files changed, 135 insertions(+) create mode 100644 src/membership/emails.py create mode 100644 src/membership/templates/membership/emails/base.txt create mode 100644 src/membership/templates/membership/emails/invitation.txt diff --git a/pyproject.toml b/pyproject.toml index cd9b9bb..7ebb19b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -147,6 +147,8 @@ ignore = [ "TD", # TODO, FIXME, XXX "ANN002", # Missing type annotation for `*args` "ANN003", # Missing type annotation for `**kwargs` + "FBT001", # Misbehaves: Complains about positional arguments that are keyworded. + "FBT002", # Misbehaves: Complains about positional arguments that are keyworded. ] [tool.ruff.lint.isort] diff --git a/src/membership/emails.py b/src/membership/emails.py new file mode 100644 index 0000000..20a243a --- /dev/null +++ b/src/membership/emails.py @@ -0,0 +1,88 @@ +"""Send email to members, using templates and contexts for the emails. + +* We keep everything as plain text for now. +* Notice that emails can be multilingual +* Generally, an email consists of templates (for body and subject) and a get_context() method. +""" + +from django.contrib import messages +from django.contrib.sites.shortcuts import get_current_site +from django.core.mail.message import EmailMessage +from django.http import HttpRequest +from django.template import loader +from django.utils import translation +from django.utils.translation import gettext_lazy as _ + + +class BaseEmail(EmailMessage): + """Send emails via templated body and subjects. + + This base class is extended for all email functionality. + """ + + template = "membership/email/base.txt" + # Optional: Set to a template path for subject + template_subject = None + default_subject = "SET SUBJECT HERE" + + def __init__(self, request: HttpRequest, *args, **kwargs) -> None: # noqa: D107 + self.context = kwargs.pop("context", {}) + self.user = kwargs.pop("user", None) + if self.user: + kwargs["to"] = [self.user.email] + self.context["user"] = self.user + self.context["recipient_name"] = self.user.get_display_name() + + # Necessary to set request before instantiating body and subject + self.request = request + kwargs.setdefault("subject", self.get_subject()) + kwargs.setdefault("body", self.get_body()) + + super().__init__(*args, **kwargs) + + def get_context_data(self) -> dict: + """Resolve common context for sending emails. + + When overwriting, remember to call this via super(). + """ + c = self.context + site = get_current_site(self.request) + c["request"] = self.request + c["domain"] = site.domain + c["site_name"] = site.name + c["protocol"] = "http" if self.request and not self.request.is_secure() else "https" + return c + + def get_body(self) -> str: + """Build the email body from template and context.""" + if self.user and self.user.language_code: + with translation.override(self.user.language_code): + body = loader.render_to_string(self.template, self.get_context_data()) + else: + body = loader.render_to_string(self.template, self.get_context_data()) + return body + + def get_subject(self) -> str: + """Build the email subject from template or self.default_subject.""" + if self.user and self.user.language_code: + with translation.override(self.user.language_code): + if self.template_subject: + subject = loader.render_to_string(self.template_subject, self.get_context_data()).strip() + else: + subject = str(self.default_subject) + elif self.template_subject: + subject = loader.render_to_string(self.template_subject, self.get_context_data()).strip() + else: + subject = str(self.default_subject) + return subject + + def send_with_feedback(self, success_msg: str | None = None, no_message: bool = False) -> None: + """Send email, possibly adding feedback via django.contrib.messages.""" + if not success_msg: + success_msg = _("Email successfully sent to {}").format(", ".join(self.to)) + try: + self.send(fail_silently=False) + if not no_message: + messages.success(self.request, success_msg) + except RuntimeError: + messages.error(self.request, _("Not sent, something wrong with the mail server.")) diff --git a/src/membership/templates/membership/emails/base.txt b/src/membership/templates/membership/emails/base.txt new file mode 100644 index 0000000..9a86384 --- /dev/null +++ b/src/membership/templates/membership/emails/base.txt @@ -0,0 +1,9 @@ +{% load i18n %}{% block greeting %}{% blocktrans %}Dear {{ recipient_name }},{% endblocktrans %}{% endblock %} + +{% block content %}{% endblock %} + + +Regards, +{{ site_name }} + +{{ protocol }}://{{ domain }} diff --git a/src/membership/templates/membership/emails/invitation.txt b/src/membership/templates/membership/emails/invitation.txt new file mode 100644 index 0000000..3e83848 --- /dev/null +++ b/src/membership/templates/membership/emails/invitation.txt @@ -0,0 +1,11 @@ +{% extends "users/mail/base.txt" %}{% load i18n %} + +{% block content %}{% url 'users:login_token' token=user.token_uuid as login_url %}{% blocktrans with expiry=user.token_expiry next=next %}Here is a 1-time code for confirming your account: + +{{ token_passphrase }} + +Use this code within 1 hour (before {{ expiry }}). You can login here: + +{{ protocol }}://{{ domain }}{{ login_url }}?next={{ next }} + +If you did not request this account, get in touch with us.{% endblocktrans %}{% endblock %} diff --git a/src/membership/views.py b/src/membership/views.py index eeb2e58..0c5ebf9 100644 --- a/src/membership/views.py +++ b/src/membership/views.py @@ -5,6 +5,7 @@ from __future__ import annotations from typing import TYPE_CHECKING from django.utils.translation import gettext_lazy as _ +from django_ratelimit.decorators import ratelimit from django_view_decorator import namespaced_decorator_factory from utils.view_utils import RenderConfig from utils.view_utils import RowAction @@ -113,3 +114,27 @@ def members_admin_detail(request: HttpRequest, member_id: int) -> HttpResponse: template_name="membership/members_admin_detail.html", context=context, ) + + +@ratelimit(group="membership", key="ip", rate="10/d", method="ALL", block=True) +@member_view( + paths="invite//", + name="membership-invite", + login_required=False, +) +def invite(request: HttpRequest, token: str) -> HttpResponse: + """View to invite a member to create a membership. + + The token belongs to a non-active Member object. If the token is valid, + the caller is allowed to create a membership. + + We ratelimit this view so it's not possible to brute-force tokens. + """ + context = { + "token": token, + } + return render( + request=request, + template_name="membership/invite.html", + context=context, + ) -- 2.43.4 From 15e422175b54fd77b9d0d57cb8ad0e3072697af8 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Thu, 8 Aug 2024 00:34:52 +0200 Subject: [PATCH 3/9] Fix error building dev project --- Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 9b7c01b..920c448 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,7 +21,8 @@ WORKDIR /app RUN groupadd -g 1000 www && useradd -u 1000 -ms /bin/bash -g www www # Only copy the requirements file first to leverage Docker cache -COPY $REQUIREMENTS_FILE . +RUN mkdir requirements/ +COPY $REQUIREMENTS_FILE $REQUIREMENTS_FILE RUN mkdir -p /app/src/static && \ chown www:www /app/src/static && \ -- 2.43.4 From 1d26bbc17a119970c2b86072fc68ff0f2581c9d8 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Thu, 8 Aug 2024 08:37:24 +0200 Subject: [PATCH 4/9] WIP: View, form and template for invitations, add exceptions for linting rules --- pyproject.toml | 9 ++++-- src/membership/emails.py | 26 +++++++++++++-- src/membership/forms.py | 19 +++++++++++ .../0009_membership_referral_code.py | 32 +++++++++++++++++++ src/membership/models.py | 4 +++ .../emails/{invitation.txt => invite.txt} | 0 .../templates/membership/invite.html | 14 ++++++++ src/membership/views.py | 29 +++++++++++++++-- src/project/settings.py | 4 +++ 9 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 src/membership/forms.py create mode 100644 src/membership/migrations/0009_membership_referral_code.py rename src/membership/templates/membership/emails/{invitation.txt => invite.txt} (100%) create mode 100644 src/membership/templates/membership/invite.html diff --git a/pyproject.toml b/pyproject.toml index 7ebb19b..a5431e3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,14 +141,19 @@ ignore = [ "EM102", # Exception must not use a f-string literal, assign to variable first "COM812", # missing-trailing-comma (https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules) "ISC001", # single-line-implicit-string-concatenation (https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules) + "D100", # Missing docstring in public module + "D101", # Missing docstring in public class + "D102", # Missing docstring in public method "D105", # Missing docstring in magic method "D106", # Missing docstring in public nested class + "D107", # Missing docstring in `__init__` "FIX", # TODO, FIXME, XXX "TD", # TODO, FIXME, XXX "ANN002", # Missing type annotation for `*args` "ANN003", # Missing type annotation for `**kwargs` - "FBT001", # Misbehaves: Complains about positional arguments that are keyworded. - "FBT002", # Misbehaves: Complains about positional arguments that are keyworded. + "FBT001", # Misbehaves: Boolean-typed positional argument in function definition + "FBT002", # Misbehaves: Boolean-typed positional argument in function definition + "TRY003", # Avoid specifying long messages outside the exception class ] [tool.ruff.lint.isort] diff --git a/src/membership/emails.py b/src/membership/emails.py index 20a243a..486108a 100644 --- a/src/membership/emails.py +++ b/src/membership/emails.py @@ -6,6 +6,7 @@ """ from django.contrib import messages +from django.contrib.auth.tokens import default_token_generator from django.contrib.sites.shortcuts import get_current_site from django.core.mail.message import EmailMessage from django.http import HttpRequest @@ -13,11 +14,15 @@ from django.template import loader from django.utils import translation from django.utils.translation import gettext_lazy as _ +from src.membership.models import Membership + class BaseEmail(EmailMessage): """Send emails via templated body and subjects. This base class is extended for all email functionality. + Because all emails are sent to the Member object, we can keep them gathered here, even when they are generated by + other apps (like the accounting app). """ template = "membership/email/base.txt" @@ -25,7 +30,7 @@ class BaseEmail(EmailMessage): template_subject = None default_subject = "SET SUBJECT HERE" - def __init__(self, request: HttpRequest, *args, **kwargs) -> None: # noqa: D107 + def __init__(self, request: HttpRequest, *args, **kwargs) -> None: self.context = kwargs.pop("context", {}) self.user = kwargs.pop("user", None) if self.user: @@ -76,7 +81,7 @@ class BaseEmail(EmailMessage): subject = str(self.default_subject) return subject - def send_with_feedback(self, success_msg: str | None = None, no_message: bool = False) -> None: + def send_with_feedback(self, *, success_msg: str | None = None, no_message: bool = False) -> None: """Send email, possibly adding feedback via django.contrib.messages.""" if not success_msg: success_msg = _("Email successfully sent to {}").format(", ".join(self.to)) @@ -86,3 +91,20 @@ class BaseEmail(EmailMessage): messages.success(self.request, success_msg) except RuntimeError: messages.error(self.request, _("Not sent, something wrong with the mail server.")) + + +class InviteEmail(BaseEmail): + template = "membership/emails/invite.txt" + default_subject = _("Invite to data.coop membership") + + def __init__(self, membership: Membership, request: HttpRequest, *args, **kwargs) -> None: + self.membership = membership + kwargs["user"] = membership.user + super().__init__(request, *args, **kwargs) + + def get_context_data(self) -> dict: + c = super().get_context_data() + c["membership"] = self.membership + c["token"] = default_token_generator.make_token(self.membership.user) + c["referral_code"] = self.membership.referral_code + return c diff --git a/src/membership/forms.py b/src/membership/forms.py new file mode 100644 index 0000000..23b8b2e --- /dev/null +++ b/src/membership/forms.py @@ -0,0 +1,19 @@ +from allauth.account.forms import SetPasswordForm + + +class CreatePasswordForm(SetPasswordForm): + """Create a new password for a user account that is created through an invite.""" + + def __init__(self, *args, **kwargs) -> None: + self.membership = kwargs.pop("membership") + super().__init__(*args, **kwargs) + + def save(self) -> None: + """Save instance to db. + + Note: You can hack a re-activation of a deactivated account + by getting a valid token before deactivation (from the reset password form). + We can block this by also setting Membership.revoked=False when deactivating someone's account. + """ + self.user.is_active = True + super().save() diff --git a/src/membership/migrations/0009_membership_referral_code.py b/src/membership/migrations/0009_membership_referral_code.py new file mode 100644 index 0000000..f8c0e07 --- /dev/null +++ b/src/membership/migrations/0009_membership_referral_code.py @@ -0,0 +1,32 @@ +# Generated by Django 5.1rc1 on 2024-08-07 22:32 + +import uuid +from django.db import migrations, models + + +def create_uuid(apps, schema_editor): + Membership = apps.get_model('membership', 'Membership') + for membership in Membership.objects.all(): + membership.referral_code = uuid.uuid4() + membership.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('membership', '0008_alter_membership_membership_type'), + ] + + operations = [ + migrations.AddField( + model_name='membership', + name='referral_code', + field=models.UUIDField(blank=True, null=True, unique=True, default=uuid.uuid4, editable=False), + ), + migrations.RunPython(create_uuid), + migrations.AddField( + model_name='membership', + name='referral_code', + field=models.UUIDField(unique=True, default=uuid.uuid4, editable=False), + ), + ] diff --git a/src/membership/models.py b/src/membership/models.py index 9483d0f..fca8c74 100644 --- a/src/membership/models.py +++ b/src/membership/models.py @@ -1,5 +1,6 @@ """Models for the membership app.""" +import uuid from typing import ClassVar from typing import Self @@ -122,6 +123,9 @@ class Membership(CreatedModifiedAbstract): user = models.ForeignKey("auth.User", on_delete=models.PROTECT, related_name="memberships") + # This code is used for inviting a user to create an account for this membership. + referral_code = models.UUIDField(unique=True, default=uuid.uuid4, editable=False) + membership_type = models.ForeignKey( "membership.MembershipType", related_name="memberships", diff --git a/src/membership/templates/membership/emails/invitation.txt b/src/membership/templates/membership/emails/invite.txt similarity index 100% rename from src/membership/templates/membership/emails/invitation.txt rename to src/membership/templates/membership/emails/invite.txt diff --git a/src/membership/templates/membership/invite.html b/src/membership/templates/membership/invite.html new file mode 100644 index 0000000..d9cbce0 --- /dev/null +++ b/src/membership/templates/membership/invite.html @@ -0,0 +1,14 @@ +{% extends "base.html" %} +{% load i18n %} + +{% block head_title %} + {% trans "Membership" %} +{% endblock %} + +{% block content %} + +
+

Membership invite

+ +
+{% endblock %} diff --git a/src/membership/views.py b/src/membership/views.py index 0c5ebf9..3331844 100644 --- a/src/membership/views.py +++ b/src/membership/views.py @@ -4,6 +4,11 @@ from __future__ import annotations from typing import TYPE_CHECKING +from django.contrib import messages +from django.contrib.auth.tokens import default_token_generator +from django.http import HttpResponseNotAllowed +from django.shortcuts import get_object_or_404 +from django.shortcuts import redirect from django.utils.translation import gettext_lazy as _ from django_ratelimit.decorators import ratelimit from django_view_decorator import namespaced_decorator_factory @@ -11,6 +16,8 @@ from utils.view_utils import RenderConfig from utils.view_utils import RowAction from utils.view_utils import render +from .forms import CreatePasswordForm +from .models import Membership from .permissions import ADMINISTRATE_MEMBERS from .selectors import get_member from .selectors import get_members @@ -118,11 +125,11 @@ def members_admin_detail(request: HttpRequest, member_id: int) -> HttpResponse: @ratelimit(group="membership", key="ip", rate="10/d", method="ALL", block=True) @member_view( - paths="invite//", + paths="invite///", name="membership-invite", login_required=False, ) -def invite(request: HttpRequest, token: str) -> HttpResponse: +def invite(request: HttpRequest, referral_code: str, token: str) -> HttpResponse: """View to invite a member to create a membership. The token belongs to a non-active Member object. If the token is valid, @@ -130,8 +137,26 @@ def invite(request: HttpRequest, token: str) -> HttpResponse: We ratelimit this view so it's not possible to brute-force tokens. """ + # Firstly, we get the membership by the referral code. + membership = get_object_or_404(Membership, referral_code=referral_code, user__is_active=False, revoked=False) + + token_valid = default_token_generator.check_token(membership.user, token) + + if not token_valid: + raise HttpResponseNotAllowed("Token not valid - maybe it expired?") + + if request.method == "POST": + form = CreatePasswordForm(membership=membership, user=membership, data=request.POST) + if form.is_valid(): + messages.info(request, _("Password is set for your account and you can now login.")) + return redirect("account_login") + else: + form = CreatePasswordForm(user=membership) + context = { "token": token, + "membership": membership, + "form": form, } return render( request=request, diff --git a/src/project/settings.py b/src/project/settings.py index 4f558a1..d6a800c 100644 --- a/src/project/settings.py +++ b/src/project/settings.py @@ -182,6 +182,10 @@ LOGGING = { STRIPE_API_KEY = env.str("STRIPE_API_KEY") STRIPE_ENDPOINT_SECRET = env.str("STRIPE_ENDPOINT_SECRET") +# The number of seconds a password reset link is valid for (default: 3 days). +# We've extended this to 7 days because invites then last for 1 week. +PASSWORD_RESET_TIMEOUT = 60 * 60 * 24 * 7 + if DEBUG: INSTALLED_APPS += ["debug_toolbar", "django_browser_reload"] MIDDLEWARE += [ -- 2.43.4 From e873df8e2eb43aecb5c287a4c4110936e2787906 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Mon, 12 Aug 2024 00:08:26 +0200 Subject: [PATCH 5/9] Send invitation emails and have an invite form to create a membership --- src/membership/admin.py | 50 ++++++++++++++++++- src/membership/emails.py | 2 +- src/membership/forms.py | 21 +++++++- .../0009_membership_referral_code.py | 2 +- .../0010_waitinglistentry_has_user.py | 18 +++++++ src/membership/models.py | 24 ++++++++- .../templates/membership/emails/invite.txt | 10 ++-- .../templates/membership/invite.html | 9 +++- src/membership/views.py | 16 +++--- src/project/templates/base.html | 10 ++++ 10 files changed, 142 insertions(+), 20 deletions(-) create mode 100644 src/membership/migrations/0010_waitinglistentry_has_user.py diff --git a/src/membership/admin.py b/src/membership/admin.py index 6ccb55e..240c108 100644 --- a/src/membership/admin.py +++ b/src/membership/admin.py @@ -14,7 +14,9 @@ from django.db import transaction from django.db.models import QuerySet from django.http import HttpRequest from django.http import HttpResponse +from django.utils.text import slugify +from .emails import InviteEmail from .models import Member from .models import Membership from .models import MembershipType @@ -63,7 +65,7 @@ def decorate_ensure_membership_type_exists(membership_type: MembershipType, labe @transaction.atomic def ensure_membership_type_exists( request: HttpRequest, - queryset: QuerySet, + queryset: QuerySet[Member], membership_type: MembershipType, ) -> HttpResponse: """Inner function that ensures that a membership exists for a given queryset of Member objects.""" @@ -95,7 +97,12 @@ class MemberAdmin(UserAdmin): """Member admin is actually an admin for User objects.""" inlines = (MembershipInlineAdmin,) - actions: list[Callable] = [] # noqa: RUF012 + actions: list[str | Callable] = ["send_invite"] # noqa: RUF012 + list_display = ("email", "current_membership", "username", "is_staff", "is_active", "date_joined") + + @admin.display(description="membership") + def current_membership(self, instance: Member) -> Membership | None: + return instance.memberships.current() def get_actions(self, request: HttpRequest) -> dict: """Populate actions with dynamic data (MembershipType).""" @@ -113,7 +120,46 @@ class MemberAdmin(UserAdmin): return super_dict + @admin.action(description="Send invite email to selected inactive accounts") + def send_invite(self, request: HttpRequest, queryset: QuerySet[Member]) -> None: + for member in queryset: + if member.is_active: + messages.error( + request, + f"This member will not receive an invite because the account is marked as active: {member.email}", + ) + continue + if not member.memberships.current(): + messages.error( + request, + f"This member will not receive an invite because it has no active membership: {member.email}", + ) + continue + membership = member.memberships.current() + email = InviteEmail(membership, request) + email.send() + messages.success(request, f"Sent an invitation to: {member.email}") + @admin.register(WaitingListEntry) class WaitingListEntryAdmin(admin.ModelAdmin): """Admin for WaitingList model.""" + + list_display = ("email", "has_user") + actions = ("create_user",) + + @admin.action(description="Create user account for entries") + def create_user(self, request: HttpRequest, queryset: QuerySet[WaitingListEntry]) -> None: + """Create a user account for this entry. + + Note that actions can soon be made available from the edit page, too: + https://github.com/django/django/pull/16012 + """ + for entry in queryset: + Member.objects.create_user(email=entry.email, username=slugify(entry.email), is_active=False) + entry.has_user = True + entry.save() + messages.info( + request, + f"Added user for f{entry.email} - ensure they have a membership and send an invite email.", + ) diff --git a/src/membership/emails.py b/src/membership/emails.py index 486108a..f1d96fc 100644 --- a/src/membership/emails.py +++ b/src/membership/emails.py @@ -14,7 +14,7 @@ from django.template import loader from django.utils import translation from django.utils.translation import gettext_lazy as _ -from src.membership.models import Membership +from .models import Membership class BaseEmail(EmailMessage): diff --git a/src/membership/forms.py b/src/membership/forms.py index 23b8b2e..b91d03b 100644 --- a/src/membership/forms.py +++ b/src/membership/forms.py @@ -1,13 +1,30 @@ +from allauth.account.adapter import get_adapter as get_allauth_adapter from allauth.account.forms import SetPasswordForm +from django import forms +from django.utils.translation import gettext_lazy as _ -class CreatePasswordForm(SetPasswordForm): +class InviteForm(SetPasswordForm): """Create a new password for a user account that is created through an invite.""" + username = forms.CharField( + label=_("Username"), + widget=forms.TextInput(attrs={"placeholder": _("Username"), "autocomplete": "username"}), + ) + def __init__(self, *args, **kwargs) -> None: self.membership = kwargs.pop("membership") + kwargs["user"] = self.membership.user super().__init__(*args, **kwargs) + def clean_username(self) -> str: + """Clean the username value. + + Taken from the allauth Signup form - we should consider that data can be leaked here. + """ + value = self.cleaned_data["username"] + return get_allauth_adapter().clean_username(value) + def save(self) -> None: """Save instance to db. @@ -15,5 +32,7 @@ class CreatePasswordForm(SetPasswordForm): by getting a valid token before deactivation (from the reset password form). We can block this by also setting Membership.revoked=False when deactivating someone's account. """ + self.user.username = self.cleaned_data["username"] self.user.is_active = True + self.user.save() super().save() diff --git a/src/membership/migrations/0009_membership_referral_code.py b/src/membership/migrations/0009_membership_referral_code.py index f8c0e07..4fcf65a 100644 --- a/src/membership/migrations/0009_membership_referral_code.py +++ b/src/membership/migrations/0009_membership_referral_code.py @@ -24,7 +24,7 @@ class Migration(migrations.Migration): field=models.UUIDField(blank=True, null=True, unique=True, default=uuid.uuid4, editable=False), ), migrations.RunPython(create_uuid), - migrations.AddField( + migrations.AlterField( model_name='membership', name='referral_code', field=models.UUIDField(unique=True, default=uuid.uuid4, editable=False), diff --git a/src/membership/migrations/0010_waitinglistentry_has_user.py b/src/membership/migrations/0010_waitinglistentry_has_user.py new file mode 100644 index 0000000..331dc28 --- /dev/null +++ b/src/membership/migrations/0010_waitinglistentry_has_user.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1rc1 on 2024-08-11 21:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('membership', '0009_membership_referral_code'), + ] + + operations = [ + migrations.AddField( + model_name='waitinglistentry', + name='has_user', + field=models.BooleanField(default=False, help_text='Once a user account is generated (use the admin action), this field will be marked.', verbose_name='has user'), + ), + ] diff --git a/src/membership/models.py b/src/membership/models.py index fca8c74..b581b51 100644 --- a/src/membership/models.py +++ b/src/membership/models.py @@ -5,6 +5,7 @@ from typing import ClassVar from typing import Self from django.contrib.auth.models import User +from django.contrib.auth.models import UserManager from django.contrib.postgres.constraints import ExclusionConstraint from django.contrib.postgres.fields import DateRangeField from django.contrib.postgres.fields import RangeOperators @@ -43,7 +44,23 @@ class Member(User): ), ) - objects = QuerySet.as_manager() + objects = UserManager.from_queryset(QuerySet)() + + def get_display_name(self) -> str: + """Choose how to display the user in emails and UI and ultimately to other users. + + It's crucial that we currently don't have a good solution for this. + We should allow the user to define their own nick. + """ + return self.username + + @property + def language_code(self) -> str: + """Returns the user's preferred language code. + + We don't have an actual setting for this... because this is a proxy table. + """ + return "da-dk" class Meta: proxy = True @@ -213,6 +230,11 @@ class WaitingListEntry(CreatedModifiedAbstract): email = models.EmailField() geography = models.CharField(verbose_name=_("geography"), blank=True, default="") comment = models.TextField(blank=True) + has_user = models.BooleanField( + default=False, + verbose_name=_("has user"), + help_text=_("Once a user account is generated (use the admin action), this field will be marked."), + ) def __str__(self) -> str: return self.email diff --git a/src/membership/templates/membership/emails/invite.txt b/src/membership/templates/membership/emails/invite.txt index 3e83848..216ba0e 100644 --- a/src/membership/templates/membership/emails/invite.txt +++ b/src/membership/templates/membership/emails/invite.txt @@ -1,11 +1,7 @@ -{% extends "users/mail/base.txt" %}{% load i18n %} +{% extends "membership/emails/base.txt" %}{% load i18n %} -{% block content %}{% url 'users:login_token' token=user.token_uuid as login_url %}{% blocktrans with expiry=user.token_expiry next=next %}Here is a 1-time code for confirming your account: +{% block content %}{% url 'member:membership-invite' token=token referral_code=referral_code as invite_url %}{% blocktrans %}Here is your secret URL for creating an account with us: -{{ token_passphrase }} - -Use this code within 1 hour (before {{ expiry }}). You can login here: - -{{ protocol }}://{{ domain }}{{ login_url }}?next={{ next }} +{{ protocol }}://{{ domain }}{{ invite_url }} If you did not request this account, get in touch with us.{% endblocktrans %}{% endblock %} diff --git a/src/membership/templates/membership/invite.html b/src/membership/templates/membership/invite.html index d9cbce0..a36ca35 100644 --- a/src/membership/templates/membership/invite.html +++ b/src/membership/templates/membership/invite.html @@ -8,7 +8,14 @@ {% block content %}
-

Membership invite

+

{% trans "Create account" %}

+

{% trans "Congratulations! You've been invited to create an account with us:" %}

+

Email: {{ membership.user.email }}

+
+ {% csrf_token %} + {{ form.as_p }} + +
{% endblock %} diff --git a/src/membership/views.py b/src/membership/views.py index 3331844..c82cf5b 100644 --- a/src/membership/views.py +++ b/src/membership/views.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING from django.contrib import messages from django.contrib.auth.tokens import default_token_generator -from django.http import HttpResponseNotAllowed +from django.http import HttpResponseForbidden from django.shortcuts import get_object_or_404 from django.shortcuts import redirect from django.utils.translation import gettext_lazy as _ @@ -16,7 +16,7 @@ from utils.view_utils import RenderConfig from utils.view_utils import RowAction from utils.view_utils import render -from .forms import CreatePasswordForm +from .forms import InviteForm from .models import Membership from .permissions import ADMINISTRATE_MEMBERS from .selectors import get_member @@ -125,7 +125,7 @@ def members_admin_detail(request: HttpRequest, member_id: int) -> HttpResponse: @ratelimit(group="membership", key="ip", rate="10/d", method="ALL", block=True) @member_view( - paths="invite///", + paths="invite///", name="membership-invite", login_required=False, ) @@ -137,21 +137,25 @@ def invite(request: HttpRequest, referral_code: str, token: str) -> HttpResponse We ratelimit this view so it's not possible to brute-force tokens. """ + if request.user.is_authenticated: + return HttpResponseForbidden("You're already logged in. So you cannot receive an invite.") + # Firstly, we get the membership by the referral code. membership = get_object_or_404(Membership, referral_code=referral_code, user__is_active=False, revoked=False) token_valid = default_token_generator.check_token(membership.user, token) if not token_valid: - raise HttpResponseNotAllowed("Token not valid - maybe it expired?") + raise HttpResponseForbidden("Token not valid - maybe it expired?") if request.method == "POST": - form = CreatePasswordForm(membership=membership, user=membership, data=request.POST) + form = InviteForm(membership=membership, data=request.POST) if form.is_valid(): + form.save() messages.info(request, _("Password is set for your account and you can now login.")) return redirect("account_login") else: - form = CreatePasswordForm(user=membership) + form = InviteForm(membership=membership) context = { "token": token, diff --git a/src/project/templates/base.html b/src/project/templates/base.html index 485fbdf..10935b0 100644 --- a/src/project/templates/base.html +++ b/src/project/templates/base.html @@ -102,6 +102,16 @@
+ {% if messages %} +
+ {% for message in messages %} +

📨

+

{{ message }}

+ {% endfor %} +
+ + {% endif %} + {% block content %}{% endblock %}
-- 2.43.4 From 9962f2622bf5f47468144fa7cbcb228566b00acf Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Mon, 12 Aug 2024 22:22:11 +0200 Subject: [PATCH 6/9] Send order emails via admin action --- src/accounting/admin.py | 27 ++++++++++++++++--- src/membership/emails.py | 16 +++++++++++ .../templates/membership/emails/base.txt | 2 +- .../templates/membership/emails/order.txt | 6 +++++ src/project/templates/index.html | 4 +++ src/project/views.py | 7 ++++- 6 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 src/membership/templates/membership/emails/order.txt diff --git a/src/accounting/admin.py b/src/accounting/admin.py index 11d2219..1b09077 100644 --- a/src/accounting/admin.py +++ b/src/accounting/admin.py @@ -2,7 +2,11 @@ from django import forms from django.contrib import admin +from django.contrib import messages +from django.db.models import QuerySet +from django.http import HttpRequest from django.utils.translation import gettext_lazy as _ +from membership.emails import OrderEmail from . import models @@ -26,7 +30,7 @@ class OrderAdminForm(forms.ModelForm): model = models.Order exclude = () # noqa: DJ006 - def clean(self): # noqa: D102, ANN201 + def clean(self): # noqa: ANN201 cd = super().clean() if not cd["account"] and cd["member"]: try: @@ -43,10 +47,25 @@ class OrderAdmin(admin.ModelAdmin): inlines = (OrderProductInline,) form = OrderAdminForm + actions = ("send_order",) + list_display = ("member", "description", "created", "is_paid", "total_with_vat") search_fields = ("member__email", "membership__membership_type__name", "description") list_filter = ("is_paid", "membership__membership_type") + @admin.action(description="Send order link to selected unpaid orders") + def send_order(self, request: HttpRequest, queryset: QuerySet[models.Order]) -> None: + for order in queryset: + if order.is_paid: + messages.error( + request, + f"Order pk={order.id} is already marked paid, not sending email to: {order.member.email}", + ) + continue + email = OrderEmail(order, request) + email.send() + messages.success(request, f"Sent an order for order pk={order.id} link to: {order.member.email}") + @admin.register(models.Payment) class PaymentAdmin(admin.ModelAdmin): @@ -61,15 +80,15 @@ class PaymentAdmin(admin.ModelAdmin): @admin.register(models.Product) -class ProductAdmin(admin.ModelAdmin): # noqa: D101 +class ProductAdmin(admin.ModelAdmin): list_display = ("name", "price", "vat") -class TransactionInline(admin.TabularInline): # noqa: D101 +class TransactionInline(admin.TabularInline): model = models.Transaction @admin.register(models.Account) -class AccountAdmin(admin.ModelAdmin): # noqa: D101 +class AccountAdmin(admin.ModelAdmin): list_display = ("owner", "balance") inlines = (TransactionInline,) diff --git a/src/membership/emails.py b/src/membership/emails.py index f1d96fc..6a57718 100644 --- a/src/membership/emails.py +++ b/src/membership/emails.py @@ -5,6 +5,7 @@ * Generally, an email consists of templates (for body and subject) and a get_context() method. """ +from accounting.models import Order from django.contrib import messages from django.contrib.auth.tokens import default_token_generator from django.contrib.sites.shortcuts import get_current_site @@ -108,3 +109,18 @@ class InviteEmail(BaseEmail): c["token"] = default_token_generator.make_token(self.membership.user) c["referral_code"] = self.membership.referral_code return c + + +class OrderEmail(BaseEmail): + template = "membership/emails/order.txt" + default_subject = _("Your data.coop order and payment") + + def __init__(self, order: Order, request: HttpRequest, *args, **kwargs) -> None: + self.order = order + kwargs["user"] = order.member + super().__init__(request, *args, **kwargs) + + def get_context_data(self) -> dict: + c = super().get_context_data() + c["order"] = self.order + return c diff --git a/src/membership/templates/membership/emails/base.txt b/src/membership/templates/membership/emails/base.txt index 9a86384..636aee0 100644 --- a/src/membership/templates/membership/emails/base.txt +++ b/src/membership/templates/membership/emails/base.txt @@ -3,7 +3,7 @@ {% block content %}{% endblock %} -Regards, +{% trans "Cooperatively yours," %} {{ site_name }} {{ protocol }}://{{ domain }} diff --git a/src/membership/templates/membership/emails/order.txt b/src/membership/templates/membership/emails/order.txt new file mode 100644 index 0000000..e976444 --- /dev/null +++ b/src/membership/templates/membership/emails/order.txt @@ -0,0 +1,6 @@ +{% extends "membership/emails/base.txt" %}{% load i18n %} + +{% block content %}{% url 'order:detail' order_id=order.id as order_url %}{% blocktrans %}You have an order in our system, you can pay it here: + +{{ protocol }}://{{ domain }}{{ order_url }} +{% endblocktrans %}{% endblock %} diff --git a/src/project/templates/index.html b/src/project/templates/index.html index 2cc3490..1cc49e6 100644 --- a/src/project/templates/index.html +++ b/src/project/templates/index.html @@ -14,6 +14,10 @@ It is very much under construction.

+ {% for order in unpaid_orders %} +

You have an unpaid order: View Order ID {{ order.id }}

+ {% endfor %} + {% comment %}

diff --git a/src/project/views.py b/src/project/views.py index 7fbde77..affd1e7 100644 --- a/src/project/views.py +++ b/src/project/views.py @@ -4,6 +4,7 @@ from __future__ import annotations from typing import TYPE_CHECKING +from accounting.models import Order from django_view_decorator import view from utils.view_utils import render @@ -19,7 +20,11 @@ if TYPE_CHECKING: ) def index(request: HttpRequest) -> HttpResponse: """View to show the index page.""" - return render(request, "index.html") + unpaid_orders = Order.objects.filter(member=request.user, is_paid=False) + + context = {"unpaid_orders": list(unpaid_orders)} + + return render(request, "index.html", context=context) @view( -- 2.43.4 From b1ff3f985c78aebbe264551e138a55f767ec9b0e Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 13 Aug 2024 09:50:07 +0200 Subject: [PATCH 7/9] Adds a README note --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 7a0256d..671414d 100644 --- a/README.md +++ b/README.md @@ -98,3 +98,8 @@ make requirements # Build Docker image with new Python requirements make build ``` + +## Important notes + +* This project uses [django-zen-queries](https://github.com/dabapps/django-zen-queries), which will sometimes raise a `QueriesDisabledError` in your templates. You can find a difference of opinion about that, but you can find a difference of opinion about many things, right? +* If a linting error annoys you, please feel free to strike back by adding a `noqa` to the line that has displeased the linter and move on with life. -- 2.43.4 From 1c740418f4bcbb00cb88a9365d276a0229d8d929 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Wed, 14 Aug 2024 09:55:09 +0200 Subject: [PATCH 8/9] Clarify error messages from admin action --- src/membership/admin.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/membership/admin.py b/src/membership/admin.py index 240c108..1c02f0a 100644 --- a/src/membership/admin.py +++ b/src/membership/admin.py @@ -126,13 +126,16 @@ class MemberAdmin(UserAdmin): if member.is_active: messages.error( request, - f"This member will not receive an invite because the account is marked as active: {member.email}", + f"Computer says no! This member will not receive an invite because the account is marked " + f"as active: {member.email}. That means the member has probably created a password and a username " + f"already, please tell them to use the password reminder function.", ) continue if not member.memberships.current(): messages.error( request, - f"This member will not receive an invite because it has no active membership: {member.email}", + f"Computer says no! This member will not receive an invite because it has no current " + f"membership: {member.email}. You need to create a current membership before sending the invite.", ) continue membership = member.memberships.current() -- 2.43.4 From e5193837416fee013e76f324cd943d6a019d5206 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Wed, 14 Aug 2024 10:20:00 +0200 Subject: [PATCH 9/9] Use FK relation for created user, rather than boolean --- src/membership/admin.py | 14 +++++++------- src/membership/forms.py | 1 + .../0010_waitinglistentry_has_user.py | 18 ------------------ .../0010_waitinglistentry_member.py | 19 +++++++++++++++++++ src/membership/models.py | 11 +++++++---- 5 files changed, 34 insertions(+), 29 deletions(-) delete mode 100644 src/membership/migrations/0010_waitinglistentry_has_user.py create mode 100644 src/membership/migrations/0010_waitinglistentry_member.py diff --git a/src/membership/admin.py b/src/membership/admin.py index 1c02f0a..69e2c22 100644 --- a/src/membership/admin.py +++ b/src/membership/admin.py @@ -148,21 +148,21 @@ class MemberAdmin(UserAdmin): class WaitingListEntryAdmin(admin.ModelAdmin): """Admin for WaitingList model.""" - list_display = ("email", "has_user") - actions = ("create_user",) + list_display = ("email", "member") + actions = ("create_member",) - @admin.action(description="Create user account for entries") - def create_user(self, request: HttpRequest, queryset: QuerySet[WaitingListEntry]) -> None: + @admin.action(description="Create member account for entries") + def create_member(self, request: HttpRequest, queryset: QuerySet[WaitingListEntry]) -> None: """Create a user account for this entry. Note that actions can soon be made available from the edit page, too: https://github.com/django/django/pull/16012 """ for entry in queryset: - Member.objects.create_user(email=entry.email, username=slugify(entry.email), is_active=False) - entry.has_user = True + member = Member.objects.create_user(email=entry.email, username=slugify(entry.email), is_active=False) + entry.member = member entry.save() messages.info( request, - f"Added user for f{entry.email} - ensure they have a membership and send an invite email.", + f"Added user for {entry.email} - ensure they have a membership and send an invite email.", ) diff --git a/src/membership/forms.py b/src/membership/forms.py index b91d03b..71afaaa 100644 --- a/src/membership/forms.py +++ b/src/membership/forms.py @@ -23,6 +23,7 @@ class InviteForm(SetPasswordForm): Taken from the allauth Signup form - we should consider that data can be leaked here. """ value = self.cleaned_data["username"] + # The allauth adapter ensures the username is unique. return get_allauth_adapter().clean_username(value) def save(self) -> None: diff --git a/src/membership/migrations/0010_waitinglistentry_has_user.py b/src/membership/migrations/0010_waitinglistentry_has_user.py deleted file mode 100644 index 331dc28..0000000 --- a/src/membership/migrations/0010_waitinglistentry_has_user.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 5.1rc1 on 2024-08-11 21:27 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('membership', '0009_membership_referral_code'), - ] - - operations = [ - migrations.AddField( - model_name='waitinglistentry', - name='has_user', - field=models.BooleanField(default=False, help_text='Once a user account is generated (use the admin action), this field will be marked.', verbose_name='has user'), - ), - ] diff --git a/src/membership/migrations/0010_waitinglistentry_member.py b/src/membership/migrations/0010_waitinglistentry_member.py new file mode 100644 index 0000000..3bc2e08 --- /dev/null +++ b/src/membership/migrations/0010_waitinglistentry_member.py @@ -0,0 +1,19 @@ +# Generated by Django 5.1rc1 on 2024-08-14 08:05 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('membership', '0009_membership_referral_code'), + ] + + operations = [ + migrations.AddField( + model_name='waitinglistentry', + name='member', + field=models.ForeignKey(help_text='Once a member account is generated (use the admin action), this field will be marked.', null=True, blank=True, on_delete=django.db.models.deletion.CASCADE, to='membership.member', verbose_name='has member'), + ), + ] diff --git a/src/membership/models.py b/src/membership/models.py index b581b51..f9e5ddd 100644 --- a/src/membership/models.py +++ b/src/membership/models.py @@ -230,10 +230,13 @@ class WaitingListEntry(CreatedModifiedAbstract): email = models.EmailField() geography = models.CharField(verbose_name=_("geography"), blank=True, default="") comment = models.TextField(blank=True) - has_user = models.BooleanField( - default=False, - verbose_name=_("has user"), - help_text=_("Once a user account is generated (use the admin action), this field will be marked."), + member = models.ForeignKey( + Member, + null=True, + blank=True, + verbose_name=_("has member"), + help_text=_("Once a member account is generated (use the admin action), this field will be marked."), + on_delete=models.CASCADE, ) def __str__(self) -> str: -- 2.43.4