From 3f1ea052cbeb4f888830c72f06cb4b3853bbaba6 Mon Sep 17 00:00:00 2001 From: fcaps <48691346+fcaps@users.noreply.github.com> Date: Wed, 29 Nov 2023 02:19:06 +0100 Subject: [PATCH] token refresh oauth clients (#492) init global api-client with auto-refresh token, loggedInUserService and some minor fixes --- config/app.js | 4 +- fafApp.js | 54 +++++++- lib/ApiErrors.js | 1 + lib/JavaApiClientFactory.js | 69 ++++++++++ lib/LeaderboardRepository.js | 19 ++- lib/LeaderboardServiceFactory.js | 14 -- lib/LoggedInUserService.js | 15 +++ lib/UserRepository.js | 32 +++++ package.json | 2 + routes/middleware.js | 43 ++++-- routes/views/account/get/connectSteam.js | 2 +- routes/views/account/get/linkGog.js | 2 +- routes/views/account/get/report.js | 2 +- .../views/account/get/requestPasswordReset.js | 72 +++++----- routes/views/account/get/resync.js | 2 +- routes/views/account/post/changeEmail.js | 2 +- routes/views/account/post/changePassword.js | 2 +- routes/views/account/post/changeUsername.js | 2 +- routes/views/account/post/linkGog.js | 4 +- routes/views/account/post/report.js | 4 +- routes/views/auth.js | 45 +------ routes/views/leaderboardRouter.js | 18 ++- src/frontend/js/entrypoint/leaderboards.js | 4 + templates/layouts/default.pug | 18 +-- templates/views/account/report.pug | 2 +- tests/JavaApiClient.test.js | 126 ++++++++++++++++++ tests/helpers/StrategyMock.js | 15 ++- tests/integration/servicesMiddleware.test.js | 60 +++++++++ yarn.lock | 21 ++- 29 files changed, 501 insertions(+), 155 deletions(-) create mode 100644 lib/ApiErrors.js create mode 100644 lib/JavaApiClientFactory.js delete mode 100644 lib/LeaderboardServiceFactory.js create mode 100644 lib/LoggedInUserService.js create mode 100644 lib/UserRepository.js create mode 100644 tests/JavaApiClient.test.js create mode 100644 tests/integration/servicesMiddleware.test.js diff --git a/config/app.js b/config/app.js index 89f4b120..85d6d104 100644 --- a/config/app.js +++ b/config/app.js @@ -11,6 +11,7 @@ const appConfig = { tokenLifespan: process.env.TOKEN_LIFESPAN || 43200 }, oauth: { + strategy: 'faforever', clientId: process.env.OAUTH_CLIENT_ID || '12345', clientSecret: process.env.OAUTH_CLIENT_SECRET || '12345', url: oauthUrl, @@ -20,7 +21,8 @@ const appConfig = { apiUrl: process.env.API_URL || 'https://api.faforever.com', wordpressUrl: process.env.WP_URL || 'https://direct.faforever.com', extractorInterval: process.env.EXTRACTOR_INTERVAL || 5, - playerCountInterval: process.env.PLAYER_COUNT_INTERVAL || 15 + playerCountInterval: process.env.PLAYER_COUNT_INTERVAL || 15, + recaptchaKey: process.env.RECAPTCHA_SITE_KEY || 'test' } module.exports = appConfig diff --git a/fafApp.js b/fafApp.js index 87fd455e..d00143b4 100644 --- a/fafApp.js +++ b/fafApp.js @@ -1,4 +1,4 @@ -const appConfig = require("./config/app") +const appConfig = require('./config/app') const express = require('express') const bodyParser = require('body-parser') const session = require('express-session') @@ -15,6 +15,10 @@ const clanRouter = require("./routes/views/clanRouter") const accountRouter = require("./routes/views/accountRouter") const dataRouter = require('./routes/views/dataRouter'); const setupCronJobs = require("./scripts/cron-jobs") +const OidcStrategy = require('passport-openidconnect') +const refresh = require('passport-oauth2-refresh') +const {JavaApiClientFactory} = require('./lib/JavaApiClientFactory') +const UserRepository = require('./lib/UserRepository') const copyFlashHandler = (req, res, next) => { res.locals.message = req.flash(); @@ -33,6 +37,44 @@ const errorHandler = (err, req, res, next) => { res.status(500).render('errors/500'); } +const configureAuth = (app) => { + app.use(passport.initialize()) + app.use(passport.session()) + + passport.serializeUser((user, done) => done(null, user)) + passport.deserializeUser((user, done) => done(null, user)) + + const authStrategy = new OidcStrategy({ + issuer: appConfig.oauth.url + '/', + tokenURL: appConfig.oauth.url + '/oauth2/token', + authorizationURL: appConfig.oauth.publicUrl + '/oauth2/auth', + userInfoURL: appConfig.oauth.url + '/userinfo?schema=openid', + clientID: appConfig.oauth.clientId, + clientSecret: appConfig.oauth.clientSecret, + callbackURL: `${appConfig.host}/${appConfig.oauth.callback}`, + scope: ['openid', 'offline', 'public_profile', 'write_account_data'] + }, async function (iss, sub, profile, jwtClaims, token, refreshToken, params, verified) { + const oAuthPassport = { + token, + refreshToken + } + + const apiClient = JavaApiClientFactory(appConfig.apiUrl, oAuthPassport) + const userRepository = new UserRepository(apiClient) + + userRepository.fetchUser(oAuthPassport).then(user => { + verified(null, user) + }).catch(e => { + console.error('[Error] oAuth verify failed with "' + e.toString() + '"') + verified(null, null) + }) + } + ) + + passport.use(appConfig.oauth.strategy, authStrategy) + refresh.use(appConfig.oauth.strategy, authStrategy) +} + module.exports.setupCronJobs = () => { setupCronJobs() } @@ -64,7 +106,6 @@ module.exports.setup = (app) => { app.set('view engine', 'pug') app.set('port', appConfig.expressPort) - app.use(middleware.injectServices) app.use(middleware.initLocals) app.use(express.static('public', { @@ -91,10 +132,13 @@ module.exports.setup = (app) => { secret: appConfig.session.key }) })) - app.use(passport.initialize()) - app.use(passport.session()) + + configureAuth(app) + + app.use(middleware.injectServices) + app.use(flash()) - app.use(middleware.username) + app.use(middleware.populatePugGlobals) app.use(middleware.webpackAsset) app.use(copyFlashHandler) } diff --git a/lib/ApiErrors.js b/lib/ApiErrors.js new file mode 100644 index 00000000..05e64490 --- /dev/null +++ b/lib/ApiErrors.js @@ -0,0 +1 @@ +module.exports.AuthFailed = class AuthFailed extends Error {} diff --git a/lib/JavaApiClientFactory.js b/lib/JavaApiClientFactory.js new file mode 100644 index 00000000..d732de79 --- /dev/null +++ b/lib/JavaApiClientFactory.js @@ -0,0 +1,69 @@ +const { Axios } = require('axios') +const refresh = require('passport-oauth2-refresh') +const { AuthFailed } = require('./ApiErrors') +const appConfig = require('../config/app') + +const getRefreshToken = (oAuthPassport) => { + return new Promise((resolve, reject) => { + refresh.requestNewAccessToken(appConfig.oauth.strategy, oAuthPassport.refreshToken, function (err, accessToken, refreshToken) { + if (err || !accessToken || !refreshToken) { + return reject(new AuthFailed('Failed to refresh token')) + } + + return resolve([accessToken, refreshToken]) + }) + }) +} + +module.exports.JavaApiClientFactory = (javaApiBaseURL, oAuthPassport) => { + if (typeof oAuthPassport !== "object") { + throw new Error("oAuthPassport not an object"); + } + + if (typeof oAuthPassport.refreshToken !== "string") { + throw new Error("oAuthPassport.refreshToken not a string") + } + + if (typeof oAuthPassport.token !== "string") { + throw new Error("oAuthPassport.token not a string") + } + + let tokenRefreshRunning = null + const client = new Axios({ + baseURL: javaApiBaseURL + }) + + client.interceptors.request.use( + async config => { + config.headers = { + Authorization: `Bearer ${oAuthPassport.token}` + } + + return config + }) + + client.interceptors.response.use((res) => { + if (!res.config._refreshTokenRequest && res.config && res.status === 401) { + res.config._refreshTokenRequest = true + + if (!tokenRefreshRunning) { + tokenRefreshRunning = getRefreshToken(oAuthPassport) + } + + return tokenRefreshRunning.then(([token, refreshToken]) => { + oAuthPassport.token = token + oAuthPassport.refreshToken = refreshToken + + return client.request(res.config) + }) + } + + if (res.status === 401) { + throw new AuthFailed('Token no longer valid and refresh did not help') + } + + return res + }) + + return client +} diff --git a/lib/LeaderboardRepository.js b/lib/LeaderboardRepository.js index d8f75f3b..1fb50f1f 100644 --- a/lib/LeaderboardRepository.js +++ b/lib/LeaderboardRepository.js @@ -45,13 +45,18 @@ class LeaderboardRepository { let leaderboardData = [] data.data.forEach((item, index) => { - leaderboardData.push({ - rating: item.attributes.rating, - totalgames: item.attributes.totalGames, - wonGames: item.attributes.wonGames, - date: item.attributes.updateTime, - label: data.included[index].attributes.login, - }) + try { + leaderboardData.push({ + rating: item.attributes.rating, + totalgames: item.attributes.totalGames, + wonGames: item.attributes.wonGames, + date: item.attributes.updateTime, + label: data.included[index]?.attributes.login || 'unknown user', + }) + } catch (e) { + console.error('LeaderboardRepository::mapResponse failed on item with "' + e.toString() + '"') + } + }) return leaderboardData diff --git a/lib/LeaderboardServiceFactory.js b/lib/LeaderboardServiceFactory.js deleted file mode 100644 index 1d9ad1a0..00000000 --- a/lib/LeaderboardServiceFactory.js +++ /dev/null @@ -1,14 +0,0 @@ -const LeaderboardService = require("./LeaderboardService"); -const LeaderboardRepository = require("./LeaderboardRepository"); -const {Axios} = require("axios"); -const cacheService = require('./CacheService') - -module.exports = (javaApiBaseURL, token) => { - const config = { - baseURL: javaApiBaseURL, - headers: {Authorization: `Bearer ${token}`} - }; - const javaApiClient = new Axios(config) - - return new LeaderboardService(cacheService, new LeaderboardRepository(javaApiClient)) -} diff --git a/lib/LoggedInUserService.js b/lib/LoggedInUserService.js new file mode 100644 index 00000000..2945d9cb --- /dev/null +++ b/lib/LoggedInUserService.js @@ -0,0 +1,15 @@ +class LoggedInUserService { + constructor (userRepository, request) { + this.request = request + + if (typeof this.request.user !== "object") { + throw new Error("request.user not an object"); + } + } + + getUser() { + return this.request.user + } +} + +module.exports = LoggedInUserService diff --git a/lib/UserRepository.js b/lib/UserRepository.js new file mode 100644 index 00000000..88c4a355 --- /dev/null +++ b/lib/UserRepository.js @@ -0,0 +1,32 @@ +class UserRepository { + constructor (javaApiClient) { + this.javaApiClient = javaApiClient + } + + fetchUser (oAuthPassport) { + return this.javaApiClient.get('/me').then(response => { + const rawUser = JSON.parse(response.data).data + + let clan = null + + if (rawUser.attributes.clan) { + clan = { + id: rawUser.attributes.clan.id, + membershipId: rawUser.attributes.clan.membershipId, + tag: rawUser.attributes.clan.tag, + name: rawUser.attributes.clan.name + } + } + + return { + id: rawUser.id, + name: rawUser.attributes.userName, + email: rawUser.attributes.email, + clan, + oAuthPassport + } + }) + } +} + +module.exports = UserRepository diff --git a/package.json b/package.json index b1b8ef69..46185d0c 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "node-fetch": "^2.6.7", "npm-check": "^6.0.1", "passport": "^0.6.0", + "passport-oauth2-refresh": "^2.2.0", "passport-openidconnect": "^0.1.1", "pug": "3.0.2", "request": "2.88.2", @@ -44,6 +45,7 @@ "jquery": "^3.7.1", "load-grunt-config": "4.0.1", "load-grunt-tasks": "5.1.0", + "nock": "^13.3.8", "octokit": "^3.1.2", "supertest": "^6.3.3", "webpack": "^5.89.0", diff --git a/routes/middleware.js b/routes/middleware.js index 5db9fa2d..8d31807c 100755 --- a/routes/middleware.js +++ b/routes/middleware.js @@ -1,8 +1,15 @@ const WordpressServiceFactory = require("../lib/WordpressServiceFactory"); +const {JavaApiClientFactory} = require('../lib/JavaApiClientFactory') +const LeaderboardService = require('../lib/LeaderboardService') +const LeaderboardRepository = require('../lib/LeaderboardRepository') +const cacheService = require('../lib/CacheService') const appConfig = require("../config/app"); const wordpressService = WordpressServiceFactory(appConfig.wordpressUrl) const fs = require('fs'); const webpackManifestJS = JSON.parse(fs.readFileSync('dist/js/manifest.json', 'utf8')); +const LoggedInUserService = require('../lib/LoggedInUserService') +const UserRepository = require('../lib/UserRepository'); + exports.initLocals = function(req, res, next) { let locals = res.locals; @@ -23,17 +30,16 @@ exports.webpackAsset = (req, res, next) => { next() } -exports.username = function(req, res, next) { - var locals = res.locals; - - if (req.isAuthenticated()) { - locals.username = req.user.data.attributes.userName; - locals.hasClan = - req.user && req.user.data.attributes.clan; - } - - next(); -}; +exports.populatePugGlobals = function (req, res, next) { + res.locals.appGlobals = { + loggedInUser: null + } + + if (req.isAuthenticated()) { + res.locals.appGlobals.loggedInUser = req.services.userService.getUser() + } + next() +} exports.isAuthenticated = (redirectUrlAfterLogin = null, isApiRequest = false) => { return (req, res, next) => { @@ -53,9 +59,18 @@ exports.isAuthenticated = (redirectUrlAfterLogin = null, isApiRequest = false) = } } -exports.injectServices = function(req, res, next) { - req.services = { - wordpressService: wordpressService +exports.injectServices = (req, res, next) => { + req.services = {} + req.services.wordpressService = wordpressService + + if (req.isAuthenticated()) { + try { + req.services.javaApiClient = JavaApiClientFactory(appConfig.apiUrl, req.user.oAuthPassport) + req.services.userService = new LoggedInUserService(new UserRepository(req.services.javaApiClient), req) + req.services.leaderboardService = new LeaderboardService(cacheService, new LeaderboardRepository(req.services.javaApiClient)) + } catch (e) { + req.logout(() => next(e)) + } } next() diff --git a/routes/views/account/get/connectSteam.js b/routes/views/account/get/connectSteam.js index fc442bbe..a3229cd4 100644 --- a/routes/views/account/get/connectSteam.js +++ b/routes/views/account/get/connectSteam.js @@ -10,7 +10,7 @@ exports = module.exports = function (req, res) { request.post({ 'url': process.env.API_URL + '/users/buildSteamLinkUrl', - 'headers': {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + 'headers': {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, form: {callbackUrl: req.protocol + '://' + req.get('host') + '/account/link?done'} }, function (err, res, body) { //Must not be valid, check to see if errors, otherwise return generic error. diff --git a/routes/views/account/get/linkGog.js b/routes/views/account/get/linkGog.js index 0615a456..fd429f5c 100644 --- a/routes/views/account/get/linkGog.js +++ b/routes/views/account/get/linkGog.js @@ -32,7 +32,7 @@ exports = module.exports = function (req, res) { request.get({ url: process.env.API_URL + '/users/buildGogProfileToken', - headers: {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + headers: {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, form: {} }, function (err, res, body) { locals.gogToken = 'unable to obtain token'; diff --git a/routes/views/account/get/report.js b/routes/views/account/get/report.js index f7e65d4e..e720b2fe 100644 --- a/routes/views/account/get/report.js +++ b/routes/views/account/get/report.js @@ -39,7 +39,7 @@ exports = module.exports = function (req, res) { { url: process.env.API_URL + '/data/moderationReport?include=reportedUsers,lastModerator&sort=-createTime', headers: { - 'Authorization': 'Bearer ' + req.user.data.attributes.token + 'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token } }, function (err, childRes, body) { diff --git a/routes/views/account/get/requestPasswordReset.js b/routes/views/account/get/requestPasswordReset.js index b0ed5fae..19d6af44 100644 --- a/routes/views/account/get/requestPasswordReset.js +++ b/routes/views/account/get/requestPasswordReset.js @@ -1,40 +1,34 @@ -let request = require('request'); -let error = require('../post/error'); - -exports = module.exports = function (req, res) { - - var locals = res.locals; - - // locals.section is used to set the currently selected - // item in the header navigation. - locals.section = 'account'; - - locals.formData = req.body || {}; - - var flash = {}; - - let overallRes = res; - - request.post({ - url: process.env.API_URL + '/users/buildSteamPasswordResetUrl', - }, function (err, res, body) { - - if (err || res.statusCode !== 200) { - console.error(err); - if (res) { - error.parseApiErrors(body, flash); - } else { - flash.class = 'alert-danger'; - flash.messages = err; - flash.type = 'Error!'; - } - overallRes.render('account/requestPasswordReset', {flash: flash, recaptchaSiteKey: process.env.RECAPTCHA_SITE_KEY}); - return; - } - - locals.steamReset = JSON.parse(body).steamUrl; - overallRes.render('account/requestPasswordReset', {flash: flash, recaptchaSiteKey: process.env.RECAPTCHA_SITE_KEY}); +const axios = require("axios"); +const appConfig = require('../../../../config/app') + +exports = module.exports = async function (req, res) { + const formData = req.body || {}; + + // funky issue: https://stackoverflow.com/questions/69169492/async-external-function-leaves-open-handles-jest-supertest-express + await new Promise(resolve => process.nextTick(resolve)) + + axios.post(appConfig.apiUrl + '/users/buildSteamPasswordResetUrl', {}, { maxRedirects: 0 }).then(response => { + if (response.status !== 200) { + throw new Error('java-api error') } - ); - -}; + + res.render('account/requestPasswordReset', { + section: 'account', + flash: {}, + steamReset: response.data.steamUrl, + formData: formData, + recaptchaSiteKey: appConfig.recaptchaKey + }) + }).catch(error => { + res.render('account/requestPasswordReset', { + section: 'account', + flash: { + class: 'alert-danger', + messages: 'issue resetting', + type: 'Error!', + }, + formData: formData, + recaptchaSiteKey: appConfig.recaptchaKey + }) + }) +} diff --git a/routes/views/account/get/resync.js b/routes/views/account/get/resync.js index e402f0f0..dfef5f03 100644 --- a/routes/views/account/get/resync.js +++ b/routes/views/account/get/resync.js @@ -16,7 +16,7 @@ exports = module.exports = function (req, res) { request.post({ url: process.env.API_URL + '/users/resyncAccount', - headers: {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + headers: {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, }, function (err, res, body) { if (res.statusCode !== 200) { diff --git a/routes/views/account/post/changeEmail.js b/routes/views/account/post/changeEmail.js index bd685b3c..11519ea2 100644 --- a/routes/views/account/post/changeEmail.js +++ b/routes/views/account/post/changeEmail.js @@ -38,7 +38,7 @@ exports = module.exports = function (req, res) { request.post({ url: `${process.env.API_URL}/users/changeEmail`, - headers: {'Authorization': `Bearer ${req.user.data.attributes.token}`}, + headers: {'Authorization': `Bearer ${req.services.userService.getUser()?.oAuthPassport.token}`}, form: {newEmail: email, currentPassword: password} }, function (err, res, body) { diff --git a/routes/views/account/post/changePassword.js b/routes/views/account/post/changePassword.js index 55ed0286..a5423884 100644 --- a/routes/views/account/post/changePassword.js +++ b/routes/views/account/post/changePassword.js @@ -43,7 +43,7 @@ exports = module.exports = function(req, res) { //Run post to reset endpoint request.post({ url: process.env.API_URL + '/users/changePassword', - headers: {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + headers: {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, form: {name: username, currentPassword: oldPassword, newPassword: newPassword} }, function (err, res, body) { diff --git a/routes/views/account/post/changeUsername.js b/routes/views/account/post/changeUsername.js index ec44a36f..94f27bf1 100644 --- a/routes/views/account/post/changeUsername.js +++ b/routes/views/account/post/changeUsername.js @@ -34,7 +34,7 @@ exports = module.exports = function (req, res) { //Run post to reset endpoint request.post({ url: process.env.API_URL + '/users/changeUsername', - headers: {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + headers: {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, form: {newUsername: username} }, function (err, res, body) { diff --git a/routes/views/account/post/linkGog.js b/routes/views/account/post/linkGog.js index 7c77d0ec..1a414465 100644 --- a/routes/views/account/post/linkGog.js +++ b/routes/views/account/post/linkGog.js @@ -35,7 +35,7 @@ exports = module.exports = function(req, res) { request.post({ url: process.env.API_URL + '/users/linkToGog', - headers: {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + headers: {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, form: {gogUsername: gogUsername} }, function (err, res, body) { @@ -55,7 +55,7 @@ exports = module.exports = function(req, res) { // it's not possible to extract it into a separate function while saving any code request.get({ url: process.env.API_URL + '/users/buildGogProfileToken', - headers: {'Authorization': 'Bearer ' + req.user.data.attributes.token}, + headers: {'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token}, form: {} }, function (err, res, body) { locals.gogToken = 'unable to obtain token'; diff --git a/routes/views/account/post/report.js b/routes/views/account/post/report.js index 70e01e75..7297a700 100644 --- a/routes/views/account/post/report.js +++ b/routes/views/account/post/report.js @@ -6,7 +6,7 @@ function promiseRequest(url, req) { return new Promise(function (resolve, reject) { request(url, { headers: { - 'Authorization': 'Bearer ' + req.user.data.attributes.token, + 'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token, } }, function (error, res, body) { if (!error && res.statusCode < 300) { @@ -185,7 +185,7 @@ exports = module.exports = async function (req, res) { url: process.env.API_URL + '/data/moderationReport', body: JSON.stringify(report), headers: { - 'Authorization': 'Bearer ' + req.user.data.attributes.token, + 'Authorization': 'Bearer ' + req.services.userService.getUser()?.oAuthPassport.token, 'Content-Type': 'application/vnd.api+json', 'Accept': 'application/vnd.api+json' } diff --git a/routes/views/auth.js b/routes/views/auth.js index 75a7f1bf..7c5aff04 100644 --- a/routes/views/auth.js +++ b/routes/views/auth.js @@ -1,44 +1,9 @@ const appConfig = require('../../config/app') -const passport = require('passport'); -const OidcStrategy = require('passport-openidconnect'); -const express = require("express"); -const axios = require("axios"); -const router = express.Router(); +const passport = require('passport') +const express = require("express") +const router = express.Router() -passport.serializeUser((user, done) => done(null, user)) -passport.deserializeUser((user, done) => done(null, user)) - -passport.use('faforever', new OidcStrategy({ - issuer: appConfig.oauth.url + '/', - tokenURL: appConfig.oauth.url + '/oauth2/token', - authorizationURL: appConfig.oauth.publicUrl + '/oauth2/auth', - userInfoURL: appConfig.oauth.url + '/userinfo?schema=openid', - clientID: appConfig.oauth.clientId, - clientSecret: appConfig.oauth.clientSecret, - callbackURL: `${appConfig.host}/${appConfig.oauth.callback}`, - scope: ['openid', 'public_profile', 'write_account_data'] - }, function (iss, sub, profile, jwtClaims, accessToken, refreshToken, params, verified) { - - axios.get( - appConfig.apiUrl + '/me', - { - headers: {'Authorization': `Bearer ${accessToken}`} - }).then((res) => { - const user = res.data - user.token = accessToken - user.data.attributes.token = accessToken; - user.data.id = user.data.attributes.userId; - - return verified(null, user); - }).catch(e => { - console.error('[Error] views/auth.js::passport::verify failed with "' + e.toString() + '"'); - - return verified(null, null); - }); - } -)); - -router.get('/login', passport.authenticate('faforever')); +router.get('/login', passport.authenticate(appConfig.oauth.strategy)); router.get( '/' + appConfig.oauth.callback, @@ -47,7 +12,7 @@ router.get( return next() }, - passport.authenticate('faforever', {failureRedirect: '/login', failureFlash: true}), + passport.authenticate(appConfig.oauth.strategy, {failureRedirect: '/login', failureFlash: true}), (req, res) => { res.redirect(res.locals.returnTo || '/') } diff --git a/routes/views/leaderboardRouter.js b/routes/views/leaderboardRouter.js index 0f67fa5a..0a667ff0 100644 --- a/routes/views/leaderboardRouter.js +++ b/routes/views/leaderboardRouter.js @@ -1,9 +1,8 @@ -const appConfig = require('../../config/app') const express = require('express'); const router = express.Router(); -const LeaderboardServiceFactory = require('../../lib/LeaderboardServiceFactory') const {AcquireTimeoutError} = require('../../lib/MutexService'); const middlewares = require('../middleware') +const {AuthFailed} = require('../../lib/ApiErrors') const getLeaderboardId = (leaderboardName) => { @@ -33,14 +32,21 @@ router.get('/:leaderboard.json', middlewares.isAuthenticated(null, true), async return res.status(404).json({error: 'Leaderboard "' + req.params.leaderboard + '" does not exist'}) } - const token = req.user.data.attributes.token - const leaderboardService = LeaderboardServiceFactory(appConfig.apiUrl, token) - - return res.json(await leaderboardService.getLeaderboard(leaderboardId)) + return res.json(await req.services.leaderboardService.getLeaderboard(leaderboardId)) } catch (e) { if (e instanceof AcquireTimeoutError) { return res.status(503).json({error: 'timeout reached'}) } + + if (e instanceof AuthFailed) { + req.logout(function(err) { + if (err) { + throw err + } + }) + + return res.status(400).json({error: 'authentication failed, reload site'}) + } console.error('[error] leaderboardRouter::get:leaderboard.json failed with "' + e.toString() + '"') diff --git a/src/frontend/js/entrypoint/leaderboards.js b/src/frontend/js/entrypoint/leaderboards.js index ffa65390..17c630e6 100644 --- a/src/frontend/js/entrypoint/leaderboards.js +++ b/src/frontend/js/entrypoint/leaderboards.js @@ -16,6 +16,10 @@ let currentDate = new Date(minusTimeFilter).toISOString() async function leaderboardOneJSON (leaderboardFile) { // Check which category is active const response = await fetch(`leaderboards/${leaderboardFile}.json`) + + if (response.status === 400) { + window.location.href = '/leaderboards' + } currentLeaderboard = leaderboardFile const data = await response.json() return await data diff --git a/templates/layouts/default.pug b/templates/layouts/default.pug index a2ef38e4..4cd0024e 100755 --- a/templates/layouts/default.pug +++ b/templates/layouts/default.pug @@ -107,7 +107,7 @@ html(lang='en') li.navList PLAY NOW ul.navAbsolute - if !username + if !appGlobals.loggedInUser .navContainer.navEnd ul.navItem a(href="/login") @@ -117,18 +117,18 @@ html(lang='en') a(href="/account/register") li.navList REGISTER - if username + if appGlobals.loggedInUser .navContainer.navEnd#loginList ul.loginItem .navList img(src='/images/fontAwesomeIcons/user.svg') - h3 #{username} + h3 #{appGlobals.loggedInUser.name} ul#loginAbsolute li.loginDropdown ul(role='menu') - if hasClan + if appGlobals.loggedInUser.clan a(href="/clans/manage"): li My Clan - if !hasClan + else a(href="/clans/create"): li Create Clan a(href="/account/changeEmail"): li Change Email a(href="/account/changePassword"): li Change Password @@ -181,16 +181,16 @@ html(lang='en') a(href='/donation') Donate a(href='/contribution') Contribute a(href='/clans') Clans - if !username + if !appGlobals.loggedInUser a(href='/login').mobileNavElement Login - if username + if appGlobals.loggedInUser .mobileNavElement.mobileNavMenu My Account .mobileNavMenuContent - if hasClan + if appGlobals.loggedInUser.clan a(href="/clans/manage") My Clan - if !hasClan + else a(href="/clans/create") Create Clan a(href="/account/changeEmail") Change Email a(href="/account/changePassword") Change Password diff --git a/templates/views/account/report.pug b/templates/views/account/report.pug index 319c76fc..d4660477 100755 --- a/templates/views/account/report.pug +++ b/templates/views/account/report.pug @@ -33,7 +33,7 @@ block content .column6 div.form-group p Reporter: - input(type='text', disabled='disabled', value=username).form-control + input(type='text', disabled='disabled', value=appGlobals.loggedInUser.name).form-control span(aria-hidden='true').glyphicon.form-control-feedback div.form-group diff --git a/tests/JavaApiClient.test.js b/tests/JavaApiClient.test.js new file mode 100644 index 00000000..255a6bbd --- /dev/null +++ b/tests/JavaApiClient.test.js @@ -0,0 +1,126 @@ +const { JavaApiClientFactory } = require('../lib/JavaApiClientFactory') +const appConfig = require('../config/app') +const refresh = require('passport-oauth2-refresh') +const OidcStrategy = require('passport-openidconnect') +const nock = require('nock') +const { AuthFailed } = require('../lib/ApiErrors') + +beforeEach(() => { + refresh.use(appConfig.oauth.strategy, new OidcStrategy({ + issuer: 'me', + tokenURL: 'http://auth-localhost/oauth2/token', + authorizationURL: 'http://auth-localhost/oauth2/auth', + clientID: 'test', + clientSecret: 'test', + scope: ['openid', 'offline'] + }, () => {})) +}) + +afterEach(() => { + refresh._strategies = {} + jest.restoreAllMocks() +}) +test('empty passport', () => { + expect(() => JavaApiClientFactory('http://api-localhost')).toThrowError('oAuthPassport not an object') +}) + +test('empty token', () => { + expect(() => JavaApiClientFactory('http://api-localhost', { refreshToken: '123' })).toThrowError('oAuthPassport.token not a string') +}) + +test('empty refresh-token', () => { + expect(() => JavaApiClientFactory('http://api-localhost', { token: '123' })).toThrowError('oAuthPassport.refreshToken not a string') +}) + +test('multiple calls with stale token will trigger refresh only once', async () => { + const client = JavaApiClientFactory('http://api-localhost', { + token: '123', + refreshToken: '456' + }) + + const refreshSpy = jest.spyOn(refresh, 'requestNewAccessToken') + const apiScope = nock('http://api-localhost') + .get('/example') + .times(2) + .reply(401, 'nope') + .get('/example') + .times(2) + .reply(200, 'OK') + + const authScope = nock('http://auth-localhost') + .post('/oauth2/token') + .times(1) + .reply(200, { access_token: 'new_tok', refresh_token: 'new_ref' }) + + const response = client.get('/example').then((res) => { + expect(res.request.headers.authorization).toBe('Bearer new_tok') + }) + + const response2 = client.get('/example').then((res) => { + expect(res.request.headers.authorization).toBe('Bearer new_tok') + }) + + await Promise.all([response, response2]) + + expect(refreshSpy).toBeCalledTimes(1) + + apiScope.done() + authScope.done() +}) + +test('refresh will throw on error', async () => { + const client = JavaApiClientFactory('http://api-localhost', { + token: '123', + refreshToken: '456' + }) + + const refreshSpy = jest.spyOn(refresh, 'requestNewAccessToken') + const apiScope = nock('http://api-localhost') + .get('/example') + .reply(401, 'nope') + + const authScope = nock('http://auth-localhost') + .post('/oauth2/token') + .times(1) + .reply(400, null) + + let thrown = false + try { + await client.get('/example') + } catch (e) { + expect(e).toBeInstanceOf(AuthFailed) + thrown = true + } + + expect(thrown).toBe(true) + expect(refreshSpy).toBeCalledTimes(1) + + apiScope.done() + authScope.done() +}) + +test('refresh will not loop to death', async () => { + const client = JavaApiClientFactory('http://api-localhost', { + token: '123', + refreshToken: '456' + }) + + const apiScope = nock('http://api-localhost') + .get('/example') + .times(2) + .reply(401, 'nope') + + const authScope = nock('http://auth-localhost') + .post('/oauth2/token') + .times(1) + .reply(200, { access_token: 'new_tok', refresh_token: 'new_ref' }) + + try { + await client.get('/example') + } catch (e) { + expect(e).toBeInstanceOf(AuthFailed) + } + + apiScope.done() + authScope.done() +}) diff --git a/tests/helpers/StrategyMock.js b/tests/helpers/StrategyMock.js index 73b5a240..5a583f5d 100644 --- a/tests/helpers/StrategyMock.js +++ b/tests/helpers/StrategyMock.js @@ -5,13 +5,14 @@ function StrategyMock (options) { this.name = 'mock' this.passAuthentication = options.passAuthentication ?? true this.user = options.user || { - token: 'test-token', - data: { - id: 1, - attributes: { - token: 'test-token' - } - } + oAuthPassport: { + token: 'test-token', + refreshToken: 'test-refresh-token' + }, + id: -1, + name: 'minion', + email: 'banana@example.org', + clan: null } } diff --git a/tests/integration/servicesMiddleware.test.js b/tests/integration/servicesMiddleware.test.js new file mode 100644 index 00000000..88f70d49 --- /dev/null +++ b/tests/integration/servicesMiddleware.test.js @@ -0,0 +1,60 @@ +const Express = require('express') +const WordpressService = require('../../lib/WordpressService') +const UserService = require('../../lib/LoggedInUserService') +const LeaderboardService = require('../../lib/LeaderboardService') +const supertestSession = require('supertest-session') +const fafApp = require('../../fafApp') +const passportMock = require('../helpers/PassportMock') +const { Axios } = require('axios') + +let testApp = null +let testSession = null + +beforeEach(() => { + const app = new Express() + fafApp.setup(app) + passportMock(app, { passAuthentication: true }) + testSession = supertestSession(app) + testApp = app +}) + +describe('Services Middleware', function () { + test('public service are loaded without a user', (done) => { + expect.assertions(3) + testApp.get('/', (req, res) => { + try { + expect(req.services).not.toBeUndefined() + expect(Object.keys(req.services).length).toBe(1) + expect(req.services.wordpressService).toBeInstanceOf(WordpressService) + + return res.send('success') + } catch (e) { + done(e) + } + }) + + testSession.get('/').then(() => done()) + }) + + test('additional services are loaded with authenticated user', (done) => { + expect.assertions(6) + testApp.get('/', (req, res) => { + try { + expect(req.services).not.toBeUndefined() + expect(Object.keys(req.services).length).toBe(4) + expect(req.services.wordpressService).toBeInstanceOf(WordpressService) + expect(req.services.userService).toBeInstanceOf(UserService) + expect(req.services.javaApiClient).toBeInstanceOf(Axios) + expect(req.services.leaderboardService).toBeInstanceOf(LeaderboardService) + + return res.send('success') + } catch (e) { + done(e) + } + }) + + testSession.get('/mock-login').then(() => { + testSession.get('/').then(() => done()) + }) + }) +}) diff --git a/yarn.lock b/yarn.lock index 0ee1a76b..ccb783a2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5281,7 +5281,7 @@ json-stable-stringify-without-jsonify@^1.0.1: resolved "https://registry.yarnpkg.com/json-stable-stringify-without-jsonify/-/json-stable-stringify-without-jsonify-1.0.1.tgz#9db7b59496ad3f3cfef30a75142d2d930ad72651" integrity sha512-Bdboy+l7tA3OGW6FjyFHWkP5LuByj1Tk33Ljyq0axyzdk9//JSi2u3fP1QSmd1KNwq6VOKYGlAu87CisVir6Pw== -json-stringify-safe@~5.0.1: +json-stringify-safe@^5.0.1, json-stringify-safe@~5.0.1: version "5.0.1" resolved "https://registry.yarnpkg.com/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz#1296a2d58fd45f19a0f6ce01d65701e2c735b6eb" integrity sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA== @@ -5922,6 +5922,15 @@ neo-async@^2.6.2: resolved "https://registry.yarnpkg.com/neo-async/-/neo-async-2.6.2.tgz#b4aafb93e3aeb2d8174ca53cf163ab7d7308305f" integrity sha512-Yd3UES5mWCSqR+qNT93S3UoYUkqAZ9lLg8a7g9rimsWmYGK8cVToA4/sF3RrshdyV3sAGMXVUmpMYOw+dLpOuw== +nock@^13.3.8: + version "13.3.8" + resolved "https://registry.yarnpkg.com/nock/-/nock-13.3.8.tgz#7adf3c66f678b02ef0a78d5697ae8bc2ebde0142" + integrity sha512-96yVFal0c/W1lG7mmfRe7eO+hovrhJYd2obzzOZ90f6fjpeU/XNvd9cYHZKZAQJumDfhXgoTpkpJ9pvMj+hqHw== + dependencies: + debug "^4.1.0" + json-stringify-safe "^5.0.1" + propagate "^2.0.0" + node-cache@^5.1.2: version "5.1.2" resolved "https://registry.yarnpkg.com/node-cache/-/node-cache-5.1.2.tgz#f264dc2ccad0a780e76253a694e9fd0ed19c398d" @@ -6386,6 +6395,11 @@ pascalcase@^0.1.1: resolved "https://registry.yarnpkg.com/pascalcase/-/pascalcase-0.1.1.tgz#b363e55e8006ca6fe21784d2db22bd15d7917f14" integrity sha512-XHXfu/yOQRy9vYOtUDVMN60OEJjW013GoObG1o+xwQTpB9eYJX/BjXMsdW13ZDPruFhYYn0AG22w0xgQMwl3Nw== +passport-oauth2-refresh@^2.2.0: + version "2.2.0" + resolved "https://registry.yarnpkg.com/passport-oauth2-refresh/-/passport-oauth2-refresh-2.2.0.tgz#e60dd4e84e8df3c6ead87b6aab0754dec7a89aca" + integrity sha512-yXwXHL7ZZH0s2oknnjugfvwzCB5mpJ5ZNpzkb+b/sTsHeZFbx2BXfzvwsoD4aq6gq/aWuCxBV89ef+L/cjjrjg== + passport-openidconnect@^0.1.1: version "0.1.1" resolved "https://registry.yarnpkg.com/passport-openidconnect/-/passport-openidconnect-0.1.1.tgz#83921ff5f87f634079f65262dada834af1972244" @@ -6671,6 +6685,11 @@ prompts@^2.0.1: kleur "^3.0.3" sisteransi "^1.0.5" +propagate@^2.0.0: + version "2.0.1" + resolved "https://registry.yarnpkg.com/propagate/-/propagate-2.0.1.tgz#40cdedab18085c792334e64f0ac17256d38f9a45" + integrity sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag== + proxy-addr@~2.0.7: version "2.0.7" resolved "https://registry.yarnpkg.com/proxy-addr/-/proxy-addr-2.0.7.tgz#f19fe69ceab311eeb94b42e70e8c2070f9ba1025"