Commit Graph

6814 Commits

Author SHA1 Message Date
Zixuan James Li
2925615ee7 test_decorators: Use the default value of user.
`HostRequestMock` has `user` default to `None`, which later gets
initialized as `AnonymousUser`. The separate initialization here is
unnecessary.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-26 18:00:24 -07:00
Sahil Batra
bcef35490d test_markdown: Use make_stream function for creating streams.
This commit removes the instances of using "Stream.objects.create"
in tests with make_stream function. This change will help us to
avoid adding code for things to be done after creating streams in
multiple places. We can instead just add it in make_stream function
only.
2022-07-26 17:25:52 -07:00
Mateusz Mandera
cf74d7d140 realm_reactivation: Prevent realm reactivation link reuse.
This uses the approach analogical to EmailChangeStatus for email change
confirmation links.
2022-07-26 17:14:26 -07:00
Mateusz Mandera
46c6f33b10 reactivate_realm: Change error status code on invalid links to 404. 2022-07-26 17:14:26 -07:00
Mateusz Mandera
0e2691815e confirmation: Prevent re-use of email change links.
The .status value of EmailChangeStatus was not being looked
at anywhere to prevent re-use of email change confirmation links. This
is not a security issue, since the EmailChangeStatus object has a fixed
value for the new_email, while the confirmation link has expiry time of
1 day, which prevents any reasonable malicious scenarios.

We fix this by making get_object_from_key look at
confirmation.content_object.status - which applies
generally to all confirmations where the attached object has the .status
attribute. This is desired, because we never want to
successfully get_object_from_key an object that has already been used or
reused.
This makes the prereg_user.status check in check_prereg_key redundant so
it can be deleted.
2022-07-26 17:14:26 -07:00
Mateusz Mandera
9992c7b6cc test_email_change: Extract generate_email_change_link helper. 2022-07-26 17:14:26 -07:00
Zixuan James Li
fe9ed2e69d settings: Make INVITATION_LINK_VALIDITY_MINUTES optional.
Type inference does not work when the default value of `REQ` is
non-optional while `ResultT` is optional. Mypy tries to unify
`json_validator` with `Validator[int]` in `invite_users_backend` instead
of the desired `Validator[Optional[int]]` because of the presence of the
default value `settings.INVITATION_LINK_VALIDITY_MINUTES`, which is
inferred to be an `int`. Mypy does not resort to a less specific type but
instead gives up early.

This issue applies to invite_users_backend and generate_multiuse_invite_backend
in zerver.views.invite.

There might be a way that we can add an overload to get around this, but
it's probably not worth the complexity until it comes up again more frequently.

We do in fact allow `invite_expires_in_minutes` to be `None` in places
like `do_invite_users`, `invite_users_backend`, etc, and we have
`settings.INVITATION_LINK_VALIDITY_MINUTES` as the default for them. So
it makes sense to allow having an optional value for this setting. And
since there isn't a way to independently set the value of this constant,
we move it to a different place.

TODO:

This is a temporary fix that should be refactored when the bug is fixed.

The encountered mypy issue: https://github.com/python/mypy/issues/13234

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-26 17:07:48 -07:00
Mateusz Mandera
39d8a81e51 registration: Tie PreregistrationUser to the original MultiUseInvite.
Fixes #21266.

We want to tie the prereg_user to the MultiUseInvite directly rather
than to the MultiUserInvite's confirmation object, because the latter is
not possible. This is because the flow is that after going through the
multiuse invite link, the PreregistrationUser is created together with a
Confirmation object, creating a confirmation link (via
create_confirmation_link) to which then the user is redirected to finish
account creation. This means that the PreregistrationUser is already
tied to a Confirmation, so that attribute is occupied.
2022-07-22 17:08:44 -07:00
Zixuan James Li
ebfd2b25b1 user_status: Add UserInfoDict.
The shared fields of `RawUserInfoDict` and `UserInfoDict` could have
been reused if they both require all keys or none. This is unfortunately
not the case, because subclassing does not override `__total__`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-22 16:31:18 -07:00
Zixuan James Li
e950b94ab5 test_urls: Remove legacy get_callback_string.
`_callback_str` was removed in Django in 1.10, and other logic relevant
to that particular attribute was removed in
32849b80ad, but not to its entirety. It
does not make sense to fall back to `_callback_str`. The
`get_callback_string` helper is no longer needed.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-22 16:22:47 -07:00
Anders Kaseorg
751b2a03e5 CVE-2022-31168: Fix authorization check for changing bot roles.
Due to an incorrect authorization check in Zulip Server 5.4 and
earlier, a member of an organization could craft an API call that
grants organization administrator privileges to one of their bots.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-07-21 17:59:09 -07:00
Lauryn Menard
ad2ca0e668 tests: Remove FORWARD_ADDRESS_CONFIG_FILE created in test.
Previously if `test_forward_address_details` failed, the file
created when setting the `forward_address` may not have been
removed, which would then cause an `EmailNotDeliveredException`
to be raised when then creating a new user in the dev environment.

Wraps the test in a try block, with a finally block for the call
to remove the file.
2022-07-21 15:25:11 -07:00
Mateusz Mandera
f787ddc7d2 get_object_from_key: Make mark_object_used an obligatory kwarg. 2022-07-21 15:18:15 -07:00
Mateusz Mandera
fa7700df11 confirmation: Rename STATUS_ACTIVE to STATUS_USED.
That's much more descriptive of what that value actually means about the
Confirmation objects.
2022-07-21 15:17:37 -07:00
Mateusz Mandera
cd1e062878 registration: Return proper error page if a registration link is reused.
Before this, a link still couldn't be re-used because it would trip up
exception further down user creation codepaths, but that was still a
bug. check_prereg_key is supposed to correctly validate the key - and
trigger an error page being returned if a key (or for any other reason,
the attached PreregistrationUser object) is reused.

test_validate_email_not_already_in_realm needs to be adjusted, because
it was actually re-using a key.
2022-07-21 15:17:37 -07:00
Mateusz Mandera
328a1fcf13 test_signup: Fix typo in test name. 2022-07-21 15:17:37 -07:00
Tim Abbott
8faa854267 Revert "message_flags: Filter msgs having (or not) the flag before updating."
This reverts commit 40fcf5a633.

This commit triggers bug that we haven't fully tracked down, where web
app clients will continually send `update_message_flags` requests,
that then send out via the events system "0 messages were marked as
read" notices, eventually leading to a load spike.

The Tornado part can likely be fixed by checking if
updated_message_ids is empty, but we need to track down the frontend
bug as well.
2022-07-21 14:29:54 -07:00
Anders Kaseorg
443b974b3e python: Apply changes from pyupgrade.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-07-20 22:33:28 -07:00
Lauryn Menard
8a0070ace7 decorator: Move tornado notify data parameter to REQ framework.
Instead of using `request.POST` to access the `data` parameter used
in the internal `notify_tornado` path, adds `has_request_variables`
decorator and accesses `data` as a `REQ` parameter.

Expands `test_tornado_endpoint` in `test_event_system.py` for
`data` being a required parameter for this path.
2022-07-20 14:23:05 -07:00
Zixuan James Li
31c7344979 test_message_fetch: Verify the value of WWW-Authenticate.
In `JsonableErrorHandler`, we convert `MissingAuthenticationError` into
a response that has `WWW-Authenticated` set for `/api` or `/json` views.

This covers and verify the value of the header for unauthenticated
access.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-18 18:01:42 -07:00
Sahil Batra
690420ffa0 populate_db: Fix data for "Favorite editor" custom field.
The field_data sent from client while creating a select
type field is a dict with a number as key.

In development database the field data for "Favorite editor"
field was of different form where the option label was used
as key in the dict.

This commit fixes it to be of the same as it is when creating
a field from web-app. As a result, we also need to update
the tests and this commit also update field_data for other
select-type fields.
2022-07-15 16:51:24 -07:00
Sahil Batra
fac5e0605b custom_profile_fields: Delete invalid values for select-type fields.
We delete the user values for options which were deleted for the
field.
2022-07-15 16:33:15 -07:00
Zixuan James Li
4099e56a1f custom_profile: Refactor test_update_profile_data_successfully.
This refactors the test case with more explicit type annotations, fixing
type errors discovered provided type annotations for
`CustomProfileField`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:55:03 -07:00
Zixuan James Li
52be020d0c custom_profile: Apply ProfileDataElementUpdateDict.
We explicitly annotate variables or parameters with
`ProfileDataElementUpdateDict` as necessary.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:55:03 -07:00
Zixuan James Li
db7a6d15dc test_auth_backends: Replace handcrafted requests with HostRequestMock.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
56855e0187 typing: Add assertions for Optional values.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
7c62af0d73 typing: Add assertions before accessing settings.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
a09c9a4063 test_auth_backends: Remove unused return.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
c5075be9aa test_retention: Add type annotation for expected_result
Otherwise mypy infers the type of `expected_result` to be incompatible
with the first argument of `fix_ordering_of_result`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
da4654d838 test_send_email: Fix the to_email argument to pass str.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
45db04a39e test_auth_backends: Fix invalid assertions.
`m.output` is a `list` of `str`s. It does not make sense comparing it to
a `str`. Guessed the intention here is to use `self.assert_length`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
2b1820cfcf message_edit: Avoid retyping variables.
Mypy disallows definition of the same variable with incompatible types.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
b775639f42 test: Use list comprehension for dictionary values.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Zixuan James Li
7950baafe2 tests: Add type narrowing for EmailMultiAlternatives.
Items in `django.core.mail.outbox` are by default typed as the less
general `EmailMessage` type. Before accessing the attribute
`alternatives`, we need to narrow the type to `EmailMultiAlternatives`.
Then narrow the tuple value we want to access to `str` before using
it in `assertIn` or `self.normalize_string`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-15 14:00:56 -07:00
Tim Abbott
05b70ba74a decorator: Explicitly require req_secret in internal_notify_view.
It's hard to come up with a realistic story where this would matter:
SHARED_SECRET is generated automatically during server setup at the
same time as SECRET_KEY, which is a required setting, but it seems
preferable to be explicit that this is a required parameter for the
internal_notify authentication model.
2022-07-15 09:20:37 -07:00
Lauryn Menard
855e14272a backend: Migrate secret parameter to REQ framework.
Instead of using request.POST to get any potential `secret`
parameter used in `authenticate_notify` for `internal_notify_view`
decorator, moves it to the REQ framework parameters as `req_secret`.

Updates existing tests to explicitly test for a request without
`secret` parameter, which defaults to `None`; this is also tested
in `test_event_system.py`.
2022-07-15 09:20:37 -07:00
Zixuan James Li
0056becd04 test_helpers: Default user_profile to AnonymousUser.
A request that has went through the auth middleware shouldn't have
`.user` being `None`. We should use `AnonymousUser` by default to
represent unauthenticated users.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-14 17:24:24 -07:00
Ganesh Pawar
135854db97 emoji: Use path converter for emoji name in URL.
If the emoji name contains forward slashes, the `str` converter
would treat it as a URL delimiter. Instead use the path converter, so
that forward slashes are included in the emoji name variable.

Fixes #22377
2022-07-14 14:29:40 -07:00
Sahil Batra
093a74bd54 models: Remove "role" field from Subscription.
This commit removes "role" field from subscription
objects since we are not moving forward with stream
administrator concept and instead working on new
permssions model as per #19525.
2022-07-13 17:07:44 -07:00
Sahil Batra
83383090f9 realm: Removed WILDCARD_MENTION_POLICY_STREAM_ADMINS option.
This commit removes WILDCARD_MENTION_POLICY_STREAM_ADMINS
option of wildcard_mention_policy since we are not moving
forward with stream administrator concept and instead working
on new permssions model as per #19525.

We also add a migration to change wildcard_mention_policy of
existing realms to WILDCARD_MENTION_POLICY_ADMINS. This change
is fine since we were already treating both the setting values
as same as stream admin concept was not implemented completely.
2022-07-13 17:07:44 -07:00
Sahil Batra
d31fc27912 streams: Remove is_stream_admin property and its uses.
This commit removes the is_stream_admin property of Subscription
model and also updates check_stream_access_for_delete_or_update
to not return true when is_stream_admin is True.

We also removes the relevant tests.

This change is done as we would not be moving forward with the
stream administrator concept as we have decided to modify the
permissions model as per #19525.
2022-07-13 17:07:44 -07:00
Adam Sah
cb6382369e merge_streams: Move logic to zerver/actions/streams.py. 2022-07-13 16:12:36 -07:00
Anders Kaseorg
81892df176 requirements: Upgrade to Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2022-07-13 16:07:17 -07:00
Alya Abbott
7e3388dd9e images: Move /team images into landing-page directory. 2022-07-13 15:14:19 -07:00
Zixuan James Li
74f59bd8d0 2fa: Rename zulip_otp_required for clarity.
The name does not really comply with the actual behavior of
the decorator since it returns True for an unauthenticated user.
This makes it clear that the 2fa check only applies to users that
are already logged in.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-13 14:49:41 -07:00
Zixuan James Li
00bd7513f2 2fa: Verify 2FA authentication status with is_2fa_verified.
This replaces user.is_verified with is_2fa_verified.

The helper does extra checks such that the user being checked for 2fa
authentication status is valid.

`request.user.is_verified` is functionally the same as `is_verified`
from `django_otp.middleware`, except that the former is monkey-patched
onto the user object by the 2FA middleware. We use the latter wrapped
in `is_2fa_verified` instead to avoid accessing the patched attribute.

See also: 6b24d56e59/docs/source/overview.rst (authentication-and-verification)

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-13 14:49:41 -07:00
Zixuan James Li
3367839839 decorator: Add test case for unauthenticated 2fa.
This simulates the situation in which the user is not
authenticated (as an AnonymousUser) and have 2FA enabled.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-13 14:49:41 -07:00
evykassirer
084b054827 emoji: Update emoji validation error messages with more information.
This change tells the user what a valid emoji name looks like, so that
they know what to fix if the name they chose doesn't pass the validator.
2022-07-12 16:43:29 -07:00
Mateusz Mandera
e6264cd85b test_import_export: Test exclusion of private uploads from realm export. 2022-07-12 06:08:07 +00:00
Zixuan James Li
5d524aa43d custom_profile_fields: Use cast to assert non-optional items.
The mypy django-stubs plugin incorrectly infers optional value for order,
which makes the `value_list` to be typed as `Iterable[Optional[int]]`.
We use a type cast here to ensure that
`try_reorder_relam_custom_profile_fields` won't causes mypy to complain
about it.

TODO: Remove the cast when https://github.com/typeddjango/django-stubs/issues/444 gets resovled.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
2022-07-07 11:28:13 -07:00