From 508dc5b6ed739134131abef3d1feca117e426778 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 4 May 2018 10:14:29 -0700 Subject: [PATCH] decorators: Add new decorators for guest users. These decorators will be part of the process for disabling access to various features for guest users. Adding this decorator to the subscribe endpoint breaks the guest users test we'd just added for the subscribe code path; we address this by adding a more base-level test on filter_stream_authorization. --- zerver/decorator.py | 20 ++++++++++++++++++++ zerver/tests/test_decorators.py | 18 +++++++++++++++++- zerver/tests/test_subs.py | 17 ++++++++++++----- zerver/views/streams.py | 3 ++- zerver/views/users.py | 7 ++++++- 5 files changed, 57 insertions(+), 8 deletions(-) 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)