diff --git a/api-server/server/boot_tests/fixtures.js b/api-server/server/boot_tests/fixtures.js index 666b1b5a9f..0bf9759a9f 100644 --- a/api-server/server/boot_tests/fixtures.js +++ b/api-server/server/boot_tests/fixtures.js @@ -1,17 +1,20 @@ export const firstChallengeUrl = '/learn/the/first/challenge'; export const requestedChallengeUrl = '/learn/my/actual/challenge'; + export const mockChallenge = { id: '123abc', block: 'actual', superBlock: 'my', dashedName: 'challenge' }; + export const mockFirstChallenge = { id: '456def', block: 'first', superBlock: 'the', dashedName: 'challenge' }; + export const mockCompletedChallenge = { id: '890xyz', challengeType: 0, @@ -26,6 +29,7 @@ export const mockCompletedChallenge = { ], completedDate: Date.now() }; + export const mockCompletedChallenges = [ { id: 'bd7123c8c441eddfaeb5bdef', @@ -57,12 +61,16 @@ export const mockCompletedChallenges = [ files: [] } ]; +export const mockUserID = '5c7d892aff9777c8b1c1a95e'; export const mockUser = { + id: mockUserID, username: 'camperbot', currentChallengeId: '123abc', timezone: 'UTC', - completedChallenges: mockCompletedChallenges + completedChallenges: mockCompletedChallenges, + progressTimestamps: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] }; + export const mockApp = { models: { Challenge: { @@ -74,10 +82,20 @@ export const mockApp = { ? cb(null, mockChallenge) : cb(new Error('challenge not found')); } + }, + User: { + findById(id, cb) { + if (id === mockUser.id) { + return cb(null, mockUser); + } + return cb(Error('No user')); + } } } }; + export const mockGetFirstChallenge = () => firstChallengeUrl; + export const firstChallengeQuery = { // first challenge of the first block of the first superBlock where: { challengeOrder: 0, superOrder: 1, order: 0 } diff --git a/api-server/server/component-passport.js b/api-server/server/component-passport.js index aa41ca6eb5..11f6f18806 100644 --- a/api-server/server/component-passport.js +++ b/api-server/server/component-passport.js @@ -7,6 +7,7 @@ import { import url from 'url'; import dedent from 'dedent'; +import { getUserById } from './utils/user-stats'; import { homeLocation } from '../../config/env'; import passportProviders from './passport-providers'; import { setAccessTokenToResponse } from './utils/getSetAccessToken'; @@ -16,28 +17,6 @@ const passportOptions = { profileToUser: null }; -const fields = { - progressTimestamps: false -}; - -function getCompletedCertCount(user) { - return [ - 'isApisMicroservicesCert', - 'is2018DataVisCert', - 'isFrontEndLibsCert', - 'isInfosecQaCert', - 'isJsAlgoDataStructCert', - 'isRespWebDesignCert' - ].reduce((sum, key) => (user[key] ? sum + 1 : sum), 0); -} - -function getLegacyCertCount(user) { - return ['isFrontEndCert', 'isBackEndCert', 'isDataVisCert'].reduce( - (sum, key) => (user[key] ? sum + 1 : sum), - 0 - ); -} - PassportConfigurator.prototype.init = function passportInit(noSession) { this.app.middleware('session:after', passport.initialize()); @@ -50,62 +29,15 @@ PassportConfigurator.prototype.init = function passportInit(noSession) { // Serialization and deserialization is only required if passport session is // enabled - passport.serializeUser((user, done) => { - done(null, user.id); - }); + passport.serializeUser((user, done) => done(null, user.id)); - passport.deserializeUser((id, done) => { - this.userModel.findById(id, { fields }, (err, user) => { - if (err || !user) { - return done(err, user); - } - - return this.app.dataSources.db.connector - .collection('user') - .aggregate([ - { $match: { _id: user.id } }, - { $project: { points: { $size: '$progressTimestamps' } } } - ]) - .get(function(err, [{ points = 1 } = {}]) { - if (err) { - console.error(err); - return done(err); - } - user.points = points; - let completedChallengeCount = 0; - let completedProjectCount = 0; - if ('completedChallenges' in user) { - completedChallengeCount = user.completedChallenges.length; - user.completedChallenges.forEach(item => { - if ( - 'challengeType' in item && - (item.challengeType === 3 || item.challengeType === 4) - ) { - completedProjectCount++; - } - }); - } - user.completedChallengeCount = completedChallengeCount; - user.completedProjectCount = completedProjectCount; - user.completedCertCount = getCompletedCertCount(user); - user.completedLegacyCertCount = getLegacyCertCount(user); - user.completedChallenges = []; - return done(null, user); - }); - }); + passport.deserializeUser(async (id, done) => { + const user = await getUserById(id).catch(done); + return done(null, user); }); }; export function setupPassport(app) { - // NOTE(Bouncey): Not sure this is doing much now - // Loopback complains about userCredentialModle when this - // setup is remoed from server/server.js - // - // I have split the custom callback in to it's own export that we can use both - // here and in boot:auth - // - // Needs more investigation... - const configurator = new PassportConfigurator(app); configurator.setupModels({ diff --git a/api-server/server/middlewares/request-authorizaion.test.js b/api-server/server/middlewares/request-authorizaion.test.js index 735db4aeb7..029499c3d0 100644 --- a/api-server/server/middlewares/request-authorizaion.test.js +++ b/api-server/server/middlewares/request-authorizaion.test.js @@ -111,7 +111,7 @@ describe('request-authorization', () => { }); it('adds the user to the request object', async done => { - expect.assertions(5); + expect.assertions(3); const validJWT = jwt.sign({ accessToken }, validJWTSecret); const req = mockReq({ path: '/internal/some-path/that-needs/auth', @@ -124,8 +124,6 @@ describe('request-authorization', () => { expect(next.called).toBe(true); expect(req).toHaveProperty('user'); expect(req.user).toEqual(users['456def']); - expect(req.user).toHaveProperty('points'); - expect(req.user.points).toEqual(4); return done(); }); @@ -200,7 +198,7 @@ describe('request-authorization', () => { }); it('adds the user to the request object', async done => { - expect.assertions(5); + expect.assertions(3); const validJWT = jwt.sign({ accessToken }, validJWTSecret); const req = mockReq({ path: '/internal/some-path/that-needs/auth', @@ -212,8 +210,6 @@ describe('request-authorization', () => { expect(next.called).toBe(true); expect(req).toHaveProperty('user'); expect(req.user).toEqual(users['456def']); - expect(req.user).toHaveProperty('points'); - expect(req.user.points).toEqual(4); return done(); }); diff --git a/api-server/server/middlewares/request-authorization.js b/api-server/server/middlewares/request-authorization.js index caf419af9b..17a7f40795 100644 --- a/api-server/server/middlewares/request-authorization.js +++ b/api-server/server/middlewares/request-authorization.js @@ -1,5 +1,6 @@ -import loopback from 'loopback'; import { isEmpty } from 'lodash'; + +import { getUserById as _getUserById } from '../utils/user-stats'; import { getAccessTokenFromRequest, errorTypes, @@ -63,7 +64,6 @@ export default ({ jwtSecret = _jwtSecret, getUserById = _getUserById } = {}) => return getUserById(userId) .then(user => { if (user) { - user.points = user.progressTimestamps.length; req.user = user; } return; @@ -76,15 +76,3 @@ export default ({ jwtSecret = _jwtSecret, getUserById = _getUserById } = {}) => } return Promise.resolve(next()); }; - -export function _getUserById(id) { - const User = loopback.getModelByType('User'); - return new Promise((resolve, reject) => - User.findById(id, (err, instance) => { - if (err) { - return reject(err); - } - return resolve(instance); - }) - ); -} diff --git a/api-server/server/utils/user-stats.js b/api-server/server/utils/user-stats.js index 2ee987bcce..b26a7854bc 100644 --- a/api-server/server/utils/user-stats.js +++ b/api-server/server/utils/user-stats.js @@ -1,10 +1,13 @@ +import loopback from 'loopback'; import compose from 'lodash/fp/compose'; import map from 'lodash/fp/map'; import sortBy from 'lodash/fp/sortBy'; import trans from 'lodash/fp/transform'; import last from 'lodash/fp/last'; import forEachRight from 'lodash/fp/forEachRight'; +import { isEmpty } from 'lodash'; import moment from 'moment-timezone'; + import { dayCount } from '../utils/date-utils'; const transform = trans.convert({ cap: false }); @@ -100,3 +103,54 @@ export function calcLongestStreak(cals, tz = 'UTC') { return dayCount(longest, tz); } + +export function getUserById(id, User = loopback.getModelByType('User')) { + return new Promise((resolve, reject) => + User.findById(id, (err, instance) => { + if (err || isEmpty(instance)) { + return reject(err || 'No user instance found'); + } + instance.points = + (instance.progressTimestamps && instance.progressTimestamps.length) || + 1; + let completedChallengeCount = 0; + let completedProjectCount = 0; + if ('completedChallenges' in instance) { + completedChallengeCount = instance.completedChallenges.length; + instance.completedChallenges.forEach(item => { + if ( + 'challengeType' in item && + (item.challengeType === 3 || item.challengeType === 4) + ) { + completedProjectCount++; + } + }); + } + instance.completedChallengeCount = completedChallengeCount; + instance.completedProjectCount = completedProjectCount; + instance.completedCertCount = getCompletedCertCount(instance); + instance.completedLegacyCertCount = getLegacyCertCount(instance); + instance.completedChallenges = []; + delete instance.progressTimestamps; + return resolve(instance); + }) + ); +} + +function getCompletedCertCount(user) { + return [ + 'isApisMicroservicesCert', + 'is2018DataVisCert', + 'isFrontEndLibsCert', + 'isInfosecQaCert', + 'isJsAlgoDataStructCert', + 'isRespWebDesignCert' + ].reduce((sum, key) => (user[key] ? sum + 1 : sum), 0); +} + +function getLegacyCertCount(user) { + return ['isFrontEndCert', 'isBackEndCert', 'isDataVisCert'].reduce( + (sum, key) => (user[key] ? sum + 1 : sum), + 0 + ); +} diff --git a/api-server/server/utils/user-stats.test.js b/api-server/server/utils/user-stats.test.js index 0ede7866be..bddf42285b 100644 --- a/api-server/server/utils/user-stats.test.js +++ b/api-server/server/utils/user-stats.test.js @@ -5,8 +5,10 @@ import sinon from 'sinon'; import { prepUniqueDaysByHours, calcCurrentStreak, - calcLongestStreak + calcLongestStreak, + getUserById } from './user-stats'; +import { mockUserID, mockApp, mockUser } from '../boot_tests/fixtures'; // setting now to 2016-02-03T11:00:00 (PST) const clock = sinon.useFakeTimers(1454526000000); @@ -577,4 +579,51 @@ describe('user stats', () => { } ); }); + + describe('getUserById', () => { + const stubUser = { + findById(id, cb) { + cb(null, { id: 123 }); + } + }; + it('returns a promise', () => { + expect.assertions(3); + expect(typeof getUserById('123', stubUser).then).toEqual('function'); + expect(typeof getUserById('123', stubUser).catch).toEqual('function'); + expect(typeof getUserById('123', stubUser).finally).toEqual('function'); + }); + + it('resolves a user for a given id', done => { + expect.assertions(7); + return getUserById(mockUserID, mockApp.models.User) + .then(user => { + expect(user).toEqual(mockUser); + + // fields added or removed by getUserById + expect(user).not.toHaveProperty('progressTimestamps'); + expect(user).toHaveProperty('completedChallengeCount'); + expect(user).toHaveProperty('completedProjectCount'); + expect(user).toHaveProperty('completedCertCount'); + expect(user).toHaveProperty('completedLegacyCertCount'); + expect(user.completedChallenges).toEqual([]); + }) + .then(done) + .catch(done); + }); + + it('throws when no user is found', done => { + const noUserError = new Error('No user found'); + const throwyUserModel = { + findById(_, cb) { + cb(noUserError); + } + }; + expect( + getUserById('not-a-real-id', throwyUserModel).catch(error => { + expect(error).toEqual(noUserError); + done(); + }) + ); + }); + }); });