Added support for permissive/restricted policy type while creating RLS Policy. Fixes #5622

This commit is contained in:
Pradip Parkale 2020-07-07 17:55:05 +05:30 committed by Akshay Joshi
parent 776bec0d82
commit 36a2c073bd
26 changed files with 381 additions and 46 deletions

Binary file not shown.

Before

Width:  |  Height:  |  Size: 72 KiB

After

Width:  |  Height:  |  Size: 290 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 69 KiB

After

Width:  |  Height:  |  Size: 83 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 117 KiB

After

Width:  |  Height:  |  Size: 100 KiB

View File

@ -12,6 +12,7 @@ New features
| `Issue #5484 <https://redmine.postgresql.org/issues/5484>`_ - Added support for LDAP authentication with different DN by setting the dedicated user for the LDAP connection.
| `Issue #5583 <https://redmine.postgresql.org/issues/5583>`_ - Added support for schema level restriction.
| `Issue #5601 <https://redmine.postgresql.org/issues/5601>`_ - Added RLS Policy support in Schema Diff.
| `Issue #5622 <https://redmine.postgresql.org/issues/5622>`_ - Added support for permissive/restricted policy type while creating RLS Policy.
Housekeeping
************

View File

@ -20,6 +20,7 @@ Use the fields in the *General* tab to define the RLS Policy:
* Use the *Name* field to add a descriptive name for the RLS Policy. The name will be displayed in the *pgAdmin* tree control.
* Use the drop-down listbox next to *Role* to select the Role to which the RLS Policy is to be applied.
* Use the drop-down listbox next to *Type* to select the type of the policy.
Click the *Commands* tab to continue.
@ -31,7 +32,7 @@ Use the fields in the *Commands* tab to define the RLS Policy:
* Use the drop-down listbox next to *Event* to select the command to which policy applies. Valid options are ALL, SELECT, INSERT, UPDATE, and DELETE. Default is ALL.
* Use the *Using* field to add a SQL conditional expression returning boolean. This expression will be added to queries that refer to the table if row level security is enabled.
* Use the *With Check* field to add a SQL conditional expression returning boolean. This expression will be used in INSERT and UPDATE queries against the table if row level security is enabled.
* Use the *With check* field to add a SQL conditional expression returning boolean. This expression will be used in INSERT and UPDATE queries against the table if row level security is enabled.
Click the *SQL* tab to continue.

View File

@ -413,12 +413,10 @@ class RowSecurityView(PGChildNodeView):
request.data, encoding='utf-8'
)
try:
sql, name = row_security_policies_utils.get_sql(self.conn, data,
did, scid,
tid, plid,
self.datlastsysoid,
self.schema,
self.table)
sql, name = row_security_policies_utils.get_sql(
self.conn, data=data, scid=scid, plid=plid,
schema=self.schema, table=self.table)
# Most probably this is due to error
if not isinstance(sql, str):
return sql
@ -439,7 +437,7 @@ class RowSecurityView(PGChildNodeView):
return internal_server_error(errormsg=str(e))
@check_precondition
def delete(self, gid, sid, did, scid, tid, plid=None, only_sql=False):
def delete(self, gid, sid, did, scid, tid, **kwargs):
"""
This function will drop the policy object
:param plid: policy id
@ -448,8 +446,12 @@ class RowSecurityView(PGChildNodeView):
:param gid: group id
:param tid: table id
:param scid: Schema ID
:param kwargs
:return:
"""
plid = kwargs.get('plid', None)
only_sql = kwargs.get('only_sql', False)
# Below will deplide if it's simple drop or drop with cascade call
if self.cmd == 'delete':
# This is a cascade operation
@ -517,11 +519,9 @@ class RowSecurityView(PGChildNodeView):
"""
data = dict(request.args)
sql, name = row_security_policies_utils.get_sql(self.conn, data, did,
scid, tid, plid,
self.datlastsysoid,
self.schema,
self.table)
sql, name = row_security_policies_utils.get_sql(
self.conn, data=data, scid=scid, plid=plid,
schema=self.schema, table=self.table)
if not isinstance(sql, str):
return sql
sql = sql.strip('\n').strip(' ')
@ -548,8 +548,8 @@ class RowSecurityView(PGChildNodeView):
"""
SQL = row_security_policies_utils.get_reverse_engineered_sql(
self.conn, self.schema, self.table, did, scid, tid, plid,
self.datlastsysoid)
self.conn, schema=self.schema, table=self.table, scid=scid,
plid=plid, datlastsysoid=self.datlastsysoid)
return ajax_response(response=SQL)
@ -600,12 +600,11 @@ class RowSecurityView(PGChildNodeView):
:param kwargs
:return:
"""
gid = kwargs.get('gid')
sid = kwargs.get('sid')
did = kwargs.get('did')
scid = kwargs.get('scid')
tid = kwargs.get('tid')
oid = kwargs.get('oid')
oid = kwargs.get('plid')
data = kwargs.get('data', None)
diff_schema = kwargs.get('diff_schema', None)
drop_req = kwargs.get('drop_req', False)
@ -615,8 +614,8 @@ class RowSecurityView(PGChildNodeView):
data['schema'] = self.schema
data['table'] = self.table
sql, name = row_security_policies_utils.get_sql(
self.conn, data, did, scid, tid, oid, self.datlastsysoid,
self.schema, self.table)
self.conn, data=data, scid=scid, plid=oid,
schema=self.schema, table=self.table)
sql = sql.strip('\n').strip(' ')
@ -624,10 +623,8 @@ class RowSecurityView(PGChildNodeView):
schema = diff_schema
sql = row_security_policies_utils.get_reverse_engineered_sql(
self.conn, schema,
self.table, did, scid, tid, oid,
self.datlastsysoid,
template_path=None, with_header=False)
self.conn, schema=schema, table=self.table, scid=scid,
plid=oid, datlastsysoid=self.datlastsysoid, with_header=False)
drop_sql = ''
if drop_req:
@ -711,7 +708,7 @@ class RowSecurityView(PGChildNodeView):
diff_dict = directory_diff(
source, target, difference={}
)
if 'event' in diff_dict:
if 'event' in diff_dict or 'type' in diff_dict:
delete_sql = self.get_sql_from_diff(gid=1,
sid=tgt_params['sid'],
did=tgt_params['did'],

View File

@ -84,6 +84,7 @@ define('pgadmin.node.row_security_policy', [
using_orig: undefined,
withcheck: undefined,
withcheck_orig: undefined,
type:'PERMISSIVE',
},
schema: [{
id: 'name', label: gettext('Name'), cell: 'string',
@ -148,7 +149,29 @@ define('pgadmin.node.row_security_policy', [
});
return res;
},
}],
},
{
id: 'type', label: gettext('Type'), control: 'select2', deps:['type'],
type: 'text',readonly: function(m) {
return !m.isNew();},
select2: {
width: '100%',
allowClear: false,
},
options:[
{label: 'PERMISSIVE', value: 'PERMISSIVE'},
{label: 'RESTRICTIVE', value: 'RESTRICTIVE'},
],
visible: function(m) {
if(!_.isUndefined(m.node_info) && !_.isUndefined(m.node_info.server)
&& !_.isUndefined(m.node_info.server.version) &&
m.node_info.server.version >= 100000)
return true;
return false;
},
},
],
validate: function(keys) {
var msg;
this.errorModel.clear();

View File

@ -0,0 +1,9 @@
-- POLICY: policy_1
-- DROP POLICY policy_1 ON public.test_rls_policy;
CREATE POLICY policy_1
ON public.test_rls_policy
AS PERMISSIVE
FOR ALL
TO public;

View File

@ -0,0 +1,2 @@
ALTER POLICY test ON public.test_rls_policy
RENAME TO policy_1;

View File

@ -0,0 +1,11 @@
-- POLICY: all_event_policy
-- DROP POLICY 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,9 @@
-- POLICY: insert_policy
-- DROP POLICY insert_policy ON public.test_rls_policy;
CREATE POLICY insert_policy
ON public.test_rls_policy
AS PERMISSIVE
FOR INSERT
TO public;

View File

@ -0,0 +1,9 @@
-- POLICY: test
-- DROP POLICY test ON public.test_rls_policy;
CREATE POLICY test
ON public.test_rls_policy
AS PERMISSIVE
FOR ALL
TO public;

View File

@ -0,0 +1,9 @@
-- POLICY: select_policy
-- DROP POLICY select_policy ON public.test_rls_policy;
CREATE POLICY select_policy
ON public.test_rls_policy
AS PERMISSIVE
FOR SELECT
TO public;

View File

@ -0,0 +1,105 @@
{
"scenarios": [
{
"type": "create",
"name": "Create Table For RLS policy",
"endpoint": "NODE-table.obj",
"sql_endpoint": "NODE-table.sql_id",
"data": {
"name": "test_rls_policy",
"columns": [
{
"name": "emp_id",
"cltype": "integer",
"is_primary_key": true
},
{
"name": "name",
"cltype": "text"
},
{
"name": "salary",
"cltype": "bigint"
}
],
"is_partitioned": false,
"schema": "public",
"spcname": "pg_default"
},
"store_object_id": true
},
{
"type": "create",
"name": "Create select RLS policy",
"endpoint": "NODE-row_security_policy.obj",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"data": {
"name": "select_policy",
"event": "SELECT",
"policyowner": "public",
"schema": "public"
},
"expected_sql_file": "create_select_policy.sql"
},
{
"type": "create",
"name": "Create INSERT RLS policy",
"endpoint": "NODE-row_security_policy.obj",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"data": {
"name": "insert_policy",
"event": "INSERT",
"policyowner": "public",
"schema": "public"
},
"expected_sql_file": "create_insert_policy.sql"
},
{
"type": "create",
"name": "Create RLS policy",
"endpoint": "NODE-row_security_policy.obj",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"data": {
"name": "test",
"schema": "public"
},
"expected_sql_file": "create_public_policy.sql"
},
{
"type": "alter",
"name": "Alter policy name",
"endpoint": "NODE-row_security_policy.obj_id",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"msql_endpoint": "NODE-row_security_policy.msql_id",
"data": {
"name": "policy_1"
},
"expected_sql_file": "alter_policy.sql",
"expected_msql_file": "alter_policy_msql.sql"
},
{
"type": "create",
"name": "Create RLS policy for event 'ALL'",
"endpoint": "NODE-row_security_policy.obj",
"sql_endpoint": "NODE-row_security_policy.sql_id",
"data": {
"name": "all_event_policy",
"event": "ALL",
"policyowner": "public",
"schema": "public",
"using": "true",
"withcheck": "true",
"type":"RESTRICTIVE"
},
"expected_sql_file": "create_all_event_policy.sql"
},
{
"type": "delete",
"name": "Drop policy",
"endpoint": "NODE-row_security_policy.delete_id",
"data": {
"name": "test_delete_policy_$%{}[]()&*^!@\"'`\\/#"
}
}
]
}

View File

@ -88,7 +88,8 @@
"policyowner": "public",
"schema": "public",
"using": "true",
"withcheck": "true"
"withcheck": "true",
"type":"RESTRICTIVE"
},
"expected_sql_file": "create_all_event_policy.sql"
},

View File

@ -61,11 +61,16 @@ def get_parent(conn, tid, template_path=None):
@get_template_path
def get_sql(conn, data, did, scid, tid, plid, datlastsysoid, schema, table,
template_path=None):
def get_sql(conn, **kwargs):
"""
This function will generate sql from model data
"""
data = kwargs.get('data')
scid = kwargs.get('scid')
plid = kwargs.get('plid')
schema = kwargs.get('schema')
table = kwargs.get('table')
template_path = kwargs.get('template_path', None)
if plid is not None:
sql = render_template("/".join([template_path, 'properties.sql']),
@ -94,24 +99,21 @@ def get_sql(conn, data, did, scid, tid, plid, datlastsysoid, schema, table,
@get_template_path
def get_reverse_engineered_sql(conn, schema, table, did, scid, tid, plid,
datlastsysoid,
template_path=None, with_header=True):
def get_reverse_engineered_sql(conn, **kwargs):
"""
This function will return reverse engineered sql for specified trigger.
:param conn: Connection Object
:param schema: Schema
:param table: Table
:param did: DB ID
:param tid: Table ID
:param plid: Policy ID
:param datlastsysoid:
:param template_path: Optional template path
:param with_header: Optional parameter to decide whether the SQL will be
returned with header or not
:param conn:
:param kwargs:
:return:
"""
schema = kwargs.get('schema')
table = kwargs.get('table')
scid = kwargs.get('scid')
plid = kwargs.get('plid')
datlastsysoid = kwargs.get('datlastsysoid')
template_path = kwargs.get('template_path', None)
with_header = kwargs.get('with_header', True)
SQL = render_template("/".join(
[template_path, 'properties.sql']), plid=plid, scid=scid)
@ -127,10 +129,9 @@ def get_reverse_engineered_sql(conn, schema, table, did, scid, tid, plid,
data['schema'] = schema
data['table'] = table
SQL, name = get_sql(conn, data, did, scid, tid, None, datlastsysoid,
schema,
table)
SQL, name = get_sql(conn, data=data, scid=scid, plid=None,
datlastsysoid=datlastsysoid, schema=schema,
table=table)
if with_header:
sql_header = u"-- POLICY: {0}\n\n-- ".format(data['name'])

View File

@ -0,0 +1,27 @@
{% set add_semicolon_after = 'to' %}
{% if data.withcheck is defined and data.withcheck != None and data.withcheck != '' %}
{% set add_semicolon_after = 'with_check' %}
{% elif data.using is defined and data.using != None and data.using != '' %}
{% set add_semicolon_after = 'using' %}
{% endif %}
CREATE POLICY {{ conn|qtIdent(data.name) }}
ON {{conn|qtIdent(data.schema, data.table)}}
{%if data.type %}
AS {{ data.type|upper }}
{% endif %}
{% if data.event %}
FOR {{ data.event|upper }}
{% endif %}
{% if data.policyowner %}
TO {{ conn|qtTypeIdent(data.policyowner) }}{% if add_semicolon_after == 'to' %};{% endif %}
{% else %}
TO public{% if add_semicolon_after == 'to' %};{% endif %}
{% endif %}
{% if data.using %}
USING ({{ data.using }}){% if add_semicolon_after == 'using' %};{% endif %}
{% endif %}
{% if data.withcheck %}
WITH CHECK ({{ data.withcheck }});
{% endif %}

View File

@ -0,0 +1,22 @@
SELECT
pl.oid AS oid,
pl.polname AS name,
rw.permissive as type,
rw.cmd AS event,
rw.qual AS using,
rw.qual AS using_orig,
rw.with_check AS withcheck,
rw.with_check AS withcheck_orig,
array_to_string(rw.roles::name[], ', ') AS policyowner
FROM
pg_policy pl
JOIN pg_policies rw ON pl.polname=rw.policyname
JOIN pg_namespace n ON n.nspname=rw.schemaname
WHERE
{% if plid %}
pl.oid = {{ plid }} and n.oid = {{ scid }};
{% endif %}
{% if tid %}
pl.polrelid = {{ tid }};
{% endif %}

View File

@ -0,0 +1,24 @@
{% set add_semicolon_after = 'to' %}
{% if data.withcheck is defined and data.withcheck != None and data.withcheck != '' %}
{% set add_semicolon_after = 'with_check' %}
{% elif data.using is defined and data.using != None and data.using != '' %}
{% set add_semicolon_after = 'using' %}
{% endif %}
CREATE POLICY {{ conn|qtIdent(data.name) }}
ON {{conn|qtIdent(data.schema, data.table)}}
{% if data.event %}
FOR {{ data.event|upper }}
{% endif %}
{% if data.policyowner %}
TO {{ conn|qtTypeIdent(data.policyowner) }}{% if add_semicolon_after == 'to' %};{% endif %}
{% else %}
TO public{% if add_semicolon_after == 'to' %};{% endif %}
{% endif %}
{% if data.using %}
USING ({{ data.using }}){% if add_semicolon_after == 'using' %};{% endif %}
{% endif %}
{% if data.withcheck %}
WITH CHECK ({{ data.withcheck }});
{% endif %}

View File

@ -0,0 +1 @@
DROP POLICY {{ conn|qtIdent(policy_name) }} ON {{conn|qtIdent(result.schema, result.table)}};

View File

@ -0,0 +1,5 @@
SELECT nsp.nspname AS schema ,rel.relname AS table
FROM pg_class rel
JOIN pg_namespace nsp
ON rel.relnamespace = nsp.oid::oid
WHERE rel.oid = {{tid}}::oid

View File

@ -0,0 +1,9 @@
{% if plid %}
SELECT
pl.oid AS oid,
pl.polname AS name
FROM
pg_policy pl
WHERE
pl.oid = {{ plid }}
{% endif %}

View File

@ -0,0 +1,2 @@
SELECT pl.oid FROM pg_policy pl
WHERE pl.polrelid = {{tid}}::oid AND pl.polname = {{data.name|qtLiteral}};

View File

@ -0,0 +1,13 @@
SELECT
pl.oid AS oid,
pl.polname AS name
FROM
pg_policy pl
WHERE
{% if tid %}
pl.polrelid = {{ tid }}
{% elif plid %}
pl.oid = {{ plid }}
{% endif %}
ORDER BY
pl.polname;

View File

@ -0,0 +1,21 @@
SELECT
pl.oid AS oid,
pl.polname AS name,
rw.cmd AS event,
rw.qual AS using,
rw.qual AS using_orig,
rw.with_check AS withcheck,
rw.with_check AS withcheck_orig,
array_to_string(rw.roles::name[], ', ') AS policyowner
FROM
pg_policy pl
JOIN pg_policies rw ON pl.polname=rw.policyname
JOIN pg_namespace n ON n.nspname=rw.schemaname
WHERE
{% if plid %}
pl.oid = {{ plid }} and n.oid = {{ scid }};
{% endif %}
{% if tid %}
pl.polrelid = {{ tid }};
{% endif %}

View File

@ -0,0 +1,33 @@
{#####################################################}
{## Change policy owner ##}
{#####################################################}
{% if data.policyowner and o_data.policyowner != data.policyowner %}
ALTER POLICY {{ o_data.name }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
TO {{ conn|qtTypeIdent(data.policyowner) }};
{% endif %}
{#####################################################}
{## Change policy using condition ##}
{#####################################################}
{% if data.using and o_data.using != data.using %}
ALTER POLICY {{ 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 {{ 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 {{ o_data.name }} ON {{conn|qtIdent(o_data.schema, o_data.table)}}
RENAME TO {{ conn|qtIdent(data.name) }};
{% endif %}