diff --git a/.travis.yml b/.travis.yml index 86cc31ae..b6612431 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: required os: linux -dist: trusty +dist: bionic group: edge diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 6f7f2185..9d169513 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -13,6 +13,9 @@ variables: trigger: - dev +pr: +- dev + jobs: - job: macOS pool: @@ -30,8 +33,8 @@ jobs: - script: | brew tap brew tap homebrew/core - brew install autoconf automake libtool - brew install php@$(phpVersion) + brew reinstall autoconf automake libtool + brew reinstall php@$(phpVersion) php -v displayName: 'Install PHP' diff --git a/source/pdo_sqlsrv/pdo_parser.cpp b/source/pdo_sqlsrv/pdo_parser.cpp index 1d0b7299..74db829e 100644 --- a/source/pdo_sqlsrv/pdo_parser.cpp +++ b/source/pdo_sqlsrv/pdo_parser.cpp @@ -187,7 +187,7 @@ void conn_string_parser::add_key_value_pair( _In_reads_(len) const char* value, memcpy_s( option, len + 1, value, len ); option[len] = '\0'; - valid = core_is_authentication_option_valid( option, len ); + valid = AzureADOptions::isAuthValid(option, len); } } if( !valid ) { diff --git a/source/pdo_sqlsrv/pdo_util.cpp b/source/pdo_sqlsrv/pdo_util.cpp index 4d76cccb..02fbee87 100644 --- a/source/pdo_sqlsrv/pdo_util.cpp +++ b/source/pdo_sqlsrv/pdo_util.cpp @@ -379,7 +379,7 @@ pdo_error PDO_ERRORS[] = { }, { PDO_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION, - { IMSSP, (SQLCHAR*) "Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.", -73, false } + { IMSSP, (SQLCHAR*) "Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectorySPA is supported.", -73, false } }, { SQLSRV_ERROR_CE_DRIVER_REQUIRED, diff --git a/source/shared/core_conn.cpp b/source/shared/core_conn.cpp index 3b43f2ba..8c90461c 100644 --- a/source/shared/core_conn.cpp +++ b/source/shared/core_conn.cpp @@ -709,18 +709,38 @@ bool core_is_conn_opt_value_escaped( _Inout_ const char* value, _Inout_ size_t v return true; } -// core_is_authentication_option_valid -// if the option for the authentication is valid, returns true. This returns false otherwise. -bool core_is_authentication_option_valid( _In_z_ const char* value, _In_ size_t value_len) -{ - if (value_len <= 0) - return false; +namespace AzureADOptions { + enum AAD_AUTH_TYPE { + MIN_AAD_AUTH_TYPE = 0, + SQL_PASSWORD = 0, + AAD_PASSWORD, + AAD_MSI, + AAD_SPA, + MAX_AAD_AUTH_TYPE + }; - if (!stricmp(value, AzureADOptions::AZURE_AUTH_SQL_PASSWORD) || !stricmp(value, AzureADOptions::AZURE_AUTH_AD_PASSWORD) || !stricmp(value, AzureADOptions::AZURE_AUTH_AD_MSI)) { - return true; + const char *AADAuths[] = { "SqlPassword", "ActiveDirectoryPassword", "ActiveDirectoryMsi", "ActiveDirectorySPA" }; + + bool isAuthValid(_In_z_ const char* value, _In_ size_t value_len) + { + if (value_len <= 0) + return false; + + bool isValid = false; + for (short i = MIN_AAD_AUTH_TYPE; i < MAX_AAD_AUTH_TYPE && !isValid; i++) + { + if (!stricmp(value, AADAuths[i])) { + isValid = true; + } + } + + return isValid; } - return false; + bool isAADMsi(_In_z_ const char* value) + { + return (value != NULL && !stricmp(value, AADAuths[AAD_MSI])); + } } @@ -789,9 +809,9 @@ void build_connection_string_and_set_conn_attr( _Inout_ sqlsrv_conn* conn, _Inou option = Z_STRVAL_P(auth_option); } - if (option != NULL && !stricmp(option, AzureADOptions::AZURE_AUTH_AD_MSI)) { - activeDirectoryMSI = true; - + //if (option != NULL && !stricmp(option, AzureADOptions::AZURE_AUTH_AD_MSI)) { + activeDirectoryMSI = AzureADOptions::isAADMsi(option); + if (activeDirectoryMSI) { // There are two types of managed identities: // (1) A system-assigned managed identity: UID must be NULL // (2) A user-assigned managed identity: UID defined but must not be an empty string diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 6907e2c7..f6a6809f 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -191,9 +191,8 @@ const int SQL_SERVER_2008_DEFAULT_DATETIME_PRECISION = 34; const int SQL_SERVER_2008_DEFAULT_DATETIME_SCALE = 7; namespace AzureADOptions { - const char AZURE_AUTH_SQL_PASSWORD[] = "SqlPassword"; - const char AZURE_AUTH_AD_PASSWORD[] = "ActiveDirectoryPassword"; - const char AZURE_AUTH_AD_MSI[] = "ActiveDirectoryMsi"; + bool isAuthValid(_In_z_ const char* value, _In_ size_t value_len); + bool isAADMsi(_In_z_ const char* value); } // the message returned by ODBC Driver for SQL Server @@ -1288,7 +1287,6 @@ void core_sqlsrv_get_server_version( _Inout_ sqlsrv_conn* conn, _Inout_ zval *se void core_sqlsrv_get_client_info( _Inout_ sqlsrv_conn* conn, _Out_ zval *client_info ); bool core_is_conn_opt_value_escaped( _Inout_ const char* value, _Inout_ size_t value_len ); size_t core_str_zval_is_true( _Inout_ zval* str_zval ); -bool core_is_authentication_option_valid( _In_z_ const char* value, _In_ size_t value_len ); bool core_search_odbc_driver_unix( _In_ DRIVER_VERSION driver_version ); bool core_compare_error_state( _In_ sqlsrv_conn* conn, _In_ SQLRETURN r, _In_ const char* error_state ); diff --git a/source/sqlsrv/conn.cpp b/source/sqlsrv/conn.cpp index d5c61cfa..129312d7 100644 --- a/source/sqlsrv/conn.cpp +++ b/source/sqlsrv/conn.cpp @@ -1383,7 +1383,7 @@ int get_conn_option_key( _Inout_ sqlsrv_context& ctx, _In_ zend_string* key, _In bool valid = true; if( stricmp( SS_CONN_OPTS[i].sqlsrv_name, SSConnOptionNames::Authentication ) == 0 ) { - valid = core_is_authentication_option_valid( value, value_len ); + valid = AzureADOptions::isAuthValid(value, value_len); } CHECK_CUSTOM_ERROR( !valid, ctx, SS_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION, SS_CONN_OPTS[i].sqlsrv_name ) { diff --git a/source/sqlsrv/util.cpp b/source/sqlsrv/util.cpp index a5dd00bb..d956c457 100644 --- a/source/sqlsrv/util.cpp +++ b/source/sqlsrv/util.cpp @@ -365,7 +365,7 @@ ss_error SS_ERRORS[] = { }, { SS_SQLSRV_ERROR_INVALID_AUTHENTICATION_OPTION, - { IMSSP, (SQLCHAR*)"Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported.", -62, false } + { IMSSP, (SQLCHAR*)"Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectorySPA is supported.", -62, false } }, { SS_SQLSRV_ERROR_AE_QUERY_SQLTYPE_REQUIRED, diff --git a/test/functional/pdo_sqlsrv/MsSetup.inc b/test/functional/pdo_sqlsrv/MsSetup.inc index deea7659..5eb45906 100644 --- a/test/functional/pdo_sqlsrv/MsSetup.inc +++ b/test/functional/pdo_sqlsrv/MsSetup.inc @@ -24,6 +24,8 @@ $adServer = 'TARGET_AD_SERVER'; $adDatabase = 'TARGET_AD_DATABASE'; $adUser = 'TARGET_AD_USERNAME'; $adPassword = 'TARGET_AD_PASSWORD'; +$adSPClientId = 'TARGET_ADSP_CLIENT_ID'; +$adSPClientSecret = 'TARGET_ADSP_CLIENT_SECRET'; $driverType = true; $driver = "ODBC Driver 17 for SQL Server"; diff --git a/test/functional/pdo_sqlsrv/pdo_azure_ad_authentication.phpt b/test/functional/pdo_sqlsrv/pdo_azure_ad_authentication.phpt index d5cf54b0..8c06d4bb 100644 --- a/test/functional/pdo_sqlsrv/pdo_azure_ad_authentication.phpt +++ b/test/functional/pdo_sqlsrv/pdo_azure_ad_authentication.phpt @@ -96,5 +96,5 @@ if ($azureServer != 'TARGET_AD_SERVER') { Connected successfully with Authentication=SqlPassword. string(1) "%d" Could not connect with Authentication=ActiveDirectoryIntegrated. -SQLSTATE[IMSSP]: Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported. +SQLSTATE[IMSSP]: Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectorySPA is supported. %s with Authentication=ActiveDirectoryPassword. diff --git a/test/functional/pdo_sqlsrv/pdo_azure_ad_service_principal.phpt b/test/functional/pdo_sqlsrv/pdo_azure_ad_service_principal.phpt new file mode 100644 index 00000000..ba24b3ac --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_azure_ad_service_principal.phpt @@ -0,0 +1,117 @@ +--TEST-- +Test some basics of Azure AD service principal support +--SKIPIF-- +getAttribute(PDO::ATTR_CLIENT_VERSION)["DriverVer"]; + $msodbcsqlMaj = explode(".", $msodbcsqlVer)[0]; + $msodbcsqlMin = explode(".", $msodbcsqlVer)[1]; + + if ($msodbcsqlMaj < 17 || $msodbcsqlMin < 7) { + die("skip: Requires ODBC driver 17.7 or above"); + } +} catch (PDOException $e) { + die("skip: Failed to connect in skipif."); +} +?> +--FILE-- +query($query); + + // Insert one row + $query = "INSERT INTO $tableName VALUES ('$col1')"; + $stmt = $conn->query($query); + + // Fetch data + $query = "SELECT * FROM $tableName"; + $stmt = $conn->query($query); + + $result = $stmt->fetch(PDO::FETCH_NUM); + $id = $result[0]; + if ($id != 1) { + echo "AzureAD service principal test: fetched id $id unexpected\n"; + } + + $field = $result[1]; + if ($field !== $col1) { + echo "AzureAD service principal test: fetched value $field unexpected\n"; + } + + dropTable($conn, $tableName); +} + +function connectAzureDB($showException) +{ + global $adServer, $adDatabase, $adSPClientId, $adSPClientSecret, $maxAttempts; + + $conn = false; + try { + $connectionInfo = "Database = $adDatabase; Authentication = ActiveDirectorySPA;"; + $conn = new PDO("sqlsrv:server = $adServer; $connectionInfo", $adSPClientId, $adSPClientSecret); + } catch (PDOException $e) { + if ($showException) { + echo "Could not connect with Azure AD Service Principal after $maxAttempts retries.\n"; + print_r($e->getMessage()); + echo PHP_EOL; + } + } + + return $conn; +} + +// First test connecting to regular sql server +require_once('MsSetup.inc'); +try { + $conn = new PDO("sqlsrv:server = $server; Authentication = ActiveDirectorySPA;", $uid, $pwd); + echo "Expect regular connection to fail\n"; +} catch(PDOException $e) { + // do nothing +} + +// Next, test connecting with a valid service principal and perform some simple tasks +$maxAttempts = 3; + +try { + if ($adServer != 'TARGET_AD_SERVER' && $adSPClientId != 'TARGET_ADSP_CLIENT_ID') { + $conn = false; + $numAttempts = 0; + do { + $conn = connectAzureDB($numAttempts == ($maxAttempts - 1)); + if ($conn === false) { + $numAttempts++; + sleep(10); + } + } while ($conn === false && $numAttempts < $maxAttempts); + + // Proceed when successfully connected + if ($conn) { + $conn->setAttribute(PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE, true); + simpleTest($conn); + unset($conn); + } + } +} catch(PDOException $e) { + print_r($e->getMessage()); + echo PHP_EOL; +} + +echo "Done\n"; +?> +--EXPECT-- +Done \ No newline at end of file diff --git a/test/functional/sqlsrv/MsSetup.inc b/test/functional/sqlsrv/MsSetup.inc index f328e17c..7f4e62c4 100644 --- a/test/functional/sqlsrv/MsSetup.inc +++ b/test/functional/sqlsrv/MsSetup.inc @@ -31,6 +31,8 @@ $adServer = 'TARGET_AD_SERVER'; $adDatabase = 'TARGET_AD_DATABASE'; $adUser = 'TARGET_AD_USERNAME'; $adPassword = 'TARGET_AD_PASSWORD'; +$adSPClientId = 'TARGET_ADSP_CLIENT_ID'; +$adSPClientSecret = 'TARGET_ADSP_CLIENT_SECRET'; if (isset($_ENV['MSSQL_SERVER']) || isset($_ENV['MSSQL_USER']) || isset($_ENV['MSSQL_PASSWORD'])) { $server = $_ENV['MSSQL_SERVER']; diff --git a/test/functional/sqlsrv/sqlsrv_azure_ad_authentication.phpt b/test/functional/sqlsrv/sqlsrv_azure_ad_authentication.phpt index f02a6f62..90b9f0ab 100644 --- a/test/functional/sqlsrv/sqlsrv_azure_ad_authentication.phpt +++ b/test/functional/sqlsrv/sqlsrv_azure_ad_authentication.phpt @@ -106,7 +106,7 @@ Array [SQLSTATE] => IMSSP [1] => -62 [code] => -62 - [2] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported. - [message] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, or ActiveDirectoryMsi is supported. + [2] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectorySPA is supported. + [message] => Invalid option for the Authentication keyword. Only SqlPassword, ActiveDirectoryPassword, ActiveDirectoryMsi or ActiveDirectorySPA is supported. ) %s with Authentication=ActiveDirectoryPassword. diff --git a/test/functional/sqlsrv/sqlsrv_azure_ad_service_principal.phpt b/test/functional/sqlsrv/sqlsrv_azure_ad_service_principal.phpt new file mode 100644 index 00000000..e6daf0e2 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_azure_ad_service_principal.phpt @@ -0,0 +1,116 @@ +--TEST-- +Test some basics of Azure AD Service Principal support +--SKIPIF-- +$userName, "PWD"=>$userPassword, "Driver" => $driver); +$conn = sqlsrv_connect($server, $connectionInfo); +if ($conn === false) { + die("skip: Failed to connect in skipif."); +} + +$msodbcsqlVer = sqlsrv_client_info($conn)['DriverVer']; +$version = explode(".", $msodbcsqlVer); + +if ($version[0] < 17 || $version[1] < 7) { + die("skip: Requires ODBC driver 17.7 or above"); +} +?> +--FILE-- +$adDatabase, + "Authentication"=>"ActiveDirectorySPA", + "UID"=>$adSPClientId, + "PWD"=>$adSPClientSecret); + + $conn = sqlsrv_connect($adServer, $connectionInfo); + if ($conn === false) { + if ($showException) { + fatalError("Could not connect with Azure AD Service Principal after $maxAttempts retries.\n"); + } + } else { + simpleTest($conn); + + sqlsrv_close($conn); + } + + return $conn; +} + +// Try connecting to an invalid server. Expect this to fail. +$connectionInfo = array("Authentication"=>"ActiveDirectorySPA"); +$conn = sqlsrv_connect('invalidServer', $connectionInfo); +if ($conn) { + fatalError("AzureAD Service Principal test: expected to fail with invalidServer\n"); +} + +// Next, test connecting with Service Principal +$maxAttempts = 3; + +if ($adServer != 'TARGET_AD_SERVER' && $adSPClientId != 'TARGET_ADSP_CLIENT_ID') { + $conn = false; + $numAttempts = 0; + do { + $conn = connectAzureDB($numAttempts == ($maxAttempts - 1)); + if ($conn === false) { + $numAttempts++; + sleep(10); + } + } while ($conn === false && $numAttempts < $maxAttempts); +} + +echo "Done\n"; +?> +--EXPECT-- +Done \ No newline at end of file