Merge pull request #47 from Authress/logout-unit-tests

Add unit tests for logout.

.vscode/launch.json

@@ -0,0 +1,18 @@
+{
+    // Use IntelliSense to learn about possible attributes.
+    // Hover to view descriptions of existing attributes.
+    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+    "version": "0.2.0",
+    "configurations": [
+      {
+        "type": "node-terminal",
+        "request": "launch",
+        "name": "Debug tests",
+        "skipFiles": [
+          "<node_internals>/**"
+        ],
+        "command": "npm test"
+      }
+    ]
+  }
+

CHANGELOG.md

@@ -7,6 +7,7 @@ This is the changelog for [Authress Login](readme.md).
 * Add `antiAbuseHash` generation as part of authentication requests
 * clear the `nonce` and `iss` parameters from the URL when they are set.
 * [Fix] Force a sessionCheck after a logout.
+* Validate logout redirect urls to ensure they are valid before attempting to log the user out.
 
 ## 2.4 ##
 * Prevent silent returns from `authenticate` when a different connectionId is used to have the user log in.

package.json

@@ -12,7 +12,7 @@
   "scripts": {
     "build": "node make.js build && NODE_ENV=production webpack --mode=production",
     "lint": "eslint --ext .js,.ts src tests make.js index.d.ts",
-    "test": "check-dts index.d.ts && mocha tests/**/*.test.js -R spec"
+    "test": "check-dts index.d.ts && mocha tests/*.test.js tests/**/*.test.js -R spec"
   },
   "dependencies": {
     "cookie": "^0.5.0",

src/index.js

@@ -691,8 +691,26 @@ class LoginClient {
   /**
    * @description Log the user out removing the current user's session. If the user is not logged in this has no effect. If the user is logged in via secure session, the the redirect url will be ignored. If the user is logged in without a secure session the user agent will be redirected to the hosted login and then redirected to the {@link redirectUrl}.
    * @param {String} [redirectUrl='window.location.href'] Optional redirect location to return the user to after logout. Will only be used for cross domain sessions.
+   * @throws InvalidRedirectUrl - When the passed in redirect url is not a valid url.
    */
-  async logout(redirectUrl) {
+  async logout(requestedRedirectUrl) {
+    let redirectUrl;
+    if (requestedRedirectUrl) {
+      try {
+        // eslint-disable-next-line no-new
+        new URL(requestedRedirectUrl);
+        redirectUrl = requestedRedirectUrl;
+      } catch (error) {
+        try {
+          redirectUrl = new URL(requestedRedirectUrl, windowManager.getCurrentLocation().href).toString();
+        } catch (relativeRedirectUrlAlsoFailed) {
+          const e = Error(`The logout redirect url is not valid URL: ${requestedRedirectUrl}`);
+          e.code = 'InvalidRedirectUrl';
+          throw e;
+        }
+      }
+    }
+
     userIdentityTokenStorageManager.clear();
 
     // Terminate all query parameters in the URL which might trick the app into thinking that the user is still logged in. Any property that is associated with Authress should be removed.
@@ -704,8 +722,10 @@ class LoginClient {
       try {
         await this.httpClient.delete('/session', this.enableCredentials);
         this.lastSessionCheck = 0;
-        if (redirectUrl && redirectUrl !== windowManager.getCurrentLocation().href) {
-          windowManager.assign(redirectUrl);
+        // We use the requestedRedirectUrl here and not the redirectUrl because `windowManager.assign` actually accepts relative urls
+        // * AND we don't want to force a navigation if we are already in the right location.
+        if (requestedRedirectUrl && requestedRedirectUrl !== windowManager.getCurrentLocation().href) {
+          windowManager.assign(requestedRedirectUrl);
         }
         return;
       } catch (error) { /**/ }

tests/loginClient/logout.test.js

@@ -0,0 +1,245 @@
+const { describe, it, beforeEach, afterEach } = require('mocha');
+const sinon = require('sinon');
+const { expect } = require('chai');
+
+const { LoginClient } = require('../../src/index');
+const windowManager = require('../../src/windowManager');
+const userIdentityTokenStorageManager = require('../../src/userIdentityTokenStorageManager.js');
+const httpClient = require('../../src/httpClient.js');
+
+let sandbox;
+beforeEach(() => { sandbox = sinon.createSandbox(); });
+afterEach(() => sandbox.restore());
+
+let requestedRedirectUrl;
+
+requestedRedirectUrl = 'https://valid-redirect.url';
+
+describe('loginClient.js', () => {
+  describe('logout', () => {
+    it('should clear the user identity token storage and sanitize query parameters', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://unit-test.authress.io', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      await loginClient.logout(requestedRedirectUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(true);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+    });
+
+    it('should attempt to delete the session if credentials are enabled', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const deleteMock = sandbox.mock(httpClient.prototype).expects('delete').once().withArgs('/session', true);
+      const assignMock = sandbox.mock(windowManager).expects('assign').once().withArgs(requestedRedirectUrl);
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://unit-test.authress.io', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      loginClient.enableCredentials = true;
+      await loginClient.logout(requestedRedirectUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(false);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      deleteMock.verify();
+      assignMock.verify();
+    });
+
+    it('should attempt to delete the session if credentials are enabled and work for relative urls as well', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const relativeUrl = '/relative-url';
+
+      const deleteMock = sandbox.mock(httpClient.prototype).expects('delete').once().withArgs('/session', true);
+      const assignMock = sandbox.mock(windowManager).expects('assign').once().withArgs(relativeUrl);
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://unit-test.authress.io', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      loginClient.enableCredentials = true;
+      await loginClient.logout(relativeUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(false);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      deleteMock.verify();
+      assignMock.verify();
+    });
+
+    it('should attempt to delete the session if credentials are enabled and work for no redirect url presented', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const relativeUrl = null;
+
+      const deleteMock = sandbox.mock(httpClient.prototype).expects('delete').once().withArgs('/session', true);
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://unit-test.authress.io', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      loginClient.enableCredentials = true;
+      await loginClient.logout(relativeUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(false);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      deleteMock.verify();
+    });
+
+    it('should assign fullLogoutUrl if session deletion fails', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const deleteMock = sandbox.mock(httpClient.prototype).expects('delete').rejects(new Error('Failed to delete session'));
+      const assignMock = sandbox.mock(windowManager).expects('assign').once();
+      const getCurrentLocationMock = sandbox.mock(windowManager).expects('getCurrentLocation').returns({ href: 'https://current.location' });
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://auth.example.com', applicationId: 'app_id', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      loginClient.enableCredentials = true;
+      await loginClient.logout(requestedRedirectUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(true);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      deleteMock.verify();
+      assignMock.verify();
+      getCurrentLocationMock.verify();
+    });
+
+    it('should assign the fullLogoutUrl with redirect_uri and client_id when credentials are not enabled', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const fullLogoutUrl = 'https://auth.example.com/logout?redirect_uri=https%3A%2F%2Fvalid-redirect.url&client_id=app_id';
+      const assignMock = sandbox.mock(windowManager).expects('assign').once().withArgs(fullLogoutUrl);
+      const getCurrentLocationMock = sandbox.mock(windowManager).expects('getCurrentLocation').returns({ href: 'https://valid-redirect.url' });
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://auth.example.com', applicationId: 'app_id', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      await loginClient.logout(requestedRedirectUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(true);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      assignMock.verify();
+      getCurrentLocationMock.verify();
+    });
+
+    it('should handle relative requestedRedirectUrl and resolve using current location as /', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const relativeUrl = '/';
+
+      const windowManagerMock = sandbox.mock(windowManager);
+      windowManagerMock.expects('assign').once().withArgs('https://auth.example.com/logout?redirect_uri=https%3A%2F%2Fcurrent.location%2F&client_id=app_id');
+      windowManagerMock.expects('getCurrentLocation').twice().returns({ href: 'https://current.location' });
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://auth.example.com', applicationId: 'app_id', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      await loginClient.logout(relativeUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(true);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      windowManagerMock.verify();
+    });
+
+    it('should handle relative requestedRedirectUrl and resolve using current location as /relative-url', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+
+      const relativeUrl = '/relative-url';
+      const windowManagerMock = sandbox.mock(windowManager);
+      windowManagerMock.expects('assign').once().withArgs('https://auth.example.com/logout?redirect_uri=https%3A%2F%2Fcurrent.location%2Frelative-url&client_id=app_id');
+      windowManagerMock.expects('getCurrentLocation').twice().returns({ href: 'https://current.location' });
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://auth.example.com', applicationId: 'app_id', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      await loginClient.logout(relativeUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(true);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      windowManagerMock.verify();
+    });
+
+    it('should set lastSessionCheck to 0 after logging out', async () => {
+      const setTimeoutStub = sandbox.stub(global, 'setTimeout').callsFake(cb => cb());
+      const assignMock = sandbox.mock(windowManager).expects('assign').once();
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://auth.example.com', applicationId: 'app_id', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      loginClient.lastSessionCheck = 12345;
+      await loginClient.logout(requestedRedirectUrl);
+
+      expect(setTimeoutStub.calledOnce).to.eql(true);
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      expect(loginClient.lastSessionCheck).to.equal(0);
+      assignMock.verify();
+    });
+
+    it('should wait for 500ms after logging out', async () => {
+      const clock = sandbox.useFakeTimers();
+      const assignMock = sandbox.mock(windowManager).expects('assign').once();
+
+      const userIdentityTokenStorageManagerMock = sandbox.mock(userIdentityTokenStorageManager);
+      userIdentityTokenStorageManagerMock.expects('clear').once();
+
+      const loginClient = new LoginClient({ authressApiUrl: 'https://auth.example.com', applicationId: 'app_id', skipBackgroundCredentialsCheck: true });
+      const sanitizeQueryParametersStub = sandbox.stub(loginClient, 'sanitizeQueryParameters');
+      sanitizeQueryParametersStub.returns();
+
+      const logoutAsync = loginClient.logout(requestedRedirectUrl);
+      clock.tick(500);
+      await logoutAsync;
+
+      expect(sanitizeQueryParametersStub.calledOnce).to.eql(true);
+      userIdentityTokenStorageManagerMock.verify();
+      assignMock.verify();
+      clock.restore();
+    });
+  });
+});