Skip to content
This repository has been archived by the owner on Dec 12, 2023. It is now read-only.

Refactor session handling #48

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5138f30
Add rolling option
interpretor Nov 24, 2022
a34d5d8
Add saveUninitialized option
interpretor Nov 25, 2022
4dc1d99
Update configuration example
interpretor Nov 28, 2022
1a261b7
Avoid modifying input arguments
interpretor Nov 28, 2022
99ac27f
Update configuration example
interpretor Nov 28, 2022
efc7dc2
Refactor saveUninitialized
interpretor Nov 28, 2022
3629aa8
Add resave option
interpretor Nov 28, 2022
8fbed4a
Update configuration example
interpretor Nov 28, 2022
1cb0215
Use types for session options
interpretor Nov 28, 2022
dbd6bf4
Use types for session options
interpretor Nov 28, 2022
46d9d1f
Use types for session options
interpretor Nov 28, 2022
59b3b09
Merge branch 'add-saveuninitialized-option' into dev
interpretor Nov 28, 2022
79eaf84
Merge branch 'add-resave-option' into dev
interpretor Nov 28, 2022
9d39b4d
Refactor handling of modified sessions
interpretor Nov 29, 2022
293ad80
Remove unnecessary interface declaration
interpretor Nov 29, 2022
50cd690
Merge branch 'main' into refactor-session-handling
interpretor Nov 30, 2022
0621975
Merge branch 'main' into refactor-session-handling
interpretor Dec 1, 2022
477769f
Merge branch 'main' into refactor-session-handling
interpretor Dec 5, 2022
b98f67e
Merge branch 'main' into refactor-session-handling
interpretor Dec 5, 2022
8238d43
Rename variable
interpretor Dec 5, 2022
7b55c78
Add better explanation to saveUninitialized
interpretor Dec 10, 2022
90d68bc
Provide better naming
interpretor Dec 12, 2022
65a5f34
Merge branch 'refactor-session-handling' of github.com:interpretor/nu…
interpretor Dec 12, 2022
0b5b25a
Provide better explanation for resave
interpretor Dec 12, 2022
0418846
Fix indentation and comma
interpretor Dec 12, 2022
dd4b7b3
Merge branch 'main' into refactor-session-handling
interpretor Feb 4, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,11 @@ Here's what the full _default_ module configuration looks like:
// Sessions aren't pinned to the user's IP address
ipPinning: false,
// Expiration of the sessions are not reset to the original expiryInSeconds on every request
rolling: false
rolling: false,
// Forces unmodified session to be saved to the store. Setting `false` is useful for implementing login sessions, reducing server storage usage, or complying with laws that require permission before setting a cookie
saveUninitialized: true,
// Sessions are saved to the store, even if they were never modified during the request. Depending on your store this may be necessary, but it can also create race conditions where a client makes two parallel requests to your server, and changes made to the session in one request may get overwritten when the other request ends
resave: true
},
api: {
// The API is enabled
Expand Down
7 changes: 3 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"argon2": "^0.30.2",
"dayjs": "^1.11.6",
"defu": "^6.1.0",
"fast-deep-equal": "^3.1.3",
"h3": "^1.0.1",
"unstorage": "^1.0.1"
},
Expand Down
4 changes: 3 additions & 1 deletion src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ const defaults: FilledModuleOptions = {
},
domain: false,
ipPinning: false as boolean|SessionIpPinningOptions,
rolling: false
rolling: false,
saveUninitialized: true,
resave: true
},
api: {
isEnabled: true,
Expand Down
48 changes: 28 additions & 20 deletions src/runtime/server/middleware/session/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { deleteCookie, eventHandler, H3Event, parseCookies, setCookie } from 'h3'
import { nanoid } from 'nanoid'
import dayjs from 'dayjs'
import equal from 'fast-deep-equal'
import { SameSiteOptions, Session, SessionOptions } from '../../../../types'
import { dropStorageSession, getStorageSession, setStorageSession } from './storage'
import { processSessionIp, getHashedIpAddress } from './ipPinning'
import { SessionExpired } from './exceptions'
import { resEndProxy } from './resEndProxy'
import { useRuntimeConfig } from '#imports'

const SESSION_COOKIE_NAME = 'sessionId'
Expand Down Expand Up @@ -66,25 +68,24 @@ export const deleteSession = async (event: H3Event) => {
}

const newSession = async (event: H3Event) => {
const runtimeConfig = useRuntimeConfig()
const sessionOptions = runtimeConfig.session.session as SessionOptions
const sessionOptions = useRuntimeConfig().session.session as SessionOptions
const now = new Date()

// (Re-)Set cookie
const sessionId = nanoid(sessionOptions.idLength)
safeSetCookie(event, SESSION_COOKIE_NAME, sessionId, now)

// Store session data in storage
const session: Session = {
id: sessionId,
id: nanoid(sessionOptions.idLength),
createdAt: now,
ip: sessionOptions.ipPinning ? await getHashedIpAddress(event) : undefined
}
await setStorageSession(sessionId, session)

return session
}

const setSession = async (session: Session, event: H3Event) => {
safeSetCookie(event, SESSION_COOKIE_NAME, session.id, session.createdAt)
await setStorageSession(session.id, session)
return session
}

const getSession = async (event: H3Event): Promise<null | Session> => {
// 1. Does the sessionId cookie exist on the request?
const existingSessionId = getCurrentSessionId(event)
Expand All @@ -98,8 +99,7 @@ const getSession = async (event: H3Event): Promise<null | Session> => {
return null
}

const runtimeConfig = useRuntimeConfig()
const sessionOptions = runtimeConfig.session.session as SessionOptions
const sessionOptions = useRuntimeConfig().session.session as SessionOptions
const sessionExpiryInSeconds = sessionOptions.expiryInSeconds

try {
Expand Down Expand Up @@ -143,21 +143,29 @@ const ensureSession = async (event: H3Event) => {

event.context.sessionId = session.id
event.context.session = session
return session

return { ...session }
}

export default eventHandler(async (event: H3Event) => {
const sessionOptions = useRuntimeConfig().session.session as SessionOptions

// 1. Ensure that a session is present by either loading or creating one
await ensureSession(event)
const session = await ensureSession(event)

// 2. Setup a hook that saves any changed made to the session by the subsequent endpoints & middlewares
event.res.on('finish', async () => {
// Session id may not exist if session was deleted
const session = await getSession(event)
if (!session) {
return
resEndProxy(event.node.res, async () => {
const contextSession = event.context.session as Session
const storedSession = await getSession(event)

if (!storedSession) {
// If there isn't a session in the storage yet, save a new session if saveUninitialized is true, or if the session in the event context has been modified
if (sessionOptions.saveUninitialized || !equal(contextSession, session)) {
await setSession(contextSession, event)
}
// Update the session in the storage if resave is true, or if the session in the event context has been modified
} else if (sessionOptions.resave || !equal(contextSession, storedSession)) {
await setStorageSession(storedSession.id, contextSession)
}

await setStorageSession(session.id, event.context.session)
})
})
14 changes: 14 additions & 0 deletions src/runtime/server/middleware/session/resEndProxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import type { ServerResponse } from 'node:http'

type MiddleWare = () => Promise<void>

// Proxy res.end() to get a callback at the end of all event handlers
export const resEndProxy = (res: ServerResponse, middleWare: MiddleWare) => {
const end = res.end

// @ts-ignore Replacing res.end() will lead to type checking error
res.end = async (chunk: any, encoding: BufferEncoding) => {
Comment on lines +9 to +10
Copy link

@valiafetisov valiafetisov Dec 6, 2022

Choose a reason for hiding this comment

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

Overwriting standard node js methods is obviously not recommended. Is there a better way doing that?

  • What is the exact type error that is thrown here?
  • Also, I'm not sure if res.end can be async, since it is expected to return this according to the docs I don't think it's being awaited. Was it at least tested with long timeouts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that there is no alternative, as already mentioned in #41. There is no hook at the end of all event handlers, and no event that fires just before sending the response.

  • res.end returns always a ServerResponse, and here it returns a Promise<ServerResponse>. Also there are multiple overloads, and res.end also accepts a callback. In express session this leads to problems, so they decided to ditch the callback, as it is very rarely used.
  • The types don't allow res.end being async, but it works perfectly. I tested with long timeouts with various conditions, so it must be awaited, but I didn't check it in the node source code.

await middleWare()
return end.call(res, chunk, encoding) as ServerResponse

Choose a reason for hiding this comment

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

Why do you need to typecast it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

res.end.call() returns any, not ServerResponse

}
}
17 changes: 17 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,23 @@ export interface SessionOptions {
* @type boolean
*/
rolling: boolean
/**
* Forces a session that is "uninitialized" to be saved to the store. A session is uninitialized when it is new but not modified.
* Choosing false is useful for implementing login sessions, reducing server storage usage, or complying with laws that require permission before setting a cookie.
* @default true
* @example false
* @type boolean
*/
saveUninitialized: boolean
/**
* Forces the session to be saved back to the session store, even if the session was never modified during the request.
* Depending on your store this may be necessary, but it can also create race conditions where a client makes two parallel requests to your server,
* and changes made to the session in one request may get overwritten when the other request ends, even if it made no changes (this behavior also depends on what store you're using).
* @default true
* @example false
* @type boolean
*/
resave: boolean
}

export interface ApiOptions {
Expand Down