-
Notifications
You must be signed in to change notification settings - Fork 22
fix: derive OIDC ALLOWED_HOSTS from connection string hostname instead of hardcoded *.azure.com (fixes #639) #721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
tnaum-ms
merged 4 commits into
microsoft:main
from
hanhan761:fix-639-oidc-allowed-hosts
Jun 22, 2026
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e88a7e6
fix: derive OIDC ALLOWED_HOSTS from connection string hostname instea…
hanhan761 96008c6
fix: use DocumentDBConnectionString for multi-host support in OIDC al…
hanhan761 8fe55d3
fix(oidc): restrict allowed hosts to the Azure-family suffix
tnaum-ms 96481dd
test(oidc): cover mongodb+srv seedlist connection strings
tnaum-ms File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { getOidcAllowedHosts } from './oidcAllowedHosts'; | ||
|
|
||
| describe('getOidcAllowedHosts', () => { | ||
| it('allows only the Azure suffix for a public-cloud host, not the full host', () => { | ||
| const result = getOidcAllowedHosts('mongodb://asdf.xyz.mongocluster.cosmos.azure.com:10255/?ssl=true'); | ||
| // The allowlist must stay broad enough to match the host (the driver | ||
| // checks host.endsWith('.azure.com')) but must NOT echo the full host. | ||
| expect(result).toEqual(['*.azure.com']); | ||
| }); | ||
|
|
||
| it('extends to sovereign-cloud TLDs', () => { | ||
| expect(getOidcAllowedHosts('mongodb://cluster.mongocluster.cosmos.azure.us:10255/')).toEqual(['*.azure.us']); | ||
| expect(getOidcAllowedHosts('mongodb://cluster.mongocluster.cosmos.azure.cn:10255/')).toEqual(['*.azure.cn']); | ||
| }); | ||
|
|
||
| it('handles mongodb+srv:// seedlist connection strings', () => { | ||
| // SRV hosts carry no port and the resolved nodes stay under the same | ||
| // Azure suffix, so *.azure.com still covers them. | ||
| expect(getOidcAllowedHosts('mongodb+srv://cluster.mongocluster.cosmos.azure.com/?tls=true')).toEqual([ | ||
| '*.azure.com', | ||
| ]); | ||
| }); | ||
|
|
||
| it('does not widen the allowlist to an attacker-supplied host', () => { | ||
| // A connection string the user was tricked into pasting must not be able | ||
| // to authorize token delivery to a non-Azure host. | ||
| expect(getOidcAllowedHosts('mongodb://evil.com:10255/')).toEqual(['*.azure.com']); | ||
| // Lookalike where "azure" is not the registrable second level. | ||
| expect(getOidcAllowedHosts('mongodb://node.azure.com.evil.com:10255/')).toEqual(['*.azure.com']); | ||
| }); | ||
|
|
||
| it('collapses multiple same-cloud hosts to a single suffix entry', () => { | ||
| const result = getOidcAllowedHosts( | ||
| 'mongodb://a.mongocluster.cosmos.azure.com:10255,b.mongocluster.cosmos.azure.com:10255/?replicaSet=rs0', | ||
| ); | ||
| expect(result).toEqual(['*.azure.com']); | ||
| }); | ||
|
|
||
| it('ignores ports when classifying the host', () => { | ||
| expect(getOidcAllowedHosts('mongodb://cluster.azure.com:27017/')).toEqual(['*.azure.com']); | ||
| }); | ||
|
|
||
| it('falls back to the safe default when the connection string cannot be parsed', () => { | ||
| expect(getOidcAllowedHosts('not a connection string')).toEqual(['*.azure.com']); | ||
| expect(getOidcAllowedHosts('')).toEqual(['*.azure.com']); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { DocumentDBConnectionString } from '../utils/DocumentDBConnectionString'; | ||
|
|
||
| // Safe default when we cannot positively identify an Azure endpoint. This is the | ||
| // historical behavior and covers the public cloud (including private endpoints, | ||
| // which still resolve under *.azure.com). | ||
| const DEFAULT_ALLOWED_HOSTS = ['*.azure.com']; | ||
|
|
||
| /** | ||
| * Resolve the OIDC ALLOWED_HOSTS list from a connection string. | ||
| * | ||
| * ALLOWED_HOSTS is a security control: the driver only sends the OIDC token to a | ||
| * server whose hostname matches one of these patterns. It must therefore stay a | ||
| * curated allowlist. We intentionally do NOT echo the raw connection-string host | ||
| * back into the allowlist: doing so would let an attacker-supplied host (e.g. | ||
| * `evil.com`) widen its own allowlist to `*.evil.com`, defeating the control. | ||
| * | ||
| * Instead we recognize the Azure-family registrable suffix (`azure.<tld>`) and | ||
| * allow only `*.azure.<tld>`. This keeps the public-cloud posture identical to | ||
| * the previous hardcoded `*.azure.com` while transparently extending it to | ||
| * sovereign clouds (`azure.us`, `azure.cn`, ...). Anything we cannot positively | ||
| * classify as Azure falls back to the safe public-cloud default. | ||
| * | ||
| * Note: sovereign clouds also use a different Entra token endpoint; wiring that | ||
| * up is tracked separately. This change only widens the host allowlist. | ||
| */ | ||
| export function getOidcAllowedHosts(connectionString: string): string[] { | ||
|
hanhan761 marked this conversation as resolved.
|
||
| try { | ||
| const parsed = new DocumentDBConnectionString(connectionString); | ||
|
|
||
| // Deduplicate so a replica-set connection string with several hosts in | ||
| // the same cloud yields a single `*.azure.<tld>` entry. | ||
| const suffixes = new Set<string>(); | ||
| for (const host of parsed.hosts ?? []) { | ||
| const suffix = getAzureHostSuffix(host); | ||
| if (suffix) { | ||
| suffixes.add(suffix); | ||
| } | ||
| } | ||
|
|
||
| if (suffixes.size > 0) { | ||
| return [...suffixes].map((suffix) => `*.${suffix}`); | ||
| } | ||
| } catch { | ||
| // Connection string could not be parsed; fall back to the safe default. | ||
| } | ||
|
|
||
| return DEFAULT_ALLOWED_HOSTS; | ||
| } | ||
|
hanhan761 marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Returns the Azure-family registrable suffix (`azure.<tld>`) for a host, or | ||
| * `undefined` if the host is not an Azure endpoint. | ||
| * | ||
| * The `azure` label must be the registrable second level (immediately before the | ||
| * top-level label) so that lookalikes such as `azure.com.evil.com` are rejected. | ||
| */ | ||
| function getAzureHostSuffix(host: string): string | undefined { | ||
| // `hosts` entries may carry a port (e.g. `cluster.azure.com:10255`) and an | ||
| // IPv6 literal is wrapped in brackets; neither is part of the suffix. | ||
| const hostname = host | ||
| .replace(/^\[.*\]/, '') // drop bracketed IPv6 literals (never Azure FQDNs) | ||
| .split(':')[0] | ||
| ?.trim() | ||
| .toLowerCase(); | ||
|
|
||
| if (!hostname) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const labels = hostname.split('.'); | ||
| if (labels.length < 2) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const topLevel = labels[labels.length - 1]; | ||
| const secondLevel = labels[labels.length - 2]; | ||
|
|
||
| if (secondLevel === 'azure' && topLevel.length > 0) { | ||
| return `azure.${topLevel}`; | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.