diff --git a/zerver/decorator.py b/zerver/decorator.py index 7c3c359af5..fab5c0d239 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -21,7 +21,7 @@ from zerver.lib.exceptions import UnexpectedWebhookEventType from zerver.lib.queue import queue_json_publish from zerver.lib.subdomains import get_subdomain, user_matches_subdomain from zerver.lib.timestamp import datetime_to_timestamp, timestamp_to_datetime -from zerver.lib.utils import statsd, is_remote_server, has_api_key_format +from zerver.lib.utils import statsd, has_api_key_format from zerver.lib.exceptions import JsonableError, ErrorCode, \ InvalidJSONError, InvalidAPIKeyError, InvalidAPIKeyFormatError, \ OrganizationAdministratorRequired @@ -186,7 +186,8 @@ def validate_api_key(request: HttpRequest, role: Optional[str], if role is not None: role = role.strip() - if settings.ZILENCER_ENABLED and role is not None and is_remote_server(role): + # If `role` doesn't look like an email, it might be a uuid. + if settings.ZILENCER_ENABLED and role is not None and '@' not in role: try: remote_server = get_remote_server_by_uuid(role) except RemoteZulipServer.DoesNotExist: diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 09b5b71957..71faf60673 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -22,7 +22,6 @@ from django.utils import translation from two_factor.models import PhoneDevice from zerver.lib.initial_password import initial_password -from zerver.lib.utils import is_remote_server from zerver.lib.users import get_api_key from zerver.lib.sessions import get_session_dict_user from zerver.lib.webhooks.common import get_fixture_http_headers, standardize_headers @@ -419,37 +418,58 @@ class ZulipTestCase(TestCase): else: raise AssertionError("Couldn't find a confirmation email.") - def encode_credentials(self, identifier: str, realm: str="zulip") -> str: + def encode_uuid(self, uuid: str) -> str: """ identifier: Can be an email or a remote server uuid. """ - if identifier in self.API_KEYS: - api_key = self.API_KEYS[identifier] + if uuid in self.API_KEYS: + api_key = self.API_KEYS[uuid] else: - if is_remote_server(identifier): - api_key = get_remote_server_by_uuid(identifier).api_key - else: - user = get_user_by_delivery_email(identifier, get_realm(realm)) - api_key = get_api_key(user) - self.API_KEYS[identifier] = api_key + api_key = get_remote_server_by_uuid(uuid).api_key + self.API_KEYS[uuid] = api_key + return self.encode_credentials(uuid, api_key) + + def encode_email(self, email: str, realm: str="zulip") -> str: + assert '@' in email + if email in self.API_KEYS: + api_key = self.API_KEYS[email] + else: + user = get_user_by_delivery_email(email, get_realm(realm)) + api_key = get_api_key(user) + self.API_KEYS[email] = api_key + + return self.encode_credentials(email, api_key) + + def encode_credentials(self, identifier: str, api_key: str) -> str: + """ + identifier: Can be an email or a remote server uuid. + """ credentials = "%s:%s" % (identifier, api_key) return 'Basic ' + base64.b64encode(credentials.encode('utf-8')).decode('utf-8') + def uuid_get(self, identifier: str, *args: Any, **kwargs: Any) -> HttpResponse: + kwargs['HTTP_AUTHORIZATION'] = self.encode_uuid(identifier) + return self.client_get(*args, **kwargs) + + def uuid_post(self, identifier: str, *args: Any, **kwargs: Any) -> HttpResponse: + kwargs['HTTP_AUTHORIZATION'] = self.encode_uuid(identifier) + return self.client_post(*args, **kwargs) + def api_get(self, identifier: str, *args: Any, **kwargs: Any) -> HttpResponse: - kwargs['HTTP_AUTHORIZATION'] = self.encode_credentials(identifier, kwargs.get('subdomain', 'zulip')) + kwargs['HTTP_AUTHORIZATION'] = self.encode_email(identifier, kwargs.get('subdomain', 'zulip')) return self.client_get(*args, **kwargs) def api_post(self, identifier: str, *args: Any, **kwargs: Any) -> HttpResponse: - kwargs['HTTP_AUTHORIZATION'] = self.encode_credentials(identifier, kwargs.get('subdomain', 'zulip')) + kwargs['HTTP_AUTHORIZATION'] = self.encode_email(identifier, kwargs.get('subdomain', 'zulip')) return self.client_post(*args, **kwargs) def api_patch(self, identifier: str, *args: Any, **kwargs: Any) -> HttpResponse: - kwargs['HTTP_AUTHORIZATION'] = self.encode_credentials(identifier, kwargs.get('subdomain', 'zulip')) + kwargs['HTTP_AUTHORIZATION'] = self.encode_email(identifier, kwargs.get('subdomain', 'zulip')) return self.client_patch(*args, **kwargs) def api_delete(self, identifier: str, *args: Any, **kwargs: Any) -> HttpResponse: - kwargs['HTTP_AUTHORIZATION'] = self.encode_credentials(identifier, kwargs.get('subdomain', 'zulip')) + kwargs['HTTP_AUTHORIZATION'] = self.encode_email(identifier, kwargs.get('subdomain', 'zulip')) return self.client_delete(*args, **kwargs) def get_streams(self, user_profile: UserProfile) -> List[str]: @@ -846,7 +866,7 @@ class WebhookTestCase(ZulipTestCase): self.url = self.build_webhook_url() def api_stream_message(self, email: str, *args: Any, **kwargs: Any) -> HttpResponse: - kwargs['HTTP_AUTHORIZATION'] = self.encode_credentials(email) + kwargs['HTTP_AUTHORIZATION'] = self.encode_email(email) return self.send_and_test_stream_message(*args, **kwargs) def send_and_test_stream_message(self, fixture_name: str, expected_topic: Optional[str]=None, diff --git a/zerver/lib/utils.py b/zerver/lib/utils.py index e918e41975..1688377f5e 100644 --- a/zerver/lib/utils.py +++ b/zerver/lib/utils.py @@ -198,11 +198,3 @@ def split_by(array: List[Any], group_size: int, filler: Any) -> List[List[Any]]: """ args = [iter(array)] * group_size return list(map(list, zip_longest(*args, fillvalue=filler))) - -def is_remote_server(identifier: str) -> bool: - """ - This function can be used to identify the source of API auth - request. We can have two types of sources, Remote Zulip Servers - and UserProfiles. - """ - return "@" not in identifier diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index b00d083fe5..b5935b373a 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -408,7 +408,7 @@ class SkipRateLimitingTest(ZulipTestCase): return json_success() request = HostRequestMock(host="zulip.testserver") - request.META['HTTP_AUTHORIZATION'] = self.encode_credentials(self.example_email("hamlet")) + request.META['HTTP_AUTHORIZATION'] = self.encode_email(self.example_email("hamlet")) request.method = 'POST' with mock.patch('zerver.decorator.rate_limit') as rate_limit_mock: @@ -476,7 +476,7 @@ class DecoratorLoggingTestCase(ZulipTestCase): webhook_bot_realm = get_realm('zulip') request = HostRequestMock() - request.META['HTTP_AUTHORIZATION'] = self.encode_credentials(webhook_bot_email) + request.META['HTTP_AUTHORIZATION'] = self.encode_email(webhook_bot_email) request.method = 'POST' request.host = "zulip.testserver" @@ -519,7 +519,7 @@ body: webhook_bot_realm = get_realm('zulip') request = HostRequestMock() - request.META['HTTP_AUTHORIZATION'] = self.encode_credentials(webhook_bot_email) + request.META['HTTP_AUTHORIZATION'] = self.encode_email(webhook_bot_email) request.method = 'POST' request.host = "zulip.testserver" @@ -560,7 +560,7 @@ body: raise Exception("raised by a non-webhook view") request = HostRequestMock() - request.META['HTTP_AUTHORIZATION'] = self.encode_credentials("aaron@zulip.com") + request.META['HTTP_AUTHORIZATION'] = self.encode_email("aaron@zulip.com") request.method = 'POST' request.host = "zulip.testserver" diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 94defbac6c..f9de22e51c 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -98,15 +98,18 @@ class BouncerTestCase(ZulipTestCase): # args[0] is method, args[1] is URL. local_url = args[1].replace(settings.PUSH_NOTIFICATION_BOUNCER_URL, "") if args[0] == "POST": - result = self.api_post(self.server_uuid, - local_url, - kwargs['data'], - subdomain="") + result = self.uuid_post( + self.server_uuid, + local_url, + kwargs['data'], + subdomain='') + elif args[0] == "GET": - result = self.api_get(self.server_uuid, - local_url, - kwargs['data'], - subdomain="") + result = self.uuid_get( + self.server_uuid, + local_url, + kwargs['data'], + subdomain='') else: raise AssertionError("Unsupported method for bounce_request") return result @@ -128,9 +131,9 @@ class PushBouncerNotificationTest(BouncerTestCase): token_kind = PushDeviceToken.GCM endpoint = '/api/v1/remotes/push/unregister' - result = self.api_post(self.server_uuid, endpoint, {'token_kind': token_kind}) + result = self.uuid_post(self.server_uuid, endpoint, {'token_kind': token_kind}) self.assert_json_error(result, "Missing 'token' argument") - result = self.api_post(self.server_uuid, endpoint, {'token': token}) + result = self.uuid_post(self.server_uuid, endpoint, {'token': token}) self.assert_json_error(result, "Missing 'token_kind' argument") # We need the root ('') subdomain to be in use for this next @@ -152,13 +155,13 @@ class PushBouncerNotificationTest(BouncerTestCase): endpoint = '/api/v1/remotes/push/register' - result = self.api_post(self.server_uuid, endpoint, {'user_id': user_id, 'token_kind': token_kind}) + result = self.uuid_post(self.server_uuid, endpoint, {'user_id': user_id, 'token_kind': token_kind}) self.assert_json_error(result, "Missing 'token' argument") - result = self.api_post(self.server_uuid, endpoint, {'user_id': user_id, 'token': token}) + result = self.uuid_post(self.server_uuid, endpoint, {'user_id': user_id, 'token': token}) self.assert_json_error(result, "Missing 'token_kind' argument") - result = self.api_post(self.server_uuid, endpoint, {'token': token, 'token_kind': token_kind}) + result = self.uuid_post(self.server_uuid, endpoint, {'token': token, 'token_kind': token_kind}) self.assert_json_error(result, "Missing 'user_id' argument") - result = self.api_post(self.server_uuid, endpoint, {'user_id': user_id, 'token': token, 'token_kind': 17}) + result = self.uuid_post(self.server_uuid, endpoint, {'user_id': user_id, 'token': token, 'token_kind': 17}) self.assert_json_error(result, "Invalid token type") result = self.api_post(self.example_email("hamlet"), endpoint, {'user_id': user_id, @@ -178,19 +181,21 @@ class PushBouncerNotificationTest(BouncerTestCase): 'token': token}) self.assert_json_error(result, "Must validate with valid Zulip server API key") - result = self.api_post(self.server_uuid, endpoint, {'user_id': user_id, - 'token_kind': token_kind, - 'token': token}, - subdomain="zulip") + result = self.uuid_post( + self.server_uuid, + endpoint, + dict(user_id=user_id, token_kind=token_kind, token=token), + subdomain="zulip") self.assert_json_error(result, "Invalid subdomain for push notifications bouncer", status_code=401) # We do a bit of hackery here to the API_KEYS cache just to # make the code simple for sending an incorrect API key. self.API_KEYS[self.server_uuid] = 'invalid' - result = self.api_post(self.server_uuid, endpoint, {'user_id': user_id, - 'token_kind': token_kind, - 'token': token}) + result = self.uuid_post( + self.server_uuid, + endpoint, + dict(user_id=user_id, token_kind=token_kind, token=token)) self.assert_json_error(result, "Zulip server auth failure: key does not match role 1234-abcd", status_code=401) @@ -215,7 +220,7 @@ class PushBouncerNotificationTest(BouncerTestCase): payload = self.get_generic_payload(method) # Verify correct results are success - result = self.api_post(self.server_uuid, endpoint, payload) + result = self.uuid_post(self.server_uuid, endpoint, payload) self.assert_json_success(result) remote_tokens = RemotePushDeviceToken.objects.filter(token=payload['token']) @@ -225,22 +230,22 @@ class PushBouncerNotificationTest(BouncerTestCase): # Try adding/removing tokens that are too big... broken_token = "x" * 5000 # too big payload['token'] = broken_token - result = self.api_post(self.server_uuid, endpoint, payload) + result = self.uuid_post(self.server_uuid, endpoint, payload) self.assert_json_error(result, 'Empty or invalid length token') def test_remote_push_unregister_all(self) -> None: payload = self.get_generic_payload('register') # Verify correct results are success - result = self.api_post(self.server_uuid, - '/api/v1/remotes/push/register', payload) + result = self.uuid_post(self.server_uuid, + '/api/v1/remotes/push/register', payload) self.assert_json_success(result) remote_tokens = RemotePushDeviceToken.objects.filter(token=payload['token']) self.assertEqual(len(remote_tokens), 1) - result = self.api_post(self.server_uuid, - '/api/v1/remotes/push/unregister/all', - dict(user_id=10)) + result = self.uuid_post(self.server_uuid, + '/api/v1/remotes/push/unregister/all', + dict(user_id=10)) self.assert_json_success(result) remote_tokens = RemotePushDeviceToken.objects.filter(token=payload['token']) @@ -257,7 +262,7 @@ class PushBouncerNotificationTest(BouncerTestCase): 'token': 'xyz uses non-hex characters', 'token_kind': PushDeviceToken.APNS, } - result = self.api_post(self.server_uuid, endpoint, payload) + result = self.uuid_post(self.server_uuid, endpoint, payload) self.assert_json_error(result, 'Invalid APNS token') @override_settings(PUSH_NOTIFICATION_BOUNCER_URL='https://push.zulip.org.example.com') @@ -466,12 +471,13 @@ class AnalyticsBouncerTest(BouncerTestCase): realmauditlog_data) = build_analytics_data(RealmCount.objects.all(), InstallationCount.objects.all(), RealmAuditLog.objects.all()) - result = self.api_post(self.server_uuid, - '/api/v1/remotes/server/analytics', - {'realm_counts': ujson.dumps(realm_count_data), - 'installation_counts': ujson.dumps(installation_count_data), - 'realmauditlog_rows': ujson.dumps(realmauditlog_data)}, - subdomain="") + result = self.uuid_post( + self.server_uuid, + '/api/v1/remotes/server/analytics', + {'realm_counts': ujson.dumps(realm_count_data), + 'installation_counts': ujson.dumps(installation_count_data), + 'realmauditlog_rows': ujson.dumps(realmauditlog_data)}, + subdomain="") self.assert_json_error(result, "Data is out of order.") with mock.patch("zilencer.views.validate_incoming_table_data"): @@ -479,7 +485,7 @@ class AnalyticsBouncerTest(BouncerTestCase): # IntegrityError that will be thrown in here from breaking # the unittest transaction. with transaction.atomic(): - result = self.api_post( + result = self.uuid_post( self.server_uuid, '/api/v1/remotes/server/analytics', {'realm_counts': ujson.dumps(realm_count_data), @@ -656,10 +662,11 @@ class HandlePushNotificationTest(PushNotificationTest): # args[0] is method, args[1] is URL. local_url = args[1].replace(settings.PUSH_NOTIFICATION_BOUNCER_URL, "") if args[0] == "POST": - result = self.api_post(self.server_uuid, - local_url, - kwargs['data'], - content_type="application/json") + result = self.uuid_post( + self.server_uuid, + local_url, + kwargs['data'], + content_type="application/json") else: raise AssertionError("Unsupported method for bounce_request") return result diff --git a/zerver/webhooks/freshdesk/tests.py b/zerver/webhooks/freshdesk/tests.py index b54657b20b..1c2378abad 100644 --- a/zerver/webhooks/freshdesk/tests.py +++ b/zerver/webhooks/freshdesk/tests.py @@ -51,7 +51,7 @@ Requester Bob updated [ticket #11](http://test1234zz self.url = self.build_webhook_url() payload = self.get_body('status_changed_fixture_with_missing_key') kwargs = { - 'HTTP_AUTHORIZATION': self.encode_credentials(self.TEST_USER_EMAIL), + 'HTTP_AUTHORIZATION': self.encode_email(self.TEST_USER_EMAIL), 'content_type': 'application/x-www-form-urlencoded', } result = self.client_post(self.url, payload, **kwargs) @@ -80,7 +80,7 @@ Requester Bob updated [ticket #11](http://test1234zz self.url = self.build_webhook_url() payload = self.get_body('unknown_payload') kwargs = { - 'HTTP_AUTHORIZATION': self.encode_credentials(self.TEST_USER_EMAIL), + 'HTTP_AUTHORIZATION': self.encode_email(self.TEST_USER_EMAIL), 'content_type': 'application/x-www-form-urlencoded', } result = self.client_post(self.url, payload, **kwargs)