Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Current https-proxy-agent dependency causes issues with unrelated code #3448

Closed
rytmis opened this issue Dec 2, 2020 · 4 comments
Closed
Labels
enhancement Enhancement to an existing feature or behavior. internal Issues created by MSAL.js team for internal tracking. passport-azure-ad Related to the passport-azure-ad library

Comments

@rytmis
Copy link

rytmis commented Dec 2, 2020

Version: 4.3.0
Behavior: Unrelated code breaks because requests are being redirected to localhost

The issue I'm seeing is panva/node-openid-client#301 (I'm trying to refresh tokens originally obtained via passport-azure-ad). Requests made via openid-client are categorically redirected to localhost instead of the intended address. This is a bug in an upstream dependency of passport-azure-ad that directly patches core node functionality when loaded.

The source of the problem is documented at TooTallNate/node-agent-base#35. Basically, the mere existence of an old version of agent-base may break other libraries by redirecting https requests to localhost instead of where they were meant to go.

Unfortunately the upstream fix was released in a major version upgrade, so in order to get the fix, it seems necessary to upgrade the https-proxy-agent dependency used by passport-azure-ad.

Here is the dependency tree, for reference:

D:\Projects\[redacted]> npm ls agent-base
[redacted]@[version] D:\Projects\[redacted]
`-- passport-azure-ad@4.3.0
  `-- https-proxy-agent@2.2.4
    `-- agent-base@4.3.0 
@jasonnutter
Copy link
Contributor

@rytmis Thanks for the info, we'll take a look at this.

@rytmis
Copy link
Author

rytmis commented Dec 3, 2020

FYI, I just tried using npm-force-resolutions to bump both https-proxy-agent and agent-base to their latest versions, and the issue went away.

rytmis referenced this issue in rytmis/passport-azure-ad Dec 7, 2020
@sameerag
Copy link
Member

@pkanher617 May be we want to patch this in the older version?

@sameerag sameerag transferred this issue from AzureAD/passport-azure-ad Apr 13, 2021
@sameerag sameerag added passport-azure-ad Related to the passport-azure-ad library enhancement Enhancement to an existing feature or behavior. labels Apr 13, 2021
@tnorling tnorling added the internal Issues created by MSAL.js team for internal tracking. label Apr 23, 2021
@jo-arroyo
Copy link
Collaborator

Closing as passport-azure-ad is in maintenance and will no longer receive updates. Future middleware scenarios will be built on top of msal-node. Keep an eye out for msal-node updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature or behavior. internal Issues created by MSAL.js team for internal tracking. passport-azure-ad Related to the passport-azure-ad library
Projects
None yet
Development

No branches or pull requests

5 participants