Skip to content

Commit

Permalink
fix: show more meaningful error message for failed auth
Browse files Browse the repository at this point in the history
  • Loading branch information
AndreDalcher committed Nov 25, 2019
1 parent 3585f53 commit 79e5e47
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 2 deletions.
25 changes: 23 additions & 2 deletions src/cli/commands/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { TokenExpiredError } from '../../../lib/errors/token-expired-error';
import { MisconfiguredAuthInCI } from '../../../lib/errors/misconfigured-auth-in-ci-error';
import { AuthFailedError } from '../../../lib/errors/authentication-failed-error';
import { verifyAPI } from './is-authed';
import { CustomError } from '../../../lib/errors';

export = auth;

Expand Down Expand Up @@ -86,7 +87,7 @@ async function testAuthComplete(token: string): Promise<{ res; body }> {
}

if (res.statusCode !== 200) {
return reject(AuthFailedError(body.message, res.statusCode));
return reject(errorForFailedAuthAttempt(res, body));
}

// we have success
Expand Down Expand Up @@ -132,6 +133,26 @@ async function auth(apiToken: string, via: AuthCliCommands) {
'be used.\n'
);
}
throw AuthFailedError(body.message, res.statusCode);
throw errorForFailedAuthAttempt(res, body);
});
}

/**
* Resolve an appropriate error for a failed attempt to authenticate
*
* @param res The response from the API
* @param body The body of the failed authentication request
*/
function errorForFailedAuthAttempt(res, body) {
if (res.statusCode === 401 || res.statusCode === 403) {
return AuthFailedError(body.userMessage, res.statusCode);
} else {
const userMessage = body && body.userMessage;
const error = new CustomError(userMessage || 'Auth request failed');
if (userMessage) {
error.userMessage = userMessage;
}
error.code = res.statusCode;
return error;
}
}
85 changes: 85 additions & 0 deletions test/system/auth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import * as _ from 'lodash';
import { test } from 'tap';
import * as sinon from 'sinon';
import * as isAuthed from '../../src/cli/commands/auth/is-authed';
import stripAnsi from 'strip-ansi';
const port = process.env.PORT || process.env.SNYK_PORT || '12345';

const apiKey = '123456789';
const notAuthorizedApiKey = 'notAuthorized';
process.env.SNYK_API = 'http://localhost:' + port + '/api/v1';
process.env.SNYK_HOST = 'http://localhost:' + port;
process.env.LOG_LEVEL = '0';

// tslint:disable-next-line:no-var-requires
const server = require('../cli-server')(
process.env.SNYK_API,
apiKey,
notAuthorizedApiKey,
);

// ensure this is required *after* the demo server, since this will
// configure our fake configuration too
import * as cli from '../../src/cli/commands';

test('setup', (t) => {
t.plan(1);

server.listen(port, () => {
t.pass('started demo server');
});
});

test('auth shows an appropriate error message when a request times out', async (t) => {
const errors = require('../../src/lib/errors/legacy-errors');
const failedReq = new Promise((resolve) => {
return resolve({ res: { statusCode: 502 } });
});
const verifyStub = sinon.stub(isAuthed, 'verifyAPI').returns(failedReq);

t.teardown(() => {
verifyStub.restore();
});

try {
await cli.auth(apiKey);
t.fail('Authentication should have failed');
} catch (e) {
const message = stripAnsi(errors.message(e));
t.ok(message.includes('request has timed out'), 'correct error message');
}
});

test('auth shows an appropriate error message when a request fails with a user message', async (t) => {
const errors = require('../../src/lib/errors/legacy-errors');
const userMessage = 'Oh no! The request failed';
const failedReq = new Promise((resolve) => {
return resolve({ res: { statusCode: 502, body: { userMessage } } });
});
const verifyStub = sinon.stub(isAuthed, 'verifyAPI').returns(failedReq);

t.teardown(() => {
verifyStub.restore();
});

try {
await cli.auth(apiKey);
t.fail('Authentication should have failed');
} catch (e) {
const message = stripAnsi(errors.message(e));
t.equal(message, userMessage, 'userMessage shown on auth failure');
}
});

test('teardown', (t) => {
t.plan(2);

delete process.env.SNYK_API;
delete process.env.SNYK_HOST;
delete process.env.SNYK_PORT;
t.notOk(process.env.SNYK_PORT, 'fake env values cleared');

server.close(() => {
t.pass('server shutdown');
});
});

0 comments on commit 79e5e47

Please sign in to comment.