From bdcf697fe9fa56f77abf1e278fa5722e643db3fe Mon Sep 17 00:00:00 2001 From: Duc Trung Le Date: Thu, 9 Mar 2023 22:58:13 +0100 Subject: [PATCH] Add tests --- ci/init-db.sh | 2 +- jupyterhub/apihandlers/services.py | 10 +- jupyterhub/orm.py | 49 ++++++---- jupyterhub/tests/test_api.py | 152 ++++++++++++++++++++++++++++- jupyterhub/tests/test_db.py | 4 +- jupyterhub/tests/test_orm.py | 27 +++++ jupyterhub/tests/test_roles.py | 10 ++ 7 files changed, 232 insertions(+), 22 deletions(-) diff --git a/ci/init-db.sh b/ci/init-db.sh index a26c30ab..0d3d3223 100755 --- a/ci/init-db.sh +++ b/ci/init-db.sh @@ -21,7 +21,7 @@ fi # Configure a set of databases in the database server for upgrade tests # this list must be in sync with versions in test_db.py:test_upgrade set -x -for SUFFIX in '' _upgrade_110 _upgrade_122 _upgrade_130 _upgrade_150 _upgrade_211; do +for SUFFIX in '' _upgrade_110 _upgrade_122 _upgrade_130 _upgrade_150 _upgrade_211 _upgrade_311; do $SQL_CLIENT "DROP DATABASE jupyterhub${SUFFIX};" 2>/dev/null || true $SQL_CLIENT "CREATE DATABASE jupyterhub${SUFFIX} ${EXTRA_CREATE_DATABASE_ARGS:-};" done diff --git a/jupyterhub/apihandlers/services.py b/jupyterhub/apihandlers/services.py index 82db46bc..ce835102 100644 --- a/jupyterhub/apihandlers/services.py +++ b/jupyterhub/apihandlers/services.py @@ -49,10 +49,16 @@ class ServiceAPIHandler(APIHandler): self._check_service_model(spec) service_name = spec["name"] - new_service = self.service_from_spec(spec) + try: + new_service = self.service_from_spec(spec) + except Exception: + msg = f"Failed to create service {service_name}" + self.log.error(msg, exc_info=True) + raise web.HTTPError(400, msg) if new_service is None: raise web.HTTPError(400, f"Failed to create service {service_name}") + if new_service.api_token: # Add api token to database await self.app._add_tokens( @@ -150,7 +156,7 @@ class ServiceAPIHandler(APIHandler): if service.oauth_no_confirm: oauth_no_confirm_list = self.settings.get('oauth_no_confirm_list') - oauth_no_confirm_list.remove(service.oauth_client_id) + oauth_no_confirm_list.discard(service.oauth_client_id) self.db.delete(orm_service) self.db.commit() diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 563f847e..43d2d077 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -458,39 +458,54 @@ class Service(Base): """ return db.query(cls).filter(cls.name == name).first() - def update_column(self, column_name: str, value: any) -> bool: - """_summary_ + def _check_data_only_column(self, column_name: str) -> bool: + """Check if a column is not a ForeignKey or in a + relationship. Args: - column_name (str): _description_ - value (any): _description_ + column_name (str): ame of the column Returns: - bool: _description_ + bool: returns `True` if the column is data-only, + returns `False` otherwise. """ - if ( - hasattr(self, column_name) - and not getattr(Service, column_name).foreign_keys - ): + if hasattr(self, column_name): + column = getattr(Service, column_name) + if hasattr(column, 'foreign_keys'): + return len(column.foreign_keys) == 0 + else: + return False + else: + return False + + def update_column(self, column_name: str, value: any) -> bool: + """Update the value of a non-ForeignKey column. + + Args: + column_name (str): Name of the column + value (any): New value + + Returns: + bool: returns `True` if the column is updated, + returns `False` otherwise. + """ + if self._check_data_only_column(column_name): setattr(self, column_name, value) return True else: return False def get_column(self, column_name: str): - """_summary_ + """Get non-ForeignKey, non-relationship column Args: - column_name (str): _description_ - value (any): _description_ + column_name (str): Name of the column Returns: - bool: _description_ + The value of requested column, None if it is + a foreign key or it does not exist. """ - if ( - hasattr(self, column_name) - and not getattr(Service, column_name).foreign_keys - ): + if self._check_data_only_column(column_name): return getattr(self, column_name) else: return None diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index b7002e79..724ad89b 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -8,12 +8,13 @@ from datetime import datetime, timedelta from unittest import mock from urllib.parse import quote, urlparse +import pytest from pytest import fixture, mark from tornado.httputil import url_concat import jupyterhub -from .. import orm +from .. import orm, roles from ..apihandlers.base import PAGINATION_MEDIA_TYPE from ..objects import Server from ..utils import url_path_join as ujoin @@ -2091,6 +2092,155 @@ async def test_get_service(app, mockservice_url): assert r.status_code == 403 +@pytest.fixture +def service_data(): + return { + "oauth_client_id": "service-oauth-client-from-api", + "api_token": "api_token-from-api", + "oauth_redirect_uri": "http://127.0.0.1:5555/oauth_callback-from-api", + "oauth_no_confirm": True, + "info": {'foo': 'bar'}, + } + + +@pytest.fixture +def service_admin_user(app): + user_name = 'admin_services' + service_role = { + 'name': 'admin-services-role', + 'description': '', + 'users': [user_name], + 'scopes': ['admin:services'], + } + roles.create_role(app.db, service_role) + user = add_user(app.db, name=user_name) + roles.update_roles(app.db, user, roles=['admin-services-role']) + return user + + +@mark.services +async def test_create_service(app, service_admin_user, service_data): + db = app.db + service_name = 'service-from-api' + r = await api_request( + app, + f'services/{service_name}', + headers=auth_header(db, service_admin_user.name), + data=json.dumps(service_data), + method='post', + ) + + r.raise_for_status() + assert r.status_code == 201 + assert r.json()['name'] == service_name + orm_service = orm.Service.find(db, service_name) + assert orm_service is not None + + oath_client = ( + db.query(orm.OAuthClient) + .filter_by(identifier=service_data['oauth_client_id']) + .first() + ) + assert oath_client.redirect_uri == service_data['oauth_redirect_uri'] + + assert service_name in app._service_map + assert ( + app._service_map[service_name].oauth_no_confirm + == service_data['oauth_no_confirm'] + ) + + +@mark.services +async def test_create_service_no_role(app, service_data): + db = app.db + service_name = 'service-from-api' + r = await api_request( + app, + f'services/{service_name}', + headers=auth_header(db, 'user'), + data=json.dumps(service_data), + method='post', + ) + + assert r.status_code == 403 + + +@mark.services +async def test_create_service_conflict(app, service_admin_user, service_data): + db = app.db + service_name = 'service-from-config' + app.services = [{'name': service_name}] + app.init_services() + + r = await api_request( + app, + f'services/{service_name}', + headers=auth_header(db, service_admin_user.name), + data=json.dumps(service_data), + method='post', + ) + + assert r.status_code == 409 + + +@mark.services +async def test_create_service_duplication(app, service_admin_user, service_data): + db = app.db + service_name = 'service-from-api' + + r = await api_request( + app, + f'services/{service_name}', + headers=auth_header(db, service_admin_user.name), + data=json.dumps(service_data), + method='post', + ) + assert r.status_code == 409 + + +@mark.services +async def test_remove_service(app, service_admin_user, service_data): + db = app.db + service_name = 'service-from-api' + + r = await api_request( + app, + f'services/{service_name}', + headers=auth_header(db, service_admin_user.name), + method='delete', + ) + assert r.status_code == 200 + + orm_service = orm.Service.find(db, service_name) + assert orm_service is None + + oath_client = ( + db.query(orm.OAuthClient) + .filter_by(identifier=service_data['oauth_client_id']) + .first() + ) + assert oath_client is None + + assert service_name not in app._service_map + + +@mark.services +async def test_remove_service_from_config(app, service_admin_user): + db = app.db + service_name = 'service-from-config' + r = await api_request( + app, + f'services/{service_name}', + headers=auth_header(db, service_admin_user.name), + method='delete', + ) + assert r.status_code == 405 + assert ( + r.json()['message'] + == 'Service service-from-config is not modifiable at runtime' + ) + + async def test_root_api(app): base_url = app.hub.url url = ujoin(base_url, 'api') diff --git a/jupyterhub/tests/test_db.py b/jupyterhub/tests/test_db.py index f8c379bc..d9941ea1 100644 --- a/jupyterhub/tests/test_db.py +++ b/jupyterhub/tests/test_db.py @@ -44,7 +44,9 @@ def generate_old_db(env_dir, hub_version, db_url): # changes to this version list must also be reflected # in ci/init-db.sh -@pytest.mark.parametrize('hub_version', ["1.1.0", "1.2.2", "1.3.0", "1.5.0", "2.1.1"]) +@pytest.mark.parametrize( + 'hub_version', ['1.1.0', '1.2.2', '1.3.0', '1.5.0', '2.1.1', '3.1.1'] +) async def test_upgrade(tmpdir, hub_version): db_url = os.getenv('JUPYTERHUB_TEST_DB_URL') if db_url: diff --git a/jupyterhub/tests/test_orm.py b/jupyterhub/tests/test_orm.py index 74deff3b..0d00c5ea 100644 --- a/jupyterhub/tests/test_orm.py +++ b/jupyterhub/tests/test_orm.py @@ -145,6 +145,33 @@ def test_token_expiry(db): assert orm_token not in user.api_tokens +def test_service_check_data_only_column(db): + orm_service = orm.Service(name='check_data_only_column', from_config=True) + db.add(orm_service) + db.commit() + assert orm_service._check_data_only_column('from_config') + assert not orm_service._check_data_only_column('server') + assert not orm_service._check_data_only_column('oauth_client_id') + + +def test_service_get_column(db): + orm_service = orm.Service(name='test_service_get_column', from_config=True) + db.add(orm_service) + db.commit() + orm_service.server = orm.Server() + assert orm_service.get_column('from_config') + assert orm_service.get_column('server') is None + + +def test_service_update_column(db): + orm_service = orm.Service(name='test_service_set_column', from_config=True) + db.add(orm_service) + db.commit() + + assert orm_service.update_column('from_config', False) + assert not orm_service.update_column('server', orm.Server()) + + def test_service_tokens(db): service = orm.Service(name='secret') db.add(service) diff --git a/jupyterhub/tests/test_roles.py b/jupyterhub/tests/test_roles.py index 7bd90d63..f5e905fc 100644 --- a/jupyterhub/tests/test_roles.py +++ b/jupyterhub/tests/test_roles.py @@ -236,6 +236,16 @@ def test_orm_roles_delete_cascade(db): ['tokens!group=hobbits'], {'tokens!group=hobbits', 'read:tokens!group=hobbits'}, ), + ( + ['admin:services'], + { + 'read:roles:services', + 'read:services:name', + 'admin:services', + 'list:services', + 'read:services', + }, + ), ], ) def test_get_expanded_scopes(db, scopes, expected_scopes):