Allow connection to servers with no token and no password (microsoft#7708)

IanMatthewHuff · web-flow · commit 2862614ef26f · 2019-10-01T15:19:28.000-07:00
diff --git a/news/2 Fixes/7137.md b/news/2 Fixes/7137.md
@@ -0,0 +1 @@
+Fix regression to allow connection to servers with no token and no password and add functional test for this scenario
diff --git a/news/requirements.txt b/news/requirements.txt
@@ -1,2 +1,2 @@
 docopt~=0.6.2
-pytest~=3.4.1
+pytest>=5.2.0
diff --git a/src/client/datascience/jupyter/jupyterPasswordConnect.ts b/src/client/datascience/jupyter/jupyterPasswordConnect.ts
@@ -53,13 +53,17 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
                 sessionCookieName = sessionResult.sessionCookieName;
                 sessionCookieValue = sessionResult.sessionCookieValue;
             }
+        } else {
+            // If userPassword is undefined or '' then the user didn't pick a password. In this case return back that we should just try to connect
+            // like a standard connection. Might be the case where there is no token and no password
+            return { emptyPassword: true, xsrfCookie: '', sessionCookieName: '', sessionCookieValue: '' };
         }
         userPassword = undefined;
 
         // If we found everything return it all back if not, undefined as partial is useless
         if (xsrfCookie && sessionCookieName && sessionCookieValue) {
             sendTelemetryEvent(Telemetry.GetPasswordSuccess);
-            return { xsrfCookie, sessionCookieName, sessionCookieValue };
+            return { xsrfCookie, sessionCookieName, sessionCookieValue, emptyPassword: false };
         } else {
             sendTelemetryEvent(Telemetry.GetPasswordFailure);
             return undefined;
@@ -69,14 +73,14 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
     // For HTTPS connections respect our allowUnauthorized setting by adding in an agent to enable that on the request
     private addAllowUnauthorized(url: string, allowUnauthorized: boolean, options: nodeFetch.RequestInit): nodeFetch.RequestInit {
         if (url.startsWith('https') && allowUnauthorized) {
-            const requestAgent = new HttpsAgent({rejectUnauthorized: false});
-            return {...options, agent: requestAgent};
+            const requestAgent = new HttpsAgent({ rejectUnauthorized: false });
+            return { ...options, agent: requestAgent };
         }
 
         return options;
     }
 
-    private async getUserPassword() : Promise<string | undefined> {
+    private async getUserPassword(): Promise<string | undefined> {
         // First get the proposed URI from the user
         return this.appShell.showInputBox({
             prompt: localize.DataScience.jupyterSelectPasswordPrompt(),
@@ -112,7 +116,7 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
         allowUnauthorized: boolean,
         xsrfCookie: string,
         password: string,
-        fetchFunction: (url: nodeFetch.RequestInfo, init?: nodeFetch.RequestInit) => Promise<nodeFetch.Response>): Promise<{sessionCookieName: string | undefined; sessionCookieValue: string | undefined}> {
+        fetchFunction: (url: nodeFetch.RequestInfo, init?: nodeFetch.RequestInit) => Promise<nodeFetch.Response>): Promise<{ sessionCookieName: string | undefined; sessionCookieValue: string | undefined }> {
         let sessionCookieName: string | undefined;
         let sessionCookieValue: string | undefined;
         // Create the form params that we need
@@ -138,7 +142,7 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
             }
         }
 
-        return {sessionCookieName, sessionCookieValue};
+        return { sessionCookieName, sessionCookieValue };
     }
 
     private getCookies(response: nodeFetch.Response): Map<string, string> {
diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts
@@ -116,11 +116,13 @@ export class JupyterSessionManager implements IJupyterSessionManager {
         if (connInfo.token === '' || connInfo.token === 'null') {
             serverSettings = { ...serverSettings, token: '' };
             const pwSettings = await this.jupyterPasswordConnect.getPasswordConnectionInfo(connInfo.baseUrl, connInfo.allowUnauthorized ? true : false);
-            if (pwSettings) {
+            if (pwSettings && !pwSettings.emptyPassword) {
                 cookieString = this.getSessionCookieString(pwSettings);
                 const requestHeaders = { Cookie: cookieString, 'X-XSRFToken': pwSettings.xsrfCookie };
                 requestInit = { ...requestInit, headers: requestHeaders };
                 requiresWebSocket = true;
+            } else if (pwSettings && pwSettings.emptyPassword) {
+                serverSettings = { ...serverSettings, token: connInfo.token };
             } else {
                 // Failed to get password info, notify the user
                 throw new Error(localize.DataScience.passwordFailure());
diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts
@@ -145,6 +145,7 @@ export interface IJupyterDebugger {
 }
 
 export interface IJupyterPasswordConnectInfo {
+    emptyPassword: boolean;
     xsrfCookie: string;
     sessionCookieName: string;
     sessionCookieValue: string;
diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts
@@ -9,10 +9,12 @@ import { injectable } from 'inversify';
 import * as os from 'os';
 import * as path from 'path';
 import { Readable, Writable } from 'stream';
+import * as TypeMoq from 'typemoq';
 import * as uuid from 'uuid/v4';
 import { Disposable, Uri } from 'vscode';
 import { CancellationToken, CancellationTokenSource } from 'vscode-jsonrpc';
 
+import { IApplicationShell } from '../../client/common/application/types';
 import { Cancellation, CancellationError } from '../../client/common/cancellation';
 import { EXTENSION_ROOT_DIR } from '../../client/common/constants';
 import { traceError, traceInfo } from '../../client/common/logger';
@@ -59,8 +61,6 @@ suite('DataScience notebook tests', () => {
     setup(() => {
         ioc = new DataScienceIocContainer();
         ioc.registerDataScienceTypes();
-        jupyterExecution = ioc.serviceManager.get<IJupyterExecution>(IJupyterExecution);
-        processFactory = ioc.serviceManager.get<IProcessServiceFactory>(IProcessServiceFactory);
     });
 
     teardown(async () => {
@@ -199,8 +199,14 @@ suite('DataScience notebook tests', () => {
         });
     }
 
-    function runTest(name: string, func: () => Promise<void>, _notebookProc?: ChildProcess) {
+    function runTest(name: string, func: () => Promise<void>, _notebookProc?: ChildProcess, rebindFunc?: () => void) {
         test(name, async () => {
+            // Give tests a chance to rebind IOC services before we fetch jupyterExecution and processFactory
+            if (rebindFunc) {
+                rebindFunc();
+            }
+            jupyterExecution = ioc.serviceManager.get<IJupyterExecution>(IJupyterExecution);
+            processFactory = ioc.serviceManager.get<IProcessServiceFactory>(IProcessServiceFactory);
             console.log(`Starting test ${name} ...`);
             if (await jupyterExecution.isNotebookSupported()) {
                 return func();
@@ -282,6 +288,53 @@ suite('DataScience notebook tests', () => {
         }
     });
 
+    // Connect to a server that doesn't have a token or password, customers use this and we regressed it once
+    runTest('Remote No Auth', async () => {
+        const python = await getNotebookCapableInterpreter(ioc, processFactory);
+        const procService = await processFactory.create();
+
+        if (procService && python) {
+            const connectionFound = createDeferred();
+            const configFile = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience', 'serverConfigFiles', 'remoteNoAuth.py');
+            const exeResult = procService.execObservable(python.path, ['-m', 'jupyter', 'notebook', `--config=${configFile}`], { env: process.env, throwOnStdErr: false });
+            disposables.push(exeResult);
+
+            exeResult.out.subscribe((output: Output<string>) => {
+                traceInfo(`remote jupyter output: ${output.out}`);
+                const connectionURL = getIPConnectionInfo(output.out);
+                if (connectionURL) {
+                    connectionFound.resolve(connectionURL);
+                }
+            });
+
+            const connString = await connectionFound.promise;
+            const uri = connString as string;
+
+            // We have a connection string here, so try to connect jupyterExecution to the notebook server
+            const server = await jupyterExecution.connectToNotebookServer({ uri, useDefaultConfig: true, purpose: '' });
+            const notebook = server ? await server.createNotebook(Uri.parse(Identifiers.InteractiveWindowIdentity)) : undefined;
+            if (!notebook) {
+                assert.fail('Failed to connect to remote password server');
+            } else {
+                await verifySimple(notebook, `a=1${os.EOL}a`, 1);
+            }
+            // Have to dispose here otherwise the process may exit before hand and mess up cleanup.
+            await server!.dispose();
+        }
+    }, undefined, () => {
+        const dummyDisposable = {
+            dispose: () => { return; }
+        };
+        const appShell = TypeMoq.Mock.ofType<IApplicationShell>();
+        appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns((e) => { throw e; });
+        appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(''));
+        appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((_a1: string, a2: string, _a3: string) => Promise.resolve(a2));
+        appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((_a1: string, a2: string, _a3: string, _a4: string) => Promise.resolve(a2));
+        appShell.setup(a => a.showInputBox(TypeMoq.It.isAny())).returns(() => Promise.resolve(''));
+        appShell.setup(a => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable);
+        ioc.serviceManager.rebindInstance<IApplicationShell>(IApplicationShell, appShell.object);
+    });
+
     runTest('Remote Password', async () => {
         const python = await getNotebookCapableInterpreter(ioc, processFactory);
         const procService = await processFactory.create();
@@ -368,6 +421,8 @@ suite('DataScience notebook tests', () => {
     });
 
     test('Not installed', async () => {
+        jupyterExecution = ioc.serviceManager.get<IJupyterExecution>(IJupyterExecution);
+        processFactory = ioc.serviceManager.get<IProcessServiceFactory>(IProcessServiceFactory);
         // Rewire our data we use to search for processes
         class EmptyInterpreterService implements IInterpreterService {
             public get hasInterpreters(): Promise<boolean> {
@@ -1001,6 +1056,8 @@ plt.show()`,
     });
 
     test('Notebook launch failure', async function () {
+        jupyterExecution = ioc.serviceManager.get<IJupyterExecution>(IJupyterExecution);
+        processFactory = ioc.serviceManager.get<IProcessServiceFactory>(IProcessServiceFactory);
         if (!ioc.mockJupyter) {
             // tslint:disable-next-line: no-invalid-this
             this.skip();
diff --git a/src/test/datascience/serverConfigFiles/remoteNoAuth.py b/src/test/datascience/serverConfigFiles/remoteNoAuth.py
@@ -0,0 +1,4 @@
+# With these settings you can connect to a server with no token and no password
+c.NotebookApp.token = ''
+c.NotebookApp.open_browser = False
+c.NotebookApp.disable_check_xsrf = True 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Fix regression to allow connection to servers with no token and no password and add functional test for this scenario`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`	`1`	`docopt~=0.6.2`
`2`		`-pytest~=3.4.1`
	`2`	`+pytest>=5.2.0`
Original file line number	Diff line number	Diff line change
`@@ -145,6 +145,7 @@ export interface IJupyterDebugger {`
`145`	`145`	`}`
`146`	`146`
`147`	`147`	`export interface IJupyterPasswordConnectInfo {`
	`148`	`+ emptyPassword: boolean;`
`148`	`149`	`xsrfCookie: string;`
`149`	`150`	`sessionCookieName: string;`
`150`	`151`	`sessionCookieValue: string;`