Commit 4c3f68cd by Ahsan Ulhaq

Forgot Password leak info about valid accounts

ECOM-4703
parent c8f0e00e
...@@ -145,18 +145,15 @@ class LoginFromCombinedPageTest(UniqueCourseTest): ...@@ -145,18 +145,15 @@ class LoginFromCombinedPageTest(UniqueCourseTest):
# Expect that we're shown a success message # Expect that we're shown a success message
self.assertIn("Password Reset Email Sent", self.login_page.wait_for_success()) self.assertIn("Password Reset Email Sent", self.login_page.wait_for_success())
def test_password_reset_failure(self): def test_password_reset_no_user(self):
# Navigate to the password reset form # Navigate to the password reset form
self.login_page.visit() self.login_page.visit()
# User account does not exist # User account does not exist
self.login_page.password_reset(email="nobody@nowhere.com") self.login_page.password_reset(email="nobody@nowhere.com")
# Expect that we're shown a failure message # Expect that we're shown a success message
self.assertIn( self.assertIn("Password Reset Email Sent", self.login_page.wait_for_success())
"No user with the provided email address exists.",
self.login_page.wait_for_errors()
)
def test_third_party_login(self): def test_third_party_login(self):
""" """
......
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
""" Tests for student account views. """ """ Tests for student account views. """
import logging
import re import re
from unittest import skipUnless from unittest import skipUnless
from urllib import urlencode from urllib import urlencode
...@@ -18,6 +19,7 @@ from django.test.utils import override_settings ...@@ -18,6 +19,7 @@ from django.test.utils import override_settings
from django.http import HttpRequest from django.http import HttpRequest
from edx_rest_api_client import exceptions from edx_rest_api_client import exceptions
from nose.plugins.attrib import attr from nose.plugins.attrib import attr
from testfixtures import LogCapture
from commerce.models import CommerceConfiguration from commerce.models import CommerceConfiguration
from commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY, factories from commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY, factories
...@@ -36,6 +38,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase ...@@ -36,6 +38,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context
LOGGER_NAME = 'audit'
@ddt.ddt @ddt.ddt
class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
""" Tests for the student account views that update the user's account information. """ """ Tests for the student account views that update the user's account information. """
...@@ -175,9 +180,11 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): ...@@ -175,9 +180,11 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
# Log out the user created during test setup # Log out the user created during test setup
self.client.logout() self.client.logout()
# Send the view an email address not tied to any user with LogCapture(LOGGER_NAME, level=logging.INFO) as logger:
response = self._change_password(email=self.NEW_EMAIL) # Send the view an email address not tied to any user
self.assertEqual(response.status_code, 400) response = self._change_password(email=self.NEW_EMAIL)
self.assertEqual(response.status_code, 200)
logger.check((LOGGER_NAME, 'INFO', 'Invalid password reset attempt'))
def test_password_change_rate_limited(self): def test_password_change_rate_limited(self):
# Log out the user created during test setup, to prevent the view from # Log out the user created during test setup, to prevent the view from
......
...@@ -110,6 +110,7 @@ def login_and_registration_form(request, initial_mode="login"): ...@@ -110,6 +110,7 @@ def login_and_registration_form(request, initial_mode="login"):
'third_party_auth': _third_party_auth_context(request, redirect_to), 'third_party_auth': _third_party_auth_context(request, redirect_to),
'third_party_auth_hint': third_party_auth_hint or '', 'third_party_auth_hint': third_party_auth_hint or '',
'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
'support_link': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK),
# Include form descriptions retrieved from the user API. # Include form descriptions retrieved from the user API.
# We could have the JS client make these requests directly, # We could have the JS client make these requests directly,
...@@ -148,8 +149,7 @@ def password_change_request_handler(request): ...@@ -148,8 +149,7 @@ def password_change_request_handler(request):
Returns: Returns:
HttpResponse: 200 if the email was sent successfully HttpResponse: 200 if the email was sent successfully
HttpResponse: 400 if there is no 'email' POST parameter, or if no user with HttpResponse: 400 if there is no 'email' POST parameter
the provided email exists
HttpResponse: 403 if the client has been rate limited HttpResponse: 403 if the client has been rate limited
HttpResponse: 405 if using an unsupported HTTP method HttpResponse: 405 if using an unsupported HTTP method
...@@ -158,6 +158,7 @@ def password_change_request_handler(request): ...@@ -158,6 +158,7 @@ def password_change_request_handler(request):
POST /account/password POST /account/password
""" """
limiter = BadRequestRateLimiter() limiter = BadRequestRateLimiter()
if limiter.is_rate_limit_exceeded(request): if limiter.is_rate_limit_exceeded(request):
AUDIT_LOG.warning("Password reset rate limit exceeded") AUDIT_LOG.warning("Password reset rate limit exceeded")
...@@ -175,8 +176,6 @@ def password_change_request_handler(request): ...@@ -175,8 +176,6 @@ def password_change_request_handler(request):
# Increment the rate limit counter # Increment the rate limit counter
limiter.tick_bad_request_counter(request) limiter.tick_bad_request_counter(request)
return HttpResponseBadRequest(_("No user with the provided email address exists."))
return HttpResponse(status=200) return HttpResponse(status=200)
else: else:
return HttpResponseBadRequest(_("No email address provided.")) return HttpResponseBadRequest(_("No email address provided."))
......
...@@ -70,6 +70,7 @@ ...@@ -70,6 +70,7 @@
}; };
this.platformName = options.platform_name; this.platformName = options.platform_name;
this.supportURL = options.support_link;
// The login view listens for 'sync' events from the reset model // The login view listens for 'sync' events from the reset model
this.resetModel = new PasswordResetModel({}, { this.resetModel = new PasswordResetModel({}, {
...@@ -120,7 +121,8 @@ ...@@ -120,7 +121,8 @@
model: model, model: model,
resetModel: this.resetModel, resetModel: this.resetModel,
thirdPartyAuth: this.thirdPartyAuth, thirdPartyAuth: this.thirdPartyAuth,
platformName: this.platformName platformName: this.platformName,
supportURL: this.supportURL
}); });
// Listen for 'password-help' event to toggle sub-views // Listen for 'password-help' event to toggle sub-views
......
;(function (define) { ;(function (define) {
'use strict'; 'use strict';
define([ define([
'jquery', 'jquery',
'underscore', 'underscore',
'gettext', 'gettext',
'js/student_account/views/FormView' 'edx-ui-toolkit/js/utils/html-utils',
'js/student_account/views/FormView'
], ],
function($, _, gettext, FormView) { function($, _, gettext, HtmlUtils, FormView) {
return FormView.extend({ return FormView.extend({
el: '#login-form', el: '#login-form',
tpl: '#login-tpl', tpl: '#login-tpl',
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
this.errorMessage = data.thirdPartyAuth.errorMessage || ''; this.errorMessage = data.thirdPartyAuth.errorMessage || '';
this.platformName = data.platformName; this.platformName = data.platformName;
this.resetModel = data.resetModel; this.resetModel = data.resetModel;
this.supportURL = data.supportURL;
this.listenTo( this.model, 'sync', this.saveSuccess ); this.listenTo( this.model, 'sync', this.saveSuccess );
this.listenTo( this.resetModel, 'sync', this.resetEmail ); this.listenTo( this.resetModel, 'sync', this.resetEmail );
...@@ -36,6 +37,13 @@ ...@@ -36,6 +37,13 @@
render: function( html ) { render: function( html ) {
var fields = html || ''; var fields = html || '';
this.successMessage = HtmlUtils.interpolateHtml(
// eslint-disable-next-line
gettext('We have sent an email message with password reset instructions to the email address you provided. If you do not receive this message, {anchorStart}contact technical support{anchorEnd}.'), { // jshint ignore:line
anchorStart: HtmlUtils.HTML('<a href="' + this.supportURL + '">'),
anchorEnd: HtmlUtils.HTML('</a>')
}
);
$(this.el).html(_.template(this.tpl)({ $(this.el).html(_.template(this.tpl)({
// We pass the context object to the template so that // We pass the context object to the template so that
...@@ -86,6 +94,16 @@ ...@@ -86,6 +94,16 @@
resetEmail: function() { resetEmail: function() {
this.element.hide( this.$errors ); this.element.hide( this.$errors );
this.resetMessage = this.$resetSuccess.find('.message-copy');
if (this.resetMessage.find('p').length === 0) {
this.resetMessage.append(
HtmlUtils.joinHtml(
HtmlUtils.HTML('<p>'),
this.successMessage,
HtmlUtils.HTML('</p>')
).toString()
);
}
this.element.show( this.$resetSuccess ); this.element.show( this.$resetSuccess );
}, },
......
...@@ -10,9 +10,6 @@ ...@@ -10,9 +10,6 @@
<div class="js-reset-success status submission-success hidden"> <div class="js-reset-success status submission-success hidden">
<h4 class="message-title"><%- gettext("Password Reset Email Sent") %></h4> <h4 class="message-title"><%- gettext("Password Reset Email Sent") %></h4>
<div class="message-copy"> <div class="message-copy">
<p>
<%- gettext("We've sent instructions for resetting your password to the email address you provided.") %>
</p>
</div> </div>
</div> </div>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment