From 096ce154dabc0073fd478f52e58ae42b2f1bb014 Mon Sep 17 00:00:00 2001 From: Fred Boniface Date: Thu, 6 Apr 2023 22:01:37 +0100 Subject: [PATCH] Add rate limiting and tidy up various code. Will need complete review long before merging to main. Signed-off-by: Fred Boniface --- app.js | 9 +++++++++ package-lock.json | 12 +++++++++++ package.json | 1 + src/middlewares/auth.middlewares.js | 2 +- src/services/mail.services.js | 2 +- src/services/registration.services.js | 29 +++++++++++++-------------- src/utils/auth.utils.js | 29 ++++++++++++--------------- src/utils/ldb.utils.js | 4 ++-- src/utils/log.utils.js | 4 +++- src/utils/sanitizer.utils.js | 4 ++-- 10 files changed, 58 insertions(+), 38 deletions(-) diff --git a/app.js b/app.js index 5c128bb..a744a97 100644 --- a/app.js +++ b/app.js @@ -12,6 +12,7 @@ const app = express(); // Middleware const compression = require('compression') +const rateLimit = require('express-rate-limit') const authenticate= require('./src/middlewares/auth.middlewares') // Internal Requires @@ -29,6 +30,13 @@ const regRtr = require('./src/routes/registration.routes'); // /auth end const srvListen = process.env.OWL_SRV_LISTEN || "0.0.0.0" const srvPort = process.env.OWL_SRV_PORT || 8460 +const limiter = rateLimit({ + windowMs: 15 * (60 * 1000), // 15 minutes + max: 100, // Limit each IP to 100 requests per `window` (here, per 15 minutes) + standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers + legacyHeaders: true, // Disable the `X-RateLimit-*` headers +}) + // Print version number: log.out(`app: Starting OwlBoard - Backend Version: ${version.app} - API versions: ${version.api}`, "init"); @@ -52,6 +60,7 @@ app.use((err, req, res, next) => { // Global Middleware: app.use(express.json()); //JSON Parsing for POST Requests app.use(compression()) // Compress API Data if supported by client +app.use(limiter) // Unauthenticated Routes app.use('/api/v1/list', listRtr); diff --git a/package-lock.json b/package-lock.json index 37d5ef9..47f9a94 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "axios": "^1.2.1", "compression": "^1.7.4", "express": "^4.18.2", + "express-rate-limit": "^6.7.0", "ldbs-json": "^1.2.1", "mongodb": "^4.13.0", "nodemailer": "^6.9.1", @@ -1426,6 +1427,17 @@ "node": ">= 0.10.0" } }, + "node_modules/express-rate-limit": { + "version": "6.7.0", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-6.7.0.tgz", + "integrity": "sha512-vhwIdRoqcYB/72TK3tRZI+0ttS8Ytrk24GfmsxDXK9o9IhHNO5bXRiXQSExPQ4GbaE5tvIS7j1SGrxsuWs+sGA==", + "engines": { + "node": ">= 12.9.0" + }, + "peerDependencies": { + "express": "^4 || ^5" + } + }, "node_modules/express/node_modules/safe-buffer": { "version": "5.2.1", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", diff --git a/package.json b/package.json index fb6934b..0d4e876 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,7 @@ "axios": "^1.2.1", "compression": "^1.7.4", "express": "^4.18.2", + "express-rate-limit": "^6.7.0", "ldbs-json": "^1.2.1", "mongodb": "^4.13.0", "nodemailer": "^6.9.1", diff --git a/src/middlewares/auth.middlewares.js b/src/middlewares/auth.middlewares.js index c740895..a97d0b0 100644 --- a/src/middlewares/auth.middlewares.js +++ b/src/middlewares/auth.middlewares.js @@ -11,7 +11,7 @@ module.exports = async function authCheck(req, res, next) { return next(err) } try { - var result = await utils.isAuthed(uuid) | false + var result = await utils.isAuthed(uuid) || false if (!result) { const err = new Error("Unauthorised"); err.status = 401 diff --git a/src/services/mail.services.js b/src/services/mail.services.js index e3fa3f6..0f30f3a 100644 --- a/src/services/mail.services.js +++ b/src/services/mail.services.js @@ -23,7 +23,7 @@ async function send(message){ // message is an object containing strings for: *t try { var res = await transporter.sendMail(message) } catch(err) { - log.out(`mailServices.send: Message send failed`, "err") + log.out(`mailServices.send: Message send failed: ${err}`, "err") return false; } log.out(`mailServices.send: SMTP Response: ${res.response}`) diff --git a/src/services/registration.services.js b/src/services/registration.services.js index 2e3813f..b47e301 100644 --- a/src/services/registration.services.js +++ b/src/services/registration.services.js @@ -5,17 +5,16 @@ const mail = require('./mail.services') const clean = require('../utils/sanitizer.utils') const domains = require('../configs/domains.configs') -async function createRegKey(body){ +async function createRegKey(body) { log.out(`registerServices.createRegKey: Incoming request`, "INFO") - log.out(`registerServices.createRegKey: ${JSON.stringify(domains)}`) - const domain = await clean.splitDomain(body.email) + const domain = await clean.getDomainFromEmail(body.email) // The function should validate the email log.out(`registerServices: New registration request from domain: ${domain}`, "info") - if (domains.includes(domain)) { // Needs testing + if (domains.includes(domain)) { log.out(`registerServices.createRegKey: Key from valid domain: ${domain}`) - const uuid = auth.generateKey(); - db.addRegReq(await uuid, domain) - const message = auth.generateConfirmationEmail(body.email, uuid) - if (await message == false) { // This error should be handled in the upstream function + const uuid = await auth.generateKey(); + db.addRegReq(uuid, domain) + const message = await auth.generateConfirmationEmail(body.email, uuid) + if (!message) { const err = new Error("Message generation error"); log.out(`registerServices.createRegKey: Error generating registration email`, "err") return 500; @@ -28,19 +27,19 @@ async function createRegKey(body){ return {status: 403, message: "forbidden, domain is not on whitelist"} } -async function regUser(req) { - log.out(`registrationServices.regUser: Checking validity of : ${req.uuid}`) - let res = await auth.checkRequest(req.uuid) - log.out(`registrationServices.regUser: checkRequest returned: ${JSON.stringify(res)}`) +async function regUser(req) { // Add input validation + log.out(`registrationServices.regUser: Checking validity of : ${req.uuid}`, "info") + const res = await auth.checkRequest(req.uuid) + log.out(`registrationServices.regUser: checkRequest returned: ${JSON.stringify(res)}`, "info") if (res.result) { - let uuid = await auth.generateKey() - let apiKey = await db.addUser(uuid, res.domain) + const uuid = await auth.generateKey() + const apiKey = await db.addUser(uuid, res.domain) if (apiKey) { db.delRegReq(req.uuid) return {status: 201, message: "User added", api_key: uuid} } } - return {status: 401, message: "Unautorised"} + return {status: 401, message: "Unauthorised"} } module.exports = { diff --git a/src/utils/auth.utils.js b/src/utils/auth.utils.js index d9eeb5e..0864137 100644 --- a/src/utils/auth.utils.js +++ b/src/utils/auth.utils.js @@ -5,27 +5,24 @@ const fs = require('fs/promises') // Checks users registration key against issued keys async function isAuthed(uuid) { // Needs testing - q = {uuid: uuid}; - res = await db.query("users", q); - log.out(`authUtils.checkUser: DB Query answer: ${JSON.stringify(res[0])}`) - // Do something here to determine if authorised or not and simply return a BOOL - if (res[0].domain) { - db.userAtime(uuid) - return true - } - return false + const q = {uuid: uuid} + const res = await db.query("users", q) + log.out(`authUtils.checkUser: DB Query answer: ${JSON.stringify(res[0])}`, "dbug") + const authorized = res && res[0] && res[0].domain; + if (authorized) db.userAtime(uuid) + return authorized } // Checks whether a registration request key is valid async function checkRequest(key) { - let collection = "registrations" - let query = {uuid: key} + const collection = "registrations" + const query = {uuid: key} const res = await db.query(collection, query) - log.out(`authUtils.checkRequest: DB Query result: ${JSON.stringify(res)}`, "info") - if (res[0].time) { - return {result: true, domain: res[0].domain} - } - return {result: false} + log.out(`authUtils.checkRequest: DB Query result: ${JSON.stringify(res)}`, "dbug") + const result = res.length > 0 && res[0].time + ? { result: true, domain: res[0].domain } + : { result: false }; + return result; } // Creates an API key for a user diff --git a/src/utils/ldb.utils.js b/src/utils/ldb.utils.js index a99b74d..c6c25e4 100644 --- a/src/utils/ldb.utils.js +++ b/src/utils/ldb.utils.js @@ -4,10 +4,10 @@ const san = require('../utils/sanitizer.utils') // Sanitiser async function checkCrs(input){ var INPUT = input.toUpperCase() - log.out(`ldbUtils.checkCrs: Building database query to find: '${INPUT}'`, "info") + log.out(`ldbUtils.checkCrs: Building database query to find: '${INPUT}'`, "dbug") var query = {'$or':[{'3ALPHA':INPUT},{'TIPLOC':INPUT},{'STANOX':INPUT}]}; var result = await db.query("stations", query) - log.out(`ldbUtils.checkCrs: Query results: ${JSON.stringify(result)}`, "info") + log.out(`ldbUtils.checkCrs: Query results: ${JSON.stringify(result)}`, "dbug") return result } diff --git a/src/utils/log.utils.js b/src/utils/log.utils.js index db0b3f0..5271bf4 100644 --- a/src/utils/log.utils.js +++ b/src/utils/log.utils.js @@ -1,7 +1,9 @@ const environment = process.env.NODE_ENV; +const hideInProduction = ["info", "dbug"] + async function out(msg, level = 'othr') { - if (environment === "production" && level === "info") { + if (environment === "production" && hideInProduction.includes(level)) { return; } else { const time = new Date().toISOString(); diff --git a/src/utils/sanitizer.utils.js b/src/utils/sanitizer.utils.js index c1fdbaa..e0e6e7e 100644 --- a/src/utils/sanitizer.utils.js +++ b/src/utils/sanitizer.utils.js @@ -38,7 +38,7 @@ function cleanNrcc(input) { return rmPara; } -async function splitDomain(mail) { // Needs testing +async function getDomainFromEmail(mail) { // Needs testing split = mail.split("@") return split[1] } @@ -47,5 +47,5 @@ module.exports = { cleanApiEndpointTxt, cleanApiEndpointNum, cleanNrcc, - splitDomain + getDomainFromEmail } \ No newline at end of file