Skip to content

Commit

Permalink
Enable field validation for Sync payloads
Browse files Browse the repository at this point in the history
  • Loading branch information
@ayoy
ayoy committed Apr 30, 2024
1 parent 2681b52 commit b117aa5
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 110 deletions.
7 changes: 0 additions & 7 deletions Sources/DDGSync/DDGSync.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 1 addition & 6 deletions Sources/DDGSync/SyncMetadataStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
12 changes: 4 additions & 8 deletions Sources/SyncDataProviders/Bookmarks/internal/SyncableBookmarkAdapter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
Expand Down
30 changes: 10 additions & 20 deletions Sources/SyncDataProviders/Credentials/internal/SyncableCredentialsAdapter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 2 additions & 13 deletions Tests/SyncDataProvidersTests/Bookmarks/BookmarksProviderTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,15 @@ internal class BookmarksProviderTests: BookmarksProviderTestsBase {

func testThatLastSyncTimestampIsNilByDefault() {
XCTAssertNil(provider.lastSyncTimestamp)
if DDGSync.isFieldValidationEnabled {
XCTAssertNil(provider.lastSyncLocalTimestamp)
}
XCTAssertNil(provider.lastSyncLocalTimestamp)
}

func testThatLastSyncTimestampIsPersisted() throws {
try provider.registerFeature(withState: .readyToSync)
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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 0 additions & 12 deletions Tests/SyncDataProvidersTests/Bookmarks/SyncableBookmarkValidationTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
16 changes: 2 additions & 14 deletions Tests/SyncDataProvidersTests/Credentials/CredentialsProviderTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,15 @@ final class CredentialsProviderTests: CredentialsProviderTestsBase {

func testThatLastSyncTimestampIsNilByDefault() {
XCTAssertNil(provider.lastSyncTimestamp)
if DDGSync.isFieldValidationEnabled {
XCTAssertNil(provider.lastSyncLocalTimestamp)
}
XCTAssertNil(provider.lastSyncLocalTimestamp)
}

func testThatLastSyncTimestampIsPersisted() throws {
try provider.registerFeature(withState: .readyToSync)
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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
24 changes: 0 additions & 24 deletions Tests/SyncDataProvidersTests/Credentials/SyncableCredentialsValidationTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
8 changes: 2 additions & 6 deletions Tests/SyncDataProvidersTests/Settings/SettingsProviderTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,15 @@ final class SettingsProviderTests: SettingsProviderTestsBase {

func testThatLastSyncTimestampIsNilByDefault() {
XCTAssertNil(provider.lastSyncTimestamp)
if DDGSync.isFieldValidationEnabled {
XCTAssertNil(provider.lastSyncLocalTimestamp)
}
XCTAssertNil(provider.lastSyncLocalTimestamp)
}

func testThatLastSyncTimestampIsPersisted() throws {
try provider.registerFeature(withState: .readyToSync)
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 {
Expand Down

0 comments on commit b117aa5

Please sign in to comment.