Rename and rework auth middleware

app/routes/api/auth.js

@@ -11,12 +11,12 @@ const User = require(ModelPath + 'User.js');
 const Invite = require(ModelPath + 'Invite.js');
 
 
-const requireAuth = require('../../util/auth').requireAuth;
+const authenticate = require('../../util/auth/authenticateRequest');
 const verifyBody = require('../../util/verifyBody');
 const rateLimit = require('../../util/rateLimit');
 
 // Wraps passport.authenticate to return a promise
-const authenticate = (req, res, next) => {
+const passportAuthenticate = (req, res, next) => {
     return new Promise((resolve) => {
         passport.authenticate('local', (err, user) => {
             resolve(user);
@@ -25,7 +25,7 @@ const authenticate = (req, res, next) => {
 };
 
 // Wraps passport session creation to return a promise
-const login = (user, req) => {
+const passportLogin = (user, req) => {
     return new Promise((resolve) => {
         req.login(user, resolve);
     });
@@ -87,7 +87,7 @@ router.post('/login',
         req.body.username = canonicalize(req.body.displayname);
 
         // Authenticate
-        const user = await authenticate(req, res, next);
+        const user = await passportAuthenticate(req, res, next);
         if (!user) {
             // Log failure
             await fs.appendFile('auth.log', `${new Date().toISOString()} login ${req.ip}\n`);
@@ -95,7 +95,7 @@ router.post('/login',
         }
 
         // Create session
-        await login(user, req);
+        await passportLogin(user, req);
 
         // Set session vars
         req.session.passport.displayname = user.displayname;
@@ -116,7 +116,7 @@ router.post('/logout', (req, res) => {
 
 
 
-router.get('/whoami', requireAuth(), (req, res) => {
+router.get('/whoami', authenticate(), (req, res) => {
     res.status(200).json({
         username: req.username,
         displayname: req.displayname,

app/routes/api/invites.js

@@ -6,19 +6,22 @@ const ModelPath = '../../models/';
 const Invite = require(ModelPath + 'Invite.js');
 const User = require(ModelPath + 'User.js');
 
-const requireAuth = require('../../util/auth').requireAuth;
-const verifyScope = require('../../util/verifyScope');
+const authenticate = require('../../util/auth/authenticateRequest');
 const verifyBody = require('../../util/verifyBody');
 
+
+
 const createParams = [{name: 'scope', instance: Array}];
-router.post('/create', requireAuth('invite.create'), verifyBody(createParams), async (req, res, next) => {
-    const scope = req.body.scope;
-    if (!scope.every(scope => verifyScope(req.scope, scope)))
+
+router.post('/create', authenticate('invite.create'), verifyBody(createParams), async (req, res, next) => {
+    // Make sure the user has all the request scope
+    const inviteScope = req.body.scope;
+    if (!inviteScope.every(s => req.scope.includes(s)))
         return res.status(403).json({message: 'Requested scope exceeds own scope.'});
 
     const invite = {
         code: crypto.randomBytes(12).toString('hex'),
-        scope: scope,
+        scope: inviteScope,
         issuer: req.username,
         issued: Date.now(),
         expires: req.body.expires
@@ -35,12 +38,15 @@ router.post('/create', requireAuth('invite.create'), verifyBody(createParams), a
     });
 });
 
+
+
 const deleteParams = [{name: 'code', type: 'string'}];
-router.post('/delete', requireAuth('invite.delete'), verifyBody(deleteParams), async (req, res, next) => {
+
+router.post('/delete', authenticate('invite.delete'), verifyBody(deleteParams), async (req, res, next) => {
     let query = {code: req.body.code};
 
     // Users need a permission to delete invites other than their own
-    if (!verifyScope(req.scope, 'invite.delete.others'))
+    if (!req.scope.includes('invite.delete.others'))
         query.issuer = req.username;
 
     // Find the invite
@@ -49,19 +55,24 @@ router.post('/delete', requireAuth('invite.delete'), verifyBody(deleteParams), a
         return res.status(422).json({message: 'Invite not found.'});
 
     // Users need a permission to delete invites that have been used
-    if (!verifyScope(req.scope, 'invite.delete.used') && invite.used != null && invite.recipient != null)
+    if (!req.scope.includes('invite.delete.used') && invite.used != null && invite.recipient != null)
         return res.status(403).json({message: 'Forbidden to delete used invites.'});
 
     await Invite.deleteOne({_id: invite._id}).catch(next);
     res.status(200).json({message: 'Invite deleted.'});
 });
 
-const getParams = [{name: 'code', type: 'string', optional: true}, {name: 'issuer', type: 'string', optional: true}];
-router.get('/get', requireAuth('invite.get'), verifyBody(getParams), async (req, res, next) => {
+
+
+const getParams = [
+    {name: 'code', type: 'string', optional: true},
+    {name: 'issuer', type: 'string', optional: true}];
+
+router.get('/get', authenticate('invite.get'), verifyBody(getParams), async (req, res, next) => {
     let query = {};
 
     // Users need a permission to list invites other than their own
-    if (!verifyScope(req.scope, 'invite.get.others'))
+    if (!req.scope.includes('invite.get.others'))
         query.issuer = req.username;
     else if (req.body.issuer)
         query.issuer = req.body.issuer;
@@ -74,4 +85,6 @@ router.get('/get', requireAuth('invite.get'), verifyBody(getParams), async (req,
     res.status(200).json(invites);
 });
 
+
+
 module.exports = router;

app/routes/api/keys.js

@@ -7,25 +7,28 @@ const ModelPath = '../../models/';
 const Key = require(ModelPath + 'Key.js');
 
 const verifyBody = require('../../util/verifyBody');
-const verifyScope = require('../../util/verifyScope');
-const requireAuth = require('../../util/auth').requireAuth;
+const authenticate = require('../../util/auth/authenticateRequest');
+
+
 
 const createParams = [
     {name: 'identifier', type: 'string', sanitize: true},
     {name: 'scope', instance: Array}];
-router.post('/create', requireAuth('key.create'), verifyBody(createParams), async (req, res) => {
+
+router.post('/create', authenticate('key.create'), verifyBody(createParams), async (req, res) => {
     const keyCount = await Key.countDocuments({issuer: req.username});
     if (keyCount >= config.get('Key.limit'))
         return res.status(403).json({message: 'Key limit reached.'});
 
-    const scope = req.body.scope;
-    if (!scope.every(scope => verifyScope(req.scope, scope)))
+    // Make sure the user has all the request scope
+    const keyScope = req.body.scope;
+    if (!keyScope.every(s => req.scope.includes(s)))
         return res.status(403).json({message: 'Requested scope exceeds own scope.'});
 
     const key = {
         key: await crypto.randomBytes(32).toString('hex'),
         identifier: req.body.identifier,
-        scope: scope,
+        scope: keyScope,
         issuer: req.username,
         date: Date.now()
     };
@@ -38,16 +41,19 @@ router.post('/create', requireAuth('key.create'), verifyBody(createParams), asyn
     });
 });
 
+
+
 const getProps = [
     {name: 'identifier', type: 'string', optional: true},
     {name: 'issuer', type: 'string', optional: true}];
-router.get('/get', requireAuth('key.get'), verifyBody(getProps), async (req, res) => {
+
+router.get('/get', authenticate('key.get'), verifyBody(getProps), async (req, res) => {
     let query = {};
 
     if (req.body.identifier)
         query.identifier = req.body.identifier;
 
-    if (!verifyScope(req.scope, 'key.get.others'))
+    if (!req.scope.includes('key.get.others'))
         query.issuer = req.username;
     else if (req.body.issuer)
         query.issuer = req.body.issuer;
@@ -57,13 +63,16 @@ router.get('/get', requireAuth('key.get'), verifyBody(getProps), async (req, res
     res.status(200).json(keys);
 });
 
+
+
 const deleteProps = [
-    {name: 'key', type: 'string'},
+    {name: 'keyid', type: 'string'},
     {name: 'issuer', type: 'string', optional: true}];
-router.post('/delete', requireAuth('key.delete'), verifyBody(deleteProps), async (req, res) => {
-    let query = {key : req.body.key};
 
-    if (!verifyScope(req.scope, 'key.delete.others'))
+router.post('/delete', authenticate('key.delete'), verifyBody(deleteProps), async (req, res) => {
+    let query = {key : req.body.keyid};
+
+    if (!req.scope.includes('key.delete.others'))
         query.issuer = req.username;
     else if (req.body.issuer)
         query.issuer = req.body.issuer;
@@ -76,4 +85,6 @@ router.post('/delete', requireAuth('key.delete'), verifyBody(deleteProps), async
     res.status(200).json({message: 'Key deleted.'});
 });
 
+
+
 module.exports = router;

app/routes/api/stats.js

@@ -6,7 +6,7 @@ const Upload = require(ModelPath + 'Upload.js');
 const View = require(ModelPath + 'View.js');
 
 const verifyBody = require('../../util/verifyBody');
-const requireAuth = require('../../util/auth').requireAuth;
+const authenticate = require('../../util/auth/authenticateRequest');
 
 const uploadProps = [
     {name: 'after', type: 'date', optional: true},
@@ -14,7 +14,7 @@ const uploadProps = [
     {name: 'limit', type: 'number', min: 1, max: 10000, optional: true}
 ];
 
-router.get('/uploads', requireAuth('stats.get'), verifyBody(uploadProps), async (req, res) => {
+router.get('/uploads', authenticate('stats.get'), verifyBody(uploadProps), async (req, res) => {
     let constraints = {uploader: req.username};
 
     // Set date constraints if specified
@@ -55,7 +55,7 @@ const viewProps = [
     {name: 'limit', type: 'number', min: 1, max: 10000, optional: true}
 ];
 
-router.get('/views', requireAuth('stats.get'), verifyBody(viewProps), async (req, res) => {
+router.get('/views', authenticate('stats.get'), verifyBody(viewProps), async (req, res) => {
     let constraints = {uploader: req.username};
 
     // Set date constraints if specified

app/routes/api/users.js

@@ -5,12 +5,12 @@ const ModelPath = '../../models/';
 const User = require(ModelPath + 'User.js');
 
 const verifyBody = require('../../util/verifyBody');
-const requireAuth = require('../../util/auth').requireAuth;
+const authenticate = require('../../util/auth/authenticateRequest');
 
 const getParams = [
     {name: 'username', type: 'string', optional: true},
     {name: 'displayname', type: 'string', optional: true}];
-router.get('/get', requireAuth('user.get'), verifyBody(getParams), async (req, res) => {
+router.get('/get', authenticate('user.get'), verifyBody(getParams), async (req, res) => {
     let query = {};
 
     if (req.body.username)
@@ -26,7 +26,7 @@ router.get('/get', requireAuth('user.get'), verifyBody(getParams), async (req, r
 });
 
 const banParams = [{name: 'username', type: 'string'}];
-router.post('/ban', requireAuth('user.ban'), verifyBody(banParams), async (req, res) => {
+router.post('/ban', authenticate('user.ban'), verifyBody(banParams), async (req, res) => {
     const user = await User.findOne({username: req.body.username});
     if (!user)
         return res.status(422).json({message: 'User not found.'});
@@ -41,7 +41,7 @@ router.post('/ban', requireAuth('user.ban'), verifyBody(banParams), async (req,
 });
 
 const unbanParams = [{name: 'username', type: 'string'}];
-router.post('/unban', requireAuth('user.unban'), verifyBody(unbanParams), async (req, res) => {
+router.post('/unban', authenticate('user.unban'), verifyBody(unbanParams), async (req, res) => {
     const user = await User.findOne({username: req.body.username});
     if (!user)
         return res.status(422).json({message: 'User not found.'});

app/routes/home.js

@@ -1,9 +1,9 @@
 const express = require('express');
 const router = express.Router();
 const path = require('path');
-const requireAuth = require('../util/auth').requireAuth;
+const authenticate = require('../util/auth/authenticateRequest');
 
-router.get('/', requireAuth(), function(req, res) {
+router.get('/', authenticate(), function(req, res) {
     res.sendFile(path.join(__dirname, '../../public/views', 'home.html'));
 });
 

app/routes/panel.js

@@ -1,9 +1,9 @@
 const express = require('express');
 const router = express.Router();
 const path = require('path');
-const requireAuth = require('../util/auth').requireAuth;
+const authenticate = require('../util/auth/authenticateRequest');
 
-router.get('/', requireAuth(), function(req, res) {
+router.get('/', authenticate(), function(req, res) {
     res.sendFile(path.join(__dirname, '../../public/views', 'panel.html'));
 });
 

app/util/auth.js

@@ -1,85 +0,0 @@
-const fs = require('fs').promises;
-const config = require('config');
-
-const ModelPath = '../models/';
-const Key = require(ModelPath + 'Key.js');
-const User = require(ModelPath + 'User.js');
-
-const verifyScope = require('./verifyScope.js');
-const rateLimit = require('express-rate-limit');
-
-const checkSession = (req, scope, status) => {
-    if (req.isAuthenticated()) {
-        status.authenticated = true;
-        if (!scope || verifyScope(req.session.passport.scope, scope)) {
-            req.username = req.session.passport.user;
-            req.displayname = req.session.passport.displayname;
-            req.scope = req.session.passport.scope;
-            req.key = null;
-            status.permission = true;
-        }
-    }
-};
-
-const checkKey = async (req, scope, status) => {
-    if (req.body.key) {
-        const key = await Key.findOne({key: req.body.key});
-        if (key) {
-            status.authenticated = true;
-            if (!scope || verifyScope(key.scope, scope)) {
-                req.username = key.issuer;
-                req.displayname = key.issuer;
-                req.scope = key.scope;
-                req.key = key.key;
-                status.permission = true;
-            }
-        } else {
-            // Log failure
-            await fs.appendFile('auth.log', `${new Date().toISOString()} key ${req.ip}\n`);
-        }
-    }
-};
-
-
-const apiLimiter = config.get('RateLimit.enable')
-    ? rateLimit({
-        windowMs: config.get('RateLimit.api.window') * 1000,
-        max: config.get('RateLimit.api.max'),
-        skip: (req, res) => res.statusCode !== 401 && res.statusCode !== 403
-    })
-    : (req, res, next) => { next(); };
-// Middleware that checks for authentication by either API key or session
-// sets req.username, req.displayname, req.scope, and req.key if authenticated properly,
-// otherwise throws an error code.
-// If the user is banned, also throw an error.
-const requireAuth = scope => (req, res, next) => {
-    apiLimiter(req, res, async () => {
-
-        const status = {
-            authenticated: false,
-            permission: false
-        };
-
-        // First, check the session
-        checkSession(req, scope, status);
-        // If not authenticated yet, check for a key
-        if (!status.authenticated)
-            await checkKey(req, scope, status);
-
-        if (!status.authenticated)
-            return res.status(401).json({message: 'Unauthorized.'});
-        else if (!status.permission)
-            return res.status(403).json({message: 'Forbidden.'});
-
-        // Check if the user is banned
-        const user = await User.findOne({username: req.username});
-        if (user && user.banned)
-            return res.status(403).json({message: 'Forbidden.'});
-
-        next();
-    });
-};
-
-module.exports.checkSession = checkSession;
-module.exports.checkKey = checkKey;
-module.exports.requireAuth = requireAuth;

app/util/auth/authenticate.js

@@ -0,0 +1,35 @@
+const ModelPath = '../../models/';
+const Key = require(ModelPath + 'Key.js');
+const User = require(ModelPath + 'User.js');
+
+// Middleware that checks for authentication by either API key or session
+// sets req.username, req.displayname, req.scope, and req.key if authenticated properly, otherwise throws an error.
+// If the user is banned, also throw an error.
+const authenticate = async (req, scope) => {
+    const keyprop = req.body.key || req.query.key;
+    let key = keyprop ? (await Key.findOne({key: keyprop})) : false;
+
+    if (key) {
+        if (!scope || key.scope.includes(scope)) {
+            if ((await User.countDocuments({username: key.issuer, banned: true})) === 0) {
+                req.username = key.issuer;
+                req.displayname = key.issuer;
+                req.scope = key.scope;
+                req.key = key.key;
+                return {authenticated: true, permission: true};
+            } else return {authenticated: true, permission: false};
+        } else return {authenticated: true, permission: false};
+    } else if (req.isAuthenticated()) {
+        if (!scope || req.session.passport.scope.includes(scope)) {
+            if ((await User.countDocuments({username: req.session.passport.user, banned: true})) === 0) {
+                req.username = req.session.passport.user;
+                req.displayname = req.session.passport.displayname;
+                req.scope = req.session.passport.scope;
+                req.key = null;
+                return {authenticated: true, permission: true};
+            } else return {authenticated: true, permission: false};
+        } else return {authenticated: true, permission: false};
+    } else return {authenticated: false, permission: false};
+};
+
+module.exports = authenticate;

app/util/auth/authenticateRequest.js

@@ -0,0 +1,16 @@
+const config = require('config');
+const authenticate = require('./authenticate');
+const rateLimit = require('../rateLimit');
+
+const authenticateRequest = scope => (req, res, next) => {
+    rateLimit(config.get('RateLimit.api.window'), config.get('RateLimit.api.max'))(req, res, async () => {
+        const status = await authenticate(req, scope);
+        if (status.authenticated) {
+            if (status.permission) {
+                next();
+            } else res.status(403).json({message: 'Forbidden.'});
+        } else res.status(401).json({message: 'Unauthorized.'});
+    });
+};
+
+module.exports = authenticateRequest;

app/util/rateLimit.js

@@ -1,9 +1,12 @@
 const config = require('config');
 const rateLimit = require('express-rate-limit');
 
-const rateLimitRequest = (window, max, skipSuccessful) =>
+const defaultSkipFn = (req, res) =>
+    res.statusCode !== 401 && res.statusCode !== 403 && res.statusCode !== 422;
+
+const rateLimitRequest = (window, max, skipFn) =>
     config.get('RateLimit.enable')
-        ? rateLimit({windowMs: window * 1000, max: max, skipSuccessfulRequests: skipSuccessful})
+        ? rateLimit({windowMs: window * 1000, max: max, skip: skipFn || defaultSkipFn})
         : (req, res, next) => { next(); };
 
 module.exports = rateLimitRequest;

app/util/upload/multipart.js

@@ -2,7 +2,7 @@ const Busboy = require('busboy');
 const is = require('type-is');
 const config = require('config');
 
-const auth = require('../auth');
+const authenticate = require('../auth/authenticate');
 const disk = require('./disk');
 const identifier = require('./id');
 
@@ -11,12 +11,8 @@ const uploadMultipart = async (req, res, next) => {
         return res.status(400).json({message: 'Bad request.'});
 
     // Store whether the user has authenticated, because an api key might be included with the form later
-    let authStatus = {
-        authenticated: false,
-        permission: false
-    };
     // If not authenticated with a session, we'll have to wait for key authentication from the multipart form data
-    await auth.checkSession(req, 'file.upload', authStatus);
+    let status = await authenticate(req, 'file.upload');
 
     // Function to call once the file is sent or an error is encountered
     let isDone = false;
@@ -65,13 +61,13 @@ const uploadMultipart = async (req, res, next) => {
         fileReceived = true;
 
         // If a key was encountered and we are not authenticated, try to authenticate with it before the final check
-        if (req.body.key && !authStatus.authenticated)
-            await auth.checkKey(req, 'file.upload', authStatus);
+        if (req.body.key && !status.authenticated)
+            status = await authenticate(req, 'file.upload', status);
 
         // Finally, check if we have auth before preceeding, keys should have been processed by now
-        if (!authStatus.authenticated)
+        if (!status.authenticated)
             return res.status(401).json({message: 'Unauthorized.'});
-        if (!authStatus.permission)
+        if (!status.permission)
             return res.status(403).json({message: 'Forbidden.'});
 
         // Don't attach to the files object if there is no file

app/util/verifyScope.js

@@ -1,3 +0,0 @@
-const verifyScope = (scope, requiredScope) => scope.indexOf(requiredScope) !== -1;
-
-module.exports = verifyScope;

test/testUtil.js

@@ -70,7 +70,7 @@ exports.registerUser = (user, agent) =>
 
 exports.whoami = (agent, key) =>
     agent.get('/api/auth/whoami')
-        .send({key: key});
+        .query({key: key});
 
 //---------------- TEST ENTRY CREATION ----------------//
 
@@ -173,7 +173,7 @@ exports.createKey = (key, agent) =>
 
 exports.deleteKey = (key, agent) =>
     agent.post('/api/keys/delete')
-        .send({key: key});
+        .send({keyid: key});
 
 exports.getKeys = (query, agent) =>
     agent.get('/api/keys/get')