Cookies, custom function, session authenication, relation bugfix

[ruslantalpa]

• Request cookies available as claims under postgrest.claims.cookie.cookie_name
• RPC functions that return session_claims type can send 'Set-Cookie' header
• new --function command line parameter, the name of the function to run after all the claims are set and before the main query
• Fix bug in relation detection when selecting parents two levels up by using the name of the FK

All things above allow common session authentication method
and a gift (which is not 100% complete/correct, figure it out) :)

create type session_claims AS (
  name text,
  value text,
  "path" text,
  expires integer,
  max_age integer,
  domain text,
  http_only boolean,
  secure boolean
);

create function
login(email text, password text) returns api.session_claims
volatile security definer
language plpgsql
as $$
declare
    usr data.users;
    eml text;
    pass text;
    sess data.sessions;
begin
    eml := email;
    pass := password;

    select * into usr
    from data.users as u
    where u.email = eml and u.password = pass;
    if not found then
        raise exception 'invalid email/password';
    else
        insert into data.sessions (user_id, company_id)
        values (usr.id, usr.company_id)
        returning * into sess;
        return (
            'SESSIONID'::text, -- cookie_name
            sess.session_id::text, -- cookie_value
            null::text, -- path
            null::integer, -- expires
            3600::integer, -- max_age
            null::text, -- domain
            null::boolean, -- http_only
            null::boolean -- secure
        );
    end if;
end
$$;



set search_path to public;

create or replace function get_user_id(sess_id text) returns int as $$
    select user_id from data.sessions where session_id=sess_id::uuid;
$$ language sql stable security definer;

create or replace function get_user(usr_id int) returns record as $$
    select id as user_id, company_id, user_type::text as user_role from data.users where id=usr_id;
$$ language sql stable security definer;


create or replace function check_session_id() returns void as $$
declare
    sess_id text;
    usr_id int;
    usr record;
begin
    sess_id = nullif(current_setting('postgrest.claims.cookie.SESSIONID'),'');
    if sess_id <> '' then
        select get_user_id(sess_id) into usr_id;
        if found then
            select * from get_user(usr_id) as (user_id int, company_id int, user_role text) into usr;
            if found then
                execute 'set local postgrest.claims.user_id = ' || quote_literal(usr.user_id);
                execute 'set local postgrest.claims.company_id = ' || quote_literal(usr.company_id);
                execute 'set local role to ' || quote_ident(usr.user_role);
            end if;
        end if;
    end if;
end; $$ language plpgsql volatile;

[begriffs]

Very nice, makes functions a lot more powerful. I had some ideas about the high level design:

• Request cookies are available in postgrest.claims.cookie.cookie_name but what about postgrest.cookies.cookie_name instead since it's not coming in as a JWT claim like the other claims.

• Is --function made for role switching as discussed in Add ability to provide jwt_claims verification. #632? What about calling the argument --pre-query-function to make it clearer?

• Instead of a special session_claims type for response cookies what if we began to use OUT parameters? When a function has only one OUT parameter (which is what happens by default for ordinary functions that say function foo() returns bar then postgrest would interpret the return value as the response body. Thus existing RPC calls will behave as they currently do. However we could specially interpret multiple OUT params with predefined names:

  create function login(email text, password text, OUT cookies json)
  volatile security definer
  language plpgsql
  as $$ # etc etc

We could have these OUT names actually

• body, a text value which is the response body
• headers, a json object whose names and values will become response headers (note this actually takes care of cookies as well)
• status, an int for the http response status code

This would make postgres functions pretty much general purpose http handlers.

[diogob]

Is there any difference between Just [] and Nothing ?

It might be easier to do something like:

iCookies = 
  case lookupHeader "Cookie" of 
    Just hdr -> parseCookiesText hdr
    Nothing -> []

In this case we could drop the fromMaybe in the cookieClaims definition inside the middleware.

[ruslantalpa]

yes, this would work

[ruslantalpa]

@begriffs
namespace - yes
parameter name - yes
return type - let's first merge this then think about your idea later, i don't think OUT works like described, it's a convenience syntax for returning a RECORD

[begriffs]

I hesitate to merge as-is because this is a partial solution to a more important problem: accessing request headers and setting response headers. Cookies are just one of many headers and I'd rather not build in a special case for cookies.

Ultimately we can give stored procedures access to the full http request context and the ability to shape the response beyond just the body. With that in mind I modify my proposal:

Requests would set the variable postgrest.claims, and postgrest.headers, and pass function arguments as they currently do. The postgrest.headers variable would up to two-levels deep, with the second level being for semicolon separated key value pairs in header values. For instance here is how it would translate.

POST /rpc/foo HTTP/1.1
Host: www.example.org
Cookie: theme=light; sessionToken=abc123

This would set

set postgrest.headers.host = 'www.example.org';
set postgrest.headers.cookie.theme = 'light';
set postgrest.headers.cookie.sessionToken='abc123';

Postgrest would inspect the function output and do the following

1. If the result is a single column named after the function itself then postgrest would behave as it currently does. (It's a regular function using returns)
2. If the result recordset contains a column named body then postgrest will treat this as the response body. (This is when OUT parameters are used.) Additionally it will look for columns named
   • headers - expected to be JSON array of keys and values. Postgrest will set response headers given by the key/value pairs. It's an array because duplicate keys are sometimes required, such as having multiple Set-Cookie headers.
   • `status' - expected to be an integer. Postgrest will set the http response code

In order to extend stored procs to deal with cookies and other http things I think we ought to get it right. Merging a special case solution would actually be a step backward.

[ruslantalpa]

Can you try and write a few RPC and then use the current sql we run to execute them and provide here the function and the result of the call to explain how we differentiate between them?
give an example of (also remember there is no response with the function name as the column):

• a function returning a scalar
• a function returning a tuple (2 scalars)
• a function returning a single row from a table (get_project_by_id)
• a function returning multiple rows from a table
• a special function returning headers and body

I think you have not considered all the implications, to inspect the response body (to look for special case columns headers/body) it means we have to parse the json which means to support this functionality for a handful of functions in a system (login/logout), you would kill performance for 95% of the system (i am exaggerating with kill but it will certainly be an order of magnitude slower for responses above 1M and they are not uncommon).

[begriffs]

Good idea, let me try those examples and mess with the calling code to see how well my speculation actually holds up.

[ruslantalpa]

i agree with the general idea of having the ability from sql to return headers/body but i don't like at all having to parse the response in haskell to inspect the keys of the json

[diogob]

@begriffs what I don't like about adding more custom variables is that it becomes a bit harder to debug and we trust a state that could change after the function call.

We could change our magic type jwt_claims but still use another magic type for custom shaping of the HTTP response. My suggestion is to have 2 kinds of functions:

1. Functions that return any type other than http_response. These behave exactly like the functions we have today minus the jwt_claims functionality that would be completely erased. This keeps the solution mostyl backwards compatible and keeps a simple way of defining functions decoupled from the HTTP interface (which is often the most elegant thing to do).
2. Functions that return the type http_response defined as (headers json, body text, status integer). This functions can set whatever headers they please in their return value as a json object. PostgREST would trust that the content type is set properly and the body would be send directly to the response body, so any encoding must be done manually.

The above solution im my POV is a generalization of the current jwt_claims approach.

[ruslantalpa]

@nikita-volkov when we run a bunch of queries (for which we don't need the result) like

query1;query2;query3

is it the case that hasql splits them and executes them one after another or is it sent somehow as a single query/string to postgresql?
It looks like we are about to have a bunch of those queries and i was wandering if it's ok to do it as before or should we call some kind of rpc that takes in a single json parameter.
the actual queries are actually like this

set postgrest.headers.host = 'www.example.org';set postgrest.headers.cookie.theme = 'light';set postgrest.headers.cookie.sessionToken='abc123';

[nikita-volkov]

@ruslantalpa

is it the case that hasql splits them and executes them one after another or is it sent somehow as a single query/string to postgresql?

It's executed in a single interaction with the server. IOW, it's more efficient than executing each statement in isolation.

[begriffs]

@diogob a magic http_response type seems like a nice approach. It differs from jwt_claims in that the latter is simply a name (whatever its underlying type) which signals to encode the result as JWT. The http_response type on the other hand needs to have columns of certain names and types. I guess we could have postgrest attempt to look for the headers, body, and status columns in the results of functions which return a type named http_response. Postgrest would raise a 500 error if the columns were not present or had the wrong types.

Also I agree with you that the headers column should be JSON, but I don't think it should be a JSON object but rather a JSON array of name-value pairs (length 2 arrays). This is because to set more than one cookie in the response you need two headers having the same name, Set-Cookie. I don't think a JSON object allows duplicate values for the same name, but correct me if I'm wrong.

[diogob]

@begriffs but if the function returns http_response the presence of said columns and types is already enforced by the type. My proposal was precisely to avoid having to code something to inspect the results. Unless there is something I'm overlooking here...

[begriffs]

Assuming the type already exists in the db and has the right definition. Do we have postgrest look for it on startup in a special schema and create it if it does not exist?

[diogob]

I see your point now, jwt_claims was not tied to any specific implementation.
And yes, for the http_type approach to be effective we would need to check/create during the startup so we would not need to do it during request time.

[ruslantalpa]

right now you can use &select and filters on the results returned by a particular function.
How would the http_type work in this context, specifically the body part of the type?
It's either body is of type text so you lose the flexibility or somehow that part needs polymorphic (some type of trick that we should find to be able to do that).

Just to state the goal again, we want want RPC functions to work just like now (&select and filters) but in addition to that, be able to set headers and maybe response code

[diogob]

True, the http_type would create too many problems. Maybe the SET local is a better solution. But I'd go for the namespace postgrest.cookies.cookie_name as suggested by @begriffs previously. Besides, if we are going for this approach I don't see any problem integrating this PR as is making cookies a special case for now.

[ruslantalpa]

it's not about the data going in (set local cookies ...), it's about data coming out and being transformed into a header (Set-Cookie). It's a special case, just like jwt, although it's a speciall case enabling an important feature.
http_type can work but it needs thought, there's not an obvious path

[begriffs]

@ruslantalpa have you given this any more thought?

[ruslantalpa]

@begriffs haven't had time to think about this, a bit busy looking into Relay, i'll get back to this and give it a day or two when i'm done with my other tasks.

[begriffs]

I'm closing this PR because some of its functionality is now merged in a different way. Thanks for blazing the trail for that functionality!

Could you open a PR for the relation detection bug fix?

We can add the http status and header functionality after the 0.4 release because it doesn't introduce any breaking change. That'll give us time to get it just right.