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,17 +91,34 @@ 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
 
 behavior:
     read_only: true
 
-user:
-    name: username
-    password: password
-
 listener:
     host: localhost
     port: 15200
@@ -117,11 +132,11 @@ listener:
 	}()
 	controller := svcs.NewController()
 	go func() {
-
 		dEnv.FS.WriteFile("config.yaml", []byte(yamlConfig), os.ModePerm)
-		StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
+		err := StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
 			"--config", "config.yaml",
 		}, dEnv, dEnv.FS, controller)
+		require.NoError(t, err)
 	}()
 	err = controller.WaitForStart()
 	require.NoError(t, err)
@@ -284,8 +299,6 @@ func TestServerFailsIfPortInUse(t *testing.T) {
 		StartServer(context.Background(), "test", "dolt sql-server", []string{
 			"-H", "localhost",
 			"-P", "15200",
-			"-u", "username",
-			"-p", "password",
 			"-t", "5",
 			"-l", "info",
 			"-r",
@@ -516,7 +529,6 @@ func TestReadReplica(t *testing.T) {
 
 func TestGenerateYamlConfig(t *testing.T) {
 	args := []string{
-		"--user", "my_name",
 		"--timeout", "11",
 		"--branch-control-file", "dir1/dir2/abc.db",
 	}
@@ -543,10 +555,6 @@ func TestGenerateYamlConfig(t *testing.T) {
   # dolt_transaction_commit: false
   # event_scheduler: "OFF"
 
-user:
-  name: my_name
-  # password: ""
-
 listener:
   # host: localhost
   # port: 3306

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

@@ -24,6 +24,7 @@ import (
 
 	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/fatih/color"
+	"github.com/sirupsen/logrus"
 
 	"github.com/dolthub/dolt/go/cmd/dolt/cli"
 	"github.com/dolthub/dolt/go/cmd/dolt/commands"
@@ -39,7 +40,6 @@ const (
 	hostFlag                    = "host"
 	portFlag                    = "port"
 	skipRootUserInitialization  = "skip-root-user-initialization"
-	passwordFlag                = "password"
 	timeoutFlag                 = "timeout"
 	readonlyFlag                = "readonly"
 	logLevelFlag                = "loglevel"
@@ -96,10 +96,6 @@ SUPPORTED CONFIG FILE FIELDS:
 
 {{.EmphasisLeft}}behavior.dolt_transaction_commit{{.EmphasisRight}}: If true all SQL transaction commits will automatically create a Dolt commit, with a generated commit message. This is useful when a system working with Dolt wants to create versioned data, but doesn't want to directly use Dolt features such as dolt_commit(). 
 
-{{.EmphasisLeft}}user.name{{.EmphasisRight}}: The username for an ephemeral superuser that will exist for the lifetime of the sql-server process. This user will not be persisted to the privileges database. 
-
-{{.EmphasisLeft}}user.password{{.EmphasisRight}}: The password for an ephemeral superuser that will exist for the lifetime of the sql-server process. This user will not be persisted to the privileges database.
-
 {{.EmphasisLeft}}listener.host{{.EmphasisRight}}: The host address that the server will run on.  This may be {{.EmphasisLeft}}localhost{{.EmphasisRight}} or an IPv4 or IPv6 address
 
 {{.EmphasisLeft}}listener.port{{.EmphasisRight}}: The port that the server should listen on
@@ -129,7 +125,7 @@ SUPPORTED CONFIG FILE FIELDS:
 If a config file is not provided many of these settings may be configured on the command line.`,
 	Synopsis: []string{
 		"--config {{.LessThan}}file{{.GreaterThan}}",
-		"[-H {{.LessThan}}host{{.GreaterThan}}] [-P {{.LessThan}}port{{.GreaterThan}}] [-u {{.LessThan}}user{{.GreaterThan}}] [-p {{.LessThan}}password{{.GreaterThan}}] [-t {{.LessThan}}timeout{{.GreaterThan}}] [-l {{.LessThan}}loglevel{{.GreaterThan}}] [--data-dir {{.LessThan}}directory{{.GreaterThan}}] [-r]",
+		"[-H {{.LessThan}}host{{.GreaterThan}}] [-P {{.LessThan}}port{{.GreaterThan}}] [-t {{.LessThan}}timeout{{.GreaterThan}}] [-l {{.LessThan}}loglevel{{.GreaterThan}}] [--data-dir {{.LessThan}}directory{{.GreaterThan}}] [-r]",
 	},
 }
 
@@ -163,9 +159,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 +231,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,22 +375,18 @@ 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)
 			yamlCfg.GoldenMysqlConn = &connStr
 		}
 	}
 
+	if cfg.UserIsSpecified() {
+		logrus.Warn("user and password are no longer supported in sql-server configuration files." +
+			"Use CREATE USER and GRANT statements to manage user accounts.")
+	}
+
 	return cfg, nil
 }
 

go/libraries/doltcore/servercfg/yaml_config.go

@@ -187,10 +187,6 @@ func ServerConfigAsYAMLConfig(cfg ServerConfig) *YAMLConfig {
 			DoltTransactionCommit:        ptr(cfg.DoltTransactionCommit()),
 			EventSchedulerStatus:         ptr(cfg.EventSchedulerStatus()),
 		},
-		UserConfig: UserYAMLConfig{
-			Name:     ptr(cfg.User()),
-			Password: ptr(cfg.Password()),
-		},
 		ListenerConfig: ListenerYAMLConfig{
 			HostStr:                 ptr(cfg.Host()),
 			PortNumber:              ptr(cfg.Port()),
@@ -260,10 +256,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)),
@@ -473,13 +465,6 @@ func (cfg YAMLConfig) withDefaultsFilledIn() YAMLConfig {
 		withDefaults.BehaviorConfig.DoltTransactionCommit = defaults.BehaviorConfig.DoltTransactionCommit
 	}
 
-	if withDefaults.UserConfig.Name == nil {
-		withDefaults.UserConfig.Name = defaults.UserConfig.Name
-	}
-	if withDefaults.UserConfig.Password == nil {
-		withDefaults.UserConfig.Password = defaults.UserConfig.Password
-	}
-
 	if withDefaults.ListenerConfig.HostStr == nil {
 		withDefaults.ListenerConfig.HostStr = defaults.ListenerConfig.HostStr
 	}

go/libraries/doltcore/servercfg/yaml_config_test.go

@@ -35,10 +35,6 @@ behavior:
     disable_client_multi_statements: false
     event_scheduler: ON
 
-user:
-    name: ""
-    password: ""
-
 listener:
     host: localhost
     port: 3306

go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go

@@ -1017,6 +1017,11 @@ func startDoltSqlServer(t *testing.T, dir string, doltPersistentSystemVars map[s
 
 	// use an admin user NOT named "root" to test that we don't require the "root" account
 	adminUser := "admin"
+	err = runDoltCommand(t, dir, "sql",
+		fmt.Sprintf("-q=%s", "CREATE USER admin@'%'; GRANT ALL ON *.* TO admin@'%';"))
+	if err != nil {
+		return -1, nil, err
+	}
 
 	if doltPersistentSystemVars != nil && len(doltPersistentSystemVars) > 0 {
 		// Initialize the dolt directory first
@@ -1036,7 +1041,6 @@ func startDoltSqlServer(t *testing.T, dir string, doltPersistentSystemVars map[s
 
 	args := []string{DoltDevBuildPath(),
 		"sql-server",
-		fmt.Sprintf("-u%s", adminUser),
 		"--loglevel=TRACE",
 		fmt.Sprintf("--data-dir=%s", dir),
 		fmt.Sprintf("--port=%v", doltPort)}

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