From 4788ba1fb15300b090d23f8206c56592f2813dc5 Mon Sep 17 00:00:00 2001 From: Aly Raffauf Date: Mon, 3 Aug 2026 08:43:27 -0400 Subject: [PATCH] Make signed domain transitions value-returning --- src/app.rs | 37 +++++++++++++++++++------------------ src/app/sync.rs | 5 ++--- src/domain.rs | 25 +++++++++++++------------ src/domain/audit.rs | 6 +++--- src/domain/roster.rs | 17 +++++++++-------- src/iroh/tests.rs | 5 ++--- src/protocol.rs | 24 ++++++++++++------------ src/storage/audit.rs | 4 ++-- src/storage/tests.rs | 10 +++++----- 9 files changed, 67 insertions(+), 66 deletions(-) diff --git a/src/app.rs b/src/app.rs index cf1b5a6..3729d38 100644 --- a/src/app.rs +++ b/src/app.rs @@ -122,8 +122,8 @@ impl AppaService { display_name: Some(OWNER_DISPLAY_NAME.to_owned()), role: MemberRole::Owner, }; - let mut roster = FolderRoster::create(folder.id, folder.capability.clone(), owner)?; - roster.sign(&owner_identity)?; + let roster = FolderRoster::create(folder.id, folder.capability.clone(), owner)? + .sign(&owner_identity)?; let audit_event = self.roster_audit_event(&roster, OffsetDateTime::now_utc())?; self.state_store .save_roster_with_audit(&roster, &audit_event) @@ -192,7 +192,7 @@ impl AppaService { .ok_or_else(|| anyhow::anyhow!("folder has no signed roster"))?; node.wait_until_online_for(INVITE_ONLINE_WAIT_TIMEOUT) .await?; - let mut invite = Invite { + let invite = Invite { protocol_version: INVITATION_PROTOCOL_VERSION, folder_id: folder.id, folder_name: folder.name.clone(), @@ -203,7 +203,7 @@ impl AppaService { expires_at: now + INVITE_TTL, signature: None, }; - invite.sign(&self.paths.load_identity()?)?; + let invite = invite.sign(&self.paths.load_identity()?)?; encode_invite(&invite) } @@ -314,13 +314,14 @@ impl AppaService { pub fn revoke_member(&self, folder_path: &Path, device_id: &str) -> AppResult<()> { let folder = self.require_folder(folder_path)?; - let mut roster = self.load_roster(folder.id)?; + let roster = self.load_roster(folder.id)?; if roster.owner_device_id != self.device_id()? { anyhow::bail!("only the folder owner can revoke a member"); } - roster.remove_member(device_id)?; - roster.rotate_capability(self.state_store.new_capability()); - roster.sign(&self.paths.load_identity()?)?; + let roster = roster + .remove_member(device_id)? + .rotate_capability(self.state_store.new_capability()) + .sign(&self.paths.load_identity()?)?; let audit_event = self.roster_audit_event(&roster, OffsetDateTime::now_utc())?; self.state_store.replace_capability_and_roster_with_audit( folder.id, @@ -481,7 +482,7 @@ impl AppaService { folder_id: crate::domain::FolderId, device_id: &str, ) -> AppResult<()> { - let mut roster = self.load_roster(folder_id)?; + let roster = self.load_roster(folder_id)?; if roster.contains_member(device_id) { return Ok(()); } @@ -489,12 +490,13 @@ impl AppaService { tracing::debug!(folder = %folder_id, device = %device_id, "Only the folder owner can enroll a discovered device"); return Ok(()); } - roster.add_member(RosterMember { - device_id: device_id.to_owned(), - display_name: Some(JOINED_DEVICE_DISPLAY_NAME.to_owned()), - role: MemberRole::Member, - })?; - roster.sign(&self.paths.load_identity()?)?; + let roster = roster + .add_member(RosterMember { + device_id: device_id.to_owned(), + display_name: Some(JOINED_DEVICE_DISPLAY_NAME.to_owned()), + role: MemberRole::Member, + })? + .sign(&self.paths.load_identity()?)?; let audit_event = self.roster_audit_event(&roster, OffsetDateTime::now_utc())?; self.state_store .save_roster_with_audit(&roster, &audit_event) @@ -514,7 +516,7 @@ impl AppaService { .state_store .next_audit_sequence_and_parent(roster.folder_id, &author_device_id)?; let manifest = self.state_store.load_manifest(roster.folder_id)?; - let mut event = AuditEvent::create( + let event = AuditEvent::create( roster.folder_id, author_device_id, sequence, @@ -524,8 +526,7 @@ impl AppaService { roster.hash()?, now, ); - event.sign(&identity)?; - Ok(event) + event.sign(&identity) } fn device_id(&self) -> AppResult { diff --git a/src/app/sync.rs b/src/app/sync.rs index 53ea244..c1f9bfe 100644 --- a/src/app/sync.rs +++ b/src/app/sync.rs @@ -174,7 +174,7 @@ impl AppaService { let (sequence, parent_hash) = self .state_store .next_audit_sequence_and_parent(manifest.folder_id, &author_device_id)?; - let mut event = AuditEvent::create( + let event = AuditEvent::create( manifest.folder_id, author_device_id, sequence, @@ -184,8 +184,7 @@ impl AppaService { roster.hash()?, now, ); - event.sign(&identity)?; - Ok(event) + event.sign(&identity) } pub(super) fn save_discovered_peers( diff --git a/src/domain.rs b/src/domain.rs index 9e6fce3..1ab018d 100644 --- a/src/domain.rs +++ b/src/domain.rs @@ -111,8 +111,8 @@ mod tests { display_name: Some("desktop".to_owned()), role: MemberRole::Owner, }; - let mut roster = FolderRoster::create(Uuid::new_v4(), "secret".to_owned(), owner)?; - roster.sign(&owner_identity)?; + let roster = FolderRoster::create(Uuid::new_v4(), "secret".to_owned(), owner)? + .sign(&owner_identity)?; roster.validate() } @@ -128,12 +128,13 @@ mod tests { let mut roster = FolderRoster::create(Uuid::new_v4(), "secret".to_owned(), owner)?; let member_identity = iroh::SecretKey::generate(); - assert!(roster.add_member(RosterMember { - device_id: member_identity.public().to_string(), - display_name: Some("laptop".to_owned()), - role: MemberRole::Member, - })?); - roster.sign(&owner_identity)?; + let roster = roster + .add_member(RosterMember { + device_id: member_identity.public().to_string(), + display_name: Some("laptop".to_owned()), + role: MemberRole::Member, + })? + .sign(&owner_identity)?; assert_eq!(roster.epoch, 2); assert!(roster.contains_member(&member_identity.public().to_string())); @@ -144,7 +145,7 @@ mod tests { fn prevents_read_only_members_from_publishing_changes() -> anyhow::Result<()> { let owner_identity = iroh::SecretKey::generate(); let read_only_identity = iroh::SecretKey::generate(); - let mut roster = FolderRoster::create( + let roster = FolderRoster::create( Uuid::new_v4(), "secret".to_owned(), RosterMember { @@ -153,7 +154,7 @@ mod tests { role: MemberRole::Owner, }, )?; - roster.add_member(RosterMember { + let roster = roster.add_member(RosterMember { device_id: read_only_identity.public().to_string(), display_name: None, role: MemberRole::ReadOnly, @@ -172,8 +173,8 @@ mod tests { display_name: None, role: MemberRole::Owner, }; - let mut roster = FolderRoster::create(Uuid::new_v4(), "secret".to_owned(), owner)?; - roster.sign(&owner_identity)?; + let mut roster = FolderRoster::create(Uuid::new_v4(), "secret".to_owned(), owner)? + .sign(&owner_identity)?; roster.epoch += 1; assert!(roster.validate().is_err()); diff --git a/src/domain/audit.rs b/src/domain/audit.rs index 0700ffa..6638805 100644 --- a/src/domain/audit.rs +++ b/src/domain/audit.rs @@ -58,12 +58,12 @@ impl AuditEvent { } } - pub fn sign(&mut self, identity: &SecretKey) -> anyhow::Result<()> { + pub fn sign(mut self, identity: &SecretKey) -> anyhow::Result { if identity.public().to_string() != self.author_device_id { anyhow::bail!("audit event author does not match its signing identity"); } self.signature = Some(identity.sign(&self.signing_bytes()?)); - Ok(()) + Ok(self) } pub fn validate(&self) -> anyhow::Result<()> { @@ -143,7 +143,7 @@ mod tests { "roster".to_owned(), time::OffsetDateTime::UNIX_EPOCH, ); - event.sign(&identity)?; + let mut event = event.sign(&identity)?; event.sequence = 2; assert!(event.validate().is_err()); diff --git a/src/domain/roster.rs b/src/domain/roster.rs index c4eb2b9..d9dfefc 100644 --- a/src/domain/roster.rs +++ b/src/domain/roster.rs @@ -54,12 +54,12 @@ impl FolderRoster { }) } - pub fn sign(&mut self, owner_identity: &SecretKey) -> anyhow::Result<()> { + pub fn sign(mut self, owner_identity: &SecretKey) -> anyhow::Result { if owner_identity.public().to_string() != self.owner_device_id { anyhow::bail!("only the folder owner can sign its roster"); } self.signature = Some(owner_identity.sign(&self.signing_bytes()?)); - Ok(()) + Ok(self) } pub fn validate(&self) -> anyhow::Result<()> { @@ -97,19 +97,20 @@ impl FolderRoster { .is_some_and(|member| matches!(member.role, MemberRole::Owner | MemberRole::Member)) } - pub fn rotate_capability(&mut self, capability: String) { + pub fn rotate_capability(mut self, capability: String) -> Self { self.capability = capability; self.epoch += 1; self.invalidate_signature(); + self } pub fn active_device_ids(&self) -> BTreeSet { self.members.keys().cloned().collect() } - pub fn add_member(&mut self, member: RosterMember) -> anyhow::Result { + pub fn add_member(mut self, member: RosterMember) -> anyhow::Result { if self.contains_member(&member.device_id) { - return Ok(false); + return Ok(self); } if member.role == MemberRole::Owner { anyhow::bail!("a folder can only have one owner"); @@ -117,10 +118,10 @@ impl FolderRoster { self.members.insert(member.device_id.clone(), member); self.epoch += 1; self.invalidate_signature(); - Ok(true) + Ok(self) } - pub fn remove_member(&mut self, device_id: &str) -> anyhow::Result<()> { + pub fn remove_member(mut self, device_id: &str) -> anyhow::Result { if device_id == self.owner_device_id { anyhow::bail!("the folder owner cannot be removed"); } @@ -129,7 +130,7 @@ impl FolderRoster { } self.epoch += 1; self.invalidate_signature(); - Ok(()) + Ok(self) } fn signing_bytes(&self) -> anyhow::Result> { diff --git a/src/iroh/tests.rs b/src/iroh/tests.rs index 5ba271e..086fb44 100644 --- a/src/iroh/tests.rs +++ b/src/iroh/tests.rs @@ -20,7 +20,7 @@ fn test_roster( crate::domain::FolderRoster::create(folder_id, "capability".to_owned(), member) .expect("roster"); for device_id in active_member_ids { - roster + roster = roster .add_member(crate::domain::RosterMember { device_id, display_name: None, @@ -28,8 +28,7 @@ fn test_roster( }) .expect("add member"); } - roster.sign(&owner).expect("sign roster"); - roster + roster.sign(&owner).expect("sign roster") } #[tokio::test] diff --git a/src/protocol.rs b/src/protocol.rs index 88c37ab..d55a332 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -59,9 +59,9 @@ pub struct Invite { } impl Invite { - pub fn sign(&mut self, identity: &SecretKey) -> anyhow::Result<()> { + pub fn sign(mut self, identity: &SecretKey) -> anyhow::Result { self.signature = Some(identity.sign(&self.signing_bytes()?)); - Ok(()) + Ok(self) } fn verify_signature(&self) -> anyhow::Result<()> { @@ -199,7 +199,7 @@ mod tests { fn rejects_an_invitation_changed_after_signing() -> anyhow::Result<()> { let identity = iroh::SecretKey::generate(); let endpoint = iroh::EndpointAddr::new(identity.public()); - let mut roster = FolderRoster::create( + let roster = FolderRoster::create( Uuid::new_v4(), "secret".to_owned(), RosterMember { @@ -207,9 +207,9 @@ mod tests { display_name: None, role: MemberRole::Owner, }, - )?; - roster.sign(&identity)?; - let mut invite = Invite { + )? + .sign(&identity)?; + let invite = Invite { protocol_version: INVITATION_PROTOCOL_VERSION, folder_id: roster.folder_id, folder_name: "notes".to_owned(), @@ -220,7 +220,7 @@ mod tests { expires_at: time::OffsetDateTime::now_utc() + Duration::hours(1), signature: None, }; - invite.sign(&identity)?; + let mut invite = invite.sign(&identity)?; let mut legacy_unsigned_invite = invite.clone(); legacy_unsigned_invite.signature = None; @@ -240,7 +240,7 @@ mod tests { fn validates_an_invitation_against_the_supplied_time() -> anyhow::Result<()> { let identity = iroh::SecretKey::generate(); let endpoint = iroh::EndpointAddr::new(identity.public()); - let mut roster = FolderRoster::create( + let roster = FolderRoster::create( Uuid::new_v4(), "secret".to_owned(), RosterMember { @@ -248,10 +248,10 @@ mod tests { display_name: None, role: MemberRole::Owner, }, - )?; - roster.sign(&identity)?; + )? + .sign(&identity)?; let expires_at = time::OffsetDateTime::UNIX_EPOCH + Duration::hours(1); - let mut invite = Invite { + let invite = Invite { protocol_version: INVITATION_PROTOCOL_VERSION, folder_id: roster.folder_id, folder_name: "notes".to_owned(), @@ -262,7 +262,7 @@ mod tests { expires_at, signature: None, }; - invite.sign(&identity)?; + let invite = invite.sign(&identity)?; let ticket = encode_invite(&invite)?; assert!(decode_invite_at(&ticket, expires_at - Duration::seconds(1)).is_ok()); diff --git a/src/storage/audit.rs b/src/storage/audit.rs index 2b92957..49818bb 100644 --- a/src/storage/audit.rs +++ b/src/storage/audit.rs @@ -281,7 +281,7 @@ mod tests { "roster".to_owned(), time::OffsetDateTime::UNIX_EPOCH, ); - first.sign(&identity)?; + let first = first.sign(&identity)?; store.append_audit_event(&first)?; let (sequence, parent) = store.next_audit_sequence_and_parent(folder_id, &identity.public().to_string())?; @@ -295,7 +295,7 @@ mod tests { "roster".to_owned(), time::OffsetDateTime::UNIX_EPOCH, ); - second.sign(&identity)?; + let second = second.sign(&identity)?; store.append_audit_event(&second)?; let verification = store.verify_audit(folder_id)?; diff --git a/src/storage/tests.rs b/src/storage/tests.rs index 79461b0..16b8b1b 100644 --- a/src/storage/tests.rs +++ b/src/storage/tests.rs @@ -254,7 +254,7 @@ fn test_invite(folder_id: uuid::Uuid) -> anyhow::Result role: crate::domain::MemberRole::Owner, }, )?; - roster.sign(&owner)?; + let roster = roster.sign(&owner)?; let mut invite = crate::protocol::Invite { protocol_version: crate::protocol::INVITATION_PROTOCOL_VERSION, folder_id, @@ -266,8 +266,7 @@ fn test_invite(folder_id: uuid::Uuid) -> anyhow::Result expires_at: OffsetDateTime::now_utc() + time::Duration::hours(1), signature: None, }; - invite.sign(&owner)?; - Ok(invite) + invite.sign(&owner) } #[test] @@ -289,8 +288,9 @@ fn rotating_a_capability_retains_its_previous_value() -> anyhow::Result<()> { role: crate::domain::MemberRole::Owner, }, )?; - roster.rotate_capability(store.new_capability()); - roster.sign(&owner)?; + let roster = roster + .rotate_capability(store.new_capability()) + .sign(&owner)?; store.replace_capability_and_roster(folder.id, &roster)?; assert_ne!(roster.capability, folder.capability); -- 2.51.2