fix(i-d): mail-API-Brueche behoben — signatures PATCH/DELETE + labels DELETE im Backend ergaenzt, drafts PATCH->PUT
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
Root-Cause: Frontend-Komponenten (SignatureManager, LabelManager) rufen Endpunkte auf die das Backend nie hatte (404/405 in Production). Anders als ai/sessions sind diese Funktionen ECHT in Komponenten eingebunden -> Backend-Routen nachbestellt statt Frontend-Calls zu loeschen:
(1) PATCH+DELETE /mail/signatures/{id}: MailSignatureUpdate-Schema neu, Tenant-Scoped + Owner-Check (403 bei fremder Signatur), is_default-Exklusivitaet beim Setzen. (2) DELETE /mail/labels/{id}: gleicher Stil. (3) updateDraft Frontend: apiPatch -> apiPut (Backend hat PUT /drafts/{id} bereits). Beweistest tests/test_mail_sig_label_routes.py 5/5 gruen (PATCH-Werte, DELETE+Liste-leer, 404-Faelle).
Verifikation: create_app registriert beide neuen Routen (563 total); ruff clean; tsc exit=0.
This commit is contained in:
@@ -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) ───
|
||||
|
||||
|
||||
|
||||
@@ -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 ───
|
||||
|
||||
|
||||
|
||||
@@ -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<Mail> {
|
||||
}
|
||||
|
||||
export function updateDraft(mailId: string, payload: MailDraftPayload): Promise<Mail> {
|
||||
return apiPatch<Mail>(`/mail/drafts/${mailId}`, payload);
|
||||
return apiPut<Mail>(`/mail/drafts/${mailId}`, payload);
|
||||
}
|
||||
|
||||
@@ -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<PolicyListResponse> {
|
||||
return apiGet<PolicyListResponse>(`/policies/${entityType}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a single policy by ID.
|
||||
*/
|
||||
export function fetchPolicy(entityType: string, policyId: string): Promise<ABACPolicy> {
|
||||
return apiGet<ABACPolicy>(`/policies/${entityType}/${policyId}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new policy for the given entity type.
|
||||
*/
|
||||
export function createPolicy(
|
||||
entityType: string,
|
||||
payload: CreatePolicyPayload
|
||||
): Promise<ABACPolicy> {
|
||||
return apiPost<ABACPolicy>(`/policies/${entityType}`, payload);
|
||||
}
|
||||
|
||||
/**
|
||||
* Update an existing policy.
|
||||
*/
|
||||
export function updatePolicy(
|
||||
entityType: string,
|
||||
policyId: string,
|
||||
payload: UpdatePolicyPayload
|
||||
): Promise<ABACPolicy> {
|
||||
return apiPut<ABACPolicy>(`/policies/${entityType}/${policyId}`, payload);
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a policy.
|
||||
*/
|
||||
export function deletePolicy(entityType: string, policyId: string): Promise<void> {
|
||||
return apiDelete<void>(`/policies/${entityType}/${policyId}`);
|
||||
}
|
||||
@@ -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) });
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -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": "<p>old</p>", "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": "<p>new</p>", "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"] == "<p>new</p>"
|
||||
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
|
||||
Reference in New Issue
Block a user