Repository: auth0/auth0-spa-js
Pull request #595 (2020-10-12 15:35:32)

Refactor logout to use buildLogoutUrl
By submitting a PR to this repository, you agree to the terms within the [Auth0 Code of Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md). Please see the [contributing guidelines](https://github.com/auth0/.github/blob/master/CONTRIBUTING.md) for how to create and submit a high-quality PR for this repo.

### Description

This PR seeks to build a logout URL builder function (`buildLogoutUrl`) that the `logout` function then uses. This is in line with the changes made for `buildAuthorizeUrl` and `authorize` in #280. This allows for custom handling of launching the URL instead of only using `window.location.assign`. This allows for easier support of hybrid apps using things like Cordova or Capacitor.

**Note**: PR #273 sought to do this, but I re-worked it a bit based on the comments on my own fork. 

### References

- Issue: #273 
- Related PR: #280, #536 36


### Testing

`__tests__/index.test.ts` has been updated accordingly to mirror the changes made to `Auth0Client`.

- [x] This change adds test coverage for new/changed/fixed functionality

One notable thing was in the Jest tests, the `expectToHaveBeenCalledWithAuth0ClientParam` was catching two mocks (one from the prior function). The logged output below: 

```
      [
        [
          'https://test.auth0.com/v2/logout?query=params&auth0Client=eyJuYW1lIjoiYXV0aDAtc3BhLWpzIiwidmVyc2lvbiI6IjEuMTIuMSJ9&federated'
        ],
        [
          'https://test.auth0.com/v2/logout?query=params&auth0Client=eyJuYW1lIjoiX190ZXN0X2NsaWVudF9uYW1lX18iLCJ2ZXJzaW9uIjoiOS45LjkifQ%3D%3D'
        ]
      ]
```

This was bewildering to me and was only happening for this test: 

https://github.com/ironbow/auth0-spa-js/blob/6ff9ece3a78d5e7db855d37ef45e649184257a45/__tests__/index.test.ts#L2236-L2244
```javascript
    it('calls `window.location.assign` with the correct url with custom `options.auth0Client`', async () => {
      const auth0Client = { name: '__test_client_name__', version: '9.9.9' };
      const { auth0 } = await setup({ auth0Client });
      auth0.logout();
      expectToHaveBeenCalledWithAuth0ClientParam(
        window.location.assign,
        auth0Client
      );
    });
```
My means of getting around this was updating the helper function to references the last mock result in the array, rather than assuming there is only one:

https://github.com/ironbow/auth0-spa-js/blob/1054ba48d8054a97dda0ed6e6fbc83fd46b6b8f1/__tests__/helpers.ts#L2
```javascript
const url = (<jest.Mock>mock).mock.calls[mock.mock.calls.length - 1][0];
```
This may not be an appropriate fix, but I could not figure out what was causing it as the mocks should be cleared after each test.

### Checklist

- [x] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [x] All active GitHub checks for tests, formatting, and security are passing
- [x] The correct base branch is being used, if not `master`


frederikprijck: Added a few more comments. 

Thanks for the PR @rnwolfe , appreciate the work. I think we are almost to a point where we can get this approved.
frederikprijck: Thanks, this looks great @rnwolfe! 🎉  I will give this a last round of testing first thing tomorrow (getting a bit late here).
rnwolfe: Sounds good @frederikprijck! 👍 Thanks for your promptness. I'll keep an eye on things for tomorrow. 

I did just notice a failed cypress test in the CI on the last commit but it may be a fluke as it was successful in my environment.
frederikprijck: @rnwolfe CI is fine, it appears to have been a flake.
rnwolfe: Made the last couple of changes 👍 
frederikprijck: @rnwolfe Looks good, could you merge in master ?
rnwolfe: @frederikprijck merged

Patch:
diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts
index c0d0e78..6392ba8 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -52,7 +52,8 @@ import {
   LogoutOptions,
   RefreshTokenOptions,
   OAuthTokenOptions,
-  CacheLocation
+  CacheLocation,
+  LogoutUrlOptions
 } from './global';
 
 // @ts-ignore
@@ -695,6 +696,28 @@ export default class Auth0Client {
     return !!user;
   }
 
+  /**
+   * ```js
+   * await auth0.buildLogoutUrl(options);
+   * ```
+   *
+   * Builds a URL to the logout endpoint using the parameters provided as arguments.
+   * @param options
+   */
+  public buildLogoutUrl(options: LogoutUrlOptions = {}): string {
+    if (options.client_id !== null) {
+      options.client_id = options.client_id || this.options.client_id;
+    } else {
+      delete options.client_id;
+    }
+
+    const { federated, ...logoutOptions } = options;
+    const federatedQuery = federated ? `&federated` : '';
+    const url = this._url(`/v2/logout?${createQueryParams(logoutOptions)}`);
+
+    return url + federatedQuery;
+  }
+
   /**
    * ```js
    * auth0.logout();
@@ -711,15 +734,9 @@ export default class Auth0Client {
    * @param options
    */
   public logout(options: LogoutOptions = {}) {
-    if (options.client_id !== null) {
-      options.client_id = options.client_id || this.options.client_id;
-    } else {
-      delete options.client_id;
-    }
-
-    const { federated, localOnly, ...logoutOptions } = options;
+    const { localOnly, ...logoutOptions } = options;
 
-    if (localOnly && federated) {
+    if (localOnly && logoutOptions.federated) {
       throw new Error(
         'It is invalid to set both the `federated` and `localOnly` options to `true`'
       );
@@ -731,11 +748,8 @@ export default class Auth0Client {
     if (localOnly) {
       return;
     }
-
-    const federatedQuery = federated ? `&federated` : '';
-    const url = this._url(`/v2/logout?${createQueryParams(logoutOptions)}`);
-
-    window.location.assign(`${url}${federatedQuery}`);
+    const url = this.buildLogoutUrl(logoutOptions);
+    window.location.assign(url);
   }
 
   private async _getTokenFromIFrame(
diff --git a/src/global.ts b/src/global.ts
index 808f4b9..544443d 100644
--- a/src/global.ts
+++ b/src/global.ts
@@ -281,6 +281,41 @@ export interface GetTokenSilentlyOptions {
 
 export interface GetTokenWithPopupOptions extends PopupLoginOptions {}
 
+export interface LogoutUrlOptions {
+  /**
+   * The URL where Auth0 will redirect your browser to after the logout.
+   *
+   * **Note**: If the `client_id` parameter is included, the
+   * `returnTo` URL that is provided must be listed in the
+   * Application's "Allowed Logout URLs" in the Auth0 dashboard.
+   * However, if the `client_id` parameter is not included, the
+   * `returnTo` URL must be listed in the "Allowed Logout URLs" at
+   * the account level in the Auth0 dashboard.
+   *
+   * [Read more about how redirecting after logout works](https://auth0.com/docs/logout/guides/redirect-users-after-logout)
+   */
+  returnTo?: string;
+
+  /**
+   * The `client_id` of your application.
+   *
+   * If this property is not set, then the `client_id` that was used during initialization of the SDK is sent to the logout endpoint.
+   *
+   * If this property is set to `null`, then no client ID value is sent to the logout endpoint.
+   *
+   * [Read more about how redirecting after logout works](https://auth0.com/docs/logout/guides/redirect-users-after-logout)
+   */
+  client_id?: string;
+
+  /**
+   * When supported by the upstream identity provider,
+   * forces the user to logout of their identity provider
+   * and from Auth0.
+   * [Read more about how federated logout works at Auth0](https://auth0.com/docs/logout/guides/logout-idps)
+   */
+  federated?: boolean;
+}
+
 export interface LogoutOptions {
   /**
    * The URL where Auth0 will redirect your browser to after the logout.


Test patch:
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index 0d319c1..880c7b7 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -2165,19 +2165,11 @@ describe('Auth0', () => {
     });
   });
 
-  describe('logout()', () => {
-    it('removes `auth0.is.authenticated` key from storage', async () => {
-      const { auth0, cookieStorage } = await setup();
-      auth0.logout();
-      expect(cookieStorage.remove).toHaveBeenCalledWith(
-        'auth0.is.authenticated'
-      );
-    });
-
+  describe('buildLogoutUrl()', () => {
     it('creates correct query params with empty options', async () => {
       const { auth0, utils } = await setup();
 
-      auth0.logout();
+      auth0.buildLogoutUrl();
       expect(utils.createQueryParams).toHaveBeenCalledWith({
         client_id: TEST_CLIENT_ID
       });
@@ -2186,14 +2178,14 @@ describe('Auth0', () => {
     it('creates correct query params with `options.client_id` is null', async () => {
       const { auth0, utils } = await setup();
 
-      auth0.logout({ client_id: null });
+      auth0.buildLogoutUrl({ client_id: null });
       expect(utils.createQueryParams).toHaveBeenCalledWith({});
     });
 
     it('creates correct query params with `options.client_id` defined', async () => {
       const { auth0, utils } = await setup();
 
-      auth0.logout({ client_id: 'another-client-id' });
+      auth0.buildLogoutUrl({ client_id: 'another-client-id' });
       expect(utils.createQueryParams).toHaveBeenCalledWith({
         client_id: 'another-client-id'
       });
@@ -2202,7 +2194,7 @@ describe('Auth0', () => {
     it('creates correct query params with `options.returnTo` defined', async () => {
       const { auth0, utils } = await setup();
 
-      auth0.logout({ returnTo: 'https://return.to', client_id: null });
+      auth0.buildLogoutUrl({ returnTo: 'https://return.to', client_id: null });
       expect(utils.createQueryParams).toHaveBeenCalledWith({
         returnTo: 'https://return.to'
       });
@@ -2211,9 +2203,19 @@ describe('Auth0', () => {
     it('creates correct query params when `options.federated` is true', async () => {
       const { auth0, utils } = await setup();
 
-      auth0.logout({ federated: true, client_id: null });
+      auth0.buildLogoutUrl({ federated: true, client_id: null });
       expect(utils.createQueryParams).toHaveBeenCalledWith({});
     });
+  });
+
+  describe('logout()', () => {
+    it('removes `auth0.is.authenticated` key from storage', async () => {
+      const { auth0, cookieStorage } = await setup();
+      auth0.logout();
+      expect(cookieStorage.remove).toHaveBeenCalledWith(
+        'auth0.is.authenticated'
+      );
+    });
 
     it('calls `window.location.assign` with the correct url', async () => {
       const { auth0 } = await setup();