Restructure ForgotPassword view
1) Remove mongo limitations (no 'or' when querying for either username
or email).
2) Lost password function revealed if an user name or email address
is registered, which can be considered a data leak.
Leaking user names is OK, they are public anyway, but don't reveal
lookup success in case the lookup happened by email address.
Simply respond: "If you have an account here, we have send you
your email"?
3) username and email search was case sensitive. Made username search
case insensitive (they are always stored lowercase in the db).
Keep email-address search case sensitive for now. This might need
further discussion
4) Remove a whole bunch of indention in the style of:
if no error:
...
if no error:
...
if no error:
actually do something in the regular case
by restructuring the function.
5) Outsource the sanity checking for username and email fields into the
validator function. This way, we get automatic case sanity checking
and sanitizing for all required fields.
6) Require 5-char password and fix tests
Originally, the Change password form required a password between 6-30
chars while the registration and login form did not require anything
special. This commit introduces a common minimum limit for all forms
which breaks the test suite which uses a 5 char password by
default. :-). As 5 chars seem sensible enough to enforce (people
should be picking much longer ones anyway), just reduce the limit to
5 chars, thereby making all tests pass.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This commit is contained in:
Sebastian Spaeth
parent
0f9cf6ef32
commit
a89df96132
@ -17,52 +17,75 @@
|
||||
import wtforms
|
||||
import re
|
||||
|
||||
from mediagoblin.tools.mail import normalize_email
|
||||
from mediagoblin.tools.translate import fake_ugettext_passthrough as _
|
||||
|
||||
def normalize_user_or_email_field(allow_email=True, allow_user=True):
|
||||
"""Check if we were passed a field that matches a username and/or email pattern
|
||||
|
||||
This is useful for fields that can take either a username or email
|
||||
address. Use the parameters if you want to only allow a username for
|
||||
instance"""
|
||||
message = _(u'Invalid User name or email address.')
|
||||
nomail_msg = _(u"This field does not take email addresses.")
|
||||
nouser_msg = _(u"This field requires an email address.")
|
||||
|
||||
def _normalize_field(form, field):
|
||||
email = u'@' in field.data
|
||||
if email: # normalize email address casing
|
||||
if not allow_email:
|
||||
raise wtforms.ValidationError(nomail_msg)
|
||||
wtforms.validators.Email()(form, field)
|
||||
field.data = normalize_email(field.data)
|
||||
else: # lower case user names
|
||||
if not allow_user:
|
||||
raise wtforms.ValidationError(nouser_msg)
|
||||
wtforms.validators.Length(min=3, max=30)(form, field)
|
||||
wtforms.validators.Regexp(r'^\w+$')(form, field)
|
||||
field.data = field.data.lower()
|
||||
if field.data is None: # should not happen, but be cautious anyway
|
||||
raise wtforms.ValidationError(message)
|
||||
return _normalize_field
|
||||
|
||||
|
||||
class RegistrationForm(wtforms.Form):
|
||||
username = wtforms.TextField(
|
||||
_('Username'),
|
||||
[wtforms.validators.Required(),
|
||||
wtforms.validators.Length(min=3, max=30),
|
||||
wtforms.validators.Regexp(r'^\w+$')])
|
||||
normalize_user_or_email_field(allow_email=False)])
|
||||
password = wtforms.PasswordField(
|
||||
_('Password'),
|
||||
[wtforms.validators.Required(),
|
||||
wtforms.validators.Length(min=6, max=30)])
|
||||
wtforms.validators.Length(min=5, max=1024)])
|
||||
email = wtforms.TextField(
|
||||
_('Email address'),
|
||||
[wtforms.validators.Required(),
|
||||
wtforms.validators.Email()])
|
||||
normalize_user_or_email_field(allow_user=False)])
|
||||
|
||||
|
||||
class LoginForm(wtforms.Form):
|
||||
username = wtforms.TextField(
|
||||
_('Username'),
|
||||
[wtforms.validators.Required(),
|
||||
wtforms.validators.Regexp(r'^\w+$')])
|
||||
normalize_user_or_email_field(allow_email=False)])
|
||||
password = wtforms.PasswordField(
|
||||
_('Password'),
|
||||
[wtforms.validators.Required()])
|
||||
[wtforms.validators.Required(),
|
||||
wtforms.validators.Length(min=5, max=1024)])
|
||||
|
||||
|
||||
class ForgotPassForm(wtforms.Form):
|
||||
username = wtforms.TextField(
|
||||
_('Username or email'),
|
||||
[wtforms.validators.Required()])
|
||||
|
||||
def validate_username(form, field):
|
||||
if not (re.match(r'^\w+$', field.data) or
|
||||
re.match(r'^.+@[^.].*\.[a-z]{2,10}$', field.data,
|
||||
re.IGNORECASE)):
|
||||
raise wtforms.ValidationError(_(u'Incorrect input'))
|
||||
[wtforms.validators.Required(),
|
||||
normalize_user_or_email_field()])
|
||||
|
||||
|
||||
class ChangePassForm(wtforms.Form):
|
||||
password = wtforms.PasswordField(
|
||||
'Password',
|
||||
[wtforms.validators.Required(),
|
||||
wtforms.validators.Length(min=6, max=30)])
|
||||
wtforms.validators.Length(min=5, max=1024)])
|
||||
userid = wtforms.HiddenField(
|
||||
'',
|
||||
[wtforms.validators.Required()])
|
||||
|
||||
@ -41,8 +41,10 @@ def email_debug_message(request):
|
||||
|
||||
|
||||
def register(request):
|
||||
"""
|
||||
Your classic registration view!
|
||||
"""The registration view.
|
||||
|
||||
Note that usernames will always be lowercased. Email domains are lowercased while
|
||||
the first part remains case-sensitive.
|
||||
"""
|
||||
# Redirects to indexpage if registrations are disabled
|
||||
if not mg_globals.app_config["allow_registration"]:
|
||||
@ -56,12 +58,8 @@ def register(request):
|
||||
|
||||
if request.method == 'POST' and register_form.validate():
|
||||
# TODO: Make sure the user doesn't exist already
|
||||
username = unicode(request.form['username'].lower())
|
||||
em_user, em_dom = unicode(request.form['email']).split("@", 1)
|
||||
em_dom = em_dom.lower()
|
||||
email = em_user + "@" + em_dom
|
||||
users_with_username = User.query.filter_by(username=username).count()
|
||||
users_with_email = User.query.filter_by(email=email).count()
|
||||
users_with_username = User.query.filter_by(username=register_form.data['username']).count()
|
||||
users_with_email = User.query.filter_by(email=register_form.data['email']).count()
|
||||
|
||||
extra_validation_passes = True
|
||||
|
||||
@ -77,8 +75,8 @@ def register(request):
|
||||
if extra_validation_passes:
|
||||
# Create the user
|
||||
user = User()
|
||||
user.username = username
|
||||
user.email = email
|
||||
user.username = register_form.data['username']
|
||||
user.email = register_form.data['email']
|
||||
user.pw_hash = auth_lib.bcrypt_gen_password_hash(
|
||||
request.form['password'])
|
||||
user.verification_key = unicode(uuid.uuid4())
|
||||
@ -115,7 +113,7 @@ def login(request):
|
||||
login_failed = False
|
||||
|
||||
if request.method == 'POST' and login_form.validate():
|
||||
user = User.query.filter_by(username=request.form['username'].lower()).first()
|
||||
user = User.query.filter_by(username=login_form.data['username']).first()
|
||||
|
||||
if user and user.check_login(request.form['password']):
|
||||
# set up login in session
|
||||
@ -227,59 +225,66 @@ def forgot_password(request):
|
||||
"""
|
||||
Forgot password view
|
||||
|
||||
Sends an email with an url to renew forgotten password
|
||||
Sends an email with an url to renew forgotten password.
|
||||
Use GET querystring parameter 'username' to pre-populate the input field
|
||||
"""
|
||||
fp_form = auth_forms.ForgotPassForm(request.form,
|
||||
username=request.GET.get('username'))
|
||||
username=request.args.get('username'))
|
||||
|
||||
if request.method == 'POST' and fp_form.validate():
|
||||
if not (request.method == 'POST' and fp_form.validate()):
|
||||
# Either GET request, or invalid form submitted. Display the template
|
||||
return render_to_response(request,
|
||||
'mediagoblin/auth/forgot_password.html', {'fp_form': fp_form})
|
||||
|
||||
# '$or' not available till mongodb 1.5.3
|
||||
user = User.query.filter_by(username=request.form['username']).first()
|
||||
if not user:
|
||||
user = User.query.filter_by(email=request.form['username']).first()
|
||||
# If we are here: method == POST and form is valid. username casing
|
||||
# has been sanitized. Store if a user was found by email. We should
|
||||
# not reveal if the operation was successful then as we don't want to
|
||||
# leak if an email address exists in the system.
|
||||
found_by_email = '@' in request.form['username']
|
||||
|
||||
if user:
|
||||
if user.email_verified and user.status == 'active':
|
||||
user.fp_verification_key = unicode(uuid.uuid4())
|
||||
user.fp_token_expire = datetime.datetime.now() + \
|
||||
datetime.timedelta(days=10)
|
||||
user.save()
|
||||
if found_by_email:
|
||||
user = User.query.filter_by(
|
||||
email = request.form['username']).first()
|
||||
# Don't reveal success in case the lookup happened by email address.
|
||||
success_message=_("If that email address (case sensitive!) is "
|
||||
"registered an email has been sent with instructions "
|
||||
"on how to change your password.")
|
||||
|
||||
send_fp_verification_email(user, request)
|
||||
else: # found by username
|
||||
user = User.query.filter_by(
|
||||
username = request.form['username']).first()
|
||||
|
||||
messages.add_message(
|
||||
request,
|
||||
messages.INFO,
|
||||
_("An email has been sent with instructions on how to "
|
||||
"change your password."))
|
||||
email_debug_message(request)
|
||||
|
||||
else:
|
||||
# special case... we can't send the email because the
|
||||
# username is inactive / hasn't verified their email
|
||||
messages.add_message(
|
||||
request,
|
||||
messages.WARNING,
|
||||
_("Could not send password recovery email as "
|
||||
"your username is inactive or your account's "
|
||||
"email address has not been verified."))
|
||||
|
||||
return redirect(
|
||||
request, 'mediagoblin.user_pages.user_home',
|
||||
user=user.username)
|
||||
return redirect(request, 'mediagoblin.auth.login')
|
||||
else:
|
||||
messages.add_message(
|
||||
request,
|
||||
messages.WARNING,
|
||||
_("Couldn't find someone with that username or email."))
|
||||
if user is None:
|
||||
messages.add_message(request,
|
||||
messages.WARNING,
|
||||
_("Couldn't find someone with that username."))
|
||||
return redirect(request, 'mediagoblin.auth.forgot_password')
|
||||
|
||||
return render_to_response(
|
||||
request,
|
||||
'mediagoblin/auth/forgot_password.html',
|
||||
{'fp_form': fp_form})
|
||||
success_message=_("An email has been sent with instructions "
|
||||
"on how to change your password.")
|
||||
|
||||
if user and not(user.email_verified and user.status == 'active'):
|
||||
# Don't send reminder because user is inactive or has no verified email
|
||||
messages.add_message(request,
|
||||
messages.WARNING,
|
||||
_("Could not send password recovery email as your username is in"
|
||||
"active or your account's email address has not been verified."))
|
||||
|
||||
return redirect(request, 'mediagoblin.user_pages.user_home',
|
||||
user=user.username)
|
||||
|
||||
# SUCCESS. Send reminder and return to login page
|
||||
if user:
|
||||
user.fp_verification_key = unicode(uuid.uuid4())
|
||||
user.fp_token_expire = datetime.datetime.now() + \
|
||||
datetime.timedelta(days=10)
|
||||
user.save()
|
||||
|
||||
email_debug_message(request)
|
||||
send_fp_verification_email(user, request)
|
||||
|
||||
messages.add_message(request, messages.INFO, success_message)
|
||||
return redirect(request, 'mediagoblin.auth.login')
|
||||
|
||||
|
||||
def verify_forgot_password(request):
|
||||
|
||||
@ -105,10 +105,8 @@ def test_register_views(test_app):
|
||||
context = template.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/register.html']
|
||||
form = context['register_form']
|
||||
|
||||
assert form.username.errors == [
|
||||
u'Field must be between 3 and 30 characters long.']
|
||||
assert form.password.errors == [
|
||||
u'Field must be between 6 and 30 characters long.']
|
||||
assert_equal (form.username.errors, [u'Field must be between 3 and 30 characters long.'])
|
||||
assert_equal (form.password.errors, [u'Field must be between 5 and 1024 characters long.'])
|
||||
|
||||
## bad form
|
||||
template.clear_test_template_context()
|
||||
@ -119,10 +117,8 @@ def test_register_views(test_app):
|
||||
context = template.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/register.html']
|
||||
form = context['register_form']
|
||||
|
||||
assert form.username.errors == [
|
||||
u'Invalid input.']
|
||||
assert form.email.errors == [
|
||||
u'Invalid email address.']
|
||||
assert_equal (form.username.errors, [u'This field does not take email addresses.'])
|
||||
assert_equal (form.email.errors, [u'This field requires an email address.'])
|
||||
|
||||
## At this point there should be no users in the database ;)
|
||||
assert_equal(User.query.count(), 0)
|
||||
@ -370,7 +366,7 @@ def test_authentication_views():
|
||||
response = test_app.post(
|
||||
'/auth/login/', {
|
||||
'username': u'chris',
|
||||
'password': 'jam'})
|
||||
'password': 'jam_and_ham'})
|
||||
context = template.TEMPLATE_TEST_CONTEXT['mediagoblin/auth/login.html']
|
||||
assert context['login_failed']
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user