Fix destructing logout response before error checking in parseLogoutR…

…esponse (#746)

* fixed destructing LogoutResponse before error checking

When the first argument indicating an error is passed to the callback of xml2js.parseString invoked from parseLogoutResponse, the second argument is undefined. However, second argument was being destructed before checking the first one, leading to an undefined dereferencing exception that ultimately overshadowed the original error.
Relevant issue: #744

* added test case for invalid xml in parseLogoutResponse

lib/logout.ts

@@ -15,11 +15,12 @@ const parseLogoutResponse = async (
     xml2js.parseString(
       rawResponse,
       { tagNameProcessors: [xml2js.processors.stripPrefix] },
-      (err: Error | null, { LogoutResponse }) => {
+      (err: Error | null, parsedData: { LogoutResponse: any }) => {
         if (err) {
           reject(err);
           return;
         }
+        const { LogoutResponse } = parsedData;
 
         resolve({
           issuer: LogoutResponse.Issuer[0]._,

test/lib/logout.spec.ts

@@ -4,6 +4,7 @@ import { parseLogoutResponse, createLogoutRequest } from '../../lib/logout';
 
 const response = fs.readFileSync('./test/assets/logout-response.xml').toString();
 const responseFailed = fs.readFileSync('./test/assets/logout-response-failed.xml').toString();
+const responseInvalid = 'invalid_data';
 
 describe('logout.ts', function () {
   it('response ok', async function () {
@@ -34,4 +35,16 @@ describe('logout.ts', function () {
     assert.strictEqual(!!req.id, true);
     assert.strictEqual(!!req.xml, true);
   });
+
+  it('should throw an expected error for response containing invalid xml', async function () {
+    await assert.rejects(
+      async () => {
+        await parseLogoutResponse(responseInvalid);
+      },
+      (error: any) => {
+        assert.strictEqual(error.message.includes('Non-whitespace before first tag'), true);
+        return true;
+      }
+    );
+  });
 });