*: Fix use of user identity in SHOW GRANTS + error messages (#30294)

pingcap · Dec 7, 2021 · 376d542 · 376d542
1 parent ffd59ec
commit 376d542
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)