From e24febd4c4a9c79694d77668a929cd9898212a2d Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 18:36:02 +0100 Subject: [PATCH 01/12] Let wfst.py to be really tested --- .../publications/wrong_input/{wfst.py => wfst_test.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/dynamic_data/publications/wrong_input/{wfst.py => wfst_test.py} (100%) diff --git a/tests/dynamic_data/publications/wrong_input/wfst.py b/tests/dynamic_data/publications/wrong_input/wfst_test.py similarity index 100% rename from tests/dynamic_data/publications/wrong_input/wfst.py rename to tests/dynamic_data/publications/wrong_input/wfst_test.py From 60416af8dc1da5bbbd99b5683d4cc4dbdd144490 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 13:38:11 +0100 Subject: [PATCH 02/12] Move ensure_admin_roles to internal_role_service.py --- src/layman/__init__.py | 3 ++- src/layman/authz/internal_role_service.py | 24 +++++++++++++++++++++++ src/layman/authz/role_service.py | 21 -------------------- src/layman/upgrade/upgrade_v1_23.py | 4 ++-- src/layman/upgrade/upgrade_v1_23_test.py | 4 ++-- 5 files changed, 30 insertions(+), 26 deletions(-) create mode 100644 src/layman/authz/internal_role_service.py diff --git a/src/layman/__init__.py b/src/layman/__init__.py index a76a05daa..dc5892070 100644 --- a/src/layman/__init__.py +++ b/src/layman/__init__.py @@ -129,7 +129,8 @@ set_after_restart() logger.info(f'Recreate Role Service admin role views') - from .authz.role_service import ensure_admin_roles + from .authz.internal_role_service import ensure_admin_roles + ensure_admin_roles() pipe.multi() diff --git a/src/layman/authz/internal_role_service.py b/src/layman/authz/internal_role_service.py new file mode 100644 index 000000000..b52bd914d --- /dev/null +++ b/src/layman/authz/internal_role_service.py @@ -0,0 +1,24 @@ +from layman import settings +from db import util as db_util + +ROLE_SERVICE_SCHEMA = settings.LAYMAN_INTERNAL_ROLE_SERVICE_SCHEMA + + +def ensure_admin_roles(): + create_admin_roles_view = f"""CREATE OR REPLACE view {ROLE_SERVICE_SCHEMA}.admin_roles + as + select 'ADMIN' as name + UNION ALL + select 'GROUP_ADMIN' + UNION ALL + select %s + ;""" + db_util.run_statement(create_admin_roles_view, (settings.LAYMAN_GS_ROLE, )) + + create_admin_user_roles_view = f"""CREATE OR REPLACE view {ROLE_SERVICE_SCHEMA}.admin_user_roles + as + select %s as username, %s as rolename + UNION ALL + select %s, 'ADMIN' + ;""" + db_util.run_statement(create_admin_user_roles_view, (settings.LAYMAN_GS_USER, settings.LAYMAN_GS_ROLE, settings.LAYMAN_GS_USER)) diff --git a/src/layman/authz/role_service.py b/src/layman/authz/role_service.py index 218f43769..8c4c4e3b5 100644 --- a/src/layman/authz/role_service.py +++ b/src/layman/authz/role_service.py @@ -2,27 +2,6 @@ from layman import settings ROLE_NAME_PATTERN = r'^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)*$' -ROLE_SERVICE_SCHEMA = settings.LAYMAN_INTERNAL_ROLE_SERVICE_SCHEMA - - -def ensure_admin_roles(): - create_admin_roles_view = f"""CREATE OR REPLACE view {ROLE_SERVICE_SCHEMA}.admin_roles - as - select 'ADMIN' as name - UNION ALL - select 'GROUP_ADMIN' - UNION ALL - select %s - ;""" - db_util.run_statement(create_admin_roles_view, (settings.LAYMAN_GS_ROLE, )) - - create_admin_user_roles_view = f"""CREATE OR REPLACE view {ROLE_SERVICE_SCHEMA}.admin_user_roles - as - select %s as username, %s as rolename - UNION ALL - select %s, 'ADMIN' - ;""" - db_util.run_statement(create_admin_user_roles_view, (settings.LAYMAN_GS_USER, settings.LAYMAN_GS_ROLE, settings.LAYMAN_GS_USER)) def get_user_roles(username): diff --git a/src/layman/upgrade/upgrade_v1_23.py b/src/layman/upgrade/upgrade_v1_23.py index 0a28cc971..930c2e738 100644 --- a/src/layman/upgrade/upgrade_v1_23.py +++ b/src/layman/upgrade/upgrade_v1_23.py @@ -2,7 +2,7 @@ from db import util as db_util from layman import settings -from layman.authz import role_service as role_service_util +from layman.authz import internal_role_service logger = logging.getLogger(__name__) DB_SCHEMA = settings.LAYMAN_PRIME_SCHEMA @@ -69,7 +69,7 @@ def create_role_service_schema(): ;""" db_util.run_statement(create_layman_users_user_roles_view) - role_service_util.ensure_admin_roles() + internal_role_service.ensure_admin_roles() create_roles_view = f"""create view {ROLE_SERVICE_SCHEMA}.roles as diff --git a/src/layman/upgrade/upgrade_v1_23_test.py b/src/layman/upgrade/upgrade_v1_23_test.py index 9b7162d85..ebafadc48 100644 --- a/src/layman/upgrade/upgrade_v1_23_test.py +++ b/src/layman/upgrade/upgrade_v1_23_test.py @@ -2,7 +2,7 @@ from db import util as db_util from layman import app, settings -from layman.authz import role_service as role_service_util +from layman.authz import internal_role_service from layman.common.prime_db_schema import ensure_whole_user from test_tools import process_client from . import upgrade_v1_23 @@ -123,7 +123,7 @@ def test_create_role_service_schema(): result = db_util.run_query(user_roles_query)[0] assert result[0] + result[1] + result[2] == result[3] - role_service_util.ensure_admin_roles() + internal_role_service.ensure_admin_roles() result = db_util.run_query(roles_query)[0] assert result[0] + result[1] + result[2] == result[3] result = db_util.run_query(user_roles_query)[0] From 69af37fff27bf12474a2cee940a16e7af2c71840 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 11:23:30 +0100 Subject: [PATCH 03/12] Move split_user_and_role_names and is_user to authz module --- src/layman/authz/__init__.py | 6 +++++- src/layman/common/prime_db_schema/publications.py | 11 +---------- src/layman/util.py | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/layman/authz/__init__.py b/src/layman/authz/__init__.py index a33361e2d..073b64899 100644 --- a/src/layman/authz/__init__.py +++ b/src/layman/authz/__init__.py @@ -155,7 +155,11 @@ def complete_access_rights(access_rights_to_complete, full_access_rights): return access_rights_to_complete +def is_user(user_or_role_name): + return any(letter.islower() for letter in user_or_role_name) + + def split_user_and_role_names(user_and_role_names): - user_names = [name for name in user_and_role_names if any(letter.islower() for letter in name)] + user_names = [name for name in user_and_role_names if is_user(name)] role_names = [name for name in user_and_role_names if name not in user_names] return user_names, role_names diff --git a/src/layman/common/prime_db_schema/publications.py b/src/layman/common/prime_db_schema/publications.py index 3de6a7456..6441b7746 100644 --- a/src/layman/common/prime_db_schema/publications.py +++ b/src/layman/common/prime_db_schema/publications.py @@ -6,6 +6,7 @@ from db import util as db_util, TableUri from layman import settings, LaymanError from layman.authn import is_user_with_name +from layman.authz import split_user_and_role_names from layman.common import get_publications_consts as consts, bbox as bbox_util from . import workspaces, users, rights @@ -382,16 +383,6 @@ def owner_can_still_write(owner, raise LaymanError(43, f'Owner of the personal workspace have to keep write right.') -def is_user(user_or_role_name): - return any(letter.islower() for letter in user_or_role_name) - - -def split_user_and_role_names(user_and_role_names): - user_names = [name for name in user_and_role_names if is_user(name)] - role_names = [name for name in user_and_role_names if name not in user_names] - return user_names, role_names - - def check_rights_axioms(can_read, can_write, actor_name, diff --git a/src/layman/util.py b/src/layman/util.py index 71d65b124..d1dd43a4b 100644 --- a/src/layman/util.py +++ b/src/layman/util.py @@ -704,7 +704,7 @@ def get_complete_publication_info(workspace, publication_type, publication_name, def get_publication_writer(workspace, publication_type, publication_name): - from layman.common.prime_db_schema.publications import is_user + from layman.authz import is_user info = get_publication_info(workspace, publication_type, publication_name, context={'keys': ['access_rights']}) return next(( user_or_role for user_or_role in info['access_rights']['write'] From 920b27950576d8b2b10b349f35246323c0ec1715 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 14:42:35 +0100 Subject: [PATCH 04/12] Encapsulate ensure_user --- .../prime_db_schema/publications_test.py | 77 +++++++++---------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index a45797379..551963339 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -18,34 +18,45 @@ } -def test_only_valid_names(): +def ensure_user(username, sub): + id_workspace_user = workspaces.ensure_workspace(username) + userinfo = userinfo_baseline.copy() + userinfo['sub'] = sub + users.ensure_user(id_workspace_user, userinfo) + + +class TestOnlyValidUserNames: workspace_name = 'test_only_valid_names_workspace' username = 'test_only_valid_names_user' - with app.app_context(): - workspaces.ensure_workspace(workspace_name) - id_workspace_user = workspaces.ensure_workspace(username) - userinfo = userinfo_baseline.copy() - userinfo['sub'] = '10' - users.ensure_user(id_workspace_user, userinfo) + @pytest.fixture(scope="class", autouse=True) + def provide_data(self, request): + ensure_user(self.username, '10') + yield + if request.node.session.testsfailed == 0: + users.delete_user(self.username) - publications.only_valid_user_names(set()) - publications.only_valid_user_names({username, }) + @classmethod + def test_raises(cls): + workspace_name = cls.workspace_name + username = cls.username - with pytest.raises(LaymanError) as exc_info: - publications.only_valid_user_names({username, workspace_name}) - assert exc_info.value.code == 43 + with app.app_context(): + workspaces.ensure_workspace(workspace_name) - with pytest.raises(LaymanError) as exc_info: - publications.only_valid_user_names({workspace_name, username}) - assert exc_info.value.code == 43 + with pytest.raises(LaymanError) as exc_info: + publications.only_valid_user_names({username, workspace_name}) + assert exc_info.value.code == 43 - with pytest.raises(LaymanError) as exc_info: - publications.only_valid_user_names({'skaljgdalskfglshfgd', }) - assert exc_info.value.code == 43 + with pytest.raises(LaymanError) as exc_info: + publications.only_valid_user_names({workspace_name, username}) + assert exc_info.value.code == 43 - users.delete_user(username) - workspaces.delete_workspace(workspace_name) + with pytest.raises(LaymanError) as exc_info: + publications.only_valid_user_names({'skaljgdalskfglshfgd', }) + assert exc_info.value.code == 43 + + workspaces.delete_workspace(workspace_name) def test_at_least_one_can_write(): @@ -179,10 +190,7 @@ def test_get_user_and_role_names_for_db(): with app.app_context(): workspaces.ensure_workspace(workspace_name) - id_workspace_user = workspaces.ensure_workspace(username) - userinfo = userinfo_baseline.copy() - userinfo['sub'] = '20' - users.ensure_user(id_workspace_user, userinfo) + ensure_user(username, '20') user_names, role_names = publications.get_user_and_role_names_for_db({username, }, workspace_name) assert user_names == {username} @@ -246,14 +254,8 @@ class TestInsertRights: def provide_data(self, request): with app.app_context(): workspaces.ensure_workspace(self.workspace_name) - id_workspace_user = workspaces.ensure_workspace(self.username) - userinfo = userinfo_baseline.copy() - userinfo['sub'] = '30' - users.ensure_user(id_workspace_user, userinfo) - id_workspace_user2 = workspaces.ensure_workspace(self.username2) - userinfo = userinfo_baseline.copy() - userinfo['sub'] = '40' - users.ensure_user(id_workspace_user2, userinfo) + ensure_user(self.username, '30') + ensure_user(self.username2, '40') yield if request.node.session.testsfailed == 0: with app.app_context(): @@ -374,15 +376,8 @@ class TestUpdateRights: def provide_data(self, request): with app.app_context(): workspaces.ensure_workspace(self.workspace_name) - id_workspace_user = workspaces.ensure_workspace(self.username) - userinfo = userinfo_baseline.copy() - userinfo['sub'] = '50' - users.ensure_user(id_workspace_user, userinfo) - id_workspace_user2 = workspaces.ensure_workspace(self.username2) - userinfo = userinfo_baseline.copy() - userinfo['sub'] = '60' - users.ensure_user(id_workspace_user2, userinfo) - + ensure_user(self.username, '50') + ensure_user(self.username2, '60') publications.insert_publication(self.username, self.publication_insert_info) yield if request.node.session.testsfailed == 0: From 93147841b0f1edc31e7c4427c442be374338643f Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 11:45:52 +0100 Subject: [PATCH 05/12] Start to test only_valid_role_names --- src/layman/authz/role_service.py | 8 ++++ .../common/prime_db_schema/publications.py | 9 ++++- .../prime_db_schema/publications_test.py | 40 +++++++++++++++++++ .../publications/access_rights/test_role.py | 12 +++++- .../patch_after_feature_change_test.py | 7 +++- 5 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/layman/authz/role_service.py b/src/layman/authz/role_service.py index 8c4c4e3b5..e0140e671 100644 --- a/src/layman/authz/role_service.py +++ b/src/layman/authz/role_service.py @@ -14,3 +14,11 @@ def get_user_roles(username): """ roles = db_util.run_query(query, (username, 'ADMIN', 'GROUP_ADMIN', settings.LAYMAN_GS_ROLE, ROLE_NAME_PATTERN), uri_str=settings.LAYMAN_ROLE_SERVICE_URI) return {role[0] for role in roles} + + +def get_existent_roles(roles_to_check): + query = f""" +select name from {settings.LAYMAN_ROLE_SERVICE_SCHEMA}.roles where name = ANY(%s) + """ + rows = db_util.run_query(query, (list(roles_to_check),), uri_str=settings.LAYMAN_ROLE_SERVICE_URI) + return {row[0] for row in rows} diff --git a/src/layman/common/prime_db_schema/publications.py b/src/layman/common/prime_db_schema/publications.py index 6441b7746..91ede7564 100644 --- a/src/layman/common/prime_db_schema/publications.py +++ b/src/layman/common/prime_db_schema/publications.py @@ -6,7 +6,7 @@ from db import util as db_util, TableUri from layman import settings, LaymanError from layman.authn import is_user_with_name -from layman.authz import split_user_and_role_names +from layman.authz import split_user_and_role_names, role_service from layman.common import get_publications_consts as consts, bbox as bbox_util from . import workspaces, users, rights @@ -358,7 +358,12 @@ def only_valid_user_names(users_list): # pylint: disable=unused-argument def only_valid_role_names(roles_list): - pass + roles_list = set(roles_list) - {settings.RIGHTS_EVERYONE_ROLE} + if not roles_list: + return + non_existent_roles = roles_list - role_service.get_existent_roles(roles_list) + if non_existent_roles: + raise LaymanError(43, f'Non-existent roles found: {non_existent_roles}') def at_least_one_can_write(user_names, role_names): diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index 551963339..f1cf469e2 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -3,6 +3,7 @@ from layman import settings, app as app, LaymanError from layman.map import MAP_TYPE +from test_tools.role_service import ensure_role, delete_role from . import publications, workspaces, users DB_SCHEMA = settings.LAYMAN_PRIME_SCHEMA @@ -59,6 +60,39 @@ def test_raises(cls): workspaces.delete_workspace(workspace_name) +class TestOnlyValidRoleNames: + role1 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE1' + role2 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE2' + non_existent_role = 'TEST_ONLY_VALID_ROLE_NAMES_NON_EXISTENT_ROLE' + + @pytest.fixture(scope="class", autouse=True) + def provide_data(self, request): + roles = [self.role1, self.role2] + for role in roles: + ensure_role(role) + yield + if request.node.session.testsfailed == 0: + for role in roles: + delete_role(role) + + @pytest.mark.parametrize("roles", [ + pytest.param({role1}, id='one-existing-role'), + pytest.param({role1, role2}, id='two-existing-roles'), + pytest.param({'EVERYONE'}, id='everyone-role'), + ]) + def test_ok(self, roles): + publications.only_valid_role_names(roles) + + @pytest.mark.parametrize("roles", [ + pytest.param({non_existent_role}, id='non-existent-role'), + pytest.param({role1, non_existent_role}, id='non-existent-role-of-two-roles'), + ]) + def test_raises(self, roles): + with pytest.raises(LaymanError) as exc_info: + publications.only_valid_role_names(roles) + assert exc_info.value.code == 43 + + def test_at_least_one_can_write(): workspace_name = 'test_at_least_one_can_write_workspace' username = 'test_at_least_one_can_write_user' @@ -256,9 +290,11 @@ def provide_data(self, request): workspaces.ensure_workspace(self.workspace_name) ensure_user(self.username, '30') ensure_user(self.username2, '40') + ensure_role(self.role1) yield if request.node.session.testsfailed == 0: with app.app_context(): + delete_role(self.role1) users.delete_user(self.username) users.delete_user(self.username2) workspaces.delete_workspace(self.workspace_name) @@ -378,12 +414,16 @@ def provide_data(self, request): workspaces.ensure_workspace(self.workspace_name) ensure_user(self.username, '50') ensure_user(self.username2, '60') + ensure_role(self.role1) + ensure_role(self.role2) publications.insert_publication(self.username, self.publication_insert_info) yield if request.node.session.testsfailed == 0: with app.app_context(): publications.delete_publication(self.username, self.publication_insert_info["publ_type_name"], self.publication_insert_info["name"]) + delete_role(self.role1) + delete_role(self.role2) users.delete_user(self.username) users.delete_user(self.username2) workspaces.delete_workspace(self.workspace_name) diff --git a/tests/dynamic_data/publications/access_rights/test_role.py b/tests/dynamic_data/publications/access_rights/test_role.py index 4251e923a..1475e0d92 100644 --- a/tests/dynamic_data/publications/access_rights/test_role.py +++ b/tests/dynamic_data/publications/access_rights/test_role.py @@ -1,6 +1,6 @@ import pytest -from test_tools import process_client +from test_tools import process_client, role_service from tests import EnumTestTypes, Publication from tests.asserts.final.publication import util as assert_util from tests.dynamic_data import base_test, base_test_classes @@ -18,6 +18,7 @@ class PublicationTypes(base_test_classes.PublicationByDefinitionBase): USER_ROLE1_ROLE3_EVERYONE = {USERNAME, 'ROLE1', 'ROLE3', 'EVERYONE'} USER_ROLE1 = {USERNAME, 'ROLE1'} USER_ROLE1_ROLE2 = {USERNAME, 'ROLE1', 'ROLE2'} +ROLES = ['ROLE1', 'ROLE2', 'ROLE3'] @pytest.mark.usefixtures('oauth2_provider_mock') @@ -34,6 +35,15 @@ class TestPublication(base_test.TestSingleRestPublication): USERNAME, ] + def before_class(self): + for role in ROLES: + role_service.ensure_role(role) + + def after_class(self, request): + if request.session.testsfailed == 0 and not request.config.option.nocleanup: + for role in ROLES: + role_service.delete_role(role) + test_cases = [base_test.TestCaseType(key='role_test', publication=lambda publ_def, cls: Publication(cls.workspace, publ_def.type, diff --git a/tests/dynamic_data/publications/map_layer_relation/patch_after_feature_change_test.py b/tests/dynamic_data/publications/map_layer_relation/patch_after_feature_change_test.py index 403e87ec3..05ce7a0e5 100644 --- a/tests/dynamic_data/publications/map_layer_relation/patch_after_feature_change_test.py +++ b/tests/dynamic_data/publications/map_layer_relation/patch_after_feature_change_test.py @@ -1,7 +1,7 @@ import os import pytest -from test_tools import process_client +from test_tools import process_client, role_service from tests import EnumTestTypes, Publication from tests.asserts.final.publication import util as assert_util, internal as assert_internal from tests.dynamic_data import base_test, base_test_classes @@ -50,6 +50,7 @@ class TestPublication(base_test.TestSingleRestPublication): )] def before_class(self): + role_service.ensure_role(ROLE) self.post_publication(MAP, args={ 'file_paths': [os.path.join(DIRECTORY, 'patch_after_feature_change_map.json')], 'access_rights': { @@ -59,6 +60,10 @@ def before_class(self): 'actor_name': USER, }, scope='class') + def after_class(self, request): + if request.session.testsfailed == 0 and not request.config.option.nocleanup: + role_service.delete_role(ROLE) + def test_publication(self, layer, rest_method, rest_args): # some initial asserts map_info = process_client.get_workspace_publication(MAP.type, MAP.workspace, MAP.name) From 9ecbde53a899bca7df1e4d91c2e443379ae8e06a Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 14:51:56 +0100 Subject: [PATCH 06/12] Internal user role name raises error --- src/layman/common/prime_db_schema/publications.py | 3 +++ src/layman/common/prime_db_schema/publications_test.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/layman/common/prime_db_schema/publications.py b/src/layman/common/prime_db_schema/publications.py index 91ede7564..9189f5e9f 100644 --- a/src/layman/common/prime_db_schema/publications.py +++ b/src/layman/common/prime_db_schema/publications.py @@ -361,6 +361,9 @@ def only_valid_role_names(roles_list): roles_list = set(roles_list) - {settings.RIGHTS_EVERYONE_ROLE} if not roles_list: return + internal_user_roles = [r for r in roles_list if r.startswith('USER_')] + if internal_user_roles: + raise LaymanError(43, f'Internal user roles found: {internal_user_roles}') non_existent_roles = roles_list - role_service.get_existent_roles(roles_list) if non_existent_roles: raise LaymanError(43, f'Non-existent roles found: {non_existent_roles}') diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index f1cf469e2..179542897 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -63,6 +63,7 @@ def test_raises(cls): class TestOnlyValidRoleNames: role1 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE1' role2 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE2' + user = 'TEST_ONLY_VALID_ROLE_NAMES_USER' non_existent_role = 'TEST_ONLY_VALID_ROLE_NAMES_NON_EXISTENT_ROLE' @pytest.fixture(scope="class", autouse=True) @@ -70,8 +71,10 @@ def provide_data(self, request): roles = [self.role1, self.role2] for role in roles: ensure_role(role) + ensure_user(self.user, '11') yield if request.node.session.testsfailed == 0: + users.delete_user(self.user) for role in roles: delete_role(role) @@ -86,6 +89,7 @@ def test_ok(self, roles): @pytest.mark.parametrize("roles", [ pytest.param({non_existent_role}, id='non-existent-role'), pytest.param({role1, non_existent_role}, id='non-existent-role-of-two-roles'), + pytest.param({f'USER_{user}'}, id='internal-user-role'), ]) def test_raises(self, roles): with pytest.raises(LaymanError) as exc_info: From add0f0139fbd79b72ba2bb44a3634e3f4c32d41c Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 17:36:46 +0100 Subject: [PATCH 07/12] Invalid role name raises error --- src/layman/common/prime_db_schema/publications.py | 7 +++++++ src/layman/common/prime_db_schema/publications_test.py | 2 ++ 2 files changed, 9 insertions(+) diff --git a/src/layman/common/prime_db_schema/publications.py b/src/layman/common/prime_db_schema/publications.py index 9189f5e9f..7e6e1945b 100644 --- a/src/layman/common/prime_db_schema/publications.py +++ b/src/layman/common/prime_db_schema/publications.py @@ -361,9 +361,16 @@ def only_valid_role_names(roles_list): roles_list = set(roles_list) - {settings.RIGHTS_EVERYONE_ROLE} if not roles_list: return + + not_matching_roles = [r for r in roles_list if not re.match(role_service.ROLE_NAME_PATTERN, r)] + if not_matching_roles: + raise LaymanError(43, f'Found roles not matching to regex {role_service.ROLE_NAME_PATTERN} ' + f': {not_matching_roles}') + internal_user_roles = [r for r in roles_list if r.startswith('USER_')] if internal_user_roles: raise LaymanError(43, f'Internal user roles found: {internal_user_roles}') + non_existent_roles = roles_list - role_service.get_existent_roles(roles_list) if non_existent_roles: raise LaymanError(43, f'Non-existent roles found: {non_existent_roles}') diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index 179542897..521feb9e1 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -90,6 +90,8 @@ def test_ok(self, roles): pytest.param({non_existent_role}, id='non-existent-role'), pytest.param({role1, non_existent_role}, id='non-existent-role-of-two-roles'), pytest.param({f'USER_{user}'}, id='internal-user-role'), + pytest.param({f'INVALID__ROLE'}, id='invalid-role-two-underscores'), + pytest.param({f'0INVALID_ROLE'}, id='invalid-role-starts-with-number'), ]) def test_raises(self, roles): with pytest.raises(LaymanError) as exc_info: From a12a7b315ba35adea5dbd597f3e867959e8d54e0 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 17:46:05 +0100 Subject: [PATCH 08/12] Admin role name raises error --- src/layman/common/prime_db_schema/publications.py | 7 +++++++ src/layman/common/prime_db_schema/publications_test.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/layman/common/prime_db_schema/publications.py b/src/layman/common/prime_db_schema/publications.py index 7e6e1945b..1a9a2b701 100644 --- a/src/layman/common/prime_db_schema/publications.py +++ b/src/layman/common/prime_db_schema/publications.py @@ -1,9 +1,11 @@ +import re from dataclasses import dataclass import logging import psycopg2.extras import crs as crs_def from db import util as db_util, TableUri +from geoserver.util import RESERVED_ROLE_NAMES from layman import settings, LaymanError from layman.authn import is_user_with_name from layman.authz import split_user_and_role_names, role_service @@ -371,6 +373,11 @@ def only_valid_role_names(roles_list): if internal_user_roles: raise LaymanError(43, f'Internal user roles found: {internal_user_roles}') + reserved_admin_roles = RESERVED_ROLE_NAMES + ['ADMIN', 'ADMIN_GROUP', settings.LAYMAN_GS_ROLE] + admin_roles = [r for r in roles_list if r in reserved_admin_roles] + if admin_roles: + raise LaymanError(43, f'Admin roles found: {admin_roles}') + non_existent_roles = roles_list - role_service.get_existent_roles(roles_list) if non_existent_roles: raise LaymanError(43, f'Non-existent roles found: {non_existent_roles}') diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index 521feb9e1..7eeee3736 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -92,6 +92,13 @@ def test_ok(self, roles): pytest.param({f'USER_{user}'}, id='internal-user-role'), pytest.param({f'INVALID__ROLE'}, id='invalid-role-two-underscores'), pytest.param({f'0INVALID_ROLE'}, id='invalid-role-starts-with-number'), + pytest.param({f'ROLE_ADMINISTRATOR'}, id='ROLE_ADMINISTRATOR'), + pytest.param({f'ROLE_GROUP_ADMIN'}, id='ROLE_GROUP_ADMIN'), + pytest.param({f'ROLE_AUTHENTICATED'}, id='ROLE_AUTHENTICATED'), + pytest.param({f'ROLE_ANONYMOUS'}, id='ROLE_ANONYMOUS'), + pytest.param({f'ADMIN'}, id='ADMIN'), + pytest.param({f'ADMIN_GROUP'}, id='ADMIN_GROUP'), + pytest.param({settings.LAYMAN_GS_ROLE}, id='value-of-LAYMAN_GS_ROLE'), ]) def test_raises(self, roles): with pytest.raises(LaymanError) as exc_info: From a9b457debd164ce1028cd3dc7b10cd9cee2ccbfa Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 17:54:30 +0100 Subject: [PATCH 09/12] Too long role name raises error --- doc/models.md | 2 +- src/layman/authz/role_service.py | 2 +- src/layman/common/prime_db_schema/publications_test.py | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/models.md b/doc/models.md index 794b83021..4029b8566 100644 --- a/doc/models.md +++ b/doc/models.md @@ -81,7 +81,7 @@ ## Role - Role is any group of users. One user can be assigned to multiple roles. - Each role is identified by name that is unique among all roles. -- The name is upper-case (in contrast with [username](#username)). +- The name is upper-case (in contrast with [username](#username)), maximum length is 64 characters. - Roles can be used for assigning access rights. ## Workspace diff --git a/src/layman/authz/role_service.py b/src/layman/authz/role_service.py index e0140e671..b09f2a4ef 100644 --- a/src/layman/authz/role_service.py +++ b/src/layman/authz/role_service.py @@ -1,7 +1,7 @@ from db import util as db_util from layman import settings -ROLE_NAME_PATTERN = r'^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)*$' +ROLE_NAME_PATTERN = r'^(?!.{65,})[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)*$' def get_user_roles(username): diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index 7eeee3736..9bac4bf15 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -63,12 +63,13 @@ def test_raises(cls): class TestOnlyValidRoleNames: role1 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE1' role2 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE2' + role64 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ_ABCDEFGHIJKLMNOPQRSTUVWXYZ_ABCDEFGHIJ' user = 'TEST_ONLY_VALID_ROLE_NAMES_USER' non_existent_role = 'TEST_ONLY_VALID_ROLE_NAMES_NON_EXISTENT_ROLE' @pytest.fixture(scope="class", autouse=True) def provide_data(self, request): - roles = [self.role1, self.role2] + roles = [self.role1, self.role2, self.role64] for role in roles: ensure_role(role) ensure_user(self.user, '11') @@ -82,6 +83,7 @@ def provide_data(self, request): pytest.param({role1}, id='one-existing-role'), pytest.param({role1, role2}, id='two-existing-roles'), pytest.param({'EVERYONE'}, id='everyone-role'), + pytest.param({role64}, id='64-characters'), ]) def test_ok(self, roles): publications.only_valid_role_names(roles) @@ -99,6 +101,7 @@ def test_ok(self, roles): pytest.param({f'ADMIN'}, id='ADMIN'), pytest.param({f'ADMIN_GROUP'}, id='ADMIN_GROUP'), pytest.param({settings.LAYMAN_GS_ROLE}, id='value-of-LAYMAN_GS_ROLE'), + pytest.param({'ABCDEFGHIJKLMNOPQRSTUVWXYZ_ABCDEFGHIJKLMNOPQRSTUVWXYZ_ABCDEFGHIJK'}, id='65-characters'), ]) def test_raises(self, roles): with pytest.raises(LaymanError) as exc_info: From 1b884f0780ac8528234833ab75216b23bdb2afea Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 17:59:08 +0100 Subject: [PATCH 10/12] Parametrize TestOnlyValidUserNames --- .../prime_db_schema/publications_test.py | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index 9bac4bf15..b9aa82f61 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -33,32 +33,24 @@ class TestOnlyValidUserNames: @pytest.fixture(scope="class", autouse=True) def provide_data(self, request): ensure_user(self.username, '10') + workspaces.ensure_workspace(self.workspace_name) yield if request.node.session.testsfailed == 0: + workspaces.delete_workspace(self.workspace_name) users.delete_user(self.username) @classmethod - def test_raises(cls): - workspace_name = cls.workspace_name - username = cls.username - + @pytest.mark.parametrize("names", [ + pytest.param({username, workspace_name}, id='username-and-workspace-name'), + pytest.param({workspace_name, username}, id='workspace-name-and-username'), + pytest.param({'skaljgdalskfglshfgd'}, id='non-existent-username'), + ]) + def test_raises(cls, names): with app.app_context(): - workspaces.ensure_workspace(workspace_name) - - with pytest.raises(LaymanError) as exc_info: - publications.only_valid_user_names({username, workspace_name}) - assert exc_info.value.code == 43 - with pytest.raises(LaymanError) as exc_info: - publications.only_valid_user_names({workspace_name, username}) + publications.only_valid_user_names(names) assert exc_info.value.code == 43 - with pytest.raises(LaymanError) as exc_info: - publications.only_valid_user_names({'skaljgdalskfglshfgd', }) - assert exc_info.value.code == 43 - - workspaces.delete_workspace(workspace_name) - class TestOnlyValidRoleNames: role1 = 'TEST_ONLY_VALID_ROLE_NAMES_ROLE1' From 6de2aa6eb447146015f04bab4f61c07cedfc0d55 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Thu, 14 Dec 2023 11:27:48 +0100 Subject: [PATCH 11/12] Add mixed-case test cases --- src/layman/authz/authz_test.py | 1 + src/layman/common/prime_db_schema/publications_test.py | 1 + 2 files changed, 2 insertions(+) diff --git a/src/layman/authz/authz_test.py b/src/layman/authz/authz_test.py index 317eaeb2a..47d16313b 100644 --- a/src/layman/authz/authz_test.py +++ b/src/layman/authz/authz_test.py @@ -64,6 +64,7 @@ def test_authorize_publications_decorator_accepts_path(request_path): pytest.param(['ROLE1', 'EVERYONE'], [], ['ROLE1', 'EVERYONE'], id='only-roles'), pytest.param(['ROLE2', 'user1', 'EVERYONE', 'user2'], ['user1', 'user2'], ['ROLE2', 'EVERYONE'], id='more-users-and-roles'), + pytest.param(['mIxEd'], ['mIxEd'], [], id='mixed-case'), ]) def test_split_user_and_role_names(roles_and_users, exp_users, exp_roles): user_names, role_names = split_user_and_role_names(roles_and_users) diff --git a/src/layman/common/prime_db_schema/publications_test.py b/src/layman/common/prime_db_schema/publications_test.py index b9aa82f61..ef9f0d4f3 100644 --- a/src/layman/common/prime_db_schema/publications_test.py +++ b/src/layman/common/prime_db_schema/publications_test.py @@ -44,6 +44,7 @@ def provide_data(self, request): pytest.param({username, workspace_name}, id='username-and-workspace-name'), pytest.param({workspace_name, username}, id='workspace-name-and-username'), pytest.param({'skaljgdalskfglshfgd'}, id='non-existent-username'), + pytest.param({'mIxEd'}, id='mixed-case'), ]) def test_raises(cls, names): with app.app_context(): From 223751636aaea779ee25a072ff1546e24127666b Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Fri, 15 Dec 2023 11:44:05 +0100 Subject: [PATCH 12/12] Move negative access rights test to wrong_input_test.py --- src/layman/rest_publication_test.py | 44 +-------------- .../wrong_input/wrong_input_test.py | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/layman/rest_publication_test.py b/src/layman/rest_publication_test.py index 9cae2bf51..4d8a952a9 100644 --- a/src/layman/rest_publication_test.py +++ b/src/layman/rest_publication_test.py @@ -1,54 +1,12 @@ import pytest -from layman import LaymanError, settings, common +from layman import settings, common from layman.common.micka import util as micka_util from test_tools import process_client db_schema = settings.LAYMAN_PRIME_SCHEMA -@pytest.mark.parametrize('publ_type', process_client.PUBLICATION_TYPES) -@pytest.mark.usefixtures('ensure_layman') -def test_wrong_post(publ_type): - def check_response(exception): - assert exception.value.http_code == 400 - assert exception.value.code == 43 - assert exception.value.message == 'Wrong access rights.' - - workspace = 'test_wrong_post_workspace' - publication = 'test_wrong_post_publication' - - with pytest.raises(LaymanError) as exc_info: - process_client.publish_workspace_publication(publ_type, workspace, publication, access_rights={'read': 'EVRBODY'}, ) - check_response(exc_info) - - with pytest.raises(LaymanError) as exc_info: - process_client.publish_workspace_publication(publ_type, workspace, publication, access_rights={'write': 'EVRBODY'}, ) - check_response(exc_info) - - with pytest.raises(LaymanError) as exc_info: - process_client.publish_workspace_publication(publ_type, workspace, publication, access_rights={'read': 'EVRBODY', 'write': 'EVRBODY'}, ) - check_response(exc_info) - - process_client.publish_workspace_publication(publ_type, workspace, publication) - - with pytest.raises(LaymanError) as exc_info: - process_client.patch_workspace_publication(publ_type, workspace, publication, access_rights={'read': 'EVRBODY'}, ) - check_response(exc_info) - - with pytest.raises(LaymanError) as exc_info: - process_client.patch_workspace_publication(publ_type, workspace, publication, access_rights={'write': 'EVRBODY'}, ) - check_response(exc_info) - - with pytest.raises(LaymanError) as exc_info: - process_client.patch_workspace_publication(publ_type, workspace, publication, access_rights={'read': 'EVRBODY', 'write': 'EVRBODY'}, ) - check_response(exc_info) - - process_client.patch_workspace_publication(publ_type, workspace, publication) - - process_client.delete_workspace_publication(publ_type, workspace, publication) - - class TestSoapClass: username = 'test_rest_soap_user' publ_name_prefix = 'test_rest_soap_' diff --git a/tests/dynamic_data/publications/wrong_input/wrong_input_test.py b/tests/dynamic_data/publications/wrong_input/wrong_input_test.py index 5333703f4..676ca6399 100644 --- a/tests/dynamic_data/publications/wrong_input/wrong_input_test.py +++ b/tests/dynamic_data/publications/wrong_input/wrong_input_test.py @@ -1391,6 +1391,62 @@ class Key(Enum): Key.RUN_ONLY_CASES: frozenset([RestMethod.PATCH, WithChunksDomain.FALSE, CompressDomain.FALSE]), Key.SPECIFIC_CASES: {}, }, + 'layer_wrong_access_rights_wrong_role': { + Key.PUBLICATION_TYPE: process_client.LAYER_TYPE, + Key.WORKSPACE: OWNER, + Key.POST_BEFORE_TEST_ARGS: { + 'access_rights': { + 'read': OWNER, + 'write': OWNER, + }, + 'actor_name': OWNER, + }, + Key.REST_ARGS: { + 'access_rights': { + 'read': f'{OWNER},EVERYONE,ROLE__WITH_TWO_UNDERSCORES', + 'write': f'{OWNER},EVERYONE,ROLE__WITH_TWO_UNDERSCORES', + }, + 'actor_name': OWNER, + }, + Key.EXCEPTION: LaymanError, + Key.EXPECTED_EXCEPTION: { + 'http_code': 400, + 'sync': True, + 'code': 43, + 'message': 'Wrong access rights.', + }, + Key.MANDATORY_CASES: frozenset([RestMethod.POST, WithChunksDomain.FALSE, CompressDomain.FALSE]), + Key.RUN_ONLY_CASES: frozenset([RestMethod, WithChunksDomain.FALSE, CompressDomain.FALSE]), + Key.SPECIFIC_CASES: {}, + }, + 'map_wrong_access_rights_wrong_role': { + Key.PUBLICATION_TYPE: process_client.MAP_TYPE, + Key.WORKSPACE: OWNER, + Key.POST_BEFORE_TEST_ARGS: { + 'access_rights': { + 'read': OWNER, + 'write': OWNER, + }, + 'actor_name': OWNER, + }, + Key.REST_ARGS: { + 'access_rights': { + 'read': f'{OWNER},EVERYONE,ROLE__WITH_TWO_UNDERSCORES', + 'write': f'{OWNER},EVERYONE,ROLE__WITH_TWO_UNDERSCORES', + }, + 'actor_name': OWNER, + }, + Key.EXCEPTION: LaymanError, + Key.EXPECTED_EXCEPTION: { + 'http_code': 400, + 'sync': True, + 'code': 43, + 'message': 'Wrong access rights.', + }, + Key.MANDATORY_CASES: frozenset([RestMethod.POST, WithChunksDomain.FALSE, CompressDomain.FALSE]), + Key.RUN_ONLY_CASES: frozenset([RestMethod, WithChunksDomain.FALSE, CompressDomain.FALSE]), + Key.SPECIFIC_CASES: {}, + }, }