* fix accidental write to global variable
properly show pending tests
log test name in suite
better log output for received/expected strings
* cc tests: enable second nestedOL test
* ignore the head tag on import
These characters are in the RFC3986 reserved set.
These characters are added to the set of characters that cannot be the
last character of a URL to avoid mislinkification.
This makes the strings easier to read, and it simplifies `append()`.
Also fix some lint errors:
* Use `const` instead of `var`.
* Convert `append()` to an arrow function.
* Wrap long lines.
eslint-config-etherpad 1.0.11 changed the comma-dangle rule to
prohibit trailing commas for function arguments. See:
673ab07acf
Re-run the automated fixes to apply the rule change.
This also fixes a few lint issues in changes that were made after
`eslint --fix` was originally run.
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.)
Travis placed an unnecessary breaking restriction on our tests and failed to respond within 72 hours to our complaint. This has forced us to introduce Github Actions to manage our testing. This is hopefully a temporary measure while Travis either gets itself together or we find a non-Github requirement.
* caching_middleware: fix gzip compression not triggered
* packages: If a client sets `Accept-Encoding: gzip`, the responseCache will
include `Content-Encoding: gzip` in all future responses, even
if a subsequent request does not set `Accept-Encoding` or another client
requests the file without setting `Accept-Encoding`.
Fix that.
* caching_middleware: use `test` instead of `match`
* add tests
* make code easier to understand
* make the regex more clear
* Avoid bashisms.
* Simplify `sed` of `settings.json`.
* Wrap long lines.
* Define and use the conventional log functions.
* Quote variable expansions.
* Fix bad paren placement in `/javascript` handler
This fixes a bug introduced in commit
ed5a635f4c.
* add regression test for #4495
* Move `/javascript` test to `specialpages.js`
Co-authored-by: webzwo0i <webzwo0i@c3d2.de>
If `settings.json` contains a user without a `password` property then
nobody should be able to log in as that user using the built-in HTTP
basic authentication. This is true both with and without this change,
but before this change it wasn't immediately obvious that a malicious
user couldn't use an empty or null password to log in as such a user.
This commit adds an explicit nullish check and some unit tests to
ensure that an empty or null password will not work if the `password`
property is null or undefined.
Rewrite the `callAll` and `aCallAll` functions to support all
reasonable hook behaviors and to report errors for unreasonable
behaviors (e.g., calling the callback twice).
Now a hook function like the following works as expected when invoked
by `aCallAll`:
```
exports.myHookFn = (hookName, context, cb) => {
cb('some value');
return;
};
```
* don't include sendkeys in index.html as it's included in helper.init
mocha opts: add default timeout and replace ignoreLeaks with checkLeaks,
as the former is deprecated
* introduce helper.edit to write to a pad
* add test to check if helper.edit() supports line numbers
* helper tests: waitFor/waitForPromise seem to be a little bit faster sometimes
* tests: refactor chat.js
* tests: refactor timeslider_numeric_padID
* tests: refactor timeslider_labels
* tests: refactor timeslider_follow
* ensure followContents is enabled, although it should be by default
* timeslider_follow: increase number of revision for Edge
* make textLines() depend on linesDiv()
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
* make linesDiv return standard Array
* use `contain` instead of `indexOf`
* more fixes from the review
* review fixes
* align waitFor and waitForPromise behaviour
* timeslider_follow: check if it's following to the correct lines
* lower expected waitFor/waitForPromise interval check
* disable responsivness and regression test in timeslider_follow
* timeslider_follow: fix Range detection
* more explicit test for linesDiv
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
* Avoid a false positive if a Promise that is expected to reject
doesn't reject.
* Use modern JavaScript language features: arrow functions,
`const`/`let` instead of `var`.
* Remove the tests that test Promise behavior.
* Add new test that checks that it returns a Promise.
There are a few problems with sleeping before checking the condition
for the first time:
* It slows down tests.
* The predicate is never checked if the interval duration is greater
than the timeout.
* 0 can't be used to test if the condition is currently true.
There is a minor disadvantage to sleeping before checking: It will
cause more tests to run without an asynchronous interruption, which
could theoretically mask some async bugs.
The `helper.waitFor()` function returns a jQuery Deferred object.
Deferred objects are supposed to have a `.fail()` method that is
chainable (it should return `this`). Before this change,
`helper.waitFor()` monkey-patched the `.fail()` method with a function
that returned `undefined`. Now the monkey-patched `.fail()` returns
the Deferred object.
Also modernize the code a bit.
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).
This makes it easier to see the test results, and it hides some
scary-looking but intentional error messages.
This code will likely have to be updated if/when we change the logging
library (see issue #1922).
This makes it possible to test various settings combinations and
examine internal state to confirm correct behavior. Also, the user
doesn't need to start an Etherpad server before running these tests.
There's no need to perform an authentication check in the socket.io
middleware because `PadMessageHandler.handleMessage` calls
`SecurityMananger.checkAccess` and that now performs authentication
and authorization checks.
This change also improves the user experience: Before, access denials
caused socket.io error events in the client, which `pad.js` mostly
ignores (the user doesn't see anything). Now a deny message is sent
back to the client, which causes `pad.js` to display an obvious
permission denied message.
This also fixes a minor bug: `settings.loadTest` is supposed to bypass
authentication and authorization checks, but they weren't bypassed
because `SecurityManager.checkAccess` did not check
`settings.loadTest`.
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.
Before this change, the authorize hook was invoked twice: once before
authentication and again after (if settings.requireAuthorization is
true). Now pre-authentication authorization is instead handled by a
new preAuthorize hook, and the authorize hook is only invoked after
the user has authenticated.
Rationale: Without this change it is too easy to write an
authorization plugin that is too permissive. Specifically:
* If the plugin does not check the path for /admin then a non-admin
user might be able to access /admin pages.
* If the plugin assumes that the user has already been authenticated
by the time the authorize function is called then unauthenticated
users might be able to gain access to restricted resources.
This change also avoids calling the plugin's authorize function twice
per access, which makes it easier for plugin authors to write an
authorization plugin that is easy to understand.
This change may break existing authorization plugins: After this
change, the authorize hook will no longer be able to authorize
non-admin access to /admin pages. This is intentional. Access to admin
pages should instead be controlled via the `is_admin` user setting,
which can be set in the config file or by an authentication plugin.
Also:
* Add tests for the authenticate and authorize hooks.
* Disable the authentication failure delay when testing.
Three of the four tests fail if `settings.allowAnyoneToImport` is
false. The fourth ("tries to import Plain Text to a pad that does not
exist") isn't particularly useful when `settings.allowAnyoneToImport`
is false: That test tests an import failure mode, and when
`settings.allowAnyoneToImport` is false the failure could be caused by
that instead of the expected cause.
This makes it possible for reverse proxies to transform 403 errors
into something like "upgrade to a premium account to access this
pad".
Also add some webaccess tests.
* `src/node/server.js` can now be run as a script (for normal
operation) or imported as a module (for tests).
* Move shutdown actions to `src/node/server.js` to be close to the
startup actions.
* Put startup and shutdown in functions so that tests can call them.
* Use `await` instead of callbacks.
* Block until the HTTP server is listening to avoid races during
test startup.
* Add a new `shutdown` hook.
* Use the `shutdown` hook to:
* close the HTTP server
* call `end()` on the stats collection to cancel its timers
* call `terminate()` on the Threads.Pool to stop the workers
* Exit with exit code 0 (instead of 1) on SIGTERM.
* Export the HTTP server so that tests can get the HTTP server's
port via `server.address().port` when `settings.port` is 0.
Before this change, `promises.timesLimit()` created `concurrency - 1`
too many promises. The only users of this function use a concurrency
of 500, so this meant that 499 extra promises were created each time
it was used. The bug didn't affect correctness, but it did result in a
large number of unnecessary database operations whenever a pad was
deleted. This change fixes that bug.
Also:
* Convert the function to async and have it resolve after all of the
created promises are resolved.
* Reject concurrency of 0 (unless total is 0).
* Document the function.
* Add tests.
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.
Before, a malicious user could bypass authorization restrictions
imposed by the authorize hook:
* Step 1: Fetch any resource that the malicious user is authorized to
access (e.g., static content).
* Step 2: Use the signed express_sid cookie generated in step 1 to
create a socket.io connection.
* Step 3: Perform the CLIENT_READY handshake for the desired pad.
* Step 4: Profit!
Now the authorization decision made by the authorize hook is
propagated to SecurityManager so that it can approve or reject
socket.io messages as appropriate.
This also sets up future support for per-user read-only and
modify-only (no create) authorization levels.
Richard Hansen