diff --git a/packages/core-api/src/lib/loginPopup.test.ts b/packages/core-api/src/lib/loginPopup.test.ts index 55f1408e8d..6527618ef7 100644 --- a/packages/core-api/src/lib/loginPopup.test.ts +++ b/packages/core-api/src/lib/loginPopup.test.ts @@ -156,6 +156,15 @@ describe('showLoginPopup', () => { expect(addEventListenerSpy).toBeCalledTimes(1); expect(removeEventListenerSpy).toBeCalledTimes(0); + const listener = addEventListenerSpy.mock.calls[0][1] as EventListener; + listener({ + source: popupMock, + origin: 'origin', + data: { + targetOrigin: 'http://localhost', + }, + } as MessageEvent); + setTimeout(() => { popupMock.closed = true; }, 150); @@ -167,4 +176,41 @@ describe('showLoginPopup', () => { expect(addEventListenerSpy).toBeCalledTimes(1); expect(removeEventListenerSpy).toBeCalledTimes(1); }); + + it('should indicate if origin does not match', async () => { + const openSpy = jest + .spyOn(window, 'open') + .mockReturnValue({ closed: false } as Window); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener'); + const popupMock = { closed: false }; + + openSpy.mockReturnValue(popupMock as Window); + + const payloadPromise = showLoginPopup({ + url: 'url', + name: 'name', + origin: 'origin', + }); + + const listener = addEventListenerSpy.mock.calls[0][1] as EventListener; + listener({ + source: popupMock, + origin: 'origin', + data: { + targetOrigin: 'http://differenthost', + }, + } as MessageEvent); + + setTimeout(() => { + popupMock.closed = true; + }, 150); + await expect(payloadPromise).rejects.toThrow( + 'Login failed, incorrect app origin', + ); + + expect(openSpy).toBeCalledTimes(1); + expect(addEventListenerSpy).toBeCalledTimes(1); + expect(removeEventListenerSpy).toBeCalledTimes(1); + }); }); diff --git a/packages/core-api/src/lib/loginPopup.ts b/packages/core-api/src/lib/loginPopup.ts index 10d6efd7a4..5c6d424363 100644 --- a/packages/core-api/src/lib/loginPopup.ts +++ b/packages/core-api/src/lib/loginPopup.ts @@ -79,6 +79,8 @@ export function showLoginPopup(options: LoginPopupOptions): Promise { `menubar=no,location=no,resizable=no,scrollbars=no,status=no,width=${width},height=${height},top=${top},left=${left}`, ); + let targetOrigin = ''; + if (!popup || typeof popup.closed === 'undefined' || popup.closed) { reject(new Error('Failed to open auth popup.')); return; @@ -92,6 +94,12 @@ export function showLoginPopup(options: LoginPopupOptions): Promise { return; } const { data } = event; + + if (data.targetOrigin) { + targetOrigin = data.targetOrigin; + return; + } + if (data.type !== 'authorization_response') { return; } @@ -111,7 +119,12 @@ export function showLoginPopup(options: LoginPopupOptions): Promise { const intervalId = setInterval(() => { if (popup.closed) { - const error = new Error('Login failed, popup was closed'); + const errMessage = `Login failed, ${ + targetOrigin !== window.location.origin + ? 'incorrect app origin' + : 'popup was closed' + }`; + const error = new Error(errMessage); error.name = 'PopupClosedError'; reject(error); done(); diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts index 6c49eaf351..5e4e5f875b 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts @@ -81,6 +81,52 @@ describe('oauth helpers', () => { expect(mockResponse.end).toBeCalledWith(expect.stringContaining(encoded)); }); + it('should call postMessage twice but only one of them with target *', () => { + let responseBody: String; + + const mockResponse = ({ + end: jest.fn(function (body) { + responseBody = body; + return this; + }), + 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', + }, + backstageIdentity: { + id: 'a', + idToken: 'a.b.c', + }, + }, + }; + postMessageResponse(mockResponse, appOrigin, data); + expect(responseBody.match(/.postMessage\(/g)).toHaveLength(2); + expect( + responseBody.match(/.postMessage\([a-zA-z.()]*, \'\*\'\)/g), + ).toHaveLength(1); + + const errData: WebMessageResponse = { + type: 'authorization_response', + error: new Error('Unknown error occurred'), + }; + postMessageResponse(mockResponse, appOrigin, errData); + expect(responseBody.match(/.postMessage\(/g)).toHaveLength(2); + expect( + responseBody.match(/.postMessage\([a-zA-z.()]*, \'\*\'\)/g), + ).toHaveLength(1); + }); + it('handles single quotes and unicode chars safely', () => { const mockResponse = ({ end: jest.fn().mockReturnThis(), diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts index d2a9557a28..5aba427905 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.ts @@ -38,10 +38,24 @@ export const postMessageResponse = ( // data. // TODO: Make target app origin configurable globally + + // + // postMessage fails silently if the targetOrigin is disallowed. + // So 2 postMessages are sent from the popup to the parent window. + // First, the origin being used to post the actual authorization response is + // shared with the parent window with a postMessage with targetOrigin '*'. + // Second, the actual authorization response is sent with the app origin + // as the targetOrigin. + // If the first message was received but the actual auth response was + // never received, the event listener can conclude that targetOrigin + // was disallowed, indicating potential misconfiguration. + // const script = ` - var json = decodeURIComponent('${base64Data}'); + var authResponse = decodeURIComponent('${base64Data}'); var origin = decodeURIComponent('${base64Origin}'); - (window.opener || window.parent).postMessage(JSON.parse(json), origin); + var originInfo = {'targetOrigin': origin}; + (window.opener || window.parent).postMessage(originInfo, '*'); + (window.opener || window.parent).postMessage(JSON.parse(authResponse), origin); window.close(); `; const hash = crypto.createHash('sha256').update(script).digest('base64');