Validate MSAL library is enabled (#23000)

This commit is contained in:
Cheena Malhotra
2023-05-05 16:44:04 -07:00
committed by GitHub
parent 127a2d2e2f
commit 77c8b3bda1
5 changed files with 61 additions and 42 deletions

View File

@@ -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;
}

View File

@@ -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;
}

View File

@@ -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());

View File

@@ -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 {