fix(xo-server/remote.get{,All}): obfuscate sensitive values (#3687)

Fixes #3682

CHANGELOG.md

@@ -22,6 +22,7 @@
 - [Plugins] Don't expose credentials in config to users (PR [#3671](https://github.com/vatesfr/xen-orchestra/pull/3671))
 - [Self/New VM] `not enough … available in the set …` error in some cases (PR [#3667](https://github.com/vatesfr/xen-orchestra/pull/3667))
 - [Backup NG] Errors listing backups on SMB remotes with extraneous files (PR [#3685](https://github.com/vatesfr/xen-orchestra/pull/3685))
+- [Remotes] Don't expose credentials to users [#3682](https://github.com/vatesfr/xen-orchestra/issues/3682) (PR [#3687](https://github.com/vatesfr/xen-orchestra/pull/3687))
 
 ### Released packages
 

packages/xo-server/src/replace-sensitive-values.js

@@ -1,17 +0,0 @@
-import mapValues from 'lodash/mapValues'
-
-export default function replaceSensitiveValues (value, replacement) {
-  function helper (value, name) {
-    if (name === 'password' && typeof value === 'string') {
-      return replacement
-    }
-
-    if (typeof value !== 'object' || value === null) {
-      return value
-    }
-
-    return Array.isArray(value) ? value.map(helper) : mapValues(value, helper)
-  }
-
-  return helper(value)
-}

packages/xo-server/src/sensitive-values.js

@@ -0,0 +1,43 @@
+import mapValues from 'lodash/mapValues'
+
+// this random value is used to obfuscate real data
+const OBFUSCATED_VALUE = 'q3oi6d9X8uenGvdLnHk2'
+
+export const merge = (newValue, oldValue) => {
+  if (newValue === OBFUSCATED_VALUE) {
+    return oldValue
+  }
+
+  let isArray
+
+  if (
+    newValue === null ||
+    oldValue === null ||
+    typeof newValue !== 'object' ||
+    typeof oldValue !== 'object' ||
+    (isArray = Array.isArray(newValue)) !== Array.isArray(oldValue)
+  ) {
+    return newValue
+  }
+
+  const iteratee = (v, k) => merge(v, oldValue[k])
+  return isArray ? newValue.map(iteratee) : mapValues(newValue, iteratee)
+}
+
+export const obfuscate = value => replace(value, OBFUSCATED_VALUE)
+
+export function replace (value, replacement) {
+  function helper (value, name) {
+    if (name === 'password' && typeof value === 'string') {
+      return replacement
+    }
+
+    if (typeof value !== 'object' || value === null) {
+      return value
+    }
+
+    return Array.isArray(value) ? value.map(helper) : mapValues(value, helper)
+  }
+
+  return helper(value)
+}

packages/xo-server/src/xo-mixins/api.js

@@ -6,7 +6,7 @@ import { forEach, isFunction } from 'lodash'
 import { MethodNotFound } from 'json-rpc-peer'
 
 import * as methods from '../api'
-import replaceSensitiveValues from '../replace-sensitive-values'
+import * as sensitiveValues from '../sensitive-values'
 import { noop, serializeError } from '../utils'
 
 import * as errors from 'xo-common/api-errors'
@@ -276,7 +276,7 @@ export default class Api {
       const data = {
         userId,
         method: name,
-        params: replaceSensitiveValues(params, '* obfuscated *'),
+        params: sensitiveValues.replace(params, '* obfuscated *'),
         duration: Date.now() - startTime,
         error: serializeError(error),
       }

packages/xo-server/src/xo-mixins/plugins.js

@@ -1,9 +1,8 @@
 import Ajv from 'ajv'
 import createLogger from '@xen-orchestra/log'
 import { invalidParameters, noSuchObject } from 'xo-common/api-errors'
-import { mapValues } from 'lodash'
 
-import replaceSensitiveValues from '../replace-sensitive-values'
+import * as sensitiveValues from '../sensitive-values'
 import { PluginsMetadata } from '../models/plugin-metadata'
 import { isFunction, mapToArray } from '../utils'
 
@@ -11,30 +10,6 @@ import { isFunction, mapToArray } from '../utils'
 
 const log = createLogger('xo:xo-mixins:plugins')
 
-// this random value is used to obfuscate real data
-const OBFUSCATED_VALUE = 'c<,R"/|+9[-&|/pI!/}'
-
-const replaceObfuscatedValues = (newConfig, currentConfig) => {
-  if (newConfig === OBFUSCATED_VALUE) {
-    return currentConfig
-  }
-
-  let isArray
-
-  if (
-    newConfig === null ||
-    currentConfig === null ||
-    typeof newConfig !== 'object' ||
-    typeof currentConfig !== 'object' ||
-    (isArray = Array.isArray(newConfig)) !== Array.isArray(currentConfig)
-  ) {
-    return newConfig
-  }
-
-  const iteratee = (v, k) => replaceObfuscatedValues(v, currentConfig[k])
-  return isArray ? newConfig.map(iteratee) : mapValues(newConfig, iteratee)
-}
-
 export default class {
   constructor (xo) {
     this._ajv = new Ajv({
@@ -145,7 +120,7 @@ export default class {
       loaded,
       unloadable,
       version,
-      configuration: replaceSensitiveValues(configuration, OBFUSCATED_VALUE),
+      configuration: sensitiveValues.obfuscate(configuration),
       configurationPresets,
       configurationSchema,
       testable,
@@ -194,7 +169,7 @@ export default class {
     const metadata = await this._getPluginMetadata()
 
     if (metadata !== undefined) {
-      configuration = replaceObfuscatedValues(
+      configuration = sensitiveValues.merge(
         configuration,
         metadata.configuration
       )

packages/xo-server/src/xo-mixins/remotes.js

@@ -1,14 +1,21 @@
 import synchronized from 'decorator-synchronized'
+import { format, parse } from 'xo-remote-parser'
 import { getHandler } from '@xen-orchestra/fs'
-import { noSuchObject } from 'xo-common/api-errors'
 import { ignoreErrors } from 'promise-toolbox'
+import { noSuchObject } from 'xo-common/api-errors'
 
+import * as sensitiveValues from '../sensitive-values'
 import patch from '../patch'
 import { mapToArray } from '../utils'
 import { Remotes } from '../models/remote'
 
 // ===================================================================
 
+const obfuscateRemote = ({ url, ...remote }) => {
+  remote.url = format(sensitiveValues.obfuscate(parse(url)))
+  return remote
+}
+
 export default class {
   constructor (xo, { remoteOptions }) {
     this._remoteOptions = remoteOptions
@@ -30,7 +37,7 @@ export default class {
           )
       )
 
-      const remotes = await this.getAllRemotes()
+      const remotes = await this._remotes.get()
       remotes.forEach(remote => {
         ignoreErrors.call(this.updateRemote(remote.id, {}))
       })
@@ -47,7 +54,7 @@ export default class {
 
   async getRemoteHandler (remote) {
     if (typeof remote === 'string') {
-      remote = await this.getRemote(remote)
+      remote = await this._getRemote(remote)
     }
 
     if (!remote.enabled) {
@@ -78,10 +85,10 @@ export default class {
   }
 
   async getAllRemotes () {
-    return this._remotes.get()
+    return (await this._remotes.get()).map(_ => obfuscateRemote(_))
   }
 
-  async getRemote (id) {
+  async _getRemote (id) {
     const remote = await this._remotes.first(id)
     if (remote === undefined) {
       throw noSuchObject(id, 'remote')
@@ -89,6 +96,10 @@ export default class {
     return remote.properties
   }
 
+  getRemote (id) {
+    return this._getRemote(id).then(obfuscateRemote)
+  }
+
   async createRemote ({ name, url, options }) {
     const params = {
       name,
@@ -120,9 +131,16 @@ export default class {
   }
 
   @synchronized()
-  async _updateRemote (id, props) {
-    const remote = await this.getRemote(id)
+  async _updateRemote (id, { url, ...props }) {
+    const remote = await this._getRemote(id)
+
+    // url is handled separately to take care of obfuscated values
+    if (typeof url === 'string') {
+      remote.url = format(sensitiveValues.merge(parse(url), parse(remote.url)))
+    }
+
     patch(remote, props)
+
     return (await this._remotes.update(remote)).properties
   }