From 0e1cdaa77f57aa7f609d5ef7da26a52814c7ff74 Mon Sep 17 00:00:00 2001 From: Vidar Tonaas Fauske <510760+vidartf@users.noreply.github.com> Date: Sat, 14 Jan 2023 17:48:53 +0000 Subject: [PATCH] fix tests when >jupyter_server-2 Fix cookie secret Remove notebook tests With jupyter_server 2.0, we cannot support launching with notebook. The tests can potentially be reenabled with notebook 7.0, as it is based on jupyter_server. Add token auth fixture When there is a token, xsrf cookies/headers are ignored, so using it makes tests easier. cleanup Also use token auth in CLI tests Also fix hg tests f --- nbdime/tests/conftest.py | 41 ++++++++++++++++++++++++++++++----- nbdime/tests/test_cli_apps.py | 22 ++++++++++--------- nbdime/tests/test_web.py | 9 ++++---- nbdime/webapp/nbdimeserver.py | 5 ++--- 4 files changed, 55 insertions(+), 22 deletions(-) diff --git a/nbdime/tests/conftest.py b/nbdime/tests/conftest.py index bb38e41b..cd118fb2 100644 --- a/nbdime/tests/conftest.py +++ b/nbdime/tests/conftest.py @@ -234,6 +234,20 @@ def app(nbdime_base_url, filespath): )[0] +@fixture +def auth_header(): + old = os.environ.get('JUPYTER_TOKEN') + os.environ['JUPYTER_TOKEN'] = TEST_TOKEN + try: + yield { + 'Authorization': 'token %s' % TEST_TOKEN + } + finally: + if old: + os.environ['JUPYTER_TOKEN'] = old + else: + del os.environ['JUPYTER_TOKEN'] + @fixture def json_schema_diff(request): schema_path = os.path.join(schema_dir, 'diff_format.schema.json') @@ -438,17 +452,33 @@ def create_server_extension_config(tmpdir_factory, cmd): } config_str = json.dumps(config) if isinstance(config_str, bytes): - config_str = unicode(config_str) + config_str = str(config_str) path.join(filename).write_text(config_str, 'utf-8') return str(path) - -@fixture(scope='module', params=('notebook', 'jupyter_server')) +# TODO: Add back 'notebook' as param when NB 7.0 is out ? +@fixture(scope='module', params=('jupyter_server',)) def server_extension_app(tmpdir_factory, request): cmd = request.param - appname = 'NotebookApp' if cmd == 'notebook' else 'ServerApp' + if cmd == 'notebook': + token_config_location = 'NotebookApp' + else: + v = None + try: + from importlib.metadata import version + v = version('jupyter_server') + except ImportError: + import pkg_resources + v = pkg_resources.get_distribution('jupyter_server').version + from packaging import version + if version.parse(v).major >= 2: + token_config_location = 'IdentityProvider' + else: + token_config_location = 'ServerApp' + + def _kill_nb_app(): try: @@ -470,7 +500,8 @@ def _kill_nb_app(): sys.executable, '-m', cmd, '--port=%i' % port, '--ip=127.0.0.1', - '--no-browser', '--%s.token=%s' % (appname, TEST_TOKEN)], + '--log-level=DEBUG', + '--no-browser', '--%s.token=%s' % (token_config_location, TEST_TOKEN)], env=env) request.addfinalizer(_kill_nb_app) diff --git a/nbdime/tests/test_cli_apps.py b/nbdime/tests/test_cli_apps.py index bc2280ca..5d789102 100644 --- a/nbdime/tests/test_cli_apps.py +++ b/nbdime/tests/test_cli_apps.py @@ -585,7 +585,7 @@ def test_git_mergedriver(git_repo, filespath): @pytest.mark.timeout(timeout=3*WEB_TEST_TIMEOUT) -def test_git_difftool(git_repo, unique_port, popen_with_terminator): +def test_git_difftool(git_repo, unique_port, popen_with_terminator, auth_header): gitdifftool.main(['config', '--enable']) cmd = get_output('git config --get --local difftool.nbdime.cmd').strip() @@ -608,7 +608,7 @@ def test_git_difftool(git_repo, unique_port, popen_with_terminator): r = requests.get(url + '/difftool') r.raise_for_status() # close it - r = requests.post(url + '/api/closetool', json={'exitCode': 0}) + r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header) r.raise_for_status() time.sleep(0.25) # wait for exit @@ -617,7 +617,7 @@ def test_git_difftool(git_repo, unique_port, popen_with_terminator): @pytest.mark.timeout(timeout=3*WEB_TEST_TIMEOUT) -def test_git_mergetool(git_repo, unique_port, popen_with_terminator): +def test_git_mergetool(git_repo, unique_port, popen_with_terminator, auth_header): gitmergetool.main(['config', '--enable']) cmd = get_output('git config --get --local mergetool.nbdime.cmd').strip() @@ -642,11 +642,12 @@ def test_git_mergetool(git_repo, unique_port, popen_with_terminator): url_concat(url + '/api/store', {'outputfilename': 'merge-conflict.ipynb'}), data=json.dumps({ 'merged': nbformat.v4.new_notebook(), - }) + }), + headers=auth_header ) r.raise_for_status() # close it - r = requests.post(url + '/api/closetool', json={'exitCode': 0}) + r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header) r.raise_for_status() # wait for exit process.wait() @@ -693,7 +694,7 @@ def test_hg_mergedriver(hg_repo, filespath, reset_log): @pytest.mark.timeout(timeout=3*WEB_TEST_TIMEOUT) -def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator): +def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator, auth_header): # enable diff/merge drivers write_local_hg_config(hg_repo) @@ -708,7 +709,7 @@ def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator): r = requests.get(url + '/difftool') r.raise_for_status() # close it - r = requests.post(url + '/api/closetool', json={'exitCode': 0}) + r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header) r.raise_for_status() time.sleep(0.25) # wait for exit @@ -717,7 +718,7 @@ def test_hg_diffweb(hg_repo, unique_port, popen_with_terminator): @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT) -def test_hg_mergetool(hg_repo, unique_port, popen_with_terminator): +def test_hg_mergetool(hg_repo, unique_port, popen_with_terminator, auth_header): # enable diff/merge drivers write_local_hg_config(hg_repo) @@ -738,11 +739,12 @@ def test_hg_mergetool(hg_repo, unique_port, popen_with_terminator): url_concat(url + '/api/store', {'outputfilename': 'merge-conflict.ipynb'}), data=json.dumps({ 'merged': nbformat.v4.new_notebook(), - }) + }), + headers=auth_header ) r.raise_for_status() # close it - r = requests.post(url + '/api/closetool', json={'exitCode': 0}) + r = requests.post(url + '/api/closetool', json={'exitCode': 0}, headers=auth_header) r.raise_for_status() # wait for exit process.wait() diff --git a/nbdime/tests/test_web.py b/nbdime/tests/test_web.py index 261ef9ad..6fb9d0f0 100644 --- a/nbdime/tests/test_web.py +++ b/nbdime/tests/test_web.py @@ -24,6 +24,7 @@ merge_a = 'multilevel-test-base.ipynb' merge_b = 'multilevel-test-local.ipynb' merge_c = 'multilevel-test-remote.ipynb' + @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT) @@ -83,12 +84,12 @@ def test_fetch_diff(http_client, base_url, nbdime_base_url): @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT) @pytest.mark.gen_test -def test_api_diff(http_client, base_url, nbdime_base_url, diff_validator, filespath): +def test_api_diff(http_client, base_url, nbdime_base_url, diff_validator, filespath, auth_header): post_data = dict(base=diff_a, remote=diff_b) body = json_encode(post_data) url = base_url + nbdime_base_url + '/api/diff' - response = yield http_client.fetch(url, method='POST', headers=None, body=body) + response = yield http_client.fetch(url, method='POST', headers=auth_header, body=body) assert response.code == 200 # Check that response is sane: data = json_decode(response.body) @@ -111,12 +112,12 @@ def test_fetch_merge(http_client, base_url, nbdime_base_url): @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT) @pytest.mark.gen_test -def test_api_merge(http_client, base_url, nbdime_base_url, merge_validator, filespath): +def test_api_merge(http_client, base_url, nbdime_base_url, merge_validator, filespath, auth_header): post_data = dict(base=merge_a, local=merge_b, remote=merge_c) body = json_encode(post_data) url = base_url + nbdime_base_url + '/api/merge' - response = yield http_client.fetch(url, method='POST', headers=None, body=body) + response = yield http_client.fetch(url, method='POST', headers=auth_header, body=body) assert response.code == 200 # Check that response is sane: data = json_decode(response.body) diff --git a/nbdime/webapp/nbdimeserver.py b/nbdime/webapp/nbdimeserver.py index 899cb9f7..bd2e5ddf 100644 --- a/nbdime/webapp/nbdimeserver.py +++ b/nbdime/webapp/nbdimeserver.py @@ -1,9 +1,7 @@ #!/usr/bin/env python # -*- coding:utf-8 -*- - - - +import base64 import io import json import logging @@ -386,6 +384,7 @@ def make_app(**params): 'jinja2_env': env, 'mathjax_url': prefix + '/nb-static/mathjax/MathJax.js', 'local_hostnames': ['localhost', '127.0.0.1'], + 'cookie_secret': base64.encodebytes(os.urandom(32)), # Needed even for an unsecured server. } if is_in_repo(nbdime_root):