cadence/out-of-your-element
cadence/out-of-your-element
3
7
Fork 5
Code Issues 19 Pull requests 3 Releases 25 Activity

feat/add-unlink-space-button #63

Manually merged
cadence merged 12 commits from Elliu/out-of-your-element:feat/add-unlink-space-button into main 2026-02-13 06:32:34 +00:00
Conversation 8 Commits 12 Files changed 5 +25 -31
1 changed files with 25 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit b45eeb150e - Show all commits

56
src/web/routes/link.js
View file

@ -41,33 +41,6 @@ function getCreateSpace(event) {
return event.context.createSpace || sync.require("../../d2m/actions/create-space")
}
/**
* @param {H3Event} event
* @param {string} guild_id
*/
async function validateUserHaveRightsOnGuild(event, guild_id) {
const managed = await auth.getManagedGuilds(event)
if (!managed.has(guild_id))
throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
}
/**
* @param {H3Event} event
* @param {string} guild_id
* @returns {Promise<DiscordTypes.APIGuild & {members: DiscordTypes.APIGuildMember[]}>}
*/
async function validateGuildAccess(event, guild_id) {
// Check guild ID or nonce
await validateUserHaveRightsOnGuild(event, guild_id)
// Check guild exists
const guild = discord.guilds.get(guild_id)
if (!guild)
throw createError({status: 400, message: "Bad Request", data: "Discord guild does not exist or bot has not joined it"})
return guild
}
/**
* @param {H3Event} event
* @param {string} channel_id
@ -117,11 +90,12 @@ const schema = {
as.router.post("/api/link-space", defineEventHandler(async event => {
const parsedBody = await readValidatedBody(event, schema.linkSpace.parse)
const session = await auth.useSession(event)
const managed = await auth.getManagedGuilds(event)
const api = getAPI(event)
// Check guild ID
cadence marked this conversation as resolved Outdated
cadence commented 2025-11-02 09:43:07 +00:00
Outdated
Copy link

You deleted the thing that checks that the user has Manage Server permissions and you didn't put it back.

Not sure how this wasn't caught by tests. Maybe I need to add more tests.

I think I preferred how this looked before, where the checks were done inline, rather than how it looks now, where you've extracted each check to a function. Sure, there was some repetition of code before, but I think it was easier to read because you can just read down the file and see everything, rather than having to jump around looking in to each function.

If you keep the checked as extracted functions, I would like to see more descriptive function names + descriptions of what happens in the /** */ comments on the function so they can be hovered to see their intention. The current names are not so good:

  • validateUserHaveRightsOnGuild - what kind of validation outcome + what kind of user + what kind of rights? try ensureDiscordUserHasManageServer
  • validateGuildAccess - what kind of validation outcome + what kind of access + whose access? try coming up with a better name
You deleted the thing that checks that the user has Manage Server permissions and you didn't put it back. Not sure how this wasn't caught by tests. Maybe I need to add more tests. I think I preferred how this looked before, where the checks were done inline, rather than how it looks now, where you've extracted each check to a function. Sure, there was some repetition of code before, but I think it was easier to read because you can just read down the file and see everything, rather than having to jump around looking in to each function. If you keep the checked as extracted functions, I would like to see more descriptive function names + descriptions of what happens in the `/** */` comments on the function so they can be hovered to see their intention. The current names are not so good: - `validateUserHaveRightsOnGuild` - what kind of validation outcome + what kind of user + what kind of rights? try `ensureDiscordUserHasManageServer` - `validateGuildAccess` - what kind of validation outcome + what kind of access + whose access? try coming up with a better name
Elliu commented 2025-11-04 15:22:39 +00:00
Outdated
Copy link

About the quoted code, I'm not sure why it doesn't appear on the quote, but I replaced it with a

	const guild = await validateGuildAccess(event, guildID)

so the error was still being detected I think

Anyway, I reverted it back to inline checks so it should be ok now

About the quoted code, I'm not sure why it doesn't appear on the quote, but I replaced it with a ```js const guild = await validateGuildAccess(event, guildID) ``` so the error was still being detected I think Anyway, I reverted it back to inline checks so it should be ok now
const guildID = parsedBody.guild_id
await validateUserHaveRightsOnGuild(event, guildID)
if (!managed.has(guildID)) throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
// Check space ID
if (!session.data.mxid) throw createError({status: 403, message: "Forbidden", data: "Can't link with your Matrix space if you aren't logged in to Matrix"})
@ -169,12 +143,18 @@ as.router.post("/api/link-space", defineEventHandler(async event => {
as.router.post("/api/link", defineEventHandler(async event => {
const parsedBody = await readValidatedBody(event, schema.link.parse)
const managed = await auth.getManagedGuilds(event)
const api = getAPI(event)
const createRoom = getCreateRoom(event)
const createSpace = getCreateSpace(event)
// Check guild ID or nonce
const guildID = parsedBody.guild_id
const guild = await validateGuildAccess(event, guildID)
if (!managed.has(guildID)) throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
// Check guild is bridged
const guild = discord.guilds.get(guildID)
if (!guild) throw createError({status: 400, message: "Bad Request", data: "Discord guild does not exist or bot has not joined it"})
const spaceID = await createSpace.ensureSpace(guild)
// Check channel exists
@ -252,7 +232,14 @@ as.router.post("/api/link", defineEventHandler(async event => {
as.router.post("/api/unlink", defineEventHandler(async event => {
const {channel_id, guild_id} = await readValidatedBody(event, schema.unlink.parse)
await validateGuildAccess(event, guild_id)
const managed = await auth.getManagedGuilds(event)
// Check guild ID or nonce
if (!managed.has(guild_id)) throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
// Check guild exists
const guild = discord.guilds.get(guild_id)
if (!guild) throw createError({status: 400, message: "Bad Request", data: "Discord guild does not exist or bot has not joined it"})
await doRoomUnlink(event, channel_id, guild_id)
@ -262,8 +249,15 @@ as.router.post("/api/unlink", defineEventHandler(async event => {
as.router.post("/api/unlink-space", defineEventHandler(async event => {
const {guild_id} = await readValidatedBody(event, schema.unlinkSpace.parse)
const managed = await auth.getManagedGuilds(event)
const api = getAPI(event)
await validateGuildAccess(event, guild_id)
// Check guild ID or nonce
if (!managed.has(guild_id)) throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
// Check guild exists
const guild = discord.guilds.get(guild_id)
if (!guild) throw createError({status: 400, message: "Bad Request", data: "Discord guild does not exist or bot has not joined it"})
const spaceID = select("guild_space", "space_id", {guild_id: guild_id}).pluck().get()
if (!spaceID) {