fix: accept HIGH security level keys for profile updates

[thepastaclaw]

Issue

Closes #760 — Editing a profile with a wallet-imported identity fails with "This identity is missing an authentication key required for this operation."

Root Cause

update_profile() in src/backend_task/dashpay/profile.rs only looked for authentication keys at SecurityLevel::CRITICAL, but Platform's default document security level requirement is HIGH. Wallet-imported identities typically have HIGH-level keys, not CRITICAL, so the key lookup failed.

Fix

Accept both CRITICAL and HIGH security levels when searching for a signing key in update_profile(), consistent with how contact_requests.rs already handles authentication key lookup (line 639).

Testing

• cargo check, cargo clippy — clean

Summary by CodeRabbit

• Bug Fixes
  • Expanded authentication key selection criteria during profile updates to support additional security levels, improving flexibility for users managing their account security.

[coderabbitai]

Warning

Review limit reached

@thepastaclaw, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 57 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 161a94b2-763e-4096-ac9d-2e8775e821e1

📥 Commits

Reviewing files that changed from the base of the PR and between 921c1de and 5d04b88.

📒 Files selected for processing (1)

• src/backend_task/dashpay/profile.rs

📝 Walkthrough

Walkthrough

The identity signing key selection logic in update_profile is modified to accept both SecurityLevel::CRITICAL and SecurityLevel::HIGH keys for Purpose::AUTHENTICATION, instead of only SecurityLevel::CRITICAL. The remainder of the control flow and error handling remain unchanged.

Changes

Cohort / File(s)	Summary
Authentication Key Selection src/backend_task/dashpay/profile.rs	Modified update_profile to permit both SecurityLevel::CRITICAL and SecurityLevel::HIGH keys for Purpose::AUTHENTICATION, expanding from the prior CRITICAL-only restriction.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A key, a door, a profile too,
HIGH and CRITICAL both will do,
The vault unlocks with gentler care,
Authentication's now more fair! ✨🔑

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	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 and accurately summarizes the main change: allowing HIGH security level keys alongside CRITICAL for profile updates.
Linked Issues check	✅ Passed	The code change directly addresses issue #760 by accepting both CRITICAL and HIGH security level keys, matching Platform defaults and enabling wallet-imported identities to update profiles.
Out of Scope Changes check	✅ Passed	The change is narrowly scoped to the security level check in update_profile(), directly addressing the linked issue without introducing unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[lklimek]

claw please find and quote documents (eg. DIP) defining which keys we should use

[thepastaclaw]

Security Level Requirements — DIP-0011 + Platform Contract Defaults

DIP-0011 (Identities) — Key Security Levels

DIP-0011 § Keys defines the security levels and their intended use:

Security Level	Examples	Worst Result if Compromised
Master	Managing Dash Identity	Loss of Identity
Critical	Exchange of assets (e.g., property deed)	Loss of funds/assets
High	Publishing a message on a social network. Sending a contact request in DashPay.	Loss of reputation. Risk of inducing spam.
Medium	Routine background transitions for application metadata	Loss of credits. Loss of anonymity.

DIP-0011 also states:

"Contracts can define for each document type the minimal security level required for signing. Only keys of a higher security level can be used. If no such field is present then all keys except the medium security level keys can be used."

Platform Default Security Level

In the Platform codebase, the document type schema parser sets a default when no signatureSecurityLevelRequirement is specified:

// packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
let security_level_requirement = schema
    .get_optional_integer::<u8>(property_names::SECURITY_LEVEL_REQUIREMENT)
    .map_err(consensus_or_protocol_value_error)?
    .map(SecurityLevel::try_from)
    .transpose()?
    .unwrap_or(SecurityLevel::HIGH);  // <-- default is HIGH

DashPay Contract — Profile Document Type

The DashPay contract schema (schema/v1/dashpay.schema.json) does not specify a signatureSecurityLevelRequirement for the profile document type, so it inherits the Platform default of HIGH.

This means HIGH or higher (i.e., CRITICAL or MASTER) keys are valid for signing profile updates. The original code only accepted CRITICAL, which was overly restrictive — wallet-imported identities that only carry HIGH-level authentication keys would fail.

Consistency

This PR aligns update_profile() with how contact_requests.rs (line 639) already handles it:

HashSet::from([SecurityLevel::CRITICAL, SecurityLevel::HIGH]),

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

Code Review

Validated the available reviewer output against commit 921c1de and found no code-backed issues to raise on this SHA.

Reviewed commit: 921c1de

[thepastaclaw]

✅ Review complete (commit 5d04b88)

[thepastaclaw]

Code Review

The fix in update_profile() correctly adds SecurityLevel::HIGH to the AUTHENTICATION key matching set, mirroring the established pattern at contact_requests.rs:639. The change is minimal, focused, and correct. Both reviewers converge on the same gap: no automated coverage for the HIGH-only key path. A secondary nit notes growing duplication of the key-selection policy across three DashPay call sites.

Reviewed commit: 921c1de

🟡 1 suggestion(s) | 💬 1 nitpick(s)

2 additional findings

🟡 suggestion: HIGH-only authentication-key path has no regression test

src/backend_task/dashpay/profile.rs (lines 122-130)

The fix changes the key-selection filter in update_profile() to accept SecurityLevel::HIGH — the entire bug fix from #760 — but no test exercises an identity carrying only HIGH-level AUTHENTICATION keys (the wallet-imported case). Without coverage, a future refactor could narrow the filter again and cargo test/clippy would still pass. Per CLAUDE.md, new backend logic should have unit/integration tests. Add a focused unit test that constructs a QualifiedIdentity with only a HIGH-level AUTHENTICATION key and asserts that update_profile()'s key-lookup step succeeds (does not return MissingAuthenticationKey). A kittest scenario driving the profile edit flow with a wallet-imported identity would also work.

💬 nitpick: DashPay key-selection policy duplicated across three call sites

src/backend_task/dashpay/profile.rs (lines 121-130)

profile.rs (122-130, post-fix), contact_requests.rs (635-644), and contact_info.rs (391-403) each independently call get_first_public_key_matching with overlapping but slightly different SecurityLevel sets — profile/contact_requests use CRITICAL+HIGH while contact_info uses CRITICAL+HIGH+MEDIUM and is also restricted to ECDSA_SECP256K1. This drift is exactly what produced the bug being fixed. A small helper (e.g. signing_key_for_dashpay_document(identity, doc_type) -> Result<&IdentityPublicKey, DashPayError>) would centralise the policy and make the relationship to Platform's required document security level explicit. Out of scope for a one-line fix, but worth a follow-up.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/backend_task/dashpay/profile.rs`:
- [SUGGESTION] lines 122-130: HIGH-only authentication-key path has no regression test
  The fix changes the key-selection filter in update_profile() to accept SecurityLevel::HIGH — the entire bug fix from #760 — but no test exercises an identity carrying only HIGH-level AUTHENTICATION keys (the wallet-imported case). Without coverage, a future refactor could narrow the filter again and `cargo test`/`clippy` would still pass. Per CLAUDE.md, new backend logic should have unit/integration tests. Add a focused unit test that constructs a QualifiedIdentity with only a HIGH-level AUTHENTICATION key and asserts that update_profile()'s key-lookup step succeeds (does not return MissingAuthenticationKey). A kittest scenario driving the profile edit flow with a wallet-imported identity would also work.

[thepastaclaw]

Code Review

Both reviewers converge on a single nitpick: the CRITICAL+HIGH AUTHENTICATION key-selection policy newly centralized in profile.rs::profile_authentication_key is still hand-rolled in contact_requests.rs and a related variant in contact_info.rs. The prior test-coverage gap is fully resolved by the new unit tests in profile.rs. No blocking issues.

Reviewed commit: 5d04b88

💬 1 nitpick(s)

[thepastaclaw]

💬 Nitpick: DashPay AUTHENTICATION key-selection policy still duplicated across modules

This PR introduces profile_authentication_key() in profile.rs to centralize the AUTHENTICATION + {CRITICAL, HIGH} selection rule, but the identical policy is still open-coded in contact_requests.rs:635-643, and a related CRITICAL+HIGH+MEDIUM/ECDSA_SECP256K1-only variant lives in contact_info.rs:391-403. This is exactly the drift pattern that caused #760 — profile.rs previously accepted only CRITICAL while contact_requests.rs already accepted CRITICAL+HIGH. Keeping the helper module-private in profile.rs makes it easy for the next contributor to miss when Platform's required document security level changes again. Consider promoting the helper to a shared DashPay module (e.g. dashpay::keys::document_signing_key(identity)) and reusing it from contact_requests.rs, with contact_info.rs either using the same helper or a clearly-named sibling that documents why it allows MEDIUM and restricts to ECDSA_SECP256K1. Out of scope for a one-line fix — not blocking.

_{source: ['claude', 'codex']}