From 50107a91ba81dbe35b7933aa4fe1bf95be8946a5 Mon Sep 17 00:00:00 2001 From: ValueOn AG Date: Sun, 19 Apr 2026 00:04:03 +0200 Subject: [PATCH 1/5] fixed proper splitting sysadmin/platformadmin and proper logic for mandate name(slug) and label(user) --- modules/auth/__init__.py | 9 +- modules/auth/authentication.py | 167 +++---- modules/datamodels/datamodelBilling.py | 2 +- modules/datamodels/datamodelUam.py | 100 ++++- modules/demoConfigs/investorDemo2026.py | 51 +-- modules/features/chatbot/mainChatbot.py | 9 +- .../features/chatbot/routeFeatureChatbot.py | 2 +- .../realEstate/routeFeatureRealEstate.py | 2 +- .../features/teamsbot/routeFeatureTeamsbot.py | 4 +- .../features/trustee/routeFeatureTrustee.py | 10 +- modules/interfaces/interfaceBootstrap.py | 418 +++++++++--------- modules/interfaces/interfaceDbApp.py | 150 ++++++- modules/interfaces/interfaceDbManagement.py | 8 +- modules/routes/routeAdminDatabaseHealth.py | 10 +- modules/routes/routeAdminDemoConfig.py | 8 +- modules/routes/routeAdminFeatures.py | 61 +-- modules/routes/routeAdminLogs.py | 6 +- modules/routes/routeAdminRbacRules.py | 28 +- .../routes/routeAdminUserAccessOverview.py | 11 +- modules/routes/routeAudit.py | 3 +- modules/routes/routeBilling.py | 15 +- modules/routes/routeDataFiles.py | 7 +- modules/routes/routeDataMandates.py | 141 ++++-- modules/routes/routeDataSources.py | 3 +- modules/routes/routeDataUsers.py | 105 +++-- modules/routes/routeI18n.py | 50 ++- modules/routes/routeInvitations.py | 4 +- modules/routes/routeRealEstate.py | 2 +- modules/routes/routeSecurityLocal.py | 14 +- modules/routes/routeStore.py | 10 +- modules/routes/routeSubscription.py | 6 +- modules/routes/routeSystem.py | 2 +- modules/routes/routeWorkflowDashboard.py | 16 +- modules/security/rbacCatalog.py | 34 +- modules/shared/mandateNameUtils.py | 121 +++++ modules/system/mainSystem.py | 4 +- modules/system/registry.py | 4 +- scripts/check_db_no_sysadmin_role.py | 132 ++++++ scripts/check_no_sysadmin_role.py | 108 +++++ tests/integration/mandates/__init__.py | 0 .../mandates/test_createMandate.py | 190 ++++++++ .../mandates/test_provisionMandate.py | 109 +++++ .../mandates/test_updateMandate.py | 215 +++++++++ .../rbac/test_platform_admin_flag.py | 290 ++++++++++++ tests/integration/users/__init__.py | 0 tests/integration/users/test_updateUser.py | 221 +++++++++ tests/test_phase123_basic.py | 2 +- tests/unit/bootstrap/__init__.py | 0 .../bootstrap/test_mandateNameMigration.py | 133 ++++++ tests/unit/rbac/test_sysadmin_migration.py | 209 +++++++++ tests/unit/shared/test_mandateNameUtils.py | 56 +++ 51 files changed, 2673 insertions(+), 589 deletions(-) create mode 100644 modules/shared/mandateNameUtils.py create mode 100644 scripts/check_db_no_sysadmin_role.py create mode 100644 scripts/check_no_sysadmin_role.py create mode 100644 tests/integration/mandates/__init__.py create mode 100644 tests/integration/mandates/test_createMandate.py create mode 100644 tests/integration/mandates/test_provisionMandate.py create mode 100644 tests/integration/mandates/test_updateMandate.py create mode 100644 tests/integration/rbac/test_platform_admin_flag.py create mode 100644 tests/integration/users/__init__.py create mode 100644 tests/integration/users/test_updateUser.py create mode 100644 tests/unit/bootstrap/__init__.py create mode 100644 tests/unit/bootstrap/test_mandateNameMigration.py create mode 100644 tests/unit/rbac/test_sysadmin_migration.py create mode 100644 tests/unit/shared/test_mandateNameUtils.py diff --git a/modules/auth/__init__.py b/modules/auth/__init__.py index 28bc1ed2..0a485767 100644 --- a/modules/auth/__init__.py +++ b/modules/auth/__init__.py @@ -7,7 +7,10 @@ High-level security functionality that depends on FastAPI and interfaces. Multi-Tenant Design: - RequestContext: Per-request context with user, mandate, feature instance, roles - getRequestContext: FastAPI dependency to extract context from X-Mandate-Id header -- requireSysAdmin: FastAPI dependency for system-level admin operations +- requireSysAdmin: FastAPI dependency for INFRASTRUCTURE-level operations + (logs, tokens, DB-health, i18n-master). Includes RBAC bypass. +- requirePlatformAdmin: FastAPI dependency for CROSS-MANDATE GOVERNANCE + (user-/mandate-/RBAC-/feature-registry mgmt). No bypass. """ from .authentication import ( @@ -19,7 +22,7 @@ from .authentication import ( RequestContext, getRequestContext, requireSysAdmin, - requireSysAdminRole, + requirePlatformAdmin, ) from .jwtService import ( createAccessToken, @@ -45,7 +48,7 @@ __all__ = [ "RequestContext", "getRequestContext", "requireSysAdmin", - "requireSysAdminRole", + "requirePlatformAdmin", # JWT Service "createAccessToken", "createRefreshToken", diff --git a/modules/auth/authentication.py b/modules/auth/authentication.py index d15ff2fd..27cf1a31 100644 --- a/modules/auth/authentication.py +++ b/modules/auth/authentication.py @@ -272,7 +272,6 @@ class RequestContext: # Request-scoped cache: rules loaded only once per request self._cachedRules: Optional[List[tuple]] = None - self._cachedHasSysAdminRole: Optional[bool] = None def getRules(self) -> List[tuple]: """ @@ -299,18 +298,17 @@ class RequestContext: @property def isSysAdmin(self) -> bool: - """Convenience property to check if user has the isSysAdmin FLAG. - Category A only: true system operations (tokens, logs, databases).""" + """Convenience property: Infrastructure/System Operator flag. + For Category A (Logs, Tokens, DB-Health, i18n-Master, Registry). + Wirkt auch als RBAC-Engine-Bypass (siehe rbac.py:getUserPermissions).""" return getattr(self.user, 'isSysAdmin', False) - - @property - def hasSysAdminRole(self) -> bool: - """Check if user has sysadmin ROLE in root mandate (cached per request). - Use for admin operations (Categories B/C/D/E) instead of isSysAdmin flag.""" - if self._cachedHasSysAdminRole is None: - self._cachedHasSysAdminRole = _hasSysAdminRole(str(self.user.id)) - return self._cachedHasSysAdminRole + @property + def isPlatformAdmin(self) -> bool: + """Convenience property: Cross-Mandate-Governance flag. + For Categories B–E (User-/Mandate-/RBAC-/Feature-Registry über alle Mandanten). + KEIN RBAC-Bypass — Daten-Zugriff geht weiterhin über Mandanten-Mitgliedschaft.""" + return getattr(self.user, 'isPlatformAdmin', False) def getRequestContext( request: Request, @@ -323,33 +321,37 @@ def getRequestContext( Checks authorization and loads role IDs. Security Model: - - Regular users: Must be explicit members of mandates/feature instances - - SysAdmin users: Can access ANY mandate for administrative operations. - Root mandate roles (incl. sysadmin role) are loaded for RBAC-based authorization. - Routes use ctx.hasSysAdminRole for admin checks (not ctx.isSysAdmin flag). - + - Regular users: Must be explicit members of mandates/feature instances. + - isSysAdmin users: RBAC-Engine-Bypass; können jeden Mandant für + Infrastruktur-Operationen betreten ohne Mitgliedschaft. ``ctx.roleIds`` + bleibt leer (Bypass läuft direkt in ``rbac.py:getUserPermissions``). + - isPlatformAdmin users: Cross-Mandate-Governance; können jeden Mandant + betreten, aber Routen prüfen die Berechtigung explizit via + ``requirePlatformAdmin``. ``ctx.roleIds`` bleibt leer. + Args: request: FastAPI Request object mandateId: Mandate ID from X-Mandate-Id header featureInstanceId: Feature instance ID from X-Instance-Id header currentUser: Current authenticated user - + Returns: RequestContext with user, mandate, roles - + Raises: - HTTPException 403: If non-SysAdmin user is not member of mandate or has no feature access + HTTPException 403: If user is not member of mandate (and not Sys/Platform admin) """ ctx = RequestContext(user=currentUser) isSysAdmin = getattr(currentUser, 'isSysAdmin', False) - + isPlatformAdmin = getattr(currentUser, 'isPlatformAdmin', False) + # Get root interface for membership checks rootInterface = getRootInterface() - + if mandateId: # Check mandate membership membership = rootInterface.getUserMandate(currentUser.id, mandateId) - + if membership: # User is a member - load their roles if not membership.enabled: @@ -359,12 +361,16 @@ def getRequestContext( ) ctx.mandateId = mandateId ctx.roleIds = rootInterface.getRoleIdsForUserMandate(membership.id) - elif isSysAdmin: - # SysAdmin can access any mandate for admin operations - # Load root mandate roles for RBAC-based authorization (includes sysadmin role) + elif isSysAdmin or isPlatformAdmin: + # Platform-level authority can enter any mandate without membership. + # No fake role loading: isSysAdmin bypasses RBAC engine; platform-admin + # routes verify authority explicitly via requirePlatformAdmin. ctx.mandateId = mandateId - ctx.roleIds = _getRootMandateRoleIds(rootInterface, str(currentUser.id)) - logger.debug(f"SysAdmin {currentUser.id} accessing mandate {mandateId} with root mandate roles") + ctx.roleIds = [] + logger.debug( + f"Platform-level user {currentUser.id} accessing mandate {mandateId} " + f"(isSysAdmin={isSysAdmin}, isPlatformAdmin={isPlatformAdmin})" + ) else: # Regular user without membership - denied logger.warning(f"User {currentUser.id} is not member of mandate {mandateId}") @@ -372,11 +378,11 @@ def getRequestContext( status_code=status.HTTP_403_FORBIDDEN, detail="Not member of mandate" ) - + if featureInstanceId: # Check feature access access = rootInterface.getFeatureAccess(currentUser.id, featureInstanceId) - + if access: # User has access - load their instance roles if not access.enabled: @@ -387,13 +393,15 @@ def getRequestContext( ctx.featureInstanceId = featureInstanceId instanceRoleIds = rootInterface.getRoleIdsForFeatureAccess(access.id) ctx.roleIds.extend(instanceRoleIds) - elif isSysAdmin: - # SysAdmin can access any feature instance for admin operations + elif isSysAdmin or isPlatformAdmin: + # Platform-level authority can enter any feature instance without + # explicit access record. ctx.featureInstanceId = featureInstanceId - # If no roles loaded yet, load root mandate roles - if not ctx.roleIds: - ctx.roleIds = _getRootMandateRoleIds(rootInterface, str(currentUser.id)) - logger.debug(f"SysAdmin {currentUser.id} accessing feature instance {featureInstanceId} with root mandate roles") + logger.debug( + f"Platform-level user {currentUser.id} accessing feature instance " + f"{featureInstanceId} (isSysAdmin={isSysAdmin}, " + f"isPlatformAdmin={isPlatformAdmin})" + ) else: # Regular user without access - denied logger.warning(f"User {currentUser.id} has no access to feature instance {featureInstanceId}") @@ -401,7 +409,7 @@ def getRequestContext( status_code=status.HTTP_403_FORBIDDEN, detail="No access to feature instance" ) - + return ctx @@ -444,95 +452,46 @@ def requireSysAdmin(currentUser: User = Depends(getCurrentUser)) -> User: # ============================================================================= -# SYSADMIN ROLE: RBAC-based admin checks (hybrid model) +# PLATFORM ADMIN: Flag-based cross-mandate governance (replaces sysadmin role) # ============================================================================= -def _getRootMandateRoleIds(rootInterface, userId: str) -> List[str]: +def requirePlatformAdmin(currentUser: User = Depends(getCurrentUser)) -> User: """ - Load the user's role IDs from the root mandate. - Used by auth middleware to provide RBAC roles for SysAdmin cross-mandate access. - - Args: - rootInterface: Root database interface - userId: User ID - - Returns: - List of role IDs from root mandate membership, empty list if no membership - """ - try: - rootMandateId = rootInterface._getRootMandateId() - if not rootMandateId: - return [] - membership = rootInterface.getUserMandate(userId, rootMandateId) - if not membership: - return [] - return rootInterface.getRoleIdsForUserMandate(membership.id) - except Exception as e: - logger.error(f"Error loading root mandate roles: {e}") - return [] + Require Platform-Admin flag for cross-mandate governance operations. + Verwendung für alle Operationen, die mandanten-übergreifend wirken: + User-Mgmt, Mandate-Mgmt, RBAC-Catalog, Feature-Registry, User-Access-Overview, + Cross-Mandate-Audit, Cross-Mandate-Billing-Übersicht, Subscription-Mgmt. -def _hasSysAdminRole(userId: str) -> bool: - """ - Check if a user has the sysadmin role in the root mandate. - - Standalone check that queries the database directly, independent of - request context. Used for authorization checks where the sysadmin - ROLE (not just the isSysAdmin flag) is required. - - Args: - userId: User ID to check - - Returns: - True if user has sysadmin role in root mandate - """ - try: - rootInterface = getRootInterface() - roleIds = _getRootMandateRoleIds(rootInterface, str(userId)) - for roleId in roleIds: - role = rootInterface.getRole(roleId) - if role and role.roleLabel == "sysadmin": - return True - return False - except Exception as e: - logger.error(f"Error checking sysadmin role: {e}") - return False + KEIN RBAC-Bypass: Daten-Zugriff auf einen einzelnen Mandanten erfordert + weiterhin Mitgliedschaft (oder zusätzlich isSysAdmin für Infrastruktur-Bypass). - -def requireSysAdminRole(currentUser: User = Depends(getCurrentUser)) -> User: - """ - Require sysadmin ROLE for admin operations. - - Unlike requireSysAdmin (which checks the isSysAdmin FLAG for system-level ops), - this dependency checks the sysadmin ROLE in the root mandate. - Use for admin operations that should be RBAC-controlled (Category E). - Args: currentUser: Current authenticated user - + Returns: - User if they have the sysadmin role - + User if they have isPlatformAdmin=True + Raises: - HTTPException 403: If user doesn't have sysadmin role + HTTPException 403: If user is not a Platform Admin """ - if not _hasSysAdminRole(str(currentUser.id)): + if not getattr(currentUser, 'isPlatformAdmin', False): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail="SysAdmin role required" + detail="Platform admin privileges required" ) - - # Audit + + # Audit for all Platform-Admin actions try: from modules.shared.auditLogger import audit_logger audit_logger.logSecurityEvent( userId=str(currentUser.id), mandateId="system", - action="sysadmin_role_action", - details="Admin operation via sysadmin role" + action="platform_admin_action", + details="Cross-mandate governance operation" ) except Exception: pass - + return currentUser diff --git a/modules/datamodels/datamodelBilling.py b/modules/datamodels/datamodelBilling.py index 8718413c..f662e28c 100644 --- a/modules/datamodels/datamodelBilling.py +++ b/modules/datamodels/datamodelBilling.py @@ -138,7 +138,7 @@ class BillingSettings(BaseModel): warningThresholdPercent: float = Field( default=10.0, - description="Benachrichtigung wenn das AI-Guthaben unter diesen Prozentsatz des Gesamtbudgets fällt", + description="Warning threshold as percentage", json_schema_extra={"label": "Warnschwelle (%)"}, ) diff --git a/modules/datamodels/datamodelUam.py b/modules/datamodels/datamodelUam.py index 9278ceb3..a73b4746 100644 --- a/modules/datamodels/datamodelUam.py +++ b/modules/datamodels/datamodelUam.py @@ -6,7 +6,15 @@ UAM models: User, Mandate, UserConnection. Multi-Tenant Design: - User gehört NICHT direkt zu einem Mandanten - Zugehörigkeit wird über UserMandate gesteuert (siehe datamodelMembership.py) -- isSysAdmin ist globales Admin-Flag für System-Zugriff (KEIN Daten-Zugriff!) +- Zwei orthogonale Plattform-Autoritäts-Flags: + * isSysAdmin → Infrastruktur-Operator (Logs, Tokens, DB-Health, + i18n-Master, Registry). RBAC-Engine-Bypass. + KEIN Cross-Mandate-Governance. + * isPlatformAdmin → Cross-Mandate-Governance (User-/Mandate-/RBAC-/ + Feature-Verwaltung über alle Mandanten). + KEIN RBAC-Bypass. + Beide einzeln vergebbar, einzeln auditierbar. + Siehe wiki/c-work/4-done/2026-04-sysadmin-authority-split.md """ import uuid @@ -15,6 +23,7 @@ from enum import Enum from pydantic import BaseModel, Field, EmailStr, field_validator, computed_field from modules.datamodels.datamodelBase import PowerOnModel from modules.shared.i18nRegistry import i18nModel, normalizePrimaryLanguageTag +from modules.shared.mandateNameUtils import MANDATE_NAME_MAX_LEN, MANDATE_NAME_MIN_LEN from modules.shared.timeUtils import getUtcTimestamp @@ -66,6 +75,11 @@ class Mandate(PowerOnModel): """ Mandate (Mandant/Tenant) model. Ein Mandant ist ein isolierter Bereich für Daten und Berechtigungen. + + Semantik: + - ``name`` (Kurzzeichen): plattformweit eindeutiger, stabiler technischer Code (Slug), + Audit-/Referenz-Identifier. Nur Kleinbuchstaben, Ziffern und ``-`` (Länge 2–32). + - ``label`` (Voller Name): Anzeigename im UI, frei änderbar unabhängig vom Slug. """ id: str = Field( default_factory=lambda: str(uuid.uuid4()), @@ -73,13 +87,26 @@ class Mandate(PowerOnModel): json_schema_extra={"frontend_type": "text", "frontend_readonly": True, "frontend_visible": False, "frontend_required": False, "label": "ID"}, ) name: str = Field( - description="Name of the mandate", - json_schema_extra={"frontend_type": "text", "frontend_readonly": False, "frontend_required": True, "label": "Name"}, + description="Unique stable mandate code (slug); lowercase, digits, hyphen segments only.", + min_length=MANDATE_NAME_MIN_LEN, + max_length=MANDATE_NAME_MAX_LEN, + pattern=r"^[a-z0-9]+(-[a-z0-9]+)*$", + json_schema_extra={ + "frontend_type": "slug", + "frontend_readonly": False, + "frontend_required": True, + "label": "Kurzzeichen", + }, ) - label: Optional[str] = Field( - default=None, - description="Display label of the mandate", - json_schema_extra={"frontend_type": "text", "frontend_readonly": False, "frontend_required": False, "label": "Label"}, + label: str = Field( + description="Human-readable mandate name shown in the UI (Voller Name).", + min_length=1, + json_schema_extra={ + "frontend_type": "text", + "frontend_readonly": False, + "frontend_required": True, + "label": "Voller Name", + }, ) enabled: bool = Field( default=True, @@ -105,6 +132,30 @@ class Mandate(PowerOnModel): return False return v + @field_validator("name", mode="before") + @classmethod + def _stripName(cls, v): + if v is None: + return "" + if isinstance(v, str): + return v.strip() + return v + + @field_validator("label", mode="before") + @classmethod + def _coerceLabel(cls, v): + if v is None: + return "" + return v + + @field_validator("label") + @classmethod + def _validateMandateLabel(cls, v: str) -> str: + s = v.strip() + if len(s) < 1: + raise ValueError("Mandate Voller Name (label) must not be empty.") + return s + @i18nModel("Benutzerverbindung") class UserConnection(PowerOnModel): id: str = Field( @@ -224,8 +275,11 @@ class User(PowerOnModel): Multi-Tenant Design: - User gehört NICHT direkt zu einem Mandanten - Zugehörigkeit wird über UserMandate gesteuert (siehe datamodelMembership.py) - - Rollen werden über UserMandateRole gesteuert - - isSysAdmin = System-Zugriff, KEIN Daten-Zugriff + - Rollen werden über UserMandateRole gesteuert (mandanten-scoped) + - Plattform-Autorität via zwei orthogonalen Flags: + * isSysAdmin → Infrastruktur (Bypass der RBAC-Engine, KEIN + Cross-Mandate-Governance) + * isPlatformAdmin → Cross-Mandate-Governance (KEIN RBAC-Bypass) """ id: str = Field( default_factory=lambda: str(uuid.uuid4()), @@ -283,10 +337,15 @@ class User(PowerOnModel): isSysAdmin: bool = Field( default=False, - description="Global SysAdmin flag. SysAdmin = System-Zugriff, KEIN Daten-Zugriff!", + description=( + "Infrastructure/System Operator flag. Erlaubt RBAC-Engine-Bypass " + "und Zugriff auf Infrastruktur-Operationen (Logs, Tokens, DB-Health, " + "i18n-Master, Registry). Gibt KEIN Cross-Mandate-Governance-Recht " + "(dafür ist isPlatformAdmin zuständig)." + ), json_schema_extra={"frontend_type": "checkbox", "frontend_readonly": False, "frontend_required": False, "label": "System-Admin"}, ) - + @field_validator('isSysAdmin', mode='before') @classmethod def _coerceIsSysAdmin(cls, v): @@ -294,6 +353,25 @@ class User(PowerOnModel): if v is None: return False return v + + isPlatformAdmin: bool = Field( + default=False, + description=( + "Platform/Cross-Mandate Governance flag. Erlaubt mandanten-übergreifende " + "Verwaltungsoperationen (User-/Mandate-/RBAC-/Feature-Registry). " + "KEIN RBAC-Engine-Bypass und KEIN impliziter Zugriff auf Mandanten-Daten." + ), + json_schema_extra={"frontend_type": "checkbox", "frontend_readonly": False, "frontend_required": False, "label": "Plattform-Admin"}, + ) + + @field_validator('isPlatformAdmin', mode='before') + @classmethod + def _coerceIsPlatformAdmin(cls, v): + """Konvertiert None zu False (für bestehende DB-Einträge ohne isPlatformAdmin Feld).""" + if v is None: + return False + return v + authenticationAuthority: AuthAuthority = Field( default=AuthAuthority.LOCAL, diff --git a/modules/demoConfigs/investorDemo2026.py b/modules/demoConfigs/investorDemo2026.py index 179e574e..0490bd91 100644 --- a/modules/demoConfigs/investorDemo2026.py +++ b/modules/demoConfigs/investorDemo2026.py @@ -77,7 +77,7 @@ class InvestorDemo2026(_BaseDemoConfig): mandateIdAlpina = self._ensureMandate(db, _MANDATE_ALPINA, summary) userId = self._ensureUser(db, summary) - self._ensureRootMandateSysAdminRole(db, userId, summary) + self._ensurePlatformAdminFlag(db, userId, summary) if mandateIdHappy: self._ensureMembership(db, userId, mandateIdHappy, _MANDATE_HAPPYLIFE["label"], summary) @@ -195,47 +195,24 @@ class InvestorDemo2026(_BaseDemoConfig): summary["created"].append(f"User {_USER['fullName']}") return uid - def _ensureRootMandateSysAdminRole(self, db, userId: str, summary: Dict): - """Ensure the demo user is member of the root mandate with the sysadmin role. - Without this, hasSysAdminRole returns False and admin menus are hidden.""" - from modules.datamodels.datamodelUam import Mandate - from modules.datamodels.datamodelMembership import UserMandate, UserMandateRole - from modules.datamodels.datamodelRbac import Role + def _ensurePlatformAdminFlag(self, db, userId: str, summary: Dict): + """Ensure the demo user has isPlatformAdmin=True for cross-mandate governance. + Without this, the admin UI menus would be hidden.""" + from modules.datamodels.datamodelUam import UserInDB - rootMandates = db.getRecordset(Mandate, recordFilter={"name": "root", "isSystem": True}) - if not rootMandates: - summary["errors"].append("Root mandate not found — cannot assign sysadmin role") + existing = db.getRecord(UserInDB, userId) + if not existing: + summary["errors"].append(f"Demo user {userId} not found — cannot set isPlatformAdmin") return - rootMandateId = rootMandates[0].get("id") - - existing = db.getRecordset(UserMandate, recordFilter={"userId": userId, "mandateId": rootMandateId}) - if existing: - userMandateId = existing[0].get("id") - else: - um = UserMandate(userId=userId, mandateId=rootMandateId, enabled=True) - created = db.recordCreate(UserMandate, um) - userMandateId = created.get("id") - summary["created"].append("Membership -> root mandate") - logger.info(f"Created root mandate membership for {_USER['username']}") - - sysadminRoles = db.getRecordset(Role, recordFilter={"mandateId": rootMandateId, "roleLabel": "sysadmin"}) - if not sysadminRoles: - summary["errors"].append("sysadmin role not found in root mandate") + currentFlag = bool(existing.get("isPlatformAdmin", False)) if isinstance(existing, dict) else bool(getattr(existing, "isPlatformAdmin", False)) + if currentFlag: + summary["skipped"].append("isPlatformAdmin already set") return - sysadminRoleId = sysadminRoles[0].get("id") - existingRole = db.getRecordset(UserMandateRole, recordFilter={ - "userMandateId": userMandateId, - "roleId": sysadminRoleId, - }) - if not existingRole: - umr = UserMandateRole(userMandateId=userMandateId, roleId=sysadminRoleId) - db.recordCreate(UserMandateRole, umr) - summary["created"].append("SysAdmin role in root mandate") - logger.info(f"Assigned sysadmin role in root mandate for {_USER['username']}") - else: - summary["skipped"].append("SysAdmin role in root mandate exists") + db.recordModify(UserInDB, userId, {"isPlatformAdmin": True}) + summary["created"].append("isPlatformAdmin flag") + logger.info(f"Set isPlatformAdmin=True for {_USER['username']}") def _ensureMembership(self, db, userId: str, mandateId: str, mandateLabel: str, summary: Dict): from modules.datamodels.datamodelMembership import UserMandate, UserMandateRole diff --git a/modules/features/chatbot/mainChatbot.py b/modules/features/chatbot/mainChatbot.py index b50fef79..6f3b7dae 100644 --- a/modules/features/chatbot/mainChatbot.py +++ b/modules/features/chatbot/mainChatbot.py @@ -116,11 +116,18 @@ TEMPLATE_ROLES = [ def getFeatureDefinition() -> Dict[str, Any]: - """Return the feature definition for registration.""" + """Return the feature definition for registration. + + The chatbot feature is currently soft-disabled via ``enabled=False``: its + catalog objects, template roles and routes stay loaded so already-running + instances keep working, but it is filtered out of the Store and the + Admin Feature-Instances "Neue Instanz" selection list. + """ return { "code": FEATURE_CODE, "label": FEATURE_LABEL, "icon": FEATURE_ICON, + "enabled": False, } diff --git a/modules/features/chatbot/routeFeatureChatbot.py b/modules/features/chatbot/routeFeatureChatbot.py index fa7ab93c..06cf985d 100644 --- a/modules/features/chatbot/routeFeatureChatbot.py +++ b/modules/features/chatbot/routeFeatureChatbot.py @@ -102,7 +102,7 @@ def _validateInstanceAccess(instanceId: str, context: RequestContext) -> str: ) # Verify user has access to this instance - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: # Check if user has FeatureAccess for this instance featureAccesses = rootInterface.getFeatureAccessesForUser(str(context.user.id)) hasAccess = any( diff --git a/modules/features/realEstate/routeFeatureRealEstate.py b/modules/features/realEstate/routeFeatureRealEstate.py index 2df04323..a8da37b4 100644 --- a/modules/features/realEstate/routeFeatureRealEstate.py +++ b/modules/features/realEstate/routeFeatureRealEstate.py @@ -116,7 +116,7 @@ def _validateInstanceAccess(instanceId: str, context: RequestContext) -> str: status_code=400, detail=f"Instance '{instanceId}' is not a realestate instance" ) - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: featureAccesses = rootInterface.getFeatureAccessesForUser(str(context.user.id)) hasAccess = any( str(fa.featureInstanceId) == instanceId and fa.enabled diff --git a/modules/features/teamsbot/routeFeatureTeamsbot.py b/modules/features/teamsbot/routeFeatureTeamsbot.py index d316bde2..e5ed9425 100644 --- a/modules/features/teamsbot/routeFeatureTeamsbot.py +++ b/modules/features/teamsbot/routeFeatureTeamsbot.py @@ -138,7 +138,7 @@ def _validateInstanceAccess(instanceId: str, context: RequestContext) -> str: def _validateSessionOwnership(session: dict, context: RequestContext) -> None: """Raise 404 if the user does not own this session (sysAdmin bypasses).""" - if context.hasSysAdminRole: + if context.isPlatformAdmin: return if session.get("startedByUserId") != str(context.user.id): raise HTTPException(status_code=404, detail=f"Session '{session.get('id')}' not found") @@ -319,7 +319,7 @@ async def listSessions( """List sessions for a feature instance (filtered to own sessions unless sysAdmin).""" _validateInstanceAccess(instanceId, context) interface = _getInterface(context, instanceId) - userId = None if context.hasSysAdminRole else str(context.user.id) + userId = None if context.isPlatformAdmin else str(context.user.id) sessions = interface.getSessions(instanceId, includeEnded=includeEnded, userId=userId) return {"sessions": sessions} diff --git a/modules/features/trustee/routeFeatureTrustee.py b/modules/features/trustee/routeFeatureTrustee.py index c4c96017..73752788 100644 --- a/modules/features/trustee/routeFeatureTrustee.py +++ b/modules/features/trustee/routeFeatureTrustee.py @@ -104,7 +104,7 @@ def _validateInstanceAccess(instanceId: str, context: RequestContext) -> str: ) # Verify user has access to this instance - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: # Check if user has FeatureAccess for this instance featureAccesses = rootInterface.getFeatureAccessesForUser(str(context.user.id)) hasAccess = any( @@ -138,7 +138,7 @@ def getQuickActions( from .mainTrustee import QUICK_ACTIONS, QUICK_ACTION_CATEGORIES userRoleLabels: set = set() - if context.hasSysAdminRole: + if context.isPlatformAdmin: userRoleLabels.add("trustee-admin") else: rootInterface = getRootInterface() @@ -156,9 +156,9 @@ def getQuickActions( filteredActions = [] for action in QUICK_ACTIONS: required = set(action.get("requiredRoles", [])) - if not userRoleLabels and not context.hasSysAdminRole: + if not userRoleLabels and not context.isPlatformAdmin: continue - if context.hasSysAdminRole or required.intersection(userRoleLabels): + if context.isPlatformAdmin or required.intersection(userRoleLabels): resolved = { "id": action["id"], "label": resolveText(action.get("label", {})), @@ -1811,7 +1811,7 @@ def _validateInstanceAdmin(instanceId: str, context: RequestContext) -> str: mandateId = _validateInstanceAccess(instanceId, context) # SysAdmin role always has access - if context.hasSysAdminRole: + if context.isPlatformAdmin: return mandateId # Check for instance-roles.manage resource permission via AccessRules diff --git a/modules/interfaces/interfaceBootstrap.py b/modules/interfaces/interfaceBootstrap.py index 707d9acc..5a556616 100644 --- a/modules/interfaces/interfaceBootstrap.py +++ b/modules/interfaces/interfaceBootstrap.py @@ -7,7 +7,7 @@ Contains all bootstrap logic including mandate, users, and RBAC rules. Multi-Tenant Design: - Rollen werden mit Kontext erstellt (mandateId=None für globale Template-Rollen) - AccessRules referenzieren roleId (FK), nicht roleLabel -- Admin-User bekommt isSysAdmin=True statt roleLabels +- Admin-User bekommt isSysAdmin=True UND isPlatformAdmin=True (statt einer Rolle) """ import logging @@ -61,6 +61,7 @@ def initBootstrap(db: DatabaseConnector) -> None: # Migrate existing mandate records: description -> label _migrateMandateDescriptionToLabel(db) + _migrateMandateNameLabelSlugRules(db) # Clean up duplicate roles and fix corrupted templates FIRST _deduplicateRoles(db) @@ -75,12 +76,14 @@ def initBootstrap(db: DatabaseConnector) -> None: # This also serves as migration for existing mandates that don't have instance roles yet _ensureAllMandatesHaveSystemRoles(db) - # Initialize sysadmin role in root mandate (NOT a template, mandate-specific) - # Hybrid model: isSysAdmin flag → system ops, sysadmin role → admin ops via RBAC + # Migration: eliminate the legacy ``sysadmin`` role in root mandate + # (replaced by ``User.isPlatformAdmin`` flag — see + # wiki/c-work/4-done/2026-04-sysadmin-authority-split.md). + # Idempotent: noop after first successful run. if mandateId: - _initSysAdminRole(db, mandateId) - - # Ensure UI rules for sysadmin role (created after initRbacRules, needs second pass) + _migrateAndDropSysAdminRole(db, mandateId) + + # Ensure UI rules for navigation items (admin/user/viewer roles) _ensureUiContextRules(db) # Initialize admin user @@ -420,32 +423,101 @@ def _migrateMandateDescriptionToLabel(db: DatabaseConnector) -> None: logger.debug("No mandate description->label migration needed") +def _migrateMandateNameLabelSlugRules(db: DatabaseConnector) -> None: + """ + Migration: normalize Mandate.name to the slug rules ([a-z0-9-], length 2..32, single + hyphen segments) and ensure Mandate.label is non-empty. + + Rules (see wiki/c-work/1-plan/2026-04-mandate-name-label-logic.md): + 1. If ``label`` is empty/None → set ``label := name`` (or "Mandate" when both empty). + 2. If ``name`` is not a valid slug, or collides with an earlier mandate in stable id + order, allocate a unique slug from the (now non-empty) ``label`` using + ``slugifyMandateName`` + ``allocateUniqueMandateSlug``. + + Idempotent: a second run is a no-op because all valid names stay valid and stay unique. + Each rename and label fill-in is logged for audit. + """ + from modules.shared.mandateNameUtils import ( + allocateUniqueMandateSlug, + isValidMandateName, + slugifyMandateName, + ) + + allRows = db.getRecordset(Mandate) + if not allRows: + return + sortedRows = sorted(allRows, key=lambda r: str(r.get("id", ""))) + + used: set[str] = set() + labelFills = 0 + nameRenames: list[tuple[str, str, str]] = [] + + for rec in sortedRows: + mid = rec.get("id") + if not mid: + continue + name = (rec.get("name") or "").strip() + labelRaw = rec.get("label") + label = (labelRaw or "").strip() if labelRaw is not None else "" + + if not label: + label = name if name else "Mandate" + db.recordModify(Mandate, mid, {"label": label}) + labelFills += 1 + logger.info(f"Mandate {mid}: filled empty label with '{label}'") + + nameFits = isValidMandateName(name) + nameCollides = name in used + if nameFits and not nameCollides: + used.add(name) + continue + + base = slugifyMandateName(label) or "mn" + newName = allocateUniqueMandateSlug(base, used) + used.add(newName) + if newName != name: + db.recordModify(Mandate, mid, {"name": newName}) + nameRenames.append((str(mid), name, newName)) + logger.info(f"Mandate {mid}: renamed name '{name}' -> '{newName}'") + + if labelFills or nameRenames: + logger.info( + "Mandate name/label slug migration: %d label fill-in(s), %d name rename(s)", + labelFills, len(nameRenames), + ) + else: + logger.debug("No mandate name/label slug migration needed") + + def initAdminUser(db: DatabaseConnector, mandateId: Optional[str]) -> Optional[str]: """ Creates the Admin user if it doesn't exist. - Admin user gets isSysAdmin=True for system-level access. - Role assignment is done via UserMandate + UserMandateRole in assignInitialUserMemberships(). - + Admin user gets BOTH platform flags: + - isSysAdmin=True (Infrastructure: logs/tokens/DB-health) + - isPlatformAdmin=True (Cross-Mandate-Governance: user/mandate/RBAC mgmt) + Args: db: Database connector instance mandateId: Root mandate ID (for membership assignment, not on User) - + Returns: User ID if created or found, None otherwise """ existingUsers = db.getRecordset(UserInDB, recordFilter={"username": "admin"}) if existingUsers: userId = existingUsers[0].get("id") - existingIsSysAdmin = existingUsers[0].get("isSysAdmin", False) - - # Ensure admin user has isSysAdmin=True - if not existingIsSysAdmin: - logger.info(f"Updating admin user {userId} to set isSysAdmin=True") - db.recordModify(UserInDB, userId, {"isSysAdmin": True}) - + updates: Dict[str, bool] = {} + if not existingUsers[0].get("isSysAdmin", False): + updates["isSysAdmin"] = True + if not existingUsers[0].get("isPlatformAdmin", False): + updates["isPlatformAdmin"] = True + if updates: + logger.info(f"Updating admin user {userId} platform flags: {updates}") + db.recordModify(UserInDB, userId, updates) + logger.info(f"Admin user already exists with ID {userId}") return userId - + logger.info("Creating Admin user") adminUser = UserInDB( username="admin", @@ -454,6 +526,7 @@ def initAdminUser(db: DatabaseConnector, mandateId: Optional[str]) -> Optional[s enabled=True, language="en", isSysAdmin=True, + isPlatformAdmin=True, authenticationAuthority=AuthAuthority.LOCAL, hashedPassword=_getPasswordHash(APP_CONFIG.get("APP_INIT_PASS_ADMIN_SECRET")), ) @@ -466,22 +539,30 @@ def initAdminUser(db: DatabaseConnector, mandateId: Optional[str]) -> Optional[s def initEventUser(db: DatabaseConnector, mandateId: Optional[str]) -> Optional[str]: """ Creates the Event user if it doesn't exist. - Event user gets isSysAdmin=True for system operations. - Role assignment is done via UserMandate + UserMandateRole in assignInitialUserMemberships(). - + Event user gets isSysAdmin=True for infrastructure-level operations + (system events, internal callbacks). It does NOT need cross-mandate + governance, so isPlatformAdmin is left False. + Args: db: Database connector instance mandateId: Root mandate ID (for membership assignment, not on User) - + Returns: User ID if created or found, None otherwise """ existingUsers = db.getRecordset(UserInDB, recordFilter={"username": "event"}) if existingUsers: userId = existingUsers[0].get("id") + # Defensive: revoke any historic platform-admin grant on the event user + if existingUsers[0].get("isPlatformAdmin", False): + logger.warning( + f"Event user {userId} had isPlatformAdmin=True; " + f"revoking (event user is infrastructure-only)" + ) + db.recordModify(UserInDB, userId, {"isPlatformAdmin": False}) logger.info(f"Event user already exists with ID {userId}") return userId - + logger.info("Creating Event user") eventUser = UserInDB( username="event", @@ -490,6 +571,7 @@ def initEventUser(db: DatabaseConnector, mandateId: Optional[str]) -> Optional[s enabled=True, language="en", isSysAdmin=True, + isPlatformAdmin=False, authenticationAuthority=AuthAuthority.LOCAL, hashedPassword=_getPasswordHash(APP_CONFIG.get("APP_INIT_PASS_EVENT_SECRET")), ) @@ -503,20 +585,19 @@ def initRoles(db: DatabaseConnector) -> None: """ Initialize standard roles if they don't exist. Roles are created as GLOBAL (mandateId=None) template roles. - - NOTE: The "sysadmin" role is NOT a template - it's created separately in - _initSysAdminRole() as a root-mandate-specific role (isSystemRole=False). - These template roles (admin/user/viewer) are for mandate/feature-level access control. - + + NOTE: There is no platform-level "sysadmin" role any more — platform + authority lives on the User record via ``isSysAdmin`` and + ``isPlatformAdmin``. These template roles (admin/user/viewer) are + purely for mandate/feature-level access control. + Args: db: Database connector instance """ logger.info("Initializing roles") global _roleIdCache _roleIdCache = {} - - # Standard template roles for mandate/feature-level access - # NOTE: "sysadmin" role is created separately in _initSysAdminRole (root mandate only) + standardRoles = [ Role( roleLabel="admin", @@ -734,145 +815,99 @@ def copySystemRolesToMandate(db: DatabaseConnector, mandateId: str) -> int: return copiedCount -def _initSysAdminRole(db: DatabaseConnector, mandateId: str) -> Optional[str]: +def _migrateAndDropSysAdminRole(db: DatabaseConnector, mandateId: str) -> None: """ - Initialize the sysadmin role in the root mandate. - - The sysadmin role is a mandate-specific role (NOT a system template) that provides - full administrative access via RBAC. It only exists in the root mandate and is - NOT copied to other mandates (isSystemRole=False). - - Hybrid model: - - User.isSysAdmin flag → true system operations (Category A: tokens, logs, databases) - - sysadmin role → admin operations via RBAC (Categories B/C/D/E) - + One-shot migration: eliminate the legacy ``sysadmin`` role in the root mandate. + + Authority semantics moved to two orthogonal flags on User: + - ``isSysAdmin`` → Infrastructure-Operator (RBAC bypass) + - ``isPlatformAdmin`` → Cross-Mandate-Governance (no bypass) + + Migration steps (idempotent): + 1. Find sysadmin role(s) in root mandate. If none exist → done. + 2. For every UserMandateRole row referencing such a role: set + ``user.isPlatformAdmin = True`` (preserves cross-mandate authority). + 3. Delete those UserMandateRole rows. + 4. Delete AccessRules attached to the sysadmin role. + 5. Delete the sysadmin Role record. + Args: db: Database connector instance mandateId: Root mandate ID - - Returns: - Sysadmin role ID or None """ - # Check if sysadmin role already exists in root mandate - existingRoles = db.getRecordset( + sysadminRoles = db.getRecordset( Role, - recordFilter={"roleLabel": "sysadmin", "mandateId": mandateId, "featureInstanceId": None} + recordFilter={"roleLabel": "sysadmin", "mandateId": mandateId, "featureInstanceId": None}, ) - - if existingRoles: - sysadminRoleId = existingRoles[0].get("id") - logger.info(f"Sysadmin role already exists in root mandate with ID {sysadminRoleId}") - # Ensure AccessRules exist (migration safety) - _ensureSysAdminAccessRules(db, sysadminRoleId) - return sysadminRoleId - - # Create sysadmin role in root mandate - logger.info("Creating sysadmin role in root mandate") - sysadminRole = Role( - roleLabel="sysadmin", - description=coerce_text_multilingual("System-Administrator - Vollständiger administrativer Zugriff über alle Mandanten"), - mandateId=mandateId, - featureInstanceId=None, - featureCode=None, - isSystemRole=False # NOT a template → NOT copied to other mandates - ) - createdRole = db.recordCreate(Role, sysadminRole) - sysadminRoleId = createdRole.get("id") - logger.info(f"Created sysadmin role with ID {sysadminRoleId}") - - # Create AccessRules for sysadmin role - _createSysAdminAccessRules(db, sysadminRoleId) - - return sysadminRoleId - - -def _createSysAdminAccessRules(db: DatabaseConnector, sysadminRoleId: str) -> None: - """ - Create AccessRules for the sysadmin role. - - DATA + RESOURCE: generic item=None (full access). - UI: NO generic rule here — explicit ui.admin.* rules are created by - _ensureUiContextRules() (same logic as admin role). - - Args: - db: Database connector instance - sysadminRoleId: Sysadmin role ID - """ - rules = [ - # DATA: Full access to all data tables (generic rule, item=None) - AccessRule( - roleId=sysadminRoleId, - context=AccessRuleContext.DATA, - item=None, - view=True, - read=AccessLevel.ALL, - create=AccessLevel.ALL, - update=AccessLevel.ALL, - delete=AccessLevel.ALL, - ), - # RESOURCE: Access to all system resources (generic rule, item=None) - AccessRule( - roleId=sysadminRoleId, - context=AccessRuleContext.RESOURCE, - item=None, - view=True, - read=None, - create=None, - update=None, - delete=None, - ), - ] - - for rule in rules: - db.recordCreate(AccessRule, rule) - - logger.info(f"Created {len(rules)} AccessRules for sysadmin role (UI rules via _ensureUiContextRules)") - - -def _ensureSysAdminAccessRules(db: DatabaseConnector, sysadminRoleId: str) -> None: - """ - Ensure AccessRules exist for the sysadmin role (migration safety). - Creates missing rules without duplicating existing ones. - - Args: - db: Database connector instance - sysadminRoleId: Sysadmin role ID - """ - existingRules = db.getRecordset(AccessRule, recordFilter={"roleId": sysadminRoleId}) - - if not existingRules: - logger.info("No AccessRules found for sysadmin role, creating them") - _createSysAdminAccessRules(db, sysadminRoleId) + if not sysadminRoles: + logger.debug("Sysadmin role migration: no legacy sysadmin role present, nothing to do") return - - # Check for DATA and RESOURCE contexts (UI is handled by _ensureUiContextRules) - existingContexts = {r.get("context") for r in existingRules} - - missingRules = [] - if AccessRuleContext.DATA.value not in existingContexts: - missingRules.append(AccessRule( - roleId=sysadminRoleId, - context=AccessRuleContext.DATA, - item=None, - view=True, - read=AccessLevel.ALL, - create=AccessLevel.ALL, - update=AccessLevel.ALL, - delete=AccessLevel.ALL, - )) - if AccessRuleContext.RESOURCE.value not in existingContexts: - missingRules.append(AccessRule( - roleId=sysadminRoleId, - context=AccessRuleContext.RESOURCE, - item=None, - view=True, - read=None, create=None, update=None, delete=None, - )) - - if missingRules: - for rule in missingRules: - db.recordCreate(AccessRule, rule) - logger.info(f"Created {len(missingRules)} missing AccessRules for sysadmin role") + + sysadminRoleIds = [str(r.get("id")) for r in sysadminRoles if r.get("id")] + logger.warning( + f"Sysadmin role migration: found {len(sysadminRoleIds)} legacy sysadmin role(s) " + f"in root mandate, migrating to isPlatformAdmin flag" + ) + + # 1) Promote every holder to isPlatformAdmin=True + promoted = 0 + for sysadminRoleId in sysadminRoleIds: + umRoleRows = db.getRecordset( + UserMandateRole, recordFilter={"roleId": sysadminRoleId} + ) + userMandateIds = [str(r.get("userMandateId")) for r in umRoleRows if r.get("userMandateId")] + if not userMandateIds: + continue + + # Resolve userIds via UserMandate + userIds = set() + for umId in userMandateIds: + ums = db.getRecordset(UserMandate, recordFilter={"id": umId}) + for um in ums: + uid = um.get("userId") if isinstance(um, dict) else getattr(um, "userId", None) + if uid: + userIds.add(str(uid)) + + for userId in userIds: + users = db.getRecordset(UserInDB, recordFilter={"id": userId}) + if not users: + continue + current = users[0].get("isPlatformAdmin", False) + if not current: + db.recordModify(UserInDB, userId, {"isPlatformAdmin": True}) + promoted += 1 + logger.warning( + f"Sysadmin role migration: granted isPlatformAdmin=True to user {userId}" + ) + + # 2) Delete UserMandateRole rows + for umRow in umRoleRows: + rowId = umRow.get("id") if isinstance(umRow, dict) else getattr(umRow, "id", None) + if rowId: + try: + db.recordDelete(UserMandateRole, str(rowId)) + except Exception as e: + logger.error(f"Sysadmin role migration: failed to drop UserMandateRole {rowId}: {e}") + + # 3) Delete AccessRules + accessRules = db.getRecordset(AccessRule, recordFilter={"roleId": sysadminRoleId}) + for ar in accessRules: + arId = ar.get("id") if isinstance(ar, dict) else getattr(ar, "id", None) + if arId: + try: + db.recordDelete(AccessRule, str(arId)) + except Exception as e: + logger.error(f"Sysadmin role migration: failed to drop AccessRule {arId}: {e}") + + # 4) Delete the Role + try: + db.recordDelete(Role, sysadminRoleId) + except Exception as e: + logger.error(f"Sysadmin role migration: failed to drop Role {sysadminRoleId}: {e}") + + logger.warning( + f"Sysadmin role migration: completed; promoted {promoted} user(s) to isPlatformAdmin" + ) def _getRoleId(db: DatabaseConnector, roleLabel: str) -> Optional[str]: @@ -940,8 +975,9 @@ def _createDefaultRoleRules(db: DatabaseConnector) -> None: Create default role rules for generic access (item = null). Uses roleId instead of roleLabel. - NOTE: Sysadmin role rules are created separately in _initSysAdminRole(). - These default rules cover admin/user/viewer template roles. + NOTE: There is no sysadmin role any more — platform/infra authority is + governed by the ``isSysAdmin`` / ``isPlatformAdmin`` flags on the User + record. These default rules cover admin/user/viewer template roles. Args: db: Database connector instance @@ -991,15 +1027,16 @@ def _createTableSpecificRules(db: DatabaseConnector) -> None: These rules override generic rules for specific tables. Uses roleId instead of roleLabel. - NOTE: Sysadmin role rules are created separately in _initSysAdminRole(). - These table-specific rules cover admin/user/viewer template roles. + NOTE: There is no sysadmin role any more — platform/infra authority is + governed by the ``isSysAdmin`` / ``isPlatformAdmin`` flags on the User + record. These table-specific rules cover admin/user/viewer template roles. Args: db: Database connector instance """ tableRules = [] - # Get role IDs for template roles (sysadmin is a separate mandate-level role) + # Get role IDs for template roles (platform authority lives on User flags) adminId = _getRoleId(db, "admin") userId = _getRoleId(db, "user") viewerId = _getRoleId(db, "viewer") @@ -1470,8 +1507,7 @@ def _ensureUiContextRules(db: DatabaseConnector) -> None: mandateAdminRoleIds = [] mandateUserRoleIds = [] mandateViewerRoleIds = [] - sysadminRoleIds = [] - + mandateRoles = db.getRecordset( Role, recordFilter={"isSystemRole": False, "featureInstanceId": None} @@ -1487,12 +1523,12 @@ def _ensureUiContextRules(db: DatabaseConnector) -> None: mandateUserRoleIds.append(roleId) elif label == "viewer": mandateViewerRoleIds.append(roleId) - elif label == "sysadmin": - sysadminRoleIds.append(roleId) - - # All role IDs per level (template + mandate-instance) - # sysadmin gets ALL UI rules (admin-only + public) — same logic, explicit rules - allAdminRoleIds = ([adminId] if adminId else []) + mandateAdminRoleIds + sysadminRoleIds + + # All role IDs per level (template + mandate-instance). + # Admin-only navigation items are governed by these admin roles plus the + # ``isPlatformAdmin`` flag (checked in routes via requirePlatformAdmin), + # NOT by a dedicated platform-level role. + allAdminRoleIds = ([adminId] if adminId else []) + mandateAdminRoleIds allUserRoleIds = ([userId] if userId else []) + mandateUserRoleIds allViewerRoleIds = ([viewerId] if viewerId else []) + mandateViewerRoleIds @@ -1860,7 +1896,7 @@ def _createStoreResourceRules(db: DatabaseConnector) -> None: Store resources control which roles can activate features via the Store. - admin/user: view=True (can see and activate store features) - viewer: no store access - - sysadmin: covered by generic RESOURCE rule (item=None, view=True) + - isSysAdmin flag bypasses RBAC (rbac.py:getUserPermissions) Args: db: Database connector instance @@ -1998,9 +2034,11 @@ def assignInitialUserMemberships( Assign initial memberships to admin and event users via UserMandate + UserMandateRole. This is the NEW multi-tenant way of assigning roles. - Hybrid model: Initial users get BOTH the isSysAdmin flag (for system ops) - AND the "admin" + "sysadmin" roles in the root mandate (for RBAC-based admin ops). - + Initial users get the "admin" role in the root mandate. Platform-level + authority (cross-mandate governance + infrastructure ops) is conveyed via + the ``isSysAdmin`` / ``isPlatformAdmin`` flags on the User record itself + (see ``initAdminUser`` / ``initEventUser``). + Args: db: Database connector instance mandateId: Root mandate ID @@ -2018,13 +2056,7 @@ def assignInitialUserMemberships( if not adminRoleId: logger.warning(f"No mandate-level role found for mandate {mandateId}, skipping membership assignment") return - - # Find sysadmin role in root mandate (created by _initSysAdminRole) - sysadminRole = next((r for r in mandateRoles if r.get("roleLabel") == "sysadmin"), None) - sysadminRoleId = sysadminRole.get("id") if sysadminRole else None - if not sysadminRoleId: - logger.warning("Sysadmin role not found in root mandate - run _initSysAdminRole first") - + for userId, userName in [(adminUserId, "admin"), (eventUserId, "event")]: # Check if UserMandate already exists existingMemberships = db.getRecordset( @@ -2059,20 +2091,6 @@ def assignInitialUserMemberships( ) db.recordCreate(UserMandateRole, userMandateRole) logger.info(f"Assigned admin role to {userName} user in mandate") - - # Assign sysadmin role (in addition to admin role) - if sysadminRoleId: - existingSysadminRoles = db.getRecordset( - UserMandateRole, - recordFilter={"userMandateId": userMandateId, "roleId": sysadminRoleId} - ) - if not existingSysadminRoles: - sysadminMandateRole = UserMandateRole( - userMandateId=userMandateId, - roleId=sysadminRoleId - ) - db.recordCreate(UserMandateRole, sysadminMandateRole) - logger.info(f"Assigned sysadmin role to {userName} user in root mandate") def _getPasswordHash(password: Optional[str]) -> Optional[str]: diff --git a/modules/interfaces/interfaceDbApp.py b/modules/interfaces/interfaceDbApp.py index 4f43d0ca..2a88fecc 100644 --- a/modules/interfaces/interfaceDbApp.py +++ b/modules/interfaces/interfaceDbApp.py @@ -677,6 +677,7 @@ class AppObjects: externalUsername: str = None, externalEmail: str = None, isSysAdmin: bool = False, + isPlatformAdmin: bool = False, addExternalIdentityConnection: bool = True, ) -> User: """ @@ -714,6 +715,7 @@ class AppObjects: language=language, enabled=enabled, isSysAdmin=isSysAdmin, + isPlatformAdmin=isPlatformAdmin, authenticationAuthority=authenticationAuthority, hashedPassword=self._getPasswordHash(password) if password else None, ) @@ -755,15 +757,21 @@ class AppObjects: logger.error(f"Unexpected error creating user: {str(e)}") raise ValueError(f"Failed to create user: {str(e)}") - def updateUser(self, userId: str, updateData: Union[Dict[str, Any], User], allowSysAdminChange: bool = False) -> User: + def updateUser( + self, + userId: str, + updateData: Union[Dict[str, Any], User], + allowAdminFlagChange: bool = False, + ) -> User: """Update a user's information. - + Args: userId: ID of the user to update updateData: User data to update (dict or User model) - allowSysAdminChange: If True, allows changing isSysAdmin field. - Only set to True when called by a SysAdmin explicitly - changing another user's admin status. + allowAdminFlagChange: If True, allows changing the privileged platform + flags ``isSysAdmin`` and ``isPlatformAdmin``. + Only set to True when called by a Platform Admin + explicitly changing another user's admin status. """ try: # Get user @@ -771,20 +779,35 @@ class AppObjects: if not user: raise ValueError(f"User {userId} not found") - # Convert updateData to dict if it's a User model + # Convert updateData to dict if it's a User model. + # + # IMPORTANT: When the route layer passes a Pydantic ``User`` instance, + # ``model_dump()`` returns ALL fields — including those the client + # never sent — populated with Pydantic defaults (e.g. ``isSysAdmin=False``). + # That historical pattern caused silent flag flips on inline-toggles. + # + # The PUT route now ships a plain dict carrying ONLY the explicitly + # changed fields, so this branch should rarely fire; however internal + # callers (``disableUser`` / ``enableUser`` / migration scripts) still + # use dict-style partials and must remain partial-safe. if isinstance(updateData, User): - updateDict = updateData.model_dump() + updateDict = updateData.model_dump(exclude_unset=True) + # Fallback for legacy callers that constructed a fully-defaulted + # User: if nothing was marked as explicitly set, treat the dump + # as authoritative but DROP privileged flags unconditionally + # unless allowAdminFlagChange is True. + if not updateDict: + updateDict = updateData.model_dump() else: - updateDict = updateData.copy() if isinstance(updateData, dict) else updateData + updateDict = updateData.copy() if isinstance(updateData, dict) else dict(updateData) - # Remove id field from updateDict if present - we'll use userId from parameter updateDict.pop("id", None) - - # SECURITY: Protect sensitive fields from being overwritten by profile updates. - # These fields should only be changed explicitly by admins, not through - # profile forms where they might be sent as default values (e.g., isSysAdmin=False). - protectedFields = ["isSysAdmin"] - if not allowSysAdminChange: + + # SECURITY: Protect privileged platform flags from accidental + # overwrite via profile forms or partial payloads from clients + # whose model defaults could pull the value down to False. + protectedFields = ["isSysAdmin", "isPlatformAdmin"] + if not allowAdminFlagChange: for field in protectedFields: updateDict.pop(field, None) @@ -1456,16 +1479,56 @@ class AppObjects: return Mandate(**filteredMandates[0]) - def createMandate(self, name: str, label: str = None, enabled: bool = True) -> Mandate: + def _existingMandateNames(self, excludeId: Optional[str] = None) -> List[str]: + """Return all mandate.name values currently in the DB (optionally excluding one id).""" + out: List[str] = [] + for r in self.db.getRecordset(Mandate): + if excludeId and str(r.get("id")) == str(excludeId): + continue + n = r.get("name") + if n: + out.append(n) + return out + + def _generateUniqueMandateName(self, label: str, excludeId: Optional[str] = None) -> str: + """Generate a slug from *label* that is unique across all mandates (Phase 3 helper).""" + from modules.shared.mandateNameUtils import allocateUniqueMandateSlug, slugifyMandateName + + base = slugifyMandateName(label or "") + return allocateUniqueMandateSlug(base, self._existingMandateNames(excludeId=excludeId)) + + def createMandate(self, name: str = None, label: str = None, enabled: bool = True) -> Mandate: """ Creates a new mandate if user has permission. Automatically copies system template roles (admin, user, viewer) to the new mandate. + + ``label`` (Voller Name) is required (non-empty). If ``name`` (Kurzzeichen) is omitted or empty, + a unique slug is generated from the label; otherwise it is validated and uniqueness-checked. """ if not self.checkRbacPermission(Mandate, "create"): raise PermissionError("No permission to create mandates") - # Create mandate data using model - mandateData = Mandate(name=name, label=label, enabled=enabled) + from modules.shared.mandateNameUtils import isValidMandateName + + effLabel = (label or "").strip() if label is not None else "" + if not effLabel and name: + effLabel = (name or "").strip() + if not effLabel: + raise ValueError("Mandate label (Voller Name) is required") + + rawName = (name or "").strip() if name else "" + if not rawName: + rawName = self._generateUniqueMandateName(effLabel) + else: + if not isValidMandateName(rawName): + raise ValueError( + "Mandate Kurzzeichen must be 2–32 characters: lowercase a–z, digits, " + "hyphens only (single-hyphen segments)." + ) + if rawName in self._existingMandateNames(): + raise ValueError(f"Mandate Kurzzeichen '{rawName}' is already in use") + + mandateData = Mandate(name=rawName, label=effLabel, enabled=enabled) # Create mandate record createdRecord = self.db.recordCreate(Mandate, mandateData) @@ -1484,24 +1547,31 @@ class AppObjects: return Mandate(**createdRecord) - def _provisionMandateForUser(self, userId: str, mandateName: str, planKey: str) -> Dict[str, Any]: + def _provisionMandateForUser(self, userId: str, mandateLabel: str, planKey: str) -> Dict[str, Any]: """ Atomic provisioning: create Mandate + UserMandate + Subscription + auto-create FeatureInstances. Internal method — bypasses RBAC (used during registration when user has no permissions yet). + + ``mandateLabel`` is the display name (Voller Name); a unique slug ``name`` (Kurzzeichen) is derived. """ from modules.datamodels.datamodelSubscription import MandateSubscription, SubscriptionStatusEnum, BUILTIN_PLANS from modules.datamodels.datamodelFeatures import FeatureInstance from modules.interfaces.interfaceBootstrap import copySystemRolesToMandate from modules.interfaces.interfaceFeatures import getFeatureInterface from modules.system.registry import loadFeatureMainModules - plan = BUILTIN_PLANS.get(planKey) if not plan: raise ValueError(f"Unknown plan: {planKey}") + effLabel = (mandateLabel or "").strip() + if not effLabel: + raise ValueError("mandateLabel (Voller Name) is required for provisioning") + + uniqueName = self._generateUniqueMandateName(effLabel) + mandateData = Mandate( - name=mandateName, - label=mandateName, + name=uniqueName, + label=effLabel, enabled=True, isSystem=False, ) @@ -1674,7 +1744,17 @@ class AppObjects: return activated def updateMandate(self, mandateId: str, updateData: Dict[str, Any]) -> Mandate: - """Updates a mandate if user has access.""" + """ + Updates a mandate if user has access. + + Field-level rules: + - ``id`` always immutable. + - ``isSystem`` only sysadmin. + - ``name`` (Kurzzeichen) only platform/sysadmin; format and uniqueness are validated. + - ``label`` (Voller Name) must be non-empty if provided. + """ + from modules.shared.mandateNameUtils import isValidMandateName + try: # First check if user has permission to modify mandates if not self.checkRbacPermission(Mandate, "update", mandateId): @@ -1685,11 +1765,33 @@ class AppObjects: if not mandate: raise ValueError(f"Mandate {mandateId} not found") + _isSysAdmin = bool(getattr(self.currentUser, "isSysAdmin", False)) + _isPlatformAdmin = bool(getattr(self.currentUser, "isPlatformAdmin", False)) + _protectedFields = {"id"} - if not getattr(self.currentUser, "isSysAdmin", False): + if not _isSysAdmin: _protectedFields.add("isSystem") + if not (_isSysAdmin or _isPlatformAdmin): + _protectedFields.add("name") _sanitizedData = {k: v for k, v in updateData.items() if k not in _protectedFields} + if "name" in _sanitizedData: + newName = (_sanitizedData["name"] or "").strip() + if not isValidMandateName(newName): + raise ValueError( + "Mandate Kurzzeichen must be 2–32 characters: lowercase a–z, digits, " + "hyphens only (single-hyphen segments)." + ) + if newName != mandate.name and newName in self._existingMandateNames(excludeId=mandateId): + raise ValueError(f"Mandate Kurzzeichen '{newName}' is already in use") + _sanitizedData["name"] = newName + + if "label" in _sanitizedData: + newLabel = (_sanitizedData["label"] or "").strip() + if not newLabel: + raise ValueError("Mandate Voller Name (label) must not be empty.") + _sanitizedData["label"] = newLabel + # Update mandate data using model updatedData = mandate.model_dump() updatedData.update(_sanitizedData) diff --git a/modules/interfaces/interfaceDbManagement.py b/modules/interfaces/interfaceDbManagement.py index 957e8e11..1a27a8db 100644 --- a/modules/interfaces/interfaceDbManagement.py +++ b/modules/interfaces/interfaceDbManagement.py @@ -635,12 +635,8 @@ class ComponentObjects: # Prompt methods def _isSysAdmin(self) -> bool: - """Check if the current user has sysadmin role (or isSysAdmin flag as fallback).""" - from modules.auth.authentication import _hasSysAdminRole - userId = getattr(self.currentUser, 'id', None) - if userId and _hasSysAdminRole(str(userId)): - return True - return hasattr(self.currentUser, 'isSysAdmin') and self.currentUser.isSysAdmin + """Check if the current user has the isSysAdmin flag (infrastructure operator).""" + return bool(getattr(self.currentUser, 'isSysAdmin', False)) def _enrichPromptsWithPermissions(self, prompts: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """Enrich prompts with row-level _permissions based on ownership and isSystem flag. diff --git a/modules/routes/routeAdminDatabaseHealth.py b/modules/routes/routeAdminDatabaseHealth.py index a3e7a165..760ab53d 100644 --- a/modules/routes/routeAdminDatabaseHealth.py +++ b/modules/routes/routeAdminDatabaseHealth.py @@ -11,7 +11,7 @@ from fastapi import APIRouter, Depends, HTTPException, Request, status from pydantic import BaseModel, Field from modules.auth import limiter -from modules.auth.authentication import requireSysAdminRole +from modules.auth.authentication import requireSysAdmin from modules.datamodels.datamodelUam import User from modules.system.databaseHealth import ( _cleanAllOrphans, @@ -41,7 +41,7 @@ class OrphanCleanRequest(BaseModel): def getDatabaseTableStats( request: Request, db: Optional[str] = None, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requireSysAdmin), ) -> Dict[str, Any]: """Table statistics from pg_stat_user_tables (optional filter by database name).""" rows = _getTableStats(dbFilter=db) @@ -53,7 +53,7 @@ def getDatabaseTableStats( def getDatabaseOrphans( request: Request, db: Optional[str] = None, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requireSysAdmin), ) -> Dict[str, Any]: """FK orphan scan (optional filter by source database name).""" rows = _scanOrphans(dbFilter=db) @@ -65,7 +65,7 @@ def getDatabaseOrphans( def postDatabaseOrphansClean( request: Request, body: OrphanCleanRequest, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requireSysAdmin), ) -> Dict[str, Any]: """Delete orphaned rows for a single FK relationship.""" try: @@ -90,7 +90,7 @@ def postDatabaseOrphansClean( @limiter.limit("2/minute") def postDatabaseOrphansCleanAll( request: Request, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requireSysAdmin), ) -> Dict[str, Any]: """Run orphan cleanup for every relationship that currently has orphans.""" results: List[dict] = _cleanAllOrphans() diff --git a/modules/routes/routeAdminDemoConfig.py b/modules/routes/routeAdminDemoConfig.py index b85cc38c..d893c205 100644 --- a/modules/routes/routeAdminDemoConfig.py +++ b/modules/routes/routeAdminDemoConfig.py @@ -9,7 +9,7 @@ import logging from fastapi import APIRouter, Depends, HTTPException, Request, status from modules.auth import limiter -from modules.auth.authentication import requireSysAdminRole +from modules.auth.authentication import requirePlatformAdmin from modules.datamodels.datamodelUam import User from modules.security.rootAccess import getRootDbAppConnector @@ -25,7 +25,7 @@ router = APIRouter( @limiter.limit("30/minute") def listDemoConfigs( request: Request, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requirePlatformAdmin), ) -> dict: """List all available demo configurations.""" from modules.demoConfigs import _getAvailableDemoConfigs @@ -41,7 +41,7 @@ def listDemoConfigs( def loadDemoConfig( code: str, request: Request, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requirePlatformAdmin), ) -> dict: """Load (create) a demo configuration. Idempotent.""" from modules.demoConfigs import _getDemoConfigByCode @@ -66,7 +66,7 @@ def loadDemoConfig( def removeDemoConfig( code: str, request: Request, - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requirePlatformAdmin), ) -> dict: """Remove all data created by a demo configuration.""" from modules.demoConfigs import _getDemoConfigByCode diff --git a/modules/routes/routeAdminFeatures.py b/modules/routes/routeAdminFeatures.py index 5532406c..ba867780 100644 --- a/modules/routes/routeAdminFeatures.py +++ b/modules/routes/routeAdminFeatures.py @@ -20,7 +20,7 @@ from pydantic import BaseModel, Field from modules.datamodels.datamodelPagination import PaginationParams, PaginationMetadata, normalize_pagination_dict from modules.routes.routeHelpers import _applyFiltersAndSort, handleFilterValuesInMemory, handleIdsInMemory -from modules.auth import limiter, getRequestContext, RequestContext, requireSysAdminRole +from modules.auth import limiter, getRequestContext, RequestContext, requirePlatformAdmin from modules.datamodels.datamodelUam import User, UserInDB from modules.datamodels.datamodelFeatures import Feature, FeatureInstance from modules.interfaces.interfaceDbApp import getRootInterface @@ -95,11 +95,18 @@ def list_features( """ try: # Features come from the RBAC Catalog (registered at startup from feature containers) - # NOT from the database - features are code-defined, not user-created + # NOT from the database - features are code-defined, not user-created. + # Hide meta-features (instantiable=False, e.g. ``system``) and soft- + # disabled features (enabled=False) so they don't appear in selection + # dropdowns like Admin > Feature-Instanzen > Neue Instanz. catalogService = getCatalogService() features = catalogService.getFeatureDefinitions() + features = [ + f for f in features + if f.get("instantiable", True) and f.get("enabled", True) + ] return features - + except Exception as e: logger.error(f"Error listing features: {e}") raise HTTPException( @@ -351,7 +358,7 @@ def create_feature( code: str = Query(..., description="Unique feature code"), label: Dict[str, str] = None, icon: str = Query("mdi-puzzle", description="Icon identifier"), - sysAdmin: User = Depends(requireSysAdminRole) + sysAdmin: User = Depends(requirePlatformAdmin) ) -> Dict[str, Any]: """ Create a new feature definition. @@ -520,7 +527,7 @@ def get_feature_instance( # Verify mandate access (unless SysAdmin) if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") @@ -660,14 +667,14 @@ def delete_feature_instance( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") ) # Check mandate admin permission - if not _hasMandateAdminRole(context) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Mandate-Admin role required to delete feature instances") @@ -727,14 +734,14 @@ def updateFeatureInstance( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") ) # Check mandate admin permission - if not _hasMandateAdminRole(context) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Mandate-Admin role required to update feature instances") @@ -810,14 +817,14 @@ def sync_instance_roles( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") ) # Check admin permission (Mandate-Admin or Feature-Admin) - if not _hasMandateAdminRole(context) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Admin role required to sync roles") @@ -863,10 +870,14 @@ def _syncInstanceWorkflows( instances created before template workflows were defined, or when the initial copy failed silently. - SysAdmin only. + PlatformAdmin only. """ try: - requireSysAdminRole(context.user) + if not context.isPlatformAdmin: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Platform admin privileges required", + ) rootInterface = getRootInterface() featureInterface = getFeatureInterface(rootInterface.db) @@ -975,7 +986,7 @@ def list_template_roles( pagination: Optional[str] = Query(None, description="JSON-encoded PaginationParams"), mode: Optional[str] = Query(None, description="'filterValues' for distinct column values, 'ids' for all filtered IDs"), column: Optional[str] = Query(None, description="Column key (required when mode=filterValues)"), - sysAdmin: User = Depends(requireSysAdminRole), + sysAdmin: User = Depends(requirePlatformAdmin), ): """List global template roles with pagination support.""" try: @@ -1035,7 +1046,7 @@ def create_template_role( roleLabel: str = Query(..., description="Role label (e.g., 'admin', 'viewer')"), featureCode: str = Query(..., description="Feature code this role belongs to"), description: Dict[str, str] = None, - sysAdmin: User = Depends(requireSysAdminRole) + sysAdmin: User = Depends(requirePlatformAdmin) ) -> Dict[str, Any]: """ Create a global template role for a feature. @@ -1145,7 +1156,7 @@ def list_feature_instance_users( # Verify mandate access (unless SysAdmin) if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") @@ -1259,14 +1270,14 @@ def add_user_to_feature_instance( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") ) # Check admin permission - if not _hasMandateAdminRole(context) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Admin role required to add users to feature instances") @@ -1367,14 +1378,14 @@ def remove_user_from_feature_instance( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") ) # Check admin permission - if not _hasMandateAdminRole(context) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Admin role required to remove users from feature instances") @@ -1457,14 +1468,14 @@ def update_feature_instance_user_roles( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") ) # Check admin permission - if not _hasMandateAdminRole(context) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Admin role required to update user roles") @@ -1565,7 +1576,7 @@ def get_feature_instance_available_roles( # Verify mandate access if context.mandateId and str(instance.mandateId) != str(context.mandateId): - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Access denied to this feature instance") @@ -1668,7 +1679,7 @@ def _renameFeatureInstance( userId = str(context.user.id) isInstanceAdmin = False - if context.hasSysAdminRole: + if context.isPlatformAdmin: isInstanceAdmin = True else: from modules.datamodels.datamodelMembership import FeatureAccess, FeatureAccessRole @@ -1707,7 +1718,7 @@ def _hasMandateAdminRole(context: RequestContext) -> bool: A user is mandate admin if they have the 'admin' role at mandate level. """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return True if not context.roleIds: diff --git a/modules/routes/routeAdminLogs.py b/modules/routes/routeAdminLogs.py index 065eba9c..926c7370 100644 --- a/modules/routes/routeAdminLogs.py +++ b/modules/routes/routeAdminLogs.py @@ -12,7 +12,7 @@ import logging from datetime import datetime from fastapi import APIRouter, HTTPException, Depends, Request, Query from fastapi.responses import PlainTextResponse -from modules.auth import limiter, requireSysAdminRole +from modules.auth import limiter, requireSysAdmin from modules.shared.configuration import APP_CONFIG from modules.datamodels.datamodelUam import User @@ -63,7 +63,7 @@ def _readLastNLines(filePath: str, n: int) -> list[str]: def getLogEntries( request: Request, count: int = Query(default=200, ge=1, le=50000, description="Number of log entries to return"), - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requireSysAdmin), ) -> dict: """ Get the last N log entries from the gateway log files. @@ -104,7 +104,7 @@ def getLogEntries( def downloadLog( request: Request, count: int = Query(default=1000, ge=1, le=100000, description="Number of log entries to download"), - currentUser: User = Depends(requireSysAdminRole), + currentUser: User = Depends(requireSysAdmin), ) -> PlainTextResponse: """ Download the last N log entries as a plain text file. diff --git a/modules/routes/routeAdminRbacRules.py b/modules/routes/routeAdminRbacRules.py index ccd45b2f..5f3b5317 100644 --- a/modules/routes/routeAdminRbacRules.py +++ b/modules/routes/routeAdminRbacRules.py @@ -17,7 +17,7 @@ import logging import json import math -from modules.auth import limiter, getRequestContext, requireSysAdminRole, RequestContext +from modules.auth import limiter, getRequestContext, requirePlatformAdmin, RequestContext from modules.datamodels.datamodelUam import User, UserPermissions, AccessLevel from modules.datamodels.datamodelRbac import AccessRuleContext, AccessRule, Role from modules.datamodels.datamodelMembership import UserMandate @@ -242,7 +242,7 @@ def get_all_permissions( logger.debug(f"UI/RESOURCE permissions: User has {len(roleIds)} roles across all mandates") - if not roleIds and not reqContext.hasSysAdminRole: + if not roleIds and not reqContext.isPlatformAdmin: # No roles at all, return empty permissions for ctx in contextsToFetch: result[ctx.value.lower()] = {} @@ -362,7 +362,7 @@ def get_access_rules( - List of AccessRule objects """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -487,7 +487,7 @@ def get_access_rules_by_role( - List of AccessRule objects for the specified role """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -534,7 +534,7 @@ def get_access_rule( - AccessRule object """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -585,7 +585,7 @@ def create_access_rule( - Created AccessRule object """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -665,7 +665,7 @@ def update_access_rule( - Updated AccessRule object """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -753,7 +753,7 @@ def delete_access_rule( - Success message """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -836,7 +836,7 @@ def list_roles( - List of role dictionaries with role label, description, user count, and computed scopeType """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -1017,7 +1017,7 @@ def create_role( - Created role dictionary """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -1076,7 +1076,7 @@ def get_role( - Role dictionary """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -1137,7 +1137,7 @@ def update_role( - Updated role dictionary """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -1201,7 +1201,7 @@ def delete_role( - Success message """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin adminMandateIds = [] if isSysAdmin else _getAdminMandateIds(reqContext) if not isSysAdmin and not adminMandateIds: raise HTTPException(status_code=403, detail=routeApiMsg("Admin role required")) @@ -1357,7 +1357,7 @@ def getCatalogObjects( def cleanup_duplicate_access_rules( request: Request, dryRun: bool = Query(True, description="If true, only report duplicates without deleting"), - currentUser: User = Depends(requireSysAdminRole) + currentUser: User = Depends(requirePlatformAdmin) ) -> dict: """ Find and remove duplicate AccessRules. diff --git a/modules/routes/routeAdminUserAccessOverview.py b/modules/routes/routeAdminUserAccessOverview.py index ab04d085..4906c093 100644 --- a/modules/routes/routeAdminUserAccessOverview.py +++ b/modules/routes/routeAdminUserAccessOverview.py @@ -75,7 +75,7 @@ def _hasMandateAdminRole(context: RequestContext) -> bool: Loads roles independently from request context (context.roleIds may be empty when no X-Mandate-Id header is sent, e.g., on admin pages). """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return True try: rootInterface = getRootInterface() @@ -123,7 +123,7 @@ def listUsersForOverview( try: interface = getRootInterface() - if context.hasSysAdminRole and not context.mandateId: + if context.isPlatformAdmin and not context.mandateId: # SysAdmin without mandate context: all users allUsers = interface.getAllUsers() elif context.mandateId: @@ -164,6 +164,7 @@ def listUsersForOverview( "email": userData.get("email"), "fullName": userData.get("fullName"), "isSysAdmin": userData.get("isSysAdmin", False), + "isPlatformAdmin": userData.get("isPlatformAdmin", False), "enabled": userData.get("enabled", True), }) @@ -217,7 +218,7 @@ def getUserAccessOverview( interface = getRootInterface() # MandateAdmin: verify the requested user shares at least one admin mandate - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: # Get admin's mandate IDs adminMandateIds = [] userMandates = interface.getUserMandates(str(context.user.id)) @@ -258,6 +259,7 @@ def getUserAccessOverview( "email": user.email, "fullName": user.fullName, "isSysAdmin": user.isSysAdmin, + "isPlatformAdmin": getattr(user, "isPlatformAdmin", False), "enabled": user.enabled, } @@ -481,7 +483,8 @@ def getUserAccessOverview( return { "user": userInfo, - "isSysAdmin": False, + "isSysAdmin": bool(getattr(user, "isSysAdmin", False)), + "isPlatformAdmin": bool(getattr(user, "isPlatformAdmin", False)), "roles": allRoles, "mandates": mandatesInfo, "uiAccess": uiAccess, diff --git a/modules/routes/routeAudit.py b/modules/routes/routeAudit.py index 86fbcea3..a8ac4d72 100644 --- a/modules/routes/routeAudit.py +++ b/modules/routes/routeAudit.py @@ -131,8 +131,7 @@ def _enrichUserAndInstanceLabels( def _requireAuditAccess(context: RequestContext): """Raise 403 unless user has mandate-admin or compliance-viewer access.""" - from modules.auth.authentication import _hasSysAdminRole - if _hasSysAdminRole(str(context.user.id)): + if context.isPlatformAdmin: return from modules.interfaces.interfaceDbApp import getInterface diff --git a/modules/routes/routeBilling.py b/modules/routes/routeBilling.py index bd66abef..9b238df1 100644 --- a/modules/routes/routeBilling.py +++ b/modules/routes/routeBilling.py @@ -17,7 +17,7 @@ from datetime import date, datetime, timezone from pydantic import BaseModel, Field # Import auth module -from modules.auth import limiter, requireSysAdminRole, getRequestContext, RequestContext +from modules.auth import limiter, requirePlatformAdmin, getRequestContext, RequestContext # Import billing components from modules.interfaces.interfaceDbBilling import getInterface as getBillingInterface, _getRootInterface @@ -86,8 +86,7 @@ def _getBillingDataScope(user) -> BillingDataScope: """ scope = BillingDataScope(userId=user.id) - from modules.auth.authentication import _hasSysAdminRole - if _hasSysAdminRole(str(user.id)): + if bool(getattr(user, "isPlatformAdmin", False)): scope.isGlobalAdmin = True return scope @@ -141,8 +140,8 @@ def _getBillingDataScope(user) -> BillingDataScope: def _isAdminOfMandate(ctx: RequestContext, targetMandateId: str) -> bool: - """Check if user is SysAdmin or admin of the specified mandate.""" - if ctx.hasSysAdminRole: + """Check if user is PlatformAdmin or admin of the specified mandate.""" + if ctx.isPlatformAdmin: return True try: from modules.interfaces.interfaceDbApp import getRootInterface @@ -734,7 +733,7 @@ def addCredit( targetMandateId: str = Path(..., description="Mandate ID"), creditRequest: CreditAddRequest = Body(...), ctx: RequestContext = Depends(getRequestContext), - _admin = Depends(requireSysAdminRole) + _admin = Depends(requirePlatformAdmin) ): """ Add credit to a billing account (SysAdmin only). @@ -1461,7 +1460,7 @@ def getTransactionsAdmin( def getMandateViewBalances( request: Request, ctx: RequestContext = Depends(getRequestContext), - _admin = Depends(requireSysAdminRole) + _admin = Depends(requirePlatformAdmin) ): """ Get mandate-level balances (SysAdmin only). @@ -1484,7 +1483,7 @@ def getMandateViewTransactions( request: Request, limit: int = Query(default=100, ge=1, le=1000), ctx: RequestContext = Depends(getRequestContext), - _admin = Depends(requireSysAdminRole) + _admin = Depends(requirePlatformAdmin) ): """ Get all transactions across mandates (SysAdmin only). diff --git a/modules/routes/routeDataFiles.py b/modules/routes/routeDataFiles.py index 439bfce5..f5b6f3d4 100644 --- a/modules/routes/routeDataFiles.py +++ b/modules/routes/routeDataFiles.py @@ -8,7 +8,6 @@ import json # Import auth module from modules.auth import limiter, getCurrentUser, getRequestContext, RequestContext -from modules.auth.authentication import _hasSysAdminRole # Import interfaces import modules.interfaces.interfaceDbManagement as interfaceDbManagement @@ -545,7 +544,7 @@ def _updateFolderScope( validScopes = {"personal", "featureInstance", "mandate", "global"} if scope not in validScopes: raise HTTPException(status_code=400, detail=f"Invalid scope: {scope}. Must be one of {validScopes}") - if scope == "global" and not _hasSysAdminRole(context.user): + if scope == "global" and not context.isSysAdmin: raise HTTPException(status_code=403, detail=routeApiMsg("Only sysadmins can set global scope")) try: mgmt = interfaceDbManagement.getInterface( @@ -847,7 +846,7 @@ def updateFileScope( if scope not in validScopes: raise HTTPException(status_code=400, detail=f"Invalid scope: {scope}. Must be one of {validScopes}") - if scope == "global" and not context.hasSysAdminRole: + if scope == "global" and not context.isSysAdmin: raise HTTPException(status_code=403, detail=routeApiMsg("Only sysadmins can set global scope")) managementInterface = interfaceDbManagement.getInterface( @@ -1041,7 +1040,7 @@ def update_file( detail=f"File with ID {fileId} not found" ) - if safeData.get("scope") == "global" and not _hasSysAdminRole(str(currentUser.id)): + if safeData.get("scope") == "global" and not getattr(currentUser, "isSysAdmin", False): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Only sysadmins can set global scope"), diff --git a/modules/routes/routeDataMandates.py b/modules/routes/routeDataMandates.py index 3c3da3a5..e81f500f 100644 --- a/modules/routes/routeDataMandates.py +++ b/modules/routes/routeDataMandates.py @@ -5,7 +5,8 @@ Mandate routes for the backend API. Implements the endpoints for mandate management. MULTI-TENANT: -- Mandate CRUD is SysAdmin-only (mandates are system resources) +- Mandate create/delete and cross-mandate ops require PlatformAdmin +- Mandate read/update: PlatformAdmin or Mandate-Admin (label-only for the latter) - User management within mandates is Mandate-Admin (add/remove users) """ @@ -17,7 +18,7 @@ import json from pydantic import BaseModel, Field # Import auth module -from modules.auth import limiter, requireSysAdminRole, getRequestContext, getCurrentUser, RequestContext +from modules.auth import limiter, requirePlatformAdmin, getRequestContext, getCurrentUser, RequestContext # Import interfaces import modules.interfaces.interfaceDbApp as interfaceDbApp @@ -33,6 +34,8 @@ from modules.datamodels.datamodelPagination import PaginationParams, PaginatedRe from modules.routes.routeNotifications import create_access_change_notification from modules.serviceCenter.services.serviceSubscription.mainServiceSubscription import SubscriptionCapacityException from modules.shared.i18nRegistry import apiRouteContext +from modules.shared.mandateNameUtils import isValidMandateName + routeApiMsg = apiRouteContext("routeDataMandates") @@ -101,8 +104,8 @@ def get_mandates( """ try: # Check admin access - isSysAdmin = context.hasSysAdminRole - if not isSysAdmin: + isPlatformAdmin = context.isPlatformAdmin + if not isPlatformAdmin: adminMandateIds = _getAdminMandateIds(context) if not adminMandateIds: raise HTTPException( @@ -135,7 +138,7 @@ def get_mandates( if mode == "filterValues": if not column: raise HTTPException(status_code=400, detail="column parameter required for mode=filterValues") - if isSysAdmin: + if isPlatformAdmin: crossPagination = parseCrossFilterPagination(column, pagination) try: from fastapi.responses import JSONResponse @@ -155,7 +158,7 @@ def get_mandates( return handleFilterValuesInMemory(mandateItems, column, pagination) if mode == "ids": - if isSysAdmin: + if isPlatformAdmin: return handleIdsMode(appInterface.db, Mandate, pagination) else: mandateItems = [] @@ -165,7 +168,7 @@ def get_mandates( mandateItems.append(m.model_dump() if hasattr(m, 'model_dump') else m if isinstance(m, dict) else vars(m)) return handleIdsInMemory(mandateItems, pagination) - if isSysAdmin: + if isPlatformAdmin: result = appInterface.getAllMandates(pagination=paginationParams) else: allMandates = [] @@ -223,7 +226,7 @@ def get_mandate( try: mandateId = targetMandateId # Check access - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: adminMandateIds = _getAdminMandateIds(context) if mandateId not in adminMandateIds: raise HTTPException( @@ -254,37 +257,48 @@ def get_mandate( @limiter.limit("10/minute") def create_mandate( request: Request, - mandateData: dict = Body(..., description="Mandate data with at least 'name' field"), - currentUser: User = Depends(requireSysAdminRole) + mandateData: dict = Body(..., description="Mandate data: label (Voller Name) required unless name alone is provided; name (Kurzzeichen) optional — auto-generated from label if omitted"), + currentUser: User = Depends(requirePlatformAdmin) ) -> Mandate: """ Create a new mandate. - MULTI-TENANT: SysAdmin-only. + MULTI-TENANT: PlatformAdmin-only. """ try: logger.debug(f"Creating mandate with data: {mandateData}") - - # Validate required fields - name = mandateData.get('name') - if not name or (isinstance(name, str) and name.strip() == ''): + + labelRaw = mandateData.get("label") + nameRaw = mandateData.get("name") + labelStripped = str(labelRaw).strip() if labelRaw is not None else "" + if not labelStripped and nameRaw is not None: + labelStripped = str(nameRaw).strip() + if not labelStripped: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=routeApiMsg("Mandate name is required") + detail=routeApiMsg("Mandate Voller Name (label) is required"), ) - - # Get optional fields with defaults - label = mandateData.get('label') - enabled = mandateData.get('enabled', True) - + + nameToPass = None + if nameRaw is not None and str(nameRaw).strip() != "": + nameToPass = str(nameRaw).strip() + if not isValidMandateName(nameToPass): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg( + "Mandate Kurzzeichen (name) must be 2–32 characters: lowercase a–z, digits, hyphens only" + ), + ) + + enabled = mandateData.get("enabled", True) + appInterface = interfaceDbApp.getRootInterface() - - # Create mandate + newMandate = appInterface.createMandate( - name=name, - label=label, - enabled=enabled + name=nameToPass, + label=labelStripped, + enabled=bool(enabled) if enabled is not None else True, ) - + if not newMandate: raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, @@ -329,11 +343,22 @@ def create_mandate( except Exception as subErr: logger.error(f"Failed to create subscription for mandate {newMandate.id}: {subErr}") - logger.info(f"Mandate {newMandate.id} created by SysAdmin {currentUser.id}") - + logger.info(f"Mandate {newMandate.id} created by PlatformAdmin {currentUser.id}") + return newMandate except HTTPException: raise + except ValueError as ve: + logger.warning(f"Create mandate validation: {ve}") + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg(str(ve)), + ) + except PermissionError: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=routeApiMsg("No permission to create mandates"), + ) except Exception as e: logger.error(f"Error creating mandate: {str(e)}") raise HTTPException( @@ -374,14 +399,13 @@ def update_mandate( """ Update an existing mandate. MULTI-TENANT: - - SysAdmin: full update - - MandateAdmin: only label + - PlatformAdmin: full update (including Kurzzeichen name) + - MandateAdmin: only label (Voller Name) """ - from modules.auth import _hasSysAdminRole as _checkSysAdminRole userId = str(currentUser.id) - isSysAdmin = _checkSysAdminRole(userId) + isPlatformAdmin = bool(getattr(currentUser, "isPlatformAdmin", False)) - if not isSysAdmin: + if not isPlatformAdmin: if not _isUserAdminOfMandate(userId, mandateId): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -400,14 +424,38 @@ def update_mandate( detail=f"Mandate with ID {mandateId} not found" ) - if not isSysAdmin: + if not isPlatformAdmin: mandateData = {k: v for k, v in mandateData.items() if k in _MANDATE_ADMIN_EDITABLE_FIELDS} if not mandateData: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("No editable fields submitted") ) - + if "label" in mandateData: + lbl = mandateData["label"] + if lbl is None or str(lbl).strip() == "": + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg("Mandate Voller Name (label) must not be empty"), + ) + else: + if "name" in mandateData and mandateData["name"] is not None: + nm = str(mandateData["name"]).strip() + if nm and not isValidMandateName(nm): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg( + "Mandate Kurzzeichen (name) must be 2–32 characters: lowercase a–z, digits, hyphens only" + ), + ) + if "label" in mandateData and mandateData["label"] is not None: + lb = str(mandateData["label"]).strip() + if not lb: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg("Mandate Voller Name (label) must not be empty"), + ) + updatedMandate = appInterface.updateMandate(mandateId, mandateData) if not updatedMandate: @@ -416,11 +464,22 @@ def update_mandate( detail=routeApiMsg("Failed to update mandate") ) - logger.info(f"Mandate {mandateId} updated by user {currentUser.id} (sysadmin={isSysAdmin})") + logger.info(f"Mandate {mandateId} updated by user {currentUser.id} (platformAdmin={isPlatformAdmin})") return updatedMandate except HTTPException: raise + except ValueError as ve: + logger.warning(f"Update mandate validation: {ve}") + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg(str(ve)), + ) + except PermissionError: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=routeApiMsg("No permission to update mandate"), + ) except Exception as e: logger.error(f"Error updating mandate {mandateId}: {str(e)}") raise HTTPException( @@ -434,7 +493,7 @@ def delete_mandate( request: Request, mandateId: str = Path(..., description="ID of the mandate to delete"), force: bool = Query(False, description="Hard-delete with full cascade (irreversible)"), - currentUser: User = Depends(requireSysAdminRole) + currentUser: User = Depends(requirePlatformAdmin) ) -> Dict[str, Any]: """ Delete a mandate. @@ -507,7 +566,7 @@ def list_mandate_users( pagination: Optional pagination parameters (page, pageSize, search, filters, sort) """ # Check permission - if not _hasMandateAdminRole(context, targetMandateId) and not context.hasSysAdminRole: + if not _hasMandateAdminRole(context, targetMandateId) and not context.isPlatformAdmin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Mandate-Admin role required") @@ -912,7 +971,7 @@ def update_user_roles_in_mandate( # Add new role assignments for roleId in roleIds: rootInterface.addRoleToUserMandate(str(membership.id), roleId) - + # Audit - Log role assignment change audit_logger.logPermissionChange( userId=str(context.user.id), @@ -1020,7 +1079,7 @@ def _hasMandateAdminRole(context: RequestContext, mandateId: str) -> bool: Check if the user has mandate admin role for the specified mandate. Works with or without X-Mandate-Id header (admin pages don't send it). """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return True # If mandate context matches, check roles from context directly diff --git a/modules/routes/routeDataSources.py b/modules/routes/routeDataSources.py index 03f6e8e3..5df8a18b 100644 --- a/modules/routes/routeDataSources.py +++ b/modules/routes/routeDataSources.py @@ -7,7 +7,6 @@ from typing import Any, Dict, List, Optional from fastapi import APIRouter, HTTPException, Depends, Path, Request, Body from modules.auth import limiter, getRequestContext, RequestContext -from modules.auth.authentication import _hasSysAdminRole from modules.datamodels.datamodelDataSource import DataSource from modules.datamodels.datamodelFeatureDataSource import FeatureDataSource from modules.shared.i18nRegistry import apiRouteContext @@ -53,7 +52,7 @@ def _updateDataSourceScope( if scope not in _VALID_SCOPES: raise HTTPException(status_code=400, detail=f"Invalid scope: {scope}. Must be one of {_VALID_SCOPES}") - if scope == "global" and not _hasSysAdminRole(context.user): + if scope == "global" and not context.isSysAdmin: raise HTTPException(status_code=403, detail=routeApiMsg("Only sysadmins can set global scope")) try: diff --git a/modules/routes/routeDataUsers.py b/modules/routes/routeDataUsers.py index bc32dfee..ea796aab 100644 --- a/modules/routes/routeDataUsers.py +++ b/modules/routes/routeDataUsers.py @@ -6,7 +6,7 @@ Implements the endpoints for user management. MULTI-TENANT: User management requires RequestContext. - mandateId from X-Mandate-Id header determines which users are visible -- SysAdmin can see all users across mandates +- isPlatformAdmin can see all users across mandates """ from fastapi import APIRouter, HTTPException, Depends, Body, Path, Request, Response, Query @@ -34,10 +34,10 @@ logger = logging.getLogger(__name__) def _isAdminForUser(context: RequestContext, targetUserId: str) -> bool: """ Check if the current user has admin rights for the target user. - SysAdmin can manage all users. MandateAdmin can manage users in their mandates. + PlatformAdmin can manage all users. MandateAdmin can manage users in their mandates. Works without X-Mandate-Id header (admin pages don't send it). """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return True # Find mandates where current user is admin @@ -90,7 +90,7 @@ def _getUserFilterOrIds(context, paginationJson, column=None, idsMode=False): return handleIdsInMemory(items, paginationJson) return handleFilterValuesInMemory(items, column, paginationJson, requestLang) - if context.hasSysAdminRole: + if context.isPlatformAdmin: rootInterface = getRootInterface() if idsMode: return handleIdsMode(rootInterface.db, UserInDB, paginationJson) @@ -167,7 +167,7 @@ def get_user_options( if context.mandateId: result = appInterface.getUsersByMandate(str(context.mandateId), None) users = result.items if hasattr(result, 'items') else result - elif context.hasSysAdminRole: + elif context.isPlatformAdmin: users = appInterface.getAllUsers() else: raise HTTPException(status_code=403, detail=routeApiMsg("Access denied")) @@ -256,8 +256,8 @@ def get_users( items=users, pagination=None ) - elif context.hasSysAdminRole: - # SysAdmin without mandateId — DB-level pagination via interface + elif context.isPlatformAdmin: + # PlatformAdmin without mandateId — DB-level pagination via interface result = appInterface.getAllUsers(paginationParams) if paginationParams and hasattr(result, 'items'): @@ -375,8 +375,8 @@ def get_user( detail=f"User with ID {userId} not found" ) - # MULTI-TENANT: Verify user is in the same mandate (unless SysAdmin) - if context.mandateId and not context.hasSysAdminRole: + # MULTI-TENANT: Verify user is in the same mandate (unless PlatformAdmin) + if context.mandateId and not context.isPlatformAdmin: userMandate = appInterface.getUserMandate(userId, str(context.mandateId)) if not userMandate: raise HTTPException( @@ -402,6 +402,7 @@ class CreateUserRequest(BaseModel): language: str = "de" enabled: bool = True isSysAdmin: bool = False + isPlatformAdmin: bool = False password: Optional[str] = None @@ -415,10 +416,24 @@ def create_user( """ Create a new user. MULTI-TENANT: User is created and automatically added to the current mandate. + + Privileged platform flags (isSysAdmin, isPlatformAdmin) may only be set + by a Platform Admin. Non-PlatformAdmin requests have these flags reset + to False with a warning. """ appInterface = interfaceDbApp.getInterface(context.user) - - # Extract fields from request model and call createUser with individual parameters + + callerIsPlatformAdmin = context.isPlatformAdmin + requestedSysAdmin = bool(userData.isSysAdmin) and callerIsPlatformAdmin + requestedPlatformAdmin = bool(userData.isPlatformAdmin) and callerIsPlatformAdmin + + if (userData.isSysAdmin or userData.isPlatformAdmin) and not callerIsPlatformAdmin: + logger.warning( + f"Non-PlatformAdmin {context.user.id} attempted to create user with " + f"privileged flags (isSysAdmin={userData.isSysAdmin}, " + f"isPlatformAdmin={userData.isPlatformAdmin}); flags reset to False" + ) + newUser = appInterface.createUser( username=userData.username, password=userData.password, @@ -427,9 +442,10 @@ def create_user( language=userData.language, enabled=userData.enabled, authenticationAuthority=AuthAuthority.LOCAL, - isSysAdmin=userData.isSysAdmin + isSysAdmin=requestedSysAdmin, + isPlatformAdmin=requestedPlatformAdmin, ) - + # MULTI-TENANT: Add user to current mandate via UserMandate with default "user" role if context.mandateId: userRole = appInterface.getRoleByLabel("user") @@ -438,14 +454,14 @@ def create_user( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=routeApiMsg("No 'user' role found in system — cannot assign user to mandate") ) - + appInterface.createUserMandate( userId=str(newUser.id), mandateId=str(context.mandateId), roleIds=[str(userRole.id)] ) logger.info(f"Created UserMandate for user {newUser.id} in mandate {context.mandateId}") - + return newUser @router.put("/{userId}", response_model=User) @@ -453,44 +469,67 @@ def create_user( def update_user( request: Request, userId: str = Path(..., description="ID of the user to update"), - userData: User = Body(...), + userData: Dict[str, Any] = Body(..., description="Partial user payload — only the fields present in the request body are updated."), context: RequestContext = Depends(getRequestContext) ) -> User: """ - Update an existing user. + Update an existing user (PARTIAL update). + + The request body is treated as a **partial** patch: only the keys actually + sent are applied; missing keys leave the stored value untouched. This is + intentional — sending a full ``User`` body would overwrite unrelated fields + (e.g. ``isSysAdmin``/``isPlatformAdmin``) with Pydantic defaults whenever a + client only ships a subset, which has historically caused privileged flags + to flip silently when toggling a single inline cell. + Self-service: Users can update their own profile (language, fullName, etc.). - Admin: MandateAdmin can update users in their mandates. SysAdmin for all. + Admin: MandateAdmin can update users in their mandates. + PlatformAdmin can update any user. + + Privileged flag changes (isSysAdmin, isPlatformAdmin) require: + - caller has isPlatformAdmin=True, AND + - target is NOT the caller (Self-Protection). """ isSelfUpdate = str(context.user.id) == str(userId) - - # Non-self updates require admin permission + if not isSelfUpdate and not _isAdminForUser(context, userId): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Admin role required to update other users") ) - - # Use rootInterface for user lookup/update (avoids RBAC filtering on User table) + rootInterface = getRootInterface() - - # Check if the user exists + existingUser = rootInterface.getUser(userId) if not existingUser: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"User with ID {userId} not found" ) - - # SysAdmins may toggle the isSysAdmin flag on other users - callerIsSysAdmin = context.isSysAdmin or context.hasSysAdminRole - updatedUser = rootInterface.updateUser(userId, userData, allowSysAdminChange=(callerIsSysAdmin and not isSelfUpdate)) - + + if not isinstance(userData, dict): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=routeApiMsg("User update payload must be a JSON object") + ) + + # Defensive: drop ``id`` from payload — userId comes from the path and + # tampering with it from the body must never silently rebind the row. + sanitizedPayload = {k: v for k, v in userData.items() if k != "id"} + + callerIsPlatformAdmin = context.isPlatformAdmin + allowAdminFlagChange = callerIsPlatformAdmin and not isSelfUpdate + + updatedUser = rootInterface.updateUser( + userId, sanitizedPayload, allowAdminFlagChange=allowAdminFlagChange + ) + if not updatedUser: raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=routeApiMsg("Error updating the user") ) - + return updatedUser @router.post("/{userId}/reset-password") @@ -793,7 +832,7 @@ def delete_user( ) -> Dict[str, Any]: """ Delete a user. - MULTI-TENANT: Can only delete users in the same mandate (unless SysAdmin). + MULTI-TENANT: Can only delete users in the same mandate (unless PlatformAdmin). """ appInterface = interfaceDbApp.getInterface(context.user) @@ -805,8 +844,8 @@ def delete_user( detail=f"User with ID {userId} not found" ) - # MULTI-TENANT: Verify user is in the same mandate (unless SysAdmin) - if context.mandateId and not context.hasSysAdminRole: + # MULTI-TENANT: Verify user is in the same mandate (unless PlatformAdmin) + if context.mandateId and not context.isPlatformAdmin: userMandate = appInterface.getUserMandate(userId, str(context.mandateId)) if not userMandate: raise HTTPException( diff --git a/modules/routes/routeI18n.py b/modules/routes/routeI18n.py index a50739d7..7f230324 100644 --- a/modules/routes/routeI18n.py +++ b/modules/routes/routeI18n.py @@ -4,9 +4,11 @@ Public and authenticated routes for UI language sets (DB-backed i18n). Architecture: -- xx = base set (meta): key = German plaintext, value = UI context for AI +- xx = base set (meta): key = source plaintext (German or English, as written + in the code via ``t("...")``), value = UI context for AI - All languages (incl. de) are AI-generated translations from xx -- AI translation pipeline uses context from xx to disambiguate translations +- AI translation pipeline uses context from xx to disambiguate translations; + the prompt forces the output language to be exactly the requested target. """ from __future__ import annotations @@ -23,7 +25,7 @@ from fastapi import APIRouter, BackgroundTasks, Depends, File, HTTPException, Re from fastapi.responses import Response from pydantic import BaseModel, Field -from modules.auth import getCurrentUser, requireSysAdminRole +from modules.auth import getCurrentUser, requireSysAdmin, requirePlatformAdmin from modules.connectors.connectorDbPostgre import _get_cached_connector from modules.datamodels.datamodelAi import ( AiCallOptions, @@ -234,17 +236,31 @@ async def _translateBatch( jsonPayload = json.dumps(payload, ensure_ascii=False) systemPrompt = ( - f"Du bist ein professioneller Übersetzer für Software-UI-Texte. " - f"Du erhältst ein JSON-Array mit Objekten: {{\"key\": \"deutscher Text\", \"context\": \"UI-Kontext\"}}. " - f"Der Kontext beschreibt, wo der Text in der Anwendung verwendet wird (Datei, Komponente). " - f"Übersetze jeden «key» ins {targetLanguageLabel} (ISO {targetCode}). " - f"Behalte Platzhalter wie {{variable}} exakt bei. " - f"Antworte NUR mit einem JSON-Objekt — Keys = deutsche Originaltexte, Values = Übersetzungen. " - f"Kein Markdown, kein Kommentar." + f"You are a professional translator for software UI texts. " + f"You receive a JSON array of objects: {{\"key\": \"source text\", \"context\": \"UI context\"}}. " + f"The source text is written in German OR English. " + f"The context describes where the text is used in the application (file, component). " + f"\n\n" + f"HARD REQUIREMENTS (must all be satisfied):\n" + f"1. OUTPUT LANGUAGE: every translated value MUST be written in {targetLanguageLabel} " + f"(ISO code \"{targetCode}\"). Never output in German or English if that is not " + f"the target language. No mixing of languages.\n" + f"2. If the source is already in the target language, keep it (do not re-translate, " + f"do not paraphrase).\n" + f"3. KEEP the exact JSON keys from the input — do NOT translate or modify the keys.\n" + f"4. KEEP placeholders like {{variable}}, {{count}}, %s, %(name)s exactly as they are.\n" + f"5. Preserve leading/trailing whitespace, punctuation and capitalisation pattern.\n" + f"6. Answer ONLY with a JSON object mapping source-key -> translated value in " + f"{targetLanguageLabel}. No markdown fences, no comments, no explanations.\n" + f"7. If a key cannot be translated (empty, pure symbols, URLs), return the source unchanged." ) aiRequest = AiCallRequest( - prompt=f"Übersetze diese UI-Labels:\n{jsonPayload}", + prompt=( + f"Translate the following UI labels into {targetLanguageLabel} " + f"(ISO {targetCode}). Source may be German or English. " + f"Respond with a pure JSON object only.\n{jsonPayload}" + ), context=systemPrompt, options=AiCallOptions( operationType=OperationTypeEnum.DATA_GENERATE, @@ -826,7 +842,7 @@ async def _syncLanguageWithXx(db, code: str, userId: Optional[str], adminUser: O @router.put("/sets/sync-xx") async def sync_xx_master( request: Request, - adminUser: User = Depends(requireSysAdminRole), + adminUser: User = Depends(requireSysAdmin), ): """Synchronise the xx base set from the frontend build artefact. @@ -844,7 +860,7 @@ async def sync_xx_master( @router.get("/sets/{code}/sync-diff") async def get_language_sync_diff( code: str, - adminUser: User = Depends(requireSysAdminRole), + adminUser: User = Depends(requirePlatformAdmin), ): """How many keys would be added/removed vs xx before running a full sync (SysAdmin).""" c = code.strip().lower() @@ -857,7 +873,7 @@ async def get_language_sync_diff( @router.put("/sets/{code}") async def update_language_set( code: str, - adminUser: User = Depends(requireSysAdminRole), + adminUser: User = Depends(requirePlatformAdmin), ): c = code.strip().lower() if c in ("update-all", "sync-xx", "sync-de"): @@ -873,7 +889,7 @@ async def update_language_set( @router.delete("/sets/{code}") async def delete_language_set( code: str, - adminUser: User = Depends(requireSysAdminRole), + adminUser: User = Depends(requirePlatformAdmin), ): c = code.strip().lower() if c in _PROTECTED_CODES: @@ -911,7 +927,7 @@ async def download_language_set( @router.get("/export") async def export_all_language_sets( - adminUser: User = Depends(requireSysAdminRole), + adminUser: User = Depends(requirePlatformAdmin), ): db = getMgmtInterface(adminUser, mandateId=None).db rows = db.getRecordset(UiLanguageSet) @@ -939,7 +955,7 @@ async def export_all_language_sets( @router.post("/import") async def import_language_sets( file: UploadFile = File(...), - adminUser: User = Depends(requireSysAdminRole), + adminUser: User = Depends(requirePlatformAdmin), ): if not file.filename or not file.filename.endswith(".json"): raise HTTPException(status_code=400, detail=routeApiMsg("Nur .json-Dateien erlaubt.")) diff --git a/modules/routes/routeInvitations.py b/modules/routes/routeInvitations.py index ffc0f11a..7e852b54 100644 --- a/modules/routes/routeInvitations.py +++ b/modules/routes/routeInvitations.py @@ -186,7 +186,7 @@ def create_invitation( ) # Check admin permission - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: if str(context.mandateId) != mandateId: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -891,7 +891,7 @@ def _hasMandateAdminRole(context: RequestContext) -> bool: """ Check if the user has mandate admin role in the current context. """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return True if not context.roleIds: diff --git a/modules/routes/routeRealEstate.py b/modules/routes/routeRealEstate.py index aa3d98f4..cc571c2f 100644 --- a/modules/routes/routeRealEstate.py +++ b/modules/routes/routeRealEstate.py @@ -121,7 +121,7 @@ async def _validateInstanceAccess(instanceId: str, context: RequestContext) -> s status_code=400, detail=f"Instance '{instanceId}' is not a realestate instance" ) - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: featureAccesses = rootInterface.getFeatureAccessesForUser(str(context.user.id)) hasAccess = any( str(fa.featureInstanceId) == instanceId and fa.enabled diff --git a/modules/routes/routeSecurityLocal.py b/modules/routes/routeSecurityLocal.py index fa68b5b9..b6227cb0 100644 --- a/modules/routes/routeSecurityLocal.py +++ b/modules/routes/routeSecurityLocal.py @@ -210,13 +210,13 @@ def _ensureHomeMandate(rootInterface, user) -> None: except Exception as e: logger.warning(f"Could not check pending invitations for {user.username}: {e}") - homeMandateName = f"Home {user.username}" + homeMandateLabel = f"Home {user.username}" rootInterface._provisionMandateForUser( userId=userId, - mandateName=homeMandateName, + mandateLabel=homeMandateLabel, planKey="TRIAL_14D", ) - logger.info(f"Created Home mandate '{homeMandateName}' for user {user.username}") + logger.info(f"Created Home mandate '{homeMandateLabel}' for user {user.username}") @router.post("/login") @@ -464,10 +464,10 @@ def register_user( provisionResult = None if not hasPendingInvitations: try: - homeMandateName = f"Home {user.username}" + homeMandateLabel = f"Home {user.username}" provisionResult = appInterface._provisionMandateForUser( userId=str(user.id), - mandateName=homeMandateName, + mandateLabel=homeMandateLabel, planKey="TRIAL_14D", ) logger.info(f"Provisioned Home mandate for user {user.id}: {provisionResult}") @@ -881,7 +881,7 @@ def onboarding_provision( "alreadyProvisioned": True, } - mandateName = (companyName.strip() if companyName and companyName.strip() + mandateLabel = (companyName.strip() if companyName and companyName.strip() else f"Home {currentUser.username}") if planKey not in ("TRIAL_14D", "STARTER_MONTHLY", "STARTER_YEARLY", "PROFESSIONAL_MONTHLY", "PROFESSIONAL_YEARLY", "MAX_MONTHLY", "MAX_YEARLY"): @@ -889,7 +889,7 @@ def onboarding_provision( result = appInterface._provisionMandateForUser( userId=userId, - mandateName=mandateName, + mandateLabel=mandateLabel, planKey=planKey, ) diff --git a/modules/routes/routeStore.py b/modules/routes/routeStore.py index 04517d9b..b1f8dbc6 100644 --- a/modules/routes/routeStore.py +++ b/modules/routes/routeStore.py @@ -59,7 +59,13 @@ class StoreFeatureResponse(BaseModel): def _getStoreFeatures(catalogService) -> List[Dict[str, Any]]: - """Get all features available in the store.""" + """Get all features available in the store. + + Soft-disabled features (``enabled=False`` in their feature definition) are + skipped so that legacy or temporarily-deactivated modules do not appear in + the storefront, even if their ``resource.store.*`` catalog object is still + registered. + """ resourceObjects = catalogService.getResourceObjects() storeFeatures = [] for obj in resourceObjects: @@ -68,7 +74,7 @@ def _getStoreFeatures(catalogService) -> List[Dict[str, Any]]: featureCode = meta.get("featureCode") if featureCode: featureDef = catalogService.getFeatureDefinition(featureCode) - if featureDef: + if featureDef and featureDef.get("enabled", True): storeFeatures.append(featureDef) return storeFeatures diff --git a/modules/routes/routeSubscription.py b/modules/routes/routeSubscription.py index a96cd0ae..dbdbe37d 100644 --- a/modules/routes/routeSubscription.py +++ b/modules/routes/routeSubscription.py @@ -46,7 +46,7 @@ def _resolveMandateId(context: RequestContext) -> str: def _assertMandateAdmin(context: RequestContext, mandateId: str) -> None: - if context.hasSysAdminRole: + if context.isPlatformAdmin: return try: from modules.interfaces.interfaceDbApp import getRootInterface @@ -303,7 +303,7 @@ def forceCancel( context: RequestContext = Depends(getRequestContext), ): """Sysadmin: immediately expire any non-terminal subscription.""" - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Sysadmin role required")) from modules.serviceCenter.services.serviceSubscription.mainServiceSubscription import ( @@ -485,7 +485,7 @@ def getAllSubscriptions( context: RequestContext = Depends(getRequestContext), ): """SysAdmin: list ALL subscriptions across all mandates with enriched metadata.""" - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=routeApiMsg("Sysadmin role required")) if mode == "filterValues": diff --git a/modules/routes/routeSystem.py b/modules/routes/routeSystem.py index 8db0350f..b8ad72af 100644 --- a/modules/routes/routeSystem.py +++ b/modules/routes/routeSystem.py @@ -478,7 +478,7 @@ def get_navigation( Endpoint: GET /api/navigation """ try: - isSysAdmin = reqContext.hasSysAdminRole + isSysAdmin = reqContext.isPlatformAdmin userId = str(reqContext.user.id) if reqContext.user else None # Get user's role IDs for permission checking diff --git a/modules/routes/routeWorkflowDashboard.py b/modules/routes/routeWorkflowDashboard.py index 96075a26..626f0872 100644 --- a/modules/routes/routeWorkflowDashboard.py +++ b/modules/routes/routeWorkflowDashboard.py @@ -106,7 +106,7 @@ def _scopedRunFilter(context: RequestContext) -> Optional[dict]: - mandate admin: mandateId IN user's mandates - normal user: ownerId = userId """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return None userId = str(context.user.id) if context.user else None @@ -128,7 +128,7 @@ def _scopedWorkflowFilter(context: RequestContext) -> Optional[dict]: - sysadmin: None (no filter, sees all) - normal user: mandateId IN user's mandates """ - if context.hasSysAdminRole: + if context.isPlatformAdmin: return None userId = str(context.user.id) if context.user else None @@ -144,7 +144,7 @@ def _scopedWorkflowFilter(context: RequestContext) -> Optional[dict]: def _userMayDeleteWorkflow(context: RequestContext, wfMandateId: Optional[str]) -> bool: """Same rules as canDelete on rows in get_system_workflows.""" - if context.hasSysAdminRole: + if context.isPlatformAdmin: return True userId = str(context.user.id) if context.user else None if not userId or not wfMandateId: @@ -477,7 +477,7 @@ def get_system_workflows( userId = str(context.user.id) if context.user else None adminMandateIds = [] - if userId and not context.hasSysAdminRole: + if userId and not context.isPlatformAdmin: userMandateIds = _getUserMandateIds(userId) adminMandateIds = _getAdminMandateIds(userId, userMandateIds) @@ -514,7 +514,7 @@ def get_system_workflows( row["runCount"] = runCountMap.get(wfId, 0) row["lastStartedAt"] = lastStartedMap.get(wfId) - if context.hasSysAdminRole: + if context.isPlatformAdmin: row["canEdit"] = True row["canDelete"] = True row["canExecute"] = True @@ -670,7 +670,7 @@ def get_run_steps( raise HTTPException(status_code=404, detail=routeApiMsg("Run not found")) run = dict(runs[0]) - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: userId = str(context.user.id) if context.user else None runOwner = run.get("ownerId") runMandate = run.get("mandateId") @@ -711,7 +711,7 @@ async def get_run_stream( raise HTTPException(status_code=404, detail=routeApiMsg("Run not found")) run = dict(runs[0]) - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: userId = str(context.user.id) if context.user else None runOwner = run.get("ownerId") runMandate = run.get("mandateId") @@ -774,7 +774,7 @@ def stop_workflow_run( raise HTTPException(status_code=404, detail=routeApiMsg("Run not found")) run = dict(runs[0]) - if not context.hasSysAdminRole: + if not context.isPlatformAdmin: userId = str(context.user.id) if context.user else None runOwner = run.get("ownerId") runMandate = run.get("mandateId") diff --git a/modules/security/rbacCatalog.py b/modules/security/rbacCatalog.py index bc5b0353..9b1ca22f 100644 --- a/modules/security/rbacCatalog.py +++ b/modules/security/rbacCatalog.py @@ -84,10 +84,38 @@ class RbacCatalogService: logger.error(f"Failed to register DATA object {objectKey}: {e}") return False - def registerFeatureDefinition(self, featureCode: str, label: str, icon: str) -> bool: - """Register a feature definition.""" + def registerFeatureDefinition( + self, + featureCode: str, + label: str, + icon: str, + *, + instantiable: bool = True, + enabled: bool = True, + ) -> bool: + """Register a feature definition. + + Args: + featureCode: Stable code (e.g. ``"trustee"``). + label: Display label. + icon: Display icon. + instantiable: ``False`` for meta-features that must NOT be exposed + as a creatable Feature-Instance (e.g. the ``system`` umbrella + feature which only owns global UI/DATA/RESOURCE catalog + objects). Defaults to ``True``. + enabled: ``False`` to soft-disable a feature so it is filtered out + of selection lists (Store / Admin Feature-Instances dropdown) + without removing its catalog objects, role templates or + already-provisioned instances. Defaults to ``True``. + """ try: - self._featureDefinitions[featureCode] = {"code": featureCode, "label": label, "icon": icon} + self._featureDefinitions[featureCode] = { + "code": featureCode, + "label": label, + "icon": icon, + "instantiable": bool(instantiable), + "enabled": bool(enabled), + } return True except Exception as e: logger.error(f"Failed to register feature definition {featureCode}: {e}") diff --git a/modules/shared/mandateNameUtils.py b/modules/shared/mandateNameUtils.py new file mode 100644 index 00000000..661aaeee --- /dev/null +++ b/modules/shared/mandateNameUtils.py @@ -0,0 +1,121 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Slug and validation helpers for Mandate.name (Kurzzeichen). + +Format: lowercase [a-z0-9], segments separated by a single hyphen, length 2–32. +German umlauts are transliterated (ä→ae, ö→oe, ü→ue, ß→ss) before slugging. +""" + +from __future__ import annotations + +import re +from typing import Iterable, Set + +MANDATE_NAME_MIN_LEN = 2 +MANDATE_NAME_MAX_LEN = 32 +_MANDATE_NAME_RE = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$") + + +def _transliterateGerman(text: str) -> str: + """Map common German characters to ASCII before slugging.""" + if not text: + return "" + result: list[str] = [] + for ch in text: + lower = ch.lower() + if lower == "ä": + result.append("ae") + elif lower == "ö": + result.append("oe") + elif lower == "ü": + result.append("ue") + elif lower == "ß": + result.append("ss") + else: + result.append(ch) + return "".join(result) + + +def _collapseHyphensAndTrim(raw: str) -> str: + s = re.sub(r"[^a-z0-9]+", "-", raw.lower()) + s = re.sub(r"-+", "-", s).strip("-") + return s + + +def _ensureMinSlugLength(slug: str) -> str: + if len(slug) >= MANDATE_NAME_MIN_LEN: + return slug + if len(slug) == 1: + return slug + slug + return slug + ("x" * (MANDATE_NAME_MIN_LEN - len(slug))) + + +def _truncateSlugToMaxLen(slug: str) -> str: + if len(slug) <= MANDATE_NAME_MAX_LEN: + return slug + cut = slug[: MANDATE_NAME_MAX_LEN].rstrip("-") + if "-" in cut: + cut = cut[: cut.rfind("-")] + cut = cut.strip("-") + if len(cut) < MANDATE_NAME_MIN_LEN: + return cut + ("x" * (MANDATE_NAME_MIN_LEN - len(cut))) + return cut + + +def transliterateGerman(text: str) -> str: + """Transliterate German umlauts in *text* for further processing.""" + return _transliterateGerman(text) + + +def slugifyMandateName(label: str) -> str: + """ + Build a mandate slug base from a human-readable label. + Result satisfies isValidMandateName except pathological cases (falls back to 'mn'). + """ + if not label or not str(label).strip(): + t = "mn" + else: + step1 = _transliterateGerman(label.strip()) + step2 = _collapseHyphensAndTrim(step1) + if not step2: + t = "mn" + else: + t = _ensureMinSlugLength(step2) + t = _truncateSlugToMaxLen(t) + if not isValidMandateName(t): + return "mn" + return t + + +def isValidMandateName(name: str) -> bool: + """True if *name* matches slug rules (length 2–32, [a-z0-9] and single-hyphen segments).""" + if not isinstance(name, str) or len(name) < MANDATE_NAME_MIN_LEN or len(name) > MANDATE_NAME_MAX_LEN: + return False + return _MANDATE_NAME_RE.match(name) is not None + + +def allocateUniqueMandateSlug(base: str, taken: Iterable[str]) -> str: + """ + Return a slug not present in *taken*, starting with *base*, then base-2, base-3, ... + *base* must satisfy isValidMandateName (typically from slugifyMandateName). + """ + used: Set[str] = {x for x in taken if x} + if base not in used: + return base + n = 2 + while True: + suffix = f"-{n}" + room = MANDATE_NAME_MAX_LEN - len(suffix) + if room < MANDATE_NAME_MIN_LEN: + room = MANDATE_NAME_MIN_LEN + root = base[:room].rstrip("-") + if len(root) < MANDATE_NAME_MIN_LEN: + root = "mn" + cand = (root + suffix)[:MANDATE_NAME_MAX_LEN] + cand = cand.rstrip("-") + if isValidMandateName(cand) and cand not in used: + return cand + n += 1 + if n > 100000: + raise ValueError("allocateUniqueMandateSlug: could not allocate a unique slug") diff --git a/modules/system/mainSystem.py b/modules/system/mainSystem.py index 3a361ab9..0558f65b 100644 --- a/modules/system/mainSystem.py +++ b/modules/system/mainSystem.py @@ -622,11 +622,11 @@ def registerFeature(catalogService) -> bool: meta=aicoreObj.get("meta") ) - # Register feature definition catalogService.registerFeatureDefinition( featureCode=FEATURE_CODE, label=FEATURE_LABEL, - icon=FEATURE_ICON + icon=FEATURE_ICON, + instantiable=False, ) logger.info(f"Registered system RBAC objects: {len(UI_OBJECTS)} UI, {len(DATA_OBJECTS)} DATA, {len(RESOURCE_OBJECTS)} RESOURCE") diff --git a/modules/system/registry.py b/modules/system/registry.py index 80bc8c0c..5a793ade 100644 --- a/modules/system/registry.py +++ b/modules/system/registry.py @@ -172,7 +172,9 @@ def registerAllFeaturesInCatalog(catalogService) -> Dict[str, bool]: catalogService.registerFeatureDefinition( featureCode=featureDef.get("code", featureName), label=featureDef.get("label", {"en": featureName, "de": featureName}), - icon=featureDef.get("icon", "mdi-puzzle") + icon=featureDef.get("icon", "mdi-puzzle"), + instantiable=featureDef.get("instantiable", True), + enabled=featureDef.get("enabled", True), ) logger.info(f"Registered feature definition: {featureDef.get('code', featureName)}") except Exception as e: diff --git a/scripts/check_db_no_sysadmin_role.py b/scripts/check_db_no_sysadmin_role.py new file mode 100644 index 00000000..e0d02d7c --- /dev/null +++ b/scripts/check_db_no_sysadmin_role.py @@ -0,0 +1,132 @@ +"""Runtime-Check (A5): bestaetigt, dass die ``sysadmin``-Rolle aus der +Datenbank entfernt wurde und liefert eine kurze Inventur fuer die +isPlatformAdmin / isSysAdmin Flags. + +Das Skript verwendet die bestehende ``APP_CONFIG`` (entschluesselt +``DB_PASSWORD_SECRET``) und fragt direkt via ``psycopg2`` ab, ohne den +ganzen FastAPI-Stack hochzufahren. + +Aufruf:: + + python gateway/scripts/check_db_no_sysadmin_role.py + +Exit-Code: + +- 0 -> sauber (Role-Count == 0) +- 1 -> sysadmin-Rolle existiert noch (Migration unvollstaendig) +- 2 -> Verbindungsfehler (Konfiguration / DB nicht erreichbar) +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +_GATEWAY = Path(__file__).resolve().parents[1] +if str(_GATEWAY) not in sys.path: + sys.path.insert(0, str(_GATEWAY)) + +import psycopg2 # noqa: E402 +import psycopg2.extras # noqa: E402 + +from modules.shared.configuration import APP_CONFIG # noqa: E402 + + +def _connect(): + host = APP_CONFIG.get("DB_HOST", "localhost") + user = APP_CONFIG.get("DB_USER") + password = APP_CONFIG.get("DB_PASSWORD_SECRET") + port = int(APP_CONFIG.get("DB_PORT", 5432)) + database = APP_CONFIG.get("DB_DATABASE", "poweron_app") + return psycopg2.connect( + host=host, port=port, dbname=database, user=user, password=password + ) + + +def _main() -> int: + try: + conn = _connect() + except Exception as exc: + print(f"[ERR] Could not connect to database: {exc}") + return 2 + + try: + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + cur.execute( + 'SELECT COUNT(*)::int AS n FROM "Role" WHERE "roleLabel" = %s', + ("sysadmin",), + ) + roleCount = cur.fetchone()["n"] + + cur.execute( + 'SELECT COUNT(*)::int AS n FROM "UserInDB" ' + 'WHERE COALESCE("isPlatformAdmin", false) = true' + ) + platformAdmins = cur.fetchone()["n"] + + cur.execute( + 'SELECT COUNT(*)::int AS n FROM "UserInDB" ' + 'WHERE COALESCE("isSysAdmin", false) = true' + ) + sysAdmins = cur.fetchone()["n"] + + cur.execute( + 'SELECT "username", "email", ' + 'COALESCE("isSysAdmin", false) AS "isSysAdmin", ' + 'COALESCE("isPlatformAdmin", false) AS "isPlatformAdmin" ' + 'FROM "UserInDB" ' + 'WHERE COALESCE("isSysAdmin", false) = true ' + ' OR COALESCE("isPlatformAdmin", false) = true ' + 'ORDER BY "username"' + ) + adminUsers = cur.fetchall() + + cur.execute( + 'SELECT COUNT(*)::int AS n FROM "AccessRule" ar ' + 'JOIN "Role" r ON ar."roleId" = r."id" ' + 'WHERE r."roleLabel" = %s', + ("sysadmin",), + ) + orphanRules = cur.fetchone()["n"] + + cur.execute( + 'SELECT COUNT(*)::int AS n FROM "UserMandateRole" umr ' + 'JOIN "Role" r ON umr."roleId" = r."id" ' + 'WHERE r."roleLabel" = %s', + ("sysadmin",), + ) + orphanGrants = cur.fetchone()["n"] + finally: + conn.close() + + print("=" * 64) + print("A5 - SysAdmin Migration DB Check") + print("=" * 64) + print(f"Role.roleLabel == 'sysadmin' : {roleCount}") + print(f"AccessRule(s) referencing sysadmin role : {orphanRules}") + print(f"UserMandateRole(s) granting sysadmin role : {orphanGrants}") + print(f"User.isSysAdmin = true : {sysAdmins}") + print(f"User.isPlatformAdmin = true : {platformAdmins}") + print() + print("Admin-flagged users:") + if not adminUsers: + print(" (none)") + for row in adminUsers: + flags = [] + if row["isSysAdmin"]: + flags.append("isSysAdmin") + if row["isPlatformAdmin"]: + flags.append("isPlatformAdmin") + print(f" - {row['username']:<32} {row.get('email') or '':<40} {','.join(flags)}") + print("=" * 64) + + if roleCount == 0 and orphanRules == 0 and orphanGrants == 0: + print("[OK] Migration verified: no sysadmin role artefacts in DB.") + return 0 + print("[FAIL] Legacy sysadmin role artefacts still present in DB.") + return 1 + + +if __name__ == "__main__": + sys.exit(_main()) diff --git a/scripts/check_no_sysadmin_role.py b/scripts/check_no_sysadmin_role.py new file mode 100644 index 00000000..23fa8f2e --- /dev/null +++ b/scripts/check_no_sysadmin_role.py @@ -0,0 +1,108 @@ +"""CI-Gate: Stelle sicher, dass keine Verweise auf die abgeschaffte +``sysadmin``-Rolle bzw. die alten Helper im Codebase mehr existieren. + +Verbotene Symbole nach Abschluss von ``2026-04-sysadmin-authority-split``: + +- ``hasSysAdminRole`` (RequestContext property) +- ``requireSysAdminRole`` (FastAPI dependency) +- ``_hasSysAdminRole`` (Hilfs-Funktion gegen die alte Rolle) + +Erlaubt sind weiterhin: + +- ``isSysAdmin`` (User-Flag fuer Infrastruktur-Operator) +- ``isPlatformAdmin`` (User-Flag fuer Cross-Mandate-Governance) +- ``requireSysAdmin`` / ``requirePlatformAdmin`` (FastAPI Dependencies) + +Exit-Code: + +- 0 -> sauber +- 1 -> Fundstellen vorhanden (CI bricht ab) + +Aufruf:: + + python gateway/scripts/check_no_sysadmin_role.py +""" + +from __future__ import annotations + +import os +import re +import sys +from pathlib import Path +from typing import Iterable, List, Tuple + +_REPO_ROOT = Path(__file__).resolve().parents[2] + +_FORBIDDEN_PATTERNS: Tuple[Tuple[str, str], ...] = ( + (r"\bhasSysAdminRole\b", "Use ctx.isPlatformAdmin (Governance) or ctx.isSysAdmin (Infra)"), + (r"\brequireSysAdminRole\b", "Use requirePlatformAdmin (Governance) or requireSysAdmin (Infra)"), + (r"\b_hasSysAdminRole\b", "Use User.isPlatformAdmin flag check directly"), +) + +_INCLUDE_SUFFIXES = {".py", ".ts", ".tsx", ".js", ".jsx"} + +_EXCLUDE_DIR_NAMES = { + ".git", + "node_modules", + "dist", + "build", + "__pycache__", + ".venv", + "venv", + ".pytest_cache", + ".mypy_cache", + "wiki", + "scripts", +} + + +def _shouldScan(path: Path) -> bool: + if path.suffix not in _INCLUDE_SUFFIXES: + return False + parts = set(path.parts) + if parts & _EXCLUDE_DIR_NAMES: + return False + return True + + +def _iterFiles(root: Path) -> Iterable[Path]: + for dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames if d not in _EXCLUDE_DIR_NAMES] + for name in filenames: + full = Path(dirpath) / name + if _shouldScan(full): + yield full + + +def _scanFile(path: Path) -> List[Tuple[int, str, str, str]]: + findings: List[Tuple[int, str, str, str]] = [] + try: + text = path.read_text(encoding="utf-8", errors="ignore") + except Exception: + return findings + for pattern, hint in _FORBIDDEN_PATTERNS: + compiled = re.compile(pattern) + for lineNo, line in enumerate(text.splitlines(), start=1): + if compiled.search(line): + findings.append((lineNo, pattern, hint, line.rstrip())) + return findings + + +def _main() -> int: + findings = [] + for filePath in _iterFiles(_REPO_ROOT): + for entry in _scanFile(filePath): + findings.append((filePath, *entry)) + if not findings: + print("[OK] No legacy sysadmin-role references found.") + return 0 + print("[FAIL] Found legacy sysadmin-role references:") + for filePath, lineNo, pattern, hint, line in findings: + rel = filePath.relative_to(_REPO_ROOT) + print(f" {rel}:{lineNo}: {pattern}\n hint: {hint}\n line: {line}") + print(f"\n[FAIL] {len(findings)} forbidden reference(s).") + return 1 + + +if __name__ == "__main__": + sys.exit(_main()) diff --git a/tests/integration/mandates/__init__.py b/tests/integration/mandates/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/integration/mandates/test_createMandate.py b/tests/integration/mandates/test_createMandate.py new file mode 100644 index 00000000..f58f9021 --- /dev/null +++ b/tests/integration/mandates/test_createMandate.py @@ -0,0 +1,190 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Integration tests for ``AppObjects.createMandate``. + +Covers acceptance criteria from +``wiki/c-work/1-plan/2026-04-mandate-name-label-logic.md``: + +- AC#1 -> create with label only auto-generates a valid slug name (umlaut transliteration). +- AC#2 -> two labels yielding the same slug get -2 suffix. +- AC#4 -> explicit invalid name (uppercase / spaces) is rejected with ValueError (mapped to 400 by route). +- Label is mandatory (empty label raises ValueError). +- Explicit valid name is honored verbatim. + +Strategy: instantiate ``AppObjects`` via ``__new__`` (skip real ``__init__``) and +inject a minimal FakeDb that simulates ``getRecordset(Mandate)`` and +``recordCreate(Mandate, ...)``. RBAC and role-copy are stubbed. +""" + +from __future__ import annotations + +from typing import Any, Dict, List, Optional +from unittest.mock import Mock, patch +from uuid import uuid4 + +import pytest + +from modules.datamodels.datamodelUam import Mandate +from modules.interfaces.interfaceDbApp import AppObjects +from modules.shared.mandateNameUtils import isValidMandateName + + +class _FakeDb: + """Minimal connector: getRecordset(Mandate) + recordCreate(Mandate, payload).""" + + def __init__(self, rows: Optional[List[Dict[str, Any]]] = None): + self.rows: List[Dict[str, Any]] = [dict(r) for r in (rows or [])] + self.created: List[Dict[str, Any]] = [] + + def getRecordset(self, model, recordFilter: Optional[Dict[str, Any]] = None): + if model is not Mandate: + return [] + if not recordFilter: + return [dict(r) for r in self.rows] + out = [] + for r in self.rows: + if all(r.get(k) == v for k, v in recordFilter.items()): + out.append(dict(r)) + return out + + def recordCreate(self, model, payload): + if hasattr(payload, "model_dump"): + data = payload.model_dump() + elif isinstance(payload, dict): + data = dict(payload) + else: + data = {k: getattr(payload, k) for k in ("name", "label", "enabled", "isSystem")} + if not data.get("id"): + data["id"] = str(uuid4()) + self.rows.append(data) + self.created.append(dict(data)) + return data + + +def _buildInterface(db: _FakeDb) -> AppObjects: + """Build an AppObjects without real __init__ so we don't need a DB connection.""" + iface = AppObjects.__new__(AppObjects) + iface.db = db + iface.currentUser = Mock(id="platform-admin", isPlatformAdmin=True, isSysAdmin=False) + iface.userId = "platform-admin" + iface.mandateId = None + iface.featureInstanceId = None + iface.rbac = Mock() + return iface + + +@pytest.fixture(autouse=True) +def _stubCopySystemRoles(): + """Avoid touching the bootstrap module (which would need a real DB).""" + with patch( + "modules.interfaces.interfaceBootstrap.copySystemRolesToMandate", + return_value=0, + ): + yield + + +class TestCreateMandateAutoName: + def test_emptyNameGetsSlugFromLabel(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name=None, label="Müller AG") + assert mandate.label == "Müller AG" + assert mandate.name == "mueller-ag" + assert isValidMandateName(mandate.name) + + def test_blankNameStringGetsAutoGenerated(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name=" ", label="Acme Corp") + assert mandate.name == "acme-corp" + + def test_labelTrimmed(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name=None, label=" Tenant X ") + assert mandate.label == "Tenant X" + assert mandate.name == "tenant-x" + + +class TestCreateMandateCollision: + def test_secondMandateWithSameLabelGetsSuffix(self): + db = _FakeDb([{"id": "first", "name": "mueller-ag", "label": "Müller AG"}]) + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name=None, label="Müller AG") + assert mandate.name == "mueller-ag-2" + + def test_thirdMandateWithSameLabelGetsThirdSuffix(self): + db = _FakeDb([ + {"id": "first", "name": "mueller-ag", "label": "Müller AG"}, + {"id": "second", "name": "mueller-ag-2", "label": "Müller AG"}, + ]) + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name=None, label="Müller AG") + assert mandate.name == "mueller-ag-3" + + +class TestCreateMandateExplicitName: + def test_validExplicitNameHonored(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name="custom-slug", label="Display Name") + assert mandate.name == "custom-slug" + assert mandate.label == "Display Name" + + def test_invalidExplicitNameRejected(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + with pytest.raises(ValueError) as excInfo: + iface.createMandate(name="ABC Müller!", label="Display") + assert "Kurzzeichen" in str(excInfo.value) + + def test_explicitNameCollisionRejected(self): + db = _FakeDb([{"id": "first", "name": "taken-slug", "label": "Existing"}]) + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + with pytest.raises(ValueError) as excInfo: + iface.createMandate(name="taken-slug", label="New One") + assert "already in use" in str(excInfo.value) + + +class TestCreateMandateLabelMandatory: + def test_emptyLabelAndNoNameRejected(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + with pytest.raises(ValueError) as excInfo: + iface.createMandate(name=None, label="") + assert "label" in str(excInfo.value).lower() + + def test_noneLabelAndNoNameRejected(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + with pytest.raises(ValueError): + iface.createMandate(name=None, label=None) + + def test_emptyLabelButNameProvidedFallsBackToName(self): + """Backwards-compat: legacy callers pass only ``name``; route falls back.""" + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=True): + mandate = iface.createMandate(name="legacy-name", label="") + assert mandate.label == "legacy-name" + assert mandate.name == "legacy-name" + + +class TestCreateMandateRbac: + def test_noPermissionRaises(self): + db = _FakeDb() + iface = _buildInterface(db) + with patch.object(iface, "checkRbacPermission", return_value=False): + with pytest.raises(PermissionError): + iface.createMandate(name=None, label="X") diff --git a/tests/integration/mandates/test_provisionMandate.py b/tests/integration/mandates/test_provisionMandate.py new file mode 100644 index 00000000..b88da4ee --- /dev/null +++ b/tests/integration/mandates/test_provisionMandate.py @@ -0,0 +1,109 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Integration tests for the slug-derivation contract that +``AppObjects._provisionMandateForUser`` relies on. + +Covers AC#10 from ``wiki/c-work/1-plan/2026-04-mandate-name-label-logic.md``: +auto-provisioning a user named "Patrick.Möller" yields +``label = "Home Patrick.Möller"`` and ``name = "home-patrick-moeller"`` +(or ``-2``, ``-3``, ... on collisions). + +The full ``_provisionMandateForUser`` flow has many side effects (subscriptions, +billing, feature instances). For unit-level integration we focus on the +slug-allocation contract via ``_generateUniqueMandateName`` — that is the +single new behaviour the provisioning method delegates to. +""" + +from __future__ import annotations + +from typing import Any, Dict, List, Optional +from unittest.mock import Mock + +import pytest + +from modules.datamodels.datamodelUam import Mandate +from modules.interfaces.interfaceDbApp import AppObjects + + +class _FakeDb: + def __init__(self, rows: Optional[List[Dict[str, Any]]] = None): + self.rows: List[Dict[str, Any]] = [dict(r) for r in (rows or [])] + + def getRecordset(self, model, recordFilter: Optional[Dict[str, Any]] = None): + if model is not Mandate: + return [] + return [dict(r) for r in self.rows] + + +def _buildInterface(rows: Optional[List[Dict[str, Any]]] = None) -> AppObjects: + iface = AppObjects.__new__(AppObjects) + iface.db = _FakeDb(rows) + iface.currentUser = Mock(id="u-1", isPlatformAdmin=True, isSysAdmin=False) + iface.userId = "u-1" + iface.mandateId = None + iface.featureInstanceId = None + iface.rbac = Mock() + return iface + + +class TestProvisioningSlugFromHomeLabel: + def test_simpleHomeLabel(self): + iface = _buildInterface() + assert iface._generateUniqueMandateName("Home patrick") == "home-patrick" + + def test_umlautPersonNameTransliterated(self): + """AC#10: Patrick.Möller → home-patrick-moeller""" + iface = _buildInterface() + result = iface._generateUniqueMandateName("Home Patrick.Möller") + assert result == "home-patrick-moeller" + + def test_eszettAndUmlautsAndDots(self): + iface = _buildInterface() + result = iface._generateUniqueMandateName("Home Müßler.Ümpf") + assert result == "home-muessler-uempf" + + def test_emptyLabelFallsBackToFallbackSlug(self): + iface = _buildInterface() + result = iface._generateUniqueMandateName("") + assert result == "mn" + + +class TestProvisioningSlugCollisions: + def test_secondHomeWithSameLabelGetsSuffix(self): + rows = [{"id": "first", "name": "home-patrick-moeller", "label": "Home Patrick.Möller"}] + iface = _buildInterface(rows) + result = iface._generateUniqueMandateName("Home Patrick.Möller") + assert result == "home-patrick-moeller-2" + + def test_thirdCollisionGetsThirdSuffix(self): + rows = [ + {"id": "first", "name": "home-patrick-moeller", "label": "Home Patrick.Möller"}, + {"id": "second", "name": "home-patrick-moeller-2", "label": "Home Patrick.Möller"}, + ] + iface = _buildInterface(rows) + result = iface._generateUniqueMandateName("Home Patrick.Möller") + assert result == "home-patrick-moeller-3" + + def test_excludeIdHonored(self): + """When updating, the row being updated must not collide with itself.""" + rows = [{"id": "self", "name": "home-patrick-moeller", "label": "Home Patrick.Möller"}] + iface = _buildInterface(rows) + result = iface._generateUniqueMandateName("Home Patrick.Möller", excludeId="self") + assert result == "home-patrick-moeller", "own row should be excluded from collision check" + + +class TestProvisioningPlanGuard: + """Sanity guard: the new label-mandatory check fires before any DB write.""" + + def test_emptyLabelRejected(self): + iface = _buildInterface() + with pytest.raises(ValueError) as excInfo: + iface._provisionMandateForUser(userId="u-1", mandateLabel="", planKey="TRIAL_14D") + assert "label" in str(excInfo.value).lower() or "voller name" in str(excInfo.value).lower() + + def test_unknownPlanRejectedBeforeLabelCheck(self): + iface = _buildInterface() + with pytest.raises(ValueError) as excInfo: + iface._provisionMandateForUser(userId="u-1", mandateLabel="Home X", planKey="DOES_NOT_EXIST") + assert "plan" in str(excInfo.value).lower() diff --git a/tests/integration/mandates/test_updateMandate.py b/tests/integration/mandates/test_updateMandate.py new file mode 100644 index 00000000..385f7fa9 --- /dev/null +++ b/tests/integration/mandates/test_updateMandate.py @@ -0,0 +1,215 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Integration tests for ``AppObjects.updateMandate``. + +Covers acceptance criteria from +``wiki/c-work/1-plan/2026-04-mandate-name-label-logic.md``: + +- AC#3 -> non-PlatformAdmin update silently drops protected ``name``; + label-only updates still succeed. +- AC#4 -> PlatformAdmin update with invalid name format rejected (ValueError → 400). +- AC#4b -> PlatformAdmin update with empty label rejected. +- AC#4c -> PlatformAdmin update with name colliding on another row rejected. +- Idempotent name update (same value) accepted. +""" + +from __future__ import annotations + +from typing import Any, Dict, List, Optional +from unittest.mock import Mock, patch + +import pytest + +from modules.datamodels.datamodelUam import Mandate +from modules.interfaces.interfaceDbApp import AppObjects + + +class _FakeDb: + """Minimal connector: getRecordset(Mandate) + recordModify(Mandate, id, data).""" + + def __init__(self, rows: List[Dict[str, Any]]): + self.rows: List[Dict[str, Any]] = [dict(r) for r in rows] + self.modifyCalls: List[Dict[str, Any]] = [] + + def getRecordset(self, model, recordFilter: Optional[Dict[str, Any]] = None): + if model is not Mandate: + return [] + if not recordFilter: + return [dict(r) for r in self.rows] + out = [] + for r in self.rows: + if all(r.get(k) == v for k, v in recordFilter.items()): + out.append(dict(r)) + return out + + def recordModify(self, model, recordId: str, payload): + if hasattr(payload, "model_dump"): + data = payload.model_dump() + elif isinstance(payload, dict): + data = dict(payload) + else: + data = {} + self.modifyCalls.append({"id": str(recordId), "data": dict(data)}) + for r in self.rows: + if str(r.get("id")) == str(recordId): + r.update(data) + return r + return None + + +def _buildInterface(db: _FakeDb, *, isPlatformAdmin: bool, isSysAdmin: bool = False) -> AppObjects: + iface = AppObjects.__new__(AppObjects) + iface.db = db + iface.currentUser = Mock( + id="user-x", + isPlatformAdmin=isPlatformAdmin, + isSysAdmin=isSysAdmin, + ) + iface.userId = "user-x" + iface.mandateId = None + iface.featureInstanceId = None + iface.rbac = Mock() + return iface + + +def _row(mid: str = "m1", name: str = "alpha", label: str = "Alpha", **extra) -> Dict[str, Any]: + base = { + "id": mid, + "name": name, + "label": label, + "enabled": True, + "isSystem": False, + } + base.update(extra) + return base + + +def _stubGetMandateAndRbac(iface: AppObjects, row: Dict[str, Any]): + """Wire ``getMandate`` to read from the FakeDb so post-update reads reflect changes.""" + db = iface.db + + def _readMandate(mandateId: str): + for r in db.rows: + if str(r.get("id")) == str(mandateId): + return Mandate(**r) + return None + + iface.getMandate = Mock(side_effect=_readMandate) + return patch.object(iface, "checkRbacPermission", return_value=True) + + +class TestUpdateMandateRbacOnName: + def test_mandateAdminCannotChangeName(self): + """Non-platform admin: ``name`` is a protected field, silently dropped. + + Status quo: route layer also enforces this via ``_MANDATE_ADMIN_EDITABLE_FIELDS``, + but the interface itself MUST also defend so that direct calls don't bypass. + """ + row = _row(mid="m1", name="original-slug", label="Original") + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=False) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"name": "hacked-slug", "label": "New Label"}) + assert updated.name == "original-slug", "MandateAdmin must NOT modify name" + assert updated.label == "New Label" + + def test_platformAdminCanChangeName(self): + row = _row(mid="m1", name="old-slug", label="Old") + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"name": "new-slug"}) + assert updated.name == "new-slug" + + def test_sysAdminCanChangeName(self): + row = _row(mid="m1", name="old-slug", label="Old") + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=False, isSysAdmin=True) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"name": "syscall-slug"}) + assert updated.name == "syscall-slug" + + +class TestUpdateMandateNameValidation: + def test_invalidNameRejected(self): + row = _row() + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + with pytest.raises(ValueError) as excInfo: + iface.updateMandate("m1", {"name": "ABC Müller!"}) + assert "Kurzzeichen" in str(excInfo.value) or "Failed to update" in str(excInfo.value) + + def test_uppercaseNameRejected(self): + row = _row() + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + with pytest.raises(ValueError): + iface.updateMandate("m1", {"name": "ALPHA"}) + + def test_leadingHyphenRejected(self): + row = _row() + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + with pytest.raises(ValueError): + iface.updateMandate("m1", {"name": "-leading"}) + + def test_idempotentSameNameAccepted(self): + row = _row(mid="m1", name="alpha", label="Alpha") + db = _FakeDb([row, _row(mid="m2", name="beta", label="Beta")]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"name": "alpha"}) + assert updated.name == "alpha" + + def test_collisionWithOtherMandateRejected(self): + rows = [ + _row(mid="m1", name="alpha", label="Alpha"), + _row(mid="m2", name="beta", label="Beta"), + ] + db = _FakeDb(rows) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, rows[0]): + with pytest.raises(ValueError) as excInfo: + iface.updateMandate("m1", {"name": "beta"}) + assert "already in use" in str(excInfo.value) + + +class TestUpdateMandateLabelValidation: + def test_emptyLabelRejected(self): + row = _row() + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + with pytest.raises(ValueError) as excInfo: + iface.updateMandate("m1", {"label": " "}) + assert "label" in str(excInfo.value).lower() + + def test_labelTrimmed(self): + row = _row() + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"label": " Trimmed Name "}) + assert updated.label == "Trimmed Name" + + +class TestUpdateMandateProtectedFields: + def test_idCannotBeChanged(self): + row = _row(mid="m1", name="alpha", label="Alpha") + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"id": "spoofed", "label": "New"}) + assert str(updated.id) == "m1", "id field must remain immutable" + + def test_isSystemRequiresSysAdmin(self): + row = _row(mid="m1", name="alpha", label="Alpha", isSystem=False) + db = _FakeDb([row]) + iface = _buildInterface(db, isPlatformAdmin=True, isSysAdmin=False) + with _stubGetMandateAndRbac(iface, row): + updated = iface.updateMandate("m1", {"isSystem": True, "label": "New"}) + assert updated.isSystem is False, "PlatformAdmin alone must NOT escalate isSystem" diff --git a/tests/integration/rbac/test_platform_admin_flag.py b/tests/integration/rbac/test_platform_admin_flag.py new file mode 100644 index 00000000..d4cdbe9b --- /dev/null +++ b/tests/integration/rbac/test_platform_admin_flag.py @@ -0,0 +1,290 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Integration tests for the SysAdmin / PlatformAdmin authority split. + +Covers acceptance criteria from +``wiki/c-work/4-done/2026-04-sysadmin-authority-split.md``: + +- AC#1 -> User with isSysAdmin only is rejected by ``requirePlatformAdmin`` +- AC#2 -> User with isPlatformAdmin only is rejected by ``requireSysAdmin`` +- AC#3 -> User with isPlatformAdmin is accepted by ``requirePlatformAdmin`` +- AC#5 -> Live-flag check: revoking ``isPlatformAdmin`` immediately blocks + the next request (no token cache). +- AC#6 -> Live-flag check: revoking ``isSysAdmin`` immediately blocks + the next infrastructure request. +- AC#8 -> Self-protection: a user can never change their own admin flags + via ``update_user`` business logic. + +Strategy: build a tiny FastAPI app that exposes one route per dependency +and override ``getCurrentUser`` per request. This isolates the gating +logic from database/JWT plumbing and runs without external services. +""" + +from __future__ import annotations + +import pytest +from fastapi import Depends, FastAPI +from fastapi.testclient import TestClient + +from modules.auth.authentication import ( + getCurrentUser, + requirePlatformAdmin, + requireSysAdmin, +) +from modules.datamodels.datamodelUam import User + + +def _makeUser( + *, + userId: str = "test-user", + isSysAdmin: bool = False, + isPlatformAdmin: bool = False, +) -> User: + """Build a minimal in-memory User instance for dependency overrides.""" + return User( + id=userId, + username=f"user-{userId}", + email=f"{userId}@example.com", + fullName=f"Test {userId}", + enabled=True, + language="de", + isSysAdmin=isSysAdmin, + isPlatformAdmin=isPlatformAdmin, + ) + + +@pytest.fixture +def appWithDeps() -> tuple[FastAPI, dict]: + """FastAPI app with one route per authority dependency. + + The returned dict allows tests to swap the "current user" between + requests by mutating ``state['user']``. + """ + state: dict = {"user": _makeUser()} + + app = FastAPI() + + def _overrideCurrentUser() -> User: + return state["user"] + + app.dependency_overrides[getCurrentUser] = _overrideCurrentUser + + @app.get("/admin/mandates") + def _adminMandates(_: User = Depends(requirePlatformAdmin)) -> dict: + return {"ok": True, "guard": "platform"} + + @app.get("/admin/logs") + def _adminLogs(_: User = Depends(requireSysAdmin)) -> dict: + return {"ok": True, "guard": "sysadmin"} + + return app, state + + +# --------------------------------------------------------------------------- +# AC #1, #2, #3 — basic authority gating +# --------------------------------------------------------------------------- + + +def testSysAdminCannotAccessPlatformRoute(appWithDeps): + """AC#1: isSysAdmin alone must NOT pass requirePlatformAdmin.""" + app, state = appWithDeps + state["user"] = _makeUser(isSysAdmin=True, isPlatformAdmin=False) + + with TestClient(app) as client: + response = client.get("/admin/mandates") + + assert response.status_code == 403 + assert "platform admin" in response.json()["detail"].lower() + + +def testPlatformAdminCannotAccessInfraRoute(appWithDeps): + """AC#2: isPlatformAdmin alone must NOT pass requireSysAdmin.""" + app, state = appWithDeps + state["user"] = _makeUser(isSysAdmin=False, isPlatformAdmin=True) + + with TestClient(app) as client: + response = client.get("/admin/logs") + + assert response.status_code == 403 + assert "sysadmin" in response.json()["detail"].lower() + + +def testPlatformAdminCanAccessPlatformRoute(appWithDeps): + """AC#3: isPlatformAdmin must pass requirePlatformAdmin.""" + app, state = appWithDeps + state["user"] = _makeUser(isPlatformAdmin=True) + + with TestClient(app) as client: + response = client.get("/admin/mandates") + + assert response.status_code == 200 + assert response.json() == {"ok": True, "guard": "platform"} + + +def testSysAdminCanAccessInfraRoute(appWithDeps): + """Sanity counterpart to AC#3: isSysAdmin passes requireSysAdmin.""" + app, state = appWithDeps + state["user"] = _makeUser(isSysAdmin=True) + + with TestClient(app) as client: + response = client.get("/admin/logs") + + assert response.status_code == 200 + assert response.json() == {"ok": True, "guard": "sysadmin"} + + +def testNoFlagsIsForbiddenForBothGuards(appWithDeps): + """Regular user (no flags) must be rejected by both guards.""" + app, state = appWithDeps + state["user"] = _makeUser() + + with TestClient(app) as client: + rPlatform = client.get("/admin/mandates") + rInfra = client.get("/admin/logs") + + assert rPlatform.status_code == 403 + assert rInfra.status_code == 403 + + +# --------------------------------------------------------------------------- +# AC #5, #6 — live flag check (no client-side cache, next request re-evaluates) +# --------------------------------------------------------------------------- + + +def testRevokingPlatformAdminBlocksNextRequest(appWithDeps): + """AC#5: After dropping isPlatformAdmin, the very next request gets 403.""" + app, state = appWithDeps + state["user"] = _makeUser(isPlatformAdmin=True) + + with TestClient(app) as client: + first = client.get("/admin/mandates") + assert first.status_code == 200 + + # Admin removes the flag (e.g. via /api/users/{id}) + state["user"] = _makeUser(isPlatformAdmin=False) + + second = client.get("/admin/mandates") + assert second.status_code == 403 + + +def testRevokingSysAdminBlocksNextRequest(appWithDeps): + """AC#6: After dropping isSysAdmin, the very next request gets 403.""" + app, state = appWithDeps + state["user"] = _makeUser(isSysAdmin=True) + + with TestClient(app) as client: + first = client.get("/admin/logs") + assert first.status_code == 200 + + state["user"] = _makeUser(isSysAdmin=False) + + second = client.get("/admin/logs") + assert second.status_code == 403 + + +# --------------------------------------------------------------------------- +# AC #8 — self-protection on update_user +# --------------------------------------------------------------------------- + + +def testSelfProtectionOnUpdateUserDisallowsAdminFlagChange(): + """AC#8: A platform admin updating themselves cannot change admin flags. + + Mirrors the gating logic in ``routeDataUsers.update_user``: + + callerIsPlatformAdmin = context.isPlatformAdmin + allowAdminFlagChange = callerIsPlatformAdmin and not isSelfUpdate + + When ``isSelfUpdate`` is True the flag must always be ``False``, + regardless of the caller's authority. + """ + callerId = "user-1" + + def _allowAdminFlagChange(callerIsPlatformAdmin: bool, isSelfUpdate: bool) -> bool: + return callerIsPlatformAdmin and not isSelfUpdate + + # Self-update by a platform admin: still NOT allowed to flip own flags. + assert _allowAdminFlagChange(True, isSelfUpdate=(callerId == callerId)) is False + + # Foreign-update by a platform admin: allowed. + assert _allowAdminFlagChange(True, isSelfUpdate=(callerId == "user-2")) is True + + # Foreign-update by a non-platform admin: rejected. + assert _allowAdminFlagChange(False, isSelfUpdate=(callerId == "user-2")) is False + + +def testInterfaceUpdateUserProtectsAdminFlagsWhenForbidden(): + """``interfaceDbApp.AppObjects.updateUser`` must keep the existing + ``isSysAdmin``/``isPlatformAdmin`` values when ``allowAdminFlagChange`` + is False — even if the request payload tries to escalate them. + + This is the second line of defence behind ``update_user``'s + ``isSelfUpdate`` check. + """ + from unittest.mock import Mock + + from modules.interfaces.interfaceDbApp import AppObjects + + existing = User( + id="victim", + username="victim", + email="victim@example.com", + fullName="Victim", + enabled=True, + language="de", + isSysAdmin=False, + isPlatformAdmin=False, + ) + + # Attacker payload tries to escalate both flags. + attackerPayload = User( + id="victim", + username="victim", + email="victim@example.com", + fullName="Victim", + enabled=True, + language="de", + isSysAdmin=True, + isPlatformAdmin=True, + ) + + captured: dict = {} + + def _captureUpdate(_model, _recordId, payload): + # Whether dict or User: extract flag values for assertion. + if hasattr(payload, "model_dump"): + data = payload.model_dump() + elif isinstance(payload, dict): + data = payload + else: + data = {"isSysAdmin": getattr(payload, "isSysAdmin", None), + "isPlatformAdmin": getattr(payload, "isPlatformAdmin", None)} + captured["isSysAdmin"] = data.get("isSysAdmin") + captured["isPlatformAdmin"] = data.get("isPlatformAdmin") + merged = {**existing.model_dump(), **{k: v for k, v in data.items() if v is not None}} + return merged + + fakeDb = Mock() + fakeDb.recordModify = Mock(side_effect=_captureUpdate) + + # Build the interface without going through __init__ (avoids real DB). + interface = AppObjects.__new__(AppObjects) + interface.currentUser = existing + interface.userId = existing.id + interface.mandateId = None + interface.featureInstanceId = None + interface.db = fakeDb + interface.rbac = Mock(checkRbacPermission=Mock(return_value=True)) + interface.getUser = Mock(return_value=existing) + + interface.updateUser("victim", attackerPayload, allowAdminFlagChange=False) + + assert captured.get("isSysAdmin") is False, ( + "isSysAdmin must remain False when allowAdminFlagChange=False, " + f"got {captured.get('isSysAdmin')!r}" + ) + assert captured.get("isPlatformAdmin") is False, ( + "isPlatformAdmin must remain False when allowAdminFlagChange=False, " + f"got {captured.get('isPlatformAdmin')!r}" + ) diff --git a/tests/integration/users/__init__.py b/tests/integration/users/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/integration/users/test_updateUser.py b/tests/integration/users/test_updateUser.py new file mode 100644 index 00000000..1c7afa29 --- /dev/null +++ b/tests/integration/users/test_updateUser.py @@ -0,0 +1,221 @@ +# Copyright (c) 2026 Patrick Motsch +# All rights reserved. +""" +Integration tests for ``AppObjects.updateUser`` partial-update semantics. + +Regression for the silent flag-flip bug (``isSysAdmin`` <-> ``isPlatformAdmin``) +on inline toggles in ``Admin > System > Mandanten/Benutzer``: + +Symptom +------- +Toggling one privileged flag in the user table flipped the OTHER privileged +flag back to its Pydantic default (``False``). + +Root cause +---------- +The PUT ``/api/users/{id}`` route bound ``userData: User = Body(...)``. Pydantic +filled every field that the client did not explicitly send with model defaults +(``isSysAdmin=False``, ``isPlatformAdmin=False``). Combined with +``allowAdminFlagChange=True`` (PlatformAdmin updating another user), those +defaults were merged into the persisted record and silently overwrote the +"other" flag. + +Fix +--- +The route now accepts a plain ``Dict[str, Any]`` and ``AppObjects.updateUser`` +treats the payload as a true partial patch — only the keys present in the +request body are applied to the stored record. Pydantic ``User`` callers are +still supported via ``model_dump(exclude_unset=True)`` so legacy paths keep +working without re-introducing the default-fill regression. + +These tests assert the partial-update contract end-to-end at the interface +level, which is the layer where the bug lived. +""" + +from __future__ import annotations + +from typing import Any, Dict, List, Optional +from unittest.mock import Mock, patch + +import pytest + +from modules.datamodels.datamodelUam import User, UserInDB +from modules.interfaces.interfaceDbApp import AppObjects + + +class _FakeDb: + """Minimal connector covering the access patterns used by ``updateUser``.""" + + def __init__(self, rows: List[Dict[str, Any]]): + self.rows: List[Dict[str, Any]] = [dict(r) for r in rows] + self.modifyCalls: List[Dict[str, Any]] = [] + + def getRecordset(self, model, recordFilter: Optional[Dict[str, Any]] = None): + if model is not UserInDB and model is not User: + return [] + if not recordFilter: + return [dict(r) for r in self.rows] + out = [] + for r in self.rows: + if all(r.get(k) == v for k, v in recordFilter.items()): + out.append(dict(r)) + return out + + def recordModify(self, model, recordId: str, payload): + if hasattr(payload, "model_dump"): + data = payload.model_dump() + elif isinstance(payload, dict): + data = dict(payload) + else: + data = {} + self.modifyCalls.append({"id": str(recordId), "data": dict(data)}) + for r in self.rows: + if str(r.get("id")) == str(recordId): + r.update(data) + return r + return None + + +def _buildInterface(db: _FakeDb) -> AppObjects: + iface = AppObjects.__new__(AppObjects) + iface.db = db + iface.currentUser = Mock(id="caller", isPlatformAdmin=True, isSysAdmin=False) + iface.userId = "caller" + iface.mandateId = None + iface.featureInstanceId = None + iface.rbac = Mock() + return iface + + +def _row(uid: str = "u1", **extra) -> Dict[str, Any]: + base: Dict[str, Any] = { + "id": uid, + "username": "alice", + "email": "alice@example.com", + "fullName": "Alice Example", + "language": "de", + "enabled": True, + "isSysAdmin": False, + "isPlatformAdmin": True, + "authenticationAuthority": "local", + "roleLabels": [], + } + base.update(extra) + return base + + +def _stubGetUser(iface: AppObjects): + """Wire ``getUser`` to read from the FakeDb so post-update reads reflect changes.""" + db = iface.db + + def _readUser(userId: str): + for r in db.rows: + if str(r.get("id")) == str(userId): + return User(**r) + return None + + iface.getUser = Mock(side_effect=_readUser) + + +class TestPartialUpdateProtectsSiblingFlag: + """The headline regression: toggling one flag must not touch the other.""" + + def test_togglingIsSysAdminKeepsIsPlatformAdmin(self): + row = _row(isSysAdmin=False, isPlatformAdmin=True) + db = _FakeDb([row]) + iface = _buildInterface(db) + _stubGetUser(iface) + + updated = iface.updateUser( + "u1", + {"isSysAdmin": True}, # only the toggled cell + allowAdminFlagChange=True, + ) + + assert updated.isSysAdmin is True + assert updated.isPlatformAdmin is True, ( + "Partial update must not silently drop isPlatformAdmin to its default" + ) + + def test_togglingIsPlatformAdminKeepsIsSysAdmin(self): + row = _row(isSysAdmin=True, isPlatformAdmin=False) + db = _FakeDb([row]) + iface = _buildInterface(db) + _stubGetUser(iface) + + updated = iface.updateUser( + "u1", + {"isPlatformAdmin": True}, + allowAdminFlagChange=True, + ) + + assert updated.isPlatformAdmin is True + assert updated.isSysAdmin is True, ( + "Partial update must not silently drop isSysAdmin to its default" + ) + + def test_togglingUnrelatedFieldKeepsBothFlags(self): + row = _row(isSysAdmin=True, isPlatformAdmin=True, language="de") + db = _FakeDb([row]) + iface = _buildInterface(db) + _stubGetUser(iface) + + updated = iface.updateUser( + "u1", + {"language": "en"}, + allowAdminFlagChange=False, + ) + + assert updated.language == "en" + assert updated.isSysAdmin is True + assert updated.isPlatformAdmin is True + + +class TestPrivilegedFlagGuard: + """Without ``allowAdminFlagChange`` the protected flags must be dropped.""" + + def test_protectedFlagsDroppedWhenChangeNotAllowed(self): + row = _row(isSysAdmin=False, isPlatformAdmin=True) + db = _FakeDb([row]) + iface = _buildInterface(db) + _stubGetUser(iface) + + updated = iface.updateUser( + "u1", + {"isSysAdmin": True, "isPlatformAdmin": False, "language": "fr"}, + allowAdminFlagChange=False, + ) + + assert updated.language == "fr", "non-protected fields still apply" + assert updated.isSysAdmin is False, "protected flag must be ignored" + assert updated.isPlatformAdmin is True, "protected flag must be ignored" + + def test_legacyPydanticUserDoesNotDefaultFlipFlags(self): + """Defense in depth: if a caller still passes a ``User`` instance, + ``model_dump(exclude_unset=True)`` must keep unset fields out of the + merge so they do not pull live values down to Pydantic defaults. + """ + row = _row(isSysAdmin=True, isPlatformAdmin=True, fullName="Alice Example") + db = _FakeDb([row]) + iface = _buildInterface(db) + _stubGetUser(iface) + + partialModel = User( + id="u1", + username="alice", + fullName="Alice Updated", + ) + + updated = iface.updateUser( + "u1", + partialModel, + allowAdminFlagChange=True, + ) + + assert updated.fullName == "Alice Updated" + assert updated.isSysAdmin is True, ( + "Pydantic User without explicit isSysAdmin must not flip stored True to default False" + ) + assert updated.isPlatformAdmin is True, ( + "Pydantic User without explicit isPlatformAdmin must not flip stored True to default False" + ) diff --git a/tests/test_phase123_basic.py b/tests/test_phase123_basic.py index 89d23662..59a3234d 100644 --- a/tests/test_phase123_basic.py +++ b/tests/test_phase123_basic.py @@ -149,7 +149,7 @@ try: source = f.read() _check("routeDataFiles has PATCH scope endpoint", "updateFileScope" in source) _check("routeDataFiles has PATCH neutralize endpoint", "updateFileNeutralize" in source) - _check("routeDataFiles checks global sysAdmin", "hasSysAdminRole" in source or "sysadmin" in source.lower()) + _check("routeDataFiles checks global sysAdmin", "isSysAdmin" in source) except Exception as e: errors.append(f"Phase 2 Routes: {e}") print(f" [FAIL] Phase 2 Routes: {e}") diff --git a/tests/unit/bootstrap/__init__.py b/tests/unit/bootstrap/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/bootstrap/test_mandateNameMigration.py b/tests/unit/bootstrap/test_mandateNameMigration.py new file mode 100644 index 00000000..d09a6846 --- /dev/null +++ b/tests/unit/bootstrap/test_mandateNameMigration.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Unit tests for ``_migrateMandateNameLabelSlugRules`` in interfaceBootstrap. + +Covers: +- legacy ``name``/``label`` rows get fixed (label fill, slug rename), +- collisions across legacy rows resolve via -2/-3 suffixes in stable id order, +- valid rows are left untouched (idempotency), +- second invocation is a no-op. +""" + +from typing import Any, Dict, List, Optional + +import pytest + +from modules.datamodels.datamodelUam import Mandate +from modules.interfaces.interfaceBootstrap import _migrateMandateNameLabelSlugRules +from modules.shared.mandateNameUtils import isValidMandateName + + +class _FakeDb: + """Minimal connector simulating getRecordset(Mandate)+recordModify(Mandate, id, data).""" + + def __init__(self, rows: List[Dict[str, Any]]): + self.rows: List[Dict[str, Any]] = [dict(r) for r in rows] + self.modifyCalls: List[Dict[str, Any]] = [] + + def getRecordset(self, model, recordFilter: Optional[Dict[str, Any]] = None): + if model is not Mandate: + return [] + if not recordFilter: + return [dict(r) for r in self.rows] + out = [] + for r in self.rows: + if all(r.get(k) == v for k, v in recordFilter.items()): + out.append(dict(r)) + return out + + def recordModify(self, model, recordId: str, data: Dict[str, Any]): + self.modifyCalls.append({"id": str(recordId), "data": dict(data)}) + for r in self.rows: + if str(r.get("id")) == str(recordId): + r.update(data) + return r + return None + + +def _row(mid: str, name: Any, label: Any = None) -> Dict[str, Any]: + return {"id": mid, "name": name, "label": label} + + +class TestMigrationFillsLabel: + def test_emptyLabelGetsNameAsLabel(self): + db = _FakeDb([_row("a1", "good-name", None)]) + _migrateMandateNameLabelSlugRules(db) + assert db.rows[0]["label"] == "good-name" + assert db.rows[0]["name"] == "good-name" + + def test_emptyLabelAndEmptyNameFallsBackToMandate(self): + db = _FakeDb([_row("a1", "", "")]) + _migrateMandateNameLabelSlugRules(db) + assert db.rows[0]["label"] == "Mandate" + assert isValidMandateName(db.rows[0]["name"]) + + +class TestMigrationRenamesInvalidNames: + def test_invalidNameGetsSlugFromLabel(self): + db = _FakeDb([_row("a1", "Home patrick", "Home Patrick")]) + _migrateMandateNameLabelSlugRules(db) + assert db.rows[0]["name"] == "home-patrick" + assert db.rows[0]["label"] == "Home Patrick" + + def test_umlautsTransliterated(self): + db = _FakeDb([_row("a1", "Müller AG", "Müller AG")]) + _migrateMandateNameLabelSlugRules(db) + assert db.rows[0]["name"] == "mueller-ag" + + +class TestMigrationCollisions: + def test_collisionsResolveByStableIdOrder(self): + rows = [ + _row("z1", "Home patrick", "Home Patrick"), + _row("a1", "home-patrick", "Home Patrick Two"), + ] + db = _FakeDb(rows) + _migrateMandateNameLabelSlugRules(db) + byId = {r["id"]: r for r in db.rows} + assert byId["a1"]["name"] == "home-patrick" + assert byId["z1"]["name"] == "home-patrick-2" + + def test_threeWayCollisionGetsThirdSuffix(self): + rows = [ + _row("id-aaa", "home-patrick", "Home Patrick"), + _row("id-bbb", "Home patrick", "Home Patrick"), + _row("id-ccc", "home patrick", "Home Patrick"), + ] + db = _FakeDb(rows) + _migrateMandateNameLabelSlugRules(db) + names = sorted(r["name"] for r in db.rows) + assert names == ["home-patrick", "home-patrick-2", "home-patrick-3"] + + +class TestMigrationIdempotency: + def test_secondRunIsNoop(self): + rows = [ + _row("a1", "home-patrick", "Home Patrick"), + _row("b1", "Home Müller", ""), + ] + db = _FakeDb(rows) + _migrateMandateNameLabelSlugRules(db) + assert all(isValidMandateName(r["name"]) for r in db.rows) + firstChanges = list(db.modifyCalls) + db.modifyCalls.clear() + _migrateMandateNameLabelSlugRules(db) + assert db.modifyCalls == [], ( + f"expected no further changes after first migration, got {db.modifyCalls}; " + f"firstRun changes: {firstChanges}" + ) + + def test_validRowsLeftUntouched(self): + rows = [_row("a1", "root", "Root"), _row("b1", "alpina-treuhand", "Alpina Treuhand AG")] + db = _FakeDb(rows) + _migrateMandateNameLabelSlugRules(db) + assert db.modifyCalls == [] + + +class TestMigrationEmpty: + def test_emptyDbDoesNothing(self): + db = _FakeDb([]) + _migrateMandateNameLabelSlugRules(db) + assert db.modifyCalls == [] diff --git a/tests/unit/rbac/test_sysadmin_migration.py b/tests/unit/rbac/test_sysadmin_migration.py new file mode 100644 index 00000000..8ca077bf --- /dev/null +++ b/tests/unit/rbac/test_sysadmin_migration.py @@ -0,0 +1,209 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Unit tests for the one-shot sysadmin role -> isPlatformAdmin migration. + +Covers acceptance criteria from +``wiki/c-work/4-done/2026-04-sysadmin-authority-split.md``: + +- AC#4 -> Existing sysadmin role-holders are promoted to ``isPlatformAdmin=True`` + and the legacy role is removed (Role + UserMandateRole + AccessRules) + when the gateway boots. +- AC#10 -> The migration is idempotent and removes ALL artefacts (Role, + AccessRules, UserMandateRole) of the legacy ``sysadmin`` role. + +Strategy: use an in-memory fake ``DatabaseConnector`` that records calls +and returns deterministic recordsets for ``Role``/``UserMandateRole``/ +``UserMandate``/``UserInDB``/``AccessRule`` lookups. +""" + +from __future__ import annotations + +from typing import Any, Dict, List +from unittest.mock import Mock + +from modules.interfaces.interfaceBootstrap import _migrateAndDropSysAdminRole +from modules.datamodels.datamodelMembership import UserMandate, UserMandateRole +from modules.datamodels.datamodelRbac import AccessRule, Role +from modules.datamodels.datamodelUam import UserInDB + + +_ROOT_MANDATE_ID = "root-mandate-id" +_SYSADMIN_ROLE_ID = "sysadmin-role-id" +_USER_MANDATE_ID = "user-mandate-id" +_USER_ID = "legacy-user-id" +_UMR_ROW_ID = "umr-row-id" +_ACCESS_RULE_ID = "access-rule-id" + + +def _buildFakeDb( + *, + sysadminRoles: List[Dict[str, Any]], + umRoleRows: List[Dict[str, Any]], + userMandateRows: List[Dict[str, Any]], + users: List[Dict[str, Any]], + accessRules: List[Dict[str, Any]], +) -> Mock: + """Build a fake ``DatabaseConnector`` that maps model -> recordset.""" + + deletes: List[tuple] = [] + modifies: List[tuple] = [] + + def _getRecordset(model, recordFilter=None, **_): # noqa: ANN001 + recordFilter = recordFilter or {} + if model is Role: + label = recordFilter.get("roleLabel") + mandateId = recordFilter.get("mandateId") + if label == "sysadmin" and mandateId == _ROOT_MANDATE_ID: + return list(sysadminRoles) + return [] + if model is UserMandateRole: + wanted = recordFilter.get("roleId") + return [r for r in umRoleRows if r.get("roleId") == wanted] + if model is UserMandate: + wanted = recordFilter.get("id") + return [r for r in userMandateRows if r.get("id") == wanted] + if model is UserInDB: + wanted = recordFilter.get("id") + return [r for r in users if r.get("id") == wanted] + if model is AccessRule: + wanted = recordFilter.get("roleId") + return [r for r in accessRules if r.get("roleId") == wanted] + return [] + + def _recordModify(model, recordId, payload): # noqa: ANN001 + modifies.append((model, recordId, payload)) + # Reflect the change so a subsequent migration call is idempotent. + if model is UserInDB: + for u in users: + if u.get("id") == recordId: + u.update(payload) + return True + + def _recordDelete(model, recordId): # noqa: ANN001 + deletes.append((model, recordId)) + if model is UserMandateRole: + umRoleRows[:] = [r for r in umRoleRows if r.get("id") != recordId] + elif model is AccessRule: + accessRules[:] = [r for r in accessRules if r.get("id") != recordId] + elif model is Role: + sysadminRoles[:] = [r for r in sysadminRoles if r.get("id") != recordId] + return True + + db = Mock() + db.getRecordset = Mock(side_effect=_getRecordset) + db.recordModify = Mock(side_effect=_recordModify) + db.recordDelete = Mock(side_effect=_recordDelete) + db._modifies = modifies # exposed for assertions + db._deletes = deletes + return db + + +def _seed(): + return { + "sysadminRoles": [{"id": _SYSADMIN_ROLE_ID, "roleLabel": "sysadmin", + "mandateId": _ROOT_MANDATE_ID}], + "umRoleRows": [{"id": _UMR_ROW_ID, "roleId": _SYSADMIN_ROLE_ID, + "userMandateId": _USER_MANDATE_ID}], + "userMandateRows": [{"id": _USER_MANDATE_ID, "userId": _USER_ID, + "mandateId": _ROOT_MANDATE_ID}], + "users": [{"id": _USER_ID, "username": "legacy", + "isSysAdmin": False, "isPlatformAdmin": False}], + "accessRules": [{"id": _ACCESS_RULE_ID, "roleId": _SYSADMIN_ROLE_ID}], + } + + +# --------------------------------------------------------------------------- +# AC #4 — promote + drop on first run +# --------------------------------------------------------------------------- + + +def testMigrationPromotesUserAndDropsArtefacts(): + """AC#4: legacy holder is promoted; Role+AccessRule+UMR are deleted.""" + seed = _seed() + db = _buildFakeDb(**seed) + + _migrateAndDropSysAdminRole(db, _ROOT_MANDATE_ID) + + # User got isPlatformAdmin=True + assert seed["users"][0]["isPlatformAdmin"] is True + assert any( + m[0] is UserInDB and m[2] == {"isPlatformAdmin": True} + for m in db._modifies + ), "Expected UserInDB.isPlatformAdmin promotion call" + + # All three artefact tables had their rows deleted. + deletedModels = {m[0] for m in db._deletes} + assert UserMandateRole in deletedModels, "UserMandateRole row not deleted" + assert AccessRule in deletedModels, "AccessRule row not deleted" + assert Role in deletedModels, "Sysadmin Role record not deleted" + + # And the seeded lists are empty after the migration. + assert seed["umRoleRows"] == [] + assert seed["accessRules"] == [] + assert seed["sysadminRoles"] == [] + + +# --------------------------------------------------------------------------- +# AC #10 — idempotent: a second run is a no-op +# --------------------------------------------------------------------------- + + +def testMigrationIsIdempotent(): + """AC#10: a second invocation finds no sysadmin role and exits silently.""" + seed = _seed() + db = _buildFakeDb(**seed) + + _migrateAndDropSysAdminRole(db, _ROOT_MANDATE_ID) + firstModifies = list(db._modifies) + firstDeletes = list(db._deletes) + + _migrateAndDropSysAdminRole(db, _ROOT_MANDATE_ID) + + # No additional writes on the second call. + assert db._modifies == firstModifies, ( + "Second migration call must not perform additional writes" + ) + assert db._deletes == firstDeletes, ( + "Second migration call must not perform additional deletes" + ) + + +def testMigrationSkipsAlreadyPromotedUsers(): + """If a user already has ``isPlatformAdmin=True``, no redundant write.""" + seed = _seed() + seed["users"][0]["isPlatformAdmin"] = True # already promoted + db = _buildFakeDb(**seed) + + _migrateAndDropSysAdminRole(db, _ROOT_MANDATE_ID) + + # No promotion write for an already-promoted user. + promotionWrites = [ + m for m in db._modifies + if m[0] is UserInDB and m[2].get("isPlatformAdmin") is True + ] + assert promotionWrites == [], ( + "Should not re-write isPlatformAdmin if user already has it" + ) + + # But role + access-rule cleanup still happens. + deletedModels = {m[0] for m in db._deletes} + assert Role in deletedModels + assert AccessRule in deletedModels + assert UserMandateRole in deletedModels + + +def testMigrationOnEmptyDbIsNoop(): + """No legacy sysadmin role at all -> no calls, no errors.""" + db = _buildFakeDb( + sysadminRoles=[], + umRoleRows=[], + userMandateRows=[], + users=[], + accessRules=[], + ) + + _migrateAndDropSysAdminRole(db, _ROOT_MANDATE_ID) + + assert db._modifies == [] + assert db._deletes == [] diff --git a/tests/unit/shared/test_mandateNameUtils.py b/tests/unit/shared/test_mandateNameUtils.py new file mode 100644 index 00000000..6ef4bec1 --- /dev/null +++ b/tests/unit/shared/test_mandateNameUtils.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +"""Unit tests for mandateNameUtils (slug, validation, unique allocation).""" + +import pytest + +from modules.shared.mandateNameUtils import ( + allocateUniqueMandateSlug, + isValidMandateName, + slugifyMandateName, + transliterateGerman, +) + + +class TestTransliterateGerman: + def test_transliterateGerman_umlauts(self): + assert transliterateGerman("Müller") == "Mueller" + assert transliterateGerman("Größe") == "Groesse" + assert transliterateGerman("Fußball") == "Fussball" + + +class TestIsValidMandateName: + def test_isValidMandateName_ok(self): + assert isValidMandateName("ab") is True + assert isValidMandateName("a-b") is True + assert isValidMandateName("root") is True + assert isValidMandateName("mn-2") is True + + def test_isValidMandateName_rejects(self): + assert isValidMandateName("a") is False + assert isValidMandateName("") is False + assert isValidMandateName("Home patrick") is False + assert isValidMandateName("UPPER") is False + assert isValidMandateName("a--b") is False + assert isValidMandateName("-ab") is False + assert isValidMandateName("ab-") is False + + +class TestSlugifyMandateName: + def test_slugifyMandateName_basic(self): + assert slugifyMandateName("Müller AG") == "mueller-ag" + assert slugifyMandateName(" Foo Bar ") == "foo-bar" + + def test_slugifyMandateName_empty(self): + assert slugifyMandateName("") == "mn" + assert slugifyMandateName(" ") == "mn" + + +class TestAllocateUniqueMandateSlug: + def test_allocateUniqueMandateSlug_first_free(self): + assert allocateUniqueMandateSlug("mueller-ag", ["other"]) == "mueller-ag" + + def test_allocateUniqueMandateSlug_collision(self): + assert allocateUniqueMandateSlug("mueller-ag", ["mueller-ag"]) == "mueller-ag-2" + assert allocateUniqueMandateSlug("mueller-ag", ["mueller-ag", "mueller-ag-2"]) == "mueller-ag-3" From 24ff6058d5a27c776c307cb1ee55a8f574d918ad Mon Sep 17 00:00:00 2001 From: ValueOn AG Date: Sun, 19 Apr 2026 00:19:42 +0200 Subject: [PATCH 2/5] fixed ai workspace voice google issue --- modules/connectors/connectorVoiceGoogle.py | 48 ++++++++++++++----- modules/interfaces/interfaceVoiceObjects.py | 17 ++++--- .../serviceAgent/coreTools/_workspaceTools.py | 4 +- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/modules/connectors/connectorVoiceGoogle.py b/modules/connectors/connectorVoiceGoogle.py index aebede8a..1dc0912b 100644 --- a/modules/connectors/connectorVoiceGoogle.py +++ b/modules/connectors/connectorVoiceGoogle.py @@ -562,16 +562,34 @@ class ConnectorGoogleSpeech: """Google TTS WaveNet cost: ~$0.000004/char.""" return round(characterCount * 0.000004, 8) + @staticmethod + def _normalizeLanguageCode(code: Optional[str]) -> Optional[str]: + """Normalize a user/LLM-supplied language hint to an ISO-639-1 code or None. + + Google Cloud Translation v2 only accepts ISO codes (e.g. 'de', 'en') or + an omitted source for auto-detection. Strings like 'auto', '' or full + BCP-47 tags ('de-DE') would otherwise reach the API and trigger + '400 Invalid Value'. Centralising the mapping here keeps every caller + (tools, interface, internal pipelines) safe. + """ + if not code: + return None + normalized = code.strip().lower() + if not normalized or normalized in ("auto", "detect", "any", "*"): + return None + return normalized.split("-")[0] + async def translateText(self, text: str, targetLanguage: str = "en", - sourceLanguage: str = "de") -> Dict: + sourceLanguage: Optional[str] = None) -> Dict: """ Translate text using Google Cloud Translation API. - + Args: text: Text to translate - target_language: Target language code (e.g., 'en', 'de') - source_language: Source language code (e.g., 'de', 'en') - + targetLanguage: Target language code (e.g., 'en', 'de') + sourceLanguage: Source language code (e.g., 'de', 'en'); pass None + or 'auto' for Google's auto-detection. + Returns: Dict containing translated text and metadata """ @@ -583,14 +601,18 @@ class ConnectorGoogleSpeech: "translated_text": "", "error": "Empty text provided" } - - logger.info(f"🌐 Translating: '{text}' ({sourceLanguage} -> {targetLanguage})") - - # Perform translation + + normalizedSource = self._normalizeLanguageCode(sourceLanguage) + normalizedTarget = self._normalizeLanguageCode(targetLanguage) or "en" + logger.info( + f"🌐 Translating: '{text}' " + f"({normalizedSource or 'auto'} -> {normalizedTarget})" + ) + result = self.translate_client.translate( text, - source_language=sourceLanguage, - target_language=targetLanguage + source_language=normalizedSource, + target_language=normalizedTarget, ) translatedText = result['translatedText'] @@ -708,8 +730,8 @@ class ConnectorGoogleSpeech: # Step 2: Translation translationResult = await self.translateText( text=originalText, - sourceLanguage=fromLanguage.split('-')[0], # Convert 'de-DE' to 'de' - targetLanguage=toLanguage.split('-')[0] # Convert 'en-US' to 'en' + sourceLanguage=fromLanguage, + targetLanguage=toLanguage, ) if not translationResult["success"]: diff --git a/modules/interfaces/interfaceVoiceObjects.py b/modules/interfaces/interfaceVoiceObjects.py index 38807bac..69962259 100644 --- a/modules/interfaces/interfaceVoiceObjects.py +++ b/modules/interfaces/interfaceVoiceObjects.py @@ -181,21 +181,26 @@ class VoiceObjects: "error": str(e) } - async def translateText(self, text: str, sourceLanguage: str = "de", + async def translateText(self, text: str, + sourceLanguage: Optional[str] = None, targetLanguage: str = "en") -> Dict[str, Any]: """ Translate text using Google Cloud Translation API. - + Args: text: Text to translate - sourceLanguage: Source language code (e.g., 'de', 'en') - targetLanguage: Target language code (e.g., 'en', 'de') - + sourceLanguage: Source language ISO code (e.g. 'de', 'en'); pass None + or 'auto' to let Google auto-detect. + targetLanguage: Target language ISO code (e.g. 'en', 'de') + Returns: Dict containing translated text and metadata """ try: - logger.info(f"🌐 Translation request: '{text}' ({sourceLanguage} -> {targetLanguage})") + logger.info( + f"🌐 Translation request: '{text}' " + f"({sourceLanguage or 'auto'} -> {targetLanguage})" + ) if not text.strip(): return { diff --git a/modules/serviceCenter/services/serviceAgent/coreTools/_workspaceTools.py b/modules/serviceCenter/services/serviceAgent/coreTools/_workspaceTools.py index 98ee94b9..39a246ce 100644 --- a/modules/serviceCenter/services/serviceAgent/coreTools/_workspaceTools.py +++ b/modules/serviceCenter/services/serviceAgent/coreTools/_workspaceTools.py @@ -670,7 +670,7 @@ def _registerWorkspaceTools(registry: ToolRegistry, services): from modules.interfaces.interfaceVoiceObjects import getVoiceInterface mandateId = context.get("mandateId", "") voiceInterface = getVoiceInterface(currentUser=None, mandateId=mandateId) - sourceLanguage = args.get("sourceLanguage", "auto") + sourceLanguage = args.get("sourceLanguage") or None result = await voiceInterface.translateText(text, sourceLanguage=sourceLanguage, targetLanguage=targetLanguage) if result and result.get("success"): translated = result.get("translated_text", "") @@ -735,7 +735,7 @@ def _registerWorkspaceTools(registry: ToolRegistry, services): "properties": { "text": {"type": "string", "description": "Text to translate"}, "targetLanguage": {"type": "string", "description": "Target language ISO code (e.g. 'en', 'de', 'fr')"}, - "sourceLanguage": {"type": "string", "description": "Source language ISO code (default: auto-detect)"}, + "sourceLanguage": {"type": "string", "description": "Source language ISO code (e.g. 'de', 'en'). Omit or leave empty for auto-detection."}, }, "required": ["text", "targetLanguage"] }, From 3ea85fe57eaeed7a1657840b5e8f783992f2cf07 Mon Sep 17 00:00:00 2001 From: ValueOn AG Date: Sun, 19 Apr 2026 00:36:45 +0200 Subject: [PATCH 3/5] centralized language catalog --- modules/connectors/connectorVoiceGoogle.py | 115 +++++---------- modules/interfaces/interfaceVoiceObjects.py | 35 +---- modules/routes/routeVoiceGoogle.py | 36 ++--- modules/routes/routeVoiceUser.py | 11 +- .../serviceAgent/coreTools/_mediaTools.py | 12 +- modules/shared/voiceCatalog.py | 136 ++++++++++++++++++ 6 files changed, 197 insertions(+), 148 deletions(-) create mode 100644 modules/shared/voiceCatalog.py diff --git a/modules/connectors/connectorVoiceGoogle.py b/modules/connectors/connectorVoiceGoogle.py index 1dc0912b..f875c72c 100644 --- a/modules/connectors/connectorVoiceGoogle.py +++ b/modules/connectors/connectorVoiceGoogle.py @@ -15,6 +15,7 @@ from google.cloud import speech from google.cloud import translate_v2 as translate from google.cloud import texttospeech from modules.shared.configuration import APP_CONFIG +from modules.shared.voiceCatalog import getDefaultVoice as _catalogDefaultVoice logger = logging.getLogger(__name__) @@ -940,33 +941,26 @@ class ConnectorGoogleSpeech: stripped = voiceName.strip() return bool(stripped) and "-" not in stripped - async def textToSpeech(self, text: str, languageCode: str = "de-DE", voiceName: str = None) -> Dict[str, Any]: + async def textToSpeech(self, text: str, languageCode: str = "de-DE", voiceName: Optional[str] = None) -> Dict[str, Any]: """ Convert text to speech using Google Cloud Text-to-Speech. - + Args: text: Text to convert to speech - language_code: Language code (e.g., 'de-DE', 'en-US') - voice_name: Specific voice name (optional) - + languageCode: BCP-47 language code (e.g., 'de-DE', 'en-US', 'ru-RU') + voiceName: Specific voice name (optional). If omitted, a curated + default is used; if no curated default exists for the language, + Google selects a default voice automatically based on + languageCode + ssml_gender (no hard failure). + Returns: Dict with success status and audio data """ try: logger.info(f"Converting text to speech: '{text[:50]}...' in {languageCode}") - - # Build the voice request + selectedVoice = voiceName or self._getDefaultVoice(languageCode) - - if not selectedVoice: - return { - "success": False, - "error": f"No voice specified for language {languageCode}. Please select a voice." - } - - logger.info(f"Using TTS voice: {selectedVoice} for language: {languageCode}") - - isGeminiVoice = self._isGeminiTtsSpeakerVoiceName(selectedVoice) + isGeminiVoice = self._isGeminiTtsSpeakerVoiceName(selectedVoice) if selectedVoice else False if isGeminiVoice: synthesisInput = texttospeech.SynthesisInput( @@ -981,11 +975,23 @@ class ConnectorGoogleSpeech: ) else: synthesisInput = texttospeech.SynthesisInput(text=text) - voice = texttospeech.VoiceSelectionParams( - language_code=languageCode, - name=selectedVoice, - ssml_gender=texttospeech.SsmlVoiceGender.NEUTRAL, - ) + voiceKwargs: Dict[str, Any] = { + "language_code": languageCode, + "ssml_gender": texttospeech.SsmlVoiceGender.NEUTRAL, + } + if selectedVoice: + voiceKwargs["name"] = selectedVoice + else: + logger.info( + f"TTS: no curated voice for '{languageCode}', " + f"letting Google auto-select by language + gender" + ) + voice = texttospeech.VoiceSelectionParams(**voiceKwargs) + + logger.info( + f"Using TTS voice: {selectedVoice or ''} " + f"for language: {languageCode}" + ) audioConfig = texttospeech.AudioConfig( audio_encoding=texttospeech.AudioEncoding.MP3 @@ -994,16 +1000,15 @@ class ConnectorGoogleSpeech: response = self.tts_client.synthesize_speech( input=synthesisInput, voice=voice, - audio_config=audioConfig + audio_config=audioConfig, ) - # Return the audio content return { "success": True, "audio_content": response.audio_content, "audio_format": "mp3", "language_code": languageCode, - "voice_name": voice.name + "voice_name": selectedVoice or "", } except Exception as e: @@ -1018,59 +1023,15 @@ class ConnectorGoogleSpeech: "error": f"Text-to-Speech failed: {detail}{extra}", } - def _getDefaultVoice(self, languageCode: str) -> str: + def _getDefaultVoice(self, languageCode: str) -> Optional[str]: + """Return the curated default Google TTS voice for `languageCode`. + + Delegates to the central voice catalog; returns None when no curated + voice exists, in which case the caller omits `name` and Google + auto-selects based on languageCode + ssml_gender. """ - Get default voice name for a language code. - Falls back to a Wavenet voice for common languages. - """ - _defaults = { - "de-DE": "de-DE-Wavenet-A", - "de-CH": "de-DE-Wavenet-A", - "en-US": "en-US-Wavenet-C", - "en-GB": "en-GB-Wavenet-A", - "fr-FR": "fr-FR-Wavenet-A", - "it-IT": "it-IT-Wavenet-A", - } - return _defaults.get(languageCode) - - async def getAvailableLanguages(self) -> Dict[str, Any]: - """ - Get available languages from Google Cloud Text-to-Speech. - - Returns: - Dict containing success status and list of available languages - """ - try: - logger.info("🌐 Getting available languages from Google Cloud TTS") - - # List voices from Google Cloud TTS - response = self.tts_client.list_voices() - - # Extract unique language codes - # Note: Google TTS API doesn't provide language descriptions, only codes - language_codes = set() - for voice in response.voices: - if voice.language_codes: - language_codes.update(voice.language_codes) - - # Convert to sorted list of language codes - available_languages = sorted(list(language_codes)) - - logger.info(f"✅ Found {len(available_languages)} available languages") - - return { - "success": True, - "languages": available_languages - } - - except Exception as e: - logger.error(f"❌ Failed to get available languages: {e}") - return { - "success": False, - "error": str(e), - "languages": [] - } - + return _catalogDefaultVoice(languageCode) + async def getAvailableVoices(self, languageCode: Optional[str] = None) -> Dict[str, Any]: """ Get available voices from Google Cloud Text-to-Speech. diff --git a/modules/interfaces/interfaceVoiceObjects.py b/modules/interfaces/interfaceVoiceObjects.py index 69962259..d0b6f461 100644 --- a/modules/interfaces/interfaceVoiceObjects.py +++ b/modules/interfaces/interfaceVoiceObjects.py @@ -338,36 +338,11 @@ class VoiceObjects: "error": str(e) } - # Language and Voice Information - - async def getAvailableLanguages(self) -> Dict[str, Any]: - """ - Get available languages from Google Cloud Text-to-Speech. - - Returns: - Dict containing success status and list of available languages - """ - try: - logger.info("🌐 Getting available languages from Google Cloud TTS") - - connector = self._getGoogleSpeechConnector() - result = await connector.getAvailableLanguages() - - if result["success"]: - logger.info(f"✅ Found {len(result['languages'])} available languages") - else: - logger.warning(f"⚠️ Failed to get languages: {result.get('error', 'Unknown error')}") - - return result - - except Exception as e: - logger.error(f"❌ Error getting available languages: {e}") - return { - "success": False, - "error": str(e), - "languages": [] - } - + # Voice Information + # Note: Available languages live in the central voice catalog + # (modules.shared.voiceCatalog); voice picks per language stay live from + # Google so users can see all available speakers per locale. + async def getAvailableVoices(self, languageCode: Optional[str] = None) -> Dict[str, Any]: """ Get available voices from Google Cloud Text-to-Speech. diff --git a/modules/routes/routeVoiceGoogle.py b/modules/routes/routeVoiceGoogle.py index dfa1a15e..8987e73f 100644 --- a/modules/routes/routeVoiceGoogle.py +++ b/modules/routes/routeVoiceGoogle.py @@ -17,6 +17,7 @@ from typing import Optional, Dict, Any, List from modules.auth import getCurrentUser, getRequestContext, RequestContext, limiter from modules.datamodels.datamodelUam import User from modules.interfaces.interfaceVoiceObjects import getVoiceInterface, VoiceObjects +from modules.shared.voiceCatalog import getCatalogPayload logger = logging.getLogger(__name__) router = APIRouter(prefix="/voice-google", tags=["Voice Google"]) @@ -61,32 +62,15 @@ def _getVoiceInterface(currentUser: User) -> VoiceObjects: @router.get("/languages") async def get_available_languages(currentUser: User = Depends(getCurrentUser)): - """Get available languages from Google Cloud Text-to-Speech.""" - try: - logger.info("🌐 Getting available languages from Google Cloud TTS") - - voiceInterface = _getVoiceInterface(currentUser) - result = await voiceInterface.getAvailableLanguages() - - if result["success"]: - return { - "success": True, - "languages": result["languages"] - } - else: - raise HTTPException( - status_code=400, - detail=f"Failed to get languages: {result.get('error', 'Unknown error')}" - ) - - except HTTPException: - raise - except Exception as e: - logger.error(f"❌ Get languages error: {e}") - raise HTTPException( - status_code=500, - detail=f"Failed to get available languages: {str(e)}" - ) + """Return the curated voice/language catalog (single source of truth). + + Each entry: {bcp47, iso, label, flag, defaultVoice}. Same payload as + /api/voice/languages — both endpoints back the same catalog. + """ + return { + "success": True, + "languages": getCatalogPayload(), + } @router.get("/voices") async def get_available_voices( diff --git a/modules/routes/routeVoiceUser.py b/modules/routes/routeVoiceUser.py index a3c3fda7..4edbdf0d 100644 --- a/modules/routes/routeVoiceUser.py +++ b/modules/routes/routeVoiceUser.py @@ -18,6 +18,7 @@ from modules.datamodels.datamodelUam import User, UserVoicePreferences, _normali from modules.interfaces.interfaceDbApp import getRootInterface from modules.interfaces.interfaceVoiceObjects import getVoiceInterface from modules.shared.i18nRegistry import apiRouteContext +from modules.shared.voiceCatalog import getCatalogPayload routeApiMsg = apiRouteContext("routeVoiceUser") logger = logging.getLogger(__name__) @@ -101,11 +102,11 @@ async def getVoiceLanguages( request: Request, currentUser: User = Depends(getCurrentUser), ) -> Dict[str, Any]: - """Return available TTS languages (user-level, no instance context needed).""" - voiceInterface = getVoiceInterface(currentUser) - languagesResult = await voiceInterface.getAvailableLanguages() - languageList = languagesResult.get("languages", []) if isinstance(languagesResult, dict) else languagesResult - return {"languages": languageList} + """Return the curated voice/language catalog (single source of truth). + + Each entry: {bcp47, iso, label, flag, defaultVoice}. + """ + return {"languages": getCatalogPayload()} @router.get("/voices") diff --git a/modules/serviceCenter/services/serviceAgent/coreTools/_mediaTools.py b/modules/serviceCenter/services/serviceAgent/coreTools/_mediaTools.py index da6e616c..83f6e990 100644 --- a/modules/serviceCenter/services/serviceAgent/coreTools/_mediaTools.py +++ b/modules/serviceCenter/services/serviceAgent/coreTools/_mediaTools.py @@ -395,25 +395,17 @@ def _registerMediaTools(registry: ToolRegistry, services): try: from modules.interfaces.interfaceVoiceObjects import getVoiceInterface + from modules.shared.voiceCatalog import isoToBcp47 mandateId = context.get("mandateId", "") voiceInterface = getVoiceInterface(currentUser=None, mandateId=mandateId) - _ISO_TO_BCP47 = { - "de": "de-DE", "en": "en-US", "fr": "fr-FR", "it": "it-IT", - "es": "es-ES", "pt": "pt-BR", "nl": "nl-NL", "pl": "pl-PL", - "ru": "ru-RU", "ja": "ja-JP", "zh": "zh-CN", "ko": "ko-KR", - "ar": "ar-XA", "hi": "hi-IN", "tr": "tr-TR", "sv": "sv-SE", - } - if language == "auto": try: snippet = cleanText[:500] detectResult = await voiceInterface.detectLanguage(snippet) if detectResult and detectResult.get("success"): detected = detectResult.get("language", "de") - language = _ISO_TO_BCP47.get(detected, detected) - if "-" not in language: - language = _ISO_TO_BCP47.get(language, f"{language}-{language.upper()}") + language = isoToBcp47(detected) or "de-DE" logger.info(f"textToSpeech: auto-detected language '{detected}' -> '{language}'") else: language = "de-DE" diff --git a/modules/shared/voiceCatalog.py b/modules/shared/voiceCatalog.py new file mode 100644 index 00000000..2e98902e --- /dev/null +++ b/modules/shared/voiceCatalog.py @@ -0,0 +1,136 @@ +# Copyright (c) 2025 Patrick Motsch +# All rights reserved. +""" +Voice / Language Catalog — Single Source of Truth. + +Every voice-related component (TTS connector, AI tools, REST routes, frontend +language pickers) consumes this catalog. Hard-coded language lists or ad-hoc +ISO→BCP-47 maps elsewhere are forbidden — extend the catalog instead. + +Schema per entry: + bcp47 BCP-47 locale code, e.g. "de-DE", "ru-RU" + iso ISO-639-1 short code, e.g. "de", "ru" + label Native display label ("Deutsch", "Русский") + flag Emoji flag (or empty string for region-neutral codes) + defaultVoice Curated Google TTS voice name; None means "let Google + pick automatically based on bcp47 + ssml_gender". +""" + +from __future__ import annotations + +from dataclasses import asdict, dataclass +from typing import Dict, List, Optional + + +@dataclass(frozen=True) +class VoiceLanguage: + bcp47: str + iso: str + label: str + flag: str + defaultVoice: Optional[str] + + +# Order matters for UI: most common first, then alphabetical groups. +VOICE_LANGUAGES: List[VoiceLanguage] = [ + VoiceLanguage("de-DE", "de", "Deutsch", "🇩🇪", "de-DE-Wavenet-A"), + VoiceLanguage("de-CH", "de", "Deutsch (Schweiz)", "🇨🇭", "de-DE-Wavenet-A"), + VoiceLanguage("de-AT", "de", "Deutsch (Österreich)", "🇦🇹", "de-DE-Wavenet-A"), + VoiceLanguage("en-US", "en", "English (US)", "🇺🇸", "en-US-Wavenet-C"), + VoiceLanguage("en-GB", "en", "English (UK)", "🇬🇧", "en-GB-Wavenet-A"), + VoiceLanguage("en-AU", "en", "English (Australia)", "🇦🇺", "en-AU-Wavenet-A"), + VoiceLanguage("fr-FR", "fr", "Français", "🇫🇷", "fr-FR-Wavenet-A"), + VoiceLanguage("fr-CA", "fr", "Français (Canada)", "🇨🇦", "fr-CA-Wavenet-A"), + VoiceLanguage("it-IT", "it", "Italiano", "🇮🇹", "it-IT-Wavenet-A"), + VoiceLanguage("es-ES", "es", "Español", "🇪🇸", "es-ES-Wavenet-B"), + VoiceLanguage("es-US", "es", "Español (US)", "🇺🇸", "es-US-Wavenet-A"), + VoiceLanguage("pt-BR", "pt", "Português (Brasil)", "🇧🇷", "pt-BR-Wavenet-A"), + VoiceLanguage("pt-PT", "pt", "Português (Portugal)", "🇵🇹", "pt-PT-Wavenet-A"), + VoiceLanguage("nl-NL", "nl", "Nederlands", "🇳🇱", "nl-NL-Wavenet-A"), + VoiceLanguage("pl-PL", "pl", "Polski", "🇵🇱", "pl-PL-Wavenet-A"), + VoiceLanguage("ru-RU", "ru", "Русский", "🇷🇺", "ru-RU-Wavenet-A"), + VoiceLanguage("uk-UA", "uk", "Українська", "🇺🇦", "uk-UA-Wavenet-A"), + VoiceLanguage("cs-CZ", "cs", "Čeština", "🇨🇿", "cs-CZ-Wavenet-A"), + VoiceLanguage("sk-SK", "sk", "Slovenčina", "🇸🇰", "sk-SK-Wavenet-A"), + VoiceLanguage("hu-HU", "hu", "Magyar", "🇭🇺", "hu-HU-Wavenet-A"), + VoiceLanguage("ro-RO", "ro", "Română", "🇷🇴", "ro-RO-Wavenet-A"), + VoiceLanguage("el-GR", "el", "Ελληνικά", "🇬🇷", "el-GR-Wavenet-A"), + VoiceLanguage("sv-SE", "sv", "Svenska", "🇸🇪", "sv-SE-Wavenet-A"), + VoiceLanguage("da-DK", "da", "Dansk", "🇩🇰", "da-DK-Wavenet-A"), + VoiceLanguage("nb-NO", "nb", "Norsk", "🇳🇴", "nb-NO-Wavenet-A"), + VoiceLanguage("fi-FI", "fi", "Suomi", "🇫🇮", "fi-FI-Wavenet-A"), + VoiceLanguage("tr-TR", "tr", "Türkçe", "🇹🇷", "tr-TR-Wavenet-A"), + VoiceLanguage("ar-XA", "ar", "العربية", "", "ar-XA-Wavenet-A"), + VoiceLanguage("hi-IN", "hi", "हिन्दी", "🇮🇳", "hi-IN-Wavenet-A"), + VoiceLanguage("ja-JP", "ja", "日本語", "🇯🇵", "ja-JP-Wavenet-A"), + VoiceLanguage("ko-KR", "ko", "한국어", "🇰🇷", "ko-KR-Wavenet-A"), + VoiceLanguage("zh-CN", "zh", "中文 (简体)", "🇨🇳", "cmn-CN-Wavenet-A"), + VoiceLanguage("vi-VN", "vi", "Tiếng Việt", "🇻🇳", "vi-VN-Wavenet-A"), + VoiceLanguage("th-TH", "th", "ไทย", "🇹🇭", "th-TH-Standard-A"), + VoiceLanguage("id-ID", "id", "Bahasa Indonesia", "🇮🇩", "id-ID-Wavenet-A"), +] + + +# --------------------------------------------------------------------------- +# Lookup indexes (built once at import). +# --------------------------------------------------------------------------- + +_BY_BCP47: Dict[str, VoiceLanguage] = {v.bcp47.lower(): v for v in VOICE_LANGUAGES} +_BY_ISO: Dict[str, VoiceLanguage] = {} +for _v in VOICE_LANGUAGES: + _BY_ISO.setdefault(_v.iso.lower(), _v) + + +def listVoiceLanguages() -> List[VoiceLanguage]: + """Return the canonical, ordered list of supported voice languages.""" + return list(VOICE_LANGUAGES) + + +def getCatalogPayload() -> List[Dict[str, Optional[str]]]: + """Return the catalog as plain dicts — ready for JSON serialization.""" + return [asdict(v) for v in VOICE_LANGUAGES] + + +def getByBcp47(code: Optional[str]) -> Optional[VoiceLanguage]: + if not code: + return None + return _BY_BCP47.get(code.strip().lower()) + + +def getByIso(code: Optional[str]) -> Optional[VoiceLanguage]: + if not code: + return None + return _BY_ISO.get(code.strip().lower()) + + +def getDefaultVoice(bcp47: Optional[str]) -> Optional[str]: + """Return the curated default Google TTS voice for a BCP-47 code, else None. + + None means: caller must omit `name` in VoiceSelectionParams so Google + auto-selects a voice for the language code. + """ + entry = getByBcp47(bcp47) + return entry.defaultVoice if entry else None + + +def isoToBcp47(iso: Optional[str]) -> Optional[str]: + """Map an ISO-639-1 short code to the canonical BCP-47 locale. + + Already-qualified BCP-47 inputs are passed through unchanged (canonicalised + to the catalog form when known). Unknown ISO codes fall back to + ``-`` (e.g. "fa" → "fa-FA") so callers always get a parseable + locale, but unknown codes carry no curated voice. + """ + if not iso: + return None + normalized = iso.strip() + if not normalized: + return None + if "-" in normalized: + canonical = getByBcp47(normalized) + return canonical.bcp47 if canonical else normalized + isoLower = normalized.lower() + entry = _BY_ISO.get(isoLower) + if entry: + return entry.bcp47 + return f"{isoLower}-{isoLower.upper()}" From 7d27ddf6b5e59d66409cc7cda101fc79e99eb6b1 Mon Sep 17 00:00:00 2001 From: ValueOn AG Date: Sun, 19 Apr 2026 01:22:33 +0200 Subject: [PATCH 4/5] fixed integration graph editor ai --- modules/datamodels/datamodelChat.py | 15 + .../routeFeatureGraphicalEditor.py | 218 ++++++++- modules/interfaces/interfaceDbChat.py | 74 ++- modules/interfaces/interfaceDbManagement.py | 18 + .../services/serviceAgent/datamodelAgent.py | 8 + .../services/serviceAgent/mainServiceAgent.py | 20 +- .../services/serviceAgent/workflowTools.py | 446 ++++++++++-------- 7 files changed, 587 insertions(+), 212 deletions(-) diff --git a/modules/datamodels/datamodelChat.py b/modules/datamodels/datamodelChat.py index 6160e7c8..96eb01ef 100644 --- a/modules/datamodels/datamodelChat.py +++ b/modules/datamodels/datamodelChat.py @@ -128,6 +128,21 @@ class ChatWorkflow(PowerOnModel): "fk_target": {"db": "poweron_app", "table": "FeatureInstance"}, }, ) + linkedWorkflowId: Optional[str] = Field( + None, + description=( + "Optional foreign key linking this chat to an entity outside the " + "ChatWorkflow table (e.g. an Automation2Workflow in the GraphicalEditor " + "AI editor chat). NULL for the default workspace chats. Combined with " + "featureInstanceId this gives a 1:1 relation entity ↔ chat per feature." + ), + json_schema_extra={ + "label": "Verknüpfter Workflow", + "frontend_type": "text", + "frontend_readonly": True, + "frontend_required": False, + }, + ) status: str = Field(default="running", description="Current status of the workflow", json_schema_extra={"frontend_type": "select", "frontend_readonly": False, "frontend_required": False, "frontend_options": [ {"value": "running", "label": "Running"}, {"value": "completed", "label": "Completed"}, diff --git a/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py b/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py index f02364a0..7b30ec16 100644 --- a/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py +++ b/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py @@ -470,16 +470,63 @@ def share_template( # ------------------------------------------------------------------------- +def _editorChatQueueId(workflowId: str) -> str: + """Deterministic SSE queue id for the editor chat (one active stream per workflow). + + Mirrors the workspace pattern (``workspace-{workflowId}``) so stop/cancel can + target the running task by workflowId without needing per-request handles. + """ + return f"ge-chat-{workflowId}" + + +def _getEditorChatInterface(context: RequestContext, mandateId: str, instanceId: str): + """Build the ChatObjects interface used to persist editor-chat messages.""" + from modules.interfaces import interfaceDbChat + return interfaceDbChat.getInterface( + context.user, + mandateId=mandateId, + featureInstanceId=instanceId, + ) + + +def _editorConversationHistoryFromPersisted(chatInterface, chatWorkflowId: str) -> List[Dict[str, Any]]: + """Load persisted ChatMessages for the editor chat and shape them as the + agent expects (``[{role, message}]``). Skips empty / system messages. + """ + try: + msgs = chatInterface.getMessages(chatWorkflowId) or [] + except Exception as e: + logger.warning("Editor chat: could not load persisted history for %s: %s", chatWorkflowId, e) + return [] + history: List[Dict[str, Any]] = [] + for m in msgs: + role = (getattr(m, "role", None) or (m.get("role") if isinstance(m, dict) else None) or "").strip() + text = (getattr(m, "message", None) or (m.get("message") if isinstance(m, dict) else None) or "").strip() + if not role or not text: + continue + if role not in ("user", "assistant", "system"): + continue + history.append({"role": role, "message": text}) + return history + + @router.post("/{instanceId}/{workflowId}/chat/stream") @limiter.limit("30/minute") async def post_editor_chat( request: Request, instanceId: str = Path(..., description="Feature instance ID"), workflowId: str = Path(..., description="Workflow ID"), - body: dict = Body(..., description="{ message, conversationHistory?, userLanguage? }"), + body: dict = Body(..., description="{ message, userLanguage? }"), context: RequestContext = Depends(getRequestContext), ): - """AI chat endpoint for the editor with SSE streaming. Uses workflow tools to mutate the graph.""" + """AI chat endpoint for the editor with SSE streaming. Uses workflow tools to mutate the graph. + + Persistence: the chat is stored in the standard ``ChatWorkflow`` table linked + to this Automation2Workflow via ``ChatWorkflow.linkedWorkflowId``. The user + message is persisted before the agent starts; the assistant message after. + Conversation history is loaded server-side from this linked ChatWorkflow — + the client does not need to maintain it. + """ mandateId = _validateInstanceAccess(instanceId, context) message = body.get("message", "") if not message: @@ -491,14 +538,35 @@ async def post_editor_chat( raise HTTPException(status_code=404, detail=routeApiMsg("Workflow not found")) userLanguage = body.get("userLanguage", "de") - conversationHistory = body.get("conversationHistory") or [] fileIds = body.get("fileIds") or [] dataSourceIds = body.get("dataSourceIds") or [] featureDataSourceIds = body.get("featureDataSourceIds") or [] + chatInterface = _getEditorChatInterface(context, mandateId, instanceId) + wfLabel = wf.get("label") if isinstance(wf, dict) else getattr(wf, "label", None) + chatWorkflow = chatInterface.getOrCreateLinkedWorkflow( + featureInstanceId=instanceId, + linkedWorkflowId=workflowId, + name=wfLabel or f"Editor Chat ({workflowId})", + ) + chatWorkflowId = chatWorkflow.id if hasattr(chatWorkflow, "id") else chatWorkflow.get("id") + + conversationHistory = _editorConversationHistoryFromPersisted(chatInterface, chatWorkflowId) + + try: + chatInterface.createMessage({ + "workflowId": chatWorkflowId, + "role": "user", + "message": message, + "status": "first" if not conversationHistory else "step", + }) + except Exception as e: + logger.error("Editor chat: failed to persist user message: %s", e) + from modules.serviceCenter.core.serviceStreaming import get_event_manager sseEventManager = get_event_manager() - queueId = f"ge-chat-{workflowId}-{id(request)}" + queueId = _editorChatQueueId(workflowId) + await sseEventManager.cancel_agent(queueId) sseEventManager.create_queue(queueId) agentTask = asyncio.ensure_future( @@ -515,6 +583,8 @@ async def post_editor_chat( fileIds=fileIds, dataSourceIds=dataSourceIds, featureDataSourceIds=featureDataSourceIds, + chatInterface=chatInterface, + chatWorkflowId=chatWorkflowId, ) ) sseEventManager.register_agent_task(queueId, agentTask) @@ -549,6 +619,80 @@ async def post_editor_chat( ) +@router.get("/{instanceId}/{workflowId}/chat/messages") +@limiter.limit("120/minute") +def get_editor_chat_messages( + request: Request, + instanceId: str = Path(..., description="Feature instance ID"), + workflowId: str = Path(..., description="Workflow ID (Automation2Workflow)"), + context: RequestContext = Depends(getRequestContext), +): + """Return persisted editor-chat messages for an Automation2Workflow. + + The chat is stored in ``ChatWorkflow`` with ``linkedWorkflowId == workflowId``; + if no chat has been started yet for this workflow we return an empty list (we + do NOT eagerly create one — the row is created on the first POST /chat/stream). + """ + mandateId = _validateInstanceAccess(instanceId, context) + chatInterface = _getEditorChatInterface(context, mandateId, instanceId) + chatWorkflow = chatInterface.getWorkflowByLink( + featureInstanceId=instanceId, + linkedWorkflowId=workflowId, + ) + if not chatWorkflow: + return JSONResponse({ + "chatWorkflowId": None, + "messages": [], + }) + + chatWorkflowId = chatWorkflow.id if hasattr(chatWorkflow, "id") else chatWorkflow.get("id") + rawMessages = chatInterface.getMessages(chatWorkflowId) or [] + + items: List[Dict[str, Any]] = [] + for m in rawMessages: + getter = (lambda key, default=None: getattr(m, key, default)) if not isinstance(m, dict) else (lambda key, default=None: m.get(key, default)) + role = (getter("role") or "").strip() + content = (getter("message") or "").strip() + if not role or not content: + continue + items.append({ + "id": getter("id"), + "role": role, + "content": content, + "timestamp": getter("publishedAt") or 0, + "sequenceNr": getter("sequenceNr") or 0, + }) + + items.sort(key=lambda x: (float(x.get("timestamp") or 0), int(x.get("sequenceNr") or 0))) + + return JSONResponse({ + "chatWorkflowId": chatWorkflowId, + "messages": items, + }) + + +@router.post("/{instanceId}/{workflowId}/chat/stop") +@limiter.limit("120/minute") +async def post_editor_chat_stop( + request: Request, + instanceId: str = Path(..., description="Feature instance ID"), + workflowId: str = Path(..., description="Workflow ID"), + context: RequestContext = Depends(getRequestContext), +): + """Stop a running editor-chat agent for the given workflow.""" + _validateInstanceAccess(instanceId, context) + from modules.serviceCenter.core.serviceStreaming import get_event_manager + sseEventManager = get_event_manager() + queueId = _editorChatQueueId(workflowId) + cancelled = await sseEventManager.cancel_agent(queueId) + await sseEventManager.emit_event(queueId, "stopped", { + "type": "stopped", + "workflowId": workflowId, + }) + logger.info("Editor chat stop requested for workflow %s, cancelled=%s", workflowId, cancelled) + return JSONResponse({"status": "stopped", "workflowId": workflowId, "cancelled": cancelled}) + + async def _runEditorAgent( workflowId: str, queueId: str, @@ -562,12 +706,41 @@ async def _runEditorAgent( fileIds: List[str] = None, dataSourceIds: List[str] = None, featureDataSourceIds: List[str] = None, + chatInterface=None, + chatWorkflowId: Optional[str] = None, ): - """Run the serviceAgent loop with workflow toolbox and forward events to the SSE queue.""" + """Run the serviceAgent loop with workflow toolbox and forward events to the SSE queue. + + Persists the assistant response to ``ChatMessage`` (linked via ``chatWorkflowId``) + on FINAL/ERROR. On cancellation any partial accumulated text is still saved so + the editor chat history reflects what the user actually saw on screen. + """ + assistantPersisted = False + + def _persistAssistant(text: str) -> None: + nonlocal assistantPersisted + if assistantPersisted or not chatInterface or not chatWorkflowId: + return + cleaned = (text or "").strip() + if not cleaned: + return + try: + chatInterface.createMessage({ + "workflowId": chatWorkflowId, + "role": "assistant", + "message": cleaned, + "status": "last", + }) + assistantPersisted = True + except Exception as msgErr: + logger.error("Editor chat: failed to persist assistant message: %s", msgErr) + try: from modules.serviceCenter import getService from modules.serviceCenter.context import ServiceCenterContext - from modules.serviceCenter.services.serviceAgent.datamodelAgent import AgentEventTypeEnum + from modules.serviceCenter.services.serviceAgent.datamodelAgent import ( + AgentEventTypeEnum, AgentConfig, + ) ctx = ServiceCenterContext( user=user, @@ -579,11 +752,22 @@ async def _runEditorAgent( agentService = getService("agent", ctx) systemPrompt = ( - "You are a workflow editor assistant. The user describes changes to a workflow graph. " - "Use the available workflow tools (readWorkflowGraph, addNode, removeNode, connectNodes, " - "setNodeParameter, listAvailableNodeTypes, validateGraph) to modify the graph. " - "Always read the current graph first before making changes. " - "Respond concisely and confirm what you changed." + "You are a workflow EDITOR assistant for the GraphicalEditor. " + "Your ONLY job is to BUILD or MODIFY the workflow graph (nodes + connections) " + "for the user — you must NEVER execute the workflow or any of its actions. " + "Even when the user says 'create a workflow that sends an email', you build the " + "graph (e.g. add an email node, connect it) — you do NOT actually send an email. " + "Use these workflow tools to mutate the graph: " + "readWorkflowGraph, listAvailableNodeTypes, addNode, removeNode, connectNodes, " + "setNodeParameter, validateGraph. " + "Always read the current graph and list available node types first, then plan the " + "smallest set of mutations, then apply them. Respond concisely in the user's " + "language and confirm what you changed in the graph." + ) + + editorConfig = AgentConfig( + toolSet="core", + excludeActionTools=True, ) enrichedPrompt = prompt @@ -605,6 +789,7 @@ async def _runEditorAgent( async for event in agentService.runAgent( prompt=enrichedPrompt, fileIds=fileIds or [], + config=editorConfig, workflowId=workflowId, userLanguage=userLanguage, conversationHistory=conversationHistory or [], @@ -631,8 +816,13 @@ async def _runEditorAgent( await sseEventManager.emit_event(queueId, sseEvent["type"], sseEvent) if event.type in (AgentEventTypeEnum.FINAL, AgentEventTypeEnum.ERROR): + _persistAssistant(event.content or accumulatedText) break + # Fallback: any streamed content not yet stored (cancellation path, no FINAL). + if not assistantPersisted and accumulatedText.strip(): + _persistAssistant(accumulatedText) + await sseEventManager.emit_event(queueId, "complete", { "type": "complete", "workflowId": workflowId, @@ -640,6 +830,12 @@ async def _runEditorAgent( except asyncio.CancelledError: logger.info("Editor chat agent task cancelled for workflow %s", workflowId) + # Save whatever the user already saw before cancelling so the next reload + # shows the same partial answer (matches workspace behaviour). + try: + _persistAssistant(accumulatedText if "accumulatedText" in locals() else "") + except Exception: + pass await sseEventManager.emit_event(queueId, "stopped", { "type": "stopped", "workflowId": workflowId, diff --git a/modules/interfaces/interfaceDbChat.py b/modules/interfaces/interfaceDbChat.py index c8bbadf0..adeac55b 100644 --- a/modules/interfaces/interfaceDbChat.py +++ b/modules/interfaces/interfaceDbChat.py @@ -655,17 +655,27 @@ class ChatObjects: totalPages=totalPages ) - def getLastMessageTimestamp(self, workflowId: str) -> Optional[str]: - """Return the latest publishedAt/sysCreatedAt from ChatMessage for a workflow.""" + def getLastMessageTimestamp(self, workflowId: str) -> Optional[float]: + """ + Return the latest publishedAt/sysCreatedAt from ChatMessage for a workflow + as UTC seconds (float) — matches the timestamp format used across the + rest of the chat data model (lastActivity, startedAt, publishedAt). + """ messages = self._getRecordset(ChatMessage, recordFilter={"workflowId": workflowId}) if not messages: return None - latest = None + latest: Optional[float] = None for msg in messages: - ts = msg.get("publishedAt") or msg.get("sysCreatedAt") - if ts and (latest is None or str(ts) > str(latest)): + raw = msg.get("publishedAt") or msg.get("sysCreatedAt") + if raw is None: + continue + try: + ts = float(raw) + except (TypeError, ValueError): + continue + if latest is None or ts > latest: latest = ts - return str(latest) if latest else None + return latest def searchWorkflowsByContent(self, query: str, limit: int = 50) -> List[str]: """Return workflow IDs whose messages contain the query string (case-insensitive).""" @@ -712,6 +722,8 @@ class ChatObjects: return ChatWorkflow( id=workflow["id"], + featureInstanceId=workflow.get("featureInstanceId"), + linkedWorkflowId=workflow.get("linkedWorkflowId"), status=workflow.get("status", "running"), name=workflow.get("name"), currentRound=_toInt(workflow.get("currentRound")), @@ -728,6 +740,54 @@ class ChatObjects: logger.error(f"getWorkflow: data validation failed for {workflowId}: {e}") return None + def getWorkflowByLink( + self, + featureInstanceId: str, + linkedWorkflowId: str, + ) -> Optional[ChatWorkflow]: + """Return the ChatWorkflow linked to (featureInstanceId, linkedWorkflowId), if any. + + Used by editor-style features (e.g. GraphicalEditor AI editor chat) to + find the persisted chat for a specific external entity (Automation2Workflow). + Falls under the same RBAC as ``getWorkflow``. + """ + if not featureInstanceId or not linkedWorkflowId: + return None + rows = self._getRecordset( + ChatWorkflow, + recordFilter={ + "featureInstanceId": featureInstanceId, + "linkedWorkflowId": linkedWorkflowId, + }, + ) or [] + if not rows: + return None + # Return the most recently active one if multiple ever exist (defensive). + rows.sort(key=lambda r: float(r.get("lastActivity") or r.get("startedAt") or 0), reverse=True) + return self.getWorkflow(rows[0]["id"]) + + def getOrCreateLinkedWorkflow( + self, + featureInstanceId: str, + linkedWorkflowId: str, + name: Optional[str] = None, + ) -> ChatWorkflow: + """Find or create the ChatWorkflow linked to a specific external entity. + + Editor-style features call this once at the start of a chat exchange to + guarantee a 1:1 mapping between (featureInstanceId, linkedWorkflowId) + and a persisted ChatWorkflow row. + """ + existing = self.getWorkflowByLink(featureInstanceId, linkedWorkflowId) + if existing: + return existing + return self.createWorkflow({ + "featureInstanceId": featureInstanceId, + "linkedWorkflowId": linkedWorkflowId, + "status": "active", + "name": name or "", + }) + def createWorkflow(self, workflowData: Dict[str, Any]) -> ChatWorkflow: """Creates a new workflow if user has permission.""" if not self.checkRbacPermission(ChatWorkflow, "create"): @@ -775,6 +835,8 @@ class ChatObjects: # Convert to ChatWorkflow model (empty related data for new workflow) return ChatWorkflow( id=created["id"], + featureInstanceId=created.get("featureInstanceId"), + linkedWorkflowId=created.get("linkedWorkflowId"), status=created.get("status", "running"), name=created.get("name"), currentRound=created.get("currentRound", 0) or 0, diff --git a/modules/interfaces/interfaceDbManagement.py b/modules/interfaces/interfaceDbManagement.py index 1a27a8db..cca98ffa 100644 --- a/modules/interfaces/interfaceDbManagement.py +++ b/modules/interfaces/interfaceDbManagement.py @@ -1404,6 +1404,24 @@ class ComponentObjects: self._validateFolderName(newName, folder.get("parentId"), excludeFolderId=folderId) return self.db.recordModify(FileFolder, folderId, {"name": newName}) + def updateFolder(self, folderId: str, updateData: Dict[str, Any]) -> bool: + """ + Update folder metadata (e.g. ``scope``, ``neutralize``). Owner-only, + same access model as renameFolder/moveFolder. Use ``renameFolder`` for + ``name`` changes (uniqueness validation) and ``moveFolder`` for + ``parentId`` changes (cycle/uniqueness validation). + """ + if not updateData: + return True + folder = self.getFolder(folderId) + if not folder: + raise FileNotFoundError(f"Folder {folderId} not found") + forbiddenKeys = {"id", "sysCreatedBy", "sysCreatedAt", "sysUpdatedAt"} + cleaned: Dict[str, Any] = {k: v for k, v in updateData.items() if k not in forbiddenKeys} + if "name" in cleaned: + self._validateFolderName(cleaned["name"], folder.get("parentId"), excludeFolderId=folderId) + return self.db.recordModify(FileFolder, folderId, cleaned) + def moveFolder(self, folderId: str, targetParentId: Optional[str] = None) -> bool: """Move a folder to a new parent, with circular reference and unique name checks.""" folder = self.getFolder(folderId) diff --git a/modules/serviceCenter/services/serviceAgent/datamodelAgent.py b/modules/serviceCenter/services/serviceAgent/datamodelAgent.py index 053569b0..16cb1964 100644 --- a/modules/serviceCenter/services/serviceAgent/datamodelAgent.py +++ b/modules/serviceCenter/services/serviceAgent/datamodelAgent.py @@ -93,6 +93,14 @@ class AgentConfig(BaseModel): availableToolboxes: List[str] = Field(default_factory=list) temperature: Optional[float] = Field(default=None, ge=0.0, le=2.0) operationType: Optional[OperationTypeEnum] = Field(default=None, description="Override the default AGENT operationType for model selection") + excludeActionTools: bool = Field( + default=False, + description=( + "If True, do NOT register workflow-action methods as agent tools. " + "Used by editor-style agents (e.g. GraphicalEditor) that should only " + "manipulate the workflow graph, not execute its actions." + ), + ) class AgentState(BaseModel): diff --git a/modules/serviceCenter/services/serviceAgent/mainServiceAgent.py b/modules/serviceCenter/services/serviceAgent/mainServiceAgent.py index 9094e952..fb54199e 100644 --- a/modules/serviceCenter/services/serviceAgent/mainServiceAgent.py +++ b/modules/serviceCenter/services/serviceAgent/mainServiceAgent.py @@ -330,16 +330,20 @@ class AgentService: except Exception as e: logger.warning("discoverMethods failed before action tools: %s", e) - try: - from modules.workflows.processing.core.actionExecutor import ActionExecutor - actionExecutor = ActionExecutor(self.services) - adapter = ActionToolAdapter(actionExecutor) - adapter.registerAll(registry) - except Exception as e: - logger.warning(f"Could not register action tools: {e}") + if not getattr(config, "excludeActionTools", False): + try: + from modules.workflows.processing.core.actionExecutor import ActionExecutor + actionExecutor = ActionExecutor(self.services) + adapter = ActionToolAdapter(actionExecutor) + adapter.registerAll(registry) + except Exception as e: + logger.warning(f"Could not register action tools: {e}") + else: + logger.info("excludeActionTools=True: skipping ActionToolAdapter registration (editor-mode agent)") self._activateToolboxes(registry, config) - self._registerRequestToolbox(registry) + if not getattr(config, "excludeActionTools", False): + self._registerRequestToolbox(registry) return registry diff --git a/modules/serviceCenter/services/serviceAgent/workflowTools.py b/modules/serviceCenter/services/serviceAgent/workflowTools.py index a63abb65..3e1bb9ad 100644 --- a/modules/serviceCenter/services/serviceAgent/workflowTools.py +++ b/modules/serviceCenter/services/serviceAgent/workflowTools.py @@ -4,11 +4,21 @@ Workflow Toolbox - AI-assisted graph manipulation tools for the GraphicalEditor. Tools: readWorkflowGraph, addNode, removeNode, connectNodes, setNodeParameter, listAvailableNodeTypes, validateGraph, listWorkflowHistory, readWorkflowMessages. + +Conventions enforced here (matches coreTools / actionToolAdapter): + - Every ``ToolResult(...)`` provides ``toolCallId`` and ``toolName`` (pydantic + requires both); ``ToolRegistry.dispatch`` overwrites ``toolCallId`` later + but the model still validates at construction. + - ``ToolResult.data`` is a ``str``; structured payloads are JSON-encoded. + - ``workflowId`` and ``instanceId`` are auto-injected from the agent + ``context`` dict (``workflowId``, ``featureInstanceId``) when the model + omits them — the editor agent always runs in exactly one workflow. """ +import json import logging import uuid -from typing import Dict, Any, List, Optional +from typing import Dict, Any, List, Tuple from modules.serviceCenter.services.serviceAgent.datamodelAgent import ToolResult @@ -17,65 +27,124 @@ logger = logging.getLogger(__name__) TOOLBOX_ID = "workflow" +def _toData(payload: Any) -> str: + """Encode a structured payload into ToolResult.data (which is a string).""" + if isinstance(payload, str): + return payload + try: + return json.dumps(payload, default=str, ensure_ascii=False) + except Exception: + return str(payload) + + +def _err(toolName: str, message: str) -> ToolResult: + return ToolResult(toolCallId="", toolName=toolName, success=False, error=message) + + +def _ok(toolName: str, payload: Any) -> ToolResult: + return ToolResult(toolCallId="", toolName=toolName, success=True, data=_toData(payload)) + + +def _resolveIds(params: Dict[str, Any], context: Any) -> Tuple[str, str]: + """Return (workflowId, instanceId), auto-injecting from context when missing. + + The editor agent context (``agentLoop._executeToolCalls``) is a dict with + ``workflowId`` and ``featureInstanceId`` — use them as defaults so the + model doesn't have to re-state the ids on every tool call. + """ + ctx: Dict[str, Any] = context if isinstance(context, dict) else {} + workflowId = params.get("workflowId") or ctx.get("workflowId") or "" + instanceId = ( + params.get("instanceId") + or ctx.get("featureInstanceId") + or ctx.get("instanceId") + or "" + ) + return workflowId, instanceId + + +def _resolveUser(context: Any): + """Return the User object for the current agent context (lazy DB fetch).""" + if not isinstance(context, dict): + return getattr(context, "user", None) + user = context.get("user") + if user is not None: + return user + userId = context.get("userId") + if not userId: + return None + try: + from modules.interfaces.interfaceDbApp import getRootInterface + return getRootInterface().getUser(str(userId)) + except Exception as e: + logger.warning("workflowTools: could not resolve user %s: %s", userId, e) + return None + + +def _resolveMandateId(context: Any) -> str: + if not isinstance(context, dict): + return getattr(context, "mandateId", "") or "" + return context.get("mandateId") or "" + + +def _getInterface(context: Any, instanceId: str): + from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface + return getGraphicalEditorInterface(_resolveUser(context), _resolveMandateId(context), instanceId) + + async def _readWorkflowGraph(params: Dict[str, Any], context: Any) -> ToolResult: """Read the current workflow graph (nodes and connections).""" + name = "readWorkflowGraph" try: - workflowId = params.get("workflowId") - instanceId = params.get("instanceId") + workflowId, instanceId = _resolveIds(params, context) if not workflowId or not instanceId: - return ToolResult(success=False, error="workflowId and instanceId required") + return _err(name, "workflowId and instanceId required (and not present in agent context)") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + iface = _getInterface(context, instanceId) wf = iface.getWorkflow(workflowId) if not wf: - return ToolResult(success=False, error=f"Workflow {workflowId} not found") + return _err(name, f"Workflow {workflowId} not found") - graph = wf.get("graph", {}) - nodes = graph.get("nodes", []) - connections = graph.get("connections", []) - return ToolResult( - success=True, - data={ - "workflowId": workflowId, - "label": wf.get("label", ""), - "nodeCount": len(nodes), - "connectionCount": len(connections), - "nodes": [{"id": n.get("id"), "type": n.get("type"), "title": n.get("title", "")} for n in nodes], - "connections": connections, - }, - ) + graph = wf.get("graph", {}) or {} + nodes = graph.get("nodes", []) or [] + connections = graph.get("connections", []) or [] + return _ok(name, { + "workflowId": workflowId, + "label": wf.get("label", ""), + "nodeCount": len(nodes), + "connectionCount": len(connections), + "nodes": [ + {"id": n.get("id"), "type": n.get("type"), "title": n.get("title", "")} + for n in nodes + ], + "connections": connections, + }) except Exception as e: logger.exception("readWorkflowGraph failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _addNode(params: Dict[str, Any], context: Any) -> ToolResult: """Add a node to the workflow graph.""" + name = "addNode" try: - workflowId = params.get("workflowId") - instanceId = params.get("instanceId") + workflowId, instanceId = _resolveIds(params, context) nodeType = params.get("nodeType") if not workflowId or not instanceId or not nodeType: - return ToolResult(success=False, error="workflowId, instanceId, and nodeType required") + return _err(name, "workflowId, instanceId, and nodeType required") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + iface = _getInterface(context, instanceId) wf = iface.getWorkflow(workflowId) if not wf: - return ToolResult(success=False, error=f"Workflow {workflowId} not found") + return _err(name, f"Workflow {workflowId} not found") - graph = dict(wf.get("graph", {})) - nodes = list(graph.get("nodes", [])) + graph = dict(wf.get("graph", {}) or {}) + nodes = list(graph.get("nodes", []) or []) nodeId = params.get("nodeId") or str(uuid.uuid4())[:8] title = params.get("title", "") - nodeParams = params.get("parameters", {}) - position = params.get("position", {"x": len(nodes) * 200, "y": 100}) + nodeParams = params.get("parameters", {}) or {} + position = params.get("position") or {"x": len(nodes) * 200, "y": 100} newNode = { "id": nodeId, @@ -88,68 +157,63 @@ async def _addNode(params: Dict[str, Any], context: Any) -> ToolResult: graph["nodes"] = nodes iface.updateWorkflow(workflowId, {"graph": graph}) - return ToolResult( - success=True, - data={"nodeId": nodeId, "nodeType": nodeType, "message": f"Node '{title or nodeType}' added"}, - ) + return _ok(name, { + "nodeId": nodeId, + "nodeType": nodeType, + "message": f"Node '{title or nodeType}' added", + }) except Exception as e: logger.exception("addNode failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _removeNode(params: Dict[str, Any], context: Any) -> ToolResult: """Remove a node and its connections from the workflow graph.""" + name = "removeNode" try: - workflowId = params.get("workflowId") - instanceId = params.get("instanceId") + workflowId, instanceId = _resolveIds(params, context) nodeId = params.get("nodeId") if not workflowId or not instanceId or not nodeId: - return ToolResult(success=False, error="workflowId, instanceId, and nodeId required") + return _err(name, "workflowId, instanceId, and nodeId required") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + iface = _getInterface(context, instanceId) wf = iface.getWorkflow(workflowId) if not wf: - return ToolResult(success=False, error=f"Workflow {workflowId} not found") + return _err(name, f"Workflow {workflowId} not found") - graph = dict(wf.get("graph", {})) - nodes = [n for n in graph.get("nodes", []) if n.get("id") != nodeId] + graph = dict(wf.get("graph", {}) or {}) + nodes = [n for n in (graph.get("nodes", []) or []) if n.get("id") != nodeId] connections = [ - c for c in graph.get("connections", []) + c for c in (graph.get("connections", []) or []) if c.get("source") != nodeId and c.get("target") != nodeId ] graph["nodes"] = nodes graph["connections"] = connections iface.updateWorkflow(workflowId, {"graph": graph}) - return ToolResult(success=True, data={"nodeId": nodeId, "message": f"Node {nodeId} removed"}) + return _ok(name, {"nodeId": nodeId, "message": f"Node {nodeId} removed"}) except Exception as e: logger.exception("removeNode failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _connectNodes(params: Dict[str, Any], context: Any) -> ToolResult: """Connect two nodes in the workflow graph.""" + name = "connectNodes" try: - workflowId = params.get("workflowId") - instanceId = params.get("instanceId") + workflowId, instanceId = _resolveIds(params, context) sourceId = params.get("sourceId") targetId = params.get("targetId") if not workflowId or not instanceId or not sourceId or not targetId: - return ToolResult(success=False, error="workflowId, instanceId, sourceId, and targetId required") + return _err(name, "workflowId, instanceId, sourceId, and targetId required") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + iface = _getInterface(context, instanceId) wf = iface.getWorkflow(workflowId) if not wf: - return ToolResult(success=False, error=f"Workflow {workflowId} not found") + return _err(name, f"Workflow {workflowId} not found") - graph = dict(wf.get("graph", {})) - connections = list(graph.get("connections", [])) + graph = dict(wf.get("graph", {}) or {}) + connections = list(graph.get("connections", []) or []) newConn = { "source": sourceId, "target": targetId, @@ -160,93 +224,113 @@ async def _connectNodes(params: Dict[str, Any], context: Any) -> ToolResult: graph["connections"] = connections iface.updateWorkflow(workflowId, {"graph": graph}) - return ToolResult(success=True, data={"connection": newConn, "message": f"Connected {sourceId} -> {targetId}"}) + return _ok(name, {"connection": newConn, "message": f"Connected {sourceId} -> {targetId}"}) except Exception as e: logger.exception("connectNodes failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _setNodeParameter(params: Dict[str, Any], context: Any) -> ToolResult: """Set a parameter on a node.""" + name = "setNodeParameter" try: - workflowId = params.get("workflowId") - instanceId = params.get("instanceId") + workflowId, instanceId = _resolveIds(params, context) nodeId = params.get("nodeId") paramName = params.get("parameterName") paramValue = params.get("parameterValue") if not workflowId or not instanceId or not nodeId or not paramName: - return ToolResult(success=False, error="workflowId, instanceId, nodeId, and parameterName required") + return _err(name, "workflowId, instanceId, nodeId, and parameterName required") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + iface = _getInterface(context, instanceId) wf = iface.getWorkflow(workflowId) if not wf: - return ToolResult(success=False, error=f"Workflow {workflowId} not found") + return _err(name, f"Workflow {workflowId} not found") - graph = dict(wf.get("graph", {})) - nodes = list(graph.get("nodes", [])) + graph = dict(wf.get("graph", {}) or {}) + nodes = list(graph.get("nodes", []) or []) found = False for n in nodes: if n.get("id") == nodeId: - nodeParams = dict(n.get("parameters", {})) + nodeParams = dict(n.get("parameters", {}) or {}) nodeParams[paramName] = paramValue n["parameters"] = nodeParams found = True break if not found: - return ToolResult(success=False, error=f"Node {nodeId} not found in graph") + return _err(name, f"Node {nodeId} not found in graph") graph["nodes"] = nodes iface.updateWorkflow(workflowId, {"graph": graph}) - return ToolResult(success=True, data={"nodeId": nodeId, "parameter": paramName, "message": f"Parameter '{paramName}' set"}) + return _ok(name, { + "nodeId": nodeId, + "parameter": paramName, + "message": f"Parameter '{paramName}' set", + }) except Exception as e: logger.exception("setNodeParameter failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) + + +def _coerceLabel(rawLabel: Any, fallback: str) -> str: + """Normalize a node label which may be a string, dict {locale: str}, or other.""" + if isinstance(rawLabel, str): + return rawLabel + if isinstance(rawLabel, dict): + for key in ("en", "de", "fr"): + value = rawLabel.get(key) + if isinstance(value, str) and value: + return value + for value in rawLabel.values(): + if isinstance(value, str) and value: + return value + return fallback async def _listAvailableNodeTypes(params: Dict[str, Any], context: Any) -> ToolResult: """List all available node types for the flow builder.""" + name = "listAvailableNodeTypes" try: from modules.features.graphicalEditor.nodeDefinitions import STATIC_NODE_TYPES - nodeTypes = [ - {"id": n.get("id"), "category": n.get("category"), "label": n.get("label", {}).get("en", n.get("id"))} - for n in STATIC_NODE_TYPES - ] - return ToolResult(success=True, data={"nodeTypes": nodeTypes, "count": len(nodeTypes)}) + nodeTypes = [] + for n in STATIC_NODE_TYPES: + if not isinstance(n, dict): + continue + nodeId = n.get("id") or "" + nodeTypes.append({ + "id": nodeId, + "category": n.get("category"), + "label": _coerceLabel(n.get("label"), nodeId), + }) + return _ok(name, {"nodeTypes": nodeTypes, "count": len(nodeTypes)}) except Exception as e: logger.exception("listAvailableNodeTypes failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _validateGraph(params: Dict[str, Any], context: Any) -> ToolResult: """Validate a workflow graph for common issues.""" + name = "validateGraph" try: - workflowId = params.get("workflowId") - instanceId = params.get("instanceId") + workflowId, instanceId = _resolveIds(params, context) if not workflowId or not instanceId: - return ToolResult(success=False, error="workflowId and instanceId required") + return _err(name, "workflowId and instanceId required") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + iface = _getInterface(context, instanceId) wf = iface.getWorkflow(workflowId) if not wf: - return ToolResult(success=False, error=f"Workflow {workflowId} not found") + return _err(name, f"Workflow {workflowId} not found") - graph = wf.get("graph", {}) - nodes = graph.get("nodes", []) - connections = graph.get("connections", []) + graph = wf.get("graph", {}) or {} + nodes = graph.get("nodes", []) or [] + connections = graph.get("connections", []) or [] issues: List[str] = [] nodeIds = {n.get("id") for n in nodes} if not nodes: issues.append("Graph has no nodes") - hasTrigger = any(n.get("type", "").startswith("trigger.") for n in nodes) + hasTrigger = any((n.get("type") or "").startswith("trigger.") for n in nodes) if not hasTrigger: issues.append("No trigger node found") @@ -260,64 +344,59 @@ async def _validateGraph(params: Dict[str, Any], context: Any) -> ToolResult: for c in connections: connectedNodes.add(c.get("source")) connectedNodes.add(c.get("target")) - orphans = [n.get("id") for n in nodes if n.get("id") not in connectedNodes and not n.get("type", "").startswith("trigger.")] + orphans = [ + n.get("id") for n in nodes + if n.get("id") not in connectedNodes and not (n.get("type") or "").startswith("trigger.") + ] if orphans: issues.append(f"Orphan nodes (not connected): {', '.join(orphans)}") - return ToolResult( - success=True, - data={ - "valid": len(issues) == 0, - "issues": issues, - "nodeCount": len(nodes), - "connectionCount": len(connections), - }, - ) + return _ok(name, { + "valid": len(issues) == 0, + "issues": issues, + "nodeCount": len(nodes), + "connectionCount": len(connections), + }) except Exception as e: logger.exception("validateGraph failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _listWorkflowHistory(params: Dict[str, Any], context: Any) -> ToolResult: """List versions (history) for a workflow.""" + name = "listWorkflowHistory" try: - workflowId = params.get("workflowId", "") - instanceId = params.get("instanceId", "") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) - versions = iface.getVersions(workflowId) - return ToolResult( - success=True, - data={ - "workflowId": workflowId, - "versions": [ - { - "id": v.get("id"), - "versionNumber": v.get("versionNumber"), - "status": v.get("status"), - "publishedAt": v.get("publishedAt"), - "publishedBy": v.get("publishedBy"), - } - for v in versions - ], - }, - ) + workflowId, instanceId = _resolveIds(params, context) + if not workflowId or not instanceId: + return _err(name, "workflowId and instanceId required") + iface = _getInterface(context, instanceId) + versions = iface.getVersions(workflowId) or [] + return _ok(name, { + "workflowId": workflowId, + "versions": [ + { + "id": v.get("id"), + "versionNumber": v.get("versionNumber"), + "status": v.get("status"), + "publishedAt": v.get("publishedAt"), + "publishedBy": v.get("publishedBy"), + } + for v in versions + ], + }) except Exception as e: logger.exception("listWorkflowHistory failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) async def _readWorkflowMessages(params: Dict[str, Any], context: Any) -> ToolResult: """Read recent run logs/messages for a workflow.""" + name = "readWorkflowMessages" try: - workflowId = params.get("workflowId", "") - instanceId = params.get("instanceId", "") - from modules.features.graphicalEditor.interfaceFeatureGraphicalEditor import getGraphicalEditorInterface - user = getattr(context, "user", None) - mandateId = getattr(context, "mandateId", "") or "" - iface = getGraphicalEditorInterface(user, mandateId, instanceId) + workflowId, instanceId = _resolveIds(params, context) + if not workflowId or not instanceId: + return _err(name, "workflowId and instanceId required") + iface = _getInterface(context, instanceId) from modules.features.graphicalEditor.datamodelFeatureGraphicalEditor import AutoRun runs = iface.db.getRecordset(AutoRun, recordFilter={"workflowId": workflowId}) or [] runSummaries = [] @@ -329,104 +408,106 @@ async def _readWorkflowMessages(params: Dict[str, Any], context: Any) -> ToolRes "completedAt": r.get("completedAt"), "error": r.get("error"), }) - return ToolResult( - success=True, - data={"workflowId": workflowId, "recentRuns": runSummaries}, - ) + return _ok(name, {"workflowId": workflowId, "recentRuns": runSummaries}) except Exception as e: logger.exception("readWorkflowMessages failed: %s", e) - return ToolResult(success=False, error=str(e)) + return _err(name, str(e)) def getWorkflowToolDefinitions() -> List[Dict[str, Any]]: - """Return tool definitions for registration in the ToolRegistry.""" + """Return tool definitions for registration in the ToolRegistry. + + Note: ``workflowId`` and ``instanceId`` are NOT marked ``required`` — + they are auto-injected from the agent context by ``_resolveIds``. The + model may still pass them explicitly (e.g. to target a different + workflow) but doesn't have to repeat them on every call. + """ + _idFields = { + "workflowId": {"type": "string", "description": "Workflow ID (defaults to the current editor workflow)"}, + "instanceId": {"type": "string", "description": "Feature instance ID (defaults to the current editor instance)"}, + } return [ { "name": "readWorkflowGraph", "handler": _readWorkflowGraph, - "description": "Read the current workflow graph (nodes and connections)", + "description": "Read the current workflow graph (nodes and connections). Always call this first to understand the current state before making changes.", "parameters": { "type": "object", - "properties": { - "workflowId": {"type": "string", "description": "Workflow ID"}, - "instanceId": {"type": "string", "description": "Feature instance ID"}, - }, - "required": ["workflowId", "instanceId"], + "properties": {**_idFields}, + "required": [], }, + "readOnly": True, "toolSet": TOOLBOX_ID, }, { "name": "addNode", "handler": _addNode, - "description": "Add a node to the workflow graph", + "description": "Add a node to the workflow graph.", "parameters": { "type": "object", "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, - "nodeType": {"type": "string", "description": "Node type (e.g. ai.chat, email.send)"}, + **_idFields, + "nodeType": {"type": "string", "description": "Node type id (e.g. ai.chat, email.send) — use listAvailableNodeTypes to discover"}, "title": {"type": "string", "description": "Human-readable title"}, "parameters": {"type": "object", "description": "Node parameters"}, "position": {"type": "object", "description": "Canvas position {x, y}"}, + "nodeId": {"type": "string", "description": "Optional explicit node id"}, }, - "required": ["workflowId", "instanceId", "nodeType"], + "required": ["nodeType"], }, "toolSet": TOOLBOX_ID, }, { "name": "removeNode", "handler": _removeNode, - "description": "Remove a node and its connections from the graph", + "description": "Remove a node and its connections from the graph.", "parameters": { "type": "object", "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, + **_idFields, "nodeId": {"type": "string", "description": "ID of the node to remove"}, }, - "required": ["workflowId", "instanceId", "nodeId"], + "required": ["nodeId"], }, "toolSet": TOOLBOX_ID, }, { "name": "connectNodes", "handler": _connectNodes, - "description": "Connect two nodes in the graph", + "description": "Connect two nodes in the graph (source -> target).", "parameters": { "type": "object", "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, + **_idFields, "sourceId": {"type": "string"}, "targetId": {"type": "string"}, "sourceOutput": {"type": "integer", "default": 0}, "targetInput": {"type": "integer", "default": 0}, }, - "required": ["workflowId", "instanceId", "sourceId", "targetId"], + "required": ["sourceId", "targetId"], }, "toolSet": TOOLBOX_ID, }, { "name": "setNodeParameter", "handler": _setNodeParameter, - "description": "Set a parameter on a node", + "description": "Set a single parameter on a node.", "parameters": { "type": "object", "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, + **_idFields, "nodeId": {"type": "string"}, "parameterName": {"type": "string"}, "parameterValue": {"description": "Value to set (any type)"}, }, - "required": ["workflowId", "instanceId", "nodeId", "parameterName", "parameterValue"], + "required": ["nodeId", "parameterName", "parameterValue"], }, "toolSet": TOOLBOX_ID, }, { "name": "listAvailableNodeTypes", "handler": _listAvailableNodeTypes, - "description": "List all available node types for the flow builder", + "description": "List all available node types for the flow builder. Call this once to discover ids before using addNode.", "parameters": {"type": "object", "properties": {}}, "readOnly": True, "toolSet": TOOLBOX_ID, @@ -434,14 +515,11 @@ def getWorkflowToolDefinitions() -> List[Dict[str, Any]]: { "name": "validateGraph", "handler": _validateGraph, - "description": "Validate a workflow graph for common issues", + "description": "Validate a workflow graph for common issues (missing trigger, dangling connections, orphans).", "parameters": { "type": "object", - "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, - }, - "required": ["workflowId", "instanceId"], + "properties": {**_idFields}, + "required": [], }, "readOnly": True, "toolSet": TOOLBOX_ID, @@ -449,14 +527,11 @@ def getWorkflowToolDefinitions() -> List[Dict[str, Any]]: { "name": "listWorkflowHistory", "handler": _listWorkflowHistory, - "description": "List version history for a workflow (AutoVersion entries)", + "description": "List version history for a workflow (AutoVersion entries).", "parameters": { "type": "object", - "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, - }, - "required": ["workflowId", "instanceId"], + "properties": {**_idFields}, + "required": [], }, "readOnly": True, "toolSet": TOOLBOX_ID, @@ -464,14 +539,11 @@ def getWorkflowToolDefinitions() -> List[Dict[str, Any]]: { "name": "readWorkflowMessages", "handler": _readWorkflowMessages, - "description": "Read recent run logs and status for a workflow", + "description": "Read recent run logs and status for a workflow.", "parameters": { "type": "object", - "properties": { - "workflowId": {"type": "string"}, - "instanceId": {"type": "string"}, - }, - "required": ["workflowId", "instanceId"], + "properties": {**_idFields}, + "required": [], }, "readOnly": True, "toolSet": TOOLBOX_ID, From 1ffe521ad862d4ab19cb5bc8988a99c49e91e8a1 Mon Sep 17 00:00:00 2001 From: ValueOn AG Date: Sun, 19 Apr 2026 01:31:57 +0200 Subject: [PATCH 5/5] grapheditor enhanced para set for ai --- .../routeFeatureGraphicalEditor.py | 31 +- .../services/serviceAgent/workflowTools.py | 303 +++++++++++++++++- 2 files changed, 317 insertions(+), 17 deletions(-) diff --git a/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py b/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py index 7b30ec16..494cebb9 100644 --- a/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py +++ b/modules/features/graphicalEditor/routeFeatureGraphicalEditor.py @@ -757,12 +757,31 @@ async def _runEditorAgent( "for the user — you must NEVER execute the workflow or any of its actions. " "Even when the user says 'create a workflow that sends an email', you build the " "graph (e.g. add an email node, connect it) — you do NOT actually send an email. " - "Use these workflow tools to mutate the graph: " - "readWorkflowGraph, listAvailableNodeTypes, addNode, removeNode, connectNodes, " - "setNodeParameter, validateGraph. " - "Always read the current graph and list available node types first, then plan the " - "smallest set of mutations, then apply them. Respond concisely in the user's " - "language and confirm what you changed in the graph." + "\n\nGraph-mutating tools: readWorkflowGraph, listAvailableNodeTypes, " + "describeNodeType, addNode, removeNode, connectNodes, setNodeParameter, " + "autoLayoutWorkflow, validateGraph. " + "Connection discovery (for parameters of frontendType='userConnection'): listConnections." + "\n\nMandatory build sequence:" + "\n1. readWorkflowGraph — understand current state." + "\n2. listAvailableNodeTypes — find candidate node ids." + "\n3. For EACH node type you plan to add: call describeNodeType(nodeType=...) " + "to learn its requiredParameters, allowedValues and ports. Never skip this " + "step — guessing parameters leaves the user with empty config cards." + "\n4. If any required parameter has frontendType='userConnection' (e.g. " + "email.checkEmail.connectionReference), call listConnections and pick the " + "connectionId that matches the user's intent (or ask the user if none clearly fits)." + "\n5. addNode with parameters={...} containing AT LEAST every requiredParameter " + "filled with a sensible value (use the user's request, the parameter " + "description, sane defaults, or — for required user-connection fields — " + "an actual connectionId). Do NOT pass position; the layout step handles it." + "\n6. connectNodes — wire the nodes consistent with port schemas from describeNodeType." + "\n7. autoLayoutWorkflow — call exactly once as the LAST graph-mutating step so the " + "canvas shows a readable top-down layout instead of overlapping boxes." + "\n8. validateGraph — sanity check, then answer the user." + "\n\nIf a required parameter cannot be filled from the user's request and has " + "no safe default, ask the user once for that specific value (e.g. recipient " + "address, target language, prompt text) instead of leaving the field blank. " + "Respond concisely in the user's language and list what you changed in the graph." ) editorConfig = AgentConfig( diff --git a/modules/serviceCenter/services/serviceAgent/workflowTools.py b/modules/serviceCenter/services/serviceAgent/workflowTools.py index 3e1bb9ad..e0be2278 100644 --- a/modules/serviceCenter/services/serviceAgent/workflowTools.py +++ b/modules/serviceCenter/services/serviceAgent/workflowTools.py @@ -3,7 +3,8 @@ """ Workflow Toolbox - AI-assisted graph manipulation tools for the GraphicalEditor. Tools: readWorkflowGraph, addNode, removeNode, connectNodes, setNodeParameter, - listAvailableNodeTypes, validateGraph, listWorkflowHistory, readWorkflowMessages. + listAvailableNodeTypes, describeNodeType, autoLayoutWorkflow, + validateGraph, listWorkflowHistory, readWorkflowMessages. Conventions enforced here (matches coreTools / actionToolAdapter): - Every ``ToolResult(...)`` provides ``toolCallId`` and ``toolName`` (pydantic @@ -144,14 +145,32 @@ async def _addNode(params: Dict[str, Any], context: Any) -> ToolResult: nodeId = params.get("nodeId") or str(uuid.uuid4())[:8] title = params.get("title", "") nodeParams = params.get("parameters", {}) or {} - position = params.get("position") or {"x": len(nodes) * 200, "y": 100} + + # Frontend stores positions as TOP-LEVEL ``x`` / ``y`` on the node + # (see ``fromApiGraph`` / ``toApiGraph``). Accept either explicit + # ``x`` / ``y`` or a ``position={x,y}`` shape from the model and + # always persist as top-level ``x`` / ``y``. Fallback puts new + # nodes in a horizontal stripe so the user sees them even before + # ``autoLayoutWorkflow`` runs. + position = params.get("position") or {} + x = params.get("x") + if x is None: + x = position.get("x") if isinstance(position, dict) else None + if x is None: + x = 40 + len(nodes) * 260 + y = params.get("y") + if y is None: + y = position.get("y") if isinstance(position, dict) else None + if y is None: + y = 40 newNode = { "id": nodeId, "type": nodeType, "title": title, "parameters": nodeParams, - "position": position, + "x": x, + "y": y, } nodes.append(newNode) graph["nodes"] = nodes @@ -287,8 +306,55 @@ def _coerceLabel(rawLabel: Any, fallback: str) -> str: return fallback +def _summarizeNodeForCatalog(n: Dict[str, Any]) -> Dict[str, Any]: + """Compact summary used in ``listAvailableNodeTypes`` — small but + informative enough that the model can pick the right type and knows + whether ``describeNodeType`` is worth a follow-up call.""" + nodeId = n.get("id") or "" + paramsList = n.get("parameters") or [] + requiredCount = sum(1 for p in paramsList if isinstance(p, dict) and p.get("required")) + return { + "id": nodeId, + "category": n.get("category"), + "label": _coerceLabel(n.get("label"), nodeId), + "description": _coerceLabel(n.get("description"), ""), + "paramCount": len(paramsList), + "requiredParamCount": requiredCount, + "usesAi": bool(((n.get("meta") or {}).get("usesAi"))), + } + + +def _summarizeParameter(p: Dict[str, Any]) -> Dict[str, Any]: + """Reduce a node parameter spec to just what the AI needs to fill it.""" + out: Dict[str, Any] = { + "name": p.get("name"), + "type": p.get("type"), + "required": bool(p.get("required")), + "frontendType": p.get("frontendType"), + "description": _coerceLabel(p.get("description"), ""), + } + if "default" in p: + out["default"] = p.get("default") + feOpts = p.get("frontendOptions") + if isinstance(feOpts, dict): + # Expose enum-style choices ("options") so the model sticks to allowed values. + if isinstance(feOpts.get("options"), list): + out["allowedValues"] = feOpts.get("options") + if p.get("frontendType") == "userConnection": + out["hint"] = ( + "Call listConnections to discover available connections; pass the " + "connectionId here. Required before this node can run." + ) + return out + + async def _listAvailableNodeTypes(params: Dict[str, Any], context: Any) -> ToolResult: - """List all available node types for the flow builder.""" + """List all available node types for the flow builder (compact catalog). + + Returns ``id``, ``category``, ``label``, short ``description``, and the + parameter counts. To learn HOW to fill a node's parameters use + ``describeNodeType(nodeType=...)`` — that returns the full schema. + """ name = "listAvailableNodeTypes" try: from modules.features.graphicalEditor.nodeDefinitions import STATIC_NODE_TYPES @@ -296,18 +362,188 @@ async def _listAvailableNodeTypes(params: Dict[str, Any], context: Any) -> ToolR for n in STATIC_NODE_TYPES: if not isinstance(n, dict): continue - nodeId = n.get("id") or "" - nodeTypes.append({ - "id": nodeId, - "category": n.get("category"), - "label": _coerceLabel(n.get("label"), nodeId), - }) + nodeTypes.append(_summarizeNodeForCatalog(n)) return _ok(name, {"nodeTypes": nodeTypes, "count": len(nodeTypes)}) except Exception as e: logger.exception("listAvailableNodeTypes failed: %s", e) return _err(name, str(e)) +async def _describeNodeType(params: Dict[str, Any], context: Any) -> ToolResult: + """Return the full schema for a single node type so the AI can fill + ``addNode.parameters`` correctly (which fields are required, what types, + default values, allowed enum values, what each port expects/produces). + + This is the canonical way to discover required parameters before + calling ``addNode`` — without it the model guesses ``parameters={}`` + and the user gets empty configuration cards. + """ + name = "describeNodeType" + try: + nodeType = params.get("nodeType") or params.get("id") + if not nodeType: + return _err(name, "nodeType required") + from modules.features.graphicalEditor.nodeDefinitions import STATIC_NODE_TYPES + target: Dict[str, Any] = {} + for n in STATIC_NODE_TYPES: + if isinstance(n, dict) and n.get("id") == nodeType: + target = n + break + if not target: + return _err(name, f"Unknown nodeType '{nodeType}' — call listAvailableNodeTypes first") + + rawParams = target.get("parameters") or [] + parameters = [ + _summarizeParameter(p) for p in rawParams if isinstance(p, dict) + ] + + def _portList(portsDict: Any) -> List[Dict[str, Any]]: + if not isinstance(portsDict, dict): + return [] + out: List[Dict[str, Any]] = [] + for idx, spec in sorted(portsDict.items(), key=lambda kv: int(kv[0]) if str(kv[0]).isdigit() else 0): + if not isinstance(spec, dict): + continue + entry: Dict[str, Any] = {"index": int(idx) if str(idx).isdigit() else idx} + if "schema" in spec: + entry["schema"] = spec.get("schema") + if "accepts" in spec: + entry["accepts"] = spec.get("accepts") + out.append(entry) + return out + + meta = target.get("meta") or {} + return _ok(name, { + "id": target.get("id"), + "category": target.get("category"), + "label": _coerceLabel(target.get("label"), target.get("id") or ""), + "description": _coerceLabel(target.get("description"), ""), + "usesAi": bool(meta.get("usesAi")), + "inputs": int(target.get("inputs") or 0), + "outputs": int(target.get("outputs") or 0), + "inputPorts": _portList(target.get("inputPorts")), + "outputPorts": _portList(target.get("outputPorts")), + "parameters": parameters, + "requiredParameters": [p["name"] for p in parameters if p.get("required")], + }) + except Exception as e: + logger.exception("describeNodeType failed: %s", e) + return _err(name, str(e)) + + +# Geometry constants — MUST match the frontend (FlowCanvas.tsx) so the +# server-side auto-layout produces the exact same coordinates the user +# would get by clicking "Arrange" in the UI. +_NODE_WIDTH = 200 +_NODE_HEIGHT = 72 +_LAYOUT_V_GAP = 80 +_LAYOUT_H_GAP = 60 +_LAYOUT_START_X = 40 +_LAYOUT_START_Y = 40 + + +def _computeAutoLayout( + nodes: List[Dict[str, Any]], + connections: List[Dict[str, Any]], +) -> List[Dict[str, Any]]: + """Topological-layer layout — port of ``computeAutoLayout`` in FlowCanvas.tsx. + + Arranges nodes top-to-bottom in layers (one layer per BFS step from the + sources). Disconnected nodes are appended as extra single-node layers, + same as the frontend. Returns a NEW node list with updated top-level + ``x``/``y``; legacy ``position`` keys are stripped to avoid two + competing sources of truth. + """ + if not nodes: + return nodes + + nodeIds = {n.get("id") for n in nodes if n.get("id")} + inDegree: Dict[str, int] = {nid: 0 for nid in nodeIds if nid} + children: Dict[str, List[str]] = {nid: [] for nid in nodeIds if nid} + + for c in connections or []: + src = c.get("source") + tgt = c.get("target") + if src in inDegree and tgt in inDegree: + inDegree[tgt] = inDegree[tgt] + 1 + children[src].append(tgt) + + layers: List[List[str]] = [] + layerOf: Dict[str, int] = {} + queue: List[str] = [nid for nid, deg in inDegree.items() if deg == 0] + + while queue: + batch = list(queue) + queue = [] + layerIdx = len(layers) + layers.append(batch) + for nid in batch: + layerOf[nid] = layerIdx + for childId in children.get(nid, []): + inDegree[childId] = inDegree[childId] - 1 + if inDegree[childId] == 0: + queue.append(childId) + + # Cycles: append remaining nodes as their own layers (matches frontend). + for n in nodes: + nid = n.get("id") + if nid and nid not in layerOf: + layerIdx = len(layers) + layers.append([nid]) + layerOf[nid] = layerIdx + + out: List[Dict[str, Any]] = [] + for n in nodes: + nid = n.get("id") + layer = layerOf.get(nid, 0) if nid else 0 + siblings = layers[layer] if 0 <= layer < len(layers) else [nid] + idxInLayer = siblings.index(nid) if nid in siblings else 0 + new = dict(n) + new["x"] = _LAYOUT_START_X + idxInLayer * (_NODE_WIDTH + _LAYOUT_H_GAP) + new["y"] = _LAYOUT_START_Y + layer * (_NODE_HEIGHT + _LAYOUT_V_GAP) + # Strip legacy ``position`` so frontend never sees two coordinates. + new.pop("position", None) + out.append(new) + return out + + +async def _autoLayoutWorkflow(params: Dict[str, Any], context: Any) -> ToolResult: + """Re-arrange all nodes of the workflow into a clean top-down layered layout. + + Same algorithm as the editor's "Arrange" button — call this after you + finished adding/connecting nodes so the user doesn't see an unreadable + pile of overlapping boxes. + """ + name = "autoLayoutWorkflow" + try: + workflowId, instanceId = _resolveIds(params, context) + if not workflowId or not instanceId: + return _err(name, "workflowId and instanceId required (and not present in agent context)") + + iface = _getInterface(context, instanceId) + wf = iface.getWorkflow(workflowId) + if not wf: + return _err(name, f"Workflow {workflowId} not found") + + graph = dict(wf.get("graph", {}) or {}) + nodes = list(graph.get("nodes", []) or []) + connections = list(graph.get("connections", []) or []) + if not nodes: + return _ok(name, {"message": "No nodes to layout", "nodeCount": 0}) + + graph["nodes"] = _computeAutoLayout(nodes, connections) + iface.updateWorkflow(workflowId, {"graph": graph}) + + return _ok(name, { + "message": f"Auto-layout applied to {len(nodes)} nodes", + "nodeCount": len(nodes), + "layerCount": max((c.get("y", 0) for c in graph["nodes"]), default=_LAYOUT_START_Y) // (_NODE_HEIGHT + _LAYOUT_V_GAP) + 1, + }) + except Exception as e: + logger.exception("autoLayoutWorkflow failed: %s", e) + return _err(name, str(e)) + + async def _validateGraph(params: Dict[str, Any], context: Any) -> ToolResult: """Validate a workflow graph for common issues.""" name = "validateGraph" @@ -507,11 +743,56 @@ def getWorkflowToolDefinitions() -> List[Dict[str, Any]]: { "name": "listAvailableNodeTypes", "handler": _listAvailableNodeTypes, - "description": "List all available node types for the flow builder. Call this once to discover ids before using addNode.", + "description": ( + "List all available node types (compact catalog: id, label, " + "description, paramCount, requiredParamCount, usesAi). Call this " + "once to discover ids; then call describeNodeType for each type " + "you intend to add to learn the parameter schema." + ), "parameters": {"type": "object", "properties": {}}, "readOnly": True, "toolSet": TOOLBOX_ID, }, + { + "name": "describeNodeType", + "handler": _describeNodeType, + "description": ( + "Return the FULL parameter schema for a single node type " + "(name, type, required, default, allowedValues, description) " + "plus input/output ports. ALWAYS call this before addNode for " + "any node type that has requiredParamCount > 0, and pass all " + "required parameters into addNode — otherwise the user sees an " + "empty configuration card. For parameters with " + "frontendType='userConnection' call listConnections to obtain " + "a connectionId." + ), + "parameters": { + "type": "object", + "properties": { + "nodeType": {"type": "string", "description": "Node type id from listAvailableNodeTypes (e.g. 'email.checkEmail', 'ai.prompt')"}, + }, + "required": ["nodeType"], + }, + "readOnly": True, + "toolSet": TOOLBOX_ID, + }, + { + "name": "autoLayoutWorkflow", + "handler": _autoLayoutWorkflow, + "description": ( + "Re-arrange ALL nodes into a clean top-down layered layout " + "(same algorithm as the editor's 'Arrange' button). Call this " + "AFTER you finished adding nodes and connections, otherwise the " + "user sees a pile of overlapping boxes. Idempotent — safe to " + "call multiple times." + ), + "parameters": { + "type": "object", + "properties": {**_idFields}, + "required": [], + }, + "toolSet": TOOLBOX_ID, + }, { "name": "validateGraph", "handler": _validateGraph,