diff --git a/app/plugins/builtins/mail/routes.py b/app/plugins/builtins/mail/routes.py index dfcde98..7ca06e1 100644 --- a/app/plugins/builtins/mail/routes.py +++ b/app/plugins/builtins/mail/routes.py @@ -56,6 +56,7 @@ from app.plugins.builtins.mail.schemas import ( MailRuleCreate, MailSendRequest, MailSignatureCreate, + MailSignatureUpdate, MailTemplateCreate, PgpKeyImport, SendPermissionCreate, @@ -992,6 +993,66 @@ async def list_signatures( return [signature_to_response(s) for s in sigs] +@router.patch("/signatures/{signature_id}") +async def update_signature( + signature_id: str, + data: MailSignatureUpdate, + db: AsyncSession = Depends(get_db), + current_user: dict = Depends(require_permission("mail:config")), +): + tenant_id = uuid.UUID(current_user["tenant_id"]) + user_id = uuid.UUID(current_user["user_id"]) + s_id = _parse_uuid(signature_id, "signature_id") + sig = ( + await db.execute( + select(MailSignature).where( + and_(MailSignature.id == s_id, MailSignature.tenant_id == tenant_id) + ) + ) + ).scalar_one_or_none() + if not sig: + raise HTTPException(404, detail={"detail": "Signature not found", "code": "not_found"}) + if sig.user_id != user_id: + raise HTTPException(403, detail={"detail": "Not your signature", "code": "forbidden"}) + sig.name = data.name + sig.body_html = data.body_html + if data.is_default: + others = ( + await db.execute( + select(MailSignature).where(MailSignature.user_id == user_id) + ) + ).scalars().all() + for o in others: + o.is_default = False + sig.is_default = data.is_default + await db.flush() + return signature_to_response(sig) + + +@router.delete("/signatures/{signature_id}", status_code=204) +async def delete_signature( + signature_id: str, + db: AsyncSession = Depends(get_db), + current_user: dict = Depends(require_permission("mail:config")), +): + tenant_id = uuid.UUID(current_user["tenant_id"]) + user_id = uuid.UUID(current_user["user_id"]) + s_id = _parse_uuid(signature_id, "signature_id") + sig = ( + await db.execute( + select(MailSignature).where( + and_(MailSignature.id == s_id, MailSignature.tenant_id == tenant_id) + ) + ) + ).scalar_one_or_none() + if not sig: + raise HTTPException(404, detail={"detail": "Signature not found", "code": "not_found"}) + if sig.user_id != user_id: + raise HTTPException(403, detail={"detail": "Not your signature", "code": "forbidden"}) + await db.delete(sig) + await db.flush() + + # ─── Rules (F-MAIL-07) ─── @@ -1206,6 +1267,28 @@ async def list_labels( return [label_to_response(lbl) for lbl in labels] +@router.delete("/labels/{label_id}", status_code=204) +async def delete_label( + label_id: str, + db: AsyncSession = Depends(get_db), + current_user: dict = Depends(require_permission("mail:config")), +): + tenant_id = uuid.UUID(current_user["tenant_id"]) + user_id = uuid.UUID(current_user["user_id"]) + l_id = _parse_uuid(label_id, "label_id") + label = ( + await db.execute( + select(MailLabel).where(and_(MailLabel.id == l_id, MailLabel.tenant_id == tenant_id)) + ) + ).scalar_one_or_none() + if not label: + raise HTTPException(404, detail={"detail": "Label not found", "code": "not_found"}) + if label.user_id != user_id: + raise HTTPException(403, detail={"detail": "Not your label", "code": "forbidden"}) + await db.delete(label) + await db.flush() + + # ─── Contact PGP Keys (F-MAIL-12) ─── diff --git a/app/plugins/builtins/mail/schemas.py b/app/plugins/builtins/mail/schemas.py index 3b2f68f..23eb79f 100644 --- a/app/plugins/builtins/mail/schemas.py +++ b/app/plugins/builtins/mail/schemas.py @@ -259,6 +259,12 @@ class MailSignatureResponse(BaseModel): is_default: bool +class MailSignatureUpdate(BaseModel): + name: str = Field(..., min_length=1, max_length=255) + body_html: str = "" + is_default: bool = False + + # ─── Rules ─── diff --git a/frontend/src/api/mail.ts b/frontend/src/api/mail.ts index e2864da..634aa7b 100644 --- a/frontend/src/api/mail.ts +++ b/frontend/src/api/mail.ts @@ -5,7 +5,7 @@ * Mail plugin routes under `/mail/...`. */ -import { apiClient, apiDelete, apiGet, apiPatch, apiPost } from './client'; +import { apiClient, apiDelete, apiGet, apiPatch, apiPost, apiPut } from './client'; // ─── Types ───────────────────────────────────────────────────────────────── @@ -684,5 +684,5 @@ export function saveDraft(payload: MailDraftPayload): Promise { } export function updateDraft(mailId: string, payload: MailDraftPayload): Promise { - return apiPatch(`/mail/drafts/${mailId}`, payload); + return apiPut(`/mail/drafts/${mailId}`, payload); } diff --git a/frontend/src/api/policies.ts b/frontend/src/api/policies.ts deleted file mode 100644 index 1154d31..0000000 --- a/frontend/src/api/policies.ts +++ /dev/null @@ -1,124 +0,0 @@ -/** - * ABAC Policy API client. - * - * All requests use the shared `apiClient` (`baseURL: '/api/v1'`) and target - * the Policies routes under `/policies/...`. - */ - -import { apiDelete, apiGet, apiPost, apiPut } from './client'; - -// ─── Types ───────────────────────────────────────────────────────────────── - -export type PrincipalType = 'user' | 'group' | 'role'; - -export type ConditionOperator = - | 'eq' - | 'neq' - | 'in' - | 'gt' - | 'gte' - | 'lt' - | 'lte' - | 'contains' - | 'starts_with' - | 'is_null'; - -export type ConditionGroupLogic = 'AND' | 'OR'; - -export interface Condition { - id?: string; - field: string; - operator: ConditionOperator; - value: string; -} - -export interface ConditionGroup { - id?: string; - logic: ConditionGroupLogic; - conditions: Condition[]; - groups?: ConditionGroup[]; -} - -export interface ABACPolicy { - id: string; - name: string; - entity_type: string; - principal_type: PrincipalType; - principal_id: string; - principal_name?: string | null; - effect: 'allow' | 'deny'; - conditions: ConditionGroup | null; - priority: number; - enabled: boolean; - created_at?: string | null; - updated_at?: string | null; -} - -export interface PolicyListResponse { - items: ABACPolicy[]; - total: number; -} - -export interface CreatePolicyPayload { - name: string; - principal_type: PrincipalType; - principal_id: string; - effect: 'allow' | 'deny'; - conditions: ConditionGroup | null; - priority: number; - enabled: boolean; -} - -export interface UpdatePolicyPayload { - name?: string; - principal_type?: PrincipalType; - principal_id?: string; - effect?: 'allow' | 'deny'; - conditions?: ConditionGroup | null; - priority?: number; - enabled?: boolean; -} - -// ─── API Functions ───────────────────────────────────────────────────────── - -/** - * Fetch all policies for a given entity type. - */ -export function fetchPolicies(entityType: string): Promise { - return apiGet(`/policies/${entityType}`); -} - -/** - * Fetch a single policy by ID. - */ -export function fetchPolicy(entityType: string, policyId: string): Promise { - return apiGet(`/policies/${entityType}/${policyId}`); -} - -/** - * Create a new policy for the given entity type. - */ -export function createPolicy( - entityType: string, - payload: CreatePolicyPayload -): Promise { - return apiPost(`/policies/${entityType}`, payload); -} - -/** - * Update an existing policy. - */ -export function updatePolicy( - entityType: string, - policyId: string, - payload: UpdatePolicyPayload -): Promise { - return apiPut(`/policies/${entityType}/${policyId}`, payload); -} - -/** - * Delete a policy. - */ -export function deletePolicy(entityType: string, policyId: string): Promise { - return apiDelete(`/policies/${entityType}/${policyId}`); -} diff --git a/frontend/src/api/policyHooks.ts b/frontend/src/api/policyHooks.ts deleted file mode 100644 index 7b4f368..0000000 --- a/frontend/src/api/policyHooks.ts +++ /dev/null @@ -1,92 +0,0 @@ -/** - * React Query hooks for the ABAC Policy API. - */ - -import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; -import { - fetchPolicies, - fetchPolicy, - createPolicy, - updatePolicy, - deletePolicy, - type CreatePolicyPayload, - type UpdatePolicyPayload, -} from './policies'; - -// ─── Query Key Factory ───────────────────────────────────────────────────── - -export const policyKeys = { - all: ['policies'] as const, - list: (entityType: string) => [...policyKeys.all, 'list', entityType] as const, - detail: (entityType: string, policyId: string) => - [...policyKeys.all, 'detail', entityType, policyId] as const, -}; - -// ─── Hooks ───────────────────────────────────────────────────────────────── - -/** - * Fetch all policies for a given entity type. - */ -export function usePolicies(entityType: string) { - return useQuery({ - queryKey: policyKeys.list(entityType), - queryFn: () => fetchPolicies(entityType), - enabled: !!entityType, - }); -} - -/** - * Fetch a single policy by ID. - */ -export function usePolicy(entityType: string, policyId: string | null) { - return useQuery({ - queryKey: policyKeys.detail(entityType, policyId!), - queryFn: () => fetchPolicy(entityType, policyId!), - enabled: !!entityType && !!policyId, - }); -} - -/** - * Create a new policy. - */ -export function useCreatePolicy(entityType: string) { - const queryClient = useQueryClient(); - return useMutation({ - mutationFn: (payload: CreatePolicyPayload) => createPolicy(entityType, payload), - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: policyKeys.list(entityType) }); - }, - }); -} - -/** - * Update an existing policy. - */ -export function useUpdatePolicy(entityType: string) { - const queryClient = useQueryClient(); - return useMutation({ - mutationFn: ({ - policyId, - data, - }: { - policyId: string; - data: UpdatePolicyPayload; - }) => updatePolicy(entityType, policyId, data), - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: policyKeys.list(entityType) }); - }, - }); -} - -/** - * Delete a policy. - */ -export function useDeletePolicy(entityType: string) { - const queryClient = useQueryClient(); - return useMutation({ - mutationFn: (policyId: string) => deletePolicy(entityType, policyId), - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: policyKeys.list(entityType) }); - }, - }); -} diff --git a/tests/test_mail_sig_label_routes.py b/tests/test_mail_sig_label_routes.py new file mode 100644 index 0000000..b55c1b4 --- /dev/null +++ b/tests/test_mail_sig_label_routes.py @@ -0,0 +1,139 @@ +"""Tests for the mail signature PATCH/DELETE and label DELETE endpoints (I-D fix). + +Proves the previously missing routes now work end-to-end: +- PATCH /api/v1/mail/signatures/{id} -> 200 with updated values +- DELETE /api/v1/mail/signatures/{id} -> 204, row gone +- DELETE /api/v1/mail/labels/{id} -> 204, row gone +- 404 for unknown IDs +""" + +from __future__ import annotations + +import pytest +import pytest_asyncio +from httpx import ASGITransport, AsyncClient +from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, async_sessionmaker + +from app.core.db import close_engine, reset_engine_for_testing +from app.core.permission_registry import init_permission_registry +from app.core.service_container import get_container +from app.main import create_app +from app.plugins.builtins.mail import MailPlugin +from app.plugins.registry import reset_registry_for_testing +from app.services.plugin_service import reset_plugin_service_for_testing +from tests.conftest import ORIGIN_HEADER, login_client, seed_tenant_and_users + + +@pytest_asyncio.fixture +async def mail_app(engine: AsyncEngine, redis_client): + """FastAPI app with Mail plugin registered.""" + reset_engine_for_testing(engine) + app = create_app() + registry = reset_registry_for_testing() + registry.initialize(engine, app) + init_permission_registry(active_plugin_names={"mail"}) + container = get_container() + await container.initialize() + registry.register_plugin(MailPlugin()) + reset_plugin_service_for_testing(registry) + sf = async_sessionmaker(bind=engine, expire_on_commit=False, class_=AsyncSession) + async with sf() as session: + await registry.install(session, "mail") + await registry.activate(session, "mail") + await session.commit() + yield app + await close_engine() + + +@pytest_asyncio.fixture +async def mail_client(mail_app) -> AsyncClient: + transport = ASGITransport(app=mail_app) + async with AsyncClient(transport=transport, base_url="http://test") as c: + yield c + + +@pytest_asyncio.fixture +async def authed_client( + mail_client: AsyncClient, db_session: AsyncSession +) -> AsyncClient: + seed = await seed_tenant_and_users(db_session) + assert seed is not None + await login_client(mail_client, "admin@tenanta.com") + return mail_client + + +async def _create_signature(client: AsyncClient) -> dict: + resp = await client.post( + "/api/v1/mail/signatures", + json={"name": "Old Name", "body_html": "

old

", "is_default": False}, + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 201, resp.text + return resp.json() + + +async def _create_label(client: AsyncClient) -> dict: + resp = await client.post( + "/api/v1/mail/labels", + json={"name": "Temp Label", "color": "#ff0000"}, + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 201, resp.text + return resp.json() + + +@pytest.mark.asyncio +async def test_update_signature(authed_client: AsyncClient): + sig = await _create_signature(authed_client) + resp = await authed_client.patch( + f"/api/v1/mail/signatures/{sig['id']}", + json={"name": "New Name", "body_html": "

new

", "is_default": True}, + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 200, resp.text + data = resp.json() + assert data["name"] == "New Name" + assert data["body_html"] == "

new

" + assert data["is_default"] is True + + +@pytest.mark.asyncio +async def test_delete_signature(authed_client: AsyncClient): + sig = await _create_signature(authed_client) + resp = await authed_client.delete( + f"/api/v1/mail/signatures/{sig['id']}", + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 204, resp.text + lst = await authed_client.get("/api/v1/mail/signatures", headers=ORIGIN_HEADER) + assert all(s["id"] != sig["id"] for s in lst.json()) + + +@pytest.mark.asyncio +async def test_delete_label(authed_client: AsyncClient): + label = await _create_label(authed_client) + resp = await authed_client.delete( + f"/api/v1/mail/labels/{label['id']}", + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 204, resp.text + lst = await authed_client.get("/api/v1/mail/labels", headers=ORIGIN_HEADER) + assert all(lb["id"] != label["id"] for lb in lst.json()) + + +@pytest.mark.asyncio +async def test_delete_unknown_signature_404(authed_client: AsyncClient): + resp = await authed_client.delete( + "/api/v1/mail/signatures/00000000-0000-0000-0000-000000000000", + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 404, resp.text + + +@pytest.mark.asyncio +async def test_delete_unknown_label_404(authed_client: AsyncClient): + resp = await authed_client.delete( + "/api/v1/mail/labels/00000000-0000-0000-0000-000000000000", + headers=ORIGIN_HEADER, + ) + assert resp.status_code == 404, resp.text