diff --git a/ghost/core/test/e2e-browser/admin/members.spec.js b/ghost/core/test/e2e-browser/admin/members.spec.js index 7fa400fbcc..4623f1b9c6 100644 --- a/ghost/core/test/e2e-browser/admin/members.spec.js +++ b/ghost/core/test/e2e-browser/admin/members.spec.js @@ -33,6 +33,30 @@ test.describe('Admin', () => { await expect(memberEmail).toHaveText(email); }); + test('A member cannot be created with invalid email', async ({page}) => { + await page.goto('/ghost'); + await page.locator('.gh-nav a[href="#/members/"]').click(); + await page.waitForSelector('a[href="#/members/new/"] span'); + await page.locator('a[href="#/members/new/"] span:has-text("New member")').click(); + await page.waitForSelector('input[name="name"]'); + let name = 'Test Member'; + let email = 'tester+invalid@testmember.com�'; + let note = 'This is a test member'; + let label = 'Test Label'; + await page.fill('input[name="name"]', name); + await page.fill('input[name="email"]', email); + await page.fill('textarea[name="note"]', note); + await page.locator('label:has-text("Labels") + div').click(); + await page.keyboard.type(label); + await page.keyboard.press('Tab'); + await page.locator('button span:has-text("Save")').click(); + await page.waitForSelector('button span:has-text("Retry")'); + + // check we are unable to save member with invalid email + await expect(page.locator('button span:has-text("Retry")')).toBeVisible(); + await expect(page.locator('text=Invalid Email')).toBeVisible(); + }); + test('A member can be edited', async ({page}) => { await page.goto('/ghost'); await page.locator('.gh-nav a[href="#/members/"]').click(); @@ -269,5 +293,36 @@ test.describe('Admin', () => { // check for content CTA and expect it to be zero await expect(page.locator('.gh-post-upgrade-cta')).toHaveCount(0); }); + + test('An existing member cannot be saved with invalid email address', async ({page}) => { + await page.goto('/ghost'); + await page.locator('.gh-nav a[href="#/members/"]').click(); + await page.waitForSelector('a[href="#/members/new/"] span'); + await page.locator('a[href="#/members/new/"] span:has-text("New member")').click(); + await page.waitForSelector('input[name="name"]'); + let name = 'Test Member'; + let email = 'tester+invalid@example.com'; + let note = 'This is a test member'; + let label = 'Test Label'; + await page.fill('input[name="name"]', name); + await page.fill('input[name="email"]', email); + await page.fill('textarea[name="note"]', note); + await page.locator('label:has-text("Labels") + div').click(); + await page.keyboard.type(label); + await page.keyboard.press('Tab'); + await page.locator('button span:has-text("Save")').click(); + await page.waitForSelector('button span:has-text("Saved")'); + + // Update email to invalid and try saving + let updatedEmail = 'tester+invalid@example.com�'; + await page.fill('input[name="email"]', updatedEmail); + await page.waitForSelector('button span:has-text("Save")'); + await page.locator('button span:has-text("Save")').click(); + await page.waitForSelector('button span:has-text("Retry")'); + + // check we are unable to save member with invalid email + await expect(page.locator('button span:has-text("Retry")')).toBeVisible(); + await expect(page.locator('text=Invalid Email')).toBeVisible(); + }); }); }); diff --git a/ghost/core/test/regression/api/admin/members-importer.test.js b/ghost/core/test/regression/api/admin/members-importer.test.js index 382382da89..3541e17704 100644 --- a/ghost/core/test/regression/api/admin/members-importer.test.js +++ b/ghost/core/test/regression/api/admin/members-importer.test.js @@ -234,15 +234,16 @@ describe('Members Importer API', function () { should.exist(jsonResponse.meta.stats); jsonResponse.meta.stats.imported.should.equal(1); - jsonResponse.meta.stats.invalid.length.should.equal(1); + jsonResponse.meta.stats.invalid.length.should.equal(2); - jsonResponse.meta.stats.invalid[0].error.should.match(/Validation \(isEmail\) failed for email/); + jsonResponse.meta.stats.invalid[0].error.should.match(/Invalid Email/); + jsonResponse.meta.stats.invalid[1].error.should.match(/Invalid Email/); should.exist(jsonResponse.meta.import_label); jsonResponse.meta.import_label.slug.should.match(/^import-/); }); }); - + it('Can import members with host emailVerification limits', async function () { // If this test fails, check if the total members that have been created with fixtures has increased a lot, and if required, increase the amount of imported members configUtils.set('hostSettings:emailVerification', { diff --git a/ghost/core/test/utils/fixtures/csv/members-invalid-values.csv b/ghost/core/test/utils/fixtures/csv/members-invalid-values.csv index 1b987b03dd..19613df891 100644 --- a/ghost/core/test/utils/fixtures/csv/members-invalid-values.csv +++ b/ghost/core/test/utils/fixtures/csv/members-invalid-values.csv @@ -1,3 +1,4 @@ email,name,note,subscribed_to_emails,complimentary_plan,stripe_customer_id,created_at,labels valid2@email.com,"good name",,true,not_boolean,,2019-10-30T14:52:08.000Z,more-labels invalid_email_value,"good name",,true,false,,2019-10-30T14:52:08.000Z,more-labels +invalid2@email.com�,"good name",,true,false,,2019-10-30T14:52:08.000Z,more-labels \ No newline at end of file diff --git a/ghost/members-api/lib/repositories/member.js b/ghost/members-api/lib/repositories/member.js index 862146bb25..8156f6a61b 100644 --- a/ghost/members-api/lib/repositories/member.js +++ b/ghost/members-api/lib/repositories/member.js @@ -6,6 +6,7 @@ const DomainEvents = require('@tryghost/domain-events'); const {MemberCreatedEvent, SubscriptionCreatedEvent, MemberSubscribeEvent, SubscriptionCancelledEvent} = require('@tryghost/member-events'); const ObjectId = require('bson-objectid').default; const {NotFoundError} = require('@tryghost/errors'); +const validator = require('@tryghost/validator'); const messages = { noStripeConnection: 'Cannot {action} without a Stripe Connection', @@ -16,7 +17,8 @@ const messages = { subscriptionNotFound: 'Could not find Subscription {id}', productNotFound: 'Could not find Product {id}', bulkActionRequiresFilter: 'Cannot perform {action} without a filter or all=true', - tierArchived: 'Cannot use archived Tiers' + tierArchived: 'Cannot use archived Tiers', + invalidEmail: 'Invalid Email' }; /** @@ -247,6 +249,14 @@ module.exports = class MemberRepository { const memberData = _.pick(data, ['email', 'name', 'note', 'subscribed', 'geolocation', 'created_at', 'products', 'newsletters']); + // Throw error if email is invalid using latest validator + if (!validator.isEmail(memberData.email, {legacy: false})) { + throw new errors.ValidationError({ + message: tpl(messages.invalidEmail), + property: 'email' + }); + } + if (memberData.products && memberData.products.length > 1) { throw new errors.BadRequestError({message: tpl(messages.moreThanOneProduct)}); } @@ -439,6 +449,18 @@ module.exports = class MemberRepository { } } + // Throw error if email is invalid and it's been changed + if ( + initialMember?.get('email') && memberData.email + && initialMember.get('email') !== memberData.email + && !validator.isEmail(memberData.email, {legacy: false}) + ) { + throw new errors.ValidationError({ + message: tpl(messages.invalidEmail), + property: 'email' + }); + } + const memberStatusData = {}; let productsToAdd = []; diff --git a/ghost/members-api/package.json b/ghost/members-api/package.json index eee064e4a2..35ac93ef55 100644 --- a/ghost/members-api/package.json +++ b/ghost/members-api/package.json @@ -36,6 +36,7 @@ "@tryghost/members-payments": "0.0.0", "@tryghost/nql": "0.11.0", "@tryghost/tpl": "0.1.20", + "@tryghost/validator": "^0.2.0", "@types/jsonwebtoken": "8.5.9", "body-parser": "1.20.1", "bson-objectid": "2.0.4", diff --git a/yarn.lock b/yarn.lock index 4cb5abe2f9..e4dac8f84a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4389,6 +4389,15 @@ lodash "^4.17.21" uuid "^9.0.0" +"@tryghost/errors@^1.2.20": + version "1.2.20" + resolved "https://registry.yarnpkg.com/@tryghost/errors/-/errors-1.2.20.tgz#bd5c96bb815f7b49cacee1bd0ce235c3a5f25e82" + integrity sha512-9QGLj3jrslEo4BEByMn/vT65d4fUHSz22p0R5ixiYVtHsuXjdKJm0oDBt8wtKXxdgE/h3CCFI709TW6hLyk+7w== + dependencies: + "@stdlib/utils" "^0.0.12" + lodash "^4.17.21" + uuid "^9.0.0" + "@tryghost/express-test@0.11.7": version "0.11.7" resolved "https://registry.yarnpkg.com/@tryghost/express-test/-/express-test-0.11.7.tgz#586cc0537a944f4e908736dff46617d4b391c52d" @@ -4720,6 +4729,13 @@ dependencies: lodash.template "^4.5.0" +"@tryghost/tpl@^0.1.21": + version "0.1.21" + resolved "https://registry.yarnpkg.com/@tryghost/tpl/-/tpl-0.1.21.tgz#60ba975d0acc7af6911e7b021b2235d6338ab6c7" + integrity sha512-ED0SR09+v7SglL+JOD1+q7QaZU4cRa+XemymQomob8hc9PNqE8Gzw0Rd4fqtMD9v91OHvg5rS/MrKvUHn6eTfw== + dependencies: + lodash.template "^4.5.0" + "@tryghost/url-utils@4.3.0", "@tryghost/url-utils@^4.0.0": version "4.3.0" resolved "https://registry.yarnpkg.com/@tryghost/url-utils/-/url-utils-4.3.0.tgz#f276ea0963f34547dca8f8f8a6c7bd4e6d4bb6e3" @@ -4743,6 +4759,17 @@ moment-timezone "^0.5.23" validator "7.2.0" +"@tryghost/validator@^0.2.0": + version "0.2.0" + resolved "https://registry.yarnpkg.com/@tryghost/validator/-/validator-0.2.0.tgz#cfb0b9447cfb50901b2a2fbf8519de4d5b992f12" + integrity sha512-sKAcyZwOCdCe7jG6B1UxzOijHjvwqwj9G9l+hQhRScT1gMT4C8zhyq7BrEQmFUvsLUXVBlpph5wn95E34oqCDw== + dependencies: + "@tryghost/errors" "^1.2.20" + "@tryghost/tpl" "^0.1.21" + lodash "^4.17.21" + moment-timezone "^0.5.23" + validator "7.2.0" + "@tryghost/version@0.1.18", "@tryghost/version@^0.1.18": version "0.1.18" resolved "https://registry.yarnpkg.com/@tryghost/version/-/version-0.1.18.tgz#ecb10e60868503b4152124d1cebaf8d94a3a2703"