Commit 7b38d746 authored by Robert Knight's avatar Robert Knight

Check annotator-sidebar version consistency and sidebar origin on startup

When deploying the new inter-frame communication code we saw errors [1]
setting up communication between the sidebar and host frames caused by:

 - Mismatches between the version of the client used in the annotator
   and the sidebar
 - The sidebar being served from an unexpected origin (eg. due to use of
   a web proxy) or an opaque origin (eg. due to the host being loaded in a
   sandboxed iframe).

In some cases these issues are beyond our control and we can't reasonably
guarantee that the client will work.

This commit adds checks for these issues when the sidebar launches. If the
checks fail client startup is allowed to continue, but we log a warning in the
browser console and disable Sentry error reporting to reduce noise.

 - Add `origin` parameter to `#config` fragment in sidebar and notebook
   app launch URLs, which contains the expected origin of the app

 - Check the `version` and `origin` parameters from the config fragment
   on app startup. If they don't match what is expected, log a warning
   and disable error reporting, but try and continue to minimize user
   inconvenience if it does work.

[1] See https://github.com/hypothesis/client/issues/3986#issuecomment-989685057
    for more details.
parent 6fd18cc4
...@@ -18,7 +18,7 @@ import { createAppConfig } from '../config/app'; ...@@ -18,7 +18,7 @@ import { createAppConfig } from '../config/app';
*/ */
function NotebookIframe({ config, groupId }) { function NotebookIframe({ config, groupId }) {
const notebookAppSrc = addConfigFragment(config.notebookAppUrl, { const notebookAppSrc = addConfigFragment(config.notebookAppUrl, {
...createAppConfig(config), ...createAppConfig(config.notebookAppUrl, config),
// Explicity set the "focused" group // Explicity set the "focused" group
group: groupId, group: groupId,
......
...@@ -28,7 +28,7 @@ describe('NotebookModal', () => { ...@@ -28,7 +28,7 @@ describe('NotebookModal', () => {
eventBus = new EventBus(); eventBus = new EventBus();
emitter = eventBus.createEmitter(); emitter = eventBus.createEmitter();
const fakeCreateAppConfig = sinon.spy(config => { const fakeCreateAppConfig = sinon.spy((appURL, config) => {
const appConfig = { ...config }; const appConfig = { ...config };
delete appConfig.notebookAppUrl; delete appConfig.notebookAppUrl;
return appConfig; return appConfig;
......
...@@ -2,10 +2,11 @@ ...@@ -2,10 +2,11 @@
* Create the JSON-serializable subset of annotator configuration that should * Create the JSON-serializable subset of annotator configuration that should
* be passed to the sidebar or notebook applications. * be passed to the sidebar or notebook applications.
* *
* @param {string} appURL - URL from which the application will be served
* @param {Record<string, any>} config * @param {Record<string, any>} config
* @return {object} * @return {object}
*/ */
export function createAppConfig(config) { export function createAppConfig(appURL, config) {
const appConfig = {}; const appConfig = {};
for (let [key, value] of Object.entries(config)) { for (let [key, value] of Object.entries(config)) {
...@@ -29,6 +30,10 @@ export function createAppConfig(config) { ...@@ -29,6 +30,10 @@ export function createAppConfig(config) {
appConfig[key] = value; appConfig[key] = value;
} }
// Pass the expected origin of the app. This is used to detect when it is
// served from a different location than expected, which may stop it working.
appConfig.origin = new URL(appURL).origin;
// Pass the version of the client, so we can check if it is the same as the // Pass the version of the client, so we can check if it is the same as the
// one used in the sidebar/notebook. // one used in the sidebar/notebook.
appConfig.version = '__VERSION__'; appConfig.version = '__VERSION__';
......
import { createAppConfig } from '../app'; import { createAppConfig } from '../app';
describe('createAppConfig', () => { describe('createAppConfig', () => {
const appURL = 'https://hypothes.is/app.html';
/** /**
* Filter configuration generated by `createAppConfig` to remove properties * Filter configuration generated by `createAppConfig` to remove properties
* which are always added regardless of the input. * which are always added regardless of the input.
...@@ -8,7 +10,7 @@ describe('createAppConfig', () => { ...@@ -8,7 +10,7 @@ describe('createAppConfig', () => {
function filterConfig(config) { function filterConfig(config) {
return Object.fromEntries( return Object.fromEntries(
Object.entries(config).filter( Object.entries(config).filter(
([key]) => key !== 'hostURL' && key !== 'version' ([key]) => key !== 'hostURL' && key !== 'origin' && key !== 'version'
) )
); );
} }
...@@ -19,7 +21,7 @@ describe('createAppConfig', () => { ...@@ -19,7 +21,7 @@ describe('createAppConfig', () => {
appType: 'via', appType: 'via',
}; };
const sidebarConfig = filterConfig(createAppConfig(config)); const sidebarConfig = filterConfig(createAppConfig(appURL, config));
assert.deepEqual(sidebarConfig, { assert.deepEqual(sidebarConfig, {
showHighlights: true, showHighlights: true,
...@@ -33,7 +35,7 @@ describe('createAppConfig', () => { ...@@ -33,7 +35,7 @@ describe('createAppConfig', () => {
notebookAppUrl: 'https://hypothes.is/notebook.html', notebookAppUrl: 'https://hypothes.is/notebook.html',
sidebarAppUrl: 'https://hypothes.is/app.html', sidebarAppUrl: 'https://hypothes.is/app.html',
}; };
const sidebarConfig = filterConfig(createAppConfig(config)); const sidebarConfig = filterConfig(createAppConfig(appURL, config));
assert.deepEqual(sidebarConfig, { assert.deepEqual(sidebarConfig, {
showHighlights: true, showHighlights: true,
}); });
...@@ -46,21 +48,26 @@ describe('createAppConfig', () => { ...@@ -46,21 +48,26 @@ describe('createAppConfig', () => {
notNullValue: 'test', notNullValue: 'test',
}; };
const appConfig = filterConfig(createAppConfig(config)); const appConfig = filterConfig(createAppConfig(appURL, config));
assert.deepEqual(appConfig, { notNullValue: 'test' }); assert.deepEqual(appConfig, { notNullValue: 'test' });
}); });
it('adds client version to configuration', () => { it('adds client version to configuration', () => {
const config = createAppConfig({}); const config = createAppConfig(appURL, {});
assert.equal(config.version, '1.0.0-dummy-version'); assert.equal(config.version, '1.0.0-dummy-version');
}); });
it('adds host URL to configuration', () => { it('adds host URL to configuration', () => {
const config = createAppConfig({}); const config = createAppConfig(appURL, {});
assert.equal(config.hostURL, window.location.href); assert.equal(config.hostURL, window.location.href);
}); });
it('adds expected app origin to configuration', () => {
const config = createAppConfig(appURL, {});
assert.equal(config.origin, 'https://hypothes.is');
});
it('sets boolean properties to indicate presence of callbacks', () => { it('sets boolean properties to indicate presence of callbacks', () => {
const callbacks = [ const callbacks = [
'onLoginRequest', 'onLoginRequest',
...@@ -77,7 +84,7 @@ describe('createAppConfig', () => { ...@@ -77,7 +84,7 @@ describe('createAppConfig', () => {
const config = { const config = {
services: [service], services: [service],
}; };
const sidebarConfig = createAppConfig(config); const sidebarConfig = createAppConfig(appURL, config);
const sidebarServiceConfig = sidebarConfig.services[0]; const sidebarServiceConfig = sidebarConfig.services[0];
for (let callback of callbacks) { for (let callback of callbacks) {
......
...@@ -34,7 +34,7 @@ export const MIN_RESIZE = 280; ...@@ -34,7 +34,7 @@ export const MIN_RESIZE = 280;
function createSidebarIframe(config) { function createSidebarIframe(config) {
const sidebarAppSrc = addConfigFragment( const sidebarAppSrc = addConfigFragment(
config.sidebarAppUrl, config.sidebarAppUrl,
createAppConfig(config) createAppConfig(config.sidebarAppUrl, config)
); );
const sidebarFrame = document.createElement('iframe'); const sidebarFrame = document.createElement('iframe');
......
...@@ -147,7 +147,7 @@ describe('Sidebar', () => { ...@@ -147,7 +147,7 @@ describe('Sidebar', () => {
fakeSendErrorsTo = sinon.stub(); fakeSendErrorsTo = sinon.stub();
const fakeCreateAppConfig = sinon.spy(config => { const fakeCreateAppConfig = sinon.spy((appURL, config) => {
const appConfig = { ...config }; const appConfig = { ...config };
delete appConfig.sidebarAppUrl; delete appConfig.sidebarAppUrl;
return appConfig; return appConfig;
......
import { parseConfigFragment } from '../../shared/config-fragment';
/**
* Perform checks for situations in which the Hypothesis client might not work
* properly.
*
* This function does not check for supported browser features. That is handled
* by the boot script.
*
* @param {Window} window
* @return {boolean} `true` if the checks passed
*/
export function checkEnvironment(window) {
const { version, origin } = parseConfigFragment(window.location.href);
// If the sidebar and annotator code are using different versions of the
// client, there might be a protocol mismatch in sidebar <-> guest/host RPC
// calls. This can happen if an old version of the client has been cached
// on the host/sidebar side. We try to set appropriate headers on the boot
// script to minimize the chances of this happening, but there are still
// situations beyond our control where it can happen.
if (version !== '__VERSION__') {
console.warn(
`The Hypothesis sidebar is using a different version (__VERSION__) than the host page (${version}). It may not work.`
);
return false;
}
// The client can't work if loaded in the wrong origin or an opaque origin
// because various cross-frame messaging (eg. login window, host <-> sidebar
// RPC) set an origin filter on `postMessage` calls.
if (window.origin === 'null') {
console.warn(
`Hypothesis has been loaded in a sandboxed frame. This is not supported.`
);
return false;
} else if (window.origin !== origin) {
console.warn(
`The Hypothesis sidebar is running in a different origin (${window.origin}) than expected (${origin}). It may not work.`
);
return false;
}
return true;
}
import { addConfigFragment } from '../../../shared/config-fragment';
import { checkEnvironment } from '../check-env';
describe('checkEnvironment', () => {
let fakeWindow;
beforeEach(() => {
fakeWindow = {
origin: 'https://hypothes.is',
location: {
href: 'https://hypothes.is/some-app',
},
};
fakeWindow.location.href = addConfigFragment(fakeWindow.location.href, {
origin: 'https://hypothes.is',
version: '1.0.0-dummy-version',
});
sinon.stub(console, 'warn');
});
afterEach(() => {
console.warn.restore();
});
it('returns true if all checks pass', () => {
assert.isTrue(checkEnvironment(fakeWindow));
assert.notCalled(console.warn);
});
it('returns false if annotator and sidebar versions do not match', () => {
fakeWindow.location.href = addConfigFragment(fakeWindow.location.href, {
origin: 'https://hypothes.is',
version: '1.0.0-other-version',
});
assert.isFalse(checkEnvironment(fakeWindow));
assert.called(console.warn);
});
it('returns false if the app is loaded in a sandboxed frame', () => {
fakeWindow.origin = 'null';
assert.isFalse(checkEnvironment(fakeWindow));
assert.called(console.warn);
});
it('returns false if the app is loaded in a different origin than expected', () => {
fakeWindow.origin = 'https://hypothes.is.some-proxy.edu';
assert.isFalse(checkEnvironment(fakeWindow));
assert.called(console.warn);
});
});
import { parseJsonConfig } from '../boot/parse-json-config'; import { parseJsonConfig } from '../boot/parse-json-config';
import * as rendererOptions from '../shared/renderer-options'; import * as rendererOptions from '../shared/renderer-options';
import { checkEnvironment } from './config/check-env';
import { fetchConfig } from './config/fetch-config';
import { import {
startServer as startRPCServer, startServer as startRPCServer,
preStartServer as preStartRPCServer, preStartServer as preStartRPCServer,
} from './cross-origin-rpc.js'; } from './cross-origin-rpc.js';
import disableOpenerForExternalLinks from './util/disable-opener-for-external-links'; import disableOpenerForExternalLinks from './util/disable-opener-for-external-links';
import { fetchConfig } from './config/fetch-config';
import * as sentry from './util/sentry'; import * as sentry from './util/sentry';
// Read settings rendered into sidebar app HTML by service/extension. // Read settings rendered into sidebar app HTML by service/extension.
...@@ -14,7 +15,13 @@ const appConfig = /** @type {import('../types/config').SidebarConfig} */ ( ...@@ -14,7 +15,13 @@ const appConfig = /** @type {import('../types/config').SidebarConfig} */ (
parseJsonConfig(document) parseJsonConfig(document)
); );
if (appConfig.sentry) { // Check for known issues which may prevent the client from working.
//
// If any checks fail we'll log warnings and disable error reporting, but try
// and continue anyway.
const envOk = checkEnvironment(window);
if (appConfig.sentry && envOk) {
// Initialize Sentry. This is required at the top of this file // Initialize Sentry. This is required at the top of this file
// so that it happens early in the app's startup flow // so that it happens early in the app's startup flow
sentry.init(appConfig.sentry); sentry.init(appConfig.sentry);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment