Remove ADAL Code (#23360)

* initial commit, removed all adal code

* remove all authLibrary references in extension/azurecore

* removed authLibrary references from src/sql

* remove MSAL/ADAL setting option

* wip fixing tests and removing Msal from method names

* fixed tests

* create accountInfo mock

* fix tests

* fix clientApplication mock

* remove clientapplication

* fix compile

* add typing

* wip

* wip

* wip

* fix tree provider

* remove SimpleTokenCache, FileDatabase & tests

* remove remaining adal / authentication library references:

* remove comma from package.nls.json

* fix error fetching subscriptions

* fix tests

* remove getAzureAuthenticationLibraryConfig

* remove adal check

* fix build

* remove test

* undo remove customProviderSettings

* fix bracket
This commit is contained in:
Christopher Suh
2023-07-20 10:37:38 -07:00
committed by GitHub
parent a2b1c2cfc5
commit 91359a32c9
36 changed files with 168 additions and 1849 deletions

View File

@@ -661,13 +661,6 @@ declare module 'azdata' {
type?: ExtensionNodeType;
}
export interface AccountKey {
/**
* Auth Library used to add the account
*/
authLibrary?: string;
}
export namespace workspace {
/**
* Creates and enters a workspace at the specified location

View File

@@ -26,8 +26,6 @@ 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';

View File

@@ -143,6 +143,5 @@ export const enum NbTelemetryAction {
export const enum TelemetryPropertyName {
ChartMaxRowCountExceeded = 'chartMaxRowCountExceeded',
ConnectionSource = 'connectionSource',
AuthLibrary = 'AuthLibrary'
}

View File

@@ -47,7 +47,6 @@ import { Iterable } from 'vs/base/common/iterator';
import { LoadingSpinner } from 'sql/base/browser/ui/loadingSpinner/loadingSpinner';
import { Tenant, TenantListDelegate, TenantListRenderer } from 'sql/workbench/services/accountManagement/browser/tenantListRenderer';
import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces';
import { ADAL_AUTH_LIBRARY, AuthLibrary, getAuthLibrary } from 'sql/workbench/services/accountManagement/common/utils';
import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles';
export const VIEWLET_ID = 'workbench.view.accountpanel';
@@ -389,14 +388,9 @@ export class AccountDialog extends Modal {
this._splitView!.layout(DOM.getContentHeight(this._container!));
// Set the initial items of the list
const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService);
let updatedAccounts: azdata.Account[];
if (authLibrary) {
updatedAccounts = filterAccounts(newProvider.initialAccounts, authLibrary);
}
providerView.updateAccounts(updatedAccounts);
providerView.updateAccounts(newProvider.initialAccounts);
if ((updatedAccounts.length > 0 && this._splitViewContainer!.hidden) || this._providerViewsMap.size > 1) {
if ((newProvider.initialAccounts.length > 0 && this._splitViewContainer!.hidden) || this._providerViewsMap.size > 1) {
this.showSplitView();
}
@@ -440,12 +434,7 @@ export class AccountDialog extends Modal {
if (!providerMapping || !providerMapping.view) {
return;
}
const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService);
let updatedAccounts: azdata.Account[];
if (authLibrary) {
updatedAccounts = filterAccounts(args.accountList, authLibrary);
}
providerMapping.view.updateAccounts(updatedAccounts);
providerMapping.view.updateAccounts(args.accountList);
if ((args.accountList.length > 0 && this._splitViewContainer!.hidden) || this._providerViewsMap.size > 1) {
this.showSplitView();
@@ -518,18 +507,3 @@ export class AccountDialog extends Modal {
v.addAccountAction.run();
}
}
// Filter accounts based on currently selected Auth Library:
// if the account key is present, filter based on current auth library
// if there is no account key (pre-MSAL account), then it is an ADAL account and
// should be displayed as long as ADAL is the currently selected auth library
export function filterAccounts(accounts: azdata.Account[], authLibrary: AuthLibrary): azdata.Account[] {
let filteredAccounts = accounts.filter(account => {
if (account.key.authLibrary) {
return account.key.authLibrary === authLibrary;
} else {
return authLibrary === ADAL_AUTH_LIBRARY;
}
});
return filteredAccounts;
}

View File

@@ -25,11 +25,8 @@ import { ILogService } from 'vs/platform/log/common/log';
import { INotificationService, Severity, INotification } from 'vs/platform/notification/common/notification';
import { Action } from 'vs/base/common/actions';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { filterAccounts } from 'sql/workbench/services/accountManagement/browser/accountDialog';
import { ADAL_AUTH_LIBRARY, MSAL_AUTH_LIBRARY, AuthLibrary, AZURE_AUTH_LIBRARY_CONFIG, getAuthLibrary } from 'sql/workbench/services/accountManagement/common/utils';
import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry';
import { TelemetryAction, TelemetryError, TelemetryPropertyName, TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys';
import { TelemetryAction, TelemetryError, TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys';
export class AccountManagementService implements IAccountManagementService {
// CONSTANTS ///////////////////////////////////////////////////////////
@@ -39,7 +36,6 @@ export class AccountManagementService implements IAccountManagementService {
public _providers: { [id: string]: AccountProviderWithMetadata } = {};
public _serviceBrand: undefined;
private _accountStore: AccountStore;
private _authLibrary: AuthLibrary;
private _accountDialogController?: AccountDialogController;
private _autoOAuthDialogController?: AutoOAuthDialogController;
private _mementoContext?: Memento;
@@ -63,7 +59,6 @@ export class AccountManagementService implements IAccountManagementService {
@IOpenerService private _openerService: IOpenerService,
@ILogService private readonly _logService: ILogService,
@INotificationService private readonly _notificationService: INotificationService,
@IConfigurationService private _configurationService: IConfigurationService,
@IAdsTelemetryService private _telemetryService: IAdsTelemetryService
) {
this._mementoContext = new Memento(AccountManagementService.ACCOUNT_MEMENTO, this._storageService);
@@ -75,12 +70,7 @@ export class AccountManagementService implements IAccountManagementService {
this._removeAccountProviderEmitter = new Emitter<azdata.AccountProviderMetadata>();
this._updateAccountListEmitter = new Emitter<UpdateAccountListEventParams>();
// Determine authentication library in use, to support filtering accounts respectively.
// When this value is changed a restart is required so there isn't a need to dynamically update this value at runtime.
this._authLibrary = getAuthLibrary(this._configurationService);
_storageService.onWillSaveState(() => this.shutdown());
this.registerListeners();
}
private get autoOAuthDialogController(): AutoOAuthDialogController {
@@ -152,9 +142,6 @@ export class AccountManagementService implements IAccountManagementService {
} else {
this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.AddAzureAccountError, accountResult.errorCode,
this.getErrorType(accountResult.errorMessage))
.withAdditionalProperties({
[TelemetryPropertyName.AuthLibrary]: this._authLibrary
})
.send();
if (accountResult.errorCode && accountResult.errorMessage) {
throw new Error(localize('addAccountFailedCodeMessage', `{0} \nError Message: {1}`, accountResult.errorCode, accountResult.errorMessage));
@@ -168,9 +155,6 @@ export class AccountManagementService implements IAccountManagementService {
this._logService.error('Adding account failed, no result received.');
this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.AddAzureAccountErrorNoResult, '-1',
this.getErrorType())
.withAdditionalProperties({
[TelemetryPropertyName.AuthLibrary]: this._authLibrary
})
.send();
throw new Error(genericAccountErrorMessage);
}
@@ -182,9 +166,6 @@ export class AccountManagementService implements IAccountManagementService {
this.spliceModifiedAccount(provider, result.changedAccount);
}
this._telemetryService.createActionEvent(TelemetryView.LinkedAccounts, TelemetryAction.AddAzureAccount)
.withAdditionalProperties({
[TelemetryPropertyName.AuthLibrary]: this._authLibrary
})
.send();
this.fireAccountListUpdate(provider, result.accountAdded);
} finally {
@@ -235,9 +216,6 @@ export class AccountManagementService implements IAccountManagementService {
} else {
this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.RefreshAzureAccountError, refreshedAccount.errorCode,
this.getErrorType(refreshedAccount.errorMessage))
.withAdditionalProperties({
[TelemetryPropertyName.AuthLibrary]: this._authLibrary
})
.send();
if (refreshedAccount.errorCode && refreshedAccount.errorMessage) {
throw new Error(localize('refreshFailed', `{0} \nError Message: {1}`, refreshedAccount.errorCode, refreshedAccount.errorMessage));
@@ -254,9 +232,6 @@ export class AccountManagementService implements IAccountManagementService {
this._logService.error('Refreshing account failed, no result received.');
this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.RefreshAzureAccountErrorNoResult, '-1',
this.getErrorType())
.withAdditionalProperties({
[TelemetryPropertyName.AuthLibrary]: this._authLibrary
})
.send();
throw new Error(genericAccountErrorMessage);
}
@@ -281,9 +256,6 @@ export class AccountManagementService implements IAccountManagementService {
}
this._telemetryService.createActionEvent(TelemetryView.LinkedAccounts, TelemetryAction.RefreshAzureAccount)
.withAdditionalProperties({
[TelemetryPropertyName.AuthLibrary]: this._authLibrary
})
.send();
this.fireAccountListUpdate(provider, result.accountAdded);
return result.changedAccount!;
@@ -325,7 +297,6 @@ export class AccountManagementService implements IAccountManagementService {
// 3) Update our local cache of accounts
return this.doWithProvider(providerId, provider => {
return self._accountStore.getAccountsByProvider(provider.metadata.id)
.then(accounts => this._authLibrary ? filterAccounts(accounts, this._authLibrary) : accounts)
.then(accounts => {
self._providers[providerId].accounts = accounts;
return accounts;
@@ -337,8 +308,7 @@ export class AccountManagementService implements IAccountManagementService {
* Retrieves all the accounts registered with ADS based on auth library in use.
*/
public getAccounts(): Promise<azdata.Account[]> {
return this._accountStore.getAllAccounts()
.then(accounts => this._authLibrary ? filterAccounts(accounts, this._authLibrary) : accounts);
return this._accountStore.getAllAccounts();
}
/**
@@ -575,15 +545,10 @@ export class AccountManagementService implements IAccountManagementService {
});
}
const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService)
let updatedAccounts: azdata.Account[]
if (authLibrary) {
updatedAccounts = filterAccounts(provider.accounts, authLibrary);
}
// Step 2) Fire the event
let eventArg: UpdateAccountListEventParams = {
providerId: provider.metadata.id,
accountList: updatedAccounts ?? provider.accounts
accountList: provider.accounts
};
this._updateAccountListEmitter.fire(eventArg);
}
@@ -598,62 +563,6 @@ export class AccountManagementService implements IAccountManagementService {
}
}
private registerListeners(): void {
this.disposables.add(this._configurationService.onDidChangeConfiguration(async e => {
if (e.affectsConfiguration(AZURE_AUTH_LIBRARY_CONFIG)) {
const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService);
let accounts = await this._accountStore.getAllAccounts();
if (accounts) {
let updatedAccounts = await this.filterAndMergeAccounts(accounts, authLibrary);
let eventArg: UpdateAccountListEventParams;
if (updatedAccounts.length > 0) {
updatedAccounts.forEach(account => {
if (account.key.authLibrary === MSAL_AUTH_LIBRARY) {
account.isStale = false;
}
});
eventArg = {
providerId: updatedAccounts[0].key.providerId,
accountList: updatedAccounts
};
} else { // default to public cloud if no accounts
eventArg = {
providerId: 'azure_publicCloud',
accountList: updatedAccounts
};
}
this._updateAccountListEmitter.fire(eventArg);
}
}
}));
}
// Filters and merges accounts from both authentication libraries
private async filterAndMergeAccounts(accounts: azdata.Account[], currentAuthLibrary: AuthLibrary): Promise<azdata.Account[]> {
// Fetch accounts for alternate authenticationLibrary
const altLibrary = currentAuthLibrary === MSAL_AUTH_LIBRARY ? ADAL_AUTH_LIBRARY : MSAL_AUTH_LIBRARY;
const altLibraryAccounts = filterAccounts(accounts, altLibrary);
// Fetch accounts for current authenticationLibrary
const currentLibraryAccounts = filterAccounts(accounts, currentAuthLibrary);
// In the list of alternate accounts, check if the accounts are present in the current library cache,
// if not, add the account and mark it stale. The original account is marked as taken so its not picked again.
for (let account of altLibraryAccounts) {
await this.removeAccount(account.key);
if (this.findAccountIndex(currentLibraryAccounts, account) >= 0) {
continue;
} else {
// TODO: Refresh access token for the account if feasible.
account.isStale = true;
account.key.authLibrary = currentAuthLibrary;
currentLibraryAccounts.push(account);
await this.addAccountWithoutPrompt(account);
}
}
return currentLibraryAccounts;
}
public findAccountIndex(accounts: azdata.Account[], accountToFind: azdata.Account): number {
let indexToRemove: number = accounts.findIndex(account => {
// corner case handling for personal accounts
@@ -664,10 +573,6 @@ export class AccountManagementService implements IAccountManagementService {
if (accountToFind.key.accountId.includes('.')) {
return account.key.accountId === accountToFind!.key.accountId.split('.')[0];
}
// ADAL account added
if (account.key.accountId.includes('.')) {
return account.key.accountId.split('.')[0] === accountToFind!.key.accountId;
}
return account.key.accountId === accountToFind!.key.accountId;
});
return indexToRemove;

View File

@@ -1,18 +0,0 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the Source EULA. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
export const AZURE_AUTH_LIBRARY_CONFIG = 'azure.authenticationLibrary';
export type AuthLibrary = 'ADAL' | 'MSAL';
export const MSAL_AUTH_LIBRARY: AuthLibrary = 'MSAL';
export const ADAL_AUTH_LIBRARY: AuthLibrary = 'ADAL';
export const DEFAULT_AUTH_LIBRARY: AuthLibrary = MSAL_AUTH_LIBRARY;
export function getAuthLibrary(configurationService: IConfigurationService): AuthLibrary {
return configurationService.getValue(AZURE_AUTH_LIBRARY_CONFIG) || DEFAULT_AUTH_LIBRARY;
}

View File

@@ -18,7 +18,6 @@ import { EventVerifierSingle } from 'sql/base/test/common/event';
import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
import { AccountDialog } from 'sql/workbench/services/accountManagement/browser/accountDialog';
import { Emitter } from 'vs/base/common/event';
import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService';
import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService';
// SUITE CONSTANTS /////////////////////////////////////////////////////////
@@ -34,8 +33,7 @@ const noAccountProvider: azdata.AccountProviderMetadata = {
const account: azdata.Account = {
key: {
providerId: hasAccountProvider.id,
accountId: 'testAccount1',
authLibrary: 'MSAL'
accountId: 'testAccount1'
},
displayInfo: {
displayName: 'Test Account 1',
@@ -538,12 +536,11 @@ function getTestState(): AccountManagementState {
.returns(() => <any>mockAccountStore.object);
const testNotificationService = new TestNotificationService();
const testConfigurationService = new TestConfigurationService();
const mockTelemetryService = new NullAdsTelemetryService();
// Create the account management service
let ams = new AccountManagementService(mockInstantiationService.object, new TestStorageService(),
undefined, undefined, undefined, testNotificationService, testConfigurationService, mockTelemetryService);
undefined, undefined, undefined, testNotificationService, mockTelemetryService);
// Wire up event handlers
let evUpdate = new EventVerifierSingle<UpdateAccountListEventParams>();

View File

@@ -1062,9 +1062,7 @@ export class ConnectionManagementService extends Disposable implements IConnecti
const azureAccounts = accounts.filter(a => a.key.providerId.startsWith('azure'));
if (azureAccounts && azureAccounts.length > 0) {
let accountId = (connection.authenticationType === Constants.AuthenticationType.AzureMFA || connection.authenticationType === Constants.AuthenticationType.AzureMFAAndUser) ? connection.azureAccount : connection.userName;
// For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID
// The OR case can be removed once we no longer support ADAL
let account = azureAccounts.find(account => account.key.accountId === accountId || account.key.accountId.split('.')[0] === accountId);
let account = azureAccounts.find(account => account.key.accountId === accountId);
if (account) {
this._logService.debug(`Getting security token for Azure account ${account.key.accountId}`);
if (account.isStale) {

View File

@@ -35,11 +35,9 @@ import Severity from 'vs/base/common/severity';
import { ConnectionStringOptions } from 'sql/platform/capabilities/common/capabilitiesService';
import { isFalsyOrWhitespace } from 'vs/base/common/strings';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { filterAccounts } from 'sql/workbench/services/accountManagement/browser/accountDialog';
import { AuthenticationType, Actions, mssqlApplicationNameOption, applicationName, mssqlProviderName, mssqlCmsProviderName } from 'sql/platform/connection/common/constants';
import { AdsWidget } from 'sql/base/browser/ui/adsWidget';
import { createCSSRule } from 'vs/base/browser/dom';
import { AuthLibrary, getAuthLibrary } from 'sql/workbench/services/accountManagement/common/utils';
import { adjustForMssqlAppName } from 'sql/platform/connection/common/utils';
import { isMssqlAuthProviderEnabled } from 'sql/workbench/services/connection/browser/utils';
import { RequiredIndicatorClassName } from 'sql/base/browser/ui/label/label';
@@ -711,11 +709,7 @@ export class ConnectionWidget extends lifecycle.Disposable {
private async fillInAzureAccountOptions(): Promise<void> {
let oldSelection = this._azureAccountDropdown.value;
const accounts = await this._accountManagementService.getAccounts();
const updatedAccounts = accounts.filter(a => a.key.providerId.startsWith('azure'));
const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService);
if (authLibrary) {
this._azureAccountList = filterAccounts(updatedAccounts, authLibrary);
}
this._azureAccountList = accounts.filter(a => a.key.providerId.startsWith('azure'));
let accountDropdownOptions: SelectOptionItemSQL[] = this._azureAccountList.map(account => {
return {
@@ -734,10 +728,7 @@ export class ConnectionWidget extends lifecycle.Disposable {
}
private updateRefreshCredentialsLink(): void {
// For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID
// The OR case can be removed once we no longer support ADAL
let chosenAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value
|| account.key.accountId.split('.')[0] === this._azureAccountDropdown.value);
let chosenAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value);
if (chosenAccount && chosenAccount.isStale) {
this._tableContainer.classList.remove('hide-refresh-link');
} else {
@@ -758,10 +749,7 @@ export class ConnectionWidget extends lifecycle.Disposable {
await this.fillInAzureAccountOptions();
// If a new account was added find it and select it, otherwise select the first account
// For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID
// The OR case can be removed once we no longer support ADAL
let newAccount = this._azureAccountList.find(option => !oldAccountIds.some(oldId => oldId === option.key.accountId
|| oldId.split('.')[0] === option.key.accountId));
let newAccount = this._azureAccountList.find(option => !oldAccountIds.some(oldId => oldId === option.key.accountId));
if (newAccount) {
this._azureAccountDropdown.selectWithOptionName(newAccount.key.accountId);
} else {
@@ -773,10 +761,7 @@ export class ConnectionWidget extends lifecycle.Disposable {
// Display the tenant select box if needed
const hideTenantsClassName = 'hide-azure-tenants';
// For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID
// The OR case can be removed once we no longer support ADAL
let selectedAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value
|| account.key.accountId.split('.')[0] === this._azureAccountDropdown.value);
let selectedAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value);
if (!selectedAccount && selectFirstByDefault && this._azureAccountList.length > 0) {
selectedAccount = this._azureAccountList[0];
}
@@ -967,10 +952,7 @@ export class ConnectionWidget extends lifecycle.Disposable {
? connectionInfo.azureAccount : connectionInfo.userName;
let account: azdata.Account;
if (accountName) {
// For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID
// The OR case can be removed once we no longer support ADAL
account = this._azureAccountList?.find(account => account.key.accountId === this.getModelValue(accountName)
|| account.key.accountId.split('.')[0] === this.getModelValue(accountName));
account = this._azureAccountList?.find(account => account.key.accountId === this.getModelValue(accountName));
if (account) {
if (!account.properties.tenants?.find(tenant => tenant.id === this._azureTenantId)) {
this._azureTenantId = account.properties.tenants[0].id;

View File

@@ -4,8 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { azureAuthenticationLibraryConfig, enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/platform/connection/common/constants';
import { MSAL_AUTH_LIBRARY } from 'sql/workbench/services/accountManagement/common/utils';
import { enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/platform/connection/common/constants';
/**
* Reads setting 'mssql.enableSqlAuthenticationProvider' returns true if it's enabled.
@@ -15,16 +14,5 @@ import { MSAL_AUTH_LIBRARY } from 'sql/workbench/services/accountManagement/comm
* @returns True if provider is MSSQL and Sql Auth provider is enabled.
*/
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;
return provider === mssqlProviderName && (configService?.getValue(enableSqlAuthenticationProviderConfig) ?? true);
}