Using MultiSamlStrategy and handling sessions

[rfossella]

Can anyone please offer some suggestions while using MultiSamlStrategy? I am using "@node-saml/passport-saml"^5.0.0 with Node 16.29.

Previously, I was using a SAML strategy that simulated multi by creating a new instance of SAMLStrategy for each login request for different IdPs. I thought this introduced a session leak since it sometimes appeared to be holding onto another SAML instance when two users logged in simultaneously. I was able to reproduce this many times by testing in Chrome and Edge simultaneously.

I then switched to MultiSamlStrategy to see if this alleviated the problem - see sample code below.

Background:

• I keep a Mongo collection called authtypes that holds IdP data for various vendors.
• I have code (authtypes.js) that handles the mechanics of retrieving data, grabbing creds, etc.
• I use the username from a login form, get its domain, get the corresponding authtype and creds.
• Use passport-saml to send requests to IdP, get the profile, etc.

I use express-session with MongoDB for my session store and do the necessary Passport init:

app.use(passport.initialize()); 
app.use(passport.session());

I understand when using SSO there are actually two calls to passport.authenticate('saml'...):

• The first (/login) to setup the IdP credentials.
• The second (within the /login/callback called from IdP) to get the profile, which is then used to validate against the logged-in username, and let the user proceed through.

The problem I sometimes run into:

The first passport.authenticate() hits my passport.use() logic, and I always have access to the req.session.user object. Using that object, I can get the SAML credentials - good.

The second passport.authenticate fires off after the IdP calls back (login/callback).

When the passport.use() middleware gets hit the second time, sometimes the req.session.user object is not there.

This is causing intermittent failures. It happens about 10% of the time.

The madness here is that it happens sometimes, so it's been a debugging nightmare. I have checked my session logic many times over (FYI: I've also been using passport-local strategy for 5+ years, although there is only one auth call for this strategy).

I'd like to know if anyone else has possibly experienced something similar? Or if anyone has time to look through my sample code below and tell me if they see anything glaring (or obvious!) that I'm missing. I included snippets from app.js, and authtypes,js, with most of the logic in users.js
Thanks in advance!

//from app.js
// Express Session and session store
const sessionStore = require('./routes/sessionStore')(db);
app.use(session({
		store: sessionStore,
		secret: utils.getConfigValue('secret'),
		resave: true,	
		saveUninitialized: false,
		rolling: true,
		cookie: {httpOnly: true},
		name: 'appsessionId'
		}))

// Passport Init
app.use(passport.initialize());
app.use(passport.session());


/////////////////////////
// users.js - where login, passport, etc.

const express = require('express');
const router = express.Router();
const passport = require('passport');
const MultiSamlStrategy = require('@node-saml/passport-saml').MultiSamlStrategy;
const User = require('../models/user');
const utils = require('./utils');

// authtypes.js holds functions related to getting auth & creds
const authtypes = require('./authtypes');	

const invalidUserPwd = message = utils.getMsgCode('invalidUserPwd');

// SAML 5.0
const samlOptions = authtypes.samlOptions;

router.param('username', (req, res, next, username) =>
{
	utils.consoleA(username);
	req.username = username;		// username of the user being acted upon
	return next();	
})


// Gets fired on passport.authenticate('saml')
passport.use(new MultiSamlStrategy(
{
passReqToCallback: true,
getSamlOptions: async (req, done) => 
{
	let errorMessage = null;
	try 
	{
		// When we get into the passport.use() middleware the SECOND TIME, 
		// "sometimes" the req.session.user object is not there.
		// This is causing intermittent fails.  Happens about 10% of the time.

		const username = req?.session?.user?.username;
		console.log('>>>>>Username:', username);

		if (!username) 
		{ 
			errorMessage = "Error processing your SSO username.<br/>Please verify and try again."; 
			console.error('Error in getSamlOptions:', errorMessage); // Log the error message 
			return done(null, false, { message: errorMessage }); // Pass the error message through 
		}

		const samlCredentials = await authtypes.getSAMLCredentials(username);
		if (!samlCredentials) 
		{
			errorMessage = "Invalid SAML credentials."; 
			return done(null, false, {message: errorMessage});
		}

		const samlOptionsSSO = {
				...samlCredentials,
				forceAuthn: samlOptions.forceAuthn 
					}

		return done(null, samlOptionsSSO);
	} 
	catch(err) 
	{
		let finalErr = errorMessage ? {success: false, message: errorMessage } : err;
		return done(finalErr); // Pass the unexpected error through
	}
}
},
async (req, profile, done) => 
{
try 
{
	console.log('SAML Profile:', profile);
	const profileUsername = authtypes.getSAMLUsername(profile);
	let mySSOUser = await User.getUserByUsernameA(profileUsername);
	console.log("~ mySSOUser:", mySSOUser);

	if (!mySSOUser) 
	{
		return done(null, false, { message: invalidUserPwd });
	}

	// Need this check because sometimes the "IdP returned user" is not same as entered
	if (!utils.areUsernamesTheSame(profileUsername, req.session.user.username)) 
	{
		const message = getSSOError(profileUsername, req.session.user.username);
		return done(null, false, { userTryingToFind: req.session.user.username, message });
	}

	const profileDisplayName = authtypes.getSAMLUserDisplayname(profile);
	console.log(`~ profileUsername: ${profileUsername}\n ~ displayName: ${profileDisplayName}`);
	return done(null, mySSOUser, {message: 'Good to go... you can login' });	
} 
catch(err) 
{
	return done(err);
}
}))
		
passport.serializeUser((user, done) => done(null, user.id));
passport.deserializeUser((id, done) =>
{
	User.getUserById(id, (err, user) => done(err, user));
})

router.get('/login', async (req, res) =>
{
	if (req.isAuthenticated())
	{
		console.log('User already logged in');
		res.redirect('/');
	}
	else
	{
		res.render('login', {title: 'LOGIN'});
	}
})


// Logout
router.get('/logout', (req, res) =>
{
	let username;
	if (req.user)
	{
		username = req.user.username;
		User.findOneAndUpdate({username}, {$unset: {"systoken": null}}, 
			(err, user) =>
			{
				err ? console.log(err) : console.log(`${user.username} had systoken unset`);
			})
	}

	req.logOut();

	// Reset duo on logout
	req.session.duo_sig_response = null;
	req.session.secondFactor = null;
	req.session.cookie.maxAge = null;
	req.flash('success_msg', 'You are logged out');

	req.session.destroy((err) => 
	{
        if (err) 
		{
            console.error('Error destroying session:', err);
            return res.status(500).send('Error logging out');
        }
        // Clear cookies
        res.clearCookie("apptoken");
        // Redirect to login page
        res.redirect('/users/login');
    })
})

router.post('/login', async (req, res, next) =>
{
	let myUsername = req.body.username;
	let myUser = await User.getUserByUsernameA(myUsername);

	console.log('Session before setting user:', req.session);
	req.session.user = myUser;
	console.log('Session after setting user:', req.session);

	if (!myUser) 
	{
		req.flash('error_msg', invalidUserPwd);
		return res.redirect('/users/login');
	} 

	// SSO logic
	// Authtypes is a Mongo collection holding the login/authentication types
	// Use the username and its domain to get things like Idp creds
	let authType = await authtypes.getAuthTypeFromUsername(myUsername);
	if (!authType)
	{
		req.flash('error_msg', "Domain not mapped to an Auth Type");
		return res.redirect('/users/login');
	}

	let credentials = authType.credentials;
	if (!credentials)
	{
		req.flash('error_msg', utils.getMessage('invalidSAMLCredentials'));
		return res.redirect(`/users/login?username=${myUsername}`);
	}
	
	// This passport.authenicate fires off SAML request to IdP and awaits response
	passport.authenticate('saml', { 
					failureRedirect: '/users/login',
					failureFlash: 'Error during SAML authentication'
				})(req, res,next);
})

// SAML - callback from IDP - e.g. MS Entra which *must* be defined there
router.post('/login/callback', (req, res, next) => 
{
	utils.logIt(`req.session.user: before SAML authenticate in login/callback: ${req?.session?.user}`, 'red', true);

	// This passport.authenicate fires off after IdP reported back and then we can look at the returned profile
	// When we get into the passport.use() middleware the second time, "sometimes" the req.session.user object is not there.
	// This is causing intermittent fails.  Happens about 10% of the time.
    passport.authenticate('saml', (err, user, info) => 
	{
        if (err) 
		{
            return next(err);
        }
  		// Generate a JSON response reflecting authentication status
		if (!user) 
		{
			var msg = info.message; 	//'Authentication failed!' + info.message;
			req.flash('error_msg', msg);
			return res.redirect('/users/login');
		}
        // If authentication is successful, log the user in
        req.logIn(user, (err) => 
		{
            if (err) 
			{
                return next(err);
            }
			const username = req.user.username;
			req.session.authType = "sso";
			req.flash('success_msg', 'You are logged in as ' + username);
			const randToken = utils.getRandomCrypto(44);
			res.cookie('apptoken', randToken, {sameSite: false, httpOnly: true});
			res.redirect('/');
        })
    })(req, res, next);
})

function getSSOError(profileUsername)
{
	let message = utils.getMsgCode('ssoUserError');
	message += 	`This could be due to a browser cache problem with your IdP.` +
				`<br/><br/>If the problem persists, it is recommended to logout of your IdP `;
	return message;
}

/////////////////////////////////////////////


////
// from authtype.js
const getSAMLCredentials = async username =>
	{
		let authType = await getAuthTypeFromUsername(username);
		let credentials = authType.credentials;
		// Configure Passport SAML
		let {idpSSOUrl, entityId, idpCert, assertionUrl, logoutUrl} = await getSAMLConfig(credentials);
	
		return {
				entryPoint: idpSSOUrl,
				issuer: entityId,
				idpCert: idpCert,
				callbackUrl: assertionUrl,
				logoutUrl: logoutUrl,
				wantAssertionsSigned: samlOptions.wantAssertionsSigned,
				wantAuthnResponseSigned: samlOptions.wantAuthnResponseSigned
			}
	}

[srd90]

It would be easier to take a quick look into your example code if it would be properly indented and syntax highlighted (about syntax highlighting see https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks#syntax-highlighting ).

For anyone who try to solve the puzzle more background is probably available at node-saml/node-saml#379

tl;dr; background for aforementioned issue seems like stress testing with chaos testing twist due to not terminating SSO at IdP (only local logout) and using same browser instances to trigger new logins to SP (IdP might be seeing still alive IdP related login session and act accordingly).

Previously, I was using a SAML strategy that simulated multi by creating a new instance of SAMLStrategy for each login request for different IdPs. I thought this introduced a session leak since it sometimes appeared to be holding onto another SAML instance when two users logged in simultaneously. I was able to reproduce this many times by testing in Chrome and Edge simultaneously.

Out of curiosity: Are you referring to this node-saml/node-saml#379 or are you you saying that passport-saml configurations/instances of passport-saml strategies were somehow mixed.

I understand when using SSO there are actually two calls to passport.authenticate('saml'...):

• The first (/login) to setup the IdP credentials.
• The second (within the /login callback called from IdP) to get the profile, which is then used to validate against the logged-in username, and let the user proceed through.

From SAML/passport-saml point and terminology point of view first call triggers SP initiated login (in case of multisaml strategy saml config is setup per request...this sample code has named saml config as credentials) with IdP and ACS endpoint (second call) receives result of login. In case of successfull login result contains assertion which contains information released by IdP for this SP about successfully autenticated user. Passport-saml validates digital signature(s) and few other things before passing information from assertion to application layer code. "Validation against logged in username" is something that this particular application does at application layer and is outside of "SAML scope".

The first passport.authenticate() hits my passport.use() logic, and I always have access to the req.session.user object. Using that object, I can get the SAML credentials - good.

Are you talking about your router.post('/login')... implementation which reads username posted via some application form and sets information related to username to req.session.user and then passes control to passport and at the end of the day control is at getSamlOption function (with same req that was augmented with userinfo)?

IMHO it is obvious why your getSAMLOptions can always access req.session.user

When the passport.use() middleware gets hit the second time, sometimes the req.session.user object is not there.

This is causing intermittent failures. It happens about 10% of the time.

The madness here is that it happens sometimes, so it's been a debugging nightmare.

So now we are talking about this router.post('/login/callback'... implementation.

Based on information from your previous issue (stress testing) I wonder whether you have IdPs at different domains (TLDs or at subdomains of tlds which are blacklisted from cookie sharing point of view) than your application's domain? If so could it be possible that about 10% of your stress test logins happen at such IdP and if thats the case modern browsers do not send cookies when such IdP forwards user's browser back to your application via HTTP POST binding. Since there aren't any express session cookies available req.session.user is not filled by session management layer when POST to /login/callback arrives.

You should use some other information from req to determine proper saml config (i.e. do not rely on information that requires cookies).

[rfossella]

Thanks for the tip on syntax highlighting - was not aware.

So yes, primarily this is a stress test situation. And some of these things were touched upon in node-saml/node-saml#379

The "missing" req.session.user does happen in the /login/callback code. I added debug traces in every conceivable location to see where the session or session.user might be lost/destroyed - but again, there's no consistency. Sharing the snippets of code was primarily to see if there was something out of the ordinary I was doing w/multistrategy.

Based on information from your previous issue (stress testing) I wonder whether you have IdPs at different domains (TLDs or at subdomains of tlds which are blacklisted from cookie sharing point of view) than your application's domain? If so could it be possible that about 10% of your stress test logins happen at such IdP and if thats the case modern browsers do not send cookies when such IdP forwards user's browser back to your application via HTTP POST binding. Since there aren't any express session cookies available req.session.user is not filled by session management layer when POST to /login/callback arrives.

You should use some other information from req to determine proper saml config (i.e. do not rely on information that requires cookies).

This was enlightening and a path I will investigate. I will post any findings back to this discussion.

Thanks very much for your insight.

[srd90]

If you need to pass state you can use SAML's RelayState for that (instead of cookie). Any string (I dont remember max length) is passed as-is over login sequence back to SP. String could be e.g. reference to user.

BUT due to your policy of hating/avoiding SLO even on this stress test situation (and not even trying to setup / cleanup things like browser state to resemble normal - you wrote at previous issue that normal is single user per browser instance) combined with possible usage of RelayState opens up corner case possibilities. I don't feel like starting to write down speculations just because you dont want to properly teardown/initialize test rounds...just think of situations when your cookies work and when they don't and if SSO is still open at IdP for previous user of browser instance and how in this case value of RelayState would amplify your stress of trying to understand whats happening etc.

About RelayState: see saml specs and search e.g. passport-saml's open and closed issues for any previous discussions (because discussions were not enabled for repos years ago):

1. https://github.com/node-saml/passport-saml/issues?q=is%3Aissue+is%3Aopen+relaystate
2. https://github.com/node-saml/passport-saml/issues?q=is%3Aissue+relaystate+is%3Aclosed
3. https://github.com/search?q=org%3Anode-saml+relaystate&type=issues

[rfossella]

Appreciate these additional tips and will try them out.

Quick note regarding SLO - I have pushed for this but the vendors we support specifically stated they don't want it. I tried to sneak it in and they complained, "why do we have to do all the login steps again." :-\

I even went so far as to add a checkbox option in our authtype editor reading, "Full Logout (SLO)" which defaults to false. Just in case they change their mind we can easily flip the state and the code is already set up to proceed w/SLO after the app logout.

Thanks, again.

[rfossella]

Great tip regarding RelayState as this helped me in the cases where the req.session was no longer holding my user object. It appears to reliably pass back my data from Idp SAML request.
Thank you.

// login
let username = req?.session?.user?.username
console.log(`req username before passport.auth :: ${username}`);
passport.authenticate('saml', { 
        additionalParams: {RelayState: username},
        failureRedirect: '/users/login',
        failureFlash: 'Error during SAML authentication'
      })(req, res, next);


//Process SAML request
passport.use(new MultiSamlStrategy(
{
  passReqToCallback: true,
  getSamlOptions: async (req, done) => 
  {
    try 
    {
      let sessUsername = req?.session?.user?.username;
      let relayState = req?.body?.RelayState;			
      const username = sessUsername || relayState;
      console.log(`>>Username: ${username}`);
      
      if (!username) 
      { 
        let errorMessage = "Error processing your SSO username.<br/>Please verify and try again."; 
        return done(null, false, {message: errorMessage}); // Pass the error message through 
      }
    
    const samlCredentials = await authtypes.getSAMLCredentials(username);