Refactor logout to use buildLogoutUrl

[rnwolfe]

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines 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: Return auth URL for usage with manual browser redirects #273
• Related PR: Added buildAuthorizeUrl and url parameter to handleRedirectCallback #280, Refactor logout to use new buildLogoutUrl method #536 36

Testing

__tests__/index.test.ts has been updated accordingly to mirror the changes made to Auth0Client.

• 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

    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

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

• I have added documentation for new/changed functionality in this PR or in auth0.com/docs
• All active GitHub checks for tests, formatting, and security are passing
• 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]

A few more things before we can get this merged.
Thanks again for making all the changes!

[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.

[frederikprijck]

Looks good, one minor test cleanup and we should be ready to go.

[rnwolfe]

Made the last couple of changes 👍

[frederikprijck]

@rnwolfe Looks good, could you merge in master in your branch?

[rnwolfe]

@frederikprijck merged

[stevehobbsdev]

Thanks for the contribution! 🎉

[rnwolfe]

@frederikprijck sorry to bolt on to the PR, but when trying to install the new committed version in npm (npm i --save https://github.com/auth0/auth0-spa-js.git#b27b3536486b23233d2b908721ac98f3632c14a2), I get the following error:

[!] Error: Cannot find module 'rollup-plugin-node-resolve'
Require stack:
- /Users/rwolfe/.npm/_cacache/tmp/git-clone-49a30027/rollup.config.js
- /Users/rwolfe/.npm-global/lib/node_modules/rollup/dist/shared/loadConfigFile.js
- /Users/rwolfe/.npm-global/lib/node_modules/rollup/dist/bin/rollup

The same was true when I tried from my fork, but this seems off as I have it installed globally, as well as it being in the local package.json. The build works when I build manually in the cloned repo and then copy the dist into my node_modules which is obviously not great.

Am I missing something here? I'm trying to use this in my app prior to npm release.

Thanks for any advice.

[frederikprijck]

I dont think this is supposed to be installed like that, as our artifacts published to npm are different from what we have in the repo (like we bundle things using rollup).

I think the easiest would be to checkout your branch locally (or the current master), build it and use npm or yarn link to link the auth0-spa-js to your local build output.

To some degree this is comparable to building and moving the files to the node_nodules, instead it doesnt involve manual copying but symlinking.

Something like this:

cd auth0-spa-js
npm run build
npm link
cd your-project
npm link auth0-spa-js

More info
https://docs.npmjs.com/cli/link
https://medium.com/dailyjs/how-to-use-npm-link-7375b6219557

However, this only helps u for local development. We should be shipping a new release early next week tho.

[rnwolfe]

@frederikprijck I see, thanks. I'll check that out for the time being.