[MohamedBassem]

Fixes #1716

[greptile-apps]

Confidence Score: 4/5

[Critical risk] Replaces the entire authentication system.

Resolve existing email collisions before merging so affected accounts retain password access.

Findings

1.  Existing accounts lose password access ▶
2.  Failed logout returns to bookmarks ▶

Fix with agent prompt

### Issue 1
packages/db/drizzle/0094_migrate_to_better_auth.sql:114-121
If an existing database contains both `Foo@example.com` and `foo@example.com`, this migration leaves the mixed-case email unchanged. The old signup code allowed both, but `validatePassword` now always lowercases the supplied email. The owner of `Foo@example.com` can no longer exchange their password for an API key, even with the original email and correct password.

Resolve these collisions before switching all lookups to lowercase, rather than silently leaving an unreachable account.

### Issue 2
apps/web/app/logout/page.tsx:17-20
`signOut` now returns an `AuthResult`, but this page ignores `ok` and always navigates home. If logout is rejected, such as when `DEGRADED_MODE` blocks `/sign-out`, the session remains active. The home page then sends the user straight back to their bookmarks without explaining why logout failed.

Check the result and show an error instead of treating a failed request as a completed logout.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Migrates web authentication from NextAuth to Better Auth, moves passwords into credential accounts, and updates account forms, admin actions, tests, and seed helpers.

• The migration leaves some existing case-colliding accounts unreachable through password-based API key exchange.
• The logout page ignores failed logout results.

_{Reviews (1) · Last reviewed commit: "feat(web): Migrate from next-auth to bet..."}

[greptile-apps]

Existing accounts lose password access

If an existing database contains both Foo@example.com and foo@example.com, this migration leaves the mixed-case email unchanged. The old signup code allowed both, but validatePassword now always lowercases the supplied email. The owner of Foo@example.com can no longer exchange their password for an API key, even with the original email and correct password.

Resolve these collisions before switching all lookups to lowercase, rather than silently leaving an unreachable account.

Knowledge Base Used:

• Data model and persistence
• Client integrations

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/db/drizzle/0094_migrate_to_better_auth.sql
Line: 114-121

Comment:
**Existing accounts lose password access**

If an existing database contains both `Foo@example.com` and `foo@example.com`, this migration leaves the mixed-case email unchanged. The old signup code allowed both, but `validatePassword` now always lowercases the supplied email. The owner of `Foo@example.com` can no longer exchange their password for an API key, even with the original email and correct password.

Resolve these collisions before switching all lookups to lowercase, rather than silently leaving an unreachable account.

**Knowledge Base Used:**
- [Data model and persistence](https://app.greptile.com/karakeep/-/custom-context/knowledge-base/karakeep-app/karakeep/-/docs/data-model-and-persistence.md)
- [Client integrations](https://app.greptile.com/karakeep/-/custom-context/knowledge-base/karakeep-app/karakeep/-/docs/client-integrations.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Failed logout returns to bookmarks

signOut now returns an AuthResult, but this page ignores ok and always navigates home. If logout is rejected, such as when DEGRADED_MODE blocks /sign-out, the session remains active. The home page then sends the user straight back to their bookmarks without explaining why logout failed.

Check the result and show an error instead of treating a failed request as a completed logout.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/app/logout/page.tsx
Line: 17-20

Comment:
**Failed logout returns to bookmarks**

`signOut` now returns an `AuthResult`, but this page ignores `ok` and always navigates home. If logout is rejected, such as when `DEGRADED_MODE` blocks `/sign-out`, the session remains active. The home page then sends the user straight back to their bookmarks without explaining why logout failed.

Check the result and show an error instead of treating a failed request as a completed logout.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[pullfrog]

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes

• Auth backend swapped to better-auth — apps/web/server/auth.ts builds a betterAuth() instance with email/password plus the custom genericOAuth provider, enforces a path allowlist + Karakeep rate limiting in a before hook, and reimplements read-only/demo/degraded, signup/turnstile, role/quota, and logging behavior.
• Sessions and password storage reworked — getServerAuthSession wraps better-auth getSession; passwords move from user.password/salt into account rows (providerId='credential'), with hashPassword/verifyPasswordHash/hasPassword/setUserPassword in packages/trpc/auth.ts.
• DB schema + 0094 migration — emailVerified becomes boolean, account/session/verificationToken are rebuilt to better-auth's shape, legacy hashes are re-encoded, emails are lowercased, and old sessions/tokens are dropped.
• Routers, UI and tooling rewired — the old users.create/changePassword/verify/reset procedures are removed in favor of better-auth endpoints; admin create/reset, invites, sign-in/signup/verify pages, OAuth provider list, config/docs, and the benchmark/e2e/seed-snapshot signup helpers are all updated.

ℹ️ OAuth ID tokens are no longer signature-verified

next-auth verified OIDC ID tokens against the provider's JWKS and enforced the OAUTH_ID_TOKEN_SIGNED_RESPONSE_ALG allowlist. better-auth's genericOAuth getUserInfo only decodeJwts the ID token — it does not check the signature — so OAuth identity and the email_verified flag used during account linking now rest entirely on the token arriving unmodified over the server-to-server token-exchange TLS channel. That is a defense-in-depth reduction rather than an end-user-exploitable bug (the token is fetched directly from the configured provider), but operators who relied on strict ID-token validation should be aware of it.

Technical details

# genericOAuth decodes ID tokens without verifying signatures

## Affected sites
- `apps/web/server/auth.ts` — `createOAuthConfig` drops `idTokenSignedResponseAlg` and delegates to `genericOAuth`, whose `getUserInfo` (`better-auth/dist/plugins/generic-oauth/routes.mjs:406-417`) returns `decodeJwt(tokens.idToken)` payload fields with no JWKS verification.
- `packages/shared/config.ts` — `OAUTH_ID_TOKEN_SIGNED_RESPONSE_ALG` and `OAUTH_TIMEOUT` are removed; existing deployments that set them will have them silently ignored.

## Required outcome
- Make the change in verification strength explicit for operators (release note / upgrade docs), and confirm no supported provider needs signed-ID-token enforcement.

## Open questions for the human
- Does any target provider require ID-token signature verification? If so, is there a better-auth hook (e.g. a custom `getUserInfo` that verifies against the discovery JWKS) worth configuring?

ℹ️ Nitpicks

• The admin user list (apps/web/components/admin/UserList.tsx) and CLI no longer show/return localUser, so there is no longer any UI signal distinguishing password accounts from OAuth-only ones. whoami still exposes it, so this looks intentional — ignore if the removal was deliberate.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

This guard deliberately leaves a mixed-case row untouched when lowercasing it would collide with another account, but better-auth resolves accounts by the lowercased email. Such a user can then no longer sign in or request a password reset, and the skip is silent. Consider logging the affected ids/emails so operators can repair them (admin reset password or email change).

Technical details

# Migration silently locks out case-colliding accounts

## Affected sites
- `packages/db/drizzle/0094_migrate_to_better_auth.sql:112-121` — rows excluded by the `NOT EXISTS` collision guard keep their mixed-case email.
- `packages/trpc/auth.ts:264` and better-auth's lookups use `email.toLowerCase()`, so a mixed-case row becomes unreachable by sign-in and password reset.

## Required outcome
- Affected accounts must be discoverable by an operator instead of silently skipped.

## Suggested approach
- Before the `UPDATE`, run a `SELECT` that lists the ids/emails that will be skipped (the migration runner surfaces query output/logs), or document the manual recovery path. A true rebuild-based lowercasing is not possible without resolving the unique conflict.

[pullfrog]

ℹ️ No new issues in this delta — prior migration note remains open.

Reviewed changes since the prior review — this push is a rebase/re-push; no production code changed between f6a6e14 and 159b379. The only delta is in packages/trpc/routers/apiKeys.test.ts:

• Added an API-key authorization test — an API key cannot revoke other keys asserts apiKeys.revoke from an API-key caller rejects with API keys can only revoke themselves and leaves the target key intact.
• Test import/formatting cleanup — the legacy-salted-password test now loads the User model locally where it's used.

The new test pins real behavior with an exact error-message assertion and a DB-state check, so it can fail if the authorization guard regresses.

The migration case-collision inline comment from the prior review is still outstanding and was not touched by this push; leaving this review non-approving until it is resolved or waived.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — one docs nitpick below.

Reviewed changes (since the prior pullfrog review at 159b3792):

• Made trusted origins configurable — added AUTH_TRUSTED_ORIGINS (default *) and replaced the fixed trustedOrigins: ["karakeep://"] with a getTrustedOrigins function. Explicit entries are trusted for both the Origin check and callbackURL redirects; * expands to the request's own Host/X-Forwarded-Host, restoring next-auth's "sign in from any address" behavior. Verified against better-auth 1.6.25: * deliberately does not widen callbackURL validation, so it cannot introduce an open redirect.
• Made OAuth token-endpoint auth configurable — added OAUTH_TOKEN_ENDPOINT_AUTH_METHOD (client_secret_basic/client_secret_post) and encodeClientSecret, which form-urlencodes the client secret for the Basic header (better-auth interpolates it raw). client_secret_post sends the raw secret, which better-auth form-encodes via URLSearchParams.
• Added coverage — new apps/web/server/auth.test.ts cases for LAN-IP and X-Forwarded-Host sign-in, cross-origin + callbackURL rejection, AUTH_TRUSTED_ORIGINS restricting/listing, and Basic vs client_secret_post token requests (exact base64 credentials and form bodies).

Commit 1569f019 (base main 292c13a5).

ℹ️ Nitpicks

• The NEXTAUTH_URL row in docs/docs/03-configuration/01-environment-variables.md still reads "Sign-in and other authentication requests coming from any other address are rejected, so it must match the URL you open Karakeep with." That is no longer true now that AUTH_TRUSTED_ORIGINS defaults to *; the row directly below explains the new behavior, so the two rows now contradict each other.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes (since the prior pullfrog review at 1569f019):

• Corrected the NEXTAUTH_URL docs — the row no longer claims that sign-ins from other addresses are rejected; it now describes the address's real roles and points to AUTH_TRUSTED_ORIGINS, resolving the contradiction flagged in the last review. The NEXTAUTH_SECRET wording is also updated in both the env-vars table and the dev setup page.
• Taught the seed helpers to pass better-auth's origin check — the signUpUser HTTP helper in packages/benchmarks/src/trpc.ts, packages/e2e_tests/utils/api.ts, and tools/seed-snapshot/src/index.ts now sends an Origin header matching the target base URL, which better-auth's origin/CSRF protection requires for server-side callers.

No production code changed in this push. The one remaining open item is the migration case-collision note on packages/db/drizzle/0094_migrate_to_better_auth.sql:112-121 from the initial review; it is untouched by these commits, so this review stays non-approving.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}