# HG changeset patch # User Thomas Junk # Date 1532949206 -7200 # Node ID 863ec6010045948a4f0b2abb7145ac75cb0bcfec # Parent 89f1aa33adcf617e03c8c792a1c287fa989273f6# Parent 61f1374f0c4412f703541e11a75e19c18a777794 merge diff -r 89f1aa33adcf -r 863ec6010045 README.md --- a/README.md Mon Jul 30 13:13:10 2018 +0200 +++ b/README.md Mon Jul 30 13:13:26 2018 +0200 @@ -22,7 +22,7 @@ necessary roles in the postgres default cluster (listening on port 5432). -- The script mus be run as a user with PostgreSQL super user rights. +- The script must be run as a user with PostgreSQL super user rights. By convention this is the "postgres" on most systems. - Standard case: as user "postgres", in the top level directory of the diff -r 89f1aa33adcf -r 863ec6010045 controllers/routes.go --- a/controllers/routes.go Mon Jul 30 13:13:10 2018 +0200 +++ b/controllers/routes.go Mon Jul 30 13:13:26 2018 +0200 @@ -12,7 +12,10 @@ api := m.PathPrefix("/api").Subrouter() - sysAdmin := auth.EnsureRole("sys_admin") + var ( + sysAdmin = auth.EnsureRole("sys_admin") + all = auth.EnsureRole("sys_admin", "waterway_admin", "waterway_user") + ) api.Handle("/users", sysAdmin(&JSONHandler{ Handle: listUsers, @@ -23,11 +26,11 @@ Handle: createUser, })).Methods(http.MethodPost) - api.Handle("/users/{user}", sysAdmin(&JSONHandler{ + api.Handle("/users/{user}", all(&JSONHandler{ Handle: listUser, })).Methods(http.MethodGet) - api.Handle("/users/{user}", sysAdmin(&JSONHandler{ + api.Handle("/users/{user}", all(&JSONHandler{ Input: func() interface{} { return new(User) }, Handle: updateUser, })).Methods(http.MethodPut) diff -r 89f1aa33adcf -r 863ec6010045 schema/auth.sql --- a/schema/auth.sql Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/auth.sql Mon Jul 30 13:13:26 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; diff -r 89f1aa33adcf -r 863ec6010045 schema/auth_tests.sql --- a/schema/auth_tests.sql Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/auth_tests.sql Mon Jul 30 13:13:26 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 diff -r 89f1aa33adcf -r 863ec6010045 schema/demo-data/users.sql --- a/schema/demo-data/users.sql Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/demo-data/users.sql Mon Jul 30 13:13:26 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) diff -r 89f1aa33adcf -r 863ec6010045 schema/gemma.sql --- a/schema/gemma.sql Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/gemma.sql Mon Jul 30 13:13:26 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 diff -r 89f1aa33adcf -r 863ec6010045 schema/install-db.sh --- a/schema/install-db.sh Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/install-db.sh Mon Jul 30 13:13:26 2018 +0200 @@ -97,7 +97,7 @@ read a if [[ $a == "yes" ]] ; then dropdb -p "$port" "$db" - for r in `psql -p 5433 -t -c '\du' | awk -F '|' \ + for r in `psql -p $port -t -c '\du' | awk -F '|' \ '$3 ~/waterway_user|waterway_admin|sys_admin/ \ || $1 ~/waterway_user|waterway_admin|sys_admin/ \ {print $1}'` diff -r 89f1aa33adcf -r 863ec6010045 schema/manage_users.sql --- a/schema/manage_users.sql Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/manage_users.sql Mon Jul 30 13:13:26 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 @@ -37,29 +38,24 @@ CREATE OR REPLACE VIEW users.list_users WITH (security_barrier) AS - WITH cur AS ( - SELECT rolname - FROM pg_roles r JOIN pg_auth_members a ON r.oid = a.roleid - WHERE member = ( - SELECT oid FROM pg_roles WHERE rolname = current_user)) SELECT r.rolname, p.* - FROM cur, users.user_profiles p + FROM 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 WHERE p.username = current_user - OR cur.rolname = 'waterway_admin' + OR pg_has_role('waterway_admin', 'MEMBER') AND p.country = current_user_country() - OR cur.rolname = 'sys_admin'; + OR pg_has_role('sys_admin', 'MEMBER'); 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 +65,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 +80,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 +96,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 +136,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 +153,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 diff -r 89f1aa33adcf -r 863ec6010045 schema/manage_users_tests.sql --- a/schema/manage_users_tests.sql Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/manage_users_tests.sql Mon Jul 30 13:13:26 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 diff -r 89f1aa33adcf -r 863ec6010045 schema/run_tests.sh --- a/schema/run_tests.sh Mon Jul 30 13:13:10 2018 +0200 +++ b/schema/run_tests.sh Mon Jul 30 13:13:26 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()'