From e116d529928306ca2cc851daa9e8fc81f352df19 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Fri, 27 Sep 2019 01:12:01 +0200 Subject: [PATCH] Use EmailAddressRequest and validate email --- backend/src/models/EmailAddressRequest.js | 12 +++++ backend/src/models/index.js | 1 + backend/src/schema/resolvers/emails.js | 18 +++++++- backend/src/schema/resolvers/emails.spec.js | 46 +++++++++++++------ .../seed/factories/emailAddressRequests.js | 10 ++++ backend/src/seed/factories/emailAddresses.js | 21 +++++---- backend/src/seed/factories/index.js | 2 + 7 files changed, 87 insertions(+), 23 deletions(-) create mode 100644 backend/src/models/EmailAddressRequest.js create mode 100644 backend/src/seed/factories/emailAddressRequests.js diff --git a/backend/src/models/EmailAddressRequest.js b/backend/src/models/EmailAddressRequest.js new file mode 100644 index 000000000..7b37b9a39 --- /dev/null +++ b/backend/src/models/EmailAddressRequest.js @@ -0,0 +1,12 @@ +module.exports = { + email: { type: 'string', primary: true, lowercase: true, email: true }, + createdAt: { type: 'string', isoDate: true, default: () => new Date().toISOString() }, + nonce: { type: 'string', token: true }, + belongsTo: { + type: 'relationship', + relationship: 'BELONGS_TO', + target: 'User', + direction: 'out', + eager: true, + }, +} diff --git a/backend/src/models/index.js b/backend/src/models/index.js index a7d3c8252..d334f460a 100644 --- a/backend/src/models/index.js +++ b/backend/src/models/index.js @@ -5,6 +5,7 @@ export default { User: require('./User.js'), InvitationCode: require('./InvitationCode.js'), EmailAddress: require('./EmailAddress.js'), + EmailAddressRequest: require('./EmailAddressRequest.js'), SocialMedia: require('./SocialMedia.js'), Post: require('./Post.js'), Comment: require('./Comment.js'), diff --git a/backend/src/schema/resolvers/emails.js b/backend/src/schema/resolvers/emails.js index d2f76ba39..2c6296627 100644 --- a/backend/src/schema/resolvers/emails.js +++ b/backend/src/schema/resolvers/emails.js @@ -2,10 +2,18 @@ import generateNonce from './helpers/generateNonce' import Resolver from './helpers/Resolver' import existingEmailAddress from './helpers/existingEmailAddress' import { UserInputError } from 'apollo-server' +import Validator from 'neode/build/Services/Validator.js' export default { Mutation: { AddEmailAddress: async (_parent, args, context, _resolveInfo) => { + try { + const { neode } = context + await new Validator(neode, neode.model('EmailAddressRequest'), args) + } catch (e) { + throw new UserInputError('must be a valid email') + } + let response = await existingEmailAddress(_parent, args, context) if (response) return response @@ -19,7 +27,7 @@ export default { const result = await txc.run( ` MATCH (user:User {id: $userId}) - MERGE (user)<-[:BELONGS_TO]-(email:EmailAddress {email: $email, nonce: $nonce}) + MERGE (user)<-[:BELONGS_TO]-(email:EmailAddressRequest {email: $email, nonce: $nonce}) SET email.createdAt = toString(datetime()) RETURN email `, @@ -46,9 +54,11 @@ export default { const result = await txc.run( ` MATCH (user:User {id: $userId})-[previous:PRIMARY_EMAIL]->(:EmailAddress) - MATCH (user)<-[:BELONGS_TO]-(email:EmailAddress {email: $email, nonce: $nonce}) + MATCH (user)<-[:BELONGS_TO]-(email:EmailAddressRequest {email: $email, nonce: $nonce}) MERGE (user)-[:PRIMARY_EMAIL]->(email) + SET email:EmailAddress SET email.verifiedAt = toString(datetime()) + REMOVE email:EmailAddressRequest DELETE previous RETURN email `, @@ -59,6 +69,10 @@ export default { try { const txResult = await writeTxResultPromise response = txResult[0] + } catch (e) { + if (e.code === 'Neo.ClientError.Schema.ConstraintValidationFailed') + throw new UserInputError('A user account with this email already exists.') + throw new Error(e) } finally { session.close() } diff --git a/backend/src/schema/resolvers/emails.spec.js b/backend/src/schema/resolvers/emails.spec.js index 6ec66ca65..f6058e19e 100644 --- a/backend/src/schema/resolvers/emails.spec.js +++ b/backend/src/schema/resolvers/emails.spec.js @@ -68,7 +68,16 @@ describe('AddEmailAddress', () => { }) describe('email attribute is not a valid email', () => { - it.todo('throws UserInputError') + beforeEach(() => { + variables = { ...variables, email: 'foobar' } + }) + + it('throws UserInputError', async () => { + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { AddEmailAddress: null }, + errors: [{ message: 'must be a valid email' }], + }) + }) }) describe('email attribute is a valid email', () => { @@ -85,24 +94,23 @@ describe('AddEmailAddress', () => { }) }) - it('connects `EmailAddress` to the authenticated user', async () => { + it('connects `EmailAddressRequest` to the authenticated user', async () => { await mutate({ mutation, variables }) const result = await neode.cypher(` MATCH(u:User)-[:PRIMARY_EMAIL]->(:EmailAddress {email: "user@example.org"}) - MATCH(u:User)<-[:BELONGS_TO]-(e:EmailAddress {email: "new-email@example.org"}) + MATCH(u:User)<-[:BELONGS_TO]-(e:EmailAddressRequest {email: "new-email@example.org"}) RETURN e `) - const email = neode.hydrateFirst(result, 'e', neode.model('EmailAddress')) + const email = neode.hydrateFirst(result, 'e', neode.model('EmailAddressRequest')) await expect(email.toJson()).resolves.toMatchObject({ email: 'new-email@example.org', nonce: expect.any(String), }) }) - describe('if a lone `EmailAddress` node already exists with that email', () => { - it('returns this `EmailAddress` node', async () => { - await factory.create('EmailAddress', { - verifiedAt: null, + describe('if another `EmailAddressRequest` node already exists with that email', () => { + it('throws no unique constraint violation error', async () => { + await factory.create('EmailAddressRequest', { createdAt: '2019-09-24T14:00:01.565Z', email: 'new-email@example.org', }) @@ -111,7 +119,6 @@ describe('AddEmailAddress', () => { AddEmailAddress: { email: 'new-email@example.org', verifiedAt: null, - createdAt: '2019-09-24T14:00:01.565Z', // this is to make sure it's the one above }, }, errors: undefined, @@ -175,10 +182,10 @@ describe('VerifyEmailAddress', () => { }) }) - describe('given an unverified `EmailAddress`', () => { + describe('given a `EmailAddressRequest`', () => { let emailAddress beforeEach(async () => { - emailAddress = await factory.create('EmailAddress', { + emailAddress = await factory.create('EmailAddressRequest', { nonce: 'abcdef', verifiedAt: null, createdAt: new Date().toISOString(), @@ -196,7 +203,7 @@ describe('VerifyEmailAddress', () => { }) }) - describe('given valid nonce for unverified `EmailAddress` node', () => { + describe('given valid nonce for `EmailAddressRequest` node', () => { beforeEach(() => { variables = { ...variables, nonce: 'abcdef' } }) @@ -210,7 +217,7 @@ describe('VerifyEmailAddress', () => { }) }) - describe('and the `EmailAddress` belongs to the authenticated user', () => { + describe('and the `EmailAddressRequest` belongs to the authenticated user', () => { beforeEach(async () => { await emailAddress.relateTo(user, 'belongsTo') }) @@ -256,6 +263,19 @@ describe('VerifyEmailAddress', () => { email = neode.hydrateFirst(result, 'e', neode.model('EmailAddress')) await expect(email).toBe(false) }) + + describe('Edge case: In the meantime someone created an `EmailAddress` node with the given email', () => { + beforeEach(async () => { + await factory.create('EmailAddress', { email: 'to-be-verified@example.org' }) + }) + + it('throws UserInputError because of unique constraints', async () => { + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { VerifyEmailAddress: null }, + errors: [{ message: 'A user account with this email already exists.' }], + }) + }) + }) }) }) }) diff --git a/backend/src/seed/factories/emailAddressRequests.js b/backend/src/seed/factories/emailAddressRequests.js new file mode 100644 index 000000000..242be6375 --- /dev/null +++ b/backend/src/seed/factories/emailAddressRequests.js @@ -0,0 +1,10 @@ +import { defaults } from './emailAddresses.js' + +export default function create() { + return { + factory: async ({ args, neodeInstance }) => { + args = defaults({ args }) + return neodeInstance.create('EmailAddressRequest', args) + }, + } +} diff --git a/backend/src/seed/factories/emailAddresses.js b/backend/src/seed/factories/emailAddresses.js index 0212dca53..41b1fe96c 100644 --- a/backend/src/seed/factories/emailAddresses.js +++ b/backend/src/seed/factories/emailAddresses.js @@ -1,16 +1,21 @@ import faker from 'faker' +export function defaults({ args }) { + const defaults = { + email: faker.internet.email(), + verifiedAt: new Date().toISOString(), + } + args = { + ...defaults, + ...args, + } + return args +} + export default function create() { return { factory: async ({ args, neodeInstance }) => { - const defaults = { - email: faker.internet.email(), - verifiedAt: new Date().toISOString(), - } - args = { - ...defaults, - ...args, - } + args = defaults({ args }) return neodeInstance.create('EmailAddress', args) }, } diff --git a/backend/src/seed/factories/index.js b/backend/src/seed/factories/index.js index ab09b438d..acfaad2d7 100644 --- a/backend/src/seed/factories/index.js +++ b/backend/src/seed/factories/index.js @@ -9,6 +9,7 @@ import createTag from './tags.js' import createSocialMedia from './socialMedia.js' import createLocation from './locations.js' import createEmailAddress from './emailAddresses.js' +import createEmailAddressRequests from './emailAddressRequests.js' export const seedServerHost = 'http://127.0.0.1:4001' @@ -32,6 +33,7 @@ const factories = { SocialMedia: createSocialMedia, Location: createLocation, EmailAddress: createEmailAddress, + EmailAddressRequest: createEmailAddressRequests, } export const cleanDatabase = async (options = {}) => {