feat: add BigBlueButton conferencing app#29474
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
Welcome to Cal.diy, @holygeek00! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a BigBlueButton app: metadata and Zod schemas for required config keys, a VideoApiAdapter implementing meeting create/update/delete with SHA1-signed BigBlueButton API URLs and response checks, unit tests and test helpers, an installation API that enforces auth/team-admin checks and persists credentials to Prisma, barrel exports, and registration of the app across generated app-store registry files. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app-store/bigbluebutton/api/add.ts`:
- Around line 21-24: The team-admin authorization call
throwIfNotHaveAdminAccessToTeam is executed outside the try/catch and can throw
before your normalized API error handling; move or wrap this invocation (and the
similar occurrences around lines referencing the same check, e.g., the other
call at 56-60) inside the existing try/catch block (or add a surrounding
try/catch) so any thrown errors are caught and returned via your standard error
response path; update references to numericTeamId and req.session.user.id usage
accordingly inside that protected block.
- Around line 33-51: The find/create queries for credentials
(prisma.credential.findFirst and prisma.credential.create in add.ts where
variables alreadyInstalled and installation are used) are returning the full
record including the sensitive key; change both queries to use explicit select {
id: true } so you only fetch the id (avoid returning credential.key) — for
findFirst add select: { id: true } to the query and for create wrap the create
call with select: { id: true } so only the new credential id is returned and no
key data is exposed.
- Around line 8-12: The handler currently permits state-changing requests
without HTTP method checks; add an explicit POST-only gate at the top of the
export default async function handler(req, res) that returns 405 for any
non-POST: if req.method !== 'POST' then set the Allow header to 'POST' and
respond with res.status(405).json({ message: 'Method Not Allowed' }) and return;
apply the same POST-only check to the other mutating endpoint in this file (the
other export default handler that performs install/uninstall logic) so all
state-changing API routes reject non-POST methods.
In `@packages/app-store/bigbluebutton/lib/VideoApiAdapter.test.ts`:
- Around line 5-12: Add a regression test that covers server URLs ending with
"/api" (no trailing slash) to prevent duplication of the API segment; update the
test in VideoApiAdapter.test.ts to call testHelpers.normalizeServerUrl with a
value like "https://bbb.example.com/api" and assert the result.toString() equals
"https://bbb.example.com/api/" so normalizeServerUrl (the helper under test) is
verified to append a single trailing slash and not duplicate "api".
In `@packages/app-store/bigbluebutton/lib/VideoApiAdapter.ts`:
- Around line 15-17: Replace the fixed ATTENDEE_PASSWORD and MODERATOR_PASSWORD
constants with per-meeting, cryptographically-generated passwords: remove those
static constants and generate strong random strings (e.g., using Node's
crypto.randomBytes or a secure UUID) when a meeting is created, assign them to
the meeting object, and use those generated values wherever
ATTENDEE_PASSWORD/MODERATOR_PASSWORD were referenced (e.g., in the createMeeting
logic and join URL construction inside VideoApiAdapter.ts and the functions that
build join links). Ensure the generated passwords are returned/stored with the
meeting metadata (instead of global constants) so getJoinUrl/joinMeeting (or
similarly named methods) read the per-meeting passwords; validate length/entropy
and avoid logging the raw secrets.
- Around line 55-57: The callBigBlueButtonApi function lacks a network timeout;
update callBigBlueButtonApi to use an AbortController with a configurable
timeout (e.g., constant or env var) so the fetch(url) is aborted after the
timeout; create the controller, pass controller.signal to fetch, schedule a
setTimeout to call controller.abort(), clear the timeout after response/text is
received, and ensure you surface/handle the abort error (e.g., rethrow or wrap
with a descriptive error) so callers know the request timed out.
- Around line 24-26: The pathname normalization in VideoApiAdapter.ts
incorrectly transforms a URL ending with "/api" into ".../api/api/"; update the
logic in the code that manipulates url.pathname (the block that checks
url.pathname.endsWith("/api/")) to explicitly handle both "/api" and "/api/"
cases and ensure the final pathname ends with exactly "/api/" without
duplicating "api" (e.g., detect and replace any trailing "/api" or "/api/" with
"/api/"). Keep this change localized to the pathname-normalization code path
that currently sets url.pathname = `${url.pathname.replace(/\/$/, "")}/api/`; to
avoid double-appending.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a11d0df-ad8f-4ed1-8d1a-def473fcfd36
⛔ Files ignored due to path filters (2)
packages/app-store/bigbluebutton/static/icon.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
packages/app-store/apps.keys-schemas.generated.tspackages/app-store/apps.metadata.generated.tspackages/app-store/apps.schemas.generated.tspackages/app-store/apps.server.generated.tspackages/app-store/bigbluebutton/DESCRIPTION.mdpackages/app-store/bigbluebutton/_metadata.tspackages/app-store/bigbluebutton/api/add.tspackages/app-store/bigbluebutton/api/index.tspackages/app-store/bigbluebutton/index.tspackages/app-store/bigbluebutton/lib/VideoApiAdapter.test.tspackages/app-store/bigbluebutton/lib/VideoApiAdapter.tspackages/app-store/bigbluebutton/lib/index.tspackages/app-store/bigbluebutton/package.jsonpackages/app-store/bigbluebutton/zod.tspackages/app-store/bookerApps.metadata.generated.tspackages/app-store/video.adapters.generated.ts
holygeek00
changed the title
|
Could a maintainer please add the |
|
|
|
CLA is signed now. Could a maintainer please add the Latest local verification:
|
|
The required workflow is still blocked by the external-contributor
Could a maintainer please review and add Current status:
|
|
please attach a working demo in the description. making it draft til then. |
3 participants
/claim #1985
What changed
Adds a BigBlueButton conferencing app integration that can be configured with a BigBlueButton server URL and shared secret.
The implementation:
createAPI calljoinAPI callendAPI callVerification
node .yarn/releases/yarn-4.12.0.cjs vitest run packages/app-store/bigbluebutton/lib/VideoApiAdapter.test.tsnode .yarn/releases/yarn-4.12.0.cjs workspace @calcom/app-store type-check