From ec77006ddfbbd8b3addff5a93ace8b6cd474c267 Mon Sep 17 00:00:00 2001
From: Valere <valeref@matrix.org>
Date: Wed, 1 Sep 2021 17:15:37 +0200
Subject: [PATCH] FIx / bad format of restricted join rule
---
.../room/model/RoomJoinRulesAllowEntry.kt | 14 +++++++----
.../room/model/RoomJoinRulesContent.kt | 4 +++-
.../session/permalinks/ViaParameterFinder.kt | 4 ++++
.../session/room/state/DefaultStateService.kt | 2 +-
.../internal/session/space/DefaultSpace.kt | 12 +++++++++-
.../createroom/CreateRoomViewModel.kt | 5 +---
.../RoomJoinRuleChooseRestrictedViewModel.kt | 24 ++++++++++++-------
.../spaces/create/CreateSpaceViewModelTask.kt | 5 +---
8 files changed, 45 insertions(+), 25 deletions(-)
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesAllowEntry.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesAllowEntry.kt
index 7b87bc34d2..16298b8ba9 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesAllowEntry.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesAllowEntry.kt
@@ -23,11 +23,15 @@ import com.squareup.moshi.JsonClass
@JsonClass(generateAdapter = true)
data class RoomJoinRulesAllowEntry(
/**
- * space: The room ID of the space to check the membership of.
+ * The room ID to check the membership of.
*/
- @Json(name = "space") val spaceID: String,
+ @Json(name = "room_id") val roomId: String?,
/**
- * via: A list of servers which may be used to peek for membership of the space.
+ * "m.room_membership" to describe that we are allowing access via room membership. Future MSCs may define other types.
*/
- @Json(name = "via") val via: List<String>
-)
+ @Json(name = "type") val type: String?
+) {
+ companion object {
+ fun restrictedToRoom(roomId: String) = RoomJoinRulesAllowEntry(roomId, "m.room_membership")
+ }
+}
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesContent.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesContent.kt
index 33f402cad3..871b299f93 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesContent.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesContent.kt
@@ -29,7 +29,9 @@ import timber.log.Timber
data class RoomJoinRulesContent(
@Json(name = "join_rule") val _joinRules: String? = null,
/**
- * If the allow key is an empty list (or not a list at all), then the room reverts to standard public join rules
+ * If the allow key is an empty list (or not a list at all),
+ * then no users are allowed to join without an invite.
+ * Each entry is expected to be an object with the following keys:
*/
@Json(name = "allow") val allowList: List<RoomJoinRulesAllowEntry>? = null
) {
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/permalinks/ViaParameterFinder.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/permalinks/ViaParameterFinder.kt
index 21f55bbc42..dff82cb42e 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/permalinks/ViaParameterFinder.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/permalinks/ViaParameterFinder.kt
@@ -78,6 +78,10 @@ internal class ViaParameterFinder @Inject constructor(
.toSet()
}
+ // not used much for now but as per MSC1772
+ // the via parameter of m.space.child must contain a via key which gives a list of candidate servers that can be used to join the room.
+ // It is possible for the list of candidate servers and the list of authorised servers to diverge.
+ // It may not be possible for a user to join a room if there's no overlap between these
fun computeViaParamsForRestricted(roomId: String, max: Int): List<String> {
val userThatCanInvite = roomGetterProvider.get().getRoom(roomId)
?.getRoomMembers(roomMemberQueryParams { memberships = listOf(Membership.JOIN) })
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt
index 7eed22f65f..4ec27976a2 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt
@@ -182,7 +182,7 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
override suspend fun setJoinRuleRestricted(allowList: List<String>) {
// we need to compute correct via parameters and check if PL are correct
val allowEntries = allowList.map { spaceId ->
- RoomJoinRulesAllowEntry(spaceId, viaParameterFinder.computeViaParamsForRestricted(spaceId, 3))
+ RoomJoinRulesAllowEntry.restrictedToRoom(spaceId)
}
updateJoinRule(RoomJoinRules.RESTRICTED, null, allowEntries)
}
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpace.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpace.kt
index 233eef45f8..7b513678e3 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpace.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpace.kt
@@ -21,6 +21,7 @@ import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.events.model.toContent
import org.matrix.android.sdk.api.session.events.model.toModel
import org.matrix.android.sdk.api.session.room.Room
+import org.matrix.android.sdk.api.session.room.model.RoomJoinRules
import org.matrix.android.sdk.api.session.room.model.RoomSummary
import org.matrix.android.sdk.api.session.space.Space
import org.matrix.android.sdk.api.session.space.model.SpaceChildContent
@@ -53,12 +54,21 @@ internal class DefaultSpace(
autoJoin: Boolean,
suggested: Boolean?) {
// Find best via
+ val bestVia = viaServers
+ ?: (spaceSummaryDataSource.getRoomSummary(roomId)
+ ?.takeIf { it.joinRules == RoomJoinRules.RESTRICTED }
+ ?.let {
+ // for restricted room, best to take via from users that can invite in the
+ // child room
+ viaParameterFinder.computeViaParamsForRestricted(roomId, 3)
+ }
+ ?: viaParameterFinder.computeViaParams(roomId, 3))
room.sendStateEvent(
eventType = EventType.STATE_SPACE_CHILD,
stateKey = roomId,
body = SpaceChildContent(
- via = viaServers ?: viaParameterFinder.computeViaParams(roomId, 3),
+ via = bestVia,
autoJoin = autoJoin,
order = order,
suggested = suggested
diff --git a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewModel.kt b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewModel.kt
index f40829e4b7..fb70003dcf 100644
--- a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewModel.kt
@@ -253,10 +253,7 @@ class CreateRoomViewModel @AssistedInject constructor(@Assisted private val init
state.parentSpaceId?.let {
featurePreset = RestrictedRoomPreset(
session.getHomeServerCapabilities(),
- listOf(RoomJoinRulesAllowEntry(
- state.parentSpaceId,
- listOf(state.homeServerName)
- ))
+ listOf(RoomJoinRulesAllowEntry.restrictedToRoom(state.parentSpaceId))
)
}
}
diff --git a/vector/src/main/java/im/vector/app/features/roomprofile/settings/joinrule/advanced/RoomJoinRuleChooseRestrictedViewModel.kt b/vector/src/main/java/im/vector/app/features/roomprofile/settings/joinrule/advanced/RoomJoinRuleChooseRestrictedViewModel.kt
index dbe4e0ced7..eaa435f853 100644
--- a/vector/src/main/java/im/vector/app/features/roomprofile/settings/joinrule/advanced/RoomJoinRuleChooseRestrictedViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/settings/joinrule/advanced/RoomJoinRuleChooseRestrictedViewModel.kt
@@ -77,14 +77,14 @@ class RoomJoinRuleChooseRestrictedViewModel @AssistedInject constructor(
val knownParentSpacesAllowed = mutableListOf<MatrixItem>()
val unknownAllowedOrRooms = mutableListOf<MatrixItem>()
initialAllowList.orEmpty().forEach { entry ->
- val summary = session.getRoomSummary(entry.spaceID)
+ val summary = entry.roomId?.let { session.getRoomSummary(it) }
if (summary == null // it's not known by me
|| summary.roomType != RoomType.SPACE // it's not a space
|| !roomSummary.flattenParentIds.contains(summary.roomId) // it's not a parent space
) {
- unknownAllowedOrRooms.add(
- summary?.toMatrixItem() ?: MatrixItem.RoomItem(entry.spaceID, null, null)
- )
+ (summary?.toMatrixItem() ?: entry.roomId?.let { MatrixItem.RoomItem(it, null, null) })?.let {
+ unknownAllowedOrRooms.add(it)
+ }
} else {
knownParentSpacesAllowed.add(summary.toMatrixItem())
}
@@ -136,8 +136,11 @@ class RoomJoinRuleChooseRestrictedViewModel @AssistedInject constructor(
currentRoomJoinRules = safeRule,
choices = choices,
initialAllowList = initialAllowList.orEmpty(),
- updatedAllowList = initialAllowList.orEmpty().map {
- session.getRoomSummary(it.spaceID)?.toMatrixItem() ?: MatrixItem.RoomItem(it.spaceID, null, null)
+ updatedAllowList = initialAllowList.orEmpty().mapNotNull {
+ it.roomId?.let { roomId ->
+ session.getRoomSummary(roomId)?.toMatrixItem()
+ ?: MatrixItem.RoomItem(roomId, null, null)
+ }
},
possibleSpaceCandidate = possibleSpaceCandidate,
unknownRestricted = unknownAllowedOrRooms,
@@ -158,7 +161,7 @@ class RoomJoinRuleChooseRestrictedViewModel @AssistedInject constructor(
}
if (state.currentRoomJoinRules == RoomJoinRules.RESTRICTED) {
- val allowDidChange = state.initialAllowList.map { it.spaceID } != state.updatedAllowList.map { it.id }
+ val allowDidChange = state.initialAllowList.map { it.roomId } != state.updatedAllowList.map { it.id }
setState {
copy(hasUnsavedChanges = allowDidChange)
}
@@ -317,8 +320,11 @@ class RoomJoinRuleChooseRestrictedViewModel @AssistedInject constructor(
// to make it easier for users to spot the changes
val union = mutableListOf<MatrixItem>().apply {
addAll(
- state.initialAllowList.map {
- session.getRoomSummary(it.spaceID)?.toMatrixItem() ?: MatrixItem.RoomItem(it.spaceID, null, null)
+ state.initialAllowList.mapNotNull {
+ it.roomId?.let { roomId ->
+ session.getRoomSummary(roomId)?.toMatrixItem()
+ ?: MatrixItem.RoomItem(roomId, null, null)
+ }
}
)
addAll(selection)
diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModelTask.kt b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModelTask.kt
index 0a3c392c83..1440efe6fe 100644
--- a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModelTask.kt
+++ b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModelTask.kt
@@ -111,10 +111,7 @@ class CreateSpaceViewModelTask @Inject constructor(
this.featurePreset = RestrictedRoomPreset(
homeServerCapabilities,
listOf(
- RoomJoinRulesAllowEntry(
- spaceID = spaceID,
- via = session.sessionParams.homeServerHost?.let { listOf(it) } ?: emptyList()
- )
+ RoomJoinRulesAllowEntry.restrictedToRoom(spaceID)
)
)
if (e2eByDefault) {