QPU auth management by sarjevane · Pull Request #79 · pasqal-io/warden

sarjevane · 2026-07-29T18:30:30Z

No description provided.

github-actions · 2026-07-29T18:32:24Z

⚠️ critical vulnerabilities were detected in generated SBOM artifacts

SBOM vulnerability check

⚠️ Found 1 critical vulnerability entries across SBOM artifacts.

Total vulnerabilities observed: 1

`warden-sbom-mariadb.cdx.json`

Severity breakdown: CRITICAL: 1
Critical vulnerabilities:

Vulnerability	Severity	Component(s)	Reference
`CVE-2025-65896`	CRITICAL	`asyncmy@0.2.11`	link

`warden-sbom-pg.cdx.json`

No vulnerabilities listed in this SBOM.

`warden-sbom.cdx.json`

No vulnerabilities listed in this SBOM.

Cache for half the lifespan (with a WARNING) instead of disabling the cache entirely when expires_in <= leeway_s. Also catch AttributeError alongside ValueError when parsing a non-object token error body, narrow the unlocked-cache ponytail comment to the async path where the race is actually reachable, tighten config.py's auth_flow typing, note that QPUClientRequestError subclasses can propagate through retry(), and add coverage for the no-auth-header and POST-401-replay paths.

badtst

LGTM from my (limited) understanding of Keycloak

badtst · 2026-08-03T14:25:14Z

+  # auth:
+  #   # Keycloak base URL (not the token endpoint; the OIDC path is appended).
+  #   url: http://keycloak:8080
+  #   realm: pasqos


Should we leave the pasqos example name visible in user-facing documentation ? At the same time it is the correct realm name

badtst · 2026-08-03T15:11:29Z

+        request.headers["Authorization"] = f"Bearer {self._sync_token()}"
+        response = yield request
+        if response.status_code == httpx.codes.UNAUTHORIZED:
+            logger.info("QPU API returned 401, refreshing token and retrying once")


Should those logs be INFO or should they be hidden in DEBUG level, token failures are documented by raising an exception

badtst · 2026-08-03T15:15:37Z

        return self.tls_verify

+    @property
+    def auth_flow(self) -> httpx.Auth | None:


nit: maybe rename to something like qpu_auth to not avoid confusion with the auth_flow method of httpx.Auth

badtst · 2026-08-03T15:27:47Z

+    url: str
+    realm: str = "pasqos"
+    # OIDC client_id
+    id: str
+    # OIDC client_secret. Provide via WARDEN_QPU_AUTH_SECRET, never in YAML.
+    secret: str
+    # Refresh this many seconds before the token actually expires.
+    leeway_s: float = 30


nit: Avoid in code config default values ?

badtst · 2026-08-03T15:58:50Z

+    token_requests = [r for r in httpx_mock.get_requests() if str(r.url) == TOKEN_URL]
+    assert len(token_requests) == 1


nit: IIRC if a mock response does not have is_reusable set to True it will raise an error if it is called more than once (httpx.TimeoutException because httpx_mock will not re-send the response to an already matched request)

If a response is not requestes, an error is raised even if the test passes

Suggested change

token_requests = [r for r in httpx_mock.get_requests() if str(r.url) == TOKEN_URL]

assert len(token_requests) == 1

badtst · 2026-08-03T16:03:47Z

+    qpu_requests = [r for r in httpx_mock.get_requests() if str(r.url) == QPU_URL]
+    assert len(qpu_requests) == 2


nit: IIRC not needed

Suggested change

qpu_requests = [r for r in httpx_mock.get_requests() if str(r.url) == QPU_URL]

assert len(qpu_requests) == 2

badtst · 2026-08-03T16:06:27Z

+    assert second.request.headers["Authorization"] == "Bearer tok-2"
+
+
+def test_401_triggers_one_refresh_and_one_retry(httpx_mock: HTTPXMock, auth_conf):


Isn't there a way to know that the 401 is specifically triggered by the token being stale ?

badtst · 2026-08-03T16:10:23Z

+    # expires_in 30 with the default leeway_s 30 would clamp to 0 without the
+    # half-lifespan fallback, disabling the cache entirely and forcing a
+    # Keycloak round-trip on every request.


nit: same comment on leeway as above

badtst · 2026-08-03T16:11:22Z

+    token_requests = [r for r in httpx_mock.get_requests() if str(r.url) == TOKEN_URL]
+    assert len(token_requests) == 1


nit: same as above

badtst · 2026-08-03T16:11:37Z

+    assert response.status_code == 200
+    assert response.request.headers["Authorization"] == "Bearer fresh"
+    qpu_requests = [r for r in httpx_mock.get_requests() if str(r.url) == QPU_URL]
+    assert len(qpu_requests) == 2


nit:same as above

badtst · 2026-08-04T07:29:17Z

+# Deliberately generous: this budget is only ever spent by a test that is
+# already failing, so it costs nothing on the happy path. Tight per-test budgets
+# turned a slow CI runner into a flake instead of catching anything.
+JOB_WAIT_TIMEOUT_S = 30


Maybe it is a bit long for a test fail in a local environment. Maybe we can set it to something lower like 10 in a local dev environment and switch it to 30 in CI ?

Suggested change

JOB_WAIT_TIMEOUT_S = 30

JOB_WAIT_TIMEOUT_S = 30 if "CI" in os.environ else 10

MatthieuMoreau0 · 2026-08-05T09:22:48Z

+            assert self._token is not None
+            return self._token
+        url, data = self._token_request()
+        with httpx.Client(verify=self.verify) as client:


We could reuse the same http client to reuse http pool connection across token requests

MatthieuMoreau0 · 2026-08-05T09:22:52Z

+            assert self._token is not None
+            return self._token
+        url, data = self._token_request()
+        async with httpx.AsyncClient(verify=self.verify) as client:


same comment

MatthieuMoreau0 · 2026-08-05T09:24:29Z


 logger = logging.getLogger(__name__)

+TERMINAL_STATUSES: tuple[JobStatus, ...] = ("ERROR", "DONE", "CANCELED")


nit: why not use the JobStatus enum values instead of strings?

MatthieuMoreau0 · 2026-08-05T09:29:06Z

+        if self._status in TERMINAL_STATUSES and not was_terminal:
+            # Logged here rather than by the caller so that the closing line is
+            # part of the same JobUpdate - hence the same transaction - as the
+            # terminal status. Flushed separately, the DB would briefly hold a
+            # finished job whose logs are truncated, and anything that stops
+            # polling once the status is terminal reads incomplete logs.
+            logger.info("Job execution ended with status '%s'", self._status)


nit: This final log line was moved out of line 169, so it won't appear for job who time out and the cancelation fail because the same logic has not been added to to_error method. Is that intentional?

MatthieuMoreau0 · 2026-08-05T09:37:34Z

+        expires_in = float(payload.get("expires_in", 0))
+        self._token = token
+        ttl = max(expires_in - self.conf.leeway_s, expires_in / 2)
+        if expires_in <= self.conf.leeway_s:


The warning log is not emitted for all cases where we use ttl=expires_in/2.

e.g. if expires_in=40, leeway=30 then ttl=20 and yet expires_in>leeway

Suggested change

if expires_in <= self.conf.leeway_s:

if ttl > expires_in - self.conf.leeway_s:

MatthieuMoreau0 · 2026-08-05T09:44:14Z

        NotRetriedHTTPStatus: If the HTTP request returns with a non-retryable error code.
        MaxRetryError: If the maximum number of retries without success has been reached.
+        QPUClientRequestError: Any subclass already classified as non-retryable
+            by the wrapped function (e.g. TokenRequestError) propagates unchanged.


That's not true in the case of no_retry=True, see line 82

MatthieuMoreau0 · 2026-08-05T09:44:56Z

LGTM, I haven't tested locally tbh but I believe Thomas has already confirmed the proper integration with local pasqos using keycloak

MatthieuMoreau0 · 2026-08-12T07:56:48Z

I finally tested locally and confirmed this works as expected 👍

sarjevane self-assigned this Jul 29, 2026

sarjevane force-pushed the sg/poc-pasqos-auth branch 4 times, most recently from 428aca8 to 514b4e4 Compare July 30, 2026 13:06

sarjevane added 7 commits July 31, 2026 14:53

feat(auth): add QPUAuthConfig for Keycloak client credentials

b6ab7a5

feat(auth): add Keycloak client_credentials httpx auth flow

c46f8ef

feat(auth): attach Keycloak auth flow to QPU API clients

d241f41

fix(retry): do not rewrap already-classified QPU client errors

e9773b3

docs(auth): document qpu.auth config block

4f5eb8c

fix(typechecking): Fix type-checking error

a84c49e

sarjevane force-pushed the sg/poc-pasqos-auth branch from 514b4e4 to a84c49e Compare July 31, 2026 12:53

sarjevane requested review from MatthieuMoreau0 and badtst July 31, 2026 12:54

sarjevane marked this pull request as ready for review July 31, 2026 12:54

fix(test): Fix test flakiness

0982676

badtst approved these changes Aug 3, 2026

View reviewed changes

badtst reviewed Aug 4, 2026

View reviewed changes

MatthieuMoreau0 approved these changes Aug 5, 2026

View reviewed changes

		token_requests = [r for r in httpx_mock.get_requests() if str(r.url) == TOKEN_URL]
		assert len(token_requests) == 1

		qpu_requests = [r for r in httpx_mock.get_requests() if str(r.url) == QPU_URL]
		assert len(qpu_requests) == 2

		assert second.request.headers["Authorization"] == "Bearer tok-2"


		def test_401_triggers_one_refresh_and_one_retry(httpx_mock: HTTPXMock, auth_conf):

	JOB_WAIT_TIMEOUT_S = 30
	JOB_WAIT_TIMEOUT_S = 30 if "CI" in os.environ else 10


		logger = logging.getLogger(__name__)

		TERMINAL_STATUSES: tuple[JobStatus, ...] = ("ERROR", "DONE", "CANCELED")

	if expires_in <= self.conf.leeway_s:
	if ttl > expires_in - self.conf.leeway_s:

Uh oh!

Conversation

sarjevane commented Jul 29, 2026

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SBOM vulnerability check

warden-sbom-mariadb.cdx.json

warden-sbom-pg.cdx.json

warden-sbom.cdx.json

Uh oh!

badtst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

badtst Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthieuMoreau0 commented Aug 5, 2026

Uh oh!

MatthieuMoreau0 commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 29, 2026 •

edited

Loading

`warden-sbom-mariadb.cdx.json`

`warden-sbom-pg.cdx.json`

`warden-sbom.cdx.json`

badtst Aug 3, 2026 •

edited

Loading