Handle edge case

It might be that people try to register email addresses that they don't own. Then if the actual owner tries to add this email address, she should not get a unique constraint violation. Instead the email will be re-used.

Is this a security issue? Because we re-use the nonce? 🤔

backend/src/schema/resolvers/emails.js

@@ -1,10 +1,13 @@
-import generateNonce from '../../helpers/generateNonce'
+import generateNonce from './helpers/generateNonce'
 import Resolver from './helpers/Resolver'
+import existingEmailAddress from './helpers/existingEmailAddress'
 
 export default {
   Mutation: {
     AddEmailAddress: async (_parent, args, context, _resolveInfo) => {
-      let response
+      let response = await existingEmailAddress(_parent, args, context)
+      if (response) return response
+
       const nonce = generateNonce()
       const {
         user: { id: userId },

backend/src/schema/resolvers/emails.spec.js

@@ -95,11 +95,33 @@ describe('AddEmailAddress', () => {
     })
 
     describe('if a lone `EmailAddress` node already exists with that email', () => {
-      it.todo('returns this `EmailAddress` node')
+      it('returns this `EmailAddress` node', async () => {
+        await factory.create('EmailAddress', {
+          verifiedAt: null,
+          createdAt: '2019-09-24T14:00:01.565Z',
+          email: 'new-email@example.org',
+        })
+        await expect(mutate({ mutation, variables })).resolves.toMatchObject({
+          data: {
+            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,
+        })
+      })
     })
 
     describe('but if another user owns an `EmailAddress` already with that email', () => {
-      it.todo('throws UserInputError because of unique constraints')
+      it('throws UserInputError because of unique constraints', async () => {
+        await factory.create('User', { email: 'new-email@example.org' })
+        await expect(mutate({ mutation, variables })).resolves.toMatchObject({
+          data: { AddEmailAddress: null },
+          errors: [{ message: 'A user account with this email already exists.' }],
+        })
+      })
     })
   })
 })

backend/src/schema/resolvers/helpers/existingEmailAddress.js

@@ -0,0 +1,26 @@
+import { UserInputError } from 'apollo-server'
+export default async function alreadyExistingMail(_parent, args, context) {
+  let { email } = args
+  email = email.toLowerCase()
+  const cypher = `
+    MATCH (email:EmailAddress {email: $email})
+    OPTIONAL MATCH (email)-[:PRIMARY_EMAIL]-(user)
+    RETURN email, user
+  `
+  let transactionRes
+  const session = context.driver.session()
+  try {
+    transactionRes = await session.run(cypher, { email })
+  } finally {
+    session.close()
+  }
+  const [result] = transactionRes.records.map(record => {
+    return {
+      alreadyExistingEmail: record.get('email').properties,
+      user: record.get('user') && record.get('user').properties,
+    }
+  })
+  const { alreadyExistingEmail, user } = result || {}
+  if (user) throw new UserInputError('A user account with this email already exists.')
+  return alreadyExistingEmail
+}

backend/src/schema/resolvers/registration.js

@@ -1,41 +1,16 @@
 import { UserInputError } from 'apollo-server'
-import uuid from 'uuid/v4'
 import { neode } from '../../bootstrap/neo4j'
 import fileUpload from './fileUpload'
 import encryptPassword from '../../helpers/encryptPassword'
+import generateNonce from './helpers/generateNonce'
+import existingEmailAddress from './helpers/existingEmailAddress'
 
 const instance = neode()
 
-const alreadyExistingMail = async (_parent, args, context) => {
-  let { email } = args
-  email = email.toLowerCase()
-  const cypher = `
-    MATCH (email:EmailAddress {email: $email})
-    OPTIONAL MATCH (email)-[:PRIMARY_EMAIL]-(user)
-    RETURN email, user
-  `
-  let transactionRes
-  const session = context.driver.session()
-  try {
-    transactionRes = await session.run(cypher, { email })
-  } finally {
-    session.close()
-  }
-  const [result] = transactionRes.records.map(record => {
-    return {
-      alreadyExistingEmail: record.get('email').properties,
-      user: record.get('user') && record.get('user').properties,
-    }
-  })
-  const { alreadyExistingEmail, user } = result || {}
-  if (user) throw new UserInputError('User account with this email already exists.')
-  return alreadyExistingEmail
-}
-
 export default {
   Mutation: {
     CreateInvitationCode: async (_parent, args, context, _resolveInfo) => {
-      args.token = uuid().substring(0, 6)
+      args.token = generateNonce()
       const {
         user: { id: userId },
       } = context
@@ -54,9 +29,9 @@ export default {
       return response
     },
     Signup: async (_parent, args, context) => {
-      const nonce = uuid().substring(0, 6)
+      const nonce = generateNonce()
       args.nonce = nonce
-      let emailAddress = await alreadyExistingMail(_parent, args, context)
+      let emailAddress = await existingEmailAddress(_parent, args, context)
       if (emailAddress) return emailAddress
       try {
         emailAddress = await instance.create('EmailAddress', args)
@@ -67,9 +42,9 @@ export default {
     },
     SignupByInvitation: async (_parent, args, context) => {
       const { token } = args
-      const nonce = uuid().substring(0, 6)
+      const nonce = generateNonce()
       args.nonce = nonce
-      let emailAddress = await alreadyExistingMail(_parent, args, context)
+      let emailAddress = await existingEmailAddress(_parent, args, context)
       if (emailAddress) return emailAddress
       try {
         const result = await instance.cypher(

backend/src/schema/resolvers/registration.spec.js

@@ -257,7 +257,7 @@ describe('SignupByInvitation', () => {
 
             it('throws unique violation error', async () => {
               await expect(mutate({ mutation, variables })).resolves.toMatchObject({
-                errors: [{ message: 'User account with this email already exists.' }],
+                errors: [{ message: 'A user account with this email already exists.' }],
               })
             })
           })
@@ -307,6 +307,7 @@ describe('Signup', () => {
       it('is allowed to signup users by email', async () => {
         await expect(mutate({ mutation, variables })).resolves.toMatchObject({
           data: { Signup: { email: 'someuser@example.org' } },
+          errors: undefined,
         })
       })
 
@@ -342,7 +343,7 @@ describe('Signup', () => {
 
             it('throws UserInputError error because of unique constraint violation', async () => {
               await expect(mutate({ mutation, variables })).resolves.toMatchObject({
-                errors: [{ message: 'User account with this email already exists.' }],
+                errors: [{ message: 'A user account with this email already exists.' }],
               })
             })
           })
@@ -351,6 +352,7 @@ describe('Signup', () => {
             it('resolves with the already existing email', async () => {
               await expect(mutate({ mutation, variables })).resolves.toMatchObject({
                 data: { Signup: { email: 'someuser@example.org' } },
+                errors: undefined,
               })
             })
 
@@ -359,6 +361,7 @@ describe('Signup', () => {
               await expect(neode.all('EmailAddress')).resolves.toHaveLength(2)
               await expect(mutate({ mutation, variables })).resolves.toMatchObject({
                 data: { Signup: { email: 'someuser@example.org' } },
+                errors: undefined,
               })
               await expect(neode.all('EmailAddress')).resolves.toHaveLength(2)
             })

webapp/components/Registration/Signup.spec.js

@@ -115,7 +115,7 @@ describe('Signup', () => {
             mocks.$apollo.mutate = jest
               .fn()
               .mockRejectedValue(
-                new Error('UserInputError: User account with this email already exists.'),
+                new Error('UserInputError: A user account with this email already exists.'),
               )
           })
 

webapp/components/Registration/Signup.vue

@@ -128,7 +128,7 @@ export default {
       } catch (err) {
         const { message } = err
         const mapping = {
-          'User account with this email already exists': 'email-exists',
+          'A user account with this email already exists': 'email-exists',
           'Invitation code already used or does not exist': 'invalid-invitation-token',
         }
         for (const [pattern, key] of Object.entries(mapping)) {