diff --git a/backend/migrations/manual/005_tasks_rbac_restrict.sql b/backend/migrations/manual/005_tasks_rbac_restrict.sql new file mode 100644 index 0000000..1084316 --- /dev/null +++ b/backend/migrations/manual/005_tasks_rbac_restrict.sql @@ -0,0 +1,61 @@ +-- 005_tasks_rbac_restrict.sql +-- Manual one-shot: restrict task CREATION in the permission database to +-- system_administrator / hr_administrator / recruiter. 004 attached the full +-- tasks_management bundle (all 8 tags) to six roles; here hiring_manager, +-- department_head and ceo swap it for a view-only bundle so the permission DB +-- itself says who may create — the tasks service additionally enforces the +-- creator-role check at request time. Idempotent; auto-applied at startup by +-- alembic_setup.run_manual_sql(). + +-- ============================================================================= +-- 1. View-only bundle (tasks.view, tasks.export) +-- ============================================================================= +INSERT INTO app.permissions (name, description, permission_tags, is_system, created_at, updated_at, is_active, is_deleted) +SELECT + 'tasks_viewer', + 'Recruiting task list: read-only access', + ( + SELECT COALESCE(jsonb_agg(id ORDER BY id), '[]'::jsonb) + FROM app.permission_tags + WHERE is_deleted = false + AND tag_name IN ('tasks.view', 'tasks.export') + ), + true, + NOW(), + NOW(), + true, + false +WHERE NOT EXISTS ( + SELECT 1 FROM app.permissions WHERE name = 'tasks_viewer' +); + +-- ============================================================================= +-- 2. Remove the full tasks_management bundle from the non-creator roles +-- (jsonb arrays hold numbers, so `-` text removal does not apply — rebuild) +-- ============================================================================= +UPDATE app.roles r +SET permissions = ( + SELECT COALESCE(jsonb_agg(elem), '[]'::jsonb) + FROM jsonb_array_elements(COALESCE(r.permissions, '[]'::jsonb)) elem + WHERE elem <> to_jsonb(p.id) + ), + updated_at = NOW() +FROM app.permissions p +WHERE p.name = 'tasks_management' + AND r.role_name IN ('hiring_manager', 'department_head', 'ceo') + AND COALESCE(r.permissions, '[]'::jsonb) @> jsonb_build_array(p.id); + +-- ============================================================================= +-- 3. Attach the view-only bundle to those roles +-- ============================================================================= +UPDATE app.roles r +SET permissions = COALESCE(r.permissions, '[]'::jsonb) || jsonb_build_array(p.id), + updated_at = NOW() +FROM app.permissions p +WHERE p.name = 'tasks_viewer' + AND r.role_name IN ( + 'hiring_manager', + 'department_head', + 'ceo' + ) + AND NOT (COALESCE(r.permissions, '[]'::jsonb) @> jsonb_build_array(p.id)); diff --git a/backend/tasks/app.py b/backend/tasks/app.py index 48e8ea0..105d680 100644 --- a/backend/tasks/app.py +++ b/backend/tasks/app.py @@ -51,6 +51,24 @@ async def fetch_tasks( raise HTTPException(status_code=500, detail=str(e)) +@router.get("/tasks/assignees/fetch") +async def fetch_task_assignees( + current_user: dict = Depends(require_permission(PermissionTag.TASKS_VIEW)), + session: AsyncSession = Depends(get_session), +): + """Assignee picker: active recruiter-role users (role id resolved from the + roles table). Separate from /users/fetch so assigning a task never requires + rbac_users.view.""" + try: + service = Task(session=session) + data, total = await service.get_assignees() + return JSONResponse(content={"data": data, "total": total, "status_code": 200}) + except HTTPException: + raise + except Exception as e: + raise HTTPException(status_code=500, detail=str(e)) + + @router.post("/tasks/create") async def create_task( payload: TaskCreate, diff --git a/backend/tasks/views.py b/backend/tasks/views.py index a5338df..b34c4d1 100644 --- a/backend/tasks/views.py +++ b/backend/tasks/views.py @@ -6,6 +6,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload +from role.models import Roles from tasks.models import Tasks from tasks.serializers import serialize_task from users.models import Users @@ -13,6 +14,12 @@ from users.models import Users VALID_STATUS = ("open", "done") VALID_PRIORITY = ("high", "medium", "low") +# Role names, not ids: the ids are resolved from the roles table at request +# time so a re-seeded database that renumbers roles cannot silently shift who +# may do what. recruiter is role_id 3 on the current seed. +ASSIGNEE_ROLE = "recruiter" +CREATOR_ROLES = ("system_administrator", "hr_administrator", "recruiter") + def _as_uuid(value): if value in (None, ""): @@ -54,16 +61,53 @@ class Task: return {u.id: u for u in result.scalars().all()} async def _validate_assignee(self, assignee_id): - """Assignees are real staff accounts: existing, not deleted, and not the - candidate role — a candidate cannot work a recruiting task.""" + """Assignees must be recruiter-role accounts (role resolved from the DB): + existing, not deleted, role_name == ASSIGNEE_ROLE.""" user = await Users.get_user_by_id(self.session, str(assignee_id)) if user is None or user.is_deleted: raise HTTPException(status_code=422, detail="Assignee not found") role_name = user.role.role_name if user.role else None - if role_name == "candidate": - raise HTTPException(status_code=422, detail="A candidate account cannot be a task assignee") + if role_name != ASSIGNEE_ROLE: + raise HTTPException( + status_code=422, + detail=f"Tasks can only be assigned to {ASSIGNEE_ROLE} accounts", + ) return user + async def _require_creator_role(self, current_user): + """Creation is limited to system admin / HR admin / recruiter. The role + ids are looked up from the roles table, and the permission-tag guard on + the route (tasks.create) still applies on top of this.""" + result = await self.session.execute( + select(Roles).where(Roles.role_name.in_(CREATOR_ROLES)) + ) + allowed_ids = {r.id for r in result.scalars().all()} + if current_user.get("role_id") not in allowed_ids: + raise HTTPException( + status_code=403, + detail="Only system administrators, HR administrators and recruiters can create tasks", + ) + + async def get_assignees(self): + """The assignee picker: every active recruiter-role user. Separate from + /users/fetch so callers do not need rbac_users.view to assign a task.""" + role = await Roles.get_role_by_name(self.session, ASSIGNEE_ROLE) + if role is None: + raise HTTPException(status_code=500, detail=f"Role {ASSIGNEE_ROLE} is not seeded") + rows = await Users.get_users(self.session, top=500, role_id=role.id) + data = [ + { + "id": str(u.id), + "name": u.name, + "email": u.email, + "role_id": u.role_id, + "role_name": role.role_name, + "is_active": u.is_active, + } + for u in rows + ] + return data, len(data) + async def get_tasks(self, status=None, priority=None, assignee_id=None, top=None, skip=0): if status and status not in VALID_STATUS: raise HTTPException(status_code=422, detail=f"status must be one of {', '.join(VALID_STATUS)}") @@ -86,6 +130,7 @@ class Task: return [serialize_task(r, users_by_id) for r in rows], total async def create_task(self, payload, current_user): + await self._require_creator_role(current_user) title = (payload.get("title") or "").strip() if not title: raise HTTPException(status_code=422, detail="title is required") @@ -93,10 +138,16 @@ class Task: if priority not in VALID_PRIORITY: raise HTTPException(status_code=422, detail=f"priority must be one of {', '.join(VALID_PRIORITY)}") creator = _user_id(current_user) - # No assignee picked -> the caller works it themselves. - assignee = _as_uuid(payload.get("assignee_id")) if payload.get("assignee_id") else creator - if assignee is None: - raise HTTPException(status_code=422, detail="Invalid assignee_id") + # Assignee must be a recruiter. Omitting it only works when the caller + # IS a recruiter (assign to self); an admin must pick one explicitly. + if payload.get("assignee_id"): + assignee = _as_uuid(payload["assignee_id"]) + if assignee is None: + raise HTTPException(status_code=422, detail="Invalid assignee_id") + elif current_user.get("role_name") == ASSIGNEE_ROLE: + assignee = creator + else: + raise HTTPException(status_code=422, detail="assignee_id is required") await self._validate_assignee(assignee) fields = { diff --git a/frontend/src/api/tasks.js b/frontend/src/api/tasks.js index 32770e1..6420d78 100644 --- a/frontend/src/api/tasks.js +++ b/frontend/src/api/tasks.js @@ -24,6 +24,15 @@ export function remove(taskId) { return request('/tasks/delete', { method: 'DELETE', params: { task_id: taskId } }) } +/** + * The assignee picker — active recruiter-role users only (role id resolved + * from the roles table server-side). A dedicated endpoint so assigning a task + * never needs rbac_users.view. + */ +export function listAssignees() { + return request('/tasks/assignees/fetch') +} + /** * tasks row -> the shape Tasks.jsx renders. `done` folds the open|done status, * `due` becomes a Date for the overdue comparison (null = no deadline), and the diff --git a/frontend/src/lib/queryKeys.js b/frontend/src/lib/queryKeys.js index ca7c7c6..b33deeb 100644 --- a/frontend/src/lib/queryKeys.js +++ b/frontend/src/lib/queryKeys.js @@ -55,7 +55,11 @@ export const qk = { offers: { all: () => ['offers'], list: (p = {}) => ['offers', 'list', p] }, interviews: { all: () => ['interviews'], range: (p = {}) => ['interviews', 'range', p] }, activity: { all: () => ['activity'], feed: (p = {}) => ['activity', 'feed', p] }, - tasks: { all: () => ['tasks'], list: (p = {}) => ['tasks', 'list', p] }, + tasks: { + all: () => ['tasks'], + list: (p = {}) => ['tasks', 'list', p], + assignees: () => ['tasks', 'assignees'], + }, // --- seed-backed buckets --- // These are not "server state" — the cache IS the store for them, so every diff --git a/frontend/src/screens/Tasks.jsx b/frontend/src/screens/Tasks.jsx index 44cbac4..597b2d3 100644 --- a/frontend/src/screens/Tasks.jsx +++ b/frontend/src/screens/Tasks.jsx @@ -2,11 +2,12 @@ Tasks — the recruiting task list, on live backend data. Rows come from GET /tasks/fetch via toTaskView; create/complete/reopen go - through POST /tasks/create and PATCH /tasks/update. Assignees are REAL user - accounts (users ⋈ roles): the picker lists /users/fetch minus candidate-role - rows and submits assignee_id. A caller without rbac_users.view cannot list - users — the picker then collapses to "assign to me", which matches the - server's default. Fields the backend does not carry (task type, notes, + through POST /tasks/create and PATCH /tasks/update. Assignees are RECRUITER + accounts only: the picker lists GET /tasks/assignees/fetch (role resolved + from the roles table server-side) and the server 422s any other role. + Creation is limited to system_administrator / hr_administrator / recruiter — + enforced server-side against the roles table, mirrored here so the button + doesn't invite a 403. Fields the backend does not carry (task type, notes, candidate link) are gone rather than rendered as placeholders — the Inbox screen precedent. Saved Searches stays decorative seed chrome. ============================================================ */ @@ -23,11 +24,13 @@ import { useFormState } from '../components/AuthLayout' import { qk } from '../lib/queryKeys' import { friendlyAuthError } from '../lib/errors' import * as tasksApi from '../api/tasks' -import * as usersApi from '../api/users' import { fmtDate, fmtShort, savedSearches } from '../data/seed' const FILTERS = ['All', 'Open', 'Completed', 'Overdue', 'High', 'Medium', 'Low'] const PRIORITY_CLASS = { High: 'b-red', Medium: 'b-amber', Low: 'b-gray' } +// Mirrors backend/tasks/views.py CREATOR_ROLES — the server is the enforcer, +// this only keeps the button honest. +const CREATOR_ROLES = ['system_administrator', 'hr_administrator', 'recruiter'] async function fetchTasks() { const res = await tasksApi.list() @@ -35,17 +38,9 @@ async function fetchTasks() { return rows.map(tasksApi.toTaskView) } -/* Staff only: the server rejects candidate-role assignees, so don't offer them. - 403 (no rbac_users.view) degrades to [] and the form falls back to - "assign to me" — the server-side default for an omitted assignee_id. */ async function fetchAssignees() { - try { - const res = await usersApi.list({ top: 200 }) - const rows = Array.isArray(res?.data) ? res.data : [] - return rows.filter((u) => u.role_name !== 'candidate' && !u.is_deleted) - } catch { - return [] - } + const res = await tasksApi.listAssignees() + return Array.isArray(res?.data) ? res.data : [] } export default function Tasks() { @@ -54,11 +49,11 @@ export default function Tasks() { const navigate = useNavigate() const qc = useQueryClient() - const canCreate = can('tasks.create') + const canCreate = can('tasks.create') && CREATOR_ROLES.includes(user?.role_name) const canEdit = can('tasks.edit') const tasksQuery = useQuery({ queryKey: qk.tasks.list(), queryFn: fetchTasks }) - const assigneesQuery = useQuery({ queryKey: qk.users.list({ pick: 'assignees' }), queryFn: fetchAssignees }) + const assigneesQuery = useQuery({ queryKey: qk.tasks.assignees(), queryFn: fetchAssignees }) const tasks = tasksQuery.data ?? [] const [filter, setFilter] = useState('All') @@ -326,16 +321,18 @@ function TaskDetail({ task: t, canEdit, canDelete, completing, deleting, onClose } function AddTask({ assignees, me, pending, onClose, onSave }) { - // '' = "assign to me" — the server defaults an omitted assignee_id to the - // caller, which is also the only option when /users/fetch is 403 for us. + // Assignees are recruiter accounts only. '' means "assign to me" and is only + // offered when the caller IS a recruiter (the server 422s otherwise). + const meIsRecruiter = me?.role_name === 'recruiter' const form = useFormState({ title: '', priority: 'Medium', assignee: '', due: '' }) function submit() { if (pending) return - if (!form.values.title.trim()) { - form.setErrors({ title: 'Required' }) - return - } + const errors = {} + if (!form.values.title.trim()) errors.title = 'Required' + if (!form.values.assignee && !meIsRecruiter) errors.assignee = 'Pick a recruiter' + form.setErrors(errors) + if (Object.keys(errors).length) return const body = { title: form.values.title.trim(), priority: form.values.priority.toLowerCase(), @@ -378,17 +375,24 @@ function AddTask({ assignees, me, pending, onClose, onSave }) {