From 1a389ec1dd3e16f65c65bb76ba814be2f1108ee8 Mon Sep 17 00:00:00 2001 From: Evert Date: Thu, 24 Aug 2017 01:25:52 +0300 Subject: [PATCH] recaptcha and rate limiting --- package-lock.json | 21 +++++ package.json | 1 + server/api/index.js | 2 +- server/api/oauth2/controller/code/code.js | 4 + server/api/oauth2/controller/code/implicit.js | 7 +- server/routes/api.js | 9 ++ server/routes/index.js | 94 ++++++++++++------- server/routes/oauth2.js | 9 ++ src/style/main.styl | 3 + views/register.pug | 3 + views/totp.pug | 2 +- 11 files changed, 117 insertions(+), 38 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4f2d726..b7f700f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -887,6 +887,11 @@ } } }, + "clone": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/clone/-/clone-1.0.2.tgz", + "integrity": "sha1-Jgt6meux7f4kdTgXX3gyQ8sZ0Uk=" + }, "co": { "version": "https://registry.npmjs.org/co/-/co-4.6.0.tgz", "integrity": "sha1-bqa989hTrlTMuOR7+gvz+QMfsYQ=" @@ -1236,6 +1241,14 @@ "integrity": "sha1-s2nW+128E+7PUk+RsHD+7cNXzzQ=", "dev": true }, + "defaults": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/defaults/-/defaults-1.0.3.tgz", + "integrity": "sha1-xlYFHpgX2f8I7YgUd/P+QBnz730=", + "requires": { + "clone": "1.0.2" + } + }, "define-properties": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/define-properties/-/define-properties-1.1.2.tgz", @@ -1785,6 +1798,14 @@ } } }, + "express-rate-limit": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-2.9.0.tgz", + "integrity": "sha1-YsKfyTnXLwoDqHQoxkf8TuDRWXg=", + "requires": { + "defaults": "1.0.3" + } + }, "express-session": { "version": "https://registry.npmjs.org/express-session/-/express-session-1.15.3.tgz", "integrity": "sha1-21RfBDWnsbIorgLagZf2UUFzXGc=", diff --git a/package.json b/package.json index 205a5c3..126cdce 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "connect-redis": "^3.3.0", "connect-session-knex": "^1.3.4", "express": "^4.15.3", + "express-rate-limit": "^2.9.0", "express-session": "^1.15.3", "knex": "^0.13.0", "mysql": "^2.13.0", diff --git a/server/api/index.js b/server/api/index.js index 3d6ecbd..e806bc5 100644 --- a/server/api/index.js +++ b/server/api/index.js @@ -153,7 +153,7 @@ const API = { return true }, totpAquire: async function (user) { - user = await API.User.ensureObject(user) + user = await API.User.ensureObject(user, ['password']) // Do not allow totp for users who have registered using an external service if (!user.password || user.password === '') return null diff --git a/server/api/oauth2/controller/code/code.js b/server/api/oauth2/controller/code/code.js index feeee78..cc2a00a 100644 --- a/server/api/oauth2/controller/code/code.js +++ b/server/api/oauth2/controller/code/code.js @@ -4,6 +4,10 @@ import model from '../../model' module.exports = async (req, res, client, scope, user, redirectUri, createAllowFuture) => { let codeValue = null + if (req.method === 'POST' && req.session.csrf && !(req.body.csrf && req.body.csrf === req.session.csrf)) { + throw new error.InvalidRequest('Invalid session') + } + if (createAllowFuture) { if (!req.body || (typeof req.body['decision']) === 'undefined') { throw new error.InvalidRequest('No decision parameter passed') diff --git a/server/api/oauth2/controller/code/implicit.js b/server/api/oauth2/controller/code/implicit.js index 1c807ad..4a2442b 100644 --- a/server/api/oauth2/controller/code/implicit.js +++ b/server/api/oauth2/controller/code/implicit.js @@ -4,10 +4,14 @@ import model from '../../model' module.exports = async (req, res, client, scope, user, redirectUri, createAllowFuture) => { let accessTokenValue = null + if (req.method === 'POST' && req.session.csrf && !(req.body.csrf && req.body.csrf === req.session.csrf)) { + throw new error.InvalidRequest('Invalid session') + } + if (createAllowFuture) { if (!req.body || (typeof req.body['decision']) === 'undefined') { throw new error.InvalidRequest('No decision parameter passed') - } else if (req.body['decision'] === 0) { + } else if (req.body['decision'] === '0') { throw new error.AccessDenied('User denied access to the resource') } else { console.debug('Decision check passed') @@ -20,6 +24,7 @@ module.exports = async (req, res, client, scope, user, redirectUri, createAllowF accessTokenValue = await req.oauth2.model.accessToken.create(req.oauth2.model.user.getId(user), req.oauth2.model.client.getId(client), scope, req.oauth2.model.accessToken.ttl) } catch (err) { + console.error(err) throw new error.ServerError('Failed to call accessToken.create function') } diff --git a/server/routes/api.js b/server/routes/api.js index 88fbbf2..bee73bf 100644 --- a/server/routes/api.js +++ b/server/routes/api.js @@ -1,5 +1,6 @@ import express from 'express' import parseurl from 'parseurl' +import RateLimit from 'express-rate-limit' import config from '../../scripts/load-config' import wrap from '../../scripts/asyncRoute' import API from '../api' @@ -7,6 +8,14 @@ import APIExtern from '../api/external' let router = express.Router() +let apiLimiter = new RateLimit({ + windowMs: 5 * 60 * 1000, // 5 minutes + max: 100, + delayMs: 0 +}) + +router.use(apiLimiter) + // Turn things like 'key1[key2]': 'value' into key1: {key2: 'value'} because facebook function objectAssembler (insane) { let object = {} diff --git a/server/routes/index.js b/server/routes/index.js index 064eeb3..60f2132 100644 --- a/server/routes/index.js +++ b/server/routes/index.js @@ -1,9 +1,11 @@ import fs from 'fs' import path from 'path' import express from 'express' +import RateLimit from 'express-rate-limit' import parseurl from 'parseurl' import config from '../../scripts/load-config' import wrap from '../../scripts/asyncRoute' +import http from '../../scripts/http' import API from '../api' import apiRouter from './api' @@ -11,6 +13,13 @@ import oauthRouter from './oauth2' let router = express.Router() +let accountLimiter = new RateLimit({ + windowMs: 60 * 60 * 1000, // 1 hour + max: 10, + delayMs: 0, + message: 'Whoa, slow down there, buddy! You just hit our rate limits. Try again in 1 hour.' +}) + router.use(wrap(async (req, res, next) => { let messages = req.flash('message') if (!messages || !messages.length) { @@ -30,11 +39,28 @@ router.use('/oauth2', oauthRouter) RENDER VIEWS ================ */ -router.get('/', wrap(async (req, res) => { +router.get('/', (req, res) => { res.render('index') -})) +}) -router.get('/login', wrap(async (req, res) => { +// Add social media login buttons +function extraButtons (req, res, next) { + if (config.twitter && config.twitter.api) { + res.locals.twitter_auth = true + } + + if (config.discord && config.discord.api) { + res.locals.discord_auth = true + } + + if (config.facebook && config.facebook.client) { + res.locals.facebook_auth = config.facebook.client + } + + next() +} + +router.get('/login', extraButtons, (req, res) => { if (req.session.user) { let uri = '/' if (req.session.redirectUri) { @@ -45,22 +71,10 @@ router.get('/login', wrap(async (req, res) => { return res.redirect(uri) } - if (config.twitter && config.twitter.api) { - res.locals.twitter_auth = true - } - - if (config.discord && config.discord.api) { - res.locals.discord_auth = true - } - - if (config.facebook && config.facebook.client) { - res.locals.facebook_auth = config.facebook.client - } - res.render('login') -})) +}) -router.get('/register', wrap(async (req, res) => { +router.get('/register', extraButtons, (req, res) => { if (req.session.user) return res.redirect('/') let dataSave = req.flash('formkeep') @@ -71,20 +85,13 @@ router.get('/register', wrap(async (req, res) => { } res.locals.formkeep = dataSave - if (config.twitter && config.twitter.api) { - res.locals.twitter_auth = true - } - if (config.discord && config.discord.api) { - res.locals.discord_auth = true - } - - if (config.facebook && config.facebook.client) { - res.locals.facebook_auth = config.facebook.client + if (config.security.recaptcha && config.security.recaptcha.site_key) { + res.locals.recaptcha = config.security.recaptcha.site_key } res.render('register') -})) +}) router.get('/user/two-factor', wrap(async (req, res) => { if (!req.session.user) return res.redirect('/login') @@ -106,9 +113,9 @@ router.get('/user/two-factor/disable', wrap(async (req, res) => { res.render('password') })) -router.get('/login/verify', wrap(async (req, res) => { +router.get('/login/verify', (req, res) => { res.render('totp-check') -})) +}) /* ================= @@ -117,7 +124,7 @@ router.get('/login/verify', wrap(async (req, res) => { */ function formError (req, res, error, redirect) { - // Security measures + // Security measures: never store any passwords in any session if (req.body.password) { delete req.body.password if (req.body.password_repeat) { @@ -254,7 +261,7 @@ router.post('/login', wrap(async (req, res) => { res.redirect(uri) })) -router.post('/register', wrap(async (req, res) => { +router.post('/register', accountLimiter, wrap(async (req, res) => { if (!req.body.username || !req.body.display_name || !req.body.password || !req.body.email) { return formError(req, res, 'Please fill in all the fields.') } @@ -293,10 +300,27 @@ router.post('/register', wrap(async (req, res) => { return formError(req, res, 'Passwords do not match!') } - // TODO: Add reCaptcha + // 6th Check: reCAPTCHA (if configuration contains key) + if (config.security.recaptcha && config.security.recaptcha.site_key) { + if (!req.body['g-recaptcha-response']) return formError(req, res, 'Please complete the reCAPTCHA!') + + try { + let data = await http.POST('https://www.google.com/recaptcha/api/siteverify', {}, { + secret: config.security.recaptcha.secret_key, + response: req.body['g-recaptcha-response'] + }) + + data = JSON.parse(data) + if (!data.success) { + return formError(req, res, 'Please complete the reCAPTCHA!') + } + } catch (e) { + console.error(e) + return formError(req, res, 'Internal server error') + } + } // Hash the password - let hash = await API.User.Register.hashPassword(password) // Attempt to create the user @@ -323,7 +347,7 @@ router.post('/register', wrap(async (req, res) => { */ const docsDir = path.join(__dirname, '../../documents') -router.get('/docs/:name', wrap(async (req, res) => { +router.get('/docs/:name', (req, res) => { let doc = path.join(docsDir, req.params.name + '.html') if (!fs.existsSync(docsDir) || !fs.existsSync(doc)) { return res.status(404).end() @@ -332,7 +356,7 @@ router.get('/docs/:name', wrap(async (req, res) => { doc = fs.readFileSync(doc, {encoding: 'utf8'}) res.render('document', {doc: doc}) -})) +}) /* ========= diff --git a/server/routes/oauth2.js b/server/routes/oauth2.js index d1fdada..a05f9a0 100644 --- a/server/routes/oauth2.js +++ b/server/routes/oauth2.js @@ -1,6 +1,7 @@ import express from 'express' import uapi from '../api' import OAuth2 from '../api/oauth2' +import RateLimit from 'express-rate-limit' import config from '../../scripts/load-config' import wrap from '../../scripts/asyncRoute' @@ -9,6 +10,14 @@ let oauth = new OAuth2() router.use(oauth.express()) +let oauthLimiter = new RateLimit({ + windowMs: 5 * 60 * 1000, // 5 minutes + max: 100, + delayMs: 0 +}) + +router.use(oauthLimiter) + function ensureLoggedIn (req, res, next) { if (req.session.user) { next() diff --git a/src/style/main.styl b/src/style/main.styl index 5ca2652..03fffe8 100644 --- a/src/style/main.styl +++ b/src/style/main.styl @@ -169,6 +169,9 @@ footer // display: block // left: 40% +.g-recaptcha + margin-top: 10px + ul.flexview position: fixed right: 0 diff --git a/views/register.pug b/views/register.pug index 2008aef..ffc738e 100644 --- a/views/register.pug +++ b/views/register.pug @@ -27,6 +27,9 @@ block body label(for="password_repeat") Repeat Password input(type="password", name="password_repeat", id="password_repeat") div#repeatcheck(style="display: none") + if recaptcha + script(src='https://www.google.com/recaptcha/api.js') + .g-recaptcha(data-sitekey=recaptcha) input(type="submit", value="Register") a#create(href="/login") Log in with an existing account .right diff --git a/views/totp.pug b/views/totp.pug index 12b04d8..c9b62d1 100644 --- a/views/totp.pug +++ b/views/totp.pug @@ -6,8 +6,8 @@ block body .wrapper .boxcont .box#login + h1 Two Factor Authentication .left - h1 Two Factor Authentication if message if message.error .message.error