fix: keep ios push renewal from reporting an extension the server refused

plugins/ios-push/Jmap.swift

@@ -21,6 +21,8 @@ enum JmapError: Error {
   case needsRefresh
   case http(Int)
   case badResponse
+  /// A /set call listed the record under notUpdated (or not under updated).
+  case notUpdated(String)
 }
 
 /// Credentials as the app stores them (src/api/jmap-client.ts StoredCredentials).
@@ -166,9 +168,30 @@ enum Jmap {
       ["PushSubscription/get", ["ids": [id], "properties": pushSubscriptionProperties], "1"],
     ]
     let responses = try await call(apiUrl: apiUrl, authorization: authorization, using: ["urn:ietf:params:jmap:core"], methodCalls: methodCalls)
+    if let refused = notUpdated(responses, method: "PushSubscription/set", callId: "0", id: id) {
+      throw JmapError.notUpdated(refused)
+    }
     return listedPushSubscription(responses, callId: "1", id: id)
   }
 
+  /// Why the /set answer `callId` didn't update `id`, or nil when it did.
+  /// A method error or an answer without `id` under updated counts as refused.
+  static func notUpdated(_ responses: [[Any]], method: String, callId: String, id: String) -> String? {
+    for response in responses where response.count >= 3 && response[2] as? String == callId {
+      guard response[0] as? String == method, let args = response[1] as? [String: Any] else {
+        let args = response[1] as? [String: Any]
+        return (args?["type"] as? String) ?? "error"
+      }
+      if let refused = (args["notUpdated"] as? [String: Any])?[id] {
+        let error = refused as? [String: Any]
+        return (error?["description"] as? String) ?? (error?["type"] as? String) ?? "notUpdated"
+      }
+      if let updated = args["updated"] as? [String: Any], updated.keys.contains(id) { return nil }
+      return "not in updated"
+    }
+    return "no answer"
+  }
+
   static func fetch(apiUrl: URL, accountId: String, emailIds: [String], authorization: String) async throws -> JmapFetchResult {
     var request = URLRequest(url: apiUrl)
     request.httpMethod = "POST"

plugins/ios-push/PushRenewal.swift

@@ -83,23 +83,32 @@ enum PushRenewal {
       }
       // The recorded expiry was stale: the server's copy has time left.
       if let expires = current.expires, !needsExtension(expires) {
-        record(key: key, account: account, expires: expires)
+        record(key: key, subscriptionId: subscriptionId, expires: expires)
         return false
       }
       let wanted = formatDate(Date().addingTimeInterval(wantedSeconds))
       let extended = try await Jmap.extendPushSubscription(
         apiUrl: apiUrl, id: subscriptionId, expires: wanted, authorization: authorization)
-      if let expires = extended?.expires { record(key: key, account: account, expires: expires) }
+      if let expires = extended?.expires { record(key: key, subscriptionId: subscriptionId, expires: expires) }
       pushLog.info("extended subscription \(subscriptionId, privacy: .public)")
       return true
+    } catch JmapError.notUpdated(let reason) {
+      // The server kept the old expiry: not extended, nothing to record.
+      pushLog.error("server did not extend subscription \(subscriptionId, privacy: .public): \(reason, privacy: .public)")
+      return false
     } catch {
       pushLog.error("renewal failed: \(String(describing: error), privacy: .public)")
       return false
     }
   }
 
-  private static func record(key: String, account: [String: Any], expires: String) {
-    var json = account
+  /// Writes the expiry into the account's entry as it is now, not as it was
+  /// when the renewal started: the app may have replaced the subscription (or
+  /// the credentials key) meanwhile. Only the expiry changes, and only while
+  /// the entry still names the subscription that was extended.
+  private static func record(key: String, subscriptionId: String, expires: String) {
+    guard var json = SharedKeychain.readJSON(key),
+          json["subscriptionId"] as? String == subscriptionId else { return }
     json["subscriptionExpires"] = expires
     SharedKeychain.update(key, json: json)
   }

src/lib/__tests__/ios-push-renewal.test.ts

@@ -143,6 +143,36 @@ describe('the native renewal', () => {
     expect(set).toBeGreaterThan(own);
   });
 
+  it('treats an update the server refused as not extended, and records nothing', () => {
+    const jmap = read('plugins/ios-push/Jmap.swift');
+    const extend = jmap.slice(jmap.indexOf('static func extendPushSubscription('));
+    const check = extend.indexOf('notUpdated(responses, method: "PushSubscription/set", callId: "0", id: id)');
+    expect(check).toBeGreaterThan(-1);
+    expect(extend.indexOf('throw JmapError.notUpdated(')).toBeGreaterThan(check);
+    expect(check).toBeLessThan(extend.indexOf('return listedPushSubscription(responses, callId: "1", id: id)'));
+    // The renewal stops on it before logging "extended" or recording.
+    const body = renewal.slice(renewal.indexOf('static func renew(key:'), renewal.indexOf('private static func record('));
+    const refused = body.match(/catch JmapError\.notUpdated[\s\S]*?\}/)?.[0] ?? '';
+    expect(refused).toContain('return false');
+    expect(refused).not.toContain('record(');
+    expect(body.indexOf('try await Jmap.extendPushSubscription(')).toBeLessThan(body.indexOf('pushLog.info("extended subscription'));
+  });
+
+  it('writes back only the expiry, into the entry as it is now, while it names the same subscription', () => {
+    const record = renewal.slice(renewal.indexOf('private static func record('));
+    expect(record).toMatch(/^private static func record\(key: String, subscriptionId: String, expires: String\)/);
+    expect(record).toMatch(/guard var json = SharedKeychain\.readJSON\(key\),\s*json\["subscriptionId"\] as\? String == subscriptionId else \{ return \}/);
+    expect(record).toMatch(/json\["subscriptionExpires"\] = expires\s*SharedKeychain\.update\(key, json: json\)/);
+    expect(renewal).not.toMatch(/record\(key: key, account:/);
+  });
+
+  it('reads a refused update the way JMAP reports it', () => {
+    const jmap = read('plugins/ios-push/Jmap.swift');
+    const helper = jmap.slice(jmap.indexOf('static func notUpdated('));
+    expect(helper).toMatch(/args\["notUpdated"\] as\? \[String: Any\]\)\?\[id\]/);
+    expect(helper).toMatch(/updated\.keys\.contains\(id\) \{ return nil \}/);
+  });
+
   it('runs from the notification extension after the fetch for the pushed account', () => {
     const renew = nse.indexOf('await PushRenewal.renewIfDue(jmapAccountId: accountId)');
     expect(renew).toBeGreaterThan(nse.indexOf('await NotificationService.enrich('));