passport strategy currentUrl does not consider port number #767
Description
What happened?
I am working on a localhost:8086 express server.
The implementation of currentUrl is creating an url such as http://localhost/xxx instead of http://localhost:8086/xxx
from https://github.com/panva/openid-client/blob/main/src/passport.ts#L412
currentUrl(req: express.Request): URL {
return new URL(`${req.protocol}://${req.host}${req.originalUrl ?? req.url}`)
}could you consider changing implementation like so instead ? :
currentUrl(req: express.Request): URL {
return new URL(`${req.protocol}://${req.get("host")}${req.originalUrl ?? req.url}`)
}note the
req.get("host")instead ofreq.host
Or if any compatibility issue with older versions, mayby
currentUrl(req: express.Request): URL {
return new URL(`${req.protocol}://${req.get?.("host") ?? req.host}${req.originalUrl ?? req.url}`)
}?
For now, I have monkey patched the code this way:
Strategy.prototype.currentUrl = (req) =>
new URL(`${req.protocol}://${req.get("host")}${req.originalUrl}`);(it's working)
Charles
Version
6.3.4
Runtime
Node.js
Runtime Details
node v22.13.1, express@4.21.2
Code to reproduce
passport.use(new Strategy(
{
config,
name: "oidc",
callbackURL: "http://localhost:8086/auth/callback",
},
(tokens, verified) => verified(null, tokens.claims()),
));
// Authentication endpoint
// The sso server complains about http://localhost/auth/callback redirection instead of http://localhost:8086/auth/callback
app.use(passport.authenticate("oidc", {
assignProperty: "user",
}),
(req, res) => {
console.log("OIDC Authenticated successfully.");
res.json({
user: req.user,
authInfo: req.authInfo,
});
},
);Required
- I have searched the issues tracker and discussions for similar topics and couldn't find anything related.
- I agree to follow this project's Code of Conduct
Activity
panva commentedon Mar 24, 2025
Hello @charleshetier
That's fine. Altho you probably only want to do that for localhost development, it is not appropriate to do so in production.
openid-client/src/passport.ts
Lines 409 to 410 in b82499c
Otherwise, please see #743, #733, #746, and #713. Those are all similar issues that I've already answered on the subject that may explain why the existing implementation is the way it is considering that
charleshetier commentedon Mar 25, 2025
Hello @panva , thank you for you clear answer. That's make sense. I'll consider migrating to expressjs@5 as soon as possible. Sorry for the duplication then, and thank you for your great work !
panva commentedon Mar 25, 2025
No worries.