Skip to content

fix: Fix backup restoration #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LouisCAD merged 3 commits into from
Apr 28, 2026
Merged

fix: Fix backup restoration #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e73985c
fix: Add missing AttemptingToConnect state emission
LouisCAD Apr 28, 2026
3e0193d
chore: Add pre-made key filters in KeyPairManager
LouisCAD Apr 28, 2026
e3cce74
fix: Stop removing all passkeys after restoration from backup
LouisCAD Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions multiplatform-lib/src/commonMain/kotlin/internal/AuthenticatorFacadeImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ internal class AuthenticatorFacadeImpl(

private suspend fun FlowCollector<Account.Status.NotConnected>.restoreFromBackupAttempts(account: AccountEntity) {
withRetries(userId = account.id) {
emit(Account.Status.NotConnected.AttemptingToConnect)
migrationManager.restore(account = account) { userId, token ->
authenticatorBridge.persistTokenForAccount(userId, token)
}
Expand Down
5 changes: 5 additions & 0 deletions multiplatform-lib/src/commonMain/kotlin/internal/KeyManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,9 @@ internal interface KeyPairManager {
val privateKeyPurposes = KeyPurposes.privateKeyDefaults
val publicKeyPurposes = KeyPurposes.publicKeyDefaults
}

object Filters {
fun forUserId(userId: Long): (name: String) -> Boolean = { it.startsWith("$userId-") }
fun forPasskeyId(passkeyId: String): (name: String) -> Boolean = { "-$passkeyId-" in it }
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filters.forPasskeyId() currently matches names containing "-$passkeyId-". That works for Android file names like "$userId-$keyId-private.key", but it won’t match the Apple Keychain tag format used in KeyPairManagerImpl.apple.kt (tag is "$userId-$keyId"), so deleteKeysMatching(Filters.forPasskeyId(...)) will be a no-op on Apple and can leave stale keys that may be picked up later by findKeyIdFor(Filters.forUserId(...)). Consider making the predicate compatible with both naming schemes (e.g., match "-$passkeyId-" OR endsWith("-$passkeyId"), or parse the keyId out of the name/tag and compare exactly).

Suggested change
fun forPasskeyId(passkeyId: String): (name: String) -> Boolean = { "-$passkeyId-" in it }
fun forPasskeyId(passkeyId: String): (name: String) -> Boolean = {
"-$passkeyId-" in it || it.endsWith("-$passkeyId")
}

Copilot uses AI. Check for mistakes.
}
}
14 changes: 8 additions & 6 deletions multiplatform-lib/src/commonMain/kotlin/internal/managers/AuthenticatorManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.infomaniak.auth.lib.internal.managers

import com.infomaniak.auth.lib.internal.CryptoObjectsBuilder
import com.infomaniak.auth.lib.internal.Failure
import com.infomaniak.auth.lib.internal.KeyPairManager
import com.infomaniak.auth.lib.internal.KeyPairManagerImpl
import com.infomaniak.auth.lib.internal.extensions.firstOrElse
import com.infomaniak.auth.lib.internal.models.ClientExtensionResults
Expand All @@ -32,14 +33,15 @@ import com.infomaniak.auth.lib.models.migration.ApiToken
import io.ktor.utils.io.core.toByteArray
import kotlinx.serialization.json.Json
import okio.ByteString.Companion.toByteString
import com.infomaniak.auth.lib.internal.KeyPairManager.Filters as KeyFilters

internal class AuthenticatorManager(
private val webAuthnRepository: WebAuthnRepository,
private val accountsRepository: AccountsRepository,
) {

private val cryptoObjectsBuilder by lazy { CryptoObjectsBuilder() }
private val keyPairManager by lazy { KeyPairManagerImpl() }
val keyPairManager: KeyPairManager by lazy { KeyPairManagerImpl() }

Comment on lines 43 to 45
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keyPairManager was changed from private to an exposed val so other classes can reach into the underlying key store. This increases coupling and makes it harder to change key management internals later. Consider keeping keyPairManager private and adding a narrow AuthenticatorManager API for the needed operation (e.g. delete keys for a specific passkeyId) so callers don’t need direct access to the manager implementation.

Copilot uses AI. Check for mistakes.
private val base64NoPadding get() = cryptoObjectsBuilder.base64UrlSafeNoPadding

Expand Down Expand Up @@ -74,7 +76,7 @@ internal class AuthenticatorManager(
userId: Long,
keyIdOrDefault: String? = null,
): Xor<ApiToken, Failure.KeyManagement.KeyNotFound> {
val keyId = keyIdOrDefault ?: keyPairManager.findKeyIdFor { it.startsWith("$userId-") }
val keyId = keyIdOrDefault ?: keyPairManager.findKeyIdFor(KeyFilters.forUserId(userId))
?: return Xor.Second(Failure.KeyManagement.KeyNotFound("No key found for user $userId"))

val authenticationOptions = webAuthnRepository.challenge(clientId)
Expand Down Expand Up @@ -124,22 +126,22 @@ internal class AuthenticatorManager(
}

suspend fun removeAccount(token: String, userId: Long) {
val passkeyId = keyPairManager.findKeyIdFor { it.startsWith("$userId-") }
val passkeyId = keyPairManager.findKeyIdFor(KeyFilters.forUserId(userId))

if (passkeyId != null) {
// If we have a passkey for this account, revoke it against the backend and delete it
webAuthnRepository.deletePasskey(token, passkeyId)
val _ = keyPairManager.deleteKeysMatching { "-$passkeyId-" in it }
val _ = keyPairManager.deleteKeysMatching(KeyFilters.forPasskeyId(passkeyId))
}

accountsRepository.deleteAccount(userId)
}

suspend fun deleteKeysFor(userId: Long) {
val _ = keyPairManager.deleteKeysMatching { it.startsWith("$userId-") }
val _ = keyPairManager.deleteKeysMatching(KeyFilters.forUserId(userId))
}

suspend fun getKeyIdFor(userId: Long): String? {
return keyPairManager.findKeyIdFor { it.startsWith("$userId-") }
return keyPairManager.findKeyIdFor(KeyFilters.forUserId(userId))
}
}
15 changes: 8 additions & 7 deletions multiplatform-lib/src/commonMain/kotlin/internal/managers/MigrationManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import kotlinx.io.IOException
import org.kotlincrypto.macs.hmac.sha2.HmacSHA256
import kotlin.uuid.ExperimentalUuidApi
import kotlin.uuid.Uuid
import com.infomaniak.auth.lib.internal.KeyPairManager.Filters as keyFilters

internal class MigrationManager(
private val accountsDatabase: AccountsDatabase,
Expand All @@ -61,26 +62,26 @@ internal class MigrationManager(
}

suspend fun restore(account: AccountEntity, persistToken: suspend (userId: Long, token: String) -> Unit) {
val keyId = authenticatorManager.getKeyIdFor(account.id) ?: return
val oldKeyId = authenticatorManager.getKeyIdFor(account.id) ?: return //TODO: Handle multiple passkeys present.
// Get token with previous passkey
val token = authenticatorManager.getToken(
val tokenFromOldPasskey = authenticatorManager.getToken(
clientId = clientId,
userId = account.id,
keyIdOrDefault = keyId,
keyIdOrDefault = oldKeyId,
).firstOrElse { error(it) }
// Register a new passkey
val newKeyId = authenticatorManager.registerPasskey(token.accessToken, account.id)
val newKeyId = authenticatorManager.registerPasskey(tokenFromOldPasskey.accessToken, account.id)
// Getting a new token with the new passkey
val tokenWithNewPassKey = authenticatorManager.getToken(
clientId = clientId,
userId = account.id,
keyIdOrDefault = newKeyId,
).firstOrElse { error(it) }
persistToken(account.id, tokenWithNewPassKey.accessToken)
dao.upsert(account.copy(status = AccountEntity.Status.LoggedIn))
// We can safely delete the old passkey, as the new one is working and the old token won't be valid anymore
authenticatorManager.deleteKeysFor(account.id)
webAuthnRepository.deletePasskey(tokenWithNewPassKey.accessToken, keyId)
webAuthnRepository.deletePasskey(tokenWithNewPassKey.accessToken, oldKeyId)
val _ = authenticatorManager.keyPairManager.deleteKeysMatching(keyFilters.forPasskeyId(oldKeyId))
dao.upsert(account.copy(status = AccountEntity.Status.LoggedIn))
Comment on lines 80 to +84
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dao.upsert(...LoggedIn) is now executed after backend/local cleanup. Since this method runs under withRetries, any failure in deletePasskey(...) will trigger a retry, but the retry path will register yet another new passkey again—potentially creating multiple passkeys server-side. Consider marking the account LoggedIn (and persisting the token) as soon as the new passkey token is confirmed, and then perform old-passkey/key cleanup as best-effort (or at least ensure cleanup failures don’t cause repeated passkey registrations).

Copilot uses AI. Check for mistakes.
}

suspend fun addLegacyAccountsToDB() {
Expand Down
Loading