diff --git a/src/govoplan_workflow/backend/governance.py b/src/govoplan_workflow/backend/governance.py index 26ccd94..92621b0 100644 --- a/src/govoplan_workflow/backend/governance.py +++ b/src/govoplan_workflow/backend/governance.py @@ -70,7 +70,7 @@ def normalize_definition_scope( ) return principal.tenant_id, "group", clean_id, f"group:{clean_id}" if clean_type == "user": - clean_id = clean_id or principal.membership_id or principal.account_id + clean_id = clean_id or principal.account_id if ( clean_id not in {principal.membership_id, principal.account_id} and not administrative @@ -78,6 +78,8 @@ def normalize_definition_scope( raise PermissionError( "Definitions can only be created for the current user." ) + if clean_id == principal.membership_id: + clean_id = principal.account_id return principal.tenant_id, "user", clean_id, f"user:{clean_id}" raise ValueError( "Definition scope must be system, tenant, group, or user." diff --git a/src/govoplan_workflow/backend/manifest.py b/src/govoplan_workflow/backend/manifest.py index 2adddb6..db0df35 100644 --- a/src/govoplan_workflow/backend/manifest.py +++ b/src/govoplan_workflow/backend/manifest.py @@ -3,6 +3,7 @@ from __future__ import annotations from pathlib import Path from govoplan_core.core.access import ( + CAPABILITY_ACCESS_DIRECTORY, CAPABILITY_AUTH_PERMISSION_EVALUATOR, CAPABILITY_AUTH_PRINCIPAL_RESOLVER, ) @@ -137,6 +138,7 @@ manifest = ModuleManifest( "views", ), optional_capabilities=( + CAPABILITY_ACCESS_DIRECTORY, CAPABILITY_AUTH_PRINCIPAL_RESOLVER, CAPABILITY_AUTH_PERMISSION_EVALUATOR, CAPABILITY_DATAFLOW_RUN_LIFECYCLE, diff --git a/src/govoplan_workflow/backend/router.py b/src/govoplan_workflow/backend/router.py index 476d1a2..13ecfa7 100644 --- a/src/govoplan_workflow/backend/router.py +++ b/src/govoplan_workflow/backend/router.py @@ -1,10 +1,19 @@ from __future__ import annotations -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, Query, status from sqlalchemy.orm import Session +from govoplan_core.api.v1.schemas import ( + ReferenceOptionListResponse, + ReferenceOptionResponse, +) from govoplan_core.audit.logging import audit_event from govoplan_core.auth import ApiPrincipal, get_api_principal, has_scope +from govoplan_core.core.references import ( + access_scope_reference_options, + access_scope_reference_provider_available, + validate_access_scope_reference, +) from govoplan_core.db.session import get_session from govoplan_workflow.backend.governance import ( definition_decision, @@ -210,6 +219,42 @@ def api_node_types( ) +@router.get("/scope-targets", response_model=ReferenceOptionListResponse) +def api_scope_targets( + scope_type: str, + q: str = "", + selected: list[str] = Query(default=[]), + limit: int = Query(default=50, ge=1, le=200), + principal: ApiPrincipal = Depends(get_api_principal), +) -> ReferenceOptionListResponse: + _require_any_scope( + principal, + DEFINITION_READ_SCOPE, + DEFINITION_WRITE_SCOPE, + ADMIN_SCOPE, + ) + registry = get_registry() + try: + options = access_scope_reference_options( + registry, + principal, + scope_type=scope_type, + query=q, + selected_values=selected, + limit=limit, + administrative=has_scope(principal, ADMIN_SCOPE), + ) + except ValueError as exc: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail=str(exc), + ) from exc + return ReferenceOptionListResponse( + options=[ReferenceOptionResponse(**item.to_dict()) for item in options], + provider_available=access_scope_reference_provider_available(registry), + ) + + @router.post( "/definitions/validate", response_model=WorkflowGraphValidationResponse, @@ -296,6 +341,13 @@ def api_create_definition( payload = payload.model_copy( update={"scope_type": scope_type, "scope_id": scope_id} ) + canonical_scope_id = validate_access_scope_reference( + get_registry(), + tenant_id=tenant_id or principal.tenant_id, + scope_type=scope_type, + scope_id=scope_id, + ) + payload = payload.model_copy(update={"scope_id": canonical_scope_id}) definition = create_definition( session, tenant_id=tenant_id or principal.tenant_id, @@ -382,6 +434,26 @@ def api_update_definition( registry=get_registry(), action="edit", ) + tenant_id, scope_type, scope_id, _scope_key = normalize_definition_scope( + principal, + scope_type=payload.scope_type, + scope_id=payload.scope_id, + administrative=has_scope(principal, ADMIN_SCOPE), + ) + scope_id = validate_access_scope_reference( + get_registry(), + tenant_id=tenant_id or principal.tenant_id, + scope_type=scope_type, + scope_id=scope_id, + preserve_existing=( + existing.scope_id + if existing.scope_type == scope_type + else None + ), + ) + payload = payload.model_copy( + update={"scope_type": scope_type, "scope_id": scope_id} + ) definition = update_definition( session, tenant_id=principal.tenant_id, @@ -389,7 +461,7 @@ def api_update_definition( actor_id=_actor_id(principal), payload=payload, ) - except PermissionError as exc: + except (PermissionError, ValueError) as exc: raise _governance_http_error(exc) from exc except WorkflowError as exc: raise _http_error(exc) from exc @@ -432,6 +504,13 @@ def api_derive_definition( payload = payload.model_copy( update={"scope_type": scope_type, "scope_id": scope_id} ) + canonical_scope_id = validate_access_scope_reference( + get_registry(), + tenant_id=tenant_id or principal.tenant_id, + scope_type=scope_type, + scope_id=scope_id, + ) + payload = payload.model_copy(update={"scope_id": canonical_scope_id}) definition = derive_definition( session, tenant_id=tenant_id or principal.tenant_id, diff --git a/tests/test_governance.py b/tests/test_governance.py index 5d80484..119c54c 100644 --- a/tests/test_governance.py +++ b/tests/test_governance.py @@ -13,6 +13,7 @@ from govoplan_workflow.backend.db.models import ( WorkflowDefinition, WorkflowDefinitionRevision, ) +from govoplan_workflow.backend.governance import normalize_definition_scope from govoplan_workflow.backend.schemas import ( WorkflowDefinitionCreateRequest, WorkflowDefinitionDeriveRequest, @@ -167,6 +168,19 @@ class WorkflowGovernanceTests(unittest.TestCase): ) self.assertFalse(response.governance.automation_runtime_available) + def test_user_scope_normalizes_membership_to_account_id(self) -> None: + tenant_id, scope_type, scope_id, scope_key = normalize_definition_scope( + self.principal, + scope_type="user", + scope_id="membership-1", + administrative=False, + ) + + self.assertEqual("tenant-1", tenant_id) + self.assertEqual("user", scope_type) + self.assertEqual("account-1", scope_id) + self.assertEqual("user:account-1", scope_key) + def test_template_cannot_be_activated(self) -> None: template = create_definition( self.session, diff --git a/webui/package.json b/webui/package.json index 852d60e..f85240c 100644 --- a/webui/package.json +++ b/webui/package.json @@ -21,7 +21,7 @@ "lucide-react": "^1.23.0", "react": "^19.0.0", "react-dom": "^19.0.0", - "react-router-dom": "^7.1.1", + "react-router-dom": ">=7.18.2 <8", "typescript": "^5.7.2" }, "peerDependenciesMeta": { diff --git a/webui/src/api/workflow.ts b/webui/src/api/workflow.ts index 1c6bcda..4d8cbee 100644 --- a/webui/src/api/workflow.ts +++ b/webui/src/api/workflow.ts @@ -1,4 +1,9 @@ -import { apiFetch, type ApiSettings } from "@govoplan/core-webui"; +import { + apiFetch, + apiReferenceOptionProvider, + type ApiSettings, + type ReferenceOptionProvider +} from "@govoplan/core-webui"; import type { DefinitionGraph, DefinitionGraphNode, @@ -216,3 +221,14 @@ export function validateWorkflowDefinition( body: JSON.stringify({ graph }) }); } + +export function workflowScopeReferenceProvider( + settings: ApiSettings, + scopeType: "user" | "group" +): ReferenceOptionProvider { + return apiReferenceOptionProvider( + settings, + "/api/v1/workflow/scope-targets", + { scope_type: scopeType } + ); +} diff --git a/webui/src/features/workflow/WorkflowPage.tsx b/webui/src/features/workflow/WorkflowPage.tsx index 33c1cae..12c3b4b 100644 --- a/webui/src/features/workflow/WorkflowPage.tsx +++ b/webui/src/features/workflow/WorkflowPage.tsx @@ -26,6 +26,7 @@ import { FormField, IconButton, LoadingFrame, + ReferenceSelect, StatusBadge, ToggleSwitch, hasScope, @@ -46,6 +47,7 @@ import { listWorkflowRevisions, updateWorkflowDefinition, validateWorkflowDefinition, + workflowScopeReferenceProvider, type WorkflowDefinition, type WorkflowDiagnostic, type WorkflowNodeType, @@ -721,6 +723,7 @@ export default function WorkflowPage({ /> setDraft((current) => current ? { ...current, ...patch } : current)} @@ -751,18 +754,30 @@ export default function WorkflowPage({ function WorkflowDefinitionSettingsDialog({ open, + settings, draft, editable, onChange, onClose }: { open: boolean; + settings: ApiSettings; draft: WorkflowDraft | null; editable: boolean; onChange: (patch: Partial) => void; onClose: () => void; }) { const provenance = draft?.governance?.actions.edit?.source_path ?? []; + const referenceScopeType = draft?.scopeType === "group" ? "group" : "user"; + const scopeProvider = useMemo( + () => workflowScopeReferenceProvider(settings, referenceScopeType), + [ + referenceScopeType, + settings.accessToken, + settings.apiBaseUrl, + settings.apiKey + ] + ); return ( User - - onChange({ scopeId: event.target.value })} - /> - + > + onChange({ scopeId: value })} + provider={scopeProvider} + aria-label={ + draft.scopeType === "user" + ? "Definition user" + : "Definition group" + } + placeholder={ + draft.scopeType === "user" + ? "Select a user" + : "Select a group" + } + disabled={!editable || Boolean(draft.id)} + required + /> + + ) : null} + workflowScopeReferenceProvider( + settings, + scopeType === "group" ? "group" : "user" + ), + [ + scopeType, + settings.accessToken, + settings.apiBaseUrl, + settings.apiKey + ] + ); useEffect(() => { if (!open) return; @@ -954,7 +993,12 @@ function DeriveWorkflowDialog({ @@ -969,7 +1013,10 @@ function DeriveWorkflowDialog({ {scopeType !== "tenant" ? ( - - + setScopeId(event.target.value)} + onChange={(value) => setScopeId(value)} + provider={scopeProvider} + aria-label={ + scopeType === "user" ? "Copy target user" : "Copy target group" + } + placeholder={ + scopeType === "user" ? "Select a user" : "Select a group" + } + disabled={busy} + required /> ) : null}