public inbox for [email protected]help / color / mirror / Atom feed
Custom oauth validator options 25+ messages / 3 participants [nested] [flat]
* Custom oauth validator options @ 2025-12-02 13:05 Zsolt Parragi <[email protected]> 0 siblings, 2 replies; 25+ messages in thread From: Zsolt Parragi @ 2025-12-02 13:05 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> Hello hackers, The current OAuth code allows validators to add custom validation logic, but does not allow them to introduce custom authentication-related parameters in the place where they belong: pg_hba.conf. As a workaround, both pg_oidc_validator and postgres-keycloak-oauth-validator (and likely others; these are simply the two I know of) rely on GUC variables instead. I see two issues with this: 1. Configuration for OAuth validation ends up split across two locations: issuer/scope and a few other parameters are defined in pg_hba.conf, while custom parameters must be set in postgresql.conf. 2. We have received multiple questions asking how to configure multiple OIDC servers with different parameter sets. I am not sure how common it is to use multiple OAuth providers with a single PostgreSQL instance, but the question is certainly reasonable. Given this, I would like to ask what you think about making pg_hba.conf extensible. At present, option parsing is hardcoded in parse_hba_auth_opt, and any unknown parameter triggers an error at the end of the function. I can see a few possible approaches: a. Add an OAuth-specific hook that allows injecting additional option-parsing logic into this function, as part of the existing OAuthValidatorCallbacks. This could be scoped to the validator used on the specific HBA line, even if multiple validators are loaded. b. Allow the existing startup callback to supply a list of additional valid configuration names, with the validation callback responsible for parsing and validating them. c. Add a more generic hook usable by any extension. I do not currently have concrete use cases outside OAuth, but perhaps others do. I would be interested in your thoughts on whether an improvement in this area would be useful. I also have two related questions, which might be addressed as part of the above or independently: 1. HBA options are parsed sequentially. If validator-specific options are tied to a particular validator, this implies that validator=... must appear before its parameters when multiple validators are loaded, since we cannot otherwise determine which validator is used. Is this acceptable behavior, or should options be allowed in any order? 2. If a validator introduces many options, an HBA line could become very long and hard to read. Would it make sense to allow loading the parameters for a given line from a separate file? This might already be useful today: for example, if a long issuer URL is repeated across several HBA lines, it could instead be defined once in an external file and referenced multiple times. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-16 23:09 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Jacob Champion @ 2025-12-16 23:09 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> Sorry for missing this thread! On Tue, Dec 2, 2025 at 5:06 AM Zsolt Parragi <[email protected]> wrote: > 1. Configuration for OAuth validation ends up split across two > locations: issuer/scope and a few other parameters are defined in > pg_hba.conf, while custom parameters must be set in postgresql.conf. Yeah. (This has come up before a couple of times that I know of, in the context of pg_hba and pg_ident splitting important configuration between them [1], and in the context of SNI's proposed pg_hosts config [2].) > 2. We have received multiple questions asking how to configure > multiple OIDC servers with different parameter sets. I am not sure how > common it is to use multiple OAuth providers with a single PostgreSQL > instance, but the question is certainly reasonable. What kinds of parameters? Having a motivating use case would be helpful; HBA isn't always as flexible as people assume and I want to make sure that we can end with a usable feature. > Given this, I would like to ask what you think about making > pg_hba.conf extensible. Your proposals (and the concerns they raise) seem reasonable enough at first glance. (I still want a motivating use case, though.) Honestly, I'd *prefer* that any solution not be OAuth-specific. I might throw two alternatives onto the pile: d. Have HBA plug into the GUC system itself A hypothetical PGC_HBA context would seem to fit nicely between PGC_SIGHUP and PGC_SU_BACKEND. e. Subsume HBA, ident, (hosts,) etc. under postgresql.conf This is my personal white whale. I think pg_hba+ident is no longer fit for purpose. It makes nonexistent use cases easy and common use cases unnecessarily difficult. Most people ignore half the columns. New users are surprised that you can't choose between authentication options. You have to correlate a bunch of different files with differing syntaxes to figure out what is going on. Etc, etc. This is why I bypassed pg_ident for validators, so that they could be free to do useful stuff while the core caught up. But I didn't intend to keep them separate forever. (I'm only halfway serious with (e) -- I don't really intend to drive your thread straight into a wall. But when I read proposals a-c, I get the sinking feeling that this *should* be solved in a more radical way, if we could only agree on a direction...) Thanks, --Jacob [1] https://postgr.es/m/0e0c038ab962c3f6dab00934fe5ae1ae115f44c0.camel%40vmware.com [2] https://postgr.es/m/CAOYmi%2B%3DZjGJLw8tCkzY88acd%3Dir1r8eAxO-%2B5wXm9gtCUV97Sg%40mail.gmail.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-17 06:30 VASUKI M <[email protected]> parent: Zsolt Parragi <[email protected]> 1 sibling, 2 replies; 25+ messages in thread From: VASUKI M @ 2025-12-17 06:30 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; [email protected]; [email protected]; Robert Haas <[email protected]>; [email protected] Hi All, The core issue,as you said,is that OAuth validators can add custom validation logic,but they can't define their own authentication-related parameters in pg_hba.conf,where they naturally belong.Because of that,validator-specific config ends up pushed into postgresql.conf via GUCs,which feels a bit awkward and scattered. That leads to the same points you mentioned: 1]OAuth configuration gets split between pg_hba.conf and postgres.conf,which is confusing to reason about. 2]using multiple OIDC/OAuth providers with different parameter sets on a single Postgresql instance is hard(or effectively impossible),even though it's a pretty reasonable use case. Given the constraints of the current HBA model(and similar issues that recently came up with SNI),I agree that anything involving generic extensibility or nested configuration would be a big hammer and likely too complex. I also have two related questions, which might be addressed as part of > the above or independently: > > 1. HBA options are parsed sequentially. If validator-specific options > are tied to a particular validator, this implies that validator=... > must appear before its parameters when multiple validators are loaded, > since we cannot otherwise determine which validator is used. Is this > acceptable behavior, or should options be allowed in any order? > > 2. If a validator introduces many options, an HBA line could become > very long and hard to read. Would it make sense to allow loading the > parameters for a given line from a separate file? This might already > be useful today: for example, if a long issuer URL is repeated across > several HBA lines, it could instead be defined once in an external > file and referenced multiple times. > > So the direction I'm most aligned with is option (b): letting OAuth validator advertise a limited list of additional valid option names for pg_hba.conf,while keeping parsing,ordering rules,and validation firmly in core.That seems like the least spicy option-incremental,OAuth-scoped,and not a redesign of HBA parsing. Reg. ordering:requiring validator= to appear before validator-specific options feels acceptable to me if this is pursued,since it keeps parsing simple and avoids ambiguity. Reg very long HBA lines: totally agree this is a real readability issue,but allowing per-line includes or external file feels like a seperate(and much bigger)topic,probably best tackled independently. Overall, +1 that this limitation is real and worth discussing.I’ll plan to send a patch shortly exploring option (b). Regards, Vasuki M CDAC,Chennai. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-17 09:33 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Zsolt Parragi @ 2025-12-17 09:33 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> > What kinds of parameters? Having a motivating use case would be > helpful; HBA isn't always as flexible as people assume and I want to > make sure that we can end with a usable feature. One issue we have is that some providers don't allow users to select what goes into the subject claim, but do allow users to define custom claims. Additionally, the subject claim is sometimes a random generated id, which gets generated on the first login to the client, and that makes it practically unusable for pg. It would require: * user trying to login to pg * getting rejected * figuring out what's the subject * adding it to pg ident / some other config * user can finally login Instead we decided to let everyone configure which claim they want to use for user mapping. But because of that, this is a GUC, and they can only configure it once pre server. The postgres-keycloak-oauth-validator is in an even worse situation, they decided to use a long list of GUC parameters[1]. The main reason is that they use an introspection endpoint for validation instead of the JWT, so they need multiple parameters for that. Some of these GUCs seem redundant to me, but some of them are definitely required. They also have parameters for the client id and debugging - those are things we are also considering adding to our validator. > (I'm only halfway serious with (e) -- I don't really intend to drive > your thread straight into a wall. But when I read proposals a-c, I get > the sinking feeling that this *should* be solved in a more radical > way, if we could only agree on a direction...) I tried to propose simple things that are relatively easy to implement, and wouldn't change too much at once, so there's a realistic change for this making into PG19. I'm not against having a bigger goal, and continuing making it even better after that. > A hypothetical PGC_HBA context would seem to fit nicely between > PGC_SIGHUP and PGC_SU_BACKEND. How would you configure that since the hba lines don't have IDs? Should we add a "guc_name" parameter to HBA for this or something like that? I like this idea, it would be fun to implement and see how it works, I'm just wondering how users could use it. [1]: https://github.com/cloudnative-pg/postgres-keycloak-oauth-validator/blob/5fceacf53c3d86fbbe18dab0341... ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-17 09:35 Zsolt Parragi <[email protected]> parent: VASUKI M <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Zsolt Parragi @ 2025-12-17 09:35 UTC (permalink / raw) To: VASUKI M <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; [email protected]; [email protected]; Robert Haas <[email protected]>; [email protected] > Overall, +1 that this limitation is real and worth discussing.I’ll plan to send a patch shortly exploring option (b). Personally I would go with either (a) or (c), and I was planning to clean up / improve / share my (c) patch as a second attempt for this thread, if it didn't receive any replies. I can still do that, so that we have multiple test implementations. (b) seemed a not as nice design to me, but maybe you find a better way to implement it than I did. Also now I really like the idea of the PGC_HBA, if there's a way for users to configure it without depending on line numbers or other easy-to-change details. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-17 18:27 Jacob Champion <[email protected]> parent: VASUKI M <[email protected]> 1 sibling, 0 replies; 25+ messages in thread From: Jacob Champion @ 2025-12-17 18:27 UTC (permalink / raw) To: VASUKI M <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Tue, Dec 16, 2025 at 10:30 PM VASUKI M <[email protected]> wrote: > Overall, +1 that this limitation is real and worth discussing.I’ll plan to send a patch shortly exploring option (b). Thanks! > Reg very long HBA lines: totally agree this is a real readability issue,but allowing per-line includes or external file feels like a seperate(and much bigger)topic,probably best tackled independently. I forgot to mention in my reply to Zsolt, but we've supported inline inclusions in HBA for a few releases now. (I just frequently forget they exist.) pg_hba.conf: hostssl all all 0.0.0.0/0 oauth @oauth-settings.conf oauth-settings.conf: issuer=https://oauth.example.org/v2 scope="openid email let-me-into-pg" validator=example_org map=examplemap And for smaller annoyances, you can wrap lines with backslash continuation. I haven't used these new features much, since I forget they exist, so if there are usability problems in practice please say something so we can fix it. --Jacob ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-17 19:01 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 2 replies; 25+ messages in thread From: Jacob Champion @ 2025-12-17 19:01 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Wed, Dec 17, 2025 at 1:28 AM Zsolt Parragi <[email protected]> wrote: > Instead we decided to let everyone configure which claim they want to > use for user mapping. But because of that, this is a GUC, and they can > only configure it once pre server. We're getting closer; I agree that this needs to be more flexible than it is, and I'm on board with a change, but I'm still missing the "killer app". What's the case where a user has multiple HBA lines that all want to use unrelated claims for authentication to one Postgres cluster? Is this multi-tenancy, or...? > I tried to propose simple things that are relatively easy to > implement, and wouldn't change too much at once, so there's a > realistic change for this making into PG19. I'm not against having a > bigger goal, and continuing making it even better after that. Absolutely -- that's a tried and true strategy. No objections to that. But I also didn't want to stay silent on my longer-term goals here. That way (hopefully), no one's surprised to find I'm lukewarm on patches that are extremely OAuth-specific, or that don't give us a way to improve/evolve later. The additional flexibility of OAuth should ideally be mirrored in other auth methods when possible. > > A hypothetical PGC_HBA context would seem to fit nicely between > > PGC_SIGHUP and PGC_SU_BACKEND. > > How would you configure that since the hba lines don't have IDs? > Should we add a "guc_name" parameter to HBA for this or something like > that? I like this idea, it would be fun to implement and see how it > works, I'm just wondering how users could use it. I hadn't thought it through very far; my initial impression was that we'd need some sort of additional syntax. But I keep coming back to httpd-style configs and then I choose something else from my TODO list to focus on. :) See also the old conversation regarding LDAP hba/ident [1]. On Wed, Dec 17, 2025 at 1:36 AM Zsolt Parragi <[email protected]> wrote: > Personally I would go with either (a) or (c), and I was planning to > clean up / improve / share my (c) patch as a second attempt for this > thread, if it didn't receive any replies. I can still do that, so that > we have multiple test implementations. The more the merrier! Thanks, --Jacob [1] https://postgr.es/m/CAOuzzgpFpuroNRabEvB9kST_TSyS2jFicBNoXvW7G2pZFixyBw%40mail.gmail.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-17 23:52 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 1 sibling, 0 replies; 25+ messages in thread From: Zsolt Parragi @ 2025-12-17 23:52 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > I forgot to mention in my reply to Zsolt, but we've supported inline > inclusions in HBA for a few releases now. (I just frequently forget > they exist.) Thanks, I didn't know about that feature, that solves half of my problem. > What's the case where a user has multiple HBA lines that > all want to use unrelated claims for authentication to one Postgres > cluster? Is this multi-tenancy, or...? For configuring the authn matching yes, the use case is multitenancy. But for some other variables that we didn't implement yet, this could be useful even without multitenancy. One thing I mentioned in the previous email is the client id validation. A practical use case of that would be restricting which oauth clients can login to which database. I can't use a SUSET variable with a check restricting it to ALTER DATABASE, because database level variables are not yet available during the oauth validator callback. I could use a login event trigger, but that seems like a bad hack to me. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-18 05:14 VASUKI M <[email protected]> parent: Jacob Champion <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: VASUKI M @ 2025-12-18 05:14 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Thu, Dec 18, 2025 at 12:31 AM Jacob Champion < [email protected]> wrote: > On Wed, Dec 17, 2025 at 1:28 AM Zsolt Parragi <[email protected]> > wrote: > > Instead we decided to let everyone configure which claim they want to > > use for user mapping. But because of that, this is a GUC, and they can > > only configure it once pre server. > > We're getting closer; I agree that this needs to be more flexible than > it is, and I'm on board with a change, but I'm still missing the > "killer app". What's the case where a user has multiple HBA lines that > all want to use unrelated claims for authentication to one Postgres > cluster? Is this multi-tenancy, or...? > > Beyond multitenancy,per -HBA OAuth cases where options are needed for safe provider migration[blue/green],per-database security policies,mixed Human/machine authentication[JWT/Introspection] and incident-response scenarios -all global GUCs are too coarse. See also the old conversation regarding LDAP hba/ident > [1] > > [1] > https://postgr.es/m/CAOuzzgpFpuroNRabEvB9kST_TSyS2jFicBNoXvW7G2pZFixyBw%40mail.gmail.com Thanks, Will go through it. Regards, Vasuki M CDAC,Chennai. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-18 09:08 Zsolt Parragi <[email protected]> parent: VASUKI M <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Zsolt Parragi @ 2025-12-18 09:08 UTC (permalink / raw) To: VASUKI M <[email protected]>; +Cc: Jacob Champion <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > Personally I would go with either (a) or (c), and I was planning to > clean up / improve / share my (c) patch as a second attempt for this > thread, if it didn't receive any replies. I can still do that, so that > we have multiple test implementations. I attached the patch. It modifies one of the existing oauth_validator tests to showcase how it works, but in theory it isn't dependent on oauth. It however requires shared_preload_libraries (that is common for all options), maybe oauth_validator_libraries could imply that? Attachments: [application/octet-stream] 0001-Adding-hooks-to-HBA-parsing-option-c.patch (9.0K, 2-0001-Adding-hooks-to-HBA-parsing-option-c.patch) download | inline diff: From 296acdb1551db523009ce7201daa03ef3e33f182 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi <[email protected]> Date: Thu, 18 Dec 2025 08:25:46 +0000 Subject: [PATCH] Adding hooks to HBA parsing, option c This commit showcases how we can add a generic, non oauth dependent hook to parsing hba entries, and also adds a simple test to the existing oauth_validator test suite. --- src/backend/libpq/hba.c | 34 ++++++++---- src/include/libpq/hba.h | 31 +++++++++++ .../modules/oauth_validator/t/002_client.pl | 51 ++++++++++++++++++ src/test/modules/oauth_validator/validator.c | 53 +++++++++++++++++++ 4 files changed, 160 insertions(+), 9 deletions(-) diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 4c259f58d77..409801ec6d8 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -125,6 +125,11 @@ static const char *const UserAuthName[] = StaticAssertDecl(lengthof(UserAuthName) == USER_AUTH_LAST + 1, "UserAuthName[] must match the UserAuth enum"); +/* + * Hook for plugins to extend pg_hba.conf option parsing. + */ +hba_parse_option_hook_type hba_parse_option_hook = NULL; + static List *tokenize_expand_file(List *tokens, const char *outer_filename, const char *inc_filename, int elevel, @@ -2507,15 +2512,26 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, } else { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("unrecognized authentication option name: \"%s\"", - name), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = psprintf("unrecognized authentication option name: \"%s\"", - name); - return false; + bool handled = false; + + if (hba_parse_option_hook) + { + handled = (*hba_parse_option_hook) (name, val, hbaline, + elevel, err_msg); + } + + if (!handled) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + *err_msg = psprintf("unrecognized authentication option name: \"%s\"", + name); + return false; + } } return true; } diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 7b93ba4a709..bd4658c72c8 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -172,6 +172,37 @@ typedef struct TokenizedAuthLine /* avoid including libpq/libpq-be.h here */ typedef struct Port Port; +/* + * Hook for plugins to extend pg_hba.conf option parsing. + * + * This hook is called by parse_hba_auth_opt() when it encounters an option + * name that it doesn't recognize. Plugins can use this to parse custom + * authentication. + * + * Parameters: + * name - The option name being parsed (e.g., "custom_option") + * val - The option value (may be NULL for boolean-style options) + * hbaline - The HbaLine structure being populated. Plugins should not + * modify standard fields, but can use this to check auth_method, + * conntype, etc. to validate option applicability. + * elevel - Error level for reporting (LOG, ERROR, etc.) + * err_msg - Output parameter for error messages. Set this to a palloc'd + * string if returning false due to a validation error. + * + * Return value: + * true - The hook recognized and successfully handled this option. + * false - The hook doesn't recognize this option, or encountered an error. + * If an error occurred, the hook should set *err_msg and/or call + * ereport(). + */ +typedef bool (*hba_parse_option_hook_type) (const char *name, + const char *val, + HbaLine *hbaline, + int elevel, + char **err_msg); + +extern PGDLLIMPORT hba_parse_option_hook_type hba_parse_option_hook; + extern bool load_hba(void); extern bool load_ident(void); extern const char *hba_authname(UserAuth auth_method); diff --git a/src/test/modules/oauth_validator/t/002_client.pl b/src/test/modules/oauth_validator/t/002_client.pl index e6c91fc911c..6576d6e41e0 100644 --- a/src/test/modules/oauth_validator/t/002_client.pl +++ b/src/test/modules/oauth_validator/t/002_client.pl @@ -29,6 +29,8 @@ $node->init; $node->append_conf('postgresql.conf', "log_connections = all\n"); $node->append_conf('postgresql.conf', "oauth_validator_libraries = 'validator'\n"); +$node->append_conf('postgresql.conf', + "shared_preload_libraries = 'validator'\n"); # Needed to inspect postmaster log after connection failure: $node->append_conf('postgresql.conf', "log_min_messages = debug2"); $node->start; @@ -115,6 +117,55 @@ test( expected_stdout => qr/connection succeeded/, log_like => [qr/oauth_validator: token="my-token", role="$user"/]); +# Test custom HBA option parsing hook +my $log_start_custom = -s $node->logfile; +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="$scope" test_custom_claim="my_custom_value" +}); +$node->reload; +$node->wait_for_log(qr/reloading configuration files/, $log_start_custom); + +$node->wait_for_log( + qr/oauth_validator: parsed custom HBA option test_custom_claim="my_custom_value"/, + $log_start_custom); + +test( + "custom HBA option is parsed and used", + flags => [ + "--token", "test-token", + "--expected-uri", "$issuer/.well-known/openid-configuration", + "--expected-scope", $scope, + ], + expected_stdout => qr/connection succeeded/, + log_like => [qr/oauth_validator: custom_claim="my_custom_value"/]); + +# Test that unknown HBA options still fail +my $log_start_unknown = -s $node->logfile; +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="$scope" unknown_option="value" +}); +$node->reload; +$node->wait_for_log(qr/reloading configuration files/, $log_start_unknown); + +# Check that the server logged the error about the unknown option +$node->wait_for_log( + qr/unrecognized authentication option name: "unknown_option"/, + $log_start_unknown); +pass("unknown HBA option is rejected"); + +# Restore working configuration +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="$scope" +}); +$node->reload; +$node->wait_for_log(qr/reloading configuration files/); + if ($ENV{with_libcurl} ne 'yes') { # libpq should help users out if no OAuth support is built in. diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 42b69646fbb..0115f228cea 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -14,6 +14,7 @@ #include "postgres.h" #include "fmgr.h" +#include "libpq/hba.h" #include "libpq/oauth.h" #include "miscadmin.h" #include "utils/guc.h" @@ -41,6 +42,50 @@ static const OAuthValidatorCallbacks validator_callbacks = { static char *authn_id = NULL; static bool authorize_tokens = true; +static char *custom_claim = NULL; + +static hba_parse_option_hook_type prev_hba_parse_option_hook = NULL; + +static bool +validator_hba_parse_option(const char *name, const char *val, + HbaLine *hbaline, int elevel, char **err_msg) +{ + int line_num = hbaline->linenumber; + char *file_name = hbaline->sourcefile; + + if (prev_hba_parse_option_hook) + { + if ((*prev_hba_parse_option_hook) (name, val, hbaline, + elevel, err_msg)) + return true; + } + + if (strcmp(name, "test_custom_claim") == 0) + { + if (val == NULL || val[0] == '\0') + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("test_custom_claim requires a value"), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + *err_msg = pstrdup("test_custom_claim requires a value"); + return false; + } + + if (custom_claim) + pfree(custom_claim); + custom_claim = pstrdup(val); + + elog(LOG, "oauth_validator: parsed custom HBA option test_custom_claim=\"%s\"", + custom_claim); + + return true; + } + + return false; +} + /*--- * Extension entry point. Sets up GUCs for use by tests: * @@ -55,6 +100,8 @@ static bool authorize_tokens = true; void _PG_init(void) { + elog(LOG, "oauth_validator: _PG_init() called, installing HBA parse option hook"); + DefineCustomStringVariable("oauth_validator.authn_id", "Authenticated identity to use for future connections", NULL, @@ -73,6 +120,9 @@ _PG_init(void) NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); + + prev_hba_parse_option_hook = hba_parse_option_hook; + hba_parse_option_hook = validator_hba_parse_option; } /* @@ -133,6 +183,9 @@ validate_token(const ValidatorModuleState *state, MyProcPort->hba->oauth_issuer, MyProcPort->hba->oauth_scope); + if (custom_claim) + elog(LOG, "oauth_validator: custom_claim=\"%s\"", custom_claim); + res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); -- 2.43.0 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-18 17:27 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Jacob Champion @ 2025-12-18 17:27 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Thu, Dec 18, 2025 at 1:08 AM Zsolt Parragi <[email protected]> wrote: > > It however requires shared_preload_libraries (that is common > for all options), maybe oauth_validator_libraries could imply that? Haven't looked at the patch yet, but I think most people probably want to use session_preload_libraries, not shared_preload_libraries, so that a security update to their validator doesn't require a restart of the cluster. If a particular validator implementation requires shared preload, so be it; but I don't think we want to force it. Might be more reason to look into the GUC system? --Jacob ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-18 18:28 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Zsolt Parragi @ 2025-12-18 18:28 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > Might be more reason to look into the GUC system? I am already thinking about that, I have some ideas for a proof of concept, but no working prototype yet. But without requiring shared_preload_libraries, we can't do early error reporting during postmaster startup about custom parameters. Is that okay? GUCs already work this way, and this could be a bit safer (reporting unknown parameters/refusing to proceed during login, when we can completely parse all parameters), but it would be different compared to how pg_hba is handled currently. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-18 18:43 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Jacob Champion @ 2025-12-18 18:43 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Thu, Dec 18, 2025 at 10:28 AM Zsolt Parragi <[email protected]> wrote: > But without requiring > shared_preload_libraries, we can't do early error reporting during > postmaster startup about custom parameters. Is that okay? I think I need to do more staring at the intersection of GUC registration and session_preload_libraries, because my memory of the order of operations was faulty. I won't be able to do that before the holidays, most likely. --Jacob ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2025-12-18 20:29 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Zsolt Parragi @ 2025-12-18 20:29 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > I think I need to do more staring at the intersection of GUC > registration and session_preload_libraries, because my memory of the > order of operations was faulty. I won't be able to do that before the > holidays, most likely. Maybe I'm missing something, but why do we need session_preload_libraries? oauth_validator_libraries is processed earlier, it can already define sighup GUCs, it should also work with a new level around that. I assume that if postgres gets another authentication plugin point later, it will be executed around the same place, during authentication, so that also shouldn't be an issue. The question is if non-validator libraries should be able to define PGC_HBA variables. If yes, then either * we don't validate that all HBA variables are valid - if somebody made a typo, we can't detect it * we add a sighup guc with a manual whitelist * require shared preload libraries or oauth_validator_libraries, because those are loaded before or during authentication * require session_preload_libraries. We proceed with authentication even with unresolved HBA variables, but abort the connection if there are still unknown parameters after loading session preload. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-02-04 11:42 Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Zsolt Parragi @ 2026-02-04 11:42 UTC (permalink / raw) To: Nikolay Shaplov <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Jacob Champion <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] I've looked into the patch in more detail. (I will post a review later to that thread, I have some notes I have to format properly) As for this use case, we could use this (for my original B or C options), but I see a potential problem: First, we either only use this code to feed the unknown parameters to the options parser, and keep the existing hba options parser as is for the hardcoded parameters. Or also include the fixed options in it, so that everything works exactly the same. Then we either make it limited to oauth validators, or try to keep it generic for any session_preload_libraries. If we only use this for unknown options, and limit it to oauth validators, then options.h/c could work as is. If we want to implement anything more generic, we'll face issues, as the current API only supports validating the input once. In the most generic case: * Parsing the core hba options in postmaster * Validating core options, ignoring unknowns * Loading the oauth validator * Validating options again, as the validator needs its custom options - having unknown remaining options is still valid * Running the validator * Loading session preload libraries * Validating options again - unknown options are an error now So up to 3 times, and it also needs a way for extensions to edit spec sets. (In the simple case, only the validator has to know about that) I think this makes this impractical for the more complex applications, but if we want to go back to the minimal original concept, limited to oauth validators, it could work. I'm also not sure how useful this non-GUC API would be for extensions other than validators, which also tells me that this version should be limited to the validators. Also, this isn't an approach with an easy way to convert it into PGC_HBA, as it requires a clearly different api in the validator - I don't see a nice way to do that without changing the API used by the validators. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-03-20 17:46 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Jacob Champion @ 2026-03-20 17:46 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] Hi all, First, an apology for the state of this thread -- I thought I had already responded to the prior message last month. Turns out, I had not... Instead, I'm dropping a surprise alternative patchset on top of a thread that I let go cold, which is rude, and I'm sorry. Here are my overall thoughts on the approaches shared so far. And thank you, Zsolt, for doing so much legwork here; that effort was not wasted at all. 1) A GUC-centric solution -- option (d) -- is the Ideal Solution here, IMO. We shouldn't have to reinvent the wheel. 2) A GUC-centric solution is not going to land for 19, and I'd be surprised if it landed for 20, given the coalition that I'll need to build to say either "yes" or "no". There are too many questions about session_preload_libraries and prefixing and etc., and I'm honestly not a fan of the guc_prefix_enforcement approach. 3) I'm not convinced yet that GUCs and relation options are similar enough to share a framework. (This is not a rejection of the reloption refactoring work, just a statement that I don't want to rely on it to solve this problem.) 4) I really don't like the hba_parse_option_hook. I prefer APIs over hooks as a general architectural point, and more practically, I don't want to hand control to extensions during HBA parsing. I don't think they're going to coordinate with each other well, and I think they're likely to couple against internals in ways we don't want to support (which is my general problem with hooks). = Option (b) = I don't want this problem to go unfixed for 2+ years, so I think it would be best to reinvent a very small wheel that doesn't take up a lot of maintenance effort once it's in, and then simply replace it with the Ideal Solution eventually. This is my take on option (b), which is what Vasuki M advocated for upthread. It's just a setter/getter API for string keys and values: static const char* opts[] = { "my_parameter" }; void startup_cb(ValidatorModuleState *state) { RegisterOAuthHBAOptions(state, lengthof(opts), opts); } bool validate_cb(const ValidatorModuleState *state, ...) { const char *param; if ((param = GetOAuthHBAOption(state, "my_parameter")) != NULL) do_something_with(param); ... } And then in your HBA: host all all ::1/128 oauth validator.my_parameter=foo ... The core implementation is conceptually simple. Most of the lines in the patch are guardrails, to reduce the probability of regret over this temporary solution: - Parameter names aren't freeform; they're restricted to almost-alphanumeric identifiers. (We've wanted to steal symbols for HBA features in the past.) - Name syntax is checked on reload, but cross-referencing the registered list must be deferred to connection time. That's unusual for users, so the WARNING that gets printed in this case is extremely verbose; that way hopefully no one will be confused about what is happening. - The compiler won't let you register names during validate_cb, and we won't let you retrieve options during startup_cb, so we retain control over the internal order of operations. (This is discussed more deeply in the patch.) I was worried that we'd need a third API call to report option parsing failures from the validator. Instead of doing that, I've made it easier to link an authentication failure to a validator internal error that caused it, in v3-0001, which serendipitously fixes a separate sharp edge [1]. I feel good about that patch even if -0002 doesn't make it. v3-0002 still lacks user documentation, which I need to write a lot of -- but if everyone dislikes this approach then I'd rather not spend the time there. = WDYT? = I realize this puts you all in a difficult position; the effect is kind of "take it or leave it", which again was not my intent. I considered letting this lapse for 19 instead. But since I believe the ideal solution is one we can't have for a while, and there's good research and discussion of alternatives in this thread, waiting may not produce a more committable short-term solution in the end. Let me know if any of you disagree, though -- especially if you think the status quo is preferable! -- and I can shelve -0002 for now. (I'll continue with -0001 at [1] either way.) Thanks, --Jacob [1] https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql 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 [application/octet-stream] v3-0002-WIP-oauth-Allow-validators-to-register-custom-HBA.patch (20.0K, 3-v3-0002-WIP-oauth-Allow-validators-to-register-custom-HBA.patch) download | inline diff: From 9726ac39442dd3af479903bfab62894765a6ab2f Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Tue, 17 Mar 2026 12:01:28 -0700 Subject: [PATCH v3 2/2] WIP: oauth: Allow validators to register custom HBA options (lacks user documentation) Two new API entry points for validator callbacks: - RegisterOAuthHBAOptions - GetOAuthHBAOption Registering options "foo" and "bar" allows a user to set validator.foo and validator.bar on an `oauth` HBA line. The bulk of the patch is not the conceptually simple API implementation, but guardrails on the simple API to make sure it doesn't bind our hands in the future, either for callback architecture or HBA syntax. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> --- src/include/libpq/hba.h | 2 + src/include/libpq/oauth.h | 15 +- src/backend/libpq/auth-oauth.c | 225 ++++++++++++++++++ src/backend/libpq/hba.c | 25 ++ .../modules/oauth_validator/t/001_server.pl | 97 ++++++++ src/test/modules/oauth_validator/validator.c | 48 +++- 6 files changed, 407 insertions(+), 5 deletions(-) diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index c4570ce9b3f..e8898561c8c 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -140,6 +140,8 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *oauth_opt_keys; + List *oauth_opt_vals; } HbaLine; typedef struct IdentLine diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 60f493acddd..86f463a284e 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -96,6 +96,17 @@ typedef struct OAuthValidatorCallbacks ValidatorValidateCB validate_cb; } OAuthValidatorCallbacks; +/* + * A validator can register a list of custom option names during its startup_cb, + * then later retrieve the user settings for each during validation. This + * enables per-HBA-line configuration. For more information, refer to the OAuth + * validator modules documentation. + */ +extern void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +extern const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); + /* * Type of the shared library symbol _PG_oauth_validator_module_init which is * required for all validator modules. This function will be invoked during @@ -107,9 +118,7 @@ extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_ini /* Implementation */ extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech; -/* - * Ensure a validator named in the HBA is permitted by the configuration. - */ extern bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg); +extern bool valid_oauth_hba_option_name(const char *name); #endif /* PG_OAUTH_H */ diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 6d4be70aada..032b49e1743 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -25,6 +25,7 @@ #include "libpq/hba.h" #include "libpq/oauth.h" #include "libpq/sasl.h" +#include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "utils/json.h" @@ -40,10 +41,15 @@ static int oauth_exchange(void *opaq, const char *input, int inputlen, static void load_validator_library(const char *libname); static void shutdown_validator_library(void *arg); +static bool check_validator_hba_options(Port *port, const char **logdetail); static ValidatorModuleState *validator_module_state; static const OAuthValidatorCallbacks *ValidatorCallbacks; +static MemoryContext ValidatorMemoryContext; +static List *ValidatorOptions; +static bool ValidatorOptionsChecked; + /* Mechanism declaration */ const pg_be_sasl_mech pg_be_oauth_mech = { .get_mechanisms = oauth_get_mechanisms, @@ -108,6 +114,9 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("client selected an invalid SASL authentication mechanism")); + /* Save our memory context for later use by client API calls. */ + ValidatorMemoryContext = CurrentMemoryContext; + ctx = palloc0_object(struct oauth_ctx); ctx->state = OAUTH_STATE_INIT; @@ -279,6 +288,16 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); + /* + * Make sure all custom HBA options are understood by the validator before + * continuing, since we couldn't check them during server start/reload. + */ + if (!check_validator_hba_options(ctx->port, logdetail)) + { + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; + } + if (!validate(ctx->port, auth, logdetail)) { generate_error_response(ctx, output, outputlen); @@ -807,6 +826,9 @@ shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) ValidatorCallbacks->shutdown_cb(validator_module_state); + + /* The backing memory for this is about to disappear. */ + ValidatorOptions = NIL; } /* @@ -892,3 +914,206 @@ done: return (*err_msg == NULL); } + +/* + * Client APIs for validator implementations + * + * Since we're currently not threaded, we only allow one validator in the + * process at a time. So we can make use of globals for now instead of looking + * up information using the state pointer. We probably shouldn't assume that the + * module hasn't temporarily changed memory contexts on us, though; functions + * here should defensively use an appropriate context when making global + * allocations. + */ + +/* + * Adds to the list of allowed validator.* HBA options. Used during the + * startup_cb. + */ +void +RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]) +{ + MemoryContext oldcontext; + + if (!state) + { + Assert(false); + return; + } + + oldcontext = MemoryContextSwitchTo(ValidatorMemoryContext); + + for (int i = 0; i < num; i++) + { + if (!valid_oauth_hba_option_name(opts[i])) + { + /* + * The user can't set this option in the HBA, so GetOAuthHBAOption + * would always return NULL. + */ + ereport(WARNING, + errmsg("HBA option name \"%s\" is invalid and will be ignored", + opts[i]), + /* translator: the second %s is a function name */ + errcontext("validator module \"%s\", in call to %s", + MyProcPort->hba->oauth_validator, + "RegisterOAuthHBAOptions")); + continue; + } + + ValidatorOptions = lappend(ValidatorOptions, pstrdup(opts[i])); + } + + MemoryContextSwitchTo(oldcontext); + + /* + * Wait to validate the HBA against the registered options until later + * (see check_validator_hba_options()). + * + * Delaying allows the validator to make multiple registration calls, to + * append to the list; it lets us make the check in a place where we can + * report the error without leaking details to the client; and it avoids + * exporting the order of operations between HBA matching and the + * startup_cb call as an API guarantee. (The last issue may become + * relevant with a threaded model.) + */ +} + +/* + * Restrict the names available to custom HBA options, so that we don't + * accidentally prevent future syntax extensions to HBA files. + */ +bool +valid_oauth_hba_option_name(const char *name) +{ + /* + * This list is not incredibly principled, since the goal is just to bound + * compatibility guarantees for our HBA parser. Alphanumerics seem + * obviously fine, and it's difficult to argue against the punctuation + * that's already included in some HBA option names and identifiers. + */ + static const char *name_allowed_set = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_-"; + + size_t span; + + if (!name[0]) + return false; + + span = strspn(name, name_allowed_set); + return name[span] == '\0'; +} + +/* + * Verifies that all validator.* HBA options specified by the user were actually + * registered by the validator library in use. + */ +static bool +check_validator_hba_options(Port *port, const char **logdetail) +{ + HbaLine *hba = port->hba; + + foreach_ptr(char, key, hba->oauth_opt_keys) + { + bool found = false; + + /* O(n^2) shouldn't be a problem here in practice. */ + foreach_ptr(char, optname, ValidatorOptions) + { + if (strcmp(key, optname) == 0) + { + found = true; + break; + } + } + + if (!found) + { + /* + * Bad option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * + * Since this is affecting live connections, which is unusual for + * HBA, be noisy with a WARNING. (Warnings aren't sent to clients + * prior to successful authentication, so this won't disclose the + * server config.) It'll duplicate some of the information in the + * logdetail, but that should make it hard to miss the connection + * between the two. + */ + char *name = psprintf("validator.%s", key); + + *logdetail = psprintf(_("unrecognized authentication option name: \"%s\""), + name); + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + /* translator: the first %s is the name of the module */ + errdetail("The installed validator module (\"%s\") did not define an option named \"%s\".", + hba->oauth_validator, key), + errhint("All OAuth connections matching this line will fail. Correct the option and reload the server configuration."), + errcontext("line %d of configuration file \"%s\"", + hba->linenumber, hba->sourcefile)); + + return false; + } + } + + ValidatorOptionsChecked = true; /* unfetter GetOAuthHBAOption() */ + return true; +} + +/* + * Retrieves the setting for a validator.* HBA option, or NULL if not found. + * This may only be used during the validate_cb and shutdown_cb. + */ +const char * +GetOAuthHBAOption(const ValidatorModuleState *state, const char *optname) +{ + HbaLine *hba = MyProcPort->hba; + ListCell *lc_k; + ListCell *lc_v; + const char *ret = NULL; + + if (!ValidatorOptionsChecked) + { + /* + * Prevent the startup_cb from retrieving HBA options that it has just + * registered. This probably seems strange -- why refuse to hand out + * information we already know? -- but this lets us reserve the + * ability to perform the startup_cb call earlier, before we know + * which HBA line is matched by a connection, without breaking this + * API. + */ + return NULL; + } + + if (!state || !hba) + { + Assert(false); + return NULL; + } + + Assert(list_length(hba->oauth_opt_keys) == list_length(hba->oauth_opt_vals)); + + forboth(lc_k, hba->oauth_opt_keys, lc_v, hba->oauth_opt_vals) + { + const char *key = lfirst(lc_k); + const char *val = lfirst(lc_v); + + if (strcmp(key, optname) == 0) + { + /* + * Don't return yet -- when regular HBA options are specified more + * than once, the last one wins. Do the same for these options. + */ + ret = val; + } + } + + return ret; +} diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..0569e8fced8 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2497,6 +2497,31 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); hbaline->oauth_validator = pstrdup(val); } + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); + + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be + * loaded into the postmaster, we don't know if the options are valid + * yet and must store them for later. Perform only a basic syntax + * check here. + */ + if (!valid_oauth_hba_option_name(key)) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("invalid OAuth validator option name: \"%s\"", name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + return false; + } + + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } else if (strcmp(name, "delegate_ident_mapping") == 0) { REQUIRE_AUTH_OPTION(uaOAuth, "delegate_ident_mapping", "oauth"); diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index 28d123b833e..0756e6038c4 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -579,10 +579,29 @@ $node->connect_fails( $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); + +# We complain when bad option names are registered, but connections may proceed +# (since users can't set those options in the HBA anyway). +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authn_id"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authorize_tokens"); +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.invalid_hba TO true"); + $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$node->connect_ok( + "$common_connstr user=test", + "bad registered HBA option", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/WARNING:\s+HBA option name "bad option name" is invalid and will be ignored/, + qr/CONTEXT:\s+validator module "validator", in call to RegisterOAuthHBAOptions/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.invalid_hba"); + # # Test user mapping. # @@ -651,6 +670,84 @@ $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$bgconn->quit; # the tests below restart the server + +# +# Test validator-specific HBA options. +# + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="openid postgres" delegate_ident_mapping=1 \\ + validator.authn_id="ignored" validator.authn_id="other-identity" +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.log="testalt message" +}); + +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_ok( + "$common_connstr user=test", + "custom HBA setting (test)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [qr/connection authenticated: identity="other-identity"/]); +$node->connect_ok( + "$common_connstr user=testalt", + "custom HBA setting (testalt)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/LOG:\s+testalt message/, + qr/connection authenticated: identity="testalt"/, + ]); + +# bad syntax +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("empty HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\."/]); + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.@@=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("invalid HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\.@@"/]); + +# unknown settings (validation is deferred to connect time) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" \\ + validator.log=ignored validator.bad=1 +}); +$node->restart; + +$node->connect_fails( + "$common_connstr user=testalt", + "bad HBA setting", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+unrecognized authentication option name: "validator\.bad"/, + qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+unrecognized authentication option name: "validator\.bad"/, + ]); + # # Test multiple validators. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 353e0e0d32a..85fb4c08bf2 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -42,13 +42,21 @@ static char *authn_id = NULL; static bool authorize_tokens = true; static char *error_detail = NULL; static bool internal_error = false; +static bool invalid_hba = false; + +/* HBA options */ +static const char *hba_opts[] = { + "authn_id", /* overrides the default authn_id */ + "log", /* logs an arbitrary string */ +}; /*--- * Extension entry point. Sets up GUCs for use by tests: * * - oauth_validator.authn_id Sets the user identifier to return during token * validation. Defaults to the username in the - * startup packet. + * startup packet, or the validator.authn_id HBA + * option if it is set. * * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming @@ -96,6 +104,14 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.invalid_hba", + "Should the validator register an invalid option?", + NULL, + &invalid_hba, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -124,6 +140,29 @@ validator_startup(ValidatorModuleState *state) if (state->sversion != PG_VERSION_NUM) elog(ERROR, "oauth_validator: sversion set to %d", state->sversion); + /* + * Test the behavior of custom HBA options. Registered options should not + * be retrievable during startup (we want to discourage modules from + * relying on the relative order of client connections and the + * startup_cb). + */ + RegisterOAuthHBAOptions(state, lengthof(hba_opts), hba_opts); + for (int i = 0; i < lengthof(hba_opts); i++) + { + if (GetOAuthHBAOption(state, hba_opts[i])) + elog(ERROR, + "oauth_validator: GetOAuthValidatorOption(\"%s\") was non-NULL during startup_cb", + hba_opts[i]); + } + + if (invalid_hba) + { + /* Register a bad option, which should print a WARNING to the logs. */ + const char *invalid = "bad option name"; + + RegisterOAuthHBAOptions(state, 1, &invalid); + } + state->private_data = PRIVATE_COOKIE; } @@ -141,7 +180,7 @@ validator_shutdown(ValidatorModuleState *state) /* * Validator implementation. Logs the incoming data and authorizes the token by - * default; the behavior can be modified via the module's GUC settings. + * default; the behavior can be modified via the module's GUC and HBA settings. */ static bool validate_token(const ValidatorModuleState *state, @@ -153,6 +192,9 @@ validate_token(const ValidatorModuleState *state, elog(ERROR, "oauth_validator: private state cookie changed to %p in validate", state->private_data); + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + elog(LOG, "oauth_validator: token=\"%s\", role=\"%s\"", token, role); elog(LOG, "oauth_validator: issuer=\"%s\", scope=\"%s\"", MyProcPort->hba->oauth_issuer, @@ -165,6 +207,8 @@ validate_token(const ValidatorModuleState *state, res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); + else if (GetOAuthHBAOption(state, "authn_id")) + res->authn_id = pstrdup(GetOAuthHBAOption(state, "authn_id")); else res->authn_id = pstrdup(role); -- 2.34.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-03-23 21:45 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 2 replies; 25+ messages in thread From: Zsolt Parragi @ 2026-03-23 21:45 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > I considered letting this lapse for 19 instead That was also my conclusion. After the discussion in the SNI thread I started working on a PoC for a more modern syntax for hba/ident/hosts, hoping that a generic extensibility/guc patch could be based on that. I also didn't want to start a thread about this before the feature freeze, so I'm still waiting/prototyping for a few weeks. I'm also not against adding an oauth-only feature for 19, that was my original intention before getting completely distracted by the guc direction :) + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); This is my only concern with this patch: since we have a list separated validatr names as a GUC already, couldn't we require a <validator_name>. prefix instead of the fixed "validator.", to keep the hba configuration consistent with gucs? Validators would still have to handle these options differently, but at least it would look consistent from the user perspective - global setting in postgresql.conf, same hba-line specific override in pg_hba.conf. (also, validators already added global GUCs in pg18, and this would also keep it consistent with that) + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); Shouldn't this check go before the name validation? ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-03-27 23:03 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 1 sibling, 1 reply; 25+ messages in thread From: Jacob Champion @ 2026-03-27 23:03 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Mar 23, 2026 at 2:45 PM Zsolt Parragi <[email protected]> wrote: > This is my only concern with this patch: since we have a list > separated validatr names as a GUC already, couldn't we require a > <validator_name>. prefix instead of the fixed "validator.", to keep > the hba configuration consistent with gucs? Well, the `validator.` prefix lets us end-run the namespace issue [1]. It's one thing if I claim that single prefix in parse_hba_auth_opt(); it's another thing if I camp out on literally every identifier containing a dot. I'm also not convinced that it's worth spending additional code here to decide _which_ of the blessed validators is in force for the current line. (Deferring the check of the option names is bad enough, but there appears to be no way around that.) > Validators would still have to handle these options differently, but > at least it would look consistent from the user perspective - global > setting in postgresql.conf, same hba-line specific override in > pg_hba.conf. (also, validators already added global GUCs in pg18, and > this would also keep it consistent with that) After the wild goose chase I sent you on, I think consistency-in-form-but-not-function is more likely to be a liability than a benefit. Sure, validator authors will be able to pretend that users can override particular GUCs per-line, but that's not what's actually happening, so that could increase user confusion and support burden for very little practical upside. (As one example, `SHOW my_validator.setting` isn't going to behave intuitively.) Since my pitch here is "this is an architectural dead end, but it'll get us moving while we pursue the better route," I prefer something that's very obviously bespoke. Especially since validators will have to migrate from the old way to the new way, if we get our wish. I don't really want anyone to spend time resolving the collision of the two behaviors; I'd rather just let the old ugly configuration solution wither (or die), and encourage everyone to switch as rapidly as possible. > + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); > > Shouldn't this check go before the name validation? Yeah, I agree. (My original code had a more generic error message when the name check failed, but now that the message is OAuth-specific, I don't think it makes sense to pretend that it could belong to any other auth method.) Thanks! --Jacob [1] https://postgr.es/m/CAOYmi%2Bn9%2BVDNayxsZuG30YLxOXrVB2Wu%3DjBR4WrEdJvxjTATKw%40mail.gmail.com ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-03-27 23:03 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 1 sibling, 0 replies; 25+ messages in thread From: Jacob Champion @ 2026-03-27 23:03 UTC (permalink / raw) To: Daniel Gustafsson <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Tue, Mar 24, 2026 at 5:29 AM Daniel Gustafsson <[email protected]> wrote: > FWIW I support this line of investigation and look forward to seeing what it > could look like. +many --Jacob ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-03-30 21:46 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Zsolt Parragi @ 2026-03-30 21:46 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > (As one example, `SHOW > my_validator.setting` isn't going to behave intuitively.) Right, I didn't think about that scenario, it might be better to keep this intentionally different. That won't be the most user friendly option, but it is still definitely better than not having the ability to configure this. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-03-30 23:54 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Jacob Champion @ 2026-03-30 23:54 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Mar 30, 2026 at 2:46 PM Zsolt Parragi <[email protected]> wrote: > That won't be the most user friendly option, but it is still > definitely better than not having the ability to configure this. Sounds good. Barring other objections, then, I'll plan to move forward with this approach for PG19. Thanks! --Jacob ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-04-02 21:26 Jacob Champion <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Jacob Champion @ 2026-04-02 21:26 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Mar 30, 2026 at 4:54 PM Jacob Champion <[email protected]> wrote: > Sounds good. Barring other objections, then, I'll plan to move forward > with this approach for PG19. v4-0002 (based on top of [1]) addresses Zsolt's feedback and adds the missing user documentation. I believe this is now a complete patch proposal; tear it apart. :D Thanks, --Jacob [1] https://postgr.es/m/CAOYmi%2BmvFS7Ukmacb1z%3DxWO7M%2BDPF41GZsJiJ6sh4U1Qm_yWOA%40mail.gmail.com 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 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]> 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] v4-0002-oauth-Allow-validators-to-register-custom-HBA-opt.patch (33.9K, 3-v4-0002-oauth-Allow-validators-to-register-custom-HBA-opt.patch) download | inline diff: From 5039ce74370bca19f133bac29fcc220e24cb9b20 Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Tue, 17 Mar 2026 12:01:28 -0700 Subject: [PATCH v4 2/2] oauth: Allow validators to register custom HBA options OAuth validators can already use custom GUCs to configure behavior globally, but we currently provide no ability to adjust settings for individual HBA entries, because the original design focused on a world where a provider covered a "single audience" of users for one database cluster. This assumption does not apply to multitenant use cases, where a single validator may be controlling access for wildly different user groups. To improve this use case, add two new API calls for use by validator callbacks: RegisterOAuthHBAOptions() and GetOAuthHBAOption(). Registering options "foo" and "bar" allows a user to set "validator.foo" and "validator.bar" in an oauth HBA entry. These options are stringly typed (syntax validation is solely the responsibility of the defining module), and names are restricted to a subset of ASCII to avoid tying our hands with future HBA syntax improvements. Unfortunately, we can't check the custom option names during a reload of the configuration, like we do with standard HBA options, unless we were to require all validators to be loaded via shared_preload_libraries. (I consider this to be a nonstarter: most validators should probably use session_preload_libraries at most, since requiring a full restart of a production server just to update authentication behavior will be unacceptable to many users.) Instead, the new validator.* options are checked against the registered list at connection time. Multiple alternatives were proposed and/or prototyped, including extending the GUC system to allow per-HBA overrides, joining forces with recent refactoring work on the reloptions subsystem, and giving the ability to customize HBA options to all PostgreSQL extensions. I personally believe per-HBA GUC overrides are the best option, because several existing GUCs like authentication_timeout and pre_auth_delay would fit there usefully. But the recent addition of SNI per-host settings in 4f433025f indicates that a more general solution is needed, and I expect that to take multiple releases' worth of discussion. This compromise patch, then, is intentionally designed to be an architectural dead end: simple to describe, cheap to maintain, and providing just enough functionality to let validators move forward for PG19. The hope is that it will be replaced in the future by a solution that can handle per-host, per-HBA, and other per-context configuration with the same functionality that GUCs provide today. In the meantime, the bulk of the code in this patch consists of strict guardrails on the simple API, to try to ensure that we don't have any reason to regret its existence during its unknown lifespan. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> Reviewed-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com --- doc/src/sgml/client-auth.sgml | 39 +++ doc/src/sgml/oauth-validators.sgml | 221 ++++++++++++++++- src/include/libpq/hba.h | 2 + src/include/libpq/oauth.h | 15 +- src/backend/libpq/auth-oauth.c | 225 ++++++++++++++++++ src/backend/libpq/hba.c | 26 ++ .../modules/oauth_validator/t/001_server.pl | 97 ++++++++ src/test/modules/oauth_validator/validator.c | 48 +++- 8 files changed, 667 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index a347ee18980..bca09a80416 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -2532,6 +2532,45 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" </listitem> </varlistentry> + <varlistentry> + <term id="auth-oauth-validator-option"> + <literal>validator.<replaceable>option</replaceable></literal> + </term> + <listitem> + <para> + Validator modules may <link linkend="oauth-validator-hba">define</link> + additional configuration options for <literal>oauth</literal> + HBA entries. These validator-specific options are accessible via the + <literal>validator.*</literal> "namespace". For example, a module may + register the <literal>validator.foo</literal> and + <literal>validator.bar</literal> options and define their effects on + authentication. + </para> + <para> + The name, syntax, and behavior of each <replaceable>option</replaceable> + are not determined by <productname>PostgreSQL</productname>; consult the + documentation for the validator module in use. + </para> + <warning> + <para> + A limitation of the current implementation is that unrecognized + <replaceable>option</replaceable> names will not be caught until + connection time. A <literal>pg_ctl reload</literal> will succeed, but + matching connections will fail: +<programlisting> +LOG: connection received: host=[local] +WARNING: unrecognized authentication option name: "validator.bad" +DETAIL: The installed validator module ("my_validator") did not define an option named "bad". +HINT: All OAuth connections matching this line will fail. Correct the option and reload the server configuration. +CONTEXT: line 2 of configuration file "data/pg_hba.conf" +</programlisting> + Use caution when making changes to validator-specific HBA options in + production systems. + </para> + </warning> + </listitem> + </varlistentry> + <varlistentry> <term><literal>map</literal></term> <listitem> diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index 5f29f2be186..7c9e3dd931a 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -251,6 +251,11 @@ <symbol>delegate_ident_mapping=1</symbol> mode, and what additional configuration is required in order to do so. </para> + <para> + If an implementation provides <link linkend="oauth-validator-hba">custom + HBA options</link>, the names and syntax of those options should be + documented as well. + </para> </listitem> </varlistentry> </variablelist> @@ -343,7 +348,8 @@ typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); <title>Startup Callback</title> <para> The <function>startup_cb</function> callback is executed directly after - loading the module. This callback can be used to set up local state and + loading the module. This callback can be used to set up local state, + define <link linkend="oauth-validator-hba">custom HBA options</link>, and perform additional initialization if required. If the validator module has state it can use <structfield>state->private_data</structfield> to store it. @@ -432,4 +438,217 @@ typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); </sect2> </sect1> + + <sect1 id="oauth-validator-hba"> + <title>Custom HBA Options</title> + + <para> + Like other preloaded libraries, validator modules may define + <link linkend="runtime-config-custom">custom GUC parameters</link> for user + configuration in <filename>postgresql.conf</filename>. However, it may be + desirable to configure behavior at a more granular level (say, for a + particular issuer or a group of users) instead of globally. + </para> + + <para> + Beginning in <productname>PostgreSQL</productname> 19, validator + implementations may define custom options for use inside + <filename>pg_hba.conf</filename>. These options are then + <link linkend="auth-oauth-validator-option">made available</link> to the user + as <literal>validator.<replaceable>option</replaceable></literal>. The API + for registering and retrieving custom options is described below. + </para> + + <sect2 id="oauth-validator-hba-api"> + <title>Options API</title> + <para> + Modules register custom HBA option names during the <function>startup_cb</function> + callback, using <function>RegisterOAuthHBAOptions()</function>: + +<programlisting> +/* + * Register a list of custom option names for use in pg_hba.conf. For each name + * "foo" registered here, that option will be provided as "validator.foo" in + * the HBA. + * + * Valid option names consist of alphanumeric ASCII, underscore (_), and hyphen + * (-). Invalid option names will be ignored with a WARNING logged at + * connection time. + * + * This function may only be called during the startup_cb callback. Multiple + * calls are permitted, which will append to the existing list of registered + * options; options cannot be unregistered. + * + * Parameters: + * + * - state: the state pointer passed to the startup_cb callback + * - num: the number of options in the opts array + * - opts: an array of null-terminated option names to register + * + * The list of option names is copied internally, and the opts array is not + * required to remain valid after the call. + */ +void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +</programlisting> + </para> + + <para> + Each option's value, if set, may be later retrieved using + <function>GetOAuthHBAOption()</function>: + +<programlisting> +/* + * Retrieve the string value of an HBA option which was registered via + * RegisterOAuthHBAOptions(). Usable only during validate_cb or shutdown_cb. + * + * If the user has set the corresponding option in pg_hba.conf, this function + * returns that value as a null-terminated string, which must not be modified + * or freed. NULL is returned instead if the user has not set this option, if + * the option name was not registered, or if this function is incorrectly called + * during the startup_cb. + * + * Parameters: + * + * - state: the state pointer passed to the validate_cb/shutdown_cb callback + * - optname: the name of the option to retrieve + */ +const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); +</programlisting> + </para> + + <para> + See <xref linkend="oauth-validator-hba-example-usage"/> for sample usage. + </para> + </sect2> + + <sect2 id="oauth-validator-hba-limitations"> + <title>Limitations</title> + <para> + <itemizedlist> + <listitem> + <para> + Option names are limited to ASCII alphanumeric characters, + underscores (<literal>_</literal>), and hyphens (<literal>-</literal>). + </para> + </listitem> + <listitem> + <para> + Option values are always freeform strings (in contrast to custom GUCs, + which support numerics, booleans, and enums). + </para> + </listitem> + <listitem> + <para> + Option names and values cannot be checked by the server during a reload of + the configuration. Any unregistered options in <filename>pg_hba.conf</filename> + will instead result in connection failures. It is the responsibility of + each module to document and verify the syntax of option values as needed. + <footnote> + <para> + If a module finds an invalid option value during <function>validate_cb</function>, + it's recommended to <link linkend="oauth-validator-callback-validate">signal + an internal error</link> by setting <structfield>result->error_detail</structfield> + to a description of the problem and returning <literal>false</literal>. + </para> + </footnote> + </para> + </listitem> + </itemizedlist> + </para> + </sect2> + + <sect2 id="oauth-validator-hba-example-usage"> + <title>Example Usage</title> + + <para> + For a hypothetical module, the options <literal>foo</literal> and + <literal>bar</literal> could be registered as follows: + +<programlisting> +static void +validator_startup(ValidatorModuleState *state) +{ + static const char *opts[] = { + "foo", /* description of access privileges */ + "bar", /* magic URL for additional administrator powers */ + }; + + RegisterOAuthHBAOptions(state, lengthof(opts), opts); + + /* ...other setup... */ +} +</programlisting> + </para> + + <para> + The following sample entries in <filename>pg_hba.conf</filename> can then + make use of these options: + +<programlisting> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.com \ + scope="pg-user openid email" \ + map=oauth-email \ + validator.foo="user access" +</programlisting> + </para> + + <para> + The module can retrieve the option settings from the HBA during validation: + +<programlisting> +static bool +validate_token(const ValidatorModuleState *state, + const char *token, const char *role, + ValidatorModuleResult *res) +{ + const char *foo = GetOAuthHBAOption(state, "foo"); /* "admin access" or "user access" */ + const char *bar = GetOAuthHBAOption(state, "bar"); /* "https://magic.example.com" or NULL */ + + if (bar && !is_valid_url(bar)) + { + res->error_detail = psprintf("validator.bar (\"%s\") is not a valid URL.", bar); + return false; + } + + /* proceed to validate token */ +} +</programlisting> + </para> + + <para> + When multiple validators are in use, their registered option lists remain + independent: + +<programlisting> +<lineannotation>in postgresql.conf:</lineannotation> +oauth_validator_libraries = 'example_org, my_validator' + +<lineannotation>in pg_hba.conf:</lineannotation> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator=my_validator \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.org \ + scope="pg-user openid profile" \ + validator=example_org \ + delegate_ident_mapping=1 \ + validator.magic=on \ + validator.more_magic=off +</programlisting> + </para> + </sect2> + </sect1> </chapter> diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index c4570ce9b3f..e8898561c8c 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -140,6 +140,8 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *oauth_opt_keys; + List *oauth_opt_vals; } HbaLine; typedef struct IdentLine diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 60f493acddd..86f463a284e 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -96,6 +96,17 @@ typedef struct OAuthValidatorCallbacks ValidatorValidateCB validate_cb; } OAuthValidatorCallbacks; +/* + * A validator can register a list of custom option names during its startup_cb, + * then later retrieve the user settings for each during validation. This + * enables per-HBA-line configuration. For more information, refer to the OAuth + * validator modules documentation. + */ +extern void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +extern const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); + /* * Type of the shared library symbol _PG_oauth_validator_module_init which is * required for all validator modules. This function will be invoked during @@ -107,9 +118,7 @@ extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_ini /* Implementation */ extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech; -/* - * Ensure a validator named in the HBA is permitted by the configuration. - */ extern bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg); +extern bool valid_oauth_hba_option_name(const char *name); #endif /* PG_OAUTH_H */ diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 6a75b79efbf..ea34ebdb733 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -25,6 +25,7 @@ #include "libpq/hba.h" #include "libpq/oauth.h" #include "libpq/sasl.h" +#include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "utils/json.h" @@ -40,10 +41,15 @@ static int oauth_exchange(void *opaq, const char *input, int inputlen, static void load_validator_library(const char *libname); static void shutdown_validator_library(void *arg); +static bool check_validator_hba_options(Port *port, const char **logdetail); static ValidatorModuleState *validator_module_state; static const OAuthValidatorCallbacks *ValidatorCallbacks; +static MemoryContext ValidatorMemoryContext; +static List *ValidatorOptions; +static bool ValidatorOptionsChecked; + /* Mechanism declaration */ const pg_be_sasl_mech pg_be_oauth_mech = { .get_mechanisms = oauth_get_mechanisms, @@ -109,6 +115,9 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("client selected an invalid SASL authentication mechanism")); + /* Save our memory context for later use by client API calls. */ + ValidatorMemoryContext = CurrentMemoryContext; + ctx = palloc0_object(struct oauth_ctx); ctx->state = OAUTH_STATE_INIT; @@ -293,6 +302,16 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); + /* + * Make sure all custom HBA options are understood by the validator before + * continuing, since we couldn't check them during server start/reload. + */ + if (!check_validator_hba_options(ctx->port, logdetail)) + { + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; + } + if (auth[0] == '\0') { /* @@ -822,6 +841,9 @@ shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) ValidatorCallbacks->shutdown_cb(validator_module_state); + + /* The backing memory for this is about to disappear. */ + ValidatorOptions = NIL; } /* @@ -907,3 +929,206 @@ done: return (*err_msg == NULL); } + +/* + * Client APIs for validator implementations + * + * Since we're currently not threaded, we only allow one validator in the + * process at a time. So we can make use of globals for now instead of looking + * up information using the state pointer. We probably shouldn't assume that the + * module hasn't temporarily changed memory contexts on us, though; functions + * here should defensively use an appropriate context when making global + * allocations. + */ + +/* + * Adds to the list of allowed validator.* HBA options. Used during the + * startup_cb. + */ +void +RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]) +{ + MemoryContext oldcontext; + + if (!state) + { + Assert(false); + return; + } + + oldcontext = MemoryContextSwitchTo(ValidatorMemoryContext); + + for (int i = 0; i < num; i++) + { + if (!valid_oauth_hba_option_name(opts[i])) + { + /* + * The user can't set this option in the HBA, so GetOAuthHBAOption + * would always return NULL. + */ + ereport(WARNING, + errmsg("HBA option name \"%s\" is invalid and will be ignored", + opts[i]), + /* translator: the second %s is a function name */ + errcontext("validator module \"%s\", in call to %s", + MyProcPort->hba->oauth_validator, + "RegisterOAuthHBAOptions")); + continue; + } + + ValidatorOptions = lappend(ValidatorOptions, pstrdup(opts[i])); + } + + MemoryContextSwitchTo(oldcontext); + + /* + * Wait to validate the HBA against the registered options until later + * (see check_validator_hba_options()). + * + * Delaying allows the validator to make multiple registration calls, to + * append to the list; it lets us make the check in a place where we can + * report the error without leaking details to the client; and it avoids + * exporting the order of operations between HBA matching and the + * startup_cb call as an API guarantee. (The last issue may become + * relevant with a threaded model.) + */ +} + +/* + * Restrict the names available to custom HBA options, so that we don't + * accidentally prevent future syntax extensions to HBA files. + */ +bool +valid_oauth_hba_option_name(const char *name) +{ + /* + * This list is not incredibly principled, since the goal is just to bound + * compatibility guarantees for our HBA parser. Alphanumerics seem + * obviously fine, and it's difficult to argue against the punctuation + * that's already included in some HBA option names and identifiers. + */ + static const char *name_allowed_set = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_-"; + + size_t span; + + if (!name[0]) + return false; + + span = strspn(name, name_allowed_set); + return name[span] == '\0'; +} + +/* + * Verifies that all validator.* HBA options specified by the user were actually + * registered by the validator library in use. + */ +static bool +check_validator_hba_options(Port *port, const char **logdetail) +{ + HbaLine *hba = port->hba; + + foreach_ptr(char, key, hba->oauth_opt_keys) + { + bool found = false; + + /* O(n^2) shouldn't be a problem here in practice. */ + foreach_ptr(char, optname, ValidatorOptions) + { + if (strcmp(key, optname) == 0) + { + found = true; + break; + } + } + + if (!found) + { + /* + * Unknown option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * + * Since this is affecting live connections, which is unusual for + * HBA, be noisy with a WARNING. (Warnings aren't sent to clients + * prior to successful authentication, so this won't disclose the + * server config.) It'll duplicate some of the information in the + * logdetail, but that should make it hard to miss the connection + * between the two. + */ + char *name = psprintf("validator.%s", key); + + *logdetail = psprintf(_("unrecognized authentication option name: \"%s\""), + name); + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + /* translator: the first %s is the name of the module */ + errdetail("The installed validator module (\"%s\") did not define an option named \"%s\".", + hba->oauth_validator, key), + errhint("All OAuth connections matching this line will fail. Correct the option and reload the server configuration."), + errcontext("line %d of configuration file \"%s\"", + hba->linenumber, hba->sourcefile)); + + return false; + } + } + + ValidatorOptionsChecked = true; /* unfetter GetOAuthHBAOption() */ + return true; +} + +/* + * Retrieves the setting for a validator.* HBA option, or NULL if not found. + * This may only be used during the validate_cb and shutdown_cb. + */ +const char * +GetOAuthHBAOption(const ValidatorModuleState *state, const char *optname) +{ + HbaLine *hba = MyProcPort->hba; + ListCell *lc_k; + ListCell *lc_v; + const char *ret = NULL; + + if (!ValidatorOptionsChecked) + { + /* + * Prevent the startup_cb from retrieving HBA options that it has just + * registered. This probably seems strange -- why refuse to hand out + * information we already know? -- but this lets us reserve the + * ability to perform the startup_cb call earlier, before we know + * which HBA line is matched by a connection, without breaking this + * API. + */ + return NULL; + } + + if (!state || !hba) + { + Assert(false); + return NULL; + } + + Assert(list_length(hba->oauth_opt_keys) == list_length(hba->oauth_opt_vals)); + + forboth(lc_k, hba->oauth_opt_keys, lc_v, hba->oauth_opt_vals) + { + const char *key = lfirst(lc_k); + const char *val = lfirst(lc_v); + + if (strcmp(key, optname) == 0) + { + /* + * Don't return yet -- when regular HBA options are specified more + * than once, the last one wins. Do the same for these options. + */ + ret = val; + } + } + + return ret; +} diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..7694506aaf7 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2497,6 +2497,32 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); hbaline->oauth_validator = pstrdup(val); } + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); + + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be + * loaded into the postmaster, we don't know if the options are valid + * yet and must store them for later. Perform only a basic syntax + * check here. + */ + if (!valid_oauth_hba_option_name(key)) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("invalid OAuth validator option name: \"%s\"", name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + return false; + } + + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } else if (strcmp(name, "delegate_ident_mapping") == 0) { REQUIRE_AUTH_OPTION(uaOAuth, "delegate_ident_mapping", "oauth"); diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index ac62555675a..cb326b51597 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -602,10 +602,29 @@ $node->connect_fails( $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); + +# We complain when bad option names are registered, but connections may proceed +# (since users can't set those options in the HBA anyway). +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authn_id"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authorize_tokens"); +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.invalid_hba TO true"); + $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$node->connect_ok( + "$common_connstr user=test", + "bad registered HBA option", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/WARNING:\s+HBA option name "bad option name" is invalid and will be ignored/, + qr/CONTEXT:\s+validator module "validator", in call to RegisterOAuthHBAOptions/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.invalid_hba"); + # # Test user mapping. # @@ -674,6 +693,84 @@ $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$bgconn->quit; # the tests below restart the server + +# +# Test validator-specific HBA options. +# + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="openid postgres" delegate_ident_mapping=1 \\ + validator.authn_id="ignored" validator.authn_id="other-identity" +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.log="testalt message" +}); + +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_ok( + "$common_connstr user=test", + "custom HBA setting (test)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [qr/connection authenticated: identity="other-identity"/]); +$node->connect_ok( + "$common_connstr user=testalt", + "custom HBA setting (testalt)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/LOG:\s+testalt message/, + qr/connection authenticated: identity="testalt"/, + ]); + +# bad syntax +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("empty HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\."/]); + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.@@=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("invalid HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\.@@"/]); + +# unknown settings (validation is deferred to connect time) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" \\ + validator.log=ignored validator.bad=1 +}); +$node->restart; + +$node->connect_fails( + "$common_connstr user=testalt", + "bad HBA setting", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+unrecognized authentication option name: "validator\.bad"/, + qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+unrecognized authentication option name: "validator\.bad"/, + ]); + # # Test multiple validators. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 353e0e0d32a..85fb4c08bf2 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -42,13 +42,21 @@ static char *authn_id = NULL; static bool authorize_tokens = true; static char *error_detail = NULL; static bool internal_error = false; +static bool invalid_hba = false; + +/* HBA options */ +static const char *hba_opts[] = { + "authn_id", /* overrides the default authn_id */ + "log", /* logs an arbitrary string */ +}; /*--- * Extension entry point. Sets up GUCs for use by tests: * * - oauth_validator.authn_id Sets the user identifier to return during token * validation. Defaults to the username in the - * startup packet. + * startup packet, or the validator.authn_id HBA + * option if it is set. * * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming @@ -96,6 +104,14 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.invalid_hba", + "Should the validator register an invalid option?", + NULL, + &invalid_hba, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -124,6 +140,29 @@ validator_startup(ValidatorModuleState *state) if (state->sversion != PG_VERSION_NUM) elog(ERROR, "oauth_validator: sversion set to %d", state->sversion); + /* + * Test the behavior of custom HBA options. Registered options should not + * be retrievable during startup (we want to discourage modules from + * relying on the relative order of client connections and the + * startup_cb). + */ + RegisterOAuthHBAOptions(state, lengthof(hba_opts), hba_opts); + for (int i = 0; i < lengthof(hba_opts); i++) + { + if (GetOAuthHBAOption(state, hba_opts[i])) + elog(ERROR, + "oauth_validator: GetOAuthValidatorOption(\"%s\") was non-NULL during startup_cb", + hba_opts[i]); + } + + if (invalid_hba) + { + /* Register a bad option, which should print a WARNING to the logs. */ + const char *invalid = "bad option name"; + + RegisterOAuthHBAOptions(state, 1, &invalid); + } + state->private_data = PRIVATE_COOKIE; } @@ -141,7 +180,7 @@ validator_shutdown(ValidatorModuleState *state) /* * Validator implementation. Logs the incoming data and authorizes the token by - * default; the behavior can be modified via the module's GUC settings. + * default; the behavior can be modified via the module's GUC and HBA settings. */ static bool validate_token(const ValidatorModuleState *state, @@ -153,6 +192,9 @@ validate_token(const ValidatorModuleState *state, elog(ERROR, "oauth_validator: private state cookie changed to %p in validate", state->private_data); + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + elog(LOG, "oauth_validator: token=\"%s\", role=\"%s\"", token, role); elog(LOG, "oauth_validator: issuer=\"%s\", scope=\"%s\"", MyProcPort->hba->oauth_issuer, @@ -165,6 +207,8 @@ validate_token(const ValidatorModuleState *state, res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); + else if (GetOAuthHBAOption(state, "authn_id")) + res->authn_id = pstrdup(GetOAuthHBAOption(state, "authn_id")); else res->authn_id = pstrdup(role); -- 2.34.1 [application/octet-stream] since-v3.nocfbot.diff (21.7K, 4-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( 2: 9726ac39442 ! 2: 5039ce74370 WIP: oauth: Allow validators to register custom HBA options @@ Metadata Author: Jacob Champion <[email protected]> ## Commit message ## - WIP: oauth: Allow validators to register custom HBA options + oauth: Allow validators to register custom HBA options - (lacks user documentation) + OAuth validators can already use custom GUCs to configure behavior + globally, but we currently provide no ability to adjust settings for + individual HBA entries, because the original design focused on a world + where a provider covered a "single audience" of users for one database + cluster. This assumption does not apply to multitenant use cases, where + a single validator may be controlling access for wildly different user + groups. - Two new API entry points for validator callbacks: - - RegisterOAuthHBAOptions - - GetOAuthHBAOption + To improve this use case, add two new API calls for use by validator + callbacks: RegisterOAuthHBAOptions() and GetOAuthHBAOption(). + Registering options "foo" and "bar" allows a user to set "validator.foo" + and "validator.bar" in an oauth HBA entry. These options are stringly + typed (syntax validation is solely the responsibility of the defining + module), and names are restricted to a subset of ASCII to avoid tying + our hands with future HBA syntax improvements. - Registering options "foo" and "bar" allows a user to set validator.foo - and validator.bar on an `oauth` HBA line. + Unfortunately, we can't check the custom option names during a reload of + the configuration, like we do with standard HBA options, unless we were + to require all validators to be loaded via shared_preload_libraries. + (I consider this to be a nonstarter: most validators should probably use + session_preload_libraries at most, since requiring a full restart of a + production server just to update authentication behavior will be + unacceptable to many users.) Instead, the new validator.* options are + checked against the registered list at connection time. - The bulk of the patch is not the conceptually simple API implementation, - but guardrails on the simple API to make sure it doesn't bind our hands - in the future, either for callback architecture or HBA syntax. + Multiple alternatives were proposed and/or prototyped, including + extending the GUC system to allow per-HBA overrides, joining forces with + recent refactoring work on the reloptions subsystem, and giving the + ability to customize HBA options to all PostgreSQL extensions. I + personally believe per-HBA GUC overrides are the best option, because + several existing GUCs like authentication_timeout and pre_auth_delay + would fit there usefully. But the recent addition of SNI per-host + settings in 4f433025f indicates that a more general solution is needed, + and I expect that to take multiple releases' worth of discussion. + + This compromise patch, then, is intentionally designed to be an + architectural dead end: simple to describe, cheap to maintain, and + providing just enough functionality to let validators move forward for + PG19. The hope is that it will be replaced in the future by a solution + that can handle per-host, per-HBA, and other per-context configuration + with the same functionality that GUCs provide today. In the meantime, + the bulk of the code in this patch consists of strict guardrails on the + simple API, to try to ensure that we don't have any reason to regret its + existence during its unknown lifespan. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> + Reviewed-by: Zsolt Parragi <[email protected]> + Discussion: https://postgr.es/m/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com + + ## doc/src/sgml/client-auth.sgml ## +@@ doc/src/sgml/client-auth.sgml: host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" + </listitem> + </varlistentry> + ++ <varlistentry> ++ <term id="auth-oauth-validator-option"> ++ <literal>validator.<replaceable>option</replaceable></literal> ++ </term> ++ <listitem> ++ <para> ++ Validator modules may <link linkend="oauth-validator-hba">define</link> ++ additional configuration options for <literal>oauth</literal> ++ HBA entries. These validator-specific options are accessible via the ++ <literal>validator.*</literal> "namespace". For example, a module may ++ register the <literal>validator.foo</literal> and ++ <literal>validator.bar</literal> options and define their effects on ++ authentication. ++ </para> ++ <para> ++ The name, syntax, and behavior of each <replaceable>option</replaceable> ++ are not determined by <productname>PostgreSQL</productname>; consult the ++ documentation for the validator module in use. ++ </para> ++ <warning> ++ <para> ++ A limitation of the current implementation is that unrecognized ++ <replaceable>option</replaceable> names will not be caught until ++ connection time. A <literal>pg_ctl reload</literal> will succeed, but ++ matching connections will fail: ++<programlisting> ++LOG: connection received: host=[local] ++WARNING: unrecognized authentication option name: "validator.bad" ++DETAIL: The installed validator module ("my_validator") did not define an option named "bad". ++HINT: All OAuth connections matching this line will fail. Correct the option and reload the server configuration. ++CONTEXT: line 2 of configuration file "data/pg_hba.conf" ++</programlisting> ++ Use caution when making changes to validator-specific HBA options in ++ production systems. ++ </para> ++ </warning> ++ </listitem> ++ </varlistentry> ++ + <varlistentry> + <term><literal>map</literal></term> + <listitem> + + ## doc/src/sgml/oauth-validators.sgml ## +@@ + <symbol>delegate_ident_mapping=1</symbol> mode, and what additional + configuration is required in order to do so. + </para> ++ <para> ++ If an implementation provides <link linkend="oauth-validator-hba">custom ++ HBA options</link>, the names and syntax of those options should be ++ documented as well. ++ </para> + </listitem> + </varlistentry> + </variablelist> +@@ doc/src/sgml/oauth-validators.sgml: typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); + <title>Startup Callback</title> + <para> + The <function>startup_cb</function> callback is executed directly after +- loading the module. This callback can be used to set up local state and ++ loading the module. This callback can be used to set up local state, ++ define <link linkend="oauth-validator-hba">custom HBA options</link>, and + perform additional initialization if required. If the validator module + has state it can use <structfield>state->private_data</structfield> to + store it. +@@ doc/src/sgml/oauth-validators.sgml: typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); + </sect2> + + </sect1> ++ ++ <sect1 id="oauth-validator-hba"> ++ <title>Custom HBA Options</title> ++ ++ <para> ++ Like other preloaded libraries, validator modules may define ++ <link linkend="runtime-config-custom">custom GUC parameters</link> for user ++ configuration in <filename>postgresql.conf</filename>. However, it may be ++ desirable to configure behavior at a more granular level (say, for a ++ particular issuer or a group of users) instead of globally. ++ </para> ++ ++ <para> ++ Beginning in <productname>PostgreSQL</productname> 19, validator ++ implementations may define custom options for use inside ++ <filename>pg_hba.conf</filename>. These options are then ++ <link linkend="auth-oauth-validator-option">made available</link> to the user ++ as <literal>validator.<replaceable>option</replaceable></literal>. The API ++ for registering and retrieving custom options is described below. ++ </para> ++ ++ <sect2 id="oauth-validator-hba-api"> ++ <title>Options API</title> ++ <para> ++ Modules register custom HBA option names during the <function>startup_cb</function> ++ callback, using <function>RegisterOAuthHBAOptions()</function>: ++ ++<programlisting> ++/* ++ * Register a list of custom option names for use in pg_hba.conf. For each name ++ * "foo" registered here, that option will be provided as "validator.foo" in ++ * the HBA. ++ * ++ * Valid option names consist of alphanumeric ASCII, underscore (_), and hyphen ++ * (-). Invalid option names will be ignored with a WARNING logged at ++ * connection time. ++ * ++ * This function may only be called during the startup_cb callback. Multiple ++ * calls are permitted, which will append to the existing list of registered ++ * options; options cannot be unregistered. ++ * ++ * Parameters: ++ * ++ * - state: the state pointer passed to the startup_cb callback ++ * - num: the number of options in the opts array ++ * - opts: an array of null-terminated option names to register ++ * ++ * The list of option names is copied internally, and the opts array is not ++ * required to remain valid after the call. ++ */ ++void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, ++ const char *opts[]); ++</programlisting> ++ </para> ++ ++ <para> ++ Each option's value, if set, may be later retrieved using ++ <function>GetOAuthHBAOption()</function>: ++ ++<programlisting> ++/* ++ * Retrieve the string value of an HBA option which was registered via ++ * RegisterOAuthHBAOptions(). Usable only during validate_cb or shutdown_cb. ++ * ++ * If the user has set the corresponding option in pg_hba.conf, this function ++ * returns that value as a null-terminated string, which must not be modified ++ * or freed. NULL is returned instead if the user has not set this option, if ++ * the option name was not registered, or if this function is incorrectly called ++ * during the startup_cb. ++ * ++ * Parameters: ++ * ++ * - state: the state pointer passed to the validate_cb/shutdown_cb callback ++ * - optname: the name of the option to retrieve ++ */ ++const char *GetOAuthHBAOption(const ValidatorModuleState *state, ++ const char *optname); ++</programlisting> ++ </para> ++ ++ <para> ++ See <xref linkend="oauth-validator-hba-example-usage"/> for sample usage. ++ </para> ++ </sect2> ++ ++ <sect2 id="oauth-validator-hba-limitations"> ++ <title>Limitations</title> ++ <para> ++ <itemizedlist> ++ <listitem> ++ <para> ++ Option names are limited to ASCII alphanumeric characters, ++ underscores (<literal>_</literal>), and hyphens (<literal>-</literal>). ++ </para> ++ </listitem> ++ <listitem> ++ <para> ++ Option values are always freeform strings (in contrast to custom GUCs, ++ which support numerics, booleans, and enums). ++ </para> ++ </listitem> ++ <listitem> ++ <para> ++ Option names and values cannot be checked by the server during a reload of ++ the configuration. Any unregistered options in <filename>pg_hba.conf</filename> ++ will instead result in connection failures. It is the responsibility of ++ each module to document and verify the syntax of option values as needed. ++ <footnote> ++ <para> ++ If a module finds an invalid option value during <function>validate_cb</function>, ++ it's recommended to <link linkend="oauth-validator-callback-validate">signal ++ an internal error</link> by setting <structfield>result->error_detail</structfield> ++ to a description of the problem and returning <literal>false</literal>. ++ </para> ++ </footnote> ++ </para> ++ </listitem> ++ </itemizedlist> ++ </para> ++ </sect2> ++ ++ <sect2 id="oauth-validator-hba-example-usage"> ++ <title>Example Usage</title> ++ ++ <para> ++ For a hypothetical module, the options <literal>foo</literal> and ++ <literal>bar</literal> could be registered as follows: ++ ++<programlisting> ++static void ++validator_startup(ValidatorModuleState *state) ++{ ++ static const char *opts[] = { ++ "foo", /* description of access privileges */ ++ "bar", /* magic URL for additional administrator powers */ ++ }; ++ ++ RegisterOAuthHBAOptions(state, lengthof(opts), opts); ++ ++ /* ...other setup... */ ++} ++</programlisting> ++ </para> ++ ++ <para> ++ The following sample entries in <filename>pg_hba.conf</filename> can then ++ make use of these options: ++ ++<programlisting> ++# TYPE DATABASE USER ADDRESS METHOD ++hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ ++ scope="pg-admin openid email" \ ++ map=oauth-email \ ++ validator.foo="admin access" \ ++ validator.bar=https://magic.example.com ++ ++hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.com \ ++ scope="pg-user openid email" \ ++ map=oauth-email \ ++ validator.foo="user access" ++</programlisting> ++ </para> ++ ++ <para> ++ The module can retrieve the option settings from the HBA during validation: ++ ++<programlisting> ++static bool ++validate_token(const ValidatorModuleState *state, ++ const char *token, const char *role, ++ ValidatorModuleResult *res) ++{ ++ const char *foo = GetOAuthHBAOption(state, "foo"); /* "admin access" or "user access" */ ++ const char *bar = GetOAuthHBAOption(state, "bar"); /* "https://magic.example.com" or NULL */ ++ ++ if (bar && !is_valid_url(bar)) ++ { ++ res->error_detail = psprintf("validator.bar (\"%s\") is not a valid URL.", bar); ++ return false; ++ } ++ ++ /* proceed to validate token */ ++} ++</programlisting> ++ </para> ++ ++ <para> ++ When multiple validators are in use, their registered option lists remain ++ independent: ++ ++<programlisting> ++<lineannotation>in postgresql.conf:</lineannotation> ++oauth_validator_libraries = 'example_org, my_validator' ++ ++<lineannotation>in pg_hba.conf:</lineannotation> ++# TYPE DATABASE USER ADDRESS METHOD ++hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ ++ scope="pg-admin openid email" \ ++ map=oauth-email \ ++ validator=my_validator \ ++ validator.foo="admin access" \ ++ validator.bar=https://magic.example.com ++ ++hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.org \ ++ scope="pg-user openid profile" \ ++ validator=example_org \ ++ delegate_ident_mapping=1 \ ++ validator.magic=on \ ++ validator.more_magic=off ++</programlisting> ++ </para> ++ </sect2> ++ </sect1> + </chapter> ## src/include/libpq/hba.h ## @@ src/include/libpq/hba.h: typedef struct HbaLine @@ src/backend/libpq/auth-oauth.c: oauth_exchange(void *opaq, const char *input, in + return PG_SASL_EXCHANGE_FAILURE; + } + - if (!validate(ctx->port, auth, logdetail)) + if (auth[0] == '\0') { - generate_error_response(ctx, output, outputlen); + /* @@ src/backend/libpq/auth-oauth.c: shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) @@ src/backend/libpq/auth-oauth.c: done: + if (!found) + { + /* -+ * Bad option name. Mirror the error messages in hba.c here, ++ * Unknown option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * @@ src/backend/libpq/hba.c: parse_hba_auth_opt(char *name, char *val, HbaLine *hbal + { + const char *key = name + strlen("validator."); + ++ REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); ++ + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be @@ src/backend/libpq/hba.c: parse_hba_auth_opt(char *name, char *val, HbaLine *hbal + return false; + } + -+ REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-04-03 23:33 Jacob Champion <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Jacob Champion @ 2026-04-03 23:33 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Thu, Apr 2, 2026 at 2:26 PM Jacob Champion <[email protected]> wrote: > v4-0002 (based on top of [1]) addresses Zsolt's feedback and adds the > missing user documentation. I believe this is now a complete patch > proposal; tear it apart. :D v5 is a quick rebase to get rid of v4-0001; no other changes. --Jacob Attachments: [application/octet-stream] v5-0001-oauth-Allow-validators-to-register-custom-HBA-opt.patch (33.9K, 2-v5-0001-oauth-Allow-validators-to-register-custom-HBA-opt.patch) download | inline diff: From 03ac60629e1346195018dc2112b778b805931013 Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Tue, 17 Mar 2026 12:01:28 -0700 Subject: [PATCH v5] oauth: Allow validators to register custom HBA options OAuth validators can already use custom GUCs to configure behavior globally, but we currently provide no ability to adjust settings for individual HBA entries, because the original design focused on a world where a provider covered a "single audience" of users for one database cluster. This assumption does not apply to multitenant use cases, where a single validator may be controlling access for wildly different user groups. To improve this use case, add two new API calls for use by validator callbacks: RegisterOAuthHBAOptions() and GetOAuthHBAOption(). Registering options "foo" and "bar" allows a user to set "validator.foo" and "validator.bar" in an oauth HBA entry. These options are stringly typed (syntax validation is solely the responsibility of the defining module), and names are restricted to a subset of ASCII to avoid tying our hands with future HBA syntax improvements. Unfortunately, we can't check the custom option names during a reload of the configuration, like we do with standard HBA options, unless we were to require all validators to be loaded via shared_preload_libraries. (I consider this to be a nonstarter: most validators should probably use session_preload_libraries at most, since requiring a full restart of a production server just to update authentication behavior will be unacceptable to many users.) Instead, the new validator.* options are checked against the registered list at connection time. Multiple alternatives were proposed and/or prototyped, including extending the GUC system to allow per-HBA overrides, joining forces with recent refactoring work on the reloptions subsystem, and giving the ability to customize HBA options to all PostgreSQL extensions. I personally believe per-HBA GUC overrides are the best option, because several existing GUCs like authentication_timeout and pre_auth_delay would fit there usefully. But the recent addition of SNI per-host settings in 4f433025f indicates that a more general solution is needed, and I expect that to take multiple releases' worth of discussion. This compromise patch, then, is intentionally designed to be an architectural dead end: simple to describe, cheap to maintain, and providing just enough functionality to let validators move forward for PG19. The hope is that it will be replaced in the future by a solution that can handle per-host, per-HBA, and other per-context configuration with the same functionality that GUCs provide today. In the meantime, the bulk of the code in this patch consists of strict guardrails on the simple API, to try to ensure that we don't have any reason to regret its existence during its unknown lifespan. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> Reviewed-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com --- doc/src/sgml/client-auth.sgml | 39 +++ doc/src/sgml/oauth-validators.sgml | 221 ++++++++++++++++- src/include/libpq/hba.h | 2 + src/include/libpq/oauth.h | 15 +- src/backend/libpq/auth-oauth.c | 225 ++++++++++++++++++ src/backend/libpq/hba.c | 26 ++ .../modules/oauth_validator/t/001_server.pl | 97 ++++++++ src/test/modules/oauth_validator/validator.c | 48 +++- 8 files changed, 667 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index a347ee18980..bca09a80416 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -2532,6 +2532,45 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" </listitem> </varlistentry> + <varlistentry> + <term id="auth-oauth-validator-option"> + <literal>validator.<replaceable>option</replaceable></literal> + </term> + <listitem> + <para> + Validator modules may <link linkend="oauth-validator-hba">define</link> + additional configuration options for <literal>oauth</literal> + HBA entries. These validator-specific options are accessible via the + <literal>validator.*</literal> "namespace". For example, a module may + register the <literal>validator.foo</literal> and + <literal>validator.bar</literal> options and define their effects on + authentication. + </para> + <para> + The name, syntax, and behavior of each <replaceable>option</replaceable> + are not determined by <productname>PostgreSQL</productname>; consult the + documentation for the validator module in use. + </para> + <warning> + <para> + A limitation of the current implementation is that unrecognized + <replaceable>option</replaceable> names will not be caught until + connection time. A <literal>pg_ctl reload</literal> will succeed, but + matching connections will fail: +<programlisting> +LOG: connection received: host=[local] +WARNING: unrecognized authentication option name: "validator.bad" +DETAIL: The installed validator module ("my_validator") did not define an option named "bad". +HINT: All OAuth connections matching this line will fail. Correct the option and reload the server configuration. +CONTEXT: line 2 of configuration file "data/pg_hba.conf" +</programlisting> + Use caution when making changes to validator-specific HBA options in + production systems. + </para> + </warning> + </listitem> + </varlistentry> + <varlistentry> <term><literal>map</literal></term> <listitem> diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index 5f29f2be186..7c9e3dd931a 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -251,6 +251,11 @@ <symbol>delegate_ident_mapping=1</symbol> mode, and what additional configuration is required in order to do so. </para> + <para> + If an implementation provides <link linkend="oauth-validator-hba">custom + HBA options</link>, the names and syntax of those options should be + documented as well. + </para> </listitem> </varlistentry> </variablelist> @@ -343,7 +348,8 @@ typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); <title>Startup Callback</title> <para> The <function>startup_cb</function> callback is executed directly after - loading the module. This callback can be used to set up local state and + loading the module. This callback can be used to set up local state, + define <link linkend="oauth-validator-hba">custom HBA options</link>, and perform additional initialization if required. If the validator module has state it can use <structfield>state->private_data</structfield> to store it. @@ -432,4 +438,217 @@ typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); </sect2> </sect1> + + <sect1 id="oauth-validator-hba"> + <title>Custom HBA Options</title> + + <para> + Like other preloaded libraries, validator modules may define + <link linkend="runtime-config-custom">custom GUC parameters</link> for user + configuration in <filename>postgresql.conf</filename>. However, it may be + desirable to configure behavior at a more granular level (say, for a + particular issuer or a group of users) instead of globally. + </para> + + <para> + Beginning in <productname>PostgreSQL</productname> 19, validator + implementations may define custom options for use inside + <filename>pg_hba.conf</filename>. These options are then + <link linkend="auth-oauth-validator-option">made available</link> to the user + as <literal>validator.<replaceable>option</replaceable></literal>. The API + for registering and retrieving custom options is described below. + </para> + + <sect2 id="oauth-validator-hba-api"> + <title>Options API</title> + <para> + Modules register custom HBA option names during the <function>startup_cb</function> + callback, using <function>RegisterOAuthHBAOptions()</function>: + +<programlisting> +/* + * Register a list of custom option names for use in pg_hba.conf. For each name + * "foo" registered here, that option will be provided as "validator.foo" in + * the HBA. + * + * Valid option names consist of alphanumeric ASCII, underscore (_), and hyphen + * (-). Invalid option names will be ignored with a WARNING logged at + * connection time. + * + * This function may only be called during the startup_cb callback. Multiple + * calls are permitted, which will append to the existing list of registered + * options; options cannot be unregistered. + * + * Parameters: + * + * - state: the state pointer passed to the startup_cb callback + * - num: the number of options in the opts array + * - opts: an array of null-terminated option names to register + * + * The list of option names is copied internally, and the opts array is not + * required to remain valid after the call. + */ +void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +</programlisting> + </para> + + <para> + Each option's value, if set, may be later retrieved using + <function>GetOAuthHBAOption()</function>: + +<programlisting> +/* + * Retrieve the string value of an HBA option which was registered via + * RegisterOAuthHBAOptions(). Usable only during validate_cb or shutdown_cb. + * + * If the user has set the corresponding option in pg_hba.conf, this function + * returns that value as a null-terminated string, which must not be modified + * or freed. NULL is returned instead if the user has not set this option, if + * the option name was not registered, or if this function is incorrectly called + * during the startup_cb. + * + * Parameters: + * + * - state: the state pointer passed to the validate_cb/shutdown_cb callback + * - optname: the name of the option to retrieve + */ +const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); +</programlisting> + </para> + + <para> + See <xref linkend="oauth-validator-hba-example-usage"/> for sample usage. + </para> + </sect2> + + <sect2 id="oauth-validator-hba-limitations"> + <title>Limitations</title> + <para> + <itemizedlist> + <listitem> + <para> + Option names are limited to ASCII alphanumeric characters, + underscores (<literal>_</literal>), and hyphens (<literal>-</literal>). + </para> + </listitem> + <listitem> + <para> + Option values are always freeform strings (in contrast to custom GUCs, + which support numerics, booleans, and enums). + </para> + </listitem> + <listitem> + <para> + Option names and values cannot be checked by the server during a reload of + the configuration. Any unregistered options in <filename>pg_hba.conf</filename> + will instead result in connection failures. It is the responsibility of + each module to document and verify the syntax of option values as needed. + <footnote> + <para> + If a module finds an invalid option value during <function>validate_cb</function>, + it's recommended to <link linkend="oauth-validator-callback-validate">signal + an internal error</link> by setting <structfield>result->error_detail</structfield> + to a description of the problem and returning <literal>false</literal>. + </para> + </footnote> + </para> + </listitem> + </itemizedlist> + </para> + </sect2> + + <sect2 id="oauth-validator-hba-example-usage"> + <title>Example Usage</title> + + <para> + For a hypothetical module, the options <literal>foo</literal> and + <literal>bar</literal> could be registered as follows: + +<programlisting> +static void +validator_startup(ValidatorModuleState *state) +{ + static const char *opts[] = { + "foo", /* description of access privileges */ + "bar", /* magic URL for additional administrator powers */ + }; + + RegisterOAuthHBAOptions(state, lengthof(opts), opts); + + /* ...other setup... */ +} +</programlisting> + </para> + + <para> + The following sample entries in <filename>pg_hba.conf</filename> can then + make use of these options: + +<programlisting> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.com \ + scope="pg-user openid email" \ + map=oauth-email \ + validator.foo="user access" +</programlisting> + </para> + + <para> + The module can retrieve the option settings from the HBA during validation: + +<programlisting> +static bool +validate_token(const ValidatorModuleState *state, + const char *token, const char *role, + ValidatorModuleResult *res) +{ + const char *foo = GetOAuthHBAOption(state, "foo"); /* "admin access" or "user access" */ + const char *bar = GetOAuthHBAOption(state, "bar"); /* "https://magic.example.com" or NULL */ + + if (bar && !is_valid_url(bar)) + { + res->error_detail = psprintf("validator.bar (\"%s\") is not a valid URL.", bar); + return false; + } + + /* proceed to validate token */ +} +</programlisting> + </para> + + <para> + When multiple validators are in use, their registered option lists remain + independent: + +<programlisting> +<lineannotation>in postgresql.conf:</lineannotation> +oauth_validator_libraries = 'example_org, my_validator' + +<lineannotation>in pg_hba.conf:</lineannotation> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator=my_validator \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.org \ + scope="pg-user openid profile" \ + validator=example_org \ + delegate_ident_mapping=1 \ + validator.magic=on \ + validator.more_magic=off +</programlisting> + </para> + </sect2> + </sect1> </chapter> diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index c4570ce9b3f..e8898561c8c 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -140,6 +140,8 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *oauth_opt_keys; + List *oauth_opt_vals; } HbaLine; typedef struct IdentLine diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 60f493acddd..86f463a284e 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -96,6 +96,17 @@ typedef struct OAuthValidatorCallbacks ValidatorValidateCB validate_cb; } OAuthValidatorCallbacks; +/* + * A validator can register a list of custom option names during its startup_cb, + * then later retrieve the user settings for each during validation. This + * enables per-HBA-line configuration. For more information, refer to the OAuth + * validator modules documentation. + */ +extern void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +extern const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); + /* * Type of the shared library symbol _PG_oauth_validator_module_init which is * required for all validator modules. This function will be invoked during @@ -107,9 +118,7 @@ extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_ini /* Implementation */ extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech; -/* - * Ensure a validator named in the HBA is permitted by the configuration. - */ extern bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg); +extern bool valid_oauth_hba_option_name(const char *name); #endif /* PG_OAUTH_H */ diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 6a75b79efbf..ea34ebdb733 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -25,6 +25,7 @@ #include "libpq/hba.h" #include "libpq/oauth.h" #include "libpq/sasl.h" +#include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "utils/json.h" @@ -40,10 +41,15 @@ static int oauth_exchange(void *opaq, const char *input, int inputlen, static void load_validator_library(const char *libname); static void shutdown_validator_library(void *arg); +static bool check_validator_hba_options(Port *port, const char **logdetail); static ValidatorModuleState *validator_module_state; static const OAuthValidatorCallbacks *ValidatorCallbacks; +static MemoryContext ValidatorMemoryContext; +static List *ValidatorOptions; +static bool ValidatorOptionsChecked; + /* Mechanism declaration */ const pg_be_sasl_mech pg_be_oauth_mech = { .get_mechanisms = oauth_get_mechanisms, @@ -109,6 +115,9 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("client selected an invalid SASL authentication mechanism")); + /* Save our memory context for later use by client API calls. */ + ValidatorMemoryContext = CurrentMemoryContext; + ctx = palloc0_object(struct oauth_ctx); ctx->state = OAUTH_STATE_INIT; @@ -293,6 +302,16 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); + /* + * Make sure all custom HBA options are understood by the validator before + * continuing, since we couldn't check them during server start/reload. + */ + if (!check_validator_hba_options(ctx->port, logdetail)) + { + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; + } + if (auth[0] == '\0') { /* @@ -822,6 +841,9 @@ shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) ValidatorCallbacks->shutdown_cb(validator_module_state); + + /* The backing memory for this is about to disappear. */ + ValidatorOptions = NIL; } /* @@ -907,3 +929,206 @@ done: return (*err_msg == NULL); } + +/* + * Client APIs for validator implementations + * + * Since we're currently not threaded, we only allow one validator in the + * process at a time. So we can make use of globals for now instead of looking + * up information using the state pointer. We probably shouldn't assume that the + * module hasn't temporarily changed memory contexts on us, though; functions + * here should defensively use an appropriate context when making global + * allocations. + */ + +/* + * Adds to the list of allowed validator.* HBA options. Used during the + * startup_cb. + */ +void +RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]) +{ + MemoryContext oldcontext; + + if (!state) + { + Assert(false); + return; + } + + oldcontext = MemoryContextSwitchTo(ValidatorMemoryContext); + + for (int i = 0; i < num; i++) + { + if (!valid_oauth_hba_option_name(opts[i])) + { + /* + * The user can't set this option in the HBA, so GetOAuthHBAOption + * would always return NULL. + */ + ereport(WARNING, + errmsg("HBA option name \"%s\" is invalid and will be ignored", + opts[i]), + /* translator: the second %s is a function name */ + errcontext("validator module \"%s\", in call to %s", + MyProcPort->hba->oauth_validator, + "RegisterOAuthHBAOptions")); + continue; + } + + ValidatorOptions = lappend(ValidatorOptions, pstrdup(opts[i])); + } + + MemoryContextSwitchTo(oldcontext); + + /* + * Wait to validate the HBA against the registered options until later + * (see check_validator_hba_options()). + * + * Delaying allows the validator to make multiple registration calls, to + * append to the list; it lets us make the check in a place where we can + * report the error without leaking details to the client; and it avoids + * exporting the order of operations between HBA matching and the + * startup_cb call as an API guarantee. (The last issue may become + * relevant with a threaded model.) + */ +} + +/* + * Restrict the names available to custom HBA options, so that we don't + * accidentally prevent future syntax extensions to HBA files. + */ +bool +valid_oauth_hba_option_name(const char *name) +{ + /* + * This list is not incredibly principled, since the goal is just to bound + * compatibility guarantees for our HBA parser. Alphanumerics seem + * obviously fine, and it's difficult to argue against the punctuation + * that's already included in some HBA option names and identifiers. + */ + static const char *name_allowed_set = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_-"; + + size_t span; + + if (!name[0]) + return false; + + span = strspn(name, name_allowed_set); + return name[span] == '\0'; +} + +/* + * Verifies that all validator.* HBA options specified by the user were actually + * registered by the validator library in use. + */ +static bool +check_validator_hba_options(Port *port, const char **logdetail) +{ + HbaLine *hba = port->hba; + + foreach_ptr(char, key, hba->oauth_opt_keys) + { + bool found = false; + + /* O(n^2) shouldn't be a problem here in practice. */ + foreach_ptr(char, optname, ValidatorOptions) + { + if (strcmp(key, optname) == 0) + { + found = true; + break; + } + } + + if (!found) + { + /* + * Unknown option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * + * Since this is affecting live connections, which is unusual for + * HBA, be noisy with a WARNING. (Warnings aren't sent to clients + * prior to successful authentication, so this won't disclose the + * server config.) It'll duplicate some of the information in the + * logdetail, but that should make it hard to miss the connection + * between the two. + */ + char *name = psprintf("validator.%s", key); + + *logdetail = psprintf(_("unrecognized authentication option name: \"%s\""), + name); + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + /* translator: the first %s is the name of the module */ + errdetail("The installed validator module (\"%s\") did not define an option named \"%s\".", + hba->oauth_validator, key), + errhint("All OAuth connections matching this line will fail. Correct the option and reload the server configuration."), + errcontext("line %d of configuration file \"%s\"", + hba->linenumber, hba->sourcefile)); + + return false; + } + } + + ValidatorOptionsChecked = true; /* unfetter GetOAuthHBAOption() */ + return true; +} + +/* + * Retrieves the setting for a validator.* HBA option, or NULL if not found. + * This may only be used during the validate_cb and shutdown_cb. + */ +const char * +GetOAuthHBAOption(const ValidatorModuleState *state, const char *optname) +{ + HbaLine *hba = MyProcPort->hba; + ListCell *lc_k; + ListCell *lc_v; + const char *ret = NULL; + + if (!ValidatorOptionsChecked) + { + /* + * Prevent the startup_cb from retrieving HBA options that it has just + * registered. This probably seems strange -- why refuse to hand out + * information we already know? -- but this lets us reserve the + * ability to perform the startup_cb call earlier, before we know + * which HBA line is matched by a connection, without breaking this + * API. + */ + return NULL; + } + + if (!state || !hba) + { + Assert(false); + return NULL; + } + + Assert(list_length(hba->oauth_opt_keys) == list_length(hba->oauth_opt_vals)); + + forboth(lc_k, hba->oauth_opt_keys, lc_v, hba->oauth_opt_vals) + { + const char *key = lfirst(lc_k); + const char *val = lfirst(lc_v); + + if (strcmp(key, optname) == 0) + { + /* + * Don't return yet -- when regular HBA options are specified more + * than once, the last one wins. Do the same for these options. + */ + ret = val; + } + } + + return ret; +} diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..7694506aaf7 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2497,6 +2497,32 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); hbaline->oauth_validator = pstrdup(val); } + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); + + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be + * loaded into the postmaster, we don't know if the options are valid + * yet and must store them for later. Perform only a basic syntax + * check here. + */ + if (!valid_oauth_hba_option_name(key)) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("invalid OAuth validator option name: \"%s\"", name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + return false; + } + + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } else if (strcmp(name, "delegate_ident_mapping") == 0) { REQUIRE_AUTH_OPTION(uaOAuth, "delegate_ident_mapping", "oauth"); diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index ac62555675a..cb326b51597 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -602,10 +602,29 @@ $node->connect_fails( $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); + +# We complain when bad option names are registered, but connections may proceed +# (since users can't set those options in the HBA anyway). +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authn_id"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authorize_tokens"); +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.invalid_hba TO true"); + $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$node->connect_ok( + "$common_connstr user=test", + "bad registered HBA option", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/WARNING:\s+HBA option name "bad option name" is invalid and will be ignored/, + qr/CONTEXT:\s+validator module "validator", in call to RegisterOAuthHBAOptions/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.invalid_hba"); + # # Test user mapping. # @@ -674,6 +693,84 @@ $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$bgconn->quit; # the tests below restart the server + +# +# Test validator-specific HBA options. +# + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="openid postgres" delegate_ident_mapping=1 \\ + validator.authn_id="ignored" validator.authn_id="other-identity" +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.log="testalt message" +}); + +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_ok( + "$common_connstr user=test", + "custom HBA setting (test)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [qr/connection authenticated: identity="other-identity"/]); +$node->connect_ok( + "$common_connstr user=testalt", + "custom HBA setting (testalt)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/LOG:\s+testalt message/, + qr/connection authenticated: identity="testalt"/, + ]); + +# bad syntax +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("empty HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\."/]); + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.@@=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("invalid HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\.@@"/]); + +# unknown settings (validation is deferred to connect time) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" \\ + validator.log=ignored validator.bad=1 +}); +$node->restart; + +$node->connect_fails( + "$common_connstr user=testalt", + "bad HBA setting", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+unrecognized authentication option name: "validator\.bad"/, + qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+unrecognized authentication option name: "validator\.bad"/, + ]); + # # Test multiple validators. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 353e0e0d32a..85fb4c08bf2 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -42,13 +42,21 @@ static char *authn_id = NULL; static bool authorize_tokens = true; static char *error_detail = NULL; static bool internal_error = false; +static bool invalid_hba = false; + +/* HBA options */ +static const char *hba_opts[] = { + "authn_id", /* overrides the default authn_id */ + "log", /* logs an arbitrary string */ +}; /*--- * Extension entry point. Sets up GUCs for use by tests: * * - oauth_validator.authn_id Sets the user identifier to return during token * validation. Defaults to the username in the - * startup packet. + * startup packet, or the validator.authn_id HBA + * option if it is set. * * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming @@ -96,6 +104,14 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.invalid_hba", + "Should the validator register an invalid option?", + NULL, + &invalid_hba, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -124,6 +140,29 @@ validator_startup(ValidatorModuleState *state) if (state->sversion != PG_VERSION_NUM) elog(ERROR, "oauth_validator: sversion set to %d", state->sversion); + /* + * Test the behavior of custom HBA options. Registered options should not + * be retrievable during startup (we want to discourage modules from + * relying on the relative order of client connections and the + * startup_cb). + */ + RegisterOAuthHBAOptions(state, lengthof(hba_opts), hba_opts); + for (int i = 0; i < lengthof(hba_opts); i++) + { + if (GetOAuthHBAOption(state, hba_opts[i])) + elog(ERROR, + "oauth_validator: GetOAuthValidatorOption(\"%s\") was non-NULL during startup_cb", + hba_opts[i]); + } + + if (invalid_hba) + { + /* Register a bad option, which should print a WARNING to the logs. */ + const char *invalid = "bad option name"; + + RegisterOAuthHBAOptions(state, 1, &invalid); + } + state->private_data = PRIVATE_COOKIE; } @@ -141,7 +180,7 @@ validator_shutdown(ValidatorModuleState *state) /* * Validator implementation. Logs the incoming data and authorizes the token by - * default; the behavior can be modified via the module's GUC settings. + * default; the behavior can be modified via the module's GUC and HBA settings. */ static bool validate_token(const ValidatorModuleState *state, @@ -153,6 +192,9 @@ validate_token(const ValidatorModuleState *state, elog(ERROR, "oauth_validator: private state cookie changed to %p in validate", state->private_data); + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + elog(LOG, "oauth_validator: token=\"%s\", role=\"%s\"", token, role); elog(LOG, "oauth_validator: issuer=\"%s\", scope=\"%s\"", MyProcPort->hba->oauth_issuer, @@ -165,6 +207,8 @@ validate_token(const ValidatorModuleState *state, res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); + else if (GetOAuthHBAOption(state, "authn_id")) + res->authn_id = pstrdup(GetOAuthHBAOption(state, "authn_id")); else res->authn_id = pstrdup(role); -- 2.34.1 ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-04-06 22:09 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 25+ messages in thread From: Zsolt Parragi @ 2026-04-06 22:09 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > I believe this is now a complete patch > proposal; tear it apart. :D The patch looks good to me as is, I don't see any real issues with it, maybe one cosmetic question in the test. + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + If we treat the test code as an example for real implementations, this and the other use could cache the option in a local variable instead of making duplicate calls, following the pattern of the documentation. ^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Custom oauth validator options @ 2026-04-06 22:16 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 0 replies; 25+ messages in thread From: Jacob Champion @ 2026-04-06 22:16 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Apr 6, 2026 at 3:09 PM Zsolt Parragi <[email protected]> wrote: > > I believe this is now a complete patch > > proposal; tear it apart. :D > > The patch looks good to me as is, I don't see any real issues with it, Great! I am prepping for a commit Sometime Very Soon. > maybe one cosmetic question in the test. > > + if (GetOAuthHBAOption(state, "log")) > + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); > + > > If we treat the test code as an example for real implementations, (It's not, though -- my test code is frequently abusive on purpose and should not be used as best-practice. If we want compilable sample code, that looks different from tests IMHO.) > this > and the other use could cache the option in a local variable instead > of making duplicate calls, following the pattern of the documentation. In this case, I want the test to pin the behavior that multiple calls should work as you'd expect. Thanks, --Jacob ^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2026-04-06 22:16 UTC | newest] Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-12-02 13:05 Custom oauth validator options Zsolt Parragi <[email protected]> 2025-12-16 23:09 ` Jacob Champion <[email protected]> 2025-12-17 09:33 ` Zsolt Parragi <[email protected]> 2025-12-17 06:30 ` VASUKI M <[email protected]> 2025-12-17 09:35 ` Zsolt Parragi <[email protected]> 2025-12-17 19:01 ` Jacob Champion <[email protected]> 2025-12-17 23:52 ` Zsolt Parragi <[email protected]> 2025-12-18 05:14 ` VASUKI M <[email protected]> 2025-12-18 09:08 ` Zsolt Parragi <[email protected]> 2025-12-18 17:27 ` Jacob Champion <[email protected]> 2025-12-18 18:28 ` Zsolt Parragi <[email protected]> 2025-12-18 18:43 ` Jacob Champion <[email protected]> 2025-12-18 20:29 ` Zsolt Parragi <[email protected]> 2025-12-17 18:27 ` Jacob Champion <[email protected]> 2026-02-04 11:42 Re: Custom oauth validator options Zsolt Parragi <[email protected]> 2026-03-20 17:46 ` Jacob Champion <[email protected]> 2026-03-23 21:45 ` Zsolt Parragi <[email protected]> 2026-03-27 23:03 ` Jacob Champion <[email protected]> 2026-03-30 21:46 ` Zsolt Parragi <[email protected]> 2026-03-30 23:54 ` Jacob Champion <[email protected]> 2026-04-02 21:26 ` Jacob Champion <[email protected]> 2026-04-03 23:33 ` Jacob Champion <[email protected]> 2026-04-06 22:09 ` Zsolt Parragi <[email protected]> 2026-04-06 22:16 ` Jacob Champion <[email protected]> 2026-03-27 23: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