feat: add ability to create a credential array

Users can now specify a credential pool by adding "_ARRAY" to the
environment variable name, and providing a comma-separated list of
credentials as the value, which will get turned into an array
internally. For example, to provide multiple GitHub tokens, you would
do:

GITHUB_TOKEN_ARRAY=token1, token2, token3, ...

This will then create an array, and anywhere that GITHUB_TOKEN is
referenced (e.g., in accept.json) it will instead pick a token from that
list (picking the first then second then third then back to the
beginning, etc).

It will also create an array of any other variables that reference it,
like BROKER_CLIENT_VALIDATION_AUTHORIZATION_HEADER - it will create a
BROKER_CLIENT_VALIDATION_AUTHORIZATION_HEADER_ARRAY value, which is
used by the systemcheck endpoint.

This includes a breaking change to the /systemcheck endpoint as it now
returns a JSON array rather than an object - this is to accommodate
checking multiple credentials at once.

.editorconfig

@@ -4,3 +4,4 @@ indent_style = space
 end_of_line = lf
 insert_final_newline = true
 trim_trailing_whitespace = true
+indent_size = 2

README.md

@@ -497,6 +497,59 @@ For example, if you are using GitHub and you would like to give the Broker acces
 More details can be found here:
 [Detecting infrastructure as code files using a broker](https://docs.snyk.io/products/snyk-infrastructure-as-code/detecting-infrastructure-as-code-files-using-a-broker)
 
+### Credential Pooling
+Under some circumstances it can be desirable to create a "pool" of credentials, e.g., to work around rate-limiting issues.
+This can be achieved by creating an environment variable ending in `_ARRAY`, separate each credential with a comma, and
+the Broker Client will then, when doing variable replacement, look to see if the variable in use has a variant with an
+`_ARRAY` suffix, and use the next item in that array if so. For example, if you have set the environment variable
+`GITHUB_TOKEN`, but want to provide multiple tokens, you would just do this:
+
+```shell
+GITHUB_TOKEN_ARRAY=token1, token2, token3
+```
+
+And then the Broker Server would, any time it needed `GITHUB_TOKEN`, instead take an item from the `GITHUB_TOKEN_ARRAY`.
+
+Credentials will be taken in a round-robin fashion, so the first, the second, the third, etc, etc, until it reaches the end
+and then takes the first one again.
+
+Calling the `/systemcheck` endpoint will validate all credentials, in order, and will return an array where the first item
+is the first credential and so on. For example, if you were running the GitHub Client and had this:
+
+```shell
+GITHUB_TOKEN_ARRAY=good_token, bad_token
+```
+
+The `/systemcheck` endpoint would return the following, where the first object is for `good_token` and the second for
+`bad_token`:
+
+```json
+[
+  {
+    "brokerClientValidationUrl": "https://api.github.com/user",
+    "brokerClientValidationMethod": "GET",
+    "brokerClientValidationTimeoutMs": 5000,
+    "ok": true
+  },
+  {
+    "brokerClientValidationUrl": "https://api.github.com/user",
+    "brokerClientValidationMethod": "GET",
+    "brokerClientValidationTimeoutMs": 5000,
+    "ok": false,
+    "error": "401 - {\"message\":\"Bad credentials\",\"documentation_url\":\"https://docs.github.com/rest\"}"
+  }
+]
+```
+The actual credentials are not included to avoid exposing sensitive data accidentally.
+
+#### Limitations
+Credential validity is not checked before using a credential, nor are invalid credentials removed from the pool, so it is
+_strongly_ recommended that credentials be used exclusively by the Broker Client to avoid credentials reaching rate limits
+at different times, and that the `/systemcheck` endpoint be called before use.
+
+Some providers, such as GitHub, do rate-limiting on a per-user basis, not a per-token or per-credential basis, and in those
+cases you will need to create multiple accounts with one credential per account.
+
 ### Custom approved-listing filter
 
 The default approved-listing filter supports the bare minimum to operate on all repositories supported by Snyk. In order to customize the approved-listing filter, create the default one locally by installing `snyk-broker` and running `broker init [Git type]`. The created `accept.json` is the default filter for the chosen Git. Place the file in a separate folder such as `./private/accept.json`, and provide it to the docker container by mounting the folder and using the `ACCEPT` environment variable:

lib/client/index.js

@@ -1,12 +1,12 @@
 const primus = require('primus');
-const request = require('request');
+const rp = require('request-promise-native');
 const socket = require('./socket');
 const relay = require('../relay');
 const logger = require('../log');
 const version = require('../version');
 
-module.exports = ({ port = null, config = {}, filters = {} }) => {
-  logger.info({ version }, 'running in client mode');
+module.exports = ({port = null, config = {}, filters = {}}) => {
+  logger.info({version}, 'running in client mode');
 
   const identifyingMetadata = {
     version,
@@ -22,7 +22,7 @@ module.exports = ({ port = null, config = {}, filters = {} }) => {
   });
 
   // start the local webserver to listen for relay requests
-  const { app, server } = require('../webserver')(config, port);
+  const {app, server} = require('../webserver')(config, port);
 
   // IMPORTANT: defined before relay (`app.all('/*', ...`)
   app.get(config.brokerHealthcheckPath || '/healthcheck', (req, res) => {
@@ -41,7 +41,7 @@ module.exports = ({ port = null, config = {}, filters = {} }) => {
     return res.status(status).json(data);
   });
 
-  app.get(config.brokerSystemcheckPath || '/systemcheck', (req, res) => {
+  app.get(config.brokerSystemcheckPath || '/systemcheck', async (req, res) => {
     // Systemcheck is the broker client's ability to assert the network
     // reachability and some correctness of credentials for the service
     // being proxied by the broker client.
@@ -52,74 +52,42 @@ module.exports = ({ port = null, config = {}, filters = {} }) => {
       config.brokerClientValidationTimeoutMs || 5000;
     const isJsonResponse = !config.brokerClientValidationJsonDisabled;
 
-    const data = {
-      brokerClientValidationUrl: logger.sanitise(
-        config.brokerClientValidationUrl,
-      ),
-      brokerClientValidationMethod,
-      brokerClientValidationTimeoutMs,
-    };
-
-    const validationRequestHeaders = {
-      'user-agent': 'Snyk Broker client ' + version,
-    };
-
     // set auth header according to config
-    if (config.brokerClientValidationAuthorizationHeader) {
-      validationRequestHeaders.authorization =
-        config.brokerClientValidationAuthorizationHeader;
+    let auths = [];
+    if (config.brokerClientValidationAuthorizationHeaderArray) {
+      auths = config.brokerClientValidationAuthorizationHeaderArray;
+    } else if (config.brokerClientValidationBasicAuthArray) {
+      auths = config.brokerClientValidationBasicAuthArray.map(s => `Basic ${Buffer.from(s).toString('base64')}`)
+    } else if (config.brokerClientValidationAuthorizationHeader) {
+      auths.push(config.brokerClientValidationAuthorizationHeader);
     } else if (config.brokerClientValidationBasicAuth) {
-      validationRequestHeaders.authorization = `Basic ${Buffer.from(
-        config.brokerClientValidationBasicAuth,
-      ).toString('base64')}`;
+      auths.push(`Basic ${Buffer.from(config.brokerClientValidationBasicAuth).toString('base64')}`)
     }
 
     // make the internal validation request
-    request(
-      {
-        url: config.brokerClientValidationUrl,
-        headers: validationRequestHeaders,
-        method: brokerClientValidationMethod,
-        timeout: brokerClientValidationTimeoutMs,
-        json: isJsonResponse,
-        agentOptions: {
-          ca: config.caCert, // Optional CA cert
-        },
-      },
-      (error, response) => {
-        // test logic requires to surface internal data
-        // which is best not exposed in production
-        if (process.env.TAP) {
-          data.testError = error;
-          data.testResponse = response;
-        }
-
-        if (error) {
-          data.ok = false;
-          data.error = error.message;
-          return res.status(500).json(data);
-        }
-
-        const responseStatusCode = response && response.statusCode;
-        data.brokerClientValidationUrlStatusCode = responseStatusCode;
-
-        // check for 2xx status code
-        const goodStatusCode = /^2/.test(responseStatusCode);
-        if (!goodStatusCode) {
-          data.ok = false;
-          data.error =
-            responseStatusCode === 401 || responseStatusCode === 403
-              ? 'Failed due to invalid credentials'
-              : 'Status code is not 2xx';
-
-          logger.error(data, response && response.body, 'Systemcheck failed');
-          return res.status(500).json(data);
-        }
+    const rv = [];
+    let errorOccurred = true;
+    if (auths.length > 0) {
+      for (let i = 0; i < auths.length; i++) {
+        logger.info(`Checking if credentials at index ${i} are valid`)
+        const auth = auths[i];
+        let [data, err] = await checkCredentials(auth, config, brokerClientValidationMethod, brokerClientValidationTimeoutMs, isJsonResponse)
+        rv.push(data)
+        errorOccurred = err;
+        logger.info("Credentials checked")
+      }
+    } else {
+      logger.info("No credentials specified - checking if target can be accessed without credentials")
+      let [data, err] = await checkCredentials(null, config, brokerClientValidationMethod, brokerClientValidationTimeoutMs, isJsonResponse)
+      rv.push(data)
+      errorOccurred = err;
+    }
 
-        data.ok = true;
-        return res.status(200).json(data);
-      },
-    );
+    if (errorOccurred) {
+      return res.status(500).json(rv);
+    } else {
+      return res.status(200).json(rv);
+    }
   });
 
   // relay all other URL paths
@@ -146,3 +114,72 @@ module.exports = ({ port = null, config = {}, filters = {} }) => {
     },
   };
 };
+
+async function checkCredentials(auth, config, brokerClientValidationMethod, brokerClientValidationTimeoutMs, isJsonResponse) {
+  const data = {
+    brokerClientValidationUrl: logger.sanitise(
+      config.brokerClientValidationUrl,
+    ),
+    brokerClientValidationMethod,
+    brokerClientValidationTimeoutMs,
+  };
+
+  const validationRequestHeaders = {
+    'user-agent': 'Snyk Broker client ' + version,
+  };
+  if (auth) {
+    validationRequestHeaders.authorization = auth;
+  }
+
+  let errorOccurred = true;
+  // This was originally `request`, but `await` is a lot easier to understand than nested callback hell.
+  await rp(
+    {
+      url: config.brokerClientValidationUrl,
+      headers: validationRequestHeaders,
+      method: brokerClientValidationMethod,
+      timeout: brokerClientValidationTimeoutMs,
+      json: isJsonResponse,
+      resolveWithFullResponse: true,
+      agentOptions: {
+        ca: config.caCert, // Optional CA cert
+      },
+    },
+  ).then(response => {
+    // test logic requires to surface internal data
+    // which is best not exposed in production
+    if (process.env.TAP) {
+      data.testResponse = response;
+    }
+
+    const responseStatusCode = response && response.statusCode;
+    data.brokerClientValidationUrlStatusCode = responseStatusCode;
+
+    // check for 2xx status code
+    const goodStatusCode = /^2/.test(responseStatusCode);
+    if (!goodStatusCode) {
+      data.ok = false;
+      data.error =
+        responseStatusCode === 401 || responseStatusCode === 403
+          ? 'Failed due to invalid credentials'
+          : 'Status code is not 2xx';
+
+      logger.error(data, response && response.body, 'Systemcheck failed');
+      return;
+    }
+
+    errorOccurred = false;
+    data.ok = true;
+  }).catch(error => {
+    // test logic requires to surface internal data
+    // which is best not exposed in production
+    if (process.env.TAP) {
+      data.testError = error;
+    }
+
+    data.ok = false;
+    data.error = error.message;
+  });
+
+  return [data, errorOccurred];
+}

lib/config.js

@@ -2,7 +2,7 @@ const fs = require('fs');
 const path = require('path');
 const camelcase = require('camelcase');
 
-const { loadConfig } = require('snyk-config');
+const {loadConfig} = require('snyk-config');
 const config = loadConfig(__dirname + '/..');
 
 function camelify(res) {
@@ -14,22 +14,62 @@ function camelify(res) {
 }
 
 function expandValue(obj, value) {
-  return value.replace(/([\\]?\$.+?\b)/g, (all, key) => {
-    if (key[0] === '$') {
-      const keyToReplace = key.slice(1);
-      return obj[keyToReplace] || key;
+  let arrayFound = undefined;
+  let keyWithArray = undefined;
+  const variableRegex = /(\\?\$.+?\b)/g;
+  const variableMatcher = value.match(variableRegex);
+  if (variableMatcher) {
+    for (const key of variableMatcher) {
+      if (key[0] === "$" && obj[(key.slice(1) + "_ARRAY")]) {
+        keyWithArray = key.slice(1);
+        arrayFound = (key.slice(1) + "_ARRAY");
+        break;
+      }
     }
+  }
+
+  if (arrayFound) {
+    const values = [];
+    let array;
+    if (Array.isArray(obj[arrayFound])) {
+      array = obj[arrayFound];
+    } else {
+      array = obj[arrayFound].split(",").map(s => s.trim());
+      obj[arrayFound] = array;
+    }
+
+    for (const o of array) {
+      values.push(value.replace(variableRegex, (all, key) => {
+        if (key[0] === '$') {
+          const keyToReplace = key.slice(1);
+          return keyToReplace === keyWithArray ? o : (obj[keyToReplace] || key);
+        }
+
+        return key;
+      }));
+    }
+    return values;
+  } else {
+    return value.replace(variableRegex, (all, key) => {
+      if (key[0] === '$') {
+        const keyToReplace = key.slice(1);
+        return obj[keyToReplace] || key;
+      }
 
-    return key;
-  });
+      return key;
+    });
+  }
 }
 
 function expand(obj) {
   const keys = Object.keys(obj);
 
   for (const key of keys) {
     const value = expandValue(obj, obj[key]);
-    if (value !== obj[key]) {
+    if (value && Array.isArray(value)) {
+      // This will get camel-cased later on
+      obj[(key + "_ARRAY")] = value;
+    } else if (value !== obj[key]) {
       obj[key] = value;
     }
   }
@@ -53,4 +93,10 @@ if (res.caCert) {
   res.caCert = fs.readFileSync(path.resolve(process.cwd(), res.caCert));
 }
 
+for (const [key, value] of Object.entries(res)) {
+  if ((key.endsWith("Array") || key.endsWith("_ARRAY")) && !Array.isArray(value)) {
+    res[key] = value.split(",").map(s => s.trim());
+  }
+}
+
 module.exports = res;

lib/replace-vars.js

@@ -5,8 +5,28 @@ module.exports = {
 
 function replace(input, source) {
   return (input || '').replace(/(\${.*?})/g, (_, match) => {
-    const key = match.slice(2, -1); // ditch the wrappers
-    return source[key] || '';
+      const key = match.slice(2, -1); // ditch the wrappers
+      let keyName;
+      let idxName;
+      if (source[(key + "_ARRAY")]) {
+          keyName = key + "_ARRAY";
+          idxName = key + "_ARRAY_IDX";
+      } else if (source[(key + "Array")]) {
+          keyName = key + "Array";
+          idxName = key + "ArrayIdx";
+      }
+
+      const array = source[keyName];
+      let idx;
+      if (array) {
+          idx = source[idxName] || 0;
+          if (idx >= array.length) {
+              idx = 0;
+          }
+          source[idxName] = idx + 1;
+      }
+
+      return array ? array[idx] : source[key] || '';
   });
 }