From 9879b8fce7bd250aa6b19f2692d54a0f8ca49df4 Mon Sep 17 00:00:00 2001 From: Prashanth Bheemagani Date: Thu, 20 Feb 2020 15:34:21 -0800 Subject: [PATCH] fix: environment variables usage and profile name check in configure command --- .../authorization-controller/index.js | 27 +++---- lib/utils/constants.js | 2 +- .../authorization-controller/index-test.js | 75 +++++-------------- test/unit/utils/string-utils-test.js | 5 ++ 4 files changed, 35 insertions(+), 74 deletions(-) diff --git a/lib/controllers/authorization-controller/index.js b/lib/controllers/authorization-controller/index.js index f289a2dc..e47be2b5 100644 --- a/lib/controllers/authorization-controller/index.js +++ b/lib/controllers/authorization-controller/index.js @@ -25,7 +25,7 @@ module.exports = class AuthorizationController { } /** - * Generates Authroization URL. + * Generates Authorization URL. */ getAuthorizeUrl() { return this.oauthClient.generateAuthorizeUrl(); @@ -57,14 +57,20 @@ module.exports = class AuthorizationController { return callback(messages.ASK_ENV_VARIABLES_ERROR_MESSAGE); } if (isNonBlankRefreshToken) { - this._getRefreshTokenAndUpdateConfig(profile, (err, token) => callback(err, err === null ? token : err)); + this.oauthClient.refreshToken({ refresh_token: askRefreshToken }, (err, token) => { + if (err) { + return callback(err); + } + callback(null, token.access_token); + }); } else if (isNonBlankAccessToken) { return callback(null, askAccessToken); } } else if (this.oauthClient.isValidToken(AppConfig.getInstance().getToken(profile))) { callback(null, AppConfig.getInstance().getToken(profile).access_token); } else { - this._getRefreshTokenAndUpdateConfig(profile, (err, token) => callback(err, err === null ? token : err)); + this._getRefreshTokenAndUpdateConfig(profile, AppConfig.getInstance().getToken(profile), + (err, token) => callback(err, err === null ? token : err)); } } @@ -74,8 +80,8 @@ module.exports = class AuthorizationController { * @param {Function} callback * @private */ - _getRefreshTokenAndUpdateConfig(profile, callback) { - this._getRefreshToken(profile, (err, refreshedAccessToken) => { + _getRefreshTokenAndUpdateConfig(profile, token, callback) { + this.oauthClient.refreshToken(token, (err, refreshedAccessToken) => { if (err) { return callback(err); } @@ -85,17 +91,6 @@ module.exports = class AuthorizationController { }); } - /** - * Helper method to call refreshToken. - * @param {String} profile | current profile. - * @param {Function} callback - * @private - */ - _getRefreshToken(profile, callback) { - const token = AppConfig.getInstance().getToken(profile); - this.oauthClient.refreshToken(token, (err, refreshedAccessToken) => callback(err, err == null ? refreshedAccessToken : err)); - } - /** * Helper method to keep listening to LWA response. * @param {Function} callback diff --git a/lib/utils/constants.js b/lib/utils/constants.js index f0f0ca03..2b715a19 100644 --- a/lib/utils/constants.js +++ b/lib/utils/constants.js @@ -315,7 +315,7 @@ module.exports.LWA = { }; module.exports.REGEX_VALIDATIONS = { - PROFILE_NAME: /^[a-zA-Z0-9-_]+$/g + PROFILE_NAME: /(^[a-zA-Z0-9-_]+$)(?!__ENVIRONMENT_ASK_PROFILE__)/g }; module.exports.COMMAND = { diff --git a/test/unit/controller/authorization-controller/index-test.js b/test/unit/controller/authorization-controller/index-test.js index ef014c1d..fed8f337 100644 --- a/test/unit/controller/authorization-controller/index-test.js +++ b/test/unit/controller/authorization-controller/index-test.js @@ -19,6 +19,7 @@ describe('Controller test - Authorization controller test', () => { const DEFAULT_SCOPE = CONSTANTS.LWA.DEFAULT_SCOPES; const authorizePath = CONSTANTS.LWA.DEFAULT_AUTHORIZE_PATH; const authorizeHost = CONSTANTS.LWA.DEFAULT_AUTHORIZE_HOST; + const TEST_TOKEN = 'testToken'; const TEST_STATE = 'state'; const TEST_PROFILE = 'testProfile'; const TEST_ENVIRONMENT_PROFILE = CONSTANTS.PLACEHOLDER.ENVIRONMENT_VAR.PROFILE_NAME; @@ -127,6 +128,7 @@ describe('Controller test - Authorization controller test', () => { sinon.stub(AppConfig, 'getInstance').returns({ getToken: getTokenStub }); + sinon.stub(httpClient, 'request'); }); describe('# returns valid token', () => { @@ -138,7 +140,7 @@ describe('Controller test - Authorization controller test', () => { it('| non-environment profile, expired access token', (done) => { // setup - sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(1, null, VALID_ACCESS_TOKEN); + sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(2, null, VALID_ACCESS_TOKEN); getTokenStub.withArgs(TEST_PROFILE).returns(TEST_RESPONSE.body); // call @@ -166,13 +168,14 @@ describe('Controller test - Authorization controller test', () => { it('| environment profile, valid refresh token', (done) => { // setup process.env.ASK_REFRESH_TOKEN = TEST_ENV_REFRESH_TOKEN; - sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(1, null, VALID_ACCESS_TOKEN); + sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(2, null, VALID_ACCESS_TOKEN); + httpClient.request.callsArgWith(3, null, TEST_RESPONSE); // call authorizationController.tokenRefreshAndRead(TEST_ENVIRONMENT_PROFILE, (error, accessToken) => { // verify expect(error).eq(null); - expect(accessToken).to.deep.eq(VALID_ACCESS_TOKEN); + expect(accessToken).to.deep.eq(TEST_RESPONSE.body.access_token); done(); }); }); @@ -191,6 +194,7 @@ describe('Controller test - Authorization controller test', () => { }); describe('# returns error', () => { + afterEach(() => { sinon.restore(); delete process.env.ASK_REFRESH_TOKEN; @@ -199,7 +203,7 @@ describe('Controller test - Authorization controller test', () => { it('| non-environment profile, expired access token, _getRefreshTokenAndUpdateConfig fails', (done) => { // setup - sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(1, TEST_ERROR_MESSAGE); + sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(2, TEST_ERROR_MESSAGE); getTokenStub.withArgs(TEST_PROFILE).returns(TEST_RESPONSE.body); // call @@ -211,16 +215,17 @@ describe('Controller test - Authorization controller test', () => { }); }); - it('| environment profile, valid refresh token, _getRefreshTokenAndUpdateConfig fails', (done) => { + it('| environment profile, valid refresh token, refreshing token fails', (done) => { // setup process.env.ASK_REFRESH_TOKEN = TEST_ENV_REFRESH_TOKEN; - sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(1, TEST_ERROR_MESSAGE); + sinon.stub(AuthorizationController.prototype, '_getRefreshTokenAndUpdateConfig').callsArgWith(2, null, VALID_ACCESS_TOKEN); + httpClient.request.callsArgWith(3, TEST_ERROR_MESSAGE); // call - authorizationController.tokenRefreshAndRead(TEST_ENVIRONMENT_PROFILE, (error, response) => { + authorizationController.tokenRefreshAndRead(TEST_ENVIRONMENT_PROFILE, (error, accessToken) => { // verify expect(error).eq(TEST_ERROR_MESSAGE); - expect(response).to.deep.eq(TEST_ERROR_MESSAGE); + expect(accessToken).eq(undefined); done(); }); }); @@ -248,6 +253,7 @@ describe('Controller test - Authorization controller test', () => { setToken: setTokenStub, write: writeStub }); + sinon.stub(httpClient, 'request'); }); afterEach(() => { @@ -256,10 +262,10 @@ describe('Controller test - Authorization controller test', () => { it('| returns valid access token and updates config', (done) => { // setup - sinon.stub(AuthorizationController.prototype, '_getRefreshToken').callsArgWith(1, null, TEST_RESPONSE.body); + httpClient.request.callsArgWith(3, null, TEST_RESPONSE); // call - authorizationController._getRefreshTokenAndUpdateConfig(TEST_PROFILE, (error, accessToken) => { + authorizationController._getRefreshTokenAndUpdateConfig(TEST_PROFILE, TEST_TOKEN, (error, accessToken) => { // verify expect(setTokenStub.args[0][0]).eq(TEST_PROFILE); expect(setTokenStub.args[0][1]).to.deep.eq(TEST_RESPONSE.body); @@ -269,60 +275,15 @@ describe('Controller test - Authorization controller test', () => { }); }); - it('| returns error', (done) => { - // setup - sinon.stub(AuthorizationController.prototype, '_getRefreshToken').callsArgWith(1, TEST_ERROR_MESSAGE); - - // call - authorizationController._getRefreshTokenAndUpdateConfig(TEST_PROFILE, (error, response) => { - // verify - expect(error).eq(TEST_ERROR_MESSAGE); - expect(response).eq(undefined); - done(); - }); - }); - }); - - describe('# test _getRefreshToken', () => { - let getTokenStub; - const authorizationController = new AuthorizationController(TEST_CONFIG); - - beforeEach(() => { - sinon.stub(httpClient, 'request'); - getTokenStub = sinon.stub(); - sinon.stub(AppConfig, 'getInstance').returns({ - getToken: getTokenStub - }); - }); - - afterEach(() => { - sinon.restore(); - }); - - it('| returns valid access token', (done) => { - // setup - httpClient.request.callsArgWith(3, null, TEST_RESPONSE); - getTokenStub.withArgs(TEST_PROFILE).returns(TEST_RESPONSE.body); - - // call - authorizationController._getRefreshToken(TEST_PROFILE, (error, accessToken) => { - // verify - expect(error).eq(null); - expect(accessToken).to.deep.eq(TEST_RESPONSE.body); - done(); - }); - }); - it('| returns error', (done) => { // setup httpClient.request.callsArgWith(3, TEST_ERROR_MESSAGE); - getTokenStub.withArgs(TEST_PROFILE).returns(TEST_RESPONSE.body); // call - authorizationController._getRefreshToken(TEST_PROFILE, (error, response) => { + authorizationController._getRefreshTokenAndUpdateConfig(TEST_PROFILE, TEST_TOKEN, (error, response) => { // verify expect(error).eq(TEST_ERROR_MESSAGE); - expect(response).eq(TEST_ERROR_MESSAGE); + expect(response).eq(undefined); done(); }); }); diff --git a/test/unit/utils/string-utils-test.js b/test/unit/utils/string-utils-test.js index 27d1e50a..ebb7889b 100644 --- a/test/unit/utils/string-utils-test.js +++ b/test/unit/utils/string-utils-test.js @@ -306,6 +306,11 @@ describe('Utils test - string utility', () => { testCase: 'input value is a valid profile name', value: 'askProfile', expectation: true + }, + { + testCase: 'input value is environment profile name', + value: '__ENVIRONMENT_ASK_PROFILE__', + expectation: false } ].forEach(({ testCase, value, expectation }) => {