Community administrators now can use checkorder and checkinvoice commands#732
Community administrators now can use checkorder and checkinvoice commands#732Luquitasjeffrey wants to merge 2 commits intomainfrom
Conversation
WalkthroughThe pull request changes two bot command handlers in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/start.ts (1)
582-595:⚠️ Potential issue | 🟠 MajorAdd community scoping for non‑super admins in
checkorder/checkinvoice.By switching to
adminMiddlewarewithout adding the community checks used elsewhere, a community admin can query any order/invoice by ID across communities. That’s a permissions regression.🔧 Suggested fix (apply to both commands)
bot.command('checkorder', adminMiddleware, async (ctx: MainContext) => { try { const [orderId] = (await validateParams(ctx, 2, '\\<_order id_\\>'))!; if (!orderId) return; if (!(await validateObjectId(ctx, orderId))) return; const order = await Order.findOne({ _id: orderId }); if (order === null) return; + if (!ctx.admin.admin) { + if (!order.community_id) return await messages.notAuthorized(ctx); + if (order.community_id != ctx.admin.default_community_id) { + return await messages.notAuthorized(ctx); + } + } const buyer = await User.findOne({ _id: order.buyer_id }); const seller = await User.findOne({ _id: order.seller_id }); await messages.checkOrderMessage(ctx, order, buyer, seller); } catch (error) { logger.error(error); } });bot.command( 'checkinvoice', adminMiddleware, async (ctx: MainContext) => { try { const [orderId] = (await validateParams(ctx, 2, '\\<_order id_\\>'))!; if (!orderId) return; if (!(await validateObjectId(ctx, orderId))) return; const order = await Order.findOne({ _id: orderId }); if (order === null) return; + if (!ctx.admin.admin) { + if (!order.community_id) return await messages.notAuthorized(ctx); + if (order.community_id != ctx.admin.default_community_id) { + return await messages.notAuthorized(ctx); + } + } if (!order.hash) return; const invoice = await getInvoice({ hash: order.hash }); if (invoice === undefined) { throw new Error('invoice is undefined'); }Also applies to: 600-623
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/start.ts (1)
582-624:⚠️ Potential issue | 🟠 MajorScope community admins to their community before revealing order/invoice details.
With
adminMiddleware, community admins can now call these commands. Without a community check, any admin can query any order/invoice by ID, which leaks cross‑community data. Add the same non‑super‑admin guard used elsewhere in this file (e.g., cancelorder/settleorder).🔒 Proposed fix (apply to both handlers)
bot.command('checkorder', adminMiddleware, async (ctx: MainContext) => { try { const [orderId] = (await validateParams(ctx, 2, '\\<_order id_\\>'))!; if (!orderId) return; if (!(await validateObjectId(ctx, orderId))) return; const order = await Order.findOne({ _id: orderId }); if (order === null) return; + + // Restrict community admins to their community orders + if (!ctx.admin.admin) { + if (!order.community_id) return await messages.notAuthorized(ctx); + if (order.community_id != ctx.admin.default_community_id) { + return await messages.notAuthorized(ctx); + } + } const buyer = await User.findOne({ _id: order.buyer_id }); const seller = await User.findOne({ _id: order.seller_id }); await messages.checkOrderMessage(ctx, order, buyer, seller); } catch (error) { logger.error(error); } }); bot.command('checkinvoice', adminMiddleware, async (ctx: MainContext) => { try { const [orderId] = (await validateParams(ctx, 2, '\\<_order id_\\>'))!; if (!orderId) return; if (!(await validateObjectId(ctx, orderId))) return; const order = await Order.findOne({ _id: orderId }); if (order === null) return; + // Restrict community admins to their community orders + if (!ctx.admin.admin) { + if (!order.community_id) return await messages.notAuthorized(ctx); + if (order.community_id != ctx.admin.default_community_id) { + return await messages.notAuthorized(ctx); + } + } if (!order.hash) return; const invoice = await getInvoice({ hash: order.hash }); if (invoice === undefined) { throw new Error('invoice is undefined'); } await messages.checkInvoiceMessage( ctx, invoice.is_confirmed, invoice.is_canceled!, invoice.is_held!, ); } catch (error) { logger.error(error); } });
Before you needed to be superadmin in order been able to run those commands
Summary by CodeRabbit