diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift index 4a4e82dfe3..cc7eb8dc91 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift @@ -404,13 +404,15 @@ public final class BuzzDevPushEnrollmentDriver { deviceToken: Data, relayURL: URL ) async throws -> BuzzPushEndpointGrantRecord { - try await enroll(deviceToken: deviceToken, relayURL: relayURL, rejectedReplacements: 0) + try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: nil) } + /// `rejected` is a journal the gateway terminally rejected. It stays durable, + /// with its rejection count, until a fresh request overwrites it. private func enroll( deviceToken: Data, relayURL: URL, - rejectedReplacements: Int + replacing rejected: BuzzPushPendingEnrollmentRecord? ) async throws -> BuzzPushEndpointGrantRecord { precondition(!deviceToken.isEmpty, "The APNs device token must not be empty") let relayOrigin = try Self.relayOrigin(relayURL) @@ -424,10 +426,11 @@ public final class BuzzDevPushEnrollmentDriver { let storedForOrigin = storedRecords.first { $0.gatewayOrigin == gatewayOrigin && $0.relayOrigin == relayOrigin.text } - let pendingEnrollment = try store.pendingEnrollment( + let journal = try store.pendingEnrollment( gatewayOrigin: gatewayOrigin, relayOrigin: relayOrigin.text ) + let pendingEnrollment = journal == rejected ? nil : journal if let pending = pendingEnrollment, pending.relayPubkey != relayPubkey || pending.endpointHash != endpointHash || pending.expiresAt <= nowSeconds @@ -435,14 +438,13 @@ public final class BuzzDevPushEnrollmentDriver { // Finish an unexpired request using its exact journaled endpoint before // replacing it. A token/key change does not make a response-lost request // safe to discard, nor does it require revoking sibling installations. - var replacements = 0 if pending.expiresAt > nowSeconds { - if case .replaced(let count) = try await resumeEnrollment( + if case .rejected = try await resumeEnrollment( pending, storedRecords: storedRecords, relayMetadataPubkey: nil ) { - replacements = count + return try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: pending) } } else { try store.removePendingEnrollment( @@ -450,11 +452,7 @@ public final class BuzzDevPushEnrollmentDriver { relayOrigin: relayOrigin.text ) } - return try await enroll( - deviceToken: deviceToken, - relayURL: relayURL, - rejectedReplacements: replacements - ) + return try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: nil) } let newestSharedGrant = storedRecords.filter { $0.gatewayOrigin == gatewayOrigin && $0.relayPubkey == relayPubkey @@ -561,9 +559,10 @@ public final class BuzzDevPushEnrollmentDriver { keyId: attestation.keyId, attestation: attestation.attestation, preparedAt: nowSeconds, - rejectedReplacements: rejectedReplacements + rejectedReplacements: rejected.map { ($0.rejectedReplacements ?? 0) + 1 } ) // The exact signed request is durable before the first network attempt. + // Saving it is also what replaces a rejected journal. try store.savePendingEnrollment(pending) } @@ -575,12 +574,8 @@ public final class BuzzDevPushEnrollmentDriver { ) { case .completed(let record): return record - case .replaced(let count): - return try await enroll( - deviceToken: deviceToken, - relayURL: relayURL, - rejectedReplacements: count - ) + case .rejected: + return try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: pending) } } return try await completeEnrollment( @@ -592,8 +587,8 @@ public final class BuzzDevPushEnrollmentDriver { private enum Resumption { case completed(BuzzPushEndpointGrantRecord) - /// The journal was discarded; a fresh request carries this rejection count. - case replaced(rejectedReplacements: Int) + /// The gateway terminally rejected the journal, which may now be replaced. + case rejected } private func resumeEnrollment( @@ -615,37 +610,31 @@ public final class BuzzDevPushEnrollmentDriver { // Only a terminal response permits replacing the old journal with a fresh // challenge and key. A fresh request's failure propagates instead of // recursively retrying forever. - try store.removePendingEnrollment( - gatewayOrigin: gatewayOrigin, - relayOrigin: pending.relayOrigin - ) - return .replaced( - rejectedReplacements: actual == 404 ? 0 : (pending.rejectedReplacements ?? 0) + 1 - ) + return .rejected } } /// 404 means the exact replay found no committed installation and its - /// challenge expired. The gateway returns 400 and 401 before looking up a - /// committed installation, so the same bytes can never recover one; those - /// journals are replaced only once old enough, with the wait doubling for - /// each consecutive replacement, because every replacement generates and - /// attests a new App Attest key. + /// challenge is gone. The gateway returns 400 and 401 before looking up a + /// committed installation, so the same bytes can never recover one, even + /// when an earlier attempt committed and its response was lost. Every + /// replacement generates and attests a new App Attest key, so only a first + /// 404 replaces at once. Otherwise the journal must be old enough, with the + /// wait doubling for each consecutive replacement. That bounds new keys when + /// a replacement itself fails, for example with 409 against a live + /// installation for the same token, whose next replay then returns 404. private func replacesRejectedJournal( _ pending: BuzzPushPendingEnrollmentRecord, _ status: Int ) -> Bool { - switch status { - case 404: + guard [400, 401, 404].contains(status) else { return false } + let replacements = pending.rejectedReplacements ?? 0 + guard status != 404 || replacements > 0, let preparedAt = pending.preparedAt else { return true - case 400, 401: - guard let preparedAt = pending.preparedAt else { return true } - let nowSeconds = Int64(now().timeIntervalSince1970) - let doublings = min(pending.rejectedReplacements ?? 0, 20) - return nowSeconds - preparedAt >= Self.rejectedEnrollmentReplacementSeconds << doublings - default: - return false } + let nowSeconds = Int64(now().timeIntervalSince1970) + let wait = Self.rejectedEnrollmentReplacementSeconds << min(replacements, 20) + return nowSeconds - preparedAt >= wait } private func completeEnrollment( diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift index 3c97a64b08..f7fa488e70 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift @@ -395,15 +395,21 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { func testRejectedEnrollmentReplayIsReplacedOnlyOnTerminalStatus() async throws { // A journaled request the gateway rejects must not be replayed for its whole // lifetime, nor replaced (with a fresh App Attest key) on every retry; a - // transient failure must keep the journal. + // transient failure must keep the journal. A fresh 409 models a request that + // committed before its response was lost and was rejected once its + // attestation certificate expired: the replacement conflicts with the live + // installation and its consumed challenge then returns 404. let interval = BuzzDevPushEnrollmentDriver.rejectedEnrollmentReplacementSeconds let cases: [(replay: Int, fresh: Int?, elapsed: Int64, legacy: Bool)] = [ (400, 201, interval, false), (401, 201, interval, false), (404, 201, 0, false), (401, 201, 0, true), (401, nil, interval - 1, false), (401, 401, interval, false), - (503, nil, interval, false), + (401, 409, interval, false), (503, nil, interval, false), ] for (replayStatus, freshStatus, elapsed, legacy) in cases { - let label = "replay \(replayStatus) after \(elapsed)s, legacy \(legacy)" + let fresh = freshStatus.map(String.init) ?? "none" + let label = "replay \(replayStatus), fresh \(fresh) after \(elapsed)s, legacy \(legacy)" + // Status for a later exact replay of a fresh request. + let freshReplayStatus = freshStatus == 409 ? 404 : freshStatus let store = MemoryGrantStore() let appAttest = RecordingAppAttest() var clock = Self.now @@ -414,8 +420,10 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { lifetimeSeconds: 30 * 86_400 ) var challengeCount = 0 + var failChallenge = false var replayRequests = 0 var freshRequests = 0 + var sentChallenges = Set() URLProtocolStub.handler = { request in switch (request.httpMethod, request.url?.absoluteString) { case ("GET", "https://relay.example/"): @@ -423,9 +431,16 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { "push": ["keys": [["pubkey": Self.relayPubkey, "current": true]]] ]) case ("POST", "http://push.example/v1/installations/challenges"): + if failChallenge { + failChallenge = false + throw URLError(.notConnectedToInternet) + } challengeCount += 1 return Self.response(request, status: 200, json: [ - "challenge_id": challengeCount == 1 ? Self.firstChallengeId : Self.secondChallengeId, + "challenge_id": [Self.firstChallengeId, Self.secondChallengeId].indices + .contains(challengeCount - 1) + ? [Self.firstChallengeId, Self.secondChallengeId][challengeCount - 1] + : UUID().uuidString.lowercased(), "challenge": Self.challenge, "expires_at": clock + 300, ]) @@ -437,7 +452,12 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { return Self.response(request, status: replayStatus, json: ["error": "rejected"]) } freshRequests += 1 - let status = try XCTUnwrap(freshStatus, "\(label) must keep the journal") + let challengeId = try XCTUnwrap(body["challenge_id"] as? String) + let isReplay = !sentChallenges.insert(challengeId).inserted + let status = try XCTUnwrap( + isReplay ? freshReplayStatus : freshStatus, + "\(label) must keep the journal" + ) guard status == 201 else { return Self.response(request, status: status, json: ["error": "rejected"]) } @@ -469,7 +489,7 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { clock += elapsed // Two retries: the second must not replace a journal again. - for _ in 0..<2 { + for attempt in 0..<2 { do { let grant = try await driver.enroll(deviceToken: token, relayURL: Self.relayURL) XCTAssertEqual(freshStatus, 201, label) @@ -479,7 +499,11 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { route: "v1/installations", _, let actual, _ ) { // The failure stays visible and leaves a durable journal to retry. - XCTAssertEqual(actual, freshStatus ?? replayStatus, label) + XCTAssertEqual( + actual, + (attempt == 0 ? freshStatus : freshReplayStatus) ?? replayStatus, + label + ) XCTAssertEqual( store.pending.first?.challengeId, freshStatus == nil ? Self.firstChallengeId : Self.secondChallengeId, @@ -490,20 +514,51 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { XCTAssertEqual(freshRequests, freshStatus == nil ? 0 : freshStatus == 201 ? 1 : 2, label) XCTAssertEqual(appAttest.preparedAttestations.count, freshStatus == nil ? 1 : 2, label) - guard freshStatus == 401 else { continue } + guard let freshStatus, let freshReplayStatus, freshStatus != 201 else { continue } // Each consecutive replacement doubles the wait before the next one. - XCTAssertEqual(store.pending.first?.rejectedReplacements, 1) - for (advance, keys) in [(interval, 2), (interval, 3)] { - clock += advance - do { - _ = try await driver.enroll(deviceToken: token, relayURL: Self.relayURL) - XCTFail("Expected the persistent rejection") - } catch BuzzDevPushEnrollmentError.unexpectedStatus( - route: "v1/installations", _, actual: 401, _ - ) {} - XCTAssertEqual(appAttest.preparedAttestations.count, keys) + XCTAssertEqual(store.pending.first?.rejectedReplacements, 1, label) + clock += interval + await Self.assertEnrollmentStatus(driver, token, freshReplayStatus, label) + XCTAssertEqual(appAttest.preparedAttestations.count, 2, label) + + // A replacement that fails before its request is durable keeps the + // rejected journal and its count, even across a driver restart. + clock += interval + failChallenge = true + do { + _ = try await driver.enroll(deviceToken: token, relayURL: Self.relayURL) + XCTFail("Expected the simulated challenge failure: \(label)") + } catch { + XCTAssertEqual((error as NSError).domain, NSURLErrorDomain, label) } - XCTAssertEqual(store.pending.first?.rejectedReplacements, 2) + XCTAssertEqual(store.pending.first?.rejectedReplacements, 1, label) + let restarted = try makeDriver( + store: store, + appAttest: appAttest, + now: { Date(timeIntervalSince1970: TimeInterval(clock)) }, + lifetimeSeconds: 30 * 86_400 + ) + await Self.assertEnrollmentStatus(restarted, token, freshStatus, label) + XCTAssertEqual(appAttest.preparedAttestations.count, 3, label) + XCTAssertEqual(store.pending.first?.rejectedReplacements, 2, label) + } + } + + private static func assertEnrollmentStatus( + _ driver: BuzzDevPushEnrollmentDriver, + _ token: Data, + _ status: Int, + _ label: String + ) async { + do { + _ = try await driver.enroll(deviceToken: token, relayURL: relayURL) + XCTFail("Expected gateway status \(status): \(label)") + } catch BuzzDevPushEnrollmentError.unexpectedStatus( + route: "v1/installations", _, let actual, _ + ) { + XCTAssertEqual(actual, status, label) + } catch { + XCTFail("Unexpected error \(error): \(label)") } }