feat(attachments): ticket-note attachment upload / download / delete (PMS-483) #358

Merged
YousifShkara merged 5 commits from feat/PMS-483-note-attachments into main 2026-06-25 09:43:40 +02:00
Owner

PMS-483 ships the upload, download, and delete surface that
ticket_attachments has been missing since migration 005. Both agent
and portal callers can now attach files to a ticket note, and the
download endpoint is symmetric.

Schema (migration 079):

  • ticket_attachments.created_by_contact_id UUID NULL REFERENCES
    contacts(id) ON DELETE SET NULL - populated on portal-originated
    uploads, NULL on agent uploads.
  • uploaded_by_id relaxed to NULL since portal contacts have no users
    row to point at. The right authorship column per origin is enforced
    at the application layer (Uploader enum in attachments.rs).
  • Partial index on (tenant_id, created_by_contact_id) WHERE
    created_by_contact_id IS NOT NULL for the eventual "your uploads"
    portal feed.

Storage backend:

  • Local disk under ATTACHMENT_DIR (default ./attachments). Blob path
    is {dir}/{tenant_id}/{attachment_id} so a hostile file_name can't
    escape the per-tenant directory or shadow a sibling tenant's blob.
  • ATTACHMENT_MAX_BYTES caps the body size (default 25 MiB). Oversize
    uploads return 413 via a new AppError::PayloadTooLarge variant.

Routes:

  • Agent: POST/GET/DELETE
    /api/v1/tickets/{id}/notes/{note_id}/attachments[/{attachment_id}]
    behind RequireAuth. Tenant-scoped via the (tenant, ticket, note)
    triple check.
  • Portal: same shape under /api/v1/portal/... behind RequirePortalAuth,
    plus a company-scope check so a contact can only touch attachments
    on tickets belonging to their own company. Portal delete additionally
    refuses to remove an attachment uploaded by someone else.

Multipart parsing reads the file field; original filename + mime
type are sanitised (path separator strip, length cap) before they hit
the DB.

Tests (tests/ticket_note_attachments.rs, requires reqwest multipart
feature added to dev-deps):

  • agent_upload_list_download_delete: round-trip of all four verbs.
  • oversize_upload_returns_413: 2 KiB body against a 1 KiB cap.
  • portal_upload_visible_to_agent_blocked_for_sibling: same-company
    contact uploads, agent downloads, sibling-company contact gets 404.

SPA UI surface lands in a follow-up; the server contract is the
invariant piece and the tests pin it.

#PMS-483

PMS-483 ships the upload, download, and delete surface that ticket_attachments has been missing since migration 005. Both agent and portal callers can now attach files to a ticket note, and the download endpoint is symmetric. Schema (migration 079): - ticket_attachments.created_by_contact_id UUID NULL REFERENCES contacts(id) ON DELETE SET NULL - populated on portal-originated uploads, NULL on agent uploads. - uploaded_by_id relaxed to NULL since portal contacts have no users row to point at. The right authorship column per origin is enforced at the application layer (Uploader enum in attachments.rs). - Partial index on (tenant_id, created_by_contact_id) WHERE created_by_contact_id IS NOT NULL for the eventual "your uploads" portal feed. Storage backend: - Local disk under ATTACHMENT_DIR (default ./attachments). Blob path is {dir}/{tenant_id}/{attachment_id} so a hostile file_name can't escape the per-tenant directory or shadow a sibling tenant's blob. - ATTACHMENT_MAX_BYTES caps the body size (default 25 MiB). Oversize uploads return 413 via a new AppError::PayloadTooLarge variant. Routes: - Agent: POST/GET/DELETE /api/v1/tickets/{id}/notes/{note_id}/attachments[/{attachment_id}] behind RequireAuth. Tenant-scoped via the (tenant, ticket, note) triple check. - Portal: same shape under /api/v1/portal/... behind RequirePortalAuth, plus a company-scope check so a contact can only touch attachments on tickets belonging to their own company. Portal delete additionally refuses to remove an attachment uploaded by someone else. Multipart parsing reads the `file` field; original filename + mime type are sanitised (path separator strip, length cap) before they hit the DB. Tests (tests/ticket_note_attachments.rs, requires reqwest `multipart` feature added to dev-deps): - agent_upload_list_download_delete: round-trip of all four verbs. - oversize_upload_returns_413: 2 KiB body against a 1 KiB cap. - portal_upload_visible_to_agent_blocked_for_sibling: same-company contact uploads, agent downloads, sibling-company contact gets 404. SPA UI surface lands in a follow-up; the server contract is the invariant piece and the tests pin it. #PMS-483
feat(attachments): ticket-note attachment upload / download / delete (PMS-483)
Some checks failed
E2E / Playwright against staging (pull_request) Successful in 1m0s
Check / fmt + clippy + build + tests (pull_request) Successful in 5m20s
Integration / integration tests (pull_request) Failing after 8m16s
5c0ad78adc
PMS-483 ships the upload, download, and delete surface that
ticket_attachments has been missing since migration 005. Both agent
and portal callers can now attach files to a ticket note, and the
download endpoint is symmetric.

Schema (migration 079):
- ticket_attachments.created_by_contact_id UUID NULL REFERENCES
  contacts(id) ON DELETE SET NULL - populated on portal-originated
  uploads, NULL on agent uploads.
- uploaded_by_id relaxed to NULL since portal contacts have no users
  row to point at. The right authorship column per origin is enforced
  at the application layer (Uploader enum in attachments.rs).
- Partial index on (tenant_id, created_by_contact_id) WHERE
  created_by_contact_id IS NOT NULL for the eventual "your uploads"
  portal feed.

Storage backend:
- Local disk under ATTACHMENT_DIR (default ./attachments). Blob path
  is {dir}/{tenant_id}/{attachment_id} so a hostile file_name can't
  escape the per-tenant directory or shadow a sibling tenant's blob.
- ATTACHMENT_MAX_BYTES caps the body size (default 25 MiB). Oversize
  uploads return 413 via a new AppError::PayloadTooLarge variant.

Routes:
- Agent: POST/GET/DELETE
  /api/v1/tickets/{id}/notes/{note_id}/attachments[/{attachment_id}]
  behind RequireAuth. Tenant-scoped via the (tenant, ticket, note)
  triple check.
- Portal: same shape under /api/v1/portal/... behind RequirePortalAuth,
  plus a company-scope check so a contact can only touch attachments
  on tickets belonging to their own company. Portal delete additionally
  refuses to remove an attachment uploaded by someone else.

Multipart parsing reads the `file` field; original filename + mime
type are sanitised (path separator strip, length cap) before they hit
the DB.

Tests (tests/ticket_note_attachments.rs, requires reqwest `multipart`
feature added to dev-deps):
- agent_upload_list_download_delete: round-trip of all four verbs.
- oversize_upload_returns_413: 2 KiB body against a 1 KiB cap.
- portal_upload_visible_to_agent_blocked_for_sibling: same-company
  contact uploads, agent downloads, sibling-company contact gets 404.

SPA UI surface lands in a follow-up; the server contract is the
invariant piece and the tests pin it.

#PMS-483
fix(test): seed distinctly-named companies in portal cross-company test (PMS-483)
Some checks failed
E2E / Playwright against staging (pull_request) Successful in 1m0s
Check / fmt + clippy + build + tests (pull_request) Successful in 2m53s
Integration / integration tests (pull_request) Failing after 10m42s
4451660d43
CI report: `portal_upload_visible_to_agent_blocked_for_sibling` panicked
on its second `common::seed_company(&pool)` call with a duplicate-key
error on the `idx_companies_tenant_name_unique` index. The shared
helper hard-codes the company name as "Acme Co", so two consecutive
calls collide on the per-tenant (tenant_id, lower(name)) unique index.

The other two tests in this file seed only one company so they
passed; this is purely a fixture-shape bug, not a real defect in the
PMS-483 attachment surface.

Drops a local `seed_company_named` helper that takes the name as a
parameter and uses it to seed "Acme Alpha" + "Beta Industries" in the
cross-company test. Keeps the shared helper untouched (every other
test relies on its current shape).

#PMS-483
fix(test): seed contact uses is_portal_user, not portal_enabled (PMS-483)
Some checks failed
E2E / Playwright against staging (pull_request) Successful in 1m1s
Check / fmt + clippy + build + tests (pull_request) Successful in 3m58s
Integration / integration tests (pull_request) Failing after 5m41s
4ae04719e4
CI report after the previous fix: the cross-company test now panics
with `column "portal_enabled" of relation "contacts" does not
exist` on the seed INSERT.

Root cause: `portal_enabled` lives on the `companies` table; the
contacts table's portal-eligibility flag is `is_portal_user`
(migrations/004_contacts.sql:81). The previous seed referenced the
wrong column name and only got caught here because no other test in
this file seeds portal contacts.

Swaps the column name. No other change.

#PMS-483
fix(test): portal login body must include tenant_slug (PMS-483)
Some checks failed
E2E / Playwright against staging (pull_request) Successful in 21s
Check / fmt + clippy + build + tests (pull_request) Successful in 1m38s
Integration / integration tests (pull_request) Failing after 7m28s
a5619c6d97
CI report after the previous fix: the cross-company test now panics
with `portal login failed: 422 Unprocessable Entity` on the
`/api/v1/portal/auth/login` call.

Root cause: `PortalLoginRequest` (src/modules/portal/models.rs:59)
requires three fields - `tenant_slug`, `email`, `password` - because
`contacts.email` is only unique within a tenant. The previous test
helper sent only `email` and `password`, which serde rejects with a
422.

Adds `tenant_slug: "default"` to the body (matches the default tenant
seeded in migration 002_tenants.sql).

#PMS-483
fix(attachments): portal sub-router needs its own auth middleware layer (PMS-483)
All checks were successful
E2E / Playwright against staging (pull_request) Successful in 58s
Check / fmt + clippy + build + tests (pull_request) Successful in 2m52s
Integration / integration tests (pull_request) Successful in 9m50s
Create release / Create release from merged PR (pull_request) Successful in 6s
a7d2f6d2d9
CI report after the previous fix: the cross-company test now panics
with `same-company portal upload should 2xx; got 401 Unauthorized`.
The portal contact logs in cleanly (200 + access_token), but the
subsequent upload against `/api/v1/portal/tickets/{id}/notes/{nid}/attachments`
gets a 401 from `RequirePortalAuth`.

Root cause: `portal_routes` applies `portal_auth_middleware` as a
`.layer()` on its own Router. `portal_attachment_routes` was merged
into the same portal sub-tree (`api/router.rs::portal_api`) but axum
layers are per-Router and `.merge()` does NOT inherit the parent's
layers - so requests to the attachment paths reached the handler
without `PortalAuthState` ever being inserted into request extensions,
and `RequirePortalAuth::from_request_parts` fell through to 401.

Fix:
- Re-export `PortalAuthMiddleware` from `crate::modules::portal`.
- `portal_attachment_routes` now takes a `PortalAuthService`
  alongside the `AttachmentService`, builds the same middleware
  inside, and layers it onto its own Router. Mirrors the pattern
  `portal_routes` uses for its routes.
- `api/router.rs` constructs a dedicated `PortalAuthService` clone
  (`portal_attachment_auth_service`) so the original service can
  still be moved into `portal_routes` without a borrow conflict; the
  service is just `(db_pool, jwt_secret)` so cloning it is free.

Agent routes are unaffected (they live under the agent tree which
applies the legacy `AuthMiddleware` separately).

#PMS-483
YousifShkara deleted branch feat/PMS-483-note-attachments 2026-06-25 09:43:40 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!358
No description provided.