Having wantMessagesSigned=True globally means that it's also applied by
python3-saml to regular authentication SAMLResponses - making it require
the response to be signed, which is an issue because a feasible
alternative way that some IdPs (e.g. AzureAD) take by default is to sign
specifically the assertions in the SAMLResponse. This is also secure,
and thus we generally want to accept it.
Without this, the setting of wantMessagesSigned=True globally
in 4105ccdb17 causes a
regression for deployments that have already set up SAML with providers
such as AzureAD, making Zulip stop accepting the SAMLResponses.
Testing that this new logic works is handled by
test_saml_idp_initiated_logout_invalid_signature, which verifies that a
LogoutRequest without signature will be rejected.
A confirmation link takes a user to the check_prereg_key_and_redirect
endpoint, before getting redirected to POST to /accounts/register/. The
problem was that validation was happening in the check_prereg_key_and_redirect
part and not in /accounts/register/ - meaning that one could submit an
expired confirmation key and be able to register.
We fix this by moving validation into /accouts/register/.
The warning was fixed in python-jose 3.3.0, which we pulled in with
commit 61e1e38a00 (#18705).
This reverts commit 1df725e6f1 (#18567).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We had an incident where someone didn't notice for a week that they'd
accidentally enabled a 30-day message retention policy, and thus we
were unable to restore the deleted the content.
After some review of what other products do (E.g. Dropbox preserves
things in a restoreable state for 30 days) we're adjusting this
setting's default value to be substantially longer, to give more time
for users to notice their mistake and correct it before data is
irrevocably deleted.
TOR users are legitimate users of the system; however, that system can
also be used for abuse -- specifically, by evading IP-based
rate-limiting.
For the purposes of IP-based rate-limiting, add a
RATE_LIMIT_TOR_TOGETHER flag, defaulting to false, which lumps all
requests from TOR exit nodes into the same bucket. This may allow a
TOR user to deny other TOR users access to the find-my-account and
new-realm endpoints, but this is a low cost for cutting off a
significant potential abuse vector.
If enabled, the list of TOR exit nodes is fetched from their public
endpoint once per hour, via a cron job, and cached on disk. Django
processes load this data from disk, and cache it in memcached.
Requests are spared from the burden of checking disk on failure via a
circuitbreaker, which trips of there are two failures in a row, and
only begins trying again after 10 minutes.
Using these tuples is clearly uglier than using classes for storing
these encoded stream. This can be built on further to implement the
various fiddly logic around handling these objects inside appropriate
class method.
This increases the possible maximum wait time to send exceptions
during shutdown. The larger value makes it possible to send larger
exceptions, and weather larger network hiccups, during shutdown. In
instances where a service is crash-looping, it is already not serving
requests reliably, and better ensuring those exceptions are captured
is of significant value.
Previously, our codebase contained links to various versions of the
Django docs, eg https://docs.djangoproject.com/en/1.8/ref/
request-response/#django.http.HttpRequest and https://
docs.djangoproject.com/en/2.2/ref/settings/#std:setting-SERVER_EMAIL
opening a link to a doc with an outdated Django version would show a
warning "This document is for an insecure version of Django that is no
longer supported. Please upgrade to a newer release!".
Most of these links are inside comments.
Following the replacement of these links in our docs, this commit uses
a search with the regex "docs.djangoproject.com/en/([0-9].[0-9]*)/"
and replaces all matches with "docs.djangoproject.com/en/3.2/".
All the new links in this commit have been generated by the above
replace and each link has then been manually checked to ensure that
(1) the page still exists and has not been moved to a new location
(and it has been found that no page has been moved like this), (2)
that the anchor that we're linking to has not been changed (and it has
been found that no anchor has been changed like this).
One comment where we mentioned a Django version in text before linking
to a page for that version has also been changed, the comment
mentioned the specific version when a change happened, and the history
is no longer relevant to us.
This more closely matches email_change_by_user and
password_reset_form_by_email limits; legitimate users are unlikely to
need to send more than 5 emails to themselves during a day.
Both `create_realm_by_ip` and `find_account_by_ip` send emails to
arbitrary email addresses, and as such can be used to spam users.
Lump their IP rate limits into the same bucket; most legitimate users
will likely not be using both of these endpoints at similar times.
The rate is set at 5 in 30 minutes, the more quickly-restrictive of
the two previous rates.
If realm is web_public, spectators can now view avatar of other
users.
There is a special exception we had to introduce in rest model to
allow `/avatar` type of urls for `anonymous` access, because they
don't have the /api/v1 prefix.
Fixes#19838.
As detailed in the comments, the default behavior is undesirable for us
because we can't really predict all possibilities of exceptions that may
be raised - and thus putting str(e) in the http response is potentially
insecure as it may leak some unexpected sensitive information that was
in the exception.
As a hypothetical example - KeyError resulting from some buggy
some_dict[secret_string] call would leak information. Though of course
we aim to never write code like that.
None of the existing custom profile field types have the value as an
integer like declared in many places - nor is it a string like currently
decalred in types.py. The correct type is Union[str, List[int]]. Rather
than tracking this in so many places throughout the codebase, we add a
new ProfileDataElementValue type and insert it where appropriate.
This new setting both serves as a guard to allow us to merge API
support for web public streams to main before we're ready for this
feature to be available on Zulip Cloud, and also long term will
protect self-hosted servers from accidentally enabling web-public
streams (which could be a scary possibility for the administrators of
a corporate Zulip server).
Fixes#17456.
The main tricky part has to do with what values the attribute should
have. LDAP defines a Boolean as
Boolean = "TRUE" / "FALSE"
so ideally we'd always see exactly those values. However,
although the issue is now marked as resolved, the discussion in
https://pagure.io/freeipa/issue/1259 shows how this may not always be
respected - meaning it makes sense for us to be more liberal in
interpreting these values.
This better matches the title of the page and more generally our
conventions around naming /help/ articles. We include a redirect
because this is referenced from Welcome Bot messages, and we
definitely don't want those links to break.
AuthnContextClassRef tells the IdP what forms of authentication the user
should use on the IdP's server for us to be okay with it. I don't think
there's a reason for us to enforce anything here and it should be up to
the IdP's configuration to handle authentication how it wants.
The default AuthnContextClassRef only allows PasswordProtectedTransport,
causing the IdP to e.g. reject authentication with Yubikey in AzureAD
SAML - which can be confusing for folks setting up SAML and is just not
necessary.
The previous commit introduced logging of attempts for username+password
backends. For completeness, we should log, in the same format,
successful attempts via social auth backends.
These details are useful to log. This only makes sense for some auth
backends, namely email and ldap backends, because other backends are
"external" in the sense that they happen at some external provider's
server (Google, SAML IdP etc.) so the failure also happens there and we
don't get useful information about what happened.
SOCIAL_AUTH_SUBDOMAIN was potentially very confusing when opened by a
user, as it had various Login/Signup buttons as if there was a realm on
it. Instead, we want to display a more informative page to the user
telling them they shouldn't even be there. If possible, we just redirect
them to the realm they most likely came from.
To make this possible, we have to exclude the subdomain from
ROOT_SUBDOMAIN_ALIASES - so that we can give it special behavior.
This utilizes the generic `BaseNotes` we added for multipurpose
patching. With this migration as an example, we can further support
more types of notes to replace the monkey-patching approach we have used
throughout the codebase for type safety.
Till now, we've been forking django-auth-ldap at
https://github.com/zulip/django-auth-ldap to put the
LDAPReverseEmailSearch feature in it, hoping to get it merged
upstream in https://github.com/django-auth-ldap/django-auth-ldap/pull/150
The efforts to get it merged have stalled for now however and we don't
want to be on the fork forever, so this commit puts the email search
feature as a clumsy workaround inside our codebase and switches to using
the latest upstream release instead of the fork.
This fixes error found with django-stubs and it is a part of #18777.
Note that there are various remaining errors that need to be fixed in
upstream or elsewhere in our codebase.
Closes#19287
This endpoint allows submitting multiple addresses so we need to "weigh"
the rate limit more heavily the more emails are submitted. Clearly e.g.
a request triggering emails to 2 addresses should weigh twice as much as
a request doing that for just 1 address.
These were added at some point in the past, but were not complete, and
it makes sense to document the current feature level as and when they
become available, since clients should not use the drafts endpoints on
older feature levels.
This removes a bunch of non-functional duplicate JavaScript, HTML, and
CSS that was interfering with maintenance on the functional originals,
because it was never clear how to update the duplicates or how to
check that you’d updated the duplicates correctly.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The code didn't account for existence of SOCIAL_AUTH_SUBDOMAIN. So the
redirects would happen to endpoints on the SOCIAL_AUTH_SUBDOMAIN, which
is incorrect. The redirects should happen to the realm from which the
user came.
These modern landing pages cover use cases previously not detailed on
our website. Technically, we had a /for/research page before, but it
wasn't finished or linked everywhere.
Removed "function-url-quotes" stylelint rule
since I need to use quotes in url to use an
svg as list bullet point. There are spacing issues
using it as an image. Also, using quotes in url
is actually the recommended way to do it otherwise
there could be issue with escaping.
There might be good reasons to have other external authentication
methods such as SAML configured, but none of them is available.
This happens, for example, when you have enabled SAML so that Zulip is
able to generate the metadata in XML format, but you haven't
configured an IdP yet. This commit makes sure that the phrase _OR_ is
only shown on the login/account page when there are actually other
authentication methods available. When they are just configured, but
not available yet, the page looks like as if no external
authentication methods are be configured.
We achieve this by deleting any_social_backend_enabled, which was very
similar to page_params.external_authentication_methods, which
correctly has one entry per configured SAML IdP.
This API change removes unnecessary complexity from a client that
wants to change a user's personal settings, and also saves developers
from needing to make decisions about what sort of setting something is
at the API level.
We preserve the old settings endpoints as mapping to the same function
as the new one for backwards-compatibility. We delete the
documentation for the old endpoints, though the documentation for the
merged /settings endpoint mentions how to use the old endpoints when
needed.
We migrate all backend tests to the new endpoints, except for
individual tests for each legacy endpoint to verify they still work.
Co-authored-by: sahil839 <sahilbatra839@gmail.com>
This fixes an issue where update-prod-static would crash with the following exception:
2021-07-19 03:59:24,601 upgrade-zulip-stage-2: Building static assets...
Traceback (most recent call last):
File "./tools/update-prod-static", line 27, in <module>
os.chdir(settings.DEPLOY_ROOT)
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/conf/__init__.py", line 82, in __getattr__
self._setup(name)
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/conf/__init__.py", line 69, in _setup
self._wrapped = Settings(settings_module)
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/conf/__init__.py", line 170, in __init__
mod = importlib.import_module(self.SETTINGS_MODULE)
File "/usr/lib/python3.6/importlib/__init__.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 994, in _gcd_import
File "<frozen importlib._bootstrap>", line 971, in _find_and_load
File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 678, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "./tools/../zproject/settings.py", line 20, in <module>
from .computed_settings import * # noqa: F401,F403 isort: skip
File "./tools/../zproject/computed_settings.py", line 1186, in <module>
from .sentry import setup_sentry
File "./tools/../zproject/sentry.py", line 12, in <module>
from zerver.lib.request import get_request_notes
File "./tools/../zerver/lib/request.py", line 28, in <module>
import zerver.lib.rate_limiter as rate_limiter
File "./tools/../zerver/lib/rate_limiter.py", line 14, in <module>
from zerver.models import UserProfile
File "./tools/../zerver/models.py", line 26, in <module>
from django.contrib.auth.models import AbstractBaseUser, PermissionsMixin, UserManager
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/contrib/auth/models.py", line 3, in <module>
from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/contrib/auth/base_user.py", line 48, in <module>
class AbstractBaseUser(models.Model):
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/db/models/base.py", line 108, in __new__
app_config = apps.get_containing_app_config(module)
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/apps/registry.py", line 253, in get_containing_app_config
self.check_apps_ready()
File "/home/zulip/deployments/2021-07-19-03-58-39/zulip-py3-venv/lib/python3.6/site-packages/django/apps/registry.py", line 136, in check_apps_ready
raise AppRegistryNotReady("Apps aren't loaded yet.")
django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.
Traceback (most recent call last):
File "/home/zulip/deployments/2021-07-19-03-58-39/scripts/lib/upgrade-zulip-stage-2", line 220, in <module>
subprocess.check_call(["./tools/update-prod-static"], preexec_fn=su_to_zulip)
File "/usr/lib/python3.6/subprocess.py", line 311, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['./tools/update-prod-static']' returned non-zero exit status 1.
This allows for greater flexibility in values for "environment," and
avoids having to have duplicate definitions of STAGING in
`zproject/config.py` and `zproject/default_settings.py` (due to import
order restrictions).
It does overload the "deploy type" concept somewhat.
Follow-up to #19185.
This concludes the HttpRequest migration to eliminate arbitrary
attributes (except private ones that are belong to django) attached
to the request object during runtime and migrated them to a
separate data structure dedicated for the purpose of adding
information (so called notes) to a HttpRequest.
We will no longer use the HttpRequest to store the rate limit data.
Using ZulipRequestNotes, we can access rate_limit and ratelimits_applied
with type hints support. We also save the process of initializing
ratelimits_applied by giving it a default value.
If the user is logged in, we'll stick to rate limiting by the
UserProfile. In case of requests without authentication, we'll apply the
same limits but to the IP address.
Running notify_server_error directly from the logging handler can lead
to database queries running in a random context. Among the many
potential problems that could cause, one actual problem is a
SynchronousOnlyOperation exception when running in an asyncio event
loop.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
* Move content on moving topics between streams to a dedicated
article. We advertise it as "move content" to hint that one can move
messages or split topics, and link to it.
* This deletes change-the-topic-of-a-message, because the same content
is already covered in rename-a-topic.
* This commit mostly just moves content between articles. Most of that
content was redundant with the first few paragraphs of the surviving
"rename a topic" article. The former "This is useful for" se ntence
was adapted to the remaining article.
* This commit also adds a redirect for the removed article, and
updates related links.
Fixes#17277.
The main limitation of this implementation is that the sync happens if
the user authing already exists. This means that a new user going
through the sign up flow will not have their custom fields synced upon
finishing it. The fields will get synced on their consecutive log in via
SAML in the future. This can be addressed in the future by moving the
syncing code further down the codepaths to login_or_register_remote_user
and plumbing the data through to the user creation process.
We detail that limitation in the documentation.
The old type in default_settings wasn't right - limit_to_subdomains is a
List[str]. We define a TypeDict for capturing the typing of the settings
dict more correctly and to allow future addition of configurable
attributes of other non-str types.
Rename poll_timeout to event_queue_longpoll_timeout_seconds
and change its value from 90000 ms to 90 sec. Expose its
value in register api response when realm data is fetched.
Bump API_FEATURE_LEVEL to 74.
This will offer users who are self-hosting to adjust
this value. Moreover, this will help to reduce the
overall time taken to test `test_markdown.py` (since
this can be now overridden with `override_settings`
Django decorator).
This is done as a prep commit for #18641.
This will be useful for deployments that want to just use the full name
provided by the IdP and thus skip the registration form. Also in
combination with disabling name changes in the organization, can force
users to just use that name without being able to change it.
This adds basic support for `postgresql.database_user` and
`postgresql.database_name` settings in `zulip.conf`; the defaults if
unspecified are left as `zulip`.
Co-authored-by: Adam Birds <adam.birds@adbwebdesigns.co.uk>
This makes it parallel with deliver_scheduled_messages, and clarifies
that it is not used for simply sending outgoing emails (e.g. the
`email_senders` queue).
This also renames the supervisor job to match.
Raising jsonableError in the authentication form was non-ideal because
it took the user to an ugly page with the returned json.
We also add logging of this rare occurence of the scenario being
handled here.
user_profile.check_password(password) in authenticate of
EmailAuthBackend can raise PasswordTooWeakError; this happens when the
user's password is weaker than the current required policies and needs
to be rehashed (E.g. because, as in Django 3.2, the minimum salt
entropy increased).
This is a very rare case, but still needs a good user-facing error
message. We raise a json error to handle this with a user-facing error
message.
See this comment by Mateusz Mandera for a detailed explanation
about this case along with a traceback it generates.
https://github.com/zulip/zulip/pull/15449#discussion_r448308614
The authenticate function of EmailAuthBackend had request param
type set Optional[HttpRequest] had `None` as default. This
function is never called without a request. So this changes it to
require an HttpRequest parameter.
It was made `Optional` in bc062e1c4d,
because this parameter was new in Django at the time.
We're safe to make it a required argument as everything worked well
before that recent commit and Mateusz Mandera and I checked if it gets
`None` anywhere and found only authenticate of non EmailAuthBackend
gets `None` in some places like `dev_direct_login`.
All the places in tests where this function got `None` as request
were fixed in previous commit.
Support for the timeouts, and tests for them, was added in
53a8b2ac87 -- though no code could have set them after 31597cf33e.
Add a 10-second default timeout. Observationally, p99 is just about
5s, with everything else being previously being destined to meet the
30s worker timeout; 10s provides a sizable buffer between them.
Fixes#17742.
Thumbor and tc-aws have been dragging their feet on Python 3 support
for years, and even the alphas and unofficial forks we’ve been running
don’t seem to be maintained anymore. Depending on these projects is
no longer viable for us.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Currently users that try to deploy Zulip through docker has errors
because LDAP group search configuration can't be automated.
Reverts a hunk of f5197518a9.
As discussed in the comment, this is a critical scalability
optimization for organizations with thousands of users.
With substantial comment updates by tabbott.
This is a straightforward upgrade in terms of changes needed.
Necessary changes were:
- Set `DEFAULT_AUTO_FIELD`
https://docs.djangoproject.com/en/3.2/releases/3.2/#customizing-type-of-auto-created-primary-keys
- `The default_app_config application configuration variable is deprecated, due
to the now automatic AppConfig discovery.`
https://docs.djangoproject.com/en/3.2/releases/3.2/#automatic-appconfig-discovery
To handle this one, we can remove default_app_config from
zerver/__init__.py because it satisfies what release notes describe in
https://docs.djangoproject.com/en/3.2/releases/3.2/#automatic-appconfig-discovery:
"Most pluggable applications define an AppConfig subclass in an apps.py
submodule. Many define a default_app_config variable pointing to this
class in their __init__.py. When the apps.py submodule exists and
defines a single AppConfig subclass, Django now uses that configuration
automatically, so you can remove default_app_config."
An important note is that rebuild-test-database needs to be run after
this upgrade in dev environment - if tests are run with test db that was
built on the previous version, they will fail due to a mysterious bug
(?), where changing attributes of a user and .save()ing after logging in
in the test via self.login_user, causes getting logged out - the next
requests via self.client_get etc. are unauthed for some reason,
unless self.login_user is called again. This behavior is no longer
exhibited upon rebuilding the test db - and I can't reproduce it in
production or dev db. So this can likely be reasonably dismissed as some
quirk of the test client system that won't be relevant in the future and
doesn't impact production.
The function get_role_for_new_user was added to get role from the
invited_as value, as invited_as values were one of (1,2,3,4)
previously, but it was then changed to be the actual role value,
i.e. one of (100, 200, 400, 600), in 1f8f227444.
So, we can safely remove this function now and use invited_as value
directly and handle realm_creation case by using an if condition.
Commit 9afde790c6 introduced a bug
concerning outgoing emails inside the development environment. These
emails are not supposed to use a real connection with a mail
server as the send_messages function is overwritten inside the
EmailLogBackEnd class.
The bug was happening inside the initialize_connection function that
was introduced in the above-mentioned commit. This function is used
to refresh the connection with an SMTP server that would have closed
it. As the socket used to communicate with the server is not
initialized inside the development environment this function was
wrongly trying to send no-op commands.
The fix just checks that the connection argument of the function is
an EmailLogBackEnd object before trying the no-op command.
Additionally as it is sometimes useful to be able to send outgoing
emails inside the development environment the get_forward_address
function is used to check if a real connection exists between Zulip
and the server. If it is the case, as EmailLogBackEnd is a subclass
of smtp.EmailBackend, the connection will be nicely refreshed.
This commit was tested manually by checking that the console prints
correctly that an email is sent to the user when it signs in inside
the development environment. It was also tested when a mail provider
is specified and the mails were correctly received.
This allows access to be more configurable than just setting one
attribute. This can be configured by setting the setting
AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL.
This commit adds an API to `zproject/urls.py` to edit/update
the realm linkifier. Its helper function to update the
database is added in `zerver/lib/actions.py`.
`zulip.yaml` is documented accordingly as well, clearly
stating that this API updates one linkifier at a time.
The tests are added for the API and helper function which
updates the realm linkifier.
Fixes#10830.
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
* This introduces a new event type `realm_linkifiers` and
a new key for the initial data fetch of the same name.
Newer clients will be expected to use these.
* Backwards compatibility is ensured by changing neither
the current event nor the /register key. The data which
these hold is the same as before, but internally, it is
generated by processing the `realm_linkifiers` data.
We send both the old and the new event types to clients
whenever the linkifiers are changed.
Older clients will simply ignore the new event type, and
vice versa.
* The `realm/filters:GET` endpoint (which returns tuples)
is currently used by none of the official Zulip clients.
This commit replaces it with `realm/linkifiers:GET` which
returns data in the new dictionary format.
TODO: Update the `get_realm_filters` method in the API
bindings, to hit this new URL instead of the old one.
* This also updates the webapp frontend to use the newer
events and keys.
This was used by the old native Zulip Android app
(zulip/zulip-android). That app has been undeveloped for enough years
that we believe it no longer functions; as a result, there's no reason
to keep a prototype API endpoint for it (that we believe never worked).
This endpoint was needed by the ancient pre-electron desktop app
written in QT; we removed support for that in practice a long time
ago, and even the custom error messages for it in
5a22e73cc6.
So we can delete this endpoint as well.
Similar to the previous commit, we have added a `do_*` function
which does the deletion from the DB. The next commit handles sending
the events when both adding and deleting a playground entry.
Added the openAPI format data to zulip.yaml for DELETE
/realm/playgrounds/{playground_id}. Also added python and curl
examples to remove-playground.md.
Tests added.
This endpoint will allow clients to create a playground entry
containing the name, pygments language and url_prefix for the
playground of their choice.
Introduced the `do_*` function in-charge of creating the entry in
the model. Handling the process of sending events which will be
done in a follow up commit.
Added the openAPI format data to zulip.yaml for POST
/realm/playgrounds. Also added python and curl examples for using
the endpoint in its markdown documented (add-playground.md).
Tests added.
Adds backend code for the mute users feature.
This is just infrastructure work (database
interactions, helpers, tests, events, API docs
etc) and does not involve any behavioral/semantic
aspects of muted users.
Adds POST and DELETE endpoints, to keep the
URL scheme mostly consistent in terms of `users/me`.
TODOs:
1. Add tests for exporting `zulip_muteduser` database table.
2. Add dedicated methods to python-zulip-api to be used
in place of the current `client.call_endpoint` implementation.
Because the logic in print_listeners doesn't have access to computed
settings in dev_settings.py, we need to duplicate the special
IS_DEV_DROPLET logic for computing the default hostname.
There's still a secondary problem that this URL 404s.
It does not seem like an official version supporting Webpack 4 (to say
nothing of 5) will be released any time soon, and we can reimplement
it in very little code.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This is a prep change to eventually completely
replace the term "filter" with "linkifier" in
the codebase.
This only renames files. Code changes will be
done in further commits.
We use GIPHY web SDK to create popover containing GIFs in a
grid format. Simply clicking on the GIFs will insert the GIF in the compose
box.
We add GIPHY logo to compose box action icons which opens the GIPHY
picker popover containing GIFs with "Powered by GIPHY"
attribution.
We're renaming "stream deletion" language to "stream archiving"
and these pages were moved in the process, so we should keep redirects
for them for a while.
The `X-Forwarded-For` header is a list of proxies' IP addresses; each
proxy appends the remote address of the host it received its request
from to the list, as it passes the request down. A naïve parsing, as
SetRemoteAddrFromForwardedFor did, would thus interpret the first
address in the list as the client's IP.
However, clients can pass in arbitrary `X-Forwarded-For` headers,
which would allow them to spoof their IP address. `nginx`'s behavior
is to treat the addresses as untrusted unless they match an allowlist
of known proxies. By setting `real_ip_recursive on`, it also allows
this behavior to be applied repeatedly, moving from right to left down
the `X-Forwarded-For` list, stopping at the right-most that is
untrusted.
Rather than re-implement this logic in Django, pass the first
untrusted value that `nginx` computer down into Django via `X-Real-Ip`
header. This allows consistent IP addresses in logs between `nginx`
and Django.
Proxied calls into Tornado (which don't use UWSGI) already passed this
header, as Tornado logging respects it.
We add a TUTORIAL_ENABLED setting for self-hosters who want to
disable the tutorial entirely on their system. For this, the
default value (True) is placed in default_settings.py, which
can be overwritten by adding an entry in /etc/zulip/settings.py.
This adds the is_user_active with the appropriate code for setting the
value correctly in the future. In the following commit a migration to
backfill the value for existing Subscriptions will be added.
To ensure correct user_profile.is_active handling also in tests, we
replace all direct .is_active mutation with calls to appropriate
functions.
For pages that don't have page_params, the default_page_params now
ensures that debug_mode will correctly follow settings.DEBUG.
This allows blueslip exception popups to work on portico pages for
development environment.
Fixes: #17540.
This adds an option for restricting a ldap user
to only be allowed to login into certain realms.
This is done by configuring an attribute mapping of "org_membership"
to an ldap attribute that will contain the list of subdomains the ldap
user is allowed to access. This is analogous to how it's done in SAML.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
This is part of our general process of replacing emails, which are not
static with time, with user_ids when referring to users in the API.
We still keep the `email` reference option, since it can be useful for
linking third-party applications to Zulip on an intranet that might
have a user's corporate email handy and not want to do the extra round
trip to lookup the user.
The name of the parameter, user_id_or_email, was chosen to to make it
clear that the default/preferred option is user_id.
Fixes#14304.
Add new rest api endpoint GET users/{email} for looking up a user by
email, which is useful especially for corporate API applications that
might already have a user's email address.
Fixes#14302.
This finds three sets of related settings to extract from the
"Miscellaneous" settings section:
- Service configuration (PostgreSQL, RabbitMQ, Redis, Memcached)
- Previews (image, URL, and Twitter)
- Logging and error reporting
Match the order of the variables between `default_settings.py` and
`settings.py`, and move the defaults into `default_settings.py` so
the section does not require any uncommented lines in `settings.py` if
LDAP is not in use.
Merge the two "misc" sections into one and place all the service
configurations next to each other. Place the TERMS_OF_SERVICE
and PRIVACY_POLICY at the bottom of the "misc" section.
EmailLogBackend used to create a new EmailMessage and copy
only certain values from the original EmailMultiAlternatives
object. This resulted in the loss of information and made
it harder to test PRs like
https://github.com/zulip/zulip/pull/17121.
So instead of creating a new EmailMessage, tweak and send the existing
EmailMultiAlternatives object.
https://docs.djangoproject.com/en/3.1/releases/3.1/
- django.contrib.postgres.fields.JSONField is deprecated and should be
replaced with models.JSONField
- The internals of the implementation in the postgresql backend have
changed a bit in
f48f671223
and thus we need to make an ugly tweak in test_runner.
- app_directories.Loader.get_dirs() now returns a list of PosixPath so
we need to make a small tweak in TwoFactorLoader for that (PosixPath
is not iterable)
Fixes#16010.
As of Feb 15th 2019, Hipchat Cloud and Stride
have reached End Of Life and are no longer
supported by Atlassian. Since it is almost 2 years
now we can remove the migration guides.
Boto3 does not allow setting the endpoint url from
the config file. Thus we create a django setting
variable (`S3_ENDPOINT_URL`) which is passed to
service clients and resources of `boto3.Session`.
We also update the uploads-backend documentation
and remove the config environment variable as now
AWS supports the SIGv4 signature format by default.
And the region name is passed as a parameter instead
of creating a config file for just this value.
Fixes#16246.
The name used to be included in the id_token, but this seems to have
been changed by Apple and now it's sent in the `user` request param.
https://github.com/python-social-auth/social-core/pull/483 is the
upstream PR for this - but upstream is currently unmaintained, so we
have to monkey patch.
We also alter the tests to reflect this situation. Tests no longer put
the name in the id_token, but rather in the `user` request param in the
browser flow, just like it happens in reality.
An adaptation has to be made in the native flow - since the name won't
be included by Apple in the id_token anymore, the app, when POSTing
to the /complete/apple/ endpoint,
can (and should for better user experience)
add the `user` param formatted as json of
{"email": "hamlet@zulip.com", "name": {"firstName": "Full", "lastName": "Name"}}
dict. This is also reflected by the change in the
native flow tests.
We previously used to to redirect to config error page with
a different URL. This commit renders config error in the same
URL where configuration error is encountered. This way when
conifguration error is fixed the user can refresh to continue
normally or go back to login page from the link provided to
choose any other backend auth.
Also moved those URLs to dev_urls.py so that they can be easily
accessed to work on styling etc.
In tests, removed some of the asserts checking status code to be 200
as the function `assert_in_success_response` does that check.
Having both of these is confusing; TORNADO_SERVER is used only when
there is one TORNADO_PORT. Its primary use is actually to be _unset_,
and signal that in-process handling is to be done.
Rename to USING_TORNADO, to parallel the existing USING_RABBITMQ, and
switch the places that used it for its contents to using
TORNADO_PORTS.
We can compute the intended number of processes from the sharding
configuration. In doing so, also validate that all of the ports are
contiguous.
This removes a discrepancy between `scripts/lib/sharding.py` and other
parts of the codebase about if merely having a `[tornado_sharding]`
section is sufficient to enable sharding. Having behaviour which
changes merely based on if an empty section exists is surprising.
This does require that a (presumably empty) `9800` configuration line
exist, but making that default explicit is useful.
After this commit, configuring sharding can be done by adding to
`zulip.conf`:
```
[tornado_sharding]
9800 = # default
9801 = other_realm
```
Followed by running `./scripts/refresh-sharding-and-restart`.
In development and test, we keep the Tornado port at 9993 and 9983,
respectively; this allows tests to run while a dev instance is
running.
In production, moving to port 9800 consistently removes an odd edge
case, when just one worker is on an entirely different port than if
two workers are used.
This was called in both if and else with the same argument.
I believe there's no reason for it to exist twice and having
it just once would be a bit cleaner.
Calling `render()` in a middleware before LocaleMiddleware has run
will pick up the most-recently-set locale. This may be from the
_previous_ request, since the current language is thread-local. This
results in the "Organization does not exist" page occasionally being
in not-English, depending on the preferences of the request which that
thread just finished serving.
Move HostDomainMiddleware below LocaleMiddleware; none of the earlier
middlewares call `render()`, so are safe. This will also allow the
"Organization does not exist" page to be localized based on the user's
browser preferences.
Unfortunately, it also means that the default LocaleMiddleware catches
the 404 from the HostDomainMiddlware and helpfully tries to check if
the failure is because the URL lacks a language component (e.g.
`/en/`) by turning it into a 304 to that new URL. We must subclass
the default LocaleMiddleware to remove this unwanted functionality.
Doing so exposes a two places in tests that relied (directly or
indirectly) upon the redirection: '/confirmation_key'
was redirected to '/en/confirmation_key', since the non-i18n version
did not exist; and requests to `/stats/realm/not_existing_realm/`
incorrectly were expecting a 302, not a 404.
This regression likely came in during f00ff1ef62, since prior to
that, the HostDomainMiddleware ran _after_ the rest of the request had
completed.
Its functionality was added to Django upstream in 2.1. Also remove
the SESSION_COOKIE_SAMESITE = 'Lax' setting since it’s the default.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This undoes a small part of b8a2e6b5f8; namely, logs to
`zulip.zerver.webhooks`, which are all exceptions from webhooks except
UnsupportedWebhookEventType, should still be logged to the main error
loggers. This maintains the property that exceptions generating 500's
are all present in `errors.log`.
Django treats path("<name>") like re_path(r"(?P<name>[^/]+)") and
path("<path:name>") like re_path(r"(?P<name>.+)").
This is more readable and consistent than the mix of slightly
different regexes we had before, and fixes various bugs:
• The r'apps/(.*)$' regex was missing a start anchor ^, so it
incorrectly matched all URLs that included apps/ as a substring
anywhere.
• The r'accounts/login/(google)/$' regex was missing a start anchor ^,
so it incorrectly matched all URLs that ended with
accounts/login/google/.
• The type annotation of zerver.views.realm_export.delete_realm_export
takes export_id as an int, but it was previously passed as a string.
• The type annotation of zerver.views.users.avatar takes medium as a
bool, but it was previously passed as a string.
• The [0-9A-Za-z]+ pattern for uidb64 was missing the - and _
characters that can validly be part of a base64url encoded
string (although I think the id is actually a decimal integer here,
in which case only 012345ADEIMNOQTUYcgjkwxyz are present in its
base64url encoding).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Replace default root logger with zulip.auth.apple for apple auth
in file zproject/backends.py and update the test cases
accordingly in file zerver/tests/test_auth_backends.py
This clears it out of the data sent to Sentry, where it is duplicative
with the indexed metadata -- and potentially exposes PHI if Sentry's
"make this issue public" feature is used.
`zulip.zerver.lib.webhooks.common` was very opaque previously,
especially since none of the logging was actually done from that
module.
Adjust to a more explicit logger name.
Any exception is an "unexpected event", which means talking about
having an "unexpected event logger" or "unexpected event exception" is
confusing. As the error message in `exceptions.py` already explains,
this is about an _unsupported_ event type.
This also switches the path that these exceptions are written to,
accordingly.
For web-public streams, clients can access full topic history
without being authenticated. They only need to additionally
send "streams:web-public" narrow with their request like all
the other web-public queries.
By default, the Django Sentry integration provides the email address
and username of pulled from the auth layer. This is potentially PII,
and not data that we wish to store. Enable sending user data at all,
by setting `send_default_pii=True`, but strip the username and
email (which are the same, in Zulip) before sending. Users will be
identified in Sentry only by their IP address, user ID, realm, and
role.
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.
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
This commit adds automatic detection of extra output (other than
printed by testing library or tools) in stderr and stdout by code under
test test-backend when it is run with flag --ban-console-output.
It also prints the test that produced the extra console output.
Fixes: #1587.
Extracting a section for presence endpoints and using path() rather
than re_path() results in a much cleaner implementation of this
concept.
This eliminates the last case where test_openapi couldn't correctly
match an endpoint documentation with the OpenAPI definitions for it.
Via API, users can now access messages which are in web-public
streams without any authentication.
If the user is not authenticated, we assume it is a web-public
query and add `streams:web-public` narrow if not already present
to the narrow. web-public streams are also directly accessible.
Any malformed narrow which is not allowed in a web-public query
results in a 400 or 401. See test_message_fetch for the allowed
queries.
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
Tim Abbott
Mateusz Mandera
Eeshan Garg
Anders Kaseorg
Alya Abbott
Sahil Batra
Tim Abbott
Alex Vandiver
YashRE42
Aman Agrawal
rht
Mateusz Mandera
PIG208
Hemanth V. Alluri
sahil839
Erik Tews
Damian Parrino
Suyash Vardhan Mathur
Gaurav Pandey
akshatdalton
Vishnu KS
Dinesh
Flávio Prado
pletinckxc
strifel
Nyior
Riken Shah
Abhijeet Prasad Bodas
Sumanth V Rao
shanukun
Strifel
Tushar912
Nikhil Maske
Sourabh Rana
ryanreh99
palash
Mohit Gupta
savish