1:  f2ba496e26 ! 1:  dc4f869365 Add OAUTHBEARER SASL mechanism
    @@ doc/src/sgml/oauth-validators.sgml (new)
     +<!-- doc/src/sgml/oauth-validators.sgml -->
     +
     +<chapter id="oauth-validators">
    -+ <title>Implementing OAuth Validator Modules</title>
    ++ <title>OAuth Validator Modules</title>
     + <indexterm zone="oauth-validators">
     +  <primary>OAuth Validators</primary>
     + </indexterm>
     + <para>
     +  <productname>PostgreSQL</productname> provides infrastructure for creating
    -+  custom modules to perform server-side validation of OAuth tokens.
    ++  custom modules to perform server-side validation of OAuth bearer tokens.
     + </para>
     + <para>
     +  OAuth validation modules must at least consist of an initialization function
    @@ doc/src/sgml/oauth-validators.sgml (new)
     +  <sect2 id="oauth-validator-callback-validate">
     +   <title>Validate Callback</title>
     +   <para>
    ++    The <function>validate_cb</function> callback is executed during the OAuth
    ++    exchange when a user attempts to authenticate using OAuth.  Any state set in
    ++    previous calls will be available in <structfield>state->private_data</structfield>.
    ++
     +<programlisting>
     +typedef ValidatorModuleResult *(*ValidatorValidateCB) (ValidatorModuleState *state, const char *token, const char *role);
     +</programlisting>
    ++
    ++    <replaceable>token</replaceable> will contain the bearer token to validate.
    ++    The server has ensured that the token is well-formed syntactically, but no
    ++    other validation has been performed.  <replaceable>role</replaceable> will
    ++    contain the role the user has requested to log in as.  The callback must
    ++    return a palloc'd <literal>ValidatorModuleResult</literal> struct, which is
    ++    defined as below:
    ++
    ++<programlisting>
    ++typedef struct ValidatorModuleResult
    ++{
    ++    bool        authorized;
    ++    char       *authn_id;
    ++} ValidatorModuleResult;
    ++</programlisting>
    ++
    ++    The connection will only proceed if the module sets
    ++    <structfield>authorized</structfield> to <literal>true</literal>.  To
    ++    authenticate the user, the authenticated user name (as determined using the
    ++    token) shall be palloc'd and returned in the <structfield>authn_id</structfield>
    ++    field.  Alternatively, <structfield>authn_id</structfield> may be set to
    ++    NULL if the token is valid but the associated user identity cannot be
    ++    determined.
    ++   </para>
    ++   <para>
    ++    The caller assumes ownership of the returned memory allocation, the
    ++    validator module should not in any way access the memory after it has been
    ++    returned.  A validator may instead return NULL to signal an internal
    ++    error.
    ++   </para>
    ++   <para>
    ++    The behavior after <function>validate_cb</function> returns depends on the
    ++    specific HBA setup.  Normally, the <structfield>authn_id</structfield> user
    ++    name must exactly match the role that the user is logging in as.  (This
    ++    behavior may be modified with a usermap.)  But when authenticating against
    ++    an HBA rule with <literal>trust_validator_authz</literal> turned on, the
    ++    server will not perform any checks on the value of
    ++    <structfield>authn_id</structfield> at all; in this case it is up to the
    ++    validator to ensure that the token carries enough privileges for the user to
    ++    log in under the indicated <replaceable>role</replaceable>.
     +   </para>
     +  </sect2>
     +
    @@ src/backend/libpq/auth-oauth.c (new)
     +oauth_exchange(void *opaq, const char *input, int inputlen,
     +			   char **output, int *outputlen, const char **logdetail)
     +{
    ++	char	   *input_copy;
     +	char	   *p;
     +	char		cbind_flag;
     +	char	   *auth;
    ++	int			status;
     +
     +	struct oauth_ctx *ctx = opaq;
     +
    @@ src/backend/libpq/auth-oauth.c (new)
     +	}
     +
     +	/* Handle the client's initial message. */
    -+	p = pstrdup(input);
    ++	p = input_copy = pstrdup(input);
     +
     +	/*
     +	 * OAUTHBEARER does not currently define a channel binding (so there is no
    @@ src/backend/libpq/auth-oauth.c (new)
     +		generate_error_response(ctx, output, outputlen);
     +
     +		ctx->state = OAUTH_STATE_ERROR;
    -+		return PG_SASL_EXCHANGE_CONTINUE;
    ++		status = PG_SASL_EXCHANGE_CONTINUE;
    ++	}
    ++	else
    ++	{
    ++		ctx->state = OAUTH_STATE_FINISHED;
    ++		status = PG_SASL_EXCHANGE_SUCCESS;
     +	}
     +
    -+	ctx->state = OAUTH_STATE_FINISHED;
    -+	return PG_SASL_EXCHANGE_SUCCESS;
    ++	/* Don't let extra copies of the bearer token hang around. */
    ++	explicit_bzero(input_copy, inputlen);
    ++
    ++	return status;
     +}
     +
     +/*
    @@ src/backend/libpq/auth-oauth.c (new)
     +	int			map_status;
     +	ValidatorModuleResult *ret;
     +	const char *token;
    ++	bool		status;
     +
     +	/* Ensure that we have a correct token to validate */
     +	if (!(token = validate_token_format(auth)))
    @@ src/backend/libpq/auth-oauth.c (new)
     +	/* Call the validation function from the validator module */
     +	ret = ValidatorCallbacks->validate_cb(validator_module_state,
     +										  token, port->user_name);
    ++	if (ret == NULL)
    ++	{
    ++		ereport(LOG, errmsg("Internal error in OAuth validator module"));
    ++		return false;
    ++	}
     +
     +	if (!ret->authorized)
    -+		return false;
    ++	{
    ++		status = false;
    ++		goto cleanup;
    ++	}
     +
     +	if (ret->authn_id)
     +		set_authn_id(port, ret->authn_id);
    @@ src/backend/libpq/auth-oauth.c (new)
     +		 * validator implementation; all that matters is that the validator
     +		 * says the user can log in with the target role.
     +		 */
    -+		return true;
    ++		status = true;
    ++		goto cleanup;
     +	}
     +
     +	/* Make sure the validator authenticated the user. */
    @@ src/backend/libpq/auth-oauth.c (new)
     +				errmsg("OAuth bearer authentication failed for user \"%s\"",
     +					   port->user_name),
     +				errdetail_log("Validator provided no identity."));
    -+		return false;
    ++
    ++		status = false;
    ++		goto cleanup;
     +	}
     +
     +	/* Finally, check the user map. */
     +	map_status = check_usermap(port->hba->usermap, port->user_name,
     +							   MyClientConnectionInfo.authn_id, false);
    -+	return (map_status == STATUS_OK);
    ++	status = (map_status == STATUS_OK);
    ++
    ++cleanup:
    ++
    ++	/*
    ++	 * Clear and free the validation result from the validator module once
    ++	 * we're done with it.
    ++	 */
    ++	if (ret->authn_id != NULL)
    ++		pfree(ret->authn_id);
    ++	pfree(ret);
    ++
    ++	return status;
     +}
     +
     +/*
    @@ src/backend/libpq/hba.c: parse_hba_line(TokenizedAuthLine *tok_line, int elevel)
     +		MANDATORY_AUTH_ARG(parsedline->oauth_issuer, "issuer", "oauth");
     +
     +		/*
    -+		 * Supplying a usermap combined with the option to skip usermapping
    -+		 * is nonsensical and indicates a configuration error.
    ++		 * Supplying a usermap combined with the option to skip usermapping is
    ++		 * nonsensical and indicates a configuration error.
     +		 */
     +		if (parsedline->oauth_skip_usermap && parsedline->usermap != NULL)
     +		{
     +			ereport(elevel,
     +					errcode(ERRCODE_CONFIG_FILE_ERROR),
    -+					/* translator: strings are replaced with hba options */
    ++			/* translator: strings are replaced with hba options */
     +					errmsg("%s cannot be used in combination with %s",
     +						   "map", "trust_validator_authz"),
     +					errcontext("line %d of configuration file \"%s\"",
    @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
     +}
     +
     +/*
    ++ * URL-Encoding Helpers
    ++ */
    ++
    ++/*
    ++ * Encodes a string using the application/x-www-form-urlencoded format, and
    ++ * appends it to the given buffer.
    ++ */
    ++static void
    ++append_urlencoded(PQExpBuffer buf, const char *s)
    ++{
    ++	char	   *escaped;
    ++	char	   *haystack;
    ++	char	   *match;
    ++
    ++	escaped = curl_easy_escape(NULL, s, 0);
    ++	if (!escaped)
    ++	{
    ++		markPQExpBufferBroken(buf);
    ++		return;
    ++	}
    ++
    ++	/*
    ++	 * curl_easy_escape() almost does what we want, but we need the
    ++	 * query-specific flavor which uses '+' instead of '%20' for spaces. The
    ++	 * Curl command-line tool does this with a simple search-and-replace, so
    ++	 * follow its lead.
    ++	 */
    ++	haystack = escaped;
    ++
    ++	while ((match = strstr(haystack, "%20")) != NULL)
    ++	{
    ++		/* Append the unmatched portion, followed by the plus sign. */
    ++		appendBinaryPQExpBuffer(buf, haystack, match - haystack);
    ++		appendPQExpBufferChar(buf, '+');
    ++
    ++		/* Keep searching after the match. */
    ++		haystack = match + 3 /* strlen("%20") */ ;
    ++	}
    ++
    ++	/* Push the remainder of the string onto the buffer. */
    ++	appendPQExpBufferStr(buf, haystack);
    ++
    ++	curl_free(escaped);
    ++}
    ++
    ++/*
    ++ * Convenience wrapper for encoding a single string. Returns NULL on allocation
    ++ * failure.
    ++ */
    ++static char *
    ++urlencode(const char *s)
    ++{
    ++	PQExpBufferData buf;
    ++
    ++	initPQExpBuffer(&buf);
    ++	append_urlencoded(&buf, s);
    ++
    ++	return PQExpBufferDataBroken(buf) ? NULL : buf.data;
    ++}
    ++
    ++/*
    ++ * Appends a key/value pair to the end of an application/x-www-form-urlencoded
    ++ * list.
    ++ */
    ++static void
    ++build_urlencoded(PQExpBuffer buf, const char *key, const char *value)
    ++{
    ++	if (buf->len)
    ++		appendPQExpBufferChar(buf, '&');
    ++
    ++	append_urlencoded(buf, key);
    ++	appendPQExpBufferChar(buf, '=');
    ++	append_urlencoded(buf, value);
    ++}
    ++
    ++/*
     + * Specific HTTP Request Handlers
     + *
     + * This is finally the beginning of the actual application logic. Generally
    @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
     +}
     +
     +/*
    ++ * Adds the client ID (and secret, if provided) to the current request, using
    ++ * either HTTP headers or the request body.
    ++ */
    ++static bool
    ++add_client_identification(struct async_ctx *actx, PQExpBuffer reqbody, PGconn *conn)
    ++{
    ++	bool		success = false;
    ++	char	   *username = NULL;
    ++	char	   *password = NULL;
    ++
    ++	if (conn->oauth_client_secret)	/* Zero-length secrets are permitted! */
    ++	{
    ++		/*----
    ++		 * Use HTTP Basic auth to send the client_id and secret. Per RFC 6749,
    ++		 * Sec. 2.3.1,
    ++		 *
    ++		 *   Including the client credentials in the request-body using the
    ++		 *   two parameters is NOT RECOMMENDED and SHOULD be limited to
    ++		 *   clients unable to directly utilize the HTTP Basic authentication
    ++		 *   scheme (or other password-based HTTP authentication schemes).
    ++		 *
    ++		 * Additionally:
    ++		 *
    ++		 *   The client identifier is encoded using the
    ++		 *   "application/x-www-form-urlencoded" encoding algorithm per Appendix
    ++		 *   B, and the encoded value is used as the username; the client
    ++		 *   password is encoded using the same algorithm and used as the
    ++		 *   password.
    ++		 *
    ++		 * (Appendix B modifies application/x-www-form-urlencoded by requiring
    ++		 * an initial UTF-8 encoding step. Since the client ID and secret must
    ++		 * both be 7-bit ASCII -- RFC 6749 Appendix A -- we don't worry about
    ++		 * that in this function.)
    ++		 *
    ++		 * client_id is not added to the request body in this case. Not only
    ++		 * would it be redundant, but some providers in the wild (e.g. Okta)
    ++		 * refuse to accept it.
    ++		 */
    ++		username = urlencode(conn->oauth_client_id);
    ++		password = urlencode(conn->oauth_client_secret);
    ++
    ++		if (!username || !password)
    ++		{
    ++			actx_error(actx, "out of memory");
    ++			goto cleanup;
    ++		}
    ++
    ++		CHECK_SETOPT(actx, CURLOPT_HTTPAUTH, CURLAUTH_BASIC, goto cleanup);
    ++		CHECK_SETOPT(actx, CURLOPT_USERNAME, username, goto cleanup);
    ++		CHECK_SETOPT(actx, CURLOPT_PASSWORD, password, goto cleanup);
    ++
    ++		actx->used_basic_auth = true;
    ++	}
    ++	else
    ++	{
    ++		/*
    ++		 * If we're not otherwise authenticating, client_id is REQUIRED in the
    ++		 * request body.
    ++		 */
    ++		build_urlencoded(reqbody, "client_id", conn->oauth_client_id);
    ++
    ++		CHECK_SETOPT(actx, CURLOPT_HTTPAUTH, CURLAUTH_NONE, goto cleanup);
    ++		actx->used_basic_auth = false;
    ++	}
    ++
    ++	success = true;
    ++
    ++cleanup:
    ++	free(username);
    ++	free(password);
    ++
    ++	return success;
    ++}
    ++
    ++/*
     + * Queue a Device Authorization Request:
     + *
     + *     https://www.rfc-editor.org/rfc/rfc8628#section-3.1
    @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
     +	Assert(conn->oauth_client_id);	/* ensured by get_auth_token() */
     +	Assert(device_authz_uri);	/* ensured by check_for_device_flow() */
     +
    -+	/* Construct our request body. TODO: url-encode */
    ++	/* Construct our request body. */
     +	resetPQExpBuffer(work_buffer);
    -+	appendPQExpBuffer(work_buffer, "client_id=%s", conn->oauth_client_id);
    -+	if (conn->oauth_scope)
    -+		appendPQExpBuffer(work_buffer, "&scope=%s", conn->oauth_scope);
    ++	if (conn->oauth_scope && conn->oauth_scope[0])
    ++		build_urlencoded(work_buffer, "scope", conn->oauth_scope);
    ++
    ++	if (!add_client_identification(actx, work_buffer, conn))
    ++		return false;
     +
     +	if (PQExpBufferBroken(work_buffer))
     +	{
    @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
     +	CHECK_SETOPT(actx, CURLOPT_URL, device_authz_uri, return false);
     +	CHECK_SETOPT(actx, CURLOPT_COPYPOSTFIELDS, work_buffer->data, return false);
     +
    -+	if (conn->oauth_client_secret)
    -+	{
    -+		/*----
    -+		 * Use HTTP Basic auth to send the password. Per RFC 6749, Sec. 2.3.1,
    -+		 *
    -+		 *   Including the client credentials in the request-body using the
    -+		 *   two parameters is NOT RECOMMENDED and SHOULD be limited to
    -+		 *   clients unable to directly utilize the HTTP Basic authentication
    -+		 *   scheme (or other password-based HTTP authentication schemes).
    -+		 *
    -+		 * TODO: should we omit client_id from the body in this case?
    -+		 * TODO: url-encode...?
    -+		 */
    -+		CHECK_SETOPT(actx, CURLOPT_HTTPAUTH, CURLAUTH_BASIC, return false);
    -+		CHECK_SETOPT(actx, CURLOPT_USERNAME, conn->oauth_client_id, return false);
    -+		CHECK_SETOPT(actx, CURLOPT_PASSWORD, conn->oauth_client_secret, return false);
    -+
    -+		actx->used_basic_auth = true;
    -+	}
    -+	else
    -+	{
    -+		CHECK_SETOPT(actx, CURLOPT_HTTPAUTH, CURLAUTH_NONE, return false);
    -+		actx->used_basic_auth = false;
    -+	}
    -+
     +	return start_request(actx);
     +}
     +
    @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
     +	Assert(token_uri);			/* ensured by get_discovery_document() */
     +	Assert(device_code);		/* ensured by run_device_authz() */
     +
    -+	/* Construct our request body. TODO: url-encode */
    ++	/* Construct our request body. */
     +	resetPQExpBuffer(work_buffer);
    -+	appendPQExpBuffer(work_buffer, "client_id=%s", conn->oauth_client_id);
    -+	appendPQExpBuffer(work_buffer, "&device_code=%s", device_code);
    -+	appendPQExpBuffer(work_buffer, "&grant_type=%s",
    -+					  OAUTH_GRANT_TYPE_DEVICE_CODE);
    -+	/* TODO check for broken buffer */
    -+
    -+	/* Make our request. */
    -+	CHECK_SETOPT(actx, CURLOPT_URL, token_uri, return false);
    -+	CHECK_SETOPT(actx, CURLOPT_COPYPOSTFIELDS, work_buffer->data, return false);
    ++	build_urlencoded(work_buffer, "device_code", device_code);
    ++	build_urlencoded(work_buffer, "grant_type", OAUTH_GRANT_TYPE_DEVICE_CODE);
     +
    -+	if (conn->oauth_client_secret)
    -+	{
    -+		/*----
    -+		 * Use HTTP Basic auth to send the password. Per RFC 6749, Sec. 2.3.1,
    -+		 *
    -+		 *   Including the client credentials in the request-body using the
    -+		 *   two parameters is NOT RECOMMENDED and SHOULD be limited to
    -+		 *   clients unable to directly utilize the HTTP Basic authentication
    -+		 *   scheme (or other password-based HTTP authentication schemes).
    -+		 *
    -+		 * TODO: should we omit client_id from the body in this case?
    -+		 * TODO: url-encode...?
    -+		 */
    -+		CHECK_SETOPT(actx, CURLOPT_HTTPAUTH, CURLAUTH_BASIC, return false);
    -+		CHECK_SETOPT(actx, CURLOPT_USERNAME, conn->oauth_client_id, return false);
    -+		CHECK_SETOPT(actx, CURLOPT_PASSWORD, conn->oauth_client_secret, return false);
    ++	if (!add_client_identification(actx, work_buffer, conn))
    ++		return false;
     +
    -+		actx->used_basic_auth = true;
    -+	}
    -+	else
    ++	if (PQExpBufferBroken(work_buffer))
     +	{
    -+		CHECK_SETOPT(actx, CURLOPT_HTTPAUTH, CURLAUTH_NONE, return false);
    -+		actx->used_basic_auth = false;
    ++		actx_error(actx, "out of memory");
    ++		return false;
     +	}
     +
    -+	resetPQExpBuffer(work_buffer);
    -+	CHECK_SETOPT(actx, CURLOPT_WRITEFUNCTION, append_data, return false);
    -+	CHECK_SETOPT(actx, CURLOPT_WRITEDATA, actx, return false);
    ++	/* Make our request. */
    ++	CHECK_SETOPT(actx, CURLOPT_URL, token_uri, return false);
    ++	CHECK_SETOPT(actx, CURLOPT_COPYPOSTFIELDS, work_buffer->data, return false);
     +
     +	return start_request(actx);
     +}
    @@ src/interfaces/libpq/meson.build: if gssapi.found()
        kwargs: gen_export_kwargs,
      )
     
    + ## src/interfaces/libpq/pqexpbuffer.c ##
    +@@ src/interfaces/libpq/pqexpbuffer.c: static const char *const oom_buffer_ptr = oom_buffer;
    +  *
    +  * Put a PQExpBuffer in "broken" state if it isn't already.
    +  */
    +-static void
    ++void
    + markPQExpBufferBroken(PQExpBuffer str)
    + {
    + 	if (str->data != oom_buffer)
    +
    + ## src/interfaces/libpq/pqexpbuffer.h ##
    +@@ src/interfaces/libpq/pqexpbuffer.h: extern void initPQExpBuffer(PQExpBuffer str);
    + extern void destroyPQExpBuffer(PQExpBuffer str);
    + extern void termPQExpBuffer(PQExpBuffer str);
    + 
    ++/*------------------------
    ++ * markPQExpBufferBroken
    ++ *		Put a PQExpBuffer in "broken" state if it isn't already.
    ++ */
    ++extern void markPQExpBufferBroken(PQExpBuffer str);
    ++
    + /*------------------------
    +  * resetPQExpBuffer
    +  *		Reset a PQExpBuffer to empty
    +
      ## src/makefiles/meson.build ##
     @@ src/makefiles/meson.build: pgxs_deps = {
        'llvm': llvm,
    @@ src/test/modules/oauth_validator/t/001_server.pl (new)
     +	$log_start = $log_end;
     +}
     +
    ++# 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
    ++# single-quoted connection string.
    ++my $vschars =
    ++  " !\"#\$%&'()*+,-./0123456789:;<=>?\@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
    ++my $vschars_esc =
    ++  " !\"#\$%&\\'()*+,-./0123456789:;<=>?\@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
    ++
    ++$node->connect_ok(
    ++	"user=$user dbname=postgres 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'",
    ++	"escapable characters: client_id and secret",
    ++	expected_stderr =>
    ++	  qr@Visit https://example\.org/ and enter the code: postgresuser@);
    ++
     +#
     +# Further tests rely on support for specific behaviors in oauth_server.py. To
     +# trigger these behaviors, we ask for the special issuer .../param (which is set
    @@ src/test/modules/oauth_validator/t/001_server.pl (new)
     +#
     +
     +my $common_connstr = "user=testparam dbname=postgres ";
    ++my $base_connstr = $common_connstr;
     +
     +sub connstr
     +{
    @@ src/test/modules/oauth_validator/t/001_server.pl (new)
     +	my $json = encode_json(\%params);
     +	my $encoded = encode_base64($json, "");
     +
    -+	return "$common_connstr oauth_client_id=$encoded";
    ++	return "$base_connstr oauth_client_id=$encoded";
     +}
     +
     +# Make sure the param system works end-to-end first.
    @@ src/test/modules/oauth_validator/t/001_server.pl (new)
     +	expected_stderr => qr/bearer authentication failed/);
     +
     +# Test behavior of the oauth_client_secret.
    -+$common_connstr = "$common_connstr oauth_client_secret=12345";
    ++$base_connstr = "$common_connstr oauth_client_secret=''";
    ++
    ++$node->connect_ok(
    ++	connstr(stage => 'all', expected_secret => ''),
    ++	"empty oauth_client_secret",
    ++	expected_stderr =>
    ++	  qr@Visit https://example\.com/ and enter the code: postgresuser@);
    ++
    ++$base_connstr = "$common_connstr oauth_client_secret='$vschars_esc'";
     +
     +$node->connect_ok(
    -+	connstr(stage => 'all', expected_secret => '12345'),
    -+	"oauth_client_secret",
    ++	connstr(stage => 'all', expected_secret => $vschars),
    ++	"nonempty oauth_client_secret",
     +	expected_stderr =>
     +	  qr@Visit https://example\.com/ and enter the code: postgresuser@);
     +
    @@ 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.
    -+$common_connstr = "user=test dbname=postgres oauth_issuer=$issuer";
    ++# things up; hardcode the issuer. (Scope is hardcoded to empty to cover that
    ++# case as well.)
    ++$common_connstr =
    ++  "user=test dbname=postgres oauth_issuer=$issuer oauth_scope=''";
     +
     +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.authn_id TO ''");
     +$node->reload;
    @@ src/test/modules/oauth_validator/t/oauth_server.py (new)
     +        if secret is None:
     +            return
     +
    -+        if "Authorization" not in self.headers:
    -+            raise RuntimeError("client did not send Authorization header")
    -+
    ++        assert "Authorization" in self.headers
     +        method, creds = self.headers["Authorization"].split()
     +
     +        if method != "Basic":
     +            raise RuntimeError(f"client used {method} auth; expected Basic")
     +
    -+        expected_creds = f"{self.client_id}:{secret}"
    ++        username = urllib.parse.quote_plus(self.client_id)
    ++        password = urllib.parse.quote_plus(secret)
    ++        expected_creds = f"{username}:{password}"
    ++
     +        if creds.encode() != base64.b64encode(expected_creds.encode()):
     +            raise RuntimeError(
     +                f"client sent '{creds}'; expected b64encode('{expected_creds}')"
    @@ src/test/modules/oauth_validator/t/oauth_server.py (new)
     +        form = self.rfile.read(size)
     +
     +        assert self.headers["Content-Type"] == "application/x-www-form-urlencoded"
    -+        return urllib.parse.parse_qs(form.decode("utf-8"), strict_parsing=True)
    ++        return urllib.parse.parse_qs(
    ++            form.decode("utf-8"),
    ++            strict_parsing=True,
    ++            keep_blank_values=True,
    ++            encoding="utf-8",
    ++            errors="strict",
    ++        )
     +
     +    @property
     +    def client_id(self) -> str:
     +        """
    -+        Returns the client_id sent in the POST body. self._parse_params() must
    -+        have been called first.
    ++        Returns the client_id sent in the POST body or the Authorization header.
    ++        self._parse_params() must have been called first.
     +        """
    -+        return self._params["client_id"][0]
    ++        if "client_id" in self._params:
    ++            return self._params["client_id"][0]
    ++
    ++        if "Authorization" not in self.headers:
    ++            raise RuntimeError("client did not send any client_id")
    ++
    ++        _, creds = self.headers["Authorization"].split()
    ++
    ++        decoded = base64.b64decode(creds).decode("utf-8")
    ++        username, _ = decoded.split(":", 1)
    ++
    ++        return urllib.parse.unquote_plus(username)
     +
     +    def do_POST(self):
     +        self._response_code = 200
    @@ src/test/modules/oauth_validator/t/oauth_server.py (new)
     +        else:
     +            self._token_state.min_delay = 5  # default
     +
    ++        # Check the scope.
    ++        if "scope" in self._params:
    ++            assert self._params["scope"][0], "empty scopes should be omitted"
    ++
     +        return resp
     +
     +    def token(self) -> JsonObject:
2:  55068dfb46 < -:  ---------- v30-review-comments
3:  c01dbdd6cb ! 2:  cca5de6726 DO NOT MERGE: Add pytest suite for OAuth
    @@ src/test/python/client/test_oauth.py (new)
     +            assert self.headers["Content-Type"] == "application/x-www-form-urlencoded"
     +
     +            body = self._request_body()
    -+            params = urllib.parse.parse_qs(body)
    ++            if body:
    ++                # parse_qs() is understandably fairly lax when it comes to
    ++                # acceptable characters, but we're stricter. Spaces must be
    ++                # encoded, and they must use the '+' encoding rather than "%20".
    ++                assert " " not in body
    ++                assert "%20" not in body
    ++
    ++                params = urllib.parse.parse_qs(
    ++                    body,
    ++                    keep_blank_values=True,
    ++                    strict_parsing=True,
    ++                    encoding="utf-8",
    ++                    errors="strict",
    ++                )
    ++            else:
    ++                params = {}
     +
     +            self._handle(params=params)
     +
    @@ src/test/python/client/test_oauth.py (new)
     +    access_token = secrets.token_urlsafe()
     +
     +    def check_client_authn(headers, params):
    -+        if not secret:
    ++        if secret is None:
    ++            assert "Authorization" not in headers
     +            assert params["client_id"] == [client_id]
     +            return
     +
     +        # Require the client to use Basic authn; request-body credentials are
     +        # NOT RECOMMENDED (RFC 6749, Sec. 2.3.1).
     +        assert "Authorization" in headers
    ++        assert "client_id" not in params
     +
     +        method, creds = headers["Authorization"].split()
     +        assert method == "Basic"
    @@ src/test/python/client/test_oauth.py (new)
     +        client.check_completed()
     +
     +
    ++# See https://datatracker.ietf.org/doc/html/rfc6749#appendix-A for character
    ++# class definitions.
    ++all_vschars = "".join([chr(c) for c in range(0x20, 0x7F)])
    ++all_nqchars = "".join([chr(c) for c in range(0x21, 0x7F) if c not in (0x22, 0x5C)])
    ++
    ++
    ++@pytest.mark.parametrize("client_id", ["", ":", " + ", r'+=&"\/~', all_vschars])
    ++@pytest.mark.parametrize("secret", [None, "", ":", " + ", r'+=&"\/~', all_vschars])
    ++@pytest.mark.parametrize("device_code", ["", " + ", r'+=&"\/~', all_vschars])
    ++@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_client_id=client_id,
    ++        oauth_client_secret=secret,
    ++        oauth_scope=scope,
    ++    )
    ++
    ++    user_code = f"{secrets.token_hex(2)}-{secrets.token_hex(2)}"
    ++    verification_url = "https://example.com/device"
    ++
    ++    access_token = secrets.token_urlsafe()
    ++
    ++    def check_client_authn(headers, params):
    ++        if secret is None:
    ++            assert "Authorization" not in headers
    ++            assert params["client_id"] == [client_id]
    ++            return
    ++
    ++        # Require the client to use Basic authn; request-body credentials are
    ++        # NOT RECOMMENDED (RFC 6749, Sec. 2.3.1).
    ++        assert "Authorization" in headers
    ++        assert "client_id" not in params
    ++
    ++        method, creds = headers["Authorization"].split()
    ++        assert method == "Basic"
    ++
    ++        decoded = base64.b64decode(creds).decode("utf-8")
    ++        username, password = decoded.split(":", 1)
    ++
    ++        expected_username = urllib.parse.quote_plus(client_id)
    ++        expected_password = urllib.parse.quote_plus(secret)
    ++
    ++        assert [username, password] == [expected_username, expected_password]
    ++
    ++    # 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):
    ++        check_client_authn(headers, params)
    ++
    ++        if scope:
    ++            assert params["scope"] == [scope]
    ++        else:
    ++            assert "scope" not in params
    ++
    ++        resp = {
    ++            "device_code": device_code,
    ++            "user_code": user_code,
    ++            "interval": 0,
    ++            "verification_url": verification_url,
    ++            "expires_in": 5,
    ++        }
    ++
    ++        return 200, resp
    ++
    ++    openid_provider.register_endpoint(
    ++        "device_authorization_endpoint", "POST", "/device", authorization_endpoint
    ++    )
    ++
    ++    def token_endpoint(headers, params):
    ++        check_client_authn(headers, params)
    ++
    ++        assert params["grant_type"] == ["urn:ietf:params:oauth:grant-type:device_code"]
    ++        assert params["device_code"] == [device_code]
    ++
    ++        # 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
    ++    )
    ++
    ++    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)
    ++
    ++
     +@pytest.mark.slow
     +@pytest.mark.parametrize("error_code", ["authorization_pending", "slow_down"])
     +@pytest.mark.parametrize("retries", [1, 2])