set function breaks session cookie

[egtoney]

The following code is causing all of the cookie parameters to be removed from the Set-Cookie header when the cookie is initially set by express-sessions.

session-pouchdb-store/lib/store.js

Lines 215 to 217 in 6f9845f

	if(session.cookie && session.cookie.toJSON) {
	session.cookie = session.cookie.toJSON();
	}

By turning the cookie object into a normal JSON object all of the computed properties on the object are lost. This causes an issue in express-sessions since it uses the computed data property to fill in the Set-Cookie header.

Ref: the data property

https://github.com/expressjs/session/blob/e18fb51d2d403bf3b5bd4e498f621dee4a52db41/session/cookie.js#L117-L127

Ref: where the data property is being used

https://github.com/expressjs/session/blob/e18fb51d2d403bf3b5bd4e498f621dee4a52db41/index.js#L243

[egtoney]

As an update: I implemented a really basic solution where I stored a reference to the old cookie and restored it before calling the passed callback to the set and that fixed the problem.

I imagine a real solution wold be preffered but this is what I have.

set(sid, session, callback) {
	callback = callback || vfn;
	debug('====> set',sid);

	if(!session._id) session._id = sid;
	session.$ts = Date.now();
	session.$dirty = true;

	const old_cookie = session.cookie;

	const restore_cookie_callback = () => {
		session.cookie = old_cookie;
		callback();
	}

	if(session.cookie && session.cookie.toJSON) {
		session.cookie = session.cookie.toJSON();
	}

	if(this.sessions[sid]) {
		let _rev = this.sessions[sid]._rev;
		extend(this.sessions[sid],session,{_rev});
	}
	else {
		this.sessions[sid] = session;
	}

	if(session.$commit) {
		session.$dirty = false;
		this.pouch.put(this.sessions[sid],restore_cookie_callback);
	}
	else {
		restore_cookie_callback();
	}
}

I'm willing to make a PR if you prefer.

[egtoney]

Alright, so after A LOT of digging around in this library and express-session I found out what was actually happening. I saw why you were converting the cookie to a basic JSON object (this seems to be a requirement in PouchDB ref). So to meet both the requirements of (1) sending basic JSON objects to PouchDB and (2) not breaking the cookie field, I decided to...

1. swap out your extend dependency with lodash. lodash has support for a deep clones using a customer function
2. converting the session object into a pure JSON object every time it is passed to the PouchDB.put method. I just check every object to see if it has a toJSON function and if so use that instead of the actual value (this is similar to what you were already doing)

The downside of this approach is that the session object must be converted into a basic JSON object on every PouchDB put call 🤷‍♂

Here's my solution: https://github.com/solzimer/session-pouchdb-store/blob/master/lib/store.js

I also fixed a problem with the destroy function

[solzimer]

Hi @egtoney,

Sorry for the delay, i've been pretty busy this days. I'll try to incorporate your changes and do some tests.

Thanks a lot for your contribution!!!

[stroiman]

Is this why some cookie options are changed:
e.g. path is set to the path that issued the cookie, instead of '/'
httpOnly is set to false, instead of default true
Is there an older version I can use that doesn't have this issue?

[stroiman]

I couldn't find @egtoney 's changes, so I wrapped up my own:
https://github.com/stroiman/session-pouchdb-store
It uses es6 object spread syntax - but any node.js version that doesn't support that has reached end of life anyway.
It seems to work (well enough) for my prototype now. (after 2 minutes of testing)

[egtoney]

@stroiman Yeah as it stands all cookie options are sometimes not sent to the browser. So the behavior you're seeing is the browser's default.
There was also a problem with the delete function so I recommend you take a look at the fixed repo I'm currently using (just a fork with a few changes), https://github.com/ViduusEntertainment/pouchdb-seed-database. Clever trick with the object spread syntax!

[TheTechmage]

Hey, @solzimer. Is there anything that I could do to help expedite a fix towards a release? As mentioned in the referenced repo, this bug is causing an infinite redirect loop when using sessions to authenticate a user with multiple (nested) routes in the app.