Feature/restrict call activity processes (#426)
* ensure users have access to run a process model in order to use it as a call activity w/ burnettk * bad some cache dir w/ burnettk * pyl --------- Co-authored-by: jasquat <jasquat@users.noreply.github.com>
This commit is contained in:
parent
ed9bdd42da
commit
8ba5616ea8
|
@ -302,7 +302,7 @@ def handle_exception(exception: Exception) -> flask.wrappers.Response:
|
|||
else:
|
||||
api_exception = ApiError(
|
||||
error_code=error_code,
|
||||
message=f"{exception.__class__.__name__} {str(exception)}",
|
||||
message=f"{exception.__class__.__name__}: {str(exception)}",
|
||||
sentry_link=sentry_link,
|
||||
status_code=status_code,
|
||||
)
|
||||
|
|
|
@ -70,6 +70,9 @@ class ProcessModelInfo:
|
|||
def id_for_file_path(self) -> str:
|
||||
return self.id.replace("/", os.sep)
|
||||
|
||||
def modified_process_model_identifier(self) -> str:
|
||||
return self.modify_process_identifier_for_path_param(self.id)
|
||||
|
||||
@classmethod
|
||||
def modify_process_identifier_for_path_param(cls, identifier: str) -> str:
|
||||
if "\\" in identifier:
|
||||
|
|
|
@ -64,7 +64,18 @@ def process_list() -> Any:
|
|||
primary process - helpful for finding possible call activities.
|
||||
"""
|
||||
references = SpecReferenceCache.query.filter_by(type="process").all()
|
||||
return SpecReferenceSchema(many=True).dump(references)
|
||||
process_model_identifiers = [r.process_model_id for r in references]
|
||||
permitted_process_model_identifiers = ProcessModelService.process_model_identifiers_with_permission_for_user(
|
||||
user=g.user,
|
||||
permission_to_check="create",
|
||||
permission_base_uri="/v1.0/process-instances",
|
||||
process_model_identifiers=process_model_identifiers,
|
||||
)
|
||||
permitted_references = []
|
||||
for spec_reference in references:
|
||||
if spec_reference.process_model_id in permitted_process_model_identifiers:
|
||||
permitted_references.append(spec_reference)
|
||||
return SpecReferenceSchema(many=True).dump(permitted_references)
|
||||
|
||||
|
||||
def process_caller_list(bpmn_process_identifiers: list[str]) -> Any:
|
||||
|
|
|
@ -517,7 +517,7 @@ def _create_or_update_process_model_file(
|
|||
|
||||
file = None
|
||||
try:
|
||||
file = SpecFileService.update_file(process_model, request_file.filename, request_file_contents)
|
||||
file = SpecFileService.update_file(process_model, request_file.filename, request_file_contents, user=g.user)
|
||||
except ProcessModelFileInvalidError as exception:
|
||||
raise (
|
||||
ApiError(
|
||||
|
|
|
@ -30,6 +30,7 @@ class DataSetupService:
|
|||
for ref in refs:
|
||||
try:
|
||||
SpecFileService.update_caches(ref)
|
||||
db.session.commit()
|
||||
except Exception as ex:
|
||||
failing_process_models.append(
|
||||
(
|
||||
|
|
|
@ -27,7 +27,6 @@ class ProcessCallerService:
|
|||
db.session.add(
|
||||
ProcessCallerCacheModel(process_identifier=called_process_id, calling_process_identifier=process_id)
|
||||
)
|
||||
db.session.commit()
|
||||
|
||||
@staticmethod
|
||||
def callers(process_ids: list[str]) -> list[str]:
|
||||
|
|
|
@ -18,6 +18,7 @@ from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
|
|||
from spiffworkflow_backend.models.process_model import PROCESS_MODEL_SUPPORTED_KEYS_FOR_DISK_SERIALIZATION
|
||||
from spiffworkflow_backend.models.process_model import ProcessModelInfo
|
||||
from spiffworkflow_backend.models.process_model import ProcessModelInfoSchema
|
||||
from spiffworkflow_backend.models.user import UserModel
|
||||
from spiffworkflow_backend.services.authorization_service import AuthorizationService
|
||||
from spiffworkflow_backend.services.file_system_service import FileSystemService
|
||||
from spiffworkflow_backend.services.user_service import UserService
|
||||
|
@ -213,6 +214,7 @@ class ProcessModelService(FileSystemService):
|
|||
process_models = cls.get_process_models(
|
||||
process_group_id=process_group_id, recursive=recursive, include_files=include_files
|
||||
)
|
||||
process_model_identifiers = [p.id for p in process_models]
|
||||
|
||||
permission_to_check = "read"
|
||||
permission_base_uri = "/v1.0/process-models"
|
||||
|
@ -224,6 +226,22 @@ class ProcessModelService(FileSystemService):
|
|||
permission_to_check = "create"
|
||||
permission_base_uri = "/v1.0/extensions"
|
||||
|
||||
permitted_process_model_identifiers = cls.process_model_identifiers_with_permission_for_user(
|
||||
user=user,
|
||||
permission_to_check=permission_to_check,
|
||||
permission_base_uri=permission_base_uri,
|
||||
process_model_identifiers=process_model_identifiers,
|
||||
)
|
||||
permitted_process_models = []
|
||||
for process_model in process_models:
|
||||
if process_model.id in permitted_process_model_identifiers:
|
||||
permitted_process_models.append(process_model)
|
||||
return permitted_process_models
|
||||
|
||||
@classmethod
|
||||
def process_model_identifiers_with_permission_for_user(
|
||||
cls, user: UserModel, permission_to_check: str, permission_base_uri: str, process_model_identifiers: list[str]
|
||||
) -> list[str]:
|
||||
# if user has access to uri/* with that permission then there's no reason to check each one individually
|
||||
guid_of_non_existent_item_to_check_perms_against = str(uuid.uuid4())
|
||||
has_permission = AuthorizationService.user_has_permission(
|
||||
|
@ -232,18 +250,21 @@ class ProcessModelService(FileSystemService):
|
|||
target_uri=f"{permission_base_uri}/{guid_of_non_existent_item_to_check_perms_against}",
|
||||
)
|
||||
if has_permission:
|
||||
return process_models
|
||||
return process_model_identifiers
|
||||
|
||||
new_process_model_list = []
|
||||
for process_model in process_models:
|
||||
modified_process_model_id = ProcessModelInfo.modify_process_identifier_for_path_param(process_model.id)
|
||||
permitted_process_model_identifiers = []
|
||||
for process_model_identifier in process_model_identifiers:
|
||||
modified_process_model_id = ProcessModelInfo.modify_process_identifier_for_path_param(
|
||||
process_model_identifier
|
||||
)
|
||||
uri = f"{permission_base_uri}/{modified_process_model_id}"
|
||||
has_permission = AuthorizationService.user_has_permission(
|
||||
user=user, permission=permission_to_check, target_uri=uri
|
||||
)
|
||||
if has_permission:
|
||||
new_process_model_list.append(process_model)
|
||||
return new_process_model_list
|
||||
permitted_process_model_identifiers.append(process_model_identifier)
|
||||
|
||||
return permitted_process_model_identifiers
|
||||
|
||||
@classmethod
|
||||
def get_parent_group_array_and_cache_it(
|
||||
|
|
|
@ -12,6 +12,8 @@ from spiffworkflow_backend.models.file import SpecReference
|
|||
from spiffworkflow_backend.models.message_triggerable_process_model import MessageTriggerableProcessModel
|
||||
from spiffworkflow_backend.models.process_model import ProcessModelInfo
|
||||
from spiffworkflow_backend.models.spec_reference import SpecReferenceCache
|
||||
from spiffworkflow_backend.models.user import UserModel
|
||||
from spiffworkflow_backend.services.authentication_service import NotAuthorizedError
|
||||
from spiffworkflow_backend.services.custom_parser import MyCustomParser
|
||||
from spiffworkflow_backend.services.file_system_service import FileSystemService
|
||||
from spiffworkflow_backend.services.process_caller_service import ProcessCallerService
|
||||
|
@ -97,6 +99,8 @@ class SpecFileService(FileSystemService):
|
|||
sub_parsers = list(parser.process_parsers.values())
|
||||
messages = parser.messages
|
||||
correlations = parser.correlations
|
||||
# to check permissions for call activities
|
||||
parser.get_process_dependencies()
|
||||
elif file_type.value == FileType.dmn.value:
|
||||
parser.add_dmn_xml(cls.get_etree_from_xml_bytes(binary_data))
|
||||
sub_parsers = list(parser.dmn_parsers.values())
|
||||
|
@ -149,7 +153,9 @@ class SpecFileService(FileSystemService):
|
|||
) from exception
|
||||
|
||||
@classmethod
|
||||
def update_file(cls, process_model_info: ProcessModelInfo, file_name: str, binary_data: bytes) -> File:
|
||||
def update_file(
|
||||
cls, process_model_info: ProcessModelInfo, file_name: str, binary_data: bytes, user: UserModel | None = None
|
||||
) -> File:
|
||||
SpecFileService.assert_valid_file_name(file_name)
|
||||
cls.validate_bpmn_xml(file_name, binary_data)
|
||||
|
||||
|
@ -157,6 +163,7 @@ class SpecFileService(FileSystemService):
|
|||
primary_process_ref = next((ref for ref in references if ref.is_primary and ref.is_executable), None)
|
||||
|
||||
SpecFileService.clear_caches_for_file(file_name, process_model_info)
|
||||
all_called_element_ids: set[str] = set()
|
||||
for ref in references:
|
||||
# If no valid primary process is defined, default to the first process in the
|
||||
# updated file.
|
||||
|
@ -176,8 +183,35 @@ class SpecFileService(FileSystemService):
|
|||
process_model_info,
|
||||
update_hash,
|
||||
)
|
||||
|
||||
all_called_element_ids = all_called_element_ids | set(ref.called_element_ids)
|
||||
SpecFileService.update_caches(ref)
|
||||
|
||||
if user is not None:
|
||||
called_element_refs = SpecReferenceCache.query.filter(
|
||||
SpecReferenceCache.identifier.in_(all_called_element_ids)
|
||||
).all()
|
||||
if len(called_element_refs) > 0:
|
||||
process_model_identifiers: list[str] = [r.process_model_id for r in called_element_refs]
|
||||
permitted_process_model_identifiers = (
|
||||
ProcessModelService.process_model_identifiers_with_permission_for_user(
|
||||
user=user,
|
||||
permission_to_check="create",
|
||||
permission_base_uri="/v1.0/process-instances",
|
||||
process_model_identifiers=process_model_identifiers,
|
||||
)
|
||||
)
|
||||
unpermitted_process_model_identifiers = set(process_model_identifiers) - set(
|
||||
permitted_process_model_identifiers
|
||||
)
|
||||
if len(unpermitted_process_model_identifiers):
|
||||
raise NotAuthorizedError(
|
||||
"You are not authorized to use one or more processes as a called element:"
|
||||
f" {','.join(unpermitted_process_model_identifiers)}"
|
||||
)
|
||||
|
||||
db.session.commit()
|
||||
|
||||
# make sure we save the file as the last thing we do to ensure validations have run
|
||||
full_file_path = SpecFileService.full_file_path(process_model_info, file_name)
|
||||
SpecFileService.write_file_data_to_system(full_file_path, binary_data)
|
||||
|
@ -251,7 +285,6 @@ class SpecFileService(FileSystemService):
|
|||
if process_id_lookup is None:
|
||||
process_id_lookup = SpecReferenceCache.from_spec_reference(ref)
|
||||
db.session.add(process_id_lookup)
|
||||
db.session.commit()
|
||||
else:
|
||||
if ref.relative_path != process_id_lookup.relative_path:
|
||||
full_bpmn_file_path = SpecFileService.full_path_from_relative_path(process_id_lookup.relative_path)
|
||||
|
@ -265,7 +298,6 @@ class SpecFileService(FileSystemService):
|
|||
else:
|
||||
process_id_lookup.relative_path = ref.relative_path
|
||||
db.session.add(process_id_lookup)
|
||||
db.session.commit()
|
||||
|
||||
@staticmethod
|
||||
def update_process_caller_cache(ref: SpecReference) -> None:
|
||||
|
@ -284,7 +316,6 @@ class SpecFileService(FileSystemService):
|
|||
process_model_identifier=ref.process_model_id,
|
||||
)
|
||||
db.session.add(message_triggerable_process_model)
|
||||
db.session.commit()
|
||||
else:
|
||||
if message_triggerable_process_model.process_model_identifier != ref.process_model_id:
|
||||
raise ProcessModelFileInvalidError(
|
||||
|
@ -315,4 +346,3 @@ class SpecFileService(FileSystemService):
|
|||
retrieval_expression=retrieval_expression,
|
||||
)
|
||||
db.session.add(new_cache)
|
||||
db.session.commit()
|
||||
|
|
|
@ -521,7 +521,9 @@ class TestProcessApi(BaseTest):
|
|||
"/v1.0/processes",
|
||||
headers=self.logged_in_headers(with_super_admin_user),
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json is not None
|
||||
assert isinstance(response.json, list)
|
||||
# We should get 5 back, as one of the items in the cache is a decision.
|
||||
assert len(response.json) == 5
|
||||
simple_form = next(p for p in response.json if p["identifier"] == "Process_WithForm")
|
||||
|
@ -531,6 +533,57 @@ class TestProcessApi(BaseTest):
|
|||
assert simple_form["is_executable"] is True
|
||||
assert simple_form["is_primary"] is True
|
||||
|
||||
def test_process_list_with_restricted_access(
|
||||
self,
|
||||
app: Flask,
|
||||
client: FlaskClient,
|
||||
with_db_and_bpmn_file_cleanup: None,
|
||||
with_super_admin_user: UserModel,
|
||||
) -> None:
|
||||
load_test_spec(
|
||||
"test_group_one/simple_form",
|
||||
process_model_source_directory="simple_form",
|
||||
bpmn_file_name="simple_form",
|
||||
)
|
||||
# When adding a process model with one Process, no decisions, and some json files, only one process is recorded.
|
||||
assert len(SpecReferenceCache.query.all()) == 1
|
||||
|
||||
self.create_group_and_model_with_bpmn(
|
||||
client=client,
|
||||
user=with_super_admin_user,
|
||||
process_group_id="test_group_two",
|
||||
process_model_id="call_activity_nested",
|
||||
bpmn_file_location="call_activity_nested",
|
||||
)
|
||||
# When adding a process model with 4 processes and a decision, 5 new records will be in the Cache
|
||||
assert len(SpecReferenceCache.query.all()) == 6
|
||||
|
||||
user_one = self.create_user_with_permission(
|
||||
username="user_one", target_uri="/v1.0/process-groups/test_group_one:*"
|
||||
)
|
||||
self.add_permissions_to_user(user=user_one, target_uri="/v1.0/processes", permission_names=["read"])
|
||||
self.add_permissions_to_user(
|
||||
user=user_one, target_uri="/v1.0/process-instances/test_group_one:*", permission_names=["create"]
|
||||
)
|
||||
|
||||
# get the results
|
||||
response = client.get(
|
||||
"/v1.0/processes",
|
||||
headers=self.logged_in_headers(user_one),
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json is not None
|
||||
|
||||
# This user should only have access to one process
|
||||
assert isinstance(response.json, list)
|
||||
assert len(response.json) == 1
|
||||
simple_form = next(p for p in response.json if p["identifier"] == "Process_WithForm")
|
||||
assert simple_form["display_name"] == "Process With Form"
|
||||
assert simple_form["process_model_id"] == "test_group_one/simple_form"
|
||||
assert simple_form["has_lanes"] is False
|
||||
assert simple_form["is_executable"] is True
|
||||
assert simple_form["is_primary"] is True
|
||||
|
||||
def test_process_callers(
|
||||
self,
|
||||
app: Flask,
|
||||
|
|
|
@ -0,0 +1,62 @@
|
|||
import io
|
||||
from hashlib import sha256
|
||||
|
||||
from flask.app import Flask
|
||||
from flask.testing import FlaskClient
|
||||
from spiffworkflow_backend.models.user import UserModel
|
||||
from spiffworkflow_backend.services.spec_file_service import SpecFileService
|
||||
|
||||
from tests.spiffworkflow_backend.helpers.base_test import BaseTest
|
||||
|
||||
|
||||
class TestProcessModelsController(BaseTest):
|
||||
def test_cannot_save_process_model_file_with_called_elements_user_does_not_have_access_to(
|
||||
self,
|
||||
app: Flask,
|
||||
client: FlaskClient,
|
||||
with_db_and_bpmn_file_cleanup: None,
|
||||
with_super_admin_user: UserModel,
|
||||
) -> None:
|
||||
process_model = self.create_group_and_model_with_bpmn(
|
||||
client=client,
|
||||
user=with_super_admin_user,
|
||||
process_group_id="caller",
|
||||
process_model_id="caller",
|
||||
bpmn_file_location="call_activity_same_directory",
|
||||
bpmn_file_name="call_activity_test.bpmn",
|
||||
)
|
||||
self.create_group_and_model_with_bpmn(
|
||||
client=client,
|
||||
user=with_super_admin_user,
|
||||
process_group_id="callee",
|
||||
process_model_id="callee",
|
||||
bpmn_file_location="call_activity_same_directory",
|
||||
bpmn_file_name="callable_process.bpmn",
|
||||
)
|
||||
|
||||
user_one = self.create_user_with_permission(username="user_one", target_uri="/v1.0/process-groups/caller:*")
|
||||
self.add_permissions_to_user(
|
||||
user=user_one, target_uri="/v1.0/process-models/caller:*", permission_names=["create", "read", "update"]
|
||||
)
|
||||
assert process_model.primary_file_name is not None
|
||||
bpmn_file_data_bytes = SpecFileService.get_data(process_model, process_model.primary_file_name)
|
||||
file_contents_hash = sha256(bpmn_file_data_bytes).hexdigest()
|
||||
|
||||
data = {"file": (io.BytesIO(bpmn_file_data_bytes), process_model.primary_file_name)}
|
||||
url = (
|
||||
f"/v1.0/process-models/{process_model.modified_process_model_identifier()}/files/"
|
||||
f"{process_model.primary_file_name}?file_contents_hash={file_contents_hash}"
|
||||
)
|
||||
response = client.put(
|
||||
url,
|
||||
data=data,
|
||||
follow_redirects=True,
|
||||
content_type="multipart/form-data",
|
||||
headers=self.logged_in_headers(user_one),
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json is not None
|
||||
assert response.json["message"].startswith(
|
||||
"NotAuthorizedError: You are not authorized to use one or more processes as a called element"
|
||||
)
|
Loading…
Reference in New Issue