Commit Graph

219 Commits

Author SHA1 Message Date
Anders Kaseorg 92ad4455ed requirements: Upgrade Django to 4.1.
zerver/migrations/0240_usermessage_migrate_bigint_id_into_id.py needs
to be updated to account for Django 4.1 creating AutoField as an
identity column rather than a serial column.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-10-06 15:59:07 -07:00
Zixuan James Li e16de8d9e7 scim: Further slim down SCIMClient removing unused attributes.
This removes everything from SCIMClient except the "is_authenticated`
method. Previously, "realm" and "name" were only needed for logging
purposes. It is the best to keep SCIMClient as minimal as possible, as
it is only intended to be used for authenticating requests to SCIM
views.

This change also gurantees that the "LogRequests" middleware will not
rely on the type unsafe access of the format_requestor_for_logs method
on SCIMClient.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-09-30 15:34:50 +02:00
Zixuan James Li a20c9ea28d scim: Use setattr to set request.user as scim_client.
This is a type-unsafe workaround before we can fix the problem that
django_scim2 relies on request.user being present to authenticate
requests.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-09-27 10:19:32 -07:00
Anders Kaseorg 9198fe4fac scim: Downgrade SCIMClient from a model to an ephemeral dataclass.
SCIMClient is a type-unsafe workaround for django-scim2’s conflation
of SCIM users with Django users.  Given that a SCIMClient is not a
UserProfile, it might as well not be a model at all, since it’s only
used to satisfy django-scim2’s request.user.is_authenticated queries.

This doesn’t solve the type safety issue with assigning a SCIMClient
to request.user, nor the performance issue with running the SCIM
middleware on non-SCIM requests.  But it reduces the risk of potential
consequences worse than crashing, since there’s no longer a
request.user.id for Django to confuse with the ID of an actual
UserProfile.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-09-26 11:36:48 -07:00
Mateusz Mandera d21a1fe47f middleware: Log 5xx json_errors in JsonErrorHandler.
django.request logs responses with 5xx response codes (our configuration
of the logger prevents it from logging 4xx as well which it normally
does too). However, it does it without the traceback which results in
quite unhelpful log message that look like
"Bad Gateway:/api/v1/users/me/apns_device_token" - particularly
confusing when sent via email to server admins.

The solution here is to do the logging ourselves, using Django's
log_response() (which is meant for this purpose), and including the
traceback. Django tracks (via response._has_been_logged attribute) that
the response has already been logged, and knows to not duplicate that
action. See log_response() in django's codebase for these details.

Fixes #19596.
2022-08-31 14:43:15 -07:00
Zixuan James Li 21fd62427d typing: Remove ViewFuncT.
This removes ViewFuncT and all the associated type casts with ParamSpec
and Concatenate. This provides more accurate type annotation for
decorators at the cost of making the concatenated parameters
positional-only. This change does not intend to introduce any other
behavioral difference. Note that we retype args in process_view as
List[object] because the view functions can not only be called with
arguments of type str.

Note that the first argument of rest_dispatch needs to be made
positional-only because of the presence of **kwargs.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-08-22 15:46:16 -07:00
Zixuan James Li b02779c005 request: Refactor remote_server into RequestNotes.
This eliminates the possibility of having `request.user` as
`RemoteZulipServer` by refactoring it as an attribute of `RequestNotes`.

So we can effectively narrow the type of `request.user` by testing
`user.is_authenticated` in most cases (except that of `SCIMClient`) in
code paths that require access to `.format_requestor_for_logs` where we
previously expect either `UserProfile` or `RemoteZulipServer` backed by
the implied polymorphism.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-28 09:38:40 -07:00
Zixuan James Li e665ec8ae2 middleware: Add isinstance check before retrieving content.
StreamingHttpResponse is inferred without the isinstance check in the
else branch. We refactor this is shorten the code and also type narrow
it appropriately.
2022-07-15 14:00:56 -07:00
Zixuan James Li 2095258aa5 middleware: Assert request.method is not None.
`request.method` is not `None` in normal use cases, unless an
`HttpRequest` is directly instantiated without the method being set.
This situation does not apply to `WSGIRequest` at all.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li 7d86d291d4 middleware: Remove inappropriate StreamingHttpResponse annotation.
Asserting response.stream is False is just suggesting the response being
an `HttpResponse`. This removes `StreamingHttpResponse` with the more
generic `HttpResponseBase` with an isinstance-check.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li 75925fe059 middleware: Reorder middleware to clean up LogRequests hasattr checks.
Similar to the previous commit, we should access request.user only
after it has been initialized, rather than having awkward hasattr
checks.

With updates to the settings comments about LogRequests by tabbott.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-14 17:28:50 -07:00
Anders Kaseorg 345ed1d09d middleware: Pass unhandled API exceptions through to the test suite.
This results in more useful stack traces in failing tests.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-06-23 19:23:08 -07:00
Anders Kaseorg bb6bd900cd response: Replace response.asynchronous attribute with new class.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-05-27 14:27:34 -07:00
Anders Kaseorg 0043c0b6b2 django: Use HttpRequest.headers.
Fixes #14769.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-05-13 20:42:20 -07:00
Anders Kaseorg cce142c61a middleware: Fix URL encoding of next parameter.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-05-12 17:51:51 -07:00
Lauryn Menard 8154b4a9af middleware: Add `client` REQ parameter to `parse_client`.
If an API request specified a `client` parameter, we were
already prioritizing that value over parsing the UserAgent.

In order to have these parameters logged in the `RequestNotes`
as processed parameters instead of ignored parameters, we add
the `has_request_variables` decorator to `parse_client` and
then process the potential `client` parameter through the REQ
framework.

Co-authored by: Tim Abbott <tabbott@zulip.com>
2022-04-08 11:29:33 -07:00
Anders Kaseorg b0ce4f1bce docs: Fix many spelling mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-02-07 18:51:06 -08:00
rht bb8504d925 lint: Fix typos found by codespell. 2021-10-19 16:51:13 -07:00
Mateusz Mandera 73a6f2a1a7 auth: Add support for using SCIM for account management. 2021-10-14 12:29:10 -07:00
Mateusz Mandera 8b906b5d2f request_notes: Set the realm appropriately for the root subdomain.
Requests to the root subdomain weren't getting request_notes.realm set
even if a realm exists on the root subdomain - which is actually a
common scenario, because simply having one organization, on the root
subdomain, is the simplest and common way for self-hosted deployments.
2021-09-28 10:02:52 -07:00
Mateusz Mandera fb3864ea3c auth: Change the look of SOCIAL_AUTH_SUBDOMAIN when directly opened.
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.
2021-09-10 10:47:15 -07:00
PIG208 53888e5a26 request: Refactor ZulipRequestNotes to RequestNotes.
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.
2021-09-03 08:48:45 -07:00
PIG208 fa09404dd7 typing: Use assertions for responses when appropriate.
This is part of #18777.
2021-08-20 06:02:56 -07:00
PIG208 f9644c8cf3 typing: Fix function signatures with django-stubs. 2021-08-20 06:02:55 -07:00
Anders Kaseorg ad5f0c05b5 python: Remove default "utf8" argument for encode(), decode().
Partially generated by pyupgrade.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-08-02 15:53:52 -07:00
PIG208 8121d2d58d typing: Fix misuse of HttpResponse.
Amend usage of HttpResponse when appropriate.
2021-07-27 14:31:19 +08:00
Tim Abbott 01ce58319d mypy: Fix most AnonymousUser type errors.
This commit fixes several mypy errors with Django stubs, by telling
mypy that we know in a given code path that the user is authenticated.
2021-07-24 14:55:46 -07:00
Anders Kaseorg 7c32134fb5 Revert "Revert "request: Refactor to record rate limit data using ZulipRequestNotes.""
This reverts commit 49eab4efef.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-07-19 16:48:23 -07:00
PIG208 49eab4efef Revert "request: Refactor to record rate limit data using ZulipRequestNotes."
This reverts commit 3f9a5e1e17.
2021-07-16 09:01:20 -07:00
PIG208 c03b9c95ad request: Store client information using ZulipRequestNotes.
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.
2021-07-14 12:01:07 -07:00
PIG208 8eb2c3ffdb request: Move realm from the request to ZulipRequestNotes. 2021-07-14 12:01:07 -07:00
PIG208 742c17399e request: Move miscellaneous attributes to ZulipRequestNotes.
This includes the migration of fields that require trivial changes
to be migrated to be stored with ZulipRequestNotes.

Specifically _requestor_for_logs, _set_language, _query, error_format,
placeholder_open_graph_description, saveed_response, which were all
previously set on the HttpRequest object at some point. This migration
allows them to be typed.
2021-07-14 12:01:07 -07:00
PIG208 5475334b16 request: Refactor to store requestor_for_logs in ZulipRequestNotes. 2021-07-14 12:01:07 -07:00
PIG208 3f9a5e1e17 request: Refactor to record rate limit data using ZulipRequestNotes.
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.
2021-07-14 12:01:07 -07:00
PIG208 da6e5ddcae request: Move log_data from HttpRequest to ZulipRequestNotes. 2021-07-14 12:01:05 -07:00
PIG208 8b9011dff8 json_error: Completely remove json_error.
This completes the migration from `return json_error` to
`raise JsonableError`.
2021-07-06 15:34:33 -07:00
Anders Kaseorg 544bbd5398 docs: Fix capitalization mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-05-10 09:57:26 -07:00
Tim Abbott 615ad2d5d8 middleware: Simplify logic for parsing user-agent.
This avoids calling parse_user_agent twice when dealing with official
Zulip clients, and also makes the logical flow hopefully easier to read.

We move get_client_name out of decorator.py, since it no longer
belongs there, and give it a nicer name.
2021-04-29 17:47:41 -07:00
orientor fe260fb892 middleware: Show client version in logging if available.
Fixes #14067.
2021-04-29 17:07:37 -07:00
orientor ac203cd9f1 middleware: Add client_version attribute to request. 2021-04-29 17:03:40 -07:00
orientor 6224d83dea middleware: Get client name in LogRequests instead of process_client.
This ensures it is present for all requests; while that was already
essentially true via process_client being called from every standard
decorator, this allows middleware and other code to rely on this
having been set.
2021-04-29 17:03:05 -07:00
Anders Kaseorg e7ed907cf6 python: Convert deprecated Django ugettext alias to gettext.
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>
2021-04-15 18:01:34 -07:00
Alex Vandiver 07779ea879 middleware: Do not trust X-Forwarded-For; use X-Real-Ip, set from nginx.
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.
2021-03-31 14:19:38 -07:00
Anders Kaseorg 6e4c3e41dc python: Normalize quotes with Black.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
Anders Kaseorg 11741543da python: Reformat with Black, except quotes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
Anders Kaseorg 9773c0f1a8 python: Fix string literal concatenation mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 08:02:51 -05:00
Mateusz Mandera f76202dd59 django3: Save language preference in a cookie rather than the session.
Support for saving it in the session is dropped in django3, the cookie
is the mechanism that needs to be used. The relevant i18n code doesn't
have access to the response objects and thus needs to delegate setting
the cookie to LocaleMiddleware.

Fixes the LocaleMiddleware point of #16030.
2021-01-17 10:38:58 -08:00
Mateusz Mandera 43a0c60e96 exceptions: Make RateLimited into a subclass of JsonableError.
This simplifies the code, as it allows using the mechanism of converting
JsonableErrors into a response instead of having separate, but
ultimately similar, logic in RateLimitMiddleware.
We don't touch tests here because "rate limited" error responses are
already verified in test_external.py.
2020-12-01 13:40:56 -08:00
Anders Kaseorg 72d6ff3c3b docs: Fix more capitalization issues.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-10-23 11:46:55 -07:00
Alex Vandiver 536bd3188e middleware: Move locale-setting before domain checking.
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.
2020-09-14 22:16:09 -07:00