Skip to content

[2.x] Realtime: extensions can't register channels, or hook the client-event relay #4912

Description

@ekumanov

Proposal

Two small extension points in flarum/realtime, so that extensions can build
presence-like features on it without borrowing a channel that belongs to
something else:

  1. Let extensions register websocket channels, authorized server-side against
    their own permission.
  2. Let extensions register a client-event relay, so a relayed event can have
    its sender resolved by the server and its payload split by audience — the
    thing [2.x] fix(realtime): honour user.viewLastSeenAt in the typing indicator #4880 just did for the typing indicator.

They are separable, and (1) is the foundation: without a channel of your own
there is no second audience to split an event into. I have (1) written with
tests and would open it as soon as there's interest — I'd rather get a read on
the direction first than send a PR nobody asked for. (2) is sketched below and I
have real open questions about its shape.

Why

1. Channel names are a closed set

AuthController takes the subject out of the channel name and looks for a method
of that name on itself:

// src/Websocket/Api/AuthController.php:55
if (preg_match('~^private-(?<subject>[a-zA-Z]+)=(?<id>[0-9]+)$~', $channel, $m)) {
    if (method_exists($this, $m['subject']) && call_user_func([$this, $m['subject']], $m['id'])) {
// src/Websocket/Api/AuthController.php:66
if (preg_match('~^presence-(?<subject>[a-z-]+)$~', $channel, $m)) {
    if (! $this->actor->isGuest() && method_exists($this, $m['subject'])

So the only channels that can exist are the ones the controller defines. An
extension can't add a method to it.

Extend\Realtime::authorizePresenceChannel() (src/Extend/Realtime.php:313)
looks like the escape hatch but isn't — it adds a guard to a channel realtime
already defines, and its own docblock example (->authorizePresenceChannel('online', …))
shows the intended scope. There is no "define a channel".

Two further consequences of that regex pair:

  • The presence subject pattern has no digits and no =, so presence channels
    are forum-wide by construction
    . presence-discussion=123 cannot match. This
    is the one that hurts most: presence channels already do member lists and
    member_added/member_removed, which is exactly what a per-object roster
    wants, and they're unreachable for anything scoped to an object.
  • [a-zA-Z]+ can't match a hyphen, which is why private-index-typing-tag={id}
    needs its own branch above the general one (AuthController.php:44).

Because a channel can't be minted, an extension has to put its data on one of
realtime's. The natural one is private-typing={id}, whose audience is "everyone
who can see the discussion" — including logged-out guests, since realtime
subscribes every visitor to it for the typing indicator. A permission the
extension checks before rendering doesn't change who receives the event. Verified
on a running forum: a logged-out tab that binds a handler receives a member's
display name.

Behaviour of the current endpoint (guest, unmodified 2.x)
POST /api/websocket/auth   channel_name=…       →  status
private-typing=1                                →  200   (guests included)
private-index-typing-tag=1                      →  200   (via the special case)
private-readingNow=1                            →  403   (extension can't mint one)
presence-discussion=1                           →  403   (no `=id` on presence)
presence-readingNow                             →  403   (extension can't mint one)

Side effect of method_exists() worth fixing anyway

The subject is matched against every method on the controller, not just the
authorizers, so any method name is a channel name:

private-handle=1   → AuthController::handle('1')  → TypeError → 500
private-online=1   → AuthController::online('1')  → TypeError → 500

Both from an unauthenticated request. No authorization is bypassed —
authorizeChannel() is never reached — so I've treated it as a bug rather than
something for the security address, but say the word if you'd rather it went
there. It disappears once subjects come from a registry.

2. The client-event relay is hardcoded

Message::respond() calls relayTyping(), relayIndexTyping() and
relayComposeTyping() by name (src/Websocket/Message/Message.php:31-42);
anything else falls through to a plain broadcastToEveryoneExcept. There's no
way to register a relay of your own.

That plain relay is the only thing available to an extension, and it means the
payload is whatever the sender's client claimed. #4880 moved typing off exactly
that: identity is now resolved from the socket's authenticated private-user={id}
subscription (Manager::userIdForConnection(), Manager.php:136) and the event
is broadcast twice, in different forms, to two audiences
(Message::relayTyping(), Message.php:120). An extension can have neither
half.

What motivates it

ekumanov/flarum-ext-reading-now —
a live "who is reading this discussion" roster. It works today, on client events
over private-typing={id}, with an announce/keepalive handshake stapled on to
replace the member list a presence channel would have given it for free. Three
things it cannot do, all of them downstream of the above:

  • Show a moderator someone who has hidden their online status. Core treats
    user.viewLastSeenAt as the override for discloseOnline; [2.x] fix(realtime): honour user.viewLastSeenAt in the typing indicator #4880 restored that
    for typing. The roster can't, because putting the name on the shared channel
    discloses it to everyone on that channel.
  • Trust who the sender is. Identity is client-asserted and spoofable.
  • Scope roster visibility to its own permission. It can gate rendering; it
    can't stop the events reaching a browser that shouldn't have them, so on a
    public discussion the roster is public information.

None of these is unique to that extension — they're what anyone gets for building
presence-like features on borrowed channels.

Proposed shape

Part 1 — channels

A ChannelRegistry the extender populates. Channel names stay
{private|presence}-{subject}[={id}]; the subject selects a registration.

(new Extend\Conditional())
    ->whenExtensionEnabled('flarum-realtime', fn () => [
        (new \Flarum\Realtime\Extend\Realtime())
            ->privateChannel('acme-readers', function (User $actor, int $id) {
                $discussion = Discussion::whereVisibleTo($actor)->find($id);

                return $discussion !== null && $actor->can('acme-readers.view', $discussion);
            })
            ->presenceChannel('acme-readers', function (User $actor, ?int $id) {
                // …return the member data to publish, or false
            }),
    ]);

Authorization stays exactly where it is today — an ordinary request, once per
subscription, real actor, full permission machinery — so the websocket server
still does no permission work per event.

In the branch I have:

  • realtime registers its own channels through the same extender rather than
    keeping a private path beside the public one; the subject authorizers move to a
    DefaultChannels class with their docblocks intact and AuthController is
    left routing;
  • subjects may contain hyphens, so the private-index-typing-tag special case
    goes away;
  • presence channels may carry ={id};
  • guest handling is unchanged but now explicit: private channels have never
    required a session so the callback decides, presence channels key their member
    list by user id so guests are refused before it runs;
  • registering a subject twice throws, rather than silently giving one extension's
    channel another's permissions.

Part 2 — client-event relay

Roughly: an extension names an event, the subject it arrives on, an optional
"identified" subject for the privileged audience, and a callback that builds the
payload for a given audience from the server-resolved sender.

(new \Flarum\Realtime\Extend\Realtime())
    ->relayClientEvent(
        event: 'client-acme-reading',
        subject: 'acme-readers',                    // private-acme-readers={id}
        identifiedSubject: 'acme-readersIdentified', // optional second audience
        payload: fn (ClientEventSender $sender, array $data, bool $identified) => $identified
            ? ['id' => $sender->userId, 'displayName' => $sender->displayName]
            : ['id' => null],
    );

Realtime would do what relayTyping() does now: resolve the sender, and either
broadcast once or broadcast the identified form to the identified channel and the
anonymised form to the general one, skipping the identified channel's subscribers
so a privileged viewer sees one event rather than two.

ClientEventSender would be the generalisation of TypingIdentity — user id,
display name, discloseOnline — resolved from the socket and cached briefly,
null when the connection can't be identified so callers fail closed.

Open questions I'd want your steer on before writing it:

  • What decides that a split is needed? Typing splits on discloseOnline.
    Making that the default is convenient and probably right for presence-shaped
    features, but it bakes one preference into a general API. A predicate is more
    honest and more rope.
  • Should the general relay keep working unchanged? I'd keep the existing
    fall-through so registering a relay is purely additive.
  • TypingIdentity is a singleton with a 5s TTL cache. Generalising it means
    every registered relay shares that cache. Fine, or should the TTL be
    per-registration?
  • Is a callback the right unit, or an invokable class the way
    Extend\ApiResource takes field classes?

Suggested split

Two PRs, since (1) stands on its own and is where the review effort is:

  • PR 1 — channels. Written, tested (a unit test on the registry and an
    integration test driving the endpoint, including a regression for the 500s
    above), verified end-to-end against a running forum. Happy to open it whenever.
  • PR 2 — relay hook. After the questions above are settled.

Both are in extensions/realtime only, and PR 1 touches no JS.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions