From 8d4f87a95d17d70999bf822531eb7ef459026bb1 Mon Sep 17 00:00:00 2001 From: gaba Date: Mon, 6 Feb 2017 13:11:39 -0800 Subject: [PATCH 1/4] Domain List. --- models/setting.js | 10 ++++ services/assets.js | 8 +++ services/domainlist.js | 112 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 services/domainlist.js diff --git a/models/setting.js b/models/setting.js index 25882b3f8..f6eff613a 100644 --- a/models/setting.js +++ b/models/setting.js @@ -62,6 +62,16 @@ const SettingSchema = new Schema({ requireEmailConfirmation: { type: Boolean, default: false + }, + domains: { + whitelist: { + type: Array, + default: ['localhost'] + }, + enable: { + type: Boolean, + default: true + } } }, { timestamps: { diff --git a/services/assets.js b/services/assets.js index 24a161865..e6d2e32d4 100644 --- a/services/assets.js +++ b/services/assets.js @@ -1,5 +1,7 @@ const AssetModel = require('../models/asset'); const SettingsService = require('./settings'); +const domainlist = require('./domainlist'); +const errors = require('../errors'); module.exports = class AssetsService { @@ -53,6 +55,12 @@ module.exports = class AssetsService { * @return {Promise} */ static findOrCreateByUrl(url) { + + // Check the URL to confirm that is in the domain whitelist + if (!domainlist.urlCheck(url)) { + return Promise.reject(errors.ErrInvalidAssetURL); + } + return AssetModel.findOneAndUpdate({url}, {url}, { // Ensure that if it's new, we return the new object created. diff --git a/services/domainlist.js b/services/domainlist.js new file mode 100644 index 000000000..3d5568fda --- /dev/null +++ b/services/domainlist.js @@ -0,0 +1,112 @@ +const debug = require('debug')('talk:services:domainlist'); +const _ = require('lodash'); +const SettingsService = require('./settings'); + +/** + * The root domainlist object. + * @type {Object} + */ +class Domainlist { + + constructor() { + this.lists = { + whitelist: [], + }; + } + + /** + * Loads domains white list in from the database + */ + load() { + return SettingsService + .retrieve() + .then((settings) => { + + // Insert the settings domains whitelist. + this.upsert(settings.domains); + }); + } + + /** + * Inserts the domains whitelist data + * @param {Array} list list of domains to be set to the whitelist + */ + upsert(lists) { + + // Add the domains to this array and also be sure are all unique domains + if (!('whitelist' in lists)) { + return; + } + + this.lists['whitelist'] = Domainlist.parseList(lists['whitelist']); + debug(`Added ${lists['whitelist'].length} domains to the whitelist.`); + + return Promise.resolve(this); + } + + /** + * Tests the url to see if it contains any of the whitelisted domains. + * @param {String} url value to match. + * @return {Boolean} true if the url contains any of the domains, false otherwise. + */ + match(list, url) { + + const domainToMatch = Domainlist.parseURL(url); + + // This will return true in the event that at least one blockword is found + // in the phrase. + return list.every((domain) => { + + // Not considering wildcards for now. + if (domain === domainToMatch) { + return true; + } + + // We've walked over all the whitelisted domains, and haven't had a + // mismatch... It is not an allowed domain! + return false; + }); + } + + /** + * Parses the list content. + * @param {Array} list array of domains to parse for a list. + * @return {Array} the parsed list + */ + static parseList(list) { + return _.uniq(list.map((domain) => Domainlist.parseURL(domain))); + } + + /** + * Parses the URL. + * @param {String} url url to parse for a domain. + * @return {String} the domain + */ + static parseURL(url){ + let domain; + + // removes protocol and get domain + if (url.indexOf('://') > -1) { + domain = url.split('/')[2]; + } else { + domain = url.split('/')[0]; + } + + // remove port number + domain = domain.split(':')[0]; + + return domain.toLowerCase(); + } + + static urlCheck(url) { + const dl = new Domainlist(); + + return dl.load() + .then(() => { + return dl.match(dl.lists['whitelist'], url); + }); + } + +} + +module.exports = Domainlist; From d08a9fb7b20dc4742fa2f94d7661de6adb983bc6 Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 7 Feb 2017 13:07:22 -0800 Subject: [PATCH 2/4] Front End for Domain Whitelist. --- client/coral-admin/src/actions/settings.js | 5 ++++ .../src/containers/Configure/Configure.js | 16 +++++++++++- .../src/containers/Configure/Domainlist.js | 26 +++++++++++++++++++ client/coral-admin/src/reducers/settings.js | 8 ++++++ client/coral-admin/src/translations.json | 11 ++++++-- models/setting.js | 4 --- 6 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 client/coral-admin/src/containers/Configure/Domainlist.js diff --git a/client/coral-admin/src/actions/settings.js b/client/coral-admin/src/actions/settings.js index 85ad5149e..32f56e111 100644 --- a/client/coral-admin/src/actions/settings.js +++ b/client/coral-admin/src/actions/settings.js @@ -11,6 +11,7 @@ export const SAVE_SETTINGS_SUCCESS = 'SAVE_SETTINGS_SUCCESS'; export const SAVE_SETTINGS_FAILED = 'SAVE_SETTINGS_FAILED'; export const WORDLIST_UPDATED = 'WORDLIST_UPDATED'; +export const DOMAINLIST_UPDATED = 'DOMAINLIST_UPDATED'; export const fetchSettings = () => dispatch => { dispatch({type: SETTINGS_LOADING}); @@ -33,6 +34,10 @@ export const updateWordlist = (listName, list) => { return {type: WORDLIST_UPDATED, listName, list}; }; +export const updateDomainlist = (listName, list) => { + return {type: DOMAINLIST_UPDATED, listName, list}; +}; + export const saveSettingsToServer = () => (dispatch, getState) => { let settings = getState().settings.toJS().settings; if (settings.charCount) { diff --git a/client/coral-admin/src/containers/Configure/Configure.js b/client/coral-admin/src/containers/Configure/Configure.js index 8ad6cacfd..c9c88eeee 100644 --- a/client/coral-admin/src/containers/Configure/Configure.js +++ b/client/coral-admin/src/containers/Configure/Configure.js @@ -5,6 +5,7 @@ import { updateSettings, saveSettingsToServer, updateWordlist, + updateDomainlist } from '../../actions/settings'; import {Button, List, Item} from 'coral-ui'; @@ -14,6 +15,7 @@ import translations from '../../translations.json'; import EmbedLink from './EmbedLink'; import CommentSettings from './CommentSettings'; import Wordlist from './Wordlist'; +import Domainlist from './Domainlist'; import has from 'lodash/has'; class Configure extends Component { @@ -47,6 +49,11 @@ class Configure extends Component { this.props.dispatch(updateWordlist(listName, list)); } + onChangeDomainlist = (listName, list) => { + this.setState({changed: true}); + this.props.dispatch(updateDomainlist(listName, list)); + } + onSettingUpdate = (setting) => { this.setState({changed: true}); this.props.dispatch(updateSettings(setting)); @@ -73,7 +80,14 @@ class Configure extends Component { errors={this.state.errors} settingsError={this.onSettingError}/>; case 'embed': - return ; + return has(this, 'props.settings.domains.whitelist') + ?
+ + +
+ : ; case 'wordlist': return has(this, 'props.settings.wordlist') ? ( +
+

{lang.t('configure.domain-list-title')}

+ +

{lang.t('configure.domain-list-text')}

+ data.split(',').map(d => d.trim())} + onChange={tags => onChangeDomainlist('whitelist', tags)} + /> + +
+); + +export default Domainlist; + +const lang = new I18n(translations); diff --git a/client/coral-admin/src/reducers/settings.js b/client/coral-admin/src/reducers/settings.js index 12b16d9ad..4f743bc0a 100644 --- a/client/coral-admin/src/reducers/settings.js +++ b/client/coral-admin/src/reducers/settings.js @@ -6,6 +6,9 @@ const initialState = Map({ wordlist: Map({ banned: List(), suspect: List() + }), + domains: Map({ + whitelist: List() }) }), saveSettingsError: null, @@ -24,6 +27,7 @@ export default (state = initialState, action) => { case types.SAVE_SETTINGS_SUCCESS: return saveComplete(state, action); case types.SAVE_SETTINGS_FAILED: return settingsSaveFailed(state, action); case types.WORDLIST_UPDATED: return updateWordlist(state, action); + case types.DOMAINLIST_UPDATED: return updateDomainlist(state, action); default: return state; } }; @@ -40,6 +44,10 @@ const updateWordlist = (state, action) => { return state.setIn(['settings', 'wordlist', action.listName], action.list); }; +const updateDomainlist = (state, action) => { + return state.setIn(['settings', 'domains', action.listName], action.list); +}; + const saveComplete = (state, action) => { const s = state.set('fetchingSettings', false).set('saveSettingsError', null); const settings = s.get('settings').merge(action.settings); diff --git a/client/coral-admin/src/translations.json b/client/coral-admin/src/translations.json index a4256848e..8ecf55449 100644 --- a/client/coral-admin/src/translations.json +++ b/client/coral-admin/src/translations.json @@ -79,7 +79,9 @@ "comment-count-header": "Limit Comment Length", "comment-count-text-pre": "Comments will be limited to ", "comment-count-text-post": " characters.", - "comment-count-error": "Please enter a valid number." + "comment-count-error": "Please enter a valid number.", + "domain-list-title": "Domain Whitelist", + "domain-list-text": "Some instructions on how to type the urls." }, "bandialog": { "ban_user": "Ban User?", @@ -187,7 +189,12 @@ "comment-count-header": "Limitar el largo del comentario", "comment-count-text-pre": "El largo de comentarios será ", "comment-count-text-post": " caracteres", - "comment-count-error": "Por favor escribe un número válido." + "comment-count-error": "Por favor escribe un número válido.", + "domain-list-title": "Lista de Dominios Permitidos", + "domain-list-text": "Instrucciones de como ingresar las URLs." + }, + "embedlink": { + "copy": "Copiar" }, "bandialog": { "ban_user": "Quieres suspender el Usuario?", diff --git a/models/setting.js b/models/setting.js index f6eff613a..adbf60d83 100644 --- a/models/setting.js +++ b/models/setting.js @@ -67,10 +67,6 @@ const SettingSchema = new Schema({ whitelist: { type: Array, default: ['localhost'] - }, - enable: { - type: Boolean, - default: true } } }, { From 3effd11ad2a2db5756959ac55d31efd109f19114 Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 7 Feb 2017 13:57:46 -0800 Subject: [PATCH 3/4] Adding test. --- services/domainlist.js | 14 +++++----- test/services/domainlist.js | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 test/services/domainlist.js diff --git a/services/domainlist.js b/services/domainlist.js index 3d5568fda..29673e2b6 100644 --- a/services/domainlist.js +++ b/services/domainlist.js @@ -55,17 +55,15 @@ class Domainlist { // This will return true in the event that at least one blockword is found // in the phrase. - return list.every((domain) => { - - // Not considering wildcards for now. - if (domain === domainToMatch) { + for (let i = 0; i < list.length; i++) { + if (list[i] === domainToMatch) { return true; } + } - // We've walked over all the whitelisted domains, and haven't had a - // mismatch... It is not an allowed domain! - return false; - }); + // We've walked over all the whitelisted domains, and haven't had a + // mismatch... It is not an allowed domain! + return false; } /** diff --git a/test/services/domainlist.js b/test/services/domainlist.js new file mode 100644 index 000000000..db53b77b4 --- /dev/null +++ b/test/services/domainlist.js @@ -0,0 +1,52 @@ +const expect = require('chai').expect; +const Domainlist = require('../../services/domainlist'); +const SettingsService = require('../../services/settings'); + +describe('services.Domainlist', () => { + + const domainlists = { + whitelist: [ + 'nytimes.com', + 'wapo.com' + ] + }; + + let domainlist = new Domainlist(); + const settings = {id: '1', moderation: 'PRE', domainlist: {whitelist: ['nytimes.com', 'wapo.com']}}; + + beforeEach(() => SettingsService.init(settings)); + + describe('#init', () => { + + before(() => domainlist.upsert(domainlists)); + + it('has entries', () => { + expect(domainlist.lists.whitelist).to.not.be.empty; + }); + + }); + + describe('#match', () => { + + const whiteList = Domainlist.parseList(domainlists['whitelist']); + + it('does match on an included domain', () => { + [ + 'wapo.com', + 'nytimes.com' + ].forEach((domain) => { + expect(domainlist.match(whiteList, domain)).to.be.true; + }); + }); + + it('does not match on a not included domain', () => { + [ + 'badsite.com', + 'www.badsite.com', + 'otherexample.com' + ].forEach((domain) => { + expect(domainlist.match(whiteList, domain)).to.be.false; + }); + }); + }); +}); From 1176a94becb4970767edda4821be725077305edb Mon Sep 17 00:00:00 2001 From: gaba Date: Tue, 7 Feb 2017 15:36:10 -0800 Subject: [PATCH 4/4] Update tests --- services/assets.js | 24 +++++++++++++----------- test/routes/api/assets/index.js | 33 +++++++++++++++++++-------------- test/services/assets.js | 20 ++++++++++++++++++-- 3 files changed, 50 insertions(+), 27 deletions(-) diff --git a/services/assets.js b/services/assets.js index e6d2e32d4..f8f795263 100644 --- a/services/assets.js +++ b/services/assets.js @@ -57,20 +57,22 @@ module.exports = class AssetsService { static findOrCreateByUrl(url) { // Check the URL to confirm that is in the domain whitelist - if (!domainlist.urlCheck(url)) { - return Promise.reject(errors.ErrInvalidAssetURL); - } + return domainlist.urlCheck(url).then((whitelisted) => { + if (!whitelisted) { + return Promise.reject(errors.ErrInvalidAssetURL); + } else { + return AssetModel.findOneAndUpdate({url}, {url}, { - return AssetModel.findOneAndUpdate({url}, {url}, { + // Ensure that if it's new, we return the new object created. + new: true, - // Ensure that if it's new, we return the new object created. - new: true, + // Perform an upsert in the event that this doesn't exist. + upsert: true, - // Perform an upsert in the event that this doesn't exist. - upsert: true, - - // Set the default values if not provided based on the mongoose models. - setDefaultsOnInsert: true + // Set the default values if not provided based on the mongoose models. + setDefaultsOnInsert: true + }); + } }); } diff --git a/test/routes/api/assets/index.js b/test/routes/api/assets/index.js index 56c20fc78..9f1e4ad66 100644 --- a/test/routes/api/assets/index.js +++ b/test/routes/api/assets/index.js @@ -10,24 +10,29 @@ chai.use(require('chai-http')); const AssetModel = require('../../../../models/asset'); const AssetsService = require('../../../../services/assets'); +const SettingsService = require('../../../../services/settings'); describe('/api/v1/assets', () => { beforeEach(() => { - return AssetModel.create([ - { - url: 'https://coralproject.net/news/asset1', - title: 'Asset 1', - description: 'term1', - closedAt: Date.now() - }, - { - url: 'https://coralproject.net/news/asset2', - title: 'Asset 2', - description: 'term2', - closedAt: null - } - ]); + const settings = {id: '1', moderation: 'PRE', domains: {whitelist: ['test.com']}}; + + return SettingsService.init(settings).then(() => { + return AssetModel.create([ + { + url: 'https://coralproject.net/news/asset1', + title: 'Asset 1', + description: 'term1', + closedAt: Date.now() + }, + { + url: 'https://coralproject.net/news/asset2', + title: 'Asset 2', + description: 'term2', + closedAt: null + } + ]); + }); }); describe('#get', () => { diff --git a/test/services/assets.js b/test/services/assets.js index ff8d9e175..3bbe8ee6e 100644 --- a/test/services/assets.js +++ b/test/services/assets.js @@ -1,5 +1,6 @@ const AssetModel = require('../../models/asset'); const AssetsService = require('../../services/assets'); +const SettingsService = require('../../services/settings'); const chai = require('chai'); const expect = chai.expect; @@ -10,8 +11,12 @@ chai.should(); describe('services.AssetsService', () => { beforeEach(() => { + const settings = {id: '1', moderation: 'PRE', domains: {whitelist: ['new.test.com', 'test.com', 'override.test.com']}}; const defaults = {url:'http://test.com'}; - return AssetModel.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); + + return SettingsService.init(settings).then(() => { + return AssetModel.update({id: '1'}, {$setOnInsert: defaults}, {upsert: true}); + }); }); describe('#findById', ()=> { @@ -54,7 +59,7 @@ describe('services.AssetsService', () => { }); }); - it('should return a new asset when the url does not exist', () => { + it('should return a new asset when the url does not exist and its domain is whitelisted', () => { return AssetsService .findOrCreateByUrl('http://new.test.com') .then((asset) => { @@ -62,6 +67,17 @@ describe('services.AssetsService', () => { .and.to.not.equal(1); }); }); + + it('should return an error when the url does not exist and its domain is not whitelisted', () => { + return AssetsService + .findOrCreateByUrl('http://bad.test.com') + .then((asset) => { + expect(asset).to.be.null; + }) + .catch((error) => { + expect(error).to.not.be.null; + }); + }); }); describe('#overrideSettings', () => {