[2.x] Realtime: extensions can't register channels, or hook the client-event relay
- Dominant language
- PHP
- Stars
- 6.7k
- Forks
- 883
- Avg merge
- 15h 16m
- Merged PRs (30d)
- 73
Description
## 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 #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:
```php
// src/Websocket/Api/AuthController.php:55
if (preg_match('~^private-(?[a-zA-Z]+)=(?[0-9]+)$~', $channel, $m)) {
if (method_exists($this, $m['subject']) && call_user_func([$this, $m['subject']], $m['id'])) {
```
```php
// src/Websocket/Api/AuthController.php:66
if (preg_match('~^presence-(?[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`](https://github.com/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`; #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.
```php
(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.
```php
(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.
Contributor guide
Research direction
Start with src/Websocket/Api/AuthController.php, src/Extend/Realtime.php, src/Websocket/Message/Message.php, and Manager.php to understand current channel authorization and relays. Review the proposed registry changes and the open API-shape questions before choosing a scope. Done means a channel registry with unit coverage, endpoint integration coverage including the 500 regression, and agreement on whether the relay hook belongs in a separate PR.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend, backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100