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

[WIP] Add extra check for dashboards behind auth #229

Merged
merged 18 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions schema/plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
"title": "Hide Cluster Manager",
"description": "Some deployments don't want to or are unable to use the cluster manager feature. Toggle to hide it from the user interface (note: this does not disable the underlying functionality).",
"default": false
},
"browserDashboardCheck": {
"type": "boolean",
"title": "Determine whether to validate dashboards via browser check.",
"description": "If set to true, the test dashboard will be validated within the browser environment. This is useful for testing the dashboard when behind a browser-cookie based authentication.",
"default": false
viniciusdc marked this conversation as resolved.
Show resolved Hide resolved
}
},
"type": "object"
Expand Down
65 changes: 52 additions & 13 deletions src/dashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -266,19 +266,21 @@ export class URLInput extends Widget {
if (newValue === oldValue.url) {
return;
}
void Private.testDaskDashboard(newValue, this._serverSettings).then(
result => {
this._urlInfo = result;
this._urlChanged.emit({ oldValue, newValue: result });
this._input.blur();
this.update();
if (!result) {
console.warn(
`${newValue} does not appear to host a valid Dask dashboard`
);
}
void Private.testDaskDashboard(
newValue,
this._serverSettings,
this._browserDashboardCheck
).then(result => {
this._urlInfo = result;
this._urlChanged.emit({ oldValue, newValue: result });
this._input.blur();
this.update();
if (!result) {
console.warn(
`${newValue} does not appear to host a valid Dask dashboard`
);
}
);
});
}

/**
Expand All @@ -296,6 +298,16 @@ export class URLInput extends Widget {
return this._urlChanged;
}

/**
* The in browser dashboard check for authenticated dashboards.
*/
get browserDashboardCheck(): boolean {
return this._browserDashboardCheck;
}
set browserDashboardCheck(value: boolean) {
viniciusdc marked this conversation as resolved.
Show resolved Hide resolved
this._browserDashboardCheck = value;
}

/**
* Dispose of the resources held by the dashboard.
*/
Expand Down Expand Up @@ -393,6 +405,7 @@ export class URLInput extends Widget {
private _urlInfo: DashboardURLInfo = { isActive: false, url: '', plots: {} };
private _input: HTMLInputElement;
private _poll: Poll;
private _browserDashboardCheck: boolean = false;
private _serverSettings: ServerConnection.ISettings;
}

Expand Down Expand Up @@ -572,7 +585,8 @@ namespace Private {
*/
export async function testDaskDashboard(
url: string,
settings: ServerConnection.ISettings
settings: ServerConnection.ISettings,
browserDashboardCheck: boolean = false
): Promise<DashboardURLInfo> {
url = normalizeDashboardUrl(url, settings.baseUrl);

Expand Down Expand Up @@ -607,6 +621,31 @@ namespace Private {
plots: {}
};
});
} else if (browserDashboardCheck) {
return fetch(URLExt.join(url, 'individual-plots.json'))
.then(async response => {
if (response.status === 200) {
const plots = (await response.json()) as { [plot: string]: string };
return {
url,
isActive: true,
plots
};
} else {
return {
url,
isActive: false,
plots: {}
};
}
})
.catch(() => {
return {
url,
isActive: false,
plots: {}
};
});
}

const response = await ServerConnection.makeRequest(
Expand Down
9 changes: 9 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ async function activate(
// or are unable to use it.
let hideClusterManager: boolean = false;

// Whether to test the Dask dashboard using a fetch request or to proceed
// with default behavior.
let browserDashboardCheck: boolean = false;

// Update the existing trackers and signals in light of a change to the
// settings system. In particular, this reacts to a change in the setting
// for auto-starting cluster client.
Expand Down Expand Up @@ -343,6 +347,11 @@ async function activate(
autoStartClient = settings.get('autoStartClient').composite as boolean;
updateTrackers();

// Determine whether to validate dashboards via browser check.
browserDashboardCheck = settings.get('browserDashboardCheck')
.composite as boolean;
sidebar.dashboardLauncher.input.browserDashboardCheck = browserDashboardCheck;

//Determine whether to hide the cluster manager
hideClusterManager = settings.get('hideClusterManager')
.composite as boolean;
Expand Down