feat(core): introduce server API via createAuth#112
Open
halvaradop wants to merge 2 commits intomasterfrom
Open
feat(core): introduce server API via createAuth#112halvaradop wants to merge 2 commits intomasterfrom
halvaradop wants to merge 2 commits intomasterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR introduces a centralized server API for session management across all framework implementations. New types ( Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthServer as Server (Request Handler)
participant SessionAPI as Server.getSession()
participant CookieStore as Cookie Store
participant JoseValidator as JWT Validator
rect rgb(200, 150, 255, 0.5)
Note over Client,JoseValidator: Old Flow (Local getSession)
Client->>AuthServer: GET /api/protected
AuthServer->>AuthServer: Build request to /api/auth/session
AuthServer->>AuthServer: Forward request headers
AuthServer->>AuthServer: Manually parse JWT from headers
AuthServer->>AuthServer: Decode JWT with jose
AuthServer->>Client: Return session or null
end
rect rgb(150, 200, 150, 0.5)
Note over Client,JoseValidator: New Flow (Centralized Server API)
Client->>AuthServer: GET /api/protected
AuthServer->>SessionAPI: server.getSession(request)
SessionAPI->>CookieStore: getCookie(request, sessionToken)
CookieStore->>SessionAPI: session_token_value
SessionAPI->>JoseValidator: decode JWT
JoseValidator->>SessionAPI: { user, exp, ...claims }
SessionAPI->>SessionAPI: Validate authenticated status
SessionAPI->>AuthServer: SessionResponse { authenticated, session }
AuthServer->>Client: Protected resource or 401
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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: 5
🧹 Nitpick comments (5)
packages/core/src/index.ts (2)
67-78: PincreateAuthreturn type toAuthInstance.Adding an explicit return type makes contract drift detectable at compile time.
♻️ Proposed typing update
-import type { AuthConfig } from "@/@types/index.ts" +import type { AuthConfig, AuthInstance } from "@/@types/index.ts" @@ -export const createAuth = (authConfig: AuthConfig) => { +export const createAuth = (authConfig: AuthConfig): AuthInstance => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 67 - 78, The createAuth function currently returns an inferred type; explicitly annotate its return as AuthInstance to prevent contract drift—update the createAuth signature to return AuthInstance (while keeping the body using createInternalConfig and createRouter with signInAction, callbackAction, sessionAction, signOutAction, csrfTokenAction) so the compiler enforces the public shape (handlers, jose, server) matches the AuthInstance interface.
8-24: Re-export the new server/session types from the public barrel.
SessionResponseandAuthServerAPIare introduced but not exposed here, which makes consumer typing less discoverable.♻️ Proposed export update
export type { AuthConfig, AuthInstance, + AuthServerAPI, JoseInstance, Session, + SessionResponse, User, CookieConfig,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/index.ts` around lines 8 - 24, The public barrel in packages/core/src/index.ts currently re-exports many types but omits the new server/session types; add SessionResponse and AuthServerAPI to the exported types list so consumers can import them from the public package. Update the export block that currently lists AuthConfig, AuthInstance, JoseInstance, Session, User, etc., to include SessionResponse and AuthServerAPI (the unique symbols to add) alongside the existing exports.packages/core/src/context.ts (2)
33-33: Avoid parsingTRUSTED_ORIGINStwice.Line 33 computes the same env array two times; caching it once makes this path clearer.
♻️ Proposed cleanup
+ const trustedOriginsEnv = getEnvArray("TRUSTED_ORIGINS") @@ - trustedOrigins: getEnvArray("TRUSTED_ORIGINS").length > 0 ? getEnvArray("TRUSTED_ORIGINS") : config?.trustedOrigins, + trustedOrigins: trustedOriginsEnv.length > 0 ? trustedOriginsEnv : config?.trustedOrigins,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/context.ts` at line 33, The trustedOrigins initialization currently calls getEnvArray("TRUSTED_ORIGINS") twice; store the result in a local variable (e.g., const envTrusted = getEnvArray("TRUSTED_ORIGINS")) and use that single cached value in the ternary expression that sets trustedOrigins (fallback to config?.trustedOrigins) to avoid duplicate parsing and clarify intent in the trustedOrigins assignment.
10-13: RenamecookieCofigbefore this typo spreads further.Line 10 and Line 35 use
cookieCofig; renaming tocookieConfignow will avoid avoidable churn across the new server API wiring.♻️ Proposed rename in this file
export type InternalContext = GlobalContext & { - cookieCofig: { + cookieConfig: { secure: CookieStoreConfig standard: CookieStoreConfig } } @@ - cookieCofig: { secure: secureCookieStore, standard: standardCookieStore }, + cookieConfig: { secure: secureCookieStore, standard: standardCookieStore },Also applies to: 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/context.ts` around lines 10 - 13, Rename the misspelled property cookieCofig to cookieConfig throughout the module: update the type/interface where cookieCofig is declared and any references (e.g., in the context object and usages that read/write cookieCofig) so the symbol is consistently cookieConfig; ensure related types CookieStoreConfig remain unchanged and run the type-checker to catch remaining references to cookieCofig (update imports/exports if the property is exposed).packages/core/src/actions/session/session.ts (1)
12-17: Prefer direct branching over throw/catch for expected unauthenticated flowLine 13–Line 15 uses exceptions for a non-exceptional branch and then handles it locally. A direct unauthorized return path is simpler and removes unnecessary control-flow indirection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/actions/session/session.ts` around lines 12 - 17, Replace the throw-based unauthenticated branch in the server.getSession handling with a direct unauthorized return: after calling server.getSession(request) check session.authenticated and if false return an unauthorized Response (e.g., Response.json({ error: "INVALID_JWT_TOKEN", message: "Session not authenticated" }, { status: 401, headers: secureApiHeaders })) instead of throwing AuthInternalError; leave the try/catch to handle real exceptions and remove the local throw usage around AuthInternalError so control flow is direct and clearer in the code that calls server.getSession and builds the Response.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/deno/src/index.ts`:
- Line 1: The code uses server.getSession but never imports server; add an
import for the exported server symbol from the module that defines it (the same
module that exports the server instance used elsewhere) and include it alongside
the existing handlers import so calls to server.getSession resolve correctly;
update the import statement that currently reads import { handlers } from
"./auth.ts" to also import server (e.g., import { handlers, server } from
<module-that-exports-server>) so server.getSession(...) works.
- Around line 9-10: The authorization check incorrectly tests the session object
itself (const session = await server.getSession(request); if (!session)) which
is always truthy; update the conditional to check the discriminant returned by
getSession instead (use if (!session.authenticated)) so unauthenticated requests
are rejected; adjust the branch that handles unauthorized access to respond with
the same 401/unauthorized behavior used in other integrations and keep the
variable name session and call site server.getSession(request) unchanged.
In `@apps/oak/src/middleware/with-auth.ts`:
- Line 21: The with-auth middleware references server.getSession(ctx.request)
but `server` is not declared or imported; fix by making the auth server
available to this module—either import the server instance used elsewhere (e.g.,
from your auth/server or auth/index export) or accept it as a parameter to the
withAuth function and use that parameter instead; update the call site to pass
the server if you change the signature, and ensure the code uses the correct
symbol (e.g., server.getSession or injectedServer.getSession) so the getSession
call is in-scope.
In `@apps/supabase/functions/auth/index.ts`:
- Line 15: The current check uses session.user which can throw when getSession()
returns null; change the authorization guard to use the discriminant
session?.authenticated (from the getSession() result) instead of accessing
session.user, and update any downstream logic in the auth handler in
apps/supabase/functions/auth/index.ts to rely on session when authenticated is
true (or handle null session) so you no longer dereference session.user when
session may be null.
In `@packages/core/src/server/create-server.ts`:
- Around line 6-12: getSession currently always reads the session cookie from
ctx.cookies.sessionToken.name (in createServerAPI -> getSession), which breaks
request-dependent cookie selection; update getSession to pick the cookie store
based on the incoming Request (use the same secure/standard selection logic used
elsewhere in your codebase) and then read the session token from that
request-specific cookie store instead of ctx.cookies.sessionToken.name so
HTTP/local requests use the correct cookie namespace.
---
Nitpick comments:
In `@packages/core/src/actions/session/session.ts`:
- Around line 12-17: Replace the throw-based unauthenticated branch in the
server.getSession handling with a direct unauthorized return: after calling
server.getSession(request) check session.authenticated and if false return an
unauthorized Response (e.g., Response.json({ error: "INVALID_JWT_TOKEN",
message: "Session not authenticated" }, { status: 401, headers: secureApiHeaders
})) instead of throwing AuthInternalError; leave the try/catch to handle real
exceptions and remove the local throw usage around AuthInternalError so control
flow is direct and clearer in the code that calls server.getSession and builds
the Response.json.
In `@packages/core/src/context.ts`:
- Line 33: The trustedOrigins initialization currently calls
getEnvArray("TRUSTED_ORIGINS") twice; store the result in a local variable
(e.g., const envTrusted = getEnvArray("TRUSTED_ORIGINS")) and use that single
cached value in the ternary expression that sets trustedOrigins (fallback to
config?.trustedOrigins) to avoid duplicate parsing and clarify intent in the
trustedOrigins assignment.
- Around line 10-13: Rename the misspelled property cookieCofig to cookieConfig
throughout the module: update the type/interface where cookieCofig is declared
and any references (e.g., in the context object and usages that read/write
cookieCofig) so the symbol is consistently cookieConfig; ensure related types
CookieStoreConfig remain unchanged and run the type-checker to catch remaining
references to cookieCofig (update imports/exports if the property is exposed).
In `@packages/core/src/index.ts`:
- Around line 67-78: The createAuth function currently returns an inferred type;
explicitly annotate its return as AuthInstance to prevent contract drift—update
the createAuth signature to return AuthInstance (while keeping the body using
createInternalConfig and createRouter with signInAction, callbackAction,
sessionAction, signOutAction, csrfTokenAction) so the compiler enforces the
public shape (handlers, jose, server) matches the AuthInstance interface.
- Around line 8-24: The public barrel in packages/core/src/index.ts currently
re-exports many types but omits the new server/session types; add
SessionResponse and AuthServerAPI to the exported types list so consumers can
import them from the public package. Update the export block that currently
lists AuthConfig, AuthInstance, JoseInstance, Session, User, etc., to include
SessionResponse and AuthServerAPI (the unique symbols to add) alongside the
existing exports.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
apps/bun/src/auth.tsapps/bun/src/index.tsapps/bun/src/lib/get-session.tsapps/deno/src/auth.tsapps/deno/src/index.tsapps/deno/src/lib/get-session.tsapps/elysia/src/auth.tsapps/elysia/src/lib/get-session.tsapps/elysia/src/plugins/with-auth.tsapps/express/src/auth.tsapps/express/src/lib/get-session.tsapps/express/src/lib/verify-session.tsapps/hono/src/auth.tsapps/hono/src/lib/get-session.tsapps/hono/src/middleware/with-auth.tsapps/oak/src/auth.tsapps/oak/src/lib/get-session.tsapps/oak/src/middleware/with-auth.tsapps/supabase/functions/_shared/auth.tsapps/supabase/functions/_shared/get-session.tsapps/supabase/functions/auth/index.tspackages/core/.vscode/settings.jsonpackages/core/src/@types/index.tspackages/core/src/actions/session/session.tspackages/core/src/context.tspackages/core/src/index.tspackages/core/src/server/create-server.tspackages/core/test/actions/session/session.test.ts
💤 Files with no reviewable changes (7)
- apps/bun/src/lib/get-session.ts
- apps/deno/src/lib/get-session.ts
- apps/supabase/functions/_shared/get-session.ts
- apps/elysia/src/lib/get-session.ts
- apps/oak/src/lib/get-session.ts
- apps/express/src/lib/get-session.ts
- apps/hono/src/lib/get-session.ts
Comment on lines
+6
to
+12
| export const createServerAPI = (ctx: Omit<GlobalContext, "server">) => { | ||
| return { | ||
| getSession: async (request: Request): Promise<SessionResponse> => { | ||
| const { cookies, jose, logger } = ctx | ||
| try { | ||
| const session = getCookie(request, cookies.sessionToken.name) | ||
| const decoded = await jose.decodeJWT(session) |
There was a problem hiding this comment.
getSession is using a static cookie store, which can break standard-cookie session reads.
Line 11 always uses ctx.cookies.sessionToken.name. Since secure/standard selection is request-dependent (see packages/core/src/index.ts Line 38), server.getSession(request) can return unauthenticated for valid HTTP/local sessions.
🐛 Proposed fix (select cookie store per request)
import { getCookie } from "../cookie.ts"
-import { getErrorName, toISOString } from "../utils.ts"
-import type { GlobalContext } from "@aura-stack/router"
+import { getErrorName, toISOString, useSecureCookies } from "../utils.ts"
+import type { InternalContext } from "../context.ts"
import type { JWTStandardClaims, SessionResponse, User } from "@/@types/index.ts"
-export const createServerAPI = (ctx: Omit<GlobalContext, "server">) => {
+export const createServerAPI = (ctx: InternalContext) => {
return {
getSession: async (request: Request): Promise<SessionResponse> => {
- const { cookies, jose, logger } = ctx
+ const { jose, logger } = ctx
+ const cookies = useSecureCookies(request, ctx.trustedProxyHeaders)
+ ? ctx.cookieCofig.secure
+ : ctx.cookieCofig.standard
try {
const session = getCookie(request, cookies.sessionToken.name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/server/create-server.ts` around lines 6 - 12, getSession
currently always reads the session cookie from ctx.cookies.sessionToken.name (in
createServerAPI -> getSession), which breaks request-dependent cookie selection;
update getSession to pick the cookie store based on the incoming Request (use
the same secure/standard selection logic used elsewhere in your codebase) and
then read the session token from that request-specific cookie store instead of
ctx.cookies.sessionToken.name so HTTP/local requests use the correct cookie
namespace.
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
Related Changes to Introduce
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes