0

Background

I'm working these days on organizing the Postgres database of a project in my work.

EDIT: This database is connected to a NodeJS server that runs Postgraphile on it, in order to expose the GraphQL interface for the client. Therefore, I have to use RLS in order to forbid the user to query and manipulate rows that he/she doesn't have permission.

One of the tasks that I've got is to add a deleted field for each table, and using RLS to hide the records that deleted = true.

Code Example

To explain my problem, I'll add an SQL code for building a fake database:

Roles

For this example, I'll use these roles:

  • Superuser role named admin
  • Role called app_users
  • 2 Users inherit from app_users:
    • bob
    • alice
CREATE ROLE admin WITH
  LOGIN
  SUPERUSER
  INHERIT
  CREATEDB
  CREATEROLE
  NOREPLICATION
  ENCRYPTED PASSWORD 'md5f6fdffe48c908deb0f4c3bd36c032e72'; -- password: admin

GRANT username TO admin;

CREATE ROLE app_users WITH
  NOLOGIN
  NOSUPERUSER
  NOINHERIT
  NOCREATEDB
  CREATEROLE
  NOREPLICATION;

CREATE ROLE bob WITH
  LOGIN
  NOSUPERUSER
  INHERIT
  NOCREATEDB
  NOCREATEROLE
  NOREPLICATION
  ENCRYPTED PASSWORD 'md5e8557d12f6551b2ddd26bbdd0395465c';

GRANT app_users TO bob;

CREATE ROLE alice WITH
  LOGIN
  NOSUPERUSER
  INHERIT
  NOCREATEDB
  NOCREATEROLE
  NOREPLICATION
  ENCRYPTED PASSWORD 'md5579e43b423b454623383471aeb85cd87';

GRANT app_users TO alice;

Database

This example will hold a database named league for a mock database for an American football league.

CREATE DATABASE league
    WITH 
    OWNER = admin
    ENCODING = 'UTF8'
    LC_COLLATE = 'en_US.utf8'
    LC_CTYPE = 'en_US.utf8'
    TABLESPACE = pg_default
    CONNECTION LIMIT = -1;

GRANT CREATE, CONNECT ON DATABASE league TO admin;
GRANT TEMPORARY ON DATABASE league TO admin WITH GRANT OPTION;

GRANT TEMPORARY, CONNECT ON DATABASE league TO PUBLIC;

Scheme: public

I've added some minor changes in the scheme, so in default, role app_users allow any command, type, execute function, etcetera.

ALTER DEFAULT PRIVILEGES IN SCHEMA public
GRANT ALL ON TABLES TO app_users;

ALTER DEFAULT PRIVILEGES IN SCHEMA public
GRANT ALL ON SEQUENCES TO app_users;

ALTER DEFAULT PRIVILEGES IN SCHEMA public
GRANT EXECUTE ON FUNCTIONS TO app_users;

ALTER DEFAULT PRIVILEGES IN SCHEMA public
GRANT USAGE ON TYPES TO app_users;

Creating Tables

Table: TEAMS

CREATE TABLE IF NOT EXISTS public."TEAMS"
(
    id integer NOT NULL GENERATED ALWAYS AS IDENTITY ( INCREMENT 1 START 1 MINVALUE 1 MAXVALUE 2147483647 CACHE 1 ),
    deleted boolean NOT NULL DEFAULT false,
    name text COLLATE pg_catalog."default" NOT NULL,
    owner text COLLATE pg_catalog."default" NOT NULL,
    CONSTRAINT "TEAMS_pkey" PRIMARY KEY (id)
)

TABLESPACE pg_default;

ALTER TABLE public."TEAMS"
    OWNER to admin;

ALTER TABLE public."TEAMS"
    ENABLE ROW LEVEL SECURITY;

GRANT ALL ON TABLE public."TEAMS" TO admin;
GRANT ALL ON TABLE public."TEAMS" TO app_users;

CREATE POLICY teams_deleted
    ON public."TEAMS"
    AS RESTRICTIVE
    FOR SELECT
    TO app_users
    USING (deleted = false);

CREATE POLICY teams_owner
    ON public."TEAMS"
    AS PERMISSIVE
    FOR ALL
    TO app_users
    USING (owner = CURRENT_USER);

Table: PLAYERS

CREATE TABLE IF NOT EXISTS public."PLAYERS"
(
    id text COLLATE pg_catalog."default" NOT NULL,
    deleted boolean NOT NULL DEFAULT false,
    first_name text COLLATE pg_catalog."default" NOT NULL,
    last_name text COLLATE pg_catalog."default" NOT NULL,
    team_id integer NOT NULL,
    jersey_number integer NOT NULL,
    CONSTRAINT "PLAYERS_pkey" PRIMARY KEY (id),
    CONSTRAINT fkey_team_id FOREIGN KEY (team_id)
        REFERENCES public."TEAMS" (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE CASCADE,
    CONSTRAINT check_player_number CHECK (jersey_number > 0 AND jersey_number < 100)
)

TABLESPACE pg_default;

ALTER TABLE public."PLAYERS"
    OWNER to admin;

ALTER TABLE public."PLAYERS"
    ENABLE ROW LEVEL SECURITY;

GRANT ALL ON TABLE public."PLAYERS" TO admin;
GRANT ALL ON TABLE public."PLAYERS" TO app_users;

CREATE POLICY players_deleted
    ON public."PLAYERS"
    AS RESTRICTIVE
    FOR SELECT
    TO app_users
    USING (deleted = false);

CREATE POLICY players_owner
    ON public."PLAYERS"
    AS PERMISSIVE
    FOR ALL
    TO app_users
    USING ((( SELECT "TEAMS".owner
   FROM "TEAMS"
  WHERE ("TEAMS".id = "PLAYERS".team_id)) = CURRENT_USER));

Test Case (Edited for better understanding)

Run this code using user bob:

INSERT INTO "TEAMS" (name, owner)
    VALUES ('Jerusalem Lions', 'bob')
    RETURNING id; -- We'll save this id for the next command

INSERT INTO "PLAYERS" (id, first_name, last_name, jersey_number, role, team_id)
    VALUES ('99999', 'Eric', 'Cohen', 29, 'linebacker', 888) -- Replace 888 with the returned id from the previous command
    RETURNING *;

-- These commands will work
SELECT * FROM "PLAYERS";

UPDATE "PLAYERS"
    SET last_name = 'Levi'
    WHERE id = '99999'
    RETURNING *; 

-- This is the command that won't work. I can't change the deleted. 
UPDATE "PLAYERS"
    SET deleted = true
    WHERE id = '99999'
    RETURNING *;

EDIT: Now, it's important to understand that The policies as defined above works when I do any query, as long as:

  1. INSERT INTO doesn't include deleted = true (that's ok).
  2. UPDATE that includes SET deleted = true. (This is the main issue).

I want to:

  1. Allow bob to delete a record using deleted = true on an UPDATE command.
  2. Hide in SELECT commands all records that deleted = true.

What should I do? ‍♂️

Bar Bokovza
  • 169
  • 1
  • 2
  • 12

3 Answers3

3

From the documentation:

When a mix of permissive and restrictive policies are present, a record is only accessible if at least one of the permissive policies passes, in addition to all the restrictive policies.

It means that you cannot permit in one policy something restricted in another policy. For this reason, restrictive policies should be used with extreme caution. When there is no policy defined, everything is restricted, so you should focus on permitting what should be allowed.

Simplified example:

create table my_table(
    id int primary key, 
    user_name text, 
    deleted bool);
    
alter table my_table enable row level security;

create policy rule_for_select
    on my_table
    as permissive
    for select
    to app_users
    using (not deleted);

create policy rule_for_all
    on my_table
    as permissive
    for all
    to app_users
    using (user_name = current_user and not deleted);

insert into my_table(id, user_name, deleted) values
(1, 'alice', false),
(2, 'bob', true),
(3, 'celine', true)

The user bob will see row 1. He would be able to update row 2 if deleted is false.

klin
  • 112,967
  • 15
  • 204
  • 232
  • Here's the problem: **Permissive** works like **OR** condition, and I don't have just `bob` as a user, I have `Alice` as well, and they are inherit their permission from a role named `app_users`. When I'm writing down those policies, I must to write `TO app_users` and not `TO bob`. – Bar Bokovza Oct 02 '21 at 12:57
  • 1
    There is no problem, as I have used `user_name = current_user`, the example will work for `app_users` as well (I've edited the answer respectively). You did not catch the main issue. You can (and should) define all you need only with permissive policies because you cannot *override* a restrictive policy. – klin Oct 02 '21 at 13:32
  • I've tried the combination of those 2 permissive policies. Bob is getting records with `deleted = true`. Doesn't achieve what I'm searching for. I don't want to get records with `deleted = true`, but I do want to allow the user bob (or any user in `app_users`) to update the value of the field `deleted`. – Bar Bokovza Oct 02 '21 at 18:34
  • Ok, just add the condition to the policy like in the updated answer. – klin Oct 02 '21 at 18:44
  • Doesn't work either – Bar Bokovza Oct 02 '21 at 18:51
  • If you want the user to be able to switch `deleted` from true to false, create a policy for `update` instead of for `all` with the condition `(user_name = current_user)` – klin Oct 02 '21 at 19:17
  • Tried it - Still doesn't work. That's messed up. – Bar Bokovza Oct 02 '21 at 19:31
  • 1
    As you may have noticed, playing "this doesn't work" and solving puzzles don't work well. Ask a new question with the simplest possible example and describe exactly what you want to achieve. – klin Oct 02 '21 at 19:48
0

What I can gather is, that

UPDATE "PLAYERS" SET deleted=true

will not fail for "Bob". But:

UPDATE "PLAYERS" SET deleted=true RETURNING *

will. Postgraphile will usually "return" something when doing mutations. After updating the row - you basically forbid the select statement.

The usual hack (feature) in Postgraphile land is usually creating a custom mutation and define it as security definer.

Maybe there are other ways around this - but this is the hammer->nail approach in Postgraphile, in my experience.

madflow
  • 7,718
  • 3
  • 39
  • 54
  • 1
    1. That `SELECT` after the mutation, because of how Postgrapile works, it's a good point, and I'll be noticed by that. 2. When I'm doing `UPDATE public."PLAYERS" SET deleted = true WHERE id = '11111';` in the Postgres Shell, it fails, so the bug is not related just for Postgraphile, it's also on the SQL itself. – Bar Bokovza Oct 02 '21 at 08:59
  • That's correct @BarBokovza . I've encountered this and asked a question about it. I now think that creating this additional function with security definer to do the update, is the solution to the problem I stated. https://stackoverflow.com/questions/74862224/why-is-the-policys-using-clause-used-for-the-new-row-while-a-with-check-cla – Christiaan Westerbeek Dec 20 '22 at 12:30
-1

Similar problem can be seen here: Unable to update row of table with Row Level Security where I proposed a workaround by implementing a grace period:

  • instead of deleted have a deleted_at column and on deletion store the current timestamp in it (i.e. timestamp when the deletion happened)
  • in the SELECT policy check if deleted_at is NULL or the time between deleted_at and now is less than 1 second
  USING (
        teams_deleted.deleted_at IS NULL
        OR
        ABS(EXTRACT(EPOCH FROM (now() - teams_deleted.deleted_at))) < 1
  )
MK2k
  • 54
  • 5
  • 1
    While this link may answer the question, it is better to include the essential parts of the answer here and provide the link for reference. Link-only answers can become invalid if the linked page changes. - [From Review](/review/low-quality-posts/34559357) – Kristian Jun 21 '23 at 13:38
  • 1
    Thanks @Kristian, I edited my answer. – MK2k Jun 21 '23 at 21:31