Merge pull request #7617 from lunarway/fix/github-refresh

Fix refresh token reuse bug in Github provider

.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.

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',
+        },
+      });
+    });
   });
 });

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),