fix: resolve 403 Forbidden error when adding attendees to existing booking via v1 api#24310
fix: resolve 403 Forbidden error when adding attendees to existing booking via v1 api#24310dhairyashiil wants to merge 1 commit intocalcom:mainfrom
Conversation
…ng bookings via API
|
@dhairyashiil is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
api
keithwillcode
added
the
community-interns
WalkthroughThe change updates the attendee creation API to add an organization owner/admin authorization path. For non-system admins who are org owners/admins, it fetches the booking’s userId, resolves accessible users, and permits attendee creation if access is valid. Existing non-admin checks are retained and expanded across booking ownership, attendee membership, event type ownership, and team owner/admin roles. These checks run in parallel via Promise.all; if none pass, a 403 is thrown. On success, the attendee is created and returned in a public schema with a success message. A 404 path for missing user in non-admin cases is preserved. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/v1/pages/api/attendees/_post.ts (1)
103-130: Scope Prisma queries toidonlyThe three
findManychecks only need to know whether a booking exists, but without aselectthey fetch every scalar on the booking. Tightening these queries toselect: { id: true }keeps us aligned with the Prisma minimal-selection guideline and avoids pulling unnecessary fields.Apply the following diff:
const bookingAsAttendee = prisma.booking.findMany({ where: { id: body.bookingId, attendees: { some: { email: user.email } }, }, + select: { id: true }, }); const bookingAsEventTypeOwner = prisma.booking.findMany({ where: { id: body.bookingId, eventType: { owner: { id: userId }, }, }, + select: { id: true }, }); const bookingAsTeamOwnerOrAdmin = prisma.booking.findMany({ where: { id: body.bookingId, eventType: { team: { members: { some: { userId, role: { in: ["ADMIN", "OWNER"] }, accepted: true }, }, }, }, }, + select: { id: true }, });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/v1/pages/api/attendees/_post.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v1/pages/api/attendees/_post.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v1/pages/api/attendees/_post.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v1/pages/api/attendees/_post.ts
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
| if (accessibleUsersIds.length > 0) { | ||
| const data = await prisma.attendee.create({ | ||
| data: { | ||
| email: body.email, | ||
| name: body.name, | ||
| timeZone: body.timeZone, | ||
| booking: { connect: { id: body.bookingId } }, | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| attendee: schemaAttendeeReadPublic.parse(data), | ||
| message: "Attendee created successfully", | ||
| }; |
There was a problem hiding this comment.
Don’t duplicate the creation logic. If the user lacks access, return an error and exit. Run all checks first; only after they pass, perform the create action once, at the end.
makes code easier to follow and understand
| const bookingAsAttendee = prisma.booking.findMany({ | ||
| where: { | ||
| id: body.bookingId, | ||
| attendees: { some: { email: user.email } }, | ||
| }, | ||
| }); | ||
|
|
||
| const bookingAsEventTypeOwner = prisma.booking.findMany({ | ||
| where: { | ||
| id: body.bookingId, | ||
| eventType: { | ||
| owner: { id: userId }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const bookingAsTeamOwnerOrAdmin = prisma.booking.findMany({ | ||
| where: { | ||
| id: body.bookingId, | ||
| eventType: { | ||
| team: { | ||
| members: { | ||
| some: { userId, role: { in: ["ADMIN", "OWNER"] }, accepted: true }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const [ownerResult, attendeeResult, eventTypeOwnerResult, teamOwnerResult] = await Promise.all([ | ||
| bookingAsOwner, | ||
| bookingAsAttendee, | ||
| bookingAsEventTypeOwner, | ||
| bookingAsTeamOwnerOrAdmin, | ||
| ]); |
There was a problem hiding this comment.
This approach triggers four parallel database calls, which creates unnecessary load on the system. Since the booking table is our largest table, I wouldn’t recommend doing this. We should look for a more efficient solution, as this approach is quite resource-intensive
|
This would be a high-risk PR. l'll handle this in the current sprint and include the implementation in v2. Closing this for now. |
3 participants
What does this PR do?
Earlier we only checked if the person making the API call is:
Other than this, we were sending 403.
This implementation now checks if the user is:
Referred code from this file to implement the logic in current PR:
apps/api/v1/pages/api/bookings/[id]/_auth-middleware.tsVisual Demo
Before:
Before we were getting 403 by using api key of an attendee on the booking,
by.host.and.attendee.before.mov
After:
by.host.and.attendee.after.mov