fastify-passport: `state` for passport causes `fastify-passport` to fail
Prerequisites
- I have written a descriptive issue title
- I have searched existing issues to ensure the bug has not already been reported
Fastify version
3.22.1
Plugin version
0.4.3
Node.js version
16.x
Operating system
macOS
Operating system version (i.e. 20.04, 11.3, 10)
macOS Big Sure 11.3.1
Description
When I am trying to pass some encoded state to the strategy the callback URLs shows the user as unauthenticated, it’s also unclear how I could pass the encoded state from the redirect endpoint dynamically.
Steps to Reproduce
fastifyPassport.use(
"github",
new Github.Strategy(
{
clientID: "xxx",
clientSecret: "xxx",
callbackURL: "http://lvh.me:3000/callback",
scope: ["user:email", "user,user:email"],
userProfileURL: "https://api.github.com/user",
state: Buffer.from(JSON.stringify({ returnTo: "/mypage" })).toString(
"base64"
),
passReqToCallback: true,
},
function (req, accessToken, refreshToken, profile, done) {
done(null, { ...profile, accessToken, query });
}
)
);
server.get(
"/github",
{
preValidation: fastifyPassport.authenticate("github", {
successRedirect: "/callback",
authInfo: false,
assignProperty: "user",
}),
},
() => {}
);
server.get(
"/callback",
{
preValidation: fastifyPassport.authenticate("github", {
authInfo: false,
session: false,
}),
},
async (request, reply, err, user = {}, info = {}, status = {}) => {
reply.send({ user: request.user || {} });
}
);
Expected Behavior
Step through the prevalidation hook and return the encoded state to the endpoint.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 16 (15 by maintainers)
Let’s link the PR so this issue is closed when the PR is merged then, please. Same with the other issue if not linked already
Thanks @climba03003 for summarizing the problem.
The reason why fastify-secure-session offers a get/set/delete methods is to protect against prototype pollution attacks. Maybe we should add an option there to use a plain object for it. I’m also ok in moving to a plain object, it’s not strictly needed anyway and familiarity is likely more important.
It’s interesting to note that
@fastify/sessionuse plain objects. So maybe the best option is to just support it out of the box: https://github.com/fastify/fastify-passport/issues/319.@mcollina The underlay issue is how
passportconsumer expect to userequest.session. Aspassportis made forexpress, it expect to directly mutate therequest.sessionobject.However,
fastify-secure-sessionis expect to use.get,.set,.deleteto update the state ofsession. It lead to the problem, when ever apassportstrategy that require the usage ofsession,fastify-secure-sessioncan never be supported.Here comes to two solution.
fastify-secure-sessionprovide aproxysession object to support this use-case.fastify-passportprovide a customrequest, and bind to aproxysession object.@simoneb ~I am no longer working on this~ I will take a look at this again today
This seems to be an issue with
fastify-secure-sessionand notfastify-passport. I tried the snippet above using fastify-session(https://github.com/SerayaEryn/fastify-session) and it seems to work fine withstate. I will dig deeper and try to find the issue.