Skip to content

Commit

Permalink
Fix Discord event link hanging issue
Browse files Browse the repository at this point in the history
When a sync has been performed, the old implementation causes a release of the database PoolClient, causing any future database operation to fail.
This is due to the event sync code being triggered from the context of GraphQL queries instead of a separate context when an incoming event from Discord is processed.

The code has now been rewritten to take the context.pgClient database client which is the current GraphQL pgClient.
This client is used in all database operations and not released so it can be used in the future.
This will stop the hanging of the server when a Discord event link sync is done.

This issue was only reliable reproducible with quite some members interested in an event. I tested this with 13 people interested.

In the future we should rewrite this code to always take a pgClient in the database code and create a new pgClient at a higher level when not available and otherwise always use the context.pgClient.
At lower levels we should not be caring about releasing or not because this can only cause issues.
  • Loading branch information
JJ-8 committed Apr 25, 2024
1 parent 5d0aac6 commit 1e19acf
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 36 deletions.
6 changes: 4 additions & 2 deletions api/src/discord/commands/linkUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ import {
import { CTF, getAccessibleCTFsForUser } from "../database/ctfs";
import { getDiscordClient } from "..";
import config from "../../config";
import { PoolClient } from "pg";

export async function changeDiscordUserRoleForCTF(
userId: bigint,
ctf: CTF | string | string[],
operation: "add" | "remove"
operation: "add" | "remove",
pgClient: PoolClient | null = null
): Promise<boolean> {
const discordUserId = await getDiscordIdFromUserId(userId);
const discordUserId = await getDiscordIdFromUserId(userId, pgClient);
if (discordUserId == null) return false;

return changeDiscordUserRoleForCTFByDiscordId(discordUserId, ctf, operation);
Expand Down
35 changes: 23 additions & 12 deletions api/src/discord/database/ctfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface CTF {
secrets_id: bigint;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function buildCtf(row: any): CTF {
return {
id: row.id as bigint,
Expand Down Expand Up @@ -68,9 +69,11 @@ export async function getAllCtfsFromDatabase(): Promise<string[]> {

// get id from ctf name
export async function getCtfFromDatabase(
ctfName: string | bigint
ctfName: string | bigint,
pgClient: PoolClient | null = null
): Promise<CTF | null> {
const pgClient = await connectToDatabase();
const useRequestClient = pgClient != null;
if (pgClient == null) pgClient = await connectToDatabase();

try {
//make a query to get all the challenges from a ctf
Expand All @@ -94,7 +97,7 @@ export async function getCtfFromDatabase(
console.error("Failed to get CTF from the database:", error);
return null;
} finally {
pgClient.release();
if (!useRequestClient) pgClient.release();
}
}

Expand Down Expand Up @@ -163,9 +166,11 @@ export async function getAccessibleCTFsForUser(
// invite the user to play the CTF, but only if they don't have access yet
export async function insertInvitation(
ctfId: bigint,
profileId: bigint
profileId: bigint,
pgClient: PoolClient | null = null
): Promise<void> {
const pgClient = await connectToDatabase();
const useRequestClient = pgClient != null;
if (pgClient == null) pgClient = await connectToDatabase();

const accessibleCTFs = await getAccessibleCTFsForUser(profileId, pgClient);
if (accessibleCTFs.find((ctf) => ctf.id === ctfId) != null) {
Expand All @@ -182,12 +187,16 @@ export async function insertInvitation(
console.error("Failed to insert invitation in the database:", error);
return;
} finally {
pgClient.release();
if (!useRequestClient) pgClient.release();
}
}

export async function getInvitedUsersByCtf(ctfId: bigint): Promise<bigint[]> {
const pgClient = await connectToDatabase();
export async function getInvitedUsersByCtf(
ctfId: bigint,
pgClient: PoolClient | null = null
): Promise<bigint[]> {
const useRequestClient = pgClient != null;
if (pgClient == null) pgClient = await connectToDatabase();

try {
const query = `SELECT profile_id FROM ctfnote.invitation WHERE ctf_id = $1`;
Expand All @@ -199,15 +208,17 @@ export async function getInvitedUsersByCtf(ctfId: bigint): Promise<bigint[]> {
console.error("Failed to get invited users from the database:", error);
return [];
} finally {
pgClient.release();
if (!useRequestClient) pgClient.release();
}
}

export async function deleteInvitation(
ctfId: bigint,
profileId: bigint
profileId: bigint,
pgClient: PoolClient | null = null
): Promise<void> {
const pgClient = await connectToDatabase();
const useRequestClient = pgClient != null;
if (pgClient == null) pgClient = await connectToDatabase();

try {
const query = `DELETE FROM ctfnote.invitation WHERE ctf_id = $1 AND profile_id = $2`;
Expand All @@ -217,6 +228,6 @@ export async function deleteInvitation(
console.error("Failed to delete invitation from the database:", error);
return;
} finally {
pgClient.release();
if (!useRequestClient) pgClient.release();
}
}
17 changes: 11 additions & 6 deletions api/src/discord/database/users.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { connectToDatabase } from "./database";
import { PoolClient } from "pg";

/*
* Only returns users that have not linked their discord account yet.
Expand Down Expand Up @@ -45,9 +46,11 @@ export async function setDiscordIdForUser(
}

export async function getUserByDiscordId(
discordId: string
discordId: string,
pgClient: PoolClient | null = null
): Promise<bigint | null> {
const pgClient = await connectToDatabase();
const useRequestClient = pgClient != null;
if (pgClient == null) pgClient = await connectToDatabase();

try {
const query =
Expand All @@ -59,14 +62,16 @@ export async function getUserByDiscordId(
} catch (error) {
return null;
} finally {
pgClient.release();
if (!useRequestClient) pgClient.release();
}
}

export async function getDiscordIdFromUserId(
userId: bigint
userId: bigint,
pgClient: PoolClient | null = null
): Promise<string | null> {
const pgClient = await connectToDatabase();
const useRequestClient = pgClient != null;
if (pgClient == null) pgClient = await connectToDatabase();
try {
const query = "SELECT discord_id FROM ctfnote.profile WHERE id = $1";
const values = [userId];
Expand All @@ -76,7 +81,7 @@ export async function getDiscordIdFromUserId(
} catch (error) {
return null;
} finally {
pgClient.release();
if (!useRequestClient) pgClient.release();
}
}

Expand Down
41 changes: 26 additions & 15 deletions api/src/discord/utils/permissionSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,28 @@ import {
} from "../database/ctfs";
import { getDiscordIdFromUserId, getUserByDiscordId } from "../database/users";
import { changeDiscordUserRoleForCTF } from "../commands/linkUser";
import { PoolClient } from "pg";

export async function syncDiscordPermissionsWithCtf(
guild: Guild,
ctfId: bigint,
discordLink: string | null
discordLink: string | null,
pgClient: PoolClient
) {
if (discordLink == null || discordLink.length == 0) return;
const ctf = await getCtfFromDatabase(ctfId);
if (ctf == null) return;
if (discordLink == null || discordLink.length == 0) {
return;
}
const ctf = await getCtfFromDatabase(ctfId, pgClient);
if (ctf == null) {
return;
}

const guildEvents = await guild.scheduledEvents.fetch();

const eventId = discordLink.match(/event=(\d+)/)?.[1];
if (eventId == null) return;
if (eventId == null) {
return;
}

const event = guildEvents.get(eventId);
if (event == null) return;
Expand All @@ -31,13 +39,13 @@ export async function syncDiscordPermissionsWithCtf(
const usersInterested = (
await Promise.all(
discordUsersInterested.map(async function (user) {
return await getUserByDiscordId(user.user.id);
return await getUserByDiscordId(user.user.id, pgClient);
})
)
).filter((user) => user != null) as Array<bigint>;

// search for users that are invited to the CTF but are not interested in the event
const invitedUsers = await getInvitedUsersByCtf(ctfId);
const invitedUsers = await getInvitedUsersByCtf(ctfId, pgClient);
const usersNotInterested = invitedUsers.filter(
(user) => !usersInterested.includes(user)
);
Expand All @@ -46,25 +54,28 @@ export async function syncDiscordPermissionsWithCtf(
await Promise.all(
usersNotInterested.map(async function (user) {
if (user == null) return;
if ((await getDiscordIdFromUserId(user)) == null) return; // don't remove permissions through the Discord sync if the user doesn't have Discord linked
await deleteInvitation(ctfId, user);
if ((await getDiscordIdFromUserId(user, pgClient)) == null) return; // don't remove permissions through the Discord sync if the user doesn't have Discord linked
await deleteInvitation(ctfId, user, pgClient);

// we just removed the invitation so we expect the user to not have access to the CTF anymore,
// however if the user has member privileges or higher we do not remove the role because the user should still have access
const accessibleCTFs = await getAccessibleCTFsForUser(user);
if (accessibleCTFs.find((ctf) => ctf.id === ctfId) != null) return;
const accessibleCTFs = await getAccessibleCTFsForUser(user, pgClient);
if (accessibleCTFs.find((ctf) => ctf.id === ctfId) != null) {
return;
}

changeDiscordUserRoleForCTF(user, ctf, "remove");
changeDiscordUserRoleForCTF(user, ctf, "remove", pgClient);
})
);

// invite the users that are interested in the event but do not have access yet to the CTF
await Promise.all(
return await Promise.all(
usersInterested.map(async function (user) {
if (user == null) return;
insertInvitation(ctfId, user);

insertInvitation(ctfId, user, pgClient);
// we only add the role if the user also exists in CTFNote and therefore prevent that users only have a Discord account but no CTFNote account
changeDiscordUserRoleForCTF(user, ctf, "add");
changeDiscordUserRoleForCTF(user, ctf, "add", pgClient);
})
);
}
17 changes: 16 additions & 1 deletion api/src/plugins/discordHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export async function handleTaskSolved(
);
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any
const discordMutationHook = (_build: Build) => (fieldContext: Context<any>) => {
const {
scope: { isRootMutation },
Expand Down Expand Up @@ -99,9 +100,13 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context<any>) => {
}

const handleDiscordMutationAfter = async (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
input: any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
args: any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
context: any,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
_resolveInfo: GraphQLResolveInfoWithMessages
) => {
const guild = getDiscordGuild();
Expand Down Expand Up @@ -264,7 +269,12 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context<any>) => {
const link = args.input.link;
const ctfId = args.input.ctfId;

await syncDiscordPermissionsWithCtf(guild, ctfId, link).catch((err) => {
await syncDiscordPermissionsWithCtf(
guild,
ctfId,
link,
context.pgClient
).catch((err) => {
console.error("Failed to sync discord permissions.", err);
});
}
Expand All @@ -273,9 +283,13 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context<any>) => {
};

const handleDiscordMutationBefore = async (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
input: any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
args: any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
context: any,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
_resolveInfo: GraphQLResolveInfoWithMessages
) => {
const guild = getDiscordGuild();
Expand Down Expand Up @@ -404,6 +418,7 @@ async function handeInvitation(
await changeDiscordUserRoleForCTF(profileId, ctf, operation);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
async function handleUpdateCtf(args: any, guild: Guild) {
const ctf = await getCtfFromDatabase(args.input.id);
if (ctf == null) return;
Expand Down

0 comments on commit 1e19acf

Please sign in to comment.