[elhoim]

BLUF — delete_user_setting sent the setting name as a row id, so every call returned HTTP 400.

• Problem — delete_user_setting in src/client.rs sent the request to userSettings/delete/{setting}, but UserSettingsController::delete($id) throws BadRequestException on a non-numeric id, so every invocation failed.
• Fix — POST to the bare userSettings/delete path with setting and an optional user_id in the JSON body, matching MISP core and PyMISP; the existing wiremock test was corrected in place and a user_id variant added.
• Effect — Deleting a user setting works instead of always failing, and the test suite now pins the real wire contract.
• CI — Red from a repo-level CI bug (ubuntu-22.04 has no php8.3 mod_php; test_event_crud_lifecycle raced the publish worker), fixed in unmerged PR fix: [ci] Run integration tests on ubuntu-24.04 to match php8.3 #53; not caused by this PR.

Finding

delete_user_setting puts setting name in userSettings/delete/{setting} -> HTTP 400
UserSettingsController::delete($id=false) requires numeric row id, or empty $id with setting/user_id in the POST body.

Change

delete_user_setting previously PUT/POSTed to userSettings/delete/{setting}[/{uid}], placing the non-numeric setting name where MISP's controller expects a numeric row id, causing a guaranteed HTTP 400 (BadRequestException "Invalid ID passed."). Fixed to POST to the bare userSettings/delete path with {"setting": ..., "user_id"?: ...} in the JSON body, matching MISP core and PyMISP. Corrected the existing wiremock test in place (was asserting the wrong URL) to check both the corrected path and the exact request body, and added a second test for the user_id variant.

Upstream evidence

MISP: app/Controller/UserSettingsController.php:318-347 — delete($id=false): if $id is empty, requires $this->request->data['setting'] and optionally 'user_id' from the POST body to build query conditions; if $id is non-empty and non-numeric, throws BadRequestException('Invalid ID passed.') (line 346) which yields HTTP 400. PyMISP: pymisp/api.py:3742-3752 — delete_user_setting() builds query={'setting':..., 'user_id'?:...} and POSTs to 'userSettings/delete' (bare path, no id segment), confirming the body-based contract.

Verified against MISP core 843b6744 and PyMISP e7debb4f.

Tests

• delete_user_setting_sends_request (corrected in place: now asserts POST /userSettings/delete with body_json {"setting":"dashboard"} instead of the old wrong path userSettings/delete/dashboard)
• delete_user_setting_with_user_id_sends_request (new: asserts POST /userSettings/delete with body_json {"setting":"dashboard","user_id":5})

Note on the original finding

Accurate, and slightly understated: the finding described the URL-path defect for the general case, but both call shapes (with and without user_id) were broken the same way — every invocation of delete_user_setting would 400, since the setting name (a non-numeric string) was always placed where MISP's controller requires either a numeric row id or an empty segment. No case worked before the fix.

Caveats

Could not run cargo build/test/clippy per task constraints (centrally serialized); verified correctness by reading the pinned MISP controller and PyMISP source directly and by inspecting client.rs's post()/request() helper to confirm the JSON body is sent as-is (no extra injected fields), so the wiremock body_json exact-match tests are valid. Separately, while reviewing this area I noticed set_user_setting (src/client.rs, a few lines above delete_user_setting) may have an analogous URL-ordering bug: MISP's setSetting action signature is setSetting($user_id=false, $setting_name=false) — first URL segment is user_id, not setting — but rustmisp's client builds userSettings/setSetting/{setting} when user_id is None, and userSettings/setSetting/{uid}/{setting} otherwise. I did NOT touch this — it's outside F03's scope and is very likely covered by a separate finding — but flagging it here so the orchestrator confirms it's tracked.

---

Verified locally: cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings, cargo test --all-features --lib, --test integration_tests --no-run, and cargo check --examples, each in an isolated target directory.

🤖 Generated with Claude Code