AppCheck JwksFetcher ignores httpAgent / proxy setting

hermanho commented 3 weeks ago

[READ] Step 1: Are you in the right place?

For issues related to the code in this repository file a Github issue.
If the issue pertains to Cloud Firestore, read the instructions in the "Firestore issue" template.
For general technical questions, post a question on StackOverflow with the firebase tag.
For general Firebase discussion, use the firebase-talk google group.
For help troubleshooting your application that does not fall under one of the above categories, reach out to the personalized Firebase support channel.

[REQUIRED] Step 2: Describe your environment

Operating System version: mac 14.6.1
Firebase SDK version: 12.4.0
Firebase Product: appcheck (auth, database, storage, etc)
Node.js version: 20.11.1
NPM version: 10.5.0

[REQUIRED] Step 3: Describe the problem

Steps to reproduce:

Although httpAgent (http proxy setting) is one of the paramteres in the app option, the httpAgent setting does not apply in JwksFetcher so that it will raise an error when request the JWSK url behine proxy requried network.

Relevant Code:

import { initializeApp } from 'firebase-admin/app';
import { HttpsProxyAgent } from 'https-proxy-agent';

const firebaseApp = initializeApp({
  httpAgent: new HttpsProxyAgent(process.env.HTTPS_PROXY)
});

https://github.com/firebase/firebase-admin-node/blob/d88c3fb234f4990df16a5d636147881c529601f8/src/app-check/token-verifier.ts#L39-L41

https://github.com/firebase/firebase-admin-node/blob/d88c3fb234f4990df16a5d636147881c529601f8/src/utils/jwt.ts#L56-L65

In the lib jwks-rsa, it is support to pass requestAgent in the paramteres.

https://github.com/auth0/node-jwks-rsa/blob/b2ec8a8839bdf93225a5cb32e6ecfa002c466eeb/src/JwksClient.js#L35-L41

google-oss-bot commented 3 weeks ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

lahirumaramba commented 2 weeks ago

Thank you @hermanho for reporting this issue and for submitting a fix in #2689 We really appreciate your time!

forty commented 2 days ago

Hello @hermanho :wave: Do you know if this issue was a regression ? We have trouble upgrading from 12.1.1 to 12.4.0, I suspect proxy issues and your fix seems to be too spot on to be a coincidence :D Thanks!

hermanho commented 2 days ago

Hello @hermanho 👋 Do you know if this issue was a regression ? We have trouble upgrading from 12.1.1 to 12.4.0, I suspect proxy issues and your fix seems to be too spot on to be a coincidence :D Thanks!

Hi @forty, I am new to AppCheck. I don't think it was working from day 1.

forty commented 1 day ago

Thanks! I noticed in the meantime that your issue was raised after the release of the change on HTTP2, which I strongly suspect is the cause of the regression on our side though, so I'm still hopeful your change might help us :D

lahirumaramba commented 1 day ago

Thanks! I noticed in the meantime that your issue was raised after the release of the change on HTTP2, which I strongly suspect is the cause of the regression on our side though, so I'm still hopeful your change might help us :D

Hey @forty, @hermanho is correct. I also don't think the App check issue is a regression. HTTP/2 changes should only apply to FCM APIs. We plan to include this fix in today's release. If you are still having issues please open a new ticket. Thanks!

firebase / firebase-admin-node