RBAC Hardening & Expansion#60
Conversation
Covers role permissions, admin decorator standardization, contact access filtering, share request workflow, impersonation system, auth audit logging, and report/export agent isolation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Impersonation: add header-to-session validation, write policy, admin endpoint blocking, max 4hr/8 extension limits - Permission matrix: add contact deletion, tasks, email, calendar, notes - Public endpoint audit: enumerate all @public endpoints individually - AdminOnly: document all 3 existing patterns (not just 2) - Share requests: add agency scope enforcement + 30-day expiration - Audit events: separate security.* namespace from entity audit.* - Role display: include client_portal in display map - Dashboard: note existing filtering, scope as verification Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical fixes: - Impersonation: use Interceptor (not middleware) for correct execution order - Impersonation: add @BypassImpersonation() for control endpoints to prevent deadlock High fixes: - Public endpoint audit: correct route paths, add missing endpoints - Contact RBAC: gate contact-linked data (trips/bookings/payments/notes) - Notes/calendar: note ownership check requirements - Dashboard: explicitly fix GET /dashboard/stats agency-wide leak - Platform settings: enumerate specific Stripe/settings endpoints - Audit logging: new security_audit_logs table, separate from entity audit Medium fixes: - Share requests: explicitly reuse ContactSharesService/ContactAccessService - Notifications: add contact.share_* event handlers + notification types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Impersonation: use Guard (not Interceptor) registered between JwtAuthGuard and RolesGuard — correct NestJS execution order so role swap happens before admin checks 2. Platform settings: correct route paths to actual /agencies/:agencyId/* Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Unify all references to ImpersonationGuard (was still saying Interceptor in one place) - Fix method names to match actual codebase: create(), canAccessSensitiveData(), send() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When IMAP credentials become invalid, the system now: - Detects auth failures via ImapFlow error properties and sets lastSyncError='IMAP_AUTH_FAILED' on the account - Excludes auth-failed accounts from background sync dispatch - Returns HTTP 502 with machine-readable code to the frontend - Shows inline auth error banner in inbox with link to update credentials - Clears error state when user updates credentials, re-enabling sync - Handles auth failures consistently across all 5 IMAP entry points - Maps sentinel to friendly text in profile tab Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidates role metadata + AdminRoleGuard into a single reusable decorator. JwtAuthGuard is global so it is intentionally excluded. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces three inconsistent admin guard patterns across 10 controllers: - Pattern 1 (UseGuards(AdminGuard)): commission, users, api-credentials, trips, reference-data, aerodatabox controllers - Pattern 2 (@roles('admin') + RolesGuard): suppliers, loyalty-programs, enrichment controllers - Pattern 3 (UseGuards(JwtAuthGuard, AdminRoleGuard)): automation controller JwtAuthGuard is global so it was redundant in Pattern 3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds ROLE_DISPLAY_NAMES constant and getRoleDisplayName() helper. Updates users-table getRoleBadge() to use the helper so the 'user' role now displays as 'Agent' and 'client_portal' displays as 'Client'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dmin CRITICAL FIX: @public() was applied at class-level on AppController, making ALL endpoints public including debug-sentry. This meant a method-level @adminonly() could not override it because JwtAuthGuard short-circuits on @public() before AdminRoleGuard can run, leaving request.user undefined. Fix: - Remove class-level @public() - Add method-level @public() to getHealth() and getInfo() - Add @adminonly() to debugSentry() - Remove production env check — admin auth is sufficient Trip share token write endpoints all have @Throttle() — verified OK. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All 5 platform settings / Stripe Connect methods now require admin role: getAgencySettings, updateAgencySettings, startOnboarding, getAccountStatus, getDashboardLink. Existing agencyId !== auth.agencyId cross-agency checks are preserved — they remain necessary to prevent cross-agency access even among admins. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(Task 9) Creates the security_audit_logs table with indexes on event, agency_id, user_id, and created_at for efficient querying. Registers migration at idx 162 in the Drizzle journal. Exports the securityAuditLogs schema from the database package index. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…or agents - GET /contacts/:id/trips: filter to agent-accessible trips for non-owners without full access - GET /contacts/:id/bookings: same ownership filter using b.trip.id - GET /contacts/:id/payment-transactions: block entirely for non-owners without full access - Add _accessLevel: 'basic' | 'full' to ContactResponseDto for frontend badge support - Update applyAccessControl() and applyAccessControlToMany() to include _accessLevel in responses - notes.controller.ts verifyEntityAccess() only checks agencyId for contact-linked notes (documented gap, not fixed here as it only gates who can create notes, not reads sensitive data) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tration (Task 10) Introduces SecurityAuditModule with a service that listens for all security.* events via EventEmitter2 wildcard and persists them to security_audit_logs. Errors are caught and logged rather than thrown so a failed audit write never disrupts the primary request path. Module is registered in AppModule imports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…agents - Create ContactShareRequestButton component: POST /contacts/:id/share-requests, shows "Request Sent" state after success - Add useUser() and isAdmin check to contact detail page - Show amber "Limited View" badge + Request Access button in contact header when _accessLevel === 'basic' and user is not admin Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ship
- dashboard.controller.ts: pass full auth (not just agencyId) to getStats()
- dashboard.service.ts: getStats() now accepts AuthContext
- Agents: trips scoped to accessible trip IDs via tripAccessService
- Agents: contacts scoped to owned contacts (ownerId = auth.userId)
- Revenue left agency-scoped (payment_transactions has no direct trip_id column)
- Early return { 0, 0, 0, 0 } when agent has no accessible trips
- getOverview() already correctly uses tripAccessService — unchanged
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…events - Create SecurityExceptionFilter catching 401/403 and emitting security events via EventEmitter2 - Register as APP_FILTER in AppModule (after SentryGlobalFilter) - Inject EventEmitter2 into UsersService and emit security.role_changed when role changes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add migration, Drizzle schema, schema index export, and journal registration for the contact_share_requests table (idx 163). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ogic Create ResolveShareRequestDto and ContactShareRequestsService. Service handles: stale request expiry (30d), duplicate prevention, approve/deny workflow that calls ContactSharesService.create(), and emits contact.share_* + security.share_* events. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…actsModule Register controller (POST /:id/share-requests, GET /share-requests/pending, PATCH /share-requests/:id) and ContactShareRequestsService in ContactsModule. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add 'contact_share' to NotificationCategory union, NOTIFICATION_CATEGORY_VALUES, CategoryPreferences, and default channels map (platform only) - Add handlers for contact.share_requested (notify owner), contact.share_approved (notify requester), contact.share_denied (notify requester) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Migration idx 164 adds table for tracking admin impersonation sessions with indexes for active session lookup, target user, and agency filtering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Global guard inserted between JwtAuthGuard and RolesGuard - Session lock: admins with active sessions must include X-Impersonate-User-Id header - Bypass decorator exempts impersonation control endpoints - Builds full AuthContext from target user (not spread from admin) - Stores originalAdmin on request for audit attribution Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- startSession: validates target (not admin, same agency, active), creates 30min session - extendSession: extends by 30min, enforces 4hr max total duration - endSession: ends session, emits security + notification events - getStatus: returns active session info with target user name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Endpoints: - POST /admin/impersonate/:userId — start session (@adminonly) - POST /admin/impersonate/extend — extend by 30min (@BypassImpersonation) - DELETE /admin/impersonate — end session (@BypassImpersonation) - GET /admin/impersonate/status — check active session (@BypassImpersonation) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Notifies the target agent when an admin ends an impersonation session, including admin name and session duration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Create useImpersonation hook: status check, 30s polling when active, start/extend/end methods with localStorage key management - Create ImpersonationBanner: fixed amber overlay with countdown timer, Extend and Exit buttons with toast feedback - Mount ImpersonationBanner inside AuthProvider in providers.tsx - Inject X-Impersonate-User-Id header in api.ts getAuthHeaders() - Add Impersonate action to UsersTable (admin-only, non-admin targets) - Wire impersonation into UsersSettingsPage via onImpersonate prop Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… leak 1. use-impersonation.ts: Clear localStorage 'impersonate-user-id' when checkStatus() returns active:false or errors. Prevents 401s on all requests after natural session expiry. 2. dashboard.service.ts: Suppress totalRevenue for non-admin agents in legacy getStats() endpoint. Revenue had no trip-scoping (no trip_id on payment_transactions). Agents use getOverview() KPIs instead. Fixes both blocking issues from Codex post-implementation review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ImpersonationGuard queries the table on every guarded request. During deploys, Railway can start the new code before migrations complete, causing ~245 PostgresError "relation does not exist" errors. Fix: cache a table-existence check. If the table doesn't exist yet, skip impersonation checks silently (session lock) or reject explicitly (active impersonation header). Cache resets on app restart after migration runs. Fixes #58 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rlap 1. Redirect to /dashboard instead of reloading the current page (users tab) when starting impersonation — agents can't access admin pages. 2. Add spacer div so the fixed banner pushes content down instead of overlapping the sticky navigation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (61)
📝 WalkthroughWalkthroughThis PR implements a comprehensive RBAC hardening and expansion system, introducing: a standardized Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant UI as Frontend UI
participant API as API Server
participant DB as Database
participant Events as Event System
Admin->>UI: Click "Impersonate User"
UI->>API: POST /admin/impersonate/{userId}
API->>DB: Create impersonation_session
API->>Events: Emit security.impersonation_started
Events->>DB: Log security audit event
API-->>UI: Session details + expiresAt
UI->>UI: Store impersonate-user-id in localStorage
UI->>UI: Redirect to /dashboard
Note over UI: ImpersonationBanner appears<br/>with countdown timer
Admin->>UI: Click "Extend" (before expiry)
UI->>API: POST /admin/impersonate/extend
API->>DB: Refresh expiresAt (max 4h from creation)
API->>API: Fetch latest session status
API-->>UI: Updated expiresAt
UI->>UI: Reset countdown timer
UI->>UI: Show success toast
Admin->>UI: Click "Exit Impersonation"
UI->>API: DELETE /admin/impersonate
API->>DB: Set endedAt + endReason='manual'
API->>Events: Emit security.impersonation_ended
Events->>DB: Log audit + notify target user
API-->>UI: Confirmation
UI->>UI: Clear localStorage + reload page
UI->>UI: Return to normal admin view
sequenceDiagram
participant Agent as Agent User
participant UI as Frontend UI
participant API as API Server
participant DB as Database
participant ContactOwner as Contact Owner
participant Events as Event System
Agent->>UI: View contact with basic access
UI->>UI: Display "Limited View" badge
UI->>UI: Show "Request Access" button
Agent->>UI: Click "Request Access"
UI->>API: POST /contacts/{id}/share-requests
API->>DB: Check contact exists in agency
API->>DB: Verify not already owned by requester
API->>DB: Check no existing pending request
API->>DB: Create contact_share_requests record<br/>(status='pending')
API->>Events: Emit contact.share_requested
Events->>DB: Create audit log
API-->>UI: Share request created
UI->>UI: Button → "Request Sent" (disabled)
UI->>UI: Show success toast
ContactOwner->>UI: View pending share requests
UI->>API: GET /share-requests/pending
API->>DB: Fetch pending requests for owner
API-->>UI: List of pending requests
ContactOwner->>UI: Click "Approve" request
UI->>API: PATCH /share-requests/{id}
API->>DB: Validate request pending status
API->>DB: Update status='approved'
API->>DB: Set resolvedAt + resolvedBy
API->>API: Create ContactShare (full access)
API->>Events: Emit contact.share_approved
Events->>DB: Log audit event
API-->>UI: Request approved
UI->>UI: Show success notification
Agent->>UI: Refresh contact view
UI->>API: GET /contacts/{id}
API->>DB: Check ContactShare record exists
API->>API: Set _accessLevel='full'
API-->>UI: Contact with full access
UI->>UI: Display full contact details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
@AdminOnly()decorator — consolidated 3 inconsistent admin guard patterns across 10 controllers@Public()on AppController, gateddebug-sentrybehind admingetStats()scoped to agent's trips/contacts, revenue suppressed for non-adminssecurity_audit_logstable +SecurityAuditService+ exception filter for login_failed/access_denied/role_changed eventscontact_share_requeststable, create/approve/deny flow with notifications, 30-day expiryimpersonation_sessionstable,ImpersonationGuardwith session lock + full context swap, 30min sessions (4hr max), frontend banner with countdown, audit trail + agent notificationDatabase Migrations
20260320120000_create_security_audit_logs(idx 162)20260320120100_create_contact_share_requests(idx 163)20260320120200_create_impersonation_sessions(idx 164)Test plan
Spec
docs/superpowers/specs/2026-03-20-rbac-hardening-design.md🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Enhancements