From b83fd991feda9819b4d40efcf7c055c517c16be9 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 29 Apr 2024 13:22:14 -0500 Subject: [PATCH 1/2] PYTHON-3601 OIDC: Clarify TOKEN_RESOURCE and client_id usage (#1621) Co-authored-by: Mike Woofter <108414937+mongoKart@users.noreply.github.com> --- doc/examples/authentication.rst | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/doc/examples/authentication.rst b/doc/examples/authentication.rst index cac9d49a9..24b3cff8d 100644 --- a/doc/examples/authentication.rst +++ b/doc/examples/authentication.rst @@ -408,8 +408,10 @@ Azure IMDS ^^^^^^^^^^ For an application running on an Azure VM or otherwise using the `Azure Internal Metadata Service`_, -you can use the built-in support for Azure, where "" below is the client id of the Azure -managed identity, and ```` is the url-encoded ``audience`` `configured on your MongoDB deployment`_. +you can use the built-in support for Azure. If using an Azure managed identity, the "" is +the client ID. If using a service principal to represent an enterprise application, the "" is +the application ID of the service principal. The ```` value is the ``audience`` +`configured on your MongoDB deployment`_. .. code-block:: python @@ -430,11 +432,24 @@ managed identity, and ```` is the url-encoded ``audience`` `configured If the application is running on an Azure VM and only one managed identity is associated with the VM, ``username`` can be omitted. +If providing the ``TOKEN_RESOURCE`` as part of a connection string, it can be given as follows. +If the ``TOKEN_RESOURCE`` contains any of the following characters [``,``, ``+``, ``&``], then +it MUST be url-encoded. + +.. code-block:: python + + import os + + uri = f'{os.environ["MONGODB_URI"]}?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:' + c = MongoClient(uri) + c.test.test.insert_one({}) + c.close() + GCP IMDS ^^^^^^^^ For an application running on an GCP VM or otherwise using the `GCP Internal Metadata Service`_, -you can use the built-in support for GCP, where ```` below is the url-encoded ``audience`` +you can use the built-in support for GCP, where ```` below is the ``audience`` `configured on your MongoDB deployment`_. .. code-block:: python @@ -448,6 +463,18 @@ you can use the built-in support for GCP, where ```` below is the url- c.test.test.insert_one({}) c.close() +If providing the ``TOKEN_RESOURCE`` as part of a connection string, it can be given as follows. +If the ``TOKEN_RESOURCE`` contains any of the following characters [``,``, ``+``, ``&``], then +it MUST be url-encoded. + +.. code-block:: python + + import os + + uri = f'{os.environ["MONGODB_URI"]}?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:gcp,TOKEN_RESOURCE:' + c = MongoClient(uri) + c.test.test.insert_one({}) + c.close() Custom Callbacks ~~~~~~~~~~~~~~~~ From 6584dd23892009548f3aa84bbe418490f19b2deb Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 29 Apr 2024 15:45:24 -0500 Subject: [PATCH 2/2] PYTHON-4256 Clean up handling of TOKEN_RESOURCE (#1620) --- pymongo/_azure_helpers.py | 1 - pymongo/auth.py | 7 ++-- pymongo/auth_oidc.py | 6 ++- pymongo/common.py | 23 +++++------ test/auth/legacy/connection-string.json | 52 +++++++++++++++++-------- test/test_auth_spec.py | 28 ++----------- test/test_uri_parser.py | 22 ++++++++--- 7 files changed, 70 insertions(+), 69 deletions(-) diff --git a/pymongo/_azure_helpers.py b/pymongo/_azure_helpers.py index a7244fb3d..704c561cd 100644 --- a/pymongo/_azure_helpers.py +++ b/pymongo/_azure_helpers.py @@ -32,7 +32,6 @@ def _get_azure_response( url += f"&client_id={client_id}" headers = {"Metadata": "true", "Accept": "application/json"} request = Request(url, headers=headers) # noqa: S310 - print("fetching url", url) # noqa: T201 try: with urlopen(request, timeout=timeout) as response: # noqa: S310 status = response.status diff --git a/pymongo/auth.py b/pymongo/auth.py index 738131be9..8bc4145ab 100644 --- a/pymongo/auth.py +++ b/pymongo/auth.py @@ -33,7 +33,7 @@ from typing import ( Optional, cast, ) -from urllib.parse import quote, unquote +from urllib.parse import quote from bson.binary import Binary from pymongo.auth_aws import _authenticate_aws @@ -138,7 +138,7 @@ def _build_credentials_tuple( raise ValueError("authentication source must be $external or None for GSSAPI") properties = extra.get("authmechanismproperties", {}) service_name = properties.get("SERVICE_NAME", "mongodb") - canonicalize = properties.get("CANONICALIZE_HOST_NAME", False) + canonicalize = bool(properties.get("CANONICALIZE_HOST_NAME", False)) service_realm = properties.get("SERVICE_REALM") props = GSSAPIProperties( service_name=service_name, @@ -173,8 +173,6 @@ def _build_credentials_tuple( human_callback = properties.get("OIDC_HUMAN_CALLBACK") environ = properties.get("ENVIRONMENT") token_resource = properties.get("TOKEN_RESOURCE", "") - if unquote(token_resource) == token_resource: - token_resource = quote(token_resource) default_allowed = [ "*.mongodb.net", "*.mongodb-dev.net", @@ -227,6 +225,7 @@ def _build_credentials_tuple( human_callback=human_callback, environment=environ, allowed_hosts=allowed_hosts, + token_resource=token_resource, username=user, ) return MongoCredential(mech, "$external", user, passwd, oidc_props, _Cache()) diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index 43b935be7..bfe2340f0 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -21,6 +21,7 @@ import threading import time from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Mapping, MutableMapping, Optional, Union +from urllib.parse import quote import bson from bson.binary import Binary @@ -72,6 +73,7 @@ class _OIDCProperties: human_callback: Optional[OIDCCallback] = field(default=None) environment: Optional[str] = field(default=None) allowed_hosts: list[str] = field(default_factory=list) + token_resource: Optional[str] = field(default=None) username: str = "" @@ -126,7 +128,7 @@ class _OIDCTestCallback(OIDCCallback): class _OIDCAzureCallback(OIDCCallback): def __init__(self, token_resource: str) -> None: - self.token_resource = token_resource + self.token_resource = quote(token_resource) def fetch(self, context: OIDCCallbackContext) -> OIDCCallbackResult: resp = _get_azure_response(self.token_resource, context.username, context.timeout_seconds) @@ -137,7 +139,7 @@ class _OIDCAzureCallback(OIDCCallback): class _OIDCGCPCallback(OIDCCallback): def __init__(self, token_resource: str) -> None: - self.token_resource = token_resource + self.token_resource = quote(token_resource) def fetch(self, context: OIDCCallbackContext) -> OIDCCallbackResult: resp = _get_gcp_response(self.token_resource, context.timeout_seconds) diff --git a/pymongo/common.py b/pymongo/common.py index e990fddf8..7f1245b7d 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -453,26 +453,21 @@ def validate_auth_mechanism_properties(option: str, value: Any) -> dict[str, Uni value = validate_string(option, value) for opt in value.split(","): - try: - key, val = opt.split(":") - except ValueError: - # Try not to leak the token. - if "AWS_SESSION_TOKEN" in opt: - opt = ( # noqa: PLW2901 - "AWS_SESSION_TOKEN:, did you forget " - "to percent-escape the token with quote_plus?" - ) - raise ValueError( - "auth mechanism properties must be " - "key:value pairs like SERVICE_NAME:" - f"mongodb, not {opt}." - ) from None + key, _, val = opt.partition(":") if key not in _MECHANISM_PROPS: + # Try not to leak the token. + if "AWS_SESSION_TOKEN" in key: + raise ValueError( + "auth mechanism properties must be " + "key:value pairs like AWS_SESSION_TOKEN:" + ) + raise ValueError( f"{key} is not a supported auth " "mechanism property. Must be one of " f"{tuple(_MECHANISM_PROPS)}." ) + if key == "CANONICALIZE_HOST_NAME": props[key] = validate_boolean_or_string(key, val) else: diff --git a/test/auth/legacy/connection-string.json b/test/auth/legacy/connection-string.json index 707b6167d..f8fe0aeb5 100644 --- a/test/auth/legacy/connection-string.json +++ b/test/auth/legacy/connection-string.json @@ -474,7 +474,7 @@ } }, { - "description": "should throw an exception if username and password is specified for test environment (MONGODB-OIDC)", + "description": "should throw an exception if username and password is specified for test environment (MONGODB-OIDC)", "uri": "mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test", "valid": false, "credential": null @@ -486,23 +486,11 @@ "credential": null }, { - "description": "should throw an exception if specified provider is not supported (MONGODB-OIDC)", + "description": "should throw an exception if specified environment is not supported (MONGODB-OIDC)", "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:invalid", "valid": false, "credential": null }, - { - "description": "should throw an exception custom callback is chosen but no callback is provided (MONGODB-OIDC)", - "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:custom", - "valid": false, - "credential": null - }, - { - "description": "should throw an exception custom callback is chosen but no callback is provided (MONGODB-OIDC)", - "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:custom", - "valid": false, - "credential": null - }, { "description": "should throw an exception if neither provider nor callbacks specified (MONGODB-OIDC)", "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC", @@ -541,7 +529,7 @@ }, { "description": "should accept a url-encoded TOKEN_RESOURCE (MONGODB-OIDC)", - "uri": "mongodb://user@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:mongodb%253A//test-cluster", + "uri": "mongodb://user@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:mongodb%3A%2F%2Ftest-cluster", "valid": true, "credential": { "username": "user", @@ -550,7 +538,37 @@ "mechanism": "MONGODB-OIDC", "mechanism_properties": { "ENVIRONMENT": "azure", - "TOKEN_RESOURCE": "mongodb%253A//test-cluster" + "TOKEN_RESOURCE": "mongodb://test-cluster" + } + } + }, + { + "description": "should accept an un-encoded TOKEN_RESOURCE (MONGODB-OIDC)", + "uri": "mongodb://user@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:mongodb://test-cluster", + "valid": true, + "credential": { + "username": "user", + "password": null, + "source": "$external", + "mechanism": "MONGODB-OIDC", + "mechanism_properties": { + "ENVIRONMENT": "azure", + "TOKEN_RESOURCE": "mongodb://test-cluster" + } + } + }, + { + "description": "should handle a complicated url-encoded TOKEN_RESOURCE (MONGODB-OIDC)", + "uri": "mongodb://user@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:abc%2Cd%25ef%3Ag%26hi", + "valid": true, + "credential": { + "username": "user", + "password": null, + "source": "$external", + "mechanism": "MONGODB-OIDC", + "mechanism_properties": { + "ENVIRONMENT": "azure", + "TOKEN_RESOURCE": "abc,d%ef:g&hi" } } }, @@ -565,7 +583,7 @@ "mechanism": "MONGODB-OIDC", "mechanism_properties": { "ENVIRONMENT": "azure", - "TOKEN_RESOURCE": "a%24b" + "TOKEN_RESOURCE": "a$b" } } }, diff --git a/test/test_auth_spec.py b/test/test_auth_spec.py index 71df9f032..6cd037e20 100644 --- a/test/test_auth_spec.py +++ b/test/test_auth_spec.py @@ -52,12 +52,7 @@ def create_test(test_case): warnings.simplefilter("default") self.assertRaises(Exception, MongoClient, uri, connect=False) else: - props = {} - if credential: - props = credential["mechanism_properties"] or {} - if props.get("CALLBACK"): - props["callback"] = SampleHumanCallback() - client = MongoClient(uri, connect=False, authmechanismproperties=props) + client = MongoClient(uri, connect=False) credentials = client.options.pool_options._credentials if credential is None: self.assertIsNone(credentials) @@ -73,25 +68,8 @@ def create_test(test_case): expected = credential["mechanism_properties"] if expected is not None: actual = credentials.mechanism_properties - for key, _val in expected.items(): - if "SERVICE_NAME" in expected: - self.assertEqual(actual.service_name, expected["SERVICE_NAME"]) - elif "CANONICALIZE_HOST_NAME" in expected: - self.assertEqual( - actual.canonicalize_host_name, expected["CANONICALIZE_HOST_NAME"] - ) - elif "SERVICE_REALM" in expected: - self.assertEqual(actual.service_realm, expected["SERVICE_REALM"]) - elif "AWS_SESSION_TOKEN" in expected: - self.assertEqual( - actual.aws_session_token, expected["AWS_SESSION_TOKEN"] - ) - elif "ENVIRONMENT" in expected: - self.assertEqual(actual.environment, expected["ENVIRONMENT"]) - elif "callback" in expected: - self.assertEqual(actual.callback, expected["callback"]) - else: - self.fail(f"Unhandled property: {key}") + for key, value in expected.items(): + self.assertEqual(getattr(actual, key.lower()), value) else: if credential["mechanism"] == "MONGODB-AWS": self.assertIsNone(credentials.mechanism_properties.aws_session_token) diff --git a/test/test_uri_parser.py b/test/test_uri_parser.py index 01dff2137..a0283eb90 100644 --- a/test/test_uri_parser.py +++ b/test/test_uri_parser.py @@ -504,20 +504,30 @@ class TestURI(unittest.TestCase): self.assertEqual(options, res["options"]) def test_redact_AWS_SESSION_TOKEN(self): - unquoted_colon = "token:" + token = "token" uri = ( "mongodb://user:password@localhost/?authMechanism=MONGODB-AWS" - "&authMechanismProperties=AWS_SESSION_TOKEN:" + unquoted_colon + "&authMechanismProperties=AWS_SESSION_TOKEN-" + token ) with self.assertRaisesRegex( ValueError, - "auth mechanism properties must be key:value pairs like " - "SERVICE_NAME:mongodb, not AWS_SESSION_TOKEN:" - ", did you forget to percent-escape the token with " - "quote_plus?", + "auth mechanism properties must be key:value pairs like AWS_SESSION_TOKEN:", ): parse_uri(uri) + def test_handle_colon(self): + token = "token:foo" + uri = ( + "mongodb://user:password@localhost/?authMechanism=MONGODB-AWS" + "&authMechanismProperties=AWS_SESSION_TOKEN:" + token + ) + res = parse_uri(uri) + options = { + "authmechanism": "MONGODB-AWS", + "authMechanismProperties": {"AWS_SESSION_TOKEN": token}, + } + self.assertEqual(options, res["options"]) + def test_special_chars(self): user = "user@ /9+:?~!$&'()*+,;=" pwd = "pwd@ /9+:?~!$&'()*+,;="