[release-23.0] vtctldclient GetPermissions: hide authentication_string from response (#18771) (#18799)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>

Author: vitess-bot[bot]

go/cmd/vtctldclient/command/permissions.go

@@ -24,6 +24,7 @@ import (
 	"vitess.io/vitess/go/cmd/vtctldclient/cli"
 	"vitess.io/vitess/go/vt/topo/topoproto"
 
+	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
 	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
 )
 
@@ -56,6 +57,26 @@ var (
 	}
 )
 
+// redactUserPermissions ensures sensitive info is redacted from a
+// *tabletmanagerdatapb.Permissions response.
+func redactUserPermissions(perms *tabletmanagerdatapb.Permissions) {
+	if perms == nil {
+		return
+	}
+	for _, up := range perms.UserPermissions {
+		if up == nil {
+			continue
+		}
+		if up.Privileges != nil {
+			// Remove the "authentication_string" field, which is a
+			// sensitive field from the mysql.users table. This is
+			// redacted server-side in v23+ so this line can be
+			// removed in the future.
+			delete(up.Privileges, "authentication_string")
+		}
+	}
+}
+
 func commandGetPermissions(cmd *cobra.Command, args []string) error {
 	alias, err := topoproto.ParseTabletAlias(cmd.Flags().Arg(0))
 	if err != nil {
@@ -70,13 +91,9 @@ func commandGetPermissions(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	// Obfuscate the checksum so as not to potentially display sensitive info.
+	// Obfuscate the secrets so as not to potentially display sensitive info.
 	if resp != nil && resp.Permissions != nil {
-		for _, up := range resp.Permissions.UserPermissions {
-			if up != nil {
-				up.PasswordChecksum = 0
-			}
-		}
+		redactUserPermissions(resp.Permissions)
 	}
 	cli.DefaultMarshalOptions.EmitUnpopulated = false
 	p, err := cli.MarshalJSON(resp.Permissions)

go/cmd/vtctldclient/command/permissions_test.go

@@ -33,6 +33,7 @@ import (
 	"vitess.io/vitess/go/vt/topo/memorytopo"
 
 	querypb "vitess.io/vitess/go/vt/proto/query"
+	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
 )
@@ -606,3 +607,27 @@ func TestPermissions(t *testing.T) {
 	})
 	require.ErrorContains(t, err, "has an extra user")
 }
+
+func TestRedactUserPermissions(t *testing.T) {
+	perms := &tabletmanagerdatapb.Permissions{
+		UserPermissions: []*tabletmanagerdatapb.UserPermission{
+			{
+				Host: "%",
+				User: "vt",
+				Privileges: map[string]string{
+					"authentication_string": "this should be removed from the response",
+				},
+			},
+		},
+	}
+	redactUserPermissions(perms)
+	require.EqualValues(t, &tabletmanagerdatapb.Permissions{
+		UserPermissions: []*tabletmanagerdatapb.UserPermission{
+			{
+				Host:       "%",
+				User:       "vt",
+				Privileges: map[string]string{},
+			},
+		},
+	}, perms)
+}

go/vt/mysqlctl/tmutils/permissions.go

@@ -73,6 +73,9 @@ func NewUserPermission(fields []*querypb.Field, values []sqltypes.Value) *tablet
 		case "password_last_changed":
 			// we skip this one, as the value may be
 			// different on primary and replicas.
+		case "authentication_string":
+		// skip authentication_string as it
+		// may contain a password hash.
 		default:
 			up.Privileges[field.Name] = values[i].ToString()
 		}

go/vt/mysqlctl/tmutils/permissions_test.go

@@ -19,9 +19,10 @@ package tmutils
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"vitess.io/vitess/go/sqltypes"
 	querypb "vitess.io/vitess/go/vt/proto/query"
-
 	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
 )
 
@@ -60,6 +61,29 @@ func testPermissionsDiff(t *testing.T, left, right *tabletmanagerdatapb.Permissi
 	}
 }
 
+func TestNewUserPermission(t *testing.T) {
+	up := NewUserPermission(mapToSQLResults(map[string]string{
+		"Host":        "%",
+		"User":        "vt",
+		"Password":    "correct horse battery staple",
+		"Select_priv": "Y",
+		"Insert_priv": "N",
+		// Test the next field is skipped (to avoid date drifts).
+		"password_last_changed": "2016-11-08 02:56:23",
+		// Test the next field is filtered.
+		"authentication_string": "this should be filtered out",
+	}))
+	require.EqualValues(t, &tabletmanagerdatapb.UserPermission{
+		Host:             "%",
+		User:             "vt",
+		PasswordChecksum: 17759204488013904955,
+		Privileges: map[string]string{
+			"Insert_priv": "N",
+			"Select_priv": "Y",
+		},
+	}, up)
+}
+
 func TestPermissionsDiff(t *testing.T) {
 
 	p1 := &tabletmanagerdatapb.Permissions{}