The old "switch to pad" logic looked buggy, and it complicates pad
initialization. Forcing a refresh after importing an `.etherpad` file
isn't much of a UX downgrade.
It is possible for the stats to be read before the
`expressCreateServer` hook is called (in particular: when there is an
error during startup), which is when the `socketio` variable is set.
Check for non-null `socketio` before attempting to count the number of
socket.io connections.
Before this commit, webaccess.checkAccess saved the authorization in
user.padAuthorizations[padId] with padId being the read-only pad ID,
however later stages, e.g. in PadMessageHandler, use the real pad ID for
access checks. This led to authorization being denied.
This commit fixes it by only storing and comparing the real pad IDs and
not read-only pad IDs.
This fixes test case "authn user readonly pad -> 200, ok" in
src/tests/backend/specs/socketio.js.
This makes it possible to change the rate limiter settings via
`/admin/settings` or by modifying the appropriate settings object and
reinvoking the hook.
It should be the client's responsibility to handle null name or color.
In the case of author names, passing null to the client allows users
to fill in the names of other users (via a suggestUserName
CLIENT_MESSAGE).
When a new client opens a socket.io connection and sends a
CLIENT_READY message, Etherpad sends the new client a bunch of
USER_NEWINFO messages, one per other user already connected to the
pad. When iterating over the other users, filter out those without an
author ID or missing from the global authors database.
Normally I would let `eslint --fix` do this for me, but there's a bug
that causes:
const x = function ()
{
// ...
};
to become:
const x = ()
=> {
// ...
};
which ESLint thinks is a syntax error. (It probably is; I don't know
enough about the automatic semicolon insertion rules to be confident.)
This will be a breaking change for some people.
We removed all internal password control logic. If this affects you, you have two options:
1. Use a plugin for authentication and use session based pad access (recommended).
1. Use a plugin for password setting.
The reasoning for removing this feature is to reduce the overall security footprint of Etherpad. It is unnecessary and cumbersome to keep this feature and with the thousands of available authentication methods available in the world our focus should be on supporting those and allowing more granual access based on their implementations (instead of half assed baking our own).
Previously Etherpad would not pass the correct client IP address through and this caused the rate limiter to limit users behind reverse proxies. This change allows Etherpad to use a client IP passed from a reverse proxy.
Note to devs: This header can be spoofed and spoofing the header could be used in an attack. To mitigate additional *steps should be taken by Etherpad site admins IE doing rate limiting at proxy.* This only really applies to large scale deployments but it's worth noting.
Benefits:
* More functions are now async which makes it possible for future
changes to use await in those functions.
* This will help keep the server from drowning in too many messages
if we ever add acknowledgements or if WebSocket backpressure ever
becomes reality.
* This might make tests less flaky because changes triggered by a
message will complete before the Promise resolves.
Before, the author ID was only saved in the session info during the
initial CLIENT_READY, not when the client sent a CLIENT_READY due to a
reconnect. This caused the handling of subsequent messages to use an
undefined author ID.
Move the handleMessageSecurity and handleMessage hooks after the call
to securityManager.checkAccess.
Benefits:
* A handleMessage plugin can safely assume the message will be
handled unless the plugin itself drops the message, so it doesn't
need to repeat the access checks done by the `handleMessage`
function.
* This paves the way for a future enhancement: pass the author ID to
the hooks.
Note: The handleMessageSecurity hook is broken in several ways:
* The hook result is ignored for `CLIENT_READY` and `SWITCH_TO_PAD`
messages because the `handleClientReady` function overwrites the
hook result. This causes the client to receive client vars with
`readonly` set to true, which causes the client to display an
immutable pad even though the pad is technically writable.
* The formatting toolbar buttons are removed for read-only pads
before the handleMessageSecurity hook even runs.
* It is awkwardly named: Without reading the documentation, how is
one supposed to know that "handle message security" actually means
"grant one-time write access to a read-only pad"?
* It is called for every message even though calls after a
`CLIENT_READY` or `SWITCH_TO_PAD` are mostly pointless.
* Why would anyone want to grant write access when the user visits a
read-only pad URL? The user should just visit the writable pad URL
instead.
* Why would anyone want to grant write access that only lasts for a
single socket.io connection?
* There are better ways to temporarily grant write access (e.g., the
authorize hook).
* This hook is inviting bugs because it breaks a core assumption
about `/p/r.*` URLs.
I think the hook should be deprecated and eventually removed.
A session's sessioninfo could go away asynchronously due to a
disconnect. Grab a reference once and use it throughout the function
to avoid dereferencing a null sessioninfo object.
Where feasible I put the await at the end of the function to
minimize the impact on latency.
My motivation for this change: Eliminate a race condition in tests I
am writing.
