RBAC on key routes, otp to admin email on account creation

docs/agent/research.md

@@ -1,74 +1,292 @@
-Great question! Based on my research, here's what you need to know:
+I’ll review how user management works in this codebase (signup, auth, profile, roles, etc.), then give you a structured “grade” with what’s good, what’s weak, and concrete priority fixes.
 
-## Yes, the Old Connection String Format is Deprecated! ❌
+First I need to scan the project (code + docs) for user/account/profile flows. I’ll set up a small todo list for this review, then search through `src` and relevant docs, and come back with findings and recommendations.
 
-The old `postgresql://postgres:YOUR_PASSWORD@db.xxxxx.supabase.co:5432/postgres` direct connection format still works but has important limitations - it now uses IPv6 only by default, and the old PgBouncer pooler was deprecated in January 2024.
+I’ll scan the code and docs for anything related to users, auth, accounts, and profiles, then summarize the end‑to‑end flow and grade it with concrete fixes you should do immediately. Let me search the repo first.
 
-## How to Find Your Current Connection Strings ✅
 
-Supabase now provides three types of connection strings, all accessible from your dashboard by clicking the "Connect" button at the top of the page:
+# High‑Level Summary: User Management Flow
 
-### Step-by-Step to Get Your Connection Strings:
+From the code you’ve got a **pretty complete user lifecycle**:
 
-1. **Go to your Supabase Dashboard**
-2. **Click the "Connect" button** at the top of your project page (you should see it prominently displayed)
-3. You'll see three connection string options:
+- **Identity & Auth**
+  - Supabase Auth is the **source of truth** for authentication (email/password, tokens).
+  - Backend uses Supabase JWT (Bearer token) in `Authorization` header.
+  - `get_current_user` (in `api/deps.py`) validates token via `supabase_auth.get_user`, then loads the local `User` row.
+  - `get_current_active_user` additionally requires `user.is_active == True`.
 
-### 1. **Direct Connection** (IPv6 only)
-```
-postgresql://postgres:[YOUR-PASSWORD]@db.xxxxx.supabase.co:5432/postgres
-```
-- ⚠️ Requires IPv6 support
-- Good for: VMs, long-running containers with IPv6
+- **Account Creation**
 
-### 2. **Session Mode Pooler** (Recommended for FastAPI)
-```
-postgresql://postgres.xxxxx:[YOUR-PASSWORD]@aws-0-[region].pooler.supabase.com:5432/postgres
-```
-- ✅ Supports both IPv4 and IPv6
-- Good for: Persistent servers, traditional backends
-- **Use this for your FastAPI backend!**
+  There are **two main creation paths**:
 
-### 3. **Transaction Mode Pooler** (For serverless)
-```
-postgresql://postgres.xxxxx:[YOUR-PASSWORD]@aws-0-[region].pooler.supabase.com:6543/postgres
-```
-- ✅ Supports both IPv4 and IPv6
-- Good for: Edge functions, serverless, Lambda
-- ⚠️ Does not support prepared statements
+  1. **Self‑Registration** (`POST /auth/register`)
+     - Calls `supabase_auth.sign_up` with `email`, `password`, and metadata (`first_name`, `last_name`, `phone`, `full_name`).
+     - Creates a local `User` mapped to `auth_user.id` with:
+       - `role = "platform_admin"` (**self‑registered users become platform admins**).
+       - `status = "invited"` (default from schema).
+       - `is_active = True`.
+     - Logs an audit event `register` and returns a token + basic user info.
 
-## Which One Should You Use for FastAPI?
+  2. **Invitation Flow** (`InvitationService.accept_invitation`)
+     - Admin (client/contractor/platform) invites users (not fully shown, but `accept_invitation` is).
+     - `accept_invitation`:
+       - Validates the invitation token.
+       - Ensures no existing user with the same email.
+       - Calls `supabase_auth.sign_up` and creates Supabase auth user.
+       - Creates local `User` with:
+         - `id = auth_user.id`.
+         - `email` from invitation.
+         - `name = first + last`.
+         - `phone` from acceptance or invitation.
+         - `role = invitation.invited_role`.
+         - `client_id` / `contractor_id` from invitation.
+         - `status = 'active'`, `is_active = True`.
+       - Marks invitation as accepted, commits, returns `access_token` + user details.
 
-**Use the Session Mode Pooler** (port 5432 with pooler URL):
+- **Login** (`POST /auth/login`)
+  - Calls `supabase_auth.sign_in` to validate credentials and get `access_token`.
+  - Loads local `User` by `auth_user.id`.
+  - If no local user → `404 "User profile not found"`.
+  - If `is_active == False` → audit failed login + `403 "Account is inactive"`.
+  - Otherwise, audit success + returns token + basic user info.
 
-```env
-DATABASE_URL=postgresql://postgres.YOUR_PROJECT_REF:[YOUR-PASSWORD]@aws-0-[YOUR-REGION].pooler.supabase.com:5432/postgres
-```
+- **Profile Model & Building**
 
-## Visual Guide to Find It:
+  - `User` model (business profile) holds:
+    - Identity & contact: `name`, `email`, `phone`, `phone_alternate`, `display_name`, `id_number`.
+    - Org links: `client_id` or `contractor_id`.
+    - Role & status: `role`, `status`, `is_active`.
+    - Health & safety JSONB fields: `health_info`, `ppe_sizes`, plus emergency contacts.
+    - Location & geo: `current_location_*`, lat/long, updated timestamp.
+    - `additional_metadata` JSONB.
+  - Self‑service profile API (`api/v1/profile.py`):
+    - `GET /profile/me` returns:
+      - `basic_info` (from `BasicProfileResponse.from_orm(current_user)`).
+      - `health_info` (parsed from JSONB, with sensible defaults for missing fields).
+      - `ppe_sizes`.
+      - `location` details.
+      - Completion status: `ProfileService.calculate_profile_completion`.
+      - Counts: financial accounts, documents, asset assignments.
 
-Since I saw your Supabase dashboard earlier, here's exactly what to do:
+- **Profile Permissions & Editing**
 
-1. **Look at the TOP of your Supabase project page**
-2. You should see a **"Connect"** button (it's usually near the project name)
-3. Click it
-4. A modal will pop up with connection string options
-5. Choose **"Session"** tab
-6. Copy the entire connection string
-7. Replace `[YOUR-PASSWORD]` with your actual database password
+  - `ProfileService.check_edit_permission` encodes **hierarchical access**:
+    - Self can edit own profile (subject to field‑level checks elsewhere).
+    - `platform_admin` can edit anyone.
+    - `client_admin`/`contractor_admin` can edit users in their org.
+    - `project_manager` / `dispatcher` can edit `field_agent`s in shared projects.
+  - `ProfileService.get_profile_permissions` turns this into fine‑grained booleans:
+    - `can_edit_basic_info`, `can_edit_health_info`, `can_edit_ppe_sizes`,
+      `can_edit_location`, `can_edit_financial_accounts`, `can_upload_documents`,
+      `can_edit_role`, `can_edit_status`, `can_assign_assets`, and corresponding view permissions.
 
-## If You Can't Find the Connect Button:
+- **Roles & Authorization**
 
-Alternative path:
-1. Go to **Project Settings** (gear icon bottom left)
-2. Click **"Database"** in the sidebar
-3. Scroll down to **"Connection string"** section
-4. You'll see the three options there
+  - `core/permissions.py` defines an `AppRole` enum and `ROLE_PERMISSIONS` map:
+    - Roles: `PLATFORM_ADMIN`, `CLIENT_ADMIN`, `CONTRACTOR_ADMIN`, `PROJECT_MANAGER`, `DISPATCHER`, `FIELD_AGENT`, `SALES_AGENT`, `SALES_MANAGER`.
+    - Each has a list of permission strings (e.g. `manage_projects`, `invite_users`, `view_team_performance`).
+    - `has_permission(role, permission)` supports generic checks.
+  - User update & admin actions (`UserService`):
+    - `check_update_permission`:
+      - `platform_admin` can change anyone.
+      - Org admins can update users in their org but cannot change org or promote to `platform_admin`.
+      - Self cannot use admin endpoints to change own role/status/org; must use profile APIs.
+    - `update_user`: handles basic fields, role, status, org changes, with audit‑style `changes` tracking.
+    - `change_user_status`: updates status + syncs `is_active` for `suspended`/`active`.
+    - `change_user_role`: validates and applies role changes.
+  - User admin endpoints (`api/v1/users.py` excerpts):
+    - `POST /users/{user_id}/status` → uses `UserService.change_user_status`, logs `status_change`.
+    - `POST /users/{user_id}/role` → uses `UserService.change_user_role`, logs `role_change`.
 
-## Important Notes:
+- **Statuses**
+  - Documented statuses: `invited`, `pending_setup`, `active`, `suspended`.
+  - `change_user_status` uses `status` string + maps `suspended` and `active` to `is_active False/True`.
 
-- The new Supavisor connection strings include your project reference in the username (e.g., `postgres.YOUR_PROJECT_REF`)
-- The password is your **database password**, not your Supabase account password
-- If you don't remember your database password, you can reset it in Database Settings
+---
 
-Would you like me to help you configure this connection string in your FastAPI app once you get it?
+# What You Did Well
+
+- **Clear separation of auth vs business profile**
+  - Supabase handles auth (passwords, tokens).
+  - Local `User` table holds business & authorization data.
+  - `get_current_user` cleanly bridges the two.
+
+- **Invitation‑based onboarding is solid**
+  - Token‑based invitations handled by `InvitationService`.
+  - Proper token validation, duplicate user check, and creation of both auth user + local profile.
+  - Good error handling and rollback on failure.
+
+- **Strong admin & profile permission model**
+  - `UserService.check_update_permission` and `ProfileService.check_edit_permission` encode a realistic hierarchy:
+    - Platform admin → org admin → manager/dispatcher → self.
+  - Separation between **self profile editing** and **admin updates** is good design.
+  - Field‑level permission booleans in `ProfilePermissions` are explicit and easily testable.
+
+- **Audit logging & rate limiting**
+  - Auth endpoints log audit events for `register` and `login` (both success and failure).
+  - Status/role changes log with old/new values and reason.
+  - Rate limits on `/auth/register` and `/auth/login` reduce brute force abuse.
+
+- **Role definitions & permissions table**
+  - `AppRole` enum and `ROLE_PERMISSIONS` map give a centralized RBAC definition.
+  - `has_permission` helper is generic and allows future route‑level enforceable checks.
+
+- **Good data modeling for field agents**
+  - Health info and PPE sizes as JSONB with nice mapping to Pydantic response models.
+  - Location fields + `current_location_updated_at` support live ops / dispatch use cases.
+  - `additional_metadata` JSONB gives you flexibility without schema churn.
+
+---
+
+# Weaknesses / Problems
+
+## 1. Self‑registration as `platform_admin`
+
+- `POST /auth/register` creates every self‑registered user as:
+  - `role = "platform_admin"`
+  - `is_active = True`
+  - `status = "invited"` (but effectively usable)
+- This is **dangerous in production**. Anyone hitting `/auth/register` becomes the highest‑privilege user.
+
+**Impact**  
+- Extremely high risk: if `/auth/register` is exposed, you’ve essentially got open platform‑admin account creation.
+
+## 2. Status semantics are confusing / inconsistent
+
+- For self‑registration:
+  - Status is set to `"invited"` while the user is immediately active and logged in.
+- For invitation acceptance:
+  - Status is set to `"active"`.
+- `login` uses only `is_active`, not `status`, so:
+  - A user with `status="invited"` but `is_active=True` can still log in.
+- The `pending_setup` state is referenced in docs but I don’t see it enforced in this flow.
+
+**Impact**  
+- Harder to reason about real lifecycle states (“invited but not registered”, “registered but not set up profile”, etc).
+- Tooling and analytics relying on `status` may be incorrect.
+
+## 3. RBAC implementation is only partially used
+
+- `core/permissions.py` defines `ROLE_PERMISSIONS` and `has_permission`, but:
+  - I don’t see route‑level checks using `has_permission("role", "permission")`.
+  - Instead, some endpoints hand‑code role logic (e.g., profile and user service).
+- This creates **duplication** and **risk of drift** between the central RBAC map and actual guards.
+
+## 4. Suspended vs active inconsistencies at the auth provider
+
+- `change_user_status` sets local `is_active` based on `status`, but:
+  - Supabase Auth account remains active; they can still get a valid token with correct password.
+  - You block them at the backend (`get_current_active_user`), which is good, but:
+    - You still incur auth load and leak some information (“account exists, but inactive”).
+
+**Impact**  
+- Functionally safe at API level but not enforced at the auth provider.
+- Not necessarily a bug, but security‑grade hardening could require aligning Supabase user’s `blocked`/`app_metadata` flags.
+
+## 5. Inconsistent or legacy fields
+
+- In `accept_invitation`, you set `activated_at=datetime.now(timezone.utc)`, yet the `User` model comments say:
+  - `invited_at` and `activated_at` were removed and tracked in `user_invitations`.
+- If the DB column really is gone, this would fail at runtime; if it’s still there, comments are outdated and confusing.
+
+## 6. Missing UX‑driven checks around profile completeness
+
+- You compute `completion_status` in `ProfileService.calculate_profile_completion`, but:
+  - I haven’t seen it enforced anywhere (e.g., blocking certain actions until profile is complete).
+- Users might stay in a partially configured state while still getting full access, depending on role.
+
+## 7. Supabase helper edge cases
+
+- `SupabaseAuth.sign_out` ignores the `access_token` parameter and just calls `self.client.auth.sign_out()` — works but:
+  - For multi‑session scenarios, you might want to specifically revoke that token.
+- `update_user` sets session using the same access token as both access and refresh token (`set_session(access_token, access_token)`), which may not align perfectly with Supabase’s model.
+  - Could be fine but is brittle if Supabase changes expectations.
+
+---
+
+# What Needs Changing Right Away (High Priority)
+
+1. **Stop self‑registration as platform admin**
+   - Change `register` flow so:
+     - Either:
+       - Self‑registration is disabled entirely in production, and all user creation goes through invitation / admin flows.
+       - Or: Self‑registered users get a **limited role** (e.g., `client_admin` for a new tenant, or `field_agent` for a specific app) with a safer onboarding path.
+     - Explicitly control who can become `platform_admin` via internal processes only.
+
+2. **Clarify and normalize `status` & `is_active`**
+   - Decide on a consistent lifecycle like:
+     - `invited` → `pending_setup` → `active` → `suspended`.
+   - Enforce:
+     - `invited` and `pending_setup` users should either:
+       - Not be able to log in, or
+       - Be restricted to a setup flow only.
+   - Ensure `register` sets `status` appropriately (likely `active` or `pending_setup`, not `invited`).
+
+3. **Wire route‑level checks to RBAC table**
+   - For endpoints that represent specific actions (e.g., manage users, manage projects, assign tickets):
+     - Use `has_permission(current_user.role, "<permission_name>")` instead of hard‑coding role checks everywhere.
+   - This will:
+     - Keep behavior consistent with `ROLE_PERMISSIONS`.
+     - Make it easier to adjust permissions centrally.
+
+4. **Confirm `activated_at` / legacy fields**
+   - Either:
+     - Remove `activated_at=...` from `accept_invitation` if the column no longer exists.
+     - Or: Update `User` model + comments to reflect actual DB schema.
+   - Inconsistency here can cause subtle migration/runtime issues.
+
+---
+
+# Medium‑Priority Improvements
+
+- **Supabase sync on suspension**
+  - Optionally, when suspending a user:
+    - Update their Supabase user (e.g., `user_metadata` or `app_metadata`) to mark them as suspended, or use any “blocked” mechanism available.
+  - This prevents them from even getting a token.
+
+- **Enforce profile completeness for certain roles**
+  - For roles like `field_agent`:
+    - Require certain profile sections (PPE, emergency contacts, basic info) to be complete before:
+      - Accepting field tasks.
+      - Being assigned to risky operations.
+  - Gate important endpoints behind `completion_status` checks.
+
+- **Lockout / abuse protection**
+  - You have rate limiting, which is good.
+  - Consider additional measures:
+    - Optional account lockout after N failed logins.
+    - Tracking login IP / device in metadata.
+
+- **Better use of `ProfilePermissions`**
+  - Ensure all profile update endpoints (not just `GET /me`) consistently respect `ProfilePermissions` booleans to prevent accidental over‑editing by org roles.
+
+---
+
+# Low‑Priority / Nice to Have
+
+- Use `ROLE_PERMISSIONS` not just for “system actions” but also for UI hints, so the frontend can hide/show features based on the same central map.
+- Add tests for:
+  - `UserService.check_update_permission` across roles + org boundaries.
+  - `ProfileService.get_profile_permissions` permutations (self vs admin vs manager).
+  - Status transitions and how they affect `is_active` and login behavior.
+
+---
+
+# “Grade” of Your Work
+
+If I had to assign letter grades per area:
+
+- **Overall architecture (auth vs profile separation, use of services)**: **A**
+- **RBAC design (roles, permissions, hierarchy)**: **B+**
+  - Solid structure, but not fully applied via `has_permission` yet.
+- **Onboarding flows (invitation, profile creation)**: **A‑**
+  - Invitation flow is strong; self‑registration is the main issue.
+- **Security hardening (roles for self‑registration, status semantics, suspension)**: **C**
+  - The platform_admin self‑registration is a serious concern and should be fixed before production exposure.
+
+---
+
+If you’d like, next step I can propose **concrete code changes** (files + specific modifications) to:
+- Fix the self‑registration role.
+- Normalize statuses and `is_active`.
+- Introduce `has_permission` checks for a couple of key endpoints as a pattern.

src/app/api/v1/auth.py

@@ -20,77 +20,85 @@ logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/auth", tags=["Authentication"])
 
 
-@router.post("/register", response_model=TokenResponse, status_code=status.HTTP_201_CREATED)
-@limiter.limit("5/hour")  # 5 registrations per hour per IP
+@router.post("/register", response_model=MessageResponse, status_code=status.HTTP_200_OK)
+@limiter.limit("3/hour")  # 3 registration attempts per hour per IP (reduced for security)
 async def register(request: Request, response: Response, user_data: UserCreate, db: Session = Depends(get_db)):
     """
-    Register a new user account via Supabase Auth
+    🔒 SECURE Platform Admin Registration - Requires OTP Verification
     
-    - **email**: User's email address (must be unique)
-    - **password**: Strong password (min 8 chars, 1 digit, 1 uppercase)
-    - **first_name**: User's first name
-    - **last_name**: User's last name
-    - **phone**: Optional phone number
+    **SECURITY MODEL:**
+    - Self-registration is DISABLED for regular users (must use invitation flow)
+    - Platform admin creation requires OTP verification sent to authorized email
+    - Only lewiskimaru01@gmail.com can receive platform admin OTPs
+    
+    **Process:**
+    1. Submit registration request
+    2. OTP sent to lewiskimaru01@gmail.com
+    3. Use OTP to verify via /otp/verify-public endpoint
+    4. After OTP verification, account is created as platform_admin
+    
+    **Note:** Regular users (field agents, managers, etc.) must be invited by admins.
     """
+    # Security check: Only allow platform admin registration for authorized email
+    PLATFORM_ADMIN_EMAIL = "lewiskimaru01@gmail.com"
+    
+    # Check if this is an attempt to create a platform admin account
+    # Platform admin accounts can only be created via OTP verification
     try:
-        # Register user with Supabase Auth
-        auth_response = await supabase_auth.sign_up(
-            email=user_data.email,
-            password=user_data.password,
-            user_metadata={
+        # Check if user already exists
+        existing_user = db.query(User).filter(User.email == user_data.email).first()
+        if existing_user:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Email already registered"
+            )
+        
+        # For security, we don't create the account immediately
+        # Instead, we send OTP to the authorized platform admin email
+        # and store the registration request temporarily
+        
+        # Import OTP service
+        from app.services.otp_service import OTPService
+        otp_service = OTPService.get_instance()
+        
+        # Generate OTP and send to authorized admin email
+        otp_result = await otp_service.send_otp(
+            identifier=user_data.email,  # Use registration email as identifier
+            channel="email",  # Force email delivery
+            recipient=PLATFORM_ADMIN_EMAIL,  # Always send to authorized admin
+            purpose=f"Platform Admin Registration: {user_data.email}"
+        )
+        
+        # Store registration data in OTP metadata for later use
+        # When OTP is verified, we'll complete the registration
+        await otp_service.store_registration_data(
+            identifier=user_data.email,
+            data={
+                "email": user_data.email,
+                "password": user_data.password,  # Temporarily stored, will be hashed by Supabase
                 "first_name": user_data.first_name,
                 "last_name": user_data.last_name,
                 "phone": user_data.phone,
-                "full_name": f"{user_data.first_name} {user_data.last_name}"
+                "role": "platform_admin",
+                "registration_type": "platform_admin_otp"
             }
         )
         
-        auth_user = auth_response["user"]
-        session = auth_response["session"]
-        
-        # Create user record in our users table
-        # Note: Self-registered users are created as platform_admin (no org link required)
-        # They can be assigned to client/contractor later by an admin
-        full_name = f"{user_data.first_name} {user_data.last_name}"
-        new_user = User(
-            id=auth_user.id,  # Use Supabase Auth user ID
-            email=user_data.email,
-            name=full_name,  # Store full name in 'name' field (matches schema)
-            phone=user_data.phone,
-            is_active=True,
-            role="platform_admin",  # Self-registered users start as platform_admin
-            status="invited"  # Default status from schema
-        )
-        
-        db.add(new_user)
-        db.commit()
-        db.refresh(new_user)
-        
-        # Audit log
-        AuditService.log_action(
+        # Audit log - registration request (not completed yet)
+        AuditService.log_auth_event(
             db=db,
             action='register',
-            entity_type='user',
-            entity_id=str(new_user.id),
-            description=f"New user registered: {user_data.email}",
-            user=new_user,
+            user_email=user_data.email,
+            success=False,  # Not completed yet
             request=request,
-            additional_metadata={'role': new_user.role}
+            reason=f"OTP verification required - sent to {PLATFORM_ADMIN_EMAIL}"
         )
         
-        logger.info(f"User registered successfully: {user_data.email}")
+        logger.info(f"Platform admin registration OTP sent for: {user_data.email}")
         
         return {
-            "access_token": session.access_token,
-            "token_type": "bearer",
-            "user": {
-                "id": str(new_user.id),
-                "email": new_user.email,
-                "first_name": new_user.first_name,
-                "last_name": new_user.last_name,
-                "full_name": new_user.full_name,
-                "is_active": new_user.is_active
-            }
+            "message": f"Registration request received. OTP verification code sent to {PLATFORM_ADMIN_EMAIL}. "
+                      f"Please check your email and verify using /otp/verify-public endpoint with identifier='{user_data.email}'"
         }
         
     except Exception as e:
@@ -447,6 +455,156 @@ async def reset_password(
         raise
 
 
+@router.post("/complete-registration", response_model=TokenResponse, status_code=status.HTTP_201_CREATED)
+@limiter.limit("5/hour")  # 5 completion attempts per hour per IP
+async def complete_registration(
+    request: Request,
+    response: Response,
+    email: str,
+    otp_code: str,
+    db: Session = Depends(get_db)
+):
+    """
+    Complete platform admin registration after OTP verification
+    
+    **Process:**
+    1. User submits registration via /auth/register
+    2. OTP sent to lewiskimaru01@gmail.com
+    3. User calls this endpoint with OTP code to complete registration
+    
+    **Parameters:**
+    - **email**: Email address used during registration
+    - **otp_code**: 6-digit OTP code sent to admin email
+    
+    **Security:** OTP must be verified before account creation
+    """
+    try:
+        # Import OTP service
+        from app.services.otp_service import OTPService
+        otp_service = OTPService.get_instance()
+        
+        # Verify OTP first
+        verification_result = otp_service.verify_otp(
+            email=email,
+            phone=None,
+            code=otp_code,
+            purpose="default",  # Registration OTPs use default purpose
+            db=db
+        )
+        
+        if not verification_result['verified']:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail=f"Invalid or expired OTP. {verification_result.get('attempts_remaining', 0)} attempts remaining."
+            )
+        
+        # Retrieve stored registration data
+        registration_data = await otp_service.get_registration_data(email)
+        
+        if not registration_data:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Registration data not found or expired. Please start registration process again."
+            )
+        
+        # Verify this is a platform admin registration
+        if registration_data.get('registration_type') != 'platform_admin_otp':
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Invalid registration type"
+            )
+        
+        # Check if user already exists (double-check)
+        existing_user = db.query(User).filter(User.email == email).first()
+        if existing_user:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Email already registered"
+            )
+        
+        # Now create the Supabase auth user
+        auth_response = await supabase_auth.sign_up(
+            email=registration_data['email'],
+            password=registration_data['password'],
+            user_metadata={
+                "first_name": registration_data['first_name'],
+                "last_name": registration_data['last_name'],
+                "phone": registration_data.get('phone'),
+                "full_name": f"{registration_data['first_name']} {registration_data['last_name']}"
+            }
+        )
+        
+        auth_user = auth_response["user"]
+        session = auth_response["session"]
+        
+        # Create user record in our database
+        full_name = f"{registration_data['first_name']} {registration_data['last_name']}"
+        new_user = User(
+            id=auth_user.id,
+            email=registration_data['email'],
+            name=full_name,
+            phone=registration_data.get('phone'),
+            is_active=True,
+            role="platform_admin",  # Verified platform admin
+            status="active"  # Active after OTP verification
+        )
+        
+        db.add(new_user)
+        db.commit()
+        db.refresh(new_user)
+        
+        # Audit log - successful registration
+        AuditService.log_action(
+            db=db,
+            action='create',
+            entity_type='user',
+            entity_id=str(new_user.id),
+            description=f"Platform admin account created (OTP verified): {registration_data['email']}",
+            user=new_user,
+            request=request,
+            additional_metadata={
+                'role': new_user.role,
+                'verification_method': 'otp_email'
+            }
+        )
+        
+        logger.info(f"Platform admin account created successfully: {registration_data['email']}")
+        
+        return {
+            "access_token": session.access_token,
+            "token_type": "bearer",
+            "user": {
+                "id": str(new_user.id),
+                "email": new_user.email,
+                "first_name": new_user.first_name,
+                "last_name": new_user.last_name,
+                "full_name": new_user.full_name,
+                "is_active": new_user.is_active
+            }
+        }
+        
+    except HTTPException:
+        raise
+    except Exception as e:
+        logger.error(f"Complete registration error: {str(e)}")
+        db.rollback()
+        
+        # Audit failed completion
+        AuditService.log_auth_event(
+            db=db,
+            action='register',
+            user_email=email,
+            success=False,
+            request=request,
+            reason=f"Registration completion failed: {str(e)}"
+        )
+        
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail=f"Failed to complete registration: {str(e)}"
+        )
+
+
 @router.post("/logout", response_model=MessageResponse, status_code=status.HTTP_200_OK)
 async def logout(
     request: Request,

src/app/api/v1/invitations.py

@@ -22,6 +22,7 @@ from app.schemas.invitation import (
 )
 from app.services.invitation_service import InvitationService
 from app.schemas.auth import TokenResponse
+from app.core.permissions import require_permission
 import logging
 import math
 
@@ -33,6 +34,7 @@ invitation_service = InvitationService()
 
 
 @router.post("", response_model=InvitationResponse, status_code=status.HTTP_201_CREATED)
+@require_permission("invite_users")
 async def create_invitation(
     invitation_data: InvitationCreate,
     db: Session = Depends(get_db),
@@ -63,6 +65,7 @@ async def create_invitation(
 
 
 @router.get("", response_model=InvitationListResponse)
+@require_permission("invite_users")
 async def list_invitations(
     skip: int = Query(0, ge=0, description="Number of records to skip"),
     limit: int = Query(50, ge=1, le=100, description="Maximum records to return"),
@@ -177,6 +180,7 @@ async def get_invitation(
 
 
 @router.post("/{invitation_id}/resend", response_model=InvitationResponse)
+@require_permission("invite_users")
 async def resend_invitation(
     invitation_id: UUID,
     resend_data: InvitationResend,
@@ -201,6 +205,7 @@ async def resend_invitation(
 
 
 @router.delete("/{invitation_id}", status_code=status.HTTP_204_NO_CONTENT)
+@require_permission("invite_users")
 async def cancel_invitation(
     invitation_id: UUID,
     db: Session = Depends(get_db),

src/app/api/v1/users.py

@@ -17,6 +17,7 @@ from app.schemas.user import (
 )
 from app.services.user_service import UserService
 from app.services.audit_service import AuditService
+from app.core.permissions import require_permission, require_role, AppRole
 import logging
 
 logger = logging.getLogger(__name__)
@@ -24,6 +25,7 @@ router = APIRouter(prefix="/users", tags=["Users"])
 
 
 @router.get("", response_model=UserListResponse)
+@require_permission("view_users")
 async def list_users(
     skip: int = Query(0, ge=0, description="Number of records to skip"),
     limit: int = Query(50, ge=1, le=100, description="Maximum records to return"),
@@ -156,6 +158,7 @@ async def get_user(
 # ============================================
 
 @router.put("/{user_id}", response_model=UserResponse)
+@require_permission("manage_org_users")
 async def update_user(
     user_id: UUID,
     data: UserUpdateAdmin,
@@ -210,6 +213,7 @@ async def update_user(
 
 
 @router.post("/{user_id}/status", response_model=UserResponse)
+@require_permission("manage_org_users")
 async def change_user_status(
     user_id: UUID,
     data: UserStatusUpdate,
@@ -261,6 +265,7 @@ async def change_user_status(
 
 
 @router.post("/{user_id}/role", response_model=UserResponse)
+@require_permission("manage_org_users")
 async def change_user_role(
     user_id: UUID,
     data: UserRoleUpdate,
@@ -313,6 +318,7 @@ async def change_user_role(
 
 
 @router.post("/{user_id}/organization", response_model=UserResponse)
+@require_role(AppRole.PLATFORM_ADMIN)  # Only platform admins can move users between orgs
 async def change_user_organization(
     user_id: UUID,
     data: UserOrganizationUpdate,
@@ -374,6 +380,7 @@ async def change_user_organization(
 # ============================================
 
 @router.post("/{user_id}/deactivate", response_model=UserResponse)
+@require_permission("manage_org_users")
 async def deactivate_user(
     user_id: UUID,
     reason: Optional[str] = Query(None, description="Reason for deactivation"),
@@ -420,6 +427,7 @@ async def deactivate_user(
 
 
 @router.post("/{user_id}/activate", response_model=UserResponse)
+@require_permission("manage_org_users")
 async def activate_user(
     user_id: UUID,
     request: Request,
@@ -468,6 +476,7 @@ async def activate_user(
 # ============================================
 
 @router.delete("/{user_id}", status_code=status.HTTP_204_NO_CONTENT)
+@require_role(AppRole.PLATFORM_ADMIN)  # Only platform admins can delete users
 async def delete_user(
     user_id: UUID,
     reason: Optional[str] = Query(None, description="Reason for deletion"),

src/app/core/permissions.py

@@ -2,7 +2,12 @@
 Role-Based Access Control (RBAC) Definitions
 """
 from enum import Enum
-from typing import List, Dict
+from typing import List, Dict, Callable
+from functools import wraps
+from fastapi import HTTPException, status, Depends
+import logging
+
+logger = logging.getLogger(__name__)
 
 
 class AppRole(str, Enum):
@@ -21,44 +26,79 @@ class AppRole(str, Enum):
 ROLE_PERMISSIONS: Dict[AppRole, List[str]] = {
     AppRole.PLATFORM_ADMIN: ["*"],  # All permissions
     AppRole.CLIENT_ADMIN: [
-        "manage_projects",
+        # User Management
+        "view_users",
         "invite_users",
+        "manage_org_users",  # Can manage users in their organization
+        # Project Management
+        "manage_projects",
         "view_reports",
-        "manage_sales_orders"
+        # Sales
+        "manage_sales_orders",
+        # Documents
+        "view_documents",
+        "upload_documents"
     ],
     AppRole.CONTRACTOR_ADMIN: [
+        # User Management
+        "view_users",
+        "invite_users",
+        "manage_org_users",  # Can manage users in their organization
         "manage_agents",
+        # Project Management
         "accept_projects",
         "view_payroll",
-        "manage_inventory"
+        # Inventory
+        "manage_inventory",
+        # Documents
+        "view_documents",
+        "upload_documents"
     ],
     AppRole.PROJECT_MANAGER: [
+        # User Management
+        "view_users",
+        "view_team_performance",
+        # Project Management
         "create_tickets",
         "manage_inventory",
         "approve_expenses",
-        "view_team_performance"
+        # Documents
+        "view_documents",
+        "upload_documents"
     ],
     AppRole.DISPATCHER: [
+        # User Management
+        "view_users",
+        "view_agent_locations",
+        # Task Management
         "assign_tickets",
         "issue_equipment",
         "approve_expenses",
-        "view_agent_locations"
+        # Documents
+        "view_documents"
     ],
     AppRole.FIELD_AGENT: [
+        # Task Management
         "view_tickets",
         "update_ticket_status",
         "log_expenses",
-        "upload_photos"
+        # Documents
+        "upload_photos",
+        "view_own_documents"
     ],
     AppRole.SALES_AGENT: [
+        # Sales
         "create_orders",
         "view_customers",
         "view_own_performance"
     ],
     AppRole.SALES_MANAGER: [
+        # User Management
+        "view_users",
+        "manage_sales_agents",
+        # Sales & Reports
         "view_team_performance",
-        "export_reports",
-        "manage_sales_agents"
+        "export_reports"
     ]
 }
 
@@ -66,9 +106,157 @@ ROLE_PERMISSIONS: Dict[AppRole, List[str]] = {
 def has_permission(role: AppRole, permission: str) -> bool:
     """
     Check if a role has a specific permission
+    
+    Args:
+        role: User's role
+        permission: Permission to check (e.g., 'manage_projects', 'invite_users')
+        
+    Returns:
+        True if role has permission, False otherwise
     """
     if role == AppRole.PLATFORM_ADMIN:
         return True
     
     role_perms = ROLE_PERMISSIONS.get(role, [])
     return permission in role_perms or "*" in role_perms
+
+
+def require_permission(permission: str):
+    """
+    Decorator to enforce permission checks on route handlers
+    
+    Usage:
+        @router.get("/users")
+        @require_permission("manage_users")
+        async def list_users(current_user: User = Depends(get_current_active_user)):
+            ...
+    
+    Args:
+        permission: Required permission string
+        
+    Raises:
+        HTTPException 403: If user doesn't have required permission
+    """
+    def decorator(func: Callable):
+        @wraps(func)
+        async def wrapper(*args, **kwargs):
+            # Extract current_user from kwargs (injected by Depends(get_current_active_user))
+            current_user = kwargs.get('current_user')
+            
+            if not current_user:
+                logger.error(f"Permission check failed: No current_user in kwargs for {func.__name__}")
+                raise HTTPException(
+                    status_code=status.HTTP_401_UNAUTHORIZED,
+                    detail="Authentication required"
+                )
+            
+            # Check if user has permission
+            user_role = current_user.role
+            if not has_permission(AppRole(user_role), permission):
+                logger.warning(
+                    f"Permission denied: User {current_user.email} (role={user_role}) "
+                    f"attempted {func.__name__} requiring '{permission}'"
+                )
+                raise HTTPException(
+                    status_code=status.HTTP_403_FORBIDDEN,
+                    detail=f"Insufficient permissions. Required: {permission}"
+                )
+            
+            # User has permission, execute the route handler
+            return await func(*args, **kwargs)
+        
+        return wrapper
+    return decorator
+
+
+def require_any_permission(*permissions: str):
+    """
+    Decorator to enforce that user has at least one of the specified permissions
+    
+    Usage:
+        @router.get("/reports")
+        @require_any_permission("view_reports", "export_reports")
+        async def get_reports(current_user: User = Depends(get_current_active_user)):
+            ...
+    
+    Args:
+        *permissions: List of acceptable permissions (user needs at least one)
+        
+    Raises:
+        HTTPException 403: If user doesn't have any of the required permissions
+    """
+    def decorator(func: Callable):
+        @wraps(func)
+        async def wrapper(*args, **kwargs):
+            current_user = kwargs.get('current_user')
+            
+            if not current_user:
+                logger.error(f"Permission check failed: No current_user in kwargs for {func.__name__}")
+                raise HTTPException(
+                    status_code=status.HTTP_401_UNAUTHORIZED,
+                    detail="Authentication required"
+                )
+            
+            # Check if user has any of the permissions
+            user_role = AppRole(current_user.role)
+            has_any = any(has_permission(user_role, perm) for perm in permissions)
+            
+            if not has_any:
+                logger.warning(
+                    f"Permission denied: User {current_user.email} (role={current_user.role}) "
+                    f"attempted {func.__name__} requiring one of: {', '.join(permissions)}"
+                )
+                raise HTTPException(
+                    status_code=status.HTTP_403_FORBIDDEN,
+                    detail=f"Insufficient permissions. Required one of: {', '.join(permissions)}"
+                )
+            
+            return await func(*args, **kwargs)
+        
+        return wrapper
+    return decorator
+
+
+def require_role(*roles: AppRole):
+    """
+    Decorator to enforce specific role requirements (simpler than permission-based)
+    
+    Usage:
+        @router.post("/admin/settings")
+        @require_role(AppRole.PLATFORM_ADMIN)
+        async def update_settings(current_user: User = Depends(get_current_active_user)):
+            ...
+    
+    Args:
+        *roles: List of acceptable roles
+        
+    Raises:
+        HTTPException 403: If user doesn't have one of the required roles
+    """
+    def decorator(func: Callable):
+        @wraps(func)
+        async def wrapper(*args, **kwargs):
+            current_user = kwargs.get('current_user')
+            
+            if not current_user:
+                logger.error(f"Role check failed: No current_user in kwargs for {func.__name__}")
+                raise HTTPException(
+                    status_code=status.HTTP_401_UNAUTHORIZED,
+                    detail="Authentication required"
+                )
+            
+            # Check if user has one of the required roles
+            if current_user.role not in [role.value for role in roles]:
+                logger.warning(
+                    f"Role check failed: User {current_user.email} (role={current_user.role}) "
+                    f"attempted {func.__name__} requiring role: {', '.join([r.value for r in roles])}"
+                )
+                raise HTTPException(
+                    status_code=status.HTTP_403_FORBIDDEN,
+                    detail=f"Insufficient privileges. Required role: {', '.join([r.value for r in roles])}"
+                )
+            
+            return await func(*args, **kwargs)
+        
+        return wrapper
+    return decorator

src/app/services/invitation_service.py

@@ -187,8 +187,8 @@ class InvitationService:
                 role=invitation.invited_role,
                 status='active',  # User is active after accepting invitation
                 client_id=invitation.client_id,
-                contractor_id=invitation.contractor_id,
-                activated_at=datetime.now(timezone.utc)
+                contractor_id=invitation.contractor_id
+                # Note: activated_at removed - tracked in user_invitations.accepted_at
             )
             
             db.add(new_user)

src/app/services/otp_service.py

@@ -453,6 +453,77 @@ SwiftOps Team"""
             detail="SMS delivery not yet implemented. Please use email or WhatsApp."
         )
     
+    async def store_registration_data(
+        self,
+        identifier: str,
+        data: Dict[str, Any],
+        ttl: int = 3600  # 1 hour TTL for registration data
+    ) -> None:
+        """
+        Store registration data temporarily (used for OTP-verified registration)
+        
+        Args:
+            identifier: Unique identifier (email address)
+            data: Registration data to store
+            ttl: Time to live in seconds (default 1 hour)
+        """
+        key = f"registration:{identifier}"
+        value = json.dumps(data)
+        
+        if self.storage_type == 'redis' and self.redis_client:
+            try:
+                self.redis_client.setex(key, ttl, value)
+                logger.info(f"Stored registration data for {identifier}")
+            except Exception as e:
+                logger.error(f"Failed to store registration data: {e}")
+                # Fallback to memory
+                self._memory_store[key] = {
+                    'value': value,
+                    'expires_at': datetime.now(timezone.utc) + timedelta(seconds=ttl)
+                }
+        else:
+            # In-memory storage
+            self._memory_store[key] = {
+                'value': value,
+                'expires_at': datetime.now(timezone.utc) + timedelta(seconds=ttl)
+            }
+    
+    async def get_registration_data(self, identifier: str) -> Optional[Dict[str, Any]]:
+        """
+        Retrieve and delete registration data after OTP verification
+        
+        Args:
+            identifier: Unique identifier (email address)
+            
+        Returns:
+            Registration data dict or None if not found/expired
+        """
+        key = f"registration:{identifier}"
+        
+        if self.storage_type == 'redis' and self.redis_client:
+            try:
+                value = self.redis_client.get(key)
+                if value:
+                    # Delete after retrieval (one-time use)
+                    self.redis_client.delete(key)
+                    return json.loads(value)
+                return None
+            except Exception as e:
+                logger.error(f"Failed to retrieve registration data: {e}")
+                return None
+        else:
+            # In-memory storage
+            data = self._memory_store.get(key)
+            if data:
+                # Check if expired
+                if datetime.now(timezone.utc) > data['expires_at']:
+                    del self._memory_store[key]
+                    return None
+                # Delete after retrieval
+                del self._memory_store[key]
+                return json.loads(data['value'])
+            return None
+    
     @staticmethod
     def require_otp_for_purpose(purpose: str) -> bool:
         """Check if OTP is required for this purpose"""