Generalizes error model for change password verification

- 404s instead of 'user not found' will limit leaking user profile
  information to the browser.
- Also fixed the wording on the login page to make it clear you are
  changing the password, not sending yourself your old one!
This commit is contained in:
Caleb Forbes Davis V 2011-08-29 00:00:59 -05:00
parent 24966c43bd
commit e1105f5dcb
2 changed files with 12 additions and 8 deletions

View File

@ -226,18 +226,19 @@ def verify_forgot_password(request):
# If we don't have userid and token parameters, we can't do anything;404 # If we don't have userid and token parameters, we can't do anything;404
if (not request.GET.has_key('userid') or if (not request.GET.has_key('userid') or
not request.GET.has_key('token')): not request.GET.has_key('token')):
return exc.HTTPNotFound('You must provide userid and token') return render_404(request)
# check if it's a valid Id # check if it's a valid Id
try: try:
user = request.db.User.find_one( user = request.db.User.find_one(
{'_id': ObjectId(unicode(request.GET['userid']))}) {'_id': ObjectId(unicode(request.GET['userid']))})
except InvalidId: except InvalidId:
return exc.HTTPNotFound('Invalid id') return render_404(request)
# check if we have a real user and correct token # check if we have a real user and correct token
if (user and if (user and
user['fp_verification_key'] == unicode(request.GET['token'])): user['fp_verification_key'] == unicode(request.GET['token']) and
datetime.datetime.now() < user['fp_token_expire']):
cp_form = auth_forms.ChangePassForm(request.GET) cp_form = auth_forms.ChangePassForm(request.GET)
return render_to_response( return render_to_response(
@ -245,27 +246,30 @@ def verify_forgot_password(request):
'mediagoblin/auth/change_fp.html', 'mediagoblin/auth/change_fp.html',
{'cp_form': cp_form}) {'cp_form': cp_form})
# in case there is a valid id but no user whit that id in the db # in case there is a valid id but no user whit that id in the db
# or the token expired
else: else:
return exc.HTTPNotFound('User not found') return render_404(request)
if request.method == 'POST': if request.method == 'POST':
# verification doing here to prevent POST values modification # verification doing here to prevent POST values modification
try: try:
user = request.db.User.find_one( user = request.db.User.find_one(
{'_id': ObjectId(unicode(request.POST['userid']))}) {'_id': ObjectId(unicode(request.POST['userid']))})
except InvalidId: except InvalidId:
return exc.HTTPNotFound('Invalid id') return render_404(request)
cp_form = auth_forms.ChangePassForm(request.POST) cp_form = auth_forms.ChangePassForm(request.POST)
# verification doing here to prevent POST values modification # verification doing here to prevent POST values modification
# if token and id are correct they are able to change their password # if token and id are correct they are able to change their password
if (user and if (user and
user['fp_verification_key'] == unicode(request.POST['token'])): user['fp_verification_key'] == unicode(request.POST['token']) and
datetime.datetime.now() < user['fp_token_expire']):
if cp_form.validate(): if cp_form.validate():
user['pw_hash'] = auth_lib.bcrypt_gen_password_hash( user['pw_hash'] = auth_lib.bcrypt_gen_password_hash(
request.POST['password']) request.POST['password'])
user['fp_verification_key'] = None user['fp_verification_key'] = None
user['fp_token_expire'] = None
user.save() user.save()
return redirect(request, return redirect(request,
@ -276,4 +280,4 @@ def verify_forgot_password(request):
'mediagoblin/auth/change_fp.html', 'mediagoblin/auth/change_fp.html',
{'cp_form': cp_form}) {'cp_form': cp_form})
else: else:
return exc.HTTPNotFound('User not found') return render_404(request)

View File

@ -48,7 +48,7 @@
{% trans %}Forgot your password?{% endtrans %} {% trans %}Forgot your password?{% endtrans %}
<br /> <br />
<a href="{{ request.urlgen('mediagoblin.auth.forgot_password') }}"> <a href="{{ request.urlgen('mediagoblin.auth.forgot_password') }}">
{%- trans %}Send yourself a reminder!{% endtrans %}</a> {%- trans %}Change it!{% endtrans %}</a>
</p> </p>
{% endif %} {% endif %}
</div> </div>