diff --git a/requirements/dev.in b/requirements/dev.in index f3e234f0aa..63b545d687 100644 --- a/requirements/dev.in +++ b/requirements/dev.in @@ -20,7 +20,7 @@ coverage https://github.com/zulip/fakeldap/archive/ff32deaad34b91d5ba5735f314362c0456c92607.zip#egg=fakeldap==0.6.1zulip # For testing mock http requests -httpretty +responses # For sorting imports isort diff --git a/requirements/dev.txt b/requirements/dev.txt index bc80d35ba6..1f143a0b17 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -307,8 +307,6 @@ html2text==2019.9.26 \ httplib2==0.14.0 \ --hash=sha256:34537dcdd5e0f2386d29e0e2c6d4a1703a3b982d34c198a5102e6e5d6194b107 \ --hash=sha256:409fa5509298f739b34d5a652df762cb0042507dc93f6633e306b11289d6249d -httpretty==0.9.7 \ - --hash=sha256:66216f26b9d2c52e81808f3e674a6fb65d4bf719721394a1a9be926177e55fbe hypchat==0.21 \ --hash=sha256:ef37a9cd8103bb13ad772b28ba9223ca9d4278371e374450c3ea2918df70a8e9 hyper==0.7.0 \ @@ -722,8 +720,7 @@ requests[security]==2.22.0 \ # via docker, hypchat, matrix-client, moto, premailer, pyoembed, python-digitalocean, python-gcm, python-twitter, requests-oauthlib, responses, social-auth-core, sphinx, stripe, transifex-client, twilio responses==0.10.7 \ --hash=sha256:46d4e546a19fc6106bc7e804edd4551ef04690405e41e7e750ebc295d042623b \ - --hash=sha256:93b1e0f2f960c0f3304ca4436856241d64c33683ef441431b9caf1d05d9d9e23 \ - # via moto + --hash=sha256:93b1e0f2f960c0f3304ca4436856241d64c33683ef441431b9caf1d05d9d9e23 rsa==4.0 \ --hash=sha256:14ba45700ff1ec9eeb206a2ce76b32814958a98e372006c8fb76ba820211be66 \ --hash=sha256:1a836406405730121ae9823e19c6e806c62bbad73f890574fff50efa4122c487 \ diff --git a/tools/test-backend b/tools/test-backend index e864f10432..14ffeda914 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -4,18 +4,18 @@ from lib import sanity_check sanity_check.check_venv(__file__) -from typing import List, Any +from typing import List, Iterator import glob import argparse +import contextlib +import mock import os import shlex import sys import subprocess import tempfile import ujson -import httplib2 -import httpretty -import requests +import responses import django from django.conf import settings @@ -154,32 +154,27 @@ def write_failed_tests(failed_tests: List[str]) -> None: with open(FAILED_TEST_PATH, 'w') as f: ujson.dump(failed_tests, f) -def block_internet() -> None: - # We are blocking internet currently by assuming mostly any test would use - # httplib2 to access internet. - requests_orig = requests.request +@contextlib.contextmanager +def block_internet() -> Iterator[None]: + # Monkey-patching - responses library raises requests.ConnectionError when access to an unregistered URL + # is attempted. We want to replace that with our own exception, so that it propagates all the way: + with mock.patch.object(responses, 'ConnectionError', new=ZulipInternetBlockedError): + # We'll run all tests in this context manager. It'll cause an error to be raised (see above comment), + # if any code attempts to access the internet. + with responses.RequestsMock(): + yield - def internet_guard_requests(*args: Any, **kwargs: Any) -> Any: - if httpretty.is_enabled(): - return requests_orig(*args, **kwargs) - raise Exception("Outgoing network requests are not allowed in the Zulip tests. " - "More details and advice are available here:" - "https://zulip.readthedocs.io/en/latest/testing/testing.html#internet-access-inside-test-suites") - - requests.request = internet_guard_requests - - http2lib_request_orig = httplib2.Http.request - - def internet_guard_httplib2(*args: Any, **kwargs: Any) -> Any: - if httpretty.is_enabled(): - return http2lib_request_orig(*args, **kwargs) - raise Exception("Outgoing network requests are not allowed in the Zulip tests. " - "More details and advice are available here:" - "https://zulip.readthedocs.io/en/latest/testing/testing.html#internet-access-inside-test-suites") - httplib2.Http.request = internet_guard_httplib2 +class ZulipInternetBlockedError(Exception): + def __init__(self, original_msg: str) -> None: + zulip_msg = ( + "Outgoing network requests are not allowed in the Zulip tests. " + "More details and advice are available here:" + "https://zulip.readthedocs.io/en/latest/testing/testing.html#internet-access-inside-test-suites" + ) + msg = "{}\nResponses library error message: {}".format(zulip_msg, original_msg) + super().__init__(msg) def main() -> None: - block_internet() TOOLS_DIR = os.path.dirname(os.path.abspath(__file__)) os.chdir(os.path.dirname(TOOLS_DIR)) sys.path.insert(0, os.path.dirname(TOOLS_DIR)) @@ -381,11 +376,12 @@ def main() -> None: else: print("-- Running tests in serial mode.", flush=True) - test_runner = TestRunner(failfast=options.fatal_errors, verbosity=2, - parallel=parallel, reverse=options.reverse, - keepdb=True) - failures, failed_tests = test_runner.run_tests(suites, full_suite=full_suite, - include_webhooks=include_webhooks) + with block_internet(): + test_runner = TestRunner(failfast=options.fatal_errors, verbosity=2, + parallel=parallel, reverse=options.reverse, + keepdb=True) + failures, failed_tests = test_runner.run_tests(suites, full_suite=full_suite, + include_webhooks=include_webhooks) write_failed_tests(failed_tests) templates_not_rendered = test_runner.get_shallow_tested_templates() diff --git a/version.py b/version.py index f7a2ca775d..c48057853a 100644 --- a/version.py +++ b/version.py @@ -26,4 +26,4 @@ LATEST_RELEASE_ANNOUNCEMENT = "https://blog.zulip.org/2019/12/13/zulip-2-1-relea # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = '67.0' +PROVISION_VERSION = '67.1' diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index b2c3d52b80..e6eacb6106 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -11,7 +11,7 @@ from typing import Any, Callable, Dict, List, Optional, Tuple from django.core import signing from django.urls import reverse -import httpretty +import responses import ldap import jwt @@ -384,38 +384,15 @@ class AuthBackendTest(ZulipTestCase): backends_to_test = { 'google': { 'urls': [ - { - 'url': "https://accounts.google.com/o/oauth2/token", - 'method': httpretty.POST, - 'status': 200, - 'body': json.dumps(token_data_dict), - }, - { - 'url': "https://www.googleapis.com/oauth2/v3/userinfo", - 'method': httpretty.GET, - 'status': 200, - 'body': json.dumps(google_email_data), - }, + # The limited process that we test here doesn't require mocking any urls. ], 'backend': GoogleAuthBackend, }, 'github': { 'urls': [ - { - 'url': "https://github.com/login/oauth/access_token", - 'method': httpretty.POST, - 'status': 200, - 'body': json.dumps(token_data_dict), - }, - { - 'url': "https://api.github.com/user", - 'method': httpretty.GET, - 'status': 200, - 'body': json.dumps(dict(email=user.email, name=user.full_name)), - }, { 'url': "https://api.github.com/user/emails", - 'method': httpretty.GET, + 'method': responses.GET, 'status': 200, 'body': json.dumps(github_email_data), }, @@ -448,42 +425,40 @@ class AuthBackendTest(ZulipTestCase): return google_email_data['email'] for backend_name in backends_to_test: - httpretty.enable(allow_net_connect=False) - urls = backends_to_test[backend_name]['urls'] # type: List[Dict[str, Any]] - for details in urls: - httpretty.register_uri( - details['method'], - details['url'], - status=details['status'], - body=details['body']) - backend_class = backends_to_test[backend_name]['backend'] - backend = backend_class() - backend.strategy = DjangoStrategy(storage=BaseDjangoStorage()) + with responses.RequestsMock(assert_all_requests_are_fired=True) as requests_mock: + urls = backends_to_test[backend_name]['urls'] # type: List[Dict[str, Any]] + for details in urls: + requests_mock.add( + details['method'], + details['url'], + status=details['status'], + body=details['body']) + backend_class = backends_to_test[backend_name]['backend'] + backend = backend_class() + backend.strategy = DjangoStrategy(storage=BaseDjangoStorage()) - orig_authenticate = backend_class.authenticate - backend.authenticate = patched_authenticate - orig_get_verified_emails = backend_class.get_verified_emails - if backend_name == "google": - backend.get_verified_emails = patched_get_verified_emails + orig_authenticate = backend_class.authenticate + backend.authenticate = patched_authenticate + orig_get_verified_emails = backend_class.get_verified_emails + if backend_name == "google": + backend.get_verified_emails = patched_get_verified_emails - good_kwargs = dict(backend=backend, strategy=backend.strategy, - storage=backend.strategy.storage, - response=token_data_dict, - subdomain='zulip') - bad_kwargs = dict(subdomain='acme') - with mock.patch('zerver.views.auth.redirect_and_log_into_subdomain', - return_value=user): - self.verify_backend(backend, - good_kwargs=good_kwargs, - bad_kwargs=bad_kwargs) - bad_kwargs['subdomain'] = "zephyr" - self.verify_backend(backend, - good_kwargs=good_kwargs, - bad_kwargs=bad_kwargs) - backend.authenticate = orig_authenticate - backend.get_verified_emails = orig_get_verified_emails - httpretty.disable() - httpretty.reset() + good_kwargs = dict(backend=backend, strategy=backend.strategy, + storage=backend.strategy.storage, + response=token_data_dict, + subdomain='zulip') + bad_kwargs = dict(subdomain='acme') + with mock.patch('zerver.views.auth.redirect_and_log_into_subdomain', + return_value=user): + self.verify_backend(backend, + good_kwargs=good_kwargs, + bad_kwargs=bad_kwargs) + bad_kwargs['subdomain'] = "zephyr" + self.verify_backend(backend, + good_kwargs=good_kwargs, + bad_kwargs=bad_kwargs) + backend.authenticate = orig_authenticate + backend.get_verified_emails = orig_get_verified_emails class CheckPasswordStrengthTest(ZulipTestCase): def test_check_password_strength(self) -> None: @@ -530,7 +505,7 @@ class SocialAuthBase(ZulipTestCase): from social_core.backends.utils import load_backends load_backends(settings.AUTHENTICATION_BACKENDS, force_load=True) - def register_extra_endpoints(self, + def register_extra_endpoints(self, requests_mock: responses.RequestsMock, account_data_dict: Dict[str, str], **extra_data: Any) -> None: pass @@ -602,66 +577,50 @@ class SocialAuthBase(ZulipTestCase): # We register callbacks for the key URLs on Identity Provider that # auth completion url will call - httpretty.enable(allow_net_connect=False) - httpretty.register_uri( - httpretty.POST, - self.ACCESS_TOKEN_URL, - match_querystring=False, - status=200, - body=json.dumps(token_data_dict)) - httpretty.register_uri( - httpretty.GET, - self.USER_INFO_URL, - status=200, - body=json.dumps(account_data_dict) - ) - self.register_extra_endpoints(account_data_dict, **extra_data) - - parsed_url = urllib.parse.urlparse(result.url) - csrf_state = urllib.parse.parse_qs(parsed_url.query)['state'] - result = self.client_get(self.AUTH_FINISH_URL, - dict(state=csrf_state), **headers) - - if expect_choose_email_screen and result.status_code == 200: - # For authentication backends such as GitHub that - # successfully authenticate multiple email addresses, - # we'll have an additional screen where the user selects - # which email address to login using (this screen is a - # "partial" state of the python-social-auth pipeline). - # - # TODO: Generalize this testing code for use with other - # authentication backends; for now, we just assert that - # it's definitely the GitHub authentication backend. - self.assert_in_success_response(["Select account"], result) - assert self.AUTH_FINISH_URL == "/complete/github/" - - # Testing hack: When the pipeline goes to the partial - # step, the below given URL is called again in the same - # test. If the below URL is not registered again as done - # below, the URL returns emails from previous tests. e.g - # email = 'invalid' may be one of the emails in the list - # in a test function followed by it. This is probably a - # bug in httpretty. - httpretty.disable() - httpretty.enable() - httpretty.register_uri( - httpretty.GET, - "https://api.github.com/user/emails", + with responses.RequestsMock(assert_all_requests_are_fired=False) as requests_mock: + requests_mock.add( + requests_mock.POST, + self.ACCESS_TOKEN_URL, + match_querystring=False, status=200, - body=json.dumps(self.email_data) + body=json.dumps(token_data_dict)) + requests_mock.add( + requests_mock.GET, + self.USER_INFO_URL, + status=200, + body=json.dumps(account_data_dict) ) - result = self.client_get(self.AUTH_FINISH_URL, - dict(state=csrf_state, email=account_data_dict['email']), **headers) - elif self.AUTH_FINISH_URL == "/complete/github/": - # We want to be explicit about when we expect a test to - # use the "choose email" screen, but of course we should - # only check for that screen with the GitHub backend, - # because this test code is shared with other - # authentication backends that structurally will never use - # that screen. - assert not expect_choose_email_screen + self.register_extra_endpoints(requests_mock, account_data_dict, **extra_data) + + parsed_url = urllib.parse.urlparse(result.url) + csrf_state = urllib.parse.parse_qs(parsed_url.query)['state'] + result = self.client_get(self.AUTH_FINISH_URL, + dict(state=csrf_state), **headers) + + if expect_choose_email_screen and result.status_code == 200: + # For authentication backends such as GitHub that + # successfully authenticate multiple email addresses, + # we'll have an additional screen where the user selects + # which email address to login using (this screen is a + # "partial" state of the python-social-auth pipeline). + # + # TODO: Generalize this testing code for use with other + # authentication backends; for now, we just assert that + # it's definitely the GitHub authentication backend. + self.assert_in_success_response(["Select account"], result) + assert self.AUTH_FINISH_URL == "/complete/github/" + + result = self.client_get(self.AUTH_FINISH_URL, + dict(state=csrf_state, email=account_data_dict['email']), **headers) + elif self.AUTH_FINISH_URL == "/complete/github/": + # We want to be explicit about when we expect a test to + # use the "choose email" screen, but of course we should + # only check for that screen with the GitHub backend, + # because this test code is shared with other + # authentication backends that structurally will never use + # that screen. + assert not expect_choose_email_screen - httpretty.disable() return result def test_social_auth_no_key(self) -> None: @@ -1340,7 +1299,7 @@ class GitHubAuthBackendTest(SocialAuthBase): AUTH_FINISH_URL = "/complete/github/" CONFIG_ERROR_URL = "/config-error/github" - def register_extra_endpoints(self, + def register_extra_endpoints(self, requests_mock: responses.RequestsMock, account_data_dict: Dict[str, str], **extra_data: Any) -> None: # Keeping a verified email before the primary email makes sure @@ -1358,15 +1317,15 @@ class GitHubAuthBackendTest(SocialAuthBase): ] email_data = extra_data.get("email_data", email_data) - httpretty.register_uri( - httpretty.GET, + requests_mock.add( + requests_mock.GET, "https://api.github.com/user/emails", status=200, body=json.dumps(email_data) ) - httpretty.register_uri( - httpretty.GET, + requests_mock.add( + requests_mock.GET, "https://api.github.com/teams/zulip-webapp/members/None", status=200, body=json.dumps(email_data) diff --git a/zerver/tests/test_internet.py b/zerver/tests/test_internet.py new file mode 100644 index 0000000000..0fee3306e5 --- /dev/null +++ b/zerver/tests/test_internet.py @@ -0,0 +1,20 @@ +from zerver.lib.test_classes import ZulipTestCase + +import responses +import requests + +class ResponsesTest(ZulipTestCase): + def test_responses(self) -> None: + # With our test setup, accessing the internet should be blocked. + with self.assertRaises(Exception): + result = requests.request('GET', 'https://www.google.com') + + # A test can invoke its own responses.RequestsMock context manager + # and register URLs to mock, accessible from within the context. + with responses.RequestsMock() as requests_mock: + requests_mock.add(responses.GET, 'https://www.google.com', + body='{}', status=200, + content_type='application/json') + result = requests.request('GET', 'https://www.google.com') + self.assertEqual(result.status_code, 200) + self.assertEqual(result.text, '{}')