Skip to content

Commit dc65753

Browse files
committed
sql: wire DROP PROVISIONED ROLES into opaque dispatch
Register DropProvisionedRoles in opaque.go so the SQL executor dispatches to the plan node. Add the missing String() method on the AST node to satisfy the tree.Statement interface. Epic CRDB-52460 fixes None Release note (sql change): The DROP PROVISIONED ROLES statement is now fully wired into the SQL execution pipeline. It bulk-drops provisioned (auto-created) users matching filter criteria, skipping users that own objects or have other dependencies. The existing DROP ROLE / DROP USER statements are not affected by this change. Examples: DROP PROVISIONED ROLES; DROP PROVISIONED ROLES WITH SOURCE = 'ldap:ldap.example.com'; DROP PROVISIONED ROLES WITH SOURCE = 'oidc:okta.corp.com', LAST ACCESS TIME OLDER THAN '2025-01-01' LIMIT 100;
1 parent 75ddc6c commit dc65753

6 files changed

Lines changed: 268 additions & 137 deletions

File tree

pkg/sql/drop_provisioned_roles.go

Lines changed: 192 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,41 @@ import (
1111
"strings"
1212

1313
"github.com/cockroachdb/cockroach/pkg/security/username"
14+
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
15+
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
1416
"github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree"
15-
"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
17+
"github.com/cockroachdb/cockroach/pkg/sql/paramparse"
18+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
19+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
1620
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
1721
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
1822
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
23+
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
1924
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
2025
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
2126
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
2227
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
28+
"github.com/cockroachdb/cockroach/pkg/sql/types"
2329
"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
2430
"github.com/cockroachdb/redact"
2531
)
2632

33+
// maxDropProvisionedRolesLimit is the maximum number of roles that can
34+
// be dropped in a single DROP PROVISIONED ROLES statement. Each role
35+
// requires dependency checks across all descriptors plus multiple
36+
// system table mutations, so we cap the batch size to keep
37+
// transactions bounded.
38+
const maxDropProvisionedRolesLimit = 1024
39+
2740
// DropProvisionedRolesNode drops provisioned users matching filter
28-
// criteria (SOURCE, LAST LOGIN BEFORE) with an optional
29-
// LIMIT. Users that own objects or have dependencies are skipped with
30-
// a NOTICE rather than failing the entire operation.
41+
// criteria (SOURCE, LAST LOGIN BEFORE) with a required LIMIT. Users
42+
// that own objects or have dependencies are skipped with a NOTICE
43+
// rather than failing the entire operation.
3144
type DropProvisionedRolesNode struct {
3245
zeroInputPlanNode
33-
options *tree.DropProvisionedRolesOptions
34-
limit *tree.Limit
46+
source tree.TypedExpr // nil if no SOURCE filter
47+
lastLoginBefore tree.TypedExpr // nil if no LAST LOGIN BEFORE filter
48+
limitCount int64
3549
}
3650

3751
// DropProvisionedRoles creates a plan node for DROP PROVISIONED ROLES.
@@ -42,29 +56,84 @@ func (p *planner) DropProvisionedRoles(
4256
if err := p.CheckGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEROLE); err != nil {
4357
return nil, err
4458
}
45-
return &DropProvisionedRolesNode{
46-
options: n.Options,
47-
limit: n.Limit,
48-
}, nil
59+
// LIMIT is required to prevent accidentally dropping an unbounded
60+
// number of roles in a single transaction.
61+
if n.Limit == nil || n.Limit.Count == nil {
62+
return nil, pgerror.Newf(pgcode.InvalidParameterValue,
63+
"LIMIT is required for DROP PROVISIONED ROLES")
64+
}
65+
// Validate that the LIMIT expression is a constant integer to
66+
// prevent subqueries or other expressions from being smuggled
67+
// into the internal query.
68+
numVal, ok := n.Limit.Count.(*tree.NumVal)
69+
if !ok {
70+
return nil, pgerror.Newf(pgcode.InvalidParameterValue,
71+
"LIMIT must be a constant integer expression")
72+
}
73+
limitInt, err := numVal.AsInt64()
74+
if err != nil {
75+
return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue,
76+
"LIMIT must be an integer")
77+
}
78+
if limitInt <= 0 || limitInt > maxDropProvisionedRolesLimit {
79+
return nil, pgerror.Newf(pgcode.InvalidParameterValue,
80+
"LIMIT must be between 1 and %d", maxDropProvisionedRolesLimit)
81+
}
82+
83+
// Type-check SOURCE and LAST LOGIN BEFORE expressions so that
84+
// expressions like now() - '7d'::interval are properly evaluated
85+
// at execution time rather than being pretty-printed as text.
86+
node := &DropProvisionedRolesNode{limitCount: limitInt}
87+
var dummyHelper tree.IndexedVarHelper
88+
89+
if n.Options != nil && n.Options.Source != nil {
90+
expr := paramparse.UnresolvedNameToStrVal(n.Options.Source)
91+
typed, err := p.analyzeExpr(
92+
ctx, expr, dummyHelper, types.String, true, /* requireType */
93+
"DROP PROVISIONED ROLES SOURCE",
94+
)
95+
if err != nil {
96+
return nil, err
97+
}
98+
node.source = typed
99+
}
100+
101+
if n.Options != nil && n.Options.LastLoginBefore != nil {
102+
expr := paramparse.UnresolvedNameToStrVal(n.Options.LastLoginBefore)
103+
typed, err := p.analyzeExpr(
104+
ctx, expr, dummyHelper, types.TimestampTZ, true, /* requireType */
105+
"DROP PROVISIONED ROLES LAST LOGIN BEFORE",
106+
)
107+
if err != nil {
108+
return nil, err
109+
}
110+
node.lastLoginBefore = typed
111+
}
112+
113+
return node, nil
49114
}
50115

51116
func (n *DropProvisionedRolesNode) startExec(params runParams) error {
52-
sqltelemetry.IncIAMDropCounter(sqltelemetry.User)
117+
sqltelemetry.IncIAMDropCounter(sqltelemetry.Role)
53118
const opName redact.RedactableString = "drop-provisioned-roles"
54119

55120
hasAdmin, err := params.p.HasAdminRole(params.ctx)
56121
if err != nil {
57122
return err
58123
}
59124

60-
// Build the query to find matching provisioned users.
61-
query, queryArgs := n.buildFilterQuery()
125+
// Evaluate filter expressions and build the query.
126+
sourceVal, lastLoginVal, err := n.evalFilterExprs(params)
127+
if err != nil {
128+
return err
129+
}
130+
query, queryArgs := buildProvisionedRolesQuery(sourceVal, lastLoginVal, n.limitCount)
62131

63132
rows, err := params.p.InternalSQLTxn().QueryBufferedEx(
64133
params.ctx,
65134
"drop-provisioned-roles-find",
66135
params.p.txn,
67-
sessiondata.NodeUserSessionDataOverride,
136+
sessiondata.InternalExecutorOverride{User: params.p.User()},
68137
query,
69138
queryArgs...,
70139
)
@@ -89,10 +158,11 @@ func (n *DropProvisionedRolesNode) startExec(params runParams) error {
89158
string(tree.MustBeDString(row[0])),
90159
)
91160

92-
// Skip reserved roles.
93-
if normalizedUsername.IsAdminRole() ||
94-
normalizedUsername.IsPublicRole() ||
95-
normalizedUsername.IsRootUser() {
161+
// Skip reserved roles (public, node, pg_*, crdb_internal_*),
162+
// root, and admin.
163+
if normalizedUsername.IsReserved() ||
164+
normalizedUsername.IsRootUser() ||
165+
normalizedUsername.IsAdminRole() {
96166
continue
97167
}
98168

@@ -171,63 +241,82 @@ func (n *DropProvisionedRolesNode) startExec(params runParams) error {
171241
return nil
172242
}
173243

174-
// buildFilterQuery constructs the SQL query to find provisioned users
175-
// matching the filter options.
176-
func (n *DropProvisionedRolesNode) buildFilterQuery() (string, []interface{}) {
244+
// evalFilterExprs evaluates the typed SOURCE and LAST LOGIN BEFORE
245+
// expressions using the session's EvalContext, returning concrete
246+
// Go values suitable for parameterized query arguments.
247+
func (n *DropProvisionedRolesNode) evalFilterExprs(
248+
params runParams,
249+
) (sourceVal *string, lastLoginVal *tree.DTimestampTZ, err error) {
250+
if n.source != nil {
251+
d, err := eval.Expr(params.ctx, params.EvalContext(), n.source)
252+
if err != nil {
253+
return nil, nil, err
254+
}
255+
s := string(tree.MustBeDString(d))
256+
sourceVal = &s
257+
}
258+
if n.lastLoginBefore != nil {
259+
d, err := eval.Expr(params.ctx, params.EvalContext(), n.lastLoginBefore)
260+
if err != nil {
261+
return nil, nil, err
262+
}
263+
v := tree.MustBeDTimestampTZ(d)
264+
lastLoginVal = &v
265+
}
266+
return sourceVal, lastLoginVal, nil
267+
}
268+
269+
// buildProvisionedRolesQuery constructs the parameterized SQL query
270+
// to find provisioned users matching the given filter values.
271+
func buildProvisionedRolesQuery(
272+
sourceVal *string, lastLoginVal *tree.DTimestampTZ, limitCount int64,
273+
) (string, []any) {
177274
var whereExprs []string
178-
var args []interface{}
275+
var args []any
179276
argIdx := 1
180277

181278
// Always filter for users that have a PROVISIONSRC role option
182279
// (i.e. are provisioned).
183-
provisionFilter := fmt.Sprintf(`EXISTS (
184-
SELECT 1 FROM system.role_options AS src
185-
WHERE src.username = u.username
186-
AND src.option = 'PROVISIONSRC'`)
187-
188-
if n.options != nil && n.options.Source != nil {
189-
sourceStr := tree.AsStringWithFlags(
190-
n.options.Source, tree.FmtBareStrings,
191-
)
192-
provisionFilter += fmt.Sprintf(
193-
"\n\t\tAND src.value = %s", lexbase.EscapeSQLString(sourceStr),
194-
)
280+
provisionFilter := "EXISTS (SELECT 1 FROM system.role_options AS src" +
281+
" WHERE src.username = u.username AND src.option = 'PROVISIONSRC'"
282+
283+
if sourceVal != nil {
284+
provisionFilter += fmt.Sprintf(" AND src.value = $%d", argIdx)
285+
args = append(args, *sourceVal)
286+
argIdx++
195287
}
196-
provisionFilter += "\n)"
288+
provisionFilter += ")"
197289
whereExprs = append(whereExprs, provisionFilter)
198290

199-
if n.options != nil && n.options.LastLoginBefore != nil {
200-
tsExpr := tree.AsStringWithFlags(
201-
n.options.LastLoginBefore, tree.FmtParsable,
202-
)
291+
if lastLoginVal != nil {
203292
whereExprs = append(whereExprs, fmt.Sprintf(
204-
"u.estimated_last_login_time < (%s)::TIMESTAMPTZ", tsExpr,
293+
"(u.estimated_last_login_time IS NULL OR u.estimated_last_login_time < $%d)", argIdx,
205294
))
295+
args = append(args, lastLoginVal.Time)
296+
argIdx++
206297
}
207298

208-
whereClause := "\nWHERE " + strings.Join(whereExprs, "\n\tAND ")
299+
whereClause := " WHERE " + strings.Join(whereExprs, " AND ")
209300

210-
var limitClause string
211-
if n.limit != nil && n.limit.Count != nil {
212-
limitClause = fmt.Sprintf("\nLIMIT %s", tree.AsString(n.limit.Count))
213-
}
301+
limitClause := fmt.Sprintf(" LIMIT $%d", argIdx)
302+
args = append(args, limitCount)
214303

215304
query := fmt.Sprintf(
216305
`SELECT u.username FROM system.users AS u%s%s`,
217306
whereClause, limitClause,
218307
)
219308

220-
_ = argIdx // args currently embedded via string escaping
221309
return query, args
222310
}
223311

224312
// userHasDependencies checks whether the given user owns any objects,
225-
// has grants, default privileges, scheduled jobs, or system
226-
// privileges that would prevent dropping.
313+
// has grants, default privileges, row-level security policies,
314+
// scheduled jobs, or system privileges that would prevent dropping.
227315
func (n *DropProvisionedRolesNode) userHasDependencies(
228316
params runParams, normalizedUsername username.SQLUsername, allDescs nstree.Catalog,
229317
) (bool, error) {
230-
// Check ownership across all descriptors.
318+
// Check ownership, grants, default privileges, and RLS policies
319+
// across all descriptors.
231320
for _, desc := range allDescs.OrderedDescriptors() {
232321
if !descriptorIsVisible(desc, true /* allowAdding */, false /* includeDropped */) {
233322
continue
@@ -240,9 +329,55 @@ func (n *DropProvisionedRolesNode) userHasDependencies(
240329
return true, nil
241330
}
242331
}
332+
333+
// Check default privileges on databases and schemas.
334+
var defaultPrivs catalog.DefaultPrivilegeDescriptor
335+
if dbDesc, ok := desc.(catalog.DatabaseDescriptor); ok {
336+
defaultPrivs = dbDesc.GetDefaultPrivilegeDescriptor()
337+
} else if schemaDesc, ok := desc.(catalog.SchemaDescriptor); ok {
338+
defaultPrivs = schemaDesc.GetDefaultPrivilegeDescriptor()
339+
}
340+
if defaultPrivs != nil {
341+
hasDep := false
342+
if err := defaultPrivs.ForEachDefaultPrivilegeForRole(
343+
func(dpForRole catpb.DefaultPrivilegesForRole) error {
344+
if dpForRole.IsExplicitRole() &&
345+
dpForRole.GetExplicitRole().UserProto.Decode() == normalizedUsername {
346+
hasDep = true
347+
}
348+
for _, privs := range dpForRole.DefaultPrivilegesPerObject {
349+
for _, u := range privs.Users {
350+
if u.User() == normalizedUsername {
351+
hasDep = true
352+
}
353+
}
354+
}
355+
return nil
356+
},
357+
); err != nil {
358+
return false, err
359+
}
360+
if hasDep {
361+
return true, nil
362+
}
363+
}
364+
365+
// Check row-level security policies on tables.
366+
if tblDesc, ok := desc.(catalog.TableDescriptor); ok {
367+
for _, p := range tblDesc.GetPolicies() {
368+
for _, rn := range p.RoleNames {
369+
if username.MakeSQLUsernameFromPreNormalizedString(rn) == normalizedUsername {
370+
return true, nil
371+
}
372+
}
373+
}
374+
}
243375
}
244376

245-
// Check scheduled jobs.
377+
// Check scheduled jobs. Use NodeUserSessionDataOverride because
378+
// CREATEROLE users cannot read system.scheduled_jobs directly.
379+
// This is safe since the query is hardcoded with only a
380+
// parameterized username — no user-controlled SQL expressions.
246381
row, err := params.p.InternalSQLTxn().QueryRowEx(
247382
params.ctx,
248383
"check-user-schedules",
@@ -258,7 +393,8 @@ func (n *DropProvisionedRolesNode) userHasDependencies(
258393
return true, nil
259394
}
260395

261-
// Check system privileges.
396+
// Check system privileges. Same as above — use node privileges
397+
// for the hardcoded parameterized query.
262398
row, err = params.p.InternalSQLTxn().QueryRowEx(
263399
params.ctx,
264400
"check-user-system-privileges",
@@ -281,9 +417,9 @@ func (n *DropProvisionedRolesNode) userHasDependencies(
281417
// its web sessions.
282418
func (n *DropProvisionedRolesNode) deleteRole(
283419
params runParams, normalizedUsername username.SQLUsername, opName redact.RedactableString,
284-
) (dbRoleSettingsDeleted int, err error) {
420+
) (int, error) {
285421
// DELETE from system.users.
286-
if _, err = params.p.InternalSQLTxn().ExecEx(
422+
if _, err := params.p.InternalSQLTxn().ExecEx(
287423
params.ctx, opName, params.p.txn,
288424
sessiondata.NodeUserSessionDataOverride,
289425
`DELETE FROM system.users WHERE username=$1`,
@@ -293,7 +429,7 @@ func (n *DropProvisionedRolesNode) deleteRole(
293429
}
294430

295431
// DELETE from system.role_members.
296-
if _, err = params.p.InternalSQLTxn().ExecEx(
432+
if _, err := params.p.InternalSQLTxn().ExecEx(
297433
params.ctx, "drop-role-membership", params.p.txn,
298434
sessiondata.NodeUserSessionDataOverride,
299435
`DELETE FROM system.role_members WHERE "role" = $1 OR "member" = $1`,
@@ -303,7 +439,7 @@ func (n *DropProvisionedRolesNode) deleteRole(
303439
}
304440

305441
// DELETE from system.role_options.
306-
if _, err = params.p.InternalSQLTxn().ExecEx(
442+
if _, err := params.p.InternalSQLTxn().ExecEx(
307443
params.ctx, opName, params.p.txn,
308444
sessiondata.NodeUserSessionDataOverride,
309445
fmt.Sprintf(
@@ -330,7 +466,7 @@ func (n *DropProvisionedRolesNode) deleteRole(
330466
}
331467

332468
// Revoke web sessions.
333-
if _, err = params.p.InternalSQLTxn().ExecEx(
469+
if _, err := params.p.InternalSQLTxn().ExecEx(
334470
params.ctx, opName, params.p.txn,
335471
sessiondata.NodeUserSessionDataOverride,
336472
`UPDATE system.web_sessions SET "revokedAt" = now() WHERE username = $1 AND "revokedAt" IS NULL`,

0 commit comments

Comments
 (0)