From 904142cf6e2a84274aee11674005a5766057ff59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Sun, 14 Jul 2019 11:17:44 +0200 Subject: [PATCH 01/10] Create a separate node for email in data import --- .../maintenance-worker/migration/neo4j/users/users.cql | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/deployment/legacy-migration/maintenance-worker/migration/neo4j/users/users.cql b/deployment/legacy-migration/maintenance-worker/migration/neo4j/users/users.cql index 84eb7074b..7574fd3b2 100644 --- a/deployment/legacy-migration/maintenance-worker/migration/neo4j/users/users.cql +++ b/deployment/legacy-migration/maintenance-worker/migration/neo4j/users/users.cql @@ -111,6 +111,13 @@ u.createdAt = user.createdAt.`$date`, u.updatedAt = user.updatedAt.`$date`, u.deleted = user.deletedAt IS NOT NULL, u.disabled = false +MERGE (e:EmailAddress { + email: user.email, + createdAt: toString(datetime()), + verifiedAt: toString(datetime()) +}) +MERGE (e)-[:BELONGS_TO]->(u) +MERGE (u)<-[:PRIMARY_EMAIL]-(e) WITH u, user, user.badgeIds AS badgeIds UNWIND badgeIds AS badgeId MATCH (b:Badge {id: badgeId}) From 46589362fac21295c1c9e161e5f2be09b6d5c99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Sun, 14 Jul 2019 11:32:19 +0200 Subject: [PATCH 02/10] Start to refactor User<->EmailAddress --- backend/src/models/User.js | 1 - backend/src/schema/resolvers/user_management.js | 8 ++++---- backend/src/schema/types/type/User.gql | 2 +- backend/src/seed/factories/users.js | 10 +++++++++- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/backend/src/models/User.js b/backend/src/models/User.js index 02ce04513..cac8fd7a6 100644 --- a/backend/src/models/User.js +++ b/backend/src/models/User.js @@ -4,7 +4,6 @@ module.exports = { id: { type: 'string', primary: true, default: uuid }, // TODO: should be type: 'uuid' but simplified for our tests actorId: { type: 'string', allow: [null] }, name: { type: 'string', min: 3 }, - email: { type: 'string', lowercase: true, email: true }, slug: 'string', encryptedPassword: 'string', avatar: { type: 'string', allow: [null] }, diff --git a/backend/src/schema/resolvers/user_management.js b/backend/src/schema/resolvers/user_management.js index b62f9a609..af5ef3222 100644 --- a/backend/src/schema/resolvers/user_management.js +++ b/backend/src/schema/resolvers/user_management.js @@ -21,8 +21,8 @@ export default { // } const session = driver.session() const result = await session.run( - 'MATCH (user:User {email: $userEmail}) ' + - 'RETURN user {.id, .slug, .name, .avatar, .email, .encryptedPassword, .role, .disabled} as user LIMIT 1', + 'MATCH (user:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail})' + + 'RETURN user {.id, .slug, .name, .avatar, .encryptedPassword, .role, .disabled} as user LIMIT 1', { userEmail: email, }, @@ -48,7 +48,7 @@ export default { changePassword: async (_, { oldPassword, newPassword }, { driver, user }) => { const session = driver.session() let result = await session.run( - `MATCH (user:User {email: $userEmail}) + `MATCH (user:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail}) RETURN user {.id, .email, .encryptedPassword}`, { userEmail: user.email, @@ -68,7 +68,7 @@ export default { } else { const newEncryptedPassword = await bcrypt.hashSync(newPassword, 10) session.run( - `MATCH (user:User {email: $userEmail}) + `MATCH (user:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail}) SET user.encryptedPassword = $newEncryptedPassword RETURN user `, diff --git a/backend/src/schema/types/type/User.gql b/backend/src/schema/types/type/User.gql index e5c43aad4..95d9119b0 100644 --- a/backend/src/schema/types/type/User.gql +++ b/backend/src/schema/types/type/User.gql @@ -2,7 +2,7 @@ type User { id: ID! actorId: String name: String - email: String! + email: String! @cypher(statement: "MATCH (this)-[:PRIMARY_EMAIL]->(e:EmailAddress) RETURN e.email") slug: String! avatar: String coverImg: String diff --git a/backend/src/seed/factories/users.js b/backend/src/seed/factories/users.js index 8bdf03b9f..c34a9b3de 100644 --- a/backend/src/seed/factories/users.js +++ b/backend/src/seed/factories/users.js @@ -21,7 +21,15 @@ export default function create() { ...args, } args = await encryptPassword(args) - return neodeInstance.create('User', args) + const [user, email] = await Promise.all([ + neodeInstance.create('User', args), + neodeInstance.create('EmailAddress', { email: args.email }), + ]) + await Promise.all([ + user.relateTo(email, 'primaryEmail'), + email.relateTo(user, 'belongsTo') + ]) + return user }, } } From dacc3bb5573b8b1376eea1dab49ee93f63ccadad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Sun, 14 Jul 2019 11:57:38 +0200 Subject: [PATCH 03/10] Implement fallback User.email resolver --- backend/src/schema/resolvers/users.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index ea076d005..aa9532e2d 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -104,6 +104,14 @@ export default { }, }, User: { + email: async (parent, params, context, resolveInfo) => { + if (typeof parent.email !== 'undefined') return parent.email + const { id } = parent + const statement = `MATCH(u:User {id: {id}})-[:PRIMARY_EMAIL]->(e:EmailAddress) RETURN e` + const result = await instance.cypher(statement, { id }) + let [{email}]= result.records.map(r => r.get('e').properties) + return email + }, ...undefinedToNull([ 'actorId', 'avatar', From 88aa8a3929b05a96818e396b52d5d3ca5ce826f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Sun, 14 Jul 2019 12:11:49 +0200 Subject: [PATCH 04/10] Encrypt email in JWT - fixes 2 test cases --- backend/src/schema/resolvers/user_management.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/schema/resolvers/user_management.js b/backend/src/schema/resolvers/user_management.js index af5ef3222..fba5fe3bd 100644 --- a/backend/src/schema/resolvers/user_management.js +++ b/backend/src/schema/resolvers/user_management.js @@ -22,7 +22,7 @@ export default { const session = driver.session() const result = await session.run( 'MATCH (user:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail})' + - 'RETURN user {.id, .slug, .name, .avatar, .encryptedPassword, .role, .disabled} as user LIMIT 1', + 'RETURN user {.id, .slug, .name, .avatar, .encryptedPassword, .role, .disabled, email:e.email} as user LIMIT 1', { userEmail: email, }, From cec3eddcefc2d1654879ce8b632261c16c95669d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Sun, 14 Jul 2019 12:46:07 +0200 Subject: [PATCH 05/10] Fix more test cases --- backend/src/schema/resolvers/passwordReset.js | 4 ++-- backend/src/schema/resolvers/registration.js | 4 ++-- backend/src/schema/resolvers/registration.spec.js | 5 +++-- backend/src/schema/resolvers/user_management.js | 12 ++---------- backend/src/schema/resolvers/users.js | 2 +- backend/src/seed/factories/users.js | 5 +---- neo4j/db_setup.sh | 2 ++ 7 files changed, 13 insertions(+), 21 deletions(-) diff --git a/backend/src/schema/resolvers/passwordReset.js b/backend/src/schema/resolvers/passwordReset.js index 415eb6f21..88d82846a 100644 --- a/backend/src/schema/resolvers/passwordReset.js +++ b/backend/src/schema/resolvers/passwordReset.js @@ -5,7 +5,7 @@ export async function createPasswordReset(options) { const { driver, code, email, issuedAt = new Date() } = options const session = driver.session() const cypher = ` - MATCH (u:User) WHERE u.email = $email + MATCH (u:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email:$email}) CREATE(pr:PasswordReset {code: $code, issuedAt: datetime($issuedAt), usedAt: NULL}) MERGE (u)-[:REQUESTED]->(pr) RETURN u @@ -35,7 +35,7 @@ export default { const encryptedNewPassword = await bcrypt.hashSync(newPassword, 10) const cypher = ` MATCH (pr:PasswordReset {code: $code}) - MATCH (u:User {email: $email})-[:REQUESTED]->(pr) + MATCH (e:EmailAddress {email: $email})<-[:PRIMARY_EMAIL]-(u:User)-[:REQUESTED]->(pr) WHERE duration.between(pr.issuedAt, datetime()).days <= 0 AND pr.usedAt IS NULL SET pr.usedAt = datetime() SET u.encryptedPassword = $encryptedNewPassword diff --git a/backend/src/schema/resolvers/registration.js b/backend/src/schema/resolvers/registration.js index 3c8243d8a..20c54a49b 100644 --- a/backend/src/schema/resolvers/registration.js +++ b/backend/src/schema/resolvers/registration.js @@ -12,8 +12,8 @@ const instance = neode() */ const checkEmailDoesNotExist = async ({ email }) => { email = email.toLowerCase() - const users = await instance.all('User', { email }) - if (users.length > 0) throw new UserInputError('User account with this email already exists.') + const emails = await instance.all('EmailAddress', { email }) + if (emails.length > 0) throw new UserInputError('User account with this email already exists.') } export default { diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index 2cbce9a36..1cf3a13bb 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -166,11 +166,12 @@ describe('SignupByInvitation', () => { await expect(action()).rejects.toThrow('"email" must be a valid email') }) - it('creates no EmailAddress node', async done => { + it('creates no additional EmailAddress node', async done => { try { await action() } catch (e) { - const emailAddresses = await instance.all('EmailAddress') + let emailAddresses = await instance.all('EmailAddress') + emailAddresses = await emailAddresses.toJson expect(emailAddresses).toHaveLength(0) done() } diff --git a/backend/src/schema/resolvers/user_management.js b/backend/src/schema/resolvers/user_management.js index fba5fe3bd..ab7438a17 100644 --- a/backend/src/schema/resolvers/user_management.js +++ b/backend/src/schema/resolvers/user_management.js @@ -47,17 +47,9 @@ export default { }, changePassword: async (_, { oldPassword, newPassword }, { driver, user }) => { const session = driver.session() - let result = await session.run( - `MATCH (user:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail}) - RETURN user {.id, .email, .encryptedPassword}`, - { - userEmail: user.email, - }, - ) + let result = await session.run('MATCH (user:User {id:$id}) RETURN user', { id: user.id }) - const [currentUser] = result.records.map(function(record) { - return record.get('user') - }) + const [currentUser] = result.records.map(record => record.get('user').properties) if (!(await bcrypt.compareSync(oldPassword, currentUser.encryptedPassword))) { throw new AuthenticationError('Old password is not correct') diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index aa9532e2d..db5342881 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -109,7 +109,7 @@ export default { const { id } = parent const statement = `MATCH(u:User {id: {id}})-[:PRIMARY_EMAIL]->(e:EmailAddress) RETURN e` const result = await instance.cypher(statement, { id }) - let [{email}]= result.records.map(r => r.get('e').properties) + let [{ email }] = result.records.map(r => r.get('e').properties) return email }, ...undefinedToNull([ diff --git a/backend/src/seed/factories/users.js b/backend/src/seed/factories/users.js index c34a9b3de..3c5131a77 100644 --- a/backend/src/seed/factories/users.js +++ b/backend/src/seed/factories/users.js @@ -25,10 +25,7 @@ export default function create() { neodeInstance.create('User', args), neodeInstance.create('EmailAddress', { email: args.email }), ]) - await Promise.all([ - user.relateTo(email, 'primaryEmail'), - email.relateTo(user, 'belongsTo') - ]) + await Promise.all([user.relateTo(email, 'primaryEmail'), email.relateTo(user, 'belongsTo')]) return user }, } diff --git a/neo4j/db_setup.sh b/neo4j/db_setup.sh index 21ed54571..d4c7b9af8 100755 --- a/neo4j/db_setup.sh +++ b/neo4j/db_setup.sh @@ -34,6 +34,8 @@ CREATE CONSTRAINT ON (p:Post) ASSERT p.slug IS UNIQUE; CREATE CONSTRAINT ON (c:Category) ASSERT c.slug IS UNIQUE; CREATE CONSTRAINT ON (u:User) ASSERT u.slug IS UNIQUE; CREATE CONSTRAINT ON (o:Organization) ASSERT o.slug IS UNIQUE; + +CREATE CONSTRAINT ON (e:EmailAddress) ASSERT e.email IS UNIQUE; ' | cypher-shell echo ' From b8ab7cacef263275db35722adedb2089178d8687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Mon, 15 Jul 2019 19:17:16 +0200 Subject: [PATCH 06/10] Fix email filter test --- backend/src/models/EmailAddress.js | 1 + backend/src/schema/resolvers/users.js | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/backend/src/models/EmailAddress.js b/backend/src/models/EmailAddress.js index ddd56c297..6afccd1ed 100644 --- a/backend/src/models/EmailAddress.js +++ b/backend/src/models/EmailAddress.js @@ -8,5 +8,6 @@ module.exports = { relationship: 'BELONGS_TO', target: 'User', direction: 'out', + eager: true, }, } diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index db5342881..820688a1a 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -65,6 +65,13 @@ export const hasOne = obj => { export default { Query: { User: async (object, args, context, resolveInfo) => { + const { email } = args + if (email) { + const e = await instance.first('EmailAddress', { email }) + let user = e.get('belongsTo') + user = await user.toJson() + return [user.node] + } return neo4jgraphql(object, args, context, resolveInfo, false) }, }, From 46cd21db1fbb1c05552d71ad76c2556303238146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Mon, 15 Jul 2019 19:35:07 +0200 Subject: [PATCH 07/10] Fix registration.spec --- backend/src/schema/resolvers/registration.spec.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index 1cf3a13bb..dc2e96348 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -192,16 +192,16 @@ describe('SignupByInvitation', () => { describe('creates a EmailAddress node', () => { it('with a `createdAt` attribute', async () => { await action() - const emailAddresses = await instance.all('EmailAddress') - const emailAddress = await emailAddresses.first().toJson() + let emailAddress = await instance.first('EmailAddress', { email: 'someuser@example.org' }) + emailAddress = await emailAddress.toJson() expect(emailAddress.createdAt).toBeTruthy() expect(Date.parse(emailAddress.createdAt)).toEqual(expect.any(Number)) }) it('with a cryptographic `nonce`', async () => { await action() - const emailAddresses = await instance.all('EmailAddress') - const emailAddress = await emailAddresses.first().toJson() + let emailAddress = await instance.first('EmailAddress', { email: 'someuser@example.org' }) + emailAddress = await emailAddress.toJson() expect(emailAddress.nonce).toEqual(expect.any(String)) }) @@ -221,6 +221,7 @@ describe('SignupByInvitation', () => { it('rejects because codes can be used only once', async done => { await action() try { + variables.email = 'yetanotheremail@example.org' await action() } catch (e) { expect(e.message).toMatch(/Invitation code already used/) @@ -283,8 +284,8 @@ describe('Signup', () => { it('creates a Signup with a cryptographic `nonce`', async () => { await action() - const emailAddresses = await instance.all('EmailAddress') - const emailAddress = await emailAddresses.first().toJson() + let emailAddress = await instance.first('EmailAddress', { email: 'someuser@example.org' }) + emailAddress = await emailAddress.toJson() expect(emailAddress.nonce).toEqual(expect.any(String)) }) }) From 4c9dbdb3781635ea6a97d69f6ca060bbb41927cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 16 Jul 2019 20:06:26 +0200 Subject: [PATCH 08/10] Refactor changePassword resolver with neode --- .../src/schema/resolvers/user_management.js | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/backend/src/schema/resolvers/user_management.js b/backend/src/schema/resolvers/user_management.js index ab7438a17..d382c2aa7 100644 --- a/backend/src/schema/resolvers/user_management.js +++ b/backend/src/schema/resolvers/user_management.js @@ -2,6 +2,9 @@ import encode from '../../jwt/encode' import bcrypt from 'bcryptjs' import { AuthenticationError } from 'apollo-server' import { neo4jgraphql } from 'neo4j-graphql-js' +import { neode } from '../../bootstrap/neo4j' + +const instance = neode() export default { Query: { @@ -46,33 +49,24 @@ export default { } }, changePassword: async (_, { oldPassword, newPassword }, { driver, user }) => { - const session = driver.session() - let result = await session.run('MATCH (user:User {id:$id}) RETURN user', { id: user.id }) + let currentUser = await instance.find('User', user.id) - const [currentUser] = result.records.map(record => record.get('user').properties) - - if (!(await bcrypt.compareSync(oldPassword, currentUser.encryptedPassword))) { + const encryptedPassword = currentUser.get('encryptedPassword') + if (!(await bcrypt.compareSync(oldPassword, encryptedPassword))) { throw new AuthenticationError('Old password is not correct') } - if (await bcrypt.compareSync(newPassword, currentUser.encryptedPassword)) { + if (await bcrypt.compareSync(newPassword, encryptedPassword)) { throw new AuthenticationError('Old password and new password should be different') - } else { - const newEncryptedPassword = await bcrypt.hashSync(newPassword, 10) - session.run( - `MATCH (user:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail}) - SET user.encryptedPassword = $newEncryptedPassword - RETURN user - `, - { - userEmail: user.email, - newEncryptedPassword, - }, - ) - session.close() - - return encode(currentUser) } + + const newEncryptedPassword = await bcrypt.hashSync(newPassword, 10) + await currentUser.update({ + encryptedPassword: newEncryptedPassword, + updatedAt: new Date().toISOString() + }) + + return encode(await currentUser.toJson()) }, }, } From 95d26a701cb20bf1acc70d39d3c6f45681a305f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 16 Jul 2019 21:00:47 +0200 Subject: [PATCH 09/10] Fix lint --- backend/src/schema/resolvers/user_management.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/schema/resolvers/user_management.js b/backend/src/schema/resolvers/user_management.js index d382c2aa7..7ed84586b 100644 --- a/backend/src/schema/resolvers/user_management.js +++ b/backend/src/schema/resolvers/user_management.js @@ -63,7 +63,7 @@ export default { const newEncryptedPassword = await bcrypt.hashSync(newPassword, 10) await currentUser.update({ encryptedPassword: newEncryptedPassword, - updatedAt: new Date().toISOString() + updatedAt: new Date().toISOString(), }) return encode(await currentUser.toJson()) From 1970baaa2d13c5176ac12ce10022af61c9959b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 16 Jul 2019 21:05:44 +0200 Subject: [PATCH 10/10] Apparently neode doesn't like multiple connections Why? :worried: --- backend/src/seed/factories/users.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/backend/src/seed/factories/users.js b/backend/src/seed/factories/users.js index 3c5131a77..af1699253 100644 --- a/backend/src/seed/factories/users.js +++ b/backend/src/seed/factories/users.js @@ -21,11 +21,10 @@ export default function create() { ...args, } args = await encryptPassword(args) - const [user, email] = await Promise.all([ - neodeInstance.create('User', args), - neodeInstance.create('EmailAddress', { email: args.email }), - ]) - await Promise.all([user.relateTo(email, 'primaryEmail'), email.relateTo(user, 'belongsTo')]) + const user = await neodeInstance.create('User', args) + const email = await neodeInstance.create('EmailAddress', { email: args.email }) + await user.relateTo(email, 'primaryEmail') + await email.relateTo(user, 'belongsTo') return user }, }