Repository: auth0/auth0-spa-js (ts)

Issue:
getTokenSilently Lock is not released on error
### Description

Whenever `getTokenSilently` runs, the `GET_TOKEN_SILENTLY_LOCK_KEY` lock is acquired, which is then released before the function returns. However, the lock is not released if an error is thrown, which is a very common occurrence given that a `"login_required"` error is thrown within `runIframe` if the user does not have an SSO session.

Within our current app this causes a major issue: Loading the application from a clean state redirects the user to universal login immediately, but upon being redirected back to our application, the initial calls to our back-end are delayed by five seconds, as the lock value is still in localStorage, and our calls to `getTokenSilently` are delayed waiting for the lock timeout. Only once we successfully get an access token through `getTokenSilently` does auth0-spa-js remove the local store value and allow us to get the value in good time through the cache.

I'm certain the issue we're seeing is related to the lock, given that manually deleting the localStorage value while awaiting a response makes the app respond immediately.

### Reproduction

The easiest way to see this problem is to use the `npm run dev` script in auth0-spa-js
- When the test page loads, open dev tools
- Click "Get access token with no interaction"
- Note that an `Uncaught (in promise) "login_required"` error is logged to the console almost immediately.
- Click "Get access token with no interaction" again
- The second time, it will (normally, see below*) take around five seconds for the error to be logged to the console. There is also normally a `TypeError: Cannot read property 'close' of undefined` error logged alongside.

(* this reproduces _most_ times I carry these steps - I imagine because browser-tabs-lock has some sort of timeout handling that is invalidating the lock if you take a while to click the button the second time?)

### Environment

Reproduces with v1.4.2 and v1.5.0, but works fine with 1.3.2.

All my tests have been in Chrome, and our app uses Angular, although I don't think this is relevant.

### Suggested fix

Not sure if it's this simple, but should the body of `getTokenSilently` not be wrapped in a `try/finally`, with the lock release in the `finally` block? I've tried this locally, and it resolves the issue as described in Reproduction above. I won't be able to try running our app with modified library code until next week, but please let me know if that would be helpful.

Pull request:
Released browser lock on getTokenSilently error
### Description

This PR ensures that the browser lock is released if `getTokenSilently` encounters an error. Without this, a potential delay of 5 seconds is introduced between calls could occur during silent authentication as the lock is not released.

### References

Fixes #275 

### Testing

Steps to reproduce:

* When the test page loads, open dev tools
* Click "Get access token with no interaction"
* Note that an Uncaught (in promise) "login_required" error is logged to the console almost immediately.
* Click "Get access token with no interaction" again
* The second time, it will (normally, see below*) take around five seconds for the error to be logged to the console. There is also normally a TypeError: Cannot read property 'close' of undefined error logged alongside.

- [X] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [ ] 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`


Patch:
diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts
index 7a00589..6b22784 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -310,61 +310,81 @@ export default class Auth0Client {
   ) {
     options.scope = getUniqueScopes(this.DEFAULT_SCOPE, options.scope);
 
-    await lock.acquireLock(GET_TOKEN_SILENTLY_LOCK_KEY, 5000);
-    if (!options.ignoreCache) {
-      const cache = this.cache.get({
-        scope: options.scope,
-        audience: options.audience || 'default'
+    try {
+      await lock.acquireLock(GET_TOKEN_SILENTLY_LOCK_KEY, 5000);
+
+      if (!options.ignoreCache) {
+        const cache = this.cache.get({
+          scope: options.scope,
+          audience: options.audience || 'default'
+        });
+
+        if (cache) {
+          lock.releaseLock(GET_TOKEN_SILENTLY_LOCK_KEY);
+          return cache.access_token;
+        }
+      }
+
+      const stateIn = encodeState(createRandomString());
+      const nonceIn = createRandomString();
+      const code_verifier = createRandomString();
+      const code_challengeBuffer = await sha256(code_verifier);
+      const code_challenge = bufferToBase64UrlEncoded(code_challengeBuffer);
+
+      const authorizeOptions = {
+        audience: options.audience,
+        scope: options.scope
+      };
+
+      const params = this._getParams(
+        authorizeOptions,
+        stateIn,
+        nonceIn,
+        code_challenge,
+        this.options.redirect_uri || window.location.origin
+      );
+
+      const url = this._authorizeUrl({
+        ...params,
+        prompt: 'none',
+        response_mode: 'web_message'
       });
-      if (cache) {
-        lock.releaseLock(GET_TOKEN_SILENTLY_LOCK_KEY);
-        return cache.access_token;
+
+      const codeResult = await runIframe(url, this.domainUrl);
+
+      if (stateIn !== codeResult.state) {
+        throw new Error('Invalid state');
       }
-    }
-    const stateIn = encodeState(createRandomString());
-    const nonceIn = createRandomString();
-    const code_verifier = createRandomString();
-    const code_challengeBuffer = await sha256(code_verifier);
-    const code_challenge = bufferToBase64UrlEncoded(code_challengeBuffer);
-    const authorizeOptions = {
-      audience: options.audience,
-      scope: options.scope
-    };
-    const params = this._getParams(
-      authorizeOptions,
-      stateIn,
-      nonceIn,
-      code_challenge,
-      this.options.redirect_uri || window.location.origin
-    );
-    const url = this._authorizeUrl({
-      ...params,
-      prompt: 'none',
-      response_mode: 'web_message'
-    });
 
-    const codeResult = await runIframe(url, this.domainUrl);
-    if (stateIn !== codeResult.state) {
-      throw new Error('Invalid state');
+      const authResult = await oauthToken({
+        baseUrl: this.domainUrl,
+        audience: options.audience || this.options.audience,
+        client_id: this.options.client_id,
+        code_verifier,
+        code: codeResult.code
+      });
+
+      const decodedToken = this._verifyIdToken(authResult.id_token, nonceIn);
+
+      const cacheEntry = {
+        ...authResult,
+        decodedToken,
+        scope: params.scope,
+        audience: params.audience || 'default'
+      };
+
+      this.cache.save(cacheEntry);
+
+      ClientStorage.save('auth0.is.authenticated', true, {
+        daysUntilExpire: 1
+      });
+
+      return authResult.access_token;
+    } catch (e) {
+      throw e;
+    } finally {
+      lock.releaseLock(GET_TOKEN_SILENTLY_LOCK_KEY);
     }
-    const authResult = await oauthToken({
-      baseUrl: this.domainUrl,
-      audience: options.audience || this.options.audience,
-      client_id: this.options.client_id,
-      code_verifier,
-      code: codeResult.code
-    });
-    const decodedToken = this._verifyIdToken(authResult.id_token, nonceIn);
-    const cacheEntry = {
-      ...authResult,
-      decodedToken,
-      scope: params.scope,
-      audience: params.audience || 'default'
-    };
-    this.cache.save(cacheEntry);
-    ClientStorage.save('auth0.is.authenticated', true, { daysUntilExpire: 1 });
-    lock.releaseLock(GET_TOKEN_SILENTLY_LOCK_KEY);
-    return authResult.access_token;
   }
 
   /**


Test patch:
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index 7ee7139..1110839 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -1068,6 +1068,24 @@ describe('Auth0', () => {
         scope: 'test:scope',
         ignoreCache: true
       };
+
+      it('releases the lock when there is an error', async () => {
+        const { auth0, lock, utils } = await setup();
+
+        utils.runIframe.mockReturnValue(Promise.reject(new Error('Failed')));
+
+        await expect(auth0.getTokenSilently()).rejects.toThrowError('Failed');
+
+        expect(lock.acquireLockMock).toHaveBeenCalledWith(
+          GET_TOKEN_SILENTLY_LOCK_KEY,
+          5000
+        );
+
+        expect(lock.releaseLockMock).toHaveBeenCalledWith(
+          GET_TOKEN_SILENTLY_LOCK_KEY
+        );
+      });
+
       it('encodes state with random string', async () => {
         const { auth0, utils } = await setup();
 
@@ -1171,18 +1189,25 @@ describe('Auth0', () => {
           'https://test.auth0.com'
         );
       });
+
       it('throws error if state from popup response is different from the provided state', async () => {
-        const { auth0, utils } = await setup();
+        const { auth0, utils, lock } = await setup();
 
         utils.runIframe.mockReturnValue(
           Promise.resolve({
             state: 'other-state'
           })
         );
+
         await expect(
           auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue)
         ).rejects.toThrowError('Invalid state');
+
+        expect(lock.releaseLockMock).toHaveBeenCalledWith(
+          GET_TOKEN_SILENTLY_LOCK_KEY
+        );
       });
+
       it('calls oauth/token with correct params', async () => {
         const { auth0, utils } = await setup();