From 3a6de2efb96204b2ea90cb03237bf9c3b6df9907 Mon Sep 17 00:00:00 2001 From: Adam Berlin Date: Mon, 9 Mar 2020 14:29:13 -0400 Subject: [PATCH] Fix gpinitsystem when setting password for numeric username gpinitsystem did not quote the username while performing ALTER USER. When the username is a numeric value the postgres parser gets upset - unless the username is quoted. See here for more details: https://www.postgresql.org/docs/9.4/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS - SQL identifiers and key words must begin with a letter (a-z, but also letters with diacritical marks and non-Latin letters) or an underscore (_). - Also, there is a second kind of identifier: the delimited identifier or quoted identifier. It is formed by enclosing an arbitrary sequence of characters in double-quotes (") - use variable interpolation provided by psql to properly quote user-provided values. - use RETVAL to perform testing due to Commit d7b7a40a87df49f53c62c55f87c84a1e6fbc2367 Co-authored-by: Jacob Champion (cherry picked from commit f188ecb526e4bcfab1e607a0fab440931b113817) --- gpMgmt/bin/Makefile | 7 ++- gpMgmt/bin/gpinitsystem | 7 --- gpMgmt/bin/lib/gp_bash_functions.sh | 15 ++++++ gpMgmt/bin/test/gpinitsystem_test.bash | 65 ++++++++++++++++++++++++++ gpMgmt/bin/test/suite.bash | 5 ++ 5 files changed, 91 insertions(+), 8 deletions(-) create mode 100755 gpMgmt/bin/test/gpinitsystem_test.bash create mode 100755 gpMgmt/bin/test/suite.bash diff --git a/gpMgmt/bin/Makefile b/gpMgmt/bin/Makefile index 8a4ba000f1..bd42bf822d 100644 --- a/gpMgmt/bin/Makefile +++ b/gpMgmt/bin/Makefile @@ -220,8 +220,13 @@ unitdevel: PYTHONPATH=$(SERVER_SRC):$(SERVER_SBIN):$(PYTHONPATH):$(PYTHONSRC_INSTALL_PYTHON_PATH):$(SRC)/ext:$(SBIN_DIR):$(LIB_DIR):$(PYLIB_DIR)/mock-1.0.1 \ python -m unittest discover --verbose -s $(SRC)/gppylib -p "test_unit*.py" + +.PHONY: installcheck-bash +installcheck-bash: + ./test/suite.bash + .PHONY: installcheck -installcheck: +installcheck: installcheck-bash $(MAKE) -C gpload_test $@ clean distclean: diff --git a/gpMgmt/bin/gpinitsystem b/gpMgmt/bin/gpinitsystem index 23acf63c9f..f63fe24bb5 100755 --- a/gpMgmt/bin/gpinitsystem +++ b/gpMgmt/bin/gpinitsystem @@ -1586,13 +1586,6 @@ START_QD_PRODUCTION () { LOG_MSG "[INFO]:-End Function $FUNCNAME" } -SET_GP_USER_PW () { - LOG_MSG "[INFO]:-Start Function $FUNCNAME" - $PSQL -p $MASTER_PORT -d "$DEFAULTDB" -c"alter user $USER_NAME password '$GP_PASSWD';" >> $LOG_FILE 2>&1 - ERROR_CHK $? "update Greenplum superuser password" 1 - LOG_MSG "[INFO]:-End Function $FUNCNAME" -} - CREATE_DATABASE () { LOG_MSG "[INFO]:-Start Function $FUNCNAME" SET_VAR $QD_PRIMARY_ARRAY diff --git a/gpMgmt/bin/lib/gp_bash_functions.sh b/gpMgmt/bin/lib/gp_bash_functions.sh index 354936091d..01419ba9d1 100755 --- a/gpMgmt/bin/lib/gp_bash_functions.sh +++ b/gpMgmt/bin/lib/gp_bash_functions.sh @@ -1297,6 +1297,21 @@ CHK_GPDB_ID () { LOG_MSG "[INFO]:-End Function $FUNCNAME" } +SET_GP_USER_PW () { + LOG_MSG "[INFO]:-Start Function $FUNCNAME" + + local alter_statement="alter user :\"username\" password :'password';" + + $PSQL --variable=ON_ERROR_STOP=1 \ + -p $MASTER_PORT \ + -d "$DEFAULTDB" \ + --variable=username="$USER_NAME" \ + --variable=password="$GP_PASSWD" <<< "$alter_statement" >> $LOG_FILE 2>&1 + + ERROR_CHK $? "update Greenplum superuser password" 1 + LOG_MSG "[INFO]:-End Function $FUNCNAME" +} + #****************************************************************************** # Main Section #****************************************************************************** diff --git a/gpMgmt/bin/test/gpinitsystem_test.bash b/gpMgmt/bin/test/gpinitsystem_test.bash new file mode 100755 index 0000000000..7177bfb4f4 --- /dev/null +++ b/gpMgmt/bin/test/gpinitsystem_test.bash @@ -0,0 +1,65 @@ +#!/usr/bin/env bash + +. lib/gp_bash_functions.sh + +__cleanupTestUsers() { + dropuser 123456 + dropuser abc123456 +} + +mimic_gpinitsystem_setup() { + # ensure MASTER_PORT is set, it is needed by SET_GP_USER_PW + GET_MASTER_PORT "$MASTER_DATA_DIRECTORY" + + # the return value set when performing ERROR_CHK + # on the status code returned from $PSQL + # + # set it to a default value + RETVAL=0 +} + +it_should_quote_the_username_during_alter_user_in_SET_GP_USER_PW() { + mimic_gpinitsystem_setup + + # given a user that is only a number + USER_NAME=123456 + createuser $USER_NAME + trap __cleanupTestUsers EXIT + + # when we run set user password + SET_GP_USER_PW + + # then it should succeed + if [ "$RETVAL" != "0" ]; then + local error_message="$(tail -n 10 "$LOG_FILE")" + echo "got an exit status of $RETVAL while running SET_GP_USER_PW for $USER_NAME, wanted success: $error_message" + exit 1 + fi +} + +it_should_quote_the_password_during_alter_user_in_SET_GP_USER_PW() { + mimic_gpinitsystem_setup + + # given a user + USER_NAME=abc123456 + createuser $USER_NAME + trap __cleanupTestUsers EXIT + + # when we run set user password with a password containing single quotes + GP_PASSWD="abc'" + SET_GP_USER_PW + + # then it should succeed + if [ "$RETVAL" != "0" ]; then + local error_message="$(tail -n 10 "$LOG_FILE")" + echo "got an exit status of $RETVAL while running SET_GP_USER_PW for $USER_NAME with password $GP_PASSWD, wanted success: $error_message" + exit 1 + fi +} + +main() { + it_should_quote_the_username_during_alter_user_in_SET_GP_USER_PW + it_should_quote_the_password_during_alter_user_in_SET_GP_USER_PW +} + +main diff --git a/gpMgmt/bin/test/suite.bash b/gpMgmt/bin/test/suite.bash new file mode 100755 index 0000000000..3cf581e0ef --- /dev/null +++ b/gpMgmt/bin/test/suite.bash @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -o errexit + +./test/gpinitsystem_test.bash -- GitLab