Repository: badgateway/oauth2-client (ts)

Issue:
Calling `oAuth2Fetch.fetch()` immediately after invoking constructor causes race condition with async IIFE invoking `getStoredToken` 
This following code fails because getStoredToken is always invoked as an async function, meaning that `fetchWrapper.fetch()` will be called before `getStoredToken()` has even been called. See [this](https://github.com/badgateway/oauth2-client/blob/674630d3f912b978c73cd500601b1d0cdf0e71e0/src/fetch-wrapper.ts#L70) async IIFE. By the time the `fetchWrapper.fetch()` is called, the private property `fetchWrapper.token` is still null, so `getToken()` attempts to refresh it, which fails, invoking `refreshToken()` which attempts to refresh the non-existent token, which fails invoking `getNewToken()` which also fails, finally invoking `onError`, which prompts the user to log in again. When this is finished, control returns to the async IIFE, which finally sets the token to the correct value, and the next time `fetchWrapper.fetch()` is invoked, it behaves correctly. I have verified this using the debugger in Chrome DevTools. It is possible to create an async constructor by [returning an IIAFE from the constructor](https://stackoverflow.com/questions/43431550/async-await-class-constructor/50885340#50885340). However, this could be tricky without introducing a breaking change. Alternatively, the IIAFE could be stored in a private property which could be awaited inside of the `fetchWrapper.fetch()` function, ensuring that `getStoredToken()` has been invoked. Until a fix is released, I have a temporary workaround, but it is quite hacky, so I would appreciate a stable fix.

**Problematic Code**
```ts
const fetchWrapper = new OAuth2Fetch({
    client,
    getNewToken: () => {
      return null; // Fail this step, we don't want to log out until the user does so explicitly
    },
    storeToken,
    getStoredToken,
    onError: (error) => {
      // Prompt the user to log in again
    },
  });

const res = await fetchWrapper.fetch(...)
```

**Temporary workaround**
```ts
let ready = false;

const initialiseFetchWrapper = async () => {
  const fetchWrapper = new OAuth2Fetch({
    client,

    getNewToken: () => {
      return null; // Fail this step, we don't want to log out until the user does so explicitly
    },

    storeToken,

    getStoredToken,

    onError: (error) => {
      if (ready) {/* Prompt user to log in */}
    },
  });
  try {
    await fetchWrapper.getToken();
  } catch {
    // Ignore we expect this to error the first time
  }
  ready = true;
  return fetchWrapper;
};

let _fetchWrapper: OAuth2Fetch | null = null;

// We defer initialisation until the first call to fetchAuthenticated to ensure that initialisation has been completed
const getFetchWrapper = async () => {
  if (!_fetchWrapper) {
    _fetchWrapper = await initialiseFetchWrapper();
  }

  return _fetchWrapper;
};
```

Pull request description:
Fix getStoredToken race condition.
Fixes #110

Added test that failed before the fix.

Patch:
diff --git a/Makefile b/Makefile
index 2cb2b9c..7021e13 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ clean:
 	rm -r browser/
 	rm -r dist/
 
-dist/build:
+dist/build: ${SOURCE_FILES}
 	./node_modules/.bin/tsc
 	touch dist/build
 
diff --git a/changelog.md b/changelog.md
index d40c2af..b0a3cfa 100644
--- a/changelog.md
+++ b/changelog.md
@@ -5,6 +5,8 @@ Changelog
 ------------------
 
 * #111 Some documentation fixes.
+* #110: Fix race condition with `getStoredToken` and calling `fetch()`
+  immediately after constructing `FetchWrapper`.
 
 
 2.2.1 (2023-07-07)
diff --git a/src/fetch-wrapper.ts b/src/fetch-wrapper.ts
index fc52e41..55e53e3 100644
--- a/src/fetch-wrapper.ts
+++ b/src/fetch-wrapper.ts
@@ -46,13 +46,11 @@ type OAuth2FetchOptions = {
    * behavior is to schedule refresh. Set this to false to disable scheduling.
    */
   scheduleRefresh?: boolean;
-}
-
 
+}
 
 export class OAuth2Fetch {
 
-
   private options: OAuth2FetchOptions;
 
   /**
@@ -60,6 +58,15 @@ export class OAuth2Fetch {
    */
   private token: OAuth2Token | null = null;
 
+  /**
+   * If the user had a storedToken, the process to fetch it
+   * may be async. We keep track of this process in this
+   * promise, so it may be awaited to avoid race conditions.
+   *
+   * As soon as this promise resolves, this property get nulled.
+   */
+  private activeGetStoredToken: null | Promise<void> = null;
+
   constructor(options: OAuth2FetchOptions) {
 
     if (options?.scheduleRefresh === undefined) {
@@ -67,8 +74,9 @@ export class OAuth2Fetch {
     }
     this.options = options;
     if (options.getStoredToken) {
-      (async () => {
+      this.activeGetStoredToken = (async () => {
         this.token = await options.getStoredToken!();
+        this.activeGetStoredToken = null;
       })();
     }
     this.scheduleRefresh();
@@ -105,6 +113,7 @@ export class OAuth2Fetch {
   mw(): FetchMiddleware {
 
     return async (request, next) => {
+
       const accessToken = await this.getAccessToken();
 
       // Make a clone. We need to clone if we need to retry the request later.
@@ -157,6 +166,9 @@ export class OAuth2Fetch {
    */
   async getAccessToken(): Promise<string> {
 
+    // Ensure getStoredToken finished.
+    await this.activeGetStoredToken;
+
     const token = await this.getToken();
     return token.accessToken;
 
@@ -170,7 +182,6 @@ export class OAuth2Fetch {
    */
   private activeRefresh: Promise<OAuth2Token> | null = null;
 
-
   /**
    * Forces an access token refresh
    */


Test patch:
diff --git a/test/fetch-wrapper.ts b/test/fetch-wrapper.ts
new file mode 100644
index 0000000..8056d1b
--- /dev/null
+++ b/test/fetch-wrapper.ts
@@ -0,0 +1,133 @@
+import { OAuth2Fetch, OAuth2Client } from '../src';
+import { expect } from 'chai';
+
+describe('FetchWrapper', () => {
+
+  it('should use the token from getNewToken', async () => {
+
+    const client = new OAuth2Client({
+      clientId: 'foo',
+      clientSecret: 'bar'
+    });
+
+    const fetchWrapper = new OAuth2Fetch({
+      client,
+      getNewToken: () => {
+        return {
+          accessToken: 'access',
+          refreshToken: 'refresh',
+          expiresAt: Date.now()+1000_0000,
+        };
+      },
+    });
+
+    const mw = fetchWrapper.mw();
+
+    const response = await mw(
+      new Request('http://example/'),
+      (req): any => req
+    );
+
+    expect(
+      response.headers.get('Authorization')
+    ).to.equal('Bearer access');
+
+  });
+
+  it('should use the token even if it\'s delayed', async () => {
+
+    const client = new OAuth2Client({
+      clientId: 'foo',
+      clientSecret: 'bar'
+    });
+
+    const fetchWrapper = new OAuth2Fetch({
+      client,
+      getNewToken: async() => {
+        await new Promise(res => setTimeout(res, 200));
+        return {
+          accessToken: 'access',
+          refreshToken: 'refresh',
+          expiresAt: Date.now()+1000_0000,
+        };
+      },
+    });
+
+    const mw = fetchWrapper.mw();
+
+    const response = await mw(
+      new Request('http://example/'),
+      (req): any => req
+    );
+
+    expect(
+      response.headers.get('Authorization')
+    ).to.equal('Bearer access');
+
+  });
+
+  it('should use a token from getStoredToken', async () => {
+
+    const client = new OAuth2Client({
+      clientId: 'foo',
+      clientSecret: 'bar'
+    });
+
+    const fetchWrapper = new OAuth2Fetch({
+      client,
+      getNewToken: () => null,
+      getStoredToken: () => {
+        return {
+          accessToken: 'access',
+          refreshToken: 'refresh',
+          expiresAt: Date.now()+1000_0000,
+        };
+      },
+    });
+
+    const mw = fetchWrapper.mw();
+
+    const response = await mw(
+      new Request('http://example/'),
+      (req): any => req
+    );
+
+    expect(
+      response.headers.get('Authorization')
+    ).to.equal('Bearer access');
+
+  });
+
+  it('should still work with getStoredToken even if it\'s delayed', async () => {
+
+    const client = new OAuth2Client({
+      clientId: 'foo',
+      clientSecret: 'bar'
+    });
+
+    const fetchWrapper = new OAuth2Fetch({
+      client,
+      getNewToken: () => null,
+      getStoredToken: async() => {
+        await new Promise(res => setTimeout(res, 200));
+        return {
+          accessToken: 'access',
+          refreshToken: 'refresh',
+          expiresAt: Date.now()+1000_0000,
+        };
+      },
+    });
+
+    const mw = fetchWrapper.mw();
+
+    const response = await mw(
+      new Request('http://example/'),
+      (req): any => req
+    );
+
+    expect(
+      response.headers.get('Authorization')
+    ).to.equal('Bearer access');
+
+  });
+});