From 9733cc602d96ed1214f19f3e49993e6fa7e369be Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Sat, 25 Jul 2020 21:04:29 +0100 Subject: [PATCH 1/9] feat(gql): search users wip --- modules/core/graph/resolvers/user.js | 122 +++++++++-------- modules/core/graph/schemas/user.graphql | 1 + modules/core/services/users.js | 169 +++++++++++++----------- 3 files changed, 159 insertions(+), 133 deletions(-) diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/user.js index cb5a6853d4..bf9c0e8228 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/user.js @@ -1,74 +1,88 @@ 'use strict' -const appRoot = require( 'app-root-path' ) -const { ApolloError, AuthenticationError, UserInputError } = require( 'apollo-server-express' ) -const { createUser, getUser, getUserByEmail, getUserRole, updateUser, deleteUser, validatePasssword } = require( '../../services/users' ) -const { createPersonalAccessToken, createAppToken, revokeToken, revokeTokenById, validateToken, getUserTokens } = require( '../../services/tokens' ) -const { validateServerRole, validateScopes, authorizeResolver } = require( `${appRoot}/modules/shared` ) -const setupCheck = require( `${appRoot}/setupcheck` ) -const zxcvbn = require( 'zxcvbn' ) +const appRoot = require('app-root-path') +const { ApolloError, AuthenticationError, UserInputError } = require('apollo-server-express') +const { createUser, getUser, getUserByEmail, getUserRole, updateUser, deleteUser, findUsers, validatePasssword } = require('../../services/users') +const { createPersonalAccessToken, createAppToken, revokeToken, revokeTokenById, validateToken, getUserTokens } = require('../../services/tokens') +const { validateServerRole, validateScopes, authorizeResolver } = require(`${appRoot}/modules/shared`) +const setupCheck = require(`${appRoot}/setupcheck`) +const zxcvbn = require('zxcvbn') module.exports = { - Query: { + Query: { - async _( ) { - return `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn.` - }, + async _() { + return `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn.` + }, + + async user(parent, args, context, info) { + + await validateServerRole(context, 'server:user') + + if (!args.id) + await validateScopes(context.scopes, 'profile:read') + else + await validateScopes(context.scopes, 'users:read') + + if (!args.id && !context.userId) { + throw new UserInputError('You must provide an user id.') + } + + return await getUser(args.id || context.userId) + }, - async user( parent, args, context, info ) { + async users(parent, args, context, info) { - await validateServerRole( context, 'server:user' ) + await validateServerRole(context, 'server:user') - if ( !args.id ) - await validateScopes( context.scopes, 'profile:read' ) - else - await validateScopes( context.scopes, 'users:read' ) + if (!args.id) + await validateScopes(context.scopes, 'profile:read') + else + await validateScopes(context.scopes, 'users:read') - if ( !args.id && !context.userId ) { - throw new UserInputError( 'You must provide an user id.' ) - } + return await findUsers(args.query) + }, + + async userPwdStrength(parent, args, context, info) { + let res = zxcvbn(args.pwd) + return { score: res.score, feedback: res.feedback } + } - return await getUser( args.id || context.userId ) }, - async userPwdStrength( parent, args, context, info ) { - let res = zxcvbn( args.pwd ) - return { score: res.score, feedback: res.feedback } - } + User: { - }, + async email(parent, args, context, info) { + // NOTE: we're redacting the field (returning null) rather than throwing a full error which would invalidate the request. + if (context.userId === parent.id) { + try { + await validateScopes(context.scopes, 'profile:email') + return parent.email + } catch (err) { + return null + } + } - User: { + try { + await validateScopes(context.scopes, 'users:email') + return parent.email + } catch (err) { + return null + } + }, - async email( parent, args, context, info ) { - // NOTE: we're redacting the field (returning null) rather than throwing a full error which would invalidate the request. - if ( context.userId === parent.id ) { - try { - await validateScopes( context.scopes, 'profile:email' ) - return parent.email - } catch ( err ) { - return null + async role(parent, args, context, info) { + return await getUserRole(parent.id) } - } - - try { - await validateScopes( context.scopes, 'users:email' ) - return parent.email - } catch ( err ) { - return null - } + }, - async role( parent, args, context, info ) { - return await getUserRole( parent.id ) - } - }, - Mutation: { - async userEdit( parent, args, context, info ) { - await validateServerRole( context, 'server:user' ) - await updateUser( context.userId, args.user ) - return true + Mutation: { + async userEdit(parent, args, context, info) { + await validateServerRole(context, 'server:user') + await updateUser(context.userId, args.user) + return true + } } - } -} +} \ No newline at end of file diff --git a/modules/core/graph/schemas/user.graphql b/modules/core/graph/schemas/user.graphql index 355ec20fd8..2767a7b3f3 100644 --- a/modules/core/graph/schemas/user.graphql +++ b/modules/core/graph/schemas/user.graphql @@ -3,6 +3,7 @@ extend type Query { Gets the profile of a user. If no id argument is provided, will return the current authenticated user's profile (as extracted from the authorization header). """ user( id: String ): User + users(query: String): [User!]! userPwdStrength( pwd: String! ): JSONObject } diff --git a/modules/core/services/users.js b/modules/core/services/users.js index 4473d051ac..e422a9bbf8 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -1,93 +1,104 @@ 'use strict' -const bcrypt = require( 'bcrypt' ) -const crs = require( 'crypto-random-string' ) -const appRoot = require( 'app-root-path' ) -const knex = require( `${appRoot}/db/knex` ) +const bcrypt = require('bcrypt') +const crs = require('crypto-random-string') +const appRoot = require('app-root-path') +const knex = require(`${appRoot}/db/knex`) -const Users = ( ) => knex( 'users' ) -const ServerRoles = ( ) => knex( 'server_acl' ) +const Users = () => knex('users') +const ServerRoles = () => knex('server_acl') module.exports = { - /* + /* - Users + Users - */ + */ - async createUser( user ) { - let [ { count } ] = await ServerRoles( ).where( { role: 'server:admin' } ).count( ) + async createUser(user) { + let [{ count }] = await ServerRoles().where({ role: 'server:admin' }).count() - user.id = crs( { length: 10 } ) + user.id = crs({ length: 10 }) - if ( user.password ) { - user.passwordDigest = await bcrypt.hash( user.password, 10 ) - } - delete user.password + if (user.password) { + user.passwordDigest = await bcrypt.hash(user.password, 10) + } + delete user.password - let usr = await Users( ).select( 'id' ).where( { email: user.email } ).first( ) - if ( usr ) throw new Error( 'Email taken. Try logging in?' ) + let usr = await Users().select('id').where({ email: user.email }).first() + if (usr) throw new Error('Email taken. Try logging in?') - let res = await Users( ).returning( 'id' ).insert( user ) + let res = await Users().returning('id').insert(user) - if ( parseInt( count ) === 0 ) { - await ServerRoles( ).insert( { userId: res[ 0 ], role: 'server:admin' } ) - } else { - await ServerRoles( ).insert( { userId: res[ 0 ], role: 'server:user' } ) - } + if (parseInt(count) === 0) { + await ServerRoles().insert({ userId: res[0], role: 'server:admin' }) + } else { + await ServerRoles().insert({ userId: res[0], role: 'server:user' }) + } + + return res[0] + }, + + async findOrCreateUser({ user, rawProfile }) { + let existingUser = await Users().select('id').where({ email: user.email }).first() + + if (existingUser) + return existingUser + + user.password = crs({ length: 20 }) + user.verified = true // because we trust the external identity provider, no? + return { id: await module.exports.createUser(user) } + }, + + async getUserById({ userId }) { + let user = await Users().where({ id: userId }).select('*').first() + delete user.passwordDigest + return user + }, + + // TODO: deprecate + async getUser(id) { + let user = await Users().where({ id: id }).select('*').first() + delete user.passwordDigest + return user + }, - return res[ 0 ] - }, - - async findOrCreateUser( { user, rawProfile } ) { - let existingUser = await Users( ).select( 'id' ).where( { email: user.email } ).first( ) - - if ( existingUser ) - return existingUser - - user.password = crs( { length: 20 } ) - user.verified = true // because we trust the external identity provider, no? - return { id: await module.exports.createUser( user ) } - }, - - async getUserById( { userId } ) { - let user = await Users( ).where( { id: userId } ).select( '*' ).first( ) - delete user.passwordDigest - return user - }, - - // TODO: deprecate - async getUser( id ) { - let user = await Users( ).where( { id: id } ).select( '*' ).first( ) - delete user.passwordDigest - return user - }, - - async getUserByEmail( { email } ) { - let user = await Users( ).where( { email: email } ).select( '*' ).first( ) - delete user.passwordDigest - return user - }, - - async getUserRole( id ) { - let { role } = await ServerRoles( ).where( { userId: id } ).select( 'role' ).first( ) - return role - }, - - async updateUser( id, user ) { - delete user.id - delete user.passwordDigest - delete user.password - delete user.email - await Users( ).where( { id: id } ).update( user ) - }, - - async validatePasssword( { email, password } ) { - let { passwordDigest } = await Users( ).where( { email: email } ).select( 'passwordDigest' ).first( ) - return bcrypt.compare( password, passwordDigest ) - }, - - async deleteUser( id ) { - throw new Error( 'not implemented' ) - } -} + async getUserByEmail({ email }) { + let user = await Users().where({ email: email }).select('*').first() + delete user.passwordDigest + return user + }, + + async getUserRole(id) { + let { role } = await ServerRoles().where({ userId: id }).select('role').first() + return role + }, + + async updateUser(id, user) { + delete user.id + delete user.passwordDigest + delete user.password + delete user.email + await Users().where({ id: id }).update(user) + }, + + async findUsers(query) { + + query = "%" + query + "%"; + let users = await Users() + .where('email', 'like', query) + .orWhere('username', 'like', query) + .orWhere('name', 'like', query) + + return users + }, + + async validatePasssword({ email, password }) { + let { passwordDigest } = await Users().where({ email: email }).select('passwordDigest').first() + return bcrypt.compare(password, passwordDigest) + }, + + async deleteUser(id) { + throw new Error('not implemented') + } +} \ No newline at end of file From 8df402e4cb0e0e0de8c08c447f4666ab2a1facaf Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Sat, 25 Jul 2020 21:21:59 +0100 Subject: [PATCH 2/9] fix(gql): query users, improves validations --- modules/core/graph/resolvers/user.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/user.js index bf9c0e8228..00903c9d9b 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/user.js @@ -33,11 +33,12 @@ module.exports = { async users(parent, args, context, info) { await validateServerRole(context, 'server:user') + await validateScopes(context.scopes, 'profile:read') + await validateScopes(context.scopes, 'users:read') - if (!args.id) - await validateScopes(context.scopes, 'profile:read') - else - await validateScopes(context.scopes, 'users:read') + if (!args.query) { + throw new UserInputError('You must provide a search query.') + } return await findUsers(args.query) }, From c6e08b2c5dd4f2d20e76b4c78fe6d8ad056324b4 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Mon, 27 Jul 2020 23:13:20 +0100 Subject: [PATCH 3/9] feat(gql): adds UserSearchResults and improves user search --- .editorconfig | 1 + modules/core/graph/resolvers/user.js | 74 ++++++++++---------- modules/core/graph/schemas/user.graphql | 14 +++- modules/core/services/users.js | 89 +++++++++++++------------ readme.md | 12 +++- 5 files changed, 109 insertions(+), 81 deletions(-) diff --git a/.editorconfig b/.editorconfig index ab561f3c56..435e72be69 100644 --- a/.editorconfig +++ b/.editorconfig @@ -7,3 +7,4 @@ charset = utf-8 end_of_line = lf insert_final_newline = true trim_trailing_whitespace = true +spaces_around_brackets = both diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/user.js index 00903c9d9b..65fc46bbb0 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/user.js @@ -1,11 +1,11 @@ 'use strict' -const appRoot = require('app-root-path') -const { ApolloError, AuthenticationError, UserInputError } = require('apollo-server-express') -const { createUser, getUser, getUserByEmail, getUserRole, updateUser, deleteUser, findUsers, validatePasssword } = require('../../services/users') -const { createPersonalAccessToken, createAppToken, revokeToken, revokeTokenById, validateToken, getUserTokens } = require('../../services/tokens') -const { validateServerRole, validateScopes, authorizeResolver } = require(`${appRoot}/modules/shared`) -const setupCheck = require(`${appRoot}/setupcheck`) -const zxcvbn = require('zxcvbn') +const appRoot = require( 'app-root-path' ) +const { ApolloError, AuthenticationError, UserInputError } = require( 'apollo-server-express' ) +const { createUser, getUser, getUserByEmail, getUserRole, updateUser, deleteUser, searchUsers, validatePasssword } = require( '../../services/users' ) +const { createPersonalAccessToken, createAppToken, revokeToken, revokeTokenById, validateToken, getUserTokens } = require( '../../services/tokens' ) +const { validateServerRole, validateScopes, authorizeResolver } = require( `${appRoot}/modules/shared` ) +const setupCheck = require( `${appRoot}/setupcheck` ) +const zxcvbn = require( 'zxcvbn' ) module.exports = { Query: { @@ -14,37 +14,41 @@ module.exports = { return `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn.` }, - async user(parent, args, context, info) { + async user( parent, args, context, info ) { - await validateServerRole(context, 'server:user') + await validateServerRole( context, 'server:user' ) - if (!args.id) - await validateScopes(context.scopes, 'profile:read') + if ( !args.id ) + await validateScopes( context.scopes, 'profile:read' ) else - await validateScopes(context.scopes, 'users:read') + await validateScopes( context.scopes, 'users:read' ) - if (!args.id && !context.userId) { - throw new UserInputError('You must provide an user id.') + if ( !args.id && !context.userId ) { + throw new UserInputError( 'You must provide an user id.' ) } - return await getUser(args.id || context.userId) + return await getUser( args.id || context.userId ) }, - async users(parent, args, context, info) { + async userSearchResults( parent, args, context, info ) { - await validateServerRole(context, 'server:user') - await validateScopes(context.scopes, 'profile:read') - await validateScopes(context.scopes, 'users:read') + await validateServerRole( context, 'server:user' ) + await validateScopes( context.scopes, 'profile:read' ) + await validateScopes( context.scopes, 'users:read' ) - if (!args.query) { - throw new UserInputError('You must provide a search query.') + if ( !args.query ) { + throw new UserInputError( 'You must provide a search query.' ) } - return await findUsers(args.query) + if ( args.query.length < 3 ) { + throw new UserInputError( 'Search query must be at least 3 carachters.' ) + } + + return await searchUsers( args.query, args.limit ) }, - async userPwdStrength(parent, args, context, info) { - let res = zxcvbn(args.pwd) + async userPwdStrength( parent, args, context, info ) { + let res = zxcvbn( args.pwd ) return { score: res.score, feedback: res.feedback } } @@ -52,27 +56,27 @@ module.exports = { User: { - async email(parent, args, context, info) { + async email( parent, args, context, info ) { // NOTE: we're redacting the field (returning null) rather than throwing a full error which would invalidate the request. - if (context.userId === parent.id) { + if ( context.userId === parent.id ) { try { - await validateScopes(context.scopes, 'profile:email') + await validateScopes( context.scopes, 'profile:email' ) return parent.email - } catch (err) { + } catch ( err ) { return null } } try { - await validateScopes(context.scopes, 'users:email') + await validateScopes( context.scopes, 'users:email' ) return parent.email - } catch (err) { + } catch ( err ) { return null } }, - async role(parent, args, context, info) { - return await getUserRole(parent.id) + async role( parent, args, context, info ) { + return await getUserRole( parent.id ) } }, @@ -80,9 +84,9 @@ module.exports = { Mutation: { - async userEdit(parent, args, context, info) { - await validateServerRole(context, 'server:user') - await updateUser(context.userId, args.user) + async userEdit( parent, args, context, info ) { + await validateServerRole( context, 'server:user' ) + await updateUser( context.userId, args.user ) return true } } diff --git a/modules/core/graph/schemas/user.graphql b/modules/core/graph/schemas/user.graphql index 2767a7b3f3..a4ae70bd5d 100644 --- a/modules/core/graph/schemas/user.graphql +++ b/modules/core/graph/schemas/user.graphql @@ -3,7 +3,7 @@ extend type Query { Gets the profile of a user. If no id argument is provided, will return the current authenticated user's profile (as extracted from the authorization header). """ user( id: String ): User - users(query: String): [User!]! + userSearchResults(query: String, limit: Int! = 100): [UserSearchResult!]! userPwdStrength( pwd: String! ): JSONObject } @@ -23,6 +23,18 @@ type User { role: String } +type UserSearchResult { + id: String! + username: String + name: String + bio: String + company: String + avatar: String + verified: Boolean + profiles: JSONObject + role: String +} + extend type Mutation { """ Edits a user's profile. diff --git a/modules/core/services/users.js b/modules/core/services/users.js index e422a9bbf8..0fb2b85a7e 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -1,11 +1,11 @@ 'use strict' -const bcrypt = require('bcrypt') -const crs = require('crypto-random-string') -const appRoot = require('app-root-path') -const knex = require(`${appRoot}/db/knex`) +const bcrypt = require( 'bcrypt' ) +const crs = require( 'crypto-random-string' ) +const appRoot = require( 'app-root-path' ) +const knex = require( `${appRoot}/db/knex` ) -const Users = () => knex('users') -const ServerRoles = () => knex('server_acl') +const Users = () => knex( 'users' ) +const ServerRoles = () => knex( 'server_acl' ) module.exports = { @@ -15,90 +15,93 @@ module.exports = { */ - async createUser(user) { - let [{ count }] = await ServerRoles().where({ role: 'server:admin' }).count() + async createUser( user ) { + let [ {count} ] = await ServerRoles().where( {role: 'server:admin'} ).count() - user.id = crs({ length: 10 }) + user.id = crs( {length: 10} ) - if (user.password) { - user.passwordDigest = await bcrypt.hash(user.password, 10) + if ( user.password ) { + user.passwordDigest = await bcrypt.hash( user.password, 10 ) } delete user.password - let usr = await Users().select('id').where({ email: user.email }).first() - if (usr) throw new Error('Email taken. Try logging in?') + let usr = await Users().select( 'id' ).where( {email: user.email} ).first() + if ( usr ) throw new Error( 'Email taken. Try logging in?' ) - let res = await Users().returning('id').insert(user) + let res = await Users().returning( 'id' ).insert( user ) - if (parseInt(count) === 0) { - await ServerRoles().insert({ userId: res[0], role: 'server:admin' }) + if ( parseInt( count ) === 0 ) { + await ServerRoles().insert( {userId: res[0], role: 'server:admin'} ) } else { - await ServerRoles().insert({ userId: res[0], role: 'server:user' }) + await ServerRoles().insert( {userId: res[0], role: 'server:user'} ) } return res[0] }, - async findOrCreateUser({ user, rawProfile }) { - let existingUser = await Users().select('id').where({ email: user.email }).first() + async findOrCreateUser( {user, rawProfile} ) { + let existingUser = await Users().select( 'id' ).where( {email: user.email} ).first() - if (existingUser) + if ( existingUser ) return existingUser - user.password = crs({ length: 20 }) + user.password = crs( {length: 20} ) user.verified = true // because we trust the external identity provider, no? - return { id: await module.exports.createUser(user) } + return {id: await module.exports.createUser( user )} }, - async getUserById({ userId }) { - let user = await Users().where({ id: userId }).select('*').first() + async getUserById( {userId} ) { + let user = await Users().where( {id: userId} ).select( '*' ).first() delete user.passwordDigest return user }, // TODO: deprecate - async getUser(id) { - let user = await Users().where({ id: id }).select('*').first() + async getUser( id ) { + let user = await Users().where( {id: id} ).select( '*' ).first() delete user.passwordDigest return user }, - async getUserByEmail({ email }) { - let user = await Users().where({ email: email }).select('*').first() + async getUserByEmail( {email} ) { + let user = await Users().where( {email: email} ).select( '*' ).first() delete user.passwordDigest return user }, - async getUserRole(id) { - let { role } = await ServerRoles().where({ userId: id }).select('role').first() + async getUserRole( id ) { + let {role} = await ServerRoles().where( {userId: id} ).select( 'role' ).first() return role }, - async updateUser(id, user) { + async updateUser( id, user ) { delete user.id delete user.passwordDigest delete user.password delete user.email - await Users().where({ id: id }).update(user) + await Users().where( {id: id} ).update( user ) }, - async findUsers(query) { - - query = "%" + query + "%"; + async searchUsers( query, limit ) { + if ( limit > 100 || limit === undefined ) + limit = 100 + + let likeQuery = "%" + query + "%" let users = await Users() - .where('email', 'like', query) - .orWhere('username', 'like', query) - .orWhere('name', 'like', query) + .where( {email: query} ) //match full email or partial username / name + .orWhere( 'username', 'like', likeQuery ) + .orWhere( 'name', 'like', likeQuery ) + .limit( limit ) return users }, - async validatePasssword({ email, password }) { - let { passwordDigest } = await Users().where({ email: email }).select('passwordDigest').first() - return bcrypt.compare(password, passwordDigest) + async validatePasssword( {email, password} ) { + let {passwordDigest} = await Users().where( {email: email} ).select( 'passwordDigest' ).first() + return bcrypt.compare( password, passwordDigest ) }, - async deleteUser(id) { - throw new Error('not implemented') + async deleteUser( id ) { + throw new Error( 'not implemented' ) } } \ No newline at end of file diff --git a/readme.md b/readme.md index ea7587e989..f9c4f6bf50 100644 --- a/readme.md +++ b/readme.md @@ -26,8 +26,16 @@ To debug, simply run `npm run dev:server`. To test, run `npm run test:server`. T ### How to commit to this repo When pushing commits to this repo, please follow the following guidelines: -1) Install [commitizen](https://www.npmjs.com/package/commitizen#commitizen-for-contributors) globally -3) When ready to commit, type in the commandline `git cz` & follow the prompts. +1. Install [commitizen](https://www.npmjs.com/package/commitizen#commitizen-for-contributors) globally +2. When ready to commit, type in the commandline `git cz` & follow the prompts. +3. Install eslint globally `npm i -g eslint` + 1. if using VS code install the `eslint` extension + 2. we also recommend setting it to run on save by adding the following VS Code setting + ``` + "editor.codeActionsOnSave": { + "source.fixAll.eslint": true + } + ``` ## Modules From 84d28396fedf6050c1645f40e44e94459aea3659 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Mon, 27 Jul 2020 23:16:49 +0100 Subject: [PATCH 4/9] docs --- readme.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/readme.md b/readme.md index 89d2341e5f..6a1b2153c0 100644 --- a/readme.md +++ b/readme.md @@ -28,6 +28,14 @@ When pushing commits to this repo, please follow the following guidelines: - Install [commitizen](https://www.npmjs.com/package/commitizen#commitizen-for-contributors) globally (`npm i -g commitizen`). - When ready to commit, type in the commandline `git cz` & follow the prompts. +- Install eslint globally `npm i -g eslint` + - if using VS code install the `eslint` extension + - we also recommend setting it to run on save by adding the following VS Code setting + ``` + "editor.codeActionsOnSave": { + "source.fixAll.eslint": true + } + ``` ## Modules From 5f176421caf989446c7d8bbed5f4aa670da96984 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Tue, 28 Jul 2020 14:38:33 +0100 Subject: [PATCH 5/9] feat(gql): returns error if limit > 100 --- modules/core/graph/resolvers/user.js | 4 ++++ modules/core/services/users.js | 5 ++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/user.js index 65fc46bbb0..8a0a692f93 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/user.js @@ -44,6 +44,10 @@ module.exports = { throw new UserInputError( 'Search query must be at least 3 carachters.' ) } + if ( args.limit && args.limit > 100 ) { + throw new UserInputError( 'Cannot return more than 100 results.' ) + } + return await searchUsers( args.query, args.limit ) }, diff --git a/modules/core/services/users.js b/modules/core/services/users.js index 0fb2b85a7e..b19adc4402 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -83,9 +83,8 @@ module.exports = { }, async searchUsers( query, limit ) { - if ( limit > 100 || limit === undefined ) - limit = 100 - + + limit = limit || 100 let likeQuery = "%" + query + "%" let users = await Users() .where( {email: query} ) //match full email or partial username / name From d2db31381bafedc14f2aa330d51b014f86136b18 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Tue, 28 Jul 2020 14:43:01 +0100 Subject: [PATCH 6/9] refractor: fixes indentation --- .eslintrc.json | 49 ++++++-- modules/core/graph/resolvers/user.js | 128 ++++++++++---------- modules/core/services/users.js | 175 +++++++++++++-------------- 3 files changed, 191 insertions(+), 161 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index afaeee7f71..be1e1314c4 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -7,13 +7,46 @@ "parserOptions": { "ecmaVersion": 11 }, - "ignorePatterns": [ "modules/*/tests/*", "node_modules/*", "frontend/*"], + "ignorePatterns": [ + "modules/*/tests/*", + "node_modules/*", + "frontend/*" + ], "rules": { - "arrow-spacing": [ 2, { "before": true, "after": true } ], - "array-bracket-spacing": [ 2, "always" ], - "block-spacing": [ 2, "always" ], - "camelcase": [ 1, { "properties": "always" } ], - "space-in-parens": [ 2, "always" ], - "keyword-spacing": 2 + "arrow-spacing": [ + 2, + { + "before": true, + "after": true + } + ], + "array-bracket-spacing": [ + 2, + "always" + ], + "block-spacing": [ + 2, + "always" + ], + "camelcase": [ + 1, + { + "properties": "always" + } + ], + "space-in-parens": [ + 2, + "always" + ], + "keyword-spacing": 2, + "semi": "off", + "indent": [ + "error", + 2 + ], + "padded-blocks": [ + "error", + "never" + ] } -} +} \ No newline at end of file diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/user.js index 8a0a692f93..dbbb04c672 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/user.js @@ -8,90 +8,88 @@ const setupCheck = require( `${appRoot}/setupcheck` ) const zxcvbn = require( 'zxcvbn' ) module.exports = { - Query: { + Query: { - async _() { - return `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn.` - }, + async _() { + return `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn.` + }, - async user( parent, args, context, info ) { + async user( parent, args, context, info ) { + await validateServerRole( context, 'server:user' ) - await validateServerRole( context, 'server:user' ) + if ( !args.id ) + await validateScopes( context.scopes, 'profile:read' ) + else + await validateScopes( context.scopes, 'users:read' ) - if ( !args.id ) - await validateScopes( context.scopes, 'profile:read' ) - else - await validateScopes( context.scopes, 'users:read' ) + if ( !args.id && !context.userId ) { + throw new UserInputError( 'You must provide an user id.' ) + } - if ( !args.id && !context.userId ) { - throw new UserInputError( 'You must provide an user id.' ) - } + return await getUser( args.id || context.userId ) + }, - return await getUser( args.id || context.userId ) - }, + async userSearchResults( parent, args, context, info ) { + await validateServerRole( context, 'server:user' ) + await validateScopes( context.scopes, 'profile:read' ) + await validateScopes( context.scopes, 'users:read' ) - async userSearchResults( parent, args, context, info ) { + if ( !args.query ) { + throw new UserInputError( 'You must provide a search query.' ) + } - await validateServerRole( context, 'server:user' ) - await validateScopes( context.scopes, 'profile:read' ) - await validateScopes( context.scopes, 'users:read' ) + if ( args.query.length < 3 ) { + throw new UserInputError( 'Search query must be at least 3 carachters.' ) + } - if ( !args.query ) { - throw new UserInputError( 'You must provide a search query.' ) - } + if ( args.limit && args.limit > 100 ) { + throw new UserInputError( 'Cannot return more than 100 results.' ) + } - if ( args.query.length < 3 ) { - throw new UserInputError( 'Search query must be at least 3 carachters.' ) - } + return await searchUsers( args.query, args.limit ) + }, - if ( args.limit && args.limit > 100 ) { - throw new UserInputError( 'Cannot return more than 100 results.' ) - } + async userPwdStrength( parent, args, context, info ) { + let res = zxcvbn( args.pwd ) + return { score: res.score, feedback: res.feedback } + } - return await searchUsers( args.query, args.limit ) - }, + }, - async userPwdStrength( parent, args, context, info ) { - let res = zxcvbn( args.pwd ) - return { score: res.score, feedback: res.feedback } - } + User: { + async email( parent, args, context, info ) { + // NOTE: we're redacting the field (returning null) rather than throwing a full error which would invalidate the request. + if ( context.userId === parent.id ) { + try { + await validateScopes( context.scopes, 'profile:email' ) + return parent.email + } catch ( err ) { + return null + } + } + + try { + await validateScopes( context.scopes, 'users:email' ) + return parent.email + } catch ( err ) { + return null + } }, - User: { - - async email( parent, args, context, info ) { - // NOTE: we're redacting the field (returning null) rather than throwing a full error which would invalidate the request. - if ( context.userId === parent.id ) { - try { - await validateScopes( context.scopes, 'profile:email' ) - return parent.email - } catch ( err ) { - return null - } - } - - try { - await validateScopes( context.scopes, 'users:email' ) - return parent.email - } catch ( err ) { - return null - } - }, - - async role( parent, args, context, info ) { - return await getUserRole( parent.id ) - } + async role( parent, args, context, info ) { + return await getUserRole( parent.id ) + } - }, + }, - Mutation: { - async userEdit( parent, args, context, info ) { - await validateServerRole( context, 'server:user' ) - await updateUser( context.userId, args.user ) - return true - } + Mutation: { + async userEdit( parent, args, context, info ) { + await validateServerRole( context, 'server:user' ) + await updateUser( context.userId, args.user ) + return true } + } } \ No newline at end of file diff --git a/modules/core/services/users.js b/modules/core/services/users.js index b19adc4402..e91e0fe8f5 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -9,98 +9,97 @@ const ServerRoles = () => knex( 'server_acl' ) module.exports = { - /* + /* Users */ - async createUser( user ) { - let [ {count} ] = await ServerRoles().where( {role: 'server:admin'} ).count() - - user.id = crs( {length: 10} ) - - if ( user.password ) { - user.passwordDigest = await bcrypt.hash( user.password, 10 ) - } - delete user.password - - let usr = await Users().select( 'id' ).where( {email: user.email} ).first() - if ( usr ) throw new Error( 'Email taken. Try logging in?' ) - - let res = await Users().returning( 'id' ).insert( user ) - - if ( parseInt( count ) === 0 ) { - await ServerRoles().insert( {userId: res[0], role: 'server:admin'} ) - } else { - await ServerRoles().insert( {userId: res[0], role: 'server:user'} ) - } - - return res[0] - }, - - async findOrCreateUser( {user, rawProfile} ) { - let existingUser = await Users().select( 'id' ).where( {email: user.email} ).first() - - if ( existingUser ) - return existingUser - - user.password = crs( {length: 20} ) - user.verified = true // because we trust the external identity provider, no? - return {id: await module.exports.createUser( user )} - }, - - async getUserById( {userId} ) { - let user = await Users().where( {id: userId} ).select( '*' ).first() - delete user.passwordDigest - return user - }, - - // TODO: deprecate - async getUser( id ) { - let user = await Users().where( {id: id} ).select( '*' ).first() - delete user.passwordDigest - return user - }, - - async getUserByEmail( {email} ) { - let user = await Users().where( {email: email} ).select( '*' ).first() - delete user.passwordDigest - return user - }, - - async getUserRole( id ) { - let {role} = await ServerRoles().where( {userId: id} ).select( 'role' ).first() - return role - }, - - async updateUser( id, user ) { - delete user.id - delete user.passwordDigest - delete user.password - delete user.email - await Users().where( {id: id} ).update( user ) - }, - - async searchUsers( query, limit ) { - - limit = limit || 100 - let likeQuery = "%" + query + "%" - let users = await Users() - .where( {email: query} ) //match full email or partial username / name - .orWhere( 'username', 'like', likeQuery ) - .orWhere( 'name', 'like', likeQuery ) - .limit( limit ) - - return users - }, - - async validatePasssword( {email, password} ) { - let {passwordDigest} = await Users().where( {email: email} ).select( 'passwordDigest' ).first() - return bcrypt.compare( password, passwordDigest ) - }, - - async deleteUser( id ) { - throw new Error( 'not implemented' ) + async createUser( user ) { + let [ {count} ] = await ServerRoles().where( {role: 'server:admin'} ).count() + + user.id = crs( {length: 10} ) + + if ( user.password ) { + user.passwordDigest = await bcrypt.hash( user.password, 10 ) } + delete user.password + + let usr = await Users().select( 'id' ).where( {email: user.email} ).first() + if ( usr ) throw new Error( 'Email taken. Try logging in?' ) + + let res = await Users().returning( 'id' ).insert( user ) + + if ( parseInt( count ) === 0 ) { + await ServerRoles().insert( {userId: res[0], role: 'server:admin'} ) + } else { + await ServerRoles().insert( {userId: res[0], role: 'server:user'} ) + } + + return res[0] + }, + + async findOrCreateUser( {user, rawProfile} ) { + let existingUser = await Users().select( 'id' ).where( {email: user.email} ).first() + + if ( existingUser ) + return existingUser + + user.password = crs( {length: 20} ) + user.verified = true // because we trust the external identity provider, no? + return {id: await module.exports.createUser( user )} + }, + + async getUserById( {userId} ) { + let user = await Users().where( {id: userId} ).select( '*' ).first() + delete user.passwordDigest + return user + }, + + // TODO: deprecate + async getUser( id ) { + let user = await Users().where( {id: id} ).select( '*' ).first() + delete user.passwordDigest + return user + }, + + async getUserByEmail( {email} ) { + let user = await Users().where( {email: email} ).select( '*' ).first() + delete user.passwordDigest + return user + }, + + async getUserRole( id ) { + let {role} = await ServerRoles().where( {userId: id} ).select( 'role' ).first() + return role + }, + + async updateUser( id, user ) { + delete user.id + delete user.passwordDigest + delete user.password + delete user.email + await Users().where( {id: id} ).update( user ) + }, + + async searchUsers( query, limit ) { + limit = limit || 100 + let likeQuery = "%" + query + "%" + let users = await Users() + .where( {email: query} ) //match full email or partial username / name + .orWhere( 'username', 'like', likeQuery ) + .orWhere( 'name', 'like', likeQuery ) + .limit( limit ) + + return users + }, + + async validatePasssword( {email, password} ) { + let {passwordDigest} = await Users().where( {email: email} ).select( 'passwordDigest' ).first() + return bcrypt.compare( password, passwordDigest ) + }, + + async deleteUser( id ) { + throw new Error( 'not implemented' ) + } } \ No newline at end of file From e51345cb9e2e3491c18e598eaebce30114edcfd6 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Tue, 28 Jul 2020 16:20:45 +0100 Subject: [PATCH 7/9] feat(gql): adds streamSearch, adds pagination to userSearch --- modules/core/graph/resolvers/streams.js | 24 +++++++++++++ .../graph/resolvers/{user.js => users.js} | 18 ++++------ modules/core/graph/schemas/streams.graphql | 7 ++-- modules/core/graph/schemas/user.graphql | 7 +++- modules/core/services/branches.js | 1 - modules/core/services/streams.js | 31 +++++++++++----- modules/core/services/users.js | 35 +++++++++++-------- 7 files changed, 85 insertions(+), 38 deletions(-) rename modules/core/graph/resolvers/{user.js => users.js} (88%) diff --git a/modules/core/graph/resolvers/streams.js b/modules/core/graph/resolvers/streams.js index df53641edb..5f54209692 100644 --- a/modules/core/graph/resolvers/streams.js +++ b/modules/core/graph/resolvers/streams.js @@ -22,6 +22,28 @@ module.exports = { let stream = await getStream( { streamId: args.id } ) return stream + }, + async streams( parent, args, context, info ) { + await validateScopes( context.scopes, 'streams:read' ) + + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 results.' ) + + let totalCount = await getUserStreamsCount( {userId: context.userId, publicOnly: false} ) + + let {cursor, streams} = await getUserStreams( {userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false} ) + return {totalCount, cursor: cursor, items: streams} + }, + async streamSearch( parent, args, context, info ) { + await validateScopes( context.scopes, 'streams:read' ) + + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 results.' ) + + let totalCount = await getUserStreamsCount( {userId: context.userId, publicOnly: false, searchQuery: args.query} ) + + let {cursor, streams} = await getUserStreams( {userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false, searchQuery: args.query} ) + return {totalCount, cursor: cursor, items: streams} } }, Stream: { @@ -35,6 +57,8 @@ module.exports = { User: { async streams( parent, args, context, info ) { + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 results.' ) // Return only the user's public streams if parent.id !== context.userId let publicOnly = parent.id !== context.userId let totalCount = await getUserStreamsCount( { userId: parent.id, publicOnly } ) diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/users.js similarity index 88% rename from modules/core/graph/resolvers/user.js rename to modules/core/graph/resolvers/users.js index dbbb04c672..b9a680135a 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/users.js @@ -29,24 +29,20 @@ module.exports = { return await getUser( args.id || context.userId ) }, - async userSearchResults( parent, args, context, info ) { + async userSearch( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) await validateScopes( context.scopes, 'profile:read' ) await validateScopes( context.scopes, 'users:read' ) - if ( !args.query ) { - throw new UserInputError( 'You must provide a search query.' ) - } - - if ( args.query.length < 3 ) { + if ( args.query.length < 3 ) throw new UserInputError( 'Search query must be at least 3 carachters.' ) - } + - if ( args.limit && args.limit > 100 ) { + if ( args.limit && args.limit > 100 ) throw new UserInputError( 'Cannot return more than 100 results.' ) - } - - return await searchUsers( args.query, args.limit ) + + let {cursor, users} = await searchUsers( args.query, args.limit, args.cursor ) + return {cursor: cursor, items: users} }, async userPwdStrength( parent, args, context, info ) { diff --git a/modules/core/graph/schemas/streams.graphql b/modules/core/graph/schemas/streams.graphql index 570d69f511..a27a8f0b5f 100644 --- a/modules/core/graph/schemas/streams.graphql +++ b/modules/core/graph/schemas/streams.graphql @@ -1,5 +1,7 @@ extend type Query { stream( id: String! ): Stream + streams( limit: Int! = 100, cursor: String ): StreamCollection + streamSearch( query: String!, limit: Int! = 100, cursor: String ): StreamCollection } type Stream { @@ -16,7 +18,7 @@ extend type User { """ All the streams that a user has access to. """ - streams( limit: Int! = 20, cursor: String ): StreamCollectionUser + streams( limit: Int! = 20, cursor: String ): StreamCollection } type StreamCollaborator { @@ -25,12 +27,13 @@ type StreamCollaborator { role: String! } -type StreamCollectionUser { +type StreamCollection { totalCount: Int! cursor: String items: [ Stream ] } + extend type Mutation { """ Creates a new stream. diff --git a/modules/core/graph/schemas/user.graphql b/modules/core/graph/schemas/user.graphql index a4ae70bd5d..85bd1cdb32 100644 --- a/modules/core/graph/schemas/user.graphql +++ b/modules/core/graph/schemas/user.graphql @@ -3,7 +3,7 @@ extend type Query { Gets the profile of a user. If no id argument is provided, will return the current authenticated user's profile (as extracted from the authorization header). """ user( id: String ): User - userSearchResults(query: String, limit: Int! = 100): [UserSearchResult!]! + userSearch( query: String!, limit: Int! = 100, cursor: String ): UserSearchResultCollection userPwdStrength( pwd: String! ): JSONObject } @@ -23,6 +23,11 @@ type User { role: String } +type UserSearchResultCollection { + cursor: String + items: [ UserSearchResult ] +} + type UserSearchResult { id: String! username: String diff --git a/modules/core/services/branches.js b/modules/core/services/branches.js index 435c3fe5b9..f3becdbec4 100644 --- a/modules/core/services/branches.js +++ b/modules/core/services/branches.js @@ -10,7 +10,6 @@ const BranchCommits = ( ) => knex( 'branch_commits' ) module.exports = { async createBranch( { name, description, streamId, authorId } ) { - let branch = {} branch.id = crs( { length: 10 } ) branch.streamId = streamId diff --git a/modules/core/services/streams.js b/modules/core/services/streams.js index 3add285096..2163395133 100644 --- a/modules/core/services/streams.js +++ b/modules/core/services/streams.js @@ -89,10 +89,10 @@ module.exports = { return await Streams( ).where( { id: streamId } ).del( ) }, - async getUserStreams( { userId, limit, cursor, publicOnly } ) { + async getUserStreams( {userId, limit, cursor, publicOnly, searchQuery } ) { limit = limit || 100 publicOnly = publicOnly !== false //defaults to true if not provided - + let likeQuery = "%" + searchQuery + "%" let query = Acl( ) .columns( [ { id: 'streams.id' }, 'name', 'description', 'isPublic', 'createdAt', 'updatedAt', 'role' ] ).select( ) .join( 'streams', 'stream_acl.resourceId', 'streams.id' ) @@ -104,16 +104,22 @@ module.exports = { if ( publicOnly ) query.andWhere( 'streams.isPublic', true ) + if ( searchQuery ) + query.andWhere( function () { + this.where( 'name', 'ILIKE', likeQuery ) + .orWhere( 'description', 'ILIKE', likeQuery ) + .orWhere( 'id', 'ILIKE', likeQuery ) //potentially useless? + } ) + query.orderBy( 'streams.updatedAt', 'desc' ).limit( limit ) let rows = await query return { streams: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].updatedAt.toISOString( ) : null } }, - async getUserStreamsCount( { userId, publicOnly } ) { - + async getUserStreamsCount( {userId, publicOnly, searchQuery } ) { publicOnly = publicOnly !== false //defaults to true if not provided - + let likeQuery = "%" + searchQuery + "%" let query = Acl( ).count( ) .join( 'streams', 'stream_acl.resourceId', 'streams.id' ) .where( { userId: userId } ) @@ -121,6 +127,13 @@ module.exports = { if ( publicOnly ) query.andWhere( 'streams.isPublic', true ) + if ( searchQuery ) + query.andWhere( function () { + this.where( 'name', 'ILIKE', likeQuery ) + .orWhere( 'description', 'ILIKE', likeQuery ) + .orWhere( 'id', 'ILIKE', likeQuery ) //potentially useless? + } ) + let [ res ] = await query return parseInt( res.count ) }, @@ -128,10 +141,10 @@ module.exports = { async getStreamUsers( { streamId } ) { let query = Acl( ).columns( { role: 'stream_acl.role' }, 'id', 'name' ).select( ) - .where( { resourceId: streamId } ) - .rightJoin( 'users', { 'users.id': 'stream_acl.userId' } ) - .select( 'stream_acl.role', 'username', 'name', 'id' ) - .orderBy( 'stream_acl.role' ) + .where( { resourceId: streamId } ) + .rightJoin( 'users', { 'users.id': 'stream_acl.userId' } ) + .select( 'stream_acl.role', 'username', 'name', 'id' ) + .orderBy( 'stream_acl.role' ) return await query } diff --git a/modules/core/services/users.js b/modules/core/services/users.js index e91e0fe8f5..e3d00992fc 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -5,7 +5,7 @@ const appRoot = require( 'app-root-path' ) const knex = require( `${appRoot}/db/knex` ) const Users = () => knex( 'users' ) -const ServerRoles = () => knex( 'server_acl' ) +const Acl = () => knex( 'server_acl' ) module.exports = { @@ -16,7 +16,7 @@ module.exports = { */ async createUser( user ) { - let [ {count} ] = await ServerRoles().where( {role: 'server:admin'} ).count() + let [ {count} ] = await Acl().where( {role: 'server:admin'} ).count() user.id = crs( {length: 10} ) @@ -31,9 +31,9 @@ module.exports = { let res = await Users().returning( 'id' ).insert( user ) if ( parseInt( count ) === 0 ) { - await ServerRoles().insert( {userId: res[0], role: 'server:admin'} ) + await Acl().insert( {userId: res[0], role: 'server:admin'} ) } else { - await ServerRoles().insert( {userId: res[0], role: 'server:user'} ) + await Acl().insert( {userId: res[0], role: 'server:user'} ) } return res[0] @@ -70,7 +70,7 @@ module.exports = { }, async getUserRole( id ) { - let {role} = await ServerRoles().where( {userId: id} ).select( 'role' ).first() + let {role} = await Acl().where( {userId: id} ).select( 'role' ).first() return role }, @@ -82,16 +82,23 @@ module.exports = { await Users().where( {id: id} ).update( user ) }, - async searchUsers( query, limit ) { + async searchUsers( searchQuery, limit, cursor ) { limit = limit || 100 - let likeQuery = "%" + query + "%" - let users = await Users() - .where( {email: query} ) //match full email or partial username / name - .orWhere( 'username', 'like', likeQuery ) - .orWhere( 'name', 'like', likeQuery ) - .limit( limit ) - - return users + let likeQuery = "%" + searchQuery + "%" + let query = Users() + .where( function () { + this.where( {email: searchQuery} ) //match full email or partial username / name + .orWhere( 'username', 'ILIKE', likeQuery ) + .orWhere( 'name', 'ILIKE', likeQuery ) + } ) + + if ( cursor ) + query.andWhere( 'users.createdAt', '<', cursor ) + + query.orderBy( 'users.createdAt', 'desc' ).limit( limit ) + + let rows = await query + return {users: rows, cursor: rows.length > 0 ? rows[rows.length - 1].createdAt.toISOString() : null} }, async validatePasssword( {email, password} ) { From 64279a867d8c91a1ebbb6760bdf3e0b836df8f02 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Wed, 29 Jul 2020 13:10:24 +0100 Subject: [PATCH 8/9] feat(gql): limits set to max=100 and default=25 for streams, users, branches & commits. Additional changes as per PR #14. --- modules/core/graph/resolvers/branches.js | 2 + modules/core/graph/resolvers/commits.js | 7 +- modules/core/graph/resolvers/streams.js | 20 +---- modules/core/graph/resolvers/users.js | 2 +- .../graph/schemas/branchesAndCommits.graphql | 8 +- modules/core/graph/schemas/streams.graphql | 8 +- modules/core/graph/schemas/user.graphql | 4 +- .../core/migrations/002-2020-05-18-scopes.js | 86 +++++++++---------- modules/core/services/branches.js | 2 +- modules/core/services/commits.js | 15 ++-- modules/core/services/streams.js | 20 ++--- modules/core/services/users.js | 13 +-- 12 files changed, 91 insertions(+), 96 deletions(-) diff --git a/modules/core/graph/resolvers/branches.js b/modules/core/graph/resolvers/branches.js index a1dcdd1d3e..cdf63d0a6c 100644 --- a/modules/core/graph/resolvers/branches.js +++ b/modules/core/graph/resolvers/branches.js @@ -30,6 +30,8 @@ module.exports = { Stream: { async branches( parent, args, context, info ) { + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) let { items, cursor, totalCount } = await getBranchesByStreamId( { streamId: parent.id, limit: args.limit, cursor: args.cursor } ) return { totalCount, cursor, items } diff --git a/modules/core/graph/resolvers/commits.js b/modules/core/graph/resolvers/commits.js index aa676faecc..acc7f0e2d6 100644 --- a/modules/core/graph/resolvers/commits.js +++ b/modules/core/graph/resolvers/commits.js @@ -32,6 +32,8 @@ module.exports = { Stream: { async commits( parent, args, context, info ) { + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) let { commits: items, cursor } = await getCommitsByStreamId( { streamId: parent.id, limit: args.limit, cursor: args.cursor } ) let totalCount = await getCommitsTotalCountByStreamId( { streamId: parent.id } ) @@ -47,9 +49,10 @@ module.exports = { User: { async commits( parent, args, context, info ) { - let publicOnly = context.userId !== parent.id let totalCount = await getCommitsTotalCountByUserId( { userId: parent.id } ) + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) let { commits: items, cursor } = await getCommitsByUserId( { userId: parent.id, limit: args.limit, cursor: args.cursor, publicOnly } ) return { items, cursor, totalCount } @@ -58,6 +61,8 @@ module.exports = { }, Branch: { async commits( parent, args, context, info ) { + if ( args.limit && args.limit > 100 ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) let { commits, cursor } = await getCommitsByBranchId( { branchId: parent.id, limit: args.limit, cursor: args.cursor } ) let totalCount = await getCommitsTotalCountByBranchId( { branchId: parent.id } ) diff --git a/modules/core/graph/resolvers/streams.js b/modules/core/graph/resolvers/streams.js index 5f54209692..424c2479e4 100644 --- a/modules/core/graph/resolvers/streams.js +++ b/modules/core/graph/resolvers/streams.js @@ -27,25 +27,13 @@ module.exports = { await validateScopes( context.scopes, 'streams:read' ) if ( args.limit && args.limit > 100 ) - throw new UserInputError( 'Cannot return more than 100 results.' ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) - let totalCount = await getUserStreamsCount( {userId: context.userId, publicOnly: false} ) + let totalCount = await getUserStreamsCount({userId: context.userId, publicOnly: false, searchQuery: args.query} ) - let {cursor, streams} = await getUserStreams( {userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false} ) + let {cursor, streams} = await getUserStreams({userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false, searchQuery: args.query} ) return {totalCount, cursor: cursor, items: streams} }, - async streamSearch( parent, args, context, info ) { - await validateScopes( context.scopes, 'streams:read' ) - - if ( args.limit && args.limit > 100 ) - throw new UserInputError( 'Cannot return more than 100 results.' ) - - let totalCount = await getUserStreamsCount( {userId: context.userId, publicOnly: false, searchQuery: args.query} ) - - let {cursor, streams} = await getUserStreams( {userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false, searchQuery: args.query} ) - return {totalCount, cursor: cursor, items: streams} - } - }, Stream: { async collaborators( parent, args, context, info ) { @@ -58,7 +46,7 @@ module.exports = { async streams( parent, args, context, info ) { if ( args.limit && args.limit > 100 ) - throw new UserInputError( 'Cannot return more than 100 results.' ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) // Return only the user's public streams if parent.id !== context.userId let publicOnly = parent.id !== context.userId let totalCount = await getUserStreamsCount( { userId: parent.id, publicOnly } ) diff --git a/modules/core/graph/resolvers/users.js b/modules/core/graph/resolvers/users.js index b9a680135a..2f5dfc66c7 100644 --- a/modules/core/graph/resolvers/users.js +++ b/modules/core/graph/resolvers/users.js @@ -39,7 +39,7 @@ module.exports = { if ( args.limit && args.limit > 100 ) - throw new UserInputError( 'Cannot return more than 100 results.' ) + throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) let {cursor, users} = await searchUsers( args.query, args.limit, args.cursor ) return {cursor: cursor, items: users} diff --git a/modules/core/graph/schemas/branchesAndCommits.graphql b/modules/core/graph/schemas/branchesAndCommits.graphql index 5fb953dd03..c15b4a59f9 100644 --- a/modules/core/graph/schemas/branchesAndCommits.graphql +++ b/modules/core/graph/schemas/branchesAndCommits.graphql @@ -1,12 +1,12 @@ extend type Stream { - commits( limit: Int! = 20, cursor: String ): CommitCollection + commits( limit: Int! = 25, cursor: String ): CommitCollection commit( id: String! ): Commit - branches( limit: Int! = 100, cursor: String ): BranchCollection + branches( limit: Int! = 25, cursor: String ): BranchCollection branch( name: String! ): Branch } extend type User { - commits( limit: Int! = 20, cursor: String ): CommitCollectionUser + commits( limit: Int! = 25, cursor: String ): CommitCollectionUser } type Branch { @@ -14,7 +14,7 @@ type Branch { name: String! author: User! description: String! - commits( limit: Int! = 20, cursor: String ): CommitCollection + commits( limit: Int! = 25, cursor: String ): CommitCollection } type Commit { diff --git a/modules/core/graph/schemas/streams.graphql b/modules/core/graph/schemas/streams.graphql index a27a8f0b5f..a4893dfffe 100644 --- a/modules/core/graph/schemas/streams.graphql +++ b/modules/core/graph/schemas/streams.graphql @@ -1,7 +1,9 @@ extend type Query { stream( id: String! ): Stream - streams( limit: Int! = 100, cursor: String ): StreamCollection - streamSearch( query: String!, limit: Int! = 100, cursor: String ): StreamCollection + """ + All the streams of the current user, pass in the `query` parameter to seach by name, description or ID. + """ + streams( query: String!, limit: Int! = 25, cursor: String ): StreamCollection } type Stream { @@ -18,7 +20,7 @@ extend type User { """ All the streams that a user has access to. """ - streams( limit: Int! = 20, cursor: String ): StreamCollection + streams( limit: Int! = 25, cursor: String ): StreamCollection } type StreamCollaborator { diff --git a/modules/core/graph/schemas/user.graphql b/modules/core/graph/schemas/user.graphql index 85bd1cdb32..33cc389908 100644 --- a/modules/core/graph/schemas/user.graphql +++ b/modules/core/graph/schemas/user.graphql @@ -3,7 +3,7 @@ extend type Query { Gets the profile of a user. If no id argument is provided, will return the current authenticated user's profile (as extracted from the authorization header). """ user( id: String ): User - userSearch( query: String!, limit: Int! = 100, cursor: String ): UserSearchResultCollection + userSearch( query: String!, limit: Int! = 25, cursor: String ): UserSearchResultCollection userPwdStrength( pwd: String! ): JSONObject } @@ -36,8 +36,6 @@ type UserSearchResult { company: String avatar: String verified: Boolean - profiles: JSONObject - role: String } extend type Mutation { diff --git a/modules/core/migrations/002-2020-05-18-scopes.js b/modules/core/migrations/002-2020-05-18-scopes.js index 7a879c3fb6..2def799c53 100644 --- a/modules/core/migrations/002-2020-05-18-scopes.js +++ b/modules/core/migrations/002-2020-05-18-scopes.js @@ -6,50 +6,50 @@ exports.up = async knex => { debug( 'Setting up core module scopes.' ) let coreModuleScopes = [ { - name: 'server:setup', - description: 'Edit server information.' - }, - { - name: 'tokens:read', - description: `Access your api tokens.` - }, - { - name: 'tokens:write', - description: `Create and delete api tokens on your behalf.` - }, - { - name: 'apps:authorize', - description: 'Grant third party applications access rights on your behalf to the api.' - }, - { - name: 'apps:create', - description: 'Register a third party application.' - }, - { - name: 'streams:read', - description: 'Read your streams & and any associated information (branches, tags, comments, objects, etc.)' - }, - { - name: 'streams:write', - description: 'Create streams on your behalf and read your streams & any associated information (any associated information (branches, tags, comments, objects, etc.)' - }, - { - name: 'profile:read', - description: `Read your profile information` - }, - { - name: 'profile:email', - description: `Access your email.` - }, + name: 'server:setup', + description: 'Edit server information.' + }, + { + name: 'tokens:read', + description: `Access your api tokens.` + }, + { + name: 'tokens:write', + description: `Create and delete api tokens on your behalf.` + }, + { + name: 'apps:authorize', + description: 'Grant third party applications access rights on your behalf to the api.' + }, + { + name: 'apps:create', + description: 'Register a third party application.' + }, + { + name: 'streams:read', + description: 'Read your streams & and any associated information (branches, tags, comments, objects, etc.)' + }, + { + name: 'streams:write', + description: 'Create streams on your behalf and read your streams & any associated information (any associated information (branches, tags, comments, objects, etc.)' + }, + { + name: 'profile:read', + description: `Read your profile information` + }, + { + name: 'profile:email', + description: `Access your email.` + }, - { - name: 'users:read', - description: `Read other users' profile on your behalf.` - }, - { - name: 'users:email', - description: 'Access the emails of other users.' - } + { + name: 'users:read', + description: `Read other users' profile on your behalf.` + }, + { + name: 'users:email', + description: 'Access the emails of other users.' + } ] await knex( 'scopes' ).insert( coreModuleScopes ) diff --git a/modules/core/services/branches.js b/modules/core/services/branches.js index f3becdbec4..3434877d34 100644 --- a/modules/core/services/branches.js +++ b/modules/core/services/branches.js @@ -31,7 +31,7 @@ module.exports = { }, async getBranchesByStreamId( { streamId, limit, cursor } ) { - limit = limit || 100 + limit = limit || 25 let query = Branches( ).select( '*' ).where( { streamId: streamId } ) if ( cursor ) diff --git a/modules/core/services/commits.js b/modules/core/services/commits.js index 1c4acb0a56..2d3f035c52 100644 --- a/modules/core/services/commits.js +++ b/modules/core/services/commits.js @@ -15,7 +15,6 @@ const { getBranchesByStreamId, getBranchByNameAndStreamId } = require( './branch module.exports = { async createCommitByBranchId( { streamId, branchId, objectId, authorId, message, previousCommitIds } ) { - // Create main table entry let [ id ] = await Commits( ).returning( 'id' ).insert( { id: crs( { length: 10 } ), @@ -84,7 +83,7 @@ module.exports = { }, async getCommitsByBranchId( { branchId, limit, cursor } ) { - limit = limit || 20 + limit = limit || 25 let query = BranchCommits( ).columns( [ { id: 'commitId' }, 'message', 'referencedObject', { authorName: 'name' }, { authorId: 'users.id' }, 'commits.createdAt' ] ).select( ) .join( 'commits', 'commits.id', 'branch_commits.commitId' ) .join( 'users', 'commits.author', 'users.id' ) @@ -124,7 +123,7 @@ module.exports = { * @return {[type]} [description] */ async getCommitsByStreamId( { streamId, limit, cursor } ) { - limit = limit || 20 + limit = limit || 25 let query = StreamCommits( ) .columns( [ { id: 'commitId' }, 'message', 'referencedObject', { authorName: 'name' }, { authorId: 'users.id' }, 'commits.createdAt' ] ).select( ) .join( 'commits', 'commits.id', 'stream_commits.commitId' ) @@ -142,15 +141,15 @@ module.exports = { }, async getCommitsByUserId( { userId, limit, cursor, publicOnly } ) { - limit = limit || 20 + limit = limit || 25 publicOnly = publicOnly !== false let query = Commits( ) - .columns( [ { id: 'commitId' }, 'message', 'referencedObject', 'commits.createdAt', { streamId: 'stream_commits.streamId' }, { streamName: 'streams.name' } ] ).select( ) - .join( 'stream_commits', 'commits.id', 'stream_commits.commitId' ) - .join( 'streams', 'stream_commits.streamId', 'streams.id' ) - .where( 'author', userId ) + .columns( [ { id: 'commitId' }, 'message', 'referencedObject', 'commits.createdAt', { streamId: 'stream_commits.streamId' }, { streamName: 'streams.name' } ] ).select( ) + .join( 'stream_commits', 'commits.id', 'stream_commits.commitId' ) + .join( 'streams', 'stream_commits.streamId', 'streams.id' ) + .where( 'author', userId ) if ( publicOnly ) query.andWhere( 'streams.isPublic', true ) diff --git a/modules/core/services/streams.js b/modules/core/services/streams.js index 2163395133..02cdce03e6 100644 --- a/modules/core/services/streams.js +++ b/modules/core/services/streams.js @@ -89,10 +89,10 @@ module.exports = { return await Streams( ).where( { id: streamId } ).del( ) }, - async getUserStreams( {userId, limit, cursor, publicOnly, searchQuery } ) { - limit = limit || 100 + async getUserStreams( { userId, limit, cursor, publicOnly, searchQuery } ) { + limit = limit || 25 publicOnly = publicOnly !== false //defaults to true if not provided - let likeQuery = "%" + searchQuery + "%" + let query = Acl( ) .columns( [ { id: 'streams.id' }, 'name', 'description', 'isPublic', 'createdAt', 'updatedAt', 'role' ] ).select( ) .join( 'streams', 'stream_acl.resourceId', 'streams.id' ) @@ -106,9 +106,9 @@ module.exports = { if ( searchQuery ) query.andWhere( function () { - this.where( 'name', 'ILIKE', likeQuery ) - .orWhere( 'description', 'ILIKE', likeQuery ) - .orWhere( 'id', 'ILIKE', likeQuery ) //potentially useless? + this.where( 'name', 'ILIKE', `%${ searchQuery }%` ) + .orWhere( 'description', 'ILIKE', `%${searchQuery}%` ) + .orWhere( 'id', 'ILIKE', `%${searchQuery}%` ) //potentially useless? } ) query.orderBy( 'streams.updatedAt', 'desc' ).limit( limit ) @@ -119,7 +119,7 @@ module.exports = { async getUserStreamsCount( {userId, publicOnly, searchQuery } ) { publicOnly = publicOnly !== false //defaults to true if not provided - let likeQuery = "%" + searchQuery + "%" + let query = Acl( ).count( ) .join( 'streams', 'stream_acl.resourceId', 'streams.id' ) .where( { userId: userId } ) @@ -129,9 +129,9 @@ module.exports = { if ( searchQuery ) query.andWhere( function () { - this.where( 'name', 'ILIKE', likeQuery ) - .orWhere( 'description', 'ILIKE', likeQuery ) - .orWhere( 'id', 'ILIKE', likeQuery ) //potentially useless? + this.where( 'name', 'ILIKE', `%${searchQuery}%` ) + .orWhere( 'description', 'ILIKE', `%${searchQuery}%` ) + .orWhere( 'id', 'ILIKE', `%${searchQuery}%` ) //potentially useless? } ) let [ res ] = await query diff --git a/modules/core/services/users.js b/modules/core/services/users.js index e3d00992fc..c61e3cccab 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -83,13 +83,14 @@ module.exports = { }, async searchUsers( searchQuery, limit, cursor ) { - limit = limit || 100 - let likeQuery = "%" + searchQuery + "%" + limit = limit || 25 + let query = Users() - .where( function () { - this.where( {email: searchQuery} ) //match full email or partial username / name - .orWhere( 'username', 'ILIKE', likeQuery ) - .orWhere( 'name', 'ILIKE', likeQuery ) + .select( 'id username name bio company verified avatar' ) + .where( queryBuilder => { + queryBuilder.where( {email: searchQuery} ) //match full email or partial username / name + queryBuilder.orWhere( 'username', 'ILIKE', `%${searchQuery}%` ) + queryBuilder.orWhere( 'name', 'ILIKE', `%${searchQuery}%` ) } ) if ( cursor ) From 80b293c592dfd70c423859943ee2b4aeef72c26d Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Wed, 29 Jul 2020 18:56:58 +0100 Subject: [PATCH 9/9] feat(gql): adds tests for stream search and user search --- modules/core/graph/resolvers/streams.js | 9 +++++---- modules/core/services/users.js | 2 +- modules/core/tests/streams.spec.js | 9 ++++++++- modules/core/tests/users.spec.js | 8 +++++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/modules/core/graph/resolvers/streams.js b/modules/core/graph/resolvers/streams.js index 424c2479e4..b37d508680 100644 --- a/modules/core/graph/resolvers/streams.js +++ b/modules/core/graph/resolvers/streams.js @@ -29,11 +29,12 @@ module.exports = { if ( args.limit && args.limit > 100 ) throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) - let totalCount = await getUserStreamsCount({userId: context.userId, publicOnly: false, searchQuery: args.query} ) + let totalCount = await getUserStreamsCount( {userId: context.userId, publicOnly: false, searchQuery: args.query} ) - let {cursor, streams} = await getUserStreams({userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false, searchQuery: args.query} ) + let {cursor, streams} = await getUserStreams( {userId: context.userId, limit: args.limit, cursor: args.cursor, publicOnly: false, searchQuery: args.query} ) return {totalCount, cursor: cursor, items: streams} - }, + } + }, Stream: { async collaborators( parent, args, context, info ) { @@ -47,7 +48,7 @@ module.exports = { async streams( parent, args, context, info ) { if ( args.limit && args.limit > 100 ) throw new UserInputError( 'Cannot return more than 100 items, please use pagination.' ) - // Return only the user's public streams if parent.id !== context.userId + // Return only the user's public streams if parent.id !== context.userId let publicOnly = parent.id !== context.userId let totalCount = await getUserStreamsCount( { userId: parent.id, publicOnly } ) diff --git a/modules/core/services/users.js b/modules/core/services/users.js index c61e3cccab..c3628f8722 100644 --- a/modules/core/services/users.js +++ b/modules/core/services/users.js @@ -86,7 +86,7 @@ module.exports = { limit = limit || 25 let query = Users() - .select( 'id username name bio company verified avatar' ) + .select( 'id', 'username', 'name', 'bio', 'company', 'verified', 'avatar', 'createdAt' ) .where( queryBuilder => { queryBuilder.where( {email: searchQuery} ) //match full email or partial username / name queryBuilder.orWhere( 'username', 'ILIKE', `%${searchQuery}%` ) diff --git a/modules/core/tests/streams.spec.js b/modules/core/tests/streams.spec.js index 828c182ec4..a3570bebaf 100644 --- a/modules/core/tests/streams.spec.js +++ b/modules/core/tests/streams.spec.js @@ -65,13 +65,20 @@ describe( 'Streams', ( ) => { expect( stream.description ).to.equal( 'Wooot' ) } ) - it( 'Should get all streams for a user', async ( ) => { + it( 'Should get all streams of a user', async ( ) => { let { streams, cursor } = await getUserStreams( { userId: userOne.id } ) // console.log( res ) expect( streams ).to.have.lengthOf( 2 ) expect( cursor ).to.exist } ) + it('Should search all streams of a user', async () => { + let {streams, cursor} = await getUserStreams({userId: userOne.id, searchQuery: "woo"}) + // console.log( res ) + expect(streams).to.have.lengthOf(1) + expect(cursor).to.exist + }) + it( 'Should delete a stream', async ( ) => { const id = await createStream( { name: 'mayfly', description: 'wonderful', ownerId: userOne.id } ) let all = await getUserStreams( { userId: userOne.id } ) diff --git a/modules/core/tests/users.spec.js b/modules/core/tests/users.spec.js index d41a73909f..aaf30dd78f 100644 --- a/modules/core/tests/users.spec.js +++ b/modules/core/tests/users.spec.js @@ -11,7 +11,7 @@ chai.use( chaiHttp ) const knex = require( `${appRoot}/db/knex` ) -const { createUser, getUser, updateUser, deleteUser, validatePasssword } = require( '../services/users' ) +const {createUser, getUser, searchUsers, updateUser, deleteUser, validatePasssword } = require( '../services/users' ) const { createPersonalAccessToken, createAppToken, revokeToken, revokeTokenById, validateToken, getUserTokens } = require( '../services/tokens' ) describe( 'Actors & Tokens', ( ) => { @@ -61,6 +61,12 @@ describe( 'Actors & Tokens', ( ) => { expect( actor ).to.not.have.property( 'passwordDigest' ) } ) + it('Should search and get an users', async () => { + let {users} = await searchUsers("gates", 20, null) + expect(users).to.have.lengthOf(1) + expect(users[0].name).to.equal("Bill Gates") + }) + it( 'Should update an actor', async ( ) => { let updatedActor = { ...myTestActor } updatedActor.username = 'didimitrie'