Identity: fix issue with logout request.

Also disconnect previous set identity server when changing url, when disconnecting, and when deactivating account
This commit is contained in:
Benoit Marty 2020-05-11 18:19:15 +02:00
parent 623056455b
commit 2beef7d816
8 changed files with 97 additions and 48 deletions

View file

@ -43,11 +43,18 @@ interface IdentityService {
/**
* Update the identity server url.
* @param url the new url. Set to null to disconnect from the identity server
* @param callback will notify the user is change successful. The String will be the final url of the identity server, or null.
* The SDK can append "https://" for instance.
* If successful, any previous identity server will be disconnected.
* In case of error, any previous identity server will remain configured.
* @param url the new url.
* @param callback will notify the user if change is successful. The String will be the final url of the identity server.
* The SDK can prepend "https://" for instance.
*/
fun setNewIdentityServer(url: String?, callback: MatrixCallback<String?>): Cancelable
fun setNewIdentityServer(url: String, callback: MatrixCallback<String>): Cancelable
/**
* Disconnect (logout) from the current identity server
*/
fun disconnect(callback: MatrixCallback<Unit>): Cancelable
/**
* This will ask the identity server to send an email or an SMS to let the user confirm he owns the ThreePid

View file

@ -19,6 +19,7 @@ package im.vector.matrix.android.internal.session.account
import im.vector.matrix.android.internal.di.UserId
import im.vector.matrix.android.internal.network.executeRequest
import im.vector.matrix.android.internal.session.cleanup.CleanupSession
import im.vector.matrix.android.internal.session.identity.IdentityDisconnectTask
import im.vector.matrix.android.internal.task.Task
import org.greenrobot.eventbus.EventBus
import javax.inject.Inject
@ -34,6 +35,7 @@ internal class DefaultDeactivateAccountTask @Inject constructor(
private val accountAPI: AccountAPI,
private val eventBus: EventBus,
@UserId private val userId: String,
private val identityDisconnectTask: IdentityDisconnectTask,
private val cleanupSession: CleanupSession
) : DeactivateAccountTask {
@ -44,6 +46,11 @@ internal class DefaultDeactivateAccountTask @Inject constructor(
apiCall = accountAPI.deactivate(deactivateAccountParams)
}
// Logout from identity server if any, ignoring errors
runCatching {
identityDisconnectTask.execute(Unit)
}
cleanupSession.handle()
}
}

View file

@ -183,37 +183,37 @@ internal class DefaultIdentityService @Inject constructor(
}
}
override fun setNewIdentityServer(url: String?, callback: MatrixCallback<String?>): Cancelable {
val urlCandidate = url?.let { param ->
buildString {
if (!param.startsWith("http")) {
append("https://")
}
append(param)
}
}
override fun disconnect(callback: MatrixCallback<Unit>): Cancelable {
return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) {
val current = getCurrentIdentityServerUrl()
when (urlCandidate) {
current ->
// Nothing to do
Timber.d("Same URL, nothing to do")
null -> {
// Disconnect previous one if any
identityServiceStore.getIdentityServerDetails()?.let {
if (it.identityServerUrl != null && it.token != null) {
// Disconnect, ignoring any error
runCatching {
identityDisconnectTask.execute(Unit)
}
}
}
identityServiceStore.setUrl(null)
updateIdentityAPI(null)
updateAccountData(null)
}
else -> {
}
override fun setNewIdentityServer(url: String, callback: MatrixCallback<String>): Cancelable {
val urlCandidate = buildString {
if (!url.startsWith("http")) {
append("https://")
}
append(url)
}
return GlobalScope.launchToCallback(coroutineDispatchers.main, callback) {
val current = getCurrentIdentityServerUrl()
if (urlCandidate == current) {
// Nothing to do
Timber.d("Same URL, nothing to do")
} else {
// Disconnect previous one if any, first, because the token will change.
// In case of error when configuring the new identity server, this is not a big deal,
// we will ask for a new token on the previous Identity server
runCatching {
identityDisconnectTask.execute(Unit)
}
// Try to get a token
val token = getNewIdentityServerToken(urlCandidate)
@ -223,7 +223,6 @@ internal class DefaultIdentityService @Inject constructor(
updateAccountData(urlCandidate)
}
}
urlCandidate
}
}
@ -329,7 +328,7 @@ internal class DefaultIdentityService @Inject constructor(
private fun Throwable.isInvalidToken(): Boolean {
return this is Failure.ServerError
&& this.httpCode == HttpsURLConnection.HTTP_UNAUTHORIZED /* 401 */
&& httpCode == HttpsURLConnection.HTTP_UNAUTHORIZED /* 401 */
}
private fun Throwable.isTermsNotSigned(): Boolean {

View file

@ -48,7 +48,7 @@ internal interface IdentityAPI {
/**
* Logs out the access token, preventing it from being used to authenticate future requests to the server.
*/
@POST(NetworkConstants.URI_IDENTITY_PATH_V2 + "logout")
@POST(NetworkConstants.URI_IDENTITY_PATH_V2 + "account/logout")
fun logout(): Call<Unit>
/**

View file

@ -20,6 +20,7 @@ import im.vector.matrix.android.api.failure.Failure
import im.vector.matrix.android.api.failure.MatrixError
import im.vector.matrix.android.internal.network.executeRequest
import im.vector.matrix.android.internal.session.cleanup.CleanupSession
import im.vector.matrix.android.internal.session.identity.IdentityDisconnectTask
import im.vector.matrix.android.internal.task.Task
import org.greenrobot.eventbus.EventBus
import timber.log.Timber
@ -35,6 +36,7 @@ internal interface SignOutTask : Task<SignOutTask.Params, Unit> {
internal class DefaultSignOutTask @Inject constructor(
private val signOutAPI: SignOutAPI,
private val eventBus: EventBus,
private val identityDisconnectTask: IdentityDisconnectTask,
private val cleanupSession: CleanupSession
) : SignOutTask {
@ -60,6 +62,12 @@ internal class DefaultSignOutTask @Inject constructor(
}
}
// Logout from identity server if any
runCatching {
identityDisconnectTask.execute(Unit)
}
Timber.d("SignOut: cleanup session...")
cleanupSession.handle()
}

View file

@ -23,7 +23,8 @@ sealed class DiscoverySettingsAction : VectorViewModelAction {
object RetrieveBinding : DiscoverySettingsAction()
object Refresh : DiscoverySettingsAction()
data class ChangeIdentityServer(val url: String?) : DiscoverySettingsAction()
object DisconnectIdentityServer : DiscoverySettingsAction()
data class ChangeIdentityServer(val url: String) : DiscoverySettingsAction()
data class RevokeThreePid(val threePid: ThreePid) : DiscoverySettingsAction()
data class ShareThreePid(val threePid: ThreePid) : DiscoverySettingsAction()
data class FinalizeBind3pid(val threePid: ThreePid) : DiscoverySettingsAction()

View file

@ -167,7 +167,7 @@ class DiscoverySettingsFragment @Inject constructor(
AlertDialog.Builder(requireActivity())
.setTitle(R.string.disconnect_identity_server)
.setMessage(message)
.setPositiveButton(R.string.disconnect) { _, _ -> viewModel.handle(DiscoverySettingsAction.ChangeIdentityServer(null)) }
.setPositiveButton(R.string.disconnect) { _, _ -> viewModel.handle(DiscoverySettingsAction.DisconnectIdentityServer) }
.setNegativeButton(R.string.cancel, null)
.show()
}

View file

@ -93,6 +93,7 @@ class DiscoverySettingsViewModel @AssistedInject constructor(
when (action) {
DiscoverySettingsAction.Refresh -> refreshPendingEmailBindings()
DiscoverySettingsAction.RetrieveBinding -> retrieveBinding()
DiscoverySettingsAction.DisconnectIdentityServer -> disconnectIdentityServer()
is DiscoverySettingsAction.ChangeIdentityServer -> changeIdentityServer(action)
is DiscoverySettingsAction.RevokeThreePid -> revokeThreePid(action)
is DiscoverySettingsAction.ShareThreePid -> shareThreePid(action)
@ -102,6 +103,32 @@ class DiscoverySettingsViewModel @AssistedInject constructor(
}.exhaustive
}
private fun disconnectIdentityServer() {
setState {
copy(
identityServer = Loading()
)
}
session.identityService().disconnect(object : MatrixCallback<Unit> {
override fun onSuccess(data: Unit) {
setState {
copy(
identityServer = Success(null)
)
}
}
override fun onFailure(failure: Throwable) {
setState {
copy(
identityServer = Fail(failure)
)
}
}
})
}
private fun changeIdentityServer(action: DiscoverySettingsAction.ChangeIdentityServer) {
setState {
copy(