From 23156f3d5e137cead89c325861aeaed1e47b9c09 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 6 Jul 2018 14:28:25 -0600 Subject: [PATCH] feat: added deprecations, docs --- docs/_config.yml | 2 + docs/source/06-03-migrating-from-3.md | 70 ++++++++++ docs/source/api/server.md | 2 +- .../server/passport.js | 2 +- .../server/passport.js | 2 +- services/users.js | 126 ++++++++++++------ 6 files changed, 161 insertions(+), 43 deletions(-) create mode 100644 docs/source/06-03-migrating-from-3.md diff --git a/docs/_config.yml b/docs/_config.yml index 5a2020c1c..fea4db246 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -164,6 +164,8 @@ sidebar: url: /when-youve-installed-talk/ - title: Migrating children: + - title: Migrating from v3.x.x + url: /migration/3/ - title: Migrating to v4.0.0 url: /migration/4/ - title: Migrating to v4.1.0 diff --git a/docs/source/06-03-migrating-from-3.md b/docs/source/06-03-migrating-from-3.md new file mode 100644 index 000000000..b6551bb1a --- /dev/null +++ b/docs/source/06-03-migrating-from-3.md @@ -0,0 +1,70 @@ +--- +title: Migrating from v3.x.x +permalink: /migration/3/ +--- + +## Deprecation Notices + +It was previously recommended to use the user service function: + +```js +Users.findOrCreateExternalUser(...); +``` + +If you are developing a social plugin, you should migrate this function to: + +```js +Users.upsertSocialUser(...); +``` + +If you are developing an external auth integration (where the integration +provides) a custom displayName, you should migrate to: + +```js +Users.upsertExternalUser(...); +``` + +## Troubleshooting Username Status + +You may be affected by a side-effect of the above mentioned deprecated function +`Users.findOrCreateExternalUser(...);` if the following are true: + +1. You have upgraded from Talk `< 3` to `>= 4` and have completed a database + migration +2. You have used a custom auth plugin in the past +3. You have disabled or not included the `talk-plugin-auth` as a `client` plugin +4. You have received reports that some users can not comment, and are instead + given a message `You are not authorized to perform this action.` + +If this is the case, you can execute the following one time MongoDB query to +repair the affected users. + +```js +db.users.update( + { + "status.username.status": { + $in: ["UNSET", "CHANGED"] + } + }, + { + $set: { + "status.username.status": "SET" + }, + $push: { + "status.username.history": { + status: "SET", + assigned_by: null, + created_at: ISODate() + } + } + }, + { + multi: true + } +); + +``` + +**Note: You must resolve and/or update your custom auth code to resolve the +above mentioned deprecation notices _before_ running the above mentioned MongoDB +query** diff --git a/docs/source/api/server.md b/docs/source/api/server.md index d25487c2d..8ead459f9 100644 --- a/docs/source/api/server.md +++ b/docs/source/api/server.md @@ -319,7 +319,7 @@ module.exports = { let user; try { const { id, provider, displayName } = profile; - user = await UsersService.findOrCreateExternalUser( + user = await UsersService.upsertSocialUser( req.context, id, provider, diff --git a/plugins/talk-plugin-facebook-auth/server/passport.js b/plugins/talk-plugin-facebook-auth/server/passport.js index 7bec1799a..b1603ee03 100644 --- a/plugins/talk-plugin-facebook-auth/server/passport.js +++ b/plugins/talk-plugin-facebook-auth/server/passport.js @@ -27,7 +27,7 @@ module.exports = passport => { try { const { id, provider, displayName } = profile; - user = await UsersService.findOrCreateExternalUser( + user = await UsersService.upsertSocialUser( req.context, id, provider, diff --git a/plugins/talk-plugin-google-auth/server/passport.js b/plugins/talk-plugin-google-auth/server/passport.js index 8386f319c..48ac6e2ef 100644 --- a/plugins/talk-plugin-google-auth/server/passport.js +++ b/plugins/talk-plugin-google-auth/server/passport.js @@ -26,7 +26,7 @@ module.exports = passport => { try { const { id, provider, displayName } = profile; - user = await UsersService.findOrCreateExternalUser( + user = await UsersService.upsertSocialUser( req.context, id, provider, diff --git a/services/users.js b/services/users.js index 061fae6fb..a12d10b35 100644 --- a/services/users.js +++ b/services/users.js @@ -48,6 +48,62 @@ const loginRateLimiter = new Limit( RECAPTCHA_WINDOW ); +// upsertUser will try to lookup the user by their profile. If the user cannot +// be looked up, it will create one with a unique username and the designated +// username status. +async function upsertUser( + ctx, + id, + provider, + displayName, + usernameStatus, + shouldSetDisplayName = false +) { + let user = await User.findOne({ + profiles: { + $elemMatch: { + id, + provider, + }, + }, + }); + if (user) { + return user; + } + + // User does not exist and need to be created. + + // Create an initial username for the user. + let username = await Users.getInitialUsername(displayName); + + // The user was not found, lets create them! + user = new User({ + username, + lowercaseUsername: username.toLowerCase(), + profiles: [{ id, provider }], + status: { + username: { + status: usernameStatus, + history: { + status: usernameStatus, + }, + }, + }, + }); + + if (shouldSetDisplayName) { + // Set the displayName on the user metadata so that it can be accessed. + user.metadata = user.metadata || {}; + user.metadata.displayName = displayName; + } + + // Save the user in the database. + await user.save(); + + // Emit that the user was created. + ctx.pubsub.publish('userCreated', user); +} + // Users is the interface for the application to interact with the // User through. class Users { @@ -478,52 +534,42 @@ class Users { ); } + /** + * upsertExternalUser will create or lookup a user where the user will not be + * able to change their username. + * + * @param {Object} ctx the graph context + * @param {String} id the ID for the user from the provider + * @param {String} provider the name of the provider + * @param {String} displayName the users desired displayName, not guaranteed + */ + static async upsertExternalUser(ctx, id, provider, displayName) { + return upsertUser(ctx, id, provider, displayName, 'SET', true); + } + + /** + * upsertSocialUser will create or lookup a user as provided from a social + * graph. + * + * @param {Object} ctx the graph context + * @param {String} id the ID for the user from the provider + * @param {String} provider the name of the provider + * @param {String} displayName the users desired displayName, not guaranteed + */ + static async upsertSocialUser(ctx, id, provider, displayName) { + return upsertUser(ctx, id, provider, displayName, 'UNSET'); + } + /** * 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(ctx, id, provider, displayName) { - let user = await User.findOne({ - profiles: { - $elemMatch: { - id, - provider, - }, - }, - }); - if (user) { - return user; - } + ctx.log.warn( + 'findOrCreateExternalUser is deprecated and will be removed in a future version, replace with upsertExternalUser' + ); - // User does not exist and need to be created. - - // Create an initial username for the user. - let username = await Users.getInitialUsername(displayName); - - // The user was not found, lets create them! - user = new User({ - username, - lowercaseUsername: username.toLowerCase(), - profiles: [{ id, provider }], - status: { - username: { - status: 'UNSET', - history: { - status: 'UNSET', - }, - }, - }, - }); - - // Save the user in the database. - await user.save(); - - // Emit that the user was created. - ctx.pubsub.publish('userCreated', user); - - return user; + return Users.upsertSocialUser(ctx, id, provider, displayName); } /**