From 7e40f8baa8b1a929350e9c2c7c07747a8a0145c6 Mon Sep 17 00:00:00 2001 From: index-git Date: Thu, 4 Jan 2024 10:11:50 +0100 Subject: [PATCH 01/10] Positive test for PATCH current user --- test_tools/process.py | 1 + tests/dynamic_data/users_roles/test_current_user.py | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 tests/dynamic_data/users_roles/test_current_user.py diff --git a/test_tools/process.py b/test_tools/process.py index b0229a85c..67d316933 100644 --- a/test_tools/process.py +++ b/test_tools/process.py @@ -68,6 +68,7 @@ def oauth2_provider_mock(): 'test_access_rights_application_reader_by_role': None, 'test_access_rights_application_other_user': None, 'test_patch_after_feature_change_role_user': None, + 'test_patch_current_user_username': None, }, }, 'host': '0.0.0.0', diff --git a/tests/dynamic_data/users_roles/test_current_user.py b/tests/dynamic_data/users_roles/test_current_user.py new file mode 100644 index 000000000..2dd6bccc0 --- /dev/null +++ b/tests/dynamic_data/users_roles/test_current_user.py @@ -0,0 +1,11 @@ +import pytest + +from test_tools import process_client + + +@pytest.mark.usefixtures('ensure_layman_module', 'oauth2_provider_mock') +def test_patch(): + user = 'test_patch_current_user_username' + headers = process_client.get_authz_headers(user) + + process_client.reserve_username(user, headers) From 73ab10fa6d8772cd0a90c639ba69a22abaeacddd Mon Sep 17 00:00:00 2001 From: index-git Date: Thu, 4 Jan 2024 15:20:17 +0100 Subject: [PATCH 02/10] Rewrite test_patch_current_user_anonymous to use process_client --- src/layman/authn/oauth2_test.py | 9 --------- .../users_roles/test_current_user.py | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/layman/authn/oauth2_test.py b/src/layman/authn/oauth2_test.py index a56d98375..7233a6784 100644 --- a/src/layman/authn/oauth2_test.py +++ b/src/layman/authn/oauth2_test.py @@ -217,15 +217,6 @@ def test_get_current_user_anonymous(client): assert claims['nickname'] == 'Anonymous', claims -@pytest.mark.usefixtures('app_context') -def test_patch_current_user_anonymous(client): - rest_path = url_for('rest_current_user.patch') - response = client.patch(rest_path) - assert response.status_code == 403 - resp_json = response.get_json() - assert resp_json['code'] == 30 - - @pytest.mark.usefixtures('active_token_introspection_url', 'user_profile_url', 'ensure_layman') def test_patch_current_user_without_username(): diff --git a/tests/dynamic_data/users_roles/test_current_user.py b/tests/dynamic_data/users_roles/test_current_user.py index 2dd6bccc0..313cde97f 100644 --- a/tests/dynamic_data/users_roles/test_current_user.py +++ b/tests/dynamic_data/users_roles/test_current_user.py @@ -1,6 +1,8 @@ import pytest +from layman import LaymanError from test_tools import process_client +from tests.asserts import processing @pytest.mark.usefixtures('ensure_layman_module', 'oauth2_provider_mock') @@ -9,3 +11,19 @@ def test_patch(): headers = process_client.get_authz_headers(user) process_client.reserve_username(user, headers) + + +@pytest.mark.usefixtures('ensure_layman_module') +@pytest.mark.parametrize('params, exp_exception', [ + pytest.param({ + 'username': 'test_patch_current_user_username' + }, { + 'http_code': 403, + 'code': 30, + 'message': 'Unauthorized access', + }, id='unauthorized'), +]) +def test_patch_current_user_raises(params, exp_exception): + with pytest.raises(LaymanError) as exc_info: + process_client.reserve_username(**params) + processing.exception.response_exception(expected=exp_exception, thrown=exc_info) From 000d6152ec9ad5e3cb09b13c234fbbd25ee49e1c Mon Sep 17 00:00:00 2001 From: index-git Date: Thu, 4 Jan 2024 15:56:46 +0100 Subject: [PATCH 03/10] Restrict new workspace names to 59 characters --- CHANGELOG.md | 1 + src/layman/common/prime_db_schema/__init__.py | 3 +++ src/layman/error_list.py | 1 + test_tools/process.py | 1 + test_tools/process_client.py | 7 ++++++- .../wrong_input/wrong_input_test.py | 17 +++++++++++++++++ .../users_roles/test_current_user.py | 8 ++++++++ 7 files changed, 37 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34b72d87a..b3ec51573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ - [GET](doc/rest.md#get-workspace-map)/[PATCH](doc/rest.md#patch-workspace-map) Workspace Map - GET Workspace [Layers](doc/rest.md#get-workspace-layers)/[Maps](doc/rest.md#get-workspace-maps) - GET [Layers](doc/rest.md#get-layers)/[Maps](doc/rest.md#get-maps)/[Publications](doc/rest.md#get-publications) +- [#165](https://github.com/LayerManager/layman/issues/165) Name of [users](doc/models.md#username) and [public workspaces](doc/models.md#public-workspace) are from now on restricted to 59 characters. - All changes from [v1.22.1](#v1221), [v1.22.2](#v1222) and [v1.22.3](#v1223). - [#960](https://github.com/LayerManager/layman/issues/960) Handle WMS requests with HTTP error more efficiently in timgen. - [#962](https://github.com/LayerManager/layman/issues/962) Make values of `layman_metadata.publication_status` and `status` key(s) more consistent in responses of PATCH Workspace [Layer](doc/rest.md#patch-workspace-layer)/[Map](doc/rest.md#patch-workspace-map) and GET Workspace [Layer](doc/rest.md#get-workspace-layer)/[Map](doc/rest.md#get-workspace-map). diff --git a/src/layman/common/prime_db_schema/__init__.py b/src/layman/common/prime_db_schema/__init__.py index a0e4ac0c3..61d0f2d24 100644 --- a/src/layman/common/prime_db_schema/__init__.py +++ b/src/layman/common/prime_db_schema/__init__.py @@ -1,4 +1,5 @@ from layman.common.prime_db_schema import users as users_util, workspaces as workspaces_util +from layman.http import LaymanError get_usernames = users_util.get_usernames get_workspaces = workspaces_util.get_workspace_names @@ -26,4 +27,6 @@ def ensure_workspace(workspace): def check_workspace_name(workspace): + if len(workspace) > 59: + raise LaymanError(56) workspaces_util.check_workspace_name(workspace) diff --git a/src/layman/error_list.py b/src/layman/error_list.py index 53d08faea..aa175397a 100644 --- a/src/layman/error_list.py +++ b/src/layman/error_list.py @@ -56,4 +56,5 @@ 53: (500, 'Error when publishing on GeoServer. It happens for example for raster files with wrong explicit CRS.'), 54: (400, 'Wrong header value'), 55: (400, 'Publication is not complete'), # raised by process_client only + 56: (403, 'Username or workspace name is too long. Maximum is 59 characters.') } diff --git a/test_tools/process.py b/test_tools/process.py index 67d316933..a52e95075 100644 --- a/test_tools/process.py +++ b/test_tools/process.py @@ -69,6 +69,7 @@ def oauth2_provider_mock(): 'test_access_rights_application_other_user': None, 'test_patch_after_feature_change_role_user': None, 'test_patch_current_user_username': None, + 'test_patch_current_user_username_fail_xxxxxxxxxxxxxxxxxxxxxx': None, }, }, 'host': '0.0.0.0', diff --git a/test_tools/process_client.py b/test_tools/process_client.py index 7d6d7b891..bb40b515a 100644 --- a/test_tools/process_client.py +++ b/test_tools/process_client.py @@ -640,8 +640,13 @@ def get_workspace_publication_metadata_comparison(publication_type, workspace, n get_workspace_map_metadata_comparison = partial(get_workspace_publication_metadata_comparison, MAP_TYPE) -def reserve_username(username, headers=None): +def reserve_username(username, headers=None, *, actor_name=None): headers = headers or {} + if actor_name: + assert TOKEN_HEADER not in headers + + if actor_name and actor_name != settings.ANONYM_USER: + headers.update(get_authz_headers(actor_name)) with app.app_context(): r_url = url_for('rest_current_user.patch') data = { 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 676ca6399..ed7e46ec9 100644 --- a/tests/dynamic_data/publications/wrong_input/wrong_input_test.py +++ b/tests/dynamic_data/publications/wrong_input/wrong_input_test.py @@ -1447,6 +1447,23 @@ class Key(Enum): Key.RUN_ONLY_CASES: frozenset([RestMethod, WithChunksDomain.FALSE, CompressDomain.FALSE]), Key.SPECIFIC_CASES: {}, }, + 'workspace_too_long': { + Key.PUBLICATION_TYPE: process_client.LAYER_TYPE, + Key.WORKSPACE: WORKSPACE.ljust(60, 'x'), + Key.REST_ARGS: { + 'name': 'workspace_too_long_layer', + }, + Key.EXCEPTION: LaymanError, + Key.FAILED_INFO_KEY: 'file', + Key.EXPECTED_EXCEPTION: { + 'http_code': 403, + 'sync': True, + 'code': 56, + }, + Key.MANDATORY_CASES: None, + Key.RUN_ONLY_CASES: frozenset([RestMethod.POST, WithChunksDomain.FALSE, CompressDomain.FALSE]), + Key.SPECIFIC_CASES: {}, + }, } diff --git a/tests/dynamic_data/users_roles/test_current_user.py b/tests/dynamic_data/users_roles/test_current_user.py index 313cde97f..1154333a1 100644 --- a/tests/dynamic_data/users_roles/test_current_user.py +++ b/tests/dynamic_data/users_roles/test_current_user.py @@ -22,6 +22,14 @@ def test_patch(): 'code': 30, 'message': 'Unauthorized access', }, id='unauthorized'), + pytest.param({ + 'username': 'test_patch_current_user_username_fail_'.ljust(60, 'x'), + 'actor_name': 'test_patch_current_user_username_fail_'.ljust(60, 'x') + }, { + 'http_code': 403, + 'code': 56, + 'message': 'Username or workspace name is too long. Maximum is 59 characters.', + }, id='too_long'), ]) def test_patch_current_user_raises(params, exp_exception): with pytest.raises(LaymanError) as exc_info: From a4386ef4cc6dff92bc1baede1bc522abc13f612a Mon Sep 17 00:00:00 2001 From: index-git Date: Fri, 5 Jan 2024 12:16:38 +0100 Subject: [PATCH 04/10] Alter workspace.name column to varchar(59) --- CHANGELOG.md | 3 ++- src/layman/upgrade/__init__.py | 1 + src/layman/upgrade/upgrade_v1_23.py | 23 +++++++++++++++++++++++ src/layman/upgrade/upgrade_v1_23_test.py | 24 ++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3ec51573..4731dd82a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ #### Schema migrations - [#165](https://github.com/LayerManager/layman/issues/165) Add column `role_name` to table `rights` in prime DB schema. Add constraint that exactly one of columns `role_name` and `id_user` is not null. - [#165](https://github.com/LayerManager/layman/issues/165) Create DB schema `_role_service` that can be used as [role service](doc/security.md#role-service). +- [#165](https://github.com/LayerManager/layman/issues/165) Column `name` in table `workspace` in prime DB schema length is changed to 59 characters. #### Data migrations - [#165](https://github.com/LayerManager/layman/issues/165) Delete technical roles and user-role relations in GeoServer `default` role service, which is now replaced by JDBC role service. ### Changes @@ -36,7 +37,7 @@ - [GET](doc/rest.md#get-workspace-map)/[PATCH](doc/rest.md#patch-workspace-map) Workspace Map - GET Workspace [Layers](doc/rest.md#get-workspace-layers)/[Maps](doc/rest.md#get-workspace-maps) - GET [Layers](doc/rest.md#get-layers)/[Maps](doc/rest.md#get-maps)/[Publications](doc/rest.md#get-publications) -- [#165](https://github.com/LayerManager/layman/issues/165) Name of [users](doc/models.md#username) and [public workspaces](doc/models.md#public-workspace) are from now on restricted to 59 characters. +- [#165](https://github.com/LayerManager/layman/issues/165) Name of [users](doc/models.md#username) and [public workspaces](doc/models.md#public-workspace) are from now on restricted to a maximum length of 59 characters. - All changes from [v1.22.1](#v1221), [v1.22.2](#v1222) and [v1.22.3](#v1223). - [#960](https://github.com/LayerManager/layman/issues/960) Handle WMS requests with HTTP error more efficiently in timgen. - [#962](https://github.com/LayerManager/layman/issues/962) Make values of `layman_metadata.publication_status` and `status` key(s) more consistent in responses of PATCH Workspace [Layer](doc/rest.md#patch-workspace-layer)/[Map](doc/rest.md#patch-workspace-map) and GET Workspace [Layer](doc/rest.md#get-workspace-layer)/[Map](doc/rest.md#get-workspace-map). diff --git a/src/layman/upgrade/__init__.py b/src/layman/upgrade/__init__.py index 6f9df0eac..5854f1282 100644 --- a/src/layman/upgrade/__init__.py +++ b/src/layman/upgrade/__init__.py @@ -44,6 +44,7 @@ ((1, 23, 0), [ upgrade_v1_23.adjust_db_for_roles, upgrade_v1_23.create_role_service_schema, + upgrade_v1_23.restrict_workspace_name_length, ]), ], consts.MIGRATION_TYPE_DATA: [ diff --git a/src/layman/upgrade/upgrade_v1_23.py b/src/layman/upgrade/upgrade_v1_23.py index 9e084add4..f8cd7ae4a 100644 --- a/src/layman/upgrade/upgrade_v1_23.py +++ b/src/layman/upgrade/upgrade_v1_23.py @@ -168,3 +168,26 @@ def delete_user_roles(): role_not_exists = response.status_code == 404 if not role_not_exists: response.raise_for_status() + + +def restrict_workspace_name_length(): + logger.info(f' Restrict workspace name length') + + select_too_long = f""" +select name +from {settings.LAYMAN_PRIME_SCHEMA}.workspaces +where length(name) > 59 +;""" + too_long_workspace_name = db_util.run_query(select_too_long) + if len(too_long_workspace_name) > 0: + raise NotImplementedError(f"Too long workspace names: {[name[0] for name in too_long_workspace_name]}") + + # For direct ALTER TABLE raises "ERROR: cannot alter type of a column used by a view or rule" + # For details see https://web.archive.org/web/20111007112138/http://sniptools.com/databases/resize-a-column-in-a-postgresql-table-without-changing-data + alter_column = f""" +UPDATE pg_attribute SET + atttypmod = 59+4 +WHERE attrelid = '{settings.LAYMAN_PRIME_SCHEMA}.workspaces'::regclass +AND attname = 'name' +;""" + db_util.run_statement(alter_column) diff --git a/src/layman/upgrade/upgrade_v1_23_test.py b/src/layman/upgrade/upgrade_v1_23_test.py index 7451434b6..69bafa8e9 100644 --- a/src/layman/upgrade/upgrade_v1_23_test.py +++ b/src/layman/upgrade/upgrade_v1_23_test.py @@ -143,3 +143,27 @@ def test_create_role_service_schema(): assert result == 1 result = db_util.run_query(table_existence_query, ('group_roles',))[0][0] assert result == 1 + + +def test_restrict_username_length(): + too_long_workspace_name = 'test_restrict_username_length'.ljust(60, 'x') + alter_column = f""" + UPDATE pg_attribute SET + atttypmod = 1043 + WHERE attrelid = '{settings.LAYMAN_PRIME_SCHEMA}.workspaces'::regclass + AND attname = 'name' + ;""" + db_util.run_statement(alter_column) + + insert_statement = f"""INSERT INTO {settings.LAYMAN_PRIME_SCHEMA}.workspaces (name) values ('{too_long_workspace_name}')""" + db_util.run_statement(insert_statement) + + with app.app_context(): + with pytest.raises(NotImplementedError): + upgrade_v1_23.restrict_workspace_name_length() + + delete_statement = f"""DELETE FROM {settings.LAYMAN_PRIME_SCHEMA}.workspaces WHERE name = '{too_long_workspace_name}'""" + db_util.run_statement(delete_statement) + + with app.app_context(): + upgrade_v1_23.restrict_workspace_name_length() From f59ac5e2b24f7c69bcfc8893b3d23bfa0e17728d Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Mon, 8 Jan 2024 10:51:19 +0100 Subject: [PATCH 05/10] Use ALTER TABLE instead of changing atttypmod --- src/layman/upgrade/__init__.py | 2 +- src/layman/upgrade/upgrade_v1_23.py | 8 +--- src/layman/upgrade/upgrade_v1_23_test.py | 59 ++++++++++++++---------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/layman/upgrade/__init__.py b/src/layman/upgrade/__init__.py index 5854f1282..3ec62d887 100644 --- a/src/layman/upgrade/__init__.py +++ b/src/layman/upgrade/__init__.py @@ -43,8 +43,8 @@ ]), ((1, 23, 0), [ upgrade_v1_23.adjust_db_for_roles, - upgrade_v1_23.create_role_service_schema, upgrade_v1_23.restrict_workspace_name_length, + upgrade_v1_23.create_role_service_schema, ]), ], consts.MIGRATION_TYPE_DATA: [ diff --git a/src/layman/upgrade/upgrade_v1_23.py b/src/layman/upgrade/upgrade_v1_23.py index f8cd7ae4a..f4c06b0e5 100644 --- a/src/layman/upgrade/upgrade_v1_23.py +++ b/src/layman/upgrade/upgrade_v1_23.py @@ -182,12 +182,8 @@ def restrict_workspace_name_length(): if len(too_long_workspace_name) > 0: raise NotImplementedError(f"Too long workspace names: {[name[0] for name in too_long_workspace_name]}") - # For direct ALTER TABLE raises "ERROR: cannot alter type of a column used by a view or rule" - # For details see https://web.archive.org/web/20111007112138/http://sniptools.com/databases/resize-a-column-in-a-postgresql-table-without-changing-data alter_column = f""" -UPDATE pg_attribute SET - atttypmod = 59+4 -WHERE attrelid = '{settings.LAYMAN_PRIME_SCHEMA}.workspaces'::regclass -AND attname = 'name' +ALTER TABLE {settings.LAYMAN_PRIME_SCHEMA}.workspaces + ALTER COLUMN name TYPE VARCHAR(59) COLLATE pg_catalog."default" ;""" db_util.run_statement(alter_column) diff --git a/src/layman/upgrade/upgrade_v1_23_test.py b/src/layman/upgrade/upgrade_v1_23_test.py index 69bafa8e9..03b9eb7b7 100644 --- a/src/layman/upgrade/upgrade_v1_23_test.py +++ b/src/layman/upgrade/upgrade_v1_23_test.py @@ -62,6 +62,33 @@ def test_adjust_db_for_roles(): assert rights_rows[0][2] is None, f"role_name is not none!" +def drop_role_schema(): + drop_statement = f'''DROP SCHEMA IF EXISTS {ROLE_SERVICE_SCHEMA} CASCADE;''' + db_util.run_statement(drop_statement) + + +def create_simple_role_schema(): + # prepare simple schema in the same way as in setup_geoserver.py + + prepare_simple_schema_statement = f""" +CREATE SCHEMA "{ROLE_SERVICE_SCHEMA}" AUTHORIZATION {settings.LAYMAN_PG_USER}; +create view {ROLE_SERVICE_SCHEMA}.roles as select 'ADMIN'::varchar(64) as name, null::varchar(64) as parent +union all select 'GROUP_ADMIN', null +union all select %s, null +; +create view {ROLE_SERVICE_SCHEMA}.role_props as select null::varchar(64) as rolename, null::varchar(64) as propname, null::varchar(2048) as propvalue; +create view {ROLE_SERVICE_SCHEMA}.user_roles as select %s::varchar(64) as username, 'ADMIN'::varchar(64) as rolename +union all select %s, %s +union all select %s, 'ADMIN' +; +create view {ROLE_SERVICE_SCHEMA}.group_roles as select null::varchar(128) as groupname, null::varchar(64) as rolename; + """ + + db_util.run_statement(prepare_simple_schema_statement, data=( + settings.LAYMAN_GS_ROLE, settings.LAYMAN_GS_USER, settings.LAYMAN_GS_USER, settings.LAYMAN_GS_ROLE, + settings.GEOSERVER_ADMIN_USER)) + + def test_create_role_service_schema(): username = 'test_create_role_service_schema_username' rolename = f'USER_{username.upper()}' @@ -75,7 +102,6 @@ def test_create_role_service_schema(): "middle_name": "ensure", } } - drop_statement = f'''DROP SCHEMA IF EXISTS {ROLE_SERVICE_SCHEMA} CASCADE;''' schema_existence_query = f'''SELECT COUNT(*) FROM information_schema.schemata WHERE schema_name = '{ROLE_SERVICE_SCHEMA}';''' table_existence_query = f'''SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = '{ROLE_SERVICE_SCHEMA}' and table_name = %s;''' layman_users_roles_query = f'''select COUNT(*) from {ROLE_SERVICE_SCHEMA}.layman_users_roles where name = %s''' @@ -93,30 +119,13 @@ def test_create_role_service_schema(): (select count(*) from {ROLE_SERVICE_SCHEMA}.admin_user_roles) admin_user_roles, (select count(*) from {ROLE_SERVICE_SCHEMA}.user_roles) user_roles''' - # prepare simple schema in the same way as in setup_geoserver.py - prepare_simple_schema_statement = f""" -CREATE SCHEMA "{ROLE_SERVICE_SCHEMA}" AUTHORIZATION {settings.LAYMAN_PG_USER}; -create view {ROLE_SERVICE_SCHEMA}.roles as select 'ADMIN'::varchar(64) as name, null::varchar(64) as parent -union all select 'GROUP_ADMIN', null -union all select %s, null -; -create view {ROLE_SERVICE_SCHEMA}.role_props as select null::varchar(64) as rolename, null::varchar(64) as propname, null::varchar(2048) as propvalue; -create view {ROLE_SERVICE_SCHEMA}.user_roles as select %s::varchar(64) as username, 'ADMIN'::varchar(64) as rolename -union all select %s, %s -union all select %s, 'ADMIN' -; -create view {ROLE_SERVICE_SCHEMA}.group_roles as select null::varchar(128) as groupname, null::varchar(64) as rolename; - """ + drop_role_schema() with app.app_context(): ensure_whole_user(username, userinfo) - db_util.run_statement(drop_statement) result = db_util.run_query(schema_existence_query)[0][0] assert result == 0 - db_util.run_statement(prepare_simple_schema_statement, data=( - settings.LAYMAN_GS_ROLE, settings.LAYMAN_GS_USER, settings.LAYMAN_GS_USER, settings.LAYMAN_GS_ROLE, - settings.GEOSERVER_ADMIN_USER)) - + create_simple_role_schema() upgrade_v1_23.create_role_service_schema() result = db_util.run_query(schema_existence_query)[0][0] @@ -147,11 +156,11 @@ def test_create_role_service_schema(): def test_restrict_username_length(): too_long_workspace_name = 'test_restrict_username_length'.ljust(60, 'x') + drop_role_schema() + alter_column = f""" - UPDATE pg_attribute SET - atttypmod = 1043 - WHERE attrelid = '{settings.LAYMAN_PRIME_SCHEMA}.workspaces'::regclass - AND attname = 'name' +ALTER TABLE {settings.LAYMAN_PRIME_SCHEMA}.workspaces + ALTER COLUMN name TYPE VARCHAR(256) COLLATE pg_catalog."default" ;""" db_util.run_statement(alter_column) @@ -167,3 +176,5 @@ def test_restrict_username_length(): with app.app_context(): upgrade_v1_23.restrict_workspace_name_length() + create_simple_role_schema() + upgrade_v1_23.create_role_service_schema() From 191c0d0b3fda448aa0b01dc3c0185e621247f931 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Mon, 8 Jan 2024 17:14:35 +0100 Subject: [PATCH 06/10] Document workspace-name length restriction --- doc/models.md | 3 ++- doc/rest.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/models.md b/doc/models.md index 46d9dffec..7b28277d8 100644 --- a/doc/models.md +++ b/doc/models.md @@ -73,7 +73,7 @@ ## Username - Username is a string identifying one [user](#user), so it is unique among all users. -- The string is lower-case (in contrast with [role name](#role)). +- The string is lower-case (in contrast with [role name](#role)), maximum length is 59 characters. - Each user is represented by max. one username. - Username is also used to identify user's [personal workspace](#personal-workspace) when communicating with [Layman REST API](rest.md). - Username can be reserved by [PATCH Current User](rest.md#patch-current-user). @@ -92,6 +92,7 @@ ## Workspace - Workspace is folder for [publications](#publication). - Each workspace is identified by name that is unique among all workspaces. +- The name is lower-case, maximum length is 59 characters. - Workspace name is sometimes used for structuring publication-related data. For example, it's part of REST API URL (`/rest/workspaces//...`), directory names (`/workspaces//...`), DB schemas, or OGC Web Services (`/geoserver//...`, `/geoserver/_wms/...`). - Workspace's REST API consists of all [map and layer endpoints](rest.md) endpoints. - There are following types of workspaces: diff --git a/doc/rest.md b/doc/rest.md index c8b9ee680..b910fb9b7 100644 --- a/doc/rest.md +++ b/doc/rest.md @@ -834,7 +834,7 @@ Query parameters: - `true`: If `username` sent in body parameter is already reserved by another user or `username` is an empty string, layman will definitely reserve some `username`, preferably similar to the value sent in `username` body parameter or to one of claims. Body parameters: -- *username*: String. [Username](models.md#username) that should be reserved for current user. Username can be reserved only once and it cannot be changed. See URL parameter `adjust_username` for other details. +- *username*: String. [Username](models.md#username) that should be reserved for current user (maximum length is 59 characters). Username can be reserved only once and cannot be changed. See URL parameter `adjust_username` for other details. #### Response Content-Type: `application/json` From 2693069d0c38751e389bf5ee6042052d884c8ac8 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Wed, 10 Jan 2024 10:12:49 +0100 Subject: [PATCH 07/10] Make wagtail DB persistent --- CHANGELOG.md | 1 + Makefile | 13 +++++++++---- deps/wagtail/docker/Dockerfile | 10 ++++------ .../laymanportal/laymanportal/settings/base.py | 2 +- deps/wagtail/laymanportal/start_wagtail.sh | 10 ++++++++++ deps/wagtail/{laymanportal => sample}/db.sqlite3 | Bin docker-compose.deps.yml | 4 ++++ 7 files changed, 29 insertions(+), 11 deletions(-) create mode 100755 deps/wagtail/laymanportal/start_wagtail.sh rename deps/wagtail/{laymanportal => sample}/db.sqlite3 (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4731dd82a..7183834a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ - GET Workspace [Layers](doc/rest.md#get-workspace-layers)/[Maps](doc/rest.md#get-workspace-maps) - GET [Layers](doc/rest.md#get-layers)/[Maps](doc/rest.md#get-maps)/[Publications](doc/rest.md#get-publications) - [#165](https://github.com/LayerManager/layman/issues/165) Name of [users](doc/models.md#username) and [public workspaces](doc/models.md#public-workspace) are from now on restricted to a maximum length of 59 characters. +- [941](https://github.com/LayerManager/layman/issues/941) Wagtail database is now persistent when restarting Layman or Wagtail. - All changes from [v1.22.1](#v1221), [v1.22.2](#v1222) and [v1.22.3](#v1223). - [#960](https://github.com/LayerManager/layman/issues/960) Handle WMS requests with HTTP error more efficiently in timgen. - [#962](https://github.com/LayerManager/layman/issues/962) Make values of `layman_metadata.publication_status` and `status` key(s) more consistent in responses of PATCH Workspace [Layer](doc/rest.md#patch-workspace-layer)/[Map](doc/rest.md#patch-workspace-map) and GET Workspace [Layer](doc/rest.md#get-workspace-layer)/[Map](doc/rest.md#get-workspace-map). diff --git a/Makefile b/Makefile index 7739fd322..f673ab8a0 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ start-demo-only: docker compose -f docker-compose.deps.demo.yml -f docker-compose.demo.yml up -d --force-recreate --no-deps layman celery_worker flower timgen layman_client nginx start-demo-full-with-optional-deps: - mkdir -p layman_data deps/qgis/data + mkdir -p layman_data deps/qgis/data deps/wagtail/data docker compose -f docker-compose.deps.demo.yml -f docker-compose.demo.yml build layman layman_client timgen docker compose -f docker-compose.deps.demo.yml -f docker-compose.demo.yml up -d postgresql docker compose -f docker-compose.deps.demo.yml -f docker-compose.demo.yml run --rm --no-deps -u root layman bash -c "cd src && python3 -B setup_geoserver.py" @@ -60,7 +60,7 @@ deps-stop: docker compose -f docker-compose.deps.yml stop start-dev: - mkdir -p layman_data layman_data_test tmp deps/qgis/data + mkdir -p layman_data layman_data_test tmp deps/qgis/data deps/wagtail/data docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml up -d postgresql docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml run --rm --no-deps -u root layman_dev bash -c "cd src && python3 -B setup_geoserver.py" docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml up --force-recreate -d @@ -107,7 +107,7 @@ upgrade-dev: docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml run --rm --no-deps layman_dev bash -c "cd src && python3 layman_flush_redis.py && python3 wait_for_deps.py && python3 standalone_upgrade.py" prepare-dirs: - mkdir -p layman_data layman_data_test tmp deps/qgis/data + mkdir -p layman_data layman_data_test tmp deps/qgis/data deps/wagtail/data build-dev: docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml build --force-rm layman_dev @@ -143,7 +143,7 @@ reset-data-directories: docker compose -f docker-compose.deps.yml rm -fsv docker volume rm layman_redis-data || true sudo rm -rf layman_data layman_data_test deps/*/data - mkdir -p layman_data layman_data_test tmp deps/qgis/data + mkdir -p layman_data layman_data_test tmp deps/qgis/data deps/wagtail/data clear-python-cache-dev: docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml run --rm --no-deps layman_dev bash /code/src/clear-python-cache.sh @@ -313,11 +313,16 @@ wagtail-exec: docker compose -f docker-compose.deps.yml exec wagtail bash wagtail-restart: + mkdir -p deps/wagtail/data docker compose -f docker-compose.deps.yml up --force-recreate --no-deps -d wagtail wagtail-stop: docker compose -f docker-compose.deps.yml stop wagtail +wagtail-reset-datadir: + mkdir -p deps/wagtail/data + rm -rf deps/wagtail/data/* + micka-restart: docker compose -f docker-compose.deps.yml up --force-recreate --no-deps -d micka diff --git a/deps/wagtail/docker/Dockerfile b/deps/wagtail/docker/Dockerfile index bdbbdc1e0..c209b1ba2 100644 --- a/deps/wagtail/docker/Dockerfile +++ b/deps/wagtail/docker/Dockerfile @@ -21,15 +21,13 @@ RUN pip install -r /app/requirements.txt # Copy the entire project code. COPY laymanportal /app/ -# Prepare the app. -RUN python manage.py migrate +# Prepare the app: migrations (not needed as db.sqlite3a was generated after migrations) +# RUN python manage.py migrate + +# Prepare the app: collect statis files RUN python manage.py collectstatic --noinput # Create a "coderedcms" user account to run the app. RUN useradd coderedcms RUN chown -R coderedcms /app/ USER coderedcms - -# Finally, run the app on port 8000. -EXPOSE 8000 -CMD exec waitress-serve --listen "*:8000" "laymanportal.wsgi:application" diff --git a/deps/wagtail/laymanportal/laymanportal/settings/base.py b/deps/wagtail/laymanportal/laymanportal/settings/base.py index de5ae3dcf..e4d317b1c 100644 --- a/deps/wagtail/laymanportal/laymanportal/settings/base.py +++ b/deps/wagtail/laymanportal/laymanportal/settings/base.py @@ -125,7 +125,7 @@ DATABASES = { "default": { "ENGINE": "django.db.backends.sqlite3", - "NAME": os.path.join(BASE_DIR, "db.sqlite3"), + "NAME": os.path.join(BASE_DIR, "data/db.sqlite3"), } } diff --git a/deps/wagtail/laymanportal/start_wagtail.sh b/deps/wagtail/laymanportal/start_wagtail.sh new file mode 100755 index 000000000..17c7f2782 --- /dev/null +++ b/deps/wagtail/laymanportal/start_wagtail.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +set -ex + +if [ ! -f /app/data/db.sqlite3 ]; then + echo "File db.sqlite3 not found, copying the default one." + cp /app/initial_data/db.sqlite3 /app/data/ +fi + +exec waitress-serve --listen "*:8000" "laymanportal.wsgi:application" diff --git a/deps/wagtail/laymanportal/db.sqlite3 b/deps/wagtail/sample/db.sqlite3 similarity index 100% rename from deps/wagtail/laymanportal/db.sqlite3 rename to deps/wagtail/sample/db.sqlite3 diff --git a/docker-compose.deps.yml b/docker-compose.deps.yml index e7539644f..7fc763753 100644 --- a/docker-compose.deps.yml +++ b/docker-compose.deps.yml @@ -67,6 +67,10 @@ services: dockerfile: docker/Dockerfile ports: - 8083:8000 + command: /app/start_wagtail.sh + volumes: + - ./deps/wagtail/data:/app/data/ + - ./deps/wagtail/sample:/app/initial_data/ micka: container_name: micka From edea3901593dd84f38460f6f54b1ab96503f9999 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Wed, 10 Jan 2024 11:57:47 +0100 Subject: [PATCH 08/10] Enable to use userId as OAuth2 sub --- .env.dev | 3 ++- CHANGELOG.md | 10 ++++++++++ Makefile | 3 +++ doc/env-settings.md | 5 ++++- src/layman/authn/oauth2/__init__.py | 8 ++++++++ src/layman_settings.py | 1 + 6 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.env.dev b/.env.dev index 603dbc27f..4f5f4232a 100644 --- a/.env.dev +++ b/.env.dev @@ -91,7 +91,8 @@ OAUTH2_AUTH_URL=http://localhost:8083/o/authorize OAUTH2_TOKEN_URL=http://wagtail:8000/o/token/ OAUTH2_CALLBACK_URL=http://localhost:3000/client/authn/oauth2-provider/callback OAUTH2_INTROSPECTION_URL=http://wagtail:8000/o/introspect/ -OAUTH2_INTROSPECTION_SUB_KEY=username +OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE=true +OAUTH2_INTROSPECTION_SUB_KEY=userId OAUTH2_USER_PROFILE_URL=http://wagtail:8000/profile diff --git a/CHANGELOG.md b/CHANGELOG.md index 7183834a6..e2792cefa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ ``` - Stop using environment variable `LAYMAN_GS_ROLE_SERVICE`, it has no effect to Layman anymore. Layman now uses [role service](doc/security.md#role-service) identified by new environment variable [LAYMAN_ROLE_SERVICE_URI](doc/env-settings.md#LAYMAN_ROLE_SERVICE_URI). The service is called `layman_role_service` on GeoServer. - Set new environment variable [LAYMAN_ROLE_SERVICE_URI](doc/env-settings.md#LAYMAN_ROLE_SERVICE_URI) +- If you are using Wagtail as OAuth2 provider + - Set new environment variable [OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE](doc/env-settings.md#OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE): + ``` + OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE=true + ``` + - Change environment variable [OAUTH2_INTROSPECTION_SUB_KEY](doc/env-settings.md#OAUTH2_INTROSPECTION_SUB_KEY): + ``` + OAUTH2_INTROSPECTION_SUB_KEY=userId + ``` ### Migrations and checks #### Schema migrations - [#165](https://github.com/LayerManager/layman/issues/165) Add column `role_name` to table `rights` in prime DB schema. Add constraint that exactly one of columns `role_name` and `id_user` is not null. @@ -38,6 +47,7 @@ - GET Workspace [Layers](doc/rest.md#get-workspace-layers)/[Maps](doc/rest.md#get-workspace-maps) - GET [Layers](doc/rest.md#get-layers)/[Maps](doc/rest.md#get-maps)/[Publications](doc/rest.md#get-publications) - [#165](https://github.com/LayerManager/layman/issues/165) Name of [users](doc/models.md#username) and [public workspaces](doc/models.md#public-workspace) are from now on restricted to a maximum length of 59 characters. +- [940](https://github.com/LayerManager/layman/issues/940) Enable to use `userId` as OAuth2 "sub" instead of `username`. This is recommended option for Wagtail. See [OAUTH2_INTROSPECTION_SUB_KEY](doc/env-settings.md#OAUTH2_INTROSPECTION_SUB_KEY) for more details. - [941](https://github.com/LayerManager/layman/issues/941) Wagtail database is now persistent when restarting Layman or Wagtail. - All changes from [v1.22.1](#v1221), [v1.22.2](#v1222) and [v1.22.3](#v1223). - [#960](https://github.com/LayerManager/layman/issues/960) Handle WMS requests with HTTP error more efficiently in timgen. diff --git a/Makefile b/Makefile index f673ab8a0..45f0600cf 100644 --- a/Makefile +++ b/Makefile @@ -271,6 +271,9 @@ postgresql-psql: postgresql-psql-test: docker compose -f docker-compose.deps.yml run -e PGPASSWORD=docker --entrypoint "psql -U docker -p 5432 -h postgresql layman_test" --rm postgresql +postgresql-bash-exec: + docker compose -f docker-compose.deps.yml exec -e PGPASSWORD=docker postgresql "bash" + redis-cli-db: docker compose -f docker-compose.deps.yml exec redis redis-cli -h redis -p 6379 -n 0 diff --git a/doc/env-settings.md b/doc/env-settings.md index f21cfba7c..b9795e75b 100644 --- a/doc/env-settings.md +++ b/doc/env-settings.md @@ -85,7 +85,10 @@ URL of LTC OAuth2 callback endpoint to be called after successful OAuth2 authori URL of OAuth2 Introspection endpoint. ### OAUTH2_INTROSPECTION_SUB_KEY -Name of the key in OAuth2 introspection response whose value is OAuth2 subject (also known as "sub"). Value `username` is suitable for Wagtail. If not set or set to empty string, `sub` is used, that is suitable option for Liferay. +Name of the key in OAuth2 introspection response whose value is OAuth2 subject (also known as "sub"). Value `userId` is suitable for Wagtail (together with setting [OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE](#OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE) to `true`). If not set or set to empty string, `sub` is used, that is suitable option for Liferay. + +### OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE +Set to `true` if you want [OAUTH2_INTROSPECTION_SUB_KEY](#OAUTH2_INTROSPECTION_SUB_KEY) to be read from [OAUTH2_USER_PROFILE_URL](#OAUTH2_USER_PROFILE_URL) instead of [OAUTH2_INTROSPECTION_URL](#OAUTH2_INTROSPECTION_URL). Default value is `false`. Value `true` is suitable for Wagtail. ### OAUTH2_USER_PROFILE_URL URL of User Profile endpoint used to obtain user's ID, name, email, etc. diff --git a/src/layman/authn/oauth2/__init__.py b/src/layman/authn/oauth2/__init__.py index cc870adf1..99909833a 100644 --- a/src/layman/authn/oauth2/__init__.py +++ b/src/layman/authn/oauth2/__init__.py @@ -72,6 +72,14 @@ def authenticate(): # current_app.logger.info(f"r_json={r_json}") if r_json['active'] is True and r_json.get('token_type', 'Bearer') == 'Bearer': valid_resp = r_json + if settings.OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE: + response = requests.get(USER_PROFILE_URL, headers={ + 'Authorization': f'Bearer {access_token}', + }, timeout=settings.DEFAULT_CONNECTION_TIMEOUT) + response.raise_for_status() + user_profile_json = response.json() + # current_app.logger.info(f"user_profile_json={user_profile_json}") + valid_resp[INTROSPECTION_SUB_KEY] = user_profile_json[INTROSPECTION_SUB_KEY] break except ValueError: continue diff --git a/src/layman_settings.py b/src/layman_settings.py index 40ef8f416..3ec3515bf 100644 --- a/src/layman_settings.py +++ b/src/layman_settings.py @@ -216,6 +216,7 @@ class EnumWfsWmsStatus(Enum): OAUTH2_INTROSPECTION_URL = os.getenv('OAUTH2_INTROSPECTION_URL', None) OAUTH2_INTROSPECTION_SUB_KEY = os.getenv('OAUTH2_INTROSPECTION_SUB_KEY') or 'sub' OAUTH2_USER_PROFILE_URL = os.getenv('OAUTH2_USER_PROFILE_URL', None) +OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE = os.getenv('OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE', 'false') == 'true' OAUTH2_CLIENTS = [ d for d in read_clients_dict_from_env() if len(d['id']) > 0 From a6a6d87fa4960a27240cf27bb1736dc78dd92d24 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Wed, 10 Jan 2024 15:01:22 +0100 Subject: [PATCH 09/10] Add script for migrating OAuth2 sub --- CHANGELOG.md | 6 +++ ...3_change_oauth2_sub_username_to_user_id.py | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/v1_23_change_oauth2_sub_username_to_user_id.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e2792cefa..0c5a2f187 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ ``` OAUTH2_INTROSPECTION_SUB_KEY=userId ``` + - After running `make upgrade-demo` or `make-upgrade-demo-full`, run also script `v1_23_change_oauth2_sub_username_to_user_id.py`: + ```bash + docker compose -f docker-compose.deps.demo.yml -f docker-compose.demo.yml run --rm --no-deps -u root -e LAYMAN_WAGTAIL_DB_URI= layman bash -c "cd src && python3 -B v1_23_change_oauth2_sub_username_to_user_id.py" + ``` + - `URI_of_Wagtail_db` is PostgreSQL connection URI to Wagtail database, e.g. `postgresql://user:password@host.docker.internal:5432/wagtail_db_name` + - The script changes OAuth2 "sub" values in Layman prime DB schema from Wagtail usernames to Wagtail user IDs. See [940](https://github.com/LayerManager/layman/issues/940). ### Migrations and checks #### Schema migrations - [#165](https://github.com/LayerManager/layman/issues/165) Add column `role_name` to table `rights` in prime DB schema. Add constraint that exactly one of columns `role_name` and `id_user` is not null. diff --git a/src/v1_23_change_oauth2_sub_username_to_user_id.py b/src/v1_23_change_oauth2_sub_username_to_user_id.py new file mode 100644 index 000000000..be39ebd8a --- /dev/null +++ b/src/v1_23_change_oauth2_sub_username_to_user_id.py @@ -0,0 +1,54 @@ +import os +import layman_settings as settings +from db import util as db_util + + +def main(): + assert settings.OAUTH2_INTROSPECTION_SUB_KEY == 'userId', f"OAUTH2_INTROSPECTION_SUB_KEY is expected to be `userId`" + assert settings.OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE is True, f"OAUTH2_INTROSPECTION_USE_SUB_KEY_FROM_USER_PROFILE is expected to be `true`" + + wagtail_db_uri = os.getenv('LAYMAN_WAGTAIL_DB_URI', None) + assert wagtail_db_uri is not None, f"LAYMAN_WAGTAIL_DB_URI must be set" + + wagtail_user_rows = db_util.run_query(f"select id, username from auth_user;", uri_str=wagtail_db_uri) + assert len(set(r[0] for r in wagtail_user_rows)) == len(wagtail_user_rows), f"Wagtail userIds are expected to be unique" + assert len(set(r[1] for r in wagtail_user_rows)) == len(wagtail_user_rows), f"Wagtail usernames are expected to be unique" + wagtail_username_to_id = { + username: f"{user_id}" + for user_id, username in wagtail_user_rows + } + + layman_user_rows = db_util.run_query(f""" +select u.id, u.issuer_id, u.sub, w.name as username +from {settings.LAYMAN_PRIME_SCHEMA}.users u + inner join {settings.LAYMAN_PRIME_SCHEMA}.workspaces w on u.id_workspace = w.id + """, uri_str=settings.PG_URI_STR) + + print(f"Found {len(wagtail_user_rows)} Wagtail users:") + for user_id, username in wagtail_user_rows: + print(f" {username}, id={user_id}") + + print(f"Found {len(layman_user_rows)} Layman users with username registered.") + print(f'Processing Layman users ...') + + changed_subs = 0 + for layman_user_id, issuer_id, sub, username in layman_user_rows: + print(f" {username} (id={layman_user_id}, sub={sub}, issuer_id={issuer_id})") + if issuer_id != 'layman.authn.oauth2': + print(f" WARNING: User has unexpected issuer_id, skipping.") + continue + new_sub = wagtail_username_to_id.get(sub) + if new_sub is None: + print(f" WARNING: Sub of the user was not found among Wagtail usernames, skipping.") + continue + print(f' Changing sub from `{sub}` to `{new_sub}`') + db_util.run_statement( + f"UPDATE {settings.LAYMAN_PRIME_SCHEMA}.users set sub = %s where id = %s", + data=(new_sub, layman_user_id), uri_str=settings.PG_URI_STR) + changed_subs += 1 + print(f' Changed!') + print(f'Processing finished, changed {changed_subs} OAuth2 subs of {len(layman_user_rows)} Layman users.') + + +if __name__ == '__main__': + main() From 0ecf7afa5aa64e7ec48b73deb2e5b3527b832374 Mon Sep 17 00:00:00 2001 From: Jiri Kozel Date: Wed, 10 Jan 2024 15:10:11 +0100 Subject: [PATCH 10/10] Enable to use SQLite3 in v1_23_change_oauth2_sub_username_to_user_id.py --- CHANGELOG.md | 4 ++++ docker-compose.dev.yml | 1 + src/db/util.py | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c5a2f187..c2db4ea62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ ``` - `URI_of_Wagtail_db` is PostgreSQL connection URI to Wagtail database, e.g. `postgresql://user:password@host.docker.internal:5432/wagtail_db_name` - The script changes OAuth2 "sub" values in Layman prime DB schema from Wagtail usernames to Wagtail user IDs. See [940](https://github.com/LayerManager/layman/issues/940). + - In case of development settings, run following script instead: + ```bash + docker compose -f docker-compose.deps.yml -f docker-compose.dev.yml run --rm --no-deps -u root -e LAYMAN_WAGTAIL_DB_URI= layman_dev bash -c "cd src && python3 -B v1_23_change_oauth2_sub_username_to_user_id.py" + ``` ### Migrations and checks #### Schema migrations - [#165](https://github.com/LayerManager/layman/issues/165) Add column `role_name` to table `rights` in prime DB schema. Add constraint that exactly one of columns `role_name` and `id_user` is not null. diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 1760e3125..7fd9c9675 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -24,6 +24,7 @@ services: - ./deps/geoserver/data:/geoserver/data_dir - ./deps/geoserver/sample/geoserver_data:/geoserver/initial_data_dir - ./deps/qgis/data:/qgis/data + - ./deps/wagtail/data:/wagtail/data depends_on: - timgen - redis diff --git a/src/db/util.py b/src/db/util.py index 78b91689c..094b13b4c 100644 --- a/src/db/util.py +++ b/src/db/util.py @@ -1,5 +1,7 @@ +import contextlib import logging import re +import sqlite3 from urllib import parse import psycopg2 import psycopg2.pool @@ -35,6 +37,35 @@ def get_connection_pool(db_uri_str=None, encapsulate_exception=True): def run_query(query, data=None, uri_str=None, encapsulate_exception=True, log_query=False): + if uri_str is None or uri_str.startswith('postgres:') or uri_str.startswith('postgresql:'): + method = _run_query_postgres + elif uri_str.startswith('sqlite:'): + method = _run_query_sqlite + else: + raise NotImplementedError(f"Unsupported database protocol: {uri_str}") + + return method(query, data=data, uri_str=uri_str, encapsulate_exception=encapsulate_exception, log_query=log_query) + + +def _run_query_sqlite(query, data=None, *, uri_str, encapsulate_exception=True, log_query=False): + assert data is None, f"data is not yet implemented" + db_uri_parsed = parse.urlparse(uri_str) + db_path = db_uri_parsed.path + try: + if log_query: + logger.info(f"query={query}") + with contextlib.closing(sqlite3.connect(db_path)) as conn: # auto-closes + with conn: # auto-commits + result = list(conn.execute(query)) + except BaseException as exc: + if encapsulate_exception: + logger.error(f"_run_query_sqlite, query={query}, data={data}, exc={exc}") + raise Error(2) from exc + raise exc + return result + + +def _run_query_postgres(query, data=None, uri_str=None, encapsulate_exception=True, log_query=False): pool = get_connection_pool(db_uri_str=uri_str, encapsulate_exception=encapsulate_exception, ) conn = pool.getconn() conn.autocommit = True @@ -47,7 +78,7 @@ def run_query(query, data=None, uri_str=None, encapsulate_exception=True, log_qu conn.commit() except BaseException as exc: if encapsulate_exception: - logger.error(f"run_query, query={query}, data={data}, exc={exc}") + logger.error(f"_run_query_postgres, query={query}, data={data}, exc={exc}") raise Error(2) from exc raise exc finally: