Merge pull request #482 from alexa/lumas/hijack2

bug: Local Credential Hijacking fix => round 2
alexa · Jul 5, 2023 · 162f62d · 162f62d
2 parents 4f3391b + e096f42
commit 162f62d
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 7 deletions.
diff --git a/lib/controllers/authorization-controller/index.js b/lib/controllers/authorization-controller/index.js
@@ -153,7 +153,7 @@ module.exports = class AuthorizationController {
       if (requestUrl.startsWith("/cb?code")) {
         response.end(messages.ASK_SIGN_IN_SUCCESS_MESSAGE);
         ui.confirmAllowSignIn((error, confirmSignInChoice) => {
-          // Closing the socket port with server.destroy() only after confirmation question. 
+          // Closing the socket port with server.destroy() only after confirmation question.
           // See https://github.com/alexa/ask-cli/issues/476
           server.destroy();
 
@@ -164,17 +164,22 @@ module.exports = class AuthorizationController {
           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}`;
+        const errorMessage = `Error: ${requestQuery.error}\nReason: ${requestQuery.error_description}`.split("\n").join(". ");
         response.end(messages.ASK_SIGN_IN_FAILURE_MESSAGE(errorMessage));
-        callback(errorMessage);
+        ui.informReceivedError((error, _) => {
+          // Closing the socket port with server.destroy() only after informing of error.
+          // See https://github.com/alexa/ask-cli/issues/476
+          server.destroy();
+          response.statusCode = 403;
+          callback(errorMessage);
+        }, errorMessage);
       }
     }
   }

diff --git a/lib/controllers/authorization-controller/questions.js b/lib/controllers/authorization-controller/questions.js
@@ -7,4 +7,11 @@ module.exports = {
       default: true,
     },
   ],
+  INFORM_ERROR: {
+    // message is filled out in code
+    type: "list",
+    choices: ["Ok"],
+    name: "choice",
+    default: "Ok",
+  },
 };
diff --git a/lib/controllers/authorization-controller/ui.js b/lib/controllers/authorization-controller/ui.js
@@ -3,6 +3,7 @@ const questions = require("./questions");
 
 module.exports = {
   confirmAllowSignIn,
+  informReceivedError
 };
 export function confirmAllowSignIn(callback) {
   inquirer
@@ -14,3 +15,14 @@ export function confirmAllowSignIn(callback) {
       callback(error);
     });
 }
+
+export function informReceivedError(callback, error) {
+  inquirer
+    .prompt([{...questions.INFORM_ERROR, message: `Sign in error: ${error}.`}])
+    .then((answer) => {
+      callback(null, answer.choice);
+    })
+    .catch((error) => {
+      callback(error);
+    });
+}
diff --git a/test/unit/controller/authorization-controller/index-test.js b/test/unit/controller/authorization-controller/index-test.js
@@ -422,6 +422,7 @@ describe("Controller test - Authorization controller test", () => {
       // setup
       sinon.stub(LocalHostServer.prototype, "listen");
       sinon.stub(LocalHostServer.prototype, "registerEvent");
+      const informReceivedErrorStub = sinon.stub(ui, "informReceivedError").callsArgWith(0, null, "Ok");
       const requestDestroyStub = sinon.stub();
       const request = {
         url: "/cb?error",
@@ -447,12 +448,13 @@ describe("Controller test - Authorization controller test", () => {
       // call
       authorizationController._listenResponseFromLWA(TEST_PORT, (err) => {
         // verify
-        const EXPECTED_ERR_MESSAGE = `Error: ${requestQuery.query.error}\nReason: ${requestQuery.query.error_description}`;
+        const EXPECTED_ERR_MESSAGE = `Error: ${requestQuery.query.error}\nReason: ${requestQuery.query.error_description}`.split("\n").join(". ");
         expect(spinnerTerminateStub.callCount).eq(1);
         expect(serverDestroyStub.callCount).eq(1);
         expect(endStub.callCount).eq(1);
         expect(endStub.args[0][0].includes(EXPECTED_ERR_MESSAGE)).equal(true);
         expect(err).eq(EXPECTED_ERR_MESSAGE);
+        expect(informReceivedErrorStub.called).to.be.true;
         done();
       });
     });