fix: handle pending bot page permission

.env.demo.local

@@ -12,13 +12,15 @@ MONGO_DATABASE=discordbots
 
 DISCORD_CLIENT_ID=CLIENT_ID
 DISCORD_CLIENT_SECRET=CLIENT_SECRET
-DISCORD_SCOPE=SCOPE
+DISCORD_SCOPE=identify email guilds
 DISCORD_TOKEN=BOT_TOKEN
 DISCORD_CLIENT_INTENTS=32767
 
 GUILD_ID=653083797763522580
 BOT_GUILD_ID=653083797763522580
 REVIEW_GUILD_ID=906537041326637086
+# Set both review IDs to sync the bot reviewer flag at login using guilds.members.read.
+REVIEW_ROLE_ID=
 
 GITHUB_CLIENT_ID=GH_CLIENT_ID
 GITHUB_CLIENT_SECRET=GH_CLIENT_SECRET

pages/api/auth/discord/callback.ts

@@ -28,7 +28,6 @@ const Callback = RequestHandler().get(async (req: ApiRequest, res) => {
 			client_id: process.env.DISCORD_CLIENT_ID,
 			redirect_uri: process.env.KOREANBOTS_URL + '/api/auth/discord/callback',
 			client_secret: process.env.DISCORD_CLIENT_SECRET,
-			scope: process.env.DISCORD_SCOPE,
 			grant_type: 'authorization_code',
 			code: req.query.code,
 		}),

pages/api/auth/discord/index.ts

@@ -1,11 +1,12 @@
 import { NextApiRequest, NextApiResponse } from 'next'
 import { generateOauthURL } from '@utils/Tools'
+import { getDiscordOAuthScope } from '@utils/DiscordOAuth'
 import RequestHandler from '@utils/RequestHandler'
 
 const Discord = RequestHandler().get(async (_req: NextApiRequest, res: NextApiResponse) => {
 	res.redirect(
-		301,
-		generateOauthURL('discord', process.env.DISCORD_CLIENT_ID, process.env.DISCORD_SCOPE)
+		302,
+		generateOauthURL('discord', process.env.DISCORD_CLIENT_ID, getDiscordOAuthScope())
 	)
 })
 

pages/pendingBots/[id]/[date].tsx

@@ -4,6 +4,7 @@ import Link from 'next/link'
 import { NextSeo } from 'next-seo'
 
 import { get } from '@utils/Query'
+import { checkUserFlag, parseCookie } from '@utils/Tools'
 import { BotSubmissionDenyReasonPresetsName, git } from '@utils/Constants'
 import Day from '@utils/Day'
 
@@ -202,15 +203,28 @@ const PendingBot: NextPage<PendingBotProps> = ({ data }) => {
 }
 
 export const getServerSideProps = async (ctx: Context) => {
+	const parsed = parseCookie(ctx.req)
+	const userID = await get.Authorization(parsed?.token)
+	if (!userID) return { notFound: true }
+
+	const user = await get.user.load(userID)
+	if (!user) return { notFound: true }
+
 	const data = await get.botSubmit.load(JSON.stringify(ctx.query))
+	if (
+		!data ||
+		(data.owner?.id !== user.id &&
+			!checkUserFlag(user.flags, 'staff') &&
+			!checkUserFlag(user.flags, 'botreviewer'))
+	)
+		return { notFound: true }
+
 	return {
 		props: {
-			data: data
-				? {
-						...data,
-						strikes: await get.botSubmitStrikes(data.id),
-				  }
-				: null,
+			data: {
+				...data,
+				strikes: await get.botSubmitStrikes(data.id),
+			},
 		},
 	}
 }

tests/discord-oauth.test.ts

@@ -0,0 +1,74 @@
+import fetch from 'node-fetch'
+
+import { Oauth } from '../utils/Constants'
+import { getBotReviewerStatus, getDiscordOAuthScope } from '../utils/DiscordOAuth'
+
+jest.mock('node-fetch')
+
+const fetchMock = fetch as jest.MockedFunction<typeof fetch>
+const originalEnv = process.env
+
+beforeEach(() => {
+	jest.resetAllMocks()
+	process.env = {
+		...originalEnv,
+		DISCORD_SCOPE: 'identify email guilds',
+		REVIEW_GUILD_ID: 'review-guild',
+		REVIEW_ROLE_ID: 'review-role',
+	}
+})
+
+afterEach(() => {
+	process.env = originalEnv
+})
+
+test('requests member access and allows consent for the additional scope', () => {
+	process.env.DISCORD_SCOPE += ' guilds.members.read'
+	const url = new URL(Oauth.discord('client', getDiscordOAuthScope()))
+	expect(url.searchParams.get('scope')).toBe('identify email guilds guilds.members.read')
+	expect(url.searchParams.get('prompt')).not.toBe('none')
+})
+
+test.each(['REVIEW_GUILD_ID', 'REVIEW_ROLE_ID'])('skips sync without %s', async (key) => {
+	delete process.env[key]
+	expect(getDiscordOAuthScope()).toBe('identify email guilds')
+	await expect(getBotReviewerStatus('access-token')).resolves.toBeNull()
+	expect(fetchMock).not.toHaveBeenCalled()
+})
+
+test.each([
+	[['review-role', 'another-role'], true],
+	[['another-role'], false],
+	[[], false],
+])('checks the authenticated member roles %j', async (roles, expected) => {
+	fetchMock.mockResolvedValue({
+		ok: true,
+		status: 200,
+		json: async () => ({ roles }),
+	} as any)
+	await expect(getBotReviewerStatus('access-token')).resolves.toBe(expected)
+	expect(fetchMock).toHaveBeenCalledWith(
+		'https://discord.com/api/v10/users/@me/guilds/review-guild/member',
+		{ headers: { Authorization: 'Bearer access-token' }, timeout: 10000 }
+	)
+})
+
+test('removes reviewer status when the user is no longer a guild member', async () => {
+	fetchMock.mockResolvedValue({ ok: false, status: 404 } as any)
+	await expect(getBotReviewerStatus('access-token')).resolves.toBe(false)
+})
+
+test.each([401, 403, 429, 500])('does not treat HTTP %i as a missing role', async (status) => {
+	fetchMock.mockResolvedValue({ ok: false, status } as any)
+	await expect(getBotReviewerStatus('access-token')).rejects.toThrow('Discord member lookup failed')
+})
+
+test('propagates network failures', async () => {
+	fetchMock.mockRejectedValue(new Error('network failure'))
+	await expect(getBotReviewerStatus('access-token')).rejects.toThrow('network failure')
+})
+
+test.each([{}, { roles: null }, { roles: [4] }])('rejects malformed members %j', async (member) => {
+	fetchMock.mockResolvedValue({ ok: true, status: 200, json: async () => member } as any)
+	await expect(getBotReviewerStatus('access-token')).rejects.toThrow('invalid roles')
+})

tests/login-flags.test.ts

@@ -0,0 +1,104 @@
+import knex from '../utils/Knex'
+import { getBotReviewerStatus } from '../utils/DiscordOAuth'
+import { get, update } from '../utils/Query'
+import { TokenRegister, UserFlags } from '../types'
+
+jest.mock('../utils/Knex', () => ({
+	__esModule: true,
+	default: Object.assign(jest.fn(), { raw: jest.fn((sql, bindings) => ({ sql, bindings })) }),
+}))
+jest.mock('../utils/DiscordOAuth')
+jest.mock('../utils/DiscordBot', () => ({}))
+jest.mock('../utils/Mongo', () => ({}))
+jest.mock('../utils/Jwt', () => ({ sign: () => 'session-token', verify: () => ({ id: 'user' }) }))
+
+const statusMock = getBotReviewerStatus as jest.MockedFunction<typeof getBotReviewerStatus>
+const userInfo: TokenRegister = {
+	id: 'user',
+	access_token: 'access-token',
+	expires_in: 3600,
+	refresh_token: 'refresh-token',
+	email: 'user@example.com',
+	username: 'user',
+	discriminator: '0',
+	verified: true,
+}
+let rows: { token: string; perm: string }[]
+let query: {
+	select: jest.Mock
+	where: jest.Mock
+	update: jest.Mock
+	insert: jest.Mock
+	then: (resolve: (value: typeof rows) => unknown) => Promise<unknown>
+}
+
+beforeEach(() => {
+	jest.clearAllMocks()
+	rows = [{ token: 'session-token', perm: 'user' }]
+	query = {
+		select: jest.fn().mockReturnThis(),
+		where: jest.fn().mockReturnThis(),
+		update: jest.fn().mockReturnThis(),
+		insert: jest.fn().mockImplementation(() => {
+			rows = [{ token: 'session-token', perm: 'user' }]
+			return query
+		}),
+		then: (resolve) => Promise.resolve(resolve(rows)),
+	}
+	;(knex as unknown as jest.Mock).mockReturnValue(query)
+	;(knex.raw as jest.Mock).mockImplementation((sql, bindings) => ({ sql, bindings }))
+	jest.spyOn(get.user, 'clear')
+	jest.spyOn(get._rawUser, 'clear')
+})
+
+afterEach(() => jest.restoreAllMocks())
+
+test.each([
+	[true, '?? | ?', UserFlags.botreviewer],
+	[false, '?? & ?', ~UserFlags.botreviewer],
+])('syncs reviewer status %s without replacing other flag bits', async (status, sql, mask) => {
+	statusMock.mockResolvedValue(status as boolean)
+	await expect(update.assignToken(userInfo)).resolves.toBe('session-token')
+	expect(statusMock).toHaveBeenCalledWith('access-token')
+	expect(query.where).toHaveBeenCalledWith({ id: 'user' })
+	expect(query.update).toHaveBeenCalledWith({ flags: { sql, bindings: ['flags', mask] } })
+	expect(get.user.clear).toHaveBeenCalledWith('user')
+	expect(get._rawUser.clear).toHaveBeenCalledWith('user')
+})
+
+test('also syncs reviewer status for a new account', async () => {
+	rows = []
+	statusMock.mockResolvedValue(true)
+	await expect(update.assignToken(userInfo)).resolves.toBe('session-token')
+	expect(query.insert).toHaveBeenCalledWith(expect.objectContaining({ id: 'user' }))
+	expect(query.update).toHaveBeenCalledWith({
+		flags: { sql: '?? | ?', bindings: ['flags', UserFlags.botreviewer] },
+	})
+})
+
+test('keeps flags untouched when sync is not configured', async () => {
+	statusMock.mockResolvedValue(null)
+	await expect(update.assignToken(userInfo)).resolves.toBe('session-token')
+	expect(knex.raw).not.toHaveBeenCalled()
+	expect(get.user.clear).not.toHaveBeenCalled()
+})
+
+test('does not change the account or issue a login result when role lookup fails', async () => {
+	statusMock.mockRejectedValue(new Error('Discord unavailable'))
+	await expect(update.assignToken(userInfo)).rejects.toThrow('Discord unavailable')
+	expect(query.update).not.toHaveBeenCalled()
+	expect(query.insert).not.toHaveBeenCalled()
+})
+
+test('rejects blocked accounts before looking up roles', async () => {
+	rows[0].perm = 'blocked'
+	await expect(update.assignToken(userInfo)).resolves.toBe(2)
+	expect(statusMock).not.toHaveBeenCalled()
+	expect(query.update).not.toHaveBeenCalled()
+})
+
+test('rejects unverified accounts before looking up roles', async () => {
+	await expect(update.assignToken({ ...userInfo, verified: false })).resolves.toBe(1)
+	expect(statusMock).not.toHaveBeenCalled()
+	expect(query.update).not.toHaveBeenCalled()
+})

utils/Constants.ts

@@ -284,6 +284,7 @@ export const DiscordEnpoints = {
 	Token: BASE_URLs.api + '/oauth2/token',
 	Me: BASE_URLs.api + '/v9/users/@me',
 	Guilds: BASE_URLs.api + '/v9/users/@me/guilds',
+	GuildMember: (id: string) => `${BASE_URLs.api}/v10/users/@me/guilds/${id}/member`,
 	InviteApplication: (
 		id: string,
 		perms: { [perm: string]: boolean },
@@ -416,7 +417,11 @@ export const GlobalRatelimitIgnore = ['/api/image/discord/avatars/']
 
 export const Oauth = {
 	discord: (clientID: string, scope: string) =>
-		`https://discord.com/oauth2/authorize?client_id=${clientID}&scope=${scope}&permissions=0&response_type=code&redirect_uri=${process.env.KOREANBOTS_URL}/api/auth/discord/callback&prompt=none`,
+		`https://discord.com/oauth2/authorize?client_id=${clientID}&scope=${encodeURIComponent(
+			scope
+		)}&permissions=0&response_type=code&redirect_uri=${
+			process.env.KOREANBOTS_URL
+		}/api/auth/discord/callback`,
 	github: (clientID: string) =>
 		`https://github.com/login/oauth/authorize?client_id=${clientID}&redirect_uri=${process.env.KOREANBOTS_URL}/api/auth/github/callback`,
 }

utils/DiscordOAuth.ts

@@ -0,0 +1,29 @@
+import fetch from 'node-fetch'
+
+import { DiscordEnpoints } from './Constants'
+
+export function getDiscordOAuthScope(): string {
+	const scopes = (process.env.DISCORD_SCOPE || 'identify email guilds').split(/\s+/)
+	if (process.env.REVIEW_GUILD_ID && process.env.REVIEW_ROLE_ID) scopes.push('guilds.members.read')
+	return [...new Set(scopes.filter(Boolean))].join(' ')
+}
+
+// A missing configuration leaves manually assigned flags unchanged.
+export async function getBotReviewerStatus(accessToken: string): Promise<boolean | null> {
+	const guildID = process.env.REVIEW_GUILD_ID
+	const roleID = process.env.REVIEW_ROLE_ID
+	if (!guildID || !roleID) return null
+
+	const response = await fetch(DiscordEnpoints.GuildMember(guildID), {
+		headers: { Authorization: `Bearer ${accessToken}` },
+		timeout: 10000,
+	})
+	if (response.status === 404) return false
+	if (!response.ok) throw new Error(`Discord member lookup failed (${response.status})`)
+
+	const member = await response.json()
+	if (!Array.isArray(member?.roles) || !member.roles.every((role) => typeof role === 'string'))
+		throw new Error('Discord member lookup returned invalid roles')
+
+	return member.roles.includes(roleID)
+}

utils/Query.ts

@@ -22,6 +22,7 @@
 	BotSpec,
 	ServerSpec,
 	ObjectType,
+	UserFlags as KoreanbotsUserFlags,
 } from '@types'
 import {
 	botCategories,
@@ -35,6 +36,7 @@
 import knex from './Knex'
 import { Bots, Servers } from './Mongo'
 import { DiscordBot, getBotGuild, getMainGuild } from './DiscordBot'
+import { getBotReviewerStatus } from './DiscordOAuth'
 import { sign, verify } from './Jwt'
 import { areArraysEqual, camoUrl, formData, getYYMMDD, serialize } from './Tools'
 import { AddBotSubmit, AddServerSubmit, ManageBot, ManageServer } from './Yup'
@@ -91,10 +93,10 @@
 		const botMember =
 			(await getMainGuild()
 				?.members?.fetch(res.id)
 				.catch((e) => null)) ??
 			((await getBotGuild()
 				?.members?.fetch(res.id)
 				.catch((e) => null)) as GuildMember)
 		const name = discordBot.displayName
 		res.flags =
 			res.flags |
@@ -887,6 +889,8 @@
 		.where({ id: info.id || '' })
 	let t: string
 	if (!info.verified) return 1
+	if (token[0]?.perm && token[0].perm !== 'user') return 2
+	const isBotReviewer = await getBotReviewerStatus(info.access_token)
 	if (token.length === 0) {
 		t = sign({ id: info.id }, { expiresIn: '30d' })
 		await knex('users').insert({
@@ -916,7 +920,17 @@
 				}),
 			})
 			.where({ id: info.id })
-	if (token[0].perm && token[0].perm !== 'user') return 2
+	if (isBotReviewer !== null) {
+		await knex('users')
+			.where({ id: info.id })
+			.update({
+				flags: isBotReviewer
+					? knex.raw('?? | ?', ['flags', KoreanbotsUserFlags.botreviewer])
+					: knex.raw('?? & ?', ['flags', ~KoreanbotsUserFlags.botreviewer]),
+			})
+		get.user.clear(info.id)
+		get._rawUser.clear(info.id)
+	}
 	if (!verify(token[0]?.token ?? '')) {
 		t = sign({ id: info.id }, { expiresIn: '30d' })
 		await knex('users').update({ token: t }).where({ id: info.id })