From 739de723a51dff7702ddbcc47fd45ca315911d4b Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt <2644648+TylerLeonhardt@users.noreply.github.com> Date: Fri, 7 Mar 2025 14:46:25 -0800 Subject: [PATCH] Fix ScopeData so that `tenantId` truly is only a GUID (#242929) Fixes https://github.com/microsoft/vscode/issues/242839 --- .../src/common/scopeData.ts | 28 +++++++++++++------ .../src/common/test/scopeData.test.ts | 17 +++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/extensions/microsoft-authentication/src/common/scopeData.ts b/extensions/microsoft-authentication/src/common/scopeData.ts index cee462d405718..88a0aad68cc11 100644 --- a/extensions/microsoft-authentication/src/common/scopeData.ts +++ b/extensions/microsoft-authentication/src/common/scopeData.ts @@ -34,12 +34,12 @@ export class ScopeData { readonly clientId: string; /** - * The tenant ID to use for the token request. This is the value of the `VSCODE_TENANT:...` scope if present, otherwise the default tenant ID. + * The tenant ID or `organizations`, `common`, `consumers` to use for the token request. This is the value of the `VSCODE_TENANT:...` scope if present, otherwise it's the default. */ readonly tenant: string; /** - * The tenant ID to use for the token request. This is the value of the `VSCODE_TENANT:...` scope if present, otherwise undefined. + * The tenant ID to use for the token request. This will only ever be a GUID if one was specified via the `VSCODE_TENANT:...` scope, otherwise undefined. */ readonly tenantId: string | undefined; @@ -50,11 +50,11 @@ export class ScopeData { this.scopeStr = modifiedScopes.join(' '); this.scopesToSend = this.getScopesToSend(modifiedScopes); this.clientId = this.getClientId(this.allScopes); - this.tenantId = this.getTenantId(this.allScopes); - this.tenant = this.tenantId ?? DEFAULT_TENANT; + this.tenant = this.getTenant(this.allScopes); + this.tenantId = this.getTenantId(this.tenant); } - private getClientId(scopes: string[]) { + private getClientId(scopes: string[]): string { return scopes.reduce((prev, current) => { if (current.startsWith('VSCODE_CLIENT_ID:')) { return current.split('VSCODE_CLIENT_ID:')[1]; @@ -63,16 +63,28 @@ export class ScopeData { }, undefined) ?? DEFAULT_CLIENT_ID; } - private getTenantId(scopes: string[]) { + private getTenant(scopes: string[]): string { return scopes.reduce((prev, current) => { if (current.startsWith('VSCODE_TENANT:')) { return current.split('VSCODE_TENANT:')[1]; } return prev; - }, undefined); + }, undefined) ?? DEFAULT_TENANT; } - private getScopesToSend(scopes: string[]) { + private getTenantId(tenant: string): string | undefined { + switch (tenant) { + case 'organizations': + case 'common': + case 'consumers': + // These are not valid tenant IDs, so we return undefined + return undefined; + default: + return this.tenant; + } + } + + private getScopesToSend(scopes: string[]): string[] { const scopesToSend = scopes.filter(s => !s.startsWith('VSCODE_')); const set = new Set(scopesToSend); diff --git a/extensions/microsoft-authentication/src/common/test/scopeData.test.ts b/extensions/microsoft-authentication/src/common/test/scopeData.test.ts index 9250d7cecbda0..4c70e4fd07c8d 100644 --- a/extensions/microsoft-authentication/src/common/test/scopeData.test.ts +++ b/extensions/microsoft-authentication/src/common/test/scopeData.test.ts @@ -56,4 +56,21 @@ suite('ScopeData', () => { const scopeData = new ScopeData(['custom_scope', 'VSCODE_TENANT:some_tenant']); assert.strictEqual(scopeData.tenant, 'some_tenant'); }); + + test('should have tenantId be undefined if no VSCODE_TENANT scope is present', () => { + const scopeData = new ScopeData(['custom_scope']); + assert.strictEqual(scopeData.tenantId, undefined); + }); + + test('should have tenantId be undefined if typical tenant values are present', () => { + for (const element of ['common', 'organizations', 'consumers']) { + const scopeData = new ScopeData(['custom_scope', `VSCODE_TENANT:${element}`]); + assert.strictEqual(scopeData.tenantId, undefined); + } + }); + + test('should have tenantId be the value of VSCODE_TENANT scope if set to a specific value', () => { + const scopeData = new ScopeData(['custom_scope', 'VSCODE_TENANT:some_guid']); + assert.strictEqual(scopeData.tenantId, 'some_guid'); + }); });