Skip to content

fix: validate room existence before generating join token (#154) - #172

Open
Muneerali199 wants to merge 2 commits into
AOSSIE-Org:mainfrom
Muneerali199:fix/154-missing-room-existence-validation
Open

fix: validate room existence before generating join token (#154)#172
Muneerali199 wants to merge 2 commits into
AOSSIE-Org:mainfrom
Muneerali199:fix/154-missing-room-existence-validation

Conversation

@Muneerali199

@Muneerali199 Muneerali199 commented Jun 26, 2026

Copy link
Copy Markdown

Description

Adds room existence validation in the join-room function before generating LiveKit access tokens. Previously, tokens were issued for any room name without checking if the room actually exists in Appwrite.

Changes

  • functions/join-room/src/main.js: Added Appwrite client initialization and room existence check via databases.getDocument() before token generation. Returns 404 if room not found.
  • appwrite.json: Added MASTER_DATABASE_ID and ROOMS_COLLECTION_ID env vars for the join-room function.

Acceptance criteria

  • Room existence verified before token generation
  • Returns 404 with "Room not found" message if no room exists
  • Existing functionality unchanged for valid rooms
  • Error handling follows existing patterns (try/catch with 500 fallback)

Fixes #154

Summary by CodeRabbit

  • Bug Fixes
    • Improved room validation when joining a room, returning a clear “Room not found” response if the requested room doesn’t exist.
    • Strengthened function setup by requiring the necessary environment values for room and database access.
  • Chores
    • Updated deployment configuration to include the required runtime values for the room-joining flow.
    • Updated the join-room function dependency to include the Appwrite SDK (node-appwrite) for database querying.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Join Room Appwrite function now receives database IDs, validates additional Appwrite environment variables, checks room existence before issuing a token, and returns 404 when the room is missing.

Changes

Join Room room validation

Layer / File(s) Summary
Appwrite vars and startup checks
appwrite.json, functions/join-room/package.json, functions/join-room/src/main.js
Join Room now declares ROOMS_COLLECTION_ID and MASTER_DATABASE_ID, adds node-appwrite, and requires the Appwrite environment variables at startup.
Room existence lookup
functions/join-room/src/main.js
The handler creates an Appwrite Databases client, checks roomName in the master rooms collection, and returns 404 when the document is missing.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant JoinRoomFunction
  participant AppwriteDatabases
  Client->>JoinRoomFunction: request with roomName and uid
  JoinRoomFunction->>AppwriteDatabases: listDocuments(name == roomName)
  AppwriteDatabases-->>JoinRoomFunction: room documents or empty list
  JoinRoomFunction-->>Client: 404 "Room not found" or success token response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A bunny hopped to check the door,
and peered for rooms that weren’t there anymore.
With Appwrite keys and a token bright,
it binkied back a 404 night. 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: validating room existence before generating a join token.
Linked Issues check ✅ Passed The PR adds room existence validation, returns 404 when missing, and keeps the join flow unchanged for valid rooms.
Out of Scope Changes check ✅ Passed The changes are focused on the room-validation fix and supporting configuration/dependency updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@functions/join-room/src/main.js`:
- Line 1: The Join Room function imports Client and Databases from
node-appwrite, but the function package dependencies do not include that module.
Update the Join Room function’s package manifest to add node-appwrite alongside
livekit-server-sdk so the import used in main.js resolves during install and
runtime.
- Around line 34-38: The room lookup in join-room’s main flow is using the
user-entered roomName as the Appwrite document ID, but create-room generates a
different unique ID via ID.unique(). Update the room existence check in
join-room’s getDocument logic to query the Rooms collection by the room name
field instead, or change the client flow to pass the वास्तविक room ID returned
by create-room. Use the existing roomExists check in join-room/src/main.js and
the create-room Appwrite document creation path as the reference points.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: d0c321d1-f567-42d3-aa6c-b33d07cd4921

📥 Commits

Reviewing files that changed from the base of the PR and between 795f29b and 2a7757f.

📒 Files selected for processing (2)
  • appwrite.json
  • functions/join-room/src/main.js

Comment thread functions/join-room/src/main.js Outdated
Comment thread functions/join-room/src/main.js Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@functions/join-room/src/main.js`:
- Around line 34-37: The join-room lookup in main.js uses Query.equal("name",
roomName), but the rooms collection does not currently index name, so the query
cannot run reliably. Update the appwrite.json configuration for the rooms
collection to add an index on the name attribute, and verify the join-room flow
continues to use databases.listDocuments with Query.equal against that indexed
field.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06b3bb39-ba83-45ce-acba-ea3256ddddf0

📥 Commits

Reviewing files that changed from the base of the PR and between 2a7757f and bca454a.

📒 Files selected for processing (2)
  • functions/join-room/package.json
  • functions/join-room/src/main.js

Comment on lines +34 to +37
const rooms = await databases.listDocuments(
process.env.MASTER_DATABASE_ID,
process.env.ROOMS_COLLECTION_ID,
[Query.equal("name", roomName)]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the Appwrite schema/config and the room creation contract for the queried field.
fd '^appwrite\.json$' . --exec sh -c '
  echo "### $1"
  sed -n "1,260p" "$1"
' sh {}

fd '^appwrite\.js$' functions/create-room --exec sh -c '
  echo "### $1"
  sed -n "1,220p" "$1"
' sh {}

rg -n -C3 'ROOMS_COLLECTION_ID|Query\.equal|createDocument|listDocuments|\bname\b' \
  appwrite.json functions/create-room functions/join-room

Repository: AOSSIE-Org/Resonate-Backend

Length of output: 34202


🏁 Script executed:

#!/bin/bash
sed -n '846,930p' appwrite.json

Repository: AOSSIE-Org/Resonate-Backend

Length of output: 2865


Add an index for rooms.name
appwrite.json defines rooms.name, but the rooms collection has no indexes. Query.equal("name", roomName) needs an indexed attribute, so joins will fail until name is indexed.

🤖 Prompt for 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.

In `@functions/join-room/src/main.js` around lines 34 - 37, The join-room lookup
in main.js uses Query.equal("name", roomName), but the rooms collection does not
currently index name, so the query cannot run reliably. Update the appwrite.json
configuration for the rooms collection to add an index on the name attribute,
and verify the join-room flow continues to use databases.listDocuments with
Query.equal against that indexed field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing room existence validation in join-room allows tokens for non-existent rooms

1 participant