Skip to content

Commit

Permalink
Merge branch '04-07-identityupdate_serialization' of github.com:xmtp/…
Browse files Browse the repository at this point in the history
…libxmtp into 04-07-identityupdate_serialization
  • Loading branch information
neekolas committed Apr 7, 2024
1 parent 78b417e commit 4744e6f
Show file tree
Hide file tree
Showing 11 changed files with 537 additions and 38 deletions.
2 changes: 1 addition & 1 deletion xmtp_id/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ tracing.workspace = true
thiserror.workspace = true
xmtp_cryptography.workspace = true
xmtp_mls.workspace = true
xmtp_proto.workspace = true
xmtp_proto = { workspace = true, features = ["proto_full"] }
openmls_traits.workspace = true
openmls.workspace = true
openmls_basic_credential.workspace = true
Expand Down
31 changes: 31 additions & 0 deletions xmtp_id/src/associations/association_log.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use super::hashes::generate_inbox_id;
use super::member::{Member, MemberIdentifier, MemberKind};
use super::serialization::{
from_identity_update_proto, to_identity_update_proto, DeserializationError, SerializationError,
};
use super::signature::{Signature, SignatureError, SignatureKind};
use super::state::AssociationState;

use thiserror::Error;
use xmtp_proto::xmtp::identity::associations::IdentityUpdate as IdentityUpdateProto;

#[derive(Debug, Error, PartialEq)]
pub enum AssociationError {
Expand All @@ -23,6 +27,8 @@ pub enum AssociationError {
LegacySignatureReuse,
#[error("The new member identifier does not match the signer")]
NewMemberIdSignatureMismatch,
#[error("Wrong inbox_id specified on association")]
WrongInboxId,
#[error("Signature not allowed for role {0:?} {1:?}")]
SignatureNotAllowed(String, String),
#[error("Replay detected")]
Expand Down Expand Up @@ -90,6 +96,7 @@ impl IdentityAction for CreateInbox {

/// AddAssociation Action
pub struct AddAssociation {
pub inbox_id: String,
pub new_member_signature: Box<dyn Signature>,
pub new_member_identifier: MemberIdentifier,
pub existing_member_signature: Box<dyn Signature>,
Expand All @@ -102,6 +109,7 @@ impl IdentityAction for AddAssociation {
) -> Result<AssociationState, AssociationError> {
let existing_state = maybe_existing_state.ok_or(AssociationError::NotCreated)?;
self.replay_check(&existing_state)?;
ensure_matching_inbox_id(&self.inbox_id, existing_state.inbox_id())?;

// Validate the new member signature and get the recovered signer
let new_member_address = self.new_member_signature.recover_signer()?;
Expand Down Expand Up @@ -186,6 +194,7 @@ impl IdentityAction for AddAssociation {

/// RevokeAssociation Action
pub struct RevokeAssociation {
pub inbox_id: String,
pub recovery_address_signature: Box<dyn Signature>,
pub revoked_member: MemberIdentifier,
}
Expand All @@ -197,6 +206,7 @@ impl IdentityAction for RevokeAssociation {
) -> Result<AssociationState, AssociationError> {
let existing_state = maybe_existing_state.ok_or(AssociationError::NotCreated)?;
self.replay_check(&existing_state)?;
ensure_matching_inbox_id(&self.inbox_id, existing_state.inbox_id())?;

if is_legacy_signature(&self.recovery_address_signature) {
return Err(AssociationError::SignatureNotAllowed(
Expand Down Expand Up @@ -238,6 +248,7 @@ impl IdentityAction for RevokeAssociation {

/// ChangeRecoveryAddress Action
pub struct ChangeRecoveryAddress {
pub inbox_id: String,
pub recovery_address_signature: Box<dyn Signature>,
pub new_recovery_address: String,
}
Expand All @@ -249,6 +260,7 @@ impl IdentityAction for ChangeRecoveryAddress {
) -> Result<AssociationState, AssociationError> {
let existing_state = existing_state.ok_or(AssociationError::NotCreated)?;
self.replay_check(&existing_state)?;
ensure_matching_inbox_id(&self.inbox_id, existing_state.inbox_id())?;

if is_legacy_signature(&self.recovery_address_signature) {
return Err(AssociationError::SignatureNotAllowed(
Expand Down Expand Up @@ -314,6 +326,14 @@ impl IdentityUpdate {
client_timestamp_ns,
}
}

pub fn to_proto(&self) -> Result<IdentityUpdateProto, SerializationError> {
to_identity_update_proto(self)
}

pub fn from_proto(proto: IdentityUpdateProto) -> Result<Self, DeserializationError> {
from_identity_update_proto(proto)
}
}

impl IdentityAction for IdentityUpdate {
Expand Down Expand Up @@ -363,6 +383,17 @@ fn allowed_association(
Ok(())
}

fn ensure_matching_inbox_id(
action_inbox_id: &String,
state_inbox_id: &String,
) -> Result<(), AssociationError> {
if action_inbox_id.ne(state_inbox_id) {
return Err(AssociationError::WrongInboxId);
}

Ok(())
}

// Ensure that the type of signature matches the new entity's role.
fn allowed_signature_for_kind(
role: &MemberKind,
Expand Down
3 changes: 3 additions & 0 deletions xmtp_id/src/associations/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ fn build_action(
.ok_or(SignatureRequestError::MissingSigner)?;

Ok(Action::AddAssociation(AddAssociation {
inbox_id: unsigned_action.inbox_id,
new_member_identifier: unsigned_action.new_member_identifier,
existing_member_signature,
new_member_signature,
Expand All @@ -294,6 +295,7 @@ fn build_action(
.ok_or(SignatureRequestError::MissingSigner)?;

Ok(Action::RevokeAssociation(RevokeAssociation {
inbox_id: unsigned_action.inbox_id,
recovery_address_signature,
revoked_member: unsigned_action.revoked_member,
}))
Expand All @@ -310,6 +312,7 @@ fn build_action(
.ok_or(SignatureRequestError::MissingSigner)?;

Ok(Action::ChangeRecoveryAddress(ChangeRecoveryAddress {
inbox_id: unsigned_action.inbox_id,
recovery_address_signature,
new_recovery_address: unsigned_action.new_recovery_address,
}))
Expand Down
26 changes: 25 additions & 1 deletion xmtp_id/src/associations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub fn get_state(updates: Vec<IdentityUpdate>) -> Result<AssociationState, Assoc

#[cfg(test)]
mod tests {
use tests::hashes::generate_inbox_id;

use self::test_utils::{rand_string, rand_u64, rand_vec, MockSignature};

use super::*;
Expand All @@ -52,6 +54,7 @@ mod tests {
let existing_member = rand_string();
let new_member = rand_vec();
return Self {
inbox_id: rand_string(),
existing_member_signature: MockSignature::new_boxed(
true,
existing_member.into(),
Expand Down Expand Up @@ -90,6 +93,7 @@ mod tests {
fn default() -> Self {
let signer = rand_string();
return Self {
inbox_id: rand_string(),
recovery_address_signature: MockSignature::new_boxed(
true,
signer.into(),
Expand All @@ -114,6 +118,7 @@ mod tests {
initial_state.recovery_address().clone().into();

let update = Action::AddAssociation(AddAssociation {
inbox_id: initial_state.inbox_id().clone(),
existing_member_signature: MockSignature::new_boxed(
true,
initial_wallet_address.clone(),
Expand Down Expand Up @@ -145,6 +150,7 @@ mod tests {
let first_member: MemberIdentifier = initial_state.recovery_address().clone().into();

let update = Action::AddAssociation(AddAssociation {
inbox_id: initial_state.inbox_id().clone(),
new_member_identifier: new_installation_identifier.clone(),
new_member_signature: MockSignature::new_boxed(
true,
Expand Down Expand Up @@ -175,6 +181,7 @@ mod tests {
let account_address = create_action.account_address.clone();
let new_member_identifier: MemberIdentifier = rand_vec().into();
let add_action = AddAssociation {
inbox_id: generate_inbox_id(&account_address, &create_action.nonce),
existing_member_signature: MockSignature::new_boxed(
true,
account_address.clone().into(),
Expand Down Expand Up @@ -224,6 +231,7 @@ mod tests {

// The legacy key can only be used once. After this, subsequent updates should fail
let update = Action::AddAssociation(AddAssociation {
inbox_id: state.inbox_id().clone(),
existing_member_signature: MockSignature::new_boxed(
true,
member_identifier,
Expand All @@ -250,6 +258,7 @@ mod tests {

let new_wallet_address: MemberIdentifier = rand_string().into();
let add_association = Action::AddAssociation(AddAssociation {
inbox_id: initial_state.inbox_id().clone(),
new_member_identifier: new_wallet_address.clone(),
new_member_signature: MockSignature::new_boxed(
true,
Expand Down Expand Up @@ -296,10 +305,12 @@ mod tests {
#[test]
fn reject_invalid_signature_on_update() {
let initial_state = new_test_inbox();
let inbox_id = initial_state.inbox_id().clone();
let bad_signature =
MockSignature::new_boxed(false, rand_string().into(), SignatureKind::Erc191, None);

let update_with_bad_existing_member = Action::AddAssociation(AddAssociation {
inbox_id: inbox_id.clone(),
existing_member_signature: bad_signature.clone(),
..Default::default()
});
Expand All @@ -315,6 +326,7 @@ mod tests {
);

let update_with_bad_new_member = Action::AddAssociation(AddAssociation {
inbox_id: inbox_id.clone(),
new_member_signature: bad_signature.clone(),
existing_member_signature: MockSignature::new_boxed(
true,
Expand All @@ -338,9 +350,12 @@ mod tests {

#[test]
fn reject_if_signer_not_existing_member() {
let create_request = Action::CreateInbox(CreateInbox::default());
let create_inbox = CreateInbox::default();
let inbox_id = generate_inbox_id(&create_inbox.account_address, &create_inbox.nonce);
let create_request = Action::CreateInbox(create_inbox);
// The default here will create an AddAssociation from a random wallet
let update = Action::AddAssociation(AddAssociation {
inbox_id,
// Existing member signature is coming from a random wallet
existing_member_signature: MockSignature::new_boxed(
true,
Expand All @@ -367,6 +382,7 @@ mod tests {
let new_installation_id: MemberIdentifier = rand_vec().into();

let update = Action::AddAssociation(AddAssociation {
inbox_id: existing_state.inbox_id().clone(),
existing_member_signature: MockSignature::new_boxed(
true,
existing_installation.identifier.clone(),
Expand Down Expand Up @@ -404,6 +420,7 @@ mod tests {
.unwrap()
.identifier;
let update = Action::RevokeAssociation(RevokeAssociation {
inbox_id: initial_state.inbox_id().clone(),
recovery_address_signature: MockSignature::new_boxed(
true,
initial_state.recovery_address().clone().into(),
Expand All @@ -430,6 +447,7 @@ mod tests {
.identifier;

let add_second_installation = Action::AddAssociation(AddAssociation {
inbox_id: initial_state.inbox_id().clone(),
existing_member_signature: MockSignature::new_boxed(
true,
wallet_address.clone(),
Expand All @@ -447,6 +465,7 @@ mod tests {
assert_eq!(new_state.members().len(), 3);

let revocation = Action::RevokeAssociation(RevokeAssociation {
inbox_id: new_state.inbox_id().clone(),
recovery_address_signature: MockSignature::new_boxed(
true,
wallet_address.clone(),
Expand Down Expand Up @@ -475,6 +494,7 @@ mod tests {

let second_wallet_address: MemberIdentifier = rand_string().into();
let add_second_wallet = Action::AddAssociation(AddAssociation {
inbox_id: initial_state.inbox_id().clone(),
new_member_identifier: second_wallet_address.clone(),
new_member_signature: MockSignature::new_boxed(
true,
Expand All @@ -492,6 +512,7 @@ mod tests {
});

let revoke_second_wallet = Action::RevokeAssociation(RevokeAssociation {
inbox_id: initial_state.inbox_id().clone(),
recovery_address_signature: MockSignature::new_boxed(
true,
wallet_address.clone(),
Expand All @@ -510,6 +531,7 @@ mod tests {
assert_eq!(state_after_remove.members().len(), 1);

let add_second_wallet_again = Action::AddAssociation(AddAssociation {
inbox_id: state_after_remove.inbox_id().clone(),
new_member_identifier: second_wallet_address.clone(),
new_member_signature: MockSignature::new_boxed(
true,
Expand Down Expand Up @@ -541,6 +563,7 @@ mod tests {
initial_state.recovery_address().clone().into();
let new_recovery_address = rand_string();
let update_recovery = Action::ChangeRecoveryAddress(ChangeRecoveryAddress {
inbox_id: initial_state.inbox_id().clone(),
new_recovery_address: new_recovery_address.clone(),
recovery_address_signature: MockSignature::new_boxed(
true,
Expand All @@ -558,6 +581,7 @@ mod tests {
assert_eq!(new_state.recovery_address(), &new_recovery_address);

let attempted_revoke = Action::RevokeAssociation(RevokeAssociation {
inbox_id: new_state.inbox_id().clone(),
recovery_address_signature: MockSignature::new_boxed(
true,
initial_recovery_address.clone(),
Expand Down
Loading

0 comments on commit 4744e6f

Please sign in to comment.