mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 13:12:53 +00:00
Migrate a saved login without asking about it
The pre-revamp release reads its unscoped Keychain item with kSecAttrSynchronizableAny and no identity in the key, so a credential retained across an Apple Account change is already served to whoever holds the device. Asking before reusing it prevents nothing that is not already happening, and it cost every upgrading user a trip into the instance editor to find a button they had no reason to look for. What silent adoption could genuinely add is re-filing that secret as synchronizable under the identity signed in now, which uploads a login of unproven provenance into that person's iCloud Keychain and spreads it to their devices. So the copy is written device-local instead, and the migration runs where the status is computed. The real owner loses nothing: the item it was copied from syncs, so each of their devices finds it and adopts its own copy, and adoption still never deletes it. Two things keep that closed. Turning iCloud sync on rewrites stored credentials to follow the setting, which would undo the whole point, so adopted ones are left out of it. And only the account the instance's legacy login became may inherit the secret — the old item is keyed by instance, so a second login the user added themselves would otherwise match it.
This commit is contained in:
@@ -897,6 +897,17 @@ final class AppEnvironment {
|
||||
|
||||
credentials.refreshLoginStatus(for: account)
|
||||
|
||||
// An account migrated from a pre-revamp login has its credential
|
||||
// under the old unscoped key. Move it across before concluding
|
||||
// anything: the upgrade is not something to ask the user about —
|
||||
// the previous release already served that item to whoever held the
|
||||
// device, and the copy made here stays on it.
|
||||
if !credentials.isLoggedIn(for: account),
|
||||
credentials.hasAdoptableLegacyCredential(for: account),
|
||||
credentials.adoptLegacyCredential(for: account) {
|
||||
credentials.refreshLoginStatus(for: account)
|
||||
}
|
||||
|
||||
guard credentials.isLoggedIn(for: account) else {
|
||||
accountsManager.setStatus(.credentialMissing, for: account.id)
|
||||
continue
|
||||
|
||||
@@ -263,36 +263,6 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"accounts.useSavedLogin" : {
|
||||
"localizations" : {
|
||||
"en" : {
|
||||
"stringUnit" : {
|
||||
"state" : "translated",
|
||||
"value" : "Use Saved Login From This Device"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"accounts.useSavedLogin.failed" : {
|
||||
"localizations" : {
|
||||
"en" : {
|
||||
"stringUnit" : {
|
||||
"state" : "translated",
|
||||
"value" : "Could not use the saved login. Sign in again."
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"accounts.useSavedLogin.footer" : {
|
||||
"localizations" : {
|
||||
"en" : {
|
||||
"stringUnit" : {
|
||||
"state" : "translated",
|
||||
"value" : "A login saved by an earlier version of Yattee is stored on this device. It cannot be confirmed to belong to the Apple Account currently signed in, so it is only used if you choose to."
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"alert.ambiguousLink.message %@" : {
|
||||
"localizations" : {
|
||||
"en" : {
|
||||
|
||||
@@ -60,14 +60,22 @@ protocol AccountCredentialsManager: AnyObject, Observable {
|
||||
/// Whether a pre-revamp credential for this account's instance exists on
|
||||
/// this device and has not been adopted by the current identity.
|
||||
///
|
||||
/// Never acted on automatically: a retained Keychain item carries no proof
|
||||
/// of which Apple account it belonged to (measured — see
|
||||
/// ``CredentialIdentityScope``), so adoption is always a deliberate user
|
||||
/// action.
|
||||
/// True only for the account the instance's legacy login was migrated into
|
||||
/// — the old item is keyed by instance, and a second login the user added
|
||||
/// themselves must never inherit it.
|
||||
func hasAdoptableLegacyCredential(for account: Account) -> Bool
|
||||
|
||||
/// Copies the legacy item into this account's scoped key after the user
|
||||
/// asks for it. Returns whether the copy verified.
|
||||
/// Copies the legacy item into this account's scoped key. Returns whether
|
||||
/// the copy verified.
|
||||
///
|
||||
/// Runs by itself as part of recomputing account status: the upgrade is not
|
||||
/// a decision to put to the user. A retained Keychain item carries no proof
|
||||
/// of which Apple account it belonged to (measured — see
|
||||
/// ``CredentialIdentityScope``), but the pre-revamp release read that item
|
||||
/// with `kSecAttrSynchronizableAny` and no identity in its key, so it was
|
||||
/// already being served to whoever held the device. The copy is written
|
||||
/// device-local so this migration cannot put a secret of unproven
|
||||
/// provenance into the current identity's iCloud Keychain.
|
||||
@discardableResult
|
||||
func adoptLegacyCredential(for account: Account) -> Bool
|
||||
|
||||
|
||||
@@ -122,8 +122,15 @@ final class AccountKeychainCredentials {
|
||||
guard let scope = identityScope.current else { return }
|
||||
|
||||
let syncEnabled = shouldSyncToiCloud
|
||||
// An adopted legacy secret is deliberately kept on this device (see
|
||||
// `adoptLegacyCredential`). Rewriting it here would undo that: the
|
||||
// toggle would push a credential of unknown provenance into whichever
|
||||
// iCloud Keychain is current, which is the one thing silent adoption is
|
||||
// careful not to do. Nothing is lost — the legacy item it was copied
|
||||
// from already syncs, so the user's other devices adopt their own copy.
|
||||
let adopted = markers.adoptedAccountIDs(service: store.service, scope: scope)
|
||||
|
||||
for accountID in loginsByScope[scope] ?? [] {
|
||||
for accountID in loginsByScope[scope] ?? [] where !adopted.contains(accountID) {
|
||||
rewrite(
|
||||
key: KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID),
|
||||
synchronizable: syncEnabled
|
||||
@@ -222,6 +229,12 @@ final class AccountKeychainCredentials {
|
||||
|
||||
func hasAdoptableLegacyCredential(for account: Account) -> Bool {
|
||||
guard let scope = currentScope() else { return false }
|
||||
// Only the account this instance's pre-revamp login became may inherit
|
||||
// its secret. The legacy item is keyed by instance, so any
|
||||
// credential-less account on the instance would otherwise match — and a
|
||||
// second login the user added themselves would silently come up holding
|
||||
// somebody else's session.
|
||||
guard account.isMigrated else { return false }
|
||||
guard !markers.hasAdopted(service: store.service, instanceID: account.instanceID, scope: scope) else { return false }
|
||||
guard credential(for: account) == nil else { return false }
|
||||
|
||||
@@ -235,9 +248,21 @@ final class AccountKeychainCredentials {
|
||||
/// here would log the user out of devices still running the previous
|
||||
/// release, mid-rollout. It is retained until the account is really
|
||||
/// deleted; general cleanup waits for a compatibility-breaking release.
|
||||
///
|
||||
/// The copy is written **device-local**, whatever the sync setting says.
|
||||
/// The pre-revamp release read its unscoped item with
|
||||
/// `kSecAttrSynchronizableAny` and no identity in the key, so a credential
|
||||
/// retained across an Apple Account change is already served to whoever
|
||||
/// holds the device — this migration does not widen that. What it must not
|
||||
/// do is re-file that secret as synchronizable under the identity that is
|
||||
/// current now, which would upload a login of unproven provenance into that
|
||||
/// person's iCloud Keychain and spread it to their other devices. Keeping
|
||||
/// the copy local costs the genuine owner nothing: their legacy item syncs,
|
||||
/// so every device of theirs finds it and adopts its own copy.
|
||||
@discardableResult
|
||||
func adoptLegacyCredential(for account: Account) -> Bool {
|
||||
guard let scope = currentScope() else { return false }
|
||||
guard account.isMigrated else { return false }
|
||||
guard !markers.hasAdopted(service: store.service, instanceID: account.instanceID, scope: scope) else { return false }
|
||||
guard let legacy = legacyCredential(for: account) else { return false }
|
||||
|
||||
@@ -254,7 +279,7 @@ final class AccountKeychainCredentials {
|
||||
}
|
||||
|
||||
let key = KeychainCredentialStore.credentialKey(scope: scope, accountID: account.id)
|
||||
let status = store.addIfAbsent(legacy, forKey: key, synchronizable: shouldSyncToiCloud)
|
||||
let status = store.addIfAbsent(legacy, forKey: key, synchronizable: false)
|
||||
|
||||
// A duplicate means a scoped credential landed between the check above
|
||||
// and this write — leave it strictly alone, it is newer than the legacy
|
||||
|
||||
@@ -66,6 +66,18 @@ final class CredentialAdoptionMarkerStore {
|
||||
marker(service: service, instanceID: instanceID, scope: scope) != nil
|
||||
}
|
||||
|
||||
/// The accounts whose credential on this device came from a legacy item.
|
||||
///
|
||||
/// Their copies are deliberately device-local, so anything that rewrites
|
||||
/// stored credentials to follow the sync setting has to leave them out.
|
||||
func adoptedAccountIDs(service: String, scope: String) -> Set<UUID> {
|
||||
Set(
|
||||
markers()
|
||||
.filter { $0.service == service && $0.identityScope == scope }
|
||||
.map(\.accountID)
|
||||
)
|
||||
}
|
||||
|
||||
private func marker(service: String, instanceID: UUID, scope: String) -> CredentialAdoptionMarker? {
|
||||
markers().first {
|
||||
$0.service == service && $0.instanceID == instanceID && $0.identityScope == scope
|
||||
|
||||
@@ -54,8 +54,6 @@ struct InstanceAccountsSection: View {
|
||||
} footer: {
|
||||
Text(String(localized: accounts.isEmpty ? "login.footer.loginBenefits" : "login.footer.loggedIn"))
|
||||
}
|
||||
|
||||
adoptionSection
|
||||
}
|
||||
|
||||
// MARK: - Rows
|
||||
@@ -114,38 +112,6 @@ struct InstanceAccountsSection: View {
|
||||
#endif
|
||||
}
|
||||
|
||||
/// The one-tap adoption of a login saved by an earlier version.
|
||||
///
|
||||
/// Shown only when such an item exists and has not been adopted. It is a
|
||||
/// deliberate action because a retained Keychain item carries no proof of
|
||||
/// which Apple Account it belonged to — the app cannot tell an inherited
|
||||
/// credential from the user's own.
|
||||
@ViewBuilder
|
||||
private var adoptionSection: some View {
|
||||
let adoptable = accounts.filter { account in
|
||||
appEnvironment?
|
||||
.accountCredentialsManager(for: account)?
|
||||
.hasAdoptableLegacyCredential(for: account) == true
|
||||
}
|
||||
|
||||
if !adoptable.isEmpty {
|
||||
Section {
|
||||
ForEach(adoptable) { account in
|
||||
Button {
|
||||
adoptLegacyCredential(for: account)
|
||||
} label: {
|
||||
Label(String(localized: "accounts.useSavedLogin"), systemImage: "key.horizontal")
|
||||
}
|
||||
#if os(tvOS)
|
||||
.buttonStyle(TVSettingsButtonStyle())
|
||||
#endif
|
||||
}
|
||||
} footer: {
|
||||
Text(String(localized: "accounts.useSavedLogin.footer"))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Presentation
|
||||
|
||||
private func displayName(for account: Account) -> String {
|
||||
@@ -198,18 +164,4 @@ struct InstanceAccountsSection: View {
|
||||
appEnvironment.instancesManager.setUsesAccountLogin(false, for: instance)
|
||||
}
|
||||
}
|
||||
|
||||
private func adoptLegacyCredential(for account: Account) {
|
||||
guard let appEnvironment,
|
||||
let credentials = appEnvironment.accountCredentialsManager(for: account)
|
||||
else { return }
|
||||
|
||||
if credentials.adoptLegacyCredential(for: account) {
|
||||
appEnvironment.accountsManager.setStatus(.signedIn, for: account.id)
|
||||
} else {
|
||||
appEnvironment.toastManager.showError(
|
||||
String(localized: "accounts.useSavedLogin.failed")
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,18 +158,21 @@ struct LegacyCredentialAdoptionTests {
|
||||
private func makeCredentials(
|
||||
service: String,
|
||||
scope: String,
|
||||
markers: CredentialAdoptionMarkerStore
|
||||
markers: CredentialAdoptionMarkerStore,
|
||||
settingsManager: SettingsManager? = nil
|
||||
) -> AccountKeychainCredentials {
|
||||
AccountKeychainCredentials(
|
||||
let credentials = AccountKeychainCredentials(
|
||||
service: service,
|
||||
credentialNoun: "credential",
|
||||
identityScope: CredentialIdentityScope(fixedScope: scope),
|
||||
markers: markers
|
||||
)
|
||||
credentials.settingsManager = settingsManager
|
||||
return credentials
|
||||
}
|
||||
|
||||
@Test("A legacy credential is never adopted automatically")
|
||||
func legacyCredentialIsNotAutoAdopted() {
|
||||
@Test("Reading a credential never falls back to the unscoped legacy item")
|
||||
func readingNeverFallsBackToLegacyItem() {
|
||||
let service = makeService()
|
||||
let instanceID = UUID()
|
||||
let account = Account.migrated(from: instanceID)
|
||||
@@ -177,17 +180,127 @@ struct LegacyCredentialAdoptionTests {
|
||||
|
||||
let credentials = makeCredentials(service: service, scope: "identity-B", markers: makeMarkers())
|
||||
|
||||
// Reading never falls back to the unscoped item — that is the whole
|
||||
// point of scoping, since a retained item proves nothing about who owns it.
|
||||
// The whole point of scoping: the legacy item is reachable only through
|
||||
// the adoption path, which copies it deliberately and records what it
|
||||
// took. An implicit read-through would leave nothing to bound deletion
|
||||
// with later.
|
||||
#expect(credentials.credential(for: account) == nil)
|
||||
#expect(credentials.isLoggedIn(for: account) == false)
|
||||
// But it is offered as an explicit action.
|
||||
#expect(credentials.hasAdoptableLegacyCredential(for: account) == true)
|
||||
|
||||
KeychainCredentialStore(service: service)
|
||||
.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
|
||||
}
|
||||
|
||||
@Test("An adopted credential is kept on this device even when sync is on")
|
||||
func adoptedCredentialIsNeverSynchronizable() {
|
||||
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 = true
|
||||
settings.syncInstances = true
|
||||
defer {
|
||||
settings.iCloudSyncEnabled = previousSync
|
||||
settings.syncInstances = previousInstances
|
||||
}
|
||||
|
||||
let credentials = makeCredentials(
|
||||
service: service,
|
||||
scope: "identity-A",
|
||||
markers: makeMarkers(),
|
||||
settingsManager: settings
|
||||
)
|
||||
#expect(credentials.adoptLegacyCredential(for: account) == true)
|
||||
|
||||
// A retained legacy item cannot be shown to belong to the identity
|
||||
// signed in now. Adopting it locally leaves exposure where the previous
|
||||
// release already had it; writing it synchronizably would upload
|
||||
// somebody else's secret into this identity's iCloud Keychain.
|
||||
let store = KeychainCredentialStore(service: service)
|
||||
let key = KeychainCredentialStore.credentialKey(scope: "identity-A", accountID: account.id)
|
||||
#expect(store.isSynchronizable(key: key) == false)
|
||||
|
||||
credentials.deleteCredential(for: account)
|
||||
store.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
|
||||
}
|
||||
|
||||
@Test("Turning iCloud sync on does not upload an adopted credential")
|
||||
func resyncLeavesAdoptedCredentialsLocal() {
|
||||
let service = makeService()
|
||||
let instanceID = UUID()
|
||||
let adopted = Account.migrated(from: instanceID)
|
||||
let typedIn = Account(instanceID: 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 credentials = makeCredentials(
|
||||
service: service,
|
||||
scope: "identity-A",
|
||||
markers: makeMarkers(),
|
||||
settingsManager: settings
|
||||
)
|
||||
#expect(credentials.adoptLegacyCredential(for: adopted) == true)
|
||||
credentials.setCredential("typed-in-secret", for: typedIn)
|
||||
|
||||
settings.iCloudSyncEnabled = true
|
||||
settings.syncInstances = true
|
||||
credentials.resyncCredentialsWithCurrentSetting()
|
||||
|
||||
let store = KeychainCredentialStore(service: service)
|
||||
// The login the user typed is theirs, and they just asked for it to
|
||||
// sync.
|
||||
#expect(
|
||||
store.isSynchronizable(
|
||||
key: KeychainCredentialStore.credentialKey(scope: "identity-A", accountID: typedIn.id)
|
||||
) == true
|
||||
)
|
||||
// The adopted one is left alone — its provenance is unproven, and the
|
||||
// legacy item it came from already syncs for whoever really owns it.
|
||||
#expect(
|
||||
store.isSynchronizable(
|
||||
key: KeychainCredentialStore.credentialKey(scope: "identity-A", accountID: adopted.id)
|
||||
) == false
|
||||
)
|
||||
|
||||
credentials.deleteCredential(for: adopted)
|
||||
credentials.deleteCredential(for: typedIn)
|
||||
store.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
|
||||
}
|
||||
|
||||
@Test("A login the user added themselves never inherits the legacy item")
|
||||
func onlyTheMigratedAccountAdopts() {
|
||||
let service = makeService()
|
||||
let instanceID = UUID()
|
||||
// Not `Account.migrated`: a second login on the same instance, created
|
||||
// by signing in. The legacy item is keyed by instance, so without the
|
||||
// migrated-account rule this one would match it.
|
||||
let sibling = Account(instanceID: instanceID)
|
||||
seedLegacyItem(service: service, instanceID: instanceID, value: "legacy-secret")
|
||||
|
||||
let credentials = makeCredentials(service: service, scope: "identity-A", markers: makeMarkers())
|
||||
|
||||
#expect(credentials.hasAdoptableLegacyCredential(for: sibling) == false)
|
||||
#expect(credentials.adoptLegacyCredential(for: sibling) == false)
|
||||
#expect(credentials.credential(for: sibling) == nil)
|
||||
|
||||
KeychainCredentialStore(service: service)
|
||||
.delete(key: KeychainCredentialStore.legacyKey(instanceID: instanceID))
|
||||
}
|
||||
|
||||
@Test("Adoption copies the legacy item and retains it")
|
||||
func adoptionCopiesAndRetains() {
|
||||
let service = makeService()
|
||||
|
||||
Reference in New Issue
Block a user