From dbb1e78429140e93f9c179052132c6da745cbfdd Mon Sep 17 00:00:00 2001 From: Hamza Ali Date: Wed, 4 May 2022 11:13:17 +0700 Subject: [PATCH] Cleanup --- A.md | 62 ---------------------- package-lock.json | 1 + src/bot/Bot.ts | 4 ++ src/bot/inhibitors/HasRole.ts | 39 ++++++++------ src/bot/inhibitors/SetupWhileRegistered.ts | 4 +- src/controllers/Info.ts | 24 ++++++++- src/controllers/OAuth.ts | 28 ++++++++-- src/controllers/Submit.ts | 6 +++ src/entity/Appeal.ts | 23 ++++++++ src/entity/Cleaner.ts | 61 ++++++++++++++------- src/entity/DiscordToken.ts | 12 ++++- src/entity/Guild.ts | 4 -- src/entity/StateToken.ts | 11 ++++ src/index.ts | 15 +++++- 14 files changed, 182 insertions(+), 112 deletions(-) delete mode 100644 A.md diff --git a/A.md b/A.md deleted file mode 100644 index b6224b1..0000000 --- a/A.md +++ /dev/null @@ -1,62 +0,0 @@ -# Criterion A: Planning - -## The Programmer's Hangout Unban System - -### Defining the Problem - -veksen is a senior moderator of TPH (The Programmer's Hangout), an online -community with over 55,000 members from all over the world. The community is on -Discord, a very popular chatting platform where people can hang out. veksen, -alongside with the rest of the TPH staff team have a rather clunky way of -handling those who misbehave, manage to get themselves *banned* from the -server, and want a second chance. Usually, they either have to create a new -account and join the server in an attempt to get in touch with the staff team, -or have to get a friend to help them out. - -In September, veksen approached me and discussed the issue at hand, wanting an -unban system with an online interface, so that it is accessible for those who -can't join the community anymore. The online interface would authenticate you -with the very open Discord API, be able to validate, that a user is in fact -banned from the server, and allow users to fill out a form to appeal their -punishment(s). This seemed like a fantastic project for me to complete for my -Internal Assessment, as I would be able to solve a programming-related problem, -the actual implementation isn't going to be completely non-trivial and simple -to solve, and I would be helping out a community that has been so helpful and -useful to not only myself, but thousands of others alike. - -In order to get more details, I decided to hop in a call with veksen on -September 11th, and get more information about how the system would work. - -### Rationale for Proposed Solution - -There is going to be three main parts to the system. Firstly, there is going to -be a REST API that their current website will hook into. This API will have -access to information about users who authorize themselves with Discord, so -that those who are appealing can be identified when filling out the request. -Next, their current website will hook up with the backend API. Finally, there -will be a "bot" on the community itself. when a request is processed, will -allow the users to rejoin with limited access, so that the staff team can -further look into their case and ask more questions. - -I have decided to write the bot, as well as the backend REST API in TypeScript -because - -- The rest of the staff team at TPH have experience with it -- Easy to write and maintain, especially in comparison with JavaScript, which - is purely dynamically typed -- Has great libraries available to create REST APIs, as well as Discord bots - -The frontend will be written with React/Gatsby, as that's what the current -website is using, and rewriting it all from scratch will be undesirable. - -### The Success Criteria - -1. Allows users to authorize themselves with Discord's OAuth Api -1. Identifies whether or not users are banned on the server -1. Interfaces with the backend REST API to submit an appeal request -1. Uses an email client to inform users information about their appeal request -1. Connects with the Discord Bot (known as Contrition) to allow users limited -access on TPH -1. Has a logging system to keep track of misuse and errors -1. Has a flexible configuration system to stay flexible if the TPH server setup -is changed diff --git a/package-lock.json b/package-lock.json index 7cc9135..b749ebc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5,6 +5,7 @@ "requires": true, "packages": { "": { + "name": "contrition", "version": "0.0.1", "license": "ISC", "dependencies": { diff --git a/src/bot/Bot.ts b/src/bot/Bot.ts index 9839ceb..dfb3586 100644 --- a/src/bot/Bot.ts +++ b/src/bot/Bot.ts @@ -23,13 +23,16 @@ class Bot extends AkairoClient { Bot.config = config; super.login(config.bot.token); + // register inhibitors in inhibitors directory this.inhibitorHandler = new InhibitorHandler(this, { directory: `${__dirname}/inhibitors`, }); + // register commands in commands directory this.commandHandler = new CommandHandler(this, { directory: `${__dirname}/commands`, prefix: config.bot.prefix, + // command defaults argumentDefaults: { prompt: { retries: 0, @@ -45,6 +48,7 @@ class Bot extends AkairoClient { }, }); + // register listeners in listeners directory this.listenerHandler = new ListenerHandler(this, { directory: `${__dirname}/listeners`, }); diff --git a/src/bot/inhibitors/HasRole.ts b/src/bot/inhibitors/HasRole.ts index f1bb410..2c1f6bd 100644 --- a/src/bot/inhibitors/HasRole.ts +++ b/src/bot/inhibitors/HasRole.ts @@ -3,26 +3,31 @@ import { Role } from "discord.js"; import { Message } from "discord.js"; import { Guild } from "../../entity/Guild"; -class RegisteredGuild extends Inhibitor { - constructor() { - super("HasRole", { - reason: "permissions", - type: "post", - priority: 0 - }) - } +class HasRole extends Inhibitor { + constructor() { + super("HasRole", { + reason: "permissions", + type: "post", + priority: 0, + }); + } - async exec(message: Message, command: Command) { - const author = message.member - if (!author) return false + async exec(message: Message, command: Command) { + const author = message.member; + if (!author) return false; - if (author.hasPermission("ADMINISTRATOR")) return false - if (command.id === "setup") return true + // ADMINISTRATOR should override all other permissions, if available + if (author.hasPermission("ADMINISTRATOR")) return false; + if (command.id === "setup") return true; - const guild = await Guild.findOneOrFail({ where: { guild: author.guild.id } }) + const guild = await Guild.findOneOrFail({ + where: { guild: author.guild.id }, + }); - return !author.roles.cache.some((role: Role) => role.id === guild.staffRole) - } + return !author.roles.cache.some( + (role: Role) => role.id === guild.staffRole + ); + } } -export default RegisteredGuild; +export default HasRole; diff --git a/src/bot/inhibitors/SetupWhileRegistered.ts b/src/bot/inhibitors/SetupWhileRegistered.ts index 3bc289a..3425d1f 100644 --- a/src/bot/inhibitors/SetupWhileRegistered.ts +++ b/src/bot/inhibitors/SetupWhileRegistered.ts @@ -2,7 +2,7 @@ import { Command, Inhibitor } from "discord-akairo"; import { Message } from "discord.js"; import { Guild } from "../../entity/Guild"; -class RegisteredGuild extends Inhibitor { +class SetupWhileRegistered extends Inhibitor { constructor() { super("SetupWhileRegistered", { reason: "already setup", @@ -25,4 +25,4 @@ class RegisteredGuild extends Inhibitor { } } -export default RegisteredGuild; +export default SetupWhileRegistered; diff --git a/src/controllers/Info.ts b/src/controllers/Info.ts index 49fb95b..521853a 100644 --- a/src/controllers/Info.ts +++ b/src/controllers/Info.ts @@ -4,19 +4,36 @@ import { DiscordToken } from "../entity/DiscordToken"; import { Guild } from "../entity/Guild"; import { fail, respond } from "./Util"; +/** + * info handles requests for users to get their information. In in this + * situation, this handler authenticates the user credentials, and then returns + * the following information: + * + * - Discord ID + * - Discord Username#Discriminator + * - Discord Profile picture + * - Registered platforms that can be appealed to at the moment + * + * @route /api/info + * + */ export const info = async (req: Request, res: Response) => { const { token } = req.body; - // enforce types + // enforce valid request body if (!(typeof token === "string")) { - return fail(res, 500, "Invalid request body"); + return fail(res, 400, "Invalid request body"); } + // if the token is bad, the user is not correctly authenticating const discordToken = await DiscordToken.findOne({ where: { id: token } }); if (!discordToken) { return fail(res, 400, "Invalid token provided"); } + // DiscordCache checks to see if the information for the user is in our + // database. If it's not, then it queries it, automatically caches it, and + // then returns it. let cache: DiscordCache; try { cache = await discordToken.queryData(); @@ -25,6 +42,8 @@ export const info = async (req: Request, res: Response) => { return; } + // Get the registered guilds that can be appealed to. This data is combined + // with the cached information to be returned to the user. let platforms: Guild[]; try { platforms = await Guild.all(); @@ -33,6 +52,7 @@ export const info = async (req: Request, res: Response) => { return fail(res, 500, "An internal error occurred."); } + // Create the JSON response data, and return it to the caller. respond(res, { id: cache.userID, username: cache.username, diff --git a/src/controllers/OAuth.ts b/src/controllers/OAuth.ts index 4683d1e..f19687e 100644 --- a/src/controllers/OAuth.ts +++ b/src/controllers/OAuth.ts @@ -1,28 +1,48 @@ import base64url from "base64url"; +// Module crypto gives an RNG source that is more secure (slower, more +// entropy), and better for security. import * as crypto from "crypto"; import { Request, Response } from "express"; import { OAUTH_SCOPE } from "../config/Constants"; import { StateToken } from "../entity/StateToken"; import { respond } from "./Util"; +/** + * genOauth is the endpoint for users to visit and authorize themselves with Discord. + * No passwords are exchanged with our servers, only receiving acknowledgement + * and identification of users, meaning that we can access their basic + * information in a safe manner. + * + * @route /api/oauth + * + */ export const genOAuth = async (req: Request, res: Response) => { + // Get application core configuration const { api } = req.app.get("config"); + // Generate secure token with package crypto. Make sure the token doesn't + // already exist using a simple do while {}. If the token exists, simply + // regenerate. Should not take more than 1 iteration in almost all + // situations. let token: string; do { token = base64url(crypto.randomBytes(18)); } while ((await StateToken.count({ where: { token } })) !== 0); + // Save the generated token to the database, so it can be validated when we + // get the response back from Discord. new StateToken(token).save(); + // Generate the redirect URL with the relevant information for the OAuth2 + // Auth flow. let url = - "https://discord.com/api/oauth2/authorize" + + "https://canary.discord.com/api/oauth2/authorize" + `?response_type=code` + `&prompt=consent` + `&client_id=${api.client_id}` + - `&scope=${encodeURIComponent(OAUTH_SCOPE)}` + - `&state=${token}` + - `&redirect_uri=${encodeURIComponent(api.redirect_uri)}`; + `&scope=${encodeURIComponent(OAUTH_SCOPE)}` + // Only get the relevant info - identity + `&state=${token}` + // Prevent impersonation with temporary state token + `&redirect_uri=${encodeURIComponent(api.redirect_uri)}`; // {baseurl}/authorize respond(res, { redirectURL: url }); }; diff --git a/src/controllers/Submit.ts b/src/controllers/Submit.ts index 907645e..b461aa7 100644 --- a/src/controllers/Submit.ts +++ b/src/controllers/Submit.ts @@ -47,6 +47,8 @@ export const submit = async (req: Request, res: Response) => { return fail(res, 400, "Guild not found"); } + // Make sure the user doesn't already have an active appeal for the same + // guild const appeals = await Appeal.activeAppeals(cache.userID, guild.id); if (appeals.length != 0) { return fail( @@ -56,6 +58,7 @@ export const submit = async (req: Request, res: Response) => { ); } + // Create a new appeal entry const appeal = new Appeal( cache.userID, `${cache.username}#${cache.discriminator}`, @@ -67,13 +70,16 @@ export const submit = async (req: Request, res: Response) => { additional_info ); + // Ensure all of the appeal fields are valid, (required, within length, etc) const msg = appeal.validationMessage(); if (msg !== null) { return fail(res, 422, msg); } + // Get the event emitter for appeals const emitter: EventEmitter = req.app.get("emitter"); + // Emit to the appeal, and use callbacks to respond to the http web request. await new Promise((res, rej) => { emitter.emit("newAppeal", guild, appeal, res, rej); }) diff --git a/src/entity/Appeal.ts b/src/entity/Appeal.ts index 6194db3..7242664 100644 --- a/src/entity/Appeal.ts +++ b/src/entity/Appeal.ts @@ -196,19 +196,29 @@ Status: ${this.status}\n`; return `${CDN_URL}/avatars/${this.userID}/${this.userAvatar}.png`; } + /** + * activeAppeals queries the database for all appeals that are currently + * active for a specifc userID in a guild. + * + * @param {string} userID The userID to query for. + * @param {string} guildID The guildID to query for. + */ static async activeAppeals(userID: string, guild: string) { return await Appeal.createQueryBuilder("appeals") + // where limits the appeals guild and user .where("appeals.userID = :userID", { userID }) .andWhere("appeals.guildId = :guildID", { guildID: guild }) .andWhere( new Brackets((qb) => qb + // if it's accepted or rejected, it's not active anymore .where("appeals.status NOT IN (:...statuses)", { statuses: ["accepted", "rejected"], }) .orWhere( new Brackets((qb) => qb + // however, if the appeal is rejected but the rejection hasn't expired, we still consider the appeal to be active .where(`appeals.status = "rejected"`) .andWhere( "appeals.rejectionExpiry IS NULL OR appeals.rejectionExpiry > datetime(:now)", @@ -222,17 +232,30 @@ Status: ${this.status}\n`; .getMany(); } + /** + * allAppeals gets all appeals for a specific userID in a guild. + * + * @param {string} userID The userID to query for. + * @param {string} guildID The guildID to query for. If not provided, will query for all appeals. + */ static async allAppeals(userID: string, guild?: string) { const query = Appeal.createQueryBuilder("appeals") .where("appeals.userID = :userID", { userID }) .leftJoinAndSelect("appeals.guild", "guilds") .orderBy("appeals.created", "DESC"); + // Conditionally add guild filter if (guild) query.andWhere("appeals.guildId = :guild", { guild }); return await query.getMany(); } + /** + * appeal gets a specific appeal, and will return a rejected promise if the appeal is not found. + * While the appeal ID is not a unique identifier, also specifying the + * guildID is done so staff can't accidentally query appeals from other + * guilds. + */ static async appeal(id: string, guild?: string) { const query = Appeal.createQueryBuilder("appeals") .where("appeals.id = :id", { id }) diff --git a/src/entity/Cleaner.ts b/src/entity/Cleaner.ts index b078f91..df3a2cb 100644 --- a/src/entity/Cleaner.ts +++ b/src/entity/Cleaner.ts @@ -3,26 +3,49 @@ import { DiscordCache } from "./DiscordCache"; import { DiscordToken } from "./DiscordToken"; import { StateToken } from "./StateToken"; +/** + * pruneDB starts a long-running process that will prune the database every + * @interval seconds. + * + * This helps keep a low storage foot print for the different transient data + * structures this application has. + * + * @param {number} interval. + * + */ export const pruneDB = (interval: number) => { - const manager = getManager(); + const manager = getManager(); - setInterval(() => { - const cleaning = [ - { type: StateToken, name: "state_tokens", col: "expiring" }, - { type: DiscordToken, name: "discord_tokens", col: "expiring" }, - { type: DiscordCache, name: "discord_cache", col: "expiring" } - ] + // setInterval is a builtin JS function that will run the embedded function + // every interval. + setInterval(() => { + // Prune these tables. Determining when to prune is based on col, and the + // name of the table is in the name field. + const cleaning = [ + { type: StateToken, name: "state_tokens", col: "expiring" }, + { type: DiscordToken, name: "discord_tokens", col: "expiring" }, + { type: DiscordCache, name: "discord_cache", col: "expiring" }, + ]; - cleaning.forEach(({ type, name, col }) => { - manager - .createQueryBuilder() - .delete() - .from(type, name) - .where(`${name}.${col} < datetime(:now)`, { now: new Date().toISOString() }) - .execute() - .catch(err => { - console.error("Error while pruning db: " + err) - }) + // Run the query for each table in the cleaning array using a foreach. + // + // Sample SQL query generated: + // DELETE FROM [name] + // WHERE [name].[col] < datetime(:now) + // + // Will only remove columns that are past expiry date due to WHERE clause. + cleaning.forEach(({ type, name, col }) => { + manager + .createQueryBuilder() + .delete() + .from(type, name) + .where(`${name}.${col} < datetime(:now)`, { + now: new Date().toISOString(), }) - }, interval); -} \ No newline at end of file + .execute() + .catch((err) => { + console.error("Error while pruning db: " + err); + }); + }); + }, interval); +}; diff --git a/src/entity/DiscordToken.ts b/src/entity/DiscordToken.ts index 7f9677a..5c69d8e 100644 --- a/src/entity/DiscordToken.ts +++ b/src/entity/DiscordToken.ts @@ -23,7 +23,15 @@ export class DiscordToken extends BaseEntity { this.expiring.setDate(this.expiring.getDate() + 7); } + /** + * + * queryData uses the discord token information to get the Discord details + * for the user. If the data is not cached, it will be cached. Otherwise, it + * returns the cached data. The data is queried from the Discord api. + * + */ async queryData(): Promise { + // If the cache exists, return it const cache = await DiscordCache.findOne({ where: { authToken: this.authToken }, }); @@ -31,9 +39,10 @@ export class DiscordToken extends BaseEntity { return cache; } + // Query the information from the Discord API let res: AxiosResponse; try { - res = await axios.get(`${API_URL}/users/@me`, { + res = await axios.get(`${API_URL}/users/@me`, { // https://discord.com/api/v9/users/@me headers: { Authorization: `Bearer ${this.authToken}` }, validateStatus: (status) => status < 500, }); @@ -45,6 +54,7 @@ export class DiscordToken extends BaseEntity { }); } + // Error 4xx means invalid token information. if (res.status != 200) { console.error("Unexpected result from discord:", this.authToken, res.data); return Promise.reject({ diff --git a/src/entity/Guild.ts b/src/entity/Guild.ts index ab24b12..a71ed36 100644 --- a/src/entity/Guild.ts +++ b/src/entity/Guild.ts @@ -19,9 +19,6 @@ export class Guild extends BaseEntity { @Column() staffRole: string - @Column() - jailedRole: string - @Column() defaultExpiry?: string @@ -31,7 +28,6 @@ export class Guild extends BaseEntity { this.guild = guild this.channel = channel this.staffRole = staffRole - this.jailedRole = jailedRole this.defaultExpiry = defaultExpiry } diff --git a/src/entity/StateToken.ts b/src/entity/StateToken.ts index aa7ee98..5b94764 100644 --- a/src/entity/StateToken.ts +++ b/src/entity/StateToken.ts @@ -11,6 +11,17 @@ import {Config} from "../config/Config"; import {API_URL, OAUTH_SCOPE} from "../config/Constants"; import {DiscordToken} from "./DiscordToken"; +/** + * StateToken is an OAuth2 state token. + * + * These are stored in the database and used to prevent attacks on the OAuth2 + * flow, stored persistently in order of server restart, and pruned. + * + * The verify method also checks the validity of the token, and completes the OAuth2 flow. + * The constructor automatically sets the expiration time for tokens, of 30 + * minutes, in case the OAuth2 flow is not completed. + * + */ @Entity({ name: "state_tokens" }) export class StateToken extends BaseEntity { @PrimaryGeneratedColumn("uuid") diff --git a/src/index.ts b/src/index.ts index d4ae2ad..37d5d87 100644 --- a/src/index.ts +++ b/src/index.ts @@ -29,11 +29,16 @@ config.bot = new BotConfig( process.env.BOT_PREFIX || "con!" ); +// AppealEmitter is used to communicate created appeals from the web form to +// Discord. const appealEmitter = new EventEmitter(); const app = express(); +// JSON parsing middleware app.use(express.json()); +// CORS protection middleware app.use(cors({ origin: config.api.cors_host })); +// Invalid JSON handler app.use( (err: Error, _req: express.Request, res: express.Response, _next: any) => { if (err instanceof SyntaxError) { @@ -41,14 +46,19 @@ app.use( return; } - console.error(err.message, err instanceof SyntaxError); + console.error("Unhandled Exception", err.message); fail(res, 500, "Something went wrong!"); } ); +// allow accessing the emitter from the handlers +// this is known as dependency injection! +// We are setting an underlying parameter for the app, which is accessible, +// but not directly passing it around in all of our handlers. app.set("config", config); app.set("emitter", appealEmitter); +// endpoints app.get("/api", getApi); app.get("/oauth", genOAuth); app.post("/authorize", authorize); @@ -56,9 +66,12 @@ app.post("/info", info); app.post("/status", status); app.post("/submit", submit); +// Conect to the database createConnection() .then(async () => { + // Start the API server app.listen(PORT, () => console.log("Server is listening on port " + PORT)); + // Create the Bot with the configuration information and the appeal emitter. new Bot(config, appealEmitter); pruneDB(60 * 1000); })