Delete ssh keys shouldn't be a POST & Cleanup routes

[rmartinoscar]

BREAKING API CHANGE
POST client/account/ssh-keys/remove (fingerprint body) => DELETE client/account/ssh-keys/{fingerprint}

[coderabbitai]

📝 Walkthrough

Walkthrough

Route groupings were restructured (API keys and network allocations moved under nested prefixes) and the SSH key deletion endpoint was changed from POST with a fingerprint in the request body to DELETE with the fingerprint in the URL; the SSHKeyController method signature was updated accordingly and tests adjusted.

Changes

Cohort / File(s)	Summary
Route Organization routes/api-client.php	Reorganized route groups: API keys moved under an /account/api-keys grouping; network allocations routing consolidated under a /servers/{server}/network/allocations style grouping (grouping prefixes changed, final endpoints preserved). SSH key routes moved under /account/ssh-keys with delete endpoint now mapped to DELETE /account/ssh-keys/{fingerprint}.
SSH Key Controller app/Http/Controllers/Api/Client/SSHKeyController.php	Updated delete method signature to accept string $fingerprint; removed request-body fingerprint validation; lookups use where('fingerprint', $fingerprint)->firstOrFail(); logs activity and unconditionally deletes the found key; returns HTTP 204.
SSH Key Tests tests/Integration/Api/Client/SSHKeyControllerTest.php	Tests updated to call DELETE /api/client/account/ssh-keys/{fingerprint} (URL param) and assert 204 on first deletion (soft-delete) and 404 on subsequent attempt; removed prior POST-based validation steps that expected 422 for missing fingerprint.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API
    participant SSHController
    participant DB

    Note over Client,SSHController: SSH key deletion (new flow)

    Client->>API: DELETE /api/client/account/ssh-keys/{fingerprint}
    API->>SSHController: route -> delete(Request, fingerprint)
    SSHController->>DB: find key where fingerprint = {fingerprint}
    alt key found
        SSHController->>SSHController: log activity (fingerprint)
        SSHController->>DB: delete (soft-delete)
        SSHController-->>API: 204 No Content
        API-->>Client: 204 No Content
    else key not found
        DB-->>SSHController: not found -> firstOrFail() throws
        SSHController-->>API: 404 Not Found
        API-->>Client: 404 Not Found
    end

Loading

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly reflects the main changes: converting SSH key deletion from POST to DELETE and reorganizing routes.
Description check	✅ Passed	The description clearly documents the breaking API change with the old and new endpoint formats, directly related to the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 162887f and d98aff4.

📒 Files selected for processing (3)

• app/Http/Controllers/Api/Client/SSHKeyController.php (1 hunks)
• routes/api-client.php (2 hunks)
• tests/Integration/Api/Client/SSHKeyControllerTest.php (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• routes/api-client.php

🧰 Additional context used

🧬 Code graph analysis (2)

app/Http/Controllers/Api/Client/SSHKeyController.php (4)

app/Http/Controllers/Api/Client/ApiKeyController.php (1)

• delete (65-80)

app/Http/Controllers/Api/Client/Servers/ScheduleController.php (1)

• delete (177-184)

app/Models/User.php (1)

• sshKeys (248-251)

app/Services/Activity/ActivityLogService.php (3)

• event (48-53)
• property (109-117)
• log (136-155)

tests/Integration/Api/Client/SSHKeyControllerTest.php (1)

app/Http/Controllers/Api/Client/SSHKeyController.php (1)

• delete (58-73)

🔇 Additional comments (2)

tests/Integration/Api/Client/SSHKeyControllerTest.php (1)

35-52: LGTM! Test correctly implements the new DELETE endpoint pattern.

The test properly exercises the new RESTful DELETE endpoint with the fingerprint as a URL parameter. The updated assertions correctly expect:

• HTTP 204 No Content for successful deletion of the user's own key
• HTTP 404 Not Found when attempting to delete another user's key (authorization check)
• Proper soft-deletion behavior

This aligns perfectly with the controller's use of firstOrFail() and follows REST conventions.

app/Http/Controllers/Api/Client/SSHKeyController.php (1)

58-73: LGTM! Clean implementation following established patterns.

The refactored delete method correctly implements the breaking API change:

1. Route parameter binding (line 58): Accepts $fingerprint as a typed parameter, eliminating the need for request body validation
2. Cleaner error handling (line 63): firstOrFail() automatically returns 404 for non-existent or unauthorized keys, removing the need for manual null checks
3. Activity logging (lines 65-68): Properly logs the deletion event before removing the key, consistent with the ApiKeyController::delete() pattern
4. RESTful response (line 72): Returns HTTP 204 No Content as expected

This implementation maintains consistency with similar deletion methods in the codebase and provides better error semantics (404 Not Found instead of 422 Unprocessable Entity for invalid identifiers).

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

routes/api-client.php (1)

36-40: Critical: Route parameter doesn't match controller signature.

The route defines /{identifier} as a URL parameter, but the SSHKeyController::delete() method doesn't accept this parameter. The controller signature is:

public function delete(ClientApiRequest $request): JsonResponse

It should be updated to match the pattern used in ApiKeyController::delete():

public function delete(ClientApiRequest $request, string $identifier): JsonResponse

Additionally, the controller still validates and reads the fingerprint from the request body ($request->input('fingerprint')) instead of using the route parameter. This needs to be updated to use the $identifier parameter from the URL.

See the detailed fix in the review comment for tests/Integration/Api/Client/SSHKeyControllerTest.php lines 43-56.

🧹 Nitpick comments (1)

routes/api-client.php (1)

96-102: LGTM: Network allocations routes reorganization.

The flattening of nested route groups into a single /network/allocations prefix is a good refactor. The final endpoint URLs remain unchanged, and this improves readability.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cdccca8 and 162887f.

📒 Files selected for processing (2)

• routes/api-client.php (2 hunks)
• tests/Integration/Api/Client/SSHKeyControllerTest.php (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

routes/api-client.php (2)

app/Http/Controllers/Api/Client/ApiKeyController.php (1)

• ApiKeyController (13-81)

app/Http/Controllers/Api/Client/SSHKeyController.php (1)

• SSHKeyController (12-78)

🔇 Additional comments (1)

routes/api-client.php (1)

30-34: LGTM: API keys routes properly grouped.

The reorganization under the /api-keys prefix is correct and consistent with the controller implementation. The ApiKeyController::delete() properly accepts the $identifier route parameter.