fix issue with language being undefined, refactoring auth middleware, new tests

2025-12-17 14:47:53 +01:00 · 2019-05-15 16:54:55 +02:00
parent 8cce38ede1
commit eff8db0afd
7 changed files with 143 additions and 91 deletions
--- a/test/api/unit/middlewares/auth.test.js
+++ b/test/api/unit/middlewares/auth.test.js
@@ -16,7 +16,7 @@ describe('auth middleware', () => {
  describe('auth with headers', () => {
    it('allows to specify a list of user field that we do not want to load', (done) => {
      const authWithHeaders = authWithHeadersFactory({
-        userFieldsToExclude: ['items', 'flags', 'auth.timestamps'],
+        userFieldsToExclude: ['items'],
      });

      req.headers['x-api-user'] = user._id;
@@ -27,11 +27,34 @@ describe('auth middleware', () => {

        const userToJSON = res.locals.user.toJSON();
        expect(userToJSON.items).to.not.exist;
-        expect(userToJSON.flags).to.not.exist;
-        expect(userToJSON.auth.timestamps).to.not.exist;
+        expect(userToJSON.auth).to.exist;
+
+        done();
+      });
+    });
+
+    it('makes sure some fields are always included', (done) => {
+      const authWithHeaders = authWithHeadersFactory({
+        userFieldsToExclude: [
+          'items', 'auth.timestamps',
+          'preferences', 'notifications', '_id', 'flags', 'auth', // these are always loaded
+        ],
+      });
+
+      req.headers['x-api-user'] = user._id;
+      req.headers['x-api-key'] = user.apiToken;
+
+      authWithHeaders(req, res, (err) => {
+        if (err) return done(err);
+
+        const userToJSON = res.locals.user.toJSON();
+        expect(userToJSON.items).to.not.exist;
+        expect(userToJSON.auth.timestamps).to.exist;
        expect(userToJSON.auth).to.exist;
        expect(userToJSON.notifications).to.exist;
        expect(userToJSON.preferences).to.exist;
+        expect(userToJSON._id).to.exist;
+        expect(userToJSON.flags).to.exist;

        done();
      });
--- a/test/api/unit/models/group.test.js
+++ b/test/api/unit/models/group.test.js
@@ -769,7 +769,7 @@ describe('Group Model', () => {
    });

    describe('translateSystemMessages', () => {
-      it('translate quest_start', () => {
+      it('translate quest_start', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -777,12 +777,11 @@ describe('Group Model', () => {
            quest: 'basilist',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate boss_damage', () => {
+      it('translate boss_damage', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -793,12 +792,11 @@ describe('Group Model', () => {
            bossDamage: 3.7,
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate boss_dont_attack', () => {
+      it('translate boss_dont_attack', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -808,12 +806,11 @@ describe('Group Model', () => {
            userDamage: 15.3,
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate boss_rage', () => {
+      it('translate boss_rage', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -821,12 +818,11 @@ describe('Group Model', () => {
            quest: 'lostMasterclasser3',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate boss_defeated', () => {
+      it('translate boss_defeated', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -834,12 +830,11 @@ describe('Group Model', () => {
            quest: 'lostMasterclasser3',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate user_found_items', () => {
+      it('translate user_found_items', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -853,24 +848,22 @@ describe('Group Model', () => {
            },
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate all_items_found', () => {
+      it('translate all_items_found', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
            type: 'all_items_found',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate spell_cast_party', () => {
+      it('translate spell_cast_party', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -880,12 +873,11 @@ describe('Group Model', () => {
            spell: 'earth',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate spell_cast_user', () => {
+      it('translate spell_cast_user', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -896,12 +888,11 @@ describe('Group Model', () => {
            target: participatingMember.profile.name,
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate quest_cancel', () => {
+      it('translate quest_cancel', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -910,12 +901,11 @@ describe('Group Model', () => {
            quest: 'basilist',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate quest_abort', () => {
+      it('translate quest_abort', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -924,12 +914,11 @@ describe('Group Model', () => {
            quest: 'basilist',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate tavern_quest_completed', () => {
+      it('translate tavern_quest_completed', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -937,12 +926,11 @@ describe('Group Model', () => {
            quest: 'stressbeast',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate tavern_boss_rage_tired', () => {
+      it('translate tavern_boss_rage_tired', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -950,12 +938,11 @@ describe('Group Model', () => {
            quest: 'stressbeast',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate tavern_boss_rage', () => {
+      it('translate tavern_boss_rage', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -964,12 +951,11 @@ describe('Group Model', () => {
            scene: 'market',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate tavern_boss_desperation', () => {
+      it('translate tavern_boss_desperation', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -977,12 +963,11 @@ describe('Group Model', () => {
            quest: 'stressbeast',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });

-      it('translate claim_task', () => {
+      it('translate claim_task', async () => {
        questLeader.preferences.language = 'en';
        party.chat = [{
          info: {
@@ -991,11 +976,49 @@ describe('Group Model', () => {
            task: 'Feed the pet',
          },
        }];
-        let toJSON = party.toJSON();
-        Group.translateSystemMessages(toJSON, questLeader);
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
        translationCheck(toJSON.chat[0].text);
      });
    });
+
+    describe('toJSONCleanChat', () => {
+      it('shows messages with 1 flag to non-admins', async () => {
+        party.chat = [{
+          flagCount: 1,
+          info: {
+            type: 'quest_start',
+            quest: 'basilist',
+          },
+        }];
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
+        expect(toJSON.chat.length).to.equal(1);
+      });
+
+      it('shows messages with >= 2 flag to admins', async () => {
+        party.chat = [{
+          flagCount: 3,
+          info: {
+            type: 'quest_start',
+            quest: 'basilist',
+          },
+        }];
+        const admin = new User({'contributor.admin': true});
+        let toJSON = await Group.toJSONCleanChat(party, admin);
+        expect(toJSON.chat.length).to.equal(1);
+      });
+
+      it('doesn\'t show flagged messages to non-admins', async () => {
+        party.chat = [{
+          flagCount: 3,
+          info: {
+            type: 'quest_start',
+            quest: 'basilist',
+          },
+        }];
+        let toJSON = await Group.toJSONCleanChat(party, questLeader);
+        expect(toJSON.chat.length).to.equal(0);
+      });
+    });
  });

  context('Instance Methods', () => {
--- a/website/server/controllers/api-v3/challenges.js
+++ b/website/server/controllers/api-v3/challenges.js
@@ -442,7 +442,8 @@ api.getGroupChallenges = {
  method: 'GET',
  url: '/challenges/groups/:groupId',
  middlewares: [authWithHeaders({
-    userFieldsToInclude: ['_id', 'party', 'guilds'],
+    // Some fields (including _id) are always loaded (see middlewares/auth)
+    userFieldsToInclude: ['party', 'guilds'], // Some fields are always loaded (see middlewares/auth)
  })],
  async handler (req, res) {
    let user = res.locals.user;
--- a/website/server/controllers/api-v3/groups.js
+++ b/website/server/controllers/api-v3/groups.js
@@ -375,7 +375,8 @@ api.getGroup = {
  method: 'GET',
  url: '/groups/:groupId',
  middlewares: [authWithHeaders({
-    userFieldsToInclude: ['_id', 'party', 'guilds', 'contributor'],
+    // Some fields (including _id, preferences) are always loaded (see middlewares/auth)
+    userFieldsToInclude: ['party', 'guilds', 'contributor'],
  })],
  async handler (req, res) {
    let user = res.locals.user;
--- a/website/server/controllers/api-v3/tasks.js
+++ b/website/server/controllers/api-v3/tasks.js
@@ -285,7 +285,8 @@ api.getUserTasks = {
  method: 'GET',
  url: '/tasks/user',
  middlewares: [authWithHeaders({
-    userFieldsToInclude: ['_id', 'tasksOrder', 'preferences'],
+    // Some fields (including _id, preferences) are always loaded (see middlewares/auth)
+    userFieldsToInclude: ['tasksOrder'],
  })],
  async handler (req, res) {
    let types = Tasks.tasksTypes.map(type => `${type}s`);
--- a/website/server/middlewares/auth.js
+++ b/website/server/middlewares/auth.js
@@ -9,22 +9,27 @@ import url from 'url';
 import gcpStackdriverTracer from '../libs/gcpTraceAgent';

 const COMMUNITY_MANAGER_EMAIL = nconf.get('EMAILS_COMMUNITY_MANAGER_EMAIL');
+const USER_FIELDS_ALWAYS_LOADED = ['_id', 'notifications', 'preferences', 'auth', 'flags'];

 function getUserFields (options, req) {
  // A list of user fields that aren't needed for the route and are not loaded from the db.
  // Must be an array
  if (options.userFieldsToExclude) {
-    return options.userFieldsToExclude.map(field => {
+    return options.userFieldsToExclude
+      .filter(field => {
+        return !USER_FIELDS_ALWAYS_LOADED.find(fieldToInclude => field.startsWith(fieldToInclude));
+      })
+      .map(field => {
        return `-${field}`; // -${field} means exclude ${field} in mongodb
-    }).join(' ');
+      })
+      .join(' ');
  }

  if (options.userFieldsToInclude) {
-    return options.userFieldsToInclude.join(' ');
+    return options.userFieldsToInclude.concat(USER_FIELDS_ALWAYS_LOADED).join(' ');
  }

  // Allows GET requests to /user to specify a list of user fields to return instead of the entire doc
-  // Notifications are always included
  const urlPath = url.parse(req.url).pathname;
  const userFields = req.query.userFields;
  if (!userFields || urlPath !== '/user') return '';
@@ -32,7 +37,7 @@ function getUserFields (options, req) {
  const userFieldOptions = userFields.split(',');
  if (userFieldOptions.length === 0) return '';

-  return `notifications ${userFieldOptions.join(' ')}`;
+  return userFieldOptions.concat(USER_FIELDS_ALWAYS_LOADED).join(' ');
 }

 // Make sure stackdriver traces are storing the user id
--- a/website/server/models/group.js
+++ b/website/server/models/group.js
@@ -333,19 +333,6 @@ schema.statics.getGroups = async function getGroups (options = {}) {
  return groupsArray;
 };

-function _translateSystemMessages (toJSON, user) {
-  let lang = user.preferences ? user.preferences.language : 'en';
-
-  toJSON.chat.map(chat => {
-    if (!_.isEmpty(chat.info)) {
-      chat.text = translateMessage(lang, chat.info);
-    }
-    return chat;
-  });
-}
-
-schema.statics.translateSystemMessages = _translateSystemMessages;
-
 // When converting to json remove chat messages with more than 1 flag and remove all flags info
 // unless the user is an admin or said chat is posted by that user
 // Not putting into toJSON because there we can't access user
@@ -357,27 +344,38 @@ schema.statics.toJSONCleanChat = async function groupToJSONCleanChat (group, use
    await getGroupChat(group);
  }

-  let toJSON = group.toJSON();
-  _translateSystemMessages(toJSON, user);
+  const groupToJson = group.toJSON();
+  const userLang = user.preferences.language;

-  if (!user.contributor.admin) {
-    _.remove(toJSON.chat, chatMsg => {
-      chatMsg.flags = {};
-      if (chatMsg._meta) chatMsg._meta = undefined;
-      return user._id !== chatMsg.uuid && chatMsg.flagCount >= 2;
-    });
+  groupToJson.chat = groupToJson.chat
+    .map(chatMsg => {
+      // Translate system messages
+      if (!_.isEmpty(chatMsg.info)) {
+        chatMsg.text = translateMessage(userLang, chatMsg.info);
      }

      // Convert to timestamps because Android expects it
-  toJSON.chat.forEach(chat => {
      // old chats are saved with a numeric timestamp
      // new chats use `Date` which then has to be converted to the numeric timestamp
-    if (chat.timestamp && chat.timestamp.getTime) {
-      chat.timestamp = chat.timestamp.getTime();
+      if (chatMsg.timestamp && chatMsg.timestamp.getTime) {
+        chatMsg.timestamp = chatMsg.timestamp.getTime();
      }
-  });

-  return toJSON;
+      if (!user.contributor.admin) {
+        // Flags are hidden to non admins
+        chatMsg.flags = {};
+        if (chatMsg._meta) chatMsg._meta = undefined;
+
+        // Messages with >= 2 flags are hidden to non admins and non authors
+        if (user._id !== chatMsg.uuid && chatMsg.flagCount >= 2) return undefined;
+      }
+
+      return chatMsg;
+    })
+    // Used to filter for undefined chat messages that should not be shown to non-admins
+    .filter(chatMsg => chatMsg !== undefined);
+
+  return groupToJson;
 };

 function getInviteError (uuids, emails, usernames) {