fix: fixed incorrect users lookup (#2229)

A few updates plus corrections.

docs/source/07-01-faq.md

@@ -33,24 +33,24 @@ Our team is small, so it's difficult for us to provide support packages. However
 
 ## Is there a hosted version I can purchase by monthly subscription?
 
-Yes! We are happy to announce that as of July 2018, we provide a SaaS version of Talk, called the Coral Cloud. For a monthly subscription, you get your own hosted Talk instance to embed on your news and blog articles. [Reach out to us](mailto:support@coralproject.net) if you're interested in this option.
+Yes! [Visit our hosting page](https://coralproject.net/pricing/) to submit information and receive a quote.
 
 ## Where is our data when we use Talk?
 
 If you are hosting Talk on your own:
 
 * Your data is stored in a MongoDB database that you provide
-* The Coral Team doesn’t have any access to your data
+* The Coral team doesn’t have any access to your data
 
-If you are using Coral Cloud Hosting for Talk:
+If you are using our Hosted SaaS version of Talk:
 
-* Your data is stored in a dedicated MongDB database that is provisioned for your in the Cloud
-* Your data is completely isolated from other customer’s data
-* The Coral Team and its third party database hosting providers use strict access controls and auditing to protected your data from unauthorized access by team members
+* Your data is stored in a dedicated MongoDB database that is provisioned for you
+* Your data is completely isolated from other customers' data
+* The Coral Team and its third-party database hosting providers use strict access controls and auditing to protected your data from unauthorized access
 
 ## Does Talk have any automated moderation features to protect against spam and trolling?
 
-Talk features a couple of plugins that provide advanced moderation:
+As well as basic word/phrase filtering and user reputation scores, Talk offers optional advanced features via third-party services:
 
 * The [Toxic Comments plugin](/talk/plugin/talk-plugin-toxic-comments) integrates with the [Perspective API from Google](https://www.perspectiveapi.com/) to detect the likelihood of toxicity of comments in real-time
 * The [Akismet plugin](/talk/plugin/talk-plugin-akismet) detects and blocks spam comments
@@ -64,4 +64,4 @@ Talk features a couple of plugins that provide advanced moderation:
 
 * The Talk software is freely available under the Apache 2.0 open source license
 * Associated costs are those for the infrastructure required to run Talk (i.e. cloud hosting fee or bare-metal server costs)
-* The Coral Project offers a SaaS version of Talk. Please [get in touch with us](mailto:support@coralproject.net) to discuss pricing for your custom requirements.
+* The Coral Project offers a SaaS/hosted version of Talk. Please [get in touch with us](https://coralproject.net/pricing/) to discuss pricing for your requirements.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "talk",
-  "version": "4.8.1",
+  "version": "4.8.3",
   "description": "A better commenting experience from Mozilla, The New York Times, and the Washington Post. https://coralproject.net",
   "main": "app.js",
   "private": true,

services/users.js

@@ -60,12 +60,17 @@ async function upsertUser(
   shouldSetDisplayName = false
 ) {
   let user = await User.findOne({
-    profiles: {
-      $elemMatch: {
-        id,
-        provider,
+    $or: [
+      { id },
+      {
+        profiles: {
+          $elemMatch: {
+            id,
+            provider,
+          },
+        },
       },
-    },
+    ],
   });
   if (user) {
     user.wasUpserted = false;
@@ -80,6 +85,7 @@ async function upsertUser(
 
   // The user was not found, lets create them!
   user = new User({
+    id,
     username,
     lowercaseUsername: username.toLowerCase(),
     profiles: [{ id, provider }],
@@ -798,8 +804,13 @@ class Users {
    * @param {Object} token  a jwt token used to sign in the user
    */
   static async findOrCreateByIDToken(id, token) {
+    // FIXME: (wyattjoh) adding the `{ 'profiles.id': id }` search condition is
+    // a workaround for the `upsertExternalUser` bug where the ID being set was
+    // incorrect. Patch the user creation functions to adapt to the correct
+    // behavior. Thankfully, this is currently still covered by an index.
+
     // Try to get the user.
-    let user = await User.findOne({ id });
+    let user = await User.findOne({ $or: [{ id }, { 'profiles.id': id }] });
 
     // If the user was not found, try to look it up.
     if (user === null) {

test/helpers/mongoose.js

@@ -1,15 +1,31 @@
 const mongoose = require('../../services/mongoose');
 
-before(function(done) {
+const models = [
+  require('../../models/action'),
+  require('../../models/asset'),
+  require('../../models/comment'),
+  require('../../models/migration'),
+  require('../../models/setting'),
+  require('../../models/user'),
+  require('../../models/migration'),
+];
+
+before(async function() {
   this.timeout(30000);
 
-  mongoose.connection.on('open', function(err) {
-    if (err) {
-      return done(err);
-    }
+  // Ensure we can connect to the database.
+  await new Promise((resolve, reject) => {
+    mongoose.connection.on('open', err => {
+      if (err) {
+        return reject(err);
+      }
 
-    return done();
+      return resolve();
+    });
   });
+
+  // Ensure all the models have indexes created.
+  await Promise.all(models.map(model => model.ensureIndexes()));
 });
 
 beforeEach(async () => {

test/server/services/users.js

@@ -4,6 +4,7 @@ const mailer = require('../../../services/mailer');
 const Context = require('../../../graph/context');
 const timekeeper = require('timekeeper');
 const moment = require('moment');
+const uuid = require('uuid/v1');
 
 const chai = require('chai');
 chai.use(require('chai-as-promised'));
@@ -387,6 +388,64 @@ describe('services.UsersService', () => {
       expect(user.wasUpserted).to.be.false;
     });
 
+    it('should handle legacy users as well', async () => {
+      const ctx = Context.forSystem();
+      let user = await UsersService.upsertExternalUser(
+        ctx,
+        'an-id',
+        'a-provider',
+        'a-display-name'
+      );
+
+      expect(user).to.be.defined;
+      expect(user).to.have.property('id', 'an-id');
+      expect(user.wasUpserted).to.be.true;
+
+      // Change the ID to something else, mirroring the legacy behavior.
+      const id = uuid();
+      user.id = id;
+      await user.save();
+      expect(user).to.have.property('id', id);
+
+      // Ensure that the ID has been changed.
+      user = await UsersService.findById(id);
+
+      expect(user).to.be.defined;
+      expect(user).to.have.property('id', id);
+
+      // Test to see that future lookups work.
+      user = await UsersService.upsertExternalUser(
+        ctx,
+        'an-id',
+        'a-provider',
+        'a-display-name'
+      );
+
+      expect(user).to.be.defined;
+      expect(user).to.have.property('id', id);
+      expect(user.wasUpserted).to.be.false;
+    });
+
+    it('should handle token user lookups created via this method', async () => {
+      const id = uuid();
+      const ctx = Context.forSystem();
+      let user = await UsersService.upsertExternalUser(
+        ctx,
+        id,
+        'a-provider',
+        'a-display-name'
+      );
+
+      expect(user).to.be.defined;
+      expect(user).to.have.property('id', id);
+      expect(user.wasUpserted).to.be.true;
+
+      user = await UsersService.findOrCreateByIDToken(id, {});
+
+      expect(user).to.be.defined;
+      expect(user).to.have.property('id', id);
+    });
+
     it('should return a user when the desired user is not found', async () => {
       const ctx = Context.forSystem();
       let user = await UsersService.upsertExternalUser(