Merge pull request #534 from snyk/chore/refactor-errors

chore: refactor snyk missing api token

src/cli/commands/monitor.ts

@@ -2,7 +2,7 @@ module.exports = monitor;
 
 import * as _ from 'lodash';
 import * as fs from 'then-fs';
-import {exists as apiTokenExists} from '../../lib/api-token';
+import {apiTokenExists} from '../../lib/api-token';
 import snyk = require('../../lib/'); // TODO(kyegupov): fix import
 import {monitor as snykMonitor} from '../../lib/monitor';
 import * as config from '../../lib/config';
@@ -74,7 +74,7 @@ async function monitor(...args0: MethodArgs): Promise<any> {
   if (options['all-sub-projects'] && options['project-name']) {
     throw new Error('`--all-sub-projects` is currently not compatible with `--project-name`');
   }
-  await apiTokenExists('snyk monitor');
+  apiTokenExists();
   // Part 1: every argument is a scan target; process them sequentially
   for (const path of args as string[]) {
     try {

src/cli/commands/test.ts

@@ -5,7 +5,7 @@ import chalk from 'chalk';
 import * as snyk from '../../lib/';
 import * as config from '../../lib/config';
 import {isCI} from '../../lib/is-ci';
-import {exists as apiTokenExists} from '../../lib/api-token';
+import {apiTokenExists} from '../../lib/api-token';
 import {SEVERITIES, WIZARD_SUPPORTED_PMS} from '../../lib/snyk-test/common';
 import * as docker from '../../lib/docker-promotion';
 import * as Debug from 'debug';
@@ -46,7 +46,7 @@ async function test(...args: MethodArgs): Promise<string> {
     return Promise.reject(new Error('INVALID_SEVERITY_THRESHOLD'));
   }
 
-  await apiTokenExists('snyk test');
+  apiTokenExists();
 
   // Promise waterfall to test all other paths sequentially
   for (const path of args as string[]) {

src/cli/index.ts

@@ -66,6 +66,7 @@ async function handleError(args, error) {
         copy(result);
         console.log('Result copied to clipboard');
       } else {
+        console.log('error', error);
         if (`${error.code}`.indexOf('AUTH_') === 0) {
           // remove the last few lines
           const erase = ansiEscapes.eraseLines(4);

src/lib/api-token.ts

@@ -0,0 +1,18 @@
+import { MissingApiTokenError } from '../lib/errors/missing-api-token';
+
+import * as config from './config';
+import * as userConfig from './user-config';
+import * as types from './types';
+
+export function api() {
+  // note: config.TOKEN will potentially come via the environment
+  return config.api || config.TOKEN || userConfig.get('api');
+}
+
+export function apiTokenExists() {
+  const configured = api();
+  if (!configured) {
+    throw MissingApiTokenError();
+  }
+  return configured;
+}

src/lib/config.ts

@@ -4,12 +4,14 @@ import * as url from 'url';
 
 const DEFAULT_TIMEOUT = 5 * 60; // in seconds
 interface Config {
-  API: string;
+  API: string; // api url
+  api: string; // token
   disableSuggestions: string;
   org: string;
   ROOT: string;
   timeout: number;
   PROJECT_NAME: string;
+  TOKEN: string;
 }
 
 const config: Config = snykConfig(__dirname + '/../..');

src/lib/errors/legacy-errors.js

@@ -25,8 +25,6 @@ const errors = {
     're-run your snyk command.',
   tryDevDeps: 'Snyk only tests production dependencies by default (which ' +
     'this project had none). Try re-running with the `--dev` flag.',
-  noApiToken: '%s requires an authenticated account. Please run `snyk auth` ' +
-    'and try again.',
   timeout: 'The request has timed out on the server side.\nPlease re-run ' +
     'this command with the `-d` flag and send the output to support@snyk.io.',
   policyFile: 'Bad policy file, please use --path=PATH to specify a ' +
@@ -55,7 +53,6 @@ const codes = {
   FAIL_PATCH: errors.patchfail,
   FAIL_UPDATE: errors.updatefail,
   NOT_FOUND_HAS_DEV_DEPS: errors.tryDevDeps,
-  NO_API_TOKEN: errors.noApiToken,
   502: errors.timeout,
   504: errors.timeout,
   UNKNOWN_COMMAND: errors.unknownCommand,

src/lib/errors/missing-api-token.ts

@@ -0,0 +1,12 @@
+import {CustomError} from './custom-error';
+import * as types from '../types';
+
+export function MissingApiTokenError() {
+  const message = '`snyk` requires an authenticated account. ' +
+  'Please run `snyk auth` and try again.';
+  const error = new CustomError(message);
+  error.code = 401;
+  error.strCode = 'NO_API_TOKEN';
+  error.userMessage = message;
+  return error;
+}

src/lib/index.js

@@ -17,7 +17,7 @@ Object.defineProperty(snyk, 'api', {
   enumerable: true,
   configurable: true,
   get: function () {
-    return apiToken();
+    return apiToken.apiTokenExists();
   },
   set: function (value) {
     snykConfig.api = value;

src/lib/monitor.ts

@@ -1,5 +1,5 @@
 import * as snyk from '../lib';
-import {exists as apiTokenExists} from './api-token';
+import {apiTokenExists} from './api-token';
 import * as request from './request';
 import * as config from './config';
 import * as os from 'os';
@@ -39,87 +39,83 @@ interface Meta {
   projectName: string;
 }
 
-export function monitor(root, meta, info: SingleDepRootResult, targetFile): Promise<any> {
+export async function monitor(root, meta, info: SingleDepRootResult, targetFile): Promise<any> {
+  apiTokenExists();
   const pkg = info.package;
   const pluginMeta = info.plugin;
-  let policyPath = meta['policy-path'];
-  if (!meta.isDocker) {
-    policyPath = policyPath || root;
-  }
+  let policy;
+  const policyPath = meta['policy-path'] || root;
   const policyLocations = [policyPath].concat(pluckPolicies(pkg))
     .filter(Boolean);
-  const opts = {loose: true};
-  const packageManager = meta.packageManager || 'npm';
-  return apiTokenExists('snyk monitor')
-    .then(() => {
-      if (policyLocations.length === 0) {
-        return snyk.policy.create();
-      }
-      return snyk.policy.load(policyLocations, opts);
-    }).then(async (policy) => {
-      analytics.add('packageManager', packageManager);
+  // docker doesn't have a policy as it can be run from anywhere
+  if (!meta.isDocker || !policyLocations.length) {
+      await snyk.policy.create();
+  }
+  policy = await snyk.policy.load(policyLocations, {loose: true});
 
-      const target = await projectMetadata.getInfo(pkg);
-      const targetFileRelativePath = targetFile ? path.relative(root, targetFile) : '';
+  const packageManager = meta.packageManager;
+  analytics.add('packageManager', packageManager);
 
-      // TODO(kyegupov): async/await
-      return new Promise((resolve, reject) => {
-        request({
-          body: {
-            meta: {
-              method: meta.method,
-              hostname: os.hostname(),
-              id: meta.id || snyk.id || pkg.name,
-              ci: isCI(),
-              pid: process.pid,
-              node: process.version,
-              master: snyk.config.isMaster,
-              name: pkg.name,
-              version: pkg.version,
-              org: config.org ? decodeURIComponent(config.org) : undefined,
-              pluginName: pluginMeta.name,
-              pluginRuntime: pluginMeta.runtime,
-              dockerImageId: pluginMeta.dockerImageId,
-              dockerBaseImage: pkg.docker ? pkg.docker.baseImage : undefined,
-              dockerfileLayers: pkg.docker ? pkg.docker.dockerfileLayers : undefined,
-              projectName: meta['project-name'],
-            },
-            policy: policy.toString(),
-            package: pkg,
-            // we take the targetFile from the plugin,
-            // because we want to send it only for specific package-managers
-            target,
-            targetFile: pluginMeta.targetFile,
-            targetFileRelativePath,
-          } as MonitorBody,
-          gzip: true,
-          method: 'PUT',
-          headers: {
-            'authorization': 'token ' + snyk.api,
-            'content-encoding': 'gzip',
-          },
-          url: config.API + '/monitor/' + packageManager,
-          json: true,
-        }, (error, res, body) => {
-          if (error) {
-            return reject(error);
-          }
+  const target = await projectMetadata.getInfo(pkg);
+  const targetFileRelativePath = targetFile ? path.relative(root, targetFile) : '';
 
-          if (res.statusCode === 200 || res.statusCode === 201) {
-            resolve(body);
-          } else {
-            const e = new MonitorError('Server returned unexpected error for the monitor request. ' +
-              `Status code: ${res.statusCode}, response: ${res.body.userMessage || res.body.message}`);
-            e.code = res.statusCode;
-            e.userMessage = body && body.userMessage;
-            if (!e.userMessage && res.statusCode === 504) {
-              e.userMessage = 'Connection Timeout';
-            }
-            reject(e);
-          }
-        });
-      });
+  // TODO(kyegupov): async/await
+  return new Promise((resolve, reject) => {
+    request({
+      body: {
+        meta: {
+          method: meta.method,
+          hostname: os.hostname(),
+          id: meta.id || snyk.id || pkg.name,
+          ci: isCI(),
+          pid: process.pid,
+          node: process.version,
+          master: snyk.config.isMaster,
+          name: pkg.name,
+          version: pkg.version,
+          org: config.org ? decodeURIComponent(config.org) : undefined,
+          pluginName: pluginMeta.name,
+          pluginRuntime: pluginMeta.runtime,
+          dockerImageId: pluginMeta.dockerImageId,
+          dockerBaseImage: pkg.docker ? pkg.docker.baseImage : undefined,
+          dockerfileLayers: pkg.docker ? pkg.docker.dockerfileLayers : undefined,
+          projectName: meta['project-name'],
+        },
+        policy: policy ? policy.toString() : undefined,
+        package: pkg,
+        // we take the targetFile from the plugin,
+        // because we want to send it only for specific package-managers
+        target,
+        targetFile: pluginMeta.targetFile,
+        targetFileRelativePath,
+      } as MonitorBody,
+      gzip: true,
+      method: 'PUT',
+      headers: {
+        'authorization': 'token ' + snyk.api,
+        'content-encoding': 'gzip',
+      },
+      url: config.API + '/monitor/' + packageManager,
+      json: true,
+    }, (error, res, body) => {
+      if (error) {
+        return reject(error);
+      }
+
+      if (res.statusCode === 200 || res.statusCode === 201) {
+        resolve(body);
+      } else {
+        const e = new MonitorError('Server returned unexpected error for the monitor request. ' +
+          `Status code: ${res.statusCode}, response: ${res.body.userMessage || res.body.message}`);
+        e.code = res.statusCode;
+        e.userMessage = body && body.userMessage;
+        if (!e.userMessage && res.statusCode === 504) {
+          e.userMessage = 'Connection Timeout';
+        }
+        reject(e);
+      }
     });
+  });
 }
 
 function pluckPolicies(pkg) {

test/acceptance/cli.acceptance.test.ts

@@ -2606,7 +2606,7 @@ test('`monitor foo:latest --docker` doesnt send policy from cwd', async (t) => {
     }], 'calls docker plugin with expected arguments');
 
   const emptyPolicy = await snykPolicy.create();
-  t.same(req.body.policy, emptyPolicy.toString(), 'empty policy is sent');
+  t.deepEqual(req.body.policy, emptyPolicy.toString(), 'empty policy is sent');
 });
 
 test('`monitor foo:latest --docker` with custom policy path', async (t) => {
@@ -2647,7 +2647,7 @@ test('`monitor foo:latest --docker` with custom policy path', async (t) => {
     path.join('custom-location', '.snyk'),
     'utf8');
   const policyString = req.body.policy;
-  t.equal(policyString, expected, 'sends correct policy');
+  t.deepEqual(policyString, expected, 'sends correct policy');
 });
 
 test('`wizard` for unsupported package managers', async (t) => {

test/cli.test.js

@@ -249,18 +249,19 @@ test('snyk ignore - not authorized', function (t) {
     .catch((err) => t.pass('no policy file saved'));
 });
 
-test('test without authentication', function (t) {
-  t.plan(1);
-  return cli.config('unset', 'api').then(function () {
-    return cli.test('semver@2');
-  }).then(function (res) {
+test('test without authentication', async (t) => {
+  t.plan(2);
+  await cli.config('unset', 'api');
+  try {
+    await cli.test('semver@2');
     t.fail('test should not pass if not authenticated');
-  }).catch(function (error) {
-    t.equals(error.code, 'NO_API_TOKEN', 'test requires authentication');
-  })
-  .then(function () {
-    return cli.config('set', 'api=' + apiKey);
-  });
+  } catch (error) {
+    t.deepEquals(error.strCode, 'NO_API_TOKEN', 'string code is as expected');
+    t.match(error.message,
+      '`snyk` requires an authenticated account. Please run `snyk auth` and try again.',
+      'error message is shown as expected');
+  }
+  await cli.config('set', 'api=' + apiKey);
 });
 
 test('auth via key', function (t) {