From 85267a0df01c136293031eb593e7726183b2131c Mon Sep 17 00:00:00 2001 From: Fabio1988 Date: Fri, 9 Oct 2026 19:10:02 +0200 Subject: [PATCH 1/6] fix(auth): link Discord and Telegram for users signed in with either The profile offers "Link Discord" / "Link Telegram" to every signed-in user, but the server only linked when the signed-in row was a local account. Discord and Telegram users got a button that did nothing, and worse: - Telegram checked the row found by the Telegram id instead of the signed-in user, so linking Telegram never worked, not even for local accounts. - Linking Discord while signed in with Telegram rewrote the Telegram row's strategy to "discord" without storing a Discord id. Both clients now share linkAccount(): when a signed-in user authenticates with a platform their row has no id for, the id is attached to their row. A row that only existed for that account is merged in (gym badges keeping the higher badge, backups, nest submissions, profile data) and deleted, in one transaction, and its sessions are cleared. A local account, or one already linked to a different account of the other platform, is never merged away; the user lands on the blocked page with an explanation instead. Signing in with an account the row already has stays a plain login. Webhooks keep following the sign-in method: a Discord sign-in alerts the Discord account and a Telegram sign-in the Telegram one, so the webhook strategy selector stays reserved for local accounts. Co-Authored-By: Claude Opus 5.5 --- packages/locales/lib/human/en.json | 1 + server/src/services/DiscordClient.js | 82 ++++++------ server/src/services/TelegramClient.js | 65 +++++----- server/src/services/linkAccount.js | 174 ++++++++++++++++++++++++++ server/test/linkAccount.test.js | 138 ++++++++++++++++++++ src/features/profile/LinkAccounts.jsx | 3 +- src/pages/Blocked.jsx | 3 +- 7 files changed, 388 insertions(+), 78 deletions(-) create mode 100644 server/src/services/linkAccount.js create mode 100644 server/test/linkAccount.test.js diff --git a/packages/locales/lib/human/en.json b/packages/locales/lib/human/en.json index b722b2c23..6abcfe1f3 100644 --- a/packages/locales/lib/human/en.json +++ b/packages/locales/lib/human/en.json @@ -736,6 +736,7 @@ "showcase_block": "Blocked due to a showcase", "signed_in_as": "Signed In As", "missing_map_perm": "You do not have permission to view the map.", + "account_already_linked": "That account is already linked to another map account and was not linked to yours. Sign in with it to use that account instead.", "access_denied": "Access Denied", "audio": "Audio", "desktop_notifications": "Desktop Notifications", diff --git a/server/src/services/DiscordClient.js b/server/src/services/DiscordClient.js index f34489106..fa2cb1fe7 100644 --- a/server/src/services/DiscordClient.js +++ b/server/src/services/DiscordClient.js @@ -9,8 +9,8 @@ const { logUserAuth } = require('./logUserAuth') const { areaPerms } = require('../utils/areaPerms') const { webhookPerms } = require('../utils/webhookPerms') const { scannerPerms, scannerCooldownBypass } = require('../utils/scannerPerms') -const { mergePerms } = require('../utils/mergePerms') const { AuthClient } = require('./AuthClient') +const { linkAccount, sessionUser } = require('./linkAccount') const { state } = require('./state') class DiscordClient extends AuthClient { @@ -328,54 +328,44 @@ class DiscordClient extends AuthClient { delete discordUser.guilds } + const selectedWebhook = Object.keys(state.event.webhookObj).find((x) => + discordUser?.perms?.webhooks.includes(x), + ) + if (req.user) { + const link = await linkAccount(state.db.models, { + userId: req.user.id, + platform: 'discord', + externalId: discordUser.id, + perms: discordUser.perms, + sessionId: req.sessionID, + }) + if (link.action === 'refuse') { + this.log.warn( + discordUser.username, + `(${discordUser.id})`, + 'is already linked to another account, not linking it to user', + req.user.id, + ) + return done(null, undefined, { message: 'account_already_linked' }) + } + if (link.action === 'link') { + this.log.info( + discordUser.username, + `(${discordUser.id})`, + 'linked to user', + req.user.id, + ) + return done(null, { + selectedWebhook, + ...sessionUser(req.user, discordUser, link.linked), + }) + } + } + await state.db.models.User.query() - .findOne(req.user ? { id: req.user.id } : { discordId: discordUser.id }) + .findOne({ discordId: discordUser.id }) .then( async (/** @type {import('@rm/types').FullUser} */ userExists) => { - const selectedWebhook = Object.keys(state.event.webhookObj).find( - (x) => discordUser?.perms?.webhooks.includes(x), - ) - if (req.user && userExists?.strategy === 'local') { - await state.db.models.User.query() - .update({ - discordId: discordUser.id, - discordPerms: JSON.stringify(discordUser.perms), - webhookStrategy: 'discord', - }) - .where('id', req.user.id) - /** @type {import('@rm/types').FullUser} */ - const oldUser = await state.db.models.User.query() - .where('discordId', discordUser.id) - .whereNot('id', req.user.id) - .first() - if (oldUser) { - await state.db.models.Badge.query() - .update({ - // @ts-ignore - userId: req.user.id, - }) - .where('userId', oldUser.id) - await state.db.models.User.query() - .update({ - data: oldUser.data, - }) - .where('id', req.user.id) - .where('data', null) - } - await state.db.models.User.query() - .where('discordId', discordUser.id) - .whereNot('id', req.user.id) - .delete() - return done(null, { - selectedWebhook, - ...discordUser, - ...req.user, - username: userExists.username || discordUser.username, - discordId: discordUser.id, - perms: mergePerms(req.user.perms, discordUser.perms), - }) - } - if (!userExists) { userExists = await state.db.models.User.query().insertAndFetch({ discordId: discordUser.id, diff --git a/server/src/services/TelegramClient.js b/server/src/services/TelegramClient.js index 96ffd691d..3f4e78d3b 100644 --- a/server/src/services/TelegramClient.js +++ b/server/src/services/TelegramClient.js @@ -11,10 +11,10 @@ const { state } = require('./state') const { areaPerms } = require('../utils/areaPerms') const { webhookPerms } = require('../utils/webhookPerms') const { scannerPerms, scannerCooldownBypass } = require('../utils/scannerPerms') -const { mergePerms } = require('../utils/mergePerms') const { getUserDisplayName } = require('../utils/getUserDisplayName') const { isOAuthStrategy } = require('../utils/getTelegramStrategy') const { AuthClient } = require('./AuthClient') +const { linkAccount, sessionUser } = require('./linkAccount') /** * @typedef {import('@rainb0w-clwn/passport-telegram-official/dist/types').PassportTelegramUser} TGUser @@ -167,39 +167,44 @@ class TelegramClient extends AuthClient { return done(null, false, { message: 'access_denied' }) } try { + const selectedWebhook = Object.keys(state.event.webhookObj).find((x) => + user?.perms?.webhooks.includes(x), + ) + if (req.user) { + const link = await linkAccount(state.db.models, { + userId: req.user.id, + platform: 'telegram', + externalId: user.id, + perms: user.perms, + sessionId: req.sessionID, + }) + if (link.action === 'refuse') { + this.log.warn( + user.username, + `(${user.id})`, + 'is already linked to another account, not linking it to user', + req.user.id, + ) + return done(null, false, { message: 'account_already_linked' }) + } + if (link.action === 'link') { + this.log.info( + user.username, + `(${user.id})`, + 'linked to user', + req.user.id, + ) + return done(null, { + selectedWebhook, + ...sessionUser(req.user, user, link.linked), + }) + } + } + await state.db.models.User.query() .findOne({ telegramId: user.id }) .then( async (/** @type {import('@rm/types').FullUser} */ userExists) => { - const selectedWebhook = Object.keys(state.event.webhookObj).find( - (x) => user?.perms?.webhooks.includes(x), - ) - if (req.user && userExists?.strategy === 'local') { - await state.db.models.User.query() - .update({ - telegramId: user.id, - telegramPerms: JSON.stringify(user.perms), - webhookStrategy: 'telegram', - }) - .where('id', req.user.id) - await state.db.models.User.query() - .where('telegramId', user.id) - .whereNot('id', req.user.id) - .delete() - this.log.info( - user.username, - `(${user.id})`, - 'Authenticated successfully.', - ) - return done(null, { - selectedWebhook, - ...user, - ...req.user, - username: userExists.username || user.username, - telegramId: user.id, - perms: mergePerms(req.user.perms, user.perms), - }) - } if (!userExists) { userExists = await state.db.models.User.query().insertAndFetch({ telegramId: user.id, diff --git a/server/src/services/linkAccount.js b/server/src/services/linkAccount.js new file mode 100644 index 000000000..6c0734f9d --- /dev/null +++ b/server/src/services/linkAccount.js @@ -0,0 +1,174 @@ +// @ts-check +const { mergePerms } = require('../utils/mergePerms') + +/** @typedef {'discord' | 'telegram'} Platform */ +/** + * @typedef {{ action: 'login' } + * | { action: 'refuse' } + * | { action: 'link', merge: import('@rm/types').FullUser | null }} LinkPlan + */ + +/** @type {Record} */ +const OTHER = { discord: 'telegram', telegram: 'discord' } + +/** + * Decides what signing in with `platform` does for a visitor who is already + * logged in as `current`. `other` is the row that already holds the incoming + * platform id, if any. + * + * - `login`: nothing to link, the row already has that platform (the same + * account re-authenticating, or a different one, which switches accounts) + * - `link`: attach the id to `current`, absorbing `other` when there is one + * - `refuse`: `other` is a whole account of its own, merging it would lose it + * + * @param {import('@rm/types').FullUser | null | undefined} current + * @param {import('@rm/types').FullUser | null | undefined} other + * @param {Platform} platform + * @returns {LinkPlan} + */ +function planLink(current, other, platform) { + if (!current || current[`${platform}Id`]) return { action: 'login' } + if (!other) return { action: 'link', merge: null } + + // a local account has a password a merge would throw away, and its strategy + // may read discord or telegram after the user last signed in with one + if (other.strategy === 'local' || other.password) return { action: 'refuse' } + + const otherField = `${OTHER[platform]}Id` + if ( + other[otherField] && + current[otherField] && + other[otherField] !== current[otherField] + ) { + return { action: 'refuse' } + } + return { action: 'link', merge: other } +} + +/** + * The session user after linking: still the logged in row, with the linked + * ids and both platforms' perms + * + * @template {{ perms: import('@rm/types').Permissions, username?: string }} T + * @param {import('@rm/types').User} reqUser + * @param {T} platformUser + * @param {{ discordId?: string, telegramId?: string, webhookStrategy?: string }} linked + */ +function sessionUser(reqUser, platformUser, linked) { + return { + ...platformUser, + ...reqUser, + ...linked, + username: reqUser.username || platformUser.username, + perms: mergePerms(reqUser.perms, platformUser.perms), + } +} + +/** + * Moves the gym badges of `fromId` to `toId`, keeping the higher badge where + * both have one for the same gym + * + * @param {import('@rm/types').Models['Badge']} Badge + * @param {number} fromId + * @param {number} toId + * @param {import('objection').Transaction} trx + */ +async function moveBadges(Badge, fromId, toId, trx) { + const [mine, theirs] = await Promise.all([ + Badge.query(trx).where('userId', toId), + Badge.query(trx).where('userId', fromId), + ]) + const byGym = new Map(mine.map((badge) => [badge.gymId, badge])) + await Promise.all( + theirs.map(async (badge) => { + const own = byGym.get(badge.gymId) + if (!own) { + return Badge.query(trx).update({ userId: toId }).where('id', badge.id) + } + if (badge.badge > own.badge) { + await Badge.query(trx) + .update({ badge: badge.badge }) + .where('id', own.id) + } + return Badge.query(trx).delete().where('id', badge.id) + }), + ) +} + +/** + * Links a Discord or Telegram account to the row of the user who is signed in. + * A row that only existed for that account is merged in and deleted. + * + * @param {Pick} models + * @param {{ + * userId: number, + * platform: Platform, + * externalId: string, + * perms: import('@rm/types').Permissions, + * sessionId?: string, + * }} options + * @returns {Promise} + */ +async function linkAccount( + { User, Badge, Backup, NestSubmission, Session }, + { userId, platform, externalId, perms, sessionId }, +) { + const field = `${platform}Id` + const otherPlatform = OTHER[platform] + const otherField = `${otherPlatform}Id` + + const [current, other] = await Promise.all([ + User.query().findOne({ id: userId }), + User.query().findOne({ [field]: externalId }), + ]) + const plan = planLink(current, other, platform) + if (plan.action !== 'link') return plan + + const { merge } = plan + const update = { + [field]: externalId, + [`${platform}Perms`]: JSON.stringify(perms), + // only a local sign-in reads this, Discord and Telegram sign-ins always + // alert the account they used; keep a local user's alerts where they were + webhookStrategy: + current.webhookStrategy || + (current[otherField] ? otherPlatform : platform), + } + if (merge) { + if (!current.data && merge.data) update.data = merge.data + if (!current[otherField] && merge[otherField]) { + update[otherField] = merge[otherField] + update[`${otherPlatform}Perms`] = merge[`${otherPlatform}Perms`] + } + } + + await User.transaction(async (trx) => { + if (merge) { + await moveBadges(Badge, merge.id, current.id, trx) + await Backup.query(trx) + .update({ userId: current.id }) + .where('userId', merge.id) + await NestSubmission.query(trx) + .update({ user_id: current.id }) + .where('user_id', merge.id) + await User.query(trx).delete().where('id', merge.id) + } + await User.query(trx).update(update).where('id', current.id) + }) + + if (merge) { + // sessions of the deleted row point at a user that no longer exists + await Session.clearOtherSessions(merge.id, sessionId).catch(() => {}) + } + + return { + ...plan, + linked: { + discordId: update.discordId ?? current.discordId, + telegramId: update.telegramId ?? current.telegramId, + webhookStrategy: update.webhookStrategy, + }, + } +} + +module.exports = { linkAccount, planLink, sessionUser } diff --git a/server/test/linkAccount.test.js b/server/test/linkAccount.test.js new file mode 100644 index 000000000..700eaf591 --- /dev/null +++ b/server/test/linkAccount.test.js @@ -0,0 +1,138 @@ +const assert = require('node:assert/strict') +const { test } = require('node:test') + +const { planLink, sessionUser } = require('../src/services/linkAccount') + +const discord = (id, extra = {}) => ({ + id, + strategy: 'discord', + discordId: `d${id}`, + ...extra, +}) +const telegram = (id, extra = {}) => ({ + id, + strategy: 'telegram', + telegramId: `t${id}`, + ...extra, +}) + +test('without a logged in row it is a plain login', () => { + assert.deepEqual(planLink(null, null, 'telegram'), { action: 'login' }) +}) + +test('a row that already has that platform logs in instead of linking', () => { + assert.deepEqual(planLink(telegram(1), null, 'telegram'), { + action: 'login', + }) + // a different account of the same platform switches accounts + assert.deepEqual(planLink(telegram(1), telegram(2), 'telegram'), { + action: 'login', + }) +}) + +test('a discord user links telegram that has no row yet', () => { + assert.deepEqual(planLink(discord(1), null, 'telegram'), { + action: 'link', + merge: null, + }) +}) + +test('a discord user links telegram and absorbs its telegram-only row', () => { + const other = telegram(2) + assert.deepEqual(planLink(discord(1), other, 'telegram'), { + action: 'link', + merge: other, + }) +}) + +test('a telegram user links discord and absorbs its discord-only row', () => { + const other = discord(2) + assert.deepEqual(planLink(telegram(1), other, 'discord'), { + action: 'link', + merge: other, + }) +}) + +test('a local user links like before', () => { + const local = { id: 1, strategy: 'local', username: 'me' } + const other = discord(2) + assert.deepEqual(planLink(local, other, 'discord'), { + action: 'link', + merge: other, + }) +}) + +test('a local account is never merged away', () => { + const other = { + id: 2, + strategy: 'local', + username: 'x', + password: 'hash', + discordId: 'd2', + } + assert.deepEqual(planLink(telegram(1), other, 'discord'), { + action: 'refuse', + }) +}) + +test('an account linked to a different account of the other platform is refused', () => { + const other = discord(2, { telegramId: 't7' }) + assert.deepEqual(planLink(telegram(1), other, 'discord'), { + action: 'refuse', + }) +}) + +test('the other row brings its link along when the current row has none', () => { + const local = { id: 1, strategy: 'local', username: 'me' } + const other = discord(2, { telegramId: 't7' }) + assert.deepEqual(planLink(local, other, 'discord'), { + action: 'link', + merge: other, + }) +}) + +test('a local account that last signed in with discord is still never merged away', () => { + const other = discord(2, { username: 'x', password: 'hash' }) + assert.deepEqual(planLink(telegram(1), other, 'discord'), { + action: 'refuse', + }) +}) + +test('a flipped row without the id its strategy claims does not count as linked', () => { + // a telegram row whose strategy the old discord link path set to discord + const flipped = { id: 1, strategy: 'discord', telegramId: 't1' } + assert.deepEqual(planLink(flipped, discord(2), 'discord'), { + action: 'link', + merge: discord(2), + }) +}) + +test('the session keeps the logged in row and gains the linked id', () => { + const reqUser = { + id: 1, + strategy: 'discord', + discordId: 'd1', + username: 'disc', + perms: { map: true, pokemon: true, webhooks: ['a'] }, + } + const platformUser = { + id: 't9', + username: 'tele', + provider: 'telegram', + perms: { map: true, raids: true, webhooks: ['b'] }, + } + const user = sessionUser(reqUser, platformUser, { + discordId: 'd1', + telegramId: 't9', + webhookStrategy: 'discord', + }) + assert.equal(user.id, 1) + assert.equal(user.strategy, 'discord') + assert.equal(user.username, 'disc') + assert.equal(user.discordId, 'd1') + assert.equal(user.telegramId, 't9') + assert.equal(user.webhookStrategy, 'discord') + assert.equal(user.perms.pokemon, true) + assert.equal(user.perms.raids, true) + assert.deepEqual([...user.perms.webhooks].sort(), ['a', 'b']) +}) diff --git a/src/features/profile/LinkAccounts.jsx b/src/features/profile/LinkAccounts.jsx index 3408e4364..0a0c41b96 100644 --- a/src/features/profile/LinkAccounts.jsx +++ b/src/features/profile/LinkAccounts.jsx @@ -61,7 +61,8 @@ export function LinkAccounts() { ) })} - {auth.discordId && auth.telegramId && ( + {/* Discord and Telegram sign-ins always alert the account they used */} + {auth.strategy === 'local' && auth.discordId && auth.telegramId && ( {t('select_webhook_strategy')} diff --git a/src/pages/Blocked.jsx b/src/pages/Blocked.jsx index c36affd73..0e5cdac55 100644 --- a/src/pages/Blocked.jsx +++ b/src/pages/Blocked.jsx @@ -32,6 +32,7 @@ export function BlockedPage() { const username = queryParams.get('username') const avatar = queryParams.get('avatar') const id = queryParams.get('id') + const alreadyLinked = queryParams.get('message') === 'account_already_linked' return ( ) : ( - {t('missing_map_perm')} + {t(alreadyLinked ? 'account_already_linked' : 'missing_map_perm')} )} {discordInvite && ( From 6219114ee71d966bb10e7f42f2ddafc79af07ab4 Mon Sep 17 00:00:00 2001 From: Fabio1988 Date: Fri, 9 Oct 2026 19:31:10 +0200 Subject: [PATCH 2/6] fix(auth): address review on account linking - Show the webhook strategy selector to local sessions again. They carry no strategy, so gating on strategy === 'local' hid it for everyone; gate on "not a Discord or Telegram sign-in" instead. - Read and lock both rows (FOR UPDATE, one query) inside the transaction that merges them, so concurrent link callbacks for the same user cannot both link. - Refuse to merge away a row that already has both ids. It is a linked account of its own, and carrying its second id over would carry it without that provider's permissions. - Hand the row read for a plain login back to the Discord and Telegram clients instead of querying it again. Co-Authored-By: Claude Opus 5.5 --- server/src/services/DiscordClient.js | 69 ++++++++++---------- server/src/services/TelegramClient.js | 75 +++++++++++----------- server/src/services/linkAccount.js | 92 +++++++++++++-------------- server/test/linkAccount.test.js | 25 +++++--- src/features/profile/LinkAccounts.jsx | 9 ++- 5 files changed, 144 insertions(+), 126 deletions(-) diff --git a/server/src/services/DiscordClient.js b/server/src/services/DiscordClient.js index fa2cb1fe7..fa2b9ab7e 100644 --- a/server/src/services/DiscordClient.js +++ b/server/src/services/DiscordClient.js @@ -331,8 +331,10 @@ class DiscordClient extends AuthClient { const selectedWebhook = Object.keys(state.event.webhookObj).find((x) => discordUser?.perms?.webhooks.includes(x), ) + /** @type {Awaited> | undefined} */ + let link if (req.user) { - const link = await linkAccount(state.db.models, { + link = await linkAccount(state.db.models, { userId: req.user.id, platform: 'discord', externalId: discordUser.id, @@ -362,38 +364,39 @@ class DiscordClient extends AuthClient { } } - await state.db.models.User.query() - .findOne({ discordId: discordUser.id }) - .then( - async (/** @type {import('@rm/types').FullUser} */ userExists) => { - if (!userExists) { - userExists = await state.db.models.User.query().insertAndFetch({ - discordId: discordUser.id, - strategy: 'discord', - tutorial: !config.getSafe('map.misc.forceTutorial'), - selectedWebhook, - }) - } - if (userExists.strategy !== 'discord') { - await state.db.models.User.query() - .update({ strategy: 'discord' }) - .where('id', userExists.id) - userExists.strategy = 'discord' - } - if (!userExists.selectedWebhook && selectedWebhook) { - await state.db.models.User.query() - .update({ selectedWebhook }) - .where('id', userExists.id) - userExists.selectedWebhook = selectedWebhook - } - return done(null, { - ...discordUser, - ...userExists, - id: userExists.id, - username: userExists.username || discordUser.username, - }) - }, - ) + // a signed-in user's link attempt already read this row + await Promise.resolve( + link?.action === 'login' + ? link.account + : state.db.models.User.query().findOne({ discordId: discordUser.id }), + ).then(async (/** @type {import('@rm/types').FullUser} */ userExists) => { + if (!userExists) { + userExists = await state.db.models.User.query().insertAndFetch({ + discordId: discordUser.id, + strategy: 'discord', + tutorial: !config.getSafe('map.misc.forceTutorial'), + selectedWebhook, + }) + } + if (userExists.strategy !== 'discord') { + await state.db.models.User.query() + .update({ strategy: 'discord' }) + .where('id', userExists.id) + userExists.strategy = 'discord' + } + if (!userExists.selectedWebhook && selectedWebhook) { + await state.db.models.User.query() + .update({ selectedWebhook }) + .where('id', userExists.id) + userExists.selectedWebhook = selectedWebhook + } + return done(null, { + ...discordUser, + ...userExists, + id: userExists.id, + username: userExists.username || discordUser.username, + }) + }) } catch (e) { this.log.error('User has failed auth.', e) } diff --git a/server/src/services/TelegramClient.js b/server/src/services/TelegramClient.js index 3f4e78d3b..1ddaa98a6 100644 --- a/server/src/services/TelegramClient.js +++ b/server/src/services/TelegramClient.js @@ -170,8 +170,10 @@ class TelegramClient extends AuthClient { const selectedWebhook = Object.keys(state.event.webhookObj).find((x) => user?.perms?.webhooks.includes(x), ) + /** @type {Awaited> | undefined} */ + let link if (req.user) { - const link = await linkAccount(state.db.models, { + link = await linkAccount(state.db.models, { userId: req.user.id, platform: 'telegram', externalId: user.id, @@ -201,42 +203,43 @@ class TelegramClient extends AuthClient { } } - await state.db.models.User.query() - .findOne({ telegramId: user.id }) - .then( - async (/** @type {import('@rm/types').FullUser} */ userExists) => { - if (!userExists) { - userExists = await state.db.models.User.query().insertAndFetch({ - telegramId: user.id, - strategy: user.provider, - tutorial: !config.getSafe('map.misc.forceTutorial'), - selectedWebhook, - }) - } - if (userExists.strategy !== 'telegram') { - await state.db.models.User.query() - .update({ strategy: 'telegram' }) - .where('id', userExists.id) - userExists.strategy = 'telegram' - } - if (!userExists.selectedWebhook && selectedWebhook) { - await state.db.models.User.query() - .update({ selectedWebhook }) - .where('id', userExists.id) - userExists.selectedWebhook = selectedWebhook - } - this.log.info( - user.username, - `(${user.id})`, - 'Authenticated successfully.', - ) - return done(null, { - ...user, - ...userExists, - username: userExists.username || user.username, - }) - }, + // a signed-in user's link attempt already read this row + await Promise.resolve( + link?.action === 'login' + ? link.account + : state.db.models.User.query().findOne({ telegramId: user.id }), + ).then(async (/** @type {import('@rm/types').FullUser} */ userExists) => { + if (!userExists) { + userExists = await state.db.models.User.query().insertAndFetch({ + telegramId: user.id, + strategy: user.provider, + tutorial: !config.getSafe('map.misc.forceTutorial'), + selectedWebhook, + }) + } + if (userExists.strategy !== 'telegram') { + await state.db.models.User.query() + .update({ strategy: 'telegram' }) + .where('id', userExists.id) + userExists.strategy = 'telegram' + } + if (!userExists.selectedWebhook && selectedWebhook) { + await state.db.models.User.query() + .update({ selectedWebhook }) + .where('id', userExists.id) + userExists.selectedWebhook = selectedWebhook + } + this.log.info( + user.username, + `(${user.id})`, + 'Authenticated successfully.', ) + return done(null, { + ...user, + ...userExists, + username: userExists.username || user.username, + }) + }) } catch (e) { this.log.error('User has failed auth.', e) } diff --git a/server/src/services/linkAccount.js b/server/src/services/linkAccount.js index 6c0734f9d..52c65191d 100644 --- a/server/src/services/linkAccount.js +++ b/server/src/services/linkAccount.js @@ -3,7 +3,7 @@ const { mergePerms } = require('../utils/mergePerms') /** @typedef {'discord' | 'telegram'} Platform */ /** - * @typedef {{ action: 'login' } + * @typedef {{ action: 'login', account: import('@rm/types').FullUser | null } * | { action: 'refuse' } * | { action: 'link', merge: import('@rm/types').FullUser | null }} LinkPlan */ @@ -17,7 +17,8 @@ const OTHER = { discord: 'telegram', telegram: 'discord' } * platform id, if any. * * - `login`: nothing to link, the row already has that platform (the same - * account re-authenticating, or a different one, which switches accounts) + * account re-authenticating, or a different one, which switches accounts); + * `account` is `other`, the row that login uses * - `link`: attach the id to `current`, absorbing `other` when there is one * - `refuse`: `other` is a whole account of its own, merging it would lose it * @@ -27,21 +28,17 @@ const OTHER = { discord: 'telegram', telegram: 'discord' } * @returns {LinkPlan} */ function planLink(current, other, platform) { - if (!current || current[`${platform}Id`]) return { action: 'login' } + if (!current || current[`${platform}Id`]) { + return { action: 'login', account: other || null } + } if (!other) return { action: 'link', merge: null } // a local account has a password a merge would throw away, and its strategy // may read discord or telegram after the user last signed in with one if (other.strategy === 'local' || other.password) return { action: 'refuse' } + // a row with both ids is a linked account of its own + if (other[`${OTHER[platform]}Id`]) return { action: 'refuse' } - const otherField = `${OTHER[platform]}Id` - if ( - other[otherField] && - current[otherField] && - other[otherField] !== current[otherField] - ) { - return { action: 'refuse' } - } return { action: 'link', merge: other } } @@ -97,7 +94,9 @@ async function moveBadges(Badge, fromId, toId, trx) { /** * Links a Discord or Telegram account to the row of the user who is signed in. - * A row that only existed for that account is merged in and deleted. + * A row that only existed for that account is merged in and deleted. Both rows + * are read and locked in the transaction that changes them, so concurrent + * callbacks for the same user cannot both link. * * @param {Pick} models * @param {{ @@ -117,33 +116,31 @@ async function linkAccount( const otherPlatform = OTHER[platform] const otherField = `${otherPlatform}Id` - const [current, other] = await Promise.all([ - User.query().findOne({ id: userId }), - User.query().findOne({ [field]: externalId }), - ]) - const plan = planLink(current, other, platform) - if (plan.action !== 'link') return plan + const result = await User.transaction(async (trx) => { + /** @type {import('@rm/types').FullUser[]} */ + const rows = await User.query(trx) + .where('id', userId) + .orWhere(field, externalId) + .orderBy('id') + .forUpdate() + const current = rows.find((row) => `${row.id}` === `${userId}`) + const other = rows.find((row) => `${row[field]}` === `${externalId}`) - const { merge } = plan - const update = { - [field]: externalId, - [`${platform}Perms`]: JSON.stringify(perms), - // only a local sign-in reads this, Discord and Telegram sign-ins always - // alert the account they used; keep a local user's alerts where they were - webhookStrategy: - current.webhookStrategy || - (current[otherField] ? otherPlatform : platform), - } - if (merge) { - if (!current.data && merge.data) update.data = merge.data - if (!current[otherField] && merge[otherField]) { - update[otherField] = merge[otherField] - update[`${otherPlatform}Perms`] = merge[`${otherPlatform}Perms`] - } - } + const plan = planLink(current, other, platform) + if (plan.action !== 'link') return plan - await User.transaction(async (trx) => { + const { merge } = plan + const update = { + [field]: externalId, + [`${platform}Perms`]: JSON.stringify(perms), + // only a local sign-in reads this, Discord and Telegram sign-ins always + // alert the account they used; keep a local user's alerts where they were + webhookStrategy: + current.webhookStrategy || + (current[otherField] ? otherPlatform : platform), + } if (merge) { + if (!current.data && merge.data) update.data = merge.data await moveBadges(Badge, merge.id, current.id, trx) await Backup.query(trx) .update({ userId: current.id }) @@ -154,21 +151,22 @@ async function linkAccount( await User.query(trx).delete().where('id', merge.id) } await User.query(trx).update(update).where('id', current.id) + + return { + ...plan, + linked: { + discordId: update.discordId ?? current.discordId, + telegramId: update.telegramId ?? current.telegramId, + webhookStrategy: update.webhookStrategy, + }, + } }) - if (merge) { + if (result.action === 'link' && result.merge) { // sessions of the deleted row point at a user that no longer exists - await Session.clearOtherSessions(merge.id, sessionId).catch(() => {}) - } - - return { - ...plan, - linked: { - discordId: update.discordId ?? current.discordId, - telegramId: update.telegramId ?? current.telegramId, - webhookStrategy: update.webhookStrategy, - }, + await Session.clearOtherSessions(result.merge.id, sessionId).catch(() => {}) } + return result } module.exports = { linkAccount, planLink, sessionUser } diff --git a/server/test/linkAccount.test.js b/server/test/linkAccount.test.js index 700eaf591..d1b1c6a54 100644 --- a/server/test/linkAccount.test.js +++ b/server/test/linkAccount.test.js @@ -17,16 +17,28 @@ const telegram = (id, extra = {}) => ({ }) test('without a logged in row it is a plain login', () => { - assert.deepEqual(planLink(null, null, 'telegram'), { action: 'login' }) + assert.deepEqual(planLink(null, null, 'telegram'), { + action: 'login', + account: null, + }) }) test('a row that already has that platform logs in instead of linking', () => { - assert.deepEqual(planLink(telegram(1), null, 'telegram'), { + const current = telegram(1) + assert.deepEqual(planLink(current, current, 'telegram'), { action: 'login', + account: current, }) // a different account of the same platform switches accounts - assert.deepEqual(planLink(telegram(1), telegram(2), 'telegram'), { + const other = telegram(2) + assert.deepEqual(planLink(current, other, 'telegram'), { + action: 'login', + account: other, + }) + // ...including one that has no row yet + assert.deepEqual(planLink(current, null, 'telegram'), { action: 'login', + account: null, }) }) @@ -82,13 +94,10 @@ test('an account linked to a different account of the other platform is refused' }) }) -test('the other row brings its link along when the current row has none', () => { +test('a row with both ids is never merged away, even into a row without either', () => { const local = { id: 1, strategy: 'local', username: 'me' } const other = discord(2, { telegramId: 't7' }) - assert.deepEqual(planLink(local, other, 'discord'), { - action: 'link', - merge: other, - }) + assert.deepEqual(planLink(local, other, 'discord'), { action: 'refuse' }) }) test('a local account that last signed in with discord is still never merged away', () => { diff --git a/src/features/profile/LinkAccounts.jsx b/src/features/profile/LinkAccounts.jsx index 0a0c41b96..f1eed1e28 100644 --- a/src/features/profile/LinkAccounts.jsx +++ b/src/features/profile/LinkAccounts.jsx @@ -23,6 +23,12 @@ export function LinkAccounts() { ) const [refreshing, setRefreshing] = React.useState(false) + // Discord and Telegram sign-ins always alert the account they used, only a + // local session (which carries no strategy) picks one + const showWebhookStrategy = + !['discord', 'telegram'].includes(auth.strategy) && + !!auth.discordId && + !!auth.telegramId const [setWebhookStrategy] = useMutation(Query.user('SET_WEBHOOK_STRATEGY')) @@ -61,8 +67,7 @@ export function LinkAccounts() { ) })} - {/* Discord and Telegram sign-ins always alert the account they used */} - {auth.strategy === 'local' && auth.discordId && auth.telegramId && ( + {showWebhookStrategy && ( {t('select_webhook_strategy')} From 75d533b13399562424867e177bd2af1a006cd8f1 Mon Sep 17 00:00:00 2001 From: Fabio1988 Date: Thu, 15 Oct 2026 16:56:58 +0200 Subject: [PATCH 3/6] fix(auth): expire linked trial perms and keep linked webhook A session that linked a second platform kept only its own rmStrategy, so the trial cleanup of the linked platform never matched it and its trial perms outlived the trial. Sessions now list every strategy they hold perms from in linkedStrategies, and clearNonDonor/clearTrial match either. Linking also overwrote the computed webhook fallback with the session's null selectedWebhook. linkAccount now stores the fallback on the row when it has none and returns it with the linked fields. Co-Authored-By: Claude Opus 5.5 --- server/src/models/Session.js | 25 ++++++++++++++++++------ server/src/services/DiscordClient.js | 6 ++---- server/src/services/TelegramClient.js | 6 ++---- server/src/services/linkAccount.js | 28 +++++++++++++++++++++------ server/test/linkAccount.test.js | 20 +++++++++++++++++++ 5 files changed, 65 insertions(+), 20 deletions(-) diff --git a/server/src/models/Session.js b/server/src/models/Session.js index 2e08c7bf1..19e27dd69 100644 --- a/server/src/models/Session.js +++ b/server/src/models/Session.js @@ -14,13 +14,28 @@ class Session extends Model { return results } + /** + * Sessions holding perms of `strategy`: signed in with it, or linked to it + * + * @param {import('objection').QueryBuilder} builder + * @param {string} strategy + */ + static fromStrategy(builder, strategy) { + return builder + .whereRaw(`json_extract(data, '$.passport.user.rmStrategy') = ?`, [ + strategy, + ]) + .orWhereRaw( + `json_contains(json_extract(data, '$.passport.user.linkedStrategies'), ?)`, + [JSON.stringify(strategy)], + ) + } + /** @param {string} strategy */ static async clearNonDonor(strategy) { const result = await this.query() .whereRaw(`json_extract(data, '$.passport.user.perms.donor') = false`) - .andWhereRaw( - `json_extract(data, '$.passport.user.rmStrategy') = '${strategy}'`, - ) + .andWhere((builder) => Session.fromStrategy(builder, strategy)) .delete() log.info( TAGS.session, @@ -36,9 +51,7 @@ class Session extends Model { static async clearTrial(strategy) { return this.query() .whereRaw(`json_extract(data, '$.passport.user.perms.trial') = true`) - .andWhereRaw( - `json_extract(data, '$.passport.user.rmStrategy') = '${strategy}'`, - ) + .andWhere((builder) => Session.fromStrategy(builder, strategy)) .delete() } diff --git a/server/src/services/DiscordClient.js b/server/src/services/DiscordClient.js index fa2b9ab7e..49daac5c7 100644 --- a/server/src/services/DiscordClient.js +++ b/server/src/services/DiscordClient.js @@ -339,6 +339,7 @@ class DiscordClient extends AuthClient { platform: 'discord', externalId: discordUser.id, perms: discordUser.perms, + selectedWebhook, sessionId: req.sessionID, }) if (link.action === 'refuse') { @@ -357,10 +358,7 @@ class DiscordClient extends AuthClient { 'linked to user', req.user.id, ) - return done(null, { - selectedWebhook, - ...sessionUser(req.user, discordUser, link.linked), - }) + return done(null, sessionUser(req.user, discordUser, link.linked)) } } diff --git a/server/src/services/TelegramClient.js b/server/src/services/TelegramClient.js index 1ddaa98a6..4b9d6ebf8 100644 --- a/server/src/services/TelegramClient.js +++ b/server/src/services/TelegramClient.js @@ -178,6 +178,7 @@ class TelegramClient extends AuthClient { platform: 'telegram', externalId: user.id, perms: user.perms, + selectedWebhook, sessionId: req.sessionID, }) if (link.action === 'refuse') { @@ -196,10 +197,7 @@ class TelegramClient extends AuthClient { 'linked to user', req.user.id, ) - return done(null, { - selectedWebhook, - ...sessionUser(req.user, user, link.linked), - }) + return done(null, sessionUser(req.user, user, link.linked)) } } diff --git a/server/src/services/linkAccount.js b/server/src/services/linkAccount.js index 52c65191d..50fa5844f 100644 --- a/server/src/services/linkAccount.js +++ b/server/src/services/linkAccount.js @@ -44,12 +44,13 @@ function planLink(current, other, platform) { /** * The session user after linking: still the logged in row, with the linked - * ids and both platforms' perms + * ids and both platforms' perms. `linkedStrategies` lists every strategy whose + * perms the session holds, so a trial of either one ending clears it * - * @template {{ perms: import('@rm/types').Permissions, username?: string }} T - * @param {import('@rm/types').User} reqUser + * @template {{ perms: import('@rm/types').Permissions, username?: string, rmStrategy?: string }} T + * @param {import('@rm/types').User & { linkedStrategies?: string[] }} reqUser * @param {T} platformUser - * @param {{ discordId?: string, telegramId?: string, webhookStrategy?: string }} linked + * @param {{ discordId?: string, telegramId?: string, webhookStrategy?: string, selectedWebhook?: string }} linked */ function sessionUser(reqUser, platformUser, linked) { return { @@ -58,6 +59,15 @@ function sessionUser(reqUser, platformUser, linked) { ...linked, username: reqUser.username || platformUser.username, perms: mergePerms(reqUser.perms, platformUser.perms), + linkedStrategies: [ + ...new Set( + [ + ...(reqUser.linkedStrategies || []), + reqUser.rmStrategy, + platformUser.rmStrategy, + ].filter(Boolean), + ), + ], } } @@ -104,13 +114,14 @@ async function moveBadges(Badge, fromId, toId, trx) { * platform: Platform, * externalId: string, * perms: import('@rm/types').Permissions, + * selectedWebhook?: string, * sessionId?: string, * }} options - * @returns {Promise} + * @returns {Promise} */ async function linkAccount( { User, Badge, Backup, NestSubmission, Session }, - { userId, platform, externalId, perms, sessionId }, + { userId, platform, externalId, perms, selectedWebhook, sessionId }, ) { const field = `${platform}Id` const otherPlatform = OTHER[platform] @@ -139,6 +150,10 @@ async function linkAccount( current.webhookStrategy || (current[otherField] ? otherPlatform : platform), } + // the linked platform may grant the first webhook this user has access to + if (!current.selectedWebhook && selectedWebhook) { + update.selectedWebhook = selectedWebhook + } if (merge) { if (!current.data && merge.data) update.data = merge.data await moveBadges(Badge, merge.id, current.id, trx) @@ -158,6 +173,7 @@ async function linkAccount( discordId: update.discordId ?? current.discordId, telegramId: update.telegramId ?? current.telegramId, webhookStrategy: update.webhookStrategy, + selectedWebhook: update.selectedWebhook ?? current.selectedWebhook, }, } }) diff --git a/server/test/linkAccount.test.js b/server/test/linkAccount.test.js index d1b1c6a54..03ccbdae7 100644 --- a/server/test/linkAccount.test.js +++ b/server/test/linkAccount.test.js @@ -145,3 +145,23 @@ test('the session keeps the logged in row and gains the linked id', () => { assert.equal(user.perms.raids, true) assert.deepEqual([...user.perms.webhooks].sort(), ['a', 'b']) }) + +test('the linked session lists both strategies for trial cleanup', () => { + const user = sessionUser( + { id: 1, rmStrategy: 'discord', perms: { donor: false, webhooks: [] } }, + { rmStrategy: 'telegram', perms: { trial: true, webhooks: [] } }, + {}, + ) + assert.equal(user.rmStrategy, 'discord') + assert.deepEqual(user.linkedStrategies, ['discord', 'telegram']) + assert.equal(user.perms.trial, true) +}) + +test('a null selected webhook does not hide the linked one', () => { + const user = sessionUser( + { id: 1, selectedWebhook: null, perms: { webhooks: [] } }, + { perms: { webhooks: ['poracle'] } }, + { selectedWebhook: 'poracle' }, + ) + assert.equal(user.selectedWebhook, 'poracle') +}) From 70e784a63cdc28084e2ba04e4946356e4cde985f Mon Sep 17 00:00:00 2001 From: Fabio1988 Date: Tue, 27 Oct 2026 23:17:38 +0100 Subject: [PATCH 4/6] fix(auth): clear linked trial perms even when the session is a donor A linked session merges both platforms' perms, so a donor on one platform kept the other platform's trial perms after that trial ended, since clearNonDonor skips donor sessions. Linked sessions now record the strategies that granted trial perms in `trialStrategies`, and clearNonDonor also clears sessions listing the ending strategy there. Co-Authored-By: Claude Opus 5.5 --- server/src/models/Session.js | 18 +++++++++-- server/src/services/linkAccount.js | 15 +++++++-- server/test/linkAccount.test.js | 33 ++++++++++++++++++++ server/test/sessionTrialCleanup.test.js | 41 +++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 server/test/sessionTrialCleanup.test.js diff --git a/server/src/models/Session.js b/server/src/models/Session.js index 19e27dd69..563472ff1 100644 --- a/server/src/models/Session.js +++ b/server/src/models/Session.js @@ -31,11 +31,23 @@ class Session extends Model { ) } - /** @param {string} strategy */ + /** + * Non-donor sessions holding perms of `strategy`, plus linked sessions whose + * trial perms came from it, since the other platform may make those donors + * + * @param {string} strategy + */ static async clearNonDonor(strategy) { const result = await this.query() - .whereRaw(`json_extract(data, '$.passport.user.perms.donor') = false`) - .andWhere((builder) => Session.fromStrategy(builder, strategy)) + .where((builder) => + builder + .whereRaw(`json_extract(data, '$.passport.user.perms.donor') = false`) + .andWhere((inner) => Session.fromStrategy(inner, strategy)), + ) + .orWhereRaw( + `json_contains(json_extract(data, '$.passport.user.trialStrategies'), ?)`, + [JSON.stringify(strategy)], + ) .delete() log.info( TAGS.session, diff --git a/server/src/services/linkAccount.js b/server/src/services/linkAccount.js index 50fa5844f..24a2cdd6f 100644 --- a/server/src/services/linkAccount.js +++ b/server/src/services/linkAccount.js @@ -45,10 +45,12 @@ function planLink(current, other, platform) { /** * The session user after linking: still the logged in row, with the linked * ids and both platforms' perms. `linkedStrategies` lists every strategy whose - * perms the session holds, so a trial of either one ending clears it + * perms the session holds, so a trial of either one ending clears it. + * `trialStrategies` lists the ones that granted trial perms, those clear the + * session even when the other platform makes it a donor * * @template {{ perms: import('@rm/types').Permissions, username?: string, rmStrategy?: string }} T - * @param {import('@rm/types').User & { linkedStrategies?: string[] }} reqUser + * @param {import('@rm/types').User & { linkedStrategies?: string[], trialStrategies?: string[] }} reqUser * @param {T} platformUser * @param {{ discordId?: string, telegramId?: string, webhookStrategy?: string, selectedWebhook?: string }} linked */ @@ -68,6 +70,15 @@ function sessionUser(reqUser, platformUser, linked) { ].filter(Boolean), ), ], + trialStrategies: [ + ...new Set( + [ + ...(reqUser.trialStrategies || + (reqUser.perms?.trial ? [reqUser.rmStrategy] : [])), + platformUser.perms?.trial && platformUser.rmStrategy, + ].filter(Boolean), + ), + ], } } diff --git a/server/test/linkAccount.test.js b/server/test/linkAccount.test.js index 03ccbdae7..f8247aa39 100644 --- a/server/test/linkAccount.test.js +++ b/server/test/linkAccount.test.js @@ -165,3 +165,36 @@ test('a null selected webhook does not hide the linked one', () => { ) assert.equal(user.selectedWebhook, 'poracle') }) + +test('the linked session records which strategy granted the trial', () => { + const user = sessionUser( + { id: 1, rmStrategy: 'discord', perms: { donor: true, webhooks: [] } }, + { rmStrategy: 'telegram', perms: { trial: true, webhooks: [] } }, + {}, + ) + assert.equal(user.perms.donor, true) + assert.deepEqual(user.trialStrategies, ['telegram']) +}) + +test('a trial of the signed in platform carries into the linked session', () => { + const user = sessionUser( + { id: 1, rmStrategy: 'discord', perms: { trial: true, webhooks: [] } }, + { rmStrategy: 'telegram', perms: { donor: true, webhooks: [] } }, + {}, + ) + assert.deepEqual(user.trialStrategies, ['discord']) +}) + +test('relinking keeps the recorded trial strategies', () => { + const user = sessionUser( + { + id: 1, + rmStrategy: 'discord', + perms: { trial: true, webhooks: [] }, + trialStrategies: ['telegram'], + }, + { rmStrategy: 'telegram', perms: { trial: false, webhooks: [] } }, + {}, + ) + assert.deepEqual(user.trialStrategies, ['telegram']) +}) diff --git a/server/test/sessionTrialCleanup.test.js b/server/test/sessionTrialCleanup.test.js new file mode 100644 index 000000000..070b82b03 --- /dev/null +++ b/server/test/sessionTrialCleanup.test.js @@ -0,0 +1,41 @@ +const assert = require('node:assert/strict') +const { mock, test } = require('node:test') + +const knex = require('knex') +const { QueryBuilder } = require('objection') +const config = require('@rm/config') + +const { getSafe } = config +mock.method(config, 'getSafe', (key) => + key === 'database.settings.sessionTableName' ? 'session' : getSafe(key), +) + +const { Session } = require('../src/models/Session') + +const db = knex({ client: 'mysql2' }) + +/** The SQL a clear method would run, without a database */ +function captureDelete(method, strategy) { + let sql = '' + const query = mock.method(Session, 'query', () => { + const builder = QueryBuilder.forClass(Session).knex(db) + builder.delete = () => { + QueryBuilder.prototype.delete.call(builder) + sql = builder.toKnexQuery().toString() + return Promise.resolve(0) + } + return builder + }) + return Session[method](strategy).then(() => { + query.mock.restore() + return sql + }) +} + +test('trial boundaries clear linked sessions whose trial came from that strategy, donor or not', async () => { + const sql = await captureDelete('clearNonDonor', 'telegram') + assert.match( + sql, + /^delete from `session` where \(json_extract\(data, '\$\.passport\.user\.perms\.donor'\) = false and \(json_extract\(data, '\$\.passport\.user\.rmStrategy'\) = 'telegram' or json_contains\(json_extract\(data, '\$\.passport\.user\.linkedStrategies'\), '\\"telegram\\"'\)\)\) or json_contains\(json_extract\(data, '\$\.passport\.user\.trialStrategies'\), '\\"telegram\\"'\)$/, + ) +}) From 5ff94c93203d423a3be471282b6f7e9eee89cc28 Mon Sep 17 00:00:00 2001 From: Fabio1988 Date: Fri, 30 Oct 2026 11:08:22 +0100 Subject: [PATCH 5/6] fix(auth): only link accounts from a local sign-in Linking from a Discord or Telegram session merged both platforms' perms into one session, which trial expiry and donor checks keyed on a single strategy cannot clear correctly. Only local sign-ins link now, and the profile hides the link buttons for other sessions; a callback for them signs in to that account like any other login. This drops the linkedStrategies and trialStrategies session tracking. Linking also has to be bound to the session that started it: the legacy Telegram widget's signed callback is not, so only the Telegram OAuth flow links, and the Discord strategy now checks the OAuth state. Co-Authored-By: Claude Opus 5.5 --- server/src/models/Session.js | 39 +++----------- server/src/services/DiscordClient.js | 10 +++- server/src/services/TelegramClient.js | 7 ++- server/src/services/linkAccount.js | 49 ++++++++--------- server/test/linkAccount.test.js | 70 +++++++++---------------- server/test/sessionTrialCleanup.test.js | 41 --------------- src/features/profile/LinkAccounts.jsx | 14 ++--- 7 files changed, 75 insertions(+), 155 deletions(-) delete mode 100644 server/test/sessionTrialCleanup.test.js diff --git a/server/src/models/Session.js b/server/src/models/Session.js index 563472ff1..2e08c7bf1 100644 --- a/server/src/models/Session.js +++ b/server/src/models/Session.js @@ -14,39 +14,12 @@ class Session extends Model { return results } - /** - * Sessions holding perms of `strategy`: signed in with it, or linked to it - * - * @param {import('objection').QueryBuilder} builder - * @param {string} strategy - */ - static fromStrategy(builder, strategy) { - return builder - .whereRaw(`json_extract(data, '$.passport.user.rmStrategy') = ?`, [ - strategy, - ]) - .orWhereRaw( - `json_contains(json_extract(data, '$.passport.user.linkedStrategies'), ?)`, - [JSON.stringify(strategy)], - ) - } - - /** - * Non-donor sessions holding perms of `strategy`, plus linked sessions whose - * trial perms came from it, since the other platform may make those donors - * - * @param {string} strategy - */ + /** @param {string} strategy */ static async clearNonDonor(strategy) { const result = await this.query() - .where((builder) => - builder - .whereRaw(`json_extract(data, '$.passport.user.perms.donor') = false`) - .andWhere((inner) => Session.fromStrategy(inner, strategy)), - ) - .orWhereRaw( - `json_contains(json_extract(data, '$.passport.user.trialStrategies'), ?)`, - [JSON.stringify(strategy)], + .whereRaw(`json_extract(data, '$.passport.user.perms.donor') = false`) + .andWhereRaw( + `json_extract(data, '$.passport.user.rmStrategy') = '${strategy}'`, ) .delete() log.info( @@ -63,7 +36,9 @@ class Session extends Model { static async clearTrial(strategy) { return this.query() .whereRaw(`json_extract(data, '$.passport.user.perms.trial') = true`) - .andWhere((builder) => Session.fromStrategy(builder, strategy)) + .andWhereRaw( + `json_extract(data, '$.passport.user.rmStrategy') = '${strategy}'`, + ) .delete() } diff --git a/server/src/services/DiscordClient.js b/server/src/services/DiscordClient.js index 49daac5c7..5006f5c29 100644 --- a/server/src/services/DiscordClient.js +++ b/server/src/services/DiscordClient.js @@ -10,7 +10,7 @@ const { areaPerms } = require('../utils/areaPerms') const { webhookPerms } = require('../utils/webhookPerms') const { scannerPerms, scannerCooldownBypass } = require('../utils/scannerPerms') const { AuthClient } = require('./AuthClient') -const { linkAccount, sessionUser } = require('./linkAccount') +const { canLink, linkAccount, sessionUser } = require('./linkAccount') const { state } = require('./state') class DiscordClient extends AuthClient { @@ -333,7 +333,8 @@ class DiscordClient extends AuthClient { ) /** @type {Awaited> | undefined} */ let link - if (req.user) { + // other sign-ins switch to this account, like a sign-in while logged out + if (canLink(req.user)) { link = await linkAccount(state.db.models, { userId: req.user.id, platform: 'discord', @@ -411,6 +412,11 @@ class DiscordClient extends AuthClient { scope: ['identify', 'guilds'], passReqToCallback: true, prompt: this.strategy.clientPrompt, + // binds the callback to the session that started it, so nobody can + // link their Discord to a signed-in user by sending them a callback + // url; keyed per strategy like Telegram's + sessionKey: `oauth2:discord:${this.rmStrategy}`, + state: true, }, (...args) => this.authHandler(...args), ), diff --git a/server/src/services/TelegramClient.js b/server/src/services/TelegramClient.js index 4b9d6ebf8..bbaf3e375 100644 --- a/server/src/services/TelegramClient.js +++ b/server/src/services/TelegramClient.js @@ -14,7 +14,7 @@ const { scannerPerms, scannerCooldownBypass } = require('../utils/scannerPerms') const { getUserDisplayName } = require('../utils/getUserDisplayName') const { isOAuthStrategy } = require('../utils/getTelegramStrategy') const { AuthClient } = require('./AuthClient') -const { linkAccount, sessionUser } = require('./linkAccount') +const { canLink, linkAccount, sessionUser } = require('./linkAccount') /** * @typedef {import('@rainb0w-clwn/passport-telegram-official/dist/types').PassportTelegramUser} TGUser @@ -172,7 +172,10 @@ class TelegramClient extends AuthClient { ) /** @type {Awaited> | undefined} */ let link - if (req.user) { + // the legacy widget's signed callback is not bound to the session, anyone + // could send theirs to a signed-in user, so only OAuth links. Everything + // else switches to this account, like a sign-in while logged out + if (isOAuthStrategy(this.strategy) && canLink(req.user)) { link = await linkAccount(state.db.models, { userId: req.user.id, platform: 'telegram', diff --git a/server/src/services/linkAccount.js b/server/src/services/linkAccount.js index 24a2cdd6f..be1a001c6 100644 --- a/server/src/services/linkAccount.js +++ b/server/src/services/linkAccount.js @@ -1,4 +1,6 @@ // @ts-check +const config = require('@rm/config') + const { mergePerms } = require('../utils/mergePerms') /** @typedef {'discord' | 'telegram'} Platform */ @@ -43,14 +45,24 @@ function planLink(current, other, platform) { } /** - * The session user after linking: still the logged in row, with the linked - * ids and both platforms' perms. `linkedStrategies` lists every strategy whose - * perms the session holds, so a trial of either one ending clears it. - * `trialStrategies` lists the ones that granted trial perms, those clear the - * session even when the other platform makes it a donor + * Only a local sign-in links accounts. A Discord or Telegram session would have + * to carry both platforms' perms, which trial expiry and donor checks, keyed on + * one strategy, cannot clear correctly + * + * @param {{ rmStrategy?: string } | undefined} user + */ +function canLink(user) { + return config + .getSafe('authentication.strategies') + .some((s) => s.type === 'local' && s.name === user?.rmStrategy) +} + +/** + * The session user after linking: still the local sign-in, with the linked + * ids and both perm sets * - * @template {{ perms: import('@rm/types').Permissions, username?: string, rmStrategy?: string }} T - * @param {import('@rm/types').User & { linkedStrategies?: string[], trialStrategies?: string[] }} reqUser + * @template {{ perms: import('@rm/types').Permissions, username?: string }} T + * @param {import('@rm/types').User} reqUser * @param {T} platformUser * @param {{ discordId?: string, telegramId?: string, webhookStrategy?: string, selectedWebhook?: string }} linked */ @@ -61,24 +73,6 @@ function sessionUser(reqUser, platformUser, linked) { ...linked, username: reqUser.username || platformUser.username, perms: mergePerms(reqUser.perms, platformUser.perms), - linkedStrategies: [ - ...new Set( - [ - ...(reqUser.linkedStrategies || []), - reqUser.rmStrategy, - platformUser.rmStrategy, - ].filter(Boolean), - ), - ], - trialStrategies: [ - ...new Set( - [ - ...(reqUser.trialStrategies || - (reqUser.perms?.trial ? [reqUser.rmStrategy] : [])), - platformUser.perms?.trial && platformUser.rmStrategy, - ].filter(Boolean), - ), - ], } } @@ -114,7 +108,8 @@ async function moveBadges(Badge, fromId, toId, trx) { } /** - * Links a Discord or Telegram account to the row of the user who is signed in. + * Links a Discord or Telegram account to the row of the local user who is + * signed in, see `canLink`. * A row that only existed for that account is merged in and deleted. Both rows * are read and locked in the transaction that changes them, so concurrent * callbacks for the same user cannot both link. @@ -196,4 +191,4 @@ async function linkAccount( return result } -module.exports = { linkAccount, planLink, sessionUser } +module.exports = { canLink, linkAccount, planLink, sessionUser } diff --git a/server/test/linkAccount.test.js b/server/test/linkAccount.test.js index f8247aa39..8d63b2a09 100644 --- a/server/test/linkAccount.test.js +++ b/server/test/linkAccount.test.js @@ -1,7 +1,23 @@ const assert = require('node:assert/strict') -const { test } = require('node:test') - -const { planLink, sessionUser } = require('../src/services/linkAccount') +const { mock, test } = require('node:test') +const config = require('@rm/config') + +const { getSafe } = config +mock.method(config, 'getSafe', (key) => + key === 'authentication.strategies' + ? [ + { name: 'local', type: 'local' }, + { name: 'discord', type: 'discord' }, + { name: 'telegram', type: 'telegram' }, + ] + : getSafe(key), +) + +const { + canLink, + planLink, + sessionUser, +} = require('../src/services/linkAccount') const discord = (id, extra = {}) => ({ id, @@ -146,17 +162,6 @@ test('the session keeps the logged in row and gains the linked id', () => { assert.deepEqual([...user.perms.webhooks].sort(), ['a', 'b']) }) -test('the linked session lists both strategies for trial cleanup', () => { - const user = sessionUser( - { id: 1, rmStrategy: 'discord', perms: { donor: false, webhooks: [] } }, - { rmStrategy: 'telegram', perms: { trial: true, webhooks: [] } }, - {}, - ) - assert.equal(user.rmStrategy, 'discord') - assert.deepEqual(user.linkedStrategies, ['discord', 'telegram']) - assert.equal(user.perms.trial, true) -}) - test('a null selected webhook does not hide the linked one', () => { const user = sessionUser( { id: 1, selectedWebhook: null, perms: { webhooks: [] } }, @@ -166,35 +171,10 @@ test('a null selected webhook does not hide the linked one', () => { assert.equal(user.selectedWebhook, 'poracle') }) -test('the linked session records which strategy granted the trial', () => { - const user = sessionUser( - { id: 1, rmStrategy: 'discord', perms: { donor: true, webhooks: [] } }, - { rmStrategy: 'telegram', perms: { trial: true, webhooks: [] } }, - {}, - ) - assert.equal(user.perms.donor, true) - assert.deepEqual(user.trialStrategies, ['telegram']) -}) - -test('a trial of the signed in platform carries into the linked session', () => { - const user = sessionUser( - { id: 1, rmStrategy: 'discord', perms: { trial: true, webhooks: [] } }, - { rmStrategy: 'telegram', perms: { donor: true, webhooks: [] } }, - {}, - ) - assert.deepEqual(user.trialStrategies, ['discord']) -}) - -test('relinking keeps the recorded trial strategies', () => { - const user = sessionUser( - { - id: 1, - rmStrategy: 'discord', - perms: { trial: true, webhooks: [] }, - trialStrategies: ['telegram'], - }, - { rmStrategy: 'telegram', perms: { trial: false, webhooks: [] } }, - {}, - ) - assert.deepEqual(user.trialStrategies, ['telegram']) +test('only a local sign-in can link accounts', () => { + assert.equal(canLink({ rmStrategy: 'local' }), true) + assert.equal(canLink({ rmStrategy: 'discord' }), false) + assert.equal(canLink({ rmStrategy: 'telegram' }), false) + assert.equal(canLink({ rmStrategy: 'unknown' }), false) + assert.equal(canLink(undefined), false) }) diff --git a/server/test/sessionTrialCleanup.test.js b/server/test/sessionTrialCleanup.test.js deleted file mode 100644 index 070b82b03..000000000 --- a/server/test/sessionTrialCleanup.test.js +++ /dev/null @@ -1,41 +0,0 @@ -const assert = require('node:assert/strict') -const { mock, test } = require('node:test') - -const knex = require('knex') -const { QueryBuilder } = require('objection') -const config = require('@rm/config') - -const { getSafe } = config -mock.method(config, 'getSafe', (key) => - key === 'database.settings.sessionTableName' ? 'session' : getSafe(key), -) - -const { Session } = require('../src/models/Session') - -const db = knex({ client: 'mysql2' }) - -/** The SQL a clear method would run, without a database */ -function captureDelete(method, strategy) { - let sql = '' - const query = mock.method(Session, 'query', () => { - const builder = QueryBuilder.forClass(Session).knex(db) - builder.delete = () => { - QueryBuilder.prototype.delete.call(builder) - sql = builder.toKnexQuery().toString() - return Promise.resolve(0) - } - return builder - }) - return Session[method](strategy).then(() => { - query.mock.restore() - return sql - }) -} - -test('trial boundaries clear linked sessions whose trial came from that strategy, donor or not', async () => { - const sql = await captureDelete('clearNonDonor', 'telegram') - assert.match( - sql, - /^delete from `session` where \(json_extract\(data, '\$\.passport\.user\.perms\.donor'\) = false and \(json_extract\(data, '\$\.passport\.user\.rmStrategy'\) = 'telegram' or json_contains\(json_extract\(data, '\$\.passport\.user\.linkedStrategies'\), '\\"telegram\\"'\)\)\) or json_contains\(json_extract\(data, '\$\.passport\.user\.trialStrategies'\), '\\"telegram\\"'\)$/, - ) -}) diff --git a/src/features/profile/LinkAccounts.jsx b/src/features/profile/LinkAccounts.jsx index f1eed1e28..f8a5f6723 100644 --- a/src/features/profile/LinkAccounts.jsx +++ b/src/features/profile/LinkAccounts.jsx @@ -23,12 +23,10 @@ export function LinkAccounts() { ) const [refreshing, setRefreshing] = React.useState(false) - // Discord and Telegram sign-ins always alert the account they used, only a - // local session (which carries no strategy) picks one - const showWebhookStrategy = - !['discord', 'telegram'].includes(auth.strategy) && - !!auth.discordId && - !!auth.telegramId + // only a local session (which carries no strategy) links accounts, and + // picks which one alerts, Discord and Telegram sign-ins use their own + const isLocal = !['discord', 'telegram'].includes(auth.strategy) + const showWebhookStrategy = isLocal && !!auth.discordId && !!auth.telegramId const [setWebhookStrategy] = useMutation(Query.user('SET_WEBHOOK_STRATEGY')) @@ -41,6 +39,10 @@ export function LinkAccounts() { {METHODS.map((method, i) => { if (!auth.methods.includes(method)) return null + // the legacy Telegram widget cannot link, its callback is not tied + // to this session + const canLink = isLocal && (!i || auth.telegramOAuth) + if (!auth[`${method}Id`] && !canLink) return null const Component = i ? ( Date: Sun, 1 Nov 2026 23:12:19 +0100 Subject: [PATCH 6/6] fix(profile): only offer account linking to local sign-ins Linking Telegram to a Discord sign-in (or the reverse) fails, and making it work means merging two platforms' perms into one session. Hide the link buttons for Discord and Telegram sessions instead and leave the server's linking for local accounts as it is on main. Co-Authored-By: Claude Opus 5.5 --- packages/locales/lib/human/en.json | 1 - server/src/services/DiscordClient.js | 149 ++++++++++---------- server/src/services/TelegramClient.js | 139 +++++++++--------- server/src/services/linkAccount.js | 194 -------------------------- server/test/linkAccount.test.js | 180 ------------------------ src/features/profile/LinkAccounts.jsx | 9 +- src/pages/Blocked.jsx | 3 +- 7 files changed, 145 insertions(+), 530 deletions(-) delete mode 100644 server/src/services/linkAccount.js delete mode 100644 server/test/linkAccount.test.js diff --git a/packages/locales/lib/human/en.json b/packages/locales/lib/human/en.json index 6abcfe1f3..b722b2c23 100644 --- a/packages/locales/lib/human/en.json +++ b/packages/locales/lib/human/en.json @@ -736,7 +736,6 @@ "showcase_block": "Blocked due to a showcase", "signed_in_as": "Signed In As", "missing_map_perm": "You do not have permission to view the map.", - "account_already_linked": "That account is already linked to another map account and was not linked to yours. Sign in with it to use that account instead.", "access_denied": "Access Denied", "audio": "Audio", "desktop_notifications": "Desktop Notifications", diff --git a/server/src/services/DiscordClient.js b/server/src/services/DiscordClient.js index 5006f5c29..f34489106 100644 --- a/server/src/services/DiscordClient.js +++ b/server/src/services/DiscordClient.js @@ -9,8 +9,8 @@ const { logUserAuth } = require('./logUserAuth') const { areaPerms } = require('../utils/areaPerms') const { webhookPerms } = require('../utils/webhookPerms') const { scannerPerms, scannerCooldownBypass } = require('../utils/scannerPerms') +const { mergePerms } = require('../utils/mergePerms') const { AuthClient } = require('./AuthClient') -const { canLink, linkAccount, sessionUser } = require('./linkAccount') const { state } = require('./state') class DiscordClient extends AuthClient { @@ -328,74 +328,82 @@ class DiscordClient extends AuthClient { delete discordUser.guilds } - const selectedWebhook = Object.keys(state.event.webhookObj).find((x) => - discordUser?.perms?.webhooks.includes(x), - ) - /** @type {Awaited> | undefined} */ - let link - // other sign-ins switch to this account, like a sign-in while logged out - if (canLink(req.user)) { - link = await linkAccount(state.db.models, { - userId: req.user.id, - platform: 'discord', - externalId: discordUser.id, - perms: discordUser.perms, - selectedWebhook, - sessionId: req.sessionID, - }) - if (link.action === 'refuse') { - this.log.warn( - discordUser.username, - `(${discordUser.id})`, - 'is already linked to another account, not linking it to user', - req.user.id, - ) - return done(null, undefined, { message: 'account_already_linked' }) - } - if (link.action === 'link') { - this.log.info( - discordUser.username, - `(${discordUser.id})`, - 'linked to user', - req.user.id, - ) - return done(null, sessionUser(req.user, discordUser, link.linked)) - } - } + await state.db.models.User.query() + .findOne(req.user ? { id: req.user.id } : { discordId: discordUser.id }) + .then( + async (/** @type {import('@rm/types').FullUser} */ userExists) => { + const selectedWebhook = Object.keys(state.event.webhookObj).find( + (x) => discordUser?.perms?.webhooks.includes(x), + ) + if (req.user && userExists?.strategy === 'local') { + await state.db.models.User.query() + .update({ + discordId: discordUser.id, + discordPerms: JSON.stringify(discordUser.perms), + webhookStrategy: 'discord', + }) + .where('id', req.user.id) + /** @type {import('@rm/types').FullUser} */ + const oldUser = await state.db.models.User.query() + .where('discordId', discordUser.id) + .whereNot('id', req.user.id) + .first() + if (oldUser) { + await state.db.models.Badge.query() + .update({ + // @ts-ignore + userId: req.user.id, + }) + .where('userId', oldUser.id) + await state.db.models.User.query() + .update({ + data: oldUser.data, + }) + .where('id', req.user.id) + .where('data', null) + } + await state.db.models.User.query() + .where('discordId', discordUser.id) + .whereNot('id', req.user.id) + .delete() + return done(null, { + selectedWebhook, + ...discordUser, + ...req.user, + username: userExists.username || discordUser.username, + discordId: discordUser.id, + perms: mergePerms(req.user.perms, discordUser.perms), + }) + } - // a signed-in user's link attempt already read this row - await Promise.resolve( - link?.action === 'login' - ? link.account - : state.db.models.User.query().findOne({ discordId: discordUser.id }), - ).then(async (/** @type {import('@rm/types').FullUser} */ userExists) => { - if (!userExists) { - userExists = await state.db.models.User.query().insertAndFetch({ - discordId: discordUser.id, - strategy: 'discord', - tutorial: !config.getSafe('map.misc.forceTutorial'), - selectedWebhook, - }) - } - if (userExists.strategy !== 'discord') { - await state.db.models.User.query() - .update({ strategy: 'discord' }) - .where('id', userExists.id) - userExists.strategy = 'discord' - } - if (!userExists.selectedWebhook && selectedWebhook) { - await state.db.models.User.query() - .update({ selectedWebhook }) - .where('id', userExists.id) - userExists.selectedWebhook = selectedWebhook - } - return done(null, { - ...discordUser, - ...userExists, - id: userExists.id, - username: userExists.username || discordUser.username, - }) - }) + if (!userExists) { + userExists = await state.db.models.User.query().insertAndFetch({ + discordId: discordUser.id, + strategy: 'discord', + tutorial: !config.getSafe('map.misc.forceTutorial'), + selectedWebhook, + }) + } + if (userExists.strategy !== 'discord') { + await state.db.models.User.query() + .update({ strategy: 'discord' }) + .where('id', userExists.id) + userExists.strategy = 'discord' + } + if (!userExists.selectedWebhook && selectedWebhook) { + await state.db.models.User.query() + .update({ selectedWebhook }) + .where('id', userExists.id) + userExists.selectedWebhook = selectedWebhook + } + return done(null, { + ...discordUser, + ...userExists, + id: userExists.id, + username: userExists.username || discordUser.username, + }) + }, + ) } catch (e) { this.log.error('User has failed auth.', e) } @@ -412,11 +420,6 @@ class DiscordClient extends AuthClient { scope: ['identify', 'guilds'], passReqToCallback: true, prompt: this.strategy.clientPrompt, - // binds the callback to the session that started it, so nobody can - // link their Discord to a signed-in user by sending them a callback - // url; keyed per strategy like Telegram's - sessionKey: `oauth2:discord:${this.rmStrategy}`, - state: true, }, (...args) => this.authHandler(...args), ), diff --git a/server/src/services/TelegramClient.js b/server/src/services/TelegramClient.js index bbaf3e375..96ffd691d 100644 --- a/server/src/services/TelegramClient.js +++ b/server/src/services/TelegramClient.js @@ -11,10 +11,10 @@ const { state } = require('./state') const { areaPerms } = require('../utils/areaPerms') const { webhookPerms } = require('../utils/webhookPerms') const { scannerPerms, scannerCooldownBypass } = require('../utils/scannerPerms') +const { mergePerms } = require('../utils/mergePerms') const { getUserDisplayName } = require('../utils/getUserDisplayName') const { isOAuthStrategy } = require('../utils/getTelegramStrategy') const { AuthClient } = require('./AuthClient') -const { canLink, linkAccount, sessionUser } = require('./linkAccount') /** * @typedef {import('@rainb0w-clwn/passport-telegram-official/dist/types').PassportTelegramUser} TGUser @@ -167,80 +167,71 @@ class TelegramClient extends AuthClient { return done(null, false, { message: 'access_denied' }) } try { - const selectedWebhook = Object.keys(state.event.webhookObj).find((x) => - user?.perms?.webhooks.includes(x), - ) - /** @type {Awaited> | undefined} */ - let link - // the legacy widget's signed callback is not bound to the session, anyone - // could send theirs to a signed-in user, so only OAuth links. Everything - // else switches to this account, like a sign-in while logged out - if (isOAuthStrategy(this.strategy) && canLink(req.user)) { - link = await linkAccount(state.db.models, { - userId: req.user.id, - platform: 'telegram', - externalId: user.id, - perms: user.perms, - selectedWebhook, - sessionId: req.sessionID, - }) - if (link.action === 'refuse') { - this.log.warn( - user.username, - `(${user.id})`, - 'is already linked to another account, not linking it to user', - req.user.id, - ) - return done(null, false, { message: 'account_already_linked' }) - } - if (link.action === 'link') { - this.log.info( - user.username, - `(${user.id})`, - 'linked to user', - req.user.id, - ) - return done(null, sessionUser(req.user, user, link.linked)) - } - } - - // a signed-in user's link attempt already read this row - await Promise.resolve( - link?.action === 'login' - ? link.account - : state.db.models.User.query().findOne({ telegramId: user.id }), - ).then(async (/** @type {import('@rm/types').FullUser} */ userExists) => { - if (!userExists) { - userExists = await state.db.models.User.query().insertAndFetch({ - telegramId: user.id, - strategy: user.provider, - tutorial: !config.getSafe('map.misc.forceTutorial'), - selectedWebhook, - }) - } - if (userExists.strategy !== 'telegram') { - await state.db.models.User.query() - .update({ strategy: 'telegram' }) - .where('id', userExists.id) - userExists.strategy = 'telegram' - } - if (!userExists.selectedWebhook && selectedWebhook) { - await state.db.models.User.query() - .update({ selectedWebhook }) - .where('id', userExists.id) - userExists.selectedWebhook = selectedWebhook - } - this.log.info( - user.username, - `(${user.id})`, - 'Authenticated successfully.', + await state.db.models.User.query() + .findOne({ telegramId: user.id }) + .then( + async (/** @type {import('@rm/types').FullUser} */ userExists) => { + const selectedWebhook = Object.keys(state.event.webhookObj).find( + (x) => user?.perms?.webhooks.includes(x), + ) + if (req.user && userExists?.strategy === 'local') { + await state.db.models.User.query() + .update({ + telegramId: user.id, + telegramPerms: JSON.stringify(user.perms), + webhookStrategy: 'telegram', + }) + .where('id', req.user.id) + await state.db.models.User.query() + .where('telegramId', user.id) + .whereNot('id', req.user.id) + .delete() + this.log.info( + user.username, + `(${user.id})`, + 'Authenticated successfully.', + ) + return done(null, { + selectedWebhook, + ...user, + ...req.user, + username: userExists.username || user.username, + telegramId: user.id, + perms: mergePerms(req.user.perms, user.perms), + }) + } + if (!userExists) { + userExists = await state.db.models.User.query().insertAndFetch({ + telegramId: user.id, + strategy: user.provider, + tutorial: !config.getSafe('map.misc.forceTutorial'), + selectedWebhook, + }) + } + if (userExists.strategy !== 'telegram') { + await state.db.models.User.query() + .update({ strategy: 'telegram' }) + .where('id', userExists.id) + userExists.strategy = 'telegram' + } + if (!userExists.selectedWebhook && selectedWebhook) { + await state.db.models.User.query() + .update({ selectedWebhook }) + .where('id', userExists.id) + userExists.selectedWebhook = selectedWebhook + } + this.log.info( + user.username, + `(${user.id})`, + 'Authenticated successfully.', + ) + return done(null, { + ...user, + ...userExists, + username: userExists.username || user.username, + }) + }, ) - return done(null, { - ...user, - ...userExists, - username: userExists.username || user.username, - }) - }) } catch (e) { this.log.error('User has failed auth.', e) } diff --git a/server/src/services/linkAccount.js b/server/src/services/linkAccount.js deleted file mode 100644 index be1a001c6..000000000 --- a/server/src/services/linkAccount.js +++ /dev/null @@ -1,194 +0,0 @@ -// @ts-check -const config = require('@rm/config') - -const { mergePerms } = require('../utils/mergePerms') - -/** @typedef {'discord' | 'telegram'} Platform */ -/** - * @typedef {{ action: 'login', account: import('@rm/types').FullUser | null } - * | { action: 'refuse' } - * | { action: 'link', merge: import('@rm/types').FullUser | null }} LinkPlan - */ - -/** @type {Record} */ -const OTHER = { discord: 'telegram', telegram: 'discord' } - -/** - * Decides what signing in with `platform` does for a visitor who is already - * logged in as `current`. `other` is the row that already holds the incoming - * platform id, if any. - * - * - `login`: nothing to link, the row already has that platform (the same - * account re-authenticating, or a different one, which switches accounts); - * `account` is `other`, the row that login uses - * - `link`: attach the id to `current`, absorbing `other` when there is one - * - `refuse`: `other` is a whole account of its own, merging it would lose it - * - * @param {import('@rm/types').FullUser | null | undefined} current - * @param {import('@rm/types').FullUser | null | undefined} other - * @param {Platform} platform - * @returns {LinkPlan} - */ -function planLink(current, other, platform) { - if (!current || current[`${platform}Id`]) { - return { action: 'login', account: other || null } - } - if (!other) return { action: 'link', merge: null } - - // a local account has a password a merge would throw away, and its strategy - // may read discord or telegram after the user last signed in with one - if (other.strategy === 'local' || other.password) return { action: 'refuse' } - // a row with both ids is a linked account of its own - if (other[`${OTHER[platform]}Id`]) return { action: 'refuse' } - - return { action: 'link', merge: other } -} - -/** - * Only a local sign-in links accounts. A Discord or Telegram session would have - * to carry both platforms' perms, which trial expiry and donor checks, keyed on - * one strategy, cannot clear correctly - * - * @param {{ rmStrategy?: string } | undefined} user - */ -function canLink(user) { - return config - .getSafe('authentication.strategies') - .some((s) => s.type === 'local' && s.name === user?.rmStrategy) -} - -/** - * The session user after linking: still the local sign-in, with the linked - * ids and both perm sets - * - * @template {{ perms: import('@rm/types').Permissions, username?: string }} T - * @param {import('@rm/types').User} reqUser - * @param {T} platformUser - * @param {{ discordId?: string, telegramId?: string, webhookStrategy?: string, selectedWebhook?: string }} linked - */ -function sessionUser(reqUser, platformUser, linked) { - return { - ...platformUser, - ...reqUser, - ...linked, - username: reqUser.username || platformUser.username, - perms: mergePerms(reqUser.perms, platformUser.perms), - } -} - -/** - * Moves the gym badges of `fromId` to `toId`, keeping the higher badge where - * both have one for the same gym - * - * @param {import('@rm/types').Models['Badge']} Badge - * @param {number} fromId - * @param {number} toId - * @param {import('objection').Transaction} trx - */ -async function moveBadges(Badge, fromId, toId, trx) { - const [mine, theirs] = await Promise.all([ - Badge.query(trx).where('userId', toId), - Badge.query(trx).where('userId', fromId), - ]) - const byGym = new Map(mine.map((badge) => [badge.gymId, badge])) - await Promise.all( - theirs.map(async (badge) => { - const own = byGym.get(badge.gymId) - if (!own) { - return Badge.query(trx).update({ userId: toId }).where('id', badge.id) - } - if (badge.badge > own.badge) { - await Badge.query(trx) - .update({ badge: badge.badge }) - .where('id', own.id) - } - return Badge.query(trx).delete().where('id', badge.id) - }), - ) -} - -/** - * Links a Discord or Telegram account to the row of the local user who is - * signed in, see `canLink`. - * A row that only existed for that account is merged in and deleted. Both rows - * are read and locked in the transaction that changes them, so concurrent - * callbacks for the same user cannot both link. - * - * @param {Pick} models - * @param {{ - * userId: number, - * platform: Platform, - * externalId: string, - * perms: import('@rm/types').Permissions, - * selectedWebhook?: string, - * sessionId?: string, - * }} options - * @returns {Promise} - */ -async function linkAccount( - { User, Badge, Backup, NestSubmission, Session }, - { userId, platform, externalId, perms, selectedWebhook, sessionId }, -) { - const field = `${platform}Id` - const otherPlatform = OTHER[platform] - const otherField = `${otherPlatform}Id` - - const result = await User.transaction(async (trx) => { - /** @type {import('@rm/types').FullUser[]} */ - const rows = await User.query(trx) - .where('id', userId) - .orWhere(field, externalId) - .orderBy('id') - .forUpdate() - const current = rows.find((row) => `${row.id}` === `${userId}`) - const other = rows.find((row) => `${row[field]}` === `${externalId}`) - - const plan = planLink(current, other, platform) - if (plan.action !== 'link') return plan - - const { merge } = plan - const update = { - [field]: externalId, - [`${platform}Perms`]: JSON.stringify(perms), - // only a local sign-in reads this, Discord and Telegram sign-ins always - // alert the account they used; keep a local user's alerts where they were - webhookStrategy: - current.webhookStrategy || - (current[otherField] ? otherPlatform : platform), - } - // the linked platform may grant the first webhook this user has access to - if (!current.selectedWebhook && selectedWebhook) { - update.selectedWebhook = selectedWebhook - } - if (merge) { - if (!current.data && merge.data) update.data = merge.data - await moveBadges(Badge, merge.id, current.id, trx) - await Backup.query(trx) - .update({ userId: current.id }) - .where('userId', merge.id) - await NestSubmission.query(trx) - .update({ user_id: current.id }) - .where('user_id', merge.id) - await User.query(trx).delete().where('id', merge.id) - } - await User.query(trx).update(update).where('id', current.id) - - return { - ...plan, - linked: { - discordId: update.discordId ?? current.discordId, - telegramId: update.telegramId ?? current.telegramId, - webhookStrategy: update.webhookStrategy, - selectedWebhook: update.selectedWebhook ?? current.selectedWebhook, - }, - } - }) - - if (result.action === 'link' && result.merge) { - // sessions of the deleted row point at a user that no longer exists - await Session.clearOtherSessions(result.merge.id, sessionId).catch(() => {}) - } - return result -} - -module.exports = { canLink, linkAccount, planLink, sessionUser } diff --git a/server/test/linkAccount.test.js b/server/test/linkAccount.test.js deleted file mode 100644 index 8d63b2a09..000000000 --- a/server/test/linkAccount.test.js +++ /dev/null @@ -1,180 +0,0 @@ -const assert = require('node:assert/strict') -const { mock, test } = require('node:test') -const config = require('@rm/config') - -const { getSafe } = config -mock.method(config, 'getSafe', (key) => - key === 'authentication.strategies' - ? [ - { name: 'local', type: 'local' }, - { name: 'discord', type: 'discord' }, - { name: 'telegram', type: 'telegram' }, - ] - : getSafe(key), -) - -const { - canLink, - planLink, - sessionUser, -} = require('../src/services/linkAccount') - -const discord = (id, extra = {}) => ({ - id, - strategy: 'discord', - discordId: `d${id}`, - ...extra, -}) -const telegram = (id, extra = {}) => ({ - id, - strategy: 'telegram', - telegramId: `t${id}`, - ...extra, -}) - -test('without a logged in row it is a plain login', () => { - assert.deepEqual(planLink(null, null, 'telegram'), { - action: 'login', - account: null, - }) -}) - -test('a row that already has that platform logs in instead of linking', () => { - const current = telegram(1) - assert.deepEqual(planLink(current, current, 'telegram'), { - action: 'login', - account: current, - }) - // a different account of the same platform switches accounts - const other = telegram(2) - assert.deepEqual(planLink(current, other, 'telegram'), { - action: 'login', - account: other, - }) - // ...including one that has no row yet - assert.deepEqual(planLink(current, null, 'telegram'), { - action: 'login', - account: null, - }) -}) - -test('a discord user links telegram that has no row yet', () => { - assert.deepEqual(planLink(discord(1), null, 'telegram'), { - action: 'link', - merge: null, - }) -}) - -test('a discord user links telegram and absorbs its telegram-only row', () => { - const other = telegram(2) - assert.deepEqual(planLink(discord(1), other, 'telegram'), { - action: 'link', - merge: other, - }) -}) - -test('a telegram user links discord and absorbs its discord-only row', () => { - const other = discord(2) - assert.deepEqual(planLink(telegram(1), other, 'discord'), { - action: 'link', - merge: other, - }) -}) - -test('a local user links like before', () => { - const local = { id: 1, strategy: 'local', username: 'me' } - const other = discord(2) - assert.deepEqual(planLink(local, other, 'discord'), { - action: 'link', - merge: other, - }) -}) - -test('a local account is never merged away', () => { - const other = { - id: 2, - strategy: 'local', - username: 'x', - password: 'hash', - discordId: 'd2', - } - assert.deepEqual(planLink(telegram(1), other, 'discord'), { - action: 'refuse', - }) -}) - -test('an account linked to a different account of the other platform is refused', () => { - const other = discord(2, { telegramId: 't7' }) - assert.deepEqual(planLink(telegram(1), other, 'discord'), { - action: 'refuse', - }) -}) - -test('a row with both ids is never merged away, even into a row without either', () => { - const local = { id: 1, strategy: 'local', username: 'me' } - const other = discord(2, { telegramId: 't7' }) - assert.deepEqual(planLink(local, other, 'discord'), { action: 'refuse' }) -}) - -test('a local account that last signed in with discord is still never merged away', () => { - const other = discord(2, { username: 'x', password: 'hash' }) - assert.deepEqual(planLink(telegram(1), other, 'discord'), { - action: 'refuse', - }) -}) - -test('a flipped row without the id its strategy claims does not count as linked', () => { - // a telegram row whose strategy the old discord link path set to discord - const flipped = { id: 1, strategy: 'discord', telegramId: 't1' } - assert.deepEqual(planLink(flipped, discord(2), 'discord'), { - action: 'link', - merge: discord(2), - }) -}) - -test('the session keeps the logged in row and gains the linked id', () => { - const reqUser = { - id: 1, - strategy: 'discord', - discordId: 'd1', - username: 'disc', - perms: { map: true, pokemon: true, webhooks: ['a'] }, - } - const platformUser = { - id: 't9', - username: 'tele', - provider: 'telegram', - perms: { map: true, raids: true, webhooks: ['b'] }, - } - const user = sessionUser(reqUser, platformUser, { - discordId: 'd1', - telegramId: 't9', - webhookStrategy: 'discord', - }) - assert.equal(user.id, 1) - assert.equal(user.strategy, 'discord') - assert.equal(user.username, 'disc') - assert.equal(user.discordId, 'd1') - assert.equal(user.telegramId, 't9') - assert.equal(user.webhookStrategy, 'discord') - assert.equal(user.perms.pokemon, true) - assert.equal(user.perms.raids, true) - assert.deepEqual([...user.perms.webhooks].sort(), ['a', 'b']) -}) - -test('a null selected webhook does not hide the linked one', () => { - const user = sessionUser( - { id: 1, selectedWebhook: null, perms: { webhooks: [] } }, - { perms: { webhooks: ['poracle'] } }, - { selectedWebhook: 'poracle' }, - ) - assert.equal(user.selectedWebhook, 'poracle') -}) - -test('only a local sign-in can link accounts', () => { - assert.equal(canLink({ rmStrategy: 'local' }), true) - assert.equal(canLink({ rmStrategy: 'discord' }), false) - assert.equal(canLink({ rmStrategy: 'telegram' }), false) - assert.equal(canLink({ rmStrategy: 'unknown' }), false) - assert.equal(canLink(undefined), false) -}) diff --git a/src/features/profile/LinkAccounts.jsx b/src/features/profile/LinkAccounts.jsx index f8a5f6723..7174a90fa 100644 --- a/src/features/profile/LinkAccounts.jsx +++ b/src/features/profile/LinkAccounts.jsx @@ -23,8 +23,8 @@ export function LinkAccounts() { ) const [refreshing, setRefreshing] = React.useState(false) - // only a local session (which carries no strategy) links accounts, and - // picks which one alerts, Discord and Telegram sign-ins use their own + // only a local session (which carries no strategy) can link accounts, and + // pick which one alerts, Discord and Telegram sign-ins use their own const isLocal = !['discord', 'telegram'].includes(auth.strategy) const showWebhookStrategy = isLocal && !!auth.discordId && !!auth.telegramId @@ -39,10 +39,7 @@ export function LinkAccounts() { {METHODS.map((method, i) => { if (!auth.methods.includes(method)) return null - // the legacy Telegram widget cannot link, its callback is not tied - // to this session - const canLink = isLocal && (!i || auth.telegramOAuth) - if (!auth[`${method}Id`] && !canLink) return null + if (!auth[`${method}Id`] && !isLocal) return null const Component = i ? ( ) : ( - {t(alreadyLinked ? 'account_already_linked' : 'missing_map_perm')} + {t('missing_map_perm')} )} {discordInvite && (