Remove join retired public guild loophole (#15626)

* fix(guilds): remove join retired public guild loophole * fix(groups): adjust test expectations and move some business logic back out of the controller * test(challenges): add cases related to public Guilds, Tavern and otherwise * fix(tests): more erroneously public test guilds, lint * fix(tests): still more setup tweaks * fix(lint): whitespace * fix(tests): couple more adjustments * fix(test): last challenge issue??
2026-03-13 08:41:14 +08:00 · 2026-03-12 19:15:00 -05:00
parent 7a6d64f158
commit c3c2607bca
11 changed files with 73 additions and 24 deletions
--- a/test/api/unit/libs/payments/amazon/cancel.test.js
+++ b/test/api/unit/libs/payments/amazon/cancel.test.js
@@ -66,13 +66,15 @@ describe('Amazon Payments - Cancel Subscription', () => {
    group = generateGroup({
      name: 'test group',
      type: 'guild',
-      privacy: 'public',
+      privacy: 'private',
      leader: user._id,
    });
    group.purchased.plan.customerId = 'customer-id';
    group.purchased.plan.planId = subKey;
    group.purchased.plan.lastBillingDate = new Date();
    await group.save();
+    user.guilds.push(group._id);
+    await user.save();

    subscriptionBlock = common.content.subscriptionBlocks[subKey];
    subscriptionLength = subscriptionBlock.months * 30;
--- a/test/api/unit/libs/payments/amazon/subscribe.test.js
+++ b/test/api/unit/libs/payments/amazon/subscribe.test.js
@@ -30,12 +30,14 @@ describe('Amazon Payments - Subscribe', () => {
    group = generateGroup({
      name: 'test group',
      type: 'guild',
-      privacy: 'public',
+      privacy: 'private',
      leader: user._id,
    });
    group.purchased.plan.customerId = 'customer-id';
    group.purchased.plan.planId = subKey;
    await group.save();
+    user.guilds.push(group._id);
+    await user.save();

    amount = common.content.subscriptionBlocks[subKey].price;
    billingAgreementId = 'billingAgreementId';
@@ -246,11 +248,6 @@ describe('Amazon Payments - Subscribe', () => {
    user.guilds.push(groupId);
    await user.save();

-    // Add existing users
-    user = new User();
-    user.guilds.push(groupId);
-    await user.save();
-
    // Set expected amount
    sub.key = 'group_monthly';
    sub.price = 9;
--- a/test/api/unit/libs/payments/group-plans/group-payments-create.test.js
+++ b/test/api/unit/libs/payments/group-plans/group-payments-create.test.js
@@ -128,11 +128,12 @@ describe('Purchasing a group plan for group', () => {
    expect(publicGroup.purchased.plan.planId).to.not.exist;
    data.groupId = publicGroup._id;

+    // Public Guilds are no longer even findable
    await expect(api.createSubscription(data))
      .to.eventually.be.rejected.and.to.eql({
-        httpCode: 401,
-        name: 'NotAuthorized',
-        message: i18n.t('onlyPrivateGuildsCanUpgrade'),
+        httpCode: 404,
+        name: 'NotFound',
+        message: i18n.t('groupNotFound'),
      });

    const updatedGroup = await Group.findById(publicGroup._id).exec();
--- a/test/api/unit/libs/payments/paypal/subscribe-cancel.test.js
+++ b/test/api/unit/libs/payments/paypal/subscribe-cancel.test.js
@@ -30,13 +30,15 @@ describe('paypal - subscribeCancel', () => {
    group = generateGroup({
      name: 'test group',
      type: 'guild',
-      privacy: 'public',
+      privacy: 'private',
      leader: user._id,
    });
    group.purchased.plan.customerId = groupCustomerId;
    group.purchased.plan.planId = subKey;
    group.purchased.plan.lastBillingDate = new Date();
    await group.save();
+    user.guilds.push(group._id);
+    await user.save();

    nextBillingDate = new Date();

--- a/test/api/unit/libs/payments/stripe/checkout.test.js
+++ b/test/api/unit/libs/payments/stripe/checkout.test.js
@@ -236,7 +236,7 @@ describe('Stripe - Checkout', () => {
      const group = generateGroup({
        name: 'test group',
        type: 'guild',
-        privacy: 'public',
+        privacy: 'private',
        leader: user._id,
      });
      const groupId = group._id;
@@ -376,11 +376,13 @@ describe('Stripe - Checkout', () => {
        group = generateGroup({
          name: 'test group',
          type: 'guild',
-          privacy: 'public',
+          privacy: 'private',
          leader: user._id,
        });
        groupId = group._id;
        await group.save();
+        user.guilds.push(group._id);
+        await user.save();
      });

      it('throws if user is not allowed to change group plan', async () => {
--- a/test/api/unit/libs/payments/stripe/subscriptions.test.js
+++ b/test/api/unit/libs/payments/stripe/subscriptions.test.js
@@ -136,7 +136,7 @@ describe('Stripe - Subscriptions', () => {
      group = generateGroup({
        name: 'test group',
        type: 'guild',
-        privacy: 'public',
+        privacy: 'private',
        leader: user._id,
      });
      groupId = group._id;
@@ -315,12 +315,14 @@ describe('Stripe - Subscriptions', () => {
      group = generateGroup({
        name: 'test group',
        type: 'guild',
-        privacy: 'public',
+        privacy: 'private',
        leader: user._id,
      });
      group.purchased.plan.customerId = 'customer-id';
      group.purchased.plan.planId = subKey;
      await group.save();
+      user.guilds.push(group._id);
+      await user.save();

      groupId = group._id;
    });
--- a/test/api/v3/integration/challenges/POST-challenges_challengeId_join.test.js
+++ b/test/api/v3/integration/challenges/POST-challenges_challengeId_join.test.js
@@ -5,6 +5,8 @@ import {
  createAndPopulateGroup,
  translate as t,
 } from '../../../../helpers/api-integration/v3';
+import { model as Group } from '../../../../../website/server/models/group';
+import { TAVERN_ID } from '../../../../../website/common/script/constants';

 describe('POST /challenges/:challengeId/join', () => {
  it('returns error when challengeId is not a valid UUID', async () => {
@@ -27,6 +29,37 @@ describe('POST /challenges/:challengeId/join', () => {
    });
  });

+  context('public Guild', () => {
+    let group;
+    let groupLeader;
+    let members;
+    let challenge;
+    before(async () => {
+      ({ group, groupLeader, members } = await createAndPopulateGroup({
+        groupDetails: {
+          name: 'test group',
+          type: 'guild',
+          privacy: 'private',
+        },
+        members: 1,
+        upgradeToGroupPlan: true,
+      }));
+      challenge = await generateChallenge(groupLeader, group);
+      // Creation API is shut down, we need to simulate an extant public group
+      await Group.updateOne({ _id: group._id }, { $set: { privacy: 'public' }, $unset: { 'purchased.plan': 1 } }).exec();
+    });
+
+    it('returns error when challengeId is in an old public Guild', async () => {
+      const authorizedUser = members[0]; // eslint-disable-line prefer-destructuring
+
+      await expect(authorizedUser.post(`/challenges/${challenge._id}/join`)).to.eventually.be.rejected.and.eql({
+        code: 404,
+        error: 'NotFound',
+        message: t('challengeNotFound'),
+      });
+    });
+  });
+
  context('Joining a valid challenge', () => {
    let groupLeader;
    let group;
@@ -66,6 +99,15 @@ describe('POST /challenges/:challengeId/join', () => {
      expect(res.name).to.equal(challenge.name);
    });

+    it('succeeds when it\'s a Tavern challenge, even if the user isn\'t a "member" of Tavern', async () => {
+      const tavern = await groupLeader.get(`/groups/${TAVERN_ID}`);
+      const tavernChallenge = await generateChallenge(groupLeader, tavern, { prize: 1 });
+      const generalUser = await generateUser();
+
+      const res = await generalUser.post(`/challenges/${tavernChallenge._id}/join`);
+      expect(res.name).to.equal(tavernChallenge.name);
+    });
+
    it('returns challenge data', async () => {
      const res = await authorizedUser.post(`/challenges/${challenge._id}/join`);

--- a/test/api/v3/integration/chat/GET-chat.test.js
+++ b/test/api/v3/integration/chat/GET-chat.test.js
@@ -62,9 +62,9 @@ describe('GET /groups/:groupId/chat', () => {

    it('returns error if user attempts to fetch a sunset Guild', async () => {
      await expect(user.get(`/groups/${group._id}/chat`)).to.eventually.be.rejected.and.eql({
-        code: 400,
-        error: 'BadRequest',
-        message: t('featureRetired'),
+        code: 404,
+        error: 'NotFound',
+        message: t('groupNotFound'),
      });
    });
  });
--- a/test/api/v3/integration/chat/POST-chat.like.test.js
+++ b/test/api/v3/integration/chat/POST-chat.like.test.js
@@ -121,9 +121,9 @@ describe('POST /chat/:chatId/like', () => {

    await expect(user.post(`/groups/${groupWithChat._id}/chat/${message.message.id}/like`))
      .to.eventually.be.rejected.and.eql({
-        code: 400,
-        error: 'BadRequest',
-        message: t('featureRetired'),
+        code: 404,
+        error: 'NotFound',
+        message: t('groupNotFound'),
      });
  });
 });
--- a/website/server/libs/payments/amazon.js
+++ b/website/server/libs/payments/amazon.js
@@ -316,6 +316,7 @@ api.subscribe = async function subscribe (options) {
    const group = await Group.getGroup({
      user, groupId, populateLeader: false, groupFields,
    });
+    if (!group) throw new NotFound(i18n.t('groupNotFound'));
    const membersCount = await group.getMemberCount();
    amount = sub.price + (membersCount - leaderCount) * priceOfSingleMember;
  }
--- a/website/server/models/group.js
+++ b/website/server/models/group.js
@@ -261,11 +261,11 @@ schema.statics.getGroup = async function getGroup (options = {}) {
  } else if (isTavern) {
    query = { _id: TAVERN_ID };
  } else if (optionalMembership === true) {
-    query = { _id: groupId };
+    query = { privacy: 'private', _id: groupId };
  } else if (isUserGuild) {
-    query = { type: 'guild', _id: groupId };
+    query = { type: 'guild', privacy: 'private', _id: groupId };
  } else {
-    query = { type: 'guild', privacy: 'public', _id: groupId };
+    return null;
  }

  const mQuery = this.findOne(query);