diff --git a/lib/controllers/authorization-controller/index.js b/lib/controllers/authorization-controller/index.js index 1f4aad65..fa30712d 100644 --- a/lib/controllers/authorization-controller/index.js +++ b/lib/controllers/authorization-controller/index.js @@ -11,6 +11,7 @@ const Messenger = require("../../view/messenger"); const SpinnerView = require("../../view/spinner-view"); const messages = require("./messages"); +const ui = require("./ui"); module.exports = class AuthorizationController { /** @@ -147,11 +148,28 @@ module.exports = class AuthorizationController { listenSpinner.terminate(); const requestUrl = request.url; const requestQuery = url.parse(requestUrl, true).query; - server.destroy(); + + // Response from the browser with authentication code if (requestUrl.startsWith("/cb?code")) { response.end(messages.ASK_SIGN_IN_SUCCESS_MESSAGE); - callback(null, requestQuery.code); + ui.confirmAllowSignIn((error, confirmSignInChoice) => { + // Closing the socket port with server.destroy() only after confirmation question. + // See https://github.com/alexa/ask-cli/issues/476 + server.destroy(); + + if (error) { + return callback(error); + } + + if (!confirmSignInChoice) { + return callback(messages.STOP_UNCONFIRMED_BROWSER_SIGNIN); + } + + callback(null, requestQuery.code); + }); + return; } + server.destroy(); if (requestUrl.startsWith("/cb?error")) { response.statusCode = 403; const errorMessage = `Error: ${requestQuery.error}\nReason: ${requestQuery.error_description}`; diff --git a/lib/controllers/authorization-controller/messages.js b/lib/controllers/authorization-controller/messages.js index a9ce9185..c3380a23 100644 --- a/lib/controllers/authorization-controller/messages.js +++ b/lib/controllers/authorization-controller/messages.js @@ -56,3 +56,5 @@ module.exports.ASK_SIGN_IN_FAILURE_MESSAGE = (error) => ` `; module.exports.ASK_ENV_VARIABLES_ERROR_MESSAGE = "Could not find either of the environment variables: ASK_ACCESS_TOKEN, ASK_REFRESH_TOKEN"; + +module.exports.STOP_UNCONFIRMED_BROWSER_SIGNIN = "Stopping configuration due to unconfirmed browser sign in."; diff --git a/lib/controllers/authorization-controller/questions.js b/lib/controllers/authorization-controller/questions.js new file mode 100644 index 00000000..451eb4d2 --- /dev/null +++ b/lib/controllers/authorization-controller/questions.js @@ -0,0 +1,10 @@ +module.exports = { + CONFIRM_ALLOW_BROWSER_SIGN_IN: [ + { + message: "Do you confirm that you used the browser to sign in to Alexa Skills Kit Tools?", + type: "confirm", + name: "choice", + default: true, + }, + ], +}; diff --git a/lib/controllers/authorization-controller/ui.js b/lib/controllers/authorization-controller/ui.js new file mode 100644 index 00000000..ea429c58 --- /dev/null +++ b/lib/controllers/authorization-controller/ui.js @@ -0,0 +1,16 @@ +const inquirer = require("inquirer"); +const questions = require("./questions"); + +module.exports = { + confirmAllowSignIn, +}; +export function confirmAllowSignIn(callback) { + inquirer + .prompt(questions.CONFIRM_ALLOW_BROWSER_SIGN_IN) + .then((answer) => { + callback(null, answer.choice); + }) + .catch((error) => { + callback(error); + }); +} diff --git a/package.json b/package.json index bdb48d34..a35bb7fb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ask-cli", - "version": "2.30.0", + "version": "2.30.1", "description": "Alexa Skills Kit (ASK) Command Line Interfaces", "bin": { "ask": "dist/bin/ask.js" diff --git a/test/unit/controller/authorization-controller/index-test.js b/test/unit/controller/authorization-controller/index-test.js index 51faae43..05f01b9b 100644 --- a/test/unit/controller/authorization-controller/index-test.js +++ b/test/unit/controller/authorization-controller/index-test.js @@ -14,6 +14,7 @@ const CONSTANTS = require("../../../../lib/utils/constants"); const LocalHostServer = require("../../../../lib/utils/local-host-server"); const Messenger = require("../../../../lib/view/messenger"); const SpinnerView = require("../../../../lib/view/spinner-view"); +const ui = require("../../../../lib/controllers/authorization-controller/ui"); describe("Controller test - Authorization controller test", () => { const DEFAULT_CLIENT_ID = CONSTANTS.LWA.CLI_INTERNAL_ONLY_LWA_CLIENT.CLIENT_ID; @@ -456,40 +457,58 @@ describe("Controller test - Authorization controller test", () => { }); }); - it("| local server returns valid authCode", (done) => { - // setup - sinon.stub(LocalHostServer.prototype, "listen"); - sinon.stub(LocalHostServer.prototype, "registerEvent"); - const requestDestroyStub = sinon.stub(); - const request = { - url: "/cb?code", - socket: { - destroy: requestDestroyStub, - }, - }; - const endStub = sinon.stub(); - const response = { - on: sinon.stub().callsArgWith(1), - end: endStub, - }; - sinon.stub(SpinnerView.prototype, "terminate"); - const requestQuery = { - query: { - code: TEST_AUTH_CODE, - }, - }; - sinon.stub(url, "parse").returns(requestQuery); - sinon.stub(LocalHostServer.prototype, "destroy"); - sinon.stub(LocalHostServer.prototype, "create").callsArgWith(0, request, response); + describe("local server ", () => { + let endStub; + beforeEach(() => { + sinon.stub(LocalHostServer.prototype, "listen"); + sinon.stub(LocalHostServer.prototype, "registerEvent"); + const requestDestroyStub = sinon.stub(); + const request = { + url: "/cb?code", + socket: { + destroy: requestDestroyStub, + }, + }; + endStub = sinon.stub(); + const response = { + on: sinon.stub().callsArgWith(1), + end: endStub, + }; + sinon.stub(SpinnerView.prototype, "terminate"); + const requestQuery = { + query: { + code: TEST_AUTH_CODE, + }, + }; + sinon.stub(url, "parse").returns(requestQuery); + sinon.stub(LocalHostServer.prototype, "destroy"); + sinon.stub(LocalHostServer.prototype, "create").callsArgWith(0, request, response); + }); - // call - authorizationController._listenResponseFromLWA(TEST_PORT, (err, authCode) => { - // verify - expect(endStub.callCount).eq(1); - expect(endStub.args[0][0]).eq(messages.ASK_SIGN_IN_SUCCESS_MESSAGE); - expect(err).eq(null); - expect(authCode).eq(TEST_AUTH_CODE); - done(); + it("| returns valid authCode", (done) => { + sinon.stub(ui, "confirmAllowSignIn").callsArgWith(0, null, true); + // call + authorizationController._listenResponseFromLWA(TEST_PORT, (err, authCode) => { + // verify + expect(endStub.callCount).eq(1); + expect(endStub.args[0][0]).eq(messages.ASK_SIGN_IN_SUCCESS_MESSAGE); + expect(err).eq(null); + expect(authCode).eq(TEST_AUTH_CODE); + done(); + }); + }); + + it("| returns error for unconfirmed browser sign in", (done) => { + sinon.stub(ui, "confirmAllowSignIn").callsArgWith(0, null, false); + // call + authorizationController._listenResponseFromLWA(TEST_PORT, (err, authCode) => { + // verify + expect(endStub.callCount).eq(1); + expect(endStub.args[0][0]).eq(messages.ASK_SIGN_IN_SUCCESS_MESSAGE); + expect(err).eq(messages.STOP_UNCONFIRMED_BROWSER_SIGNIN); + expect(authCode).eq(undefined); + done(); + }); }); }); });