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",