Fix tenant Id design for Azure accounts (#21086)

extensions/azurecore/src/account-provider/auths/azureAuth.ts

@@ -133,12 +133,6 @@ export abstract class AzureAuth implements vscode.Disposable {
 		}
 	}
 
-	private getHomeTenant(account: AzureAccount): Tenant {
-		// Home is defined by the API
-		// Lets pick the home tenant - and fall back to commonTenant if they don't exist
-		return account.properties.tenants.find(t => t.tenantCategory === 'Home') ?? account.properties.tenants[0] ?? this.commonTenant;
-	}
-
 	public async refreshAccess(account: AzureAccount): Promise<AzureAccount> {
 		// Deprecated account - delete it.
 		if (account.key.accountVersion !== AzureAuth.ACCOUNT_VERSION) {
@@ -146,7 +140,10 @@ export abstract class AzureAuth implements vscode.Disposable {
 			return account;
 		}
 		try {
-			const tenant = this.getHomeTenant(account);
+			// There can be multiple home tenants
+			// We want to return the one that owns the Azure account.
+			// Not doing so can result in token being issued for the wrong tenant
+			const tenant = account.properties.owningTenant;
 			const tokenResult = await this.getAccountSecurityToken(account, tenant.id, azdata.AzureResource.MicrosoftResourceManagement);
 			if (!tokenResult) {
 				account.isStale = true;
@@ -186,7 +183,14 @@ export abstract class AzureAuth implements vscode.Disposable {
 			return undefined;
 		}
 
-		const tenant = account.properties.tenants.find(t => t.id === tenantId);
+		if (!account.properties.owningTenant) {
+			// Should never happen
+			throw new AzureAuthError(localize('azure.owningTenantNotFound', "Owning Tenant information not found for account."), 'Owning tenant not found.', undefined);
+		}
+
+		const tenant = account.properties.owningTenant?.id === tenantId
+			? account.properties.owningTenant
+			: account.properties.tenants.find(t => t.id === tenantId);
 
 		if (!tenant) {
 			throw new AzureAuthError(localize('azure.tenantNotFound', "Specified tenant with ID '{0}' not found.", tenantId), `Tenant ${tenantId} not found.`, undefined);
@@ -557,6 +561,10 @@ export abstract class AzureAuth implements vscode.Disposable {
 		const name = tokenClaims.name ?? tokenClaims.email ?? tokenClaims.unique_name;
 		const email = tokenClaims.email ?? tokenClaims.unique_name;
 
+		// Read more about tid > https://learn.microsoft.com/azure/active-directory/develop/id-tokens
+		const owningTenant = tenants.find(t => t.id === tokenClaims.tid)
+			?? { 'id': tokenClaims.tid, 'displayName': 'Microsoft Account' };
+
 		let displayName = name;
 		if (email) {
 			displayName = `${displayName} - ${email}`;
@@ -596,6 +604,7 @@ export abstract class AzureAuth implements vscode.Disposable {
 			properties: {
 				providerSettings: this.metadata,
 				isMsAccount: accountIssuer === 'msft',
+				owningTenant: owningTenant,
 				tenants,
 				azureAuthType: this.authType
 			},

extensions/azurecore/src/azurecore.d.ts

@@ -38,11 +38,17 @@ declare module 'azurecore' {
 		azureAuthType?: AzureAuthType
 
 		providerSettings: AzureAccountProviderMetadata;
+
 		/**
 		 * Whether or not the account is a Microsoft account
 		 */
 		isMsAccount: boolean;
 
+		/**
+		 * Represents the tenant that the user would be signing in to. For work and school accounts, the GUID is the immutable tenant ID of the organization that the user is signing in to.
+		 * For sign-ins to the personal Microsoft account tenant (services like Xbox, Teams for Life, or Outlook), the value is 9188040d-6c67-4c5b-b112-36a304b66dad.
+		 */
+		owningTenant: Tenant;
 		/**
 		 * A list of tenants (aka directories) that the account belongs to
 		 */

extensions/azurecore/src/test/account-provider/auths/azureAuth.test.ts

@@ -53,6 +53,7 @@ describe('Azure Authentication', function () {
 		mockAccount = {
 			isStale: false,
 			properties: {
+				owningTenant: mockTenant,
 				tenants: [mockTenant]
 			}
 		} as AzureAccount;

extensions/azurecore/src/test/azureResource/providers/database/databaseTreeDataProvider.test.ts

@@ -35,6 +35,10 @@ const mockAccount: AzureAccount = {
 	properties: {
 		providerSettings: settings[0].metadata,
 		isMsAccount: true,
+		owningTenant: {
+			id: 'tenantId',
+			displayName: 'tenantDisplayName',
+		},
 		tenants: []
 	},
 	isStale: false

extensions/azurecore/src/test/azureResource/providers/databaseServer/databaseServerTreeDataProvider.test.ts

@@ -35,6 +35,10 @@ const mockAccount: AzureAccount = {
 	properties: {
 		providerSettings: settings[0].metadata,
 		isMsAccount: true,
+		owningTenant: {
+			id: 'tenantId',
+			displayName: 'tenantDisplayName',
+		},
 		tenants: []
 	},
 	isStale: false

extensions/azurecore/src/test/azureResource/resourceService.test.ts

@@ -28,13 +28,17 @@ const mockAccount: AzureAccount = {
 	properties: {
 		providerSettings: settings[0].metadata,
 		isMsAccount: true,
+		owningTenant: {
+			id: 'tenantId',
+			displayName: 'tenantDisplayName',
+		},
 		tenants: []
 	},
 	isStale: false
 };
 
 const mockTenantId: string = 'mock_tenant';
-const mockSubscriptionId ='mock_subscription';
+const mockSubscriptionId = 'mock_subscription';
 
 const mockSubscription: azureResource.AzureResourceSubscription = {
 	id: mockSubscriptionId,
@@ -50,7 +54,7 @@ let mockResourceProvider2: TypeMoq.IMock<azureResource.IAzureResourceProvider>;
 
 let resourceService: AzureResourceService;
 
-describe('AzureResourceService.listResourceProviderIds', function(): void {
+describe('AzureResourceService.listResourceProviderIds', function (): void {
 	beforeEach(() => {
 		mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType<azureResource.IAzureResourceTreeDataProvider>();
 		mockResourceTreeDataProvider1.setup((o) => o.getRootChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType<azdata.TreeItem>().object]));
@@ -71,7 +75,7 @@ describe('AzureResourceService.listResourceProviderIds', function(): void {
 		resourceService.areResourceProvidersLoaded = true;
 	});
 
-	it('Should be correct when registering providers.', async function(): Promise<void> {
+	it('Should be correct when registering providers.', async function (): Promise<void> {
 		resourceService.registerResourceProvider(mockResourceProvider1.object);
 		let providerIds = await resourceService.listResourceProviderIds();
 		should(providerIds).Array();
@@ -87,7 +91,7 @@ describe('AzureResourceService.listResourceProviderIds', function(): void {
 	});
 });
 
-describe('AzureResourceService.getRootChildren', function(): void {
+describe('AzureResourceService.getRootChildren', function (): void {
 	beforeEach(() => {
 		mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType<azureResource.IAzureResourceTreeDataProvider>();
 		mockResourceTreeDataProvider1.setup((o) => o.getRootChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType<azdata.TreeItem>().object]));
@@ -101,13 +105,13 @@ describe('AzureResourceService.getRootChildren', function(): void {
 		resourceService.areResourceProvidersLoaded = true;
 	});
 
-	it('Should be correct when provider id is correct.', async function(): Promise<void> {
+	it('Should be correct when provider id is correct.', async function (): Promise<void> {
 		const children = await resourceService.getRootChildren(mockResourceProvider1.object.providerId, mockAccount, mockSubscription, mockTenantId);
 
 		should(children).Array();
 	});
 
-	it('Should throw exceptions when provider id is incorrect.', async function(): Promise<void> {
+	it('Should throw exceptions when provider id is incorrect.', async function (): Promise<void> {
 		const providerId = 'non_existent_provider_id';
 		try {
 			await resourceService.getRootChildren(providerId, mockAccount, mockSubscription, mockTenantId);
@@ -120,7 +124,7 @@ describe('AzureResourceService.getRootChildren', function(): void {
 	});
 });
 
-describe('AzureResourceService.getChildren', function(): void {
+describe('AzureResourceService.getChildren', function (): void {
 	beforeEach(() => {
 		mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType<azureResource.IAzureResourceTreeDataProvider>();
 		mockResourceTreeDataProvider1.setup((o) => o.getRootChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType<azdata.TreeItem>().object]));
@@ -135,12 +139,12 @@ describe('AzureResourceService.getChildren', function(): void {
 		resourceService.areResourceProvidersLoaded = true;
 	});
 
-	it('Should be correct when provider id is correct.', async function(): Promise<void> {
+	it('Should be correct when provider id is correct.', async function (): Promise<void> {
 		const children = await resourceService.getChildren(mockResourceProvider1.object.providerId, TypeMoq.It.isAny());
 		should(children).Array();
 	});
 
-	it('Should throw exceptions when provider id is incorrect.', async function(): Promise<void> {
+	it('Should throw exceptions when provider id is incorrect.', async function (): Promise<void> {
 		const providerId = 'non_existent_provider_id';
 		try {
 			await resourceService.getRootChildren(providerId, mockAccount, mockSubscription, mockTenantId);
@@ -153,7 +157,7 @@ describe('AzureResourceService.getChildren', function(): void {
 	});
 });
 
-describe('AzureResourceService.getTreeItem', function(): void {
+describe('AzureResourceService.getTreeItem', function (): void {
 	beforeEach(() => {
 		mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType<azureResource.IAzureResourceTreeDataProvider>();
 		mockResourceTreeDataProvider1.setup((o) => o.getRootChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType<azdata.TreeItem>().object]));
@@ -168,12 +172,12 @@ describe('AzureResourceService.getTreeItem', function(): void {
 		resourceService.areResourceProvidersLoaded = true;
 	});
 
-	it('Should be correct when provider id is correct.', async function(): Promise<void> {
+	it('Should be correct when provider id is correct.', async function (): Promise<void> {
 		const treeItem = await resourceService.getTreeItem(mockResourceProvider1.object.providerId, TypeMoq.It.isAny());
 		should(treeItem).Object();
 	});
 
-	it('Should throw exceptions when provider id is incorrect.', async function(): Promise<void> {
+	it('Should throw exceptions when provider id is incorrect.', async function (): Promise<void> {
 		const providerId = 'non_existent_provider_id';
 		try {
 			await resourceService.getRootChildren(providerId, mockAccount, mockSubscription, mockTenantId);

extensions/azurecore/src/test/azureResource/resourceTreeNode.test.ts

@@ -31,6 +31,10 @@ const mockAccount: AzureAccount = {
 	properties: {
 		providerSettings: settings[0].metadata,
 		isMsAccount: true,
+		owningTenant: {
+			id: 'tenantId',
+			displayName: 'tenantDisplayName',
+		},
 		tenants: []
 	},
 	isStale: false

extensions/azurecore/src/test/azureResource/tree/accountTreeNode.test.ts

@@ -35,7 +35,10 @@ let mockTreeChangeHandler: TypeMoq.IMock<IAzureResourceTreeChangeHandler>;
 
 // Mock test data
 const mockTenantId = 'mock_tenant_id';
-
+const mockTenant = {
+	id: mockTenantId,
+	displayName: 'Mock Tenant'
+};
 const mockAccount: AzureAccount = {
 	key: {
 		accountId: '97915f6d-84fa-4926-b60c-38db64327ad7',
@@ -50,11 +53,9 @@ const mockAccount: AzureAccount = {
 	},
 	properties: {
 		tenants: [
-			{
+			mockTenant
-				id: mockTenantId,
-				displayName: 'Mock Tenant'
-			}
 		],
+		owningTenant: mockTenant,
 		providerSettings: {
 			settings: allSettings[0].metadata.settings,
 			id: 'azure',
@@ -360,8 +361,7 @@ describe('AzureResourceAccountTreeNode.clearCache', function (): void {
 		sinon.stub(azdata.accounts, 'getAccountSecurityToken').returns(Promise.resolve(mockToken));
 		mockCacheService.setup((o) => o.generateKey(TypeMoq.It.isAnyString())).returns(() => generateGuid());
 		mockCacheService.setup((o) => o.get(TypeMoq.It.isAnyString())).returns(() => mockSubscriptionCache);
-		mockCacheService.setup((o) => o.update(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns(() =>
+		mockCacheService.setup((o) => o.update(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns(() => {
-		{
 			mockSubscriptionCache = mockSubscriptions;
 			return Promise.resolve();
 		});

extensions/azurecore/tsconfig.json

@@ -3,6 +3,7 @@
 	"compilerOptions": {
 		"outDir": "./out",
 		"noUnusedParameters": false,
+		"sourceMap": true,
 		"typeRoots": [
 			"./node_modules/@types"
 		],