auth-backend: Handle non-7-bit safe characters in the posted message data (#3280)

.changeset/modern-islands-joke.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend': patch
+---
+
+Handle non-7-bit safe characters in the posted message data

plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts

@@ -15,10 +15,20 @@
  */
 
 import express from 'express';
-import { ensuresXRequestedWith, postMessageResponse } from './authFlowHelpers';
+import {
+  safelyEncodeURIComponent,
+  ensuresXRequestedWith,
+  postMessageResponse,
+} from './authFlowHelpers';
 import { WebMessageResponse } from './types';
 
 describe('oauth helpers', () => {
+  describe('safelyEncodeURIComponent', () => {
+    it('encodes all occurrences of single quotes', () => {
+      expect(safelyEncodeURIComponent("a'ö'b")).toBe('a%27%C3%B6%27b');
+    });
+  });
+
   describe('postMessageResponse', () => {
     const appOrigin = 'http://localhost:3000';
     it('should post a message back with payload success', () => {
@@ -45,15 +55,12 @@ describe('oauth helpers', () => {
           },
         },
       };
-      const jsonData = JSON.stringify(data);
-      const base64Data = Buffer.from(jsonData, 'utf8').toString('base64');
+      const encoded = safelyEncodeURIComponent(JSON.stringify(data));
 
       postMessageResponse(mockResponse, appOrigin, data);
       expect(mockResponse.setHeader).toBeCalledTimes(3);
       expect(mockResponse.end).toBeCalledTimes(1);
-      expect(mockResponse.end).toBeCalledWith(
-        expect.stringContaining(base64Data),
-      );
+      expect(mockResponse.end).toBeCalledWith(expect.stringContaining(encoded));
     });
 
     it('should post a message back with payload error', () => {
@@ -64,16 +71,47 @@ describe('oauth helpers', () => {
 
       const data: WebMessageResponse = {
         type: 'authorization_response',
-        error: new Error('Unknown error occured'),
+        error: new Error('Unknown error occurred'),
+      };
+      const encoded = safelyEncodeURIComponent(JSON.stringify(data));
+
+      postMessageResponse(mockResponse, appOrigin, data);
+      expect(mockResponse.setHeader).toBeCalledTimes(3);
+      expect(mockResponse.end).toBeCalledTimes(1);
+      expect(mockResponse.end).toBeCalledWith(expect.stringContaining(encoded));
+    });
+
+    it('handles single quotes and unicode chars safely', () => {
+      const mockResponse = ({
+        end: jest.fn().mockReturnThis(),
+        setHeader: jest.fn().mockReturnThis(),
+      } as unknown) as express.Response;
+
+      const data: WebMessageResponse = {
+        type: 'authorization_response',
+        response: {
+          providerInfo: {
+            accessToken: 'ACCESS_TOKEN',
+            idToken: 'ID_TOKEN',
+            expiresInSeconds: 10,
+            scope: 'email',
+          },
+          profile: {
+            email: 'foo@bar.com',
+            displayName: "Adam l'Hôpital",
+          },
+          backstageIdentity: {
+            id: 'a',
+            idToken: 'a.b.c',
+          },
+        },
       };
-      const jsonData = JSON.stringify(data);
-      const base64Data = Buffer.from(jsonData, 'utf8').toString('base64');
 
       postMessageResponse(mockResponse, appOrigin, data);
       expect(mockResponse.setHeader).toBeCalledTimes(3);
       expect(mockResponse.end).toBeCalledTimes(1);
       expect(mockResponse.end).toBeCalledWith(
-        expect.stringContaining(base64Data),
+        expect.stringContaining('Adam%20l%27H%C3%B4pital'),
       );
     });
   });

plugins/auth-backend/src/lib/flow/authFlowHelpers.ts

@@ -18,37 +18,42 @@ import express from 'express';
 import crypto from 'crypto';
 import { WebMessageResponse } from './types';
 
+export const safelyEncodeURIComponent = (value: string) => {
+  // Note the g at the end of the regex; all occurrences of single quotes must
+  // be replaced, which encodeURIComponent does not do itself by default
+  return encodeURIComponent(value).replace(/'/g, '%27');
+};
+
 export const postMessageResponse = (
   res: express.Response,
   appOrigin: string,
   response: WebMessageResponse,
 ) => {
   const jsonData = JSON.stringify(response);
-  const base64Data = Buffer.from(jsonData, 'utf8').toString('base64');
+  const base64Data = safelyEncodeURIComponent(jsonData);
+  const base64Origin = safelyEncodeURIComponent(appOrigin);
 
-  res.setHeader('Content-Type', 'text/html');
-  res.setHeader('X-Frame-Options', 'sameorigin');
+  // NOTE: It is absolutely imperative that we use the safe encoder above, to
+  // be sure that the js code below does not allow the injection of malicious
+  // data.
 
   // TODO: Make target app origin configurable globally
   const script = `
-    (window.opener || window.parent).postMessage(JSON.parse(atob('${base64Data}')), '${appOrigin}')
-    window.close()
+    var json = decodeURIComponent('${base64Data}');
+    var origin = decodeURIComponent('${base64Origin}');
+    (window.opener || window.parent).postMessage(JSON.parse(json), origin);
+    window.close();
   `;
   const hash = crypto.createHash('sha256').update(script).digest('base64');
-  res.setHeader('Content-Security-Policy', `script-src 'sha256-${hash}'`);
 
-  res.end(`
-<html>
-<body>
-  <script>${script}</script>
-</body>
-</html>
-  `);
+  res.setHeader('Content-Type', 'text/html');
+  res.setHeader('X-Frame-Options', 'sameorigin');
+  res.setHeader('Content-Security-Policy', `script-src 'sha256-${hash}'`);
+  res.end(`<html><body><script>${script}</script></body></html>`);
 };
 
 export const ensuresXRequestedWith = (req: express.Request) => {
   const requiredHeader = req.header('X-Requested-With');
-
   if (!requiredHeader || requiredHeader !== 'XMLHttpRequest') {
     return false;
   }