Merge pull request #3950 from vector-im/feature/bca/fix_restricted_format

FIx / bad format of restricted join rule

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")
+    }
+}

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
 ) {

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) })

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)
     }

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

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(
+                                        listOf(RoomJoinRulesAllowEntry.restrictedToRoom(state.parentSpaceId))
-                                                state.parentSpaceId,
-                                                listOf(state.homeServerName)
-                                        ))
                                 )
                             }
                         }

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() ?: entry.roomId?.let { MatrixItem.RoomItem(it, null, null) })?.let {
-                            summary?.toMatrixItem() ?: MatrixItem.RoomItem(entry.spaceID, null, null)
+                        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 {
+                        updatedAllowList = initialAllowList.orEmpty().mapNotNull {
-                            session.getRoomSummary(it.spaceID)?.toMatrixItem() ?: MatrixItem.RoomItem(it.spaceID, null, null)
+                            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 {
+                    state.initialAllowList.mapNotNull {
-                        session.getRoomSummary(it.spaceID)?.toMatrixItem() ?: MatrixItem.RoomItem(it.spaceID, null, null)
+                        it.roomId?.let { roomId ->
+                            session.getRoomSummary(roomId)?.toMatrixItem()
+                                    ?: MatrixItem.RoomItem(roomId, null, null)
+                        }
                     }
             )
             addAll(selection)

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(
+                                                        RoomJoinRulesAllowEntry.restrictedToRoom(spaceID)
-                                                                spaceID = spaceID,
-                                                                via = session.sessionParams.homeServerHost?.let { listOf(it) } ?: emptyList()
-                                                        )
                                                 )
                                         )
                                         if (e2eByDefault) {