1: 3b27f54b4dc ! 1: 8312bf30726 libpq-fe.h: Don't claim SOCKTYPE in the global namespace @@ Commit message doesn't seem too far-fetched, given its proximity to existing POSIX macro names. - Add a PQ_ prefix to avoid collisions, and backpatch. + Add a PQ_ prefix to avoid collisions, update and improve the surrounding + documentation, and backpatch. + Reviewed-by: Chao Li + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com Backpatch-through: 18 ## doc/src/sgml/libpq.sgml ## @@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest - /* Callback implementing a custom asynchronous OAuth flow. */ + + /* Hook outputs */ + +- /* Callback implementing a custom asynchronous OAuth flow. */ ++ /* ++ * Callback implementing a custom asynchronous OAuth flow. The signature is ++ * platform-dependent: PQ_SOCKTYPE is SOCKET on Windows, and int everywhere ++ * else. ++ */ PostgresPollingStatusType (*async) (PGconn *conn, struct PGoauthBearerRequest *request, - SOCKTYPE *altsock); @@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest /* Callback to clean up custom allocations. */ void (*cleanup) (PGconn *conn, struct PGoauthBearerRequest *request); +@@ doc/src/sgml/libpq.sgml: typedef struct PGoauthBearerRequest + hook. When the callback cannot make further progress without blocking, + it should return either PGRES_POLLING_READING or + PGRES_POLLING_WRITING after setting +- *pgsocket to the file descriptor that will be marked ++ *altsock to the file descriptor that will be marked + ready to read/write when progress can be made again. (This descriptor + is then provided to the top-level polling loop via + PQsocket().) Return PGRES_POLLING_OK ## src/interfaces/libpq/libpq-fe.h ## @@ src/interfaces/libpq/libpq-fe.h: typedef struct _PGpromptOAuthDevice + int expires_in; /* seconds until user code expires */ + } PGpromptOAuthDevice; - /* for PGoauthBearerRequest.async() */ +-/* for PGoauthBearerRequest.async() */ ++/* ++ * For PGoauthBearerRequest.async(). This macro just allows clients to avoid ++ * depending on libpq-int.h or Winsock for the "socket" type; it's undefined ++ * immediately below. ++ */ #ifdef _WIN32 -#define SOCKTYPE uintptr_t /* avoids depending on winsock2.h for SOCKET */ +#define PQ_SOCKTYPE uintptr_t /* avoids depending on winsock2.h for SOCKET */ @@ src/interfaces/libpq/libpq-fe.h: typedef struct _PGpromptOAuthDevice typedef struct PGoauthBearerRequest @@ src/interfaces/libpq/libpq-fe.h: typedef struct PGoauthBearerRequest + * blocking during the original call to the PQAUTHDATA_OAUTH_BEARER_TOKEN + * hook, it may be returned directly, but one of request->async or + * request->token must be set by the hook. ++ * ++ * The (PQ_SOCKTYPE *) in the signature is a placeholder for the platform's ++ * native socket type: (SOCKET *) on Windows, and (int *) everywhere else. */ PostgresPollingStatusType (*async) (PGconn *conn, struct PGoauthBearerRequest *request, 2: 3b5690a49d7 ! 2: 899597d5777 libpq-oauth: use correct c_args in meson.build @@ Commit message libpq_so_c_args, rather than libpq_oauth_so_c_args. (At the moment, the two lists are identical, but that won't be true forever.) + Reviewed-by: Chao Li + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com Backpatch-through: 18 ## src/interfaces/libpq-oauth/meson.build ## 3: 4933e9b53fb ! 3: 953c743f692 oauth_validator: Avoid races in log_check() @@ Commit message Commit e0f373ee4 fixed up races in Cluster::connect_fails when using log_like. t/002_client.pl didn't get the memo, though, because it - doesn't use Test::Cluster to perform its custom hook tests. Introduce - the fix, based on debug2 logging, to its use of log_check() as well, and - move the logic into the test() helper so that any additions don't need - to continually duplicate it. + doesn't use Test::Cluster to perform its custom hook tests. (This is + probably not an issue at the moment, since the log check is only done + after authentication success and not failure, but there's no reason to + wait for someone to hit it.) + Introduce the fix, based on debug2 logging, to its use of log_check() as + well, and move the logic into the test() helper so that any additions + don't need to continually duplicate it. + + Reviewed-by: Chao Li + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com Backpatch-through: 18 ## src/test/modules/oauth_validator/t/002_client.pl ## 4: b2cece52ba6 ! 4: 3c047e6cf24 libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 @@ Commit message TODO: Could we just add to the end of PGoauthBearerRequest, and tell users not to use the additional fields if they have version 1? + Reviewed-by: Chao Li + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com + ## doc/src/sgml/libpq.sgml ## @@ doc/src/sgml/libpq.sgml: PQauthDataHook_type PQgetAuthDataHook(void); PQAUTHDATA_PROMPT_OAUTH_DEVICE @@ src/interfaces/libpq/libpq-fe.h: typedef enum * URL */ - PQAUTHDATA_OAUTH_BEARER_TOKEN, /* server requests an OAuth Bearer token */ + PQAUTHDATA_OAUTH_BEARER_TOKEN, /* server requests an OAuth Bearer token -+ * (prefer v2, below, instead) */ ++ * (v2 is preferred; see below) */ + PQAUTHDATA_OAUTH_BEARER_TOKEN_V2, /* newest API for OAuth Bearer tokens */ } PGauthData; @@ src/interfaces/libpq/fe-auth-oauth.c: cleanup: } +/* -+ * Helper for handling user flow failures. If the implementation put anything -+ * into request->error, it's added to conn->errorMessage here. ++ * Helper for handling flow failures. If anything was put into request->error, ++ * it's added to conn->errorMessage here. + */ +static void +report_user_flow_error(PGconn *conn, const PGoauthBearerRequestV2 *request) @@ src/test/modules/oauth_validator/t/002_client.pl: test( + expect_success => 1, log_like => [qr/oauth_validator: token="my-token", role="$user"/]); -+# Make sure the v1 hook continues to work. */ ++# Make sure the v1 hook continues to work. +test( + "v1 synchronous hook can provide a token", + flags => [ 5: c012bc715e1 ! 5: e655d4ee091 libpq-oauth: Use the PGoauthBearerRequestV2 API @@ Commit message to the private NLS initialization. Should we expose the lock to that function as well? + Reviewed-by: Chao Li + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com + ## src/interfaces/libpq-oauth/README ## @@ src/interfaces/libpq-oauth/README: results in a failed connection. @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy + * also the documentation for struct async_ctx. + */ + if (actx->errctx) -+ appendPQExpBuffer(errbuf, "%s: ", libpq_gettext(actx->errctx)); ++ appendPQExpBuffer(errbuf, "%s: ", actx->errctx); + + if (PQExpBufferDataBroken(actx->errbuf)) + appendPQExpBufferStr(errbuf, libpq_gettext("out of memory")); @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn) @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn) { case OAUTH_STEP_INIT: - actx->errctx = "failed to fetch OpenID discovery document"; + actx->errctx = libpq_gettext("failed to fetch OpenID discovery document"); - if (!start_discovery(actx, conn_oauth_discovery_uri(conn))) + if (!start_discovery(actx, actx->discovery_uri)) goto error_return; @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn) - * also the documentation for struct async_ctx. - */ - if (actx->errctx) -- appendPQExpBuffer(errbuf, "%s: ", libpq_gettext(actx->errctx)); +- appendPQExpBuffer(errbuf, "%s: ", actx->errctx); - - if (PQExpBufferDataBroken(actx->errbuf)) - appendPQExpBufferStr(errbuf, libpq_gettext("out of memory")); @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_init(PGconn *conn, const char *passw static void oauth_free(void *opaq) @@ src/interfaces/libpq/fe-auth-oauth.c: cleanup: - } - - /* -- * Helper for handling user flow failures. If the implementation put anything -- * into request->error, it's added to conn->errorMessage here. -+ * Helper for handling flow failures. If the implementation put anything into -+ * request->error, it's added to conn->errorMessage here. + * it's added to conn->errorMessage here. */ static void -report_user_flow_error(PGconn *conn, const PGoauthBearerRequestV2 *request) 6: dd491541a01 ! 6: 41132991bb4 libpq-oauth: Never link against libpq's encoding functions @@ Commit message dependency on the exported APIs will simply fail to link the shared module. + Reviewed-by: Chao Li + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com + ## src/interfaces/libpq-oauth/meson.build ## @@ src/interfaces/libpq-oauth/meson.build: libpq_oauth_sources = files( libpq_oauth_so_sources = files( 7: eda0c6f9059 ! 7: e20b555aba5 WIP: Introduce third-party OAuth flow plugins? @@ Commit message new vulnerabilities TODO: how hard would it be to support Windows here? + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com + ## src/interfaces/libpq/meson.build ## @@ src/interfaces/libpq/meson.build: pkgconfig.generate( install_headers( @@ src/test/modules/oauth_validator/t/002_client.pl: sub test test( @@ src/test/modules/oauth_validator/t/002_client.pl: test( - # Make sure the v1 hook continues to work. */ + # Make sure the v1 hook continues to work. test( "v1 synchronous hook can provide a token", + hook_only => 1, # plugins don't support API v1