From 182d657d67556e831ca6c45148c95a0b78db2272 Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 21 Feb 2023 15:28:54 -0500 Subject: [PATCH] fixed tests w/ burnettk --- .../services/authorization_service.py | 3 ++- .../services/git_service.py | 16 ++++++++++------ .../scripts/test_get_all_permissions.py | 1 + .../unit/test_authorization_service.py | 18 ++++++++++++++---- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/spiffworkflow_backend/services/authorization_service.py b/src/spiffworkflow_backend/services/authorization_service.py index 3a054ec6..9d2f80cb 100644 --- a/src/spiffworkflow_backend/services/authorization_service.py +++ b/src/spiffworkflow_backend/services/authorization_service.py @@ -576,7 +576,8 @@ class AuthorizationService: # both for-me and NOT for-me URLs for the instance in question to see if you should get access to its logs. # if we implemented things this way, there would also be no way to restrict access to logs when you do not # restrict access to instances. everything would be inheriting permissions from instances. - # if we want to really codify this rule, we could change logs from a prefix to a suffix (just add it to the end of the process instances path). + # if we want to really codify this rule, we could change logs from a prefix to a suffix + # (just add it to the end of the process instances path). # but that makes it harder to change our minds in the future. for target_uri in [ f"/process-instances/for-me/{process_related_path_segment}", diff --git a/src/spiffworkflow_backend/services/git_service.py b/src/spiffworkflow_backend/services/git_service.py index 0638cca9..d885e4b6 100644 --- a/src/spiffworkflow_backend/services/git_service.py +++ b/src/spiffworkflow_backend/services/git_service.py @@ -154,17 +154,21 @@ class GitService: cls, command: list[str], return_success_state: bool = False ) -> Union[subprocess.CompletedProcess[bytes], bool]: """Run_shell_command.""" - env = { - 'GIT_COMMITTER_NAME': current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_USERNAME", "unknown"), - 'GIT_COMMITTER_EMAIL': current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL", "unknown@example.org"), + git_env_options = { + "GIT_COMMITTER_NAME": current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_USERNAME") or 'unknown', + "GIT_COMMITTER_EMAIL": current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_USER_EMAIL") or "unknown@example.org", } # SSH authentication can be also provided via gitconfig. - ssh_key_path = current_app.config.get("SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH") + ssh_key_path = current_app.config.get( + "SPIFFWORKFLOW_BACKEND_GIT_SSH_PRIVATE_KEY_PATH" + ) if ssh_key_path is not None: - env['GIT_SSH_COMMAND'] = 'ssh -F /dev/null -i %s' % ssh_key_path + git_env_options["GIT_SSH_COMMAND"] = "ssh -F /dev/null -i %s" % ssh_key_path # this is fine since we pass the commands directly - result = subprocess.run(command, check=False, capture_output=True, env=env) # noqa + result = subprocess.run( # noqa + command, check=False, capture_output=True, env=git_env_options + ) if return_success_state: return result.returncode == 0 diff --git a/tests/spiffworkflow_backend/scripts/test_get_all_permissions.py b/tests/spiffworkflow_backend/scripts/test_get_all_permissions.py index 95d15fbf..928299d4 100644 --- a/tests/spiffworkflow_backend/scripts/test_get_all_permissions.py +++ b/tests/spiffworkflow_backend/scripts/test_get_all_permissions.py @@ -61,6 +61,7 @@ class TestGetAllPermissions(BaseTest): "uri": "/tasks", "permissions": ["create", "read", "update", "delete"], }, + {'group_identifier': 'my_test_group', 'uri': '/process-data-file-download/hey:group:*', 'permissions': ['read']} ] permissions = GetAllPermissions().run(script_attributes_context) diff --git a/tests/spiffworkflow_backend/unit/test_authorization_service.py b/tests/spiffworkflow_backend/unit/test_authorization_service.py index d414616c..ea1978ef 100644 --- a/tests/spiffworkflow_backend/unit/test_authorization_service.py +++ b/tests/spiffworkflow_backend/unit/test_authorization_service.py @@ -156,9 +156,10 @@ class TestAuthorizationService(BaseTest): with_db_and_bpmn_file_cleanup: None, ) -> None: """Test_explode_permissions_all_on_process_group.""" - expected_permissions = [ + expected_permissions = sorted([ ("/logs/some-process-group:some-process-model:*", "read"), ("/process-data/some-process-group:some-process-model:*", "read"), + ("/process-data-file-download/some-process-group:some-process-model:*", "read"), ("/process-groups/some-process-group:some-process-model:*", "create"), ("/process-groups/some-process-group:some-process-model:*", "delete"), ("/process-groups/some-process-group:some-process-model:*", "read"), @@ -180,7 +181,7 @@ class TestAuthorizationService(BaseTest): ("/process-models/some-process-group:some-process-model:*", "update"), ("/task-data/some-process-group:some-process-model:*", "read"), ("/task-data/some-process-group:some-process-model:*", "update"), - ] + ]) permissions_to_assign = AuthorizationService.explode_permissions( "all", "PG:/some-process-group/some-process-model" ) @@ -201,6 +202,10 @@ class TestAuthorizationService(BaseTest): "/logs/some-process-group:some-process-model:*", "read", ), + ( + "/process-data-file-download/some-process-group:some-process-model:*", + "read", + ), ( "/process-instances/for-me/some-process-group:some-process-model:*", "read", @@ -222,8 +227,9 @@ class TestAuthorizationService(BaseTest): with_db_and_bpmn_file_cleanup: None, ) -> None: """Test_explode_permissions_all_on_process_model.""" - expected_permissions = [ + expected_permissions = sorted([ ("/logs/some-process-group:some-process-model/*", "read"), + ("/process-data-file-download/some-process-group:some-process-model/*", "read"), ("/process-data/some-process-group:some-process-model/*", "read"), ( "/process-instance-suspend/some-process-group:some-process-model/*", @@ -242,7 +248,7 @@ class TestAuthorizationService(BaseTest): ("/process-models/some-process-group:some-process-model/*", "update"), ("/task-data/some-process-group:some-process-model/*", "read"), ("/task-data/some-process-group:some-process-model/*", "update"), - ] + ]) permissions_to_assign = AuthorizationService.explode_permissions( "all", "PM:/some-process-group/some-process-model" ) @@ -263,6 +269,10 @@ class TestAuthorizationService(BaseTest): "/logs/some-process-group:some-process-model/*", "read", ), + ( + "/process-data-file-download/some-process-group:some-process-model/*", + "read", + ), ( "/process-instances/for-me/some-process-group:some-process-model/*", "read",