[Test, do not merge] Claude review of #26132

Modules/Sources/WordPressData/Swift/Blog+WordPressComRestAPI.swift

@@ -17,4 +17,22 @@ extension Blog {
     public var isAccessibleThroughWPCom: Bool {
         wordPressComRestApi != nil
     }
+
+    /// Whether the blog has no usable remote because its WordPress.com account has no token.
+    ///
+    /// A blog hosted at WordPress.com, or one without a stored XML-RPC password, can only be reached with the
+    /// account's token. Remote factories return no remote for such a blog instead of falling back to Core REST
+    /// or XML-RPC with empty credentials. A Jetpack-connected self-hosted blog with a stored password keeps
+    /// using XML-RPC.
+    ///
+    /// Reading this property does not trigger the WordPress.com sign-in, unlike `wordPressComRestApi`.
+    @objc public var isMissingRequiredWordPressComToken: Bool {
+        guard let account else {
+            return false
+        }
+        if let token = account.authToken, !token.isEmpty {
+            return false
+        }
+        return isHostedAtWPcom || password == nil
+    }
 }

Modules/Sources/WordPressData/Swift/WPAccount+RestApi.swift

@@ -43,8 +43,12 @@ extension WPAccount {
             // We use a static function here because it's not safe to access `self` in this closure.
             // The `WPAccount` instance can be bound to any context object. There is no guarantee that the thread
             // from which this closure is called is the same as the one in the context object.
-            context?.perform {
-                WPAccount.handleInvalidToken(accountID: accountID, context: context)
+            //
+            // `WordPressComRestApi` calls this handler on the main queue. Waiting for the context blocks the main
+            // queue, where sign-in stores replacement tokens, so a replacement cannot be written between the
+            // comparison and the clear.
+            context?.performAndWait {
+                WPAccount.handleInvalidToken(accountID: accountID, rejectedToken: authToken, context: context)
             }
         }
 
@@ -69,30 +73,34 @@ extension WPAccount {
         return account?.wordPressComRestApi
     }
 
-    static func handleInvalidToken(accountID: TaggedManagedObjectID<WPAccount>, context: NSManagedObjectContext?) {
-        let account = try? context?.existingObject(with: accountID)
-        account?.authToken = nil
+    /// Clears the account's token after the server rejected it, and asks for the WordPress.com sign-in.
+    ///
+    /// This does not post `wpAccountDefaultWordPressComAccountChanged`: the account and its sites still exist, and
+    /// observers treat that notification without an object as a logout.
+    static func handleInvalidToken(
+        accountID: TaggedManagedObjectID<WPAccount>,
+        rejectedToken: String,
+        context: NSManagedObjectContext?
+    ) {
+        guard let account = try? context?.existingObject(with: accountID) else {
+            return
+        }
+
+        // Storing a new token does not cancel requests made with the previous one. A late rejection of the
+        // previous token must not erase its replacement. This instance may still cache the previous token, if the
+        // replacement was stored through another context, so make it read the replacement next time.
+        guard account.authTokenInKeychain == rejectedToken else {
+            WPLogInfo("Ignoring a rejection of a WordPress.com token that is no longer current")
+            account.discardCachedToken()
+            return
+        }
+
+        account.authToken = nil
 
         NotificationCenter.default.post(
             name: .wpAccountRequiresShowingSigninForWPComFixingAuthToken,
             object: account
         )
-
-        if account?.isDefaultWordPressComAccount == true {
-            // At the time of writing, there is an implicit assumption on what the object parameter value means.
-            // For example, the WordPressAppDelegate.handleDefaultAccountChangedNotification(_:) subscriber inspects the object parameter to decide whether the notification was sent as a result of a login.
-            // If the object is non-nil, then the method considers the source a login.
-            //
-            // The code path in which we are is that of an invalid token, and that's neither a login nor a logout, it's more appropriate to consider it a logout.
-            // That's because if the token is invalid the app will soon received errors from the API and it's therefore better to force the user to login again.
-
-            DispatchQueue.main.async {
-                NotificationCenter.default.post(
-                    name: .wpAccountDefaultWordPressComAccountChanged,
-                    object: nil
-                )
-            }
-        }
     }
 }
 

Modules/Sources/WordPressData/Swift/WPAccount.swift

@@ -104,8 +104,7 @@ public class WPAccount: NSManagedObject {
 
     public override func didTurnIntoFault() {
         super.didTurnIntoFault()
-        _private_wordPressComRestApi = nil
-        cachedToken = nil
+        discardCachedToken()
     }
 
     // MARK: - Auth Token (Keychain)
@@ -120,21 +119,33 @@ public class WPAccount: NSManagedObject {
         if let cachedToken {
             return cachedToken
         }
-        let token = try? Self.token(
+        let token = authTokenInKeychain
+        cachedToken = token
+        return token
+    }
+
+    /// The token stored in the keychain, bypassing `authToken`'s cache.
+    ///
+    /// The cache belongs to this managed object instance. A token written through an instance in another
+    /// context does not invalidate it, so only the keychain tells which token is current.
+    var authTokenInKeychain: String? {
+        try? Self.token(
             forUsername: username,
             serviceName: keychainServiceName,
             migration: keychainMigration,
             keychain: keychain
         )
-        cachedToken = token
-        return token
     }
 
-    private func _setAuthToken(_ authToken: String?) {
+    /// Drops this instance's cached token and REST API, so the next access reads the token from the keychain.
+    func discardCachedToken() {
         cachedToken = nil
+        _private_wordPressComRestApi = nil
+    }
 
+    private func _setAuthToken(_ authToken: String?) {
         // Make sure to release any RestAPI alloc'ed, since it might have an invalid token
-        _private_wordPressComRestApi = nil
+        discardCachedToken()
 
         do {
             try keychain.setPassword(for: username, to: authToken, serviceName: keychainServiceName)

Modules/Sources/WordPressData/Swift/WordPressOrgRestApi+WordPress.swift

@@ -3,8 +3,10 @@ import WordPressShared
 import WordPressKit
 
 private func apiBase(blog: Blog) -> URL? {
+    // A blog with an account is reached through WordPress.com, never through its own REST API. It reaches this
+    // point when the account has no token, an expected state (the app shows the WordPress.com re-authentication
+    // gate), or when the blog has no WordPress.com ID.
     guard blog.account == nil else {
-        assertionFailure(".com support has not been implemented yet")
         return nil
     }
 

Modules/Tests/WordPressDataTests/BlogTokenlessAccountTests.swift

@@ -0,0 +1,59 @@
+import CoreData
+import Testing
+import WordPressKit
+@testable import WordPressData
+
+@MainActor
+struct BlogTokenlessAccountTests {
+    private let contextManager = ContextManager.forTesting()
+
+    private func makeBlog(hostedAtWPCom: Bool, authToken: String?, xmlrpcPassword: String? = nil) -> Blog {
+        let blog = BlogBuilder(contextManager.mainContext)
+            .with(isHostedAtWPCom: hostedAtWPCom)
+            .build()
+        let account = WPAccount.fixture(context: contextManager.mainContext)
+        account.authToken = authToken
+        blog.account = account
+        if let xmlrpcPassword {
+            blog.username = "xmlrpc-user"
+            blog.password = xmlrpcPassword
+        }
+        return blog
+    }
+
+    @Test func dotComBlogWithTokenHasRemote() {
+        let blog = makeBlog(hostedAtWPCom: true, authToken: "token")
+        #expect(!blog.isMissingRequiredWordPressComToken)
+    }
+
+    @Test func dotComBlogWithoutTokenHasNoRemote() {
+        #expect(makeBlog(hostedAtWPCom: true, authToken: nil).isMissingRequiredWordPressComToken)
+        #expect(makeBlog(hostedAtWPCom: true, authToken: "").isMissingRequiredWordPressComToken)
+    }
+
+    @Test func dotComBlogWithoutTokenHasNoRemoteEvenWithXMLRPCPassword() {
+        let blog = makeBlog(hostedAtWPCom: true, authToken: nil, xmlrpcPassword: "password")
+        #expect(blog.isMissingRequiredWordPressComToken)
+    }
+
+    @Test func jetpackBlogWithoutTokenKeepsStoredXMLRPCCredentials() {
+        let blog = makeBlog(hostedAtWPCom: false, authToken: nil, xmlrpcPassword: "password")
+        #expect(!blog.isMissingRequiredWordPressComToken)
+    }
+
+    @Test func jetpackBlogWithoutTokenOrXMLRPCPasswordHasNoRemote() {
+        let blog = makeBlog(hostedAtWPCom: false, authToken: nil)
+        #expect(blog.isMissingRequiredWordPressComToken)
+    }
+
+    @Test func selfHostedBlogIsUnaffected() {
+        let blog = BlogBuilder(contextManager.mainContext, dotComID: nil).build()
+        blog.account = nil
+        #expect(!blog.isMissingRequiredWordPressComToken)
+    }
+
+    @Test func noSelfHostedRestApiForDotComBlogWithoutToken() {
+        let blog = makeBlog(hostedAtWPCom: true, authToken: nil)
+        #expect(WordPressOrgRestApi(blog: blog) == nil)
+    }
+}

Tests/KeystoneTests/Helpers/RemoteSelectionBlogFixture.swift

@@ -0,0 +1,60 @@
+import CoreData
+import Foundation
+@testable import WordPressData
+
+/// Blogs for testing which remote a factory selects, with and without a WordPress.com token.
+///
+/// Keychain access is isolated, so these blogs never read or write the simulator keychain.
+@objc public final class RemoteSelectionBlogFixture: NSObject {
+
+    /// A blog hosted at WordPress.com whose account has the given token.
+    @objc public static func dotComBlog(authToken: String?, context: NSManagedObjectContext) -> Blog {
+        let blog = makeBlog(host: "example.wordpress.com", context: context)
+        blog.isHostedAtWPcom = true
+        blog.dotComID = 1
+        blog.account = makeAccount(authToken: authToken, context: context)
+        return blog
+    }
+
+    /// A Jetpack-connected self-hosted blog whose account has the given token and that may store XML-RPC credentials.
+    @objc public static func jetpackBlog(
+        authToken: String?,
+        xmlrpcPassword: String?,
+        context: NSManagedObjectContext
+    ) -> Blog {
+        let blog = makeBlog(host: "jetpack.example.com", context: context)
+        blog.isHostedAtWPcom = false
+        blog.dotComID = 2
+        blog.account = makeAccount(authToken: authToken, context: context)
+        if let xmlrpcPassword {
+            blog.username = "xmlrpc-\(UUID().uuidString)"
+            blog.password = xmlrpcPassword
+        }
+        return blog
+    }
+
+    /// A self-hosted blog without an account that stores XML-RPC credentials.
+    @objc public static func selfHostedBlog(context: NSManagedObjectContext) -> Blog {
+        let blog = makeBlog(host: "self-hosted.example.com", context: context)
+        blog.username = "xmlrpc-\(UUID().uuidString)"
+        blog.password = "password"
+        return blog
+    }
+
+    private static func makeBlog(host: String, context: NSManagedObjectContext) -> Blog {
+        let blog = Blog(context: context)
+        blog.keychain = TestKeychain()
+        blog.url = "https://\(host)/"
+        blog.xmlrpc = "https://\(host)/xmlrpc.php"
+        return blog
+    }
+
+    private static func makeAccount(authToken: String?, context: NSManagedObjectContext) -> WPAccount {
+        let account = WPAccount(context: context)
+        account.mockKeychain()
+        // A unique username keeps the real keychain, which `WordPressSite(blog:)` falls back to, from matching.
+        account.username = "account-\(UUID().uuidString)"
+        account.authToken = authToken
+        return account
+    }
+}

Tests/KeystoneTests/Tests/Login/WordPressDotComAuthenticatorTests.swift

@@ -180,25 +180,66 @@ class WordPressDotComAuthenticatorTests: CoreDataTestCase {
 
     func testReAuthenticationWithUnexpectedAccount() async throws {
         // Given the app is already signed in with a WP.com account.
-        let account = AccountBuilder(mainContext).with(email: "test@example.com").with(username: "default_account").with(authToken: "token").build()
+        let account = AccountBuilder(mainContext).with(id: 1).with(email: "test@example.com").with(username: "default_account").with(authToken: "token").build()
         try mainContext.save()
         AccountService(coreDataStack: contextManager).setDefaultWordPressComAccount(account)
 
         // When A different account is authenticated via web login
         stubTokenExchange()
         stubGetAccountDetails()
 
-        // Then `mismatchedEmail` error should be returned
+        // Then `mismatchedAccount` error should be returned
         let authenticator = WordPressDotComAuthenticator(coreDataStack: contextManager, authenticator: fakeAuthenticator(callback: ["code": "random"]), redirectURIScheme: "testapp")
         do {
-            let _ = try await authenticator.attemptSignIn(from: UIViewController(), context: .reauthentication(accountEmail: account.email))
+            let _ = try await authenticator.attemptSignIn(from: UIViewController(), context: .reauthentication(userID: account.userID, emailHint: account.email))
             XCTFail("Unexpected successful result")
-        } catch let .mismatchedEmail(expectedEmail) {
+        } catch let .mismatchedAccount(expectedEmail) {
             XCTAssertEqual(expectedEmail, "test@example.com")
         } catch {
             XCTFail("Unexpected error: \(error)")
         }
     }
+
+    @MainActor
+    func testReAuthenticationAfterEmailChangeSucceeds() async throws {
+        // Given the default account's stored email is outdated.
+        let account = AccountBuilder(mainContext).with(id: 55511).with(email: "old@example.com").with(username: "jimthetester").with(authToken: "old-token").build()
+        try mainContext.save()
+        AccountService(coreDataStack: contextManager).setDefaultWordPressComAccount(account)
+
+        stubTokenExchange()
+        stubGetAccountDetails()
+        stubGetSites()
+
+        // When the same user signs in again.
+        let authenticator = WordPressDotComAuthenticator(coreDataStack: contextManager, authenticator: fakeAuthenticator(callback: ["code": "random"]), redirectURIScheme: "testapp")
+        let accountID = try await authenticator.attemptSignIn(from: UIViewController(), context: .reauthentication(userID: account.userID, emailHint: account.email))
+
+        // Then the stored account is updated.
+        XCTAssertEqual(accountID.objectID, account.objectID)
+        XCTAssertEqual(account.email, "jim@wptestaccounts.com")
+        XCTAssertEqual(try WPAccount.token(forUsername: "jimthetester"), "token")
+    }
+
+    func testSignInWithAnotherUserIDIsRejectedEvenWithTheSameEmail() async throws {
+        // Given the app is already signed in with a WP.com account that has the same email as the stubbed user.
+        let account = AccountBuilder(mainContext).with(id: 1).with(email: "jim@wptestaccounts.com").with(username: "default_account").with(authToken: "token").build()
+        try mainContext.save()
+        AccountService(coreDataStack: contextManager).setDefaultWordPressComAccount(account)
+
+        stubTokenExchange()
+        stubGetAccountDetails()
+
+        let authenticator = WordPressDotComAuthenticator(coreDataStack: contextManager, authenticator: fakeAuthenticator(callback: ["code": "random"]), redirectURIScheme: "testapp")
+        do {
+            let _ = try await authenticator.attemptSignIn(from: UIViewController(), context: .jetpackSite(accountEmail: nil))
+            XCTFail("Unexpected successful result")
+        } catch .alreadySignedIn {
+            // Do nothing
+        } catch {
+            XCTFail("Unexpected error: \(error)")
+        }
+    }
 }
 
 private extension WordPressDotComAuthenticatorTests {

Tests/KeystoneTests/Tests/Models/WPAccount+Fixture.swift

@@ -25,9 +25,10 @@ extension WPAccount {
         return account
     }
 
+    /// Pass the same `keychain` to instances in different contexts to make them share it, like the real keychain.
     @discardableResult
-    func mockKeychain() -> WPAccount {
-        keychain = TestKeychain()
+    func mockKeychain(_ keychain: TestKeychain = TestKeychain()) -> WPAccount {
+        self.keychain = keychain
         keychainServiceName = "test-service"
         keychainMigration = MockAuthKeyMigration()
         return self