From e92ff9737a57cae6009c5d9f3bfb717643ad883d Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Thu, 2 Apr 2020 21:46:01 +0200 Subject: [PATCH] Automatically Logout Banned Users (#12037) * wip * logout banned users, fix and refactor language library and middleware * req.locals -> res.locals * fix tests * redirect to login page --- test/api/unit/libs/language.test.js | 111 +++++++++++++++++++ test/api/unit/middlewares/analytics.test.js | 2 +- test/api/unit/middlewares/cronMiddleware.js | 23 +--- test/api/unit/middlewares/language.test.js | 15 ++- website/client/src/app.vue | 17 +-- website/client/src/store/actions/auth.js | 5 +- website/server/controllers/top-level/auth.js | 4 +- website/server/libs/i18n.js | 32 +----- website/server/libs/language.js | 52 +++++++++ website/server/middlewares/auth.js | 14 ++- website/server/middlewares/language.js | 74 +++---------- 11 files changed, 228 insertions(+), 121 deletions(-) create mode 100644 test/api/unit/libs/language.test.js create mode 100644 website/server/libs/language.js diff --git a/test/api/unit/libs/language.test.js b/test/api/unit/libs/language.test.js new file mode 100644 index 0000000000..ec2df2ced2 --- /dev/null +++ b/test/api/unit/libs/language.test.js @@ -0,0 +1,111 @@ +import { + getLanguageFromBrowser, + getLanguageFromUser, +} from '../../../../website/server/libs/language'; +import { + generateReq, +} from '../../../helpers/api-unit.helper'; + +describe('language lib', () => { + let req; + + beforeEach(() => { + req = generateReq(); + }); + + describe('getLanguageFromUser', () => { + it('uses the user preferred language if avalaible', () => { + const user = { + preferences: { + language: 'it', + }, + }; + + expect(getLanguageFromUser(user, req)).to.equal('it'); + }); + + it('falls back to english if the user preferred language is not avalaible', () => { + const user = { + preferences: { + language: 'bla', + }, + }; + + expect(getLanguageFromUser(user, req)).to.equal('en'); + }); + }); + + describe('getLanguageFromBrowser', () => { + it('uses browser specificed language', () => { + req.headers['accept-language'] = 'pt'; + + expect(getLanguageFromBrowser(req)).to.equal('pt'); + }); + + it('uses first language in series if browser specifies multiple', () => { + req.headers['accept-language'] = 'he, pt, it'; + + expect(getLanguageFromBrowser(req)).to.equal('he'); + }); + + it('skips invalid lanaguages and uses first language in series if browser specifies multiple', () => { + req.headers['accept-language'] = 'blah, he, pt, it'; + + expect(getLanguageFromBrowser(req)).to.equal('he'); + }); + + it('uses normal version of language if specialized locale is passed in', () => { + req.headers['accept-language'] = 'fr-CA'; + + expect(getLanguageFromBrowser(req)).to.equal('fr'); + }); + + it('uses normal version of language if specialized locale is passed in', () => { + req.headers['accept-language'] = 'fr-CA'; + + expect(getLanguageFromBrowser(req)).to.equal('fr'); + }); + + it('uses es if es is passed in', () => { + req.headers['accept-language'] = 'es'; + + expect(getLanguageFromBrowser(req)).to.equal('es'); + }); + + it('uses es_419 if applicable es-languages are passed in', () => { + req.headers['accept-language'] = 'es-mx'; + + expect(getLanguageFromBrowser(req)).to.equal('es_419'); + }); + + it('uses es_419 if multiple es languages are passed in', () => { + req.headers['accept-language'] = 'es-GT, es-MX, es-CR'; + + expect(getLanguageFromBrowser(req)).to.equal('es_419'); + }); + + it('zh', () => { + req.headers['accept-language'] = 'zh-TW'; + + expect(getLanguageFromBrowser(req)).to.equal('zh_TW'); + }); + + it('uses english if browser specified language is not compatible', () => { + req.headers['accept-language'] = 'blah'; + + expect(getLanguageFromBrowser(req)).to.equal('en'); + }); + + it('uses english if browser does not specify', () => { + req.headers['accept-language'] = ''; + + expect(getLanguageFromBrowser(req)).to.equal('en'); + }); + + it('uses english if browser does not supply an accept-language header', () => { + delete req.headers['accept-language']; + + expect(getLanguageFromBrowser(req)).to.equal('en'); + }); + }); +}); diff --git a/test/api/unit/middlewares/analytics.test.js b/test/api/unit/middlewares/analytics.test.js index 32b88d00b8..8ce68aee73 100644 --- a/test/api/unit/middlewares/analytics.test.js +++ b/test/api/unit/middlewares/analytics.test.js @@ -19,7 +19,7 @@ describe('analytics middleware', () => { next = generateNext(); }); - it('attaches analytics object res.locals', () => { + it('attaches analytics object to res', () => { const attachAnalytics = requireAgain(pathToAnalyticsMiddleware).default; attachAnalytics(req, res, next); diff --git a/test/api/unit/middlewares/cronMiddleware.js b/test/api/unit/middlewares/cronMiddleware.js index bd93b06e65..873bbb70a7 100644 --- a/test/api/unit/middlewares/cronMiddleware.js +++ b/test/api/unit/middlewares/cronMiddleware.js @@ -21,28 +21,11 @@ describe('cron middleware', () => { req; let user; - beforeEach(done => { + beforeEach(async () => { res = generateRes(); req = generateReq(); - user = new User({ - auth: { - local: { - username: 'username', - lowerCaseUsername: 'username', - email: 'email@email.email', - salt: 'salt', - hashed_password: 'hashed_password', // eslint-disable-line camelcase - }, - }, - }); - - user.save() - .then(savedUser => { - res.locals.user = savedUser; - res.analytics = analyticsService; - done(); - }) - .catch(done); + user = await res.locals.user.save(); + res.analytics = analyticsService; }); afterEach(() => { diff --git a/test/api/unit/middlewares/language.test.js b/test/api/unit/middlewares/language.test.js index 4965da9a49..8aad89e714 100644 --- a/test/api/unit/middlewares/language.test.js +++ b/test/api/unit/middlewares/language.test.js @@ -12,6 +12,9 @@ import { model as User } from '../../../../website/server/models/user'; const { i18n } = common; +// TODO some of the checks here can be simplified to simply check +// that the right parameters are passed to the functions in libs/language + describe('language middleware', () => { describe('res.t', () => { let res; let req; let @@ -19,6 +22,8 @@ describe('language middleware', () => { beforeEach(() => { res = generateRes(); + // remove the defaul user + res.locals.user = undefined; req = generateReq(); next = generateNext(); @@ -57,6 +62,8 @@ describe('language middleware', () => { beforeEach(() => { res = generateRes(); + // remove the defaul user + res.locals.user = undefined; req = generateReq(); next = generateNext(); attachTranslateFunction(req, res, next); @@ -88,7 +95,7 @@ describe('language middleware', () => { lang: 'es', }; - req.locals = { + res.locals = { user: { preferences: { language: 'it', @@ -108,7 +115,7 @@ describe('language middleware', () => { context('authorized request', () => { it('uses the user preferred language if avalaible', () => { - req.locals = { + res.locals = { user: { preferences: { language: 'it', @@ -122,7 +129,7 @@ describe('language middleware', () => { }); it('falls back to english if the user preferred language is not avalaible', done => { - req.locals = { + res.locals = { user: { preferences: { language: 'bla', @@ -138,7 +145,7 @@ describe('language middleware', () => { }); it('uses the user preferred language even if a session is included in request', () => { - req.locals = { + res.locals = { user: { preferences: { language: 'it', diff --git a/website/client/src/app.vue b/website/client/src/app.vue index 5b386e7a7f..ff1a6ec44b 100644 --- a/website/client/src/app.vue +++ b/website/client/src/app.vue @@ -33,7 +33,7 @@ 'resting': showRestingBanner }" > - + @@ -266,7 +266,6 @@ import { } from '@/libs/userlocalManager'; import svgClose from '@/assets/svg/close.svg'; -import bannedAccountModal from '@/components/bannedAccountModal'; const COMMUNITY_MANAGER_EMAIL = process.env.EMAILS_COMMUNITY_MANAGER_EMAIL; // eslint-disable-line @@ -281,7 +280,6 @@ export default { BuyModal, SelectMembersModal, amazonPaymentsModal, - bannedAccountModal, paymentsSuccessModal, subCancelModalConfirm, subCanceledModal, @@ -385,7 +383,8 @@ export default { return response; }, error => { if (error.response.status >= 400) { - this.checkForBannedUser(error); + const isBanned = this.checkForBannedUser(error); + if (isBanned === true) return null; // eslint-disable-line consistent-return // Don't show errors from getting user details. These users have delete their account, // but their chat message still exists. @@ -403,7 +402,8 @@ export default { // TODO use a specific error like NotificationNotFound instead of checking for the string const invalidUserMessage = [this.$t('invalidCredentials'), 'Missing authentication headers.']; if (invalidUserMessage.indexOf(errorMessage) !== -1) { - this.$store.dispatch('auth:logout'); + this.$store.dispatch('auth:logout', { redirectToLogin: true }); + return null; } // Most server errors should return is click to dismiss errors, with some exceptions @@ -553,7 +553,7 @@ export default { // Case where user is not logged in if (!parseSettings) { - return; + return false; } const bannedMessage = this.$t('accountSuspended', { @@ -561,9 +561,10 @@ export default { userId: parseSettings.auth.apiId, }); - if (errorMessage !== bannedMessage) return; + if (errorMessage !== bannedMessage) return false; - this.$root.$emit('bv::show::modal', 'banned-account'); + this.$store.dispatch('auth:logout', { redirectToLogin: true }); + return true; }, initializeModalStack () { // Manage modals diff --git a/website/client/src/store/actions/auth.js b/website/client/src/store/actions/auth.js index af909201b8..8d80da0ecd 100644 --- a/website/client/src/store/actions/auth.js +++ b/website/client/src/store/actions/auth.js @@ -82,7 +82,8 @@ export async function socialAuth (store, params) { localStorage.setItem(LOCALSTORAGE_AUTH_KEY, userLocalData); } -export function logout () { +export function logout (store, options = {}) { localStorage.clear(); - window.location.href = '/logout-server'; + const query = options.redirectToLogin === true ? '?redirectToLogin=true' : ''; + window.location.href = `/logout-server${query}`; } diff --git a/website/server/controllers/top-level/auth.js b/website/server/controllers/top-level/auth.js index ae8e08b87f..2d469960fe 100644 --- a/website/server/controllers/top-level/auth.js +++ b/website/server/controllers/top-level/auth.js @@ -28,7 +28,9 @@ api.logout = { async handler (req, res) { if (req.logout) req.logout(); // passportjs method req.session = null; - res.redirect('/'); + + const redirectUrl = req.query.redirectToLogin === 'true' ? '/login' : '/'; + res.redirect(redirectUrl); }, }; diff --git a/website/server/libs/i18n.js b/website/server/libs/i18n.js index 85f8c14880..7fd6069014 100644 --- a/website/server/libs/i18n.js +++ b/website/server/libs/i18n.js @@ -22,34 +22,10 @@ const momentLangsMapping = { }; export const approvedLanguages = [ - 'bg', - 'cs', - 'da', - 'de', - 'en', - 'en_GB', - 'en@pirate', - 'es', - 'es_419', - 'fr', - 'he', - 'hu', - 'id', - 'it', - 'ja', - 'nl', - 'pl', - 'pt', - 'pt_BR', - 'ro', - 'ru', - 'sk', - 'sr', - 'sv', - 'tr', - 'uk', - 'zh', - 'zh_TW', + 'bg', 'cs', 'da', 'de', 'en', 'en_GB', 'en@pirate', + 'es', 'es_419', 'fr', 'he', 'hu', 'id', 'it', + 'ja', 'nl', 'pl', 'pt', 'pt_BR', 'ro', 'ru', 'sk', + 'sr', 'sv', 'tr', 'uk', 'zh', 'zh_TW', ]; function _loadTranslations (locale) { diff --git a/website/server/libs/language.js b/website/server/libs/language.js new file mode 100644 index 0000000000..639698b25d --- /dev/null +++ b/website/server/libs/language.js @@ -0,0 +1,52 @@ +import accepts from 'accepts'; +import _ from 'lodash'; +import { + translations, + defaultLangCodes, + multipleVersionsLanguages, +} from './i18n'; + +function getUniqueListOfLanguages (languages) { + const acceptableLanguages = _(languages).map(lang => lang.slice(0, 2)).uniq().value(); + + const uniqueListOfLanguages = _.intersection(acceptableLanguages, defaultLangCodes); + + return uniqueListOfLanguages; +} + +function checkForApplicableLanguageVariant (originalLanguageOptions) { + const languageVariant = _.find(originalLanguageOptions, accepted => { + const trimmedAccepted = accepted.slice(0, 2); + + return multipleVersionsLanguages[trimmedAccepted]; + }); + + return languageVariant; +} + +export function getLanguageFromBrowser (req) { + const originalLanguageOptions = accepts(req).languages(); + const uniqueListOfLanguages = getUniqueListOfLanguages(originalLanguageOptions); + const baseLanguage = (uniqueListOfLanguages[0] || '').toLowerCase(); + const languageMapping = multipleVersionsLanguages[baseLanguage]; + + if (languageMapping) { + let languageVariant = checkForApplicableLanguageVariant(originalLanguageOptions); + + if (languageVariant) { + languageVariant = languageVariant.toLowerCase(); + } else { + return 'en'; + } + + return languageMapping[languageVariant] || baseLanguage; + } + return baseLanguage || 'en'; +} + +export function getLanguageFromUser (user, req) { + const preferredLang = user && user.preferences && user.preferences.language; + const lang = translations[preferredLang] ? preferredLang : getLanguageFromBrowser(req); + + return lang; +} diff --git a/website/server/middlewares/auth.js b/website/server/middlewares/auth.js index b9f680db34..5527a0d0fb 100644 --- a/website/server/middlewares/auth.js +++ b/website/server/middlewares/auth.js @@ -7,6 +7,8 @@ import { model as User, } from '../models/user'; import gcpStackdriverTracer from '../libs/gcpTraceAgent'; +import common from '../../common'; +import { getLanguageFromUser } from '../libs/language'; const COMMUNITY_MANAGER_EMAIL = nconf.get('EMAILS_COMMUNITY_MANAGER_EMAIL'); const USER_FIELDS_ALWAYS_LOADED = ['_id', 'notifications', 'preferences', 'auth', 'flags']; @@ -72,7 +74,17 @@ export function authWithHeaders (options = {}) { .exec() .then(user => { if (!user) throw new NotAuthorized(res.t('invalidCredentials')); - if (user.auth.blocked) throw new NotAuthorized(res.t('accountSuspended', { communityManagerEmail: COMMUNITY_MANAGER_EMAIL, userId: user._id })); + + if (user.auth.blocked) { + // We want the accountSuspended message to be translated but the language + // middleware hasn't run yet so we pick it manually + const language = getLanguageFromUser(user, req); + + throw new NotAuthorized(common.i18n.t('accountSuspended', { + communityManagerEmail: COMMUNITY_MANAGER_EMAIL, + userId: user._id, + }, language)); + } res.locals.user = user; req.session.userId = user._id; diff --git a/website/server/middlewares/language.js b/website/server/middlewares/language.js index 1c49130464..bb9bbacb0e 100644 --- a/website/server/middlewares/language.js +++ b/website/server/middlewares/language.js @@ -1,60 +1,15 @@ -import accepts from 'accepts'; -import _ from 'lodash'; import { model as User } from '../models/user'; import common from '../../common'; import { translations, - defaultLangCodes, - multipleVersionsLanguages, } from '../libs/i18n'; +import { + getLanguageFromUser, + getLanguageFromBrowser, +} from '../libs/language'; const { i18n } = common; -function _getUniqueListOfLanguages (languages) { - const acceptableLanguages = _(languages).map(lang => lang.slice(0, 2)).uniq().value(); - - const uniqueListOfLanguages = _.intersection(acceptableLanguages, defaultLangCodes); - - return uniqueListOfLanguages; -} - -function _checkForApplicableLanguageVariant (originalLanguageOptions) { - const languageVariant = _.find(originalLanguageOptions, accepted => { - const trimmedAccepted = accepted.slice(0, 2); - - return multipleVersionsLanguages[trimmedAccepted]; - }); - - return languageVariant; -} - -function _getFromBrowser (req) { - const originalLanguageOptions = accepts(req).languages(); - const uniqueListOfLanguages = _getUniqueListOfLanguages(originalLanguageOptions); - const baseLanguage = (uniqueListOfLanguages[0] || '').toLowerCase(); - const languageMapping = multipleVersionsLanguages[baseLanguage]; - - if (languageMapping) { - let languageVariant = _checkForApplicableLanguageVariant(originalLanguageOptions); - - if (languageVariant) { - languageVariant = languageVariant.toLowerCase(); - } else { - return 'en'; - } - - return languageMapping[languageVariant] || baseLanguage; - } - return baseLanguage || 'en'; -} - -function _getFromUser (user, req) { - const preferredLang = user && user.preferences && user.preferences.language; - const lang = translations[preferredLang] ? preferredLang : _getFromBrowser(req); - - return lang; -} - export function attachTranslateFunction (req, res, next) { res.t = function reqTranslation (...args) { return i18n.t(...args, req.language); @@ -64,26 +19,33 @@ export function attachTranslateFunction (req, res, next) { } export function getUserLanguage (req, res, next) { - if (req.query.lang) { // In case the language is specified in the request url, use it + // In case the language is specified in the request url, use intersection + if (req.query.lang) { req.language = translations[req.query.lang] ? req.query.lang : 'en'; return next(); + } // If the request is authenticated, use the user's preferred language - } if (req.locals && req.locals.user) { - req.language = _getFromUser(req.locals.user, req); + if (res.locals && res.locals.user) { + req.language = getLanguageFromUser(res.locals.user, req); return next(); - } if (req.session && req.session.userId) { // Same thing if the user has a valid session + } + + // Same thing if the user has a valid session + if (req.session && req.session.userId) { return User.findOne({ _id: req.session.userId, }, 'preferences.language') .lean() .exec() .then(user => { - req.language = _getFromUser(user, req); + req.language = getLanguageFromUser(user, req); return next(); }) .catch(next); - } // Otherwise get from browser - req.language = _getFromUser(null, req); + } + + // Otherwise get from browser + req.language = getLanguageFromBrowser(req); return next(); }