-
Notifications
You must be signed in to change notification settings - Fork 11
Implement session management with express-session and MemoryStorage #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dtoxvanilla1991
wants to merge
1
commit into
kinde-oss:main
Choose a base branch
from
dtoxvanilla1991:feat/add-express-manager-storage
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| import { describe, it, expect, beforeEach, vi } from "vitest"; | ||
| import { ExpressStore } from "../../main"; | ||
| import { StorageKeys } from "../types"; | ||
| import type { Request } from "express"; | ||
| import { storageSettings } from ".."; | ||
|
|
||
| const mockRequest = ( | ||
| sessionData: Record<string, unknown> | null, | ||
| destroyError: Error | null = null, | ||
| ) => { | ||
| const session = sessionData | ||
| ? { | ||
| ...sessionData, | ||
| destroy: vi.fn((callback: (err: Error | null) => void) => { | ||
| callback(destroyError); | ||
| }), | ||
| } | ||
| : undefined; | ||
|
|
||
| return { | ||
| session, | ||
| } as unknown as Request; | ||
| }; | ||
|
|
||
| describe("ExpressStore", () => { | ||
| let req: Request; | ||
| let sessionManager: ExpressStore; | ||
|
|
||
| describe("constructor", () => { | ||
| it("should throw an error if session is not available on the request", () => { | ||
| req = mockRequest(null); | ||
| expect(() => new ExpressStore(req)).toThrow( | ||
| "Session not available on the request. Please ensure the 'express-session' middleware is configured and running before the Kinde middleware.", | ||
| ); | ||
| }); | ||
|
|
||
| it("should not throw an error if session is available on the request", () => { | ||
| req = mockRequest({}); | ||
| expect(() => new ExpressStore(req)).not.toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("with a valid session", () => { | ||
| const keyPrefix = storageSettings.keyPrefix; | ||
| beforeEach(() => { | ||
| const initialSession = { | ||
| [`${keyPrefix}${StorageKeys.accessToken}0`]: "access-token", | ||
| [`${keyPrefix}${StorageKeys.idToken}0`]: "id-token", | ||
| }; | ||
| req = mockRequest(initialSession); | ||
| sessionManager = new ExpressStore(req); | ||
| }); | ||
|
|
||
| it("should get an item from the session", async () => { | ||
| const accessToken = await sessionManager.getSessionItem( | ||
| StorageKeys.accessToken, | ||
| ); | ||
| expect(accessToken).toBe("access-token"); | ||
| }); | ||
|
|
||
| it("should return null for a non-existent item", async () => { | ||
| const refreshToken = await sessionManager.getSessionItem( | ||
| StorageKeys.refreshToken, | ||
| ); | ||
| expect(refreshToken).toBeNull(); | ||
| }); | ||
|
|
||
| it("should set an item in the session", async () => { | ||
| await sessionManager.setSessionItem( | ||
| StorageKeys.refreshToken, | ||
| "refresh-token", | ||
| ); | ||
| expect(req.session![`${keyPrefix}${StorageKeys.refreshToken}0`]).toBe( | ||
| "refresh-token", | ||
| ); | ||
| }); | ||
|
|
||
| it("should remove an item from the session", async () => { | ||
| await sessionManager.removeSessionItem(StorageKeys.accessToken); | ||
| expect( | ||
| req.session![`${keyPrefix}${StorageKeys.accessToken}0`], | ||
| ).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("should destroy the session", async () => { | ||
| await sessionManager.destroySession(); | ||
| expect(req.session!.destroy).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should reject with an error if destroying the session fails", async () => { | ||
| const error = new Error("Failed to destroy Kinde session"); | ||
| req = mockRequest({}, error); | ||
| sessionManager = new ExpressStore(req); | ||
| await expect(sessionManager.destroySession()).rejects.toThrow(error); | ||
| }); | ||
| }); | ||
|
|
||
| describe("splitting and reassembly logic", () => { | ||
| const longString = "a".repeat(5000); // longer than default maxLength (2000) | ||
| const keyPrefix = storageSettings.keyPrefix; | ||
| const maxLength = storageSettings.maxLength; | ||
| let req: Request; | ||
| let sessionManager: ExpressStore; | ||
|
|
||
| beforeEach(() => { | ||
| req = mockRequest({}); | ||
| sessionManager = new ExpressStore(req); | ||
| }); | ||
|
|
||
| it("should split and store a long string value across multiple session keys", async () => { | ||
| await sessionManager.setSessionItem(StorageKeys.state, longString); | ||
| expect(req.session![`${keyPrefix}state0`]).toBe( | ||
| longString.slice(0, maxLength), | ||
| ); | ||
| expect(req.session![`${keyPrefix}state1`]).toBe( | ||
| longString.slice(maxLength, maxLength * 2), | ||
| ); | ||
| expect(req.session![`${keyPrefix}state2`]).toBe( | ||
| longString.slice(maxLength * 2), | ||
| ); | ||
| expect(req.session![`${keyPrefix}state3`]).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("should reassemble a long string value from multiple session keys", async () => { | ||
| // Simulate split storage | ||
| req.session![`${keyPrefix}state0`] = longString.slice(0, maxLength); | ||
| req.session![`${keyPrefix}state1`] = longString.slice( | ||
| maxLength, | ||
| maxLength * 2, | ||
| ); | ||
| req.session![`${keyPrefix}state2`] = longString.slice(maxLength * 2); | ||
| const value = await sessionManager.getSessionItem(StorageKeys.state); | ||
| expect(value).toBe(longString); | ||
| }); | ||
|
|
||
| it("should remove all split keys for a long string value", async () => { | ||
| req.session![`${keyPrefix}state0`] = "part1"; | ||
| req.session![`${keyPrefix}state1`] = "part2"; | ||
| req.session![`${keyPrefix}state2`] = "part3"; | ||
| await sessionManager.removeSessionItem(StorageKeys.state); | ||
| expect(req.session![`${keyPrefix}state0`]).toBeUndefined(); | ||
| expect(req.session![`${keyPrefix}state1`]).toBeUndefined(); | ||
| expect(req.session![`${keyPrefix}state2`]).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("should store and retrieve non-string values without splitting", async () => { | ||
| const obj = { foo: "bar" }; | ||
| await sessionManager.setSessionItem(StorageKeys.nonce, obj); | ||
| expect(req.session![`${keyPrefix}nonce0`]).toEqual(obj); | ||
| const value = await sessionManager.getSessionItem(StorageKeys.nonce); | ||
| expect(value).toEqual(obj); // Should return the original object | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| import type { Request } from "express"; | ||
| import { SessionBase, StorageKeys, type SessionManager } from "../types.js"; | ||
| import { storageSettings } from "../index.js"; | ||
| import { splitString } from "../../utils/splitString.js"; | ||
|
|
||
| declare global { | ||
| // eslint-disable-next-line @typescript-eslint/no-namespace | ||
| namespace Express { | ||
| interface Request { | ||
| session?: { | ||
| [key: string]: unknown; | ||
| destroy: (callback: (err?: Error | null) => void) => void; | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Provides an Express session-based session manager. | ||
| * This class acts as a structured interface to the 'req.session' object, | ||
| * that is populated by the express-session middleware. | ||
| * @class ExpressStore | ||
| */ | ||
| export class ExpressStore<V extends string = StorageKeys> | ||
| extends SessionBase<V> | ||
| implements SessionManager<V> | ||
| { | ||
| /** | ||
| * Indicates this store uses async operations | ||
| */ | ||
| asyncStore = true; | ||
|
|
||
| /** | ||
| * The Express req obj which holds the session's data | ||
| */ | ||
| private req: Request; | ||
|
|
||
| constructor(req: Request) { | ||
| super(); | ||
| if (!req.session) { | ||
| throw new Error( | ||
| "Session not available on the request. Please ensure the 'express-session' middleware is configured and running before the Kinde middleware.", | ||
| ); | ||
| } | ||
| this.req = req; | ||
| } | ||
|
|
||
| /** | ||
| * Gets a value from the Express session. | ||
| * @param {string} itemKey | ||
| * @returns {Promise<unknown | null>} | ||
| */ | ||
| async getSessionItem(itemKey: V | StorageKeys): Promise<unknown | null> { | ||
| // Reassemble split string values if present | ||
| const baseKey = `${storageSettings.keyPrefix}${String(itemKey)}`; | ||
| if (this.req.session![`${baseKey}0`] === undefined) { | ||
| return null; | ||
| } | ||
|
|
||
| // if under settingConfig maxLength - return as-is | ||
| if (this.req.session![`${baseKey}1`] === undefined) { | ||
| return this.req.session![`${baseKey}0`]; | ||
| } | ||
|
|
||
| // Multiple items exist, concatenate them as strings (for split strings) | ||
| let itemValue = ""; | ||
| let index = 0; | ||
| let key = `${baseKey}${index}`; | ||
| while (this.req.session![key] !== undefined) { | ||
| itemValue += this.req.session![key] as string; | ||
| index++; | ||
| key = `${baseKey}${index}`; | ||
| } | ||
| return itemValue; | ||
| } | ||
|
|
||
| /** | ||
| * Sets a value in the Express session. | ||
| * @param {string} itemKey | ||
| * @param {unknown} itemValue | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async setSessionItem( | ||
| itemKey: V | StorageKeys, | ||
| itemValue: unknown, | ||
| ): Promise<void> { | ||
| // Remove any existing split items first | ||
| await this.removeSessionItem(itemKey); | ||
| const baseKey = `${storageSettings.keyPrefix}${String(itemKey)}`; | ||
| if (typeof itemValue === "string") { | ||
| splitString(itemValue, storageSettings.maxLength).forEach( | ||
| (splitValue, index) => { | ||
| this.req.session![`${baseKey}${index}`] = splitValue; | ||
| }, | ||
| ); | ||
| return; | ||
| } | ||
| this.req.session![`${baseKey}0`] = itemValue; | ||
| } | ||
|
|
||
| /** | ||
| * Removes a value from the Express session. | ||
| * @param {string} itemKey | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async removeSessionItem(itemKey: V | StorageKeys): Promise<void> { | ||
| // Remove all items with the key prefix | ||
| const baseKey = `${storageSettings.keyPrefix}${String(itemKey)}`; | ||
| for (const key in this.req.session!) { | ||
| if (key.startsWith(baseKey)) { | ||
| delete this.req.session![key]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Clears the entire Express session. | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async destroySession(): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| this.req.session!.destroy((err) => { | ||
| if (err) { | ||
| return reject(err); | ||
| } | ||
| resolve(); | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, shouldn't there be a this.notifyListeners() call in all session methods (setSessionItem, removeSessionItem, destroySession)? The other stores (memory, localStorage) all call it.