Feat/dpop by Avantol13 · Pull Request #113 · uc-cdis/authutils

Avantol13 · 2026-08-21T14:58:17Z

New Features

DPoP implementation for clients and servers
Improved JWT validation without requiring Flask as a dependency, async support
denylist_callback provided as a hook to validation

Breaking Changes

core.validate_jwt: issuers renamed to allowed_issuers, now required and
rejected when empty - there is no way to skip the issuer check. aud and
scope remain required, as before. New purpose and denylist_callback params.
core.validate_jwt positional slots past 5 shifted: slot 6 was options, now
purpose; slot 7 was logger, now options. Move those to keywords.
Flask token.validate.validate_jwt now raises ValueError when none of
OIDC_ISSUER/USER_API/BASE_URL is configured, instead of failing every token
with JWTError.
Exception messages no longer wrap the pyjwt exception (JWTAudienceError reports
got unknown; JWTExpiredError → "token has expired"). Don't match on text.
In the new async key path, a kid the issuer does not publish is an error rather
than falling back to another key.

Bug Fixes

token/fastapi.py: the issuer derived from the first request leaked into every
later request, so one token with a junk iss would 403 all subsequent valid
tokens for the life of the process. Now per-request.
JWK n/e are strictly base64url-decoded. Previously a malformed JWKS silently
produced a junk key, so every token read as "bad signature" rather than
"issuer published a bad key".
keys.py handles both .well-known JWKS and legacy Fence /jwt/keys formats;
10s timeout on key discovery; the public-key cache is mutex-guarded.
Updated all pre-commit tool versions (black 26.5.1 reformat).

Improvements

Replace httpx (no longer developed) with httpx2.
Updated all pre-commit tool versions

Dependency updates

Added joserfc >=1.7.3 (+ truststore, transitive via httpx2)
httpx >=0.23,<1.0 → httpx2 >=2.9.1; pyjwt >=2.4.0 → >=2.11.0

Deployment changes

New env var config for DPoP: DPOP_SHARED_SECRET - required by any service that mints or verifies DPoP
nonces, MUST BE THE SAME value across ALL participating services.
New config for DPoP: DPOP_NONCE_TTL - optional, seconds, default 300.

…ical non-general flask validation. more tests

…n be passed from config)

github-actions · 2026-08-21T14:58:57Z

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

coveralls · 2026-08-21T15:00:39Z

Coverage Report for CI Build 37072616144

Coverage increased (+20.0%) to 80.283%

Details

Coverage increased (+20.0%) from the base build.
Patch coverage: No coverable lines changed in this PR.
34 coverage regressions across 4 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

34 previously-covered lines in 4 files lost coverage.

File	Lines Losing Coverage	Coverage
authutils/token/core.py	11	88.78%
authutils/token/keys.py	11	94.44%
authutils/oauth2/client/blueprint.py	9	0.0%
authutils/globals.py	3	0.0%

Coverage Stats


Relevant Lines:	989
Covered Lines:	794
Line Coverage:	80.28%
Coverage Strength:	0.8 hits per line

💛 - Coveralls

paulineribeyre · 2026-09-10T22:22:24Z

+        raise ValueError(
+            f"htu mismatch: request URL '{actual_url}' != proof htu '{htu_value}'"
+        )


I got this error during my testing, with:

actual_url:
https://pauline.planx-pla.net/workflows/s3/gen3wf-pauline-planx-pla-net-244/ga4gh-tes/5a/60b64fd64fa67b8371c5ded4a3e1e4/outputs/img-1.png

proof htu:
https://pauline.planx-pla.net/workflows/s3/gen3wf-pauline-planx-pla-net-244/ga4gh-tes/5a/60b64fd64fa67b8371c5ded4a3e1e4//outputs/img-1.png'

I fixed it on my side by getting rid of the double slash, but maybe there's a way to make this more robust

imo it shouldn't be more robust here. b/c technically, foobar//outputs/img-1.png is a different resource than foobar/outputs/img-1.png. We tend to be forgiving in our APIs with trailing slashes, but empty resource paths are legitimately allowed and are technically different resources.

Do you know why that happened in the first place? I'd rather fix the source of accidental double-slashes than make the validation itself more flexible.

I added an extra slash when building the key in my call to (boto3) s3_client.get_object(bucket, key)

Leaving as-is unless you think we shouldn't. working on adding percent decoding based on feedback from the gen3 workflow PR though

I'm still a bit unsure about this one but you know dpop better so i trust your judgement

…to httpx2 call

nss10

Some questions and minor change suggestions

nss10 · 2026-09-24T13:54:07Z

+            f"aud must be string, list or None. Instead received aud of type {type(aud)}"
        )
-    if not isinstance(scope, set) and not isinstance(scope, list) and not scope is None:
+    if not isinstance(scope, set) and not isinstance(scope, list) and scope is not None:


Suggested change

if not isinstance(scope, set) and not isinstance(scope, list) and scope is not None:

if scope is not None and not isinstance(scope,(set,list)):

good call, will fix

nss10 · 2026-09-24T17:01:30Z

+    # get rid of any prefixed 'DPoP ' / 'dpop '
+    dpop_header = _strip_auth_scheme(dpop_header)


Why do it twice? Is this for future proofing?

Currently, extract_and_validate_jwk is called only once in the entire library, and the dpop_header used there has already stripped away the auth scheme.

it's for direct callers. extract_and_validate_jwk is public for clients to call, so someone can hand it the raw DPoP eyJ... header value and it should still work. the strip up in validate_dpop_proof is needed separately b/c _verify_signature_and_claims uses dpop_header too. calling it on an already-stripped value is a no-op (returns early when there's no space), so it should be safe

I noticed the same thing. I was going to comment that you can remove the _strip_auth_scheme call from validate_dpop_proof since it's called in extract_and_validate_jwk

nss10 · 2026-09-24T18:17:48Z

+    if "exp" in claims_dict:
+        if current_time >= claims_dict["exp"]:
+            raise ValueError("Invalid DPoP proof: expired")


Suggested change

if "exp" in claims_dict:

if current_time >= claims_dict["exp"]:

raise ValueError("Invalid DPoP proof: expired")

if "exp" in claims_dict and if current_time >= claims_dict["exp"]:

raise ValueError("Invalid DPoP proof: expired")

cleaner 👍 will apply by hand b/c the suggestion has a stray if

Oops! Sorry! :D

nss10 · 2026-09-24T19:50:09Z

+
+    # Normalize BOTH sides so that an explicit default port (e.g. :443 added by
+    # a reverse proxy) does not cause a mismatch.
+    actual_url = _get_url(request_url)


Why a new actual_url variable? Can't we simply reuse request_url?

actual_url isn't the same value as request_url. It's _get_url(request_url), so the query and fragment are gone, the scheme and host are lowercased, and a default :443/:80 is dropped. Both the comparison and the error message use that normalized form. We could write request_url = _get_url(request_url) instead, but then request_url would mean the raw input above that line and the normalized value below it, which is harder to follow. I'd rather keep them separate. I can rename it to normalized_request_url if that reads better

nss10 · 2026-09-24T22:53:02Z

+      algorithm" - which includes symetric algs not allowed.
+    """
+
+    max_header_length = 4096


Shouldn't this be in sync with MAX_DPOP_HEADER_LENGTH

they bound different things - MAX_DPOP_HEADER_LENGTH is the whole header value (header.payload.signature) checked before we parse anything, while joserfc's max_header_length only applies to the first part of the JWT (the header). so it has to be smaller. will add to the docstring so that's clear

nss10 · 2026-09-24T23:00:09Z

+        assert ticks > 5, f"event loop appears to have been blocked (ticks={ticks})"
+
+
+_JWKS_URL = "https://example.com/issuer/.well-known/jwks.json"


You might want to hoist this to the top of the file, since it is used above.

good catch, will move it up

nss10 · 2026-09-24T23:00:45Z

+
+
+@pytest.mark.anyio
+@pytest.mark.anyio


Duplicate decorator alert!

🤦 will remove

nss10 · 2026-09-24T23:03:42Z

+        result1 = await _get_key(tokens[0], allowed_issuers=issuer)
+        result2 = await _get_key(tokens[1], allowed_issuers=issuer)


Super minor! But, result1 and result2 are never used. Maybe add another assert verifying their values? or maybe we can skip storing them altogether. maybe?

will drop these here and add a new test. these tests are about cache, and both kids share the same key material in this fixture, so asserting on the values wouldn't prove anything

Co-authored-by: Sai Shanmukha Narumanchi <nss10@outlook.com>

paulineribeyre · 2026-10-02T20:39:40Z

I did not review the tests

paulineribeyre · 2026-10-05T15:33:30Z

+[tool.pytest.ini_options]
+# Every async test runs on asyncio without a per-test mark. The library is
+# asyncio-only (token/fastapi.py depends on asyncio directly).
+asyncio_mode = "auto"


paulineribeyre

lgtm, did not review the tests

Avantol13 added 27 commits July 15, 2026 14:33

feat(dpop): initial dpop support

de224f0

feat(dpop): return more info in validate call

7df2e93

feat(jwt): validate handles pur, and denylist, more closely match typ…

af233c8

…ical non-general flask validation. more tests

fix(dpop): handle rsa keys, improve return of dpop functions, more tests

a8ae229

feat(secret): allow passing through shared secret for nonce (so it ca…

74a9682

…n be passed from config)

chore(deps): increase min version of pyjwt for required types

a6690eb

chore(dpop): make compute_ath public

fd68324

fix(nonce): return new nonce on error

b6ef43d

chore(logs): debug logs

33e7e9b

fix(dpop): helpful error with invalid token info passed

04f5df5

fix(dpop): correct nonce error handling per the spec

a3e2727

chore(dpop): debug lines

ca83aba

chore(dpop): update error

5f3b0c3

temp(dpop): debug lines

ce2c490

fix(dpop): handle dpop prefix in header

7794485

chore(dpop): debug lines

7457116

fix(dpop): safely handle dpop header

b2120bc

chore(dpop): log ath

babc33f

chore(dpop): more logs

5f2ca22

chore(dpop): increase timeout and more logs on public key fetching

54a1d71

fix(keys): handle custom and official public keys endpoints

68b32d9

chore(logs): update log levels

ea5257a

fix(dpop): increase public keys timeout

a5aefff

chore(dpop): more logs

9e9db82

chore(dpop): more logging

62e0d66

chore(tools): update and small black fixes

ac55e55

feat(dpop): migrate to httpx2, edge cases, cleanup, tests

86cdf9a

fix(issuers): require allowed_iss, don't blindly allow all

fbdd95b

Avantol13 mentioned this pull request Aug 21, 2026

Feat/dpop uc-cdis/gen3sdk-python#314

Merged

paulineribeyre reviewed Sep 10, 2026

View reviewed changes

Avantol13 and others added 2 commits September 14, 2026 10:32

Merge branch 'master' into feat/dpop

ab96c87

fix(security): jti callback last, clarify error, add missing timeout …

5066d08

…to httpx2 call

paulineribeyre requested a review from nss10 September 16, 2026 16:15

nss10 requested changes Sep 28, 2026

View reviewed changes

Avantol13 and others added 6 commits September 30, 2026 11:47

chore(deps): relock

b0a7958

fix(dpop): percent-decode for url comparisons, add tests

3ddae3f

Update src/authutils/token/core.py

9149ad9

Co-authored-by: Sai Shanmukha Narumanchi <nss10@outlook.com>

Update src/authutils/errors.py

8d85fcc

Co-authored-by: Sai Shanmukha Narumanchi <nss10@outlook.com>

Update tests/test_jwt.py

22192cd

Co-authored-by: Sai Shanmukha Narumanchi <nss10@outlook.com>

fix(dpop): various fixes from review

5097241

paulineribeyre reviewed Oct 2, 2026

View reviewed changes

Avantol13 added 2 commits October 2, 2026 17:14

fix(dpop): fix based on reviews

87ad2f9

feat(tests): remove anyio, use asyncio

12bd550

paulineribeyre reviewed Oct 5, 2026

View reviewed changes

paulineribeyre approved these changes Oct 5, 2026

View reviewed changes

nss10 approved these changes Oct 5, 2026

View reviewed changes

Avantol13 merged commit ffa4656 into master Oct 6, 2026
8 checks passed

Avantol13 deleted the feat/dpop branch October 6, 2026 19:30

	if not isinstance(scope, set) and not isinstance(scope, list) and scope is not None:
	if scope is not None and not isinstance(scope,(set,list)):

		# get rid of any prefixed 'DPoP ' / 'dpop '
		dpop_header = _strip_auth_scheme(dpop_header)

		assert ticks > 5, f"event loop appears to have been blocked (ticks={ticks})"


		_JWKS_URL = "https://example.com/issuer/.well-known/jwks.json"

		result1 = await _get_key(tokens[0], allowed_issuers=issuer)
		result2 = await _get_key(tokens[1], allowed_issuers=issuer)



		@pytest.mark.anyio
		@pytest.mark.anyio

Conversation

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

Uh oh!

New Features

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

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

Uh oh!

Coverage Report for CI Build 37072616144

Coverage increased (+20.0%) to 80.283%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

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!

nss10 left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Avantol13 commented Aug 21, 2026 •

edited

Loading

coveralls commented Aug 21, 2026 •

edited

Loading

paulineribeyre commented Oct 2, 2026 •

edited

Loading