feat(core): add credentials sign-in provider#136
Open
halvaradop wants to merge 1 commit intomasterfrom
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe pull request introduces credential-based authentication (username/password sign-in) to the authentication system. It adds types for credentials provider configuration, password hashing/verification utilities, a credentials sign-in endpoint handler, and related logging and error codes, alongside integration into the main auth router. Changes
Sequence DiagramsequenceDiagram
participant Client
participant SignInAction as Sign-In Action
participant CredProvider as Credentials Provider
participant Identity as Identity Schema
participant Session as Session Strategy
participant Response
Client->>SignInAction: POST /signIn/credentials<br/>{username, password}
SignInAction->>SignInAction: Parse request body
SignInAction->>SignInAction: Prepare context<br/>(hash, verify utilities)
SignInAction->>CredProvider: authorize(ctx, request)
alt Provider returns user
CredProvider-->>SignInAction: User object
SignInAction->>Identity: Validate user schema
alt Schema valid
Identity-->>SignInAction: Validated user
SignInAction->>Session: createSession(user)
Session-->>SignInAction: Session token
SignInAction->>SignInAction: Generate CSRF token
SignInAction->>Response: 200 OK<br/>{success, user}
Response-->>Client: Set session & CSRF cookies
else Schema invalid
Identity-->>SignInAction: Validation error
SignInAction->>Response: IDENTITY_VALIDATION_FAILED error
Response-->>Client: Error response
end
else Provider returns null
CredProvider-->>SignInAction: null
SignInAction->>Response: 401 Unauthorized<br/>Invalid credentials
Response-->>Client: Error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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/createAuth.ts (1)
35-44:⚠️ Potential issue | 🟠 MajorRegister
/signIn/credentialsonly when a credentials provider is configured.This mounts the route for every auth instance. In OAuth-only setups, the handler immediately fails with
CREDENTIALS_PROVIDER_NOT_CONFIGURED, so an unsupported auth method shows up as a broken internal route instead of being absent.♻️ Suggested change
export const createAuthInstance = <Identity extends EditableShape<UserShape>>(authConfig: AuthConfig<Identity>) => { const config = createInternalConfig<Identity>(authConfig) + const actions = [ + signInAction(config.context.oauth), + ...(config.context.credentials ? [signInCredentialsAction] : []), + callbackAction(config.context.oauth), + sessionAction, + signOutAction, + csrfTokenAction, + updateSessionAction(config.context.identity), + ] const router = createRouter( - [ - signInAction(config.context.oauth), - signInCredentialsAction, - callbackAction(config.context.oauth), - sessionAction, - signOutAction, - csrfTokenAction, - updateSessionAction(config.context.identity), - ], + actions, config )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/createAuth.ts` around lines 35 - 44, The router currently always registers signInCredentialsAction causing a broken route when no credentials provider exists; update the array passed to createRouter so signInCredentialsAction is only included when a credentials provider is configured (e.g., compute hasCredentials = config.providers?.some(p => p.type === 'credentials') or similar) and conditionally push/add signInCredentialsAction into the list before calling createRouter (referencing createRouter and signInCredentialsAction).
🧹 Nitpick comments (1)
packages/core/src/shared/security.ts (1)
94-111: Version the stored password-hash format now.
iterations:salt:hashworks today, butverifyPasswordhas no way to distinguish PBKDF2-SHA-256 from a future scheme. Adding a scheme/version prefix here avoids a breaking migration once apps start persisting these hashes.♻️ Suggested shape
- return `${iterations}:${saltStr}:${hash}` + return `pbkdf2-sha256:${iterations}:${saltStr}:${hash}`- if (segments.length !== 3) return false - const [iterationsStr, saltStr] = segments + if (segments.length !== 4) return false + const [scheme, iterationsStr, saltStr] = segments + if (scheme !== "pbkdf2-sha256") return falseAlso applies to: 121-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/shared/security.ts` around lines 94 - 111, The current hashPassword returns "iterations:salt:hash" which is ambiguous for future schemes; update hashPassword to prefix the stored string with a stable scheme/version token (e.g. "pbkdf2-sha256:v1$iterations:salt:hash") and ensure the counterpart verifyPassword (and any code that reads these values) is updated to parse the new prefix, support the new "pbkdf2-sha256:v1$..." form, and still accept the legacy "iterations:salt:hash" format for backward compatibility by treating it as the equivalent implicit v0 or mapping it to the new parser; reference the functions hashPassword and verifyPassword when making these changes.
🤖 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/config.ts:
- Line 294: RouterGlobalContext.credentials is typed as
CredentialsProvider<any>, which erases identity typing; change it to use the
provider's default generic instead of any by removing the explicit any so the
type becomes CredentialsProvider (i.e., rely on CredentialsProvider's default
Identity = EditableShape<UserShape>) to preserve correct session/user identity
types and maintain type safety between provider output and RouterGlobalContext.
In `@packages/core/src/actions/signIn/credentials.ts`:
- Around line 13-16: The signInCredentialsAction currently authenticates and
issues cookies without validating request origin or CSRF, so add a precondition
that verifies a CSRF token or trusted origin before any authentication or
session minting: extract and validate a CSRF token from the request body/header
(or validate Origin/Referer) using the available context (e.g., request, jose or
sessionStrategy helpers) and reject requests with missing/invalid tokens with an
early response; only proceed to call identity and set cookies via
cookies/sessionStrategy after the CSRF/origin check passes, and ensure failure
paths log via logger and do not create a session.
- Around line 75-80: The response currently returns the provider-controlled
validatedUser directly (see validatedUser and the Response.json call), which can
leak private fields when identity.skipValidation is used; change the sign-in
response to avoid exposing validatedUser by either returning a minimal safe
shape (e.g., userId/role/flags) built from validatedUser or by omitting user
data entirely and instructing clients to call the session endpoint instead;
update the Response.json payload accordingly and keep the existing headers built
by HeadersBuilder (cacheControl, cookies.sessionToken, cookies.csrfToken)
intact.
- Around line 21-35: The parsed request body assigned to body (from
request.clone().json()) must be runtime-validated before calling
provider.authorize: ensure the value is a plain object (not null/array/number)
and that every own property value is a string; if validation fails, throw the
existing AuthSecurityError("INVALID_REQUEST_BODY", "...") with a clear message.
Update the logic around request.clone().json() and before calling
provider.authorize in this module to perform these checks (validate body is
Record<string,string> and that Object.values(body).every(v => typeof v ===
'string')), and only pass the validated body into provider.authorize.
---
Outside diff comments:
In `@packages/core/src/createAuth.ts`:
- Around line 35-44: The router currently always registers
signInCredentialsAction causing a broken route when no credentials provider
exists; update the array passed to createRouter so signInCredentialsAction is
only included when a credentials provider is configured (e.g., compute
hasCredentials = config.providers?.some(p => p.type === 'credentials') or
similar) and conditionally push/add signInCredentialsAction into the list before
calling createRouter (referencing createRouter and signInCredentialsAction).
---
Nitpick comments:
In `@packages/core/src/shared/security.ts`:
- Around line 94-111: The current hashPassword returns "iterations:salt:hash"
which is ambiguous for future schemes; update hashPassword to prefix the stored
string with a stable scheme/version token (e.g.
"pbkdf2-sha256:v1$iterations:salt:hash") and ensure the counterpart
verifyPassword (and any code that reads these values) is updated to parse the
new prefix, support the new "pbkdf2-sha256:v1$..." form, and still accept the
legacy "iterations:salt:hash" format for backward compatibility by treating it
as the equivalent implicit v0 or mapping it to the new parser; reference the
functions hashPassword and verifyPassword when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0cfda270-a465-4adc-b05c-e3717a534011
📒 Files selected for processing (9)
packages/core/src/@types/config.tspackages/core/src/@types/errors.tspackages/core/src/actions/index.tspackages/core/src/actions/signIn/credentials.tspackages/core/src/createAuth.tspackages/core/src/router/context.tspackages/core/src/shared/logger.tspackages/core/src/shared/security.tspackages/core/tsconfig.json
|
|
||
| export interface RouterGlobalContext<DefaultUser extends User = User> { | ||
| oauth: OAuthProviderRecord | ||
| credentials?: CredentialsProvider<any> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where type erasure occurs and where credentials provider typing is consumed.
rg -nP --type=ts '\bcredentials\?:\s*CredentialsProvider<any>' -C3
rg -nP --type=ts '\binterface\s+RouterGlobalContext\b|\btype\s+AuthRuntimeConfig\b|\bInternalContext\b' -C3
rg -nP --type=ts '\bCredentialsProvider<' -C2Repository: aura-stack-ts/auth
Length of output: 1787
🏁 Script executed:
#!/bin/bash
# Attempt to verify the specific file and line mentioned in the review comment
# and examine CredentialsProvider usage patterns
# First, let's check the specific file and line
if [ -f "packages/core/src/@types/config.ts" ]; then
echo "=== File found at target location ==="
wc -l "packages/core/src/@types/config.ts"
echo "=== Content around line 294 ==="
sed -n '290,300p' "packages/core/src/@types/config.ts" | cat -n
else
echo "File not found at expected path"
fi
# Search for RouterGlobalContext interface definition
echo "=== Searching for RouterGlobalContext ==="
rg -n "interface RouterGlobalContext" --type ts -A 10
# Search for CredentialsProvider type definition
echo "=== Searching for CredentialsProvider definition ==="
rg -n "type CredentialsProvider\|interface CredentialsProvider" --type ts -A 5Repository: aura-stack-ts/auth
Length of output: 1524
🏁 Script executed:
#!/bin/bash
# Search for CredentialsProvider import and definition more broadly
echo "=== Imports in config.ts ==="
head -50 "packages/core/src/@types/config.ts" | cat -n
echo ""
echo "=== Search for CredentialsProvider across codebase ==="
rg -n "CredentialsProvider" --type ts | head -20
echo ""
echo "=== Search for EditableShape and UserShape types ==="
rg -n "type EditableShape\|interface EditableShape\|type UserShape\|interface UserShape" --type ts -A 3Repository: aura-stack-ts/auth
Length of output: 3011
🏁 Script executed:
#!/bin/bash
# Verify how RouterGlobalContext.credentials is actually used/assigned
echo "=== Where RouterGlobalContext.credentials is assigned or used ==="
rg -n "\.credentials\s*=" --type ts -B2 -A2 | head -40
echo ""
echo "=== Assignments to RouterGlobalContext.credentials field ==="
rg -n "credentials\s*:" --type ts -B2 -A2 | grep -A2 -B2 "RouterGlobalContext\|router.*context" | head -40
echo ""
echo "=== Check AuthConfig credentials usage for pattern ==="
rg -n "AuthConfig" --type ts | grep -i credentialsRepository: aura-stack-ts/auth
Length of output: 844
Remove any from RouterGlobalContext.credentials.
The CredentialsProvider<any> type erases the identity type information. Since CredentialsProvider is properly defined with default type CredentialsProvider<Identity extends EditableShape<UserShape> = EditableShape<UserShape>>, the context should use the same default to maintain type safety between provider output and session/user typing.
Recommended fix
export interface RouterGlobalContext<DefaultUser extends User = User> {
oauth: OAuthProviderRecord
- credentials?: CredentialsProvider<any>
+ credentials?: CredentialsProvider<EditableShape<UserShape>>
cookies: CookieStoreConfig
jose: JoseInstance<DefaultUser>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| credentials?: CredentialsProvider<any> | |
| credentials?: CredentialsProvider<EditableShape<UserShape>> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/`@types/config.ts at line 294,
RouterGlobalContext.credentials is typed as CredentialsProvider<any>, which
erases identity typing; change it to use the provider's default generic instead
of any by removing the explicit any so the type becomes CredentialsProvider
(i.e., rely on CredentialsProvider's default Identity =
EditableShape<UserShape>) to preserve correct session/user identity types and
maintain type safety between provider output and RouterGlobalContext.
Comment on lines
+13
to
+16
| export const signInCredentialsAction = createEndpoint("POST", "/signIn/credentials", async (ctx) => { | ||
| const { request, context } = ctx | ||
| const { credentials: provider, sessionStrategy, cookies, jose, logger, identity } = context | ||
|
|
There was a problem hiding this comment.
Add a CSRF or trusted-origin gate before this handler authenticates and sets cookies.
I don't see any precondition here before a session is minted. For a browser-facing credentials endpoint, that leaves you open to login CSRF: a third-party site can submit attacker-controlled credentials and bind the victim browser to that account.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/actions/signIn/credentials.ts` around lines 13 - 16, The
signInCredentialsAction currently authenticates and issues cookies without
validating request origin or CSRF, so add a precondition that verifies a CSRF
token or trusted origin before any authentication or session minting: extract
and validate a CSRF token from the request body/header (or validate
Origin/Referer) using the available context (e.g., request, jose or
sessionStrategy helpers) and reject requests with missing/invalid tokens with an
early response; only proceed to call identity and set cookies via
cookies/sessionStrategy after the CSRF/origin check passes, and ensure failure
paths log via logger and do not create a session.
Comment on lines
+21
to
+35
| let body: Record<string, string> | ||
| try { | ||
| body = await request.clone().json() | ||
| } catch { | ||
| throw new AuthSecurityError("INVALID_REQUEST_BODY", "The request body must be a valid JSON object.") | ||
| } | ||
|
|
||
| const user = await provider.authorize( | ||
| { | ||
| credentials: body, | ||
| hash: hashPassword, | ||
| verify: verifyPassword, | ||
| }, | ||
| request | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' packages/core/src/shared/errors.ts
printf '\n---\n'
sed -n '1,220p' packages/core/src/@types/errors.ts
printf '\n---\n'
rg -n -C2 'INVALID_REQUEST_BODY|AuthSecurityErrorCode|new AuthSecurityError|class AuthSecurityError' packages/core/src packages/core/src/@typesRepository: aura-stack-ts/auth
Length of output: 14986
Validate the credentials payload before calling authorize.
request.json() can return arrays, numbers, or nested objects, but body is declared as Record<string, string>. Without a runtime shape check, invalid client input will escape into provider.authorize, causing provider crashes instead of returning a clean 4xx error. Validate both that the parsed value is an object and that all values are strings.
♻️ Suggested validation
let body: Record<string, string>
try {
- body = await request.clone().json()
+ const parsed: unknown = await request.json()
+ if (!parsed || Array.isArray(parsed) || typeof parsed !== "object") {
+ throw new Error("INVALID_REQUEST_BODY")
+ }
+ const entries = Object.entries(parsed as Record<string, unknown>)
+ if (entries.some(([, value]) => typeof value !== "string")) {
+ throw new Error("INVALID_REQUEST_BODY")
+ }
+ body = Object.fromEntries(entries) as Record<string, string>
} catch {
throw new AuthSecurityError("INVALID_REQUEST_BODY", "The request body must be a valid JSON object.")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let body: Record<string, string> | |
| try { | |
| body = await request.clone().json() | |
| } catch { | |
| throw new AuthSecurityError("INVALID_REQUEST_BODY", "The request body must be a valid JSON object.") | |
| } | |
| const user = await provider.authorize( | |
| { | |
| credentials: body, | |
| hash: hashPassword, | |
| verify: verifyPassword, | |
| }, | |
| request | |
| ) | |
| let body: Record<string, string> | |
| try { | |
| const parsed: unknown = await request.json() | |
| if (!parsed || Array.isArray(parsed) || typeof parsed !== "object") { | |
| throw new Error("INVALID_REQUEST_BODY") | |
| } | |
| const entries = Object.entries(parsed as Record<string, unknown>) | |
| if (entries.some(([, value]) => typeof value !== "string")) { | |
| throw new Error("INVALID_REQUEST_BODY") | |
| } | |
| body = Object.fromEntries(entries) as Record<string, string> | |
| } catch { | |
| throw new AuthSecurityError("INVALID_REQUEST_BODY", "The request body must be a valid JSON object.") | |
| } | |
| const user = await provider.authorize( | |
| { | |
| credentials: body, | |
| hash: hashPassword, | |
| verify: verifyPassword, | |
| }, | |
| request | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/actions/signIn/credentials.ts` around lines 21 - 35, The
parsed request body assigned to body (from request.clone().json()) must be
runtime-validated before calling provider.authorize: ensure the value is a plain
object (not null/array/number) and that every own property value is a string; if
validation fails, throw the existing AuthSecurityError("INVALID_REQUEST_BODY",
"...") with a clear message. Update the logic around request.clone().json() and
before calling provider.authorize in this module to perform these checks
(validate body is Record<string,string> and that Object.values(body).every(v =>
typeof v === 'string')), and only pass the validated body into
provider.authorize.
Comment on lines
+75
to
+80
| const headers = new HeadersBuilder(cacheControl) | ||
| .setCookie(cookies.sessionToken.name, session, cookies.sessionToken.attributes) | ||
| .setCookie(cookies.csrfToken.name, csrfToken, cookies.csrfToken.attributes) | ||
| .toHeaders() | ||
|
|
||
| return Response.json({ success: true, user: validatedUser }, { status: 200, headers }) |
There was a problem hiding this comment.
Don't return the raw authenticated user object.
validatedUser is still provider-controlled, and with identity.skipValidation it can include private fields verbatim. Returning it here turns successful sign-in into a data-exposure path.
♻️ Safer response shape
- return Response.json({ success: true, user: validatedUser }, { status: 200, headers })
+ return Response.json({ success: true }, { status: 200, headers })If the client needs user data immediately after sign-in, fetch it from the existing session endpoint or serialize a dedicated public shape instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const headers = new HeadersBuilder(cacheControl) | |
| .setCookie(cookies.sessionToken.name, session, cookies.sessionToken.attributes) | |
| .setCookie(cookies.csrfToken.name, csrfToken, cookies.csrfToken.attributes) | |
| .toHeaders() | |
| return Response.json({ success: true, user: validatedUser }, { status: 200, headers }) | |
| const headers = new HeadersBuilder(cacheControl) | |
| .setCookie(cookies.sessionToken.name, session, cookies.sessionToken.attributes) | |
| .setCookie(cookies.csrfToken.name, csrfToken, cookies.csrfToken.attributes) | |
| .toHeaders() | |
| return Response.json({ success: true }, { status: 200, headers }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/actions/signIn/credentials.ts` around lines 75 - 80, The
response currently returns the provider-controlled validatedUser directly (see
validatedUser and the Response.json call), which can leak private fields when
identity.skipValidation is used; change the sign-in response to avoid exposing
validatedUser by either returning a minimal safe shape (e.g., userId/role/flags)
built from validatedUser or by omitting user data entirely and instructing
clients to call the session endpoint instead; update the Response.json payload
accordingly and keep the existing headers built by HeadersBuilder (cacheControl,
cookies.sessionToken, cookies.csrfToken) intact.
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
Summary by CodeRabbit