From e9b16515ea542cb5ee0e42cf5c5a1c7c246df081 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Feb 2026 19:28:05 +0100 Subject: [PATCH v3] 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 improves this by handling the situation properly in the SASL/Oauth code, by introducing a new SASL authentication status, PG_SASL_EXCHANGE_RESTART. The expectation is that authentication mechanisms can set this if they request a restart of the authentication flow. Restart currently requires starting with a new connection, so this simply sets STATUS_EOF. The above prevents logging a fatal error at the end, so instead the OAuth exchange code outputs a simple log message instead. --- src/backend/libpq/auth-oauth.c | 39 ++++++++++++------- src/backend/libpq/auth-sasl.c | 14 ++++--- src/include/libpq/sasl.h | 1 + .../modules/oauth_validator/t/001_server.pl | 8 +++- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 11365048951..3790d1686ee 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,6 +195,14 @@ 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(DEBUG1, + errmsg("OAuth issuer discovery requested")); + return PG_SASL_EXCHANGE_ABANDONED; + } + ctx->state = OAUTH_STATE_FINISHED; return PG_SASL_EXCHANGE_FAILURE; @@ -279,7 +289,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 +586,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/libpq/auth-sasl.c b/src/backend/libpq/auth-sasl.c index 36cb748d927..62bb06b579a 100644 --- a/src/backend/libpq/auth-sasl.c +++ b/src/backend/libpq/auth-sasl.c @@ -167,7 +167,7 @@ CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass, * PG_SASL_EXCHANGE_FAILURE with some output is forbidden by SASL. * Make sure here that the mechanism used got that right. */ - if (result == PG_SASL_EXCHANGE_FAILURE) + if (result == PG_SASL_EXCHANGE_FAILURE || result == PG_SASL_EXCHANGE_ABANDONED) elog(ERROR, "output message found after SASL exchange failure"); /* @@ -184,11 +184,13 @@ CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass, } } while (result == PG_SASL_EXCHANGE_CONTINUE); - /* Oops, Something bad happened */ - if (result != PG_SASL_EXCHANGE_SUCCESS) + switch (result) { - return STATUS_ERROR; + case PG_SASL_EXCHANGE_SUCCESS: + return STATUS_OK; + case PG_SASL_EXCHANGE_ABANDONED: + return STATUS_EOF; + default: + return STATUS_ERROR; } - - return STATUS_OK; } diff --git a/src/include/libpq/sasl.h b/src/include/libpq/sasl.h index 1e8ec7d6293..31a97747fed 100644 --- a/src/include/libpq/sasl.h +++ b/src/include/libpq/sasl.h @@ -25,6 +25,7 @@ #define PG_SASL_EXCHANGE_CONTINUE 0 #define PG_SASL_EXCHANGE_SUCCESS 1 #define PG_SASL_EXCHANGE_FAILURE 2 +#define PG_SASL_EXCHANGE_ABANDONED 3 /* * Maximum accepted size of SASL messages. diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index 6b649c0b06f..9f2f9f49077 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -114,11 +114,13 @@ $node->connect_ok( expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@, log_like => [ + qr/OAuth issuer discovery requested/, qr/oauth_validator: token="9243959234", role="$user"/, 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. @@ -129,11 +131,13 @@ $node->connect_ok( expected_stderr => qr@Visit https://example\.org/ and enter the code: postgresuser@, log_like => [ + qr/OAuth issuer discovery requested/, qr/oauth_validator: token="9243959234-alt", role="$user"/, 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