From 05a198c89fb6bf4f34e26d75dde2b527c286b218 Mon Sep 17 00:00:00 2001 From: Sarah Jamie Lewis Date: Sat, 24 Feb 2024 11:57:46 -0800 Subject: [PATCH] Fix Error in ACL-V1 that Prevented ShareFiles (for some) Also aligns model.DeserializeAttributes to best practice --- .gitignore | 1 + model/constants/attributes.go | 1 + model/conversation.go | 8 ++++++-- peer/cwtch_peer.go | 27 +++++++++++++++++++-------- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 24ffb60..777e0f5 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ tordir/ testing/autodownload/download_dir testing/autodownload/storage *.swp +testing/managerstorage/* \ No newline at end of file diff --git a/model/constants/attributes.go b/model/constants/attributes.go index 0856f36..67a33ab 100644 --- a/model/constants/attributes.go +++ b/model/constants/attributes.go @@ -71,3 +71,4 @@ const Description = "description" // Used to store the status of acl migrations const ACLVersion = "acl-version" const ACLVersionOne = "acl-v1" +const ACLVersionTwo = "acl-v2" diff --git a/model/conversation.go b/model/conversation.go index 324fdb4..1f18f6c 100644 --- a/model/conversation.go +++ b/model/conversation.go @@ -59,8 +59,12 @@ func (a *Attributes) Serialize() []byte { // DeserializeAttributes converts a JSON struct into an Attributes map func DeserializeAttributes(data []byte) Attributes { - var attributes Attributes - json.Unmarshal(data, &attributes) + attributes := make(Attributes) + err := json.Unmarshal(data, &attributes) + if err != nil { + log.Error("error deserializing attributes (this is likely a programming error): %v", err) + return make(Attributes) + } return attributes } diff --git a/peer/cwtch_peer.go b/peer/cwtch_peer.go index 55e685e..18c5fb6 100644 --- a/peer/cwtch_peer.go +++ b/peer/cwtch_peer.go @@ -312,13 +312,14 @@ func (cp *cwtchPeer) GenerateProtocolEngine(acn connectivity.ACN, bus event.Mana if tor.IsValidHostname(conversation.Handle) { - // if this profile does not have an ACL version, and the profile is accepted, then migrate - // the permissions to the v1 ACL + // if this profile does not have an ACL version, and the profile is accepted (OR the acl version is v1 and the profile is accepted...) + // then migrate the permissions to the v2 ACL // migrate the old accepted AC to a new fine-grained one // we only do this for previously trusted connections // NOTE: this does not supercede global cwthch experiments settings - // if share files is turned off globally then acl.ShareFiles will be ignored. - if _, exists := conversation.GetAttribute(attr.LocalScope, attr.ProfileZone, constants.ACLVersion); !exists { + // if share files is turned off globally then acl.ShareFiles will be ignored + // Note: There was a bug in the original EP code that meant that some acl-v1 profiles did not get ShareFiles or RenderImages - this corrects that. + if version, exists := conversation.GetAttribute(attr.LocalScope, attr.ProfileZone, constants.ACLVersion); !exists || version == constants.ACLVersionOne { if conversation.Accepted { if ac, exists := conversation.ACL[conversation.Handle]; exists { ac.ShareFiles = true @@ -329,7 +330,7 @@ func (cp *cwtchPeer) GenerateProtocolEngine(acn connectivity.ACN, bus event.Mana } // Update the ACL Version - cp.storage.SetConversationAttribute(conversation.ID, attr.LocalScope.ConstructScopedZonedPath(attr.ProfileZone.ConstructZonedPath(constants.ACLVersion)), constants.ACLVersionOne) + cp.storage.SetConversationAttribute(conversation.ID, attr.LocalScope.ConstructScopedZonedPath(attr.ProfileZone.ConstructZonedPath(constants.ACLVersion)), constants.ACLVersionTwo) // Store the updated ACL cp.storage.SetConversationACL(conversation.ID, conversation.ACL) } @@ -713,8 +714,18 @@ func (cp *cwtchPeer) NewContactConversation(handle string, acl model.AccessContr conversationInfo, _ := cp.storage.GetConversationByHandle(handle) if conversationInfo == nil { conversationID, err := cp.storage.NewConversation(handle, model.Attributes{event.SaveHistoryKey: event.DeleteHistoryDefault}, model.AccessControlList{handle: acl}, accepted) - cp.SetConversationAttribute(conversationID, attr.LocalScope.ConstructScopedZonedPath(attr.ProfileZone.ConstructZonedPath(constants.ACLVersion)), constants.ACLVersionOne) + if err != nil { + log.Errorf("unable to create a new contact conversation: %v", err) + return -1, err + } cp.SetConversationAttribute(conversationID, attr.LocalScope.ConstructScopedZonedPath(attr.ProfileZone.ConstructZonedPath(constants.AttrLastConnectionTime)), time.Now().Format(time.RFC3339Nano)) + if accepted { + // If this call came from a trusted action (i.e. import bundle or accept button then accept the conversation) + // This assigns all permissions (and in v2 is currently the default state of trusted contacts) + // Accept conversation does PeerWithOnion + cp.AcceptConversation(conversationID) + } + cp.SetConversationAttribute(conversationID, attr.LocalScope.ConstructScopedZonedPath(attr.ProfileZone.ConstructZonedPath(constants.ACLVersion)), constants.ACLVersionTwo) cp.eventBus.Publish(event.NewEvent(event.ContactCreated, map[event.Field]string{event.ConversationID: strconv.Itoa(conversationID), event.RemotePeer: handle})) return conversationID, err } @@ -1257,9 +1268,9 @@ func (cp *cwtchPeer) ImportBundle(importString string) error { return ConstructResponse(constants.ImportBundlePrefix, "success") } else if tor.IsValidHostname(importString) { _, err := cp.NewContactConversation(importString, model.DefaultP2PAccessControl(), true) + // NOTE: Not NewContactConversation implictly does AcceptConversation AND PeerWithOnion if relevant so + // we no longer need to do it here... if err == nil { - // Assuming all is good, we should peer with this contact. - cp.PeerWithOnion(importString) return ConstructResponse(constants.ImportBundlePrefix, "success") } return ConstructResponse(constants.ImportBundlePrefix, err.Error())