Files
graph-requirements-planner/.planning/research/PITFALLS.md
ANDREW HOSFORD 0e8978f48c Update project files
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 11:59:48 -05:00

444 lines
26 KiB
Markdown

# Pitfalls Research
**Domain:** Adding multi-tenancy, user accounts, invite systems, and RBAC to existing single-user FastAPI+SQLite app
**Researched:** 2026-03-18
**Confidence:** MEDIUM-HIGH
## Critical Pitfalls
### Pitfall 1: Forgetting tenant_id Filters in Queries
**What goes wrong:**
Endpoints return all data across all users/tenants instead of just the authenticated user's data. A single missing `.filter(owner_id == current_user.id)` exposes another user's projects, requirements, or edit history.
**Why it happens:**
When migrating from single-user to multi-user, the existing codebase has no tenant scoping. Every SQLAlchemy query was written assuming global access. Developers update auth but forget to audit and update every single query in the application.
**How to avoid:**
- Create base query functions that automatically inject tenant filters
- Use SQLAlchemy events or custom query classes to enforce tenant scoping
- Never trust frontend filtering — always enforce at backend/database level
- Code review checklist: "Does this query filter by user/tenant?"
- Write integration tests that attempt cross-tenant access
**Warning signs:**
- "I can see another user's data in the API response"
- Tests pass in isolation but fail when multiple users exist
- Postman/curl requests with different tokens return overlapping data
**Phase to address:**
Phase 1 (Database Schema & User Model) — Establish the tenant scoping pattern from the start, create helper functions/base classes that enforce it.
---
### Pitfall 2: SQLite Foreign Key Constraint Failures During Migration
**What goes wrong:**
Adding `user_id` foreign keys to existing tables (projects, requirements, history, layouts) causes migration failures or silently drops data. SQLite's ALTER TABLE limitations force table recreation, and foreign key constraints can cascade-delete dependent data unexpectedly.
**Why it happens:**
SQLite doesn't support true ALTER TABLE — it creates a new table, copies data, drops the old one. If foreign keys are enabled (`PRAGMA foreign_keys = ON`) during this process, cascade relationships can destroy data. The pragma can't be toggled mid-transaction, and migration tools run in transactions.
**How to avoid:**
- Use Alembic's batch mode for SQLite schema changes
- For simple column additions, use `add_column` + `add_index` without foreign key constraints
- Enforce user_id constraints at application level for SQLite
- Consider backfilling data BEFORE adding constraints
- Test migrations on production data copy, not empty database
- Have rollback plan and backup before running migration
**Warning signs:**
- Migration succeeds but table is empty
- "FOREIGN KEY constraint failed" errors
- Dependent tables lose records after migration
- `__old_tablename` tables remain in schema
**Phase to address:**
Phase 1 (Database Schema & User Model) — Design migration strategy carefully, test thoroughly with realistic data volumes.
---
### Pitfall 3: Existing JWT Tokens Become Invalid
**What goes wrong:**
After deploying user account changes, the single shared password JWT tokens become invalid. All active users are logged out. Worse, the JWT payload structure changes (adding user_id, roles) but the backend doesn't handle old token format, causing 401s or crashes.
**Why it happens:**
Current JWTs encode the shared password authentication, not a user identity. After migration, the auth system expects `user_id` in the token payload. Old tokens lack this field. Token verification fails or returns None for current_user.
**How to avoid:**
- Plan for token format versioning (include `version: 2` in new tokens)
- Handle both old and new token formats during transition period
- Accept that a forced re-login is sometimes necessary — communicate this
- Use a migration window: deploy new auth, give users 24 hours to re-login before breaking old tokens
- Consider token migration endpoint: exchange old token for new token
**Warning signs:**
- All users logged out immediately after deployment
- "Invalid token" errors spike in logs
- `current_user.id` is None/undefined in dependency injection
- Frontend can't decode JWT payload
**Phase to address:**
Phase 2 (Authentication & Authorization) — Handle gracefully with versioned tokens or planned re-login window. Document in migration notes.
---
### Pitfall 4: Orphaned Data Without Clear Ownership
**What goes wrong:**
Existing projects, requirements, and layouts in the database have no `owner_id`. After adding user accounts, who owns this data? Assigning it all to the first admin is wrong. Leaving it NULL breaks foreign key constraints. Making it globally accessible defeats the purpose of multi-tenancy.
**Why it happens:**
The migration script adds `user_id` columns but doesn't define a strategy for pre-existing data. It's a business logic decision (who should own legacy data?) disguised as a technical problem.
**How to avoid:**
- Decide ownership strategy BEFORE migration:
- Option A: Assign all to designated "admin" user (create during migration)
- Option B: Create "shared" workspace owned by system user
- Option C: Mark legacy data as owned by each user (copy to all users)
- Option D: Delete legacy data (if acceptable)
- Document the decision and rationale
- Make it reversible if possible (backup tables)
- Allow users to claim/import legacy data post-migration
**Warning signs:**
- NULL user_id values in production
- Foreign key constraint errors after migration
- Users complain about missing historical data
- Unclear who can edit legacy projects
**Phase to address:**
Phase 1 (Database Schema & User Model) — Define ownership strategy as part of migration plan, not an afterthought.
---
### Pitfall 5: Role Explosion and Privilege Creep
**What goes wrong:**
Starting with simple roles (viewer, editor, admin) quickly becomes dozens of overly-specific roles: "ProjectOwnerCanEditGraphsButNotDelete", "EditorForPillarRequirementsOnly". Permission checking logic becomes unmaintainable. Users accumulate permissions as they're granted roles for different projects.
**Why it happens:**
Creating new roles is easy. Removing or merging roles is hard. Each edge case ("what if user needs read on Project A but write on Project B?") tempts adding a new role instead of using proper RBAC patterns. Users change roles but old permissions aren't revoked.
**How to avoid:**
- Start with MINIMAL roles: Owner, Editor, Viewer
- Roles are per-project, not global
- Don't create roles for every permission combination
- Implement permission inheritance: Editor includes Viewer permissions
- Use project-level assignments, not feature-level roles
- Audit user roles quarterly, revoke unused permissions
- Track role changes in audit log
**Warning signs:**
- More roles than users
- Role names longer than 3 words
- Permission checks require nested if/else trees
- Users have 5+ roles assigned
- "I don't know what role to give this user"
**Phase to address:**
Phase 3 (Project Ownership & Sharing) — Define minimal, composable role set from the start. Resist adding roles.
---
### Pitfall 6: Invite System Security Weaknesses
**What goes wrong:**
Invite codes are predictable (sequential IDs, simple UUIDs), reusable after first use, never expire, or lack the invite token in the registration endpoint. Attackers enumerate invite codes or reuse leaked ones to create unauthorized accounts.
**Why it happens:**
Invite system feels like a simple feature: generate token, send email, validate token on signup. The security edge cases (expiration, single-use, brute force protection) get deprioritized as "nice to have."
**How to avoid:**
- Use cryptographically secure random tokens (secrets.token_urlsafe(32))
- Make invites single-use: mark as consumed on successful registration
- Set expiration (7-30 days) and enforce it
- Rate-limit invite generation and redemption endpoints
- Don't expose invite codes in URLs if possible (use POST body)
- Log invite creation/redemption for audit trail
- Allow invite revocation before use
**Warning signs:**
- Invite codes are short (< 20 chars) or sequential
- Same invite works multiple times
- Invites never expire
- No rate limiting on /register endpoint
- Invite validation happens client-side only
**Phase to address:**
Phase 2 (Authentication & Authorization) — Build invite system with security as primary concern, not afterthought.
---
### Pitfall 7: Frontend State Management Assumes Single User
**What goes wrong:**
Angular services cache user context globally, localStorage keys don't include user_id, or the app doesn't clear state on logout. User A logs out, User B logs in on the same browser, and sees User A's cached project data.
**Why it happens:**
Single-user app stored state globally (localStorage, service singletons) with no concept of user identity. After adding multi-user, the frontend team updates login/logout but forgets cached state.
**How to avoid:**
- Clear localStorage/sessionStorage on logout
- Namespace storage keys by user_id: `user_${userId}_graph_state`
- Use request-scoped state, not global singletons where possible
- Angular Guards should verify token validity on route change
- Test user switching: logout → login as different user → verify no leaked state
- Consider using Angular signals for reactive user-scoped state
**Warning signs:**
- User sees previous user's data briefly after login
- Editing as User B modifies User A's project
- "My dashboard shows someone else's stats"
- Graph layouts persist across user accounts
**Phase to address:**
Phase 2 (Authentication & Authorization) — Update frontend state management to be user-aware.
---
### Pitfall 8: Cascading Deletes Destroy Shared Data
**What goes wrong:**
User A shares a project with User B. User A deletes their account. CASCADE DELETE removes the project, wiping out User B's work and edit history. Or worse, the delete fails with foreign key violations because of shared data.
**Why it happens:**
Foreign key CASCADE DELETE seems convenient for cleanup. But in multi-tenant systems with sharing, data ownership is not 1:1. A project might have an owner but also collaborators. Deleting the owner shouldn't delete the project.
**How to avoid:**
- Transfer ownership instead of deleting when user has owned projects
- Use soft deletes (deleted_at timestamp) instead of CASCADE DELETE
- Require explicit "delete all my data" confirmation with warnings
- Archive user data instead of deleting (GDPR compliance)
- For shared projects: transfer to co-owner or mark as archived
- Test deletion scenarios: user with projects, user as collaborator, user as viewer
**Warning signs:**
- User deletion is instant with no warnings
- "I lost my project when the owner left the team"
- Foreign key constraint errors on user deletion
- No way to recover deleted data
**Phase to address:**
Phase 3 (Project Ownership & Sharing) — Design deletion/archival strategy that respects data relationships.
---
### Pitfall 9: Missing Edit Attribution and Audit Trail
**What goes wrong:**
Edit history shows changes but not WHO made them. In a collaborative environment, users can't see "User B edited my requirement" or "Who broke the graph?" The blame/credit information is lost.
**Why it happens:**
Single-user app had no concept of attribution — every edit was "me." The existing `requirement_history` table might have timestamps but no `edited_by_user_id` field.
**How to avoid:**
- Add `created_by_user_id` and `modified_by_user_id` to all tables with user-generated content
- Update history/audit tables to capture user identity
- Display "Last edited by [username] at [time]" in UI
- Allow filtering history by user
- Consider including user's display name in history JSON (denormalized) to survive user deletions
**Warning signs:**
- Edit history shows timestamps but no usernames
- "Who made this change?" has no answer
- Audit log exists but lacks user attribution
- Collaboration features planned but no user tracking
**Phase to address:**
Phase 1 (Database Schema & User Model) — Add attribution columns from the start.
---
### Pitfall 10: Dependency Injection Fails to Scope Tenant Context
**What goes wrong:**
FastAPI dependency that extracts `current_user` is request-scoped, but a service or background task uses a module-level database session. The tenant filter isn't applied, causing data leaks. Or, background tasks receive user objects with secrets instead of just user_id.
**Why it happens:**
Mixing dependency lifespans: per-request (auth), application-lifespan (DB connection pool), and background tasks (out-of-request context). The tenant_id from the JWT is only available in request scope, but business logic runs elsewhere.
**How to avoid:**
- Use per-request DB sessions, not global singletons
- FastAPI dependencies: extract user, pass user_id to service layer
- Background tasks: pass IDs, not objects; rehydrate via dependencies
- Never store user context in global variables
- Service layer methods take user_id as explicit parameter
- Test with multiple concurrent users to catch scope leaks
**Warning signs:**
- Background task sends email to wrong user
- "Current user is None" errors in async tasks
- Data visible to wrong user under concurrent load
- DB session reused across requests
**Phase to address:**
Phase 2 (Authentication & Authorization) — Establish dependency injection patterns that properly scope user context.
---
## Technical Debt Patterns
Shortcuts that seem reasonable but create long-term problems.
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|----------|-------------------|----------------|-----------------|
| Skip invite system, use direct registration | Faster MVP | No growth control, spam signups, unclear user source | Never for v2 — stated requirement |
| Check permissions in frontend only | Less backend code | Trivial to bypass, security theater | Never — always verify backend |
| Use global admin toggle instead of RBAC | Simple boolean check | Can't do granular sharing, rewrite needed for collaboration | Never — RBAC is v2 requirement |
| Store user_id in localStorage instead of JWT | Simpler token structure | Easily spoofed, XSS vulnerability, defeats authentication | Never — security risk |
| Soft-delete users but not their projects | Cleaner user table | Orphaned projects, unclear ownership | Acceptable if projects transferred first |
| SQLite without foreign keys | Easier migration | No referential integrity, manual cleanup, data inconsistency | Acceptable for MVP, fix in v2 |
| Add `user_id` as nullable column | Non-breaking migration | Half the codebase forgets to filter by it | Only during transition, make NOT NULL ASAP |
| Embed role in JWT instead of database | Fewer DB queries | Role changes require re-login, harder to audit | Acceptable if roles rarely change |
## Integration Gotchas
Common mistakes when connecting to external services.
| Integration | Common Mistake | Correct Approach |
|-------------|----------------|------------------|
| Gitea issue integration | Include user's Gitea token in JWT | Store Gitea token per-user in database, encrypt at rest |
| Email (invite system) | Send from user's email address | Use system email (noreply@), track sent_by_user_id separately |
| Frontend SPA | Auth state in component, not service | Centralized auth service with RxJS for reactive state |
| Background tasks | Pass entire user object to task | Pass user_id only, re-fetch in task context |
## Performance Traps
Patterns that work at small scale but fail as usage grows.
| Trap | Symptoms | Prevention | When It Breaks |
|------|----------|------------|----------------|
| N+1 queries loading user for each project | 100s of queries for dashboard | Eager load user with project: `.options(joinedload(Project.owner))` | >50 projects per view |
| Check permissions on every graph node render | Slow graph rendering | Bulk permission check on page load, cache results | >100 nodes |
| No pagination on shared projects list | Slow API calls | Paginate, default limit 50 | User in >20 projects |
| Recalculate full audit history on every edit | Edit lag increases | Incremental history, paginate old history | >1000 history entries |
| JWT validation queries DB for user on every request | DB bottleneck | Cache user in request context after first validation | >10 requests/second |
## Security Mistakes
Domain-specific security issues beyond general web security.
| Mistake | Risk | Prevention |
|---------|------|------------|
| Include user_id in URL path (/projects/user/123/items) | Enumeration attack, user ID leak | Use /projects/me or /projects (filter by JWT user) |
| Expose user emails in project collaborator API | Privacy leak, phishing target list | Return user IDs + display names only |
| Allow project transfer without new owner consent | Unwanted ownership, storage quota bypass | Require new owner to accept transfer |
| Reuse password reset tokens as invite tokens | Token confusion, security model mismatch | Separate token types with different validation |
| No rate limit on invite endpoint | Spam, resource exhaustion | Rate limit: 10 invites/hour per user |
| Allow changing your own role on a project | Privilege escalation | Only project owner can change roles, can't change own |
| DELETE /users/:id accepts any user_id | Account takeover, data destruction | Only allow /users/me, verify JWT user matches |
## UX Pitfalls
Common user experience mistakes in this domain.
| Pitfall | User Impact | Better Approach |
|---------|-------------|-----------------|
| No indication of project owner vs. collaborator | Confusion about edit permissions | Badge/icon showing role, owner prominently displayed |
| Can't tell who made an edit | Blame culture, lost context | "Edited by [name] [time ago]" on every change |
| Force logout on every deployment | Frustration, work loss | 24-hour token expiry, graceful re-auth |
| Invite links expire too quickly (1 day) | Missed invites, support burden | 7-day default, 30-day option |
| No way to leave a shared project | Cluttered project list | "Leave project" button for non-owners |
| Shared projects look same as owned projects | Can't find my projects | Visual distinction, filter by role |
| Delete account is one-click | Accidental deletion, data loss | Require typing account name to confirm |
## "Looks Done But Isn't" Checklist
Things that appear complete but are missing critical pieces.
- [ ] **User registration:** Often missing email verification — verify emails are unique, consider confirmation
- [ ] **Invite system:** Often missing invite revocation — verify admin can cancel pending invites
- [ ] **RBAC implementation:** Often missing inheritance (editor includes viewer) — verify role permissions compose correctly
- [ ] **Multi-user queries:** Often missing tenant filter on JOIN tables — verify cross_pillar_links filtered by project owner
- [ ] **Logout:** Often missing token revocation/blacklist — decide if tokens are valid until expiry or revocable
- [ ] **Project sharing:** Often missing "can they still edit after shared?" test — verify permissions persist correctly
- [ ] **User deletion:** Often missing "what happens to their projects?" flow — verify ownership transfer or archival
- [ ] **Edit history:** Often missing user attribution on old edits — verify migration backfills created_by/modified_by
- [ ] **Role changes:** Often missing "does it take effect immediately?" — verify role updates don't require re-login
- [ ] **Password reset:** Often missing rate limiting — verify can't spam reset emails
- [ ] **Frontend state:** Often missing user-scoped caching — verify localStorage cleared on logout
- [ ] **Audit log:** Often missing sensitive action logging (role changes, deletions) — verify admin actions tracked
## Recovery Strategies
When pitfalls occur despite prevention, how to recover.
| Pitfall | Recovery Cost | Recovery Steps |
|---------|---------------|----------------|
| Deployed without tenant filters | HIGH | Emergency rollback, add filters, test with multiple users, redeploy |
| Migration destroyed data | HIGH | Restore from backup, fix migration script, re-run with copy of production data |
| Role explosion (30+ roles) | MEDIUM | Audit actual role usage, merge similar roles, migrate users to simplified roles |
| Invite codes leaked/enumerated | MEDIUM | Revoke all pending invites, generate new codes, notify legitimate users |
| JWT tokens invalidated prematurely | LOW | Communicate forced re-login, monitor for issues, extend token expiry next time |
| User deleted with active projects | MEDIUM | Soft-delete user instead, transfer projects to team lead, restore from backup if recent |
| Frontend leaks previous user data | LOW | Force cache clear (version localStorage key), deploy fix, notify affected users |
| Cascading delete destroyed shared data | HIGH | Restore from backup, change CASCADE to RESTRICT, add ownership transfer logic |
## Pitfall-to-Phase Mapping
How roadmap phases should address these pitfalls.
| Pitfall | Prevention Phase | Verification |
|---------|------------------|--------------|
| Forgetting tenant_id filters | Phase 1 (Database Schema) | Integration test: User A can't see User B's data |
| SQLite migration failures | Phase 1 (Database Schema) | Test migration on production data copy, verify no data loss |
| Invalid JWT tokens | Phase 2 (Auth & Authorization) | Test old token format handled gracefully |
| Orphaned data without owner | Phase 1 (Database Schema) | Verify all rows have non-null user_id after migration |
| Role explosion | Phase 3 (Project Ownership) | Verify <= 5 roles exist, permission matrix simple |
| Invite system security | Phase 2 (Auth & Authorization) | Security audit: tokens secure, single-use, expire |
| Frontend state leaks | Phase 2 (Auth & Authorization) | Test user switching, verify state cleared |
| Cascading deletes | Phase 3 (Project Ownership) | Test user deletion with owned/shared projects |
| Missing edit attribution | Phase 1 (Database Schema) | Verify edited_by_user_id on all history entries |
| Tenant context leaks | Phase 2 (Auth & Authorization) | Load test with concurrent users, verify isolation |
## Sources
**Multi-Tenancy Patterns:**
- [MergeBoard - Multitenancy with FastAPI, SQLAlchemy and PostgreSQL](https://mergeboard.com/blog/6-multitenancy-fastapi-sqlalchemy-postgresql/)
- [Multi-Tenant Architecture with FastAPI: Design Patterns and Pitfalls](https://medium.com/@koushiksathish3/multi-tenant-architecture-with-fastapi-design-patterns-and-pitfalls-aa3f9e75bf8c)
- [Python FastAPI Postgres SqlAlchemy Row Level Security Multitenancy](https://adityamattos.com/multi-tenancy-in-python-fastapi-and-sqlalchemy-using-postgres-row-level-security)
**SQLite Limitations:**
- [The SQLite Renaissance: Why the World's Most Deployed Database Is Taking Over Production in 2026](https://dev.to/pockit_tools/the-sqlite-renaissance-why-the-worlds-most-deployed-database-is-taking-over-production-in-2026-3jcc)
- [A mere add_foreign_key can wipe out your whole Rails+SQLite production table](https://kyrylo.org/software/2025/09/27/a-mere-add-foreign-key-can-wipe-out-your-whole-rails-sqlite-production-table.html)
- [Fixing ALTER TABLE errors with Flask-Migrate and SQLite](https://blog.miguelgrinberg.com/post/fixing-alter-table-errors-with-flask-migrate-and-sqlite)
**JWT Security:**
- [JWT vs PASETO vs Branca — The Future of Secure Tokens in 2026](https://mojoauth.com/blog/jwt-vs-paseto-vs-branca-the-future-of-secure-tokens-in-2026)
- [Beyond the Secret: The Silent Risks of JWT and Machine Identity](https://medium.com/@instatunnel/beyond-the-secret-the-silent-risks-of-jwt-and-machine-identity-49bea4aa4547)
- [JWT Security Best Practices:Checklist for APIs](https://curity.io/resources/learn/jwt-best-practices/)
**RBAC Implementation:**
- [6 Common Role Based Access Control (RBAC) Implementation Pitfalls](https://idenhaus.com/rbac-implementation-pitfalls/)
- [Role-Based Access Control Best Practices for 2026](https://www.techprescient.com/blogs/role-based-access-control-best-practices/)
- [Common Challenges in Role-Based Access Control Implementation](https://censinet.com/perspectives/common-challenges-role-based-access-control-implementation)
**Invite System Security:**
- [How can I enhance the privacy and security of my invite-only event?](https://help.rsvpify.com/en/articles/5517672-how-can-i-enhance-the-privacy-and-security-of-my-invite-only-event)
**Data Migration:**
- [Top data migration mistakes to avoid in 2026](https://www.kellton.com/kellton-tech-blog/top-data-migration-mistakes-to-avoid)
- [Top 10 Data Migration Risks and How to Avoid Them in 2026](https://medium.com/@kanerika/top-10-data-migration-risks-and-how-to-avoid-them-in-2026-fb5dc93c12f5)
**FastAPI Dependency Injection:**
- [Dependency Injection in FastAPI: 2026 Playbook for Modular, Testable APIs](https://thelinuxcode.com/dependency-injection-in-fastapi-2026-playbook-for-modular-testable-apis/)
- [How to Use Dependency Injection in FastAPI](https://oneuptime.com/blog/post/2026-02-02-fastapi-dependency-injection/view)
**Cascading Deletes:**
- [Why on_delete=models.CASCADE Doesn't Work as Expected in Django's Multi-Tenant Architecture](https://medium.com/@kevinrawal/why-on-delete-models-cascade-doesnt-work-as-expected-in-django-s-multi-tenant-architecture-45aac3faad3d)
- [Foreign Keys vs Performance (Part 3): The CASCADE DELETE Story](https://medium.com/@thyagodoliveiraperez/foreign-keys-vs-performance-part-3-the-cascade-delete-story-aac5cabd843b)
**Permission Inheritance:**
- [SharePoint: Breaking and Managing Permission Inheritance](https://td.usnh.edu/TDClient/60/Portal/KB/ArticleDet?ID=3173)
- [Top 5 Common SharePoint Permissions Mistakes and How to Fix Them](https://lightningtools.com/blog/top-5-common-sharepoint-permissions-mistakes-and-how-to-fix-them/)
**Angular State Management:**
- [Angular State Management for 2025](https://nx.dev/blog/angular-state-management-2025)
- [7 Angular 2026 Predictions That Could Redefine Frontend Architecture](https://dev.to/karol_modelski/7-angular-2026-predictions-that-could-redefine-frontend-architecture-e5n)
---
*Pitfalls research for: req-planner v2.0 multi-tenancy migration*
*Researched: 2026-03-18*