Skip to content

Commit

Permalink
refactor: remove checkCorsConfig (#1955)
Browse files Browse the repository at this point in the history
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
  • Loading branch information
hacdias and SgtPooki committed Mar 14, 2022
1 parent cc06bcc commit a2289f4
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 92 deletions.
50 changes: 0 additions & 50 deletions src/daemon/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,55 +99,6 @@ function migrateConfig (ipfsd) {
store.set(REVISION_KEY, REVISION)
}

// Check for * and webui://- in allowed origins on API headers.
// The wildcard was a ipfsd-ctl default, that we don't want, and
// webui://- was an earlier experiement that should be cleared out.
//
// We remove them the first time we find them. If we find it again on subsequent
// runs then we leave them in, under the assumption that you really want it.
// TODO: show warning in UI when wildcard is in the allowed origins.
function checkCorsConfig (ipfsd) {
if (store.get('checkedCorsConfig')) {
// We've already checked so skip it.
return
}

let config = null

try {
config = readConfigFile(ipfsd)
} catch (err) {
// This is a best effort check, dont blow up here, that should happen else where.
// TODO: gracefully handle config errors elsewhere!
logger.error(`[daemon] checkCorsConfig: error reading config file: ${err.message || err}`)
return
}

if (config.API && config.API.HTTPHeaders && config.API.HTTPHeaders['Access-Control-Allow-Origin']) {
const allowedOrigins = config.API.HTTPHeaders['Access-Control-Allow-Origin']
const originsToRemove = ['*', 'webui://-']

if (Array.isArray(allowedOrigins)) {
const specificOrigins = allowedOrigins.filter(origin => !originsToRemove.includes(origin))

if (specificOrigins.length !== allowedOrigins.length) {
config.API.HTTPHeaders['Access-Control-Allow-Origin'] = specificOrigins

try {
writeConfigFile(ipfsd, config)
store.set('updatedCorsConfig', Date.now())
} catch (err) {
logger.error(`[daemon] checkCorsConfig: error writing config file: ${err.message || err}`)
// don't skip setting checkedCorsConfig so we try again next time time.
return
}
}
}
}

store.set('checkedCorsConfig', true)
}

const parseCfgMultiaddr = (addr) => (addr.includes('/http')
? multiaddr(addr)
: multiaddr(addr).encapsulate('/http')
Expand Down Expand Up @@ -325,6 +276,5 @@ module.exports = Object.freeze({
rmApiFile,
applyDefaults,
migrateConfig,
checkCorsConfig,
checkPorts
})
3 changes: 1 addition & 2 deletions src/daemon/daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const i18n = require('i18next')
const { showDialog } = require('../dialogs')
const logger = require('../common/logger')
const { getCustomBinary } = require('../custom-ipfs-binary')
const { applyDefaults, migrateConfig, checkCorsConfig, checkPorts, configExists, rmApiFile, apiFileExists } = require('./config')
const { applyDefaults, migrateConfig, checkPorts, configExists, rmApiFile, apiFileExists } = require('./config')
const showMigrationPrompt = require('./migration-prompt')

function cannotConnectDialog (addr) {
Expand Down Expand Up @@ -42,7 +42,6 @@ async function spawn ({ flags, path }) {

if (configExists(ipfsd)) {
migrateConfig(ipfsd)
checkCorsConfig(ipfsd)
return { ipfsd, isRemote: false }
}

Expand Down
40 changes: 0 additions & 40 deletions test/e2e/launch.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,46 +106,6 @@ test.describe.serial('Application launch', async () => {
expect(config.Discovery.MDNS.Enabled).toBeTruthy()
})

test('fixes cors config if access to "*" is granted', async () => {
// create config
const { repoPath, configPath, peerId: expectedId } = await makeRepository({ start: false })
let config = fs.readJsonSync(configPath)

// pretend someone set dangerous "*" (allowing global access to API)
// Note: '*' is the default when running ipfsd-ctl with test=true, but we set it here just to be sure
config.API.HTTPHeaders['Access-Control-Allow-Origin'] = ['*']
fs.writeJsonSync(configPath, config, { spaces: 2 })

const { app } = await startApp({ repoPath })
const { peerId } = await daemonReady(app)
expect(peerId).toBe(expectedId)

// ensure app has enabled cors checking
config = fs.readJsonSync(configPath)
expect(config.API.HTTPHeaders['Access-Control-Allow-Origin']).toEqual([])
})

test('fixes cors config with multiple allowed origins', async () => {
// create preexisting, initialized repo and config
const { repoPath, configPath, peerId: expectedId } = await makeRepository({ start: false })

// setup CORS config for the test
const initConfig = fs.readJsonSync(configPath)
// update origins to include multiple entries, including wildcard.
const newOrigins = ['https://webui.ipfs.io', '*']
initConfig.API.HTTPHeaders['Access-Control-Allow-Origin'] = newOrigins
fs.writeJsonSync(configPath, initConfig, { spaces: 2 })

const { app } = await startApp({ repoPath })
const { peerId } = await daemonReady(app)
expect(peerId).toBe(expectedId)

const config = fs.readJsonSync(configPath)
// ensure app has enabled cors checking
const specificOrigins = newOrigins.filter(origin => origin !== '*')
expect(config.API.HTTPHeaders['Access-Control-Allow-Origin']).toEqual(specificOrigins)
})

test('starts with repository with "IPFS_PATH/api" file and no daemon running', async () => {
// create "remote" repo
const { ipfsd } = await makeRepository({ start: true })
Expand Down

0 comments on commit a2289f4

Please sign in to comment.