From de3e26aeccd11a15333d5ead4a398904fbefb554 Mon Sep 17 00:00:00 2001
From: Crevil <bjoern.soerensen@gmail.com>
Date: Thu, 14 Oct 2021 21:26:43 +0200
Subject: [PATCH] Fix refresh reuse bug in Github provider

Currently when the Github provider refreshes an access token it doesn't forward
the new refresh token to the caller. As Github only allows refresh tokens to be
used once, the next refresh call will fail.

This change fixes the bug by returning the new refresh token upon refresh.

This change is similar to 25a613bdd775b984a236944d6d97ff30b65ef4d8 (#7110) for
the GitLab provider.

Signed-off-by: Crevil <bjoern.soerensen@gmail.com>
---
 .changeset/brown-paws-marry.md                |  5 ++
 .../src/providers/github/provider.test.ts     | 54 +++++++++++++++++++
 .../src/providers/github/provider.ts          |  9 +++-
 3 files changed, 66 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/brown-paws-marry.md

diff --git a/.changeset/brown-paws-marry.md b/.changeset/brown-paws-marry.md
new file mode 100644
index 0000000000..c771d5e1f4
--- /dev/null
+++ b/.changeset/brown-paws-marry.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend': patch
+---
+
+Fix a bug preventing an access token to be refreshed a second time with the GitHub provider.
diff --git a/plugins/auth-backend/src/providers/github/provider.test.ts b/plugins/auth-backend/src/providers/github/provider.test.ts
index f67c41b461..d224e8d723 100644
--- a/plugins/auth-backend/src/providers/github/provider.test.ts
+++ b/plugins/auth-backend/src/providers/github/provider.test.ts
@@ -278,5 +278,59 @@ describe('GithubAuthProvider', () => {
         refreshToken: 'refresh-me',
       });
     });
+
+    it('should forward a new refresh token on refresh', async () => {
+      const mockRefreshToken = jest.spyOn(
+        helpers,
+        'executeRefreshTokenStrategy',
+      ) as unknown as jest.MockedFunction<() => Promise<{}>>;
+
+      mockRefreshToken.mockResolvedValueOnce({
+        accessToken: 'a.b.c',
+        refreshToken: 'dont-forget-to-send-refresh',
+        params: {
+          id_token: 'my-id',
+          expires_in: '123',
+          scope: 'read_user',
+        },
+      });
+
+      const mockUserProfile = jest.spyOn(
+        helpers,
+        'executeFetchUserProfileStrategy',
+      ) as unknown as jest.MockedFunction<() => Promise<PassportProfile>>;
+
+      mockUserProfile.mockResolvedValueOnce({
+        id: 'mockid',
+        username: 'mockuser',
+        provider: 'github',
+        displayName: 'Mocked User',
+        emails: [
+          {
+            value: 'mockuser@gmail.com',
+          },
+        ],
+      });
+
+      const response = await provider.refresh({} as any);
+
+      expect(response).toEqual({
+        backstageIdentity: {
+          id: 'mockuser',
+          token: 'token-for-mockuser',
+        },
+        profile: {
+          displayName: 'Mocked User',
+          email: 'mockuser@gmail.com',
+          picture: undefined,
+        },
+        providerInfo: {
+          accessToken: 'a.b.c',
+          refreshToken: 'dont-forget-to-send-refresh',
+          expiresInSeconds: 123,
+          scope: 'read_user',
+        },
+      });
+    });
   });
 });
diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts
index f1949eafa3..82d231edfa 100644
--- a/plugins/auth-backend/src/providers/github/provider.ts
+++ b/plugins/auth-backend/src/providers/github/provider.ts
@@ -126,7 +126,11 @@ export class GithubAuthProvider implements OAuthHandlers {
   }
 
   async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
-    const { accessToken, params } = await executeRefreshTokenStrategy(
+    const {
+      accessToken,
+      refreshToken: newRefreshToken,
+      params,
+    } = await executeRefreshTokenStrategy(
       this._strategy,
       req.refreshToken,
       req.scope,
@@ -139,7 +143,7 @@ export class GithubAuthProvider implements OAuthHandlers {
       fullProfile,
       params,
       accessToken,
-      refreshToken: req.refreshToken,
+      refreshToken: newRefreshToken,
     });
   }
 
@@ -150,6 +154,7 @@ export class GithubAuthProvider implements OAuthHandlers {
     const response: OAuthResponse = {
       providerInfo: {
         accessToken: result.accessToken,
+        refreshToken: result.refreshToken, // GitHub expires the old refresh token when used
         scope: result.params.scope,
         expiresInSeconds:
           expiresInStr === undefined ? undefined : Number(expiresInStr),