Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle api tokens and xsrf #1847

Merged
merged 12 commits into from
May 14, 2024
3 changes: 1 addition & 2 deletions binderhub/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 11 additions & 2 deletions binderhub/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", {})
Expand Down
4 changes: 4 additions & 0 deletions binderhub/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 4 additions & 5 deletions binderhub/static/js/index.js
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion binderhub/static/js/src/repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions binderhub/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
{% block head %}
<meta id="base-url" data-url="{{base_url}}">
<meta id="badge-base-url" data-url="{{badge_base_url}}">
<meta id="api-token" data-token="{{ api_token }}">
<meta id="xsrf-token" data-token="{{ xsrf }}">
<script src="{{static_url("dist/bundle.js")}}"></script>
{{ super() }}
{% endblock head %}
Expand Down
2 changes: 2 additions & 0 deletions binderhub/templates/loading.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
<meta id="base-url" data-url="{{base_url}}">
<meta id="badge-base-url" data-url="{{badge_base_url}}">
<meta id="build-token" data-token="{{ build_token }}">
<meta id="api-token" data-token="{{ api_token }}">
<meta id="xsrf-token" data-token="{{ xsrf }}">
{{ super() }}
<script src="{{static_url("dist/bundle.js")}}"></script>
<link href="{{static_url("loading.css")}}" rel="stylesheet">
Expand Down
161 changes: 133 additions & 28 deletions js/packages/binderhub-client/lib/index.js
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regexes in anything security sensitive always make me nervous haha. Can you either add a comment here explaining what's going on, or a direct link to the specific place from @jupyterlab/services this was stolen from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added link and explanation. It's still astonishing to me that there is no API for geting cookies by name in browsers, but it is {name}={anything but a semicolon}[; again]

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
Expand All @@ -12,10 +34,14 @@ export class BinderRepository {
*
* @param {string} providerSpec Spec of the form <provider>/<repo>/<ref> 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)) {
Expand All @@ -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;
}

/**
Expand All @@ -67,26 +95,100 @@ export class BinderRepository {
* @returns {AsyncIterable<Line>} 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();
}
},
});
});
}
Expand All @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion js/packages/binderhub-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Loading
Loading