diff --git a/src/sql/platform/connection/common/constants.ts b/src/sql/platform/connection/common/constants.ts index 6ce872631b..12b419731a 100644 --- a/src/sql/platform/connection/common/constants.ts +++ b/src/sql/platform/connection/common/constants.ts @@ -26,6 +26,8 @@ export const passwordChars = '***************'; export const enableSqlAuthenticationProviderConfig = 'mssql.enableSqlAuthenticationProvider'; +/** Configuration for Azure Authentication Library */ +export const azureAuthenticationLibraryConfig = 'azure.authenticationLibrary'; /* default authentication type setting name*/ export const defaultAuthenticationType = 'defaultAuthenticationType'; diff --git a/src/sql/workbench/contrib/welcome/notifyHiddenTenant/notifyHiddenTenantDialog.ts b/src/sql/workbench/contrib/welcome/notifyHiddenTenant/notifyHiddenTenantDialog.ts index 4d1f8ef032..a02d315435 100644 --- a/src/sql/workbench/contrib/welcome/notifyHiddenTenant/notifyHiddenTenantDialog.ts +++ b/src/sql/workbench/contrib/welcome/notifyHiddenTenant/notifyHiddenTenantDialog.ts @@ -23,6 +23,8 @@ import { mssqlProviderName } from 'sql/platform/connection/common/constants'; import { TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys'; import { NOTIFY_HIDETENANT_SHOWN, NOTIFY_READMORE_LINK } from 'sql/workbench/contrib/welcome/constants'; import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces'; +import { isMssqlAuthProviderEnabled } from 'sql/workbench/services/connection/browser/utils'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; export class NotifyHiddenTenantDialog extends ErrorMessageDialog { @@ -38,7 +40,8 @@ export class NotifyHiddenTenantDialog extends ErrorMessageDialog { @IInstantiationService private _instantiationService: IInstantiationService, @IStorageService private _storageService: IStorageService, @IConnectionManagementService private _connectionManagementService: IConnectionManagementService, - @IAccountManagementService private _accountManagementService: IAccountManagementService + @IAccountManagementService private _accountManagementService: IAccountManagementService, + @IConfigurationService private _configurationService: IConfigurationService, ) { super(themeService, clipboardService, layoutService, telemetryService, contextKeyService, logService, textResourcePropertiesService, openerService); } @@ -50,9 +53,11 @@ export class NotifyHiddenTenantDialog extends ErrorMessageDialog { this._storageService.store(NOTIFY_HIDETENANT_SHOWN, true, StorageScope.APPLICATION, StorageTarget.MACHINE); this._accountManagementService.getAccounts().then(accounts => { - // Do not notify users who don't have any Azure connection in their list of connections and no Azure accounts registered. + // Do not notify users who don't have any Azure connection in their list of connections and no Azure accounts registered + // or are not yet impacted with the changes from 'Enable Sql Authentication Provider' with MSAL if (!this._connectionManagementService.getConnections()?.some(conn => conn.providerName === mssqlProviderName - && conn.authenticationType === 'AzureMFA') && (!accounts || accounts.length === 0)) { + && conn.authenticationType === 'AzureMFA') && (!accounts || accounts.length === 0) + && isMssqlAuthProviderEnabled(mssqlProviderName, this._configurationService)) { return; } diff --git a/src/sql/workbench/services/connection/browser/utils.ts b/src/sql/workbench/services/connection/browser/utils.ts index d86350c67b..30bbacf6df 100644 --- a/src/sql/workbench/services/connection/browser/utils.ts +++ b/src/sql/workbench/services/connection/browser/utils.ts @@ -4,7 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/platform/connection/common/constants'; +import { azureAuthenticationLibraryConfig, enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/platform/connection/common/constants'; +import { MSAL_AUTH_LIBRARY } from 'sql/workbench/services/accountManagement/utils'; /** * Reads setting 'mssql.enableSqlAuthenticationProvider' returns true if it's enabled. @@ -13,6 +14,17 @@ import { enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/pl * @param configService Configuration service instance * @returns True if provider is MSSQL and Sql Auth provider is enabled. */ -export function isMssqlAuthProviderEnabled(provider: string, configService: IConfigurationService): boolean { - return provider === mssqlProviderName && (configService?.getValue(enableSqlAuthenticationProviderConfig) ?? true); +export function isMssqlAuthProviderEnabled(provider: string, configService: IConfigurationService | undefined): boolean { + return provider === mssqlProviderName && isMSALAuthLibraryEnabled(configService) && (configService?.getValue(enableSqlAuthenticationProviderConfig) ?? true); +} + +/** + * We need Azure core extension configuration for fetching Authentication Library setting in use. + * This is required for 'enableSqlAuthenticationProvider' to be enabled (as it applies to MSAL only). + * This can be removed in future when ADAL support is dropped. + * @param configService Configuration Service to use. + * @returns true if MSAL_AUTH_LIBRARY is enabled. + */ +export function isMSALAuthLibraryEnabled(configService: IConfigurationService | undefined): boolean { + return configService?.getValue(azureAuthenticationLibraryConfig) === MSAL_AUTH_LIBRARY /*default*/ ?? true; } diff --git a/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts b/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts index a4ce4433c9..85520ba893 100644 --- a/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts +++ b/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts @@ -95,7 +95,7 @@ suite('ConnectionDialogService tests', () => { testInstantiationService, // instantiation service undefined, // editor service undefined, // telemetry service - undefined, // configuration service + new TestConfigurationService(), // configuration service new TestCapabilitiesService()); testInstantiationService.stub(IConnectionManagementService, mockConnectionManagementService.object); testInstantiationService.stub(IContextKeyService, new MockContextKeyService()); diff --git a/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts b/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts index 3da07f667f..4540c42f8a 100644 --- a/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts +++ b/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts @@ -1583,7 +1583,7 @@ suite('SQL ConnectionManagementService tests', () => { const testInstantiationService = new TestInstantiationService(); testInstantiationService.stub(IStorageService, new TestStorageService()); testInstantiationService.stubCreateInstance(ConnectionStore, connectionStoreMock.object); - const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, undefined, new TestCapabilitiesService(), undefined, undefined, undefined, errorDiagnosticsService, undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); + const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, workspaceConfigurationServiceMock.object, new TestCapabilitiesService(), undefined, undefined, undefined, errorDiagnosticsService, undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); assert.strictEqual(profile.password, '', 'Profile should not have password initially'); assert.strictEqual(profile.options['password'], '', 'Profile options should not have password initially'); // Check for invalid profile id @@ -1613,7 +1613,7 @@ suite('SQL ConnectionManagementService tests', () => { testInstantiationService.stub(IStorageService, new TestStorageService()); testInstantiationService.stubCreateInstance(ConnectionStore, connectionStoreMock.object); - const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, undefined, new TestCapabilitiesService(), undefined, undefined, undefined, errorDiagnosticsService, undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); + const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, workspaceConfigurationServiceMock.object, new TestCapabilitiesService(), undefined, undefined, undefined, errorDiagnosticsService, undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); assert.strictEqual(profile.password, '', 'Profile should not have password initially'); assert.strictEqual(profile.options['password'], '', 'Profile options should not have password initially'); let credentials = await connectionManagementService.getConnectionCredentials(profile.id); @@ -1933,7 +1933,7 @@ suite('SQL ConnectionManagementService tests', () => { createInstanceStub.withArgs(ConnectionStore).returns(connectionStoreMock.object); createInstanceStub.withArgs(ConnectionStatusManager).returns(connectionStatusManagerMock.object); - const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, undefined, new TestCapabilitiesService(), undefined, undefined, undefined, errorDiagnosticsService, undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); + const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, workspaceConfigurationServiceMock.object, new TestCapabilitiesService(), undefined, undefined, undefined, errorDiagnosticsService, undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); // dupe connections have been seeded the numbers below already reflected the de-duped results @@ -1954,42 +1954,42 @@ suite('SQL ConnectionManagementService tests', () => { connections = connectionManagementService.getConnections(true); verifyConnections(connections, ['1', '2'], 'parameter is true'); }); -}); -test('isRecent should evaluate whether a profile was recently connected or not', () => { - const connectionStoreMock = TypeMoq.Mock.ofType(ConnectionStore, TypeMoq.MockBehavior.Loose, new TestStorageService()); - const testInstantiationService = new TestInstantiationService(); - testInstantiationService.stub(IStorageService, new TestStorageService()); - sinon.stub(testInstantiationService, 'createInstance').withArgs(ConnectionStore).returns(connectionStoreMock.object); - connectionStoreMock.setup(x => x.getRecentlyUsedConnections()).returns(() => { - return [createConnectionProfile('1')]; + test('isRecent should evaluate whether a profile was recently connected or not', () => { + const connectionStoreMock = TypeMoq.Mock.ofType(ConnectionStore, TypeMoq.MockBehavior.Loose, new TestStorageService()); + const testInstantiationService = new TestInstantiationService(); + testInstantiationService.stub(IStorageService, new TestStorageService()); + sinon.stub(testInstantiationService, 'createInstance').withArgs(ConnectionStore).returns(connectionStoreMock.object); + connectionStoreMock.setup(x => x.getRecentlyUsedConnections()).returns(() => { + return [createConnectionProfile('1')]; + }); + let profile1 = createConnectionProfile('1'); + let profile2 = createConnectionProfile('2'); + const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, workspaceConfigurationServiceMock.object, new TestCapabilitiesService(), undefined, undefined, undefined, new TestErrorDiagnosticsService(), undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); + assert(connectionManagementService.isRecent(profile1)); + assert(!connectionManagementService.isRecent(profile2)); }); - let profile1 = createConnectionProfile('1'); - let profile2 = createConnectionProfile('2'); - const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, undefined, new TestCapabilitiesService(), undefined, undefined, undefined, new TestErrorDiagnosticsService(), undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); - assert(connectionManagementService.isRecent(profile1)); - assert(!connectionManagementService.isRecent(profile2)); -}); -test('clearRecentConnection and ConnectionsList should call connectionStore functions', () => { - const connectionStoreMock = TypeMoq.Mock.ofType(ConnectionStore, TypeMoq.MockBehavior.Loose, new TestStorageService()); - let called = false; - connectionStoreMock.setup(x => x.clearRecentlyUsed()).returns(() => { - called = true; + test('clearRecentConnection and ConnectionsList should call connectionStore functions', () => { + const connectionStoreMock = TypeMoq.Mock.ofType(ConnectionStore, TypeMoq.MockBehavior.Loose, new TestStorageService()); + let called = false; + connectionStoreMock.setup(x => x.clearRecentlyUsed()).returns(() => { + called = true; + }); + connectionStoreMock.setup(x => x.removeRecentConnection(TypeMoq.It.isAny())).returns(() => { + called = true; + }); + const testInstantiationService = new TestInstantiationService(); + testInstantiationService.stub(IStorageService, new TestStorageService()); + sinon.stub(testInstantiationService, 'createInstance').withArgs(ConnectionStore).returns(connectionStoreMock.object); + let profile1 = createConnectionProfile('1'); + const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, workspaceConfigurationServiceMock.object, new TestCapabilitiesService(), undefined, undefined, undefined, new TestErrorDiagnosticsService(), undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); + connectionManagementService.clearRecentConnection(profile1); + assert(called); + called = false; + connectionManagementService.clearRecentConnectionsList(); + assert(called); }); - connectionStoreMock.setup(x => x.removeRecentConnection(TypeMoq.It.isAny())).returns(() => { - called = true; - }); - const testInstantiationService = new TestInstantiationService(); - testInstantiationService.stub(IStorageService, new TestStorageService()); - sinon.stub(testInstantiationService, 'createInstance').withArgs(ConnectionStore).returns(connectionStoreMock.object); - let profile1 = createConnectionProfile('1'); - const connectionManagementService = new ConnectionManagementService(undefined, testInstantiationService, undefined, undefined, undefined, new TestCapabilitiesService(), undefined, undefined, undefined, new TestErrorDiagnosticsService(), undefined, undefined, undefined, undefined, getBasicExtensionService(), undefined, undefined, undefined); - connectionManagementService.clearRecentConnection(profile1); - assert(called); - called = false; - connectionManagementService.clearRecentConnectionsList(); - assert(called); }); export function createConnectionProfile(id: string, password?: string): ConnectionProfile {