From b0571d4c0c11ee540fe2a687360a174ced9e77fd Mon Sep 17 00:00:00 2001 From: alppp Date: Mon, 17 Aug 2026 14:09:28 +0300 Subject: [PATCH 1/2] =?UTF-8?q?px=20p4:=20fenrir=20=E2=80=94=20/link=20/un?= =?UTF-8?q?link=20/linked=20(bifrost.players.discord=5Fid=20as=20a=20strin?= =?UTF-8?q?g,=20Verified=20role=20optional)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 40 +++ discord/commands/link.js | 92 +++++++ discord/commands/linked.js | 39 +++ discord/commands/unlink.js | 107 ++++++++ discord/commands/util/linkCode.js | 73 ++++++ discord/commands/util/verifiedRole.js | 74 ++++++ modules/initializer.js | 5 + modules/mongo.js | 181 +++++++++++++ test/link.test.js | 350 ++++++++++++++++++++++++++ 9 files changed, 961 insertions(+) create mode 100644 discord/commands/link.js create mode 100644 discord/commands/linked.js create mode 100644 discord/commands/unlink.js create mode 100644 discord/commands/util/linkCode.js create mode 100644 discord/commands/util/verifiedRole.js create mode 100644 test/link.test.js diff --git a/README.md b/README.md index 54af002..9535c42 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,9 @@ falling back to a legacy shared archive with the per-server snapshot laid over t | `/banall ` | Ban a list of players across servers | | `/notice` | Author the in-game notices players see (create, broadcast, list, expire, enable, translate, show) | | `/reply ` | Send a player an in-game message from staff | +| `/link ` | Link your Discord to your Minecraft account (code comes from `/link` in game) | +| `/unlink [player]` | Unlink a Minecraft account from your Discord | +| `/linked` | List the Minecraft accounts linked to your Discord | | `/ping` | Health check | | `/reloadCommands` | Reload all bot commands | @@ -169,6 +172,43 @@ a doc is written at the start of **every** countdown, at the one choke point all Writes are fire-and-forget and wrapped: a Mongo outage can never fail or delay a reboot. Turn the relay off with `scheduler.rebootScheduler.rebootEvents: false` in `config/config.json`. +## Discord ↔ Minecraft linking (Bifrost contract) + +The proxy mints a 6-character code in game (`/link`), the player brings it here with +`/link code:`, and the link is written onto their `bifrost.players` doc. The proxy watches +that collection, so the in-game confirmation card lands about a second later. + +**One Discord ↔ many Minecraft accounts, one Minecraft account ↔ one Discord.** A second Discord +claiming an already-linked account is refused — that account has to run `/unlink` in game (or here) +first. + +**`bifrost.players`** (the three fields this writes): + +| Field | Notes | +| --- | --- | +| `discord_id` | the snowflake **as a string** — a snowflake does not survive a JS number | +| `discord_name` | the Discord username at link time | +| `discord_linked_at` | `Date` | + +**`bifrost.discord_link_codes`** (minted in game, burnt here): `code` (6 Crockford base32 chars, no +I/L/O/U), `uuid`, `username`, `createdAt`, `expiresAt` (10 min), `usedAt`, `usedBy` (Discord id). +A typed code is folded before the lookup: upper-cased, spaces and dashes stripped, `O`→`0`, +`I`/`L`→`1` (`discord/commands/util/linkCode.js`, the same rules as the proxy's `codes.ts`). + +**`bifrost.discord_link_audit`** (history, nothing reads it in flight): `uuid`, `discordId`, +`action` (`link` / `unlink`), `by: 'discord'`, `discordName`, `at`, plus `actor` when staff undid +someone else's link. + +`/unlink [player]` removes a link: players only their own accounts (the `player` option +autocompletes over them, and is only needed with more than one linked), a member with **Manage +Guild** anyone's. `/linked` lists what this Discord holds. + +**Optional Verified role:** set `discordLink.verifiedRoleId` in `config/config.json` to a role id +string and `/link` grants it, `/unlink` takes it back once that Discord has no linked accounts +left. `false` (the default) leaves roles alone — don't use `null` or `""`, the config check treats +an empty value as unfilled and stops the bot. The bot needs **Manage Roles** and its own top role +**above** the Verified role; a role failure is logged and never fails the link itself. + ## Schedulers | Scheduler | Description | diff --git a/discord/commands/link.js b/discord/commands/link.js new file mode 100644 index 0000000..9f0db91 --- /dev/null +++ b/discord/commands/link.js @@ -0,0 +1,92 @@ +/* + * /link code: + * + * The Discord half of the in-game /link flow: the proxy mints a 6-character code and + * shows it on a card, the player brings it here, and this writes the link onto their + * bifrost.players doc. The proxy watches that collection, so the confirmation card + * lands in game about a second later. + * + * One Discord may hold several Minecraft accounts; one Minecraft account holds at most + * one Discord (a second Discord has to wait for an in-game /unlink). Any member can run + * it, replies are ephemeral, and the optional Verified role never fails a link. + */ + +const { SlashCommandBuilder } = require('discord.js'); +const mongo = require('../../modules/mongo'); +const sessionLogger = require('../../modules/sessionLogger'); +const { CODE_LENGTH, normalizeCode, isValidCode, buildLinkAudit } = require('./util/linkCode'); +const verifiedRole = require('./util/verifiedRole'); + +const BAD_CODE = '❌ That code is not valid or has expired — run `/link` in game for a fresh one.'; + +module.exports = { + data: new SlashCommandBuilder() + .setName('link') + .setDescription('Link your Discord to your Minecraft account with the code from /link in game') + .setDMPermission(false) + .addStringOption(option => + option.setName('code') + .setDescription(`The ${CODE_LENGTH}-character code the proxy showed you in game`) + .setRequired(true)), + + async execute(interaction) { + await interaction.deferReply({ ephemeral: true }); + + const code = normalizeCode(interaction.options.getString('code')); + if (!isValidCode(code)) { + await interaction.editReply(BAD_CODE); + return; + } + + const codeDoc = await mongo.findLinkCode(code); + if (!codeDoc || !codeDoc.uuid) { + await interaction.editReply(BAD_CODE); + return; + } + + const player = await mongo.getBifrostPlayerByUuid(codeDoc.uuid); + if (!player) { + await interaction.editReply( + '❌ That code points at an account the proxy no longer knows — run `/link` in game again.'); + return; + } + + const username = player.username || codeDoc.username || 'your account'; + const linkedTo = player.discord_id == null ? null : String(player.discord_id); + + if (linkedTo === interaction.user.id) { + // Already theirs: burn the code so it can't be reused, change nothing else. + await mongo.markLinkCodeUsed(code, interaction.user.id); + await interaction.editReply(`✅ **${username}** is already linked to this Discord account.`); + return; + } + if (linkedTo) { + await interaction.editReply( + `❌ **${username}** is linked to another Discord account — run \`/unlink\` in game first, then use a fresh code.`); + return; + } + + await mongo.setBifrostDiscordLink(codeDoc.uuid, { + discordId: interaction.user.id, + discordName: interaction.user.username + }); + await mongo.markLinkCodeUsed(code, interaction.user.id); + + try { + await mongo.insertLinkAudit(buildLinkAudit({ + uuid: codeDoc.uuid, + discordId: interaction.user.id, + action: 'link', + discordName: interaction.user.username + })); + } catch (error) { + // The link itself is already written - an audit row is not worth failing it. + sessionLogger.error('DiscordLink', `Could not audit the link for ${codeDoc.uuid}`, error.message); + } + + await verifiedRole.addVerifiedRole(interaction, interaction.user.id); + + sessionLogger.info('DiscordLink', `${interaction.user.username} linked ${username} (${codeDoc.uuid})`); + await interaction.editReply(`✅ Linked to **${username}** — you'll see it in game.`); + }, +}; diff --git a/discord/commands/linked.js b/discord/commands/linked.js new file mode 100644 index 0000000..1d60ac1 --- /dev/null +++ b/discord/commands/linked.js @@ -0,0 +1,39 @@ +/* + * /linked + * + * Shows which Minecraft accounts this Discord holds. One Discord may hold several, so + * this is also how someone finds the name to pass to /unlink. + */ + +const { SlashCommandBuilder } = require('discord.js'); +const mongo = require('../../modules/mongo'); + +module.exports = { + data: new SlashCommandBuilder() + .setName('linked') + .setDescription('Show the Minecraft accounts linked to your Discord') + .setDMPermission(false), + + async execute(interaction) { + await interaction.deferReply({ ephemeral: true }); + + const mine = await mongo.findBifrostPlayersByDiscordId(interaction.user.id); + if (mine.length === 0) { + await interaction.editReply( + 'No Minecraft accounts are linked to this Discord — run `/link` in game to get a code.'); + return; + } + + const lines = mine + .sort((a, b) => String(a.username || '').localeCompare(String(b.username || ''))) + .map(p => { + const since = p.discord_linked_at + ? ` — linked ` + : ''; + return `• **${p.username || p.uuid}**${since}`; + }); + + await interaction.editReply( + `Linked to this Discord:\n${lines.join('\n')}\n\nUse \`/unlink\` to remove one.`); + }, +}; diff --git a/discord/commands/unlink.js b/discord/commands/unlink.js new file mode 100644 index 0000000..5e6ebbd --- /dev/null +++ b/discord/commands/unlink.js @@ -0,0 +1,107 @@ +/* + * /unlink [player] + * + * Drops a Discord <-> Minecraft link from the Discord side (the in-game /unlink does the + * same from the other end). Players only ever unlink their own accounts; a member with + * Manage Guild may unlink anyone's, which is how staff undo a mislink. + * + * The Verified role comes off once that Discord user has no linked accounts left. + */ + +const { SlashCommandBuilder, PermissionFlagsBits } = require('discord.js'); +const mongo = require('../../modules/mongo'); +const sessionLogger = require('../../modules/sessionLogger'); +const verifiedRole = require('./util/verifiedRole'); +const { buildLinkAudit } = require('./util/linkCode'); + +/** Manage Guild is the staff bar - the same permission that edits the server itself. */ +function isStaff(interaction) { + try { + const perms = interaction.memberPermissions || (interaction.member && interaction.member.permissions); + return Boolean(perms && perms.has(PermissionFlagsBits.ManageGuild)); + } catch (_) { + return false; + } +} + +module.exports = { + data: new SlashCommandBuilder() + .setName('unlink') + .setDescription('Unlink a Minecraft account from your Discord') + .setDMPermission(false) + .addStringOption(option => + option.setName('player') + .setDescription('Which account (only needed when you have more than one linked)') + .setRequired(false) + .setAutocomplete(true)), + + async autocomplete(interaction) { + const focused = interaction.options.getFocused(true); + if (focused.name !== 'player') return; + + const mine = await mongo.findBifrostPlayersByDiscordId(interaction.user.id); + const typed = String(focused.value || '').toLowerCase(); + const names = mine + .map(p => p.username) + .filter(Boolean) + .filter(name => name.toLowerCase().startsWith(typed)) + .sort((a, b) => a.localeCompare(b)) + .slice(0, 25); + await interaction.respond(names.map(name => ({ name: name, value: name }))); + }, + + async execute(interaction) { + await interaction.deferReply({ ephemeral: true }); + + const wanted = interaction.options.getString('player'); + const mine = await mongo.findBifrostPlayersByDiscordId(interaction.user.id); + + let target = null; + if (wanted) { + target = mine.find(p => String(p.username || '').toLowerCase() === wanted.toLowerCase()) || null; + if (!target && isStaff(interaction)) { + const identity = await mongo.getPlayerIdentity(wanted); + if (identity && identity.uuid && identity.discord_id != null) target = identity; + } + if (!target) { + await interaction.editReply(`❌ **${wanted}** is not linked to your Discord account.`); + return; + } + } else { + if (mine.length === 0) { + await interaction.editReply( + '❌ You have no linked Minecraft accounts — run `/link` in game to get a code.'); + return; + } + if (mine.length > 1) { + const names = mine.map(p => `\`${p.username}\``).join(', '); + await interaction.editReply(`❓ You have several accounts linked (${names}) — say which one: \`/unlink player:\`.`); + return; + } + target = mine[0]; + } + + const ownerId = String(target.discord_id); + await mongo.unsetBifrostDiscordLink(target.uuid); + + try { + // `actor` only lands when staff undid someone else's link (buildLinkAudit drops it otherwise). + await mongo.insertLinkAudit(buildLinkAudit({ + uuid: target.uuid, + discordId: ownerId, + action: 'unlink', + discordName: target.discord_name || null, + actor: interaction.user.id + })); + } catch (error) { + sessionLogger.error('DiscordLink', `Could not audit the unlink for ${target.uuid}`, error.message); + } + + const remaining = await mongo.findBifrostPlayersByDiscordId(ownerId); + if (remaining.length === 0) await verifiedRole.removeVerifiedRole(interaction, ownerId); + + sessionLogger.info('DiscordLink', + `${interaction.user.username} unlinked ${target.username} (${target.uuid}) from ${ownerId}`); + await interaction.editReply(`✅ **${target.username}** is no longer linked${ownerId === interaction.user.id ? '' : ` to <@${ownerId}>`}.`); + }, +}; diff --git a/discord/commands/util/linkCode.js b/discord/commands/util/linkCode.js new file mode 100644 index 0000000..51e3d48 --- /dev/null +++ b/discord/commands/util/linkCode.js @@ -0,0 +1,73 @@ +/* + * File: linkCode.js + * Project: valhalla-updater + * ----- + * The link-code rules and the two doc shapes /link and /unlink write, shared with the + * proxy (Bifrost's discord-link `codes.ts` mints the codes). Codes are 6 Crockford + * base32 chars - the alphabet drops I, L, O and U so a player reading one off a chat + * card can't turn it into a different code by mistyping. + * + * Normalisation forgives what people actually type: lower case, spaces and dashes, and + * the three lookalikes (O for zero, I and L for one). Anything else is simply not a code. + * (The commands/ loader only picks up top-level files, so this util is never a command.) + */ + +const CODE_LENGTH = 6; +const ALPHABET = '0123456789ABCDEFGHJKMNPQRSTVWXYZ'; +const CODE_RE = /^[0-9A-HJKMNP-TV-Z]{6}$/; + +/** + * Folds what someone typed into the canonical code form. + * @param {string} input Whatever came in on the slash command. + * @returns {string} Upper-case, unspaced, lookalikes resolved (not necessarily valid). + */ +function normalizeCode(input) { + return String(input == null ? '' : input) + .toUpperCase() + .replace(/[\s\-‐-―]+/g, '') + .replace(/O/g, '0') + .replace(/[IL]/g, '1'); +} + +/** + * Checks a normalised code against the alphabet and length. + * @param {string} code A code from normalizeCode. + * @returns {boolean} Whether it can exist at all. + */ +function isValidCode(code) { + return CODE_RE.test(String(code == null ? '' : code)); +} + +/** + * Builds the three fields the proxy reads off a bifrost.players doc. `discord_id` is a + * STRING: a snowflake is past 2^53, so a JS number loses its last digits. + * @param {object} input `{discordId, discordName, now}`. + * @returns {object} The `$set` for the player doc. + */ +function buildLinkFields(input) { + return { + discord_id: String(input.discordId), + discord_name: String(input.discordName || ''), + discord_linked_at: input.now instanceof Date ? input.now : new Date() + }; +} + +/** + * Builds one row for bifrost.discord_link_audit. History only - the proxy never reads it. + * @param {object} input `{uuid, discordId, action, discordName, actor, now}`. + * @returns {object} The audit doc (`actor` only when someone unlinked another's account). + */ +function buildLinkAudit(input) { + const doc = { + uuid: String(input.uuid), + discordId: String(input.discordId), + action: input.action, + by: 'discord', + discordName: input.discordName == null ? null : String(input.discordName), + at: input.now instanceof Date ? input.now : new Date() + }; + if (input.actor && String(input.actor) !== doc.discordId) doc.actor = String(input.actor); + return doc; +} + +module.exports = { CODE_LENGTH, ALPHABET, normalizeCode, isValidCode, buildLinkFields, buildLinkAudit }; diff --git a/discord/commands/util/verifiedRole.js b/discord/commands/util/verifiedRole.js new file mode 100644 index 0000000..72128ba --- /dev/null +++ b/discord/commands/util/verifiedRole.js @@ -0,0 +1,74 @@ +/* + * File: verifiedRole.js + * Project: valhalla-updater + * ----- + * The optional Verified role /link and /unlink keep in sync with the Minecraft link. + * Off by default: `discordLink.verifiedRoleId` in config/config.json holds the role id, + * anything else (missing, false) means no role is touched at all. + * + * Nothing here may fail a link - the link lives in Mongo and the role is decoration, so + * every path swallows its own error and logs it. The member is fetched over REST + * (`guild.members.fetch`), which needs no GuildMembers intent - same as roleAssigner. + * The bot needs Manage Roles and its own top role above the Verified role. + */ + +const sessionLogger = require('../../../modules/sessionLogger'); + +/** + * Reads the configured role id. + * @returns {string|null} The role id, or null when the feature is off (the default). + */ +function getVerifiedRoleId() { + try { + const config = require('../../../config/config.json'); + const id = config.discordLink && config.discordLink.verifiedRoleId; + return typeof id === 'string' && id ? id : null; + } catch (_) { + return null; // no config = feature off + } +} + +/** + * Gives a Discord user the Verified role. + * @param {object} interaction The guild interaction (its guild is used for the fetch). + * @param {string} userId Discord id to grant it to. + * @returns {Promise<'off'|'added'|'failed'>} What happened; never throws. + */ +async function addVerifiedRole(interaction, userId) { + const roleId = module.exports.getVerifiedRoleId(); + if (!roleId) return 'off'; + + try { + const member = await interaction.guild.members.fetch(userId); + await member.roles.add(roleId); + return 'added'; + } catch (error) { + sessionLogger.error('DiscordLink', + `Could not add the Verified role ${roleId} to ${userId} (does the bot have Manage Roles, and is its top role above that one?)`, + error.message); + return 'failed'; + } +} + +/** + * Takes the Verified role back once a Discord user has no linked accounts left. + * @param {object} interaction The guild interaction. + * @param {string} userId Discord id to remove it from. + * @returns {Promise<'off'|'removed'|'failed'>} What happened; never throws. + */ +async function removeVerifiedRole(interaction, userId) { + const roleId = module.exports.getVerifiedRoleId(); + if (!roleId) return 'off'; + + try { + const member = await interaction.guild.members.fetch(userId); + await member.roles.remove(roleId); + return 'removed'; + } catch (error) { + sessionLogger.error('DiscordLink', + `Could not remove the Verified role ${roleId} from ${userId}`, error.message); + return 'failed'; + } +} + +module.exports = { getVerifiedRoleId, addVerifiedRole, removeVerifiedRole }; diff --git a/modules/initializer.js b/modules/initializer.js index 0dcefb4..8e31d3e 100644 --- a/modules/initializer.js +++ b/modules/initializer.js @@ -86,6 +86,11 @@ function generateConfigFiles() { "notices": { "packUpdateEvents": true }, + "discordLink": { + // a role id string turns the Verified role on; false leaves roles alone + // (checkConfig below treats null and "" as unfilled and stops the bot) + "verifiedRoleId": false + }, "scheduler": {} }; diff --git a/modules/mongo.js b/modules/mongo.js index 587e3db..5c58386 100644 --- a/modules/mongo.js +++ b/modules/mongo.js @@ -15,6 +15,9 @@ const { Long } = require('mongodb'); require('dotenv').config(); +const sessionLogger = require('./sessionLogger'); +// one source for the link field shape - /link writes it, this writes it, the proxy reads it +const { buildLinkFields } = require('../discord/commands/util/linkCode'); const { mongoDBName } = require("../config/config.json").mongodb; @@ -22,6 +25,8 @@ const { const mongoClient = new MongoClient(process.env.MONGODB_URL); let mainClientConnected = false; +// The discord-link indexes are created once per process, on the first code lookup. +let discordLinkIndexesEnsured = false; // bifrost.logs starts here; anything older lives only in valhallamc.logs (the archive) const ARCHIVE_CUTOFF = new Date('2026-03-01T00:00:00Z'); @@ -1028,6 +1033,182 @@ module.exports = { .insertOne(doc); }, + // Discord <-> Minecraft linking (bifrost.discord_link_codes, bifrost.players, + // bifrost.discord_link_audit). The proxy mints the code in game and watches + // bifrost.players, so the confirmation card follows the write here in about a second. + // `discord_id` is ALWAYS a string: a snowflake does not survive a JS number. + /** + * Finds an unused, unexpired link code. + * @param {string} code Normalised code (discord/commands/util/linkCode.js). + * @returns {Promise} The code doc or null. + */ + findLinkCode: async function (code) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + await module.exports.ensureDiscordLinkIndexes(); + return mongoClient + .db('bifrost') + .collection('discord_link_codes') + .findOne({ + code: String(code), + usedAt: null, + expiresAt: { $gt: new Date() } + }); + }, + + /** + * Burns a code so it can only ever link once. + * @param {string} code Normalised code. + * @param {string} discordId Who used it. + * @returns {Promise} The updateOne result. + */ + markLinkCodeUsed: async function (code, discordId) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + return mongoClient + .db('bifrost') + .collection('discord_link_codes') + .updateOne({ code: String(code), usedAt: null }, { + $set: { + usedAt: new Date(), + usedBy: String(discordId) + } + }); + }, + + /** + * Gets a Bifrost player doc by uuid (the link code carries the uuid). + * @param {string} uuid Dashed uuid string. + * @returns {Promise} Player doc or null. + */ + getBifrostPlayerByUuid: async function (uuid) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + return mongoClient + .db('bifrost') + .collection('players') + .findOne({ uuid: String(uuid) }); + }, + + /** + * Lists the Minecraft accounts one Discord user has linked. One Discord may hold + * several accounts; one Minecraft account holds at most one Discord. + * @param {string} discordId Discord snowflake, as a string. + * @returns {Promise} `{uuid, username, discord_id, discord_name, discord_linked_at}` docs. + */ + findBifrostPlayersByDiscordId: async function (discordId) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + return mongoClient + .db('bifrost') + .collection('players') + .find({ discord_id: String(discordId) }, { + projection: { + _id: 0, + uuid: 1, + username: 1, + discord_id: 1, + discord_name: 1, + discord_linked_at: 1 + } + }) + .toArray(); + }, + + /** + * Writes the link onto the player doc. The proxy reads these three fields. + * @param {string} uuid Dashed uuid of the Minecraft account. + * @param {object} link `{discordId, discordName}`. + * @returns {Promise} The updateOne result. + */ + setBifrostDiscordLink: async function (uuid, link) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + return mongoClient + .db('bifrost') + .collection('players') + .updateOne({ uuid: String(uuid) }, { $set: buildLinkFields(link) }); + }, + + /** + * Drops the link from a player doc. + * @param {string} uuid Dashed uuid of the Minecraft account. + * @returns {Promise} The updateOne result. + */ + unsetBifrostDiscordLink: async function (uuid) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + return mongoClient + .db('bifrost') + .collection('players') + .updateOne({ uuid: String(uuid) }, { + $unset: { + discord_id: '', + discord_name: '', + discord_linked_at: '' + } + }); + }, + + /** + * Appends one link/unlink audit row. History only - nothing reads it in flight. + * @param {object} doc `{uuid, discordId, action, by, discordName, at}`. + * @returns {Promise} The insertOne result. + */ + insertLinkAudit: async function (doc) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + return mongoClient + .db('bifrost') + .collection('discord_link_audit') + .insertOne(doc); + }, + + /** + * Creates the indexes the link flow needs, once per process. The proxy creates the + * same ones, so the specs are kept identical (default names, same options) and a + * conflict is logged rather than thrown - an index is not worth a failed link. + * @returns {Promise} Resolves when the attempt is done. + */ + ensureDiscordLinkIndexes: async function () { + if (discordLinkIndexesEnsured) return; + discordLinkIndexesEnsured = true; + + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + + try { + const db = mongoClient.db('bifrost'); + await db.collection('players').createIndex({ discord_id: 1 }, { sparse: true }); + await db.collection('discord_link_codes').createIndex({ code: 1 }, { unique: true }); + } catch (error) { + sessionLogger.warn('Mongo', 'Could not ensure the discord-link indexes', error.message); + } + }, + /** * Gets the main MongoDB client (for advanced queries). * @returns {MongoClient} The main MongoDB client diff --git a/test/link.test.js b/test/link.test.js new file mode 100644 index 0000000..01f8d2b --- /dev/null +++ b/test/link.test.js @@ -0,0 +1,350 @@ +/* + * Unit tests for /link, /unlink and /linked — the docs written into the Bifrost + * collections and the refusals that keep a link honest. + * Run: npm test (node --test test/) + * + * The proxy reads `discord_id` off bifrost.players and renders it in game, so the + * contract is: a STRING snowflake, a Date for `discord_linked_at`, the code burnt so it + * can only link once, and an audit row. A Minecraft account already linked to another + * Discord is never overwritten, and the optional Verified role can fail all it likes - + * the link is in Mongo and stays there. + */ + +const { test, beforeEach } = require('node:test'); +const assert = require('node:assert'); +const codes = require('../discord/commands/util/linkCode'); +const verifiedRole = require('../discord/commands/util/verifiedRole'); +const link = require('../discord/commands/link'); +const unlink = require('../discord/commands/unlink'); +const linked = require('../discord/commands/linked'); +const mongo = require('../modules/mongo'); + +const NOW = new Date('2026-08-17T12:00:00Z'); + +let codeDocs; // code -> doc returned by findLinkCode +let players; // uuid -> bifrost.players doc +let identities; // lowercased username -> doc for getPlayerIdentity +let sets; // setBifrostDiscordLink calls +let unsets; // unsetBifrostDiscordLink calls +let used; // markLinkCodeUsed calls +let audits; // insertLinkAudit docs +let roleCalls; // {action, userId, roleId} from the fake guild +let roleFetchThrows; +let configuredRole; + +beforeEach(() => { + codeDocs = { + ABC234: { code: 'ABC234', uuid: 'uuid-alp', username: 'Alp', usedAt: null } + }; + players = { + 'uuid-alp': { uuid: 'uuid-alp', username: 'Alp' }, + 'uuid-taken': { uuid: 'uuid-taken', username: 'Taken', discord_id: '999', discord_name: 'someone' } + }; + identities = {}; + sets = []; + unsets = []; + used = []; + audits = []; + roleCalls = []; + roleFetchThrows = false; + configuredRole = null; + + mongo.findLinkCode = async (code) => codeDocs[code] || null; + mongo.getBifrostPlayerByUuid = async (uuid) => players[uuid] || null; + mongo.getPlayerIdentity = async (name) => identities[String(name).toLowerCase()] || null; + mongo.findBifrostPlayersByDiscordId = async (discordId) => + Object.values(players).filter(p => p.discord_id === String(discordId)); + mongo.setBifrostDiscordLink = async (uuid, linkFields) => { + sets.push({ uuid, ...linkFields }); + // exactly what modules/mongo.js $sets - the field shape is the proxy's contract + players[uuid] = { ...players[uuid], ...codes.buildLinkFields(linkFields) }; + return { modifiedCount: 1 }; + }; + mongo.unsetBifrostDiscordLink = async (uuid) => { + unsets.push(uuid); + if (players[uuid]) { + delete players[uuid].discord_id; + delete players[uuid].discord_name; + delete players[uuid].discord_linked_at; + } + return { modifiedCount: 1 }; + }; + mongo.markLinkCodeUsed = async (code, discordId) => { + used.push({ code, discordId }); + if (codeDocs[code]) delete codeDocs[code]; // burnt: a second lookup finds nothing + return { modifiedCount: 1 }; + }; + mongo.insertLinkAudit = async (doc) => { audits.push(doc); return { insertedId: 'a' }; }; + mongo.ensureDiscordLinkIndexes = async () => {}; + + verifiedRole.getVerifiedRoleId = () => configuredRole; +}); + +function interaction(options, opts = {}) { + const replies = []; + const user = { id: opts.userId || '4242', username: opts.username || 'alpdiscord' }; + return { + replies, + user: user, + memberPermissions: { has: () => Boolean(opts.staff) }, + guild: { + members: { + fetch: async (id) => { + if (roleFetchThrows) throw new Error('Missing Permissions'); + return { + id: id, + roles: { + add: async (roleId) => roleCalls.push({ action: 'add', userId: id, roleId }), + remove: async (roleId) => roleCalls.push({ action: 'remove', userId: id, roleId }) + } + }; + } + } + }, + options: { + getString: (name) => (typeof options[name] === 'string' ? options[name] : null), + getFocused: () => ({ name: 'player', value: options.focused || '' }) + }, + deferReply: async () => {}, + editReply: async (payload) => { replies.push(payload); return payload; }, + respond: async (choices) => { replies.push(choices); return choices; } + }; +} + +test('link code normalisation: case, spaces, dashes and the O/I/L lookalikes', () => { + assert.strictEqual(codes.normalizeCode(' abc-2 34 '), 'ABC234'); + assert.strictEqual(codes.normalizeCode('abc234'), 'ABC234'); + assert.strictEqual(codes.normalizeCode('oil234'), '011234'); + assert.strictEqual(codes.normalizeCode('ABC—234'), 'ABC234', 'an em dash is a dash too'); + assert.strictEqual(codes.normalizeCode(null), ''); +}); + +test('link code validity: 6 Crockford chars, no I L O U', () => { + assert.strictEqual(codes.isValidCode('ABC234'), true); + assert.strictEqual(codes.isValidCode('011234'), true); + assert.strictEqual(codes.isValidCode('ABC23'), false, 'too short'); + assert.strictEqual(codes.isValidCode('ABC2345'), false, 'too long'); + assert.strictEqual(codes.isValidCode('abc234'), false, 'normalise first'); + for (const bad of ['I', 'L', 'O', 'U']) { + assert.strictEqual(codes.isValidCode(`ABC2${bad}4`), false, `${bad} is not in the alphabet`); + } + assert.strictEqual(codes.ALPHABET.length, 32); + assert.ok(codes.ALPHABET.split('').every(c => codes.isValidCode(c.repeat(6)))); +}); + +test('the player fields are a STRING snowflake and a Date - a number loses digits', () => { + const fields = codes.buildLinkFields({ discordId: '1362840000000000123', discordName: 'alpdiscord', now: NOW }); + assert.deepStrictEqual(fields, { + discord_id: '1362840000000000123', + discord_name: 'alpdiscord', + discord_linked_at: NOW + }); + assert.strictEqual(typeof fields.discord_id, 'string'); + assert.notStrictEqual(String(Number(fields.discord_id)), fields.discord_id, 'past 2^53 - why it is a string'); + assert.strictEqual(codes.buildLinkFields({ discordId: 4242, now: NOW }).discord_id, '4242'); + assert.strictEqual(codes.buildLinkFields({ discordId: '1', now: NOW }).discord_name, ''); + assert.ok(codes.buildLinkFields({ discordId: '1' }).discord_linked_at instanceof Date); +}); + +test('the audit row carries an actor only when someone else did the unlinking', () => { + assert.deepStrictEqual(codes.buildLinkAudit({ + uuid: 'uuid-alp', discordId: '4242', action: 'link', discordName: 'alpdiscord', now: NOW + }), { + uuid: 'uuid-alp', discordId: '4242', action: 'link', by: 'discord', + discordName: 'alpdiscord', at: NOW + }); + assert.strictEqual(codes.buildLinkAudit({ + uuid: 'u', discordId: '4242', action: 'unlink', actor: '4242', now: NOW + }).actor, undefined, 'unlinking your own account is not a staff action'); + assert.strictEqual(codes.buildLinkAudit({ + uuid: 'u', discordId: '999', action: 'unlink', actor: '1111', now: NOW + }).actor, '1111'); + assert.strictEqual(codes.buildLinkAudit({ uuid: 'u', discordId: '1', action: 'unlink' }).discordName, null); +}); + +test('/link writes discord_id as a STRING, burns the code and audits it', async () => { + const it = interaction({ code: 'abc-234' }); + await link.execute(it); + + assert.deepStrictEqual(sets, [{ uuid: 'uuid-alp', discordId: '4242', discordName: 'alpdiscord' }]); + assert.strictEqual(typeof players['uuid-alp'].discord_id, 'string', 'a snowflake never survives a JS number'); + assert.strictEqual(players['uuid-alp'].discord_id, '4242'); + assert.strictEqual(players['uuid-alp'].discord_name, 'alpdiscord'); + assert.ok(players['uuid-alp'].discord_linked_at instanceof Date); + + assert.deepStrictEqual(used, [{ code: 'ABC234', discordId: '4242' }]); + assert.strictEqual(audits.length, 1); + assert.strictEqual(audits[0].uuid, 'uuid-alp'); + assert.strictEqual(audits[0].discordId, '4242'); + assert.strictEqual(audits[0].action, 'link'); + assert.strictEqual(audits[0].by, 'discord'); + assert.strictEqual(audits[0].discordName, 'alpdiscord'); + assert.ok(audits[0].at instanceof Date); + assert.match(it.replies[0], /Linked to \*\*Alp\*\*/); +}); + +test('/link refuses a malformed code before it reaches Mongo', async () => { + let looked = 0; + mongo.findLinkCode = async () => { looked++; return null; }; + const it = interaction({ code: 'nope' }); + await link.execute(it); + + assert.strictEqual(looked, 0); + assert.strictEqual(sets.length, 0); + assert.match(it.replies[0], /not valid or has expired/); +}); + +test('/link refuses an expired or already-used code (the lookup filters both)', async () => { + codeDocs = {}; // findLinkCode only returns usedAt:null and expiresAt in the future + const it = interaction({ code: 'ABC234' }); + await link.execute(it); + + assert.strictEqual(sets.length, 0); + assert.strictEqual(used.length, 0); + assert.strictEqual(audits.length, 0); + assert.match(it.replies[0], /not valid or has expired/); +}); + +test('/link refuses a Minecraft account already linked to another Discord', async () => { + codeDocs.XYZ789 = { code: 'XYZ789', uuid: 'uuid-taken', username: 'Taken', usedAt: null }; + const it = interaction({ code: 'XYZ789' }); + await link.execute(it); + + assert.strictEqual(sets.length, 0, 'never overwrite someone else`s link'); + assert.strictEqual(used.length, 0, 'and never burn the code doing it'); + assert.strictEqual(audits.length, 0); + assert.strictEqual(players['uuid-taken'].discord_id, '999'); + assert.match(it.replies[0], /another Discord account/); + assert.match(it.replies[0], /unlink/); +}); + +test('/link twice from the same Discord is idempotent — one write, one audit', async () => { + const first = interaction({ code: 'ABC234' }); + await link.execute(first); + codeDocs.DEF567 = { code: 'DEF567', uuid: 'uuid-alp', username: 'Alp', usedAt: null }; + const second = interaction({ code: 'DEF567' }); + await link.execute(second); + + assert.strictEqual(sets.length, 1); + assert.strictEqual(audits.length, 1); + assert.strictEqual(used.length, 2, 'the second code is still burnt'); + assert.match(second.replies[0], /already linked/); +}); + +test('/link grants the Verified role when one is configured', async () => { + configuredRole = 'role-1'; + const it = interaction({ code: 'ABC234' }); + await link.execute(it); + + assert.deepStrictEqual(roleCalls, [{ action: 'add', userId: '4242', roleId: 'role-1' }]); + assert.match(it.replies[0], /Linked to \*\*Alp\*\*/); +}); + +test('/link touches no role when none is configured', async () => { + const it = interaction({ code: 'ABC234' }); + await link.execute(it); + assert.deepStrictEqual(roleCalls, []); + assert.strictEqual(sets.length, 1); +}); + +test('a throwing role fetch does NOT fail the link', async () => { + configuredRole = 'role-1'; + roleFetchThrows = true; + const it = interaction({ code: 'ABC234' }); + await link.execute(it); + + assert.strictEqual(sets.length, 1, 'the link is in Mongo either way'); + assert.strictEqual(audits.length, 1); + assert.deepStrictEqual(roleCalls, []); + assert.match(it.replies[0], /Linked to \*\*Alp\*\*/); +}); + +test('/unlink drops the link, audits it and takes the role back when nothing is left', async () => { + configuredRole = 'role-1'; + players['uuid-alp'].discord_id = '4242'; + players['uuid-alp'].discord_name = 'alpdiscord'; + + const it = interaction({}); + await unlink.execute(it); + + assert.deepStrictEqual(unsets, ['uuid-alp']); + assert.strictEqual(audits.length, 1); + assert.strictEqual(audits[0].action, 'unlink'); + assert.strictEqual(audits[0].by, 'discord'); + assert.strictEqual(audits[0].discordId, '4242'); + assert.strictEqual(audits[0].actor, undefined, 'unlinking your own account has no separate actor'); + assert.deepStrictEqual(roleCalls, [{ action: 'remove', userId: '4242', roleId: 'role-1' }]); + assert.match(it.replies[0], /Alp/); +}); + +test('/unlink keeps the role while another account is still linked', async () => { + configuredRole = 'role-1'; + players['uuid-alp'].discord_id = '4242'; + players['uuid-alt'] = { uuid: 'uuid-alt', username: 'AlpAlt', discord_id: '4242' }; + + const it = interaction({ player: 'Alp' }); + await unlink.execute(it); + + assert.deepStrictEqual(unsets, ['uuid-alp']); + assert.deepStrictEqual(roleCalls, [], 'AlpAlt is still linked'); +}); + +test('/unlink with several accounts and no name asks which one', async () => { + players['uuid-alp'].discord_id = '4242'; + players['uuid-alt'] = { uuid: 'uuid-alt', username: 'AlpAlt', discord_id: '4242' }; + + const it = interaction({}); + await unlink.execute(it); + + assert.strictEqual(unsets.length, 0); + assert.match(it.replies[0], /several accounts/); +}); + +test('/unlink refuses someone else`s account for a normal member', async () => { + identities.taken = players['uuid-taken']; + const it = interaction({ player: 'Taken' }); + await unlink.execute(it); + + assert.strictEqual(unsets.length, 0); + assert.strictEqual(audits.length, 0); + assert.strictEqual(players['uuid-taken'].discord_id, '999'); + assert.match(it.replies[0], /not linked to your Discord/); +}); + +test('/unlink lets a Manage Guild member undo anyone`s link, and records the actor', async () => { + configuredRole = 'role-1'; + identities.taken = players['uuid-taken']; + const it = interaction({ player: 'Taken' }, { staff: true, userId: '1111', username: 'mod' }); + await unlink.execute(it); + + assert.deepStrictEqual(unsets, ['uuid-taken']); + assert.strictEqual(audits[0].discordId, '999', 'the link that was removed, not the staff member'); + assert.strictEqual(audits[0].actor, '1111'); + assert.deepStrictEqual(roleCalls, [{ action: 'remove', userId: '999', roleId: 'role-1' }]); +}); + +test('/unlink autocompletes over the caller`s own linked accounts only', async () => { + players['uuid-alp'].discord_id = '4242'; + players['uuid-alt'] = { uuid: 'uuid-alt', username: 'AlpAlt', discord_id: '4242' }; + + const it = interaction({ focused: 'alp' }); + await unlink.autocomplete(it); + + assert.deepStrictEqual(it.replies[0], [ + { name: 'Alp', value: 'Alp' }, + { name: 'AlpAlt', value: 'AlpAlt' } + ]); +}); + +test('/linked lists the caller`s accounts, and says so when there are none', async () => { + const empty = interaction({}); + await linked.execute(empty); + assert.match(empty.replies[0], /No Minecraft accounts are linked/); + + players['uuid-alp'].discord_id = '4242'; + players['uuid-alp'].discord_linked_at = NOW; + const it = interaction({}); + await linked.execute(it); + assert.match(it.replies[0], /\*\*Alp\*\*/); + assert.match(it.replies[0], //); +}); From 5438e629eddd34c2f7b6c81288771d2aac47c6a7 Mon Sep 17 00:00:00 2001 From: alppp Date: Mon, 17 Aug 2026 14:49:19 +0300 Subject: [PATCH 2/2] =?UTF-8?q?px=20p4:=20fenrir=20=E2=80=94=20/link=20rev?= =?UTF-8?q?iew=20fix:=20atomic=20code=20claim,=20index=20parity,=20code=20?= =?UTF-8?q?normalisation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 13 ++- discord/commands/link.js | 39 ++++---- discord/commands/util/linkCode.js | 7 +- modules/mongo.js | 99 +++++++++++--------- test/link.test.js | 147 +++++++++++++++++++++++++----- 5 files changed, 218 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index 9535c42..7822069 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,12 @@ that collection, so the in-game confirmation card lands about a second later. claiming an already-linked account is refused — that account has to run `/unlink` in game (or here) first. +**The code is claimed, not read.** `/link` burns it with the lookup (one `findOneAndUpdate` on +`usedAt: null`) and only then writes the player, with that update filtered on the account still +being free. Two Discords racing one code therefore make exactly one link, and an in-game link +landing mid-flow wins instead of being overwritten. A refusal still spends the code — the reply +says so, and the player just runs `/link` in game again for a fresh one. + **`bifrost.players`** (the three fields this writes): | Field | Notes | @@ -192,8 +198,11 @@ first. **`bifrost.discord_link_codes`** (minted in game, burnt here): `code` (6 Crockford base32 chars, no I/L/O/U), `uuid`, `username`, `createdAt`, `expiresAt` (10 min), `usedAt`, `usedBy` (Discord id). -A typed code is folded before the lookup: upper-cased, spaces and dashes stripped, `O`→`0`, -`I`/`L`→`1` (`discord/commands/util/linkCode.js`, the same rules as the proxy's `codes.ts`). +A typed code is folded before the lookup: upper-cased, spaces, dashes and underscores stripped, +`O`→`0`, `I`/`L`→`1` (`discord/commands/util/linkCode.js`, the same rules as the proxy's +`codes.ts`). The indexes are created here under the proxy's exact names and options (`link_code` +unique, `link_uuid`, `link_ttl` on `expiresAt`), so whichever side gets there first the other finds +them already right. **`bifrost.discord_link_audit`** (history, nothing reads it in flight): `uuid`, `discordId`, `action` (`link` / `unlink`), `by: 'discord'`, `discordName`, `at`, plus `actor` when staff undid diff --git a/discord/commands/link.js b/discord/commands/link.js index 9f0db91..389d0da 100644 --- a/discord/commands/link.js +++ b/discord/commands/link.js @@ -9,6 +9,12 @@ * One Discord may hold several Minecraft accounts; one Minecraft account holds at most * one Discord (a second Discord has to wait for an in-game /unlink). Any member can run * it, replies are ephemeral, and the optional Verified role never fails a link. + * + * The code is CLAIMED before anything else: reading it first and burning it after let two + * Discords redeeming the same code both pass the check and both write the player. The + * write is guarded the same way, so an in-game link landing in between loses the race + * rather than getting overwritten - a refusal spends the code, and the player just runs + * /link in game again. */ const { SlashCommandBuilder } = require('discord.js'); @@ -38,7 +44,8 @@ module.exports = { return; } - const codeDoc = await mongo.findLinkCode(code); + // Claim first - one atomic write decides who holds this code. + const codeDoc = await mongo.claimLinkCode(code, interaction.user.id); if (!codeDoc || !codeDoc.uuid) { await interaction.editReply(BAD_CODE); return; @@ -52,25 +59,25 @@ module.exports = { } const username = player.username || codeDoc.username || 'your account'; - const linkedTo = player.discord_id == null ? null : String(player.discord_id); - if (linkedTo === interaction.user.id) { - // Already theirs: burn the code so it can't be reused, change nothing else. - await mongo.markLinkCodeUsed(code, interaction.user.id); - await interaction.editReply(`✅ **${username}** is already linked to this Discord account.`); - return; - } - if (linkedTo) { - await interaction.editReply( - `❌ **${username}** is linked to another Discord account — run \`/unlink\` in game first, then use a fresh code.`); - return; - } - - await mongo.setBifrostDiscordLink(codeDoc.uuid, { + const result = await mongo.setBifrostDiscordLink(codeDoc.uuid, { discordId: interaction.user.id, discordName: interaction.user.username }); - await mongo.markLinkCodeUsed(code, interaction.user.id); + + if (result && result.matchedCount === 0) { + // The account was linked between the claim and the write. Never overwrite it - + // the code is spent either way, so say what to do about it. + const current = await mongo.getBifrostPlayerByUuid(codeDoc.uuid); + const linkedTo = !current || current.discord_id == null ? null : String(current.discord_id); + if (linkedTo === interaction.user.id) { + await interaction.editReply(`✅ **${username}** is already linked to this Discord account.`); + return; + } + await interaction.editReply( + `❌ **${username}** is linked to another Discord account — run \`/unlink\` in game first, then \`/link\` again for a fresh code.`); + return; + } try { await mongo.insertLinkAudit(buildLinkAudit({ diff --git a/discord/commands/util/linkCode.js b/discord/commands/util/linkCode.js index 51e3d48..0ea7b1e 100644 --- a/discord/commands/util/linkCode.js +++ b/discord/commands/util/linkCode.js @@ -7,8 +7,9 @@ * base32 chars - the alphabet drops I, L, O and U so a player reading one off a chat * card can't turn it into a different code by mistyping. * - * Normalisation forgives what people actually type: lower case, spaces and dashes, and - * the three lookalikes (O for zero, I and L for one). Anything else is simply not a code. + * Normalisation forgives what people actually type: lower case, spaces, dashes (including + * the ones a phone keyboard autocorrects into U+2010-U+2015), underscores, and the three + * lookalikes (O for zero, I and L for one). Anything else is simply not a code. * (The commands/ loader only picks up top-level files, so this util is never a command.) */ @@ -24,7 +25,7 @@ const CODE_RE = /^[0-9A-HJKMNP-TV-Z]{6}$/; function normalizeCode(input) { return String(input == null ? '' : input) .toUpperCase() - .replace(/[\s\-‐-―]+/g, '') + .replace(/[\s\-_‐-―]+/g, '') .replace(/O/g, '0') .replace(/[IL]/g, '1'); } diff --git a/modules/mongo.js b/modules/mongo.js index 5c58386..6e3b01d 100644 --- a/modules/mongo.js +++ b/modules/mongo.js @@ -25,7 +25,15 @@ const { const mongoClient = new MongoClient(process.env.MONGODB_URL); let mainClientConnected = false; -// The discord-link indexes are created once per process, on the first code lookup. +// The discord-link indexes are created once per process, on the first code claim. The +// three code indexes are the proxy's specs verbatim (src/plugins/discord-link/index.ts) - +// same names, same options, so whichever side gets there first the other finds them right. +const DISCORD_LINK_INDEXES = [ + { collection: 'players', keys: { discord_id: 1 }, options: { sparse: true } }, + { collection: 'discord_link_codes', keys: { code: 1 }, options: { name: 'link_code', unique: true } }, + { collection: 'discord_link_codes', keys: { uuid: 1 }, options: { name: 'link_uuid' } }, + { collection: 'discord_link_codes', keys: { expiresAt: 1 }, options: { name: 'link_ttl', expireAfterSeconds: 0 } } +]; let discordLinkIndexesEnsured = false; // bifrost.logs starts here; anything older lives only in valhallamc.logs (the archive) @@ -1038,48 +1046,35 @@ module.exports = { // bifrost.players, so the confirmation card follows the write here in about a second. // `discord_id` is ALWAYS a string: a snowflake does not survive a JS number. /** - * Finds an unused, unexpired link code. + * Claims a code: the lookup and the burn are ONE write, so two Discords racing the + * same code can never both walk away holding it. Reading first and burning after is + * how the same code linked twice. * @param {string} code Normalised code (discord/commands/util/linkCode.js). - * @returns {Promise} The code doc or null. + * @param {string} discordId Who is claiming it. + * @returns {Promise} The claimed doc, or null when the code was already + * used, has expired, or never existed. */ - findLinkCode: async function (code) { + claimLinkCode: async function (code, discordId) { if (!mainClientConnected) { await mongoClient.connect(); mainClientConnected = true; } await module.exports.ensureDiscordLinkIndexes(); + // driver 6 hands back the document itself, not a {value} wrapper return mongoClient .db('bifrost') .collection('discord_link_codes') - .findOne({ + .findOneAndUpdate({ code: String(code), usedAt: null, expiresAt: { $gt: new Date() } - }); - }, - - /** - * Burns a code so it can only ever link once. - * @param {string} code Normalised code. - * @param {string} discordId Who used it. - * @returns {Promise} The updateOne result. - */ - markLinkCodeUsed: async function (code, discordId) { - if (!mainClientConnected) { - await mongoClient.connect(); - mainClientConnected = true; - } - - return mongoClient - .db('bifrost') - .collection('discord_link_codes') - .updateOne({ code: String(code), usedAt: null }, { + }, { $set: { usedAt: new Date(), usedBy: String(discordId) } - }); + }, { returnDocument: 'after' }); }, /** @@ -1128,10 +1123,13 @@ module.exports = { }, /** - * Writes the link onto the player doc. The proxy reads these three fields. + * Writes the link onto the player doc, but only while that account is still free. + * The filter is what makes a race lose instead of overwrite - an in-game link landing + * between the claim and this write used to be silently replaced. * @param {string} uuid Dashed uuid of the Minecraft account. * @param {object} link `{discordId, discordName}`. - * @returns {Promise} The updateOne result. + * @returns {Promise} The updateOne result - `matchedCount` 0 means somebody + * else got there first and the caller must refuse. */ setBifrostDiscordLink: async function (uuid, link) { if (!mainClientConnected) { @@ -1139,10 +1137,12 @@ module.exports = { mainClientConnected = true; } + // $in:[null] is 'missing or null' - a doc from before the field existed matches too return mongoClient .db('bifrost') .collection('players') - .updateOne({ uuid: String(uuid) }, { $set: buildLinkFields(link) }); + .updateOne({ uuid: String(uuid), discord_id: { $in: [null] } }, + { $set: buildLinkFields(link) }); }, /** @@ -1186,27 +1186,42 @@ module.exports = { }, /** - * Creates the indexes the link flow needs, once per process. The proxy creates the - * same ones, so the specs are kept identical (default names, same options) and a - * conflict is logged rather than thrown - an index is not worth a failed link. + * Creates the indexes the link flow needs, once per process. Each spec is attempted + * on its own (one failure used to skip every later one for the life of the process) + * and the ensured flag is only set once they all landed, so the next lookup retries. + * @param {object} [db] Bifrost db handle - only tests pass one. * @returns {Promise} Resolves when the attempt is done. */ - ensureDiscordLinkIndexes: async function () { + ensureDiscordLinkIndexes: async function (db) { if (discordLinkIndexesEnsured) return; - discordLinkIndexesEnsured = true; - if (!mainClientConnected) { - await mongoClient.connect(); - mainClientConnected = true; + if (!db) { + if (!mainClientConnected) { + await mongoClient.connect(); + mainClientConnected = true; + } + db = mongoClient.db('bifrost'); } - try { - const db = mongoClient.db('bifrost'); - await db.collection('players').createIndex({ discord_id: 1 }, { sparse: true }); - await db.collection('discord_link_codes').createIndex({ code: 1 }, { unique: true }); - } catch (error) { - sessionLogger.warn('Mongo', 'Could not ensure the discord-link indexes', error.message); + let allOk = true; + for (const spec of DISCORD_LINK_INDEXES) { + try { + await db.collection(spec.collection).createIndex(spec.keys, spec.options); + } catch (error) { + // 85/86: the same keys already exist under another name or options. A retry + // can never fix that, so stop asking - a human has to drop the old index. + if (error && (error.code === 85 || error.code === 86)) { + sessionLogger.warn('Mongo', + `The ${spec.collection} link index already exists differently (code ${error.code})`, + error.message); + continue; + } + allOk = false; + sessionLogger.warn('Mongo', + `Could not ensure a ${spec.collection} discord-link index`, error.message); + } } + discordLinkIndexesEnsured = allOk; }, /** diff --git a/test/link.test.js b/test/link.test.js index 01f8d2b..8fb8f0e 100644 --- a/test/link.test.js +++ b/test/link.test.js @@ -8,6 +8,10 @@ * can only link once, and an audit row. A Minecraft account already linked to another * Discord is never overwritten, and the optional Verified role can fail all it likes - * the link is in Mongo and stays there. + * + * The two races are covered here because neither shows up in a single-caller test: the + * code is claimed atomically (two Discords, one code, one link) and the player write is + * filtered on the account still being free (an in-game link landing mid-flow wins). */ const { test, beforeEach } = require('node:test'); @@ -18,15 +22,17 @@ const link = require('../discord/commands/link'); const unlink = require('../discord/commands/unlink'); const linked = require('../discord/commands/linked'); const mongo = require('../modules/mongo'); +// captured before beforeEach stubs it out - the index test drives the real one +const ensureDiscordLinkIndexes = mongo.ensureDiscordLinkIndexes; const NOW = new Date('2026-08-17T12:00:00Z'); -let codeDocs; // code -> doc returned by findLinkCode +let codeDocs; // code -> doc in bifrost.discord_link_codes let players; // uuid -> bifrost.players doc let identities; // lowercased username -> doc for getPlayerIdentity let sets; // setBifrostDiscordLink calls let unsets; // unsetBifrostDiscordLink calls -let used; // markLinkCodeUsed calls +let claims; // claimLinkCode calls let audits; // insertLinkAudit docs let roleCalls; // {action, userId, roleId} from the fake guild let roleFetchThrows; @@ -43,22 +49,35 @@ beforeEach(() => { identities = {}; sets = []; unsets = []; - used = []; + claims = []; audits = []; roleCalls = []; roleFetchThrows = false; configuredRole = null; - mongo.findLinkCode = async (code) => codeDocs[code] || null; + // the real one is ONE findOneAndUpdate: the read and the burn cannot interleave, so + // this fake must not await before it mutates either + mongo.claimLinkCode = async (code, discordId) => { + const doc = codeDocs[code]; + if (!doc || doc.usedAt) return null; + doc.usedAt = NOW; + doc.usedBy = String(discordId); + claims.push({ code, discordId }); + return doc; + }; mongo.getBifrostPlayerByUuid = async (uuid) => players[uuid] || null; mongo.getPlayerIdentity = async (name) => identities[String(name).toLowerCase()] || null; mongo.findBifrostPlayersByDiscordId = async (discordId) => Object.values(players).filter(p => p.discord_id === String(discordId)); mongo.setBifrostDiscordLink = async (uuid, linkFields) => { + // the real filter is {uuid, discord_id: {$in: [null]}} - an account that is already + // linked matches nothing, and that is what stops an overwrite + const player = players[uuid]; + if (!player || player.discord_id != null) return { matchedCount: 0, modifiedCount: 0 }; sets.push({ uuid, ...linkFields }); // exactly what modules/mongo.js $sets - the field shape is the proxy's contract - players[uuid] = { ...players[uuid], ...codes.buildLinkFields(linkFields) }; - return { modifiedCount: 1 }; + players[uuid] = { ...player, ...codes.buildLinkFields(linkFields) }; + return { matchedCount: 1, modifiedCount: 1 }; }; mongo.unsetBifrostDiscordLink = async (uuid) => { unsets.push(uuid); @@ -69,11 +88,6 @@ beforeEach(() => { } return { modifiedCount: 1 }; }; - mongo.markLinkCodeUsed = async (code, discordId) => { - used.push({ code, discordId }); - if (codeDocs[code]) delete codeDocs[code]; // burnt: a second lookup finds nothing - return { modifiedCount: 1 }; - }; mongo.insertLinkAudit = async (doc) => { audits.push(doc); return { insertedId: 'a' }; }; mongo.ensureDiscordLinkIndexes = async () => {}; @@ -111,12 +125,21 @@ function interaction(options, opts = {}) { }; } -test('link code normalisation: case, spaces, dashes and the O/I/L lookalikes', () => { - assert.strictEqual(codes.normalizeCode(' abc-2 34 '), 'ABC234'); - assert.strictEqual(codes.normalizeCode('abc234'), 'ABC234'); - assert.strictEqual(codes.normalizeCode('oil234'), '011234'); - assert.strictEqual(codes.normalizeCode('ABC—234'), 'ABC234', 'an em dash is a dash too'); - assert.strictEqual(codes.normalizeCode(null), ''); +test('link code normalisation: case, spaces, dashes, underscores and the O/I/L lookalikes', () => { + const table = [ + ['abc-234', 'ABC234'], + ['ABC 234', 'ABC234'], + ['abc_234', 'ABC234', 'the proxy strips _ as well - a copied code often carries one'], + ['ABC\u2010234', 'ABC234', 'U+2010, what a phone keyboard makes of a hyphen'], + ['ABC\u2015234', 'ABC234', 'U+2015 is the top of the dash range'], + [' abc-2 34 ', 'ABC234'], + ['abc234', 'ABC234'], + ['oil234', '011234'], + [null, ''] + ]; + for (const [input, want, why] of table) { + assert.strictEqual(codes.normalizeCode(input), want, why || JSON.stringify(input)); + } }); test('link code validity: 6 Crockford chars, no I L O U', () => { @@ -172,7 +195,7 @@ test('/link writes discord_id as a STRING, burns the code and audits it', async assert.strictEqual(players['uuid-alp'].discord_name, 'alpdiscord'); assert.ok(players['uuid-alp'].discord_linked_at instanceof Date); - assert.deepStrictEqual(used, [{ code: 'ABC234', discordId: '4242' }]); + assert.deepStrictEqual(claims, [{ code: 'ABC234', discordId: '4242' }]); assert.strictEqual(audits.length, 1); assert.strictEqual(audits[0].uuid, 'uuid-alp'); assert.strictEqual(audits[0].discordId, '4242'); @@ -185,7 +208,7 @@ test('/link writes discord_id as a STRING, burns the code and audits it', async test('/link refuses a malformed code before it reaches Mongo', async () => { let looked = 0; - mongo.findLinkCode = async () => { looked++; return null; }; + mongo.claimLinkCode = async () => { looked++; return null; }; const it = interaction({ code: 'nope' }); await link.execute(it); @@ -194,13 +217,13 @@ test('/link refuses a malformed code before it reaches Mongo', async () => { assert.match(it.replies[0], /not valid or has expired/); }); -test('/link refuses an expired or already-used code (the lookup filters both)', async () => { - codeDocs = {}; // findLinkCode only returns usedAt:null and expiresAt in the future +test('/link refuses an expired or already-used code (the claim filters both)', async () => { + codeDocs = {}; // the claim only matches usedAt:null and expiresAt in the future const it = interaction({ code: 'ABC234' }); await link.execute(it); assert.strictEqual(sets.length, 0); - assert.strictEqual(used.length, 0); + assert.strictEqual(claims.length, 0); assert.strictEqual(audits.length, 0); assert.match(it.replies[0], /not valid or has expired/); }); @@ -211,11 +234,52 @@ test('/link refuses a Minecraft account already linked to another Discord', asyn await link.execute(it); assert.strictEqual(sets.length, 0, 'never overwrite someone else`s link'); - assert.strictEqual(used.length, 0, 'and never burn the code doing it'); assert.strictEqual(audits.length, 0); assert.strictEqual(players['uuid-taken'].discord_id, '999'); + assert.strictEqual(claims.length, 1, 'the claim came first, so the code is spent'); assert.match(it.replies[0], /another Discord account/); assert.match(it.replies[0], /unlink/); + assert.match(it.replies[0], /`\/link` again/, 'the code is burnt - say how to get another'); +}); + +test('two Discords redeeming the same code: exactly one link and one role', async () => { + configuredRole = 'role-1'; + const first = interaction({ code: 'ABC234' }, { userId: '4242', username: 'alpdiscord' }); + const second = interaction({ code: 'ABC234' }, { userId: '5555', username: 'someoneelse' }); + + // reading the code and burning it afterwards let both of these through the check + await Promise.all([link.execute(first), link.execute(second)]); + + assert.strictEqual(claims.length, 1, 'the claim is the atomic step - one of them wins it'); + assert.strictEqual(sets.length, 1); + assert.strictEqual(audits.length, 1); + assert.strictEqual(roleCalls.length, 1, 'the loser is not verified either'); + const winner = sets[0].discordId; + assert.strictEqual(players['uuid-alp'].discord_id, winner); + assert.deepStrictEqual(roleCalls, [{ action: 'add', userId: winner, roleId: 'role-1' }]); + const loser = winner === '4242' ? second : first; + assert.match(loser.replies[0], /not valid or has expired/); +}); + +test('a link landing between the claim and the write is refused, never overwritten', async () => { + configuredRole = 'role-1'; + let reads = 0; + mongo.getBifrostPlayerByUuid = async (uuid) => { + const snapshot = players[uuid] ? { ...players[uuid] } : null; + // in game /link (or the legacy import) lands right after this read + if (++reads === 1) players[uuid] = { ...players[uuid], discord_id: '999', discord_name: 'someone' }; + return snapshot; + }; + + const it = interaction({ code: 'ABC234' }); + await link.execute(it); + + assert.strictEqual(sets.length, 0, 'the filtered write loses the race instead of winning it'); + assert.strictEqual(players['uuid-alp'].discord_id, '999'); + assert.strictEqual(players['uuid-alp'].discord_name, 'someone'); + assert.strictEqual(audits.length, 0); + assert.deepStrictEqual(roleCalls, []); + assert.match(it.replies[0], /another Discord account/); }); test('/link twice from the same Discord is idempotent — one write, one audit', async () => { @@ -227,7 +291,7 @@ test('/link twice from the same Discord is idempotent — one write, one audit', assert.strictEqual(sets.length, 1); assert.strictEqual(audits.length, 1); - assert.strictEqual(used.length, 2, 'the second code is still burnt'); + assert.strictEqual(claims.length, 2, 'the second code is still burnt'); assert.match(second.replies[0], /already linked/); }); @@ -348,3 +412,38 @@ test('/linked lists the caller`s accounts, and says so when there are none', asy assert.match(it.replies[0], /\*\*Alp\*\*/); assert.match(it.replies[0], //); }); + +test('the discord-link indexes are the proxy`s specs, and one failure is retried', async () => { + const calls = []; + let failFirst = true; + const db = { + collection: (name) => ({ + createIndex: async (keys, options) => { + calls.push({ collection: name, keys, options }); + if (failFirst && calls.length === 1) throw new Error('no primary'); + return name; + } + }) + }; + + await ensureDiscordLinkIndexes(db); + assert.deepStrictEqual(calls, [ + { collection: 'players', keys: { discord_id: 1 }, options: { sparse: true } }, + { collection: 'discord_link_codes', keys: { code: 1 }, options: { name: 'link_code', unique: true } }, + { collection: 'discord_link_codes', keys: { uuid: 1 }, options: { name: 'link_uuid' } }, + { + collection: 'discord_link_codes', + keys: { expiresAt: 1 }, + options: { name: 'link_ttl', expireAfterSeconds: 0 } + } + ], 'same names and options as src/plugins/discord-link, and a throw skips no later spec'); + + failFirst = false; + calls.length = 0; + await ensureDiscordLinkIndexes(db); + assert.strictEqual(calls.length, 4, 'nothing was marked ensured while one spec was missing'); + + calls.length = 0; + await ensureDiscordLinkIndexes(db); + assert.deepStrictEqual(calls, [], 'all four landed - once per process is enough'); +});