[patrickbradley-cog]

Summary

Linear: COG-1155 · W1 forward-compatible refactor, still on Boot 1.5.4 / Java 8 (no version bump).

Recipient delete was a GET mutation, so once CSRF is enabled (W2) it would still be unprotected (CsrfFilter skips safe methods). This PR makes it POST now, while CSRF is still disabled. For users nothing changes.

• TransferController: GET /transfer/recipient/delete?recipientName=… → @PostMapping("/recipient/delete") (under the class-level /transfer), still taking @RequestParam recipientName. The other 7 @RequestMapping(value=…, method=…) handlers become @GetMapping/@PostMapping with the same paths. No trailing-slash variant was added.
• recipient.html: the <a th:href=".../delete?recipientName=…"> link is now a <form th:action="@{/transfer/recipient/delete}" method="post"> with a hidden recipientName input and a btn btn-link submit, so it still looks like a link. It uses th:action so that Spring Security's CsrfRequestDataValueProcessor adds _csrf automatically once W2 enables CSRF.

Merge order: merge this behind the money smoke suite (#106). MoneyMovementSmokeTest exists only in #106, not on master, so this PR can't edit it without overlapping that PR's files. That test needs this one-line change (get( → post() to pass on top of this PR. It is already anticipated in #106's comment:

-		// Delete is a GET today; COG-1155 converts it to a POST and updates this test.
-		mockMvc.perform(get("/transfer/recipient/delete").with(user(username)).param("recipientName", recipientName))
+		// Delete is a POST (COG-1155).
+		mockMvc.perform(post("/transfer/recipient/delete").with(user(username)).param("recipientName", recipientName))

Whichever PR lands second should apply it (a comment has been posted on #106).

Verification (Temurin 8, MySQL 5.7 in docker)

Build + existing tests: mvn -B -f UserFront/pom.xml verify -Dspring.datasource.url='jdbc:mysql://localhost:3306/onlinebanking?useSSL=false'

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO] Building jar: .../UserFront/target/userFront-0.0.1-SNAPSHOT.jar
[INFO] BUILD SUCCESS

(useSSL=false: connector 5.1.42 can't do a TLS handshake on modern JDK 8. MySQL 8 is incompatible with this connector (query_cache_size), so 5.7 was used, the same image as #106's Testcontainers setup.)

Money smoke gate: #106 + this commit + the diff above, mvn -B -f UserFront/pom.xml verify -Dtest=MoneyMovementSmokeTest:

Tests run: 9, Failures: 0, Errors: 0, Skipped: 0 - in com.userFront.MoneyMovementSmokeTest
[INFO] BUILD SUCCESS

(On this VM's Docker 29, Testcontainers 1.19.7 needed ~/.docker-java.properties → api.version=1.44. That is only a local environment workaround.)

Manual flow against the running jar (curl):

POST /signup: 302 -> http://localhost:8080/
POST /index (login): 302 -> http://localhost:8080/userFront
POST /transfer/recipient/save: 302 -> http://localhost:8080/transfer/recipient
--- GET /transfer/recipient, delete cell:
    <form method="post" style="margin: 0;" action="/transfer/recipient/delete">
        <input type="hidden" name="recipientName" value="Payeecog1155u10989" />
        <button type="submit" class="btn btn-link" ...>delete</button>
    </form>
--- DB: select name from recipient → Payeecog1155u10989
GET /transfer/recipient/delete?recipientName=... (old link): 405
--- DB after GET (row still present) → Payeecog1155u10989
POST /transfer/recipient/delete recipientName=...: 200
--- DB after POST: count = 0 ; payee rows in returned page: 0
GET /transfer/betweenAccounts: 200 | GET /transfer/recipient: 200 | GET /transfer/toSomeoneElse: 200 | GET /transfer/recipient/edit?recipientName=x: 200

(A fresh DB has no ROLE_USER row, so signup fails until one is inserted. That problem exists on master already and is not caused by this PR. role was seeded by hand for this test.)

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/3ba0f073ef3e4aa3835f31c1c2399cdb
Open in Devin Desktop: https://app.devin.ai/desktop/session/3ba0f073ef3e4aa3835f31c1c2399cdb?variant=devin
Requested by: @patrickbradley-cog

---

Devin Review

Status	Commit
⚪ Not started	—

Run Devin Review

---

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 2 potential issues.

[devin-ai-integration]

🟡 Refreshing recipient page repeats deletion

After a delete POST, refreshing the returned page resubmits recipientDelete. Because deleteRecipientByName uses the name again, a newly added recipient with that name gets deleted.

Learn more

The delete form now submits a POST, but the handler renders recipient as its response. Browsers can resend that POST when refreshing the resulting page. deleteRecipientByName deletes by name rather than by a unique ID, so a second submission can remove a replacement recipient.

Example: A user deletes a recipient named Alex, adds another Alex in a second tab, then refreshes the first tab and confirms resubmission. The new Alex disappears instead of remaining in the recipient list.

Recommended fix: Return redirect:/transfer/recipient after the deletion instead of rendering recipient. The existing GET handler loads the updated list; refreshing its response will only repeat the GET.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Agreed that PRG (redirect:/transfer/recipient) is the better pattern. I'm deliberately leaving it out of this PR: the W1 contract is behaviour-neutral, and the money smoke test in #106 asserts status().isOk() + view().name("recipient") for delete, so switching to a redirect would change that recorded contract. On master, refreshing after a GET delete also repeated the delete. I'm flagging it to the coordinator as a follow-up, which should update the smoke assertion in the same change.

[devin-ai-integration]

🟥 Other users' recipients can be deleted

An authenticated user can POST another user's name to recipientDelete and remove that recipient. deleteRecipientByName deletes globally by name; listing recipients afterward does not limit the deletion.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Confirmed, and it is a real problem, but it is not new in this PR. master already had it on the GET mapping: TransactionServiceImpl.deleteRecipientByName → recipientDao.deleteByName deletes by name globally. This happens with findRecipientByName too (edit, and toSomeoneElse transfers). COG-1155 is limited to changing the HTTP method (a W1 behaviour-neutral refactor), and the money smoke suite (#106) records current behaviour, so I have not scoped the service/DAO by owner here. I'm reporting it to the coordinator so it can get its own ticket.

[devin-ai-integration]

Browser check of COG-1155 at 7348dd2: UserFront running locally on JDK 8 against MySQL 5.7, signed in as a test user.

• Created two recipients. Both delete controls still look like links.
• Clicking delete removed only COG1155Delete. COG1155Keep was still there in the UI and in MySQL.
• Opening the old GET delete URL gave 405 Method Not Allowed. The remaining recipient was still there after reloading the page and in the DB.
• Other pages still work: clicking a recipient name opens the edit form with its saved values, and the To Someone Else and Between Accounts forms load.

Before deletion	After deletion

Old GET rejected	Recipient survives GET

Recording:

Not tested: money-transfer submissions and behaviour with CSRF turned on (W2).

Written by Devin