diff --git a/test/api/v3/integration/hall/GET-hall_heroes.test.js b/test/api/v3/integration/hall/GET-hall_heroes.test.js index a8fb9f70ec..412c343d04 100644 --- a/test/api/v3/integration/hall/GET-hall_heroes.test.js +++ b/test/api/v3/integration/hall/GET-hall_heroes.test.js @@ -10,6 +10,9 @@ describe('GET /hall/heroes', () => { const nonHero = await generateUser(); const hero1 = await generateUser({ contributor: { level: 1 }, + secret: { + text: 'Super-Hero', + }, }); const hero2 = await generateUser({ contributor: { level: 3 }, @@ -21,6 +24,8 @@ describe('GET /hall/heroes', () => { expect(heroes[1]._id).to.equal(hero1._id); expect(heroes[0]).to.have.all.keys(['_id', 'contributor', 'backer', 'profile']); + + // should not contain the secret expect(heroes[1]).to.have.all.keys(['_id', 'contributor', 'backer', 'profile']); expect(heroes[0].profile).to.have.all.keys(['name']); @@ -28,5 +33,6 @@ describe('GET /hall/heroes', () => { expect(heroes[0].profile.name).to.equal(hero2.profile.name); expect(heroes[1].profile.name).to.equal(hero1.profile.name); + expect(heroes[1].secret).to.equal(undefined); }); }); diff --git a/test/api/v3/integration/hall/GET-hall_heroes_heroId.test.js b/test/api/v3/integration/hall/GET-hall_heroes_heroId.test.js index c3e78a8f79..bcafdcfd80 100644 --- a/test/api/v3/integration/hall/GET-hall_heroes_heroId.test.js +++ b/test/api/v3/integration/hall/GET-hall_heroes_heroId.test.js @@ -43,15 +43,19 @@ describe('GET /heroes/:heroId', () => { it('returns only necessary hero data given user id', async () => { const hero = await generateUser({ contributor: { tier: 23 }, + secret: { + text: 'Super Hero', + }, }); const heroRes = await user.get(`/hall/heroes/${hero._id}`); expect(heroRes).to.have.all.keys([ // works as: object has all and only these keys '_id', 'id', 'balance', 'profile', 'purchased', - 'contributor', 'auth', 'items', + 'contributor', 'auth', 'items', 'secret', ]); expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); expect(heroRes.profile).to.have.all.keys(['name']); + expect(heroRes.secret.text).to.be.eq('Super Hero'); }); it('returns only necessary hero data given username', async () => { @@ -62,7 +66,7 @@ describe('GET /heroes/:heroId', () => { expect(heroRes).to.have.all.keys([ // works as: object has all and only these keys '_id', 'id', 'balance', 'profile', 'purchased', - 'contributor', 'auth', 'items', + 'contributor', 'auth', 'items', 'secret', ]); expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); expect(heroRes.profile).to.have.all.keys(['name']); diff --git a/test/api/v3/integration/hall/PUT-hall_heores_heroId.test.js b/test/api/v3/integration/hall/PUT-hall_heores_heroId.test.js index db6eaf82ae..5e9d042ea5 100644 --- a/test/api/v3/integration/hall/PUT-hall_heores_heroId.test.js +++ b/test/api/v3/integration/hall/PUT-hall_heores_heroId.test.js @@ -7,6 +7,12 @@ import { describe('PUT /heroes/:heroId', () => { let user; + const heroFields = [ + '_id', 'balance', 'profile', 'purchased', + 'contributor', 'auth', 'items', 'flags', + 'secret', + ]; + before(async () => { user = await generateUser({ contributor: { admin: true }, @@ -51,10 +57,8 @@ describe('PUT /heroes/:heroId', () => { }); // test response - expect(heroRes).to.have.all.keys([ // works as: object has all and only these keys - '_id', 'balance', 'profile', 'purchased', - 'contributor', 'auth', 'items', 'flags', - ]); + // works as: object has all and only these keys + expect(heroRes).to.have.all.keys(heroFields); expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); expect(heroRes.profile).to.have.all.keys(['name']); @@ -130,10 +134,8 @@ describe('PUT /heroes/:heroId', () => { }); // test response - expect(heroRes).to.have.all.keys([ // works as: object has all and only these keys - '_id', 'balance', 'profile', 'purchased', - 'contributor', 'auth', 'items', 'flags', - ]); + // works as: object has all and only these keys + expect(heroRes).to.have.all.keys(heroFields); expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); expect(heroRes.profile).to.have.all.keys(['name']); @@ -157,10 +159,8 @@ describe('PUT /heroes/:heroId', () => { }); // test response - expect(heroRes).to.have.all.keys([ // works as: object has all and only these keys - '_id', 'balance', 'profile', 'purchased', - 'contributor', 'auth', 'items', 'flags', - ]); + // works as: object has all and only these keys + expect(heroRes).to.have.all.keys(heroFields); expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); expect(heroRes.profile).to.have.all.keys(['name']); @@ -173,6 +173,40 @@ describe('PUT /heroes/:heroId', () => { expect(hero.contributor.text).to.equal('Astronaut'); }); + it('updates contributor secret', async () => { + const secretText = 'my super hero'; + + const hero = await generateUser({ + contributor: { level: 5 }, + secret: { + text: 'supr hro typo', + }, + }); + const heroRes = await user.put(`/hall/heroes/${hero._id}`, { + contributor: { text: 'Astronaut' }, + secret: { + text: secretText, + }, + }); + + // test response + // works as: object has all and only these keys + expect(heroRes).to.have.all.keys(heroFields); + expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); + expect(heroRes.profile).to.have.all.keys(['name']); + + // test response values + expect(heroRes.contributor.level).to.equal(5); // doesn't modify previous values + expect(heroRes.contributor.text).to.equal('Astronaut'); + expect(heroRes.secret.text).to.equal(secretText); + + // test hero values + await hero.sync(); + expect(hero.contributor.level).to.equal(5); // doesn't modify previous values + expect(hero.contributor.text).to.equal('Astronaut'); + expect(hero.secret.text).to.equal(secretText); + }); + it('updates items', async () => { const hero = await generateUser(); const heroRes = await user.put(`/hall/heroes/${hero._id}`, { @@ -181,10 +215,8 @@ describe('PUT /heroes/:heroId', () => { }); // test response - expect(heroRes).to.have.all.keys([ // works as: object has all and only these keys - '_id', 'balance', 'profile', 'purchased', - 'contributor', 'auth', 'items', 'flags', - ]); + // works as: object has all and only these keys + expect(heroRes).to.have.all.keys(heroFields); expect(heroRes.auth.local).not.to.have.keys(['salt', 'hashed_password']); expect(heroRes.profile).to.have.all.keys(['name']); diff --git a/test/api/v3/integration/members/GET-members_id.test.js b/test/api/v3/integration/members/GET-members_id.test.js index 9b58dd61c1..597ff082dc 100644 --- a/test/api/v3/integration/members/GET-members_id.test.js +++ b/test/api/v3/integration/members/GET-members_id.test.js @@ -29,6 +29,9 @@ describe('GET /members/:memberId', () => { costume: false, background: 'volcano', }, + secret: { + text: 'Clark Kent', + }, }); const memberRes = await user.get(`/members/${member._id}`); expect(memberRes).to.have.all.keys([ // works as: object has all and only these keys @@ -46,6 +49,29 @@ describe('GET /members/:memberId', () => { expect(memberRes.stats.toNextLevel).to.equal(common.tnl(memberRes.stats.lvl)); expect(memberRes.inbox.optOut).to.exist; expect(memberRes.inbox.messages).to.not.exist; + expect(memberRes.secret).to.not.exist; + }); + + it('does not return secret for the own account', async () => { + // make sure user has all the fields that can be returned by the getMember call + const member = await generateUser({ + contributor: { level: 1 }, + backer: { tier: 3 }, + preferences: { + costume: false, + background: 'volcano', + }, + secret: { + text: 'Clark Kent', + }, + }); + const memberRes = await member.get(`/members/${member._id}`); + expect(memberRes).to.have.keys([ // works as: object has all and only these keys + '_id', 'id', 'preferences', 'profile', 'stats', 'achievements', 'party', + 'backer', 'contributor', 'auth', 'items', 'inbox', 'loginIncentives', 'flags', + ]); + + expect(memberRes.secret).to.not.exist; }); it('handles non-existing members', async () => { diff --git a/test/api/v3/integration/user/GET-user.test.js b/test/api/v3/integration/user/GET-user.test.js index 437cc70cb3..8e49aed515 100644 --- a/test/api/v3/integration/user/GET-user.test.js +++ b/test/api/v3/integration/user/GET-user.test.js @@ -26,6 +26,7 @@ describe('GET /user', () => { expect(returnedUser.auth.local.passwordHashMethod).to.not.exist; expect(returnedUser.auth.local.salt).to.not.exist; expect(returnedUser.apiToken).to.not.exist; + expect(returnedUser.secret).to.not.exist; }); it('returns only user properties requested', async () => { @@ -38,4 +39,11 @@ describe('GET /user', () => { expect(returnedUser.notifications).to.exist; expect(returnedUser.stats).to.not.exist; }); + + it('does not return requested private properties', async () => { + const returnedUser = await user.get('/user?userFields=apiToken,secret.text'); + + expect(returnedUser.apiToken).to.not.exist; + expect(returnedUser.secret).to.not.exist; + }); }); diff --git a/test/api/v3/integration/user/GET-user_anonymized.test.js b/test/api/v3/integration/user/GET-user_anonymized.test.js index b9c8207de5..c7d1d91b21 100644 --- a/test/api/v3/integration/user/GET-user_anonymized.test.js +++ b/test/api/v3/integration/user/GET-user_anonymized.test.js @@ -12,7 +12,11 @@ describe('GET /user/anonymized', () => { const endpoint = '/user/anonymized'; before(async () => { - user = await generateUser(); + user = await generateUser({ + secret: { + text: 'Clark Kent', + }, + }); await user.update({ newMessages: ['some', 'new', 'messages'], 'profile.name': 'profile', @@ -100,5 +104,7 @@ describe('GET /user/anonymized', () => { }); } }); + + expect(returnedUser.secret).to.not.exist; }); }); diff --git a/test/api/v3/integration/user/POST-user_reset.test.js b/test/api/v3/integration/user/POST-user_reset.test.js index 389b840d92..d25f360df0 100644 --- a/test/api/v3/integration/user/POST-user_reset.test.js +++ b/test/api/v3/integration/user/POST-user_reset.test.js @@ -117,4 +117,24 @@ describe('POST /user/reset', () => { expect(userChallengeTask).to.exist; expect(syncedGroupTask).to.exist; }); + + it('does not delete secret', async () => { + const admin = await generateUser({ + contributor: { admin: true }, + }); + + const hero = await generateUser({ + contributor: { level: 1 }, + secret: { + text: 'Super-Hero', + }, + }); + + await hero.post('/user/reset'); + + const heroRes = await admin.get(`/hall/heroes/${hero.auth.local.username}`); + + expect(heroRes.secret).to.exist; + expect(heroRes.secret.text).to.be.eq('Super-Hero'); + }); }); diff --git a/test/api/v3/integration/user/PUT-user.test.js b/test/api/v3/integration/user/PUT-user.test.js index e3929cbb80..60c47a7748 100644 --- a/test/api/v3/integration/user/PUT-user.test.js +++ b/test/api/v3/integration/user/PUT-user.test.js @@ -107,6 +107,7 @@ describe('PUT /user', () => { 'customization gem purchases': { 'purchased.background.tavern': true, 'purchased.skin.bear': true }, notifications: [{ type: 123 }], webhooks: { webhooks: [{ url: 'https://foobar.com' }] }, + secret: { secret: { text: 'Some new text' } }, }; each(protectedOperations, (data, testName) => { @@ -129,6 +130,7 @@ describe('PUT /user', () => { webhooks: { 'preferences.webhooks': [1, 2, 3] }, sleep: { 'preferences.sleep': true }, 'disable classes': { 'preferences.disableClasses': true }, + secret: { secret: { text: 'Some new text' } }, }; each(protectedOperations, (data, testName) => { diff --git a/test/api/v4/user/GET-user.test.js b/test/api/v4/user/GET-user.test.js index fd05479560..f17c19e797 100644 --- a/test/api/v4/user/GET-user.test.js +++ b/test/api/v4/user/GET-user.test.js @@ -26,6 +26,7 @@ describe('GET /user', () => { expect(returnedUser.auth.local.passwordHashMethod).to.not.exist; expect(returnedUser.auth.local.salt).to.not.exist; expect(returnedUser.apiToken).to.not.exist; + expect(returnedUser.secret).to.not.exist; }); it('returns only user properties requested', async () => { @@ -39,6 +40,13 @@ describe('GET /user', () => { expect(returnedUser.stats).to.not.exist; }); + it('does not return requested private properties', async () => { + const returnedUser = await user.get('/user?userFields=apiToken,secret.text'); + + expect(returnedUser.apiToken).to.not.exist; + expect(returnedUser.secret).to.not.exist; + }); + it('does not return new inbox messages', async () => { const otherUser = await generateUser(); diff --git a/test/api/v4/user/POST-user_reset.test.js b/test/api/v4/user/POST-user_reset.test.js index a664ba31ea..bd6e568c7d 100644 --- a/test/api/v4/user/POST-user_reset.test.js +++ b/test/api/v4/user/POST-user_reset.test.js @@ -117,4 +117,25 @@ describe('POST /user/reset', () => { expect(userChallengeTask).to.exist; expect(syncedGroupTask).to.exist; }); + + it('does not delete secret', async () => { + const admin = await generateUser({ + contributor: { admin: true }, + }); + + const hero = await generateUser({ + contributor: { level: 1 }, + secret: { + text: 'Super-Hero', + }, + }); + + + await hero.post('/user/reset'); + + const heroRes = await admin.get(`/hall/heroes/${hero.auth.local.username}`); + + expect(heroRes.secret).to.exist; + expect(heroRes.secret.text).to.be.eq('Super-Hero'); + }); }); diff --git a/test/api/v4/user/PUT-user.test.js b/test/api/v4/user/PUT-user.test.js index b340f66ffd..d43bf7d6af 100644 --- a/test/api/v4/user/PUT-user.test.js +++ b/test/api/v4/user/PUT-user.test.js @@ -91,6 +91,7 @@ describe('PUT /user', () => { 'customization gem purchases': { 'purchased.background.tavern': true, 'purchased.skin.bear': true }, notifications: [{ type: 123 }], webhooks: { webhooks: [{ url: 'https://foobar.com' }] }, + secret: { secret: { text: 'Some new text' } }, }; each(protectedOperations, (data, testName) => { @@ -113,6 +114,7 @@ describe('PUT /user', () => { webhooks: { 'preferences.webhooks': [1, 2, 3] }, sleep: { 'preferences.sleep': true }, 'disable classes': { 'preferences.disableClasses': true }, + secret: { secret: { text: 'Some new text' } }, }; each(protectedOperations, (data, testName) => { diff --git a/website/server/controllers/api-v3/hall.js b/website/server/controllers/api-v3/hall.js index 81b6c97999..91622f51a3 100644 --- a/website/server/controllers/api-v3/hall.js +++ b/website/server/controllers/api-v3/hall.js @@ -145,7 +145,7 @@ api.getHeroes = { // Note, while the following routes are called getHero / updateHero // they can be used by admins to get/update any user -const heroAdminFields = 'contributor balance profile.name purchased items auth flags.chatRevoked flags.chatShadowMuted'; +const heroAdminFields = 'contributor balance profile.name purchased items auth flags.chatRevoked flags.chatShadowMuted secret'; /** * @api {get} /api/v3/hall/heroes/:heroId Get any user ("hero") given the UUID or Username @@ -189,6 +189,8 @@ api.getHero = { if (!hero) throw new NotFound(res.t('userWithIDNotFound', { userId: heroId })); const heroRes = hero.toJSON({ minimize: true }); + heroRes.secret = hero.getSecretData(); + // supply to the possible absence of hero.contributor // if we didn't pass minimize: true it would have returned all fields as empty if (!heroRes.contributor) heroRes.contributor = {}; @@ -303,8 +305,15 @@ api.updateHero = { hero.flags.chatShadowMuted = updateData.flags.chatShadowMuted; } + if (updateData.secret) { + if (typeof updateData.secret.text !== 'undefined') { + hero.secret.text = updateData.secret.text; + } + } + const savedHero = await hero.save(); const heroJSON = savedHero.toJSON(); + heroJSON.secret = savedHero.getSecretData(); const responseHero = { _id: heroJSON._id }; // only respond with important fields heroAdminFields.split(' ').forEach(field => { _.set(responseHero, field, _.get(heroJSON, field)); diff --git a/website/server/controllers/api-v3/user.js b/website/server/controllers/api-v3/user.js index d46b72c61c..34304e5034 100644 --- a/website/server/controllers/api-v3/user.js +++ b/website/server/controllers/api-v3/user.js @@ -381,6 +381,7 @@ api.getUserAnonymized = { delete user.webhooks; delete user.achievements.challenges; delete user.notifications; + delete user.secret; _.forEach(user.inbox.messages, msg => { msg.text = 'inbox message text'; diff --git a/website/server/libs/user/index.js b/website/server/libs/user/index.js index bf1a185a2d..a46a235efc 100644 --- a/website/server/libs/user/index.js +++ b/website/server/libs/user/index.js @@ -8,7 +8,6 @@ import { import { model as User, schema as UserSchema } from '../../models/user'; import { nameContainsSlur } from './validation'; - export async function get (req, res, { isV3 = false }) { const { user } = res.locals; let userToJSON; diff --git a/website/server/models/user/hooks.js b/website/server/models/user/hooks.js index 63aa42ed57..98c580d8da 100644 --- a/website/server/models/user/hooks.js +++ b/website/server/models/user/hooks.js @@ -21,7 +21,7 @@ schema.plugin(baseModel, { // noSet is not used as updating uses a whitelist and creating only accepts // specific params (password, email, username, ...) noSet: [], - private: ['auth.local.hashed_password', 'auth.local.passwordHashMethod', 'auth.local.salt', '_cronSignature', '_ABtests'], + private: ['auth.local.hashed_password', 'auth.local.passwordHashMethod', 'auth.local.salt', '_cronSignature', '_ABtests', 'secret'], toJSONTransform: function userToJSON (plainObj, originalDoc) { plainObj._tmp = originalDoc._tmp; // be sure to send down drop notifs diff --git a/website/server/models/user/methods.js b/website/server/models/user/methods.js index 70821e0b95..5596fde843 100644 --- a/website/server/models/user/methods.js +++ b/website/server/models/user/methods.js @@ -502,3 +502,9 @@ schema.methods.toJSONWithInbox = async function userToJSONWithInbox () { return toJSON; }; + +schema.methods.getSecretData = function getSecretData () { + const user = this; + + return user.secret; +}; diff --git a/website/server/models/user/schema.js b/website/server/models/user/schema.js index b4d1d14633..4788278a15 100644 --- a/website/server/models/user/schema.js +++ b/website/server/models/user/schema.js @@ -655,6 +655,11 @@ export default new Schema({ path: { $type: String }, type: { $type: String }, }], + + // only visible to staff and moderators + secret: { + text: String, + }, }, { skipVersioning: { notifications: true }, strict: true,