Mercurial > gemma

changeset 268:72062ca52746

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Make user_profiles table invisible for users users.list_users should be the single point to access user profile data. Keeping user_profiles visible would imply having to maintain RLS policies that are otherwise obsolete. Tests run as superuser still use user_profiles, because list_users does not show any data to a superuser.
author Tom Gottfried <tom@intevation.de>
date Mon, 30 Jul 2018 11:38:09 +0200
parents 7f030ec3472d
children 0b2d9f96ddb8
files schema/auth.sql schema/auth_tests.sql schema/demo-data/users.sql schema/gemma.sql schema/manage_users.sql schema/manage_users_tests.sql schema/run_tests.sh
diffstat 7 files changed, 52 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/schema/auth.sql	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/auth.sql	Mon Jul 30 11:38:09 2018 +0200
@@ -35,10 +35,10 @@
 --
 -- 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
+-- Sometimes using 'username IN(SELECT username FROM users.list_users)' 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).
+-- filtering on list_users depending on current_user).
 --
 
 -- Staging area
@@ -62,12 +62,8 @@
 SELECT create_hide_staging_policy();
 DROP FUNCTION create_hide_staging_policy;
 
-CREATE POLICY see_yourself ON users.user_profiles FOR SELECT TO waterway_user
-    USING (username = current_user);
-ALTER TABLE users.user_profiles ENABLE ROW LEVEL SECURITY;
-
 CREATE POLICY user_templates ON users.user_templates FOR ALL TO waterway_user
-    USING (username IN(SELECT username FROM users.user_profiles));
+    USING (username IN(SELECT username FROM users.list_users));
 ALTER TABLE users.user_templates ENABLE ROW LEVEL SECURITY;
 
 CREATE POLICY user_templates ON users.templates FOR ALL TO waterway_user
@@ -90,8 +86,4 @@
     USING (ST_Within(area, (SELECT area FROM users.responsibility_areas
         WHERE country = current_user_country())));
 
-CREATE POLICY country_profiles ON users.user_profiles
-    FOR SELECT TO waterway_admin
-    USING (country = current_user_country());
-
 COMMIT;
--- a/schema/auth_tests.sql	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/auth_tests.sql	Mon Jul 30 11:38:09 2018 +0200
@@ -15,12 +15,6 @@
 SELECT is_empty('SELECT * FROM waterway.bottlenecks WHERE NOT staging_done',
                 'Only staged data should be visible');
 
-SELECT set_eq('SELECT count(*) FROM users.user_profiles', ARRAY[1],
-              'User should only see his own profile');
-SELECT results_eq('SELECT username FROM users.user_profiles',
-                  'SELECT CAST(current_user AS varchar)',
-                  'User should only see his own profile');
-
 SELECT isnt_empty('SELECT * FROM users.templates',
                   'User should see templates associated to him');
 SELECT is_empty('SELECT * FROM users.templates
--- a/schema/demo-data/users.sql	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/demo-data/users.sql	Mon Jul 30 11:38:09 2018 +0200
@@ -5,7 +5,7 @@
 -- responsibility areas from responsibility_areas.sql are imported
 
 -- Fill in Profiles
-COPY users.user_profiles (username, country, email_address, map_extent) FROM stdin;
+COPY internal.user_profiles (username, country, email_address, map_extent) FROM stdin;
 sophie	AT	sophie@example.com	BOX(9.52115482500011 46.3786430870001,17.1483378500001 49.0097744750001)
 lucian	RO	lucian@example.com	BOX(20.2428259690001 43.6500499480001,29.6995548840001 48.2748322560001)
 oana	RO	oana@example.com	BOX(20.2428259690001 43.6500499480001,29.6995548840001 48.2748322560001)
--- a/schema/gemma.sql	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/gemma.sql	Mon Jul 30 11:38:09 2018 +0200
@@ -36,6 +36,18 @@
 -- GEMMA data
 --
 
+-- Namespace not to be accessed directly by any user
+CREATE SCHEMA internal
+    -- Profile data are only accessible via the view users.list_users.
+    CREATE TABLE user_profiles (
+        username varchar PRIMARY KEY,
+        map_extent box2d NOT NULL,
+        email_address varchar NOT NULL
+    )
+    -- Columns referencing user-visible schemas added below.
+;
+
+
 -- Namespace to be accessed by sys_admin only
 CREATE SCHEMA sys_admin
     CREATE TABLE sys_admin.system_config (
@@ -133,13 +145,6 @@
         --XXX: Should be geography (elsewhere too)
     )
 
-    CREATE TABLE user_profiles (
-        username varchar PRIMARY KEY,
-        country char(2) NOT NULL REFERENCES responsibility_areas,
-        map_extent box2d NOT NULL,
-        email_address varchar NOT NULL
-    )
-
     CREATE TABLE templates (
         template_name varchar PRIMARY KEY,
         template_data bytea NOT NULL,
@@ -150,11 +155,14 @@
 
     CREATE TABLE user_templates (
         username varchar NOT NULL
-            REFERENCES user_profiles ON DELETE CASCADE ON UPDATE CASCADE,
+            REFERENCES internal.user_profiles
+                ON DELETE CASCADE ON UPDATE CASCADE,
         template_name varchar NOT NULL REFERENCES templates ON DELETE CASCADE,
         PRIMARY KEY (username, template_name)
     )
 ;
+ALTER TABLE internal.user_profiles ADD
+    country char(2) NOT NULL REFERENCES users.responsibility_areas;
 
 -- Namespace for waterway data that can change in a running system
 CREATE SCHEMA waterway
--- a/schema/manage_users.sql	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/manage_users.sql	Mon Jul 30 11:38:09 2018 +0200
@@ -27,9 +27,10 @@
 -- Security-definer function to get current users country, which allows to
 -- restrict the view on user_profiles by country without infinite recursion
 CREATE FUNCTION current_user_country()
-    RETURNS users.user_profiles.country%TYPE
+    RETURNS internal.user_profiles.country%TYPE
     AS $$
-        SELECT country FROM users.user_profiles WHERE username = session_user
+        SELECT country FROM internal.user_profiles
+            WHERE username = session_user
     $$
     LANGUAGE SQL
     SECURITY DEFINER
@@ -43,7 +44,7 @@
             WHERE member = (
                 SELECT oid FROM pg_roles WHERE rolname = current_user))
     SELECT r.rolname, p.*
-        FROM cur, users.user_profiles p
+        FROM cur, internal.user_profiles p
             JOIN pg_roles u ON p.username = u.rolname
             JOIN pg_auth_members a ON u.oid = a.member
             JOIN pg_roles r ON a.roleid = r.oid
@@ -55,11 +56,11 @@
 
 CREATE OR REPLACE FUNCTION sys_admin.create_user(
        userrole varchar,
-       username users.user_profiles.username%TYPE,
+       username internal.user_profiles.username%TYPE,
        pw varchar,
-       country users.user_profiles.country%TYPE,
-       map_extent users.user_profiles.map_extent%TYPE,
-       email_address users.user_profiles.email_address%TYPE
+       country internal.user_profiles.country%TYPE,
+       map_extent internal.user_profiles.map_extent%TYPE,
+       email_address internal.user_profiles.email_address%TYPE
     )
     RETURNS void
 AS $$
@@ -69,8 +70,9 @@
         map_extent = ST_Extent(area) FROM users.responsibility_areas ra
             WHERE ra.country = create_user.country;
     END IF;
-    INSERT INTO users.user_profiles VALUES (
-        username, country, map_extent, email_address);
+    INSERT INTO internal.user_profiles (
+        username, country, map_extent, email_address)
+        VALUES (username, country, map_extent, email_address);
     EXECUTE format(
         'CREATE ROLE %I IN ROLE %I LOGIN PASSWORD %L',
         username,
@@ -83,13 +85,13 @@
 
 
 CREATE OR REPLACE FUNCTION sys_admin.update_user(
-       username users.user_profiles.username%TYPE,
+       username internal.user_profiles.username%TYPE,
        new_userrole varchar,
-       new_username users.user_profiles.username%TYPE,
+       new_username internal.user_profiles.username%TYPE,
        new_pw varchar,
-       new_country users.user_profiles.country%TYPE,
-       new_map_extent users.user_profiles.map_extent%TYPE,
-       new_email_address users.user_profiles.email_address%TYPE
+       new_country internal.user_profiles.country%TYPE,
+       new_map_extent internal.user_profiles.map_extent%TYPE,
+       new_email_address internal.user_profiles.email_address%TYPE
     )
     RETURNS void
 AS $$
@@ -99,7 +101,7 @@
 BEGIN
     cur_username = username;
 
-    UPDATE users.user_profiles p
+    UPDATE internal.user_profiles p
         SET (username, country, map_extent, email_address)
         = (new_username, new_country, new_map_extent, new_email_address)
         WHERE p.username = cur_username;
@@ -139,7 +141,7 @@
 
 
 CREATE OR REPLACE FUNCTION sys_admin.delete_user(
-       username users.user_profiles.username%TYPE
+       username internal.user_profiles.username%TYPE
     )
     RETURNS void
 AS $$
@@ -156,7 +158,8 @@
 
     -- Delete user
     EXECUTE format('DROP ROLE %I', username);
-    DELETE FROM users.user_profiles p WHERE p.username = delete_user.username;
+    DELETE FROM internal.user_profiles p
+        WHERE p.username = delete_user.username;
 END;
 $$
     LANGUAGE plpgsql
--- a/schema/manage_users_tests.sql	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/manage_users_tests.sql	Mon Jul 30 11:38:09 2018 +0200
@@ -107,7 +107,7 @@
         'waterway_user', 'test2', 'secret1$', 'AT', NULL, 'test2');
     SELECT sys_admin.update_user('test2',
         'waterway_user', 'test2_new', 'new_secret1$', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test5')
     $$,
     'Existing user can be updated');
@@ -115,7 +115,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user('test_non_existent',
         'waterway_user', 'test_non_existent', '', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test5')
     $$,
     42704, NULL,
@@ -124,7 +124,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user(CAST(current_user AS varchar),
         'waterway_user', 'test_new_name', 'secret1$', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test6')
     $$,
     '0A000', NULL,
@@ -133,7 +133,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user('test_user_at',
         'invalid', 'test2', 'secret1$', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test2')
     $$,
     42704, NULL,
@@ -142,7 +142,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user('test_user_at',
         'waterway_user', NULL, 'secret1$', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test3')
     $$,
     23502, NULL,
@@ -152,7 +152,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user('test_user_at',
         'waterway_user', 'waterway_user', 'secret1$', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test4')
     $$,
     42710, NULL,
@@ -161,7 +161,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user('test_user_at',
         'waterway_user', 'test_user_ro', 'secret1$', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test4')
     $$,
     23505, NULL,
@@ -171,7 +171,7 @@
 SELECT throws_ok($$
     SELECT sys_admin.update_user('test_user_at',
         'waterway_user', 'test_user_at', 'secret', 'AT',
-        (SELECT map_extent FROM users.user_profiles
+        (SELECT map_extent FROM users.list_users
             WHERE username = 'test_user_at'), 'test4')
     $$,
     '28P01', NULL,
@@ -185,7 +185,7 @@
 
 SELECT sys_admin.update_user('test_user_at',
     'waterway_user', 'test_user_at', NULL, 'AT',
-    (SELECT map_extent FROM users.user_profiles
+    (SELECT map_extent FROM internal.user_profiles
         WHERE username = 'test_user_at'), 'xxx');
 SELECT set_eq($$
     SELECT rolpassword FROM old_pw_hash
@@ -197,7 +197,7 @@
 
 SELECT sys_admin.update_user('test_user_at',
     'waterway_user', 'test_user_at', '', 'AT',
-    (SELECT map_extent FROM users.user_profiles
+    (SELECT map_extent FROM internal.user_profiles
         WHERE username = 'test_user_at'), 'xxx');
 SELECT set_eq($$
     SELECT rolpassword FROM old_pw_hash
@@ -209,7 +209,7 @@
 
 SELECT sys_admin.update_user('test_user_at',
     'waterway_user', 'test_user_at', 'new_pw1$', 'AT',
-    (SELECT map_extent FROM users.user_profiles
+    (SELECT map_extent FROM internal.user_profiles
         WHERE username = 'test_user_at'), 'xxx');
 SELECT set_ne($$
     SELECT rolpassword FROM old_pw_hash
--- a/schema/run_tests.sh	Mon Jul 30 11:08:17 2018 +0200
+++ b/schema/run_tests.sh	Mon Jul 30 11:38:09 2018 +0200
@@ -15,7 +15,7 @@
 psql -qXv ON_ERROR_STOP= -v -d gemma_test \
     -c "DROP ROLE IF EXISTS $TEST_ROLES" \
     -f tap_tests_data.sql \
-    -c 'SELECT plan(42)' \
+    -c 'SELECT plan(40)' \
     -f auth_tests.sql \
     -f manage_users_tests.sql \
     -c 'SELECT * FROM finish()'