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;
};
```
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).
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.
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.
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.
* `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.
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.
Authentication plugins almost always want to read and modify
`settings.users`. The settings can already be accessed in a few other
ways, but this is much more convenient.
It's not much, but these images will hopefully be downloaded many times. The
smaller they are, the lowest the latency will be.
Command:
optipng {etherpad_basic.png,etherpad_full_features.png}
BEFORE:
$ du -sch *.png
16K etherpad_basic.png
104K etherpad_full_features.png
120K total
AFTER:
$ du -sch *.png
12K etherpad_basic.png
92K etherpad_full_features.png
104K total
In this way, we also gain an explicit place for the default setting (still not
filled in).
No functional changes.
This is in preparation of a future commit by Paul Tiedke.
The mechanism used for determining if the application is being served over SSL
is wrapped by the "express-session" library for "express_sid", and manual for
the "language" cookie, but it's very similar in both cases.
The "secure" flag is set if one of these is true:
1. we are directly serving Etherpad over SSL using the native nodejs
functionality, via the "ssl" options in settings.json
2. Etherpad is being served in plaintext by nodejs, but we are using a reverse
proxy for terminating the SSL for us;
In this case, the user has to be instructed to properly set trustProxy: true
in settings.json, and the information wheter the application is over SSL or
not will be extracted from the X-Forwarded-Proto HTTP header.
Please note that this will not be compatible with applications being served over
http and https at the same time.
The change on webaccess.js amends 009b61b338, which did not work when the SSL
termination was performed by a reverse proxy.
Reference for automatic "express_sid" configuration:
https://github.com/expressjs/session/blob/v1.17.0/README.md#cookiesecureCloses#3561.
With this change, the Dockerfile builds the Docker image from the code
checked out in the local filesystem, instead of downloading a revision
from git.
Implements #3657
Do not touch vendorized files (e.g. libraries that were imported from external
projects).
No functional changes.
Command:
find . -name '*.<EXTENSION>' -type f -print0 | xargs -0 sed -i 's/[[:space:]]*$//'
Next version will be Etherpad 1.8. As planned in #3424, we are going to require
NodeJS >=8.9.0 and npm >= 6.4.
This commit implements that change and updates documentation and scripts.
Subsequent changes will get rid of old idioms, dating back to node < 0.7, that
still survive in the code.
Once migrated to NodeJS 8, we will be able to start working on migrating the
code base from callbacks to async/await, greatly simplifying legibility (see
#3540).
Closes#3557
The .tex source was updated to fix some typos, but the corresponding pdf was not
regenerated.
Command used to generate the pdf:
pdflatex easysync-full-description.tex
The incorporated changes are:
- 49114d2b7a (2014-06-01)
- c7548450c0 (2017-09-14)
The documentation was written on 2011-08-03:
31067f163f: added a documentation for the database structure
The implementation using sha512 instead of bcrypt was committed two days later,
on 2011-08-10:
ce1012438e: added setPublicStatus, getPublicStatus, setPassword and isPasswordProtected
This commit implements the following behaviour:
1. adds a function clientPluginNames() to hooks.js (mimicking what is done in
static.js), which returns an array containing the list of currently installed
client side plugins. The array is eventually empty.
2. calls that function in pad.html at rendering time (thus server-side) to
populate a class attribute.
Example results:
- with no client-side plugins installed:
<div id="editorcontainerbox" class="">
- with some client-side plugins installed:
<div id="editorcontainerbox" class="ep_author_neat ep_adminpads">
Looking at the existing code (src/node/hooks/express/static.js#L39-L57), a
client-side plugin is defined as a plugin that implements at least a client side
hook.
NOTE: there is currently no support for notifying plugin removal/installation
to the connected clients: for now, in order to get an updated class list,
the clients will have to refresh the page.
Fixes#3488
Etherpad 1.6.6 does not run on node <= 5 already.
Node 6.9 is the first LTS release in the 6 series, and comes with npm 3.10.8.
Declarations in package.json are advisory unless the user has set
`engine-strict` config flag.
Updated the docs accordingly.
The new hook does the same as exportHtmlAdditionalTags, but is declared
in another hook to avoid confusion about how to export tags when they
are stored as ['tag', 'value'] on attribute pool.
This complements #2762, as per @Gared suggestions.
