public inbox for [email protected]  
help / color / mirror / Atom feed
unclear OAuth error message
12+ messages / 4 participants
[nested] [flat]

* unclear OAuth error message
@ 2026-01-24 14:50 Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Álvaro Herrera @ 2026-01-24 14:50 UTC (permalink / raw)
  To: Pg Hackers <[email protected]>; +Cc: Jacob Champion <[email protected]>

Hello,

While updating the translation, I noticed this code

    /*
     * Log any authentication results even if the token isn't authorized; it
     * might be useful for auditing or troubleshooting.
     */
    if (ret->authn_id)
        set_authn_id(port, ret->authn_id);

    if (!ret->authorized)
    {
        ereport(LOG,
                errmsg("OAuth bearer authentication failed for user \"%s\"",
                       port->user_name),
                errdetail_log("Validator failed to authorize the provided token."));

        status = false;
        goto cleanup;
    }

I'm not sure I understand the errdetail() part of it.  At first it made
me wonder if it was about a user-supplied module that had an internal
failure preventing it from deciding whether the user was authorized or
not (which would have been something like "Validator failed while ...").
But the code suggests that the module worked fine and made the
determination not to authorize the user.  If that's so, then why do we
have the errdetail at all?  Can't we just get rid of it and let the
errmsg stand on its own merit?

There is one more case for this exact errmsg to be given:

    /* Make sure the validator authenticated the user. */
    if (ret->authn_id == NULL || ret->authn_id[0] == '\0')
    {
        ereport(LOG,
                errmsg("OAuth bearer authentication failed for user \"%s\"",
                       port->user_name),
                errdetail_log("Validator provided no identity."));

Here it seems the validator did indeed have an internal problem of some
sort, because while it did declare that the user was authorized, it did
not provide what we were expecting from it.  Should in this case the
errmsg() be different?

(Actually, there's also auth_failed() giving the same message.)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"The saddest aspect of life right now is that science gathers knowledge faster
 than society gathers wisdom."  (Isaac Asimov)






^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
@ 2026-01-26 22:17 ` Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Jacob Champion @ 2026-01-26 22:17 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>

On Sat, Jan 24, 2026 at 6:50 AM Álvaro Herrera <[email protected]> wrote:
> But the code suggests that the module worked fine and made the
> determination not to authorize the user.  If that's so, then why do we
> have the errdetail at all?  Can't we just get rid of it and let the
> errmsg stand on its own merit?

For that code path I suspect we could get rid of the entire message,
because of what you mentioned later: auth_failed() is already going to
give us that. The validator can log what's important if needed, or
not. We could add some DEBUGs, maybe, so that you can still figure out
what's going on if a validator fails silently?

> Here it seems the validator did indeed have an internal problem of some
> sort, because while it did declare that the user was authorized, it did
> not provide what we were expecting from it.  Should in this case the
> errmsg() be different?

Yeah, I think so. The errdetail should probably become the errmsg,
essentially (but with more context).

Thanks,
--Jacob






^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
@ 2026-03-18 16:47   ` Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Jacob Champion @ 2026-03-18 16:47 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>; Zsolt Parragi <[email protected]>

On Mon, Jan 26, 2026 at 2:17 PM Jacob Champion
<[email protected]> wrote:
> For that code path I suspect we could get rid of the entire message,
> because of what you mentioned later: auth_failed() is already going to
> give us that. The validator can log what's important if needed, or
> not. We could add some DEBUGs, maybe, so that you can still figure out
> what's going on if a validator fails silently?

I only just remembered that this is exactly what logdetail is designed
to do. It's passed in from CheckSASLAuth, but OAuth doesn't make use
of it. (My original patchset carried a TODO for this for a long time,
but I lost it at some point...)

I have a parallel patchset that also needs logdetail, so this fix can
piggyback on some of that work.

--Jacob





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
@ 2026-03-20 17:52     ` Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Jacob Champion @ 2026-03-20 17:52 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>; Zsolt Parragi <[email protected]>

On Wed, Mar 18, 2026 at 9:47 AM Jacob Champion
<[email protected]> wrote:
> I have a parallel patchset that also needs logdetail, so this fix can
> piggyback on some of that work.

Here is (the confusingly named) v3-0001, which I hope fixes both
upthread complaints. It comes from [1].

--Jacob

[1] https://postgr.es/m/CAOYmi%2BnTXGcroZD_Mnkc8LYWYFbfDYNR4ML_yQ5sF9%2BDY2amcg%40mail.gmail.com


Attachments:

  [application/octet-stream] v3-0001-oauth-Let-validators-provide-failure-DETAILs.patch (12.6K, 2-v3-0001-oauth-Let-validators-provide-failure-DETAILs.patch)
  download | inline diff:
From f36e6becc34d05834c3d808575b04d39badf7569 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Thu, 19 Mar 2026 09:37:20 -0700
Subject: [PATCH v3 1/2] oauth: Let validators provide failure DETAILs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

At the moment, the only way for a validator module to report error
details on failure is to log them separately before returning from
validate_cb. Independently of that problem, the ereport() calls that we
make during validation failure partially duplicate some of the work of
auth_failed().

The end result is overly verbose and confusing for readers of the logs:

    [768233] LOG:  [my_validator] bad signature in bearer token
    [768233] LOG:  OAuth bearer authentication failed for user "jacob"
    [768233] DETAIL:  Validator failed to authorize the provided token.
    [768233] FATAL:  OAuth bearer authentication failed for user "jacob"
    [768233] DETAIL:  Connection matched file ".../pg_hba.conf" line ...

Solve both problems by making use of the existing logdetail pointer
that's provided by ClientAuthentication. Validator modules may set
ValidatorModuleResult->error_detail to override our default generic
message.

The end result looks something like

    [242284] FATAL:  OAuth bearer authentication failed for user "jacob"
    [242284] DETAIL:  [my_validator] bad signature in bearer token
        Connection matched file ".../pg_hba.conf" line ...

Reported-by: Álvaro Herrera <[email protected]>
Reported-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql
Discussion: TODO
---
 doc/src/sgml/oauth-validators.sgml            | 21 +++++++++-
 src/include/libpq/oauth.h                     | 14 +++++++
 src/backend/libpq/auth-oauth.c                | 24 +++++------
 src/backend/libpq/auth.c                      |  2 +-
 .../modules/oauth_validator/t/001_server.pl   | 40 ++++++++++++++++++-
 src/test/modules/oauth_validator/validator.c  | 29 ++++++++++++++
 6 files changed, 114 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml
index 704089dd7b3..a7140eae84e 100644
--- a/doc/src/sgml/oauth-validators.sgml
+++ b/doc/src/sgml/oauth-validators.sgml
@@ -192,11 +192,18 @@
      <term>Logging</term>
      <listitem>
       <para>
-       Modules may use the same <link linkend="error-message-reporting">logging
+       To simply log the reason for a validation failure, validators may set
+       the freeform <structfield>error_detail</structfield> field during the
+       <xref linkend="oauth-validator-callback-validate"/>. This is printed only
+       to the server log, as part of the final authentication failure message,
+       and it is not shared with the client.
+      </para>
+      <para>
+       Modules may also use the same <link linkend="error-message-reporting">logging
        facilities</link> as standard extensions; however, the rules for emitting
        log entries to the client are subtly different during the authentication
        phase of the connection. Generally speaking, modules should log
-       verification problems at the <symbol>COMMERROR</symbol> level and return
+       problems at the <symbol>COMMERROR</symbol> level and return
        normally, instead of using <symbol>ERROR</symbol>/<symbol>FATAL</symbol>
        to unwind the stack, to avoid leaking information to unauthenticated
        clients.
@@ -370,6 +377,7 @@ typedef struct ValidatorModuleResult
 {
     bool        authorized;
     char       *authn_id;
+    char       *error_detail;
 } ValidatorModuleResult;
 </programlisting>
 
@@ -387,6 +395,15 @@ typedef struct ValidatorModuleResult
     Otherwise the validator should return <literal>true</literal> to indicate
     that it has processed the token and made an authorization decision.
    </para>
+   <para>
+    In either failure case (validation error or internal error) the module may
+    store a user-readable reason for the failure in <structfield>result->error_detail</structfield>.
+    This will be printed to the server logs (not sent to the client) as a
+    <literal>DETAIL</literal> entry for the authentication failure. The memory
+    pointed to by <structfield>error_detail</structfield> may be either palloc'd
+    or of static duration. <structfield>error_detail</structfield> is ignored
+    on success.
+   </para>
    <para>
     The behavior after <function>validate_cb</function> returns depends on the
     specific HBA setup.  Normally, the <structfield>result->authn_id</structfield> user
diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h
index 4a822e9a1f2..60f493acddd 100644
--- a/src/include/libpq/oauth.h
+++ b/src/include/libpq/oauth.h
@@ -49,6 +49,20 @@ typedef struct ValidatorModuleResult
 	 * delegation. See the validator module documentation for details.
 	 */
 	char	   *authn_id;
+
+	/*
+	 * When validation fails, this may optionally be set to a string
+	 * containing an explanation for the failure. It will be sent to the
+	 * server log only; it is not provided to the client, and it's ignored if
+	 * validation succeeds.
+	 *
+	 * This description will be attached to the final authentication failure
+	 * message in the logs, as a DETAIL, which may be preferable to separate
+	 * ereport() calls that have to be correlated by the reader.
+	 *
+	 * This string may be either of static duration or palloc'd.
+	 */
+	char	   *error_detail;
 } ValidatorModuleResult;
 
 /*
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index 11365048951..6d4be70aada 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -73,7 +73,7 @@ struct oauth_ctx
 static char *sanitize_char(char c);
 static char *parse_kvpairs_for_auth(char **input);
 static void generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen);
-static bool validate(Port *port, const char *auth);
+static bool validate(Port *port, const char *auth, const char **logdetail);
 
 /* Constants seen in an OAUTHBEARER client initial response. */
 #define KVSEP 0x01				/* separator byte for key/value pairs */
@@ -279,7 +279,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 				errmsg("malformed OAUTHBEARER message"),
 				errdetail("Message contains additional data after the final terminator."));
 
-	if (!validate(ctx->port, auth))
+	if (!validate(ctx->port, auth, logdetail))
 	{
 		generate_error_response(ctx, output, outputlen);
 
@@ -635,7 +635,7 @@ validate_token_format(const char *header)
  * authorization. Returns true if validation succeeds.
  */
 static bool
-validate(Port *port, const char *auth)
+validate(Port *port, const char *auth, const char **logdetail)
 {
 	int			map_status;
 	ValidatorModuleResult *ret;
@@ -662,7 +662,10 @@ validate(Port *port, const char *auth)
 	{
 		ereport(WARNING,
 				errcode(ERRCODE_INTERNAL_ERROR),
-				errmsg("internal error in OAuth validator module"));
+				errmsg("internal error in OAuth validator module"),
+				ret->error_detail ? errdetail_log("%s", ret->error_detail) : 0);
+
+		*logdetail = ret->error_detail;
 		return false;
 	}
 
@@ -675,10 +678,10 @@ validate(Port *port, const char *auth)
 
 	if (!ret->authorized)
 	{
-		ereport(LOG,
-				errmsg("OAuth bearer authentication failed for user \"%s\"",
-					   port->user_name),
-				errdetail_log("Validator failed to authorize the provided token."));
+		if (ret->error_detail)
+			*logdetail = ret->error_detail;
+		else
+			*logdetail = _("Validator failed to authorize the provided token.");
 
 		status = false;
 		goto cleanup;
@@ -699,10 +702,7 @@ validate(Port *port, const char *auth)
 	/* Make sure the validator authenticated the user. */
 	if (ret->authn_id == NULL || ret->authn_id[0] == '\0')
 	{
-		ereport(LOG,
-				errmsg("OAuth bearer authentication failed for user \"%s\"",
-					   port->user_name),
-				errdetail_log("Validator provided no identity."));
+		*logdetail = _("Validator provided no identity.");
 
 		status = false;
 		goto cleanup;
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index e04aa2e68ed..6801ea9c566 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -625,7 +625,7 @@ ClientAuthentication(Port *port)
 			status = STATUS_OK;
 			break;
 		case uaOAuth:
-			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL);
+			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail);
 			break;
 	}
 
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index cdad2ae8011..28d123b833e 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -521,8 +521,8 @@ $node->connect_fails(
 	expected_stderr => qr/OAuth bearer authentication failed/,
 	log_like => [
 		qr/connection authenticated: identity=""/,
-		qr/DETAIL:\s+Validator provided no identity/,
 		qr/FATAL:\s+OAuth bearer authentication failed/,
+		qr/DETAIL:\s+Validator provided no identity/,
 	]);
 
 # Even if a validator authenticates the user, if the token isn't considered
@@ -541,10 +541,48 @@ $node->connect_fails(
 	expected_stderr => qr/OAuth bearer authentication failed/,
 	log_like => [
 		qr/connection authenticated: identity="test\@example\.org"/,
+		qr/FATAL:\s+OAuth bearer authentication failed/,
 		qr/DETAIL:\s+Validator failed to authorize the provided token/,
+	]);
+
+# Validators can provide their own explanations.
+$bgconn->query_safe(
+	"ALTER SYSTEM SET oauth_validator.error_detail TO 'something failed'");
+$node->reload;
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
+$node->connect_fails(
+	"$common_connstr user=test",
+	"validator must authorize token explicitly (custom logdetail)",
+	expected_stderr => qr/OAuth bearer authentication failed/,
+	log_like => [
+		qr/connection authenticated: identity="test\@example\.org"/,
 		qr/FATAL:\s+OAuth bearer authentication failed/,
+		qr/DETAIL:\s+something failed/,
 	]);
 
+$bgconn->query_safe(
+	"ALTER SYSTEM SET oauth_validator.internal_error TO true");
+$node->reload;
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
+$node->connect_fails(
+	"$common_connstr user=test",
+	"validator internal error (custom logdetail)",
+	expected_stderr => qr/OAuth bearer authentication failed/,
+	log_like => [
+		qr/WARNING:\s+internal error in OAuth validator module/,
+		qr/DETAIL:\s+something failed/,
+	]);
+
+$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail");
+$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error");
+$node->reload;
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
 #
 # Test user mapping.
 #
diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c
index 0b983a9dc8f..353e0e0d32a 100644
--- a/src/test/modules/oauth_validator/validator.c
+++ b/src/test/modules/oauth_validator/validator.c
@@ -40,6 +40,8 @@ static const OAuthValidatorCallbacks validator_callbacks = {
 /* GUCs */
 static char *authn_id = NULL;
 static bool authorize_tokens = true;
+static char *error_detail = NULL;
+static bool internal_error = false;
 
 /*---
  * Extension entry point. Sets up GUCs for use by tests:
@@ -51,6 +53,13 @@ static bool authorize_tokens = true;
  * - oauth_validator.authorize_tokens
  *								Sets whether to successfully validate incoming
  *								tokens. Defaults to true.
+ *
+ * - oauth_validator.error_detail
+ *                              Sets an error message to be included as a
+ *                              DETAIL on failure.
+ *
+ * - oauth_validator.internal_error
+ *                              Reports an internal error to the server.
  */
 void
 _PG_init(void)
@@ -71,6 +80,22 @@ _PG_init(void)
 							 PGC_SIGHUP,
 							 0,
 							 NULL, NULL, NULL);
+	DefineCustomStringVariable("oauth_validator.error_detail",
+							   "Error message to print during failures",
+							   NULL,
+							   &error_detail,
+							   NULL,
+							   PGC_SIGHUP,
+							   0,
+							   NULL, NULL, NULL);
+	DefineCustomBoolVariable("oauth_validator.internal_error",
+							 "Should the validator report an internal error?",
+							 NULL,
+							 &internal_error,
+							 false,
+							 PGC_SIGHUP,
+							 0,
+							 NULL, NULL, NULL);
 
 	MarkGUCPrefixReserved("oauth_validator");
 }
@@ -133,6 +158,10 @@ validate_token(const ValidatorModuleState *state,
 		 MyProcPort->hba->oauth_issuer,
 		 MyProcPort->hba->oauth_scope);
 
+	res->error_detail = error_detail;	/* only relevant for failures */
+	if (internal_error)
+		return false;
+
 	res->authorized = authorize_tokens;
 	if (authn_id)
 		res->authn_id = pstrdup(authn_id);
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
@ 2026-03-23 21:21       ` Zsolt Parragi <[email protected]>
  2026-03-27 17:01         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-27 17:03         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 2 replies; 12+ messages in thread

From: Zsolt Parragi @ 2026-03-23 21:21 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>

This is definitely a nice improvement, I only have two minor questions:

- errmsg("internal error in OAuth validator module"));
+ errmsg("internal error in OAuth validator module"),
+ ret->error_detail ? errdetail_log("%s", ret->error_detail) : 0);
+

Isn't including the detail for both the warning and the fatal error
still overly verbose?

+ res->error_detail = error_detail; /* only relevant for failures */
+ if (internal_error)
+ return false;
+

Shouldn't the oauth code include a sanity check to ensure validators
return no error_detail on success instead of silently ignoring it?





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
@ 2026-03-27 17:01         ` Jacob Champion <[email protected]>
  2026-03-27 22:24           ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
  1 sibling, 1 reply; 12+ messages in thread

From: Jacob Champion @ 2026-03-27 17:01 UTC (permalink / raw)
  To: Zsolt Parragi <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>

On Mon, Mar 23, 2026 at 2:21 PM Zsolt Parragi <[email protected]> wrote:
> Isn't including the detail for both the warning and the fatal error
> still overly verbose?

I'm not too worried about verbosity for an internal error situation;
users shouldn't see it. If they do, I don't mind being very loud about
whose fault it is.

(I'm also influenced by some recent support work on clusters that have
huge log volumes. If someone is focused on the internal error, they
should be able to see at a glance what caused that error, and if
someone is focused on the authentication failure, they should be able
to see at a glance what caused that. The more logs you have to
correlate in a "help! no one can log in" panic situation, the less
likely you are to succeed.)

> Shouldn't the oauth code include a sanity check to ensure validators
> return no error_detail on success instead of silently ignoring it?

IMO, no. I don't want error_detail to add semantics to the API, just
descriptive power. Plus, I think a design that sets a possible error
message before entering a complex operation, knowing that it will be
ignored on success, is perfectly valid. libpq-oauth, and to a lesser
extent libpq, make use of that pattern.

--Jacob





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
  2026-03-27 17:01         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
@ 2026-03-27 22:24           ` Daniel Gustafsson <[email protected]>
  2026-03-27 23:20             ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Daniel Gustafsson @ 2026-03-27 22:24 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>

> On 27 Mar 2026, at 18:01, Jacob Champion <[email protected]> wrote:
> 
> On Mon, Mar 23, 2026 at 2:21 PM Zsolt Parragi <[email protected]> wrote:
>> Isn't including the detail for both the warning and the fatal error
>> still overly verbose?
> 
> I'm not too worried about verbosity for an internal error situation;
> users shouldn't see it. If they do, I don't mind being very loud about
> whose fault it is.
> 
> (I'm also influenced by some recent support work on clusters that have
> huge log volumes. If someone is focused on the internal error, they
> should be able to see at a glance what caused that error, and if
> someone is focused on the authentication failure, they should be able
> to see at a glance what caused that. The more logs you have to
> correlate in a "help! no one can log in" panic situation, the less
> likely you are to succeed.)

Agreed.

>> Shouldn't the oauth code include a sanity check to ensure validators
>> return no error_detail on success instead of silently ignoring it?
> 
> IMO, no. I don't want error_detail to add semantics to the API, just
> descriptive power. Plus, I think a design that sets a possible error
> message before entering a complex operation, knowing that it will be
> ignored on success, is perfectly valid. libpq-oauth, and to a lesser
> extent libpq, make use of that pattern.

Callsites can also clear the error message on success and not even rely on it
being ignored.

+	 * This string may be either of static duration or palloc'd.
+	 */
+	char	   *error_detail;

I'm not a big fan of "either static or allocated" and prefer if we just require
one or the other.  We have this pattern in other places so it's not a blocker
for going it, but.

--
Daniel Gustafsson






^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
  2026-03-27 17:01         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-27 22:24           ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
@ 2026-03-27 23:20             ` Jacob Champion <[email protected]>
  2026-04-01 22:57               ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Jacob Champion @ 2026-03-27 23:20 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>

On Fri, Mar 27, 2026 at 3:24 PM Daniel Gustafsson <[email protected]> wrote:
> > IMO, no. I don't want error_detail to add semantics to the API, just
> > descriptive power. Plus, I think a design that sets a possible error
> > message before entering a complex operation, knowing that it will be
> > ignored on success, is perfectly valid. libpq-oauth, and to a lesser
> > extent libpq, make use of that pattern.
>
> Callsites can also clear the error message on success and not even rely on it
> being ignored.

Agreed, but are you saying that as an argument for my approach, or for Zsolt's?

> +        * This string may be either of static duration or palloc'd.
> +        */
> +       char       *error_detail;
>
> I'm not a big fan of "either static or allocated" and prefer if we just require
> one or the other.  We have this pattern in other places so it's not a blocker
> for going it, but.

I don't think I can enforce either choice, though: I pass the
error_detail into the ereport(FATAL), so the process is about to go
down, and I'm never going to pfree() it.

--Jacob





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
  2026-03-27 17:01         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-27 22:24           ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
  2026-03-27 23:20             ` Re: unclear OAuth error message Jacob Champion <[email protected]>
@ 2026-04-01 22:57               ` Jacob Champion <[email protected]>
  2026-04-02 21:25                 ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Jacob Champion @ 2026-04-01 22:57 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>; Chao Li <[email protected]>

On Fri, Mar 27, 2026 at 4:20 PM Jacob Champion
<[email protected]> wrote:
> I don't think I can enforce either choice, though: I pass the
> error_detail into the ereport(FATAL), so the process is about to go
> down, and I'm never going to pfree() it.

(I also think it's reasonable to do something like

    res->error_detail = "Help! it's again.";

if there's nothing to format, without mandating a superfluous pstrdup().)

v4 rebases, incorporates Chao Li's feedback, rewrites the surrounding
paragraph a bit, and fixes a silly misuse of <xref> when I meant
<link>.

--Jacob


Attachments:

  [application/octet-stream] v4-0001-oauth-Let-validators-provide-failure-DETAILs.patch (12.9K, 2-v4-0001-oauth-Let-validators-provide-failure-DETAILs.patch)
  download | inline diff:
From a9a507604c579ac1bf3c07cd8353ada2fbac1fb8 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Thu, 19 Mar 2026 09:37:20 -0700
Subject: [PATCH v4] oauth: Let validators provide failure DETAILs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

At the moment, the only way for a validator module to report error
details on failure is to log them separately before returning from
validate_cb. Independently of that problem, the ereport() calls that we
make during validation failure partially duplicate some of the work of
auth_failed().

The end result is overly verbose and confusing for readers of the logs:

    [768233] LOG:  [my_validator] bad signature in bearer token
    [768233] LOG:  OAuth bearer authentication failed for user "jacob"
    [768233] DETAIL:  Validator failed to authorize the provided token.
    [768233] FATAL:  OAuth bearer authentication failed for user "jacob"
    [768233] DETAIL:  Connection matched file ".../pg_hba.conf" line ...

Solve both problems by making use of the existing logdetail pointer
that's provided by ClientAuthentication. Validator modules may set
ValidatorModuleResult->error_detail to override our default generic
message.

The end result looks something like

    [242284] FATAL:  OAuth bearer authentication failed for user "jacob"
    [242284] DETAIL:  [my_validator] bad signature in bearer token
        Connection matched file ".../pg_hba.conf" line ...

Reported-by: Álvaro Herrera <[email protected]>
Reported-by: Zsolt Parragi <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql
---
 doc/src/sgml/oauth-validators.sgml            | 23 ++++++++++-
 src/include/libpq/oauth.h                     | 14 +++++++
 src/backend/libpq/auth-oauth.c                | 24 +++++------
 src/backend/libpq/auth.c                      |  2 +-
 .../modules/oauth_validator/t/001_server.pl   | 40 ++++++++++++++++++-
 src/test/modules/oauth_validator/validator.c  | 29 ++++++++++++++
 6 files changed, 116 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml
index 704089dd7b3..5f29f2be186 100644
--- a/doc/src/sgml/oauth-validators.sgml
+++ b/doc/src/sgml/oauth-validators.sgml
@@ -192,11 +192,20 @@
      <term>Logging</term>
      <listitem>
       <para>
-       Modules may use the same <link linkend="error-message-reporting">logging
+       To simply log the reason for a validation failure, modules may set the
+       freeform <structfield>error_detail</structfield> field during the
+       <link linkend="oauth-validator-callback-validate">validate callback</link>.
+       (<xref linkend="error-style-guide"/> has guidelines for writing good
+       <literal>DETAIL</literal> messages.) <structfield>error_detail</structfield>
+       is printed only to the server log, as part of the final authentication
+       failure message, and it is not shared with the client.
+      </para>
+      <para>
+       Modules may also use the same <link linkend="error-message-reporting">logging
        facilities</link> as standard extensions; however, the rules for emitting
        log entries to the client are subtly different during the authentication
        phase of the connection. Generally speaking, modules should log
-       verification problems at the <symbol>COMMERROR</symbol> level and return
+       problems at the <symbol>COMMERROR</symbol> level and return
        normally, instead of using <symbol>ERROR</symbol>/<symbol>FATAL</symbol>
        to unwind the stack, to avoid leaking information to unauthenticated
        clients.
@@ -370,6 +379,7 @@ typedef struct ValidatorModuleResult
 {
     bool        authorized;
     char       *authn_id;
+    char       *error_detail;
 } ValidatorModuleResult;
 </programlisting>
 
@@ -387,6 +397,15 @@ typedef struct ValidatorModuleResult
     Otherwise the validator should return <literal>true</literal> to indicate
     that it has processed the token and made an authorization decision.
    </para>
+   <para>
+    In either failure case (validation error or internal error) the module may
+    store a user-readable reason for the failure in <structfield>result->error_detail</structfield>.
+    This will be printed to the server logs (not sent to the client) as a
+    <literal>DETAIL</literal> entry for the authentication failure. The memory
+    pointed to by <structfield>error_detail</structfield> may be either palloc'd
+    or of static duration. <structfield>error_detail</structfield> is ignored
+    on success.
+   </para>
    <para>
     The behavior after <function>validate_cb</function> returns depends on the
     specific HBA setup.  Normally, the <structfield>result->authn_id</structfield> user
diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h
index 4a822e9a1f2..60f493acddd 100644
--- a/src/include/libpq/oauth.h
+++ b/src/include/libpq/oauth.h
@@ -49,6 +49,20 @@ typedef struct ValidatorModuleResult
 	 * delegation. See the validator module documentation for details.
 	 */
 	char	   *authn_id;
+
+	/*
+	 * When validation fails, this may optionally be set to a string
+	 * containing an explanation for the failure. It will be sent to the
+	 * server log only; it is not provided to the client, and it's ignored if
+	 * validation succeeds.
+	 *
+	 * This description will be attached to the final authentication failure
+	 * message in the logs, as a DETAIL, which may be preferable to separate
+	 * ereport() calls that have to be correlated by the reader.
+	 *
+	 * This string may be either of static duration or palloc'd.
+	 */
+	char	   *error_detail;
 } ValidatorModuleResult;
 
 /*
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index 894efe3c904..6a75b79efbf 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -74,7 +74,7 @@ struct oauth_ctx
 static char *sanitize_char(char c);
 static char *parse_kvpairs_for_auth(char **input);
 static void generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen);
-static bool validate(Port *port, const char *auth);
+static bool validate(Port *port, const char *auth, const char **logdetail);
 
 /* Constants seen in an OAUTHBEARER client initial response. */
 #define KVSEP 0x01				/* separator byte for key/value pairs */
@@ -305,7 +305,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 		ctx->state = OAUTH_STATE_ERROR_DISCOVERY;
 		status = PG_SASL_EXCHANGE_CONTINUE;
 	}
-	else if (!validate(ctx->port, auth))
+	else if (!validate(ctx->port, auth, logdetail))
 	{
 		generate_error_response(ctx, output, outputlen);
 
@@ -650,7 +650,7 @@ validate_token_format(const char *header)
  * authorization. Returns true if validation succeeds.
  */
 static bool
-validate(Port *port, const char *auth)
+validate(Port *port, const char *auth, const char **logdetail)
 {
 	int			map_status;
 	ValidatorModuleResult *ret;
@@ -677,7 +677,10 @@ validate(Port *port, const char *auth)
 	{
 		ereport(WARNING,
 				errcode(ERRCODE_INTERNAL_ERROR),
-				errmsg("internal error in OAuth validator module"));
+				errmsg("internal error in OAuth validator module"),
+				ret->error_detail ? errdetail_log("%s", ret->error_detail) : 0);
+
+		*logdetail = ret->error_detail;
 		return false;
 	}
 
@@ -690,10 +693,10 @@ validate(Port *port, const char *auth)
 
 	if (!ret->authorized)
 	{
-		ereport(LOG,
-				errmsg("OAuth bearer authentication failed for user \"%s\"",
-					   port->user_name),
-				errdetail_log("Validator failed to authorize the provided token."));
+		if (ret->error_detail)
+			*logdetail = ret->error_detail;
+		else
+			*logdetail = _("Validator failed to authorize the provided token.");
 
 		status = false;
 		goto cleanup;
@@ -714,10 +717,7 @@ validate(Port *port, const char *auth)
 	/* Make sure the validator authenticated the user. */
 	if (ret->authn_id == NULL || ret->authn_id[0] == '\0')
 	{
-		ereport(LOG,
-				errmsg("OAuth bearer authentication failed for user \"%s\"",
-					   port->user_name),
-				errdetail_log("Validator provided no identity."));
+		*logdetail = _("Validator provided no identity.");
 
 		status = false;
 		goto cleanup;
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index fdacc060381..47b5eeb8f22 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -639,7 +639,7 @@ ClientAuthentication(Port *port)
 			status = STATUS_OK;
 			break;
 		case uaOAuth:
-			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL,
+			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail,
 								   &abandoned);
 			break;
 	}
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index c9c46e63539..ac62555675a 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -544,8 +544,8 @@ $node->connect_fails(
 	expected_stderr => qr/OAuth bearer authentication failed/,
 	log_like => [
 		qr/connection authenticated: identity=""/,
-		qr/DETAIL:\s+Validator provided no identity/,
 		qr/FATAL:\s+OAuth bearer authentication failed/,
+		qr/DETAIL:\s+Validator provided no identity/,
 	]);
 
 # Even if a validator authenticates the user, if the token isn't considered
@@ -564,10 +564,48 @@ $node->connect_fails(
 	expected_stderr => qr/OAuth bearer authentication failed/,
 	log_like => [
 		qr/connection authenticated: identity="test\@example\.org"/,
+		qr/FATAL:\s+OAuth bearer authentication failed/,
 		qr/DETAIL:\s+Validator failed to authorize the provided token/,
+	]);
+
+# Validators can provide their own explanations.
+$bgconn->query_safe(
+	"ALTER SYSTEM SET oauth_validator.error_detail TO 'something failed'");
+$node->reload;
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
+$node->connect_fails(
+	"$common_connstr user=test",
+	"validator must authorize token explicitly (custom logdetail)",
+	expected_stderr => qr/OAuth bearer authentication failed/,
+	log_like => [
+		qr/connection authenticated: identity="test\@example\.org"/,
 		qr/FATAL:\s+OAuth bearer authentication failed/,
+		qr/DETAIL:\s+something failed/,
 	]);
 
+$bgconn->query_safe(
+	"ALTER SYSTEM SET oauth_validator.internal_error TO true");
+$node->reload;
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
+$node->connect_fails(
+	"$common_connstr user=test",
+	"validator internal error (custom logdetail)",
+	expected_stderr => qr/OAuth bearer authentication failed/,
+	log_like => [
+		qr/WARNING:\s+internal error in OAuth validator module/,
+		qr/DETAIL:\s+something failed/,
+	]);
+
+$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail");
+$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error");
+$node->reload;
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
 #
 # Test user mapping.
 #
diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c
index 0b983a9dc8f..353e0e0d32a 100644
--- a/src/test/modules/oauth_validator/validator.c
+++ b/src/test/modules/oauth_validator/validator.c
@@ -40,6 +40,8 @@ static const OAuthValidatorCallbacks validator_callbacks = {
 /* GUCs */
 static char *authn_id = NULL;
 static bool authorize_tokens = true;
+static char *error_detail = NULL;
+static bool internal_error = false;
 
 /*---
  * Extension entry point. Sets up GUCs for use by tests:
@@ -51,6 +53,13 @@ static bool authorize_tokens = true;
  * - oauth_validator.authorize_tokens
  *								Sets whether to successfully validate incoming
  *								tokens. Defaults to true.
+ *
+ * - oauth_validator.error_detail
+ *                              Sets an error message to be included as a
+ *                              DETAIL on failure.
+ *
+ * - oauth_validator.internal_error
+ *                              Reports an internal error to the server.
  */
 void
 _PG_init(void)
@@ -71,6 +80,22 @@ _PG_init(void)
 							 PGC_SIGHUP,
 							 0,
 							 NULL, NULL, NULL);
+	DefineCustomStringVariable("oauth_validator.error_detail",
+							   "Error message to print during failures",
+							   NULL,
+							   &error_detail,
+							   NULL,
+							   PGC_SIGHUP,
+							   0,
+							   NULL, NULL, NULL);
+	DefineCustomBoolVariable("oauth_validator.internal_error",
+							 "Should the validator report an internal error?",
+							 NULL,
+							 &internal_error,
+							 false,
+							 PGC_SIGHUP,
+							 0,
+							 NULL, NULL, NULL);
 
 	MarkGUCPrefixReserved("oauth_validator");
 }
@@ -133,6 +158,10 @@ validate_token(const ValidatorModuleState *state,
 		 MyProcPort->hba->oauth_issuer,
 		 MyProcPort->hba->oauth_scope);
 
+	res->error_detail = error_detail;	/* only relevant for failures */
+	if (internal_error)
+		return false;
+
 	res->authorized = authorize_tokens;
 	if (authn_id)
 		res->authn_id = pstrdup(authn_id);
-- 
2.34.1



  [application/octet-stream] since-v3.nocfbot.diff (3.2K, 3-since-v3.nocfbot.diff)
  download | inline diff:
1:  f36e6becc34 ! 1:  a9a507604c5 oauth: Let validators provide failure DETAILs
    @@ Commit message
     
         Reported-by: Álvaro Herrera <[email protected]>
         Reported-by: Zsolt Parragi <[email protected]>
    +    Reviewed-by: Chao Li <[email protected]>
    +    Reviewed-by: Daniel Gustafsson <[email protected]>
    +    Reviewed-by: Zsolt Parragi <[email protected]>
         Discussion: https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql
    -    Discussion: TODO
     
      ## doc/src/sgml/oauth-validators.sgml ##
     @@
    @@ doc/src/sgml/oauth-validators.sgml
           <listitem>
            <para>
     -       Modules may use the same <link linkend="error-message-reporting">logging
    -+       To simply log the reason for a validation failure, validators may set
    -+       the freeform <structfield>error_detail</structfield> field during the
    -+       <xref linkend="oauth-validator-callback-validate"/>. This is printed only
    -+       to the server log, as part of the final authentication failure message,
    -+       and it is not shared with the client.
    ++       To simply log the reason for a validation failure, modules may set the
    ++       freeform <structfield>error_detail</structfield> field during the
    ++       <link linkend="oauth-validator-callback-validate">validate callback</link>.
    ++       (<xref linkend="error-style-guide"/> has guidelines for writing good
    ++       <literal>DETAIL</literal> messages.) <structfield>error_detail</structfield>
    ++       is printed only to the server log, as part of the final authentication
    ++       failure message, and it is not shared with the client.
     +      </para>
     +      <para>
     +       Modules may also use the same <link linkend="error-message-reporting">logging
    @@ src/backend/libpq/auth-oauth.c: struct oauth_ctx
      /* Constants seen in an OAUTHBEARER client initial response. */
      #define KVSEP 0x01				/* separator byte for key/value pairs */
     @@ src/backend/libpq/auth-oauth.c: oauth_exchange(void *opaq, const char *input, int inputlen,
    - 				errmsg("malformed OAUTHBEARER message"),
    - 				errdetail("Message contains additional data after the final terminator."));
    - 
    --	if (!validate(ctx->port, auth))
    -+	if (!validate(ctx->port, auth, logdetail))
    + 		ctx->state = OAUTH_STATE_ERROR_DISCOVERY;
    + 		status = PG_SASL_EXCHANGE_CONTINUE;
    + 	}
    +-	else if (!validate(ctx->port, auth))
    ++	else if (!validate(ctx->port, auth, logdetail))
      	{
      		generate_error_response(ctx, output, outputlen);
      
    @@ src/backend/libpq/auth.c: ClientAuthentication(Port *port)
      			status = STATUS_OK;
      			break;
      		case uaOAuth:
    --			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL);
    -+			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail);
    +-			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL,
    ++			status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail,
    + 								   &abandoned);
      			break;
      	}
    - 
     
      ## src/test/modules/oauth_validator/t/001_server.pl ##
     @@ src/test/modules/oauth_validator/t/001_server.pl: $node->connect_fails(


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
  2026-03-27 17:01         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-27 22:24           ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
  2026-03-27 23:20             ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-04-01 22:57               ` Re: unclear OAuth error message Jacob Champion <[email protected]>
@ 2026-04-02 21:25                 ` Daniel Gustafsson <[email protected]>
  2026-04-03 23:26                   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Daniel Gustafsson @ 2026-04-02 21:25 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>; Chao Li <[email protected]>

> On 2 Apr 2026, at 00:57, Jacob Champion <[email protected]> wrote:

> v4 rebases, incorporates Chao Li's feedback, rewrites the surrounding
> paragraph a bit, and fixes a silly misuse of <xref> when I meant
> <link>.

This version LGTM.

--
Daniel Gustafsson






^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
  2026-03-27 17:01         ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-27 22:24           ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
  2026-03-27 23:20             ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-04-01 22:57               ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-04-02 21:25                 ` Re: unclear OAuth error message Daniel Gustafsson <[email protected]>
@ 2026-04-03 23:26                   ` Jacob Champion <[email protected]>
  0 siblings, 0 replies; 12+ messages in thread

From: Jacob Champion @ 2026-04-03 23:26 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>; Chao Li <[email protected]>

On Thu, Apr 2, 2026 at 2:25 PM Daniel Gustafsson <[email protected]> wrote:
> This version LGTM.

Pushed, thanks!

--Jacob





^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: unclear OAuth error message
  2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
  2026-01-26 22:17 ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-18 16:47   ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-20 17:52     ` Re: unclear OAuth error message Jacob Champion <[email protected]>
  2026-03-23 21:21       ` Re: unclear OAuth error message Zsolt Parragi <[email protected]>
@ 2026-03-27 17:03         ` Jacob Champion <[email protected]>
  1 sibling, 0 replies; 12+ messages in thread

From: Jacob Champion @ 2026-03-27 17:03 UTC (permalink / raw)
  To: Chao Li <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Pg Hackers <[email protected]>; Zsolt Parragi <[email protected]>

On Mon, Mar 23, 2026 at 10:18 PM Chao Li <[email protected]> wrote:
> I also have a small comment on the doc change. The documentation already mentions the memory requirements for error_detail, so I wonder if it also makes sense to mention its style. Since it is emitted as a DETAIL message, I think it should probably follow the usual style rules for detail messages, i.e., start with a capital letter and end with a period.

Sure, I think it makes sense to add a quick link to our style guide as
part of the "best practices" section.

Thanks,
--Jacob





^ permalink  raw  reply  [nested|flat] 12+ messages in thread


end of thread, other threads:[~2026-04-03 23:26 UTC | newest]

Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-24 14:50 unclear OAuth error message Álvaro Herrera <[email protected]>
2026-01-26 22:17 ` Jacob Champion <[email protected]>
2026-03-18 16:47   ` Jacob Champion <[email protected]>
2026-03-20 17:52     ` Jacob Champion <[email protected]>
2026-03-23 21:21       ` Zsolt Parragi <[email protected]>
2026-03-27 17:01         ` Jacob Champion <[email protected]>
2026-03-27 22:24           ` Daniel Gustafsson <[email protected]>
2026-03-27 23:20             ` Jacob Champion <[email protected]>
2026-04-01 22:57               ` Jacob Champion <[email protected]>
2026-04-02 21:25                 ` Daniel Gustafsson <[email protected]>
2026-04-03 23:26                   ` Jacob Champion <[email protected]>
2026-03-27 17:03         ` Jacob Champion <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox