feat(core): support pluggable session strategies#125
Merged
halvaradop merged 7 commits intomasterfrom Mar 22, 2026
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughIntroduces a session management layer and types, adds a stateless JWT session strategy with cookie and JOSE managers, updates jose utilities and context to initialize session strategy, and refactors API endpoints to use the strategy instead of direct JWT handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as getSession API
participant SessionStrategy
participant JWTManager
participant CookieManager
Client->>API: GET /session (headers)
API->>SessionStrategy: getSession(new Headers(headers))
SessionStrategy->>CookieManager: getCookie(request)
CookieManager-->>SessionStrategy: { sessionToken }
SessionStrategy->>JWTManager: verifyToken(sessionToken)
JWTManager-->>SessionStrategy: TypedJWTPayload<User>
SessionStrategy-->>API: { session, authenticated: true }
API-->>Client: 200 { session, authenticated: true }
sequenceDiagram
participant Client
participant API as callback API
participant SessionStrategy
participant JWTManager
participant CookieManager
Client->>API: POST /callback (userInfo)
API->>SessionStrategy: createSession(userInfo)
SessionStrategy->>JWTManager: createToken(userInfo)
JWTManager-->>SessionStrategy: sessionToken
SessionStrategy->>CookieManager: setCookie({ sessionToken })
CookieManager-->>API: Headers (Set-Cookie)
API-->>Client: 302 redirect + Set-Cookie
sequenceDiagram
participant Client
participant API as signOut API
participant SessionStrategy
participant CookieManager
Client->>API: POST /signout (headers)
API->>SessionStrategy: destroySession(new Headers(headers), skipCSRFCheck?)
SessionStrategy->>CookieManager: getCookie(request)
CookieManager-->>SessionStrategy: { sessionToken, csrfToken }
SessionStrategy->>SessionStrategy: verifyCSRF(csrfToken, header)
SessionStrategy->>CookieManager: clear()
CookieManager-->>API: Headers (expired cookies)
API-->>Client: 302 redirect + expired cookies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/jose.ts (1)
156-160:⚠️ Potential issue | 🔴 Critical
JWTModenever changes the token primitive; both modes produce sealed tokens.
encodeJWT/decodeJWTalways route throughcreateJWT()which implements sign→encrypt / decrypt→verify regardless of mode. The mode predicate only controls which algorithms are included in the options objects passed toencodeJWT(lines 184-185), not whether both signing and encryption occur. This means"signed"mode still produces encrypted tokens and"encrypted"mode still signs them.Additionally,
signJWSis not a fallback for signed mode—it's a separate function usingjwshelper backed byderivedCsrfTokenKey(line 158), while thejwthelper usesderivedSigningKeyandderivedEncryptionKey(line 157).The configuration accepts asymmetric signing algorithms (RS256, ES256, EdDSA) and asymmetric key-wrapping algorithms (RSA-OAEP, ECDH-ES), but all derived keys are symmetric shared secrets from HKDF (line 151-153). This creates an incompatibility when users configure asymmetric algorithms with this symmetric-only key derivation.
Also applies to: 181-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/jose.ts` around lines 156 - 160, The current implementation always produces sealed tokens because createJWT (used by encodeJWT/decodeJWT) always performs sign→encrypt and decrypt→verify regardless of JWTMode; also signJWS is a separate path and asymmetric algorithms are incompatible with the symmetric keys derived via HKDF (derivedSigningKey, derivedEncryptionKey, derivedCsrfTokenKey). Fix by making the mode actually control the primitive: either (A) split createJWT into two explicit flows or add parameters to createJWT/encodeJWT/decodeJWT to conditionally skip signing or encryption based on JWTMode so "signed" emits only JWS and "encrypted" emits only JWE; ensure signJWS remains a distinct JWS-only path and wire encodeJWT to use it for signed mode; and add validation in the configuration or key-derivation path to reject asymmetric algorithms (RS256/ES256/EdDSA, RSA-OAEP/ECDH-ES) when keys are derived symmetrically via HKDF or implement proper asymmetric key derivation/acceptance (generate/accept RSA/EC/ED keys) so algorithm choices match the derived key types (applies to createJWT/encodeJWT/decodeJWT and related code around derivedSigningKey/derivedEncryptionKey and the algorithm option handling at the lines noted).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/`@types/session.ts:
- Around line 118-138: The public SessionConfig surface currently exposes
unsupported options (RefreshTokenConfig, SessionBehaviorConfig and
adapter-backed strategies like database/hybrid/refreshToken) that are no-ops;
either remove these fields from the exported types or add an initialization-time
validator that rejects them. Update the exported type definitions (remove
refreshToken, SessionBehaviorConfig, database/hybrid fields from
SessionConfig/RefreshTokenConfig) so only the JWT-related fields used by jose.ts
are public, or implement a check in the session initialization code that
inspects config.session and throws a clear error if any of the unsupported keys
(refreshToken, SessionBehaviorConfig, database, hybrid, etc.) are present.
Ensure references to SessionConfig and RefreshTokenConfig in context forwarding
code remain compatible.
In `@packages/core/src/jose.ts`:
- Around line 44-53: getJWTClaims, encodeJWT, and the verification/decryption
paths are ignoring session.jwt.maxAge so token lifetime is never enforced;
update encodeJWT (the function that signs/creates JWTs) to set iat and exp based
on config.session.jwt.maxAge (parse the configured duration or default "15m")
and include those claims (iss/aud already handled in getJWTClaims), and update
the verify/decrypt logic (the functions that validate incoming tokens) to
enforce token age by validating exp/iat (or pass a maxAge option into the JOSE
verification call) and fail on expired tokens; reference getJWTClaims,
encodeJWT, and the verify/decrypt functions when making these changes.
- Around line 61-79: The SessionConfig may contain asymmetric algorithms that
are incompatible with the symmetric keys derived in createJoseInstance; add
validation inside createJoseInstance to detect and reject unsupported algorithm
families (referencing getSignOptions, getEncryptOptions, getVerifyOptions,
getDecryptOptions and the SessionConfig.jwt fields). Specifically, assert that
signing algs are one of HS256/HS384/HS512 (or throw a clear initialization
error) and that key-wrapping/alg values are one of A128KW/A192KW/A256KW/dir (or
similarly reject); do this check at startup inside createJoseInstance and
surface a descriptive error if an asymmetric alg (e.g.,
RS256/ES256/EdDSA/RSA-OAEP/ECDH-ES*) is present so runtime
sign/verify/encrypt/decrypt calls cannot proceed with incompatible symmetric
keys.
In `@packages/jose/src/index.ts`:
- Around line 44-46: The new optional DecodeJWTOptions type is defined but not
being surfaced to consumers because the package's public type entrypoint wasn't
updated/emitted; export the DecodeJWTOptions from the package's public API
(ensure packages/jose/src/index.ts re-exports the DecodeJWTOptions interface)
and then rebuild so the generated .d.ts includes the change, and verify the
package.json "types" (or "typings") entry still points at the generated
declaration file so consumers (e.g., packages/core resolving `@aura-stack/jose`)
see the updated, optional decrypt/verify fields.
---
Outside diff comments:
In `@packages/core/src/jose.ts`:
- Around line 156-160: The current implementation always produces sealed tokens
because createJWT (used by encodeJWT/decodeJWT) always performs sign→encrypt and
decrypt→verify regardless of JWTMode; also signJWS is a separate path and
asymmetric algorithms are incompatible with the symmetric keys derived via HKDF
(derivedSigningKey, derivedEncryptionKey, derivedCsrfTokenKey). Fix by making
the mode actually control the primitive: either (A) split createJWT into two
explicit flows or add parameters to createJWT/encodeJWT/decodeJWT to
conditionally skip signing or encryption based on JWTMode so "signed" emits only
JWS and "encrypted" emits only JWE; ensure signJWS remains a distinct JWS-only
path and wire encodeJWT to use it for signed mode; and add validation in the
configuration or key-derivation path to reject asymmetric algorithms
(RS256/ES256/EdDSA, RSA-OAEP/ECDH-ES) when keys are derived symmetrically via
HKDF or implement proper asymmetric key derivation/acceptance (generate/accept
RSA/EC/ED keys) so algorithm choices match the derived key types (applies to
createJWT/encodeJWT/decodeJWT and related code around
derivedSigningKey/derivedEncryptionKey and the algorithm option handling at the
lines noted).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 512b0618-3fb5-4b56-a5db-445018155be2
📒 Files selected for processing (7)
packages/core/src/@types/adapter.tspackages/core/src/@types/index.tspackages/core/src/@types/session.tspackages/core/src/context.tspackages/core/src/jose.tspackages/core/test/jose.test.tspackages/jose/src/index.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/core/src/api/getSession.ts (1)
6-10: Treat a missing session as a normal miss.
SessionStrategy.getSession()already usesnullfor absent/invalid/expired auth. Throwing here sends every anonymous request through the error path and collapses the reason to a genericError.Suggested fix
const session = await ctx.session.getSession(new Headers(headers)) - if (!session) throw new Error("No session found") + if (!session) { + return { session: null, authenticated: false } + } return { session, authenticated: true, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/api/getSession.ts` around lines 6 - 10, The code currently throws when ctx.session.getSession(new Headers(headers)) returns null; instead treat a null session as a normal cache miss by returning { session: null, authenticated: false } (or equivalent) rather than throwing. Update the getSession logic in getSession (and any callers expecting a throw) to check the result of SessionStrategy.getSession() and return the unauthenticated response when session is null so anonymous requests follow the normal path instead of error handling.packages/core/src/jose.ts (1)
80-105: Verify options includeissuer/audiencebut sign options do not set corresponding claims.
getVerifyOptionssetsissuerandaudiencefor validation (lines 86-87, 101-102), butgetJWTClaimsonly adds these to the payload. For JWS mode, thejwtVerifyfunction validates these claims against the token payload. EnsuregetPayloadClaimsis always called when encoding tokens so these claims are present for verification to succeed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/jose.ts` around lines 80 - 105, getVerifyOptions sets issuer/audience for validation but the token encoding path doesn't guarantee those claims are present; update the token creation/signing flow to always call getPayloadClaims (or ensure getJWTClaims includes issuer/audience) when building the JWS payload so issuer and audience are embedded for jwtVerify to validate. Locate the code that creates/sings tokens (the encode/sign function that currently constructs payloads) and merge/apply getPayloadClaims into the payload before signing; ensure this behavior applies for signed and sealed modes so getVerifyOptions' issuer/audience checks will pass.packages/core/src/session/jwt-strategy.ts (2)
40-43: Stub methods may confuse callers expecting actual functionality.
refreshSessionreturnsundefined(via empty function body), andrevokeSessionis a no-op. TheSessionStrategyinterface documents these as meaningful operations. Consider:
- Throwing
NotImplementedErrorto fail fast- Returning
nullexplicitly fromrefreshSessionto match the documented "Returns null session ... on any failure" contract💡 Proposed change
/** `@todo`: implement refresh session logic */ - const refreshSession = async (_headers: Headers): Promise<any> => {} + const refreshSession = async (_headers: Headers): Promise<Session | null> => { + // JWT strategy: refresh not implemented; return null per interface contract + return null + } - const revokeSession = async (_sessionId: string): Promise<void> => {} + const revokeSession = async (_sessionId: string): Promise<void> => { + // JWT strategy: stateless tokens cannot be revoked server-side + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/session/jwt-strategy.ts` around lines 40 - 43, The stub methods refreshSession and revokeSession currently do nothing and can confuse callers; replace their empty bodies so they fail fast by throwing a NotImplementedError (or a descriptive Error) from both refreshSession(_headers: Headers): Promise<any> and revokeSession(_sessionId: string): Promise<void> — this makes missing implementation explicit to callers and testing; if you prefer the documented contract for refreshSession, instead return Promise.resolve(null) to explicitly return null on failure rather than undefined.
33-35: Blanket catch swallows all errors, hiding expired vs tampered token distinction.Returning
nullfor any exception prevents callers from distinguishing between an expired token (which might trigger a refresh flow) and a tampered/invalid token (which should log out immediately). Per the context snippets,verifyJWSwraps all errors intoJWSVerificationError, so the distinction must be made here if needed.Consider logging or categorizing errors for observability, even if the external behavior remains
null.💡 Example: Log verification failures for observability
- } catch { + } catch (error) { + // Log for observability; could distinguish JWTExpiredError if exposed + console.debug("[jwt-strategy] Token verification failed", error) return null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/session/jwt-strategy.ts` around lines 33 - 35, The blanket catch in the function that calls verifyJWS should be replaced with a caught error variable so you can categorize and log verification failures instead of silently swallowing them: change the empty catch to catch (err) and detect JWSVerificationError (reference the verifyJWS call and the JWSVerificationError type) then log or metric the outcome (e.g., expired vs tampered/invalid) by inspecting error properties (name/message/code/reason) and emit a distinct processLogger.warn/error or metric before returning null; keep external behavior (returning null) but ensure observability by classifying and logging the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/`@types/session.ts:
- Around line 137-140: The type-safety issue is that StatelessStrategyConfig
declares jwt?: JWTConfig but jwt-strategy.ts uses config.jwt! (non-null
assertion); fix by making jwt required: change StatelessStrategyConfig's jwt?:
JWTConfig to jwt: JWTConfig and update any callers/constructors that create a
StatelessStrategyConfig to provide a JWTConfig, or alternatively remove the
non-null assertion in jwt-strategy.ts and add an explicit runtime guard (e.g.,
throw a clear error or handle the undefined case) before using config.jwt;
reference: StatelessStrategyConfig, JWTConfig, and the usage config.jwt! in
jwt-strategy.ts.
In `@packages/core/src/session/cookie-manager.ts`:
- Around line 20-24: clear() currently always uses expiredCookieAttributes which
can differ from the original cookie's path/domain/secure flags; update clear()
so it reads the existing cookie attributes for config().csrfToken.name and
config().sessionToken.name (e.g., by inspecting incoming Set-Cookie/secure
headers or stored cookie metadata) and merge those values into the expired
attributes before calling setCookie. Use the original cookie's path, domain, and
secure flag when building the expiring Set-Cookie entries (fall back to
expiredCookieAttributes if no original attributes are found), and keep using
setCookie on the same names so browsers reliably remove the cookies.
In `@packages/core/src/session/jwt-manager.ts`:
- Around line 12-15: The selection logic for createToken and verifyToken
currently falls through to JWE for any mode other than "sealed" or "signed", so
update the factory in jwt-manager.ts to explicitly handle only "sealed" and
"signed" and throw a clear error for any other mode; replace the ternary cascade
with an explicit switch/if on the mode variable and set createToken/verifyToken
to jose.encodeJWT/jose.decodeJWT for "sealed" and jose.signJWS/jose.verifyJWS
for "signed", and throw (e.g. new Error) with a descriptive message when mode is
unrecognized instead of defaulting to jose.encryptJWE/jose.decryptJWE.
In `@packages/core/src/session/jwt-strategy.ts`:
- Around line 15-16: createJWTStrategy currently calls
createJWTManager(config.jwt!, jose) using a non-null assertion which can throw
at runtime if config.jwt is undefined; update createJWTStrategy to validate the
presence of config.jwt (from JWTStrategyOptions) before calling createJWTManager
and either throw a clear, descriptive error (e.g., "Missing jwt configuration
for JWT strategy") or supply a safe default JWTConfig, then pass that validated
value into createJWTManager; reference the functions createJWTStrategy and
createJWTManager and the option type JWTStrategyOptions when making this change.
- Around line 25-32: The check `if (!user)` is ineffective because destructuring
`const { exp, iat: _iat, jti: _jti, nbf: _nbf, aud: _aud, iss: _iss, ...user } =
decoded` always yields an object; instead verify whether `user` has any own
properties (e.g., Object.keys(user).length === 0) and return null for an empty
user object. Also change the `expires` logic so it does not default to epoch 0
when `exp` is absent: set `expires` to null (or omit it) when `exp` is
undefined, otherwise compute `new Date(exp * 1000).toISOString()`; update the
return shape accordingly where `expires` may be null/undefined. Ensure this
change is applied to the function handling the JWT decoding and returned object
(the block using `decoded`, `user`, and `exp`).
---
Nitpick comments:
In `@packages/core/src/api/getSession.ts`:
- Around line 6-10: The code currently throws when ctx.session.getSession(new
Headers(headers)) returns null; instead treat a null session as a normal cache
miss by returning { session: null, authenticated: false } (or equivalent) rather
than throwing. Update the getSession logic in getSession (and any callers
expecting a throw) to check the result of SessionStrategy.getSession() and
return the unauthenticated response when session is null so anonymous requests
follow the normal path instead of error handling.
In `@packages/core/src/jose.ts`:
- Around line 80-105: getVerifyOptions sets issuer/audience for validation but
the token encoding path doesn't guarantee those claims are present; update the
token creation/signing flow to always call getPayloadClaims (or ensure
getJWTClaims includes issuer/audience) when building the JWS payload so issuer
and audience are embedded for jwtVerify to validate. Locate the code that
creates/sings tokens (the encode/sign function that currently constructs
payloads) and merge/apply getPayloadClaims into the payload before signing;
ensure this behavior applies for signed and sealed modes so getVerifyOptions'
issuer/audience checks will pass.
In `@packages/core/src/session/jwt-strategy.ts`:
- Around line 40-43: The stub methods refreshSession and revokeSession currently
do nothing and can confuse callers; replace their empty bodies so they fail fast
by throwing a NotImplementedError (or a descriptive Error) from both
refreshSession(_headers: Headers): Promise<any> and revokeSession(_sessionId:
string): Promise<void> — this makes missing implementation explicit to callers
and testing; if you prefer the documented contract for refreshSession, instead
return Promise.resolve(null) to explicitly return null on failure rather than
undefined.
- Around line 33-35: The blanket catch in the function that calls verifyJWS
should be replaced with a caught error variable so you can categorize and log
verification failures instead of silently swallowing them: change the empty
catch to catch (err) and detect JWSVerificationError (reference the verifyJWS
call and the JWSVerificationError type) then log or metric the outcome (e.g.,
expired vs tampered/invalid) by inspecting error properties
(name/message/code/reason) and emit a distinct processLogger.warn/error or
metric before returning null; keep external behavior (returning null) but ensure
observability by classifying and logging the error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ab6bcc1-f814-4347-8967-d6d45499342c
📒 Files selected for processing (13)
packages/core/src/@types/index.tspackages/core/src/@types/session.tspackages/core/src/actions/callback/callback.tspackages/core/src/api/getSession.tspackages/core/src/context.tspackages/core/src/cookie.tspackages/core/src/createAuth.tspackages/core/src/jose.tspackages/core/src/session/cookie-manager.tspackages/core/src/session/jwt-manager.tspackages/core/src/session/jwt-strategy.tspackages/core/src/session/session.tspackages/core/test/actions/session/session.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/context.ts
- packages/core/src/@types/index.ts
halvaradop
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/core/src/session/manager/cookie.ts (1)
20-24:⚠️ Potential issue | 🟠 MajorExpire cookies with the same scope they were issued with.
Lines 22-23 still spread
expiredCookieAttributeslast, so its defaultpathandsecurevalues override the configured cookie attributes. Sign-out can miss cookies created on a non-default path or withsecure: falsein local HTTP flows.🔧 Suggested fix
const clear = () => { return new HeadersBuilder(secureApiHeaders) - .setCookie(store().csrfToken.name, "", { ...store().csrfToken.attributes, ...expiredCookieAttributes }) - .setCookie(store().sessionToken.name, "", { ...store().sessionToken.attributes, ...expiredCookieAttributes }) + .setCookie(store().csrfToken.name, "", { + ...expiredCookieAttributes, + ...store().csrfToken.attributes, + expires: new Date(0), + maxAge: 0, + }) + .setCookie(store().sessionToken.name, "", { + ...expiredCookieAttributes, + ...store().sessionToken.attributes, + expires: new Date(0), + maxAge: 0, + }) .toHeaders() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/session/manager/cookie.ts` around lines 20 - 24, The clear function currently spreads expiredCookieAttributes last when calling HeadersBuilder.setCookie, causing expired defaults (path, secure) to override the original cookie scope; update the calls in clear so expiredCookieAttributes are spread first and the original attributes (store().csrfToken.attributes and store().sessionToken.attributes) are spread after, preserving the original path/secure scope when expiring cookies (references: clear, HeadersBuilder, setCookie, expiredCookieAttributes, store().csrfToken.attributes, store().sessionToken.attributes).
🧹 Nitpick comments (2)
packages/core/src/@types/session.ts (2)
99-107: OptionalmodeinJWTSealedModeweakens discriminated union.
JWTSignedModeandJWTEncryptedModehave requiredmodeproperties, butJWTSealedModedeclaresmode?: "sealed". Whenmodeisundefined, TypeScript cannot discriminateJWTConfigBasemembers properly, potentially allowing invalid algorithm combinations to slip through.If "sealed" is the default mode, consider making
moderequired here too and applying the default at runtime during config initialization rather than in the type.♻️ Make mode required for proper discrimination
export type JWTSealedMode = { - mode?: "sealed" + mode: "sealed" signingAlgorithm?: JWTSigningAlgorithm keyAlgorithm?: JWTKeyAlgorithm - encryptionAlgorithm?: JwtEncryptionAlgorithm + encryptionAlgorithm?: JWTEncryptionAlgorithm }Then apply
mode: "sealed"as the default in the config initialization code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 99 - 107, JWTSealedMode currently declares mode?: "sealed", which breaks the discriminated union JWTConfigBase with JWTSignedMode and JWTEncryptedMode; change JWTSealedMode to require mode: "sealed" so TypeScript can discriminate properly, and if you want "sealed" as the default, set that default during runtime config initialization (not in the type) where JWTConfigBase is constructed/parsed.
83-84: Naming inconsistency:JwtEncryptionAlgorithmvsJWT*prefix pattern.All other JWT-related types use uppercase
JWTprefix (JWTSigningAlgorithm,JWTKeyAlgorithm,JWTMode, etc.), but this one usesJwt. Consider renaming for consistency.♻️ Suggested rename
-export type JwtEncryptionAlgorithm = "A128CBC-HS256" | "A192CBC-HS384" | "A256CBC-HS512" | "A128GCM" | "A192GCM" | "A256GCM" +export type JWTEncryptionAlgorithm = "A128CBC-HS256" | "A192CBC-HS384" | "A256CBC-HS512" | "A128GCM" | "A192GCM" | "A256GCM"Update usages in
JWTEncryptedModeandJWTSealedModeaccordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 83 - 84, Rename the type JwtEncryptionAlgorithm to JWTEncryptionAlgorithm to match existing JWT-* naming, and update all references (notably JWTEncryptedMode and JWTSealedMode) to use the new name; ensure exported type declaration and any imports/uses across the codebase are updated to the new identifier to keep naming consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/`@types/index.ts:
- Line 197: RouterGlobalContext.secret is still typed as string while
config?.secret and ctx.secret can be JWTKey (e.g., Uint8Array or CryptoKey);
update the RouterGlobalContext.secret type to match the widened JWTKey type (or
JWTKey | undefined if optional) so createContext(), ctx.secret, and any
downstream code accept Uint8Array/CryptoKey values; search for
RouterGlobalContext and change the secret property type from string to JWTKey
(or optional JWTKey) to align types.
In `@packages/core/src/`@types/session.ts:
- Around line 167-171: The docstring for refreshSession says it "Returns null
session + cookie-clearing response" but the signature refreshSession(request:
Headers): Promise<Session | null> cannot return headers; update either the
docstring or the function signature: if refreshSession must return headers on
failure, change its return type to Promise<Session | null | Headers> or
Promise<{ session: Session | null; headers?: Headers }>, and update all callers;
otherwise remove the cookie-clearing claim and document how cookie clearing is
performed (e.g., via a separate destroySession method or injected response
handler). Refer to refreshSession and destroySession in your changes to keep
behavior consistent.
In `@packages/core/src/session/index.ts`:
- Around line 1-16: The factory createSessionStrategy currently only builds the
"jwt" path via createStatelessStrategy but the thrown error lists "database" and
"hybrid" as valid options; update the code so the advertised valid options match
what's implemented: either add switch cases for "database" and "hybrid" that
return the appropriate SessionStrategy implementations (or
placeholder/NotImplemented errors) or change the error text to only list "jwt".
Locate createSessionStrategy, the strategy variable and the switch block and
make the error message or added branches consistent with the actual supported
strategies.
In `@packages/core/src/session/strategies/stateless.ts`:
- Around line 13-18: The getSession function currently lets
cookieConfig.getCookie and jwt.verifyToken throw for normal anonymous flows;
update getSession to treat missing or invalid session cookies as unauthenticated
by wrapping the cookie extraction and token verification in try/catch blocks (or
checking for falsy values) and returning null on any error instead of
propagating exceptions; specifically handle failures from
cookieConfig.getCookie(...) and jwt.verifyToken(sessionToken) so that getSession
returns null for absent/expired/invalid tokens while preserving successful
decoding and the existing destructuring of decoded into user.
---
Duplicate comments:
In `@packages/core/src/session/manager/cookie.ts`:
- Around line 20-24: The clear function currently spreads
expiredCookieAttributes last when calling HeadersBuilder.setCookie, causing
expired defaults (path, secure) to override the original cookie scope; update
the calls in clear so expiredCookieAttributes are spread first and the original
attributes (store().csrfToken.attributes and store().sessionToken.attributes)
are spread after, preserving the original path/secure scope when expiring
cookies (references: clear, HeadersBuilder, setCookie, expiredCookieAttributes,
store().csrfToken.attributes, store().sessionToken.attributes).
---
Nitpick comments:
In `@packages/core/src/`@types/session.ts:
- Around line 99-107: JWTSealedMode currently declares mode?: "sealed", which
breaks the discriminated union JWTConfigBase with JWTSignedMode and
JWTEncryptedMode; change JWTSealedMode to require mode: "sealed" so TypeScript
can discriminate properly, and if you want "sealed" as the default, set that
default during runtime config initialization (not in the type) where
JWTConfigBase is constructed/parsed.
- Around line 83-84: Rename the type JwtEncryptionAlgorithm to
JWTEncryptionAlgorithm to match existing JWT-* naming, and update all references
(notably JWTEncryptedMode and JWTSealedMode) to use the new name; ensure
exported type declaration and any imports/uses across the codebase are updated
to the new identifier to keep naming consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c024090f-ec50-4f1d-bd84-c67eb95285d9
📒 Files selected for processing (13)
packages/core/package.jsonpackages/core/src/@types/index.tspackages/core/src/@types/session.tspackages/core/src/api/getSession.tspackages/core/src/api/signOut.tspackages/core/src/context.tspackages/core/src/errors.tspackages/core/src/jose.tspackages/core/src/session/index.tspackages/core/src/session/manager/cookie.tspackages/core/src/session/manager/jose.tspackages/core/src/session/strategies/stateless.tspackages/jose/package.json
✅ Files skipped from review due to trivial changes (2)
- packages/jose/package.json
- packages/core/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/api/getSession.ts
- packages/core/src/jose.ts
This was referenced Mar 23, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
This pull request introduces a configurable session management system based on pluggable session strategies, including support for
jwt, with future extensibility fordatabaseand hybrid approaches.The update introduces a new abstraction layer through the
SessionStrategyinterface, which defines the contract for all session strategies. This design focuses on what a session strategy should do, rather than how it is implemented, enabling flexible and extensible session management across the library.Additionally, this PR enhances JWT-based session management by introducing multiple JWT modes:
signedencryptedsealed(default: signed + encrypted, secure by default)These modes allow fine-grained control over how sessions are created and validated. Depending on the selected mode, developers can configure values such as issuer, audience, signing algorithm, key management algorithm, and encryption algorithm. These values are used internally for validation and consistency across authentication flows.
Usage
mode: sealed(by default)mode: signedmode: encryptedSummary by CodeRabbit
Release Notes
New Features
Chores