Fix order-dependent test failures from shared JWKS cache - #246
Open
pouwerkerk wants to merge 1 commit into
Open
Conversation
The module-global __jwkcache in clerk_backend_api.security.verifytoken
is keyed only by JWT kid, so once any test verifies a session token with
a valid secret key, the cached PEM lets every later test skip the JWKS
fetch entirely. This made the credentialed error-path tests
order-dependent: TestJwtVerificationAsync::test_verify_token_invalid_secret_key
expects JWK_FAILED_TO_LOAD from a network round-trip that never happens
after the sync TestJwtVerification::test_verify_token_remote_ok has
warmed the cache.
Deterministic repro (requires CLERK_SECRET_KEY and CLERK_SESSION_TOKEN):
pytest "tests/test_verify_token.py::TestJwtVerification::test_verify_token_remote_ok" \
"tests/test_verify_token.py::TestJwtVerificationAsync::test_verify_token_invalid_secret_key"
Add an autouse fixture that clears the cache before each test so every
test starts cold. No production code is touched; the uncredentialed run
is unchanged (59 passed, 14 skipped before and after).
Contributor
|
Thank you for your contribution!
I think it's a bug and we already had a ticket for it. I'll have the PR up shortly. We can then see if your PR is still needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The credentialed test suite has one test that fails or passes depending on execution order. This PR fixes it by clearing the JWKS cache between tests. It touches only
tests/conftest.py, which is hand-maintained (not listed in.speakeasy/gen.lock). No generated or production code changes.Steps to reproduce
With
CLERK_SECRET_KEYand a freshCLERK_SESSION_TOKENset, run these two tests in the same pytest process, in this order (the same order the full suite uses):Expected and actual behavior
Expected:
test_verify_token_invalid_secret_keycallsverify_token_asyncwithsecret_key='sk_test_invalid', the JWKS fetch gets a 401, and the test seesJWK_FAILED_TO_LOAD.Actual: the test fails with
DID NOT RAISE(or, if the session token has expired, withTOKEN_EXPIRED). Run alone, the same test passes.Root cause
src/clerk_backend_api/security/verifytoken.pykeeps a module-global__jwkcacheshared by the sync and async paths and keyed only by JWTkid. The synctest_verify_token_remote_okruns earlier in the suite, fetches the JWKS with the valid secret key, and caches the PEM. When the async invalid-secret-key test runs,_get_remote_jwt_key_asyncfinds the cached PEM and returns it without contacting Clerk, so the invalid key is never used and the expected error never happens.This is the cache working as designed in production. The problem is test isolation: nothing resets the cache between tests, so the error-path tests only pass from a cold start. The sync twin of this test passes today only because it happens to run before
remote_ok.Fix
An autouse fixture in
tests/conftest.pythat clears the cache before each test. Results with credentials: 71 passed, 2 skipped (was 1 failed, 70 passed, 2 skipped). Without credentials: 59 passed, 14 skipped, same as before, so CI and uncredentialed runs are unaffected. Verified identical under cryptography 48.0.0 and 50.0.0; the failure predates any dependency changes and is unrelated to #245.Environment
Python 3.12.12, macOS (darwin arm64), pytest 8.x, pytest-asyncio 0.24.0,
PyJWT 2.13.0. Reproduced with cryptography 48.0.0 and 50.0.0.