Skip to content

Commit 4b0129e

Browse files
authored
Merge branch 'main' into telemetry-4-event-aggregation
2 parents df71818 + 6a4a7c4 commit 4b0129e

2 files changed

Lines changed: 71 additions & 5 deletions

File tree

lib/connection/auth/DatabricksOAuth/OAuthManager.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,10 @@ export class AzureOAuthManager extends OAuthManager {
277277
public static datatricksAzureApp = '2ff814a6-3304-4ab8-85cb-cd0e6f879c1d';
278278

279279
protected getOIDCConfigUrl(): string {
280-
return 'https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration';
280+
// Use logical OR so empty / whitespace-only azureTenantId also falls back to /organizations/
281+
// (`??` only substitutes for null/undefined, leaving `''` to produce a malformed `//v2.0/...` URL).
282+
const tenantPath = this.options.azureTenantId?.trim() || 'organizations';
283+
return `https://login.microsoftonline.com/${tenantPath}/v2.0/.well-known/openid-configuration`;
281284
}
282285

283286
protected getAuthorizationUrl(): string {
@@ -293,17 +296,18 @@ export class AzureOAuthManager extends OAuthManager {
293296
}
294297

295298
protected getScopes(requestedScopes: OAuthScopes): OAuthScopes {
296-
// There is no corresponding scopes in Azure, instead, access control will be delegated to Databricks
297-
const tenantId = this.options.azureTenantId ?? AzureOAuthManager.datatricksAzureApp;
299+
// There is no corresponding scopes in Azure, instead, access control will be delegated to Databricks.
300+
// Scope must be the Azure *resource* ID (the Databricks Azure Login App), NOT the tenant ID.
301+
const resourceId = AzureOAuthManager.datatricksAzureApp;
298302

299303
const azureScopes = [];
300304

301305
switch (this.options.flow) {
302306
case OAuthFlow.U2M:
303-
azureScopes.push(`${tenantId}/user_impersonation`);
307+
azureScopes.push(`${resourceId}/user_impersonation`);
304308
break;
305309
case OAuthFlow.M2M:
306-
azureScopes.push(`${tenantId}/.default`);
310+
azureScopes.push(`${resourceId}/.default`);
307311
break;
308312
// no default
309313
}

tests/unit/connection/auth/DatabricksOAuth/OAuthManager.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,3 +519,65 @@ class OpenIDClientStub implements BaseClient {
519519
});
520520
});
521521
});
522+
523+
describe('AzureOAuthManager (tenant awareness)', () => {
524+
function makeAzure(overrides: Partial<OAuthManagerOptions> = {}) {
525+
return new AzureOAuthManager({
526+
host: 'adb-1234567890123456.1.azuredatabricks.net',
527+
flow: OAuthFlow.M2M,
528+
context: new ClientContextStub(),
529+
...overrides,
530+
});
531+
}
532+
533+
// Access protected methods for unit inspection.
534+
const call = <T>(mgr: AzureOAuthManager, name: string, ...args: unknown[]): T =>
535+
(mgr as unknown as Record<string, (...a: unknown[]) => T>)[name](...args);
536+
537+
describe('getOIDCConfigUrl', () => {
538+
it('falls back to /organizations/ when azureTenantId is not set (baseline-compatible)', () => {
539+
const mgr = makeAzure();
540+
const url = call<string>(mgr, 'getOIDCConfigUrl');
541+
expect(url).to.equal('https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration');
542+
});
543+
544+
it('uses the caller-supplied tenant in the discovery URL when provided', () => {
545+
const tenant = '9f37a392-f0ae-4280-9796-f1864a10effc';
546+
const mgr = makeAzure({ azureTenantId: tenant });
547+
const url = call<string>(mgr, 'getOIDCConfigUrl');
548+
expect(url).to.equal(`https://login.microsoftonline.com/${tenant}/v2.0/.well-known/openid-configuration`);
549+
});
550+
551+
it('falls back to /organizations/ when azureTenantId is empty or whitespace', () => {
552+
for (const tenant of ['', ' ']) {
553+
const mgr = makeAzure({ azureTenantId: tenant });
554+
const url = call<string>(mgr, 'getOIDCConfigUrl');
555+
expect(url).to.equal('https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration');
556+
}
557+
});
558+
});
559+
560+
describe('getScopes — resource ID is always the Azure Login App, never a tenant GUID', () => {
561+
it('M2M scope uses the Databricks Azure Login App resource ID even when azureTenantId is set', () => {
562+
const tenant = '9f37a392-f0ae-4280-9796-f1864a10effc';
563+
const mgr = makeAzure({ azureTenantId: tenant, flow: OAuthFlow.M2M });
564+
const scopes = call<string[]>(mgr, 'getScopes', []);
565+
expect(scopes).to.include(`${AzureOAuthManager.datatricksAzureApp}/.default`);
566+
expect(scopes).to.not.include(`${tenant}/.default`);
567+
});
568+
569+
it('U2M scope uses the Databricks Azure Login App resource ID even when azureTenantId is set', () => {
570+
const tenant = '9f37a392-f0ae-4280-9796-f1864a10effc';
571+
const mgr = makeAzure({ azureTenantId: tenant, flow: OAuthFlow.U2M });
572+
const scopes = call<string[]>(mgr, 'getScopes', []);
573+
expect(scopes).to.include(`${AzureOAuthManager.datatricksAzureApp}/user_impersonation`);
574+
expect(scopes).to.not.include(`${tenant}/user_impersonation`);
575+
});
576+
577+
it('preserves offline_access when requested alongside M2M', () => {
578+
const mgr = makeAzure({ flow: OAuthFlow.M2M });
579+
const scopes = call<string[]>(mgr, 'getScopes', [OAuthScope.offlineAccess]);
580+
expect(scopes).to.include(OAuthScope.offlineAccess);
581+
});
582+
});
583+
});

0 commit comments

Comments
 (0)