public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jacob Champion <[email protected]>
To: [email protected] <[email protected]>
To: mahendrakar s <[email protected]>
To: Andrey Chudnovsky <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Stephen Frost <[email protected]>
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Date: Fri, 19 May 2023 15:01:11 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAWbhmgWHhqVUkd0nRh59OpVpy_ewfUimqx+NMaWwGNqk=660g@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAAWbhmiWudPQk2euOQQPPa=o14zCN9U_qLwU1pShKO4A-F9yeA@mail.gmail.com>
<CAAWbhmgWHhqVUkd0nRh59OpVpy_ewfUimqx+NMaWwGNqk=660g@mail.gmail.com>
On 4/27/23 10:35, Jacob Champion wrote:
> Moving forward, the first thing I plan to tackle is asynchronous
> operation, so that polling clients can still operate sanely. If I can
> find a good solution there, the conversations about possible extension
> points should get a lot easier.
Attached is patchset v8, now with concurrency and 300% more cURL! And
many more questions to answer.
This is a full reimplementation of the client-side OAuth flow. It's an
async-first engine built on top of cURL's multi handles. All pending
operations are multiplexed into a single epoll set (the "altsock"),
which is exposed through PQsocket() for the duration of the OAuth flow.
Clients return to the flow on their next call to PQconnectPoll().
Andrey and Mahendrakar: you'll probably be interested in the
conn->async_auth() callback, conn->altsock, and the pg_fe_run_oauth_flow
entry point. This is intended to be the foundation for alternative flows.
I've kept the blocking iddawc implementation for comparison, but if
you're running the tests against it, be aware that the asynchronous
tests will, predictably, hang. Skip them with `py.test -k 'not
asynchronous'`.
= The Good =
- PQconnectPoll() is no longer indefinitely blocked on a single
connection's OAuth handshake. (iddawc doesn't appear to have any
asynchronous primitives in its API, unless I've missed something crucial.)
- We now have a swappable entry point. Alternative flows could be
implemented by applications without forcing clients to redesign their
polling loops (PQconnect* should just work as expected).
- We have full control over corner cases in our default flow. Debugging
failures is much nicer, with explanations of exactly what has gone wrong
and where, compared to iddawc's "I_ERROR" messages.
- cURL is not a lightweight library by any means, but we're no longer
bundling things like web servers that we're not going to use.
= The Bad =
- Unsurprisingly, there's a lot more code now that we're implementing
the flow ourselves. The client patch has tripled in size, and we'd be on
the hook for implementing and staying current with the RFCs.
- The client implementation is currently epoll-/Linux-specific. I think
kqueue shouldn't be too much trouble for the BSDs, but it's even more
code to maintain.
- Some clients in the wild (psycopg2/psycopg) suppress all notifications
during PQconnectPoll(). To accommodate them, I no longer use the
noticeHooks for communicating the user code, but that means we have to
come up with some other way to let applications override the printing to
stderr. Something like the OpenSSL decryption callback, maybe?
= The Ugly =
- Unless someone is aware of some amazing Winsock magic, I'm pretty sure
the multiplexed-socket approach is dead in the water on Windows. I think
the strategy there probably has to be a background thread plus a fake
"self-pipe" (loopback socket) for polling... which may be controversial?
- We have to figure out how to initialize cURL in a thread-safe manner.
Newer versions of libcurl and OpenSSL improve upon this situation, but I
don't think there's a way to check at compile time whether the
initialization strategy is safe or not (and even at runtime, I think
there may be a chicken-and-egg problem with the API, where it's not safe
to check for thread-safe initialization until after you've safely
initialized).
= Next Steps =
There are so many TODOs in the cURL implementation: it's been a while
since I've done any libcurl programming, it all needs to be hardened,
and I need to comb through the relevant specs again. But I don't want to
gold-plate it if this overall approach is unacceptable. So, questions
for the gallery:
1) Would starting up a background thread (pooled or not) be acceptable
on Windows? Alternatively, does anyone know enough Winsock deep magic to
combine multiple pending events into one (selectable!) socket?
2) If a background thread is acceptable on one platform, does it make
more sense to use one on every platform and just have synchronous code
everywhere? Or should we use a threadless async implementation when we can?
3) Is the current conn->async_auth() entry point sufficient for an
application to implement the Microsoft flows discussed upthread?
4) Would we want to try to require a new enough cURL/OpenSSL to avoid
thread safety problems during initialization, or do we need to introduce
some API equivalent to PQinitOpenSSL?
5) Does this maintenance tradeoff (full control over the client vs. a
large amount of RFC-governed code) seem like it could be okay?
Thanks,
--Jacob
1: c3698bbc3d = 1: 6434d90105 common/jsonapi: support FRONTEND clients
2: 0cd726fd55 < -: ---------- libpq: add OAUTHBEARER SASL mechanism
-: ---------- > 2: 13ddf2b6b3 libpq: add OAUTHBEARER SASL mechanism
3: 77889eb986 = 3: 0b0b0f2b33 backend: add OAUTHBEARER SASL mechanism
4: 573a2ca3bc ! 4: 0e8ddadcbf Add pytest suite for OAuth
@@ Commit message
dependencies will be installed into ./venv for you. See the README for
more details.
+ For iddawc, asynchronous tests still hang, as expected. Bad-interval
+ tests fail because iddawc apparently doesn't care that the interval is
+ bad.
+
## src/test/python/.gitignore (new) ##
@@
+__pycache__/
@@ src/test/python/client/conftest.py (new)
+import threading
+
+import psycopg2
++import psycopg2.extras
+import pytest
+
+import pq3
@@ src/test/python/client/conftest.py (new)
+ def run(self):
+ try:
+ conn = psycopg2.connect(host="127.0.0.1", **self._kwargs)
++ self._pump_async(conn)
+ conn.close()
+ except Exception as e:
+ self.exception = e
@@ src/test/python/client/conftest.py (new)
+ self.exception = None
+ raise e
+
++ def _pump_async(self, conn):
++ """
++ Polls a psycopg2 connection until it's completed. (Synchronous
++ connections will work here too; they'll just immediately return OK.)
++ """
++ psycopg2.extras.wait_select(conn)
++
+
[email protected]
+def accept(server_socket):
@@ src/test/python/client/conftest.py (new)
+ return sock, client
+
+ yield factory
-+ client.check_completed()
++
++ if client is not None:
++ client.check_completed()
+
+
[email protected]
@@ src/test/python/client/test_oauth.py (new)
@@
+#
+# Copyright 2021 VMware, Inc.
++# Portions Copyright 2023 Timescale, Inc.
+# SPDX-License-Identifier: PostgreSQL
+#
+
@@ src/test/python/client/test_oauth.py (new)
+import threading
+import time
+import urllib.parse
++from numbers import Number
+
+import psycopg2
+import pytest
@@ src/test/python/client/test_oauth.py (new)
+ finish_handshake(conn)
+
+
++class RawResponse(str):
++ """
++ Returned by registered endpoint callbacks to take full control of the
++ response. Usually, return values are converted to JSON; a RawResponse body
++ will be passed to the client as-is, allowing endpoint implementations to
++ issue invalid JSON.
++ """
++
++ pass
++
++
+class OpenIDProvider(threading.Thread):
+ """
+ A thread that runs a mock OpenID provider server.
@@ src/test/python/client/test_oauth.py (new)
+
+ def run(self):
+ try:
-+ self.server.serve_forever()
++ # XXX socketserver.serve_forever() has a serious architectural
++ # issue: its select loop wakes up every `poll_interval` seconds to
++ # see if the server is shutting down. The default, 500 ms, only lets
++ # us run two tests every second. But the faster we go, the more CPU
++ # we burn unnecessarily...
++ self.server.serve_forever(poll_interval=0.01)
+ except Exception as e:
+ self.exception = e
+
@@ src/test/python/client/test_oauth.py (new)
+ self.endpoint_paths = {}
+ self._endpoints = {}
+
++ # Provide a standard discovery document by default; tests can
++ # override it.
++ self.register_endpoint(
++ None,
++ "GET",
++ "/.well-known/openid-configuration",
++ self._default_discovery_handler,
++ )
++
+ def register_endpoint(self, name, method, path, func):
+ if method not in self._endpoints:
+ self._endpoints[method] = {}
+
+ self._endpoints[method][path] = func
-+ self.endpoint_paths[name] = path
++
++ if name is not None:
++ self.endpoint_paths[name] = path
+
+ def endpoint(self, method, path):
+ if method not in self._endpoints:
@@ src/test/python/client/test_oauth.py (new)
+
+ return self._endpoints[method].get(path)
+
++ def _default_discovery_handler(self, headers, params):
++ doc = {
++ "issuer": self.issuer,
++ "response_types_supported": ["token"],
++ "subject_types_supported": ["public"],
++ "id_token_signing_alg_values_supported": ["RS256"],
++ "grant_types_supported": [
++ "urn:ietf:params:oauth:grant-type:device_code"
++ ],
++ }
++
++ for name, path in self.endpoint_paths.items():
++ doc[name] = self.issuer + path
++
++ return 200, doc
++
+ class _Server(http.server.HTTPServer):
+ def handle_error(self, request, addr):
+ self.shutdown_request(request)
@@ src/test/python/client/test_oauth.py (new)
+ class _Handler(http.server.BaseHTTPRequestHandler):
+ timeout = BLOCKING_TIMEOUT
+
-+ def _discovery_handler(self, headers, params):
-+ oauth = self.server.oauth
-+
-+ doc = {
-+ "issuer": oauth.issuer,
-+ "response_types_supported": ["token"],
-+ "subject_types_supported": ["public"],
-+ "id_token_signing_alg_values_supported": ["RS256"],
-+ }
-+
-+ for name, path in oauth.endpoint_paths.items():
-+ doc[name] = oauth.issuer + path
-+
-+ return 200, doc
-+
+ def _handle(self, *, params=None, handler=None):
+ oauth = self.server.oauth
+ assert self.headers["Host"] == oauth.host
@@ src/test/python/client/test_oauth.py (new)
+ handler is not None
+ ), f"no registered endpoint for {self.command} {self.path}"
+
-+ code, resp = handler(self.headers, params)
++ result = handler(self.headers, params)
++
++ if len(result) == 2:
++ headers = {"Content-Type": "application/json"}
++ code, resp = result
++ else:
++ code, headers, resp = result
+
+ self.send_response(code)
-+ self.send_header("Content-Type", "application/json")
++ for h, v in headers.items():
++ self.send_header(h, v)
+ self.end_headers()
+
-+ resp = json.dumps(resp)
-+ resp = resp.encode("utf-8")
-+ self.wfile.write(resp)
++ if resp is not None:
++ if not isinstance(resp, RawResponse):
++ resp = json.dumps(resp)
++ resp = resp.encode("utf-8")
++ self.wfile.write(resp)
+
+ self.close_connection = True
+
+ def do_GET(self):
-+ if self.path == "/.well-known/openid-configuration":
-+ self._handle(handler=self._discovery_handler)
-+ return
-+
+ self._handle()
+
+ def _request_body(self):
@@ src/test/python/client/test_oauth.py (new)
[email protected]("secret", [None, "", "hunter2"])
[email protected]("scope", [None, "", "openid email"])
[email protected]("retries", [0, 1])
[email protected](
++ "asynchronous",
++ [
++ pytest.param(False, id="synchronous"),
++ pytest.param(True, id="asynchronous"),
++ ],
++)
+def test_oauth_with_explicit_issuer(
-+ capfd, accept, openid_provider, retries, scope, secret
++ capfd, accept, openid_provider, asynchronous, retries, scope, secret
+):
+ client_id = secrets.token_hex()
+
@@ src/test/python/client/test_oauth.py (new)
+ oauth_client_id=client_id,
+ oauth_client_secret=secret,
+ oauth_scope=scope,
++ async_=asynchronous,
+ )
+
+ device_code = secrets.token_hex()
@@ src/test/python/client/test_oauth.py (new)
+ assert expected in stderr
+
+
-+def test_oauth_requires_client_id(accept, openid_provider):
-+ sock, client = accept(
-+ oauth_issuer=openid_provider.issuer,
-+ # Do not set a client ID; this should cause a client error after the
-+ # server asks for OAUTHBEARER and the client tries to contact the
-+ # issuer.
-+ )
-+
++def expect_disconnected_handshake(sock):
++ """
++ Helper for any tests that expect the client to disconnect immediately after
++ being sent the OAUTHBEARER SASL method. Generally speaking, this requires
++ 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.
@@ src/test/python/client/test_oauth.py (new)
+ # The client should disconnect at this point.
+ assert not conn.read()
+
++
++def test_oauth_requires_client_id(accept, openid_provider):
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ # Do not set a client ID; this should cause a client error after the
++ # server asks for OAUTHBEARER and the client tries to contact the
++ # issuer.
++ )
++
++ expect_disconnected_handshake(sock)
++
+ expected_error = "no oauth_client_id is set"
+ with pytest.raises(psycopg2.OperationalError, match=expected_error):
+ client.check_completed()
@@ src/test/python/client/test_oauth.py (new)
+ finish_handshake(conn)
+
+
++def alt_patterns(*patterns):
++ """
++ Just combines multiple alternative regexes into one. It's not very efficient
++ but IMO it's easier to read and maintain.
++ """
++ pat = ""
++
++ for p in patterns:
++ if pat:
++ pat += "|"
++ pat += f"({p})"
++
++ return pat
++
++
[email protected](
+ "failure_mode, error_pattern",
+ [
+ pytest.param(
-+ {
-+ "error": "invalid_client",
-+ "error_description": "client authentication failed",
-+ },
-+ r"client authentication failed \(invalid_client\)",
++ (
++ 400,
++ {
++ "error": "invalid_client",
++ "error_description": "client authentication failed",
++ },
++ ),
++ r"failed to obtain device authorization: client authentication failed \(invalid_client\)",
+ id="authentication failure with description",
+ ),
+ pytest.param(
-+ {"error": "invalid_request"},
-+ r"\(invalid_request\)",
++ (400, {"error": "invalid_request"}),
++ r"failed to obtain device authorization: \(invalid_request\)",
+ id="invalid request without description",
+ ),
+ pytest.param(
-+ {},
-+ r"failed to obtain device authorization",
++ (400, {}),
++ alt_patterns(
++ r'failed to parse token error response: field "error" is missing',
++ r"failed to obtain device authorization: \(iddawc error I_ERROR_PARAM\)",
++ ),
+ id="broken error response",
+ ),
++ pytest.param(
++ (200, RawResponse(r'{ "interval": 3.5.8 }')),
++ alt_patterns(
++ r"failed to parse device authorization: Token .* is invalid",
++ r"failed to obtain device authorization: \(iddawc error I_ERROR\)",
++ ),
++ id="non-numeric interval",
++ ),
++ pytest.param(
++ (200, RawResponse(r'{ "interval": 08 }')),
++ alt_patterns(
++ r"failed to parse device authorization: Token .* is invalid",
++ r"failed to obtain device authorization: \(iddawc error I_ERROR\)",
++ ),
++ id="invalid numeric interval",
++ ),
+ ],
+)
+def test_oauth_device_authorization_failures(
@@ src/test/python/client/test_oauth.py (new)
+ # any unprotected state mutation here.
+
+ def authorization_endpoint(headers, params):
-+ return 400, failure_mode
++ return failure_mode
+
+ openid_provider.register_endpoint(
+ "device_authorization_endpoint", "POST", "/device", authorization_endpoint
@@ src/test/python/client/test_oauth.py (new)
+ "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.
-+ startup = pq3.recv1(conn, cls=pq3.Startup)
-+ assert startup.proto == pq3.protocol(3, 0)
++ expect_disconnected_handshake(sock)
+
-+ pq3.send(
-+ conn,
-+ pq3.types.AuthnRequest,
-+ type=pq3.authn.SASL,
-+ body=[b"OAUTHBEARER", b""],
-+ )
++ # Now make sure the client correctly failed.
++ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
++ client.check_completed()
+
-+ # The client should not continue the connection due to the hardcoded
-+ # provider failure; we disconnect here.
++
++Missing = object() # sentinel for test_oauth_device_authorization_bad_json()
++
++
[email protected](
++ "bad_value",
++ [
++ pytest.param({"device_code": 3}, id="object"),
++ pytest.param([1, 2, 3], id="array"),
++ pytest.param("some string", id="string"),
++ pytest.param(4, id="numeric"),
++ pytest.param(False, id="boolean"),
++ pytest.param(None, id="null"),
++ pytest.param(Missing, id="missing"),
++ ],
++)
[email protected](
++ "field_name,ok_type,required",
++ [
++ ("device_code", str, True),
++ ("user_code", str, True),
++ ("verification_uri", str, True),
++ ("interval", int, False),
++ ],
++)
++def test_oauth_device_authorization_bad_json_schema(
++ accept, openid_provider, field_name, ok_type, required, bad_value
++):
++ # To make the test matrix easy, just skip the tests that aren't actually
++ # interesting (field of the correct type, missing optional field).
++ if bad_value is Missing and not required:
++ pytest.skip("not interesting: optional field")
++ elif type(bad_value) == ok_type: # not isinstance(), because bool is an int
++ pytest.skip("not interesting: correct type")
++
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ oauth_client_id=secrets.token_hex(),
++ )
++
++ # 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):
++ # Begin with an acceptable base response...
++ resp = {
++ "device_code": "my-device-code",
++ "user_code": "my-user-code",
++ "interval": 0,
++ "verification_uri": "https://example.com";,
++ "expires_in": 5,
++ }
++
++ # ...then tweak it so the client fails.
++ if bad_value is Missing:
++ del resp[field_name]
++ else:
++ resp[field_name] = bad_value
++
++ return 200, resp
++
++ openid_provider.register_endpoint(
++ "device_authorization_endpoint", "POST", "/device", authorization_endpoint
++ )
++
++ def token_endpoint(headers, params):
++ assert False, "token endpoint was invoked unexpectedly"
++
++ openid_provider.register_endpoint(
++ "token_endpoint", "POST", "/token", token_endpoint
++ )
++
++ expect_disconnected_handshake(sock)
+
+ # Now make sure the client correctly failed.
++ if bad_value is Missing:
++ error_pattern = f'field "{field_name}" is missing'
++ elif ok_type == str:
++ error_pattern = f'field "{field_name}" must be a string'
++ elif ok_type == int:
++ error_pattern = f'field "{field_name}" must be a number'
++ else:
++ assert False, "update error_pattern for new failure mode"
++
++ # XXX iddawc doesn't really check for problems in the device authorization
++ # response, leading to this patchwork:
++ if field_name == "verification_uri":
++ error_pattern = alt_patterns(
++ error_pattern,
++ "issuer did not provide a verification URI",
++ )
++ elif field_name == "user_code":
++ error_pattern = alt_patterns(
++ error_pattern,
++ "issuer did not provide a user code",
++ )
++ else:
++ error_pattern = alt_patterns(
++ error_pattern,
++ r"failed to obtain access token: \(iddawc error I_ERROR_PARAM\)",
++ )
++
+ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
+ client.check_completed()
+
@@ src/test/python/client/test_oauth.py (new)
+ "failure_mode, error_pattern",
+ [
+ pytest.param(
-+ {
-+ "error": "expired_token",
-+ "error_description": "the device code has expired",
-+ },
-+ r"the device code has expired \(expired_token\)",
++ (
++ 400,
++ {
++ "error": "expired_token",
++ "error_description": "the device code has expired",
++ },
++ ),
++ r"failed to obtain access token: the device code has expired \(expired_token\)",
+ id="expired token with description",
+ ),
+ pytest.param(
-+ {"error": "access_denied"},
-+ r"\(access_denied\)",
++ (400, {"error": "access_denied"}),
++ r"failed to obtain access token: \(access_denied\)",
+ id="access denied without description",
+ ),
+ pytest.param(
-+ {},
-+ r"OAuth token retrieval failed",
-+ id="broken error response",
++ (400, {}),
++ alt_patterns(
++ r'failed to parse token error response: field "error" is missing',
++ r"failed to obtain access token: \(iddawc error I_ERROR_PARAM\)",
++ ),
++ id="empty error response",
++ ),
++ pytest.param(
++ (200, {}, {}),
++ alt_patterns(
++ r"failed to parse access token response: no content type was provided",
++ r"failed to obtain access token: \(iddawc error I_ERROR\)",
++ ),
++ id="missing content type",
++ ),
++ pytest.param(
++ (200, {"Content-Type": "text/plain"}, {}),
++ alt_patterns(
++ r"failed to parse access token response: unexpected content type",
++ r"failed to obtain access token: \(iddawc error I_ERROR\)",
++ ),
++ id="wrong content type",
+ ),
+ ],
+)
@@ src/test/python/client/test_oauth.py (new)
+ )
+
+ retry_lock = threading.Lock()
++ final_sent = False
+
+ def token_endpoint(headers, params):
+ with retry_lock:
-+ nonlocal retries
++ nonlocal retries, final_sent
+
+ # If the test wants to force the client to retry, return an
+ # authorization_pending response and decrement the retry count.
@@ src/test/python/client/test_oauth.py (new)
+ retries -= 1
+ return 400, {"error": "authorization_pending"}
+
-+ return 400, failure_mode
++ # We should only return our failure_mode response once; any further
++ # requests indicate that the client isn't correctly bailing out.
++ assert not final_sent, "client continued after token error"
++
++ final_sent = True
++
++ return failure_mode
+
+ 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.
-+ startup = pq3.recv1(conn, cls=pq3.Startup)
-+ assert startup.proto == pq3.protocol(3, 0)
++ expect_disconnected_handshake(sock)
+
-+ pq3.send(
-+ conn,
-+ pq3.types.AuthnRequest,
-+ type=pq3.authn.SASL,
-+ body=[b"OAUTHBEARER", b""],
-+ )
++ # Now make sure the client correctly failed.
++ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
++ client.check_completed()
+
-+ # The client should not continue the connection due to the hardcoded
-+ # provider failure; we disconnect here.
++
[email protected](
++ "bad_value",
++ [
++ pytest.param({"device_code": 3}, id="object"),
++ pytest.param([1, 2, 3], id="array"),
++ pytest.param("some string", id="string"),
++ pytest.param(4, id="numeric"),
++ pytest.param(False, id="boolean"),
++ pytest.param(None, id="null"),
++ pytest.param(Missing, id="missing"),
++ ],
++)
[email protected](
++ "field_name,ok_type,required",
++ [
++ ("access_token", str, True),
++ ("token_type", str, True),
++ ],
++)
++def test_oauth_token_bad_json_schema(
++ accept, openid_provider, field_name, ok_type, required, bad_value
++):
++ # To make the test matrix easy, just skip the tests that aren't actually
++ # interesting (field of the correct type, missing optional field).
++ if bad_value is Missing and not required:
++ pytest.skip("not interesting: optional field")
++ elif type(bad_value) == ok_type: # not isinstance(), because bool is an int
++ pytest.skip("not interesting: correct type")
++
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ oauth_client_id=secrets.token_hex(),
++ )
++
++ # 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": "my-device-code",
++ "user_code": "my-user-code",
++ "interval": 0,
++ "verification_uri": "https://example.com";,
++ "expires_in": 5,
++ }
++
++ return 200, resp
++
++ openid_provider.register_endpoint(
++ "device_authorization_endpoint", "POST", "/device", authorization_endpoint
++ )
++
++ def token_endpoint(headers, params):
++ # Begin with an acceptable base response...
++ resp = {
++ "access_token": secrets.token_urlsafe(),
++ "token_type": "bearer",
++ }
++
++ # ...then tweak it so the client fails.
++ if bad_value is Missing:
++ del resp[field_name]
++ else:
++ resp[field_name] = bad_value
++
++ return 200, resp
++
++ openid_provider.register_endpoint(
++ "token_endpoint", "POST", "/token", token_endpoint
++ )
++
++ expect_disconnected_handshake(sock)
+
+ # Now make sure the client correctly failed.
++ error_pattern = "failed to parse access token response: "
++ if bad_value is Missing:
++ error_pattern += f'field "{field_name}" is missing'
++ elif ok_type == str:
++ error_pattern += f'field "{field_name}" must be a string'
++ elif ok_type == int:
++ error_pattern += f'field "{field_name}" must be a number'
++ else:
++ assert False, "update error_pattern for new failure mode"
++
++ # XXX iddawc is fairly silent on the topic.
++ error_pattern = alt_patterns(
++ error_pattern,
++ r"failed to obtain access token: \(iddawc error I_ERROR_PARAM\)",
++ )
++
+ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
+ client.check_completed()
+
@@ src/test/python/client/test_oauth.py (new)
+
+
[email protected](
++ "bad_response,expected_error",
++ [
++ pytest.param(
++ (200, {"Content-Type": "text/plain"}, {}),
++ r'failed to parse OpenID discovery document: unexpected content type "text/plain"',
++ id="not JSON",
++ ),
++ pytest.param(
++ (200, {}, {}),
++ r"failed to parse OpenID discovery document: no content type was provided",
++ id="no Content-Type",
++ ),
++ pytest.param(
++ (204, {}, None),
++ r"failed to fetch OpenID discovery document: unexpected response code 204",
++ id="no content",
++ ),
++ pytest.param(
++ (301, {"Location": "https://localhost/";}, None),
++ r"failed to fetch OpenID discovery document: unexpected response code 301",
++ id="redirection",
++ ),
++ pytest.param(
++ (404, {}),
++ r"failed to fetch OpenID discovery document: unexpected response code 404",
++ id="not found",
++ ),
++ pytest.param(
++ (200, RawResponse("blah\x00blah")),
++ r"failed to parse OpenID discovery document: response contains embedded NULLs",
++ id="NULL bytes in document",
++ ),
++ pytest.param(
++ (200, 123),
++ r"failed to parse OpenID discovery document: top-level element must be an object",
++ id="scalar at top level",
++ ),
++ pytest.param(
++ (200, []),
++ r"failed to parse OpenID discovery document: top-level element must be an object",
++ id="array at top level",
++ ),
++ pytest.param(
++ (200, RawResponse("{")),
++ r"failed to parse OpenID discovery document.* input string ended unexpectedly",
++ id="unclosed object",
++ ),
++ pytest.param(
++ (200, RawResponse(r'{ "hello": ] }')),
++ r"failed to parse OpenID discovery document.* Expected JSON value",
++ id="bad array",
++ ),
++ pytest.param(
++ (200, {"issuer": 123}),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="non-string issuer",
++ ),
++ pytest.param(
++ (200, {"issuer": ["something"]}),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="issuer array",
++ ),
++ pytest.param(
++ (200, {"issuer": {}}),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="issuer object",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": 123}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="scalar grant types field",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": {}}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="object grant types field",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": [123]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="non-string grant types",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": ["something", 123]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="non-string grant types later in the list",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": ["something", {}]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="object grant types later in the list",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": ["something", ["something"]]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="embedded array grant types later in the list",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "grant_types_supported": ["something"],
++ "token_endpoint": "https://example.com/";,
++ "issuer": 123,
++ },
++ ),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="non-string issuer after other valid fields",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "ignored": {"grant_types_supported": 123, "token_endpoint": 123},
++ "issuer": 123,
++ },
++ ),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="non-string issuer after other ignored fields",
++ ),
++ pytest.param(
++ (200, {"token_endpoint": "https://example.com/";}),
++ r'failed to parse OpenID discovery document: field "issuer" is missing',
++ id="missing issuer",
++ ),
++ pytest.param(
++ (200, {"issuer": "https://example.com/";}),
++ r'failed to parse OpenID discovery document: field "token_endpoint" is missing',
++ id="missing token endpoint",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "issuer": "https://example.com";,
++ "token_endpoint": "https://example.com/token";,
++ "device_authorization_endpoint": "https://example.com/dev";,
++ },
++ ),
++ r'cannot run OAuth device authorization: issuer "https://example.com"; does not support device code grants',
++ id="missing device code grants",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "issuer": "https://example.com";,
++ "token_endpoint": "https://example.com/token";,
++ "grant_types_supported": [
++ "urn:ietf:params:oauth:grant-type:device_code"
++ ],
++ },
++ ),
++ r'cannot run OAuth device authorization: issuer "https://example.com"; does not provide a device authorization endpoint',
++ id="missing device_authorization_endpoint",
++ ),
++ #
++ # Exercise HTTP-level failures by breaking the protocol. Note that the
++ # error messages here are implementation-dependent.
++ #
++ pytest.param(
++ (1000, {}),
++ r"failed to fetch OpenID discovery document: Unsupported protocol \(.*\)",
++ id="invalid HTTP response code",
++ ),
++ pytest.param(
++ (200, {"Content-Length": -1}, {}),
++ r"failed to fetch OpenID discovery document: Weird server reply \(.*Content-Length.*\)",
++ id="bad HTTP Content-Length",
++ ),
++ ],
++)
++def test_oauth_discovery_provider_failure(
++ accept, openid_provider, bad_response, expected_error
++):
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ oauth_client_id=secrets.token_hex(),
++ )
++
++ def failing_discovery_handler(headers, params):
++ return bad_response
++
++ openid_provider.register_endpoint(
++ None,
++ "GET",
++ "/.well-known/openid-configuration",
++ failing_discovery_handler,
++ )
++
++ expect_disconnected_handshake(sock)
++
++ # XXX iddawc doesn't differentiate...
++ expected_error = alt_patterns(
++ expected_error,
++ r"failed to fetch OpenID discovery document \(iddawc error I_ERROR(_PARAM)?\)",
++ )
++
++ with pytest.raises(psycopg2.OperationalError, match=expected_error):
++ client.check_completed()
++
++
[email protected](
+ "sasl_err,resp_type,resp_payload,expected_error",
+ [
+ pytest.param(
@@ src/test/python/client/test_oauth.py (new)
+ "server sent additional OAuth data",
+ id="broken server: SASL success after error",
+ ),
++ pytest.param(
++ {"status": "invalid_request"},
++ pq3.types.AuthnRequest,
++ dict(type=pq3.authn.SASL, body=[b"OAUTHBEARER", b""]),
++ "duplicate SASL authentication request",
++ id="broken server: SASL reinitialization after error",
++ ),
+ ],
+)
+def test_oauth_server_error(accept, sasl_err, resp_type, resp_payload, expected_error):
5: 4490d029b5 ! 5: 65c319a6a3 squash! Add pytest suite for OAuth
@@ .cirrus.yml: task:
sysctl kern.corefile='/tmp/cores/%N.%P.core'
setup_additional_packages_script: |
- #pkg install -y ...
-+ pkg install -y iddawc
++ pkg install -y curl
# NB: Intentionally build without -Dllvm. The freebsd image size is already
# large enough to make VM startup slow, and even without llvm freebsd
@@ .cirrus.yml: task:
--buildtype=debug \
-Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-+ -Doauth=enabled \
++ -Doauth=curl \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
EOF
@@ .cirrus.yml: LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
--with-libxslt
--with-llvm
--with-lz4
-+ --with-oauth
++ --with-oauth=curl
--with-pam
--with-perl
--with-python
@@ .cirrus.yml: LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
-+ -Doauth=enabled
++ -Doauth=curl
-Duuid=e2fs
@@ .cirrus.yml: task:
- #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
+ apt-get update
+ DEBIAN_FRONTEND=noninteractive apt-get -y install \
-+ libiddawc-dev \
-+ libiddawc-dev:i386 \
++ libcurl4-openssl-dev \
++ libcurl4-openssl-dev:i386 \
+ python3-venv \
matrix:
@@ .cirrus.yml: task:
- #apt-get update
- #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
+ apt-get update
-+ DEBIAN_FRONTEND=noninteractive apt-get -y install libiddawc-dev
++ DEBIAN_FRONTEND=noninteractive apt-get -y install libcurl4-openssl-dev
###
# Test that code can be built with gcc/clang without warnings
@@ meson.build: foreach test_dir : tests
+ test_group = test_dir['name']
+ test_output = test_result_dir / test_group / kind
+ test_kwargs = {
-+ 'protocol': 'tap',
++ #'protocol': 'tap',
+ 'suite': test_group,
+ 'timeout': 1000,
+ 'depends': test_deps,
@@ meson.build: foreach test_dir : tests
+ pytest = venv_path / 'bin' / 'py.test'
+ test_command = [
+ pytest,
++ # Avoid running these tests against an existing database.
++ '--temp-instance', test_output / 'tmp_check',
++
+ # FIXME pytest-tap's stream feature accidentally suppresses errors that
+ # are critical for debugging:
+ # https://github.com/python-tap/pytest-tap/issues/30
-+ # Fix -- or maybe don't use the meson TAP protocol for now?
-+ '--tap-stream',
-+ # Avoid running these tests against an existing database.
-+ '--temp-instance', test_output / 'tmp_check',
++ # Don't use the meson TAP protocol for now...
++ #'--tap-stream',
+ ]
+
+ foreach pyt : t['tests']
@@ src/test/python/client/test_oauth.py: import pq3
+# The client tests need libpq to have been compiled with OAuth support; skip
+# them otherwise.
+pytestmark = pytest.mark.skipif(
-+ os.getenv("with_oauth") == "no",
++ os.getenv("with_oauth") == "none",
+ reason="OAuth client tests require --with-oauth support",
+)
+
@@ src/test/python/meson.build (new)
+ './test_pq3.py',
+ ],
+ 'env': {
-+ 'with_oauth': oauth.found() ? 'yes' : 'no',
++ 'with_oauth': oauth_library,
+
+ # Point to the default database; the tests will create their own databases
+ # as needed.
Attachments:
[text/plain] since-v7.diff.txt (39.9K, ../[email protected]/2-since-v7.diff.txt)
download | inline:
1: c3698bbc3d = 1: 6434d90105 common/jsonapi: support FRONTEND clients
2: 0cd726fd55 < -: ---------- libpq: add OAUTHBEARER SASL mechanism
-: ---------- > 2: 13ddf2b6b3 libpq: add OAUTHBEARER SASL mechanism
3: 77889eb986 = 3: 0b0b0f2b33 backend: add OAUTHBEARER SASL mechanism
4: 573a2ca3bc ! 4: 0e8ddadcbf Add pytest suite for OAuth
@@ Commit message
dependencies will be installed into ./venv for you. See the README for
more details.
+ For iddawc, asynchronous tests still hang, as expected. Bad-interval
+ tests fail because iddawc apparently doesn't care that the interval is
+ bad.
+
## src/test/python/.gitignore (new) ##
@@
+__pycache__/
@@ src/test/python/client/conftest.py (new)
+import threading
+
+import psycopg2
++import psycopg2.extras
+import pytest
+
+import pq3
@@ src/test/python/client/conftest.py (new)
+ def run(self):
+ try:
+ conn = psycopg2.connect(host="127.0.0.1", **self._kwargs)
++ self._pump_async(conn)
+ conn.close()
+ except Exception as e:
+ self.exception = e
@@ src/test/python/client/conftest.py (new)
+ self.exception = None
+ raise e
+
++ def _pump_async(self, conn):
++ """
++ Polls a psycopg2 connection until it's completed. (Synchronous
++ connections will work here too; they'll just immediately return OK.)
++ """
++ psycopg2.extras.wait_select(conn)
++
+
[email protected]
+def accept(server_socket):
@@ src/test/python/client/conftest.py (new)
+ return sock, client
+
+ yield factory
-+ client.check_completed()
++
++ if client is not None:
++ client.check_completed()
+
+
[email protected]
@@ src/test/python/client/test_oauth.py (new)
@@
+#
+# Copyright 2021 VMware, Inc.
++# Portions Copyright 2023 Timescale, Inc.
+# SPDX-License-Identifier: PostgreSQL
+#
+
@@ src/test/python/client/test_oauth.py (new)
+import threading
+import time
+import urllib.parse
++from numbers import Number
+
+import psycopg2
+import pytest
@@ src/test/python/client/test_oauth.py (new)
+ finish_handshake(conn)
+
+
++class RawResponse(str):
++ """
++ Returned by registered endpoint callbacks to take full control of the
++ response. Usually, return values are converted to JSON; a RawResponse body
++ will be passed to the client as-is, allowing endpoint implementations to
++ issue invalid JSON.
++ """
++
++ pass
++
++
+class OpenIDProvider(threading.Thread):
+ """
+ A thread that runs a mock OpenID provider server.
@@ src/test/python/client/test_oauth.py (new)
+
+ def run(self):
+ try:
-+ self.server.serve_forever()
++ # XXX socketserver.serve_forever() has a serious architectural
++ # issue: its select loop wakes up every `poll_interval` seconds to
++ # see if the server is shutting down. The default, 500 ms, only lets
++ # us run two tests every second. But the faster we go, the more CPU
++ # we burn unnecessarily...
++ self.server.serve_forever(poll_interval=0.01)
+ except Exception as e:
+ self.exception = e
+
@@ src/test/python/client/test_oauth.py (new)
+ self.endpoint_paths = {}
+ self._endpoints = {}
+
++ # Provide a standard discovery document by default; tests can
++ # override it.
++ self.register_endpoint(
++ None,
++ "GET",
++ "/.well-known/openid-configuration",
++ self._default_discovery_handler,
++ )
++
+ def register_endpoint(self, name, method, path, func):
+ if method not in self._endpoints:
+ self._endpoints[method] = {}
+
+ self._endpoints[method][path] = func
-+ self.endpoint_paths[name] = path
++
++ if name is not None:
++ self.endpoint_paths[name] = path
+
+ def endpoint(self, method, path):
+ if method not in self._endpoints:
@@ src/test/python/client/test_oauth.py (new)
+
+ return self._endpoints[method].get(path)
+
++ def _default_discovery_handler(self, headers, params):
++ doc = {
++ "issuer": self.issuer,
++ "response_types_supported": ["token"],
++ "subject_types_supported": ["public"],
++ "id_token_signing_alg_values_supported": ["RS256"],
++ "grant_types_supported": [
++ "urn:ietf:params:oauth:grant-type:device_code"
++ ],
++ }
++
++ for name, path in self.endpoint_paths.items():
++ doc[name] = self.issuer + path
++
++ return 200, doc
++
+ class _Server(http.server.HTTPServer):
+ def handle_error(self, request, addr):
+ self.shutdown_request(request)
@@ src/test/python/client/test_oauth.py (new)
+ class _Handler(http.server.BaseHTTPRequestHandler):
+ timeout = BLOCKING_TIMEOUT
+
-+ def _discovery_handler(self, headers, params):
-+ oauth = self.server.oauth
-+
-+ doc = {
-+ "issuer": oauth.issuer,
-+ "response_types_supported": ["token"],
-+ "subject_types_supported": ["public"],
-+ "id_token_signing_alg_values_supported": ["RS256"],
-+ }
-+
-+ for name, path in oauth.endpoint_paths.items():
-+ doc[name] = oauth.issuer + path
-+
-+ return 200, doc
-+
+ def _handle(self, *, params=None, handler=None):
+ oauth = self.server.oauth
+ assert self.headers["Host"] == oauth.host
@@ src/test/python/client/test_oauth.py (new)
+ handler is not None
+ ), f"no registered endpoint for {self.command} {self.path}"
+
-+ code, resp = handler(self.headers, params)
++ result = handler(self.headers, params)
++
++ if len(result) == 2:
++ headers = {"Content-Type": "application/json"}
++ code, resp = result
++ else:
++ code, headers, resp = result
+
+ self.send_response(code)
-+ self.send_header("Content-Type", "application/json")
++ for h, v in headers.items():
++ self.send_header(h, v)
+ self.end_headers()
+
-+ resp = json.dumps(resp)
-+ resp = resp.encode("utf-8")
-+ self.wfile.write(resp)
++ if resp is not None:
++ if not isinstance(resp, RawResponse):
++ resp = json.dumps(resp)
++ resp = resp.encode("utf-8")
++ self.wfile.write(resp)
+
+ self.close_connection = True
+
+ def do_GET(self):
-+ if self.path == "/.well-known/openid-configuration":
-+ self._handle(handler=self._discovery_handler)
-+ return
-+
+ self._handle()
+
+ def _request_body(self):
@@ src/test/python/client/test_oauth.py (new)
[email protected]("secret", [None, "", "hunter2"])
[email protected]("scope", [None, "", "openid email"])
[email protected]("retries", [0, 1])
[email protected](
++ "asynchronous",
++ [
++ pytest.param(False, id="synchronous"),
++ pytest.param(True, id="asynchronous"),
++ ],
++)
+def test_oauth_with_explicit_issuer(
-+ capfd, accept, openid_provider, retries, scope, secret
++ capfd, accept, openid_provider, asynchronous, retries, scope, secret
+):
+ client_id = secrets.token_hex()
+
@@ src/test/python/client/test_oauth.py (new)
+ oauth_client_id=client_id,
+ oauth_client_secret=secret,
+ oauth_scope=scope,
++ async_=asynchronous,
+ )
+
+ device_code = secrets.token_hex()
@@ src/test/python/client/test_oauth.py (new)
+ assert expected in stderr
+
+
-+def test_oauth_requires_client_id(accept, openid_provider):
-+ sock, client = accept(
-+ oauth_issuer=openid_provider.issuer,
-+ # Do not set a client ID; this should cause a client error after the
-+ # server asks for OAUTHBEARER and the client tries to contact the
-+ # issuer.
-+ )
-+
++def expect_disconnected_handshake(sock):
++ """
++ Helper for any tests that expect the client to disconnect immediately after
++ being sent the OAUTHBEARER SASL method. Generally speaking, this requires
++ 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.
@@ src/test/python/client/test_oauth.py (new)
+ # The client should disconnect at this point.
+ assert not conn.read()
+
++
++def test_oauth_requires_client_id(accept, openid_provider):
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ # Do not set a client ID; this should cause a client error after the
++ # server asks for OAUTHBEARER and the client tries to contact the
++ # issuer.
++ )
++
++ expect_disconnected_handshake(sock)
++
+ expected_error = "no oauth_client_id is set"
+ with pytest.raises(psycopg2.OperationalError, match=expected_error):
+ client.check_completed()
@@ src/test/python/client/test_oauth.py (new)
+ finish_handshake(conn)
+
+
++def alt_patterns(*patterns):
++ """
++ Just combines multiple alternative regexes into one. It's not very efficient
++ but IMO it's easier to read and maintain.
++ """
++ pat = ""
++
++ for p in patterns:
++ if pat:
++ pat += "|"
++ pat += f"({p})"
++
++ return pat
++
++
[email protected](
+ "failure_mode, error_pattern",
+ [
+ pytest.param(
-+ {
-+ "error": "invalid_client",
-+ "error_description": "client authentication failed",
-+ },
-+ r"client authentication failed \(invalid_client\)",
++ (
++ 400,
++ {
++ "error": "invalid_client",
++ "error_description": "client authentication failed",
++ },
++ ),
++ r"failed to obtain device authorization: client authentication failed \(invalid_client\)",
+ id="authentication failure with description",
+ ),
+ pytest.param(
-+ {"error": "invalid_request"},
-+ r"\(invalid_request\)",
++ (400, {"error": "invalid_request"}),
++ r"failed to obtain device authorization: \(invalid_request\)",
+ id="invalid request without description",
+ ),
+ pytest.param(
-+ {},
-+ r"failed to obtain device authorization",
++ (400, {}),
++ alt_patterns(
++ r'failed to parse token error response: field "error" is missing',
++ r"failed to obtain device authorization: \(iddawc error I_ERROR_PARAM\)",
++ ),
+ id="broken error response",
+ ),
++ pytest.param(
++ (200, RawResponse(r'{ "interval": 3.5.8 }')),
++ alt_patterns(
++ r"failed to parse device authorization: Token .* is invalid",
++ r"failed to obtain device authorization: \(iddawc error I_ERROR\)",
++ ),
++ id="non-numeric interval",
++ ),
++ pytest.param(
++ (200, RawResponse(r'{ "interval": 08 }')),
++ alt_patterns(
++ r"failed to parse device authorization: Token .* is invalid",
++ r"failed to obtain device authorization: \(iddawc error I_ERROR\)",
++ ),
++ id="invalid numeric interval",
++ ),
+ ],
+)
+def test_oauth_device_authorization_failures(
@@ src/test/python/client/test_oauth.py (new)
+ # any unprotected state mutation here.
+
+ def authorization_endpoint(headers, params):
-+ return 400, failure_mode
++ return failure_mode
+
+ openid_provider.register_endpoint(
+ "device_authorization_endpoint", "POST", "/device", authorization_endpoint
@@ src/test/python/client/test_oauth.py (new)
+ "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.
-+ startup = pq3.recv1(conn, cls=pq3.Startup)
-+ assert startup.proto == pq3.protocol(3, 0)
++ expect_disconnected_handshake(sock)
+
-+ pq3.send(
-+ conn,
-+ pq3.types.AuthnRequest,
-+ type=pq3.authn.SASL,
-+ body=[b"OAUTHBEARER", b""],
-+ )
++ # Now make sure the client correctly failed.
++ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
++ client.check_completed()
+
-+ # The client should not continue the connection due to the hardcoded
-+ # provider failure; we disconnect here.
++
++Missing = object() # sentinel for test_oauth_device_authorization_bad_json()
++
++
[email protected](
++ "bad_value",
++ [
++ pytest.param({"device_code": 3}, id="object"),
++ pytest.param([1, 2, 3], id="array"),
++ pytest.param("some string", id="string"),
++ pytest.param(4, id="numeric"),
++ pytest.param(False, id="boolean"),
++ pytest.param(None, id="null"),
++ pytest.param(Missing, id="missing"),
++ ],
++)
[email protected](
++ "field_name,ok_type,required",
++ [
++ ("device_code", str, True),
++ ("user_code", str, True),
++ ("verification_uri", str, True),
++ ("interval", int, False),
++ ],
++)
++def test_oauth_device_authorization_bad_json_schema(
++ accept, openid_provider, field_name, ok_type, required, bad_value
++):
++ # To make the test matrix easy, just skip the tests that aren't actually
++ # interesting (field of the correct type, missing optional field).
++ if bad_value is Missing and not required:
++ pytest.skip("not interesting: optional field")
++ elif type(bad_value) == ok_type: # not isinstance(), because bool is an int
++ pytest.skip("not interesting: correct type")
++
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ oauth_client_id=secrets.token_hex(),
++ )
++
++ # 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):
++ # Begin with an acceptable base response...
++ resp = {
++ "device_code": "my-device-code",
++ "user_code": "my-user-code",
++ "interval": 0,
++ "verification_uri": "https://example.com",
++ "expires_in": 5,
++ }
++
++ # ...then tweak it so the client fails.
++ if bad_value is Missing:
++ del resp[field_name]
++ else:
++ resp[field_name] = bad_value
++
++ return 200, resp
++
++ openid_provider.register_endpoint(
++ "device_authorization_endpoint", "POST", "/device", authorization_endpoint
++ )
++
++ def token_endpoint(headers, params):
++ assert False, "token endpoint was invoked unexpectedly"
++
++ openid_provider.register_endpoint(
++ "token_endpoint", "POST", "/token", token_endpoint
++ )
++
++ expect_disconnected_handshake(sock)
+
+ # Now make sure the client correctly failed.
++ if bad_value is Missing:
++ error_pattern = f'field "{field_name}" is missing'
++ elif ok_type == str:
++ error_pattern = f'field "{field_name}" must be a string'
++ elif ok_type == int:
++ error_pattern = f'field "{field_name}" must be a number'
++ else:
++ assert False, "update error_pattern for new failure mode"
++
++ # XXX iddawc doesn't really check for problems in the device authorization
++ # response, leading to this patchwork:
++ if field_name == "verification_uri":
++ error_pattern = alt_patterns(
++ error_pattern,
++ "issuer did not provide a verification URI",
++ )
++ elif field_name == "user_code":
++ error_pattern = alt_patterns(
++ error_pattern,
++ "issuer did not provide a user code",
++ )
++ else:
++ error_pattern = alt_patterns(
++ error_pattern,
++ r"failed to obtain access token: \(iddawc error I_ERROR_PARAM\)",
++ )
++
+ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
+ client.check_completed()
+
@@ src/test/python/client/test_oauth.py (new)
+ "failure_mode, error_pattern",
+ [
+ pytest.param(
-+ {
-+ "error": "expired_token",
-+ "error_description": "the device code has expired",
-+ },
-+ r"the device code has expired \(expired_token\)",
++ (
++ 400,
++ {
++ "error": "expired_token",
++ "error_description": "the device code has expired",
++ },
++ ),
++ r"failed to obtain access token: the device code has expired \(expired_token\)",
+ id="expired token with description",
+ ),
+ pytest.param(
-+ {"error": "access_denied"},
-+ r"\(access_denied\)",
++ (400, {"error": "access_denied"}),
++ r"failed to obtain access token: \(access_denied\)",
+ id="access denied without description",
+ ),
+ pytest.param(
-+ {},
-+ r"OAuth token retrieval failed",
-+ id="broken error response",
++ (400, {}),
++ alt_patterns(
++ r'failed to parse token error response: field "error" is missing',
++ r"failed to obtain access token: \(iddawc error I_ERROR_PARAM\)",
++ ),
++ id="empty error response",
++ ),
++ pytest.param(
++ (200, {}, {}),
++ alt_patterns(
++ r"failed to parse access token response: no content type was provided",
++ r"failed to obtain access token: \(iddawc error I_ERROR\)",
++ ),
++ id="missing content type",
++ ),
++ pytest.param(
++ (200, {"Content-Type": "text/plain"}, {}),
++ alt_patterns(
++ r"failed to parse access token response: unexpected content type",
++ r"failed to obtain access token: \(iddawc error I_ERROR\)",
++ ),
++ id="wrong content type",
+ ),
+ ],
+)
@@ src/test/python/client/test_oauth.py (new)
+ )
+
+ retry_lock = threading.Lock()
++ final_sent = False
+
+ def token_endpoint(headers, params):
+ with retry_lock:
-+ nonlocal retries
++ nonlocal retries, final_sent
+
+ # If the test wants to force the client to retry, return an
+ # authorization_pending response and decrement the retry count.
@@ src/test/python/client/test_oauth.py (new)
+ retries -= 1
+ return 400, {"error": "authorization_pending"}
+
-+ return 400, failure_mode
++ # We should only return our failure_mode response once; any further
++ # requests indicate that the client isn't correctly bailing out.
++ assert not final_sent, "client continued after token error"
++
++ final_sent = True
++
++ return failure_mode
+
+ 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.
-+ startup = pq3.recv1(conn, cls=pq3.Startup)
-+ assert startup.proto == pq3.protocol(3, 0)
++ expect_disconnected_handshake(sock)
+
-+ pq3.send(
-+ conn,
-+ pq3.types.AuthnRequest,
-+ type=pq3.authn.SASL,
-+ body=[b"OAUTHBEARER", b""],
-+ )
++ # Now make sure the client correctly failed.
++ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
++ client.check_completed()
+
-+ # The client should not continue the connection due to the hardcoded
-+ # provider failure; we disconnect here.
++
[email protected](
++ "bad_value",
++ [
++ pytest.param({"device_code": 3}, id="object"),
++ pytest.param([1, 2, 3], id="array"),
++ pytest.param("some string", id="string"),
++ pytest.param(4, id="numeric"),
++ pytest.param(False, id="boolean"),
++ pytest.param(None, id="null"),
++ pytest.param(Missing, id="missing"),
++ ],
++)
[email protected](
++ "field_name,ok_type,required",
++ [
++ ("access_token", str, True),
++ ("token_type", str, True),
++ ],
++)
++def test_oauth_token_bad_json_schema(
++ accept, openid_provider, field_name, ok_type, required, bad_value
++):
++ # To make the test matrix easy, just skip the tests that aren't actually
++ # interesting (field of the correct type, missing optional field).
++ if bad_value is Missing and not required:
++ pytest.skip("not interesting: optional field")
++ elif type(bad_value) == ok_type: # not isinstance(), because bool is an int
++ pytest.skip("not interesting: correct type")
++
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ oauth_client_id=secrets.token_hex(),
++ )
++
++ # 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": "my-device-code",
++ "user_code": "my-user-code",
++ "interval": 0,
++ "verification_uri": "https://example.com",
++ "expires_in": 5,
++ }
++
++ return 200, resp
++
++ openid_provider.register_endpoint(
++ "device_authorization_endpoint", "POST", "/device", authorization_endpoint
++ )
++
++ def token_endpoint(headers, params):
++ # Begin with an acceptable base response...
++ resp = {
++ "access_token": secrets.token_urlsafe(),
++ "token_type": "bearer",
++ }
++
++ # ...then tweak it so the client fails.
++ if bad_value is Missing:
++ del resp[field_name]
++ else:
++ resp[field_name] = bad_value
++
++ return 200, resp
++
++ openid_provider.register_endpoint(
++ "token_endpoint", "POST", "/token", token_endpoint
++ )
++
++ expect_disconnected_handshake(sock)
+
+ # Now make sure the client correctly failed.
++ error_pattern = "failed to parse access token response: "
++ if bad_value is Missing:
++ error_pattern += f'field "{field_name}" is missing'
++ elif ok_type == str:
++ error_pattern += f'field "{field_name}" must be a string'
++ elif ok_type == int:
++ error_pattern += f'field "{field_name}" must be a number'
++ else:
++ assert False, "update error_pattern for new failure mode"
++
++ # XXX iddawc is fairly silent on the topic.
++ error_pattern = alt_patterns(
++ error_pattern,
++ r"failed to obtain access token: \(iddawc error I_ERROR_PARAM\)",
++ )
++
+ with pytest.raises(psycopg2.OperationalError, match=error_pattern):
+ client.check_completed()
+
@@ src/test/python/client/test_oauth.py (new)
+
+
[email protected](
++ "bad_response,expected_error",
++ [
++ pytest.param(
++ (200, {"Content-Type": "text/plain"}, {}),
++ r'failed to parse OpenID discovery document: unexpected content type "text/plain"',
++ id="not JSON",
++ ),
++ pytest.param(
++ (200, {}, {}),
++ r"failed to parse OpenID discovery document: no content type was provided",
++ id="no Content-Type",
++ ),
++ pytest.param(
++ (204, {}, None),
++ r"failed to fetch OpenID discovery document: unexpected response code 204",
++ id="no content",
++ ),
++ pytest.param(
++ (301, {"Location": "https://localhost/"}, None),
++ r"failed to fetch OpenID discovery document: unexpected response code 301",
++ id="redirection",
++ ),
++ pytest.param(
++ (404, {}),
++ r"failed to fetch OpenID discovery document: unexpected response code 404",
++ id="not found",
++ ),
++ pytest.param(
++ (200, RawResponse("blah\x00blah")),
++ r"failed to parse OpenID discovery document: response contains embedded NULLs",
++ id="NULL bytes in document",
++ ),
++ pytest.param(
++ (200, 123),
++ r"failed to parse OpenID discovery document: top-level element must be an object",
++ id="scalar at top level",
++ ),
++ pytest.param(
++ (200, []),
++ r"failed to parse OpenID discovery document: top-level element must be an object",
++ id="array at top level",
++ ),
++ pytest.param(
++ (200, RawResponse("{")),
++ r"failed to parse OpenID discovery document.* input string ended unexpectedly",
++ id="unclosed object",
++ ),
++ pytest.param(
++ (200, RawResponse(r'{ "hello": ] }')),
++ r"failed to parse OpenID discovery document.* Expected JSON value",
++ id="bad array",
++ ),
++ pytest.param(
++ (200, {"issuer": 123}),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="non-string issuer",
++ ),
++ pytest.param(
++ (200, {"issuer": ["something"]}),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="issuer array",
++ ),
++ pytest.param(
++ (200, {"issuer": {}}),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="issuer object",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": 123}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="scalar grant types field",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": {}}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="object grant types field",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": [123]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="non-string grant types",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": ["something", 123]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="non-string grant types later in the list",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": ["something", {}]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="object grant types later in the list",
++ ),
++ pytest.param(
++ (200, {"grant_types_supported": ["something", ["something"]]}),
++ r'failed to parse OpenID discovery document: field "grant_types_supported" must be an array of strings',
++ id="embedded array grant types later in the list",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "grant_types_supported": ["something"],
++ "token_endpoint": "https://example.com/",
++ "issuer": 123,
++ },
++ ),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="non-string issuer after other valid fields",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "ignored": {"grant_types_supported": 123, "token_endpoint": 123},
++ "issuer": 123,
++ },
++ ),
++ r'failed to parse OpenID discovery document: field "issuer" must be a string',
++ id="non-string issuer after other ignored fields",
++ ),
++ pytest.param(
++ (200, {"token_endpoint": "https://example.com/"}),
++ r'failed to parse OpenID discovery document: field "issuer" is missing',
++ id="missing issuer",
++ ),
++ pytest.param(
++ (200, {"issuer": "https://example.com/"}),
++ r'failed to parse OpenID discovery document: field "token_endpoint" is missing',
++ id="missing token endpoint",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "issuer": "https://example.com",
++ "token_endpoint": "https://example.com/token",
++ "device_authorization_endpoint": "https://example.com/dev",
++ },
++ ),
++ r'cannot run OAuth device authorization: issuer "https://example.com" does not support device code grants',
++ id="missing device code grants",
++ ),
++ pytest.param(
++ (
++ 200,
++ {
++ "issuer": "https://example.com",
++ "token_endpoint": "https://example.com/token",
++ "grant_types_supported": [
++ "urn:ietf:params:oauth:grant-type:device_code"
++ ],
++ },
++ ),
++ r'cannot run OAuth device authorization: issuer "https://example.com" does not provide a device authorization endpoint',
++ id="missing device_authorization_endpoint",
++ ),
++ #
++ # Exercise HTTP-level failures by breaking the protocol. Note that the
++ # error messages here are implementation-dependent.
++ #
++ pytest.param(
++ (1000, {}),
++ r"failed to fetch OpenID discovery document: Unsupported protocol \(.*\)",
++ id="invalid HTTP response code",
++ ),
++ pytest.param(
++ (200, {"Content-Length": -1}, {}),
++ r"failed to fetch OpenID discovery document: Weird server reply \(.*Content-Length.*\)",
++ id="bad HTTP Content-Length",
++ ),
++ ],
++)
++def test_oauth_discovery_provider_failure(
++ accept, openid_provider, bad_response, expected_error
++):
++ sock, client = accept(
++ oauth_issuer=openid_provider.issuer,
++ oauth_client_id=secrets.token_hex(),
++ )
++
++ def failing_discovery_handler(headers, params):
++ return bad_response
++
++ openid_provider.register_endpoint(
++ None,
++ "GET",
++ "/.well-known/openid-configuration",
++ failing_discovery_handler,
++ )
++
++ expect_disconnected_handshake(sock)
++
++ # XXX iddawc doesn't differentiate...
++ expected_error = alt_patterns(
++ expected_error,
++ r"failed to fetch OpenID discovery document \(iddawc error I_ERROR(_PARAM)?\)",
++ )
++
++ with pytest.raises(psycopg2.OperationalError, match=expected_error):
++ client.check_completed()
++
++
[email protected](
+ "sasl_err,resp_type,resp_payload,expected_error",
+ [
+ pytest.param(
@@ src/test/python/client/test_oauth.py (new)
+ "server sent additional OAuth data",
+ id="broken server: SASL success after error",
+ ),
++ pytest.param(
++ {"status": "invalid_request"},
++ pq3.types.AuthnRequest,
++ dict(type=pq3.authn.SASL, body=[b"OAUTHBEARER", b""]),
++ "duplicate SASL authentication request",
++ id="broken server: SASL reinitialization after error",
++ ),
+ ],
+)
+def test_oauth_server_error(accept, sasl_err, resp_type, resp_payload, expected_error):
5: 4490d029b5 ! 5: 65c319a6a3 squash! Add pytest suite for OAuth
@@ .cirrus.yml: task:
sysctl kern.corefile='/tmp/cores/%N.%P.core'
setup_additional_packages_script: |
- #pkg install -y ...
-+ pkg install -y iddawc
++ pkg install -y curl
# NB: Intentionally build without -Dllvm. The freebsd image size is already
# large enough to make VM startup slow, and even without llvm freebsd
@@ .cirrus.yml: task:
--buildtype=debug \
-Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-+ -Doauth=enabled \
++ -Doauth=curl \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
EOF
@@ .cirrus.yml: LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
--with-libxslt
--with-llvm
--with-lz4
-+ --with-oauth
++ --with-oauth=curl
--with-pam
--with-perl
--with-python
@@ .cirrus.yml: LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
-+ -Doauth=enabled
++ -Doauth=curl
-Duuid=e2fs
@@ .cirrus.yml: task:
- #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
+ apt-get update
+ DEBIAN_FRONTEND=noninteractive apt-get -y install \
-+ libiddawc-dev \
-+ libiddawc-dev:i386 \
++ libcurl4-openssl-dev \
++ libcurl4-openssl-dev:i386 \
+ python3-venv \
matrix:
@@ .cirrus.yml: task:
- #apt-get update
- #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
+ apt-get update
-+ DEBIAN_FRONTEND=noninteractive apt-get -y install libiddawc-dev
++ DEBIAN_FRONTEND=noninteractive apt-get -y install libcurl4-openssl-dev
###
# Test that code can be built with gcc/clang without warnings
@@ meson.build: foreach test_dir : tests
+ test_group = test_dir['name']
+ test_output = test_result_dir / test_group / kind
+ test_kwargs = {
-+ 'protocol': 'tap',
++ #'protocol': 'tap',
+ 'suite': test_group,
+ 'timeout': 1000,
+ 'depends': test_deps,
@@ meson.build: foreach test_dir : tests
+ pytest = venv_path / 'bin' / 'py.test'
+ test_command = [
+ pytest,
++ # Avoid running these tests against an existing database.
++ '--temp-instance', test_output / 'tmp_check',
++
+ # FIXME pytest-tap's stream feature accidentally suppresses errors that
+ # are critical for debugging:
+ # https://github.com/python-tap/pytest-tap/issues/30
-+ # Fix -- or maybe don't use the meson TAP protocol for now?
-+ '--tap-stream',
-+ # Avoid running these tests against an existing database.
-+ '--temp-instance', test_output / 'tmp_check',
++ # Don't use the meson TAP protocol for now...
++ #'--tap-stream',
+ ]
+
+ foreach pyt : t['tests']
@@ src/test/python/client/test_oauth.py: import pq3
+# The client tests need libpq to have been compiled with OAuth support; skip
+# them otherwise.
+pytestmark = pytest.mark.skipif(
-+ os.getenv("with_oauth") == "no",
++ os.getenv("with_oauth") == "none",
+ reason="OAuth client tests require --with-oauth support",
+)
+
@@ src/test/python/meson.build (new)
+ './test_pq3.py',
+ ],
+ 'env': {
-+ 'with_oauth': oauth.found() ? 'yes' : 'no',
++ 'with_oauth': oauth_library,
+
+ # Point to the default database; the tests will create their own databases
+ # as needed.
[application/gzip] v8-0001-common-jsonapi-support-FRONTEND-clients.patch.gz (6.6K, ../[email protected]/3-v8-0001-common-jsonapi-support-FRONTEND-clients.patch.gz)
download
[application/gzip] v8-0002-libpq-add-OAUTHBEARER-SASL-mechanism.patch.gz (30.1K, ../[email protected]/4-v8-0002-libpq-add-OAUTHBEARER-SASL-mechanism.patch.gz)
download
[application/gzip] v8-0003-backend-add-OAUTHBEARER-SASL-mechanism.patch.gz (12.4K, ../[email protected]/5-v8-0003-backend-add-OAUTHBEARER-SASL-mechanism.patch.gz)
download
[application/gzip] v8-0004-Add-pytest-suite-for-OAuth.patch.gz (31.9K, ../[email protected]/6-v8-0004-Add-pytest-suite-for-OAuth.patch.gz)
download
[application/gzip] v8-0005-squash-Add-pytest-suite-for-OAuth.patch.gz (7.5K, ../[email protected]/7-v8-0005-squash-Add-pytest-suite-for-OAuth.patch.gz)
download
view thread (27+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox