diff --git a/.changeset/modern-islands-joke.md b/.changeset/modern-islands-joke.md new file mode 100644 index 0000000000..5737c4f762 --- /dev/null +++ b/.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 diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts index f42e4c4270..6c49eaf351 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts +++ b/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'), ); }); }); diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts index 63d7c28b50..d2a9557a28 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts +++ b/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(` - - - - - - `); + res.setHeader('Content-Type', 'text/html'); + res.setHeader('X-Frame-Options', 'sameorigin'); + res.setHeader('Content-Security-Policy', `script-src 'sha256-${hash}'`); + res.end(``); }; export const ensuresXRequestedWith = (req: express.Request) => { const requiredHeader = req.header('X-Requested-With'); - if (!requiredHeader || requiredHeader !== 'XMLHttpRequest') { return false; }