Merge branch 'develop' into feature/bca/crypto_better_key_share

2022-04-27 12:54:41 +02:00 · 2022-04-27 12:54:41 +02:00 · 23f32aae84
parent 8920ed3de8 942bc13ccc
commit 23f32aae84
14 changed files with 114 additions and 38 deletions
--- a/changelog.d/5721.bugfix
+++ b/changelog.d/5721.bugfix
@ -0,0 +1 @@
+Improving deactivation experience along with a crash fix
--- a/changelog.d/5836.doc
+++ b/changelog.d/5836.doc
@ -0,0 +1 @@
+Update the PR process doc with 2 reviewers and a new reviewer team.
--- a/changelog.d/5847.bugfix
+++ b/changelog.d/5847.bugfix
@ -0,0 +1 @@
+Fixes missing call icons when threads are enabled
--- a/docs/pull_request.md
+++ b/docs/pull_request.md
@ -32,14 +32,15 @@ Also, draft PR should not stay indefinitely in this state. It may be removed if

 ##### PR Review Assignment

-We use automatic assignment for PR reviews. A PR is automatically routed by GitHub to a team member using the round robin algorithm. The process is the following:
+We use automatic assignment for PR reviews. A PR is automatically routed by GitHub to 2 team members using the round robin algorithm. The process is the following:

- The PR creator assigns the [element-android](https://github.com/orgs/vector-im/teams/element-android) team as a reviewer. They can skip this process and assign directly a specific member if they think they should take a look at it.
- GitHub automatically assigns one reviewer. If the chosen reviewer is not available (holiday, etc.), remove them and set again the team, GitHub will select another reviewer.
- The reviewer gets a notification to make the review: they review the code following the good practice (see the rest of this document).
+- The PR creator can assign specific people if they have another Android developer in their team or they think a specific reviewer should take a look at the PR.
+- If there are missing reviewers, the PR creator assigns the [element-android-reviewers](https://github.com/orgs/vector-im/teams/element-android-reviewers) team as a reviewer.
+- GitHub automatically assigns other reviewers. If one of the chosen reviewers is not available (holiday, etc.), remove them and set again the team, GitHub will select another reviewer.
+- Reviewers get a notification to make the review: they review the code following the good practice (see the rest of this document).
 - After making their own review, if they feel not confident enough, they can ask another person for a full review, or they can tag someone within a PR comment to check specific lines.

-For PRs coming from the community, the issue wrangler can assign either the team [element-android](https://github.com/orgs/vector-im/teams/element-android) or any member directly.
+For PRs coming from the community, the issue wrangler can assign either the team [element-android-reviewers](https://github.com/orgs/vector-im/teams/element-android-reviewers) or any members directly.

 ##### PR review time

--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/uia/UiaResult.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/uia/UiaResult.kt
@ -0,0 +1,23 @@
+/*
+ * Copyright 2022 The Matrix.org Foundation C.I.C.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.matrix.android.sdk.api.session.uia
+
+enum class UiaResult {
+    SUCCESS,
+    FAILURE,
+    CANCELLED
+}
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/uia/exceptions/UiaCancelledException.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/uia/exceptions/UiaCancelledException.kt
@ -0,0 +1,19 @@
+/*
+ * Copyright 2022 The Matrix.org Foundation C.I.C.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.matrix.android.sdk.api.session.uia.exceptions
+
+class UiaCancelledException(message: String? = null) : Exception(message)
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/UIAExt.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/UIAExt.kt
@ -20,6 +20,8 @@ import org.matrix.android.sdk.api.auth.UIABaseAuth
 import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
 import org.matrix.android.sdk.api.failure.Failure
 import org.matrix.android.sdk.api.failure.toRegistrationFlowResponse
+import org.matrix.android.sdk.api.session.uia.UiaResult
+import org.matrix.android.sdk.api.session.uia.exceptions.UiaCancelledException
 import timber.log.Timber
 import kotlin.coroutines.suspendCoroutine

@ -30,14 +32,15 @@ import kotlin.coroutines.suspendCoroutine
 * @param interceptor see doc in [UserInteractiveAuthInterceptor]
 * @param retryBlock called at the end of the process, in this block generally retry executing the task, with
 * provided authUpdate
- * @return true if UIA is handled without error
+ * @return UiaResult if UIA handled, failed or cancelled
+ *
 */
 internal suspend fun handleUIA(failure: Throwable,
                               interceptor: UserInteractiveAuthInterceptor,
-                               retryBlock: suspend (UIABaseAuth) -> Unit): Boolean {
+                               retryBlock: suspend (UIABaseAuth) -> Unit): UiaResult {
    Timber.d("## UIA: check error ${failure.message}")
    val flowResponse = failure.toRegistrationFlowResponse()
-            ?: return false.also {
+            ?: return UiaResult.FAILURE.also {
                Timber.d("## UIA: not a UIA error")
            }

@ -50,14 +53,19 @@ internal suspend fun handleUIA(failure: Throwable,
            interceptor.performStage(flowResponse, (failure as? Failure.ServerError)?.error?.code, continuation)
        }
    } catch (failure2: Throwable) {
-        Timber.w(failure2, "## UIA: failed to participate")
-        return false
+        return if (failure2 is UiaCancelledException) {
+            Timber.w(failure2, "## UIA: cancelled")
+            UiaResult.CANCELLED
+        } else {
+            Timber.w(failure2, "## UIA: failed to participate")
+            UiaResult.FAILURE
+        }
    }

    Timber.d("## UIA: updated auth")
    return try {
        retryBlock(authUpdate)
-        true
+        UiaResult.SUCCESS
    } catch (failure3: Throwable) {
        handleUIA(failure3, interceptor, retryBlock)
    }
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/DeleteDeviceTask.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/DeleteDeviceTask.kt
@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.crypto.tasks

 import org.matrix.android.sdk.api.auth.UIABaseAuth
 import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
+import org.matrix.android.sdk.api.session.uia.UiaResult
 import org.matrix.android.sdk.internal.auth.registration.handleUIA
 import org.matrix.android.sdk.internal.crypto.api.CryptoApi
 import org.matrix.android.sdk.internal.crypto.model.rest.DeleteDeviceParams
@ -47,13 +48,13 @@ internal class DefaultDeleteDeviceTask @Inject constructor(
            }
        } catch (throwable: Throwable) {
            if (params.userInteractiveAuthInterceptor == null ||
-                    !handleUIA(
+                    handleUIA(
                            failure = throwable,
                            interceptor = params.userInteractiveAuthInterceptor,
                            retryBlock = { authUpdate ->
                                execute(params.copy(userAuthParam = authUpdate))
                            }
-                    )
+                    ) != UiaResult.SUCCESS
            ) {
                Timber.d("## UIA: propagate failure")
                throw throwable
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/InitializeCrossSigningTask.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/InitializeCrossSigningTask.kt
@ -20,6 +20,7 @@ import dagger.Lazy
 import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
 import org.matrix.android.sdk.api.session.crypto.crosssigning.CryptoCrossSigningKey
 import org.matrix.android.sdk.api.session.crypto.crosssigning.KeyUsage
+import org.matrix.android.sdk.api.session.uia.UiaResult
 import org.matrix.android.sdk.api.util.toBase64NoPadding
 import org.matrix.android.sdk.internal.auth.registration.handleUIA
 import org.matrix.android.sdk.internal.crypto.MXOlmDevice
@ -126,13 +127,13 @@ internal class DefaultInitializeCrossSigningTask @Inject constructor(
                uploadSigningKeysTask.execute(uploadSigningKeysParams)
            } catch (failure: Throwable) {
                if (params.interactiveAuthInterceptor == null ||
-                        !handleUIA(
+                        handleUIA(
                                failure = failure,
                                interceptor = params.interactiveAuthInterceptor,
                                retryBlock = { authUpdate ->
                                    uploadSigningKeysTask.execute(uploadSigningKeysParams.copy(userAuthParam = authUpdate))
                                }
-                        )
+                        ) != UiaResult.SUCCESS
                ) {
                    Timber.d("## UIA: propagate failure")
                    throw failure
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/account/DeactivateAccountTask.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/account/DeactivateAccountTask.kt
@ -18,6 +18,8 @@ package org.matrix.android.sdk.internal.session.account

 import org.matrix.android.sdk.api.auth.UIABaseAuth
 import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
+import org.matrix.android.sdk.api.session.uia.UiaResult
+import org.matrix.android.sdk.api.session.uia.exceptions.UiaCancelledException
 import org.matrix.android.sdk.internal.auth.registration.handleUIA
 import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
 import org.matrix.android.sdk.internal.network.executeRequest
@ -51,18 +53,24 @@ internal class DefaultDeactivateAccountTask @Inject constructor(
            }
            true
        } catch (throwable: Throwable) {
-            if (!handleUIA(
-                            failure = throwable,
-                            interceptor = params.userInteractiveAuthInterceptor,
-                            retryBlock = { authUpdate ->
-                                execute(params.copy(userAuthParam = authUpdate))
-                            }
-                    )
-            ) {
-                Timber.d("## UIA: propagate failure")
-                throw throwable
-            } else {
-                false
+            when (handleUIA(
+                    failure = throwable,
+                    interceptor = params.userInteractiveAuthInterceptor,
+                    retryBlock = { authUpdate ->
+                        execute(params.copy(userAuthParam = authUpdate))
+                    }
+            )) {
+                UiaResult.SUCCESS   -> {
+                    false
+                }
+                UiaResult.FAILURE   -> {
+                    Timber.d("## UIA: propagate failure")
+                    throw throwable
+                }
+                UiaResult.CANCELLED -> {
+                    Timber.d("## UIA: cancelled")
+                    throw UiaCancelledException()
+                }
            }
        }

--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/profile/FinalizeAddingThreePidTask.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/profile/FinalizeAddingThreePidTask.kt
@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
 import org.matrix.android.sdk.api.failure.Failure
 import org.matrix.android.sdk.api.failure.toRegistrationFlowResponse
 import org.matrix.android.sdk.api.session.identity.ThreePid
+import org.matrix.android.sdk.api.session.uia.UiaResult
 import org.matrix.android.sdk.internal.auth.registration.handleUIA
 import org.matrix.android.sdk.internal.database.model.PendingThreePidEntity
 import org.matrix.android.sdk.internal.database.model.PendingThreePidEntityFields
@ -72,13 +73,13 @@ internal class DefaultFinalizeAddingThreePidTask @Inject constructor(
                true
            } catch (throwable: Throwable) {
                if (params.userInteractiveAuthInterceptor == null ||
-                        !handleUIA(
+                        handleUIA(
                                failure = throwable,
                                interceptor = params.userInteractiveAuthInterceptor,
                                retryBlock = { authUpdate ->
                                    execute(params.copy(userAuthParam = authUpdate))
                                }
-                        )
+                        ) != UiaResult.SUCCESS
                ) {
                    Timber.d("## UIA: propagate failure")
                    throw throwable.toRegistrationFlowResponse()
--- a/vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountFragment.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountFragment.kt
@ -34,6 +34,7 @@ import im.vector.app.features.analytics.plan.MobileScreen
 import im.vector.app.features.auth.ReAuthActivity
 import im.vector.app.features.settings.VectorSettingsActivity
 import org.matrix.android.sdk.api.auth.data.LoginFlowTypes
+import org.matrix.android.sdk.api.session.uia.exceptions.UiaCancelledException
 import javax.inject.Inject

 class DeactivateAccountFragment @Inject constructor() : VectorBaseFragment<FragmentDeactivateAccountBinding>() {
@ -114,7 +115,9 @@ class DeactivateAccountFragment @Inject constructor() : VectorBaseFragment<Fragm
                is DeactivateAccountViewEvents.OtherFailure  -> {
                    settingsActivity?.ignoreInvalidTokenError = false
                    dismissLoadingDialog()
-                    displayErrorDialog(it.throwable)
+                    if (it.throwable !is UiaCancelledException) {
+                        displayErrorDialog(it.throwable)
+                    }
                }
                DeactivateAccountViewEvents.Done             -> {
                    MainActivity.restartApp(requireActivity(), MainActivityArgs(clearCredentials = true, isAccountDeactivated = true))
@ -123,7 +126,8 @@ class DeactivateAccountFragment @Inject constructor() : VectorBaseFragment<Fragm
                    ReAuthActivity.newIntent(requireContext(),
                            it.registrationFlowResponse,
                            it.lastErrorCode,
-                            getString(R.string.deactivate_account_title)).let { intent ->
+                            getString(R.string.deactivate_account_title)
+                    ).let { intent ->
                        reAuthActivityResultLauncher.launch(intent)
                    }
                }
--- a/vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
@ -32,6 +32,7 @@ import org.matrix.android.sdk.api.auth.registration.RegistrationFlowResponse
 import org.matrix.android.sdk.api.failure.isInvalidUIAAuth
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.uia.DefaultBaseAuth
+import org.matrix.android.sdk.api.session.uia.exceptions.UiaCancelledException
 import org.matrix.android.sdk.api.util.fromBase64
 import timber.log.Timber
 import kotlin.coroutines.Continuation
@ -59,6 +60,7 @@ class DeactivateAccountViewModel @AssistedInject constructor(@Assisted private v
            is DeactivateAccountAction.DeactivateAccount -> handleDeactivateAccount(action)
            DeactivateAccountAction.SsoAuthDone          -> {
                Timber.d("## UIA - FallBack success")
+                _viewEvents.post(DeactivateAccountViewEvents.Loading())
                if (pendingAuth != null) {
                    uiaContinuation?.resume(pendingAuth!!)
                } else {
@ -66,6 +68,7 @@ class DeactivateAccountViewModel @AssistedInject constructor(@Assisted private v
                }
            }
            is DeactivateAccountAction.PasswordAuthDone  -> {
+                _viewEvents.post(DeactivateAccountViewEvents.Loading())
                val decryptedPass = session.secureStorageService()
                        .loadSecureSecret<String>(action.password.fromBase64().inputStream(), ReAuthActivity.DEFAULT_RESULT_KEYSTORE_ALIAS)
                uiaContinuation?.resume(
@ -78,7 +81,7 @@ class DeactivateAccountViewModel @AssistedInject constructor(@Assisted private v
            }
            DeactivateAccountAction.ReAuthCancelled      -> {
                Timber.d("## UIA - Reauth cancelled")
-                uiaContinuation?.resumeWithException(Exception())
+                uiaContinuation?.resumeWithException(UiaCancelledException())
                uiaContinuation = null
                pendingAuth = null
            }
@ -101,7 +104,7 @@ class DeactivateAccountViewModel @AssistedInject constructor(@Assisted private v
                        }
                )
                DeactivateAccountViewEvents.Done
-            } catch (failure: Exception) {
+            } catch (failure: Throwable) {
                if (failure.isInvalidUIAAuth()) {
                    DeactivateAccountViewEvents.InvalidAuth
                } else {
--- a/vector/src/main/res/menu/menu_timeline.xml
+++ b/vector/src/main/res/menu/menu_timeline.xml
@ -1,7 +1,8 @@
 <?xml version="1.0" encoding="utf-8"?>
 <menu xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
-    xmlns:tools="http://schemas.android.com/tools">
+    xmlns:tools="http://schemas.android.com/tools"
+    tools:ignore="AlwaysShowAction">

    <item
        android:id="@+id/timeline_setting"
@ -19,24 +20,27 @@
        android:title="@string/action_invite"
        app:showAsAction="never" />

+    <!-- We always want to show this item as an icon -->
    <item
        android:id="@+id/video_call"
        android:icon="@drawable/ic_video"
        android:title="@string/action_video_call"
        android:visible="false"
        app:iconTint="?colorPrimary"
-        app:showAsAction="ifRoom"
+        app:showAsAction="always"
        tools:visible="true" />

+    <!-- We always want to show this item as an icon -->
    <item
        android:id="@+id/voice_call"
        android:icon="@drawable/ic_phone"
-        android:title="@string/call"
+        android:title="@string/action_voice_call"
        android:visible="false"
        app:iconTint="?colorPrimary"
-        app:showAsAction="ifRoom"
+        app:showAsAction="always"
        tools:visible="true" />

+    <!-- We always want to show this item as an icon -->
    <item
        android:id="@+id/menu_timeline_thread_list"
        android:title="@string/action_view_threads"
@ -86,4 +90,4 @@
        android:title="@string/action_share"
        app:iconTint="?vctr_content_secondary"
        app:showAsAction="withText" />
-</menu>
+</menu>
				`@ -0,0 +1 @@`
				`Improving deactivation experience along with a crash fix`
				`@ -0,0 +1 @@`
				`Update the PR process doc with 2 reviewers and a new reviewer team.`
				`@ -0,0 +1 @@`
				`Fixes missing call icons when threads are enabled`