diff --git a/zerver/decorator.py b/zerver/decorator.py index 53b523fb73..17ca1f096c 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -195,39 +195,23 @@ def validate_api_key(request, role, api_key, is_webhook=False): raise JsonableError(_("This API key only works on the root subdomain")) return remote_server - try: - profile = get_user_profile_by_email(role) - except UserProfile.DoesNotExist: - raise JsonableError(_("Invalid user: %s") % (role,)) - - if api_key != profile.api_key: - raise JsonableError(_("Invalid API key")) - - if not profile.is_active: - raise JsonableError(_("Account not active")) - if profile.realm.deactivated: - raise JsonableError(_("Realm for account has been deactivated")) - - if (not check_subdomain(get_subdomain(request), profile.realm.subdomain) and - # Allow access to localhost for Tornado - not (settings.RUNNING_INSIDE_TORNADO and - request.META["SERVER_NAME"] == "127.0.0.1" and - request.META["REMOTE_ADDR"] == "127.0.0.1")): - logging.warning("User %s attempted to access API on wrong subdomain %s" % ( - profile.email, get_subdomain(request))) - raise JsonableError(_("Account is not associated with this subdomain")) - + profile = access_user_by_api_key(request, api_key, email=role) if profile.is_incoming_webhook and not is_webhook: raise JsonableError(_("This API is not available to incoming webhook bots.")) return profile -def access_user_by_api_key(request, api_key): - # type: (HttpRequest, Text) -> UserProfile +def access_user_by_api_key(request, api_key, email=None): + # type: (HttpRequest, Text, Optional[Text]) -> UserProfile try: user_profile = UserProfile.objects.get(api_key=api_key) except UserProfile.DoesNotExist: raise JsonableError(_("Invalid API key")) + if email is not None and email != user_profile.email: + # This covers the case that the API key is correct, but for a + # different user. We may end up wanting to relaxing this + # constraint or give a different error message in the future. + raise JsonableError(_("Invalid API key")) if not user_profile.is_active: raise JsonableError(_("Account not active")) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index ea02dac3cf..2e5a2c62d5 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -1602,7 +1602,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub, dict(principals=ujson.dumps([email1, email2])), ) - self.assert_length(queries, 42) + self.assert_length(queries, 43) self.assert_length(events, 7) for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]: @@ -1630,7 +1630,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub, dict(principals=ujson.dumps([self.test_email])), ) - self.assert_length(queries, 15) + self.assert_length(queries, 16) self.assert_length(events, 2) add_event, add_peer_event = events @@ -1849,7 +1849,7 @@ class SubscriptionAPITest(ZulipTestCase): # Make sure Zephyr mirroring realms such as MIT do not get # any tornado subscription events self.assert_length(events, 0) - self.assert_length(queries, 8) + self.assert_length(queries, 9) def test_bulk_subscribe_many(self): # type: () -> None @@ -1866,7 +1866,7 @@ class SubscriptionAPITest(ZulipTestCase): dict(principals=ujson.dumps([self.test_email])), ) # Make sure we don't make O(streams) queries - self.assert_length(queries, 19) + self.assert_length(queries, 20) @slow("common_subscribe_to_streams is slow") def test_subscriptions_add_for_principal(self):