From b117aa5db149dcb6c1b2b7a76a42c2f39e095ef3 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 30 Apr 2024 15:29:15 +0200 Subject: [PATCH] Enable field validation for Sync payloads --- Sources/DDGSync/DDGSync.swift | 7 ----- Sources/DDGSync/SyncMetadataStore.swift | 7 +---- .../internal/SyncableBookmarkAdapter.swift | 12 +++----- .../internal/SyncableCredentialsAdapter.swift | 30 +++++++------------ .../Bookmarks/BookmarksProviderTests.swift | 15 ++-------- .../SyncableBookmarkValidationTests.swift | 12 -------- .../CredentialsProviderTests.swift | 16 ++-------- .../SyncableCredentialsValidationTests.swift | 24 --------------- .../Settings/SettingsProviderTests.swift | 8 ++--- 9 files changed, 21 insertions(+), 110 deletions(-) diff --git a/Sources/DDGSync/DDGSync.swift b/Sources/DDGSync/DDGSync.swift index e107a47bd..9d560056d 100644 --- a/Sources/DDGSync/DDGSync.swift +++ b/Sources/DDGSync/DDGSync.swift @@ -23,13 +23,6 @@ import DDGSyncCrypto import Foundation public class DDGSync: DDGSyncing { - /** - * Temporary feature flag that controls field validation feature. - * - * The flag must be set to false at all times before field validation is released. - * Before field validations release, it should be removed. - */ - public static let isFieldValidationEnabled = false public static let bundle = Bundle.module diff --git a/Sources/DDGSync/SyncMetadataStore.swift b/Sources/DDGSync/SyncMetadataStore.swift index 6bc5df234..d987eb90c 100644 --- a/Sources/DDGSync/SyncMetadataStore.swift +++ b/Sources/DDGSync/SyncMetadataStore.swift @@ -124,9 +124,6 @@ public final class LocalSyncMetadataStore: SyncMetadataStore { } public func updateLocalTimestamp(_ localTimestamp: Date?, forFeatureNamed name: String) { - guard DDGSync.isFieldValidationEnabled else { - return - } context.performAndWait { let feature = SyncFeatureUtils.fetchFeature(with: name, in: context) feature?.lastSyncLocalTimestamp = localTimestamp @@ -138,9 +135,7 @@ public final class LocalSyncMetadataStore: SyncMetadataStore { context.performAndWait { let feature = SyncFeatureUtils.fetchFeature(with: name, in: context) feature?.lastModified = serverTimestamp - if DDGSync.isFieldValidationEnabled { - feature?.lastSyncLocalTimestamp = localTimestamp - } + feature?.lastSyncLocalTimestamp = localTimestamp feature?.featureState = state try? context.save() diff --git a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift index 6a9d83f90..34a2ec828 100644 --- a/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift +++ b/Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift @@ -125,19 +125,15 @@ extension Syncable { if let title = bookmark.title { let encryptedTitle = try encrypt(title) - if DDGSync.isFieldValidationEnabled { - guard encryptedTitle.count <= BookmarkValidationConstraints.maxEncryptedBookmarkTitleLength else { - throw SyncableBookmarkError.validationFailed - } + guard encryptedTitle.count <= BookmarkValidationConstraints.maxEncryptedBookmarkTitleLength else { + throw SyncableBookmarkError.validationFailed } payload["title"] = encryptedTitle } if let url = bookmark.url { let encryptedURL = try encrypt(url) - if DDGSync.isFieldValidationEnabled { - guard encryptedURL.count <= BookmarkValidationConstraints.maxEncryptedBookmarkURLLength else { - throw SyncableBookmarkError.validationFailed - } + guard encryptedURL.count <= BookmarkValidationConstraints.maxEncryptedBookmarkURLLength else { + throw SyncableBookmarkError.validationFailed } payload["page"] = ["url": encryptedURL] } diff --git a/Sources/SyncDataProviders/Credentials/internal/SyncableCredentialsAdapter.swift b/Sources/SyncDataProviders/Credentials/internal/SyncableCredentialsAdapter.swift index 2352a719e..ee658b5b3 100644 --- a/Sources/SyncDataProviders/Credentials/internal/SyncableCredentialsAdapter.swift +++ b/Sources/SyncDataProviders/Credentials/internal/SyncableCredentialsAdapter.swift @@ -85,47 +85,37 @@ extension Syncable { if let title = credential.account.title { let encryptedTitle = try encrypt(title) - if DDGSync.isFieldValidationEnabled { - guard encryptedTitle.count <= CredentialValidationConstraints.maxEncryptedTitleLength else { - throw SyncableCredentialError.validationFailed - } + guard encryptedTitle.count <= CredentialValidationConstraints.maxEncryptedTitleLength else { + throw SyncableCredentialError.validationFailed } payload["title"] = encryptedTitle } if let domain = credential.account.domain { let encryptedDomain = try encrypt(domain) - if DDGSync.isFieldValidationEnabled { - guard encryptedDomain.count <= CredentialValidationConstraints.maxEncryptedDomainLength else { - throw SyncableCredentialError.validationFailed - } + guard encryptedDomain.count <= CredentialValidationConstraints.maxEncryptedDomainLength else { + throw SyncableCredentialError.validationFailed } payload["domain"] = encryptedDomain } if let username = credential.account.username { let encryptedUsername = try encrypt(username) - if DDGSync.isFieldValidationEnabled { - guard encryptedUsername.count <= CredentialValidationConstraints.maxEncryptedUsernameLength else { - throw SyncableCredentialError.validationFailed - } + guard encryptedUsername.count <= CredentialValidationConstraints.maxEncryptedUsernameLength else { + throw SyncableCredentialError.validationFailed } payload["username"] = encryptedUsername } if let notes = credential.account.notes { let encryptedNotes = try encrypt(notes) - if DDGSync.isFieldValidationEnabled { - guard encryptedNotes.count <= CredentialValidationConstraints.maxEncryptedNotesLength else { - throw SyncableCredentialError.validationFailed - } + guard encryptedNotes.count <= CredentialValidationConstraints.maxEncryptedNotesLength else { + throw SyncableCredentialError.validationFailed } payload["notes"] = encryptedNotes } if let passwordData = credential.password, let password = String(data: passwordData, encoding: .utf8) { let encryptedPassword = try encrypt(password) - if DDGSync.isFieldValidationEnabled { - guard encryptedPassword.count <= CredentialValidationConstraints.maxEncryptedPasswordLength else { - throw SyncableCredentialError.validationFailed - } + guard encryptedPassword.count <= CredentialValidationConstraints.maxEncryptedPasswordLength else { + throw SyncableCredentialError.validationFailed } payload["password"] = encryptedPassword } diff --git a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift index d3cc0bf8a..974097040 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift @@ -29,9 +29,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { func testThatLastSyncTimestampIsNilByDefault() { XCTAssertNil(provider.lastSyncTimestamp) - if DDGSync.isFieldValidationEnabled { - XCTAssertNil(provider.lastSyncLocalTimestamp) - } + XCTAssertNil(provider.lastSyncLocalTimestamp) } func testThatLastSyncTimestampIsPersisted() throws { @@ -39,9 +37,7 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { let date = Date() provider.updateSyncTimestamps(server: "12345", local: date) XCTAssertEqual(provider.lastSyncTimestamp, "12345") - if DDGSync.isFieldValidationEnabled { - XCTAssertEqual(provider.lastSyncLocalTimestamp, date) - } + XCTAssertEqual(provider.lastSyncLocalTimestamp, date) } func testThatPrepareForFirstSyncClearsLastSyncTimestampAndSetsModifiedAtForAllBookmarks() async throws { @@ -146,9 +142,6 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { } func testThatFetchChangedObjectsFiltersOutInvalidBookmarksAndTruncatesFolderTitles() async throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let longTitle = String(repeating: "x", count: 10000) @@ -262,10 +255,6 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase { } func testThatItemsThatFailedValidationRetainTheirTimestamps() async throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - let context = bookmarksDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) let longValue = String(repeating: "x", count: 10000) let timestamp = Date() diff --git a/Tests/SyncDataProvidersTests/Bookmarks/SyncableBookmarkValidationTests.swift b/Tests/SyncDataProvidersTests/Bookmarks/SyncableBookmarkValidationTests.swift index 9af6ca83a..f478c7d3c 100644 --- a/Tests/SyncDataProvidersTests/Bookmarks/SyncableBookmarkValidationTests.swift +++ b/Tests/SyncDataProvidersTests/Bookmarks/SyncableBookmarkValidationTests.swift @@ -60,27 +60,15 @@ final class SyncableBookmarkValidationTests: XCTestCase { } func testWhenBookmarkFieldsPassLengthValidationThenSyncableIsInitializedWithoutThrowingErrors() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - XCTAssertNoThrow(try Syncable(bookmark: bookmark, encryptedUsing: { $0 })) } func testWhenBookmarkTitleIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - bookmark.title = String(repeating: "x", count: 10000) assertSyncableInitializerThrowsValidationError() } func testWhenBookmarkURLIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - bookmark.url = String(repeating: "x", count: 10000) assertSyncableInitializerThrowsValidationError() } diff --git a/Tests/SyncDataProvidersTests/Credentials/CredentialsProviderTests.swift b/Tests/SyncDataProvidersTests/Credentials/CredentialsProviderTests.swift index 950ced4f8..3f160133a 100644 --- a/Tests/SyncDataProvidersTests/Credentials/CredentialsProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Credentials/CredentialsProviderTests.swift @@ -28,9 +28,7 @@ final class CredentialsProviderTests: CredentialsProviderTestsBase { func testThatLastSyncTimestampIsNilByDefault() { XCTAssertNil(provider.lastSyncTimestamp) - if DDGSync.isFieldValidationEnabled { - XCTAssertNil(provider.lastSyncLocalTimestamp) - } + XCTAssertNil(provider.lastSyncLocalTimestamp) } func testThatLastSyncTimestampIsPersisted() throws { @@ -38,9 +36,7 @@ final class CredentialsProviderTests: CredentialsProviderTestsBase { let date = Date() provider.updateSyncTimestamps(server: "12345", local: date) XCTAssertEqual(provider.lastSyncTimestamp, "12345") - if DDGSync.isFieldValidationEnabled { - XCTAssertEqual(provider.lastSyncLocalTimestamp, date) - } + XCTAssertEqual(provider.lastSyncLocalTimestamp, date) } func testThatPrepareForFirstSyncClearsLastSyncTimestampAndSetsModifiedAtForAllCredentials() throws { @@ -82,10 +78,6 @@ final class CredentialsProviderTests: CredentialsProviderTestsBase { } func testThatFetchChangedObjectsFiltersOutInvalidCredentials() async throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - let longValue = String(repeating: "x", count: 10000) try secureVault.inDatabaseTransaction { database in @@ -148,10 +140,6 @@ final class CredentialsProviderTests: CredentialsProviderTestsBase { } func testThatItemsThatFailedValidationRetainTheirTimestamps() async throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - let longValue = String(repeating: "x", count: 10000) let timestamp = Date().withMillisecondPrecision diff --git a/Tests/SyncDataProvidersTests/Credentials/SyncableCredentialsValidationTests.swift b/Tests/SyncDataProvidersTests/Credentials/SyncableCredentialsValidationTests.swift index 14af5e54f..03b757355 100644 --- a/Tests/SyncDataProvidersTests/Credentials/SyncableCredentialsValidationTests.swift +++ b/Tests/SyncDataProvidersTests/Credentials/SyncableCredentialsValidationTests.swift @@ -34,54 +34,30 @@ final class SyncableCredentialsValidationTests: XCTestCase { } func testWhenCredentialsFieldsPassLengthValidationThenSyncableIsInitializedWithoutThrowingErrors() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - XCTAssertNoThrow(try Syncable(syncableCredentials: syncableCredentials, encryptedUsing: { $0 })) } func testWhenAccountTitleIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - syncableCredentials.account?.title = String(repeating: "x", count: 10000) assertSyncableInitializerThrowsValidationError() } func testWhenAccountUsernameIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - syncableCredentials.account?.username = String(repeating: "x", count: 10000) assertSyncableInitializerThrowsValidationError() } func testWhenAccountDomainIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - syncableCredentials.account?.domain = String(repeating: "x", count: 10000) assertSyncableInitializerThrowsValidationError() } func testWhenAccountNotesIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - syncableCredentials.account?.notes = String(repeating: "x", count: 10000) assertSyncableInitializerThrowsValidationError() } func testWhenPasswordIsTooLongThenSyncableInitializerThrowsError() throws { - guard DDGSync.isFieldValidationEnabled else { - throw XCTSkip("Field validation is disabled") - } - syncableCredentials.credentials?.password = String(repeating: "x", count: 10000).data(using: .utf8) assertSyncableInitializerThrowsValidationError() } diff --git a/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift b/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift index fbf7bda1f..af1166625 100644 --- a/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift +++ b/Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift @@ -28,9 +28,7 @@ final class SettingsProviderTests: SettingsProviderTestsBase { func testThatLastSyncTimestampIsNilByDefault() { XCTAssertNil(provider.lastSyncTimestamp) - if DDGSync.isFieldValidationEnabled { - XCTAssertNil(provider.lastSyncLocalTimestamp) - } + XCTAssertNil(provider.lastSyncLocalTimestamp) } func testThatLastSyncTimestampIsPersisted() throws { @@ -38,9 +36,7 @@ final class SettingsProviderTests: SettingsProviderTestsBase { let date = Date() provider.updateSyncTimestamps(server: "12345", local: date) XCTAssertEqual(provider.lastSyncTimestamp, "12345") - if DDGSync.isFieldValidationEnabled { - XCTAssertEqual(provider.lastSyncLocalTimestamp, date) - } + XCTAssertEqual(provider.lastSyncLocalTimestamp, date) } func testThatPrepareForFirstSyncClearsLastSyncTimestampAndSetsModifiedAtForAllSettings() throws {