From ed8149599c243dda3454e65536c2fea9a5993474 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Fri, 28 Apr 2023 16:19:29 -0700 Subject: [PATCH] Update error for ignored tenants (#22881) --- extensions/azurecore/package.nls.json | 7 +-- .../src/account-provider/auths/azureAuth.ts | 46 +++++++++++---- .../account-provider/azureAccountProvider.ts | 59 +++++++++++-------- .../services/subscriptionService.ts | 3 +- .../src/azureResource/tree/accountTreeNode.ts | 9 ++- .../azureResource/tree/flatAccountTreeNode.ts | 8 ++- .../azurecore/src/azureResource/utils.ts | 31 ++++++---- extensions/azurecore/src/constants.ts | 2 + .../azurecore/src/utils/TenantIgnoredError.ts | 6 ++ 9 files changed, 112 insertions(+), 59 deletions(-) create mode 100644 extensions/azurecore/src/utils/TenantIgnoredError.ts diff --git a/extensions/azurecore/package.nls.json b/extensions/azurecore/package.nls.json index 1fcc595df4..a3e9b1949f 100644 --- a/extensions/azurecore/package.nls.json +++ b/extensions/azurecore/package.nls.json @@ -2,7 +2,6 @@ "azure.displayName": "Azure (Core)", "azure.description": "Browse and work with Azure resources", "azure.title": "Azure", - "azure.resource.config.title": "Azure Resource Configuration", "azure.resource.config.filter.description": "The resource filter, each element is an account id, a subscription id and name separated by a slash", "azure.resource.explorer.title": "Azure", @@ -14,12 +13,9 @@ "azure.resource.connectsqlserver.title": "Connect", "azure.resource.connectsqldb.title": "Add to Servers", "azure.resource.view.title": "Azure (Preview)", - "azure.tenant.config.filter.description": "The list of tenant IDs to ignore when querying azure resources. Each element is a tenant id.", - + "azure.tenant.config.filter.description": "The list of tenant IDs which will be skipped when querying Azure resources or requesting authentication tokens.", "accounts.clearTokenCache": "Clear Azure Account Token Cache", - "azure.openInAzurePortal.title": "Open in Azure Portal", - "config.azureAccountConfigurationSection": "Azure Account Configuration", "config.enablePublicCloudDescription": "Should Azure public cloud integration be enabled", "config.enableUsGovCloudDescription": "Should US Government Azure cloud (Fairfax) integration be enabled", @@ -33,5 +29,4 @@ "config.piiLogging": "Should Personally Identifiable Information (PII) be logged in the Azure Accounts output channel and the output channel log file.", "config.loggingLevel": "[Optional] The verbosity of logging for the Azure Accounts extension.", "config.authenticationLibrary": "The library used for the AAD auth flow. Please restart ADS after changing this option." - } diff --git a/extensions/azurecore/src/account-provider/auths/azureAuth.ts b/extensions/azurecore/src/account-provider/auths/azureAuth.ts index a9c4c270f5..f1aaa7c7ee 100644 --- a/extensions/azurecore/src/account-provider/auths/azureAuth.ts +++ b/extensions/azurecore/src/account-provider/auths/azureAuth.ts @@ -330,12 +330,6 @@ export abstract class AzureAuth implements vscode.Disposable { return null; } - // The user wants to ignore this tenant. - if (getTenantIgnoreList().includes(tenantId)) { - Logger.info(`Tenant ${tenantId} found in the ignore list, authentication will not be attempted.`); - return null; - } - // Resource endpoint must end with '/' to form a valid scope for MSAL token request. const endpoint = resource.endpoint.endsWith('/') ? resource.endpoint : resource.endpoint + '/'; @@ -509,6 +503,7 @@ export abstract class AzureAuth implements vscode.Disposable { tenantCategory: tenantInfo.tenantCategory } as Tenant; }); + Logger.verbose(`Tenants: ${tenantList}`); const homeTenantIndex = tenants.findIndex(tenant => tenant.tenantCategory === Constants.HomeCategory); // remove home tenant from list of tenants @@ -683,7 +678,7 @@ export abstract class AzureAuth implements vscode.Disposable { interface ConsentMessageItem extends vscode.MessageItem { booleanResult: boolean; - action?: (tenantId: string) => Promise; + action?: (tenantId: string) => Promise; } const openItem: ConsentMessageItem = { @@ -697,23 +692,50 @@ export abstract class AzureAuth implements vscode.Disposable { booleanResult: false }; + const cancelAndAuthenticate: ConsentMessageItem = { + title: localize('azurecore.consentDialog.authenticate', "Cancel and Authenticate"), + isCloseAffordance: true, + booleanResult: true + }; + const dontAskAgainItem: ConsentMessageItem = { title: localize('azurecore.consentDialog.ignore', "Ignore Tenant"), booleanResult: false, + action: async (tenantId: string) => { + return await confirmIgnoreTenantDialog(); + } + }; + + const confirmIgnoreTenantItem: ConsentMessageItem = { + title: localize('azurecore.confirmIgnoreTenantDialog.confirm', "Confirm"), + booleanResult: false, action: async (tenantId: string) => { tenantIgnoreList.push(tenantId); await updateTenantIgnoreList(tenantIgnoreList); + return false; } }; - const messageBody = localize('azurecore.consentDialog.body', "Your tenant '{0} ({1})' requires you to re-authenticate again to access {2} resources. Press Open to start the authentication process.", tenant.displayName, tenant.id, resource.endpoint); - const result = await vscode.window.showInformationMessage(messageBody, { modal: true }, openItem, closeItem, dontAskAgainItem); + const confirmIgnoreTenantDialog = async () => { + const confirmMessage = localize('azurecore.confirmIgnoreTenantDialog.body', "Azure Data Studio will no longer trigger authentication for this tenant {0} ({1}) and resources will not be accessible. \n\nTo allow access to resources for this tenant again, you will need to remove the tenant from the exclude list in the '{2}' setting.\n\nDo you wish to proceed?", tenant.displayName, tenant.id, Constants.AzureTenantConfigFilterSetting); + let confirmation = await vscode.window.showInformationMessage(confirmMessage, { modal: true }, cancelAndAuthenticate, confirmIgnoreTenantItem); - if (result?.action) { - await result.action(tenant.id); + if (confirmation?.action) { + await confirmation.action(tenant.id); + } + + return confirmation?.booleanResult || false; } - return result?.booleanResult || false; + const messageBody = localize('azurecore.consentDialog.body', "Your tenant {0} ({1}) requires you to re-authenticate again to access {2} resources. Press Open to start the authentication process.", tenant.displayName, tenant.id, resource.endpoint); + const result = await vscode.window.showInformationMessage(messageBody, { modal: true }, openItem, closeItem, dontAskAgainItem); + + let response = false; + if (result?.action) { + response = await result.action(tenant.id); + } + + return response; } //#endregion diff --git a/extensions/azurecore/src/account-provider/azureAccountProvider.ts b/extensions/azurecore/src/account-provider/azureAccountProvider.ts index 1bd82942a6..804afb890e 100644 --- a/extensions/azurecore/src/account-provider/azureAccountProvider.ts +++ b/extensions/azurecore/src/account-provider/azureAccountProvider.ts @@ -22,6 +22,8 @@ import { AzureDeviceCode } from './auths/azureDeviceCode'; import { filterAccounts } from '../azureResource/utils'; import * as Constants from '../constants'; import { MsalCachePluginProvider } from './utils/msalCachePlugin'; +import { getTenantIgnoreList } from '../utils'; +import { TenantIgnoredError } from '../utils/TenantIgnoredError'; const localize = nls.loadMessageBundle(); @@ -133,7 +135,6 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp return accounts; } - getSecurityToken(account: AzureAccount, resource: azdata.AzureResource): Thenable { return this._getSecurityToken(account, resource); } @@ -159,29 +160,35 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp Logger.info(`Could not fetch access token from cache: ${e}, fetching new access token instead.`); } tenantId = tenantId || account.properties.owningTenant.id; - let authResult = await azureAuth.getTokenMsal(account.key.accountId, resource, tenantId); - if (this.isAuthenticationResult(authResult) && authResult.account && authResult.account.idTokenClaims) { - const token: Token = { - key: authResult.account.homeAccountId, - token: authResult.accessToken, - tokenType: authResult.tokenType, - expiresOn: authResult.account.idTokenClaims.exp!, - tenantId: tenantId, - resource: resource - }; - try { - await this.msalCacheProvider.writeTokenToLocalCache(token); - } catch (e) { - Logger.error(`Could not save access token to local cache: ${e}, this might cause throttling of AAD requests.`); - } - return token; + if (getTenantIgnoreList().includes(tenantId)) { + // Tenant found in ignore list, don't fetch access token. + Logger.info(`Tenant ${tenantId} found in the ignore list, authentication will not be attempted. Please remove tenant from setting: '${Constants.AzureTenantConfigFilterSetting}' if you want to re-enable tenant for authentication.`); + throw new TenantIgnoredError(localize('tenantIgnoredError', 'Tenant found in ignore list, authentication not attempted. You can remove tenant {0} from ignore list in settings.json file: {1} if you wish to access resources from this tenant.', tenantId, Constants.AzureTenantConfigFilterSetting)); } else { - Logger.error(`MSAL: getToken call failed`); - // Throw error with MSAL-specific code/message, else throw generic error message - if (this.isProviderError(authResult)) { - throw new Error(localize('msalTokenError', `{0} occurred when acquiring token. \n{1}`, authResult.errorCode, authResult.errorMessage)); + let authResult = await azureAuth.getTokenMsal(account.key.accountId, resource, tenantId); + if (this.isAuthenticationResult(authResult) && authResult.account && authResult.account.idTokenClaims) { + const token: Token = { + key: authResult.account.homeAccountId, + token: authResult.accessToken, + tokenType: authResult.tokenType, + expiresOn: authResult.account.idTokenClaims.exp!, + tenantId: tenantId, + resource: resource + }; + try { + await this.msalCacheProvider.writeTokenToLocalCache(token); + } catch (e) { + Logger.error(`Could not save access token to local cache: ${e}, this might cause throttling of AAD requests.`); + } + return token; } else { - throw new Error(localize('genericTokenError', 'Failed to get token')); + Logger.error(`MSAL: getToken call failed`); + // Throw error with MSAL-specific code/message, else throw generic error message + if (this.isProviderError(authResult)) { + throw new Error(localize('msalTokenError', `{0} occurred when acquiring token. \n{1}`, authResult.errorCode, authResult.errorMessage)); + } else { + throw new Error(localize('genericTokenError', 'Failed to get token')); + } } } } else { // fallback to ADAL as default @@ -226,7 +233,13 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp const azureAccount = account as AzureAccount; const response: MultiTenantTokenResponse = {}; for (const tenant of azureAccount.properties.tenants) { - response[tenant.id] = await this._getAccountSecurityToken(account, tenant.id, resource); + try { + response[tenant.id] = await this._getAccountSecurityToken(account, tenant.id, resource); + } catch (e) { + if (!(e instanceof TenantIgnoredError)) { + throw e; + } + } } return response; diff --git a/extensions/azurecore/src/azureResource/services/subscriptionService.ts b/extensions/azurecore/src/azureResource/services/subscriptionService.ts index 6ceda71739..37bbf3da68 100644 --- a/extensions/azurecore/src/azureResource/services/subscriptionService.ts +++ b/extensions/azurecore/src/azureResource/services/subscriptionService.ts @@ -14,6 +14,7 @@ import { AzureResourceErrorMessageUtil } from '../utils'; import { Logger } from '../../utils/Logger'; import * as nls from 'vscode-nls'; +import { TenantIgnoredError } from '../../utils/TenantIgnoredError'; const localize = nls.loadMessageBundle(); export class AzureResourceSubscriptionService implements IAzureResourceSubscriptionService { @@ -50,7 +51,7 @@ export class AzureResourceSubscriptionService implements IAzureResourceSubscript void vscode.window.showWarningMessage(errorMsg); } } catch (error) { - if (!account.isStale) { + if (!account.isStale && !(error instanceof TenantIgnoredError)) { const errorMsg = localize('azure.resource.tenantSubscriptionsError', "Failed to get subscriptions for account {0} (tenant '{1}'). {2}", account.displayInfo.displayName, tenantId, AzureResourceErrorMessageUtil.getErrorMessage(error)); Logger.error(`Failed to get subscriptions for account ${account.displayInfo.displayName} (tenant '${tenantId}'). ${AzureResourceErrorMessageUtil.getErrorMessage(error)}`); errors.push(error); diff --git a/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts b/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts index 251e7b3856..272808473f 100644 --- a/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts +++ b/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts @@ -20,6 +20,7 @@ import { AzureResourceErrorMessageUtil } from '../utils'; import { IAzureResourceTreeChangeHandler } from './treeChangeHandler'; import { IAzureResourceSubscriptionService, IAzureResourceSubscriptionFilterService } from '../../azureResource/interfaces'; import { AzureAccount, azureResource } from 'azurecore'; +import { TenantIgnoredError } from '../../utils/TenantIgnoredError'; export class AzureResourceAccountTreeNode extends AzureResourceContainerTreeNodeBase { public constructor( @@ -75,10 +76,14 @@ export class AzureResourceAccountTreeNode extends AzureResourceContainerTreeNode try { token = await azdata.accounts.getAccountSecurityToken(this.account, s.tenant!, azdata.AzureResource.ResourceManagement); } catch (err) { - errMsg = AzureResourceErrorMessageUtil.getErrorMessage(err); + if (!(err instanceof TenantIgnoredError)) { + errMsg = AzureResourceErrorMessageUtil.getErrorMessage(err); + } } if (!token) { - void vscode.window.showWarningMessage(localize('azure.unableToAccessSubscription', "Unable to access subscription {0} ({1}). Please [refresh the account](command:azure.resource.signin) to try again. {2}", s.name, s.id, errMsg)); + if (errMsg !== '') { + void vscode.window.showWarningMessage(localize('azure.unableToAccessSubscription', "Unable to access subscription {0} ({1}). Please [refresh the account](command:azure.resource.signin) to try again. {2}", s.name, s.id, errMsg)); + } return false; } return true; diff --git a/extensions/azurecore/src/azureResource/tree/flatAccountTreeNode.ts b/extensions/azurecore/src/azureResource/tree/flatAccountTreeNode.ts index 8d28bd158d..43fd3fb128 100644 --- a/extensions/azurecore/src/azureResource/tree/flatAccountTreeNode.ts +++ b/extensions/azurecore/src/azureResource/tree/flatAccountTreeNode.ts @@ -187,8 +187,12 @@ class FlatAccountTreeNodeLoader { let tenants = this._account.properties.tenants; // Filter out tenants that we can't authenticate to. tenants = tenants.filter(async tenant => { - const token = await azdata.accounts.getAccountSecurityToken(this._account, tenant.id, azdata.AzureResource.ResourceManagement); - return token !== undefined; + try { + const token = await azdata.accounts.getAccountSecurityToken(this._account, tenant.id, azdata.AzureResource.ResourceManagement); + return token !== undefined; + } catch (e) { + return false; + } }); let subscriptions: azureResource.AzureResourceSubscription[] = (await getSubscriptionInfo(this._account, this._subscriptionService, this._subscriptionFilterService)).subscriptions; diff --git a/extensions/azurecore/src/azureResource/utils.ts b/extensions/azurecore/src/azureResource/utils.ts index 327f5bcc0b..61ef3de4b0 100644 --- a/extensions/azurecore/src/azureResource/utils.ts +++ b/extensions/azurecore/src/azureResource/utils.ts @@ -21,6 +21,7 @@ import { getProxyEnabledHttpClient } from '../utils'; import { HttpClient } from '../account-provider/auths/httpClient'; import { NetworkRequestOptions } from '@azure/msal-common'; import { ErrorResponseBody } from '@azure/arm-subscriptions/esm/models'; +import { TenantIgnoredError } from '../utils/TenantIgnoredError'; const localize = nls.loadMessageBundle(); @@ -178,18 +179,20 @@ export async function getResourceGroups(appContext: AppContext, account?: AzureA result.resourceGroups.push(...await service.getResources([subscription], new TokenCredentials(token, tokenType), account)); } catch (err) { - const error = new Error(localize('azure.accounts.getResourceGroups.queryError', "Error fetching resource groups for account {0} ({1}) subscription {2} ({3}) tenant {4} : {5}", - account.displayInfo.displayName, - account.displayInfo.userId, - subscription.id, - subscription.name, - tenant.id, - err instanceof Error ? err.message : err)); - console.warn(error); - if (!ignoreErrors) { - throw error; + if (!(err instanceof TenantIgnoredError)) { + const error = new Error(localize('azure.accounts.getResourceGroups.queryError', "Error fetching resource groups for account {0} ({1}) subscription {2} ({3}) tenant {4} : {5}", + account.displayInfo.displayName, + account.displayInfo.userId, + subscription.id, + subscription.name, + tenant.id, + err instanceof Error ? err.message : err)); + console.warn(error); + if (!ignoreErrors) { + throw error; + } + result.errors.push(error); } - result.errors.push(error); } })); return result; @@ -282,8 +285,10 @@ export async function runResourceQuery