fix authentication - skip client cache

Signed-off-by: Adam Setch <adam.setch@outlook.com>

Author: setchy

src/renderer/utils/api/client.ts

@@ -37,8 +37,11 @@ import { getNumberFromUrl } from './utils';
 /**
  * Fetch details of the currently authenticated GitHub user.
  */
-export async function fetchAuthenticatedUserDetails(account: Account) {
-  const octokit = await createOctokitClient(account, 'rest');
+export async function fetchAuthenticatedUserDetails(
+  account: Account,
+  skipClientCache = false,
+) {
+  const octokit = await createOctokitClient(account, 'rest', skipClientCache);
 
   return await octokit.rest.users.getAuthenticated();
 }

src/renderer/utils/api/octokit.test.ts

@@ -127,6 +127,34 @@ describe('renderer/utils/api/octokit.ts', () => {
       expect(getGitHubAPIBaseUrlSpy).toHaveBeenCalledTimes(1);
     });
 
+    it('should bypass cache when skipClientCache is true', async () => {
+      const getGitHubAPIBaseUrlSpy = jest.spyOn(utils, 'getGitHubAPIBaseUrl');
+      getGitHubAPIBaseUrlSpy.mockReturnValue(
+        new URL('https://api.github.com/'),
+      );
+
+      const cachedClient = await createOctokitClient(
+        mockGitHubCloudAccount,
+        'rest',
+      );
+
+      const nonCachedClient = await createOctokitClient(
+        mockGitHubCloudAccount,
+        'rest',
+        true,
+      );
+
+      const cachedClientAgain = await createOctokitClient(
+        mockGitHubCloudAccount,
+        'rest',
+      );
+
+      expect(cachedClient).toBe(cachedClientAgain);
+      expect(nonCachedClient).not.toBe(cachedClient);
+      expect(mockDecryptValue).toHaveBeenCalledTimes(2);
+      expect(getGitHubAPIBaseUrlSpy).toHaveBeenCalledTimes(2);
+    });
+
     it('should create different clients for different accounts with same api type', async () => {
       const getGitHubAPIBaseUrlSpy = jest.spyOn(utils, 'getGitHubAPIBaseUrl');
       getGitHubAPIBaseUrlSpy.mockReturnValue(

src/renderer/utils/api/octokit.ts

@@ -39,13 +39,17 @@ export function clearOctokitClientCache(): void {
 export async function createOctokitClient(
   account: Account,
   type: APIClientType,
+  skipClientCache = false,
 ): Promise<OctokitClient> {
-  const cacheKey = getClientCacheKey(account, type);
+  let cacheKey: string;
 
   // Return cached client if it exists
-  const cachedClient = octokitClientCache.get(cacheKey);
-  if (cachedClient) {
-    return cachedClient;
+  if (!skipClientCache) {
+    cacheKey = getClientCacheKey(account, type);
+    const cachedClient = octokitClientCache.get(cacheKey);
+    if (cachedClient) {
+      return cachedClient;
+    }
   }
 
   const decryptedToken = await decryptValue(account.token);
@@ -61,8 +65,10 @@ export async function createOctokitClient(
     userAgent: userAgent,
   });
 
-  // Cache the client keyed by account UUID + type
-  octokitClientCache.set(cacheKey, client);
+  // Cache the client keyed by account UUID + type unless explicitly skipped
+  if (!skipClientCache) {
+    octokitClientCache.set(cacheKey, client);
+  }
 
   return client;
 }

src/renderer/utils/auth/utils.ts

@@ -91,7 +91,9 @@ export async function exchangeAuthCodeForAccessToken(
     clientSecret: authOptions.clientSecret,
     code: authCode,
     request: request.defaults({
-      baseUrl: getGitHubAuthBaseUrl(authOptions.hostname).toString(),
+      baseUrl: getGitHubAuthBaseUrl(authOptions.hostname)
+        .toString()
+        .replace(/\/$/, ''),
     }),
   });
 
@@ -114,7 +116,7 @@ export async function addAccount(
     token: encryptedToken,
   } as Account;
 
-  newAccount = await refreshAccount(newAccount);
+  newAccount = await refreshAccount(newAccount, true);
   const newAccountUUID = getAccountUUID(newAccount);
 
   const accountAlreadyExists = accountList.some(
@@ -145,9 +147,16 @@ export function removeAccount(auth: AuthState, account: Account): AuthState {
   };
 }
 
-export async function refreshAccount(account: Account): Promise<Account> {
+export async function refreshAccount(
+  account: Account,
+  skipClientCache = false,
+): Promise<Account> {
   try {
-    const response = await fetchAuthenticatedUserDetails(account);
+    const response = await fetchAuthenticatedUserDetails(
+      account,
+      skipClientCache,
+    );
+
     const user = response.data;
 
     // Refresh user data
@@ -185,7 +194,7 @@ export async function refreshAccount(account: Account): Promise<Account> {
   } catch (err) {
     rendererLogError(
       'refreshAccount',
-      `failed to refresh account for user ${account.user.login}`,
+      `failed to refresh account for user ${account.user?.login ?? account.hostname}`,
       err,
     );
   }