*: fix revoke statement for CURRENT_USER() and refine error message (#…

…24052)

* *: fix revoke statement for CURRENT_USER() and refine error message

errno/errname.go

@@ -1020,7 +1020,7 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{
 	ErrBuildExecutor:                       mysql.Message("Failed to build executor", nil),
 	ErrBatchInsertFail:                     mysql.Message("Batch insert failed, please clean the table and try again.", nil),
 	ErrGetStartTS:                          mysql.Message("Can not get start ts", nil),
-	ErrPrivilegeCheckFail:                  mysql.Message("privilege check fail", nil), // this error message should begin lowercased to be compatible with the test
+	ErrPrivilegeCheckFail:                  mysql.Message("privilege check for '%s' fail", nil), // this error message should begin lowercased to be compatible with the test
 	ErrInvalidWildCard:                     mysql.Message("Wildcard fields without any table name appears in wrong place", nil),
 	ErrMixOfGroupFuncAndFieldsIncompatible: mysql.Message("In aggregated query without GROUP BY, expression #%d of SELECT list contains nonaggregated column '%s'; this is incompatible with sql_mode=only_full_group_by", nil),
 	ErrUnsupportedSecondArgumentType:       mysql.Message("JSON_OBJECTAGG: unsupported second argument type %v", nil),

errors.toml

@@ -1133,7 +1133,7 @@ Schema has changed
 
 ["planner:8121"]
 error = '''
-privilege check fail
+privilege check for '%s' fail
 '''
 
 ["planner:8122"]

executor/revoke.go

@@ -88,8 +88,14 @@ func (e *RevokeExec) Next(ctx context.Context, req *chunk.Chunk) error {
 		return err
 	}
 
+	sessVars := e.ctx.GetSessionVars()
 	// Revoke for each user.
 	for _, user := range e.Users {
+		if user.User.CurrentUser {
+			user.User.Username = sessVars.User.AuthUsername
+			user.User.Hostname = sessVars.User.AuthHostname
+		}
+
 		// Check if user exists.
 		exists, err := userExists(e.ctx, user.User.Username, user.User.Hostname)
 		if err != nil {

planner/core/optimizer.go

@@ -103,13 +103,13 @@ func CheckPrivilege(activeRoles []*auth.RoleIdentity, pm privilege.Manager, vs [
 		if v.privilege == mysql.ExtendedPriv {
 			if !pm.RequestDynamicVerification(activeRoles, v.dynamicPriv, v.dynamicWithGrant) {
 				if v.err == nil {
-					return ErrPrivilegeCheckFail
+					return ErrPrivilegeCheckFail.GenWithStackByArgs(v.dynamicPriv)
 				}
 				return v.err
 			}
 		} else if !pm.RequestVerification(activeRoles, v.db, v.table, v.column, v.privilege) {
 			if v.err == nil {
-				return ErrPrivilegeCheckFail
+				return ErrPrivilegeCheckFail.GenWithStackByArgs(v.privilege.String())
 			}
 			return v.err
 		}

planner/core/point_get_plan.go

@@ -994,7 +994,7 @@ func checkFastPlanPrivilege(ctx sessionctx.Context, dbName, tableName string, ch
 	var visitInfos []visitInfo
 	for _, checkType := range checkTypes {
 		if pm != nil && !pm.RequestVerification(ctx.GetSessionVars().ActiveRoles, dbName, tableName, "", checkType) {
-			return errors.New("privilege check fail")
+			return ErrPrivilegeCheckFail.GenWithStackByArgs(checkType.String())
 		}
 		// This visitInfo is only for table lock check, so we do not need column field,
 		// just fill it empty string.

privilege/privileges/privileges_test.go

@@ -843,6 +843,12 @@ func (s *testPrivilegeSuite) TestRevokePrivileges(c *C) {
 	c.Assert(se.Auth(&auth.UserIdentity{Username: "hasgrant", Hostname: "localhost", AuthUsername: "hasgrant", AuthHostname: "%"}, nil, nil), IsTrue)
 	mustExec(c, se, "REVOKE SELECT ON mysql.* FROM 'withoutgrant'")
 	mustExec(c, se, "REVOKE ALL ON mysql.* FROM withoutgrant")
+
+	// For issue https://github.com/pingcap/tidb/issues/23850
+	mustExec(c, se, "CREATE USER u4")
+	mustExec(c, se, "GRANT ALL ON *.* TO u4 WITH GRANT OPTION")
+	c.Assert(se.Auth(&auth.UserIdentity{Username: "u4", Hostname: "localhost", AuthUsername: "u4", AuthHostname: "%"}, nil, nil), IsTrue)
+	mustExec(c, se, "REVOKE ALL ON *.* FROM CURRENT_USER()")
 }
 
 func (s *testPrivilegeSuite) TestSetGlobal(c *C) {
@@ -1006,14 +1012,14 @@ func (s *testPrivilegeSuite) TestSystemSchema(c *C) {
 	_, err = se.ExecuteInternal(context.Background(), "drop table information_schema.tables")
 	c.Assert(strings.Contains(err.Error(), "denied to user"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "update information_schema.tables set table_name = 'tst' where table_name = 'mysql'")
-	c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)
+	c.Assert(strings.Contains(err.Error(), "privilege check"), IsTrue)
 
 	// Test performance_schema.
 	mustExec(c, se, `select * from performance_schema.events_statements_summary_by_digest`)
 	_, err = se.ExecuteInternal(context.Background(), "drop table performance_schema.events_statements_summary_by_digest")
 	c.Assert(strings.Contains(err.Error(), "denied to user"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "update performance_schema.events_statements_summary_by_digest set schema_name = 'tst'")
-	c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)
+	c.Assert(strings.Contains(err.Error(), "privilege check"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "delete from performance_schema.events_statements_summary_by_digest")
 	c.Assert(strings.Contains(err.Error(), "DELETE command denied to user"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "create table performance_schema.t(a int)")
@@ -1025,7 +1031,7 @@ func (s *testPrivilegeSuite) TestSystemSchema(c *C) {
 	_, err = se.ExecuteInternal(context.Background(), "drop table metrics_schema.tidb_query_duration")
 	c.Assert(strings.Contains(err.Error(), "denied to user"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "update metrics_schema.tidb_query_duration set instance = 'tst'")
-	c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)
+	c.Assert(strings.Contains(err.Error(), "privilege check"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "delete from metrics_schema.tidb_query_duration")
 	c.Assert(strings.Contains(err.Error(), "DELETE command denied to user"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "create table metric_schema.t(a int)")
@@ -1041,9 +1047,9 @@ func (s *testPrivilegeSuite) TestAdminCommand(c *C) {
 
 	c.Assert(se.Auth(&auth.UserIdentity{Username: "test_admin", Hostname: "localhost"}, nil, nil), IsTrue)
 	_, err := se.ExecuteInternal(context.Background(), "ADMIN SHOW DDL JOBS")
-	c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)
+	c.Assert(strings.Contains(err.Error(), "privilege check"), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "ADMIN CHECK TABLE t")
-	c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)
+	c.Assert(strings.Contains(err.Error(), "privilege check"), IsTrue)
 
 	c.Assert(se.Auth(&auth.UserIdentity{Username: "root", Hostname: "localhost"}, nil, nil), IsTrue)
 	_, err = se.ExecuteInternal(context.Background(), "ADMIN SHOW DDL JOBS")

session/session_test.go

@@ -2892,7 +2892,7 @@ func (s *testSessionSuite2) TestUpdatePrivilege(c *C) {
 
 	_, err := tk1.Exec("update t2 set id = 666 where id = 1;")
 	c.Assert(err, NotNil)
-	c.Assert(strings.Contains(err.Error(), "privilege check fail"), IsTrue)
+	c.Assert(strings.Contains(err.Error(), "privilege check"), IsTrue)
 
 	// Cover a bug that t1 and t2 both require update privilege.
 	// In fact, the privlege check for t1 should be update, and for t2 should be select.