DRIVERS-2915 OIDC: Disallow comma character in authMechanismProperties connection string value (#1588)

* DRIVERS-2915 OIDC: Disallow comma character in authMechanismProperties connection string value

* changelog

* restore test without comma

* Update source/auth/auth.md

Co-authored-by: Jeff Yemin <[email protected]>

* Update source/auth/tests/legacy/connection-string.json

Co-authored-by: Durran Jordan <[email protected]>

* update connection string logic

* Update source/connection-string/connection-string-spec.md

Co-authored-by: Maxim Katcharov <[email protected]>

* clarify handling of key-value pairs and add specification test

* Update source/connection-string/tests/valid-options.yml

Co-authored-by: Maxim Katcharov <[email protected]>

* update json test

* Add a warning test

* address review

* Update source/connection-string/connection-string-spec.md

Co-authored-by: Maxim Katcharov <[email protected]>

* Update source/connection-string/connection-string-spec.md

Co-authored-by: Maxim Katcharov <[email protected]>

* Update source/connection-string/connection-string-spec.md

Co-authored-by: Maxim Katcharov <[email protected]>

* formatting

---------

Co-authored-by: Jeff Yemin <[email protected]>
Co-authored-by: Durran Jordan <[email protected]>
Co-authored-by: Maxim Katcharov <[email protected]>

Author: 4 people

source/auth/auth.md

@@ -1201,15 +1201,6 @@ in the MONGODB-OIDC specification, including sections or blocks that specificall
 
 #### [MongoCredential](#mongocredential) Properties
 
-> [!NOTE]
-> Drivers MUST NOT url-decode the entire `authMechanismProperties` given in an connection string when the
-> `authMechanism` is `MONGODB-OIDC`. This is because the `TOKEN_RESOURCE` itself will typically be a URL and may contain
-> a `,` character. The values of the individual `authMechanismProperties` MUST still be url-decoded when given as part
-> of the connection string, and MUST NOT be url-decoded when not given as part of the connection string, such as through
-> a `MongoClient` or `Credential` property. Drivers MUST parse the `TOKEN_RESOURCE` by splitting only on the first `:`
-> character. Drivers MUST document that users must url-encode `TOKEN_RESOURCE` when it is provided in the connection
-> string and it contains and of the special characters in \[`,`, `+`, `&`, `%`\].
-
 - username\
   MAY be specified. Its meaning varies depending on the OIDC provider integration used.
 
@@ -1233,7 +1224,9 @@ in the MONGODB-OIDC specification, including sections or blocks that specificall
   - TOKEN_RESOURCE\
     The URI of the target resource. If `TOKEN_RESOURCE` is provided and `ENVIRONMENT` is not one of
     `["azure", "gcp"]` or `TOKEN_RESOURCE` is not provided and `ENVIRONMENT` is one of `["azure", "gcp"]`, the driver
-    MUST raise an error.
+    MUST raise an error. Note: because the `TOKEN_RESOURCE` is often itself a URL, drivers MUST document that a
+    `TOKEN_RESOURCE` with a comma `,` must be given as a `MongoClient` configuration and not as part of the connection
+    string, and that the `TOKEN_RESOURCE` value can contain a colon `:` character.
 
   - OIDC_CALLBACK\
     An [OIDC Callback](#oidc-callback) that returns OIDC credentials. Drivers MAY allow the user to
@@ -2049,6 +2042,8 @@ to EC2 instance metadata in ECS, for security reasons, Amazon states it's best p
 
 ## Changelog
 
+- 2024-05-29: Disallow comma character when `TOKEN_RESOURCE` is given in a connection string.
+
 - 2024-05-03: Clarify timeout behavior for OIDC machine callback. Add `serverless:forbid` to OIDC unified tests. Add an
   additional prose test for the behavior of `ALLOWED_HOSTS`.
 

source/auth/tests/legacy/connection-string.json

@@ -435,7 +435,7 @@ tests:
       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
+  uri: mongodb://user@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:abcd%25ef%3Ag%26hi
   valid: true
   credential:
     username: user
@@ -444,7 +444,7 @@ tests:
     mechanism: MONGODB-OIDC
     mechanism_properties:
       ENVIRONMENT: azure
-      TOKEN_RESOURCE: 'abc,d%ef:g&hi'
+      TOKEN_RESOURCE: 'abcd%ef:g&hi'
 - description: should url-encode a TOKEN_RESOURCE (MONGODB-OIDC)
   uri: mongodb://user@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:azure,TOKEN_RESOURCE:a$b
   valid: true

source/auth/tests/legacy/connection-string.yml

@@ -178,7 +178,7 @@ source the `secrets-export.sh` file and use the associated env variables in your
 - Assert that the callback was called 2 times.
 - Close the client.
 
-\*\*4.3 Write Commands Fail If Reauthentication Fails
+#### 4.3 Write Commands Fail If Reauthentication Fails
 
 - Create a `MongoClient` whose OIDC callback returns one good token and then bad tokens after the first call.
 - Perform an `insert` operation that succeeds.

source/auth/tests/mongodb-oidc.md

@@ -216,13 +216,26 @@ The values in connection options MUST be URL decoded by the parser. The values c
   ```
 
 - Key value pairs: A value that represents one or more key and value pairs. Multiple key value pairs are delimited by a
-  comma (","). The key is everything up to the first colon sign (":") and the value is everything afterwards. If any
-  keys or values containing a comma (",") or a colon (":") they must be URL encoded. For example:
+  comma (","). The key is everything up to the first colon sign (":") and the value is everything afterwards.
+
+  For example:
 
   ```
   ?readPreferenceTags=dc:ny,rack:1
   ```
 
+  Drivers MUST handle unencoded colon signs (":") within the value. For example, given the connection string:
+
+  ```
+  ?authMechanismProperties=TOKEN_RESOURCE:mongodb://foo
+  ```
+
+  the driver MUST interpret the key as `TOKEN_RESOURCE` and the value as `mongodb://foo`.
+
+  For any option key-value pair that may contain a comma (such as `TOKEN_RESOURCE`), drivers MUST document that: a value
+  containing a comma (",") MUST NOT be provided as part of the connection string. This prevents use of values that would
+  interfere with parsing.
+
 Any invalid Values for a given key MUST be ignored and MUST log a WARN level message. For example:
 
 ```
@@ -444,6 +457,8 @@ many languages treat strings as `x-www-form-urlencoded` data by default.
 
 ## Changelog
 
+- 2024-05-29: Clarify handling of key-value pairs and add specification test.
+
 - 2024-02-15: Migrated from reStructuredText to Markdown.
 
 - 2016-07-22: In Port section, clarify that zero is not an acceptable port.

source/connection-string/connection-string-spec.md

@@ -249,5 +249,3 @@ tests:
         hosts: ~
         auth: ~
         options: ~
-
-    

source/connection-string/tests/invalid-uris.yml

@@ -28,3 +28,17 @@ tests:
         auth: ~
         options:
               tls: true
+    -
+        description: Colon in a key value pair
+        uri: mongodb://example.com?authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster
+        valid: true
+        warning: false
+        hosts:
+            - 
+                type: hostname
+                host: example.com
+                port: ~
+        auth: ~
+        options:
+        authmechanismProperties:
+            TOKEN_RESOURCE: 'mongodb://test-cluster'

source/connection-string/tests/valid-options.json

@@ -73,3 +73,15 @@ tests:
                 port: ~
         auth: ~
         options: ~
+    -
+        description: Comma in a key value pair causes a warning
+        uri: mongodb://example.com?authMechanismProperties=TOKEN_RESOURCE:mongodb://host1%2Chost2
+        valid: true
+        warning: true
+        hosts:
+            -
+                type: "hostname"
+                host: "localhost"
+                port: ~
+        auth: ~
+        options: ~