From 6e43e9b51c36dcce14323bc598ae9492e2a242f7 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoitm@matrix.org>
Date: Thu, 7 May 2020 22:51:18 +0200
Subject: [PATCH] Identity: refresh pepper, logout feature and other
improvements
---
.../matrix/android/api/failure/MatrixError.kt | 8 ++-
.../session/identity/BulkLookupTask.kt | 72 ++++++++++++++-----
.../identity/DefaultIdentityService.kt | 22 +++---
.../identity/IdentityDisconnectTask.kt | 37 ++++++++++
.../session/identity/IdentityModule.kt | 3 +
.../settings/VectorSettingsGeneralFragment.kt | 4 ++
.../features/terms/ReviewTermsFragment.kt | 6 ++
7 files changed, 127 insertions(+), 25 deletions(-)
create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityDisconnectTask.kt
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/MatrixError.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/MatrixError.kt
index 2dcead7477..7c9ace5d82 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/MatrixError.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/MatrixError.kt
@@ -39,7 +39,10 @@ data class MatrixError(
// For M_LIMIT_EXCEEDED
@Json(name = "retry_after_ms") val retryAfterMillis: Long? = null,
// For M_UNKNOWN_TOKEN
- @Json(name = "soft_logout") val isSoftLogout: Boolean = false
+ @Json(name = "soft_logout") val isSoftLogout: Boolean = false,
+ // For M_INVALID_PEPPER
+ // {"error": "pepper does not match 'erZvr'", "lookup_pepper": "pQgMS", "algorithm": "sha256", "errcode": "M_INVALID_PEPPER"}
+ @Json(name = "lookup_pepper") val newLookupPepper: String? = null
) {
companion object {
@@ -131,6 +134,9 @@ data class MatrixError(
const val M_TERMS_NOT_SIGNED = "M_TERMS_NOT_SIGNED"
+ // For identity service
+ const val M_INVALID_PEPPER = "M_INVALID_PEPPER"
+
// Possible value for "limit_type"
const val LIMIT_TYPE_MAU = "monthly_active_user"
}
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/BulkLookupTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/BulkLookupTask.kt
index 0524e704f3..e780e33478 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/BulkLookupTask.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/BulkLookupTask.kt
@@ -16,6 +16,8 @@
package im.vector.matrix.android.internal.session.identity
+import im.vector.matrix.android.api.failure.Failure
+import im.vector.matrix.android.api.failure.MatrixError
import im.vector.matrix.android.api.session.identity.FoundThreePid
import im.vector.matrix.android.api.session.identity.IdentityServiceError
import im.vector.matrix.android.api.session.identity.ThreePid
@@ -26,6 +28,7 @@ import im.vector.matrix.android.internal.session.identity.db.IdentityServiceStor
import im.vector.matrix.android.internal.session.identity.model.IdentityHashDetailResponse
import im.vector.matrix.android.internal.session.identity.model.IdentityLookUpV2Params
import im.vector.matrix.android.internal.session.identity.model.IdentityLookUpV2Response
+import im.vector.matrix.android.internal.session.profile.ThirdPartyIdentifier
import im.vector.matrix.android.internal.task.Task
import java.util.Locale
import javax.inject.Inject
@@ -47,16 +50,14 @@ internal class DefaultBulkLookupTask @Inject constructor(
val pepper = entity.hashLookupPepper
val hashDetailResponse = if (pepper == null) {
// We need to fetch the hash details first
- executeRequest<IdentityHashDetailResponse>(null) {
- apiCall = identityAPI.hashDetails()
- }
- .also { identityServiceStore.setHashDetails(it) }
+ fetchAndStoreHashDetails(identityAPI)
} else {
IdentityHashDetailResponse(pepper, entity.hashLookupAlgorithm.toList())
}
if (hashDetailResponse.algorithms.contains("sha256").not()) {
- // TODO We should ask the user if he is ok to send their 3Pid in clear, but for the moment do not do it
+ // TODO We should ask the user if he is ok to send their 3Pid in clear, but for the moment we do not do it
+ // Also, what we have in cache could be outdated, the identity server maybe now supports sha256
throw IdentityServiceError.BulkLookupSha256NotSupported
}
@@ -69,20 +70,59 @@ internal class DefaultBulkLookupTask @Inject constructor(
}
}
- val identityLookUpV2Response = executeRequest<IdentityLookUpV2Response>(null) {
- apiCall = identityAPI.bulkLookupV2(IdentityLookUpV2Params(
- hashedAddresses,
- "sha256",
- hashDetailResponse.pepper
- ))
- }
-
- // TODO Catch invalid hash pepper and retry
+ val identityLookUpV2Response = lookUpInternal(identityAPI, hashedAddresses, hashDetailResponse, true)
// Convert back to List<FoundThreePid>
return handleSuccess(params.threePids, hashedAddresses, identityLookUpV2Response)
}
+ private suspend fun lookUpInternal(identityAPI: IdentityAPI,
+ hashedAddresses: List<String>,
+ hashDetailResponse: IdentityHashDetailResponse,
+ canRetry: Boolean): IdentityLookUpV2Response {
+ return try {
+ executeRequest(null) {
+ apiCall = identityAPI.bulkLookupV2(IdentityLookUpV2Params(
+ hashedAddresses,
+ "sha256",
+ hashDetailResponse.pepper
+ ))
+ }
+ } catch (failure: Throwable) {
+ // Catch invalid hash pepper and retry
+ if (canRetry && failure is Failure.ServerError && failure.error.code == MatrixError.M_INVALID_PEPPER) {
+ // This is not documented, by the error can contain the new pepper!
+ if (!failure.error.newLookupPepper.isNullOrEmpty()) {
+ // Store it and use it right now
+ hashDetailResponse.copy(pepper = failure.error.newLookupPepper)
+ .also { identityServiceStore.setHashDetails(it) }
+ .let { lookUpInternal(identityAPI, hashedAddresses, it, false /* Avoid infinite loop */) }
+ } else {
+ // Retrieve the new hash details
+ val newHashDetailResponse = fetchAndStoreHashDetails(identityAPI)
+
+ if (hashDetailResponse.algorithms.contains("sha256").not()) {
+ // TODO We should ask the user if he is ok to send their 3Pid in clear, but for the moment we do not do it
+ // Also, what we have in cache is maybe outdated, the identity server maybe now support sha256
+ throw IdentityServiceError.BulkLookupSha256NotSupported
+ }
+
+ lookUpInternal(identityAPI, hashedAddresses, newHashDetailResponse, false /* Avoid infinite loop */)
+ }
+ } else {
+ // Other error
+ throw failure
+ }
+ }
+ }
+
+ private suspend fun fetchAndStoreHashDetails(identityAPI: IdentityAPI): IdentityHashDetailResponse {
+ return executeRequest<IdentityHashDetailResponse>(null) {
+ apiCall = identityAPI.hashDetails()
+ }
+ .also { identityServiceStore.setHashDetails(it) }
+ }
+
private fun handleSuccess(threePids: List<ThreePid>, hashedAddresses: List<String>, identityLookUpV2Response: IdentityLookUpV2Response): List<FoundThreePid> {
return identityLookUpV2Response.mappings.keys.map { hashedAddress ->
FoundThreePid(threePids[hashedAddresses.indexOf(hashedAddress)], identityLookUpV2Response.mappings[hashedAddress] ?: error(""))
@@ -91,8 +131,8 @@ internal class DefaultBulkLookupTask @Inject constructor(
private fun ThreePid.toMedium(): String {
return when (this) {
- is ThreePid.Email -> "email"
- is ThreePid.Msisdn -> "msisdn"
+ is ThreePid.Email -> ThirdPartyIdentifier.MEDIUM_EMAIL
+ is ThreePid.Msisdn -> ThirdPartyIdentifier.MEDIUM_MSISDN
}
}
}
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/DefaultIdentityService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/DefaultIdentityService.kt
index ab264dd8b9..fa7facae73 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/DefaultIdentityService.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/DefaultIdentityService.kt
@@ -42,7 +42,6 @@ import im.vector.matrix.android.internal.session.openid.GetOpenIdTokenTask
import im.vector.matrix.android.internal.session.sync.model.accountdata.IdentityServerContent
import im.vector.matrix.android.internal.session.sync.model.accountdata.UserAccountData
import im.vector.matrix.android.internal.session.user.accountdata.UpdateUserAccountDataTask
-import im.vector.matrix.android.internal.task.TaskExecutor
import im.vector.matrix.android.internal.task.launchToCallback
import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
import kotlinx.coroutines.GlobalScope
@@ -57,7 +56,7 @@ internal class DefaultIdentityService @Inject constructor(
private val getOpenIdTokenTask: GetOpenIdTokenTask,
private val bulkLookupTask: BulkLookupTask,
private val identityRegisterTask: IdentityRegisterTask,
- private val taskExecutor: TaskExecutor,
+ private val identityDisconnectTask: IdentityDisconnectTask,
@Unauthenticated
private val unauthenticatedOkHttpClient: Lazy<OkHttpClient>,
@AuthenticatedIdentity
@@ -147,12 +146,19 @@ internal class DefaultIdentityService @Inject constructor(
// Nothing to do
Timber.d("Same URL, nothing to do")
null -> {
- // TODO Disconnect previous one if any
+ // Disconnect previous one if any
+ identityServiceStore.get()?.let {
+ if (it.identityServerUrl != null && it.token != null) {
+ // Disconnect, ignoring any error
+ runCatching {
+ identityDisconnectTask.execute(Unit)
+ }
+ }
+ }
identityServiceStore.setUrl(null)
updateAccountData(null)
}
else -> {
- // TODO: check first that it is a valid identity server
// Try to get a token
getIdentityServerToken(urlCandidate)
@@ -179,7 +185,7 @@ internal class DefaultIdentityService @Inject constructor(
}
}
- private suspend fun lookUpInternal(firstTime: Boolean, threePids: List<ThreePid>): List<FoundThreePid> {
+ private suspend fun lookUpInternal(canRetry: Boolean, threePids: List<ThreePid>): List<FoundThreePid> {
ensureToken()
return try {
@@ -187,12 +193,12 @@ internal class DefaultIdentityService @Inject constructor(
} catch (throwable: Throwable) {
// Refresh token?
when {
- throwable.isInvalidToken() && firstTime -> {
+ throwable.isInvalidToken() && canRetry -> {
identityServiceStore.setToken(null)
lookUpInternal(false, threePids)
}
- throwable.isTermsNotSigned() -> throw IdentityServiceError.TermsNotSignedException
- else -> throw throwable
+ throwable.isTermsNotSigned() -> throw IdentityServiceError.TermsNotSignedException
+ else -> throw throwable
}
}
}
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityDisconnectTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityDisconnectTask.kt
new file mode 100644
index 0000000000..0d0c24a4b0
--- /dev/null
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityDisconnectTask.kt
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2020 New Vector Ltd
+ *
+ * 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 im.vector.matrix.android.internal.session.identity
+
+import im.vector.matrix.android.api.session.identity.IdentityServiceError
+import im.vector.matrix.android.internal.network.executeRequest
+import im.vector.matrix.android.internal.task.Task
+import javax.inject.Inject
+
+internal interface IdentityDisconnectTask : Task<Unit, Unit>
+
+internal class DefaultIdentityDisconnectTask @Inject constructor(
+ private val identityApiProvider: IdentityApiProvider
+) : IdentityDisconnectTask {
+
+ override suspend fun execute(params: Unit) {
+ val identityAPI = identityApiProvider.identityApi ?: throw IdentityServiceError.NoIdentityServerConfigured
+
+ executeRequest<Unit>(null) {
+ apiCall = identityAPI.logout()
+ }
+ }
+}
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityModule.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityModule.kt
index b320bac28d..d735e8a785 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityModule.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/IdentityModule.kt
@@ -70,4 +70,7 @@ internal abstract class IdentityModule {
@Binds
abstract fun bindBulkLookupTask(task: DefaultBulkLookupTask): BulkLookupTask
+
+ @Binds
+ abstract fun bindIdentityDisconnectTask(task: DefaultIdentityDisconnectTask): IdentityDisconnectTask
}
diff --git a/vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsGeneralFragment.kt b/vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsGeneralFragment.kt
index b11ffbcd84..9bc524781c 100644
--- a/vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsGeneralFragment.kt
+++ b/vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsGeneralFragment.kt
@@ -130,6 +130,10 @@ class VectorSettingsGeneralFragment : VectorSettingsBaseFragment() {
// Unfortunately, this is not supported in lib v7
// it.editText.inputType = InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_EMAIL_ADDRESS
+ it.setOnPreferenceClickListener {
+ notImplemented()
+ true
+ }
it.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue ->
notImplemented()
diff --git a/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsFragment.kt b/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsFragment.kt
index 70d8ab8658..7e73a7c1b8 100644
--- a/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsFragment.kt
+++ b/vector/src/main/java/im/vector/riotx/features/terms/ReviewTermsFragment.kt
@@ -28,6 +28,7 @@ import im.vector.riotx.core.epoxy.onClick
import im.vector.riotx.core.extensions.cleanup
import im.vector.riotx.core.extensions.configureWith
import im.vector.riotx.core.extensions.exhaustive
+import im.vector.riotx.core.platform.VectorBaseActivity
import im.vector.riotx.core.platform.VectorBaseFragment
import im.vector.riotx.core.utils.openUrlInExternalBrowser
import kotlinx.android.synthetic.main.fragment_review_terms.*
@@ -76,6 +77,11 @@ class ReviewTermsFragment @Inject constructor(
super.onDestroyView()
}
+ override fun onResume() {
+ super.onResume()
+ (activity as? VectorBaseActivity)?.supportActionBar?.setTitle(R.string.terms_of_service)
+ }
+
override fun invalidate() = withState(reviewTermsViewModel) { state ->
when (state.termsList) {
is Loading -> {