diff --git a/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts b/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts index c07bb9a2ac..406eb0cc08 100644 --- a/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts +++ b/extensions/azurecore/src/azureResource/tree/accountTreeNode.ts @@ -68,14 +68,21 @@ export class AzureResourceAccountTreeNode extends AzureResourceContainerTreeNode return [AzureResourceMessageTreeNode.create(AzureResourceAccountTreeNode.noSubscriptionsLabel, this)]; } else { // Filter out everything that we can't authenticate to. - subscriptions = subscriptions.filter(async s => { - const token = await azdata.accounts.getAccountSecurityToken(this.account, s.tenant, azdata.AzureResource.ResourceManagement); + const hasTokenResults = await Promise.all(subscriptions.map(async s => { + let token: azdata.accounts.AccountSecurityToken | undefined = undefined; + let errMsg = ''; + try { + token = await azdata.accounts.getAccountSecurityToken(this.account, s.tenant, azdata.AzureResource.ResourceManagement); + } catch (err) { + errMsg = AzureResourceErrorMessageUtil.getErrorMessage(err); + } if (!token) { - console.info(`Account does not have permissions to view subscription ${JSON.stringify(s)}.`); + 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; - }); + })); + subscriptions = subscriptions.filter((_s, i) => hasTokenResults[i]); let subTreeNodes = await Promise.all(subscriptions.map(async (subscription) => { return new AzureResourceSubscriptionTreeNode(this.account, subscription, subscription.tenant, this.appContext, this.treeChangeHandler, this); diff --git a/extensions/azurecore/src/azureResource/utils.ts b/extensions/azurecore/src/azureResource/utils.ts index 715e20e642..0ffd03a69b 100644 --- a/extensions/azurecore/src/azureResource/utils.ts +++ b/extensions/azurecore/src/azureResource/utils.ts @@ -353,7 +353,7 @@ export async function makeHttpRequest(account: azdata.Account, subscription: azu return result; } - let securityToken: { token: string, tokenType?: string }; + let securityToken: azdata.accounts.AccountSecurityToken; try { securityToken = await azdata.accounts.getAccountSecurityToken( account, diff --git a/extensions/azurecore/src/test/azureResource/tree/accountTreeNode.test.ts b/extensions/azurecore/src/test/azureResource/tree/accountTreeNode.test.ts index c1560bfe33..20a14f81d4 100644 --- a/extensions/azurecore/src/test/azureResource/tree/accountTreeNode.test.ts +++ b/extensions/azurecore/src/test/azureResource/tree/accountTreeNode.test.ts @@ -131,6 +131,7 @@ describe('AzureResourceAccountTreeNode.info', function (): void { it('Should be correct when there are subscriptions listed.', async function (): Promise { mockSubscriptionService.setup((o) => o.getSubscriptions(mockAccount, TypeMoq.It.isAny())).returns(() => Promise.resolve(mockSubscriptions)); mockSubscriptionFilterService.setup((o) => o.getSelectedSubscriptions(mockAccount)).returns(() => Promise.resolve(undefined)); + sinon.stub(azdata.accounts, 'getAccountSecurityToken').resolves(mockToken); const accountTreeNodeLabel = `${mockAccount.displayInfo.displayName} (${mockSubscriptions.length} / ${mockSubscriptions.length} subscriptions)`; @@ -148,10 +149,30 @@ describe('AzureResourceAccountTreeNode.info', function (): void { should(nodeInfo.label).equal(accountTreeNodeLabel); }); - it('Should be correct when there are subscriptions filtered.', async function (): Promise { - mockSubscriptionService.setup((o) => o.getSubscriptions(mockAccount, TypeMoq.It.isAny())).returns(() => Promise.resolve(mockSubscriptions)); + it('Should only show subscriptions with valid tokens.', async function (): Promise { + mockSubscriptionService.setup((o) => o.getSubscriptions(mockAccount)).returns(() => Promise.resolve(mockSubscriptions)); mockSubscriptionFilterService.setup((o) => o.getSelectedSubscriptions(mockAccount)).returns(() => Promise.resolve(mockFilteredSubscriptions)); + sinon.stub(azdata.accounts, 'getAccountSecurityToken').onFirstCall().resolves(mockToken); + const accountTreeNodeLabel = `${mockAccount.displayInfo.displayName} (${mockFilteredSubscriptions.length} / ${mockSubscriptions.length} subscriptions)`; + const accountTreeNode = new AzureResourceAccountTreeNode(mockAccount, mockAppContext, mockTreeChangeHandler.object); + + const subscriptionNodes = await accountTreeNode.getChildren(); + + should(subscriptionNodes).Array(); + should(subscriptionNodes.length).equal(1); + + const treeItem = await accountTreeNode.getTreeItem(); + should(treeItem.label).equal(accountTreeNodeLabel); + + const nodeInfo = accountTreeNode.getNodeInfo(); + should(nodeInfo.label).equal(accountTreeNodeLabel); + }); + + it('Should be correct when there are subscriptions filtered.', async function (): Promise { + mockSubscriptionService.setup((o) => o.getSubscriptions(mockAccount)).returns(() => Promise.resolve(mockSubscriptions)); + mockSubscriptionFilterService.setup((o) => o.getSelectedSubscriptions(mockAccount)).returns(() => Promise.resolve(mockFilteredSubscriptions)); + sinon.stub(azdata.accounts, 'getAccountSecurityToken').resolves(mockToken); const accountTreeNodeLabel = `${mockAccount.displayInfo.displayName} (${mockFilteredSubscriptions.length} / ${mockSubscriptions.length} subscriptions)`; const accountTreeNode = new AzureResourceAccountTreeNode(mockAccount, mockAppContext, mockTreeChangeHandler.object); diff --git a/extensions/machine-learning/src/common/apiWrapper.ts b/extensions/machine-learning/src/common/apiWrapper.ts index 4881624737..b28ffebdca 100644 --- a/extensions/machine-learning/src/common/apiWrapper.ts +++ b/extensions/machine-learning/src/common/apiWrapper.ts @@ -99,7 +99,7 @@ export class ApiWrapper { return azdata.accounts.getAllAccounts(); } - public getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Thenable<{ token: string, tokenType?: string } | undefined> { + public getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Thenable { return azdata.accounts.getAccountSecurityToken(account, tenant, resource); } diff --git a/extensions/machine-learning/src/modelManagement/azureModelRegistryService.ts b/extensions/machine-learning/src/modelManagement/azureModelRegistryService.ts index c3ad9614aa..fe6dff9322 100644 --- a/extensions/machine-learning/src/modelManagement/azureModelRegistryService.ts +++ b/extensions/machine-learning/src/modelManagement/azureModelRegistryService.ts @@ -304,7 +304,7 @@ export class AzureModelRegistryService { if (this._amlClient) { return this._amlClient; } else { - const tokens: { token: string, tokenType?: string } | undefined = await this._apiWrapper.getAccountSecurityToken(account, tenant.id, azdata.AzureResource.ResourceManagement); + const tokens: azdata.accounts.AccountSecurityToken | undefined = await this._apiWrapper.getAccountSecurityToken(account, tenant.id, azdata.AzureResource.ResourceManagement); let token: string = ''; let tokenType: string | undefined = undefined; if (tokens) { diff --git a/src/sql/azdata.d.ts b/src/sql/azdata.d.ts index acc5487379..2fc0e23afb 100644 --- a/src/sql/azdata.d.ts +++ b/src/sql/azdata.d.ts @@ -2175,6 +2175,20 @@ declare module 'azdata' { */ export function getSecurityToken(account: Account, resource?: AzureResource): Thenable<{ [key: string]: any }>; + /** + * The token used to authenticate an account. + */ + export interface AccountSecurityToken { + /** + * The token to use + */ + token: string, + /** + * What type of token this is (such as Bearer) + */ + tokenType?: string | undefined + } + /** * Generates a security token by asking the account's provider * @param account The account to retrieve the security token for @@ -2182,7 +2196,7 @@ declare module 'azdata' { * @param resource Type of resource to get the security token for (defaults to * AzureResource.ResourceManagement if not given) */ - export function getAccountSecurityToken(account: Account, tenantId: string, resource: AzureResource): Thenable<{ token: string, tokenType?: string | undefined } | undefined>; + export function getAccountSecurityToken(account: Account, tenantId: string, resource: AzureResource): Thenable; /** * An [event](#Event) which fires when the accounts have changed. diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index 6ce02c7801..245747381c 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -836,7 +836,7 @@ declare module 'azdata' { * @param resource The resource to get the token for * @return Promise to return a security token object */ - getAccountSecurityToken(account: Account, tenant: string, resource: AzureResource): Thenable<{ token: string } | undefined>; + getAccountSecurityToken(account: Account, tenant: string, resource: AzureResource): Thenable; } export interface AccountKey { diff --git a/src/sql/platform/accounts/common/interfaces.ts b/src/sql/platform/accounts/common/interfaces.ts index 0d833a95d8..301abcc740 100644 --- a/src/sql/platform/accounts/common/interfaces.ts +++ b/src/sql/platform/accounts/common/interfaces.ts @@ -25,7 +25,7 @@ export interface IAccountManagementService { * @deprecated */ getSecurityToken(account: azdata.Account, resource: azdata.AzureResource): Promise<{ [key: string]: { token: string } } | undefined>; - getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Promise<{ token: string } | undefined>; + getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Promise; removeAccount(accountKey: azdata.AccountKey): Promise; removeAccounts(): Promise; refreshAccount(account: azdata.Account): Promise; diff --git a/src/sql/platform/accounts/test/common/testAccountManagementService.ts b/src/sql/platform/accounts/test/common/testAccountManagementService.ts index 2a300f95c9..263cb106ed 100644 --- a/src/sql/platform/accounts/test/common/testAccountManagementService.ts +++ b/src/sql/platform/accounts/test/common/testAccountManagementService.ts @@ -55,7 +55,7 @@ export class TestAccountManagementService implements IAccountManagementService { return Promise.resolve([]); } - getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Promise<{ token: string }> { + getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Promise { return Promise.resolve(undefined!); } diff --git a/src/sql/workbench/api/browser/mainThreadAccountManagement.ts b/src/sql/workbench/api/browser/mainThreadAccountManagement.ts index 16d0b5ccd3..5de5606715 100644 --- a/src/sql/workbench/api/browser/mainThreadAccountManagement.ts +++ b/src/sql/workbench/api/browser/mainThreadAccountManagement.ts @@ -78,7 +78,7 @@ export class MainThreadAccountManagement extends Disposable implements MainThrea getSecurityToken(account: azdata.Account, resource: azdata.AzureResource): Thenable<{}> { return self._proxy.$getSecurityToken(account, resource); }, - getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Thenable<{ token: string }> { + getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Thenable { return self._proxy.$getAccountSecurityToken(account, tenant, resource); }, initialize(restoredAccounts: azdata.Account[]): Thenable { diff --git a/src/sql/workbench/api/common/extHostAccountManagement.ts b/src/sql/workbench/api/common/extHostAccountManagement.ts index 20984ac367..31681ff4cd 100644 --- a/src/sql/workbench/api/common/extHostAccountManagement.ts +++ b/src/sql/workbench/api/common/extHostAccountManagement.ts @@ -104,7 +104,7 @@ export class ExtHostAccountManagement extends ExtHostAccountManagementShape { }); } - public override $getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource = AzureResource.ResourceManagement): Thenable<{ token: string }> { + public override $getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource = AzureResource.ResourceManagement): Thenable { return this.getAllProvidersAndAccounts().then(providerAndAccounts => { const providerAndAccount = providerAndAccounts.find(providerAndAccount => providerAndAccount.account.key.accountId === account.key.accountId); if (providerAndAccount) { diff --git a/src/sql/workbench/api/common/sqlExtHost.api.impl.ts b/src/sql/workbench/api/common/sqlExtHost.api.impl.ts index 8dae62fdad..ce32a09f3c 100644 --- a/src/sql/workbench/api/common/sqlExtHost.api.impl.ts +++ b/src/sql/workbench/api/common/sqlExtHost.api.impl.ts @@ -165,7 +165,7 @@ export function createAdsApiFactory(accessor: ServicesAccessor): IAdsExtensionAp getSecurityToken(account: azdata.Account, resource?: azdata.AzureResource): Thenable<{}> { return extHostAccountManagement.$getSecurityToken(account, resource); }, - getAccountSecurityToken(account: azdata.Account, tenant: string, resource?: azdata.AzureResource): Thenable<{ token: string }> { + getAccountSecurityToken(account: azdata.Account, tenant: string, resource?: azdata.AzureResource): Thenable { return extHostAccountManagement.$getAccountSecurityToken(account, tenant, resource); }, onDidChangeAccounts(listener: (e: azdata.DidChangeAccountsParams) => void, thisArgs?: any, disposables?: extHostTypes.Disposable[]) { diff --git a/src/sql/workbench/api/common/sqlExtHost.protocol.ts b/src/sql/workbench/api/common/sqlExtHost.protocol.ts index f00376a43c..8e36424923 100644 --- a/src/sql/workbench/api/common/sqlExtHost.protocol.ts +++ b/src/sql/workbench/api/common/sqlExtHost.protocol.ts @@ -32,7 +32,7 @@ export abstract class ExtHostAccountManagementShape { $autoOAuthCancelled(handle: number): Thenable { throw ni(); } $clear(handle: number, accountKey: azdata.AccountKey): Thenable { throw ni(); } $getSecurityToken(account: azdata.Account, resource?: azdata.AzureResource): Thenable<{}> { throw ni(); } - $getAccountSecurityToken(account: azdata.Account, tenant: string, resource?: azdata.AzureResource): Thenable<{ token: string }> { throw ni(); } + $getAccountSecurityToken(account: azdata.Account, tenant: string, resource?: azdata.AzureResource): Thenable { throw ni(); } $initialize(handle: number, restoredAccounts: azdata.Account[]): Thenable { throw ni(); } $prompt(handle: number): Thenable { throw ni(); } $refresh(handle: number, account: azdata.Account): Thenable { throw ni(); } diff --git a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts index c46ba834aa..f204868be5 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts @@ -246,7 +246,7 @@ export class AccountManagementService implements IAccountManagementService { * @param resource The resource to get the security token for * @return Promise to return the security token */ - public getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Promise<{ token: string } | undefined> { + public getAccountSecurityToken(account: azdata.Account, tenant: string, resource: azdata.AzureResource): Promise { return this.doWithProvider(account.key.providerId, provider => { return Promise.resolve(provider.provider.getAccountSecurityToken(account, tenant, resource)); });