Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,16 @@ final class ConnectionsLocalStore: ConnectionsLocalStoreProtocol {

// `ConnectionValidator` cleans up stale connections between users, so we normally (re)set this link here.
// But when the two users already have an established MLS conversation, we keep it: overwriting it with the
// Proteus connection conversation would break the link and hide the conversation from the list — which is
// what happens when blocking the user.
// Proteus connection conversation would break the link and hide the conversation from the list.
//
// `migratedToMLS` is only set on the proteus→MLS migration path, so it misses MLS one-on-ones that were
// established directly. Relying on it alone lets the proteus connection conversation overwrite the MLS
// link, which surfaces the proteus (read-only) conversation instead of the MLS one — including on the
// side of a user who was blocked, since that side is never notified and should keep messaging. [WPB-24403]
let existing = connection.to.oneOnOneConversation
if existing?.messageProtocol != .mls || existing?.migratedToMLS != true {
let isEstablishedMLS = existing?.messageProtocol == .mls
&& (existing?.mlsStatus == .ready || existing?.migratedToMLS == true)
if !isEstablishedMLS {
connection.to.oneOnOneConversation = conversation
}
connection.status = connectionInfo.status
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,18 @@ extension ConversationLocalStore {
}

guard let otherUser = localConversation.localParticipantsExcludingSelf.first else {
localConversation.isForcedReadOnly = true
if localConversation.messageProtocol.isOne(of: .mls, .mixed) {
localConversation.mlsStatus = .invalid
// The other participant is absent from the synced member list. This is exactly what the
// backend returns to the party that was *blocked*: the blocker is omitted from the 1:1
// member list. That side is never notified of the block and must keep its conversation
// usable, so we must not tear down an established 1:1 — forcing it read-only and marking
// the MLS group invalid (which wipes it and drops the user back to the read-only Proteus
// conversation). Only do so when there is genuinely no user behind the conversation
// (e.g. a malformed/empty 1:1 that was never linked). [WPB-24403]
if localConversation.oneOnOneUser == nil {
localConversation.isForcedReadOnly = true
if localConversation.messageProtocol.isOne(of: .mls, .mixed) {
localConversation.mlsStatus = .invalid
}
}
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,46 @@ final class ConnectionsLocalStoreTests: XCTestCase {
}
}

func testStoreConnection_GivenLinkedToEstablishedMLSOneOnOne_ItPreservesTheMLSLink() async throws {
// Given a stored connection whose user is linked to an established MLS one-on-one.
// The conversation is created directly as MLS, so `migratedToMLS` stays `false` — the
// case the previous `migratedToMLS`-only guard failed to protect.
try await sut.storeConnection(Scaffolding.connection)

let mlsConversationID = UUID()
try await context.perform { [context, modelHelper] in
let storedConnection = try XCTUnwrap(ZMConnection.fetch(
userID: Scaffolding.member2ID.uuid,
domain: Scaffolding.member2ID.domain,
in: context
))
let mlsConversation = modelHelper!.createMLSConversation(
id: mlsConversationID,
mlsStatus: .ready,
conversationType: .oneOnOne,
in: context
)
storedConnection.to.oneOnOneConversation = mlsConversation
try context.save()
}

// When the backend reports the proteus connection conversation again (e.g. on a connection
// update such as a block, which the blocked side is never notified about).
try await sut.storeConnection(Scaffolding.connection)

// Then the MLS link is preserved instead of being overwritten by the proteus conversation.
try await context.perform { [context] in
let storedConnection = try XCTUnwrap(ZMConnection.fetch(
userID: Scaffolding.member2ID.uuid,
domain: Scaffolding.member2ID.domain,
in: context
))
let relatedConversation = try XCTUnwrap(storedConnection.to.oneOnOneConversation)
XCTAssertEqual(relatedConversation.messageProtocol, .mls)
XCTAssertEqual(relatedConversation.remoteIdentifier, mlsConversationID)
}
}

private enum Scaffolding {
static let member1ID = WireDataModel.QualifiedID(
uuid: .mockID1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,57 @@ final class ConversationLocalStoreTests: XCTestCase {
}
}

func testLinkOneOnOneUserIfNeeded_GivenLinkedUserButNoParticipants_ItKeepsMLSConversationUsable() async {
// Given an established MLS 1:1 linked to its one-on-one user but with no participants — the
// state the blocked party ends up in once the backend removes the blocker from the members.
let conversation = await context.perform { [self] in
_ = modelHelper.createSelfUser(in: context)
let conversation = modelHelper.createMLSConversation(
mlsStatus: .ready,
conversationType: .oneOnOne,
in: context
)
let otherUser = modelHelper.createUser(id: Scaffolding.otherUserID, domain: Scaffolding.domain, in: context)
conversation.oneOnOneUser = otherUser
return conversation
}

// When
await context.perform { [self] in
sut.linkOneOnOneUserIfNeeded(for: conversation)
}

// Then the MLS 1:1 is preserved and remains usable (not wiped, not read-only).
await context.perform {
XCTAssertEqual(conversation.mlsStatus, .ready)
XCTAssertEqual(conversation.messageProtocol, .mls)
XCTAssertFalse(conversation.isForcedReadOnly)
}
}

func testLinkOneOnOneUserIfNeeded_GivenNoLinkedUserAndNoParticipants_ItInvalidatesMLSConversation() async {
// Given an MLS 1:1 with no linked one-on-one user (a genuinely empty/malformed 1:1).
let conversation = await context.perform { [self] in
_ = modelHelper.createSelfUser(in: context)
return modelHelper.createMLSConversation(
mlsStatus: .ready,
conversationType: .oneOnOne,
in: context
)
}

// When
await context.perform { [self] in
sut.linkOneOnOneUserIfNeeded(for: conversation)
}

// Then the MLS group is invalidated and the conversation forced read-only.
await context.perform {
XCTAssertEqual(conversation.mlsStatus, .invalid)
XCTAssertTrue(conversation.isForcedReadOnly)
}
}

private enum Scaffolding {

static let selfUserId = UUID.mockID1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,16 @@ final class ConnectionPayloadProcessor {

// `ConnectionValidator` cleans up stale connections between users, so we normally (re)set this link here.
// But when the two users already have an established MLS conversation, we keep it: overwriting it with the
// Proteus connection conversation would break the link and hide the conversation from the list — which is
// what happens when blocking the user.
// Proteus connection conversation would break the link and hide the conversation from the list.
//
// `migratedToMLS` is only set on the proteus→MLS migration path, so it misses MLS one-on-ones that were
// established directly. Relying on it alone lets the proteus connection conversation overwrite the MLS
// link, which surfaces the proteus (read-only) conversation instead of the MLS one — including on the
// side of a user who was blocked, since that side is never notified and should keep messaging. [WPB-24403]
let existing = connection.to.oneOnOneConversation
if existing?.messageProtocol != .mls || existing?.migratedToMLS != true {
let isEstablishedMLS = existing?.messageProtocol == .mls
&& (existing?.mlsStatus == .ready || existing?.migratedToMLS == true)
if !isEstablishedMLS {
connection.to.oneOnOneConversation = conversation
}
connection.status = payload.status.internalStatus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,18 @@ struct ConversationEventPayloadProcessor {
}

guard let otherUser = localConversation.localParticipantsExcludingSelf.first else {
localConversation.isForcedReadOnly = true
if localConversation.messageProtocol.isOne(of: .mls, .mixed) {
localConversation.mlsStatus = .invalid
// The other participant is absent from the synced member list. This is exactly what the
// backend returns to the party that was *blocked*: it removes the blocker from the 1:1
// member list (Proteus and MLS) without notifying this side. We must not tear down an
// established 1:1 — forcing it read-only and marking the MLS group invalid (which wipes
// it and drops the user back to the read-only Proteus conversation) — when we still have
// a linked one-on-one user. Only do so when there is genuinely no user behind the
// conversation (e.g. a malformed/empty 1:1 that was never linked). [WPB-24403]
if localConversation.oneOnOneUser == nil {
localConversation.isForcedReadOnly = true
if localConversation.messageProtocol.isOne(of: .mls, .mixed) {
localConversation.mlsStatus = .invalid
}
}
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,61 @@ final class ConnectionPayloadProcessorTests: MessagingTestBase {
}
}

func testThatAnEstablishedMLSConversationIsNotOverwrittenByTheProteusConnectionConversation() {
syncMOC.performGroupedAndWait {
// given an established MLS one-on-one that was *not* flagged as migrated
// (e.g. created directly as MLS rather than via the proteus→MLS migration)
let mlsConversation = ZMConversation.insertNewObject(in: self.syncMOC)
mlsConversation.domain = self.owningDomain
mlsConversation.remoteIdentifier = UUID.create()
mlsConversation.conversationType = .oneOnOne
mlsConversation.messageProtocol = .mls
mlsConversation.mlsStatus = .ready
mlsConversation.migratedToMLS = false
self.otherUser.oneOnOneConversation = mlsConversation

// when the backend reports the proteus connection conversation
let payload = self.createConnectionPayload(
to: self.otherUser.qualifiedID!,
conversation: self.oneToOneConversation.qualifiedID!
)
self.sut.updateOrCreateConnection(
from: payload,
in: self.syncMOC
)

// then the MLS link is preserved (the proteus conversation does not shadow it)
XCTAssertEqual(self.otherUser.oneOnOneConversation, mlsConversation)
}
}

func testThatANonEstablishedMLSConversationIsOverwrittenByTheProteusConnectionConversation() {
syncMOC.performGroupedAndWait {
// given an MLS one-on-one whose group is not yet established and was not migrated
let mlsConversation = ZMConversation.insertNewObject(in: self.syncMOC)
mlsConversation.domain = self.owningDomain
mlsConversation.remoteIdentifier = UUID.create()
mlsConversation.conversationType = .oneOnOne
mlsConversation.messageProtocol = .mls
mlsConversation.mlsStatus = .pendingJoin
mlsConversation.migratedToMLS = false
self.otherUser.oneOnOneConversation = mlsConversation

// when the backend reports the proteus connection conversation
let payload = self.createConnectionPayload(
to: self.otherUser.qualifiedID!,
conversation: self.oneToOneConversation.qualifiedID!
)
self.sut.updateOrCreateConnection(
from: payload,
in: self.syncMOC
)

// then the link is (re)set to the proteus connection conversation
XCTAssertEqual(self.otherUser.oneOnOneConversation, self.oneToOneConversation)
}
}

func testThatOtherUserIsAddedToConversation() {
syncMOC.performGroupedAndWait {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,51 @@ final class ConversationEventPayloadProcessorTests: MessagingTestBase {

// MARK: 1:1 / Connection Conversations

func testUpdateOrCreateConversation_OneToOne_PreservesEstablishedMLSWhenOtherMemberMissing() async throws {
// given an established MLS 1:1 linked to its one-on-one user, with the other member already
// absent — as the backend leaves it for the party that was blocked (and never notified).
let qualifiedID = QualifiedID(uuid: .create(), domain: owningDomain)
await syncMOC.perform { [self] in
let conversation = ZMConversation.insertNewObject(in: syncMOC)
conversation.remoteIdentifier = qualifiedID.uuid
conversation.domain = qualifiedID.domain
conversation.conversationType = .oneOnOne
conversation.messageProtocol = .mls
conversation.mlsStatus = .ready
conversation.oneOnOneUser = otherUser
conversation.addParticipantAndUpdateConversationState(
user: ZMUser.selfUser(in: syncMOC),
role: nil
)
}

let payload = await syncMOC.perform { [self] in
let selfUser = ZMUser.selfUser(in: syncMOC)
let selfMember = Payload.ConversationMember(qualifiedID: selfUser.qualifiedID!)
let members = Payload.ConversationMembers(selfMember: selfMember, others: [])
return Payload.Conversation(
qualifiedID: qualifiedID,
type: BackendConversationType.oneOnOne.rawValue,
members: members
)
}

// when the 1:1 is synced without the other member
await sut.updateOrCreateConversation(
from: payload,
in: syncMOC
)

// then the established MLS 1:1 is preserved (not wiped, not read-only)
try await syncMOC.perform { [self] in
let conversation = try XCTUnwrap(ZMConversation.fetch(with: qualifiedID, in: syncMOC))
XCTAssertEqual(conversation.mlsStatus, .ready)
XCTAssertEqual(conversation.messageProtocol, .mls)
XCTAssertFalse(conversation.isForcedReadOnly)
XCTAssertEqual(conversation.oneOnOneUser, otherUser)
}
}

func testUpdateOrCreateConversation_OneToOne_CreatesConversation() async throws {
// given
sut = ConversationEventPayloadProcessor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import WireLogging
/// `GET /conversations/{id}` for a 1:1 once the connection is blocked, and the work item
/// blindly deleted the local conversation in that case. This migration restores those
/// conversations so the user can see and unblock them from the conversation list.
struct AppVersionMigration_4_21_0: AppVersionMigration {
struct AppVersionMigration_4_22_0: AppVersionMigration {

let version: SemanticVersion = "4.21.0"
let version: SemanticVersion = "4.22.0"
let coreDataStack: CoreDataStackProtocol

func perform() async throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,7 @@ extension ZMUserSession {
AppVersionMigration_4_18_0(
coreDataStack: coreDataStack
),
AppVersionMigration_4_21_0(
AppVersionMigration_4_22_0(
coreDataStack: coreDataStack
)
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ import WireDataModelSupport

@testable import WireSyncEngine

struct AppVersionMigration_4_21_0Tests {
struct AppVersionMigration_4_22_0Tests {

let coreDataHelper = CoreDataStackHelper()
let modelHelper = ModelHelper()

let stack: CoreDataStack
let sut: AppVersionMigration_4_21_0
let sut: AppVersionMigration_4_22_0

init() async throws {
self.stack = try await coreDataHelper.createStack()
self.sut = AppVersionMigration_4_21_0(coreDataStack: stack)
self.sut = AppVersionMigration_4_22_0(coreDataStack: stack)
}

@Test("Restores blocked 1:1 conversation that was marked as deleted remotely")
Expand Down
Loading
Loading