API: Adding secret.text to the user-schema (#12121)

This commit is contained in:
negue
2020-05-02 19:59:05 +02:00
committed by GitHub
parent 643d3802cc
commit 26767f598b
17 changed files with 177 additions and 22 deletions

View File

@@ -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);
});
});

View File

@@ -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']);

View File

@@ -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']);

View File

@@ -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 () => {

View File

@@ -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;
});
});

View File

@@ -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;
});
});

View File

@@ -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');
});
});

View File

@@ -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) => {

View File

@@ -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();

View File

@@ -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');
});
});

View File

@@ -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) => {

View File

@@ -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));

View File

@@ -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';

View File

@@ -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;

View File

@@ -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

View File

@@ -502,3 +502,9 @@ schema.methods.toJSONWithInbox = async function userToJSONWithInbox () {
return toJSON;
};
schema.methods.getSecretData = function getSecretData () {
const user = this;
return user.secret;
};

View File

@@ -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,