From c12fde401d198b855dacb6c2a3c9709789e6c38e Mon Sep 17 00:00:00 2001 From: Alexey Date: Sat, 8 Feb 2020 17:00:16 +0300 Subject: [PATCH] csrf protection; validation middleware; morgan removing --- package.json | 1 - server/controllers/authorize.js | 36 +++++++++++++++++++-------------- server/index.js | 7 ------- yarn.lock | 20 +----------------- 4 files changed, 22 insertions(+), 42 deletions(-) diff --git a/package.json b/package.json index 8b61857..f088f47 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,6 @@ "dotenv": "^8.2.0", "express": "^4.16.4", "express-session": "^1.17.0", - "morgan": "^1.9.1", "multer": "^1.4.2", "mysql2": "^2.1.0", "nuxt": "^2.0.0", diff --git a/server/controllers/authorize.js b/server/controllers/authorize.js index 823ae6d..8189ca6 100644 --- a/server/controllers/authorize.js +++ b/server/controllers/authorize.js @@ -1,7 +1,8 @@ +/* eslint-disable no-console */ +const crypto = require('crypto') const express = require('express') const router = express.Router() const oauth = require('oauth') -const logger = require('morgan') const { TWITTER_CONSUMER_KEY, @@ -33,18 +34,26 @@ const github = new oauth.OAuth2( 'login/oauth/access_token' ) -router.get('/connect/:provider', (req, res) => { +function validateProvider(req, res, next) { const { provider } = req.params if (!providers.includes(provider)) { res.status(404).send('Wrong provider') + } else { + next() } +} + +router.get('/connect/:provider', validateProvider, (req, res) => { + const { provider } = req.params if (provider === 'github') { + const CSRFToken = crypto.randomBytes(32).toString('hex') const authorizeUrl = github.getAuthorizeUrl({ redirect_uri: GITHUB_CALLBACK_URL, scope: [], // 'gist' - https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ - state: 'some random string to protect against cross-site request forgery attacks' + state: CSRFToken }) + req.session.CSRFToken = CSRFToken res.redirect(authorizeUrl) } else if (provider === 'twitter') { twitter.getOAuthRequestToken(function(error, oauthToken, oauthTokenSecret) { @@ -61,21 +70,18 @@ router.get('/connect/:provider', (req, res) => { } }) -router.get('/oauth_callback/:provider', (req, res) => { +router.get('/oauth_callback/:provider', validateProvider, (req, res) => { const { provider } = req.params - if (!providers.includes(provider)) { - res.status(404).send('Wrong provider') - } if (provider === 'github') { - github.getOAuthAccessToken(req.query.code, {}, function( - error, - accessToken, - refreshToken, - results - ) { + const { code, state } = req.query + if (state !== req.session.CSRFToken) { + res.status(404).send('Malformed request') + return + } + github.getOAuthAccessToken(code, {}, function(error, accessToken, refreshToken, results) { if (error || results.error) { - logger.error('getOAuthAccessToken error', error || results.error) + console.error('getOAuthAccessToken error', error || results.error) res.status(500).send(error || results.error) } else { req.session.refreshToken = refreshToken @@ -90,7 +96,7 @@ router.get('/oauth_callback/:provider', (req, res) => { req.query.oauth_verifier, (error, oauthAccessToken, oauthAccessTokenSecret) => { if (error) { - logger.error(error) + console.error('getOAuthAccessToken error', error) res.status(500).send(error) } else { req.session.oauthAccessToken = oauthAccessToken diff --git a/server/index.js b/server/index.js index caaa9c5..19b794b 100644 --- a/server/index.js +++ b/server/index.js @@ -1,10 +1,8 @@ const path = require('path') const { NODE_ENV } = process.env require('dotenv').config({ path: path.join(__dirname, `../.env.${NODE_ENV}`) }) -const fs = require('fs') const express = require('express') const bodyParser = require('body-parser') -const morgan = require('morgan') const session = require('express-session') const { Nuxt, Builder } = require('nuxt') const config = require('../nuxt.config.js') @@ -46,11 +44,6 @@ async function start() { app.use(bodyParser.urlencoded({ extended: true })) app.use(bodyParser.json()) - const accessLogStream = fs.createWriteStream(path.join(__dirname, 'access.log'), { - flags: 'a' - }) - app.use(morgan('combined', { stream: accessLogStream })) - // Give nuxt middleware to express app.use(nuxt.render) diff --git a/yarn.lock b/yarn.lock index 39b79fc..0eb68c0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1755,13 +1755,6 @@ base@^0.11.1: mixin-deep "^1.2.0" pascalcase "^0.1.1" -basic-auth@~2.0.0: - version "2.0.1" - resolved "https://registry.yarnpkg.com/basic-auth/-/basic-auth-2.0.1.tgz#b998279bf47ce38344b4f3cf916d4679bbf51e3a" - integrity sha512-NF+epuEdnUYVlGuhaxbbq+dvJttwLnGY+YixlXlME5KpQ5W3CnXA5cVTneY3SPbPDRkcjMbifrwmFYcClgOZeg== - dependencies: - safe-buffer "5.1.2" - bcrypt-pbkdf@^1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/bcrypt-pbkdf/-/bcrypt-pbkdf-1.0.2.tgz#a4301d389b6a43f9b67ff3ca11a3f6637e360e9e" @@ -5811,17 +5804,6 @@ moment-timezone@^0.5.21: resolved "https://registry.yarnpkg.com/moment/-/moment-2.24.0.tgz#0d055d53f5052aa653c9f6eb68bb5d12bf5c2b5b" integrity sha512-bV7f+6l2QigeBBZSM/6yTNq4P2fNpSWj/0e7jQcy87A8e7o2nAfP/34/2ky5Vw4B9S446EtIhodAzkFCcR4dQg== -morgan@^1.9.1: - version "1.9.1" - resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.1.tgz#0a8d16734a1d9afbc824b99df87e738e58e2da59" - integrity sha512-HQStPIV4y3afTiCYVxirakhlCfGkI161c76kKFca7Fk1JusM//Qeo1ej2XaMniiNeaZklMVrh3vTtIzpzwbpmA== - dependencies: - basic-auth "~2.0.0" - debug "2.6.9" - depd "~1.1.2" - on-finished "~2.3.0" - on-headers "~1.0.1" - move-concurrently@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/move-concurrently/-/move-concurrently-1.0.1.tgz#be2c005fda32e0b29af1f05d7c4b33214c701f92" @@ -6317,7 +6299,7 @@ on-finished@^2.3.0, on-finished@~2.3.0: dependencies: ee-first "1.1.1" -on-headers@^1.0.2, on-headers@~1.0.1, on-headers@~1.0.2: +on-headers@^1.0.2, on-headers@~1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/on-headers/-/on-headers-1.0.2.tgz#772b0ae6aaa525c399e489adfad90c403eb3c28f" integrity sha512-pZAE+FJLoyITytdqK0U5s+FIpjN0JP3OzFi/u8Rx+EV5/W+JTWGXG8xFzevE7AjBfDqHv/8vL8qQsIhHnqRkrA==