Support backup key lost in 4S migration #8013
Conversation
|
|
BillCarsonFr
requested review from
a team and
mnaturel
and removed request for
a team
|
SonarCloud Quality Gate failed. |
| @@ -60,6 +62,11 @@ import java.io.OutputStream | |||
| import kotlin.coroutines.Continuation | |||
| import kotlin.coroutines.resumeWithException | |||
|
|
|||
| enum class BackupMigrationStrategy { | |||
| IgnoreExistingBackup, | |||
| ProposeToConvertTo4S | |||
There was a problem hiding this comment.
We could add a trailing comma to follow the convention on the project.
| // } | ||
| BootstrapActions.MigrationHandleKeyLost -> { | ||
| // delete the backups as key is lost | ||
| session.coroutineScope.launch { |
There was a problem hiding this comment.
I think it would be better to encapsulate this logic into a dedicated method such as handleMigrationKeyLost().
| session.coroutineScope.launch { | ||
| clearAllExistingBackups() | ||
| } | ||
| doesKeyBackupExist = false |
There was a problem hiding this comment.
I wonder if this part should also be inside the coroutine so that we are sure to have a consistent state. Let's say there is an error when clearing the existing backups for instance, then we should not update the state and the doesKeyBackupExist field to false I guess?
| @@ -275,6 +313,22 @@ class BootstrapSharedViewModel @AssistedInject constructor( | |||
| } | |||
| } | |||
|
|
|||
| private suspend fun clearAllExistingBackups() { | |||
There was a problem hiding this comment.
Could we move this code into a dedicated use case ClearAllExistingBackupsUseCase?
| doesKeyBackupExist = true | ||
| isBackupCreatedFromPassphrase = keyVersion.getAuthDataAsMegolmBackupAuthData()?.privateKeySalt != null | ||
| setState { | ||
| copy(step = BootstrapStep.FirstForm(keyBackUpExist = doesKeyBackupExist, methods = this.secureBackupMethod)) |
There was a problem hiding this comment.
It does not make big difference but since doesKeyBackupExist is mutable should we pass true to keyBackUpExist when updating the state instead?
|
According to @BillCarsonFr , this PR can be closed, Synapse issue is fixed: matrix-org/synapse#14923 |
Type of change
Content
Adds a button if you forgot your backup pass during 4S migration.
Also added a config option that allows to discard the existing backup and setup 4S with a fresh one (default false).
Motivation and context
When setting up 4S for the first time on an account with an existing key backup (Migration from old accounts pre 4S), there is no option when the user did forget his backup passphrase/key.
It's particularly annoying if secure backup is required, because you will be locked out the app (the backup pass is required to setup 4S.
This state of no 4S with existing backup can also happens if a user is deactivated then reactivated, created an issue on synapse to track that matrix-org/synapse#14923
Screenshots / GIFs
New forgot button
|
|
|
If existing backup ignored user will directly start the setup flow
Tests
Tested devices
Checklist