From 5e7887e7063b8d44303a118b90525c689d86082c Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Feb 2026 19:28:05 +0100 Subject: [PATCH v7] Improve OAuth discovery logging Currently when the client sends an empty OAuth token to request the issuer URL, the server logs the attempt with FATAL: OAuth bearer authentication failed for user Which is quite confusing, as this is an expected part of the OAuth authentication flow and not an error at all. This in practice results in the server spamming the log with these messages, which are difficult to separate from real (OAuth) authentication failures. This patch introduces FATAL_CLIENT_ONLY, a new ereport level analogous to WARNING_CLIENT_ONLY. It sends the ErrorResponse to the client (properly completing the SASL exchange per protocol requirements) but suppresses the message from the server log. The OAuth discovery handler uses this to terminate the discovery exchange cleanly: the client receives the expected ErrorResponse, but no misleading FATAL entry appears in the server log. --- src/backend/libpq/auth-oauth.c | 42 ++++++++++++------- src/backend/utils/error/elog.c | 7 +++- src/include/utils/elog.h | 4 +- .../modules/oauth_validator/t/001_server.pl | 6 ++- 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 11365048951..319dbc5bbbc 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -58,6 +58,7 @@ enum oauth_state { OAUTH_STATE_INIT = 0, OAUTH_STATE_ERROR, + OAUTH_STATE_ERROR_DISCOVERY, OAUTH_STATE_FINISHED, }; @@ -181,6 +182,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen, break; case OAUTH_STATE_ERROR: + case OAUTH_STATE_ERROR_DISCOVERY: /* * Only one response is valid for the client during authentication @@ -193,7 +195,18 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errdetail("Client did not send a kvsep response.")); /* The (failed) handshake is now complete. */ + if (ctx->state == OAUTH_STATE_ERROR_DISCOVERY) + { + ctx->state = OAUTH_STATE_FINISHED; + ereport(FATAL_CLIENT_ONLY, + errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), + errmsg("OAuth bearer authentication failed for user \"%s\"", + ctx->port->user_name), + errdetail("Empty request, discovery requested?")); + } + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; default: @@ -279,7 +292,19 @@ 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 (auth[0] == '\0') + { + /* + * An empty auth value represents a discovery request; the client + * expects it to fail. Skip validation entirely and move directly + * to the error response. + */ + generate_error_response(ctx, output, outputlen); + + ctx->state = OAUTH_STATE_ERROR_DISCOVERY; + status = PG_SASL_EXCHANGE_CONTINUE; + } + else if (!validate(ctx->port, auth)) { generate_error_response(ctx, output, outputlen); @@ -564,19 +589,8 @@ validate_token_format(const char *header) /* Missing auth headers should be handled by the caller. */ Assert(header); - - if (header[0] == '\0') - { - /* - * A completely empty auth header represents a query for - * authentication parameters. The client expects it to fail; there's - * no need to make any extra noise in the logs. - * - * TODO: should we find a way to return STATUS_EOF at the top level, - * to suppress the authentication error entirely? - */ - return NULL; - } + /* Empty auth (discovery) should be handled before calling validate(). */ + Assert(header[0] != '\0'); if (pg_strncasecmp(header, BEARER_SCHEME, strlen(BEARER_SCHEME))) { diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 0d0bf0f6aa5..2727d790e32 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -217,7 +217,7 @@ is_log_level_output(int elevel, int log_min_level) if (log_min_level == LOG || log_min_level <= ERROR) return true; } - else if (elevel == WARNING_CLIENT_ONLY) + else if (elevel == WARNING_CLIENT_ONLY || elevel == FATAL_CLIENT_ONLY) { /* never sent to log, regardless of log_min_level */ return false; @@ -573,7 +573,7 @@ errfinish(const char *filename, int lineno, const char *funcname) /* * Perform error recovery action as specified by elevel. */ - if (elevel == FATAL) + if (elevel == FATAL || elevel == FATAL_CLIENT_ONLY) { /* * For a FATAL error, we let proc_exit clean up and exit. @@ -2966,6 +2966,7 @@ write_eventlog(int level, const char *line, int len) break; case ERROR: case FATAL: + case FATAL_CLIENT_ONLY: case PANIC: default: eventlevel = EVENTLOG_ERROR_TYPE; @@ -3797,6 +3798,7 @@ send_message_to_server_log(ErrorData *edata) syslog_level = LOG_WARNING; break; case FATAL: + case FATAL_CLIENT_ONLY: syslog_level = LOG_ERR; break; case PANIC: @@ -4179,6 +4181,7 @@ error_severity(int elevel) prefix = gettext_noop("ERROR"); break; case FATAL: + case FATAL_CLIENT_ONLY: prefix = gettext_noop("FATAL"); break; case PANIC: diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index a12b379e09a..13e664e8210 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -53,7 +53,9 @@ struct Node; * known state */ #define PGERROR 21 /* Must equal ERROR; see NOTE below. */ #define FATAL 22 /* fatal error - abort process */ -#define PANIC 23 /* take down the other backends with me */ +#define FATAL_CLIENT_ONLY 23 /* Fatal error sent to the client, but never + * to the server log. */ +#define PANIC 24 /* take down the other backends with me */ /* * NOTE: the alternate names PGWARNING and PGERROR are useful for dealing diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index 6b649c0b06f..0e3ae599352 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -118,7 +118,8 @@ $node->connect_ok( qr/oauth_validator: issuer="\Q$issuer\E", scope="openid postgres"/, qr/connection authenticated: identity="test" method=oauth/, qr/connection authorized/, - ]); + ], + log_unlike => [qr/FATAL.*OAuth bearer authentication failed/]); # The /alternate issuer uses slightly different parameters, along with an # OAuth-style discovery document. @@ -133,7 +134,8 @@ $node->connect_ok( qr|oauth_validator: issuer="\Q$issuer/.well-known/oauth-authorization-server/alternate\E", scope="openid postgres alt"|, qr/connection authenticated: identity="testalt" method=oauth/, qr/connection authorized/, - ]); + ], + log_unlike => [qr/FATAL.*OAuth bearer authentication failed/]); # The issuer linked by the server must match the client's oauth_issuer setting. $node->connect_fails( -- 2.43.0