Instead of directly changing the `POST` attribute of a request, we
utilize the `HostRequestMock` initializer to produce requests with
different post data.
The decorators require the decorated function to be a valid view
function. This changes the way the mocked view functions and requests
are implemented such that we can invoke view functions without future
type errors.
Of the two other logging mocks left in this file, one checks
a logging call isn't made and another makes sure errors
aren't allowed by raising an exception as a side_effect
to the logger.
This migrates some mocked Request class and mocked request achieved
with namedtuple in test_decorators and test_mirror_users to use the
refactored HostMockRequest.
Since weakref cannot be used with namedtuple, this old way of mocking a
request object should be migrated to using HostRequestMock. Only after
this change we can extract client from the request object and store it
via ZulipRequestNotes.
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.
Apparently, after upgrading to Django 3.2, mutating is_staff and then
saving can result in a user's session being destroyed.
In any case, this test is probably better written using two different
users with the different roles, which we have in our initial database
anyway.
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.
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.
This makes it much more clear that this feature does JSON encoding,
which previously was only indicated in the documentation.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
In `validate_account_and_subdomain` we check
if user's realm is not deactivated. In case
of failure of this check, we raise our standard
JsonableError. While this works well in most
cases but it creates difficulties in handling
of users with deactivated realms for non-browser
clients.
So we register a new REALM_DEACTIVATED error
code so that clients can distinguish if error
is because of deactivated account. Following
these changes `validate_account_and_subdomain`
raises RealmDeactivatedError if user's realm
is deactivated.
This error is also documented in
`/api/rest-error-handling`.
Testing: I have mostly relied on automated
backend tests to test this.
Fixes#17763.
In validate_account_and_subdomain we check if
user's account is not deactivated. In case of
failure of this check we raise our standard
JsonableError. While this works well in most
cases but it creates difficulties in handling
of deactivated accounts for non-browser clients.
So we register a new USER_DEACTIVATED error
code so that clients can distinguish if error
is because of deactivated account. Following
these changes `validate_account_and_subdomain`
raises UserDeactivatedError if user's account
is deactivated.
This error is also documented in
`/api/rest-error-handling`.
Testing: I have mostly relied on automated
backend tests to test this.
Partially addresses issue #17763.
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.
This commit removes mock.patch with assertLogs().
* Adds return value to do_rest_call() in outgoing_webhook.py, to
support asserting log output in test_outgoing_webhook_system.py.
* Logs are not asserted in test_realm.py because it would require to users
to be queried using users=User.objects.filter(realm=realm) and the order
of resulting queryset varies for each run.
* In test_decorators.py, replacement of mock.patch is not done because
I'm not sure if it's worth the effort to replace it as it's a return
value of a function.
Tweaked by tabbott to set proper mypy types.
This argument does not define if an endpoint "is a webhook"; it is set
for "/api/v1/messages", which is not really a webhook, but allows
access from webhooks.
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.
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.
8e10ab282a moved UnexpectedWebhookEventType into
`zerver.lib.exceptions`, but left the import into
`zserver.lib.webhooks.common` so that webhooks could continue to
import the exception from there.
This clutters things and adds complexity; there is no compelling
reason that the exception's source of truth should not move alongside
all other exceptions.
Before this the only way we took advantage
of the summary from UnexpectedWebhookEventType
was by looking at exc_info().
Now we just explicitly add it to the log
message, which also sets us up to call
log_exception_to_webhook_logger directly
with some sort of "summary" info
when we don't actually want a real
exception (for example, we might want to
report anomalous webhook data but still
continue the transaction).
A minor change in passing is that I move
the payload parameter lexically.
Anders Kaseorg
PIG208
Dinesh
Sahil Batra
Mateusz Mandera
Tim Abbott
seiwailai
Vishnu KS
orientor
shanukun
m-e-l-u-h-a-n
angela s
m-e-l-u-h-a-n
Alex Vandiver
palash
Aman Agrawal
Steve Howell