PATCH call permission denied when table user do not have SELECT rights, and a potential bug

[LMJW]

Environment

• PostgreSQL version: (postgresql docker 11 image, lastest tag)
• PostgREST version: (PostgREST 6.0.0 (dd86fe3))
• Operating system: (windows WSL ubuntu 18.04 LTS)

Description of issue

actual behavior: Use PATCH call fails without Prefer header (permission denied) to update table even when UPDATA right is granted. When Prefer header is added, PATCH call returns 404 while the table is updated.

Expected behavior :

• 404 should not be returned if the table is updated.
• Without Prefer header, the PATCH call will return permission denied response. (Not sure this is expected behavior or not.)

How to reproduce

1. run the following SQL script in the postgres database to set up the basic schema and roles.

NOTE: There are table drop & role drop in the script. check before you run

-- setup database
drop schema if exists test_issue_api cascade;
drop role if exists anno_user;

create role anno_user;
create schema test_issue_api;

grant usage on schema test_issue_api to anno_user;

create table if not exists test_issue_api.users (
    id serial not null,
    email character varying(4096) unique not null,
    password character varying(1024) not null
);

alter table test_issue_api.users enable row level security;

create policy anno_user_can_insert on test_issue_api.users
    as permissive for insert to anno_user with check(true);

create policy anno_user_can_update on test_issue_api.users
    as permissive
    for update
    to anno_user
    using (true)
    with check(true);

create policy anno_user_can_select on test_issue_api.users
    as permissive
    for select
    to anno_user
    using (true);

revoke all on table test_issue_api.users from public;

grant select(id, email) on table test_issue_api.users to anno_user;
grant update(email) on table test_issue_api.users to anno_user;
grant insert on table test_issue_api.users to anno_user;
grant usage on sequence test_issue_api.users_id_seq to anno_user;

-- insert data

set role anno_user;

select id, email from test_issue_api.users;
-- should be successful but return nothing

insert into test_issue_api.users (email, "password") values('test@123.com','password');
-- should be successful

select id, email from test_issue_api.users;
-- should be successful and return 1 row with id=1 && email=test@123.com

update test_issue_api.users
set email='ab234.com' where id = 1;

select id, email from test_issue_api.users;
-- should be successful and return 1 row with id=1 && email=ab234.com

select * from test_issue_api.users where id=1;
-- should return permission deneied

2. use the postgrest configure file as below.
   change the connection string as you need. save as a conf file and run postgrest xxx.conf in terminal.

db-uri = "postgres://postgres:_change_me_to_your_password_@localhost:5432"
db-schema = "test_issue_api"
db-anon-role = "anno_user"
server-port = 3000

3. use the following postman script to test the restful api.

Run GET, POST request first. It works fine.

The PATCH users.email [successful] request is the one working without using Prefer:return=minimal header.

The PATCH users.email [BUGGY] request is the one that has a potential bug. This request contains the Prefer header as well as query string to do filtering, but it returns 404. However, it still updates the users table successfully. So I believe this is a bug

The PATCH without Prefer[Permission denied] is the request that does not have Prefer header, which returns the permission denied. I am not sure whether this is the expected behavior. But I am wondering if postgrest can figure this out automatically.

{
	"info": {
		"_postman_id": "89e0fb45-9e94-4c95-8b63-1f2476c6f274",
		"name": "postgrest",
		"description": "for postgrest issue",
		"schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json"
	},
	"item": [
		{
			"name": "GET users",
			"request": {
				"method": "GET",
				"header": [],
				"url": {
					"raw": ":3000/users?select=id,email",
					"host": [
						""
					],
					"port": "3000",
					"path": [
						"users"
					],
					"query": [
						{
							"key": "select",
							"value": "id,email"
						}
					]
				}
			},
			"response": []
		},
		{
			"name": "POST user",
			"request": {
				"method": "POST",
				"header": [
					{
						"key": "Content-Type",
						"name": "Content-Type",
						"value": "application/json",
						"type": "text"
					},
					{
						"key": "Prefer",
						"value": "return=minimal",
						"type": "text"
					}
				],
				"body": {
					"mode": "raw",
					"raw": "{\n\t\"email\":\"this@abac.com\",\n\t\"password\":\"password\"\n}"
				},
				"url": {
					"raw": ":3000/users",
					"host": [
						""
					],
					"port": "3000",
					"path": [
						"users"
					]
				}
			},
			"response": []
		},
		{
			"name": "PATCH users.email [successful]",
			"request": {
				"method": "PATCH",
				"header": [
					{
						"key": "Content-Type",
						"name": "Content-Type",
						"value": "application/json",
						"type": "text"
					}
				],
				"body": {
					"mode": "raw",
					"raw": "{\n\t\"email\":\"is@bug.huiasdfhuiasduhf\"\n}"
				},
				"url": {
					"raw": ":3000/users?select=id&id=eq.1",
					"host": [
						""
					],
					"port": "3000",
					"path": [
						"users"
					],
					"query": [
						{
							"key": "select",
							"value": "id"
						},
						{
							"key": "id",
							"value": "eq.1"
						}
					]
				}
			},
			"response": []
		},
		{
			"name": "PATCH users.email[BUGGY?]",
			"request": {
				"method": "PATCH",
				"header": [
					{
						"key": "Prefer",
						"value": "return=minimal",
						"type": "text"
					},
					{
						"key": "Content-Type",
						"name": "Content-Type",
						"value": "application/json",
						"type": "text"
					}
				],
				"body": {
					"mode": "raw",
					"raw": "{\n\t\"email\":\"is.this@a.bug\"\n}"
				},
				"url": {
					"raw": ":3000/users?id=eq.1",
					"host": [
						""
					],
					"port": "3000",
					"path": [
						"users"
					],
					"query": [
						{
							"key": "id",
							"value": "eq.1"
						}
					]
				}
			},
			"response": []
		},
		{
			"name": "PATCH without Prefer[Permission denied]",
			"request": {
				"method": "PATCH",
				"header": [
					{
						"key": "Content-Type",
						"name": "Content-Type",
						"value": "application/json",
						"type": "text"
					}
				],
				"body": {
					"mode": "raw",
					"raw": "{\n\t\"email\":\"this.can@not.update\"\n}"
				},
				"url": {
					"raw": ":3000/users?id=eq.1",
					"host": [
						""
					],
					"port": "3000",
					"path": [
						"users"
					],
					"query": [
						{
							"key": "id",
							"value": "eq.1"
						}
					]
				}
			},
			"response": []
		}
	],
	"event": [
		{
			"listen": "prerequest",
			"script": {
				"id": "9a7baf4b-b7ee-4177-8236-bf8a5ad64558",
				"type": "text/javascript",
				"exec": [
					""
				]
			}
		},
		{
			"listen": "test",
			"script": {
				"id": "5aed87d6-6b4f-414d-bea2-e6409a8fb761",
				"type": "text/javascript",
				"exec": [
					""
				]
			}
		}
	]
}

[steve-chavez]

Let's focus first on the PATCH without Prefer and select, this does seem to be a bug.

This is the query that pgrst generates in that case:

WITH
pgrst_payload AS (SELECT $1::json AS json_data), 
pgrst_body AS ( SELECT CASE WHEN json_typeof(json_data) = 'array' THEN json_data ELSE json_build_array(json_data) END AS val FROM pgrst_payload) 
UPDATE "test"."users" 
SET "email" = _."email" FROM (SELECT * FROM json_populate_recordset (null:: "test"."users" , (SELECT val FROM pgrst_body))) _  
WHERE  "test"."users"."id" = '1'::unknown 
RETURNING "test"."users".*;

The RETURNING * at the last part seems to be the culprit of the error.

(I recalled this bug was fixed before, but it was related to INSERTs #417, we need to do the same for UPDATE now)

[steve-chavez]

Actually with POST(INSERT) the semantics are the same, it needs Prefer: return=minimal to succeed.
Though it does result in 201 Created and PATCH returns 404(I see your point now).

Since by default POST/PATCH return nothing is strange to me why this choice was made, I think it should work without Prefer: return=minimal. I'll have to think this through, maybe it can be solved with a simple RETURNING 1.