Allow guilds edit (#8800)

* test: test that admin users can update guilds

* test: test admin removeMember privileges

* fix: allow admins to edit guilds

* fix: add edit guild options for admins

* test: test that admin can't remove current leader

* Add error msg for removing current leader

* Taskwoods Quest Line (#8156)

* feat(content): Gold Quest 2016-10

* chore(news): Bailey

* chore(i18n): update locales

* chore(sprites): compile

* 3.49.0

* chore: update express

* Fix for the ReDOS vulnerability

habitica is currently affected by the high-severity [ReDOS vulnerability](https://snyk.io/vuln/npm:tough-cookie:20160722). 

Vulnerable module: `tough-cookie`
Introduced through: ` request`

This PR fixes the ReDOS vulnerability by upgrading ` request` to version 2.74.0

Check out the [Snyk test report](https://snyk.io/test/github/HabitRPG/habitica) to review other vulnerabilities that affect this repo. 

[Watch the repo](https://snyk.io/add) to 
* get alerts if newly disclosed vulnerabilities affect this repo in the future. 
* generate pull requests with the fixes you want, or let us do the work: when a newly disclosed vulnerability affects you, we'll submit a fix to you right away. 

Stay secure, 
The Snyk team

* Documentation - coupon

closes #8109

* fix(client): Allow member hp to be clickable

fixes #8016
closes #8155

* chore(npm): shrinkwrap

* test: test isAbleToEditGroup

* Add isAbleToEditGroup to groupsCtrl

* Remove unnecessary ternary

* Fix linting

* Move edit permission logic out to groupsCtrl

* fix: change ternary to boolean

* Fix linting

* Fixed merge issues

test/api/v3/integration/groups/POST-groups_id_removeMember.test.js

@@ -11,6 +11,7 @@ describe('POST /groups/:groupId/removeMember/:memberId', () => {
   let guild;
   let member;
   let member2;
+  let adminUser;
 
   beforeEach(async () => {
     let { group, groupLeader, invitees, members } = await createAndPopulateGroup({
@@ -28,6 +29,7 @@ describe('POST /groups/:groupId/removeMember/:memberId', () => {
     invitedUser = invitees[0];
     member = members[0];
     member2 = members[1];
+    adminUser = await generateUser({ 'contributor.admin': true });
   });
 
   context('All Groups', () => {
@@ -42,7 +44,7 @@ describe('POST /groups/:groupId/removeMember/:memberId', () => {
         });
     });
 
-    it('returns an error when user is a non-leader member of a group', async () => {
+    it('returns an error when user is a non-leader member of a group and not an admin', async () => {
       expect(member2.post(`/groups/${guild._id}/removeMember/${member._id}`))
         .to.eventually.be.rejected.and.eql({
           code: 401,
@@ -87,7 +89,30 @@ describe('POST /groups/:groupId/removeMember/:memberId', () => {
 
       let invitedUserWithoutInvite = await invitedUser.get('/user');
 
-      expect(_.findIndex(invitedUserWithoutInvite.invitations.guilds, {id: guild._id})).eql(-1);
+      expect(_.findIndex(invitedUserWithoutInvite.invitations.guilds, { id: guild._id })).eql(-1);
+    });
+
+    it('allows an admin to remove other members', async () => {
+      await adminUser.post(`/groups/${guild._id}/removeMember/${member._id}`);
+      let memberRemoved = await member.get('/user');
+
+      expect(memberRemoved.guilds.indexOf(guild._id)).eql(-1);
+    });
+
+    it('allows an admin to remove other invites', async () => {
+      await adminUser.post(`/groups/${guild._id}/removeMember/${invitedUser._id}`);
+
+      let invitedUserWithoutInvite = await invitedUser.get('/user');
+
+      expect(_.findIndex(invitedUserWithoutInvite.invitations.guilds, { id: guild._id })).eql(-1);
+    });
+
+    it('does not allow an admin to remove a leader', async () => {
+      expect(adminUser.post(`/groups/${guild._id}/removeMember/${leader._id}`))
+        .to.eventually.be.rejected.and.eql({
+          code: 401,
+          text: t('cannotRemoveCurrentLeader'),
+        });
     });
 
     it('sends email to user with rescinded invite', async () => {

test/api/v3/integration/groups/PUT-groups.test.js

@@ -1,10 +1,11 @@
 import {
   createAndPopulateGroup,
+  generateUser,
   translate as t,
 } from '../../../../helpers/api-v3-integration.helper';
 
 describe('PUT /group', () => {
-  let leader, nonLeader, groupToUpdate;
+  let leader, nonLeader, groupToUpdate, adminUser;
   let groupName = 'Test Public Guild';
   let groupType = 'guild';
   let groupUpdatedName = 'Test Public Guild Updated';
@@ -18,13 +19,13 @@ describe('PUT /group', () => {
       },
       members: 1,
     });
-
+    adminUser = await generateUser({ 'contributor.admin': true });
     groupToUpdate = group;
     leader = groupLeader;
     nonLeader = members[0];
   });
 
-  it('returns an error when a non group leader tries to update', async () => {
+  it('returns an error when a user that is not an admin or group leader tries to update', async () => {
     await expect(nonLeader.put(`/groups/${groupToUpdate._id}`, {
       name: groupUpdatedName,
     })).to.eventually.be.rejected.and.eql({
@@ -44,6 +45,15 @@ describe('PUT /group', () => {
     expect(updatedGroup.name).to.equal(groupUpdatedName);
   });
 
+  it('allows an admin to update a guild', async () => {
+    let updatedGroup = await adminUser.put(`/groups/${groupToUpdate._id}`, {
+      name: groupUpdatedName,
+    });
+    expect(updatedGroup.leader._id).to.eql(leader._id);
+    expect(updatedGroup.leader.profile.name).to.eql(leader.profile.name);
+    expect(updatedGroup.name).to.equal(groupUpdatedName);
+  });
+
   it('allows a leader to change leaders', async () => {
     let updatedGroup = await leader.put(`/groups/${groupToUpdate._id}`, {
       name: groupUpdatedName,

test/client-old/spec/controllers/groupCtrlSpec.js

@@ -112,6 +112,41 @@ describe('Groups Controller', function() {
     });
   });
 
+  describe('isAbleToEditGroup', () => {
+    var guild;
+
+    beforeEach(() => {
+      user.contributor = {};
+      guild = specHelper.newGroup({
+        _id: 'unique-guild-id',
+        type: 'guild',
+        members: ['not-user-id'],
+        $save: sandbox.spy(),
+      });
+    });
+
+    it('returns true if user is an admin', () => {
+      guild.leader = 'not-user-id';
+      user.contributor.admin = true;
+      expect(scope.isAbleToEditGroup(guild)).to.be.ok;
+    });
+
+    it('returns true if user is group leader', () => {
+      guild.leader = {_id: user._id}
+      expect(scope.isAbleToEditGroup(guild)).to.be.ok;
+    });
+
+    it('returns false is user is not a leader or admin', () => {
+      expect(scope.isAbleToEditGroup(guild)).to.not.be.ok;
+    });
+
+    it('returns false is user is an admin but group is a party', () => {
+      guild.type = 'party';
+      user.contributor.admin = true;
+      expect(scope.isAbleToEditGroup(guild)).to.not.be.ok;
+    });
+  });
+
   describe('editGroup', () => {
     var guild;
 

website/client-old/js/controllers/groupsCtrl.js

@@ -35,6 +35,12 @@ habitrpg.controller("GroupsCtrl", ['$scope', '$rootScope', 'Shared', 'Groups', '
       return ~(memberIds.indexOf(userid));
     };
 
+    $scope.isAbleToEditGroup = function (group) {
+      if (group.leader._id === User.user._id) return true;
+      if (User.user.contributor.admin && group.type === "guild") return true;
+      return false;
+    };
+
     $scope.isMember = function (user, group) {
       return ~(group.members.indexOf(user._id));
     };

website/common/locales/en/groups.json

@@ -185,6 +185,7 @@
   "questLeaderCannotLeaveGroup": "You cannot leave your party when you have started a quest. Abort the quest first.",
   "cannotLeaveWhileActiveQuest": "You cannot leave party during an active quest. Please leave the quest first.",
   "onlyLeaderCanRemoveMember": "Only group leader can remove a member!",
+  "cannotRemoveCurrentLeader": "You cannot remove the group leader. Assign a new a leader first.",
   "memberCannotRemoveYourself": "You cannot remove yourself!",
   "groupMemberNotFound": "User not found among group's members",
   "mustBeGroupMember": "Must be member of the group.",

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

@@ -397,7 +397,7 @@ api.getGroup = {
  * @apiUse groupIdRequired
  * @apiUse GroupNotFound
  *
- * @apiPermission GroupLeader
+ * @apiPermission GroupLeader, Admin
  */
 api.updateGroup = {
   method: 'PUT',
@@ -410,11 +410,13 @@ api.updateGroup = {
 
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
+    let optionalMembership = Boolean(user.contributor.admin);
+    let group = await Group.getGroup({user, groupId: req.params.groupId, optionalMembership});
 
-    let group = await Group.getGroup({user, groupId: req.params.groupId});
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
-    if (group.leader !== user._id) throw new NotAuthorized(res.t('messageGroupOnlyLeaderCanUpdate'));
+    if (group.leader !== user._id && group.type === 'party') throw new NotAuthorized(res.t('messageGroupOnlyLeaderCanUpdate'));
+    else if (group.leader !== user._id && !user.contributor.admin) throw new NotAuthorized(res.t('messageGroupOnlyLeaderCanUpdate'));
 
     if (req.body.leader !== user._id && group.hasNotCancelled()) throw new NotAuthorized(res.t('cannotChangeLeaderWithActiveGroupPlan'));
 
@@ -473,7 +475,7 @@ api.joinGroup = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-     // Works even if the user is not yet a member of the group
+    // Works even if the user is not yet a member of the group
     let group = await Group.getGroup({user, groupId: req.params.groupId, optionalMembership: true}); // Do not fetch chat and work even if the user is not yet a member of the group
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
@@ -761,7 +763,7 @@ function _sendMessageToRemoved (group, removedUser, message, isInGroup) {
  *
  * @apiSuccess {Object} data An empty object
  *
- * @apiPermission GroupLeader
+ * @apiPermission GroupLeader, Admin
  *
  * @apiUse groupIdRequired
  * @apiUse GroupNotFound
@@ -778,13 +780,18 @@ api.removeGroupMember = {
 
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
+    let optionalMembership = Boolean(user.contributor.admin);
+    let group = await Group.getGroup({user, groupId: req.params.groupId, optionalMembership, fields: '-chat'}); // Do not fetch chat
 
-    let group = await Group.getGroup({user, groupId: req.params.groupId, fields: '-chat'}); // Do not fetch chat
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     let uuid = req.params.memberId;
 
-    if (group.leader !== user._id) throw new NotAuthorized(res.t('onlyLeaderCanRemoveMember'));
+    if (group.leader !== user._id && group.type === 'party') throw new NotAuthorized(res.t('onlyLeaderCanRemoveMember'));
+    if (group.leader !== user._id && !user.contributor.admin) throw new NotAuthorized(res.t('onlyLeaderCanRemoveMember'));
+
+    if (group.leader === uuid && user.contributor.admin) throw new NotAuthorized(res.t('cannotRemoveCurrentLeader'));
+
     if (user._id === uuid) throw new NotAuthorized(res.t('memberCannotRemoveYourself'));
 
     let member = await User.findOne({_id: uuid}).exec();
@@ -947,12 +954,12 @@ async function _inviteByEmail (invite, group, inviter, req, res) {
   if (!invite.email) throw new BadRequest(res.t('inviteMissingEmail'));
 
   let userToContact = await User.findOne({$or: [
-    {'auth.local.email': invite.email},
-    {'auth.facebook.emails.value': invite.email},
-    {'auth.google.emails.value': invite.email},
+      {'auth.local.email': invite.email},
+      {'auth.facebook.emails.value': invite.email},
+      {'auth.google.emails.value': invite.email},
   ]})
-  .select({_id: true, 'preferences.emailNotifications': true})
-  .exec();
+    .select({_id: true, 'preferences.emailNotifications': true})
+    .exec();
 
   if (userToContact) {
     userReturnInfo = await _inviteByUUID(userToContact._id, group, inviter, req, res);

website/views/options/social/group.jade

@@ -39,7 +39,7 @@ a.pull-right.gem-wallet(ng-if='group.type!="party"', popover-trigger='mouseenter
                   span.glyphicon.glyphicon-ban-circle
                   =env.t('leave')
                 a.btn.btn-success.pull-right(ng-if='!isMemberOfGroup(User.user._id, group)', ng-click='join(group)')=env.t('join')
-              span(ng-if='group.leader._id == user.id')
+              span(ng-if='isAbleToEditGroup(group)')
                 button.btn.btn-sm.btn-primary.pull-right(ng-click='cancelEdit(group)', ng-hide='!group._editing')=env.t('cancel')
                 button.btn.btn-sm.btn-primary.pull-right(ng-click='saveEdit(group)', ng-show='group._editing')=env.t('save')
                 button.btn.btn-sm.btn-default.pull-right(ng-click='editGroup(group)', ng-hide='group._editing')=env.t('editGroup')
@@ -107,8 +107,8 @@ a.pull-right.gem-wallet(ng-if='group.type!="party"', popover-trigger='mouseenter
             table.table.table-striped(ng-show='::group.memberCount > 1 || group.type !== "party"' bindonce='group')
               tr(ng-repeat='member in group.members track by member._id')
                 td.media
-                  // allow leaders to ban members
-                  .pull-left(ng-show='group.leader._id == user.id && member._id != user._id')
+                  // allow leaders and administrators to ban members
+                  .pull-left(ng-show='(isAbleToEditGroup(group) && member._id != user._id)')
                     a.media-object(ng-click='removeMember(group, member, true)')
                       span.glyphicon.glyphicon-ban-circle(tooltip=env.t('banTip'))
                   a.media-body
@@ -130,8 +130,8 @@ a.pull-right.gem-wallet(ng-if='group.type!="party"', popover-trigger='mouseenter
             table.table.table-striped
               tr(ng-repeat='invite in group.invites')
                 td.media
-                  // allow leaders to ban members
-                  .pull-left(ng-show='group.leader._id == user.id')
+                  // allow leaders and administrators to ban members
+                  .pull-left(ng-show='isAbleToEditGroup(group)')
                     a.media-object(ng-click='removeMember(group, invite, false)')
                       span.glyphicon.glyphicon-ban-circle(tooltip=env.t('banTip'))
                   a.media-body