Skip to content

Commit 474d6c2

Browse files
committed
sql/schemachanger: require table ownership for RLS DDL operations
Previously, executing row-level security (RLS) DDL statements (e.g., CREATE POLICY, DROP POLICY) required only the CREATE privilege. This change updates the requirement so that only the table owner can perform these operations, aligning with postgres' behaviour. Closes cockroachdb#143080 Epic: CRDB-45203 Release note: none
1 parent 8a3a6f6 commit 474d6c2

File tree

3 files changed

+89
-1
lines changed

3 files changed

+89
-1
lines changed

pkg/sql/logictest/testdata/logic_test/row_level_security

+81
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,9 @@ GRANT ALL ON db1.* to testuser;
323323
statement ok
324324
GRANT ALL ON db1.* to john;
325325

326+
statement ok
327+
ALTER TABLE target OWNER TO john;
328+
326329
statement ok
327330
GRANT SYSTEM MODIFYCLUSTERSETTING TO testuser;
328331

@@ -1236,6 +1239,78 @@ DROP FUNCTION my_non_sec_definer_reader_function;
12361239
statement ok
12371240
DROP TABLE sensitive_data_table CASCADE;
12381241

1242+
# Verify that you need to be the table owner to do any of the RLS DDLs
1243+
subtest table_owner_and_rls_ddl
1244+
1245+
statement ok
1246+
CREATE USER tab_owner;
1247+
1248+
statement ok
1249+
CREATE USER nontab_owner;
1250+
1251+
statement ok
1252+
CREATE TABLE table_owner_test ();
1253+
1254+
statement ok
1255+
ALTER TABLE table_owner_test OWNER TO tab_owner;
1256+
1257+
statement ok
1258+
GRANT ALL ON table_owner_test TO nontab_owner;
1259+
1260+
statement ok
1261+
SET ROLE tab_owner;
1262+
1263+
statement ok
1264+
ALTER TABLE table_owner_test ENABLE ROW LEVEL SECURITY, FORCE ROW LEVEL SECURITY;
1265+
1266+
statement ok
1267+
CREATE POLICY p1 on table_owner_test;
1268+
1269+
statement ok
1270+
DROP POLICY p1 on table_owner_test;
1271+
1272+
statement ok
1273+
CREATE POLICY new_p1 on table_owner_test;
1274+
1275+
statement error pq: unimplemented: ALTER POLICY is not yet implemented
1276+
ALTER POLICY new_p1 on table_owner_test RENAME TO p1;
1277+
1278+
statement error pq: unimplemented: ALTER POLICY is not yet implemented
1279+
ALTER POLICY p1 on table_owner_test RENAME TO new_p1;
1280+
1281+
statement error pq: unimplemented: ALTER POLICY is not yet implemented
1282+
ALTER POLICY p1 on table_owner_test USING (true);
1283+
1284+
statement ok
1285+
SET ROLE nontab_owner;
1286+
1287+
statement error pq: must be owner of relation table_owner_test
1288+
ALTER TABLE table_owner_test DISABLE ROW LEVEL SECURITY;
1289+
1290+
statement error pq: must be owner of relation table_owner_test
1291+
ALTER TABLE table_owner_test NO FORCE ROW LEVEL SECURITY;
1292+
1293+
statement error pq: must be owner of relation table_owner_test
1294+
CREATE POLICY p2 on table_owner_test;
1295+
1296+
statement error pq: must be owner of relation table_owner_test
1297+
DROP POLICY new_p1 on table_owner_test;
1298+
1299+
statement error pq: unimplemented: ALTER POLICY is not yet implemented
1300+
ALTER POLICY new_p1 on table_owner_test WITH CHECK (true);
1301+
1302+
statement error pq: unimplemented: ALTER POLICY is not yet implemented
1303+
ALTER POLICY new_p1 on table_owner_test RENAME TO p1;
1304+
1305+
statement ok
1306+
SET ROLE root
1307+
1308+
statement ok
1309+
DROP TABLE table_owner_test;
1310+
1311+
statement ok
1312+
DROP ROLE nontab_owner, tab_owner;
1313+
12391314
subtest force
12401315

12411316
statement ok
@@ -1366,10 +1441,16 @@ SELECT c1, c2 FROM force_check WHERE c1 > 0 ORDER BY c1;
13661441
----
13671442
50 fifty
13681443

1444+
statement ok
1445+
SET ROLE root;
1446+
13691447
# Turn on force again, but it shouldn't matter because we aren't the owner anymore
13701448
statement ok
13711449
ALTER TABLE force_check FORCE ROW LEVEL SECURITY;
13721450

1451+
statement ok
1452+
SET ROLE forcer;
1453+
13731454
# q2 - should not reuse because table version change
13741455
query IT
13751456
SELECT c1, c2 FROM force_check WHERE c1 > 0 ORDER BY c1;

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_set_rls_mode.go

+7
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ func alterTableSetRLSMode(
1919
) {
2020
failIfRLSIsNotEnabled(b)
2121

22+
// The table is already known to exist, and we would have checked for
23+
// the CREATE privilege. However, changing the RLS mode is different,
24+
// as it can only be done by the table owner.
25+
_ = b.ResolveTable(tn.ToUnresolvedObjectName(), ResolveParams{
26+
RequireOwnership: true,
27+
})
28+
2229
switch n.Mode {
2330
case tree.TableRLSEnable:
2431
b.Add(&scpb.RowLevelSecurityEnabled{

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_policy.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func CreatePolicy(b BuildCtx, n *tree.CreatePolicy) {
3131
b.IncrementSchemaChangeCreateCounter("policy")
3232

3333
tableElems := b.ResolveTable(n.TableName, ResolveParams{
34-
RequiredPrivilege: privilege.CREATE,
34+
RequireOwnership: true,
3535
})
3636
panicIfSchemaChangeIsDisallowed(tableElems, n)
3737
tbl := tableElems.FilterTable().MustGetOneElement()

0 commit comments

Comments
 (0)