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/session
use 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
passport
consumer expect to userequest.session
. Aspassport
is made forexpress
, it expect to directly mutate therequest.session
object.However,
fastify-secure-session
is expect to use.get
,.set
,.delete
to update the state ofsession
. It lead to the problem, when ever apassport
strategy that require the usage ofsession
,fastify-secure-session
can never be supported.Here comes to two solution.
fastify-secure-session
provide aproxy
session object to support this use-case.fastify-passport
provide a customrequest
, and bind to aproxy
session 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-session
and 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.