From d04f681986f4b63b88f7f111687b4bc73701af67 Mon Sep 17 00:00:00 2001 From: Jack Foltz Date: Wed, 2 Jan 2019 15:20:53 -0500 Subject: [PATCH] Clean up auth code and util --- app/routes/api/auth.js | 159 ++++++++++++++++++------------------------ app/util/auth/canonicalize.js | 3 + app/util/rateLimit.js | 9 +++ test/api.js | 7 +- 4 files changed, 81 insertions(+), 97 deletions(-) create mode 100644 app/util/auth/canonicalize.js create mode 100644 app/util/rateLimit.js diff --git a/app/routes/api/auth.js b/app/routes/api/auth.js index a029fa8..4c44d01 100644 --- a/app/routes/api/auth.js +++ b/app/routes/api/auth.js @@ -2,17 +2,18 @@ const express = require('express'); const router = express.Router(); const config = require('config'); const fs = require('fs').promises; +const passport = require('passport'); + +const canonicalize = require('../../util/auth/canonicalize'); 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/auth').requireAuth; const verifyBody = require('../../util/verifyBody'); -const rateLimit = require('express-rate-limit'); +const rateLimit = require('../../util/rateLimit'); // Wraps passport.authenticate to return a promise const authenticate = (req, res, next) => { @@ -23,113 +24,87 @@ const authenticate = (req, res, next) => { }); }; -// Wraps passport session creation for async usage +// Wraps passport session creation to return a promise const login = (user, req) => { return new Promise((resolve) => { req.login(user, resolve); }); }; -// Query the database for a valid invite code. An error message property is set if invalid. -const validateInvite = async (req, res, next) => { - const invite = await Invite.findOne({code: req.body.invite}).catch(next); - - if (!invite) { - // Log failure - await fs.appendFile('auth.log', `${new Date().toISOString()} register ${req.ip}\n`); - return res.status(422).json({message: 'Invalid invite code.'}); - } - - if (invite.used) - return res.status(422).json({message: 'Invite already used.'}); - - if (invite.expires != null && invite.expires < Date.now()) - return res.status(422).json({message: 'Invite expired.'}); - - req.invite = invite; - next(); -}; - -// Check if the requested username is valid -const validateUsername = async (req, res, next) => { - const username = req.body.username; - - const count = await User.countDocuments({username: username}).catch(next); - if (count !== 0) - return res.status(422).json({message: 'Username in use.'}); - - next(); -}; - -const registerLimiter = config.get('RateLimit.enable') - ? rateLimit({ - windowMs: config.get('RateLimit.register.window') * 1000, - max: config.get('RateLimit.register.max'), - skipSuccessfulRequests: true - }) - : (req, res, next) => { next(); }; -const registerProps = [ - { - name: 'displayname', - type: 'string', - maxLength: config.get('User.Username.maxLength'), - sanitize: true, - restrict: new RegExp(config.get('User.Username.restrictedChars')), - }, +const registerParams = [ + {name: 'displayname', type: 'string', maxLength: config.get('User.Username.maxLength'), sanitize: true, restrict: new RegExp(config.get('User.Username.restrictedChars'))}, {name: 'password', type: 'string'}, {name: 'invite', type: 'string'}]; + router.post('/register', - registerLimiter, - verifyBody(registerProps), canonicalizeRequest, - validateInvite, validateUsername, - async (req, res, next) => { - // Update the database - await Promise.all([ - User.register({ - username: req.body.username, + rateLimit(config.get('RateLimit.register.window'), config.get('RateLimit.register.max'), true), + verifyBody(registerParams), + async (req, res) => { + const username = canonicalize(req.body.displayname); + + // Retrieve invite and username status + const [invite, usernameCount] = await Promise.all([ + Invite.findOne({code: req.body.invite}), + User.countDocuments({username: username}) + ]); + + // Validate the invite + if (!invite) + return res.status(422).json({message: 'Invalid invite code.'}); + if (invite.used) + return res.status(422).json({message: 'Invite already used.'}); + if (invite.expires != null && invite.expires < Date.now()) + return res.status(422).json({message: 'Invite expired.'}); + + // Validate the username + if (usernameCount !== 0) + return res.status(422).json({message: 'Username in use.'}); + + // Create the user object + await User.register({ + username: username, displayname: req.body.displayname, - scope: req.invite.scope, + scope: invite.scope, date: Date.now() - }, req.body.password).catch(next), - Invite.updateOne({code: req.invite.code}, {recipient: req.body.username, used: Date.now()}).catch(next) - ]); + }, req.body.password); - res.status(200).json({'message': 'Registration successful.'}); -}); + // Update the invite as used + await Invite.updateOne({code: invite.code}, {recipient: username, used: Date.now()}); -const loginLimiter = config.get('RateLimit.enable') - ? rateLimit({ - windowMs: config.get('RateLimit.login.window') * 1000, - max: config.get('RateLimit.login.max'), - skipSuccessfulRequests: true - }) - : (req, res, next) => { next(); }; -const loginProps = [ - {name: 'username', type: 'string', optional: true}, - {name: 'displayname', type: 'string', optional: true}, + res.status(200).json({'message': 'Registration successful.'}); + }); + + + +const loginParams = [ + {name: 'displayname', type: 'string'}, {name: 'password', type: 'string'}]; + router.post('/login', - loginLimiter, - verifyBody(loginProps), - canonicalizeRequest, + rateLimit(config.get('RateLimit.login.window'), config.get('RateLimit.login.max'), true), + verifyBody(loginParams), async (req, res, next) => { - // Authenticate - const user = await authenticate(req, res, next); - if (!user) { - // Log failure - await fs.appendFile('auth.log', `${new Date().toISOString()} login ${req.ip}\n`); - return res.status(401).json({'message': 'Unauthorized.'}); - } + req.body.username = canonicalize(req.body.displayname); - // Create session - await login(user, req); + // Authenticate + const user = await authenticate(req, res, next); + if (!user) { + // Log failure + await fs.appendFile('auth.log', `${new Date().toISOString()} login ${req.ip}\n`); + return res.status(401).json({'message': 'Unauthorized.'}); + } + + // Create session + await login(user, req); + + // Set session vars + req.session.passport.displayname = user.displayname; + req.session.passport.scope = user.scope; + + res.status(200).json({'message': 'Logged in.'}); + }); - // Set session vars - req.session.passport.displayname = user.displayname; - req.session.passport.scope = user.scope; - res.status(200).json({'message': 'Logged in.'}); -}); router.post('/logout', (req, res) => { if (!req.isAuthenticated()) @@ -139,6 +114,8 @@ router.post('/logout', (req, res) => { res.status(200).json({'message': 'Logged out.'}); }); + + router.get('/whoami', requireAuth(), (req, res) => { res.status(200).json({ username: req.username, diff --git a/app/util/auth/canonicalize.js b/app/util/auth/canonicalize.js new file mode 100644 index 0000000..d840cfb --- /dev/null +++ b/app/util/auth/canonicalize.js @@ -0,0 +1,3 @@ +const canonicalize = str => str.normalize('NFKD').toLowerCase(); + +module.exports = canonicalize; \ No newline at end of file diff --git a/app/util/rateLimit.js b/app/util/rateLimit.js new file mode 100644 index 0000000..d7b934c --- /dev/null +++ b/app/util/rateLimit.js @@ -0,0 +1,9 @@ +const config = require('config'); +const rateLimit = require('express-rate-limit'); + +const rateLimitRequest = (window, max, skipSuccessful) => + config.get('RateLimit.enable') + ? rateLimit({windowMs: window * 1000, max: max, skipSuccessfulRequests: skipSuccessful}) + : (req, res, next) => { next(); }; + +module.exports = rateLimitRequest; diff --git a/test/api.js b/test/api.js index 1128b64..a1335fe 100644 --- a/test/api.js +++ b/test/api.js @@ -13,7 +13,7 @@ const Invite = require(ModelPath + 'Invite.js'); const View = require(ModelPath + 'View.js'); const util = require('./testUtil.js'); -const canonicalize = require('../app/util/canonicalize').canonicalize; +const canonicalize = require('../app/util/auth/canonicalize'); const config = require('config'); let app; @@ -171,11 +171,6 @@ describe('Authentication', () => { return verifySuccessfulLogin({displayname: 'user', password: 'pass'}); }); - it('SHOULD accept a username instead of a displayname', async () => { - await util.createTestUser(agent); - return verifySuccessfulLogin({username: 'user', password: 'pass'}); - }); - it('SHOULD accept any non-normalized variant of a username with a valid password', async () => { await util.createTestInvite(); await util.registerUser({displayname: 'ᴮᴵᴳᴮᴵᴿᴰ', password: 'pass', invite: 'code'}, agent);