# Code Review Report: Feature 25 — Multi-Tenant Identity Model

This report performs a deep-dive audit of the Hub's user and group identity models in `user.py`, focusing on **Account Security**, **Group Isolation**, and **Preference Consistency**.

---

## 🏗️ 12-Factor App Compliance Audit

| Factor | Status | Observation |
| :--- | :--- | :--- |
| **III. Config** | ✅ **Success** | **Role Sanitization**: The default role of `user` (Line 29) correctly follows the principle of least privilege, ensuring that new OIDC-registered accounts do not gain administrative access by default. |
| **VI. Processes** | 🟡 **Warning** | **Flattened Preference Blob**: The `preferences` column (Line 34) is a single `JSON` field. While flexible, this pattern is susceptible to "Last-Writer-Wins" data loss if a user has multiple concurrent browser tabs open updating different preference subsections (like "Custom CSS" vs "LLM Providers"). |

---

## 🔍 File-by-File Diagnostic

### 1. `app/db/models/user.py`
The source of truth for user identity, credentials, and organizational memberships.

> [!CAUTION]
> **Relational Inconsistency Hazard (Groups)**
> Line 30: `group_id = Column(String, ForeignKey('groups.id'), nullable=True)`
> There is no explicit `ondelete` constraint. If a `Group` is deleted via a direct SQL query or a low-level DB tool, the `User` table will contain invalid `group_id` strings (dangling pointers).
> 
> **Recommendation**: Set `ondelete="SET NULL"` for the `group_id` foreign key and implement an application-level "Default Group" (e.g., `ungrouped`) to ensure all users always belong to a valid organizational policy.

**Identified Problems**:
*   **Lack of Password Sensitivity Markers**: The `password_hash` field (Line 27) should be omitted from any generic `model_dump` using Pydantic to prevent accidental leakage in user-profile endpoints.
*   **Weak Constraint Audit**: While `email` (Line 25) should likely be unique for the "Local Login" flow, it is not marked as `unique=True`. This could lead to account duplication and credential confusion in certain Hub configurations.

---

## 🛠️ Summary Recommendations

1.  **Harden Foreign Keys**: Update the `group_id` relationship with `ondelete="SET NULL"` to maintain database referential integrity during bulk organizational restructuring.
2.  **Enforce Uniqueness**: Explicitly mark the `email` column as `unique=True` to prevent the creation of duplicate accounts that share the same identity during OIDC resolution.
3.  **Atomic Prefs Updates**: Implement a specialized partial-update method for the `preferences` blob (using `jsonb_set` for Postgres or similar logic) to handle concurrent sub-preference changes safely.

---

**This concludes Feature 25. I have persisted this report to `/app/docs/reviews/feature_review_identity_models.md`. Shall I proceed to audit the Asset and Node models?**