From 1936db41cfc792cd76badc3713973a5d2173ba44 Mon Sep 17 00:00:00 2001 From: Liliana Kastilio Date: Mon, 3 Jun 2019 12:53:45 +0100 Subject: [PATCH] "fix: revert refactor snyk missing api token" This reverts commit 76b05655539e3bd5519c88958ced541affe67be5. --- src/cli/commands/monitor.ts | 4 +- src/cli/commands/test.ts | 4 +- src/cli/index.ts | 1 - src/lib/api-token.js | 17 +++ src/lib/api-token.ts | 18 --- src/lib/config.ts | 4 +- src/lib/errors/legacy-errors.js | 3 + src/lib/errors/missing-api-token.ts | 12 -- src/lib/index.js | 2 +- src/lib/monitor.ts | 150 +++++++++++++------------ test/acceptance/cli.acceptance.test.ts | 4 +- test/cli.test.js | 23 ++-- 12 files changed, 116 insertions(+), 126 deletions(-) create mode 100644 src/lib/api-token.js delete mode 100644 src/lib/api-token.ts delete mode 100644 src/lib/errors/missing-api-token.ts diff --git a/src/cli/commands/monitor.ts b/src/cli/commands/monitor.ts index 07375b61e0..ec00d12a54 100644 --- a/src/cli/commands/monitor.ts +++ b/src/cli/commands/monitor.ts @@ -2,7 +2,7 @@ module.exports = monitor; import * as _ from 'lodash'; import * as fs from 'then-fs'; -import {apiTokenExists} from '../../lib/api-token'; +import {exists as 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'; @@ -73,7 +73,7 @@ async function monitor(...args0: MethodArgs): Promise { if (options['all-sub-projects'] && options['project-name']) { throw new Error('`--all-sub-projects` is currently not compatible with `--project-name`'); } - apiTokenExists(); + await apiTokenExists('snyk monitor'); // Part 1: every argument is a scan target; process them sequentially for (const path of args as string[]) { try { diff --git a/src/cli/commands/test.ts b/src/cli/commands/test.ts index 4ff6b78e24..dc7992d494 100644 --- a/src/cli/commands/test.ts +++ b/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 {apiTokenExists} from '../../lib/api-token'; +import {exists as apiTokenExists} from '../../lib/api-token'; import {SEVERITIES, WIZARD_SUPPORTED_PMS} from '../../lib/snyk-test/common'; import * as Debug from 'debug'; import {TestOptions} from '../../lib/types'; @@ -45,7 +45,7 @@ async function test(...args: MethodArgs): Promise { return Promise.reject(new Error('INVALID_SEVERITY_THRESHOLD')); } - apiTokenExists(); + await apiTokenExists('snyk test'); // Promise waterfall to test all other paths sequentially for (const path of args as string[]) { diff --git a/src/cli/index.ts b/src/cli/index.ts index 1d394a3924..7cb87ab8ba 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -66,7 +66,6 @@ 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); diff --git a/src/lib/api-token.js b/src/lib/api-token.js new file mode 100644 index 0000000000..601b1621d0 --- /dev/null +++ b/src/lib/api-token.js @@ -0,0 +1,17 @@ +module.exports = api; +module.exports.exists = exists; + +var config = require('./config'); +var userConfig = require('./user-config'); +var error = new Error('NO_API_TOKEN'); +error.code = 'NO_API_TOKEN'; + +function api() { + // note: config.TOKEN will potentially come via the environment + return config.api || config.TOKEN || userConfig.get('api'); +} + +function exists(label) { + error.message = label || error.code; + return api() ? Promise.resolve() : Promise.reject(error); +} diff --git a/src/lib/api-token.ts b/src/lib/api-token.ts deleted file mode 100644 index aa525b38f7..0000000000 --- a/src/lib/api-token.ts +++ /dev/null @@ -1,18 +0,0 @@ -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; -} diff --git a/src/lib/config.ts b/src/lib/config.ts index 1ff1b241b1..947969f042 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -4,14 +4,12 @@ import * as url from 'url'; const DEFAULT_TIMEOUT = 5 * 60; // in seconds interface Config { - API: string; // api url - api: string; // token + API: string; disableSuggestions: string; org: string; ROOT: string; timeout: number; PROJECT_NAME: string; - TOKEN: string; } const config: Config = snykConfig(__dirname + '/../..'); diff --git a/src/lib/errors/legacy-errors.js b/src/lib/errors/legacy-errors.js index 5961ffa639..8f7bb4913a 100644 --- a/src/lib/errors/legacy-errors.js +++ b/src/lib/errors/legacy-errors.js @@ -25,6 +25,8 @@ 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 ' + @@ -53,6 +55,7 @@ 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, diff --git a/src/lib/errors/missing-api-token.ts b/src/lib/errors/missing-api-token.ts deleted file mode 100644 index 2bcd46739c..0000000000 --- a/src/lib/errors/missing-api-token.ts +++ /dev/null @@ -1,12 +0,0 @@ -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; -} diff --git a/src/lib/index.js b/src/lib/index.js index 1456892c55..974bf52361 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -17,7 +17,7 @@ Object.defineProperty(snyk, 'api', { enumerable: true, configurable: true, get: function () { - return apiToken.apiTokenExists(); + return apiToken(); }, set: function (value) { snykConfig.api = value; diff --git a/src/lib/monitor.ts b/src/lib/monitor.ts index 0abbb0c800..784b66a4b8 100644 --- a/src/lib/monitor.ts +++ b/src/lib/monitor.ts @@ -1,5 +1,5 @@ import * as snyk from '../lib'; -import {apiTokenExists} from './api-token'; +import {exists as apiTokenExists} from './api-token'; import * as request from './request'; import * as config from './config'; import * as os from 'os'; @@ -39,87 +39,91 @@ interface Meta { projectName: string; } -export async function monitor(root, meta, info: SingleDepRootResult, targetFile): Promise { - apiTokenExists(); +export function monitor(root, meta, info: SingleDepRootResult, targetFile): Promise { const pkg = info.package; const pluginMeta = info.plugin; - let policy; - const policyPath = meta['policy-path'] || root; + let policyPath = meta['policy-path']; + if (!meta.isDocker) { + policyPath = policyPath || root; + } const policyLocations = [policyPath].concat(pluckPolicies(pkg)) .filter(Boolean); - // 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 packageManager = meta.packageManager; - analytics.add('packageManager', packageManager); - - const target = await projectMetadata.getInfo(pkg); - const targetFileRelativePath = targetFile ? path.relative(root, targetFile) : ''; + 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); - if (target && target.branch) { - analytics.add('targetBranch', target.branch); - } + const target = await projectMetadata.getInfo(pkg); + const targetFileRelativePath = targetFile ? path.relative(root, targetFile) : ''; - // 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 (target && target.branch) { + analytics.add('targetBranch', target.branch); } - 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) { diff --git a/test/acceptance/cli.acceptance.test.ts b/test/acceptance/cli.acceptance.test.ts index d790fc5752..82e337a7f2 100644 --- a/test/acceptance/cli.acceptance.test.ts +++ b/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.deepEqual(req.body.policy, emptyPolicy.toString(), 'empty policy is sent'); + t.same(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.deepEqual(policyString, expected, 'sends correct policy'); + t.equal(policyString, expected, 'sends correct policy'); }); test('`wizard` for unsupported package managers', async (t) => { diff --git a/test/cli.test.js b/test/cli.test.js index faaa2c1d87..73b2785bba 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -249,19 +249,18 @@ test('snyk ignore - not authorized', function (t) { .catch((err) => t.pass('no policy file saved')); }); -test('test without authentication', async (t) => { - t.plan(2); - await cli.config('unset', 'api'); - try { - await cli.test('semver@2'); +test('test without authentication', function (t) { + t.plan(1); + return cli.config('unset', 'api').then(function () { + return cli.test('semver@2'); + }).then(function (res) { t.fail('test should not pass if not authenticated'); - } 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); + }).catch(function (error) { + t.equals(error.code, 'NO_API_TOKEN', 'test requires authentication'); + }) + .then(function () { + return cli.config('set', 'api=' + apiKey); + }); }); test('auth via key', function (t) {