feat: user management — admin create, password reset, archive/delete, quick invite
Phase 1: must_change_password enforcement + change password endpoint/page Phase 2: Admin user creation (M365-style) with temp password Phase 3: Password reset (self-service forgot + admin-triggered) Phase 4: User archive (soft delete) + hard delete with precheck Phase 5: Quick invite from admin Users page Also fixes: - Auto-create subscription for accounts missing one - Hard delete precheck ignores sole-member personal accounts - Seed script patches tree nodes for validation compliance Migrations: 031 (must_change_password), 032 (password_reset_tokens), 033 (user soft delete) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
|
||||
> **Purpose:** This file documents bugs, fixes, and gotchas encountered during development.
|
||||
> **For Claude Code:** Read this file at the start of each session to avoid repeating past mistakes.
|
||||
> **Last Updated:** February 1, 2026
|
||||
> **Last Updated:** February 12, 2026
|
||||
|
||||
---
|
||||
|
||||
@@ -724,6 +724,172 @@ python -m scripts.seed_trees ...
|
||||
|
||||
---
|
||||
|
||||
## SQLAlchemy / Database
|
||||
|
||||
### Two Foreign Keys to Same Table Require `foreign_keys=` ⚠️ CRITICAL
|
||||
**Problem:** SQLAlchemy raises `AmbiguousForeignKeysError` when a model has two ForeignKey columns pointing to the same table (e.g., `deleted_by` and `author_id` both referencing `users.id`).
|
||||
|
||||
**Cause:** SQLAlchemy can't figure out which FK to use for each relationship.
|
||||
|
||||
**Solution:** Add `foreign_keys=` parameter on BOTH relationship sides:
|
||||
```python
|
||||
# In User model
|
||||
deleted_by: Mapped[Optional[uuid.UUID]] = mapped_column(
|
||||
UUID(as_uuid=True), ForeignKey("users.id"), nullable=True
|
||||
)
|
||||
|
||||
# In Tree model with two user FKs
|
||||
author: Mapped["User"] = relationship("User", foreign_keys=[author_id])
|
||||
deleted_by_user: Mapped[Optional["User"]] = relationship("User", foreign_keys=[deleted_by])
|
||||
```
|
||||
|
||||
**Files affected:** Any model with multiple FK references to the same table (User, Tree)
|
||||
|
||||
---
|
||||
|
||||
### Circular FK Between Tables Requires Nullable FK
|
||||
**Problem:** `Account.owner_id → users.id` and `User.account_id → accounts.id` creates a circular dependency. `CREATE TABLE` fails because neither table can be created first.
|
||||
|
||||
**Cause:** Both tables reference each other, so neither can exist before the other.
|
||||
|
||||
**Solution:** Make one FK nullable and set it after both records exist:
|
||||
```python
|
||||
# Create Account first (owner_id=NULL)
|
||||
account = Account(name="My Account")
|
||||
db.add(account)
|
||||
await db.flush()
|
||||
|
||||
# Create User with account_id
|
||||
user = User(account_id=account.id, ...)
|
||||
db.add(user)
|
||||
await db.flush()
|
||||
|
||||
# Now set owner_id
|
||||
account.owner_id = user.id
|
||||
await db.commit()
|
||||
```
|
||||
|
||||
**Files affected:** `backend/app/models/account.py`, registration endpoint
|
||||
|
||||
---
|
||||
|
||||
## Authentication / Security
|
||||
|
||||
### Backend Enforcement of `must_change_password` via Dependency Injection
|
||||
**Problem:** Need to force users to change their password before accessing any endpoint, but some endpoints (like the change-password endpoint itself) must be exempt.
|
||||
|
||||
**Solution:** Add a check in `get_current_active_user` dependency with a route allowlist:
|
||||
```python
|
||||
async def get_current_active_user(request: Request, ...):
|
||||
user = ... # existing auth logic
|
||||
|
||||
# Check must_change_password (with route allowlist)
|
||||
if user.must_change_password:
|
||||
allowed = ["/auth/password/change", "/auth/logout", "/auth/me"]
|
||||
if not any(request.url.path.endswith(p) for p in allowed):
|
||||
raise HTTPException(403, detail="password_change_required")
|
||||
|
||||
return user
|
||||
```
|
||||
|
||||
**Key insight:** This requires adding `request: Request` parameter to the dependency. The frontend checks the 403 detail string and redirects to `/change-password`.
|
||||
|
||||
**Files affected:** `backend/app/api/deps.py`, `frontend/src/components/layout/ProtectedRoute.tsx`
|
||||
|
||||
---
|
||||
|
||||
### Password Reset Tokens: DB-Backed Single-Use via Hashed JTI
|
||||
**Pattern:** Password reset tokens use JWT for transport but are tracked in the database for single-use enforcement.
|
||||
|
||||
**How it works:**
|
||||
1. Generate JWT with `type: "password_reset"`, `jti: uuid4()`, `exp: 30min`
|
||||
2. Store `SHA-256(jti)` in `password_reset_tokens` table
|
||||
3. On reset: decode JWT → hash jti → look up in DB → check `used_at IS NULL`
|
||||
4. After use: set `used_at = now()` to prevent reuse
|
||||
|
||||
**Why not just JWT?** JWTs are stateless — you can't revoke them. DB tracking ensures each token is used exactly once.
|
||||
|
||||
**Files affected:** `backend/app/models/password_reset_token.py`, `backend/app/core/security.py`
|
||||
|
||||
---
|
||||
|
||||
### Anti-Enumeration on Forgot Password Endpoint
|
||||
**Pattern:** The `POST /auth/password/forgot` endpoint always returns the same generic success message, regardless of whether the email exists.
|
||||
|
||||
**Why:** If the endpoint returned "email not found" for non-existent users, an attacker could use it to enumerate valid email addresses.
|
||||
|
||||
```python
|
||||
# Always return success, even if email doesn't exist
|
||||
return {"message": "If an account exists with that email, a reset link has been sent."}
|
||||
```
|
||||
|
||||
**Files affected:** `backend/app/api/endpoints/auth.py`
|
||||
|
||||
---
|
||||
|
||||
## Seed Scripts
|
||||
|
||||
### Seed Tree Data Missing Required Validation Fields
|
||||
**Problem:** All 18 seed trees fail to create with validation errors: "Action nodes must have a non-empty action" and "Solution nodes must have a non-empty solution".
|
||||
|
||||
**Cause:** The tree validation (added after the seed scripts were written) requires:
|
||||
- Action nodes: `action` field (not just `description`)
|
||||
- Solution nodes: `solution` field (not just `description`)
|
||||
- Decision nodes with children: at least 2 branches
|
||||
|
||||
The seed data uses `title` and `description` but not the specific `action`/`solution` fields.
|
||||
|
||||
**Solution:** Add a recursive `_fix_node_fields()` function in the seeder that patches nodes before sending to the API:
|
||||
```python
|
||||
def _fix_node_fields(node):
|
||||
if node["type"] == "action" and not node.get("action"):
|
||||
node["action"] = node.get("title") or node.get("description") or "Action"
|
||||
elif node["type"] == "solution" and not node.get("solution"):
|
||||
node["solution"] = node.get("title") or node.get("description") or "Solution"
|
||||
elif node["type"] == "decision":
|
||||
children = node.get("children", [])
|
||||
if len(children) == 1: # Add fallback branch
|
||||
children.append({"id": "..._fallback", "type": "solution", ...})
|
||||
for child in node.get("children", []):
|
||||
_fix_node_fields(child)
|
||||
```
|
||||
|
||||
**Files affected:** `backend/scripts/seed_trees_v2.py`
|
||||
|
||||
---
|
||||
|
||||
## Admin Features
|
||||
|
||||
### Two-Step Hard Delete Pattern
|
||||
**Pattern:** Hard-deleting a user requires two API calls — a precheck followed by the actual delete.
|
||||
|
||||
1. `GET /admin/users/{id}/hard-delete-check` → returns `{can_delete: bool, blockers: {owned_accounts: 3, sessions: 12, ...}}`
|
||||
2. `DELETE /admin/users/{id}/hard-delete` → only succeeds if user is archived AND precheck passes
|
||||
|
||||
**Pre-conditions enforced:**
|
||||
- User must be archived (`deleted_at IS NOT NULL`) before hard delete
|
||||
- User must have no blockers (owned accounts, authored trees, sessions, audit logs, invite codes)
|
||||
- Cannot delete yourself or other super admins
|
||||
- Audit log entry created BEFORE the delete (since user won't exist after)
|
||||
|
||||
**Files affected:** `backend/app/api/endpoints/admin.py`
|
||||
|
||||
---
|
||||
|
||||
### Admin User Creation with Temp Password (M365-Style)
|
||||
**Pattern:** Admin creates a user → gets a one-time temp password → user must change on first login.
|
||||
|
||||
**Key design decisions:**
|
||||
- Temp password is generated server-side (`secrets`-based, 16 chars, guaranteed complexity)
|
||||
- Temp password is returned once in the response, never stored as plaintext
|
||||
- `must_change_password=True` is set on the user, enforced at the dependency level
|
||||
- Welcome email with temp password is best-effort (never blocks user creation)
|
||||
- Two modes: "existing account" (join with role) or "personal" (new account created)
|
||||
|
||||
**Files affected:** `backend/app/api/endpoints/admin.py`, `backend/app/core/security.py`
|
||||
|
||||
---
|
||||
|
||||
## Adding New Lessons
|
||||
|
||||
When you encounter and fix a bug, add it here with:
|
||||
|
||||
Reference in New Issue
Block a user