diff --git a/application/single_app/config.py b/application/single_app/config.py index 840e27c15..c82b90dd7 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -100,7 +100,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.208" +VERSION = "0.261.209" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_orchestration_bootstrap.py b/application/single_app/functions_orchestration_bootstrap.py index 6ddf461bd..968a232a5 100644 --- a/application/single_app/functions_orchestration_bootstrap.py +++ b/application/single_app/functions_orchestration_bootstrap.py @@ -1,22 +1,22 @@ # functions_orchestration_bootstrap.py """Application-owned factories shared by web requests and scheduler continuations. -Version: 0.261.140 +Version: 0.261.209 Unlike the result/rendering services, this is an application composition root. Import it only after config has initialized the existing clients. Registering the artifact factory performs no I/O; each use rebuilds current actor/source access. +External-source identity trusts the signed-in session's app roles, like classic +chat, and makes no Microsoft Graph calls (0.261.209). """ import hashlib import hmac from copy import deepcopy -from urllib.parse import urlsplit -import requests +from flask import has_request_context import config -import functions_authentication as authentication from agent_execution_context import capture_execution_identity from functions_generated_export_contracts import GeneratedFileExportRequest from functions_generated_export_registry import resolve_generated_file_export_format @@ -94,7 +94,23 @@ def private_external_configuration_digest(value): def build_external_identity_reader(actor_user_id, actor_conversation_id, *, execution_check=None): - """Create a lazy current-directory reader, never a saved-session role fallback.""" + """Create a lazy current-access reader over the signed-in session's app roles. + + Like classic chat, roles come from the signed-in session. They are captured + now, while the request context exists, because execution continues on a + worker thread. Without a matching session, as in a scheduler continuation, + every external-source read fails closed instead of restoring saved roles. + Conversation ownership and Control Center restrictions are still point-read + on each access. Nothing calls Microsoft Graph or persists roles. + """ + session_roles, session_email = None, None + if has_request_context(): + try: + identity = capture_execution_identity(actor_user_id, actor_conversation_id) + except PermissionError: + identity = None + if identity is not None: + session_roles, session_email = identity.roles, identity.email reader = None timeout = 10.0 @@ -107,7 +123,8 @@ def authorize_conversation(*, user_id, conversation_id): raise ResultUnavailableError("external_identity_access_denied") from None def read_user_settings(user_id): - authorize_conversation(user_id=user_id, conversation_id=actor_conversation_id) + if user_id != actor_user_id: + raise ResultUnavailableError("external_identity_access_denied") return _document_response(config.cosmos_user_settings_container.read_item( item=user_id, partition_key=user_id, connection_timeout=timeout, read_timeout=timeout, retry_total=0, @@ -118,50 +135,14 @@ def read_identity(*, user_id, conversation_id): if user_id != actor_user_id or conversation_id != actor_conversation_id: raise ResultUnavailableError("external_identity_access_denied") if reader is None: - # Optional directory authorization is initialized only when external data is accessed. - from functions_orchestration_external_identity import ( - ExternalIdentityServiceError, - GraphExternalIdentityReader, - ) + # The reader is initialized only when external data is accessed. + from functions_orchestration_external_identity import SessionExternalIdentityReader - graph_base = authentication.get_graph_base_url() - graph_origin = urlsplit(graph_base) - graph_scope = f"{graph_origin.scheme}://{graph_origin.netloc}/.default" - app_client_id = authentication.CLIENT_ID - application = None - - def get_access_token(scope): - nonlocal application - if scope != graph_scope or authentication.CLIENT_ID != app_client_id: - raise ResultUnavailableError("external_identity_access_denied") - if application is None: - application = authentication._build_msal_app( - authority_override=authentication.get_graph_authority(), timeout=timeout, - ) - result = application.acquire_token_for_client(scopes=[scope]) - if type(result) is not dict: - raise ExternalIdentityServiceError("external_identity_response_invalid") - token = result.get("access_token") - if type(token) is str and token: - return token - error = result.get("error") - if error in ("server_error", "temporarily_unavailable"): - raise ExternalIdentityServiceError() - if error == "too_many_requests": - raise ExternalIdentityServiceError("external_identity_throttled") - if error in ( - "invalid_client", "invalid_grant", "unauthorized_client", "invalid_scope", - "access_denied", "consent_required", "interaction_required", - ): - raise ResultUnavailableError("external_identity_access_denied") - raise ExternalIdentityServiceError("external_identity_response_invalid") - - reader = GraphExternalIdentityReader( + reader = SessionExternalIdentityReader( user_id=actor_user_id, conversation_id=actor_conversation_id, - app_client_id=app_client_id, graph_base_url=graph_base, graph_scope=graph_scope, - get_access_token=get_access_token, http_get=requests.get, + roles=session_roles, email=session_email, authorize_conversation=authorize_conversation, read_user_settings=read_user_settings, - execution_check=execution_check, request_timeout=timeout, + execution_check=execution_check, ) return reader(user_id=user_id, conversation_id=conversation_id) diff --git a/application/single_app/functions_orchestration_composition.py b/application/single_app/functions_orchestration_composition.py index 08ae8b1da..42468d26c 100644 --- a/application/single_app/functions_orchestration_composition.py +++ b/application/single_app/functions_orchestration_composition.py @@ -1,7 +1,8 @@ # functions_orchestration_composition.py """Explicit one-call content preparation from named authorized result readers. -Version: 0.261.141 +Version: 0.261.209 +Refusals for a missing signed-in session reported as their own failure in: 0.261.209 No retrieval, file-format inference, upload, publication, or implicit sibling inputs. Answer-writing steps receive saved memory, the resolved conversation references, the @@ -42,8 +43,8 @@ ) from functions_orchestration_results import MAX_VALUE_BYTES, NamedOutput, ResultUnavailableError from functions_orchestration_schema import ( - STEP_STATUS_COMPLETED, STEP_STATUS_FAILED, STEP_STATUS_PARTIAL, build_failure, build_step_result, - failure_from_exception, safe_failure, step_input_specs, validate_inline_output_schema, + STEP_STATUS_COMPLETED, STEP_STATUS_FAILED, STEP_STATUS_PARTIAL, access_failure, build_failure, + build_step_result, failure_from_exception, safe_failure, step_input_specs, validate_inline_output_schema, ) from functions_orchestration_visuals import ( build_answer_visual_guidance, build_existing_charts_note, collect_run_charts, @@ -516,12 +517,15 @@ def prepared_values(response): except Exception as exc: raise_source_service_failure(exc) code = getattr(exc, 'code', '') + authority_reason = getattr(exc, 'authority_reason', None) + if type(authority_reason) is not str and isinstance(exc, ResultUnavailableError): + authority_reason = code if code == 'result_requires_streaming' or isinstance(exc, WorkflowContextBudgetError): failure = build_failure('result_input_too_large') elif isinstance(exc, OrchestrationMemoryError): failure = build_failure('context_unavailable') elif isinstance(exc, (ResultUnavailableError, PermissionError, ScreeningError)): - failure = build_failure('result_unavailable') + failure = access_failure(exc) elif isinstance(exc, (ResultContractError, ValueError)): failure = build_failure('result_invalid') else: @@ -537,6 +541,7 @@ def prepared_values(response): 'capability_id': step['capability_id'], 'error_type': type(exc).__name__, 'failure_code': failure['code'], **({'execution_code': code} if type(code) is str and code else {}), + **({'authority_reason': authority_reason} if type(authority_reason) is str and authority_reason else {}), }, ) return build_step_result( diff --git a/application/single_app/functions_orchestration_executor.py b/application/single_app/functions_orchestration_executor.py index 09fb1bb71..fde0cc116 100644 --- a/application/single_app/functions_orchestration_executor.py +++ b/application/single_app/functions_orchestration_executor.py @@ -31,7 +31,8 @@ A plan from the removed legacy contract is refused before anything runs. -Version: 0.261.141 +Version: 0.261.209 +Refusals for a missing signed-in session reported as their own failure in: 0.261.209 """ import logging @@ -90,6 +91,7 @@ STEP_STATUS_WAITING, STEP_STATUS_PARTIAL, STEP_STATUS_SKIPPED, + access_failure, build_step_result, build_failure, safe_failure, @@ -154,6 +156,14 @@ def _error_code(error): return code if isinstance(code, str) else None +def _authority_reason(error): + """The stable reason an access refusal carries, for diagnostics only.""" + reason = getattr(error, 'authority_reason', None) + if isinstance(reason, str): + return reason + return _error_code(error) if isinstance(error, ResultUnavailableError) else None + + def _partial_input_log_fields(error): """Which producer returned the partial result a complete-only input refused, as codes and counts.""" if not isinstance(error, PartialInputNotAcceptedError): @@ -893,7 +903,7 @@ def _run_dependency_step( if isinstance(exc, OrchestrationFilePolicyError): failure = build_failure('file_publication_not_allowed') elif isinstance(exc, (ResultUnavailableError, ElicitationContextError, PermissionError, ScreeningError)): - failure = build_failure('result_unavailable') + failure = access_failure(exc) elif isinstance(exc, PartialInputNotAcceptedError): failure = build_failure('input_partial_not_accepted') elif isinstance(exc, ResultContractError): @@ -906,6 +916,7 @@ def _run_dependency_step( extra={ **_step_log_context(context, step), 'error_type': type(exc).__name__, 'failure_code': failure['code'], 'execution_code': _error_code(exc), + 'authority_reason': _authority_reason(exc), **_partial_input_log_fields(exc), }, ) @@ -1266,15 +1277,17 @@ def save(record): _raise_dependency_service_failure(step, exc) if step['role'] == 'render': raise + failure = access_failure(exc) log_event( f'{_LOG_PREFIX} A saved wait could not be resumed.', level=logging.WARNING, extra={ **_step_log_context(context, step), 'error_type': type(exc).__name__, - 'failure_code': 'result_unavailable', 'execution_code': _error_code(exc), + 'failure_code': failure['code'], 'execution_code': _error_code(exc), + 'authority_reason': _authority_reason(exc), }, ) - result = build_step_result(status=STEP_STATUS_FAILED, failure=build_failure('result_unavailable')) + result = build_step_result(status=STEP_STATUS_FAILED, failure=failure) task = result.get('task_result') if result['status'] in (STEP_STATUS_COMPLETED, STEP_STATUS_PARTIAL, STEP_STATUS_WAITING): if step['role'] == 'render': @@ -1507,11 +1520,15 @@ def finished_stop(): WorkflowResultIntegrityError, WorkflowResultStorageUnavailableError, ) as exc: raise_source_service_failure(exc) + failure = access_failure(exc) log_event( f'{_LOG_PREFIX} Retained content could not be reauthorized for finalization.', - level=logging.WARNING, extra={'run_id': context.run_id, 'error_type': type(exc).__name__}, + level=logging.WARNING, extra={ + 'run_id': context.run_id, 'error_type': type(exc).__name__, + 'failure_code': failure['code'], 'authority_reason': _authority_reason(exc), + }, ) - context.failures.append(build_failure('result_unavailable')) + context.failures.append(failure) message, complete, response_reference, citations = '', False, None, [] context.artifacts = [] file_outputs, committed_artifacts = _dependency_file_outcomes( diff --git a/application/single_app/functions_orchestration_external_configuration.py b/application/single_app/functions_orchestration_external_configuration.py index a403fd277..d508bc32f 100644 --- a/application/single_app/functions_orchestration_external_configuration.py +++ b/application/single_app/functions_orchestration_external_configuration.py @@ -1,7 +1,8 @@ # functions_orchestration_external_configuration.py """Opaque invocation-configuration proof and capture-independent current reads. -Version: 0.261.127 +Version: 0.261.209 +Research planner profile restriction removed in: 0.261.209 The owning engines supply actual acquisition evidence. The application root supplies current metadata reads; this module discovers no settings or clients, @@ -531,23 +532,6 @@ def _fetch_policy(settings, *, direct): return policy -def is_research_acquisition_profile_supported(settings): - """Return whether normalized research settings avoid unattested planner calls.""" - if type(settings) is not dict: - raise ResultContractError("external_configuration_invalid") - web_enabled = settings.get("enable_web_search", False) - if type(web_enabled) is not bool: - _invalid_metadata() - effective = get_source_review_config(settings) - return not ( - web_enabled - and effective["deep_research_max_search_queries_per_turn"] > 1 - and effective["deep_research_enable_query_planning"] - or effective["enable_deep_source_review"] - and effective["source_review_enable_llm_planning"] - ) - - def _components(source_type, *, settings, source, selector, private_digest, require_run=False): source = _envelope(source) if source_type == "web": @@ -901,11 +885,6 @@ def validate_acquisition( if type(settings) is not dict or type(current_settings) is not dict: raise ResultContractError("external_configuration_invalid") _selector(source_type, selector) - if source_type == "deep_research" and ( - not is_research_acquisition_profile_supported(current_settings) - or not is_research_acquisition_profile_supported(settings) - ): - raise ResultUnavailableError("external_configuration_research_profile_unsupported") current_selector, current_parts = self._current_components( source_type, producer=producer, settings=current_settings, source=current_source, ) diff --git a/application/single_app/functions_orchestration_external_identity.py b/application/single_app/functions_orchestration_external_identity.py index 1e7f2530b..3c2ca061d 100644 --- a/application/single_app/functions_orchestration_external_identity.py +++ b/application/single_app/functions_orchestration_external_identity.py @@ -1,41 +1,33 @@ # functions_orchestration_external_identity.py -"""Read current external-result identity through injected Microsoft Graph I/O. +"""Read current external-result identity from the signed-in session. -Version: 0.261.127 +Version: 0.261.209 +Implemented in: 0.261.127 +Signed-in session roles replaced per-call Microsoft Graph reads in: 0.261.209 -Only the owner supplies credentials, the trusted cloud endpoint, conversation -authorization and uncached, read-only user settings. No directory authority, -tokens or settings are persisted or cached by this module. +Orchestration trusts the app roles in the signed-in session, the same authority +classic chat uses. The owner captures those roles while a request context is +available, then supplies current conversation authorization and uncached, +read-only user settings. This module makes no directory calls and persists or +caches no roles, tokens or settings. """ -from collections.abc import Callable -from dataclasses import dataclass from datetime import datetime, timezone -import json -import math -import re -import string -from time import monotonic -from urllib.parse import parse_qsl, urlencode, urlsplit from azure.core.exceptions import AzureError, ClientAuthenticationError, HttpResponseError, ResourceNotFoundError -from requests import Response from requests.exceptions import RequestException, Timeout from functions_orchestration_external_sources import CurrentExternalSourceIdentity from functions_orchestration_invocation_capture import ( OrchestrationInvocationCancelledError, OrchestrationInvocationServiceError, ) -from functions_orchestration_result_contracts import ResultContractError, identifier +from functions_orchestration_result_contracts import ( + EXTERNAL_SESSION_UNAVAILABLE_REASON, ResultContractError, identifier, +) from functions_orchestration_results import ResultUnavailableError -_GUID = re.compile(r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\Z") -_ZERO_GUID = "00000000-0000-0000-0000-000000000000" -_ASSIGNMENT_ID = re.compile(r"[A-Za-z0-9_-]{1,256}\Z") -_BEARER_TOKEN = re.compile(r"[A-Za-z0-9._~+/-]+=*\Z") -_ROLE_CHARACTERS = frozenset(string.ascii_letters + string.digits + ":!#$%&'()*+,-./;<=>?@[]^_`{|}~") -_PATH_CHARACTERS = frozenset(string.ascii_letters + string.digits + "/-._~") +MAX_SESSION_ROLES = 64 _SERVICE_ERRORS = { "external_identity_service_unavailable": True, "external_identity_timeout": True, @@ -60,16 +52,16 @@ def __init__(self, code="external_identity_service_unavailable"): raise ValueError("Invalid external identity error code.") self.code = code self.retryable = _SERVICE_ERRORS[code] - super().__init__("Current directory authorization could not be verified.") + super().__init__("Current user authorization could not be verified.") class ExternalIdentityCancelledError(OrchestrationInvocationCancelledError): - """An owning execution check explicitly stopped directory authorization.""" + """An owning execution check explicitly stopped current authorization.""" code = "external_identity_cancelled" def __init__(self): - super().__init__("Current directory authorization was cancelled.") + super().__init__("Current user authorization was cancelled.") def _http_error(status): @@ -109,142 +101,75 @@ def _call_io(callback, *args, **kwargs): raise failure -def _guid(value, *, allow_zero=False): - return ( - type(value) is str and _GUID.fullmatch(value) is not None - and (allow_zero or value != _ZERO_GUID) - ) +def _valid_text(value, limit): + try: + identifier(value, limit=limit) + except ResultContractError: + return False + return True -def _url_parts(value): - if ( - type(value) is not str or not 1 <= len(value) <= 8192 - or any(ord(character) < 33 or ord(character) > 126 for character in value) - or "\\" in value or "#" in value or re.search(r"%(?![0-9A-Fa-f]{2})", value) - ): - return None - try: - parts = urlsplit(value) - port = parts.port - query = tuple(sorted(parse_qsl(parts.query, keep_blank_values=True, strict_parsing=True, max_num_fields=32))) - except ValueError: - parts = None - if parts is None: +def _session_roles(roles): + if roles is None: return None - if ( - parts.scheme != "https" or not parts.hostname or parts.username is not None - or parts.password is not None or parts.fragment or not set(parts.path) <= _PATH_CHARACTERS - or not set(parts.hostname) <= set(string.ascii_letters + string.digits + ".-_:") - or parts.netloc.endswith(":") or (port is not None and not 1 <= port <= 65535) - or any(segment in {".", ".."} for segment in parts.path.split("/")) - or "//" in parts.path or len({key for key, _ in query}) != len(query) - ): + if type(roles) not in (list, tuple): + raise ResultContractError("external_identity_configuration_invalid") + return tuple(sorted({role for role in roles if _valid_text(role, 128)})) + + +def _session_email(email): + if not _valid_text(email, 320) or any(character.isspace() for character in email): return None - return parts, ("https", parts.hostname, port if port is not None else 443), query - - -def _unique_object(pairs): - value = {} - for key, item in pairs: - if key in value: - raise ValueError("Duplicate directory response field.") - value[key] = item - return value - - -def _invalid_json_constant(_value): - raise ValueError("Invalid directory response number.") - - -@dataclass -class _ReadBudget: - deadline: float - max_pages: int - max_records: int - execution_check: Callable | None - pages: int = 0 - records: int = 0 - - def check(self): - if monotonic() >= self.deadline: - raise ExternalIdentityServiceError("external_identity_timeout") - if self.execution_check is not None: - allowed = self.execution_check() - if allowed is False: - raise ExternalIdentityCancelledError() - if allowed is not None and allowed is not True: - raise ExternalIdentityServiceError("external_identity_callback_invalid") - remaining = self.deadline - monotonic() - if remaining <= 0: - raise ExternalIdentityServiceError("external_identity_timeout") - return remaining - - def add_records(self, count): - self.records += count - if self.records > self.max_records: - raise ExternalIdentityServiceError("external_identity_limit_exceeded") + return email -class GraphExternalIdentityReader: +class SessionExternalIdentityReader: """Callable read_identity boundary for OrchestrationExternalSourceProvider. - get_access_token(scope) returns a bearer string. The owner may lazily acquire - it through its existing MSAL application or an initialized credential. - http_get is an initialized, non-caching requests.Session.get-compatible - transport. authorize_conversation(*, user_id, conversation_id) returns the - current owned conversation. read_user_settings(user_id) returns its current - actor-bound document, without caching, repairs, defaults for missing records, - or writes. Owner callbacks must bound their own I/O and may raise normal - Azure/requests I/O errors, PermissionError, or ExternalIdentityServiceError. + roles and email come from the signed-in session that the owner captured + while a request context existed. roles=None means no signed-in session was + available, for example in a scheduler continuation, and every read then + fails closed. Saved run roles are never restored. + authorize_conversation(*, user_id, conversation_id) returns the current owned + conversation. read_user_settings(user_id) returns its current actor-bound + document, without caching, repairs, defaults for missing records, or writes. + Owner callbacks must bound their own I/O and may raise normal Azure/requests + I/O errors, PermissionError, or ExternalIdentityServiceError. """ def __init__( - self, *, user_id, conversation_id, app_client_id, graph_base_url, graph_scope, - get_access_token, http_get, authorize_conversation, read_user_settings, - execution_check=None, request_timeout=10.0, max_elapsed=30.0, - max_pages=20, max_records=4096, max_response_bytes=1048576, + self, *, user_id, conversation_id, roles, email, authorize_conversation, + read_user_settings, execution_check=None, ): - if not _guid(user_id) or not _guid(app_client_id): - raise ResultContractError("external_identity_configuration_invalid") - identifier(conversation_id, limit=256) - base = _url_parts(graph_base_url) - scope = _url_parts(graph_scope) - if ( - base is None or "?" in graph_base_url or scope is None or "?" in graph_scope - or not scope[0].path.endswith("/.default") - ): - raise ResultContractError("external_identity_configuration_invalid") - for callback in (get_access_token, http_get, authorize_conversation, read_user_settings): + try: + identifier(user_id) + identifier(conversation_id, limit=256) + except ResultContractError: + raise ResultContractError("external_identity_configuration_invalid") from None + for callback in (authorize_conversation, read_user_settings): if not callable(callback): raise ResultContractError("external_identity_callback_required") if execution_check is not None and not callable(execution_check): raise ResultContractError("external_identity_callback_required") - for value, maximum in ((request_timeout, 30), (max_elapsed, 120)): - if type(value) not in (int, float) or not math.isfinite(value) or not 0 < value <= maximum: - raise ResultContractError("external_identity_limit_invalid") - for value, maximum in ((max_pages, 100), (max_records, 10000), (max_response_bytes, 8388608)): - if type(value) is not int or not 1 <= value <= maximum: - raise ResultContractError("external_identity_limit_invalid") self.user_id = user_id self.conversation_id = conversation_id - self.app_client_id = app_client_id - self.graph_base_url = graph_base_url.rstrip("/") - self.graph_scope = graph_scope - self._origin = base[1] - self._path_prefix = base[0].path.rstrip("/") - self.get_access_token = get_access_token - self.http_get = http_get + self.roles = _session_roles(roles) + self.email = _session_email(email) self.authorize_conversation = authorize_conversation self.read_user_settings = read_user_settings self.execution_check = execution_check - self.request_timeout = request_timeout - self.max_elapsed = max_elapsed - self.max_pages = max_pages - self.max_records = max_records - self.max_response_bytes = max_response_bytes - - def _authorize_conversation(self, budget): - budget.check() + + def _check(self): + # Owner lease and cancellation errors are theirs to report; don't reclassify them. + if self.execution_check is None: + return + allowed = self.execution_check() + if allowed is False: + raise ExternalIdentityCancelledError() + if allowed is not None and allowed is not True: + raise ExternalIdentityServiceError("external_identity_callback_invalid") + + def _authorize_conversation(self): conversation = _call_io( self.authorize_conversation, user_id=self.user_id, conversation_id=self.conversation_id, ) @@ -256,10 +181,8 @@ def _authorize_conversation(self, budget): for key in ("deleted", "orchestration_deleted")) ): raise ResultUnavailableError("external_identity_conversation_unavailable") - budget.check() - def _settings(self, budget): - budget.check() + def _settings(self): document = _call_io(self.read_user_settings, self.user_id) if ( type(document) is not dict or type(document.get("id")) is not str or document["id"] != self.user_id @@ -270,205 +193,11 @@ def _settings(self, budget): preference = document["settings"].get("enable_agents", True) if type(preference) is not bool: raise ResultUnavailableError("external_identity_settings_unavailable") - budget.check() return document["settings"], preference - def _checked_url(self, url, path): - checked = _url_parts(url) - if ( - checked is None or checked[1] != self._origin - or not checked[0].path.startswith(f"{self._path_prefix}/") - or checked[0].path != path - ): - raise ExternalIdentityServiceError("external_identity_pagination_invalid") - return checked[1], checked[0].path, checked[2] - - def _read_response(self, response, url_key, path, budget): - try: - if ( - type(response.history) is not list or response.history - or self._checked_url(response.url, path) != url_key - or type(response.status_code) is not int - ): - raise ExternalIdentityServiceError("external_identity_response_invalid") - if response.status_code != 200: - raise _http_error(response.status_code) - content_type = response.headers.get("Content-Type") - if type(content_type) is not str or content_type.split(";", 1)[0].strip().lower() != "application/json": - raise ExternalIdentityServiceError("external_identity_response_invalid") - chunks = [] - size = 0 - iterator = response.iter_content(chunk_size=16384) - while True: - budget.check() - try: - chunk = _call_io(next, iterator) - except StopIteration: - break - if type(chunk) is not bytes: - raise ExternalIdentityServiceError("external_identity_response_invalid") - if not chunk: - continue - size += len(chunk) - if size > self.max_response_bytes: - raise ExternalIdentityServiceError("external_identity_limit_exceeded") - chunks.append(chunk) - budget.check() - try: - value = json.loads( - b"".join(chunks).decode("utf-8"), object_pairs_hook=_unique_object, - parse_constant=_invalid_json_constant, - ) - except (ValueError, UnicodeError, RecursionError): - value = None - if type(value) is not dict or "@odata.deltaLink" in value or "error" in value: - raise ExternalIdentityServiceError("external_identity_response_invalid") - return value - finally: - _call_io(response.close) - - def _get(self, url, path, budget, *, counted=False): - url_key = self._checked_url(url, path) - self._authorize_conversation(budget) - if budget.pages >= budget.max_pages: - raise ExternalIdentityServiceError("external_identity_limit_exceeded") - budget.pages += 1 - token = _call_io(self.get_access_token, self.graph_scope) - if type(token) is not str or not 1 <= len(token) <= 65536 or _BEARER_TOKEN.fullmatch(token) is None: - raise ExternalIdentityServiceError("external_identity_callback_invalid") - remaining = budget.check() - headers = { - "Authorization": f"Bearer {token}", "Accept": "application/json", - "Cache-Control": "no-cache, no-store", "Accept-Encoding": "identity", - } - if counted: - headers["ConsistencyLevel"] = "eventual" - response = _call_io( - self.http_get, url, headers=headers, timeout=min(self.request_timeout, remaining), - allow_redirects=False, stream=True, - ) - if not isinstance(response, Response): - raise ExternalIdentityServiceError("external_identity_callback_invalid") - return self._read_response(response, url_key, path, budget) - - def _collection(self, path, parameters, budget, *, counted=False): - url = f"{self.graph_base_url}{path.removeprefix(self._path_prefix)}?{urlencode(parameters)}" - seen = set() - values = [] - expected_count = None - while True: - url_key = self._checked_url(url, path) - if url_key in seen: - raise ExternalIdentityServiceError("external_identity_pagination_invalid") - seen.add(url_key) - page = self._get(url, path, budget, counted=counted) - items = page.get("value") - if type(items) is not list or any(type(item) is not dict for item in items): - raise ExternalIdentityServiceError("external_identity_response_invalid") - count = page.get("@odata.count") - if "@odata.count" in page: - if type(count) is not int or count < 0 or (expected_count is not None and count != expected_count): - raise ExternalIdentityServiceError("external_identity_response_invalid") - if count > budget.max_records: - raise ExternalIdentityServiceError("external_identity_limit_exceeded") - expected_count = count - if counted and expected_count is None: - raise ExternalIdentityServiceError("external_identity_response_invalid") - budget.add_records(len(items)) - values.extend(items) - if expected_count is not None and len(values) > expected_count: - raise ExternalIdentityServiceError("external_identity_response_invalid") - if "@odata.nextLink" not in page: - break - url = page["@odata.nextLink"] - if expected_count is not None and len(values) != expected_count: - raise ExternalIdentityServiceError("external_identity_response_invalid") - return values - - @staticmethod - def _active_object(value, object_type, expected_id=None): - if ( - not _guid(value.get("id")) or type(value.get("accountEnabled")) is not bool - or "@odata.nextLink" in value - or ("@odata.type" in value and value["@odata.type"] != f"#microsoft.graph.{object_type}") - ): - raise ExternalIdentityServiceError("external_identity_response_invalid") - if ( - (expected_id is not None and value["id"] != expected_id) - or not value["accountEnabled"] or value.get("deletedDateTime") is not None - or "@removed" in value - ): - raise ResultUnavailableError("external_identity_account_unavailable") - - def _role_definitions(self, principal, budget): - definitions = principal.get("appRoles") - if type(definitions) is not list or "appRoles@odata.nextLink" in principal: - raise ExternalIdentityServiceError("external_identity_response_invalid") - if "appRoles@odata.count" in principal and ( - type(principal["appRoles@odata.count"]) is not int - or principal["appRoles@odata.count"] != len(definitions) - ): - raise ExternalIdentityServiceError("external_identity_response_invalid") - budget.add_records(len(definitions)) - roles = {} - names = set() - for role in definitions: - if type(role) is not dict: - raise ExternalIdentityServiceError("external_identity_response_invalid") - role_id, value, members = role.get("id"), role.get("value"), role.get("allowedMemberTypes") - if ( - not _guid(role_id) or role_id in roles or type(role.get("isEnabled")) is not bool - or type(value) is not str or not 1 <= len(value) <= 120 - or value.startswith(".") or not set(value) <= _ROLE_CHARACTERS - or type(members) is not list or not 1 <= len(members) <= 2 - or any(type(member) is not str or member not in {"User", "Application"} for member in members) - or len(set(members)) != len(members) - ): - raise ExternalIdentityServiceError("external_identity_response_invalid") - eligible = role["isEnabled"] and "User" in members - if eligible and value in names: - raise ExternalIdentityServiceError("external_identity_response_invalid") - roles[role_id] = value if eligible else None - if eligible: - names.add(value) - return roles - - def _assigned_roles(self, assignments, principal_id, definitions): - identities = set() - roles = set() - for assignment in assignments: - assignment_id = assignment.get("id") - role_id = assignment.get("appRoleId") - principal_type = assignment.get("principalType") - principal = assignment.get("principalId") - resource = assignment.get("resourceId") - if ( - type(assignment_id) is not str or _ASSIGNMENT_ID.fullmatch(assignment_id) is None - or assignment_id in identities or not _guid(role_id, allow_zero=True) - or not _guid(principal) or not _guid(resource) - or type(principal_type) is not str or principal_type not in {"User", "Group"} - or (principal_type == "User" and principal != self.user_id) - or (principal_type == "Group" and principal == self.user_id) - or assignment.get("deletedDateTime") is not None or "@removed" in assignment - or ("@odata.type" in assignment and assignment["@odata.type"] != "#microsoft.graph.appRoleAssignment") - ): - raise ExternalIdentityServiceError("external_identity_response_invalid") - identities.add(assignment_id) - if resource != principal_id or role_id == _ZERO_GUID: - continue - if role_id not in definitions: - raise ResultUnavailableError("external_identity_role_unavailable") - value = definitions[role_id] - if value is not None: - roles.add(value) - if not {"User", "Admin"}.intersection(roles): - raise ResultUnavailableError("external_identity_role_required") - if len(roles) > 64: - raise ExternalIdentityServiceError("external_identity_limit_exceeded") - return tuple(sorted(roles)) - @staticmethod def _check_access(settings, roles): + """Apply the Control Center restriction that user_required enforces for chat.""" if "Admin" in roles: return access = settings.get("access", {}) @@ -495,49 +224,16 @@ def __call__(self, *, user_id, conversation_id): or type(conversation_id) is not str or conversation_id != self.conversation_id ): raise ResultUnavailableError("external_identity_actor_mismatch") - budget = _ReadBudget( - monotonic() + self.max_elapsed, self.max_pages, self.max_records, self.execution_check, - ) - self._authorize_conversation(budget) - self._settings(budget) - user_path = f"{self._path_prefix}/users/{self.user_id}" - user = self._get( - f"{self.graph_base_url}/users/{self.user_id}?{urlencode({'$select': 'id,accountEnabled,userPrincipalName'})}", - user_path, budget, - ) - budget.add_records(1) - self._active_object(user, "user", self.user_id) - email = user.get("userPrincipalName") - if email is not None: - if type(email) is not str or not 1 <= len(email) <= 320 or any(character.isspace() for character in email): - raise ExternalIdentityServiceError("external_identity_response_invalid") - identifier(email, limit=320) - principals = self._collection( - f"{self._path_prefix}/servicePrincipals", - {"$filter": f"appId eq '{self.app_client_id}'", "$select": "id,appId,accountEnabled,appRoles"}, - budget, - ) - if not principals: - raise ResultUnavailableError("external_identity_application_unavailable") - if len(principals) != 1: - raise ExternalIdentityServiceError("external_identity_response_invalid") - principal = principals[0] - self._active_object(principal, "servicePrincipal") - if not _guid(principal.get("appId")) or principal["appId"] != self.app_client_id: - raise ResultUnavailableError("external_identity_application_unavailable") - definitions = self._role_definitions(principal, budget) - assignments = self._collection( - f"{user_path}/appRoleAssignments", - { - "$filter": f"resourceId eq {principal['id']}", - "$select": "id,appRoleId,principalId,principalType,resourceId", - "$count": "true", - }, - budget, counted=True, - ) - roles = self._assigned_roles(assignments, principal["id"], definitions) - self._authorize_conversation(budget) - settings, enable_agents = self._settings(budget) - self._check_access(settings, roles) - budget.check() - return CurrentExternalSourceIdentity(self.user_id, roles, enable_agents, email) + if self.roles is None: + raise ResultUnavailableError(EXTERNAL_SESSION_UNAVAILABLE_REASON) + if len(self.roles) > MAX_SESSION_ROLES: + raise ExternalIdentityServiceError("external_identity_limit_exceeded") + if not {"User", "Admin"}.intersection(self.roles): + raise ResultUnavailableError("external_identity_role_required") + self._check() + self._authorize_conversation() + self._check() + settings, enable_agents = self._settings() + self._check_access(settings, self.roles) + self._check() + return CurrentExternalSourceIdentity(self.user_id, self.roles, enable_agents, self.email) diff --git a/application/single_app/functions_orchestration_external_sources.py b/application/single_app/functions_orchestration_external_sources.py index dfc793bc1..6e32ae1ae 100644 --- a/application/single_app/functions_orchestration_external_sources.py +++ b/application/single_app/functions_orchestration_external_sources.py @@ -1,7 +1,7 @@ # functions_orchestration_external_sources.py """Server admission and current access for retained external content. -Version: 0.261.139 +Version: 0.261.209 No fetch, recall, plugin invocation, settings discovery, or credential persistence occurs here. Content digests attest the exact retained payload, not a remote page @@ -66,11 +66,13 @@ @dataclass(frozen=True) class CurrentExternalSourceIdentity: - """Returned by an owner callback that refreshes roles on EVERY access. + """Current actor identity returned by the owner's read_identity callback. - A saved RunContext, session/login-time role snapshot, or persisted token is - not a refresher. If current roles/account access cannot be established, the - callback must fail rather than construct this value from cached claims. + Roles come from the signed-in session, the same authority classic chat + trusts; conversation ownership and user access restrictions are re-read on + every access. A saved RunContext or persisted token is never a source of + roles. Without a signed-in session, the callback must fail rather than + construct this value. """ user_id: str @@ -141,7 +143,8 @@ class OrchestrationExternalSourceProvider: Catalog callbacks have the existing resolve_*_catalog protocol. Exact resolvers default to real agent/action authorization, not catalog membership alone. All callbacks run again on reads, including with a fresh provider and - an empty admission catalog. The owner must supply restart-capable callbacks. + an empty admission catalog. The owner must supply restart-capable callbacks; + read_identity refuses when no signed-in session is available. """ def __init__( diff --git a/application/single_app/functions_orchestration_images.py b/application/single_app/functions_orchestration_images.py index c246389ad..f9227ef7f 100644 --- a/application/single_app/functions_orchestration_images.py +++ b/application/single_app/functions_orchestration_images.py @@ -1,8 +1,9 @@ # functions_orchestration_images.py """Planned image generation for Gather / Reason / Render plans. -Version: 0.261.138 +Version: 0.261.209 Implemented in: 0.261.138 +Refusals for a missing signed-in session reported as their own failure in: 0.261.209 An image the user explicitly asked for is generated by a ``generate_image`` step. Plan approval is the consent to generate it (in Run automatically mode, the explicit ask is). @@ -38,7 +39,8 @@ ) from functions_orchestration_results import NamedOutput, ResultUnavailableError from functions_orchestration_schema import ( - STEP_STATUS_COMPLETED, STEP_STATUS_FAILED, build_failure, build_step_result, failure_from_exception, + STEP_STATUS_COMPLETED, STEP_STATUS_FAILED, access_failure, build_failure, build_step_result, + failure_from_exception, ) @@ -131,7 +133,7 @@ def _failure(exc): if isinstance(exc, OrchestrationFilePolicyError): return build_failure('file_publication_not_allowed') if isinstance(exc, (ResultUnavailableError, ScreeningError)): - return build_failure('result_unavailable') + return access_failure(exc) if isinstance(exc, ResultContractError): return build_failure('result_input_too_large' if code == 'result_input_too_large' else 'result_invalid') if code == 'image_content_refused': diff --git a/application/single_app/functions_orchestration_invocation_capture.py b/application/single_app/functions_orchestration_invocation_capture.py index f2e6981c5..31d063757 100644 --- a/application/single_app/functions_orchestration_invocation_capture.py +++ b/application/single_app/functions_orchestration_invocation_capture.py @@ -1,7 +1,9 @@ # functions_orchestration_invocation_capture.py """Private invocation-capture state, independent of application initialization. -Version: 0.261.127 +Version: 0.261.209 +Implemented in: 0.261.127 +Denial reason code carried on refused sources in: 0.261.209 The server supplies an already producer-bound callback. SDKs and tool loops may catch exceptions, so a refused capture remains sticky until the owning @@ -11,6 +13,7 @@ from copy import deepcopy import inspect +import re from threading import RLock from typing import NoReturn @@ -18,6 +21,14 @@ from functions_orchestration_result_contracts import ResultContractError +# A refusal's stable application code, never exception text or an identifier. +_AUTHORITY_REASON = re.compile(r"[a-z][a-z0-9_]{0,79}\Z") + + +def _authority_reason(value): + return value if type(value) is str and _AUTHORITY_REASON.fullmatch(value) is not None else None + + class OrchestrationInvocationCaptureError(ResultContractError): """The actual invocation configuration could not be attested.""" @@ -38,12 +49,17 @@ class OrchestrationInvocationControlError(RuntimeError): class OrchestrationInvocationDeniedError(PermissionError): - """An acquired source is denied, without retaining private authority details.""" + """An acquired source is denied, without retaining private authority details. + + ``authority_reason`` keeps only the refusal's stable snake_case code, such as + a missing signed-in session, so the owner can explain and diagnose the denial. + """ code = "result_unavailable" retryable = False - def __init__(self): + def __init__(self, authority_reason=None): + self.authority_reason = _authority_reason(authority_reason) super().__init__("The acquired source is unavailable under the current access policy.") @@ -122,6 +138,11 @@ def _record_failure(self, error): self._failure_type = OrchestrationInvocationHeldError elif isinstance(error, PermissionError): self._failure_type = OrchestrationInvocationDeniedError + self._failure_code = _authority_reason( + getattr(error, "authority_reason", None) + if isinstance(error, OrchestrationInvocationDeniedError) + else getattr(error, "code", None) + ) def fail(self, error) -> NoReturn: """Record an owned engine failure without retaining its private exception.""" diff --git a/application/single_app/functions_orchestration_result_contracts.py b/application/single_app/functions_orchestration_result_contracts.py index 4c636a047..cd1674a62 100644 --- a/application/single_app/functions_orchestration_result_contracts.py +++ b/application/single_app/functions_orchestration_result_contracts.py @@ -19,6 +19,9 @@ EXTERNAL_LINEAGE_VERSION = "orchestration-lineage-v2" EXTERNAL_SOURCE_TYPES = frozenset({"web", "url", "deep_research", "agent", "action", "fact_memory"}) MAX_EXTERNAL_SOURCES = 64 +# Why current access to external sources was refused when no signed-in session can supply +# the actor's roles, for example in a background continuation. +EXTERNAL_SESSION_UNAVAILABLE_REASON = "external_identity_session_unavailable" # A generated image kept for the answer and for files. The retained value describes the # image; its bytes stay in the conversation's image message and are read only by the # server-owned image resolver after checking this digest. diff --git a/application/single_app/functions_orchestration_schema.py b/application/single_app/functions_orchestration_schema.py index 703c0e849..76615dd47 100644 --- a/application/single_app/functions_orchestration_schema.py +++ b/application/single_app/functions_orchestration_schema.py @@ -33,7 +33,8 @@ render through the very same card. Our own paging lives in a sibling ``ui_hints`` field rather than inside the schema, which keeps the schema itself MCP-clean. -Version: 0.261.141 +Version: 0.261.209 +Missing signed-in session reported as its own step failure in: 0.261.209 """ import hashlib @@ -65,8 +66,8 @@ resolve_available_capability_ids, ) from functions_orchestration_result_contracts import ( - IMAGE_ASSET_KIND, InputBinding, InputSpec, OutputSpec, RecordColumn, ResultContractError, - StepBindings, TaskResult, canonical_bytes, output_name, validate_input_bindings, + EXTERNAL_SESSION_UNAVAILABLE_REASON, IMAGE_ASSET_KIND, InputBinding, InputSpec, OutputSpec, RecordColumn, + ResultContractError, StepBindings, TaskResult, canonical_bytes, output_name, validate_input_bindings, ) from functions_orchestration_deliverables import DeliverableError, compile_deliverables @@ -1274,6 +1275,11 @@ def summarize_plan(plan): 'analysis_result_not_saved': 'The analysis completed, but its final data could not be saved for reuse.', 'analysis_input_too_large': 'The complete saved analysis exceeds the selected model input budget. No data was truncated or re-analyzed. Select a larger model or use a supported complete-record reader.', 'result_unavailable': 'A required retained result is unavailable or changed. No preview was substituted.', + 'external_session_required': ( + 'This step continued in the background, where your sign-in is not available to confirm ' + 'access to web search, web pages, deep research, agents or actions. Send the request ' + 'again to use them.' + ), 'result_invalid': 'The operation did not produce the complete named results declared by the plan.', 'result_input_too_large': 'The complete named inputs exceed the selected model budget. No input was truncated. Use a larger model or revise the plan.', 'result_not_ready': 'Required computation is still pending. Its result is not ready to consume.', @@ -1345,6 +1351,20 @@ def failure_from_exception(exc, *, answering=False, _depth=0): return build_failure('model_failed' if answering else 'step_failed') +def access_failure(exc, *, _depth=0): + """Explain a refused retained source, naming a missing signed-in session when that was why. + + Only the refusal's stable reason code is read, never exception text. + """ + for reason in (getattr(exc, 'authority_reason', None), getattr(exc, 'code', None)): + if type(reason) is str and reason == EXTERNAL_SESSION_UNAVAILABLE_REASON: + return build_failure('external_session_required') + cause = getattr(exc, '__cause__', None) + if cause is not None and cause is not exc and _depth < 3: + return access_failure(cause, _depth=_depth + 1) + return build_failure('result_unavailable') + + # Failures one bounded retry of a read-only step could plausibly outlast. Configuration, # authorization and validation failures are never retried; they would fail the same way. TRANSIENT_FAILURE_CODES = frozenset({'provider_timeout', 'connection_failed', 'provider_failed'}) diff --git a/application/single_app/functions_source_review.py b/application/single_app/functions_source_review.py index 44bae38fa..35e477bd1 100644 --- a/application/single_app/functions_source_review.py +++ b/application/single_app/functions_source_review.py @@ -533,20 +533,20 @@ def get_deep_research_config(settings: Optional[Dict[str, Any]]) -> Dict[str, An def capture_research_planner_configuration(*, settings, planner_client, planner_model, invocation_capture): - """Attest the actual research constructor only for non-model planning profiles.""" + """Attest the actual research planner constructor before planning or acquisition. + + Query and link planning keep their Source Review settings; the captured policy + records those flags, so a later settings change is detected at admission. + """ capture = require_invocation_capture(invocation_capture) if capture is None: return # These owners depend on Source Review; load them only after application initialization. - from functions_orchestration_external_configuration import ( - EXTERNAL_ACQUISITION_VERSION, is_research_acquisition_profile_supported, - ) + from functions_orchestration_external_configuration import EXTERNAL_ACQUISITION_VERSION from functions_orchestration_models import OrchestrationModelError, get_planner_acquisition_configuration try: - if not is_research_acquisition_profile_supported(settings): - capture.refuse() configuration = get_planner_acquisition_configuration(planner_client) if type(planner_model) is not str or planner_model != configuration["deployment"]: raise OrchestrationModelError() diff --git a/docs/admin/orchestration.md b/docs/admin/orchestration.md index cf82f31e0..312c7dfd1 100644 --- a/docs/admin/orchestration.md +++ b/docs/admin/orchestration.md @@ -120,26 +120,26 @@ recover one file without repeating completed work. ### Retained external-source authorization -Restart-safe reuse of web, URL, deep-research, agent, action, and explicit Fact -Memory results requires current account and app-role authorization, not the roles -saved when a plan was approved. The initialized root supplies current identity, -source-configuration, pre-effect authorization and capture readers for supported -external capabilities. - -The read-only Microsoft Graph identity reader uses the existing -application registration's confidential credentials and requires administrator -consent for the **application** permission `Directory.Read.All`. This is separate -from the delegated sign-in scopes. Orchestration does not request consent, grant -permissions, change role assignments, or widen the ordinary login scopes -automatically. Missing permission must leave affected external results -unavailable, rather than fall back to old session claims. - -This dependency is specific to retained external-source authorization; it is not -a new Graph permission requirement for document-backed or source-free results, -the shared file serializers, ordinary chat, or plans created by an earlier orchestration version. Group, -workspace, agent/action governance, and content-screening checks still apply -independently. See [Retained external source access]({{ '/explanation/features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS/' | relative_url }}) -for the server callback boundary and the current integration status. +Web search, linked-page reading, deep research, agents, actions and explicit Fact +Memory results use the same authority as classic chat: the app roles in the user's +signed-in session. Since **0.261.209**, orchestration no longer looks up each user in +Microsoft Graph and needs no `Directory.Read.All` permission. Earlier versions +required that application permission, and without it every such step failed with +"A required retained result is unavailable or changed." If you granted +`Directory.Read.All` to the app registration only for orchestration, you can revoke it. + +Each access still rechecks conversation ownership, Control Center access +restrictions, capability settings, and agent or action governance. A role change in +Entra ID takes effect when the user's session is refreshed, for example at their next +sign-in, as it does for classic chat. + +Work that continues in the background, such as a run recovered after a restart, has +no signed-in session. Its steps can't use web search, linked pages, deep research, +agents or actions, and they ask the user to send the request again. Document-backed +and source-free results are unaffected. Group, workspace, agent and action +governance, and content-screening checks still apply independently. See +[Retained external source access]({{ '/explanation/features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS/' | relative_url }}) +for the server callback boundary. ### Plan Approval {#chat-orchestration-approval-section} @@ -256,10 +256,12 @@ parts of a request. The step's rationale explains why that depth of gathering is choosing ordinary search does not start an automatic research-upgrade loop afterward. Discovery honours the existing Deep Research query limit and source-review limits in -[Knowledge settings]({{ '/admin/knowledge/' | relative_url }}). It does not enable web -search if that capability is disabled globally; permitted supplied sources can still be -reviewed. A query-planning model that is unavailable or fails leaves the existing backup -query generation available. Recovery is recorded in logs, without a user-facing fallback +[Knowledge settings]({{ '/admin/knowledge/' | relative_url }}), including its query +planning and linked-page planning options, as manual Deep Research does. It does not +enable web search if that capability is disabled globally; permitted supplied sources +can still be reviewed. Before **0.261.209**, those planners, which are on by default, +stopped every orchestrated research step before it started. A query-planning model +that is unavailable or fails leaves the existing backup query generation available. Recovery is recorded in logs, without a user-facing fallback notice when useful evidence is obtained. If no usable evidence is found, the answer must not claim that research verified the requested details. @@ -511,6 +513,8 @@ then report a rejected proposal. | A follow-up loses its subject | The relevant message is outside the history window, masked, inactive, or truncated. | Check Conversation History Limit in Chat settings and whether the earlier turn is still eligible. Repeat the missing detail if it is outside the retained context. | | A question is asked that was already answered | The earlier answer may be outside retained message history and the activity ledger. | Check the history window and ledger limits; the ledger alone does not contain the full earlier answer. | | A pending plan reports changed conversation context | A referenced message or its visibility changed after planning. | Create a new plan using the current conversation. | +| A web or research step says it continued in the background, where the user's sign-in is not available | The run continued without the user's browser session, for example after a restart, so their app roles couldn't be confirmed. | Ask the user to send the request again from the chat. The new run uses their signed-in session. | +| Every web search, linked-page or deep research step says a required retained result is unavailable or changed | Before 0.261.209, orchestration checked each user's roles through Microsoft Graph, which needs `Directory.Read.All`, and refused research whenever query or linked-page planning was on. | Upgrade to 0.261.209 or later; no Graph permission or settings change is needed. If a step still fails, check `sc_authority_reason` on its failure event in [orchestration failure diagnostics](../reference/logging-tags.md#orchestration-failure-diagnostics). | | Plans never propose deep research or reading a link | The user does not hold the required app role, or the capability is disabled in its own settings group. | Confirm the user holds `DeepResearchUser` or `UrlAccessUser` where your deployment requires them, and that the capability is enabled outside this page. | | Plans never propose an agent | Semantic Kernel is off, the user has turned agents off in their own settings, or the user has no agent they can reach. | Confirm Semantic Kernel is enabled, then check the user's own agent setting and that at least one agent is shared with them. | | The planner model shows "not in the current model list" | Its connection or model was removed or disabled, or connections were switched on or off since it was chosen. | Choose a listed model, or **Use the answer model (default)**. Until then planning keeps trying the saved model and fails rather than switching. | diff --git a/docs/explanation/features/ANALYZE_RESULTS.md b/docs/explanation/features/ANALYZE_RESULTS.md index fd12cd2e5..aa99c511e 100644 --- a/docs/explanation/features/ANALYZE_RESULTS.md +++ b/docs/explanation/features/ANALYZE_RESULTS.md @@ -207,7 +207,7 @@ Optional engine hooks were implemented in **0.261.127**, recorded in | Direct action | The actual authorized scoped manifest, the prepared loader input, and the constructed model transport/deployment/API version, factory protocol, model selectors and execution parameters. Each tool call repeats current authorization and capture before execution; changed configuration or refused capture stops the remaining calls. | | Local agent | The optional loader hook captures the actual effective configuration, prepared inline OpenAPI manifests and constructed Azure model binding for a fresh, explicit-tools-only kernel. The ordinary orchestration local-agent path remains withheld because its implicit core tools do not have complete configuration/source proofs. | | Classic Foundry agent/web search | The existing `get_agent` definition followed by the actual `ThreadRun` returned by the invocation's existing create/poll operations. Scoped remote agents first capture the actual `resolve_agent_config` result. Model, instructions, sampling and response format are explicit overrides; the SDK takes tools from an isolated definition. | -| Bounded research | The owning planner's actual constructor envelope, followed by each real Foundry search definition/run under the same `deep_research` producer. Query and link planning must not require planner-model calls. | +| Bounded research | The owning planner's actual constructor envelope, followed by each real Foundry search definition/run under the same `deep_research` producer. Since 0.261.209, query and link planning may call the planner model; the constructor is recaptured before planning and around every search and page fetch. | | Direct URL access | The effective bounded fetch and URL-access policy supplied to the real page reader. | These engine events use `orchestration-external-acquisition-v1`. A preparation @@ -318,16 +318,20 @@ definition/run events. Missing, closed, replaced or mutated construction bindings refuse execution; preparation alone cannot replace that proof or a missing search run. -The supported version-2 research subset uses deterministic query/link planning. -It refuses server profiles with web search, more than one search query and -`deep_research_enable_query_planning`, or both `enable_deep_source_review` and -`source_review_enable_llm_planning`. The planner tries different token and -temperature controls, and its model wrapper can further transform them. -Constructor defaults do not prove those effective request controls, so these -modes remain unavailable before search/page effects; the adapter does not -silently disable the configured planner. A one-query profile never invokes the -query planner. Multiple deterministic searches and deterministic link traversal -remain subject to the existing bounded review policy. +Before 0.261.209, the supported version-2 research subset used only +deterministic query/link planning and refused profiles with web search, more +than one search query and `deep_research_enable_query_planning`, or both +`enable_deep_source_review` and `source_review_enable_llm_planning`. The +default Knowledge settings matched those profiles, so orchestrated deep +research always failed. Since 0.261.209, both planners run as they do in manual +Deep Research. Each planner request follows a fresh capture of the pinned +planner construction, its token and temperature controls are fixed in code +rather than read from settings, and the captured research policy records the +planning flags so a mid-run settings change stops the step with +`external_configuration_changed`. A one-query profile never invokes the query +planner. Searches and link traversal remain subject to the existing bounded +review policy. See +[external source access](ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md#research-planner-requests). Optional `invocation_capture` flows through `perform_research_web_searches` and `perform_source_review`; the web helper forwards the existing diff --git a/docs/explanation/features/ORCHESTRATION_CHECKPOINT_RECOVERY.md b/docs/explanation/features/ORCHESTRATION_CHECKPOINT_RECOVERY.md index ba5718a19..337d3ed13 100644 --- a/docs/explanation/features/ORCHESTRATION_CHECKPOINT_RECOVERY.md +++ b/docs/explanation/features/ORCHESTRATION_CHECKPOINT_RECOVERY.md @@ -560,6 +560,13 @@ exception remains server-side in `__cause__`. | `ownership_lost` | The current execution no longer owns its guarded writes. | | `result_unavailable` | Retained source access was denied, integrity or ownership validation failed, or a definite screening hold applies; no preview may replace it. | +Since **0.261.209**, external-source identity comes from the signed-in session, +which a scheduler continuation doesn't have. A step, saved wait or final-answer +check refused only for that reason (`external_identity_session_unavailable`) +reports the step failure `external_session_required`, which asks the user to +send the request again, instead of `result_unavailable`. The refusal code is +logged as `sc_authority_reason`; it is never shown to the user. + Render authorization must translate `checkpoint_storage_unavailable` to its storage/service-unavailable path rather than returning `False` or raising an access-denied error. Unexpected exceptions from current-identity callbacks must diff --git a/docs/explanation/features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md b/docs/explanation/features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md index 07571d86d..41eee2036 100644 --- a/docs/explanation/features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md +++ b/docs/explanation/features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md @@ -4,6 +4,11 @@ `application/single_app/config.py`. Refs [#1509](https://github.com/microsoft/simplechat/issues/1509). +**Updated in version: 0.261.209.** External-source identity now uses the app +roles of the signed-in session, as classic chat does, instead of reading +Microsoft Graph on every call. Deep research can use its query and link planners +again. See the [session identity fix](../fixes/ORCHESTRATION_SESSION_IDENTITY_FIX.md). + ## Purpose and scope An external Gather result is the content an authorized adapter actually returned, @@ -31,7 +36,7 @@ The provider requires these owner-supplied callbacks: | Callback | Required result and boundary | | --- | --- | -| `read_identity(*, user_id, conversation_id)` | A new `CurrentExternalSourceIdentity` from a **current** account/app-role check on every call. Missing identity or loss of the User/Admin role denies access. | +| `read_identity(*, user_id, conversation_id)` | A new `CurrentExternalSourceIdentity` on every call. The application root supplies `SessionExternalIdentityReader`, which uses the signed-in session's app roles and point-reads conversation ownership and Control Center access on each call. No signed-in session, or no User/Admin role, denies access. | | `read_settings()` | Current normalized application settings, supplied by their owner. The provider never calls `get_settings()` or imports configuration to rediscover them. | | `read_conversation(id)` and `read_run(id)` | Current server records used by the existing result-access producer checks. The producer must belong to this actor/conversation and a recorded current-version plan. | | `read_configuration(source_type, *, producer, settings, source)` | An `ExternalSourceConfiguration` reconstructed from current configuration. `source` is the currently authorized agent/action record, or `None` for web/URL/deep research. Reads after restart must not require a live invocation-capture map. | @@ -89,7 +94,7 @@ conversation ownership/audience and current capability policy. Agent/action calls require the original exact server `catalog_key`/`action_ref`, a fresh catalog, matching authoritative saved-step selection, and the real exact scoped resolver. Catalog membership or a same-name replacement is not sufficient. -Known recorded audience changes are refused. Directory service failures and +Known recorded audience changes are refused. Identity service failures and cancellation propagate distinctly rather than becoming denied or empty results. The parent composes the acquisition operation into the existing hook, with @@ -164,86 +169,82 @@ Agent/action acquisition remains unavailable until its actual internal configuration proof is also supplied. Fact Memory retains its separate explicit scope/audience path; this method does not enable or implicitly read it. -### Deterministic research acquisition profile - -Captured research initially supports deterministic query and link selection. -This avoids paying for search or page acquisition when later LLM planner request -controls cannot be attested. Constructor `response_length`/`reasoning_effort` -defaults do not prove the source-review planner's temperature/token variants, -fallbacks, or negotiated reasoning controls. - -`is_research_acquisition_profile_supported(settings)` in -`functions_orchestration_external_configuration.py` is the shared pure support -predicate. It uses the existing `get_source_review_config` normalization and -clamping; it grants no authority and creates no invocation evidence. The exact -captured Source Review boundary is: - -- Query planning is unsupported when `enable_web_search` is true, the normalized - `deep_research_max_search_queries_per_turn` exceeds one, and - `deep_research_enable_query_planning` is true. With one query, or no web - discovery, an enabled query-planning flag cannot initiate a planner request. -- Link planning is unsupported when both `enable_deep_source_review` and - `source_review_enable_llm_planning` are true. A zero depth, one-page budget, or - an input that happens to contain no links does not relax this profile boundary. - -For example, multiple deterministic search queries require the existing query -planning flag to be false; deterministic deep link selection requires the -existing LLM link-planning flag to be false. No new settings or runtime flags are -introduced. Omitted values retain the source-review normalizer's existing -defaults, including enabled query/link planning and an eight-query limit. -Existing boolean-string normalization is reused, so `"false"` is not treated as -truthy. `enable_web_search` remains an exact boolean in the private configuration -contract. - -`preflight_gather_acquisition` validates **both** fresh current settings and the -actual invocation settings at every capture boundary, before current model or -Foundry metadata work. Unsupported profiles raise `ResultUnavailableError` with -code `external_configuration_research_profile_unsupported`. Settings, tools and -overrides are never rewritten to fit the supported subset. - -A supported profile still needs genuine `planner` / `resolved` construction -evidence and, with web discovery, the actual Foundry definition/run events under -the same `deep_research` producer. Deterministic planning does not remove those -proofs or the existing model, token, time, step and transport guards. It prevents -LLM query/link planning, not the explicitly attested Foundry search itself. - -This is a new-acquisition restriction, not a settings migration or a new -retained-read policy. Authorization-only preflight, `attestor.current`, saved -configuration comparison, and capture-free recovery remain unchanged. -Workflow Source Review remains outside this orchestration capture path and -retains its existing defaults and planner behavior. The regression matrix in -`functional_tests/test_orchestration_research_pre_effect.py` covers normalized -profiles, current/actual mismatch, changing current policy between events, zero -effects, and deterministic acquisition/restart. - -### Current role refresh is an integration requirement - -The existing orchestration route's `_request_identity()` captures session claims -for a request/worker. It is **not** a background role refresher. Reconstructing -`CurrentExternalSourceIdentity` from saved `RunContext.user_roles`, old login claims, -or persisted tokens would violate this provider's callback contract. - -The web and scheduler owners must supply a role/account reader that refreshes the -authorization state at each access, including after restart. If that cannot be -done, the callback must fail and the affected retained source remains unavailable. -There is no permissive default or fallback to cached role claims. - -## Read-only Microsoft Graph identity reader +### Research planner requests + +Before 0.261.209, captured research accepted only deterministic query and link +selection. With the default Knowledge settings, where query planning and LLM +link planning are both on, every orchestrated deep-research acquisition was +refused with `external_configuration_research_profile_unsupported` before it +started. The step then reported "A required retained result is unavailable or +changed." + +That restriction is removed. Orchestrated deep research runs the same query +planner and link planner as manual Deep Research, within the existing Knowledge +limits. The research engine calls `capture_research_planner_configuration` +before query planning starts and again before and after every search and page +fetch, so each planner request follows a fresh capture. Through the application +root's `capture` wrapper, every call runs a fresh `preflight_gather_acquisition` +and records `planner` / `resolved` evidence for the model client actually +constructed for the run. A planner request therefore still happens only after +current authorization and the actual planner deployment are attested. The +planner's request controls, a response-token cap and temperature fallbacks, +are fixed in code by `SOURCE_REVIEW_HARD_LIMITS` rather than read from settings. + +The research policy projected by `_fetch_policy` includes +`enable_deep_source_review`, `source_review_enable_llm_planning`, +`deep_research_enable_query_planning`, `deep_research_max_search_queries_per_turn`, +and the depth and URL limits. `validate_acquisition` compares that policy from +fresh current settings with the invocation's actual settings at every capture +boundary. If an administrator changes a planner setting while a step runs, the +next boundary raises `external_configuration_changed` before any further +search, page fetch or planner request. Settings, tools and overrides are never +rewritten to fit a profile. + +With web discovery, a research result still needs the actual Foundry definition +and run events under the same `deep_research` producer. The existing model, +token, time, step and transport guards are unchanged. Workflow Source Review +remains outside this orchestration capture path. + +`functional_tests/test_orchestration_research_pre_effect.py` covers the removed +gate, planner profiles, current/actual mismatch, a change after preparation, and +zero effects on refusal. `functional_tests/test_orchestration_research_capture.py` +asserts that each planner request happens only after its attestation and with +the attested deployment, and that the result is retained and recovered. + +### Signed-in session roles + +Classic chat authorizes web search and deep research with the app roles in the +user's signed-in session. Since 0.261.209, orchestration does the same. When the +application root builds services for a request, `build_external_identity_reader` +captures the session's `roles` and `preferred_username` while the Flask request +context exists, because execution then continues on a worker thread. The roles +are not saved on the run, and saved `RunContext.user_roles` are never restored. + +A role change in Entra ID therefore takes effect when the user's session picks +up new claims, for example at the next sign-in, as it does for classic chat. +There is no per-call directory lookup, and no Microsoft Graph application +permission is needed. + +A scheduler continuation, such as recovery after a restart, has no signed-in +session. Its reader has no roles, so every external-source read fails closed +with the refusal code `external_identity_session_unavailable`. The step reports +the `external_session_required` failure, which asks the user to send the request +again. Document-backed and source-free results don't use this reader and are +unaffected. + +## Signed-in session identity reader `application/single_app/functions_orchestration_external_identity.py` provides -`GraphExternalIdentityReader`. It implements the provider's `read_identity` -callback without trusting saved login roles or treating conversation ownership -as an application role. Construction validates its inputs but performs no I/O. +`SessionExternalIdentityReader`, the provider's `read_identity` callback. +Construction validates its inputs but performs no I/O, and the module makes no +directory calls. ```python -reader = GraphExternalIdentityReader( +reader = SessionExternalIdentityReader( user_id=user_id, conversation_id=conversation_id, - app_client_id=simplechat_app_client_id, - graph_base_url=graph_api_base, - graph_scope=graph_application_scope, - get_access_token=get_initialized_credential_token, - http_get=initialized_http_session.get, + roles=session_roles, + email=session_email, authorize_conversation=authorize_current_conversation, read_user_settings=read_current_user_settings, execution_check=check_current_execution, @@ -253,108 +254,78 @@ identity = reader(user_id=user_id, conversation_id=conversation_id) ``` Inject `reader` directly as `OrchestrationExternalSourceProvider(read_identity=...)`. -The actor and configured application client ID must be canonical lowercase GUID -strings. The conversation remains an exact, bounded application identifier. -Calling a reader with a different actor or conversation fails before token or -directory access. +Calling a reader with a different actor or conversation fails with +`external_identity_actor_mismatch` before any callback runs. ### Owner-supplied boundaries | Argument | Contract | | --- | --- | -| `app_client_id` | The SimpleChat application's client ID whose service principal and role definitions establish authority. This is not a role display name or an arbitrary application selected by a caller. | -| `graph_base_url` | Trusted HTTPS Graph API base including the configured version/path prefix, such as a national-cloud `/v1.0` base or an explicitly trusted Graph gateway. No endpoint is discovered from source content. | -| `graph_scope` | The configured HTTPS application resource scope ending in `/.default`. There is no global-cloud default or change to interactive OAuth scopes. | -| `get_access_token(scope)` | Returns the raw bearer **string**, for example the `access_token` from an owner-controlled MSAL application callback or `initialized_credential.get_token(scope).token`. Do not return a token-result dictionary or add a `Bearer ` prefix. The owner may cache tokens, never account or role results. | -| `http_get(url, *, headers, timeout, allow_redirects, stream)` | An initialized, non-caching `requests.Session.get`-compatible transport returning a `requests.Response`. The reader supplies bounded timeouts, `allow_redirects=False`, and `stream=True`; the transport must honor these and retain normal TLS verification. | -| `authorize_conversation(*, user_id, conversation_id)` | Revalidates the trusted requesting actor's current access and returns the owned server record with matching `id` and `user_id`. This must not merely echo caller-supplied IDs. It runs before directory requests and again before authority is returned. | +| `roles` | The session's app-role values, captured while a request context exists, or `None` when no signed-in session matches the actor. The bootstrap uses `capture_execution_identity`, which returns roles only when the session's `oid` equals the actor. Values are deduplicated, and entries that aren't bounded identifiers are dropped. | +| `email` | The session's `preferred_username`, or `None`. A value that isn't a bounded identifier, or that contains whitespace, becomes `None`. | +| `authorize_conversation(*, user_id, conversation_id)` | Revalidates the actor's current access and returns the owned server record with matching `id` and `user_id`, not deleted. It must not merely echo caller-supplied IDs. | | `read_user_settings(user_id)` | A fresh, scoped, read-only document with matching `id` and a dictionary `settings` field. Missing records, failures, malformed data, and wrong actors deny access. | -| `execution_check()` | Optional owning cancellation/lease/budget check. `None` or `True` means continue; `False` raises `ExternalIdentityCancelledError`. Owner control-flow exceptions propagate. | - -The application root can reuse the existing authentication configuration: -`functions_authentication._build_msal_app(cache=None, -authority_override=get_graph_authority())` supplies a configured confidential -client. Its creation and `acquire_token_for_client(scopes=[scope])` must remain -inside the injected token callback, reached only for an external identity read. -The lower reader never imports or constructs this authentication owner. -The root may retain the MSAL application/token cache without caching directory -authority. - -Pass the existing `get_graph_base_url()` result as the API base; it already -includes `/v1.0`, so do not append the version again. The token callback receives -the separately configured trusted Graph origin's `/.default` scope. Existing -`get_graph_authority()` and `get_graph_base_url()` preserve Public/Gov/custom -cloud selection. Do not reuse the interactive `SCOPE` list or create a new global -OAuth scope. Document/source-free operations must not invoke this token callback. -The root must translate MSAL error dictionaries into the safe denied/service -exception categories below, preserving transient `server_error` or -`temporarily_unavailable` failures without exposing `error_description`. - -Do not use the existing `get_user_settings()` as this reader's settings callback: -it can return a request-cached document and create or repair records. Do not call +| `execution_check()` | Optional owning cancellation, lease or budget check. `None` or `True` means continue; `False` raises `ExternalIdentityCancelledError`. Owner control-flow exceptions propagate. | + +The application root reads user settings with a Cosmos point read of +`cosmos_user_settings_container`, with 10-second connection and read timeouts +and no retries. Don't use `get_user_settings()` as this callback: it can return a +request-cached document and create or repair records. Don't call `check_user_access_status()` either: its broad error handler can default to -allow, and expired restrictions can trigger a write. The owner must instead -provide an authorized current point read using its initialized resources. - -The credential and settings/conversation callbacks must also bound their own -I/O. The reader cannot forcibly interrupt an arbitrary synchronous callback. -Its elapsed budget is checked around owner operations and while consuming -response bodies, so an over-budget operation cannot return authority afterward. - -### Directory authority and Control Center restrictions - -Every invocation reads the current user account, the service principal selected -by the exact configured `appId`, and the user's assignments for that resource. -The account and service principal must both be enabled. Exactly one service -principal must match. Only enabled role definitions that allow `User` members -can contribute role values; application-only, disabled, foreign-resource and -default-access assignments do not imply `User` or `Admin`. - -The [user app-role assignment API](https://learn.microsoft.com/en-us/graph/api/user-list-approleassignments?view=graph-rest-1.0) -includes grants through groups of which the user is a **direct** member. -The reader uses the documented `ConsistencyLevel: eventual` header and -`$count=true`, exhausts every page, and verifies the returned count before -returning roles. It does not infer nested-group grants. Canonical object IDs, -assignment principals, role shapes, duplicate definitions/assignments, missing -pages and ambiguous service principals are checked rather than coerced. - -A fresh `User` or `Admin` role is mandatory. Non-admins are denied by current -Control Center `settings.access.status="deny"` unless a valid, timezone-aware -`datetime_to_allow` has elapsed. Expiration permits access without changing the -stored restriction. Unknown statuses and malformed restrictions fail closed. -Only a freshly resolved `Admin` bypasses this access restriction, matching -`user_required`; it does not bypass account disablement or the need for a -current settings record. The `enable_agents` preference remains effective even -for admins. Its existing enabled default applies only to a valid current -document where that optional preference is absent, never to a failed read. - -### Bounded reads and safe failures - -Defaults are 20 total Graph requests/pages, 4,096 total directory records -(including role definitions), 1 MiB per response, a 10-second request timeout, -and a 30-second elapsed authorization budget. The corresponding constructor -options are `max_pages`, `max_records`, `max_response_bytes`, `request_timeout`, -and `max_elapsed`; each also has a finite upper bound. - -Every continuation URL must remain HTTPS, on the exact configured origin and -API path prefix, and on the same collection path. Credentials, fragments, -traversal, cycles, malformed links and cross-user/cross-collection continuations -are rejected before requesting another token. Redirects are not followed. -Response bodies are bounded, streamed, closed and decoded without accepting -duplicate JSON keys or non-finite JSON numbers. No error response body is used -as a public message. +allow, and expired restrictions can trigger a write. + +### Access checks on every call + +Each call applies these checks in order and returns a new identity only if all +of them pass: + +1. The actor and conversation match the reader, otherwise + `external_identity_actor_mismatch`. +2. A signed-in session was captured, otherwise + `external_identity_session_unavailable`. +3. The session holds no more than 64 roles, otherwise the non-retryable service + error `external_identity_limit_exceeded`. +4. The session holds `User` or `Admin`, otherwise + `external_identity_role_required`. +5. The conversation is still owned and not deleted, otherwise + `external_identity_conversation_unavailable`. +6. The user settings document is valid, otherwise + `external_identity_settings_unavailable`. +7. Control Center access allows the user, otherwise + `external_identity_access_restricted`. + +Non-admins are denied by `settings.access.status="deny"` unless a valid, +timezone-aware `datetime_to_allow` has elapsed. Expiration permits access without +changing the stored restriction. Unknown statuses and malformed restrictions fail +closed. Only `Admin` bypasses this restriction, matching `user_required`; it +doesn't bypass the need for a current settings record. The `enable_agents` +preference remains effective even for admins. Its enabled default applies only +to a valid current document where that optional preference is absent, never to +a failed read. + +### Safe failures | Failure | Parent-facing exception | | --- | --- | -| Missing current authority, disabled/deleted account or service principal, access restriction, HTTP 401/403/404/410 | Existing `ResultUnavailableError` (`PermissionError`) with a safe code. | -| Throttling, HTTP 5xx, network failures, timeouts or HTTP 202 incomplete work | `ExternalIdentityServiceError` with `retryable=True` and a safe `code`. These are service failures, not an empty role set or proof of revoked access. | -| Invalid/incomplete response shape, pagination, limits or callback contract | `ExternalIdentityServiceError` with `retryable=False`. No authority is returned. | +| No session, no User/Admin role, lost conversation, missing settings, access restriction, or an owner callback's `PermissionError` or HTTP 401/403/404/410 | `ResultUnavailableError` (`PermissionError`) with a safe code. | +| Throttling, HTTP 5xx, network failures, timeouts or HTTP 202 incomplete work in an owner callback | `ExternalIdentityServiceError` with `retryable=True` and a safe `code`. These are service failures, not proof of revoked access. | +| An invalid callback result or more than 64 session roles | `ExternalIdentityServiceError` with `retryable=False`. No authority is returned. | | Explicit owning cancellation | `ExternalIdentityCancelledError` (`InterruptedError`), distinct from access denial. | -Expected Azure/requests I/O exceptions are translated without preserving raw -provider text or credential-bearing exception chains. Parent code should map -these categories explicitly rather than silently hiding results on a transient -directory failure. +Expected Azure and requests I/O exceptions are translated without preserving raw +provider text or credential-bearing exception chains. + +### How a refusal reaches the user + +Acquisition capture normalizes an access refusal to +`OrchestrationInvocationDeniedError` with the code `result_unavailable`, keeping +only the refusal's stable code in `authority_reason`. `access_failure` in +`functions_orchestration_schema.py` reads that code, or the refusal's own `code`, +and follows at most three `__cause__` links. Only +`external_identity_session_unavailable` becomes the `external_session_required` +failure; every other refusal stays `result_unavailable`. Exception text is never +read, and the reason code is never shown to the user. Step, saved-wait, +finalization and composition failure events log it as `sc_authority_reason`. ### Invocation failure classification @@ -404,21 +375,20 @@ boundaries with actual current metadata and no planner requests. ### Deployment boundary and limitations -Using this reader requires an operator's explicit application-permission grant -of **Directory.Read.All** to an already-configured application identity, as -documented by Microsoft for the assignment API. Existing interactive read scopes -are insufficient. This implementation performs only GET requests: it does not -grant permissions, construct credentials, change login/scopes/settings, write -user records, assign roles, or enable orchestration. The parent owns initialized -factory wiring and operator-facing administration documentation; Chat -Orchestration remains default-off. - -Fresh HTTP reads do not eliminate Microsoft's directory replication delay: -the complete group-grant API uses eventual consistency. There is no local role -cache or fallback to old claims during that delay. Current source-specific -execution-configuration capture is a separate requirement; this reader does not -make post-execution current configuration equivalent to the configuration an -adapter actually used. +The session reader needs no Microsoft Graph permission, admin consent, or +deployer change. Before 0.261.209, a Graph reader made several directory +requests on every external-source call and required the **application** +permission `Directory.Read.All`. Without it, every orchestrated web search, URL +read and deep-research step failed as "A required retained result is +unavailable or changed." Deployments that granted `Directory.Read.All` only for +orchestration can revoke it. + +Trusting session roles has the same limits as classic chat. A role removed in +Entra ID still applies until the user's session is refreshed, and a background +continuation can't use external sources because it has no session. Current +source-specific execution-configuration capture is a separate requirement; this +reader doesn't make post-execution current configuration equivalent to the +configuration an adapter actually used. ## Invocation configuration and current metadata @@ -1036,14 +1006,15 @@ cover all external source types, crash-after-commit recovery, and current-role revocation in fresh normal and optimized Python processes. `functional_tests/test_orchestration_external_identity.py` exercises the real -reader through prepared HTTP requests and streamed `requests.Response` objects, -with only the HTTP adapter and owner I/O doubled. It covers direct/group grants, -the exact target application, current account/role/access revocation, read-only -restriction expiration, paging/counts/bounds/origin checks, malformed data, -transient versus denied failures and national/custom cloud configuration. -Fresh normal and optimized Python processes prohibit network access and -credential/client construction during real application-module imports. No test -contacts a tenant or grants a permission. +session reader with only the owner callbacks doubled and networking blocked. It +covers session role validation and bounds, the missing-session refusal, the +User/Admin requirement, actor and conversation binding, per-call conversation and +Control Center rereads, read-only restriction expiration, the Admin bypass, +settings that never default to allow, safe I/O error categories and cancellation. +Fresh normal and optimized Python processes import the real module without +settings, authentication, route or credential modules. No test contacts a tenant. +`functional_tests/test_orchestration_external_bootstrap.py` checks that the root +captures session roles only inside a request context and makes no Graph calls. `functional_tests/test_orchestration_external_configuration.py` covers actual URL policy normalization, source-specific projections, keyed opaque revisions, diff --git a/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md b/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md index 8f94a77ed..6d156ba07 100644 --- a/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md +++ b/docs/explanation/features/ORCHESTRATION_GATHER_REASON_RENDER.md @@ -159,16 +159,15 @@ Malformed server catalog metadata remains an operational configuration error: HTTP callers return a safe 503 rather than reporting changed sources or claiming the plan. Valid but unadmitted pairs remain explicit plan refusals. -For retained external sources, the root also provides a lazy current-directory -identity reader using the configured application and Graph cloud. It reads -current account/app-role authority and uncached, read-only user restrictions; -it never substitutes saved session roles. Credential, directory, and metadata -I/O have explicit bounds, and temporary service failures remain distinct from -denied access. Document/source-free service construction does not acquire a -Graph token. This optional path requires operator-consented application -permissions as described in -[external source access](ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md); it does not -grant permissions or alter interactive sign-in scopes. +For retained external sources, the root also provides a lazy identity reader +over the signed-in session's app roles, the same authority classic chat uses. +Since 0.261.209 it makes no Microsoft Graph calls and needs no application +permission. Roles are captured while the request context exists and are never +restored from a saved run, so a background continuation fails closed and asks +the user to send the request again. Conversation ownership and uncached, +read-only user restrictions are read on every access, with explicit I/O +bounds, and temporary service failures remain distinct from denied access. +See [external source access](ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md). Initialized services compose the independent current-metadata reader, configuration attestor and external-source provider. Before capture, the provider diff --git a/docs/explanation/fixes/ORCHESTRATION_SESSION_IDENTITY_FIX.md b/docs/explanation/fixes/ORCHESTRATION_SESSION_IDENTITY_FIX.md new file mode 100644 index 000000000..fd3b4f0cc --- /dev/null +++ b/docs/explanation/fixes/ORCHESTRATION_SESSION_IDENTITY_FIX.md @@ -0,0 +1,164 @@ +# Orchestration Web Search and Deep Research Access Fix + +Fixed in version: **0.261.209** + +Application version reference: `application/single_app/config.py`. +Refs [#1509](https://github.com/microsoft/simplechat/issues/1509). + +## Issue + +In V2 chat, orchestrated runs that used web search, a linked page or deep research +failed with "A required retained result is unavailable or changed. No preview was +substituted." The same requests worked in classic chat. The message didn't say +which part failed, so it looked like a web search or deep research defect. + +## Root cause + +Two separate refusals stopped these steps before any search ran. Both were +reported with the same generic `result_unavailable` failure, and the reason wasn't +logged. + +1. **A directory lookup on every call.** Each external-source access ran + `GraphExternalIdentityReader`. It read the user, the app's service principal and + the user's app-role assignments from Microsoft Graph with an app-only token. + That needs the **application** permission `Directory.Read.All`, which + deployments don't grant by default. Graph returned HTTP 403, the reader raised + `external_identity_access_denied`, and every web search, URL read and deep + research step failed. Classic chat never makes this call: it trusts the app + roles in the signed-in session. The per-call lookup also added several Graph + requests to every step, which doesn't scale to large tenants. +2. **A deep research planner gate.** `is_research_acquisition_profile_supported()` + refused a research acquisition whenever query planning (with more than one + query) or LLM link planning was on. Both are on by default in Knowledge + settings, so every orchestrated deep research step was refused with + `external_configuration_research_profile_unsupported`. + +## Changes + +### Signed-in session roles, as classic chat + +`SessionExternalIdentityReader` replaces `GraphExternalIdentityReader`. +`build_external_identity_reader` captures the session's app roles and +`preferred_username` while the request context exists, because the run then +continues on a worker thread. Each access still point-reads conversation +ownership and the user's settings, and applies Control Center access +restrictions (with the Admin bypass that `user_required` has) and the +`enable_agents` preference. There are no Graph calls, tokens or MSAL clients, +and roles are never saved on the run. + +A scheduler continuation, such as recovery after a restart, has no signed-in +session. Its reads fail closed with the refusal code +`external_identity_session_unavailable`. Saved run roles are never restored. + +### Deep research planners admitted + +The profile gate is removed from `validate_acquisition` and +`capture_research_planner_configuration`, and the predicate is deleted. The +research engine still captures the planner before query planning and around +every search and page fetch, so each planner request follows a fresh +`preflight_gather_acquisition` and `planner` / `resolved` attestation of the +model client actually constructed. The planner's token and temperature controls +are fixed in code, not read from settings. The planner settings are part of the +captured research policy, so an administrator change during a run stops the next +boundary with `external_configuration_changed` before further searches, page +fetches or planner requests. + +### A clear message for background refusals + +`OrchestrationInvocationDeniedError` keeps the refusal's stable code in +`authority_reason`, including when a sticky capture failure is raised again. The +new `access_failure()` in `functions_orchestration_schema.py` maps only +`external_identity_session_unavailable` to the new `external_session_required` +failure: + +> This step continued in the background, where your sign-in is not available to +> confirm access to web search, web pages, deep research, agents or actions. Send +> the request again to use them. + +Every other refusal stays `result_unavailable`. Dependency steps, saved waits, +final-answer verification, content preparation and image steps use this mapping. +Step, saved-wait, finalization and content-preparation failure events log the +code as `sc_authority_reason`. Exception text is never read and the code is +never shown to the user. + +### Directory permission work reverted + +An earlier, unreleased attempt on this branch granted `Directory.Read.All`, added +it to the deployers, and warned admins when it was missing. That work was +reverted, because orchestration no longer needs the permission. No deployer, +setting or app registration change is needed for this fix. + +## Files modified + +| File | Change | +| --- | --- | +| `functions_orchestration_external_identity.py` | `SessionExternalIdentityReader` replaces the Graph reader. The safe error types and codes are kept; their messages now say "Current user authorization" instead of "Current directory authorization". | +| `functions_orchestration_bootstrap.py` | Captures session roles inside a request context and builds the session reader. Graph, MSAL and `requests` use is removed. | +| `functions_orchestration_external_configuration.py` | Deletes `is_research_acquisition_profile_supported` and its refusal in `validate_acquisition`. | +| `functions_source_review.py` | `capture_research_planner_configuration` attests the planner without refusing a profile. | +| `functions_orchestration_invocation_capture.py` | `OrchestrationInvocationDeniedError(authority_reason)` keeps the refusal's validated code. | +| `functions_orchestration_result_contracts.py` | Adds `EXTERNAL_SESSION_UNAVAILABLE_REASON`. | +| `functions_orchestration_schema.py` | Adds the `external_session_required` failure and `access_failure()`. | +| `functions_orchestration_executor.py`, `functions_orchestration_composition.py` | Use `access_failure()` and log `authority_reason` on failure events, falling back to a `ResultUnavailableError` code. | +| `functions_orchestration_images.py` | Uses `access_failure()` for refused inputs. | +| `functions_orchestration_external_sources.py` | Documents the session-based identity contract. | +| `config.py` | Version `0.261.209`. | + +## Validation + +Updated and new functional tests: + +- `test_orchestration_external_identity.py`: the real session reader, including + the missing-session refusal, role bounds, per-call rereads, restrictions and + fresh normal and optimized imports with networking blocked. +- `test_orchestration_external_bootstrap.py`: the real root captures roles only in + a request context, keeps them for the worker thread, refuses background, + anonymous and other-user sessions before any read, and makes no directory calls. +- `test_orchestration_research_pre_effect.py`: the gate is gone, planner profiles + are admitted, a profile change on either side or after preparation is refused + with no effects, turning off either planner switch alone on one side is + refused, and default research settings run both planners. +- `test_orchestration_research_capture.py`: each planner request happens only + after its attestation, with the attested deployment, and the result is + retained and recovered. +- `test_orchestration_dependency_runtime.py`, + `test_orchestration_source_authority_runtime.py`, + `test_orchestration_external_preflight_adapter.py`: the session refusal reaches + steps, saved waits, finalization and composition as `external_session_required`, + while other refusals stay `result_unavailable`, and each failure event logs the + refusal code as `authority_reason`. +- `test_orchestration_image_references.py`: an image step whose retained input is + refused for a missing session reports `external_session_required` before any + image is generated. +- `test_orchestration_external_error_markers.py`: the safe identity service and + cancellation messages say "Current user authorization". + +These nine files pass together: 674 tests and 92 subtests. Each of 31 deliberate +code breaks makes at least one of them fail. The breaks include trusting a +missing session, skipping the role check, reading exception text, and leaving a +planner switch out of the compared settings. The documentation coverage (7/7) +and site quality (6/6) checks also pass. + +### Before and after + +| Before | After | +| --- | --- | +| Every orchestrated web search, URL read and deep research step made several Microsoft Graph calls and failed without `Directory.Read.All`. | Steps use the signed-in session's app roles, as classic chat does, with no Graph calls. | +| Default Knowledge settings refused every orchestrated deep research step. | Query and link planners run after attestation, within existing limits. | +| All refusals showed "A required retained result is unavailable or changed." with no logged reason. | A missing session asks the user to send the request again, and step, saved-wait, finalization and content-preparation failures log `sc_authority_reason`. | + +## Limitations + +- A role removed in Entra ID applies until the user's session is refreshed, as + in classic chat. +- A background continuation can't use web search, linked pages, deep research, + agents or actions, because it has no signed-in session. +- If `Directory.Read.All` was granted to the app registration only for + orchestration, it can be revoked. + +## Related documentation + +- [Retained external source access](../features/ORCHESTRATION_EXTERNAL_SOURCE_ACCESS.md) +- [Checkpoint recovery](../features/ORCHESTRATION_CHECKPOINT_RECOVERY.md) +- [Orchestration settings](../../admin/orchestration.md) +- [Logging tags](../../reference/logging-tags.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 9d3f4cce6..cc1262e02 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,17 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.209)** + +#### Bug Fixes + +* **Orchestrated Web Search And Deep Research Access** + * In V2 chat, orchestrated web search, linked-page and deep research steps failed with "A required retained result is unavailable or changed." Orchestration looked up each user's app roles in Microsoft Graph on every call, which needs the `Directory.Read.All` application permission that deployments don't grant by default. Classic chat never makes that call. + * Orchestration now trusts the app roles in the signed-in session, as classic chat does. Each access still rechecks conversation ownership and Control Center access restrictions. No Graph permission, deployer change or setting is needed, and a `Directory.Read.All` grant made only for orchestration can be revoked. + * Orchestrated deep research also refused to start whenever query planning or linked-page planning was on, which is the default. Both planners now run within the existing Knowledge limits, each planner request still follows a fresh attestation of the planner model, and a planning-setting change during a run stops the step. + * A step that continues in the background, where there is no signed-in session, now says so and asks the user to send the request again instead of showing the generic message. Step, saved-wait, finalization and content-preparation failure events log the refusal code as `sc_authority_reason`. + * (Ref: #1509, `functions_orchestration_external_identity.py`, `functions_orchestration_bootstrap.py`, `functions_orchestration_external_configuration.py`, `functions_source_review.py`, `access_failure`, [Orchestration Web Search and Deep Research Access Fix](fixes/ORCHESTRATION_SESSION_IDENTITY_FIX.md)) + ### **(v0.261.208)** #### New Features diff --git a/docs/reference/logging-tags.md b/docs/reference/logging-tags.md index 751594bb0..db131ede5 100644 --- a/docs/reference/logging-tags.md +++ b/docs/reference/logging-tags.md @@ -315,7 +315,7 @@ Insights `customDimensions` or Log Analytics `AppTraces.Properties`. | `sc_attempt` | Planner proposal number: 1 for the initial proposal and 2 for its single correction. Present on deliverables correction/failure events. | | `sc_execution_code`, `sc_output_code` | Safe execution or output-store failure category, where available. | | `sc_failure_code` | On step and adapter events, a step failure category, such as `step_timeout`, `run_timeout`, or `context_unavailable`. On `[CONTENT_SCREENING]` events, the source-authority code, such as `source_authority_unverified`, `source_authority_unavailable`, or `screening_revision_conflict`. | -| `sc_authority_reason` | Which check failed for a source. With `source_authority_unverified`, the check that could not verify the source's current authority: `document_record_invalid`, `screening_marker_invalid`, `screening_state_unknown`, `screening_marker_incomplete`, `document_scope_invalid`, `workspace_record_invalid`, `scan_record_invalid`, `provenance_mismatch`, `document_id_invalid`, `manifest_context_invalid`, `manifest_scope_missing`, `manifest_revision_invalid`, or `manifest_batch_invalid`. With `screening_revision_conflict`, what saved evidence no longer matches in the current document: `source_revision_changed` (the document was re-versioned), `screening_generation_changed` (it was re-screened or remediated), `source_scope_changed` (its id or workspace differs), `provenance_shape_invalid` (the saved provenance has a different set of fields), `screened_record_changed` (a saved copy of the screened record differs), `source_version_changed` (a legacy reference without provenance), or `cached_evidence_unproven` (cached evidence from before screening enrollment). | +| `sc_authority_reason` | Which check failed for a source. With `source_authority_unverified`, the check that could not verify the source's current authority: `document_record_invalid`, `screening_marker_invalid`, `screening_state_unknown`, `screening_marker_incomplete`, `document_scope_invalid`, `workspace_record_invalid`, `scan_record_invalid`, `provenance_mismatch`, `document_id_invalid`, `manifest_context_invalid`, `manifest_scope_missing`, `manifest_revision_invalid`, or `manifest_batch_invalid`. With `screening_revision_conflict`, what saved evidence no longer matches in the current document: `source_revision_changed` (the document was re-versioned), `screening_generation_changed` (it was re-screened or remediated), `source_scope_changed` (its id or workspace differs), `provenance_shape_invalid` (the saved provenance has a different set of fields), `screened_record_changed` (a saved copy of the screened record differs), `source_version_changed` (a legacy reference without provenance), or `cached_evidence_unproven` (cached evidence from before screening enrollment). On `[ORCHESTRATION_EXECUTOR]` step, saved-wait, finalization and content-preparation failure events (since 0.261.209), the stable code of a refused external source, such as `external_identity_session_unavailable` (the step ran in a background continuation with no signed-in session), `external_identity_role_required`, `external_identity_access_restricted`, `external_identity_conversation_unavailable`, `external_identity_settings_unavailable`, or `external_configuration_changed`. | | `sc_capability_id` | The capability the step ran, such as `compose`, `render_file`, `generate_image`, or `document_compare`. | | `sc_output_format` | The file format of a render attempt, such as `csv`, `xlsx`, or `docx`. | | `sc_error_type`, `sc_response_type` | Exception class and, on runner admission/preparation failures, the record's Python type. | @@ -379,7 +379,7 @@ file names, file content, prompts, or model responses. | `[ORCHESTRATION_EXECUTOR] A file render attempt finished.` | Information when the file completed; otherwise Warning | `sc_status`, `sc_output_code`, `sc_output_format`, `sc_size_bytes`, and the timing and check counts described below. | | `[ORCHESTRATION_EXECUTOR] A step finished after its time budget; its finished result was kept.` | Warning | `sc_failure_code` (`step_timeout` or `run_timeout`), `sc_elapsed_ms`, `sc_step_timeout_seconds`. | | `[ORCHESTRATION_EXECUTOR] Prepared content did not match its declared outputs; asking once more.` | Information | `sc_reason` (`compose_output_retry`), `sc_error_type`, and `sc_execution_code` when the mismatch has one. | -| `[ORCHESTRATION_EXECUTOR] Content preparation could not complete.` | Warning | `sc_failure_code`, `sc_execution_code`, `sc_error_type`. | +| `[ORCHESTRATION_EXECUTOR] Content preparation could not complete.` | Warning | `sc_failure_code`, `sc_execution_code`, `sc_error_type`, and `sc_authority_reason` when a retained source was refused. | | `[ORCHESTRATION] Retrying the answer model without a JSON response format.` | Information | `sc_reason` (`json_format_retry`), `sc_stage`, `sc_error_type`. Content preparation asks the endpoint for a JSON object; an endpoint that refuses that option is asked again without it. | | `[ORCHESTRATION_ADAPTERS] Native result bridge did not complete.` | Warning | `sc_stage`, `sc_failure_code`, `sc_execution_code`, such as `native_compute_source_identity_mismatch` when a saved spreadsheet location now points at a different revision. | | `[CONTENT_SCREENING] Current source authority could not be verified.` | Warning | `sc_failure_code`, `sc_authority_reason`, `sc_error_type`. | diff --git a/functional_tests/test_orchestration_dependency_runtime.py b/functional_tests/test_orchestration_dependency_runtime.py index b406d5e7d..9a7cfd33f 100644 --- a/functional_tests/test_orchestration_dependency_runtime.py +++ b/functional_tests/test_orchestration_dependency_runtime.py @@ -1,11 +1,12 @@ # test_orchestration_dependency_runtime.py """Real Gather / Reason / Render compiler, executor, composition, retained readers and checkpoints. -Version: 0.261.191 +Version: 0.261.209 Implemented in: 0.261.127 Pending-Gather regression implemented in: 0.261.129 Single orchestration contract updated in: 0.261.139 Partial-input refusal diagnostics implemented in: 0.261.191 +Missing signed-in session failure messages implemented in: 0.261.209 External model/search/storage I/O is isolated; no paid or provider calls. """ @@ -69,7 +70,7 @@ def __call__(self, messages, **kwargs): def make( steps, replies=(), *, final_response=None, settings=None, aliases=None, - profiles=None, profile_validator=None, + profiles=None, profile_validator=None, available=None, ): settings = {**SETTINGS, **(settings or {})} raw = {'steps': deepcopy(steps), 'run_id': 'run-1', 'plan_id': 'plan-1', 'turn_id': 'turn-1'} @@ -77,7 +78,9 @@ def make( raw['final_response'] = final_response plan = schema.normalize_plan( raw, 'conversation-1', 'owner', settings=settings, contract_version=2, - available_capability_ids=['compose', 'document_search', 'document_analyze', 'document_compare'], + available_capability_ids=list(available) if available is not None else [ + 'compose', 'document_search', 'document_analyze', 'document_compare', + ], existing_results=aliases, composition_profiles=profiles, ) fixture = ResultFixture(blob=True) @@ -320,6 +323,126 @@ def attempted_file(step, context, **kwargs): assert after is True +SESSION_REASON = 'external_identity_session_unavailable' +SESSION_MESSAGE = 'where your sign-in is not available to confirm access' + + +@pytest.mark.parametrize('reason, code', [ + (SESSION_REASON, 'external_session_required'), + ('external_identity_role_required', 'result_unavailable'), + ('external_identity_access_denied', 'result_unavailable'), + (None, 'result_unavailable'), +]) +def test_background_source_refusal_asks_the_user_to_send_the_request_again(runtime, monkeypatch, reason, code): + """Version 0.261.209: only a missing signed-in session gets its own explanation.""" + from functions_orchestration_invocation_capture import OrchestrationInvocationDeniedError + case = runtime.make( + [{'step_id': 'search', 'capability_id': 'web_search', 'arguments': {'query': 'Find current facts.'}}], + settings={'enable_web_search': True}, available=['compose', 'web_search'], + ) + # The executor offers web search again only when every server acquisition guard is present. + case.context.external_source_admission = lambda **kwargs: None + case.context.external_source_preflight = lambda **kwargs: None + case.context.capture_external_source_configuration = lambda *args, **kwargs: None + case.context.result_service.access.external_source_authorizer = lambda *args, **kwargs: None + logs = [] + monkeypatch.setattr( + runtime.executor, 'log_event', lambda message, **kwargs: logs.append((message, kwargs.get('extra') or {})), + ) + + def refused(step, context, **kwargs): + raise OrchestrationInvocationDeniedError(reason) + + result = execute(runtime, case, get_adapter=lambda capability: refused) + assert result['status'] == 'failed' + assert result['steps'][0]['failure']['code'] == result['failure']['code'] == code + assert runtime.schema.FAILURE_MESSAGES[code] in result['message'] + assert (SESSION_MESSAGE in result['message']) is (code == 'external_session_required') + assert case.model.calls == [] + # Diagnostics carry the stable refusal code as sc_authority_reason, never the message. + [event] = [extra for message, extra in logs if message.endswith('A dependency-bound step could not complete.')] + assert event['failure_code'] == code and event['authority_reason'] == reason + + +@pytest.mark.parametrize('reason, code', [ + (SESSION_REASON, 'external_session_required'), + ('external_identity_access_denied', 'result_unavailable'), +]) +def test_finalization_explains_a_missing_signed_in_session(runtime, monkeypatch, reason, code): + from functions_orchestration_results import ResultUnavailableError + case = runtime.make([compose()], ['Retained answer.'], final_response=binding('draft')) + logs = [] + monkeypatch.setattr( + runtime.executor, 'log_event', lambda message, **kwargs: logs.append((message, kwargs.get('extra') or {})), + ) + + def refuse(*args, **kwargs): + raise ResultUnavailableError(reason) + + def event(value): + # Only finalization reopens the retained answer after its producer completes. + if value.get('step_id') == 'draft' and value.get('phase') == 'completed': + monkeypatch.setattr(case.context.result_service, 'open_result', refuse) + + result = execute(runtime, case, emit=event) + assert result['status'] == 'failed' + assert result['steps'][0]['status'] == 'completed' + assert result['failure'] == runtime.schema.build_failure(code) + assert runtime.schema.FAILURE_MESSAGES[code] in result['message'] + assert 'Retained answer.' not in result['message'] + assert len(case.model.calls) == 1 + [logged] = [extra for message, extra in logs if message.endswith('could not be reauthorized for finalization.')] + assert logged['failure_code'] == code and logged['authority_reason'] == reason + + +@pytest.mark.parametrize('reason, code', [ + (SESSION_REASON, 'external_session_required'), + ('external_identity_access_denied', 'result_unavailable'), +]) +def test_composition_explains_a_missing_signed_in_session(runtime, monkeypatch, reason, code): + from functions_orchestration_results import ResultUnavailableError + logs = [] + monkeypatch.setattr( + runtime.composition, 'log_event', lambda message, **kwargs: logs.append((message, kwargs.get('extra') or {})), + ) + # Stands in for a retained external input whose recheck is refused while content is prepared. + case = runtime.make([compose()], [ResultUnavailableError(reason)], final_response=binding('draft')) + result = execute(runtime, case) + assert result['status'] == 'failed' + assert result['steps'][0]['failure']['code'] == code + assert runtime.schema.FAILURE_MESSAGES[code] in result['message'] + assert len(case.model.calls) == 1 + [logged] = [extra for message, extra in logs if message.endswith('Content preparation could not complete.')] + assert logged['failure_code'] == code and logged['authority_reason'] == reason + + +def test_access_failure_reads_only_stable_reason_codes(runtime): + from functions_orchestration_invocation_capture import OrchestrationInvocationDeniedError + from functions_orchestration_results import ResultUnavailableError + schema = runtime.schema + expected = schema.build_failure('external_session_required') + assert schema.access_failure(ResultUnavailableError(SESSION_REASON)) == expected + assert schema.access_failure(OrchestrationInvocationDeniedError(SESSION_REASON)) == expected + try: + try: + raise ResultUnavailableError(SESSION_REASON) + except ResultUnavailableError as inner: + raise RuntimeError('Wrapped retained read failure.') from inner + except RuntimeError as wrapped: + assert schema.access_failure(wrapped) == expected + unavailable = schema.build_failure('result_unavailable') + lookalike = PermissionError(SESSION_REASON) + lookalike.authority_reason = f'{SESSION_REASON} ' + assert schema.access_failure(lookalike) == unavailable + assert schema.access_failure(ResultUnavailableError('external_identity_access_denied')) == unavailable + assert schema.access_failure(OrchestrationInvocationDeniedError()) == unavailable + looping = PermissionError('Cycle') + looping.__cause__ = looping + assert schema.access_failure(looping) == unavailable + assert expected['message'] == schema.FAILURE_MESSAGES['external_session_required'] + assert SESSION_REASON not in expected['message'] + + def test_pending_work_is_waiting_not_a_successful_preview_or_synthesis(runtime): case = runtime.make([ compose('native'), compose('answer', inputs={'data': source_input('native', 'answer')}), diff --git a/functional_tests/test_orchestration_external_bootstrap.py b/functional_tests/test_orchestration_external_bootstrap.py index b4c8b6519..e846b703a 100644 --- a/functional_tests/test_orchestration_external_bootstrap.py +++ b/functional_tests/test_orchestration_external_bootstrap.py @@ -1,120 +1,157 @@ # test_orchestration_external_bootstrap.py """ -Application-owned current identity and acquisition wiring for retained results. -Version: 0.261.140 +Functional test for application-owned session identity and acquisition wiring on retained results. +Version: 0.261.209 Implemented in: 0.261.127 Single orchestration contract updated in: 0.261.139 +Signed-in session roles replaced per-call Microsoft Graph reads in: 0.261.209 -Runs the real bootstrap, authentication factory and directory reader with only -the MSAL client, Cosmos storage and HTTP wire doubled. No tenant calls, consent -changes, settings repairs, cached role fallback or interactive scope changes. +Runs the real bootstrap, session capture, current-access reader, retention and +result store with only Cosmos storage doubled and networking blocked. Roles come +from the signed-in session, as in classic chat, so no MSAL client or directory +call is made. Background continuations without that session fail closed with a +distinct reason instead of restoring saved roles. Refs microsoft/simplechat#1509. """ import hashlib import hmac import importlib -import json +import threading +from contextlib import contextmanager from copy import deepcopy from dataclasses import replace from types import SimpleNamespace from unittest.mock import Mock -from urllib.parse import urlsplit import pytest from azure.core import MatchConditions from azure.core.exceptions import AzureError +from flask import Flask, session from requests.exceptions import Timeout from functions_orchestration_external_identity import ExternalIdentityServiceError from functions_orchestration_invocation_capture import OrchestrationInvocationServiceError -from functions_orchestration_result_contracts import canonical_digest +from functions_orchestration_result_contracts import EXTERNAL_SESSION_UNAVAILABLE_REASON, canonical_digest from functions_orchestration_results import ResultUnavailableError from test_orchestration_external_identity import ( - APP_ID, - CONVERSATION_ID, - GraphWorld, - TOKEN, - URL_ROLE, - USER_ID, - assignment, + CONVERSATION_ID, EMAIL, OTHER_ID, SECRET, USER_ID, SessionWorld, read_identity, ) -from test_orchestration_harness_routes import modules +from test_orchestration_harness_routes import modules # noqa: F401 from test_support.orchestration_results import ResultContainer from test_support.orchestration_revisions import AtomicMemoryContainer +from test_support.versioning import assert_app_version_at_least + + +URL_ROLES = ("User", "UrlAccessUser") +POINT_READ = {"connection_timeout": 10.0, "read_timeout": 10.0, "retry_total": 0} +APP_ID = "b2222222-2222-4222-8222-222222222222" class BoundedMemoryContainer(AtomicMemoryContainer): def read_item(self, item, partition_key, **options): - if options and options != { - "connection_timeout": 10.0, "read_timeout": 10.0, "retry_total": 0, - }: + if options and options != POINT_READ: raise AssertionError("Unexpected point-read options.") return super().read_item(item, partition_key) @pytest.fixture def external_root(modules, monkeypatch): - with GraphWorld() as world: + with SessionWorld() as world: conversations = BoundedMemoryContainer("id") settings = BoundedMemoryContainer("id") runs = AtomicMemoryContainer("conversation_id") + result_items = ResultContainer() conversations.create_item(deepcopy(world.conversation)) settings.create_item(deepcopy(world.settings)) settings_read = Mock(wraps=settings.read_item) + conversation_read = Mock(wraps=conversations.read_item) monkeypatch.setattr(settings, "read_item", settings_read) - token = Mock(return_value={"access_token": TOKEN}) - client = SimpleNamespace(acquire_token_for_client=token) - client_factory = Mock(return_value=client) - root = modules.bootstrap - monkeypatch.setattr(modules.config, "cosmos_conversations_container", conversations) - monkeypatch.setattr(modules.config, "cosmos_user_settings_container", settings) - monkeypatch.setattr(modules.config, "cosmos_orchestration_runs_container", runs) + monkeypatch.setattr(conversations, "read_item", conversation_read) + directory_client = Mock(side_effect=AssertionError("Session identity must not build an MSAL client.")) + for name, value in { + "cosmos_conversations_container": conversations, + "cosmos_user_settings_container": settings, + "cosmos_orchestration_runs_container": runs, + "cosmos_personal_workflow_run_items_container": result_items, + }.items(): + monkeypatch.setattr(modules.config, name, value) monkeypatch.setattr(modules.runs, "cosmos_orchestration_runs_container", runs) - result_items = ResultContainer() - monkeypatch.setattr(modules.config, "cosmos_personal_workflow_run_items_container", result_items) - monkeypatch.setattr(modules.auth, "CLIENT_ID", APP_ID) - monkeypatch.setattr(modules.auth, "CLIENT_SECRET", "synthetic-client-secret") - monkeypatch.setattr(modules.auth, "ConfidentialClientApplication", client_factory) - monkeypatch.setattr(modules.auth, "get_graph_base_url", lambda: world.base_url) - monkeypatch.setattr(modules.auth, "get_graph_authority", lambda: "https://login.example.test/tenant") - monkeypatch.setattr(root.requests, "get", world.session.get) + monkeypatch.setattr(modules.auth, "ConfidentialClientApplication", directory_client) + app = Flask(__name__) + app.config.update(TESTING=True, SECRET_KEY="local-session-identity-test") yield SimpleNamespace( - root=root, auth=modules.auth, world=world, settings=settings, - settings_read=settings_read, conversations=conversations, - token=token, client_factory=client_factory, runs=runs, result_items=result_items, + root=modules.bootstrap, auth=modules.auth, app=app, world=world, + settings=settings, settings_read=settings_read, + conversations=conversations, conversation_read=conversation_read, + directory_client=directory_client, runs=runs, result_items=result_items, ) -def reader_for(runtime): - return runtime.root.build_external_identity_reader(USER_ID, CONVERSATION_ID) +@contextmanager +def signed_in(runtime, *, oid=USER_ID, roles=("User",), email=EMAIL): + """Enter a request whose session has the app's MSAL sign-in shape.""" + with runtime.app.test_request_context("/api/orchestration/runs"): + user = {"oid": oid, "preferred_username": email, "name": "Session Owner"} + if roles is not None: + user["roles"] = list(roles) + session["user"] = user + session["token_cache"] = SECRET + yield -def read_identity(reader): - return reader(user_id=USER_ID, conversation_id=CONVERSATION_ID) +def in_request(runtime, build, *, signed_in_session=True, **claims): + if not signed_in_session: + return build() + with signed_in(runtime, **claims): + return build() -def test_source_free_service_construction_never_acquires_directory_authority(external_root): - runtime = external_root - unused_reader = reader_for(runtime) - services = runtime.root.build_orchestration_services( - USER_ID, CONVERSATION_ID, settings={"max_generated_chat_artifact_size_mb": 1}, +def reader_for(runtime, **options): + return in_request( + runtime, lambda: runtime.root.build_external_identity_reader(USER_ID, CONVERSATION_ID), **options, ) - catalog = services.export_catalog() - bindings = services.capability_request_bindings() - assert callable(unused_reader) - assert catalog - assert all(callable(bindings[name]) for name in ( - "external_source_preflight", - "external_source_admission", "external_source_authorizer", - "capture_external_source_configuration", - )) - assert runtime.world.adapter.requests == [] - runtime.client_factory.assert_not_called() - runtime.settings_read.assert_not_called() -def retained_url_runtime(runtime, monkeypatch): - """Use the real root, Graph wire, current policy, retention and result store.""" +def services_for(runtime, settings, **options): + return in_request( + runtime, + lambda: runtime.root.build_orchestration_services(USER_ID, CONVERSATION_ID, settings=settings), + **options, + ) + + +def update_user_settings(runtime, **changes): + record = deepcopy(runtime.settings.items[(USER_ID, USER_ID)]) + record["settings"].update(changes) + runtime.settings.upsert_item(record) + + +def change_conversation(runtime, change): + key = (CONVERSATION_ID, CONVERSATION_ID) + if change == "missing": + runtime.conversations.items.pop(key) + return + record = deepcopy(runtime.conversations.items[key]) + record.update({ + "owner": {"user_id": "another-owner"}, + "deleted": {"deleted": True}, + "orchestration_deleted": {"orchestration_deleted": True}, + }[change]) + runtime.conversations.upsert_item(record) + + +def bind_services(runtime, bound, **options): + services = services_for(runtime, bound.settings, **options) + bound.services = services + bound.context.result_service = services.results + bound.context.external_source_admission = services.external_source_admission + bound.context.external_source_preflight = services.external_source_preflight + bound.context.capture_external_source_configuration = services.capture_external_source_configuration + return services + + +def retained_url_runtime(runtime, monkeypatch, **options): + """Use the real root, signed-in session, current policy, retention and result store.""" # Runtime imports must follow the initialized application fixture. executor = importlib.import_module("functions_orchestration_executor") url = "https://example.com/source" @@ -125,7 +162,6 @@ def retained_url_runtime(runtime, monkeypatch): } monkeypatch.setattr(runtime.root, "get_settings", lambda: deepcopy(settings)) monkeypatch.setitem(runtime.root.config.CLIENTS, "storage_account_office_docs_client", None) - runtime.world.assignments.append(assignment("url-assignment", role_id=URL_ROLE)) step = { "step_id": "gather", "capability_id": "url_fetch", "arguments": {"urls": [url]}, "enabled": True, @@ -134,31 +170,27 @@ def retained_url_runtime(runtime, monkeypatch): context = executor.RunContext( user_id=USER_ID, conversation_id=CONVERSATION_ID, run_id="external-root-run", attempt_index=1, plan_contract_version=2, user_message=f"Review {url}", - user_roles=["User", "UrlAccessUser"], allowed_user_urls=[url], + user_roles=list(URL_ROLES), allowed_user_urls=[url], result_guard_token_for_step=lambda _step_id: "server-attempt-token", result_input_fingerprint_for_step=lambda _step_id: fingerprint, ) - record = { + runtime.runs.create_item({ "id": context.run_id, "user_id": USER_ID, "conversation_id": CONVERSATION_ID, "attempt_index": 1, "status": "running", "user_message": context.user_message, "memory_audience": {"kind": "personal", "owner_id": USER_ID, "collaboration_id": ""}, "plan": {"planner_contract_version": 2, "steps": [deepcopy(step)]}, - } - runtime.runs.create_item(record) - services = runtime.root.build_orchestration_services(USER_ID, CONVERSATION_ID, settings=settings) - context.result_service = services.results - context.external_source_admission = services.external_source_admission - context.external_source_preflight = services.external_source_preflight - context.capture_external_source_configuration = services.capture_external_source_configuration - return SimpleNamespace( + }) + bound = SimpleNamespace( context=context, producer=context.result_producer(step), step=step, settings=settings, - services=services, fingerprint=fingerprint, + fingerprint=fingerprint, result={ "status": "completed", "evidence": [], "notes": ["Retained source line.\n" * 600 + "The final line: caf\u00e9."], "citations": [{"url": url, "title": "Original source"}], }, ) + bind_services(runtime, bound, **{"roles": URL_ROLES, **options}) + return bound def retain_root_url(bound): @@ -170,25 +202,118 @@ def retain_root_url(bound): return retention.retain_gather_result(bound.step, bound.context, bound.result, source_manifest=[]) +def test_version_includes_session_identity(): + assert_app_version_at_least("0.261.209") + + +def test_source_free_service_construction_reads_no_identity(external_root): + runtime = external_root + unused_reader = reader_for(runtime) + services = services_for(runtime, {"max_generated_chat_artifact_size_mb": 1}) + catalog = services.export_catalog() + bindings = services.capability_request_bindings() + assert callable(unused_reader) + assert catalog + assert all(callable(bindings[name]) for name in ( + "external_source_preflight", + "external_source_admission", "external_source_authorizer", + "capture_external_source_configuration", + )) + runtime.settings_read.assert_not_called() + runtime.directory_client.assert_not_called() + + +def test_real_root_reads_session_roles_without_directory_or_token_calls(external_root): + runtime = external_root + interactive_scopes = deepcopy(runtime.auth.SCOPE) + identity = read_identity(reader_for(runtime, roles=URL_ROLES)) + assert identity.user_id == USER_ID + assert identity.roles == ("UrlAccessUser", "User") + assert identity.email == EMAIL + assert identity.user_enable_agents is True + assert runtime.auth.SCOPE == interactive_scopes + assert not hasattr(runtime.root, "requests") + runtime.directory_client.assert_not_called() + assert runtime.settings_read.call_count == 1 + assert runtime.settings_read.call_args.kwargs == {"item": USER_ID, "partition_key": USER_ID, **POINT_READ} + assert runtime.conversation_read.call_args.kwargs == { + "item": CONVERSATION_ID, "partition_key": CONVERSATION_ID, **POINT_READ, + } + + +def test_session_roles_are_captured_for_the_worker_thread(external_root): + runtime = external_root + with signed_in(runtime, roles=URL_ROLES): + reader = runtime.root.build_external_identity_reader(USER_ID, CONVERSATION_ID) + session["user"]["roles"] = [] + identities, errors = [], [] + + def work(): + try: + identities.append(read_identity(reader)) + except Exception as error: # Reported by the assertion below. + errors.append(error) + + worker = threading.Thread(target=work) + worker.start() + worker.join(10) + assert not worker.is_alive() and errors == [] + assert [identity.roles for identity in identities] == [("UrlAccessUser", "User")] + assert read_identity(reader).roles == ("UrlAccessUser", "User") + + +@pytest.mark.parametrize("session_state", ["background", "anonymous", "another_user"]) +def test_reader_without_the_actors_signed_in_session_fails_closed_before_reads(external_root, session_state): + runtime = external_root + if session_state == "background": + reader = reader_for(runtime, signed_in_session=False) + elif session_state == "anonymous": + with runtime.app.test_request_context("/api/orchestration/runs"): + reader = runtime.root.build_external_identity_reader(USER_ID, CONVERSATION_ID) + else: + reader = reader_for(runtime, oid=OTHER_ID, roles=URL_ROLES) + for _attempt in range(2): + with pytest.raises(ResultUnavailableError) as raised: + read_identity(reader) + assert raised.value.code == EXTERNAL_SESSION_UNAVAILABLE_REASON + runtime.conversation_read.assert_not_called() + runtime.settings_read.assert_not_called() + + +@pytest.mark.parametrize("roles", [None, (), ("UrlAccessUser",), ("user",)]) +def test_signed_in_session_without_user_or_admin_role_is_refused(external_root, roles): + runtime = external_root + with pytest.raises(ResultUnavailableError) as raised: + read_identity(reader_for(runtime, roles=roles)) + assert raised.value.code == "external_identity_role_required" + runtime.conversation_read.assert_not_called() + runtime.settings_read.assert_not_called() + + def test_default_root_entry_authorization_does_not_create_acquisition_proof(external_root, monkeypatch): runtime = external_root bound = retained_url_runtime(runtime, monkeypatch) authorized = bound.services.external_source_preflight(producer=bound.producer, selector=None) assert authorized is None - assert runtime.world.adapter.requests + assert runtime.settings_read.called with pytest.raises(ResultUnavailableError): bound.services.external_source_admission(producer=bound.producer, prepared=bound.result) assert bound.services.results.access.external_source_catalog == {} assert runtime.result_items.items == {} - runtime.world.assignments = [] - with pytest.raises(ResultUnavailableError): - bound.services.external_source_preflight(producer=bound.producer, selector=None) + for options, code in ( + ({"roles": ("User",)}, "result_external_capability_unavailable"), + ({"signed_in_session": False}, EXTERNAL_SESSION_UNAVAILABLE_REASON), + ): + services = services_for(runtime, bound.settings, **options) + with pytest.raises(ResultUnavailableError) as raised: + services.external_source_preflight(producer=bound.producer, selector=None) + assert raised.value.code == code def test_default_root_admits_url_and_reopens_full_result_after_restart(external_root, monkeypatch): runtime = external_root bound = retained_url_runtime(runtime, monkeypatch) - assert runtime.world.adapter.requests == [] + runtime.settings_read.assert_not_called() task = retain_root_url(bound) assert len(bound.services.results.access.external_source_catalog) == 1 assert len(task.outputs) == 1 and task.output("prepared").kind == "structured-v1" @@ -199,9 +324,7 @@ def test_default_root_admits_url_and_reopens_full_result_after_restart(external_ assert original["content_scope"] == "reported_external_content" assert len(original["notes"][0]) > 4096 - restarted = runtime.root.build_orchestration_services( - USER_ID, CONVERSATION_ID, settings=bound.settings, - ) + restarted = services_for(runtime, bound.settings, roles=URL_ROLES) monkeypatch.setattr(restarted, "external_source_admission", Mock(side_effect=AssertionError("No reacquisition"))) monkeypatch.setattr( restarted, "capture_external_source_configuration", Mock(side_effect=AssertionError("No recapture")), @@ -216,22 +339,29 @@ def test_default_root_admits_url_and_reopens_full_result_after_restart(external_ assert restored == original assert metadata["origin"] == "grounded" and metadata["external_source_count"] == 1 assert restarted.results.access.external_source_catalog == {} - assert runtime.world.adapter.requests - wire = json.dumps(task.to_dict()) - assert TOKEN not in wire and "synthetic-client-secret" not in wire + assert runtime.settings_read.called + saved = repr((task.to_dict(), runtime.result_items.items, runtime.runs.items)) + assert SECRET not in saved and "UrlAccessUser" not in saved restarted.external_source_admission.assert_not_called() restarted.capture_external_source_configuration.assert_not_called() + runtime.directory_client.assert_not_called() -@pytest.mark.parametrize("change", ["roles", "account", "settings", "user_url", "owner", "deleted", "missing"]) +@pytest.mark.parametrize("change", [ + "roles", "background", "another_user", "access", "settings", "user_url", + "owner", "deleted", "orchestration_deleted", "missing", +]) def test_default_root_preflight_uses_current_authority_before_acquisition(external_root, monkeypatch, change): runtime = external_root - bound = retained_url_runtime(runtime, monkeypatch) + session_options = { + "roles": {"roles": ("User",)}, + "background": {"signed_in_session": False}, + "another_user": {"oid": OTHER_ID}, + }.get(change, {}) + bound = retained_url_runtime(runtime, monkeypatch, **session_options) supplied_settings = deepcopy(bound.settings) - if change == "roles": - runtime.world.assignments = [] - elif change == "account": - runtime.world.user["accountEnabled"] = False + if change == "access": + update_user_settings(runtime, access={"status": "deny"}) elif change == "settings": bound.settings["enable_url_access"] = False elif change == "user_url": @@ -240,30 +370,23 @@ def test_default_root_preflight_uses_current_authority_before_acquisition(extern runtime.runs.replace_item( record["id"], record, etag=record["_etag"], match_condition=MatchConditions.IfNotModified, ) - else: - conversation = runtime.conversations.read_item(CONVERSATION_ID, CONVERSATION_ID) - if change == "missing": - runtime.conversations.delete_item( - CONVERSATION_ID, CONVERSATION_ID, etag=conversation["_etag"], - ) - else: - conversation.update( - {"user_id": "another-owner"} if change == "owner" else {"orchestration_deleted": True}, - ) - runtime.conversations.replace_item( - CONVERSATION_ID, conversation, - etag=conversation["_etag"], match_condition=MatchConditions.IfNotModified, - ) - with pytest.raises(ResultUnavailableError): + elif change in ("owner", "deleted", "orchestration_deleted", "missing"): + change_conversation(runtime, change) + with pytest.raises(ResultUnavailableError) as raised: bound.services.capture_external_source_configuration( "url", producer=bound.producer, settings=supplied_settings, ) + if change in ("background", "another_user"): + assert raised.value.code == EXTERNAL_SESSION_UNAVAILABLE_REASON + runtime.settings_read.assert_not_called() + elif change == "access": + assert raised.value.code == "external_identity_access_restricted" assert bound.services.results.access.external_source_catalog == {} assert runtime.result_items.items == {} @pytest.mark.parametrize("field", ["user_id", "conversation_id"]) -def test_default_root_capture_refuses_a_foreign_producer_before_directory_io(external_root, monkeypatch, field): +def test_default_root_capture_refuses_a_foreign_producer_before_identity_io(external_root, monkeypatch, field): runtime = external_root bound = retained_url_runtime(runtime, monkeypatch) foreign = replace(bound.producer, **{field: "foreign"}) @@ -271,44 +394,33 @@ def test_default_root_capture_refuses_a_foreign_producer_before_directory_io(ext bound.services.capture_external_source_configuration( "url", producer=foreign, settings=bound.settings, ) - assert runtime.world.adapter.requests == [] + runtime.settings_read.assert_not_called() assert runtime.result_items.items == {} - runtime.client_factory.assert_not_called() -@pytest.mark.parametrize("change", ["roles", "account", "settings"]) +@pytest.mark.parametrize("change", ["roles", "background", "another_user", "access", "settings"]) def test_default_root_retained_reads_recheck_current_authority(external_root, monkeypatch, change): runtime = external_root bound = retained_url_runtime(runtime, monkeypatch) task = retain_root_url(bound) before = deepcopy(runtime.result_items.items) - if change == "roles": - runtime.world.assignments = [] - elif change == "account": - runtime.world.user["accountEnabled"] = False - else: + session_options = { + "roles": {"roles": ("User",)}, + "background": {"signed_in_session": False}, + "another_user": {"roles": URL_ROLES, "oid": OTHER_ID}, + }.get(change, {"roles": URL_ROLES}) + if change == "access": + update_user_settings(runtime, access={"status": "deny"}) + elif change == "settings": bound.settings["enable_url_access"] = False - restarted = runtime.root.build_orchestration_services(USER_ID, CONVERSATION_ID, settings=bound.settings) - with pytest.raises(ResultUnavailableError): + restarted = services_for(runtime, bound.settings, **session_options) + with pytest.raises(ResultUnavailableError) as raised: restarted.results.open_result(task.output("prepared"), require_current_sources=True) + if change in ("background", "another_user"): + assert raised.value.code == EXTERNAL_SESSION_UNAVAILABLE_REASON assert runtime.result_items.items == before -def test_default_root_preserves_directory_timeout_instead_of_denial(external_root, monkeypatch): - runtime = external_root - bound = retained_url_runtime(runtime, monkeypatch) - runtime.world.adapter.error = Timeout("Private directory transport details.") - with pytest.raises(ExternalIdentityServiceError) as raised: - bound.services.capture_external_source_configuration( - "url", producer=bound.producer, settings=bound.settings, - ) - assert raised.value.code == "external_identity_timeout" - assert raised.value.retryable is True - assert "Private" not in str(raised.value) - assert bound.services.results.access.external_source_catalog == {} - assert runtime.result_items.items == {} - - @pytest.mark.parametrize("store", ["conversations", "runs", "app_settings"]) @pytest.mark.parametrize("error_type, code, retryable", [ (AzureError, "external_configuration_service_unavailable", True), @@ -354,8 +466,8 @@ def test_private_configuration_digest_is_stable_keyed_and_purpose_scoped(externa assert first == repeated == text_key == expected assert first != changed and first != rotated and first != unkeyed assert len(first) == 64 and all(character in "0123456789abcdef" for character in first) - assert external_root.world.adapter.requests == [] - external_root.client_factory.assert_not_called() + external_root.settings_read.assert_not_called() + external_root.directory_client.assert_not_called() @pytest.mark.parametrize("key", [ @@ -368,8 +480,8 @@ def test_private_configuration_digest_refuses_missing_weak_or_public_default_key with pytest.raises(ResultUnavailableError) as raised: root.private_external_configuration_digest(b'{"source":"action"}') assert raised.value.code == "external_configuration_private_digest_required" - assert external_root.world.adapter.requests == [] - external_root.client_factory.assert_not_called() + external_root.settings_read.assert_not_called() + external_root.directory_client.assert_not_called() @pytest.mark.parametrize("value", [None, {}, "configuration", bytearray(b"configuration")]) @@ -380,50 +492,36 @@ def test_private_configuration_digest_requires_canonical_bytes(external_root, va assert raised.value.code == "external_configuration_payload_invalid" -@pytest.mark.parametrize("graph_base, expected_scope", [ - ("https://graph.microsoft.com/v1.0", "https://graph.microsoft.com/.default"), - ("https://graph.microsoft.us/v1.0", "https://graph.microsoft.us/.default"), - ("https://graph.example.test/gateway/v1.0", "https://graph.example.test/.default"), -]) -def test_real_root_uses_current_application_authority_and_trusted_cloud_origin( - external_root, graph_base, expected_scope, -): - runtime = external_root - runtime.world.base_url = graph_base - runtime.world.prefix = urlsplit(graph_base).path.rstrip("/") - interactive_scopes = deepcopy(runtime.auth.SCOPE) - identity = read_identity(reader_for(runtime)) - assert identity.user_id == USER_ID - assert identity.roles == ("User",) - runtime.client_factory.assert_called_once_with( - APP_ID, authority="https://login.example.test/tenant", - client_credential="synthetic-client-secret", token_cache=None, timeout=10.0, - ) - assert all(call.kwargs == {"scopes": [expected_scope]} for call in runtime.token.call_args_list) - assert runtime.auth.SCOPE == interactive_scopes - assert len(runtime.world.adapter.requests) == 3 - assert runtime.settings_read.call_count >= 2 - assert all(call.kwargs == { - "item": USER_ID, "partition_key": USER_ID, - "connection_timeout": 10.0, "read_timeout": 10.0, "retry_total": 0, - } for call in runtime.settings_read.call_args_list) - runtime.world.settings_reader.assert_not_called() - - def test_current_settings_are_point_read_without_repairs_or_request_cache(external_root): runtime = external_root reader = reader_for(runtime) initial = read_identity(reader) - settings = runtime.settings.read_item(USER_ID, USER_ID) - settings["settings"]["enable_agents"] = False - runtime.settings.upsert_item(settings) + update_user_settings(runtime, enable_agents=False) changed = read_identity(reader) assert initial.user_enable_agents is True assert changed.user_enable_agents is False - runtime.client_factory.assert_called_once() - runtime.world.assignments = [] - with pytest.raises(ResultUnavailableError): + update_user_settings(runtime, access={"status": "deny"}) + stored = deepcopy(runtime.settings.items) + with pytest.raises(ResultUnavailableError) as raised: read_identity(reader) + assert raised.value.code == "external_identity_access_restricted" + assert runtime.settings.items == stored + assert runtime.settings_read.call_count == 3 + assert all(call.kwargs == { + "item": USER_ID, "partition_key": USER_ID, **POINT_READ, + } for call in runtime.settings_read.call_args_list) + + +def test_session_admin_bypasses_restrictions_but_not_ownership(external_root): + runtime = external_root + reader = reader_for(runtime, roles=("Admin",)) + update_user_settings(runtime, access={"status": "deny"}, enable_agents=False) + identity = read_identity(reader) + assert identity.roles == ("Admin",) and identity.user_enable_agents is False + change_conversation(runtime, "owner") + with pytest.raises(ResultUnavailableError) as raised: + read_identity(reader) + assert raised.value.code == "external_identity_access_denied" def test_current_identity_accepts_sdk_conversation_and_settings_responses(external_root): @@ -432,14 +530,12 @@ def test_current_identity_accepts_sdk_conversation_and_settings_responses(extern runtime.settings.sdk_responses = True reader = reader_for(runtime) initial = read_identity(reader) - settings = runtime.settings.read_item(USER_ID, USER_ID) - settings["settings"]["enable_agents"] = False - runtime.settings.upsert_item(settings) + update_user_settings(runtime, enable_agents=False) changed = read_identity(reader) assert initial.user_id == changed.user_id == USER_ID assert initial.roles == changed.roles == ("User",) assert initial.user_enable_agents is True and changed.user_enable_agents is False - runtime.world.assignments = [] + update_user_settings(runtime, access={"status": "deny"}) with pytest.raises(ResultUnavailableError): read_identity(reader) @@ -451,32 +547,43 @@ def test_identity_does_not_coerce_non_mapping_settings_into_authority(external_r runtime.settings, "read_item", lambda *args, **kwargs: list(read_settings(*args, **kwargs).items()), ) - with pytest.raises(ResultUnavailableError): + with pytest.raises(ResultUnavailableError) as raised: read_identity(reader_for(runtime)) + assert raised.value.code == "external_identity_settings_unavailable" -@pytest.mark.parametrize("change", ["deleted", "foreign", "missing_settings", "denied"]) -def test_current_conversation_or_user_restriction_never_uses_saved_roles(external_root, change): +@pytest.mark.parametrize("change, code", [ + ("deleted", "external_identity_access_denied"), + ("orchestration_deleted", "external_identity_access_denied"), + ("owner", "external_identity_access_denied"), + ("missing", "external_identity_access_denied"), + ("missing_settings", "external_identity_access_denied"), + ("denied", "external_identity_access_restricted"), + ("future_allow", "external_identity_access_restricted"), +]) +def test_current_conversation_or_user_restriction_is_reread_on_every_access(external_root, change, code): runtime = external_root reader = reader_for(runtime) - initial = read_identity(reader) - assert initial.roles == ("User",) - if change in ("deleted", "foreign"): - conversation = runtime.conversations.read_item(CONVERSATION_ID, CONVERSATION_ID) - if change == "deleted": - conversation["orchestration_deleted"] = True - else: - conversation["user_id"] = "other-owner" - runtime.conversations.upsert_item(conversation) - elif change == "missing_settings": - settings = runtime.settings.read_item(USER_ID, USER_ID) - runtime.settings.delete_item(USER_ID, USER_ID, etag=settings["_etag"]) + assert read_identity(reader).roles == ("User",) + if change == "missing_settings": + runtime.settings.items.pop((USER_ID, USER_ID)) + elif change == "denied": + update_user_settings(runtime, access={"status": "deny"}) + elif change == "future_allow": + update_user_settings(runtime, access={"status": "deny", "datetime_to_allow": "2999-01-01T00:00:00Z"}) else: - settings = runtime.settings.read_item(USER_ID, USER_ID) - settings["settings"]["access"] = {"status": "deny"} - runtime.settings.upsert_item(settings) - with pytest.raises(ResultUnavailableError): + change_conversation(runtime, change) + with pytest.raises(ResultUnavailableError) as raised: read_identity(reader) + assert raised.value.code == code + + +def test_expired_restriction_is_allowed_without_repairing_the_settings_document(external_root): + runtime = external_root + update_user_settings(runtime, access={"status": "deny", "datetime_to_allow": "2000-01-01T00:00:00Z"}) + stored = deepcopy(runtime.settings.items) + assert read_identity(reader_for(runtime)).roles == ("User",) + assert runtime.settings.items == stored @pytest.mark.parametrize("field", ["user_id", "conversation_id"]) @@ -484,50 +591,11 @@ def test_reader_rejects_another_actor_or_conversation_before_io(external_root, f runtime = external_root reader = reader_for(runtime) arguments = {"user_id": USER_ID, "conversation_id": CONVERSATION_ID, field: "other"} - with pytest.raises(ResultUnavailableError): + with pytest.raises(ResultUnavailableError) as raised: reader(**arguments) - runtime.client_factory.assert_not_called() + assert raised.value.code == "external_identity_access_denied" + runtime.conversation_read.assert_not_called() runtime.settings_read.assert_not_called() - assert runtime.world.adapter.requests == [] - - -@pytest.mark.parametrize("error, code, retryable", [ - ("server_error", "external_identity_service_unavailable", True), - ("temporarily_unavailable", "external_identity_service_unavailable", True), - ("too_many_requests", "external_identity_throttled", True), - ("unknown_error", "external_identity_response_invalid", False), -]) -def test_token_service_failure_is_not_reported_as_role_revocation(external_root, error, code, retryable): - runtime = external_root - runtime.token.return_value = {"error": error, "error_description": "synthetic-private-error"} - with pytest.raises(ExternalIdentityServiceError) as caught: - read_identity(reader_for(runtime)) - runtime.token.assert_called_once_with(scopes=[runtime.world.scope]) - assert caught.value.code == code - assert caught.value.retryable is retryable - assert "synthetic-private-error" not in str(caught.value) - assert runtime.world.adapter.requests == [] - - -@pytest.mark.parametrize("error", ["invalid_client", "invalid_scope", "access_denied", "consent_required"]) -def test_missing_application_permission_fails_without_interactive_fallback(external_root, error): - runtime = external_root - runtime.token.return_value = {"error": error, "error_description": "synthetic-private-error"} - with pytest.raises(ResultUnavailableError) as caught: - read_identity(reader_for(runtime)) - assert "synthetic-private-error" not in str(caught.value) - assert runtime.world.adapter.requests == [] - - -def test_token_network_timeout_is_a_safe_retryable_service_failure(external_root): - runtime = external_root - runtime.token.side_effect = Timeout("synthetic-private-error") - with pytest.raises(ExternalIdentityServiceError) as caught: - read_identity(reader_for(runtime)) - assert caught.value.code == "external_identity_timeout" - assert caught.value.retryable is True - assert "synthetic-private-error" not in str(caught.value) - assert caught.value.__context__ is None @pytest.mark.parametrize("store", ["conversations", "settings"]) @@ -538,14 +606,20 @@ def test_current_authorization_storage_outage_is_not_an_access_denial(external_r read_identity(reader_for(runtime)) assert caught.value.code == "external_identity_service_unavailable" assert caught.value.retryable is True + assert "Private" not in str(caught.value) -def test_existing_auth_factory_keeps_its_default_arguments(external_root): +def test_existing_auth_factory_keeps_its_default_arguments(external_root, monkeypatch): runtime = external_root + client = SimpleNamespace(acquire_token_for_client=Mock()) + client_factory = Mock(return_value=client) + monkeypatch.setattr(runtime.auth, "CLIENT_ID", APP_ID) + monkeypatch.setattr(runtime.auth, "CLIENT_SECRET", "synthetic-client-secret") + monkeypatch.setattr(runtime.auth, "ConfidentialClientApplication", client_factory) cache = object() application = runtime.auth._build_msal_app(cache=cache) - assert application.acquire_token_for_client is runtime.token - runtime.client_factory.assert_called_once_with( + assert application is client + client_factory.assert_called_once_with( APP_ID, authority=runtime.auth.AUTHORITY, client_credential="synthetic-client-secret", token_cache=cache, ) diff --git a/functional_tests/test_orchestration_external_error_markers.py b/functional_tests/test_orchestration_external_error_markers.py index fb5670ac6..7f6a57e03 100644 --- a/functional_tests/test_orchestration_external_error_markers.py +++ b/functional_tests/test_orchestration_external_error_markers.py @@ -1,8 +1,9 @@ # test_orchestration_external_error_markers.py """ Functional tests for safe external authority errors across invocation capture. -Version: 0.261.127 +Version: 0.261.209 Implemented in: 0.261.127 +Signed-in session identity messages updated in: 0.261.209 Real identity/configuration exceptions preserve public codes, retryability and cancellation through initial and sticky capture failures without retaining the @@ -50,7 +51,7 @@ "external_identity_limit_exceeded": False, "external_identity_callback_invalid": False, }, - "Current directory authorization could not be verified.", + "Current user authorization could not be verified.", ), ( ExternalConfigurationServiceError, @@ -72,7 +73,7 @@ CANCEL_CASES = ( ( ExternalIdentityCancelledError, "external_identity_cancelled", - "Current directory authorization was cancelled.", + "Current user authorization was cancelled.", ), ( ExternalConfigurationCancelledError, "external_configuration_cancelled", diff --git a/functional_tests/test_orchestration_external_identity.py b/functional_tests/test_orchestration_external_identity.py index df63735bd..b98a4e8cf 100644 --- a/functional_tests/test_orchestration_external_identity.py +++ b/functional_tests/test_orchestration_external_identity.py @@ -1,12 +1,14 @@ # test_orchestration_external_identity.py """ -Current Microsoft Graph identity for retained external orchestration results. -Version: 0.261.127 +Functional test for signed-in session identity on retained external orchestration results. +Version: 0.261.209 Implemented in: 0.261.127 +Signed-in session roles replaced per-call Microsoft Graph reads in: 0.261.209 -Real requests preparation, response streaming, JSON parsing and identity readers -run against a doubled HTTP adapter, never a tenant. Fresh processes import real -modules with networking blocked. Required operations remain active under -O. +Orchestration trusts the app roles in the signed-in session, as classic chat does, +and makes no directory calls. The real reader runs against doubled owner callbacks +with networking blocked. A fresh process imports the real module without settings, +authentication, route or credential modules, in normal and optimized Python. Refs microsoft/simplechat#1509. """ @@ -14,150 +16,63 @@ from copy import deepcopy from dataclasses import asdict from datetime import datetime, timezone -from io import BytesIO -import json from pathlib import Path import socket import subprocess import sys import unittest from unittest.mock import Mock, patch -from urllib.parse import parse_qs, urlencode, urlsplit from azure.core.exceptions import ClientAuthenticationError, HttpResponseError, ResourceNotFoundError import requests -from requests.adapters import BaseAdapter -# These real application modules are imported after adding the repository path. +# These real application modules are imported after adding the repository paths. ROOT = Path(__file__).resolve().parents[1] APP = ROOT / "application" / "single_app" +TESTS = Path(__file__).resolve().parent sys.path.insert(0, str(APP)) +sys.path.insert(0, str(TESTS)) import functions_orchestration_external_identity as identity_module from functions_orchestration_external_identity import ( + MAX_SESSION_ROLES, ExternalIdentityCancelledError, ExternalIdentityServiceError, - GraphExternalIdentityReader, + SessionExternalIdentityReader, ) from functions_orchestration_external_sources import CurrentExternalSourceIdentity -from functions_orchestration_result_contracts import ResultContractError +from functions_orchestration_result_contracts import EXTERNAL_SESSION_UNAVAILABLE_REASON, ResultContractError from functions_orchestration_results import ResultUnavailableError +from test_support.versioning import assert_app_version_at_least USER_ID = "a1111111-1111-4111-8111-111111111111" -APP_ID = "22222222-2222-4222-8222-222222222222" -PRINCIPAL_ID = "33333333-3333-4333-8333-333333333333" OTHER_ID = "44444444-4444-4444-8444-444444444444" -USER_ROLE = "55555555-5555-4555-8555-555555555555" -ADMIN_ROLE = "66666666-6666-4666-8666-666666666666" -GROUP_ID = "77777777-7777-4777-8777-777777777777" -URL_ROLE = "88888888-8888-4888-8888-888888888888" CONVERSATION_ID = "conversation-1" -TOKEN = "synthetic-sensitive-access-token" -BASE_URL = "https://graph.example.test/v1.0" -SCOPE = "https://graph.example.test/.default" - - -def role(role_id, name, *, enabled=True, members=None): - return { - "id": role_id, "value": name, "isEnabled": enabled, - "allowedMemberTypes": ["User"] if members is None else members, - } - - -def assignment(assignment_id="assignment-1", *, role_id=USER_ROLE, group=False, resource=PRINCIPAL_ID): - return { - "id": assignment_id, "appRoleId": role_id, "resourceId": resource, - "principalId": GROUP_ID if group else USER_ID, - "principalType": "Group" if group else "User", - } - - -class ResponseBody(BytesIO): - def __init__(self, data, *, failure=None): - super().__init__(data) - self.failure = failure - self.released = False - - def read(self, size=-1): - if self.failure is not None: - raise self.failure - return super().read(size) - - def release_conn(self): - self.released = True - self.close() - - -class GraphHTTPAdapter(BaseAdapter): - """Double the wire only; requests still prepares real HTTP requests/responses.""" - - def __init__(self, world): - super().__init__() - self.world = world - self.requests = [] - self.responses = [] - self.on_send = None - self.error = None - - def send(self, request, **kwargs): - self.requests.append((request, kwargs)) - if self.on_send is not None: - self.on_send(request) - if self.error is not None: - raise self.error - specification = self.world.response_for(request) - response = requests.Response() - response.status_code = specification.get("status", 200) - response.url = specification.get("url", request.url) - response.request = request - response.headers.update(specification.get("headers", {"Content-Type": "application/json"})) - raw = specification.get("raw") - if raw is None: - raw = json.dumps(specification.get("body"), ensure_ascii=True).encode("utf-8") - response.raw = ResponseBody(raw, failure=specification.get("read_error")) - self.responses.append(response) - return response - - def close(self): - for response in self.responses: - response.close() - - -class GraphWorld: - def __init__(self, *, base_url=BASE_URL, scope=SCOPE): +EMAIL = "owner@example.test" +SECRET = "synthetic-sensitive-session-secret" + + +class SessionWorld: + """Double only the owner's conversation and settings reads; the reader is real.""" + + def __init__(self): self.stack = ExitStack() - self.base_url = base_url - self.scope = scope - self.prefix = urlsplit(base_url).path.rstrip("/") - self.user = {"id": USER_ID, "accountEnabled": True, "userPrincipalName": "owner@example.test"} - self.principals = [{ - "id": PRINCIPAL_ID, "appId": APP_ID, "accountEnabled": True, - "appRoles": [role(USER_ROLE, "User"), role(ADMIN_ROLE, "Admin"), role(URL_ROLE, "UrlAccessUser")], - }] - self.assignments = [assignment()] self.conversation = {"id": CONVERSATION_ID, "user_id": USER_ID} - self.settings = {"id": USER_ID, "settings": {"enable_agents": True}} - self.page_size = 100 - self.overrides = {} + self.settings = {"id": USER_ID, "user_id": USER_ID, "settings": {"enable_agents": True}} self.events = [] - self.token_provider = Mock(side_effect=self.token) self.authorizer = Mock(side_effect=self.authorize) self.settings_reader = Mock(side_effect=self.read_settings) self.execution_check = Mock(return_value=True) - self.session = requests.Session() - self.session.trust_env = False - self.adapter = GraphHTTPAdapter(self) - self.session.mount("https://", self.adapter) - self.session.mount("http://", self.adapter) def __enter__(self): def no_network(*_args, **_kwargs): - raise AssertionError("External identity tests must not contact a tenant") + raise AssertionError("Session identity must not contact a directory or the network") - self.stack.enter_context(patch.object(socket.socket, "connect", no_network)) - self.stack.enter_context(patch.object(socket, "getaddrinfo", no_network)) - self.stack.callback(self.session.close) + for target, name in ( + (socket.socket, "connect"), (socket, "getaddrinfo"), (requests.Session, "request"), + ): + self.stack.enter_context(patch.object(target, name, no_network)) return self def __exit__(self, *args): @@ -171,222 +86,143 @@ def read_settings(self, user_id): self.events.append(("settings", user_id)) return deepcopy(self.settings) - def token(self, scope): - self.events.append(("token", scope)) - return TOKEN - def reader(self, **overrides): options = { "user_id": USER_ID, "conversation_id": CONVERSATION_ID, - "app_client_id": APP_ID, "graph_base_url": self.base_url, "graph_scope": self.scope, - "get_access_token": self.token_provider, "http_get": self.session.get, + "roles": ["User"], "email": EMAIL, "authorize_conversation": self.authorizer, "read_user_settings": self.settings_reader, "execution_check": self.execution_check, } - return GraphExternalIdentityReader(**{**options, **overrides}) - - def response_for(self, request): - parts = urlsplit(request.url) - query = {key: values[0] for key, values in parse_qs(parts.query).items()} - index = int(query.get("$skiptoken", "0")) - if parts.path == f"{self.prefix}/users/{USER_ID}": - kind, default = "user", {"body": deepcopy(self.user)} - elif parts.path == f"{self.prefix}/servicePrincipals": - kind, default = "principals", self.page(request.url, self.principals, index, counted=False) - elif parts.path == f"{self.prefix}/users/{USER_ID}/appRoleAssignments": - kind, default = "assignments", self.page(request.url, self.assignments, index, counted=True) - else: - raise AssertionError("Unexpected directory endpoint") - return deepcopy(self.overrides.get((kind, index), default)) - - def page(self, url, values, index, *, counted): - body = {"value": deepcopy(values[index:index + self.page_size])} - if counted: - body["@odata.count"] = len(values) - if index + self.page_size < len(values): - parts = urlsplit(url) - query = {key: values[0] for key, values in parse_qs(parts.query).items()} - query["$skiptoken"] = str(index + self.page_size) - body["@odata.nextLink"] = f"{parts.scheme}://{parts.netloc}{parts.path}?{urlencode(query)}" - return {"body": body} + return SessionExternalIdentityReader(**{**options, **overrides}) def read_identity(reader): return reader(user_id=USER_ID, conversation_id=CONVERSATION_ID) -class GraphExternalIdentityTests(unittest.TestCase): - def test_real_http_protocol_and_exact_direct_and_group_role_mapping(self): - with GraphWorld() as world: - world.assignments.extend([ - assignment("assignment-2", role_id=URL_ROLE, group=True), - assignment("assignment-3", group=True), - assignment("assignment-4", role_id=ADMIN_ROLE, resource=OTHER_ID), - ]) - identity = read_identity(world.reader()) +class SessionExternalIdentityTests(unittest.TestCase): + def test_version_includes_session_identity(self): + assert_app_version_at_least("0.261.209") + + def test_session_roles_supply_current_identity_without_directory_calls(self): + with SessionWorld() as world: + identity = read_identity(world.reader(roles=["User", "UrlAccessUser", "User"])) self.assertIs(type(identity), CurrentExternalSourceIdentity) self.assertEqual(identity.user_id, USER_ID) self.assertEqual(identity.roles, ("UrlAccessUser", "User")) self.assertTrue(identity.user_enable_agents) - self.assertEqual(identity.email, "owner@example.test") - self.assertEqual(world.events[0], ("conversation", USER_ID, CONVERSATION_ID)) - self.assertGreaterEqual(world.settings_reader.call_count, 2) - self.assertEqual(len(world.adapter.requests), 3) - for request, options in world.adapter.requests: - self.assertEqual(request.method, "GET") - self.assertIsNone(request.body) - self.assertEqual(request.headers["Authorization"], f"Bearer {TOKEN}") - self.assertTrue(options["stream"]) - self.assertGreater(options["timeout"], 0) - self.assertLessEqual(options["timeout"], 10) - user_request = world.adapter.requests[0][0] - self.assertIn("accountEnabled", parse_qs(urlsplit(user_request.url).query)["$select"][0]) - principal_request = world.adapter.requests[1][0] - self.assertEqual(parse_qs(urlsplit(principal_request.url).query)["$filter"], [f"appId eq '{APP_ID}'"]) - assignment_request = world.adapter.requests[2][0] - self.assertEqual(assignment_request.headers["ConsistencyLevel"], "eventual") - query = parse_qs(urlsplit(assignment_request.url).query) - self.assertEqual(query["$count"], ["true"]) - self.assertEqual(query["$filter"], [f"resourceId eq {PRINCIPAL_ID}"]) - for response in world.adapter.responses: - self.assertTrue(response.raw.released) - - def test_group_assignment_alone_can_supply_current_user_or_admin(self): - for role_id, expected in ((USER_ROLE, "User"), (ADMIN_ROLE, "Admin")): - with self.subTest(role=expected), GraphWorld() as world: - world.assignments = [assignment(group=True, role_id=role_id)] - identity = read_identity(world.reader()) - self.assertEqual(identity.roles, (expected,)) - - def test_injected_msal_callback_stays_lazy_and_uses_the_configured_application_scope(self): - with GraphWorld( - base_url="https://graph.microsoft.us/v1.0", - scope="https://graph.microsoft.us/.default", - ) as world: - application = Mock() - application.acquire_token_for_client.return_value = {"access_token": TOKEN} - factory = Mock(return_value=application) - initialized = [] - - def get_access_token(scope): - if not initialized: - initialized.append(factory( - cache=None, authority_override="https://login.microsoftonline.us/configured-tenant", - )) - result = initialized[0].acquire_token_for_client(scopes=[scope]) - return result["access_token"] - - reader = world.reader(get_access_token=get_access_token) - factory.assert_not_called() - application.acquire_token_for_client.assert_not_called() - self.assertEqual(world.adapter.requests, []) - current = read_identity(reader) - self.assertEqual(current.roles, ("User",)) - factory.assert_called_once_with( - cache=None, authority_override="https://login.microsoftonline.us/configured-tenant", - ) - self.assertEqual(application.acquire_token_for_client.call_count, 3) - for call in application.acquire_token_for_client.call_args_list: - self.assertEqual(call.kwargs, {"scopes": [world.scope]}) - requests_before = len(world.adapter.requests) - with self.assertRaises(ExternalIdentityServiceError): - read_identity(world.reader(get_access_token=lambda _scope: {"access_token": TOKEN})) - self.assertEqual(len(world.adapter.requests), requests_before) - - def test_foreign_default_and_application_only_assignments_are_not_user_authority(self): - for change in ("foreign", "default", "application", "disabled", "deleted", "no-base-role"): - with self.subTest(change=change), GraphWorld() as world: - definitions = world.principals[0]["appRoles"] - if change == "foreign": - world.assignments[0]["resourceId"] = OTHER_ID - elif change == "default": - world.assignments[0]["appRoleId"] = "00000000-0000-0000-0000-000000000000" - elif change == "application": - definitions[0]["allowedMemberTypes"] = ["Application"] - elif change == "disabled": - definitions[0]["isEnabled"] = False - elif change == "deleted": - del definitions[0] - else: - world.assignments[0]["appRoleId"] = URL_ROLE - with self.assertRaises(ResultUnavailableError): - read_identity(world.reader()) - - def test_account_and_service_principal_disable_delete_or_identity_change_deny(self): - cases = ( - ("user", "accountEnabled", False), ("user", "id", OTHER_ID), - ("user", "deletedDateTime", "2026-01-01T00:00:00Z"), - ("principal", "accountEnabled", False), ("principal", "appId", OTHER_ID), - ("principal", "deletedDateTime", "2026-01-01T00:00:00Z"), - ) - for target, field, value in cases: - with self.subTest(target=target, field=field), GraphWorld() as world: - record = world.user if target == "user" else world.principals[0] - record[field] = value - with self.assertRaises(ResultUnavailableError): - read_identity(world.reader()) - with GraphWorld() as world: - world.principals = [] - with self.assertRaises(ResultUnavailableError): - read_identity(world.reader()) + self.assertEqual(identity.email, EMAIL) + self.assertEqual(world.events, [ + ("conversation", USER_ID, CONVERSATION_ID), ("settings", USER_ID), + ]) + self.assertEqual(world.execution_check.call_count, 3) + for name in ("GraphExternalIdentityReader", "requests", "msal"): + self.assertFalse(hasattr(identity_module, name), name) + + def test_user_or_admin_session_role_is_required_before_any_read(self): + for roles in ([], (), ["UrlAccessUser"], ["user"], ["admin"], ["Users"]): + with self.subTest(roles=roles), SessionWorld() as world: + with self.assertRaises(ResultUnavailableError) as caught: + read_identity(world.reader(roles=roles)) + self.assertEqual(caught.exception.code, "external_identity_role_required") + self.assertEqual(world.events, []) + for roles, expected in ( + (["Admin"], ("Admin",)), (("User",), ("User",)), (["User", "Admin"], ("Admin", "User")), + ): + with self.subTest(roles=roles), SessionWorld() as world: + self.assertEqual(read_identity(world.reader(roles=roles)).roles, expected) + + def test_missing_signed_in_session_fails_closed_before_any_read(self): + self.assertEqual(EXTERNAL_SESSION_UNAVAILABLE_REASON, "external_identity_session_unavailable") + with SessionWorld() as world: + reader = world.reader(roles=None, email=None) + for _attempt in range(2): + with self.assertRaises(ResultUnavailableError) as caught: + read_identity(reader) + self.assertEqual(caught.exception.code, EXTERNAL_SESSION_UNAVAILABLE_REASON) + self.assertIsInstance(caught.exception, PermissionError) + self.assertEqual(world.events, []) + world.execution_check.assert_not_called() + + def test_session_roles_are_validated_deduplicated_and_bounded(self): + with SessionWorld() as world: + identity = read_identity(world.reader(roles=[ + "User", "User", " Admin", "Admin\n", "", 1, None, ["Admin"], "U" * 129, "UrlAccessUser", + ])) + self.assertEqual(identity.roles, ("UrlAccessUser", "User")) + for roles in ("User", {"User": True}, {"User"}, 1, True): + with self.subTest(roles=roles), SessionWorld() as world: + with self.assertRaises(ResultContractError) as caught: + world.reader(roles=roles) + self.assertEqual(caught.exception.code, "external_identity_configuration_invalid") + self.assertEqual(world.events, []) + with SessionWorld() as world: + roles = ["User", *(f"Role{index}" for index in range(MAX_SESSION_ROLES))] + with self.assertRaises(ExternalIdentityServiceError) as caught: + read_identity(world.reader(roles=roles)) + self.assertEqual(caught.exception.code, "external_identity_limit_exceeded") + self.assertFalse(caught.exception.retryable) + self.assertEqual(world.events, []) + with SessionWorld() as world: + roles = ["User", *(f"Role{index}" for index in range(MAX_SESSION_ROLES - 1))] + self.assertEqual(len(read_identity(world.reader(roles=roles)).roles), MAX_SESSION_ROLES) + + def test_email_comes_from_the_session_and_is_never_malformed(self): + for email, expected in ( + (EMAIL, EMAIL), (None, None), ("", None), ("owner @example.test", None), + (" owner@example.test", None), (1, None), ("o" * 321, None), + ): + with self.subTest(email=email), SessionWorld() as world: + self.assertEqual(read_identity(world.reader(email=email)).email, expected) - def test_actor_and_conversation_binding_precedes_tokens_and_directory_reads(self): + def test_actor_and_conversation_binding_precedes_reads(self): for kwargs in ( {"user_id": OTHER_ID, "conversation_id": CONVERSATION_ID}, {"user_id": USER_ID, "conversation_id": "different-conversation"}, {"user_id": 1, "conversation_id": CONVERSATION_ID}, {"user_id": USER_ID, "conversation_id": True}, ): - with self.subTest(kwargs=kwargs), GraphWorld() as world: - reader = world.reader() - with self.assertRaises(ResultUnavailableError): - reader(**kwargs) - self.assertEqual(world.events, []) - self.assertEqual(world.adapter.requests, []) + for roles in (["User"], None): + with self.subTest(kwargs=kwargs, roles=roles), SessionWorld() as world: + with self.assertRaises(ResultUnavailableError) as caught: + world.reader(roles=roles)(**kwargs) + self.assertEqual(caught.exception.code, "external_identity_actor_mismatch") + self.assertEqual(world.events, []) + + def test_conversation_ownership_is_checked_before_settings(self): for conversation in ( None, True, {}, {"id": CONVERSATION_ID, "user_id": OTHER_ID}, {"id": "wrong", "user_id": USER_ID}, {"id": CONVERSATION_ID, "user_id": USER_ID, "deleted": True}, + {"id": CONVERSATION_ID, "user_id": USER_ID, "orchestration_deleted": "2026-09-21T00:00:00Z"}, ): - with self.subTest(conversation=conversation), GraphWorld() as world: + with self.subTest(conversation=conversation), SessionWorld() as world: world.conversation = conversation - with self.assertRaises(ResultUnavailableError): + with self.assertRaises(ResultUnavailableError) as caught: read_identity(world.reader()) - world.token_provider.assert_not_called() - self.assertEqual(world.adapter.requests, []) - - def test_conversation_and_preferences_are_rechecked_during_the_call(self): - for change in ("conversation", "settings"): - with self.subTest(change=change), GraphWorld() as world: - def revoke_on_assignments(request): - if "/appRoleAssignments?" in request.url: - if change == "conversation": - world.conversation["orchestration_deleted"] = True - else: - world.settings["settings"]["access"] = {"status": "deny"} - - world.adapter.on_send = revoke_on_assignments - with self.assertRaises(ResultUnavailableError): - read_identity(world.reader()) - - def test_live_and_new_readers_do_not_keep_revoked_directory_authority(self): - for restart in (False, True): - for mutation in ("role", "account", "principal"): - with self.subTest(restart=restart, mutation=mutation), GraphWorld() as world: - reader = world.reader() - first = read_identity(reader) - self.assertIn("User", first.roles) - if mutation == "role": - world.assignments = [] - elif mutation == "account": - world.user["accountEnabled"] = False - else: - world.principals[0]["appRoles"][0]["isEnabled"] = False - if restart: - reader = world.reader() - with self.assertRaises(ResultUnavailableError): - read_identity(reader) - self.assertGreater(len(world.adapter.requests), 3) + self.assertEqual(caught.exception.code, "external_identity_conversation_unavailable") + world.settings_reader.assert_not_called() + with SessionWorld() as world: + world.conversation.update({"deleted": False, "orchestration_deleted": None}) + self.assertEqual(read_identity(world.reader()).roles, ("User",)) + + def test_live_reader_rereads_conversation_and_restrictions_on_every_access(self): + with SessionWorld() as world: + reader = world.reader() + self.assertEqual(read_identity(reader).roles, ("User",)) + world.conversation["orchestration_deleted"] = True + with self.assertRaises(ResultUnavailableError) as caught: + read_identity(reader) + self.assertEqual(caught.exception.code, "external_identity_conversation_unavailable") + del world.conversation["orchestration_deleted"] + world.settings["settings"]["access"] = {"status": "deny"} + with self.assertRaises(ResultUnavailableError) as caught: + read_identity(reader) + self.assertEqual(caught.exception.code, "external_identity_access_restricted") + world.settings["settings"] = {"enable_agents": False} + self.assertFalse(read_identity(reader).user_enable_agents) + self.assertEqual(world.authorizer.call_count, 4) + self.assertEqual(world.settings_reader.call_count, 3) def test_current_control_center_restrictions_and_expiration_are_read_only(self): now = datetime(2026, 9, 21, 22, 0, tzinfo=timezone.utc) @@ -402,377 +238,181 @@ def test_current_control_center_restrictions_and_expiration_are_read_only(self): ({"status": "allow"}, True), ({}, True), ) for access, allowed in cases: - with self.subTest(access=access), GraphWorld() as world: + with self.subTest(access=access), SessionWorld() as world: world.settings["settings"]["access"] = access before = deepcopy(world.settings) with patch.object(identity_module, "datetime", wraps=datetime) as clock: clock.now.return_value = now if allowed: - current = read_identity(world.reader()) - self.assertIn("User", current.roles) + self.assertEqual(read_identity(world.reader()).roles, ("User",)) else: - with self.assertRaises(ResultUnavailableError): + with self.assertRaises(ResultUnavailableError) as caught: read_identity(world.reader()) + self.assertEqual(caught.exception.code, "external_identity_access_restricted") self.assertEqual(world.settings, before) + self.assertEqual(world.settings_reader.call_count, 1) - def test_only_fresh_admin_bypasses_restrictions_and_agent_preference_still_applies(self): - with GraphWorld() as world: + def test_only_session_admin_bypasses_restrictions_and_agent_preference_still_applies(self): + with SessionWorld() as world: world.settings["settings"] = {"access": {"status": "deny"}, "enable_agents": False} - world.assignments = [assignment(role_id=ADMIN_ROLE)] - reader = world.reader() - current = read_identity(reader) + current = read_identity(world.reader(roles=["Admin"])) self.assertEqual(current.roles, ("Admin",)) self.assertFalse(current.user_enable_agents) - world.assignments = [assignment()] - with self.assertRaises(ResultUnavailableError): - read_identity(reader) + with self.assertRaises(ResultUnavailableError) as caught: + read_identity(world.reader(roles=["User"])) + self.assertEqual(caught.exception.code, "external_identity_access_restricted") def test_missing_malformed_or_wrong_actor_settings_never_default_to_allow(self): for value in ( None, True, {}, {"id": OTHER_ID, "settings": {}}, {"id": USER_ID}, - {"id": USER_ID, "settings": None}, {"id": USER_ID, "settings": {"enable_agents": "false"}}, + {"id": USER_ID, "settings": None}, {"id": USER_ID, "user_id": OTHER_ID, "settings": {}}, + {"id": USER_ID, "user_id": 1, "settings": {}}, + {"id": USER_ID, "settings": {"enable_agents": "false"}}, {"id": USER_ID, "settings": {"enable_agents": 1}}, ): - with self.subTest(settings=value), GraphWorld() as world: + with self.subTest(settings=value), SessionWorld() as world: world.settings = value - with self.assertRaises(ResultUnavailableError): - read_identity(world.reader()) - world.token_provider.assert_not_called() - with GraphWorld() as world: - world.settings["settings"] = {} - current = read_identity(world.reader()) - self.assertTrue(current.user_enable_agents) - - def test_all_pages_are_exhausted_including_late_group_roles(self): - with GraphWorld() as world: - world.page_size = 1 - world.assignments = [ - assignment("other-app", resource=OTHER_ID), - assignment("late-user", group=True), - assignment("last-role", role_id=URL_ROLE, group=True), - ] - current = read_identity(world.reader()) - self.assertEqual(current.roles, ("UrlAccessUser", "User")) - pages = [request for request, _ in world.adapter.requests if "/appRoleAssignments?" in request.url] - self.assertEqual(len(pages), 3) - for page in pages: - self.assertEqual(page.headers["ConsistencyLevel"], "eventual") - self.assertEqual(parse_qs(urlsplit(page.url).query)["$count"], ["true"]) - - def test_late_page_failure_never_returns_earlier_roles(self): - for status in (403, 429, 503): - with self.subTest(status=status), GraphWorld() as world: - world.page_size = 1 - world.assignments.append(assignment("second", role_id=URL_ROLE, group=True)) - world.overrides["assignments", 1] = {"status": status, "raw": b"sensitive upstream message"} - expected = ResultUnavailableError if status == 403 else ExternalIdentityServiceError - with self.assertRaises(expected): - read_identity(world.reader()) - - def test_counts_missing_pages_and_duplicate_assignments_are_rejected(self): - for body in ( - {"value": [assignment()]}, - {"value": [assignment()], "@odata.count": True}, - {"value": [assignment()], "@odata.count": 2}, - {"value": [assignment()], "@odata.count": 0}, - {"value": [assignment(), assignment()], "@odata.count": 2}, - {"value": [assignment()], "@odata.count": 1, "@odata.nextLink": None}, - {"value": "not-a-collection", "@odata.count": 1}, - {"value": [None], "@odata.count": 1}, - ): - with self.subTest(body=body), GraphWorld() as world: - world.overrides["assignments", 0] = {"body": body} - with self.assertRaises(ExternalIdentityServiceError): - read_identity(world.reader()) - - def test_ambiguous_service_principals_and_role_definitions_are_rejected(self): - for mutation in ("principal", "role-id", "role-value", "role-page", "role-count"): - with self.subTest(mutation=mutation), GraphWorld() as world: - principal = world.principals[0] - if mutation == "principal": - world.principals.append(deepcopy(principal)) - world.page_size = 1 - elif mutation == "role-id": - principal["appRoles"].append(deepcopy(principal["appRoles"][0])) - elif mutation == "role-value": - principal["appRoles"].append(role(OTHER_ID, "User")) - elif mutation == "role-page": - principal["appRoles@odata.nextLink"] = f"{BASE_URL}/servicePrincipals" - else: - principal["appRoles@odata.count"] = 4 - with self.assertRaises(ExternalIdentityServiceError): - read_identity(world.reader()) - - def test_next_link_cannot_escape_origin_version_or_exact_collection_before_token(self): - path = f"/v1.0/users/{USER_ID}/appRoleAssignments" - links = ( - f"http://graph.example.test{path}", - f"https://evil.example.test{path}", - f"https://graph.example.test.evil.example{path}", - f"https://graph.example.test:444{path}", - f"https://user:secret@graph.example.test{path}", - f"//graph.example.test{path}", - f"https://graph.example.test/beta/users/{USER_ID}/appRoleAssignments", - f"https://graph.example.test/v1.0evil/users/{USER_ID}/appRoleAssignments", - f"https://graph.example.test/v1.0/users/{OTHER_ID}/appRoleAssignments", - f"{BASE_URL}/servicePrincipals", - f"{BASE_URL}/../users/{USER_ID}/appRoleAssignments", - f"{BASE_URL}/%2e%2e/users/{USER_ID}/appRoleAssignments", - f"https://graph.example.test{path}#secret", - f"https://graph.example.test\\@evil.example{path}", - f"https://graph.example.test{path}\n", - ) - for link in links: - with self.subTest(link=link), GraphWorld() as world: - world.overrides["assignments", 0] = { - "body": {"value": [assignment()], "@odata.count": 1, "@odata.nextLink": link}, - } - with self.assertRaises(ExternalIdentityServiceError): + with self.assertRaises(ResultUnavailableError) as caught: read_identity(world.reader()) - self.assertEqual(len(world.adapter.requests), 3) - self.assertEqual(world.token_provider.call_count, 3) - - def test_pagination_cycle_and_global_page_record_and_byte_limits_fail_closed(self): - with GraphWorld() as world: - link = f"{BASE_URL}/users/{USER_ID}/appRoleAssignments?$skiptoken=1" - world.overrides["assignments", 0] = { - "body": {"value": [], "@odata.count": 1, "@odata.nextLink": link}, - } - world.overrides["assignments", 1] = { - "body": {"value": [], "@odata.count": 1, "@odata.nextLink": link}, - } - with self.assertRaises(ExternalIdentityServiceError) as caught: + self.assertEqual(caught.exception.code, "external_identity_settings_unavailable") + with SessionWorld() as world: + world.settings_reader.side_effect = ResourceNotFoundError(SECRET) + with self.assertRaises(ResultUnavailableError) as caught: read_identity(world.reader()) - self.assertEqual(caught.exception.code, "external_identity_pagination_invalid") - self.assertEqual(len(world.adapter.requests), 4) - for override in ({"max_pages": 2}, {"max_records": 4}, {"max_response_bytes": 10}): - with self.subTest(override=override), GraphWorld() as world: - with self.assertRaises(ExternalIdentityServiceError) as caught: - read_identity(world.reader(**override)) - self.assertEqual(caught.exception.code, "external_identity_limit_exceeded") - - def test_directory_status_denials_are_distinct_from_transient_failures(self): - for kind in ("user", "principals", "assignments"): - for status in (401, 403, 404, 410, 202, 408, 429, 500, 502, 503): - with self.subTest(kind=kind, status=status), GraphWorld() as world: - world.overrides[kind, 0] = {"status": status, "raw": TOKEN.encode()} - expected = ResultUnavailableError if status in (401, 403, 404, 410) else ExternalIdentityServiceError - with self.assertRaises(expected) as caught: - read_identity(world.reader()) - self.assertNotIn(TOKEN, str(caught.exception)) - self.assertIsNone(caught.exception.__cause__) - self.assertIsNone(caught.exception.__context__) - if isinstance(caught.exception, ExternalIdentityServiceError): - self.assertTrue(caught.exception.retryable) + self.assertEqual(caught.exception.code, "external_identity_access_denied") + with SessionWorld() as world: + world.settings = {"id": USER_ID, "settings": {}} + self.assertTrue(read_identity(world.reader()).user_enable_agents) - def test_redirects_are_not_followed_and_response_origin_is_checked(self): - for specification in ( - {"status": 302, "headers": {"Location": "https://evil.example/secret"}}, - {"body": {}, "url": "https://evil.example/v1.0/users"}, - ): - with self.subTest(specification=specification), GraphWorld() as world: - world.overrides["user", 0] = specification - with self.assertRaises(ExternalIdentityServiceError): - read_identity(world.reader()) - self.assertEqual(len(world.adapter.requests), 1) + def test_owner_io_errors_have_safe_exception_contracts(self): + def http_error(status): + error = HttpResponseError(SECRET) + error.status_code = status + return error - def test_timeout_network_and_credential_errors_have_safe_exception_contracts(self): cases = ( - (requests.Timeout(TOKEN), ExternalIdentityServiceError, "external_identity_timeout"), - (requests.ConnectionError(TOKEN), ExternalIdentityServiceError, "external_identity_service_unavailable"), - (TimeoutError(TOKEN), ExternalIdentityServiceError, "external_identity_timeout"), - (ClientAuthenticationError(TOKEN), ResultUnavailableError, "external_identity_access_denied"), - (ResourceNotFoundError(TOKEN), ResultUnavailableError, "external_identity_access_denied"), - (PermissionError(TOKEN), ResultUnavailableError, "external_identity_access_denied"), - (RuntimeError(TOKEN), ExternalIdentityServiceError, "external_identity_callback_invalid"), + (requests.Timeout(SECRET), ExternalIdentityServiceError, "external_identity_timeout", True), + (TimeoutError(SECRET), ExternalIdentityServiceError, "external_identity_timeout", True), + ( + requests.ConnectionError(SECRET), ExternalIdentityServiceError, + "external_identity_service_unavailable", True, + ), + (http_error(429), ExternalIdentityServiceError, "external_identity_throttled", True), + (http_error(503), ExternalIdentityServiceError, "external_identity_service_unavailable", True), + (http_error(403), ResultUnavailableError, "external_identity_access_denied", None), + (ClientAuthenticationError(SECRET), ResultUnavailableError, "external_identity_access_denied", None), + (ResourceNotFoundError(SECRET), ResultUnavailableError, "external_identity_access_denied", None), + (PermissionError(SECRET), ResultUnavailableError, "external_identity_access_denied", None), + (RuntimeError(SECRET), ExternalIdentityServiceError, "external_identity_callback_invalid", False), ) - for exception, expected, code in cases: - for target in ("token", "http", "settings"): - with self.subTest(error=type(exception).__name__, target=target), GraphWorld() as world: - if target == "token": - world.token_provider.side_effect = exception - elif target == "settings": - world.settings_reader.side_effect = exception - else: - world.adapter.error = exception + for exception, expected, code, retryable in cases: + for target in ("conversation", "settings"): + with self.subTest(code=code, error=type(exception).__name__, target=target), SessionWorld() as world: + owner = world.authorizer if target == "conversation" else world.settings_reader + owner.side_effect = exception with self.assertRaises(expected) as caught: read_identity(world.reader()) self.assertEqual(caught.exception.code, code) - self.assertNotIn(TOKEN, repr(caught.exception)) + if retryable is not None: + self.assertIs(caught.exception.retryable, retryable) + self.assertNotIn(SECRET, repr(caught.exception)) + self.assertNotIn(SECRET, str(caught.exception)) self.assertIsNone(caught.exception.__cause__) self.assertIsNone(caught.exception.__context__) - with GraphWorld() as world: - world.overrides["user", 0] = {"body": world.user, "read_error": requests.ReadTimeout(TOKEN)} - with self.assertRaises(ExternalIdentityServiceError) as caught: - read_identity(world.reader()) - self.assertEqual(caught.exception.code, "external_identity_timeout") - self.assertTrue(world.adapter.responses[0].raw.released) - - def test_sdk_http_statuses_keep_transient_and_permission_categories(self): - for status in (403, 429, 503): - with self.subTest(status=status), GraphWorld() as world: - error = HttpResponseError(TOKEN) - error.status_code = status - world.settings_reader.side_effect = error - expected = ResultUnavailableError if status == 403 else ExternalIdentityServiceError - with self.assertRaises(expected) as caught: - read_identity(world.reader()) - self.assertNotIn(TOKEN, str(caught.exception)) - def test_elapsed_budget_and_execution_cancellation_prevent_authority(self): - with GraphWorld() as world: + def test_execution_cancellation_stops_authority_between_reads(self): + with SessionWorld() as world: world.execution_check.return_value = False with self.assertRaises(ExternalIdentityCancelledError): read_identity(world.reader()) - world.token_provider.assert_not_called() - with GraphWorld() as world: - clock = Mock(return_value=1.0) - world.adapter.on_send = lambda _request: clock.configure_mock(return_value=100.0) - with patch.object(identity_module, "monotonic", clock): - with self.assertRaises(ExternalIdentityServiceError) as caught: - read_identity(world.reader(max_elapsed=2)) - self.assertEqual(caught.exception.code, "external_identity_timeout") - self.assertEqual(len(world.adapter.requests), 1) - with GraphWorld() as world: - world.adapter.on_send = lambda _request: world.execution_check.configure_mock(return_value=False) + self.assertEqual(world.events, []) + + with SessionWorld() as world: + def authorize_then_stop(**kwargs): + world.execution_check.return_value = False + return world.authorize(**kwargs) + + world.authorizer.side_effect = authorize_then_stop + with self.assertRaises(ExternalIdentityCancelledError): + read_identity(world.reader()) + world.settings_reader.assert_not_called() + + with SessionWorld() as world: + def read_then_stop(user_id): + world.execution_check.return_value = False + return world.read_settings(user_id) + + world.settings_reader.side_effect = read_then_stop with self.assertRaises(ExternalIdentityCancelledError): read_identity(world.reader()) - self.assertEqual(len(world.adapter.requests), 1) - def test_owning_lease_or_budget_exception_is_not_reclassified_as_directory_io(self): + for value in ("yes", 1, [], "true"): + with self.subTest(value=value), SessionWorld() as world: + world.execution_check.return_value = value + with self.assertRaises(ExternalIdentityServiceError) as caught: + read_identity(world.reader()) + self.assertEqual(caught.exception.code, "external_identity_callback_invalid") + self.assertEqual(world.events, []) + with SessionWorld() as world: + world.execution_check.return_value = None + self.assertEqual(read_identity(world.reader()).roles, ("User",)) + with SessionWorld() as world: + self.assertEqual(read_identity(world.reader(execution_check=None)).roles, ("User",)) + + def test_owning_lease_exception_is_not_reclassified(self): class OwnerExecutionStopped(RuntimeError): pass - with GraphWorld() as world: + with SessionWorld() as world: stopped = OwnerExecutionStopped("The owning execution stopped.") def check_execution(): - if world.adapter.requests: + if world.events: raise stopped - return None + return True world.execution_check.side_effect = check_execution with self.assertRaises(OwnerExecutionStopped) as caught: read_identity(world.reader()) self.assertIs(caught.exception, stopped) - self.assertTrue(world.adapter.responses[0].raw.released) + world.settings_reader.assert_not_called() - def test_transient_failure_after_success_does_not_reuse_prior_identity(self): - with GraphWorld() as world: - reader = world.reader() - current = read_identity(reader) - self.assertEqual(current.roles, ("User",)) - world.adapter.error = requests.Timeout(TOKEN) - with self.assertRaises(ExternalIdentityServiceError) as caught: - read_identity(reader) - self.assertEqual(caught.exception.code, "external_identity_timeout") - self.assertTrue(caught.exception.retryable) - - def test_invalid_record_types_and_role_value_shapes_never_become_authority(self): - cases = ( - ("user", "accountEnabled", "true"), ("user", "id", 1), - ("user", "@odata.type", "#microsoft.graph.servicePrincipal"), - ("principal", "appId", True), ("principal", "appRoles", {}), - ("role", "id", "not-a-guid"), ("role", "value", " User"), - ("role", "value", ".User"), ("role", "value", ["User"]), - ("role", "value", "U" * 121), ("role", "value", "User\n"), - ("role", "allowedMemberTypes", "User"), ("role", "allowedMemberTypes", ["User", "User"]), - ("role", "allowedMemberTypes", [True]), ("role", "isEnabled", 1), - ("assignment", "principalId", OTHER_ID), ("assignment", "principalType", "ServicePrincipal"), - ("assignment", "appRoleId", True), ("assignment", "id", {}), - ("assignment", "resourceId", 1), - ) - for target, field, value in cases: - with self.subTest(target=target, field=field, value=value), GraphWorld() as world: - targets = { - "user": world.user, "principal": world.principals[0], - "role": world.principals[0]["appRoles"][0], "assignment": world.assignments[0], - } - targets[target][field] = value - with self.assertRaises((ExternalIdentityServiceError, ResultUnavailableError)): - read_identity(world.reader()) - - def test_ambiguous_or_malformed_json_and_media_types_are_rejected_safely(self): - bodies = ( - b"not-json-sensitive", b'{"id": "one", "id": "two"}', - b'{"number": NaN}', b"[]", b"null", b"\xff", - ) - for raw in bodies: - with self.subTest(raw=raw), GraphWorld() as world: - world.overrides["user", 0] = {"raw": raw} - with self.assertRaises(ExternalIdentityServiceError) as caught: - read_identity(world.reader()) - self.assertEqual(caught.exception.code, "external_identity_response_invalid") - self.assertIsNone(caught.exception.__context__) - with GraphWorld() as world: - world.overrides["user", 0] = {"body": world.user, "headers": {"Content-Type": "text/html"}} - with self.assertRaises(ExternalIdentityServiceError): - read_identity(world.reader()) - with GraphWorld() as world: - world.overrides["user", 0] = {"body": {**world.user, "error": {"message": TOKEN}}} - with self.assertRaises(ExternalIdentityServiceError) as caught: - read_identity(world.reader()) - self.assertNotIn(TOKEN, str(caught.exception)) - - def test_national_and_custom_cloud_base_and_scope_are_injected_not_rewritten(self): - for base, scope in ( - ("https://graph.microsoft.us/v1.0", "https://graph.microsoft.us/.default"), - ("https://dod-graph.microsoft.us/v1.0", "https://dod-graph.microsoft.us/.default"), - ("https://microsoftgraph.chinacloudapi.cn/v1.0", "https://microsoftgraph.chinacloudapi.cn/.default"), - ("https://gateway.example.test/tenant/graph/v1.0", "https://upstream.example.test/.default"), + def test_invalid_constructor_values_perform_no_reads(self): + for override in ( + {"user_id": True}, {"user_id": ""}, {"user_id": " owner"}, + {"conversation_id": None}, {"conversation_id": "c" * 257}, ): - with self.subTest(base=base), GraphWorld(base_url=base, scope=scope) as world: - identity = read_identity(world.reader()) - self.assertIn("User", identity.roles) - for request, _ in world.adapter.requests: - self.assertTrue(request.url.startswith(f"{base}/")) - for call in world.token_provider.call_args_list: - self.assertEqual(call.args, (scope,)) - - def test_invalid_constructor_values_and_tokens_do_not_trigger_graph_calls(self): + with self.subTest(override=override), SessionWorld() as world: + with self.assertRaises(ResultContractError) as caught: + world.reader(**override) + self.assertEqual(caught.exception.code, "external_identity_configuration_invalid") + self.assertEqual(world.events, []) for override in ( - {"user_id": True}, {"user_id": USER_ID.upper()}, {"user_id": "owner"}, - {"app_client_id": "https://secret.invalid"}, {"app_client_id": 123}, - {"graph_base_url": "http://graph.example.test/v1.0"}, - {"graph_base_url": f"{BASE_URL}?secret=private"}, {"graph_base_url": f"{BASE_URL}#"}, - {"graph_base_url": "https://%67raph.example.test/v1.0"}, - {"graph_base_url": "https://graph.example.test:0/v1.0"}, - {"graph_scope": "User.Read"}, {"graph_scope": 1}, {"get_access_token": None}, - {"request_timeout": True}, {"max_elapsed": float("inf")}, {"max_pages": 0}, - {"max_pages": 101}, {"max_records": False}, {"max_response_bytes": -1}, + {"authorize_conversation": None}, {"read_user_settings": "reader"}, {"execution_check": 1}, ): - with self.subTest(override=override), GraphWorld() as world: - with self.assertRaises(ResultContractError): + with self.subTest(override=override), SessionWorld() as world: + with self.assertRaises(ResultContractError) as caught: world.reader(**override) + self.assertEqual(caught.exception.code, "external_identity_callback_required") self.assertEqual(world.events, []) - for token in (None, True, b"token", "", "token\r\ninjected: yes", "token with whitespace"): - with self.subTest(token=token), GraphWorld() as world: - world.token_provider.side_effect = None - world.token_provider.return_value = token - with self.assertRaises(ExternalIdentityServiceError): - read_identity(world.reader()) - self.assertEqual(world.adapter.requests, []) - def test_returned_identity_contains_no_directory_config_credentials_or_cached_roles(self): - with GraphWorld() as world: + def test_returned_identity_and_reader_keep_no_tokens_or_writes(self): + with SessionWorld() as world: reader = world.reader() - before = deepcopy((world.user, world.principals, world.assignments, world.settings)) + before = deepcopy((world.conversation, world.settings)) identity = read_identity(reader) - encoded = json.dumps(asdict(identity)) - self.assertNotIn(TOKEN, encoded) - self.assertNotIn(BASE_URL, encoded) - self.assertNotIn(SCOPE, encoded) - self.assertNotIn(APP_ID, encoded) - self.assertNotIn(PRINCIPAL_ID, encoded) - self.assertEqual(before, (world.user, world.principals, world.assignments, world.settings)) - self.assertFalse(any("cache" in key or "token" == key or "roles" in key for key in vars(reader))) - - def test_fresh_process_real_imports_and_directory_revocation_normal_and_optimized(self): + self.assertEqual(asdict(identity), { + "user_id": USER_ID, "roles": ("User",), "user_enable_agents": True, "email": EMAIL, + }) + self.assertFalse(any("token" in key or "cache" in key for key in vars(reader))) + self.assertEqual(before, (world.conversation, world.settings)) + + def test_fresh_process_real_imports_and_missing_session_normal_and_optimized(self): for optimized in (False, True): with self.subTest(optimized=optimized): command = [sys.executable, "-B"] + (["-O"] if optimized else []) result = subprocess.run( - command + ["-c", PROCESS_PROBE, str(APP), str(Path(__file__).resolve().parent)], + command + ["-c", PROCESS_PROBE, str(APP), str(TESTS)], cwd=ROOT, capture_output=True, text=True, timeout=60, check=False, ) self.assertEqual(result.returncode, 0, result.stdout + result.stderr) @@ -795,11 +435,11 @@ def checked_import(name, globals=None, locals=None, fromlist=(), level=0): name in {"config", "functions_settings", "functions_authentication"} or name.startswith("route_") ): - raise AssertionError("Unexpected application/credential discovery import") + raise AssertionError("Unexpected application or credential discovery import") return original_import(name, globals, locals, fromlist, level) def no_network(*args, **kwargs): - raise AssertionError("Unexpected network access") + raise AssertionError("Unexpected network or credential access") with ( patch.object(builtins, "__import__", checked_import), @@ -807,32 +447,32 @@ def no_network(*args, **kwargs): patch.object(socket, "getaddrinfo", no_network), ): import azure.identity - import requests + import msal with ExitStack() as constructors: for name, credential in vars(azure.identity).items(): if name.endswith("Credential") and isinstance(credential, type): constructors.enter_context(patch.object(credential, "__init__", no_network)) - constructors.enter_context(patch.object(requests.Session, "__init__", no_network)) - module = importlib.import_module("functions_orchestration_external_identity") - fixtures = importlib.import_module("test_orchestration_external_identity") - with fixtures.GraphWorld() as world: - reader = world.reader() - if world.events or world.adapter.requests: - raise AssertionError("Constructing a reader performed I/O") - current = fixtures.read_identity(reader) - if current.roles != ("User",): - raise AssertionError("Current directory role was not returned") - world.assignments = [] - restarted = world.reader() - try: - fixtures.read_identity(restarted) - except fixtures.ResultUnavailableError: - pass - else: - raise AssertionError("Restart reused obsolete roles") - if len(world.adapter.requests) != 6: - raise AssertionError("Required directory reads were skipped") + constructors.enter_context(patch.object(msal.ClientApplication, "__init__", no_network)) + importlib.import_module("functions_orchestration_external_identity") + fixtures = importlib.import_module("test_orchestration_external_identity") + with fixtures.SessionWorld() as world: + reader = world.reader() + if world.events: + raise AssertionError("Constructing a reader performed I/O") + current = fixtures.read_identity(reader) + if current.roles != ("User",): + raise AssertionError("The session role was not returned") + background = world.reader(roles=None) + try: + fixtures.read_identity(background) + except fixtures.ResultUnavailableError as error: + if error.code != "external_identity_session_unavailable": + raise AssertionError("A missing session was not reported") + else: + raise AssertionError("A reader without a session returned authority") + if len(world.events) != 2: + raise AssertionError("Required conversation and settings reads were skipped") """ diff --git a/functional_tests/test_orchestration_external_preflight_adapter.py b/functional_tests/test_orchestration_external_preflight_adapter.py index 856b7efd7..3c4470600 100644 --- a/functional_tests/test_orchestration_external_preflight_adapter.py +++ b/functional_tests/test_orchestration_external_preflight_adapter.py @@ -1,9 +1,10 @@ # test_orchestration_external_preflight_adapter.py """Strict invocation authorization and acquisition support for all five v2 adapters. -Version: 0.261.129 +Version: 0.261.209 Implemented in: 0.261.127 Early acquisition-support regressions implemented in: 0.261.129 +Missing signed-in session refusal reason implemented in: 0.261.209 The auth-only runtime hook invokes the real provider before capture or engine setup. Capture separately invokes the real combined acquisition guard. The @@ -295,6 +296,27 @@ def test_real_current_authority_overrides_stale_context_before_effects(authorize assert_no_effects(state) +def test_missing_signed_in_session_keeps_only_its_reason_before_effects(authorized_runtime): + # A background continuation has no signed-in session to supply the user's roles. + state = authorized_runtime + results = importlib.import_module("functions_orchestration_results") + reason = "external_identity_session_unavailable" + failure = results.ResultUnavailableError(reason) + failure.private_detail = "PRIVATE_SESSION_DETAIL" + state.provider.read_identity = Mock(side_effect=failure) + with pytest.raises(PermissionError) as caught: + execute(state) + assert caught.value.code == "result_unavailable" and caught.value.retryable is False + assert caught.value.authority_reason == reason + assert caught.value is not failure and not hasattr(caught.value, "private_detail") + assert caught.value.__context__ is None and caught.value.__cause__ is None + state.preflight.assert_called_once() + state.acquisition.assert_not_called() + state.validator.assert_not_called() + state.capture.assert_not_called() + assert_no_effects(state) + + @pytest.mark.parametrize("authorized_runtime", ["agent_invoke", "action_invoke"], indirect=True) @pytest.mark.parametrize("fault", [ "membership_revoked", "source_removed", "saved_selection_changed", "current_selection_changed", diff --git a/functional_tests/test_orchestration_image_references.py b/functional_tests/test_orchestration_image_references.py index 8c8f50daf..87767516f 100644 --- a/functional_tests/test_orchestration_image_references.py +++ b/functional_tests/test_orchestration_image_references.py @@ -2,8 +2,9 @@ #!/usr/bin/env python3 """ Functional test for orchestration image references. -Version: 0.261.192 +Version: 0.261.209 Implemented in: 0.261.192 +Missing signed-in session failure for image inputs implemented in: 0.261.209 This test ensures orchestration plans can seed, validate, bind, and execute image references without trusting planner or browser-supplied labels. @@ -613,6 +614,31 @@ def test_adapter_fails_closed_without_an_authorized_reference_source(monkeypatch assert captured["generated"] == 0 and "resolved" not in captured and "persisted" not in captured +@pytest.mark.parametrize("reason, code", [ + ("external_identity_session_unavailable", "external_session_required"), + ("external_identity_access_denied", "result_unavailable"), +]) +def test_adapter_explains_a_refused_retained_input(monkeypatch, reason, code): + """A background continuation has no signed-in session, so the image step asks for a resend.""" + from functions_orchestration_results import ResultUnavailableError + + images, context, step, captured = _adapter_harness(monkeypatch, execution_manifest=[_execution_source()]) + + class RefusedInput: + reference = "retained-web-result" + + def recheck(self): + raise ResultUnavailableError(reason) + + monkeypatch.setattr(images, "resolve_step_inputs", lambda step, context: {"facts": RefusedInput()}) + failed = images.adapter_generate_image(step, context, settings={}, user_id="user-1") + + assert failed["status"] == "failed" + assert failed["failure"]["code"] == code + assert reason not in failed["failure"]["message"] + assert captured["generated"] == 0 and "resolved" not in captured and "persisted" not in captured + + def test_reference_ids_change_step_fingerprint(): from functions_orchestration_result_contracts import canonical_digest diff --git a/functional_tests/test_orchestration_research_capture.py b/functional_tests/test_orchestration_research_capture.py index 15e6d985b..329daee1e 100644 --- a/functional_tests/test_orchestration_research_capture.py +++ b/functional_tests/test_orchestration_research_capture.py @@ -1,12 +1,15 @@ # test_orchestration_research_capture.py """Bounded research acquisition through actual construction, engines and readers. -Version: 0.261.139 +Version: 0.261.209 Implemented in: 0.261.127 Single orchestration contract updated in: 0.261.139 +Research planner request profiles admitted in: 0.261.209 Only external storage, metadata/provider transport and page I/O are doubled. The real planner constructor, attestor, current reader and result facade run. +Query and link planning call the run's own attested planner model. +Refs microsoft/simplechat#1509. """ from copy import deepcopy @@ -124,6 +127,61 @@ def observe_capture(*args, **kwargs): binding.close() +PLANNED_QUERY = "Current source facts official announcements" +CHILD_LINKS = ( + "https://example.com/source/current-facts-news", + "https://example.com/source/current-facts-report", +) + + +def answer_planner(state): + """Let the attested research planner answer with JSON, recording what it was asked. + + Each call records how many planner configurations were already attested, so a + test can prove the model was never reached before its constructor was captured. + """ + calls = [] + + def reply(**kwargs): + system, request = kwargs["messages"][0]["content"], json.loads(kwargs["messages"][1]["content"]) + attested = sum(1 for _, source in state.events if source and source["kind"] == "planner") + if "Deep Research web searches" in system: + kind = "query" + content = {"queries": [{"query": PLANNED_QUERY, "reason": "Official sources"}], "reason": "Planned"} + else: + kind = "link" + chosen = [candidate["url"] for candidate in request["candidates"]][-1:] + content = { + "selected_urls": [{"url": url, "reason": "Most relevant"} for url in chosen], + "needs_more_sources": False, "reason": "Planned", + } + calls.append({"kind": kind, "model": kwargs["model"], "attested": attested, "request": request}) + return SimpleNamespace(choices=[SimpleNamespace(message=SimpleNamespace(content=json.dumps(content)))]) + + state.planner_calls.side_effect = reply + return calls + + +def serve_linked_pages(state, monkeypatch): + """Give the seed page two same-site child links so link planning has a real choice.""" + fetched = [] + page_io = state.runtime.modules.review._fetch_source_page + + async def linked_page(**kwargs): + result = await page_io(**kwargs) + fetched.append(kwargs["url"]) + result["depth"] = kwargs.get("depth", 0) + if result["depth"] == 0: + result["links"] = [ + {"url": url, "anchor_text": f"Current source facts {url.rsplit('-', 1)[-1]}", "same_domain": True} + for url in CHILD_LINKS + ] + return result + + monkeypatch.setattr(state.runtime.modules.review, "_fetch_source_page", linked_page) + return fetched + + def test_research_consumes_the_canonical_descriptor_without_the_old_wrapper(research, monkeypatch): state = research models = state.metadata.modules.models @@ -213,29 +271,46 @@ def test_real_research_constructor_and_search_events_survive_current_only_restar state.planner_calls.assert_not_called() -@pytest.mark.parametrize("settings", [ - {"deep_research_enable_query_planning": True, "deep_research_max_search_queries_per_turn": 2}, - {"enable_deep_source_review": True, "source_review_enable_llm_planning": True}, -]) -def test_unattested_research_planner_request_profiles_stop_before_effects(research, settings): +@pytest.mark.parametrize("profile", ["query-planning", "link-planning"]) +def test_research_planner_request_profiles_are_attested_before_the_model_runs(research, monkeypatch, profile): + """Version 0.261.209: model planning runs, but only after its constructor is captured.""" state = research - state.runtime.settings.update(settings) - errors = importlib.import_module("functions_orchestration_invocation_capture") - capture = state.runtime.modules.adapters._external_invocation_capture( - state.step, state.context, state.runtime.settings, user_id="owner", capability_id="deep_research", - ) - with pytest.raises(errors.OrchestrationInvocationCaptureError): - state.runtime.modules.review.capture_research_planner_configuration( - settings=state.runtime.settings, planner_client=state.context.planner_client, - planner_model=state.context.planner_deployment, invocation_capture=capture, + fetched = None + if profile == "query-planning": + state.runtime.settings.update( + deep_research_enable_query_planning=True, deep_research_max_search_queries_per_turn=2, ) - assert state.events == [] - with pytest.raises(PermissionError) as refused: - run_gather(state.runtime, "deep_research") - assert refused.value.code == "result_unavailable" and refused.value.retryable is False - assert state.runtime.state.web == state.runtime.state.pages == [] - assert state.admissions == [] - state.planner_calls.assert_not_called() + else: + state.runtime.settings.update(enable_deep_source_review=True, source_review_enable_llm_planning=True) + fetched = serve_linked_pages(state, monkeypatch) + before = deepcopy(state.runtime.settings) + calls = answer_planner(state) + _, result = run_gather(state.runtime, "deep_research") + assert result["status"] == "completed", result + assert [call["kind"] for call in calls] == ["query" if profile == "query-planning" else "link"] + assert all(call["attested"] > 0 and call["model"] == state.binding.deployment for call in calls) + planner_sources = [source for _, source in state.events if source and source["kind"] == "planner"] + assert all(source["model"]["deployment"] == state.binding.deployment for source in planner_sources) + if profile == "query-planning": + assert len(state.runtime.state.web) == 2 + searched = [repr(invocation["messages"]) for invocation in state.runtime.state.invocations] + assert any(PLANNED_QUERY in text for text in searched) + else: + assert len(state.runtime.state.web) == 1 + assert fetched[-1] == CHILD_LINKS[-1] and CHILD_LINKS[0] not in fetched + assert state.runtime.settings == before + task = state.retention.retain_gather_result(state.step, state.context, result, source_manifest=[]) + assert len(state.admissions) == 1 + fresh = state.make_attestor() + service = state.world.service(state.world.provider(read_configuration=fresh.current, configuration_admitter=None)) + recovered = service.recover_task_result( + producer=state.context.result_producer(state.step), input_fingerprint=state.fingerprint, + ) + assert recovered == task + reader = service.open_result(recovered.output("prepared"), require_current_sources=True) + assert reader.read_value() == state.admissions[0][0] + assert fresh._captures == {} + assert state.planner_calls.call_count == len(calls) == 1 @pytest.mark.parametrize("mutation", ["foreign-client", "deployment", "closed", "completions", "model-binding"]) diff --git a/functional_tests/test_orchestration_research_pre_effect.py b/functional_tests/test_orchestration_research_pre_effect.py index 94693827c..572a77d0d 100644 --- a/functional_tests/test_orchestration_research_pre_effect.py +++ b/functional_tests/test_orchestration_research_pre_effect.py @@ -1,187 +1,151 @@ # test_orchestration_research_pre_effect.py """ Functional tests for deterministic research admission before acquisition. -Version: 0.261.139 +Version: 0.261.209 Implemented in: 0.261.127 Single orchestration contract updated in: 0.261.139 +Research planner profile restriction removed in: 0.261.209 -Canonical Source Review settings determine whether a captured research profile -can make unattested LLM planner requests. Check current and actual settings -without changing either, before metadata/provider/page/model work. Existing +Deep research with the default Source Review settings plans its searches and +links with the run's own attested planner model, as classic chat does. Current +and actual settings are compared without changing either, before any provider, +page or model effect; only a difference between them stops the step. Existing constructor evidence, observed Foundry runs and current-only reads stay distinct. Refs microsoft/simplechat#1509. """ from copy import deepcopy -from unittest.mock import Mock import pytest from test_orchestration_external_configuration_capture import capture_runtime, gather_modules, run_gather from test_orchestration_external_metadata import metadata_world, new_attestor, producer from test_orchestration_external_pre_effect import pre_effect_world -from test_orchestration_research_capture import research +from test_orchestration_research_capture import ( + CHILD_LINKS, PLANNED_QUERY, answer_planner, research, serve_linked_pages, +) +from test_support.versioning import assert_app_version_at_least -@pytest.mark.parametrize("settings,expected", [ - pytest.param({}, False, id="existing-defaults-use-link-planning"), - pytest.param({"enable_deep_source_review": False}, True, id="no-web-or-deep-review"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - }, False, id="existing-query-planning-default"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": 1, - }, True, id="one-query-never-needs-query-planner"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": 0, - }, True, id="query-count-clamped-to-one"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": "1", - }, True, id="normalized-one-query"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": 2, - }, False, id="multiple-queries-can-use-model"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": "invalid", - }, False, id="invalid-query-count-keeps-existing-default"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": None, - }, False, id="missing-query-count-keeps-existing-default"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": False, - "deep_research_max_search_queries_per_turn": 99, - "deep_research_enable_query_planning": False, - }, True, id="bounded-deterministic-multiple-queries"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": "off", - "deep_research_enable_query_planning": "false", - }, True, id="false-strings-use-canonical-normalization"), - pytest.param({ - "enable_web_search": True, "enable_deep_source_review": "off", - "deep_research_enable_query_planning": "true", - }, False, id="true-query-planning-string"), - pytest.param({ - "enable_web_search": False, "enable_deep_source_review": False, - "deep_research_enable_query_planning": True, - }, True, id="query-planning-inactive-without-web"), - pytest.param({ - "enable_web_search": False, "enable_deep_source_review": True, - "source_review_enable_llm_planning": False, - }, True, id="deterministic-deep-link-selection"), - pytest.param({ - "enable_web_search": False, "enable_deep_source_review": True, - "source_review_enable_llm_planning": "false", - }, True, id="normalized-deterministic-link-selection"), - pytest.param({ - "enable_web_search": False, "enable_deep_source_review": True, - "source_review_enable_llm_planning": True, "source_review_max_depth": 0, - }, False, id="zero-depth-does-not-relax-profile-boundary"), - pytest.param({ - "enable_web_search": False, "enable_deep_source_review": True, - "source_review_enable_llm_planning": True, "source_review_max_pages_per_turn": 1, - }, False, id="one-page-does-not-relax-profile-boundary"), - pytest.param({ - "enable_web_search": False, "enable_deep_source_review": "false", - "source_review_enable_llm_planning": "true", - }, True, id="link-planning-inactive-without-deep-review"), -]) -def test_shared_profile_uses_existing_normalization_without_mutation_or_io( - metadata_world, settings, expected, -): - world = metadata_world - before = deepcopy(settings) - supported = world.modules.configuration.is_research_acquisition_profile_supported(settings) - assert supported is expected - assert settings == before - assert world.requests == world.credentials == world.run_store.reads == [] +PLANNER_FLAGS = ( + "enable_deep_source_review", "source_review_enable_llm_planning", + "deep_research_enable_query_planning", "deep_research_max_search_queries_per_turn", +) +PROFILES = ["query-planning", "link-planning", "existing-defaults"] -@pytest.mark.parametrize("web_enabled", [None, 0, 1, "false", "true"]) -def test_profile_rejects_invalid_web_flags_without_boolean_coercion(metadata_world, web_enabled): - world = metadata_world - settings = {"enable_web_search": web_enabled, "enable_deep_source_review": False} - before = deepcopy(settings) - with pytest.raises(world.modules.configuration.ExternalConfigurationServiceError) as raised: - world.modules.configuration.is_research_acquisition_profile_supported(settings) - assert raised.value.code == "external_configuration_metadata_invalid" - assert raised.value.retryable is False - assert settings == before - assert world.requests == world.credentials == [] +def apply_profile(settings, profile): + if profile == "query-planning": + settings.update(deep_research_enable_query_planning=True, deep_research_max_search_queries_per_turn=2) + elif profile == "link-planning": + settings.update(enable_deep_source_review=True, source_review_enable_llm_planning=True) + else: + for name in PLANNER_FLAGS: + settings.pop(name, None) -@pytest.mark.parametrize("settings", [None, [], False]) -def test_profile_rejects_non_settings_objects(metadata_world, settings): - with pytest.raises(metadata_world.modules.contracts.ResultContractError): - metadata_world.modules.configuration.is_research_acquisition_profile_supported(settings) +def assert_no_paid_effects(world): + assert world.runtime.state.web == world.runtime.state.pages == world.runtime.state.invocations == [] + assert all(method == "GET" and "/assistants/" in url for method, url, _ in world.requests) + + +def test_research_planner_profile_gate_is_removed(metadata_world): + """Version 0.261.209: no settings profile refuses deep research on its own.""" + assert_app_version_at_least("0.261.209") + configuration = metadata_world.modules.configuration + assert not hasattr(configuration, "is_research_acquisition_profile_supported") + + +@pytest.mark.parametrize("profile", PROFILES) +def test_pre_effect_admits_matching_planner_profiles_without_captures(pre_effect_world, profile): + world = pre_effect_world + owner = world.configure("deep_research") + apply_profile(world.settings, profile) + actual = deepcopy(world.settings) + before_current, before_actual = deepcopy(world.settings), deepcopy(actual) + checked = world.provider.preflight_gather_acquisition("deep_research", producer=owner, settings=actual) + assert checked is None + assert world.identity_reads == [("owner", "conversation")] + assert world.attestor._captures == {} + assert world.settings == before_current and actual == before_actual + assert_no_paid_effects(world) @pytest.mark.parametrize("changed_side", ["current", "actual"]) -@pytest.mark.parametrize("profile", ["query-planning", "link-planning", "existing-defaults"]) -def test_pre_effect_rejects_either_unattested_profile_before_metadata( - pre_effect_world, changed_side, profile, -): +@pytest.mark.parametrize("profile", PROFILES) +def test_pre_effect_refuses_a_planner_profile_changed_on_one_side(pre_effect_world, changed_side, profile): world = pre_effect_world owner = world.configure("deep_research") actual = deepcopy(world.settings) - changed = world.settings if changed_side == "current" else actual - if profile == "query-planning": - changed.update(deep_research_enable_query_planning=True, deep_research_max_search_queries_per_turn=2) - elif profile == "link-planning": - changed.update(enable_deep_source_review=True, source_review_enable_llm_planning=True) - else: - for name in ( - "enable_deep_source_review", "source_review_enable_llm_planning", - "deep_research_enable_query_planning", "deep_research_max_search_queries_per_turn", - ): - changed.pop(name, None) - metadata = Mock(side_effect=AssertionError("Unsupported profiles must stop before metadata acquisition.")) - world.attestor.read_current_source = metadata + apply_profile(world.settings if changed_side == "current" else actual, profile) before_current, before_actual = deepcopy(world.settings), deepcopy(actual) with pytest.raises(world.modules.configuration.ResultUnavailableError) as raised: - world.provider.preflight_gather_acquisition( - "deep_research", producer=owner, settings=actual, - ) - assert raised.value.code == "external_configuration_research_profile_unsupported" - metadata.assert_not_called() - assert world.identity_reads == [("owner", "conversation")] + world.provider.preflight_gather_acquisition("deep_research", producer=owner, settings=actual) + assert raised.value.code == "external_configuration_changed" assert world.attestor._captures == {} - assert world.requests == world.credentials == world.run_store.reads == [] assert world.settings == before_current and actual == before_actual - assert world.runtime.state.web == world.runtime.state.pages == world.runtime.state.invocations == [] + assert_no_paid_effects(world) @pytest.mark.parametrize("changed_side", ["current", "actual"]) -@pytest.mark.parametrize("overrides", [ - {"deep_research_enable_query_planning": True, "deep_research_max_search_queries_per_turn": 2}, - {"enable_deep_source_review": True, "source_review_enable_llm_planning": True}, -]) -def test_real_research_refuses_profiles_before_constructor_capture_or_paid_effects( - research, monkeypatch, changed_side, overrides, -): - state = research - state.runtime.settings = deepcopy(state.world.settings) - changed = state.world.settings if changed_side == "current" else state.runtime.settings - changed.update(overrides) - before_current, before_actual = deepcopy(state.world.settings), deepcopy(state.runtime.settings) - getter = Mock(wraps=state.metadata.modules.models.planner_client_construction_source) - monkeypatch.setattr(state.metadata.modules.models, "planner_client_construction_source", getter) - with pytest.raises(PermissionError) as raised: - run_gather(state.runtime, "deep_research") - assert raised.value.code == "result_unavailable" +@pytest.mark.parametrize("switch", ["deep_research_enable_query_planning", "source_review_enable_llm_planning"]) +def test_pre_effect_refuses_a_planner_switch_changed_alone(pre_effect_world, changed_side, switch): + """Version 0.261.209: each planner switch is compared on its own, now that planners run.""" + world = pre_effect_world + owner = world.configure("deep_research") + apply_profile(world.settings, "existing-defaults") + actual = deepcopy(world.settings) + (world.settings if changed_side == "current" else actual)[switch] = False + before_current, before_actual = deepcopy(world.settings), deepcopy(actual) + with pytest.raises(world.modules.configuration.ResultUnavailableError) as raised: + world.provider.preflight_gather_acquisition("deep_research", producer=owner, settings=actual) + assert raised.value.code == "external_configuration_changed" + assert world.attestor._captures == {} + assert world.settings == before_current and actual == before_actual + assert_no_paid_effects(world) + + +@pytest.mark.parametrize("web_enabled", [None, 0, 1, "false", "true"]) +def test_pre_effect_rejects_invalid_web_flags_without_boolean_coercion(pre_effect_world, web_enabled): + world = pre_effect_world + owner = world.configure("deep_research") + world.settings["enable_web_search"] = web_enabled + actual = deepcopy(world.settings) + with pytest.raises(world.modules.configuration.ExternalConfigurationServiceError) as raised: + world.provider.preflight_gather_acquisition("deep_research", producer=owner, settings=actual) + assert raised.value.code == "external_configuration_metadata_invalid" assert raised.value.retryable is False - getter.assert_not_called() - state.planner_calls.assert_not_called() - assert state.events == state.admissions == [] - assert state.attestor._captures == {} - assert state.metadata.requests == state.runtime.state.web == state.runtime.state.pages == [] - assert state.runtime.state.invocations == state.runtime.state.clients == state.runtime.state.credentials == [] - assert state.world.settings == before_current and state.runtime.settings == before_actual + assert world.settings["enable_web_search"] == actual["enable_web_search"] == web_enabled + assert world.attestor._captures == {} + assert_no_paid_effects(world) + + +def test_default_research_settings_plan_searches_and_links_with_the_attested_planner(research, monkeypatch): + """The settings that were refused before 0.261.209 now complete, retain and recover.""" + state = research + for name in PLANNER_FLAGS: + state.runtime.settings.pop(name, None) + fetched = serve_linked_pages(state, monkeypatch) + calls = answer_planner(state) + before = deepcopy(state.runtime.settings) + _, result = run_gather(state.runtime, "deep_research") + assert result["status"] == "completed", result + assert [call["kind"] for call in calls] == ["query", "link"] + assert all(call["attested"] > 0 and call["model"] == state.binding.deployment for call in calls) + assert calls[0]["request"]["max_total_queries"] == 8 + assert 1 < len(state.runtime.state.web) <= 8 + assert any(PLANNED_QUERY in repr(invocation["messages"]) for invocation in state.runtime.state.invocations) + assert fetched[-1] == CHILD_LINKS[-1] and CHILD_LINKS[0] not in fetched + assert state.runtime.settings == before + task = state.retention.retain_gather_result(state.step, state.context, result, source_manifest=[]) + fresh = state.make_attestor() + reader = state.world.provider(read_configuration=fresh.current, configuration_admitter=None) + recovered = state.world.service(reader).recover_task_result( + producer=state.context.result_producer(state.step), input_fingerprint=state.fingerprint, + ) + assert recovered == task + assert fresh._captures == {} + assert state.planner_calls.call_count == 2 @pytest.mark.parametrize("overrides,expected_queries", [ @@ -201,9 +165,7 @@ def test_real_research_refuses_profiles_before_constructor_capture_or_paid_effec "enable_deep_source_review": True, "source_review_enable_llm_planning": "false", }, 3), ]) -def test_supported_profiles_use_real_research_without_changing_flags_or_request_controls( - research, overrides, expected_queries, -): +def test_profiles_without_an_active_planner_never_call_the_model(research, overrides, expected_queries): state = research state.runtime.settings.update(overrides) before = deepcopy(state.runtime.settings) @@ -232,7 +194,7 @@ def test_supported_profiles_use_real_research_without_changing_flags_or_request_ state.planner_calls.assert_not_called() -def test_new_acquisition_profile_check_does_not_rewrite_current_only_configuration(pre_effect_world): +def test_current_only_read_does_not_rewrite_planner_settings(pre_effect_world): world = pre_effect_world owner = world.configure("deep_research") world.settings.update( @@ -244,11 +206,13 @@ def test_new_acquisition_profile_check_does_not_rewrite_current_only_configurati assert current.identity and current.revision assert fresh._captures == {} assert world.settings == before - with pytest.raises(world.modules.configuration.ResultUnavailableError): - world.provider.preflight_gather_acquisition( - "deep_research", producer=producer(world, "deep_research"), settings=world.settings, - ) - assert world.runtime.state.web == world.runtime.state.pages == [] + checked = world.provider.preflight_gather_acquisition( + "deep_research", producer=producer(world, "deep_research"), settings=world.settings, + ) + assert checked is None + assert world.settings == before + assert world.attestor._captures == {} + assert_no_paid_effects(world) def test_current_profile_change_after_preparation_stops_before_search(research): @@ -264,10 +228,12 @@ def change_current_profile(source_type, **kwargs): ) state.context.capture_external_source_configuration = change_current_profile - with pytest.raises(PermissionError): + with pytest.raises(PermissionError) as raised: run_gather(state.runtime, "deep_research") + assert raised.value.code == "result_unavailable" + assert raised.value.authority_reason == "external_configuration_changed" assert state.events == [("deep_research", None)] - assert len(state.metadata.requests) == 1 + assert all(method == "GET" and "/assistants/" in url for method, url, _ in state.metadata.requests) assert state.admissions == [] assert state.runtime.state.web == state.runtime.state.pages == state.runtime.state.invocations == [] state.planner_calls.assert_not_called() diff --git a/functional_tests/test_orchestration_source_authority_runtime.py b/functional_tests/test_orchestration_source_authority_runtime.py index 04dbb2512..f8e5ba8ab 100644 --- a/functional_tests/test_orchestration_source_authority_runtime.py +++ b/functional_tests/test_orchestration_source_authority_runtime.py @@ -1,12 +1,13 @@ # test_orchestration_source_authority_runtime.py """V2 runtime/recovery must not turn uncertain source authority into a denial. -Version: 0.261.130 +Version: 0.261.209 Implemented in: 0.261.127 Configuration, recovery and invocation-control coverage added in: 0.261.130 +Saved-wait missing signed-in session message added in: 0.261.209 Real dispatch, composition, leases, checkpoints and retained-result recovery -preserve typed screening, directory and configuration errors. Caller-owned +preserve typed screening, identity and configuration errors. Caller-owned strict scopes fence caught failures; known holds and access denials keep their existing outcome. """ @@ -322,6 +323,34 @@ def test_saved_wait_uncertainty_never_clears_pending_results( assert context.failures == [] and len(waiting.case.model.calls) == 1 +@pytest.mark.parametrize("reason, code", [ + ("external_identity_session_unavailable", "external_session_required"), + ("external_identity_access_denied", "result_unavailable"), +]) +def test_saved_wait_without_a_signed_in_session_is_explained_to_the_user(waiting, monkeypatch, reason, code): + # A scheduler continuation has no signed-in session, so the wait ends with a clear next step. + results = importlib.import_module("functions_orchestration_results") + acquired = claim(waiting) + context = waiting.fresh_context(acquired["record"]) + monkeypatch.setattr( + waiting.runtime.executor, "resume_waiting_dependency_step", + unavailable(results.ResultUnavailableError(reason)), + ) + logs = [] + monkeypatch.setattr( + waiting.runtime.executor, "log_event", + lambda message, **kwargs: logs.append((message, kwargs.get("extra") or {})), + ) + result = waiting.run(acquired["record"], context) + [resumed] = [step for step in result["steps"] if step["step_id"] == "failed"] + assert result["status"] == "failed" + assert resumed["status"] == "failed" and resumed["failure"]["code"] == code + assert waiting.runtime.schema.FAILURE_MESSAGES[code] in result["message"] + assert len(waiting.case.model.calls) == 1 + [logged] = [extra for message, extra in logs if message.endswith("A saved wait could not be resumed.")] + assert logged["failure_code"] == code and logged["authority_reason"] == reason + + def test_saved_wait_metadata_failure_preserves_original_work(waiting, monkeypatch, metadata_failure): before = waiting.read_run("run-1") acquired = claim(waiting)