Skip to content

Fix Azure AD OAuth: tenant-aware discovery URL + correct scope resource#363

Merged
msrathore-db merged 4 commits into
mainfrom
spog-fixes
May 15, 2026
Merged

Fix Azure AD OAuth: tenant-aware discovery URL + correct scope resource#363
msrathore-db merged 4 commits into
mainfrom
spog-fixes

Conversation

@msrathore-db
Copy link
Copy Markdown
Contributor

@msrathore-db msrathore-db commented Apr 21, 2026

Summary

AzureOAuthManager had two coupled bugs in how it constructed the Azure AD OAuth request. They were masked on the default path by two unrelated defaults happening to line up, but fired the moment a customer brought their own Entra app or set azureTenantId — which is the production-recommended posture (single-tenant Entra app per workspace, tenant supplied explicitly).

This PR fixes both Azure-AD-side bugs, plus a third small footgun (empty/whitespace azureTenantId) that the new template URL introduced.

When this fires

Both U2M and M2M go through the same AzureOAuthManager.getOIDCConfigUrl() and getScopes() — Node has a single Azure code path, so any bug in those methods affects both flows. The bugs surface whenever any of the following is true:

  • Caller sets azureTenantId
  • Caller uses a single-tenant Entra app via oauthClientId (the security-recommended posture)
  • Caller uses M2M with their own Entra app (typical for automated pipelines / SPN auth)

The default path (no oauthClientId, no azureTenantId, default multi-tenant Node client) works on baseline only because two unrelated defaults coincide. Any deviation from that exact default flips one or both bugs into a hard failure.

Bug 1 — OIDC discovery URL is hardcoded to /organizations/, ignores azureTenantId

// before
return 'https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration';

// after
const tenantPath = this.options.azureTenantId?.trim() || 'organizations';
return `https://login.microsoftonline.com/${tenantPath}/v2.0/.well-known/openid-configuration`;

The /organizations/ endpoint is Microsoft's "any work/school AAD tenant" multi-tenant endpoint. It only resolves for multi-tenant Entra apps. Single-tenant Entra apps fail at discovery with:

AADSTS50059: No tenant-identifying information found in either the request or implied by any provided credentials.

The customer-supplied azureTenantId was previously dropped on the floor — never used. After the fix, when azureTenantId is set the URL routes through that tenant; when unset the byte-identical /organizations/ fallback is preserved for the default path.

The ?.trim() || 'organizations' form (rather than ?? 'organizations') also handles empty-string and whitespace-only inputs (e.g. process.env.AZURE_TENANT_ID ?? '') — without this, an empty value would render https://login.microsoftonline.com//v2.0/... (double slash) and surface as an opaque Azure 404.

Bug 2 — OAuth scope used the tenant GUID where the Azure resource App ID belongs

// before
const tenantId = this.options.azureTenantId ?? AzureOAuthManager.datatricksAzureApp;
azureScopes.push(`${tenantId}/user_impersonation`);   // U2M
azureScopes.push(`${tenantId}/.default`);             // M2M

// after
const resourceId = AzureOAuthManager.datatricksAzureApp;
azureScopes.push(`${resourceId}/user_impersonation`);
azureScopes.push(`${resourceId}/.default`);

Azure v2.0 scope grammar is <resource-app-id>/<permission-name> — the segment before the slash is the application you're requesting a token for (the Databricks Azure Login App, 2ff814a6-3304-4ab8-85cb-cd0e6f879c1d), not the user's tenant. The pre-fix variable name tenantId was a misnomer; the fallback value (datatricksAzureApp) happened to be the correct resource ID, so the default path produced a working scope by coincidence. The moment a caller set azureTenantId, the scope became <tenant-GUID>/... and Azure rejected with:

AADSTS500011: The resource principal named <tenant-GUID> was not found in the tenant named <tenant-GUID>.

The fix separates the two concepts cleanly:

  • Tenant identifies an Entra directory → goes in the URL path of the discovery/token endpoints.
  • Resource identifies an application you want a token for → goes in the scope= parameter.

Empirical verification — pecotesting production workspace

Same SPN credential, same workspace, same tenant — only the SDK code differs.

AzureOAuthManager.getToken() directly, M2M flow with a customer Entra app (d154b9ed-...) and Azure-Portal client secret, against adb-6436897454825492.12.azuredatabricks.net, tenant 9f37a392-f0ae-4280-9796-f1864a10effc:

Branch URL the manager built Scope it sent Azure response
main https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration 9f37a392-.../.default (tenant GUID — Bug 2) AADSTS50059 — auth dies at discovery (Bug 1)
this PR https://login.microsoftonline.com/9f37a392-.../v2.0/.well-known/openid-configuration 2ff814a6-.../.default (resource App ID — correct) real access_token issued

End-to-end through DBSQLClient.connect() with the same credentials:

Branch Result
main AADSTS50059 — driver cannot acquire an Azure token; never reaches the workspace
this PR Azure issues access token; Thrift call reaches workspace; HTTP 403 from workspace ACL on 2f03dd43e35e2aa0 (separate authorization layer — the test SPN doesn't have warehouse permissions). With a permissioned identity, this row is a clean PASS.

The 403 in the post-fix row is the same workspace-level ACL check every Azure-authenticated client hits; it's downstream of any auth code. On main you cannot reach that check at all because Azure-AD authentication itself fails.

Backward compatibility

When azureTenantId is unset, both methods produce byte-identical output to baseline:

  • Discovery URL: …/organizations/v2.0/… — character-for-character unchanged.
  • Scope: 2ff814a6-3304-4ab8-85cb-cd0e6f879c1d/.default (M2M) or …/user_impersonation (U2M) — same value the pre-fix fallback produced.

Verified by reading both code paths and by running the default-path test against production (identical request, identical Azure response). No regression risk for the default multi-tenant Node client path that "worked by luck" before.

Test plan

  • Unit tests: 6 new cases covering getOIDCConfigUrl (fallback, tenant-specific, empty/whitespace-fallback) and getScopes (resource-ID invariant for M2M+U2M, offline_access preserved). All 34 AzureOAuthManager + DatabricksOAuthManager suite tests pass locally.
  • Full unit test suite passes.
  • Live verification against pecotesting production workspace: main reproduces AADSTS50059; this branch produces a real Azure access token. End-to-end query blocked only by workspace ACL (separate from auth — confirmed by PERMISSION_DENIED from the SQL endpoint, not from Azure).

…ource

The AzureOAuthManager had two coupled bugs that prevented Azure AD OAuth
from working for tenant-specific Entra apps and any single-tenant Entra
app.

1) OIDC discovery URL was hardcoded to /organizations/, ignoring
   azureTenantId. Single-tenant Entra apps cannot be resolved via
   /organizations/ and return AADSTS50059. Now:

     https://login.microsoftonline.com/${azureTenantId ?? 'organizations'}/v2.0/
                                        .well-known/openid-configuration

   When azureTenantId is unset, the URL is byte-identical to before
   (/organizations/) — no regression for the multi-tenant default.

2) The OAuth scope was built from azureTenantId when provided:

     const tenantId = this.options.azureTenantId ?? datatricksAzureApp;
     azureScopes.push(`${tenantId}/.default`);

   The Azure v2.0 scope format is <resource-app-id>/.default — a tenant
   GUID isn't a resource, and Azure rejects with AADSTS500011. The scope
   now always uses the Databricks Azure Login App ID.

Empirical verification (Prod Legacy, baseline vs. patched):

  PAT:                                            PASS / PASS  (identical)
  DB-M2M via DatabricksOAuthManager:              PASS / PASS  (identical)
  AzureOAuthManager, no azureTenantId:            AADSTS50059 / AADSTS50059
                                                  (identical; test cred is
                                                   single-tenant)
  AzureOAuthManager + azureTenantId:              AADSTS50059 / AADSTS7000215
                                                  (patched reaches Azure AD;
                                                   both fail because test env
                                                   has a Databricks-side
                                                   secret, not an Azure-Portal
                                                   secret)
  AzureOAuthManager via useDatabricksOAuthInAzure: 403 / 403  (identical)

No prod path that worked on baseline fails on patched. The common
multi-tenant-app + no-azureTenantId path is byte-identical.

Unit tests cover:
  - getOIDCConfigUrl fallback (no tenant) and tenant-specific
  - getScopes: M2M+U2M scope always uses the Databricks Azure Login App
    resource ID (never the tenant GUID); offline_access preserved

Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
@msrathore-db msrathore-db changed the title Fix Azure AD OAuth: discovery URL, scope resource, staging resource ID Fix Azure AD OAuth: tenant-aware discovery URL + correct scope resource Apr 21, 2026
@msrathore-db msrathore-db changed the title Fix Azure AD OAuth: tenant-aware discovery URL + correct scope resource Fix Azure AD OAuth: tenant-aware discovery URL, correct scope, staging resource ID Apr 21, 2026
@msrathore-db msrathore-db changed the title Fix Azure AD OAuth: tenant-aware discovery URL, correct scope, staging resource ID Fix Azure AD OAuth: tenant-aware discovery URL + correct scope resource Apr 21, 2026
??-coalescing only substitutes for null/undefined, so an empty or
whitespace-only azureTenantId would build
`https://login.microsoftonline.com//v2.0/...` (double slash) and surface
as an opaque Azure 404. Trim first and use || so any falsy/blank value
falls back to /organizations/ like the unset case.

Co-authored-by: Isaac
@github-actions
Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Comment thread CHANGELOG.md Outdated
Entries are added under a versioned header in a dedicated "prepare
release" PR — feature/fix PRs don't touch CHANGELOG.

Co-authored-by: Isaac
@github-actions
Copy link
Copy Markdown

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@msrathore-db msrathore-db merged commit 6a4a7c4 into main May 15, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants