diff --git a/zerver/decorator.py b/zerver/decorator.py index ac94d3e2d2..2ccf66f8ba 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -459,6 +459,26 @@ def require_server_admin_api(view_func: ViewFuncT) -> ViewFuncT: return view_func(request, user_profile, *args, **kwargs) return _wrapped_view_func # type: ignore # https://github.com/python/mypy/issues/1927 +def require_non_guest_user(view_func: ViewFuncT) -> ViewFuncT: + @wraps(view_func) + def _wrapped_view_func(request: HttpRequest, user_profile: UserProfile, *args: Any, + **kwargs: Any) -> HttpResponse: + if user_profile.is_guest: + raise JsonableError(_("Not allowed for guest users")) + return view_func(request, user_profile, *args, **kwargs) + return _wrapped_view_func # type: ignore # https://github.com/python/mypy/issues/1927 + +def require_non_guest_human_user(view_func: ViewFuncT) -> ViewFuncT: + @wraps(view_func) + def _wrapped_view_func(request: HttpRequest, user_profile: UserProfile, *args: Any, + **kwargs: Any) -> HttpResponse: + if user_profile.is_guest: + raise JsonableError(_("Not allowed for guest users")) + if user_profile.is_bot: + return json_error(_("This endpoint does not accept bot requests.")) + return view_func(request, user_profile, *args, **kwargs) + return _wrapped_view_func # type: ignore # https://github.com/python/mypy/issues/1927 + # authenticated_api_view will add the authenticated user's # user_profile to the view function's arguments list, since we have to # look it up anyway. It is deprecated in favor on the REST API diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index fd849bbfd5..d8e52d3e88 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -1343,7 +1343,7 @@ class TestZulipLoginRequiredDecorator(ZulipTestCase): result = self.client_get('/accounts/accept_terms/') self.assertEqual(result.status_code, 302) -class TestRequireServerAdminDecorator(ZulipTestCase): +class TestRequireDecorators(ZulipTestCase): def test_require_server_admin_decorator(self) -> None: user_email = self.example_email('hamlet') user_realm = get_realm('zulip') @@ -1359,6 +1359,22 @@ class TestRequireServerAdminDecorator(ZulipTestCase): result = self.client_get('/activity') self.assertEqual(result.status_code, 200) + def test_require_non_guest_user_decorator(self) -> None: + guest_user = self.example_user('polonius') + self.login(guest_user.email) + result = self.common_subscribe_to_streams(guest_user.email, ["Denmark"]) + self.assert_json_error(result, "Not allowed for guest users") + + def test_require_non_guest_human_user_decorator(self) -> None: + result = self.api_get("outgoing-webhook@zulip.com", '/api/v1/bots') + self.assert_json_error(result, "This endpoint does not accept bot requests.") + + guest_user = self.example_user('polonius') + self.login(guest_user.email) + result = self.client_get('/json/bots') + self.assert_json_error(result, "Not allowed for guest users") + + class ReturnSuccessOnHeadRequestDecorator(ZulipTestCase): def test_returns_200_if_request_method_is_head(self) -> None: class HeadRequest: diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 57c48b30f8..fae991eb1c 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -27,7 +27,7 @@ from zerver.lib.response import ( ) from zerver.lib.streams import ( - access_stream_by_id, access_stream_by_name + access_stream_by_id, access_stream_by_name, filter_stream_authorization ) from zerver.lib.stream_subscription import ( @@ -2102,16 +2102,23 @@ class SubscriptionAPITest(ZulipTestCase): self.assertEqual(add_event['event']['op'], 'add') self.assertEqual(add_event['users'], [self.example_user("iago").id]) - def test_guest_user_subscribe_public(self) -> None: + def test_guest_user_subscribe(self) -> None: """Guest users cannot subscribe themselves to anything""" guest_user = self.example_user("polonius") guest_email = guest_user.email result = self.common_subscribe_to_streams(guest_email, ["Denmark"]) - self.assert_json_error(result, "Unable to access stream (Denmark).") + self.assert_json_error(result, "Not allowed for guest users") - self.make_stream('private_stream', invite_only=True) + # Verify the internal checks also block guest users. + stream = get_stream("Denmark", guest_user.realm) + self.assertEqual(filter_stream_authorization(guest_user, [stream]), + ([], [stream])) + + stream = self.make_stream('private_stream', invite_only=True) result = self.common_subscribe_to_streams(guest_email, ["private_stream"]) - self.assert_json_error(result, "Unable to access stream (private_stream).") + self.assert_json_error(result, "Not allowed for guest users") + self.assertEqual(filter_stream_authorization(guest_user, [stream]), + ([], [stream])) def test_users_getting_add_peer_event(self) -> None: """ diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 9c24adf1d5..43aaf93cd4 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -9,7 +9,7 @@ from django.http import HttpRequest, HttpResponse from zerver.lib.exceptions import JsonableError, ErrorCode from zerver.lib.request import REQ, has_request_variables from zerver.decorator import authenticated_json_post_view, \ - require_realm_admin, to_non_negative_int + require_realm_admin, to_non_negative_int, require_non_guest_user from zerver.lib.actions import bulk_remove_subscriptions, \ do_change_subscription_property, internal_prep_private_message, \ internal_prep_stream_message, \ @@ -284,6 +284,7 @@ def you_were_just_subscribed_message(acting_user: UserProfile, return msg +@require_non_guest_user @has_request_variables def add_subscriptions_backend( request: HttpRequest, user_profile: UserProfile, diff --git a/zerver/views/users.py b/zerver/views/users.py index c19033e434..d827b6625f 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -10,7 +10,8 @@ from django.shortcuts import redirect, render from django.conf import settings from django.core.exceptions import ValidationError -from zerver.decorator import require_realm_admin, zulip_login_required +from zerver.decorator import require_realm_admin, zulip_login_required, \ + require_non_guest_human_user from zerver.forms import CreateUserForm from zerver.lib.actions import do_change_avatar_fields, do_change_bot_owner, \ do_change_is_admin, do_change_default_all_public_streams, \ @@ -154,6 +155,7 @@ def get_stream_name(stream: Optional[Stream]) -> Optional[str]: return stream.name return None +@require_non_guest_human_user @has_request_variables def patch_bot_backend( request: HttpRequest, user_profile: UserProfile, email: str, @@ -242,6 +244,7 @@ def patch_bot_backend( return json_success(json_result) +@require_non_guest_human_user @has_request_variables def regenerate_bot_api_key(request: HttpRequest, user_profile: UserProfile, email: str) -> HttpResponse: try: @@ -267,6 +270,7 @@ def add_service(name: str, user_profile: UserProfile, base_url: str=None, interface=interface, token=token) +@require_non_guest_human_user @has_request_variables def add_bot_backend( request: HttpRequest, user_profile: UserProfile, @@ -362,6 +366,7 @@ def add_bot_backend( ) return json_success(json_result) +@require_non_guest_human_user def get_bots_backend(request: HttpRequest, user_profile: UserProfile) -> HttpResponse: bot_profiles = UserProfile.objects.filter(is_bot=True, is_active=True, bot_owner=user_profile)