From ac90a40be589045e3f72dbdf0b68ede99989ef1f Mon Sep 17 00:00:00 2001 From: Brian Fenton Date: Sun, 27 May 2018 09:41:56 -0500 Subject: [PATCH] Api quest restrictions - no purchase/start without fulfilling eligibility requirements (#10387) * removing duplicate translation key * fixing typos * extracting quest prerequisite check. adding check for previous quest completion, if required * fixing (undoing) static change, adding tests * more typos * correcting test failures * honoring quest prerequisites in quest invite API call. updating format of il8n string replacement arg * no longer using apiError, use translate method instead (msg key was not defined) * adding @apiError to docblock as requested in issue * removing checks on quest invite method. small window of opportunity/low risk --- .../user/buy/POST-user_buy_quest.test.js | 28 +++++++++++++++++++ test/common/ops/buy/buyQuest.js | 15 ++++++++++ .../api-integration/v3/object-generators.js | 2 +- website/common/locales/en/quests.json | 1 - website/common/script/ops/buy/buy.js | 2 +- website/common/script/ops/buy/buyQuest.js | 17 ++++++++--- website/server/controllers/api-v3/user.js | 6 ++-- website/server/libs/password.js | 2 +- website/server/server.js | 2 +- 9 files changed, 64 insertions(+), 11 deletions(-) diff --git a/test/api/v3/integration/user/buy/POST-user_buy_quest.test.js b/test/api/v3/integration/user/buy/POST-user_buy_quest.test.js index 0bc9b40d3d..8f1e6bf896 100644 --- a/test/api/v3/integration/user/buy/POST-user_buy_quest.test.js +++ b/test/api/v3/integration/user/buy/POST-user_buy_quest.test.js @@ -38,4 +38,32 @@ describe('POST /user/buy-quest/:key', () => { itemText: item.text(), })); }); + + it('returns an error if quest prerequisites are not met', async () => { + let key = 'dilatoryDistress2'; + + await expect(user.post(`/user/buy-quest/${key}`)) + .to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('mustComplete', {quest: 'dilatoryDistress1'}), + }); + }); + + it('allows purchase of a quest if prerequisites are met', async () => { + const prerequisite = 'dilatoryDistress1'; + const key = 'dilatoryDistress2'; + const item = content.quests[key]; + + const achievementName = `achievements.quests.${prerequisite}`; + + await user.update({[achievementName]: true, 'stats.gp': 9999}); + let res = await user.post(`/user/buy-quest/${key}`); + await user.sync(); + + expect(res.data).to.eql(user.items.quests); + expect(res.message).to.equal(t('messageBought', { + itemText: item.text(), + })); + }); }); diff --git a/test/common/ops/buy/buyQuest.js b/test/common/ops/buy/buyQuest.js index 4994e38af2..4554b9832a 100644 --- a/test/common/ops/buy/buyQuest.js +++ b/test/common/ops/buy/buyQuest.js @@ -156,4 +156,19 @@ describe('shared.ops.buyQuest', () => { done(); } }); + + it('does not buy a quest without completing previous quests', (done) => { + try { + buyQuest(user, { + params: { + key: 'dilatoryDistress3', + }, + }); + } catch (err) { + expect(err).to.be.an.instanceof(NotAuthorized); + expect(err.message).to.equal(i18n.t('mustComplete', {quest: 'dilatoryDistress2'})); + expect(user.items.quests).to.eql({}); + done(); + } + }); }); diff --git a/test/helpers/api-integration/v3/object-generators.js b/test/helpers/api-integration/v3/object-generators.js index dd8c4ef3fe..4e177ace00 100644 --- a/test/helpers/api-integration/v3/object-generators.js +++ b/test/helpers/api-integration/v3/object-generators.js @@ -10,7 +10,7 @@ import * as Tasks from '../../../../website/server/models/task'; // If you need the user to have specific requirements, // such as a balance > 0, just pass in the adjustment // to the update object. If you want to adjust a nested -// paramter, such as the number of wolf eggs the user has, +// parameter, such as the number of wolf eggs the user has, // , you can do so by passing in the full path as a string: // { 'items.eggs.Wolf': 10 } export async function generateUser (update = {}) { diff --git a/website/common/locales/en/quests.json b/website/common/locales/en/quests.json index 5c09433384..a67a4b97b4 100644 --- a/website/common/locales/en/quests.json +++ b/website/common/locales/en/quests.json @@ -110,7 +110,6 @@ "questAlreadyRejected": "You already rejected the quest invitation.", "cantCancelActiveQuest": "You can not cancel an active quest, use the abort functionality.", "onlyLeaderCancelQuest": "Only the group or quest leader can cancel the quest.", - "questInvitationDoesNotExist": "No quest invitation has been sent out yet.", "questNotPending": "There is no quest to start.", "questOrGroupLeaderOnlyStartQuest": "Only the quest leader or group leader can force start the quest", "createAccountReward": "Create Account", diff --git a/website/common/script/ops/buy/buy.js b/website/common/script/ops/buy/buy.js index 9bcee0706e..413ca79e7d 100644 --- a/website/common/script/ops/buy/buy.js +++ b/website/common/script/ops/buy/buy.js @@ -23,7 +23,7 @@ module.exports = function buy (user, req = {}, analytics) { if (!key) throw new BadRequest(errorMessage('missingKeyParam')); // @TODO: Slowly remove the need for key and use type instead - // This should evenutally be the 'factory' function with vendor classes + // This should eventually be the 'factory' function with vendor classes let type = get(req, 'type'); if (!type) type = get(req, 'params.type'); if (!type) type = key; diff --git a/website/common/script/ops/buy/buyQuest.js b/website/common/script/ops/buy/buyQuest.js index 1fd60949fd..acc0b88940 100644 --- a/website/common/script/ops/buy/buyQuest.js +++ b/website/common/script/ops/buy/buyQuest.js @@ -37,10 +37,6 @@ export class BuyQuestWithGoldOperation extends AbstractGoldItemOperation { let key = this.key = get(req, 'params.key'); if (!key) throw new BadRequest(errorMessage('missingKeyParam')); - if (key === 'lostMasterclasser1' && !this.userAbleToStartMasterClasser(user)) { - throw new NotAuthorized(this.i18n('questUnlockLostMasterclasser')); - } - let item = content.quests[key]; if (!item) throw new NotFound(errorMessage('questNotFound', {key})); @@ -49,9 +45,22 @@ export class BuyQuestWithGoldOperation extends AbstractGoldItemOperation { throw new NotAuthorized(this.i18n('questNotGoldPurchasable', {key})); } + this.checkPrerequisites(user, key); + this.canUserPurchase(user, item); } + checkPrerequisites (user, questKey) { + const item = content.quests[questKey]; + if (questKey === 'lostMasterclasser1' && !this.userAbleToStartMasterClasser(user)) { + throw new NotAuthorized(this.i18n('questUnlockLostMasterclasser')); + } + + if (item && item.previous && !user.achievements.quests[item.previous]) { + throw new NotAuthorized(this.i18n('mustComplete', {quest: item.previous})); + } + } + executeChanges (user, item, req) { user.items.quests[item.key] = user.items.quests[item.key] || 0; user.items.quests[item.key] += this.quantity; diff --git a/website/server/controllers/api-v3/user.js b/website/server/controllers/api-v3/user.js index e987721c11..bf8dd9f75e 100644 --- a/website/server/controllers/api-v3/user.js +++ b/website/server/controllers/api-v3/user.js @@ -830,7 +830,9 @@ api.buyMysterySet = { * * @apiErrorExample {json} Quest chosen does not exist * {"success":false,"error":"NotFound","message":"Quest \"dilatoryDistress99\" not found."} - * @apiErrorExample {json} NotAuthorized Not enough gold + * @apiErrorExample {json} You must first complete this quest's prerequisites + * {"success":false,"error":"NotAuthorized","message":"You must first complete dilatoryDistress2."} + * @apiErrorExample {json} NotAuthorized Not enough gold * {"success":false,"error":"NotAuthorized","message":"Not Enough Gold"} * */ @@ -912,7 +914,7 @@ api.buySpecialSpell = { * } * * @apiError {NotAuthorized} messageAlreadyPet Already have the specific pet combination - * @apiError {NotFound} messageMissingEggPotion One or both of the ingrediants are missing. + * @apiError {NotFound} messageMissingEggPotion One or both of the ingredients are missing. * @apiError {NotFound} messageInvalidEggPotionCombo Cannot use that combination of egg and potion. * * @apiErrorExample {json} Already have that pet. diff --git a/website/server/libs/password.js b/website/server/libs/password.js index 12d162f6ef..4dfcfc4162 100644 --- a/website/server/libs/password.js +++ b/website/server/libs/password.js @@ -35,7 +35,7 @@ export function sha1MakeSalt (len = 10) { } // Compare the password for an user -// Works with bcrypt and sha1 indipendently +// Works with bcrypt and sha1 independently // An async function is used so that a promise is always returned // even for comparing sha1 hashed passwords that use a sync method export async function compare (user, passwordToCheck) { diff --git a/website/server/server.js b/website/server/server.js index bf22c445e4..f331929038 100644 --- a/website/server/server.js +++ b/website/server/server.js @@ -20,7 +20,7 @@ const app = express(); app.set('port', nconf.get('PORT')); // Setup translations -// Must come before attach middlwares so Mongoose validations can use translations +// Must come before attach middlewares so Mongoose validations can use translations import './libs/i18n'; import attachMiddlewares from './middlewares/index';