Any campaign player can update/delete the campaign and manage its players #32

New issue

Closed

opened 2026-08-23 05:42:04 +00:00 by nasandre · 1 comment

nasandre commented

2026-08-23 05:42:04 +00:00

Owner

High (authorization)

Mutation endpoints only check `isCampaignMember` (GM or any player) instead of requiring GM/owner:

`campaignController.ts:111` updateCampaign
`campaignController.ts:150` deleteCampaign — a player can delete the whole campaign
`campaignController.ts:185` addPlayer / `:225` removePlayer — players can invite/remove others
Same pattern in `shipController.ts` (`canAccessShip`) for updateShip/deleteShip: any campaign member can edit/delete any ship in that campaign, even ones they don't own

```ts
const hasAccess = await isCampaignMember(user.userId, id, user.isGM); // true for plain players too
await campaignService.deleteCampaign(id);
```

Suggested fix

Add `isCampaignGM(userId, campaignId)` (campaigns.gm_id === userId OR global GM) and use it for update/delete/player-management on campaigns and for ship delete; keep member-check for reads.

## High (authorization) Mutation endpoints only check \`isCampaignMember\` (GM **or any player**) instead of requiring GM/owner: - \`campaignController.ts:111\` updateCampaign - \`campaignController.ts:150\` deleteCampaign — a player can delete the whole campaign - \`campaignController.ts:185\` addPlayer / \`:225\` removePlayer — players can invite/remove others - Same pattern in \`shipController.ts\` (\`canAccessShip\`) for updateShip/deleteShip: any campaign member can edit/delete any ship in that campaign, even ones they don't own \`\`\`ts const hasAccess = await isCampaignMember(user.userId, id, user.isGM); // true for plain players too await campaignService.deleteCampaign(id); \`\`\` ## Suggested fix Add \`isCampaignGM(userId, campaignId)\` (campaigns.gm_id === userId OR global GM) and use it for update/delete/player-management on campaigns and for ship delete; keep member-check for reads.

nasandre added the

Security

label

2026-08-23 05:42:04 +00:00

nasandre commented

2026-08-23 06:18:48 +00:00

Author

Owner

✅ Fixed (commit pending deploy):

Added isCampaignGM(userId, campaignId, isGM) in campaignService.ts — true only for global GMs or campaigns.gm_id owner
campaignController: updateCampaign / deleteCampaign / addPlayer / removePlayer now use it; getCampaign keeps member-level access
shipController: added canModifyShip() (owner, global GM, or campaign GM) — updateShip/deleteShip switched to it; reads unchanged

Players remain read-only members. Will close after build + verification.

✅ **Fixed** (commit pending deploy): - Added `isCampaignGM(userId, campaignId, isGM)` in `campaignService.ts` — true only for global GMs or `campaigns.gm_id` owner - `campaignController`: updateCampaign / deleteCampaign / addPlayer / removePlayer now use it; getCampaign keeps member-level access - `shipController`: added `canModifyShip()` (owner, global GM, or campaign GM) — updateShip/deleteShip switched to it; reads unchanged Players remain read-only members. Will close after build + verification.

nasandre referenced this issue from a commit

2026-08-23 06:55:48 +00:00

Fix issues #30-#40: security hardening, edit mode, mobile nav, transactions

nasandre closed this issue

2026-08-23 06:56:15 +00:00