add tests for getting challenge members and fix a lot of bugs

common/locales/en/api-v3.json

@@ -37,6 +37,7 @@
   "onlyLeaderCanRemoveMember": "Only group leader can remove a member!",
   "memberCannotRemoveYourself": "You cannot remove yourself!",
   "groupMemberNotFound": "User not found among group's members",
+  "mustBeGroupMember": "Must be member of the group.",
   "keepOrRemoveAll": "req.query.keep must be either \"keep-all\" or \"remove-all\"",
   "keepOrRemove": "req.query.keep must be either \"keep\" or \"remove\"",
   "canOnlyInviteEmailUuid": "Can only invite using uuids or emails.",

website/src/controllers/api-v3/challenges.js

@@ -38,8 +38,9 @@ api.createChallenge = {
     let groupId = req.body.groupId;
     let prize = req.body.prize;
 
-    let group = await Group.getGroup({user, groupId, fields: '-chat'});
+    let group = await Group.getGroup({user, groupId, fields: '-chat', mustBeMember: true});
     if (!group) throw new NotFound(res.t('groupNotFound'));
+    if (!group.isMember(user)) throw new NotAuthorized(res.t('mustBeGroupMember'));
 
     if (group.leaderOnly && group.leaderOnly.challenges && group.leader !== user._id) {
       throw new NotAuthorized(res.t('onlyGroupLeaderChal'));
@@ -150,10 +151,10 @@ api.getChallenge = {
     let challengeId = req.params.challengeId;
 
     let challenge = await Challenge.findById(challengeId).exec();
+    if (!challenge) throw new NotFound(res.t('challengeNotFound'));
 
-    if (!challenge || !challenge.hasAccess(user)) {
-      throw new NotFound(res.t('challengeNotFound'));
-    }
+    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+    if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
 
     res.respond(200, challenge);
   },

website/src/controllers/api-v3/members.js

@@ -73,8 +73,10 @@ function _getMembersForItem (type) {
     let group;
 
     if (type === 'challenge-members') {
-      challenge = await Challenge.findById(challengeId).select('_id type leader').exec();
-      if (!challenge || !challenge.hasAccess(user)) throw new NotFound(res.t('groupNotFound'));
+      challenge = await Challenge.findById(challengeId).select('_id type leader groupId').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+      if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
     } else {
       group = await Group.getGroup({user, groupId, fields: '_id type'});
       if (!group) throw new NotFound(res.t('groupNotFound'));

website/src/models/challenge.js

@@ -20,7 +20,7 @@ let schema = new Schema({
     rewards: [{type: String, ref: 'Task'}],
   },
   leader: {type: String, ref: 'User', validate: [validator.isUUID, 'Invalid uuid.'], required: true},
-  groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true},
+  groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true}, // TODO no update, no set?
   timestamp: {type: Date, default: Date.now, required: true}, // TODO what is this? use timestamps from plugin? not settable?
   memberCount: {type: Number, default: 0},
   challengeCount: {type: Number, default: 0},
@@ -31,7 +31,7 @@ schema.plugin(baseModel, {
   noSet: ['_id', 'memberCount', 'challengeCount', 'tasksOrder'],
 });
 
-// Return true if user has access to the challenge
+// Returns true if user has access to the challenge (can join)
 schema.methods.hasAccess = function hasAccessToChallenge (user) {
   let userGroups = user.guilds.slice(0);
   if (user.party._id) userGroups.push(user.party._id);
@@ -39,12 +39,19 @@ schema.methods.hasAccess = function hasAccessToChallenge (user) {
   return this.leader === user._id || user.contributor.admin || userGroups.indexOf(this.groupId) !== -1;
 };
 
-// Return true if user is a member of the challenge
+// Returns true if user can view the challenge
+// Different from hasAccess because challenges of public guilds can be viewed by everyone
+schema.methods.canView = function canViewChallenge (user, group) {
+  if (group.type === 'guild' && group.privacy === 'public') return true;
+  return this.hasAccess(user);
+};
+
+// Returns true if user is a member of the challenge
 schema.methods.isMember = function isChallengeMember (user) {
   return user.challenges.indexOf(this._id) !== -1;
 };
 
-// Return true if the user can modify (close, selectWinner, ...) the challenge
+// Returns true if the user can modify (close, selectWinner, ...) the challenge
 schema.methods.canModify = function canModifyChallenge (user) {
   return user.contributor.admin || this.leader === user._id;
 };

website/src/models/group.js

@@ -150,6 +150,17 @@ schema.statics.getGroup = function getGroup (options = {}) {
   // TODO purge chat flags info? in tojson?
 };
 
+// Return true if user is a member of the group
+schema.methods.isMember = function isGroupMember (user) {
+  if (this._id === 'habitrpg') {
+    return true; // everyone is considered part of the tavern
+  } else if (this.type === 'party') {
+    return user.party._id === this._id ? true : false;
+  } else { // guilds
+    return user.guilds.indexOf(this._id) !== -1;
+  }
+};
+
 export function chatDefaults (msg, user) {
   let message = {
     id: shared.uuid(),