Improve DPoP nonce caching for manually constructed responses from customFetch

[RyotaUshio]

Hi! First of all, thank you for creating and maintaining this library. It’s been incredibly helpful.

I’d like to propose a small improvement around DPoP nonce caching. As a user of the library, I ran into a pitfall when using customFetch that I think could be addressed with a minimal API adjustment or, alternatively, a documentation note.

Context

In my use case, I must return a manually constructed Response from customFetch (i.e., one created with new Response(...)). As noted in the comment here, that can interfere with DPoP nonce caching:

oauth4webapi/src/index.ts

Lines 218 to 219 in fa7414e

	* - Returning self-constructed {@link !Response} instances prohibits AS/RS-signalled DPoP Nonce
	* caching.

The issue appears to stem from DPoPHandle / DPoPHandler’s cacheNonce using response.url as the cache key. Unlike a Response returned by the platform fetch, a Response constructed via new Response(...) does not have its url property set, so the cacheNonce method fails to compute the URL to be used as the cache key and the nonce is not cached.

oauth4webapi/src/index.ts

Lines 2335 to 2342 in fa7414e

	cacheNonce(response: Response): void {
	try {
	const nonce = response.headers.get('dpop-nonce')
	if (nonce) {
	this.#set(new URL(response.url).origin, nonce)
	}
	} catch {}
	}

Proposal

Allow cacheNonce to accept the request URL explicitly, in addition to the Response. For example:

cacheNonce(response: Response, url: URL): void {
    try {
      const nonce = response.headers.get('dpop-nonce')
      if (nonce) {
        this.#set(url.origin, nonce)
      }
    } catch {}
}

By passing the correct URL even when response.url is unset, we can reliably cache nonces for manually constructed responses. From reading the current code, the code that invokes it already has access to the URL object, so this change should require minimal changes

Alternative (Docs-only mitigation)

If changing the method signature is not feasible, could we at least add a note to the customFetch comment showing how to avoid the cache miss by setting the url property on a self-constructed response? For example:

const response = new Response(/* ... */);
Object.defineProperty(response, 'url', { value: url }); // set url explicitly

Documenting this workaround would likely save other users time when they must return a synthetic Response.

---

If you’re open to either approach (API change or docs note), I’m happy to open a PR. Please let me know what you think!

[panva]

so this change should require minimal changes

Except that cacheNonce is not part of the public API.

Alternative (Docs-only mitigation)

I'm kinda not a fan.

Honestly, when you're running into nonce cache issues due to self-contructed Response objects (that the docs do elude to), i would expect you to keep the cache outside yourself and use DPoP's modify assertion option to add the nonce before it is signed. That is already part of the public API.

[RyotaUshio]

Thank you for the response!

Except that cacheNonce is not part of the public API.

Sorry my wording was misleading. I suggested changing the cacheNonce signature and also updating the all places in oauth4webapi that is calling cacheNonce. So I'm aware that it's not part of the public API, and I did not intend this as a public API change. I was trying to suggest changing the inner workings of the library a bit.

I imagine it could be beneficial for many users if oauth4webapi handled nonce caching in this way, but if this change is not acceptable, I'm totally good with your suggested ModifyAssertionOptions approach (thank you for the pointer!).

[panva]

Got it, thanks that's a great suggestion, I'll look into it.

[panva]

https://github.com/panva/oauth4webapi/releases/tag/v3.8.2