Commit Graph

8 Commits

Author SHA1 Message Date
Alex Vandiver 012113c542 models: Force the translated role into a translated string.
The return type of `ugettext_lazy('...')` (aliased as `_`) is a
promise, which is only forced into a string when it is dealt with in
string context.  This `django.utils.functional.lazy.__proxy__` object
is not entirely transparent, however -- it cannot be serialized by
`orjson`, and `isinstance(x, str) == False`, which can lead to
surprising action-at-a-distance.

In the two places which will serialize the role value (either into
Zulip's own error reporting queue, or Sentry's), force the return
value.  Failure to do this results in errors being dropped
mostly-silently, as they cannot be serialized and enqueued by the
error reporter logger, which has no recourse but to just log a
warning; see previous commit.

When we do this forcing, explicitly override the language to be the
realm default.  Failure to provide this override would translate the
role into the role in the language of the _request_, yielding varying
results.
2020-09-03 17:26:54 -07:00
Alex Vandiver 7941ea915e sentry: Ignore SuspiciousOperation exceptions themselves.
596cf2580b ignored the loggers of all SuspiciousOperation subclasses,
but not SuspiciousOperation itself.  Almost all locations raise one of
the more specific subclasses, with the exception of one location in
the session middleware[1].

Ignore the overall django.security.SuspiciousOperation logger as well.

[1] https://code.djangoproject.com/ticket/31962
2020-08-30 15:27:51 -07:00
Alex Vandiver 596cf2580b sentry: Ignore all SuspiciousOperation loggers.
django.security.DisallowedHost is only one of a set of exceptions that
are "SuspiciousOperation" exceptions; all return a 400 to the user
when they bubble up[1]; all of them are uninteresting to Sentry.
While they may, in bulk, show a mis-configuration of some sort of the
application, such a failure should be detected via the increase in
400's, not via these, which are uninteresting individually.

While all of these are subclasses of SuspiciousOperation, we enumerate
them explicitly for a number of reasons:

 - There is no one logger we can ignore that captures all of them.
   Each of the errors uses its own logger, and django does not supply
   a `django.security` logger that all of them feed into.

 - Nor can we catch this by examining the exception object.  The
   SuspiciousOperation exception is raised too early in the stack for
   us to catch the exception by way of middleware and check
   `isinstance`.  But at the Sentry level, in `add_context`, it is no
   longer an exception but a log entry, and as such we have no
   `isinstance` that can be applied; we only know the logger name.

 - Finally, there is the semantic argument that while we have decided
   to ignore this set of security warnings, we _may_ wish to log new
   ones that may be added at some point in the future.  It is better
   to opt into those ignores than to blanket ignore all messages from
   the security logger.

This moves the DisallowedHost `ignore_logger` to be adjacent to its
kin, and not on the middleware that may trigger it.  Consistency is
more important than locality in this case.

Of these, the DisallowedHost logger if left as the only one that is
explicitly ignored in the LOGGING configuration in
`computed_settings.py`; it is by far the most frequent, and the least
likely to be malicious or impactful (unlike, say, RequestDataTooBig).

[1] https://docs.djangoproject.com/en/3.0/ref/exceptions/#suspiciousoperation
2020-08-12 16:08:38 -07:00
Alex Vandiver 2e97d2b9f7 sentry: Provide ZULIP_VERSION as "release". 2020-08-12 11:56:48 -07:00
Alex Vandiver 83645a3115 sentry: Ignore SystemExit and similar exceptions.
There are three exceptions in Python3 which are descended from
BaseException, but not Exception: GeneratorExit, KeyboardInterrupt,
and SystemExit.  None of these are suitable to be sent to Sentry.
For example, SystemExit is raised by `sys.exit`; in that sense, it is
never "uncaught" because we chose to cause it explicitly.

Use the suggested form[1] for ignoring specific classes of exceptions.

[1] https://github.com/getsentry/sentry-python/issues/149#issuecomment-434448781
2020-08-09 16:49:12 -07:00
Clara Dantas f9cbefac41 sentry: Add user role in user_info object.
As part of issue #15344, the error report emails add the user role
information. This commit adds the user role information to be used
by sentry as well.
2020-08-03 13:35:51 -07:00
Alex Vandiver 1c8f4b936b sentry: Capture realm name in user information. 2020-07-27 11:07:55 -07:00
Alex Vandiver bfa809181a sentry: Allow reporting errors to sentry.io.
Use the default configuration, which catches Error logging and
exceptions.  This is placed in `computed_settings.py` to match the
suggested configuration from Sentry[1], which places it in `settings.py`
to ensure it is consistently loaded early enough.

It is placed behind a check for SENTRY_DSN soas to not incur the
additional overhead of importing the `sentry_sdk` modules if Sentry is
not configured.

[1] https://docs.sentry.io/platforms/python/django/
2020-07-27 11:07:55 -07:00