Skip to content

fix: filter client get/delete by client_id client-side - #34

Merged
NathaelB merged 1 commit into
mainfrom
fix/client-get-filters-by-client-id
Sep 2, 2026
Merged

fix: filter client get/delete by client_id client-side#34
NathaelB merged 1 commit into
mainfrom
fix/client-get-filters-by-client-id

Conversation

@NathaelB

@NathaelB NathaelB commented Sep 2, 2026

Copy link
Copy Markdown
Member

Bug

client get and client delete acted on the wrong client: GET /realms/{realm}/clients ignores the clientId query filter and always returns the full list, so get_client kept the first element regardless of what was asked for. delete_client resolves its target UUID through the same call, making the mismatch destructive — e.g. client delete zukquote-realm could delete security-admin-console instead.

Fix

  • get_client now fetches the full client list and matches client_id client-side, instead of trusting the (ignored) server-side filter. Still errors with ClientNotFound when nothing matches.
  • delete_client's confirmation prompt now runs after resolving the client, and names the resolved client_id + uuid — so what's shown is what gets deleted, not just what was typed.

Issue

Closes #20.

Verification

Reproduced live against a local FerrisKey server: created two throwaway clients (smoke-client-a, smoke-client-b) in a realm where security-admin-console sorts first — confirmed client get smoke-client-b used to matter (would have returned security-admin-console before the fix, per the bug report's exact mechanism). After the fix: client get smoke-client-b returns the right client, client delete smoke-client-b --force deletes only that one, security-admin-console/smoke-client-a/others untouched. Cleaned up both test fixtures afterward.

Test plan

  • cargo build --workspace
  • cargo test --workspace (68 passed)
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • Manual reproduction + fix verification against a running FerrisKey server (see above)

Summary by CodeRabbit

  • Bug Fixes
    • Improved client lookup to select the exact matching client, preventing incorrect results when similar client identifiers exist.
    • Improved client deletion accuracy by resolving the selected client before confirmation and deletion.
    • Confirmation prompts and success messages now display the resolved client identifier and unique record ID for greater clarity.

GET /realms/{realm}/clients ignores the clientId query filter and
always returns the full list, so get_client kept whatever the server
happened to return first — usually not the requested client. client
delete resolved its target the same way, making the mismatch
destructive: 'client delete zukquote-realm' could delete an unrelated
client instead.

Filter the list client-side by client_id, error when nothing matches,
and move delete's confirmation prompt after resolution so it names the
client actually about to be deleted.
@NathaelB NathaelB self-assigned this Sep 2, 2026
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 31c6201a-8b25-447e-b549-8f34fce482d1

📥 Commits

Reviewing files that changed from the base of the PR and between 410ab62 and a83a70b.

📒 Files selected for processing (2)
  • libs/ferriskey-cli-client/src/lib.rs
  • libs/ferriskey-cli-core/src/client.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The CLI now performs exact client ID matching from the full realm client list. Client deletion uses the resolved client UUID and ID for lookup, confirmation, deletion, and success output.

Changes

Client resolution

Layer / File(s) Summary
Exact client lookup
libs/ferriskey-cli-client/src/lib.rs
get_client now lists realm clients and returns the first exact client_id match.
Resolved client deletion
libs/ferriskey-cli-core/src/client.rs
delete_client now derives the UUID and displayed client ID from the resolved client record. The confirmation prompt and success message show both values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to a83a7

Client lookup and deletion now target the exact requested client and show the resolved identity before deletion. The change is mergeable with owner awareness that correctness still depends on the backend returning a complete, uniquely identifiable client list.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: client-side filtering by client_id for client retrieval and deletion.
Linked Issues check ✅ Passed The changes satisfy issue #20. get_client now matches client_id against the full client list, and delete_client resolves the client before confirmation and uses the resolved client_id and UUID.
Out of Scope Changes check ✅ Passed The changes are limited to the client lookup and deletion flows required by issue #20. No unrelated changes are identified.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/client-get-filters-by-client-id

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

Comment @coderabbitai help to get the list of available commands.

@NathaelB
NathaelB merged commit 399ff7c into main Sep 2, 2026
3 of 4 checks passed
@NathaelB
NathaelB deleted the fix/client-get-filters-by-client-id branch September 2, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

client get and client delete act on the wrong client

1 participant