From d691c249dbc85dd8bde96b2491cf9479b53aeb30 Mon Sep 17 00:00:00 2001
From: Mateusz Mandera <mateusz.mandera@protonmail.com>
Date: Mon, 16 Dec 2019 08:12:39 +0100
Subject: [PATCH] api: Return a JsonableError if API key of invalid format is
 given.

---
 tools/test-api                  |  2 +-
 zerver/decorator.py             |  7 +++++--
 zerver/lib/exceptions.py        |  5 +++++
 zerver/tests/test_decorators.py | 10 +++++-----
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/test-api b/tools/test-api
index 306864a76c..d2f5787878 100755
--- a/tools/test-api
+++ b/tools/test-api
@@ -78,7 +78,7 @@ with test_server_running(force=options.force, external_host='zulipdev.com:9981')
     # Test error payloads
     client = Client(
         email=email,
-        api_key='abcedrsdfd',
+        api_key='X'*32,
         site=site
     )
     test_invalid_api_key(client)
diff --git a/zerver/decorator.py b/zerver/decorator.py
index 9491c9f7ed..ffbc1e4bfb 100644
--- a/zerver/decorator.py
+++ b/zerver/decorator.py
@@ -21,9 +21,9 @@ 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
+from zerver.lib.utils import statsd, is_remote_server, has_api_key_format
 from zerver.lib.exceptions import JsonableError, ErrorCode, \
-    InvalidJSONError, InvalidAPIKeyError, \
+    InvalidJSONError, InvalidAPIKeyError, InvalidAPIKeyFormatError, \
     OrganizationAdministratorRequired
 from zerver.lib.types import ViewFuncT
 from zerver.lib.validator import to_non_negative_int
@@ -266,6 +266,9 @@ def validate_account_and_subdomain(request: HttpRequest, user_profile: UserProfi
         raise JsonableError(_("Account is not associated with this subdomain"))
 
 def access_user_by_api_key(request: HttpRequest, api_key: str, email: Optional[str]=None) -> UserProfile:
+    if not has_api_key_format(api_key):
+        raise InvalidAPIKeyFormatError()
+
     try:
         user_profile = get_user_profile_by_api_key(api_key)
     except UserProfile.DoesNotExist:
diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py
index e89537c530..da67efaf84 100644
--- a/zerver/lib/exceptions.py
+++ b/zerver/lib/exceptions.py
@@ -216,6 +216,11 @@ class InvalidAPIKeyError(JsonableError):
     def msg_format() -> str:
         return _("Invalid API key")
 
+class InvalidAPIKeyFormatError(InvalidAPIKeyError):
+    @staticmethod
+    def msg_format() -> str:
+        return _("Malformed API key")
+
 class UnexpectedWebhookEventType(JsonableError):
     code = ErrorCode.UNEXPECTED_WEBHOOK_EVENT_TYPE
     data_fields = ['webhook_name', 'event_type']
diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py
index e71746dd50..5a604fde6a 100644
--- a/zerver/tests/test_decorators.py
+++ b/zerver/tests/test_decorators.py
@@ -14,7 +14,7 @@ from django.conf import settings
 from zerver.forms import OurAuthenticationForm
 from zerver.lib.actions import do_deactivate_realm, do_deactivate_user, \
     do_reactivate_user, do_reactivate_realm, do_set_realm_property
-from zerver.lib.exceptions import JsonableError
+from zerver.lib.exceptions import JsonableError, InvalidAPIKeyError, InvalidAPIKeyFormatError
 from zerver.lib.initial_password import initial_password
 from zerver.lib.test_helpers import (
     HostRequestMock,
@@ -291,7 +291,7 @@ class DecoratorTestCase(TestCase):
         webhook_client_name = "ZulipClientNameWebhook"
 
         request = HostRequestMock()
-        request.POST['api_key'] = 'not_existing_api_key'
+        request.POST['api_key'] = 'X'*32
 
         with self.assertRaisesRegex(JsonableError, "Invalid API key"):
             my_webhook(request)  # type: ignore # mypy doesn't seem to apply the decorator
@@ -1306,13 +1306,13 @@ class TestValidateApiKey(ZulipTestCase):
 
     def test_validate_api_key_if_profile_does_not_exist(self) -> None:
         with self.assertRaises(JsonableError):
-            validate_api_key(HostRequestMock(), 'email@doesnotexist.com', 'api_key')
+            validate_api_key(HostRequestMock(), 'email@doesnotexist.com', 'VIzRVw2CspUOnEm9Yu5vQiQtJNkvETkp')
 
     def test_validate_api_key_if_api_key_does_not_match_profile_api_key(self) -> None:
-        with self.assertRaises(JsonableError):
+        with self.assertRaises(InvalidAPIKeyFormatError):
             validate_api_key(HostRequestMock(), self.webhook_bot.email, 'not_32_length')
 
-        with self.assertRaises(JsonableError):
+        with self.assertRaises(InvalidAPIKeyError):
             # We use default_bot's key but webhook_bot's email address to test
             # the logic when an API key is passed and it doesn't belong to the
             # user whose email address has been provided.