feature/get-pg-of-readable-pm (#832)
* get parent process groups of process models that the user has access to w/ burnettk * use the process group list to get the info we need for the group show page for permissions w/ burnettk * clear the browser cache when updating a process group w/ burnettk * fixed broken test w/ burnettk --------- Co-authored-by: jasquat <jasquat@users.noreply.github.com>
This commit is contained in:
parent
2f83a68787
commit
a7a48ee9fc
|
@ -35,3 +35,12 @@ permissions:
|
|||
groups: [group2]
|
||||
actions: [read]
|
||||
uri: PG:ALL
|
||||
|
||||
pm-read:
|
||||
groups: [group3]
|
||||
actions: [read]
|
||||
uri: PM:site-administration:set-permissions
|
||||
pg-read:
|
||||
groups: [group3]
|
||||
actions: [read]
|
||||
uri: PG:misc
|
||||
|
|
|
@ -101,7 +101,8 @@ def process_group_show(
|
|||
) -> Any:
|
||||
process_group_id = _un_modify_modified_process_model_id(modified_process_group_id)
|
||||
try:
|
||||
process_group = ProcessModelService.get_process_group(process_group_id, find_all_nested_items=False)
|
||||
# do not return child models and groups here since this call does not check permissions of them
|
||||
process_group = ProcessModelService.get_process_group(process_group_id, find_direct_nested_items=False)
|
||||
except ProcessEntityNotFoundError as exception:
|
||||
raise (
|
||||
ApiError(
|
||||
|
|
|
@ -156,8 +156,11 @@ class AuthorizationService:
|
|||
@classmethod
|
||||
def target_uri_matches_actual_uri(cls, target_uri: str, actual_uri: str) -> bool:
|
||||
if target_uri.endswith("%"):
|
||||
target_uri_without_suffix = target_uri.removesuffix("%").removesuffix(":").removesuffix("/")
|
||||
return actual_uri.startswith(target_uri_without_suffix) or actual_uri == target_uri_without_suffix
|
||||
target_uri_without_wildcard = target_uri.removesuffix("%")
|
||||
target_uri_without_wildcard_and_without_delimiters = target_uri_without_wildcard.removesuffix(":").removesuffix("/")
|
||||
return actual_uri == target_uri_without_wildcard_and_without_delimiters or actual_uri.startswith(
|
||||
target_uri_without_wildcard
|
||||
)
|
||||
return actual_uri == target_uri
|
||||
|
||||
@classmethod
|
||||
|
|
|
@ -11,6 +11,7 @@ from spiffworkflow_backend.exceptions.api_error import ApiError
|
|||
from spiffworkflow_backend.exceptions.process_entity_not_found_error import ProcessEntityNotFoundError
|
||||
from spiffworkflow_backend.interfaces import ProcessGroupLite
|
||||
from spiffworkflow_backend.interfaces import ProcessGroupLitesWithCache
|
||||
from spiffworkflow_backend.models.permission_assignment import PermitDeny
|
||||
from spiffworkflow_backend.models.process_group import PROCESS_GROUP_SUPPORTED_KEYS_FOR_DISK_SERIALIZATION
|
||||
from spiffworkflow_backend.models.process_group import ProcessGroup
|
||||
from spiffworkflow_backend.models.process_group import ProcessGroupSchema
|
||||
|
@ -329,11 +330,14 @@ class ProcessModelService(FileSystemService):
|
|||
def get_process_groups_for_api(
|
||||
cls,
|
||||
process_group_id: str | None = None,
|
||||
user: UserModel | None = None,
|
||||
) -> list[ProcessGroup]:
|
||||
process_groups = cls.get_process_groups(process_group_id)
|
||||
|
||||
permission_to_check = "read"
|
||||
permission_base_uri = "/v1.0/process-groups"
|
||||
permission_base_uri = "/process-groups"
|
||||
|
||||
if user is None:
|
||||
user = UserService.current_user()
|
||||
|
||||
# if user has access to uri/* with that permission then there's no reason to check each one individually
|
||||
|
@ -346,14 +350,49 @@ class ProcessModelService(FileSystemService):
|
|||
if has_permission:
|
||||
return process_groups
|
||||
|
||||
permission_assignments = AuthorizationService.all_permission_assignments_for_user(user=user)
|
||||
|
||||
new_process_group_list = []
|
||||
denied_parent_ids: set[str] = set()
|
||||
for process_group in process_groups:
|
||||
modified_process_group_id = ProcessModelInfo.modify_process_identifier_for_path_param(process_group.id)
|
||||
uri = f"{permission_base_uri}/{modified_process_group_id}"
|
||||
has_permission = AuthorizationService.user_has_permission(user=user, permission=permission_to_check, target_uri=uri)
|
||||
target_uri = f"{permission_base_uri}/{modified_process_group_id}"
|
||||
has_permission = AuthorizationService.permission_assignments_include(
|
||||
permission_assignments=permission_assignments,
|
||||
permission=permission_to_check,
|
||||
target_uri=target_uri,
|
||||
)
|
||||
if not has_permission:
|
||||
for pa in permission_assignments:
|
||||
if (
|
||||
pa.permission == permission_to_check
|
||||
and pa.grant_type == PermitDeny.deny.value
|
||||
and AuthorizationService.target_uri_matches_actual_uri(pa.permission_target.uri, target_uri)
|
||||
):
|
||||
denied_parent_ids.add(f"{process_group.id}")
|
||||
elif (
|
||||
pa.permission == permission_to_check
|
||||
and pa.grant_type == PermitDeny.permit.value
|
||||
and (
|
||||
pa.permission_target.uri.startswith(f"{target_uri}:")
|
||||
or pa.permission_target.uri.startswith(f"/process-models/{modified_process_group_id}:")
|
||||
)
|
||||
):
|
||||
has_permission = True
|
||||
if has_permission:
|
||||
new_process_group_list.append(process_group)
|
||||
return new_process_group_list
|
||||
|
||||
# remove any process group that also matched a deny permission
|
||||
permitted_process_groups = []
|
||||
for process_group in new_process_group_list:
|
||||
has_denied_permission = False
|
||||
for dpi in denied_parent_ids:
|
||||
if process_group.id.startswith(f"{dpi}:") or process_group.id == dpi:
|
||||
has_denied_permission = True
|
||||
if not has_denied_permission:
|
||||
permitted_process_groups.append(process_group)
|
||||
|
||||
return permitted_process_groups
|
||||
|
||||
@classmethod
|
||||
def get_process_group(
|
||||
|
@ -477,10 +516,14 @@ class ProcessModelService(FileSystemService):
|
|||
# we don't store `id` in the json files, so we add it in here
|
||||
process_group.id = process_group_id
|
||||
|
||||
if find_direct_nested_items or find_all_nested_items:
|
||||
with os.scandir(dir_path) as nested_items:
|
||||
process_group.process_models = []
|
||||
process_group.process_groups = []
|
||||
|
||||
if find_direct_nested_items is False:
|
||||
return process_group
|
||||
|
||||
if find_all_nested_items:
|
||||
with os.scandir(dir_path) as nested_items:
|
||||
for nested_item in nested_items:
|
||||
if nested_item.is_dir():
|
||||
# TODO: check whether this is a group or model
|
||||
|
|
|
@ -105,6 +105,12 @@ class BaseTest:
|
|||
process_group_id: str,
|
||||
display_name: str = "",
|
||||
) -> ProcessGroup:
|
||||
process_group_parent_id = "/".join(process_group_id.rsplit("/", 1)[:-1])
|
||||
if process_group_parent_id != "":
|
||||
if not ProcessModelService.is_process_group_identifier(process_group_parent_id):
|
||||
raise Exception(
|
||||
f"Parent process group does not exist for '{process_group_id}'. Parent was '{process_group_parent_id}'"
|
||||
)
|
||||
process_group = ProcessGroup(id=process_group_id, display_name=display_name, display_order=0, admin=False)
|
||||
return ProcessModelService.add_process_group(process_group)
|
||||
|
||||
|
@ -127,6 +133,21 @@ class BaseTest:
|
|||
assert response.json["id"] == process_group_id
|
||||
return process_group_id
|
||||
|
||||
def create_process_model(
|
||||
self,
|
||||
process_model_id: str,
|
||||
display_name: str | None = None,
|
||||
) -> ProcessModelInfo:
|
||||
process_group_parent_id = "/".join(process_model_id.rsplit("/", 1)[:-1])
|
||||
if process_group_parent_id != "":
|
||||
if not ProcessModelService.is_process_group_identifier(process_group_parent_id):
|
||||
raise Exception(
|
||||
f"Parent process group does not exist for '{process_model_id}'. Parent was '{process_group_parent_id}'"
|
||||
)
|
||||
process_model = ProcessModelInfo(id=process_model_id, display_name=process_model_id, description=process_model_id)
|
||||
ProcessModelService.save_process_model(process_model)
|
||||
return process_model
|
||||
|
||||
def create_process_model_with_api(
|
||||
self,
|
||||
client: FlaskClient,
|
||||
|
|
|
@ -1186,7 +1186,7 @@ class TestProcessApi(BaseTest):
|
|||
assert response.status_code == 200
|
||||
assert response.json is not None
|
||||
assert response.json["id"] == process_group_id
|
||||
assert response.json["process_models"][0]["id"] == process_model.id
|
||||
assert response.json["process_models"] == []
|
||||
assert response.json["parent_groups"] == []
|
||||
|
||||
def test_get_process_group_show_when_nested(
|
||||
|
|
|
@ -1,7 +1,9 @@
|
|||
import re
|
||||
|
||||
from flask import Flask
|
||||
from spiffworkflow_backend.services.authorization_service import AuthorizationService
|
||||
from spiffworkflow_backend.services.process_model_service import ProcessModelService
|
||||
from spiffworkflow_backend.services.user_service import UserService
|
||||
|
||||
from tests.spiffworkflow_backend.helpers.base_test import BaseTest
|
||||
from tests.spiffworkflow_backend.helpers.test_data import load_test_spec
|
||||
|
@ -51,3 +53,79 @@ class TestProcessModelService(BaseTest):
|
|||
assert len(pm_dict["files"]) == 1
|
||||
file = pm_dict["files"][0]
|
||||
assert re.search("hello", file["file_contents"]) is not None
|
||||
|
||||
def test_can_get_sub_process_groups_when_no_permission_to_parent(
|
||||
self,
|
||||
app: Flask,
|
||||
with_db_and_bpmn_file_cleanup: None,
|
||||
) -> None:
|
||||
user = self.find_or_create_user(username="user_one")
|
||||
user_group = UserService.find_or_create_group("group_one")
|
||||
noread = "DENY:read"
|
||||
UserService.add_user_to_group(user, user_group)
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:a1:b2:c2")
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:a1:b2:c3:d1")
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, noread, "PG:a1:b2:c4")
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PG:a1:b2:c4:d1")
|
||||
|
||||
self.create_process_group("a1")
|
||||
self.create_process_group("a1/b1")
|
||||
self.create_process_group("a1/b2")
|
||||
self.create_process_group("a1/b3")
|
||||
self.create_process_group("a1/b2/c1")
|
||||
self.create_process_group("a1/b2/c2")
|
||||
self.create_process_group("a1/b2/c3")
|
||||
self.create_process_group("a1/b2/c4")
|
||||
self.create_process_group("a1/b2/c3/d1")
|
||||
self.create_process_group("a1/b2/c4/d1")
|
||||
|
||||
process_groups = ProcessModelService.get_process_groups_for_api(user=user)
|
||||
assert len(process_groups) == 1
|
||||
assert process_groups[0].id == "a1"
|
||||
|
||||
process_groups = ProcessModelService.get_process_groups_for_api("a1", user=user)
|
||||
assert len(process_groups) == 1
|
||||
assert process_groups[0].id == "a1/b2"
|
||||
|
||||
process_groups = ProcessModelService.get_process_groups_for_api("a1/b2", user=user)
|
||||
pg_identifiers = [pg.id for pg in process_groups]
|
||||
assert len(pg_identifiers) == 2
|
||||
assert pg_identifiers == ["a1/b2/c2", "a1/b2/c3"]
|
||||
|
||||
process_groups = ProcessModelService.get_process_groups_for_api("a1/b2/c4", user=user)
|
||||
assert len(process_groups) == 0
|
||||
process_groups = ProcessModelService.get_process_groups_for_api("a1/b2/c4/d1", user=user)
|
||||
assert len(process_groups) == 0
|
||||
|
||||
def test_can_get_sub_process_models_when_no_permission_to_parent(
|
||||
self,
|
||||
app: Flask,
|
||||
with_db_and_bpmn_file_cleanup: None,
|
||||
) -> None:
|
||||
user = self.find_or_create_user(username="user_one")
|
||||
user_group = UserService.find_or_create_group("group_one")
|
||||
noread = "DENY:read"
|
||||
UserService.add_user_to_group(user, user_group)
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "read", "PM:a1:b2:pm3")
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, noread, "PM:a1:b2:pm4")
|
||||
AuthorizationService.add_permission_from_uri_or_macro(user_group.identifier, "DENY:read", "PM:a1:b3:c2")
|
||||
|
||||
self.create_process_group("a1")
|
||||
self.create_process_group("a1/b1")
|
||||
self.create_process_group("a1/b2")
|
||||
self.create_process_group("a1/b3")
|
||||
self.create_process_model("a1/b2/pm1")
|
||||
self.create_process_model("a1/b2/pm2")
|
||||
self.create_process_model("a1/b2/pm3")
|
||||
self.create_process_model("a1/b2/pm4")
|
||||
self.create_process_group("a1/b3/c2")
|
||||
self.create_process_group("a1/b3/c2/pm1")
|
||||
|
||||
process_groups = ProcessModelService.get_process_groups_for_api(user=user)
|
||||
assert len(process_groups) == 1
|
||||
assert process_groups[0].id == "a1"
|
||||
|
||||
process_groups = ProcessModelService.get_process_groups_for_api("a1", user=user)
|
||||
pg_identifiers = [pg.id for pg in process_groups]
|
||||
assert len(pg_identifiers) == 1
|
||||
assert process_groups[0].id == "a1/b2"
|
||||
|
|
|
@ -5,6 +5,7 @@ import { Button, Form, Stack, TextInput, TextArea } from '@carbon/react';
|
|||
import { modifyProcessIdentifierForPathParam, slugifyString } from '../helpers';
|
||||
import HttpService from '../services/HttpService';
|
||||
import { ProcessGroup } from '../interfaces';
|
||||
import useProcessGroupFetcher from '../hooks/useProcessGroupFetcher';
|
||||
|
||||
type OwnProps = {
|
||||
mode: string;
|
||||
|
@ -24,8 +25,11 @@ export default function ProcessGroupForm({
|
|||
const navigate = useNavigate();
|
||||
let newProcessGroupId = processGroup.id;
|
||||
|
||||
const navigateToProcessGroup = (_result: any) => {
|
||||
const { updateProcessGroupCache } = useProcessGroupFetcher(processGroup.id);
|
||||
|
||||
const handleProcessGroupUpdateResponse = (_result: any) => {
|
||||
if (newProcessGroupId) {
|
||||
updateProcessGroupCache(processGroup);
|
||||
navigate(
|
||||
`/process-groups/${modifyProcessIdentifierForPathParam(
|
||||
newProcessGroupId
|
||||
|
@ -82,7 +86,7 @@ export default function ProcessGroupForm({
|
|||
|
||||
HttpService.makeCallToBackend({
|
||||
path,
|
||||
successCallback: navigateToProcessGroup,
|
||||
successCallback: handleProcessGroupUpdateResponse,
|
||||
httpMethod,
|
||||
postBody,
|
||||
});
|
||||
|
|
|
@ -1,13 +1,7 @@
|
|||
import { ReactElement, useEffect, useState } from 'react';
|
||||
import { useSearchParams } from 'react-router-dom';
|
||||
import {
|
||||
ArrowRight,
|
||||
// @ts-ignore
|
||||
} from '@carbon/icons-react';
|
||||
import {
|
||||
ClickableTile,
|
||||
// @ts-ignore
|
||||
} from '@carbon/react';
|
||||
import { ArrowRight } from '@carbon/icons-react';
|
||||
import { ClickableTile } from '@carbon/react';
|
||||
import HttpService from '../services/HttpService';
|
||||
import { ProcessGroup } from '../interfaces';
|
||||
import {
|
||||
|
|
|
@ -0,0 +1,85 @@
|
|||
import { useCallback, useEffect, useState } from 'react';
|
||||
import { ProcessGroup } from '../interfaces';
|
||||
import HttpService from '../services/HttpService';
|
||||
import { useUriListForPermissions } from './UriListForPermissions';
|
||||
|
||||
// cache will only help if user is clicking around a lot but we need to avoid issues
|
||||
// where if a user updates a process group another user cannot see the update.
|
||||
const CACHE_DURATION_MS = 60 * 1000;
|
||||
|
||||
const LOCAL_STORAGE_PROCESS_GROUP_CACHE_KEY = 'processGroupCache';
|
||||
|
||||
export default function useProcessGroupFetcher(processGroupIdentifier: string) {
|
||||
const [processGroup, setProcessGroup] = useState<ProcessGroup | null>(null);
|
||||
const { targetUris } = useUriListForPermissions();
|
||||
|
||||
const getProcessGroupCache = useCallback(() => {
|
||||
return JSON.parse(
|
||||
localStorage.getItem(LOCAL_STORAGE_PROCESS_GROUP_CACHE_KEY) || '{}'
|
||||
);
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
const storedProcessGroups = getProcessGroupCache();
|
||||
|
||||
const handleProcessGroupResponse = (result: any) => {
|
||||
const timestamp = Date.now();
|
||||
const processGroups = result.results;
|
||||
processGroups.forEach((pg: ProcessGroup) => {
|
||||
storedProcessGroups[pg.id] = { processGroup: pg, timestamp };
|
||||
if (pg.id === processGroupIdentifier) {
|
||||
setProcessGroup(pg);
|
||||
}
|
||||
});
|
||||
localStorage.setItem(
|
||||
'storedProcessGroups',
|
||||
JSON.stringify(storedProcessGroups)
|
||||
);
|
||||
};
|
||||
|
||||
const fetchProcessGroups = () => {
|
||||
const parentProcessGroupIdentifier = processGroupIdentifier
|
||||
.split('/')
|
||||
.slice(0, -1)
|
||||
.join('/');
|
||||
|
||||
HttpService.makeCallToBackend({
|
||||
path: `${targetUris.processGroupListPath}?process_group_identifier=${parentProcessGroupIdentifier}`,
|
||||
successCallback: handleProcessGroupResponse,
|
||||
});
|
||||
};
|
||||
|
||||
if (processGroupIdentifier in storedProcessGroups) {
|
||||
const pg = storedProcessGroups[processGroupIdentifier].processGroup;
|
||||
const { timestamp } = storedProcessGroups[processGroupIdentifier];
|
||||
|
||||
if (Date.now() - timestamp < CACHE_DURATION_MS) {
|
||||
setProcessGroup(pg);
|
||||
} else {
|
||||
fetchProcessGroups();
|
||||
}
|
||||
} else {
|
||||
fetchProcessGroups();
|
||||
}
|
||||
}, [
|
||||
processGroupIdentifier,
|
||||
targetUris.processGroupListPath,
|
||||
getProcessGroupCache,
|
||||
]);
|
||||
|
||||
const updateProcessGroupCache = (updatedProcessGroup: ProcessGroup) => {
|
||||
const storedProcessGroups = getProcessGroupCache();
|
||||
const timestamp = Date.now();
|
||||
|
||||
storedProcessGroups[updatedProcessGroup.id] = {
|
||||
processGroup: updatedProcessGroup,
|
||||
timestamp,
|
||||
};
|
||||
localStorage.setItem(
|
||||
LOCAL_STORAGE_PROCESS_GROUP_CACHE_KEY,
|
||||
JSON.stringify(storedProcessGroups)
|
||||
);
|
||||
};
|
||||
|
||||
return { processGroup, updateProcessGroupCache };
|
||||
}
|
|
@ -1,33 +1,25 @@
|
|||
import { useEffect, useState } from 'react';
|
||||
import { useParams, useNavigate } from 'react-router-dom';
|
||||
import {
|
||||
TrashCan,
|
||||
Edit,
|
||||
// @ts-ignore
|
||||
} from '@carbon/icons-react';
|
||||
// @ts-ignore
|
||||
import { TrashCan, Edit } from '@carbon/icons-react';
|
||||
import { Button, Stack } from '@carbon/react';
|
||||
import { Can } from '@casl/react';
|
||||
import ProcessBreadcrumb from '../components/ProcessBreadcrumb';
|
||||
import HttpService from '../services/HttpService';
|
||||
import { modifyProcessIdentifierForPathParam, setPageTitle } from '../helpers';
|
||||
import {
|
||||
PermissionsToCheck,
|
||||
ProcessGroup,
|
||||
// ProcessModel,
|
||||
} from '../interfaces';
|
||||
modifyProcessIdentifierForPathParam,
|
||||
unModifyProcessIdentifierForPathParam,
|
||||
} from '../helpers';
|
||||
import { PermissionsToCheck } from '../interfaces';
|
||||
import { useUriListForPermissions } from '../hooks/UriListForPermissions';
|
||||
import { usePermissionFetcher } from '../hooks/PermissionService';
|
||||
import ProcessGroupListTiles from '../components/ProcessGroupListTiles';
|
||||
import ButtonWithConfirmation from '../components/ButtonWithConfirmation';
|
||||
import ProcessModelListTiles from '../components/ProcessModelListTiles';
|
||||
import useProcessGroupFetcher from '../hooks/useProcessGroupFetcher';
|
||||
|
||||
export default function ProcessGroupShow() {
|
||||
const params = useParams();
|
||||
const navigate = useNavigate();
|
||||
|
||||
const [processGroup, setProcessGroup] = useState<ProcessGroup | null>(null);
|
||||
|
||||
const { targetUris } = useUriListForPermissions();
|
||||
const permissionRequestData: PermissionsToCheck = {
|
||||
[targetUris.processGroupListPath]: ['POST'],
|
||||
|
@ -37,17 +29,10 @@ export default function ProcessGroupShow() {
|
|||
const { ability, permissionsLoaded } = usePermissionFetcher(
|
||||
permissionRequestData
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
const processResult = (result: any) => {
|
||||
setProcessGroup(result);
|
||||
setPageTitle([result.display_name]);
|
||||
};
|
||||
HttpService.makeCallToBackend({
|
||||
path: `/process-groups/${params.process_group_id}`,
|
||||
successCallback: processResult,
|
||||
});
|
||||
}, [params.process_group_id]);
|
||||
const unModifiedProcessGroupId = unModifyProcessIdentifierForPathParam(
|
||||
`${params.process_group_id}`
|
||||
);
|
||||
const { processGroup } = useProcessGroupFetcher(unModifiedProcessGroupId);
|
||||
|
||||
const navigateToProcessGroups = (_result: any) => {
|
||||
navigate(`/process-groups`);
|
||||
|
@ -137,7 +122,6 @@ export default function ProcessGroupShow() {
|
|||
<ProcessModelListTiles
|
||||
headerElement={<h2>Process Models</h2>}
|
||||
processGroup={processGroup}
|
||||
defaultProcessModels={processGroup.process_models}
|
||||
showNoItemsDisplayText={showNoItemsDisplayText}
|
||||
userCanCreateProcessModels={ability.can(
|
||||
'POST',
|
||||
|
@ -149,7 +133,6 @@ export default function ProcessGroupShow() {
|
|||
<ProcessGroupListTiles
|
||||
processGroup={processGroup}
|
||||
headerElement={<h2 className="clear-left">Process Groups</h2>}
|
||||
defaultProcessGroups={processGroup.process_groups}
|
||||
showNoItemsDisplayText={showNoItemsDisplayText}
|
||||
userCanCreateProcessModels={ability.can(
|
||||
'POST',
|
||||
|
|
Loading…
Reference in New Issue