search

cesium-ml / baselayer

Fully customizable (scientific, compute-intensive) web application template
http://cesium-ml.org/baselayer/
31 stars 18 forks source link

Security Improvements #217

Closed dannygoldstein closed 3 years ago

dannygoldstein commented 3 years ago

This PR contains three improvements around security.

  1. I added a Security section to config.defaults.yaml. This allows users to enable or disable "strict" mode. When strict is on, any illegal access detected by self.verify_permissions() is raised as an exception. Otherwise, it is simply issued as a warning. I also added an option to send security reports to a webhook channel (i.e., slack) in strict or non-strict mode.
  2. I augmented the error messages sent to the slack channel with a full traceback to show what handler a particular access violation occurred in.
  3. I vectorized the security checks within self.verify_permissions(). Previously, each record would issue a separate database query for permissions checks. Although each check was fast on its own (~0.5ms), for pages with lots of the same type of record in the session, O(10^4) checks could be issued, adding a few extra seconds to the handler response time. This PR issues one (fast) database query per (access mode, record type) pair, potentially reducing the access checking overhead for pages which deal with many records by several orders of magnitude.
pep8speaks commented 3 years ago

Hello @dannygoldstein! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 39:80: E501 line too long (93 > 79 characters) Line 53:80: E501 line too long (80 > 79 characters)

Comment last updated at 2021-04-02 00:33:20 UTC
dannygoldstein commented 3 years ago

Woops thanks just pushed had to pull first

On Thu, Apr 1, 2021 at 5:28 PM Kyung Min Shin @.***> wrote:

@.**** commented on this pull request.

In app/models.py https://github.com/cesium-ml/baselayer/pull/217#discussion_r606013893:

+

  • if use_webhook:
  • try:
  • requests.post(webhook_url, json={'text': err_msg_w_traceback})
  • except requests.HTTPError as e:
  • post_fail_warn_msg = f'Encountered HTTPError "{e.args[0]}" ' \
  • f'attempting to post AccessError "{err_msg}"' \
  • f'to {webhook_url}.'
  • warnings.warn(post_fail_warn_msg)
  • else:
  • warnings.warn(err_msg)
  • if strict:
  • raise AccessError(err_msg)

  • +def verify(mode, row, accessor):

Did you push the commit to remove this? I still see it

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cesium-ml/baselayer/pull/217#discussion_r606013893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVEFYETME6DWONBEJ4CHHDTGUFSHANCNFSM42HZV4WQ .

-- Danny Goldstein http://astro.caltech.edu/~danny