since-v30.diff.txt
text/plain
Filename: since-v30.diff.txt
Type: text/plain
Part: 0
1: a2b4d75bf8 = 1: 696b0b2af4 Make SASL max message length configurable
2: e339e7844e ! 2: 80afe8b107 libpq: add OAUTHBEARER SASL mechanism
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+ int running; /* is asynchronous work in progress? */
+ bool user_prompted; /* have we already sent the authz prompt? */
++ bool used_basic_auth; /* did we send a client secret? */
+ bool debugging; /* can we give unsafe developer assistance? */
+};
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ return result;
+}
+
++/*
++ * Constructs a message from the token error response and puts it into
++ * actx->errbuf.
++ */
++static void
++record_token_error(struct async_ctx *actx, const struct token_error *err)
++{
++ if (err->error_description)
++ appendPQExpBuffer(&actx->errbuf, "%s ", err->error_description);
++ else
++ {
++ /*
++ * Try to get some more helpful detail into the error string. A 401
++ * status in particular implies that the oauth_client_secret is
++ * missing or wrong.
++ */
++ long response_code;
++
++ CHECK_GETINFO(actx, CURLINFO_RESPONSE_CODE, &response_code, response_code = 0);
++
++ if (response_code == 401)
++ {
++ actx_error(actx, actx->used_basic_auth
++ ? "provider rejected the oauth_client_secret"
++ : "provider requires client authentication, and no oauth_client_secret is set");
++ actx_error_str(actx, " ");
++ }
++ }
++
++ appendPQExpBuffer(&actx->errbuf, "(%s)", err->error);
++}
++
+static bool
+parse_access_token(struct async_ctx *actx, struct token *tok)
+{
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * 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)
+ CHECK_GETINFO(actx, CURLINFO_RESPONSE_CODE, &response_code, return false);
+
+ /*
-+ * The device authorization endpoint uses the same error response as the
-+ * token endpoint, so we have to handle 400/401 here too.
++ * Per RFC 8628, Section 3, a successful device authorization response
++ * uses 200 OK.
+ */
-+ if (response_code != 200
-+ && response_code != 400
-+ /* && response_code != 401 TODO */ )
++ if (response_code == 200)
+ {
-+ actx_error(actx, "unexpected response code %ld", response_code);
-+ return false;
++ actx->errctx = "failed to parse device authorization";
++ if (!parse_device_authz(actx, &actx->authz))
++ return false; /* error message already set */
++
++ return true;
+ }
+
-+ if (response_code != 200)
++ /*
++ * The device authorization endpoint uses the same error response as the
++ * token endpoint, so the error handling roughly follows
++ * finish_token_request(). The key difference is that an error here is
++ * immediately fatal.
++ */
++ if (response_code == 400 || response_code == 401)
+ {
+ struct token_error err = {0};
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ return false;
+ }
+
-+ if (err.error_description)
-+ appendPQExpBuffer(&actx->errbuf, "%s ", err.error_description);
-+
-+ appendPQExpBuffer(&actx->errbuf, "(%s)", err.error);
++ record_token_error(actx, &err);
+
+ free_token_error(&err);
+ return false;
+ }
+
-+ /*
-+ * Pull the fields we care about from the document.
-+ */
-+ actx->errctx = "failed to parse device authorization";
-+ if (!parse_device_authz(actx, &actx->authz))
-+ return false; /* error message already set */
-+
-+ return true;
++ /* Any other response codes are considered invalid */
++ actx_error(actx, "unexpected response code %ld", response_code);
++ return false;
+}
+
+/*
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * 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;
++ }
+
+ resetPQExpBuffer(work_buffer);
+ CHECK_SETOPT(actx, CURLOPT_WRITEFUNCTION, append_data, return false);
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ CHECK_GETINFO(actx, CURLINFO_RESPONSE_CODE, &response_code, return false);
+
+ /*
-+ * Per RFC 6749, Section 5, a successful response uses 200 OK. An error
-+ * response uses either 400 Bad Request or 401 Unauthorized.
-+ *
-+ * TODO: there are references online to 403 appearing in the wild...
++ * Per RFC 6749, Section 5, a successful response uses 200 OK.
+ */
-+ if (response_code != 200
-+ && response_code != 400
-+ /* && response_code != 401 TODO */ )
++ if (response_code == 200)
+ {
-+ actx_error(actx, "unexpected response code %ld", response_code);
-+ return false;
++ actx->errctx = "failed to parse access token response";
++ if (!parse_access_token(actx, tok))
++ return false; /* error message already set */
++
++ return true;
+ }
+
+ /*
-+ * Pull the fields we care about from the document.
++ * An error response uses either 400 Bad Request or 401 Unauthorized.
++ * There are references online to implementations using 403 for error
++ * return which would violate the specification. For now we stick to the
++ * specification but we might have to revisit this.
+ */
-+ if (response_code == 200)
++ if (response_code == 400 || response_code == 401)
+ {
-+ actx->errctx = "failed to parse access token response";
-+ if (!parse_access_token(actx, tok))
-+ return false; /* error message already set */
++ if (!parse_token_error(actx, &tok->err))
++ return false;
++
++ return true;
+ }
-+ else if (!parse_token_error(actx, &tok->err))
-+ return false;
+
-+ return true;
++ /* Any other response codes are considered invalid */
++ actx_error(actx, "unexpected response code %ld", response_code);
++ return false;
+}
+
+/*
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ if (strcmp(err->error, "authorization_pending") != 0 &&
+ strcmp(err->error, "slow_down") != 0)
+ {
-+ if (err->error_description)
-+ appendPQExpBuffer(&actx->errbuf, "%s ", err->error_description);
-+
-+ appendPQExpBuffer(&actx->errbuf, "(%s)", err->error);
++ record_token_error(actx, err);
+ goto token_cleanup;
+ }
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+ if (!res)
+ {
-+ fprintf(stderr, "Visit %s and enter the code: %s",
++ /*
++ * translator: The first %s is a URL for the user to visit in a
++ * browser, and the second %s is a code to be copy-pasted there.
++ */
++ fprintf(stderr, libpq_gettext("Visit %s and enter the code: %s\n"),
+ prompt.verification_uri, prompt.user_code);
+ }
+ else if (res < 0)
3: 1a9422911f ! 3: 66505336d6 backend: add OAUTHBEARER SASL mechanism
@@ src/test/modules/oauth_validator/t/001_server.pl (new)
+);
+
+$node->connect_fails(
++ connstr(stage => 'token', error_code => "invalid_grant"),
++ "bad token response: invalid_grant, no description",
++ expected_stderr => qr/failed to obtain access token: \(invalid_grant\)/);
++$node->connect_fails(
++ connstr(
++ stage => 'token',
++ error_code => "invalid_grant",
++ error_desc => "grant expired"),
++ "bad token response: expired grant",
++ expected_stderr =>
++ qr/failed to obtain access token: grant expired \(invalid_grant\)/);
++$node->connect_fails(
++ connstr(
++ stage => 'token',
++ error_code => "invalid_client",
++ error_status => 401),
++ "bad token response: client authentication failure, default description",
++ expected_stderr =>
++ qr/failed to obtain access token: provider requires client authentication, and no oauth_client_secret is set \(invalid_client\)/
++);
++$node->connect_fails(
++ connstr(
++ stage => 'token',
++ error_code => "invalid_client",
++ error_status => 401,
++ error_desc => "authn failure"),
++ "bad token response: client authentication failure, provided description",
++ expected_stderr =>
++ qr/failed to obtain access token: authn failure \(invalid_client\)/);
++
++$node->connect_fails(
+ connstr(stage => 'token', token => ""),
+ "server rejects access: empty token",
+ expected_stderr => qr/bearer authentication failed/);
@@ src/test/modules/oauth_validator/t/001_server.pl (new)
+ "server rejects access: invalid token contents",
+ expected_stderr => qr/bearer authentication failed/);
+
++# Test behavior of the oauth_client_secret.
++$common_connstr = "$common_connstr oauth_client_secret=12345";
++
++$node->connect_ok(
++ connstr(stage => 'all', expected_secret => '12345'),
++ "oauth_client_secret",
++ expected_stderr =>
++ qr@Visit https://example\.com/ and enter the code: postgresuser@);
++
++$node->connect_fails(
++ connstr(
++ stage => 'token',
++ error_code => "invalid_client",
++ error_status => 401),
++ "bad token response: client authentication failure, default description with oauth_client_secret",
++ expected_stderr =>
++ qr/failed to obtain access token: provider rejected the oauth_client_secret \(invalid_client\)/
++);
++$node->connect_fails(
++ connstr(
++ stage => 'token',
++ error_code => "invalid_client",
++ error_status => 401,
++ error_desc => "mutual TLS required for client"),
++ "bad token response: client authentication failure, provided description with oauth_client_secret",
++ expected_stderr =>
++ qr/failed to obtain access token: mutual TLS required for client \(invalid_client\)/
++);
++
+#
+# This section of tests reconfigures the validator module itself, rather than
+# the OAuth server.
@@ src/test/modules/oauth_validator/t/oauth_server.py (new)
+ elif self._parameterized:
+ self.path = self.path.removeprefix("/param")
+
++ def _check_authn(self):
++ """
++ Checks the expected value of the Authorization header, if any.
++ """
++ secret = self._get_param("expected_secret", None)
++ if secret is None:
++ return
++
++ if "Authorization" not in self.headers:
++ raise RuntimeError("client did not send Authorization header")
++
++ 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}"
++ if creds.encode() != base64.b64encode(expected_creds.encode()):
++ raise RuntimeError(
++ f"client sent '{creds}'; expected b64encode('{expected_creds}')"
++ )
++
+ def do_GET(self):
+ self._response_code = 200
+ self._check_issuer()
@@ src/test/modules/oauth_validator/t/oauth_server.py (new)
+ js = base64.b64decode(self.client_id)
+ self._test_params = json.loads(js)
+
++ self._check_authn()
++
+ if self.path == "/authorize":
+ resp = self.authorization()
+ elif self.path == "/token":
@@ src/test/modules/oauth_validator/t/oauth_server.py (new)
+ return resp
+
+ def token(self) -> JsonObject:
++ if err := self._get_param("error_code", None):
++ self._response_code = self._get_param("error_status", 400)
++
++ resp = {"error": err}
++ if desc := self._get_param("error_desc", ""):
++ resp["error_description"] = desc
++
++ return resp
++
+ if self._should_modify() and "retries" in self._test_params:
+ retries = self._test_params["retries"]
+
4: 0b522f3117 < -: ---------- Review comments
5: 4a1cd6e1ce ! 4: 3c32d8f59c DO NOT MERGE: Add pytest suite for OAuth
@@ src/test/python/client/test_oauth.py (new)
+ [
+ pytest.param(
+ (
-+ 400,
++ 401,
+ {
+ "error": "invalid_client",
+ "error_description": "client authentication failed",
@@ src/test/python/client/test_oauth.py (new)
+ id="broken error response",
+ ),
+ pytest.param(
++ (401, {"error": "invalid_client"}),
++ r"failed to obtain device authorization: provider requires client authentication, and no oauth_client_secret is set \(invalid_client\)",
++ id="failed authentication without description",
++ ),
++ pytest.param(
+ (200, RawResponse(r'{ "interval": 3.5.8 }')),
+ r"failed to parse device authorization: Token .* is invalid",
+ id="non-numeric interval",
@@ src/test/python/client/test_oauth.py (new)
+ id="empty error response",
+ ),
+ pytest.param(
++ (401, {"error": "invalid_client"}),
++ r"failed to obtain access token: provider requires client authentication, and no oauth_client_secret is set \(invalid_client\)",
++ id="authentication failure without description",
++ ),
++ pytest.param(
+ (200, {}, {}),
+ r"failed to parse access token response: no content type was provided",
+ id="missing content type",