auth-backend: always exchange and never return refresh tokens to clients

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/four-phones-shave.md

@@ -0,0 +1,29 @@
+---
+'@backstage/plugin-auth-backend': minor
+---
+
+Avoid ever returning OAuth refresh tokens back to the client, and always exchange refresh tokens for a new one when available for all providers.
+
+This comes with a breaking change to the TypeScript API for custom auth providers. The `refresh` method of `OAuthHandlers` implementation must now return a `{ response, refreshToken }` object rather than a direct response. Existing `refresh` implementations are typically migrated by changing an existing return expression that looks like this:
+
+```ts
+return await this.handleResult({
+  fullProfile,
+  params,
+  accessToken,
+  refreshToken,
+});
+```
+
+Into the following:
+
+```ts
+return {
+  response: await this.handleResult({
+    fullProfile,
+    params,
+    accessToken,
+  }),
+  refreshToken,
+};
+```

plugins/auth-backend/api-report.md

@@ -27,10 +27,13 @@ export class AtlassianAuthProvider implements OAuthHandlers {
   // (undocumented)
   handler(req: express.Request): Promise<{
     response: OAuthResponse;
-    refreshToken: string;
+    refreshToken: string | undefined;
   }>;
   // (undocumented)
-  refresh(req: OAuthRefreshRequest): Promise<OAuthResponse>;
+  refresh(req: OAuthRefreshRequest): Promise<{
+    response: OAuthResponse;
+    refreshToken: string | undefined;
+  }>;
   // Warning: (ae-forgotten-export) The symbol "RedirectInfo" needs to be exported by the entry point index.d.ts
   //
   // (undocumented)
@@ -488,7 +491,10 @@ export interface OAuthHandlers {
   // Warning: (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
   // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
   // Warning: (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
-  refresh?(req: OAuthRefreshRequest): Promise<OAuthResponse>;
+  refresh?(req: OAuthRefreshRequest): Promise<{
+    response: OAuthResponse;
+    refreshToken?: string;
+  }>;
   // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
   // Warning: (tsdoc-param-tag-with-invalid-type) The @param block should not include a JSDoc-style '{type}'
   // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
@@ -503,7 +509,6 @@ export type OAuthProviderInfo = {
   idToken?: string;
   expiresInSeconds?: number;
   scope: string;
-  refreshToken?: string;
 };
 
 // Warning: (ae-missing-release-tag) "OAuthProviderOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
@@ -703,11 +708,11 @@ export type WebMessageResponse =
 //
 // src/identity/types.d.ts:31:9 - (ae-forgotten-export) The symbol "AnyJWK" needs to be exported by the entry point index.d.ts
 // src/providers/aws-alb/provider.d.ts:77:5 - (ae-forgotten-export) The symbol "AwsAlbResult" needs to be exported by the entry point index.d.ts
-// src/providers/github/provider.d.ts:71:58 - (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag
-// src/providers/github/provider.d.ts:71:90 - (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag
-// src/providers/github/provider.d.ts:71:89 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
-// src/providers/github/provider.d.ts:71:67 - (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name
-// src/providers/github/provider.d.ts:71:68 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
-// src/providers/github/provider.d.ts:78:5 - (ae-forgotten-export) The symbol "StateEncoder" needs to be exported by the entry point index.d.ts
+// src/providers/github/provider.d.ts:74:58 - (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag
+// src/providers/github/provider.d.ts:74:90 - (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag
+// src/providers/github/provider.d.ts:74:89 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag
+// src/providers/github/provider.d.ts:74:67 - (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name
+// src/providers/github/provider.d.ts:74:68 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@"
+// src/providers/github/provider.d.ts:81:5 - (ae-forgotten-export) The symbol "StateEncoder" needs to be exported by the entry point index.d.ts
 // src/providers/types.d.ts:100:5 - (ae-forgotten-export) The symbol "AuthProviderConfig" needs to be exported by the entry point index.d.ts
 ```

plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts

@@ -57,7 +57,10 @@ describe('OAuthAdapter', () => {
       };
     }
     async refresh() {
-      return mockResponseData;
+      return {
+        response: mockResponseData,
+        refreshToken: 'token',
+      };
     }
   }
   const providerInstance = new MyAuthProvider();
@@ -257,7 +260,10 @@ describe('OAuthAdapter', () => {
   });
 
   it('correctly populates incomplete identities', async () => {
-    const mockRefresh = jest.fn<Promise<OAuthResponse>, [express.Request]>();
+    const mockRefresh = jest.fn<
+      Promise<{ response: OAuthResponse }>,
+      [express.Request]
+    >();
 
     const oauthProvider = new OAuthAdapter(
       {
@@ -291,10 +297,12 @@ describe('OAuthAdapter', () => {
 
     // Without a token
     mockRefresh.mockResolvedValueOnce({
-      ...mockResponseData,
-      backstageIdentity: {
-        id: 'foo',
-        token: '',
+      response: {
+        ...mockResponseData,
+        backstageIdentity: {
+          id: 'foo',
+          token: '',
+        },
       },
     });
     await oauthProvider.refresh(mockRequest, mockResponse);
@@ -315,10 +323,12 @@ describe('OAuthAdapter', () => {
 
     // With a token
     mockRefresh.mockResolvedValueOnce({
-      ...mockResponseData,
-      backstageIdentity: {
-        id: 'foo',
-        token: `z.${mkTokenBody({ sub: 'user:my-ns/foo' })}.z`,
+      response: {
+        ...mockResponseData,
+        backstageIdentity: {
+          id: 'foo',
+          token: `z.${mkTokenBody({ sub: 'user:my-ns/foo' })}.z`,
+        },
       },
     });
     await oauthProvider.refresh(mockRequest, mockResponse);

plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts

@@ -212,19 +212,15 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
       const forwardReq = Object.assign(req, { scope, refreshToken });
 
       // get new access_token
-      const response = await this.handlers.refresh(
-        forwardReq as OAuthRefreshRequest,
-      );
+      const { response, refreshToken: newRefreshToken } =
+        await this.handlers.refresh(forwardReq as OAuthRefreshRequest);
 
       const backstageIdentity = await this.populateIdentity(
         response.backstageIdentity,
       );
 
-      if (
-        response.providerInfo.refreshToken &&
-        response.providerInfo.refreshToken !== refreshToken
-      ) {
-        this.setRefreshTokenCookie(res, response.providerInfo.refreshToken);
+      if (newRefreshToken && newRefreshToken !== refreshToken) {
+        this.setRefreshTokenCookie(res, newRefreshToken);
       }
 
       res.status(200).json({ ...response, backstageIdentity });

plugins/auth-backend/src/lib/oauth/types.ts

@@ -79,10 +79,6 @@ export type OAuthProviderInfo = {
    * Scopes granted for the access token.
    */
   scope: string;
-  /**
-   * A refresh token issued for the signed in user
-   */
-  refreshToken?: string;
 };
 
 export type OAuthState = {
@@ -130,7 +126,10 @@ export interface OAuthHandlers {
    * @param {string} refreshToken
    * @param {string} scope
    */
-  refresh?(req: OAuthRefreshRequest): Promise<OAuthResponse>;
+  refresh?(req: OAuthRefreshRequest): Promise<{
+    response: OAuthResponse;
+    refreshToken?: string;
+  }>;
 
   /**
    * (Optional) Sign out of the auth provider.

plugins/auth-backend/src/providers/atlassian/provider.test.ts

@@ -78,20 +78,22 @@ describe('createAtlassianProvider', () => {
         refreshToken: 'wacka',
       },
     });
-    const { response } = await provider.handler({} as any);
-    expect(response).toEqual({
-      providerInfo: {
-        accessToken: 'accessToken',
-        expiresInSeconds: 123,
-        idToken: 'idToken',
-        scope: 'scope',
-        refreshToken: 'wacka',
-      },
-      profile: {
-        email: 'conrad@example.com',
-        displayName: 'Conrad',
-        picture: 'http://google.com/lols',
+    const result = await provider.handler({} as any);
+    expect(result).toEqual({
+      response: {
+        providerInfo: {
+          accessToken: 'accessToken',
+          expiresInSeconds: 123,
+          idToken: 'idToken',
+          scope: 'scope',
+        },
+        profile: {
+          email: 'conrad@example.com',
+          displayName: 'Conrad',
+          picture: 'http://google.com/lols',
+        },
       },
+      refreshToken: 'wacka',
     });
   });
 
@@ -127,20 +129,22 @@ describe('createAtlassianProvider', () => {
       ],
     });
 
-    const response = await provider.refresh({} as any);
+    const result = await provider.refresh({} as any);
 
-    expect(response).toEqual({
-      profile: {
-        displayName: 'Mocked User',
-        email: 'mockuser@gmail.com',
-        picture: 'http://google.com/lols',
-      },
-      providerInfo: {
-        accessToken: 'a.b.c',
-        idToken: 'my-id',
-        refreshToken: 'dont-forget-to-send-refresh',
-        scope: 'read_user',
+    expect(result).toEqual({
+      response: {
+        profile: {
+          displayName: 'Mocked User',
+          email: 'mockuser@gmail.com',
+          picture: 'http://google.com/lols',
+        },
+        providerInfo: {
+          accessToken: 'a.b.c',
+          idToken: 'my-id',
+          scope: 'read_user',
+        },
       },
+      refreshToken: 'dont-forget-to-send-refresh',
     });
   });
 });

plugins/auth-backend/src/providers/atlassian/provider.ts

@@ -107,17 +107,15 @@ export class AtlassianAuthProvider implements OAuthHandlers {
     });
   }
 
-  async handler(
-    req: express.Request,
-  ): Promise<{ response: OAuthResponse; refreshToken: string }> {
+  async handler(req: express.Request) {
     const { result } = await executeFrameHandlerStrategy<OAuthResult>(
       req,
       this._strategy,
     );
 
     return {
       response: await this.handleResult(result),
-      refreshToken: result.refreshToken ?? '',
+      refreshToken: result.refreshToken,
     };
   }
 
@@ -128,7 +126,6 @@ export class AtlassianAuthProvider implements OAuthHandlers {
       providerInfo: {
         idToken: result.params.id_token,
         accessToken: result.accessToken,
-        refreshToken: result.refreshToken,
         scope: result.params.scope,
         expiresInSeconds: result.params.expires_in,
       },
@@ -152,28 +149,27 @@ export class AtlassianAuthProvider implements OAuthHandlers {
     return response;
   }
 
-  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
-    const {
-      accessToken,
-      params,
-      refreshToken: newRefreshToken,
-    } = await executeRefreshTokenStrategy(
-      this._strategy,
-      req.refreshToken,
-      req.scope,
-    );
+  async refresh(req: OAuthRefreshRequest) {
+    const { accessToken, params, refreshToken } =
+      await executeRefreshTokenStrategy(
+        this._strategy,
+        req.refreshToken,
+        req.scope,
+      );
 
     const fullProfile = await executeFetchUserProfileStrategy(
       this._strategy,
       accessToken,
     );
 
-    return this.handleResult({
-      fullProfile,
-      params,
-      accessToken,
-      refreshToken: newRefreshToken,
-    });
+    return {
+      response: await this.handleResult({
+        fullProfile,
+        params,
+        accessToken,
+      }),
+      refreshToken,
+    };
   }
 }
 

plugins/auth-backend/src/providers/auth0/provider.ts

@@ -113,9 +113,7 @@ export class Auth0AuthProvider implements OAuthHandlers {
     });
   }
 
-  async handler(
-    req: express.Request,
-  ): Promise<{ response: OAuthResponse; refreshToken: string }> {
+  async handler(req: express.Request) {
     const { result, privateInfo } = await executeFrameHandlerStrategy<
       OAuthResult,
       PrivateInfo
@@ -127,24 +125,27 @@ export class Auth0AuthProvider implements OAuthHandlers {
     };
   }
 
-  async refresh(req: OAuthRefreshRequest): Promise<OAuthResponse> {
-    const { accessToken, params } = await executeRefreshTokenStrategy(
-      this._strategy,
-      req.refreshToken,
-      req.scope,
-    );
+  async refresh(req: OAuthRefreshRequest) {
+    const { accessToken, refreshToken, params } =
+      await executeRefreshTokenStrategy(
+        this._strategy,
+        req.refreshToken,
+        req.scope,
+      );
 
     const fullProfile = await executeFetchUserProfileStrategy(
       this._strategy,
       accessToken,
     );
 
-    return this.handleResult({
-      fullProfile,
-      params,
-      accessToken,
-      refreshToken: req.refreshToken,
-    });
+    return {
+      response: await this.handleResult({
+        fullProfile,
+        params,
+        accessToken,
+      }),
+      refreshToken,
+    };
   }
 
   private async handleResult(result: OAuthResult) {