feat(forms): client request forms (MACD) by magic link #498

Merged

longjacksonle merged 5 commits from feat/PMS-730-client-request-forms into main 2026-08-06 20:54:29 +02:00

Conversation 0 Commits 5 Files changed 10 +1483 -10

longjacksonle commented 2026-08-06 20:51:17 +02:00

Owner

Copy link

Implements PMS-730: a client fills in a request form from a magic link, and the submission becomes a ticket attributed to their company and carrying the KB article that describes how to perform the change. Builds on the PMS-731 substrate (#497) and the procedure link (#495).

Flow

1. An agent POSTs /api/v1/form-request-links with a form definition, a company, and either a contact or an email address.
2. The client receives a forms.request_link email containing {app_url}/request-forms/{token}.
3. GET /api/v1/public/request-forms/{token} returns the form to render.
4. POST the same path validates the answers, stores the submission, creates the ticket, and burns the link, returning the ticket number.

Decisions worth reviewing

The token is {token_id}.{secret}, not the portal's {contact_id}.{secret}. This is the design question I flagged before starting. portal_setup_tokens stores a salted Argon2 hash, which cannot be looked up by value, so its redemption path fetches every token for the contact and verifies each in turn; that is affordable only because contact_id narrows the candidate set first. (Migration 042 indexes token_hash and the code cannot use that index, for exactly this reason.) A request link has no equivalent narrowing key, so copying the shape would Argon2-verify against the whole table on every submission. Keying the prefix on the row's own id makes resolution a primary-key lookup plus exactly one verify, with identical single-use and expiry semantics.

Attribution comes from the token, never the payload. Company and contact are recorded when the link is issued, so a forwarded link cannot file a request against a different company no matter what the submitter types. The acting user on the created ticket is the MSP user who issued the link, because the submitter is a client with no users row and tickets.created_by_id is NOT NULL.

The token is burned last, and guarded. The submission and ticket are created first, then the link is marked used in the same transaction, so a failure anywhere rolls the whole thing back and the client retries with a live link. Burning first would spend a single-use link on a request that never produced a ticket. The final UPDATE carries used_at IS NULL, so two submissions racing yield one ticket and the loser gets Gone rather than being quietly accepted.

A rejected submission does NOT burn the link. A client who mistypes a date should not need a new link emailed to them. Pinned by a test.

The status contract is deliberately non-committal. Expired, malformed, unmatched, and a real token id with the wrong secret all return the same BadRequest with the same message; only an already-submitted link differs (Gone), which the holder of that link already knows. A response that distinguished the others would be an oracle for which token ids exist. Pinned by a test that compares the message bodies.

The client view is minimal. The public form response carries what is needed to render and validate the inputs and nothing else: no ids, no author, no timestamps, and no KB article, which is an internal procedure for whoever works the ticket rather than something the client is entitled to read.

Rate limiting is the backstop, not the control. Per-IP, in-memory, 30/min, deliberately looser than the portal's login bucket. The token is a 64 character secret behind an Argon2 verify, so the limiter exists to make grinding expensive rather than to be the thing that stops it, and a false 429 on a client-facing form is a support call.

A trap this had to fix

TenantService::copy_default_config copies notification templates by an explicit event-type allowlist, so a template seeded by a migration reaches existing tenants but not ones created afterwards. Without forms.request_link in that list, a tenant created from here on would persist tokens and silently send no email, since migration 097 removed the direct-send fallback and left the dispatcher as the only delivery path. That is its own commit.

What is here

• migrations/101_form_request_tokens.sql: the token table with its fail-closed RLS policy, plus the forms.request_link template and rule seeded for every tenant (flat placeholders, E'...' body, avoiding both halves of the PMS-702 defect).
• src/modules/forms/request_links.rs: issue, resolve, redeem, list.
• src/modules/forms/public_routes.rs: the unauthenticated surface and its limiter.
• Agent routes for issuing and listing links; a new /api/v1/public nest outside AuthMiddleware.
• tests/request_forms.rs: 5 tests covering every acceptance criterion.

The tests recover the token the way the recipient does, out of the queued email body, because the API deliberately never returns it. That makes "an MSP user can send a client a request-form link by email" part of every test rather than something asserted once and then bypassed.

RLS and pool safety

form_request_tokens attaches ENABLE + FORCE + the NULLIF'd GUC comparison explicitly, since the 024 / 038 loops have already run. Resolution is pre-auth: the tenant is what the lookup resolves, so there is no GUC to set beforehand and that single query runs on the migrator pool with a SAFETY comment, exactly as portal setup-token redemption and the tenant_intake_tokens bearer lookup do. check-pool-safety.nu passes. Everything after resolution is tenant-scoped through begin_with_tenant.

Verification

Full just test-integration green, plus fmt, clippy -D warnings, and all six guard scripts.

Not here

Google and portal request paths (PMS-730 scopes v1 to change requests), and PMS-732's time aggregation, which consumes this.

Note on CI

The Playwright job fails on staging for a reason unrelated to this branch: the bunyip hub rejects the E2E account's TOTP at /login/2fa, reproducing on main and every open PR. Diagnosis is on #496.

Implements PMS-730: a client fills in a request form from a magic link, and the submission becomes a ticket attributed to their company and carrying the KB article that describes how to perform the change. Builds on the PMS-731 substrate (#497) and the procedure link (#495). ## Flow 1. An agent POSTs `/api/v1/form-request-links` with a form definition, a company, and either a contact or an email address. 2. The client receives a `forms.request_link` email containing `{app_url}/request-forms/{token}`. 3. `GET /api/v1/public/request-forms/{token}` returns the form to render. 4. `POST` the same path validates the answers, stores the submission, creates the ticket, and burns the link, returning the ticket number. ## Decisions worth reviewing **The token is `{token_id}.{secret}`, not the portal's `{contact_id}.{secret}`.** This is the design question I flagged before starting. `portal_setup_tokens` stores a salted Argon2 hash, which cannot be looked up by value, so its redemption path fetches every token for the contact and verifies each in turn; that is affordable only because `contact_id` narrows the candidate set first. (Migration 042 indexes `token_hash` and the code cannot use that index, for exactly this reason.) A request link has no equivalent narrowing key, so copying the shape would Argon2-verify against the whole table on every submission. Keying the prefix on the row's own id makes resolution a primary-key lookup plus exactly one verify, with identical single-use and expiry semantics. **Attribution comes from the token, never the payload.** Company and contact are recorded when the link is issued, so a forwarded link cannot file a request against a different company no matter what the submitter types. The acting user on the created ticket is the MSP user who issued the link, because the submitter is a client with no `users` row and `tickets.created_by_id` is NOT NULL. **The token is burned last, and guarded.** The submission and ticket are created first, then the link is marked used in the same transaction, so a failure anywhere rolls the whole thing back and the client retries with a live link. Burning first would spend a single-use link on a request that never produced a ticket. The final UPDATE carries `used_at IS NULL`, so two submissions racing yield one ticket and the loser gets Gone rather than being quietly accepted. **A rejected submission does NOT burn the link.** A client who mistypes a date should not need a new link emailed to them. Pinned by a test. **The status contract is deliberately non-committal.** Expired, malformed, unmatched, and a real token id with the wrong secret all return the same BadRequest with the same message; only an already-submitted link differs (Gone), which the holder of that link already knows. A response that distinguished the others would be an oracle for which token ids exist. Pinned by a test that compares the message bodies. **The client view is minimal.** The public form response carries what is needed to render and validate the inputs and nothing else: no ids, no author, no timestamps, and no KB article, which is an internal procedure for whoever works the ticket rather than something the client is entitled to read. **Rate limiting is the backstop, not the control.** Per-IP, in-memory, 30/min, deliberately looser than the portal's login bucket. The token is a 64 character secret behind an Argon2 verify, so the limiter exists to make grinding expensive rather than to be the thing that stops it, and a false 429 on a client-facing form is a support call. ## A trap this had to fix `TenantService::copy_default_config` copies notification templates by an explicit event-type allowlist, so a template seeded by a migration reaches existing tenants but not ones created afterwards. Without `forms.request_link` in that list, a tenant created from here on would persist tokens and silently send no email, since migration 097 removed the direct-send fallback and left the dispatcher as the only delivery path. That is its own commit. ## What is here - `migrations/101_form_request_tokens.sql`: the token table with its fail-closed RLS policy, plus the `forms.request_link` template and rule seeded for every tenant (flat placeholders, `E'...'` body, avoiding both halves of the PMS-702 defect). - `src/modules/forms/request_links.rs`: issue, resolve, redeem, list. - `src/modules/forms/public_routes.rs`: the unauthenticated surface and its limiter. - Agent routes for issuing and listing links; a new `/api/v1/public` nest outside `AuthMiddleware`. - `tests/request_forms.rs`: 5 tests covering every acceptance criterion. The tests recover the token the way the recipient does, out of the queued email body, because the API deliberately never returns it. That makes "an MSP user can send a client a request-form link by email" part of every test rather than something asserted once and then bypassed. ## RLS and pool safety `form_request_tokens` attaches `ENABLE` + `FORCE` + the NULLIF'd GUC comparison explicitly, since the 024 / 038 loops have already run. Resolution is pre-auth: the tenant is what the lookup resolves, so there is no GUC to set beforehand and that single query runs on the migrator pool with a SAFETY comment, exactly as portal setup-token redemption and the `tenant_intake_tokens` bearer lookup do. `check-pool-safety.nu` passes. Everything after resolution is tenant-scoped through `begin_with_tenant`. ## Verification Full `just test-integration` green, plus fmt, clippy `-D warnings`, and all six guard scripts. ## Not here Google and portal request paths (PMS-730 scopes v1 to change requests), and PMS-732's time aggregation, which consumes this. ## Note on CI The Playwright job fails on staging for a reason unrelated to this branch: the bunyip hub rejects the E2E account's TOTP at `/login/2fa`, reproducing on `main` and every open PR. Diagnosis is on #496.

longjacksonle added 5 commits 2026-08-06 20:51:17 +02:00

feat(forms): add request-link tokens and the request-link email ... cda83544d4

PMS-730: an MSP user sends a client a magic link to a form definition; the client fills it in without logging in and the submission becomes a ticket.

The emailed token is `{token_id}.{secret}` with only the Argon2 hash of the secret stored, which deliberately does NOT copy the `portal_setup_tokens` shape. That token is `{contact_id}.{secret}`, and because a salted Argon2 hash cannot be looked up by value the redemption path fetches EVERY token for the contact and verifies each in turn; it is affordable only because `contact_id` narrows the set first. (Migration 042 indexes `token_hash` and the code cannot use that index, for exactly this reason.) A request link has no equivalent narrowing key, so the same shape would Argon2-verify against the whole table on every submission. Keying the prefix on the row's own id makes resolution a primary-key lookup plus exactly one verify, with the same single-use and expiry semantics.

Company is recorded on the token at issue time, so the ticket is attributed from the link rather than from anything the submitter types and a forwarded link cannot file a request against a different company.

The `forms.request_link` template and rule are seeded for EVERY tenant rather than the default one only, for the reason migration 097 established: the dispatcher is the only delivery path, so a tenant missing the row would get no mail at all. Placeholders are flat single-brace keys and the body is an E'...' literal, avoiding both halves of the PMS-702 defect that migration 096 had to repair.

RLS is attached explicitly, and the migration carries a note for the serving code: resolving a presented token is pre-auth, so the tenant is what the lookup resolves and there is no GUC to set beforehand. That single query runs on the migrator pool with a SAFETY comment, as portal setup-token redemption and the tenant_intake_tokens bearer lookup already do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

fix(tenants): copy the request-link template to new tenants ... 2ca1ef2329

`copy_default_config` copies notification templates by an explicit event-type allowlist, so a template seeded by a migration reaches existing tenants but not ones created afterwards. Without 'forms.request_link' in that list, a tenant created from here on would persist request-link tokens and silently send no email, since migration 097 removed the direct-send fallback and left the dispatcher as the only delivery path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

feat(forms): issue, resolve and redeem a client request link ... 8fe96af748

The service half of PMS-730. Issuing verifies the company and contact inside the tenant transaction, mints the token, and queues the email after the commit, mirroring the portal setup-link path: a failed send must not roll back the token, because the row is what makes a resend possible.

Resolution keeps the portal's status contract deliberately, so a guessed link cannot be told from a stale one: valid is Ok, already submitted is Gone, and expired, malformed or unmatched are all the same BadRequest with the same message. A response that distinguished them would be an oracle for which token ids are real.

Redemption creates the submission and the ticket first and burns the token LAST, in the same transaction, so a failure anywhere rolls the whole thing back and the client can retry with the link still live. Burning first would spend a single-use link on a request that never produced a ticket. The final UPDATE is guarded on `used_at IS NULL`, so two submissions racing produce one ticket and the loser is reported as already submitted rather than quietly accepted.

Company and contact come from the TOKEN, never from the payload. The acting user is the MSP user who issued the link, because the submitter is a client with no `users` row and `tickets.created_by_id` is NOT NULL. The ticket carries `procedure_kb_article_id` from the definition's `kb_article_id`, which is PMS-730's "article selected by the requested change type" now that the definition IS the request type.

The client-facing view of a form carries what is needed to render and validate the inputs and nothing else: no ids, no author, no timestamps, and no KB article, which is an internal procedure for whoever works the ticket rather than something the client is entitled to read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

feat(forms): mount the public request-form surface ... ee7676d891

Adds the two client-facing routes under a new `/api/v1/public` nest, OUTSIDE `AuthMiddleware` and outside the portal tree, because the caller has no session at all: the magic-link token is the only identity they present and it resolves its own tenant. The tree gets the same error-envelope normalization as the others, so a 400 here looks like a 400 anywhere else.

The agent side gains issuing a link and listing the links already sent for a company. Issuing is RequireAuth rather than admin-gated: emailing a client a form is ordinary work for whoever handles the account, while authoring the definition it points at stays admin-only. Neither response ever carries the token; it is a credential for the recipient, and echoing it back would put it into every response log and browser history entry that touches the endpoint.

Rate limiting is per-IP and explicitly the second line of defence, since the token is a 64 character secret behind an Argon2 verify. The quota is deliberately looser than the portal's login bucket (30/min): a real client opening a form, fixing two validation errors and resubmitting is several requests in a minute, and a false 429 on a client-facing form is a support call, while 30/min still puts the token space out of reach.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

test(forms): cover the request-link flow end to end ... 679edeb516

Walks every PMS-730 acceptance criterion over HTTP.

The tests recover the token the way the recipient does, out of the queued email body, because the API deliberately never returns it. That makes "an MSP user can send a client a request-form link by email" part of every test here rather than something asserted once and then bypassed.

Covered: the link resolves to the form and the client view leaks no ids, author or KB article; an invalid submission is rejected per field AND leaves the link usable, since a client who mistypes a date should not need a new link; a valid submission creates a ticket attributed to the company from the link, carrying the KB article and the answers rendered under the form's own labels, with the link burned and the chain link -> submission -> ticket traceable both ways; a used link refuses both submission and re-read; and an expired link, a guessed one, a malformed one and a real id with the wrong secret all return an identical message, so the response is not an oracle.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

longjacksonle scheduled this pull request to auto merge when all checks succeed 2026-08-06 20:54:14 +02:00

longjacksonle merged commit 45c1a6211d into main 2026-08-06 20:54:29 +02:00

longjacksonle deleted branch feat/PMS-730-client-request-forms 2026-08-06 20:54:30 +02:00

longjacksonle referenced this pull request from a commit 2026-08-06 20:54:31 +02:00

Merge pull request 'feat(forms): client request forms (MACD) by magic link' (#498) from feat/PMS-730-client-request-forms into main

longjacksonle referenced this pull request 2026-08-06 21:03:38 +02:00

feat(reports): measured duration per client-request type #499

longjacksonle referenced this pull request 2026-08-09 18:45:42 +02:00

Render submitted form answers as a list (PMS-747) #504

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!498

No description provided.