From ebfcd60c71c1b57041ba51152ee5f2a7d66d57f6 Mon Sep 17 00:00:00 2001 From: michael Date: Sun, 22 Feb 2026 07:42:42 -0600 Subject: [PATCH] Fix campaign visibility bug for unassigned users after agency reassignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unassigned (no agency) non-admin users previously saw ALL campaigns due to a truthiness check that treated None agency_id as "no filter". This was a security bug — they should see NO campaigns and be blocked from creating them. Backend: Add _NO_AGENCY sentinel to distinguish "no filter" from "no agency", add early-returns at all 5 list/analytics endpoints, fix _check_campaign_access to explicitly reject unassigned users, and block campaign creation with 403. Frontend: Add isUnassigned boolean to UserContext, show informational empty state on Campaigns view, and reinforce readOnly for defense-in-depth. Co-Authored-By: Claude Opus 4.6 --- backend/app/api/routes.py | 31 ++++++++++++++++++++++++++----- frontend/App.tsx | 17 +++++++++++++++-- frontend/contexts/UserContext.tsx | 5 +++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/backend/app/api/routes.py b/backend/app/api/routes.py index 9b30007..f137053 100755 --- a/backend/app/api/routes.py +++ b/backend/app/api/routes.py @@ -49,27 +49,36 @@ from app.services.pdf_service import pdf_service router = APIRouter() +# Sentinel: distinguishes "no filter (admins see all)" from "user has no agency" +_NO_AGENCY = object() + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- -def _resolve_agency_filter(current_user: User, agency_id_param: Optional[uuid.UUID] = None) -> Optional[uuid.UUID]: +def _resolve_agency_filter(current_user: User, agency_id_param: Optional[uuid.UUID] = None): """Determine which agency_id to filter by based on user role and query param. - - super_admin / oversight_admin: use agency_id_param if provided, else None (all) - - agency_admin / basic_user: forced to current_user.agency_id + Returns: + uuid.UUID – filter to a specific agency + None – no filter (admins see all) + _NO_AGENCY – user is unassigned; callers should return empty results """ if current_user.role in ("super_admin", "oversight_admin"): return agency_id_param # None means "all" - return current_user.agency_id # may be None (user not assigned yet) + if current_user.agency_id is None: + return _NO_AGENCY + return current_user.agency_id def _check_campaign_access(current_user: User, campaign) -> None: """Raise 404 if the user's role restricts them and the campaign doesn't belong to their agency.""" if current_user.role in ("super_admin", "oversight_admin"): return - if current_user.agency_id and campaign.agency_id == current_user.agency_id: + if current_user.agency_id is None: + raise HTTPException(status_code=404, detail="Campaign not found") + if campaign.agency_id == current_user.agency_id: return raise HTTPException(status_code=404, detail="Campaign not found") @@ -107,6 +116,8 @@ async def list_campaigns( ): """List campaigns, filtered by the user's role and optional agency filter.""" effective_agency_id = _resolve_agency_filter(current_user, agency_id) + if effective_agency_id is _NO_AGENCY: + return [] repo = CampaignRepository(db) campaigns_with_counts = await repo.get_with_proof_counts(agency_id=effective_agency_id) @@ -135,6 +146,8 @@ async def create_campaign( current_user: User = Depends(require_write_access), ): """Create a new campaign. Blocked for oversight_admin.""" + if current_user.agency_id is None and current_user.role not in ("super_admin", "oversight_admin"): + raise HTTPException(status_code=403, detail="You must be assigned to an agency before creating campaigns.") repo = CampaignRepository(db) # Check if campaign name already exists @@ -464,6 +477,8 @@ async def list_flagged_items( ): """List flagged items, filtered by role.""" effective_agency_id = _resolve_agency_filter(current_user, agency_id) + if effective_agency_id is _NO_AGENCY: + return [] audit_repo = AuditRepository(db) flagged_items = await audit_repo.get_flagged_items(agency_id=effective_agency_id, limit=limit, offset=offset) @@ -494,6 +509,8 @@ async def list_resolved_items( ): """List resolved items, filtered by role.""" effective_agency_id = _resolve_agency_filter(current_user, agency_id) + if effective_agency_id is _NO_AGENCY: + return [] audit_repo = AuditRepository(db) resolved_items = await audit_repo.get_resolved_items(agency_id=effective_agency_id, limit=limit, offset=offset) @@ -525,6 +542,8 @@ async def list_error_items( ): """List error items, filtered by role.""" effective_agency_id = _resolve_agency_filter(current_user, agency_id) + if effective_agency_id is _NO_AGENCY: + return [] audit_repo = AuditRepository(db) error_items = await audit_repo.get_error_items(agency_id=effective_agency_id, limit=limit, offset=offset) @@ -556,6 +575,8 @@ async def get_analytics( ): """Get analytics data, filtered by role.""" effective_agency_id = _resolve_agency_filter(current_user, agency_id) + if effective_agency_id is _NO_AGENCY: + return AnalyticsResponse(total_reviews=0, passed=0, failed=0, errors=0, legal_review=0) repo = CampaignRepository(db) analytics = await repo.get_analytics(agency_id=effective_agency_id) return AnalyticsResponse(**analytics) diff --git a/frontend/App.tsx b/frontend/App.tsx index ebd95dc..f10c60a 100755 --- a/frontend/App.tsx +++ b/frontend/App.tsx @@ -74,7 +74,7 @@ const App: React.FC = () => { const AppContent: React.FC<{ msalInstance: any }> = ({ msalInstance }) => { const isAuthenticated = true; // We're inside the authenticated boundary - const { user, isLoading: isUserLoading, canWrite, canSeeAnalytics, canSeeAuditing, canSeeKnowledgeBase, canSeeSettings, canSeeUserManagement, canEditSettings, isSuperAdmin, isOversightAdmin } = useUser(); + const { user, isLoading: isUserLoading, canWrite, canSeeAnalytics, canSeeAuditing, canSeeKnowledgeBase, canSeeSettings, canSeeUserManagement, canEditSettings, isSuperAdmin, isOversightAdmin, isUnassigned } = useUser(); // Get initial state from URL const initialUrlState = parseUrlState(); @@ -840,6 +840,19 @@ const AppContent: React.FC<{ msalInstance: any }> = ({ msalInstance }) => { case 'CopyGenAI': return ; case 'Campaigns': + if (isUnassigned) { + return ( +
+
+
🏢
+

No Agency Assigned

+

+ You are not currently assigned to an agency. Please contact your administrator to be assigned before you can view or create campaigns. +

+
+
+ ); + } return = ({ msalInstance }) => { onResolveSubmit={handleResolveSubmit} flaggedItems={flaggedItems} resolvedItems={resolvedItems} - readOnly={readOnly} + readOnly={readOnly || isUnassigned} />; case 'WIP Reviewer': return ; diff --git a/frontend/contexts/UserContext.tsx b/frontend/contexts/UserContext.tsx index 4b51410..b132c04 100644 --- a/frontend/contexts/UserContext.tsx +++ b/frontend/contexts/UserContext.tsx @@ -15,6 +15,8 @@ interface UserContextValue { canSeeSettings: boolean; canSeeUserManagement: boolean; canEditSettings: boolean; + /** True when the user exists but has no agency and is not an admin */ + isUnassigned: boolean; /** Re-fetch user from backend (e.g. after role change) */ refresh: () => Promise; } @@ -31,6 +33,7 @@ const UserContext = createContext({ canSeeSettings: false, canSeeUserManagement: false, canEditSettings: false, + isUnassigned: false, refresh: async () => {}, }); @@ -77,6 +80,8 @@ export const UserProvider: React.FC<{ children: React.ReactNode }> = ({ children canSeeSettings: role === 'super_admin' || role === 'oversight_admin' || role === 'agency_admin', canSeeUserManagement: role === 'super_admin', canEditSettings: role === 'super_admin', + isUnassigned: user != null && user.agencyId == null + && role !== 'super_admin' && role !== 'oversight_admin', refresh: fetchUser, };