From 057d8cabd2a0f952b1675c7716bf5b9ae38a98fc Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2017 11:47:51 -0600 Subject: [PATCH] Increased debugging and optimized a query path --- graph/loaders/users.js | 17 +++- graph/setupFunctions.js | 117 +++++++++++++++++++++++ graph/subscriptions.js | 180 +++++++---------------------------- middleware/authentication.js | 6 ++ services/passport.js | 4 + 5 files changed, 178 insertions(+), 146 deletions(-) create mode 100644 graph/setupFunctions.js diff --git a/graph/loaders/users.js b/graph/loaders/users.js index f11effbd1..999ad1938 100644 --- a/graph/loaders/users.js +++ b/graph/loaders/users.js @@ -5,9 +5,20 @@ const util = require('./util'); const UsersService = require('../../services/users'); const UserModel = require('../../models/user'); -const genUserByIDs = (context, ids) => UsersService - .findByIdArray(ids) - .then(util.singleJoinBy(ids, 'id')); +const genUserByIDs = async (context, ids) => { + if (!ids || ids.length === 0) { + return []; + } + + if (ids.length === 1) { + const user = await UsersService.findById(ids[0]); + return [user]; + } + + return UsersService + .findByIdArray(ids) + .then(util.singleJoinBy(ids, 'id')); +}; /** * Retrieves users based on the passed in query that is filtered by the diff --git a/graph/setupFunctions.js b/graph/setupFunctions.js new file mode 100644 index 000000000..273aedfa2 --- /dev/null +++ b/graph/setupFunctions.js @@ -0,0 +1,117 @@ +const { + SUBSCRIBE_COMMENT_ACCEPTED, + SUBSCRIBE_COMMENT_REJECTED, + SUBSCRIBE_COMMENT_FLAGGED, + SUBSCRIBE_ALL_COMMENT_EDITED, + SUBSCRIBE_ALL_COMMENT_ADDED, + SUBSCRIBE_ALL_USER_SUSPENDED, + SUBSCRIBE_ALL_USER_BANNED, + SUBSCRIBE_ALL_USERNAME_REJECTED, +} = require('../perms/constants'); + +const merge = require('lodash/merge'); +const debug = require('debug')('talk:graph:setupFunctions'); +const plugins = require('../services/plugins'); + +/** + * Plugin support requires that we merge in existing setupFunctions with our new + * plugin based ones. This allows plugins to extend existing setupFunctions as well + * as provide new ones. + */ +const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plugin, setupFunctions}) => { + debug(`added plugin '${plugin.name}'`); + + return merge(acc, setupFunctions); +}, { + commentAdded: (options, args) => ({ + commentAdded: { + filter: (comment, context) => { + if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_ADDED))) { + return false; + } + return !args.asset_id || comment.asset_id === args.asset_id; + } + }, + }), + commentEdited: (options, args) => ({ + commentEdited: { + filter: (comment, context) => { + if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_EDITED))) { + return false; + } + return !args.asset_id || comment.asset_id === args.asset_id; + } + }, + }), + commentFlagged: (options, args) => ({ + commentFlagged: { + filter: (comment, context) => { + if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_FLAGGED)) { + return false; + } + return !args.asset_id || comment.asset_id === args.asset_id; + } + }, + }), + commentAccepted: (options, args) => ({ + commentAccepted: { + filter: (comment, context) => { + if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_ACCEPTED)) { + return false; + } + return !args.asset_id || comment.asset_id === args.asset_id; + } + }, + }), + commentRejected: (options, args) => ({ + commentRejected: { + filter: (comment, context) => { + if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_REJECTED)) { + return false; + } + return !args.asset_id || comment.asset_id === args.asset_id; + } + }, + }), + userSuspended: (options, args) => ({ + userSuspended: { + filter: (user, context) => { + if ( + !context.user + || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_SUSPENDED) + ) { + return false; + } + return !args.user_id || user.id === args.user_id; + } + }, + }), + userBanned: (options, args) => ({ + userBanned: { + filter: (user, context) => { + if ( + !context.user + || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_BANNED) + ) { + return false; + } + return !args.user_id || user.id === args.user_id; + } + }, + }), + usernameRejected: (options, args) => ({ + usernameRejected: { + filter: (user, context) => { + if ( + !context.user + || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USERNAME_REJECTED) + ) { + return false; + } + return !args.user_id || user.id === args.user_id; + } + }, + }), +}); + +module.exports = setupFunctions; diff --git a/graph/subscriptions.js b/graph/subscriptions.js index 71b8abc6a..59dd6e923 100644 --- a/graph/subscriptions.js +++ b/graph/subscriptions.js @@ -1,133 +1,56 @@ const {SubscriptionManager} = require('graphql-subscriptions'); const {SubscriptionServer} = require('subscriptions-transport-ws'); -const _ = require('lodash'); const debug = require('debug')('talk:graph:subscriptions'); const pubsub = require('../services/pubsub'); const schema = require('./schema'); const Context = require('./context'); -const plugins = require('../services/plugins'); const {deserializeUser} = require('../services/subscriptions'); +const setupFunctions = require('./setupFunctions'); const ms = require('ms'); const { KEEP_ALIVE } = require('../config'); -const { - SUBSCRIBE_COMMENT_ACCEPTED, - SUBSCRIBE_COMMENT_REJECTED, - SUBSCRIBE_COMMENT_FLAGGED, - SUBSCRIBE_ALL_COMMENT_EDITED, - SUBSCRIBE_ALL_COMMENT_ADDED, - SUBSCRIBE_ALL_USER_SUSPENDED, - SUBSCRIBE_ALL_USER_BANNED, - SUBSCRIBE_ALL_USERNAME_REJECTED, -} = require('../perms/constants'); - const {BASE_PATH} = require('../url'); -/** - * Plugin support requires that we merge in existing setupFunctions with our new - * plugin based ones. This allows plugins to extend existing setupFunctions as well - * as provide new ones. - */ -const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plugin, setupFunctions}) => { - debug(`added plugin '${plugin.name}'`); +const onConnect = ({token}, connection) => { - return _.merge(acc, setupFunctions); -}, { - commentAdded: (options, args) => ({ - commentAdded: { - filter: (comment, context) => { - if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_ADDED))) { - return false; - } - return !args.asset_id || comment.asset_id === args.asset_id; - } - }, - }), - commentEdited: (options, args) => ({ - commentEdited: { - filter: (comment, context) => { - if (!args.asset_id && (!context.user || !context.user.can(SUBSCRIBE_ALL_COMMENT_EDITED))) { - return false; - } - return !args.asset_id || comment.asset_id === args.asset_id; - } - }, - }), - commentFlagged: (options, args) => ({ - commentFlagged: { - filter: (comment, context) => { - if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_FLAGGED)) { - return false; - } - return !args.asset_id || comment.asset_id === args.asset_id; - } - }, - }), - commentAccepted: (options, args) => ({ - commentAccepted: { - filter: (comment, context) => { - if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_ACCEPTED)) { - return false; - } - return !args.asset_id || comment.asset_id === args.asset_id; - } - }, - }), - commentRejected: (options, args) => ({ - commentRejected: { - filter: (comment, context) => { - if (!context.user || !context.user.can(SUBSCRIBE_COMMENT_REJECTED)) { - return false; - } - return !args.asset_id || comment.asset_id === args.asset_id; - } - }, - }), - userSuspended: (options, args) => ({ - userSuspended: { - filter: (user, context) => { - if ( - !context.user - || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_SUSPENDED) - ) { - return false; - } - return !args.user_id || user.id === args.user_id; - } - }, - }), - userBanned: (options, args) => ({ - userBanned: { - filter: (user, context) => { - if ( - !context.user - || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USER_BANNED) - ) { - return false; - } - return !args.user_id || user.id === args.user_id; - } - }, - }), - usernameRejected: (options, args) => ({ - usernameRejected: { - filter: (user, context) => { - if ( - !context.user - || args.user_id !== user.id && !context.user.can(SUBSCRIBE_ALL_USERNAME_REJECTED) - ) { - return false; - } - return !args.user_id || user.id === args.user_id; - } - }, - }), -}); + // Attach the token from the connection options if it was provided. + if (token) { + + debug('token sent via onConnect, attaching to the headers of the upgrade request'); + + // Attach it to the upgrade request. + connection.upgradeReq.headers['authorization'] = `Bearer ${token}`; + } +}; + +const onOperation = (parsedMessage, baseParams, connection) => { + + // Cache the upgrade request. + let upgradeReq = connection.upgradeReq; + + // Attach the context per request. + baseParams.context = async () => { + let req; + + try { + req = await deserializeUser(upgradeReq); + debug(`user ${req.user ? 'was' : 'was not'} on websocket request`); + } catch (e) { + console.error(e); + + return new Context({}); + } + + return new Context(req); + }; + + return baseParams; +}; /** * This creates a new subscription manager. @@ -138,37 +61,8 @@ const createSubscriptionManager = (server) => new SubscriptionServer({ pubsub: pubsub.getClient(), setupFunctions, }), - onConnect: ({token}, connection) => { - - // Attach the token from the connection options if it was provided. - if (token) { - - // Attach it to the upgrade request. - connection.upgradeReq.headers['authorization'] = `Bearer ${token}`; - } - }, - onOperation: (parsedMessage, baseParams, connection) => { - - // Cache the upgrade request. - let upgradeReq = connection.upgradeReq; - - // Attach the context per request. - baseParams.context = async () => { - let req; - - try { - req = await deserializeUser(upgradeReq); - } catch (e) { - console.error(e); - - return new Context({}); - } - - return new Context(req); - }; - - return baseParams; - }, + onConnect, + onOperation, keepAlive: ms(KEEP_ALIVE) }, { server, diff --git a/middleware/authentication.js b/middleware/authentication.js index ca6949a5c..3e4b3fbff 100644 --- a/middleware/authentication.js +++ b/middleware/authentication.js @@ -1,16 +1,22 @@ const {passport} = require('../services/passport'); +const debug = require('debug')('talk:middleware:authentication'); const authentication = (req, res, next) => passport.authenticate('jwt', { session: false }, (err, user) => { if (err) { + debug(`cannot get the user: ${err}`); return next(err); } if (user) { + debug('user was on request'); + // Attach the user to the request object, now that we know it exists. req.user = user; + } else { + debug('user was not on request'); } next(); diff --git a/services/passport.js b/services/passport.js index 14a2134ce..4e5bbd708 100644 --- a/services/passport.js +++ b/services/passport.js @@ -54,11 +54,14 @@ const GenerateToken = (user) => { const SetTokenForSafari = (req, res, token) => { const browser = bowser._detect(req.headers['user-agent']); if (browser.ios || browser.safari) { + debug('browser was safari/ios, setting a cookie'); res.cookie(JWT_SIGNING_COOKIE_NAME, token, { httpOnly: true, secure: process.env.NODE_ENV === 'production', expires: new Date(Date.now() + ms(JWT_EXPIRY)) }); + } else { + debug('browser wasn\'t safari/ios, didn\'t set a cookie'); } }; @@ -170,6 +173,7 @@ const HandleLogout = (req, res, next) => { // Only clear the cookie on logout if enabled. if (JWT_CLEAR_COOKIE_LOGOUT) { + debug('clearing the login cookie'); res.clearCookie(JWT_SIGNING_COOKIE_NAME); }