From 86c36f7d39bf6282aeccff3910c020257fea9803 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 1 Mar 2018 14:43:29 -0700 Subject: [PATCH] Email confirmation check - filtering for confirmed emails - fixes to logger to expose logger controls - refactored graphql calls in other notification plugins --- config.js | 8 ++ graph/mutators/comment.js | 67 ++++++----- graph/resolvers/index.js | 8 +- graph/resolvers/local_user_profile.js | 7 ++ graph/resolvers/user_profile.js | 12 ++ graph/typeDefs.graphql | 27 ++++- .../index.js | 19 +--- .../index.js | 21 +--- .../index.js | 25 ++--- .../server/NotificationManager.js | 104 +++++++++++++++--- .../server/config.js | 5 + services/logging.js | 2 + 12 files changed, 213 insertions(+), 92 deletions(-) create mode 100644 graph/resolvers/local_user_profile.js create mode 100644 graph/resolvers/user_profile.js diff --git a/config.js b/config.js index 9b7c7adcc..daf06c56c 100644 --- a/config.js +++ b/config.js @@ -48,6 +48,14 @@ const CONFIG = { // request all of the records. Otherwise, minimum limits of 0 are enforced. ALLOW_NO_LIMIT_QUERIES: process.env.TALK_ALLOW_NO_LIMIT_QUERIES === 'TRUE', + // TODO: document this config. + // LOGGING_LEVEL specifies the logging level used by the bunyan logger. + LOGGING_LEVEL: ['fatal', 'error', 'warn', 'info', 'debug', 'trace'].includes( + process.env.TALK_LOGGING_LEVEL + ) + ? process.env.TALK_LOGGING_LEVEL + : 'info', + //------------------------------------------------------------------------------ // JWT based configuration //------------------------------------------------------------------------------ diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 7e0607287..a64ca427a 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -12,12 +12,9 @@ const { ADD_COMMENT_TAG, EDIT_COMMENT, } = require('../../perms/constants'); -const debug = require('debug')('talk:graph:mutators:comment'); -const resolveTagsForComment = async ( - { user, loaders: { Tags } }, - { asset_id, tags = [] } -) => { +const resolveTagsForComment = async (ctx, { asset_id, tags = [] }) => { + const { user, loaders: { Tags } } = ctx; const item_type = 'COMMENTS'; // Handle Tags @@ -46,6 +43,7 @@ const resolveTagsForComment = async ( // Add the staff tag for comments created as a staff member. if (user.can(ADD_COMMENT_TAG)) { + ctx.log.info({ author_id: user.id }, 'Added staff tag to comment'); tags.push( TagsService.newTagLink(user, { name: 'STAFF', @@ -61,7 +59,7 @@ const resolveTagsForComment = async ( * adjustKarma will adjust the affected user's karma depending on the moderators * action. */ -const adjustKarma = (Comments, id, status) => async () => { +const adjustKarma = (ctx, Comments, id, status) => async () => { try { // Use the dataloader to get the comment that was just moderated and // get the flag user's id's so we can adjust their karma too. @@ -86,17 +84,27 @@ const adjustKarma = (Comments, id, status) => async () => { }), ]); - debug(`Comment[${id}] by User[${comment.author_id}] was Status[${status}]`); + ctx.log.info( + { + state: { + comment_id: id, + author_id: comment.author_id, + status: status, + }, + }, + 'Processing karma modification' + ); switch (status) { case 'REJECTED': // Reduce the user's karma. - debug(`CommentUser[${comment.author_id}] had their karma reduced`); + ctx.log.info('Comment author had their karma reduced'); // Decrease the flag user's karma, the moderator disagreed with this // action. - debug( - `FlaggingUser[${flagUserIDs.join(', ')}] had their karma increased` + ctx.log.info( + { flagUserIDs }, + 'Flagging users had their karma increased' ); await Promise.all([ KarmaService.modifyUser(comment.author_id, -1, 'comment'), @@ -107,13 +115,11 @@ const adjustKarma = (Comments, id, status) => async () => { case 'ACCEPTED': // Increase the user's karma. - debug(`CommentUser[${comment.author_id}] had their karma increased`); + ctx.log.info('Comment author had their karma increased'); // Increase the flag user's karma, the moderator agreed with this // action. - debug( - `FlaggingUser[${flagUserIDs.join(', ')}] had their karma reduced` - ); + ctx.log.info({ flagUserIDs }, `Flagging users had their karma reduced`); await Promise.all([ KarmaService.modifyUser(comment.author_id, 1, 'comment'), KarmaService.modifyUser(flagUserIDs, -1, 'flag', true), @@ -140,7 +146,7 @@ const adjustKarma = (Comments, id, status) => async () => { * @return {Promise} resolves to the created comment */ const createComment = async ( - context, + ctx, { tags = [], body, @@ -150,10 +156,10 @@ const createComment = async ( metadata = {}, } ) => { - const { user, loaders: { Comments }, pubsub } = context; + const { user, loaders: { Comments }, pubsub } = ctx; // Resolve the tags for the comment. - tags = await resolveTagsForComment(context, { asset_id, tags }); + tags = await resolveTagsForComment(ctx, { asset_id, tags }); let comment = await CommentsService.publicCreate({ body, @@ -165,6 +171,11 @@ const createComment = async ( metadata, }); + ctx.log.info( + { comment_id: comment.id, comment_status: status }, + 'Created comment' + ); + // If the loaders are present, clear the caches for these values because we // just added a new comment, hence the counts should be updated. We should // perform these increments in the event that we do have a new comment that @@ -228,12 +239,14 @@ const createActions = async (item_id, actions = []) => /** * Sets the status of a comment - * @param {Object} context graphql context + * @param {Object} ctx graphql context * @param {String} comment comment in graphql context * @param {String} id identifier of the comment (uuid) * @param {String} status the new status of the comment */ -const setStatus = async ({ user, loaders: { Comments } }, { id, status }) => { +const setStatus = async (ctx, { id, status }) => { + const { user, loaders: { Comments } } = ctx; + let comment = await CommentsService.pushStatus( id, status, @@ -253,7 +266,7 @@ const setStatus = async ({ user, loaders: { Comments } }, { id, status }) => { // postSetCommentStatus will use the arguments from the mutation and // adjust the affected user's karma in the next tick. - process.nextTick(adjustKarma(Comments, id, status)); + process.nextTick(adjustKarma(ctx, Comments, id, status)); return comment; }; @@ -292,7 +305,7 @@ const edit = async (ctx, { id, asset_id, edit: { body } }) => { return comment; }; -module.exports = context => { +module.exports = ctx => { let mutators = { Comment: { create: () => Promise.reject(errors.ErrNotAuthorized), @@ -301,16 +314,16 @@ module.exports = context => { }, }; - if (context.user && context.user.can(CREATE_COMMENT)) { - mutators.Comment.create = comment => createPublicComment(context, comment); + if (ctx.user && ctx.user.can(CREATE_COMMENT)) { + mutators.Comment.create = comment => createPublicComment(ctx, comment); } - if (context.user && context.user.can(SET_COMMENT_STATUS)) { - mutators.Comment.setStatus = action => setStatus(context, action); + if (ctx.user && ctx.user.can(SET_COMMENT_STATUS)) { + mutators.Comment.setStatus = action => setStatus(ctx, action); } - if (context.user && context.user.can(EDIT_COMMENT)) { - mutators.Comment.edit = action => edit(context, action); + if (ctx.user && ctx.user.can(EDIT_COMMENT)) { + mutators.Comment.edit = action => edit(ctx, action); } return mutators; diff --git a/graph/resolvers/index.js b/graph/resolvers/index.js index 74b006ebb..a8765f743 100644 --- a/graph/resolvers/index.js +++ b/graph/resolvers/index.js @@ -15,6 +15,7 @@ const DontAgreeActionSummary = require('./dont_agree_action_summary'); const FlagAction = require('./flag_action'); const FlagActionSummary = require('./flag_action_summary'); const GenericUserError = require('./generic_user_error'); +const LocalUserProfile = require('./local_user_profile'); const RootMutation = require('./root_mutation'); const RootQuery = require('./root_query'); const Settings = require('./settings'); @@ -24,8 +25,9 @@ const Tag = require('./tag'); const TagLink = require('./tag_link'); const User = require('./user'); const UserError = require('./user_error'); -const UserState = require('./user_state'); const UsernameStatusHistory = require('./username_status_history'); +const UserProfile = require('./user_profile'); +const UserState = require('./user_state'); const ValidationUserError = require('./validation_user_error'); const plugins = require('../../services/plugins'); @@ -46,6 +48,7 @@ let resolvers = { FlagAction, FlagActionSummary, GenericUserError, + LocalUserProfile, RootMutation, RootQuery, Settings, @@ -55,8 +58,9 @@ let resolvers = { TagLink, User, UserError, - UserState, UsernameStatusHistory, + UserProfile, + UserState, ValidationUserError, }; diff --git a/graph/resolvers/local_user_profile.js b/graph/resolvers/local_user_profile.js new file mode 100644 index 000000000..1c3e0874b --- /dev/null +++ b/graph/resolvers/local_user_profile.js @@ -0,0 +1,7 @@ +const { property } = require('lodash'); + +const LocalUserProfile = { + confirmedAt: property('metadata.confirmed_at'), +}; + +module.exports = LocalUserProfile; diff --git a/graph/resolvers/user_profile.js b/graph/resolvers/user_profile.js new file mode 100644 index 000000000..6f26fa318 --- /dev/null +++ b/graph/resolvers/user_profile.js @@ -0,0 +1,12 @@ +const UserProfile = { + __resolveType({ provider }) { + switch (provider) { + case 'local': + return 'LocalUserProfile'; + default: + return undefined; + } + }, +}; + +module.exports = UserProfile; diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index f108a7ad4..79d259122 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -62,7 +62,7 @@ type Token { jwt: String } -type UserProfile { +interface UserProfile { # The id is an identifier for the user profile (email, facebook id, etc) id: String! @@ -70,6 +70,31 @@ type UserProfile { provider: String! } +# DefaultUserProfile is a fallback if the type of UserProfile can't be +# determined (like if it was from a plugin that was removed). +type DefaultUserProfile implements UserProfile { + # The id is an identifier for the user profile (email, facebook id, etc) + id: String! + + # name of the provider attached to the authentication mode + provider: String! +} + +# LocalUserProfile is for a User who has an authentication profile linked to +# their email address. +type LocalUserProfile implements UserProfile { + # id is the User's email address. + id: String! + + # name of the provider attached to the authentication mode, in this case, + # 'local'. + provider: String! + + # confirmedAt is the Date that the user had their email address confirmed, + # which is null if it has not been verified. + confirmedAt: Date +} + # USER_STATUS_USERNAME is the different states that a username can be in. enum USER_STATUS_USERNAME { # UNSET is used when the username can be changed, and does not necessarily diff --git a/plugins/talk-plugin-notifications-category-featured/index.js b/plugins/talk-plugin-notifications-category-featured/index.js index 802c37a9a..c6b0ea2bb 100644 --- a/plugins/talk-plugin-notifications-category-featured/index.js +++ b/plugins/talk-plugin-notifications-category-featured/index.js @@ -1,20 +1,16 @@ -const { graphql } = require('graphql'); const { get } = require('lodash'); const path = require('path'); const handle = async (ctx, { comment }) => { - const { connectors: { graph: { schema } } } = ctx; - // Check to see if this is a reply to an existing comment. const commentID = get(comment, 'id', null); if (commentID === null) { - ctx.log.debug('could not get comment id'); + ctx.log.info('could not get comment id'); return; } // Execute the graph request. - const reply = await graphql( - schema, + const reply = await ctx.graphql( ` query GetAuthorUserMetadata($comment_id: ID!) { comment(id: $comment_id) { @@ -28,8 +24,6 @@ const handle = async (ctx, { comment }) => { } } `, - {}, - ctx, { comment_id: commentID } ); if (reply.errors) { @@ -49,7 +43,7 @@ const handle = async (ctx, { comment }) => { const userID = get(reply, 'data.comment.user.id', null); if (!userID) { - ctx.log.debug('could not get comment user id'); + ctx.log.info('could not get comment user id'); return; } @@ -59,10 +53,7 @@ const handle = async (ctx, { comment }) => { }; const hydrate = async (ctx, category, context) => { - const { connectors: { graph: { schema } } } = ctx; - - const reply = await graphql( - schema, + const reply = await ctx.graphql( ` query GetNotificationData($context: ID!) { comment(id: $context) { @@ -74,8 +65,6 @@ const hydrate = async (ctx, category, context) => { } } `, - {}, - ctx, { context } ); if (reply.errors) { diff --git a/plugins/talk-plugin-notifications-category-reply/index.js b/plugins/talk-plugin-notifications-category-reply/index.js index 32d90e05d..16cfbfb72 100644 --- a/plugins/talk-plugin-notifications-category-reply/index.js +++ b/plugins/talk-plugin-notifications-category-reply/index.js @@ -1,20 +1,16 @@ -const { graphql } = require('graphql'); const { get } = require('lodash'); const path = require('path'); const handle = async (ctx, comment) => { - const { connectors: { graph: { schema } } } = ctx; - // Check to see if this is a reply to an existing comment. const parentID = get(comment, 'parent_id', null); if (parentID === null) { - ctx.log.debug('could not get parent comment id'); + ctx.log.info('could not get parent comment id'); return; } // Execute the graph request. - const reply = await graphql( - schema, + const reply = await ctx.graphql( ` query GetAuthorUserMetadata($comment_id: ID!) { comment(id: $comment_id) { @@ -28,8 +24,6 @@ const handle = async (ctx, comment) => { } } `, - {}, - ctx, { comment_id: parentID } ); if (reply.errors) { @@ -49,14 +43,14 @@ const handle = async (ctx, comment) => { const userID = get(reply, 'data.comment.user.id', null); if (!userID) { - ctx.log.debug('could not get parent comment user id'); + ctx.log.info('could not get parent comment user id'); return; } // Check to see if this is yourself replying to yourself, if that's the case // don't send a notification. if (userID === get(comment, 'author_id')) { - ctx.log.debug('user id of parent comment is the same as the new comment'); + ctx.log.info('user id of parent comment is the same as the new comment'); return; } @@ -66,10 +60,7 @@ const handle = async (ctx, comment) => { }; const hydrate = async (ctx, category, context) => { - const { connectors: { graph: { schema } } } = ctx; - - const reply = await graphql( - schema, + const reply = await ctx.graphql( ` query GetNotificationData($context: ID!) { comment(id: $context) { @@ -84,8 +75,6 @@ const hydrate = async (ctx, category, context) => { } } `, - {}, - ctx, { context } ); if (reply.errors) { diff --git a/plugins/talk-plugin-notifications-category-staff/index.js b/plugins/talk-plugin-notifications-category-staff/index.js index c5a59253d..d229456f7 100644 --- a/plugins/talk-plugin-notifications-category-staff/index.js +++ b/plugins/talk-plugin-notifications-category-staff/index.js @@ -1,14 +1,11 @@ -const { graphql } = require('graphql'); const { get } = require('lodash'); const path = require('path'); const handle = async (ctx, comment) => { - const { connectors: { graph: { schema } } } = ctx; - // Check to see if this is a reply to an existing comment. const parentID = get(comment, 'parent_id', null); if (parentID === null) { - ctx.log.debug('could not get parent comment id'); + ctx.log.info('could not get parent comment id'); return; } @@ -19,8 +16,7 @@ const handle = async (ctx, comment) => { } // Execute the graph request. - const reply = await graphql( - schema, + const reply = await ctx.graphql( ` query GetAuthorUserMetadata($comment_id: ID!, $author_id: ID!) { author: user(id: $author_id) { @@ -37,8 +33,6 @@ const handle = async (ctx, comment) => { } } `, - {}, - ctx, { comment_id: parentID, author_id: authorID } ); if (reply.errors) { @@ -53,27 +47,27 @@ const handle = async (ctx, comment) => { false ); if (!enabled) { - ctx.log.debug('onStaffReply is false, will not send the notification'); + ctx.log.info('onStaffReply is false, will not send the notification'); return; } const userID = get(reply, 'data.comment.user.id', null); if (!userID) { - ctx.log.debug('could not get parent comment user id'); + ctx.log.info('could not get parent comment user id'); return; } // Check to see if this is yourself replying to yourself, if that's the case // don't send a notification. if (userID === authorID) { - ctx.log.debug('user id of parent comment is the same as the new comment'); + ctx.log.info('user id of parent comment is the same as the new comment'); return; } // Check to see that this comment was indeed from a staff member. const role = get(reply, 'data.author.role'); if (!['ADMIN', 'MODERATOR', 'STAFF'].includes(role)) { - ctx.log.debug({ role }, 'reply author is not a staff member'); + ctx.log.info({ role }, 'reply author is not a staff member'); return; } @@ -83,10 +77,7 @@ const handle = async (ctx, comment) => { }; const hydrate = async (ctx, category, context) => { - const { connectors: { graph: { schema } } } = ctx; - - const reply = await graphql( - schema, + const reply = await ctx.graphql( ` query GetNotificationData($context: ID!) { comment(id: $context) { @@ -104,8 +95,6 @@ const hydrate = async (ctx, category, context) => { } } `, - {}, - ctx, { context } ); if (reply.errors) { diff --git a/plugins/talk-plugin-notifications/server/NotificationManager.js b/plugins/talk-plugin-notifications/server/NotificationManager.js index 6c1d2556d..a7ea7258f 100644 --- a/plugins/talk-plugin-notifications/server/NotificationManager.js +++ b/plugins/talk-plugin-notifications/server/NotificationManager.js @@ -1,7 +1,10 @@ -const { groupBy, forEach, property } = require('lodash'); +const { get, find, groupBy, forEach, property } = require('lodash'); const debug = require('debug')('talk-plugin-notifications'); const uuid = require('uuid/v4'); -const { UNSUBSCRIBE_SUBJECT } = require('./config'); +const { + UNSUBSCRIBE_SUBJECT, + DISABLE_REQUIRE_EMAIL_VERIFICATIONS, +} = require('./config'); // handleHandlers will call the handle method on each handler to determine if a // notification should be sent for it. @@ -15,12 +18,12 @@ const handleHandlers = (ctx, handlers, ...args) => // Attempt to create a notification out of it. const notification = await handle(ctx, ...args); if (!notification) { - ctx.log.debug('no notification deemed by event handler'); + ctx.log.info('no notification deemed by event handler'); return; } // Send the notification back. - ctx.log.debug({ category, event }, 'notification detected for event'); + ctx.log.info({ category, event }, 'notification detected for event'); return { handler, notification }; } catch (err) { ctx.log.error({ err }, 'could not handle the event'); @@ -31,13 +34,82 @@ const handleHandlers = (ctx, handlers, ...args) => // filterSuperseded will filter all the possible notifications and only send // those notifications that are not superseded by another type of notification. -const filterSuperseded = ({ handler: { category } }, index, notifications) => - !notifications.some(({ handler: { supersedesCategories = [] } }) => - supersedesCategories.some( - supersededCategory => supersededCategory === category - ) +const filterSuperseded = ( + { handler: { category }, notification: { userID: destinationUserID } }, + index, + notifications +) => + !notifications.some( + ({ + handler: { supersedesCategories = [] }, + notification: { userID: notificationUserID }, + }) => + // Only allow notifications to supersede another notification if that + // notification is also destined for the same user. + notificationUserID === destinationUserID && + // If another notification that is destined for the same user also exists + // and declares that it supersedes this one, return true so we can filter + // this one from the list. + supersedesCategories.some( + supersededCategory => supersededCategory === category + ) ); +const filterVerified = async (ctx, notifications) => { + notifications = await Promise.all( + notifications.map(async notification => { + // Grab the user that we're supposed to be sending the notification to. + const { notification: { userID } } = notification; + + // Check their confirmed status. + const { errors, data } = await ctx.graphql( + ` + query CheckUserConfirmation($userID: ID!) { + user(id: $userID) { + profiles { + provider + ... on LocalUserProfile { + confirmedAt + } + } + } + } + `, + { userID } + ); + if (errors) { + ctx.log.error( + { err: errors }, + 'could not query for user confirmation status' + ); + return; + } + + const profile = find(get(data, 'user.profiles', []), [ + 'provider', + 'local', + ]); + if (!profile) { + ctx.log.warn({ user_id: userID }, 'user did not have a local profile'); + return; + } + + const confirmed = get(profile, 'confirmedAt', null) !== null; + if (!confirmed) { + ctx.log.info( + { user_id: userID }, + 'user did not have their local profile confirmed, but had settings enabled, not mailing' + ); + return; + } + + return notification; + }) + ); + + return notifications.filter(property('notification')); +}; + class NotificationManager { constructor(context) { this.context = context; @@ -93,6 +165,12 @@ class NotificationManager { // had this notification superseded. notifications = notifications.filter(filterSuperseded); + // Only let notifications through for users who have their email addresses + // verified if we are configured to do so. + if (!DISABLE_REQUIRE_EMAIL_VERIFICATIONS) { + notifications = await filterVerified(ctx, notifications); + } + // Send the remaining notifications. return Promise.all( notifications.map( @@ -120,7 +198,7 @@ class NotificationManager { 'organizationName' ); if (organizationName === null) { - ctx.log.debug( + ctx.log.error( 'could not send the notification, organization name not in settings' ); return; @@ -153,7 +231,7 @@ class NotificationManager { user: userID, }); - ctx.log.debug(`Sent the notification for Job.ID[${task.id}]`); + ctx.log.info(`Sent the notification for Job.ID[${task.id}]`); } catch (err) { ctx.log.error( { err, message: err.message }, @@ -172,10 +250,10 @@ class NotificationManager { */ async getBody(ctx, handler, context) { const { connectors: { services: { I18n: { t } } } } = ctx; - const { category } = handler; + const { category, hydrate = () => [] } = handler; // Get the body replacement variables for the translation key. - const replacements = await handler.hydrate(ctx, category, context); + const replacements = await hydrate(ctx, category, context); // Generate the body. return t( diff --git a/plugins/talk-plugin-notifications/server/config.js b/plugins/talk-plugin-notifications/server/config.js index 17bd94bf9..c1c0ec50f 100644 --- a/plugins/talk-plugin-notifications/server/config.js +++ b/plugins/talk-plugin-notifications/server/config.js @@ -1,3 +1,8 @@ module.exports = { UNSUBSCRIBE_SUBJECT: 'nunsub', + + // TODO: replace this with a config option in the plugin config when we get there.. + DISABLE_REQUIRE_EMAIL_VERIFICATIONS: + process.env.TALK_DISABLE_REQUIRE_EMAIL_VERIFICATIONS_NOTIFICATIONS === + 'TRUE', }; diff --git a/services/logging.js b/services/logging.js index 48367f834..9aad098ab 100644 --- a/services/logging.js +++ b/services/logging.js @@ -1,6 +1,7 @@ const { version } = require('../package.json'); const Logger = require('bunyan'); const uuid = require('uuid/v1'); +const { LOGGING_LEVEL } = require('../config'); // Create the logging instance that all logger's are branched from. function createLogger(name, id = uuid()) { @@ -9,6 +10,7 @@ function createLogger(name, id = uuid()) { name, id, version, + level: LOGGING_LEVEL, serializers: { req: Logger.stdSerializers.req }, }); }