2
votes

Below you can find code for a minimal test case for an issue I am having in a system where RLS is used to manage access to a hierarchical datastructure. I am using Postgres v11.

In the code I have units, which is a top level object. units have subunits in a 1-n relation.

There are also users, where a user can have access to a number of units through the unit_owner table.

The RLS policies are designed to let a user insert new subunits into the units he owns.

All this works fine as can be seen up until the 2nd last line in the code.

BUT here is my issue: This database is exposed through a GraphQL middleware (Postgraphile) which needs the result of an insert back by means of the INSERT ... RETURNING feature.

And as can be seen in the last insert statement, this does not work, it gets "ERROR: new row violates row-level security policy".

The problem seems to arise from the fact that RETURNING needs the select rights, and the select policy function is evaluated using the set of subunit ids available before the insert, not after.

Any tips on how I can let my users insert subunits into their units would be appreciated!

CREATE SCHEMA insert_returning;
CREATE ROLE users;

GRANT USAGE ON SCHEMA insert_returning TO users;

DROP TABLE IF EXISTS insert_returning.unit;
DROP TABLE IF EXISTS insert_returning.subunit;
DROP TABLE IF EXISTS insert_returning.unit_owner;

CREATE TABLE insert_returning.unit (
    id integer NOT NULL,
    description varchar NULL,
    CONSTRAINT unit_pk PRIMARY KEY (id)
);

CREATE TABLE insert_returning.subunit (
    id integer NOT NULL,
    unit_id integer NOT NULL,
    description varchar NULL,
    CONSTRAINT subunit_pk PRIMARY KEY (id)
);

CREATE TABLE insert_returning.unit_owner (
    user_id integer NOT NULL,
    unit_id integer NOT NULL
);

GRANT SELECT,INSERT,UPDATE ON TABLE insert_returning.unit TO users;
GRANT SELECT,INSERT,UPDATE ON TABLE insert_returning.subunit TO users;

GRANT SELECT ON TABLE insert_returning.unit_owner TO users;

CREATE OR REPLACE FUNCTION insert_returning.get_users_units()
RETURNS SETOF integer
LANGUAGE sql VOLATILE SECURITY DEFINER AS
$$
  SELECT uo.unit_id FROM insert_returning.unit_owner uo
    WHERE uo.user_id = 17;
$$;


CREATE OR REPLACE FUNCTION insert_returning.get_users_subunits()
RETURNS SETOF integer
LANGUAGE sql VOLATILE SECURITY DEFINER AS
$$
  SELECT s.id FROM insert_returning.subunit s
    JOIN insert_returning.unit_owner uo ON uo.unit_id = s.unit_id
    WHERE uo.user_id = 17;
$$;


ALTER TABLE insert_returning.unit ENABLE ROW LEVEL SECURITY;
ALTER TABLE insert_returning.subunit ENABLE ROW LEVEL SECURITY;

DROP POLICY IF EXISTS select_unit ON insert_returning.unit;
DROP POLICY IF EXISTS select_subunit ON insert_returning.subunit;
DROP POLICY IF EXISTS insert_subunit ON insert_returning.subunit;

CREATE POLICY select_unit ON insert_returning.unit FOR SELECT TO PUBLIC USING ((
    SELECT (id IN ( SELECT unit_id FROM insert_returning.unit_owner WHERE user_id = 17))
));

CREATE POLICY select_subunit ON insert_returning.subunit FOR SELECT TO PUBLIC USING ((
    SELECT (id IN (SELECT insert_returning.get_users_subunits()) )
));

CREATE POLICY insert_subunit ON insert_returning.subunit FOR INSERT TO PUBLIC WITH CHECK ((
    SELECT (unit_id IN (SELECT insert_returning.get_users_units()) )
));


INSERT INTO insert_returning.unit (id, description) VALUES (1, 'I am visible');
INSERT INTO insert_returning.unit (id, description) VALUES (2, 'I am hidden');

INSERT INTO insert_returning.subunit (id, unit_id, description) VALUES (1, 1, 'I belong to a visible unit');
INSERT INTO insert_returning.subunit (id, unit_id, description) VALUES (2, 2, 'I belong to a hidden unit');
INSERT INTO insert_returning.subunit (id, unit_id, description) VALUES (3, 1, 'I too belong to a visible unit');

INSERT INTO insert_returning.unit_owner (user_id,unit_id) VALUES (17,1);

SET ROLE users;

SELECT * FROM insert_returning.subunit; -- works

INSERT INTO insert_returning.subunit VALUES (4, 1, 'I am a new subunit'); -- works

INSERT INTO insert_returning.subunit VALUES (5, 1, 'I am another new subunit') RETURNING *; -- FAILS

--
2
Why are you using id IN get_users_subunits() for the SELECT policy instead of the simpler unit_id in get_users_units()? This approach worked for the INSERT policy, it should be used for the SELECT policy as well I guess.Bergi
:-) sure. This is a heavily reduced/minimized version of the actual code, just to make sure I post a working test case relevant to the issue. No effort spent in making it good at anything else. In the real code the algorithm to get the set of subunits the user has access to is more involved.Jesper We
It still suffers from the same issue though: The select policy gets evaluated before the insert and thus does not permit read access to the just inserted row.Jesper We
Yes, I thought so (and the question is very well written), but can't you do the same trick in your actual code to use the same rule in the select policy as in the insert policy?Bergi
The only alternative I can think of would be another SECURITY DEFINER function that works around the rls, checks the permission manually and returns the id of the inserted row.Bergi

2 Answers

1
votes

You analyzed the problem right: the inserted row is not available to the subquery in the FOR SELECT policy on subunit.

There is no way to "make it work" like this. You will have to find a different test for the policy, one that does not expect to find the new row in the table. The way your case is written, you could directly use the unit_id of the new row for a simpler test, but you assure us that this wouldn't work in your real use case...

You cannot select the new row, but you can use all attributes of the new row. So try to write the condition using an SQL expression that does not involve a subselect on the table itself.

0
votes

In order to make it work (and you do not see a way to change the underlying RLS), you can create a custom mutation function which you can mark as SECURITY DEFINER.

In this mutation function you will have to make the checking yourself.

This does not answer your question regarding the RLS - which I think has already been properly accessed in another answer in this question. It is more likely a tip from a fellow Postgraphile user.

Also:

  • In my experience using functions in RLS is almost always a performance penalty. Especially when they are not inlined. In your case VOLATILE and SECURITY DEFINER should already prevent inlining.

  • It is almost always faster to use EXISTS instead of IN in RLS definitions. Your experience may differ.