Fix matrix api joinRoom() for remote rooms #60
|
@ -22,7 +22,11 @@ function path(p, mxid, otherParams = {}) {
|
|||
const u = new URL(p, "http://localhost")
|
||||
if (mxid) u.searchParams.set("user_id", mxid)
|
||||
for (const entry of Object.entries(otherParams)) {
|
||||
if (entry[1] != undefined) {
|
||||
if (Array.isArray(entry[1])) {
|
||||
|
||||
for (const element of entry[1]) {
|
||||
u.searchParams.append(entry[0], element)
|
||||
}
|
||||
} else if (entry[1] != undefined) {
|
||||
u.searchParams.set(entry[0], entry[1])
|
||||
}
|
||||
}
|
||||
|
@ -62,11 +66,14 @@ async function createRoom(content) {
|
|||
}
|
||||
|
||||
/**
|
||||
* @param {string} roomIDOrAlias
|
||||
* @param {string?} [mxid]
|
||||
* @param {string[]?} [via]
|
||||
* @returns {Promise<string>} room ID
|
||||
*/
|
||||
async function joinRoom(roomIDOrAlias, mxid) {
|
||||
async function joinRoom(roomIDOrAlias, mxid, via) {
|
||||
/** @type {Ty.R.RoomJoined} */
|
||||
const root = await mreq.mreq("POST", path(`/client/v3/join/${roomIDOrAlias}`, mxid), {})
|
||||
const root = await mreq.mreq("POST", path(`/client/v3/join/${roomIDOrAlias}`, mxid, {via}), {})
|
||||
cadence
commented
Third parameter needs to be an object, so I fixed that for you. I also labelled the inbound parameters so that it highlights the call site if the wrong data is passed to Third parameter needs to be an object, so I fixed that for you. I also labelled the inbound parameters so that it highlights the call site if the wrong data is passed to `joinRoom`. :)
|
||||
return root.room_id
|
||||
}
|
||||
|
||||
|
|
|
@ -24,3 +24,7 @@ test("api path: real world mxid", t => {
|
|||
test("api path: extras number works", t => {
|
||||
t.equal(path(`/client/v3/rooms/!example/timestamp_to_event`, null, {ts: 1687324651120}), "/client/v3/rooms/!example/timestamp_to_event?ts=1687324651120")
|
||||
})
|
||||
|
||||
test("api path: multiple via params", t => {
|
||||
t.equal(path(`/client/v3/rooms/!example/join`, null, {via: ["cadence.moe", "matrix.org"], ts: 1687324651120}), "/client/v3/rooms/!example/join?via=cadence.moe&via=matrix.org&ts=1687324651120")
|
||||
})
|
||||
|
|
10
src/types.d.ts
vendored
|
@ -148,6 +148,14 @@ export namespace Event {
|
|||
prev_content?: any
|
||||
}
|
||||
|
||||
export type Outer_StrippedChildStateEvent = {
|
||||
type: string
|
||||
state_key: string
|
||||
sender: string
|
||||
origin_server_ts: number
|
||||
content: any
|
||||
}
|
||||
|
||||
export type M_Room_Message = {
|
||||
msgtype: "m.text" | "m.emote"
|
||||
body: string
|
||||
|
@ -344,7 +352,7 @@ export namespace R {
|
|||
export type Hierarchy = {
|
||||
avatar_url?: string
|
||||
canonical_alias?: string
|
||||
children_state: {}
|
||||
children_state: Event.Outer_StrippedChildStateEvent[]
|
||||
guest_can_join: boolean
|
||||
join_rule?: string
|
||||
name?: string
|
||||
|
|
|
@ -75,11 +75,14 @@ as.router.post("/api/link-space", defineEventHandler(async event => {
|
|||
const existing = select("guild_space", "guild_id", {}, "WHERE guild_id = ? OR space_id = ?").get(guildID, spaceID)
|
||||
if (existing) throw createError({status: 400, message: "Bad Request", data: `Guild ID ${guildID} or space ID ${spaceID} are already bridged and cannot be reused`})
|
||||
|
||||
const inviteSender = select("invite", "mxid", {mxid: session.data.mxid, room_id: spaceID}).pluck().get()
|
||||
const via = [ inviteSender?.match(/:(.*)/)?.[1] ?? "" ]
|
||||
|
||||
cadence
commented
Where are you going to find Where are you going to find `via` here?
Elliu
commented
Hmmm, I didn't change that because the bridge needed to be manually invited to the space so that it can be selected. However indeed, if the bridge is invited to the space, then kicked, the button to the space remains and if clicked, we have the same issue than for joining rooms, didn't notice that. From the Client-Server API doc there doesn't seem to be "via" or "server_name" information on the (unless the invite is old, all users from this HS left the space, and the room was purged from the HS, but nothing we can do about that) Hmmm, I didn't change that because the bridge needed to be manually invited to the space so that it can be selected.
However indeed, if the bridge is invited to the space, then kicked, the button to the space remains and if clicked, we have the same issue than for joining rooms, didn't notice that.
From the Client-Server API doc there doesn't seem to be "via" or "server_name" information on the `m.room.member` event, so I'm not sure.
I guess it should be safe to take the HS part of the user from `sender` in the `m.room.member`: as this user is able to send an invite, the space must be reachable through the inviter's HS
(unless the invite is old, all users from this HS left the space, and the room was purged from the HS, but nothing we can do about that)
Elliu
commented
Last commit uses that, should we add a check to enrich the error message too if for some reasons we cannot detect the via? Last commit uses that, should we add a check to enrich the error message too if for some reasons we cannot detect the via?
Elliu
commented
Added the error message asking to manually invite in case if /api/link-space joinRoom fails, similarly to /api/link Added the error message asking to manually invite in case if /api/link-space joinRoom fails, similarly to /api/link
|
||||
// Check space exists and bridge is joined
|
||||
try {
|
||||
await api.joinRoom(parsedBody.space_id)
|
||||
await api.joinRoom(parsedBody.space_id, null, via)
|
||||
} catch (e) {
|
||||
throw createError({status: 403, message: e.errcode, data: `${e.errcode} - ${e.message}`})
|
||||
throw createError({status: 400, message: "Unable To Join", data: `Unable to join the requested Matrix space. Please invite the bridge to the space and try again. (Server said: ${e.errcode} - ${e.message})`})
|
||||
}
|
||||
|
||||
// Check bridge has PL 100
|
||||
|
@ -134,19 +137,33 @@ as.router.post("/api/link", defineEventHandler(async event => {
|
|||
if (row) throw createError({status: 400, message: "Bad Request", data: `Channel ID ${row.channel_id} or room ID ${parsedBody.matrix} are already bridged and cannot be reused`})
|
||||
|
||||
// Check room is part of the guild's space
|
||||
let found = false
|
||||
let foundRoom = false
|
||||
/** @type {string[]?} */
|
||||
let foundVia = null
|
||||
for await (const room of api.generateFullHierarchy(spaceID)) {
|
||||
// When finding a space during iteration, look at space's children state, because we need a `via` to join the room (when we find it later)
|
||||
for (const state of room.children_state) {
|
||||
if (state.type === "m.space.child" && state.state_key === parsedBody.matrix) {
|
||||
foundVia = state.content.via
|
||||
}
|
||||
}
|
||||
|
||||
// When finding a room during iteration, see if it was the requested room (to confirm that the room is in the space)
|
||||
if (room.room_id === parsedBody.matrix && !room.room_type) {
|
||||
found = true
|
||||
break
|
||||
foundRoom = true
|
||||
}
|
||||
|
||||
if (foundRoom && foundVia) break
|
||||
}
|
||||
if (!found) throw createError({status: 400, message: "Bad Request", data: "Matrix room needs to be part of the bridged space"})
|
||||
if (!foundRoom) throw createError({status: 400, message: "Bad Request", data: "Matrix room needs to be part of the bridged space"})
|
||||
|
||||
// Check room exists and bridge is joined
|
||||
try {
|
||||
await api.joinRoom(parsedBody.matrix)
|
||||
await api.joinRoom(parsedBody.matrix, null, foundVia)
|
||||
cadence marked this conversation as resolved
Outdated
Elliu
commented
Not sure if adding this hint is a good idea, or if we should rather put it elsewhere (like in a "hint" member), or if we should not put it at all, or if we should add a "please consider manually inviting the bot user <> in the room" Not sure if adding this hint is a good idea, or if we should rather put it elsewhere (like in a "hint" member), or if we should not put it at all, or if we should add a "please consider manually inviting the bot user <> in the room"
cadence
commented
I added a separate branch for it to say a different error message when the join fails. I felt the ternary was too complicated.
I decided to do it this way because it's better to see actionable feedback (do this) rather than merely identifying a problem (it tells you that the via data is wrong, but not what correct data would look like, or how to actually fix it) (I don't know any clients that allow you to edit the via data without devtools) I added a separate branch for it to say a different error message when the join fails. I felt the ternary was too complicated.
> or if we should add a "please consider manually inviting
I decided to do it this way because it's better to see actionable feedback (do this) rather than merely identifying a problem (it tells you that the via data is wrong, but not what correct data would look like, or how to actually fix it) (I don't know any clients that allow you to edit the via data without devtools)
|
||||
} catch (e) {
|
||||
if (!foundVia) {
|
||||
throw createError({status: 400, message: "Unable To Join", data: `Unable to join the requested Matrix room. Please invite the bridge to the room and try again. (Server said: ${e.errcode} - ${e.message})`})
|
||||
}
|
||||
throw createError({status: 403, message: e.errcode, data: `${e.errcode} - ${e.message}`})
|
||||
}
|
||||
|
||||
|
|
|
@ -360,7 +360,7 @@ test("web link room: check that room is part of space (not in hierarchy)", async
|
|||
t.equal(called, 1)
|
||||
})
|
||||
|
||||
test("web link room: check that bridge can join room", async t => {
|
||||
test("web link room: check that bridge can join room (notices lack of via and asks for invite instead)", async t => {
|
||||
let called = 0
|
||||
const [error] = await tryToCatch(() => router.test("post", "/api/link", {
|
||||
sessionData: {
|
||||
|
@ -381,7 +381,55 @@ test("web link room: check that bridge can join room", async t => {
|
|||
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
|
||||
yield {
|
||||
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
children_state: {},
|
||||
children_state: [],
|
||||
guest_can_join: false,
|
||||
num_joined_members: 2
|
||||
}
|
||||
/* c8 ignore next */
|
||||
}
|
||||
}
|
||||
}))
|
||||
t.equal(error.data, "Unable to join the requested Matrix room. Please invite the bridge to the room and try again. (Server said: M_FORBIDDEN - not allowed to join I guess)")
|
||||
t.equal(called, 2)
|
||||
})
|
||||
|
||||
test("web link room: check that bridge can join room (uses via for join attempt)", async t => {
|
||||
let called = 0
|
||||
const [error] = await tryToCatch(() => router.test("post", "/api/link", {
|
||||
sessionData: {
|
||||
managedGuilds: ["665289423482519565"]
|
||||
},
|
||||
body: {
|
||||
discord: "665310973967597573",
|
||||
matrix: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
guild_id: "665289423482519565"
|
||||
},
|
||||
api: {
|
||||
async joinRoom(roomID, _, via) {
|
||||
called++
|
||||
t.deepEqual(via, ["cadence.moe", "hashi.re"])
|
||||
throw new MatrixServerError({errcode: "M_FORBIDDEN", error: "not allowed to join I guess"})
|
||||
},
|
||||
async *generateFullHierarchy(spaceID) {
|
||||
called++
|
||||
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
|
||||
yield {
|
||||
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
children_state: [],
|
||||
guest_can_join: false,
|
||||
num_joined_members: 2
|
||||
}
|
||||
yield {
|
||||
room_id: "!zTMspHVUBhFLLSdmnS:cadence.moe",
|
||||
children_state: [{
|
||||
type: "m.space.child",
|
||||
state_key: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
sender: "@elliu:hashi.re",
|
||||
content: {
|
||||
via: ["cadence.moe", "hashi.re"]
|
||||
},
|
||||
origin_server_ts: 0
|
||||
}],
|
||||
guest_can_join: false,
|
||||
num_joined_members: 2
|
||||
}
|
||||
|
@ -414,7 +462,7 @@ test("web link room: check that bridge has PL 100 in target room (event missing)
|
|||
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
|
||||
yield {
|
||||
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
children_state: {},
|
||||
children_state: [],
|
||||
guest_can_join: false,
|
||||
num_joined_members: 2
|
||||
}
|
||||
|
@ -454,7 +502,7 @@ test("web link room: check that bridge has PL 100 in target room (users default)
|
|||
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
|
||||
yield {
|
||||
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
children_state: {},
|
||||
children_state: [],
|
||||
guest_can_join: false,
|
||||
num_joined_members: 2
|
||||
}
|
||||
|
@ -494,7 +542,7 @@ test("web link room: successfully calls createRoom", async t => {
|
|||
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
|
||||
yield {
|
||||
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
|
||||
children_state: {},
|
||||
children_state: [],
|
||||
guest_can_join: false,
|
||||
num_joined_members: 2
|
||||
}
|
||||
|
|
|
@ -17,6 +17,8 @@ const {reg} = require("../src/matrix/read-registration")
|
|||
reg.ooye.discord_token = "Njg0MjgwMTkyNTUzODQ0NzQ3.Xl3zlw.baby"
|
||||
reg.ooye.server_origin = "https://matrix.cadence.moe" // so that tests will pass even when hard-coded
|
||||
reg.ooye.server_name = "cadence.moe"
|
||||
reg.ooye.namespace_prefix = "_ooye_"
|
||||
reg.sender_localpart = "_ooye_bot"
|
||||
reg.id = "baby"
|
||||
reg.as_token = "don't actually take authenticated actions on the server"
|
||||
reg.hs_token = "don't actually take authenticated actions on the server"
|
||||
|
|
I fixed
path
so that passing arrays (likevia
) to it should work correctly. :)