firebase-admin-node icon indicating copy to clipboard operation
firebase-admin-node copied to clipboard

AppCheck JwksFetcher ignores httpAgent / proxy setting

Open hermanho opened this issue 1 year ago • 1 comments

[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

hermanho avatar Aug 23 '24 17:08 hermanho

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

google-oss-bot avatar Aug 23 '24 17:08 google-oss-bot

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

lahirumaramba avatar Aug 30 '24 17:08 lahirumaramba

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!

forty avatar Sep 11 '24 07:09 forty

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.

hermanho avatar Sep 11 '24 22:09 hermanho

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

forty avatar Sep 12 '24 08:09 forty

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!

lahirumaramba avatar Sep 12 '24 15:09 lahirumaramba