Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jwt claim parsing misbehavior #1642

Closed
Iced-Sun opened this issue Nov 2, 2020 · 9 comments
Closed

jwt claim parsing misbehavior #1642

Iced-Sun opened this issue Nov 2, 2020 · 9 comments

Comments

@Iced-Sun
Copy link

Iced-Sun commented Nov 2, 2020

Environment

  • PostgreSQL version: 12 (although should be not relevant)
  • PostgREST version: 7.0.1
  • Operating system: Ubuntu 20.04 LTS

Description of issue

It seems that postgrest would cache the jwt claim keys and generate an empty string for a claim key that does not actually exist in following requests. Here are the steps to reproduce:

  1. Issue two JWTs with at least one different key, e.g. jwt_1 = {"role": "user"} and jwt_1 = {"role": "user", "id": "whateverthevalueis"}.
  2. Send a request with jwt_1, the current_setting('jwt.request.claim.id', true) is <NULL>, which is totally correct.
  3. Send a request with jwt_2, the current_setting('jwt.request.claim.id', true) is 'whateverthevalueis', which is right again.
  4. Send a request with jwt_1, the current_setting('jwt.request.claim.id', true) now becomes '', which breaks the code.
  5. Repeating 4, current_setting('jwt.request.claim.id', true) is always '', until certain time elapses and it comes back to <NULL>.

The misbehavior stops some logics depending on the existense of a particular claim key. We can surely get around it by using nullif(supposed_to_be_null_claim_value,''), but I wonder if a quick fix could be done.

Regards.

@wolfgangwalther
Copy link
Member

wolfgangwalther commented Nov 2, 2020

This seems to be a postgres thing. I get the same behaviour on PG 12.4 with:

SELECT current_setting('x.y', true); -- returns NULL
BEGIN;
SET LOCAL x.y='z';
COMMIT;
SELECT current_setting('x.y', true); -- returns ''

So the local setting is not reverted completely after the transaction finishes.

@steve-chavez
Copy link
Member

Can confirm. I've also bumped into this bug before:

https://gist.github.com/steve-chavez/8d7033ea5655096903f3b52f8ed09a15

@wolfgangwalther
Copy link
Member

This has already been brought up multiple times in the mailing lists:
https://www.postgresql.org/message-id/flat/CA%2BQ86ij0KDCB0G45G509-8q0DNR611gcKG-sSM83GA1EBL7boA%40mail.gmail.com

https://www.postgresql.org/message-id/flat/CAB_pDVVa84w7hXhzvyuMTb8f5kKV3bee_p9QTZZ58Rg7zYM7sw%40mail.gmail.com

It seems like it is not considered a bug, but expected behaviour.

@wolfgangwalther
Copy link
Member

The misbehavior stops some logics depending on the existense of a particular claim key. We can surely get around it by using nullif(supposed_to_be_null_claim_value,''), but I wonder if a quick fix could be done.

Instead of wrapping all those calls in nullif, you could create your own wrapper function for current_setting like this:

CREATE FUNCTION my_current_setting(text) RETURNS text
LANGUAGE SQL AS $$
  SELECT nullif(current_setting($1, true), '')
$$;

Should be inlined anyway and saves you all the , true as well ;).

@Iced-Sun
Copy link
Author

Iced-Sun commented Nov 2, 2020

Thanks for the clarification and tips.

Will wait for the user variable mechanism of postgresql. Until then, I'll just take empty strings and NULLs as equals for GUC.

@steve-chavez
Copy link
Member

Wrapper functions are a must for current_setting.

We should add a note on the docs: PostgREST/postgrest-docs#362

@elimisteve
Copy link
Contributor

CREATE FUNCTION my_current_setting(text) RETURNS text
LANGUAGE SQL AS $$
  SELECT nullif(current_setting($1, true), '')
$$;

Unfortunately this seems to result in errors like

function my_current_setting(unknown) does not exist

Is there a way to wrap current_setting without needing to do an explicit cast on the input string, like my_current_setting('request.jwt.claim.user_id'::text) ?

@wolfgangwalther
Copy link
Member

function my_current_setting(unknown) does not exist

This error just indicates that the function can not be found at all in your current search path. So you either didn't create it at all or in a different schema or ... idk!

@steve-chavez
Copy link
Member

@elimisteve Another options is to do it like this: https://github.com/steve-chavez/socnet/blob/master/schema/util.sql#L11-L14

function my_current_setting(unknown) does not exist

Maybe you created that function in a non-exposed(not in db-schema) schema. If that's the case, check db-extra-search-path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants