Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: add error handling when refresh lwa token fails, pass in debug flag to all the authorizationController #165

Merged
merged 1 commit into from
May 6, 2020
Merged

fix: add error handling when refresh lwa token fails, pass in debug flag to all the authorizationController #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/clients/lwa-auth-code-client/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ const queryString = require('querystring');
const addSeconds = require('date-fns/addSeconds');
const isAfter = require('date-fns/isAfter');
const parseISO = require('date-fns/parseISO');
const stringUtils = require('@src/utils/string-utils');
const CONSTANTS = require('@src/utils/constants');

const httpClient = require('@src/clients/http-client');
const CONSTANTS = require('@src/utils/constants');
const providerChainUtils = require('@src/utils/provider-chain-utils');
const stringUtils = require('@src/utils/string-utils');
const jsonView = require('@src/view/json-view');

module.exports = class LWAAuthCodeClient {
constructor(config) {
Expand Down Expand Up @@ -67,8 +69,17 @@ module.exports = class LWAAuthCodeClient {
if (err) {
return callback(err);
}
const responseErr = R.view(R.lensPath(['body', 'error']), response);
if (stringUtils.isNonBlankString(responseErr)) {
return callback(`Refresh LWA tokens failed, please run "ask configure" to manually update your tokens. Error: ${responseErr}.`);
}
const expiresIn = R.view(R.lensPath(['body', 'expires_in']), response);
if (!expiresIn) {
return callback(`Received invalid response body from LWA without "expires_in":\n${jsonView.toString(response.body)}`);
}

const tokenBody = R.clone(response.body);
tokenBody.expires_at = this._getExpiresAt(tokenBody.expires_in).toISOString();
tokenBody.expires_at = this._getExpiresAt(expiresIn).toISOString();
callback(null, tokenBody);
});
}
Expand Down
6 changes: 4 additions & 2 deletions lib/clients/smapi-client/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ module.exports = class SmapiClient {
body: NULL_PAYLOAD,
};
const authorizationController = new AuthorizationController({
auth_client_type: 'LWA'
auth_client_type: 'LWA',
doDebug: this.doDebug
});
authorizationController.tokenRefreshAndRead(this.profile, (tokenErr, token) => {
if (tokenErr) {
Expand Down Expand Up @@ -152,7 +153,8 @@ module.exports = class SmapiClient {
json: !!payload
};
const authorizationController = new AuthorizationController({
auth_client_type: 'LWA'
auth_client_type: 'LWA',
doDebug: this.doDebug
});
authorizationController.tokenRefreshAndRead(this.profile, (tokenErr, token) => {
if (tokenErr) {
Expand Down
3 changes: 2 additions & 1 deletion lib/commands/configure/ask-profile-setup-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ function _buildAuthorizationConfiguration(config) {
scopes: CONSTANTS.LWA.DEFAULT_SCOPES,
state: CONSTANTS.LWA.DEFAULT_STATE,
auth_client_type: 'LWA',
redirectUri: CONSTANTS.LWA.S3_RESPONSE_PARSER_URL
redirectUri: CONSTANTS.LWA.S3_RESPONSE_PARSER_URL,
doDebug: config.doDebug
};
}
2 changes: 1 addition & 1 deletion lib/commands/configure/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ function _buildAskConfig(cmd, askFolderPath, configFilePath) {
isFirstTimeProfileCreation: AppConfig.getInstance().getProfilesList().length < 1,
askProfile: cmd.profile,
needBrowser: cmd.browser,
debug: cmd.debug
doDebug: cmd.debug
};
}

Expand Down
1 change: 0 additions & 1 deletion lib/commands/new/hosted-skill-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ function createHostedSkill(hostedSkillController, userInput, vendorId, callback)
}
skillMetadataController.enableSkill((enableErr) => {
if (enableErr) {
Messenger.getInstance().error(enableErr);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a little out of the context, but I noticed when I run unit test. This error will anyway be displayed in the upstream, so it's duplicate

return callback(enableErr);
}
const templateUrl = CONSTANTS.HOSTED_SKILL.GIT_HOOKS_TEMPLATES.PRE_PUSH.URL;
Expand Down
6 changes: 4 additions & 2 deletions lib/commands/smapi/smapi-command-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,16 @@ const _sdkFunctionName = (swaggerApiOperationName) => `call${swaggerApiOperation
* @param {Map} flatParamsMap Flattened parameters.
* @param {Map} commanderToApiCustomizationMap Map of commander options to custom options
* for api properties.
* @param {Boolean} doDebug
* @param {Object} cmdObj Commander object with passed values.
*/
const smapiCommandHandler = (swaggerApiOperationName, flatParamsMap, commanderToApiCustomizationMap, inputCmdObj, modelIntrospector) => {
new AppConfig(configFilePath);
const inputOpts = inputCmdObj.opts();
const authorizationController = new AuthorizationController({
auth_client_type: 'LWA'
auth_client_type: 'LWA',
doDebug: inputOpts.debug
});
const inputOpts = inputCmdObj.opts();
const profile = profileHelper.runtimeProfile(inputOpts.profile);
const refreshTokenConfig = {
clientId: authorizationController.oauthClient.config.clientId,
Expand Down
77 changes: 59 additions & 18 deletions ...lients/lwa-auth-code-client-test/index.js → ...unit/clients/lwa-auth-code-client-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const proxyquire = require('proxyquire');
const httpClient = require('@src/clients/http-client');
const LWAClient = require('@src/clients/lwa-auth-code-client');
const CONSTANTS = require('@src/utils/constants');
const jsonView = require('@src/view/json-view');

describe('# Clients test - LWA OAuth2 client test', () => {
const TEST_BASIC_CONFIGURATION = {
Expand All @@ -32,7 +33,10 @@ describe('# Clients test - LWA OAuth2 client test', () => {
};
const TEST_REQUEST_RESPONSE_ACCESS_TOKEN = {
statusCode: 200,
body: { token: 'BODY' },
body: {
access_token: 'BODY',
expires_in: 3600
},
headers: {}
};
const VALID_AUTH_CODE = 'authCode';
Expand Down Expand Up @@ -130,10 +134,59 @@ describe('# Clients test - LWA OAuth2 client test', () => {
sinon.stub(LWAClient.prototype, '_getExpiresAt');
});

it('| httpClient request fails while refreshing access token', (done) => {
// setup
const TEST_ERROR = 'error';
const lwaClient = new LWAClient(TEST_BASIC_CONFIGURATION);
httpClient.request.callsArgWith(3, TEST_ERROR);
// call
lwaClient.refreshToken(VALID_ACCESS_TOKEN, (err, res) => {
// verify
expect(err).equal(TEST_ERROR);
expect(res).equal(undefined);
done();
});
});

it('| httpClient response contains failure message, expect process fails with correct message', (done) => {
// setup
const TEST_ERROR = 'error';
const TEST_ERROR_RESPONSE = {
body: { error: TEST_ERROR }
};
const lwaClient = new LWAClient(TEST_BASIC_CONFIGURATION);
httpClient.request.callsArgWith(3, undefined, TEST_ERROR_RESPONSE);
// call
lwaClient.refreshToken(VALID_ACCESS_TOKEN, (err, res) => {
// verify
expect(err).equal(`Refresh LWA tokens failed, please run "ask configure" to manually update your tokens. Error: ${TEST_ERROR}.`);
expect(res).equal(undefined);
done();
});
});

it('| httpClient response is invalid, expect process fails with correct message', (done) => {
// setup
const TEST_INVALID_RESPONSE = {
body: {
access_token: 'token'
}
};
const lwaClient = new LWAClient(TEST_BASIC_CONFIGURATION);
httpClient.request.callsArgWith(3, undefined, TEST_INVALID_RESPONSE);
// call
lwaClient.refreshToken(VALID_ACCESS_TOKEN, (err, res) => {
// verify
expect(err).equal(`Received invalid response body from LWA without "expires_in":\n${jsonView.toString(TEST_INVALID_RESPONSE.body)}`);
expect(res).equal(undefined);
done();
});
});

it('| refreshing access token successful', (done) => {
// setup
const lwaClient = new LWAClient(TEST_BASIC_CONFIGURATION);
httpClient.request.callsArgWith(3, null, TEST_REQUEST_RESPONSE_ACCESS_TOKEN);
httpClient.request.callsArgWith(3, undefined, TEST_REQUEST_RESPONSE_ACCESS_TOKEN);
LWAClient.prototype._getExpiresAt.returns(TEST_EXPIRES_AT);
// call
lwaClient.refreshToken(VALID_ACCESS_TOKEN, (err, res) => {
Expand All @@ -153,27 +206,14 @@ describe('# Clients test - LWA OAuth2 client test', () => {
expect(httpClient.request.args[0][2]).equal(false);
expect(err).equal(null);
expect(res).deep.equal({
token: 'BODY',
access_token: 'BODY',
expires_in: 3600,
expires_at: 'expires_at'
});
done();
});
});

it('| Failure while refreshing access token', (done) => {
// setup
const TEST_ERROR = 'error';
const lwaClient = new LWAClient(TEST_BASIC_CONFIGURATION);
httpClient.request.callsArgWith(3, TEST_ERROR);
// call
lwaClient.refreshToken(VALID_ACCESS_TOKEN, (err, res) => {
// verify
expect(err).equal(TEST_ERROR);
expect(res).equal(undefined);
done();
});
});

afterEach(() => {
sinon.restore();
});
Expand Down Expand Up @@ -209,7 +249,8 @@ describe('# Clients test - LWA OAuth2 client test', () => {
expect(httpClient.request.args[0][2]).equal(false);
expect(err).equal(null);
expect(res).deep.equal({
token: 'BODY',
access_token: 'BODY',
expires_in: 3600,
expires_at: 'expires_at'
});
done();
Expand Down