feat(core): add credentials sign-in provider#136
Merged
halvaradop merged 4 commits intomasterfrom Apr 9, 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 (4)
📝 WalkthroughWalkthroughAdds username/password credentials authentication: new types and context wiring, PBKDF2-SHA256 password helpers, a credentials sign-in API + endpoint + client method, logging entries, and tests; the router/context and API surfaces are extended to carry and use a credentials provider. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Endpoint as "SignIn Endpoint"
participant API as "signInCredentials API"
participant Provider as "Credentials Provider"
participant Session as "Session Strategy"
participant Logger as "Logger"
participant Response as "HTTP Response"
Client->>Endpoint: POST /signIn/credentials {username,password,redirectTo?}
Endpoint->>API: signInCredentials({ctx, payload, request, headers, redirectTo})
API->>Provider: authorize({credentials, deriveSecret, verifySecret})
Provider-->>API: identity | null
alt authorization fails
API->>Logger: INVALID_CREDENTIALS (warning)
API-->>Response: {success:false} + headers (401)
else authorization succeeds
API->>Session: createSession(identity)
Session-->>API: session token
API->>API: createCSRF(token)
API->>Logger: CREDENTIALS_SIGN_IN_SUCCESS (info)
API-->>Response: {success:true, redirectURL} + Set-Cookie headers (200)
end
Response-->>Client: JSON + Set-Cookie
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/core/src/actions/signIn/credentials.ts (1)
24-34:⚠️ Potential issue | 🔴 CriticalAdd a CSRF or trusted-origin gate before credential sign-in executes.
At Line 27, authentication/session creation is reachable without an explicit origin/CSRF precondition. For a browser-facing credentials endpoint, this leaves a login-CSRF gap.
🤖 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 24 - 34, The credentials endpoint signInCredentialsAction currently calls signInCredentials without validating CSRF or request origin; add a precondition that validates a CSRF token or trusted-origin before invoking signInCredentials (e.g., inspect ctx.headers['origin'] or 'referer' against a configured trusted origins list and/or validate a CSRF token from ctx.headers or ctx.body), and if the check fails return an early Response with a 403 status and no session creation; update signInCredentialsAction to perform this gate using the existing ctx (ctx.body, ctx.context) and fail fast rather than proceeding to call signInCredentials.
🤖 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/api/credentials.ts`:
- Line 7: signInCredentials currently ignores the redirectTo field from
SignInCredentialsOptions; update the function to read redirectTo from the
incoming options/payload and apply it after successful sign-in (e.g., call
ctx.redirect(redirectTo) or include redirectTo in the returned success response)
so callers using createApi can cause an actual redirect. Locate the
signInCredentials function and the SignInCredentialsOptions usage, extract
redirectTo from the payload/options, and ensure it is acted upon on the
successful sign-in path rather than being ignored.
- Around line 31-41: The current broad catch in credentials.ts collapses all
failures into an INVALID_CREDENTIALS path; change the catch to distinguish
authentication failures from internal/runtime errors: inside the try/catch
around the sign-in logic (the block that currently uses logger?.log and returns
{ success: false, headers: new Headers(secureApiHeaders) }), catch specific
auth-related errors (e.g., InvalidCredentialsError or whatever your sign-in
validator throws) and keep the existing logger?.log("INVALID_CREDENTIALS", ...)
+ auth-failure response for those, but for unexpected/internal errors
(crypto/session/cookie failures) log them as errors with full details via
logger?.error (include the error message/stack) and return an appropriate 5xx
response (or rethrow) rather than classifying them as INVALID_CREDENTIALS;
reference the logger variable and secureApiHeaders in your changes.
---
Duplicate comments:
In `@packages/core/src/actions/signIn/credentials.ts`:
- Around line 24-34: The credentials endpoint signInCredentialsAction currently
calls signInCredentials without validating CSRF or request origin; add a
precondition that validates a CSRF token or trusted-origin before invoking
signInCredentials (e.g., inspect ctx.headers['origin'] or 'referer' against a
configured trusted origins list and/or validate a CSRF token from ctx.headers or
ctx.body), and if the check fails return an early Response with a 403 status and
no session creation; update signInCredentialsAction to perform this gate using
the existing ctx (ctx.body, ctx.context) and fail fast rather than proceeding to
call signInCredentials.
🪄 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: 7c1e2652-a057-45c5-8e89-d22cb708744f
📒 Files selected for processing (10)
packages/core/src/@types/config.tspackages/core/src/@types/session.tspackages/core/src/actions/signIn/credentials.tspackages/core/src/api/createApi.tspackages/core/src/api/credentials.tspackages/core/src/api/index.tspackages/core/src/shared/errors.tspackages/core/test/actions/signIn/credentials.test.tspackages/core/test/api/signInCredentials.test.tspackages/core/test/presets.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/shared/errors.ts
- packages/core/src/api/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/@types/config.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/actions/signInCredentials/signInCredentials.ts (1)
27-33:⚠️ Potential issue | 🟠 Major
redirectTois validated but not passed tosignInCredentials.At Line 11 and Line 29, the endpoint accepts
redirectToinsearchParamsbut drops it when calling the API function.Suggested fix
async (ctx) => { const payload = ctx.body + const redirectTo = ctx.searchParams?.redirectTo const { headers, success } = await signInCredentials({ ctx: ctx.context, payload, - request: ctx.request + request: ctx.request, + redirectTo, }) return Response.json({ success }, { headers, status: success ? 200 : 401 }) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/actions/signInCredentials/signInCredentials.ts` around lines 27 - 33, The endpoint validates a redirectTo search param but never forwards it to the API call; update the handler so the validated redirectTo (from ctx.request.searchParams or the earlier parsed searchParams) is included in the call to signInCredentials by adding redirectTo: redirectTo (or the local variable name) to the object passed to signInCredentials (alongside ctx: ctx.context, payload, request: ctx.request) so signInCredentials receives the redirect target.packages/core/src/api/credentials.ts (1)
47-67:⚠️ Potential issue | 🟠 MajorUnexpected runtime failures are still being returned as auth failures.
At Line 63, non-auth exceptions are converted to
success: false, which the action layer maps to401. That still masks internal failures as invalid credentials and weakens incident diagnosis.Suggested fix
} catch (error) { if (error instanceof AuthValidationError) { logger?.log("INVALID_CREDENTIALS", { severity: "warning", structuredData: { path: "/signIn/credentials" }, }) return { success: false, headers: new Headers(secureApiHeaders), redirectURL: null } } logger?.log("CREDENTIALS_SIGN_IN_FAILED", { severity: "error", - structuredData: { path: "/signIn/credentials" }, + structuredData: { + path: "/signIn/credentials", + message: error instanceof Error ? error.message : String(error), + }, }) - return { - success: false, - headers: new Headers(secureApiHeaders), - redirectURL: null - } + throw error }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/api/credentials.ts` around lines 47 - 67, The catch block currently treats all exceptions as auth failures; only handle AuthValidationError as an invalid credential case and return { success: false, ... } for that branch, but for any other exception (non-AuthValidationError) log the full error details via logger?.log("CREDENTIALS_SIGN_IN_FAILED", ...) and rethrow the error (or throw a new wrapped error) so the caller/action layer receives an exception (allowing a 5xx response) instead of mapping it to success:false; update the catch in credentials.ts around the AuthValidationError check to preserve the existing secureApiHeaders usage for the auth-failure branch and to rethrow other errors after logging.
🧹 Nitpick comments (1)
packages/core/test/actions/signInCredentials/signInCredentials.test.ts (1)
14-121: Add an action-level regression test forredirectTo.Current tests never send
redirectToin query params, so the action-path redirect contract is unverified.Suggested test case
describe("signInCredentials action", () => { + test("forwards redirectTo from query params", async () => { + const response = await POST( + new Request("http://localhost:3000/auth/signIn/credentials?redirectTo=/dashboard", { + method: "POST", + body: JSON.stringify({ + username: "johndoe", + password: "1234567890", + }), + }) + ) + expect(response.status).toBe(200) + // Assert redirect behavior once action response exposes it or downstream behavior is defined. + }) + test("success signIn flow", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/actions/signInCredentials/signInCredentials.test.ts` around lines 14 - 121, Tests for the signInCredentials action never exercise the action-level redirect contract (redirectTo query param); add a new test that uses the existing POST handler returned by createAuth to call the sign-in endpoint with a redirectTo query param (e.g., "/auth/signIn/credentials?redirectTo=/dashboard"), provide valid credentials via the request body, and then assert the action returns the correct redirect behavior (status and Location header or appropriate response body per the action implementation). Use the same helpers already in the file (POST, createAuth, getSetCookie, jose.decodeJWT if needed) and mirror the "success signIn flow" setup so the test only verifies presence and correctness of redirect handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/actions/signInCredentials/signInCredentials.ts`:
- Around line 27-33: The endpoint validates a redirectTo search param but never
forwards it to the API call; update the handler so the validated redirectTo
(from ctx.request.searchParams or the earlier parsed searchParams) is included
in the call to signInCredentials by adding redirectTo: redirectTo (or the local
variable name) to the object passed to signInCredentials (alongside ctx:
ctx.context, payload, request: ctx.request) so signInCredentials receives the
redirect target.
In `@packages/core/src/api/credentials.ts`:
- Around line 47-67: The catch block currently treats all exceptions as auth
failures; only handle AuthValidationError as an invalid credential case and
return { success: false, ... } for that branch, but for any other exception
(non-AuthValidationError) log the full error details via
logger?.log("CREDENTIALS_SIGN_IN_FAILED", ...) and rethrow the error (or throw a
new wrapped error) so the caller/action layer receives an exception (allowing a
5xx response) instead of mapping it to success:false; update the catch in
credentials.ts around the AuthValidationError check to preserve the existing
secureApiHeaders usage for the auth-failure branch and to rethrow other errors
after logging.
---
Nitpick comments:
In `@packages/core/test/actions/signInCredentials/signInCredentials.test.ts`:
- Around line 14-121: Tests for the signInCredentials action never exercise the
action-level redirect contract (redirectTo query param); add a new test that
uses the existing POST handler returned by createAuth to call the sign-in
endpoint with a redirectTo query param (e.g.,
"/auth/signIn/credentials?redirectTo=/dashboard"), provide valid credentials via
the request body, and then assert the action returns the correct redirect
behavior (status and Location header or appropriate response body per the action
implementation). Use the same helpers already in the file (POST, createAuth,
getSetCookie, jose.decodeJWT if needed) and mirror the "success signIn flow"
setup so the test only verifies presence and correctness of redirect handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 389793cd-2055-49b6-9f6d-3718cff892f5
📒 Files selected for processing (9)
packages/core/src/@types/session.tspackages/core/src/actions/index.tspackages/core/src/actions/signInCredentials/signInCredentials.tspackages/core/src/api/credentials.tspackages/core/src/shared/logger.tspackages/core/src/shared/security.tspackages/core/test/actions/signInCredentials/signInCredentials.test.tspackages/core/test/api/signInCredentials.test.tspackages/rate-limiter/tsconfig.json
✅ Files skipped from review due to trivial changes (2)
- packages/rate-limiter/tsconfig.json
- packages/core/src/actions/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/test/api/signInCredentials.test.ts
- packages/core/src/shared/security.ts
- packages/core/src/shared/logger.ts
- packages/core/src/@types/session.ts
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 experimental credentials-based authentication, enabling username and password sign-in functionality within Aura Auth.
The new flow is exposed via:
/signIn/credentialsendpointapi.signInCredentialsfunction fromcreateAuth.apiConfiguration is provided through the
createAuth.credentialsoption, where anauthorizefunction must be defined. This function is responsible for validating the provided credentials and returning a user session object if authentication succeeds, ornullif it fails.Additionally, built-in utilities for password hashing and verification are included to support secure credential handling.
Key Changes
/signIn/credentialsendpointsignInCredentialsfunction viacreateAuth.apicredentialsconfiguration increateAuthUsage
Configuration
Using API
Using Handlers
Using Client API
Summary by CodeRabbit
New Features
signInCredentialsAPI and client method for user sign-in with credentialsTests
Chores