1: 386e7c4df31 = 1: 1f38ec8039b Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h 2: b829f7a8ac7 = 2: 9f87ffea1c7 require_auth: prepare for multiple SASL mechanisms 3: f88f98df97d ! 3: bda684d19cc libpq: handle asynchronous actions during SASL @@ src/interfaces/libpq/fe-auth.c: pg_SASL_continue(PGconn *conn, int payloadlen, b + * need to do is signal the caller. + */ + *async = true; -+ return STATUS_OK; ++ ++ /* ++ * The mechanism may optionally generate some output to send before ++ * switching over to async auth, so continue onwards. ++ */ + } + if (final && status == SASL_CONTINUE) @@ src/interfaces/libpq/fe-auth.c: pg_fe_sendauth(AuthRequest areq, int payloadlen, case AUTH_REQ_SASL_CONT: case AUTH_REQ_SASL_FIN: - if (conn->sasl_state == NULL) -- { + { - appendPQExpBufferStr(&conn->errorMessage, - "fe_sendauth: invalid authentication request from server: AUTH_REQ_SASL_CONT without AUTH_REQ_SASL\n"); - return STATUS_ERROR; @@ src/interfaces/libpq/fe-auth.c: pg_fe_sendauth(AuthRequest areq, int payloadlen, - oldmsglen = conn->errorMessage.len; - if (pg_SASL_continue(conn, payloadlen, - (areq == AUTH_REQ_SASL_FIN)) != STATUS_OK) - { +- { - /* Use this message if pg_SASL_continue didn't supply one */ - if (conn->errorMessage.len == oldmsglen) + bool final = false; 4: d96712cda1d ! 4: 3dc6dd3433c Add OAUTHBEARER SASL mechanism @@ Commit message Several TODOs: - perform several sanity checks on the OAuth issuer's responses - - handle cases where the client has been set up with an issuer and - scope, but the Postgres server wants to use something different - improve error debuggability during the OAuth handshake - fix libcurl initialization thread-safety - harden the libcurl flow implementation - - figure out pgsocket/int difference on Windows - fill in documentation stubs - support protocol "variants" implemented by major providers - implement more helpful handling of HBA misconfigurations @@ doc/src/sgml/installation.sgml: ninja install ## doc/src/sgml/libpq.sgml ## +@@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + + ++ ++ oauth ++ ++ ++ The server must request an OAuth bearer token from the client. ++ ++ ++ ++ + + none + @@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname @@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + 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 URL, you may set oauth_issuer to a -+ /.well-known/ URI used for OAuth discovery. (In this -+ case, it is recommended that ++ You may also explicitly set oauth_issuer to the ++ /.well-known/ URI used for OAuth discovery. In this ++ case, if the server asks for a different URL, the connection will fail, ++ but a custom OAuth flow ++ may be able to speed up the standard handshake by using previously ++ cached tokens. (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 @@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + An OAuth 2.0 client identifier, as issued by the authorization server. + If the PostgreSQL server + requests an OAuth token for the -+ connection (and if no custom OAuth -+ hook is installed to provide one), then this parameter must be -+ set; otherwise, the connection will fail. ++ connection (and if no custom ++ OAuth hook is installed to provide one), then this parameter must ++ be set; otherwise, the connection will fail. + + + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + break; + + case OAUTH_STEP_TOKEN_REQUEST: -+ if (!handle_token_response(actx, &state->token)) ++ if (!handle_token_response(actx, &conn->oauth_token)) + goto error_return; + + if (!actx->user_prompted) @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + actx->user_prompted = true; + } + -+ if (state->token) ++ if (conn->oauth_token) + break; /* done! */ + + /* @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + * point, actx->running will be set. But there are some corner cases + * where we can immediately loop back around; see start_request(). + */ -+ } while (!state->token && !actx->running); ++ } while (!conn->oauth_token && !actx->running); + + /* If we've stored a token, we're done. Otherwise come back later. */ -+ return state->token ? PGRES_POLLING_OK : PGRES_POLLING_READING; ++ return conn->oauth_token ? PGRES_POLLING_OK : PGRES_POLLING_READING; + +error_return: + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + /* Any async authentication state should have been cleaned up already. */ + Assert(!state->async_ctx); + -+ free(state->token); + free(state); +} + @@ src/interfaces/libpq/fe-auth-oauth.c (new) +/* + * Constructs an OAUTHBEARER client initial response (RFC 7628, Sec. 3.1). + * -+ * If discover is true, the token pointer will be ignored and the initial -+ * response will instead contain a request for the server's required OAuth -+ * parameters (Sec. 4.3). Otherwise, a bearer token must be provided. ++ * If discover is true, the initial response will contain a request for the ++ * server's required OAuth parameters (Sec. 4.3). Otherwise, conn->token must ++ * be set; it will be sent as the connection's bearer token. + * + * Returns the response as a null-terminated string, or NULL on error. + */ +static char * -+client_initial_response(PGconn *conn, bool discover, const char *token) ++client_initial_response(PGconn *conn, bool discover) +{ + static const char *const resp_format = "n,," kvsep "auth=%s%s" kvsep kvsep; + + PQExpBufferData buf; + const char *authn_scheme; + char *response = NULL; ++ const char *token = conn->oauth_token; + + if (discover) + { @@ src/interfaces/libpq/fe-auth-oauth.c (new) + */ + authn_scheme = "Bearer "; + -+ /* We must have a token. */ ++ /* conn->token must have been set in this case. */ + if (!token) + { -+ /* -+ * Either programmer error, or something went badly wrong during -+ * the asynchronous fetch. -+ * -+ * TODO: users shouldn't see this; what action should they take if -+ * they do? -+ */ ++ Assert(false); + libpq_append_conn_error(conn, -+ "no OAuth token was set for the connection"); ++ "internal error: no OAuth token was set for the connection"); + return NULL; + } + } @@ src/interfaces/libpq/fe-auth-oauth.c (new) +/* + * Parses the server error result (RFC 7628, Sec. 3.2.2) contained in msg and + * stores any discovered openid_configuration and scope settings for the -+ * connection. conn->oauth_want_retry will be set if the error status is -+ * suitable for a second attempt. ++ * connection. + */ +static bool +handle_oauth_sasl_error(PGconn *conn, const char *msg, int msglen) @@ src/interfaces/libpq/fe-auth-oauth.c (new) + if (errmsg) + goto cleanup; + -+ /* 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; @@ src/interfaces/libpq/fe-auth-oauth.c (new) + + free(discovery_issuer); + -+ if (conn->oauth_discovery_uri) -+ free(conn->oauth_discovery_uri); -+ -+ conn->oauth_discovery_uri = ctx.discovery_uri; -+ ctx.discovery_uri = NULL; ++ if (!conn->oauth_discovery_uri) ++ { ++ conn->oauth_discovery_uri = ctx.discovery_uri; ++ ctx.discovery_uri = NULL; ++ } ++ else ++ { ++ /* This must match the URI we'd previously determined. */ ++ if (strcmp(conn->oauth_discovery_uri, ctx.discovery_uri) != 0) ++ { ++ libpq_append_conn_error(conn, ++ "server's discovery document has moved to %s (previous location was %s)", ++ ctx.discovery_uri, ++ conn->oauth_discovery_uri); ++ goto cleanup; ++ } ++ } + } + + if (ctx.scope) + { -+ if (conn->oauth_scope) -+ free(conn->oauth_scope); -+ -+ conn->oauth_scope = ctx.scope; -+ ctx.scope = NULL; ++ /* Servers may not override a previously set oauth_scope. */ ++ if (!conn->oauth_scope) ++ { ++ conn->oauth_scope = ctx.scope; ++ ctx.scope = NULL; ++ } + } -+ /* TODO: missing error scope should clear any existing connection scope */ + + if (!ctx.status) + { @@ src/interfaces/libpq/fe-auth-oauth.c (new) + goto cleanup; + } + -+ if (strcmp(ctx.status, "invalid_token") == 0) ++ if (strcmp(ctx.status, "invalid_token") != 0) + { + /* -+ * invalid_token is the only error code we'll automatically retry for, -+ * but only if we have enough information to do so and we haven't -+ * already retried this connection once. ++ * invalid_token is the only error code we'll automatically retry for; ++ * otherwise, just bail out now. + */ -+ if (conn->oauth_discovery_uri && conn->oauth_want_retry == PG_BOOL_UNKNOWN) -+ conn->oauth_want_retry = PG_BOOL_YES; ++ libpq_append_conn_error(conn, ++ "server rejected OAuth bearer token: %s", ++ ctx.status); ++ goto cleanup; + } -+ /* TODO: include status in hard failure message */ + + success = true; + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + else if (status == PGRES_POLLING_OK) + { + /* -+ * We already have a token, so copy it into the state. (We can't hold ++ * We already have a token, so copy it into the conn. (We can't hold + * onto the original string, since it may not be safe for us to free() + * it.) + */ @@ src/interfaces/libpq/fe-auth-oauth.c (new) + return PGRES_POLLING_FAILED; + } + -+ state->token = strdup(request->token); -+ if (!state->token) ++ conn->oauth_token = strdup(request->token); ++ if (!conn->oauth_token) + { + libpq_append_conn_error(conn, "out of memory"); + return PGRES_POLLING_FAILED; @@ src/interfaces/libpq/fe-auth-oauth.c (new) + return PGRES_POLLING_OK; + } + -+ /* TODO: what if no altsock was set? */ ++ /* The hook wants the client to poll the altsock. Make sure it set one. */ ++ if (conn->altsock == PGINVALID_SOCKET) ++ { ++ libpq_append_conn_error(conn, ++ "user-defined OAuth flow did not provide a socket for polling"); ++ return PGRES_POLLING_FAILED; ++ } ++ + return status; +} + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + if (request.token) + { + /* -+ * We already have a token, so copy it into the state. (We can't ++ * We already have a token, so copy it into the conn. (We can't + * hold onto the original string, since it may not be safe for us + * to free() it.) + */ -+ state->token = strdup(request.token); -+ if (!state->token) ++ conn->oauth_token = strdup(request.token); ++ if (!conn->oauth_token) + { + libpq_append_conn_error(conn, "out of memory"); + goto fail; @@ src/interfaces/libpq/fe-auth-oauth.c (new) + else + { +#if USE_LIBCURL -+ /* -+ * Hand off to our built-in OAuth flow. -+ * -+ * Only allow one try per connection, since we're not performing any -+ * caching at the moment. (Custom flows might be more sophisticated.) -+ */ ++ /* Hand off to our built-in OAuth flow. */ + conn->async_auth = pg_fe_run_oauth_flow; + conn->cleanup_async_auth = pg_fe_cleanup_oauth_flow; -+ conn->oauth_want_retry = PG_BOOL_NO; + +#else + libpq_append_conn_error(conn, "no custom OAuth flows are available, and libpq was not built with libcurl support"); @@ src/interfaces/libpq/fe-auth-oauth.c (new) + if (!setup_oauth_parameters(conn)) + return SASL_FAILED; + -+ if (conn->oauth_discovery_uri) ++ if (conn->oauth_token) + { + /* -+ * Decide whether we're using a user-provided OAuth flow, or -+ * the one we have built in. ++ * A previous connection already fetched the token; we'll use ++ * it below. ++ */ ++ } ++ else if (conn->oauth_discovery_uri) ++ { ++ /* ++ * We don't have a token, but we have a discovery URI already ++ * stored. Decide whether we're using a user-provided OAuth ++ * flow or the one we have built in. + */ + if (!setup_token_request(conn, state)) + return SASL_FAILED; + -+ if (state->token) ++ if (conn->oauth_token) + { + /* + * A really smart user implementation may have already + * given us the token (e.g. if there was an unexpired copy -+ * already cached). In that case, we can just fall -+ * through. ++ * already cached), and we can use it immediately. + */ + } + else + { + /* -+ * Otherwise, we have to hand the connection over to our -+ * OAuth implementation. This involves a number of HTTP -+ * connections and timed waits, so we escape the -+ * synchronous auth processing and tell PQconnectPoll to -+ * transfer control to our async implementation. ++ * Otherwise, we'll have to hand the connection over to ++ * our OAuth implementation. ++ * ++ * This could take a while, since it generally involves a ++ * user in the loop. To avoid consuming the server's ++ * authentication timeout, we'll continue this handshake ++ * to the end, so that the server can close its side of ++ * the connection. We'll open a second connection later ++ * once we've retrieved a token. + */ -+ Assert(conn->async_auth); /* should have been set -+ * already */ -+ state->step = FE_OAUTH_REQUESTING_TOKEN; -+ return SASL_ASYNC; ++ discover = true; + } + } + else + { + /* -+ * If we don't have a discovery URI to be able to request a -+ * token, we ask the server for one explicitly. This doesn't -+ * require any asynchronous work. ++ * If we don't have a token, and we don't have a discovery URI ++ * to be able to request a token, we ask the server for one ++ * explicitly. + */ + discover = true; + } + -+ /* fall through */ -+ -+ case FE_OAUTH_REQUESTING_TOKEN: -+ /* We should still be in the initial response phase. */ -+ Assert(inputlen == -1); -+ -+ *output = client_initial_response(conn, discover, state->token); ++ /* ++ * Generate an initial response. This either contains a token, if ++ * we have one, or an empty discovery response which is doomed to ++ * fail. ++ */ ++ *output = client_initial_response(conn, discover); + if (!*output) + return SASL_FAILED; + + *outputlen = strlen(*output); + state->step = FE_OAUTH_BEARER_SENT; + ++ if (conn->oauth_token) ++ { ++ /* ++ * For the purposes of require_auth, our side of ++ * authentication is done at this point; the server will ++ * either accept the connection or send an error. Unlike ++ * SCRAM, there is no additional server data to check upon ++ * success. ++ */ ++ conn->client_finished_auth = true; ++ } ++ + return SASL_CONTINUE; + + case FE_OAUTH_BEARER_SENT: + if (final) + { -+ /* TODO: ensure there is no message content here. */ -+ return SASL_COMPLETE; -+ } -+ -+ /* -+ * Error message sent by the server. -+ */ -+ if (!handle_oauth_sasl_error(conn, input, inputlen)) ++ /* ++ * OAUTHBEARER does not make use of additional data with a ++ * successful SASL exchange, so we shouldn't get an ++ * AuthenticationSASLFinal message. ++ */ ++ libpq_append_conn_error(conn, ++ "server sent unexpected additional OAuth data"); + return SASL_FAILED; ++ } + + /* -+ * Respond with the required dummy message (RFC 7628, sec. 3.2.3). ++ * An error message was sent by the server. Respond with the ++ * required dummy message (RFC 7628, sec. 3.2.3). + */ + *output = strdup(kvsep); + if (unlikely(!*output)) @@ src/interfaces/libpq/fe-auth-oauth.c (new) + } + *outputlen = strlen(*output); /* == 1 */ + -+ state->step = FE_OAUTH_SERVER_ERROR; -+ return SASL_CONTINUE; ++ /* Grab the settings from discovery. */ ++ if (!handle_oauth_sasl_error(conn, input, inputlen)) ++ return SASL_FAILED; ++ ++ if (conn->oauth_token) ++ { ++ /* ++ * The server rejected our token. Continue onwards towards the ++ * expected FATAL message, but mark our state to catch any ++ * unexpected "success" from the server. ++ */ ++ state->step = FE_OAUTH_SERVER_ERROR; ++ return SASL_CONTINUE; ++ } ++ ++ if (!conn->async_auth) ++ { ++ /* ++ * No OAuth flow is set up yet. Did we get enough information ++ * from the server to create one? ++ */ ++ if (!conn->oauth_discovery_uri) ++ { ++ libpq_append_conn_error(conn, ++ "server requires OAuth authentication, but no discovery metadata was provided"); ++ return SASL_FAILED; ++ } ++ ++ /* Yes. Set up the flow now. */ ++ if (!setup_token_request(conn, state)) ++ return SASL_FAILED; ++ ++ if (conn->oauth_token) ++ { ++ /* ++ * A token was available in a custom flow's cache. Skip ++ * the asynchronous processing. ++ */ ++ goto reconnect; ++ } ++ } ++ ++ /* ++ * Time to retrieve a token. This involves a number of HTTP ++ * connections and timed waits, so we escape the synchronous auth ++ * processing and tell PQconnectPoll to transfer control to our ++ * async implementation. ++ */ ++ Assert(conn->async_auth); /* should have been set already */ ++ state->step = FE_OAUTH_REQUESTING_TOKEN; ++ return SASL_ASYNC; ++ ++ case FE_OAUTH_REQUESTING_TOKEN: ++ ++ /* ++ * We've returned successfully from token retrieval. Double-check ++ * that we have what we need for the next connection. ++ */ ++ if (!conn->oauth_token) ++ { ++ Assert(false); /* should have failed before this point! */ ++ libpq_append_conn_error(conn, ++ "internal error: OAuth flow did not set a token"); ++ return SASL_FAILED; ++ } ++ ++ goto reconnect; + + case FE_OAUTH_SERVER_ERROR: + @@ src/interfaces/libpq/fe-auth-oauth.c (new) + break; + } + ++ Assert(false); /* should never get here */ ++ return SASL_FAILED; ++ ++reconnect: ++ ++ /* ++ * Despite being a failure from the point of view of SASL, we have enough ++ * information to restart with a new connection. ++ */ ++ libpq_append_conn_error(conn, "retrying connection with new bearer token"); ++ conn->oauth_want_retry = true; + return SASL_FAILED; +} + @@ src/interfaces/libpq/fe-auth-oauth.c (new) +} + +/* ++ * Fully clears out any stored OAuth token. This is done proactively upon ++ * successful connection as well as during pqClosePGconn(). ++ */ ++void ++pqClearOAuthToken(PGconn *conn) ++{ ++ if (!conn->oauth_token) ++ return; ++ ++ explicit_bzero(conn->oauth_token, strlen(conn->oauth_token)); ++ free(conn->oauth_token); ++ conn->oauth_token = NULL; ++} ++ ++/* + * Returns true if the PGOAUTHDEBUG=UNSAFE flag is set in the environment. + */ +bool @@ src/interfaces/libpq/fe-auth-oauth.h (new) +enum fe_oauth_step +{ + FE_OAUTH_INIT, -+ FE_OAUTH_REQUESTING_TOKEN, + FE_OAUTH_BEARER_SENT, ++ FE_OAUTH_REQUESTING_TOKEN, + FE_OAUTH_SERVER_ERROR, +}; + @@ src/interfaces/libpq/fe-auth-oauth.h (new) + enum fe_oauth_step step; + + PGconn *conn; -+ char *token; -+ + void *async_ctx; +} fe_oauth_state; + +extern PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn); +extern void pg_fe_cleanup_oauth_flow(PGconn *conn); ++extern void pqClearOAuthToken(PGconn *conn); +extern bool oauth_unsafe_debugging_enabled(void); + +/* Mechanisms in fe-auth-oauth.c */ @@ src/interfaces/libpq/fe-auth.c: pg_SASL_init(PGconn *conn, int payloadlen, bool } if (!selected_mechanism) +@@ src/interfaces/libpq/fe-auth.c: pg_SASL_init(PGconn *conn, int payloadlen, bool *async) + + if (!allowed) + { +- /* +- * TODO: this is dead code until a second SASL mechanism is added; +- * the connection can't have proceeded past check_expected_areq() +- * if no SASL methods are allowed. +- */ +- Assert(false); +- + libpq_append_conn_error(conn, "authentication method requirement \"%s\" failed: server requested %s authentication", + conn->require_auth, selected_mechanism); + goto error; @@ src/interfaces/libpq/fe-auth.c: PQchangePassword(PGconn *conn, const char *user, const char *passwd) } } @@ src/interfaces/libpq/fe-connect.c #include "libpq-int.h" #include "mb/pg_wchar.h" @@ src/interfaces/libpq/fe-connect.c: static const internalPQconninfoOption PQconninfoOptions[] = { - "Load-Balance-Hosts", "", 8, /* sizeof("disable") = 8 */ - offsetof(struct pg_conn, load_balance_hosts)}, + {"scram_server_key", NULL, NULL, NULL, "SCRAM-Server-Key", "D", SCRAM_MAX_KEY_LEN * 2, + offsetof(struct pg_conn, scram_server_key)}, + /* OAuth v2 */ + {"oauth_issuer", NULL, NULL, NULL, @@ src/interfaces/libpq/fe-connect.c: pqDropServerData(PGconn *conn) conn->write_failed = false; free(conn->write_err_msg); conn->write_err_msg = NULL; -+ /* conn->oauth_want_retry = false; TODO */ ++ conn->oauth_want_retry = false; /* * Cancel connections need to retain their be_pid and be_key across +@@ src/interfaces/libpq/fe-connect.c: static inline void + fill_allowed_sasl_mechs(PGconn *conn) + { + /*--- +- * We only support one mechanism at the moment, so rather than deal with a ++ * We only support two mechanisms at the moment, so rather than deal with a + * linked list, conn->allowed_sasl_mechs is an array of static length. We + * rely on the compile-time assertion here to keep us honest. + * +@@ src/interfaces/libpq/fe-connect.c: fill_allowed_sasl_mechs(PGconn *conn) + * - handle the new mechanism name in the require_auth portion of + * pqConnectOptions2(), below. + */ +- StaticAssertDecl(lengthof(conn->allowed_sasl_mechs) == 1, ++ StaticAssertDecl(lengthof(conn->allowed_sasl_mechs) == 2, + "fill_allowed_sasl_mechs() must be updated when resizing conn->allowed_sasl_mechs[]"); + + conn->allowed_sasl_mechs[0] = &pg_scram_mech; ++ conn->allowed_sasl_mechs[1] = &pg_oauth_mech; + } + + /* +@@ src/interfaces/libpq/fe-connect.c: pqConnectOptions2(PGconn *conn) + { + mech = &pg_scram_mech; + } ++ else if (strcmp(method, "oauth") == 0) ++ { ++ mech = &pg_oauth_mech; ++ } + + /* + * Final group: meta-options. @@ src/interfaces/libpq/fe-connect.c: keep_going: /* We will come back to here until there is - /* Check to see if we should mention pgpassfile */ - pgpassfileWarning(conn); + conn->inStart = conn->inCursor; + if (res != STATUS_OK) ++ { + /* + * OAuth connections may perform two-step discovery, where + * the first connection is a dummy. + */ -+ if (conn->sasl == &pg_oauth_mech -+ && conn->oauth_want_retry == PG_BOOL_YES) ++ if (conn->sasl == &pg_oauth_mech && conn->oauth_want_retry) + { -+ /* Only allow retry once. */ -+ conn->oauth_want_retry = PG_BOOL_NO; + need_new_connection = true; + goto keep_going; + } + - CONNECTION_FAILED(); + goto error_return; ++ } + + /* + * Just make sure that any data sent by pg_fe_sendauth is +@@ src/interfaces/libpq/fe-connect.c: keep_going: /* We will come back to here until there is + } } - else if (beresp == PqMsg_NegotiateProtocolVersion) + ++ /* Don't hold onto any OAuth tokens longer than necessary. */ ++ pqClearOAuthToken(conn); ++ + /* + * For non cancel requests we can release the address list + * now. For cancel requests we never actually resolve @@ src/interfaces/libpq/fe-connect.c: freePGconn(PGconn *conn) - free(conn->rowBuf); - free(conn->target_session_attrs); free(conn->load_balance_hosts); + free(conn->scram_client_key); + free(conn->scram_server_key); + free(conn->oauth_issuer); + free(conn->oauth_issuer_id); + free(conn->oauth_discovery_uri); @@ src/interfaces/libpq/fe-connect.c: freePGconn(PGconn *conn) termPQExpBuffer(&conn->errorMessage); termPQExpBuffer(&conn->workBuffer); +@@ src/interfaces/libpq/fe-connect.c: pqClosePGconn(PGconn *conn) + conn->asyncStatus = PGASYNC_IDLE; + conn->xactStatus = PQTRANS_IDLE; + conn->pipelineStatus = PQ_PIPELINE_OFF; ++ pqClearOAuthToken(conn); + pqClearAsyncResult(conn); /* deallocate result */ + pqClearConnErrorState(conn); + ## src/interfaces/libpq/libpq-fe.h ## @@ src/interfaces/libpq/libpq-fe.h: extern "C" @@ src/interfaces/libpq/libpq-int.h: struct pg_conn + char *oauth_client_id; /* client identifier */ + char *oauth_client_secret; /* client secret */ + char *oauth_scope; /* access token scope */ -+ PGTernaryBool oauth_want_retry; /* should we retry on failure? */ ++ char *oauth_token; /* access token */ ++ bool oauth_want_retry; /* should we retry on failure? */ + /* Optional file to write trace info to */ FILE *Pfdebug; int traceFlags; +@@ src/interfaces/libpq/libpq-int.h: struct pg_conn + * the server? */ + uint32 allowed_auth_methods; /* bitmask of acceptable AuthRequest + * codes */ +- const pg_fe_sasl_mech *allowed_sasl_mechs[1]; /* and acceptable SASL ++ const pg_fe_sasl_mech *allowed_sasl_mechs[2]; /* and acceptable SASL + * mechanisms */ + bool client_finished_auth; /* have we finished our half of the + * authentication exchange? */ ## src/interfaces/libpq/meson.build ## @@ @@ src/makefiles/meson.build: pgxs_deps = { 'libxslt': libxslt, 'llvm': llvm, + ## src/test/authentication/t/001_password.pl ## +@@ src/test/authentication/t/001_password.pl: $node->connect_fails( + $node->connect_fails( + "user=scram_role require_auth=!scram-sha-256", + "SCRAM authentication forbidden, fails with SCRAM auth", +- expected_stderr => qr/server requested SASL authentication/); ++ expected_stderr => qr/server requested SCRAM-SHA-256 authentication/); + $node->connect_fails( + "user=scram_role require_auth=!password,!md5,!scram-sha-256", + "multiple authentication types forbidden, fails with SCRAM auth", +- expected_stderr => qr/server requested SASL authentication/); ++ expected_stderr => qr/server requested SCRAM-SHA-256 authentication/); + + # Test that bad passwords are rejected. + $ENV{"PGPASSWORD"} = 'badpass'; +@@ src/test/authentication/t/001_password.pl: $node->connect_fails( + "user=scram_role require_auth=!scram-sha-256", + "password authentication forbidden, fails with SCRAM auth", + expected_stderr => +- qr/authentication method requirement "!scram-sha-256" failed: server requested SASL authentication/ ++ qr/authentication method requirement "!scram-sha-256" failed: server requested SCRAM-SHA-256 authentication/ + ); + $node->connect_fails( + "user=scram_role require_auth=!password,!md5,!scram-sha-256", + "multiple authentication types forbidden, fails with SCRAM auth", + expected_stderr => +- qr/authentication method requirement "!password,!md5,!scram-sha-256" failed: server requested SASL authentication/ ++ qr/authentication method requirement "!password,!md5,!scram-sha-256" failed: server requested SCRAM-SHA-256 authentication/ + ); + + # Test SYSTEM_USER <> NULL with parallel workers. + ## src/test/modules/Makefile ## @@ src/test/modules/Makefile: SUBDIRS = \ dummy_index_am \ @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) +static PostgresPollingStatusType async_cb(PGconn *conn, + PGoauthBearerRequest *req, + pgsocket *altsock); ++static PostgresPollingStatusType misbehave_cb(PGconn *conn, ++ PGoauthBearerRequest *req, ++ pgsocket *altsock); + +static void +usage(char *argv[]) @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) + printf(" -h, --help show this message\n"); + printf(" --expected-scope SCOPE fail if received scopes do not match SCOPE\n"); + printf(" --expected-uri URI fail if received configuration link does not match URI\n"); ++ printf(" --misbehave=MODE have the hook fail required postconditions\n" ++ " (MODEs: no-hook, fail-async, no-token, no-socket)\n"); + printf(" --no-hook don't install OAuth hooks (connection will fail)\n"); + printf(" --hang-forever don't ever return a token (combine with connect_timeout)\n"); + printf(" --token TOKEN use the provided TOKEN value\n"); @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) +static bool hang_forever = false; +static const char *expected_uri = NULL; +static const char *expected_scope = NULL; ++static const char *misbehave_mode = NULL; +static char *token = NULL; + +int @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) + {"no-hook", no_argument, NULL, 1002}, + {"token", required_argument, NULL, 1003}, + {"hang-forever", no_argument, NULL, 1004}, ++ {"misbehave", required_argument, NULL, 1005}, + {0} + }; + @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) + hang_forever = true; + break; + ++ case 1005: /* --misbehave */ ++ misbehave_mode = optarg; ++ break; ++ + default: + usage(argv); + return 1; @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) + return 1; + } + ++ if (misbehave_mode) ++ { ++ if (strcmp(misbehave_mode, "no-hook") != 0) ++ req->async = misbehave_cb; ++ return 1; ++ } ++ + if (expected_uri) + { + if (!req->openid_configuration) @@ src/test/modules/oauth_validator/oauth_hook_client.c (new) + + req->token = token; + return PGRES_POLLING_OK; ++} ++ ++static PostgresPollingStatusType ++misbehave_cb(PGconn *conn, PGoauthBearerRequest *req, pgsocket *altsock) ++{ ++ if (strcmp(misbehave_mode, "fail-async") == 0) ++ { ++ /* Just fail "normally". */ ++ return PGRES_POLLING_FAILED; ++ } ++ else if (strcmp(misbehave_mode, "no-token") == 0) ++ { ++ /* Callbacks must assign req->token before returning OK. */ ++ return PGRES_POLLING_OK; ++ } ++ else if (strcmp(misbehave_mode, "no-socket") == 0) ++ { ++ /* Callbacks must assign *altsock before asking for polling. */ ++ return PGRES_POLLING_READING; ++ } ++ else ++ { ++ fprintf(stderr, "unrecognized --misbehave mode: %s\n", misbehave_mode); ++ exit(1); ++ } +} ## src/test/modules/oauth_validator/t/001_server.pl (new) ## @@ src/test/modules/oauth_validator/t/001_server.pl (new) + 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$@ +); + ++# Test require_auth settings against OAUTHBEARER. ++my @cases = ( ++ { require_auth => "oauth" }, ++ { require_auth => "oauth,scram-sha-256" }, ++ { require_auth => "password,oauth" }, ++ { require_auth => "none,oauth" }, ++ { require_auth => "!scram-sha-256" }, ++ { require_auth => "!none" }, ++ ++ { ++ require_auth => "!oauth", ++ failure => qr/server requested OAUTHBEARER authentication/ ++ }, ++ { ++ require_auth => "scram-sha-256", ++ failure => qr/server requested OAUTHBEARER authentication/ ++ }, ++ { ++ require_auth => "!password,!oauth", ++ failure => qr/server requested OAUTHBEARER authentication/ ++ }, ++ { ++ require_auth => "none", ++ failure => qr/server requested SASL authentication/ ++ }, ++ { ++ require_auth => "!oauth,!scram-sha-256", ++ failure => qr/server requested SASL authentication/ ++ }); ++ ++$user = "test"; ++foreach my $c (@cases) ++{ ++ my $connstr = ++ "user=$user dbname=postgres oauth_issuer=$issuer oauth_client_id=f02c6361-0635 require_auth=$c->{'require_auth'}"; ++ ++ if (defined $c->{'failure'}) ++ { ++ $node->connect_fails( ++ $connstr, ++ "require_auth=$c->{'require_auth'} fails", ++ expected_stderr => $c->{'failure'}); ++ } ++ else ++ { ++ $node->connect_ok( ++ $connstr, ++ "require_auth=$c->{'require_auth'} succeeds", ++ expected_stderr => ++ qr@Visit https://example\.com/ and enter the code: postgresuser@ ++ ); ++ } ++} ++ +# 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_issuer=$issuer/alternate oauth_client_id='$vschars_esc'", ++ "user=$user dbname=postgres oauth_issuer=$issuer oauth_client_id='$vschars_esc'", + "escapable characters: client_id", + expected_stderr => -+ qr@Visit https://example\.org/ and enter the code: postgresuser@); ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); +$node->connect_ok( -+ "user=$user dbname=postgres oauth_issuer=$issuer/alternate oauth_client_id='$vschars_esc' oauth_client_secret='$vschars_esc'", ++ "user=$user dbname=postgres oauth_issuer=$issuer 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@); ++ qr@Visit https://example\.com/ and enter the code: postgresuser@); + +# +# Further tests rely on support for specific behaviors in oauth_server.py. To @@ src/test/modules/oauth_validator/t/001_server.pl (new) +$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" ++local all testalt oauth validator=fail_validator issuer="$issuer/.well-known/oauth-authorization-server/alternate" scope="openid postgres alt" +}); +$node->restart; + @@ src/test/modules/oauth_validator/t/002_client.pl (new) + flags => ["--hang-forever"], + expected_stderr => qr/failed: timeout expired/); + ++# Test various misbehaviors of the client hook. ++my @cases = ( ++ { ++ flag => "--misbehave=no-hook", ++ expected_error => ++ qr/user-defined OAuth flow provided neither a token nor an async callback/, ++ }, ++ { ++ flag => "--misbehave=fail-async", ++ expected_error => qr/user-defined OAuth flow failed/, ++ }, ++ { ++ flag => "--misbehave=no-token", ++ expected_error => qr/user-defined OAuth flow did not provide a token/, ++ }, ++ { ++ flag => "--misbehave=no-socket", ++ expected_error => ++ qr/user-defined OAuth flow did not provide a socket for polling/, ++ }); ++ ++foreach my $c (@cases) ++{ ++ test( ++ "hook misbehavior: $c->{'flag'}", ++ flags => [ $c->{'flag'} ], ++ expected_stderr => $c->{'expected_error'}); ++} ++ +done_testing(); ## src/test/modules/oauth_validator/t/OAuth/Server.pm (new) ## 5: 18507c6978b < -: ----------- squash! Add OAUTHBEARER SASL mechanism 6: 8e82059700b = 5: 97e0a2aae26 XXX fix libcurl link error 7: 5339b3f2617 ! 6: db0167009b9 DO NOT MERGE: Add pytest suite for OAuth @@ src/test/python/client/conftest.py (new) + client.start() + + sock, _ = server_socket.accept() ++ sock.settimeout(BLOCKING_TIMEOUT) + return sock, client + + yield factory @@ src/test/python/client/test_oauth.py (new) + ) + + ++def handle_discovery_connection(sock, discovery=None, *, response=None): ++ """ ++ Helper for all tests that expect an initial discovery connection from the ++ client. The provided discovery URI will be used in a standard error response ++ from the server (or response may be set, to provide a custom dictionary), ++ and the SASL exchange will be failed. ++ ++ By default, the client is expected to complete the entire handshake. Set ++ finish to False if the client should immediately disconnect when it receives ++ the error response. ++ """ ++ if response is None: ++ response = {"status": "invalid_token"} ++ if discovery is not None: ++ response["openid-configuration"] = discovery ++ ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ # Initiate a handshake. ++ 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"" ++ ++ # The discovery handshake is doomed to fail. ++ fail_oauth_handshake(conn, response) ++ ++ +class RawResponse(str): + """ + Returned by registered endpoint callbacks to take full control of the @@ src/test/python/client/test_oauth.py (new) + libpq.PQsetAuthDataHook(None) + + -+@pytest.mark.parametrize("success", [True, False]) ++@pytest.mark.parametrize( ++ "success, abnormal_failure", ++ [ ++ pytest.param(True, False, id="success"), ++ pytest.param(False, False, id="normal failure"), ++ pytest.param(False, True, id="abnormal failure"), ++ ], ++) +@pytest.mark.parametrize("secret", [None, "", "hunter2"]) +@pytest.mark.parametrize("scope", [None, "", "openid email"]) +@pytest.mark.parametrize("retries", [0, 1]) @@ src/test/python/client/test_oauth.py (new) + secret, + auth_data_cb, + success, ++ abnormal_failure, +): + client_id = secrets.token_hex() + openid_provider.content_type = content_type @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + ++ # First connection is a discovery request, which should result in the above ++ # endpoints being called. ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) ++ ++ # Client should reconnect. ++ sock, _ = accept() + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: -+ # Initiate a handshake, which should result in the above endpoints -+ # being called. + initial = start_oauth_handshake(conn) + + # Validate and accept the token. @@ src/test/python/client/test_oauth.py (new) + assert auth == f"Bearer {access_token}".encode("ascii") + + if success: -+ pq3.send(conn, pq3.types.AuthnRequest, type=pq3.authn.SASLFinal) + finish_handshake(conn) + ++ elif abnormal_failure: ++ # Send an empty error response, which should result in a ++ # mechanism-level failure in the client. This test ensures that ++ # the client doesn't try a third connection for this case. ++ expected_error = "server sent error response without a status" ++ fail_oauth_handshake(conn, {}) ++ + else: + # Simulate token validation failure. + resp = { @@ src/test/python/client/test_oauth.py (new) + "openid-configuration": openid_provider.discovery_uri, + } + expected_error = "test token validation failure" -+ + fail_oauth_handshake(conn, resp, errmsg=expected_error) + + if retries: @@ src/test/python/client/test_oauth.py (new) + + 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: ++ handle_discovery_connection(sock, discovery_uri) + ++ # Expect the client to connect again. ++ sock, _ = accept() + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: + initial = start_oauth_handshake(conn) @@ src/test/python/client/test_oauth.py (new) + 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) + + @@ src/test/python/client/test_oauth.py (new) + "{issuer}", + "/.well-known/oauth-authorization-server/", + None, -+ id="extra empty segment", ++ id="extra empty segment (no path)", ++ ), ++ pytest.param( ++ "{issuer}/path", ++ "/.well-known/oauth-authorization-server/path/", ++ None, ++ id="extra empty segment (with path)", + ), + pytest.param( + "{issuer}", @@ src/test/python/client/test_oauth.py (new) + 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) ++ with sock: ++ if expected_error and not server_discovery: ++ # If the client already knows the URL, it should disconnect as soon ++ # as it realizes it's not valid. ++ expect_disconnected_handshake(sock) ++ else: ++ # Otherwise, it should complete the connection. ++ handle_discovery_connection(sock, discovery_uri) ++ ++ # The client should not reconnect. + + if expected_error is None: + if server_discovery: @@ src/test/python/client/test_oauth.py (new) + the client to have an oauth_issuer set so that it doesn't try to go through + discovery. + """ -+ with sock: -+ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: -+ # Initiate a handshake. -+ startup = pq3.recv1(conn, cls=pq3.Startup) -+ assert startup.proto == pq3.protocol(3, 0) ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ # Initiate a handshake. ++ startup = pq3.recv1(conn, cls=pq3.Startup) ++ assert startup.proto == pq3.protocol(3, 0) + -+ pq3.send( -+ conn, -+ pq3.types.AuthnRequest, -+ type=pq3.authn.SASL, -+ body=[b"OAUTHBEARER", b""], -+ ) ++ pq3.send( ++ conn, ++ pq3.types.AuthnRequest, ++ type=pq3.authn.SASL, ++ body=[b"OAUTHBEARER", b""], ++ ) + -+ # The client should disconnect at this point. -+ assert not conn.read(1), "client sent unexpected data" ++ # The client should disconnect at this point. ++ assert not conn.read(1), "client sent unexpected data" + + +@pytest.mark.parametrize( @@ src/test/python/client/test_oauth.py (new) + del params[k] + + sock, client = accept(**params) -+ expect_disconnected_handshake(sock) ++ with sock: ++ expect_disconnected_handshake(sock) + + expected_error = "oauth_issuer and oauth_client_id are not both set" + with pytest.raises(psycopg2.OperationalError, match=expected_error): @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + ++ # First connection is a discovery request, which should result in the above ++ # endpoints being called. ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) ++ ++ # Second connection sends the token. ++ sock, _ = accept() + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: -+ # Initiate a handshake, which should result in the above endpoints -+ # being called. + initial = start_oauth_handshake(conn) + + # Validate and accept 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) + + @@ src/test/python/client/test_oauth.py (new) + attempts = 0 + last_retry = None + retry_lock = threading.Lock() ++ token_sent = threading.Event() + + def token_endpoint(headers, params): + now = time.monotonic() @@ src/test/python/client/test_oauth.py (new) + + return 400, {"error": error_code} + -+ # Successfully finish the request by sending the access bearer token. ++ # Successfully finish the request by sending the access bearer token, ++ # and signal the main thread to continue. + resp = { + "access_token": access_token, + "token_type": "bearer", + } ++ token_sent.set() + + return 200, resp + @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + ++ # First connection is a discovery request, which should result in the above ++ # endpoints being called. ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) ++ ++ # At this point the client is talking to the authorization server. Wait for ++ # that to succeed so we don't run into the accept() timeout. ++ token_sent.wait() ++ ++ # Client should reconnect and send the token. ++ sock, _ = accept() + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: -+ # Initiate a handshake, which should result in the above endpoints -+ # being called. + initial = start_oauth_handshake(conn) + + # Validate and accept 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) + + @@ src/test/python/client/test_oauth.py (new) + auth_data_cb.impl = bearer_hook + + # Now drive the server side. ++ if retries >= 0: ++ # First connection is a discovery request, which should result in the ++ # hook being invoked. ++ with sock: ++ handle_discovery_connection(sock, discovery_uri) ++ ++ # Client should reconnect to send the token. ++ sock, _ = accept() ++ + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: + # Initiate a handshake, which should result in our custom callback @@ src/test/python/client/test_oauth.py (new) + 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) + + # Check the data provided to the hook. @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + -+ expect_disconnected_handshake(sock) ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) + + # Now make sure the client correctly failed. + with pytest.raises(psycopg2.OperationalError, match=error_pattern): @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + -+ expect_disconnected_handshake(sock) ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) + + # Now make sure the client correctly failed. + if bad_value is Missing: @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + -+ expect_disconnected_handshake(sock) ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) + + # Now make sure the client correctly failed. + with pytest.raises(psycopg2.OperationalError, match=error_pattern): @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + -+ expect_disconnected_handshake(sock) ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) + + # Now make sure the client correctly failed. + error_pattern = "failed to parse access token response: " @@ src/test/python/client/test_oauth.py (new) + fail_resp["scope"] = scope + + 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 first SASL exchange. -+ fail_oauth_handshake(conn, fail_resp) ++ handle_discovery_connection(sock, response=fail_resp) + + # The client will connect to us a second time, using the parameters we sent + # it. @@ src/test/python/client/test_oauth.py (new) + assert auth == f"Bearer {access_token}".encode("ascii") + + if success: -+ pq3.send(conn, pq3.types.AuthnRequest, type=pq3.authn.SASLFinal) + finish_handshake(conn) + + else: @@ src/test/python/client/test_oauth.py (new) + failing_discovery_handler, + ) + -+ expect_disconnected_handshake(sock) ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) + + with pytest.raises(psycopg2.OperationalError, match=expected_error): + client.check_completed() @@ src/test/python/client/test_oauth.py (new) + dict( + fields=[b"SFATAL", b"C28000", b"Mexpected error message", b""], + ), -+ "expected error message", ++ "server rejected OAuth bearer token: invalid_request", + id="standard server error: invalid_request", + ), + pytest.param( @@ src/test/python/client/test_oauth.py (new) + id="standard server error: invalid_token without discovery URI", + ), + pytest.param( -+ {"status": "invalid_request"}, ++ {"status": "invalid_token", "openid-configuration": ""}, + pq3.types.AuthnRequest, + dict(type=pq3.authn.SASLContinue, body=b""), + "server sent additional OAuth data", + id="broken server: additional challenge after error", + ), + pytest.param( -+ {"status": "invalid_request"}, ++ {"status": "invalid_token", "openid-configuration": ""}, + pq3.types.AuthnRequest, + dict(type=pq3.authn.SASLFinal), + "server sent additional OAuth data", + id="broken server: SASL success after error", + ), + pytest.param( -+ {"status": "invalid_request"}, ++ {"status": "invalid_token", "openid-configuration": ""}, + pq3.types.AuthnRequest, + dict(type=pq3.authn.SASL, body=[b"OAUTHBEARER", b""]), + "duplicate SASL authentication request", @@ src/test/python/client/test_oauth.py (new) + ), + ], +) -+def test_oauth_server_error(accept, sasl_err, resp_type, resp_payload, expected_error): ++def test_oauth_server_error( ++ accept, auth_data_cb, sasl_err, resp_type, resp_payload, expected_error ++): ++ wkuri = f"https://256.256.256.256/.well-known/openid-configuration" + sock, client = accept( -+ oauth_issuer="https://example.com", ++ oauth_issuer=wkuri, + oauth_client_id="some-id", + ) + ++ def bearer_hook(typ, pgconn, request): ++ """ ++ Implementation of the PQAuthDataHook, which returns a token directly so ++ we don't need an openid_provider instance. ++ """ ++ assert typ == PQAUTHDATA_OAUTH_BEARER_TOKEN ++ request.token = secrets.token_urlsafe().encode() ++ return 1 ++ ++ auth_data_cb.impl = bearer_hook ++ + with sock: + with pq3.wrap(sock, debug_stream=sys.stdout) as conn: + start_oauth_handshake(conn) + + # Ignore the client data. Return an error "challenge". ++ if "openid-configuration" in sasl_err: ++ sasl_err["openid-configuration"] = wkuri ++ + resp = json.dumps(sasl_err) + resp = resp.encode("utf-8") + @@ src/test/python/client/test_oauth.py (new) + assert pkt.type == pq3.types.PasswordMessage + assert pkt.payload == b"\x01" + -+ # Now fail the SASL exchange (in either a valid way, or an invalid -+ # one, depending on the test). ++ # Now fail the SASL exchange (in either a valid way, or an ++ # invalid one, depending on the test). + pq3.send(conn, resp_type, **resp_payload) + + with pytest.raises(psycopg2.OperationalError, match=expected_error): @@ src/test/python/client/test_oauth.py (new) + "token_endpoint", "POST", "/token", token_endpoint + ) + -+ expect_disconnected_handshake(sock) ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) + + expected_error = "slow_down interval overflow" + with pytest.raises(psycopg2.OperationalError, match=expected_error): @@ src/test/python/client/test_oauth.py (new) + # No provider callbacks necessary; we should fail immediately. + + with sock: -+ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: -+ initial = start_oauth_handshake(conn) -+ -+ resp = { -+ "status": "invalid_token", -+ "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"), -+ ) -+ -+ # FIXME: the client disconnects at this point; it'd be nicer if -+ # it completed the exchange. ++ handle_discovery_connection(sock, to_http(openid_provider.discovery_uri)) + + expected_error = r'OAuth discovery URI ".*" must use HTTPS' + with pytest.raises(psycopg2.OperationalError, match=expected_error): ++ client.check_completed() ++ ++ ++@pytest.mark.parametrize("auth_type", [pq3.authn.OK, pq3.authn.SASLFinal]) ++def test_discovery_incorrectly_permits_connection(accept, auth_type): ++ """ ++ Incorrectly responds to a client's discovery request with AuthenticationOK ++ or AuthenticationSASLFinal. require_auth=oauth should catch the former, and ++ the mechanism itself should catch the latter. ++ """ ++ issuer = "https://256.256.256.256" ++ sock, client = accept( ++ oauth_issuer=issuer, ++ oauth_client_id=secrets.token_hex(), ++ require_auth="oauth", ++ ) ++ ++ with sock: ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ initial = start_oauth_handshake(conn) ++ ++ auth = get_auth_value(initial) ++ assert auth == b"" ++ ++ # Incorrectly log the client in. It should immediately disconnect. ++ pq3.send(conn, pq3.types.AuthnRequest, type=auth_type) ++ assert not conn.read(1), "client sent unexpected data" ++ ++ if auth_type == pq3.authn.OK: ++ expected_error = "server did not complete authentication" ++ else: ++ expected_error = "server sent unexpected additional OAuth data" ++ ++ with pytest.raises(psycopg2.OperationalError, match=expected_error): ++ client.check_completed() ++ ++ ++def test_no_discovery_url_provided(accept): ++ """ ++ Tests what happens when the client doesn't know who to contact and the ++ server doesn't tell it. ++ """ ++ issuer = "https://256.256.256.256" ++ sock, client = accept( ++ oauth_issuer=issuer, ++ oauth_client_id=secrets.token_hex(), ++ ) ++ ++ with sock: ++ handle_discovery_connection(sock, discovery=None) ++ ++ expected_error = "no discovery metadata was provided" ++ with pytest.raises(psycopg2.OperationalError, match=expected_error): ++ client.check_completed() ++ ++ ++@pytest.mark.parametrize("change_between_connections", [False, True]) ++def test_discovery_url_changes(accept, openid_provider, change_between_connections): ++ """ ++ Ensures that the client complains if the server agrees on the issuer, but ++ disagrees on the discovery URL to be used. ++ """ ++ ++ # Set up our provider callbacks. ++ # NOTE that these callbacks will be called on a background thread. Don't do ++ # any unprotected state mutation here. ++ ++ def authorization_endpoint(headers, params): ++ resp = { ++ "device_code": "DEV", ++ "user_code": "USER", ++ "interval": 0, ++ "verification_uri": "https://example.org", ++ "expires_in": 5, ++ } ++ ++ return 200, resp ++ ++ openid_provider.register_endpoint( ++ "device_authorization_endpoint", "POST", "/device", authorization_endpoint ++ ) ++ ++ def token_endpoint(headers, params): ++ resp = { ++ "access_token": secrets.token_urlsafe(), ++ "token_type": "bearer", ++ } ++ ++ return 200, resp ++ ++ openid_provider.register_endpoint( ++ "token_endpoint", "POST", "/token", token_endpoint ++ ) ++ ++ # Have the client connect. ++ sock, client = accept( ++ oauth_issuer=openid_provider.discovery_uri, ++ oauth_client_id="some-id", ++ ) ++ ++ other_wkuri = f"{openid_provider.issuer}/.well-known/oauth-authorization-server" ++ ++ if not change_between_connections: ++ # Immediately respond with the wrong URL. ++ with sock: ++ handle_discovery_connection(sock, other_wkuri) ++ ++ else: ++ # First connection; use the right URL to begin with. ++ with sock: ++ handle_discovery_connection(sock, openid_provider.discovery_uri) ++ ++ # Second connection. Reject the token and switch the URL. ++ sock, _ = accept() ++ with sock: ++ with pq3.wrap(sock, debug_stream=sys.stdout) as conn: ++ initial = start_oauth_handshake(conn) ++ get_auth_value(initial) ++ ++ # Ignore the token; fail with a different discovery URL. ++ resp = { ++ "status": "invalid_token", ++ "openid-configuration": other_wkuri, ++ } ++ fail_oauth_handshake(conn, resp) ++ ++ expected_error = rf"server's discovery document has moved to {other_wkuri} $previous location was {openid_provider.discovery_uri}$" ++ with pytest.raises(psycopg2.OperationalError, match=expected_error): + client.check_completed() ## src/test/python/conftest.py (new) ##