CDRIVER-4489 revert aggressive validation of credential field requirements (#1952)

Author: eramongodb

Diff for: NEWS

@@ -10,9 +10,13 @@ Unreleased (2.0.0)
 * `mongoc_server_description_host` changes the return type from `mongoc_host_list_t *` to `const mongoc_host_list_t *`.
 * URI authentication credentials validation (only applicable during creation of a new `mongoc_uri_t` object from a connection string):
   * The requirement that a username is non-empty when specified is now enforced regardless of authentication mechanism.
+  * Username and password specification requirements are now validated and returns a client error for the specified authentication mechanism.
+    * e.g. it is a client error to not specify a username or a password for SCRAM-SHA-1, SCRAM-SHA-256, and PLAIN.
+    * e.g. it is a client error to specify a password for MONGODB-X509.
+    * e.g. it is a client error to specify a username or a password without the other for MONGODB-AWS.
   * `authSource` is now correctly defaulted to `"$external"` for MONGODB-AWS (instead of the database name or `"admin"`).
   * `authMechanism` is now validated and returns a client error for invalid or unsupported values.
-  * Requirements for the inclusion, exclusion, and supported values of authentication-related URI components (e.g. username and password), options (e.g. `authSource`), and mechanism properties (e.g. `authMechanismProperties` and its key-value pairs) are now validated and return a client error when able for invalid or unsupported configurations according to the specified authentication mechanism (`authMechanism`).
+  * `authMechanismProperties` is now validated and returns a client error for invalid or unsupported properties for the specified authentication mechanism.
   * `authMechanismProperties` now correctly supports `':'` within property values.
     * Old behavior: `authMechanismProperties=A:B,C:D:E,F:G` is parsed as `{'A': 'B', 'C': 'D:E,F:G'}`.
     * New behavior: `authMechanismProperties=A:B,C:D:E,F:G` is parsed as `{'A': 'B': 'C': 'D:E', 'F': 'G'}`.

Diff for: src/libmongoc/src/mongoc/mongoc-uri.c

@@ -1360,14 +1360,17 @@ _finalize_auth_username (const char *username,
 }
 
 static bool
-_finalize_auth_source_external_required (const char *source, const char *mechanism, bson_error_t *error)
+_finalize_auth_source_external (const char *source, const char *mechanism, bson_error_t *error)
 {
    BSON_OPTIONAL_PARAM (source);
    BSON_ASSERT_PARAM (mechanism);
    BSON_OPTIONAL_PARAM (error);
 
-   if (!source || strcasecmp (source, "$external") != 0) {
-      MONGOC_URI_ERROR (error, "'%s' authentication mechanism requires \"$external\" authSource", mechanism);
+   if (source && strcasecmp (source, "$external") != 0) {
+      MONGOC_URI_ERROR (error,
+                        "'%s' authentication mechanism requires \"$external\" authSource, but \"%s\" was specified",
+                        mechanism,
+                        source);
       return false;
    }
 
@@ -1408,23 +1411,6 @@ _finalize_auth_password (const char *password,
    return true;
 }
 
-static bool
-_finalize_auth_mechanism_properties_prohibited (const bson_t *mechanism_properties,
-                                                const char *mechanism,
-                                                bson_error_t *error)
-{
-   BSON_OPTIONAL_PARAM (mechanism_properties);
-   BSON_ASSERT_PARAM (mechanism);
-   BSON_OPTIONAL_PARAM (error);
-
-   if (mechanism_properties) {
-      MONGOC_URI_ERROR (error, "'%s' authentication mechanism does not accept mechanism properties", mechanism);
-      return false;
-   }
-
-   return true;
-}
-
 typedef struct __supported_mechanism_properties {
    const char *name;
    bson_type_t type;
@@ -1536,6 +1522,13 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
    BSON_ASSERT_PARAM (uri);
    BSON_OPTIONAL_PARAM (error);
 
+   // Most validation of MongoCredential fields below according to the Authentication spec must be deferred to the
+   // implementation of the Authentication Handshake algorithm (i.e. `_mongoc_cluster_auth_node`) due to support for
+   // partial and late setting of credential fields via `mongoc_uri_set_*` functions. Limit validation to requirements
+   // for individual field which are explicitly specified. Do not validate requirements on fields in relation to one
+   // another (e.g. "given field A, field B must..."). The username, password, and authSource credential fields are
+   // exceptions to this rule for both backward compatibility and spec test compliance.
+
    bool ret = false;
 
    bson_iter_t iter;
@@ -1591,9 +1584,7 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
          goto fail;
       }
 
-      // Defer validation of `MongoCredential` fields to `_mongoc_cluster_auth_node` during handshake according to
-      // `saslSupportedMechs`. Defaults for `MongoCredential.source` is handled by `mongoc_uri_get_auth_source` for
-      // backward compatibility.
+      // Defer remaining validation of `MongoCredential` fields to Authentication Handshake.
    }
 
    // SCRAM-SHA-1, SCRAM-SHA-256, and PLAIN (same validation requirements)
@@ -1604,21 +1595,12 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
          goto fail;
       }
 
-      // Authentication spec: source: MUST be specified. Defaults to the database name if
-      // supplied on the connection string or:
-      //   - "admin" (for SCRAM-SHA-1 and SCRAM-SHA-256).
-      //   - "$external" (for PLAIN).
-      // Handled by `mongoc_uri_get_auth_source` for backward compatibility.
-
       // Authentication spec: password: MUST be specified.
       if (!_finalize_auth_password (password, mechanism, _mongoc_uri_finalize_required, error)) {
          goto fail;
       }
 
-      // Authentication spec: mechanism_properties: MUST NOT be specified.
-      if (!_finalize_auth_mechanism_properties_prohibited (mechanism_properties, mechanism, error)) {
-         goto fail;
-      }
+      // Defer remaining validation of `MongoCredential` fields to Authentication Handshake.
    }
 
    // MONGODB-X509
@@ -1630,6 +1612,11 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
          goto fail;
       }
 
+      // Authentication spec: password: MUST NOT be specified.
+      if (!_finalize_auth_password (password, mechanism, _mongoc_uri_finalize_prohibited, error)) {
+         goto fail;
+      }
+
       // Authentication spec: source: MUST be "$external" and defaults to "$external".
       if (!source) {
          bsonBuildAppend (uri->credentials, kv (MONGOC_URI_AUTHSOURCE, cstr ("$external")));
@@ -1640,19 +1627,11 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
                               bsonBuildError);
             goto fail;
          }
-      } else if (!_finalize_auth_source_external_required (source, mechanism, error)) {
+      } else if (!_finalize_auth_source_external (source, mechanism, error)) {
          goto fail;
       }
 
-      // Authentication spec: password: MUST NOT be specified.
-      if (!_finalize_auth_password (password, mechanism, _mongoc_uri_finalize_prohibited, error)) {
-         goto fail;
-      }
-
-      // Authentication spec: mechanism_properties: MUST NOT be specified.
-      if (!_finalize_auth_mechanism_properties_prohibited (mechanism_properties, mechanism, error)) {
-         goto fail;
-      }
+      // Defer remaining validation of `MongoCredential` fields to Authentication Handshake.
    }
 
    // GSSAPI
@@ -1672,7 +1651,7 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
                               bsonBuildError);
             goto fail;
          }
-      } else if (!_finalize_auth_source_external_required (source, mechanism, error)) {
+      } else if (!_finalize_auth_source_external (source, mechanism, error)) {
          goto fail;
       }
 
@@ -1721,6 +1700,8 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
             goto fail;
          }
       }
+
+      // Defer remaining validation of `MongoCredential` fields to Authentication Handshake.
    }
 
    // MONGODB-AWS
@@ -1740,7 +1721,7 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
                               bsonBuildError);
             goto fail;
          }
-      } else if (!_finalize_auth_source_external_required (source, mechanism, error)) {
+      } else if (!_finalize_auth_source_external (source, mechanism, error)) {
          goto fail;
       }
 
@@ -1762,17 +1743,7 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, bson_error_t *error)
          goto fail;
       }
 
-      // Authentication spec: if *only* a session token is provided, Drivers MUST raise an error.
-      if (!username && mechanism_properties) {
-         bson_iter_t iter;
-         if (bson_iter_init_find_case (&iter, mechanism_properties, "AWS_SESSION_TOKEN")) {
-            MONGOC_URI_ERROR (error,
-                              "%s",
-                              "'MONGODB-AWS' authentication mechanism requires AWS_SESSION_TOKEN to be accompanied by "
-                              "a username and a password");
-            goto fail;
-         }
-      }
+      // Defer remaining validation of `MongoCredential` fields to Authentication Handshake.
    }
 
    // Invalid or unsupported authentication mechanism.

Diff for: src/libmongoc/tests/test-mongoc-aws.c

@@ -28,9 +28,9 @@ test_obtain_credentials (void *unused)
    BSON_UNUSED (unused);
 
    /* A username specified with a password is parsed correctly. */
-   uri = mongoc_uri_new_with_error ("mongodb://access_key_id:secret_access_key@localhost/", &error);
+   uri = mongoc_uri_new_with_error ("mongodb://access_key_id:secret_access_key@localhost/?authMechanism=MONGODB-AWS",
+                                    &error);
    ASSERT_OR_PRINT (uri, error);
-   ASSERT (mongoc_uri_set_auth_mechanism (uri, "MONGODB-AWS"));
    ret = _mongoc_aws_credentials_obtain (uri, &creds, &error);
    ASSERT_OR_PRINT (ret, error);
    ASSERT_CMPSTR (creds.access_key_id, "access_key_id");
@@ -40,9 +40,8 @@ test_obtain_credentials (void *unused)
    mongoc_uri_destroy (uri);
 
    /* A username specified with no password is an error. */
-   uri = mongoc_uri_new_with_error ("mongodb://access_key_id:@localhost/", &error);
+   uri = mongoc_uri_new_with_error ("mongodb://access_key_id:@localhost/?authMechanism=MONGODB-AWS", &error);
    ASSERT_OR_PRINT (uri, error);
-   ASSERT (mongoc_uri_set_auth_mechanism (uri, "MONGODB-AWS"));
    ret = _mongoc_aws_credentials_obtain (uri, &creds, &error);
    ASSERT (!ret);
    ASSERT_ERROR_CONTAINS (error,
@@ -53,9 +52,9 @@ test_obtain_credentials (void *unused)
    mongoc_uri_destroy (uri);
 
    /* Password not set at all (not empty string) */
-   uri = mongoc_uri_new_with_error ("mongodb://access_key_id@localhost/", &error);
+   uri = mongoc_uri_new_with_error ("mongodb://localhost/?authMechanism=MONGODB-AWS", &error);
+   ASSERT (mongoc_uri_set_username (uri, "access_key_id"));
    ASSERT_OR_PRINT (uri, error);
-   ASSERT (mongoc_uri_set_auth_mechanism (uri, "MONGODB-AWS"));
    ret = _mongoc_aws_credentials_obtain (uri, &creds, &error);
    ASSERT (!ret);
    ASSERT_ERROR_CONTAINS (error,
@@ -67,10 +66,10 @@ test_obtain_credentials (void *unused)
 
    /* A session token may be set through the AWS_SESSION_TOKEN auth mechanism
     * property */
-   uri = mongoc_uri_new_with_error (
-      "mongodb://access_key_id:secret_access_key@localhost/?authMechanismProperties=AWS_SESSION_TOKEN:token", &error);
+   uri = mongoc_uri_new_with_error ("mongodb://access_key_id:secret_access_key@localhost/"
+                                    "?authMechanism=MONGODB-AWS&authMechanismProperties=AWS_SESSION_TOKEN:token",
+                                    &error);
    ASSERT_OR_PRINT (uri, error);
-   ASSERT (mongoc_uri_set_auth_mechanism (uri, "MONGODB-AWS"));
    ret = _mongoc_aws_credentials_obtain (uri, &creds, &error);
    ASSERT_OR_PRINT (ret, error);
    ASSERT_CMPSTR (creds.access_key_id, "access_key_id");
@@ -80,9 +79,9 @@ test_obtain_credentials (void *unused)
    mongoc_uri_destroy (uri);
 
    /* A session token in the URI with no username/password is an error. */
-   uri = mongoc_uri_new_with_error ("mongodb://localhost/?authMechanismProperties=AWS_SESSION_TOKEN:token", &error);
+   uri = mongoc_uri_new_with_error (
+      "mongodb://localhost/?authMechanism=MONGODB-AWS&authMechanismProperties=AWS_SESSION_TOKEN:token", &error);
    ASSERT_OR_PRINT (uri, error);
-   ASSERT (mongoc_uri_set_auth_mechanism (uri, "MONGODB-AWS"));
    ret = _mongoc_aws_credentials_obtain (uri, &creds, &error);
    ASSERT (!ret);
    ASSERT_ERROR_CONTAINS (error,