Skip to content

Commit

Permalink
More tests for parsing/validating Jupyter Notebook/Lab Urls (#14340)
Browse files Browse the repository at this point in the history
* Wip

* Updates

* More mandatory tests

* More tests

* More tests

* Revert

* Support for certs

* oops

* misc changes

* support for https

* Misc

* Fixes to extraction of Url from jupyter CLI output

* Further fixes

* Revert
  • Loading branch information
DonJayamanne authored Sep 19, 2023
1 parent e3cbadc commit 71aa6ab
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 84 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,13 @@ jobs:
tags: '^[^@]+$|@mandatory'
os: ubuntu-latest
ipywidgetsVersion: ''
- jupyterConnection: remote
python: python
pythonVersion: '3.10'
packageVersion: 'prerelease'
tags: '^[^@]+$|@mandatory'
os: ubuntu-latest
ipywidgetsVersion: ''
- jupyterConnection: web
python: python
pythonVersion: '3.10'
Expand Down
4 changes: 0 additions & 4 deletions build/venv-test-ipywidgets8-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
pytest < 6.0.0; python_version > '2.7' # Tests do not support pytest 6 yet.
# Python 2.7 compatibility (pytest)
pytest==7.3.1; python_version == '2.7'
# Requirements needed to run install_debugpy.py
packaging
# List of requirements for ipython tests
Expand All @@ -21,7 +18,6 @@ py4j
bqplot
K3D
ipyleaflet
jinja2==3.1.2 # https://github.com/microsoft/vscode-jupyter/issues/9468#issuecomment-1078468039
matplotlib
ipympl
traitlets==5.9.0 # https://github.com/microsoft/vscode-jupyter/issues/14338
6 changes: 5 additions & 1 deletion src/standalone/api/api.jupyterProvider.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import { JupyterServer, JupyterServerProvider } from '../../api';
import { openAndShowNotebook } from '../../platform/common/utils/notebooks';
import { JupyterServer as JupyterServerStarter } from '../../test/datascience/jupyterServer.node';
import { IS_REMOTE_NATIVE_TEST } from '../../test/constants';
import { IS_CONDA_TEST, IS_REMOTE_NATIVE_TEST } from '../../test/constants';
import { isWeb } from '../../platform/common/utils/misc';
import { MultiStepInput } from '../../platform/common/utils/multiStepInput';

Expand All @@ -43,6 +43,10 @@ suite('Jupyter Provider Tests', function () {
if (IS_REMOTE_NATIVE_TEST() || isWeb()) {
return this.skip();
}
if (IS_CONDA_TEST()) {
// Due to upstream issue documented here https://github.com/microsoft/vscode-jupyter/issues/14338
return this.skip();
}
this.timeout(120_000);
api = await initialize();
const tokenSource = new CancellationTokenSource();
Expand Down
48 changes: 41 additions & 7 deletions src/standalone/userJupyterServer/userServerUrlProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import {
} from '../../platform/common/types';
import { Common, DataScience } from '../../platform/common/utils/localize';
import { noop } from '../../platform/common/utils/misc';
import { traceError, traceWarning } from '../../platform/logging';
import { traceError, traceVerbose, traceWarning } from '../../platform/logging';
import { JupyterPasswordConnect } from './jupyterPasswordConnect';
import {
IJupyterServerUri,
Expand Down Expand Up @@ -130,7 +130,7 @@ export class UserJupyterServerUrlProvider
// eslint-disable-next-line @typescript-eslint/no-use-before-define
this.secureConnectionValidator = new SecureConnectionValidator(applicationShell, globalMemento);
// eslint-disable-next-line @typescript-eslint/no-use-before-define
this.jupyterServerUriInput = new UserJupyterServerUriInput(clipboard, applicationShell);
this.jupyterServerUriInput = new UserJupyterServerUriInput(clipboard, applicationShell, requestCreator);
// eslint-disable-next-line @typescript-eslint/no-use-before-define
this.jupyterServerUriDisplayName = new UserJupyterServerDisplayName(applicationShell);
this.jupyterPasswordConnect = new JupyterPasswordConnect(
Expand Down Expand Up @@ -388,7 +388,7 @@ export class UserJupyterServerUrlProvider
let initialUrlWasValid = false;
if (initialUrl) {
// Validate the URI first, which would otherwise be validated when user enters the Uri into the input box.
const initialVerification = this.jupyterServerUriInput.parseUserUriAndGetValidationError(initialUrl);
const initialVerification = await this.jupyterServerUriInput.parseUserUriAndGetValidationError(initialUrl);
if (typeof initialVerification.validationError === 'string') {
// Uri has an error, show the error message by displaying the input box and pre-populating the url.
validationErrorMessage = initialVerification.validationError;
Expand Down Expand Up @@ -709,7 +709,8 @@ export class UserJupyterServerUrlProvider
export class UserJupyterServerUriInput {
constructor(
@inject(IClipboard) private readonly clipboard: IClipboard,
@inject(IApplicationShell) private readonly applicationShell: IApplicationShell
@inject(IApplicationShell) private readonly applicationShell: IApplicationShell,
@inject(IJupyterRequestCreator) private readonly requestCreator: IJupyterRequestCreator
) {}

async getUrlFromUser(
Expand Down Expand Up @@ -753,7 +754,7 @@ export class UserJupyterServerUriInput {
);

input.onDidAccept(async () => {
const result = this.parseUserUriAndGetValidationError(input.value);
const result = await this.parseUserUriAndGetValidationError(input.value);
if (typeof result.validationError === 'string') {
input.validationMessage = result.validationError;
return;
Expand All @@ -763,22 +764,55 @@ export class UserJupyterServerUriInput {
return deferred.promise;
}

public parseUserUriAndGetValidationError(
public async parseUserUriAndGetValidationError(
value: string
): { validationError: string } | { jupyterServerUri: IJupyterServerUri; url: string; validationError: undefined } {
): Promise<
{ validationError: string } | { jupyterServerUri: IJupyterServerUri; url: string; validationError: undefined }
> {
// If it ends with /lab? or /lab or /tree? or /tree, then remove that part.
const uri = value.trim().replace(/\/(lab|tree)(\??)$/, '');
const jupyterServerUri = parseUri(uri, '');
if (!jupyterServerUri) {
return { validationError: DataScience.jupyterSelectURIInvalidURI };
}
jupyterServerUri.baseUrl = (await getBaseJupyterUrl(uri, this.requestCreator)) || jupyterServerUri.baseUrl;
if (!uri.toLowerCase().startsWith('http:') && !uri.toLowerCase().startsWith('https:')) {
return { validationError: DataScience.jupyterSelectURIMustBeHttpOrHttps };
}
return { jupyterServerUri, url: uri, validationError: undefined };
}
}

export async function getBaseJupyterUrl(url: string, requestCreator: IJupyterRequestCreator) {
// Jupyter URLs can contain a path, but we only want the base URL
// E.g. user can enter http://localhost:8000/tree?token=1234
// and we need http://localhost:8000/
// Similarly user can enter http://localhost:8888/lab/workspaces/auto-R
// or http://localhost:8888/notebooks/Untitled.ipynb?kernel_name=python3
// In all of these cases, once we remove the token, and we make a request to the url
// then the jupyter server will redirect the user the loging page
// which is of the form http://localhost:8000/login?next....
// And the base url is easily identifiable as what ever is before `login?`
try {
// parseUri has special handling of `tree?` and `lab?`
// For some reasson Jupyter does not redirecto those the the a
url = parseUri(url, '')?.baseUrl || url;
if (new URL(url).pathname === '/') {
// No need to make a request, as we already have the base url.
return url;
}
const urlWithoutToken = url.indexOf('token=') > 0 ? url.substring(0, url.indexOf('token=')) : url;
const fetch = requestCreator.getFetchMethod();
const response = await fetch(urlWithoutToken, { method: 'GET', redirect: 'manual' });
const loginPage = response.headers.get('location');
if (loginPage && loginPage.includes('login?')) {
return loginPage.substring(0, loginPage.indexOf('login?'));
}
} catch (ex) {
traceVerbose(`Unable to identify the baseUrl of the Jupyter Server`, ex);
}
}

function sendRemoteTelemetryForAdditionOfNewRemoteServer(
handle: string,
baseUrl: string,
Expand Down
104 changes: 90 additions & 14 deletions src/test/datascience/jupyter/connection.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import {
IExtensionContext
} from '../../../platform/common/types';
import { IS_REMOTE_NATIVE_TEST, initialize } from '../../initialize.node';
import { startJupyterServer, closeNotebooksAndCleanUpAfterTests } from '../notebook/helper.node';
import { startJupyterServer, closeNotebooksAndCleanUpAfterTests, hijackPrompt } from '../notebook/helper.node';
import {
SecureConnectionValidator,
UserJupyterServerDisplayName,
UserJupyterServerUriInput,
UserJupyterServerUrlProvider,
getBaseJupyterUrl,
parseUri
} from '../../../standalone/userJupyterServer/userServerUrlProvider';
import {
Expand All @@ -33,7 +34,7 @@ import {
import { JupyterConnection } from '../../../kernels/jupyter/connection/jupyterConnection';
import { dispose } from '../../../platform/common/helpers';
import { anything, instance, mock, when } from 'ts-mockito';
import { CancellationTokenSource, Disposable, EventEmitter, InputBox, Memento } from 'vscode';
import { CancellationTokenSource, Disposable, EventEmitter, InputBox, Memento, workspace } from 'vscode';
import { noop } from '../../../platform/common/utils/misc';
import { DataScience } from '../../../platform/common/utils/localize';
import * as sinon from 'sinon';
Expand All @@ -42,9 +43,12 @@ import { createDeferred, createDeferredFromPromise } from '../../../platform/com
import { IMultiStepInputFactory } from '../../../platform/common/utils/multiStepInput';
import { IFileSystem } from '../../../platform/common/platform/types';

suite('Connect to Remote Jupyter Servers', function () {
suite('Connect to Remote Jupyter Servers @mandatory', function () {
// On conda these take longer for some reason.
this.timeout(120_000);
let jupyterNotebookWithAutoGeneratedToken = { url: '', dispose: noop };
let jupyterLabWithAutoGeneratedToken = { url: '', dispose: noop };
let jupyterNotebookWithCerts = { url: '', dispose: noop };
let jupyterNotebookWithHelloPassword = { url: '', dispose: noop };
let jupyterLabWithHelloPasswordAndWorldToken = { url: '', dispose: noop };
let jupyterNotebookWithHelloToken = { url: '', dispose: noop };
Expand All @@ -57,12 +61,28 @@ suite('Connect to Remote Jupyter Servers', function () {
this.timeout(120_000);
await initialize();
[
jupyterNotebookWithAutoGeneratedToken,
jupyterLabWithAutoGeneratedToken,
jupyterNotebookWithCerts,
jupyterNotebookWithHelloPassword,
jupyterLabWithHelloPasswordAndWorldToken,
jupyterNotebookWithHelloToken,
jupyterNotebookWithEmptyPasswordToken,
jupyterLabWithHelloPasswordAndEmptyToken
] = await Promise.all([
startJupyterServer({
jupyterLab: false,
standalone: true
}),
startJupyterServer({
jupyterLab: true,
standalone: true
}),
startJupyterServer({
jupyterLab: false,
standalone: true,
useCert: true
}),
startJupyterServer({
jupyterLab: false,
password: 'Hello',
Expand Down Expand Up @@ -95,6 +115,8 @@ suite('Connect to Remote Jupyter Servers', function () {
});
suiteTeardown(() => {
dispose([
jupyterNotebookWithAutoGeneratedToken,
jupyterLabWithAutoGeneratedToken,
jupyterNotebookWithHelloPassword,
jupyterLabWithHelloPasswordAndWorldToken,
jupyterNotebookWithHelloToken,
Expand All @@ -111,6 +133,7 @@ suite('Connect to Remote Jupyter Servers', function () {
let commands: ICommandManager;
let inputBox: InputBox;
let token: CancellationTokenSource;
let requestCreator: IJupyterRequestCreator;
setup(async function () {
if (!IS_REMOTE_NATIVE_TEST()) {
return this.skip();
Expand Down Expand Up @@ -165,6 +188,7 @@ suite('Connect to Remote Jupyter Servers', function () {
const onDidRemoveUriStorage = new EventEmitter<JupyterServerProviderHandle[]>();
disposables.push(onDidRemoveUriStorage);
when(serverUriStorage.onDidRemove).thenReturn(onDidRemoveUriStorage.event);
requestCreator = api.serviceContainer.get<IJupyterRequestCreator>(IJupyterRequestCreator);

userUriProvider = new UserJupyterServerUrlProvider(
instance(clipboard),
Expand Down Expand Up @@ -198,7 +222,7 @@ suite('Connect to Remote Jupyter Servers', function () {
});
suiteTeardown(() => closeNotebooksAndCleanUpAfterTests(disposables));

async function testConnection({
async function testConnectionAndVerifyBaseUrl({
password,
userUri,
failWithInvalidPassword
Expand All @@ -207,12 +231,23 @@ suite('Connect to Remote Jupyter Servers', function () {
userUri: string;
failWithInvalidPassword?: boolean;
}) {
const config = workspace.getConfiguration('jupyter');
await config.update('allowUnauthorizedRemoteConnection', false);
const prompt = await hijackPrompt(
'showErrorMessage',
{ contains: 'certificate' },
{ result: DataScience.jupyterSelfCertEnable, clickImmediately: true }
);
disposables.push(prompt);
const displayName = 'Test Remove Server Name';
when(clipboard.readText()).thenResolve(userUri);
sinon.stub(UserJupyterServerUriInput.prototype, 'getUrlFromUser').resolves({
url: userUri,
jupyterServerUri: parseUri(userUri, '')!
});
const baseUrl = `${new URL(userUri).protocol}//localhost:${new URL(userUri).port}/`;
const computedBaseUrl = await getBaseJupyterUrl(userUri, requestCreator);
assert.strictEqual(computedBaseUrl?.endsWith('/') ? computedBaseUrl : `${computedBaseUrl}/`, baseUrl);
sinon.stub(SecureConnectionValidator.prototype, 'promptToUseInsecureConnections').resolves(true);
sinon.stub(UserJupyterServerDisplayName.prototype, 'getDisplayName').resolves(displayName);
const errorMessageDisplayed = createDeferred<string>();
Expand All @@ -226,6 +261,9 @@ suite('Connect to Remote Jupyter Servers', function () {
assert.strictEqual(errorMessageDisplayed.value, DataScience.passwordFailure);
assert.ok(!handlePromise.completed);
} else {
if (new URL(userUri).protocol.includes('https')) {
assert.ok(await prompt.displayed, 'Prompt for trusting certs not displayed');
}
assert.equal(errorMessageDisplayed.value || '', '', `Password should be valid, ${errorMessageDisplayed}`);
assert.ok(handlePromise.completed, 'Did not complete');
const value = handlePromise.value;
Expand Down Expand Up @@ -255,42 +293,80 @@ suite('Connect to Remote Jupyter Servers', function () {
// assert.strictEqual(serverInfo.displayName, `Title of Server`, 'Invalid Title');
}
}
test('Connect to server with auto generated Token in URL', () =>
testConnectionAndVerifyBaseUrl({ userUri: jupyterNotebookWithAutoGeneratedToken.url, password: undefined }));
test('Connect to JuyterLab server with auto generated Token in URL', () =>
testConnectionAndVerifyBaseUrl({ userUri: jupyterLabWithAutoGeneratedToken.url, password: undefined }));
test('Connect to server with certificates', () =>
testConnectionAndVerifyBaseUrl({ userUri: jupyterNotebookWithCerts.url, password: undefined }));
test('Connect to server with auto generated Token in URL and path has tree in it', async () => {
const token = new URL(jupyterNotebookWithAutoGeneratedToken.url).searchParams.get('token')!;
const port = new URL(jupyterNotebookWithAutoGeneratedToken.url).port;
await testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${port}/tree?token=${token}`,
password: undefined
});
});
test('Connect to server with auto generated Token in URL and custom path', async () => {
const token = new URL(jupyterLabWithAutoGeneratedToken.url).searchParams.get('token')!;
const port = new URL(jupyterLabWithAutoGeneratedToken.url).port;
await testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${port}/notebooks/Untitled.ipynb?kernel_name=python3&token=${token}`,
password: undefined
});
});
test('Connect to Jupyter Lab server with auto generated Token in URL and path has lab in it', async () => {
const token = new URL(jupyterLabWithAutoGeneratedToken.url).searchParams.get('token')!;
const port = new URL(jupyterLabWithAutoGeneratedToken.url).port;
await testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${port}/lab?token=${token}`,
password: undefined
});
});
test('Connect to Jupyter Lab server with auto generated Token in URL and custom path', async () => {
const token = new URL(jupyterLabWithAutoGeneratedToken.url).searchParams.get('token')!;
const port = new URL(jupyterLabWithAutoGeneratedToken.url).port;
await testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${port}/lab/workspaces/auto-R?token=${token}`,
password: undefined
});
});
test('Connect to server with Token in URL', () =>
testConnection({ userUri: jupyterNotebookWithHelloToken.url, password: undefined }));
testConnectionAndVerifyBaseUrl({ userUri: jupyterNotebookWithHelloToken.url, password: undefined }));
test('Connect to server with Password and Token in URL', () =>
testConnection({ userUri: jupyterNotebookWithHelloPassword.url, password: 'Hello' }));
testConnectionAndVerifyBaseUrl({ userUri: jupyterNotebookWithHelloPassword.url, password: 'Hello' }));
test('Connect to Notebook server with Password and no Token in URL', () =>
testConnection({
testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${new URL(jupyterNotebookWithHelloPassword.url).port}/`,
password: 'Hello'
}));
test('Connect to Lab server with Password and no Token in URL', () =>
testConnection({
testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${new URL(jupyterLabWithHelloPasswordAndWorldToken.url).port}/`,
password: 'Hello'
}));
test('Connect to server with Invalid Password', () =>
testConnection({
testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${new URL(jupyterNotebookWithHelloPassword.url).port}/`,
password: 'Bogus',
failWithInvalidPassword: true
}));
test('Connect to Lab server with Password & Token in URL', async () =>
testConnection({ userUri: jupyterLabWithHelloPasswordAndWorldToken.url, password: 'Hello' }));
testConnectionAndVerifyBaseUrl({ userUri: jupyterLabWithHelloPasswordAndWorldToken.url, password: 'Hello' }));
test('Connect to server with empty Password & empty Token in URL', () =>
testConnection({ userUri: jupyterNotebookWithEmptyPasswordToken.url, password: '' }));
testConnectionAndVerifyBaseUrl({ userUri: jupyterNotebookWithEmptyPasswordToken.url, password: '' }));
test('Connect to server with empty Password & empty Token (nothing in URL)', () =>
testConnection({
testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${new URL(jupyterNotebookWithEmptyPasswordToken.url).port}/`,
password: ''
}));
test('Connect to Lab server with Hello Password & empty Token (not even in URL)', () =>
testConnection({
testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${new URL(jupyterLabWithHelloPasswordAndEmptyToken.url).port}/`,
password: 'Hello'
}));
test('Connect to Lab server with bogus Password & empty Token (not even in URL)', () =>
testConnection({
testConnectionAndVerifyBaseUrl({
userUri: `http://localhost:${new URL(jupyterLabWithHelloPasswordAndEmptyToken.url).port}/`,
password: 'Bogus',
failWithInvalidPassword: true
Expand Down
Loading

0 comments on commit 71aa6ab

Please sign in to comment.