From 96f5217e6e7ac5b24d294a601dfcb9fed69e3a36 Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 11:16:30 +0200 Subject: [PATCH 01/18] Create migration for account invitation with Discord link --- .../55-discord-account-invitation-link.sql | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 api/migrations/55-discord-account-invitation-link.sql diff --git a/api/migrations/55-discord-account-invitation-link.sql b/api/migrations/55-discord-account-invitation-link.sql new file mode 100644 index 00000000..46f7e6ad --- /dev/null +++ b/api/migrations/55-discord-account-invitation-link.sql @@ -0,0 +1,101 @@ +ALTER TABLE ctfnote_private.invitation_link + ADD COLUMN "discord_id" TEXT UNIQUE DEFAULT NULL; + +DROP FUNCTION ctfnote.create_invitation_link ("role" ctfnote.role); +CREATE OR REPLACE FUNCTION ctfnote.create_invitation_link ("role" ctfnote.role, "discord_id" text default null) + RETURNS ctfnote.invitation_link_response + AS $$ +DECLARE + invitation_link ctfnote_private.invitation_link; +BEGIN + INSERT INTO ctfnote_private.invitation_link ("role", "token", "discord_id") + VALUES (create_invitation_link.role, gen_random_uuid (), create_invitation_link.discord_id) + RETURNING + * INTO invitation_link; + RETURN ROW (invitation_link.token::text)::ctfnote.invitation_link_response; +END; +$$ +LANGUAGE plpgsql +SECURITY DEFINER; + +GRANT EXECUTE ON FUNCTION ctfnote.create_invitation_link (ctfnote.role, text) TO user_admin; + +CREATE OR REPLACE FUNCTION ctfnote.register_with_token ("token" text, "login" text, "password" text) + RETURNS ctfnote.jwt + AS $$ +DECLARE + invitation_role ctfnote.role; + invitation_discord_id text; +BEGIN + SELECT + ROLE, discord_id INTO invitation_role, invitation_discord_id + FROM + ctfnote_private.invitation_link + WHERE + invitation_link.token::text = register_with_token.token + AND expiration > now(); + IF invitation_role IS NOT NULL THEN + DELETE FROM ctfnote_private.invitation_link + WHERE invitation_link.token::text = register_with_token.token; + IF invitation_discord_id IS NOT NULL THEN + RETURN ctfnote_private.do_register (register_with_token.login, register_with_token.password, invitation_role, invitation_discord_id); + ELSE + RETURN ctfnote_private.do_register (register_with_token.login, register_with_token.password, invitation_role); + END IF; + ELSE + RAISE EXCEPTION 'Invalid token'; + END IF; +END +$$ +LANGUAGE plpgsql +SECURITY DEFINER; + +GRANT EXECUTE ON FUNCTION ctfnote.register_with_token (text, text, text) TO user_anonymous; + +DROP FUNCTION ctfnote_private.do_register ("login" text, "password" text, "role" ctfnote.role); + +CREATE OR REPLACE FUNCTION ctfnote_private.do_register ("login" text, "password" text, "role" ctfnote.role) + RETURNS ctfnote.jwt + AS $$ +DECLARE + new_user ctfnote_private.user; +BEGIN + INSERT INTO ctfnote_private.user ("login", "password", "role") + VALUES (do_register.login, crypt(do_register.password, gen_salt('bf')), do_register.role) + RETURNING + * INTO new_user; + INSERT INTO ctfnote.profile ("id", "username") + VALUES (new_user.id, do_register.login); + RETURN (ctfnote_private.new_token (new_user.id))::ctfnote.jwt; +EXCEPTION + WHEN unique_violation THEN + RAISE EXCEPTION 'Username already taken'; +END; +$$ +LANGUAGE plpgsql +STRICT +SECURITY DEFINER; + + +CREATE OR REPLACE FUNCTION ctfnote_private.do_register ("login" text, "password" text, "role" ctfnote.role, "discord_id" text) + RETURNS ctfnote.jwt + AS $$ +DECLARE + new_user ctfnote_private.user; +BEGIN + INSERT INTO ctfnote_private.user ("login", "password", "role") + VALUES (do_register.login, crypt(do_register.password, gen_salt('bf')), do_register.role) + RETURNING + * INTO new_user; + INSERT INTO ctfnote.profile ("id", "username", "discord_id") + VALUES (new_user.id, do_register.login, do_register.discord_id); + RETURN (ctfnote_private.new_token (new_user.id))::ctfnote.jwt; +EXCEPTION + WHEN unique_violation THEN + RAISE EXCEPTION 'Username already taken'; +END; +$$ +LANGUAGE plpgsql +STRICT +SECURITY DEFINER; + From 3011f16f9cc59667ec7a7781b5420ac4a963450b Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 16:02:13 +0200 Subject: [PATCH 02/18] Removes the unique constraint from discord_id in `ctfnote_private.invitation_link`. It is preferred we unique this column however whenever you accidentally dismiss the discord bot and want to make a new token you cant. This is why we removed the unique constraint for now. --- api/migrations/55-discord-account-invitation-link.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/migrations/55-discord-account-invitation-link.sql b/api/migrations/55-discord-account-invitation-link.sql index 46f7e6ad..29d28ab5 100644 --- a/api/migrations/55-discord-account-invitation-link.sql +++ b/api/migrations/55-discord-account-invitation-link.sql @@ -1,5 +1,5 @@ ALTER TABLE ctfnote_private.invitation_link - ADD COLUMN "discord_id" TEXT UNIQUE DEFAULT NULL; + ADD COLUMN "discord_id" TEXT DEFAULT NULL; DROP FUNCTION ctfnote.create_invitation_link ("role" ctfnote.role); CREATE OR REPLACE FUNCTION ctfnote.create_invitation_link ("role" ctfnote.role, "discord_id" text default null) From 00f3b475476b2375305b4c2f727b79070179eb5c Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 16:04:37 +0200 Subject: [PATCH 03/18] Update the GraphQL schema Graphql schema updated, hopefully this is right because windows/wsl version of `yarn run codegen` likes to break the schema sometimes. --- front/graphql.schema.json | 12 ++++++++++++ front/src/generated/graphql.ts | 1 + 2 files changed, 13 insertions(+) diff --git a/front/graphql.schema.json b/front/graphql.schema.json index dd9db357..5b22861a 100644 --- a/front/graphql.schema.json +++ b/front/graphql.schema.json @@ -1297,6 +1297,18 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "discordId", + "description": null, + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "role", "description": null, diff --git a/front/src/generated/graphql.ts b/front/src/generated/graphql.ts index 8f374bea..4cf56778 100644 --- a/front/src/generated/graphql.ts +++ b/front/src/generated/graphql.ts @@ -300,6 +300,7 @@ export type CreateInvitationLinkInput = { * payload verbatim. May be used to track mutations by the client. */ clientMutationId?: InputMaybe; + discordId?: InputMaybe; role?: InputMaybe; }; From 8b22bd22f1585e0ac0304d4fefbaaf803792ae74 Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 16:24:33 +0200 Subject: [PATCH 04/18] Add base implementation of register command. This commit implements the base of some utils that are required and the base of the command itself. --- api/src/discord/commands.ts | 2 + api/src/discord/commands/register.ts | 76 ++++++++++++++++++++++++++++ api/src/discord/database/users.ts | 30 +++++++++++ 3 files changed, 108 insertions(+) create mode 100644 api/src/discord/commands/register.ts diff --git a/api/src/discord/commands.ts b/api/src/discord/commands.ts index 7e729ac6..18c9705b 100644 --- a/api/src/discord/commands.ts +++ b/api/src/discord/commands.ts @@ -5,6 +5,7 @@ import { SolveTask } from "./commands/solveTask"; import { LinkUser } from "./commands/linkUser"; import { StartWorking, StopWorking } from "./commands/workingOn"; import { DeleteCtf } from "./commands/deleteCtf"; +import { Register } from "./commands/register"; export const Commands: Command[] = [ ArchiveCtf, @@ -14,4 +15,5 @@ export const Commands: Command[] = [ StartWorking, StopWorking, DeleteCtf, + Register, ]; diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts new file mode 100644 index 00000000..7fbeb79a --- /dev/null +++ b/api/src/discord/commands/register.ts @@ -0,0 +1,76 @@ +import { ApplicationCommandType, Client, CommandInteraction } from "discord.js"; +import { Command } from "../command"; +import { + createInvitationURLForDiscordId, + getUserByDiscordId, +} from "../database/users"; +import config from "../../config"; + +async function getInvitationUrl(invitation_code: string | null = null) { + if (config.pad.domain == "") return ""; + + if (invitation_code == null) return ""; + + const ssl = config.pad.useSSL == "false" ? "" : "s"; + + return `http${ssl}://${config.pad.domain}/#/auth/register/${invitation_code}`; +} + +async function createAccountLogic( + client: Client, + interaction: CommandInteraction +) { + // TODO: check if user has role + // TODO: check if feature is enabled in environment variables + + const userId = await getUserByDiscordId(interaction.user.id); + if (userId != null) { + await interaction.editReply({ + content: + "You can't link the same Discord account twice! If you do not have a CTFNote account or haven't linked it, contact an administrator.", + }); + return; + } + + await interaction.editReply({ + content: + "Generating private invitation url... If you already have a CTFNote account you should link it using the /link command instead.", + }); + + const invitation_code = await createInvitationURLForDiscordId( + "user_friend", // TODO: use environment variable + interaction.user.id + ); + + if (invitation_code == null) { + await interaction.editReply({ + content: "Something went wrong.", // TODO: Meaningful error messages? + }); + return; + } + + const invitation_url = await getInvitationUrl(invitation_code); + if (invitation_url == null) { + await interaction.editReply({ + content: "Something went wrong.", // TODO: Meaningful error messages? + }); + return; + } + + await interaction.editReply({ + content: `Your personal invitation url: ${invitation_url}. If you already have a CTFNote account you should link it using the /link command instead.`, + }); + return; +} + +export const Register: Command = { + name: "register", + description: + "Create an account on the CTFNote platform if you are allowed to!", + type: ApplicationCommandType.ChatInput, + run: async (client, interaction) => { + return createAccountLogic(client, interaction).catch((e) => { + console.error("Error during register logic: ", e); + }); + }, +}; diff --git a/api/src/discord/database/users.ts b/api/src/discord/database/users.ts index ecca8306..8b4a79ad 100644 --- a/api/src/discord/database/users.ts +++ b/api/src/discord/database/users.ts @@ -45,6 +45,36 @@ export async function setDiscordIdForUser( } } +type AllowedRoles = + | "user_guest" + | "user_friend" + | "user_member" + | "user_manager" + | "user_admin"; + +export async function createInvitationURLForDiscordId( + role: AllowedRoles, + discordId: string, + pgClient: PoolClient | null = null +): Promise { + // TODO: Verify if valid role is passed. + + const useRequestClient = pgClient != null; + if (pgClient == null) pgClient = await connectToDatabase(); + + try { + const query = "SELECT token FROM ctfnote.create_invitation_link($1, $2)"; + const values = [role, discordId]; + const queryResult = await pgClient.query(query, values); + + return queryResult.rows[0].token as string; + } catch (error) { + return null; + } finally { + if (!useRequestClient) pgClient.release(); + } +} + export async function getUserByDiscordId( discordId: string, pgClient: PoolClient | null = null From 6d44414182bd84c039d401dabd247485869b4b50 Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 19:47:14 +0200 Subject: [PATCH 05/18] Add checks to make sure user only has one token. This commit implements fixes for the issues talked about in comment: https://github.com/TFNS/CTFNote/pull/301#issuecomment-2250729493 Also changed the migration back to unique --- .../55-discord-account-invitation-link.sql | 2 +- api/src/discord/commands/register.ts | 18 +++++++++++++++ api/src/discord/database/users.ts | 23 +++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/api/migrations/55-discord-account-invitation-link.sql b/api/migrations/55-discord-account-invitation-link.sql index 29d28ab5..46f7e6ad 100644 --- a/api/migrations/55-discord-account-invitation-link.sql +++ b/api/migrations/55-discord-account-invitation-link.sql @@ -1,5 +1,5 @@ ALTER TABLE ctfnote_private.invitation_link - ADD COLUMN "discord_id" TEXT DEFAULT NULL; + ADD COLUMN "discord_id" TEXT UNIQUE DEFAULT NULL; DROP FUNCTION ctfnote.create_invitation_link ("role" ctfnote.role); CREATE OR REPLACE FUNCTION ctfnote.create_invitation_link ("role" ctfnote.role, "discord_id" text default null) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index 7fbeb79a..ad905b8f 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -32,6 +32,24 @@ async function createAccountLogic( return; } + const existing_invitation_code = await getInvitationTokenForDiscordId( + interaction.user.id + ); + if (existing_invitation_code != null) { + const invitation_url = await getInvitationUrl(existing_invitation_code); + if (invitation_url == "") { + await interaction.editReply({ + content: "Something went wrong.", // TODO: Meaningful error messages? + }); + return; + } + + await interaction.editReply({ + content: `Your personal invitation url: ${invitation_url}. If you already have a CTFNote account you should link it using the /link command instead.`, + }); + return; + } + await interaction.editReply({ content: "Generating private invitation url... If you already have a CTFNote account you should link it using the /link command instead.", diff --git a/api/src/discord/database/users.ts b/api/src/discord/database/users.ts index 8b4a79ad..dcf384ea 100644 --- a/api/src/discord/database/users.ts +++ b/api/src/discord/database/users.ts @@ -52,8 +52,27 @@ type AllowedRoles = | "user_manager" | "user_admin"; -export async function createInvitationURLForDiscordId( - role: AllowedRoles, +export async function getInvitationTokenForDiscordId( + discordId: string, + pgClient: PoolClient | null = null +): Promise { + const useRequestClient = pgClient != null; + if (pgClient == null) pgClient = await connectToDatabase(); + + try { + const query = + "SELECT token FROM ctfnote_private.invitation_link WHERE discord_id = $1"; + const values = [discordId]; + const queryResult = await pgClient.query(query, values); + + return queryResult.rows[0].token as string; + } catch (error) { + return null; + } finally { + if (!useRequestClient) pgClient.release(); + } +} + discordId: string, pgClient: PoolClient | null = null ): Promise { From 59f64ff56e1fd2c1cea41d9a701b1bfc1434336a Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 19:55:25 +0200 Subject: [PATCH 06/18] Add ENV vars for configuring register command. Adds the Environment variables that are needed to configure the register command and some explanation for them. --- .env.example | 10 ++++++++++ api/src/config.ts | 11 ++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 5a977bb2..59d90f07 100644 --- a/.env.example +++ b/.env.example @@ -18,6 +18,16 @@ # DISCORD_VOICE_CHANNELS=3 # DISCORD_BOT_NAME=CTFNote +# Enable this if you want users to be able to make accounts through the /register command in discord +# DISCORD_REGISTRATION_ENABLED=false + +# Which role the user should be granted on the ctfnote when creating a account through the bot +# DISCORD_REGISTRATION_CTFNOTE_ROLE=user_guest + +# If you want the bot to verify if a user has a specific role in the discord before allowing them to make a account through +# the Discord command, set the ID of the role below, else leave this field empty. +#DISCORD_REGISTRATION_ROLE_ID=discord_id + # Configure timezone and locale # TZ=Europe/Paris # LC_ALL=en_US.UTF-8 diff --git a/api/src/config.ts b/api/src/config.ts index 4076db41..353a6906 100644 --- a/api/src/config.ts +++ b/api/src/config.ts @@ -42,6 +42,9 @@ export type CTFNoteConfig = DeepReadOnly<{ voiceChannels: number; botName: string; maxChannelsPerCategory: number; + registrationEnabled: string; + registrationAccountRole: string; + registrationRoleId: string; }; }>; @@ -92,7 +95,13 @@ const config: CTFNoteConfig = { serverId: getEnv("DISCORD_SERVER_ID"), voiceChannels: getEnvInt("DISCORD_VOICE_CHANNELS"), botName: getEnv("DISCORD_BOT_NAME", "CTFNote"), - maxChannelsPerCategory: 50, // 50 is the hard Discord limit + maxChannelsPerCategory: 50, //! 50 is the hard Discord limit + registrationEnabled: getEnv("DISCORD_REGISTRATION_ENABLED", "false"), + registrationAccountRole: getEnv( + "DISCORD_REGISTRATION_CTFNOTE_ROLE", + "user_guest" + ), + registrationRoleId: getEnv("DISCORD_REGISTRATION_ROLE_ID", ""), }, }; From 841941d21cad7eff944965ae1890eda0f21860c7 Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Thu, 25 Jul 2024 20:23:22 +0200 Subject: [PATCH 07/18] Adds checks to register command and some cleanup Some general cleanup and fixes of todo's. Also implemented the required checks and used data from the environment variables. This command implements what showcased in issue message: https://github.com/TFNS/CTFNote/pull/301#issuecomment-2251140196 --- api/src/discord/commands/register.ts | 48 +++++++++++++++++++++++----- api/src/discord/database/users.ts | 4 ++- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index ad905b8f..b35cd176 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -1,14 +1,19 @@ -import { ApplicationCommandType, Client, CommandInteraction } from "discord.js"; +import { + ApplicationCommandType, + Client, + CommandInteraction, + GuildMemberRoleManager, +} from "discord.js"; import { Command } from "../command"; import { - createInvitationURLForDiscordId, + createInvitationTokenForDiscordId, + getInvitationTokenForDiscordId, getUserByDiscordId, } from "../database/users"; import config from "../../config"; async function getInvitationUrl(invitation_code: string | null = null) { if (config.pad.domain == "") return ""; - if (invitation_code == null) return ""; const ssl = config.pad.useSSL == "false" ? "" : "s"; @@ -16,12 +21,39 @@ async function getInvitationUrl(invitation_code: string | null = null) { return `http${ssl}://${config.pad.domain}/#/auth/register/${invitation_code}`; } +//? Refactor this to not have this type in two places. +type AllowedRoles = + | "user_guest" + | "user_friend" + | "user_member" + | "user_manager" + | "user_admin"; + async function createAccountLogic( client: Client, interaction: CommandInteraction ) { - // TODO: check if user has role - // TODO: check if feature is enabled in environment variables + if (config.discord.registrationEnabled.toLowerCase() === "false") { + await interaction.editReply({ + content: + "The functionality to create your own account this way has been disabled by an administrator", + }); + return; + } + + if (config.discord.registrationRoleId !== "") { + if ( + !(interaction.member?.roles as GuildMemberRoleManager).cache.has( + config.discord.registrationRoleId + ) + ) { + await interaction.editReply({ + content: + "You do not have the role required to create an account yourself.", + }); + return; + } + } const userId = await getUserByDiscordId(interaction.user.id); if (userId != null) { @@ -55,8 +87,8 @@ async function createAccountLogic( "Generating private invitation url... If you already have a CTFNote account you should link it using the /link command instead.", }); - const invitation_code = await createInvitationURLForDiscordId( - "user_friend", // TODO: use environment variable + const invitation_code = await createInvitationTokenForDiscordId( + (config.discord.registrationAccountRole as AllowedRoles) ?? "user_guest", interaction.user.id ); @@ -68,7 +100,7 @@ async function createAccountLogic( } const invitation_url = await getInvitationUrl(invitation_code); - if (invitation_url == null) { + if (invitation_url == "") { await interaction.editReply({ content: "Something went wrong.", // TODO: Meaningful error messages? }); diff --git a/api/src/discord/database/users.ts b/api/src/discord/database/users.ts index dcf384ea..548d92ac 100644 --- a/api/src/discord/database/users.ts +++ b/api/src/discord/database/users.ts @@ -73,10 +73,12 @@ export async function getInvitationTokenForDiscordId( } } +export async function createInvitationTokenForDiscordId( + role: AllowedRoles = "user_guest", discordId: string, pgClient: PoolClient | null = null ): Promise { - // TODO: Verify if valid role is passed. + role = (role as AllowedRoles) ?? "user_guest"; const useRequestClient = pgClient != null; if (pgClient == null) pgClient = await connectToDatabase(); From 7a272549297451466072f6e47a690dd1d03af1f3 Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 16:58:18 +0200 Subject: [PATCH 08/18] Add role sync after `/register` command With a setTimeout we can run code after the request lifecycle. This is ugly, but it works. See also the comment in the code about this. Solves the problem for https://github.com/TFNS/CTFNote/pull/301#issuecomment-2254222694 --- api/src/discord/database/users.ts | 20 +++++++++++++++++++ api/src/plugins/discordHooks.ts | 32 +++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/api/src/discord/database/users.ts b/api/src/discord/database/users.ts index 548d92ac..2e58778a 100644 --- a/api/src/discord/database/users.ts +++ b/api/src/discord/database/users.ts @@ -157,3 +157,23 @@ export async function getDiscordUsersThatCanPlayCTF( pgClient.release(); } } + +export async function getUserIdFromUsername( + username: string, + pgClient: PoolClient | null = null +): Promise { + const useRequestClient = pgClient != null; + if (pgClient == null) pgClient = await connectToDatabase(); + + try { + const query = "SELECT id FROM ctfnote.profile WHERE username = $1"; + const values = [username]; + const queryResult = await pgClient.query(query, values); + + return queryResult.rows[0].id as bigint; + } catch (error) { + return null; + } finally { + if (!useRequestClient) pgClient.release(); + } +} diff --git a/api/src/plugins/discordHooks.ts b/api/src/plugins/discordHooks.ts index f90309bc..35eab994 100644 --- a/api/src/plugins/discordHooks.ts +++ b/api/src/plugins/discordHooks.ts @@ -9,7 +9,10 @@ import { } from "../discord/database/ctfs"; import { getDiscordGuild, usingDiscordBot } from "../discord"; import { changeDiscordUserRoleForCTF } from "../discord/commands/linkUser"; -import { getDiscordIdFromUserId } from "../discord/database/users"; +import { + getDiscordIdFromUserId, + getUserIdFromUsername, +} from "../discord/database/users"; import { Task, getTaskByCtfIdAndNameFromDatabase, @@ -94,7 +97,8 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context) => { fieldContext.scope.fieldName !== "resetDiscordId" && fieldContext.scope.fieldName !== "deleteCtf" && fieldContext.scope.fieldName !== "updateUserRole" && - fieldContext.scope.fieldName !== "setDiscordEventLink" + fieldContext.scope.fieldName !== "setDiscordEventLink" && + fieldContext.scope.fieldName !== "registerWithToken" ) { return null; } @@ -314,6 +318,30 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context) => { }); } + /* + * We have a nice ductape solution for the following problem: + * During the handling of these hooks, the changes to the database are not committed yet. + * This means that we can't query the database for the new user id. + * We have to wait a bit to make sure the user is in the database. + * Alternatively we can hook the postgraphile lifecycle but that is not compatible with the current setup. + * The outgoing request is probably handling within 1 second, so this works fine. + */ + if (fieldContext.scope.fieldName === "registerWithToken") { + const username = args.input.login; // the login is equal to the username at registration + setTimeout(async () => { + const userId = await getUserIdFromUsername(username, null); // use null to get a new client which is privileged as the Discord bot + if (userId == null) return; + const ctfs = await getAccessibleCTFsForUser(userId, null); + for (let i = 0; i < ctfs.length; i++) { + await changeDiscordUserRoleForCTF(userId, ctfs[i], "add").catch( + (err) => { + console.error("Error while adding role to user: ", err); + } + ); + } + }, 2000); + } + return input; }; From 192cc380bd568930948819296505d57985cb8fe5 Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:06:25 +0200 Subject: [PATCH 09/18] Refactor AllowedRoles to enum of strings --- api/src/discord/commands/register.ts | 19 ++++++------------- api/src/discord/database/users.ts | 18 ++++++++++-------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index b35cd176..d0b86243 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -6,6 +6,7 @@ import { } from "discord.js"; import { Command } from "../command"; import { + AllowedRoles, createInvitationTokenForDiscordId, getInvitationTokenForDiscordId, getUserByDiscordId, @@ -21,14 +22,6 @@ async function getInvitationUrl(invitation_code: string | null = null) { return `http${ssl}://${config.pad.domain}/#/auth/register/${invitation_code}`; } -//? Refactor this to not have this type in two places. -type AllowedRoles = - | "user_guest" - | "user_friend" - | "user_member" - | "user_manager" - | "user_admin"; - async function createAccountLogic( client: Client, interaction: CommandInteraction @@ -36,7 +29,7 @@ async function createAccountLogic( if (config.discord.registrationEnabled.toLowerCase() === "false") { await interaction.editReply({ content: - "The functionality to create your own account this way has been disabled by an administrator", + "The functionality to create your own account this way has been disabled by an administrator.", }); return; } @@ -88,8 +81,9 @@ async function createAccountLogic( }); const invitation_code = await createInvitationTokenForDiscordId( - (config.discord.registrationAccountRole as AllowedRoles) ?? "user_guest", - interaction.user.id + interaction.user.id, + (config.discord.registrationAccountRole as AllowedRoles) ?? + AllowedRoles.user_guest ); if (invitation_code == null) { @@ -115,8 +109,7 @@ async function createAccountLogic( export const Register: Command = { name: "register", - description: - "Create an account on the CTFNote platform if you are allowed to!", + description: "Create an account on CTFNote if you are allowed to!", type: ApplicationCommandType.ChatInput, run: async (client, interaction) => { return createAccountLogic(client, interaction).catch((e) => { diff --git a/api/src/discord/database/users.ts b/api/src/discord/database/users.ts index 2e58778a..6a34997a 100644 --- a/api/src/discord/database/users.ts +++ b/api/src/discord/database/users.ts @@ -45,12 +45,14 @@ export async function setDiscordIdForUser( } } -type AllowedRoles = - | "user_guest" - | "user_friend" - | "user_member" - | "user_manager" - | "user_admin"; +// refactor above to an enum +export enum AllowedRoles { + user_guest = "user_guest", + user_friend = "user_friend", + user_member = "user_member", + user_manager = "user_manager", + user_admin = "user_admin", +} export async function getInvitationTokenForDiscordId( discordId: string, @@ -74,11 +76,11 @@ export async function getInvitationTokenForDiscordId( } export async function createInvitationTokenForDiscordId( - role: AllowedRoles = "user_guest", discordId: string, + role: AllowedRoles = AllowedRoles.user_guest, pgClient: PoolClient | null = null ): Promise { - role = (role as AllowedRoles) ?? "user_guest"; + role = (role as AllowedRoles) ?? AllowedRoles.user_guest; const useRequestClient = pgClient != null; if (pgClient == null) pgClient = await connectToDatabase(); From 950afddae8b97856ba14b4b2a79f67b18176ec5f Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:09:46 +0200 Subject: [PATCH 10/18] Rename variables and change signature of functions Variable renaming for consistency. The `getInvitationUrl` should return null in case of error instead of an empty string in order clarify error state. --- api/src/discord/commands/register.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index d0b86243..84e59c29 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -13,13 +13,13 @@ import { } from "../database/users"; import config from "../../config"; -async function getInvitationUrl(invitation_code: string | null = null) { - if (config.pad.domain == "") return ""; - if (invitation_code == null) return ""; +async function getInvitationUrl(invitationCode: string | null = null) { + if (config.pad.domain == "") return null; + if (invitationCode == null) return null; const ssl = config.pad.useSSL == "false" ? "" : "s"; - return `http${ssl}://${config.pad.domain}/#/auth/register/${invitation_code}`; + return `http${ssl}://${config.pad.domain}/#/auth/register/${invitationCode}`; } async function createAccountLogic( @@ -57,12 +57,12 @@ async function createAccountLogic( return; } - const existing_invitation_code = await getInvitationTokenForDiscordId( + const existingInvitationCode = await getInvitationTokenForDiscordId( interaction.user.id ); - if (existing_invitation_code != null) { - const invitation_url = await getInvitationUrl(existing_invitation_code); - if (invitation_url == "") { + if (existingInvitationCode != null) { + const invitationUrl = await getInvitationUrl(existingInvitationCode); + if (invitationUrl == null) { await interaction.editReply({ content: "Something went wrong.", // TODO: Meaningful error messages? }); @@ -70,7 +70,7 @@ async function createAccountLogic( } await interaction.editReply({ - content: `Your personal invitation url: ${invitation_url}. If you already have a CTFNote account you should link it using the /link command instead.`, + content: `Your personal invitation url: ${invitationUrl}. If you already have a CTFNote account you should link it using the /link command instead.`, }); return; } @@ -80,21 +80,21 @@ async function createAccountLogic( "Generating private invitation url... If you already have a CTFNote account you should link it using the /link command instead.", }); - const invitation_code = await createInvitationTokenForDiscordId( + const invitationCode = await createInvitationTokenForDiscordId( interaction.user.id, (config.discord.registrationAccountRole as AllowedRoles) ?? AllowedRoles.user_guest ); - if (invitation_code == null) { + if (invitationCode == null) { await interaction.editReply({ content: "Something went wrong.", // TODO: Meaningful error messages? }); return; } - const invitation_url = await getInvitationUrl(invitation_code); - if (invitation_url == "") { + const invitationUrl = await getInvitationUrl(invitationCode); + if (invitationUrl == null) { await interaction.editReply({ content: "Something went wrong.", // TODO: Meaningful error messages? }); @@ -102,7 +102,7 @@ async function createAccountLogic( } await interaction.editReply({ - content: `Your personal invitation url: ${invitation_url}. If you already have a CTFNote account you should link it using the /link command instead.`, + content: `Your personal invitation url: ${invitationUrl}. If you already have a CTFNote account you should link it using the /link command instead.`, }); return; } From 854dfcd4b9cb1f3f61857f26fbb1f7f11a22f20b Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:11:50 +0200 Subject: [PATCH 11/18] Move the registerWithToken role sync to the after mutation hook It makes more sense to have it there. It was by accident placed in the before hook previously. --- api/src/plugins/discordHooks.ts | 48 ++++++++++++++++----------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/api/src/plugins/discordHooks.ts b/api/src/plugins/discordHooks.ts index 35eab994..45571d64 100644 --- a/api/src/plugins/discordHooks.ts +++ b/api/src/plugins/discordHooks.ts @@ -283,6 +283,30 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context) => { }); } + /* + * We have a nice ductape solution for the following problem: + * During the handling of these hooks, the changes to the database are not committed yet. + * This means that we can't query the database for the new user id. + * We have to wait a bit to make sure the user is in the database. + * Alternatively we can hook the postgraphile lifecycle but that is not compatible with the current setup. + * The outgoing request is probably handling within 1 second, so this works fine. + */ + if (fieldContext.scope.fieldName === "registerWithToken") { + const username = args.input.login; // the login is equal to the username at registration + setTimeout(async () => { + const userId = await getUserIdFromUsername(username, null); // use null to get a new client which is privileged as the Discord bot + if (userId == null) return; + const ctfs = await getAccessibleCTFsForUser(userId, null); + for (let i = 0; i < ctfs.length; i++) { + await changeDiscordUserRoleForCTF(userId, ctfs[i], "add").catch( + (err) => { + console.error("Error while adding role to user: ", err); + } + ); + } + }, 2000); + } + return input; }; @@ -318,30 +342,6 @@ const discordMutationHook = (_build: Build) => (fieldContext: Context) => { }); } - /* - * We have a nice ductape solution for the following problem: - * During the handling of these hooks, the changes to the database are not committed yet. - * This means that we can't query the database for the new user id. - * We have to wait a bit to make sure the user is in the database. - * Alternatively we can hook the postgraphile lifecycle but that is not compatible with the current setup. - * The outgoing request is probably handling within 1 second, so this works fine. - */ - if (fieldContext.scope.fieldName === "registerWithToken") { - const username = args.input.login; // the login is equal to the username at registration - setTimeout(async () => { - const userId = await getUserIdFromUsername(username, null); // use null to get a new client which is privileged as the Discord bot - if (userId == null) return; - const ctfs = await getAccessibleCTFsForUser(userId, null); - for (let i = 0; i < ctfs.length; i++) { - await changeDiscordUserRoleForCTF(userId, ctfs[i], "add").catch( - (err) => { - console.error("Error while adding role to user: ", err); - } - ); - } - }, 2000); - } - return input; }; From 640ee9036c43ddaf0480b03eceee5a3566d4533c Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:13:59 +0200 Subject: [PATCH 12/18] Improve Discord messages text --- api/src/discord/commands/register.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index 84e59c29..f4b29b23 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -64,20 +64,21 @@ async function createAccountLogic( const invitationUrl = await getInvitationUrl(existingInvitationCode); if (invitationUrl == null) { await interaction.editReply({ - content: "Something went wrong.", // TODO: Meaningful error messages? + content: + "Could not generate invitation URL. Please contact an administrator.", }); return; } await interaction.editReply({ - content: `Your personal invitation url: ${invitationUrl}. If you already have a CTFNote account you should link it using the /link command instead.`, + content: `Your personal invitation url: ${invitationUrl}. If you already have a CTFNote account you should link it using the \`/link\` command instead.`, }); return; } await interaction.editReply({ content: - "Generating private invitation url... If you already have a CTFNote account you should link it using the /link command instead.", + "Generating private invitation url... If you already have a CTFNote account you should link it using the `/link` command instead.", }); const invitationCode = await createInvitationTokenForDiscordId( @@ -88,7 +89,8 @@ async function createAccountLogic( if (invitationCode == null) { await interaction.editReply({ - content: "Something went wrong.", // TODO: Meaningful error messages? + content: + "Could not generate an invitation code. Please contact an administrator.", }); return; } @@ -96,7 +98,8 @@ async function createAccountLogic( const invitationUrl = await getInvitationUrl(invitationCode); if (invitationUrl == null) { await interaction.editReply({ - content: "Something went wrong.", // TODO: Meaningful error messages? + content: + "Could not get an invitation URL. Please contact an administrator.", }); return; } From e7e6842721d7d37e30bb7bae27d6f38423660f31 Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:22:18 +0200 Subject: [PATCH 13/18] Add strict 'true' check for Discord registration --- api/src/discord/commands/register.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index f4b29b23..04836de3 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -26,7 +26,7 @@ async function createAccountLogic( client: Client, interaction: CommandInteraction ) { - if (config.discord.registrationEnabled.toLowerCase() === "false") { + if (config.discord.registrationEnabled.toLowerCase() !== "true") { await interaction.editReply({ content: "The functionality to create your own account this way has been disabled by an administrator.", @@ -112,7 +112,7 @@ async function createAccountLogic( export const Register: Command = { name: "register", - description: "Create an account on CTFNote if you are allowed to!", + description: "Create an account on CTFNote (if enabled)!", type: ApplicationCommandType.ChatInput, run: async (client, interaction) => { return createAccountLogic(client, interaction).catch((e) => { From eaeaea36015e6a2ac47162a7d6c99c09ee834237 Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:24:01 +0200 Subject: [PATCH 14/18] Small text improvements --- api/src/discord/commands/register.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index 04836de3..b4ebdd5d 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -78,7 +78,7 @@ async function createAccountLogic( await interaction.editReply({ content: - "Generating private invitation url... If you already have a CTFNote account you should link it using the `/link` command instead.", + "Generating a private invitation URL... If you already have a CTFNote account you should link it using the `/link` command instead.", }); const invitationCode = await createInvitationTokenForDiscordId( @@ -116,7 +116,7 @@ export const Register: Command = { type: ApplicationCommandType.ChatInput, run: async (client, interaction) => { return createAccountLogic(client, interaction).catch((e) => { - console.error("Error during register logic: ", e); + console.error("Error during /register Discord logic: ", e); }); }, }; From ee4466521ca3ba45aae23db4d7111eb01d2ca628 Mon Sep 17 00:00:00 2001 From: JJ-8 <34778827+JJ-8@users.noreply.github.com> Date: Sun, 4 Aug 2024 17:33:12 +0200 Subject: [PATCH 15/18] Improve `/register` output even more --- api/src/discord/commands/register.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index b4ebdd5d..a99dd060 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -22,6 +22,14 @@ async function getInvitationUrl(invitationCode: string | null = null) { return `http${ssl}://${config.pad.domain}/#/auth/register/${invitationCode}`; } +async function getProfileUrl() { + if (config.pad.domain == "") return null; + + const ssl = config.pad.useSSL == "false" ? "" : "s"; + + return `http${ssl}://${config.pad.domain}/#/user/settings`; +} + async function createAccountLogic( client: Client, interaction: CommandInteraction @@ -71,7 +79,7 @@ async function createAccountLogic( } await interaction.editReply({ - content: `Your personal invitation url: ${invitationUrl}. If you already have a CTFNote account you should link it using the \`/link\` command instead.`, + content: `Your personal invitation url: ${invitationUrl}.\n-# If you already have a CTFNote account you should link it using the \`/link\` command using the Discord token from your profile: ${await getProfileUrl()}.`, }); return; } @@ -105,7 +113,7 @@ async function createAccountLogic( } await interaction.editReply({ - content: `Your personal invitation url: ${invitationUrl}. If you already have a CTFNote account you should link it using the /link command instead.`, + content: `Your personal invitation url: ${invitationUrl}.\n-# If you already have a CTFNote account you should link it using the \`/link\` command using the Discord token from your profile: ${await getProfileUrl()}.`, }); return; } From 064425627038852fcac619ef7e402ee31503a50e Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Sun, 4 Aug 2024 18:22:18 +0200 Subject: [PATCH 16/18] Add comments to sql migration file --- api/migrations/55-discord-account-invitation-link.sql | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/migrations/55-discord-account-invitation-link.sql b/api/migrations/55-discord-account-invitation-link.sql index 46f7e6ad..dff46b91 100644 --- a/api/migrations/55-discord-account-invitation-link.sql +++ b/api/migrations/55-discord-account-invitation-link.sql @@ -52,6 +52,9 @@ SECURITY DEFINER; GRANT EXECUTE ON FUNCTION ctfnote.register_with_token (text, text, text) TO user_anonymous; +-- first we remove and re-apply the old internal registration function to be extra verbose +-- we implement the additional logic for registration with discord_id in a seperate function with the same name, thus overloading this function for normal original operation and +-- operation with the new discord id linking. DROP FUNCTION ctfnote_private.do_register ("login" text, "password" text, "role" ctfnote.role); CREATE OR REPLACE FUNCTION ctfnote_private.do_register ("login" text, "password" text, "role" ctfnote.role) @@ -76,7 +79,7 @@ LANGUAGE plpgsql STRICT SECURITY DEFINER; - +-- overloaded function, implements the logic needed for discord linking. CREATE OR REPLACE FUNCTION ctfnote_private.do_register ("login" text, "password" text, "role" ctfnote.role, "discord_id" text) RETURNS ctfnote.jwt AS $$ From d5053c2d8bbb2e5d9c7f1e85d46b0c4055b9d0d6 Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Sun, 4 Aug 2024 16:38:20 +0000 Subject: [PATCH 17/18] Rename registration logic function to match command name --- api/src/discord/commands/register.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index a99dd060..7d73345e 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -30,7 +30,7 @@ async function getProfileUrl() { return `http${ssl}://${config.pad.domain}/#/user/settings`; } -async function createAccountLogic( +async function registerLogic( client: Client, interaction: CommandInteraction ) { @@ -123,7 +123,7 @@ export const Register: Command = { description: "Create an account on CTFNote (if enabled)!", type: ApplicationCommandType.ChatInput, run: async (client, interaction) => { - return createAccountLogic(client, interaction).catch((e) => { + return registerLogic(client, interaction).catch((e) => { console.error("Error during /register Discord logic: ", e); }); }, From 22cdd89920df44d8ce435f839dd30dc96e487129 Mon Sep 17 00:00:00 2001 From: Daan Breur Date: Sun, 4 Aug 2024 16:49:43 +0000 Subject: [PATCH 18/18] format fix --- api/src/discord/commands/register.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/api/src/discord/commands/register.ts b/api/src/discord/commands/register.ts index 7d73345e..a6a68b91 100644 --- a/api/src/discord/commands/register.ts +++ b/api/src/discord/commands/register.ts @@ -30,10 +30,7 @@ async function getProfileUrl() { return `http${ssl}://${config.pad.domain}/#/user/settings`; } -async function registerLogic( - client: Client, - interaction: CommandInteraction -) { +async function registerLogic(client: Client, interaction: CommandInteraction) { if (config.discord.registrationEnabled.toLowerCase() !== "true") { await interaction.editReply({ content: