Preparatory Work for Smaller user doc (WIP) (#10245)

* protect all paths in user.pre(save using this.isDirectSelected to see if a field is available

* fix linting

* authWithHeaders: specify user fields to exclude instead of the ones to include, add comments, doc and improve test

* add more options to unit helper generateReq and add tests for excluding fields in authWithHeaders
This commit is contained in:
Matteo Pagliazzi
2018-04-12 21:17:47 +02:00
committed by GitHub
parent ace02893e5
commit 1ea9be8aa2
6 changed files with 90 additions and 31 deletions

View File

@@ -34,6 +34,8 @@ describe('GET /user', () => {
expect(returnedUser._id).to.equal(user._id);
expect(returnedUser.achievements).to.exist;
expect(returnedUser.items.mounts).to.exist;
// Notifications are always returned
expect(returnedUser.notifications).to.exist;
expect(returnedUser.stats).to.not.exist;
});
});

View File

@@ -0,0 +1,40 @@
import {
generateRes,
generateReq,
} from '../../../../helpers/api-unit.helper';
import { authWithHeaders as authWithHeadersFactory } from '../../../../../website/server/middlewares/auth';
describe('auth middleware', () => {
let res, req, user;
beforeEach(async () => {
res = generateRes();
req = generateReq();
user = await res.locals.user.save();
});
describe('auth with headers', () => {
it('allows to specify a list of user field that we do not want to load', (done) => {
const authWithHeaders = authWithHeadersFactory(false, {
userFieldsToExclude: ['items', 'flags', 'auth.timestamps'],
});
req.headers['x-api-user'] = user._id;
req.headers['x-api-key'] = user.apiToken;
authWithHeaders(req, res, (err) => {
if (err) return done(err);
const userToJSON = res.locals.user.toJSON();
expect(userToJSON.items).to.not.exist;
expect(userToJSON.flags).to.not.exist;
expect(userToJSON.auth.timestamps).to.not.exist;
expect(userToJSON.auth).to.exist;
expect(userToJSON.notifications).to.exist;
expect(userToJSON.preferences).to.exist;
done();
});
});
});
});

View File

@@ -54,10 +54,15 @@ export function generateReq (options = {}) {
body: {},
query: {},
headers: {},
header: sandbox.stub().returns(null),
header (header) {
return this.headers[header];
},
session: {},
};
return defaultsDeep(options, defaultReq);
const req = defaultsDeep(options, defaultReq);
return req;
}
export function generateNext (func) {

View File

@@ -55,6 +55,11 @@ let api = {};
* Tags
* TasksOrder (list of all ids for dailys, habits, rewards and todos)
*
* @apiParam (Query) {UUID} userFields A list of comma separated user fields to be returned instead of the entire document. Notifications are always returned.
*
* @apiExample {curl} Example use:
* curl -i https://habitica.com/api/v3/user?userFields=achievements,items.mounts
*
* @apiSuccess {Object} data The user object
*
* @apiSuccessExample {json} Result:

View File

@@ -9,13 +9,22 @@ import url from 'url';
const COMMUNITY_MANAGER_EMAIL = nconf.get('EMAILS:COMMUNITY_MANAGER_EMAIL');
function getUserFields (userFieldProjection, req) {
if (userFieldProjection) return `notifications ${userFieldProjection}`;
function getUserFields (userFieldsToExclude, req) {
// A list of user fields that aren't needed for the route and are not loaded from the db.
// Must be an array
if (userFieldsToExclude) {
return userFieldsToExclude.map(field => {
return `-${field}`; // -${field} means exclude ${field} in mongodb
}).join(' ');
}
// Allows GET requests to /user to specify a list of user fields to return instead of the entire doc
// Notifications are always included
const urlPath = url.parse(req.url).pathname;
if (!req.query.userFields || urlPath !== '/user') return '';
const userFields = req.query.userFields;
if (!userFields || urlPath !== '/user') return '';
const userFieldOptions = req.query.userFields.split(',');
const userFieldOptions = userFields.split(',');
if (userFieldOptions.length === 0) return '';
return `notifications ${userFieldOptions.join(' ')}`;
@@ -25,7 +34,7 @@ function getUserFields (userFieldProjection, req) {
// Authenticate a request through the x-api-user and x-api key header
// If optional is true, don't error on missing authentication
export function authWithHeaders (optional = false, userFieldProjection = '') {
export function authWithHeaders (optional = false, options = {}) {
return function authWithHeadersHandler (req, res, next) {
let userId = req.header('x-api-user');
let apiToken = req.header('x-api-key');
@@ -40,8 +49,8 @@ export function authWithHeaders (optional = false, userFieldProjection = '') {
apiToken,
};
const fields = getUserFields(userFieldProjection, req);
const findPromise = fields ? User.findOne(userQuery, fields) : User.findOne(userQuery);
const fields = getUserFields(options.userFieldsToExclude, req);
const findPromise = fields ? User.findOne(userQuery).select(fields) : User.findOne(userQuery);
return findPromise
.exec()

View File

@@ -208,19 +208,11 @@ schema.pre('save', true, function preSaveUser (next, done) {
// we do not want to run any hook that relies on user.items because it will
// use the default values defined in the user schema and not the real ones.
//
// To check if a field was selected Document.isSelected('field') can be used.
// more info on its usage can be found at http://mongoosejs.com/docs/api.html#document_Document-isSelected
// IMPORTANT NOTE2 : due to a bug in mongoose (https://github.com/Automattic/mongoose/issues/5063)
// document.isSelected('items') will return true even if only a sub field (like 'items.mounts')
// was selected. So this fix only works as long as the entire subdoc is selected
// For example in the code below it won't work if only `achievements.beastMasterCount` is selected
// which is why we should only ever select the full paths and not subdocs,
// or if we really have to do the document.isSelected() calls should check for
// every specific subpath (items.mounts, items.pets, ...) but it's better to avoid it
// since it'll break as soon as a new field is added to the schema but not here.
// To check if a field was selected Document.isDirectSelected('field') can be used.
// more info on its usage can be found at http://mongoosejs.com/docs/api.html#document_Document-isDirectSelected
// do not calculate achievements if items or achievements are not selected
if (this.isSelected('items') && this.isSelected('achievements')) {
if (this.isDirectSelected('items') && this.isDirectSelected('achievements')) {
// Determines if Beast Master should be awarded
let beastMasterProgress = common.count.beastMasterProgress(this.items.pets);
@@ -250,7 +242,7 @@ schema.pre('save', true, function preSaveUser (next, done) {
}
// Manage unallocated stats points notifications
if (this.isSelected('stats') && this.isSelected('notifications') && this.isSelected('flags') && this.isSelected('preferences')) {
if (this.isDirectSelected('stats') && this.isDirectSelected('notifications') && this.isDirectSelected('flags') && this.isDirectSelected('preferences')) {
const pointsToAllocate = this.stats.points;
const classNotEnabled = !this.flags.classSelected || this.preferences.disableClasses;
@@ -287,21 +279,27 @@ schema.pre('save', true, function preSaveUser (next, done) {
}
}
// Enable weekly recap emails for old users who sign in
if (this.flags.lastWeeklyRecapDiscriminator) {
// Enable weekly recap emails in 24 hours
this.flags.lastWeeklyRecap = moment().subtract(6, 'days').toDate();
// Unset the field so this is run only once
this.flags.lastWeeklyRecapDiscriminator = undefined;
if (this.isDirectSelected('flags')) {
// Enable weekly recap emails for old users who sign in
if (this.flags.lastWeeklyRecapDiscriminator) {
// Enable weekly recap emails in 24 hours
this.flags.lastWeeklyRecap = moment().subtract(6, 'days').toDate();
// Unset the field so this is run only once
this.flags.lastWeeklyRecapDiscriminator = undefined;
}
}
if (_.isNaN(this.preferences.dayStart) || this.preferences.dayStart < 0 || this.preferences.dayStart > 23) {
this.preferences.dayStart = 0;
if (this.isDirectSelected('preferences')) {
if (_.isNaN(this.preferences.dayStart) || this.preferences.dayStart < 0 || this.preferences.dayStart > 23) {
this.preferences.dayStart = 0;
}
}
// our own version incrementer
if (_.isNaN(this._v) || !_.isNumber(this._v)) this._v = 0;
this._v++;
if (this.isDirectSelected('_v')) {
if (_.isNaN(this._v) || !_.isNumber(this._v)) this._v = 0;
this._v++;
}
// Populate new users with default content
if (this.isNew) {