Merge pull request #1012

mongodb · Aug 26, 2019 · 929e2ca · 929e2ca
2 parents e65d114 + 2f94d95
commit 929e2ca
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 17 deletions.
diff --git a/php_phongo.c b/php_phongo.c
@@ -1461,6 +1461,45 @@ static const char* php_phongo_bson_type_to_string(bson_type_t type) /* {{{ */
 		bson_iter_key(&(iter)),                        \
 		php_phongo_bson_type_to_string(bson_iter_type(&(iter))))
 
+static bool php_phongo_uri_finalize_auth(mongoc_uri_t* uri TSRMLS_DC) /* {{{ */
+{
+	/* authSource with GSSAPI or X509 should always be external */
+	if (mongoc_uri_get_auth_mechanism(uri)) {
+		if (!strcasecmp(mongoc_uri_get_auth_mechanism(uri), "GSSAPI") ||
+			!strcasecmp(mongoc_uri_get_auth_mechanism(uri), "MONGODB-X509")) {
+			const char *source = mongoc_uri_get_auth_source(uri);
+
+			if (source) {
+				if (strcasecmp(source, "$external")) {
+					phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Failed to parse URI options: GSSAPI and X509 require \"$external\" authSource.");
+					return false;
+				}
+			} else {
+				mongoc_uri_set_auth_source(uri, "$external");
+			}
+		}
+
+		/* MONGODB-X509 is the only mechanism that doesn't require username */
+		if (strcasecmp(mongoc_uri_get_auth_mechanism(uri), "MONGODB-X509")) {
+			if (!mongoc_uri_get_username(uri) ||
+				!strcmp(mongoc_uri_get_username(uri), "")) {
+				phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Failed to parse URI options: '%s' authentication mechanism requires username.", mongoc_uri_get_auth_mechanism(uri));
+				return false;
+			}
+		}
+
+		/* MONGODB-X509 errors if a password is supplied. */
+		if (!strcasecmp(mongoc_uri_get_auth_mechanism(uri), "MONGODB-X509")) {
+			if (mongoc_uri_get_password(uri)) {
+				phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Failed to parse URI options: X509 authentication mechanism does not accept a password.");
+				return false;
+			}
+		}
+	}
+
+	return true;
+} /* }}} */
+
 static bool php_phongo_apply_options_to_uri(mongoc_uri_t* uri, bson_t* options TSRMLS_DC) /* {{{ */
 {
 	bson_iter_t iter;
@@ -1657,6 +1696,12 @@ static bool php_phongo_apply_options_to_uri(mongoc_uri_t* uri, bson_t* options T
 		}
 	}
 
+	// Finalize auth options
+	if (!php_phongo_uri_finalize_auth(uri TSRMLS_CC)) {
+		/* Exception should already have been thrown */
+		return false;
+	}
+
 	return true;
 } /* }}} */
 

diff --git a/tests/connect/bug1045.phpt b/tests/connect/bug1045.phpt
@@ -2,27 +2,21 @@
 PHPC-1045: Segfault if username is not provided for SCRAM-SHA-1 authMechanism
 --SKIPIF--
 <?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
-<?php skip_if_not_libmongoc_ssl(); ?>
-<?php skip_if_not_live(); ?>
-<?php skip_if_auth(); ?>
-<?php skip_if_not_clean(); ?>
+<?php skip_if_not_libmongoc_crypto(); ?>
 --FILE--
 <?php
 
 require_once __DIR__ . "/../utils/basic.inc";
 
-// URI may or may not support auth, but that is not necessary for the test
-$m = new MongoDB\Driver\Manager(URI, ['authMechanism' => 'SCRAM-SHA-1']);
-
-// Execute a basic ping command to trigger connection initialization
-echo throws(function() use ($m) {
-    $m->executeCommand('admin', new MongoDB\Driver\Command(['ping'=>1]));
-}, 'MongoDB\Driver\Exception\RuntimeException'), "\n";
+echo throws(function() {
+    // URI may or may not support auth, but that is not necessary for the test
+    new MongoDB\Driver\Manager('mongodb://127.0.0.1/', ['authMechanism' => 'SCRAM-SHA-1']);
+}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
 
 ?>
 ===DONE===
 <?php exit(0); ?>
 --EXPECTF--
-OK: Got MongoDB\Driver\Exception\RuntimeException
-SCRAM Failure: username is not set
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse URI options: 'SCRAM-SHA-1' authentication mechanism requires username.
 ===DONE===
diff --git a/tests/manager/manager-ctor-auth_mechanism-001.phpt b/tests/manager/manager-ctor-auth_mechanism-001.phpt
@@ -5,9 +5,11 @@ MongoDB\Driver\Manager::__construct(): authMechanism option
 
 $tests = [
     ['mongodb://username@127.0.0.1/?authMechanism=MONGODB-X509', []],
+    ['mongodb://127.0.0.1/?authMechanism=MONGODB-X509', []],
     ['mongodb://username@127.0.0.1/?authMechanism=GSSAPI', []],
+    [null, ['authMechanism' => 'MONGODB-X509', 'username' => 'username']],
     [null, ['authMechanism' => 'MONGODB-X509']],
-    [null, ['authMechanism' => 'GSSAPI']],
+    [null, ['authMechanism' => 'GSSAPI', 'username' => 'username']],
 ];
 
 foreach ($tests as $test) {

diff --git a/tests/manager/manager-ctor-auth_mechanism-002.phpt b/tests/manager/manager-ctor-auth_mechanism-002.phpt
@@ -5,12 +5,12 @@ MongoDB\Driver\Manager::__construct(): authMechanismProperties option
 
 $tests = [
     ['mongodb://username@127.0.0.1/?authMechanism=GSSAPI&authMechanismProperties=CANONICALIZE_HOST_NAME:true,SERVICE_NAME:foo,SERVICE_REALM:bar', []],
-    [null, ['authMechanism' => 'GSSAPI', 'authMechanismProperties' => ['CANONICALIZE_HOST_NAME' => 'true', 'SERVICE_NAME' => 'foo', 'SERVICE_REALM' => 'bar']]],
+    [null, ['username' => 'username', 'authMechanism' => 'GSSAPI', 'authMechanismProperties' => ['CANONICALIZE_HOST_NAME' => 'true', 'SERVICE_NAME' => 'foo', 'SERVICE_REALM' => 'bar']]],
     // Options are case-insensitive
     ['mongodb://username@127.0.0.1/?authMechanism=GSSAPI&authMechanismProperties=canonicalize_host_name:TRUE,service_name:foo,service_realm:bar', []],
-    [null, ['authMechanism' => 'GSSAPI', 'authMechanismProperties' => ['canonicalize_host_name' => 'TRUE', 'service_name' => 'foo', 'service_realm' => 'bar']]],
+    [null, ['username' => 'username', 'authMechanism' => 'GSSAPI', 'authMechanismProperties' => ['canonicalize_host_name' => 'TRUE', 'service_name' => 'foo', 'service_realm' => 'bar']]],
     // Boolean true "CANONICALIZE_HOST_NAME" value is converted to "true"
-    [null, ['authMechanism' => 'GSSAPI', 'authMechanismProperties' => ['canonicalize_host_name' => true]]],
+    [null, ['username' => 'username', 'authMechanism' => 'GSSAPI', 'authMechanismProperties' => ['canonicalize_host_name' => true]]],
 ];
 
 foreach ($tests as $test) {

diff --git a/tests/manager/manager-ctor-auth_mechanism-error-001.phpt b/tests/manager/manager-ctor-auth_mechanism-error-001.phpt
@@ -0,0 +1,54 @@
+--TEST--
+MongoDB\Driver\Manager::__construct(): authentication options are validated
+--FILE--
+<?php
+
+require_once __DIR__ . '/../utils/tools.php';
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://localhost:27017/?authMechanism=GSSAPI&authSource=admin');
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://localhost:27017/', ['authMechanism' => 'GSSAPI', 'authSource' => 'admin']);
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://localhost:27017/?authMechanism=MONGODB-X509&authSource=admin');
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://localhost:27017/', ['authMechanism' => 'MONGODB-X509', 'authSource' => 'admin']);
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://@localhost:27017/?authMechanism=SCRAM-SHA-1');
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://localhost:27017/', ['username' => '', 'authMechanism' => 'SCRAM-SHA-1']);
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+echo throws(function() {
+    new MongoDB\Driver\Manager('mongodb://localhost:27017/', ['password' => 'password', 'authMechanism' => 'MONGODB-X509']);
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECT--
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse MongoDB URI: 'mongodb://localhost:27017/?authMechanism=GSSAPI&authSource=admin'. GSSAPI and X509 require "$external" authSource.
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse URI options: GSSAPI and X509 require "$external" authSource.
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse MongoDB URI: 'mongodb://localhost:27017/?authMechanism=MONGODB-X509&authSource=admin'. GSSAPI and X509 require "$external" authSource.
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse URI options: GSSAPI and X509 require "$external" authSource.
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse MongoDB URI: 'mongodb://@localhost:27017/?authMechanism=SCRAM-SHA-1'. 'SCRAM-SHA-1' authentication mechanism requires username.
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse URI options: 'SCRAM-SHA-1' authentication mechanism requires username.
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Failed to parse URI options: X509 authentication mechanism does not accept a password.
+===DONE===