changeset 180:0423eab4ad45

Improve RLS policies for template data The removed POLICY manage_templates missed a WITH CHECK (true), because the USING clause is applied to new rows, too, if no WITH CHECK is provided, thus implying a dead-lock situation with the FK constraint on user_templates (the POLICY requiring a row in user_templates while INSERTing such row requires a row in templates). New POLICY on user_templates prevents waterway_admin from relating templates to users from other countries and allows to write other policies more compact.
author Tom Gottfried <tom@intevation.de>
date Tue, 17 Jul 2018 19:08:18 +0200
parents 382f631d8dd8
children e509eccff303
files schema/auth.sql schema/tap_tests.sql
diffstat 2 files changed, 43 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/schema/auth.sql	Tue Jul 17 18:21:56 2018 +0200
+++ b/schema/auth.sql	Tue Jul 17 19:08:18 2018 +0200
@@ -34,6 +34,13 @@
 --
 -- RLS policies for waterway_user
 --
+-- Sometimes using FOR ALL because we rely on GRANTed privileges for allowing
+-- data modifications generally.
+-- Sometimes using 'username IN(SELECT username FROM user_profiles)' instead
+-- of 'username = current_user', because waterway_admin is intentionally
+-- allowed more with these policies (note that the subselect implies different
+-- policies on user_profiles depending on current_user).
+--
 
 -- Staging area
 CREATE FUNCTION create_hide_staging_policy() RETURNS void AS $$
@@ -56,9 +63,13 @@
        USING (username = current_user);
 ALTER TABLE user_profiles ENABLE ROW LEVEL SECURITY;
 
-CREATE POLICY own_templates ON templates FOR SELECT TO waterway_user
-       USING (template_name IN(SELECT template_name FROM user_templates
-                    WHERE username = current_user));
+CREATE POLICY user_templates ON user_templates FOR ALL TO waterway_user
+       USING (username IN(SELECT username FROM user_profiles));
+ALTER TABLE user_templates ENABLE ROW LEVEL SECURITY;
+
+CREATE POLICY user_templates ON templates FOR ALL TO waterway_user
+       USING (template_name IN(SELECT template_name FROM user_templates))
+       WITH CHECK (true);
 ALTER TABLE templates ENABLE ROW LEVEL SECURITY;
 
 --
@@ -86,9 +97,4 @@
 CREATE POLICY country_profiles ON user_profiles FOR SELECT TO waterway_admin
        USING (country = current_user_country());
 
-CREATE POLICY manage_templates ON templates FOR ALL TO waterway_admin
-       USING (template_name IN(SELECT template_name FROM user_templates ut
-                    JOIN user_profiles p ON ut.username = p.username
-                    WHERE p.country = current_user_country()));
-
 COMMIT;
--- a/schema/tap_tests.sql	Tue Jul 17 18:21:56 2018 +0200
+++ b/schema/tap_tests.sql	Tue Jul 17 19:08:18 2018 +0200
@@ -3,7 +3,7 @@
 --
 CREATE EXTENSION pgtap;
 
-SELECT plan(10); -- Give number of tests that have to be run
+SELECT plan(16); -- Give number of tests that have to be run
 
 SET search_path TO public, gemma, gemma_waterway, gemma_fairway;
 
@@ -68,7 +68,34 @@
 SELECT isnt_empty('SELECT * FROM templates
                    JOIN user_templates USING (template_name)
                    WHERE username <> current_user',
-                  'Waterway admin should see templates of users in country');
+                  'Waterway admin should see templates of other users');
+
+SELECT lives_ok('INSERT INTO templates (template_name, template_data)
+                 VALUES (''New AT'', ''\x'');
+                 INSERT INTO user_templates
+                 VALUES (''waterway_user'', ''New AT'')',
+                'Waterway admin can add templates for users in his country');
+
+SELECT throws_ok('INSERT INTO user_templates
+                  VALUES (''waterway_user2'', ''AT'')',
+                 42501, NULL,
+                 'Waterway admin cannot add template for other country');
+
+SELECT isnt_empty('UPDATE templates SET template_data = ''\xDABE''
+                   WHERE template_name = ''AT'' RETURNING *',
+                  'Waterway admin can alter templates for own country');
+
+SELECT is_empty('UPDATE templates SET template_data = ''\xDABE''
+                 WHERE template_name = ''RO'' RETURNING *',
+                'Waterway admin cannot alter templates for other country');
+
+SELECT isnt_empty('DELETE FROM templates WHERE template_name = ''AT''
+                   RETURNING *',
+                  'Waterway admin can delete templates for own country');
+
+SELECT is_empty('DELETE FROM templates WHERE template_name = ''RO''
+                 RETURNING *',
+                'Waterway admin cannot delete templates for other country');
 
 --
 -- finish tests