Skip to content

Comments

OLH-2783: Use a session scoped session cookie#2273

Draft
alex9smith wants to merge 1 commit intomainfrom
OLH-2783-use-session-scoped-session-cookie
Draft

OLH-2783: Use a session scoped session cookie#2273
alex9smith wants to merge 1 commit intomainfrom
OLH-2783-use-session-scoped-session-cookie

Conversation

@alex9smith
Copy link
Contributor

Proposed changes

What changed

Remove the max age definition from our session cookie config. This means express-session won't set an Expires field on the cookie and therefore browsers will treat our session cookie as scoped to that browser session.

In practice, this means that we'll log users out when they close the last tab / window with Home open in it.

Why did it change

Related links

Checklists

Environment variables or secrets

  • Application configuration is up-to-date
  • Added to local startup config

How to review

Remove the max age definition from our session cookie config. This means
express-session won't set an `Expires` field on the cookie and therefore
browsers will treat our session cookie as scoped to that browser
session.

In practice, this means that we'll log users out when they close the last
tab / window with Home open in it.
@sonarqubecloud
Copy link

Copy link
Contributor

@liegeandlief liegeandlief left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user doesn't close their browser then their session will last as long it exists in DynamoDB. Where is the expiry of sessions in DynamoDB configured? Do we need to check and maybe adjust that expiry time

secret: string
): any {
return {
name: "ams",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the name here is redundant as the cookies seem to get called am from the session(...) call in app.ts

@alex9smith
Copy link
Contributor Author

Hmm good point. We do have the TTL configured on that table but I'm not sure how this expires object is set.

TimeToLiveSpecification:
AttributeName: "expires"
Enabled: true

@alex9smith
Copy link
Contributor Author

It's set by the library we're using and will default to "now plus a day" if our session cookie doesn't have an expiry time. That might not work for us 😞

https://github.com/ca98am79/connect-dynamodb/blob/eeeb8d384fd71e7571f6c586bee0813b3e9d7702/lib/connect-dynamodb.js#L340-L352

@liegeandlief
Copy link
Contributor

liegeandlief commented Jul 2, 2025

It's set by the library we're using and will default to "now plus a day" if our session cookie doesn't have an expiry time. That might not work for us 😞

https://github.com/ca98am79/connect-dynamodb/blob/eeeb8d384fd71e7571f6c586bee0813b3e9d7702/lib/connect-dynamodb.js#L340-L352

Frustrating that the cookie and session expiry can't be configured independently 😖. I wonder how active that library is and whether we could open a PR on it

@alex9smith
Copy link
Contributor Author

I've opened a PR with the feature - we shall see!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants