diff --git a/binderhub/app.py b/binderhub/app.py index 4c9e8f4c2..5eb97aa9c 100644 --- a/binderhub/app.py +++ b/binderhub/app.py @@ -962,8 +962,7 @@ def initialize(self, *args, **kwargs): "enable_api_only_mode": self.enable_api_only_mode, } ) - if self.auth_enabled: - self.tornado_settings["cookie_secret"] = os.urandom(32) + self.tornado_settings["cookie_secret"] = os.urandom(32) if self.cors_allow_origin: self.tornado_settings.setdefault("headers", {})[ "Access-Control-Allow-Origin" diff --git a/binderhub/base.py b/binderhub/base.py index b002bbd95..5f198c401 100644 --- a/binderhub/base.py +++ b/binderhub/base.py @@ -137,11 +137,20 @@ def get_current_user(self): @property def template_namespace(self): - return dict( + + ns = dict( static_url=self.static_url, banner=self.settings["banner_message"], - **self.settings.get("template_variables", {}), + auth_enabled=self.settings["auth_enabled"], + ) + if self.settings["auth_enabled"]: + ns["xsrf"] = self.xsrf_token.decode("ascii") + ns["api_token"] = self.hub_auth.get_token(self) or "" + + ns.update( + self.settings.get("template_variables", {}), ) + return ns def set_default_headers(self): headers = self.settings.get("headers", {}) diff --git a/binderhub/builder.py b/binderhub/builder.py index ca1deafbe..c97704726 100644 --- a/binderhub/builder.py +++ b/binderhub/builder.py @@ -247,6 +247,10 @@ def _get_build_only(self): return build_only + def redirect(self, *args, **kwargs): + # disable redirect to login, which won't work for EventSource + raise HTTPError(403) + @authenticated async def get(self, provider_prefix, _unescaped_spec): """Get a built image for a given spec and repo provider. diff --git a/binderhub/static/js/index.js b/binderhub/static/js/index.js index 536a4e278..dbd9639ab 100644 --- a/binderhub/static/js/index.js +++ b/binderhub/static/js/index.js @@ -1,7 +1,6 @@ /* If this file gets over 200 lines of code long (not counting docs / comments), start using a framework */ import ClipboardJS from "clipboard"; -import "event-source-polyfill"; import { BinderRepository } from "@jupyterhub/binderhub-client"; import { updatePathText } from "./src/path"; @@ -61,12 +60,12 @@ async function build(providerSpec, log, fitAddon, path, pathType) { $(".on-build").removeClass("hidden"); const buildToken = $("#build-token").data("token"); + const apiToken = $("#api-token").data("token"); const buildEndpointUrl = new URL("build", BASE_URL); - const image = new BinderRepository( - providerSpec, - buildEndpointUrl, + const image = new BinderRepository(providerSpec, buildEndpointUrl, { + apiToken, buildToken, - ); + }); for await (const data of image.fetch()) { // Write message to the log terminal if there is a message diff --git a/binderhub/static/js/src/repo.js b/binderhub/static/js/src/repo.js index e848b49ed..17a3524b2 100644 --- a/binderhub/static/js/src/repo.js +++ b/binderhub/static/js/src/repo.js @@ -24,8 +24,16 @@ function setLabels() { */ export function updateRepoText(baseUrl) { if (Object.keys(configDict).length === 0) { + const xsrf = $("#xsrf-token").data("token"); + const apiToken = $("#api-token").data("token"); const configUrl = new URL("_config", baseUrl); - fetch(configUrl).then((resp) => { + const headers = {}; + if (apiToken && apiToken.length > 0) { + headers["Authorization"] = `Bearer ${apiToken}`; + } else if (xsrf && xsrf.length > 0) { + headers["X-Xsrftoken"] = xsrf; + } + fetch(configUrl, { headers }).then((resp) => { resp.json().then((data) => { configDict = data; setLabels(); diff --git a/binderhub/templates/index.html b/binderhub/templates/index.html index 6260c9c9d..cc55b89a7 100644 --- a/binderhub/templates/index.html +++ b/binderhub/templates/index.html @@ -3,6 +3,8 @@ {% block head %} + + {{ super() }} {% endblock head %} diff --git a/binderhub/templates/loading.html b/binderhub/templates/loading.html index dd6369ef2..4fe8af1b0 100644 --- a/binderhub/templates/loading.html +++ b/binderhub/templates/loading.html @@ -14,6 +14,8 @@ + + {{ super() }} diff --git a/js/packages/binderhub-client/lib/index.js b/js/packages/binderhub-client/lib/index.js index 9401eea7f..f33f35600 100644 --- a/js/packages/binderhub-client/lib/index.js +++ b/js/packages/binderhub-client/lib/index.js @@ -1,8 +1,30 @@ -import { NativeEventSource, EventSourcePolyfill } from "event-source-polyfill"; +import { fetchEventSource } from "@microsoft/fetch-event-source"; import { EventIterator } from "event-iterator"; -// Use native browser EventSource if available, and use the polyfill if not available -const EventSource = NativeEventSource || EventSourcePolyfill; +function _getXSRFToken() { + // from @jupyterlab/services + // https://github.com/jupyterlab/jupyterlab/blob/69223102d717f3d3e9f976d32e657a4e2456e85d/packages/services/src/contents/index.ts#L1178-L1184 + let cookie = ""; + try { + cookie = document.cookie; + } catch (e) { + // e.g. SecurityError in case of CSP Sandbox + return null; + } + // extracts the value of the cookie named `_xsrf` + // by picking up everything between `_xsrf=` and the next semicolon or end-of-line + // `\b` ensures word boundaries, so it doesn't pick up `something_xsrf=`... + const xsrfTokenMatch = cookie.match("\\b_xsrf=([^;]*)\\b"); + if (xsrfTokenMatch) { + return xsrfTokenMatch[1]; + } + return null; +} + +/* throw this to close the event stream */ +class EventStreamClose extends Error {} +/* throw this to close the event stream */ +class EventStreamRetry extends Error {} /** * Build (and optionally launch) a repository by talking to a BinderHub API endpoint @@ -12,10 +34,14 @@ export class BinderRepository { * * @param {string} providerSpec Spec of the form // to pass to the binderhub API. * @param {URL} buildEndpointUrl API URL of the build endpoint to talk to - * @param {string} [buildToken] Optional JWT based build token if this binderhub installation requires using build tokens - * @param {boolean} [buildOnly] Opt out of launching built image by default by passing `build_only` param + * @param {Object} [options] - optional arguments + * @param {string} [options.buildToken] Optional JWT based build token if this binderhub installation requires using build tokens + * @param {boolean} [options.buildOnly] Opt out of launching built image by default by passing `build_only` param + * @param {string} [options.apiToken] Optional Bearer token for authenticating requests */ - constructor(providerSpec, buildEndpointUrl, buildToken, buildOnly) { + constructor(providerSpec, buildEndpointUrl, options) { + const { apiToken, buildToken, buildOnly } = options || {}; + this.providerSpec = providerSpec; // Make sure that buildEndpointUrl is a real URL - this ensures hostname is properly set if (!(buildEndpointUrl instanceof URL)) { @@ -40,8 +66,10 @@ export class BinderRepository { if (buildOnly) { this.buildUrl.searchParams.append("build_only", "true"); } + this.apiToken = apiToken; this.eventIteratorQueue = null; + this.abortSignal = null; } /** @@ -67,26 +95,100 @@ export class BinderRepository { * @returns {AsyncIterable} An async iterator yielding responses from the API as they come in */ fetch() { - this.eventSource = new EventSource(this.buildUrl); + const headers = {}; + this.abortController = new AbortController(); + + if (this.apiToken && this.apiToken.length > 0) { + headers["Authorization"] = `Bearer ${this.apiToken}`; + } else { + const xsrf = _getXSRFToken(); + if (xsrf) { + headers["X-Xsrftoken"] = xsrf; + } + } + // setTimeout(() => this.close(), 1000); return new EventIterator((queue) => { this.eventIteratorQueue = queue; - this.eventSource.onerror = () => { - queue.push({ - phase: "failed", - message: "Failed to connect to event stream\n", - }); - queue.stop(); - }; - - this.eventSource.addEventListener("message", (event) => { - // console.log("message received") - // console.log(event) - const data = JSON.parse(event.data); - // FIXME: fix case of phase/state upstream - if (data.phase) { - data.phase = data.phase.toLowerCase(); - } - queue.push(data); + fetchEventSource(this.buildUrl, { + headers, + // signal used for closing + signal: this.abortController.signal, + // openWhenHidden leaves connection open (matches default) + // otherwise fetch-event closes connections, + // which would be nice if our javascript handled restarting messages better + openWhenHidden: true, + onopen: (response) => { + if (response.ok) { + return; // everything's good + } else if ( + response.status >= 400 && + response.status < 500 && + response.status !== 429 + ) { + queue.push({ + phase: "failed", + message: `Failed to connect to event stream: ${response.status} - ${response.text}\n`, + }); + throw new EventStreamClose(); + } else { + queue.push({ + phase: "unknown", + message: `Error connecting to event stream, retrying: ${response.status} - ${response.text}\n`, + }); + throw new EventStreamRetry(); + } + }, + + onclose: () => { + if (!queue.isStopped) { + // close called before queue finished + queue.push({ + phase: "failed", + message: `Event stream closed unexpectedly\n`, + }); + queue.stop(); + // throw new EventStreamClose(); + } + }, + onerror: (error) => { + console.log("Event stream error", error); + if (error.name === "EventStreamRetry") { + // if we don't re-raise, connection will be retried; + queue.push({ + phase: "unknown", + message: `Error in event stream: ${error}\n`, + }); + return; + } + if ( + !(error.name === "EventStreamClose" || error.name === "AbortError") + ) { + // errors _other_ than EventStreamClose get displayed + queue.push({ + phase: "failed", + message: `Error in event stream: ${error}\n`, + }); + } + queue.stop(); + // need to rethrow to prevent reconnection + throw error; + }, + + onmessage: (event) => { + if (!event.data || event.data === "") { + // onmessage is called for the empty lines + return; + } + const data = JSON.parse(event.data); + // FIXME: fix case of phase/state upstream + if (data.phase) { + data.phase = data.phase.toLowerCase(); + } + queue.push(data); + if (data.phase === "failed") { + throw new EventStreamClose(); + } + }, }); }); } @@ -95,12 +197,15 @@ export class BinderRepository { * Close the EventSource connection to the BinderHub API if it is open */ close() { - if (this.eventSource !== undefined) { - this.eventSource.close(); - } - if (this.eventIteratorQueue !== null) { + if (this.eventIteratorQueue) { // Stop any currently running fetch() iterations this.eventIteratorQueue.stop(); + this.eventIteratorQueue = null; + } + if (this.abortController) { + // close event source + this.abortController.abort(); + this.abortController = null; } } diff --git a/js/packages/binderhub-client/package.json b/js/packages/binderhub-client/package.json index b2ab42c3c..db3a2ec7c 100644 --- a/js/packages/binderhub-client/package.json +++ b/js/packages/binderhub-client/package.json @@ -14,7 +14,7 @@ }, "homepage": "https://github.com/jupyterhub/binderhub#readme", "dependencies": { - "event-source-polyfill": "^1.0.31", + "@microsoft/fetch-event-source": "^2.0.1", "event-iterator": "^2.0.0" } } diff --git a/js/packages/binderhub-client/tests/index.test.js b/js/packages/binderhub-client/tests/index.test.js index 0d35cdea0..04e35685b 100644 --- a/js/packages/binderhub-client/tests/index.test.js +++ b/js/packages/binderhub-client/tests/index.test.js @@ -1,3 +1,6 @@ +// fetch polyfill (only needed for node tests) +import { fetch, TextDecoder } from "@whatwg-node/fetch"; + import { BinderRepository, makeShareableBinderURL, @@ -6,9 +9,32 @@ import { import { parseEventSource, simpleEventSourceServer } from "./utils"; import { readFileSync } from "node:fs"; +async function wrapFetch(resource, options) { + /* like fetch, but ignore signal input + // abort signal shows up as uncaught in tests, despite working fine + */ + if (options) { + options.signal = null; + } + return fetch.apply(null, [resource, options]); +} + +beforeAll(() => { + // inject globals for fetch + global.TextDecoder = TextDecoder; + if (!global.window) { + global.window = {}; + } + if (!global.window.fetch) { + global.window.fetch = wrapFetch; + } +}); + test("Passed in URL object is not modified", () => { const buildEndpointUrl = new URL("https://test-binder.org/build"); - const br = new BinderRepository("gh/test/test", buildEndpointUrl, "token"); + const br = new BinderRepository("gh/test/test", buildEndpointUrl, { + buildToken: "token", + }); expect(br.buildEndpointUrl.toString()).not.toEqual( buildEndpointUrl.toString(), ); @@ -16,13 +42,15 @@ test("Passed in URL object is not modified", () => { test("Invalid URL errors out", () => { expect(() => { - new BinderRepository("gh/test/test", "/build", "token"); + new BinderRepository("gh/test/test", "/build", { buildToken: "token" }); }).toThrow(TypeError); }); test("Passing buildOnly flag works", () => { const buildEndpointUrl = new URL("https://test-binder.org/build"); - const br = new BinderRepository("gh/test/test", buildEndpointUrl, null, true); + const br = new BinderRepository("gh/test/test", buildEndpointUrl, { + buildOnly: true, + }); expect(br.buildUrl.toString()).toEqual( "https://test-binder.org/build/gh/test/test?build_only=true", ); @@ -46,7 +74,9 @@ test("Build URL correctly built from Build Endpoint", () => { test("Build URL correctly built from Build Endpoint when used with token", () => { const buildEndpointUrl = new URL("https://test-binder.org/build"); - const br = new BinderRepository("gh/test/test", buildEndpointUrl, "token"); + const br = new BinderRepository("gh/test/test", buildEndpointUrl, { + buildToken: "token", + }); expect(br.buildUrl.toString()).toEqual( "https://test-binder.org/build/gh/test/test?build_token=token", ); @@ -244,12 +274,16 @@ describe("Invalid eventsource response causes failure", () => { test("Invalid eventsource response should cause failure", async () => { const buildEndpointUrl = new URL(`${serverUrl}/build`); const br = new BinderRepository("gh/test/test", buildEndpointUrl); + let messages = []; for await (const item of br.fetch()) { - expect(item).toStrictEqual({ - phase: "failed", - message: "Failed to connect to event stream\n", - }); + messages.push(item); } + expect(messages).toStrictEqual([ + { + phase: "failed", + message: "Event stream closed unexpectedly\n", + }, + ]); }); }); diff --git a/package.json b/package.json index e08b94bbc..916d542cf 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,6 @@ "dependencies": { "bootstrap": "^3.4.1", "clipboard": "^2.0.11", - "event-source-polyfill": "^1.0.31", "jquery": "^3.6.4", "xterm": "^5.1.0", "xterm-addon-fit": "^0.7.0" @@ -15,6 +14,7 @@ "@babel/eslint-parser": "^7.22.15", "@babel/preset-env": "^7.21.4", "@types/jest": "^29.5.5", + "@whatwg-node/fetch": "^0.9.17", "babel-jest": "^29.7.0", "babel-loader": "^9.1.2", "css-loader": "^6.7.3",