Warn on insecure remote (microsoft#13060)

Author: IanMatthewHuff

news/1 Enhancements/12980.md

@@ -0,0 +1 @@
+Warn users if they are connecting over http without a token.

package.nls.json

@@ -36,6 +36,8 @@
  "DataScience.launchNotebookTrustPrompt.yes": "Trust",
  "DataScience.launchNotebookTrustPrompt.no": "Do not trust",
  "DataScience.launchNotebookTrustPrompt.trustAllNotebooks": "Trust all notebooks",
+ "DataScience.insecureSessionMessage": "Connecting over HTTP without a token may be an insecure connection. Do you want to connect to a possibly insecure server?",
+ "DataScience.insecureSessionDenied": "Denied connection to insecure server.",
  "python.command.python.viewLanguageServerOutput.title": "Show Language Server Output",
  "python.command.python.selectAndRunTestMethod.title": "Run Test Method ...",
  "python.command.python.selectAndDebugTestMethod.title": "Debug Test Method ...",

src/client/common/utils/localize.ts

@@ -1055,6 +1055,14 @@ export namespace DataScience {
  'DataScience.launchNotebookTrustPrompt.trustAllNotebooks',
  'Trust all notebooks'
  );
+ export const insecureSessionMessage = localize(
+ 'DataScience.insecureSessionMessage',
+ 'Connecting over HTTP without a token may be an insecure connection. Do you want to connect to a possibly insecure server?'
+ );
+ export const insecureSessionDenied = localize(
+ 'DataScience.insecureSessionDenied',
+ 'Denied connection to insecure server.'
+ );
  export const previewNotebookOnlySupportedInVSCInsiders = localize(
  'DataScience.previewNotebookOnlySupportedInVSCInsiders',
  'The Preview Notebook Editor is supported only in the Insiders version of Visual Studio Code.'

src/client/datascience/jupyter/jupyterSessionManager.ts

@@ -5,9 +5,10 @@ import type { ContentsManager, ServerConnection, Session, SessionManager } from
 import { Agent as HttpsAgent } from 'https';
 import * as nodeFetch from 'node-fetch';
 import { CancellationToken } from 'vscode-jsonrpc';
+import { IApplicationShell } from '../../common/application/types';
 
 import { traceError, traceInfo } from '../../common/logger';
-import { IConfigurationService, IOutputChannel } from '../../common/types';
+import { IConfigurationService, IOutputChannel, IPersistentState, IPersistentStateFactory } from '../../common/types';
 import { sleep } from '../../common/utils/async';
 import * as localize from '../../common/utils/localize';
 import { noop } from '../../common/utils/misc';
@@ -27,14 +28,19 @@ import { JupyterKernelSpec } from './kernels/jupyterKernelSpec';
 import { KernelSelector } from './kernels/kernelSelector';
 import { LiveKernelModel } from './kernels/types';
 
+// Key for our insecure connection global state
+const GlobalStateUserAllowsInsecureConnections = 'DataScienceAllowInsecureConnections';
+
 // tslint:disable: no-any
 
 export class JupyterSessionManager implements IJupyterSessionManager {
+ private static secureServers = new Map<string, Promise<boolean>>();
  private sessionManager: SessionManager | undefined;
  private contentsManager: ContentsManager | undefined;
  private connInfo: IJupyterConnection | undefined;
  private serverSettings: ServerConnection.ISettings | undefined;
  private _jupyterlab?: typeof import('@jupyterlab/services');
+ private readonly userAllowsInsecureConnections: IPersistentState<boolean>;
  private get jupyterlab(): typeof import('@jupyterlab/services') {
  if (!this._jupyterlab) {
  // tslint:disable-next-line: no-require-imports
@@ -48,8 +54,15 @@ export class JupyterSessionManager implements IJupyterSessionManager {
  private failOnPassword: boolean | undefined,
  private kernelSelector: KernelSelector,
  private outputChannel: IOutputChannel,
- private configService: IConfigurationService
- ) {}
+ private configService: IConfigurationService,
+ private readonly appShell: IApplicationShell,
+ private readonly stateFactory: IPersistentStateFactory
+ ) {
+ this.userAllowsInsecureConnections = this.stateFactory.createGlobalPersistentState<boolean>(
+ GlobalStateUserAllowsInsecureConnections,
+ false
+ );
+ }
 
  public async dispose() {
  traceInfo(`Disposing session manager`);
@@ -229,6 +242,9 @@ export class JupyterSessionManager implements IJupyterSessionManager {
  wsUrl: connInfo.baseUrl.replace('http', 'ws')
  };
 
+ // Before we connect, see if we are trying to make an insecure connection, if we are, warn the user
+ await this.secureConnectionCheck(connInfo);
+
  // Agent is allowed to be set on this object, but ts doesn't like it on RequestInit, so any
  // tslint:disable-next-line:no-any
  let requestInit: any = { cache: 'no-store', credentials: 'same-origin' };
@@ -305,4 +321,58 @@ export class JupyterSessionManager implements IJupyterSessionManager {
  traceInfo(`Creating server with settings : ${JSON.stringify(serverSettings)}`);
  return this.jupyterlab.ServerConnection.makeSettings(serverSettings);
  }
+
+ // If connecting on HTTP without a token prompt the user that this connection may not be secure
+ private async insecureServerWarningPrompt(): Promise<boolean> {
+ const insecureMessage = localize.DataScience.insecureSessionMessage();
+ const insecureLabels = [
+ localize.Common.bannerLabelYes(),
+ localize.Common.bannerLabelNo(),
+ localize.Common.doNotShowAgain()
+ ];
+ const response = await this.appShell.showWarningMessage(insecureMessage, ...insecureLabels);
+
+ switch (response) {
+ case localize.Common.bannerLabelYes():
+ // On yes just proceed as normal
+ return true;
+
+ case localize.Common.doNotShowAgain():
+ // For don't ask again turn on the global true
+ await this.userAllowsInsecureConnections.updateValue(true);
+ return true;
+
+ case localize.Common.bannerLabelNo():
+ default:
+ // No or for no choice return back false to block
+ return false;
+ }
+ }
+
+ // Check if our server connection is considered secure. If it is not, ask the user if they want to connect
+ // If not, throw to bail out on the process
+ private async secureConnectionCheck(connInfo: IJupyterConnection): Promise<void> {
+ // If they have turned on global server trust then everything is secure
+ if (this.userAllowsInsecureConnections.value) {
+ return;
+ }
+
+ // If they are local launch, https, or have a token, then they are secure
+ if (connInfo.localLaunch || connInfo.baseUrl.startsWith('https') || connInfo.token !== 'null') {
+ return;
+ }
+
+ // At this point prompt the user, cache the promise so we don't ask multiple times for the same server
+ let serverSecurePromise = JupyterSessionManager.secureServers.get(connInfo.baseUrl);
+
+ if (serverSecurePromise === undefined) {
+ serverSecurePromise = this.insecureServerWarningPrompt();
+ JupyterSessionManager.secureServers.set(connInfo.baseUrl, serverSecurePromise);
+ }
+
+ // If our server is not secure, throw here to bail out on the process
+ if (!(await serverSecurePromise)) {
+ throw new Error(localize.DataScience.insecureSessionDenied());
+ }
+ }
 }

src/client/datascience/jupyter/jupyterSessionManagerFactory.ts

@@ -2,8 +2,9 @@
 // Licensed under the MIT License.
 'use strict';
 import { inject, injectable, named } from 'inversify';
+import { IApplicationShell } from '../../common/application/types';
 
-import { IConfigurationService, IOutputChannel } from '../../common/types';
+import { IConfigurationService, IOutputChannel, IPersistentStateFactory } from '../../common/types';
 import { JUPYTER_OUTPUT_CHANNEL } from '../constants';
 import {
  IJupyterConnection,
@@ -20,7 +21,9 @@ export class JupyterSessionManagerFactory implements IJupyterSessionManagerFacto
  @inject(IJupyterPasswordConnect) private jupyterPasswordConnect: IJupyterPasswordConnect,
  @inject(IConfigurationService) private config: IConfigurationService,
  @inject(KernelSelector) private kernelSelector: KernelSelector,
- @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) private jupyterOutput: IOutputChannel
+ @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) private jupyterOutput: IOutputChannel,
+ @inject(IApplicationShell) private readonly appShell: IApplicationShell,
+ @inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory
  ) {}
 
  /**
@@ -35,7 +38,9 @@ export class JupyterSessionManagerFactory implements IJupyterSessionManagerFacto
  failOnPassword,
  this.kernelSelector,
  this.jupyterOutput,
- this.config
+ this.config,
+ this.appShell,
+ this.stateFactory
  );
  await result.initialize(connInfo);
  return result;

src/test/datascience/notebook.functional.test.ts

@@ -520,12 +520,95 @@ suite('DataScience notebook tests', () => {
  )
  )
  .returns((_a1: string, a2: string, _a3: string, _a4: string) => Promise.resolve(a2));
+ appShell
+ .setup((a) =>
+ a.showWarningMessage(
+ 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);
  }
  );
 
+ // For a connection to a remote machine that is not secure deny the connection and we should not connect
+ runTest(
+ 'Remote Deny Insecure',
+ async () => {
+ const pythonService = await createPythonService();
+
+ if (pythonService) {
+ const configFile = path.join(
+ EXTENSION_ROOT_DIR,
+ 'src',
+ 'test',
+ 'datascience',
+ 'serverConfigFiles',
+ 'remoteNoAuth.py'
+ );
+ const uri = await startRemoteServer(pythonService, [
+ '-m',
+ 'jupyter',
+ 'notebook',
+ `--config=${configFile}`
+ ]);
+
+ // Try to create, we expect a failure here as we will deny the insecure connection
+ await createNotebook(uri, undefined, true);
+ }
+ },
+ 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));
+ // This is the call to app shell that we are changing from the above test
+ appShell
+ .setup((a) =>
+ a.showWarningMessage(
+ TypeMoq.It.isAny(),
+ TypeMoq.It.isAny(),
+ TypeMoq.It.isAny(),
+ TypeMoq.It.isAny()
+ )
+ )
+ .returns((_a1: string, _a2: string, a3: string, _a4: string) => Promise.resolve(a3));
+ 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 pythonService = await createPythonService();