Skip to content

Commit

Permalink
Add support for allowlist setting and replace whitelist (#1196)
Browse files Browse the repository at this point in the history
Only keep allowlist in cache

Rename all references to whitelist

Re add method that was not unused
  • Loading branch information
OskarDamkjaer authored Sep 24, 2020
1 parent 2c249c8 commit adb0575
Show file tree
Hide file tree
Showing 15 changed files with 163 additions and 114 deletions.
4 changes: 2 additions & 2 deletions src/browser/modules/Stream/CypherScriptFrame/Summary.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
StyledCypherInfoMessage
} from 'browser/modules/Stream/styled'
import { MessageArea, PaddedStatsBar } from './styled'
import { whitelistedMultiCommands } from 'shared/modules/commands/commandsDuck'
import { allowlistedMultiCommands } from 'shared/modules/commands/commandsDuck'

const ucFirst = str => str[0].toUpperCase() + str.slice(1)

Expand Down Expand Up @@ -65,7 +65,7 @@ const GenericSummary = ({ status }) => {
<StyledCypherWarningMessage>WARNING</StyledCypherWarningMessage>
<MessageArea>
Only the commands{' '}
{whitelistedMultiCommands().map((cmd, i) => {
{allowlistedMultiCommands().map((cmd, i) => {
const maybeComma = i > 0 ? ', ' : ''
return [maybeComma, <code key={cmd}>{cmd}</code>]
})}{' '}
Expand Down
8 changes: 4 additions & 4 deletions src/browser/modules/Stream/PlayFrame.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import React, { useEffect, useState } from 'react'
import { withBus } from 'react-suber'
import { fetchGuideFromWhitelistAction } from 'shared/modules/commands/commandsDuck'
import { fetchGuideFromAllowlistAction } from 'shared/modules/commands/commandsDuck'

import Docs from '../Docs/Docs'
import docs from '../../documentation'
Expand Down Expand Up @@ -201,7 +201,7 @@ function generateContent(stackFrame, bus, onSlide, shouldUseSlidePointer) {
}
}

// Some other error. Whitelist error etc.
// Some other error. Allowlist error etc.
if (stackFrame.error && stackFrame.error.error) {
return {
guide: (
Expand Down Expand Up @@ -245,10 +245,10 @@ function generateContent(stackFrame, bus, onSlide, shouldUseSlidePointer) {
}
}

// Check whitelisted remote URLs for name matches
// Check allowlisted remote URLs for name matches
if (bus) {
const topicInput = (splitStringOnFirst(stackFrame.cmd, ' ')[1] || '').trim()
const action = fetchGuideFromWhitelistAction(topicInput)
const action = fetchGuideFromAllowlistAction(topicInput)
return new Promise(resolve => {
bus.self(action.type, action, res => {
if (!res.success) {
Expand Down
48 changes: 24 additions & 24 deletions src/shared/modules/commands/commandsDuck.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ import {
extractStatementsFromString
} from 'services/commandUtils'
import {
extractWhitelistFromConfigString,
extractAllowlistFromConfigString,
addProtocolsToUrlList,
firstSuccessPromise,
serialExecution,
resolveWhitelistWildcard
resolveAllowlistWildcard
} from 'services/utils'
import helper from 'services/commandInterpreterHelper'
import { addHistory } from '../history/historyDuck'
Expand All @@ -46,8 +46,8 @@ import { CONNECTION_SUCCESS } from '../connections/connectionsDuck'
import {
UPDATE_SETTINGS,
getAvailableSettings,
getRemoteContentHostnameWhitelist,
getDefaultRemoteContentHostnameWhitelist
getRemoteContentHostnameAllowlist,
getDefaultRemoteContentHostnameAllowlist
} from '../dbMeta/dbMetaDuck'
import { APP_START, USER_CLEAR } from 'shared/modules/app/appDuck'
import { add as addFrame } from 'shared/modules/stream/streamDuck'
Expand All @@ -63,15 +63,15 @@ export const CLEAR_ERROR_MESSAGE = `${NAME}/CLEAR_ERROR_MESSAGE`
export const CYPHER = `${NAME}/CYPHER`
export const CYPHER_SUCCEEDED = `${NAME}/CYPHER_SUCCEEDED`
export const CYPHER_FAILED = `${NAME}/CYPHER_FAILED`
export const FETCH_GUIDE_FROM_WHITELIST = `${NAME}FETCH_GUIDE_FROM_WHITELIST`
export const FETCH_GUIDE_FROM_ALLOWLIST = `${NAME}FETCH_GUIDE_FROM_ALLOWLIST`

export const useDbCommand = 'use'
export const listDbsCommand = 'dbs'
export const autoCommitTxCommand = 'auto'

const initialState = {}
export const getErrorMessage = state => state[NAME].errorMessage
export const whitelistedMultiCommands = () => [':param', ':use']
export const allowlistedMultiCommands = () => [':param', ':use']

export default function reducer(state = initialState, action) {
switch (action.type) {
Expand Down Expand Up @@ -142,8 +142,8 @@ export const clearErrorMessage = () => ({
export const cypher = query => ({ type: CYPHER, query })
export const successfulCypher = query => ({ type: CYPHER_SUCCEEDED, query })
export const unsuccessfulCypher = query => ({ type: CYPHER_FAILED, query })
export const fetchGuideFromWhitelistAction = url => ({
type: FETCH_GUIDE_FROM_WHITELIST,
export const fetchGuideFromAllowlistAction = url => ({
type: FETCH_GUIDE_FROM_ALLOWLIST,
url
})

Expand Down Expand Up @@ -187,13 +187,13 @@ export const handleCommandEpic = (action$, store) =>
const cleanCmd = cleanCommand(cmd)
const requestId = v4()
const cmdId = v4()
const whitelistedCommands = whitelistedMultiCommands()
const isWhitelisted = whitelistedCommands.some(wcmd =>
const allowlistedCommands = allowlistedMultiCommands()
const isAllowlisted = allowlistedCommands.some(wcmd =>
cleanCmd.startsWith(wcmd)
)

// Ignore client commands that aren't whitelisted
const ignore = cleanCmd.startsWith(cmdchar) && !isWhitelisted
// Ignore client commands that aren't allowlisted
const ignore = cleanCmd.startsWith(cmdchar) && !isAllowlisted

const { action, interpreted } = buildCommandObject(
{ cmd: cleanCmd, ignore },
Expand Down Expand Up @@ -276,26 +276,26 @@ export const postConnectCmdEpic = (some$, store) =>
.take(1)
)

export const fetchGuideFromWhitelistEpic = (some$, store) =>
some$.ofType(FETCH_GUIDE_FROM_WHITELIST).mergeMap(action => {
export const fetchGuideFromAllowlistEpic = (some$, store) =>
some$.ofType(FETCH_GUIDE_FROM_ALLOWLIST).mergeMap(action => {
if (!action.$$responseChannel || !action.url) {
return Rx.Observable.of({ type: 'NOOP' })
}
const whitelistStr = getRemoteContentHostnameWhitelist(store.getState())
const whitelist = extractWhitelistFromConfigString(whitelistStr)
const defaultWhitelist = extractWhitelistFromConfigString(
getDefaultRemoteContentHostnameWhitelist(store.getState())
const allowlistStr = getRemoteContentHostnameAllowlist(store.getState())
const allowlist = extractAllowlistFromConfigString(allowlistStr)
const defaultAllowlist = extractAllowlistFromConfigString(
getDefaultRemoteContentHostnameAllowlist(store.getState())
)
const resolvedWildcardWhitelist = resolveWhitelistWildcard(
whitelist,
defaultWhitelist
const resolvedWildcardAllowlist = resolveAllowlistWildcard(
allowlist,
defaultAllowlist
)
const urlWhitelist = addProtocolsToUrlList(resolvedWildcardWhitelist)
const guidesUrls = urlWhitelist.map(url => `${url}/${action.url}`)
const urlAllowlist = addProtocolsToUrlList(resolvedWildcardAllowlist)
const guidesUrls = urlAllowlist.map(url => `${url}/${action.url}`)

return firstSuccessPromise(guidesUrls, url => {
// Get first successful fetch
return fetchRemoteGuide(url, whitelistStr).then(r => ({
return fetchRemoteGuide(url, allowlistStr).then(r => ({
type: action.$$responseChannel,
success: true,
result: r
Expand Down
8 changes: 4 additions & 4 deletions src/shared/modules/commands/helpers/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
replace
} from 'shared/modules/settings/settingsDuck'
import { splitStringOnFirst } from 'services/commandUtils'
import { getRemoteContentHostnameWhitelist } from 'shared/modules/dbMeta/dbMetaDuck'
import { getRemoteContentHostnameAllowlist } from 'shared/modules/dbMeta/dbMetaDuck'
import { hostIsAllowed } from 'services/utils'
import { getJSON } from 'services/remote'
import { isValidURL } from 'shared/modules/commands/helpers/http'
Expand Down Expand Up @@ -72,11 +72,11 @@ export function handleUpdateConfigCommand(action, cmdchar, put, store) {
}
}
// It's an URL
const whitelist = getRemoteContentHostnameWhitelist(store.getState())
if (!hostIsAllowed(param, whitelist)) {
const allowlist = getRemoteContentHostnameAllowlist(store.getState())
if (!hostIsAllowed(param, allowlist)) {
// Make sure we're allowed to request entities on this host
return reject(
new Error('Hostname is not allowed according to server whitelist')
new Error('Hostname is not allowed according to server allowlist')
)
}
getJSON(param)
Expand Down
38 changes: 35 additions & 3 deletions src/shared/modules/commands/helpers/config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import nock from 'nock'
import * as config from './config'
import { update, replace } from 'shared/modules/settings/settingsDuck'
import dbMetaReducer, { updateSettings } from 'shared/modules/dbMeta/dbMetaDuck'

function FetchError(message) {
this.name = 'FetchError'
Expand All @@ -35,7 +36,7 @@ describe('commandsDuck config helper', () => {
return {
meta: {
settings: {
'browser.remote_content_hostname_whitelist': 'okurl.com'
'browser.remote_content_hostname_allowlist': 'okurl.com'
}
}
}
Expand Down Expand Up @@ -137,7 +138,7 @@ describe('commandsDuck config helper', () => {
expect(put).toHaveBeenCalledWith(replace({ x: 1, y: 2 }))
})
})
test('rejects hostnames not in whitelist', () => {
test('rejects hostnames not in allowlist', () => {
// Given
const action = { cmd: ':config https://bad.com/cnf.json' }
const cmdchar = ':'
Expand All @@ -149,7 +150,38 @@ describe('commandsDuck config helper', () => {
// Then
return expect(p)
.rejects.toEqual(
new Error('Hostname is not allowed according to server whitelist')
new Error('Hostname is not allowed according to server allowlist')
)
.then(() => expect(put).not.toHaveBeenCalled())
})
test('allowlist and whitelist both update allowlist', () => {
// Given
const action = { cmd: ':config https://okurl.com/cnf.json' }
const cmdchar = ':'
const put = jest.fn()

const updatedStore = {
getState: () => ({
meta: dbMetaReducer(
store.getState(),
updateSettings({
'browser.remote_content_hostname_whitelist': 'replaceUrl.com'
})
)
})
}
// When
const p = config.handleUpdateConfigCommand(
action,
cmdchar,
put,
updatedStore
)

// Then
return expect(p)
.rejects.toEqual(
new Error('Hostname is not allowed according to server allowlist')
)
.then(() => expect(put).not.toHaveBeenCalled())
})
Expand Down
6 changes: 3 additions & 3 deletions src/shared/modules/commands/helpers/grass.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
import { hostIsAllowed } from 'services/utils'
import remote from 'services/remote'

export const fetchRemoteGrass = (url, whitelist = null) => {
export const fetchRemoteGrass = (url, allowlist = null) => {
return new Promise((resolve, reject) => {
if (!hostIsAllowed(url, whitelist)) {
if (!hostIsAllowed(url, allowlist)) {
return reject(
new Error('Hostname is not allowed according to server whitelist')
new Error('Hostname is not allowed according to server allowlist')
)
}
resolve()
Expand Down
20 changes: 10 additions & 10 deletions src/shared/modules/commands/helpers/grass.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,20 @@ jest.mock('services/remote', () => {
})

describe('Grass remote fetch', () => {
test('should not fetch from url not in the whitelist', () => {
const whitelist = 'http://foo'
const urlNotInWhitelist = 'http://bar'
test('should not fetch from url not in the allowlist', () => {
const allowlist = 'http://foo'
const urlNotInAllowlist = 'http://bar'
return expect(
fetchRemoteGrass(urlNotInWhitelist, whitelist)
fetchRemoteGrass(urlNotInAllowlist, allowlist)
).rejects.toMatchObject(
new Error('Hostname is not allowed according to server whitelist')
new Error('Hostname is not allowed according to server allowlist')
)
})
test('should fetch from url in the whitelist', () => {
const whitelist = 'http://foo'
const urlInWhitelist = 'http://foo'
return expect(fetchRemoteGrass(urlInWhitelist, whitelist)).resolves.toBe(
urlInWhitelist
test('should fetch from url in the allowlist', () => {
const allowlist = 'http://foo'
const urlInAllowlist = 'http://foo'
return expect(fetchRemoteGrass(urlInAllowlist, allowlist)).resolves.toBe(
urlInAllowlist
)
})
})
Expand Down
6 changes: 3 additions & 3 deletions src/shared/modules/commands/helpers/play.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ import { hostIsAllowed } from 'services/utils'
import { cleanHtml } from 'services/remoteUtils'
import remote from 'services/remote'

export const fetchRemoteGuide = (url, whitelist = null) => {
export const fetchRemoteGuide = (url, allowlist = null) => {
return new Promise((resolve, reject) => {
if (!hostIsAllowed(url, whitelist)) {
if (!hostIsAllowed(url, allowlist)) {
return reject(
new Error('Hostname is not allowed according to server whitelist')
new Error('Hostname is not allowed according to server allowlist')
)
}
resolve()
Expand Down
Loading

0 comments on commit adb0575

Please sign in to comment.