diff options
7 files changed, 284 insertions, 23 deletions
diff --git a/service/pixelated/resources/account_recovery_resource.py b/service/pixelated/resources/account_recovery_resource.py index 39ebb8d0..209a7693 100644 --- a/service/pixelated/resources/account_recovery_resource.py +++ b/service/pixelated/resources/account_recovery_resource.py @@ -15,14 +15,23 @@ # along with Pixelated. If not, see <http://www.gnu.org/licenses/>. import os +import json -from pixelated.resources import BaseResource from twisted.python.filepath import FilePath -from pixelated.resources import get_public_static_folder -from twisted.web.http import OK +from twisted.web.http import OK, INTERNAL_SERVER_ERROR from twisted.web.template import Element, XMLFile, renderElement from twisted.web.server import NOT_DONE_YET from twisted.internet import defer +from twisted.logger import Logger + +from pixelated.resources import BaseResource +from pixelated.resources import get_public_static_folder + +log = Logger() + + +class InvalidPasswordError(Exception): + pass class AccountRecoveryPage(Element): @@ -52,10 +61,27 @@ class AccountRecoveryResource(BaseResource): request.setResponseCode(OK) request.finish() - def error_response(response): + def error_response(failure): + log.warn(failure) request.setResponseCode(INTERNAL_SERVER_ERROR) request.finish() - d = defer.succeed('Done!') + d = self._handle_post(request) d.addCallbacks(success_response, error_response) return NOT_DONE_YET + + def _get_post_form(self, request): + return json.loads(request.content.getvalue()) + + def _validate_password(self, password, confirm_password): + return password == confirm_password and len(password) >= 8 and len(password) <= 9999 + + def _handle_post(self, request): + form = self._get_post_form(request) + password = form.get('password') + confirm_password = form.get('confirmPassword') + + if not self._validate_password(password, confirm_password): + return defer.fail(InvalidPasswordError('The user entered an invalid password or confirmation')) + + return defer.succeed('Done!') diff --git a/service/test/unit/resources/test_account_recovery_resource.py b/service/test/unit/resources/test_account_recovery_resource.py index cd9acae7..4e26fc5b 100644 --- a/service/test/unit/resources/test_account_recovery_resource.py +++ b/service/test/unit/resources/test_account_recovery_resource.py @@ -14,14 +14,12 @@ # You should have received a copy of the GNU Affero General Public License # along with Pixelated. If not, see <http://www.gnu.org/licenses/>. -import os - from mock import MagicMock from twisted.trial import unittest from twisted.web.test.requesthelper import DummyRequest from twisted.internet import defer -from pixelated.resources.account_recovery_resource import AccountRecoveryResource +from pixelated.resources.account_recovery_resource import AccountRecoveryResource, InvalidPasswordError from test.unit.resources import DummySite @@ -46,6 +44,8 @@ class TestAccountRecoveryResource(unittest.TestCase): def test_post_returns_successfully(self): request = DummyRequest(['/account-recovery']) request.method = 'POST' + self.resource._handle_post = MagicMock(return_value=defer.succeed(None)) + d = self.web.get(request) def assert_successful_response(_): @@ -53,3 +53,56 @@ class TestAccountRecoveryResource(unittest.TestCase): d.addCallback(assert_successful_response) return d + + def test_post_returns_failure(self): + request = DummyRequest(['/account-recovery']) + request.method = 'POST' + self.resource._handle_post = MagicMock(return_value=defer.fail(InvalidPasswordError)) + + d = self.web.get(request) + + def assert_error_response(_): + self.assertEqual(500, request.responseCode) + + d.addCallback(assert_error_response) + return d + + def test_handle_post_successfully(self): + request = MagicMock() + self.resource._get_post_form = MagicMock() + self.resource._validate_password = MagicMock(return_value=True) + + d = self.resource._handle_post(request) + + def assert_successful(success): + self.assertEqual(success, 'Done!') + + d.addCallback(assert_successful) + return d + + @defer.inlineCallbacks + def test_handle_post_failed(self): + request = MagicMock() + self.resource._get_post_form = MagicMock() + self.resource._validate_password = MagicMock(return_value=False) + + with self.assertRaises(InvalidPasswordError): + yield self.resource._handle_post(request) + + def test_get_post_form(self): + request = MagicMock() + request.content.getvalue.return_value = '{"userCode": "abc", "password": "123", "confirmPassword": "456"}' + form = self.resource._get_post_form(request) + + self.assertEqual(form.get('userCode'), 'abc') + self.assertEqual(form.get('password'), '123') + self.assertEqual(form.get('confirmPassword'), '456') + + def test_validate_password_successfully(self): + self.assertTrue(self.resource._validate_password('12345678', '12345678')) + + def test_validate_password_failed_by_confirmation(self): + self.assertFalse(self.resource._validate_password('12345678', '1234')) + + def test_validate_password_failed_by_length(self): + self.assertFalse(self.resource._validate_password('1234', '1234')) diff --git a/web-ui/app/locales/en_US/translation.json b/web-ui/app/locales/en_US/translation.json index 160e71ff..6ca72283 100644 --- a/web-ui/app/locales/en_US/translation.json +++ b/web-ui/app/locales/en_US/translation.json @@ -101,7 +101,11 @@ "image-description": "New Password - Step 3 of 4", "title": "Now, create a new password", "input-label1": "create new password", - "input-label2": "confirm your new password" + "input-label2": "confirm your new password", + "error": { + "invalid-password": "A better password has at least 8 characters", + "invalid-confirm-password": "Password and confirmation don't match" + } }, "backup-account-step": { "image-description": "Backup Account - Step 4 of 4", diff --git a/web-ui/app/locales/pt_BR/translation.json b/web-ui/app/locales/pt_BR/translation.json index b7cac507..1baf9b07 100644 --- a/web-ui/app/locales/pt_BR/translation.json +++ b/web-ui/app/locales/pt_BR/translation.json @@ -101,7 +101,11 @@ "image-description": "Nova Senha - Passo 3 de 4", "title": "Agora, crie uma nova senha", "input-label1": "digite a nova senha", - "input-label2": "confirme a nova senha" + "input-label2": "confirme a nova senha", + "error": { + "invalid-password": "Uma senha boa tem pelo menos 8 caracteres", + "invalid-confirm-password": "Senha e confirmação não são iguais" + } }, "backup-account-step": { "image-description": "E-mail de Recuperação - Passo 4 de 4", diff --git a/web-ui/src/account_recovery/new_password_form/new_password_form.js b/web-ui/src/account_recovery/new_password_form/new_password_form.js index e7f689e8..5e1e72c9 100644 --- a/web-ui/src/account_recovery/new_password_form/new_password_form.js +++ b/web-ui/src/account_recovery/new_password_form/new_password_form.js @@ -18,6 +18,7 @@ import 'isomorphic-fetch'; import React from 'react'; import { translate } from 'react-i18next'; +import validator from 'validator'; import { submitForm } from 'src/common/util'; import InputField from 'src/common/input_field/input_field'; @@ -27,22 +28,51 @@ import BackLink from 'src/common/back_link/back_link'; import './new_password_form.scss'; export class NewPasswordForm extends React.Component { + + constructor(props) { + super(props); + this.state = { + submitButtonDisabled: true, + password: '', + errorPassword: '', + confirmPassword: '', + errorConfirmPassword: '' + }; + } + submitHandler = (event) => { event.preventDefault(); submitForm(event, '/account-recovery', { userCode: this.props.userCode, password: this.state.password, - confirmation: this.state.confirmation + confirmPassword: this.state.confirmPassword }).then(() => this.props.next()); } - handlePasswordChange = (event) => { + handleChangePassword = (event) => { this.setState({ password: event.target.value }); - } + this.validatePassword(event.target.value, this.state.confirmPassword); + }; - handlePasswordConfirmationChange = (event) => { - this.setState({ confirmation: event.target.value }); - } + handleChangeConfirmPassword = (event) => { + this.setState({ confirmPassword: event.target.value }); + this.validatePassword(this.state.password, event.target.value); + }; + + validatePassword = (password, confirmPassword) => { + const emptyPassword = validator.isEmpty(password); + const validPassword = validator.isLength(password, { min: 8, max: 9999 }); + const emptyConfirmPassword = validator.isEmpty(confirmPassword); + const validConfirmPassword = confirmPassword === password; + + const t = this.props.t; + + this.setState({ + errorPassword: !emptyPassword && !validPassword ? t('account-recovery.new-password-form.error.invalid-password') : '', + errorConfirmPassword: !emptyConfirmPassword && !validConfirmPassword ? t('account-recovery.new-password-form.error.invalid-confirm-password') : '', + submitButtonDisabled: !validPassword || !validConfirmPassword + }); + }; render() { const { t, previous } = this.props; @@ -55,16 +85,16 @@ export class NewPasswordForm extends React.Component { /> <h1>{t('account-recovery.new-password-form.title')}</h1> <InputField - type='password' name='new-password' + type='password' name='new-password' value={this.state.password} label={t('account-recovery.new-password-form.input-label1')} - onChange={this.handlePasswordChange} + errorText={this.state.errorPassword} onChange={this.handleChangePassword} /> <InputField - type='password' name='confirm-password' + type='password' name='confirm-password' value={this.state.confirmPassword} label={t('account-recovery.new-password-form.input-label2')} - onChange={this.handlePasswordConfirmationChange} + errorText={this.state.errorConfirmPassword} onChange={this.handleChangeConfirmPassword} /> - <SubmitButton buttonText={t('account-recovery.button-next')} /> + <SubmitButton buttonText={t('account-recovery.button-next')} disabled={this.state.submitButtonDisabled} /> <BackLink text={t('account-recovery.back')} onClick={previous} /> </form> ); diff --git a/web-ui/src/account_recovery/new_password_form/new_password_form.spec.js b/web-ui/src/account_recovery/new_password_form/new_password_form.spec.js index b57dd42e..c29487a7 100644 --- a/web-ui/src/account_recovery/new_password_form/new_password_form.spec.js +++ b/web-ui/src/account_recovery/new_password_form/new_password_form.spec.js @@ -65,8 +65,93 @@ describe('NewPasswordForm', () => { expect(fetchMock.lastOptions('/account-recovery').body).toContain('"password":"123"'); }); - it('sends password confirmation as content', () => { - expect(fetchMock.lastOptions('/account-recovery').body).toContain('"confirmation":"456"'); + it('sends confirm password as content', () => { + expect(fetchMock.lastOptions('/account-recovery').body).toContain('"confirmPassword":"456"'); + }); + }); + + describe('Password validation', () => { + let newPasswordFormInstance; + + beforeEach(() => { + newPasswordFormInstance = newPasswordForm.instance(); + }); + + it('verifies initial state', () => { + expect(newPasswordFormInstance.state.errorPassword).toEqual(''); + expect(newPasswordFormInstance.state.errorConfirmPassword).toEqual(''); + expect(newPasswordForm.find('SubmitButton').props().disabled).toBe(true); + }); + + context('with valid fields', () => { + beforeEach(() => { + newPasswordFormInstance.validatePassword('12345678', '12345678'); + }); + + it('does not set error in state', () => { + expect(newPasswordFormInstance.state.errorPassword).toEqual(''); + expect(newPasswordFormInstance.state.errorConfirmPassword).toEqual(''); + }); + + it('enables submit button', () => { + expect(newPasswordForm.find('SubmitButton').props().disabled).toBe(false); + }); + }); + + context('with invalid password', () => { + beforeEach(() => { + newPasswordFormInstance.validatePassword('1234', ''); + }); + + it('sets password error in state', () => { + expect(newPasswordFormInstance.state.errorPassword).toEqual('account-recovery.new-password-form.error.invalid-password'); + }); + + it('disables submit button', () => { + expect(newPasswordForm.find('SubmitButton').props().disabled).toBe(true); + }); + }); + + context('with invalid confirm password', () => { + beforeEach(() => { + newPasswordFormInstance.validatePassword('12345678', '1234'); + }); + + it('sets confirm password error in state', () => { + expect(newPasswordFormInstance.state.errorConfirmPassword).toEqual('account-recovery.new-password-form.error.invalid-confirm-password'); + }); + + it('disables submit button', () => { + expect(newPasswordForm.find('SubmitButton').props().disabled).toBe(true); + }); + }); + + context('with empty fields', () => { + it('does not set error in state if both empty', () => { + newPasswordFormInstance.validatePassword('', ''); + expect(newPasswordFormInstance.state.errorPassword).toEqual(''); + expect(newPasswordFormInstance.state.errorConfirmPassword).toEqual(''); + }); + + it('does not set confirm password error in state if empty', () => { + newPasswordFormInstance.validatePassword('12345678', ''); + expect(newPasswordFormInstance.state.errorConfirmPassword).toEqual(''); + }); + + it('sets confirm password error in state if not empty', () => { + newPasswordFormInstance.validatePassword('', '12345678'); + expect(newPasswordFormInstance.state.errorConfirmPassword).toEqual('account-recovery.new-password-form.error.invalid-confirm-password'); + }); + + it('disables submit button if empty confirm password', () => { + newPasswordFormInstance.validatePassword('12345678', ''); + expect(newPasswordForm.find('SubmitButton').props().disabled).toBe(true); + }); + + it('disables submit button if empty password', () => { + newPasswordFormInstance.validatePassword('', '12345678'); + expect(newPasswordForm.find('SubmitButton').props().disabled).toBe(true); + }); }); it('calls next handler on success', () => { diff --git a/web-ui/test/integration/account_recovery.spec.js b/web-ui/test/integration/account_recovery.spec.js new file mode 100644 index 00000000..708b693f --- /dev/null +++ b/web-ui/test/integration/account_recovery.spec.js @@ -0,0 +1,59 @@ +import { mount } from 'enzyme'; +import expect from 'expect'; +import React from 'react'; +import App from 'src/common/app'; +import AccountRecoveryPage from 'src/account_recovery/page'; +import testI18n from './i18n'; + +describe('Account Recovery Page', () => { + context('New password validation', () => { + let app; + let accountRecoveryPage; + + beforeEach(() => { + app = mount(<App i18n={testI18n} child={<AccountRecoveryPage />} />); + accountRecoveryPage = app.find('Page'); + accountRecoveryPage.find('form').simulate('submit'); + accountRecoveryPage.find('form').simulate('submit'); + }); + + it('shows no validation error with valid password', () => { + accountRecoveryPage.find('input[name="new-password"]').simulate('change', { target: { value: '12345678' } }); + // workaround because of an enzyme bug https://github.com/airbnb/enzyme/issues/534 + const inputField = accountRecoveryPage.findWhere(element => element.props().name === 'new-password').find('InputField'); + expect(inputField.props().errorText).toEqual(''); + }); + + it('shows validation error with invalid password', () => { + accountRecoveryPage.find('input[name="new-password"]').simulate('change', { target: { value: '1234' } }); + const inputField = accountRecoveryPage.findWhere(element => element.props().name === 'new-password').find('InputField'); + expect(inputField.props().errorText).toEqual('A better password has at least 8 characters'); + }); + + it('shows no validation error with valid confirm password', () => { + accountRecoveryPage.find('input[name="new-password"]').simulate('change', { target: { value: '12345678' } }); + accountRecoveryPage.find('input[name="confirm-password"]').simulate('change', { target: { value: '12345678' } }); + const inputField = accountRecoveryPage.findWhere(element => element.props().name === 'confirm-password').find('InputField'); + expect(inputField.props().errorText).toEqual(''); + }); + + it('shows validation error with invalid confirm password', () => { + accountRecoveryPage.find('input[name="new-password"]').simulate('change', { target: { value: '12345678' } }); + accountRecoveryPage.find('input[name="confirm-password"]').simulate('change', { target: { value: '1234' } }); + const inputField = accountRecoveryPage.findWhere(element => element.props().name === 'confirm-password').find('InputField'); + expect(inputField.props().errorText).toEqual('Password and confirmation don\'t match'); + }); + + it('disables button if empty fields', () => { + accountRecoveryPage.find('input[name="new-password"]').simulate('change', { target: { value: '' } }); + accountRecoveryPage.find('input[name="confirm-password"]').simulate('change', { target: { value: '' } }); + expect(accountRecoveryPage.find('SubmitButton').props().disabled).toEqual(true); + }); + + it('enables button if valid fields', () => { + accountRecoveryPage.find('input[name="new-password"]').simulate('change', { target: { value: '12345678' } }); + accountRecoveryPage.find('input[name="confirm-password"]').simulate('change', { target: { value: '12345678' } }); + expect(accountRecoveryPage.find('SubmitButton').props().disabled).toEqual(false); + }); + }); +}); |