Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: extra logging in OtherUserIdentity #4415

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Dec 13, 2024

Add some extra logging in these two methods, to try to narrow down a bug report
we received.

  • Public API changes documented in changelogs (optional)

@richvdh richvdh requested review from a team as code owners December 13, 2024 17:29
@richvdh richvdh requested review from jmartinesp and andybalaam and removed request for a team December 13, 2024 17:29
@@ -392,6 +392,7 @@ impl OtherUserIdentity {

/// Pin the current identity (public part of the master signing key).
pub async fn pin_current_master_key(&self) -> Result<(), CryptoStoreError> {
info!(master_key = ?self.master_key.get_first_key(), "Pinning current identity for user '{}'", self.user_id());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sample output (on Element web):

 INFO matrix_sdk_crypto::identities::user: Pinning current identity for user '@g:xps9320.sw1v.org'
    master_key=Some("ed25519:pKj/3plhPwWMRhfe0F5CzrPk3RHzU0BPxBb/QpL0kNs")
    at /home/rav/work/matrix-rust-sdk/crates/matrix-sdk-crypto/src/identities/user.rs:395

Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.30%. Comparing base (8d2e672) to head (ed1178c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4415      +/-   ##
==========================================
- Coverage   85.31%   85.30%   -0.01%     
==========================================
  Files         282      282              
  Lines       31259    31261       +2     
==========================================
  Hits        26668    26668              
- Misses       4591     4593       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, assuming the logged key is safe to have in logs.

@richvdh richvdh enabled auto-merge (rebase) December 16, 2024 11:11
@richvdh
Copy link
Member Author

richvdh commented Dec 16, 2024

Approved, assuming the logged key is safe to have in logs.

It's someone else's cross-signing key, so by definition, a public key.

auto-merge was automatically disabled December 16, 2024 11:28

Rebase failed

Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, thanks!

Add some extra logging in these two methids, to try to narrow down a bug report
we received.
@richvdh richvdh force-pushed the rav/identity_pin_logging branch from 23e189a to ed1178c Compare December 16, 2024 12:42
@richvdh richvdh merged commit 2703f7f into main Dec 16, 2024
40 checks passed
@richvdh richvdh deleted the rav/identity_pin_logging branch December 16, 2024 12:59
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.

3 participants