|
| 1 | +# Chore: Remove Legacy Authentication Tables and Code |
| 2 | + |
| 3 | +**Project:** freundebuch2 |
| 4 | +**Type:** Chore |
| 5 | +**Related Epic:** [Epic 18: Better Auth Migration](../epics/epic-18-planned-better-auth-migration.md) |
| 6 | +**Phase:** Phase 1.5 (Post-MVP Enhancement) |
| 7 | +**Priority:** Medium |
| 8 | + |
| 9 | +--- |
| 10 | + |
| 11 | +## Summary |
| 12 | + |
| 13 | +The authentication system was migrated from a custom JWT/session-based implementation to Better Auth in commit `3681be2`. The legacy database tables (`auth.users`, `auth.sessions`, `auth.password_reset_tokens`), the old service layer, utility functions, SQL queries, and configuration variables are still present in the codebase. Now that Better Auth is the sole authentication provider, this dead code and the legacy tables should be removed to reduce confusion, maintenance burden, and attack surface. |
| 14 | + |
| 15 | +--- |
| 16 | + |
| 17 | +## Background |
| 18 | + |
| 19 | +The original auth system used three tables in the `auth` schema: |
| 20 | + |
| 21 | +- **`auth.users`** — email/password credentials with bcrypt hashes |
| 22 | +- **`auth.sessions`** — SHA-256 hashed session tokens with expiry |
| 23 | +- **`auth.password_reset_tokens`** — time-limited password reset tokens |
| 24 | + |
| 25 | +Better Auth replaced these with its own tables (`auth.user`, `auth.session`, `auth.account`, `auth.verification`, `auth.passkey`). The migration (`1773320702000_better-auth-migration.ts`) copied all users from `auth.users` to `auth.user`, preserving bcrypt password hashes with transparent dual verification. |
| 26 | + |
| 27 | +The legacy code is fully dead — no production code path imports or calls it. However, two systems still reference `auth.users`: |
| 28 | + |
| 29 | +1. **SabreDAV** (`apps/sabredav/src/Auth/AppPasswordBackend.php`, `FreundebuchCardDAVBackend.php`, `FreundebuchPrincipalBackend.php`) queries `auth.users` to resolve users by email and joins against `auth.app_passwords.user_id` (integer FK to `auth.users.id`). |
| 30 | +2. **Backend app-passwords queries** (`apps/backend/src/models/queries/app-passwords.sql`) join against `auth.users` for app password management. |
| 31 | + |
| 32 | +These dependencies must be migrated to query `auth.user` (Better Auth) before the legacy `auth.users` table can be dropped. |
| 33 | + |
| 34 | +--- |
| 35 | + |
| 36 | +## Phase 1: Remove Dead Code (No Blockers) |
| 37 | + |
| 38 | +The following artifacts are completely unused and can be removed immediately. |
| 39 | + |
| 40 | +### Files to Delete |
| 41 | + |
| 42 | +| File | Reason | |
| 43 | +|------|--------| |
| 44 | +| `apps/backend/src/services/auth.service.ts` | Entire legacy auth service (~490 lines). Not imported anywhere. | |
| 45 | +| `apps/backend/src/models/queries/sessions.sql` | Legacy session queries. Not used by any production code. | |
| 46 | +| `apps/backend/src/models/queries/sessions.queries.ts` | PgTyped-generated file from `sessions.sql`. Not imported anywhere. | |
| 47 | +| `apps/backend/src/models/queries/password-reset-tokens.sql` | Legacy password reset queries. Not used by any production code. | |
| 48 | +| `apps/backend/src/models/queries/password-reset-tokens.queries.ts` | PgTyped-generated file from `password-reset-tokens.sql`. Not imported anywhere. | |
| 49 | + |
| 50 | +### Functions to Remove from `apps/backend/src/utils/auth.ts` |
| 51 | + |
| 52 | +The following exported functions are only imported by `auth.service.ts` (which is itself dead code): |
| 53 | + |
| 54 | +- `generateToken()` — JWT generation |
| 55 | +- `verifyToken()` — JWT verification |
| 56 | +- `generateSessionToken()` — random session token generation |
| 57 | +- `hashSessionToken()` — SHA-256 hashing |
| 58 | +- `getSessionExpiry()` — session expiry calculation |
| 59 | +- `generatePasswordResetToken()` — random reset token generation |
| 60 | +- `getPasswordResetExpiry()` — reset token expiry calculation |
| 61 | +- `getAuthTokenCookieOptions()` — legacy JWT cookie options |
| 62 | + |
| 63 | +**Keep** `hashPassword()`, `verifyPassword()`, `getSessionCookieOptions()`, and `getClearCookieOptions()` if still referenced by Better Auth or other production code. If not, remove the entire file. |
| 64 | + |
| 65 | +### Legacy Queries to Remove from `apps/backend/src/models/queries/users.sql` |
| 66 | + |
| 67 | +Remove the following queries that target the legacy `auth.users` table: |
| 68 | + |
| 69 | +- `GetUserByExternalId` |
| 70 | +- `GetUserByEmail` |
| 71 | +- `GetUserByEmailWithSelfProfile` |
| 72 | +- `CreateUser` |
| 73 | +- `UpdateUser` |
| 74 | +- `UpdateUserPassword` |
| 75 | +- `DeleteUser` |
| 76 | + |
| 77 | +**Keep** queries that target `auth."user"` (Better Auth): `GetUserWithPreferences`, `GetUserSelfProfile`, `SetUserSelfProfile`, `HasSelfProfile`. |
| 78 | + |
| 79 | +### Configuration Cleanup |
| 80 | + |
| 81 | +**`apps/backend/src/utils/config.ts`** — Remove legacy optional env vars: |
| 82 | +- `JWT_SECRET?` |
| 83 | +- `JWT_EXPIRY?` |
| 84 | +- `SESSION_SECRET?` |
| 85 | +- `SESSION_EXPIRY_DAYS?` |
| 86 | +- `PASSWORD_RESET_EXPIRY_HOURS?` |
| 87 | + |
| 88 | +**`.env.example`** — Remove the commented-out legacy auth section. |
| 89 | + |
| 90 | +### Dependencies to Audit |
| 91 | + |
| 92 | +Check whether `jsonwebtoken` and `bcrypt` in `apps/backend/package.json` are still needed after removing the legacy code. Better Auth uses its own password hashing (scrypt) but the dual-verification path may still use bcrypt via the custom `verifyPassword` callback in `apps/backend/src/lib/auth.ts`. If bcrypt is still needed for legacy password verification, keep it; otherwise remove it. `jsonwebtoken` should be safe to remove entirely. |
| 93 | + |
| 94 | +### Documentation |
| 95 | + |
| 96 | +- Update or archive `docs/security-session-tokens.md` (describes the legacy session approach) |
| 97 | +- Update Epic 18 status to reflect cleanup completion |
| 98 | + |
| 99 | +--- |
| 100 | + |
| 101 | +## Phase 2: Migrate SabreDAV and App Passwords off Legacy Tables |
| 102 | + |
| 103 | +Before the legacy `auth.users` table can be dropped, the following must be updated: |
| 104 | + |
| 105 | +### SabreDAV PHP Files |
| 106 | + |
| 107 | +| File | Change | |
| 108 | +|------|--------| |
| 109 | +| `apps/sabredav/src/Auth/AppPasswordBackend.php` | Query `auth.user` (text `id`) instead of `auth.users` (integer `id`) | |
| 110 | +| `apps/sabredav/src/CardDAV/FreundebuchCardDAVBackend.php` | Query `auth.user` instead of `auth.users` | |
| 111 | +| `apps/sabredav/src/Principal/FreundebuchPrincipalBackend.php` | Query `auth.user` instead of `auth.users` | |
| 112 | +| Integration tests referencing `auth.users` | Update to use `auth.user` | |
| 113 | + |
| 114 | +### Backend App Passwords |
| 115 | + |
| 116 | +| File | Change | |
| 117 | +|------|--------| |
| 118 | +| `apps/backend/src/models/queries/app-passwords.sql` | Change JOINs from `auth.users` to `auth.user`; update `user_id` references from integer to text | |
| 119 | + |
| 120 | +### Database Migration |
| 121 | + |
| 122 | +Create a migration to: |
| 123 | + |
| 124 | +1. Alter `auth.app_passwords.user_id` from `INTEGER` (FK to `auth.users.id`) to `TEXT` (FK to `auth.user.id`), mapping existing values via the `auth.users.external_id` <-> `auth.user.id` relationship |
| 125 | +2. Drop `auth.password_reset_tokens` |
| 126 | +3. Drop `auth.sessions` |
| 127 | +4. Drop `auth.users` |
| 128 | + |
| 129 | +--- |
| 130 | + |
| 131 | +## Acceptance Criteria |
| 132 | + |
| 133 | +### Phase 1 |
| 134 | + |
| 135 | +- [ ] `apps/backend/src/services/auth.service.ts` is deleted |
| 136 | +- [ ] `apps/backend/src/models/queries/sessions.sql` and its `.queries.ts` are deleted |
| 137 | +- [ ] `apps/backend/src/models/queries/password-reset-tokens.sql` and its `.queries.ts` are deleted |
| 138 | +- [ ] Legacy JWT/session utility functions are removed from `apps/backend/src/utils/auth.ts` |
| 139 | +- [ ] Legacy queries are removed from `apps/backend/src/models/queries/users.sql` (and `.queries.ts` regenerated) |
| 140 | +- [ ] Legacy env vars are removed from `config.ts` and `.env.example` |
| 141 | +- [ ] `jsonwebtoken` dependency is removed from `apps/backend/package.json` |
| 142 | +- [ ] `docs/security-session-tokens.md` is updated or archived |
| 143 | +- [ ] All existing tests pass |
| 144 | +- [ ] No production code imports any deleted file (verify with `grep -r` across `apps/backend/src/`) |
| 145 | + |
| 146 | +### Phase 2 |
| 147 | + |
| 148 | +- [ ] SabreDAV PHP files query `auth.user` instead of `auth.users` |
| 149 | +- [ ] `app-passwords.sql` queries join against `auth.user` instead of `auth.users` |
| 150 | +- [ ] `auth.app_passwords.user_id` is migrated from integer to text with correct FK |
| 151 | +- [ ] `auth.password_reset_tokens`, `auth.sessions`, and `auth.users` tables are dropped via migration |
| 152 | +- [ ] SabreDAV integration tests pass |
| 153 | +- [ ] App password integration tests pass |
| 154 | +- [ ] CardDAV sync continues to work end-to-end |
| 155 | + |
| 156 | +--- |
| 157 | + |
| 158 | +## Out of Scope |
| 159 | + |
| 160 | +- Changes to Better Auth configuration or its tables |
| 161 | +- Frontend authentication changes (already fully migrated) |
| 162 | +- Password rehashing strategy (dual bcrypt/scrypt verification remains as-is) |
| 163 | +- Any new auth features (e.g., OAuth providers, MFA beyond passkeys) |
| 164 | + |
| 165 | +--- |
| 166 | + |
| 167 | +## Dependencies |
| 168 | + |
| 169 | +- **Phase 1** has no blockers and can proceed immediately |
| 170 | +- **Phase 2** blocks on Phase 1 being complete (to avoid partial states) |
| 171 | +- **Dropping `auth.users`** blocks on SabreDAV and app-passwords being migrated (Phase 2) |
| 172 | +- Related: [Better Auth Production Deployment](./better-auth-production-deployment.md) should be completed before or alongside this work |
| 173 | + |
| 174 | +--- |
| 175 | + |
| 176 | +## Related Issues |
| 177 | + |
| 178 | +- [Better Auth Production Deployment Checklist](./better-auth-production-deployment.md) — documents the SabreDAV legacy dependency |
| 179 | +- [Epic 18: Better Auth Migration](../epics/epic-18-planned-better-auth-migration.md) — parent epic, cleanup steps outlined in lines 305–309 |
0 commit comments