WIp

src/kernels/jupyter/connection/jupyterConnection.ts

@@ -61,7 +61,7 @@ export class JupyterConnection {
         try {
             // Attempt to list the running kernels. It will return empty if there are none, but will
             // throw if can't connect.
-            sessionManager = await this.jupyterSessionManagerFactory.create(connection, false);
+            sessionManager = this.jupyterSessionManagerFactory.create(connection);
             await Promise.all([sessionManager.getRunningKernels(), sessionManager.getKernelSpecs()]);
             // We should throw an exception if any of that fails.
         } catch (err) {

src/kernels/jupyter/connection/jupyterConnection.unit.test.ts

@@ -73,7 +73,7 @@ suite('Jupyter Connection', async () => {
         );
 
         (instance(sessionManager) as any).then = undefined;
-        when(sessionManagerFactory.create(anything(), anything())).thenResolve(instance(sessionManager));
+        when(sessionManagerFactory.create(anything())).thenReturn(instance(sessionManager));
         const serverConnectionChangeEvent = new EventEmitter<void>();
         disposables.push(serverConnectionChangeEvent);
 

src/kernels/jupyter/finder/remoteKernelFinder.ts

@@ -322,7 +322,7 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable {
     public async listKernelsFromConnection(connInfo: IJupyterConnection): Promise<RemoteKernelConnectionMetadata[]> {
         const disposables: IAsyncDisposable[] = [];
         try {
-            const sessionManager = await this.jupyterSessionManagerFactory.create(connInfo);
+            const sessionManager = this.jupyterSessionManagerFactory.create(connInfo);
             disposables.push(sessionManager);
 
             // Get running and specs at the same time

src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts

@@ -139,7 +139,7 @@ suite(`Remote Kernel Finder`, () => {
         jupyterSessionManager = mock(JupyterSessionManager);
         when(jupyterSessionManager.dispose()).thenResolve();
         const jupyterSessionManagerFactory = mock(JupyterSessionManagerFactory);
-        when(jupyterSessionManagerFactory.create(anything())).thenResolve(instance(jupyterSessionManager));
+        when(jupyterSessionManagerFactory.create(anything())).thenReturn(instance(jupyterSessionManager));
         const extensionChecker = mock(PythonExtensionChecker);
         when(extensionChecker.isPythonExtensionInstalled).thenReturn(true);
         fs = mock(FileSystem);

src/kernels/jupyter/session/jupyterKernelSessionFactory.ts

@@ -82,7 +82,7 @@ export class JupyterKernelSessionFactory implements IKernelSessionFactory {
 
             Cancellation.throwIfCanceled(options.token);
 
-            const sessionManager = await this.sessionManagerFactory.create(connection);
+            const sessionManager = this.sessionManagerFactory.create(connection);
             this.asyncDisposables.push(sessionManager);
             disposablesWhenThereAreFailures.push(new Disposable(() => sessionManager.dispose().catch(noop)));
 

src/kernels/jupyter/session/jupyterSessionManagerFactory.ts

@@ -3,15 +3,8 @@
 
 import { inject, injectable, named, optional } from 'inversify';
 import { JupyterSessionManager } from './jupyterSessionManager';
-import { IApplicationShell } from '../../../platform/common/application/types';
 import { JUPYTER_OUTPUT_CHANNEL } from '../../../platform/common/constants';
-import {
-    GLOBAL_MEMENTO,
-    IAsyncDisposableRegistry,
-    IConfigurationService,
-    IMemento,
-    IOutputChannel
-} from '../../../platform/common/types';
+import { IAsyncDisposableRegistry, IConfigurationService, IOutputChannel } from '../../../platform/common/types';
 import { IJupyterConnection } from '../../types';
 import {
     IJupyterSessionManagerFactory,
@@ -21,21 +14,12 @@ import {
     IJupyterRequestAgentCreator,
     IJupyterRequestCreator
 } from '../types';
-import { Common, DataScience } from '../../../platform/common/utils/localize';
-import { isBuiltInJupyterServerProvider } from '../helpers';
-import { Memento } from 'vscode';
-
-// Key for our insecure connection global state
-const GlobalStateUserAllowsInsecureConnections = 'DataScienceAllowInsecureConnections';
 
 @injectable()
 export class JupyterSessionManagerFactory implements IJupyterSessionManagerFactory {
-    private static secureServers = new Map<string, Promise<boolean>>();
     constructor(
         @inject(IConfigurationService) private config: IConfigurationService,
         @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) private jupyterOutput: IOutputChannel,
-        @inject(IApplicationShell) private readonly appShell: IApplicationShell,
-        @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento,
         @inject(IJupyterKernelService) @optional() private readonly kernelService: IJupyterKernelService | undefined,
         @inject(IJupyterBackingFileCreator) private readonly backingFileCreator: IJupyterBackingFileCreator,
         @inject(IJupyterRequestAgentCreator)
@@ -49,10 +33,7 @@ export class JupyterSessionManagerFactory implements IJupyterSessionManagerFacto
      * Creates a new IJupyterSessionManager.
      * @param connInfo - connection information to the server that's already running.
      */
-    public async create(connInfo: IJupyterConnection): Promise<IJupyterSessionManager> {
-        // Before we connect, see if we are trying to make an insecure connection, if we are, warn the user
-        await this.secureConnectionCheck(connInfo);
-
+    public create(connInfo: IJupyterConnection): IJupyterSessionManager {
         const result = new JupyterSessionManager(
             this.jupyterOutput,
             this.config,
@@ -65,61 +46,4 @@ export class JupyterSessionManagerFactory implements IJupyterSessionManagerFacto
         this.asyncDisposables.push(result);
         return result;
     }
-
-    // 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.globalMemento.get<boolean>(GlobalStateUserAllowsInsecureConnections, false)) {
-            return;
-        }
-
-        // If they are local launch, https, or have a token, then they are secure
-        const isEmptyToken = connInfo.token === '' || connInfo.token === 'null';
-        if (connInfo.localLaunch || connInfo.baseUrl.startsWith('https') || !isEmptyToken) {
-            return;
-        }
-
-        // At this point prompt the user, cache the promise so we don't ask multiple times for the same server
-        let serverSecurePromise = JupyterSessionManagerFactory.secureServers.get(connInfo.baseUrl);
-
-        if (serverSecurePromise === undefined) {
-            if (!isBuiltInJupyterServerProvider(connInfo.serverHandle.id) || connInfo.localLaunch) {
-                // If a Jupyter URI provider is providing this URI, then we trust it.
-                serverSecurePromise = Promise.resolve(true);
-                JupyterSessionManagerFactory.secureServers.set(connInfo.baseUrl, serverSecurePromise);
-            } else {
-                serverSecurePromise = this.insecureServerWarningPrompt();
-                JupyterSessionManagerFactory.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(DataScience.insecureSessionDenied);
-        }
-    }
-
-    // If connecting on HTTP without a token prompt the user that this connection may not be secure
-    private async insecureServerWarningPrompt(): Promise<boolean> {
-        const insecureMessage = DataScience.insecureSessionMessage;
-        const insecureLabels = [Common.bannerLabelYes, Common.bannerLabelNo, Common.doNotShowAgain];
-        const response = await this.appShell.showWarningMessage(insecureMessage, ...insecureLabels);
-
-        switch (response) {
-            case Common.bannerLabelYes:
-                // On yes just proceed as normal
-                return true;
-
-            case Common.doNotShowAgain:
-                // For don't ask again turn on the global true
-                await this.globalMemento.update(GlobalStateUserAllowsInsecureConnections, true);
-                return true;
-
-            case Common.bannerLabelNo:
-            default:
-                // No or for no choice return back false to block
-                return false;
-        }
-    }
 }

src/kernels/jupyter/session/kernelSessionFactory.unit.test.ts

@@ -49,7 +49,7 @@ suite('NotebookProvider', () => {
             jupyterSessionManager.startNew(anything(), anything(), anything(), anything(), anything(), anything())
         ).thenResolve(instance(mockSession));
         const sessionManagerFactory = mock<IJupyterSessionManagerFactory>();
-        when(sessionManagerFactory.create(anything())).thenResolve(instance(jupyterSessionManager));
+        when(sessionManagerFactory.create(anything())).thenReturn(instance(jupyterSessionManager));
         const jupyterConnection = mock<JupyterConnection>();
         when(jupyterConnection.createConnectionInfo(anything())).thenResolve({
             localLaunch: true,

src/kernels/jupyter/types.ts

@@ -55,7 +55,7 @@ export interface IJupyterServerHelper extends IAsyncDisposable {
 
 export const IJupyterSessionManagerFactory = Symbol('IJupyterSessionManagerFactory');
 export interface IJupyterSessionManagerFactory {
-    create(connInfo: IJupyterConnection, failOnPassword?: boolean): Promise<IJupyterSessionManager>;
+    create(connInfo: IJupyterConnection): IJupyterSessionManager;
 }
 
 export interface IJupyterSessionManager extends IAsyncDisposable {

src/platform/common/cache.ts

@@ -27,6 +27,7 @@ export class OldCacheCleaner implements IExtensionSyncActivationService {
             [
                 await this.getUriAccountKey(),
                 'currentServerHash',
+                'DataScienceAllowInsecureConnections',
                 'connectToLocalKernelsOnly',
                 'JUPYTER_LOCAL_KERNELSPECS',
                 'JUPYTER_LOCAL_KERNELSPECS_V1',

src/standalone/userJupyterServer/jupyterPasswordConnect.ts

@@ -65,17 +65,15 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
         let result = this.savedConnectInfo.get(id);
         if (!result) {
             const deferred = (JupyterPasswordConnect._prompt = createDeferred());
-            result = this.getNonCachedPasswordConnectionInfo({ url: newUrl, isTokenEmpty, displayName }).then(
-                (value) => {
-                    // If we fail to get a valid password connect info, don't save the value
-                    traceWarning(`Password for ${newUrl} was invalid.`);
-                    if (!value) {
-                        this.savedConnectInfo.delete(id);
-                    }
-
-                    return value;
+            result = this.getJupyterConnectionInfo({ url: newUrl, isTokenEmpty, displayName }).then((value) => {
+                // If we fail to get a valid password connect info, don't save the value
+                traceWarning(`Password for ${newUrl} was invalid.`);
+                if (!value) {
+                    this.savedConnectInfo.delete(id);
                 }
-            );
+
+                return value;
+            });
             result.finally(() => {
                 deferred.resolve();
                 if (JupyterPasswordConnect._prompt === deferred) {
@@ -88,13 +86,6 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
         return result;
     }
 
-    private async getNonCachedPasswordConnectionInfo(options: {
-        url: string;
-        isTokenEmpty: boolean;
-        displayName?: string;
-    }): Promise<IJupyterPasswordConnectInfo | undefined> {
-        return this.getJupyterConnectionInfo(options);
-    }
     private async getJupyterConnectionInfo({
         url,
         isTokenEmpty,
@@ -134,12 +125,12 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
             } 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 {};
+                return;
             }
             userPassword = undefined;
         } else {
             // If no password needed, act like empty password and no cookie
-            return {};
+            return;
         }
 
         // If we found everything return it all back if not, undefined as partial is useless
@@ -151,7 +142,7 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect {
             return { requestHeaders };
         } else {
             sendTelemetryEvent(Telemetry.GetPasswordFailure);
-            return undefined;
+            return;
         }
     }
 

src/standalone/userJupyterServer/userServerUrlProvider.ts

@@ -31,7 +31,7 @@ import {
     IMemento,
     IsWebExtension
 } from '../../platform/common/types';
-import { DataScience } from '../../platform/common/utils/localize';
+import { Common, DataScience } from '../../platform/common/utils/localize';
 import { traceError, traceWarning } from '../../platform/logging';
 import { JupyterPasswordConnect } from './jupyterPasswordConnect';
 import { jupyterServerHandleFromString } from '../../kernels/jupyter/jupyterUtils';
@@ -191,6 +191,24 @@ export class UserJupyterServerUrlProvider
                     if (result?.requestHeaders) {
                         jupyterServerUri.authorizationHeader = result?.requestHeaders;
                     }
+
+                    // If we have auth headers info, and this is HTTP, then this means we have a password.
+                    // If on the other hand we do not have any auth header information & there is no token & no password, & this is HTTP then this is an insecure server
+                    // & we need to ask the user for consent to use this insecure server.
+                    if (
+                        !result &&
+                        jupyterServerUri.token.length === 0 &&
+                        new URL(jupyterServerUri.baseUrl).protocol.toLowerCase() === 'http'
+                    ) {
+                        const proceed = await this.secureConnectionCheck();
+                        if (!proceed) {
+                            resolve(undefined);
+                            input.hide();
+                            return;
+                        }
+                    }
+
+                    //
                     let message = '';
                     try {
                         await this.jupyterConnection.validateJupyterServer(serverHandle, jupyterServerUri, true);
@@ -297,6 +315,16 @@ export class UserJupyterServerUrlProvider
         await this.encryptedStorage.store(NewSecretStorageKey, JSON.stringify(this._servers));
         this._onDidChangeHandles.fire();
     }
+
+    /**
+     * Check if our server connection is considered secure. If it is not, ask the user if they want to connect
+     */
+    private async secureConnectionCheck(): Promise<boolean> {
+        const insecureMessage = DataScience.insecureSessionMessage;
+        const insecureLabels = [Common.bannerLabelYes, Common.bannerLabelNo];
+        const response = await this.applicationShell.showWarningMessage(insecureMessage, ...insecureLabels);
+        return response === Common.bannerLabelYes;
+    }
 }
 
 /**

src/test/datascience/notebook/helper.ts

@@ -348,7 +348,7 @@ async function shutdownRemoteKernels() {
                 await serverUriStorage.getAll()
             )[0].serverHandle
         );
-        const sessionManager = await jupyterSessionManagerFactory.create(connection);
+        const sessionManager = jupyterSessionManagerFactory.create(connection);
         const liveKernels = await sessionManager.getRunningKernels();
         await Promise.all(
             liveKernels.filter((item) => item.id).map((item) => KernelAPI.shutdownKernel(item.id!).catch(noop))