Skip to content

Commit

Permalink
fix(refactor): merging write-api auth middlewares with core middlewares
Browse files Browse the repository at this point in the history
  • Loading branch information
@julianlam
julianlam committed Oct 8, 2020
1 parent ec5c48b commit f6433ef
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 100 deletions.
71 changes: 33 additions & 38 deletions src/controllers/helpers.js
View file
Expand Up @@ -123,12 +123,7 @@ helpers.notAllowed = async function (req, res, error) {

if (req.loggedIn || req.uid === -1) {
if (res.locals.isAPI) {
res.status(403).json({
path: req.path.replace(/^\/api/, ''),
loggedIn: req.loggedIn,
error: data.error,
title: '[[global:403.title]]',
});
helpers.formatApiResponse(403, res, error);
} else {
await middleware.buildHeaderAsync(req, res);
res.status(403).render('403', {
Expand All @@ -140,7 +135,7 @@ helpers.notAllowed = async function (req, res, error) {
}
} else if (res.locals.isAPI) {
req.session.returnTo = req.url.replace(/^\/api/, '');
res.status(401).json('not-authorized');
helpers.formatApiResponse(401, res, error);
} else {
req.session.returnTo = req.url;
res.redirect(nconf.get('relative_path') + '/login');
Expand Down Expand Up @@ -353,16 +348,16 @@ helpers.formatApiResponse = async (statusCode, res, payload) => {
},
response: payload || {},
});
} else if (!payload) {
// Non-2xx statusCode, generate predefined error
res.status(statusCode).json(helpers.generateError(statusCode));
} else if (payload instanceof Error) {
if (isLanguageKey.test(payload.message)) {
const translated = await translator.translate(payload.message, 'en-GB');
res.status(statusCode).json(helpers.generateError(statusCode, translated));
} else {
res.status(statusCode).json(helpers.generateError(statusCode, payload.message));
}
} else if (!payload) {
// Non-2xx statusCode, generate predefined error
res.status(statusCode).json(helpers.generateError(statusCode));
}
};

Expand All @@ -377,34 +372,34 @@ helpers.generateError = (statusCode, message) => {

// Need to turn all these into translation strings
switch (statusCode) {
case 400:
payload.status.code = 'bad-request';
payload.status.message = message || 'Something was wrong with the request payload you passed in.';
break;

case 401:
payload.status.code = 'not-authorised';
payload.status.message = 'A valid login session was not found. Please log in and try again.';
break;

case 403:
payload.status.code = 'forbidden';
payload.status.message = 'You are not authorised to make this call';
break;

case 404:
payload.status.code = 'not-found';
payload.status.message = 'Invalid API call';
break;

case 426:
payload.status.code = 'upgrade-required';
payload.status.message = 'HTTPS is required for requests to the write api, please re-send your request via HTTPS';
break;

case 500:
payload.status.code = 'internal-server-error';
payload.status.message = message || payload.status.message;
case 400:
payload.status.code = 'bad-request';
payload.status.message = message || 'Something was wrong with the request payload you passed in.';
break;

case 401:
payload.status.code = 'not-authorised';
payload.status.message = message || 'A valid login session was not found. Please log in and try again.';
break;

case 403:
payload.status.code = 'forbidden';
payload.status.message = message || 'You are not authorised to make this call';
break;

case 404:
payload.status.code = 'not-found';
payload.status.message = message || 'Invalid API call';
break;

case 426:
payload.status.code = 'upgrade-required';
payload.status.message = message || 'HTTPS is required for requests to the write api, please re-send your request via HTTPS';
break;

case 500:
payload.status.code = 'internal-server-error';
payload.status.message = message || payload.status.message;
}

return payload;
Expand Down
48 changes: 0 additions & 48 deletions src/middleware/api.js
View file

This file was deleted.

41 changes: 40 additions & 1 deletion src/middleware/user.js
View file
@@ -1,6 +1,8 @@
'use strict';

const nconf = require('nconf');
const winston = require('winston');
const passport = require('passport');

const meta = require('../meta');
const user = require('../user');
Expand All @@ -11,12 +13,49 @@ const auth = require('../routes/authentication');

const controllers = {
helpers: require('../controllers/helpers'),
authentication: require('../controllers/authentication'),
};

module.exports = function (middleware) {
async function authenticate(req, res) {
if (req.loggedIn) {
return true;
} else if (req.headers.hasOwnProperty('authorization')) {
passport.authenticate('bearer', { session: false }, function (err, user) {
if (err) { throw new Error(err); }
if (!user) { return false; }

// If the token received was a master token, a _uid must also be present for all calls
if (user.hasOwnProperty('uid')) {
req.login(user, async function (err) {
if (err) { throw new Error(err); }

await controllers.authentication.onSuccessfulLogin(req, user.uid);
req.uid = user.uid;
req.loggedIn = req.uid > 0;
return true;
});
} else if (user.hasOwnProperty('master') && user.master === true) {
if (req.body.hasOwnProperty('_uid') || req.query.hasOwnProperty('_uid')) {
user.uid = req.body._uid || req.query._uid;
delete user.master;

req.login(user, async function (err) {
if (err) { throw new Error(err); }

await controllers.authentication.onSuccessfulLogin(req, user.uid);
req.uid = user.uid;
req.loggedIn = req.uid > 0;
return true;
});
} else {
throw new Error('A master token was received without a corresponding `_uid` in the request body');
}
} else {
winston.warn('[api/authenticate] Unable to find user after verifying token');
return false;
}
})(req, res);
}

await plugins.fireHook('response:middleware.authenticate', {
Expand Down Expand Up @@ -180,7 +219,7 @@ module.exports = function (middleware) {
req.session.returnTo = returnTo;
req.session.forceLogin = 1;
if (res.locals.isAPI) {
res.status(401).json({});
controllers.helpers.formatApiResponse(401, res);
} else {
res.redirect(nconf.get('relative_path') + '/login?local=1');
}
Expand Down
2 changes: 1 addition & 1 deletion src/routes/index.js
View file
Expand Up @@ -112,7 +112,7 @@ module.exports = async function (app, middleware) {

await plugins.reloadRoutes({ router: router });
await authRoutes.reloadRoutes({ router: router });
await writeRoutes.reload({ router: router });
writeRoutes.reload({ router: router });
addCoreRoutes(app, router, middleware);

winston.info('Routes added');
Expand Down
27 changes: 15 additions & 12 deletions src/routes/write/index.js
View file
@@ -1,36 +1,39 @@
'use strict';

const middleware = require('../../middleware/api');
const middleware = require('../../middleware');
const helpers = require('../../controllers/helpers');

const Write = module.exports;

Write.reload = (params) => {
const router = params.router;

// router.use('/api', function (req, res, next) {
// if (req.protocol !== 'https') {
// res.set('Upgrade', 'TLS/1.0, HTTP/1.1');
// return helpers.formatApiResponse(426, res);
// } else {
// next();
// }
// });
router.use('/api/v1', function (req, res, next) {
// if (req.protocol !== 'https') {
// res.set('Upgrade', 'TLS/1.0, HTTP/1.1');
// return helpers.formatApiResponse(426, res);
// } else {
// next();
// }

res.locals.isAPI = true;
next();
});

// router.use('/users', require('./users')(coreMiddleware));
router.use('/api/v1/users', require('./users')());
// router.use('/groups', require('./groups')(coreMiddleware));
// router.use('/posts', require('./posts')(coreMiddleware));
// router.use('/topics', require('./topics')(coreMiddleware));
// router.use('/categories', require('./categories')(coreMiddleware));
// router.use('/util', require('./util')(coreMiddleware));

router.get('/api/ping', function (req, res) {
router.get('/api/v1/ping', function (req, res) {
helpers.formatApiResponse(200, res, {
pong: true,
});
});

router.post('/api/ping', middleware.authenticate, function (req, res) {
router.post('/api/v1/ping', middleware.authenticate, function (req, res) {
helpers.formatApiResponse(200, res, {
uid: req.user.uid,
});
Expand Down

0 comments on commit f6433ef

Please sign in to comment.