Partially replace deprecated Kinde TS SDK with js-utils ExpressStore and enhance session management #85
Conversation
WalkthroughInitialisation now runs session setup before creating the internal client; session management was refactored to use an ExpressSessionManager per request, setup modules were converted to pure re-exports, Express request session typings were simplified, Vite marks Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant App as Express App
participant setupKinde as setupKinde (bootstrap)
participant setupKindeSession as setupKindeSession
participant Router as Express router / middleware stack
participant getSessionManager as getSessionManager middleware
participant ESMgr as ExpressSessionManager
participant setupInternalClient as setupInternalClient
Dev->>App: create app and call setupKinde(app, config)
App->>setupKinde: initialize
setupKinde->>setupKindeSession: setupKindeSession(app)
setupKindeSession->>App: register session middleware (if missing)
App->>Router: incoming request -> middleware chain
Router->>getSessionManager: invoke getSessionManager middleware
getSessionManager->>ESMgr: create ExpressSessionManager instance (per request)
ESMgr-->>Router: bind async session methods onto req
setupKinde->>setupInternalClient: setupInternalClient(config)
setupInternalClient-->>App: internal client configured
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2024-10-08T23:57:58.113ZApplied to files:
🧬 Code graph analysis (2)src/index.ts (4)
src/setup/sessionManager.ts (2)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/setup/index.ts (1)
1-2: Fix typo in comment.There's a typo in the comment: "teh" should be "the".
-// this file only re-exports the necessary functions and types from teh other files +// this file only re-exports the necessary functions and types from the other filessrc/setup/sessionManager.ts (3)
4-5: Remove duplicate express-session import.The bare import of "express-session" on line 5 appears unnecessary since you're already importing the named exports on line 3.
-import {ExpressSessionManager} from '@kinde/js-utils'; -import "express-session"; +import {ExpressSessionManager} from '@kinde/js-utils';
23-24: Update comment to reflect actual package name.The comment mentions "utils lib" but should reference the actual package name for clarity.
- * Sets up the session manager by creating an instance of our reusable - * `ExpressSessionManager` class for each req. + * Sets up the session manager by creating an instance of the + * `ExpressSessionManager` class from @kinde/js-utils for each request.
52-52: Update comment to reflect the actual package name.The comment should reference the correct package name for accuracy.
- * Attaches the `express-session` middleware and the Kinde ExpressSessionManager. + * Attaches the `express-session` middleware and the ExpressSessionManager from @kinde/js-utils.src/index.ts (1)
14-14: Consider documenting the local union type.The local
KindeGrantTypeunion type could benefit from a JSDoc comment explaining why it's defined locally instead of imported.-// not sure for now what terminology to use here +/** + * Local grant type definition for Kinde setup. + * Limited to the supported grant types for this Express integration. + */ type KindeGrantType = "authorization_code" | "client_credentials";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamlpnpm-workspace.yamlis excluded by!**/*.yaml
📒 Files selected for processing (5)
src/index.ts(1 hunks)src/setup/index.ts(1 hunks)src/setup/sessionManager.ts(3 hunks)src/types/express-session.d.ts(1 hunks)vite.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/setup/sessionManager.ts (2)
src/utils.ts (1)
ExpressMiddleware(13-17)src/types/express-session.d.ts (1)
Request(17-22)
src/index.ts (3)
src/setup/index.ts (2)
setupKindeSession(9-9)setupInternalClient(6-6)src/setup/sessionManager.ts (1)
setupKindeSession(55-60)src/setup/kindeClient.ts (1)
setupInternalClient(97-127)
🔇 Additional comments (5)
vite.config.ts (1)
9-9: LGTM! Correct external dependency addition.Adding
@kinde/js-utilsto the external dependencies is appropriate since it's now being used in the session manager and should remain unbundled.src/setup/index.ts (1)
3-10: Good separation of concerns.Converting this file to a pure re-exporter improves modularity and makes the codebase easier to maintain.
src/index.ts (1)
24-35: Good separation of concerns in setup flow.The sequential setup approach (session → client → routes) is logical and improves maintainability. The function signature simplification also makes the API cleaner.
Note: This change may be breaking for existing users who relied on the generic parameter, but the local union type maintains type safety while simplifying the API.
src/types/express-session.d.ts (2)
8-13: Simplified session interface is cleaner.The simplified
Requestinterface extension with optional session object is much cleaner than the previous approach.
16-23: Verify runtime availability of optional session methods.The session methods are defined as optional properties on the
Requestinterface. Ensure that code using these methods checks for their existence before calling them, since they're only available after the session manager middleware runs.#!/bin/bash # Description: Search for usage of session methods to verify proper existence checks # Expected: Usage should include optional chaining or existence checks echo "Searching for session method usage..." rg -A 3 -B 1 "(setSessionItem|getSessionItem|removeSessionItem|destroySession)" --type ts
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/setup/kindeSetupTypes.ts (1)
50-56: Inconsistent migration: ClientOptions still uses deprecated GrantType.Similar to
ClientType, this type definition still uses the deprecatedGrantTypeinstead of the newKindeGrantType. This creates inconsistency in the migration.Consider updating to use
KindeGrantType:-export type ClientOptions<G> = G extends GrantType.PKCE +export type ClientOptions<G> = G extends KindeGrantType.PKCE ? PKCEClientOptions - : G extends GrantType.AUTHORIZATION_CODE + : G extends KindeGrantType.AUTHORIZATION_CODE ? ACClientOptions - : G extends GrantType.CLIENT_CREDENTIALS + : G extends KindeGrantType.CLIENT_CREDENTIALS ? CCClientOptions : never;
🧹 Nitpick comments (2)
src/setup/sessionManager.test.ts (2)
25-27: Consider avoiding access to Express private propertiesThe tests access
app._router.stackwhich is a private property of Express. This approach could be brittle and may break with future Express updates.Consider using a more robust testing approach:
- const sessionMiddleware = app._router.stack.find( - (layer) => layer.name === 'session' - ); - expect(sessionMiddleware).toBeDefined(); + // Test middleware functionality instead of implementation details + const res = await request(app).get('/test-route'); + expect(res.status).not.toBe(500); // Verify middleware doesn't cause errorsAlso applies to: 35-43
84-91: Improve error handling specificityThe generic catch block doesn't provide specific error information, which could make debugging difficult.
Consider improving error handling:
- } catch { - res.status(500).send('Error destroying session'); - } + } catch (error) { + console.error('Session destruction failed:', error); + res.status(500).send('Error destroying session'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/index.ts(1 hunks)src/mocks.ts(1 hunks)src/setup/index.ts(1 hunks)src/setup/kindeSetupTypes.ts(3 hunks)src/setup/sessionManager.test.ts(1 hunks)src/setup/sessionManager.ts(3 hunks)src/types/express-session.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/mocks.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/index.ts
- src/setup/index.ts
- src/setup/sessionManager.ts
- src/types/express-session.d.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: DanielRivers
PR: kinde-oss/kinde-node-express#26
File: src/setup/sessionManager.ts:10-10
Timestamp: 2024-06-10T19:15:23.818Z
Learning: The session secret in `src/setup/sessionManager.ts` can be set via an environment variable `SESSION_SECRET`, with a fallback to a randomly generated string if the environment variable is not set. This ensures session persistence across server restarts unless the secret is not explicitly set.
Learnt from: DanielRivers
PR: kinde-oss/kinde-node-express#26
File: src/setup/sessionManager.ts:10-10
Timestamp: 2024-10-08T23:57:58.113Z
Learning: The session secret in `src/setup/sessionManager.ts` can be set via an environment variable `SESSION_SECRET`, with a fallback to a randomly generated string if the environment variable is not set. This ensures session persistence across server restarts unless the secret is not explicitly set.
src/setup/sessionManager.test.ts (2)
Learnt from: DanielRivers
PR: kinde-oss/kinde-node-express#26
File: src/setup/sessionManager.ts:10-10
Timestamp: 2024-06-10T19:15:23.818Z
Learning: The session secret in `src/setup/sessionManager.ts` can be set via an environment variable `SESSION_SECRET`, with a fallback to a randomly generated string if the environment variable is not set. This ensures session persistence across server restarts unless the secret is not explicitly set.
Learnt from: DanielRivers
PR: kinde-oss/kinde-node-express#26
File: src/setup/sessionManager.ts:10-10
Timestamp: 2024-10-08T23:57:58.113Z
Learning: The session secret in `src/setup/sessionManager.ts` can be set via an environment variable `SESSION_SECRET`, with a fallback to a randomly generated string if the environment variable is not set. This ensures session persistence across server restarts unless the secret is not explicitly set.
🧬 Code Graph Analysis (1)
src/setup/sessionManager.test.ts (1)
src/setup/sessionManager.ts (1)
setupKindeSession(58-63)
🪛 ast-grep (0.38.1)
src/setup/sessionManager.test.ts
[warning] 32-32: A hard-coded credential was detected. It is not recommended to store credentials in source-code, as this risks secrets being leaked and used by either an internal or external malicious adversary. It is recommended to use environment variables to securely provide credentials or retrieve credentials from a secure vault or HSM (Hardware Security Module).
Context: secret: 'secret'
Note: [CWE-798] Use of Hard-coded Credentials. [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html
(express-session-hardcoded-secret-typescript)
🔇 Additional comments (10)
src/setup/sessionManager.test.ts (3)
1-12: LGTM: Clean import structureThe imports are well-organized and use proper TypeScript typing conventions.
105-112: Excellent session management test coverageThe tests comprehensively cover the ExpressSessionManager functionality including set, get, remove, and destroy operations. The use of
request.agent()properly maintains session state across requests.Also applies to: 114-122, 124-132
94-103: Good verification of method attachmentThis test effectively verifies that the ExpressSessionManager methods are properly attached to the Express request object, which is crucial for the middleware integration.
src/setup/kindeSetupTypes.ts (7)
1-1: Good addition of local KindeGrantType import.This import aligns with the migration from the deprecated SDK.
10-18: Correct migration to KindeGrantType.The generic constraint has been properly updated to use
KindeGrantTypeinstead ofGrantType.
20-25: Correct migration to KindeGrantType enum values.The interface correctly uses
KindeGrantType.AUTHORIZATION_CODEinstead of the deprecatedGrantType.AUTHORIZATION_CODE.
27-31: Correct migration to KindeGrantType enum values.The interface correctly uses
KindeGrantType.PKCEinstead of the deprecatedGrantType.PKCE.
33-36: Correct migration to KindeGrantType enum values.The interface correctly uses
KindeGrantType.CLIENT_CREDENTIALSinstead of the deprecatedGrantType.CLIENT_CREDENTIALS.
42-48: Correct migration to KindeGrantType in conditional type.The conditional type has been properly updated to use
KindeGrantTypeinstead ofGrantTypein all branches.
2-8: Update deprecated SDK import insrc/setup/kindeSetupTypes.tsThis file still pulls types and helpers from the old
@kinde-oss/kinde-typescript-sdk, but the PR’s goal is to migrate fully to@kinde/js-utils:-import type { - GrantType, - createKindeServerClient, - PKCEClientOptions, - ACClientOptions, - CCClientOptions, -} from "@kinde-oss/kinde-typescript-sdk"; +import type { + GrantType, + createKindeServerClient, + PKCEClientOptions, + ACClientOptions, + CCClientOptions, +} from "@kinde/js-utils";• File: src/setup/kindeSetupTypes.ts (lines 2–8)
• Ensure that@kinde/js-utilsindeed exports these identifiers (or adjust the module/path if they live elsewhere).
dtoxvanilla1991
changed the title
dtoxvanilla1991
force-pushed
the
feat/js-utils-express-manager
branch
from
744c0ed to
0c98d32
Compare
dtoxvanilla1991
force-pushed
the
feat/js-utils-express-manager
branch
from
d2296b6 to
c28a25c
Compare
- Replace deprecated Kinde TS SDK session handling with js-utils ExpressStore - Add @kinde/js-utils dependency (v0.29.0) - Add sessionManager tests - Update type definitions for express-session - Configure pnpm workspace
dtoxvanilla1991
force-pushed
the
feat/js-utils-express-manager
branch
from
5ff81bb to
c363d68
Compare
|
|
1 participant
Remove the deprecated
@kinde-oss/kinde-typescript-sdkand integrate@kinde/js-utilsfor improved session management. Update session methods and TypeScript definitions to streamline the Kinde setup process.