passport / discuss

A forum for discussing topics related to the usage of Passport.js.
1 stars 0 forks source link

keepSessionInfo not working as expected #81

Open thestockgenius opened 3 months ago

thestockgenius commented 3 months ago

I have a pre-exsisting passport session setup using session: Passport version 0.7.0 (and checked with 0.6.0 as well). Issue

app.use(session({ secret: vars.SESSIONSECRET, resave: false, saveUninitialized: false, cookie: { sameSite: true, secure: true, httpOnly: true, maxAge: 86400000 // 1 day in milliseconds }, store: new RedisStore({ client: redisClient }), }));

I have a pre-existing session (cookie at client and session store in redis backend store) , using passport.authenticate using the google token strategy (not sure it's important but referencing just in case).

In my case, I am trying to extend the user's granted permissions (in this case google drive - app only space), and using googleStrategy to do so. I have the following code:

Note, I have tried with session:false as well as session:true and both result in broken behavior, difference in behavior will be explained below.

// Google OAuth2 authentication routes router.get('/google/extend-scope', checkSessionData, (req, res, next) => { // Redirect the user to initiate authentication with the new scope passport.authenticate('googleExtended', { scope: ['openid', 'profile', 'email', 'https://www.googleapis.com/auth/drive.file'], session: false, accessType: 'offline', // Add this line prompt: 'consent', // Add this line display: 'popup', authorizationParams: function () { return { include_granted_scopes: true, }; }, failureRedirect: '/oops', keepSessionInfo: true

    })(req, res, next);
});

router.get('/google/extend-scope/callback',
    passport.authenticate('googleExtended', {
        failureRedirect: '/oops',
        session: false,
        keepSessionInfo: true
    }),
    checkRestoreSessionData,
    async (req, res) => {
        console.log("req.session.id", req.session.id);
        ... stores new data in the session ...

const checkSessionData = (req, res, next) => { console.log('saved session data', req.session.passport.user); req.session.save((err) => { if (err) { console.error('Error saving session:', err); } console.log("Session saved and going to next()", req.session.id) next(); }); next(); }; const checkRestoreSessionData= (req, res, next) => { if (!req.session) { console.log ("Session is not there") } else { console.log ("Session is there:", req.session.id) }

if (req.session.passport.user) {
    console.log('restoring saved session data', req.session.passport.user);
} else {
    console.log('no saved session data to restore');
}
next();

}; I originally did not have the checkSessionData and checkRestoreSessionData, but added them to try and get closer to what was happening.

In the case where I set session: false, the checkRestoreSessionData has no req.session at all (ok, that is somewhat to be expected, but I do have a session, I don't want passport to create a session, so, it my session is no longer available at all in this route, but it does seem to still exist, just not on this route anymore )

In the case where I set session:true (note I also have keepSessionInfo: true) I get an entirely new session.id, not the old one that was logged prior to the authentication callback being called. keepSessionInfo seems to have no effect. One case, I can't access any session, the other I simply get a new session.

keepSessionInfo seems to have no impact.

If I set session: false, I do not appear to get a new session, as when I restart the server, and try and continue with the same session, the session is still valid. If I set session: true, I have a new session when I restart the app regardless of the keepSessionInfo setting. (However, the session store is still in the redis db backend. The only way to prevent this is to force a crash in the callback function for the strategy, preventing the strategy from returning to the callback route - to me this just indicates the session is saved (it seems to be started before the callback but not saved) upon the strategy next() being called, not that a change in behavior did or didn't occur.)

Minor note, I realized I had never tried logging req.session.id in the strategy callback function, I did so, and the session id is different within the callback function.