mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-01-20 22:42:53 -05:00
🐛 Fixed invalid email getting saved for members (#16021)
refs https://github.com/TryGhost/Team/issues/2235 We found some cases which can cause a site to have member emails that have invalid characters like `member@example.com�`. This happened due to the `validator` version used by Ghost not able to catch some specific cases as invalid email, allowing members to be created with them either via Admin or Importer or direct signup. Portal UI already blocked these email as invalid. This change: - updates `@tryghost/validator` to include a latest version of email validator that catches these invalid cases - doesn't allow member creation with invalid email like above - doesn't allow existing member emails to be edited to invalid
This commit is contained in:
Rishabh Garg
GitHub
parent
6a266be239
commit
2eac41b1f4
6 changed files with 111 additions and 4 deletions
|
|
@ -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<6F>';
|
||||
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<6F>';
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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', {
|
||||
|
|
|
|||
|
|
@ -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<6F>,"good name",,true,false,,2019-10-30T14:52:08.000Z,more-labels
|
||||
|
|
|
@ -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 = [];
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
27
yarn.lock
27
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"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue