Conversation
| @@ -2284,6 +2287,15 @@ export default class Reactor { | |||
| getPresence(roomType, roomId, opts = {}) { | |||
| const room = this._rooms[roomId]; | |||
There was a problem hiding this comment.
What happens if someone joins two rooms, different roomType, but same id?
I wonder if now that we are using roomType, we should create a kind of composite key: roomType+roomId
| (.build))) | ||
|
|
||
| (def ^:private ^CelCompiler cel-join-compiler | ||
| (-> (CelCompilerFactory/standardCelCompilerBuilder) |
There was a problem hiding this comment.
do we need a custom one? this looks like cel-runtime-compiler. Maybe the missing thing is ruleParams, but perhaps that's fine? (Maybe at some point we'll want to support ruleParams)
There was a problem hiding this comment.
Yea Claude originally wanted to group this with view-delete but I thought conceptually it might be nicer to separate it
server/src/instant/model/rule.clj
Outdated
| :else errors))) | ||
| [] | ||
| rules)) | ||
| (dissoc rules "$rooms"))) |
There was a problem hiding this comment.
would not add a dissoc here. (same reasoning as in rule-validation-errors)
| {:message (.getMessage cel-issue)}))))))) | ||
| (recur (next paths)))))) | ||
|
|
||
| (defn get-room-program! |
There was a problem hiding this comment.
It feels like there's a lot of special-purpose code written specifically for $rooms here. Why is it necessary to split it out like this?
| (ex/assert-permitted! | ||
| :has-room-join-permission? | ||
| ["$rooms" nil "join"] | ||
| false)) |
There was a problem hiding this comment.
nit: would pass the room-type conditional where, and remove the when-not
| (rs/send-event! store app-id sess-id {:op :join-room-ok | ||
| :room-id room-id | ||
| :client-event-id client-event-id}) | ||
| (catch Exception e |
There was a problem hiding this comment.
Is this necessary? I would have expected this exception would have bubbled up to a top-level exception handler
There was a problem hiding this comment.
Without the try/catch, the error would bubble to handle-instant-exception, which sends a generic {:op :error} message. On the client, the Reactor handles that in _handleReceiveError — but that code doesn't know anything about rooms. It wouldn't set room.error or notify presence subscribers.
So without it, here's what the user would see:
- No join-room-ok sent (permission denied)
- No join-room-error sent (bubbled to generic handler)
- Generic :error sent, but Reactor doesn't route it to room state
- Room stays stuck on isLoading: true forever
The top-level handler includes client-event-id and original-event so Reactor could match it back to the room join. But that would mean adding room-specific routing logic inside the generic error handler
Wdyt?
|
View Vercel preview at instant-www-js-rooms-perms-jsv.vercel.app. |
Adds permission checks for room joins, gated by room type. Rules are defined under a
$roomskey in permissions, keyed by room type, with a singlejoinaction. The CEL context exposesdata.idandauth.Default behavior
$roomsis not defined in rules, all joins succeed (backwards compatible)$roomsis defined, room types not listed fall back to$rooms.$defaultif present, otherwise allowRule syntax
This pattern also let's us expand to other actions we may want to support like
broadcastandsetPresenceHow it works
Client: The client now sends
room-typealongsideroom-idin thejoin-roomWebSocket message. The room type is stored in the Reactor's room state so it persists across reconnects.Server: When handling a
join-roommessage, the server:$roomsrules for the app$roomskey exists → allow (backwards compat)$roomsexists but client didn't sendroom-type→ deny (old clients must upgrade)$default{data: {id: "..."}, auth: ...}join-room-errorinstead ofjoin-room-okFiles changed
client/packages/core/src/Reactor.jsroomTypethroughjoinRoom,_tryJoinRoom, reconnect,subscribePresence,subscribeTopicclient/packages/core/src/index.tsroomTypeto reactorclient/packages/react-common/src/InstantReactRoom.tsroom.typein hooksclient/packages/core/src/rulesTypes.ts$roomstype toInstantRulesserver/src/instant/db/cel.clj"join"toaction->compilerserver/src/instant/model/rule.clj$roomsvalidation,get-room-program!with fallback logicserver/src/instant/reactive/session.cljassert-room-permission!, modifyhandle-join-room!server/test/instant/model/rule_test.cljserver/test/instant/reactive/session_test.cljclient/sandbox/react-nextjs/pages/play/room-perms.tsx