Commit graph

8 commits

Author SHA1 Message Date
Richard Hansen
180983736d security: Enable authorize plugins to grant read-only access 2020-09-27 22:55:49 +01:00
Richard Hansen
304318b618 webaccess: Move pre-authn authz check to a separate hook
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.
2020-09-27 21:19:58 +01:00
Richard Hansen
02757079c0 security: Enable authorize plugins to grant modify-only access 2020-09-26 18:36:36 +01:00
Richard Hansen
72ed1816ec security: Fix authz check for pad names with encoded characters
Also:
  * Minor test cleanups (`function` instead of arrow functions, etc.).
  * Add a test for a case that was previously not covered.
2020-09-26 10:47:27 +01:00
Richard Hansen
94f944160d security: Don't require express_sid if authn not required
This should make it possible to embed a pad in an iframe from another
site as long as `settings.requireAuthentication` is false.
2020-09-24 10:42:41 +01:00
Richard Hansen
a000a93dc6 Refactor startup/shutdown for 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.
2020-09-22 11:07:21 +01:00
Richard Hansen
b80a37173e security: Fix authorization bypass vulnerability
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.
2020-09-15 21:40:25 +01:00
Richard Hansen
9e6d3f3f63 tests: Add authentication, authorization bypass tests 2020-09-15 20:03:30 +01:00