Trying to Right an AND-ed SQL Expression

Question

I'm trying to create a SQL expression for an INSERT policy for a resource_authors table (a JOIN table linking resources and users):

CREATE POLICY insert_resources_authors ON public.resource_authors
    FOR INSERT TO public_user
    WITH CHECK (*some expression*)

What I'd like to say is "the user must be logged in and there must not be an existing author record for that resource yet". I can express the first part using a custom current_user_id function I have:

current_user_id() IS NOT NULL

And I can get the second with:

SELECT count(user_id) > 0 
FROM resource_authors 
WHERE resource_id = resource_id

... but I can't figure out how to combine them. Can anyone clue me in to how I can select both, as a single boolean?

I am not sure I understand... In what circumstance would a user be able to insert anything without being logged in? In regard to your second condition, of course `resource_id = resource_id` for all the records, that is tautological. You need a `PRIMARY KEY` or `UNIQUE` constraint on your table for that, not a policy. — Atmo, Feb 04 '23 at 22:08
1) I'm using Postgraphile, which sets a setting with the logged in user's ID based on a bearer token provided in the HTTP request. This function checks that setting (along with a `NULLIF` to make it null if not set). 2) Inside a policy, I thought the expression had access to the record being INSERT-ed ... and I thought the second `resource_id` was therefore referring to the `resource_id` of the INSERT-ed record. — machineghost, Feb 04 '23 at 22:13
@machineghost No, `resource_id` is indeed ambiguous. You'll need to qualify it by adding the table name. Can you [edit] your question to show the full SQL of the policy, please? — Bergi, Feb 04 '23 at 22:21
Well, I'm trying to write the policy now, so it doesn't exist yet. But I edited with the `CREATE POLICY` SQL. As for qualifying, I don't quite understand ... what would be the table name of the INSERT-ed record? — machineghost, Feb 04 '23 at 22:24
Then write a sample of a table (2-3 records) and a few `INSERT` queries that either are fine or are blocked with the explanation as why they are fine/blocked. — Atmo, Feb 04 '23 at 22:27

Bergi · Accepted Answer · 2023-02-04T22:41:59.907

1

Literally place an AND between them (and wrap them in parenthesis to make clear they're subqueries):

(current_user_id() IS NOT NULL)
AND
NOT (SELECT count(user_id) > 0 
  FROM resource_authors 
  WHERE resource_id = resource_id)

I'd write

CREATE POLICY insert_resources_authors ON public.resource_authors
    FOR INSERT TO public_user
    WITH CHECK (
        current_user_id() IS NOT NULL
        AND
        NOT EXISTS (SELECT *
            FROM resource_authors ra
            WHERE ra.resource_id = resource_authors.resource_id
        )
    );

However I find it questionable to allow anyone to make themselves author of any authorless resource. I'd rather record the author via a trigger when they create the resource.

edited Feb 04 '23 at 22:41

answered Feb 04 '23 at 22:20

Bergi

630,263
148
957
1,375

And that's a valid expression (with no `SELECT` in front)? I'm still new to what is/isn't a valid expression in SQL. – machineghost Feb 04 '23 at 22:27
Yes, that's a valid *expression*. It's not a valid full statement that you could run, though. – Bergi Feb 04 '23 at 22:34
However, your policy might have a different problem: the policy for `INSERT` depends on the visibility of rows in the same table, which might be restricted by a `SELECT` policy. See https://stackoverflow.com/questions/72369134/infinite-recursion-rls and https://www.postgresql.org/message-id/flat/CALgFm5hwf=HRFCcs7ZUyB6oNpmRRthVH0YmFm=Z3HxvYxCCFEQ@mail.gmail.com – Bergi Feb 04 '23 at 22:38
`current_user_id() IS NOT NULL` is to test someone is connected; it is not about anyone making themselves author of anything. It would have made more sense to me, had it been the case. – Atmo Feb 04 '23 at 23:17
@Atmo But the "*there must not be an existing author record for that resource yet*" part is – Bergi Feb 04 '23 at 23:18
... which like I said should be a `UNIQUE` or `PRIMARY KEY` constraint. I do not think a policy has any benefit over the constraints. Does it? – Atmo Feb 04 '23 at 23:58
@Atmo If there is only/at most a single author for a resource, yes, a unique constraint should be used to model that. But if a resource can have multiple authors, this doesn't work; my *guess* was that the OP wants the prevent anyone from adding themselves to a non-empty author list of a resource. I could be wrong though, we'll need clarification from the OP on that matter. – Bergi Feb 05 '23 at 00:02
You guessed correctly. The idea I was shooting for was "if we just made this resource, let the person declare themselves to be an author ... but if there's already an author for this resource, don't let someone else make themselves an author". – machineghost Feb 05 '23 at 00:33
I see. Actually, I thought about another possible benefit: if you want to have an admin who can bypass it. – Atmo Feb 05 '23 at 00:34
1

@machineghost, in that case, I think the condition you need is not `current_user_id() IS NOT NULL`, but rather `resource_authors.user_id = current_user_id()` – Atmo Feb 05 '23 at 00:35
@machineghost Will you allow author-less resources at all? – Bergi Feb 05 '23 at 00:36
Only temporarily, because the resource record has to exist (for at least a millisecond) before I can add the `resource_author` record. And yes @atmo, that sounds better ... I'm still very new to all this. – machineghost Feb 05 '23 at 00:36
Yeah, in that case I'd definitely recommend to use a trigger (or function that inserts both the resource and its authorship) with `SECURITY DEFINER` to insert these – Bergi Feb 05 '23 at 00:38
... and I'm sorry to insist but from your description, resources only have 1 author (the person who created it) each, right? – Atmo Feb 05 '23 at 00:39
No, I want to allow authors to add "co-authors" later on. Having a single `createdBy` author would have been simpler, but wouldn't have allowed co-authors :( As for a trigger ... I'll look into it (more crazy SQL stuff to learn!) Might just go with a function, either way thanks for the suggestion. – machineghost Feb 05 '23 at 00:41
@machineghost You might still use the simple approach if you distinguish (primary) authors from co-authors – Bergi Feb 05 '23 at 00:43
Thanks, but I think that would ultimately make more work, as then I'd have to check both a `created_by` and a `SELECT FROM resource_authors` in deciding who can mess with a record. – machineghost Feb 05 '23 at 00:44
I leave it to @Bergi if he wants. On my end, I have determined I will not be able to come up with the cleanest possible solution unless given exactly what you are trying to achieve (describing a policy is about the *how*, not the *what*). – Atmo Feb 05 '23 at 00:51
1

For starters, I would insert records in the `resource` and `resource_authors` tables as 1 query (not even allowing 1ms before the author is inserted). With the description in your question, I thought you were talking about inserting an authors long after the resource was created, not just in the next query. This sort of details can change the approach a lot. – Atmo Feb 05 '23 at 00:55
I'm using Postgraphile, so I can't do both INSERTs as one query without making a custom endpoint, or using a custom function or trigger. But in all those cases, wouldn't the policy rules still apply? If I have a custom function (or trigger, or application logic) create both records, one still has to be created first, no? – machineghost Feb 05 '23 at 01:21
@machineghost If you don't want resources without authors, you should definitely not expose a mutation that creates only a resource row. Indeed, use a custom mutation resolver, a custom function or a trigger. For the latter two, the policy may not apply if you use `SECURITY DEFINER`. And yes, one row has to be inserted first (probably the resource one since that creates the id to reference from the authorship row), but the database should prevent finishing the transaction without also inserting the second row. – Bergi Feb 05 '23 at 01:46
Well, I *was* trying to use https://github.com/mlipscombe/postgraphile-plugin-nested-mutations ... but it's a mess, so now I'm just debating whether to do the function or the custom mutation. My bias is to go back to application logic (where I'm comfortable), but a function seems better so I'm going to *try* to leave my comfort zone and do it that way. – machineghost Feb 05 '23 at 02:27
@machineghost I would advise against leaving this to the client, hoping that they would always create the authorship info with the resource. Whether you think of this as an application constraint or a database (integrity) constraint is up to you, and depends on your system architecture what is more appropriate. Doing this in the nodejs code is simpler and more flexible, doing it in the database is harder to change (migrate) but protects better against (your own) mistakes and other systems that might directly connect to the db. (Personally I mostly went with the pragmatic option) – Bergi Feb 05 '23 at 04:15

Trying to Right an AND-ed SQL Expression

1 Answers1