From 376d5428cca95ae51143162ee0f7f3972e84cff7 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 6 Dec 2021 23:59:55 -0700 Subject: [PATCH] *: Fix use of user identity in SHOW GRANTS + error messages (#30294) --- expression/builtin_info.go | 5 ++--- expression/builtin_info_vec.go | 4 ++-- parser/auth/auth.go | 12 +++++++--- privilege/privileges/privileges_test.go | 29 +++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/expression/builtin_info.go b/expression/builtin_info.go index 66c166c0d785f..013ee74d66bac 100644 --- a/expression/builtin_info.go +++ b/expression/builtin_info.go @@ -190,7 +190,7 @@ func (b *builtinCurrentUserSig) evalString(row chunk.Row) (string, bool, error) if data == nil || data.User == nil { return "", true, errors.Errorf("Missing session variable when eval builtin") } - return data.User.AuthIdentityString(), false, nil + return data.User.String(), false, nil } type currentRoleFunctionClass struct { @@ -278,8 +278,7 @@ func (b *builtinUserSig) evalString(row chunk.Row) (string, bool, error) { if data == nil || data.User == nil { return "", true, errors.Errorf("Missing session variable when eval builtin") } - - return data.User.String(), false, nil + return data.User.LoginString(), false, nil } type connectionIDFunctionClass struct { diff --git a/expression/builtin_info_vec.go b/expression/builtin_info_vec.go index 20c6af4de9e4b..c4343f669e60b 100644 --- a/expression/builtin_info_vec.go +++ b/expression/builtin_info_vec.go @@ -114,7 +114,7 @@ func (b *builtinCurrentUserSig) vecEvalString(input *chunk.Chunk, result *chunk. return errors.Errorf("Missing session variable when eval builtin") } for i := 0; i < n; i++ { - result.AppendString(data.User.AuthIdentityString()) + result.AppendString(data.User.String()) } return nil } @@ -168,7 +168,7 @@ func (b *builtinUserSig) vecEvalString(input *chunk.Chunk, result *chunk.Column) result.ReserveString(n) for i := 0; i < n; i++ { - result.AppendString(data.User.String()) + result.AppendString(data.User.LoginString()) } return nil } diff --git a/parser/auth/auth.go b/parser/auth/auth.go index cb94346f82208..5657c6c276646 100644 --- a/parser/auth/auth.go +++ b/parser/auth/auth.go @@ -41,18 +41,24 @@ func (user *UserIdentity) Restore(ctx *format.RestoreCtx) error { } // String converts UserIdentity to the format user@host. +// It defaults to providing the AuthIdentity (the matching entry in priv tables) +// To use the actual identity use LoginString() func (user *UserIdentity) String() string { // TODO: Escape username and hostname. if user == nil { return "" } + if user.AuthUsername != "" { + return fmt.Sprintf("%s@%s", user.AuthUsername, user.AuthHostname) + } return fmt.Sprintf("%s@%s", user.Username, user.Hostname) } -// AuthIdentityString returns matched identity in user@host format -func (user *UserIdentity) AuthIdentityString() string { +// LoginString returns matched identity in user@host format +// It matches the login user. +func (user *UserIdentity) LoginString() string { // TODO: Escape username and hostname. - return fmt.Sprintf("%s@%s", user.AuthUsername, user.AuthHostname) + return fmt.Sprintf("%s@%s", user.Username, user.Hostname) } type RoleIdentity struct { diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index 4ad6be46c339f..c4cee0cbf1ce2 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -372,6 +372,35 @@ func TestShowGrants(t *testing.T) { require.Len(t, gs, 3) } +// TestErrorMessage checks that the identity in error messages matches the mysql.user table one. +// MySQL is inconsistent in its error messages, as some match the loginHost and others the +// identity from mysql.user. In TiDB we now use the identity from mysql.user in error messages +// for consistency. +func TestErrorMessage(t *testing.T) { + t.Parallel() + store, clean := newStore(t) + defer clean() + + rootSe := newSession(t, store, dbName) + mustExec(t, rootSe, `CREATE USER wildcard`) + mustExec(t, rootSe, `CREATE USER specifichost@192.168.1.1`) + mustExec(t, rootSe, `GRANT SELECT on test.* TO wildcard`) + mustExec(t, rootSe, `GRANT SELECT on test.* TO specifichost@192.168.1.1`) + + wildSe := newSession(t, store, dbName) + + // The session.Auth() func will populate the AuthUsername and AuthHostname fields. + // We don't have to explicitly specify them. + require.True(t, wildSe.Auth(&auth.UserIdentity{Username: "wildcard", Hostname: "192.168.1.1"}, nil, nil)) + _, err := wildSe.ExecuteInternal(context.Background(), "use mysql;") + require.Equal(t, "[executor:1044]Access denied for user 'wildcard'@'%' to database 'mysql'", err.Error()) + + specificSe := newSession(t, store, dbName) + require.True(t, specificSe.Auth(&auth.UserIdentity{Username: "specifichost", Hostname: "192.168.1.1"}, nil, nil)) + _, err = specificSe.ExecuteInternal(context.Background(), "use mysql;") + require.Equal(t, "[executor:1044]Access denied for user 'specifichost'@'192.168.1.1' to database 'mysql'", err.Error()) +} + func TestShowColumnGrants(t *testing.T) { t.Parallel() store, clean := newStore(t)