From 7441eaaf02efc40462bd74a1f161e2467029ba3d Mon Sep 17 00:00:00 2001 From: Jack Foltz Date: Thu, 26 Jul 2018 19:40:42 -0400 Subject: [PATCH] Make use of username/displayname field consistent throughout api --- app/models/Invite.js | 36 +++++++++++++++++++++++----- app/models/Key.js | 31 +++++++++++++++++++----- app/models/User.js | 20 ++++++++++++---- app/routes/auth.js | 42 ++++++++++++++++----------------- app/routes/upload.js | 10 ++++---- app/util/canonicalize.js | 4 ++-- app/util/requireAuth.js | 16 ++++++------- test/api.js | 61 ++++++++++++++++++++++++++---------------------- test/testUtil.js | 18 +++++++------- 9 files changed, 148 insertions(+), 90 deletions(-) diff --git a/app/models/Invite.js b/app/models/Invite.js index 2159cb5..a558cbc 100644 --- a/app/models/Invite.js +++ b/app/models/Invite.js @@ -6,12 +6,36 @@ var InviteSchema = mongoose.Schema({ unique: true, required: true }, - scope: [String], - issuer: String, - recipient: String, - issued: Date, - used: Date, - exp: Date + + scope: { + type: [String], + required: true + }, + + issuer: { + type: String, + required: true + }, + + recipient: { + type: String, + default: null + }, + + issued: { + type: Date, + default: Date.now + }, + + used: { + type: Date, + default: null + }, + + exp: { + type: Date, + default: null + } }); /*InviteSchema.methods.use = function(canonicalname, cb) { diff --git a/app/models/Key.js b/app/models/Key.js index 03dea07..3df9395 100644 --- a/app/models/Key.js +++ b/app/models/Key.js @@ -1,22 +1,41 @@ -var mongoose = require('mongoose'); +const mongoose = require('mongoose'); + +const KeySchema = mongoose.Schema({ + key: { + type: String, + unique: true, + required: true + }, -var KeySchema = mongoose.Schema({ - key: String, identifier: { type: String, required: true }, - scope: [String], + + scope: { + type: [String], + required: true, + }, + uploadCount: { type: Number, default: 0 }, + uploadSize: { type: Number, default: 0 }, - username: String, - date: Date + + issuer: { + type: String, + required: true + }, + + date: { + type: Date, + default: Date.now + } }); module.exports = mongoose.model('Key', KeySchema); \ No newline at end of file diff --git a/app/models/User.js b/app/models/User.js index 87a1e44..110b4cb 100644 --- a/app/models/User.js +++ b/app/models/User.js @@ -7,26 +7,36 @@ var UserSchema = mongoose.Schema({ unique: true, required: true }, - canonicalname: { + + displayname: { type: String, unique: true, required: true }, - scope: [String], + + scope: { + type: [String], + required: true + }, + uploadCount: { type: Number, default: 0 }, + uploadSize: { type: Number, default: 0 }, - date: Date + + date: { + type: Date, + default: Date.now + } }); UserSchema.plugin(passportLocalMongoose, { - usernameField: 'canonicalname', - saltlen: 32, + saltlen: 64, iterations: 10000, limitAttempts: true }); diff --git a/app/routes/auth.js b/app/routes/auth.js index 640fea4..26ea4f3 100644 --- a/app/routes/auth.js +++ b/app/routes/auth.js @@ -1,17 +1,17 @@ -'use strict'; - const express = require('express'); const router = express.Router(); -const User = require('../models/User.js'); -const Invite = require('../models/Invite.js'); -const passport = require('passport'); const config = require('config'); +const ModelPath = '../models/'; +const User = require(ModelPath + 'User.js'); +const Invite = require(ModelPath + 'Invite.js'); + +const passport = require('passport'); + const canonicalizeRequest = require('../util/canonicalize').canonicalizeRequest; const requireAuth = require('../util/requireAuth').requireAuth; const wrap = require('../util/wrap.js').wrap; - // Wraps passport.authenticate to return a promise function authenticate(req, res, next) { return new Promise((resolve) => { @@ -28,16 +28,16 @@ function login(user, req) { }); } -// Check if a canonical name is valid -async function validateUsername(username, canonicalName, sanitize) { - if (canonicalName.length > config.get('User.Username.maxLength')) +// Check if the requested username is valid +async function validateUsername(username, sanitize) { + if (username.length > config.get('User.Username.maxLength')) return {valid: false, message: 'Username too long.'}; const restrictedRegex = new RegExp(config.get('User.Username.restrictedChars'), 'g'); - if (canonicalName !== sanitize(canonicalName).replace(restrictedRegex, '')) + if (username !== sanitize(username).replace(restrictedRegex, '')) return {valid: false, message: 'Username contains invalid characters.'}; - const count = await User.countDocuments({canonicalname: canonicalName}); + const count = await User.countDocuments({username: username}); if (count !== 0) return {valid: false, message: 'Username in use.'}; @@ -55,19 +55,19 @@ async function validateInvite(code) { if (invite.used) return {valid: false, message: 'Invite already used.'}; - if (invite.exp < Date.now()) + if (invite.exp != null && invite.exp < Date.now()) return {valid: false, message: 'Invite expired.'}; return {valid: true, invite: invite}; } -router.post('/register', canonicalizeRequest, wrap(async (req, res, next) => { +router.post('/register', canonicalizeRequest, wrap(async (req, res) => { // Validate the invite and username const [inviteStatus, usernameStatus] = await Promise.all([ validateInvite(req.body.invite), - validateUsername(req.body.username, req.body.canonicalname, req.sanitize) + validateUsername(req.body.username, req.sanitize) ]); // Error if validation failed @@ -80,11 +80,11 @@ router.post('/register', canonicalizeRequest, wrap(async (req, res, next) => { await Promise.all([ User.register({ username: req.body.username, - canonicalname: req.body.canonicalname, + displayname: req.body.displayname, scope: inviteStatus.invite.scope, date: Date.now() }, req.body.password), - Invite.updateOne({code: inviteStatus.invite.code}, {recipient: req.body.canonicalname, used: Date.now()}) + Invite.updateOne({code: inviteStatus.invite.code}, {recipient: req.body.username, used: Date.now()}) ]); res.status(200).json({'message': 'Registration successful.'}); @@ -100,7 +100,7 @@ router.post('/login', canonicalizeRequest, wrap(async (req, res, next) => { await login(user, req); // Set session vars - req.session.passport.display = user.username; + req.session.passport.displayname = user.displayname; req.session.passport.scope = user.scope; res.status(200).json({'message': 'Logged in.'}); @@ -113,10 +113,10 @@ router.post('/logout', function (req, res) { router.get('/whoami', requireAuth(), (req, res) => { res.status(200).json({ - user: req.authUser, - display: req.authDisplay, - scope: req.authScope, - key: req.authKey + user: req.username, + display: req.displayname, + scope: req.scope, + key: req.key }); }); diff --git a/app/routes/upload.js b/app/routes/upload.js index 9dcf410..2f4b732 100644 --- a/app/routes/upload.js +++ b/app/routes/upload.js @@ -32,9 +32,9 @@ const generateId = async () => { const updateStats = async req => Promise.all([ - User.updateOne({username: req.authUser}, {$inc: {uploadCount: 1, uploadSize: req.file.size}}), - req.authKey - ? Key.updateOne({key: req.authKey}, {$inc: {uploadCount: 1, uploadSize: req.file.size}}) + User.updateOne({username: req.username}, {$inc: {uploadCount: 1, uploadSize: req.file.size}}), + req.key + ? Key.updateOne({key: req.key}, {$inc: {uploadCount: 1, uploadSize: req.file.size}}) : Promise.resolve() ]); @@ -50,8 +50,8 @@ router.post('/', requireAuth('file.upload'), fileUpload, wrap(async (req, res) = const upload = { id: await generateId(), - uploader: req.authUser, - uploaderKey: req.authKey, + uploader: req.username, + uploaderKey: req.key, date: Date.now(), file: req.file }; diff --git a/app/util/canonicalize.js b/app/util/canonicalize.js index e27df4c..1649891 100644 --- a/app/util/canonicalize.js +++ b/app/util/canonicalize.js @@ -1,8 +1,8 @@ // Normalizes, decomposes, and lowercases a utf-8 string -exports.canonicalize = (username) => username.normalize('NFKD').toLowerCase(); +exports.canonicalize = displayname => displayname.normalize('NFKD').toLowerCase(); exports.canonicalizeRequest = (req, res, next) => { - req.body.canonicalname = exports.canonicalize(req.body.username); + req.body.username = exports.canonicalize(req.body.displayname); next(); }; \ No newline at end of file diff --git a/app/util/requireAuth.js b/app/util/requireAuth.js index a21ca8d..fe7be9e 100644 --- a/app/util/requireAuth.js +++ b/app/util/requireAuth.js @@ -12,10 +12,10 @@ exports.requireAuth = scope => wrap(async (req, res, next) => { if (req.isAuthenticated()) { if (scope ? verifyScope(req.session.passport.scope, scope) : true) { - req.authUser = req.session.passport.user; - req.authDisplay = req.session.passport.display; - req.authScope = req.session.passport.scope; - req.authKey = null; + req.username = req.session.passport.user; + req.displayname = req.session.passport.displayname; + req.scope = req.session.passport.scope; + req.key = null; next(); } else { res.status(403).json({message: 'Forbidden.'}); @@ -23,10 +23,10 @@ exports.requireAuth = scope => } else if (req.body.apikey) { const key = await Key.findOne({key: apikey}); if (scope ? verifyScope(key.scope, scope) : true) { - req.authUser = key.username; - req.authDisplay = key.username; - req.authScope = key.scope; - req.authKey = key.key; + req.username = key.username; + req.displayname = key.username; + req.scope = key.scope; + req.key = key.key; next(); } else { res.status(403).json({message: 'Forbidden.'}); diff --git a/test/api.js b/test/api.js index b04a21d..7b884ed 100644 --- a/test/api.js +++ b/test/api.js @@ -2,6 +2,7 @@ process.env.NODE_ENV = 'test'; const chai = require('chai'); chai.use(require('chai-http')); +const should = chai.should(); const ModelPath = '../app/models/'; const User = require(ModelPath + 'User.js'); @@ -43,26 +44,26 @@ describe('Accounts', function() { res.body.should.be.a('object'); res.body.should.have.property('message').eql('Registration successful.'); - const userCount = await User.countDocuments({username: user.username}); + const userCount = await User.countDocuments({displayname: user.displayname}); userCount.should.equal(1); - const inviteCount = await Invite.countDocuments({code: user.invite, recipient: canonicalize(user.username)}); + const inviteCount = await Invite.countDocuments({code: user.invite, recipient: canonicalize(user.displayname)}); inviteCount.should.equal(1); } it('MUST register a valid user with a valid invite', async () => - verifySuccessfulRegister({username: 'user', password: 'pass', invite: 'code'}) + verifySuccessfulRegister({displayname: 'user', password: 'pass', invite: 'code'}) ); it('MUST register a username with unicode symbols and a valid invite', async () => - verifySuccessfulRegister({username: 'ᴮᴵᴳᴮᴵᴿᴰ', password: 'pass', invite: 'code'}) + verifySuccessfulRegister({displayname: 'ᴮᴵᴳᴮᴵᴿᴰ', password: 'pass', invite: 'code'}) ); }); describe('1 Invalid Invites', () => { async function verifyRejectedInvite(invite, message) { - const user = {username: 'user', password: 'pass', invite: 'code'}; + const user = {displayname: 'user', password: 'pass', invite: 'code'}; if (invite) { await util.createInvite(invite, agent); user.invite = invite.code; @@ -73,7 +74,7 @@ describe('Accounts', function() { res.body.should.be.a('object'); res.body.should.have.property('message').eql(message); - const inviteCount = await Invite.countDocuments({code: user.invite, recipient: canonicalize(user.username)}); + const inviteCount = await Invite.countDocuments({code: user.invite, recipient: canonicalize(user.displayname)}); inviteCount.should.equal(0); } @@ -82,30 +83,30 @@ describe('Accounts', function() { ); it('MUST NOT register a used invite', async () => - verifyRejectedInvite({code: 'code', used: new Date()}, 'Invite already used.') + verifyRejectedInvite({code: 'code', used: new Date(), issuer: 'Mocha'}, 'Invite already used.') ); it('MUST NOT register an expired invite', async () => - verifyRejectedInvite({code: 'code', exp: new Date()}, 'Invite expired.') + verifyRejectedInvite({code: 'code', exp: new Date(), issuer: 'Mocha'}, 'Invite expired.') ); }); - describe('2 Invalid Usernames', () => { + describe('2 Invalid Displaynames', () => { async function verifyRejectedUsername(user, message) { const res = await util.registerUser(user, agent); res.should.have.status(422); res.body.should.be.a('object'); res.body.should.have.property('message').equal(message); - const inviteCount = await Invite.countDocuments({code: user.invite, recipient: canonicalize(user.username)}); + const inviteCount = await Invite.countDocuments({code: user.invite, recipient: canonicalize(user.displayname)}); inviteCount.should.equal(0); } it('MUST NOT register a duplicate username', async () => { await util.createTestInvites(2); - const user0 = {username: 'user', password: 'pass', invite: 'code0'}; - const user1 = {username: 'user', password: 'diff', invite: 'code1'}; + const user0 = {displayname: 'user', password: 'pass', invite: 'code0'}; + const user1 = {displayname: 'user', password: 'diff', invite: 'code1'}; await util.registerUser(user0, agent); return verifyRejectedUsername(user1, 'Username in use.'); @@ -113,8 +114,8 @@ describe('Accounts', function() { it('MUST NOT register a username with a duplicate canonical name', async () => { await util.createTestInvites(2); - const user0 = {username: 'bigbird', password: 'pass', invite: 'code0'}; - const user1 = {username: 'ᴮᴵᴳᴮᴵᴿᴰ', password: 'diff', invite: 'code1'}; + const user0 = {displayname: 'bigbird', password: 'pass', invite: 'code0'}; + const user1 = {displayname: 'ᴮᴵᴳᴮᴵᴿᴰ', password: 'diff', invite: 'code1'}; await util.registerUser(user0, agent); return verifyRejectedUsername(user1, 'Username in use.'); @@ -123,9 +124,9 @@ describe('Accounts', function() { it('MUST NOT register a username containing whitespace', async () => { await util.createTestInvites(3); const users = [ - {username: 'user name', password: 'pass', invite: 'code0'}, - {username: 'user　name', password: 'pass', invite: 'code1'}, - {username: 'user name', password: 'pass', invite: 'code2'} + {displayname: 'user name', password: 'pass', invite: 'code0'}, + {displayname: 'user　name', password: 'pass', invite: 'code1'}, + {displayname: 'user name', password: 'pass', invite: 'code2'} ]; const failMsg = 'Username contains invalid characters.'; @@ -134,13 +135,13 @@ describe('Accounts', function() { it('MUST NOT register a username containing HTML', async () => { await util.createTestInvite(); - const user = {username: 'user', password: 'pass', invite: 'code'}; + const user = {displayname: 'user', password: 'pass', invite: 'code'}; return verifyRejectedUsername(user, 'Username contains invalid characters.'); }); it('MUST NOT register a username with too many characters', async () => { await util.createTestInvite(); - const user = {username: '123456789_123456789_123456789_1234567', password: 'pass', invite: 'code'}; + const user = {displayname: '123456789_123456789_123456789_1234567', password: 'pass', invite: 'code'}; return verifyRejectedUsername(user, 'Username too long.'); }) }); @@ -167,7 +168,7 @@ describe('Accounts', function() { describe('0 Valid Request', () => { it('SHOULD accept a valid user with a valid password', async () => { await util.createTestUser(agent); - return verifySuccessfulLogin({username: 'user', password: 'pass'}); + return verifySuccessfulLogin({displayname: 'user', password: 'pass'}); }); it('SHOULD accept any non-normalized variant of a username with a valid password', async () => { @@ -179,13 +180,13 @@ describe('Accounts', function() { describe('1 Invalid Password', () => { it('SHOULD NOT accept an invalid password', async () => { await util.createTestUser(agent); - return verifyFailedLogin({username: 'user', password: 'bogus'}); + return verifyFailedLogin({displayname: 'user', password: 'bogus'}); }); }); describe('2 Invalid User', () => { it('SHOULD NOT accept an invalid user', async () => - verifyFailedLogin({username: 'bogus', password: 'bogus'}) + verifyFailedLogin({displayname: 'bogus', password: 'bogus'}) ); }); }); @@ -195,12 +196,12 @@ describe('Uploads', () => { beforeEach(async () => util.clearDatabase()); describe('/POST upload', () => { - async function verifySuccessfulUpload(file, user) { + async function verifySuccessfulUpload(file, username) { // Get file stats beforehand const [fileHash, fileSize] = await Promise.all([util.fileHash(file), util.fileSize(file)]); // Get the user stats beforehand - const userBefore = await User.findOne({canonicalname: user}, {_id: 0, uploadCount: 1, uploadSize: 1}); + const userBefore = await User.findOne({username: username}, {_id: 0, uploadCount: 1, uploadSize: 1}); // Submit the upload and verify the result const res = await util.upload(file, agent); @@ -221,11 +222,15 @@ describe('Uploads', () => { uploadSize.should.equal(fileSize); // Verify the user's stats have been updated correctly - const userAfter = await User.findOne({canonicalname: user}, {_id: 0, uploadCount: 1, uploadSize: 1}); + const userAfter = await User.findOne({username: username}, {_id: 0, uploadCount: 1, uploadSize: 1}); userAfter.uploadCount.should.equal(userBefore.uploadCount + 1); userAfter.uploadSize.should.equal(userBefore.uploadSize + fileSize); } + async function verifySuccessfulKeyUpload(key, file) { + + } + async function verifyFailedUpload(file, status, message) { const fileCountBefore = await util.directoryFileCount(config.get('Upload.path')); const uploadCountBefore = await Upload.countDocuments({}); @@ -264,9 +269,9 @@ describe('Uploads', () => { ); it('SHOULD NOT accept a request without file.upload scope', async () => { - await util.createInvite({code: 'code', scope: []}); - await util.registerUser({username: 'user', password: 'pass', invite: 'code'}, agent); - await util.login({username: 'user', password: 'pass'}, agent); + await util.createInvite({code: 'code', scope: [], issuer: 'Mocha'}); + await util.registerUser({displayname: 'user', password: 'pass', invite: 'code'}, agent); + await util.login({displayname: 'user', password: 'pass'}, agent); await util.createTestFile(2048, 'test.bin'); diff --git a/test/testUtil.js b/test/testUtil.js index ca92dcd..7f10c19 100644 --- a/test/testUtil.js +++ b/test/testUtil.js @@ -2,12 +2,12 @@ process.env.NODE_ENV = 'test'; const chai = require('chai'); chai.use(require('chai-http')); -const should = chai.should(); -const User = require('../app/models/User.js'); -const Invite = require('../app/models/Invite.js'); -const Upload = require('../app/models/Upload.js'); -const Key = require('../app/models/Key.js'); +const ModelPath = '../app/models/'; +const User = require(ModelPath + 'User.js'); +const Upload = require(ModelPath + 'Upload.js'); +const Key = require(ModelPath + 'Key.js'); +const Invite = require(ModelPath + 'Invite.js'); const Buffer = require('buffer').Buffer; const crypto = require('crypto'); @@ -51,22 +51,22 @@ exports.whoami = (agent) => //---------------- TEST ENTRY CREATION ----------------// exports.createTestInvite = () => - exports.createInvite({code: 'code', scope: ['file.upload']}); + exports.createInvite({code: 'code', scope: ['file.upload'], issuer: 'Mocha'}); exports.createTestInvites = (n) => Promise.all( Array.from(new Array(n), (val, index) => 'code' + index) - .map(code => exports.createInvite({code: code})) + .map(code => exports.createInvite({code: code, scope: ['file.upload'], issuer: 'Mocha'})) ); exports.createTestUser = async agent => { await exports.createTestInvite(); - return exports.registerUser({username: 'user', password: 'pass', invite: 'code'}, agent); + return exports.registerUser({displayname: 'user', password: 'pass', invite: 'code'}, agent); }; exports.createTestSession = async agent => { await exports.createTestUser(agent); - return exports.login({username: 'user', password: 'pass'}, agent); + return exports.login({displayname: 'user', password: 'pass'}, agent); }; exports.createTestFile = (size, name) =>