Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

jian he <jian.universality@gmail.com>

View thread
From: jian he <jian.universality@gmail.com>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Antonin Houska <ah@cybertec.at>, Daniel Gustafsson <daniel@yesql.se>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-11-04T05:00:00Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. meson: Fix install-quiet after clean

  2. oauth: Run Autoconf tests with correct compiler flags

  3. Link libpq with libdl if the platform needs that.

  4. Doc: correct spelling of meson switch.

  5. oauth: Correct SSL dependency for libpq-oauth.a

  6. oauth: Fix Autoconf build on macOS

  7. oauth: Move the builtin flow into a separate module

  8. Remove a stray "pgrminclude" annotation

  9. oauth: Simplify copy of PGoauthBearerRequest

  10. oauth: Improve validator docs on interruptibility

  11. oauth: Disallow synchronous DNS in libcurl

  12. oauth: Fix postcondition for set_timer on macOS

  13. oauth: Use IPv4-only issuer in oauth_validator tests

  14. Work around OAuth/EVFILT_TIMER quirk on NetBSD.

  15. oauth: Fix incorrect const markers in struct

  16. Add missing entry to oauth_validator test .gitignore

  17. cirrus: Temporarily fix libcurl link error

  18. Add support for OAUTHBEARER SASL mechanism

  19. libpq: Handle asynchronous actions during SASL

  20. require_auth: prepare for multiple SASL mechanisms

  21. Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h

  22. Make SASL max message length configurable

  23. jsonapi: fully initialize dummy lexer

  24. common/jsonapi: support libpq as a client

  25. Remove fe_memutils from libpgcommon_shlib

  26. Revert ECPG's use of pnstrdup()

  27. Explicitly require password for SCRAM exchange

  28. Refactor SASL exchange to return tri-state status

Hi there.
zero knowledge of Oath, just reading through the v35-0001.
forgive me if my comments are naive.

+static int
+parse_interval(struct async_ctx *actx, const char *interval_str)
+{
+ double parsed;
+ int cnt;
+
+ /*
+ * The JSON lexer has already validated the number, which is stricter than
+ * the %f format, so we should be good to use sscanf().
+ */
+ cnt = sscanf(interval_str, "%lf", &parsed);
+
+ if (cnt != 1)
+ {
+ /*
+ * Either the lexer screwed up or our assumption above isn't true, and
+ * either way a developer needs to take a look.
+ */
+ Assert(cnt == 1);
+ return 1; /* don't fall through in release builds */
+ }
+
+ parsed = ceil(parsed);
+
+ if (parsed < 1)
+ return actx->debugging ? 0 : 1;
+
+ else if (INT_MAX <= parsed)
+ return INT_MAX;
+
+ return parsed;
+}
The above Assert looks very wrong to me.

we can also use PG_INT32_MAX, instead of INT_MAX
(generally i think PG_INT32_MAX looks more intuitive to me)


+/*
+ * The Device Authorization response, described by RFC 8628:
+ *
+ *     https://www.rfc-editor.org/rfc/rfc8628#section-3.2
+ */
+struct device_authz
+{
+ char   *device_code;
+ char   *user_code;
+ char   *verification_uri;
+ char   *interval_str;
+
+ /* Fields below are parsed from the corresponding string above. */
+ int interval;
+};

click through the link https://www.rfc-editor.org/rfc/rfc8628#section-3.2
it says
"
   expires_in
      REQUIRED.  The lifetime in seconds of the "device_code" and
      "user_code".
   interval
      OPTIONAL.  The minimum amount of time in seconds that the client
      SHOULD wait between polling requests to the token endpoint.  If no
      value is provided, clients MUST use 5 as the default.
"
these two fields seem to differ from struct device_authz.