Fixed an issue that prevented assigning multiple users to an RLS policy. #9304

pull/9296/head
Akshay Joshi 2025-10-30 18:54:20 +05:30
parent 282a956f4f
commit b62212b8e5
15 changed files with 206 additions and 26 deletions

View File

@ -20,7 +20,9 @@ Bundled PostgreSQL Utilities
New features
************
| `Issue #6698 <https://github.com/pgadmin-org/pgadmin4/issues/6698>`_ - Add support for setting image download resolution in the ERD tool.
| `Issue #7885 <https://github.com/pgadmin-org/pgadmin4/issues/7885>`_ - Add support for displaying detailed Citus query plans instead of 'Custom Scan' placeholder.
| `Issue #8912 <https://github.com/pgadmin-org/pgadmin4/issues/8912>`_ - Add support for formatting .pgerd ERD project file.
Housekeeping
************
@ -31,4 +33,5 @@ Bug fixes
*********
| `Issue #8504 <https://github.com/pgadmin-org/pgadmin4/issues/8504>`_ - Fixed an issue where data output column resize is not sticking in Safari.
| `Issue #9117 <https://github.com/pgadmin-org/pgadmin4/issues/9117>`_ - Fixed an issue where Schema Diff does not ignore Tablespace for indexes.
| `Issue #9117 <https://github.com/pgadmin-org/pgadmin4/issues/9117>`_ - Fixed an issue where Schema Diff does not ignore Tablespace for indexes.
| `Issue #9304 <https://github.com/pgadmin-org/pgadmin4/issues/9304>`_ - Fixed an issue that prevented assigning multiple users to an RLS policy.

View File

@ -541,6 +541,8 @@ class RowSecurityView(PGChildNodeView):
This function returns modified sql
"""
data = dict(request.args)
if 'policyowner' in data and isinstance(data['policyowner'], str):
data['policyowner'] = json.loads(data['policyowner'])
sql, _ = row_security_policies_utils.get_sql(
self.conn, data=data, scid=scid, plid=plid, policy_table_id=tid,

View File

@ -15,7 +15,7 @@ export default class RowSecurityPolicySchema extends BaseUISchema {
constructor(fieldOptions={}, initValues={}) {
super({
name: undefined,
policyowner: 'public',
policyowner: ['public'],
event: 'ALL',
using: undefined,
using_orig: undefined,
@ -106,7 +106,7 @@ export default class RowSecurityPolicySchema extends BaseUISchema {
id: 'policyowner', label: gettext('Role'), cell: 'text',
type: 'select',
options: obj.fieldOptions.role,
controlProps: { allowClear: false }
controlProps: { multiple: true }
},
{
id: 'type', label: gettext('Type'), type: 'select', deps:['type'],

View File

@ -28,6 +28,48 @@
},
"store_object_id": true
},
{
"type": "create",
"name": "Create Role to test multiple role for RLS policy",
"endpoint": "NODE-role.obj",
"sql_endpoint": "NODE-role.sql_id",
"data": {
"rolname": "Role_1",
"rolcanlogin": false,
"rolpassword": null,
"rolconnlimit": -1,
"rolsuper": false,
"rolcreaterole": false,
"rolcreatedb": false,
"rolinherit": true,
"rolreplication": false,
"rolmembership": [],
"seclabels": [],
"variables": []
},
"store_object_id": true
},
{
"type": "create",
"name": "Create Role to test multiple role for RLS policy",
"endpoint": "NODE-role.obj",
"sql_endpoint": "NODE-role.sql_id",
"data": {
"rolname": "role_2",
"rolcanlogin": false,
"rolpassword": null,
"rolconnlimit": -1,
"rolsuper": false,
"rolcreaterole": false,
"rolcreatedb": false,
"rolinherit": true,
"rolreplication": false,
"rolmembership": [],
"seclabels": [],
"variables": []
},
"store_object_id": true
},
{
"type": "create",
"name": "Create simple select event RLS policy",
@ -37,7 +79,7 @@
"data": {
"name": "test_select_policy_rls_$%{}[]()&*^!@\"'`\\/#",
"event": "SELECT",
"policyowner": "public",
"policyowner": ["public"],
"schema": "public",
"type": "PERMISSIVE",
"description": "This is test description"
@ -75,7 +117,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_simple_insert_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "INSERT",
"withcheck": "",
"type": "PERMISSIVE",
@ -114,7 +156,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_update_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "UPDATE",
"using": "true",
"withcheck": "name != null",
@ -154,7 +196,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_delete_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "DELETE",
"using": "current_user = name",
"withcheck": "",
@ -194,7 +236,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_all_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "ALL",
"type": "RESTRICTIVE",
"schema": "public"
@ -256,7 +298,7 @@
"data": {
"name": "all_event_policy",
"event": "ALL",
"policyowner": "public",
"policyowner": ["Role_1", "role_2"],
"schema": "public",
"using": "true",
"withcheck": "true",
@ -264,6 +306,30 @@
},
"expected_sql_file": "create_all_event_policy.sql"
},
{
"type": "alter",
"name": "Alter policy owner",
"endpoint": "NODE-row_security_policy.obj_id",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"msql_endpoint": "NODE-row_security_policy.msql_id",
"data": {
"policyowner": ["role_2"]
},
"expected_sql_file": "alter_owner_policy.sql",
"expected_msql_file": "alter_owner_policy_msql.sql"
},
{
"type": "alter",
"name": "Alter policy owner remove all users",
"endpoint": "NODE-row_security_policy.obj_id",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"msql_endpoint": "NODE-row_security_policy.msql_id",
"data": {
"policyowner": []
},
"expected_sql_file": "alter_remove_all_owner_policy.sql",
"expected_msql_file": "alter_remove_all_owner_policy_msql.sql"
},
{
"type": "delete",
"name": "Drop policy",
@ -271,6 +337,13 @@
"data": {
"name": "test_delete_policy_$%{}[]()&*^!@\"'`\\/#"
}
},
{
"type": "delete",
"name": "Drop Role",
"endpoint": "NODE-role.obj",
"data": {"ids": ["<Role_1>", "<role_2>"]},
"preprocess_data": true
}
]
}

View File

@ -0,0 +1,11 @@
-- POLICY: all_event_policy
-- DROP POLICY IF EXISTS all_event_policy ON public.test_rls_policy;
CREATE POLICY all_event_policy
ON public.test_rls_policy
AS RESTRICTIVE
FOR ALL
TO role_2
USING (true)
WITH CHECK (true);

View File

@ -0,0 +1,2 @@
ALTER POLICY all_event_policy ON public.test_rls_policy
TO role_2;

View File

@ -0,0 +1,11 @@
-- POLICY: all_event_policy
-- DROP POLICY IF EXISTS all_event_policy ON public.test_rls_policy;
CREATE POLICY all_event_policy
ON public.test_rls_policy
AS RESTRICTIVE
FOR ALL
TO public
USING (true)
WITH CHECK (true);

View File

@ -0,0 +1,2 @@
ALTER POLICY all_event_policy ON public.test_rls_policy
TO PUBLIC;

View File

@ -6,6 +6,6 @@ CREATE POLICY all_event_policy
ON public.test_rls_policy
AS RESTRICTIVE
FOR ALL
TO public
TO "Role_1", role_2
USING (true)
WITH CHECK (true);

View File

@ -28,6 +28,48 @@
},
"store_object_id": true
},
{
"type": "create",
"name": "Create Role to test multiple role for RLS policy",
"endpoint": "NODE-role.obj",
"sql_endpoint": "NODE-role.sql_id",
"data": {
"rolname": "Role_1",
"rolcanlogin": false,
"rolpassword": null,
"rolconnlimit": -1,
"rolsuper": false,
"rolcreaterole": false,
"rolcreatedb": false,
"rolinherit": true,
"rolreplication": false,
"rolmembership": [],
"seclabels": [],
"variables": []
},
"store_object_id": true
},
{
"type": "create",
"name": "Create Role to test multiple role for RLS policy",
"endpoint": "NODE-role.obj",
"sql_endpoint": "NODE-role.sql_id",
"data": {
"rolname": "role_2",
"rolcanlogin": false,
"rolpassword": null,
"rolconnlimit": -1,
"rolsuper": false,
"rolcreaterole": false,
"rolcreatedb": false,
"rolinherit": true,
"rolreplication": false,
"rolmembership": [],
"seclabels": [],
"variables": []
},
"store_object_id": true
},
{
"type": "create",
"name": "Create simple select event RLS policy",
@ -37,7 +79,7 @@
"data": {
"name": "test_select_policy_rls_$%{}[]()&*^!@\"'`\\/#",
"event": "SELECT",
"policyowner": "public",
"policyowner": ["public"],
"schema": "public",
"type": "PERMISSIVE",
"description": "This is test description"
@ -75,7 +117,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_simple_insert_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "INSERT",
"withcheck": "",
"type": "PERMISSIVE",
@ -114,7 +156,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_update_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "UPDATE",
"using": "true",
"withcheck": "name != null",
@ -154,7 +196,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_delete_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "DELETE",
"using": "current_user = name",
"withcheck": "",
@ -194,7 +236,7 @@
"msql_endpoint": "NODE-row_security_policy.msql",
"data": {
"name": "test_all_rls_policy_$%{}[]()&*^!@\"'`\\/#",
"policyowner": "public",
"policyowner": ["public"],
"event": "ALL",
"type": "RESTRICTIVE",
"schema": "public"
@ -256,7 +298,7 @@
"data": {
"name": "all_event_policy",
"event": "ALL",
"policyowner": "public",
"policyowner": ["Role_1", "role_2"],
"schema": "public",
"using": "true",
"withcheck": "true",
@ -264,6 +306,30 @@
},
"expected_sql_file": "create_all_event_policy.sql"
},
{
"type": "alter",
"name": "Alter policy owner",
"endpoint": "NODE-row_security_policy.obj_id",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"msql_endpoint": "NODE-row_security_policy.msql_id",
"data": {
"policyowner": ["role_2"]
},
"expected_sql_file": "alter_owner_policy.sql",
"expected_msql_file": "alter_owner_policy_msql.sql"
},
{
"type": "alter",
"name": "Alter policy owner remove all users",
"endpoint": "NODE-row_security_policy.obj_id",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"msql_endpoint": "NODE-row_security_policy.msql_id",
"data": {
"policyowner": []
},
"expected_sql_file": "alter_remove_all_owner_policy.sql",
"expected_msql_file": "alter_remove_all_owner_policy_msql.sql"
},
{
"type": "delete",
"name": "Drop policy",
@ -271,6 +337,13 @@
"data": {
"name": "test_delete_policy_$%{}[]()&*^!@\"'`\\/#"
}
},
{
"type": "delete",
"name": "Drop Role",
"endpoint": "NODE-role.obj",
"data": {"ids": ["<Role_1>", "<role_2>"]},
"preprocess_data": true
}
]
}

View File

@ -65,7 +65,7 @@ class RulesAddTestCase(BaseTestGenerator):
"test_comment_add_%s" % (str(uuid.uuid4())[1:8])
if hasattr(self, "owner_policy"):
self.test_data['policyowner'] = self.role_name
self.test_data['policyowner'] = [self.role_name]
data = self.test_data
if self.is_positive_test:

View File

@ -80,7 +80,7 @@ class PolicyUpdateTestCase(BaseTestGenerator):
self.test_data['id'] = self.policy_id
if hasattr(self, 'owner_policy'):
self.test_data['policyowner'] = self.role_name
self.test_data['policyowner'] = [self.role_name]
if not policy_name:
raise Exception("Could not find the policy to update.")

View File

@ -13,7 +13,7 @@ CREATE POLICY {{ conn|qtIdent(data.name) }}
FOR {{ data.event|upper }}
{% endif %}
{% if data.policyowner %}
TO {{ conn|qtTypeIdent(data.policyowner) }}{% if add_semicolon_after == 'to' %};{% endif %}
TO {{ conn|qtIdent(data.policyowner)|join(', ') }}{% if add_semicolon_after == 'to' %};{% endif %}
{% else %}
TO public{% if add_semicolon_after == 'to' %};{% endif %}
{% endif %}
@ -31,4 +31,4 @@ CREATE POLICY {{ conn|qtIdent(data.name) }}
COMMENT ON POLICY {{ conn|qtIdent(data.name) }}
ON {{ conn|qtIdent(data.schema, data.table) }}
IS {{ data.description|qtLiteral(conn) }};
{% endif %}
{% endif %}

View File

@ -7,7 +7,7 @@ SELECT
rw.qual AS using_orig,
rw.with_check AS withcheck,
rw.with_check AS withcheck_orig,
pg_catalog.array_to_string(rw.roles::name[], ', ') AS policyowner,
rw.roles AS policyowner,
des.description
FROM
pg_catalog.pg_policy pl

View File

@ -3,37 +3,40 @@
{#####################################################}
{% if data.policyowner and o_data.policyowner != data.policyowner %}
ALTER POLICY {{ conn|qtIdent(o_data.name) }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
TO {{ conn|qtTypeIdent(data.policyowner) }};
TO {{ conn|qtIdent(data.policyowner)|join(', ') }};
{% elif data.policyowner is defined and data.policyowner|length == 0 %}
ALTER POLICY {{ conn|qtIdent(o_data.name) }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
TO PUBLIC;
{% endif %}
{#####################################################}
{## Change policy using condition ##}
{#####################################################}
{% if data.using and o_data.using != data.using %}
ALTER POLICY {{ conn|qtIdent(o_data.name) }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
USING ({{ data.using }});
{% endif %}
{#####################################################}
{## Change policy with check condition ##}
{#####################################################}
{% if data.withcheck and o_data.withcheck != data.withcheck %}
ALTER POLICY {{ conn|qtIdent(o_data.name) }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
WITH CHECK ({{ data.withcheck }});
{% endif %}
{#####################################################}
{## Change policy name ##}
{#####################################################}
{% if data.name and o_data.name != data.name %}
ALTER POLICY {{ conn|qtIdent(o_data.name) }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
RENAME TO {{ conn|qtIdent(data.name) }};
{% endif %}
{#####################################################}
{## Change policy comment ##}
{#####################################################}
{% if data.description is defined and data.description != o_data.description %}
COMMENT ON POLICY {{ conn|qtIdent(o_data.name) }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
IS {{ data.description|qtLiteral(conn) }};
{% endif %}