Merge pull request #13542 from afscrome/feature/OptimiseGithubAccessTokens

Optimise GitHub access tokens

.changeset/sweet-grapes-explain.md

@@ -0,0 +1,8 @@
+---
+'@backstage/integration': patch
+---
+
+Improved caching around github app tokens.
+Tokens are now cached for 50 minutes, not 10.
+Calls to get app installations are also included in this cache.
+If you have more than one github app configured, consider adding `allowedInstallationOwners` to your apps configuration to gain the most benefit from these performance changes.

docs/integrations/github/github-apps.md

@@ -98,7 +98,9 @@ integrations:
 ### Limiting the GitHub App installations
 
 If you want to limit the GitHub app installations visible to backstage you may
-optionally include the `allowedInstallationOwners` option.
+optionally include the `allowedInstallationOwners` option. If you configure
+multiple apps, specifying this will bring some small performance benefits
+as backstage can more easily select which app to use for a URL.
 
 ```yaml
 appId: app id

packages/integration/src/github/SingleInstanceGithubCredentialsProvider.test.ts

@@ -57,6 +57,7 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => {
       token: 'hardcoded_token',
     });
   });
+
   it('create repository specific tokens', async () => {
     octokit.apps.listInstallations.mockResolvedValue({
       headers: {
@@ -66,11 +67,6 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => {
         {
           id: 1,
           repository_selection: 'selected',
-          account: null,
-        },
-        {
-          id: 2,
-          repository_selection: 'selected',
           account: {
             login: 'backstage',
           },
@@ -195,9 +191,14 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => {
       data: {
         expires_at: DateTime.local().plus({ hours: 1 }).toString(),
         token: 'secret_token',
+        repository_selection: 'selected',
       },
     } as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']);
 
+    octokit.apps.listReposAccessibleToInstallation.mockReturnValue({
+      data: [{ name: 'some-repo' }],
+    } as unknown as RestEndpointMethodTypes['apps']['listReposAccessibleToInstallation']['response']);
+
     const { token, headers } = await github.getCredentials({
       url: 'https://github.com/backstage',
     });
@@ -400,4 +401,70 @@ describe('SingleInstanceGithubCredentialsProvider tests', () => {
       }),
     ).resolves.not.toThrow();
   });
+
+  it('should cache access token', async () => {
+    octokit.apps.listInstallations.mockReturnValue({
+      headers: {
+        etag: '123',
+      },
+      data: [
+        {
+          id: 1,
+          repository_selection: 'all',
+          account: {
+            login: 'backstage',
+          },
+        },
+      ],
+    } as RestEndpointMethodTypes['apps']['listInstallations']['response']);
+
+    octokit.apps.createInstallationAccessToken.mockReturnValue({
+      data: {
+        expires_at: DateTime.local().plus({ minutes: 11 }).toString(),
+        token: 'secret_token',
+      },
+    } as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']);
+
+    await github.getCredentials({ url: 'https://github.com/backstage' });
+    await github.getCredentials({ url: 'https://github.com/backstage' });
+
+    expect(octokit.apps.listInstallations.mock.calls.length).toBe(1);
+    expect(octokit.apps.createInstallationAccessToken.mock.calls.length).toBe(
+      1,
+    );
+  });
+
+  it('should expire access token cache when less than 10 mins before token expires', async () => {
+    octokit.apps.listInstallations.mockReturnValue({
+      headers: {
+        etag: '123',
+      },
+      data: [
+        {
+          id: 1,
+          repository_selection: 'all',
+          account: {
+            login: 'backstage',
+          },
+        },
+      ],
+    } as RestEndpointMethodTypes['apps']['listInstallations']['response']);
+
+    octokit.apps.createInstallationAccessToken.mockReturnValue({
+      data: {
+        expires_at: DateTime.local()
+          .plus({ minutes: 9, seconds: 59, milliseconds: 999 })
+          .toString(),
+        token: 'secret_token',
+      },
+    } as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']);
+
+    await github.getCredentials({ url: 'https://github.com/backstage' });
+    await github.getCredentials({ url: 'https://github.com/backstage' });
+
+    expect(octokit.apps.listInstallations.mock.calls.length).toBe(2);
+    expect(octokit.apps.createInstallationAccessToken.mock.calls.length).toBe(
+      2,
+    );
+  });
 });

packages/integration/src/github/SingleInstanceGithubCredentialsProvider.ts

@@ -30,29 +30,56 @@ type InstallationData = {
   suspended: boolean;
 };
 
+type InstallationTokenData = {
+  token: string;
+  expiresAt: DateTime;
+  repositories?: String[];
+};
+
 class Cache {
-  private readonly tokenCache = new Map<
-    string,
-    { token: string; expiresAt: DateTime }
-  >();
+  private readonly tokenCache = new Map<string, InstallationTokenData>();
 
   async getOrCreateToken(
-    key: string,
-    supplier: () => Promise<{ token: string; expiresAt: DateTime }>,
+    owner: string,
+    repo: string | undefined,
+    supplier: () => Promise<InstallationTokenData>,
   ): Promise<{ accessToken: string }> {
-    const item = this.tokenCache.get(key);
-    if (item && this.isNotExpired(item.expiresAt)) {
-      return { accessToken: item.token };
+    let existingInstallationData = this.tokenCache.get(owner);
+
+    if (
+      !existingInstallationData ||
+      this.isExpired(existingInstallationData.expiresAt)
+    ) {
+      existingInstallationData = await supplier();
+      // Allow 10 minutes grace to account for clock skew
+      existingInstallationData.expiresAt =
+        existingInstallationData.expiresAt.minus({ minutes: 10 });
+      this.tokenCache.set(owner, existingInstallationData);
     }
 
-    const result = await supplier();
-    this.tokenCache.set(key, result);
-    return { accessToken: result.token };
+    if (!this.appliesToRepo(existingInstallationData, repo)) {
+      throw new Error(
+        `The Backstage GitHub application used in the ${owner} organization does not have access to a repository with the name ${repo}`,
+      );
+    }
+
+    return { accessToken: existingInstallationData.token };
   }
 
-  // consider timestamps older than 50 minutes to be expired.
-  private isNotExpired = (date: DateTime) =>
-    date.diff(DateTime.local(), 'minutes').minutes > 50;
+  private isExpired = (date: DateTime) => DateTime.local() > date;
+
+  private appliesToRepo(tokenData: InstallationTokenData, repo?: string) {
+    // If no specific repo has been requested the token is applicable
+    if (repo === undefined) {
+      return true;
+    }
+    // If the token is restricted to repositories, the token only applies if the repo is in the allow list
+    if (tokenData.repositories !== undefined) {
+      return tokenData.repositories.includes(repo);
+    }
+    // Otherwise the token is applicable
+    return true;
+  }
 }
 
 /**
@@ -93,25 +120,29 @@ class GithubAppManager {
     owner: string,
     repo?: string,
   ): Promise<{ accessToken: string | undefined }> {
-    const { installationId, suspended } = await this.getInstallationData(owner);
     if (this.allowedInstallationOwners) {
       if (!this.allowedInstallationOwners?.includes(owner)) {
         return { accessToken: undefined }; // An empty token allows anonymous access to public repos
       }
     }
-    if (suspended) {
-      throw new Error(`The GitHub application for ${owner} is suspended`);
-    }
-
-    const cacheKey = repo ? `${owner}/${repo}` : owner;
 
     // Go and grab an access token for the app scoped to a repository if provided, if not use the organisation installation.
-    return this.cache.getOrCreateToken(cacheKey, async () => {
+    return this.cache.getOrCreateToken(owner, repo, async () => {
+      const { installationId, suspended } = await this.getInstallationData(
+        owner,
+      );
+      if (suspended) {
+        throw new Error(`The GitHub application for ${owner} is suspended`);
+      }
+
       const result = await this.appClient.apps.createInstallationAccessToken({
         installation_id: installationId,
         headers: HEADERS,
       });
-      if (repo && result.data.repository_selection === 'selected') {
+
+      let repositoryNames;
+
+      if (result.data.repository_selection === 'selected') {
         const installationClient = new Octokit({
           baseUrl: this.baseUrl,
           auth: result.data.token,
@@ -122,18 +153,13 @@ class GithubAppManager {
         // The return type of the paginate method is incorrect.
         const repositories: RestEndpointMethodTypes['apps']['listReposAccessibleToInstallation']['response']['data']['repositories'] =
           repos.repositories ?? repos;
-        const hasRepo = repositories.some(repository => {
-          return repository.name === repo;
-        });
-        if (!hasRepo) {
-          throw new Error(
-            `The Backstage GitHub application used in the ${owner} organization does not have access to a repository with the name ${repo}`,
-          );
-        }
+
+        repositoryNames = repositories.map(repository => repository.name);
       }
       return {
         token: result.data.token,
         expiresAt: DateTime.fromISO(result.data.expires_at),
+        repositories: repositoryNames,
       };
     });
   }