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
Our intent throughout the codebase is to treat email
case-insensitively.
The only codepath affected by this bug is remote_user_sso, as that's the
only one that currently passes potentially both a user_profile and
ExternalAuthDataDict when creating the ExternalAuthResult. That's why we
add a test specifically for that codepath.
This commit adds EMAIL_PORT setting for explicitly specifying the
port of SMTP provider in dev_settings.py.
We also change email_backends.send_email_smtp to pass EMAIL_PORT
along with EMAIL_HOST to smtplib.SMTP.
After this change, we will not need to include the port along with
host in EMAIL_HOST.
Also updated the email.md docs accordingly for this change.
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.
The apple developer webapp consistently refers this App ID. So,
this clears any confusion that can occur.
Since python social auth only requires us to include App ID in
_AUDIENCE(a list), we do that in computed settings making it easier for
server admin and we make it much clear by having it set to
APP_ID instead of BUNDLE_ID.
Uses git release as this version 3.4.0 is not released to pypi.
This is required for removing some overriden functions of
apple auth backend class AppleAuthBackend.
With the update we also make following changes:
* Fix full name being populated as "None None".
c5c74f27dd that's included in update assigns first_name and last_name
to None when no name is provided by apple. Due to this our
code is filling return_data['full_name'] to 'None None'.
This commit fixes it by making first and last name strings empty.
* Remove decode_id_token override.
Python social auth merged the PR we sent including the changes
we made to decode_id_token function. So, now there is no
necessity for the override.
* Add _AUDIENCE setting in computed_settings.py.
`decode_id_token` is dependent on this setting.
This lets us test the recursion bug behavior of this logging handler
without resulting in `logging.error` output being printed to the
console in the event that the test passes.
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/
Fixes#15904.
settings is supposed to be a proper OneLogin_Saml2_Settings object,
rather than an empty dictionary. This bug wasn't easy to spot because
the codepath that causes this to demonstrate runs only if the
SAMLResponse contains encrypted assertions.
In particular importing gitter data leads to having accounts with these
noreply github emails. We generally only want users to have emails that
we can actually send messages to, so we'll keep the old behavior of
disallowing sign up with such an email address. However, if an account
of this type already exists, we should allow the user to have access to
it.
A few major themes here:
- We remove short_name from UserProfile
and add the appropriate migration.
- We remove short_name from various
cache-related lists of fields.
- We allow import tools to continue to
write short_name to their export files,
and then we simply ignore the field
at import time.
- We change functions like do_create_user,
create_user_profile, etc.
- We keep short_name in the /json/bots
API. (It actually gets turned into
an email.)
- We don't modify our LDAP code much
here.
This particular commit has been a long time coming. For reference,
!avatar(email) was an undocumented syntax that simply rendered an
inline 50px avatar for a user in a message, essentially allowing
you to create a user pill like:
`!avatar(alice@example.com) Alice: hey!`
---
Reimplementation
If we decide to reimplement this or a similar feature in the future,
we could use something like `<avatar:userid>` syntax which is more
in line with creating links in markdown. Even then, it would not be
a good idea to add this instead of supporting inline images directly.
Since any usecases of such a syntax are in automation, we do not need
to make it userfriendly and something like the following is a better
implementation that doesn't need a custom syntax:
` Alice: hey!`
---
History
We initially added this syntax back in 2012 and it was 'deprecated'
from the get go. Here's what the original commit had to say about
the new syntax:
> We'll use this internally for the commit bot. We might eventually
> disable it for external users.
We eventually did start using this for our github integrations in 2013
but since then, those integrations have been neglected in favor of
our GitHub webhooks which do not use this syntax.
When we copied `!gravatar` to add the `!avatar` syntax, we also noted
that we want to deprecate the `!gravatar` syntax entirely - in 2013!
Since then, we haven't advertised either of these syntaxes anywhere
in our docs, and the only two places where this syntax remains is
our game bots that could easily do without these, and the git commit
integration that we have deprecated anyway.
We do not have any evidence of someone asking about this syntax on
chat.zulip.org when developing an integration and rightfully so- only
the people who work on Zulip (and specifically, markdown) are likely
to stumble upon it and try it out.
This is also the only peice of code due to which we had to look up
emails -> userid mapping in our backend markdown. By removing this,
we entirely remove the backend markdown's dependency on user emails
to render messages.
---
Relevant commits:
- Oct 2012, Initial commit c31462c278
- Nov 2013, Update commit bot 968c393826
- Nov 2013, Add avatar syntax 761c0a0266
- Sep 2017, Avoid email use c3032a7fe8
- Apr 2019, Remove from webhook 674fcfcce1
We use the EMAIL_TIMEOUT django setting to timeout after 15s of trying
to send an email. This will nicely lead to retries in the email_senders
queue, due to the retry_send_email_failures decorator.
smtlib documentation suggests that socket.timeout can be raised as the
result of timing out, so in attempts I'm getting
smtplib.SMTPServerDisconnected. Either way, seems appropriate to add
socket.timeout to the exception that we catch.
I checked that this does not interfere with the MRO of the auth
backends:
In [1]: import zproject.backends; zproject.backends.GitHubAuthBackend.__mro__
Out[1]:
(zproject.backends.GitHubAuthBackend,
zproject.backends.SocialAuthMixin,
zproject.backends.ZulipAuthMixin,
zproject.backends.ExternalAuthMethod,
abc.ABC,
social_core.backends.github.GithubOAuth2,
social_core.backends.oauth.BaseOAuth2,
social_core.backends.oauth.OAuthAuth,
social_core.backends.base.BaseAuth,
object)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We remove support for the old clients which required an event for
each message to clear notification.
This is justified since it has been around 1.5 years since we started
supporting the bulk operation (and so essentially nobody is using a
mobile app version so old that it doesn't support the batched
approach) and the unbatched approach has a maintenance and reliability
cost.
There is still some miscellaneous cleanup that
has to happen for things like analytics queries
and dead code in node tests, but this should
remove the main use of pointers in the backend.
(We will also still need to drop the DB field.)
Due to authentication restrictions, a deployment may need to direct
traffic for mobile applications to an alternate uri to take advantage
alternate authentication mechansism. By default the standard realm URI
will be usedm but if overridden in the settings file, an alternate uri
can be substituted.
Because of other validation on these values, I don't believe any of
these does anything different, but these changes improve readability
and likely make GitHub's code scanners happy.
I believe the Bundle ID (aka App ID) and Services ID have meaning only
relative to a specific Team ID. In particular, in some places in the
developer.apple.com UI, they're displayed in a fully-qualified form
like "ABCDE12345.com.example.app", where "com.example.app" is the
App ID or Services ID and ABCDE12345 is the Team ID.
Adds the ability to set a SAML attribute which contains a
list of subdomains the user is allowed to access. This allows a Zulip
server with multiple organizations to filter using SAML attributes
which organization each user can access.
Cleaned up and adapted by Mateusz Mandera to fit our conventions and
needs more.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
This migrations use of url() to path() or re_path(). In this commit,
we only migration regular expressions to path where the translation is trivial:
* URLs with no parameters in them
* URLs with only integer parameters in them
* Strings where there regular expression just checked for `/`s
path; strings, which can have variable validation in the URLs that
need by-hand auditing, we leave for future commits that are easier to
review and think about the individual changes.
Modified by tabbott to convert back to `re_path` various URLs with
strings that had been converted to use `path()` with string
validation to simplify review.
Fixes#14770.
Fixes#14960.
The default of 6 thread may not be appropriate in certain
configurations. Taking half of the numer of CPUs available to the
process will be more flexible.
Old: a validator returns None on success and returns an error string
on error.
New: a validator returns the validated value on success and raises
ValidationError on error.
This allows mypy to catch mismatches between the annotated type of a
REQ parameter and the type that the validator actually validates.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This prevents memcached from automatically appending the hostname to
the username, which was a source of problems on servers where the
hostname was changed.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The zerver.models hack does not appear to be necessary now.
Meanwhile, get_wsgi_application has its own django.setup call, which
would overwrite the parts of our logging configuration pulled in by
zerver.models.
This fixes part of #15391; specifically, fixes it in production, but
not in development, where ‘manage.py runserver’ calls its own
django.setup and then imports various bits of our code before finding
zproject.wsgi.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Overrides some of internal functions of python-social-auth
to handle native flow.
Credits to Mateusz Mandera for the overridden functions.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Fixes#2665.
Regenerated by tabbott with `lint --fix` after a rebase and change in
parameters.
Note from tabbott: In a few cases, this converts technical debt in the
form of unsorted imports into different technical debt in the form of
our largest files having very long, ugly import sequences at the
start. I expect this change will increase pressure for us to split
those files, which isn't a bad thing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Automatically generated by the following script, based on the output
of lint with flake8-comma:
import re
import sys
last_filename = None
last_row = None
lines = []
for msg in sys.stdin:
m = re.match(
r"\x1b\[35mflake8 \|\x1b\[0m \x1b\[1;31m(.+):(\d+):(\d+): (\w+)", msg
)
if m:
filename, row_str, col_str, err = m.groups()
row, col = int(row_str), int(col_str)
if filename == last_filename:
assert last_row != row
else:
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
with open(filename) as f:
lines = f.readlines()
last_filename = filename
last_row = row
line = lines[row - 1]
if err in ["C812", "C815"]:
lines[row - 1] = line[: col - 1] + "," + line[col - 1 :]
elif err in ["C819"]:
assert line[col - 2] == ","
lines[row - 1] = line[: col - 2] + line[col - 1 :].lstrip(" ")
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This new endpoint returns a 'user' dictionary which, as of now,
contains a single key 'is_subscribed' with a boolean value that
represents whether the user with the given 'user_id' is subscribed
to the stream with the given 'stream_id'.
Fixes#14966.
Fixes this error in the dev environment:
$ ./manage.py checkconfig
Error: You must set ZULIP_ADMINISTRATOR in /etc/zulip/settings.py.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The logger defines a full list of handlers, meaning propagate=False is
needed, to avoid the log line propagating further up the logging tree
and getting logged multiple times by the duplicated handlers.
This implementation overrides some of PSA's internal backend
functions to handle `state` value with redis as the standard
way doesn't work because of apple sending required details
in the form of POST request.
Includes a mixin test class that'll be useful for testing
Native auth flow.
Thanks to Mateusz Mandera for the idea of using redis and
other important work on this.
Documentation rewritten by tabbott.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
This was written by Rishi for a very brief purpose a few years ago,
and it doesn't serve much purpose now other than to be a place we
update in code sweeps.
We're migrating to using the cleaner zulip.com domain, which involves
changing all of our links from ReadTheDocs and other places to point
to the cleaner URL.
This commit adds `name` attribute for the backends that do not
have them.
This is just a kind of prep commit in case if we want to use
`self.logger.xxxx()` in the future which is dependent on the
`name` attribute. But right now these logging calls aren't used
anywhere in those backends.
`HTTPError` has empty string for `str(HTTPError())`. Logging it
as it is would not be much helpful. So, this commits adds code
to log the name of error also.
Adds a top-level logger in `settings.LOGGING` `zulip.auth`
with the default handlers `DEFAULT_ZULIP_HANDLERS` and
an extra hanlder that writes to `/var/log/zulip/auth.log`.
Each auth backend uses it's own logger, `self.logger` which
is in form 'zulip.auth.<backend name>'.
This way it's clear which auth backend generated the log
and is easier to look for all authentication logs in one file.
Besides the above mentioned changes, `name` attribute is added to
`ZulipAuthMixin` so that these logging kind of calls wouldn't raise
any issues when logging is tried in a class without `name` attribute.
Also in the tests we use a new way to check if logger calls are made
i.e. we use `assertLogs` to test if something is logged.
Thanks to Mateusz Mandera for the idea of having a seperate logger
for auth backends and suggestion of using `assertLogs`.
Calling jwt.decode without an algorithms list raises a
DeprecationWarning. This is for protecting against
symmetric/asymmetric key confusion attacks.
This is a backwards-incompatible configuration change.
Fixes#15207.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Generated by pyupgrade --py36-plus --keep-percent-format, but with the
NamedTuple changes reverted (see commit
ba7906a3c6, #15132).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Anders Kaseorg
Alex Vandiver
Mateusz Mandera
sahil839
Clara Dantas
Hemanth V. Alluri
Dinesh
Tim Abbott
Mohit Gupta
Steve Howell
Vishnu KS
Rohitt Vashishtha
arpit551
Aman Agrawal
Tim April
Greg Price
Brainrecursion
wowol
Felix
Anders Kaseorg
Kartik Srivastava