From a96e9d2a9cdd93e8563406f44cf4204343319bef Mon Sep 17 00:00:00 2001
From: Fayner Brack <github@fagnermartins.com>
Date: Sat, 4 Jul 2026 23:04:40 +1000
Subject: [PATCH] test(ios-readplace): backfill untested logic seams and
 ratchet coverage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cover error and edge paths that had no tests, and raise their coverage floors:

- OAuthService (90%→98%): a malformed 200 body and a 200 missing refresh_token
  both surface .malformedResponse; refresh without a stored token throws
  .noRefreshToken and makes no request; revoke without a token skips the network
  but still clears.
- AppSession completeSignIn (79%→83%): a callback carrying error= is denied, a
  callback with no code is .missingCode, and a failed code exchange surfaces as a
  failure — none exchanging or logging in.
- ReadingListViewModel (94%→95%): a 401 whose refresh also fails routes to the
  logout callback without an error banner; a collection warning populates
  warningText.
- KeychainTokenStorage (57%→98%): a real simulator-keychain round trip through
  the production path (add / read / update / remove, key isolation) — the test
  double never exercised it.
- TokenStore (85%→99%): extract the embedded-provisioning-profile parsing into a
  pure `parseAppGroupId(fromProvisioningProfile:)` and cover it with fixtures
  (valid profile, no plist, no app-group entitlement, empty group array); the
  `Bundle.main` read stays the only untested line.
---
 .../ios-readplace/Shared/TokenStore.swift     | 19 +++++--
 .../Tests/KeychainTokenStorageTests.swift     | 45 +++++++++++++++++
 .../ios-readplace/Tests/LoginFlowTests.swift  | 47 ++++++++++++++++++
 .../Tests/OAuthServiceTests.swift             | 42 ++++++++++++++++
 .../Tests/ReadingListViewModelTests.swift     | 40 +++++++++++++++
 .../ios-readplace/Tests/TokenStoreTests.swift | 49 +++++++++++++++++++
 .../scripts/coverage-baseline.json            | 12 ++---
 7 files changed, 245 insertions(+), 9 deletions(-)
 create mode 100644 projects/ios-readplace/Tests/KeychainTokenStorageTests.swift

diff --git a/projects/ios-readplace/Shared/TokenStore.swift b/projects/ios-readplace/Shared/TokenStore.swift
index ab7ef6c9d..d9c1d560e 100644
--- a/projects/ios-readplace/Shared/TokenStore.swift
+++ b/projects/ios-readplace/Shared/TokenStore.swift
@@ -61,10 +61,23 @@ struct TokenStore {
 		guard
 			let url = Bundle.main.url(forResource: "embedded", withExtension: "mobileprovision"),
 			let data = try? Data(contentsOf: url),
+			let group = parseAppGroupId(fromProvisioningProfile: data)
+		else { return AppConfig.appGroupId }
+		return group
+	}()
+
+	/// Extracts the first application-group entitlement from an embedded
+	/// `.mobileprovision` blob, or nil when it can't be read. The profile is a
+	/// CMS-signed container with a plist inside; the signature isn't verified here
+	/// (the OS already did at install), so this just scans out the `<?xml…</plist>`
+	/// slice and reads the entitlement. Pulled out of `resolvedAppGroupId` so the
+	/// parsing is unit-testable without a real `Bundle.main`.
+	static func parseAppGroupId(fromProvisioningProfile data: Data) -> String? {
+		guard
 			let raw = String(data: data, encoding: .isoLatin1),
 			let start = raw.range(of: "<?xml"),
 			let end = raw.range(of: "</plist>")
-		else { return AppConfig.appGroupId }
+		else { return nil }
 		let plistString = String(raw[start.lowerBound..<end.upperBound])
 		guard
 			let plistData = plistString.data(using: .isoLatin1),
@@ -72,9 +85,9 @@ struct TokenStore {
 			let entitlements = plist["Entitlements"] as? [String: Any],
 			let groups = entitlements["com.apple.security.application-groups"] as? [String],
 			let group = groups.first
-		else { return AppConfig.appGroupId }
+		else { return nil }
 		return group
-	}()
+	}
 
 	var tokens: OAuthTokens? {
 		guard
diff --git a/projects/ios-readplace/Tests/KeychainTokenStorageTests.swift b/projects/ios-readplace/Tests/KeychainTokenStorageTests.swift
new file mode 100644
index 000000000..0348a138f
--- /dev/null
+++ b/projects/ios-readplace/Tests/KeychainTokenStorageTests.swift
@@ -0,0 +1,45 @@
+import XCTest
+@testable import Readplace
+
+/// Exercises the real Keychain-backed storage against the simulator keychain. The
+/// test host app carries the `group.com.readplace` app-group entitlement (which
+/// doubles as the keychain access group), so generic-password items in that group
+/// round-trip here — the path production uses, not the `UserDefaults` test double.
+final class KeychainTokenStorageTests: XCTestCase {
+	private let keychain = KeychainTokenStorage(accessGroup: AppConfig.appGroupId)
+
+	override func setUp() {
+		super.setUp()
+		for key in TokenKey.allCases { keychain.removeValue(for: key) }
+	}
+
+	override func tearDown() {
+		for key in TokenKey.allCases { keychain.removeValue(for: key) }
+		super.tearDown()
+	}
+
+	func testAddsReadsUpdatesAndRemovesAToken() {
+		XCTAssertNil(keychain.value(for: .accessToken), "starts empty")
+
+		keychain.setValue("first", for: .accessToken) // insert path (SecItemAdd)
+		XCTAssertEqual(keychain.value(for: .accessToken), "first")
+
+		keychain.setValue("second", for: .accessToken) // update path (SecItemUpdate)
+		XCTAssertEqual(keychain.value(for: .accessToken), "second")
+
+		keychain.removeValue(for: .accessToken)
+		XCTAssertNil(keychain.value(for: .accessToken))
+	}
+
+	func testKeysAreStoredIndependently() {
+		keychain.setValue("acc", for: .accessToken)
+		keychain.setValue("ref", for: .refreshToken)
+
+		XCTAssertEqual(keychain.value(for: .accessToken), "acc")
+		XCTAssertEqual(keychain.value(for: .refreshToken), "ref")
+
+		keychain.removeValue(for: .accessToken)
+		XCTAssertNil(keychain.value(for: .accessToken))
+		XCTAssertEqual(keychain.value(for: .refreshToken), "ref", "removing one key leaves the other")
+	}
+}
diff --git a/projects/ios-readplace/Tests/LoginFlowTests.swift b/projects/ios-readplace/Tests/LoginFlowTests.swift
index d79248417..a01c04d5a 100644
--- a/projects/ios-readplace/Tests/LoginFlowTests.swift
+++ b/projects/ios-readplace/Tests/LoginFlowTests.swift
@@ -161,4 +161,51 @@ final class LoginFlowTests: XCTestCase {
 		XCTAssertEqual(queueRequest.value(forHTTPHeaderField: "Authorization"), "Bearer access-1")
 		XCTAssertEqual(queueRequest.value(forHTTPHeaderField: "Accept"), "application/vnd.siren+json")
 	}
+
+	func testCallbackCarryingAnErrorParamIsDeniedWithoutExchanging() async {
+		let store = TokenStore(defaults: TestSupport.ephemeralDefaults())
+		let session = AppSession(store: store, sessionConfiguration: TestSupport.stubbedConfiguration())
+
+		let result = await session.completeSignIn(
+			callbackURL: URL(string: "\(AppConfig.nativeCallbackURL)?error=access_denied&state=S")!,
+			verifier: "v", expectedState: "S", redirectURI: AppConfig.nativeCallbackURL
+		)
+
+		guard case .failure(let error) = result else { return XCTFail("expected .failure, got \(result)") }
+		XCTAssertEqual((error as? AuthFlowError)?.errorDescription, AuthFlowError.denied("access_denied").errorDescription)
+		XCTAssertFalse(session.isLoggedIn)
+		XCTAssertTrue(StubURLProtocol.records(path: "/oauth/token").isEmpty, "a denied authorization exchanges no code")
+	}
+
+	func testCallbackWithoutACodeIsMissingCodeWithoutExchanging() async {
+		let store = TokenStore(defaults: TestSupport.ephemeralDefaults())
+		let session = AppSession(store: store, sessionConfiguration: TestSupport.stubbedConfiguration())
+
+		let result = await session.completeSignIn(
+			callbackURL: URL(string: "\(AppConfig.nativeCallbackURL)?state=S")!,
+			verifier: "v", expectedState: "S", redirectURI: AppConfig.nativeCallbackURL
+		)
+
+		guard case .failure(let error) = result else { return XCTFail("expected .failure, got \(result)") }
+		XCTAssertEqual((error as? AuthFlowError)?.errorDescription, AuthFlowError.missingCode.errorDescription)
+		XCTAssertTrue(StubURLProtocol.records(path: "/oauth/token").isEmpty)
+	}
+
+	func testExchangeFailureDuringSignInSurfacesAsFailureAndStaysLoggedOut() async {
+		let store = TokenStore(defaults: TestSupport.ephemeralDefaults())
+		let session = AppSession(store: store, sessionConfiguration: TestSupport.stubbedConfiguration())
+		StubURLProtocol.setHandler { _, _ in .json(400, "{\"error\":\"invalid_grant\"}") }
+
+		let result = await session.completeSignIn(
+			callbackURL: URL(string: "\(AppConfig.nativeCallbackURL)?code=abc&state=S")!,
+			verifier: "v", expectedState: "S", redirectURI: AppConfig.nativeCallbackURL
+		)
+
+		guard case .failure(let error) = result else { return XCTFail("expected .failure, got \(result)") }
+		XCTAssertEqual(
+			(error as? OAuthError)?.errorDescription,
+			OAuthError.tokenExchangeFailed(status: 400).errorDescription
+		)
+		XCTAssertFalse(session.isLoggedIn)
+	}
 }
diff --git a/projects/ios-readplace/Tests/OAuthServiceTests.swift b/projects/ios-readplace/Tests/OAuthServiceTests.swift
index 7ee845431..b27cc0a9c 100644
--- a/projects/ios-readplace/Tests/OAuthServiceTests.swift
+++ b/projects/ios-readplace/Tests/OAuthServiceTests.swift
@@ -91,4 +91,46 @@ final class OAuthServiceTests: XCTestCase {
 		let json = TestSupport.jsonObject(record.body)
 		XCTAssertEqual(json["token"] as? String, "r1")
 	}
+
+	func testExchangeCodeWithAMalformedBodyThrowsMalformedResponse() async {
+		let store = TestSupport.loggedInStore()
+		StubURLProtocol.setHandler { _, _ in .json(200, "not json at all") }
+		do {
+			try await makeService(store: store).exchangeCode("c", verifier: "v", redirectURI: AppConfig.nativeCallbackURL)
+			XCTFail("expected .malformedResponse")
+		} catch {
+			XCTAssertEqual((error as? OAuthError)?.errorDescription, OAuthError.malformedResponse.errorDescription)
+		}
+	}
+
+	func testExchangeCodeWithoutARefreshTokenThrowsMalformedResponse() async {
+		let store = TestSupport.loggedInStore()
+		// A 200 that carries an access token but no refresh_token, and no fallback
+		// (exchange has none), is malformed — the session can't be refreshed later.
+		StubURLProtocol.setHandler { _, _ in .json(200, Fixtures.tokenResponse(access: "a", refresh: nil)) }
+		do {
+			try await makeService(store: store).exchangeCode("c", verifier: "v", redirectURI: AppConfig.nativeCallbackURL)
+			XCTFail("expected .malformedResponse")
+		} catch {
+			XCTAssertEqual((error as? OAuthError)?.errorDescription, OAuthError.malformedResponse.errorDescription)
+		}
+	}
+
+	func testRefreshWithoutAStoredRefreshTokenThrowsAndMakesNoRequest() async {
+		let store = TokenStore(defaults: TestSupport.ephemeralDefaults())
+		do {
+			_ = try await makeService(store: store).refresh()
+			XCTFail("expected .noRefreshToken")
+		} catch {
+			XCTAssertEqual((error as? OAuthError)?.errorDescription, OAuthError.noRefreshToken.errorDescription)
+		}
+		XCTAssertTrue(StubURLProtocol.records(path: "/oauth/token").isEmpty, "no token request without a refresh token")
+	}
+
+	func testRevokeWithoutAStoredTokenSkipsTheNetworkButStillClears() async {
+		let store = TokenStore(defaults: TestSupport.ephemeralDefaults())
+		await makeService(store: store).revoke()
+		XCTAssertNil(store.tokens)
+		XCTAssertTrue(StubURLProtocol.records(path: "/oauth/revoke").isEmpty, "nothing to revoke without a token")
+	}
 }
diff --git a/projects/ios-readplace/Tests/ReadingListViewModelTests.swift b/projects/ios-readplace/Tests/ReadingListViewModelTests.swift
index 43d086222..84df31442 100644
--- a/projects/ios-readplace/Tests/ReadingListViewModelTests.swift
+++ b/projects/ios-readplace/Tests/ReadingListViewModelTests.swift
@@ -858,4 +858,44 @@ final class ReadingListViewModelTests: XCTestCase {
 		XCTAssertNil(cookies, "a failed bootstrap mints no session, so the sheet shows its unavailable view")
 		XCTAssertNotNil(viewModel.errorText)
 	}
+
+	// MARK: - Session expiry & warnings
+
+	func testUnauthorizedLoadLogsOutWithoutAnErrorBanner() async {
+		let api = ReadplaceAPI(
+			baseURL: AppConfig.serverBaseURL,
+			store: TestSupport.loggedInStore(),
+			sessionConfiguration: TestSupport.stubbedConfiguration()
+		)
+		var expired = false
+		let viewModel = ReadingListViewModel(api: api, onSessionExpired: { expired = true })
+		// 401 everywhere: the entry-point load 401s, the single refresh 401s, and
+		// the load surfaces .unauthorized.
+		StubURLProtocol.setHandler { _, _ in .json(401, "{}") }
+
+		await viewModel.refresh()
+
+		XCTAssertTrue(expired, "a 401 whose refresh also fails logs the user out")
+		XCTAssertNil(viewModel.errorText, "a session-expiry logout is not shown as an error banner")
+	}
+
+	func testCollectionWarningPopulatesWarningText() async {
+		let warnedQueue = """
+		{
+		  "class": ["collection", "articles"],
+		  "properties": { "total": 1, "page": 1, "pageSize": 20, "warning": { "code": "not-saveable", "message": "Cannot save that link." } },
+		  "entities": [\(Fixtures.article(id: "a1"))],
+		  "links": [{ "rel": ["self"], "href": "/queue" }, { "rel": ["root"], "href": "/queue" }],
+		  "actions": []
+		}
+		"""
+		StubURLProtocol.setHandler { request, _ in
+			request.url?.path == "/" ? .redirect(to: "/queue") : .json(200, warnedQueue)
+		}
+		let viewModel = makeViewModel(store: TestSupport.loggedInStore())
+
+		await viewModel.refresh()
+
+		XCTAssertEqual(viewModel.warningText, "Cannot save that link.")
+	}
 }
diff --git a/projects/ios-readplace/Tests/TokenStoreTests.swift b/projects/ios-readplace/Tests/TokenStoreTests.swift
index c84fff1b4..8ceb4202e 100644
--- a/projects/ios-readplace/Tests/TokenStoreTests.swift
+++ b/projects/ios-readplace/Tests/TokenStoreTests.swift
@@ -86,4 +86,53 @@ final class TokenStoreTests: XCTestCase {
 		XCTAssertNil(target.value(for: .accessToken), "a partial legacy token is not a valid session")
 		XCTAssertNil(legacy.string(forKey: TokenKey.accessToken.rawValue), "the stray partial token is cleared")
 	}
+
+	// MARK: - parseAppGroupId (embedded provisioning profile)
+
+	private func profile(_ innerPlist: String) -> Data {
+		// A .mobileprovision is a CMS blob with a plist inside; the parser only
+		// scans out the <?xml…</plist> slice, so wrapping it in arbitrary bytes
+		// models the real container without needing a signed profile.
+		Data("....signature-bytes....\(innerPlist)....trailer....".utf8)
+	}
+
+	func testParsesTheFirstApplicationGroupFromAProfile() {
+		let plist = """
+		<?xml version="1.0" encoding="UTF-8"?>
+		<plist version="1.0"><dict>
+		<key>Entitlements</key><dict>
+		<key>com.apple.security.application-groups</key>
+		<array><string>group.com.rewritten.readplace</string><string>group.other</string></array>
+		</dict></dict></plist>
+		"""
+		XCTAssertEqual(
+			TokenStore.parseAppGroupId(fromProvisioningProfile: profile(plist)),
+			"group.com.rewritten.readplace"
+		)
+	}
+
+	func testReturnsNilWhenTheProfileHasNoPlist() {
+		XCTAssertNil(TokenStore.parseAppGroupId(fromProvisioningProfile: Data("no plist here".utf8)))
+	}
+
+	func testReturnsNilWhenTheProfileHasNoAppGroupEntitlement() {
+		let plist = """
+		<?xml version="1.0" encoding="UTF-8"?>
+		<plist version="1.0"><dict>
+		<key>Entitlements</key><dict><key>application-identifier</key><string>ABCDE.com.x</string></dict>
+		</dict></plist>
+		"""
+		XCTAssertNil(TokenStore.parseAppGroupId(fromProvisioningProfile: profile(plist)))
+	}
+
+	func testReturnsNilWhenTheAppGroupArrayIsEmpty() {
+		let plist = """
+		<?xml version="1.0" encoding="UTF-8"?>
+		<plist version="1.0"><dict>
+		<key>Entitlements</key><dict>
+		<key>com.apple.security.application-groups</key><array></array>
+		</dict></dict></plist>
+		"""
+		XCTAssertNil(TokenStore.parseAppGroupId(fromProvisioningProfile: profile(plist)))
+	}
 }
diff --git a/projects/ios-readplace/scripts/coverage-baseline.json b/projects/ios-readplace/scripts/coverage-baseline.json
index 6529d41fe..6323ef461 100644
--- a/projects/ios-readplace/scripts/coverage-baseline.json
+++ b/projects/ios-readplace/scripts/coverage-baseline.json
@@ -24,14 +24,14 @@
     "PKCE.swift": 95,
     "ReadplaceAPI.swift": 94,
     "URLDetection.swift": 94,
-    "ReadingListViewModel.swift": 93,
+    "ReadingListViewModel.swift": 94,
     "SaveSharedPage.swift": 93,
     "LoginView.swift": 91,
     "AffordancePresentation.swift": 91,
-    "OAuthService.swift": 89,
-    "TokenStore.swift": 85,
-    "AppSession.swift": 79,
-    "ReadplaceApp.swift": 64,
-    "KeychainTokenStorage.swift": 57
+    "TokenStore.swift": 98,
+    "OAuthService.swift": 97,
+    "KeychainTokenStorage.swift": 97,
+    "AppSession.swift": 82,
+    "ReadplaceApp.swift": 64
   }
 }