From 020231bb02313bc3f3fe575da57809edb9452a33 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Sun, 25 Aug 2019 11:30:24 -0400 Subject: [PATCH 1/6] add local, remote email validation to validate library --- src/lib/validate.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/lib/validate.js b/src/lib/validate.js index b4fd84f31..c4deee007 100644 --- a/src/lib/validate.js +++ b/src/lib/validate.js @@ -1,5 +1,6 @@ module.exports = {}; const api = require('./api'); +const emailValidator = require('email-validator'); module.exports.validateUsernameLocally = username => { if (!username || username === '') { @@ -67,3 +68,37 @@ module.exports.validatePasswordConfirm = (password, passwordConfirm) => { } return {valid: true}; }; + +module.exports.validateEmailLocally = email => { + if (!email || email === '') { + return {valid: false, errMsgId: 'general.required'}; + } else if (emailValidator.validate(email)) { + return {valid: true}; + } + return ({valid: false, errMsgId: 'registration.validationEmailInvalid'}); +}; + +module.exports.validateEmailRemotely = email => ( + new Promise(resolve => { + api({ + uri: `/accounts/check_email/?email=${email}`, + host: '' // Not handled by the API, use existing infrastructure + }, (err, body, res) => { + if (err || res.statusCode !== 200 || !body || body.length < 1 || !body[0].msg) { + resolve({valid: false, errMsgId: 'general.apiError'}); + } + switch (body[0].msg) { + case 'valid email': + resolve({valid: true}); + break; + case 'Scratch is not allowed to send email to this address.': // e.g., bad TLD or block-listed + resolve({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + break; + case 'Enter a valid email address.': + default: + resolve({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + break; + } + }); + }) +); From d6e5637dbe1c6cc05a6b680ef0a9d896cfab1372 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Sun, 25 Aug 2019 11:31:07 -0400 Subject: [PATCH 2/6] in email step, use remote validation --- src/components/join-flow/email-step.jsx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 88a24c97a..54105a107 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -4,9 +4,9 @@ const React = require('react'); const PropTypes = require('prop-types'); import {Formik} from 'formik'; const {injectIntl, intlShape} = require('react-intl'); -const emailValidator = require('email-validator'); const FormattedMessage = require('react-intl').FormattedMessage; +const validate = require('../../lib/validate'); const JoinFlowStep = require('./join-flow-step.jsx'); const FormikInput = require('../../components/formik-forms/formik-input.jsx'); const FormikCheckbox = require('../../components/formik-forms/formik-checkbox.jsx'); @@ -75,11 +75,16 @@ class EmailStep extends React.Component { } validateEmail (email) { if (!email) return this.props.intl.formatMessage({id: 'general.required'}); - const isValidLocally = emailValidator.validate(email); - if (isValidLocally) { - return null; // TODO: validate email address remotely - } - return this.props.intl.formatMessage({id: 'registration.validationEmailInvalid'}); + const localResult = validate.validateEmailLocally(email); + if (!localResult.valid) return this.props.intl.formatMessage({id: localResult.errMsgId}); + return validate.validateEmailRemotely(email).then( + remoteResult => { + if (remoteResult.valid === true) { + return null; + } + return this.props.intl.formatMessage({id: remoteResult.errMsgId}); + } + ); } validateForm () { return {}; From fa91a57b355a0bc27f9915ef395d2c7d428ea8c6 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Sun, 25 Aug 2019 11:31:29 -0400 Subject: [PATCH 3/6] show email validation error until you make a change --- src/components/join-flow/email-step.jsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 54105a107..5e42edeac 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -124,6 +124,8 @@ class EmailStep extends React.Component { handleSubmit, isSubmitting, setFieldError, + setFieldTouched, + setFieldValue, validateField } = props; return ( @@ -166,7 +168,11 @@ class EmailStep extends React.Component { validationClassName="validation-full-width-input" /* eslint-disable react/jsx-no-bind */ onBlur={() => validateField('email')} - onFocus={() => setFieldError('email', null)} + onChange={e => { + setFieldValue('email', e.target.value); + setFieldTouched('email'); + setFieldError('email', null); + }} /* eslint-enable react/jsx-no-bind */ onSetRef={this.handleSetEmailRef} /> From ee8cdac7481f6c67b4cce2f97a95db51ec6789ad Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 27 Aug 2019 12:18:53 -0400 Subject: [PATCH 4/6] add tests for local email validation --- test/unit/lib/validate.test.js | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/unit/lib/validate.test.js b/test/unit/lib/validate.test.js index 03922882f..a051e1faa 100644 --- a/test/unit/lib/validate.test.js +++ b/test/unit/lib/validate.test.js @@ -63,4 +63,52 @@ describe('unit test lib/validate.js', () => { response = validate.validatePasswordConfirm('', 'abcdefg'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationPasswordConfirmNotEquals'}); }); + + test('validate email address locally', () => { + let response; + expect(typeof validate.validateEmailLocally).toBe('function'); + + // permitted addresses: + response = validate.validateEmailLocally('abc@def.com'); + expect(response).toEqual({valid: true}); + response = validate.validateEmailLocally('abcdefghijklmnopqrst@abcdefghijklmnopqrst.info'); + expect(response).toEqual({valid: true}); + response = validate.validateEmailLocally('abc-def-ghi@jkl-mno.org'); + expect(response).toEqual({valid: true}); + response = validate.validateEmailLocally('_______@example.com'); + expect(response).toEqual({valid: true}); + response = validate.validateEmailLocally('email@example.museum'); + expect(response).toEqual({valid: true}); + response = validate.validateEmailLocally('email@example.co.jp'); + expect(response).toEqual({valid: true}); + + // non-permitted addresses: + response = validate.validateEmailLocally(''); + expect(response).toEqual({valid: false, errMsgId: 'general.required'}); + response = validate.validateEmailLocally('a'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('abc@def'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('abc@def.c'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('abcπŸ˜„def@emoji.pizza'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('γ‚γ„γ†γˆγŠ@example.com'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('Abc..123@example.com'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('Joe Smith '); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('email@example@example.com'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('email@example..com'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + + // edge cases: + // these are strictly legal according to email addres spec, but rejected by library we use: + response = validate.validateEmailLocally('email@123.123.123.123'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + response = validate.validateEmailLocally('much."more unusual"@example.com'); + expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); + }); }); From 55cb112ee995e1d17a1d872d152931239935af7a Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 27 Aug 2019 12:26:56 -0400 Subject: [PATCH 5/6] use params prop instead of inline query params --- src/lib/validate.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/validate.js b/src/lib/validate.js index c4deee007..b2ce9ab5a 100644 --- a/src/lib/validate.js +++ b/src/lib/validate.js @@ -81,8 +81,9 @@ module.exports.validateEmailLocally = email => { module.exports.validateEmailRemotely = email => ( new Promise(resolve => { api({ - uri: `/accounts/check_email/?email=${email}`, - host: '' // Not handled by the API, use existing infrastructure + host: '', // not handled by API; use existing infrastructure + params: {email: email}, + uri: '/accounts/check_email/' }, (err, body, res) => { if (err || res.statusCode !== 200 || !body || body.length < 1 || !body[0].msg) { resolve({valid: false, errMsgId: 'general.apiError'}); From 46351c116dff2adf224a1d832b93290e74625ffb Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 28 Aug 2019 10:35:39 -0400 Subject: [PATCH 6/6] collapse switch cases --- src/lib/validate.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/validate.js b/src/lib/validate.js index b2ce9ab5a..5df20b0e4 100644 --- a/src/lib/validate.js +++ b/src/lib/validate.js @@ -93,8 +93,6 @@ module.exports.validateEmailRemotely = email => ( resolve({valid: true}); break; case 'Scratch is not allowed to send email to this address.': // e.g., bad TLD or block-listed - resolve({valid: false, errMsgId: 'registration.validationEmailInvalid'}); - break; case 'Enter a valid email address.': default: resolve({valid: false, errMsgId: 'registration.validationEmailInvalid'});