fix: get bookings handler for pbac and fallback roles#1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 544a43bf4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }): Promise<number[]> { | ||
| return this.getTeamIdsWithPermissions({ userId, permissions: [permission], fallbackRoles }); |
There was a problem hiding this comment.
getTeamIdsWithPermission now accepts orgId, but it never forwards it to getTeamIdsWithPermissions, so org scoping is silently ignored for single-permission calls. Since PermissionCheckService.getTeamIdsWithPermission now passes orgId (e.g., for booking.read), this can return teams from other orgs and expand booking visibility beyond the intended org scope. Consider passing orgId through to keep the permission check correctly scoped.
Useful? React with 👍 / 👎.
| const areUserIdsWithinUserOrgOrTeam = filters.userIds.every((userId) => | ||
| userIdsWhereUserIsAdminOrOwner.includes(userId) | ||
| allAccessibleUserIds.includes(userId) | ||
| ); | ||
|
|
||
| const isCurrentUser = filters.userIds.length === 1 && user.id === filters.userIds[0]; | ||
| const isCurrentUser = filters.userIds.includes(user.id); |
There was a problem hiding this comment.
isCurrentUser now uses filters.userIds.includes(user.id), which means a caller can include their own ID alongside other users and bypass the FORBIDDEN guard. The subsequent queries use filters.userIds directly, so bookings for unauthorized users can be returned whenever the list contains the current user plus any disallowed IDs. This should only treat the “current user” exception as valid when the filter contains only the current user (or otherwise enforce that the filter is a subset of accessible users).
Useful? React with 👍 / 👎.
1 participant
Benchmark PR from agentic-review-benchmarks#1