diff --git a/application/single_app/background_tasks.py b/application/single_app/background_tasks.py index 4d72fde48..b6cc370b1 100644 --- a/application/single_app/background_tasks.py +++ b/application/single_app/background_tasks.py @@ -245,6 +245,7 @@ def check_logging_timers_once(): def check_expired_approvals_once(): """Auto-deny expired approval requests and return the affected count.""" from functions_approvals import auto_deny_expired_approvals + from functions_simplechat_operations import auto_deny_expired_generated_file_approvals lock_document = acquire_distributed_task_lock('approval_expiry', lease_seconds=1800) if not lock_document: @@ -255,6 +256,18 @@ def check_expired_approvals_once(): denied_count = auto_deny_expired_approvals() if denied_count > 0: print(f"Auto-denied {denied_count} expired approval request(s).") + + try: + expired_file_count = auto_deny_expired_generated_file_approvals() + if expired_file_count > 0: + print(f"Auto-denied {expired_file_count} expired generated file approval(s).") + except Exception as exc: + # Staged file expiry must never take down the Control Center approval sweep. + print(f"Error expiring staged generated file approvals: {exc}") + log_event( + f"Error expiring staged generated file approvals: {exc}", + level=logging.ERROR, + ) finally: release_distributed_task_lock(lock_document) diff --git a/application/single_app/collaboration_models.py b/application/single_app/collaboration_models.py index e2f322ea2..60dbd875a 100644 --- a/application/single_app/collaboration_models.py +++ b/application/single_app/collaboration_models.py @@ -7,6 +7,7 @@ COLLABORATION_KIND = 'collaborative' +COLLABORATION_SOURCE_KIND = 'collaboration_source' PERSONAL_MULTI_USER_CHAT_TYPE = 'personal_multi_user' GROUP_MULTI_USER_CHAT_TYPE = 'group_multi_user' diff --git a/application/single_app/config.py b/application/single_app/config.py index 0f2645f31..179e25894 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.260.005" +VERSION = "0.260.006" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_collaboration.py b/application/single_app/functions_collaboration.py index 75e9e78c0..f0ec2325b 100644 --- a/application/single_app/functions_collaboration.py +++ b/application/single_app/functions_collaboration.py @@ -8,6 +8,7 @@ from config import * from collaboration_models import ( COLLABORATION_KIND, + COLLABORATION_SOURCE_KIND, GROUP_MULTI_USER_CHAT_TYPE, MEMBERSHIP_ROLE_ADMIN, MEMBERSHIP_ROLE_MEMBER, @@ -1568,6 +1569,91 @@ def assert_user_can_participate_in_collaboration_conversation(user_id, conversat return access_context +def is_collaboration_source_conversation(conversation_item): + """Return True when a personal conversation is the hidden backing store of a shared one.""" + normalized_item = conversation_item or {} + return bool( + str(normalized_item.get('conversation_kind') or '').strip() == COLLABORATION_SOURCE_KIND + or str(normalized_item.get('collaboration_conversation_id') or '').strip() + ) + + +def get_collaboration_conversation_for_source(conversation_item): + """Load the shared conversation that owns a collaboration source conversation.""" + collaboration_conversation_id = str( + (conversation_item or {}).get('collaboration_conversation_id') or '' + ).strip() + if not collaboration_conversation_id: + return None + + try: + return get_collaboration_conversation(collaboration_conversation_id) + except CosmosResourceNotFoundError: + return None + + +def build_conversation_participation_context(user_id, conversation_item): + """Authorize one caller against a personal conversation or its linked shared conversation. + + Ordinary personal conversations stay owner-only. Collaboration source conversations are + always owned by the shared conversation creator, so every other participant fails a plain + ownership comparison even though they are legitimate members. Those callers are authorized + against the linked collaboration conversation instead, mirroring the chat upload path in + ``route_frontend_chats._resolve_chat_upload_context``. + + Returns a context describing how access was granted so callers can distinguish an owner + acting on their own conversation from a participant acting inside a shared one. + """ + normalized_user_id = str(user_id or '').strip() + normalized_item = conversation_item or {} + owner_user_id = str(normalized_item.get('user_id') or '').strip() + conversation_id = str(normalized_item.get('id') or '').strip() + + if normalized_user_id and owner_user_id == normalized_user_id: + return { + 'user_id': normalized_user_id, + 'conversation_id': conversation_id, + 'owner_user_id': owner_user_id, + 'is_owner': True, + 'is_collaboration_source': is_collaboration_source_conversation(normalized_item), + 'collaboration_conversation': None, + 'collaboration_conversation_id': str( + normalized_item.get('collaboration_conversation_id') or '' + ).strip(), + 'collaboration_access': None, + 'group_id': '', + 'group_role': '', + } + + collaboration_conversation = get_collaboration_conversation_for_source(normalized_item) + if not collaboration_conversation: + raise PermissionError('You can only access your own conversations') + + collaboration_access = assert_user_can_participate_in_collaboration_conversation( + normalized_user_id, + collaboration_conversation, + ) + + group_id = '' + if is_group_collaboration_conversation(collaboration_conversation): + group_id = str( + (collaboration_conversation.get('scope') or {}).get('group_id') or '' + ).strip() + + return { + 'user_id': normalized_user_id, + 'conversation_id': conversation_id, + 'owner_user_id': owner_user_id, + 'is_owner': False, + 'is_collaboration_source': True, + 'collaboration_conversation': collaboration_conversation, + 'collaboration_conversation_id': str(collaboration_conversation.get('id') or '').strip(), + 'collaboration_access': collaboration_access, + 'group_id': group_id, + 'group_role': str((collaboration_access or {}).get('group_role') or '').strip(), + } + + def record_personal_invite_response(conversation_id, user_id, action): conversation_doc = get_collaboration_conversation(conversation_id) if not is_explicit_membership_collaboration(conversation_doc): @@ -1930,7 +2016,7 @@ def ensure_collaboration_source_conversation(conversation_doc, current_user): 'locked_contexts': list((conversation_doc or {}).get('locked_contexts', []) or []), 'classification': list((conversation_doc or {}).get('classification', []) or []), 'summary': (conversation_doc or {}).get('summary'), - 'conversation_kind': 'collaboration_source', + 'conversation_kind': COLLABORATION_SOURCE_KIND, 'collaboration_conversation_id': (conversation_doc or {}).get('id'), 'is_hidden': True, } @@ -1950,7 +2036,7 @@ def ensure_collaboration_source_conversation(conversation_doc, current_user): 'locked_contexts': list((conversation_doc or {}).get('locked_contexts', []) or source_conversation_doc.get('locked_contexts', []) or []), 'classification': list((conversation_doc or {}).get('classification', []) or source_conversation_doc.get('classification', []) or []), 'summary': (conversation_doc or {}).get('summary', source_conversation_doc.get('summary')), - 'conversation_kind': 'collaboration_source', + 'conversation_kind': COLLABORATION_SOURCE_KIND, 'collaboration_conversation_id': (conversation_doc or {}).get('id'), 'is_hidden': True, } diff --git a/application/single_app/functions_generated_file_approvals.py b/application/single_app/functions_generated_file_approvals.py new file mode 100644 index 000000000..f84fd7673 --- /dev/null +++ b/application/single_app/functions_generated_file_approvals.py @@ -0,0 +1,334 @@ +# functions_generated_file_approvals.py + +"""Owner approval gate for files generated by participants in shared conversations. + +Shared conversations are backed by a hidden source conversation owned by the shared +conversation creator, so any generated file a participant asks for is written into the +owner's storage scope. Rather than failing those requests outright, the artifact is written +immediately in a ``pending_approval`` state and only becomes readable once an authorized +approver releases it. Approval is therefore a cheap state flip and the model never re-runs. + +Approval state lives on the artifact message metadata so it travels with the artifact and +cannot be bypassed by reaching the download route directly. Approver discovery is handled by +notifications, which avoids a second index that could drift out of sync with the artifact. +""" + +import logging +import os +from datetime import datetime, timedelta, timezone +from typing import Any, Dict, List, Optional + +from config import cosmos_messages_container +from functions_appinsights import log_event +from functions_group import find_group_by_id, get_user_role_in_group +from functions_settings import get_settings + + +APPROVAL_STATE_PENDING = "pending_approval" +APPROVAL_STATE_APPROVED = "approved" +APPROVAL_STATE_DENIED = "denied" +APPROVAL_STATE_AUTO_DENIED = "auto_denied" + +APPROVAL_TERMINAL_STATES = frozenset({ + APPROVAL_STATE_APPROVED, + APPROVAL_STATE_DENIED, + APPROVAL_STATE_AUTO_DENIED, +}) + +APPROVAL_SCOPE_PERSONAL = "personal" +APPROVAL_SCOPE_GROUP = "group" + +# Mirrors functions_approvals.TTL_AUTO_DENY_DAYS so both approval surfaces expire alike. +APPROVAL_TTL_DAYS = 3 +APPROVAL_TTL_SECONDS = APPROVAL_TTL_DAYS * 24 * 60 * 60 + +APPROVAL_SETTING_KEY = "require_shared_conversation_file_approval" + +# Downloadable deliverables only. Generated images and charts are inline conversation +# rendering, and gating those would make ordinary chat look broken. +APPROVAL_GATED_FILE_EXTENSIONS = frozenset({ + "csv", + "xlsx", + "xls", + "xlsm", + "docx", + "pdf", + "json", + "xml", +}) + +GROUP_DOCUMENT_APPROVER_ROLES = ("Owner", "Admin", "DocumentManager") + +GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE = "generated_file_approval_pending" + + +def _utc_now() -> datetime: + return datetime.now(timezone.utc) + + +def _utc_now_iso() -> str: + return _utc_now().isoformat() + + +def _clean(value: Any) -> str: + return str(value or "").strip() + + +def _metadata_of(message_item: Optional[Dict[str, Any]]) -> Dict[str, Any]: + metadata = (message_item or {}).get("metadata") + return metadata if isinstance(metadata, dict) else {} + + +def is_generated_file_approval_enabled(settings: Optional[Dict[str, Any]] = None) -> bool: + """Return whether the approval gate is switched on for this tenant.""" + resolved_settings = settings if isinstance(settings, dict) else (get_settings() or {}) + return bool(resolved_settings.get(APPROVAL_SETTING_KEY, True)) + + +def normalize_approval_file_extension(file_name: str = "", output_format: str = "") -> str: + """Return the lowercase extension used to decide whether a write is gated.""" + normalized_format = _clean(output_format).lower().lstrip(".") + if normalized_format: + return normalized_format + return os.path.splitext(_clean(file_name))[1].lower().lstrip(".") + + +def is_approval_gated_file(file_name: str = "", output_format: str = "") -> bool: + """Return whether this artifact format is a downloadable deliverable needing approval.""" + return normalize_approval_file_extension(file_name, output_format) in APPROVAL_GATED_FILE_EXTENSIONS + + +def requires_generated_file_approval( + access_context: Optional[Dict[str, Any]], + file_name: str = "", + output_format: str = "", + settings: Optional[Dict[str, Any]] = None, +) -> bool: + """Return whether one artifact write must be staged for approval. + + Owners writing into their own conversation are never gated. Only a non-owner participant + of a shared conversation producing a downloadable deliverable triggers the gate. + """ + context = access_context if isinstance(access_context, dict) else {} + if not context or context.get("is_owner", True): + return False + if not context.get("collaboration_conversation_id"): + return False + if not is_approval_gated_file(file_name, output_format): + return False + return is_generated_file_approval_enabled(settings) + + +def build_generated_file_approval_metadata( + access_context: Dict[str, Any], + requester: Optional[Dict[str, Any]] = None, + requested_at: str = "", +) -> Dict[str, Any]: + """Build the pending-approval metadata fragment stored on the artifact message.""" + context = access_context if isinstance(access_context, dict) else {} + requester_info = requester if isinstance(requester, dict) else {} + normalized_requested_at = _clean(requested_at) or _utc_now_iso() + expires_at = ( + _utc_now() + timedelta(days=APPROVAL_TTL_DAYS) + ).isoformat() + + group_id = _clean(context.get("group_id")) + approval_scope = APPROVAL_SCOPE_GROUP if group_id else APPROVAL_SCOPE_PERSONAL + + return { + "generated_artifact_approval_required": True, + "generated_artifact_approval_state": APPROVAL_STATE_PENDING, + "generated_artifact_approval_scope": approval_scope, + "generated_artifact_approval_group_id": group_id, + "generated_artifact_approval_owner_user_id": _clean(context.get("owner_user_id")), + "generated_artifact_approval_collaboration_conversation_id": _clean( + context.get("collaboration_conversation_id") + ), + "generated_artifact_approval_requested_by_id": _clean( + requester_info.get("user_id") or requester_info.get("userId") or context.get("user_id") + ), + "generated_artifact_approval_requested_by_name": _clean( + requester_info.get("display_name") or requester_info.get("displayName") + ), + "generated_artifact_approval_requested_by_email": _clean(requester_info.get("email")), + "generated_artifact_approval_requested_at": normalized_requested_at, + "generated_artifact_approval_expires_at": expires_at, + "generated_artifact_approval_resolved_by_id": None, + "generated_artifact_approval_resolved_by_name": None, + "generated_artifact_approval_resolved_at": None, + } + + +def get_generated_file_approval_state(message_item: Optional[Dict[str, Any]]) -> str: + """Return the approval state recorded on an artifact message, or an empty string.""" + metadata = _metadata_of(message_item) + if not metadata.get("generated_artifact_approval_required"): + return "" + return _clean(metadata.get("generated_artifact_approval_state")).lower() + + +def generated_file_approval_is_pending(message_item: Optional[Dict[str, Any]]) -> bool: + """Return whether an artifact is still waiting on an approval decision.""" + return get_generated_file_approval_state(message_item) == APPROVAL_STATE_PENDING + + +def assert_generated_file_approval_allows_download( + user_id: str, + message_item: Optional[Dict[str, Any]], +) -> None: + """Block artifact content access until an approver has released the file. + + Enforced independently of the tabular export manifest checks so a staged artifact cannot + be reached by any caller, including the participant who requested it. + """ + approval_state = get_generated_file_approval_state(message_item) + if not approval_state or approval_state == APPROVAL_STATE_APPROVED: + return + + if approval_state == APPROVAL_STATE_PENDING: + raise PermissionError("This file is waiting for owner approval") + raise PermissionError("This file was not approved") + + +def resolve_generated_file_approver_role( + user_id: str, + message_item: Optional[Dict[str, Any]], +) -> str: + """Return how a user qualifies to approve an artifact, or an empty string if they cannot.""" + normalized_user_id = _clean(user_id) + if not normalized_user_id: + return "" + + metadata = _metadata_of(message_item) + if not metadata.get("generated_artifact_approval_required"): + return "" + + # A requester is never their own approver, whatever else they hold. Without this a group + # Admin or DocumentManager who is only a participant could stage a file and release it + # themselves, which would make the gate meaningless for them. + if _clean(metadata.get("generated_artifact_approval_requested_by_id")) == normalized_user_id: + return "" + + approval_scope = _clean(metadata.get("generated_artifact_approval_scope")).lower() + if approval_scope == APPROVAL_SCOPE_GROUP: + group_id = _clean(metadata.get("generated_artifact_approval_group_id")) + if not group_id: + return "" + group_doc = find_group_by_id(group_id) + if not group_doc: + return "" + group_role = _clean(get_user_role_in_group(group_doc, normalized_user_id)) + allowed_roles = {role.lower() for role in GROUP_DOCUMENT_APPROVER_ROLES} + if group_role.lower() in allowed_roles: + return group_role + return "" + + if _clean(metadata.get("generated_artifact_approval_owner_user_id")) == normalized_user_id: + return "ConversationOwner" + return "" + + +def user_can_approve_generated_file(user_id: str, message_item: Optional[Dict[str, Any]]) -> bool: + """Return whether a user may approve or deny one staged artifact.""" + return bool(resolve_generated_file_approver_role(user_id, message_item)) + + +def apply_generated_file_approval_decision( + message_item: Dict[str, Any], + decision: str, + resolver: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """Record an approval decision on the artifact message and return the updated document. + + Approving also releases the underlying artifact lifecycle so the existing publication + checks treat the file as readable. + """ + normalized_decision = _clean(decision).lower() + if normalized_decision not in APPROVAL_TERMINAL_STATES: + raise ValueError(f"Unsupported generated file approval decision: {decision}") + + resolver_info = resolver if isinstance(resolver, dict) else {} + metadata = _metadata_of(message_item) + if not metadata.get("generated_artifact_approval_required"): + raise ValueError("Artifact is not gated by an approval") + + current_state = _clean(metadata.get("generated_artifact_approval_state")).lower() + if current_state in APPROVAL_TERMINAL_STATES: + raise ValueError("This file approval was already resolved") + + metadata.update({ + "generated_artifact_approval_state": normalized_decision, + "generated_artifact_approval_resolved_by_id": _clean( + resolver_info.get("user_id") or resolver_info.get("userId") + ) or None, + "generated_artifact_approval_resolved_by_name": _clean( + resolver_info.get("display_name") or resolver_info.get("displayName") + ) or None, + "generated_artifact_approval_resolved_at": _utc_now_iso(), + }) + message_item["metadata"] = metadata + return message_item + + +def build_generated_file_approval_client_payload( + message_item: Optional[Dict[str, Any]], + viewer_user_id: str = "", +) -> Dict[str, Any]: + """Build the approval descriptor the chat UI renders on a gated artifact.""" + approval_state = get_generated_file_approval_state(message_item) + if not approval_state: + return {} + + metadata = _metadata_of(message_item) + normalized_viewer_id = _clean(viewer_user_id) + requested_by_id = _clean(metadata.get("generated_artifact_approval_requested_by_id")) + + return { + "state": approval_state, + "is_pending": approval_state == APPROVAL_STATE_PENDING, + "scope": _clean(metadata.get("generated_artifact_approval_scope")), + "requested_by_id": requested_by_id, + "requested_by_name": _clean(metadata.get("generated_artifact_approval_requested_by_name")), + "requested_at": _clean(metadata.get("generated_artifact_approval_requested_at")), + "expires_at": _clean(metadata.get("generated_artifact_approval_expires_at")), + "resolved_by_name": _clean(metadata.get("generated_artifact_approval_resolved_by_name")), + "resolved_at": _clean(metadata.get("generated_artifact_approval_resolved_at")), + "viewer_is_requester": bool( + normalized_viewer_id and normalized_viewer_id == requested_by_id + ), + "viewer_can_approve": bool( + approval_state == APPROVAL_STATE_PENDING + and user_can_approve_generated_file(normalized_viewer_id, message_item) + ), + } + + +def list_expired_pending_generated_file_artifacts(limit: int = 200) -> List[Dict[str, Any]]: + """Return staged artifacts whose approval window has elapsed.""" + normalized_limit = max(1, min(int(limit or 200), 1000)) + query = ( + "SELECT TOP @limit * FROM c " + "WHERE c.role = 'file' " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_state) " + "AND c.metadata.generated_artifact_approval_state = @pending_state " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_expires_at) " + "AND c.metadata.generated_artifact_approval_expires_at < @now" + ) + try: + return list(cosmos_messages_container.query_items( + query=query, + parameters=[ + {"name": "@limit", "value": normalized_limit}, + {"name": "@pending_state", "value": APPROVAL_STATE_PENDING}, + {"name": "@now", "value": _utc_now_iso()}, + ], + enable_cross_partition_query=True, + )) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to query expired staged artifacts", + {"error": str(exc)}, + level=logging.ERROR, + exceptionTraceback=True, + ) + return [] diff --git a/application/single_app/functions_generated_file_exports.py b/application/single_app/functions_generated_file_exports.py index edd46aa24..c36df4ef5 100644 --- a/application/single_app/functions_generated_file_exports.py +++ b/application/single_app/functions_generated_file_exports.py @@ -613,6 +613,17 @@ def build_generated_file_artifact_metadata( row_source = str(export_payload.get('row_source') or '').strip() if row_source: artifact_metadata['row_source'] = row_source + + # Surfaced immediately so the participant who asked for the file sees the pending state in + # the same response instead of a download button that would be refused. + approval_state = str(uploaded_message.get('approval_state') or '').strip() + if approval_state: + artifact_metadata['approval'] = { + 'state': approval_state, + 'is_pending': approval_state == 'pending_approval', + 'viewer_is_requester': True, + 'viewer_can_approve': False, + } return artifact_metadata diff --git a/application/single_app/functions_notifications.py b/application/single_app/functions_notifications.py index d8865fd43..d0f608d3a 100644 --- a/application/single_app/functions_notifications.py +++ b/application/single_app/functions_notifications.py @@ -174,6 +174,18 @@ 'icon': 'bi-trash', 'color': 'secondary' }, + 'generated_file_approval_pending': { + 'icon': 'bi-file-earmark-lock', + 'color': 'warning' + }, + 'generated_file_approval_approved': { + 'icon': 'bi-file-earmark-check', + 'color': 'success' + }, + 'generated_file_approval_denied': { + 'icon': 'bi-file-earmark-x', + 'color': 'danger' + }, WORKFLOW_ALERT_NOTIFICATION_TYPE: { 'icon': 'bi-bell', 'color': 'secondary' diff --git a/application/single_app/functions_settings.py b/application/single_app/functions_settings.py index 0f542118e..73ad6c8e5 100644 --- a/application/single_app/functions_settings.py +++ b/application/single_app/functions_settings.py @@ -1694,6 +1694,8 @@ def get_settings(use_cosmos=False, include_source=False): # Collaborative Conversations 'enable_collaborative_conversations': True, + # Stage files generated by non-owner participants until an approver releases them. + 'require_shared_conversation_file_approval': True, # Search and Extract 'azure_ai_search_endpoint': '', diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index 2aff415c3..567877176 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -43,6 +43,7 @@ ) from functions_collaboration import ( assert_user_can_participate_in_collaboration_conversation, + build_conversation_participation_context, create_collaboration_message_notifications, create_group_collaboration_conversation_record, create_personal_collaboration_conversation_record, @@ -52,16 +53,30 @@ persist_collaboration_message, ) from functions_documents import allowed_file, create_document, process_document_upload_background, update_document +from functions_generated_file_approvals import ( + APPROVAL_STATE_APPROVED, + APPROVAL_STATE_AUTO_DENIED, + APPROVAL_STATE_DENIED, + APPROVAL_STATE_PENDING, + GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE, + apply_generated_file_approval_decision, + build_generated_file_approval_client_payload, + build_generated_file_approval_metadata, + list_expired_pending_generated_file_artifacts, + requires_generated_file_approval, + user_can_approve_generated_file, +) from functions_chat_bootstrap_cache import bump_chat_bootstrap_global_cache_version from functions_group import ( assert_group_role, check_group_status_allows_operation, create_group, find_group_by_id, + get_user_groups, get_user_role_in_group, require_active_group, ) -from functions_notifications import create_notification +from functions_notifications import create_notification, delete_notifications_by_metadata from functions_personal_workflows import save_personal_workflow from functions_public_workspaces import ( check_public_workspace_status_allows_operation, @@ -1329,8 +1344,9 @@ def delete_generated_chat_artifact_for_user( except CosmosResourceNotFoundError: return False - if str(conversation_item.get("user_id") or "").strip() != current_user_id: - raise PermissionError("Forbidden") + # Shared conversations are owned by their creator, so a participant rolling back their own + # generated artifact is authorized through the linked shared conversation instead. + build_conversation_participation_context(current_user_id, conversation_item) message_metadata = message_item.get("metadata") if isinstance(message_item.get("metadata"), dict) else {} if ( str(message_item.get("conversation_id") or "").strip() != normalized_conversation_id @@ -2553,6 +2569,433 @@ def _upload_generated_document_for_current_user( raise +def _get_current_user_summary_or_none(fallback_user_id: str = "") -> Dict[str, str]: + """Return a best-effort identity summary for the acting user. + + Background workers run outside a request context, so the caller-supplied id is used as the + fallback rather than failing the artifact write. + """ + normalized_fallback_id = str(fallback_user_id or "").strip() + try: + current_user = get_current_user_info() or {} + except Exception: + current_user = {} + + resolved_user_id = str(current_user.get("userId") or "").strip() or normalized_fallback_id + return { + "user_id": resolved_user_id, + "display_name": str(current_user.get("displayName") or "").strip(), + "email": str(current_user.get("email") or "").strip(), + } + + +def _resolve_generated_file_approver_ids(access_context: Dict[str, Any]) -> List[str]: + """Return the users allowed to release one staged artifact.""" + context = access_context if isinstance(access_context, dict) else {} + group_id = str(context.get("group_id") or "").strip() + + if not group_id: + owner_user_id = str(context.get("owner_user_id") or "").strip() + return [owner_user_id] if owner_user_id else [] + + group_doc = find_group_by_id(group_id) + if not group_doc: + return [] + + approver_ids = [] + owner_user_id = str((group_doc.get("owner") or {}).get("id") or "").strip() + if owner_user_id: + approver_ids.append(owner_user_id) + for candidate_id in list(group_doc.get("admins", []) or []) + list(group_doc.get("documentManagers", []) or []): + normalized_candidate_id = str(candidate_id or "").strip() + if normalized_candidate_id and normalized_candidate_id not in approver_ids: + approver_ids.append(normalized_candidate_id) + return approver_ids + + +def _notify_generated_file_approval_requested( + message_doc: Dict[str, Any], + access_context: Dict[str, Any], +) -> None: + """Notify every eligible approver that a participant staged a file for release.""" + metadata = message_doc.get("metadata") if isinstance(message_doc.get("metadata"), dict) else {} + collaboration_conversation_id = str( + metadata.get("generated_artifact_approval_collaboration_conversation_id") or "" + ).strip() + requester_name = str( + metadata.get("generated_artifact_approval_requested_by_name") or "" + ).strip() or "A participant" + file_name = str(message_doc.get("filename") or "").strip() or "a generated file" + + link_url = ( + f"/chats?conversationId={collaboration_conversation_id}" + if collaboration_conversation_id + else "" + ) + notification_metadata = { + "generated_artifact_message_id": str(message_doc.get("id") or "").strip(), + "source_conversation_id": str(message_doc.get("conversation_id") or "").strip(), + "collaboration_conversation_id": collaboration_conversation_id, + "approval_scope": str(metadata.get("generated_artifact_approval_scope") or "").strip(), + "group_id": str(metadata.get("generated_artifact_approval_group_id") or "").strip(), + } + + for approver_user_id in _resolve_generated_file_approver_ids(access_context): + _create_personal_notification( + user_id=approver_user_id, + notification_type=GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE, + title="File approval requested", + message=f"{requester_name} generated {file_name} in a shared conversation and needs your approval.", + link_url=link_url, + link_context={ + "conversation_id": collaboration_conversation_id, + "conversation_kind": "collaborative", + }, + metadata=notification_metadata, + ) + + +def list_pending_generated_file_approvals_for_user(user_id: str, limit: int = 50) -> List[Dict[str, Any]]: + """Return staged artifacts the supplied user is allowed to release. + + Candidates are narrowed in the query by the approval scopes this user could possibly + approve, so the row cap never truncates another user's items ahead of the caller's. + """ + normalized_user_id = str(user_id or "").strip() + if not normalized_user_id: + return [] + + normalized_limit = max(1, min(int(limit or 50), 200)) + approver_group_ids = [] + try: + for group_doc in get_user_groups(normalized_user_id) or []: + group_id = str((group_doc or {}).get("id") or "").strip() + if not group_id or group_id in approver_group_ids: + continue + if str(get_user_role_in_group(group_doc, normalized_user_id) or "").strip() in ( + "Owner", + "Admin", + "DocumentManager", + ): + approver_group_ids.append(group_id) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to resolve approver group scope", + {"error": str(exc)}, + level=logging.WARNING, + exceptionTraceback=True, + ) + + scope_clauses = ["c.metadata.generated_artifact_approval_owner_user_id = @user_id"] + parameters = [ + {"name": "@limit", "value": normalized_limit}, + {"name": "@pending_state", "value": APPROVAL_STATE_PENDING}, + {"name": "@user_id", "value": normalized_user_id}, + ] + if approver_group_ids: + group_placeholders = [] + for index, group_id in enumerate(approver_group_ids): + placeholder = f"@group_id_{index}" + group_placeholders.append(placeholder) + parameters.append({"name": placeholder, "value": group_id}) + scope_clauses.append( + f"c.metadata.generated_artifact_approval_group_id IN ({', '.join(group_placeholders)})" + ) + + query = ( + "SELECT TOP @limit * FROM c " + "WHERE c.role = 'file' " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_state) " + "AND c.metadata.generated_artifact_approval_state = @pending_state " + "AND c.metadata.generated_artifact_approval_requested_by_id != @user_id " + f"AND ({' OR '.join(scope_clauses)})" + ) + try: + candidates = list(cosmos_messages_container.query_items( + query=query, + parameters=parameters, + enable_cross_partition_query=True, + )) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to list pending approvals", + {"error": str(exc)}, + level=logging.ERROR, + exceptionTraceback=True, + ) + return [] + + pending_approvals = [] + for candidate in candidates: + # The query narrows candidates; authorization is still decided by the shared predicate. + if not user_can_approve_generated_file(normalized_user_id, candidate): + continue + candidate_metadata = candidate.get("metadata") or {} + pending_approvals.append({ + "artifact_message_id": str(candidate.get("id") or "").strip(), + "source_conversation_id": str(candidate.get("conversation_id") or "").strip(), + "collaboration_conversation_id": str( + candidate_metadata.get("generated_artifact_approval_collaboration_conversation_id") or "" + ).strip(), + "file_name": str(candidate.get("filename") or "").strip(), + "output_format": str(candidate_metadata.get("generated_artifact_output_format") or "").strip(), + "approval": build_generated_file_approval_client_payload(candidate, normalized_user_id), + }) + return pending_approvals + + +def resolve_generated_file_approval_for_user( + user_id: str, + source_conversation_id: str, + artifact_message_id: str, + decision: str, +) -> Dict[str, Any]: + """Approve or deny one staged artifact after re-authorizing the acting user. + + Denial keeps the artifact message so the conversation still shows who declined it, but the + stored blob is removed immediately so unapproved content does not linger in storage. + """ + normalized_user_id = str(user_id or "").strip() + normalized_conversation_id = str(source_conversation_id or "").strip() + normalized_message_id = str(artifact_message_id or "").strip() + normalized_decision = str(decision or "").strip().lower() + + if not normalized_user_id or not normalized_conversation_id or not normalized_message_id: + raise ValueError("Approval target is incomplete") + if normalized_decision not in {APPROVAL_STATE_APPROVED, APPROVAL_STATE_DENIED}: + raise ValueError("Approval decision must be approved or denied") + + try: + message_item = cosmos_messages_container.read_item( + item=normalized_message_id, + partition_key=normalized_conversation_id, + ) + except CosmosResourceNotFoundError as exc: + raise LookupError("Generated file approval not found") from exc + + metadata = message_item.get("metadata") if isinstance(message_item.get("metadata"), dict) else {} + if ( + str(message_item.get("conversation_id") or "").strip() != normalized_conversation_id + or message_item.get("role") != "file" + or not metadata.get("is_generated_chat_artifact") + or not metadata.get("generated_artifact_approval_required") + ): + raise LookupError("Generated file approval not found") + + # Re-authorize against the stored approval scope on every call so a client can never + # nominate itself as the approver. + if not user_can_approve_generated_file(normalized_user_id, message_item): + raise PermissionError("You are not allowed to approve this file") + + resolver_summary = _get_current_user_summary_or_none(normalized_user_id) + updated_message = apply_generated_file_approval_decision( + message_item, + normalized_decision, + resolver=resolver_summary, + ) + + if normalized_decision == APPROVAL_STATE_DENIED: + delete_blob_backed_chat_message_files([updated_message]) + + cosmos_messages_container.upsert_item(updated_message) + _clear_generated_file_approval_notifications(normalized_message_id) + _notify_generated_file_approval_resolved(updated_message, normalized_decision, resolver_summary) + + log_event( + "[GENERATED_FILE_APPROVALS] Resolved staged artifact", + { + "conversation_id": normalized_conversation_id, + "message_id": normalized_message_id, + "decision": normalized_decision, + }, + debug_only=True, + ) + return updated_message + + +def _clear_generated_file_approval_notifications(artifact_message_id: str) -> None: + """Remove approver notifications once a staged artifact has been resolved.""" + normalized_message_id = str(artifact_message_id or "").strip() + if not normalized_message_id: + return + + try: + delete_notifications_by_metadata( + metadata_filters={"generated_artifact_message_id": normalized_message_id}, + notification_types=[GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE], + ) + except Exception as exc: + log_event( + f"[GENERATED_FILE_APPROVALS] Failed to clear approval notifications: {exc}", + level=logging.WARNING, + exceptionTraceback=True, + ) + + +def _notify_generated_file_approval_resolved( + message_item: Dict[str, Any], + decision: str, + resolver: Optional[Dict[str, Any]] = None, +) -> None: + """Tell the requester what happened to the file they generated.""" + metadata = message_item.get("metadata") if isinstance(message_item.get("metadata"), dict) else {} + requester_user_id = str(metadata.get("generated_artifact_approval_requested_by_id") or "").strip() + if not requester_user_id: + return + + collaboration_conversation_id = str( + metadata.get("generated_artifact_approval_collaboration_conversation_id") or "" + ).strip() + file_name = str(message_item.get("filename") or "").strip() or "your generated file" + resolver_name = str((resolver or {}).get("display_name") or "").strip() or "An approver" + approved = decision == APPROVAL_STATE_APPROVED + + _create_personal_notification( + user_id=requester_user_id, + notification_type=( + "generated_file_approval_approved" if approved else "generated_file_approval_denied" + ), + title="File approved" if approved else "File not approved", + message=( + f"{resolver_name} approved {file_name}. It is now available to download." + if approved + else f"{resolver_name} declined {file_name}." + ), + link_url=( + f"/chats?conversationId={collaboration_conversation_id}" + if collaboration_conversation_id + else "" + ), + link_context={ + "conversation_id": collaboration_conversation_id, + "conversation_kind": "collaborative", + }, + metadata={ + "generated_artifact_message_id": str(message_item.get("id") or "").strip(), + "source_conversation_id": str(message_item.get("conversation_id") or "").strip(), + "collaboration_conversation_id": collaboration_conversation_id, + }, + ) + + +def _collect_generated_artifact_entries(message: Dict[str, Any]) -> List[Dict[str, Any]]: + """Return the generated-artifact descriptors embedded in one serialized message.""" + metadata = message.get("metadata") if isinstance(message.get("metadata"), dict) else {} + entries = [] + for collection_key in ("generated_analysis_artifacts", "generated_tabular_outputs"): + for entry in metadata.get(collection_key) or []: + if isinstance(entry, dict) and str(entry.get("artifact_message_id") or "").strip(): + entries.append(entry) + return entries + + +def attach_generated_file_approval_state( + messages: Optional[List[Dict[str, Any]]], + viewer_user_id: str, +) -> Optional[List[Dict[str, Any]]]: + """Attach live approval state to generated-file artifacts in serialized messages. + + Approval state is read fresh rather than trusted from the stored assistant metadata so an + approver sees actionable controls and a requester sees the current decision after a reload. + """ + normalized_viewer_id = str(viewer_user_id or "").strip() + if not messages: + return messages + + entries_by_conversation: Dict[str, List[Dict[str, Any]]] = {} + for message in messages: + if not isinstance(message, dict): + continue + for entry in _collect_generated_artifact_entries(message): + entry_conversation_id = str(entry.get("conversation_id") or "").strip() + if entry_conversation_id: + entries_by_conversation.setdefault(entry_conversation_id, []).append(entry) + + if not entries_by_conversation: + return messages + + query = ( + "SELECT c.id, c.conversation_id, c.filename, c.role, c.metadata FROM c " + "WHERE c.role = 'file' " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_state)" + ) + for entry_conversation_id, entries in entries_by_conversation.items(): + try: + artifact_docs = list(cosmos_messages_container.query_items( + query=query, + partition_key=entry_conversation_id, + )) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to hydrate approval state for a conversation", + {"conversation_id": entry_conversation_id, "error": str(exc)}, + debug_only=True, + ) + continue + + approval_by_message_id = { + str(artifact_doc.get("id") or "").strip(): build_generated_file_approval_client_payload( + artifact_doc, + normalized_viewer_id, + ) + for artifact_doc in artifact_docs + } + for entry in entries: + approval_payload = approval_by_message_id.get( + str(entry.get("artifact_message_id") or "").strip() + ) + if approval_payload: + entry["approval"] = approval_payload + + return messages + + +def auto_deny_expired_generated_file_approvals() -> int: + """Auto-deny staged artifacts whose approval window elapsed and drop their blobs.""" + expired_artifacts = list_expired_pending_generated_file_artifacts() + denied_count = 0 + + for message_item in expired_artifacts: + message_id = str(message_item.get("id") or "").strip() + conversation_id = str(message_item.get("conversation_id") or "").strip() + if not message_id or not conversation_id: + continue + + try: + updated_message = apply_generated_file_approval_decision( + message_item, + APPROVAL_STATE_AUTO_DENIED, + resolver={"display_name": "Automatic expiry"}, + ) + delete_blob_backed_chat_message_files([updated_message]) + cosmos_messages_container.upsert_item(updated_message) + _clear_generated_file_approval_notifications(message_id) + _notify_generated_file_approval_resolved( + updated_message, + APPROVAL_STATE_AUTO_DENIED, + resolver={"display_name": "Automatic expiry"}, + ) + denied_count += 1 + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to auto-deny an expired staged artifact", + { + "conversation_id": conversation_id, + "message_id": message_id, + "error": str(exc), + }, + level=logging.WARNING, + exceptionTraceback=True, + ) + + if denied_count: + log_event( + "[GENERATED_FILE_APPROVALS] Auto-denied expired staged artifacts", + {"artifact_count": denied_count}, + ) + return denied_count + + def _upload_generated_chat_artifact_for_current_user( current_user_id: str, conversation_id: str, @@ -2569,8 +3012,23 @@ def _upload_generated_chat_artifact_for_current_user( except CosmosResourceNotFoundError as exc: raise LookupError(f"Conversation {conversation_id} not found") from exc - if str(conversation_item.get("user_id") or "").strip() != current_user_id: - raise PermissionError("Forbidden") + # Shared conversations are backed by a source conversation owned by their creator, so a + # participant can never satisfy a plain ownership comparison even though they are a + # legitimate member. Authorize against the linked shared conversation instead, then decide + # whether this deliverable has to be staged for approval. + access_context = build_conversation_participation_context(current_user_id, conversation_item) + + artifact_metadata = artifact_metadata if isinstance(artifact_metadata, dict) else {} + approval_metadata = {} + if requires_generated_file_approval( + access_context, + file_name=normalized_file_name, + output_format=str(artifact_metadata.get("output_format") or ""), + ): + approval_metadata = build_generated_file_approval_metadata( + access_context, + requester=_get_current_user_summary_or_none(current_user_id), + ) blob_service_client = CLIENTS.get("storage_account_office_docs_client") if not blob_service_client: @@ -2639,7 +3097,10 @@ def _upload_generated_chat_artifact_for_current_user( artifact_capability = str(artifact_metadata.get("capability") or "analysis").strip().lower() or "analysis" artifact_output_format = str(artifact_metadata.get("output_format") or file_extension).strip().lower() or file_extension artifact_summary = str(artifact_metadata.get("summary") or "").strip() - lifecycle_metadata = _build_generated_chat_artifact_lifecycle_metadata(artifact_metadata) + lifecycle_metadata = _build_generated_chat_artifact_lifecycle_metadata( + artifact_metadata, + artifact_run_user_id=current_user_id, + ) message_doc = { "id": artifact_message_id, @@ -2660,6 +3121,7 @@ def _upload_generated_chat_artifact_for_current_user( "generated_artifact_summary": artifact_summary, "generated_artifact_idempotency_key": normalized_idempotency_key or None, **lifecycle_metadata, + **approval_metadata, "thread_info": { "thread_id": current_thread_id, "previous_thread_id": previous_thread_id, @@ -2670,6 +3132,9 @@ def _upload_generated_chat_artifact_for_current_user( } cosmos_messages_container.upsert_item(message_doc) + if approval_metadata: + _notify_generated_file_approval_requested(message_doc, access_context) + log_event( "[SIMPLE_CHAT] Generated chat artifact saved", { @@ -2680,6 +3145,7 @@ def _upload_generated_chat_artifact_for_current_user( "storage_scope": "chat", "capability": artifact_capability, "output_format": artifact_output_format, + "approval_state": approval_metadata.get("generated_artifact_approval_state") or "", }, debug_only=True, ) @@ -2692,6 +3158,8 @@ def _upload_generated_chat_artifact_for_current_user( "blob_path": blob_path, "capability": artifact_capability, "output_format": artifact_output_format, + "approval_state": approval_metadata.get("generated_artifact_approval_state") or "", + "approval_required": bool(approval_metadata), **_build_generated_chat_artifact_lifecycle_response(message_doc.get("metadata")), }, "conversation_id": conversation_id, @@ -2705,7 +3173,10 @@ def _safe_positive_int(value: Any) -> int: return 0 -def _build_generated_chat_artifact_lifecycle_metadata(artifact_metadata: Dict[str, Any]) -> Dict[str, Any]: +def _build_generated_chat_artifact_lifecycle_metadata( + artifact_metadata: Dict[str, Any], + artifact_run_user_id: str = "", +) -> Dict[str, Any]: metadata = artifact_metadata if isinstance(artifact_metadata, dict) else {} run_id = str(metadata.get("artifact_run_id") or metadata.get("run_id") or "").strip() set_id = str(metadata.get("artifact_set_id") or "").strip() @@ -2727,6 +3198,12 @@ def _build_generated_chat_artifact_lifecycle_metadata(artifact_metadata: Dict[st "generated_artifact_run_id": run_id, "generated_artifact_set_id": set_id, "generated_artifact_member_id": member_id, + # Export runs are partitioned by the user who queued them. In a shared conversation the + # participant queues the run while the owner may be the one downloading, so the run + # owner is recorded here instead of being inferred from the caller. + "generated_artifact_run_user_id": str( + metadata.get("artifact_run_user_id") or artifact_run_user_id or "" + ).strip(), "generated_artifact_lifecycle_state": lifecycle_state, "generated_artifact_validation_state": validation_state, "generated_artifact_publication_generation": _safe_positive_int( @@ -2782,13 +3259,17 @@ def assert_generated_chat_artifact_is_published_for_user(current_user_id: str, m set_id = str(metadata.get("generated_artifact_set_id") or "").strip() member_id = str(metadata.get("generated_artifact_member_id") or "").strip() conversation_id = str(message_item.get("conversation_id") or "").strip() + # Older artifacts predate the recorded run owner, so fall back to the caller for them. + run_user_id = str( + metadata.get("generated_artifact_run_user_id") or current_user_id or "" + ).strip() if not run_id or not set_id or not member_id or not conversation_id: raise PermissionError("Artifact publication metadata is incomplete") try: run = cosmos_tabular_export_runs_container.read_item( item=run_id, - partition_key=str(current_user_id or "").strip(), + partition_key=run_user_id, ) except CosmosResourceNotFoundError as exc: raise PermissionError("Artifact publication run is unavailable") from exc @@ -2796,7 +3277,7 @@ def assert_generated_chat_artifact_is_published_for_user(current_user_id: str, m manifest = run.get("artifact_set_manifest") if isinstance(run.get("artifact_set_manifest"), dict) else {} if ( str(run.get("conversation_id") or "").strip() != conversation_id - or str(run.get("user_id") or "").strip() != str(current_user_id or "").strip() + or str(run.get("user_id") or "").strip() != run_user_id or str(manifest.get("set_id") or "").strip() != set_id or str(manifest.get("lifecycle_state") or "").strip().lower() != "completed" or str(manifest.get("validation_state") or "").strip().lower() != GENERATED_CHAT_ARTIFACT_VALIDATION_VALIDATED @@ -2838,8 +3319,9 @@ def commit_generated_chat_artifact_publication_for_user( item=normalized_conversation_id, partition_key=normalized_conversation_id, ) - if str(conversation_item.get("user_id") or "").strip() != current_user_id: - raise PermissionError("Forbidden") + # Shared conversations are owned by their creator, so the participant whose export run + # produced this artifact must be authorized through the linked shared conversation. + build_conversation_participation_context(current_user_id, conversation_item) message_item = cosmos_messages_container.read_item( item=normalized_message_id, partition_key=normalized_conversation_id, @@ -2883,11 +3365,21 @@ def _resolve_group_upload_target_for_current_user( if not allowed: raise PermissionError(reason) - assert_group_role( - current_user_id, - normalized_group_id, - allowed_roles=("Owner", "Admin", "DocumentManager"), - ) + try: + assert_group_role( + current_user_id, + normalized_group_id, + allowed_roles=("Owner", "Admin", "DocumentManager"), + ) + except PermissionError as exc: + # Workspace writes feed the group search index, so they cannot be staged the way chat + # deliverables are. Tell the requester who can complete it instead of failing blankly. + group_name = str(group_doc.get("name") or "this group").strip() or "this group" + raise PermissionError( + f"Saving documents to the {group_name} workspace requires the Owner, Admin, or " + "Document Manager role. Ask a document manager to add this file, or request it as " + "a downloadable file in the conversation instead." + ) from exc return normalized_group_id diff --git a/application/single_app/route_backend_chats.py b/application/single_app/route_backend_chats.py index b4224c1a0..1587cd4ad 100644 --- a/application/single_app/route_backend_chats.py +++ b/application/single_app/route_backend_chats.py @@ -186,6 +186,7 @@ merge_cited_documents_into_conversation, resolve_citation_location, ) +from functions_collaboration import build_conversation_participation_context from functions_conversation_metadata import collect_conversation_metadata, update_conversation_with_metadata from functions_conversation_unread import mark_conversation_unread from functions_image_messages import build_image_message_documents, decode_image_content @@ -3465,8 +3466,15 @@ def _create_personal_conversation(user_id, conversation_id=None): return conversation_item -def _authorize_personal_conversation_access(user_id, conversation_id): - """Load a personal conversation and ensure the caller owns it.""" +def _resolve_authorized_conversation_context(user_id, conversation_id): + """Load a conversation plus the access context that authorized the caller. + + Shared conversations are backed by a hidden source conversation owned by the shared + conversation creator, so participants can never satisfy a plain ownership comparison even + though they are legitimate members. The returned context distinguishes an owner acting in + their own conversation from a participant acting inside a shared one, which downstream + artifact writes use to decide whether an approval is required. + """ try: conversation_item = cosmos_conversations_container.read_item( item=conversation_id, @@ -3475,9 +3483,13 @@ def _authorize_personal_conversation_access(user_id, conversation_id): except CosmosResourceNotFoundError as exc: raise LookupError(f"Conversation {conversation_id} not found") from exc - if conversation_item.get('user_id') != user_id: - raise PermissionError('You can only access your own conversations') + access_context = build_conversation_participation_context(user_id, conversation_item) + return conversation_item, access_context + +def _authorize_personal_conversation_access(user_id, conversation_id): + """Load a personal conversation and ensure the caller may act in it.""" + conversation_item, _ = _resolve_authorized_conversation_context(user_id, conversation_id) return conversation_item diff --git a/application/single_app/route_backend_collaboration.py b/application/single_app/route_backend_collaboration.py index 3c706ff45..a3246d71a 100644 --- a/application/single_app/route_backend_collaboration.py +++ b/application/single_app/route_backend_collaboration.py @@ -59,6 +59,11 @@ ) from functions_notifications import mark_collaboration_message_notifications_read_for_conversation from functions_message_artifacts import make_json_serializable +from functions_simplechat_operations import ( + attach_generated_file_approval_state, + list_pending_generated_file_approvals_for_user, + resolve_generated_file_approval_for_user, +) from functions_settings import get_settings from swagger_wrapper import swagger_route, get_auth_security @@ -413,6 +418,91 @@ def _sync_collaboration_mask_metadata_to_source(message_doc): def register_route_backend_collaboration(bp): + @bp.route('/api/collaboration/file-approvals', methods=['GET']) + @swagger_route(security=get_auth_security()) + @login_required + @user_required + def list_generated_file_approvals_api(): + """List generated files staged in shared conversations that this user may release.""" + try: + _require_collaboration_feature_enabled() + current_user = _get_current_collaboration_user() + if not current_user: + return jsonify({'error': 'User not authenticated'}), 401 + + approvals = list_pending_generated_file_approvals_for_user(current_user['user_id']) + return jsonify({'approvals': approvals}) + except PermissionError as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Permission denied while listing pending file approvals: {exc}', + level=logging.WARNING, + exceptionTraceback=True, + ) + return jsonify({'error': 'Permission denied'}), 403 + except Exception as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Failed to list pending file approvals: {exc}', + level=logging.ERROR, + exceptionTraceback=True, + ) + return jsonify({'error': 'Failed to load pending file approvals'}), 500 + + @bp.route( + '/api/collaboration/file-approvals///', + methods=['POST'], + ) + @swagger_route(security=get_auth_security()) + @login_required + @user_required + def resolve_generated_file_approval_api(source_conversation_id, artifact_message_id, decision): + """Approve or deny one generated file staged in a shared conversation.""" + try: + _require_collaboration_feature_enabled() + current_user = _get_current_collaboration_user() + if not current_user: + return jsonify({'error': 'User not authenticated'}), 401 + + normalized_decision = str(decision or '').strip().lower() + if normalized_decision not in ('approve', 'deny'): + return jsonify({'error': 'Decision must be approve or deny'}), 400 + + updated_message = resolve_generated_file_approval_for_user( + user_id=current_user['user_id'], + source_conversation_id=source_conversation_id, + artifact_message_id=artifact_message_id, + decision='approved' if normalized_decision == 'approve' else 'denied', + ) + message_metadata = updated_message.get('metadata') or {} + return jsonify({ + 'artifact_message_id': updated_message.get('id'), + 'approval_state': message_metadata.get('generated_artifact_approval_state'), + 'resolved_by_name': message_metadata.get('generated_artifact_approval_resolved_by_name'), + 'resolved_at': message_metadata.get('generated_artifact_approval_resolved_at'), + }) + except LookupError: + return jsonify({'error': 'Generated file approval not found'}), 404 + except PermissionError as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Permission denied while resolving file approval: {exc}', + level=logging.WARNING, + exceptionTraceback=True, + ) + return jsonify({'error': 'Permission denied'}), 403 + except ValueError as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Invalid request while resolving file approval: {exc}', + level=logging.WARNING, + exceptionTraceback=True, + ) + return jsonify({'error': 'Invalid request'}), 400 + except Exception as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Failed to resolve file approval: {exc}', + level=logging.ERROR, + exceptionTraceback=True, + ) + return jsonify({'error': 'Failed to resolve the file approval'}), 500 + @bp.route('/api/collaboration/conversations', methods=['GET']) @swagger_route(security=get_auth_security()) @login_required @@ -1178,6 +1268,7 @@ def get_collaboration_messages_api(conversation_id): allow_pending=True, ) messages = [serialize_collaboration_message(doc) for doc in list_collaboration_messages(conversation_id)] + attach_generated_file_approval_state(messages, current_user['user_id']) return jsonify({'messages': messages}), 200 except CosmosResourceNotFoundError: return jsonify({'error': 'Collaborative conversation not found'}), 404 diff --git a/application/single_app/route_enhanced_citations.py b/application/single_app/route_enhanced_citations.py index e4e380814..a6057cee9 100644 --- a/application/single_app/route_enhanced_citations.py +++ b/application/single_app/route_enhanced_citations.py @@ -24,6 +24,8 @@ from functions_group import check_group_status_allows_operation, find_group_by_id, get_user_groups, require_active_group from functions_notifications import create_group_notification, create_notification, create_public_workspace_notification from functions_public_workspaces import check_public_workspace_status_allows_operation, get_user_visible_public_workspace_ids_from_settings, require_active_public_workspace +from functions_collaboration import build_conversation_participation_context +from functions_generated_file_approvals import assert_generated_file_approval_allows_download from functions_simplechat_operations import ( assert_generated_chat_artifact_is_published_for_user, download_blob_content, @@ -48,8 +50,9 @@ def _get_authorized_chat_artifact_message(user_id, conversation_id, message_id): except CosmosResourceNotFoundError as exc: raise LookupError('Conversation not found') from exc - if str(conversation_item.get('user_id') or '').strip() != str(user_id or '').strip(): - raise PermissionError('Forbidden') + # Shared conversations are owned by their creator, so participants fail a plain ownership + # comparison. Authorize them against the linked shared conversation instead. + build_conversation_participation_context(user_id, conversation_item) try: message_item = cosmos_messages_container.read_item( @@ -69,6 +72,9 @@ def _get_authorized_chat_artifact_message(user_id, conversation_id, message_id): if not str(message_item.get('blob_container') or '').strip() or not str(message_item.get('blob_path') or '').strip(): raise LookupError('Chat artifact content is unavailable') + # Enforced independently of the export manifest checks below so a staged artifact stays + # unreachable for every caller, including the participant who requested it. + assert_generated_file_approval_allows_download(user_id, message_item) assert_generated_chat_artifact_is_published_for_user(user_id, message_item) return message_item @@ -482,6 +488,12 @@ def get_enhanced_citation_tabular(): file_msg = items[0] file_content_source = file_msg.get('file_content_source', '') + # Generated artifacts can be served here too, so the approval gate must be enforced + # on this reader as well. The source conversation owner is not necessarily an + # approver: a plain group User can create a group shared conversation while the + # approvers are that group's Owner, Admin, and Document Manager roles. + assert_generated_file_approval_allows_download(user_id, file_msg) + if file_content_source != 'blob': return jsonify({"error": "File is not stored in blob storage"}), 400 @@ -518,6 +530,9 @@ def get_enhanced_citation_tabular(): } ) + except PermissionError as exc: + debug_print(f"Forbidden serving tabular citation: {exc}") + return jsonify({"error": "Forbidden"}), 403 except Exception as e: debug_print(f"Error serving tabular citation: {e}") return jsonify({"error": str(e)}), 500 @@ -608,7 +623,8 @@ def download_chat_artifact(): }, force_download=True, ) - except PermissionError: + except PermissionError as exc: + debug_print(f"Forbidden chat artifact download attempt: {exc}") return jsonify({"error": "Forbidden"}), 403 except LookupError as exc: return jsonify({"error": str(exc)}), 404 @@ -616,7 +632,7 @@ def download_chat_artifact(): return jsonify({"error": str(exc)}), 400 except Exception as e: debug_print(f"Error serving chat artifact download: {e}") - return jsonify({"error": str(e)}), 500 + return jsonify({"error": "An internal error has occurred"}), 500 @bp.route("/api/chat_artifacts/promote", methods=["POST"]) @swagger_route(security=get_auth_security()) diff --git a/application/single_app/route_frontend_admin_settings.py b/application/single_app/route_frontend_admin_settings.py index 610cea6d0..c81e9f65d 100644 --- a/application/single_app/route_frontend_admin_settings.py +++ b/application/single_app/route_frontend_admin_settings.py @@ -2681,6 +2681,7 @@ def is_valid_url(url): 'enable_desktop_notifications': form_data.get('enable_desktop_notifications') == 'on', 'enable_conversation_archiving': form_data.get('enable_conversation_archiving') == 'on', 'enable_thoughts': form_data.get('enable_thoughts') == 'on', + 'require_shared_conversation_file_approval': form_data.get('require_shared_conversation_file_approval') == 'on', # Search (Web Search via Azure AI Foundry agent) 'enable_web_search': enable_web_search, diff --git a/application/single_app/static/js/chat/chat-file-approvals.js b/application/single_app/static/js/chat/chat-file-approvals.js new file mode 100644 index 000000000..f335a650f --- /dev/null +++ b/application/single_app/static/js/chat/chat-file-approvals.js @@ -0,0 +1,184 @@ +// chat-file-approvals.js + +import { showToast } from "./chat-toast.js"; + +const APPROVAL_STATE_PENDING = 'pending_approval'; +const APPROVAL_STATE_APPROVED = 'approved'; +const APPROVAL_STATE_DENIED = 'denied'; +const APPROVAL_STATE_AUTO_DENIED = 'auto_denied'; + +/** + * Read the approval descriptor attached to a generated artifact, when one exists. + * Artifacts produced by a conversation owner carry no descriptor and stay ungated. + */ +export function getGeneratedFileApproval(outputMetadata) { + const approval = outputMetadata && typeof outputMetadata.approval === 'object' + ? outputMetadata.approval + : null; + if (!approval) { + return null; + } + + const state = String(approval.state || '').trim().toLowerCase(); + if (!state) { + return null; + } + + return { + state, + isPending: state === APPROVAL_STATE_PENDING, + isApproved: state === APPROVAL_STATE_APPROVED, + isDenied: state === APPROVAL_STATE_DENIED || state === APPROVAL_STATE_AUTO_DENIED, + isAutoDenied: state === APPROVAL_STATE_AUTO_DENIED, + viewerCanApprove: approval.viewer_can_approve === true, + viewerIsRequester: approval.viewer_is_requester === true, + requestedByName: String(approval.requested_by_name || '').trim(), + resolvedByName: String(approval.resolved_by_name || '').trim(), + }; +} + +/** + * Return whether the artifact content is currently unavailable to everyone. + * A staged file is withheld from the requester too, so the download control is suppressed. + */ +export function generatedFileApprovalBlocksDownload(outputMetadata) { + const approval = getGeneratedFileApproval(outputMetadata); + if (!approval) { + return false; + } + return approval.isPending || approval.isDenied; +} + +function buildApprovalMessage(approval) { + if (approval.isApproved) { + return approval.resolvedByName + ? `Approved by ${approval.resolvedByName}.` + : 'Approved.'; + } + + if (approval.isAutoDenied) { + return 'This file expired before it was approved and is no longer available.'; + } + + if (approval.isDenied) { + return approval.resolvedByName + ? `${approval.resolvedByName} declined this file, so it is not available.` + : 'This file was declined and is not available.'; + } + + if (approval.viewerCanApprove) { + return approval.requestedByName + ? `${approval.requestedByName} generated this file in a shared conversation. Approve it to make it available.` + : 'A participant generated this file in a shared conversation. Approve it to make it available.'; + } + + if (approval.viewerIsRequester) { + return 'This file is waiting for the conversation owner to approve it before it can be downloaded.'; + } + + return 'This file is waiting for approval before it can be downloaded.'; +} + +async function submitApprovalDecision(outputMetadata, decision) { + const sourceConversationId = String(outputMetadata?.conversation_id || '').trim(); + const artifactMessageId = String(outputMetadata?.artifact_message_id || '').trim(); + if (!sourceConversationId || !artifactMessageId) { + throw new Error('This file approval is missing its conversation reference.'); + } + + const response = await fetch( + `/api/collaboration/file-approvals/${encodeURIComponent(sourceConversationId)}` + + `/${encodeURIComponent(artifactMessageId)}/${encodeURIComponent(decision)}`, + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'same-origin', + }, + ); + + const payload = await response.json().catch(() => ({})); + if (!response.ok) { + throw new Error(payload.error || `Failed to ${decision} this file.`); + } + return payload; +} + +/** + * Build the approval banner rendered on a gated artifact card. + * Returns null when the artifact is approved or was never gated, so ordinary + * downloads render unchanged. + */ +export function buildGeneratedFileApprovalBlock(outputMetadata, onResolved = null) { + const approval = getGeneratedFileApproval(outputMetadata); + if (!approval || approval.isApproved) { + return null; + } + + const wrapper = document.createElement('div'); + wrapper.className = approval.isDenied + ? 'alert alert-secondary d-flex flex-column gap-2 mt-3 mb-0' + : 'alert alert-warning d-flex flex-column gap-2 mt-3 mb-0'; + wrapper.setAttribute('role', 'status'); + + const headline = document.createElement('div'); + headline.className = 'd-flex align-items-start gap-2'; + + const icon = document.createElement('i'); + icon.className = approval.isDenied ? 'bi bi-file-earmark-x' : 'bi bi-file-earmark-lock'; + icon.setAttribute('aria-hidden', 'true'); + headline.appendChild(icon); + + const messageText = document.createElement('span'); + messageText.className = 'small'; + messageText.textContent = buildApprovalMessage(approval); + headline.appendChild(messageText); + wrapper.appendChild(headline); + + if (!approval.isPending || !approval.viewerCanApprove) { + return wrapper; + } + + const actions = document.createElement('div'); + actions.className = 'd-flex flex-wrap gap-2'; + + const approveButton = document.createElement('button'); + approveButton.type = 'button'; + approveButton.className = 'btn btn-sm btn-success generated-file-approve-btn'; + approveButton.textContent = 'Approve'; + + const denyButton = document.createElement('button'); + denyButton.type = 'button'; + denyButton.className = 'btn btn-sm btn-outline-danger generated-file-deny-btn'; + denyButton.textContent = 'Deny'; + + const setBusy = isBusy => { + approveButton.disabled = isBusy; + denyButton.disabled = isBusy; + }; + + const handleDecision = async decision => { + setBusy(true); + try { + const payload = await submitApprovalDecision(outputMetadata, decision); + showToast( + decision === 'approve' ? 'File approved.' : 'File declined.', + decision === 'approve' ? 'success' : 'secondary', + ); + if (typeof onResolved === 'function') { + onResolved(decision, payload); + } + } catch (error) { + showToast(error.message || 'Failed to update this file approval.', 'danger'); + setBusy(false); + } + }; + + approveButton.addEventListener('click', () => handleDecision('approve')); + denyButton.addEventListener('click', () => handleDecision('deny')); + + actions.appendChild(approveButton); + actions.appendChild(denyButton); + wrapper.appendChild(actions); + + return wrapper; +} diff --git a/application/single_app/static/js/chat/chat-messages.js b/application/single_app/static/js/chat/chat-messages.js index 68061045d..5df9d3986 100644 --- a/application/single_app/static/js/chat/chat-messages.js +++ b/application/single_app/static/js/chat/chat-messages.js @@ -17,6 +17,10 @@ import { updateSidebarConversationTitle } from "./chat-sidebar-conversations.js" import { getActiveConversationContext, getActiveConversationScope } from "./chat-conversation-scope.js"; import { escapeHtml, isColorLight, addTargetBlankToExternalLinks, sanitizeHttpUrl } from "./chat-utils.js"; import { showToast } from "./chat-toast.js"; +import { + buildGeneratedFileApprovalBlock, + generatedFileApprovalBlocksDownload, +} from "./chat-file-approvals.js"; import { autoplayTTSIfEnabled, isTTSAutoplayEnabled, playTTS } from "./chat-tts.js"; import { saveUserSetting } from "./chat-layout.js"; import { sendMessageWithStreaming } from "./chat-streaming.js"; @@ -5429,6 +5433,27 @@ function renderReplyQuoteHtml(fullMessageObject = null) { const actions = document.createElement('div'); actions.className = 'd-flex flex-wrap gap-2 mt-3'; + // A staged file is withheld from everyone until an approver releases it, so the download + // and preview controls are replaced by the approval banner rather than left to fail. + const approvalBlock = buildGeneratedFileApprovalBlock(outputMetadata, (decision, payload) => { + if (outputMetadata && typeof outputMetadata.approval === 'object') { + outputMetadata.approval.state = payload?.approval_state + || (decision === 'approve' ? 'approved' : 'denied'); + outputMetadata.approval.viewer_can_approve = false; + outputMetadata.approval.resolved_by_name = payload?.resolved_by_name || ''; + } + const refreshedCard = createGeneratedAnalysisArtifactCard(outputMetadata); + if (refreshedCard && card.parentNode) { + card.parentNode.replaceChild(refreshedCard, card); + } + }); + if (approvalBlock) { + card.appendChild(approvalBlock); + } + if (generatedFileApprovalBlocksDownload(outputMetadata)) { + return card; + } + if (outputMetadata?.background_export) { const backgroundRunId = String(outputMetadata?.export_run_id || outputMetadata?.run_id || '').trim(); if (!backgroundRunId) { diff --git a/application/single_app/templates/admin_settings.html b/application/single_app/templates/admin_settings.html index 7bcd8410f..947f7f1ba 100644 --- a/application/single_app/templates/admin_settings.html +++ b/application/single_app/templates/admin_settings.html @@ -5016,6 +5016,27 @@
+ +
+
+ Shared Conversation File Approvals +
+

Files generated by participants in a shared conversation are saved into the conversation owner's storage. When enabled, those files are held until an approver releases them.

+
+ + + +
+
+
diff --git a/docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md b/docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md new file mode 100644 index 000000000..cb055e746 --- /dev/null +++ b/docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md @@ -0,0 +1,168 @@ +# Shared Conversation File Approvals + +## Overview + +Files that a participant asks the assistant to generate inside a shared (collaborative) +conversation are saved into the conversation owner's storage scope. This feature lets those +requests succeed by creating the file immediately and holding it in a `pending_approval` state +until an authorized approver releases it, instead of refusing the request outright. + +- **Implemented in version:** **0.260.006** +- **Depends on:** Collaborative conversations (`enable_collaborative_conversations`), generated + chat artifacts, notifications +- **Related fix:** `docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md` + +## Technical Specifications + +### Architecture + +A collaborative conversation is backed by a hidden **source conversation** +(`conversation_kind: 'collaboration_source'`) whose `user_id` is always the shared conversation +creator. Every participant streams through that source conversation, so any artifact they cause +to be written lands under the owner's conversation. + +```mermaid +flowchart TD + A[Participant asks for a CSV] --> B[Collaboration stream bridge] + B --> C[chat_stream_api on the source conversation] + C --> D{Is the caller the conversation owner?} + D -- Yes --> E[Artifact written and downloadable] + D -- No --> F{Downloadable format and approval enabled?} + F -- No --> E + F -- Yes --> G[Artifact written as pending_approval] + G --> H[Approvers notified] + H --> I{Decision} + I -- Approve --> J[Artifact released for download] + I -- Deny --> K[Blob deleted, decision recorded] + I -- No action for 3 days --> L[Auto-denied, blob deleted] +``` + +### Approval scope + +Only downloadable deliverables are gated. Generated images and charts are inline conversation +rendering and are never gated. + +| Gated | Not gated | +|-------|-----------| +| `csv`, `xlsx`, `xls`, `xlsm`, `docx`, `pdf`, `json`, `xml` | images, charts, plain assistant text | + +### Approvers + +| Conversation type | Who can approve | +|-------------------|-----------------| +| Personal shared conversation | The conversation owner | +| Group shared conversation | Any group `Owner`, `Admin`, or `DocumentManager` | + +The requester can never approve their own file — the requester check is applied before the +scope branch, so a group `Admin` or `DocumentManager` who is only a participant still needs a +different approver. A staged file is not downloadable by anyone, including the requester, until +it is released. + +Every route that streams a stored artifact blob enforces the gate, not just the generated +artifact download: `/api/chat_artifacts/download`, `/api/chat_artifacts/promote`, and +`/api/enhanced_citations/tabular` all call +`assert_generated_file_approval_allows_download` before reading blob content. This matters +because the source conversation owner is not necessarily an approver — a plain group `User` can +create a group shared conversation while approval belongs to that group's document roles. + +### Approval states + +`pending_approval` -> `approved` | `denied` | `auto_denied` + +State is stored on the artifact message `metadata` under +`generated_artifact_approval_*` fields, so it travels with the artifact and cannot be bypassed +by calling the download route directly. A decision is single-use; re-applying one raises a +`ValueError`. + +### Configuration + +| Setting | Default | Description | +|---------|---------|-------------| +| `require_shared_conversation_file_approval` | `True` | When disabled, participant-generated files are saved and downloadable immediately. | + +The toggle is exposed in **Admin Settings -> Shared Conversation File Approvals**. It contains +no sensitive terms, so it passes through `sanitize_settings_for_user()` and is readable by the +chat UI. + +### API endpoints + +| Method | Route | Purpose | +|--------|-------|---------| +| `GET` | `/api/collaboration/file-approvals` | List staged files the caller may release | +| `POST` | `/api/collaboration/file-approvals///approve` | Approve one staged file | +| `POST` | `/api/collaboration/file-approvals///deny` | Deny one staged file | + +Every route carries `@swagger_route(security=get_auth_security())`, `@login_required`, and +`@user_required`. The approver is re-authorized from the stored approval scope on every call, +so a client can never nominate itself as the approver. The listing endpoint narrows candidates +to the caller's own approval scopes inside the query, so the row cap cannot truncate another +user's items ahead of the caller's. + +### File structure + +| File | Purpose | +|------|---------| +| `functions_generated_file_approvals.py` | Approval states, gating decision, approver resolution, client payloads, expiry query | +| `functions_simplechat_operations.py` | Staging on artifact write, approval resolution, notifications, auto-deny sweep | +| `functions_collaboration.py` | `build_conversation_participation_context` shared authorization helper | +| `route_backend_collaboration.py` | Approval list and decision endpoints | +| `route_enhanced_citations.py` | Download-time approval enforcement | +| `static/js/chat/chat-file-approvals.js` | Inline approve/deny card and pending state | +| `background_tasks.py` | 3-day auto-deny sweep | + +### Expiry + +Staged files auto-deny after **3 days**, matching `functions_approvals.TTL_AUTO_DENY_DAYS`. The +sweep runs on the existing approval expiration loop and deletes the stored blob so unapproved +content does not linger in storage. + +## Usage Instructions + +### Enabling + +The feature is on by default. To disable it, clear **Require approval for participant-generated +files** in Admin Settings. + +### Participant workflow + +1. A participant asks the assistant for a file in a shared conversation. +2. The assistant answers normally and the file is created. +3. The artifact card shows *"This file is waiting for the conversation owner to approve it."* + No download button is offered. +4. When approved, the participant receives a notification and the file becomes downloadable. + +### Approver workflow + +1. A notification appears in the bell: *"File approval requested."* +2. Opening the conversation shows an inline card with **Approve** and **Deny** buttons on the + pending artifact. +3. Approving releases the file for everyone in the conversation. Denying deletes the stored file + and records who declined it. + +### Group workspace documents + +Saving a generated **document into a group workspace** is a different operation: it feeds the +group search index and still requires the `Owner`, `Admin`, or `DocumentManager` role. Users +without that role now receive an actionable message naming who can complete the request rather +than a bare permission error. Requesting the same content as a downloadable file in the +conversation goes through the approval flow instead. + +## Testing and Validation + +- `functional_tests/test_shared_conversation_file_approval_fix.py` (16 checks) covers format + scoping, owner bypass, the admin toggle, staged download refusal for all callers, personal and + group approver resolution, the requester self-approval guard across every group role, approval + enforcement on every artifact blob reader, scoped approval listing, single-use decisions, and + the wiring of each authorization gate. +- `functional_tests/route_tests/` confirms the new routes carry the required security + decorators and unauthenticated-access policy. + +### Known limitations + +- Workspace document writes are not staged, only chat deliverables. Holding a workspace document + back would require withholding search indexing, which is tracked as follow-up work. +- Participants can already upload files into a shared conversation without approval, so the + policy is intentionally asymmetric between uploading and generating. +- While a background export is still running, the live status poll may briefly offer a download + control. The download itself is refused with *"This file is waiting for owner approval"* and + the card corrects itself on reload. diff --git a/docs/explanation/features/index.md b/docs/explanation/features/index.md index 93ae1e6d8..03353656b 100644 --- a/docs/explanation/features/index.md +++ b/docs/explanation/features/index.md @@ -41,6 +41,10 @@ category: Version History - [Outlook MSG File Ingestion](v0.242.063/MSG_FILE_INGESTION.md) - [Chat Upload Personal Workspace Handoff](CHAT_UPLOAD_PERSONAL_WORKSPACE_HANDOFF.md) +## Collaborative Conversation Features + +- [Shared Conversation File Approvals](SHARED_CONVERSATION_FILE_APPROVALS.md) + ## Workspace Branding Features - [Group And Public Workspace Custom Hero Colors](GROUP_PUBLIC_WORKSPACE_CUSTOM_HERO_COLORS.md) diff --git a/docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md b/docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md new file mode 100644 index 000000000..4bca7e9fa --- /dev/null +++ b/docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md @@ -0,0 +1,158 @@ +# Shared Conversation File Generation "Forbidden" Fix + +## Issue + +In a shared (collaborative) conversation, a user who was invited into the conversation received: + +``` +Stream interrupted before any content was received. +Stream interrupted: Forbidden +``` + +The reported case was a participant asking `@Telemetry generate a csv of the 900 samples` in a +conversation shared from a personal chat. No content was returned at all. + +- **Fixed in version:** **0.260.006** +- **Related feature:** `docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md` + +## Root Cause Analysis + +A collaborative conversation is backed by a hidden **source conversation** +(`conversation_kind: 'collaboration_source'`, created in +`functions_collaboration.ensure_collaboration_source_conversation`) whose `user_id` is always the +shared conversation creator. + +`/api/collaboration/conversations//stream` bridges into the internal `chat_stream_api` view +using that source conversation id, but keeps the **requesting participant's** session. Every +downstream owner-equality comparison therefore failed for participants. + +Four separate gates were involved: + +| # | Location | Check | Effect | +|---|----------|-------|--------| +| 1 | `route_backend_chats._authorize_personal_conversation_access`, called by `chat_stream_api` | `conversation_item.get('user_id') != user_id` | Returned `{'error': 'Forbidden'}, 403`. The collaboration bridge converts any `>= 400` response into a stream error, producing the reported banner. This blocked **all** AI invocation by participants, not just file generation. | +| 2 | `functions_simplechat_operations._upload_generated_chat_artifact_for_current_user` | `conversation.user_id != current_user_id` | Raised `PermissionError("Forbidden")` when persisting the artifact. `maybe_create_generated_file_output` swallowed it, so the file silently disappeared. | +| 3 | `functions_simplechat_operations._resolve_group_upload_target_for_current_user` | `assert_group_role(Owner/Admin/DocumentManager)` | Plain group `User` members could not save generated documents into a group workspace. | +| 4 | `route_enhanced_citations._get_authorized_chat_artifact_message` | `conversation.user_id != user_id` | Participants could not **download** a generated artifact even once it existed. | + +Gate 1 explains why the failure looked file-specific: collaborative conversations default to +`ai_invocation_mode: 'explicit_only'`, so ordinary participant messages never reach the stream +bridge. The first explicit AI invocation was the first time the gate was hit. + +Two secondary defects were found while fixing this: + +- `assert_generated_chat_artifact_is_published_for_user` read the export run using the + **caller's** id as the partition key. Background exports are queued by the participant while + the owner may be the one downloading, so an approved large CSV would have been unreadable. +- `commit_generated_chat_artifact_publication_for_user` and + `delete_generated_chat_artifact_for_user` carried the same owner-only comparison, which would + have broken publication and rollback for participant-queued background exports. 900 rows + exceeds the 500-row inline threshold, so the reported case used exactly this path. + +Three further defects were caught in review of the approval gate itself and fixed before +merge: + +- **Requester self-approval.** `resolve_generated_file_approver_role` resolved group-scope + approvers purely by group role, so a group `Admin` or `DocumentManager` who was only a + participant could stage a file and immediately approve it themselves. The requester check now + runs before the scope branch. +- **Bypass via `/api/enhanced_citations/tabular`.** That route streams any blob-backed file + message after a single conversation-ownership check and never consulted the approval gate. A + plain group `User` who created a group shared conversation — explicitly not an approver — could + fetch a staged CSV or XLSX directly. The gate now runs there before the blob is read, and the + route returns `403` rather than `500` for a withheld file. +- **Truncation before authorization.** `list_pending_generated_file_approvals_for_user` applied + `TOP @limit` across the whole messages container and filtered by approver afterwards in + Python, so a tenant with more than 50 pending files could return an empty list to an approver + who genuinely had items waiting. Candidates are now narrowed to the caller's own approval + scopes inside the query. + +## Technical Details + +### Files modified + +| File | Change | +|------|--------| +| `collaboration_models.py` | Added `COLLABORATION_SOURCE_KIND` and replaced the string literals | +| `functions_collaboration.py` | Added `is_collaboration_source_conversation`, `get_collaboration_conversation_for_source`, and `build_conversation_participation_context` | +| `route_backend_chats.py` | `_authorize_personal_conversation_access` now delegates to the collaboration-aware `_resolve_authorized_conversation_context` | +| `functions_simplechat_operations.py` | Artifact upload, publication commit, and rollback authorize through the participation context; artifact writes by participants are staged; run owner recorded for publication checks | +| `route_enhanced_citations.py` | Download authorizes participants and enforces the approval gate before the export manifest check | +| `functions_generated_file_approvals.py` | New module holding approval state and gating logic | +| `functions_notifications.py` | Added the three generated-file approval notification types | +| `functions_settings.py` | Added `require_shared_conversation_file_approval`, default `True` | +| `background_tasks.py` | Auto-deny sweep for expired staged files | +| `static/js/chat/chat-file-approvals.js` | New inline approval card | +| `static/js/chat/chat-messages.js` | Renders the approval card and suppresses downloads while staged | + +### Key change + +All five call sites of `_authorize_personal_conversation_access` now resolve access through a +single shared helper, which mirrors the pattern already used by chat file uploads in +`route_frontend_chats._resolve_chat_upload_context`: + +```python +def build_conversation_participation_context(user_id, conversation_item): + # Owners keep their existing behavior. + if normalized_user_id and owner_user_id == normalized_user_id: + return {..., 'is_owner': True} + + # Participants are authorized against the linked shared conversation instead. + collaboration_conversation = get_collaboration_conversation_for_source(normalized_item) + if not collaboration_conversation: + raise PermissionError('You can only access your own conversations') + + collaboration_access = assert_user_can_participate_in_collaboration_conversation( + normalized_user_id, + collaboration_conversation, + ) + return {..., 'is_owner': False} +``` + +Ordinary personal conversations with no collaboration link remain strictly owner-only. + +### Testing approach + +`functional_tests/test_shared_conversation_file_approval_fix.py` loads the approval module +against dependency stubs, because `config.py` connects to Cosmos DB at import time. The +authorization wiring for each gate is asserted against the real source using AST extraction, the +same technique used by `test_broken_access_control_findings_fix.py`. + +Two existing tests that extract these functions via AST were updated to supply the new +dependency: `test_generated_artifact_lifecycle_authorization.py` and +`test_tabular_row_orchestration_scale.py`. + +### Impact analysis + +- Participants can now invoke the AI in shared conversations at all — the primary regression. +- Owners see no behavior change: `is_owner` short-circuits before any approval logic. +- Non-shared personal conversations are unchanged and remain owner-only. +- Downloads gain one extra check that runs before the existing publication assertion, so a + staged file can never be retrieved. + +## Validation + +| Test | Result | +|------|--------| +| `functional_tests/test_shared_conversation_file_approval_fix.py` | 16/16 passed | +| `functional_tests/test_generated_artifact_lifecycle_authorization.py` | 6/6 passed | +| `functional_tests/test_tabular_row_orchestration_scale.py` | passed | +| `functional_tests/test_assistant_table_csv_artifact.py` | 35/35 passed | +| `functional_tests/test_generated_json_xml_exports.py` | 7/7 passed | +| `functional_tests/route_tests/` | 12/12 passed across three suites | + +Pre-existing failures unrelated to this change and confirmed against a clean baseline: +`test_mixed_source_hardening.py` (one assertion), `test_tabular_generated_output_exports.py` +(stale exact-version assertions pinned to `0.241.144`), and any test requiring live Azure +credentials. + +### Before and after + +| Scenario | Before | After | +|----------|--------|-------| +| Participant asks the AI anything in a shared conversation | `Stream interrupted: Forbidden`, no content | Normal response | +| Participant asks for a CSV | Forbidden, or artifact silently dropped | File created, held for approval, approver notified | +| Owner approves | Not possible | File becomes downloadable for the conversation | +| Owner denies | Not possible | Stored file deleted, decision recorded in the card | +| Nobody responds | Not possible | Auto-denied after 3 days, blob deleted | +| Owner generates their own file | Worked | Unchanged | diff --git a/docs/explanation/fixes/index.md b/docs/explanation/fixes/index.md index c380093cd..d81b36e2a 100644 --- a/docs/explanation/fixes/index.md +++ b/docs/explanation/fixes/index.md @@ -14,6 +14,7 @@ category: Version History - [CosmosClient Import Binding CodeQL Fix](COSMOSCLIENT_IMPORT_BINDING_CODEQL_FIX.md) - [Log Credential Key Redaction Fix](LOG_CREDENTIAL_KEY_REDACTION_FIX.md) - [Conversation Cache Invalidation Authorization Fix](CONVERSATION_CACHE_INVALIDATION_AUTHORIZATION_FIX.md) +- [Shared Conversation File Generation Forbidden Fix](SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md) - [Chat Completion Background Unread Guard Fix](CHAT_COMPLETION_BACKGROUND_UNREAD_GUARD_FIX.md) - [Settings Container RU Write Suppression Fix](SETTINGS_CONTAINER_RU_WRITE_SUPPRESSION_FIX.md) - [Tabular SK Python 3.13 Kernel Parameter Fix](v0.242.068/TABULAR_SK_PY313_KERNEL_PARAMETER_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 07bd188b8..4ebb51b9e 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,36 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.260.006)** + +#### New Features + +* **Shared Conversation File Approvals** + * Files generated by a participant in a shared conversation are now created immediately and held in a **pending approval** state instead of being refused, because they are saved into the conversation owner's storage. + * The conversation owner approves personal shared conversations; any group **Owner**, **Admin**, or **Document Manager** approves group shared conversations. Requesters can never approve their own file. + * Approvers get an inline **Approve / Deny** card on the pending file plus a notification. Approving releases the file, denying deletes the stored file and records who declined it. + * A staged file is not downloadable by anyone, including the requester, until it is released. + * Only downloadable deliverables are gated (CSV, XLSX, DOCX, PDF, JSON, XML). Generated images and charts are never gated. + * Unapproved files are automatically declined and deleted after 3 days, matching the existing Control Center approval window. + * New Admin Settings toggle **Require approval for participant-generated files**, enabled by default. + * (Ref: `functions_generated_file_approvals.py`, `chat-file-approvals.js`, `require_shared_conversation_file_approval`, `/api/collaboration/file-approvals`) + +#### Bug Fixes + +* **Shared Conversations No Longer Fail With "Stream interrupted: Forbidden"** + * Fixed invited participants being unable to invoke the AI at all in a shared conversation. Any explicit AI request returned `Forbidden` with no content. + * Root cause was the hidden source conversation behind every shared conversation being owned by its creator, so participants failed a plain ownership comparison in the chat streaming route even though they are legitimate members. + * Because shared conversations only call the AI on an explicit mention, this surfaced the first time a participant asked the assistant for something, which made it look file-specific. + * Participants can also now download generated files from a shared conversation, which was blocked by the same comparison. + * Also fixed background CSV exports queued by a participant becoming unreadable for the owner, because publication checks looked up the export run under the wrong user partition. + * (Ref: `build_conversation_participation_context`, `route_backend_chats.py`, `route_enhanced_citations.py`, `functions_simplechat_operations.py`) + +#### User Interface Enhancements + +* **Clearer Group Workspace Save Errors** + * Attempting to save a generated document into a group workspace without document rights now names the roles that can complete it and suggests requesting the content as a downloadable file instead of failing with a bare permission error. + * (Ref: `_resolve_group_upload_target_for_current_user`) + ### **(v0.260.005)** #### User Interface Enhancements diff --git a/functional_tests/test_generated_artifact_lifecycle_authorization.py b/functional_tests/test_generated_artifact_lifecycle_authorization.py index feda366c3..5d1f9680b 100644 --- a/functional_tests/test_generated_artifact_lifecycle_authorization.py +++ b/functional_tests/test_generated_artifact_lifecycle_authorization.py @@ -47,6 +47,20 @@ def upsert_item(self, body): return body +def _fake_participation_context(user_id, conversation_item): + """Stand in for the collaboration-aware authorization used by artifact helpers.""" + owner_user_id = str((conversation_item or {}).get("user_id") or "").strip() + if owner_user_id and owner_user_id != str(user_id or "").strip(): + raise PermissionError("You can only access your own conversations") + return { + "user_id": user_id, + "owner_user_id": owner_user_id, + "is_owner": True, + "collaboration_conversation_id": "", + "group_id": "", + } + + def load_operation_helpers(conversation_item, message_item, run_item=None): source = OPERATIONS_FILE.read_text(encoding="utf-8") tree = ast.parse(source, filename=str(OPERATIONS_FILE)) @@ -73,6 +87,9 @@ def load_operation_helpers(conversation_item, message_item, run_item=None): "datetime": datetime, "timezone": timezone, "CosmosResourceNotFoundError": FakeNotFound, + # Shared conversations authorize through the participation context, which owner-only + # fixtures satisfy without any collaboration linkage. + "build_conversation_participation_context": _fake_participation_context, "cosmos_conversations_container": FakeContainer({"conversation-1": conversation_item}), "cosmos_messages_container": FakeContainer({"message-1": message_item}), "cosmos_tabular_export_runs_container": FakeContainer({"run-1": run_item} if run_item else {}), @@ -83,7 +100,7 @@ def load_operation_helpers(conversation_item, message_item, run_item=None): return namespace -def load_route_helper(message_item, publication_assertion): +def load_route_helper(message_item, publication_assertion, approval_assertion=None): source = ROUTE_FILE.read_text(encoding="utf-8") tree = ast.parse(source, filename=str(ROUTE_FILE)) helper = next( @@ -94,6 +111,8 @@ def load_route_helper(message_item, publication_assertion): "CosmosResourceNotFoundError": FakeNotFound, "cosmos_conversations_container": FakeContainer({"conversation-1": {"user_id": "user-1"}}), "cosmos_messages_container": FakeContainer({"message-1": message_item}), + "build_conversation_participation_context": _fake_participation_context, + "assert_generated_file_approval_allows_download": approval_assertion or (lambda user_id, message_item: None), "assert_generated_chat_artifact_is_published_for_user": publication_assertion, } module = ast.Module(body=[helper], type_ignores=[]) diff --git a/functional_tests/test_shared_conversation_file_approval_fix.py b/functional_tests/test_shared_conversation_file_approval_fix.py new file mode 100644 index 000000000..24744cee6 --- /dev/null +++ b/functional_tests/test_shared_conversation_file_approval_fix.py @@ -0,0 +1,688 @@ +#!/usr/bin/env python3 +# test_shared_conversation_file_approval_fix.py +""" +Functional test for shared conversation file generation and owner approvals. +Version: 0.260.006 +Implemented in: 0.260.006 + +This test ensures that a non-owner participant of a shared (collaborative) conversation can +invoke the AI without a "Forbidden" stream interruption, that downloadable files they generate +are staged for approval instead of failing, and that a staged file stays unreachable until an +authorized approver releases it. + +The application package connects to Cosmos DB at import time, so the approval module is loaded +against lightweight dependency stubs. Wiring into the routes that own each authorization gate +is verified against the real source. +""" + +import ast +import importlib.util +import os +import sys +import types + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +APP_ROOT = os.path.join(REPO_ROOT, 'application', 'single_app') +TESTS_ROOT = os.path.dirname(os.path.abspath(__file__)) +if TESTS_ROOT not in sys.path: + sys.path.insert(0, TESTS_ROOT) + +from test_support.versioning import assert_app_version_at_least + + +OWNER_USER_ID = 'owner-user-001' +PARTICIPANT_USER_ID = 'participant-user-002' +OUTSIDER_USER_ID = 'outsider-user-003' +GROUP_ADMIN_USER_ID = 'group-admin-004' +GROUP_DOC_MANAGER_USER_ID = 'group-docmgr-005' +COLLABORATION_CONVERSATION_ID = 'collaboration-conversation-001' +GROUP_ID = 'group-001' + +GROUP_DOC = { + 'id': GROUP_ID, + 'owner': {'id': OWNER_USER_ID}, + 'admins': [GROUP_ADMIN_USER_ID], + 'documentManagers': [GROUP_DOC_MANAGER_USER_ID], + 'users': [ + {'userId': PARTICIPANT_USER_ID}, + {'userId': GROUP_ADMIN_USER_ID}, + {'userId': GROUP_DOC_MANAGER_USER_ID}, + {'userId': OWNER_USER_ID}, + ], +} + + +def read_app_source(file_name): + with open(os.path.join(APP_ROOT, file_name), 'r', encoding='utf-8') as source_file: + return source_file.read() + + +def extract_function_source(source_text, function_name): + parsed = ast.parse(source_text) + for node in ast.walk(parsed): + if isinstance(node, ast.FunctionDef) and node.name == function_name: + return ast.get_source_segment(source_text, node) + raise AssertionError(f'Function {function_name} not found') + + +def _stub_get_user_role_in_group(group_doc, user_id): + """Mirror functions_group.get_user_role_in_group without importing the Azure-backed module.""" + if not group_doc: + return None + if group_doc.get('owner', {}).get('id') == user_id: + return 'Owner' + if user_id in group_doc.get('admins', []): + return 'Admin' + if user_id in group_doc.get('documentManagers', []): + return 'DocumentManager' + for member in group_doc.get('users', []): + if member.get('userId') == user_id: + return 'User' + return None + + +def load_approvals_module(settings=None): + """Load functions_generated_file_approvals against dependency stubs.""" + resolved_settings = settings or {'require_shared_conversation_file_approval': True} + + config_stub = types.ModuleType('config') + config_stub.cosmos_messages_container = None + sys.modules['config'] = config_stub + + appinsights_stub = types.ModuleType('functions_appinsights') + appinsights_stub.log_event = lambda *args, **kwargs: None + sys.modules['functions_appinsights'] = appinsights_stub + + group_stub = types.ModuleType('functions_group') + group_stub.find_group_by_id = lambda group_id: GROUP_DOC if group_id == GROUP_ID else None + group_stub.get_user_role_in_group = _stub_get_user_role_in_group + sys.modules['functions_group'] = group_stub + + settings_stub = types.ModuleType('functions_settings') + settings_stub.get_settings = lambda: dict(resolved_settings) + sys.modules['functions_settings'] = settings_stub + + module_path = os.path.join(APP_ROOT, 'functions_generated_file_approvals.py') + spec = importlib.util.spec_from_file_location('functions_generated_file_approvals', module_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def build_participant_context(group_id=''): + return { + 'is_owner': False, + 'user_id': PARTICIPANT_USER_ID, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': group_id, + } + + +def test_participation_context_allows_shared_conversation_participants(): + """Gate 1: participants resolve access through the linked shared conversation.""" + print('Testing shared conversation participation authorization...') + source = read_app_source('functions_collaboration.py') + function_source = extract_function_source(source, 'build_conversation_participation_context') + + assert 'assert_user_can_participate_in_collaboration_conversation' in function_source, ( + 'Participants must be authorized against the linked collaboration conversation' + ) + assert 'get_collaboration_conversation_for_source' in function_source, ( + 'The helper must resolve the shared conversation from the source conversation' + ) + assert "raise PermissionError('You can only access your own conversations')" in function_source, ( + 'Unlinked personal conversations must stay owner-only' + ) + assert "'is_owner': True" in function_source and "'is_owner': False" in function_source, ( + 'The context must distinguish owners from participants for staging decisions' + ) + + print('Shared conversation participation authorization verified.') + return True + + +def test_stream_authorization_no_longer_rejects_participants(): + """Gate 1 regression: chat_stream_api must not hard-compare conversation ownership.""" + print('Testing chat stream authorization source...') + source = read_app_source('route_backend_chats.py') + + authorize_source = extract_function_source(source, '_authorize_personal_conversation_access') + assert "conversation_item.get('user_id') != user_id" not in authorize_source, ( + 'The owner-only equality check must no longer gate conversation access' + ) + assert '_resolve_authorized_conversation_context' in authorize_source, ( + 'Authorization must flow through the collaboration-aware resolver' + ) + + resolver_source = extract_function_source(source, '_resolve_authorized_conversation_context') + assert 'build_conversation_participation_context' in resolver_source, ( + 'The resolver must use the shared participation context' + ) + assert 'from functions_collaboration import build_conversation_participation_context' in source, ( + 'route_backend_chats must import the shared participation context helper' + ) + + print('Chat stream authorization verified.') + return True + + +def test_only_gated_formats_require_approval(): + """Only downloadable deliverables are gated; images and charts stay ungated.""" + print('Testing approval format scope...') + approvals = load_approvals_module() + + for gated_format in ('csv', 'xlsx', 'xls', 'xlsm', 'docx', 'pdf', 'json', 'xml'): + assert approvals.is_approval_gated_file(f'report.{gated_format}'), ( + f'{gated_format} should require approval' + ) + + for ungated_format in ('png', 'jpg', 'gif', 'svg', 'webp', 'txt', 'md'): + assert not approvals.is_approval_gated_file(f'chart.{ungated_format}'), ( + f'{ungated_format} should not require approval' + ) + + assert approvals.is_approval_gated_file('', output_format='csv'), ( + 'An explicit output format should be honored when no file name is present' + ) + + print('Approval format scope verified.') + return True + + +def test_owner_writes_are_never_gated(): + """Owners bypass the gate, participants are staged, and the admin setting disables it.""" + print('Testing owner bypass and setting toggle...') + approvals = load_approvals_module() + + owner_context = { + 'is_owner': True, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': '', + } + enabled_settings = {'require_shared_conversation_file_approval': True} + disabled_settings = {'require_shared_conversation_file_approval': False} + + assert not approvals.requires_generated_file_approval( + owner_context, file_name='report.csv', settings=enabled_settings, + ), 'Owners must never be gated' + + assert approvals.requires_generated_file_approval( + build_participant_context(), file_name='report.csv', settings=enabled_settings, + ), 'Participants generating downloadable files must be gated' + + assert not approvals.requires_generated_file_approval( + build_participant_context(), file_name='report.csv', settings=disabled_settings, + ), 'Disabling the admin setting must restore direct behavior' + + assert not approvals.requires_generated_file_approval( + build_participant_context(), file_name='diagram.png', settings=enabled_settings, + ), 'Inline image artifacts must stay ungated' + + solo_context = { + 'is_owner': False, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': '', + 'group_id': '', + } + assert not approvals.requires_generated_file_approval( + solo_context, file_name='report.csv', settings=enabled_settings, + ), 'Non-shared conversations must not create approvals' + + print('Owner bypass and setting toggle verified.') + return True + + +def test_staged_artifact_is_not_downloadable_until_approved(): + """A staged artifact must be unreachable for every caller, including the requester.""" + print('Testing staged artifact download enforcement...') + approvals = load_approvals_module() + + approval_metadata = approvals.build_generated_file_approval_metadata( + build_participant_context(), + requester={'user_id': PARTICIPANT_USER_ID, 'display_name': 'Participant'}, + ) + assert approval_metadata['generated_artifact_approval_state'] == approvals.APPROVAL_STATE_PENDING + assert approval_metadata['generated_artifact_approval_expires_at'], 'Staged files must expire' + assert approval_metadata['generated_artifact_approval_scope'] == approvals.APPROVAL_SCOPE_PERSONAL + + staged_message = {'id': 'artifact-1', 'metadata': dict(approval_metadata)} + for caller_id in (PARTICIPANT_USER_ID, OWNER_USER_ID, OUTSIDER_USER_ID): + try: + approvals.assert_generated_file_approval_allows_download(caller_id, staged_message) + raise AssertionError('Pending artifacts must never be downloadable') + except PermissionError: + pass + + denied_message = {'id': 'artifact-1', 'metadata': dict(approval_metadata)} + denied_message['metadata']['generated_artifact_approval_state'] = approvals.APPROVAL_STATE_DENIED + try: + approvals.assert_generated_file_approval_allows_download(OWNER_USER_ID, denied_message) + raise AssertionError('Denied artifacts must not be downloadable') + except PermissionError: + pass + + approved_message = {'id': 'artifact-1', 'metadata': dict(approval_metadata)} + approved_message['metadata']['generated_artifact_approval_state'] = approvals.APPROVAL_STATE_APPROVED + approvals.assert_generated_file_approval_allows_download(OWNER_USER_ID, approved_message) + approvals.assert_generated_file_approval_allows_download(PARTICIPANT_USER_ID, approved_message) + + # Artifacts written by an owner carry no approval contract and stay downloadable. + approvals.assert_generated_file_approval_allows_download(OWNER_USER_ID, {'id': 'a2', 'metadata': {}}) + + print('Staged artifact download enforcement verified.') + return True + + +def test_personal_and_group_approver_resolution(): + """Personal approvals route to the owner; group approvals route to document managers.""" + print('Testing approver resolution...') + approvals = load_approvals_module() + + personal_message = { + 'id': 'artifact-1', + 'metadata': approvals.build_generated_file_approval_metadata( + build_participant_context(), + requester={'user_id': PARTICIPANT_USER_ID}, + ), + } + assert approvals.user_can_approve_generated_file(OWNER_USER_ID, personal_message) + assert not approvals.user_can_approve_generated_file(PARTICIPANT_USER_ID, personal_message), ( + 'A requester must not approve their own file' + ) + assert not approvals.user_can_approve_generated_file(OUTSIDER_USER_ID, personal_message) + + group_metadata = approvals.build_generated_file_approval_metadata( + build_participant_context(group_id=GROUP_ID), + requester={'user_id': PARTICIPANT_USER_ID}, + ) + assert group_metadata['generated_artifact_approval_scope'] == approvals.APPROVAL_SCOPE_GROUP + group_message = {'id': 'artifact-2', 'metadata': group_metadata} + + assert approvals.user_can_approve_generated_file(OWNER_USER_ID, group_message), 'Group owner approves' + assert approvals.user_can_approve_generated_file(GROUP_ADMIN_USER_ID, group_message), 'Group admin approves' + assert approvals.user_can_approve_generated_file(GROUP_DOC_MANAGER_USER_ID, group_message), ( + 'Group document manager approves' + ) + assert not approvals.user_can_approve_generated_file(PARTICIPANT_USER_ID, group_message), ( + 'A plain group User must not approve their own file' + ) + assert not approvals.user_can_approve_generated_file(OUTSIDER_USER_ID, group_message), ( + 'Non-members must not approve' + ) + + print('Approver resolution verified.') + return True + + +def test_requester_can_never_approve_their_own_file(): + """A requester is never their own approver, whatever group role they hold.""" + print('Testing requester self-approval guard...') + approvals = load_approvals_module() + + # A group Admin or DocumentManager who is only a participant still gets staged, so the + # requester check must win over their group role. + for privileged_requester in (GROUP_ADMIN_USER_ID, GROUP_DOC_MANAGER_USER_ID, OWNER_USER_ID): + metadata = approvals.build_generated_file_approval_metadata( + { + 'is_owner': False, + 'user_id': privileged_requester, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': GROUP_ID, + }, + requester={'user_id': privileged_requester}, + ) + message_item = {'id': 'artifact-self', 'metadata': metadata} + assert not approvals.user_can_approve_generated_file(privileged_requester, message_item), ( + f'{privileged_requester} must not approve a file they requested' + ) + payload = approvals.build_generated_file_approval_client_payload( + message_item, privileged_requester, + ) + assert payload['viewer_can_approve'] is False, ( + 'The requester must not be offered approve controls' + ) + assert payload['viewer_is_requester'] is True + + # Another eligible approver must still be able to release it. + assert approvals.user_can_approve_generated_file( + GROUP_ADMIN_USER_ID if privileged_requester != GROUP_ADMIN_USER_ID else GROUP_DOC_MANAGER_USER_ID, + message_item, + ), 'A different eligible approver must still be able to release the file' + + # The same guard applies in a personal shared conversation. + personal_metadata = approvals.build_generated_file_approval_metadata( + { + 'is_owner': False, + 'user_id': OWNER_USER_ID, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': '', + }, + requester={'user_id': OWNER_USER_ID}, + ) + assert not approvals.user_can_approve_generated_file( + OWNER_USER_ID, {'id': 'artifact-self-personal', 'metadata': personal_metadata}, + ), 'An owner-requester must not self-approve' + + print('Requester self-approval guard verified.') + return True + + +def test_all_artifact_blob_readers_enforce_the_approval_gate(): + """Every route that streams a stored artifact blob must consult the approval gate.""" + print('Testing artifact blob reader coverage...') + source = read_app_source('route_enhanced_citations.py') + + # The tabular citation route serves arbitrary blob-backed file messages from a conversation, + # and the source conversation owner is not necessarily an approver in a group shared + # conversation, so it must enforce the gate too. + enforcement_count = source.count('assert_generated_file_approval_allows_download(user_id, ') + assert enforcement_count >= 2, ( + 'Both the generated-artifact reader and the tabular citation reader must enforce ' + f'the approval gate (found {enforcement_count})' + ) + + tabular_index = source.index('def get_enhanced_citation_tabular') + tabular_source = source[tabular_index:source.index('@bp.route', tabular_index + 1)] + assert 'assert_generated_file_approval_allows_download' in tabular_source, ( + 'The tabular citation route must enforce the approval gate before streaming a blob' + ) + assert 'except PermissionError' in tabular_source, ( + 'The tabular citation route must return 403 rather than 500 for a withheld file' + ) + gate_index = tabular_source.index('assert_generated_file_approval_allows_download') + download_index = tabular_source.index('blob_client.download_blob()') + assert gate_index < download_index, 'The gate must run before the blob is read' + + print('Artifact blob reader coverage verified.') + return True + + +def test_pending_approval_listing_is_scoped_before_truncation(): + """The pending list must narrow to the caller's scopes before applying the row cap.""" + print('Testing pending approval listing scope...') + source = read_app_source('functions_simplechat_operations.py') + function_source = extract_function_source(source, 'list_pending_generated_file_approvals_for_user') + + assert 'generated_artifact_approval_owner_user_id = @user_id' in function_source, ( + 'Personal-scope candidates must be filtered in the query' + ) + assert 'generated_artifact_approval_group_id IN (' in function_source, ( + 'Group-scope candidates must be filtered in the query' + ) + assert 'generated_artifact_approval_requested_by_id != @user_id' in function_source, ( + 'A requester must never appear in their own approval queue' + ) + assert 'get_user_groups' in function_source, ( + 'Group scope must come from the caller group membership' + ) + + scope_index = function_source.index('scope_clauses') + query_index = function_source.index('"SELECT TOP @limit * FROM c "') + assert scope_index < query_index, 'Scope filtering must be built before the capped query' + + # Authorization is still decided by the shared predicate, not by the query alone. + assert 'user_can_approve_generated_file' in function_source, ( + 'The shared authorization predicate must still gate every returned row' + ) + + print('Pending approval listing scope verified.') + return True + + +def test_approval_decisions_are_recorded_and_single_use(): + """A decision must be recorded once and never re-applied.""" + print('Testing approval decision transitions...') + approvals = load_approvals_module() + + message_item = { + 'id': 'artifact-1', + 'metadata': approvals.build_generated_file_approval_metadata( + build_participant_context(), + requester={'user_id': PARTICIPANT_USER_ID, 'display_name': 'Participant'}, + ), + } + + pending_payload = approvals.build_generated_file_approval_client_payload(message_item, OWNER_USER_ID) + assert pending_payload['is_pending'] is True + assert pending_payload['viewer_can_approve'] is True, 'Owner should see approve controls' + + requester_payload = approvals.build_generated_file_approval_client_payload( + message_item, PARTICIPANT_USER_ID, + ) + assert requester_payload['viewer_can_approve'] is False + assert requester_payload['viewer_is_requester'] is True + + updated = approvals.apply_generated_file_approval_decision( + message_item, + approvals.APPROVAL_STATE_APPROVED, + resolver={'user_id': OWNER_USER_ID, 'display_name': 'Owner User'}, + ) + assert updated['metadata']['generated_artifact_approval_state'] == approvals.APPROVAL_STATE_APPROVED + assert updated['metadata']['generated_artifact_approval_resolved_by_name'] == 'Owner User' + assert updated['metadata']['generated_artifact_approval_resolved_at'] + + try: + approvals.apply_generated_file_approval_decision( + updated, + approvals.APPROVAL_STATE_APPROVED, + resolver={'user_id': OWNER_USER_ID}, + ) + raise AssertionError('A resolved approval must not be re-applied') + except ValueError: + pass + + try: + approvals.apply_generated_file_approval_decision( + {'id': 'ungated', 'metadata': {}}, + approvals.APPROVAL_STATE_APPROVED, + ) + raise AssertionError('Ungated artifacts must not accept approval decisions') + except ValueError: + pass + + print('Approval decision transitions verified.') + return True + + +def test_download_route_enforces_approval_before_publication(): + """Gate 4: the download route checks approval independently of the export manifest.""" + print('Testing download route enforcement...') + source = read_app_source('route_enhanced_citations.py') + function_source = extract_function_source(source, '_get_authorized_chat_artifact_message') + + assert 'build_conversation_participation_context' in function_source, ( + 'The download route must authorize shared conversation participants' + ) + assert "raise PermissionError('Forbidden')" not in function_source, ( + 'The owner-only equality check must no longer gate artifact downloads' + ) + + approval_index = function_source.index('assert_generated_file_approval_allows_download') + publication_index = function_source.index('assert_generated_chat_artifact_is_published_for_user') + assert approval_index < publication_index, ( + 'Approval must be enforced before the export manifest short-circuit' + ) + + # The original bug was an unactionable bare "Forbidden"; the download route must surface + # the specific reason so a pending file explains itself. + assert 'return jsonify({"error": str(exc) or "Forbidden"}), 403' in source, ( + 'The artifact download route must surface the specific permission message' + ) + + print('Download route enforcement verified.') + return True + + +def test_artifact_upload_stages_participant_files(): + """Gate 2: participant artifact writes are staged rather than refused.""" + print('Testing artifact staging wiring...') + source = read_app_source('functions_simplechat_operations.py') + function_source = extract_function_source(source, '_upload_generated_chat_artifact_for_current_user') + + assert 'build_conversation_participation_context' in function_source, ( + 'Artifact uploads must authorize through the shared participation context' + ) + assert 'raise PermissionError("Forbidden")' not in function_source, ( + 'Participant artifact writes must no longer fail with a bare Forbidden' + ) + assert 'requires_generated_file_approval' in function_source, ( + 'Artifact uploads must consult the approval decision' + ) + assert '**approval_metadata' in function_source, ( + 'Approval state must be persisted on the artifact message metadata' + ) + assert '_notify_generated_file_approval_requested' in function_source, ( + 'Approvers must be notified when a file is staged' + ) + + resolve_source = extract_function_source(source, 'resolve_generated_file_approval_for_user') + assert 'user_can_approve_generated_file' in resolve_source, ( + 'Approval resolution must re-authorize the acting user on every call' + ) + assert 'delete_blob_backed_chat_message_files' in resolve_source, ( + 'Denied files must have their stored blob removed' + ) + + print('Artifact staging wiring verified.') + return True + + +def test_expired_staged_files_are_auto_denied(): + """Staged files that nobody approves expire and release their storage.""" + print('Testing auto-deny sweep wiring...') + approvals_source = read_app_source('functions_generated_file_approvals.py') + assert 'APPROVAL_TTL_DAYS = 3' in approvals_source, ( + 'Staged files should expire on the same 3 day window as Control Center approvals' + ) + + operations_source = read_app_source('functions_simplechat_operations.py') + sweep_source = extract_function_source( + operations_source, 'auto_deny_expired_generated_file_approvals', + ) + assert 'list_expired_pending_generated_file_artifacts' in sweep_source + assert 'APPROVAL_STATE_AUTO_DENIED' in sweep_source + assert 'delete_blob_backed_chat_message_files' in sweep_source, ( + 'Expired staged files must not leak blob storage' + ) + + background_source = read_app_source('background_tasks.py') + assert 'auto_deny_expired_generated_file_approvals' in background_source, ( + 'The expiry sweep must be scheduled' + ) + + print('Auto-deny sweep wiring verified.') + return True + + +def test_admin_setting_is_exposed_and_safe_to_share(): + """The admin toggle exists, defaults on, and survives settings sanitization.""" + print('Testing admin setting...') + settings_source = read_app_source('functions_settings.py') + assert "'require_shared_conversation_file_approval': True," in settings_source, ( + 'The approval requirement must default to enabled' + ) + + admin_route_source = read_app_source('route_frontend_admin_settings.py') + assert "'require_shared_conversation_file_approval': form_data.get(" in admin_route_source, ( + 'The admin form must persist the approval toggle' + ) + + template_path = os.path.join(APP_ROOT, 'templates', 'admin_settings.html') + with open(template_path, 'r', encoding='utf-8') as template_file: + template_source = template_file.read() + assert 'id="require_shared_conversation_file_approval"' in template_source, ( + 'The admin settings page must expose the toggle' + ) + + # sanitize_settings_for_user drops keys containing these terms; the toggle must survive. + sensitive_terms = ('key', 'secret', 'password', 'connection', 'base64', 'storage_account_url') + setting_key = 'require_shared_conversation_file_approval' + assert not any(term in setting_key for term in sensitive_terms), ( + 'The toggle name must not collide with sanitization filters' + ) + + print('Admin setting verified.') + return True + + +def test_frontend_approval_module_is_local_and_safe(): + """The approval UI is a local asset that renders untrusted values safely.""" + print('Testing frontend approval module...') + module_path = os.path.join(APP_ROOT, 'static', 'js', 'chat', 'chat-file-approvals.js') + assert os.path.exists(module_path), 'The approval UI module must be a local static asset' + + with open(module_path, 'r', encoding='utf-8') as module_file: + module_source = module_file.read() + + assert module_source.startswith('// chat-file-approvals.js'), 'Missing filename comment' + assert 'innerHTML' not in module_source, 'Approval UI must not use innerHTML' + assert 'display:none' not in module_source and 'display: none' not in module_source, ( + 'Use Bootstrap d-none instead of inline display styles' + ) + assert 'alert(' not in module_source, 'Use Bootstrap alerts and toasts, not alert()' + assert '//cdn' not in module_source and 'https://' not in module_source, ( + 'Browser assets must not reference remote sources' + ) + + messages_path = os.path.join(APP_ROOT, 'static', 'js', 'chat', 'chat-messages.js') + with open(messages_path, 'r', encoding='utf-8') as messages_file: + messages_source = messages_file.read() + assert 'buildGeneratedFileApprovalBlock' in messages_source, ( + 'The artifact card must render the approval block' + ) + assert 'generatedFileApprovalBlocksDownload' in messages_source, ( + 'The artifact card must suppress downloads while a file is staged' + ) + + print('Frontend approval module verified.') + return True + + +def test_version_supports_shared_conversation_file_approvals(): + """The fix must be present in at least its implementation version.""" + print('Testing application version...') + assert_app_version_at_least( + '0.260.006', + reason='Shared conversation file approvals were implemented in 0.260.006', + ) + print('Application version verified.') + return True + + +if __name__ == '__main__': + tests = [ + test_participation_context_allows_shared_conversation_participants, + test_stream_authorization_no_longer_rejects_participants, + test_only_gated_formats_require_approval, + test_owner_writes_are_never_gated, + test_staged_artifact_is_not_downloadable_until_approved, + test_personal_and_group_approver_resolution, + test_requester_can_never_approve_their_own_file, + test_approval_decisions_are_recorded_and_single_use, + test_download_route_enforces_approval_before_publication, + test_all_artifact_blob_readers_enforce_the_approval_gate, + test_pending_approval_listing_is_scoped_before_truncation, + test_artifact_upload_stages_participant_files, + test_expired_staged_files_are_auto_denied, + test_admin_setting_is_exposed_and_safe_to_share, + test_frontend_approval_module_is_local_and_safe, + test_version_supports_shared_conversation_file_approvals, + ] + + results = [] + for test in tests: + print(f'\nRunning {test.__name__}...') + try: + results.append(bool(test())) + except Exception as error: + print(f'FAILED: {error}') + import traceback + traceback.print_exc() + results.append(False) + + print(f'\nResults: {sum(results)}/{len(results)} tests passed') + sys.exit(0 if all(results) else 1) diff --git a/functional_tests/test_tabular_row_orchestration_scale.py b/functional_tests/test_tabular_row_orchestration_scale.py index 504a3daaa..a8757caf7 100644 --- a/functional_tests/test_tabular_row_orchestration_scale.py +++ b/functional_tests/test_tabular_row_orchestration_scale.py @@ -1422,6 +1422,23 @@ def get_blob_client(self, container, blob): 'uuid': uuid, 'storage_account_personal_chat_container_name': 'personal-chat', '_get_latest_personal_thread_id': lambda conversation_id: None, + # Shared conversations authorize through the participation context. This fixture is a + # single-owner conversation, so no approval staging applies. + 'build_conversation_participation_context': lambda user_id, conversation_item: { + 'user_id': user_id, + 'owner_user_id': (conversation_item or {}).get('user_id', ''), + 'is_owner': True, + 'collaboration_conversation_id': '', + 'group_id': '', + }, + 'requires_generated_file_approval': lambda *args, **kwargs: False, + 'build_generated_file_approval_metadata': lambda *args, **kwargs: {}, + '_get_current_user_summary_or_none': lambda fallback_user_id='': { + 'user_id': fallback_user_id, + 'display_name': '', + 'email': '', + }, + '_notify_generated_file_approval_requested': lambda *args, **kwargs: None, 'log_event': lambda *args, **kwargs: None, } extracted_module = ast.Module(body=selected_nodes, type_ignores=[])