diff --git a/package.json b/package.json index 9cfd21a5..70591b1d 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "request": "2.88.2", "session-file-store": "^1.5.0", "showdown": "^2.1.0", + "supertest-session": "^5.0.1", "url-slug": "^4.0.1" }, "devDependencies": { diff --git a/routes/middleware.js b/routes/middleware.js index 1721a403..16c4bab7 100755 --- a/routes/middleware.js +++ b/routes/middleware.js @@ -34,3 +34,21 @@ exports.username = function(req, res, next) { next(); }; + +exports.isAuthenticated = (redirectUrlAfterLogin = null, isApiRequest = false) => { + return (req, res, next) => { + if (req.isAuthenticated()) { + return next() + } + + if (req.xhr || req.headers?.accept?.indexOf('json') > -1 || isApiRequest) { + return res.status(401).json({error: 'Unauthorized'}) + } + + if (req.session) { + req.session.returnTo = redirectUrlAfterLogin || req.originalUrl + } + + return res.redirect('/login') + } +} diff --git a/routes/views/auth.js b/routes/views/auth.js index 9957ce14..75a7f1bf 100644 --- a/routes/views/auth.js +++ b/routes/views/auth.js @@ -42,8 +42,15 @@ router.get('/login', passport.authenticate('faforever')); router.get( '/' + appConfig.oauth.callback, + (req, res, next)=>{ + res.locals.returnTo = req.session.returnTo + + return next() + }, passport.authenticate('faforever', {failureRedirect: '/login', failureFlash: true}), - (req, res) => res.redirect('/') + (req, res) => { + res.redirect(res.locals.returnTo || '/') + } ) router.get('/logout', (req, res, next) => { diff --git a/routes/views/leaderboardRouter.js b/routes/views/leaderboardRouter.js index c2f5c2bf..ec5c17ec 100644 --- a/routes/views/leaderboardRouter.js +++ b/routes/views/leaderboardRouter.js @@ -1,8 +1,9 @@ +const appConfig = require('../../config/app') const express = require('express'); const router = express.Router(); const LeaderboardServiceFactory = require('../../lib/LeaderboardServiceFactory') const {AcquireTimeoutError} = require('../../lib/MutexService'); -const appConfig = require('../../config/app') +const middlewares = require('../middleware') const getLeaderboardId = (leaderboardName) => { @@ -20,19 +21,11 @@ const getLeaderboardId = (leaderboardName) => { return null } -router.get('/', (req, res) => { - if (!req.isAuthenticated()) { - return res.redirect('/login'); - } - +router.get('/', middlewares.isAuthenticated(), (req, res) => { return res.render('leaderboards') }) -router.get('/:leaderboard.json', async (req, res) => { - if (!req.isAuthenticated()) { - return res.status(403).json({error: 'Not Authenticated'}) - } - +router.get('/:leaderboard.json', middlewares.isAuthenticated(null, true), async (req, res) => { try { const leaderboardId = getLeaderboardId(req.params.leaderboard ?? null); diff --git a/tests/integration/IsAuthenticatedMiddleware.test.js b/tests/integration/IsAuthenticatedMiddleware.test.js new file mode 100644 index 00000000..ce72ac66 --- /dev/null +++ b/tests/integration/IsAuthenticatedMiddleware.test.js @@ -0,0 +1,96 @@ +const express = require('express') +const middlewares = require('../../routes/middleware') +const passport = require("passport"); +const appConfig = require("../../config/app"); +const supertestSession = require('supertest-session'); +const session = require('express-session'); + +let testApp = null +let testSession = null + +beforeEach(() => { + const app = new express() + testSession = supertestSession(app) + app.use(session({ + resave: false, + saveUninitialized: true, + secret: appConfig.session.key, + })); + app.use(passport.initialize()) + app.use(passport.session()) + testApp = app +}) + +describe('Authenticate Middleware', function () { + + test('route is protected and redirects to "/login"', async () => { + testApp.get('/', middlewares.isAuthenticated(), () => { + fail('did not protect') + }) + + const res = await testSession.get('/') + + expect(res.status).toBe(302) + expect(res.headers['location']).toBe('/login') + }) + + test('default url after login', async () => { + testApp.get('/', middlewares.isAuthenticated(), () => { + fail('did not protect') + }) + + testApp.get('/redirect-me', (req, res) => { + res.redirect(req.session.returnTo) + }) + + await testSession.get('/') + const res = await testSession.get('/redirect-me') + expect(res.status).toBe(302) + expect(res.headers['location']).toBe('/') + }) + + test('custom url after login', async () => { + testApp.get('/', middlewares.isAuthenticated('/go-here'), () => { + fail('did not protect') + }) + + testApp.get('/redirect-me', (req, res) => { + res.redirect(req.session.returnTo) + }) + + await testSession.get('/') + const res = await testSession.get('/redirect-me') + expect(res.status).toBe(302) + expect(res.headers['location']).toBe('/go-here') + }) + + test('api url flag return 401', async () => { + testApp.get('/', middlewares.isAuthenticated(null, true), () => { + fail('did not protect') + }) + + const res = await testSession.get('/') + expect(res.status).toBe(401) + expect(res.body).toEqual({error: 'Unauthorized'}) + }) + + test('identifies xhr request', async () => { + testApp.get('/', middlewares.isAuthenticated(), () => { + fail('did not protect') + }) + + const res = await testSession.get('/').set('X-Requested-With', 'XMLHttpRequest') + expect(res.status).toBe(401) + expect(res.body).toEqual({error: 'Unauthorized'}) + }) + + test('identifies json accept request', async () => { + testApp.get('/', middlewares.isAuthenticated(), () => { + fail('did not protect') + }) + + const res = await testSession.get('/').set('Accept', 'application/json') + expect(res.status).toBe(401) + expect(res.body).toEqual({error: 'Unauthorized'}) + }) +}) diff --git a/yarn.lock b/yarn.lock index 22f0888d..b8fea215 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1811,7 +1811,7 @@ cookie@0.5.0: resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.5.0.tgz#d1f5d71adec6558c58f389987c366aa47e994f8b" integrity sha512-YZ3GUyn/o8gfKJlnlX7g7xq4gyO6OSuhGPKaaGssGB2qgDUS0gPgtTvoyZLTt9Ab6dC4hfc9dV5arkvc/OCmrw== -cookiejar@^2.1.4: +cookiejar@^2.1.2, cookiejar@^2.1.4: version "2.1.4" resolved "https://registry.yarnpkg.com/cookiejar/-/cookiejar-2.1.4.tgz#ee669c1fea2cf42dc31585469d193fef0d65771b" integrity sha512-LDx6oHrK+PhzLKJU9j5S7/Y3jM/mUHvD/DeI1WQmJn652iPC5Y4TBzC9l+5OMOXlyTTA+SmVUPm0HQUwpD5Jqw== @@ -6705,6 +6705,15 @@ superagent@^8.0.5: qs "^6.11.0" semver "^7.3.8" +supertest-session@^5.0.1: + version "5.0.1" + resolved "https://registry.yarnpkg.com/supertest-session/-/supertest-session-5.0.1.tgz#046beefab6b5cc5c1f38e7ff9a532a341a2925e1" + integrity sha512-RpR8tGQZGreQsOCiW3YMSPKMwPlAB8lA0Jyat+8VUSJaYvLHTMqhMW6gooJ2htzjr3w/kgqJTQDnmuFenzA9JA== + dependencies: + cookiejar "^2.1.2" + methods "^1.1.2" + object-assign "^4.0.1" + supertest@^6.3.3: version "6.3.3" resolved "https://registry.yarnpkg.com/supertest/-/supertest-6.3.3.tgz#42f4da199fee656106fd422c094cf6c9578141db"