Conversation
… configuration support
|
🚅 Deployed to the reqcore-pr-138 environment in applirank
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional social OAuth (Google, GitHub, Microsoft): env docs and schema, provider discovery API, Better Auth provider wiring with token encryption/rate-limit schema, frontend sign-in/sign-up social buttons with loading/error handling, email senders for verification/password reset, and related tests. Changes
Sequence DiagramsequenceDiagram
participant User
participant Frontend as Frontend (Vue)
participant API as Backend API (/api/auth/providers)
participant BetterAuth as Better Auth
participant OAuth as OAuth Provider
User->>Frontend: Clicks "Continue with Provider"
Frontend->>API: (optional) fetch provider flags
Frontend->>Frontend: set socialLoading, compute callbackURL
Frontend->>BetterAuth: authClient.signIn.social(providerId)
BetterAuth->>OAuth: Redirect user to provider consent
OAuth->>User: Consent screen
User->>OAuth: Approve
OAuth->>BetterAuth: Redirect with code
BetterAuth->>BetterAuth: Exchange code → tokens
BetterAuth->>Backend: Persist session / encrypted tokens
BetterAuth->>Frontend: Redirect to callbackURL
Frontend->>User: Finalize redirect / show signed-in state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 1
🧹 Nitpick comments (2)
app/pages/auth/sign-in.vue (1)
30-36: Extract shared social-provider metadata/UI to avoid drift with sign-up page.The provider list and icon/button rendering here are duplicated in
app/pages/auth/sign-up.vue(Line 29-Line 35 and Line 162-Line 197). A shared constant/composable/component would reduce maintenance risk.♻️ Suggested direction
+// app/composables/useSocialProviders.ts +export const SOCIAL_PROVIDERS = [ + { id: "google", name: "Google" }, + { id: "github", name: "GitHub" }, + { id: "microsoft", name: "Microsoft" }, +] as const; + +export type SocialProviderId = (typeof SOCIAL_PROVIDERS)[number]["id"];-const socialProviders = computed(() => { - const providers: { id: string; name: string }[] = []; - if (config.public.authGoogleEnabled) providers.push({ id: "google", name: "Google" }); - if (config.public.authGithubEnabled) providers.push({ id: "github", name: "GitHub" }); - if (config.public.authMicrosoftEnabled) providers.push({ id: "microsoft", name: "Microsoft" }); - return providers; -}); +const socialProviders = computed(() => + SOCIAL_PROVIDERS.filter((p) => + p.id === "google" ? config.public.authGoogleEnabled : + p.id === "github" ? config.public.authGithubEnabled : + config.public.authMicrosoftEnabled + ) +);Also applies to: 217-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/auth/sign-in.vue` around lines 30 - 36, The social provider list is duplicated between sign-in and sign-up; extract the logic and UI into a shared module and replace both copies: create a small exported composable or constant (e.g., useSocialProviders or SOCIAL_PROVIDERS) that returns the providers array and a reusable presentational component (e.g., SocialAuthButtons) that renders the icon/button markup, then import and use those in app/pages/auth/sign-in.vue (replace socialProviders and its rendering) and app/pages/auth/sign-up.vue (replace the duplicated arrays and render blocks at the indicated ranges) so both pages consume the single source of truth.server/utils/env.ts (1)
111-124: Add all-or-none validation per social provider to fail fast on partial config.Right now, setting only one of
AUTH_<PROVIDER>_CLIENT_ID/SECRETsilently disables that provider downstream. Consider validating pair completeness insuperRefine(similar to OIDC) to surface misconfiguration early.🧩 Proposed validation addition
.superRefine((data, ctx) => { + const socialPairs = [ + ["AUTH_GOOGLE_CLIENT_ID", data.AUTH_GOOGLE_CLIENT_ID, "AUTH_GOOGLE_CLIENT_SECRET", data.AUTH_GOOGLE_CLIENT_SECRET], + ["AUTH_GITHUB_CLIENT_ID", data.AUTH_GITHUB_CLIENT_ID, "AUTH_GITHUB_CLIENT_SECRET", data.AUTH_GITHUB_CLIENT_SECRET], + ["AUTH_MICROSOFT_CLIENT_ID", data.AUTH_MICROSOFT_CLIENT_ID, "AUTH_MICROSOFT_CLIENT_SECRET", data.AUTH_MICROSOFT_CLIENT_SECRET], + ] as const; + + for (const [idName, idVal, secretName, secretVal] of socialPairs) { + if ((idVal && !secretVal) || (!idVal && secretVal)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [!idVal ? idName : secretName], + message: `${idName} and ${secretName} must both be set or both be unset.`, + }); + } + } + // BETTER_AUTH_URL can be derived at runtime from RAILWAY_PUBLIC_DOMAIN,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/env.ts` around lines 111 - 124, Add an all-or-none validation for each social provider inside the schema's superRefine: for each pair AUTH_GOOGLE_CLIENT_ID/AUTH_GOOGLE_CLIENT_SECRET, AUTH_GITHUB_CLIENT_ID/AUTH_GITHUB_CLIENT_SECRET, and AUTH_MICROSOFT_CLIENT_ID/AUTH_MICROSOFT_CLIENT_SECRET, detect if one is present and the other is missing and call ctx.addIssue with a custom error for the missing key(s) so partial configs fail fast; keep the existing AUTH_MICROSOFT_TENANT_ID handling as-is but ensure the Microsoft client pair is validated the same way, referencing those exact symbol names in the superRefine implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nuxt.config.ts`:
- Around line 225-239: The public auth feature flags (authGoogleEnabled,
authGithubEnabled, authMicrosoftEnabled) in nuxt.config.ts are being computed
from process.env at build time and frozen into runtimeConfig.public, causing
UI/backend mismatches versus the runtime evaluation in server/utils/auth.ts's
getAuth(); change those public flags to read from NUXT_PUBLIC_AUTH_* environment
variables (NUXT_PUBLIC_AUTH_GOOGLE_CLIENT_ID / SECRET etc.) so they can be
overridden at runtime, or alternatively move the flag computation out of
nuxt.config.ts into a runtime-evaluated utility used by both the UI and server
(keeping getAuth() as the single source of truth) to ensure consistent runtime
behavior.
---
Nitpick comments:
In `@app/pages/auth/sign-in.vue`:
- Around line 30-36: The social provider list is duplicated between sign-in and
sign-up; extract the logic and UI into a shared module and replace both copies:
create a small exported composable or constant (e.g., useSocialProviders or
SOCIAL_PROVIDERS) that returns the providers array and a reusable presentational
component (e.g., SocialAuthButtons) that renders the icon/button markup, then
import and use those in app/pages/auth/sign-in.vue (replace socialProviders and
its rendering) and app/pages/auth/sign-up.vue (replace the duplicated arrays and
render blocks at the indicated ranges) so both pages consume the single source
of truth.
In `@server/utils/env.ts`:
- Around line 111-124: Add an all-or-none validation for each social provider
inside the schema's superRefine: for each pair
AUTH_GOOGLE_CLIENT_ID/AUTH_GOOGLE_CLIENT_SECRET,
AUTH_GITHUB_CLIENT_ID/AUTH_GITHUB_CLIENT_SECRET, and
AUTH_MICROSOFT_CLIENT_ID/AUTH_MICROSOFT_CLIENT_SECRET, detect if one is present
and the other is missing and call ctx.addIssue with a custom error for the
missing key(s) so partial configs fail fast; keep the existing
AUTH_MICROSOFT_TENANT_ID handling as-is but ensure the Microsoft client pair is
validated the same way, referencing those exact symbol names in the superRefine
implementation.
🪄 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: 6fb1dc4d-67f4-4088-a27d-b9a03cf17041
📒 Files selected for processing (6)
.env.exampleapp/pages/auth/sign-in.vueapp/pages/auth/sign-up.vuenuxt.config.tsserver/utils/auth.tsserver/utils/env.ts
Comment on lines
+225
to
+239
| /** Whether Google social sign-in is enabled */ | ||
| authGoogleEnabled: !!( | ||
| process.env.AUTH_GOOGLE_CLIENT_ID && | ||
| process.env.AUTH_GOOGLE_CLIENT_SECRET | ||
| ), | ||
| /** Whether GitHub social sign-in is enabled */ | ||
| authGithubEnabled: !!( | ||
| process.env.AUTH_GITHUB_CLIENT_ID && | ||
| process.env.AUTH_GITHUB_CLIENT_SECRET | ||
| ), | ||
| /** Whether Microsoft social sign-in is enabled */ | ||
| authMicrosoftEnabled: !!( | ||
| process.env.AUTH_MICROSOFT_CLIENT_ID && | ||
| process.env.AUTH_MICROSOFT_CLIENT_SECRET | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Nuxt 4, if runtimeConfig.public values are computed from process.env.AUTH_* inside nuxt.config.ts, are those values fixed at config/build evaluation unless overridden by NUXT_PUBLIC_* runtime env vars?
💡 Result:
Yes, in Nuxt 4, if runtimeConfig.public values are computed from process.env.AUTH_* (e.g., process.env.AUTH_SECRET) inside nuxt.config.ts, those values are fixed at build/config evaluation time. They can only be overridden at runtime by the corresponding NUXT_PUBLIC_* environment variables (e.g., NUXT_PUBLIC_AUTH_SECRET for public.runtimeConfig.authSecret). The official Nuxt documentation explicitly states: "Setting the default of runtimeConfig values to differently named environment variables (for example setting myVar to process.env.OTHER_VARIABLE) will only work during build-time and will break on runtime." Runtime overrides only work with matching NUXT_ prefixed variables that follow the runtimeConfig structure.
Citations:
- 1: https://nuxt.com/docs/4.x/guide/going-further/runtime-config
- 2: https://nuxt.com/docs/guide/going-further/runtime-config
- 3: How to override environment variables after build / where to define them nuxt/nuxt#20860
- 4: nuxt.config.ts runtimeConfig is giving build time value nuxt/nuxt#22594
🏁 Script executed:
cat -n nuxt.config.ts | sed -n '220,245p'Repository: reqcore-inc/reqcore
Length of output: 1072
🏁 Script executed:
find . -name "auth.ts" -type f | head -20Repository: reqcore-inc/reqcore
Length of output: 143
🏁 Script executed:
cat -n server/utils/auth.tsRepository: reqcore-inc/reqcore
Length of output: 13558
Public social flags are fixed at build time while server-side providers are evaluated at runtime, causing UI/backend mismatch in runtime-only deployments.
Lines 225-239 compute authGoogleEnabled, authGithubEnabled, and authMicrosoftEnabled from process.env.AUTH_* inside nuxt.config.ts. In Nuxt 4, these build-time values are frozen into runtimeConfig.public and cannot be changed at runtime unless overridden by corresponding NUXT_PUBLIC_* environment variables.
However, server/utils/auth.ts (lines 167–193) evaluates the same AUTH_* variables at runtime when getAuth() is first called. This creates a critical mismatch:
- Scenario A: If
AUTH_*vars are not set at build time but injected at runtime (common in Railway and similar platforms), the UI flags remainfalsewhile the server can enable providers — users won't see the buttons despite functional backends. - Scenario B: If
AUTH_*vars are set at build time but removed at runtime, the UI shows buttons for disabled providers — users encounter auth failures.
Fix: Use NUXT_PUBLIC_AUTH_* environment variables for the runtimeConfig.public flags, or move the flag computation logic to a runtime-evaluated utility that reads directly from env at request time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nuxt.config.ts` around lines 225 - 239, The public auth feature flags
(authGoogleEnabled, authGithubEnabled, authMicrosoftEnabled) in nuxt.config.ts
are being computed from process.env at build time and frozen into
runtimeConfig.public, causing UI/backend mismatches versus the runtime
evaluation in server/utils/auth.ts's getAuth(); change those public flags to
read from NUXT_PUBLIC_AUTH_* environment variables
(NUXT_PUBLIC_AUTH_GOOGLE_CLIENT_ID / SECRET etc.) so they can be overridden at
runtime, or alternatively move the flag computation out of nuxt.config.ts into a
runtime-evaluated utility used by both the UI and server (keeping getAuth() as
the single source of truth) to ensure consistent runtime behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/plugins/auth-flags.ts (1)
16-40: Consider a tiny helper to reduce repeated env checks.This is optional, but it lowers copy/paste risk when adding more providers/flags later.
♻️ Suggested cleanup
export default defineNitroPlugin(() => { const cfg = useRuntimeConfig(); + const hasEnv = (...keys: string[]) => keys.every((k) => Boolean(process.env[k])); - cfg.public.authGoogleEnabled = !!( - process.env.AUTH_GOOGLE_CLIENT_ID && - process.env.AUTH_GOOGLE_CLIENT_SECRET - ); + cfg.public.authGoogleEnabled = hasEnv( + "AUTH_GOOGLE_CLIENT_ID", + "AUTH_GOOGLE_CLIENT_SECRET", + ); - cfg.public.authGithubEnabled = !!( - process.env.AUTH_GITHUB_CLIENT_ID && - process.env.AUTH_GITHUB_CLIENT_SECRET - ); + cfg.public.authGithubEnabled = hasEnv( + "AUTH_GITHUB_CLIENT_ID", + "AUTH_GITHUB_CLIENT_SECRET", + ); - cfg.public.authMicrosoftEnabled = !!( - process.env.AUTH_MICROSOFT_CLIENT_ID && - process.env.AUTH_MICROSOFT_CLIENT_SECRET - ); + cfg.public.authMicrosoftEnabled = hasEnv( + "AUTH_MICROSOFT_CLIENT_ID", + "AUTH_MICROSOFT_CLIENT_SECRET", + ); - cfg.public.oidcEnabled = !!( - process.env.OIDC_CLIENT_ID && - process.env.OIDC_CLIENT_SECRET && - process.env.OIDC_DISCOVERY_URL - ); + cfg.public.oidcEnabled = hasEnv( + "OIDC_CLIENT_ID", + "OIDC_CLIENT_SECRET", + "OIDC_DISCOVERY_URL", + ); - cfg.public.feedbackEnabled = !!( - process.env.GITHUB_FEEDBACK_TOKEN && - process.env.GITHUB_FEEDBACK_REPO - ); + cfg.public.feedbackEnabled = hasEnv( + "GITHUB_FEEDBACK_TOKEN", + "GITHUB_FEEDBACK_REPO", + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/plugins/auth-flags.ts` around lines 16 - 40, Extract the repeated environment checks into a small helper like hasEnvVars(varNames: string[]) and replace each boolean assignment (cfg.public.authGoogleEnabled, cfg.public.authGithubEnabled, cfg.public.authMicrosoftEnabled, cfg.public.oidcEnabled, cfg.public.feedbackEnabled) to call that helper with the relevant env variable names; the helper should return !!varNames.every(name => !!process.env[name]) so future flags only need to list required env keys and you avoid copy/paste and inconsistent checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/plugins/auth-flags.ts`:
- Around line 16-40: Extract the repeated environment checks into a small helper
like hasEnvVars(varNames: string[]) and replace each boolean assignment
(cfg.public.authGoogleEnabled, cfg.public.authGithubEnabled,
cfg.public.authMicrosoftEnabled, cfg.public.oidcEnabled,
cfg.public.feedbackEnabled) to call that helper with the relevant env variable
names; the helper should return !!varNames.every(name => !!process.env[name]) so
future flags only need to list required env keys and you avoid copy/paste and
inconsistent checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d91acc23-9523-46d3-b818-0ba2d9c048dd
📒 Files selected for processing (1)
server/plugins/auth-flags.ts
… environment variables
…iders and remove entrypoint script
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/api/auth/providers.get.ts (1)
11-27: Reduce drift risk by centralizing provider-enable checks.This logic is duplicated from
server/utils/auth.ts. Consider extracting a shared helper (e.g.,server/utils/authProviderFlags.ts) and reusing it here and in auth initialization so UI flags and actual provider registration can’t diverge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/auth/providers.get.ts` around lines 11 - 27, The provider-enable boolean checks (google, github, microsoft, oidc) are duplicated here and in server/utils/auth.ts; extract them into a single helper (e.g., export a function or object from server/utils/authProviderFlags.ts) that performs the environment checks for AUTH_GOOGLE_CLIENT_ID/SECRET, AUTH_GITHUB_CLIENT_ID/SECRET, AUTH_MICROSOFT_CLIENT_ID/SECRET, and OIDC_CLIENT_ID/SECRET/DISCOVERY_URL, then import and use that helper in providers.get.ts (replacing the inline boolean expressions) and in server/utils/auth.ts so both the UI flags and provider registration derive from the same source.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/api/auth/providers.get.ts`:
- Around line 11-27: The provider-enable boolean checks (google, github,
microsoft, oidc) are duplicated here and in server/utils/auth.ts; extract them
into a single helper (e.g., export a function or object from
server/utils/authProviderFlags.ts) that performs the environment checks for
AUTH_GOOGLE_CLIENT_ID/SECRET, AUTH_GITHUB_CLIENT_ID/SECRET,
AUTH_MICROSOFT_CLIENT_ID/SECRET, and OIDC_CLIENT_ID/SECRET/DISCOVERY_URL, then
import and use that helper in providers.get.ts (replacing the inline boolean
expressions) and in server/utils/auth.ts so both the UI flags and provider
registration derive from the same source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31ebd5d8-84f3-4a0e-bd6f-e583de734e39
📒 Files selected for processing (3)
app/pages/auth/sign-in.vueapp/pages/auth/sign-up.vueserver/api/auth/providers.get.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/pages/auth/sign-in.vue
- app/pages/auth/sign-up.vue
…email verification, and session management
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/database/schema/auth.ts`:
- Around line 116-123: Create a new migration that creates the rate_limit table
to match the schema defined in server/database/schema/auth.ts: columns id (text
primary key), key (text not null), count (integer not null), last_request
(bigint not null) and an index on key (rate_limit_key_idx); include a down
migration that drops the rate_limit table so rollbacks work; name the migration
consistently with the project’s migration naming convention and export it so
your migration runner picks it up before deploying since server/utils/auth.ts
sets rateLimit.storage = "database".
In `@server/utils/auth.ts`:
- Around line 171-181: The auth config currently wires sendResetPassword under
emailAndPassword but never registers the new sendVerificationEmail or enables
email verification; add an emailVerification block that calls
sendVerificationEmail (importing it from server/utils/email.ts) and add
requireEmailVerification: true under the existing emailAndPassword config so
Better Auth will trigger verification; ensure you reference the existing
sendResetPassword handler and the emailAndPassword object when adding the
emailVerification handler and importing sendVerificationEmail.
- Around line 183-215: The custom databaseHooks on account (create/update) that
call encrypt(...) with env.BETTER_AUTH_SECRET should be removed and replaced by
enabling Better Auth's built-in transparent encryption: set the account option
encryptOAuthTokens: true (remove the manual databaseHooks for
account.create.before and account.update.before and any direct encrypt(...)
usage), so Better Auth will handle encryption on write and automatic decryption
on read; ensure no leftover references to env.BETTER_AUTH_SECRET or encrypt(...)
remain for account tokens.
In `@server/utils/email.ts`:
- Around line 43-57: The Resend send call can throw (network/SDK exceptions)
which aren't caught—wrap the call inside sendVerificationEmail in a try/catch so
thrown exceptions are logged as well as the returned { error } handled; call
resend.emails.send inside try, if it returns an { error } call
logError('email.verification_send_failed', { provider: 'resend', error_message:
error.message }), and in catch logError('email.verification_send_failed', {
provider: 'resend', error_message: err.message, stack: err.stack }); repeat the
same try/catch + returned-error handling pattern for the other similar helper
(the block around lines 84-98).
- Around line 33-38: The fallback logging is leaking bearer-token URLs and
recipient emails (see the console.info guarded by the resend flag referencing
data.user.email and data.url and the similar block at lines 74-79); change these
logs to avoid printing the full email or URL — either remove the URL and email
entirely or redact them (e.g., log "verification email sent" with a
non-sensitive identifier like a hashed email or just the domain), and ensure any
remaining log text does not include data.url or data.user.email; update both the
first console.info block and the similar block later so no bearer token or full
address is emitted.
In `@tests/unit/auth-security-hardening.test.ts`:
- Around line 14-56: Replace the local magic constants in the tests with the
real exported auth config so the assertions validate actual runtime values:
remove the local MIN_LENGTH, MAX_LENGTH, SESSION_EXPIRES_IN, and
SESSION_UPDATE_AGE declarations and instead import the matching exports from the
auth config module (e.g., the constants exported by server/utils/auth.ts or your
config builder), then update the specs to assert against those imported symbols
(MIN_LENGTH, MAX_LENGTH, SESSION_EXPIRES_IN, SESSION_UPDATE_AGE) so changes in
the implementation will fail the tests.
- Around line 104-131: The test duplicates and diverges from production logic;
replace the local rateLimitConfig and the `'CI' in process.env` recompute by
importing the actual exported rate-limit configuration or factory from
server/utils/auth.ts (e.g., rateLimitConfig or getRateLimitConfig) and assert
against that object, and when simulating CI checks use the same boolean
expression the production code uses (!process.env.CI &&
!process.env.GITHUB_ACTIONS) to determine enabled/disabled so the test fails if
the real config drifts.
- Around line 136-157: The test currently asserts on hand-written mocks
(oidcConfig and defaultOAuthState) instead of exercising real code; update the
test to import and assert against the actual exported genericOAuth config from
auth.ts (verify pkce and requireIssuerValidation there) and call the
repository's real OAuth state builder/helper (e.g., buildOAuthState or
createOAuthState) to generate state and assert it includes codeVerifier; if no
shared state builder exists yet, remove the defaultOAuthState test until one is
available.
🪄 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: 7334bf57-c1f9-45e5-9ec1-ac9f547c8624
📒 Files selected for processing (5)
server/database/schema/auth.tsserver/utils/auth.tsserver/utils/email.tsserver/utils/rateLimit.tstests/unit/auth-security-hardening.test.ts
✅ Files skipped from review due to trivial changes (1)
- server/utils/rateLimit.ts
Comment on lines
171
to
+181
| emailAndPassword: { | ||
| enabled: true, | ||
| // Server-side password policy — prevents bypass via direct API calls. | ||
| // Client-side validation (sign-up.vue) is UX only; this is the enforcement. | ||
| minPasswordLength: 8, | ||
| maxPasswordLength: 128, | ||
| // Password reset via email. | ||
| async sendResetPassword({ user, url, token }, request) { | ||
| void sendPasswordResetEmail({ user, url, token }); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect a real auth config path to wire the verification sender.
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' '\bsendVerificationEmail\b|\bemailVerification\b|\brequireEmailVerification\b'Repository: reqcore-inc/reqcore
Length of output: 231
🏁 Script executed:
cat -n server/utils/auth.tsRepository: reqcore-inc/reqcore
Length of output: 16934
🏁 Script executed:
find . -type f \( -name '*.ts' -o -name '*.js' -o -name '*.tsx' -o -name '*.jsx' \) ! -path '*/node_modules/*' ! -path '*/.git/*' -exec grep -l "emailVerification\|requireEmailVerification\|sendVerificationEmail" {} \;Repository: reqcore-inc/reqcore
Length of output: 86
🏁 Script executed:
sed -n '20,35p' server/utils/email.tsRepository: reqcore-inc/reqcore
Length of output: 595
The new verification-email flow still looks unwired.
This file hooks up sendResetPassword, but the new sendVerificationEmail function from server/utils/email.ts is never imported or referenced here. There is no emailVerification block and no requireEmailVerification setting in the auth config. Better Auth requires both to activate email verification—the function needs to be wired through a dedicated emailVerification config block while requireEmailVerification belongs under emailAndPassword. (better-auth.com)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/auth.ts` around lines 171 - 181, The auth config currently wires
sendResetPassword under emailAndPassword but never registers the new
sendVerificationEmail or enables email verification; add an emailVerification
block that calls sendVerificationEmail (importing it from server/utils/email.ts)
and add requireEmailVerification: true under the existing emailAndPassword
config so Better Auth will trigger verification; ensure you reference the
existing sendResetPassword handler and the emailAndPassword object when adding
the emailVerification handler and importing sendVerificationEmail.
Comment on lines
+14
to
+56
| describe('Server-side password policy', () => { | ||
| const MIN_LENGTH = 8 | ||
| const MAX_LENGTH = 128 | ||
|
|
||
| it('rejects passwords shorter than minimum', () => { | ||
| const password = 'short' | ||
| expect(password.length).toBeLessThan(MIN_LENGTH) | ||
| }) | ||
|
|
||
| it('accepts passwords at minimum length', () => { | ||
| const password = 'a'.repeat(MIN_LENGTH) | ||
| expect(password.length).toBeGreaterThanOrEqual(MIN_LENGTH) | ||
| }) | ||
|
|
||
| it('rejects passwords exceeding maximum length', () => { | ||
| const password = 'a'.repeat(MAX_LENGTH + 1) | ||
| expect(password.length).toBeGreaterThan(MAX_LENGTH) | ||
| }) | ||
|
|
||
| it('minimum length is at least 8 characters', () => { | ||
| expect(MIN_LENGTH).toBeGreaterThanOrEqual(8) | ||
| }) | ||
| }) | ||
|
|
||
| // ── Issue #6: Explicit session expiry ─────────────────────────────── | ||
|
|
||
| describe('Session expiry configuration', () => { | ||
| const SESSION_EXPIRES_IN = 60 * 60 * 24 // 24 hours (seconds) | ||
| const SESSION_UPDATE_AGE = 60 * 60 // 1 hour (seconds) | ||
|
|
||
| it('session expiry is explicitly set (not default 7 days)', () => { | ||
| const DEFAULT_BETTER_AUTH_EXPIRY = 60 * 60 * 24 * 7 // 7 days | ||
| expect(SESSION_EXPIRES_IN).toBeLessThan(DEFAULT_BETTER_AUTH_EXPIRY) | ||
| }) | ||
|
|
||
| it('session expiry is at most 24 hours', () => { | ||
| expect(SESSION_EXPIRES_IN).toBeLessThanOrEqual(60 * 60 * 24) | ||
| }) | ||
|
|
||
| it('session update age is shorter than expiry', () => { | ||
| expect(SESSION_UPDATE_AGE).toBeLessThan(SESSION_EXPIRES_IN) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
These assertions don't protect the real password/session config.
Everything here is derived from local constants, so server/utils/auth.ts can change and this block will still stay green. Please extract the auth constants into a shared module or a pure config builder and assert against the imported values instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/auth-security-hardening.test.ts` around lines 14 - 56, Replace the
local magic constants in the tests with the real exported auth config so the
assertions validate actual runtime values: remove the local MIN_LENGTH,
MAX_LENGTH, SESSION_EXPIRES_IN, and SESSION_UPDATE_AGE declarations and instead
import the matching exports from the auth config module (e.g., the constants
exported by server/utils/auth.ts or your config builder), then update the specs
to assert against those imported symbols (MIN_LENGTH, MAX_LENGTH,
SESSION_EXPIRES_IN, SESSION_UPDATE_AGE) so changes in the implementation will
fail the tests.
…social sign-in options and enhancing OAuth token encryption
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.
Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit
New Features
Documentation