Merge pull request #1737 from coralproject/username-status

findOrCreateExternalUser Deprecation

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

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**

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,

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,

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,

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);
   }
 
   /**