Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,9 @@ export class BookingsController_2024_08_13 {
}

@Get("/:bookingUid/recordings")
// @Pbac(["booking.readRecordings"])
@Permissions([BOOKING_READ])
@UseGuards(BookingUidGuard)
// @UseGuards(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
@Pbac(["booking.readRecordings"])
@Permissions([BOOKING_WRITE])
@UseGuards(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
@ApiHeader(API_KEY_OR_ACCESS_TOKEN_HEADER)
@ApiOperation({
summary: "Get all the recordings for the booking",
Expand All @@ -225,21 +224,18 @@ export class BookingsController_2024_08_13 {
`,
})
async getBookingRecordings(@Param("bookingUid") bookingUid: string): Promise<GetBookingRecordingsOutput> {
const recordings = await this.calVideoService.getRecordings(bookingUid);
const recordings = this.calVideoService.getRecordings(bookingUid);

return {
status: SUCCESS_STATUS,
data: recordings,
message:
"This endpoint will require authentication in a future release. Please update your integration to include valid credentials. See https://cal.com/docs/api-reference/v2/introduction#authentication for details.",
};
Comment on lines 226 to 232

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing await causes API to return Promise instead of recordings data.

getRecordings is an async method but is called without await. This returns a Promise object to data instead of the actual recordings array. When serialized to JSON, the Promise becomes {}, breaking the API response.

Compare with line 252 where getTranscripts correctly uses await.

Proposed fix
   async getBookingRecordings(`@Param`("bookingUid") bookingUid: string): Promise<GetBookingRecordingsOutput> {
-    const recordings = this.calVideoService.getRecordings(bookingUid);
+    const recordings = await this.calVideoService.getRecordings(bookingUid);

     return {
       status: SUCCESS_STATUS,
       data: recordings,
     };
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async getBookingRecordings(@Param("bookingUid") bookingUid: string): Promise<GetBookingRecordingsOutput> {
const recordings = await this.calVideoService.getRecordings(bookingUid);
const recordings = this.calVideoService.getRecordings(bookingUid);
return {
status: SUCCESS_STATUS,
data: recordings,
message:
"This endpoint will require authentication in a future release. Please update your integration to include valid credentials. See https://cal.com/docs/api-reference/v2/introduction#authentication for details.",
};
async getBookingRecordings(`@Param`("bookingUid") bookingUid: string): Promise<GetBookingRecordingsOutput> {
const recordings = await this.calVideoService.getRecordings(bookingUid);
return {
status: SUCCESS_STATUS,
data: recordings,
};
}
🤖 Prompt for AI Agents
In `@apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts`
around lines 226 - 232, The getBookingRecordings handler is assigning a Promise
to data because it calls the async method getRecordings without awaiting it;
update the getBookingRecordings method to await
this.calVideoService.getRecordings(bookingUid) (same pattern as getTranscripts)
so recordings holds the resolved array before returning the
GetBookingRecordingsOutput response.

}

@Get("/:bookingUid/transcripts")
// @Pbac(["booking.readRecordings"])
@Pbac(["booking.readRecordings"])
@Permissions([BOOKING_READ])
@UseGuards(BookingUidGuard)
// @UseGuards(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
@UseGuards(BookingPbacGuard, ApiAuthGuard, BookingUidGuard)
Comment on lines +236 to +238

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent guard ordering may cause authorization issues.

The guard order for transcripts (BookingPbacGuard, ApiAuthGuard, BookingUidGuard) differs from recordings (line 217) and conferencing-sessions (line 563) which both use ApiAuthGuard, BookingUidGuard, BookingPbacGuard.

BookingPbacGuard likely depends on user context set by ApiAuthGuard. Running it first may cause failures or unexpected behavior.

Proposed fix - align with recordings/conferencing-sessions order
   `@Pbac`(["booking.readRecordings"])
   `@Permissions`([BOOKING_READ])
-  `@UseGuards`(BookingPbacGuard, ApiAuthGuard, BookingUidGuard)
+  `@UseGuards`(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
   `@ApiHeader`(API_KEY_OR_ACCESS_TOKEN_HEADER)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Pbac(["booking.readRecordings"])
@Permissions([BOOKING_READ])
@UseGuards(BookingUidGuard)
// @UseGuards(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
@UseGuards(BookingPbacGuard, ApiAuthGuard, BookingUidGuard)
`@Pbac`(["booking.readRecordings"])
`@Permissions`([BOOKING_READ])
`@UseGuards`(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
🤖 Prompt for AI Agents
In `@apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts`
around lines 236 - 238, The guards for the transcripts endpoint are ordered
incorrectly: change the `@UseGuards` invocation that currently lists
BookingPbacGuard, ApiAuthGuard, BookingUidGuard to the consistent order
ApiAuthGuard, BookingUidGuard, BookingPbacGuard so that ApiAuthGuard establishes
the user context before BookingPbacGuard runs; update the `@UseGuards` decorator
on the transcripts controller method (the one annotated with
`@Pbac`(["booking.readRecordings"]) and `@Permissions`([BOOKING_READ])) to use
ApiAuthGuard, BookingUidGuard, BookingPbacGuard.

@ApiHeader(API_KEY_OR_ACCESS_TOKEN_HEADER)
@ApiOperation({
summary: "Get Cal Video real time transcript download links for the booking",
Expand All @@ -258,8 +254,6 @@ export class BookingsController_2024_08_13 {
return {
status: SUCCESS_STATUS,
data: transcripts ?? [],
message:
"This endpoint will require authentication in a future release. Please update your integration to include valid credentials. See https://cal.com/docs/api-reference/v2/introduction#authentication for details.",
};
}

Expand Down Expand Up @@ -397,18 +391,18 @@ export class BookingsController_2024_08_13 {
<Note>Please make sure to pass in the cal-api-version header value as mentioned in the Headers section. Not passing the correct value will default to an older version of this endpoint.</Note>
`,
})
async markNoShow(
@Param("bookingUid") bookingUid: string,
@Body() body: MarkAbsentBookingInput_2024_08_13,
@GetUser() user: ApiAuthGuardUser
): Promise<MarkAbsentBookingOutput_2024_08_13> {
const booking = await this.bookingsService.markAbsent(bookingUid, user.id, body, user.uuid);

return {
status: SUCCESS_STATUS,
data: booking,
};
}
async markNoShow(
@Param("bookingUid") bookingUid: string,
@Body() body: MarkAbsentBookingInput_2024_08_13,
@GetUser() user: ApiAuthGuardUser
): Promise<MarkAbsentBookingOutput_2024_08_13> {
const booking = await this.bookingsService.markAbsent(bookingUid, user.id, body, user.uuid);

return {
status: SUCCESS_STATUS,
data: booking,
};
}

@Post("/:bookingUid/reassign")
@HttpCode(HttpStatus.OK)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,51 +185,51 @@ describe("Bookings Endpoints 2024-08-13", () => {
});
});

// describe("GET /v2/bookings/:bookingUid/recordings - Authorization", () => {
// it("should allow booking organizer to access recordings", async () => {
// const calVideoService = app.get(CalVideoService);
// jest.spyOn(calVideoService, "getRecordings").mockResolvedValue([]);

// const response = await request(app.getHttpServer())
// .get(`/v2/bookings/${testBooking.uid}/recordings`)
// .set("Authorization", `Bearer ${ownerApiKey}`)
// .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
// .expect(200);

// expect(response.body.status).toEqual(SUCCESS_STATUS);
// });

// it("should return 403 when unauthorized user tries to access recordings", async () => {
// await request(app.getHttpServer())
// .get(`/v2/bookings/${testBooking.uid}/recordings`)
// .set("Authorization", `Bearer ${unauthorizedApiKey}`)
// .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
// .expect(403);
// });
// });

// describe("GET /v2/bookings/:bookingUid/transcripts - Authorization", () => {
// it("should allow booking organizer to access transcripts", async () => {
// const calVideoService = app.get(CalVideoService);
// jest.spyOn(calVideoService, "getTranscripts").mockResolvedValue([]);

// const response = await request(app.getHttpServer())
// .get(`/v2/bookings/${testBooking.uid}/transcripts`)
// .set("Authorization", `Bearer ${ownerApiKey}`)
// .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
// .expect(200);

// expect(response.body.status).toEqual(SUCCESS_STATUS);
// });

// it("should return 403 when unauthorized user tries to access transcripts", async () => {
// await request(app.getHttpServer())
// .get(`/v2/bookings/${testBooking.uid}/transcripts`)
// .set("Authorization", `Bearer ${unauthorizedApiKey}`)
// .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
// .expect(403);
// });
// });
describe("GET /v2/bookings/:bookingUid/recordings - Authorization", () => {
it("should allow booking organizer to access recordings", async () => {
const calVideoService = app.get(CalVideoService);
jest.spyOn(calVideoService, "getRecordings").mockResolvedValue([]);

const response = await request(app.getHttpServer())
.get(`/v2/bookings/${testBooking.uid}/recordings`)
.set("Authorization", `Bearer ${ownerApiKey}`)
.set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
.expect(200);

expect(response.body.status).toEqual(SUCCESS_STATUS);
});

it("should return 403 when unauthorized user tries to access recordings", async () => {
await request(app.getHttpServer())
.get(`/v2/bookings/${testBooking.uid}/recordings`)
.set("Authorization", `Bearer ${unauthorizedApiKey}`)
.set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
.expect(403);
});
});

describe("GET /v2/bookings/:bookingUid/transcripts - Authorization", () => {
it("should allow booking organizer to access transcripts", async () => {
const calVideoService = app.get(CalVideoService);
jest.spyOn(calVideoService, "getTranscripts").mockResolvedValue([]);

const response = await request(app.getHttpServer())
.get(`/v2/bookings/${testBooking.uid}/transcripts`)
.set("Authorization", `Bearer ${ownerApiKey}`)
.set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
.expect(200);

expect(response.body.status).toEqual(SUCCESS_STATUS);
});

it("should return 403 when unauthorized user tries to access transcripts", async () => {
await request(app.getHttpServer())
.get(`/v2/bookings/${testBooking.uid}/transcripts`)
.set("Authorization", `Bearer ${unauthorizedApiKey}`)
.set(CAL_API_VERSION_HEADER, VERSION_2024_08_13)
.expect(403);
});
});

afterAll(async () => {
await bookingsRepositoryFixture.deleteById(testBooking.id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Type } from "class-transformer";
import { IsEnum, ValidateNested, IsNumber, IsString, IsOptional, IsUrl } from "class-validator";

import { SUCCESS_STATUS, ERROR_STATUS } from "@calcom/platform-constants";
import type { BookingRepository } from "@calcom/features/bookings/lib/BookingRepository";

export class RecordingItem {
@ApiProperty({ example: "1234567890" })
Expand Down Expand Up @@ -55,12 +56,4 @@ export class GetBookingRecordingsOutput {
@ValidateNested({ each: true })
@Type(() => RecordingItem)
data!: RecordingItem[];

@ApiProperty({
example: "This endpoint will require authentication in a future release.",
required: false,
})
@IsString()
@IsOptional()
message?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,4 @@ export class GetBookingTranscriptsOutput {
@IsArray()
@IsString({ each: true })
data!: string[];

@ApiProperty({
example: "This endpoint will require authentication in a future release.",
required: false,
})
@IsString()
message?: string;
}