public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: Tom Lane <[email protected]>
To: Dave Cramer <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Antonin Houska <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Date: Mon, 24 Feb 2025 10:00:25 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAOYmi+kHXMG2Z2KR7y36Cbsop_7t-YugrpfCnOXuXz8BpfaXaw@mail.gmail.com>
<[email protected]>
<CAOYmi+=ee8H=GaGNH_gZvBE+YPtmK9eqUdXqemk=zKDnO8YXRw@mail.gmail.com>
<CAOYmi+kLTJ1wZ6gxRbgtR52E=EyiCpmp6J3mmSvtc1a6i7sZ3Q@mail.gmail.com>
<CAOYmi+=ceXepXOrQXsxca-u57GxM+x_q34fpyNJ8PLmbUWPwEQ@mail.gmail.com>
<[email protected]>
<CAOYmi+=vJQ8EUiVf-iHYBwz4-tLHLrvp8rYDoNicB2yJrBKraw@mail.gmail.com>
<CAOYmi+=FzVg+C-pQHCwjW0qU-POHmzZaD2z3CdsACj==14H8kQ@mail.gmail.com>
<[email protected]>
<CAOYmi+kjPLfT7iFqNMVV44sAUJ9m10TfrSMRPUYU2OgH5j0AYg@mail.gmail.com>
<[email protected]>
<CAOYmi+kn0_FioLpDqctn97a_W8n90XLt_OmU1xL6ZdK8DP8ufA@mail.gmail.com>
<[email protected]>
<CAOYmi+mEkXu9Jfg9v-5z9DmBCRCMS2dt43Z=TCbgVkbrqJXxRg@mail.gmail.com>
<06102deb-00ef-431d-a3ea-65afb24! [email protected]>
<CAOYmi+kZAka0sdxCOBxsQc2ozEZGZKHWU_9nrPXg3sG1NJ-zJw@mail.gmail.com>
<[email protected]>
<CAOYmi+kQFrKGmC0=Um-QryU1fvCAOC2bpjcz-4rxgLrp69mSOg@mail.gmail.com>
<[email protected]>
<CAOYmi+=K3BBRXwvJpt5D_4nK_EimCJBj7a42maACszsQC3=t9w@mail.gmail.com>
<[email protected]>
<CAOYmi+kJqzo6XsR9TEhvVfeVNQ-TyFM5LATypm9yoQVYk=4Wrw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAOYmi+nHG7oy+ybHH72WjiXAQG3tE6v_at-K9ebRy2oqo92V+A@mail.gmail.com>
<[email protected]>
<CAOYmi+mFZsExAYWy-bS9=yki_fWC+MC0MufpEZ_SycrbCAJrEg@mail.gmail.com>
<[email protected]>
<CAOYmi+=zgJinCoqdQ70fvKmvyrLekd9-supkuj57Poa1FmX8bw@mail.gmail.com>
<[email protected]>
<CAOYmi+=1_AqPL0Y7qdqg1ER0-bBjCUKUZfrLcgob5bHjzSsvbA@mail.gmail.com>
<CAOYmi+nP8AM9xm+xUW5kDz7XDF7M! [email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
> On 24 Feb 2025, at 00:45, Daniel Gustafsson <[email protected]> wrote:
> The attached diff passes CI and works for me, will revisit in the morning.
Dave reported (on Discord) that the OPTIONAL macro collided with windef.h, so
attached is a small fix for that as well (even though we don't support Windows
here right now there is little point in clashing since we don't need that
particular macro name so rename anyways).
--
Daniel Gustafsson
Attachments:
[application/octet-stream] 0002-oauth-Rename-macro-to-avoid-collisions-on-Windows.patch (5.5K, ../[email protected]/2-0002-oauth-Rename-macro-to-avoid-collisions-on-Windows.patch)
download | inline diff:
From 9f13bd3e416263d7cc444245dc5f858478cc0562 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Mon, 24 Feb 2025 09:49:18 +0100
Subject: [PATCH 2/2] oauth: Rename macro to avoid collisions on Windows
Our json parsing defined the macros OPTIONAL and REQUIRED to decorate the
structs with for increased readability. This however collides with macros
in the <windef.h> header on Windows.
../src/interfaces/libpq/fe-auth-oauth-curl.c:398:9: warning: "OPTIONAL" redefined
398 | #define OPTIONAL false
| ^~~~~~~~
In file included from D:/a/_temp/msys64/ucrt64/include/windef.h:9,
from D:/a/_temp/msys64/ucrt64/include/windows.h:69,
from D:/a/_temp/msys64/ucrt64/include/winsock2.h:23,
from ../src/include/port/win32_port.h:60,
from ../src/include/port.h:24,
from ../src/include/c.h:1331,
from ../src/include/postgres_fe.h:28,
from ../src/interfaces/libpq/fe-auth-oauth-curl.c:16:
include/minwindef.h:65:9: note: this is the location of the previous definition
65 | #define OPTIONAL
| ^~~~~~~~
Rename to avoid compilation errors in anticipation of implementing
support for Windows.
Reported-by: Dave Cramer (on PostgreSQL Hacking Discord)
---
src/interfaces/libpq/fe-auth-oauth-curl.c | 34 +++++++++++------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c
index a80e2047bb7..ae339579f88 100644
--- a/src/interfaces/libpq/fe-auth-oauth-curl.c
+++ b/src/interfaces/libpq/fe-auth-oauth-curl.c
@@ -394,8 +394,8 @@ struct json_field
};
/* Documentation macros for json_field.required. */
-#define REQUIRED true
-#define OPTIONAL false
+#define PG_OAUTH_REQUIRED true
+#define PG_OAUTH_OPTIONAL false
/* Parse state for parse_oauth_json(). */
struct oauth_parse
@@ -844,8 +844,8 @@ static bool
parse_provider(struct async_ctx *actx, struct provider *provider)
{
struct json_field fields[] = {
- {"issuer", JSON_TOKEN_STRING, {&provider->issuer}, REQUIRED},
- {"token_endpoint", JSON_TOKEN_STRING, {&provider->token_endpoint}, REQUIRED},
+ {"issuer", JSON_TOKEN_STRING, {&provider->issuer}, PG_OAUTH_REQUIRED},
+ {"token_endpoint", JSON_TOKEN_STRING, {&provider->token_endpoint}, PG_OAUTH_REQUIRED},
/*----
* The following fields are technically REQUIRED, but we don't use
@@ -857,8 +857,8 @@ parse_provider(struct async_ctx *actx, struct provider *provider)
* - id_token_signing_alg_values_supported
*/
- {"device_authorization_endpoint", JSON_TOKEN_STRING, {&provider->device_authorization_endpoint}, OPTIONAL},
- {"grant_types_supported", JSON_TOKEN_ARRAY_START, {.array = &provider->grant_types_supported}, OPTIONAL},
+ {"device_authorization_endpoint", JSON_TOKEN_STRING, {&provider->device_authorization_endpoint}, PG_OAUTH_OPTIONAL},
+ {"grant_types_supported", JSON_TOKEN_ARRAY_START, {.array = &provider->grant_types_supported}, PG_OAUTH_OPTIONAL},
{0},
};
@@ -955,24 +955,24 @@ static bool
parse_device_authz(struct async_ctx *actx, struct device_authz *authz)
{
struct json_field fields[] = {
- {"device_code", JSON_TOKEN_STRING, {&authz->device_code}, REQUIRED},
- {"user_code", JSON_TOKEN_STRING, {&authz->user_code}, REQUIRED},
- {"verification_uri", JSON_TOKEN_STRING, {&authz->verification_uri}, REQUIRED},
- {"expires_in", JSON_TOKEN_NUMBER, {&authz->expires_in_str}, REQUIRED},
+ {"device_code", JSON_TOKEN_STRING, {&authz->device_code}, PG_OAUTH_REQUIRED},
+ {"user_code", JSON_TOKEN_STRING, {&authz->user_code}, PG_OAUTH_REQUIRED},
+ {"verification_uri", JSON_TOKEN_STRING, {&authz->verification_uri}, PG_OAUTH_REQUIRED},
+ {"expires_in", JSON_TOKEN_NUMBER, {&authz->expires_in_str}, PG_OAUTH_REQUIRED},
/*
* Some services (Google, Azure) spell verification_uri differently.
* We accept either.
*/
- {"verification_url", JSON_TOKEN_STRING, {&authz->verification_uri}, REQUIRED},
+ {"verification_url", JSON_TOKEN_STRING, {&authz->verification_uri}, PG_OAUTH_REQUIRED},
/*
* There is no evidence of verification_uri_complete being spelled
* with "url" instead with any service provider, so only support
* "uri".
*/
- {"verification_uri_complete", JSON_TOKEN_STRING, {&authz->verification_uri_complete}, OPTIONAL},
- {"interval", JSON_TOKEN_NUMBER, {&authz->interval_str}, OPTIONAL},
+ {"verification_uri_complete", JSON_TOKEN_STRING, {&authz->verification_uri_complete}, PG_OAUTH_OPTIONAL},
+ {"interval", JSON_TOKEN_NUMBER, {&authz->interval_str}, PG_OAUTH_OPTIONAL},
{0},
};
@@ -1010,9 +1010,9 @@ parse_token_error(struct async_ctx *actx, struct token_error *err)
{
bool result;
struct json_field fields[] = {
- {"error", JSON_TOKEN_STRING, {&err->error}, REQUIRED},
+ {"error", JSON_TOKEN_STRING, {&err->error}, PG_OAUTH_REQUIRED},
- {"error_description", JSON_TOKEN_STRING, {&err->error_description}, OPTIONAL},
+ {"error_description", JSON_TOKEN_STRING, {&err->error_description}, PG_OAUTH_OPTIONAL},
{0},
};
@@ -1069,8 +1069,8 @@ static bool
parse_access_token(struct async_ctx *actx, struct token *tok)
{
struct json_field fields[] = {
- {"access_token", JSON_TOKEN_STRING, {&tok->access_token}, REQUIRED},
- {"token_type", JSON_TOKEN_STRING, {&tok->token_type}, REQUIRED},
+ {"access_token", JSON_TOKEN_STRING, {&tok->access_token}, PG_OAUTH_REQUIRED},
+ {"token_type", JSON_TOKEN_STRING, {&tok->token_type}, PG_OAUTH_REQUIRED},
/*---
* We currently have no use for the following OPTIONAL fields:
--
2.39.3 (Apple Git-146)
[application/octet-stream] 0001-oauth-Fix-incorrect-const-markers-in-struct.patch (3.5K, ../[email protected]/3-0001-oauth-Fix-incorrect-const-markers-in-struct.patch)
download | inline diff:
From 577e8918c67dc66c0ec1686f92c85eb63c045bcc Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Mon, 24 Feb 2025 09:46:53 +0100
Subject: [PATCH 1/2] oauth: Fix incorrect const markers in struct
Two members in PGoauthBearerRequest were incorrectly marked as const.
While in there, align the name of the struct with the typedef as per
project style.
Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/libpq.sgml | 10 +++++-----
src/interfaces/libpq/libpq-fe.h | 10 +++++-----
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index ddb3596df83..8fa0515c6a0 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -10318,21 +10318,21 @@ typedef struct _PGpromptOAuthDevice
of <symbol>PGoauthBearerRequest</symbol>, which should be filled in
by the implementation:
<synopsis>
-typedef struct _PGoauthBearerRequest
+typedef struct PGoauthBearerRequest
{
/* Hook inputs (constant across all calls) */
- const char *const openid_configuration; /* OIDC discovery URL */
- const char *const scope; /* required scope(s), or NULL */
+ const char *openid_configuration; /* OIDC discovery URL */
+ const char *scope; /* required scope(s), or NULL */
/* Hook outputs */
/* Callback implementing a custom asynchronous OAuth flow. */
PostgresPollingStatusType (*async) (PGconn *conn,
- struct _PGoauthBearerRequest *request,
+ struct PGoauthBearerRequest *request,
SOCKTYPE *altsock);
/* Callback to clean up custom allocations. */
- void (*cleanup) (PGconn *conn, struct _PGoauthBearerRequest *request);
+ void (*cleanup) (PGconn *conn, struct PGoauthBearerRequest *request);
char *token; /* acquired Bearer token */
void *user; /* hook-defined allocated data */
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index b7399dee58e..34ddfdb1831 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -745,11 +745,11 @@ typedef struct _PGpromptOAuthDevice
#define SOCKTYPE int
#endif
-typedef struct _PGoauthBearerRequest
+typedef struct PGoauthBearerRequest
{
/* Hook inputs (constant across all calls) */
- const char *const openid_configuration; /* OIDC discovery URI */
- const char *const scope; /* required scope(s), or NULL */
+ const char *openid_configuration; /* OIDC discovery URI */
+ const char *scope; /* required scope(s), or NULL */
/* Hook outputs */
@@ -770,7 +770,7 @@ typedef struct _PGoauthBearerRequest
* request->token must be set by the hook.
*/
PostgresPollingStatusType (*async) (PGconn *conn,
- struct _PGoauthBearerRequest *request,
+ struct PGoauthBearerRequest *request,
SOCKTYPE * altsock);
/*
@@ -780,7 +780,7 @@ typedef struct _PGoauthBearerRequest
* This is technically optional, but highly recommended, because there is
* no other indication as to when it is safe to free the token.
*/
- void (*cleanup) (PGconn *conn, struct _PGoauthBearerRequest *request);
+ void (*cleanup) (PGconn *conn, struct PGoauthBearerRequest *request);
/*
* The hook should set this to the Bearer token contents for the
--
2.39.3 (Apple Git-146)
view thread (101+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox