since-v34.diff.txt
text/plain
Filename: since-v34.diff.txt
Type: text/plain
Part: 0
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])