From 25dbb3e9eac4d738afa5459f9f7100c62e793ffd Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:26:10 +0100 Subject: [PATCH 1/7] Fix bad copy/paste --- .../sdk/internal/session/identity/IdentityBulkLookupTask.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index 773d1066b5..2d2e5f823d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -107,9 +107,8 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( // Retrieve the new hash details val newHashDetailResponse = fetchAndStoreHashDetails(identityAPI) - if (hashDetailResponse.algorithms.contains(IdentityHashDetailResponse.ALGORITHM_SHA256).not()) { + if (newHashDetailResponse.algorithms.contains(IdentityHashDetailResponse.ALGORITHM_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 } From 07ffd3ded37b4756f2d580ab6377c1daf2914287 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:32:00 +0100 Subject: [PATCH 2/7] Improve code --- .../session/identity/IdentityBulkLookupTask.kt | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index 2d2e5f823d..ed206d237b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -98,22 +98,19 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( // Catch invalid hash pepper and retry if (canRetry && failure is Failure.ServerError && failure.error.code == MatrixError.M_INVALID_PEPPER) { // This is not documented, but the error can contain the new pepper! - if (!failure.error.newLookupPepper.isNullOrEmpty()) { + val newHashDetailResponse = if (!failure.error.newLookupPepper.isNullOrEmpty()) { // Store it and use it right now hashDetailResponse.copy(pepper = failure.error.newLookupPepper) .also { identityStore.setHashDetails(it) } - .let { lookUpInternal(identityAPI, threePids, it, false /* Avoid infinite loop */) } } else { // Retrieve the new hash details - val newHashDetailResponse = fetchAndStoreHashDetails(identityAPI) - - if (newHashDetailResponse.algorithms.contains(IdentityHashDetailResponse.ALGORITHM_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 - throw IdentityServiceError.BulkLookupSha256NotSupported - } - - lookUpInternal(identityAPI, threePids, newHashDetailResponse, false /* Avoid infinite loop */) + fetchAndStoreHashDetails(identityAPI) } + if (newHashDetailResponse.algorithms.contains(IdentityHashDetailResponse.ALGORITHM_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 + throw IdentityServiceError.BulkLookupSha256NotSupported + } + lookUpInternal(identityAPI, threePids, newHashDetailResponse, false /* Avoid infinite loop */) } else { // Other error throw failure From 887da0a3d61aa8c6b7175981b376501b01f53c3f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:37:25 +0100 Subject: [PATCH 3/7] Improve code #2 --- .../session/identity/IdentityBulkLookupTask.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index ed206d237b..c064b81e4a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -63,7 +63,8 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( val pepper = identityData.hashLookupPepper val hashDetailResponse = if (pepper == null) { // We need to fetch the hash details first - fetchAndStoreHashDetails(identityAPI) + fetchHashDetails(identityAPI) + .also { identityStore.setHashDetails(it) } } else { IdentityHashDetailResponse(pepper, identityData.hashLookupAlgorithm) } @@ -101,11 +102,11 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( val newHashDetailResponse = if (!failure.error.newLookupPepper.isNullOrEmpty()) { // Store it and use it right now hashDetailResponse.copy(pepper = failure.error.newLookupPepper) - .also { identityStore.setHashDetails(it) } } else { // Retrieve the new hash details - fetchAndStoreHashDetails(identityAPI) + fetchHashDetails(identityAPI) } + .also { identityStore.setHashDetails(it) } if (newHashDetailResponse.algorithms.contains(IdentityHashDetailResponse.ALGORITHM_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 throw IdentityServiceError.BulkLookupSha256NotSupported @@ -118,11 +119,10 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( } } - private suspend fun fetchAndStoreHashDetails(identityAPI: IdentityAPI): IdentityHashDetailResponse { - return executeRequest(null) { + private suspend fun fetchHashDetails(identityAPI: IdentityAPI): IdentityHashDetailResponse { + return executeRequest(null) { apiCall = identityAPI.hashDetails() } - .also { identityStore.setHashDetails(it) } } private fun handleSuccess(threePids: List, hashedAddresses: List, identityLookUpResponse: IdentityLookUpResponse): List { From 267ae457eeaee7a0f36ec3d33ebece01105e6ce2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:38:17 +0100 Subject: [PATCH 4/7] Use const --- .../sdk/internal/session/identity/IdentityBulkLookupTask.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index c064b81e4a..e429289a24 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -69,7 +69,7 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( IdentityHashDetailResponse(pepper, identityData.hashLookupAlgorithm) } - if (hashDetailResponse.algorithms.contains("sha256").not()) { + if (hashDetailResponse.algorithms.contains(IdentityHashDetailResponse.ALGORITHM_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 could be outdated, the identity server maybe now supports sha256 throw IdentityServiceError.BulkLookupSha256NotSupported From a44d00a31c84ecfb9fb0b7a6b84ef7b08eb1ea48 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:44:24 +0100 Subject: [PATCH 5/7] Create data class. --- .../identity/IdentityBulkLookupTask.kt | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index e429289a24..ccee5f7d47 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -78,16 +78,21 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( val lookupResult = lookUpInternal(identityAPI, params.threePids, hashDetailResponse, true) // Convert back to List - return handleSuccess(params.threePids, lookupResult.first, lookupResult.second) + return handleSuccess(params.threePids, lookupResult) } + data class LookUpData( + val hashedAddresses: List, + val identityLookUpResponse: IdentityLookUpResponse + ) + private suspend fun lookUpInternal(identityAPI: IdentityAPI, threePids: List, hashDetailResponse: IdentityHashDetailResponse, - canRetry: Boolean): Pair, IdentityLookUpResponse> { + canRetry: Boolean): LookUpData { val hashedAddresses = getHashedAddresses(threePids, hashDetailResponse.pepper) return try { - Pair(hashedAddresses, + LookUpData(hashedAddresses, executeRequest(null) { apiCall = identityAPI.lookup(IdentityLookUpParams( hashedAddresses, @@ -125,9 +130,12 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( } } - private fun handleSuccess(threePids: List, hashedAddresses: List, identityLookUpResponse: IdentityLookUpResponse): List { - return identityLookUpResponse.mappings.keys.map { hashedAddress -> - FoundThreePid(threePids[hashedAddresses.indexOf(hashedAddress)], identityLookUpResponse.mappings[hashedAddress] ?: error("")) + private fun handleSuccess(threePids: List, lookupData: LookUpData): List { + return lookupData.identityLookUpResponse.mappings.keys.map { hashedAddress -> + FoundThreePid( + threePids[lookupData.hashedAddresses.indexOf(hashedAddress)], + lookupData.identityLookUpResponse.mappings[hashedAddress] ?: error("") + ) } } } From 401b5e2b7a38cd67924d24a633195c7d6a0ab783 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:48:13 +0100 Subject: [PATCH 6/7] Move private fun --- .../identity/IdentityBulkLookupTask.kt | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index ccee5f7d47..61d44a7a65 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -46,17 +46,6 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( @UserId private val userId: String ) : IdentityBulkLookupTask { - private fun getHashedAddresses(threePids: List, pepper: String): List { - return withOlmUtility { olmUtility -> - threePids.map { threePid -> - base64ToBase64Url( - olmUtility.sha256(threePid.value.toLowerCase(Locale.ROOT) - + " " + threePid.toMedium() + " " + pepper) - ) - } - } - } - override suspend fun execute(params: IdentityBulkLookupTask.Params): List { val identityAPI = getIdentityApiAndEnsureTerms(identityApiProvider, userId) val identityData = identityStore.getIdentityData() ?: throw IdentityServiceError.NoIdentityServerConfigured @@ -124,6 +113,17 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( } } + private fun getHashedAddresses(threePids: List, pepper: String): List { + return withOlmUtility { olmUtility -> + threePids.map { threePid -> + base64ToBase64Url( + olmUtility.sha256(threePid.value.toLowerCase(Locale.ROOT) + + " " + threePid.toMedium() + " " + pepper) + ) + } + } + } + private suspend fun fetchHashDetails(identityAPI: IdentityAPI): IdentityHashDetailResponse { return executeRequest(null) { apiCall = identityAPI.hashDetails() From b65fc4f46b19fe3399e1e555044efbf33eecae00 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 22 Jan 2021 17:59:56 +0100 Subject: [PATCH 7/7] rename val --- .../sdk/internal/session/identity/IdentityBulkLookupTask.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt index 61d44a7a65..67f3b2aa56 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/IdentityBulkLookupTask.kt @@ -64,10 +64,10 @@ internal class DefaultIdentityBulkLookupTask @Inject constructor( throw IdentityServiceError.BulkLookupSha256NotSupported } - val lookupResult = lookUpInternal(identityAPI, params.threePids, hashDetailResponse, true) + val lookUpData = lookUpInternal(identityAPI, params.threePids, hashDetailResponse, true) // Convert back to List - return handleSuccess(params.threePids, lookupResult) + return handleSuccess(params.threePids, lookUpData) } data class LookUpData(