Mirrored_Repos/yattee
1
0
Fork 0
mirror of https://github.com/yattee/yattee.git synced 2026-10-23 13:12:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Close the two ways an adopted secret could still be uploaded

Browse Source 
Review of the silent migration found two paths that defeat the point of
writing the adopted copy device-local.

The marker is written after the copy it describes, so a launch that ends
in between leaves an adopted credential nothing claims — and adoption
never retries, because the key is no longer empty. Enabling iCloud sync
then rewrites it as synchronizable. The resync now also recognizes the
value: a scoped credential identical to the unscoped item it could only
have come from is an adopted copy whether or not a marker says so.

The opposite failure is a marker that outlives what it described.
Signing in again replaces the credential with one the user proved they
hold, but the marker stayed, so enabling sync afterwards skipped the
account and left every other device showing a login it never receives.
Storing a credential now retires the marker.

Also corrects four comments that still describe adoption as something
the user is asked about, and states plainly what the legacy-deletion
conditions do and do not prove now that no one is asked.
This commit is contained in:
Arkadiusz Fal
2026-08-31 23:59:37 +02:00
parent 53adfd2d66
commit 4a1378f8d6
6 changed files with 136 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
Yattee/Core/InstancesManager.swift
View File

@@ -98,8 +98,8 @@ final class InstancesManager {
// Instances arriving from iCloud are as much a change as a local edit.
// Without this, a legacy authenticated instance delivered here never
// reaches account migration — the account, and the action to adopt its
// saved login, would appear only after an unrelated edit or a relaunch.
// reaches account migration — the account, and the adoption of its
// saved login, would happen only after an unrelated edit or a relaunch.
NotificationCenter.default.post(name: .instancesDidChange, object: nil)
}

28
Yattee/Services/Credentials/AccountKeychainCredentials.swift
View File

@@ -92,6 +92,14 @@ final class AccountKeychainCredentials {
loginsByScope[scope, default: []].insert(account.id)
// Whatever was adopted here is gone, replaced by a login the user just
// proved they hold. Leaving the marker would keep this account out of
// the resync that follows the iCloud toggle, so the new credential
// would stay on this device and every other one would show an account
// it never receives a login for. It would also let a deletion retire an
// unscoped item this account no longer has anything to do with.
markers.removeMarker(service: store.service, instanceID: account.instanceID, scope: scope)
LoggingService.shared.info(
"Stored \(credentialNoun)",
category: .keychain,
@@ -131,10 +139,22 @@ final class AccountKeychainCredentials {
let adopted = markers.adoptedAccountIDs(service: store.service, scope: scope)
for accountID in loginsByScope[scope] ?? [] where !adopted.contains(accountID) {
rewrite(
key: KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID),
synchronizable: syncEnabled
)
let credentialKey = KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID)
// The marker is written *after* the copy it describes, so a launch
// that ends in between leaves an adopted credential no marker will
// ever claim — and adoption will not retry, because the key is no
// longer empty. Recognising the value itself closes that gap: a
// scoped credential identical to the unscoped item it could only
// have come from is an adopted copy, marker or not. Migrated
// accounts reuse their instance's UUID, which is what the legacy
// key is made of, and they are the only accounts allowed to adopt.
if let stored = store.value(forKey: credentialKey),
stored == store.value(forKey: KeychainCredentialStore.legacyKey(instanceID: accountID)) {
continue
}
rewrite(key: credentialKey, synchronizable: syncEnabled)
rewrite(
key: KeychainCredentialStore.usernameKey(scope: scope, accountID: accountID),
synchronizable: syncEnabled

11
Yattee/Services/Credentials/CredentialAdoptionMarker.swift
View File

@@ -60,8 +60,10 @@ final class CredentialAdoptionMarkerStore {
/// Whether this identity has already adopted the legacy item for an instance.
///
/// Once true, the app stops offering adoption and stops reading the legacy
/// key entirely for that account.
/// Once true, the app stops adopting and stops reading the legacy key
/// entirely for that account. Cleared when the user signs in again: the
/// credential is then one they proved they hold, and nothing about the
/// adopted copy applies to it any more.
func hasAdopted(service: String, instanceID: UUID, scope: String) -> Bool {
marker(service: service, instanceID: instanceID, scope: scope) != nil
}
@@ -121,6 +123,11 @@ final class CredentialAdoptionMarkerStore {
///
/// 1. This device has a marker for the instance — no marker means this
/// device never adopted anything, so the legacy item is not ours to touch.
/// Since adoption became automatic this proves only that the device took
/// the secret, not that the identity signed in now originally owned it.
/// Deleting anyway is deliberate: the user is removing this login from
/// this device, which is exactly what the pre-revamp release did to the
/// same item on logout. Condition 4 is what carries the real weight now.
/// 2. The marker belongs to the current identity — Apple account A's marker
/// cannot authorize a deletion while B is signed in.
/// 3. The marker belongs to the account being deleted. An instance can hold

6
Yattee/Services/Credentials/CredentialIdentityScope.swift
View File

@@ -24,8 +24,10 @@ import Foundation
/// keeps `kSecAttrSynchronizable = true` and still matches a strict
/// `== true` query with **no** Apple account signed in — so the attribute
/// cannot be used to tell "belongs to the current account" from "left over
/// from the previous one". Scoping the key is the only reliable defence, and
/// legacy items are never adopted automatically (see
/// from the previous one". Scoping the key is the only reliable defence.
/// A pre-revamp item is adopted into a scoped key automatically, but only as a
/// device-local copy, so an unproven secret never reaches the current
/// identity's iCloud Keychain (see
/// ``AccountKeychainCredentials/adoptLegacyCredential(for:)``).
///
/// ### Why the CloudKit user record, not the ubiquity token

11
Yattee/Services/Credentials/KeychainCredentialStore.swift
View File

@@ -39,8 +39,9 @@ struct KeychainCredentialStore: Sendable {
/// The pre-revamp key: a bare instance UUID, with no identity scope.
///
/// Only the adoption path may read this, and only when the user explicitly
/// asks for it.
/// Only the adoption path may read this. Ordinary credential reads never
/// fall back to it — an implicit read-through would leave no record of what
/// was taken, and nothing to bound a later deletion with.
static func legacyKey(instanceID: UUID) -> String {
instanceID.uuidString
}
@@ -75,9 +76,9 @@ struct KeychainCredentialStore: Sendable {
/// Writes a value **only if the key is currently empty**, returning
/// `errSecDuplicateItem` otherwise.
///
/// Used by legacy adoption: between rendering the offer and the user
/// tapping it, iCloud Keychain may deliver the real scoped credential, and
/// a replacing write would silently overwrite it with the older secret.
/// Used by legacy adoption: iCloud Keychain may deliver the real scoped
/// credential while the migration is deciding to copy the older one, and a
/// replacing write would silently overwrite the newer secret.
@discardableResult
func addIfAbsent(_ value: String, forKey key: String, synchronizable: Bool) -> OSStatus {
let query: [String: Any] = [

91
YatteeTests/AccountCredentialsTests.swift
View File

@@ -281,6 +281,97 @@ struct LegacyCredentialAdoptionTests {
store.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
}
@Test("An adopted credential stays local even if its marker was never written")
func resyncRecognizesAnAdoptedCopyWithoutItsMarker() {
let service = makeService()
let instanceID = UUID()
let account = Account.migrated(from: instanceID)
seedLegacyItem(service: service, instanceID: instanceID, value: "legacy-secret")
let settings = SettingsManager()
let previousSync = settings.iCloudSyncEnabled
let previousInstances = settings.syncInstances
settings.iCloudSyncEnabled = false
settings.syncInstances = false
defer {
settings.iCloudSyncEnabled = previousSync
settings.syncInstances = previousInstances
}
let store = KeychainCredentialStore(service: service)
let key = KeychainCredentialStore.credentialKey(scope: "identity-A", accountID: account.id)
// Exactly the state left behind when the app dies between the adoption
// copy and the marker that describes it: the scoped item exists, no
// marker claims it, and adoption will not retry because the key is
// occupied.
store.set("legacy-secret", forKey: key, synchronizable: false)
let credentials = makeCredentials(
service: service,
scope: "identity-A",
markers: makeMarkers(),
settingsManager: settings
)
#expect(credentials.isLoggedIn(for: account) == true)
settings.iCloudSyncEnabled = true
settings.syncInstances = true
credentials.resyncCredentialsWithCurrentSetting()
#expect(store.isSynchronizable(key: key) == false)
credentials.deleteCredential(for: account)
store.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
}
@Test("Signing in again lets the new credential sync")
func signingInAgainClearsTheAdoptionMarker() {
let service = makeService()
let instanceID = UUID()
let account = Account.migrated(from: instanceID)
seedLegacyItem(service: service, instanceID: instanceID, value: "legacy-secret")
let settings = SettingsManager()
let previousSync = settings.iCloudSyncEnabled
let previousInstances = settings.syncInstances
settings.iCloudSyncEnabled = false
settings.syncInstances = false
defer {
settings.iCloudSyncEnabled = previousSync
settings.syncInstances = previousInstances
}
let markers = makeMarkers()
let credentials = makeCredentials(
service: service,
scope: "identity-A",
markers: markers,
settingsManager: settings
)
#expect(credentials.adoptLegacyCredential(for: account) == true)
// The user signs in again while sync is off, so the replacement is
// stored locally like any other credential.
credentials.setCredential("freshly-typed-secret", for: account)
#expect(markers.hasAdopted(service: service, instanceID: instanceID, scope: "identity-A") == false)
// Turning sync on must now carry it to the other devices — they are
// showing an account whose credential never arrives.
settings.iCloudSyncEnabled = true
settings.syncInstances = true
credentials.resyncCredentialsWithCurrentSetting()
let store = KeychainCredentialStore(service: service)
#expect(
store.isSynchronizable(
key: KeychainCredentialStore.credentialKey(scope: "identity-A", accountID: account.id)
) == true
)
credentials.deleteCredential(for: account)
store.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
}
@Test("A login the user added themselves never inherits the legacy item")
func onlyTheMigratedAccountAdopts() {
let service = makeService()