Migrate auth from hand-rolled Spring JWT to better-auth (Next.js IdP)…#24
Open
OffCrazyFreak wants to merge 1 commit into
Open
Migrate auth from hand-rolled Spring JWT to better-auth (Next.js IdP)…#24OffCrazyFreak wants to merge 1 commit into
OffCrazyFreak wants to merge 1 commit into
Conversation
… + Spring resource server
WHY
The backend previously hand-rolled authentication (AuthService, JwtService, RefreshToken,
BCrypt, custom password rules) and the frontend stored an access token in localStorage and
refreshed via an HttpOnly cookie. This replaces all of it with better-auth as the identity
provider, supporting email+password and Google sign-in, structured so Resend (password
reset / email verification) and Facebook can be added later.
KEY ARCHITECTURAL FACT
better-auth is TypeScript/Node — it cannot run inside the Java/Spring backend. So better-auth
lives in the Next.js app and is the identity provider; the Spring backend becomes an OAuth2
resource server that validates better-auth JWTs via JWKS. We deleted auth *issuance* from the
backend, not identity *verification* — every per-user endpoint still needs to know who is calling.
DATA MODEL — split identity + profile (same id)
- better-auth owns the auth tables in Postgres (user, session, account, verification, jwks),
managed via the Drizzle adapter + drizzle-kit migrations.
- Spring keeps a slim app_user PROFILE table (username, subscription, notifications, AI quota,
pinned stores/places relations) keyed by the SAME id as the better-auth user.
- better-auth is configured to generate UUID ids (advanced.database.generateId: "uuid") so the
8 business entities that FK to the user (uuid user_id / owner_id) keep working unchanged.
- The Spring backend lazily provisions the app_user profile from JWT claims (sub, email) on the
first authenticated request (UserProvisioningFilter), so no signup-sync step is needed.
FRONTEND (Next.js 16 / Turbopack, pnpm 11)
- Added deps: drizzle-orm 0.45.2, pg 8.21.0, drizzle-kit 0.31.10, @types/pg, dotenv. better-auth 1.6.14 + @daveyplate/better-auth-ui 3.4.0 were already present.
- New: src/lib/auth.ts (better-auth server: drizzleAdapter(pg), emailAndPassword minLength 8,
socialProviders.google, jwt plugin ES256 with definePayload {email,name}, nextCookies,
account.accountLinking, user.deleteUser, trustedOrigins, generateId uuid).
- New: src/lib/auth-client.ts (createAuthClient(react) + jwtClient), src/app/api/auth/[...all]/route.ts
(toNextJsHandler), src/db/index.ts (drizzle node-postgres pool), src/db/auth-schema.ts (generated by
`npx @better-auth/cli generate`), drizzle.config.ts.
- next.config.ts: rewrite that proxies /api/* to Spring now excludes /api/auth/* (better-auth) and
/api/cijene/* (Next route handlers).
- src/lib/api/api-base.ts: attaches a better-auth JWT (from authClient.token()) as the Bearer token
instead of the old localStorage token; on 401 refetches once; added a negative-cache backoff so a
logged-out client stops hammering /api/auth/token; exports clearAuthToken/resetAuthToken.
- src/context/user-context.tsx: same IUserContext shape (so all ~22 useUser() consumers are untouched);
identity (id/name/email/image) comes from better-auth useSession(), business profile still from
GET /api/users/me; logout = authClient.signOut(); handleUserLogin resets the token backoff.
- Auth UI: login by EMAIL (was usernameOrEmail), signup adds a required `name` field, auth-modal Google
button is real (signIn.social), account-details delete flow is 2-step (backend anonymize +
authClient.deleteUser), "logout all" -> authClient.revokeOtherSessions(), removed stayLoggedInDays.
- Schemas (auth-user.ts): login {email,password}; register {name,email,password,confirmPassword};
userRequest drops stayLoggedInDays; userDto drops stayLoggedInDays/lastLoginAt, adds optional
name/image (merged client-side from the session).
- Gated per-user React Query hooks on auth (watchlist in notifications-context + product-action-buttons;
shopping lists in add-to-shopping-list-form via { enabled }) to stop 401 spam on public pages.
- Deleted: src/lib/api/auth/ (old axios auth service), the localStorage access-token helpers,
AuthResponse type, and dead user endpoints (checkEmailExists/checkUsernameExists/getAllUsers — the
backend never had those routes).
BACKEND (Spring Boot 3.1.0 / Java 21)
- pom.xml: + spring-boot-starter-oauth2-resource-server; removed jjwt (api/impl/jackson),
google-api-client, spring-security-oauth2-client.
- application.properties: spring.security.oauth2.resourceserver.jwt.jwk-set-uri=${BETTER_AUTH_JWKS_URI:
http://localhost:3000/api/auth/jwks} and jws-algorithms=ES256; removed all jwt.* props.
- SecurityConfig: now an OAuth2 resource server (oauth2ResourceServer.jwt()); removed the custom
JwtAuthenticationFilter and the BCryptPasswordEncoder bean; principal name = JWT `sub` (a UUID), so
SecurityUtils.getCurrentUserId() is unchanged. Added UserProvisioningFilter after BearerTokenAuthenticationFilter.
- New UserProvisioningFilter: reads sub+email from the validated JWT and calls userService.ensureActiveProfile.
- User entity: id is now ASSIGNED (no @GeneratedValue), = better-auth UUID; removed passwordHash, googleId,
stayLoggedInDays, lastLoginAt; email is now NULLABLE (so deletion can null it).
- UserService: ensureActiveProfile (create-if-missing, or REVIVE a soft-deleted profile + restore email —
the partial-failure safety net); deleteAccount (anonymize: null username+email, set deletedAt, KEEP all
business fields/data); removed stayLoggedInDays from updateProfile; UserDto trimmed.
- UserController: /api/users/me GET/PATCH/DELETE; DELETE now anonymizes.
- UserRepository trimmed to existsByUsername.
- Deleted: disscount.auth.* (AuthController, AuthService, JwtService, RefreshToken+repo, auth DTOs),
JwtAuthenticationFilter, PasswordValidator, EmailValidator; PublicEndpointConfig trimmed to Swagger only.
- Updated ShoppingListControllerTest for the new User shape (assigned id, no passwordHash).
DATABASE / ENV
- better-auth tables created via drizzle-kit (UUID PKs, gen_random_uuid()).
- app_user rebuilt by Hibernate (ddl-auto=update); ALTER app_user.email DROP NOT NULL for anonymization.
- frontend/.env.local: added DATABASE_URL (same Postgres as the backend). backend/.env: removed JWT_SECRET,
added BETTER_AUTH_JWKS_URI.
- pnpm 11: config moved to frontend/pnpm-workspace.yaml (top-level overrides:, allowBuilds:); .npmrc has
minimum-release-age=0.
ACCOUNT DELETION SEMANTICS (business model)
Deleting an account keeps the business data as anonymized orphans (for dashboard/analytics) and frees the
email for re-use: backend nulls PII on app_user (keeps the row + its watchlists/lists), and the client
deletes the better-auth identity (user/session/account) which releases the unique email. Re-signup with the
same email creates a fresh better-auth user (new UUID) + new profile.
PROBLEMS ENCOUNTERED & SOLUTIONS
- drizzle-kit migrate hung in the foreground (2-min tool timeout killed it mid-transaction, leaving 3 of 5
tables). Fixed by running it in the background; verified all 5 tables + the migration journal.
- Turbopack build error "Export DEFAULT_MIGRATION_LOCK_TABLE doesn't exist": better-auth's core always
statically imports its bundled @better-auth/kysely-adapter, which imports DEFAULT_MIGRATION_TABLE/_LOCK_TABLE
from kysely's main entry — kysely 0.29.x moved those to kysely/migration. We use the Drizzle adapter, so
kysely only needs to satisfy the static import. Fixed by pinning kysely to 0.28.17 via pnpm override.
- pnpm 11 upgrade: stopped reading the package.json "pnpm" field; overrides must be TOP-LEVEL in
pnpm-workspace.yaml (not nested under pnpm:). Also added allowBuilds (esbuild/sharp/etc.) and .npmrc
minimum-release-age=0 (new supply-chain guard). NOTE: `pnpm install` won't re-apply an override change
unless BOTH node_modules and pnpm-lock.yaml are deleted.
- 404 on GET /api/users/me after login: accounts were soft-deleted during testing; the better-auth identity
persisted while the Spring profile stayed soft-deleted, so provisioning skipped it (existsById) and
getCurrentUser filtered it out -> permanent 404. Fixed by making ensureActiveProfile revive soft-deleted profiles.
- account_not_linked persisted despite trustedProviders: source has TWO checks; trustedProviders only
satisfies clause 1 ((!isTrustedProvider && !provider.emailVerified)). Clause 2
(requireLocalEmailVerified && !existingUser.emailVerified) defaults requireLocalEmailVerified to TRUE and
blocked linking to unverified credential accounts. Fixed with account.accountLinking.requireLocalEmailVerified=false.
- 401 spam when logged out (/api/watchlist/me + /api/auth/token): the global notifications provider fetched
the watchlist unconditionally and the token interceptor hit /api/auth/token per request. Fixed by gating
per-user queries on auth + the token negative-cache backoff.
KNOWN ISSUES (left as-is)
- invalid_code on the very FIRST Google consent for an account (better-auth#3401): the account is created and
the retry/subsequent logins work; benign, not fixed.
- Radix "DialogContent missing Description/aria-describedby" a11y warnings (pre-existing).
- Dev hydration mismatch on <body> caused by react-scan / browser extensions (NEXT_PUBLIC_ENABLE_REACT_SCAN).
TODO (follow-ups)
- Resend: implement emailAndPassword.sendResetPassword (forgot password) + sendVerificationEmail; then enable
requireEmailVerification, and REMOVE account.accountLinking.requireLocalEmailVerified=false (and reconsider
trustedProviders) to close the pre-registration account-takeover tradeoff.
- Remove the kysely 0.28.17 pnpm override once better-auth ships the kysely 0.29 import fix (track better-auth#9810).
- Add Facebook (socialProviders.facebook + a button mirroring Google).
- Regenerate frontend/disscount-api-docs.json from the backend (still documents the old /api/auth/* endpoints + old UserDto).
- Docker: set BETTER_AUTH_JWKS_URI to the internal frontend URL (e.g. http://frontend:3000/api/auth/jwks).
- Optional: scrub digital_card PII on deletion; add DialogDescription to dialogs; restore a sane minimum-release-age.
✅ Deploy Preview for disscount ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
WalkthroughThis PR completes a comprehensive architectural migration from custom JWT authentication to better-auth on both backend and frontend. The backend transitions from Spring Security JWT configuration with custom services to OAuth2 resource server validation via JWKS. The frontend migrates from custom auth API calls and token persistence to better-auth client with session-driven state and server-side Drizzle ORM persistence with PostgreSQL. ChangesAuthentication System Migration to Better-Auth
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/context/user-context.tsx (1)
43-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against stale
refreshUser()responses after logout/session changes.A pending
refreshUser()can complete after logout and callsetUser(userData), restoring stale authenticated state.Proposed fix
import React, { createContext, useContext, useEffect, useState, useCallback, + useRef, } from "react"; @@ const [user, setUser] = useState<UserDto | null>(null); const [isLoading, setIsLoading] = useState<boolean>(true); + const refreshVersionRef = useRef(0); @@ const refreshUser = useCallback(async () => { + const version = ++refreshVersionRef.current; try { setIsLoading(true); const userData = await userService.getCurrentUser(); @@ - setUser(userData); + if (version === refreshVersionRef.current) { + setUser(userData); + } return userData; @@ const handleLogout = useCallback(async () => { try { await authClient.signOut(); } finally { + refreshVersionRef.current += 1; // invalidate in-flight refreshes clearAuthToken(); setUser(null); queryClient.clear(); } }, [queryClient]); @@ } else { + refreshVersionRef.current += 1; // invalidate in-flight refreshes setUser(null); setIsLoading(false); }Also applies to: 73-83, 103-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/context/user-context.tsx` around lines 43 - 71, refreshUser can race with logout/session changes and setUser with stale data; introduce a request counter ref (e.g., const latestRequestRef = useRef(0)) and increment it at the start of refreshUser, capture the current id in a local variable before any awaits, then before calling setUser verify the captured id === latestRequestRef.current and bail out if not; ensure the same guard is applied wherever setUser is called in this file (the other user-fetching helpers at the ranges noted) and increment latestRequestRef when performing logout or any session reset so in-flight responses are ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 74: Update the sentence that currently reads "Never edit the package.json
or packege-lock.json files directly..." to fix the typo and reference pnpm's
lockfile: change "packege-lock.json" to "pnpm-lock.yaml" and ensure the line
instructs using pnpm commands (e.g., "pnpm add" / "pnpm remove") rather than
editing package.json or pnpm-lock.yaml directly; look for the exact phrase
"Never edit the package.json or packege-lock.json files directly" in AGENTS.md
to locate and update it.
In `@backend/src/main/java/disscount/user/service/UserService.java`:
- Around line 30-67: The current ensureActiveProfile uses a read-then-create
(userRepository.findById then userRepository.save) which races on concurrent
first requests; change the logic to perform an atomic upsert or to attempt
create and recover on duplicate-key: try to save a new User when not found (or
call a repository upsert method if available), and if save throws a
unique-constraint/DataIntegrityViolationException, catch it, re-query with
userRepository.findById and apply the revive/email restore logic on the fetched
entity; keep the existing revive/email-setting behavior (clearing deletedAt,
setting email) and persist only when changed. Ensure you update
ensureActiveProfile to use this create-then-handle-duplicate pattern (or a DB
upsert) instead of the current check-then-insert to avoid the race.
- Around line 44-47: The current logic in UserService.java only updates the DB
email when user.getEmail() is null; change it to synchronize whenever the IdP
JWT claim differs from stored value: replace the condition that checks
"user.getEmail() == null && email != null" with a comparison that sets
user.setEmail(email) and changed = true when email != null and
!email.equals(user.getEmail()) (handle nulls safely so we don't NPE), ensuring
the stored email is updated whenever the JWT claim changes.
In `@frontend/.env.local.example`:
- Line 9: The BETTER_AUTH_URL env example contains an inline comment that breaks
dotenv parsing; update the BETTER_AUTH_URL line in .env.local.example to contain
only the placeholder value (e.g., BETTER_AUTH_URL=your_app_url_here) and move
the explanatory example into its own comment line above or below (e.g., # for
example: http://localhost:3000) so the parser sees a clean key=value and the
guidance remains visible.
In `@frontend/drizzle/0000_married_the_fury.sql`:
- Around line 1-15: Add a composite UNIQUE constraint to the "account" table to
ensure the pair ("provider_id","account_id") is unique: modify the schema in the
migration that creates the "account" table (the CREATE TABLE for "account") to
include a UNIQUE constraint named account_provider_account_unique on columns
provider_id and account_id, or add an ALTER TABLE statement to add the
constraint; reference the table "account" and the columns "provider_id" and
"account_id" and use the constraint name account_provider_account_unique when
implementing the change.
In `@frontend/src/components/custom/header/forms/account-details-modal.tsx`:
- Around line 154-170: The handleLogoutAll function can leave isLoggingOutAll
true if authClient.revokeOtherSessions() throws; wrap the async call in a
try/finally so setIsLoggingOutAll(false) always runs: set isLoggingOutAll(true)
before the try, call await authClient.revokeOtherSessions() inside try, handle
thrown errors in a catch to call toast.error (or preserve existing error
handling using the returned { error }) and in finally call
setIsLoggingOutAll(false); keep the existing success toast, queryClient.clear
and onOpenChange(false) only when the revoke succeeds. Ensure you reference
handleLogoutAll, authClient.revokeOtherSessions, setIsLoggingOutAll, toast,
queryClient.clear and onOpenChange when making the change.
- Around line 122-154: Convert the non-inline arrow handlers to named function
declarations: change the const arrow definitions for handleDeleteUser and
handleLogoutAll into async function handleDeleteUser() { ... } and async
function handleLogoutAll() { ... }, preserving the async keyword, any type
annotations (e.g. : Promise<void> if present), internal logic (confirm,
deleteUserMutation.mutateAsync, authClient.deleteUser, toast calls, logout,
setIsDeleting) and references to closure vars (onOpenChange, authClient,
deleteUserMutation, logout, toast). Keep their exported/used names unchanged so
any JSX/props that reference handleDeleteUser or handleLogoutAll continue to
work.
In `@frontend/src/components/custom/header/forms/signup-form.tsx`:
- Around line 45-68: Replace the inline arrow handler with a function
declaration: change the const onSubmit = async (data: RegisterRequest) => { ...
} to an async function declaration async function onSubmit(data:
RegisterRequest) { ... } while preserving the current body (calls to
form.clearErrors, signUp.email, form.setError, toast.success, form.reset, await
handleUserLogin(), and onSuccess?.()). Ensure the function name remains onSubmit
and its signature and return behavior are unchanged so references (e.g., passed
to the form submit handler) continue to work.
In `@frontend/src/context/notifications-context.tsx`:
- Around line 65-67: NotificationsProvider currently uses
watchlistService.useGetCurrentUserWatchlist(...) directly so cached
watchlistItems persist when isAuthenticated flips to false; fix by scoping the
data inside NotificationsProvider: create a scopedWatchlistItems =
isAuthenticated ? watchlistItems : [] and replace usages of watchlistItems with
scopedWatchlistItems (including the groupedWatchlistItems memo that calls
groupWatchlistItemsByProduct and the hasWatchlistItems boolean) so the provider
computes notifications only from scopedWatchlistItems and clears
watchlist-driven state when auth is false.
In `@frontend/src/db/auth-schema.ts`:
- Around line 53-55: The account table schema (symbols: accountId, providerId,
and the account table in frontend/src/db/auth-schema.ts) lacks a composite
unique constraint on (account_id, provider_id), allowing the same external
identity to map to multiple users; update the Drizzle table definition to add a
composite unique index/constraint for those columns (e.g., using Drizzle's
unique index helper or index(...).unique() for the account table) so
(account_id, provider_id) is enforced unique; also mirror this change in the SQL
migration used to create the account table so the database and schema stay in
sync.
In `@frontend/src/db/index.ts`:
- Around line 9-11: The Pool instantiation in frontend/src/db/index.ts currently
uses only connectionString which risks connection exhaustion and hung queries in
production; update the Pool creation (the pool constant created via new
Pool(...)) to include production-friendly configuration such as max (e.g., 20),
idleTimeoutMillis (e.g., 30000), and connectionTimeoutMillis (e.g., 2000) so the
pool enforces max connections, closes idle clients, and limits connection wait
time.
- Line 10: Validate process.env.DATABASE_URL before constructing the DB Pool:
check that the value used for connectionString (the variable passed to new
Pool/PoolConfig) is non-empty and defined, and if not throw or exit with a clear
error message (e.g., "DATABASE_URL is not set"). Update the Pool creation logic
in frontend/src/db/index.ts (the configuration that sets connectionString:
process.env.DATABASE_URL) to perform this guard so the app fails fast with an
actionable message rather than creating a Pool with an invalid connection
string.
In `@frontend/src/lib/api/api-base.ts`:
- Around line 111-115: The 401-refresh path is being retried again by the
generic retry logic; modify the flow in the functions handling requests
(referencing originalRequest and the "X-Retry-After-Refresh" header and the
MAX_RETRIES-based retry block) so that when you perform the refresh retry you
set originalRequest.headers["X-Retry-After-Refresh"]= "1" (or another sentinel)
and ensure the generic retry clause checks and skips any retry when that header
is present; i.e., after hitting the 401 refresh handler (the block checking
error.response?.status === 401) do the refresh and retry once with the header
set, and make the generic retry block also bail out if
originalRequest.headers["X-Retry-After-Refresh"] is set so no further
MAX_RETRIES attempts occur.
In `@frontend/src/lib/api/shopping-lists/index.ts`:
- Around line 126-128: Replace the exported arrow function with a named function
declaration: change the current "export const useGetCurrentUserShoppingLists =
(options?: { enabled?: boolean; }) => { ... }" into "export function
useGetCurrentUserShoppingLists(options?: { enabled?: boolean; }) { ... }" so it
follows the repo's function-declaration rule; keep the same parameter type
(options?: { enabled?: boolean }) and the same exported name and body (refer to
useGetCurrentUserShoppingLists) without altering callers or behavior.
In `@frontend/src/lib/auth.ts`:
- Around line 47-61: The accountLinking configuration currently sets
requireLocalEmailVerified: false which allows unverified credential accounts to
be hijacked via social logins; either re-enable verification by setting
requireLocalEmailVerified: true (restore safe default) or disable accountLinking
by setting account.accountLinking.enabled = false until email verification
(Resend) is implemented, and update the surrounding comment to note the
temporary change and reference the Resend/email-verification task and the
trustedProviders setting so reviewers can find accountLinking and
trustedProviders quickly.
- Around line 65-66: The Google OAuth clientId and clientSecret are being
force-cast with "as string" which can hide missing env values and cause runtime
failures; update the auth initialization code that sets clientId and
clientSecret (where NEXT_PUBLIC_GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET are
used) to explicitly validate these environment variables first (throw a clear
error or return a safe fallback) before assigning them to the provider config,
and remove the unsafe "as string" assertions so the values are guaranteed
present or the app fails fast with a helpful message.
- Around line 75-77: trustedOrigins is being set to an empty array when
NEXT_PUBLIC_APP_URL is unset which breaks CORS; update the trustedOrigins
assignment to validate NEXT_PUBLIC_APP_URL and fall back to a sensible default
(e.g., include 'http://localhost:3000' and/or window.location.origin when
available) so it is never empty, and validate the value is a well-formed URL
before adding it; change the code that sets trustedOrigins (the trustedOrigins
variable in frontend/src/lib/auth.ts) to use the validated env value or the
fallback array and log or throw a clear error if neither yields a valid origin.
- Around line 16-17: Check and fail-fast when initializing the auth config:
verify process.env.BETTER_AUTH_URL and process.env.BETTER_AUTH_SECRET before
they are used to build the auth client (the object containing baseURL and
secret). If either is missing or empty, throw a clear error (or call a fail
function) with both variable names included so initialization stops rather than
proceeding with undefined values; update the auth initialization site that sets
baseURL: process.env.BETTER_AUTH_URL and secret: process.env.BETTER_AUTH_SECRET
to perform this validation first.
---
Outside diff comments:
In `@frontend/src/context/user-context.tsx`:
- Around line 43-71: refreshUser can race with logout/session changes and
setUser with stale data; introduce a request counter ref (e.g., const
latestRequestRef = useRef(0)) and increment it at the start of refreshUser,
capture the current id in a local variable before any awaits, then before
calling setUser verify the captured id === latestRequestRef.current and bail out
if not; ensure the same guard is applied wherever setUser is called in this file
(the other user-fetching helpers at the ranges noted) and increment
latestRequestRef when performing logout or any session reset so in-flight
responses are ignored.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2704ea9e-5f1e-4225-ba5a-1528ad3ccff3
⛔ Files ignored due to path filters (2)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/public/disscount-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (64)
AGENTS.mdbackend/.env.examplebackend/pom.xmlbackend/src/main/java/disscount/auth/dao/RefreshTokenRepository.javabackend/src/main/java/disscount/auth/domain/RefreshToken.javabackend/src/main/java/disscount/auth/dto/AuthResponse.javabackend/src/main/java/disscount/auth/dto/GoogleAuthRequest.javabackend/src/main/java/disscount/auth/dto/LoginRequest.javabackend/src/main/java/disscount/auth/dto/RegisterRequest.javabackend/src/main/java/disscount/auth/rest/AuthController.javabackend/src/main/java/disscount/auth/service/AuthService.javabackend/src/main/java/disscount/auth/service/JwtService.javabackend/src/main/java/disscount/config/JwtAuthenticationFilter.javabackend/src/main/java/disscount/config/PublicEndpointConfig.javabackend/src/main/java/disscount/config/SecurityConfig.javabackend/src/main/java/disscount/config/UserProvisioningFilter.javabackend/src/main/java/disscount/user/dao/UserRepository.javabackend/src/main/java/disscount/user/domain/User.javabackend/src/main/java/disscount/user/dto/UserDto.javabackend/src/main/java/disscount/user/dto/UserRequest.javabackend/src/main/java/disscount/user/rest/UserController.javabackend/src/main/java/disscount/user/service/UserService.javabackend/src/main/java/disscount/util/EmailValidator.javabackend/src/main/java/disscount/util/PasswordValidator.javabackend/src/main/resources/application.propertiesbackend/src/test/java/disscount/shoppingList/rest/ShoppingListControllerTest.javafrontend/.env.local.examplefrontend/.npmrcfrontend/drizzle.config.tsfrontend/drizzle/0000_married_the_fury.sqlfrontend/drizzle/meta/0000_snapshot.jsonfrontend/drizzle/meta/_journal.jsonfrontend/next.config.tsfrontend/package.jsonfrontend/pnpm-workspace.yamlfrontend/src/app/(root)/components/sections/hero-section.tsxfrontend/src/app/api/auth/[...all]/route.tsfrontend/src/app/products/components/forms/add-to-shopping-list-form.tsxfrontend/src/app/products/components/product-action-buttons.tsxfrontend/src/app/statistics/page.tsxfrontend/src/components/custom/app-sidebar.tsxfrontend/src/components/custom/footer.tsxfrontend/src/components/custom/header/forms/account-details-modal.tsxfrontend/src/components/custom/header/forms/auth-modal.tsxfrontend/src/components/custom/header/forms/login-form.tsxfrontend/src/components/custom/header/forms/signup-form.tsxfrontend/src/components/custom/header/header.tsxfrontend/src/components/custom/header/user-menu.tsxfrontend/src/context/notifications-context.tsxfrontend/src/context/user-context.tsxfrontend/src/db/auth-schema.tsfrontend/src/db/index.tsfrontend/src/lib/api/api-base.tsfrontend/src/lib/api/auth/index.tsfrontend/src/lib/api/index.tsfrontend/src/lib/api/schemas/auth-user.tsfrontend/src/lib/api/shopping-lists/index.tsfrontend/src/lib/api/types.tsfrontend/src/lib/api/users/index.tsfrontend/src/lib/api/watchlist/index.tsfrontend/src/lib/auth-client.tsfrontend/src/lib/auth.tsfrontend/src/typings/local-storage.tsfrontend/src/utils/browser/local-storage.ts
💤 Files with no reviewable changes (18)
- frontend/src/typings/local-storage.ts
- backend/src/main/java/disscount/auth/dto/LoginRequest.java
- backend/src/main/java/disscount/auth/domain/RefreshToken.java
- backend/src/main/java/disscount/auth/dao/RefreshTokenRepository.java
- frontend/src/lib/api/auth/index.ts
- backend/src/main/java/disscount/auth/service/AuthService.java
- backend/src/main/java/disscount/auth/dto/RegisterRequest.java
- backend/src/main/java/disscount/user/dto/UserRequest.java
- backend/src/main/java/disscount/util/PasswordValidator.java
- backend/src/main/java/disscount/auth/service/JwtService.java
- backend/src/main/java/disscount/util/EmailValidator.java
- frontend/src/lib/api/index.ts
- backend/src/main/java/disscount/auth/dto/AuthResponse.java
- backend/src/main/java/disscount/auth/rest/AuthController.java
- backend/src/main/java/disscount/config/JwtAuthenticationFilter.java
- frontend/src/utils/browser/local-storage.ts
- backend/src/main/java/disscount/auth/dto/GoogleAuthRequest.java
- backend/src/main/java/disscount/config/PublicEndpointConfig.java
|
|
||
| If I tell you to refactor something into a separate component or function, make sure to check if there already exists a similar component or function in the codebase and reuse it instead of writing a new one. If there is no similar component or function, then create a new one in a separate file, instead of writing it in the same file. | ||
|
|
||
| Never edit the package.json or packege-lock.json files directly, but instead use "pnpm add package-name@version" or "pnpm remove package-name" to manage dependencies. |
Contributor
There was a problem hiding this comment.
Correct the lockfile name for pnpm projects.
The line contains a typo ("packege-lock.json") and references the wrong lockfile type. pnpm uses pnpm-lock.yaml, not package-lock.json (which is npm's lockfile). Since this project uses pnpm 11.5.2, the guidance should reference the correct lockfile.
📝 Proposed fix
-Never edit the package.json or packege-lock.json files directly, but instead use "pnpm add package-name@version" or "pnpm remove package-name" to manage dependencies.
+Never edit the package.json or pnpm-lock.yaml files directly, but instead use "pnpm add package-name@version" or "pnpm remove package-name" to manage dependencies.📝 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
| Never edit the package.json or packege-lock.json files directly, but instead use "pnpm add package-name@version" or "pnpm remove package-name" to manage dependencies. | |
| Never edit the package.json or pnpm-lock.yaml files directly, but instead use "pnpm add package-name@version" or "pnpm remove package-name" to manage dependencies. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 74, Update the sentence that currently reads "Never edit
the package.json or packege-lock.json files directly..." to fix the typo and
reference pnpm's lockfile: change "packege-lock.json" to "pnpm-lock.yaml" and
ensure the line instructs using pnpm commands (e.g., "pnpm add" / "pnpm remove")
rather than editing package.json or pnpm-lock.yaml directly; look for the exact
phrase "Never edit the package.json or packege-lock.json files directly" in
AGENTS.md to locate and update it.
Comment on lines
+30
to
+67
| public void ensureActiveProfile(UUID id, String email) { | ||
| Optional<User> existing = userRepository.findById(id); | ||
|
|
||
| if (existing.isPresent()) { | ||
| User user = existing.get(); | ||
| boolean changed = false; | ||
|
|
||
| // Safety net: if a profile is still soft-deleted/anonymized but the | ||
| // same identity is authenticating (e.g. better-auth deletion failed | ||
| // partway), revive it and restore the email from the token claim. | ||
| if (user.getDeletedAt() != null) { | ||
| user.setDeletedAt(null); | ||
| changed = true; | ||
| } | ||
| if (user.getEmail() == null && email != null) { | ||
| user.setEmail(email); | ||
| changed = true; | ||
| } | ||
|
|
||
| if (changed) { | ||
| userRepository.save(user); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // Can't create a profile without an email. | ||
| if (email == null) { | ||
| return; | ||
| } | ||
|
|
||
| User user = User.builder() | ||
| .id(id) | ||
| .email(email) | ||
| .build(); | ||
|
|
||
| userRepository.save(user); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid check-then-insert race in profile provisioning.
Line 31 and Line 61 perform a read-then-create sequence. Concurrent first requests for the same UUID can both see “missing”, then one insert fails with a duplicate key and returns 500.
Suggested fix
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
+import org.springframework.dao.DataIntegrityViolationException;
@@
- User user = User.builder()
- .id(id)
- .email(email)
- .build();
-
- userRepository.save(user);
+ User user = User.builder()
+ .id(id)
+ .email(email)
+ .build();
+
+ try {
+ userRepository.save(user);
+ } catch (DataIntegrityViolationException ignored) {
+ // Another concurrent request already created this profile.
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/java/disscount/user/service/UserService.java` around lines
30 - 67, The current ensureActiveProfile uses a read-then-create
(userRepository.findById then userRepository.save) which races on concurrent
first requests; change the logic to perform an atomic upsert or to attempt
create and recover on duplicate-key: try to save a new User when not found (or
call a repository upsert method if available), and if save throws a
unique-constraint/DataIntegrityViolationException, catch it, re-query with
userRepository.findById and apply the revive/email restore logic on the fetched
entity; keep the existing revive/email-setting behavior (clearing deletedAt,
setting email) and persist only when changed. Ensure you update
ensureActiveProfile to use this create-then-handle-duplicate pattern (or a DB
upsert) instead of the current check-then-insert to avoid the race.
Comment on lines
+44
to
+47
| if (user.getEmail() == null && email != null) { | ||
| user.setEmail(email); | ||
| changed = true; | ||
| } |
Contributor
There was a problem hiding this comment.
Sync email when JWT claim changes, not only when DB email is null.
Line 44 only restores email if it is null. If the IdP email changes, the backend profile keeps stale email indefinitely.
Suggested fix
import java.util.Optional;
+import java.util.Objects;
import java.util.UUID;
@@
- if (user.getEmail() == null && email != null) {
+ if (email != null && !Objects.equals(user.getEmail(), email)) {
user.setEmail(email);
changed = true;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/java/disscount/user/service/UserService.java` around lines
44 - 47, The current logic in UserService.java only updates the DB email when
user.getEmail() is null; change it to synchronize whenever the IdP JWT claim
differs from stored value: replace the condition that checks "user.getEmail() ==
null && email != null" with a comparison that sets user.setEmail(email) and
changed = true when email != null and !email.equals(user.getEmail()) (handle
nulls safely so we don't NPE), ensuring the stored email is updated whenever the
JWT claim changes.
|
|
||
| # Better Auth | ||
| BETTER_AUTH_SECRET=your_better_auth_secret_here | ||
| BETTER_AUTH_URL=your_app_url_here # for example: http://localhost:3000 |
Contributor
There was a problem hiding this comment.
Avoid inline comment inside BETTER_AUTH_URL value.
This format is parser-dependent and is already flagged by dotenv-linter; keep the example on its own comment line to prevent accidental bad values.
Proposed fix
-BETTER_AUTH_URL=your_app_url_here # for example: http://localhost:3000
+# Example: http://localhost:3000
+BETTER_AUTH_URL=your_app_url_here📝 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
| BETTER_AUTH_URL=your_app_url_here # for example: http://localhost:3000 | |
| # Example: http://localhost:3000 | |
| BETTER_AUTH_URL=your_app_url_here |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 9-9: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/.env.local.example` at line 9, The BETTER_AUTH_URL env example
contains an inline comment that breaks dotenv parsing; update the
BETTER_AUTH_URL line in .env.local.example to contain only the placeholder value
(e.g., BETTER_AUTH_URL=your_app_url_here) and move the explanatory example into
its own comment line above or below (e.g., # for example: http://localhost:3000)
so the parser sees a clean key=value and the guidance remains visible.
Source: Linters/SAST tools
Comment on lines
+1
to
+15
| CREATE TABLE "account" ( | ||
| "id" uuid PRIMARY KEY DEFAULT pg_catalog.gen_random_uuid() NOT NULL, | ||
| "account_id" text NOT NULL, | ||
| "provider_id" text NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "access_token" text, | ||
| "refresh_token" text, | ||
| "id_token" text, | ||
| "access_token_expires_at" timestamp, | ||
| "refresh_token_expires_at" timestamp, | ||
| "scope" text, | ||
| "password" text, | ||
| "created_at" timestamp DEFAULT now() NOT NULL, | ||
| "updated_at" timestamp NOT NULL | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if better-auth expects a unique constraint on (provider_id, account_id)
rg -n -C3 "provider_id.*account_id" --type=sql
rg -n -C3 "unique.*provider.*account" --type=sqlRepository: OffCrazyFreak/Disscount
Length of output: 49
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="frontend/drizzle/0000_married_the_fury.sql"
echo "=== File: $file (head) ==="
sed -n '1,120p' "$file"
echo
echo "=== File: $file (tail) ==="
tail -n 80 "$file"
echo
echo "=== Search within file for constraints/indexes on account ==="
rg -n "account|UNIQUE|constraint|primary key|unique index|index" "$file" --type=sql
echo
echo "=== Search repo for the account table definition and unique constraints ==="
rg -n --type=sql 'CREATE TABLE\s+"account"|CREATE TABLE\s+account|UNIQUE\s*\(|UNIQUE\s+INDEX|UNIQUE\s+'Repository: OffCrazyFreak/Disscount
Length of output: 5882
🌐 Web query:
better-auth account table unique constraint provider_id account_id composite unique
💡 Result:
In Better Auth, the account table is designed to support the linking of multiple authentication providers to a user. To ensure data integrity and prevent the same external account (from an OAuth provider, for example) from being claimed by multiple users, a composite unique constraint on (provider_id, account_id) is recommended and widely used [1][2][3]. While early or custom implementations of Better Auth might have incorrectly used a (user_id, provider_id) unique constraint, this is generally considered a mistake as it prevents account linking and recovery scenarios (e.g., if a user's OAuth identity changes its sub ID) [2]. The standard, intended invariant is that a specific provider-account pair must be unique across the entire database, preventing two different users from claiming the same external identity [2]. If you are setting up your database schema (for example, using Drizzle ORM or raw SQL), you should define this constraint as follows: PostgreSQL / Raw SQL Example: ALTER TABLE account ADD CONSTRAINT account_provider_account_unique UNIQUE (provider_id, account_id); [2] Drizzle ORM Example: You can add this to your table definition within the table builder function: export const account = pgTable("account", { //... columns }, (table) => [ uniqueIndex("account_provider_account_unique").on(table.providerId, table.accountId), ]); [3] This composite unique constraint ensures that the combination of the provider and the provider-specific account ID remains unique, fulfilling the requirements for robust identity management [1][2][3].
Citations:
- 1: https://better-auth.rs/docs/concepts/database
- 2: netzbegruenung/Gruenerator@e74c317
- 3: https://makerkit.dev/docs/nextjs-drizzle/database/schema
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=sql --glob 'frontend/drizzle/*.sql' \
'(UNIQUE\s*\(|uniqueIndex\(|CONSTRAINT\s+")' frontend/drizzle || true
rg -n --type=sql --glob 'frontend/drizzle/*.sql' \
'UNIQUE\s*\(\s*"provider_id"\s*,\s*"account_id"\s*\)|UNIQUE\s*\(\s*provider_id\s*,\s*account_id\s*\)|provider_id.*account_id.*UNIQUE|account_id.*provider_id.*UNIQUE|uniqueIndex\([^)]*providerId[^)]*accountId[^)]*\)' \
frontend/drizzle || trueRepository: OffCrazyFreak/Disscount
Length of output: 722
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Listing migrations in frontend/drizzle ==="
ls -la frontend/drizzle
echo
echo "=== Count of *.sql in frontend/drizzle ==="
find frontend/drizzle -maxdepth 1 -type f -name '*.sql' -print | wc -l
echo
echo "=== Filenames ==="
find frontend/drizzle -maxdepth 1 -type f -name '*.sql' -print | sortRepository: OffCrazyFreak/Disscount
Length of output: 462
🌐 Web query:
site:better-auth.rs database account provider_id account_id unique constraint
💡 Result:
In database schema design, applying a composite unique constraint on both provider_id and account_id is a standard practice for managing authentication records in multi-tenant or multi-provider systems [1][2]. This constraint ensures that for any given identity provider (such as "google", "github", or a "credential" provider), a specific account ID is registered to exactly one record in the database [1]. By pairing these two fields as a composite unique key, you prevent the duplication of external identity mappings, which is critical for maintaining data integrity when linking multiple OAuth providers or authentication methods to user entities [1][3]. Example implementation in a SQL schema: CREATE TABLE accounts ( id TEXT PRIMARY KEY, account_id TEXT NOT NULL, provider_id TEXT NOT NULL, user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, --... other fields UNIQUE(provider_id, account_id)); In this pattern: 1. provider_id acts as the namespace for the identifier [2]. 2. account_id is the unique identifier issued by that specific provider [2]. 3. The composite UNIQUE(provider_id, account_id) constraint enforces that the combination of these two values is unique across the entire table [1]. This design is commonly used in authentication frameworks to ensure that external accounts are correctly associated with local user profiles and to prevent conflicts during account linking or social login flows [3][4]. Attempting to insert a duplicate pair will typically result in a unique constraint violation error, which should be handled at the application layer to manage scenarios like existing account linking or social login conflicts [5].
Citations:
- 1: https://better-auth.rs/docs/concepts/database
- 2: https://better-auth.rs/docs/concepts/users-accounts
- 3: https://better-auth.rs/docs/concepts/oauth
- 4: https://better-auth.rs/docs/plugins/oauth
- 5: https://better-auth.rs/docs/reference/errors
Add composite UNIQUE constraint to account on (provider_id, account_id).
frontend/drizzle/0000_married_the_fury.sql defines "provider_id" and "account_id" on the "account" table but does not enforce a uniqueness constraint for their pair. Better Auth’s accounts model treats the provider+account identifier as a unique mapping across the database; without this, the same external identity could be linked to multiple users.
ALTER TABLE "account"
ADD CONSTRAINT "account_provider_account_unique" UNIQUE ("provider_id", "account_id");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/drizzle/0000_married_the_fury.sql` around lines 1 - 15, Add a
composite UNIQUE constraint to the "account" table to ensure the pair
("provider_id","account_id") is unique: modify the schema in the migration that
creates the "account" table (the CREATE TABLE for "account") to include a UNIQUE
constraint named account_provider_account_unique on columns provider_id and
account_id, or add an ALTER TABLE statement to add the constraint; reference the
table "account" and the columns "provider_id" and "account_id" and use the
constraint name account_provider_account_unique when implementing the change.
Comment on lines
+126
to
+128
| export const useGetCurrentUserShoppingLists = (options?: { | ||
| enabled?: boolean; | ||
| }) => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/lib/api/shopping-lists/index.ts"
echo "== File exists? =="
ls -la "$FILE" || true
echo
echo "== Lines 110-160 =="
nl -ba "$FILE" | sed -n '110,160p'Repository: OffCrazyFreak/Disscount
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/lib/api/shopping-lists/index.ts"
echo "== Lines 110-160 =="
awk 'NR>=110 && NR<=160 {printf "%d\t%s\n", NR, $0}' "$FILE"Repository: OffCrazyFreak/Disscount
Length of output: 1669
Use function declaration for exported hook useGetCurrentUserShoppingLists (frontend/src/lib/api/shopping-lists/index.ts:126-135).
The hook is currently exported as an arrow (export const ... = (...) => {}), which violates the repo’s function-declaration style rule for non-inline functions.
Proposed fix
-export const useGetCurrentUserShoppingLists = (options?: {
+export function useGetCurrentUserShoppingLists(options?: {
enabled?: boolean;
-}) => {
+}) {
return useQuery<ShoppingListDto[], Error>({
queryKey: ["shoppingLists", "me"],
queryFn: getCurrentUserShoppingLists,
// Only hit the per-user endpoint when logged in (avoids 401 spam on public pages).
enabled: options?.enabled ?? true,
});
-};
+}📝 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
| export const useGetCurrentUserShoppingLists = (options?: { | |
| enabled?: boolean; | |
| }) => { | |
| export function useGetCurrentUserShoppingLists(options?: { | |
| enabled?: boolean; | |
| }) { | |
| return useQuery<ShoppingListDto[], Error>({ | |
| queryKey: ["shoppingLists", "me"], | |
| queryFn: getCurrentUserShoppingLists, | |
| // Only hit the per-user endpoint when logged in (avoids 401 spam on public pages). | |
| enabled: options?.enabled ?? true, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/api/shopping-lists/index.ts` around lines 126 - 128, Replace
the exported arrow function with a named function declaration: change the
current "export const useGetCurrentUserShoppingLists = (options?: { enabled?:
boolean; }) => { ... }" into "export function
useGetCurrentUserShoppingLists(options?: { enabled?: boolean; }) { ... }" so it
follows the repo's function-declaration rule; keep the same parameter type
(options?: { enabled?: boolean }) and the same exported name and body (refer to
useGetCurrentUserShoppingLists) without altering callers or behavior.
Source: Coding guidelines
Comment on lines
+16
to
+17
| baseURL: process.env.BETTER_AUTH_URL, | ||
| secret: process.env.BETTER_AUTH_SECRET, |
Contributor
There was a problem hiding this comment.
Validate required environment variables.
BETTER_AUTH_URL and BETTER_AUTH_SECRET are required for better-auth to function securely, but there's no validation. If either is undefined, runtime errors or security issues will occur.
🛡️ Proposed fix
+if (!process.env.BETTER_AUTH_URL || !process.env.BETTER_AUTH_SECRET) {
+ throw new Error(
+ "BETTER_AUTH_URL and BETTER_AUTH_SECRET environment variables are required"
+ );
+}
+
export const auth = betterAuth({
baseURL: process.env.BETTER_AUTH_URL,
secret: process.env.BETTER_AUTH_SECRET,📝 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
| baseURL: process.env.BETTER_AUTH_URL, | |
| secret: process.env.BETTER_AUTH_SECRET, | |
| if (!process.env.BETTER_AUTH_URL || !process.env.BETTER_AUTH_SECRET) { | |
| throw new Error( | |
| "BETTER_AUTH_URL and BETTER_AUTH_SECRET environment variables are required" | |
| ); | |
| } | |
| export const auth = betterAuth({ | |
| baseURL: process.env.BETTER_AUTH_URL, | |
| secret: process.env.BETTER_AUTH_SECRET, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/auth.ts` around lines 16 - 17, Check and fail-fast when
initializing the auth config: verify process.env.BETTER_AUTH_URL and
process.env.BETTER_AUTH_SECRET before they are used to build the auth client
(the object containing baseURL and secret). If either is missing or empty, throw
a clear error (or call a fail function) with both variable names included so
initialization stops rather than proceeding with undefined values; update the
auth initialization site that sets baseURL: process.env.BETTER_AUTH_URL and
secret: process.env.BETTER_AUTH_SECRET to perform this validation first.
Comment on lines
+47
to
+61
| account: { | ||
| accountLinking: { | ||
| enabled: true, | ||
| // better-auth refuses to auto-link via TWO checks: | ||
| // 1) the incoming provider must be trusted OR verify the email, | ||
| // 2) requireLocalEmailVerified (default TRUE) — the EXISTING account's | ||
| // email must be verified. | ||
| // `trustedProviders` only satisfies (1). Our credential signups are | ||
| // unverified (no email verification yet), so we must also relax (2). | ||
| // NOTE: this is the pre-registration takeover tradeoff you accepted — | ||
| // revisit once Resend / email verification is wired up. | ||
| trustedProviders: ["google"], | ||
| requireLocalEmailVerified: false, | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
Account takeover risk: requireLocalEmailVerified: false.
The comment documents the tradeoff, but this configuration allows an attacker to:
- Create a credential account with an unverified email (e.g.,
victim@example.com) - Link a Google account with that same email
- Take over the account before the legitimate owner can verify
This persists until email verification is implemented. Consider either:
- Implementing email verification immediately (via Resend as mentioned in line 35), OR
- Disabling account linking until verification is in place, OR
- Adding prominent warnings in the UI that accounts are vulnerable until verified
Based on learnings: Always provide explanation of changes and why. The security risk here is significant because unverified credential accounts can be hijacked via social login, defeating the purpose of email-based identity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/auth.ts` around lines 47 - 61, The accountLinking
configuration currently sets requireLocalEmailVerified: false which allows
unverified credential accounts to be hijacked via social logins; either
re-enable verification by setting requireLocalEmailVerified: true (restore safe
default) or disable accountLinking by setting account.accountLinking.enabled =
false until email verification (Resend) is implemented, and update the
surrounding comment to note the temporary change and reference the
Resend/email-verification task and the trustedProviders setting so reviewers can
find accountLinking and trustedProviders quickly.
Source: Learnings
Comment on lines
+65
to
+66
| clientId: process.env.NEXT_PUBLIC_GOOGLE_CLIENT_ID as string, | ||
| clientSecret: process.env.GOOGLE_CLIENT_SECRET as string, |
Contributor
There was a problem hiding this comment.
Validate Google OAuth credentials before type assertion.
Type assertions (as string) bypass TypeScript's type safety. If these environment variables are undefined, better-auth will fail at runtime when Google sign-in is attempted.
🛡️ Proposed fix
+ const googleClientId = process.env.NEXT_PUBLIC_GOOGLE_CLIENT_ID;
+ const googleClientSecret = process.env.GOOGLE_CLIENT_SECRET;
+
+ if (!googleClientId || !googleClientSecret) {
+ throw new Error(
+ "NEXT_PUBLIC_GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET are required for Google OAuth"
+ );
+ }
+
google: {
- clientId: process.env.NEXT_PUBLIC_GOOGLE_CLIENT_ID as string,
- clientSecret: process.env.GOOGLE_CLIENT_SECRET as string,
+ clientId: googleClientId,
+ clientSecret: googleClientSecret,
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/auth.ts` around lines 65 - 66, The Google OAuth clientId and
clientSecret are being force-cast with "as string" which can hide missing env
values and cause runtime failures; update the auth initialization code that sets
clientId and clientSecret (where NEXT_PUBLIC_GOOGLE_CLIENT_ID and
GOOGLE_CLIENT_SECRET are used) to explicitly validate these environment
variables first (throw a clear error or return a safe fallback) before assigning
them to the provider config, and remove the unsafe "as string" assertions so the
values are guaranteed present or the app fails fast with a helpful message.
Comment on lines
+75
to
+77
| trustedOrigins: process.env.NEXT_PUBLIC_APP_URL | ||
| ? [process.env.NEXT_PUBLIC_APP_URL] | ||
| : [], |
Contributor
There was a problem hiding this comment.
Empty trustedOrigins array breaks CORS for authenticated requests.
If NEXT_PUBLIC_APP_URL is not set, trustedOrigins becomes an empty array. better-auth will reject cross-origin authenticated requests (e.g., from mobile apps or different domains), causing 403 errors. At minimum, validate the variable or add a sensible default.
🛡️ Proposed fix
+ if (!process.env.NEXT_PUBLIC_APP_URL) {
+ throw new Error("NEXT_PUBLIC_APP_URL environment variable is required");
+ }
+
trustedOrigins: process.env.NEXT_PUBLIC_APP_URL
? [process.env.NEXT_PUBLIC_APP_URL]
: [],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/auth.ts` around lines 75 - 77, trustedOrigins is being set
to an empty array when NEXT_PUBLIC_APP_URL is unset which breaks CORS; update
the trustedOrigins assignment to validate NEXT_PUBLIC_APP_URL and fall back to a
sensible default (e.g., include 'http://localhost:3000' and/or
window.location.origin when available) so it is never empty, and validate the
value is a well-formed URL before adding it; change the code that sets
trustedOrigins (the trustedOrigins variable in frontend/src/lib/auth.ts) to use
the validated env value or the fallback array and log or throw a clear error if
neither yields a valid origin.
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.
… + Spring resource server
WHY
The backend previously hand-rolled authentication (AuthService, JwtService, RefreshToken, BCrypt, custom password rules) and the frontend stored an access token in localStorage and refreshed via an HttpOnly cookie. This replaces all of it with better-auth as the identity provider, supporting email+password and Google sign-in, structured so Resend (password reset / email verification) and Facebook can be added later.
KEY ARCHITECTURAL FACT
better-auth is TypeScript/Node — it cannot run inside the Java/Spring backend. So better-auth lives in the Next.js app and is the identity provider; the Spring backend becomes an OAuth2 resource server that validates better-auth JWTs via JWKS. We deleted auth issuance from the backend, not identity verification — every per-user endpoint still needs to know who is calling.
DATA MODEL — split identity + profile (same id)
FRONTEND (Next.js 16 / Turbopack, pnpm 11)
npx @better-auth/cli generate), drizzle.config.ts.namefield, auth-modal Google button is real (signIn.social), account-details delete flow is 2-step (backend anonymize + authClient.deleteUser), "logout all" -> authClient.revokeOtherSessions(), removed stayLoggedInDays.BACKEND (Spring Boot 3.1.0 / Java 21)
sub(a UUID), so SecurityUtils.getCurrentUserId() is unchanged. Added UserProvisioningFilter after BearerTokenAuthenticationFilter.DATABASE / ENV
ACCOUNT DELETION SEMANTICS (business model)
Deleting an account keeps the business data as anonymized orphans (for dashboard/analytics) and frees the email for re-use: backend nulls PII on app_user (keeps the row + its watchlists/lists), and the client deletes the better-auth identity (user/session/account) which releases the unique email. Re-signup with the same email creates a fresh better-auth user (new UUID) + new profile.
PROBLEMS ENCOUNTERED & SOLUTIONS
pnpm installwon't re-apply an override change unless BOTH node_modules and pnpm-lock.yaml are deleted.KNOWN ISSUES (left as-is)
TODO (follow-ups)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation