From c10975098d99b46f31e30250de8da283055658e3 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 1 Sep 2025 15:52:21 -0500 Subject: [PATCH 1/4] Fix up some permission loopholes. --- .../CloudKit/Metadatabase.swift | 2 +- .../SharingGRDBCore/CloudKit/SyncEngine.swift | 5 +- .../SharingPermissionsTests.swift | 279 ++++++++++++++++++ .../CloudKitTests/SharingTests.swift | 266 ----------------- 4 files changed, 284 insertions(+), 268 deletions(-) create mode 100644 Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift diff --git a/Sources/SharingGRDBCore/CloudKit/Metadatabase.swift b/Sources/SharingGRDBCore/CloudKit/Metadatabase.swift index e728a8bf..62c617b7 100644 --- a/Sources/SharingGRDBCore/CloudKit/Metadatabase.swift +++ b/Sources/SharingGRDBCore/CloudKit/Metadatabase.swift @@ -116,7 +116,7 @@ func defaultMetadatabase( ) .execute(db) } - migrator.registerMigration("Create PendingRecodZoneChanges Table") { db in + migrator.registerMigration("Create PendingRecordZoneChanges Table") { db in try SQLQueryExpression(""" CREATE TABLE IF NOT EXISTS "\(raw: .sqliteDataCloudKitSchemaName)_pendingRecordZoneChanges" ( "pendingRecordZoneChange" BLOB NOT NULL diff --git a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift index 2851a3d1..4ff00f3d 100644 --- a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift +++ b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift @@ -1265,10 +1265,13 @@ try open(table) } + case .permissionFailure: + fatalError() + case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable, .notAuthenticated, .operationCancelled, .batchRequestFailed, .internalError, .partialFailure, .badContainer, .requestRateLimited, .missingEntitlement, - .permissionFailure, .invalidArguments, .resultsTruncated, .assetFileNotFound, + .invalidArguments, .resultsTruncated, .assetFileNotFound, .assetFileModified, .incompatibleVersion, .constraintViolation, .changeTokenExpired, .badDatabase, .quotaExceeded, .limitExceeded, .userDeletedZone, .tooManyParticipants, .alreadyShared, .managedAccountRestricted, .participantMayNeedVerification, diff --git a/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift b/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift new file mode 100644 index 00000000..267888a0 --- /dev/null +++ b/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift @@ -0,0 +1,279 @@ +import CloudKit +import CustomDump +import Foundation +import InlineSnapshotTesting +import OrderedCollections +import SharingGRDB +import SnapshotTestingCustomDump +import Testing + +extension BaseCloudKitTests { + @MainActor + final class SharingPermissionsTests: BaseCloudKitTests, @unchecked Sendable { + /// Inserting record into shared record when user does not have permission should be rejected. + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func insertRecordInReadOnlyRemindersList() async throws { + let externalZone = CKRecordZone( + zoneID: CKRecordZone.ID( + zoneName: "external.zone", + ownerName: "external.owner" + ) + ) + try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() + + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + let share = CKShare( + rootRecord: remindersListRecord, + shareID: CKRecord.ID( + recordName: "share-\(remindersListRecord.recordID.recordName)", + zoneID: remindersListRecord.recordID.zoneID + ) + ) + share.publicPermission = .readOnly + share.currentUserParticipant?.permission = .readOnly + + try await syncEngine + .acceptShare( + metadata: ShareMetadata( + containerIdentifier: container.containerIdentifier!, + hierarchicalRootRecordID: remindersListRecord.recordID, + rootRecord: remindersListRecord, + share: share + ) + ) + + + try await self.userDatabase.userWrite { db in + let error = #expect(throws: DatabaseError.self) { + try db.seed { + Reminder(id: 1, title: "Get milk", remindersListID: 1) + } + } + #expect(error?.message == SyncEngine.writePermissionError) + try #expect(Reminder.all.fetchCount(db) == 0) + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), + recordType: "cloudkit.share", + parent: nil, + share: nil + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), + recordType: "remindersLists", + parent: nil, + share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), + id: 1, + title: "Personal" + ) + ] + ) + ) + """ + } + } + + /// Delete record in shared record when user does not have permission. + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func deleteReminderInReadOnlyRemindersList() async throws { + let externalZone = CKRecordZone( + zoneID: CKRecordZone.ID( + zoneName: "external.zone", + ownerName: "external.owner" + ) + ) + try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() + + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + let share = CKShare( + rootRecord: remindersListRecord, + shareID: CKRecord.ID( + recordName: "share-\(remindersListRecord.recordID.recordName)", + zoneID: remindersListRecord.recordID.zoneID + ) + ) + share.publicPermission = .readOnly + share.currentUserParticipant?.permission = .readOnly + + try await syncEngine + .acceptShare( + metadata: ShareMetadata( + containerIdentifier: container.containerIdentifier!, + hierarchicalRootRecordID: remindersListRecord.recordID, + rootRecord: remindersListRecord, + share: share + ) + ) + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) + ) + reminderRecord.setValue(1, forKey: "id", at: now) + reminderRecord.setValue("Get milk", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) + try await syncEngine.modifyRecords(scope: .shared, saving: [reminderRecord]).notify() + + try await self.userDatabase.userWrite { db in + let error = #expect(throws: DatabaseError.self) { + try Reminder.find(1).delete().execute(db) + } + #expect(error?.message == SyncEngine.writePermissionError) + try #expect(Reminder.count().fetchOne(db) == 1) + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), + recordType: "cloudkit.share", + parent: nil, + share: nil + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:reminders/external.zone/external.owner), + recordType: "reminders", + parent: CKReference(recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner)), + share: nil, + id: 1, + remindersListID: 1, + title: "Get milk" + ), + [2]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), + recordType: "remindersLists", + parent: nil, + share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), + id: 1, + title: "Personal" + ) + ] + ) + ) + """ + } + } + + /// Editing record in shared record when user does not have permission. + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func editReminderInReadOnlyRemindersList() async throws { + let externalZone = CKRecordZone( + zoneID: CKRecordZone.ID( + zoneName: "external.zone", + ownerName: "external.owner" + ) + ) + try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() + + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + let share = CKShare( + rootRecord: remindersListRecord, + shareID: CKRecord.ID( + recordName: "share-\(remindersListRecord.recordID.recordName)", + zoneID: remindersListRecord.recordID.zoneID + ) + ) + share.publicPermission = .readOnly + share.currentUserParticipant?.permission = .readOnly + + try await syncEngine + .acceptShare( + metadata: ShareMetadata( + containerIdentifier: container.containerIdentifier!, + hierarchicalRootRecordID: remindersListRecord.recordID, + rootRecord: remindersListRecord, + share: share + ) + ) + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) + ) + reminderRecord.setValue(1, forKey: "id", at: now) + reminderRecord.setValue("Get milk", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.setValue(false, forKey: "isCompleted", at: now) + reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) + try await syncEngine.modifyRecords(scope: .shared, saving: [reminderRecord]).notify() + + try await self.userDatabase.userWrite { db in + let error = #expect(throws: DatabaseError.self) { + try Reminder.update { $0.isCompleted = true }.execute(db) + } + #expect(error?.message == SyncEngine.writePermissionError) + try #expect(Reminder.where(\.isCompleted).fetchCount(db) == 0) + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), + recordType: "cloudkit.share", + parent: nil, + share: nil + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:reminders/external.zone/external.owner), + recordType: "reminders", + parent: CKReference(recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner)), + share: nil, + id: 1, + isCompleted: 0, + remindersListID: 1, + title: "Get milk" + ), + [2]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), + recordType: "remindersLists", + parent: nil, + share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), + id: 1, + title: "Personal" + ) + ] + ) + ) + """ + } + } + } +} diff --git a/Tests/SharingGRDBTests/CloudKitTests/SharingTests.swift b/Tests/SharingGRDBTests/CloudKitTests/SharingTests.swift index 58c7e2a1..671de0fa 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/SharingTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/SharingTests.swift @@ -897,272 +897,6 @@ extension BaseCloudKitTests { """ } } - - /// Inserting record into shared record when user does not have permission should be rejected. - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func insertRecordInReadOnlyRemindersList() async throws { - let externalZone = CKRecordZone( - zoneID: CKRecordZone.ID( - zoneName: "external.zone", - ownerName: "external.owner" - ) - ) - try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() - - let remindersListRecord = CKRecord( - recordType: RemindersList.tableName, - recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) - ) - remindersListRecord.setValue(1, forKey: "id", at: now) - remindersListRecord.setValue("Personal", forKey: "title", at: now) - let share = CKShare( - rootRecord: remindersListRecord, - shareID: CKRecord.ID( - recordName: "share-\(remindersListRecord.recordID.recordName)", - zoneID: remindersListRecord.recordID.zoneID - ) - ) - share.publicPermission = .readOnly - share.currentUserParticipant?.permission = .readOnly - - try await syncEngine - .acceptShare( - metadata: ShareMetadata( - containerIdentifier: container.containerIdentifier!, - hierarchicalRootRecordID: remindersListRecord.recordID, - rootRecord: remindersListRecord, - share: share - ) - ) - - - try await self.userDatabase.userWrite { db in - let error = #expect(throws: DatabaseError.self) { - try db.seed { - Reminder(id: 1, title: "Get milk", remindersListID: 1) - } - } - #expect(error?.message == SyncEngine.writePermissionError) - try #expect(Reminder.all.fetchCount(db) == 0) - } - assertInlineSnapshot(of: container, as: .customDump) { - """ - MockCloudContainer( - privateCloudDatabase: MockCloudDatabase( - databaseScope: .private, - storage: [] - ), - sharedCloudDatabase: MockCloudDatabase( - databaseScope: .shared, - storage: [ - [0]: CKRecord( - recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), - recordType: "cloudkit.share", - parent: nil, - share: nil - ), - [1]: CKRecord( - recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), - recordType: "remindersLists", - parent: nil, - share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), - id: 1, - title: "Personal" - ) - ] - ) - ) - """ - } - } - - /// Delete record in shared record when user does not have permission. - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func deleteReminderInReadOnlyRemindersList() async throws { - let externalZone = CKRecordZone( - zoneID: CKRecordZone.ID( - zoneName: "external.zone", - ownerName: "external.owner" - ) - ) - try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() - - let remindersListRecord = CKRecord( - recordType: RemindersList.tableName, - recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) - ) - remindersListRecord.setValue(1, forKey: "id", at: now) - remindersListRecord.setValue("Personal", forKey: "title", at: now) - let share = CKShare( - rootRecord: remindersListRecord, - shareID: CKRecord.ID( - recordName: "share-\(remindersListRecord.recordID.recordName)", - zoneID: remindersListRecord.recordID.zoneID - ) - ) - share.publicPermission = .readOnly - share.currentUserParticipant?.permission = .readOnly - - try await syncEngine - .acceptShare( - metadata: ShareMetadata( - containerIdentifier: container.containerIdentifier!, - hierarchicalRootRecordID: remindersListRecord.recordID, - rootRecord: remindersListRecord, - share: share - ) - ) - let reminderRecord = CKRecord( - recordType: Reminder.tableName, - recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) - ) - reminderRecord.setValue(1, forKey: "id", at: now) - reminderRecord.setValue("Get milk", forKey: "title", at: now) - reminderRecord.setValue(1, forKey: "remindersListID", at: now) - reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) - try await syncEngine.modifyRecords(scope: .shared, saving: [reminderRecord]).notify() - - try await self.userDatabase.userWrite { db in - let error = #expect(throws: DatabaseError.self) { - try Reminder.find(1).delete().execute(db) - } - #expect(error?.message == SyncEngine.writePermissionError) - try #expect(Reminder.count().fetchOne(db) == 1) - } - assertInlineSnapshot(of: container, as: .customDump) { - """ - MockCloudContainer( - privateCloudDatabase: MockCloudDatabase( - databaseScope: .private, - storage: [] - ), - sharedCloudDatabase: MockCloudDatabase( - databaseScope: .shared, - storage: [ - [0]: CKRecord( - recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), - recordType: "cloudkit.share", - parent: nil, - share: nil - ), - [1]: CKRecord( - recordID: CKRecord.ID(1:reminders/external.zone/external.owner), - recordType: "reminders", - parent: CKReference(recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner)), - share: nil, - id: 1, - remindersListID: 1, - title: "Get milk" - ), - [2]: CKRecord( - recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), - recordType: "remindersLists", - parent: nil, - share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), - id: 1, - title: "Personal" - ) - ] - ) - ) - """ - } - } - - /// Editing record in shared record when user does not have permission. - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func editReminderInReadOnlyRemindersList() async throws { - let externalZone = CKRecordZone( - zoneID: CKRecordZone.ID( - zoneName: "external.zone", - ownerName: "external.owner" - ) - ) - try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() - - let remindersListRecord = CKRecord( - recordType: RemindersList.tableName, - recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) - ) - remindersListRecord.setValue(1, forKey: "id", at: now) - remindersListRecord.setValue("Personal", forKey: "title", at: now) - let share = CKShare( - rootRecord: remindersListRecord, - shareID: CKRecord.ID( - recordName: "share-\(remindersListRecord.recordID.recordName)", - zoneID: remindersListRecord.recordID.zoneID - ) - ) - share.publicPermission = .readOnly - share.currentUserParticipant?.permission = .readOnly - - try await syncEngine - .acceptShare( - metadata: ShareMetadata( - containerIdentifier: container.containerIdentifier!, - hierarchicalRootRecordID: remindersListRecord.recordID, - rootRecord: remindersListRecord, - share: share - ) - ) - let reminderRecord = CKRecord( - recordType: Reminder.tableName, - recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) - ) - reminderRecord.setValue(1, forKey: "id", at: now) - reminderRecord.setValue("Get milk", forKey: "title", at: now) - reminderRecord.setValue(1, forKey: "remindersListID", at: now) - reminderRecord.setValue(false, forKey: "isCompleted", at: now) - reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) - try await syncEngine.modifyRecords(scope: .shared, saving: [reminderRecord]).notify() - - try await self.userDatabase.userWrite { db in - let error = #expect(throws: DatabaseError.self) { - try Reminder.update { $0.isCompleted = true }.execute(db) - } - #expect(error?.message == SyncEngine.writePermissionError) - try #expect(Reminder.where(\.isCompleted).fetchCount(db) == 0) - } - assertInlineSnapshot(of: container, as: .customDump) { - """ - MockCloudContainer( - privateCloudDatabase: MockCloudDatabase( - databaseScope: .private, - storage: [] - ), - sharedCloudDatabase: MockCloudDatabase( - databaseScope: .shared, - storage: [ - [0]: CKRecord( - recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), - recordType: "cloudkit.share", - parent: nil, - share: nil - ), - [1]: CKRecord( - recordID: CKRecord.ID(1:reminders/external.zone/external.owner), - recordType: "reminders", - parent: CKReference(recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner)), - share: nil, - id: 1, - isCompleted: 0, - remindersListID: 1, - title: "Get milk" - ), - [2]: CKRecord( - recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), - recordType: "remindersLists", - parent: nil, - share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), - id: 1, - title: "Personal" - ) - ] - ) - ) - """ - } - } } } From 55fa6908883dfddc6c1de2cfb28d0ce70a5699cc Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 1 Sep 2025 17:22:27 -0500 Subject: [PATCH 2/4] wip --- .../CloudKit/Internal/MockCloudDatabase.swift | 37 +++- .../CloudKit/Internal/MockSyncEngine.swift | 2 +- .../SharingGRDBCore/CloudKit/SyncEngine.swift | 28 ++- .../MockCloudDatabaseTests.swift | 16 ++ .../SharingPermissionsTests.swift | 173 ++++++++++++++++++ 5 files changed, 249 insertions(+), 7 deletions(-) diff --git a/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift index 7bf55819..3bf25625 100644 --- a/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift @@ -88,7 +88,9 @@ package final class MockCloudDatabase: CloudDatabase { case .ifServerRecordUnchanged: for recordToSave in recordsToSave { if let share = recordToSave as? CKShare { - let isSavingRootRecord = recordsToSave.contains(where: { $0.share?.recordID == share.recordID }) + let isSavingRootRecord = recordsToSave.contains(where: { + $0.share?.recordID == share.recordID + }) let shareWasPreviouslySaved = storage[share.recordID.zoneID]?[share.recordID] != nil guard shareWasPreviouslySaved || isSavingRootRecord else { @@ -102,7 +104,7 @@ package final class MockCloudDatabase: CloudDatabase { continue } } - + guard storage[recordToSave.recordID.zoneID] != nil else { saveResults[recordToSave.recordID] = .failure(CKError(.zoneNotFound)) @@ -124,6 +126,35 @@ package final class MockCloudDatabase: CloudDatabase { return } + func root(of record: CKRecord) -> CKRecord { + guard let parent = record.parent + else { return record } + return (storage[parent.recordID.zoneID]?[parent.recordID]).map(root) ?? record + } + func share(for rootRecord: CKRecord) -> CKShare? { + for (_, record) in storage[rootRecord.recordID.zoneID] ?? [:] { + guard record.recordID == rootRecord.share?.recordID + else { continue } + return record as? CKShare + } + return nil + } + let share = share(for: root(of: recordToSave)) + if + !(recordToSave is CKShare), + let share, + !(share.publicPermission == .readWrite + || share.currentUserParticipant?.permission == .readWrite) + { + saveResults[recordToSave.recordID] = .failure(CKError(.permissionFailure)) +// reportIssue( +// """ +// You do not have permission to write to this record: \(recordToSave.recordID.recordName) +// """ +// ) + return + } + guard let copy = recordToSave.copy() as? CKRecord else { fatalError("Could not copy CKRecord.") } copy._recordChangeTag = UUID().uuidString @@ -137,7 +168,7 @@ package final class MockCloudDatabase: CloudDatabase { } } - // TODO: this should merge copy's values into storage but not sure how right now. + // TODO: this should merge copy's values into storage but not sure how right now. storage[recordToSave.recordID.zoneID]?[recordToSave.recordID] = copy saveResults[recordToSave.recordID] = .success(copy) } diff --git a/Sources/SharingGRDBCore/CloudKit/Internal/MockSyncEngine.swift b/Sources/SharingGRDBCore/CloudKit/Internal/MockSyncEngine.swift index 07bee97f..82d4819d 100644 --- a/Sources/SharingGRDBCore/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SharingGRDBCore/CloudKit/Internal/MockSyncEngine.swift @@ -79,7 +79,7 @@ package final class MockSyncEngine: SyncEngineProtocol { } } - state.remove(pendingRecordZoneChanges: recordIDsSkipped.map { .saveRecord($0) }) + state.remove(pendingRecordZoneChanges: recordsToSave.map { .saveRecord($0.recordID) }) return CKSyncEngine.RecordZoneChangeBatch( recordsToSave: recordsToSave, diff --git a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift index 93aae0f2..cebf4b81 100644 --- a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift +++ b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift @@ -1266,7 +1266,26 @@ } case .permissionFailure: - fatalError() + guard + let recordPrimaryKey = failedRecord.recordID.recordPrimaryKey, + let table = tablesByName[failedRecord.recordType] + else { continue } + func open(_: T.Type) async throws { + do { + let serverRecord = try await container.sharedCloudDatabase.record(for: failedRecord.recordID) + upsertFromServerRecord(serverRecord, force: true) + } catch let error as CKError where error.code == .unknownItem { + try await userDatabase.write { db in + try T + .where { SQLQueryExpression("\($0.primaryKey) = \(bind: recordPrimaryKey)") } + .delete() + .execute(db) + } + } + } + await withErrorReporting(.sqliteDataCloudKitFailure) { + try await open(table) + } case .networkFailure, .networkUnavailable, .zoneBusy, .serviceUnavailable, .notAuthenticated, .operationCancelled, .batchRequestFailed, @@ -1341,7 +1360,10 @@ } } - private func upsertFromServerRecord(_ serverRecord: CKRecord) { + private func upsertFromServerRecord( + _ serverRecord: CKRecord, + force: Bool = false + ) { withErrorReporting(.sqliteDataCloudKitFailure) { guard let table = tablesByName[serverRecord.recordType] else { @@ -1379,7 +1401,7 @@ func open(_: T.Type) throws { var columnNames = T.TableColumns.writableColumns.map(\.name) - if let metadata, let allFields = metadata._lastKnownServerRecordAllFields { + if !force, let metadata, let allFields = metadata._lastKnownServerRecordAllFields { let row = try userDatabase.read { db in try T.find(SQLQueryExpression("\(bind: metadata.recordPrimaryKey)")).fetchOne(db) } diff --git a/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift index cc143b6f..df848bc1 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -414,5 +414,21 @@ extension BaseCloudKitTests { let newShare = try syncEngine.private.database.record(for: CKRecord.ID(recordName: "share")) _ = try syncEngine.modifyRecords(scope: .private, saving: [newShare]) } + + @Test func writePermission() async throws { + let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1")) + let share = CKShare(rootRecord: record, shareID: CKRecord.ID(recordName: "share")) + share.publicPermission = .readOnly + _ = try syncEngine.modifyRecords(scope: .private, saving: [share, record]) + + let freshRecord = try syncEngine.private.database.record(for: record.recordID) + try await withKnownIssue { + try await syncEngine.modifyRecords(scope: .private, saving: [freshRecord]).notify() + } matching: { issue in + issue.description.hasSuffix(""" + You do not have permission to write to this record: 1 + """) + } + } } } diff --git a/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift b/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift index 267888a0..a2f480fb 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift @@ -275,5 +275,178 @@ extension BaseCloudKitTests { """ } } + + // Edit a record while locally we think we have permission, but CloudKit has newer permissions + // that are read only. + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func createRecordWhenLocalHasPermissionsButCloudKitDoesNot() async throws { + let externalZone = CKRecordZone( + zoneID: CKRecordZone.ID( + zoneName: "external.zone", + ownerName: "external.owner" + ) + ) + try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() + + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + let share = CKShare( + rootRecord: remindersListRecord, + shareID: CKRecord.ID( + recordName: "share-\(remindersListRecord.recordID.recordName)", + zoneID: remindersListRecord.recordID.zoneID + ) + ) + share.publicPermission = .readWrite + share.currentUserParticipant?.permission = .readWrite + + try await syncEngine + .acceptShare( + metadata: ShareMetadata( + containerIdentifier: container.containerIdentifier!, + hierarchicalRootRecordID: remindersListRecord.recordID, + rootRecord: remindersListRecord, + share: share + ) + ) + + let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare + freshShare.publicPermission = .readOnly + let _ = try syncEngine.modifyRecords(scope: .shared, saving: [freshShare]) + try await withDependencies { + $0.datetime.now.addTimeInterval(1) + } operation: { + try await self.userDatabase.userWrite { db in + try db.seed { + Reminder(id: 1, title: "Get milk", remindersListID: 1) + } + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .shared) + try await syncEngine.processPendingRecordZoneChanges(scope: .shared) + + try await self.userDatabase.userRead { db in + try #expect(Reminder.all.fetchCount(db) == 0) + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), + recordType: "cloudkit.share", + parent: nil, + share: nil + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), + recordType: "remindersLists", + parent: nil, + share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), + id: 1, + title: "Personal" + ) + ] + ) + ) + """ + } + } + + + // Edit a record while locally we think we have permission, but CloudKit has newer permissions + // that are read only. + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func editRecordWhenLocalHasPermissionsButCloudKitDoesNot() async throws { + let externalZone = CKRecordZone( + zoneID: CKRecordZone.ID( + zoneName: "external.zone", + ownerName: "external.owner" + ) + ) + try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() + + let remindersListRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID) + ) + remindersListRecord.setValue(1, forKey: "id", at: now) + remindersListRecord.setValue("Personal", forKey: "title", at: now) + let share = CKShare( + rootRecord: remindersListRecord, + shareID: CKRecord.ID( + recordName: "share-\(remindersListRecord.recordID.recordName)", + zoneID: remindersListRecord.recordID.zoneID + ) + ) + share.publicPermission = .readWrite + share.currentUserParticipant?.permission = .readWrite + + try await syncEngine + .acceptShare( + metadata: ShareMetadata( + containerIdentifier: container.containerIdentifier!, + hierarchicalRootRecordID: remindersListRecord.recordID, + rootRecord: remindersListRecord, + share: share + ) + ) + + let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare + freshShare.publicPermission = .readOnly + let _ = try syncEngine.modifyRecords(scope: .shared, saving: [freshShare]) + + try await withDependencies { + $0.datetime.now.addTimeInterval(1) + } operation: { + try await self.userDatabase.userWrite { db in + try RemindersList.find(1).update { $0.title = "Business" }.execute(db) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .shared) + + try await self.userDatabase.userRead { db in + try #expect(RemindersList.find(1).fetchOne(db) == RemindersList(id: 1, title: "Personal")) + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner), + recordType: "cloudkit.share", + parent: nil, + share: nil + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), + recordType: "remindersLists", + parent: nil, + share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)), + id: 1, + title: "Personal" + ) + ] + ) + ) + """ + } + } } } From 12bafa81dfbf658f480b19de6787f70d8cf23881 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 1 Sep 2025 21:31:41 -0500 Subject: [PATCH 3/4] wip --- .../CloudKit/Internal/MockCloudDatabase.swift | 5 +- .../SharingGRDBCore/CloudKit/SyncEngine.swift | 2 +- .../MockCloudDatabaseTests.swift | 16 ---- .../SharingPermissionsTests.swift | 77 +++++++++++-------- .../SyncEngineLifecycleTests.swift | 7 ++ 5 files changed, 59 insertions(+), 48 deletions(-) diff --git a/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift index 3bf25625..ac484864 100644 --- a/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift @@ -139,8 +139,11 @@ package final class MockCloudDatabase: CloudDatabase { } return nil } - let share = share(for: root(of: recordToSave)) + let rootRecord = root(of: recordToSave) + let share = share(for: rootRecord) + let isSavingShare = recordsToSave.contains { $0.recordID == share?.recordID } if + !isSavingShare, !(recordToSave is CKShare), let share, !(share.publicPermission == .readWrite diff --git a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift index cebf4b81..35c9e6ce 100644 --- a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift +++ b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift @@ -356,7 +356,7 @@ .select(\.pendingRecordZoneChange) .fetchAll(db) } - let changesByIsPrivate = Dictionary.init(grouping: pendingRecordZoneChanges) { + let changesByIsPrivate = Dictionary(grouping: pendingRecordZoneChanges) { switch $0 { case .deleteRecord(let recordID), .saveRecord(let recordID): recordID.zoneID.ownerName == CKCurrentUserDefaultName diff --git a/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift index df848bc1..cc143b6f 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -414,21 +414,5 @@ extension BaseCloudKitTests { let newShare = try syncEngine.private.database.record(for: CKRecord.ID(recordName: "share")) _ = try syncEngine.modifyRecords(scope: .private, saving: [newShare]) } - - @Test func writePermission() async throws { - let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1")) - let share = CKShare(rootRecord: record, shareID: CKRecord.ID(recordName: "share")) - share.publicPermission = .readOnly - _ = try syncEngine.modifyRecords(scope: .private, saving: [share, record]) - - let freshRecord = try syncEngine.private.database.record(for: record.recordID) - try await withKnownIssue { - try await syncEngine.modifyRecords(scope: .private, saving: [freshRecord]).notify() - } matching: { issue in - issue.description.hasSuffix(""" - You do not have permission to write to this record: 1 - """) - } - } } } diff --git a/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift b/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift index a2f480fb..3207d88b 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/SharingPermissionsTests.swift @@ -105,11 +105,29 @@ extension BaseCloudKitTests { ) remindersListRecord.setValue(1, forKey: "id", at: now) remindersListRecord.setValue("Personal", forKey: "title", at: now) + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) + ) + reminderRecord.setValue(1, forKey: "id", at: now) + reminderRecord.setValue("Get milk", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) + + _ = try syncEngine.modifyRecords( + scope: .shared, + saving: [reminderRecord, remindersListRecord] + ) + + let freshRemindersListRecord = try syncEngine.shared.database.record( + for: remindersListRecord.recordID + ) + let share = CKShare( - rootRecord: remindersListRecord, + rootRecord: freshRemindersListRecord, shareID: CKRecord.ID( - recordName: "share-\(remindersListRecord.recordID.recordName)", - zoneID: remindersListRecord.recordID.zoneID + recordName: "share-\(freshRemindersListRecord.recordID.recordName)", + zoneID: freshRemindersListRecord.recordID.zoneID ) ) share.publicPermission = .readOnly @@ -119,20 +137,11 @@ extension BaseCloudKitTests { .acceptShare( metadata: ShareMetadata( containerIdentifier: container.containerIdentifier!, - hierarchicalRootRecordID: remindersListRecord.recordID, - rootRecord: remindersListRecord, + hierarchicalRootRecordID: freshRemindersListRecord.recordID, + rootRecord: freshRemindersListRecord, share: share ) ) - let reminderRecord = CKRecord( - recordType: Reminder.tableName, - recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) - ) - reminderRecord.setValue(1, forKey: "id", at: now) - reminderRecord.setValue("Get milk", forKey: "title", at: now) - reminderRecord.setValue(1, forKey: "remindersListID", at: now) - reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) - try await syncEngine.modifyRecords(scope: .shared, saving: [reminderRecord]).notify() try await self.userDatabase.userWrite { db in let error = #expect(throws: DatabaseError.self) { @@ -198,11 +207,28 @@ extension BaseCloudKitTests { ) remindersListRecord.setValue(1, forKey: "id", at: now) remindersListRecord.setValue("Personal", forKey: "title", at: now) + let reminderRecord = CKRecord( + recordType: Reminder.tableName, + recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) + ) + reminderRecord.setValue(1, forKey: "id", at: now) + reminderRecord.setValue("Get milk", forKey: "title", at: now) + reminderRecord.setValue(1, forKey: "remindersListID", at: now) + reminderRecord.setValue(false, forKey: "isCompleted", at: now) + reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) + _ = try syncEngine.modifyRecords( + scope: .shared, + saving: [remindersListRecord, reminderRecord] + ) + + let freshRemindersListRecord = try syncEngine.shared.database.record( + for: remindersListRecord.recordID + ) let share = CKShare( - rootRecord: remindersListRecord, + rootRecord: freshRemindersListRecord, shareID: CKRecord.ID( - recordName: "share-\(remindersListRecord.recordID.recordName)", - zoneID: remindersListRecord.recordID.zoneID + recordName: "share-\(freshRemindersListRecord.recordID.recordName)", + zoneID: freshRemindersListRecord.recordID.zoneID ) ) share.publicPermission = .readOnly @@ -212,21 +238,11 @@ extension BaseCloudKitTests { .acceptShare( metadata: ShareMetadata( containerIdentifier: container.containerIdentifier!, - hierarchicalRootRecordID: remindersListRecord.recordID, - rootRecord: remindersListRecord, + hierarchicalRootRecordID: freshRemindersListRecord.recordID, + rootRecord: freshRemindersListRecord, share: share ) ) - let reminderRecord = CKRecord( - recordType: Reminder.tableName, - recordID: Reminder.recordID(for: 1, zoneID: externalZone.zoneID) - ) - reminderRecord.setValue(1, forKey: "id", at: now) - reminderRecord.setValue("Get milk", forKey: "title", at: now) - reminderRecord.setValue(1, forKey: "remindersListID", at: now) - reminderRecord.setValue(false, forKey: "isCompleted", at: now) - reminderRecord.parent = CKRecord.Reference(record: remindersListRecord, action: .none) - try await syncEngine.modifyRecords(scope: .shared, saving: [reminderRecord]).notify() try await self.userDatabase.userWrite { db in let error = #expect(throws: DatabaseError.self) { @@ -316,6 +332,7 @@ extension BaseCloudKitTests { let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare freshShare.publicPermission = .readOnly + freshShare.currentUserParticipant?.permission = .readOnly let _ = try syncEngine.modifyRecords(scope: .shared, saving: [freshShare]) try await withDependencies { $0.datetime.now.addTimeInterval(1) @@ -327,7 +344,6 @@ extension BaseCloudKitTests { } } try await syncEngine.processPendingRecordZoneChanges(scope: .shared) - try await syncEngine.processPendingRecordZoneChanges(scope: .shared) try await self.userDatabase.userRead { db in try #expect(Reminder.all.fetchCount(db) == 0) @@ -404,6 +420,7 @@ extension BaseCloudKitTests { let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare freshShare.publicPermission = .readOnly + freshShare.currentUserParticipant?.permission = .readOnly let _ = try syncEngine.modifyRecords(scope: .shared, saving: [freshShare]) try await withDependencies { diff --git a/Tests/SharingGRDBTests/CloudKitTests/SyncEngineLifecycleTests.swift b/Tests/SharingGRDBTests/CloudKitTests/SyncEngineLifecycleTests.swift index 1d757710..769d8c2c 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/SyncEngineLifecycleTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/SyncEngineLifecycleTests.swift @@ -26,6 +26,8 @@ extension BaseCloudKitTests { } } + try await Task.sleep(for: .seconds(0.5)) + try await userDatabase.userRead { db in let remindersListMetadata = try #require(try RemindersList.metadata(for: 1).fetchOne(db)) #expect(remindersListMetadata.lastKnownServerRecord == nil) @@ -103,6 +105,8 @@ extension BaseCloudKitTests { try RemindersList.find(1).delete().execute(db) } + try await Task.sleep(for: .seconds(0.5)) + try await syncEngine.start() try await syncEngine.processPendingRecordZoneChanges(scope: .private) @@ -139,6 +143,7 @@ extension BaseCloudKitTests { try await userDatabase.userWrite { db in try RemindersList.find(1).update { $0.title += "!" }.execute(db) } + try await Task.sleep(for: .seconds(0.5)) try await userDatabase.read { db in try #expect(PendingRecordZoneChange.all.fetchCount(db) == 1) @@ -209,6 +214,7 @@ extension BaseCloudKitTests { } } + try await Task.sleep(for: .seconds(0.5)) try await syncEngine.start() try await syncEngine.processPendingRecordZoneChanges(scope: .shared) @@ -337,6 +343,7 @@ extension BaseCloudKitTests { try RemindersList.find(1).delete().execute(db) } + try await Task.sleep(for: .seconds(0.5)) try await syncEngine.start() try await syncEngine.processPendingRecordZoneChanges(scope: .private) From 0ee58c5348222cf5854c94b3c5a1ba8af888eb9d Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 1 Sep 2025 21:37:39 -0500 Subject: [PATCH 4/4] clean up --- .../CloudKit/Internal/MockCloudDatabase.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift index ac484864..74388699 100644 --- a/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift @@ -150,11 +150,6 @@ package final class MockCloudDatabase: CloudDatabase { || share.currentUserParticipant?.permission == .readWrite) { saveResults[recordToSave.recordID] = .failure(CKError(.permissionFailure)) -// reportIssue( -// """ -// You do not have permission to write to this record: \(recordToSave.recordID.recordName) -// """ -// ) return }