*: make conversions between string and usernames more explicit

Makefile

@@ -812,7 +812,7 @@ SQLPARSER_TARGETS = \
 	pkg/sql/parser/help_messages.go \
 	pkg/sql/lex/tokens.go \
 	pkg/sql/lex/keywords.go \
-	pkg/sql/lex/reserved_keywords.go
+	pkg/sql/lexbase/reserved_keywords.go
 
 PROTOBUF_TARGETS := bin/.go_protobuf_sources bin/.gw_protobuf_sources
 
@@ -1472,7 +1472,7 @@ pkg/sql/parser/gen/sql-gen.y: pkg/sql/parser/sql.y pkg/sql/parser/replace_help_r
 	mv -f [email protected] $@
 	rm pkg/sql/parser/gen/types_regex.tmp
 
-pkg/sql/lex/reserved_keywords.go: pkg/sql/parser/sql.y pkg/sql/parser/reserved_keywords.awk | bin/.bootstrap
+pkg/sql/lexbase/reserved_keywords.go: pkg/sql/parser/sql.y pkg/sql/parser/reserved_keywords.awk | bin/.bootstrap
 	awk -f pkg/sql/parser/reserved_keywords.awk < $< > [email protected] || rm [email protected]
 	mv -f [email protected] $@
 	gofmt -s -w $@

docs/generated/http/full.md

@@ -752,7 +752,7 @@ The underlying response type is something we're looking to get rid of.
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| username | [string](#cockroach.server.serverpb.ListSessionsRequest-string) |  | Username of the user making this request. |
+| username | [string](#cockroach.server.serverpb.ListSessionsRequest-string) |  | Username of the user making this request. The caller is responsible to normalize the username (= case fold and perform unicode NFC normalization). |
 
 
 
@@ -864,7 +864,7 @@ The underlying response type is something we're looking to get rid of.
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| username | [string](#cockroach.server.serverpb.ListSessionsRequest-string) |  | Username of the user making this request. |
+| username | [string](#cockroach.server.serverpb.ListSessionsRequest-string) |  | Username of the user making this request. The caller is responsible to normalize the username (= case fold and perform unicode NFC normalization). |
 
 
 
@@ -978,7 +978,7 @@ The underlying response type is something we're looking to get rid of.
 | ----- | ---- | ----- | ----------- |
 | node_id | [string](#cockroach.server.serverpb.CancelQueryRequest-string) |  | ID of gateway node for the query to be canceled.<br><br>TODO(itsbilal): use [(gogoproto.customname) = "NodeID"] below. Need to figure out how to teach grpc-gateway about custom names.<br><br>node_id is a string so that "local" can be used to specify that no forwarding is necessary. |
 | query_id | [string](#cockroach.server.serverpb.CancelQueryRequest-string) |  | ID of query to be canceled (converted to string). |
-| username | [string](#cockroach.server.serverpb.CancelQueryRequest-string) |  | Username of the user making this cancellation request. This may be omitted if the user is the same as the one issuing the CancelQueryRequest. |
+| username | [string](#cockroach.server.serverpb.CancelQueryRequest-string) |  | Username of the user making this cancellation request. This may be omitted if the user is the same as the one issuing the CancelQueryRequest. The caller is responsible for case-folding and NFC normalization. |
 
 
 
@@ -1017,7 +1017,7 @@ The underlying response type is something we're looking to get rid of.
 | ----- | ---- | ----- | ----------- |
 | node_id | [string](#cockroach.server.serverpb.CancelSessionRequest-string) |  | TODO(abhimadan): use [(gogoproto.customname) = "NodeID"] below. Need to figure out how to teach grpc-gateway about custom names.<br><br>node_id is a string so that "local" can be used to specify that no forwarding is necessary. |
 | session_id | [bytes](#cockroach.server.serverpb.CancelSessionRequest-bytes) |  |  |
-| username | [string](#cockroach.server.serverpb.CancelSessionRequest-string) |  | Username of the user making this cancellation request. This may be omitted if the user is the same as the one issuing the CancelSessionRequest. |
+| username | [string](#cockroach.server.serverpb.CancelSessionRequest-string) |  | Username of the user making this cancellation request. This may be omitted if the user is the same as the one issuing the CancelSessionRequest. The caller is responsiblef or case-folding and NFC normalization. |
 
 
 

docs/generated/sql/bnf/stmt_block.bnf

@@ -315,7 +315,7 @@ role_spec_list ::=
 	( role_spec ) ( ( ',' role_spec ) )*
 
 role_spec ::=
-	non_reserved_word_or_sconst
+	username_or_sconst
 	| 'CURRENT_USER'
 	| 'SESSION_USER'
 
@@ -1181,7 +1181,7 @@ type_name_list ::=
 type_list ::=
 	( typename ) ( ( ',' typename ) )*
 
-non_reserved_word_or_sconst ::=
+username_or_sconst ::=
 	non_reserved_word
 	| 'SCONST'
 
@@ -1400,6 +1400,10 @@ drop_type_stmt ::=
 explain_option_name ::=
 	non_reserved_word
 
+non_reserved_word_or_sconst ::=
+	non_reserved_word
+	| 'SCONST'
+
 kv_option_list ::=
 	( kv_option ) ( ( ',' kv_option ) )*
 

pkg/acceptance/cluster/certs.go

@@ -39,12 +39,12 @@ func GenerateCerts(ctx context.Context) func() {
 	// Root user.
 	maybePanic(security.CreateClientPair(
 		certsDir, filepath.Join(certsDir, security.EmbeddedCAKey),
-		1024, 48*time.Hour, false, security.RootUser, true /* generate pk8 key */))
+		1024, 48*time.Hour, false, security.RootUserName(), true /* generate pk8 key */))
 
 	// Test user.
 	maybePanic(security.CreateClientPair(
 		certsDir, filepath.Join(certsDir, security.EmbeddedCAKey),
-		1024, 48*time.Hour, false, "testuser", true /* generate pk8 key */))
+		1024, 48*time.Hour, false, security.TestUserName(), true /* generate pk8 key */))
 
 	// Certs for starting a cockroach server. Key size is from cli/cert.go:defaultKeySize.
 	maybePanic(security.CreateNodePair(

pkg/acceptance/localcluster/cluster.go

@@ -268,7 +268,7 @@ func (c *Cluster) RPCPort(nodeIdx int) string {
 
 func (c *Cluster) makeNode(ctx context.Context, nodeIdx int, cfg NodeConfig) (*Node, <-chan error) {
 	baseCtx := &base.Config{
-		User:     security.NodeUser,
+		User:     security.NodeUserName(),
 		Insecure: true,
 	}
 	rpcCtx := rpc.NewContext(rpc.ContextOptions{

pkg/base/config.go

@@ -169,7 +169,7 @@ type Config struct {
 
 	// User running this process. It could be the user under which
 	// the server is running or the user passed in client calls.
-	User string
+	User security.SQLUsername
 
 	// Addr is the address the server is listening on.
 	Addr string
@@ -255,7 +255,7 @@ func (*Config) HistogramWindowInterval() time.Duration {
 // This is also used in tests to reset global objects.
 func (cfg *Config) InitDefaults() {
 	cfg.Insecure = defaultInsecure
-	cfg.User = defaultUser
+	cfg.User = security.MakeSQLUsernameFromPreNormalizedString(defaultUser)
 	cfg.Addr = defaultAddr
 	cfg.AdvertiseAddr = cfg.Addr
 	cfg.HTTPAddr = defaultHTTPAddr

pkg/ccl/backupccl/backup_destination.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/storage/cloud"
@@ -31,7 +32,7 @@ import (
 // chain.
 func fetchPreviousBackups(
 	ctx context.Context,
-	user string,
+	user security.SQLUsername,
 	makeCloudStorage cloud.ExternalStorageFromURIFactory,
 	prevBackupURIs []string,
 	encryptionParams backupEncryptionParams,
@@ -70,7 +71,7 @@ func fetchPreviousBackups(
 //  can determine the latter from the former.
 func resolveDest(
 	ctx context.Context,
-	user string,
+	user security.SQLUsername,
 	nested, appendToLatest bool,
 	defaultURI string,
 	urisByLocalityKV map[string]string,
@@ -151,7 +152,7 @@ func resolveDest(
 // getBackupManifests fetches the backup manifest from a list of backup URIs.
 func getBackupManifests(
 	ctx context.Context,
-	user string,
+	user security.SQLUsername,
 	makeCloudStorage cloud.ExternalStorageFromURIFactory,
 	backupURIs []string,
 	encryption *jobspb.BackupEncryptionOptions,
@@ -195,7 +196,7 @@ func getBackupManifests(
 // base backups.
 func getEncryptionFromBase(
 	ctx context.Context,
-	user string,
+	user security.SQLUsername,
 	makeCloudStorage cloud.ExternalStorageFromURIFactory,
 	baseBackupURI string,
 	encryptionParams backupEncryptionParams,
@@ -236,7 +237,7 @@ func getEncryptionFromBase(
 // should use for a backup that is pointing to a collection.
 func resolveBackupCollection(
 	ctx context.Context,
-	user string,
+	user security.SQLUsername,
 	defaultURI string,
 	appendToLatest bool,
 	makeCloudStorage cloud.ExternalStorageFromURIFactory,

pkg/ccl/backupccl/backup_destination_test.go

@@ -36,7 +36,7 @@ func newTestStorageFactory(t *testing.T) (cloud.ExternalStorageFromURIFactory, f
 	settings := cluster.MakeTestingClusterSettings()
 	settings.ExternalIODir = dir
 	clientFactory := blobs.TestBlobServiceClient(settings.ExternalIODir)
-	externalStorageFromURI := func(ctx context.Context, uri, user string) (cloud.ExternalStorage,
+	externalStorageFromURI := func(ctx context.Context, uri string, user security.SQLUsername) (cloud.ExternalStorage,
 		error) {
 		conf, err := cloudimpl.ExternalStorageConfFromURI(uri, user)
 		require.NoError(t, err)
@@ -60,7 +60,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 
 	// writeManifest writes an empty backup manifest file to the given URI.
 	writeManifest := func(t *testing.T, uri string) {
-		storage, err := externalStorageFromURI(ctx, uri, security.RootUser)
+		storage, err := externalStorageFromURI(ctx, uri, security.RootUserName())
 		defer storage.Close()
 		require.NoError(t, err)
 		require.NoError(t, storage.WriteFile(ctx, backupManifestName, emptyReader))
@@ -69,7 +69,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 	// writeLatest writes latestBackupSuffix to the LATEST file in the given
 	// collection.
 	writeLatest := func(t *testing.T, collectionURI, latestBackupSuffix string) {
-		storage, err := externalStorageFromURI(ctx, collectionURI, security.RootUser)
+		storage, err := externalStorageFromURI(ctx, collectionURI, security.RootUserName())
 		defer storage.Close()
 		require.NoError(t, err)
 		require.NoError(t, storage.WriteFile(ctx, latestFileName, bytes.NewReader([]byte(latestBackupSuffix))))
@@ -136,7 +136,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 					require.NoError(t, err)
 
 					collectionURI, defaultURI, chosenSuffix, urisByLocalityKV, prevBackupURIs, err := resolveDest(
-						ctx, security.RootUser,
+						ctx, security.RootUserName(),
 						false /* nested */, false, /* appendToLatest */
 						defaultDest, localitiesDest,
 						externalStorageFromURI, endTime,
@@ -205,7 +205,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 					dest, localitiesDest, err := getURIsByLocalityKV(to, "")
 					require.NoError(t, err)
 					collectionURI, defaultURI, chosenSuffix, urisByLocalityKV, prevBackupURIs, err := resolveDest(
-						ctx, security.RootUser,
+						ctx, security.RootUserName(),
 						false /* nested */, false, /* appendToLatest */
 						dest, localitiesDest,
 						externalStorageFromURI, endTime,
@@ -308,7 +308,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
 					defaultCollection, localityCollections, err := getURIsByLocalityKV(collectionTo, "")
 					require.NoError(t, err)
 					collectionURI, defaultURI, chosenSuffix, urisByLocalityKV, prevBackupURIs, err := resolveDest(
-						ctx, security.RootUser,
+						ctx, security.RootUserName(),
 						true /* nested */, appendToLatest,
 						defaultCollection, localityCollections,
 						externalStorageFromURI, endTime,

pkg/ccl/backupccl/backup_job.go

@@ -609,7 +609,7 @@ func (b *backupResumer) maybeNotifyScheduledJobCompletion(
 			ctx,
 			"lookup-schedule-info",
 			txn,
-			sessiondata.InternalExecutorOverride{User: security.NodeUser},
+			sessiondata.InternalExecutorOverride{User: security.NodeUserName()},
 			fmt.Sprintf(
 				"SELECT created_by_id FROM %s WHERE id=$1 AND created_by_type=$2",
 				env.SystemJobsTableName()),
@@ -678,7 +678,7 @@ func (b *backupResumer) OnFailOrCancel(ctx context.Context, phs interface{}) err
 }
 
 func (b *backupResumer) deleteCheckpoint(
-	ctx context.Context, cfg *sql.ExecutorConfig, user string,
+	ctx context.Context, cfg *sql.ExecutorConfig, user security.SQLUsername,
 ) {
 	// Attempt to delete BACKUP-CHECKPOINT.
 	if err := func() error {

pkg/ccl/backupccl/backup_processor.go

@@ -141,13 +141,13 @@ func runBackupProcessor(
 
 	// For all backups, partitioned or not, the main BACKUP manifest is stored at
 	// details.URI.
-	defaultConf, err := cloudimpl.ExternalStorageConfFromURI(spec.DefaultURI, spec.User)
+	defaultConf, err := cloudimpl.ExternalStorageConfFromURI(spec.DefaultURI, spec.User())
 	if err != nil {
 		return err
 	}
 	storageByLocalityKV := make(map[string]*roachpb.ExternalStorage)
 	for kv, uri := range spec.URIsByLocalityKV {
-		conf, err := cloudimpl.ExternalStorageConfFromURI(uri, spec.User)
+		conf, err := cloudimpl.ExternalStorageConfFromURI(uri, spec.User())
 		if err != nil {
 			return err
 		}

pkg/ccl/backupccl/backup_processor_planning.go

@@ -14,6 +14,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	roachpb "github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
 	"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
@@ -136,7 +137,7 @@ func makeBackupDataProcessorSpecs(
 	mvccFilter roachpb.MVCCFilter,
 	encryption *jobspb.BackupEncryptionOptions,
 	startTime, endTime hlc.Timestamp,
-	user string,
+	user security.SQLUsername,
 	execCfg *sql.ExecutorConfig,
 ) (map[roachpb.NodeID]*execinfrapb.BackupDataSpec, error) {
 	var spanPartitions []sql.SpanPartition
@@ -191,7 +192,7 @@ func makeBackupDataProcessorSpecs(
 			PKIDs:            pkIDs,
 			BackupStartTime:  startTime,
 			BackupEndTime:    endTime,
-			User:             user,
+			UserProto:        user.EncodeProto(),
 		}
 		nodeToSpec[partition.Node] = spec
 	}
@@ -212,7 +213,7 @@ func makeBackupDataProcessorSpecs(
 				PKIDs:            pkIDs,
 				BackupStartTime:  startTime,
 				BackupEndTime:    endTime,
-				User:             user,
+				UserProto:        user.EncodeProto(),
 			}
 			nodeToSpec[partition.Node] = spec
 		}

pkg/ccl/backupccl/backup_test.go

@@ -598,7 +598,8 @@ func TestBackupRestoreAppend(t *testing.T) {
 			store, err := cloudimpl.ExternalStorageFromURI(ctx, "userfile:///0",
 				base.ExternalIODirConfig{},
 				tc.Servers[0].ClusterSettings(),
-				blobs.TestEmptyBlobClientFactory, "root",
+				blobs.TestEmptyBlobClientFactory,
+				security.RootUserName(),
 				tc.Servers[0].InternalExecutor().(*sql.InternalExecutor), tc.Servers[0].DB())
 			require.NoError(t, err)
 			defer store.Close()
@@ -1094,7 +1095,7 @@ func TestBackupRestoreSystemJobs(t *testing.T) {
 
 	sqlDB.Exec(t, `BACKUP TABLE bank TO $1 INCREMENTAL FROM $2`, incDir, fullDir)
 	if err := jobutils.VerifySystemJob(t, sqlDB, 1, jobspb.TypeBackup, jobs.StatusSucceeded, jobs.Record{
-		Username: security.RootUser,
+		Username: security.RootUserName(),
 		Description: fmt.Sprintf(
 			`BACKUP TABLE bank TO '%s' INCREMENTAL FROM '%s'`,
 			sanitizedIncDir+"redacted", sanitizedFullDir+"redacted",
@@ -1109,7 +1110,7 @@ func TestBackupRestoreSystemJobs(t *testing.T) {
 
 	sqlDB.Exec(t, `RESTORE TABLE bank FROM $1, $2 WITH OPTIONS (into_db='restoredb')`, fullDir, incDir)
 	if err := jobutils.VerifySystemJob(t, sqlDB, 0, jobspb.TypeRestore, jobs.StatusSucceeded, jobs.Record{
-		Username: security.RootUser,
+		Username: security.RootUserName(),
 		Description: fmt.Sprintf(
 			`RESTORE TABLE bank FROM '%s', '%s' WITH into_db='restoredb'`,
 			sanitizedFullDir+"redacted", sanitizedIncDir+"redacted",
@@ -1195,7 +1196,7 @@ func TestEncryptedBackupRestoreSystemJobs(t *testing.T) {
 			// Verify the BACKUP job description is sanitized.
 			if err := jobutils.VerifySystemJob(t, sqlDB, 0, jobspb.TypeBackup, jobs.StatusSucceeded,
 				jobs.Record{
-					Username: security.RootUser,
+					Username: security.RootUserName(),
 					Description: fmt.Sprintf(
 						`BACKUP DATABASE data TO '%s' WITH %s`,
 						backupLoc1, sanitizedEncryptionOption),
@@ -1213,7 +1214,7 @@ into_db='restoredb', %s)`, encryptionOption), backupLoc1)
 
 			// Verify the RESTORE job description is sanitized.
 			if err := jobutils.VerifySystemJob(t, sqlDB, 0, jobspb.TypeRestore, jobs.StatusSucceeded, jobs.Record{
-				Username: security.RootUser,
+				Username: security.RootUserName(),
 				Description: fmt.Sprintf(
 					`RESTORE TABLE data.bank FROM '%s' WITH %s, into_db='restoredb'`,
 					backupLoc1, sanitizedEncryptionOption,
@@ -1419,7 +1420,7 @@ func createAndWaitForJob(
 	t.Helper()
 	now := timeutil.ToUnixMicros(timeutil.Now())
 	payload, err := protoutil.Marshal(&jobspb.Payload{
-		Username:      security.RootUser,
+		UsernameProto: security.RootUserName().EncodeProto(),
 		DescriptorIDs: descriptorIDs,
 		StartedMicros: now,
 		Details:       jobspb.WrapPayloadDetails(details),

pkg/ccl/backupccl/create_scheduled_backup.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/scheduledjobs"
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
@@ -408,7 +409,7 @@ func checkForExistingBackupsInCollection(
 
 func makeBackupSchedule(
 	env scheduledjobs.JobSchedulerEnv,
-	owner string,
+	owner security.SQLUsername,
 	label string,
 	recurrence *scheduleRecurrence,
 	details jobspb.ScheduleDetails,

pkg/ccl/backupccl/create_scheduled_backup_test.go

@@ -140,7 +140,7 @@ func (h *testHelper) createBackupSchedule(
 		// Query system.scheduled_job table and load those schedules.
 		datums, cols, err := h.cfg.InternalExecutor.QueryWithCols(
 			context.Background(), "sched-load", nil,
-			sessiondata.InternalExecutorOverride{User: security.RootUser},
+			sessiondata.InternalExecutorOverride{User: security.RootUserName()},
 			"SELECT * FROM system.scheduled_jobs WHERE schedule_id = $1",
 			id,
 		)