mirror of
https://github.com/fosrl/pangolin.git
synced 2026-06-06 23:59:02 +00:00
fix(middleware): prevent cross-org site binding in target create/update
Extend verifySiteAccess to check that when req.userOrgId is already set by a prior middleware (e.g. verifyResourceAccess/verifyTargetAccess), the site from req.body.siteId belongs to the same organization. This prevents the cross-organization tunnel boundary bypass where an attacker with resource access in one org binds that resource's target to a site in another org. Add verifySiteAccess to both target route stacks: - PUT /resource/:resourceId/target (after verifyResourceAccess) - POST /target/:targetId (after verifyTargetAccess) The org-match check runs before req.userOrg is overwritten, so the resource's organization context is preserved for comparison. Signed-off-by: Marc Schäfer <git@marcschaeferger.de>
This commit is contained in:
@@ -71,6 +71,15 @@ export async function verifySiteAccess(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (req.userOrgId && site.orgId !== req.userOrgId) {
|
||||||
|
return next(
|
||||||
|
createHttpError(
|
||||||
|
HttpCode.FORBIDDEN,
|
||||||
|
"User does not have access to this site"
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
if (!req.userOrg) {
|
if (!req.userOrg) {
|
||||||
// Get user's role ID in the organization
|
// Get user's role ID in the organization
|
||||||
const userOrgRole = await db
|
const userOrgRole = await db
|
||||||
@@ -128,10 +137,7 @@ export async function verifySiteAccess(
|
|||||||
.where(
|
.where(
|
||||||
and(
|
and(
|
||||||
eq(roleSites.siteId, site.siteId),
|
eq(roleSites.siteId, site.siteId),
|
||||||
inArray(
|
inArray(roleSites.roleId, req.userOrgRoleIds!)
|
||||||
roleSites.roleId,
|
|
||||||
req.userOrgRoleIds!
|
|
||||||
)
|
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
.limit(1)
|
.limit(1)
|
||||||
|
|||||||
@@ -561,6 +561,7 @@ authenticated.delete(
|
|||||||
authenticated.put(
|
authenticated.put(
|
||||||
"/resource/:resourceId/target",
|
"/resource/:resourceId/target",
|
||||||
verifyResourceAccess,
|
verifyResourceAccess,
|
||||||
|
verifySiteAccess,
|
||||||
verifyLimits,
|
verifyLimits,
|
||||||
verifyUserHasAction(ActionsEnum.createTarget),
|
verifyUserHasAction(ActionsEnum.createTarget),
|
||||||
logActionAudit(ActionsEnum.createTarget),
|
logActionAudit(ActionsEnum.createTarget),
|
||||||
@@ -612,6 +613,7 @@ authenticated.get(
|
|||||||
authenticated.post(
|
authenticated.post(
|
||||||
"/target/:targetId",
|
"/target/:targetId",
|
||||||
verifyTargetAccess,
|
verifyTargetAccess,
|
||||||
|
verifySiteAccess,
|
||||||
verifyLimits,
|
verifyLimits,
|
||||||
verifyUserHasAction(ActionsEnum.updateTarget),
|
verifyUserHasAction(ActionsEnum.updateTarget),
|
||||||
logActionAudit(ActionsEnum.updateTarget),
|
logActionAudit(ActionsEnum.updateTarget),
|
||||||
@@ -1234,7 +1236,8 @@ export const authRouter = Router();
|
|||||||
unauthenticated.use("/auth", authRouter);
|
unauthenticated.use("/auth", authRouter);
|
||||||
authRouter.use(
|
authRouter.use(
|
||||||
rateLimit({
|
rateLimit({
|
||||||
windowMs: config.getRawConfig().rate_limits.auth.window_minutes * 60 * 1000,
|
windowMs:
|
||||||
|
config.getRawConfig().rate_limits.auth.window_minutes * 60 * 1000,
|
||||||
max: config.getRawConfig().rate_limits.auth.max_requests,
|
max: config.getRawConfig().rate_limits.auth.max_requests,
|
||||||
keyGenerator: (req) =>
|
keyGenerator: (req) =>
|
||||||
`authRouterGlobal:${ipKeyGenerator(req.ip || "")}:${req.path}`,
|
`authRouterGlobal:${ipKeyGenerator(req.ip || "")}:${req.path}`,
|
||||||
|
|||||||
Reference in New Issue
Block a user