fix: address maintainer review feedback for multi-user PR

Critical fixes:
- Update APP_DB_VERSION to 234 to trigger migration (was 233)
  * Without this, the migration would never run
  * Migration is now correctly applied on server start

Documentation improvements in MULTI_USER.md:
- Clarify use of user_data table (OAuth v229) vs user_info (MFA)
- Explain why users are NOT Becca entities:
  * Auth data should never be synced for security
  * Becca is for synchronized content only
  * Each instance needs isolated user databases
- Document future sync support requirements
- Add note about migration triggering mechanism

This addresses eliandoran's comments on PR #7441:
- Migration not applying due to version mismatch
- Question about user_info vs user_data table
- Concern about Becca entity model integration
- Question about cross-instance synchronization
This commit is contained in:
Somoru 2025-10-21 15:25:27 +05:30
parent 6cde730553
commit ccaabcf933
2 changed files with 21 additions and 3 deletions

View File

@ -10,7 +10,23 @@ Trilium now supports multiple users with role-based access control. Each user ha
### Database Schema ### Database Schema
Multi-user support extends the existing `user_data` table (introduced in migration v229 for OAuth): Multi-user support extends the existing `user_data` table (introduced in migration v229 for OAuth support).
**Important Design Decisions:**
1. **Why `user_data` table?** eliandoran asked about using `user_info` table from MFA. We use `user_data` because it's the established table from OAuth migration (v229) with existing password hashing infrastructure.
2. **Why not Becca entities?** Users are NOT implemented as Becca entities because:
- Becca entities are for **synchronized content** (notes, branches, attributes, etc.)
- User authentication data should **never be synced** across instances for security
- Each Trilium instance needs its own isolated user database
- Syncing user credentials would create massive security risks
3. **Future sync support:** When multi-user sync is implemented, it will need:
- Per-user sync credentials on each instance
- User-to-user mappings across instances
- Separate authentication from content synchronization
- This is documented as a future enhancement
**user_data table fields:** **user_data table fields:**
- `tmpID`: INTEGER primary key - `tmpID`: INTEGER primary key
@ -34,11 +50,13 @@ Multi-user support extends the existing `user_data` table (introduced in migrati
### Migration (v234) ### Migration (v234)
**Migration Triggering:** This migration runs automatically on next server start because the database version was updated to 234 in `app_info.ts`.
The migration automatically: The migration automatically:
1. Extends the `user_data` table with role and status fields 1. Extends the `user_data` table with role and status fields
2. Adds `userId` columns to notes, branches, etapi_tokens, and recent_notes tables 2. Adds `userId` columns to notes, branches, etapi_tokens, and recent_notes tables
3. Creates a default admin user from existing single-user credentials 3. Creates a default admin user from existing single-user credentials
4. Associates all existing data with the admin user 4. Associates all existing data with the admin user (tmpID=1)
5. Maintains backward compatibility with single-user installations 5. Maintains backward compatibility with single-user installations
## Setup ## Setup

View File

@ -4,7 +4,7 @@ import packageJson from "../../package.json" with { type: "json" };
import dataDir from "./data_dir.js"; import dataDir from "./data_dir.js";
import { AppInfo } from "@triliumnext/commons"; import { AppInfo } from "@triliumnext/commons";
const APP_DB_VERSION = 233; const APP_DB_VERSION = 234;
const SYNC_VERSION = 36; const SYNC_VERSION = 36;
const CLIPPER_PROTOCOL_VERSION = "1.0"; const CLIPPER_PROTOCOL_VERSION = "1.0";