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.
The export request hook wasn't testing if the pad's id was from a read-only
pad before validating with the pad manager.
This includes an extra step that makes the read-only id verification and also
avoids setting the original pad's id as the file's name.
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.
New feature to copy a pad without copying entire history. This is useful to perform a low CPU intensive operation while still copying current pad state.
There are two different ways an author ID becomes associated with a
user: either bound to a token or bound to a session ID. (The token and
session ID come from the `token` and `sessionID` cookies, or, in the
case of socket.io messages, from the `token` and `sessionID` message
properties.) When `settings.requireSession` is true or the user is
accessing a group pad, the session ID should be used. Otherwise the
token should be used.
Before this change, the `/p/:pad/import` handler was always using the
token, even when `settings.requireSession` was true. This caused the
following error because a different author ID was bound to the token
versus the session ID:
> Unable to import file into ${pad}. Author ${authorID} exists but he
> never contributed to this pad
This bug was reported in issue #4006. PR #4012 worked around the
problem by binding the same author ID to the token as well as the
session ID.
This change does the following:
* Modifies the import handler to use the session ID to obtain the
author ID (when appropriate).
* Expands the documentation for the SecurityManager checkAccess
function.
* Removes the workaround from PR #4012.
* Cleans up the `bin/createUserSession.js` test script.
Includes settings
Includes i18n
Includes a nice notification
Disconnects on rate limit
Includes feeding into metrics/stats
Include console warn to server console.
Before this change, a client would require two versions of the same assets (with
and without randomVersionString), wasting resources and triggering all sorts of
hard to debug inconsistencies.
This change should have been part of 95fd5ce2a4 and completes it.
From Etherpad 1.8.3 onwards, the maximum allowed size for a single imported
file will always be bounded.
The maximum allowed size can be configured via importMaxFileSize.
This change is meant to ease using LibreOffice as converter. When LibreOffice
converts a file, it adds some classes to the <title> tag.
This is a quick & dirty way of matching the <title> and comment it out
independently on the classes that are set on it.
Clearing the authorship colors of a document with at least two authors, and then
undoing that action caused a disconnect from the pad.
This change disallows undoing clearing authorship colors in order to prevent
the problem from affecting users, and adds the relative test coverage.
This is a change of behaviour, and is documented in the changelog.
Fixes#2802 (sidestepping it).
For some reason authorInfo is sometimes null, and therefore it is not possible
to get colorId from it.
This resulted in the following stack trace:
[2020-03-16 09:27:17.291] [ERROR] console - (node:1746) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'colorId' of null
at <BASEDIR>/src/node/handler/PadMessageHandler.js:1199:37
at runMicrotasks (<anonymous>)
at processTicksAndRejections (internal/process/task_queues.js:97:5)
at async Promise.all (index 0)
at async handleClientReady (<BASEDIR>/src/node/handler/PadMessageHandler.js:1171:5)
[2020-03-16 09:27:17.291] [ERROR] console - (node:1746) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 76)
[2020-03-16 09:27:19.034] [WARN] message - Dropped message, USERINFO_UPDATE Session not ready.[object Object]
Which is due to a bug in Etherpad that we are not going to solve now.
As a workaround, when this happens, let's set the username to "Anonymous" (if
it is not already set), and colorId to the fixed value "#daf0b2". Warning
messages are written in the logs to signal this condition.
This is no definitive solution, but fixes#3612 (via a workaround).
