nginx sets the value of the `$http_host` variable to the empty string
when using http/3, as there is technically no `Host:` header sent:
https://github.com/nginx-quic/nginx-quic/issues/3
Users with a browser that support http/3 will send their first request
to nginx with http/2, and get an expected HTTP 200 -- but any
subsequent requests will fail with am HTTP 400, since the browser will
have upgraded to http/3, which has an empty `Host` header, which Zulip
rejects.
Switch to the `$host` variable, which works for all HTTP versions.
Co-authored-by: Alex Vandiver <alexmv@zulip.com>
This commit encompasses the following changes:
* Replace the [More...] link with a button titled "Show more".
* Replace the [Show Less...] link with a button titled "Show less".
* Add various on-hover interactions to the buttons.
* In the condensed view, add fading to the bottom of the message to
visually communicate that the message is truncated.
* Update /help/ description.
Fixes#22801.
Co-authored-by: Evy Kassirer <evy.kassirer@gmail.com>
Restore the default django.utils.log.AdminEmailHandler when
ERROR_REPORTING is enabled. Those with more sophisticated needs can
turn it off and use Sentry or a Sentry-compatible system.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Uploads are well-positioned to use S3's "intelligent tiering" storage
class. Add a setting to let uploaded files to declare their desired
storage class at upload time, and document how to move existing files
to the same storage class.
It’s unclear what was supposed to be “safe” about this wrapper. The
hashlib API is fine without it, and we don’t want to encourage further
use of SHA-1.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This includes changing the URL to #settings/preferences, with a
transparent redirect so that existing links, like the one from Welcome
Bot, continue to work.
Users can, quite understandably, assume that upgrading Zulip upgraded
the underlying PostgreSQL version. Though it is mentioned at the top
of the page, mentioning it here clarifies that it is an additional
step.
- Updates instances of "private message", "PM", and "private_message",
excluding historical references in `overview/changelog.md`.
- Also excludes `/docs/translating` since we would need new
translations for "direct messages" and "DMs".
Previously, `X-Forwarded-Proto` did not need to be set, and failure to
set `loadbalancer.ips` would merely result in bad IP-address
rate-limiting and incorrect access logs; after 0935d388f0, however,
failure to do either of those, if Zulip is deployed with `http_only`,
will lead to infinite redirect loops after login. These are
accompanied by a misleading error, from Tornado, of:
Forbidden (Origin checking failed - https://zulip.example.com does not match any trusted origins.): /json/events
This is most common with Docker deployments, where deployments use
another docker container, such as nginx or Traefik, to do SSL
termination. See zulip/docker-zulip#403.
Update the documentation to reinforce that `loadbalancer.ips` also
controls trust of `X-Forwarded-Proto`, and that failure to set it will
cause the application to not function correctly.
- Create a dedicated "Reporting bugs" page to learly document
where and how bugs should be reported.
- Drop "Reporting issues" section from the Contributing guide.
- Delete "Bug report guidelines" page.
04cf68b45e make nginx responsible for downloading (and caching)
files from S3. As noted in that commit, nginx implements its own
non-blocking DNS resolver, since the base syscall is blocking, so
requires an explicit nameserver configuration. That commit used
127.0.0.53, which is provided by systemd-resolved, as the resolver.
However, that service may not always be enabled and running, and may
in fact not even be installed (e.g. on Docker). Switch to parsing
`/etc/resolv.conf` and using the first-provided nameserver. In many
deployments, this will still be `127.0.0.53`, but for others it will
provide a working DNS server which is external to the host.
In the event that a server is misconfigured and has no resolvers in
`/etc/resolv.conf`, it will error out:
```console
Error: Evaluation Error: Error while evaluating a Function Call, No nameservers found in /etc/resolv.conf! Configure one by setting application_server.nameserver in /etc/zulip/zulip.conf (file: /home/zulip/deployments/current/puppet/zulip/manifests/app_frontend_base.pp, line: 76, column: 70) on node example.zulipdev.org
```
This better presents the CSS organization for readers, and also
removes a stale reference and link to Bootstrap.
Because postcss-nesting's spec-aligned syntax has tripped up some
contributors, the mention of PostCSS now includes a link to the
postcss-nesting README and the CSS Nesting spec from the W3C, which
PostCSS Nesting attempts to adhere to.
This commit represents an in-place reordering of the document. No
headings or content has been changed (that will happen in subsequent
commits).
The goal is to open the document with generic advice and guidance
applicable to all Zulip developers across all languages:
1. Consistency, enforced by linters and automated tests, opens the
document.
2. General, largely language-neutral advice about line length,
third-party code, translation, paths, and secrets come next.
3. Next up is language-specific advice and conventions: Python,
followed by JavaScript and TypeScript, followed by HTML and CSS
(although the HTML and CSS will be moved in a subsequent commit
to their own file).
4. Closing the file, rather than opening it, is the section on
Dangerous constructs. Some of these are fairly specialized, so
it makes sense not to ask readers to read through them before
presenting, say, our philosophy on line length.
Finally, in trying to come up with a sensible order for all sections
of this document, the "More arbitrary style things" heading has been
removed.
This is a useful improvement in general for making correct
LogoutRequests to Idps and a necessary one to make SP-initiated logout
fully work properly in the desktop application. During desktop auth
flow, the user goes through the browser, where they log in through their
IdP. This gives them a logged in browser session at the IdP. However,
SAML SP-initiated logout is fully conducted within the desktop
application. This means that proper information needs to be given to the
the IdP in the LogoutRequest to let it associate the LogoutRequest with
that logged in session that was established in the browser. SessionIndex
is exactly the tool for that in the SAML spec.
This gives more flexibility on a server with multiple organizations and
SAML IdPs. Such a server can have some organizations handled by IdPs
with SLO set up, and some without it set up. In such a scenario, having
a generic True/False server-wide setting is insufficient and instead
being able to specify the IdPs/orgs for SLO is needed.
Closes#20084
This is the flow that this implements:
1. A logged-in user clicks "Logout".
2. If they didn't auth via SAML, just do normal logout. Otherwise:
3. Form a LogoutRequest and redirect the user to
https://idp.example.com/slo-endpoint?SAMLRequest=<LogoutRequest here>
4. The IdP validates the LogoutRequest, terminates its own user session
and redirects the user to
https://thezuliporg.example.com/complete/saml/?SAMLRequest=<LogoutResponse>
with the appropriate LogoutResponse. In case of failure, the
LogoutResponse is expected to express that.
5. Zulip validates the LogoutResponse and if the response is a success
response, it executes the regular Zulip logout and the full flow is
finished.
Django 4.0 and higher began checking the `Origin` header, which made
it important that Zulip know accurately if the request came over HTTPS
or HTTP; failure to do so would result in "CSRF verification failed"
errors.
For Zulip servers which are accessed via proxies, this means that
`X-Fowarded-Proto` must be set accurately. Adjust the documentation
for the suggested configurations to add the header.
Fixes: #24599.
Co-authored-by: Alex Vandiver <alexmv@zulip.com>
1c76036c61 raised the number of `minfds` in Supervisor from 40k to
1M. If Supervisor cannot guarantee that number of available file
descriptors, it will fail to start; `/etc/security/limits.conf` was
hence adjusted upwards as well. However, on some virtualized
environments, including Proxmox LXC, setting
`/etc/security/limits.conf` may not be enough to raise the
system-level limits. This causes `supervisord` with the larger
`minfds` to fail to start.
The limit of 1000000 was chosen to be arbitrarily high, assuming it
came without cost; it is not expected to ever be reached on any
deployment. 262b19346e already lowered one aspect of that
changeset, upon determining it did come with a cost. Potentially
breaking virtualized deployments during upgrade is another cost of
that change.
Lower the `minfds` it back down to 40k, partially reverting
1c76036c61, but allow adjusting it upwards for extremely large
deployments. We do not expect any except the largest deployments to
ever hit the 40k limit, and a frictionless deployment for the
vanishingly small number of huge deployments is not worth the
potential upgrade hiccups for the much more frequent smaller
deployments.
Instead of copying over a mostly-unchanged `postgresql.conf`, we
transition to deploying a `conf.d/zulip.conf` which contains the
only material changes we made to the file, which were previously
appended to the end.
While shipping separate while `postgresql.conf` files for each
supported version is useful if there is large variety in supported
options between versions, there is not no such variation at current,
and the burden of overriding the entire default configuration is that
it must be keep up to date wit the package's version.
This commit updates the recommended commit summary length
to 72 characters in the documentation. If the commit message
exceeds this length than GitHub cutoffs the remaining characters.
This should help miscueing users into thinking that the provisioning
steps, etc. are a part of the python3 installation--which is now more
explicitly aimed at Centos, Fedora, and RHEL users.
In #23380 we want to change all ocurrences of `uri` to `url`. This
commit changes the ocurrences of `uri` appeared in files related to
email, including templates (`.html`, `.txt`) and backend (`.py`)
codes.
In `email.md`, `base_images_uri` is changed to `images_base_url` -
the words `base` and `images` are swapped and plural form is added
for `image`. This is becasue the former is not found anywhere in
the codebase while the later appears a lot. To reduce confusion,
this doccumentation changed accordingly.
Taking backups on the database primary adds additional disk load,
which can impact the performance of the application.
Switch to taking backups on replicas, if they exist. Some deployments
may have multiple replicas, and taking backups on all of them is
wasteful and potentially confusing; add a flag to inhibit taking
nightly snapshots on the host.
If the deployment is a single instance of PostgreSQL, with no
replicas, it takes backups as before, modulo the extra flag to allow
skipping taking them.
Added unmute.svg in web/shared/icons. Also added
source and license information for the icon in
docs/THIRDPARTY.
Additionally, used unmute icon for unmute option in
topic_sidebar menu for topics in muted stream.
Fixes#25124
This commit make changes in create stream panel by moving
announce stream option below stream description and adds a
"Stream permissions" section heading just above
"Who can access the stream?" and also rewords the text
"Announce stream".
Also now announce stream option is only shown if the user creating
stream have access to the announcement stream name. When option
is not shown, default behaviour is to announce public and
web-public streams.
Fixes#23327.
This removes the production reporting to `/json/report/error` upon
`blueslip.error`, and replaces it with reporting to Sentry, if
enabled. Sentry provides better reporting and grouping for exceptions
than the email- and `#errors`-reporting provided by the
`/json/report/error` endpoint.
The development behaviour of rendering `blueslip.error` messages and
stacktraces immediately, and stopping execution, is preserved.
To better chain exception information, the whole previous exception is
passed to `blueslip.error`, not just the stack, and the second
parameter is formalized to be an object to map to Sentry's "context"
concept.
We currently use the GitHub logo to show the bots settings in both
personal and organization settings. This icon is not a correct
representation since we use have support for several different bots
including but not limited to a GitHub integration bot.
This commit replaces the GitHub logo with Smart Toy material icon
from Google, while also adding the required licencing details at
docs/THIRDPARTY.
Part of #19797.
Since logrotate runs in a daily cron, this practically means "daily,
but only if it's larger than 500M." For large installs with large
traffic, this is effectively daily for 10 days; for small installs, it
is an unknown amount of time.
Switch to daily logfiles, defaulting to 14 days to match nginx; this
can be overridden using a zulip.conf setting. This makes it easier to
ensure that access logs are only kept for a bounded period of time.
Previously, we had an architecture where CSS inlining for emails was
done at provision time in inline_email_css.py. This was necessary
because the library we were using for this, Premailer, was extremely
slow, and doing the inlining for every outgoing email would have been
prohibitively expensive.
Now that we've migrated to a more modern library that inlines the
small amount of CSS we have into emails nearly instantly, we are able
to remove the complex architecture built to work around Premailer
being slow and just do the CSS inlining as the final step in sending
each individual email.
This has several significant benefits:
* Removes a fiddly provisioning step that made the edit/refresh cycle
for modifying email templates confusing; there's no longer a CSS
inlining step that, if you forget to do it, results in your testing a
stale variant of the email templates.
* Fixes internationalization problems related to translators working
with pre-CSS-inlined emails, and then Django trying to apply the
translators to the post-CSS-inlined version.
* Makes the send_custom_email pipeline simpler and easier to improve.
Signed-off-by: Daniil Fadeev <fadeevd@zulip.com>
Primary goal of library replacement is improving execution speed.
This commit should not affect the functionality of the system
or make any changes to it.
This adds a new search icon which we prefer over the one made
available from bootstrap, and replaces search icons in navbar
search with the Ionic icon.
Increasing worker_connections has a memory cost, unlike the rest of
the changes in 1c76036c61d8; setting it to 1 million caused nginx to
consume several GB of memory.
Reduce the default down to 10k, and allow deploys to configure it up
if necessary. `worker_rlimit_nofile` is left at 1M, since it has no
impact on memory consumption.
Updates this section to use the shared `api-doc-template.md` as a
guide, as well as the current documentation for the `render-message`
endpoint. Previously, this section referred to a file that had been
removed with the transition to a shared template file.
Fixes#24485.
Adds a point in the "Tips and best practices" section of the
article on writing reviewable pull requests about cross-linking
between GitHub pull requests and conversations in the Zulip
development community.
Fixes a few typos / errors in commit a633890d8 that updated
these documentation articles on pull requests.
Also adjusts language around screenshots to match what we use
in the pull request template on GitHub for the zulip server and
web app repository.
Zulip Server 4.0 is now 22 months old, which is more than 18 months.
Per the general policy in the "Client apps" section below, that
means it's time to drop support for older versions.
We released 5.0 near the end of 2022-03, so near the end of 2023-09
we can update this further to say 5.0.
Since we are migrating from JavaScript to TypeScript
some files in the documentation are still with their
old extension. This commit changes those file extensions.
Adds a 'Tips and best practices' section to the documentation on
writing reviewable pull requests.
Also, updates step 3 of the documentation on creating pull requests
to link to the new section and to not have an out-of-date screenshot
of the GitHub pull request template.
Because `yarn.lock` includes transitive dependencies, it already pins
our dependencies more comprehensively than `package.json` would if we
followed this bad advice, which we don’t, as of commit
9b0401b76d (#13118).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This is the behaviour inherited from Django[^1]. While setting the
password to empty (`email_password = `) in
`/etc/zulip/zulip-secrets.conf` also would suffice, it's unclear what
the user would have been putting into `EMAIL_HOST_USER` in that
context.
Because we previously did not warn when `email_password` was not
present in `zulip-secrets.conf`, having the error message clarify the
correct configuration for disabling SMTP auth is important.
Fixes: #23938.
[^1]: https://docs.djangoproject.com/en/4.1/ref/settings/#std-setting-EMAIL_HOST_USER
The documentation for restoring backups referenced that it needed to
be to the same version of PostgreSQL, but did not explain how to do
that.
Link to the relevant section of the installer documentation, and name
the flag explicitly.
Fixes: #23691
This lets us simplify the long-ish ‘../../static/js’ paths, and will
remove the need for the ‘zrequire’ wrapper.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Ever since we started bundling the app with webpack, there’s been less
and less overlap between our ‘static’ directory (files belonging to
the frontend app) and Django’s interpretation of the ‘static’
directory (files served directly to the web).
Split the app out to its own ‘web’ directory outside of ‘static’, and
remove all the custom collectstatic --ignore rules. This makes it
much clearer what’s actually being served to the web, and what’s being
bundled by webpack. It also shrinks the release tarball by 3%.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These hooks are run immediately around the critical section of the
upgrade. If the upgrade fails for preparatory reasons, the pre-deploy
hook may not be run; if it fails during the upgrade, the post-deploy
hook will not be run. Hooks are called from the CWD of the new
deploy, with arguments of the old version and the new version. If
they exit with non-0 exit code, the deploy aborts.
Corepack manages multiple per-project version of Yarn and PNPM, which
means we have to maintain less installation code, and could help us
switch away from Yarn 1 without making the system unusable for
development of other Yarn 1 projects.
https://nodejs.org/api/corepack.html
The Unicode spaces in the timerender test resulted from an ICU
upgrade: https://github.com/nodejs/node/pull/45068.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We will hopefully be able to just this in #16208 to document what
users need to configure in order to do this manually, but the content
here will be useful for anyone who hasn't set that up regardless.
This overrides the settings in the web interface, and enables much
more flexible configuration of the build environment (which previously
defaulted to Python 3.7).
https://docs.readthedocs.io/en/stable/config-file/v2.html
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This adds a new endpoint /jwt/fetch_api_key that accepts a JWT and can
be used to fetch API keys for a certain user. The target realm is
inferred from the request and the user email is part of the JWT.
A JSON containing an user API key, delivery email and (optionally)
raw user profile data is returned in response.
The profile data in the response is optional and can be retrieved by
setting the POST param "include_profile" to "true" (default=false).
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
- Updates `.prettierignore` for the new directory.
- Updates any reference to the API documentation directory for
markdown files to be `api_docs/` instead of `zerver/api/`.
- Removes a reference link from `docs/documentation/api.md` that
hasn't referenced anything in the text since commit 0542c60.
- Update rendering of API documentation for new directory.
- Renames "Customize Zulip" to "Server configuration".
- Cross-links "Server configuration" with "System and deployment
configuration".
Fixes part of #23984.
The `postfix.mailname` setting in `/etc/zulip.conf` was previously
only used for incoming mail, to identify in Postfix configuration
which messages were "local."
Also set `/etc/mailname`, which is used by Postfix to set how it
identifies to other hosts when sending outgoing email.
Co-authored-by: Alex Vandiver <alexmv@zulip.com>
These files are not Jinja2 templates, so there's no reason that they needed
to be inside `templates/zerver`. Moving them to the top level reflects their
importance and also makes it feel nicer to work on editing the help center content,
without it being unnecessary buried deep in the codebase.
For descriptive endpoints, such as `/register`, that might raise
Schema Validation errors via `validate_against_openapi_schema`,
omits the OpenAPI schema definition in the error output.
Also omits the error instance definition in the error output
when it is a jsonschema object with over 100 properties. This
means that the test instance for objects, like user settings,
will be printed in the error output, but the test instance for
the entire endpoint will not be printed to the console.
The omitted output can be thousands of lines long making it
difficult to find the initial console output that actually helps
the contributor with debugging.
Adds a section in "Documenting REST API endpoints" about
debugging and understanding these errors that is linked to
in the error console output.
When file uploads are stored in S3, this means that Zulip serves as a
302 to S3. Because browsers do not cache redirects, this means that
no image contents can be cached -- and upon every page load or reload,
every recently-posted image must be re-fetched. This incurs extra
load on the Zulip server, as well as potentially excessive bandwidth
usage from S3, and on the client's connection.
Switch to fetching the content from S3 in nginx, and serving the
content from nginx. These have `Cache-control: private, immutable`
headers set on the response, allowing browsers to cache them locally.
Because nginx fetching from S3 can be slow, and requests for uploads
will generally be bunched around when a message containing them are
first posted, we instruct nginx to cache the contents locally. This
is safe because uploaded file contents are immutable; access control
is still mediated by Django. The nginx cache key is the URL without
query parameters, as those parameters include a time-limited signed
authentication parameter which lets nginx fetch the non-public file.
This adds a number of nginx-level configuration parameters to control
the caching which nginx performs, including the amount of in-memory
index for he cache, the maximum storage of the cache on disk, and how
long data is retained in the cache. The currently-chosen figures are
reasonable for small to medium deployments.
The most notable effect of this change is in allowing browsers to
cache uploaded image content; however, while there will be many fewer
requests, it also has an improvement on request latency. The
following tests were done with a non-AWS client in SFO, a server and
S3 storage in us-east-1, and with 100 requests after 10 requests of
warm-up (to fill the nginx cache). The mean and standard deviation
are shown.
| | Redirect to S3 | Caching proxy, hot | Caching proxy, cold |
| ----------------- | ------------------- | ------------------- | ------------------- |
| Time in Django | 263.0 ms ± 28.3 ms | 258.0 ms ± 12.3 ms | 258.0 ms ± 12.3 ms |
| Small file (842b) | 586.1 ms ± 21.1 ms | 266.1 ms ± 67.4 ms | 288.6 ms ± 17.7 ms |
| Large file (660k) | 959.6 ms ± 137.9 ms | 609.5 ms ± 13.0 ms | 648.1 ms ± 43.2 ms |
The hot-cache performance is faster for both large and small files,
since it saves the client the time having to make a second request to
a separate host. This performance improvement remains at least 100ms
even if the client is on the same coast as the server.
Cold nginx caches are only slightly slower than hot caches, because
VPC access to S3 endpoints is extremely fast (assuming it is in the
same region as the host), and nginx can pool connections to S3 and
reuse them.
However, all of the 648ms taken to serve a cold-cache large file is
occupied in nginx, as opposed to the only 263ms which was spent in
nginx when using redirects to S3. This means that to overall spend
less time responding to uploaded-file requests in nginx, clients will
need to find files in their local cache, and skip making an
uploaded-file request, at least 60% of the time. Modeling shows a
reduction in the number of client requests by about 70% - 80%.
The `Content-Disposition` header logic can now also be entirely shared
with the local-file codepath, as can the `url_only` path used by
mobile clients. While we could provide the direct-to-S3 temporary
signed URL to mobile clients, we choose to provide the
served-from-Zulip signed URL, to better control caching headers on it,
and greater consistency. In doing so, we adjust the salt used for the
URL; since these URLs are only valid for 60s, the effect of this salt
change is minimal.
Moving `/user_avatars/` to being served partially through Django
removes the need for the `no_serve_uploads` nginx reconfiguring when
switching between S3 and local backends. This is important because a
subsequent commit will move S3 attachments to being served through
nginx, which would make `no_serve_uploads` entirely nonsensical of a
name.
Serve the files through Django, with an offload for the actual image
response to an internal nginx route. In development, serve the files
directly in Django.
We do _not_ mark the contents as immutable for caching purposes, since
the path for avatar images is hashed only by their user-id and a salt,
and as such are reused when a user's avatar is updated.
The authenticate_by_username limit of 5 attempts per 30 minutes can get
annoying in some cases where the user really forgot their password and
should be allowed to keep trying with admin approvial - so we should
document the command that allows unblocking them.
Changes all the uses of the word "operators" to "filters" in
contributor docs, help center, and landing page to align with
the updated help center documentation.
Moves files in `templates/zerver/help/include` that are used
specifically for API documentation pages to be in a new directory:
`templates/zerver/api/include`.
Adds a boolean parameter to `render_markdown_path` to be used
for help center documentation articles.
Also moves the test file `empty.md` to the new directory since
this is the default directory for these special include macros
that are used in documentation pages.
Moves files in `templates/zerver/help/include` that are used
specifically for integrations documentation to be in a new
directory: `templates/zerver/integrations/include`.
Adds a boolean parameter to `render_markdown_path` to be used
for integrations documentation pages.
The documentation included the full policy for the file uploads
bucket, but only one additional statement for the avatars bucket; the
reader needed to assemble the full policy themselves.
Switch to explicitly providing the full policy for both.
Fixes#23110.
n0099
Ishita Gupta
Anders Kaseorg
Alex Vandiver
nimish
Aman Agrawal
Mateusz Mandera
Karl Stolley
Matthew Volpe
David Rosa
Alya Abbott
Tim Abbott
Zixuan James Li
Lauryn Menard
Toyam Cox
sbansal1999
Karl Stolley
AcKindle3
Hardik Dharmani
Ujjawal Modi
sayamsamal
Daniil Fadeev
YashRE42
oribe1115
Alya Abbott
Chris Bobbe
Lakshay Mittal
Alessandro Toppi
Dhruv Gupta
Tran Sang
Lauryn Menard
Sahil Batra
khadeom
Josh Klar