From 1a0308e15be3d8d46a0a844ac5c8c0014ddf3049 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 29 Oct 2024 12:42:38 -0500 Subject: [PATCH 1/6] PYTHON-3906 Implement GSSAPI ServiceHost support --- pymongo/auth_shared.py | 12 +- pymongo/common.py | 1 + test/auth/legacy/connection-string.json | 117 +++++++++++------- test/connection_string/test/valid-auth.json | 32 +---- .../connection_string/test/valid-options.json | 4 +- 5 files changed, 88 insertions(+), 78 deletions(-) diff --git a/pymongo/auth_shared.py b/pymongo/auth_shared.py index 7e3acd9dfb..e6110e3114 100644 --- a/pymongo/auth_shared.py +++ b/pymongo/auth_shared.py @@ -78,11 +78,13 @@ def __hash__(self) -> int: GSSAPIProperties = namedtuple( - "GSSAPIProperties", ["service_name", "canonicalize_host_name", "service_realm"] + "GSSAPIProperties", ["service_name", "canonicalize_host_name", "service_realm", "service_host"] ) """Mechanism properties for GSSAPI authentication.""" +_CANONICALIZE_HOST_NAME_VALUES = ["false", "true", "none", "forward", "forwardAndReverse"] + _AWSProperties = namedtuple("_AWSProperties", ["aws_session_token"]) """Mechanism properties for MONGODB-AWS authentication.""" @@ -103,12 +105,18 @@ 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 = bool(properties.get("CANONICALIZE_HOST_NAME", False)) + service_host = properties.get("SERVICE_HOST", None) + canonicalize = properties.get("CANONICALIZE_HOST_NAME", "false") + if canonicalize not in _CANONICALIZE_HOST_NAME_VALUES: + raise ConfigurationError( + f"CANONICALIZE_HOST_NAME '{canonicalize}' not in valid options: {_CANONICALIZE_HOST_NAME_VALUES}" + ) service_realm = properties.get("SERVICE_REALM") props = GSSAPIProperties( service_name=service_name, canonicalize_host_name=canonicalize, service_realm=service_realm, + service_host=service_host, ) # Source is always $external. return MongoCredential(mech, "$external", user, passwd, props, None) diff --git a/pymongo/common.py b/pymongo/common.py index 87aa936f5d..4d35515ef2 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -423,6 +423,7 @@ def validate_read_preference_tags(name: str, value: Any) -> list[dict[str, str]] _MECHANISM_PROPS = frozenset( [ "SERVICE_NAME", + "SERVICE_HOST", "CANONICALIZE_HOST_NAME", "SERVICE_REALM", "AWS_SESSION_TOKEN", diff --git a/test/auth/legacy/connection-string.json b/test/auth/legacy/connection-string.json index 57fd9d4a11..3a099c8137 100644 --- a/test/auth/legacy/connection-string.json +++ b/test/auth/legacy/connection-string.json @@ -80,7 +80,7 @@ }, { "description": "should accept generic mechanism property (GSSAPI)", - "uri": "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:true", + "uri": "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:forward,SERVICE_HOST:example.com", "valid": true, "credential": { "username": "user@DOMAIN.COM", @@ -89,10 +89,46 @@ "mechanism": "GSSAPI", "mechanism_properties": { "SERVICE_NAME": "other", - "CANONICALIZE_HOST_NAME": true + "SERVICE_HOST": "example.com", + "CANONICALIZE_HOST_NAME": "forward" } } }, + { + "description": "should accept forwardAndReverse hostname canonicalization (GSSAPI)", + "uri": "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:forwardAndReverse", + "valid": true, + "credential": { + "username": "user@DOMAIN.COM", + "password": null, + "source": "$external", + "mechanism": "GSSAPI", + "mechanism_properties": { + "SERVICE_NAME": "other", + "CANONICALIZE_HOST_NAME": "forwardAndReverse" + } + } + }, + { + "description": "should accept no hostname canonicalization (GSSAPI)", + "uri": "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:none", + "valid": true, + "credential": { + "username": "user@DOMAIN.COM", + "password": null, + "source": "$external", + "mechanism": "GSSAPI", + "mechanism_properties": { + "SERVICE_NAME": "other", + "CANONICALIZE_HOST_NAME": "none" + } + } + }, + { + "description": "must raise an error when the hostname canonicalization is invalid", + "uri": "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:invalid", + "valid": false + }, { "description": "should accept the password (GSSAPI)", "uri": "mongodb://user%40DOMAIN.COM:password@localhost/?authMechanism=GSSAPI&authSource=$external", @@ -127,47 +163,6 @@ "uri": "mongodb://localhost/?authMechanism=GSSAPI", "valid": false }, - { - "description": "should recognize the mechanism (MONGODB-CR)", - "uri": "mongodb://user:password@localhost/?authMechanism=MONGODB-CR", - "valid": true, - "credential": { - "username": "user", - "password": "password", - "source": "admin", - "mechanism": "MONGODB-CR", - "mechanism_properties": null - } - }, - { - "description": "should use the database when no authSource is specified (MONGODB-CR)", - "uri": "mongodb://user:password@localhost/foo?authMechanism=MONGODB-CR", - "valid": true, - "credential": { - "username": "user", - "password": "password", - "source": "foo", - "mechanism": "MONGODB-CR", - "mechanism_properties": null - } - }, - { - "description": "should use the authSource when specified (MONGODB-CR)", - "uri": "mongodb://user:password@localhost/foo?authMechanism=MONGODB-CR&authSource=bar", - "valid": true, - "credential": { - "username": "user", - "password": "password", - "source": "bar", - "mechanism": "MONGODB-CR", - "mechanism_properties": null - } - }, - { - "description": "should throw an exception if no username is supplied (MONGODB-CR)", - "uri": "mongodb://localhost/?authMechanism=MONGODB-CR", - "valid": false - }, { "description": "should recognize the mechanism (MONGODB-X509)", "uri": "mongodb://CN%3DmyName%2COU%3DmyOrgUnit%2CO%3DmyOrg%2CL%3DmyLocality%2CST%3DmyState%2CC%3DmyCountry@localhost/?authMechanism=MONGODB-X509", @@ -474,14 +469,14 @@ } }, { - "description": "should throw an exception if username and password is specified for test environment (MONGODB-OIDC)", + "description": "should throw an exception if supplied a password (MONGODB-OIDC)", "uri": "mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test", "valid": false, "credential": null }, { - "description": "should throw an exception if username is specified for test environment (MONGODB-OIDC)", - "uri": "mongodb://principalName@localhost/?authMechanism=MONGODB-OIDC&ENVIRONMENT:test", + "description": "should throw an exception if username is specified for test (MONGODB-OIDC)", + "uri": "mongodb://principalName@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test", "valid": false, "credential": null }, @@ -492,11 +487,17 @@ "credential": null }, { - "description": "should throw an exception if neither provider nor callbacks specified (MONGODB-OIDC)", + "description": "should throw an exception if neither environment nor callbacks specified (MONGODB-OIDC)", "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC", "valid": false, "credential": null }, + { + "description": "should throw an exception when unsupported auth property is specified (MONGODB-OIDC)", + "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=UnsupportedProperty:unexisted", + "valid": false, + "credential": null + }, { "description": "should recognise the mechanism with azure provider (MONGODB-OIDC)", "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:foo", @@ -625,6 +626,26 @@ "uri": "mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:gcp", "valid": false, "credential": null + }, + { + "description": "should recognise the mechanism with k8s provider (MONGODB-OIDC)", + "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:k8s", + "valid": true, + "credential": { + "username": null, + "password": null, + "source": "$external", + "mechanism": "MONGODB-OIDC", + "mechanism_properties": { + "ENVIRONMENT": "k8s" + } + } + }, + { + "description": "should throw an error for a username and password with k8s provider (MONGODB-OIDC)", + "uri": "mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:k8s", + "valid": false, + "credential": null } ] -} \ No newline at end of file +} diff --git a/test/connection_string/test/valid-auth.json b/test/connection_string/test/valid-auth.json index 4f684ff185..60f63f4e3f 100644 --- a/test/connection_string/test/valid-auth.json +++ b/test/connection_string/test/valid-auth.json @@ -220,29 +220,8 @@ "options": null }, { - "description": "Escaped user info and database (MONGODB-CR)", - "uri": "mongodb://%24am:f%3Azzb%40z%2Fz%3D@127.0.0.1/admin%3F?authMechanism=MONGODB-CR", - "valid": true, - "warning": false, - "hosts": [ - { - "type": "ipv4", - "host": "127.0.0.1", - "port": null - } - ], - "auth": { - "username": "$am", - "password": "f:zzb@z/z=", - "db": "admin?" - }, - "options": { - "authmechanism": "MONGODB-CR" - } - }, - { - "description": "Subdelimiters in user/pass don't need escaping (MONGODB-CR)", - "uri": "mongodb://!$&'()*+,;=:!$&'()*+,;=@127.0.0.1/admin?authMechanism=MONGODB-CR", + "description": "Subdelimiters in user/pass don't need escaping (PLAIN)", + "uri": "mongodb://!$&'()*+,;=:!$&'()*+,;=@127.0.0.1/admin?authMechanism=PLAIN", "valid": true, "warning": false, "hosts": [ @@ -258,7 +237,7 @@ "db": "admin" }, "options": { - "authmechanism": "MONGODB-CR" + "authmechanism": "PLAIN" } }, { @@ -284,7 +263,7 @@ }, { "description": "Escaped username (GSSAPI)", - "uri": "mongodb://user%40EXAMPLE.COM:secret@localhost/?authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:true&authMechanism=GSSAPI", + "uri": "mongodb://user%40EXAMPLE.COM:secret@localhost/?authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:forward,SERVICE_HOST:example.com&authMechanism=GSSAPI", "valid": true, "warning": false, "hosts": [ @@ -303,7 +282,8 @@ "authmechanism": "GSSAPI", "authmechanismproperties": { "SERVICE_NAME": "other", - "CANONICALIZE_HOST_NAME": true + "SERVICE_HOST": "example.com", + "CANONICALIZE_HOST_NAME": "forward" } } }, diff --git a/test/connection_string/test/valid-options.json b/test/connection_string/test/valid-options.json index 3c79fe7ae5..6c86172d08 100644 --- a/test/connection_string/test/valid-options.json +++ b/test/connection_string/test/valid-options.json @@ -2,7 +2,7 @@ "tests": [ { "description": "Option names are normalized to lowercase", - "uri": "mongodb://alice:secret@example.com/admin?AUTHMechanism=MONGODB-CR", + "uri": "mongodb://alice:secret@example.com/admin?AUTHMechanism=PLAIN", "valid": true, "warning": false, "hosts": [ @@ -18,7 +18,7 @@ "db": "admin" }, "options": { - "authmechanism": "MONGODB-CR" + "authmechanism": "PLAIN" } }, { From 7a4dfea089a5704edee14b99d1becfbbf3a92900 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 29 Oct 2024 13:01:04 -0500 Subject: [PATCH 2/6] fix validation --- pymongo/auth_shared.py | 17 +++++++++++------ pymongo/common.py | 4 +++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/pymongo/auth_shared.py b/pymongo/auth_shared.py index e6110e3114..8e703010d1 100644 --- a/pymongo/auth_shared.py +++ b/pymongo/auth_shared.py @@ -83,12 +83,20 @@ def __hash__(self) -> int: """Mechanism properties for GSSAPI authentication.""" -_CANONICALIZE_HOST_NAME_VALUES = ["false", "true", "none", "forward", "forwardAndReverse"] - _AWSProperties = namedtuple("_AWSProperties", ["aws_session_token"]) """Mechanism properties for MONGODB-AWS authentication.""" +def _validate_canonicalize_host_name(value: str | bool) -> str | bool: + valid_names = [False, True, "none", "forward", "forwardAndReverse"] + if value in ["true", "false", True, False]: + return value in ["true", True] + + if value not in valid_names: + raise ValueError(f"CANONICALIZE_HOST_NAME '{value}' not in valid options: {valid_names}") + return value + + def _build_credentials_tuple( mech: str, source: Optional[str], @@ -107,10 +115,7 @@ def _build_credentials_tuple( service_name = properties.get("SERVICE_NAME", "mongodb") service_host = properties.get("SERVICE_HOST", None) canonicalize = properties.get("CANONICALIZE_HOST_NAME", "false") - if canonicalize not in _CANONICALIZE_HOST_NAME_VALUES: - raise ConfigurationError( - f"CANONICALIZE_HOST_NAME '{canonicalize}' not in valid options: {_CANONICALIZE_HOST_NAME_VALUES}" - ) + canonicalize = _validate_canonicalize_host_name(canonicalize) service_realm = properties.get("SERVICE_REALM") props = GSSAPIProperties( service_name=service_name, diff --git a/pymongo/common.py b/pymongo/common.py index 4d35515ef2..a9f5896e88 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -477,7 +477,9 @@ def validate_auth_mechanism_properties(option: str, value: Any) -> dict[str, Uni ) if key == "CANONICALIZE_HOST_NAME": - props[key] = validate_boolean_or_string(key, val) + from pymongo.auth_shared import _validate_canonicalize_host_name + + props[key] = _validate_canonicalize_host_name(val) else: props[key] = val From d1ece65f41b09d979549b5b5d01553593671101e Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 29 Oct 2024 13:58:20 -0500 Subject: [PATCH 3/6] fix tests --- test/auth/legacy/connection-string.json | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test/auth/legacy/connection-string.json b/test/auth/legacy/connection-string.json index 3a099c8137..67aafbff6e 100644 --- a/test/auth/legacy/connection-string.json +++ b/test/auth/legacy/connection-string.json @@ -626,26 +626,6 @@ "uri": "mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:gcp", "valid": false, "credential": null - }, - { - "description": "should recognise the mechanism with k8s provider (MONGODB-OIDC)", - "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:k8s", - "valid": true, - "credential": { - "username": null, - "password": null, - "source": "$external", - "mechanism": "MONGODB-OIDC", - "mechanism_properties": { - "ENVIRONMENT": "k8s" - } - } - }, - { - "description": "should throw an error for a username and password with k8s provider (MONGODB-OIDC)", - "uri": "mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:k8s", - "valid": false, - "credential": null } ] } From 5ed3ed45f20854052d3c4145b7b3645747b2e644 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 29 Oct 2024 18:09:45 -0500 Subject: [PATCH 4/6] fix tests --- pymongo/common.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pymongo/common.py b/pymongo/common.py index a9f5896e88..26b67df689 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -139,6 +139,9 @@ # Default value for serverMonitoringMode SERVER_MONITORING_MODE = "auto" # poll/stream/auto +# Options that must raise an error instead of warning if they invalidate. +_OPTION_MUST_RAISE = ["CANONICALIZE_HOST_NAME"] + def partition_node(node: str) -> tuple[str, int]: """Split a host:port string into (host, int(port)) pair.""" @@ -870,7 +873,7 @@ def get_setter_key(x: str) -> str: validator = _get_validator(opt, URI_OPTIONS_VALIDATOR_MAP, normed_key=normed_key) validated = validator(opt, value) except (ValueError, TypeError, ConfigurationError) as exc: - if warn: + if warn and opt not in _OPTION_MUST_RAISE: warnings.warn(str(exc), stacklevel=2) else: raise From 9e222141ca65a63ccde643e6b0c04eeee088f8f5 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 29 Oct 2024 18:43:03 -0500 Subject: [PATCH 5/6] fix test --- pymongo/common.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pymongo/common.py b/pymongo/common.py index 26b67df689..358b58e26d 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -139,8 +139,8 @@ # Default value for serverMonitoringMode SERVER_MONITORING_MODE = "auto" # poll/stream/auto -# Options that must raise an error instead of warning if they invalidate. -_OPTION_MUST_RAISE = ["CANONICALIZE_HOST_NAME"] +# Auth mechanism properties that must raise an error instead of warning if they invalidate. +_MECH_PROP_MUST_RAISE = ["CANONICALIZE_HOST_NAME"] def partition_node(node: str) -> tuple[str, int]: @@ -873,7 +873,9 @@ def get_setter_key(x: str) -> str: validator = _get_validator(opt, URI_OPTIONS_VALIDATOR_MAP, normed_key=normed_key) validated = validator(opt, value) except (ValueError, TypeError, ConfigurationError) as exc: - if warn and opt not in _OPTION_MUST_RAISE: + if normed_key == "authmechanismproperties" and _MECH_PROP_MUST_RAISE in str(exc): + raise + if warn: warnings.warn(str(exc), stacklevel=2) else: raise From 8f7372bc186e28c8b686f14cc762d7dc864de5b6 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 29 Oct 2024 20:35:06 -0500 Subject: [PATCH 6/6] fix test --- pymongo/common.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pymongo/common.py b/pymongo/common.py index 358b58e26d..d4601a0eb5 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -873,7 +873,9 @@ def get_setter_key(x: str) -> str: validator = _get_validator(opt, URI_OPTIONS_VALIDATOR_MAP, normed_key=normed_key) validated = validator(opt, value) except (ValueError, TypeError, ConfigurationError) as exc: - if normed_key == "authmechanismproperties" and _MECH_PROP_MUST_RAISE in str(exc): + if normed_key == "authmechanismproperties" and any( + p in str(exc) for p in _MECH_PROP_MUST_RAISE + ): raise if warn: warnings.warn(str(exc), stacklevel=2)