fix(teams): fix fix fix

Removed testing banner
Fixed a JS console error when assigning a user to a previously open task
Fixed a potential abuse where user might be able to score someone else's 
task via API call
Fixed an issue where finding tasks by alias could return tasks belonging 
to other users
Fixed an issue that was appending the user's party ID to their list of 
Guilds
Fixed an issue where group tasks were not receiving the default tag 
needed for filtering them on user's personal list

test/api/unit/models/task.test.js

@@ -246,13 +246,23 @@ describe('Task Model', () => {
         expect(foundTasks[0].text).to.eql(taskWithAlias.text);
       });
 
-      it('scopes alias lookup to user', async () => {
+      it('scopes alias lookup to user when querying aliases only', async () => {
         await Tasks.Task.findMultipleByIdOrAlias([taskWithAlias.alias], user._id);
 
+        expect(Tasks.Task.find).to.be.calledOnce;
+        expect(Tasks.Task.find).to.be.calledWithMatch({
+          alias: { $in: [taskWithAlias.alias] },
+          userId: user._id,
+        });
+      });
+
+      it('scopes alias lookup to user when querying aliases and IDs', async () => {
+        await Tasks.Task.findMultipleByIdOrAlias([taskWithAlias.alias, secondTask._id], user._id);
+
         expect(Tasks.Task.find).to.be.calledOnce;
         expect(Tasks.Task.find).to.be.calledWithMatch({
           $or: [
-            { _id: { $in: [] } },
+            { _id: { $in: [secondTask._id] } },
             { alias: { $in: [taskWithAlias.alias] } },
           ],
           userId: user._id,
@@ -270,10 +280,7 @@ describe('Task Model', () => {
 
         expect(Tasks.Task.find).to.be.calledOnce;
         expect(Tasks.Task.find).to.be.calledWithMatch({
-          $or: [
-            { _id: { $in: [] } },
-            { alias: { $in: [taskWithAlias.alias] } },
-          ],
+          alias: { $in: [taskWithAlias.alias] },
           userId: user._id,
           foo: 'bar',
         });

website/client/src/app.vue

@@ -42,7 +42,6 @@
           <damage-paused-banner />
           <gems-promo-banner />
           <gift-promo-banner />
-          <teams-testing-banner />
           <notifications-display />
           <app-menu />
           <div
@@ -159,7 +158,6 @@ import AppHeader from './components/header/index';
 import DamagePausedBanner from './components/header/banners/damagePaused';
 import GemsPromoBanner from './components/header/banners/gemsPromo';
 import GiftPromoBanner from './components/header/banners/giftPromo';
-import TeamsTestingBanner from './components/header/banners/teamsTesting';
 import AppFooter from './components/appFooter';
 import notificationsDisplay from './components/notifications';
 import snackbars from './components/snackbars/notifications';
@@ -196,7 +194,6 @@ export default {
     DamagePausedBanner,
     GemsPromoBanner,
     GiftPromoBanner,
-    TeamsTestingBanner,
     notificationsDisplay,
     snackbars,
     BuyModal,

website/client/src/components/header/banners/teamsTesting.vue

@@ -1,56 +0,0 @@
-<template>
-  <base-banner
-    banner-id="teams-testing"
-    banner-class="testing-banner"
-    height="48px"
-    :canClose="false"
-  >
-    <div
-      slot="content"
-      class="content m-auto d-flex justify-content-center align-items-center"
-    >
-      <span class="mr-1">
-        <strong>You're previewing new Habitica Groups!</strong>
-      </span>
-      <a href="mailto:admin@habitica.com" class="send-feedback">
-        Have a question?
-      </a>
-    </div>
-  </base-banner>
-</template>
-
-<style lang="scss" scoped>
-@import '~@/assets/scss/colors.scss';
-
-.testing-banner {
-  background-color: $black;
-  background-image: url('~@/assets/images/group-plans/purple-diagonal.png');
-
-  .content {
-    line-height: 1.71;
-    color: $white;
-  }
-
-  @media only screen and (max-width: 992px) {
-    .content {
-      font-size: 12px;
-      line-height: 1.4;
-    }
-  }
-
-  .send-feedback {
-    color: $white;
-    text-decoration: underline;
-  }
-}
-</style>
-
-<script>
-import BaseBanner from './base';
-
-export default {
-  components: {
-    BaseBanner,
-  },
-};
-</script>

website/client/src/components/snackbars/notifications.vue

@@ -143,7 +143,7 @@ export default {
         scrollPosToCheck += this.eventPromoBannerHeight ?? 0;
       }
 
-      return scrollPosToCheck + 48; // teams testing banner is always on, for now
+      return scrollPosToCheck;
     },
     visibleNotificationsWithoutErrors () {
       return this.visibleNotifications.filter(n => n.type !== 'error');

website/client/src/components/tasks/taskSummary.vue

@@ -213,7 +213,8 @@ export default {
       user: 'user.data',
     }),
     assignedUsernames () {
-      if (!this.task.group || !this.task.group.assignedUsers) return [];
+      if (!this.task.group || !this.task.group.assignedUsers
+        || !this.task.group.assignedUsersDetail) return [];
       const usernames = [];
       for (const user of this.task.group.assignedUsers) {
         usernames.push(this.task.group.assignedUsersDetail[user].assignedUsername);

website/common/script/ops/scoreTask.js

@@ -4,6 +4,7 @@ import reduce from 'lodash/reduce';
 import moment from 'moment';
 import max from 'lodash/max';
 import {
+  BadRequest,
   NotAuthorized,
 } from '../libs/errors';
 import i18n from '../i18n';
@@ -247,6 +248,12 @@ export default function scoreTask (options = {}, req = {}, analytics) {
 
   // If they're trying to purchase a too-expensive reward, don't allow them to do that.
   if (task.value > user.stats.gp && task.type === 'reward') throw new NotAuthorized(i18n.t('messageNotEnoughGold', req.language));
+  // Thanks to open group tasks, userId is not guaranteed. Don't allow scoring inaccessible tasks
+  if (task.userId && task.userId !== user._id) {
+    throw new BadRequest('Cannot score task belonging to another user.');
+  } else if (user.guilds.indexOf(task.group.id) === -1 && user.party._id !== task.group.id) {
+    throw new BadRequest('Cannot score task belonging to another user.');
+  }
 
   if (task.type === 'habit') {
     delta += _changeTaskValue(user, task, direction, times, cron);

website/server/controllers/api-v3/tasks.js

@@ -816,7 +816,7 @@ api.moveTask = {
     const group = await getGroupFromTaskAndUser(task, user);
     const challenge = await getChallengeFromTask(task);
     if (task.group.id && !task.userId) {
-      if (!group || user.guilds.concat(user.party._id).indexOf(group._id) === -1) {
+      if (!group || (user.guilds.indexOf(group._id) === -1 && user.party._id !== group._id)) {
         throw new NotFound(res.t('groupNotFound'));
       }
       if (task.group.assignedUsers.length !== 0

website/server/libs/tasks/index.js

@@ -67,9 +67,7 @@ async function createTasks (req, res, options = {}) {
       newTask.challenge.id = challenge.id;
     } else if (group) {
       newTask.group.id = group._id;
-      if (taskData.requiresApproval) {
-        newTask.group.approval.required = true;
-      }
+      newTask.tags = [group._id];
       newTask.group.managerNotes = taskData.managerNotes || '';
     } else {
       newTask.userId = user._id;

website/server/models/task.js

@@ -205,7 +205,7 @@ TaskSchema.statics.findByIdOrAlias = async function findByIdOrAlias (
   return task;
 };
 
-TaskSchema.statics.findMultipleByIdOrAlias = async function findByIdOrAlias (
+TaskSchema.statics.findMultipleByIdOrAlias = async function findMultipleByIdOrAlias (
   identifiers,
   userId,
   additionalQueries = {},
@@ -226,6 +226,7 @@ TaskSchema.statics.findMultipleByIdOrAlias = async function findByIdOrAlias (
   });
 
   if (ids.length > 0 && aliases.length > 0) {
+    query.userId = userId;
     query.$or = [
       { _id: { $in: ids } },
       { alias: { $in: aliases } },
@@ -233,6 +234,7 @@ TaskSchema.statics.findMultipleByIdOrAlias = async function findByIdOrAlias (
   } else if (ids.length > 0) {
     query._id = { $in: ids };
   } else if (aliases.length > 0) {
+    query.userId = userId;
     query.alias = { $in: aliases };
   } else {
     throw new Error('No identifiers found.'); // Should be covered by the !identifiers check, but..