Skip to content

Scope DPoP nonces per request instead of mutating shared opts#567

Merged
kevinlzw merged 2 commits into
masterfrom
fix/dpop-nonce-request-scoping
Jun 21, 2026
Merged

Scope DPoP nonces per request instead of mutating shared opts#567
kevinlzw merged 2 commits into
masterfrom
fix/dpop-nonce-request-scoping

Conversation

@zandbelt

Copy link
Copy Markdown
Contributor

Summary

DPoP nonces were cached on the opts table (opts.dpop_token_nonce / opts.dpop_userinfo_nonce). When a deployment defines opts once at module/init scope and reuses it across requests — a common, documented pattern — that is shared mutable state across concurrent requests in a worker. The sharpest edge was the refresh-path seed:

opts.dpop_token_nonce = session:get("dpop_token_nonce") or opts.dpop_token_nonce

The or opts.dpop_token_nonce fallback means that if the current user's session has no nonce, the request inherits whatever nonce the previous request left on opts — i.e. one user's DPoP nonce bleeding into another user's token request.

The durable, per-user nonce already lives in the session (session:set/get("dpop_token_nonce")); the opts.dpop_*_nonce fields were only a transient bridge from session → call_token_endpoint. This PR moves that transient state into request-scoped ngx.ctx via two small helpers (openidc_get_dpop_nonce / openidc_set_dpop_nonce). The session persistence and the static, caller-provided opts.dpop_nonce seed are untouched, so no legitimate cross-request optimization is lost — the session still carries the token nonce between requests.

Regression test

Added a test driving two independent sessions through one server and asserting the second session's token request does not inherit the first's nonce.

Because the test harness rebuilds opts per request, the shared-opts leak can't be reproduced as-is, so the test uses a new opt-in share_oidc_opts harness flag (default off; all other tests unaffected) that reuses a single opts table across requests, modeling the "define opts once" deployment. The test fails against the previous opts-mutating code (second_payload.nonce == "session-one-nonce") and passes now.

Checklist

  • Tests pass with docker build -f tests/Dockerfile . -t lua-resty-openidc/test
  • Tests pass with docker run -t --rm lua-resty-openidc/test:latest550 successes / 0 failures / 0 errors / 1 pending (the pending is the pre-existing upstream test)
  • ChangeLog is updated

DPoP nonces were cached on the opts table (opts.dpop_token_nonce /
opts.dpop_userinfo_nonce). When a deployment defines opts once at
module/init scope and reuses it across requests, this is shared mutable
state across concurrent requests; the `or opts.dpop_token_nonce` seed in
the refresh path could carry one session's nonce into another request.

Move the transient, per-exchange nonce into request-scoped ngx.ctx via
openidc_get_dpop_nonce/openidc_set_dpop_nonce. The durable per-user token
nonce continues to live in the session (the legitimate cross-request
store), and opts.dpop_nonce remains a static, caller-provided seed, so no
optimization is lost.

Add a regression test driving two independent sessions through one server
with a shared opts table (new share_oidc_opts harness flag) and asserting
the second session's token request does not inherit the first's nonce.
The test fails against the previous opts-mutating code and passes now.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a concurrency issue where DPoP nonces could leak across requests when deployments reuse a single opts table (module/init-scoped), by moving transient DPoP nonce state out of opts and into request-scoped storage. It also extends the test harness to model shared opts tables across requests and adds a regression test for the nonce leak.

Changes:

  • Store per-exchange DPoP nonces in request-scoped state (instead of mutating opts.dpop_*_nonce).
  • Add a test harness flag to reuse a single opts table across requests.
  • Add a regression test ensuring two independent sessions don’t share token-request DPoP nonces.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
lib/resty/openidc.lua Moves transient DPoP nonce handling from shared opts into request-scoped storage.
tests/spec/test_support.lua Adds share_oidc_opts support to reuse the same opts table across requests in tests.
tests/spec/dpop_spec.lua Adds a regression test for cross-session nonce leakage when opts is shared.
ChangeLog Documents the behavioral/security fix for scoping DPoP nonces to a request.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/resty/openidc.lua
Comment thread tests/spec/test_support.lua
Add share_oidc_opts to the start_server custom_config option list so test
authors can discover it. Comment-only change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kevinlzw

Copy link
Copy Markdown
Collaborator

Hi @zandbelt ,

Thanks for putting this together. The approach looks good to me: keeping the transient DPoP nonce in ngx.ctx avoids mutating shared opts, while preserving the session-backed token nonce behavior.

The regression test also nicely covers the shared-opts deployment pattern that exposed the issue.

Would you be comfortable with me merging this?

@zandbelt

Copy link
Copy Markdown
Contributor Author

yes, feel free to merge; also contact me at my e-mail to receive a luarocks API key for uploading a new release when you deem it ready

@kevinlzw

Copy link
Copy Markdown
Collaborator

Hi @zandbelt ,

Thank you. I will email you when it is ready to release.

@kevinlzw kevinlzw merged commit cdc7c7a into master Jun 21, 2026
2 checks passed
@kevinlzw kevinlzw deleted the fix/dpop-nonce-request-scoping branch June 21, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants