Mercurial > gemma
changeset 5025:4c658a8f34da
Fix row level security policies for waterway admin
Since 'staging_done OR' was added to the conditions to improve
performance for read access, it was also allowed to delete and partly
update entries with staging_done set to true but otherwise being
outside the country of the respective waterway admin. Using an extra
policy for each command and using the 'staging_done OR' tweak only
FOR SELECT should fix authorization while keeping performance.
| author | Tom Gottfried <tom@intevation.de> |
|---|---|
| date | Wed, 18 Mar 2020 12:16:42 +0100 |
| parents | 36a3dce20232 |
| children | d02f0c5c60b3 |
| files | schema/auth.sql schema/auth_tests.sql schema/run_tests.sh schema/tap_tests_data.sql schema/updates/1427/01.fix_rls_policies.sql |
| diffstat | 5 files changed, 216 insertions(+), 52 deletions(-) [+] |
line wrap: on
line diff
--- a/schema/auth.sql Mon Mar 16 17:11:53 2020 +0100 +++ b/schema/auth.sql Wed Mar 18 12:16:42 2020 +0100 @@ -134,58 +134,87 @@ -- Staging area --- In many cases it is more efficient to check for "staging_done" to --- prevent the more expensive checks for read only access (which is --- allowed for all users, when staging is done). -CREATE POLICY same_country ON waterway.gauge_measurements - FOR ALL TO waterway_admin - USING (staging_done - OR (location).country_code = - (SELECT country FROM users.list_users - WHERE username = current_user)) - WITH CHECK ((location).country_code = - (SELECT country FROM users.list_users - WHERE username = current_user)); - -CREATE POLICY same_country ON waterway.waterway_profiles - FOR ALL TO waterway_admin - USING (staging_done - OR (location).country_code = - (SELECT country FROM users.list_users - WHERE username = current_user)) - WITH CHECK ((location).country_code = - (SELECT country FROM users.list_users - WHERE username = current_user)); +DO LANGUAGE plpgsql +$do$ +DECLARE + the_table varchar; + condition CONSTANT text = $$ + (location).country_code = + (SELECT country FROM users.list_users + WHERE username = current_user) + $$; +BEGIN + FOREACH the_table IN ARRAY ARRAY[ + 'gauge_measurements', + 'waterway_profiles'] + LOOP + EXECUTE format($$ + CREATE POLICY same_country_insert ON waterway.%I + FOR INSERT TO waterway_admin + WITH CHECK (%s) + $$, the_table, condition); + -- In many cases it is more efficient to check for "staging_done" to + -- prevent the more expensive checks for read only access (which is + -- allowed for all users, when staging is done). + EXECUTE format($$ + CREATE POLICY same_country_select ON waterway.%I + FOR SELECT TO waterway_admin + USING (staging_done OR %s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY same_country_update ON waterway.%I + FOR UPDATE TO waterway_admin + USING (%s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY same_country_delete ON waterway.%I + FOR DELETE TO waterway_admin + USING (%s) + $$, the_table, condition); + END LOOP; +END; +$do$; -CREATE POLICY responsibility_area ON waterway.bottlenecks - FOR ALL TO waterway_admin - USING (staging_done - OR (SELECT ST_Covers(a, - ST_Transform(CAST(area AS geometry), ST_SRID(a))) - FROM users.current_user_area_utm() AS a (a))) - WITH CHECK ((SELECT ST_Covers(a, - ST_Transform(CAST(area AS geometry), ST_SRID(a))) - FROM users.current_user_area_utm() AS a (a))); - -CREATE POLICY responsibility_area ON waterway.sounding_results - FOR ALL TO waterway_admin - USING (staging_done - OR (SELECT ST_Covers(a, - ST_Transform(CAST(area AS geometry), ST_SRID(a))) - FROM users.current_user_area_utm() AS a (a))) - WITH CHECK ((SELECT ST_Covers(a, - ST_Transform(CAST(area AS geometry), ST_SRID(a))) - FROM users.current_user_area_utm() AS a (a))); - -CREATE POLICY responsibility_area ON waterway.fairway_dimensions - FOR ALL TO waterway_admin - USING (staging_done - OR (SELECT ST_Covers(a, - ST_Transform(CAST(area AS geometry), ST_SRID(a))) - FROM users.current_user_area_utm() AS a (a))) - WITH CHECK ((SELECT ST_Covers(a, - ST_Transform(CAST(area AS geometry), ST_SRID(a))) - FROM users.current_user_area_utm() AS a (a))); +DO LANGUAGE plpgsql +$do$ +DECLARE + the_table varchar; + condition CONSTANT text = $$ + (SELECT ST_Covers(a, ST_Transform(CAST(area AS geometry), ST_SRID(a))) + FROM users.current_user_area_utm() AS a (a)) + $$; +BEGIN + FOREACH the_table IN ARRAY ARRAY[ + 'fairway_dimensions', + 'bottlenecks', + 'sounding_results'] + LOOP + EXECUTE format($$ + CREATE POLICY responsibility_area_insert ON waterway.%I + FOR INSERT TO waterway_admin + WITH CHECK (%s) + $$, the_table, condition); + -- In many cases it is more efficient to check for "staging_done" to + -- prevent the more expensive checks for read only access (which is + -- allowed for all users, when staging is done). + EXECUTE format($$ + CREATE POLICY responsibility_area_select ON waterway.%I + FOR SELECT TO waterway_admin + USING (staging_done OR %s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY responsibility_area_update ON waterway.%I + FOR UPDATE TO waterway_admin + USING (%s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY responsibility_area_delete ON waterway.%I + FOR DELETE TO waterway_admin + USING (%s) + $$, the_table, condition); + END LOOP; +END; +$do$; -- In the case of sections differentiating between read and write -- access is not neccessary: the country code based access check is
--- a/schema/auth_tests.sql Mon Mar 16 17:11:53 2020 +0100 +++ b/schema/auth_tests.sql Wed Mar 18 12:16:42 2020 +0100 @@ -102,6 +102,41 @@ 42501, NULL, 'Waterway admin cannot insert data outside his region'); +-- Ensure a USING clause prevents access in an UPDATE +SELECT is_empty($$ + WITH a AS (SELECT users.current_user_area_utm() AS a) + UPDATE waterway.bottlenecks + SET objnam = 'Now it''s mine', + area = ST_geomfromtext( + 'MULTIPOLYGON(((0 0, 0 1, 1 1, 1 0, 0 0)))', 4326) + WHERE bottleneck_id = 'testbottleneck3' + RETURNING * + $$, + 'Waterway admin cannot move data from outside his region inside'); + +-- Ensure a WITH CHECK or USING clause prevents writing such rows +SELECT throws_ok($$ + WITH a AS (SELECT users.current_user_area_utm() AS a) + UPDATE waterway.bottlenecks + SET objnam = 'Give-away', + area = ST_geomfromtext( + 'MULTIPOLYGON(((1 0, 1 1, 2 1, 2 0, 1 0)))', 4326) + WHERE bottleneck_id = 'testbottleneck2' + RETURNING * + $$, + 42501, NULL, + 'Waterway admin cannot move data from inside his region outside'); + +SELECT is_empty($$ + WITH a AS (SELECT users.current_user_area_utm() AS a) + DELETE FROM waterway.bottlenecks + WHERE NOT ST_Covers((SELECT a FROM a), + ST_Transform( + CAST(area AS geometry), ST_SRID((SELECT a FROM a)))) + RETURNING * + $$, + 'Waterway admin cannot delete data outside his region'); + -- template management SELECT lives_ok($$ INSERT INTO users.templates (template_name, template_data, country)
--- a/schema/run_tests.sh Mon Mar 16 17:11:53 2020 +0100 +++ b/schema/run_tests.sh Wed Mar 18 12:16:42 2020 +0100 @@ -80,7 +80,7 @@ -c 'SET client_min_messages TO WARNING' \ -c "DROP ROLE IF EXISTS $TEST_ROLES" \ -f "$BASEDIR"/tap_tests_data.sql \ - -c "SELECT plan(82 + ( + -c "SELECT plan(85 + ( SELECT count(*)::int FROM information_schema.tables WHERE table_schema = 'waterway'))" \
--- a/schema/tap_tests_data.sql Mon Mar 16 17:11:53 2020 +0100 +++ b/schema/tap_tests_data.sql Wed Mar 18 12:16:42 2020 +0100 @@ -102,6 +102,20 @@ ST_geomfromtext('MULTIPOLYGON(((0 0, 0 1, 1 1, 1 0, 0 0)))', 4326), 'AT', 'AT', 'AT', 1, 'depth', current_timestamp, 'testorganization', true + ), ( + 'testbottleneck3', + isrsrange(('RO', 'XXX', '00001', '00000', 0)::isrs, + ('RO', 'XXX', '00001', '00000', 2)::isrs), + ST_geomfromtext('MULTIPOLYGON(((1 0, 1 1, 2 1, 2 0, 1 0)))', 4326), + 'RO', 'RO', 'RO', + 1, 'depth', current_timestamp, 'testorganization', true + ), ( + 'testbottleneck4', + isrsrange(('RO', 'XXX', '00001', '00000', 0)::isrs, + ('RO', 'XXX', '00001', '00000', 2)::isrs), + ST_geomfromtext('MULTIPOLYGON(((1 0, 1 1, 2 1, 2 0, 1 0)))', 4326), + 'RO', 'RO', 'RO', + 1, 'depth', current_timestamp, 'testorganization', true )) INSERT INTO waterway.bottlenecks ( gauge_location, validity,
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/schema/updates/1427/01.fix_rls_policies.sql Wed Mar 18 12:16:42 2020 +0100 @@ -0,0 +1,86 @@ +DROP POLICY same_country ON waterway.gauge_measurements; +DROP POLICY same_country ON waterway.waterway_profiles; + +DO LANGUAGE plpgsql +$do$ +DECLARE + the_table varchar; + condition CONSTANT text = $$ + (location).country_code = + (SELECT country FROM users.list_users + WHERE username = current_user) + $$; +BEGIN + FOREACH the_table IN ARRAY ARRAY[ + 'gauge_measurements', + 'waterway_profiles'] + LOOP + EXECUTE format($$ + CREATE POLICY same_country_insert ON waterway.%I + FOR INSERT TO waterway_admin + WITH CHECK (%s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY same_country_select ON waterway.%I + FOR SELECT TO waterway_admin + USING (staging_done OR %s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY same_country_update ON waterway.%I + FOR UPDATE TO waterway_admin + USING (%s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY same_country_delete ON waterway.%I + FOR DELETE TO waterway_admin + USING (%s) + $$, the_table, condition); + END LOOP; +END; +$do$; + + +DROP POLICY responsibility_area ON waterway.bottlenecks; +DROP POLICY responsibility_area ON waterway.sounding_results; +DROP POLICY responsibility_area ON waterway.fairway_dimensions; + +DO LANGUAGE plpgsql +$do$ +DECLARE + the_table varchar; + condition CONSTANT text = $$ + (SELECT ST_Covers(a, ST_Transform(CAST(area AS geometry), ST_SRID(a))) + FROM users.current_user_area_utm() AS a (a)) + $$; +BEGIN + FOREACH the_table IN ARRAY ARRAY[ + 'fairway_dimensions', + 'bottlenecks', + 'sounding_results'] + LOOP + EXECUTE format($$ + CREATE POLICY responsibility_area_insert ON waterway.%I + FOR INSERT TO waterway_admin + WITH CHECK (%s) + $$, the_table, condition); + -- In many cases it is more efficient to check for "staging_done" to + -- prevent the more expensive checks for read only access (which is + -- allowed for all users, when staging is done). + EXECUTE format($$ + CREATE POLICY responsibility_area_select ON waterway.%I + FOR SELECT TO waterway_admin + USING (staging_done OR %s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY responsibility_area_update ON waterway.%I + FOR UPDATE TO waterway_admin + USING (%s) + $$, the_table, condition); + EXECUTE format($$ + CREATE POLICY responsibility_area_delete ON waterway.%I + FOR DELETE TO waterway_admin + USING (%s) + $$, the_table, condition); + END LOOP; +END; +$do$;