improve access control for challenges

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

@@ -192,7 +192,7 @@ api.getChallenge = {
     let challenge = await Challenge.findById(challengeId).exec();
     if (!challenge) throw new NotFound(res.t('challengeNotFound'));
 
-    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy', optionalMembership: true});
     if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
 
     res.respond(200, challenge);

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

@@ -76,7 +76,10 @@ function _getMembersForItem (type) {
     if (type === 'challenge-members') {
       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'});
+
+      // optionalMembership is set to true because even if you're not member of the group you may be able to access the challenge
+      // for example if you've been booted from it, are the leader or a site admin
+      group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy', optionalMembership: true});
       if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
     } else {
       group = await Group.getGroup({user, groupId, fields: '_id type'});
@@ -207,7 +210,9 @@ api.getChallengeMemberProgress = {
     let challenge = await Challenge.findById(challengeId).exec();
     if (!challenge) throw new NotFound(res.t('challengeNotFound'));
 
-    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+    // optionalMembership is set to true because even if you're not member of the group you may be able to access the challenge
+    // for example if you've been booted from it, are the leader or a site admin
+    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy', optionalMembership: true});
     if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
     if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound'));
 

website/src/models/challenge.js

@@ -30,22 +30,6 @@ schema.plugin(baseModel, {
   noSet: ['_id', 'memberCount', 'challengeCount', 'tasksOrder'],
 });
 
-// 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);
-  userGroups.push('habitrpg'); // tavern challenges
-  return this.leader === user._id || userGroups.indexOf(this.groupId) !== -1;
-};
-
-// 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 (user.contributor.admin) return true;
-  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;
@@ -56,6 +40,23 @@ schema.methods.canModify = function canModifyChallenge (user) {
   return user.contributor.admin || this.leader === user._id;
 };
 
+// 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);
+  userGroups.push('habitrpg'); // tavern challenges
+  return this.canModify(user) || userGroups.indexOf(this.groupId) !== -1;
+};
+
+// Returns true if user can view the challenge
+// Different from hasAccess because challenges of public guilds can be viewed by everyone
+// And also because you can see challenges of groups you've been removed from
+schema.methods.canView = function canViewChallenge (user, group) {
+  if (group.type === 'guild' && group.privacy === 'public') return true;
+  if (this.isMember(user)) return true;
+  return this.hasAccess(user);
+};
+
 // Takes a Task document and return a plain object of attributes that can be synced to the user
 function _syncableAttrs (task) {
   let t = task.toObject(); // lodash doesn't seem to like _.omit on Document
@@ -65,13 +66,6 @@ function _syncableAttrs (task) {
   return _.omit(t, omitAttrs);
 }
 
-schema.methods.hasAccess = function hasAccessToChallenge (user) {
-  let userGroups = user.guilds.slice(0);
-  if (user.party._id) userGroups.push(user.party._id);
-  userGroups.push('habitrpg'); // tavern challenges
-  return this.leader === user._id || userGroups.indexOf(this.groupId) !== -1;
-};
-
 // Sync challenge to user, including tasks and tags.
 // Used when user joins the challenge or to force sync.
 schema.methods.syncToUser = async function syncChallengeToUser (user) {