1: 5730b875b8 ! 1: 16f1b8fc02 Add OAUTHBEARER SASL mechanism @@ doc/src/sgml/client-auth.sgml: host ... radius radiusservers="server1,server2" r + issuer + + -+ The URL of the OAuth issuing party, which the client -+ must contact to receive a bearer token. This parameter is required. ++ The issuer identifier of the authorization server, as defined by its ++ discovery document, or a well-known URI pointing to that discovery ++ document. This parameter is required. + ++ ++ When an OAuth client connects to the server, a discovery document URI ++ will be constructed using the issuer identifier. By default, the URI ++ uses the conventions of OpenID Connect Discovery: the path ++ /.well-known/openid-configuration will be appended ++ to the issuer identifier. Alternatively, if the ++ issuer contains a /.well-known/ ++ path segment, the URI will be provided to the client as-is. ++ ++ ++ ++ The OAuth client in libpq requires the server's issuer setting to ++ exactly match the issuer identifier which is provided in the discovery ++ document, which must in turn match the client's ++ setting. No variations in ++ case or format are permitted. ++ ++ + + + @@ doc/src/sgml/client-auth.sgml: host ... radius radiusservers="server1,server2" r + + + ++ validator ++ ++ ++ The library to use for validating bearer tokens. If given, the name must ++ exactly match one of the libraries listed in ++ . This parameter is ++ optional unless oauth_validator_libraries contains ++ more than one library, in which case it is required. ++ ++ ++ ++ ++ + map + + @@ doc/src/sgml/config.sgml: include_dir 'conf.d' + -+ -+ oauth_validator_library (string) ++ ++ oauth_validator_libraries (string) + -+ oauth_validator_library configuration parameter ++ oauth_validator_libraries configuration parameter + + + + -+ The library to use for validating OAuth connection tokens. If set to -+ an empty string (the default), OAuth connections will be refused. For -+ more information on implementing OAuth validators see ++ The library/libraries to use for validating OAuth connection tokens. If ++ only one validator library is provided, it will be used by default for ++ any OAuth connections; otherwise, all ++ oauth HBA entries ++ must explicitly set a validator chosen from this ++ list. If set to an empty string (the default), OAuth connections will be ++ refused. For more information on implementing OAuth validators see + . This parameter can only be set in + the postgresql.conf file. + @@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + ++ ++ oauth_issuer ++ ++ ++ The HTTPS URL of an issuer to contact if the server requests an OAuth ++ token for the connection. This parameter is required for all OAuth ++ connections; it should exactly match the issuer ++ setting in the server's HBA configuration. ++ ++ ++ As part of the standard authentication handshake, libpq will ask the ++ server for a discovery document: a URI providing a ++ set of OAuth configuration parameters. The server must provide a URI ++ that is directly constructed from the components of the ++ oauth_issuer, and this value must exactly match the ++ issuer identifier that is declared in the discovery document itself, or ++ the connection will fail. This is required to prevent a class of "mix-up ++ attacks" on OAuth clients. ++ ++ ++ This standard handshake requires two separate network connections to the ++ server per authentication attempt. To skip asking the server for a ++ discovery document URI, you may set oauth_issuer to a ++ /.well-known/ URI used for OAuth discovery. (In this ++ case, it is recommended that ++ be set as well, since the ++ client will not have a chance to ask the server for a correct scope ++ setting, and the default scopes for a token may not be sufficient to ++ connect.) libpq currently supports the following well-known endpoints: ++ ++ /.well-known/openid-configuration ++ /.well-known/oauth-authorization-server ++ ++ ++ ++ ++ + + oauth_client_id + @@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + + -+ -+ oauth_issuer -+ -+ -+ The HTTPS URL of an issuer to contact if the server requests an OAuth -+ token for the connection. This parameter is optional and intended for -+ advanced usage; see also . -+ -+ -+ If no oauth_issuer is provided, the client will ask -+ the PostgreSQL server to provide an -+ acceptable issuer URL (as configured in its -+ HBA settings). This is convenient, but -+ it requires two separate network connections to the server per attempt. -+ -+ -+ Providing an explicit oauth_issuer (and, typically, -+ an accompanying oauth_scope) skips this initial -+ "discovery" phase, which may speed up certain custom OAuth flows. -+ -+ This parameter may also be set defensively, to prevent the backend -+ server from directing the client to arbitrary URLs. -+ However: if the client's issuer setting differs -+ from the server's expected issuer, the server is likely to reject the -+ issued token, and the connection will fail. -+ -+ -+ -+ + + oauth_scope + @@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + identifiers. This parameter is optional and intended for advanced usage. + + -+ -+ If neither nor -+ oauth_scope is specified, the client will obtain -+ appropriate scope settings from the -+ PostgreSQL server. Otherwise, the value of -+ this parameter will be used. Similarly to -+ oauth_issuer, if the client's scope setting does not -+ contain the server's required scopes, the server is likely to reject the -+ issued token, and the connection will fail. ++ Usually the client will obtain appropriate scope settings from the ++ PostgreSQL server. If this parameter is used, ++ the server's requested scope list will be ignored. This can prevent a ++ less-trusted server from requesting inappropriate access scopes from the ++ end user. However, if the client's scope setting does not contain the ++ server's required scopes, the server is likely to reject the issued ++ token, and the connection will fail. + + + The meaning of an empty scope list is provider-dependent. An OAuth @@ doc/src/sgml/oauth-validators.sgml (new) + _PG_oauth_validator_module_init + + -+ An OAuth validator module is loaded by dynamically loading a shared library -+ with the 's name as the library -+ base name. The normal library search path is used to locate the library. To ++ An OAuth validator module is loaded by dynamically loading one of the shared ++ libraries listed in . ++ The normal library search path is used to locate the library. To + provide the validator callbacks and to indicate that the library is an OAuth + validator module a function named + _PG_oauth_validator_module_init must be provided. The @@ src/backend/libpq/auth-oauth.c (new) +#include "storage/fd.h" +#include "storage/ipc.h" +#include "utils/json.h" ++#include "utils/varlena.h" + +/* GUC */ -+char *OAuthValidatorLibrary = ""; ++char *oauth_validator_libraries_string = NULL; + +static void oauth_get_mechanisms(Port *port, StringInfo buf); +static void *oauth_init(Port *port, const char *selected_mech, const char *shadow_pass); +static int oauth_exchange(void *opaq, const char *input, int inputlen, + char **output, int *outputlen, const char **logdetail); + -+static void load_validator_library(void); ++static void load_validator_library(const char *libname); +static void shutdown_validator_library(int code, Datum arg); + +static ValidatorModuleState *validator_module_state; @@ src/backend/libpq/auth-oauth.c (new) + ctx->issuer = port->hba->oauth_issuer; + ctx->scope = port->hba->oauth_scope; + -+ load_validator_library(); ++ load_validator_library(port->hba->oauth_validator); + + return ctx; +} @@ src/backend/libpq/auth-oauth.c (new) + errmsg("OAuth is not properly configured for this user"), + errdetail_log("The issuer and scope parameters must be set in pg_hba.conf.")); + -+ /*------ -+ * Build the .well-known URI based on our issuer. -+ * TODO: RFC 8414 defines a competing well-known URI, so we'll probably -+ * have to make this configurable too. ++ /* ++ * Build a default .well-known URI based on our issuer, unless the HBA has ++ * already provided one. + */ + initStringInfo(&issuer); + appendStringInfoString(&issuer, ctx->issuer); -+ appendStringInfoString(&issuer, "/.well-known/openid-configuration"); ++ if (strstr(ctx->issuer, "/.well-known/") == NULL) ++ appendStringInfoString(&issuer, "/.well-known/openid-configuration"); + + initStringInfo(&buf); + @@ src/backend/libpq/auth-oauth.c (new) + * since token validation won't be possible. + */ +static void -+load_validator_library(void) ++load_validator_library(const char *libname) +{ + OAuthValidatorModuleInit validator_init; + -+ if (OAuthValidatorLibrary[0] == '\0') -+ ereport(ERROR, -+ errcode(ERRCODE_INVALID_PARAMETER_VALUE), -+ errmsg("oauth_validator_library is not set")); ++ Assert(libname && *libname); + + validator_init = (OAuthValidatorModuleInit) -+ load_external_function(OAuthValidatorLibrary, -+ "_PG_oauth_validator_module_init", false, NULL); ++ load_external_function(libname, "_PG_oauth_validator_module_init", ++ false, NULL); + + /* + * The validator init function is required since it will set the callbacks @@ src/backend/libpq/auth-oauth.c (new) + */ + if (validator_init == NULL) + ereport(ERROR, -+ errmsg("%s modules \"%s\" have to define the symbol %s", -+ "OAuth validator", OAuthValidatorLibrary, "_PG_oauth_validator_module_init")); ++ errmsg("%s module \"%s\" must define the symbol %s", ++ "OAuth validator", libname, "_PG_oauth_validator_module_init")); + + ValidatorCallbacks = (*validator_init) (); + @@ src/backend/libpq/auth-oauth.c (new) +{ + if (ValidatorCallbacks->shutdown_cb != NULL) + ValidatorCallbacks->shutdown_cb(validator_module_state); ++} ++ ++/* ++ * Ensure an OAuth validator named in the HBA is permitted by the configuration. ++ * ++ * If the validator is currently unset and exactly one library is declared in ++ * oauth_validator_libraries, then that library will be used as the validator. ++ * Otherwise the name must be present in the list of oauth_validator_libraries. ++ */ ++bool ++check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg) ++{ ++ int line_num = hbaline->linenumber; ++ char *file_name = hbaline->sourcefile; ++ char *rawstring; ++ List *elemlist = NIL; ++ ListCell *l; ++ ++ *err_msg = NULL; ++ ++ if (oauth_validator_libraries_string[0] == '\0') ++ { ++ ereport(elevel, ++ errcode(ERRCODE_CONFIG_FILE_ERROR), ++ errmsg("oauth_validator_libraries must be set for authentication method %s", ++ "oauth"), ++ errcontext("line %d of configuration file \"%s\"", ++ line_num, file_name)); ++ *err_msg = psprintf("oauth_validator_libraries must be set for authentication method %s", ++ "oauth"); ++ return false; ++ } ++ ++ /* SplitDirectoriesString needs a modifiable copy */ ++ rawstring = pstrdup(oauth_validator_libraries_string); ++ ++ if (!SplitDirectoriesString(rawstring, ',', &elemlist)) ++ { ++ /* syntax error in list */ ++ ereport(elevel, ++ errcode(ERRCODE_CONFIG_FILE_ERROR), ++ errmsg("invalid list syntax in parameter \"%s\"", ++ "oauth_validator_libraries")); ++ *err_msg = psprintf("invalid list syntax in parameter \"%s\"", ++ "oauth_validator_libraries"); ++ goto done; ++ } ++ ++ if (!hbaline->oauth_validator) ++ { ++ if (elemlist->length == 1) ++ { ++ hbaline->oauth_validator = pstrdup(linitial(elemlist)); ++ goto done; ++ } ++ ++ ereport(elevel, ++ errcode(ERRCODE_CONFIG_FILE_ERROR), ++ errmsg("authentication method \"oauth\" requires argument \"validator\" to be set when oauth_validator_libraries contains multiple options"), ++ errcontext("line %d of configuration file \"%s\"", ++ line_num, file_name)); ++ *err_msg = "authentication method \"oauth\" requires argument \"validator\" to be set when oauth_validator_libraries contains multiple options"; ++ goto done; ++ } ++ ++ foreach(l, elemlist) ++ { ++ char *allowed = lfirst(l); ++ ++ if (strcmp(allowed, hbaline->oauth_validator) == 0) ++ goto done; ++ } ++ ++ ereport(elevel, ++ errcode(ERRCODE_INVALID_PARAMETER_VALUE), ++ errmsg("validator \"%s\" is not permitted by %s", ++ hbaline->oauth_validator, "oauth_validator_libraries"), ++ errcontext("line %d of configuration file \"%s\"", ++ line_num, file_name)); ++ *err_msg = psprintf("validator \"%s\" is not permitted by %s", ++ hbaline->oauth_validator, "oauth_validator_libraries"); ++ ++done: ++ list_free_deep(elemlist); ++ pfree(rawstring); ++ ++ return (*err_msg == NULL); +} ## src/backend/libpq/auth.c ## @@ src/backend/libpq/auth.c: ClientAuthentication(Port *port) if ((status == STATUS_OK && port->hba->clientcert == clientCertFull) ## src/backend/libpq/hba.c ## +@@ + #include "libpq/hba.h" + #include "libpq/ifaddr.h" + #include "libpq/libpq-be.h" ++#include "libpq/oauth.h" + #include "postmaster/postmaster.h" + #include "regex/regex.h" + #include "replication/walsender.h" @@ src/backend/libpq/hba.c: static const char *const UserAuthName[] = "ldap", "cert", @@ src/backend/libpq/hba.c: parse_hba_line(TokenizedAuthLine *tok_line, int elevel) + MANDATORY_AUTH_ARG(parsedline->oauth_scope, "scope", "oauth"); + MANDATORY_AUTH_ARG(parsedline->oauth_issuer, "issuer", "oauth"); + ++ /* Ensure a validator library is set and permitted by the config. */ ++ if (!check_oauth_validator(parsedline, elevel, err_msg)) ++ return NULL; ++ + /* + * Supplying a usermap combined with the option to skip usermapping is + * nonsensical and indicates a configuration error. @@ src/backend/libpq/hba.c: parse_hba_auth_opt(char *name, char *val, HbaLine *hbal + REQUIRE_AUTH_OPTION(uaOAuth, "scope", "oauth"); + hbaline->oauth_scope = pstrdup(val); + } ++ else if (strcmp(name, "validator") == 0) ++ { ++ REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); ++ hbaline->oauth_validator = pstrdup(val); ++ } + else if (strcmp(name, "trust_validator_authz") == 0) + { + REQUIRE_AUTH_OPTION(uaOAuth, "trust_validator_authz", "oauth"); @@ src/backend/utils/misc/guc_tables.c: struct config_string ConfigureNamesString[] }, + { -+ {"oauth_validator_library", PGC_SIGHUP, CONN_AUTH_AUTH, -+ gettext_noop("Sets the library that will be called to validate OAuth v2 bearer tokens."), ++ {"oauth_validator_libraries", PGC_SIGHUP, CONN_AUTH_AUTH, ++ gettext_noop("Lists libraries that may be called to validate OAuth v2 bearer tokens."), + NULL, -+ GUC_SUPERUSER_ONLY | GUC_NOT_IN_SAMPLE ++ GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_SUPERUSER_ONLY + }, -+ &OAuthValidatorLibrary, ++ &oauth_validator_libraries_string, + "", + NULL, NULL, NULL + }, @@ src/backend/utils/misc/guc_tables.c: struct config_string ConfigureNamesString[] { {NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL + ## src/backend/utils/misc/postgresql.conf.sample ## +@@ + #ssl_passphrase_command = '' + #ssl_passphrase_command_supports_reload = off + ++# OAuth ++#oauth_validator_libraries = '' ++ + + #------------------------------------------------------------------------------ + # RESOURCE USAGE (except WAL) + ## src/include/common/oauth-common.h (new) ## @@ +/*------------------------------------------------------------------------- @@ src/include/libpq/hba.h: typedef struct HbaLine char *radiusports_s; + char *oauth_issuer; + char *oauth_scope; ++ char *oauth_validator; + bool oauth_skip_usermap; } HbaLine; @@ src/include/libpq/oauth.h (new) +#include "libpq/libpq-be.h" +#include "libpq/sasl.h" + -+extern PGDLLIMPORT char *OAuthValidatorLibrary; ++extern PGDLLIMPORT char *oauth_validator_libraries_string; + +typedef struct ValidatorModuleState +{ @@ src/include/libpq/oauth.h (new) +/* Implementation */ +extern 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 *hba, int elevel, char **err_msg); ++ +#endif /* PG_OAUTH_H */ ## src/include/pg_config.h.in ## @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + CHECK_SETOPT(actx, popt, protos, return false); + } + ++ /* TODO: would anyone use this in "real" situations, or just testing? */ ++ if (actx->debugging) ++ { ++ const char *env; ++ ++ if ((env = getenv("PGOAUTHCAFILE")) != NULL) ++ CHECK_SETOPT(actx, CURLOPT_CAINFO, env, return false); ++ } ++ + /* + * Suppress the Accept header to make our request as minimal as possible. + * (Ideally we would set it to "application/json" instead, but OpenID is @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + + if (actx->running) + { -+ /* ++ /*--- + * There's an async request in progress. Pump the multi handle. + * -+ * TODO: curl_multi_socket_all() is deprecated, presumably because -+ * it's inefficient and pointless if your event loop has already -+ * handed you the exact sockets that are ready. But that's not our use -+ * case -- our client has no way to tell us which sockets are ready. -+ * (They don't even know there are sockets to begin with.) ++ * curl_multi_socket_all() is officially deprecated, because it's ++ * inefficient and pointless if your event loop has already handed you ++ * the exact sockets that are ready. But that's not our use case -- ++ * our client has no way to tell us which sockets are ready. (They ++ * don't even know there are sockets to begin with.) + * + * We can grab the list of triggered events from the multiplexer + * ourselves, but that's effectively what curl_multi_socket_all() is -+ * going to do... so it appears to be exactly the API we need. ++ * going to do. And there are currently no plans for the Curl project ++ * to remove or break this API, so ignore the deprecation. See ++ * ++ * https://curl.se/mail/lib-2024-11/0028.html + * -+ * Ignore the deprecation for now. This needs a followup on -+ * curl-library@, to make sure we're not shooting ourselves in the -+ * foot in some other way. + */ + CURL_IGNORE_DEPRECATION( + err = curl_multi_socket_all(actx->curlm, &actx->running); @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + return true; +} + ++/* ++ * Ensure that the discovery document is provided by the expected issuer. ++ * Currently, issuers are statically configured in the connection string. ++ */ ++static bool ++check_issuer(struct async_ctx *actx, PGconn *conn) ++{ ++ const struct provider *provider = &actx->provider; ++ ++ Assert(conn->oauth_issuer_id); /* ensured by setup_oauth_parameters() */ ++ Assert(provider->issuer); /* ensured by parse_provider() */ ++ ++ /*--- ++ * We require strict equality for issuer identifiers -- no path or case ++ * normalization, no substitution of default ports and schemes, etc. This ++ * is done to match the rules in OIDC Discovery Sec. 4.3 for config ++ * validation: ++ * ++ * The issuer value returned MUST be identical to the Issuer URL that ++ * was used as the prefix to /.well-known/openid-configuration to ++ * retrieve the configuration information. ++ * ++ * as well as the rules set out in RFC 9207 for avoiding mix-up attacks: ++ * ++ * Clients MUST then [...] compare the result to the issuer identifier ++ * of the authorization server where the authorization request was ++ * sent to. This comparison MUST use simple string comparison as defined ++ * in Section 6.2.1 of [RFC3986]. ++ * ++ * TODO: Encoding support? ++ */ ++ if (strcmp(conn->oauth_issuer_id, provider->issuer) != 0) ++ { ++ actx_error(actx, ++ "the issuer identifier (%s) does not match oauth_issuer (%s)", ++ provider->issuer, conn->oauth_issuer_id); ++ return false; ++ } ++ ++ return true; ++} ++ +#define OAUTH_GRANT_TYPE_DEVICE_CODE "urn:ietf:params:oauth:grant-type:device_code" + +/* @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + const struct curl_slist *grant; + bool device_grant_found = false; + -+ Assert(provider->issuer); /* ensured by get_discovery_document() */ ++ Assert(provider->issuer); /* ensured by parse_provider() */ + + /*------ + * First, sanity checks for discovery contents that are OPTIONAL in the @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + const char *device_authz_uri = actx->provider.device_authorization_endpoint; + PQExpBuffer work_buffer = &actx->work_data; + -+ Assert(conn->oauth_client_id); /* ensured by get_auth_token() */ ++ Assert(conn->oauth_client_id); /* ensured by setup_oauth_parameters() */ + Assert(device_authz_uri); /* ensured by check_for_device_flow() */ + + /* Construct our request body. */ @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + const char *device_code = actx->authz.device_code; + PQExpBuffer work_buffer = &actx->work_data; + -+ Assert(conn->oauth_client_id); /* ensured by get_auth_token() */ -+ Assert(token_uri); /* ensured by get_discovery_document() */ -+ Assert(device_code); /* ensured by run_device_authz() */ ++ Assert(conn->oauth_client_id); /* ensured by setup_oauth_parameters() */ ++ Assert(token_uri); /* ensured by parse_provider() */ ++ Assert(device_code); /* ensured by parse_device_authz() */ + + /* Construct our request body. */ + resetPQExpBuffer(work_buffer); @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + + if (!state->async_ctx) + { -+ const char *env; -+ + /* + * Create our asynchronous state, and hook it into the upper-level + * OAuth state immediately, so any failures below won't leak the @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +#endif + + /* Should we enable unsafe features? */ -+ env = getenv("PGOAUTHDEBUG"); -+ if (env && strcmp(env, "UNSAFE") == 0) -+ actx->debugging = true; ++ actx->debugging = oauth_unsafe_debugging_enabled(); + + state->async_ctx = actx; + state->free_async_ctx = free_curl_async_ctx; @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + if (!finish_discovery(actx)) + goto error_return; + -+ /* TODO: check issuer */ ++ if (!check_issuer(actx, conn)) ++ goto error_return; + + actx->errctx = "cannot run OAuth device authorization"; + if (!check_for_device_flow(actx)) @@ src/interfaces/libpq/fe-auth-oauth.c (new) + * TODO: users shouldn't see this; what action should they take if + * they do? + */ -+ libpq_append_conn_error(conn, "no OAuth token was set for the connection"); ++ libpq_append_conn_error(conn, ++ "no OAuth token was set for the connection"); + return NULL; + } + } @@ src/interfaces/libpq/fe-auth-oauth.c (new) + return oauth_json_has_error(ctx) ? JSON_SEM_ACTION_FAILED : JSON_SUCCESS; +} + ++#define HTTPS_SCHEME "https://" ++#define HTTP_SCHEME "http://" ++ ++/* We support both well-known suffixes defined by RFC 8414. */ ++#define WK_PREFIX "/.well-known/" ++#define OPENID_WK_SUFFIX "openid-configuration" ++#define OAUTH_WK_SUFFIX "oauth-authorization-server" ++ ++/* ++ * Derives an issuer identifier from one of our recognized .well-known URIs, ++ * using the rules in RFC 8414. ++ */ ++static char * ++issuer_from_well_known_uri(PGconn *conn, const char *wkuri) ++{ ++ const char *authority_start = NULL; ++ const char *wk_start; ++ const char *wk_end; ++ char *issuer; ++ ptrdiff_t start_offset, ++ end_offset; ++ size_t end_len; ++ ++ /* ++ * https:// is required for issuer identifiers (RFC 8414, Sec. 2; OIDC ++ * Discovery 1.0, Sec. 3). This is a case-insensitive comparison at this ++ * level (but issuer identifier comparison at the level above this is ++ * case-sensitive, so in practice it's probably moot). ++ */ ++ if (pg_strncasecmp(wkuri, HTTPS_SCHEME, strlen(HTTPS_SCHEME)) == 0) ++ authority_start = wkuri + strlen(HTTPS_SCHEME); ++ ++ if (!authority_start ++ && oauth_unsafe_debugging_enabled() ++ && pg_strncasecmp(wkuri, HTTP_SCHEME, strlen(HTTP_SCHEME)) == 0) ++ { ++ /* Allow http:// for testing only. */ ++ authority_start = wkuri + strlen(HTTP_SCHEME); ++ } ++ ++ if (!authority_start) ++ { ++ libpq_append_conn_error(conn, ++ "OAuth discovery URI \"%s\" must use HTTPS", ++ wkuri); ++ return NULL; ++ } ++ ++ /* ++ * Well-known URIs in general may support queries and fragments, but the ++ * two types we support here do not. (They must be constructed from the ++ * components of issuer identifiers, which themselves may not contain any ++ * queries or fragments.) ++ * ++ * It's important to check this first, to avoid getting tricked later by a ++ * prefix buried inside a query or fragment. ++ */ ++ if (strpbrk(authority_start, "?#") != NULL) ++ { ++ libpq_append_conn_error(conn, ++ "OAuth discovery URI \"%s\" must not contain query or fragment components", ++ wkuri); ++ return NULL; ++ } ++ ++ /* ++ * Find the start of the .well-known prefix. IETF rules state this must be ++ * at the beginning of the path component, but OIDC defined it at the end ++ * instead, so we have to search for it anywhere. ++ */ ++ wk_start = strstr(authority_start, WK_PREFIX); ++ if (!wk_start) ++ { ++ libpq_append_conn_error(conn, ++ "OAuth discovery URI \"%s\" is not a .well-known URI", ++ wkuri); ++ return NULL; ++ } ++ ++ /* ++ * Now find the suffix type. We only support the two defined in OIDC ++ * Discovery 1.0 and RFC 8414. ++ */ ++ wk_end = wk_start + strlen(WK_PREFIX); ++ ++ if (strncmp(wk_end, OPENID_WK_SUFFIX, strlen(OPENID_WK_SUFFIX)) == 0) ++ wk_end += strlen(OPENID_WK_SUFFIX); ++ else if (strncmp(wk_end, OAUTH_WK_SUFFIX, strlen(OAUTH_WK_SUFFIX)) == 0) ++ wk_end += strlen(OAUTH_WK_SUFFIX); ++ else ++ wk_end = NULL; ++ ++ /* ++ * Even if there's a match, we still need to check to make sure the suffix ++ * takes up the entire path segment, to weed out constructions like ++ * "/.well-known/openid-configuration-bad". ++ */ ++ if (!wk_end || (*wk_end != '/' && *wk_end != '\0')) ++ { ++ libpq_append_conn_error(conn, ++ "OAuth discovery URI \"%s\" uses an unsupported .well-known suffix", ++ wkuri); ++ return NULL; ++ } ++ ++ /* ++ * Finally, make sure the .well-known components are provided either as a ++ * prefix (IETF style) or as a postfix (OIDC style). In other words, ++ * "https://localhost/a/.well-known/openid-configuration/b" is not allowed ++ * to claim association with "https://localhost/a/b". ++ */ ++ if (*wk_end != '\0') ++ { ++ /* ++ * It's not at the end, so it's required to be at the beginning at the ++ * path. Find the starting slash. ++ */ ++ const char *path_start; ++ ++ path_start = strchr(authority_start, '/'); ++ Assert(path_start); /* otherwise we wouldn't have found WK_PREFIX */ ++ ++ if (wk_start != path_start) ++ { ++ libpq_append_conn_error(conn, ++ "OAuth discovery URI \"%s\" uses an invalid format", ++ wkuri); ++ return NULL; ++ } ++ } ++ ++ /* Checks passed! Now build the issuer. */ ++ issuer = strdup(wkuri); ++ if (!issuer) ++ { ++ libpq_append_conn_error(conn, "out of memory"); ++ return NULL; ++ } ++ ++ /* ++ * The .well-known components are from [wk_start, wk_end). Remove those to ++ * form the issuer ID, by shifting the path suffix (which may be empty) ++ * leftwards. ++ */ ++ start_offset = wk_start - wkuri; ++ end_offset = wk_end - wkuri; ++ end_len = strlen(wk_end) + 1; /* move the NULL terminator too */ ++ ++ memmove(issuer + start_offset, issuer + end_offset, end_len); ++ ++ return issuer; ++} ++ +static bool +handle_oauth_sasl_error(PGconn *conn, char *msg, int msglen) +{ @@ src/interfaces/libpq/fe-auth-oauth.c (new) + /* Sanity check. */ + if (strlen(msg) != msglen) + { -+ appendPQExpBufferStr(&conn->errorMessage, -+ libpq_gettext("server's error message contained an embedded NULL, and was discarded")); ++ libpq_append_conn_error(conn, ++ "server's error message contained an embedded NULL, and was discarded"); + return false; + } + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + errmsg = json_errdetail(err, &lex); + + if (errmsg) -+ appendPQExpBuffer(&conn->errorMessage, -+ libpq_gettext("failed to parse server's error response: %s"), -+ errmsg); ++ libpq_append_conn_error(conn, ++ "failed to parse server's error response: %s", ++ errmsg); + + /* Don't need the error buffer or the JSON lexer anymore. */ + termPQExpBuffer(&ctx.errbuf); @@ src/interfaces/libpq/fe-auth-oauth.c (new) + return false; + + /* TODO: what if these override what the user already specified? */ ++ /* TODO: what if there's no discovery URI? */ + if (ctx.discovery_uri) + { ++ char *discovery_issuer; ++ ++ /* The URI must correspond to our existing issuer, to avoid mix-ups. */ ++ discovery_issuer = issuer_from_well_known_uri(conn, ctx.discovery_uri); ++ if (!discovery_issuer) ++ return false; /* error message already set */ ++ ++ if (strcmp(conn->oauth_issuer_id, discovery_issuer) != 0) ++ { ++ libpq_append_conn_error(conn, ++ "server's discovery document at %s (issuer \"%s\") is incompatible with oauth_issuer (%s)", ++ ctx.discovery_uri, discovery_issuer, ++ conn->oauth_issuer_id); ++ ++ free(discovery_issuer); ++ return false; ++ } ++ ++ free(discovery_issuer); ++ + if (conn->oauth_discovery_uri) + free(conn->oauth_discovery_uri); + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + + if (!ctx.status) + { -+ appendPQExpBuffer(&conn->errorMessage, -+ libpq_gettext("server sent error response without a status")); ++ libpq_append_conn_error(conn, ++ "server sent error response without a status"); + return false; + } + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + + if (!request->async) + { -+ libpq_append_conn_error(conn, "user-defined OAuth flow provided neither a token nor an async callback"); ++ libpq_append_conn_error(conn, ++ "user-defined OAuth flow provided neither a token nor an async callback"); + return PGRES_POLLING_FAILED; + } + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + */ + if (!request->token) + { -+ libpq_append_conn_error(conn, "user-defined OAuth flow did not provide a token"); ++ libpq_append_conn_error(conn, ++ "user-defined OAuth flow did not provide a token"); + return PGRES_POLLING_FAILED; + } + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + return false; +} + ++/* ++ * Fill in our issuer identifier and discovery URI, if possible, using the ++ * connection parameters. If conn->oauth_discovery_uri can't be populated in ++ * this function, it will be requested from the server. ++ */ +static bool -+derive_discovery_uri(PGconn *conn) ++setup_oauth_parameters(PGconn *conn) +{ -+ PQExpBufferData discovery_buf; -+ -+ if (conn->oauth_discovery_uri || !conn->oauth_issuer) ++ /*--- ++ * To talk to a server, we require the user to provide issuer and client ++ * identifiers. ++ * ++ * While it's possible for an OAuth client to support multiple issuers, it ++ * requires additional effort to make sure the flows in use are safe -- to ++ * quote RFC 9207, ++ * ++ * OAuth clients that interact with only one authorization server are ++ * not vulnerable to mix-up attacks. However, when such clients decide ++ * to add support for a second authorization server in the future, they ++ * become vulnerable and need to apply countermeasures to mix-up ++ * attacks. ++ * ++ * For now, we allow only one. ++ */ ++ if (!conn->oauth_issuer || !conn->oauth_client_id) + { -+ /* -+ * Either we already have one, or we aren't able to derive one -+ * ourselves. The latter case is not an error condition; we'll just -+ * ask the server to provide one for us. -+ */ -+ return true; ++ libpq_append_conn_error(conn, ++ "server requires OAuth authentication, but oauth_issuer and oauth_client_id are not both set"); ++ return false; + } + -+ initPQExpBuffer(&discovery_buf); -+ -+ Assert(!conn->oauth_discovery_uri); -+ Assert(conn->oauth_issuer); -+ + /* -+ * If we don't yet have a discovery URI, but the user gave us an explicit -+ * issuer, use the .well-known discovery URI for that issuer. ++ * oauth_issuer is interpreted differently if it's a well-known discovery ++ * URI rather than just an issuer identifier. + */ -+ appendPQExpBufferStr(&discovery_buf, conn->oauth_issuer); -+ appendPQExpBufferStr(&discovery_buf, "/.well-known/openid-configuration"); -+ -+ if (PQExpBufferDataBroken(discovery_buf)) -+ goto cleanup; ++ if (strstr(conn->oauth_issuer, WK_PREFIX) != NULL) ++ { ++ /* ++ * Convert the URI back to an issuer identifier. (This also performs ++ * validation of the URI format.) ++ */ ++ conn->oauth_issuer_id = issuer_from_well_known_uri(conn, ++ conn->oauth_issuer); ++ if (!conn->oauth_issuer_id) ++ return false; /* error message already set */ + -+ conn->oauth_discovery_uri = strdup(discovery_buf.data); ++ conn->oauth_discovery_uri = strdup(conn->oauth_issuer); ++ if (!conn->oauth_discovery_uri) ++ { ++ libpq_append_conn_error(conn, "out of memory"); ++ return false; ++ } ++ } ++ else ++ { ++ /* ++ * Treat oauth_issuer as an issuer identifier. We'll ask the server ++ * for the discovery URI. ++ */ ++ conn->oauth_issuer_id = strdup(conn->oauth_issuer); ++ if (!conn->oauth_issuer_id) ++ { ++ libpq_append_conn_error(conn, "out of memory"); ++ return false; ++ } ++ } + -+cleanup: -+ termPQExpBuffer(&discovery_buf); -+ return (conn->oauth_discovery_uri != NULL); ++ return true; +} + +static SASLStatus @@ src/interfaces/libpq/fe-auth-oauth.c (new) + /* We begin in the initial response phase. */ + Assert(inputlen == -1); + -+ if (!derive_discovery_uri(conn)) ++ if (!setup_oauth_parameters(conn)) + return SASL_FAILED; + + if (conn->oauth_discovery_uri) + { -+ if (!conn->oauth_client_id) -+ { -+ /* We can't talk to a server without a client identifier. */ -+ libpq_append_conn_error(conn, "no oauth_client_id is set for the connection"); -+ return SASL_FAILED; -+ } -+ + /* + * Decide whether we're using a user-provided OAuth flow, or + * the one we have built in. @@ src/interfaces/libpq/fe-auth-oauth.c (new) + * successfully after telling us it was going to fail. Neither is + * acceptable. + */ -+ appendPQExpBufferStr(&conn->errorMessage, -+ libpq_gettext("server sent additional OAuth data after error\n")); ++ libpq_append_conn_error(conn, ++ "server sent additional OAuth data after error"); + return SASL_FAILED; + + default: -+ appendPQExpBufferStr(&conn->errorMessage, -+ libpq_gettext("invalid OAuth exchange state\n")); ++ libpq_append_conn_error(conn, "invalid OAuth exchange state"); + break; + } + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + state->free_async_ctx(state->conn, state->async_ctx); + + free(state); ++} ++ ++/* ++ * Returns true if the PGOAUTHDEBUG=UNSAFE flag is set in the environment. ++ */ ++bool ++oauth_unsafe_debugging_enabled(void) ++{ ++ const char *env = getenv("PGOAUTHDEBUG"); ++ ++ return (env && strcmp(env, "UNSAFE") == 0); +} ## src/interfaces/libpq/fe-auth-oauth.h (new) ## @@ src/interfaces/libpq/fe-auth-oauth.h (new) +} fe_oauth_state; + +extern PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn, pgsocket *altsock); ++extern bool oauth_unsafe_debugging_enabled(void); + +#endif /* FE_AUTH_OAUTH_H */ @@ src/interfaces/libpq/fe-connect.c: freePGconn(PGconn *conn) free(conn->target_session_attrs); free(conn->load_balance_hosts); + free(conn->oauth_issuer); ++ free(conn->oauth_issuer_id); + free(conn->oauth_discovery_uri); + free(conn->oauth_client_id); + free(conn->oauth_client_secret); @@ src/interfaces/libpq/libpq-int.h: struct pg_conn * connection that's used for queries */ + /* OAuth v2 */ -+ char *oauth_issuer; /* token issuer URL */ ++ char *oauth_issuer; /* token issuer/URL */ ++ char *oauth_issuer_id; /* token issuer identifier */ + char *oauth_discovery_uri; /* URI of the issuer's discovery + * document */ + char *oauth_client_id; /* client identifier */ @@ src/test/modules/oauth_validator/Makefile (new) +# +#------------------------------------------------------------------------- + -+MODULES = validator ++MODULES = validator fail_validator +PGFILEDESC = "validator - test OAuth validator module" + +PROGRAM = oauth_hook_client @@ src/test/modules/oauth_validator/Makefile (new) + +endif + ## src/test/modules/oauth_validator/fail_validator.c (new) ## +@@ ++/*------------------------------------------------------------------------- ++ * ++ * fail_validator.c ++ * Test module for serverside OAuth token validation callbacks, which always ++ * fails ++ * ++ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group ++ * Portions Copyright (c) 1994, Regents of the University of California ++ * ++ * src/test/modules/oauth_validator/fail_validator.c ++ * ++ *------------------------------------------------------------------------- ++ */ ++ ++#include "postgres.h" ++ ++#include "fmgr.h" ++#include "libpq/oauth.h" ++#include "miscadmin.h" ++#include "utils/guc.h" ++#include "utils/memutils.h" ++ ++PG_MODULE_MAGIC; ++ ++static ValidatorModuleResult *fail_token(ValidatorModuleState *state, ++ const char *token, ++ const char *role); ++ ++static const OAuthValidatorCallbacks validator_callbacks = { ++ .validate_cb = fail_token, ++}; ++ ++const OAuthValidatorCallbacks * ++_PG_oauth_validator_module_init(void) ++{ ++ return &validator_callbacks; ++} ++ ++static ValidatorModuleResult * ++fail_token(ValidatorModuleState *state, const char *token, const char *role) ++{ ++ elog(FATAL, "fail_validator: sentinel error"); ++ pg_unreachable(); ++} + ## src/test/modules/oauth_validator/meson.build (new) ## @@ +# Copyright (c) 2024, PostgreSQL Global Development Group @@ src/test/modules/oauth_validator/meson.build (new) +) +test_install_libs += validator + ++fail_validator_sources = files( ++ 'fail_validator.c', ++) ++ ++if host_system == 'windows' ++ fail_validator_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ ++ '--NAME', 'fail_validator', ++ '--FILEDESC', 'fail_validator - failing OAuth validator module',]) ++endif ++ ++fail_validator = shared_module('fail_validator', ++ fail_validator_sources, ++ kwargs: pg_test_mod_args, ++) ++test_install_libs += fail_validator ++ +oauth_hook_client_sources = files( + 'oauth_hook_client.c', +) @@ src/test/modules/oauth_validator/t/001_server.pl (new) +use strict; +use warnings FATAL => 'all'; + -+use JSON::PP qw(encode_json); ++use JSON::PP qw(encode_json); +use MIME::Base64 qw(encode_base64); +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; @@ src/test/modules/oauth_validator/t/001_server.pl (new) +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init; +$node->append_conf('postgresql.conf', "log_connections = on\n"); -+$node->append_conf('postgresql.conf', "shared_preload_libraries = 'validator'\n"); -+$node->append_conf('postgresql.conf', "oauth_validator_library = 'validator'\n"); ++$node->append_conf('postgresql.conf', ++ "oauth_validator_libraries = 'validator'\n"); +$node->start; + +$node->safe_psql('postgres', 'CREATE USER test;'); @@ src/test/modules/oauth_validator/t/001_server.pl (new) +{ + my $exit_code = $?; + -+ $webserver->stop() if defined $webserver; # might have been SKIP'd ++ $webserver->stop() if defined $webserver; # might have been SKIP'd + + $? = $exit_code; +} + +my $port = $webserver->port(); -+my $issuer = "127.0.0.1:$port"; ++my $issuer = "http://localhost:$port"; + +unlink($node->data_dir . '/pg_hba.conf'); -+$node->append_conf('pg_hba.conf', qq{ -+local all test oauth issuer="$issuer" scope="openid postgres" -+local all testalt oauth issuer="$issuer/alternate" scope="openid postgres alt" -+local all testparam oauth issuer="$issuer/param" scope="openid postgres" ++$node->append_conf( ++ 'pg_hba.conf', qq{ ++local all test oauth issuer="$issuer" scope="openid postgres" ++local all testalt oauth issuer="$issuer/.well-known/oauth-authorization-server/alternate" scope="openid postgres alt" ++local all testparam oauth issuer="$issuer/param" scope="openid postgres" +}); +$node->reload; + @@ src/test/modules/oauth_validator/t/001_server.pl (new) + +# To test against HTTP rather than HTTPS, we need to enable PGOAUTHDEBUG. But +# first, check to make sure the client refuses such connections by default. -+$node->connect_fails("user=test dbname=postgres oauth_client_id=f02c6361-0635", -+ "HTTPS is required without debug mode", -+ expected_stderr => qr/failed to fetch OpenID discovery document: Unsupported protocol/); ++$node->connect_fails( ++ "user=test dbname=postgres oauth_issuer=$issuer oauth_client_id=f02c6361-0635", ++ "HTTPS is required without debug mode", ++ expected_stderr => ++ qr@OAuth discovery URI "\Q$issuer\E/.well-known/openid-configuration" must use HTTPS@ ++); + +$ENV{PGOAUTHDEBUG} = "UNSAFE"; + +my $user = "test"; -+if ($node->connect_ok("user=$user dbname=postgres oauth_client_id=f02c6361-0635", "connect", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@)) ++if ($node->connect_ok( ++ "user=$user dbname=postgres oauth_issuer=$issuer oauth_client_id=f02c6361-0635", ++ "connect", ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@)) +{ + $log_end = $node->wait_for_log(qr/connection authorized/, $log_start); -+ $node->log_check("user $user: validator receives correct parameters", $log_start, -+ log_like => [ -+ qr/oauth_validator: token="9243959234", role="$user"/, -+ qr/oauth_validator: issuer="\Q$issuer\E", scope="openid postgres"/, -+ ]); -+ $node->log_check("user $user: validator sets authenticated identity", $log_start, -+ log_like => [ -+ qr/connection authenticated: identity="test" method=oauth/, -+ ]); ++ $node->log_check( ++ "user $user: validator receives correct parameters", ++ $log_start, ++ log_like => [ ++ qr/oauth_validator: token="9243959234", role="$user"/, ++ qr/oauth_validator: issuer="\Q$issuer\E", scope="openid postgres"/, ++ ]); ++ $node->log_check( ++ "user $user: validator sets authenticated identity", ++ $log_start, ++ log_like => ++ [ qr/connection authenticated: identity="test" method=oauth/, ]); + $log_start = $log_end; +} + -+# The /alternate issuer uses slightly different parameters. ++# The /alternate issuer uses slightly different parameters, along with an ++# OAuth-style discovery document. +$user = "testalt"; -+if ($node->connect_ok("user=$user dbname=postgres oauth_client_id=f02c6361-0636", "connect", -+ expected_stderr => qr@Visit https://example\.org/ and enter the code: postgresuser@)) ++if ($node->connect_ok( ++ "user=$user dbname=postgres oauth_issuer=$issuer/alternate oauth_client_id=f02c6361-0636", ++ "connect", ++ expected_stderr => ++ qr@Visit https://example\.org/ and enter the code: postgresuser@)) +{ + $log_end = $node->wait_for_log(qr/connection authorized/, $log_start); -+ $node->log_check("user $user: validator receives correct parameters", $log_start, -+ log_like => [ -+ qr/oauth_validator: token="9243959234-alt", role="$user"/, -+ qr|oauth_validator: issuer="\Q$issuer/alternate\E", scope="openid postgres alt"|, -+ ]); -+ $node->log_check("user $user: validator sets authenticated identity", $log_start, -+ log_like => [ -+ qr/connection authenticated: identity="testalt" method=oauth/, -+ ]); ++ $node->log_check( ++ "user $user: validator receives correct parameters", ++ $log_start, ++ log_like => [ ++ qr/oauth_validator: token="9243959234-alt", role="$user"/, ++ qr|oauth_validator: issuer="\Q$issuer/alternate\E", scope="openid postgres alt"|, ++ ]); ++ $node->log_check( ++ "user $user: validator sets authenticated identity", ++ $log_start, ++ log_like => ++ [ qr/connection authenticated: identity="testalt" method=oauth/, ]); + $log_start = $log_end; +} + ++# The issuer linked by the server must match the client's oauth_issuer setting. ++$node->connect_fails( ++ "user=$user dbname=postgres oauth_issuer=$issuer oauth_client_id=f02c6361-0636", ++ "oauth_issuer must match discovery", ++ expected_stderr => ++ qr@server's discovery document at \Q$issuer/.well-known/oauth-authorization-server/alternate\E $issuer "\Q$issuer/alternate\E"$ is incompatible with oauth_issuer $\Q$issuer\E$@ ++); ++ +# Make sure the client_id and secret are correctly encoded. $vschars contains +# every allowed character for a client_id/_secret (the "VSCHAR" class). +# $vschars_esc is additionally backslash-escaped for inclusion in a @@ src/test/modules/oauth_validator/t/001_server.pl (new) + " !\"#\$%&\\'()*+,-./0123456789:;<=>?\@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~"; + +$node->connect_ok( -+ "user=$user dbname=postgres oauth_client_id='$vschars_esc'", ++ "user=$user dbname=postgres oauth_issuer=$issuer/alternate oauth_client_id='$vschars_esc'", + "escapable characters: client_id", + expected_stderr => + qr@Visit https://example\.org/ and enter the code: postgresuser@); +$node->connect_ok( -+ "user=$user dbname=postgres oauth_client_id='$vschars_esc' oauth_client_secret='$vschars_esc'", ++ "user=$user dbname=postgres oauth_issuer=$issuer/alternate oauth_client_id='$vschars_esc' oauth_client_secret='$vschars_esc'", + "escapable characters: client_id and secret", + expected_stderr => + qr@Visit https://example\.org/ and enter the code: postgresuser@); @@ src/test/modules/oauth_validator/t/001_server.pl (new) +# oauth_client_id. +# + -+my $common_connstr = "user=testparam dbname=postgres "; ++my $common_connstr = ++ "user=testparam dbname=postgres oauth_issuer=$issuer/param "; +my $base_connstr = $common_connstr; + +sub connstr @@ src/test/modules/oauth_validator/t/001_server.pl (new) +$node->connect_ok( + connstr(), + "connect to /param", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); + +$node->connect_ok( + connstr(stage => 'token', retries => 1), + "token retry", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); +$node->connect_ok( + connstr(stage => 'token', retries => 2), + "token retry (twice)", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); +$node->connect_ok( + connstr(stage => 'all', retries => 1, interval => 2), + "token retry (two second interval)", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); +$node->connect_ok( + connstr(stage => 'all', retries => 1, interval => JSON::PP::null), + "token retry (default interval)", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); + +$node->connect_ok( + connstr(stage => 'all', content_type => 'application/json;charset=utf-8'), + "content type with charset", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); +$node->connect_ok( -+ connstr(stage => 'all', content_type => "application/json \t;\t charset=utf-8"), ++ connstr( ++ stage => 'all', ++ content_type => "application/json \t;\t charset=utf-8"), + "content type with charset (whitespace)", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); +$node->connect_ok( + connstr(stage => 'device', uri_spelling => "verification_url"), + "alternative spelling of verification_uri", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ -+); ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); + +$node->connect_fails( + connstr(stage => 'device', huge_response => JSON::PP::true), + "bad device authz response: overlarge JSON", -+ expected_stderr => qr/failed to obtain device authorization: response is too large/ -+); ++ expected_stderr => ++ qr/failed to obtain device authorization: response is too large/); +$node->connect_fails( + connstr(stage => 'token', huge_response => JSON::PP::true), + "bad token response: overlarge JSON", -+ expected_stderr => qr/failed to obtain access token: response is too large/ -+); ++ expected_stderr => ++ qr/failed to obtain access token: response is too large/); + +$node->connect_fails( + connstr(stage => 'device', content_type => 'text/plain'), + "bad device authz response: wrong content type", -+ expected_stderr => qr/failed to parse device authorization: unexpected content type/ -+); ++ expected_stderr => ++ qr/failed to parse device authorization: unexpected content type/); +$node->connect_fails( + connstr(stage => 'token', content_type => 'text/plain'), + "bad token response: wrong content type", -+ expected_stderr => qr/failed to parse access token response: unexpected content type/ -+); ++ expected_stderr => ++ qr/failed to parse access token response: unexpected content type/); +$node->connect_fails( + connstr(stage => 'token', content_type => 'application/jsonx'), + "bad token response: wrong content type (correct prefix)", -+ expected_stderr => qr/failed to parse access token response: unexpected content type/ -+); ++ expected_stderr => ++ qr/failed to parse access token response: unexpected content type/); + +$node->connect_fails( -+ connstr(stage => 'all', interval => ~0, retries => 1, retry_code => "slow_down"), ++ connstr( ++ stage => 'all', ++ interval => ~0, ++ retries => 1, ++ retry_code => "slow_down"), + "bad token response: server overflows the device authz interval", -+ expected_stderr => qr/failed to obtain access token: slow_down interval overflow/ -+); ++ expected_stderr => ++ qr/failed to obtain access token: slow_down interval overflow/); + +$node->connect_fails( + connstr(stage => 'token', error_code => "invalid_grant"), @@ src/test/modules/oauth_validator/t/001_server.pl (new) +# + +# Searching the logs is easier if OAuth parameter discovery isn't cluttering -+# things up; hardcode the issuer. (Scope is hardcoded to empty to cover that -+# case as well.) ++# things up; hardcode the discovery URI. (Scope is hardcoded to empty to cover ++# that case as well.) +$common_connstr = -+ "user=test dbname=postgres oauth_issuer=$issuer oauth_scope=''"; ++ "user=test dbname=postgres oauth_issuer=$issuer/.well-known/openid-configuration oauth_scope=''"; + +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.authn_id TO ''"); +$node->reload; @@ src/test/modules/oauth_validator/t/001_server.pl (new) +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + ++# ++# Test multiple validators. ++# ++ ++$node->append_conf('postgresql.conf', ++ "oauth_validator_libraries = 'validator, fail_validator'\n"); ++ ++# With multiple validators, every HBA line must explicitly declare one. ++my $result = $node->restart(fail_ok => 1); ++is($result, 0, ++ 'restart fails without explicit validators in oauth HBA entries'); ++ ++$log_start = $node->wait_for_log( ++ qr/authentication method "oauth" requires argument "validator" to be set/, ++ $log_start); ++ ++unlink($node->data_dir . '/pg_hba.conf'); ++$node->append_conf( ++ 'pg_hba.conf', qq{ ++local all test oauth validator=validator issuer="$issuer" scope="openid postgres" ++local all testalt oauth validator=fail_validator issuer="$issuer/alternate" scope="openid postgres alt" ++}); ++$node->restart; ++ ++$log_start = $node->wait_for_log(qr/ready to accept connections/, $log_start); ++ ++# The test user should work as before. ++$user = "test"; ++if ($node->connect_ok( ++ "user=$user dbname=postgres oauth_issuer=$issuer oauth_client_id=f02c6361-0635", ++ "validator is used for $user", ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@)) ++{ ++ $log_start = $node->wait_for_log(qr/connection authorized/, $log_start); ++} ++ ++# testalt should be routed through the fail_validator. ++$user = "testalt"; ++$node->connect_fails( ++ "user=$user dbname=postgres oauth_issuer=$issuer/.well-known/oauth-authorization-server/alternate oauth_client_id=f02c6361-0636", ++ "fail_validator is used for $user", ++ expected_stderr => qr/FATAL:\s+fail_validator: sentinel error/); ++ +$node->stop; + +done_testing(); @@ src/test/modules/oauth_validator/t/002_client.pl (new) +use MIME::Base64 qw(encode_base64); +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; -+use PostgreSQL::Test::OAuthServer; +use Test::More; + +if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\boauth\b/) @@ src/test/modules/oauth_validator/t/002_client.pl (new) +$node->init; +$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', -+ "oauth_validator_library = 'validator'\n"); ++ "oauth_validator_libraries = 'validator'\n"); +$node->start; + +$node->safe_psql('postgres', 'CREATE USER test;'); + -+my $issuer = "https://127.0.0.1:54321"; ++# These tests don't use the builtin flow, and we don't have an authorization ++# server running, so the address used here shouldn't matter. Use an invalid IP ++# address, so if there's some cascade of errors that causes the client to ++# attempt a connection, we'll fail noisily. ++my $issuer = "https://256.256.256.256"; +my $scope = "openid postgres"; + +unlink($node->data_dir . '/pg_hba.conf'); @@ src/test/modules/oauth_validator/t/002_client.pl (new) + +my $user = "test"; +my $base_connstr = $node->connstr() . " user=$user"; -+my $common_connstr = "$base_connstr oauth_client_id=myID"; ++my $common_connstr = ++ "$base_connstr oauth_issuer=$issuer oauth_client_id=myID"; + +sub test +{ @@ src/test/modules/oauth_validator/t/oauth_server.py (new) + """ + Switches the behavior of the provider depending on the issuer URI. + """ -+ self._alt_issuer = self.path.startswith("/alternate/") ++ self._alt_issuer = ( ++ self.path.startswith("/alternate/") ++ or self.path == "/.well-known/oauth-authorization-server/alternate" ++ ) + self._parameterized = self.path.startswith("/param/") + + if self._alt_issuer: -+ self.path = self.path.removeprefix("/alternate") ++ # The /alternate issuer uses IETF-style .well-known URIs. ++ if self.path.startswith("/.well-known/"): ++ self.path = self.path.removesuffix("/alternate") ++ else: ++ self.path = self.path.removeprefix("/alternate") + elif self._parameterized: + self.path = self.path.removeprefix("/param") + @@ src/test/modules/oauth_validator/t/oauth_server.py (new) + self._response_code = 200 + self._check_issuer() + -+ if self.path == "/.well-known/openid-configuration": ++ config_path = "/.well-known/openid-configuration" ++ if self._alt_issuer: ++ config_path = "/.well-known/oauth-authorization-server" ++ ++ if self.path == config_path: + resp = self.config() + else: + self.send_error(404, "Not Found") 2: 01df79980b ! 2: 3d169848db DO NOT MERGE: Add pytest suite for OAuth @@ .cirrus.tasks.yml: task: matrix: - name: Linux - Debian Bookworm - Autoconf @@ .cirrus.tasks.yml: task: - - # Also build & test in a 32bit build - it's gotten rare to test that - # locally. + # can easily provide some here by running one of the sets of tests that + # way. Newer versions of python insist on changing the LC_CTYPE away + # from C, prevent that with PYTHONCOERCECLOCALE. + # XXX 32-bit Python tests are currently disabled, as the system's 64-bit + # Python modules can't link against libpq. - configure_32_script: | + test_world_32_script: | su postgres <<-EOF - export CC='ccache gcc -m32' + export PG_TEST_EXTRA="${PG_TEST_EXTRA//python}" - meson setup \ - --buildtype=debug \ - -Dcassert=true -Dinjection_points=true \ + ulimit -c unlimited + PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS} + EOF ## meson.build ## @@ meson.build: else @@ meson.build: foreach test_dir : tests + env.set(name, value) + endforeach + -+ if get_option('PG_TEST_EXTRA').contains('python') -+ reqs = files(t['requirements']) -+ test('install_' + venv_name, -+ python, -+ args: [ make_venv, '--requirements', reqs, venv_path ], -+ env: env, -+ priority: setup_tests_priority - 1, # must run after tmp_install -+ is_parallel: false, -+ suite: ['setup'], -+ timeout: 60, # 30s is too short for the cryptography package compile -+ ) -+ endif ++ reqs = files(t['requirements']) ++ test('install_' + venv_name, ++ python, ++ args: [ make_venv, '--requirements', reqs, venv_path ], ++ env: env, ++ priority: setup_tests_priority - 1, # must run after tmp_install ++ is_parallel: false, ++ suite: ['setup'], ++ timeout: 60, # 30s is too short for the cryptography package compile ++ ) + + test_group = test_dir['name'] + test_output = test_result_dir / test_group / kind @@ meson.build: foreach test_dir : tests + 'timeout': 1000, + 'depends': test_deps, + 'env': env, -+ } ++ } + t.get('test_kwargs', {}) + + if fs.is_dir(venv_path / 'Scripts') + # Windows virtualenv layout @@ meson.build: foreach test_dir : tests + testwrap_pytest = testwrap_base + [ + '--testgroup', test_group, + '--testname', pyt_p, ++ '--skip-without-extra', 'python', + ] -+ if not get_option('PG_TEST_EXTRA').contains('python') -+ testwrap_pytest += ['--skip', '"python" tests not enabled in PG_TEST_EXTRA'] -+ endif + + test(test_group / pyt_p, + python, @@ src/test/python/client/conftest.py (new) +# + +import contextlib ++import datetime ++import functools ++import ipaddress ++import os +import socket +import sys +import threading @@ src/test/python/client/conftest.py (new) +import psycopg2 +import psycopg2.extras +import pytest ++from cryptography import x509 ++from cryptography.hazmat.primitives import hashes, serialization ++from cryptography.hazmat.primitives.asymmetric import rsa ++from cryptography.x509.oid import NameOID + +import pq3 + @@ src/test/python/client/conftest.py (new) + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: + yield conn ++ ++ ++@pytest.fixture(scope="session") ++def certpair(tmp_path_factory): ++ """ ++ Yields a (cert, key) pair of file paths that can be used by a TLS server. ++ The certificate is issued for "localhost" and its standard IPv4/6 addresses. ++ """ ++ ++ tmpdir = tmp_path_factory.mktemp("certs") ++ now = datetime.datetime.now(datetime.timezone.utc) ++ ++ # https://cryptography.io/en/latest/x509/tutorial/#creating-a-self-signed-certificate ++ key = rsa.generate_private_key(public_exponent=65537, key_size=2048) ++ ++ subject = issuer = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "localhost")]) ++ altNames = [ ++ x509.DNSName("localhost"), ++ x509.IPAddress(ipaddress.IPv4Address("127.0.0.1")), ++ x509.IPAddress(ipaddress.IPv6Address("::1")), ++ ] ++ cert = ( ++ x509.CertificateBuilder() ++ .subject_name(subject) ++ .issuer_name(issuer) ++ .public_key(key.public_key()) ++ .serial_number(x509.random_serial_number()) ++ .not_valid_before(now) ++ .not_valid_after(now + datetime.timedelta(minutes=10)) ++ .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) ++ .add_extension(x509.SubjectAlternativeName(altNames), critical=False) ++ ).sign(key, hashes.SHA256()) ++ ++ # Writing the key with mode 0600 lets us use this from the server side, too. ++ keypath = str(tmpdir / "key.pem") ++ with open(keypath, "wb", opener=functools.partial(os.open, mode=0o600)) as f: ++ f.write( ++ key.private_bytes( ++ encoding=serialization.Encoding.PEM, ++ format=serialization.PrivateFormat.PKCS8, ++ encryption_algorithm=serialization.NoEncryption(), ++ ) ++ ) ++ ++ certpath = str(tmpdir / "cert.pem") ++ with open(certpath, "wb") as f: ++ f.write(cert.public_bytes(serialization.Encoding.PEM)) ++ ++ return certpath, keypath ## src/test/python/client/test_client.py (new) ## @@ @@ src/test/python/client/test_oauth.py (new) + +import base64 +import collections ++import contextlib +import ctypes +import http.server +import json @@ src/test/python/client/test_oauth.py (new) +import os +import platform +import secrets ++import socket ++import ssl +import sys +import threading +import time @@ src/test/python/client/test_oauth.py (new) + ) + + -+def xtest_oauth_success(conn): # TODO -+ initial = start_oauth_handshake(conn) -+ -+ auth = get_auth_value(initial) -+ assert auth.startswith(b"Bearer ") -+ -+ # Accept the token. TODO actually validate -+ pq3.send(conn, pq3.types.AuthnRequest, type=pq3.authn.SASLFinal) -+ finish_handshake(conn) -+ -+ +class RawResponse(str): + """ + Returned by registered endpoint callbacks to take full control of the @@ src/test/python/client/test_oauth.py (new) + +class OpenIDProvider(threading.Thread): + """ -+ A thread that runs a mock OpenID provider server. ++ A thread that runs a mock OpenID provider server on an SSL-enabled socket. + """ + -+ def __init__(self, *, port): ++ def __init__(self, ssl_socket): + super().__init__() + + self.exception = None + -+ addr = ("", port) -+ self.server = self._Server(addr, self._Handler) ++ _, port = ssl_socket.getsockname() + -+ # TODO: allow HTTPS only, somehow + oauth = self._OAuthState() + oauth.host = f"localhost:{port}" -+ oauth.issuer = f"http://localhost:{port}" ++ oauth.issuer = f"https://localhost:{port}" + + # The following endpoints are required to be advertised by providers, + # even though our chosen client implementation does not actually make @@ src/test/python/client/test_oauth.py (new) + ) + oauth.register_endpoint("jwks_uri", "GET", "/keys", self._jwks_handler) + ++ self.server = self._HTTPSServer(ssl_socket, self._Handler) + self.server.oauth = oauth + + def run(self): @@ src/test/python/client/test_oauth.py (new) + + return 200, doc + -+ class _Server(http.server.HTTPServer): ++ class _HTTPSServer(http.server.HTTPServer): ++ def __init__(self, ssl_socket, handler_cls): ++ # Attach the SSL socket to the server. We don't bind/activate since ++ # the socket is already listening. ++ super().__init__(None, handler_cls, bind_and_activate=False) ++ self.socket = ssl_socket ++ self.server_address = self.socket.getsockname() ++ ++ def shutdown_request(self, request): ++ # Cleanly unwrap the SSL socket before shutting down the connection; ++ # otherwise careful clients will complain about truncation. ++ try: ++ request = request.unwrap() ++ except (ssl.SSLEOFError, ConnectionResetError, BrokenPipeError): ++ # The client already closed (or aborted) the connection without ++ # a clean shutdown. This is seen on some platforms during tests ++ # that break the HTTP protocol. Just return and have the server ++ # close the socket. ++ return ++ ++ super().shutdown_request(request) ++ + def handle_error(self, request, addr): + self.shutdown_request(request) + raise @@ src/test/python/client/test_oauth.py (new) + oauth = self.server.oauth + assert self.headers["Host"] == oauth.host + ++ # XXX: BaseHTTPRequestHandler collapses leading slashes in the path ++ # to work around an open redirection vuln (gh-87389) in ++ # SimpleHTTPServer. But we're not using SimpleHTTPServer, and we ++ # want to test repeating leading slashes, so that's not very ++ # helpful. Put them back. ++ orig_path = self.raw_requestline.split()[1] ++ orig_path = str(orig_path, "iso-8859-1") ++ assert orig_path.endswith(self.path) # sanity check ++ self.path = orig_path ++ + if handler is None: + handler = oauth.endpoint(self.command, self.path) + assert ( @@ src/test/python/client/test_oauth.py (new) + monkeypatch.setenv("PGOAUTHDEBUG", "UNSAFE") + + ++@pytest.fixture(autouse=True) ++def trust_certpair_in_client(monkeypatch, certpair): ++ """ ++ Set a trusted CA file for OAuth client connections. ++ """ ++ monkeypatch.setenv("PGOAUTHCAFILE", certpair[0]) ++ ++ ++@pytest.fixture(scope="session") ++def ssl_socket(certpair): ++ """ ++ A listening server-side socket for SSL connections, using the certpair ++ fixture. ++ """ ++ sock = socket.create_server(("", 0)) ++ ++ # The TLS connections we're making are incredibly sensitive to delayed ACKs ++ # from the client. (Without TCP_NODELAY, test performance degrades 4-5x.) ++ sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) ++ ++ with contextlib.closing(sock): ++ # Wrap the server socket for TLS. ++ ctx = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) ++ ctx.load_cert_chain(*certpair) ++ ++ yield ctx.wrap_socket(sock, server_side=True) ++ ++ +@pytest.fixture -+def openid_provider(unused_tcp_port_factory): ++def openid_provider(ssl_socket): + """ + A fixture that returns the OAuth state of a running OpenID provider server. The + server will be stopped when the fixture is torn down. + """ -+ thread = OpenIDProvider(port=unused_tcp_port_factory()) ++ thread = OpenIDProvider(ssl_socket) + thread.start() + + try: @@ src/test/python/client/test_oauth.py (new) + pytest.param(True, id="asynchronous"), + ], +) -+def test_oauth_with_explicit_issuer( ++def test_oauth_with_explicit_discovery_uri( + accept, + openid_provider, + asynchronous, @@ src/test/python/client/test_oauth.py (new) + openid_provider.content_type = content_type + + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=client_id, + oauth_client_secret=secret, + oauth_scope=scope, @@ src/test/python/client/test_oauth.py (new) + client.check_completed() + + ++@pytest.mark.parametrize( ++ "server_discovery", ++ [ ++ pytest.param(True, id="server discovery"), ++ pytest.param(False, id="direct discovery"), ++ ], ++) ++@pytest.mark.parametrize( ++ "issuer, path", ++ [ ++ pytest.param( ++ "{issuer}", ++ "/.well-known/oauth-authorization-server", ++ id="oauth", ++ ), ++ pytest.param( ++ "{issuer}/alt", ++ "/.well-known/oauth-authorization-server/alt", ++ id="oauth with path, IETF style", ++ ), ++ pytest.param( ++ "{issuer}/alt", ++ "/alt/.well-known/oauth-authorization-server", ++ id="oauth with path, broken OIDC style", ++ ), ++ pytest.param( ++ "{issuer}/alt", ++ "/alt/.well-known/openid-configuration", ++ id="openid with path, OIDC style", ++ ), ++ pytest.param( ++ "{issuer}/alt", ++ "/.well-known/openid-configuration/alt", ++ id="openid with path, IETF style", ++ ), ++ pytest.param( ++ "{issuer}/", ++ "//.well-known/openid-configuration", ++ id="empty path segment, OIDC style", ++ ), ++ pytest.param( ++ "{issuer}/", ++ "/.well-known/openid-configuration/", ++ id="empty path segment, IETF style", ++ ), ++ ], ++) ++def test_alternate_well_known_paths( ++ accept, openid_provider, issuer, path, server_discovery ++): ++ issuer = issuer.format(issuer=openid_provider.issuer) ++ discovery_uri = openid_provider.issuer + path ++ ++ client_id = secrets.token_hex() ++ access_token = secrets.token_urlsafe() ++ ++ def discovery_handler(*args): ++ """ ++ Pass-through implementation of the discovery handler. Modifies the ++ default document to contain this test's issuer identifier. ++ """ ++ code, doc = openid_provider._default_discovery_handler(*args) ++ doc["issuer"] = issuer ++ return code, doc ++ ++ openid_provider.register_endpoint(None, "GET", path, discovery_handler) ++ ++ def authorization_endpoint(headers, params): ++ resp = { ++ "device_code": "12345", ++ "user_code": "ABCDE", ++ "interval": 0, ++ "verification_url": "https://example.com/device", ++ "expires_in": 5, ++ } ++ ++ return 200, resp ++ ++ openid_provider.register_endpoint( ++ "device_authorization_endpoint", "POST", "/device", authorization_endpoint ++ ) ++ ++ def token_endpoint(headers, params): ++ # Successfully finish the request by sending the access bearer token. ++ resp = { ++ "access_token": access_token, ++ "token_type": "bearer", ++ } ++ ++ return 200, resp ++ ++ openid_provider.register_endpoint( ++ "token_endpoint", "POST", "/token", token_endpoint ++ ) ++ ++ kwargs = dict(oauth_client_id=client_id) ++ if server_discovery: ++ kwargs.update(oauth_issuer=issuer) ++ else: ++ kwargs.update(oauth_issuer=discovery_uri) ++ ++ sock, client = accept(**kwargs) ++ ++ if server_discovery: ++ with sock: ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ initial = start_oauth_handshake(conn) ++ ++ # For discovery, the client should send an empty auth header. ++ # See RFC 7628, Sec. 4.3. ++ auth = get_auth_value(initial) ++ assert auth == b"" ++ ++ # Always fail the discovery exchange. ++ fail_oauth_handshake( ++ conn, ++ { ++ "status": "invalid_token", ++ "openid-configuration": discovery_uri, ++ }, ++ ) ++ ++ # Expect the client to connect again. ++ sock, client = accept() ++ ++ with sock: ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ initial = start_oauth_handshake(conn) ++ ++ # Validate the token. ++ auth = get_auth_value(initial) ++ assert auth == f"Bearer {access_token}".encode("ascii") ++ ++ pq3.send(conn, pq3.types.AuthnRequest, type=pq3.authn.SASLFinal) ++ finish_handshake(conn) ++ ++ ++@pytest.mark.parametrize( ++ "server_discovery", ++ [ ++ pytest.param(True, id="server discovery"), ++ pytest.param(False, id="direct discovery"), ++ ], ++) ++@pytest.mark.parametrize( ++ "issuer, path, expected_error", ++ [ ++ pytest.param( ++ "{issuer}", ++ "/.well-known/oauth-authorization-server/", ++ None, ++ id="extra empty segment", ++ ), ++ pytest.param( ++ "{issuer}", ++ "?/.well-known/oauth-authorization-server", ++ r'OAuth discovery URI ".*" must not contain query or fragment components', ++ id="query", ++ ), ++ pytest.param( ++ "{issuer}", ++ "#/.well-known/oauth-authorization-server", ++ r'OAuth discovery URI ".*" must not contain query or fragment components', ++ id="fragment", ++ ), ++ pytest.param( ++ "{issuer}/sub/path", ++ "/sub/.well-known/oauth-authorization-server/path", ++ r'OAuth discovery URI ".*" uses an invalid format', ++ id="sandwiched prefix", ++ ), ++ pytest.param( ++ "{issuer}/path", ++ "/path/openid-configuration", ++ r'OAuth discovery URI ".*" is not a .well-known URI', ++ id="not .well-known", ++ ), ++ pytest.param( ++ "{issuer}", ++ "https://.well-known/oauth-authorization-server", ++ r'OAuth discovery URI ".*" is not a .well-known URI', ++ id=".well-known prefix buried in the authority", ++ ), ++ pytest.param( ++ "{issuer}", ++ "/.well-known/oauth-protected-resource", ++ r'OAuth discovery URI ".*" uses an unsupported .well-known suffix', ++ id="unknown well-known suffix", ++ ), ++ pytest.param( ++ "{issuer}/path", ++ "/path/.well-known/openid-configuration-2", ++ r'OAuth discovery URI ".*" uses an unsupported .well-known suffix', ++ id="unknown well-known suffix, OIDC style", ++ ), ++ pytest.param( ++ "{issuer}/path", ++ "/.well-known/oauth-authorization-server-2/path", ++ r'OAuth discovery URI ".*" uses an unsupported .well-known suffix', ++ id="unknown well-known suffix, IETF style", ++ ), ++ pytest.param( ++ "{issuer}", ++ "file:///.well-known/oauth-authorization-server", ++ r'OAuth discovery URI ".*" must use HTTPS', ++ id="unsupported scheme", ++ ), ++ ], ++) ++def test_bad_well_known_paths( ++ accept, openid_provider, issuer, path, expected_error, server_discovery ++): ++ if not server_discovery and "/.well-known/" not in path: ++ # An oauth_issuer without a /.well-known/ path segment is just a normal ++ # issuer identifier, so this isn't an interesting test. ++ pytest.skip("not interesting: direct discovery requires .well-known") ++ ++ issuer = issuer.format(issuer=openid_provider.issuer) ++ discovery_uri = urllib.parse.urljoin(openid_provider.issuer, path) ++ ++ client_id = secrets.token_hex() ++ ++ def discovery_handler(*args): ++ """ ++ Pass-through implementation of the discovery handler. Modifies the ++ default document to contain this test's issuer identifier. ++ """ ++ code, doc = openid_provider._default_discovery_handler(*args) ++ doc["issuer"] = issuer ++ return code, doc ++ ++ openid_provider.register_endpoint(None, "GET", path, discovery_handler) ++ ++ def fail(*args): ++ """ ++ No other endpoints should be contacted; fail if the client tries. ++ """ ++ assert False, "endpoint unexpectedly called" ++ ++ openid_provider.register_endpoint( ++ "device_authorization_endpoint", "POST", "/device", fail ++ ) ++ openid_provider.register_endpoint("token_endpoint", "POST", "/token", fail) ++ ++ kwargs = dict(oauth_client_id=client_id) ++ if server_discovery: ++ kwargs.update(oauth_issuer=issuer) ++ else: ++ kwargs.update(oauth_issuer=discovery_uri) ++ ++ sock, client = accept(**kwargs) ++ ++ if server_discovery: ++ with sock: ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ initial = start_oauth_handshake(conn) ++ ++ # For discovery, the client should send an empty auth header. ++ # See RFC 7628, Sec. 4.3. ++ auth = get_auth_value(initial) ++ assert auth == b"" ++ ++ # Always fail the discovery exchange. ++ resp = { ++ "status": "invalid_token", ++ "openid-configuration": discovery_uri, ++ } ++ pq3.send( ++ conn, ++ pq3.types.AuthnRequest, ++ type=pq3.authn.SASLContinue, ++ body=json.dumps(resp).encode("utf-8"), ++ ) ++ ++ # FIXME: the client disconnects at this point; it'd be nicer if ++ # it completed the exchange. ++ ++ # The client should not reconnect. ++ ++ else: ++ expect_disconnected_handshake(sock) ++ ++ if expected_error is None: ++ if server_discovery: ++ expected_error = rf"server's discovery document at {discovery_uri} $issuer \".*\"$ is incompatible with oauth_issuer ${issuer}$" ++ else: ++ expected_error = rf"the issuer identifier ${issuer}$ does not match oauth_issuer $.*$" ++ ++ with pytest.raises(psycopg2.OperationalError, match=expected_error): ++ client.check_completed() ++ ++ +def expect_disconnected_handshake(sock): + """ + Helper for any tests that expect the client to disconnect immediately after @@ src/test/python/client/test_oauth.py (new) + ) + + # The client should disconnect at this point. -+ assert not conn.read() ++ assert not conn.read(1), "client sent unexpected data" + + -+def test_oauth_requires_client_id(accept, openid_provider): -+ sock, client = accept( ++@pytest.mark.parametrize( ++ "missing", ++ [ ++ pytest.param(["oauth_issuer"], id="missing oauth_issuer"), ++ pytest.param(["oauth_client_id"], id="missing oauth_client_id"), ++ pytest.param(["oauth_client_id", "oauth_issuer"], id="missing both"), ++ ], ++) ++def test_oauth_requires_issuer_and_client_id(accept, openid_provider, missing): ++ params = dict( + oauth_issuer=openid_provider.issuer, -+ # Do not set a client ID; this should cause a client error after the -+ # server asks for OAUTHBEARER and the client tries to contact the -+ # issuer. ++ oauth_client_id="some-id", + ) + ++ # Remove required parameters. This should cause a client error after the ++ # server asks for OAUTHBEARER and the client tries to contact the issuer. ++ for k in missing: ++ del params[k] ++ ++ sock, client = accept(**params) + expect_disconnected_handshake(sock) + -+ expected_error = "no oauth_client_id is set" ++ expected_error = "oauth_issuer and oauth_client_id are not both set" + with pytest.raises(psycopg2.OperationalError, match=expected_error): + client.check_completed() + @@ src/test/python/client/test_oauth.py (new) +@pytest.mark.parametrize("scope", ["&", r"+=&/", all_nqchars]) +def test_url_encoding(accept, openid_provider, client_id, secret, device_code, scope): + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=client_id, + oauth_client_secret=secret, + oauth_scope=scope, @@ src/test/python/client/test_oauth.py (new) + accept, openid_provider, omit_interval, retries, error_code +): + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id="some-id", + ) + @@ src/test/python/client/test_oauth.py (new) + access_token = secrets.token_urlsafe() + + sock, client = accept( -+ oauth_issuer=issuer, ++ oauth_issuer=discovery_uri, + oauth_client_id="some-id", + oauth_scope=scope, + async_=asynchronous, @@ src/test/python/client/test_oauth.py (new) + client_id = secrets.token_hex() + + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=client_id, + ) + @@ src/test/python/client/test_oauth.py (new) + pytest.skip("not interesting: correct type") + + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=secrets.token_hex(), + ) + @@ src/test/python/client/test_oauth.py (new) + client_id = secrets.token_hex() + + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=client_id, + ) + @@ src/test/python/client/test_oauth.py (new) + pytest.skip("not interesting: correct type") + + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=secrets.token_hex(), + ) + @@ src/test/python/client/test_oauth.py (new) + ], +) +def test_oauth_discovery(accept, openid_provider, base_response, scope, success): -+ sock, client = accept(oauth_client_id=secrets.token_hex()) ++ sock, client = accept( ++ oauth_issuer=openid_provider.issuer, ++ oauth_client_id=secrets.token_hex(), ++ ) + + device_code = secrets.token_hex() + user_code = f"{secrets.token_hex(2)}-{secrets.token_hex(2)}" @@ src/test/python/client/test_oauth.py (new) + ], +) +def test_oauth_discovery_server_error(accept, response, expected_error): -+ sock, client = accept(oauth_client_id=secrets.token_hex()) ++ sock, client = accept( ++ oauth_issuer="https://example.com", ++ oauth_client_id=secrets.token_hex(), ++ ) + + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: @@ src/test/python/client/test_oauth.py (new) + client.check_completed() + + ++# All of these tests are expected to fail before libpq tries to actually attempt ++# a connection to any endpoint. To avoid hitting the network in the event that a ++# test fails, an invalid IPv4 address (256.256.256.256) is used as a hostname. +@pytest.mark.parametrize( + "bad_response,expected_error", + [ @@ src/test/python/client/test_oauth.py (new) + 200, + { + "grant_types_supported": ["something"], -+ "token_endpoint": "https://example.com/", ++ "token_endpoint": "https://256.256.256.256/", + "issuer": 123, + }, + ), @@ src/test/python/client/test_oauth.py (new) + id="non-string issuer after other ignored fields", + ), + pytest.param( -+ (200, {"token_endpoint": "https://example.com/"}), ++ (200, {"token_endpoint": "https://256.256.256.256/"}), + r'failed to parse OpenID discovery document: field "issuer" is missing', + id="missing issuer", + ), + pytest.param( -+ (200, {"issuer": "https://example.com/"}), ++ (200, {"issuer": "{issuer}"}), + r'failed to parse OpenID discovery document: field "token_endpoint" is missing', + id="missing token endpoint", + ), @@ src/test/python/client/test_oauth.py (new) + ( + 200, + { -+ "issuer": "https://example.com", -+ "token_endpoint": "https://example.com/token", -+ "device_authorization_endpoint": "https://example.com/dev", ++ "issuer": "{issuer}", ++ "token_endpoint": "https://256.256.256.256/token", ++ "device_authorization_endpoint": "https://256.256.256.256/dev", + }, + ), -+ r'cannot run OAuth device authorization: issuer "https://example.com" does not support device code grants', ++ r'cannot run OAuth device authorization: issuer "https://.*" does not support device code grants', + id="missing device code grants", + ), + pytest.param( + ( + 200, + { -+ "issuer": "https://example.com", -+ "token_endpoint": "https://example.com/token", ++ "issuer": "{issuer}", ++ "token_endpoint": "https://256.256.256.256/token", + "grant_types_supported": [ + "urn:ietf:params:oauth:grant-type:device_code" + ], + }, + ), -+ r'cannot run OAuth device authorization: issuer "https://example.com" does not provide a device authorization endpoint', ++ r'cannot run OAuth device authorization: issuer "https://.*" does not provide a device authorization endpoint', + id="missing device_authorization_endpoint", + ), + pytest.param( + ( + 200, + { -+ "issuer": "https://example.com", -+ "token_endpoint": "https://example.com/token", ++ "issuer": "{issuer}", ++ "token_endpoint": "https://256.256.256.256/token", + "grant_types_supported": [ + "urn:ietf:params:oauth:grant-type:device_code" + ], -+ "device_authorization_endpoint": "https://example.com/dev", ++ "device_authorization_endpoint": "https://256.256.256.256/dev", + "filler": "x" * 1024 * 1024, + }, + ), + r"failed to fetch OpenID discovery document: response is too large", + id="gigantic discovery response", + ), ++ pytest.param( ++ ( ++ 200, ++ { ++ "issuer": "{issuer}/path", ++ "token_endpoint": "https://256.256.256.256/token", ++ "grant_types_supported": [ ++ "urn:ietf:params:oauth:grant-type:device_code" ++ ], ++ "device_authorization_endpoint": "https://256.256.256.256/dev", ++ }, ++ ), ++ r"failed to parse OpenID discovery document: the issuer identifier $https://.*/path$ does not match oauth_issuer $https://.*$", ++ id="mismatched issuer identifier", ++ ), + # + # Exercise HTTP-level failures by breaking the protocol. Note that the + # error messages here are implementation-dependent. @@ src/test/python/client/test_oauth.py (new) + accept, openid_provider, bad_response, expected_error +): + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=secrets.token_hex(), + ) + + def failing_discovery_handler(headers, params): ++ try: ++ # Insert the correct issuer value if the test wants to. ++ resp = bad_response[1] ++ iss = resp["issuer"] ++ resp["issuer"] = iss.format(issuer=openid_provider.issuer) ++ except (AttributeError, KeyError, TypeError): ++ pass ++ + return bad_response + + openid_provider.register_endpoint( @@ src/test/python/client/test_oauth.py (new) + ], +) +def test_oauth_server_error(accept, sasl_err, resp_type, resp_payload, expected_error): -+ sock, client = accept() ++ sock, client = accept( ++ oauth_issuer="https://example.com", ++ oauth_client_id="some-id", ++ ) + + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: @@ src/test/python/client/test_oauth.py (new) + int_max = ctypes.c_uint(-1).value // 2 + + sock, client = accept( -+ oauth_issuer=openid_provider.issuer, ++ oauth_issuer=openid_provider.discovery_uri, + oauth_client_id=secrets.token_hex(), + ) + @@ src/test/python/client/test_oauth.py (new) + HTTP must be refused without PGOAUTHDEBUG. + """ + monkeypatch.delenv("PGOAUTHDEBUG") ++ ++ def to_http(uri): ++ """Swaps out a URI's scheme for http.""" ++ parts = urllib.parse.urlparse(uri) ++ parts = parts._replace(scheme="http") ++ return urllib.parse.urlunparse(parts) ++ + sock, client = accept( ++ oauth_issuer=to_http(openid_provider.issuer), + oauth_client_id=secrets.token_hex(), + ) + @@ src/test/python/client/test_oauth.py (new) + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: + initial = start_oauth_handshake(conn) + -+ # Fail the SASL exchange and link to the HTTP provider. + resp = { + "status": "invalid_token", -+ "openid-configuration": openid_provider.discovery_uri, ++ "openid-configuration": to_http(openid_provider.discovery_uri), + } ++ pq3.send( ++ conn, ++ pq3.types.AuthnRequest, ++ type=pq3.authn.SASLContinue, ++ body=json.dumps(resp).encode("utf-8"), ++ ) + -+ fail_oauth_handshake(conn, resp) -+ -+ # FIXME: We'll get a second connection, but it won't do anything. -+ sock, _ = accept() -+ expect_disconnected_handshake(sock) ++ # FIXME: the client disconnects at this point; it'd be nicer if ++ # it completed the exchange. + -+ expected_error = "failed to fetch OpenID discovery document: Unsupported protocol" ++ expected_error = r'OAuth discovery URI ".*" must use HTTPS' + with pytest.raises(psycopg2.OperationalError, match=expected_error): + client.check_completed() @@ src/test/python/meson.build (new) + './test_pq3.py', + ], + 'env': pytest_env, ++ 'test_kwargs': {'priority': 50}, # python tests are slow, start early + }, +} @@ src/test/python/server/conftest.py (new) + f"-c port={port}", + "-c listen_addresses=localhost", + "-c log_connections=on", -+ "-c shared_preload_libraries=oauthtest", -+ "-c oauth_validator_library=oauthtest", ++ "-c session_preload_libraries=oauthtest", ++ "-c oauth_validator_libraries=oauthtest", + ] + ), + "start", @@ src/tools/make_venv (new) +run(pip, 'install', 'pytest', 'pytest-tap') +if args.requirements: + run(pip, 'install', '-r', args.requirements) + + ## src/tools/testwrap ## +@@ src/tools/testwrap: parser.add_argument('--testgroup', help='test group', type=str) + parser.add_argument('--testname', help='test name', type=str) + parser.add_argument('--skip', help='skip test (with reason)', type=str) + parser.add_argument('--pg-test-extra', help='extra tests', type=str) ++parser.add_argument('--skip-without-extra', help='skip if PG_TEST_EXTRA is missing this arg', type=str) + parser.add_argument('test_command', nargs='*') + + args = parser.parse_args() +@@ src/tools/testwrap: if args.skip is not None: + print('1..0 # Skipped: ' + args.skip) + sys.exit(0) + ++if args.skip_without_extra is not None: ++ extras = os.environ.get("PG_TEST_EXTRA", args.pg_test_extra) ++ if extras is None or args.skip_without_extra not in extras.split(): ++ print(f'1..0 # Skipped: PG_TEST_EXTRA does not contain "{args.skip_without_extra}"') ++ sys.exit(0) ++ + if os.path.exists(testdir) and os.path.isdir(testdir): + shutil.rmtree(testdir) + os.makedirs(testdir)