First pass on removing user and pass options for sql-server

go/cmd/dolt/commands/sqlserver/server.go

@@ -376,9 +376,6 @@ func ConfigureServices(
 	// instead of dolt db initialization, because we only want to create the privileges database when it's
 	// used for a server, and because we want the same root initialization logic when a sql-server is started
 	// for a clone. More details: https://dev.mysql.com/doc/mysql-security-excerpt/8.0/en/default-privileges.html
-	//
-	// NOTE: The MySQL root user is created for host 'localhost', not any host ('%'). We could do the same here,
-	//       but it seems like it would cause problems for users who want to connect from outside of Docker.
 	InitImplicitRootSuperUser := &svcs.AnonService{
 		InitF: func(ctx context.Context) error {
 			// If privileges.db has already been initialized, indicating that this is NOT the
@@ -395,9 +392,8 @@ func ConfigureServices(
 			ed := mysqlDb.Editor()
 			defer ed.Close()
 
-			// If no ephemeral superuser has been configured and root user initialization wasn't skipped,
-			// then create a root@localhost superuser.
-			if !serverConfig.UserIsSpecified() && !config.SkipRootUserInitialization {
+			// Create the root@localhost superuser, unless --skip-root-user-initialization was specified
+			if !config.SkipRootUserInitialization {
 				// Allow the user to override the default root host (localhost) and password ("").
 				// This is particularly useful in a Docker container, where you need to connect
 				// to the sql-server from outside the container and can't rely on localhost.
@@ -436,26 +432,6 @@ func ConfigureServices(
 	}
 	controller.Register(InitImplicitRootSuperUser)
 
-	// Add an ephemeral superuser if one was requested
-	InitEphemeralSuperUser := &svcs.AnonService{
-		InitF: func(context.Context) error {
-			mysqlDb := sqlEngine.GetUnderlyingEngine().Analyzer.Catalog.MySQLDb
-			ed := mysqlDb.Editor()
-
-			userSpecified := config.ServerUser != ""
-			if userSpecified {
-				superuser := mysqlDb.GetUser(ed, config.ServerUser, "%", false)
-				if superuser == nil {
-					mysqlDb.AddEphemeralSuperUser(ed, config.ServerUser, "%", config.ServerPass)
-				}
-			}
-			ed.Close()
-
-			return nil
-		},
-	}
-	controller.Register(InitEphemeralSuperUser)
-
 	var metListener *metricsListener
 	InitMetricsListener := &svcs.AnonService{
 		InitF: func(context.Context) (err error) {

go/cmd/dolt/commands/sqlserver/server_test.go

@@ -75,8 +75,6 @@ func TestServerArgs(t *testing.T) {
 		StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
 			"-H", "localhost",
 			"-P", "15200",
-			"-u", "username",
-			"-p", "password",
 			"-t", "5",
 			"-l", "info",
 			"-r",
@@ -93,6 +91,27 @@ func TestServerArgs(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestDeprecatedUserPasswordServerArgs(t *testing.T) {
+	controller := svcs.NewController()
+	dEnv, err := sqle.CreateEnvWithSeedData()
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, dEnv.DoltDB.Close())
+	}()
+	err = StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
+		"-H", "localhost",
+		"-P", "15200",
+		"-u", "username",
+		"-p", "password",
+		"-t", "5",
+		"-l", "info",
+		"-r",
+	}, dEnv, dEnv.FS, controller)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "--user and --password have been removed from the sql-server command.")
+	require.Contains(t, err.Error(), "Create users explicitly with CREATE USER and GRANT statements instead.")
+}
+
 func TestYAMLServerArgs(t *testing.T) {
 	const yamlConfig = `
 log_level: info

go/cmd/dolt/commands/sqlserver/sqlserver.go

@@ -39,7 +39,6 @@ const (
 	hostFlag                    = "host"
 	portFlag                    = "port"
 	skipRootUserInitialization  = "skip-root-user-initialization"
-	passwordFlag                = "password"
 	timeoutFlag                 = "timeout"
 	readonlyFlag                = "readonly"
 	logLevelFlag                = "loglevel"
@@ -163,9 +162,9 @@ func (cmd SqlServerCmd) ArgParserWithName(name string) *argparser.ArgParser {
 	ap.SupportsString(configFileFlag, "", "file", "When provided configuration is taken from the yaml config file and all command line parameters are ignored.")
 	ap.SupportsString(hostFlag, "H", "host address", fmt.Sprintf("Defines the host address that the server will run on. Defaults to `%v`.", serverConfig.Host()))
 	ap.SupportsUint(portFlag, "P", "port", fmt.Sprintf("Defines the port that the server will run on. Defaults to `%v`.", serverConfig.Port()))
-	ap.SupportsString(commands.UserFlag, "u", "user", fmt.Sprintf("Defines the server user. Defaults to `%v`. This should be explicit if desired.", serverConfig.User()))
+	ap.SupportsString(commands.UserFlag, "u", "user", "This option is no longer supported. Instead, you can create users using CREATE USER and GRANT SQL statements.")
 	ap.SupportsFlag(skipRootUserInitialization, "", "Skips the automatic creation of a default root super user on the first launch of a SQL server.")
-	ap.SupportsString(passwordFlag, "p", "password", fmt.Sprintf("Defines the server password. Defaults to `%v`.", serverConfig.Password()))
+	ap.SupportsString("password", "p", "password", "This option is no longer supported. Instead, you can create users using CREATE USER and GRANT SQL statements.")
 	ap.SupportsInt(timeoutFlag, "t", "connection timeout", fmt.Sprintf("Defines the timeout, in seconds, used for connections\nA value of `0` represents an infinite timeout. Defaults to `%v`.", serverConfig.ReadTimeout()))
 	ap.SupportsFlag(readonlyFlag, "r", "Disable modification of the database.")
 	ap.SupportsString(logLevelFlag, "l", "log level", fmt.Sprintf("Defines the level of logging provided\nOptions are: `trace`, `debug`, `info`, `warning`, `error`, `fatal`. Defaults to `%v`.", serverConfig.LogLevel()))
@@ -235,6 +234,12 @@ func validateSqlServerArgs(apr *argparser.ArgParseResults) error {
 	if multiDbDir {
 		cli.PrintErrln("WARNING: --multi-db-dir is deprecated, use --data-dir instead")
 	}
+	_, userSpecified := apr.GetValue(commands.UserFlag)
+	if userSpecified {
+		return fmt.Errorf("ERROR: --user and --password have been removed from the sql-server command. " +
+			"Create users explicitly with CREATE USER and GRANT statements instead.")
+	}
+
 	return nil
 }
 
@@ -373,15 +378,6 @@ func getServerConfig(cwdFS filesys.Filesys, apr *argparser.ArgParseResults, data
 		return nil, err
 	}
 
-	// if command line user argument was given, override the config file's user and password
-	if user, hasUser := apr.GetValue(commands.UserFlag); hasUser {
-		if wcfg, ok := cfg.(servercfg.WritableServerConfig); ok {
-			pass, _ := apr.GetValue(passwordFlag)
-			wcfg.SetUserName(user)
-			wcfg.SetPassword(pass)
-		}
-	}
-
 	if connStr, ok := apr.GetValue(goldenMysqlConn); ok {
 		if yamlCfg, ok := cfg.(servercfg.YAMLConfig); ok {
 			cli.Println(connStr)

go/libraries/doltcore/servercfg/yaml_config.go

@@ -260,10 +260,6 @@ func ServerConfigSetValuesAsYAMLConfig(cfg ServerConfig) *YAMLConfig {
 			DoltTransactionCommit:        zeroIf(ptr(cfg.DoltTransactionCommit()), !cfg.ValueSet(DoltTransactionCommitKey)),
 			EventSchedulerStatus:         zeroIf(ptr(cfg.EventSchedulerStatus()), !cfg.ValueSet(EventSchedulerKey)),
 		},
-		UserConfig: UserYAMLConfig{
-			Name:     zeroIf(ptr(cfg.User()), !cfg.ValueSet(UserKey)),
-			Password: zeroIf(ptr(cfg.Password()), !cfg.ValueSet(PasswordKey)),
-		},
 		ListenerConfig: ListenerYAMLConfig{
 			HostStr:                 zeroIf(ptr(cfg.Host()), !cfg.ValueSet(HostKey)),
 			PortNumber:              zeroIf(ptr(cfg.Port()), !cfg.ValueSet(PortKey)),

integration-tests/bats/branch-control.bats

@@ -16,6 +16,7 @@ setup_test_user() {
     dolt sql -q "create user test identified by ''"
     dolt sql -q "grant all on *.* to test"
     dolt sql -q "delete from dolt_branch_control where user='%'"
+    SQL_USER=test
 }
 
 @test "branch-control: fresh database. branch control tables exist" {

integration-tests/bats/cli-hosted.bats

@@ -22,7 +22,8 @@ setup() {
   # creation is done here. We may want to move this to helper/query-server-common.bash later.
   PORT=$( definePORT )
   DOLT_CLI_PASSWORD="d01t"
-  dolt sql-server --host 0.0.0.0 --port=$PORT --user="dolt" --password=$DOLT_CLI_PASSWORD --socket "dolt.$PORT.sock" &
+  dolt sql -q "create user dolt@'%' identified by '$DOLT_CLI_PASSWORD'; grant all on *.* to dolt@'%'"
+  dolt sql-server --host 0.0.0.0 --port=$PORT --socket "dolt.$PORT.sock" &
   SERVER_PID=$!
 
   # Also, wait_for_connection code is pulled in here and replaced with a use of `dolt sql` instead. This
@@ -33,7 +34,7 @@ setup() {
   end_time=$((SECONDS+($timeout/1000)))
 
   while [ $SECONDS -lt $end_time ]; do
-    run dolt --no-tls --host localhost --port $PORT -u "dolt" sql -q "SELECT 1;"
+    run dolt --no-tls --host localhost --port $PORT sql -q "SELECT 1;"
     if [ $status -eq 0 ]; then
       echo "Connected successfully!"
       break

integration-tests/bats/events.bats

@@ -12,9 +12,9 @@ make_test_repo_and_start_server() {
   export DOLT_EVENT_SCHEDULER_PERIOD=1
   start_sql_server
 
-  dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls --use-db information_schema sql -q "CREATE DATABASE repo1;"
-  dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "CREATE TABLE totals (id int PRIMARY KEY AUTO_INCREMENT, int_col int);"
-  dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "call dolt_commit('-Am', 'creating table');"
+  dolt --port $PORT --host 0.0.0.0 --no-tls --use-db information_schema sql -q "CREATE DATABASE repo1;"
+  dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "CREATE TABLE totals (id int PRIMARY KEY AUTO_INCREMENT, int_col int);"
+  dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "call dolt_commit('-Am', 'creating table');"
 }
 
 setup() {

integration-tests/bats/helper/query-server-common.bash

@@ -4,7 +4,7 @@ SERVER_PID=""
 DEFAULT_DB=""
 
 # wait_for_connection(<PORT>, <TIMEOUT IN MS>) attempts to connect to the sql-server at the specified
-# port on localhost, using the $SQL_USER (or 'dolt' if unspecified) as the user name, and trying once
+# port on localhost, using $SQL_USER (or 'root' if unspecified) as the user name, and trying once
 # per second until the millisecond timeout is reached. If a connection is successfully established,
 # this function returns 0. If a connection was not able to be established within the timeout period,
 # this function returns 1.
@@ -17,7 +17,7 @@ wait_for_connection() {
       echo "Running in AWS Lambda; increasing timeout to: $timeout"
   fi
 
-  user=${SQL_USER:-dolt}
+  user=${SQL_USER:-root}
   end_time=$((SECONDS+($timeout/1000)))
 
   while [ $SECONDS -lt $end_time ]; do
@@ -40,15 +40,15 @@ start_sql_server() {
     if [[ $logFile ]]
     then
         if [ "$IS_WINDOWS" == true ]; then
-          dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" > $logFile 2>&1 &
+          dolt sql-server --host 0.0.0.0 --port=$PORT > $logFile 2>&1 &
         else
-          dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" > $logFile 2>&1 &
+          dolt sql-server --host 0.0.0.0 --port=$PORT --socket "dolt.$PORT.sock" > $logFile 2>&1 &
         fi
     else
         if [ "$IS_WINDOWS" == true ]; then
-          dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" &
+          dolt sql-server --host 0.0.0.0 --port=$PORT &
         else
-          dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" &
+          dolt sql-server --host 0.0.0.0 --port=$PORT --socket "dolt.$PORT.sock" &
         fi
     fi
     echo db:$DEFAULT_DB logFile:$logFile PORT:$PORT CWD:$PWD
@@ -83,9 +83,6 @@ start_sql_server_with_config() {
     echo "
 log_level: debug
 
-user:
-  name: dolt
-
 listener:
   host: 0.0.0.0
   port: $PORT
@@ -134,9 +131,9 @@ start_multi_db_server() {
     DEFAULT_DB="$1"
     PORT=$( definePORT )
     if [ "$IS_WINDOWS" == true ]; then
-      dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ &
+      dolt sql-server --host 0.0.0.0 --port=$PORT --data-dir ./ &
     else
-      dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ --socket "dolt.$PORT.sock" &
+      dolt sql-server --host 0.0.0.0 --port=$PORT --data-dir ./ --socket "dolt.$PORT.sock" &
     fi
     SERVER_PID=$!
     wait_for_connection $PORT 8500

integration-tests/bats/profile.bats

@@ -88,7 +88,7 @@ teardown() {
     cd -
 
     start_sql_server altDb
-    dolt --user dolt --password "" sql -q "CREATE USER 'steph' IDENTIFIED BY 'pass'; GRANT ALL PRIVILEGES ON altDB.* TO 'steph' WITH GRANT OPTION;";
+    dolt sql -q "CREATE USER 'steph' IDENTIFIED BY 'pass'; GRANT ALL PRIVILEGES ON altDB.* TO 'steph' WITH GRANT OPTION;";
     dolt profile add --user "not-steph" --password "pass" --use-db altDB userWithDBProfile
 
     run dolt --profile userWithDBProfile --user steph sql -q "select * from test"
@@ -105,7 +105,7 @@ teardown() {
     cd -
 
     start_sql_server altDb
-    dolt --user dolt --password "" sql -q "CREATE USER 'steph' IDENTIFIED BY 'pass'; GRANT ALL PRIVILEGES ON altDB.* TO 'steph' WITH GRANT OPTION;";
+    dolt sql -q "CREATE USER 'steph' IDENTIFIED BY 'pass'; GRANT ALL PRIVILEGES ON altDB.* TO 'steph' WITH GRANT OPTION;";
     dolt profile add --user "not-steph" --password "pass" userProfile
 
     run dolt --profile userProfile --user steph --use-db altDB sql -q "select * from test"

integration-tests/bats/remotes-sql-server.bats

@@ -434,7 +434,7 @@ teardown() {
     [[ "$output" =~ "Tables_in_repo2/feature" ]] || false
     [[ "$output" =~ "test" ]] || false
 
-    run dolt -u dolt branch
+    run dolt branch
     [[ "$output" =~ "feature" ]] || false
 }
 
@@ -462,11 +462,11 @@ teardown() {
     dolt branch newbranch
     dolt push remote1 newbranch
 
-    run dolt --use-db repo2/feature --port $PORT --host 0.0.0.0 --no-tls -u dolt sql -q "select active_branch()"
+    run dolt --use-db repo2/feature --port $PORT --host 0.0.0.0 --no-tls sql -q "select active_branch()"
     [ $status -eq 0 ]
     [[ "$output" =~ "feature" ]] || false
 
-    run dolt --use-db repo2/newbranch --port $PORT --host 0.0.0.0 --no-tls -u dolt sql -q "select active_branch()"
+    run dolt --use-db repo2/newbranch --port $PORT --host 0.0.0.0 --no-tls sql -q "select active_branch()"
     [ $status -eq 0 ]
     [[ "$output" =~ "newbranch" ]] || false
 

integration-tests/bats/replication-multidb.bats

@@ -294,15 +294,15 @@ SQL
     start_multi_db_server repo1
     cd ..
 
-    dolt --use-db repo1 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "create table t1 (a int primary key)"
-    dolt --use-db repo1 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_add('.')"
-    dolt --use-db repo1 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_commit('-am', 'cm')"
-    dolt --use-db repo2 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "create table t2 (a int primary key)"
-    dolt --use-db repo2 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_add('.')"
-    dolt --use-db repo2 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_commit('-am', 'cm')"
-    dolt --use-db repo3 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "create table t3 (a int primary key)"
-    dolt --use-db repo3 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_add('.')"
-    dolt --use-db repo3 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_commit('-am', 'cm')"
+    dolt --use-db repo1 --port $PORT --host 0.0.0.0 --no-tls sql -q "create table t1 (a int primary key)"
+    dolt --use-db repo1 --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_add('.')"
+    dolt --use-db repo1 --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_commit('-am', 'cm')"
+    dolt --use-db repo2 --port $PORT --host 0.0.0.0 --no-tls sql -q "create table t2 (a int primary key)"
+    dolt --use-db repo2 --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_add('.')"
+    dolt --use-db repo2 --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_commit('-am', 'cm')"
+    dolt --use-db repo3 --port $PORT --host 0.0.0.0 --no-tls sql -q "create table t3 (a int primary key)"
+    dolt --use-db repo3 --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_add('.')"
+    dolt --use-db repo3 --port $PORT --host 0.0.0.0 --no-tls sql -q "call dolt_commit('-am', 'cm')"
 
     clone_helper $TMPDIRS
 
@@ -357,21 +357,21 @@ SQL
     dolt config --global --unset sqlserver.global.dolt_replicate_heads
 
     # Assert that no databases are synced to the read replica server yet
-    run dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "show databases"
+    run dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "show databases"
     [ $status -eq 0 ]
     [[ "$output" =~ "information_schema" ]] || false
     [[ ! "$output" =~ "repo1" ]] || false
     [[ ! "$output" =~ "repo2" ]] || false
 
     # Sync repo1 to the read replica server by use'ing it
-    run dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "use repo1; show databases;"
+    run dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "use repo1; show databases;"
     [ $status -eq 0 ]
     [[ "$output" =~ "information_schema" ]] || false
     [[ "$output" =~ "repo1" ]] || false
     [[ ! "$output" =~ "repo2" ]] || false
 
     # Sync repo1 by using it
-    run dolt --use-db repo1 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "select * from t1;"
+    run dolt --use-db repo1 --port $PORT --host 0.0.0.0 --no-tls sql -q "select * from t1;"
     [ $status -eq 0 ]
     [[ "$output" =~ "42" ]] || false
 
@@ -382,7 +382,7 @@ SQL
     dolt push remote1 main:main
 
     # Verify that the changes from repo1 have been pulled
-    run dolt --use-db repo1 -u dolt --port $PORT --host 0.0.0.0 --no-tls sql -q "select * from t1;"
+    run dolt --use-db repo1 --port $PORT --host 0.0.0.0 --no-tls sql -q "select * from t1;"
     [ $status -eq 0 ]
     [[ "$output" =~ "42" ]] || false
     [[ "$output" =~ "43" ]] || false