Skip to content

Commit 6c19187

Browse files
Use a practical method of testing permissions by attempting to perform CRUD on the source sidecar database
Signed-off-by: Rohit Nayak <[email protected]>
1 parent 826d78d commit 6c19187

File tree

1 file changed

+83
-2
lines changed

1 file changed

+83
-2
lines changed

go/vt/vttablet/tabletmanager/rpc_vreplication.go

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ limit 1
9999
sqlGetMaxSequenceVal = "select max(%a) as maxval from %a.%a"
100100
sqlInitSequenceTable = "insert into %a.%a (id, next_id, cache) values (0, %d, 1000) on duplicate key update next_id = if(next_id < %d, %d, next_id)"
101101
sqlCreateSequenceTable = "create table if not exists %a (id int, next_id bigint, cache bigint, primary key(id)) comment 'vitess_sequence'"
102+
103+
// Functional permission testing queries - test each permission individually without accessing mysql.user table
104+
sqlTestVReplicationSelectPermission = "select count(*) from %s.vreplication limit 1"
105+
sqlTestVReplicationInsertPermission = "insert into %s.vreplication (workflow, source, pos, max_tps, max_replication_lag, cell, tablet_types, time_updated, transaction_timestamp, state, db_name, workflow_type, workflow_sub_type, defer_secondary_keys, options) values (%a, '', '', 0, 0, '', '', now(), 0, 'Stopped', '__test__', 0, 0, false, '{}')"
106+
sqlTestVReplicationUpdatePermission = "update %s.vreplication set message = '__test_update__' where workflow = %a and db_name = '__test__'"
107+
sqlTestVReplicationDeletePermission = "delete from %s.vreplication where workflow = %a and db_name = '__test__'"
102108
)
103109

104110
var (
@@ -839,10 +845,13 @@ func (tm *TabletManager) createSequenceTable(ctx context.Context, escapedTableNa
839845
return err
840846
}
841847

842-
// ValidateVReplicationPermissions validates that the --db_filtered_user has
848+
// ValidateVReplicationPermissionsOld validates that the --db_filtered_user has
843849
// the minimum permissions required on the sidecardb vreplication table
844850
// needed in order to manage vreplication metadata.
845-
func (tm *TabletManager) ValidateVReplicationPermissions(ctx context.Context, req *tabletmanagerdatapb.ValidateVReplicationPermissionsRequest) (*tabletmanagerdatapb.ValidateVReplicationPermissionsResponse, error) {
851+
// Switching to use a functional test approach in ValidateVReplicationPermissions below
852+
// instead of querying mysql.user table directly as that requires permissions on the mysql.user table.
853+
// Leaving this here for now in case we want to revert back.
854+
func (tm *TabletManager) ValidateVReplicationPermissionsOld(ctx context.Context, req *tabletmanagerdatapb.ValidateVReplicationPermissionsRequest) (*tabletmanagerdatapb.ValidateVReplicationPermissionsResponse, error) {
846855
query, err := sqlparser.ParseAndBind(sqlValidateVReplicationPermissions,
847856
sqltypes.StringBindVariable(tm.DBConfigs.Filtered.User),
848857
sqltypes.StringBindVariable(sidecar.GetName()),
@@ -883,6 +892,78 @@ func (tm *TabletManager) ValidateVReplicationPermissions(ctx context.Context, re
883892
}, nil
884893
}
885894

895+
// ValidateVReplicationPermissions validates that the --db_filtered_user has
896+
// the minimum permissions required on the sidecardb vreplication table
897+
// using a functional testing approach that doesn't require access to mysql.user table.
898+
func (tm *TabletManager) ValidateVReplicationPermissions(ctx context.Context, req *tabletmanagerdatapb.ValidateVReplicationPermissionsRequest) (*tabletmanagerdatapb.ValidateVReplicationPermissionsResponse, error) {
899+
log.Infof("Validating VReplication permissions on sidecar db %s", tm.tabletAlias)
900+
901+
conn, err := tm.MysqlDaemon.GetAllPrivsConnection(ctx)
902+
if err != nil {
903+
return nil, err
904+
}
905+
defer conn.Close()
906+
907+
if _, err := conn.ExecuteFetch("START TRANSACTION", 1, false); err != nil {
908+
return nil, vterrors.Wrap(err, "failed to start transaction for permission testing")
909+
}
910+
defer func() {
911+
conn.ExecuteFetch("ROLLBACK", 1, false)
912+
}()
913+
914+
// Create a unique test workflow name to avoid conflicts using timestamp and random component
915+
testWorkflow := fmt.Sprintf("__permission_test_%d_%d", time.Now().Unix(), time.Now().Nanosecond()%1000000)
916+
sidecarDB := sidecar.GetName()
917+
918+
permissionTests := []struct {
919+
permission string
920+
sqlTemplate string
921+
}{
922+
{"SELECT", sqlTestVReplicationSelectPermission},
923+
{"INSERT", sqlTestVReplicationInsertPermission},
924+
{"UPDATE", sqlTestVReplicationUpdatePermission},
925+
{"DELETE", sqlTestVReplicationDeletePermission},
926+
}
927+
928+
for _, test := range permissionTests {
929+
var query string
930+
var err error
931+
932+
if test.permission == "SELECT" {
933+
parsed := sqlparser.BuildParsedQuery(test.sqlTemplate, sidecar.GetIdentifier())
934+
query, err = parsed.GenerateQuery(nil, nil)
935+
} else {
936+
parsed := sqlparser.BuildParsedQuery(test.sqlTemplate, sidecar.GetIdentifier(), ":workflow")
937+
query, err = parsed.GenerateQuery(map[string]*querypb.BindVariable{
938+
"workflow": sqltypes.StringBindVariable(testWorkflow),
939+
}, nil)
940+
}
941+
942+
if err != nil {
943+
return nil, vterrors.Wrapf(err, "failed to bind %s query for permission testing", test.permission)
944+
}
945+
946+
log.Infof("Testing %s permission using query: %s", test.permission, query)
947+
if _, err := conn.ExecuteFetch(query, 1, false); err != nil {
948+
return &tabletmanagerdatapb.ValidateVReplicationPermissionsResponse{
949+
User: tm.DBConfigs.Filtered.User,
950+
Ok: false,
951+
Error: fmt.Sprintf("user %s does not have %s permission on %s.vreplication table on tablet %s: %v",
952+
tm.DBConfigs.Filtered.User, test.permission, sidecarDB, topoproto.TabletAliasString(tm.tabletAlias), err),
953+
}, nil
954+
}
955+
}
956+
957+
log.Infof("VReplication sidecardb permission validation succeeded for user %s on tablet %s",
958+
tm.DBConfigs.Filtered.User, tm.tabletAlias)
959+
960+
return &tabletmanagerdatapb.ValidateVReplicationPermissionsResponse{
961+
User: tm.DBConfigs.Filtered.User,
962+
Ok: true,
963+
Error: "",
964+
}, nil
965+
}
966+
886967
// VReplicationExec executes a vreplication command.
887968
func (tm *TabletManager) VReplicationExec(ctx context.Context, query string) (*querypb.QueryResult, error) {
888969
// Replace any provided sidecar database qualifiers with the correct one.

0 commit comments

Comments
 (0)