Add optional headers for fetch_jwk

[rolandzwaga]

Similar to the agent option, this simply adds optional http headers to be sent with the fetch_wks http request.

[panva]

Why do you need this?

[rolandzwaga]

Why do you need this?

In my current project I have the use-case where I need to send a custom header along with the request to the keystore otherwise it gets blocked.

[panva]

• Only push changes related to the feature. Reordering imports seems to have been done by the editor and makes reviewing difficult.
• You also have to accommodate the browser runtime as well
• Tests are needed

[rolandzwaga]

• Only push changes related to the feature. Reordering imports seems to have been done by the editor and makes reviewing difficult.

You're absolutely right, I apologize and can fix that easily

• You also have to accommodate the browser runtime as well

I followed the implementation of the agent option, which is also only implemented for the node runtime (AFAICS). I can still add it to the browser runtime though, but shall I add the agent option then as well while I'm at it?

• Tests are needed

I followed the implementation of the agent option, and couldn't find any unit tests for this. I tried to grok the current unit test setup but didn't find it so easy to quickly grasp. Maybe I overlooked though, so if you can point me to the tests for the agent option, it will probably be easier for me to add tests for the headers.

[panva]

but shall I add the agent option then as well while I'm at it?

Is the node-specific http(s).agent option applicable to browser runtime using fetch? No.

Tests should be added in test/jwks/remote.test.mjs

[rolandzwaga]

but shall I add the agent option then as well while I'm at it?

Is the node-specific http(s).agent option applicable to browser runtime using fetch? No.

This might be a poor text-to-real-language translation, but do I detect sarcasm here?

Tests should be added in test/jwks/remote.test.mjs

Yes, I had already found this file, but couldn't find a mention of the agent option here. As I said, I found the tests quite hard to grok, so having a test for the agent option there would probably make things easier for me to implement. I would really appreciate some guidance.

[panva]

but shall I add the agent option then as well while I'm at it?

Is the node-specific http(s).agent option applicable to browser runtime using fetch? No.

This might be a poor text-to-real-language translation, but do I detect sarcasm here?

A tiny bit, yeah. The agent option is defined and documented as

An instance of http.Agent or https.Agent to pass to the http.get or https.get method's options. Use when behind an http(s) proxy. This is a Node.js runtime specific option, it is ignored when used outside of Node.js runtime.

So it does not apply to the other runtime.

Tests should be added in test/jwks/remote.test.mjs

Yes, I had already found this file, but couldn't find a mention of the agent option here. As I said, I found the tests quite hard to grok, so having a test for the agent option there would probably make things easier for me to implement. I would really appreciate some guidance.

There are no agent specific tests. Don't worry about it then, just clean up the PR, add the option to the browser runtime and I'll add a simple test then.

[rolandzwaga]

but shall I add the agent option then as well while I'm at it?

Is the node-specific http(s).agent option applicable to browser runtime using fetch? No.

This might be a poor text-to-real-language translation, but do I detect sarcasm here?

A tiny bit, yeah. The agent option is defined and documented as

An instance of http.Agent or https.Agent to pass to the http.get or https.get method's options. Use when behind an http(s) proxy. This is a Node.js runtime specific option, it is ignored when used outside of Node.js runtime.

So it does not apply to the other runtime.

Ah, ok, now I see, this was a question of RTFM for me. So, the sarcasm was valid hehe, my apologies. ;)

Tests should be added in test/jwks/remote.test.mjs

Yes, I had already found this file, but couldn't find a mention of the agent option here. As I said, I found the tests quite hard to grok, so having a test for the agent option there would probably make things easier for me to implement. I would really appreciate some guidance.

There are no agent specific tests. Don't worry about it then, just clean up the PR, add the option to the browser runtime and I'll add a simple test then.

Ok, thank you ;)
I'll clean it up after my meeting. (and add the header option to the browser functionality as well)

[rolandzwaga]

Ok, I pushed my last changes. I hope everything is in order now.