From 9c376395dbc06202159001832b3329e47410029e Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Thu, 22 Dec 2022 07:03:21 +0000 Subject: [PATCH] Invites overhaul No email tie, added uses and expires, now works by URL. --- api/views/instance.py | 4 +- core/models/config.py | 1 - takahe/urls.py | 1 + templates/admin/invite_create.html | 3 +- templates/admin/invite_view.html | 5 +- templates/admin/invites.html | 16 ++++- templates/auth/signup.html | 40 +++++------ tests/users/views/test_auth.py | 69 ++++++++++--------- ...invite_email_invite_expires_invite_uses.py | 27 ++++++++ users/models/invite.py | 24 +++++-- users/views/activitypub.py | 3 +- users/views/admin/invites.py | 35 +++++++--- users/views/admin/settings.py | 11 ++- users/views/auth.py | 43 ++++++------ 14 files changed, 178 insertions(+), 104 deletions(-) create mode 100644 users/migrations/0007_remove_invite_email_invite_expires_invite_uses.py diff --git a/api/views/instance.py b/api/views/instance.py index 45de4a6..7f932f1 100644 --- a/api/views/instance.py +++ b/api/views/instance.py @@ -25,9 +25,7 @@ def instance_info(request): }, "thumbnail": Config.system.site_banner, "languages": ["en"], - "registrations": ( - Config.system.signup_allowed and not Config.system.signup_invite_only - ), + "registrations": (Config.system.signup_allowed), "approval_required": False, "invites_enabled": False, "configuration": { diff --git a/core/models/config.py b/core/models/config.py index a651396..2341598 100644 --- a/core/models/config.py +++ b/core/models/config.py @@ -211,7 +211,6 @@ class Config(models.Model): policy_rules: str = "" signup_allowed: bool = True - signup_invite_only: bool = False signup_text: str = "" content_warning_text: str = "Content Warning" diff --git a/takahe/urls.py b/takahe/urls.py index 124a9ee..3ba71a0 100644 --- a/takahe/urls.py +++ b/takahe/urls.py @@ -182,6 +182,7 @@ urlpatterns = [ path("auth/login/", auth.Login.as_view(), name="login"), path("auth/logout/", auth.Logout.as_view(), name="logout"), path("auth/signup/", auth.Signup.as_view(), name="signup"), + path("auth/signup//", auth.Signup.as_view(), name="signup"), path("auth/reset/", auth.TriggerReset.as_view(), name="trigger_reset"), path("auth/reset//", auth.PerformReset.as_view(), name="password_reset"), # Identity selection diff --git a/templates/admin/invite_create.html b/templates/admin/invite_create.html index f7d7e26..4708d52 100644 --- a/templates/admin/invite_create.html +++ b/templates/admin/invite_create.html @@ -7,7 +7,8 @@ {% csrf_token %}
Details - {% include "forms/_field.html" with field=form.email %} + {% include "forms/_field.html" with field=form.uses %} + {% include "forms/_field.html" with field=form.expires_days %} {% include "forms/_field.html" with field=form.notes %}
diff --git a/templates/admin/invite_view.html b/templates/admin/invite_view.html index c666af0..595d612 100644 --- a/templates/admin/invite_view.html +++ b/templates/admin/invite_view.html @@ -7,8 +7,9 @@ {% csrf_token %}
Details - {% include "forms/_field.html" with field=form.code %} - {% include "forms/_field.html" with field=form.email %} + {% include "forms/_field.html" with field=form.link %} + {% include "forms/_field.html" with field=form.uses %} + {% include "forms/_field.html" with field=form.expires_days %} {% include "forms/_field.html" with field=form.notes %}
diff --git a/templates/admin/invites.html b/templates/admin/invites.html index 11433e1..e823c20 100644 --- a/templates/admin/invites.html +++ b/templates/admin/invites.html @@ -17,12 +17,22 @@ {{ invite.token }} - {% if invite.email %} - (email {{ invite.email }}) + {% if invite.expires %} + Expires in {{ invite.expires|timeuntil }} + {% if invite.notes %}|{% endif %} + {% endif %} + {% if invite.notes %} + {{ invite.notes }} {% endif %} - + {% empty %}

diff --git a/templates/auth/signup.html b/templates/auth/signup.html index 445f610..3007416 100644 --- a/templates/auth/signup.html +++ b/templates/auth/signup.html @@ -4,27 +4,27 @@ {% block content %}

- {% csrf_token %} -
- Create An Account - {% if signup_text %}{{ signup_text }}{% endif %} - {% if config.signup_allowed %} - {% for field in form %} - {% include "forms/_field.html" %} - {% endfor %} - {% else %} - {% if not config.signup_text %} -

Not accepting new users at this time

+ {% if form %} + {% csrf_token %} +
+ Create An Account + {% if signup_text %}{{ signup_text }}{% endif %} + {% for field in form %} + {% include "forms/_field.html" %} + {% endfor %} +
+
+ +
+ {% else %} +
+ Create An Account + {% if signup_text %} + {{ signup_text }} + {% else %} +

We're not accepting new users at this time.

{% endif %} - {% endif %} - -
- - {% if config.signup_allowed %} -
- -
+
{% endif %} -
{% endblock %} diff --git a/tests/users/views/test_auth.py b/tests/users/views/test_auth.py index 6dd1010..9653a1b 100644 --- a/tests/users/views/test_auth.py +++ b/tests/users/views/test_auth.py @@ -1,5 +1,8 @@ +import datetime + import pytest from django.core import mail +from django.utils import timezone from pytest_django.asserts import assertContains, assertNotContains from users.models import Invite, User @@ -13,7 +16,9 @@ def test_signup_disabled(client, config_system): # Signup disabled and no signup text config_system.signup_allowed = False response = client.get("/auth/signup/") - assertContains(response, "Not accepting new users at this time", status_code=200) + assertContains( + response, "We're not accepting new users at this time", status_code=200 + ) assertNotContains(response, "") # Signup disabled with signup text configured @@ -32,7 +37,7 @@ def test_signup_disabled(client, config_system): config_system.signup_allowed = True response = client.get("/auth/signup/") assertContains(response, "", status_code=200) - assertNotContains(response, "Not accepting new users at this time") + assertNotContains(response, "We're not accepting new users at this time") @pytest.mark.django_db @@ -40,43 +45,45 @@ def test_signup_invite_only(client, config_system): """ Tests that invite codes work with signup """ - config_system.signup_allowed = True - config_system.signup_invite_only = True + config_system.signup_allowed = False # Try to sign up without an invite code response = client.post("/auth/signup/", {"email": "random@example.com"}) assertNotContains(response, "Email Sent", status_code=200) - # Make an invite code for any email - invite_any = Invite.create_random() + # Make an invite code for any email with infinite uses + invite_infinite = Invite.create_random() response = client.post( - "/auth/signup/", - {"email": "random@example.com", "invite_code": invite_any.token}, - ) - assertNotContains(response, "not a valid invite") - assertContains(response, "Email Sent", status_code=200) - - # Make sure you can't reuse an invite code - response = client.post( - "/auth/signup/", - {"email": "random2@example.com", "invite_code": invite_any.token}, - ) - assertNotContains(response, "Email Sent", status_code=200) - - # Make an invite code for a specific email - invite_specific = Invite.create_random(email="special@example.com") - response = client.post( - "/auth/signup/", - {"email": "random3@example.com", "invite_code": invite_specific.token}, - ) - assertContains(response, "valid invite code for this email", status_code=200) - assertNotContains(response, "Email Sent") - response = client.post( - "/auth/signup/", - {"email": "special@example.com", "invite_code": invite_specific.token}, + f"/auth/signup/{invite_infinite.token}/", + {"email": "random@example.com"}, ) assertContains(response, "Email Sent", status_code=200) + # Ensure it still has infinite uses + assert Invite.objects.get(token=invite_infinite.token).uses is None + + # Make an invite code for any email with one use + invite_single = Invite.create_random(uses=1) + response = client.post( + f"/auth/signup/{invite_single.token}/", + {"email": "random2@example.com"}, + ) + assertContains(response, "Email Sent", status_code=200) + + # Verify it was used up + assert Invite.objects.filter(token=invite_single.token).count() == 0 + + # Make an invite code that's invalid + invite_expired = Invite.create_random( + expires=timezone.now() - datetime.timedelta(days=1) + ) + response = client.post( + f"/auth/signup/{invite_expired.token}/", + {"email": "random3@example.com"}, + ) + print(response.content) + assert response.status_code == 404 + @pytest.mark.django_db def test_signup_policy(client, config_system): @@ -84,7 +91,6 @@ def test_signup_policy(client, config_system): Tests that you must agree to policies to sign up """ config_system.signup_allowed = True - config_system.signup_invite_only = False # Make sure we can sign up when there are no policies response = client.post("/auth/signup/", {"email": "random@example.com"}) @@ -103,7 +109,6 @@ def test_signup_email(client, config_system, stator): Tests that you can sign up and get an email sent to you """ config_system.signup_allowed = True - config_system.signup_invite_only = False # Sign up with a user response = client.post("/auth/signup/", {"email": "random@example.com"}) diff --git a/users/migrations/0007_remove_invite_email_invite_expires_invite_uses.py b/users/migrations/0007_remove_invite_email_invite_expires_invite_uses.py new file mode 100644 index 0000000..7d98079 --- /dev/null +++ b/users/migrations/0007_remove_invite_email_invite_expires_invite_uses.py @@ -0,0 +1,27 @@ +# Generated by Django 4.1.4 on 2022-12-22 06:18 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0006_identity_actor_type"), + ] + + operations = [ + migrations.RemoveField( + model_name="invite", + name="email", + ), + migrations.AddField( + model_name="invite", + name="expires", + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AddField( + model_name="invite", + name="uses", + field=models.IntegerField(blank=True, null=True), + ), + ] diff --git a/users/models/invite.py b/users/models/invite.py index e5c11c3..d1d8adb 100644 --- a/users/models/invite.py +++ b/users/models/invite.py @@ -2,6 +2,7 @@ import random import urlman from django.db import models +from django.utils import timezone class Invite(models.Model): @@ -12,12 +13,15 @@ class Invite(models.Model): # Should always be lowercase token = models.CharField(max_length=500, unique=True) - # Is it limited to a specific email? - email = models.EmailField(null=True, blank=True) - # Admin note about this code note = models.TextField(null=True, blank=True) + # Uses remaining (null means "infinite") + uses = models.IntegerField(null=True, blank=True) + + # Expiry date + expires = models.DateTimeField(null=True, blank=True) + created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) @@ -27,11 +31,21 @@ class Invite(models.Model): admin_view = "{admin}{self.pk}/" @classmethod - def create_random(cls, email=None, note=None): + def create_random(cls, uses=None, expires=None, note=None): return cls.objects.create( token="".join( random.choice("abcdefghkmnpqrstuvwxyz23456789") for i in range(20) ), - email=email, + uses=uses, + expires=expires, note=note, ) + + @property + def valid(self): + if self.uses is not None: + if self.uses <= 0: + return False + if self.expires is not None: + return self.expires >= timezone.now() + return True diff --git a/users/views/activitypub.py b/users/views/activitypub.py index 7a9170c..c28fed3 100644 --- a/users/views/activitypub.py +++ b/users/views/activitypub.py @@ -82,8 +82,7 @@ class NodeInfo2(View): "users": {"total": local_identities}, "localPosts": local_posts, }, - "openRegistrations": Config.system.signup_allowed - and not Config.system.signup_invite_only, + "openRegistrations": Config.system.signup_allowed, "metadata": {}, } ) diff --git a/users/views/admin/invites.py b/users/views/admin/invites.py index ef76adb..db9e8bb 100644 --- a/users/views/admin/invites.py +++ b/users/views/admin/invites.py @@ -1,5 +1,9 @@ +import datetime + from django import forms +from django.conf import settings from django.shortcuts import get_object_or_404, redirect +from django.utils import timezone from django.utils.decorators import method_decorator from django.views.generic import FormView, ListView @@ -32,19 +36,28 @@ class InviteCreate(FormView): } class form_class(forms.Form): - email = forms.EmailField( + uses = forms.IntegerField( required=False, - help_text="Optional email to tie the invite to.\nYou will still need to email the user this code yourself!", + help_text="Number of times this can be used. Leave blank for infinite uses.", + ) + expires_days = forms.IntegerField( + required=False, + help_text="Number of days until this expires. Leave blank to make it last forever.", ) notes = forms.CharField( required=False, - widget=forms.Textarea, help_text="Notes for other admins", ) def form_valid(self, form): + expires_days = form.cleaned_data.get("expires_days") invite = Invite.create_random( - email=form.cleaned_data.get("email") or None, + uses=form.cleaned_data.get("uses") or None, + expires=( + timezone.now() + datetime.timedelta(days=expires_days) + if expires_days is not None + else None + ), note=form.cleaned_data.get("notes"), ) return redirect(invite.urls.admin_view) @@ -59,7 +72,7 @@ class InviteView(FormView): } class form_class(InviteCreate.form_class): - code = forms.CharField(disabled=True, required=False) + link = forms.CharField(disabled=True, required=False) def dispatch(self, request, id, *args, **kwargs): self.invite = get_object_or_404(Invite, id=id) @@ -74,13 +87,19 @@ class InviteView(FormView): def get_initial(self): return { "notes": self.invite.note, - "email": self.invite.email, - "code": self.invite.token, + "uses": self.invite.uses, + "link": f"https://{settings.MAIN_DOMAIN}/auth/signup/{self.invite.token}/", } def form_valid(self, form): + expires_days = form.cleaned_data.get("expires_days") self.invite.note = form.cleaned_data.get("notes") or "" - self.invite.email = form.cleaned_data.get("email") or None + self.invite.uses = form.cleaned_data.get("uses") or None + self.invite.expires = ( + timezone.now() + datetime.timedelta(days=expires_days) + if expires_days is not None + else None + ) self.invite.save() return redirect(self.invite.urls.admin) diff --git a/users/views/admin/settings.py b/users/views/admin/settings.py index d87fb7e..beb331e 100644 --- a/users/views/admin/settings.py +++ b/users/views/admin/settings.py @@ -69,11 +69,7 @@ class BasicSettings(AdminSettingsPage): }, "signup_allowed": { "title": "Signups Allowed", - "help_text": "If signups are allowed at all", - }, - "signup_invite_only": { - "title": "Invite-Only", - "help_text": "If signups require an invite code", + "help_text": "If uninvited signups are allowed.\nInvite codes will always work.", }, "signup_text": { "title": "Signup Page Text", @@ -103,7 +99,10 @@ class BasicSettings(AdminSettingsPage): "site_banner", "highlight_color", ], - "Signups": ["signup_allowed", "signup_invite_only", "signup_text"], + "Signups": [ + "signup_allowed", + "signup_text", + ], "Posts": [ "post_length", "post_minimum_interval", diff --git a/users/views/auth.py b/users/views/auth.py index 546aa49..137ed4c 100644 --- a/users/views/auth.py +++ b/users/views/auth.py @@ -4,6 +4,7 @@ from django.conf import settings from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.password_validation import validate_password from django.contrib.auth.views import LoginView, LogoutView +from django.http import Http404 from django.shortcuts import get_object_or_404, render from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ @@ -39,11 +40,6 @@ class Signup(FormView): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # Add the invite field if it's enabled - if Config.system.signup_invite_only: - self.fields["invite_code"] = forms.CharField( - help_text="Your invite code from one of our admins" - ) # Add the policies if they're defined policies = [] if Config.system.policy_rules: @@ -82,30 +78,33 @@ class Signup(FormView): raise forms.ValidationError("This email already has an account") return email - def clean_invite_code(self): - invite_code = self.cleaned_data["invite_code"].lower().strip() - invite = Invite.objects.filter(token=invite_code).first() - if not invite: - raise forms.ValidationError("That is not a valid invite code") - if invite.email and invite.email != self.cleaned_data.get("email"): - raise forms.ValidationError( - "That is not a valid invite code for this email address" - ) - return invite_code - - def clean(self): - if not Config.system.signup_allowed: - raise forms.ValidationError("Not accepting new users at this time") + def dispatch(self, request, token=None, *args, **kwargs): + if token: + self.invite = get_object_or_404(Invite, token=token) + if not self.invite.valid: + raise Http404() + else: + self.invite = None + return super().dispatch(request, *args, **kwargs) def form_valid(self, form): + # Don't allow anything if there's no invite and no signup allowed + if not Config.system.signup_allowed and not self.invite: + return self.render_to_response(self.get_context_data()) + # Make the new user user = User.objects.create(email=form.cleaned_data["email"]) # Auto-promote the user to admin if that setting is set if settings.AUTO_ADMIN_EMAIL and user.email == settings.AUTO_ADMIN_EMAIL: user.admin = True user.save() PasswordReset.create_for_user(user) - if "invite_code" in form.cleaned_data: - Invite.objects.filter(token=form.cleaned_data["invite_code"]).delete() + # Drop invite uses down if it has them + if self.invite and self.invite.uses is not None: + self.invite.uses -= 1 + if self.invite.uses <= 0: + self.invite.delete() + else: + self.invite.save() return render( self.request, "auth/signup_success.html", @@ -114,6 +113,8 @@ class Signup(FormView): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) + if not Config.system.signup_allowed and not self.invite: + del context["form"] if Config.system.signup_text: context["signup_text"] = mark_safe( markdown_it.MarkdownIt().render(Config.system.signup_text)