Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/bma/OIDC session end #8620

Merged
merged 8 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8616.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If an external account manager is configured on the server, use it to delete other sessions and hide the multi session deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ internal class DefaultAuthenticationService @Inject constructor(
}

// If an m.login.sso flow is present that is flagged as being for MSC3824 OIDC compatibility then we only return that flow
val oidcCompatibilityFlow = loginFlowResponse.flows.orEmpty().firstOrNull { it.type == "m.login.sso" && it.delegatedOidcCompatibilty == true }
val oidcCompatibilityFlow = loginFlowResponse.flows.orEmpty().firstOrNull { it.type == "m.login.sso" && it.delegatedOidcCompatibility == true }
val flows = if (oidcCompatibilityFlow != null) listOf(oidcCompatibilityFlow) else loginFlowResponse.flows

val supportsGetLoginTokenFlow = loginFlowResponse.flows.orEmpty().firstOrNull { it.type == "m.login.token" && it.getLoginToken == true } != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal data class LoginFlow(
* See [MSC3824](https://github.com/matrix-org/matrix-spec-proposals/pull/3824)
*/
@Json(name = "org.matrix.msc3824.delegated_oidc_compatibility")
val delegatedOidcCompatibilty: Boolean? = null,
val delegatedOidcCompatibility: Boolean? = null,

/**
* Whether a login flow of type m.login.token could accept a token issued using /login/get_token.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,6 @@ sealed class DevicesViewEvents : VectorViewEvents {
data class ShowManuallyVerify(val cryptoDeviceInfo: CryptoDeviceInfo) : DevicesViewEvents()

object PromptResetSecrets : DevicesViewEvents()

data class OpenBrowser(val url: String) : DevicesViewEvents()
}
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,20 @@ class DevicesViewModel @AssistedInject constructor(
private fun handleDelete(action: DevicesAction.Delete) {
val deviceId = action.deviceId

val accountManagementUrl = session.homeServerCapabilitiesService().getHomeServerCapabilities().externalAccountManagementUrl
if (accountManagementUrl != null) {
// Open external browser to delete this session
_viewEvents.post(
DevicesViewEvents.OpenBrowser(
url = accountManagementUrl.removeSuffix("/") + "?action=session_end&device_id=$deviceId"
)
)
} else {
doDelete(deviceId)
}
}

private fun doDelete(deviceId: String) {
setState {
copy(
request = Loading()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import im.vector.app.core.extensions.cleanup
import im.vector.app.core.extensions.configureWith
import im.vector.app.core.extensions.registerStartForActivityResult
import im.vector.app.core.platform.VectorBaseFragment
import im.vector.app.core.utils.openUrlInChromeCustomTab
import im.vector.app.databinding.DialogBaseEditTextBinding
import im.vector.app.databinding.FragmentGenericRecyclerBinding
import im.vector.app.features.auth.ReAuthActivity
Expand Down Expand Up @@ -95,6 +96,9 @@ class VectorSettingsDevicesFragment :
is DevicesViewEvents.PromptResetSecrets -> {
navigator.open4SSetup(requireActivity(), SetupMode.PASSPHRASE_AND_NEEDED_SECRETS_RESET)
}
is DevicesViewEvents.OpenBrowser -> {
openUrlInChromeCustomTab(requireContext(), null, it.url)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import timber.log.Timber

class DevicesViewModel @AssistedInject constructor(
@Assisted initialState: DevicesViewState,
activeSessionHolder: ActiveSessionHolder,
private val activeSessionHolder: ActiveSessionHolder,
private val getCurrentSessionCrossSigningInfoUseCase: GetCurrentSessionCrossSigningInfoUseCase,
private val getDeviceFullInfoListUseCase: GetDeviceFullInfoListUseCase,
private val refreshDevicesOnCryptoDevicesChangeUseCase: RefreshDevicesOnCryptoDevicesChangeUseCase,
Expand Down Expand Up @@ -69,6 +69,18 @@ class DevicesViewModel @AssistedInject constructor(
refreshDeviceList()
refreshIpAddressVisibility()
observePreferences()
initExternalAccountManagementUrl()
}

private fun initExternalAccountManagementUrl() {
setState {
copy(
externalAccountManagementUrl = activeSessionHolder.getSafeActiveSession()
?.homeServerCapabilitiesService()
?.getHomeServerCapabilities()
?.externalAccountManagementUrl
)
}
}

override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences?, key: String?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ data class DevicesViewState(
val devices: Async<DeviceFullInfoList> = Uninitialized,
val isLoading: Boolean = false,
val isShowingIpAddress: Boolean = false,
val externalAccountManagementUrl: String? = null,
) : MavericksState

data class DeviceFullInfoList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ class VectorSettingsDevicesFragment :
val unverifiedSessionsCount = deviceFullInfoList?.unverifiedSessionsCount ?: 0

renderSecurityRecommendations(inactiveSessionsCount, unverifiedSessionsCount)
renderCurrentSessionView(currentDeviceInfo, hasOtherDevices = otherDevices?.isNotEmpty().orFalse())
renderOtherSessionsView(otherDevices, state.isShowingIpAddress)
renderCurrentSessionView(currentDeviceInfo, hasOtherDevices = otherDevices?.isNotEmpty().orFalse(), state)
renderOtherSessionsView(otherDevices, state)
} else {
hideSecurityRecommendations()
hideCurrentSessionView()
Expand Down Expand Up @@ -347,13 +347,16 @@ class VectorSettingsDevicesFragment :
hideInactiveSessionsRecommendation()
}

private fun renderOtherSessionsView(otherDevices: List<DeviceFullInfo>?, isShowingIpAddress: Boolean) {
private fun renderOtherSessionsView(otherDevices: List<DeviceFullInfo>?, state: DevicesViewState) {
val isShowingIpAddress = state.isShowingIpAddress
if (otherDevices.isNullOrEmpty()) {
hideOtherSessionsView()
} else {
views.deviceListHeaderOtherSessions.isVisible = true
val colorDestructive = colorProvider.getColorFromAttribute(R.attr.colorError)
val multiSignoutItem = views.deviceListHeaderOtherSessions.menu.findItem(R.id.otherSessionsHeaderMultiSignout)
// Hide multi signout if we have an external account manager
multiSignoutItem.isVisible = state.externalAccountManagementUrl == null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because delegated OIDC can be enabled on the homeserver, but the homeserver does not have to provide an external account managemnt URL this should be tried to a new property like HomeServerCapabilities.delegatedOidcAuthEnabled.

See https://github.com/vector-im/element-android/pull/8627/files#r1305379005 for more context.

val nbDevices = otherDevices.size
multiSignoutItem.title = stringProvider.getQuantityString(R.plurals.device_manager_other_sessions_multi_signout_all, nbDevices, nbDevices)
multiSignoutItem.setTextColor(colorDestructive)
Expand All @@ -377,23 +380,24 @@ class VectorSettingsDevicesFragment :
views.deviceListOtherSessions.isVisible = false
}

private fun renderCurrentSessionView(currentDeviceInfo: DeviceFullInfo?, hasOtherDevices: Boolean) {
private fun renderCurrentSessionView(currentDeviceInfo: DeviceFullInfo?, hasOtherDevices: Boolean, state: DevicesViewState) {
currentDeviceInfo?.let {
renderCurrentSessionHeaderView(hasOtherDevices)
renderCurrentSessionHeaderView(hasOtherDevices, state)
renderCurrentSessionListView(it)
} ?: run {
hideCurrentSessionView()
}
}

private fun renderCurrentSessionHeaderView(hasOtherDevices: Boolean) {
private fun renderCurrentSessionHeaderView(hasOtherDevices: Boolean, state: DevicesViewState) {
views.deviceListHeaderCurrentSession.isVisible = true
val colorDestructive = colorProvider.getColorFromAttribute(R.attr.colorError)
val signoutSessionItem = views.deviceListHeaderCurrentSession.menu.findItem(R.id.currentSessionHeaderSignout)
signoutSessionItem.setTextColor(colorDestructive)
val signoutOtherSessionsItem = views.deviceListHeaderCurrentSession.menu.findItem(R.id.currentSessionHeaderSignoutOtherSessions)
signoutOtherSessionsItem.setTextColor(colorDestructive)
signoutOtherSessionsItem.isVisible = hasOtherDevices
// Hide signout other sessions if we have an external account manager
signoutOtherSessionsItem.isVisible = hasOtherDevices && state.externalAccountManagementUrl == null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto should be linked to HomeServerCapabilities.delegatedOidcAuthEnabled.

}

private fun renderCurrentSessionListView(currentDeviceInfo: DeviceFullInfo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,15 @@ class OtherSessionsFragment :
val nbDevices = viewState.devices()?.size ?: 0
stringProvider.getQuantityString(R.plurals.device_manager_other_sessions_multi_signout_all, nbDevices, nbDevices)
}
multiSignoutItem.isVisible = if (viewState.isSelectModeEnabled) {
viewState.devices.invoke()?.any { it.isSelected }.orFalse()
multiSignoutItem.isVisible = if (viewState.externalAccountManagementUrl != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto should be linked to HomeServerCapabilities.delegatedOidcAuthEnabled.

// Hide multi signout if we have an external account manager
false
} else {
viewState.devices.invoke()?.isNotEmpty().orFalse()
if (viewState.isSelectModeEnabled) {
viewState.devices.invoke()?.any { it.isSelected }.orFalse()
} else {
viewState.devices.invoke()?.isNotEmpty().orFalse()
}
}
val showAsActionFlag = if (viewState.isSelectModeEnabled) MenuItem.SHOW_AS_ACTION_IF_ROOM else MenuItem.SHOW_AS_ACTION_NEVER
multiSignoutItem.setShowAsAction(showAsActionFlag or MenuItem.SHOW_AS_ACTION_WITH_TEXT)
Expand Down Expand Up @@ -308,7 +313,10 @@ class OtherSessionsFragment :
)
)
views.otherSessionsNotFoundTextView.text = getString(R.string.device_manager_other_sessions_no_inactive_sessions_found)
updateSecurityLearnMoreButton(R.string.device_manager_learn_more_sessions_inactive_title, R.string.device_manager_learn_more_sessions_inactive)
updateSecurityLearnMoreButton(
R.string.device_manager_learn_more_sessions_inactive_title,
R.string.device_manager_learn_more_sessions_inactive
)
}
DeviceManagerFilterType.ALL_SESSIONS -> { /* NOOP. View is not visible */
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import timber.log.Timber

class OtherSessionsViewModel @AssistedInject constructor(
@Assisted private val initialState: OtherSessionsViewState,
activeSessionHolder: ActiveSessionHolder,
private val activeSessionHolder: ActiveSessionHolder,
private val getDeviceFullInfoListUseCase: GetDeviceFullInfoListUseCase,
private val signoutSessionsUseCase: SignoutSessionsUseCase,
private val pendingAuthHandler: PendingAuthHandler,
Expand All @@ -65,6 +65,18 @@ class OtherSessionsViewModel @AssistedInject constructor(
observeDevices(initialState.currentFilter)
refreshIpAddressVisibility()
observePreferences()
initExternalAccountManagementUrl()
}

private fun initExternalAccountManagementUrl() {
setState {
copy(
externalAccountManagementUrl = activeSessionHolder.getSafeActiveSession()
?.homeServerCapabilitiesService()
?.getHomeServerCapabilities()
?.externalAccountManagementUrl
)
}
}

override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences?, key: String?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ data class OtherSessionsViewState(
val isSelectModeEnabled: Boolean = false,
val isLoading: Boolean = false,
val isShowingIpAddress: Boolean = false,
val externalAccountManagementUrl: String? = null,
) : MavericksState {

constructor(args: OtherSessionsArgs) : this(excludeCurrentDevice = args.excludeCurrentDevice)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import im.vector.app.core.platform.VectorMenuProvider
import im.vector.app.core.resources.ColorProvider
import im.vector.app.core.resources.DrawableProvider
import im.vector.app.core.resources.StringProvider
import im.vector.app.core.utils.openUrlInChromeCustomTab
import im.vector.app.databinding.FragmentSessionOverviewBinding
import im.vector.app.features.auth.ReAuthActivity
import im.vector.app.features.crypto.recover.SetupMode
Expand Down Expand Up @@ -135,10 +136,19 @@ class SessionOverviewFragment :
activity?.let { SignOutUiWorker(it).perform() }
}

private fun confirmSignoutOtherSession() {
activity?.let {
buildConfirmSignoutDialogUseCase.execute(it, this::signoutSession)
.show()
private fun confirmSignoutOtherSession() = withState(viewModel) { state ->
if (state.externalAccountManagementUrl != null) {
// Manage in external account manager
openUrlInChromeCustomTab(
requireContext(),
null,
state.externalAccountManagementUrl.removeSuffix("/") + "?action=session_end&device_id=${state.deviceId}"
)
} else {
activity?.let {
buildConfirmSignoutDialogUseCase.execute(it, this::signoutSession)
.show()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ class SessionOverviewViewModel @AssistedInject constructor(
observeNotificationsStatus(initialState.deviceId)
refreshIpAddressVisibility()
observePreferences()
initExternalAccountManagementUrl()
}

private fun initExternalAccountManagementUrl() {
setState {
copy(
externalAccountManagementUrl = activeSessionHolder.getSafeActiveSession()
?.homeServerCapabilitiesService()
?.getHomeServerCapabilities()
?.externalAccountManagementUrl
)
}
}

override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences?, key: String?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ data class SessionOverviewViewState(
val isLoading: Boolean = false,
val notificationsStatus: NotificationsStatus = NotificationsStatus.NOT_SUPPORTED,
val isShowingIpAddress: Boolean = false,
val externalAccountManagementUrl: String? = null,
) : MavericksState {
constructor(args: SessionOverviewArgs) : this(
deviceId = args.deviceId
Expand Down