diff --git a/.gitignore b/.gitignore index 811e2ea2b..7c0007dc9 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ client/coral-framework/graphql/introspection.json *.swp *.DS_STORE .prettierrc.json +.vscode coverage/ test/e2e/reports/ diff --git a/bin/cli-setup b/bin/cli-setup index 282a8a2d7..572e8c7fc 100755 --- a/bin/cli-setup +++ b/bin/cli-setup @@ -15,6 +15,7 @@ const SetupService = require('../services/setup'); const UsersService = require('../services/users'); const MigrationService = require('../services/migration'); const errors = require('../errors'); +const Context = require('../graph/context'); // Register the shutdown criteria. util.onshutdown([() => mongoose.disconnect()]); @@ -184,7 +185,8 @@ const performSetup = async () => { }, ]); - let { user: newUser } = await SetupService.setup({ + const ctx = Context.forSystem(); + let { user: newUser } = await SetupService.setup(ctx, { settings: settings.toObject(), user: { email: user.email, diff --git a/bin/cli-users b/bin/cli-users index 950f2a538..cfcbcd13a 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -7,8 +7,6 @@ const util = require('./util'); const program = require('commander'); const inquirer = require('inquirer'); -const { graphql } = require('graphql'); -const helpers = require('../services/migration/helpers'); const { stripIndent } = require('common-tags'); const Table = require('cli-table'); @@ -21,31 +19,15 @@ inquirer.registerPrompt( require('inquirer-autocomplete-prompt') ); -const schema = require('../graph/schema'); const Context = require('../graph/context'); const UsersService = require('../services/users'); const UserModel = require('../models/user'); -const CommentModel = require('../models/comment'); -const ActionModel = require('../models/action'); const USER_ROLES = require('../models/enum/user_roles'); const mongoose = require('../services/mongoose'); // Register the shutdown criteria. util.onshutdown([() => mongoose.disconnect()]); -/** - * transforms a specific action to a removal action on the target model. - */ -const actionDecrTransformer = ({ item_id, action_type, group_id }) => ({ - query: { id: item_id }, - update: { - $inc: { - [`action_counts.${action_type.toLowerCase()}`]: -1, - [`action_counts.${action_type.toLowerCase()}_${group_id.toLowerCase()}`]: -1, - }, - }, -}); - /** * Deletes a user and cleans up their associated verifications. */ @@ -76,66 +58,29 @@ async function deleteUser(userID) { return util.shutdown(); } - const { transformSingleWithCursor } = helpers({ - queryBatchSize: 10000, - updateBatchSize: 10000, - }); + const ctx = Context.forSystem(); - console.warn("Removing user's actions"); - - // Remove all actions against comments. - await transformSingleWithCursor( - ActionModel.collection.find({ user_id: user.id, item_type: 'COMMENTS' }), - actionDecrTransformer, - CommentModel + const { data, errors } = await ctx.graphql( + ` + mutation DeleteUser($user_id: ID!) { + delUser(id: $user_id) { + errors { + translation_key + } + } + } + `, + { user_id: user.id } ); + if (errors) { + throw errors; + } - // Remove all actions against users. - await transformSingleWithCursor( - ActionModel.collection.find({ user_id: user.id, item_type: 'USERS' }), - actionDecrTransformer, - UserModel - ); + if (data.errors) { + throw data.errors; + } - // Remove all the user's actions. - await ActionModel.where({ user_id: user.id }) - .setOptions({ multi: true }) - .remove(); - - console.warn("Removing user's comments"); - - // Removes all the user's reply counts on each of the comments that they - // have commented on. - await transformSingleWithCursor( - CommentModel.collection.aggregate([ - { $match: { author_id: user.id } }, - { - $group: { - _id: '$parent_id', - count: { $sum: 1 }, - }, - }, - ]), - ({ _id: parent_id, count }) => ({ - query: { id: parent_id }, - update: { - $inc: { - reply_count: -1 * count, - }, - }, - }), - CommentModel - ); - - // Remove all the user's comments. - await CommentModel.where({ author_id: user.id }) - .setOptions({ multi: true }) - .remove(); - - console.warn('Removing the user'); - - // Remove the user. - await user.remove(); + console.log('User was deleted.'); util.shutdown(); } catch (err) { @@ -197,7 +142,7 @@ async function searchUsers() { value = ''; } - const { data, errors } = await graphql(schema, searchQuery, {}, ctx, { + const { data, errors } = await ctx.graphql(searchQuery, { value, }); if (errors && errors.length > 0) { diff --git a/client/coral-admin/src/components/UserDetailComment.js b/client/coral-admin/src/components/UserDetailComment.js index ba185a9df..206242058 100644 --- a/client/coral-admin/src/components/UserDetailComment.js +++ b/client/coral-admin/src/components/UserDetailComment.js @@ -76,7 +76,8 @@ class UserDetailComment extends React.Component {
- Story: {comment.asset.title} + {t('common.story')}:{' '} + {comment.asset.title ? comment.asset.title : comment.asset.url} { {t('modqueue.moderate')} @@ -109,6 +110,7 @@ class UserDetailComment extends React.Component {
+ {/* TODO: translate string */} Contains Link diff --git a/client/coral-admin/src/routes/Moderation/components/Comment.js b/client/coral-admin/src/routes/Moderation/components/Comment.js index f865908b2..efdcfbd79 100644 --- a/client/coral-admin/src/routes/Moderation/components/Comment.js +++ b/client/coral-admin/src/routes/Moderation/components/Comment.js @@ -122,7 +122,8 @@ class Comment extends React.Component {
- Story: {comment.asset.title} + {t('common.story')}:{' '} + {comment.asset.title ? comment.asset.title : comment.asset.url} {!currentAsset && ( {t('modqueue.moderate')} @@ -156,6 +157,7 @@ class Comment extends React.Component {
+ {/* TODO: translate string */} Contains Link diff --git a/docs/_config.yml b/docs/_config.yml index fe7146988..2a6fa2725 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -100,6 +100,8 @@ sidebar: children: - title: Authentication url: /integrating/authentication/ + - title: Asset Managment + url: /integrating/asset-management/ - title: Configuring the Comment Stream url: /integrating/configuring-comment-stream/ - title: Configuring the Admin diff --git a/docs/source/integrating/asset-management.md b/docs/source/integrating/asset-management.md new file mode 100644 index 000000000..a4888f4c5 --- /dev/null +++ b/docs/source/integrating/asset-management.md @@ -0,0 +1,314 @@ +--- +title: Asset Management +permalink: /integrating/asset-management/ +--- + +One of the most frequent questions that we get asked by organizations trying to +integrate Talk is: _How do we hook our CMS up to Talk so that articles are in +sync?_ + +This guide is designed to explain the steps to take your base installation of +Talk and configure it to allow only assets pushed into it from your CMS, and +keep your URL/title in sync. We won't cover here how to install the plugin, as +it is covered in our [Plugins Overview](/talk/plugins/). + +## Why do we need to create a plugin? + +By default, Talk will use "Lazy Asset Creation" to dynamically generate Assets +in Talk in order to make it easier for lighter installations. In order to have +more strict control over this flow, we will create a plugin that will: + +1. Disable "Lazy Asset Creation" by [Overriding a Resolver](#Overriding-a-Resolver). +2. Create Assets from our CMS by [Creating a New Asset Route](#Creating-a-New-Asset-Route). +3. Facilitate updates from our CMS to keep Talk in sync by [Creating an Asset Update Route](#Creating-an-Asset-Update-Route). + +We will then modify our embed so that we can [Target the Asset](#Target-the-Asset). + +But first we should grab our basic plugin structure: + +```sh +# clone our example repo (that comes with all the code below!) +git clone https://github.com/coralproject/talk-plugin-asset-manager-example.git + +# checkout the step-1 tag that starts us off with the basic file structure of +# the plugin. +git checkout step-1 +``` + +## Overriding a Resolver + +First we'll replace the content of the `resolver.js` file with the following: + +```js +// We'll need to modify the behavior of how assets are +// "resolved" in Talk, so we override the base asset resolver +// for the RootQuery type. +module.exports = { + RootQuery: { + asset: async (root, args, ctx) => { + // We'll grab the id of the asset being requested + // such that we'll be able to lookup the asset. + const { id } = args; + if (!id) { + // If the ID isn't provided, we don't want to do + // anything. + return null; + } + + // A mouthful for sure, but we need to use the loader + // that is available on the graph context in order to + // lookup the asset by ID. + const asset = await ctx.loaders.Assets.getByID.load(id); + if (!asset) { + // If the asset can't be found, we don't want to do + // anything. + return null; + } + + // Send the asset back. + return asset; + }, + }, +}; +``` + +This serves to override the default asset resolver. You can of course, override +any other field in the schema to perform whatever action your business needs +require, including adding additional resolvers! You can refer to our +[GraphQL API Docs](/talk/reference/graphql/) to see what other fields you can +override. + +Without this, Talk will continue to use the "Lazy Asset Creation" to handle +resolving the `asset` edge, which is what we want to change. + +_Note, you can also get to this point by running `git checkout step-2`!_ + +## Creating a New Asset Route + +In order to create Assets now, we have to get our CMS to push those into Talk, +the easiest way to do this is by creating a custom route. We won't cover +specific CMS integrations, but will assume that there is some type of webhook +system you are able to utilize that will trigger when a new article is created. + +We'll replace the contents of the `router.js` file with the following: + +```js +// This file we'll create routes that will facilitate asset creation and +// updates. + +const authz = require('middleware/authorization'); + +module.exports = router => { + // We'll respond to a POST request on the following route where the request + // must have a valid ADMIN access token. + router.post( + '/api/v1/plugin/asset-manager-example', + authz.needed('ADMIN'), + async (req, res, next) => { + // Get the graph context from the request. + const { context } = req; + + // Grab from the graph context, the AssetModel that we can use to create + // the new Asset. Lots of object destructuring here, but this lets us keep + // the important business logic cleaner. + const { connectors: { models: { Assets } } } = context; + + try { + // Now we can create the asset that was passed to us in the body of the + // request as JSON. Check the schema of the Asset model by looking at: + // https://github.com/coralproject/talk/blob/master/models/asset.js + await Assets.create(req.body); + + // Let your webhook callback know we got it! + return res.status(204).end(); + } catch (err) { + return next(err); + } + } + ); +}; +``` + +This request handler when mounted on Talk will allow your CMS to send a POST +request to `${TALK_ROOT_URL}/api/v1/plugin/asset-manager-example` with the +Asset as a JSON payload. In order to protect the endpoint from abuse, we add the +authorization middleware. This middleware essentially says, _you must be an +admin to hit this route_. We need to generate a token that can be used by your +CMS using the Talk cli tool: + +```sh +# find or create an admin user that can be used as the basis for the token +./bin/cli users list + +# create a token for the user with the given id +./bin/cli token create ${USER_ID} cms-token +``` + +You can attach the generated token to the request a few ways: + +1. HTTP Header: + + curl ${TALK_ROOT_URL}/api/v1/plugin/asset-manager-example \ + -XPOST \ + -H "Authorization: Bearer ${TOKEN}" \ + -H "Content-Type: application/json" \ + --data "${ASSET_JSON}" + +2. Query Parameter: + + curl ${TALK_ROOT_URL}/api/v1/plugin/asset-manager-example?access_token=${TOKEN} + -XPOST \ + -H "Content-Type: application/json" \ + --data "${ASSET_JSON}" + +Where `${ASSET_JSON}` is the JSON for your Asset matching the +[AssetSchema](https://github.com/coralproject/talk/blob/master/models/asset.js). + +_Note, you can also get to this point by running `git checkout step-3`!_ + +## Creating an Asset Update Route + +Now imagine the situation where you decide that you want to change the url slug +of the page, or update the title, now Talk is out of sync! Let's fix that. + +Update your `router.js` to the following: + +```js +// This file we'll create routes that will facilitate asset creation and +// updates. + +const authz = require('middleware/authorization'); + +module.exports = router => { + // We'll respond to a POST request on the following route where the request + // must have a valid ADMIN access token. + router.post( + '/api/v1/plugin/asset-manager-example', + authz.needed('ADMIN'), + async (req, res, next) => { + // Get the graph context from the request. + const { context } = req; + + // Grab from the graph context, the AssetModel that we can use to create + // the new Asset. Lots of object destructuring here, but this lets us keep + // the important business logic cleaner. + const { connectors: { models: { Assets } } } = context; + + try { + // Now we can create the asset that was passed to us in the body of the + // request as JSON. Check the schema of the Asset model by looking at: + // https://github.com/coralproject/talk/blob/master/models/asset.js + await Assets.create(req.body); + + // Let your webhook callback know we got it! + return res.status(204).end(); + } catch (err) { + return next(err); + } + } + ); + + // We'll respond to a PUT request on the following route where the request + // must also have a valid ADMIN access token. + router.put( + '/api/v1/plugin/asset-manager-example/:id', + authz.needed('ADMIN'), + async (req, res, next) => { + // Get the graph context from the request. + const { context } = req; + + // Grab from the graph context, the AssetModel that we can use to update + // the Asset. Lots of object destructuring here, but this lets us keep + // the important business logic cleaner. + const { connectors: { models: { Assets } } } = context; + + try { + // Now we can lookup the asset we're updating and apply out updates to + // the model atomically. + const asset = await Assets.findOneAndUpdate( + { id: req.params.id }, + req.body, + { + // We want to validate the model being updated. + runValidators: true, + } + ); + if (!asset) { + // The asset indicated by the ID wasn't found, let the webhook know! + return res.status(404).end(); + } + + // Let your webhook callback know we got it! + return res.status(204).end(); + } catch (err) { + return next(err); + } + } + ); +}; +``` + +As you can see from the previous step of [Creating a New Asset Route](#Creating-a-New-Asset-Route) +, we have added the new `PUT` route to the router. This is a simple addition +that allows your CMS to call into Talk when the asset has updated it's title, +it's url (or really anything in the [AssetSchema](https://github.com/coralproject/talk/blob/master/models/asset.js)) to keep the Talk Admin and links up to date. + +Following the previous example, you can issue the request as follows: + +```sh +curl ${TALK_ROOT_URL}/api/v1/plugin/asset-manager-example/${ASSET_ID} \ + -XPUT \ + -H "Authorization: Bearer ${TOKEN}" \ + -H "Content-Type: application/json" \ + --data "${ASSET_JSON}" +``` + +The difference from the previous curl example, is that this one changes the +method from a `POST` to a `PUT`, and we add the `${ASSET_ID}` to the end of the +url. + +_Note, you can also get to this point by running `git checkout step-4`!_ + +## Target the Asset + +Now that we have a way to create and update Assets, we now need a way to +reference it. One of the most important fields in the Asset model, is the `id`. +This `id` can be one generated from your CMS, or some other system, but must +be kept consistent. + +When you install Talk, and visit the admin panel, we can see under +`/admin/configure` in the tab for Tech Settings, an embed snippet: + +```html +
+ +``` + +We'll modify this to the following: + +```html +
+ +``` + +Adding the `asset_id` parameter to the render function will accomplish a very +important task. It will provide Talk with the specific ID of the asset to +associate with the displayed page. This is important because even if you update +the URL in the future, the embed will still reference the correct Asset. The +`${ASSET_ID}` should be replaced by your CMS with the correct Asset id using +your desired scripting/templating tools. + +At this point, you should have a fully built Talk plugin that can be paired with +some work on your CMS to create a fully integrated asset management pipeline! + +To view the fully completed source code, visit +https://github.com/coralproject/talk-plugin-asset-manager-example. \ No newline at end of file diff --git a/docs/source/reference/server.md b/docs/source/reference/server.md index ce8cebae1..93b9ee69f 100644 --- a/docs/source/reference/server.md +++ b/docs/source/reference/server.md @@ -318,7 +318,13 @@ module.exports = { let user; try { - user = await UsersService.findOrCreateExternalUser(profile); + const { id, provider, displayName } = profile; + user = await UsersService.findOrCreateExternalUser( + req.context, + id, + provider, + displayName + ); } catch (err) { return done(err); } diff --git a/graph/mutators/user.js b/graph/mutators/user.js index f8db7a19e..9d31e6dbd 100644 --- a/graph/mutators/user.js +++ b/graph/mutators/user.js @@ -1,5 +1,6 @@ const errors = require('../../errors'); const UsersService = require('../../services/users'); +const migrationHelpers = require('../../services/migration/helpers'); const { CHANGE_USERNAME, SET_USERNAME, @@ -7,6 +8,7 @@ const { SET_USER_BAN_STATUS, SET_USER_SUSPENSION_STATUS, UPDATE_USER_ROLES, + DELETE_USER, } = require('../../perms/constants'); const setUserUsernameStatus = async (ctx, id, status) => { @@ -70,6 +72,87 @@ const setRole = (ctx, id, role) => { return UsersService.setRole(id, role); }; +/** + * transforms a specific action to a removal action on the target model. + */ +const actionDecrTransformer = ({ item_id, action_type, group_id }) => ({ + query: { id: item_id }, + update: { + $inc: { + [`action_counts.${action_type.toLowerCase()}`]: -1, + [`action_counts.${action_type.toLowerCase()}_${group_id.toLowerCase()}`]: -1, + }, + }, +}); + +// delUser will delete a given user with the specified id. +const delUser = async (ctx, id) => { + const { connectors: { models: { User, Action, Comment } } } = ctx; + + // Find the user we're removing. + const user = await User.findOne({ id }); + if (!user) { + throw errors.ErrNotFound; + } + + // Get the query transformer we'll use to help batch process the user + // deletion. + const { transformSingleWithCursor } = migrationHelpers({ + queryBatchSize: 10000, + updateBatchSize: 10000, + }); + + // Remove all actions against comments. + await transformSingleWithCursor( + Action.collection.find({ user_id: user.id, item_type: 'COMMENTS' }), + actionDecrTransformer, + Comment + ); + + // Remove all actions against users. + await transformSingleWithCursor( + Action.collection.find({ user_id: user.id, item_type: 'USERS' }), + actionDecrTransformer, + User + ); + + // Remove all the user's actions. + await Action.where({ user_id: user.id }) + .setOptions({ multi: true }) + .remove(); + + // Removes all the user's reply counts on each of the comments that they + // have commented on. + await transformSingleWithCursor( + Comment.collection.aggregate([ + { $match: { author_id: user.id } }, + { + $group: { + _id: '$parent_id', + count: { $sum: 1 }, + }, + }, + ]), + ({ _id: parent_id, count }) => ({ + query: { id: parent_id }, + update: { + $inc: { + reply_count: -1 * count, + }, + }, + }), + Comment + ); + + // Remove all the user's comments. + await Comment.where({ author_id: user.id }) + .setOptions({ multi: true }) + .remove(); + + // Remove the user. + await user.remove(); +}; + module.exports = ctx => { let mutators = { User: { @@ -81,6 +164,7 @@ module.exports = ctx => { setUserUsernameStatus: () => Promise.reject(errors.ErrNotAuthorized), setUsername: () => Promise.reject(errors.ErrNotAuthorized), stopIgnoringUser: () => Promise.reject(errors.ErrNotAuthorized), + del: () => Promise.reject(errors.ErrNotAuthorized), }, }; @@ -116,6 +200,10 @@ module.exports = ctx => { mutators.User.setUserSuspensionStatus = (id, until, message) => setUserSuspensionStatus(ctx, id, until, message); } + + if (ctx.user.can(DELETE_USER)) { + mutators.User.del = id => delUser(ctx, id); + } } return mutators; diff --git a/graph/resolvers/root_mutation.js b/graph/resolvers/root_mutation.js index 3757bb3c4..2838f0f99 100644 --- a/graph/resolvers/root_mutation.js +++ b/graph/resolvers/root_mutation.js @@ -136,6 +136,9 @@ const RootMutation = { forceScrapeAsset: async (_, { id }, { mutators: { Asset } }) => { await Asset.scrape(id); }, + delUser: async (_, { id }, { mutators: { User } }) => { + await User.del(id); + }, }; module.exports = RootMutation; diff --git a/graph/subscriptions/setupFunctions.js b/graph/subscriptions/setupFunctions.js index 467c4e98f..ee29d6971 100644 --- a/graph/subscriptions/setupFunctions.js +++ b/graph/subscriptions/setupFunctions.js @@ -1,16 +1,17 @@ const { - SUBSCRIBE_COMMENT_ACCEPTED, - SUBSCRIBE_COMMENT_REJECTED, - SUBSCRIBE_COMMENT_FLAGGED, - SUBSCRIBE_COMMENT_RESET, - SUBSCRIBE_ALL_COMMENT_EDITED, SUBSCRIBE_ALL_COMMENT_ADDED, - SUBSCRIBE_ALL_USER_SUSPENDED, + SUBSCRIBE_ALL_COMMENT_EDITED, SUBSCRIBE_ALL_USER_BANNED, - SUBSCRIBE_ALL_USERNAME_REJECTED, + SUBSCRIBE_ALL_USER_CREATED, + SUBSCRIBE_ALL_USER_SUSPENDED, SUBSCRIBE_ALL_USERNAME_APPROVED, - SUBSCRIBE_ALL_USERNAME_FLAGGED, SUBSCRIBE_ALL_USERNAME_CHANGED, + SUBSCRIBE_ALL_USERNAME_FLAGGED, + SUBSCRIBE_ALL_USERNAME_REJECTED, + SUBSCRIBE_COMMENT_ACCEPTED, + SUBSCRIBE_COMMENT_FLAGGED, + SUBSCRIBE_COMMENT_REJECTED, + SUBSCRIBE_COMMENT_RESET, } = require('../../perms/constants'); const merge = require('lodash/merge'); @@ -139,6 +140,8 @@ const setupFunctions = { } return !args.user_id || user.id === args.user_id; }, + userCreated: (options, args, user, ctx) => + ctx.user && ctx.user.can(SUBSCRIBE_ALL_USER_CREATED), }; /** @@ -153,19 +156,17 @@ module.exports = plugins.get('server', 'setupFunctions').reduce( return merge(acc, setupFunctions); }, - Object.keys(setupFunctions) - .map(key => { - const filter = setupFunctions[key]; - - return { - [key]: (options, args) => ({ - [key]: { - filter: (user, ctx) => filter(options, args, user, ctx), - }, - }), - }; - }) - .reduce((setupFunction, setupFunctions) => { - return merge(setupFunctions, setupFunction); - }, {}) + // Process the default setupFunctions. + Object.entries(setupFunctions) + .map(([key, filter]) => ({ + [key]: (options, args) => ({ + [key]: { + filter: (user, ctx) => filter(options, args, user, ctx), + }, + }), + })) + .reduce( + (setupFunction, setupFunctions) => merge(setupFunctions, setupFunction), + {} + ) ); diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index ea574ee6b..2414faf1f 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -1426,6 +1426,12 @@ type ForceScrapeAssetResponse implements Response { errors: [UserError!] } +type DelUserResponse implements Response { + + # An array of errors relating to the mutation that occurred. + errors: [UserError!] +} + # All mutations for the application are defined on this object. type RootMutation { @@ -1523,6 +1529,9 @@ type RootMutation { # forceScrapeAsset will force scrape the Asset with the given ID. forceScrapeAsset(id: ID!): ForceScrapeAssetResponse + + # delUser will delete the user with the specified id. + delUser(id: ID!): DelUserResponse } type UsernameChangedPayload { @@ -1588,6 +1597,10 @@ type Subscription { # Get an update whenever a username has been changed. `user_id` must match id # of current user except for users with the `ADMIN` or `MODERATOR` role. usernameChanged(user_id: ID): UsernameChangedPayload + + # Get an update whenever a user is created. Only accessible to users with the + # `ADMIN` or `MODERATOR` roles. + userCreated: User } ################################################################################ diff --git a/perms/constants/mutation.js b/perms/constants/mutation.js index 06e5b6f0f..d2ebe73f1 100644 --- a/perms/constants/mutation.js +++ b/perms/constants/mutation.js @@ -18,4 +18,5 @@ module.exports = { UPDATE_ASSET_SETTINGS: 'UPDATE_ASSET_SETTINGS', UPDATE_ASSET_STATUS: 'UPDATE_ASSET_STATUS', UPDATE_SETTINGS: 'UPDATE_SETTINGS', + DELETE_USER: 'DELETE_USER', }; diff --git a/perms/constants/subscription.js b/perms/constants/subscription.js index e27730396..30ef4656d 100644 --- a/perms/constants/subscription.js +++ b/perms/constants/subscription.js @@ -11,4 +11,5 @@ module.exports = { SUBSCRIBE_ALL_USERNAME_APPROVED: 'SUBSCRIBE_ALL_USERNAME_APPROVED', SUBSCRIBE_ALL_USERNAME_FLAGGED: 'SUBSCRIBE_ALL_USERNAME_FLAGGED', SUBSCRIBE_ALL_USERNAME_CHANGED: 'SUBSCRIBE_ALL_USERNAME_CHANGED', + SUBSCRIBE_ALL_USER_CREATED: 'SUBSCRIBE_ALL_USER_CREATED', }; diff --git a/perms/reducers/subscription.js b/perms/reducers/subscription.js index 44fb43af6..345a79b73 100644 --- a/perms/reducers/subscription.js +++ b/perms/reducers/subscription.js @@ -15,6 +15,7 @@ module.exports = (user, perm) => { case types.SUBSCRIBE_ALL_USERNAME_APPROVED: case types.SUBSCRIBE_ALL_USERNAME_FLAGGED: case types.SUBSCRIBE_ALL_USERNAME_CHANGED: + case types.SUBSCRIBE_ALL_USER_CREATED: return check(user, ['ADMIN', 'MODERATOR']); default: break; diff --git a/plugins/talk-plugin-facebook-auth/server/passport.js b/plugins/talk-plugin-facebook-auth/server/passport.js index 477ed48a0..7bec1799a 100644 --- a/plugins/talk-plugin-facebook-auth/server/passport.js +++ b/plugins/talk-plugin-facebook-auth/server/passport.js @@ -25,7 +25,14 @@ module.exports = passport => { async (req, accessToken, refreshToken, profile, done) => { let user; try { - user = await UsersService.findOrCreateExternalUser(profile); + const { id, provider, displayName } = profile; + + user = await UsersService.findOrCreateExternalUser( + req.context, + id, + provider, + displayName + ); } catch (err) { return done(err); } diff --git a/plugins/talk-plugin-google-auth/server/passport.js b/plugins/talk-plugin-google-auth/server/passport.js index c3463557b..8386f319c 100644 --- a/plugins/talk-plugin-google-auth/server/passport.js +++ b/plugins/talk-plugin-google-auth/server/passport.js @@ -24,9 +24,16 @@ module.exports = passport => { async (req, accessToken, refreshToken, profile, done) => { let user; try { - user = await UsersService.findOrCreateExternalUser(profile); + const { id, provider, displayName } = profile; + + user = await UsersService.findOrCreateExternalUser( + req.context, + id, + provider, + displayName + ); } catch (err) { - return done(err.toString()); + return done(err); } return ValidateUserLogin(profile, user, done); diff --git a/routes/api/v1/auth.js b/routes/api/v1/auth.js index 369f4278a..d8e0544a9 100644 --- a/routes/api/v1/auth.js +++ b/routes/api/v1/auth.js @@ -4,6 +4,7 @@ const { HandleGenerateCredentials, HandleLogout, } = require('../../../services/passport'); +const authz = require('../../../middleware/authorization'); const router = express.Router(); /** @@ -26,7 +27,7 @@ router.get('/', (req, res, next) => { /** * This blacklists the token used to authenticate. */ -router.delete('/', HandleLogout); +router.delete('/', authz.needed(), HandleLogout); //============================================================================== // PASSPORT ROUTES diff --git a/routes/api/v1/setup.js b/routes/api/v1/setup.js index c3cf9518e..553e42f44 100644 --- a/routes/api/v1/setup.js +++ b/routes/api/v1/setup.js @@ -21,7 +21,10 @@ router.post('/', async (req, res, next) => { const { settings, user: { email, password, username } } = req.body; try { - await SetupService.setup({ settings, user: { email, password, username } }); + await SetupService.setup(req.context, { + settings, + user: { email, password, username }, + }); res.status(204).end(); } catch (err) { return next(err); diff --git a/routes/api/v1/users.js b/routes/api/v1/users.js index 8c1a7b7f3..e0de3b5db 100644 --- a/routes/api/v1/users.js +++ b/routes/api/v1/users.js @@ -11,7 +11,13 @@ router.post('/', async (req, res, next) => { const redirectUri = req.header('X-Pym-Url') || req.header('Referer'); try { - let user = await UsersService.createLocalUser(email, password, username); + // Adjusted the user creation endpoint. + let user = await UsersService.createLocalUser( + req.context, + email, + password, + username + ); // Send an email confirmation. The Front end will know about the // requireEmailConfirmation as it's included in the settings get endpoint. diff --git a/services/passport.js b/services/passport.js index 59215dd54..5748c911b 100644 --- a/services/passport.js +++ b/services/passport.js @@ -184,24 +184,24 @@ async function ValidateUserLogin(loginProfile, user, done) { * Revoke the token on the request. */ const HandleLogout = async (req, res, next) => { - const { jwt } = req; - - const now = new Date(); - const expiry = (jwt.exp - now.getTime() / 1000).toFixed(0); - try { + const { jwt } = req; + + const now = new Date(); + const expiry = (jwt.exp - now.getTime() / 1000).toFixed(0); + await client().set(`jtir[${jwt.jti}]`, now.toISOString(), 'EX', expiry); + + // Only clear the cookie on logout if enabled. + if (JWT_CLEAR_COOKIE_LOGOUT) { + debug('clearing the login cookie'); + res.clearCookie(JWT_SIGNING_COOKIE_NAME); + } + + res.status(204).end(); } catch (err) { return next(err); } - - // Only clear the cookie on logout if enabled. - if (JWT_CLEAR_COOKIE_LOGOUT) { - debug('clearing the login cookie'); - res.clearCookie(JWT_SIGNING_COOKIE_NAME); - } - - res.status(204).end(); }; const checkGeneralTokenBlacklist = jwt => diff --git a/services/setup.js b/services/setup.js index 29f84ff40..84f915ff2 100644 --- a/services/setup.js +++ b/services/setup.js @@ -61,7 +61,7 @@ module.exports = class SetupService { /** * This will perform the setup. */ - static async setup({ settings, user: { email, password, username } }) { + static async setup(ctx, { settings, user: { email, password, username } }) { // Validate the settings first. await SetupService.validate({ settings, @@ -79,7 +79,12 @@ module.exports = class SetupService { // Settings are created! Create the user. // Create the user. - let user = await UsersService.createLocalUser(email, password, username); + let user = await UsersService.createLocalUser( + ctx, + email, + password, + username + ); // Grant them administrative privileges and confirm the email account. await Promise.all([ diff --git a/services/users.js b/services/users.js index f5319122c..af91cacc7 100644 --- a/services/users.js +++ b/services/users.js @@ -1,7 +1,7 @@ const uuid = require('uuid'); const bcrypt = require('bcryptjs'); const errors = require('../errors'); -const { some, merge } = require('lodash'); +const { difference, sample, some, merge, random } = require('lodash'); const { ROOT_URL } = require('../config'); const { jwt: JWT_SECRET } = require('../secrets'); const debug = require('debug')('talk:services:users'); @@ -346,13 +346,77 @@ class UsersService { return username.replace(/ /g, '_').replace(/[^a-zA-Z_]/g, ''); } + /** + * Creates the initial username for an external account. Searches to make + * sure username not already used. Adds a random number if username already + * in use. + */ + static async getInitialUsername(username) { + const MAX_ATTEMPTS = 10; + const END_NUMBER_MAX = 99999; + const GROUP_ATTEMPTS = 50; + + // Cast the original username. + const castedName = UsersService.castUsername(username); + const lowercaseUsername = castedName.toLowerCase(); + + // Try to see if our first guess has been taken. + const existingUserWithName = await UserModel.findOne({ + lowercaseUsername, + }); + if (!existingUserWithName) { + return castedName; + } + + // Our first username was taken, lets try to find a non-taken name. + for (let i = 0; i < MAX_ATTEMPTS; i++) { + // Generate `GROUP_ATTEMPTS` guesses for the username. + const usernameGuesses = Array.from(Array(GROUP_ATTEMPTS)).map( + () => `${castedName}_${random(0, END_NUMBER_MAX)}` + ); + + // Map them all to lowercase. + const lowercaseUsernameGuesses = usernameGuesses.map(guess => + guess.toLowerCase() + ); + + // See if any of these users aren't taken already. + const existingUsernames = (await UserModel.find( + { + lowercaseUsername: { $in: lowercaseUsernameGuesses }, + }, + { lowercaseUsername: 1 } + )).map(({ lowercaseUsername }) => lowercaseUsername); + if (existingUsernames.length === lowercaseUsernameGuesses.length) { + // The number of found users is the same as the number of username + // guesses, aka, all the usernames are taken. + continue; + } + + // At least one of the usernames wasn't taken! Let's filter this to only + // include unused usernames and grab one random entry from the list. + const foundLowercaseUsernameIndex = lowercaseUsernameGuesses.indexOf( + sample(difference(lowercaseUsernameGuesses, existingUsernames)) + ); + + // Now we get the uppercase version of that string. + return usernameGuesses[foundLowercaseUsernameIndex]; + } + + throw new Error( + 'cannot find free name after ' + + (MAX_ATTEMPTS * GROUP_ATTEMPTS + 1) + + ' tries' + ); + } + /** * Finds a user given a social profile and if the user does not exist, creates * them. * @param {Object} profile - User social/external profile * @param {Function} done [description] */ - static async findOrCreateExternalUser({ id, provider, displayName }) { + static async findOrCreateExternalUser(ctx, id, provider, displayName) { let user = await UserModel.findOne({ profiles: { $elemMatch: { @@ -368,7 +432,7 @@ class UsersService { // User does not exist and need to be created. // Create an initial username for the user. - let username = UsersService.castUsername(displayName); + let username = await UsersService.getInitialUsername(displayName); // The user was not found, lets create them! user = new UserModel({ @@ -388,6 +452,9 @@ class UsersService { // Save the user in the database. await user.save(); + // Emit that the user was created. + ctx.pubsub.publish('userCreated', user); + return user; } @@ -438,23 +505,6 @@ class UsersService { ); } - /** - * Creates local users. - * @param {Array} users Users to create - * @return {Promise} Resolves with the users that were created - */ - static createLocalUsers(users) { - return Promise.all( - users.map(user => { - return UsersService.createLocalUser( - user.email, - user.password, - user.username - ); - }) - ); - } - /** * Check the requested username for blocked words and special chars * @param {String} username word to be checked for profanity @@ -489,24 +539,24 @@ class UsersService { */ static isValidPassword(password) { if (!password) { - return Promise.reject(errors.ErrMissingPassword); + throw errors.ErrMissingPassword; } if (password.length < 8) { - return Promise.reject(errors.ErrPasswordTooShort); + throw errors.ErrPasswordTooShort; } - return Promise.resolve(password); + return password; } /** * Creates the local user with a given email, password, and name. + * @param {Object} ctx application context for the request * @param {String} email email of the new user * @param {String} password plaintext password of the new user - * @param {String} username name of the display user - * @param {Function} done callback + * @param {String} username name of the display user */ - static async createLocalUser(email, password, username) { + static async createLocalUser(ctx, email, password, username) { if (!email) { throw errors.ErrMissingEmail; } @@ -553,6 +603,9 @@ class UsersService { throw err; } + // Emit that the user was created. + ctx.pubsub.publish('userCreated', user); + return user; } diff --git a/test/server/graph/mutations/changeUsername.js b/test/server/graph/mutations/changeUsername.js index f54ea9ac1..eaff85434 100644 --- a/test/server/graph/mutations/changeUsername.js +++ b/test/server/graph/mutations/changeUsername.js @@ -11,7 +11,9 @@ describe('graph.mutations.changeUsername', () => { let user; beforeEach(async () => { await SettingsService.init(); + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'test@test.com', 'testpassword1!', 'kirk' diff --git a/test/server/graph/mutations/editComment.js b/test/server/graph/mutations/editComment.js index 476279ae8..7ce8d865a 100644 --- a/test/server/graph/mutations/editComment.js +++ b/test/server/graph/mutations/editComment.js @@ -16,8 +16,10 @@ describe('graph.mutations.editComment', () => { beforeEach(async () => { timekeeper.reset(); await SettingsService.init(); + const ctx = Context.forSystem(); asset = await AssetModel.create({}); user = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' @@ -124,7 +126,9 @@ describe('graph.mutations.editComment', () => { body: `hello there! ${String(Math.random()).slice(2)}`, }); + const ctx = Context.forSystem(); const userB = await UsersService.createLocalUser( + ctx, 'usernameB@example.com', 'password', 'usernameB' diff --git a/test/server/graph/mutations/ignoreUser.js b/test/server/graph/mutations/ignoreUser.js index e1cb620e6..72b59a474 100644 --- a/test/server/graph/mutations/ignoreUser.js +++ b/test/server/graph/mutations/ignoreUser.js @@ -34,12 +34,15 @@ describe('graph.mutations.ignoreUser', () => { }); it('users can ignoreUser', async () => { + const ctx = Context.forSystem(); let currentUser = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' ); const userToIgnore = await UsersService.createLocalUser( + ctx, 'usernameB@example.com', 'password', 'usernameB' @@ -83,7 +86,9 @@ describe('graph.mutations.ignoreUser', () => { }); it('users cannot ignore themselves', async () => { + const ctx = Context.forSystem(); const user = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' @@ -124,18 +129,22 @@ describe('graph.mutations.stopIgnoringUser', () => { // We're going to ignore 2 users, // then stopIgnoring 1 of them // then assert myIgnoredUsers only lists the one remaining + const ctx = Context.forSystem(); let currentUser = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' ); const usersToIgnore = await Promise.all([ UsersService.createLocalUser( + ctx, 'usernameB@example.com', 'password', 'usernameB' ), UsersService.createLocalUser( + ctx, 'usernameC@example.com', 'password', 'usernameC' diff --git a/test/server/graph/mutations/setUserBanStatus.js b/test/server/graph/mutations/setUserBanStatus.js index 4d2973b5a..497ec6ea2 100644 --- a/test/server/graph/mutations/setUserBanStatus.js +++ b/test/server/graph/mutations/setUserBanStatus.js @@ -17,7 +17,9 @@ describe('graph.mutations.banUser', () => { beforeEach(async () => { await SettingsService.init(); + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' diff --git a/test/server/graph/mutations/setUserSuspensionStatus.js b/test/server/graph/mutations/setUserSuspensionStatus.js index f0b81cf49..fb0f4b96c 100644 --- a/test/server/graph/mutations/setUserSuspensionStatus.js +++ b/test/server/graph/mutations/setUserSuspensionStatus.js @@ -19,7 +19,9 @@ describe('graph.mutations.suspendUser', () => { beforeEach(async () => { await SettingsService.init(); + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' diff --git a/test/server/graph/mutations/setUserUsernameStatus.js b/test/server/graph/mutations/setUserUsernameStatus.js index d37bf4bf3..cf3f8b6ad 100644 --- a/test/server/graph/mutations/setUserUsernameStatus.js +++ b/test/server/graph/mutations/setUserUsernameStatus.js @@ -19,7 +19,9 @@ const { expect } = chai; beforeEach(async () => { await SettingsService.init(); + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' diff --git a/test/server/graph/queries/asset.js b/test/server/graph/queries/asset.js index f976bd37d..1bd5a0ee3 100644 --- a/test/server/graph/queries/asset.js +++ b/test/server/graph/queries/asset.js @@ -17,23 +17,28 @@ describe('graph.queries.asset', () => { { id: '1', url: 'https://example.com/?id=1' }, { id: '2', url: 'https://example.com/?id=2' }, ]); - users = await UsersService.createLocalUsers([ - { - email: 'usernameA@example.com', - password: 'password', - username: 'usernameA', - }, - { - email: 'usernameB@example.com', - password: 'password', - username: 'usernameB', - }, - { - email: 'usernameC@example.com', - password: 'password', - username: 'usernameC', - }, - ]); + const ctx = Context.forSystem(); + users = await Promise.all( + [ + { + email: 'usernameA@example.com', + password: 'password', + username: 'usernameA', + }, + { + email: 'usernameB@example.com', + password: 'password', + username: 'usernameB', + }, + { + email: 'usernameC@example.com', + password: 'password', + username: 'usernameC', + }, + ].map(({ email, username, password }) => + UsersService.createLocalUser(ctx, email, password, username) + ) + ); comments = await Promise.all( [0, 0, 1, 1].map(idx => CommentsService.publicCreate({ diff --git a/test/server/graph/queries/user.js b/test/server/graph/queries/user.js index 3ec8abaf9..16bb116d6 100644 --- a/test/server/graph/queries/user.js +++ b/test/server/graph/queries/user.js @@ -14,8 +14,9 @@ describe('graph.queries.user', () => { let user; beforeEach(async () => { await SettingsService.init(); - + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'usernameA@example.com', 'password', 'usernameA' diff --git a/test/server/routes/api/auth/index.js b/test/server/routes/api/auth/index.js index 7080cf157..f80fbbc37 100644 --- a/test/server/routes/api/auth/index.js +++ b/test/server/routes/api/auth/index.js @@ -1,12 +1,12 @@ const app = require('../../../../../app'); +const Context = require('../../../../../graph/context'); +const UsersService = require('../../../../../services/users'); const chai = require('chai'); chai.should(); chai.use(require('chai-http')); const expect = chai.expect; -const UsersService = require('../../../../../services/users'); - describe('/api/v1/auth', () => { describe('#get', () => { it('should return nothing when no user is logged in', () => { @@ -32,7 +32,9 @@ describe('/api/v1/auth/local', () => { }; await SettingsService.init(settings); + const ctx = Context.forSystem(); mockUser = await UsersService.createLocalUser( + ctx, 'maria@gmail.com', 'password!', 'Maria' diff --git a/test/server/routes/api/user/index.js b/test/server/routes/api/user/index.js index e7e3f6c72..3faadba6e 100644 --- a/test/server/routes/api/user/index.js +++ b/test/server/routes/api/user/index.js @@ -3,6 +3,7 @@ const passport = require('../../../passport'); const app = require('../../../../../app'); const mailer = require('../../../../../services/mailer'); +const Context = require('../../../../../graph/context'); const SettingsService = require('../../../../../services/settings'); const settings = { id: '1', @@ -20,19 +21,16 @@ const UsersService = require('../../../../../services/users'); describe('/api/v1/users/:user_id/email/confirm', () => { let mockUser; - beforeEach(() => - SettingsService.init(settings) - .then(() => { - return UsersService.createLocalUser( - 'ana@gmail.com', - '123321123', - 'Ana' - ); - }) - .then(user => { - mockUser = user; - }) - ); + beforeEach(async () => { + await SettingsService.init(settings); + const ctx = Context.forSystem(); + mockUser = await UsersService.createLocalUser( + ctx, + 'ana@gmail.com', + '123321123', + 'Ana' + ); + }); describe('#post', () => { it('should send an email when we hit the endpoint', () => { diff --git a/test/server/services/comments.js b/test/server/services/comments.js index 0c1a72ab7..f5b974e7b 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -1,9 +1,9 @@ -const CommentModel = require('../../../models/comment'); const ActionModel = require('../../../models/action'); - -const UsersService = require('../../../services/users'); -const SettingsService = require('../../../services/settings'); +const CommentModel = require('../../../models/comment'); const CommentsService = require('../../../services/comments'); +const Context = require('../../../graph/context'); +const SettingsService = require('../../../services/settings'); +const UsersService = require('../../../services/users'); const settings = { id: '1', @@ -119,9 +119,14 @@ describe('services.CommentsService', () => { beforeEach(async () => { await SettingsService.init(settings); + const ctx = Context.forSystem(); await Promise.all([ CommentModel.create(comments), - UsersService.createLocalUsers(users), + Promise.all( + users.map(({ email, password, username }) => + UsersService.createLocalUser(ctx, email, password, username) + ) + ), ActionModel.create(actions), ]); }); diff --git a/test/server/services/tags.js b/test/server/services/tags.js index 92e566f7c..c13cf69c9 100644 --- a/test/server/services/tags.js +++ b/test/server/services/tags.js @@ -1,8 +1,8 @@ const TagsService = require('../../../services/tags'); const UsersService = require('../../../services/users'); const SettingsService = require('../../../services/settings'); - const CommentModel = require('../../../models/comment'); +const Context = require('../../../graph/context'); const chai = require('chai'); const expect = chai.expect; @@ -11,7 +11,9 @@ describe('services.TagsService', () => { let comment, user; beforeEach(async () => { await SettingsService.init(); + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'stampi@gmail.com', '1Coral!!', 'Stampi' diff --git a/test/server/services/tokens.js b/test/server/services/tokens.js index b62aa328a..a6224eda4 100644 --- a/test/server/services/tokens.js +++ b/test/server/services/tokens.js @@ -1,6 +1,7 @@ const TokensService = require('../../../services/tokens'); const UsersService = require('../../../services/users'); const SettingsService = require('../../../services/settings'); +const Context = require('../../../graph/context'); const chai = require('chai'); chai.use(require('chai-as-promised')); @@ -10,7 +11,9 @@ describe('services.TokensService', () => { let user; beforeEach(async () => { await SettingsService.init(); + const ctx = Context.forSystem(); user = await UsersService.createLocalUser( + ctx, 'sockmonster@gmail.com', '2Coral!!', 'Sockmonster' diff --git a/test/server/services/users.js b/test/server/services/users.js index 1676f517b..e8a0172f7 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -1,6 +1,7 @@ const UsersService = require('../../../services/users'); const SettingsService = require('../../../services/settings'); const mailer = require('../../../services/mailer'); +const Context = require('../../../graph/context'); const chai = require('chai'); chai.use(require('chai-as-promised')); @@ -18,23 +19,28 @@ describe('services.UsersService', () => { }; await SettingsService.init(settings); - mockUsers = await UsersService.createLocalUsers([ - { - email: 'stampi@gmail.com', - username: 'Stampi', - password: '1Coral!-', - }, - { - email: 'sockmonster@gmail.com', - username: 'Sockmonster', - password: '2Coral!2', - }, - { - email: 'marvel@gmail.com', - username: 'Marvel', - password: '3Coral!3', - }, - ]); + const ctx = Context.forSystem(); + mockUsers = await Promise.all( + [ + { + email: 'stampi@gmail.com', + username: 'Stampi', + password: '1Coral!-', + }, + { + email: 'sockmonster@gmail.com', + username: 'Sockmonster', + password: '2Coral!2', + }, + { + email: 'marvel@gmail.com', + username: 'Marvel', + password: '3Coral!3', + }, + ].map(({ email, username, password }) => + UsersService.createLocalUser(ctx, email, password, username) + ) + ); sinon.spy(mailer, 'send'); }); @@ -58,6 +64,19 @@ describe('services.UsersService', () => { }); }); + describe('#getInitialUsername', () => { + it('should find the first result when there is no conflict', async () => { + const username = await UsersService.getInitialUsername( + 'TheGreatSockmonster' + ); + expect(username).to.equal('TheGreatSockmonster'); + }); + it('should find a first result when there is a conflict', async () => { + const username = await UsersService.getInitialUsername('Sockmonster'); + expect(username).to.match(/Sockmonster_[0-9]+/); + }); + }); + describe('#findPublicByIdArray()', () => { it('should find an array of users from an array of ids', async () => { const ids = mockUsers.map(user => user.id); @@ -89,19 +108,16 @@ describe('services.UsersService', () => { describe('#createLocalUser', () => { it('should not create a user with duplicate username', () => { - return UsersService.createLocalUsers([ - { - email: 'otrostampi@gmail.com', - username: 'StampiTheSecond', - password: '1Coralito!', - }, - ]) - .then(user => { - expect(user).to.be.null; - }) - .catch(error => { - expect(error).to.not.be.null; - }); + const ctx = Context.forSystem(); + + return expect( + UsersService.createLocalUser( + ctx, + 'otrostampi@gmail.com', + '1Coralito!', + 'Stampi' + ) + ).be.rejected; }); });