Skip to content

Custom loginCallback.handler loses session data, including the library's own returnTo #112

Description

@bnielsen1965

Describe the bug

Incoming wall of text. I ran into an issue where custom info I was passing in the relay state was lost. I am currently using a work around but I thought I should share, so I used AI to format a detailed post outlining the issue...

Custom loginCallback.handler loses all session data, including the library's own req.session.returnTo

Summary

createLoginCallbackHandler has two branches. The default branch passes
keepSessionInfo: true to passport. The custom-handler branch calls req.logIn(user, nextHandler)
with no options.

Since passport 0.6, req.logIn regenerates the session and restores the previous session
only when keepSessionInfo is set. So any application that supplies
routes.loginCallback.handler finds req.session emptied by the time its handler runs —
silently, with no error and nothing logged.

This also breaks the library's own behaviour: req.session.returnTo is set by
oidcUtil.js, and the custom-handler branch reads it back at connectUtil.js:133 as its
redirect fallback, after req.logIn has discarded it. That fallback can never fire.

Environment

@okta/oidc-middleware 6.0.1 (latest) and 5.5.1 — both affected
passport 0.7.0, via this package's own ^0.7.0 dependency
express-session 1.19.0
node 24.13.0

Expected

A custom loginCallback.handler sees the session as it was before the redirect to Okta,
in the same way the default branch does.

Actual

req.session is a brand new, empty session. The session ID has changed. Anything the
application stored before redirecting to Okta is gone, and so is returnTo.

Root cause

src/connectUtil.js, createLoginCallbackHandler (line numbers from 6.0.1):

  if (!customHandler) {
    const redirectOptions = {
      failureRedirect: routes.loginCallback.failureRedirect,
      keepSessionInfo: true, // preserve req.session.returnTo during session regeneration   <-- line 116
    };
    ...
    return passport.authenticate('oidc', redirectOptions);                                  //  line 124
  }

  const customHandlerArity = customHandler.length;
  return (req, res, next) => {
    const afterCustomNextHandler = (err) => {
      if (err) {
        next(err);
      } else if (!res.headersSent) {
        res.redirect(routes.loginCallback.afterCallback || req.session.returnTo || '/');     //  line 133
      }
    };
    ...
    passport.authenticate("oidc", (err, user, challenge) => {
      if (user) {
        req.logIn(user, nextHandler);                                                       //  line 151
      } else {
        nextHandler(err || challenge);
      }
    })(req, res, nextHandler);
  }

passport/lib/sessionmanager.js:

  var prevSession = req.session;

  // regenerate the session, which is good practice to help
  // guard against forms of session fixation
  req.session.regenerate(function(err) {
    ...
      if (options.keepSessionInfo) {
        merge(req.session, prevSession);
      }

The comment on line 116 shows the intent is understood; the custom-handler branch was
simply not given the same option. In 5.5.1 the corresponding lines are 117 and 152.

Reproduction

The behaviour is entirely in the passport call the custom-handler branch makes, so it
reproduces without an Okta org. This mirrors connectUtil.js:149-151 in shape and
contrasts it with the default branch.

mkdir oidc-keepsessioninfo && cd oidc-keepsessioninfo
npm init -y
npm install express@4 express-session@1 passport@0.7.0 passport-custom@1

repro.js:

const express = require('express');
const session = require('express-session');
const passport = require('passport');
const CustomStrategy = require('passport-custom');

passport.use('stub', new CustomStrategy((req, done) => done(null, { id: 'user-1' })));
passport.serializeUser((u, done) => done(null, u.id));
passport.deserializeUser((id, done) => done(null, { id }));

const app = express();
app.use(session({ secret: 'repro', resave: true, saveUninitialized: false }));
app.use(passport.initialize());
app.use(passport.session());

// stands in for the state held across the redirect to Okta: oidcUtil.js sets
// req.session.returnTo, and an application storing its own return address is the same shape
app.get('/before-login', (req, res) => {
  req.session.returnTo = '/where-the-user-was';
  req.session.appRelayState = 'https://app.example.com/callback';
  req.session.save(() => res.json({ stored: true, sid: req.sessionID }));
});

// what the DEFAULT branch does
app.get('/callback-default', passport.authenticate('stub', { keepSessionInfo: true }),
  (req, res) => res.json({
    branch: 'default (keepSessionInfo: true)', sid: req.sessionID,
    returnTo: req.session.returnTo ?? null, appRelayState: req.session.appRelayState ?? null
  }));

// what the CUSTOM HANDLER branch does
app.get('/callback-custom', (req, res, next) => {
  passport.authenticate('stub', (err, user) => {
    if (user) {
      req.logIn(user, () => res.json({
        branch: 'custom handler (no options)', sid: req.sessionID,
        returnTo: req.session.returnTo ?? null, appRelayState: req.session.appRelayState ?? null
      }));
    } else { next(err); }
  })(req, res, next);
});

const server = app.listen(0, async () => {
  const base = `http://127.0.0.1:${server.address().port}`;
  for (const branch of ['default', 'custom']) {
    let r = await fetch(`${base}/before-login`);
    const cookie = r.headers.getSetCookie()[0].split(';')[0];
    const before = await r.json();
    r = await fetch(`${base}/callback-${branch}`, { headers: { cookie } });
    const after = await r.json();
    console.log(`\n${after.branch}`);
    console.log(`  session id  ${before.sid}  ->  ${after.sid}`);
    console.log(`  returnTo         ${JSON.stringify(after.returnTo)}`);
    console.log(`  appRelayState    ${JSON.stringify(after.appRelayState)}`);
  }
  server.close();
});

node repro.js:

default (keepSessionInfo: true)
  session id  fJOud-NFkwdjTo0MSHD2CcNz1AjC-fdn  ->  D5chAh3P-uhALqTKWi1Fkf2jiHmiEWoR
  returnTo         "/where-the-user-was"
  appRelayState    "https://app.example.com/callback"

custom handler (no options)
  session id  hpK5aRhpgB2TV9_ye5XybG4UXKJss2VJ  ->  WzLxa-kHGpnPXlirTJ9hOx9NsFf0JtGw
  returnTo         null
  appRelayState    null

The session is regenerated in both cases, which is the point of the passport change. Only
the default branch restores what was on it.

End to end, against a real Okta org

  1. Configure ExpressOIDC with routes.loginCallback.handler set to a function of three
    or four arguments.
  2. Add middleware on routes.login.path that stores something on the session, for example
    req.session.myState = req.query.state.
  3. Complete a login.
  4. In the custom handler, req.session.myState is undefined and req.sessionID differs
    from the one issued before the redirect to Okta.

Observed in a browser HTTP capture: the callback request carries the original
connect.sid, and the next request carries a different one. So the cookie is not lost
in transit — the session is being replaced server side.

Suggested fix

req.logIn accepts (user, options, done):

     passport.authenticate("oidc", (err, user, challenge) => {
       if (user) {
-        req.logIn(user, nextHandler);
+        req.logIn(user, { keepSessionInfo: true }, nextHandler);
       } else {
         nextHandler(err || challenge);
       }
     })(req, res, nextHandler);

That makes the custom-handler branch consistent with the default branch and restores the
req.session.returnTo fallback on line 133.

Workaround

Do not rely on the session for anything that has to survive the callback when using a
custom handler. We now write the value to a short-lived HttpOnly, SameSite=Lax cookie
on the login route and read it back in the callback, preferring the session so that no
change is needed once this is fixed.

Why it is easy to miss

Nothing errors. The login completes, a valid token is issued, and the handler runs — it
just sees an empty session, so an application that redirects based on stored state
redirects to its default instead. In our case every SSO login silently returned users to
our own service's home page rather than to the requesting application, and no log on
either side reported a problem.

Reproduction Steps?

// Reproduces the session data loss in @okta/oidc-middleware's custom loginCallback
// handler path, isolated to the passport call that path makes.
//
// connectUtil.js createLoginCallbackHandler:
//   default branch        -> passport.authenticate('oidc', { keepSessionInfo: true, ... })
//   custom handler branch -> req.logIn(user, nextHandler)          <-- no options
//
// passport 0.7 SessionManager.logIn regenerates the session and restores the previous
// one only when options.keepSessionInfo is set, so the custom handler branch loses
// everything the application (and the library itself) stored on the session.

const express = require('express');
const session = require('express-session');
const passport = require('passport');
const CustomStrategy = require('passport-custom');

passport.use('stub', new CustomStrategy((req, done) => done(null, { id: 'user-1' })));
passport.serializeUser((u, done) => done(null, u.id));
passport.deserializeUser((id, done) => done(null, { id }));

const app = express();
app.use(session({ secret: 'repro', resave: true, saveUninitialized: false }));
app.use(passport.initialize());
app.use(passport.session());

// stands in for what the library and the application put on the session before the
// redirect to the identity provider: oidcUtil.js sets req.session.returnTo, and an
// application storing its own return address does the same kind of thing
app.get('/before-login', (req, res) => {
  req.session.returnTo = '/where-the-user-was';
  req.session.appRelayState = 'https://app.example.com/callback';
  req.session.save(() => res.json({ stored: true, sid: req.sessionID }));
});

// what the DEFAULT branch does
app.get('/callback-default', passport.authenticate('stub', { keepSessionInfo: true }),
  (req, res) => res.json({
    branch: 'default (keepSessionInfo: true)',
    sid: req.sessionID,
    returnTo: req.session.returnTo ?? null,
    appRelayState: req.session.appRelayState ?? null
  }));

// what the CUSTOM HANDLER branch does, connectUtil.js:149-152 verbatim in shape
app.get('/callback-custom', (req, res, next) => {
  passport.authenticate('stub', (err, user) => {
    if (user) {
      req.logIn(user, () => res.json({
        branch: 'custom handler (no options)',
        sid: req.sessionID,
        returnTo: req.session.returnTo ?? null,
        appRelayState: req.session.appRelayState ?? null
      }));
    } else { next(err); }
  })(req, res, next);
});

const server = app.listen(0, async () => {
  const base = `http://127.0.0.1:${server.address().port}`;
  for (const branch of ['default', 'custom']) {
    let r = await fetch(`${base}/before-login`);
    const cookie = r.headers.getSetCookie()[0].split(';')[0];
    const before = await r.json();
    r = await fetch(`${base}/callback-${branch}`, { headers: { cookie } });
    const after = await r.json();
    console.log(`\n${after.branch}`);
    console.log(`  session id  ${before.sid}  ->  ${after.sid}`);
    console.log(`  returnTo         ${JSON.stringify(after.returnTo)}`);
    console.log(`  appRelayState    ${JSON.stringify(after.appRelayState)}`);
  }
  server.close();
});

SDK Versions

"@okta/okta-sdk-nodejs": "^8.1.0"

System:
OS: Linux 7.1 Fedora Linux 44 (Workstation Edition)
CPU: (12) x64 AMD Ryzen 5 7600 6-Core Processor
Memory: 12.61 GB / 30.46 GB
Container: Yes
Shell: 5.3.9 - /bin/bash
Binaries:
Node: 24.13.0 - /usr/bin/node
npm: 11.6.2 - /usr/bin/npm
pnpm: 10.33.0 - /usr/bin/pnpm
Browsers:
Chrome: 152.0.7977.64
Firefox: 154.0
Firefox Developer Edition: 154.0

Additional Information

No response

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions