# Security Backlog Security defects and hardening work, one PBI per issue. Kept separate from `ROADMAP.md` (general engineering) and `TESTING-BACKLOG.md` (test coverage) so open security work is visible at a glance. **Priorities** - **P0**: exploitable now by an ordinary user or anonymous visitor, leading to account takeover, data loss or data exposure. Fix before other work. - **P1**: exploitable, but needs unusual preconditions or has limited impact. - **P2**: hardening / defence in depth. **Workflow** (see `CLAUDE.md`): verify the finding empirically → propose the fix → go-ahead → implement with a regression test → merge. Each fix goes on its **own branch cut from `master`** and is merged back into `master`. It is never bundled into a feature branch, because all servers deploy `master`, so that is the only way a fix reaches test and prod. Update the Status column in the same change that moves an item forward. Older security items still tracked in `ROADMAP.md` → "Security": the support login backdoor stopgap, and the least-privilege MySQL backup user. **Why user hashids are not secret.** Several items below take a user hashid. Every public poll link embeds the poll owner's user hashid: `publicKey = ownerId.substring(0, 4) + ownerId` (`frontend/src/store/modules/poll.js`), so `/pr/{pollId}/{publicKey}` contains it after the first 4 characters. The poll payload also carries it as `poll.user.id`, and the unsubscribe link in emails is `/unsubscribe/{userHashid}`. Anyone who has received a poll link or a Kulibri email has the hashid. --- ## Index | ID | Priority | Title | Status | Branch | |---|---|---|---|---| | SEC-001 | **P0** | `PUT /api/user` updates any user by request `id` (account takeover) | Fixed on branch, awaiting merge | `security/sec-001-user-update-idor` | | SEC-002 | **P0** | `POST /api/user/delete` deletes any account by request `email` | Fixed on `master` (admins may delete any account, 2026-09-17) | `security/sec-002-user-delete-by-email` (stacked on SEC-001) | | SEC-003 | **P0** | `GET /api/users/{id}` and `POST /api/users/{id}/unsubscribe` return any user's personal data without login | Fixed on branch, awaiting merge | `security/sec-003-user-data-exposure` (stacked on SEC-002) | | SEC-004 | P1 | `POST /api/users/{user}/assign-beta` lets any user grant the beta role to any account | Verified, not fixed | `security/sec-004-assign-beta-authz` (to create) | | SEC-005 | P2 | Email change needs no confirmation and doesn't notify the old address | Not started (product decision) | — | | SEC-006 | P2 | Unsubscribe link uses the plain user hashid, so anyone with it can opt a user out | Not started | — | | SEC-007 | **P0** | `PUT /api/participantattributes` lets anyone with a poll link add or delete a poll's extra fields, deleting every answer in them | Found by code review 2026-09-18, not reproduced | — | | SEC-008 | P1 | Participant attributes marked `admin_only` are served and rendered to everyone | Found by code review 2026-09-18, not reproduced | — | | SEC-009 | **P1** | `hiddenPoll`'s server-side `preference_list` redaction (added 2026-09-03) hid a participant's own answers from themselves and crashed the poll page | Enforcement rolled back on `master` 2026-09-22 and kept (decision 2026-10-08); re-fix not started | `fix/hidden-poll-preference-list-crash` | | SEC-010 | P1 | `/api/designs` had no authentication and no ownership check: any design could be changed or deleted by anyone | Fixed on `master` (2026-09-30) | `fix/designs-auth` | | SEC-011 | **P0** | Membership, payment and invoice routes trust the user id in the URL: cancel, start a payment, read payment data and invoices of any user; start-trial gives a paid plan to anyone | Fixed on `master` | `fix/subscription-ownership` | | SEC-012 | **P0** | Unauthenticated helper routes in `routes/web.php` and `routes/api.php` (`/migrate`, `/get-last-user/{email}`, `/update-memberships-premium-price/...`, `/sent-mail/{email}`, `/get-designs`, `/api/sync-invoice`, `/delete-designs`, `/update-choice-with-default-value` ...) | Found by code review 2026-10-02, list completed 2026-10-06; four data-overwriting routes removed (branch `security/sec-012-remove-data-migration-routes`), the rest not fixed | `security/sec-012-remove-data-migration-routes` | | SEC-013 | **P0** | `PUT /api/poll/{id}/archive` and `DELETE /api/poll/{id}` archive or delete any poll without login | Archive part fixed on branch `feat/archive-per-user` (login required, only polls on the caller's own dashboard, archive per user, G18); `DELETE /api/poll/{id}` still open | `feat/archive-per-user`; `security/sec-013-poll-archive-delete-authz` for the delete part (to create) | All four verified findings (SEC-001 to SEC-004) were reproduced on 2026-09-17 with throwaway feature tests against the real routes and middleware (not committed). --- ## SEC-001: `PUT /api/user` updates any user by request `id` (account takeover) **Priority:** P0 **Status:** Fixed on branch `security/sec-001-user-update-idor`, awaiting merge to `master` **Found:** 2026-09-17, while adding `PUT /api/user/ui-version` for the Vue 3 redesign ### Problem `UserController::update()` (route `PUT /api/user`, group `auth:sanctum, verified, hashids, logger`) loaded the user to modify from the **request body**: ```php $id = $request->id; $user = User::find($id); $user->name = $request->name; $user->email = $request->email; $user->locale = $request->locale; ``` The route only checked that the caller was *some* logged-in, verified user. ### Verified ``` PUT /api/user { id: , name, email: "attacker-controlled@example.com", locale } → 200, victim.email == "attacker-controlled@example.com" ``` ### Impact: full account takeover 1. The attacker sets the victim's email to an address they control. The account stays verified and nobody is notified. 2. The attacker requests a password reset, which is mailed to the new address. 3. The attacker logs in as the victim: polls, participant data, contacts, invoices, the paid subscription. The victim is locked out. Needed: any free account plus a poll link from the victim (see "Why user hashids are not secret" above). ### Fix (done) - `update()` acts on `$request->user()`. A body `id` is ignored. The legacy profile dialog (`EditProfile.vue`, the only caller) posts a copy of the whole user object including its own `id`, so no frontend change is needed. - Validation, deliberately lenient where production data requires it (checked against the full local copy of the users table, 272,941 rows): - `name` nullable, stored as `''` when empty. **239,123 users have an empty name**, the column is NOT NULL, and `ConvertEmptyStringsToNull` turns `""` into `null`. This also fixes a second existing bug: those users got a 500 (silently swallowed by the frontend) whenever they saved their profile, for example to change language. - `email` required, max 255, unique among other users. The address format is checked **only when the email changes**, so the ~530 old accounts holding non-email values (import leftovers like `"Lutz"`, 2 of them active) can still save the rest of their profile. - `locale` `size:2`, not limited to `de`/`en`: 15 different locales are stored. - `opt_out` optional boolean, updated only when sent (unchanged behaviour). - Email changes are logged at warning level (`User email changed`) with user id, old address, new address and IP, so a takeover attempt is auditable from now on. ### Tests `tests/Feature/UserProfileUpdateTest.php` (14 tests): cross-user `id` ignored (attacker's request must itself succeed, so the test can't pass because the request failed), request without `id`, own update, empty name, other locales, `opt_out`, unchanged legacy non-email value, duplicate / invalid / missing email, malformed locale, email-change log entry, 401 without login, 403 unverified. - 8 of the 14 failed on the unfixed code (the 3 SEC-001 tests, the empty-name 500, the new validation and logging); the other 6 describe existing behaviour that had to keep working. - Mutation check: putting `User::find($request->id)` back fails exactly the 3 SEC-001 tests. - Full suite: 135 passed. ### Deploy - Merge `security/sec-001-user-update-idor` to `master`, deploy test → prod via Ansible. No migration. - Visible change: a profile save with a duplicate or invalid **new** email now returns 422. The legacy frontend ignores errors on this call (empty `.catch`), so the dialog closes as if it saved. A follow-up could show the error; not needed for the security fix. - **Past abuse can't be reconstructed from logs.** `LoggerMiddleware` records only path, method and timestamp, and the old `update()` log line dropped `$user` (a `sprintf` without placeholders). The best signal is the data: accounts whose `users.updated_at` changed without a matching login by that user, or whose email domain no longer fits their invoices or contacts. --- ## SEC-002: `POST /api/user/delete` deletes any account by request `email` **Priority:** P0 **Status:** Fixed and merged to `master`; adjusted the same day so admins can delete any account (see below) ### Problem `UserController::destroy()` (route `POST /api/user/delete`, group `auth:sanctum, verified, hashids, logger`) picks the account to delete from the **request body**: `User::where('email', $input['email'])->first()`. It then deletes that user's designs, tasks, participants, preferences, comments, invitations, uploaded images and logos, closes their polls, renames the email to `removed_` and deactivates the account. ### Verified ``` logged in as attacker: POST /api/user/delete { email: "victim@example.com" } → 200, victim.email == "removed_2", victim.active == 0 ``` ### Impact Any logged-in user can irreversibly wipe any account's data and lock the owner out by knowing their email address. No hashid needed. Email addresses of poll owners are also exposed by SEC-003. ### Fix (done) **Correction, 2026-09-17:** the delete dialog is admin-only (`v-if="isAdmin"` in `NavUser.vue`), and admins use it to delete *other* users' accounts. The first fix below broke that. The route stays in the general logged-in group, so the role is now checked on the server: a user with `User::ROLE_ADMIN` deletes the account with the typed email (trimmed, case-insensitive lookup; `"Email not found."` if none), everyone else can only delete their own account as described below. That also covers a later self-service "delete my account" for all users. Simply reverting the fix was rejected because the UI-only admin check would have left the hole open to any logged-in user calling the API directly. First fix, still what applies to non-admins: - `destroy()` always acts on `$request->user()`. The email the user types in the delete dialog (`NavUser.vue`) is now only a **confirmation**: it must match the logged-in account (trimmed, case-insensitive), or nothing is deleted. - The response contract is unchanged, 200 + `message` for every outcome, because the dialog `alert()`s the message and its store action swallows error statuses. A non-matching email answers the existing `"Email not found."`. - This also closes an enumeration leak: before, `"Email not found."` vs. a deletion told the caller whether an address was registered. Now a mismatch gets the same answer either way. - A missing email used to 500 (undefined array key); now it deletes nothing. - Unchanged on purpose: what gets deleted (including other users' contact entries holding the deleted address), and that the session stays logged in after deletion. - Not done, possible follow-up: require the current password for deletion. The dialog doesn't ask for one, so it would need a frontend change. ### Tests `tests/Feature/UserAccountDeleteTest.php` (12 tests). Admin cases (4): an admin deletes another user's account without touching anyone else; the admin bit counts when combined with other roles (and the lookup is case-insensitive); an unknown or missing email deletes nothing. Mutation check: dropping the admin branch fails the 2 admin deletion tests, treating everyone as admin fails the attack and enumeration tests. Full suite: 155 passed; the 1 failure (`AuthFlowsTest` backdoor login) also fails without this change, because a local `.env` defining `SUPPORT_LOGIN_BACKDOOR_PASSWORD` wins over the test's `putenv`. Original non-admin cases (8): another user's email deletes nothing for either account; same answer for registered and unknown emails; missing email; own deletion (email renamed, deactivated, roles cleared, designs deleted, polls closed) without touching another user; case-insensitive match; contacts holding the deleted address removed; 401; 403 unverified. - 4 failed on the unfixed code (attack, enumeration, missing email, case). - Mutation check: putting back the lookup by typed email fails the attack, the enumeration and the case tests. - Full suite: 143 passed. --- ## SEC-003: user personal data exposed without login **Priority:** P0 (data exposure to anonymous visitors) **Status:** Fixed on branch `security/sec-003-user-data-exposure` (stacked on the SEC-002 branch), awaiting merge to `master` ### Problem Two routes in `routes/api.php` sit **outside every auth group** (only the `api` middleware group) and return the full `User` model as JSON. Only `password` and `remember_token` are hidden. - `GET /api/users/{id}` → `UserController::index()` - `POST /api/users/{id}/unsubscribe` → `UserController::unsubscribe()`. The unsubscribe link in emails (`/unsubscribe/{userHashid}`) is intentionally login-free, but the response includes the whole user record. ### Verified ``` not logged in: GET /api/users/ → 200 {"name":"Victim Person","email":"victim@example.com","customer_id":"cst_victim123","roles":0,"last_login_at":...} not logged in: POST /api/users//unsubscribe → 200, victim opted out, response includes "email":"victim@example.com" ``` ### Impact Anyone holding a poll link or a Kulibri email can read the owner's email address, name, Mollie customer id, roles and login timestamps. That is a GDPR-relevant personal data leak, and it hands over the email address SEC-002 needs. ### Fix (done) Both routes were added together in `3722425f` ("admin notification and unsubscribe", June 2026). - `GET /api/users/{id}`: route now requires `auth:sanctum`, and `index()` answers only for the user themselves or a `User::ROLE_ADMIN` (403 otherwise). No caller exists in `frontend/src`, the Blade views or `wordpress/`, and the mobile app uses `/api/me`, so it was locked down rather than removed. Any legitimate own-record read keeps working. Removing it later is still an option. - `POST /api/users/{id}/unsubscribe`: stays login-free, so unsubscribe links in emails already sent keep working. It now returns only `{"name": ...}`, which is all `UnsubscribeView.vue` displays. The view renders only when the response is an object, which `{"name": ""}` still is for the many users with an empty name. - Both: an undecodable or unknown hashid now answers 404 instead of a 500 (`User::find([])->first()` on null). - Remaining by design: anyone with the hashid can still opt a user out of emails. Low impact, tracked as SEC-006. ### Tests `tests/Feature/UserDataExposureTest.php` (9 tests): 401 without login and 403 for another user, neither response containing the email or Mollie customer id; own record and admin read still work; 404 for unknown ids; unsubscribe link works without login, returns exactly `{name}`, contains no personal data, still renders for an empty name; 404 for invalid unsubscribe links. - 7 failed on the unfixed code. - Mutation checks, one per safeguard: removing the route's `auth:sanctum` fails the login test; removing the owner/admin check fails the other-user test; returning the full record from unsubscribe again fails 3 tests. - Full suite: 152 passed. --- ## SEC-004: `POST /api/users/{user}/assign-beta` has no permission check **Priority:** P1 (grants a role, not data access) **Status:** Verified 2026-09-17, not fixed ### Problem `UserController::assignBetaRole()` (group `auth:sanctum, verified, hashids, logger`) grants `User::ROLE_BETA` to the user in the route, for any logged-in caller, and sends that user a "beta access granted" notification. ### Verified ``` logged in as ordinary user: POST /api/users//assign-beta → 200, victim.roles 0 → 2 ``` ### Impact Any user can give themselves or anyone else beta access and trigger notification emails to arbitrary users. Severity depends on what the beta role unlocks; check before fixing. ### Fix (proposed) - Decide who may call it. The caller is `frontend/src/store/store.js` (`/api/users/${id}/assign-beta`). If it's an admin action, require `User::ROLE_ADMIN`; if it's self-service beta signup, act only on `$request->user()`. - Regression tests for both the allowed and the rejected caller. --- ## SEC-005: email change needs no confirmation **Priority:** P2 (hardening; product decision) **Status:** Not started Split out of SEC-001. Changing your own email keeps `email_verified_at` and sends nothing to the old address, so a hijacked session (or a future bug like SEC-001) can silently move an account to another inbox. Options: - Notify the old address with a way to report the change (cheapest). - Require confirmation from the new address before switching. Simply clearing `email_verified_at` is **not** enough: nearly every API route requires `verified`, so it would lock the user out of the app until they verify. --- ## SEC-006: unsubscribe link uses the plain user hashid **Priority:** P2 (low impact) **Status:** Not started Split out of SEC-003. `POST /api/users/{id}/unsubscribe` is login-free by design, and the link in notification emails (`/unsubscribe/{userHashid}`) contains only the user hashid. The same hashid is in every public poll link, so anyone who has one of a user's poll links can opt that user out of Kulibri emails. Since SEC-003 it no longer returns personal data; the remaining effect is an unwanted opt-out. Fix idea: sign the link, for example Laravel's `URL::signedRoute()` or an HMAC of the user id, and accept the old unsigned links only for a transition period, since emails already sent must keep working. --- ## SEC-007: `PUT /api/participantattributes` has no authentication and no ownership check **Priority:** P0 (data loss, anonymous visitor) **Status:** Found by code review 2026-09-18 while summarising how extended participant attributes work. Not yet reproduced against the running app. ### Problem `PUT /api/participantattributes` (`ParticipantAttributeController::update()`) sits in the public `['hashids', 'logger']` middleware group (`routes/api.php`), the same group that serves public poll links. It takes the poll from the **request body** and never checks a session, the poll's admin key or the poll's owner: ```php $attributes = $request->pattributes; $poll_id = $request->poll_id; $pollAttributes = ParticipantAttribute::where('poll_id', $poll_id)->get(); ``` It then synchronises the poll's attribute definitions against the posted list: every existing attribute whose id is *not* in `pattributes` is deleted, entries with `id: null` are created. There is no validation of `pattributes` either, so `label` is taken as sent. ### Impact `participant_values.participant_attribute_id` is a foreign key with `onDelete('cascade')` (`2026_05_21_090803_update_participant_values_table.php`). Deleting an attribute definition therefore deletes **every participant's answer in that field** for that poll, with no soft delete and no way to restore it from the app. An attacker needs only the poll hashid, which is in every poll link (`/pr/{pollId}/{publicKey}`) and in every invitation email. With it they can: - wipe all collected extra-field data of a poll by posting `pattributes: []`, - rename fields (the `label` is shown to every respondent), - add fields, which also writes an empty `participant_values` row for every existing participant of that poll. Blast radius is one poll per request rather than a whole account, but it is unauthenticated, destructive and trivially scriptable, hence P0. ### Fix (proposed) - Resolve the poll and authorise the caller the way the other admin-side poll routes do — reuse `PollAccessResolver` (used by `PollController::getPoll()`) so the poll's admin key or the authenticated owner is required, and return 403 otherwise. The only caller, `updateParticipantAttributes` in `frontend/src/store/modules/poll.js`, runs in the poll editor, where the admin key is already available. - Validate `pattributes`: `label` required/string/max length, `mandatory` boolean, `visibility` in `public,admin_only`, and ids restricted to attributes that belong to the posted poll (today `getAttribute()` matches by id alone). - Regression tests: anonymous caller gets 403 and the attributes survive; owner or admin key can still add, rename and remove fields; an id from another poll is rejected. ### Related Deleting fields is destructive by design for the poll owner too, so this is also the place to decide whether removing an attribute that already holds answers should warn or soft-delete. Product decision, not part of the security fix. --- ## SEC-008: participant attributes marked `admin_only` are served and rendered to everyone **Priority:** P1 (exposure of data the owner marked admin-only, one poll at a time) **Status:** Found by code review 2026-09-18. Not yet reproduced against the running app. ### Problem `participant_attributes.visibility` (enum `public` | `admin_only`, added in `2026_05_11_141014_add_visibility_to_participant_attributes_table.php`) is offered in the poll editor as a per-field "Admin" option (`frontend/src/components/Edit/EditParticipantAttributes.vue`, option `{ code: 'admin_only', label: 'Admin' }`) and is stored and round-tripped correctly — but **nothing consumes it**: - Backend: the only occurrences of `visibility` outside the model are in `ParticipantAttributeController` (writing it). `PollController::getPoll()` eager-loads `participantAttributes` and `participants.attributes` in full and returns them to every caller, including anonymous visitors on a public poll link. - Frontend: `ModalParticipant.vue` builds `attributeValues` from `poll.participant_attributes` and renders them with a plain `v-for="attribute in attributeValues"` — no filter on `visibility`. (The unrelated `show_admin_only` flag found nearby belongs to `Participant`, not to attributes.) So a field the owner marked "Admin" behaves exactly like a public one: every respondent sees the field, is asked to fill it, and receives every other participant's value for it in the poll payload. ### Impact Whatever poll owners put behind the "Admin" label — internal notes, prices, staff-only fields — is visible to anyone holding the poll link. The severity depends entirely on how the option has been used in production; worth checking `participant_attributes WHERE visibility = 'admin_only'` before prioritising. ### Fix (proposed) - Decide the intended semantics first: admin-only means "only the poll admin *sees* the values" and/or "only the admin *fills* the field"? The answer determines whether respondents should be shown the input at all. - Enforce it server-side in `PollController::getPoll()`: when the caller is not the poll admin (`PollAccessResolver` already computes `isAdmin`), strip `admin_only` definitions and the matching `participant_values` from the response. Client-side filtering alone is not a fix — the data would still be in the payload. - Then filter in the legacy participant modal so the field is not rendered for non-admins, and make sure a missing `admin_only` value can't fail the mandatory check. - Regression tests: public poll payload contains no `admin_only` definition or value; admin payload contains both. --- ## SEC-009: `hiddenPoll` server-side `preference_list` redaction rolled back — hid a participant's own answers from themselves **Priority:** P1 (breaks the app for real users; not a data exposure — the rollback below is intentionally the *more* permissive direction) **Status:** Enforcement rolled back on `master` 2026-09-22. Re-fix not started. ### Problem Commit `53d7d459` (2026-09-03) added server-side enforcement of the `hiddenPoll` option to `PollController::getPoll()` and the new `.../preferences` endpoint: participants other than the one identified by `PollAccessResolver` had their `preference_list` stripped from the response. Identity was resolved purely from the URL's invitation key. A production poll opened via a bare `/pr/{pollId}` link (no key segment at all) crashed the whole page: with no key, the resolver could identify nobody, so `preference_list` was stripped from **every** participant, including the row belonging to the logged-in viewer themselves. Two frontend call sites assumed `preference_list` always exists (it always had, pre-2026-09-03) and threw: `RenderData.vue` `compareChoices()` and `RenderMixins.js` `isChoiceVotesLimitReached()`. **First fix (2026-09-22, `fix/hidden-poll-preference-list-crash`):** `PollAccessResolver` was broadened to also identify the requester's own participant via their authenticated session (`poll_id` + `user_id`) when the URL key is absent or unmatched, and the two frontend call sites were guarded against a missing `preference_list`. Verified with tests (`PollAccessResolverTest`, `PublicPollFetchTest`) — including that this does **not** let a plain non-privileged participant see other participants' *emails* (a related but separate stripping rule, gated by `isSubAdmin`/`isAdmin`, confirmed unaffected). **Same day, second occurrence:** testing surfaced a real production poll crashing again, in a third, unguarded call site (`RenderMixins.js` `isAnswerEmpty()`), for a participant the frontend itself identified as the "current user" (`this.userId == participant.user_id` logged `true`). This means the backend's strict identity match (`Participant::where('poll_id', ...)->where('user_id', $authUser->id)->first()`) and the frontend's own, looser notion of "current user" (`ModalParticipantMixins.identifyCurrentUser`: matches via `participant.user.id`, then extends `isCurrentUser` to every participant sharing that `user_id`, with further fallbacks for temp participants and a same-email pass) are not proven equivalent — e.g. a poll with more than one participant row sharing the same `user_id` would make the backend's unordered `->first()` and the frontend's array-order match disagree about which specific row is "own". There are also several more read sites in `RenderMixins.js`/`RenderData.vue`/`RenderWorkshop*.vue` that assume `preference_list` is present, not yet audited one-by-one. ### Decision Rather than keep chasing individual crash sites with a not-yet-proven-correct identity rule, `preference_list` redaction is disabled entirely for now, in both `PollController::getPoll()` and `getPollPreferences()` — back to the pre-2026-09-03 behaviour, where `hiddenPoll` is enforced only in the frontend (display-only, not a real privacy boundary; anyone can already read the raw response). This is a **regression from the privacy improvement**, not a new exposure: it restores exactly what was already true for the ~3 weeks before that commit, and for however long before that hiddenPoll has existed. Kept as-is (still verified correct, not part of the rollback): - `PollAccessResolver`'s authenticated-user identity fallback, which still correctly gates **email** visibility (`isSubAdmin`/`isAdmin`) — a sub-admin sees every participant's email without needing their key on the URL, a plain participant sees none of the others', with or without a key. - The pagination/shell-mode work from the same Sept 3 commit (unrelated: fixes a real 500 on large polls). - The frontend null-guards added in the first fix (defense in depth; still correct even with redaction disabled, e.g. for the unrelated `tasks.resources.participant` and `?paginate=1` shell-mode hiding, which are unaffected by this rollback and still omit the field). ### Fix (proposed, not started) 1. Establish one identity rule both sides actually use, instead of the backend inventing its own. Likely means exposing whatever the frontend's `identifyCurrentUser` chain effectively computes (own `user_id` match, same -email group, temp-participant, last-participant-from-local-storage) as something the backend can also compute deterministically — or, simpler, stop trying to redact per-participant server-side and instead have the frontend ask a dedicated "is this my data" endpoint / rely on the already-scoped `.../preferences` endpoint exclusively, removing the need to embed ambiguous per-participant flags in the full payload at all. 2. Audit every `participant.preference_list` read site in `frontend/src/components/mixins/RenderMixins.js`, `frontend/src/components/Rendering/RenderData.vue`, and the `RenderWorkshop*.vue`/`RenderMobileParticipants.vue` family, and guard each one, not just the two/three that happened to crash first — hidden-poll redaction, whenever it returns, must not depend on frontend code having audited every call site by luck. 3. Before re-enabling, reproduce with a poll that has multiple participant rows sharing one `user_id` and confirm the backend and frontend agree on which row is "own". 4. Regression tests: re-add assertions that other participants' `preference_list` is hidden, but include a factory-built duplicate-`user_id` case, and drive at least one assertion through the actual Vue render path (Jest), not just the API response shape, since both prior crashes were frontend-only failures the backend's own tests couldn't see. --- ## SEC-010: `/api/designs` had no authentication and no ownership check **Priority:** P1 (any design could be changed or deleted by anyone) **Status:** Fixed 2026-09-30 on `fix/designs-auth` (test: `tests/Feature/DesignAccessTest.php`). Found while connecting the Bolt frontend (bearer-token client) to the design feature. ### Problem The `/designs` resource routes sat in the `hashids` + `logger` group without `auth:sanctum`. `DesignController` read `Auth::user()`, which is only resolved for a bearer token by the sanctum guard, so for Bolt it was always null: `POST` crashed (`Attempt to read property "id" on null`), `GET` returned only the default designs. `update`, `edit` and `destroy` looked the design up by id alone, so anyone who knew a design hashid could rewrite or delete another user's design, or a shared default one. `update` also read `$input['poll_id']` unchecked (500 when a client omits it). ### Fix Routes moved into the `auth:sanctum, verified` group (the legacy SPA session works unchanged; only the owner-only branding editor calls them). `update` and `destroy` only accept the caller's own, non-default design (404 otherwise); `edit` (select) also allows the shared defaults. `poll_id` is optional on update. ### Still open `poll_id` is not checked against the caller: an authenticated user can still point any poll at their own design. Same class as SEC-007 for polls in general. --- ## SEC-011: membership, payment and invoice routes trust the user id in the URL **Priority:** P0 (money and personal data, plus free paid plans) **Status:** Fixed on `master` (branch `fix/subscription-ownership`). Found 2026-10-02 while preparing the membership pages for the rewrite; checked in code and with failing tests, not tried against a live server. ### Problem `PaymentController` took the user from the URL (or the body) and never compared it with the caller: | Route | Login before | Effect for anyone who knows a user's hashid | |---|---|---| | `POST /api/subscription/{user_id}` | none | creates a Mollie customer and a payment for that user | | `GET /api/user/{id}/payment` | none | the membership page data: subscription, card label and last digits | | `GET /api/invoices/{user_id}` | none | the invoice list with the payment details | | `GET /api/print-invoice-details/{key}` | none | any invoice by key | | `POST /api/start-trial/{user_id}` | none | a paid subscription (14 days) for any user, again after it ends | | `DELETE /api/delete-trial/{user_id}` | none | cancels the user's active subscription and removes a role | | `POST /api/user/{id}/cancel-subscription` | `auth:sanctum` | **any logged-in user** cancels anyone's subscription (cancels at Mollie, mails the user) | | `POST /api/subscribe/{id}` | `auth:sanctum` | the `user_id` in the body was not compared with the caller | ### Fix (done) - The payment routes moved into the `auth:sanctum, verified, hashids, logger` group; the two invoice routes into `auth:sanctum, verified`. The classic UI keeps working: its session counts as a Sanctum login and it already sends the caller's own id. - `PaymentController::ensureCaller()` refuses (403) every id that is not the caller's. An invoice that is not the caller's is answered 404, so a key is no permission. - `start-trial` is removed. The trial button is switched off in the classic UI, and starting a trial was a way to get a paid plan for free. Its controller method stays as dead code. - `delete-trial` stays, as an **admin-only** tool: it backs the admin menu item "Delete Subscription" in `NavUser.vue`. A caller without `User::ROLE_ADMIN` gets 403, a user without an active subscription a 422 (it used to crash with a 500). ### Tests [`tests/Feature/SubscriptionOwnershipTest.php`](tests/Feature/SubscriptionOwnershipTest.php), 23 tests. 12 of the first 19 failed on the unfixed code (no login needed, another user's id accepted, trial routes present); the owner paths pass before and after. Verified by mutation: dropping the check on cancel, on the invoice, or the admin check on `delete-trial` fails the matching tests. ### Not part of this fix `getUserPaymentInformation` and the daily job still reconcile with Mollie as before; the money-moving logic itself is unchanged. Open: see SEC-012. --- ## SEC-012: unauthenticated helper routes act on real data **Priority:** P0 (some of them) **Status:** Found by code review 2026-10-02 (`php artisan route:list`: only the `web` or `api` group, no login); nine more routes added 2026-10-06 (second table). **Partly fixed 2026-10-06** on branch `security/sec-012-remove-data-migration-routes`: the four routes that overwrite every poll or design are removed (`/delete-designs`, `/update-design/{id}`, `/update-choice-with-default-value`, `/update-user-in-design/{id}`), with a regression test; everything else in this entry is still open. Not reproduced on purpose: a GET on several of them runs the action or returns personal data. Whether the web server blocks them in production is not known. | Route | What it does | |---|---| | `GET /migrate` | runs `php artisan migrate` | | `GET /clear_cache`, `/storage_link` | run Artisan commands | | `GET /get-last-user/{email}` | returns the user record with subscriptions for any email, or the newest user for `-1` | | `GET /update-memberships-premium-price/{price}/{vat_amount}/{interval}` | changes the price of the premium membership | | `GET /sent-mail/{email}` | sends a payment confirmation mail to any registered address | | `GET /getPayment` | downgrades users whose subscription ended (`updateLatestPaymentStatus`) and returns those subscription rows (user ids, dates) to the caller | | `GET /api/sync-invoice` | syncs Mollie payments into invoices for every subscription | | `GET /api/get-subscription` | debug call with a fixed Mollie payment id | | `GET /get-designs`, `/copy-design-config/{from}/{to}`, `/delete-customer/{email}`, `/verfiy-email/{email}`, `/import-users`, `/convert-users`, `/import-contact` | other debug and import helpers | **Added 2026-10-06** (found while checking how poll links are built; all closures in `routes/web.php`, same state on `master` and `vue3-redesign`). The first four overwrite data of **every** poll or design with one GET: | Route | What it does | |---|---| | `GET /delete-designs` | deletes every row of `designs` (`Design::query()->delete()`) | | `GET /update-design/{id}` | sets `design_id` of every poll to `{id}` | | `GET /update-choice-with-default-value` | overwrites `choice_config` of every poll with a fixed default, so all custom answer labels are lost | | `GET /update-user-in-design/{id}` | makes `{id}` the owner of every default design (`-1`: a fixed account) | | `GET /get-poll-with-default-bg` | despite the name, rewrites `bg_image_path` of matching polls | | `GET /update-poll-options` | sets `options.initialised = true` on polls created since 2023-03-13 | | `GET /time` | returns full poll rows (no `$hidden` on `Poll`) for polls whose deadline is 1, 3 or 5 days away, including the raw `key`. Turning it into an admin link also needs the hashids salt, but titles, owners and options are exposed | | `GET /admin/test-db` | returns the database name and row counts | | `GET /send-admin-links` | mails the admin link of poll 1 to its owner | `GET /admin/reports/polls` from the same file is fine: `PollReportController::index()` checks the admin role. ### Fix (proposed) - Decide per route whether it is still used (some may be deploy or ops steps: `Hosting.md` mentions `storage_link`). Remove the unused ones; put the rest behind a login with `User::ROLE_ADMIN`, or turn them into Artisan commands. - Check the web server configuration for these paths first; if they are blocked there, the risk is lower but the code should not rely on it. - The one-off data migrations among them (`/delete-designs`, `/update-design/{id}`, `/update-choice-with-default-value`, `/update-user-in-design/{id}`, `/get-poll-with-default-bg`, `/update-poll-options`, `/copy-design-config/...`) have done their job and should simply be removed; they are the most urgent, because one GET (also from a crawler or a link preview) damages every poll. - Regression test: every route of this list answers 401/403/404 without an admin login. - Guard against new ones: a test over `Route::getRoutes()` that fails when a GET route outside an allow-list has no `auth` middleware and is not the frontend catch-all. ## SEC-013: archive and delete a poll without login or ownership **Priority:** P0 **Status:** Verified, not fixed (found 2026-09-24 while building the v3 dashboard; numbered SEC-010 on `vue3-redesign` until `master` was merged in, renumbered because `master` had used SEC-010 for the designs routes) `routes/api.php` puts `PUT /poll/{id}/archive` and `DELETE /poll/{id}` in the `['hashids', 'logger']` group, the public poll link group, with no `auth:sanctum`. `PollController::archivePoll()` and `deletePoll()` look the poll up by id and act on it; neither checks who is asking. - `archivePoll` toggles `isArchive`. The owner's dashboard hides it (legacy shows archived polls only behind a checkbox). - `deletePoll` sets `type = 0`, which removes the poll from every dashboard (`getMyPolls` lists `type > 0`), and deletes its tasks. It is a soft delete, so the data is recoverable by hand, but the owner sees the poll vanish. - `archivePoll` also returns the whole poll row to the caller, including `key`. The poll hashid is in every poll link, so anyone who ever received a link (every participant, everyone a link was forwarded to) can hide or remove the owner's poll, logged in or not. Reproduced with a throwaway feature test against the real routes and middleware (test database, file deleted afterwards): an anonymous `PUT` returned 200 and the poll was archived; a logged-in stranger's `PUT` returned 200 and archived it; an anonymous `DELETE` returned 200 and set `type` to 0. Related and not yet checked: the other mutating routes in the same group (`POST /poll/details/{id}`, `POST /poll/choices/{id}`, `POST /poll/options/{id}`) may have the same gap. Worth reading them before choosing the fix. Fix idea: move both routes behind `auth:sanctum` and allow only the poll's owner or a sub-admin, using the resolver `PollAccessResolver` already shared by the poll endpoints. Answer 403 otherwise. Return only what the UI needs from `archivePoll` (`isArchive`), not the row. The legacy dashboard shows the archive icon on every card, including polls created by others, so it should show it for owners only. The v3 dashboard already does. ## SEC-014: Premium is only partly enforced by the server **Priority:** P1 **Status:** Partly fixed 8 Oct 2026 (the four routes below); the rest is open Premium used to be a frontend rule. The server enforced it in two places: the invitation count, with `roles != 1` (which counted an admin, 256, or premium + beta, 3, as free), and the image upload count, which reads the user's first subscription instead of `roles`. **Done (branch `vue3-redesign`, 8 Oct 2026):** `User::isPremium()` is the one rule (premium bit or admin bit, also next to other bits). These routes now answer 402 `{error: {message: 'premium_required', feature}}` when the poll's owner is not Premium: switching `options.hiddenPoll` on (`postDetails`, `postOptions`, `postNew`), `POST /download-excel/{id}`, `POST /invitation` above `app.free_invite_limit` (50, the poll's participants counted in), and `POST /reminder-to-participant`. Tests: `tests/Feature/PremiumEnforcementTest.php`. The "SPAM Alert" mail to support for a free user over the limit is gone; a free user reaching the limit is not a spam case. Classic shows the refusals too (parity plan, P10); before that it would have reported a refused invitation as sent. **Still open:** - All of these routes work with a poll id and need no login, so the owner's Premium is checked, not the caller's. Anyone holding a poll id can still call them. That is SEC-013 and finding G16 (the reminder route does not check that the sender owns the poll), not part of this item. - `UserController::imageUpload` limits by the first subscription, not by `roles`; `logoUpload` and `POST /poll/logo/{id}` have no Premium check, and neither does the `/designs` resource (`THEMING-AND-BRANDING.md` asks for one). - The sub-admin flag (`is_sub_admin` in `ParticipantController::updateParticipant`) is not checked against Premium on the server; only the frontends gate it. Downgrading clears the flags, but `User::downGradeUserRole()` iterates a query builder instead of `->get()`, so it probably clears none. - Granting and revoking Premium use different rules in three places (`roles += 1` only when `roles` is 0 or 256; `roles -= 1` with different guards in `CheckAndCreateSubscription` and `PaymentController`). See `MOLLIE.md`. - The Excel export in the new UI is built in the browser from data the visitor already has, so only the server route that classic uses is guarded; the new UI's lock is advisory.