diff --git a/backend/src/BigRegister.Api/Contracts/Dtos.cs b/backend/src/BigRegister.Api/Contracts/Dtos.cs index 1a8c787..211b5c0 100644 --- a/backend/src/BigRegister.Api/Contracts/Dtos.cs +++ b/backend/src/BigRegister.Api/Contracts/Dtos.cs @@ -74,7 +74,6 @@ public sealed record DocumentRefDto(string CategoryId, string Channel, string? D // Submit requests carry only the fields the server re-validates (UX-only fields // stay on the client). ponytail: a real submit would carry the full application. -public sealed record RegistratieRequest(string DiplomaHerkomst, IReadOnlyList? Documents = null); public sealed record ChangeRequestRequest(string Telefoon); diff --git a/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs b/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs index 1db366a..c9ee980 100644 --- a/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs +++ b/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs @@ -9,12 +9,6 @@ namespace BigRegister.Domain.Submissions; /// public static class SubmissionRules { - // RULE: a manually entered diploma cannot be auto-verified. - public static string? RejectRegistratie(string diplomaHerkomst) => - diplomaHerkomst == "handmatig" - ? "Een handmatig ingevoerd diploma kan niet automatisch worden geverifieerd. Uw aanvraag is doorgestuurd voor handmatige beoordeling." - : null; - // RULE: an application reporting zero worked hours is rejected. public static string? RejectZeroUren(int uren) => uren == 0 ? "Aanvraag afgewezen: geen gewerkte uren geregistreerd." : null; diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index bc7e98f..fbadce2 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -185,11 +185,6 @@ api.MapGet("/stamdata/{table}", (string table, string? peildatum, HttpContext ct // --- POST: submits. The server is the authority; it re-validates and decides. --- -api.MapPost("/registrations", (RegistratieRequest req, HttpContext ctx) => - Submit(ctx, "registratie", SubmissionRules.RejectRegistratie(req.DiplomaHerkomst), req.Documents)) -.Produces() -.ProducesProblem(StatusCodes.Status422UnprocessableEntity); - api.MapPost("/change-requests", (ChangeRequestRequest req, HttpContext ctx) => Submit(ctx, "telefoonwijziging", SubmissionRules.RejectPhoneChange(req.Telefoon))) .Produces() diff --git a/backend/swagger.json b/backend/swagger.json index 6ca507f..af86f93 100644 --- a/backend/swagger.json +++ b/backend/swagger.json @@ -210,45 +210,6 @@ } } }, - "/api/v1/registrations": { - "post": { - "tags": [ - "BigRegister.Api, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/RegistratieRequest" - } - } - }, - "required": true - }, - "responses": { - "200": { - "description": "OK", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ReferentieResponse" - } - } - } - }, - "422": { - "description": "Unprocessable Content", - "content": { - "application/problem+json": { - "schema": { - "$ref": "#/components/schemas/ProblemDetails" - } - } - } - } - } - } - }, "/api/v1/change-requests": { "post": { "tags": [ @@ -2468,23 +2429,6 @@ }, "additionalProperties": false }, - "RegistratieRequest": { - "type": "object", - "properties": { - "diplomaHerkomst": { - "type": "string", - "nullable": true - }, - "documents": { - "type": "array", - "items": { - "$ref": "#/components/schemas/DocumentRefDto" - }, - "nullable": true - } - }, - "additionalProperties": false - }, "RegistrationDto": { "type": "object", "properties": { diff --git a/backend/tests/BigRegister.Tests/Domain/SubmissionRuleTests.cs b/backend/tests/BigRegister.Tests/Domain/SubmissionRuleTests.cs index 2265864..ff3028e 100644 --- a/backend/tests/BigRegister.Tests/Domain/SubmissionRuleTests.cs +++ b/backend/tests/BigRegister.Tests/Domain/SubmissionRuleTests.cs @@ -4,14 +4,6 @@ namespace BigRegister.Tests.Domain; public class SubmissionRuleTests { - [Fact] - public void Manual_diploma_is_rejected() => - Assert.NotNull(SubmissionRules.RejectRegistratie("handmatig")); - - [Fact] - public void Duo_diploma_is_accepted() => - Assert.Null(SubmissionRules.RejectRegistratie("duo")); - [Fact] public void Zero_hours_is_rejected() => Assert.NotNull(SubmissionRules.RejectZeroUren(0)); diff --git a/backend/tests/BigRegister.Tests/EndpointTests.cs b/backend/tests/BigRegister.Tests/EndpointTests.cs index d7335a1..c364a96 100644 --- a/backend/tests/BigRegister.Tests/EndpointTests.cs +++ b/backend/tests/BigRegister.Tests/EndpointTests.cs @@ -69,26 +69,6 @@ public class EndpointTests(TestWebApplicationFactory factory) : IClassFixture(); - Assert.NotNull(body); - Assert.StartsWith("BIG-2026-", body.Referentie); - } - - [Fact] - public async Task Registration_with_manual_diploma_is_rejected_with_problem_details() - { - var res = await _client.PostAsJsonAsync("/api/v1/registrations", new RegistratieRequest("handmatig")); - Assert.Equal(HttpStatusCode.UnprocessableEntity, res.StatusCode); - var contentType = res.Content.Headers.ContentType; - Assert.NotNull(contentType); - Assert.Contains("application/problem+json", contentType.ToString()); - } - [Fact] public async Task Change_request_with_valid_phone_succeeds() { @@ -101,11 +81,16 @@ public class EndpointTests(TestWebApplicationFactory factory) : IClassFixture())!; + var submit = await _client.PostAsJsonAsync($"/api/v1/applications/{aanvraag.Id}/submit", + new { diplomaHerkomst = "duo", documents = new[] { new DocumentRefDto("diploma", "digital", doc.DocumentId) } }); submit.EnsureSuccessStatusCode(); Assert.Equal(HttpStatusCode.Conflict, (await _client.DeleteAsync($"/api/v1/uploads/{doc.DocumentId}")).StatusCode); } diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-01.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-01.md index 27ee3a9..402caf3 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-01.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-01.md @@ -14,12 +14,12 @@ whether a given client-chosen `localId` exists anywhere in the store, plus its d ## What changed -| File | Change | -| ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | -| `Program.cs` `/uploads/{documentId}/content` | takes `HttpContext`; allowed for the owning `ZorgverlenerCaller` or a caller passing `Authz.CanBeoordelen`; else `404` | -| `Program.cs` `/uploads/status` | takes `HttpContext`; scoped to `ctx.Zorgverlener().Bsn` | -| `Data/DocumentStore.cs` `ByLocalIds` | second parameter `owner`; filters on it (the only call site is the endpoint above) | -| `tests/BigRegister.Tests/UploadAccessTests.cs` | **new** — 5 cases | +| File | Change | +| ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| `Program.cs` `/uploads/{documentId}/content` | takes `HttpContext`; allowed for the owning `ZorgverlenerCaller` or a caller passing `Authz.CanBeoordelen`; else `404` | +| `Program.cs` `/uploads/status` | takes `HttpContext`; scoped to `ctx.Zorgverlener().Bsn` | +| `Data/DocumentStore.cs` `ByLocalIds` | second parameter `owner`; filters on it (the only call site is the endpoint above) | +| `tests/BigRegister.Tests/UploadAccessTests.cs` | **new** — 5 cases | The two actor kinds are matched, not branched on a boolean, because `ctx.Zorgverlener()` **throws** for a `MedewerkerCaller` — a behandelaar reading an aanvraag's linked documents diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md index 2d2ff8a..6a5d8fb 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md @@ -10,7 +10,7 @@ Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md audit page — so a BSN was written to durable storage and shown in a UI, on the one trail four documents describe as data-minimised and PII-free. -The endpoint is the *BIG-nummer reveal*, whose own comment says the audit carries +The endpoint is the _BIG-nummer reveal_, whose own comment says the audit carries "NO PII. Never the value that was (or wasn't) revealed" — and it did not carry the BIG-nummer. It carried the BSN instead, in the adjacent argument. @@ -28,10 +28,10 @@ value-asserting test is part of this ticket's definition of done rather than a f ## What changed -| File | Change | -| ----------------------- | ---------------------------------------------------------------------------------------------------- | -| `Program.cs` | resource ref is `"brief"`; a comment records why the id added nothing | -| `AuthzAuditTests.cs` | **new** `No_audit_row_carries_a_subjects_bsn` — asserts on stored **values**, every string field | +| File | Change | +| -------------------- | ------------------------------------------------------------------------------------------------ | +| `Program.cs` | resource ref is `"brief"`; a comment records why the id added nothing | +| `AuthzAuditTests.cs` | **new** `No_audit_row_carries_a_subjects_bsn` — asserts on stored **values**, every string field | No identifier was lost. `BriefStore` keys one brief per owner, so `brief/` named the same thing the row's acting principal already implies; there is no second brief the ref diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-03.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-03.md index 3cb43a1..7b669e0 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-03.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-03.md @@ -10,21 +10,21 @@ both cross-owner lists read by someone who is **not** the subject: - `GET /admin/cases` (`cases:manage`) - `GET /werkvoorraad` (`aanvraag:beoordelen`) -`GET /beoordeling/{id}` — the *detail* view of the same data — already masked. So the +`GET /beoordeling/{id}` — the _detail_ view of the same data — already masked. So the detail screen showed `******782` while the list one click earlier showed the whole BSN. ## What changed -| File | Change | -| --------------------------- | ----------------------------------------------------------------------------------- | -| `Domain/People/Pii.cs` | **new** — `Pii.MaskTail`, moved out of `Program.cs` | -| `Contracts/Mappers.cs` | `Owner = Pii.MaskTail(a.Owner, 3)` | -| `Program.cs` | local `MaskTail` deleted; two call sites point at `Pii.MaskTail` | -| `AdminCasesTests.cs` | asserts the masked value and that `DemoOwner` does not appear | -| `WerkvoorraadTests.cs` | same assertion, replacing the `IsNullOrEmpty` one | +| File | Change | +| ---------------------- | ---------------------------------------------------------------- | +| `Domain/People/Pii.cs` | **new** — `Pii.MaskTail`, moved out of `Program.cs` | +| `Contracts/Mappers.cs` | `Owner = Pii.MaskTail(a.Owner, 3)` | +| `Program.cs` | local `MaskTail` deleted; two call sites point at `Pii.MaskTail` | +| `AdminCasesTests.cs` | asserts the masked value and that `DemoOwner` does not appear | +| `WerkvoorraadTests.cs` | same assertion, replacing the `IsNullOrEmpty` one | **Masked in the mapper, not at the endpoints.** The point of the ticket is that both -lists *inherit* it, so a third cross-owner list cannot be added that forgets to mask. +lists _inherit_ it, so a third cross-owner list cannot be added that forgets to mask. **`MaskTail` moved to `Domain/People/Pii.cs`** because it now has three callers across three folders (`Contracts`, `Program.cs`, and `Data` once **RB-04** lands), and a second diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-04.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-04.md index 817072e..890b5b2 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-04.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-04.md @@ -11,12 +11,12 @@ every row is precisely other PII. Same failure shape as RB-02, in a second store ## What changed -| File | Change | -| ------------------------------- | ----------------------------------------------------------------- | -| `Data/DocumentStore.cs` `Add` | `Audit("upload", …, Pii.MaskTail(owner, 3))` | +| File | Change | +| ------------------------------------- | --------------------------------------------------------- | +| `Data/DocumentStore.cs` `Add` | `Audit("upload", …, Pii.MaskTail(owner, 3))` | | `Data/DocumentStore.cs` `DeleteOwned` | `Audit("delete-user", …, Pii.MaskTail(owner, 3))` | -| `Data/DocumentStore.cs` `Audit` | doc comment: actors arrive **already redacted** | -| `UploadAccessTests.cs` | **new** `The_document_audit_trail_records_a_masked_actor` | +| `Data/DocumentStore.cs` `Audit` | doc comment: actors arrive **already redacted** | +| `UploadAccessTests.cs` | **new** `The_document_audit_trail_records_a_masked_actor` | **Masked at the two call sites, not inside `Audit`** — unlike RB-03, where masking in the mapper was the point. `Audit`'s third actor is the literal `"admin"` (from `AdminDelete`), diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md index d633420..47e9203 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md @@ -26,10 +26,10 @@ The two `"returned null body"` throws in `GetAsync`/`PostAsync` interpolated the ## What changed -| File | Change | -| ------------------------ | --------------------------------------------------------------------------------- | -| `Zgw/ZgwHttpClient.cs` | `Redact(url)` (path only) at all three sites; snippet → `res.ReasonPhrase` | -| `ZgwDivergenceTests.cs` | **new** `A_recorded_divergence_carries_no_response_body_and_no_query_string` | +| File | Change | +| ----------------------- | ---------------------------------------------------------------------------- | +| `Zgw/ZgwHttpClient.cs` | `Redact(url)` (path only) at all three sites; snippet → `res.ReasonPhrase` | +| `ZgwDivergenceTests.cs` | **new** `A_recorded_divergence_carries_no_response_body_and_no_query_string` | Status + path is enough to route a failure to the right endpoint. The diagnostic detail that was lost already has a deliberate home: `ZGW_DEBUG_HTTP=1` wires diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-06.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-06.md new file mode 100644 index 0000000..0e74589 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-06.md @@ -0,0 +1,69 @@ +# RB-06 — delete the dead `POST /registrations` + +Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-010 · `99-backlog.md` RB-06 + +## What was wrong + +`POST /registrations` took a `Documents` list and passed it straight to `Submit`, which +calls `DocumentStore.Link(...)` on every digital `documentId` in it. Linking a document +**blocks its owner from deleting it** (`DeleteOwned` → 409 `Linked`). + +There was no `ForeignIds` ownership check on that path. The real submit endpoint, +`POST /applications/{id}/submit`, has had one since it was written: + +```csharp +if (documentIds is { Count: > 0 } && DocumentStore.ForeignIds(documentIds, ctx.Zorgverlener().Bsn) is { Count: > 0 } foreignIds) + return Results.Problem(detail: $"Onbekend of niet-eigen document(en): …", statusCode: 400); +``` + +So any authenticated citizen could post another citizen's document id and permanently +block them from deleting their own diploma scan. + +## Deleted rather than guarded + +The ticket allowed either. Deleted, because the endpoint is dead: no frontend caller (the +generated client's `registrations` method was unreferenced), and the whole registratie flow +goes through `POST /applications/{id}/submit`. + +| File | Change | +| -------------------------------------------- | ------------------------------------------------- | +| `Program.cs` | endpoint deleted | +| `Contracts/Dtos.cs` | `RegistratieRequest` deleted (no other reference) | +| `Domain/Submissions/SubmissionRules.cs` | `RejectRegistratie` deleted — see below | +| `backend/swagger.json`, `api-client.ts` | regenerated (`npm run gen:api`) | +| `EndpointTests.cs`, `SubmissionRuleTests.cs` | retargeted, see below | + +### Why `RejectRegistratie` went with it + +It was reachable only from this endpoint, and the live path deliberately **contradicts** +it. `RejectRegistratie("handmatig")` returned a 422 rejection; the modern submit does + +```csharp +"registratie" => (null, req.DiplomaHerkomst == "duo"), +``` + +— a manual diploma is not rejected, it simply does not auto-approve and goes to a +behandelaar. Its own message even said so ("doorgestuurd voor handmatige beoordeling") +while being returned as a rejection. Leaving it behind would have left an obsolete rule +with a passing spec, which is exactly how it gets reintroduced. + +**This is the one judgement call in this ticket** — the backlog row says "delete the dead +endpoint", not "delete the rule". Reverting just the `SubmissionRules`/`SubmissionRuleTests` +hunks restores it without touching anything else. + +### Test coverage that moved rather than vanished + +- `Registration_with_manual_diploma_is_rejected_with_problem_details` was the only test + asserting the `Submit` helper's `application/problem+json` rejection shape. That assertion + moved into `Change_request_with_bad_phone_is_rejected_with_problem_details` — + `/change-requests` is the other endpoint on the same helper. +- `User_delete_blocked_with_409_once_linked_to_submission` covered `DocumentStore.Link` + blocking a delete. Retargeted to `POST /applications/{id}/submit`, i.e. the path that is + actually in use. `POST /registrations` was the only other caller of `Link`. +- `Registration_with_duo_diploma_succeeds` was deleted outright — `Change_request_with_valid_phone_succeeds` + is the same assertion on the same helper. + +## Verification + +`npm run ci`. `dotnet test`: **249 passed, 1 failed** — the pre-existing +`OpenZaakIntegrationTests.Admin_cases_…`, which needs a live container. diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index b0507b8..39566bf 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -21,7 +21,7 @@ tested where._ Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test method name (backend), read as a sentence. Nothing here is hand-written prose: this page **is** the suite, reshaped for a business reader. 402 frontend behaviours across -8 contexts; 221 backend behaviours across 37 test +8 contexts; 225 backend behaviours across 38 test classes. ## Frontend (by context) @@ -825,6 +825,7 @@ classes. - A denied admin action is recorded - A reveal attempt is recorded +- No audit row carries a subjects bsn - The audit schema carries no pii ### AuthzTests @@ -921,10 +922,8 @@ classes. - Brp returns address - Duo lookup carries server decided questions and professions - IntakePolicy returns scholing threshold -- Registration with duo diploma succeeds -- Registration with manual diploma is rejected with problem details - Change request with valid phone succeeds -- Change request with bad phone is rejected +- Change request with bad phone is rejected with problem details - Health endpoint is ok - Correlation id supplied by the caller is echoed back - Correlation id is generated when the caller omits it @@ -1087,12 +1086,19 @@ classes. ### SubmissionRuleTests -- Manual diploma is rejected -- Duo diploma is accepted - Zero hours is rejected - Worked hours are accepted - Phone change is validated +### UploadAccessTests + +- The owner can read the bytes +- Another citizen gets 404 not 403 +- A behandelaar can read a linked document +- A medewerker without the behandelaar rol does not +- The document audit trail records a masked actor +- Status reports another citizens localId as unknown + ### WerkvoorraadTests - Behandelaar sees submitted cases in the queue @@ -1109,6 +1115,7 @@ classes. - Submit with a failing zgw flags the divergence instead of diverging silently - Submit with a healthy zgw leaves no divergence flag +- A recorded divergence carries no response body and no query string ### ZgwTokenProviderTests diff --git a/libs/shared/src/infrastructure/api-client.ts b/libs/shared/src/infrastructure/api-client.ts index ee5e57f..05eb07b 100644 --- a/libs/shared/src/infrastructure/api-client.ts +++ b/libs/shared/src/infrastructure/api-client.ts @@ -359,52 +359,6 @@ export class ApiClient { return Promise.resolve(null as any); } - /** - * @return OK - */ - registrations(body: RegistratieRequest): Promise { - let url_ = this.baseUrl + "/api/v1/registrations"; - url_ = url_.replace(/[?&]$/, ""); - - const content_ = JSON.stringify(body); - - let options_: RequestInit = { - body: content_, - method: "POST", - headers: { - "Content-Type": "application/json", - "Accept": "application/json" - } - }; - - return this.http.fetch(url_, options_).then((_response: Response) => { - return this.processRegistrations(_response); - }); - } - - protected processRegistrations(response: Response): Promise { - const status = response.status; - let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v: any, k: any) => _headers[k] = v); }; - if (status === 200) { - return response.text().then((_responseText) => { - let result200: any = null; - result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ReferentieResponse; - return result200; - }); - } else if (status === 422) { - return response.text().then((_responseText) => { - let result422: any = null; - result422 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; - return throwException("Unprocessable Content", status, _responseText, _headers, result422); - }); - } else if (status !== 200 && status !== 204) { - return response.text().then((_responseText) => { - return throwException("An unexpected server error occurred.", status, _responseText, _headers); - }); - } - return Promise.resolve(null as any); - } - /** * @return OK */ @@ -2237,11 +2191,6 @@ export interface ReferentieResponse { referentie?: string | undefined; } -export interface RegistratieRequest { - diplomaHerkomst?: string | undefined; - documents?: DocumentRefDto[] | undefined; -} - export interface RegistrationDto { bigNummer?: string | undefined; naam?: string | undefined;