Skip to content

Commit bf17161

Browse files
committed
Merged PR 962; support email validation when users change email address
2 parents 0ccb301 + 4f3855c commit bf17161

13 files changed

Lines changed: 484 additions & 31 deletions

File tree

askbot/conf/access_control.py

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -60,32 +60,23 @@
6060
)
6161
)
6262

63-
EMAIL_VALIDATION_CASE_CHOICES = (
64-
('nothing', _('nothing - not required')),
65-
('see-content', _('account registration')),
66-
# ('post-content', _('posting content')),
67-
)
68-
6963
settings.register(
70-
livesettings.StringValue(
64+
livesettings.BooleanValue(
7165
ACCESS_CONTROL,
72-
'REQUIRE_VALID_EMAIL_FOR',
73-
default='nothing',
74-
choices=EMAIL_VALIDATION_CASE_CHOICES,
75-
description=_('Require valid email for')
66+
'EMAIL_VALIDATION_REQUIRED',
67+
default=True,
68+
description=_('Require a valid email address'),
69+
help_text=_(
70+
'When enabled, users must verify their email address '
71+
'at registration or when changing their email address. '
72+
'To verify it, the users must click an emailed confirmation link '
73+
'from their inbox. '
74+
'Disable only on trusted installations where email '
75+
'ownership is guaranteed by other means.'
76+
)
7677
)
7778
)
7879

79-
# TODO: move REQUIRE_VALID_EMAIL_FOR to boolean setting
80-
# settings.register(
81-
# livesettings.BooleanValue(
82-
# ACCESS_CONTROL,
83-
# 'EMAIL_VALIDATION_REQUIRED',
84-
# default=False,
85-
# description=_('Require valid email address to register')
86-
# )
87-
# )
88-
8980

9081
def update_email_callback(old, new):
9182
if new.strip():

askbot/conf/email.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ def get_default_admin_email():
341341
help_text=format_lazy('{} {}',
342342
_('DANGER: makes impossible account recovery by email.'),
343343
settings.get_related_settings_info(
344-
('ACCESS_CONTROL', 'REQUIRE_VALID_EMAIL_FOR', True, _('Must be optional')),
344+
('ACCESS_CONTROL', 'EMAIL_VALIDATION_REQUIRED', True, _('Must be disabled')),
345345
)
346346
)
347347
)

askbot/conf/login_providers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@
135135
' ',
136136
settings.get_related_settings_info(
137137
('EMAIL', 'BLANK_EMAIL_ALLOWED', True, _('Must be enabled')),
138-
('ACCESS_CONTROL', 'REQUIRE_VALID_EMAIL_FOR', True, _('Must be optional')),
138+
('ACCESS_CONTROL', 'EMAIL_VALIDATION_REQUIRED', True, _('Must be disabled')),
139139
)
140140
),
141141
)
@@ -306,7 +306,7 @@
306306
'authentication.'),
307307
settings.get_related_settings_info(
308308
('EMAIL', 'BLANK_EMAIL_ALLOWED', True, _('Must be enabled')),
309-
('ACCESS_CONTROL', 'REQUIRE_VALID_EMAIL_FOR', True, _('Must be not be required')),
309+
('ACCESS_CONTROL', 'EMAIL_VALIDATION_REQUIRED', True, _('Must be disabled')),
310310
)
311311
),
312312
)

askbot/deps/django_authopenid/tests.py

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
from unittest import TestCase
2-
from askbot.deps.django_authopenid.forms import LoginForm
2+
3+
from django.contrib.auth.models import User
4+
from django.core import mail
5+
from django.test import TestCase as DjangoTestCase
6+
from django.urls import reverse
7+
38
from askbot import const
9+
from askbot.conf import settings as askbot_settings
10+
from askbot.deps.django_authopenid.forms import LoginForm
11+
from askbot.deps.django_authopenid.models import UserEmailVerifier
12+
from askbot.tests.utils import with_settings
13+
414

515
class LoginFormTests(TestCase):
616

@@ -19,3 +29,83 @@ def test_fail_change_to_short_password(self):
1929
self.assertFalse(result)
2030
self.assertEqual(form.initial.get('new_password'), None)
2131
#self.assertFalse('new_password_retyped' in form.cleaned_data)
32+
33+
34+
class SignupWithPasswordEmailValidationTests(DjangoTestCase):
35+
"""Exercise EMAIL_VALIDATION_REQUIRED toggle in the password signup
36+
flow (views.py:1338).
37+
38+
Forces ``BLANK_EMAIL_ALLOWED=False`` (the ambient default and the
39+
permanent direction — the setting is slated for removal) and forces
40+
off ``TERMS_CONSENT_REQUIRED``, ``USE_RECAPTCHA``, and
41+
``NEW_REGISTRATIONS_DISABLED`` so the minimal POST payload below is
42+
sufficient regardless of ambient livesettings state in the test DB.
43+
"""
44+
45+
def _post_signup(self, username, email):
46+
return self.client.post(
47+
reverse('user_signup_with_password'),
48+
{
49+
'next': '/',
50+
'username': username,
51+
'email': email,
52+
'password1': 'TestPass12345!',
53+
'password2': 'TestPass12345!',
54+
},
55+
)
56+
57+
@with_settings(EMAIL_VALIDATION_REQUIRED=False,
58+
BLANK_EMAIL_ALLOWED=False,
59+
TERMS_CONSENT_REQUIRED=False,
60+
USE_RECAPTCHA=False,
61+
NEW_REGISTRATIONS_DISABLED=False)
62+
def test_validation_off_creates_user_directly(self):
63+
response = self._post_signup('ua', 'a@example.com')
64+
# Validation-off path redirects via get_next_url(request)
65+
# (views.py:1347). Asserting 302 catches a silent form
66+
# rejection (which would render 200) before the body
67+
# assertions below could be misleadingly satisfied.
68+
self.assertEqual(response.status_code, 302)
69+
self.assertTrue(User.objects.filter(username='ua').exists())
70+
# No verification email on the fast path.
71+
self.assertEqual(len(mail.outbox), 0)
72+
73+
74+
@with_settings(EMAIL_VALIDATION_REQUIRED=True,
75+
BLANK_EMAIL_ALLOWED=False,
76+
TERMS_CONSENT_REQUIRED=False,
77+
USE_RECAPTCHA=False,
78+
NEW_REGISTRATIONS_DISABLED=False)
79+
def test_validation_on_delays_user_creation(self):
80+
response = self._post_signup('ub', 'b@example.com')
81+
# Validation-on path redirects to verify_email_and_register
82+
# (views.py:1357-1359). Without this 302 check, a regression
83+
# that silently re-renders the signup page (HTTP 200) would
84+
# leave User absent and outbox empty in the same shape these
85+
# assertions expect — the test would falsely pass.
86+
self.assertEqual(response.status_code, 302)
87+
self.assertTrue(
88+
response['Location'].startswith(
89+
reverse('verify_email_and_register')
90+
)
91+
)
92+
# With validation on, the view stashes the registration in a
93+
# UserEmailVerifier row (views.py:1349 constructs
94+
# ``UserEmailVerifier(key=...)`` and ``.save()``s it) and does
95+
# NOT create the user yet. Assert both sides so a regression
96+
# gives a clear signal rather than failing silently.
97+
self.assertFalse(User.objects.filter(username='ub').exists())
98+
99+
# UserEmailVerifier.value is a PickledObjectField (see
100+
# ``askbot/deps/django_authopenid/models.py:102``) — ORM
101+
# lookups into the pickled dict won't work. Filter in Python.
102+
verifiers = [
103+
v for v in UserEmailVerifier.objects.all()
104+
if v.value.get('username') == 'ub'
105+
]
106+
self.assertEqual(len(verifiers), 1)
107+
self.assertEqual(verifiers[0].value.get('email'), 'b@example.com')
108+
109+
# send_email_key fires on the validation-required path
110+
# (views.py:1355).
111+
self.assertEqual(len(mail.outbox), 1)

askbot/deps/django_authopenid/views.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ def email_is_acceptable(email):
8989
is_blank = (email == '')
9090
is_blank_and_ok = is_blank \
9191
and askbot_settings.BLANK_EMAIL_ALLOWED \
92-
and askbot_settings.REQUIRE_VALID_EMAIL_FOR == 'nothing'
92+
and not askbot_settings.EMAIL_VALIDATION_REQUIRED
9393
if is_blank_and_ok:
9494
return True
9595

@@ -1200,7 +1200,7 @@ def register(request, login_provider_name=None,
12001200
cleanup_post_register_session(request)
12011201
return HttpResponseRedirect(next_url)
12021202

1203-
if askbot_settings.REQUIRE_VALID_EMAIL_FOR == 'nothing':
1203+
if not askbot_settings.EMAIL_VALIDATION_REQUIRED:
12041204
user = create_authenticated_user_account(
12051205
username=username,
12061206
email=email,
@@ -1335,7 +1335,7 @@ def signup_with_password(request):
13351335
password = form.cleaned_data['password1']
13361336
email = form.cleaned_data['email']
13371337

1338-
if askbot_settings.REQUIRE_VALID_EMAIL_FOR == 'nothing':
1338+
if not askbot_settings.EMAIL_VALIDATION_REQUIRED:
13391339
user = create_authenticated_user_account(
13401340
username=username,
13411341
email=email,

askbot/doc/source/changelog.rst

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ Development (not yet released)
2020
* Fixed an issue in the markdown editor, where two different files
2121
could not be uploaded in one editing session.
2222
* Added ability to control who can access api v1.
23-
* Fixes to support JSON session serializer, changed default session serializer from Pickle to JSON
23+
* Fixed the support of the JSON session serializer,
24+
changed the default session serializer from Pickle to JSON
25+
* Added the option to require validation of email address when users change it.
26+
Controlled by the ``EMAIL_VALIDATION_REQUIRED`` livesetting.
2427

2528
0.12.8 (Mar 15, 2026)
2629
---------------------
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{% extends "email/base_mail.html"%}
2+
{% block title %}{% trans %}Confirm your new email address{% endtrans %}{% endblock %}
3+
{% block headline %}{% trans %}Confirm your new email address{% endtrans %}{% endblock %}
4+
5+
{% block content %}
6+
<p>{% trans %}You requested to change your email address on {{ site_name }}. Please follow the link below to confirm this change:{% endtrans %}</p>
7+
8+
<p><a href="{{ validation_link }}">{{ validation_link }}</a></p>
9+
10+
<p>{% trans %}If you did not request this change, please ignore this email. Your email address will not be changed.{% endtrans %}</p>
11+
{% endblock %}
12+
13+
{% block footer %}
14+
{% include "email/footer.html" %}
15+
{% endblock %}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{% trans %}Confirm your new email address on {{ site_name }}{% endtrans %}

askbot/mail/messages.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,22 @@ def process_context(self, context):
841841
# 'recipient_user': get_user()
842842
# }
843843

844+
class EmailChangeVerification(BaseEmail):
845+
template_path = 'email/email_change_verification'
846+
title = _('Email change verification')
847+
description = _('Sent when a user changes their email address, to verify the new address')
848+
mock_contexts = ({'key': 'a4umkaeuaousthsth'},)
849+
850+
def process_context(self, context):
851+
context.update({
852+
'site_name': askbot_settings.APP_SHORT_NAME,
853+
'recipient_user': None,
854+
'validation_link': site_url(reverse('verify_email_change')) + \
855+
'?validation_code=' + context['key']
856+
})
857+
return context
858+
859+
844860
class FeedbackEmail(BaseEmail):
845861
template_path = 'email/feedback'
846862
title = _('Feedback email')
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
"""Data migration: collapse ``REQUIRE_VALID_EMAIL_FOR`` into the boolean
2+
``EMAIL_VALIDATION_REQUIRED``.
3+
4+
New default is ``True`` (email validation required) - the safer default
5+
for fresh installs. Upgrading sites that had the old default
6+
(``'nothing'`` / no row) while carrying real content (any ``Post`` rows)
7+
get an explicit ``False`` row written to preserve their previous
8+
behavior. Sites that had ``'see-content'`` (email was required) have any
9+
stale row removed so the new ``True`` default applies. Fresh installs
10+
(no posts) get no row either and inherit the new ``True`` default.
11+
12+
The forward function is schema-agnostic: it uses
13+
``apps.get_model('livesettings', 'Setting')`` instead of raw SQL against
14+
``livesettings_setting``. That shields the migration from livesettings
15+
schema changes and DB-specific quoting of the reserved word ``group``.
16+
17+
``Post.objects.exists()`` is used (not ``User.objects.exists()``) so the
18+
``createsuperuser`` before ``migrate`` sequence does not falsely mark a
19+
fresh install as an upgrading site. Any real activity on the forum
20+
produces at least one Post.
21+
"""
22+
from django.db import migrations, transaction
23+
24+
25+
OLD_KEY = 'REQUIRE_VALID_EMAIL_FOR'
26+
NEW_KEY = 'EMAIL_VALIDATION_REQUIRED'
27+
GROUP = 'ACCESS_CONTROL'
28+
29+
30+
def forward(apps, schema_editor):
31+
Setting = apps.get_model('livesettings', 'Setting')
32+
Site = apps.get_model('sites', 'Site')
33+
Post = apps.get_model('askbot', 'Post')
34+
35+
has_posts = Post.objects.exists()
36+
37+
with transaction.atomic():
38+
for site in Site.objects.all():
39+
old_rows = list(Setting.objects.filter(
40+
site=site, group=GROUP, key=OLD_KEY
41+
))
42+
old_value = old_rows[0].value if old_rows else 'nothing'
43+
44+
# Preserve old behavior for upgrading sites that had validation
45+
# off ('nothing' was the old default). Fresh installs (no Post
46+
# rows) get no row written and inherit the new True default.
47+
if old_value != 'see-content' and has_posts:
48+
Setting.objects.update_or_create(
49+
site=site, group=GROUP, key=NEW_KEY,
50+
defaults={'value': 'False'},
51+
)
52+
else:
53+
# 'see-content' (validation was required) or fresh install:
54+
# delete any stale NEW_KEY row so the new True default
55+
# applies cleanly.
56+
Setting.objects.filter(
57+
site=site, group=GROUP, key=NEW_KEY
58+
).delete()
59+
60+
# Always drop the obsolete OLD_KEY row.
61+
for row in old_rows:
62+
row.delete()
63+
64+
65+
def reverse(apps, schema_editor):
66+
"""Reverse: reconstruct ``REQUIRE_VALID_EMAIL_FOR`` from the boolean.
67+
68+
Mapping is the inverse of forward:
69+
70+
- ``EMAIL_VALIDATION_REQUIRED = False`` (explicit) -> write
71+
``REQUIRE_VALID_EMAIL_FOR = 'nothing'``.
72+
- ``EMAIL_VALIDATION_REQUIRED = True`` / missing -> write
73+
``REQUIRE_VALID_EMAIL_FOR = 'see-content'`` only when there are
74+
posts (an upgrading site). Fresh sites get no row so the old
75+
``'nothing'`` default applies.
76+
77+
The old default was ``'nothing'`` while the new default is ``True``,
78+
so a naive reverse would silently flip fresh sites from "no
79+
validation" (pre-migration) to "validation required" (post-reverse).
80+
Gating the ``'see-content'`` write on ``has_posts`` preserves the
81+
pre-migration default on fresh installs and round-trips cleanly for
82+
upgrading sites.
83+
"""
84+
Setting = apps.get_model('livesettings', 'Setting')
85+
Site = apps.get_model('sites', 'Site')
86+
Post = apps.get_model('askbot', 'Post')
87+
88+
has_posts = Post.objects.exists()
89+
90+
with transaction.atomic():
91+
for site in Site.objects.all():
92+
new_rows = list(Setting.objects.filter(
93+
site=site, group=GROUP, key=NEW_KEY
94+
))
95+
new_value = new_rows[0].value if new_rows else 'True'
96+
97+
if new_value == 'False':
98+
Setting.objects.update_or_create(
99+
site=site, group=GROUP, key=OLD_KEY,
100+
defaults={'value': 'nothing'},
101+
)
102+
elif has_posts:
103+
Setting.objects.update_or_create(
104+
site=site, group=GROUP, key=OLD_KEY,
105+
defaults={'value': 'see-content'},
106+
)
107+
else:
108+
Setting.objects.filter(
109+
site=site, group=GROUP, key=OLD_KEY
110+
).delete()
111+
112+
for row in new_rows:
113+
row.delete()
114+
115+
116+
class Migration(migrations.Migration):
117+
118+
dependencies = [
119+
('askbot', '0035_set_global_group_used_for_analytics'),
120+
('sites', '0001_initial'),
121+
('livesettings', '0001_initial'),
122+
]
123+
124+
operations = [
125+
migrations.RunPython(forward, reverse),
126+
]

0 commit comments

Comments
 (0)