sql: consolidate username normalization in one place

All username normalization now happens through RoleSpec. This removes
the special logic in create_role that was validating usernames.

Release note: None

pkg/sql/alter_database.go

@@ -64,7 +64,7 @@ func (p *planner) AlterDatabaseOwner(
 }
 
 func (n *alterDatabaseOwnerNode) startExec(params runParams) error {
-	newOwner, err := n.n.Owner.ToSQLUsername(params.p.SessionData())
+	newOwner, err := n.n.Owner.ToSQLUsername(params.p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}

pkg/sql/alter_default_privileges.go

@@ -14,6 +14,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
@@ -76,7 +77,7 @@ func (p *planner) alterDefaultPrivileges(
 }
 
 func (n *alterDefaultPrivilegesNode) startExec(params runParams) error {
-	targetRoles, err := n.n.Roles.ToSQLUsernames(params.SessionData())
+	targetRoles, err := n.n.Roles.ToSQLUsernames(params.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}
@@ -98,7 +99,7 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error {
 		objectType = n.n.Revoke.Target
 	}
 
-	granteeSQLUsernames, err := grantees.ToSQLUsernames(params.p.SessionData())
+	granteeSQLUsernames, err := grantees.ToSQLUsernames(params.p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}
@@ -157,7 +158,7 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error {
 	}
 
 	var events []eventLogEntry
-	granteeSQLUsernames, err = grantees.ToSQLUsernames(params.SessionData())
+	granteeSQLUsernames, err = grantees.ToSQLUsernames(params.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}

pkg/sql/alter_role.go

@@ -103,7 +103,7 @@ func (p *planner) AlterRoleNode(
 		return nil, err
 	}
 
-	roleName, err := roleSpec.ToSQLUsername(p.SessionData())
+	roleName, err := roleSpec.ToSQLUsername(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}
@@ -330,7 +330,7 @@ func (p *planner) AlterRoleSet(ctx context.Context, n *tree.AlterRoleSet) (planN
 	var roleName security.SQLUsername
 	if !n.AllRoles {
 		var err error
-		roleName, err = n.RoleName.ToSQLUsername(p.SessionData())
+		roleName, err = n.RoleName.ToSQLUsername(p.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/alter_schema.go

@@ -118,7 +118,7 @@ func (n *alterSchemaNode) startExec(params runParams) error {
 			NewSchemaName: newQualifiedSchemaName.String(),
 		})
 	case *tree.AlterSchemaOwner:
-		newOwner, err := t.Owner.ToSQLUsername(params.p.SessionData())
+		newOwner, err := t.Owner.ToSQLUsername(params.p.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return err
 		}

pkg/sql/alter_table_owner.go

@@ -63,7 +63,7 @@ func (p *planner) AlterTableOwner(ctx context.Context, n *tree.AlterTableOwner)
 		return nil, err
 	}
 
-	owner, err := n.Owner.ToSQLUsername(p.SessionData())
+	owner, err := n.Owner.ToSQLUsername(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/alter_type.go

@@ -111,7 +111,7 @@ func (n *alterTypeNode) startExec(params runParams) error {
 		// See https://github.com/cockroachdb/cockroach/issues/57741
 		err = params.p.setTypeSchema(params.ctx, n, string(t.Schema))
 	case *tree.AlterTypeOwner:
-		owner, err := t.Owner.ToSQLUsername(params.SessionData())
+		owner, err := t.Owner.ToSQLUsername(params.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return err
 		}

pkg/sql/create_role.go

@@ -86,7 +86,7 @@ func (p *planner) CreateRoleNode(
 		return nil, err
 	}
 
-	roleName, err := roleSpec.ToSQLUsernameWithPurpose(p.SessionData(), security.UsernameCreation)
+	roleName, err := roleSpec.ToSQLUsername(p.SessionData(), security.UsernameCreation)
 	if err != nil {
 		return nil, err
 	}
@@ -251,36 +251,6 @@ func (*CreateRoleNode) Values() tree.Datums { return tree.Datums{} }
 // Close implements the planNode interface.
 func (*CreateRoleNode) Close(context.Context) {}
 
-var blocklistedUsernames = map[security.SQLUsername]struct{}{
-	security.NodeUserName(): {},
-}
-
-// NormalizeAndValidateUsername case folds the specified username and verifies
-// it validates according to the usernameRE regular expression.
-// It rejects reserved user names.
-func NormalizeAndValidateUsername(input string) (security.SQLUsername, error) {
-	username, err := NormalizeAndValidateUsernameNoBlocklist(input)
-	if err != nil {
-		return username, err
-	}
-	if _, ok := blocklistedUsernames[username]; ok {
-		return username, pgerror.Newf(pgcode.ReservedName, "username %q reserved", username)
-	}
-	return username, nil
-}
-
-// NormalizeAndValidateUsernameNoBlocklist case folds the specified username and verifies
-// it validates according to the usernameRE regular expression.
-func NormalizeAndValidateUsernameNoBlocklist(input string) (security.SQLUsername, error) {
-	username, err := security.MakeSQLUsernameFromUserInput(input, security.UsernameCreation)
-	if errors.Is(err, security.ErrUsernameTooLong) {
-		err = pgerror.WithCandidateCode(err, pgcode.NameTooLong)
-	} else if errors.IsAny(err, security.ErrUsernameInvalid, security.ErrUsernameEmpty) {
-		err = pgerror.WithCandidateCode(err, pgcode.InvalidName)
-	}
-	return username, errors.Wrapf(err, "%q", username)
-}
-
 var errNoUserNameSpecified = errors.New("no username specified")
 
 func (p *planner) checkPasswordAndGetHash(

pkg/sql/create_role_test.go

@@ -13,9 +13,11 @@ package sql_test
 import (
 	"testing"
 
-	"github.com/cockroachdb/cockroach/pkg/sql"
+	"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/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -51,7 +53,8 @@ func TestUserName(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		normalized, err := sql.NormalizeAndValidateUsername(tc.username)
+		roleSpec := tree.RoleSpec{RoleSpecType: tree.RoleName, Name: tc.username}
+		normalized, err := roleSpec.ToSQLUsername(&sessiondata.SessionData{}, security.UsernameCreation)
 		if !testutils.IsError(err, tc.err) {
 			t.Errorf("%q: expected %q, got %v", tc.username, tc.err, err)
 			continue

pkg/sql/create_schema.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv"
@@ -51,7 +52,7 @@ func CreateUserDefinedSchemaDescriptor(
 	db catalog.DatabaseDescriptor,
 	allocateID bool,
 ) (*schemadesc.Mutable, *descpb.PrivilegeDescriptor, error) {
-	authRole, err := n.AuthRole.ToSQLUsername(sessionData)
+	authRole, err := n.AuthRole.ToSQLUsername(sessionData, security.UsernameValidation)
 	if err != nil {
 		return nil, nil, err
 	}

pkg/sql/delegate/BUILD.bazel

@@ -40,6 +40,7 @@ go_library(
         "//pkg/jobs",
         "//pkg/jobs/jobspb",
         "//pkg/keys",
+        "//pkg/security",
         "//pkg/settings",
         "//pkg/sql/catalog/catconstants",
         "//pkg/sql/catalog/colinfo",

pkg/sql/delegate/show_default_privileges.go

@@ -13,6 +13,7 @@ package delegate
 import (
 	"fmt"
 
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 )
 
@@ -33,7 +34,7 @@ func (d *delegator) delegateShowDefaultPrivileges(
 	if n.ForAllRoles {
 		query += " AND for_all_roles=true"
 	} else if len(n.Roles) > 0 {
-		targetRoles, err := n.Roles.ToSQLUsernames(d.evalCtx.SessionData())
+		targetRoles, err := n.Roles.ToSQLUsernames(d.evalCtx.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/delegate/show_grants.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -227,7 +228,7 @@ FROM "".information_schema.type_privileges`
 
 	if n.Grantees != nil {
 		params = params[:0]
-		grantees, err := n.Grantees.ToSQLUsernames(d.evalCtx.SessionData())
+		grantees, err := n.Grantees.ToSQLUsernames(d.evalCtx.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return nil, err
 		}
@@ -243,7 +244,7 @@ FROM "".information_schema.type_privileges`
 	// Terminate on invalid users.
 	for _, p := range n.Grantees {
 
-		user, err := p.ToSQLUsername(d.evalCtx.SessionData())
+		user, err := p.ToSQLUsername(d.evalCtx.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/delegate/show_role_grants.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 )
@@ -34,7 +35,7 @@ SELECT role AS role_name,
 
 	if n.Roles != nil {
 		var roles []string
-		sqlUsernames, err := n.Roles.ToSQLUsernames(d.evalCtx.SessionData())
+		sqlUsernames, err := n.Roles.ToSQLUsernames(d.evalCtx.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return nil, err
 		}
@@ -54,7 +55,7 @@ SELECT role AS role_name,
 		}
 
 		var grantees []string
-		granteeSQLUsernames, err := n.Grantees.ToSQLUsernames(d.evalCtx.SessionData())
+		granteeSQLUsernames, err := n.Grantees.ToSQLUsernames(d.evalCtx.SessionData(), security.UsernameValidation)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/drop_role.go

@@ -57,7 +57,7 @@ func (p *planner) DropRoleNode(
 			return nil, pgerror.Newf(pgcode.InvalidParameterValue, "cannot use special role specifier in DROP ROLE")
 		}
 	}
-	roleNames, err := roleSpecs.ToSQLUsernames(p.SessionData())
+	roleNames, err := roleSpecs.ToSQLUsernames(p.SessionData(), security.UsernameCreation)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/grant_revoke.go

@@ -47,7 +47,7 @@ func (p *planner) Grant(ctx context.Context, n *tree.Grant) (planNode, error) {
 		return nil, err
 	}
 
-	grantees, err := n.Grantees.ToSQLUsernames(p.SessionData())
+	grantees, err := n.Grantees.ToSQLUsernames(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}
@@ -77,7 +77,7 @@ func (p *planner) Revoke(ctx context.Context, n *tree.Revoke) (planNode, error)
 		return nil, err
 	}
 
-	grantees, err := n.Grantees.ToSQLUsernames(p.SessionData())
+	grantees, err := n.Grantees.ToSQLUsernames(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/grant_role.go

@@ -64,7 +64,7 @@ func (p *planner) GrantRoleNode(ctx context.Context, n *tree.GrantRole) (*GrantR
 	if err != nil {
 		return nil, err
 	}
-	inputMembers, err := n.Members.ToSQLUsernames(p.SessionData())
+	inputMembers, err := n.Members.ToSQLUsernames(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}

pkg/sql/logictest/testdata/logic_test/drop_user

@@ -142,7 +142,7 @@ DROP USER IF EXISTS user1,user3
 
 statement ok
 PREPARE du AS DROP USER user4;
-EXECUTE du()
+EXECUTE du
 
 query TTT colnames
 SHOW USERS

pkg/sql/logictest/testdata/logic_test/role

@@ -802,10 +802,10 @@ CREATE ROLE crdb_internal_otan
 statement error "foo☂": username is invalid
 CREATE ROLE foo☂
 
-statement error cannot drop role/user public: grants still exist on system.public.comments
+statement error role name "public" is reserved
 DROP USER public
 
-statement error cannot drop role/user public: grants still exist on system.public.comments
+statement error role name "public" is reserved
 DROP ROLE public
 
 statement error role/user public does not exist
@@ -1104,7 +1104,7 @@ role_name  member  is_admin
 statement error role name "public" is reserved
 CREATE USER public
 
-statement error cannot drop role/user public: grants still exist on system.public.comments
+statement error role name "public" is reserved
 DROP USER public
 
 statement ok

pkg/sql/logictest/testdata/logic_test/user

@@ -50,7 +50,7 @@ statement error pgcode 42939 role name "public" is reserved
 CREATE USER public
 
 statement error pgcode 42939 role name "none" is reserved
-CREATE USER none
+CREATE USER "none"
 
 statement error empty passwords are not permitted
 CREATE USER test WITH PASSWORD ''
@@ -102,7 +102,7 @@ user2     ·        {}
 user3     ·        {}
 ομηρος    ·        {}
 
-statement error no username specified
+statement error "": username is empty
 CREATE USER ""
 
 query TTTTT

pkg/sql/reassign_owned_by.go

@@ -43,7 +43,7 @@ func (p *planner) ReassignOwnedBy(ctx context.Context, n *tree.ReassignOwnedBy)
 		return nil, err
 	}
 
-	normalizedOldRoles, err := n.OldRoles.ToSQLUsernames(p.SessionData())
+	normalizedOldRoles, err := n.OldRoles.ToSQLUsernames(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}
@@ -59,7 +59,7 @@ func (p *planner) ReassignOwnedBy(ctx context.Context, n *tree.ReassignOwnedBy)
 			return nil, pgerror.Newf(pgcode.UndefinedObject, "role/user %q does not exist", oldRole)
 		}
 	}
-	newRole, err := n.NewRole.ToSQLUsername(p.SessionData())
+	newRole, err := n.NewRole.ToSQLUsername(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}
@@ -167,7 +167,7 @@ func (n *reassignOwnedByNode) reassignDatabaseOwner(
 	if err != nil {
 		return err
 	}
-	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData())
+	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}
@@ -192,7 +192,7 @@ func (n *reassignOwnedByNode) reassignSchemaOwner(
 	if err != nil {
 		return err
 	}
-	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData())
+	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}
@@ -223,7 +223,7 @@ func (n *reassignOwnedByNode) reassignTableOwner(
 		return err
 	}
 
-	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData())
+	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}
@@ -262,7 +262,7 @@ func (n *reassignOwnedByNode) reassignTypeOwner(
 		return err
 	}
 
-	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData())
+	owner, err := n.n.NewRole.ToSQLUsername(params.p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return err
 	}

pkg/sql/revoke_role.go

@@ -61,7 +61,7 @@ func (p *planner) RevokeRoleNode(ctx context.Context, n *tree.RevokeRole) (*Revo
 	if err != nil {
 		return nil, err
 	}
-	inputMembers, err := n.Members.ToSQLUsernames(p.SessionData())
+	inputMembers, err := n.Members.ToSQLUsernames(p.SessionData(), security.UsernameValidation)
 	if err != nil {
 		return nil, err
 	}