From a4d5bcdbc99e576f7840572a1537b9a22157210b Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Thu, 8 Oct 2026 12:24:39 -0400 Subject: [PATCH 1/2] Keep concurrent requests from erasing Microsoft 365 sign-ins, and create the workflow key automatically Flask-Session rewrote the whole session at the end of every request, so a slow V2 bootstrap refresh that overlapped a Microsoft 365 connect wrote back a stale copy, erased the pending sign-in, and /getAToken answered m365_auth_state_invalid. - Install merging Redis and filesystem session interfaces that save only the keys a request changed onto the latest stored session, refresh only the TTL when nothing changed, and never recreate a session deleted mid-request. - Create the deployment-wide workflow encryption key in Key Vault on first connect and on admin settings saves, report readiness, and route workflow sign-in through /getAToken. - Show sign-in results on a page that reports back to the popup's opener instead of raw JSON. - Derive the chat connection's sources from the session token cache, keep V2 links in V2, and skip V2 focus refreshes while a recent one exists. - Bump to 0.261.302 with tests, fix documentation and release notes. Fixes #1714 Refs #1604 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/app.py | 6 +- application/single_app/config.py | 2 +- .../single_app/functions_m365_connections.py | 267 ++++++++- .../single_app/functions_session_store.py | 262 +++++++++ application/single_app/route_backend_m365.py | 198 +++++-- application/single_app/route_backend_v2.py | 16 + .../route_frontend_admin_settings.py | 17 + .../route_frontend_authentication.py | 6 +- .../static/js/chat/chat-m365-connect.js | 12 +- .../profile/profile-m365-connection-result.js | 83 +++ .../static/js/profile/profile-m365.js | 60 +- .../templates/m365_connection_result.html | 37 ++ application/single_app/templates/profile.html | 6 +- application/v2_ui/src/App.tsx | 13 +- .../approvals/PausedRequestsPanel.tsx | 11 +- .../approvals/PendingActionsPanel.tsx | 11 +- .../chat/OrchestrationM365Notice.tsx | 19 +- .../components/chat/WorkflowProposalCard.tsx | 12 +- .../src/components/chat/WorkflowRunCard.tsx | 4 +- .../src/components/settings/M365Cards.tsx | 101 +++- application/v2_ui/src/lib/m365Connect.ts | 171 ++++-- application/v2_ui/src/lib/m365Links.ts | 12 +- application/v2_ui/src/pages/SettingsPage.tsx | 16 + .../v2_ui/src/stores/bootstrapStore.ts | 45 +- docs/_data/app_surface.yml | 4 +- docs/admin/security.md | 2 + .../CHAT_ORCHESTRATION_WORKFLOWS_ROADMAP.md | 5 +- .../features/MICROSOFT_365_ACTIONS.md | 36 +- .../fixes/M365_CONNECTION_SESSION_RACE_FIX.md | 227 ++++++++ docs/explanation/release_notes.md | 25 + docs/reference/logging-tags.md | 1 + ...st_ai_connection_embedding_defaults_api.py | 3 + ...st_content_screening_admin_settings_fix.py | 3 + functional_tests/test_m365_connections.py | 235 +++++++- functional_tests/test_m365_routes.py | 160 +++++- functional_tests/test_session_store_merge.py | 525 ++++++++++++++++++ .../test_v2_user_settings_memory_m365.py | 44 +- ..._v2_workflow_run_tracking_xss_guardrail.py | 21 +- .../fixtures/orchestration/harness_entry.tsx | 3 + ui_tests/test_m365_connection_result_page.py | 229 ++++++++ ui_tests/test_m365_lifecycle_and_approvals.py | 12 +- ui_tests/test_m365_pending_action_cards.py | 4 +- .../test_v2_orchestration_m365_recovery.py | 66 ++- ...v2_orchestration_workflow_proposal_card.py | 9 +- ui_tests/test_v2_settings_m365_connections.py | 233 ++++++++ 45 files changed, 2961 insertions(+), 273 deletions(-) create mode 100644 application/single_app/functions_session_store.py create mode 100644 application/single_app/static/js/profile/profile-m365-connection-result.js create mode 100644 application/single_app/templates/m365_connection_result.html create mode 100644 docs/explanation/fixes/M365_CONNECTION_SESSION_RACE_FIX.md create mode 100644 functional_tests/test_session_store_merge.py create mode 100644 ui_tests/test_m365_connection_result_page.py create mode 100644 ui_tests/test_v2_settings_m365_connections.py diff --git a/application/single_app/app.py b/application/single_app/app.py index 1f72fd2ce..8e4b4b4b2 100644 --- a/application/single_app/app.py +++ b/application/single_app/app.py @@ -38,6 +38,7 @@ from functions_rate_limit import build_rate_limit_error_payload from functions_appinsights import * from functions_activity_logging import * +from functions_session_store import install_merging_session_interface import threading import time @@ -186,6 +187,7 @@ log_event(f"Unable to create session directory {app.config.get('SESSION_FILE_DIR')}: {e}", level=logging.ERROR) Session(app) +install_merging_session_interface(app) def register_route_blueprint(name, registrar, auth_guard=None): @@ -302,8 +304,10 @@ def configure_sessions(settings): log_event(f"Session configuration error; falling back to filesystem: {e}", level=logging.ERROR) app.config['SESSION_TYPE'] = 'filesystem' - # Initialize session interface + # Initialize session interface. Saves merge into the latest stored session so a slow + # request cannot overwrite what a concurrent request saved meanwhile. Session(app) + install_merging_session_interface(app) # =================== Helper Functions =================== def start_background_tasks(): diff --git a/application/single_app/config.py b/application/single_app/config.py index 143f847b8..ef6a7d47c 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -101,7 +101,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.301" +VERSION = "0.261.302" 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_m365_connections.py b/application/single_app/functions_m365_connections.py index 7281a3144..53e8aafe7 100644 --- a/application/single_app/functions_m365_connections.py +++ b/application/single_app/functions_m365_connections.py @@ -22,7 +22,7 @@ import msal import requests from azure.core import MatchConditions -from azure.core.exceptions import AzureError +from azure.core.exceptions import AzureError, HttpResponseError, ResourceNotFoundError from azure.cosmos import exceptions as cosmos_exceptions from cryptography.exceptions import InvalidTag from cryptography.hazmat.primitives.ciphers.aead import AESGCM @@ -43,6 +43,9 @@ KEY_SECRET_ENV = "M365_WORKFLOW_TOKEN_KEY_SECRET_NAME" +DEFAULT_KEY_SECRET_NAME = "simplechat-m365-workflow-token-key" +KEY_SECRET_CONTENT_TYPE = "application/vnd.simplechat.m365-workflow-key+base64" +KEY_SECRET_TAGS = {"simplechat-purpose": "m365-workflow-token-key"} ENCRYPTION_VERSION = 1 MAX_CACHE_BYTES = 512 * 1024 AUTH_FLOW_SECONDS = 600 @@ -51,6 +54,11 @@ CHAT_CALLBACK_PATH = "/getAToken" CHAT_AUTH_SESSION_KEY = "m365_chat_auth_flow" CHAT_AUTH_STATE_PREFIX = "m365-chat-" +# Workflow sign-in returns to the registered /getAToken callback too; the prefix routes it. +WORKFLOW_AUTH_STATE_PREFIX = "m365-workflow-" +# How the browser that started a sign-in hears the result: a full-page redirect, or a popup +# that reports to the window that opened it. +COMPLETION_MODES = frozenset({"page", "popup"}) CHAT_CONNECTION_SESSION_KEY = "m365_chat_connection" CHAT_RECONNECT_SESSION_KEY = "m365_chat_reconnect_required" _SOURCE_SCOPE_NAMES = { @@ -284,6 +292,33 @@ def _source_connection_scopes(sources): return sorted(set().union(*(_SOURCE_CONNECT_SCOPE_NAMES[source] for source in sources))) +def _granted_session_sources(cache, account, config): + """Sources whose permissions this cached sign-in already holds. Reads the cache only. + + Chat preflight silently requests each source's connect scopes with the session's refresh + token, so a source is usable when a refresh token exists and Microsoft granted those + scopes. Microsoft returns every scope consented for the resource on each token, which is + what MSAL records as the cached access token's target. + """ + home_account_id = account.get("home_account_id") + + def own(token): + return token.get("home_account_id") == home_account_id and token.get("client_id") == config.client_id + + if not any(own(token) for token in cache.search(msal.TokenCache.CredentialType.REFRESH_TOKEN)): + return [] + prefix = f"{config.graph_resource}/".lower() + granted = { + scope.lower().removeprefix(prefix) + for token in cache.search(msal.TokenCache.CredentialType.ACCESS_TOKEN) if own(token) + for scope in str(token.get("target") or "").split() + } + return [ + source for source in M365_SOURCES + if {name.lower() for name in _SOURCE_CONNECT_SCOPE_NAMES[source]} <= granted + ] + + def mark_m365_chat_reconnect_required(context): """Fence rejected interactive credentials without revoking workflow connections.""" if not has_request_context() or context is None or context.workflow_id: @@ -312,9 +347,12 @@ def _require_granted_scopes(requested_scopes, granted_scope, config): def _validate_callback_uri(redirect_uri, *, interactive=False): + # Workflow sign-in uses the registered /getAToken callback; the dedicated workflow + # callback stays accepted for deployments that registered it. + allowed_paths = {CHAT_CALLBACK_PATH} if interactive else {CHAT_CALLBACK_PATH, CONNECTION_CALLBACK_PATH} redirect = urlsplit(redirect_uri) if ( - redirect.path != (CHAT_CALLBACK_PATH if interactive else CONNECTION_CALLBACK_PATH) + redirect.path not in allowed_paths or redirect.query or redirect.fragment or redirect.username or redirect.password or not redirect.hostname or (redirect.scheme != "https" and not ( @@ -368,37 +406,134 @@ def _default_msal_factory(cache, config): ) -def _default_key_provider(version=None, name=None): +_KEY_VAULT_NAME_PATTERN = r"[A-Za-z0-9][A-Za-z0-9-]{1,22}[A-Za-z0-9]" +_KEY_SECRET_NAME_PATTERN = r"[A-Za-z0-9-]{1,127}" +_KEY_VAULT_SETTINGS_LOCATION = "Admin Settings > Security > Secrets > Key Vault" + + +def workflow_key_secret_name(): + """The Key Vault secret holding the deployment's one workflow encryption key. + + Every user's saved workflow sign-in is encrypted separately with this key and bound to + that user's connection record. The app setting is optional; it renames the secret. + """ + return (os.environ.get(KEY_SECRET_ENV) or "").strip() or DEFAULT_KEY_SECRET_NAME + + +def workflow_connection_readiness(settings): + """Whether saved workflow connections can work here, from settings alone (no Key Vault call). + + The encryption key itself is created on first use, so a missing key is not a reason. + """ + settings = settings if isinstance(settings, dict) else {} + vault_name = settings.get("key_vault_name") + if not settings.get("enable_key_vault_secret_storage"): + return { + "available": False, "reason": "key_vault_disabled", + "message": ( + "Workflow connections are not set up on this deployment yet: they need Key Vault " + f"secret storage. An administrator can turn it on in {_KEY_VAULT_SETTINGS_LOCATION}." + ), + } + if not isinstance(vault_name, str) or not re.fullmatch(_KEY_VAULT_NAME_PATTERN, vault_name.strip()): + return { + "available": False, "reason": "key_vault_name_missing", + "message": ( + "Workflow connections are not set up on this deployment yet: they need a Key Vault " + f"name. An administrator can set it in {_KEY_VAULT_SETTINGS_LOCATION}." + ), + } + if not re.fullmatch(_KEY_SECRET_NAME_PATTERN, workflow_key_secret_name()): + return { + "available": False, "reason": "key_secret_name_invalid", + "message": ( + f"The {KEY_SECRET_ENV} app setting is not a valid Key Vault secret name. An " + "administrator must correct it or remove it to use the default name." + ), + } + return {"available": True, "reason": None, "message": ""} + + +def _log_key_created(name, version): + # The logger depends on config/settings; only a live key creation reaches this. + from functions_appinsights import log_event + log_event( + "[AUTH] Created the Microsoft 365 workflow encryption key in Key Vault.", + extra={"secret_name": name, "key_version": version}, + ) + + +def _create_workflow_key_secret(client, name): + """Store a new random 256-bit key and return its properties and value. + + The value is never logged and never leaves the server. + """ + value = base64.b64encode(secrets.token_bytes(32)).decode("ascii") + try: + created = client.set_secret( + name, value, content_type=KEY_SECRET_CONTENT_TYPE, tags=dict(KEY_SECRET_TAGS), + ) + except HttpResponseError as exc: + _log_failure("m365_key_provision_failed", exc) + if getattr(exc, "status_code", None) == 403: + raise M365ConnectionError( + "m365_key_provision_forbidden", + "SimpleChat could not create the workflow encryption key because its identity cannot " + "write Key Vault secrets. An administrator must grant it Key Vault Secrets Officer on the vault.", + ) from exc + if getattr(exc, "status_code", None) == 409: + raise M365ConnectionError( + "m365_key_deleted", + f"The workflow encryption key secret '{name}' was deleted and is still recoverable. " + "An administrator must recover or purge it in Key Vault.", + ) from exc + raise M365ConnectionError("m365_key_unavailable", "The workflow encryption key could not be created.") from exc + except AzureError as exc: + _log_failure("m365_key_provision_failed", exc) + raise M365ConnectionError("m365_key_unavailable", "The workflow encryption key could not be created.") from exc + _log_key_created(name, created.properties.version) + return created.properties, value + + +def _load_workflow_key(settings, version=None, name=None, *, create_missing=False): + """Read the workflow key from Key Vault, creating it only when asked and it does not exist. + + Returns the key and whether it was just created. Decryption always pins the stored key + version and never creates one, so two instances creating it at once is harmless: each + version stays readable for what it encrypted. + """ # Key Vault is mandatory; there is deliberately no Flask-secret/plaintext fallback. from azure.keyvault.secrets import SecretClient import config as app_config from functions_keyvault import get_keyvault_credential - from functions_settings import get_settings - configured_name = os.environ.get(KEY_SECRET_ENV, "") - settings = get_settings() - vault_name = settings.get("key_vault_name") - if ( - not settings.get("enable_key_vault_secret_storage") - or not isinstance(vault_name, str) - or not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9-]{1,22}[A-Za-z0-9]", vault_name.strip()) - or not re.fullmatch(r"[A-Za-z0-9-]{1,127}", configured_name) - or (name is not None and name != configured_name) - ): + readiness = workflow_connection_readiness(settings) + if not readiness["available"]: + raise M365ConnectionError("m365_key_vault_required", readiness["message"]) + configured_name = workflow_key_secret_name() + if name is not None and name != configured_name: raise M365ConnectionError( - "m365_key_vault_required", - "Configure Key Vault and a dedicated workflow encryption-key secret before connecting Microsoft 365.", + "m365_key_unavailable", + "The workflow encryption key that protected this connection is no longer configured. " + "Reconnect Microsoft 365 for workflows.", ) client = SecretClient( - vault_url=f"https://{vault_name.strip()}{app_config.KEY_VAULT_DOMAIN}", + vault_url=f"https://{settings['key_vault_name'].strip()}{app_config.KEY_VAULT_DOMAIN}", credential=get_keyvault_credential(settings=settings), ) + created = False try: secret = client.get_secret(configured_name, version=version) + properties, secret_value = secret.properties, secret.value + except ResourceNotFoundError as exc: + if version is not None or not create_missing: + _log_failure("m365_key_unavailable", exc) + raise M365ConnectionError("m365_key_unavailable", "The workflow encryption key is unavailable.") from exc + properties, secret_value = _create_workflow_key_secret(client, configured_name) + created = True except AzureError as exc: _log_failure("m365_key_unavailable", exc) raise M365ConnectionError("m365_key_unavailable", "The workflow encryption key is unavailable.") from exc - properties = secret.properties if ( properties.enabled is False or (properties.expires_on is not None and utc_datetime(properties.expires_on) <= utc_now()) @@ -407,10 +542,46 @@ def _default_key_provider(version=None, name=None): ): raise M365ConnectionError("m365_key_unavailable", "The workflow encryption-key version is unavailable.") try: - raw_key = base64.b64decode(secret.value, validate=True) + raw_key = base64.b64decode(secret_value, validate=True) except (ValueError, TypeError, binascii.Error) as exc: raise M365ConnectionError("m365_key_invalid", "The workflow encryption key must contain a base64-encoded 256-bit key.") from exc - return M365EncryptionKey(key=raw_key, version=properties.version, name=configured_name) + return M365EncryptionKey(key=raw_key, version=properties.version, name=configured_name), created + + +def _default_key_provider(version=None, name=None): + from functions_settings import get_settings + + # Encrypting (no version) creates the deployment's key on first use. + key, _created = _load_workflow_key(get_settings(), version, name, create_missing=version is None) + return key + + +def ensure_m365_workflow_encryption_key(settings): + """Create the workflow encryption key now if it is missing. Never raises. + + Admin settings saves call this so the key exists before anyone connects. The result is + safe to show an administrator. + """ + readiness = workflow_connection_readiness(settings) + if not readiness["available"]: + return {"status": "not_configured", "reason": readiness["reason"], "message": readiness["message"]} + try: + _key, created = _load_workflow_key(settings, create_missing=True) + except M365ConnectionError as exc: + return {"status": "unavailable", "reason": exc.code, "message": exc.payload["message"]} + except Exception as exc: + # Admin saves call this after storing settings; an optional key check must not fail them. + _log_failure("m365_key_provision_failed", exc) + return { + "status": "unavailable", "reason": "m365_key_unavailable", + "message": "The workflow encryption key could not be checked in Key Vault.", + } + if created: + return { + "status": "created", "reason": None, + "message": f"Created the Microsoft 365 workflow encryption key '{workflow_key_secret_name()}' in Key Vault.", + } + return {"status": "ready", "reason": None, "message": ""} def sanitize_m365_connection(connection): @@ -513,12 +684,16 @@ def _ensure_connection(self, user_id, config): except cosmos_exceptions.CosmosResourceExistsError: return self._read(connection_id, user_id) - def start_connection(self, user_id, tenant_id, sources, redirect_uri, session_binding, scopes=None): + def start_connection( + self, user_id, tenant_id, sources, redirect_uri, session_binding, scopes=None, completion="page", + ): config = self.config_provider() _identifier(user_id) _identifier(session_binding) if tenant_id != config.tenant_id: raise M365ConnectionError("m365_account_mismatch", "Connect only your account in this deployment's tenant.") + if completion not in COMPLETION_MODES: + raise ValueError("Unsupported sign-in completion mode.") if ( not isinstance(sources, list) or not sources or len(sources) > len(M365_SOURCES) or any(source not in M365_SOURCES for source in sources) @@ -541,7 +716,7 @@ def start_connection(self, user_id, tenant_id, sources, redirect_uri, session_bi client = self.msal_factory(cache, config) flow = client.initiate_auth_code_flow( scopes=required, redirect_uri=redirect_uri, - state=secrets.token_urlsafe(32), prompt="select_account", + state=f"{WORKFLOW_AUTH_STATE_PREFIX}{secrets.token_urlsafe(32)}", prompt="select_account", ) _validate_auth_flow(flow, config) expires_at = self.clock() + timedelta(seconds=AUTH_FLOW_SECONDS) @@ -554,7 +729,7 @@ def start_connection(self, user_id, tenant_id, sources, redirect_uri, session_bi "session_binding": hashlib.sha256(session_binding.encode("utf-8")).hexdigest(), "status": "pending", "sources": sorted(set(sources)), "requested_scopes": required, "expires_at": expires_at.isoformat(), - "ttl": AUTH_FLOW_SECONDS * 2, + "completion": completion, "ttl": AUTH_FLOW_SECONDS * 2, } record["encrypted_flow"] = encrypt_m365_cache( json.dumps(flow, separators=(",", ":")), record, key, @@ -565,6 +740,23 @@ def start_connection(self, user_id, tenant_id, sources, redirect_uri, session_bi "expires_at": expires_at.isoformat(), } + def connection_flow_completion(self, user_id, state): + """How the browser that started a workflow sign-in expects the result: page, popup or auto. + + Read before completing so even a failed callback can answer a popup. It never raises: + an unknown or unreadable flow reports "auto" and the result page decides. + """ + if not isinstance(state, str) or not 20 <= len(state) <= 256: + return "auto" + try: + record = self._read(f"m365-oauth-{hashlib.sha256(state.encode('utf-8')).hexdigest()}", user_id) + except (AzureError, ValueError, TypeError): + return "auto" + if not isinstance(record, dict) or record.get("purpose") != "m365_workflow_connection": + return "auto" + completion = record.get("completion") + return completion if completion in COMPLETION_MODES else "page" + def complete_connection(self, user_id, tenant_id, auth_response, session_binding, *, cache_writer=None): config = self.config_provider() state = auth_response.get("state") if isinstance(auth_response, dict) else None @@ -660,7 +852,14 @@ def _interactive_config(self, user_id, tenant_id): return config def read_chat_connection(self, user_id, tenant_id): - """Report local session state without probing Graph or exposing credential material.""" + """Report local session state without probing Graph or exposing credential material. + + ``sources`` are the sources chat can use now: the session holds a refresh token for + the user and Microsoft has granted each source's permissions, as recorded on the + cached access tokens. That includes consent given at sign-in or by an administrator, + not only an explicit reconnect. When a reconnect is needed, ``sources`` lists the + sources last connected here, to offer them again. + """ config = self._interactive_config(user_id, tenant_id) metadata = session.get(CHAT_CONNECTION_SESSION_KEY) if not isinstance(metadata, dict) or ( @@ -679,24 +878,27 @@ def read_chat_connection(self, user_id, tenant_id): try: cache = deserialize_m365_cache(session["token_cache"]) accounts = list(cache.search(msal.TokenCache.CredentialType.ACCOUNT)) - select_m365_account(accounts, user_id, tenant_id) + account = select_m365_account(accounts, user_id, tenant_id) result["status"] = "available" + result["sources"] = _granted_session_sources(cache, account, config) except M365ConnectionError: result["status"] = "reconnect_required" if metadata.get("connected_at"): result["connected_at"] = metadata["connected_at"] return result - def start_profile_chat_connection(self, user_id, tenant_id, sources, redirect_uri): + def start_profile_chat_connection(self, user_id, tenant_id, sources, redirect_uri, completion="page"): scopes = _source_connection_scopes(sources) + if completion not in COMPLETION_MODES: + raise ValueError("Unsupported sign-in completion mode.") return self._start_interactive_connection( user_id, tenant_id, scopes, redirect_uri, - purpose="profile_reconnect", sources=sorted(set(sources)), + purpose="profile_reconnect", sources=sorted(set(sources)), completion=completion, ) def _start_interactive_connection( self, user_id, tenant_id, scopes, redirect_uri, *, - purpose, request_id=None, conversation_id=None, sources=None, + purpose, request_id=None, conversation_id=None, sources=None, completion="page", ): config = self._interactive_config(user_id, tenant_id) _validate_callback_uri(redirect_uri, interactive=True) @@ -719,7 +921,7 @@ def _start_interactive_connection( "flow": flow, "user_id": user_id, "tenant_id": tenant_id, "request_id": request_id, "conversation_id": conversation_id, "configuration": config.binding(), "required_scopes": required, - "purpose": purpose, "sources": sources, + "purpose": purpose, "sources": sources, "completion": completion, "expires_at": expires_at.isoformat(), } return {"authorization_url": flow["auth_uri"], "expires_at": expires_at.isoformat()} @@ -738,7 +940,10 @@ def complete_chat_connection(self, user_id, tenant_id, auth_response): or record.get("configuration") != config.binding() or utc_datetime(record["expires_at"]) <= self.clock() ): - raise M365ConnectionError("m365_auth_state_invalid", "This sign-in request expired or changed. Connect again from chat.") + raise M365ConnectionError( + "m365_auth_state_invalid", + "This Microsoft 365 sign-in request expired or was replaced by a newer one. Start the connection again.", + ) session.pop(CHAT_AUTH_SESSION_KEY) cache = msal.SerializableTokenCache() client = self.msal_factory(cache, config) @@ -768,6 +973,8 @@ def complete_chat_connection(self, user_id, tenant_id, auth_response): } session.pop(CHAT_RECONNECT_SESSION_KEY, None) if record.get("purpose") == "profile_reconnect": + if record.get("completion") == "popup": + return {"return_to": "profile", "completion": "popup"} return {"return_to": "profile"} return { "request_id": record["request_id"], "conversation_id": record["conversation_id"], diff --git a/application/single_app/functions_session_store.py b/application/single_app/functions_session_store.py new file mode 100644 index 000000000..c3dd085ba --- /dev/null +++ b/application/single_app/functions_session_store.py @@ -0,0 +1,262 @@ +# functions_session_store.py +"""Server-side session persistence that does not lose concurrent writes. + +Flask-Session 0.8 writes the whole stored session back at the end of every request, so the +request that finishes last wins. A slow request that loaded the session before another +request saved a change then writes its stale copy over that change. SimpleChat runs several +instances and threads against one Redis store, and the V2 interface keeps several requests in +flight, so this happens in practice: a pending Microsoft 365 sign-in saved by one request was +erased by a bootstrap request that had started earlier and finished later. + +These interfaces keep Flask-Session's cookie and storage format and change only how a request +writes its changes back: + +* A request that changed nothing only refreshes the stored session's expiry. +* A request that changed keys writes just those keys, and removes just the keys it removed, + into the latest stored copy. Redis applies the merge atomically. +* A new session, a cleared session and a session whose id changed are written whole, as + before. +* A session deleted while the request ran, by sign-out or the idle timeout, is not recreated + from the request's stale copy. + +Two requests that change the same key still resolve last writer wins, as before. +""" + +import copy +import logging +import warnings + +from flask_session.defaults import Defaults +from flask_session.filesystem import FileSystemSession, FileSystemSessionInterface +from flask_session.redis import RedisSession, RedisSessionInterface +from redis.exceptions import WatchError + +from functions_appinsights import log_event + + +MERGE_ATTEMPTS = 5 +PERMANENT_KEY = "_permanent" +_BASELINE_ATTR = "_simplechat_baseline" +_BASELINE_SID_ATTR = "_simplechat_baseline_sid" +_REPLACED_ATTR = "_simplechat_replaced" + + +def _ttl_seconds(session_lifetime): + """Whole seconds in the session lifetime, as Flask-Session computes its storage TTL.""" + return session_lifetime.days * 86400 + session_lifetime.seconds + + +def _has_content(data): + """A stored session needs at least one key besides the permanent flag.""" + return any(key != PERMANENT_KEY for key in data) + + +def _apply_changes(stored, changed, removed): + """The stored session with this request's changes applied, other keys untouched.""" + merged = dict(stored) if isinstance(stored, dict) else {} + merged.update(changed) + for key in removed: + merged.pop(key, None) + return merged + + +def session_changes(session): + """Keys this request changed and removed, or None when the session must be written whole. + + A session opened by another interface carries no baseline, which happens on a worker's + first request when initialization swaps the session backend mid-request, and is written + whole as before. + """ + baseline = getattr(session, _BASELINE_ATTR, None) + if ( + baseline is None + or getattr(session, _REPLACED_ATTR, False) + or getattr(session, _BASELINE_SID_ATTR, None) != session.sid + ): + return None + current = dict(session) + changed = { + key: value for key, value in current.items() + if key not in baseline or baseline[key] != value + } + removed = [key for key in baseline if key not in current] + return changed, removed + + +class _MergingSessionMixin: + """Record what a request loaded so the save can write only what it changed.""" + + def open_session(self, app, request): + session = super().open_session(app, request) + # A stored session always holds more than the permanent flag; a new one does not. + if session: + setattr(session, _BASELINE_ATTR, copy.deepcopy(dict(session))) + setattr(session, _BASELINE_SID_ATTR, session.sid) + return session + + def _upsert_session(self, session_lifetime, session, store_id): + changes = session_changes(session) + if changes is None: + self._write_whole(session_lifetime, session, store_id) + return + changed, removed = changes + ttl = _ttl_seconds(session_lifetime) + if not changed and not removed: + self._refresh_expiry(store_id, ttl) + return + self._merge_changes(session_lifetime, session, store_id, changed, removed, ttl) + + def _write_whole(self, session_lifetime, session, store_id): + """Flask-Session's own save, which replaces the stored session with this one.""" + super()._upsert_session(session_lifetime, session, store_id) + + def _refresh_expiry(self, store_id, ttl): + raise NotImplementedError() + + def _merge_changes(self, session_lifetime, session, store_id, changed, removed, ttl): + raise NotImplementedError() + + +class MergingRedisSession(RedisSession): + def clear(self): + setattr(self, _REPLACED_ATTR, True) + super().clear() + + +class MergingRedisSessionInterface(_MergingSessionMixin, RedisSessionInterface): + """Redis sessions whose saves merge into the latest stored copy with WATCH/MULTI/EXEC.""" + + session_class = MergingRedisSession + + def _refresh_expiry(self, store_id, ttl): + # EXPIRE never recreates a key, so a session deleted meanwhile stays deleted. + self.client.expire(store_id, ttl) + + def _decode_stored(self, stored): + try: + return self.serializer.decode(stored) + except Exception as exc: + log_event( + "[SESSION_STORE] The stored session could not be read; writing this request's session whole.", + extra={"exception_type": type(exc).__name__}, + level=logging.WARNING, + ) + return None + + def _merge_changes(self, session_lifetime, session, store_id, changed, removed, ttl): + for _attempt in range(MERGE_ATTEMPTS): + with self.client.pipeline() as pipe: + try: + pipe.watch(store_id) + stored = pipe.get(store_id) + if stored is None: + # Signed out or expired while this request ran: do not recreate it. + return + current = self._decode_stored(stored) + if current is None: + pipe.reset() + self._write_whole(session_lifetime, session, store_id) + return + merged = _apply_changes(current, changed, removed) + pipe.multi() + if _has_content(merged): + pipe.set(store_id, self.serializer.encode(merged), ex=ttl) + else: + pipe.delete(store_id) + pipe.execute() + return + except WatchError: + continue + log_event( + "[SESSION_STORE] Session save kept losing an optimistic-lock race; merging without the lock.", + extra={"attempts": MERGE_ATTEMPTS, "changed_key_count": len(changed), "removed_key_count": len(removed)}, + level=logging.WARNING, + ) + stored = self.client.get(store_id) + if stored is None: + return + current = self._decode_stored(stored) + if current is None: + self._write_whole(session_lifetime, session, store_id) + return + merged = _apply_changes(current, changed, removed) + if _has_content(merged): + self.client.set(name=store_id, value=self.serializer.encode(merged), ex=ttl) + else: + self.client.delete(store_id) + + +class MergingFileSystemSession(FileSystemSession): + def clear(self): + setattr(self, _REPLACED_ATTR, True) + super().clear() + + +class MergingFileSystemSessionInterface(_MergingSessionMixin, FileSystemSessionInterface): + """Filesystem sessions that merge into the latest stored copy. + + The file cache has no transactions, so the merge is a read and an atomic file replace a + moment apart rather than the whole request apart. This backend serves single-instance + and development deployments, and the Redis fallback. + """ + + session_class = MergingFileSystemSession + + def _refresh_expiry(self, store_id, ttl): + stored = self.cache.get(store_id) + if stored is not None: + self.cache.set(key=store_id, value=stored, timeout=ttl) + + def _merge_changes(self, session_lifetime, session, store_id, changed, removed, ttl): + stored = self.cache.get(store_id) + if stored is None: + return + merged = _apply_changes(stored, changed, removed) + if _has_content(merged): + self.cache.set(key=store_id, value=merged, timeout=ttl) + else: + self.cache.delete(store_id) + + +def install_merging_session_interface(app): + """Replace the interface ``Session(app)`` installed with its merging equivalent. + + Call it straight after every ``Session(app)``. The replacement reuses the installed + interface's Redis client or file cache, so storage and existing sessions are unchanged. + """ + current = app.session_interface + if isinstance(current, _MergingSessionMixin): + return current + config = app.config + common = { + "app": app, + "key_prefix": config.get("SESSION_KEY_PREFIX", Defaults.SESSION_KEY_PREFIX), + "use_signer": config.get("SESSION_USE_SIGNER", Defaults.SESSION_USE_SIGNER), + "permanent": config.get("SESSION_PERMANENT", Defaults.SESSION_PERMANENT), + "sid_length": config.get("SESSION_ID_LENGTH", Defaults.SESSION_ID_LENGTH), + "serialization_format": config.get( + "SESSION_SERIALIZATION_FORMAT", Defaults.SESSION_SERIALIZATION_FORMAT, + ), + } + # Session(app) already raised these deprecation warnings for the same configuration. + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + if isinstance(current, RedisSessionInterface): + replacement = MergingRedisSessionInterface(client=current.client, **common) + elif isinstance(current, FileSystemSessionInterface): + replacement = MergingFileSystemSessionInterface( + cache_dir=config.get("SESSION_FILE_DIR", Defaults.SESSION_FILE_DIR), + threshold=config.get("SESSION_FILE_THRESHOLD", Defaults.SESSION_FILE_THRESHOLD), + mode=config.get("SESSION_FILE_MODE", Defaults.SESSION_FILE_MODE), + **common, + ) + replacement.cache = current.cache + else: + log_event( + "[SESSION_STORE] The configured session backend has no merging interface; concurrent saves stay last writer wins.", + extra={"session_interface": type(current).__name__}, + level=logging.WARNING, + ) + return current + app.session_interface = replacement + return replacement diff --git a/application/single_app/route_backend_m365.py b/application/single_app/route_backend_m365.py index e03052009..d1dff333e 100644 --- a/application/single_app/route_backend_m365.py +++ b/application/single_app/route_backend_m365.py @@ -8,7 +8,7 @@ import requests from azure.core.exceptions import AzureError -from flask import Blueprint, jsonify, make_response, redirect, request, session +from flask import Blueprint, jsonify, make_response, redirect, render_template, request, session from functions_appinsights import log_event from functions_authentication import login_required, user_required, user_required_blueprint @@ -23,15 +23,29 @@ sanitize_m365_approval, ) from functions_m365_connections import ( + CHAT_AUTH_SESSION_KEY, CHAT_AUTH_STATE_PREFIX, CHAT_CALLBACK_PATH, CHAT_RECONNECT_SESSION_KEY, + COMPLETION_MODES, CONNECTION_CALLBACK_PATH, get_m365_connection_service, + workflow_connection_readiness, ) from swagger_wrapper import swagger_route, get_auth_security +# Fixed classic destinations for full-page sign-ins. Popups report to their opener instead, +# which is how the V2 interface connects, so it never lands on these pages. +CLASSIC_CHAT_CONNECTION_URL = "/profile?tab=settings#m365-chat-connection" +CLASSIC_CHAT_CONNECTED_URL = "/profile?tab=settings&m365_chat_connection=connected#m365-chat-connection" +CLASSIC_WORKFLOW_CONNECTION_URL = "/profile?tab=settings#m365-connection-status" +CLASSIC_WORKFLOW_CONNECTED_URL = "/profile?tab=settings&m365_connection=connected#m365-connection-status" +CLASSIC_CHATS_URL = "/chats" +CONNECTION_RESULT_TEMPLATE = "m365_connection_result.html" +_CALLBACK_RESPONSE_FIELDS = {"state", "code", "error", "error_description", "error_uri", "session_state", "client_info"} + + _conversation_authorizer = None _decision_callback = None _audit_conversation_resolver = None @@ -93,7 +107,8 @@ def _page_options(): } -def _error_response(exc): +def _error_details(exc): + """The safe error payload and HTTP status for an M365 failure, shared by JSON and pages.""" if isinstance(exc, M365PolicyError): code = exc.code if code == "not_logged_in": @@ -107,6 +122,7 @@ def _error_response(exc): status = 409 elif code in { "m365_key_vault_required", "m365_key_unavailable", "m365_key_invalid", + "m365_key_provision_forbidden", "m365_key_deleted", "m365_configuration_invalid", "m365_tenant_authority_required", "m365_callback_invalid", "m365_workflow_validation_unavailable", "m365_audit_validation_unavailable", "m365_approval_validation_unavailable", @@ -118,11 +134,11 @@ def _error_response(exc): status = 404 else: status = 400 - return jsonify({"success": False, **exc.payload}), status + return dict(exc.payload), status if isinstance(exc, PermissionError): - return jsonify({"success": False, "error": "forbidden", "message": "You cannot perform this Microsoft 365 operation."}), 403 + return {"error": "forbidden", "message": "You cannot perform this Microsoft 365 operation."}, 403 if isinstance(exc, LookupError): - return jsonify({"success": False, "error": "not_found", "message": "Microsoft 365 request not found."}), 404 + return {"error": "not_found", "message": "Microsoft 365 request not found."}, 404 if isinstance(exc, ValueError): log_event( "[AUTH] Microsoft 365 request validation failed", @@ -130,16 +146,21 @@ def _error_response(exc): level=logging.WARNING, exceptionTraceback=True, ) - return jsonify({"success": False, "error": "invalid_request", "message": "Invalid Microsoft 365 request."}), 400 + return {"error": "invalid_request", "message": "Invalid Microsoft 365 request."}, 400 log_event( "[AUTH] Microsoft 365 request dependency unavailable", extra={"exception_type": type(exc).__name__, "endpoint": request.endpoint}, level=logging.ERROR, ) - return jsonify({ - "success": False, "error": "m365_service_unavailable", + return { + "error": "m365_service_unavailable", "message": "Microsoft 365 request storage or authentication is temporarily unavailable.", - }), 503 + }, 503 + + +def _error_response(exc): + payload, status = _error_details(exc) + return jsonify({"success": False, **payload}), status @user_required @@ -192,13 +213,80 @@ def _callback_uri(callback_path=CONNECTION_CALLBACK_PATH): return f"{origin}{callback_path}" +def _completion_mode(data): + """The optional completion mode a connect request asks for; full-page by default.""" + completion = data.get("completion", "page") + if completion not in COMPLETION_MODES: + raise ValueError("Unsupported sign-in completion mode.") + return completion + + +def _callback_auth_response(): + auth_response = request.args.to_dict() + if ( + set(auth_response) - _CALLBACK_RESPONSE_FIELDS + or any(len(value) > 16384 for value in auth_response.values()) + ): + raise ValueError("Invalid authorization response.") + return auth_response + + +def _connection_result_page(*, kind, outcome, completion, continue_url, code=None, message="", status=200): + """A small page that ends a Microsoft 365 sign-in instead of raw JSON. + + In a popup its script tells the window that opened it (same origin only) and closes; + on a full page it shows the outcome with a link back. Every value is server-chosen or a + safe error message, and continue_url is always one of this module's fixed paths. + """ + if outcome == "connected": + message_type = "m365-profile-reconnected" if kind == "chat" else "m365-workflow-connected" + title = "Microsoft 365 is connected" + message = message or ( + "Sign-in completed. You can return to SimpleChat." if kind == "chat" + else "Workflows can now run as your Microsoft 365 account once you authorize them." + ) + else: + message_type = "m365-connect-failed" + title = "Microsoft 365 was not connected" + result = { + "type": message_type, "kind": kind, "outcome": outcome, "completion": completion, + "code": code, "message": message, "continue_url": continue_url, + } + response = make_response(render_template( + CONNECTION_RESULT_TEMPLATE, result=result, title=title, message=message, + continue_url=continue_url, + continue_label="Return to settings" if continue_url != CLASSIC_CHATS_URL else "Return to chat", + ), status) + response.headers["Cache-Control"] = "private, no-store" + response.headers["Pragma"] = "no-cache" + return response + + +def _pending_chat_flow(state): + """Completion mode and purpose of the chat sign-in this callback answers, read before it is consumed.""" + record = session.get(CHAT_AUTH_SESSION_KEY) + flow = record.get("flow") if isinstance(record, dict) else None + expected = flow.get("state") if isinstance(flow, dict) else None + if ( + not isinstance(expected, str) or not isinstance(state, str) + or not hmac.compare_digest(expected.encode("utf-8"), state.encode("utf-8")) + ): + return "auto", None + completion = record.get("completion") + return (completion if completion in COMPLETION_MODES else "page"), record.get("purpose") + + def _complete_chat_connection(user_id, tenant_id, auth_response): # These owners are initialized before an authenticated OAuth callback. from functions_m365_request_resume import get_m365_chat_request from functions_m365_runtime import _conversation_access completed = get_m365_connection_service().complete_chat_connection(user_id, tenant_id, auth_response) if completed.get("return_to") == "profile": - return redirect("/profile?tab=settings&m365_chat_connection=connected#m365-chat-connection") + if completed.get("completion") == "popup": + return _connection_result_page( + kind="chat", outcome="connected", completion="popup", continue_url=CLASSIC_CHAT_CONNECTED_URL, + ) + return redirect(CLASSIC_CHAT_CONNECTED_URL) job = get_m365_chat_request(completed["request_id"], user_id) if job.get("conversation_id") != completed["conversation_id"]: raise M365PolicyError("m365_request_changed", "The conversation request changed during sign-in.") @@ -219,21 +307,60 @@ def _publish_verified_workflow_cache_to_session(serialized): session.pop(CHAT_RECONNECT_SESSION_KEY, None) +def _complete_workflow_connection(user_id, tenant_id, auth_response, completion): + session_binding = session.pop("m365_workflow_oauth_binding", None) + if not session_binding: + raise M365PolicyError( + "m365_auth_state_invalid", + "This workflow sign-in request expired or was replaced by a newer one. Start Connect for workflows again.", + ) + get_m365_connection_service().complete_connection( + user_id, tenant_id, auth_response, session_binding, + cache_writer=_publish_verified_workflow_cache_to_session, + ) + if completion == "popup": + return _connection_result_page( + kind="workflow", outcome="connected", completion="popup", continue_url=CLASSIC_WORKFLOW_CONNECTED_URL, + ) + return redirect(CLASSIC_WORKFLOW_CONNECTED_URL) + + @login_required @user_required def complete_m365_chat_connection_callback(): """Use the registered login callback without replacing the SimpleChat principal.""" + completion, purpose = _pending_chat_flow(request.args.get("state")) + try: + user_id, tenant_id = _subject() + result = _complete_chat_connection(user_id, tenant_id, _callback_auth_response()) + except (M365PolicyError, PermissionError, LookupError, ValueError, AzureError, requests.RequestException) as exc: + payload, status = _error_details(exc) + result = _connection_result_page( + kind="chat", outcome="failed", completion=completion, status=status, + code=payload["error"], message=payload["message"], + continue_url=CLASSIC_CHATS_URL if purpose == "chat_request" else CLASSIC_CHAT_CONNECTION_URL, + ) + response = make_response(result) + response.headers["Cache-Control"] = "private, no-store" + response.headers["Pragma"] = "no-cache" + return response + + +@login_required +@user_required +def complete_m365_workflow_connection_callback(): + """Finish a workflow sign-in that returned to the registered /getAToken callback.""" + completion = "auto" try: user_id, tenant_id = _subject() - auth_response = request.args.to_dict() - if ( - set(auth_response) - {"state", "code", "error", "error_description", "error_uri", "session_state", "client_info"} - or any(len(value) > 16384 for value in auth_response.values()) - ): - raise ValueError("Invalid authorization response.") - result = _complete_chat_connection(user_id, tenant_id, auth_response) + completion = get_m365_connection_service().connection_flow_completion(user_id, request.args.get("state")) + result = _complete_workflow_connection(user_id, tenant_id, _callback_auth_response(), completion) except (M365PolicyError, PermissionError, LookupError, ValueError, AzureError, requests.RequestException) as exc: - result = _error_response(exc) + payload, status = _error_details(exc) + result = _connection_result_page( + kind="workflow", outcome="failed", completion=completion, status=status, + code=payload["error"], message=payload["message"], continue_url=CLASSIC_WORKFLOW_CONNECTION_URL, + ) response = make_response(result) response.headers["Cache-Control"] = "private, no-store" response.headers["Pragma"] = "no-cache" @@ -328,10 +455,11 @@ def reconnect_m365_chat_connection(): user_id, tenant_id = _subject() validate_m365_csrf() data = _body() - if set(data) != {"sources"}: + if "sources" not in data or set(data) - {"sources", "completion"}: raise ValueError("Select only the Microsoft 365 sources to reconnect.") result = get_m365_connection_service().start_profile_chat_connection( user_id, tenant_id, data["sources"], _callback_uri(CHAT_CALLBACK_PATH), + completion=_completion_mode(data), ) return jsonify({"success": True, **result}) @@ -431,10 +559,13 @@ def list_m365_conversation_audit(conversation_id): @login_required @user_required def read_m365_profile_connection(): + # Settings are initialized before an authenticated profile request. + from functions_settings import get_settings user_id, tenant_id = _subject() return jsonify({ "success": True, "connection": get_m365_connection_service().current_connection(user_id, tenant_id), + "workflow_connections": workflow_connection_readiness(get_settings()), "csrf_token": get_m365_csrf_token(), }) @@ -446,13 +577,16 @@ def connect_m365_profile_account(): user_id, tenant_id = _subject() validate_m365_csrf() data = _body() - if set(data) - {"sources", "scopes"}: + if set(data) - {"sources", "scopes", "completion"}: raise ValueError("Invalid connection fields.") + completion = _completion_mode(data) session_binding = secrets.token_urlsafe(32) session["m365_workflow_oauth_binding"] = session_binding + # The registered sign-in callback; its state prefix routes the result here. result = get_m365_connection_service().start_connection( user_id, tenant_id, data.get("sources"), - _callback_uri(), session_binding, scopes=data.get("scopes"), + _callback_uri(CHAT_CALLBACK_PATH), session_binding, scopes=data.get("scopes"), + completion=completion, ) return jsonify({"success": True, **result}) @@ -461,24 +595,10 @@ def connect_m365_profile_account(): @login_required @user_required def complete_m365_profile_connection(): - user_id, tenant_id = _subject() - auth_response = request.args.to_dict() - if ( - set(auth_response) - {"state", "code", "error", "error_description", "error_uri", "session_state", "client_info"} - or any(len(value) > 16384 for value in auth_response.values()) - ): - raise ValueError("Invalid authorization response.") - service = get_m365_connection_service() - if (auth_response.get("state") or "").startswith(CHAT_AUTH_STATE_PREFIX): - return _complete_chat_connection(user_id, tenant_id, auth_response) - session_binding = session.pop("m365_workflow_oauth_binding", None) - if not session_binding: - raise M365PolicyError("m365_auth_state_invalid", "Start Connect again from Profile.") - service.complete_connection( - user_id, tenant_id, auth_response, session_binding, - cache_writer=_publish_verified_workflow_cache_to_session, - ) - return redirect("/profile?m365_connection=connected") + # Kept for deployments that registered this callback; new sign-ins use /getAToken. + if (request.args.get("state") or "").startswith(CHAT_AUTH_STATE_PREFIX): + return complete_m365_chat_connection_callback() + return complete_m365_workflow_connection_callback() @bp.route("/api/m365/connections/disconnect", methods=["POST"]) @swagger_route(security=get_auth_security()) diff --git a/application/single_app/route_backend_v2.py b/application/single_app/route_backend_v2.py index edde09277..ec304d463 100644 --- a/application/single_app/route_backend_v2.py +++ b/application/single_app/route_backend_v2.py @@ -171,6 +171,7 @@ keyvault_model_endpoint_save_helper, ) from functions_keyvault_errors import KeyVaultSecretStorageError +from functions_m365_connections import ensure_m365_workflow_encryption_key from functions_safety_remediation import count_pending_safety_warnings from functions_source_review import ( get_source_review_runtime_capabilities, @@ -2115,6 +2116,21 @@ def v2_admin_patch_settings(): # a branding change has to refresh them. _refresh_branding_static_files() + # Saved Microsoft 365 workflow connections need the deployment's encryption key. + # Create it now rather than when someone first connects; a failure is reported + # beside the Key Vault settings only when this save changed them. + m365_workflow_key = ensure_m365_workflow_encryption_key(get_settings()) + if m365_workflow_key["status"] == "unavailable" and { + "enable_key_vault_secret_storage", "key_vault_name", "key_vault_identity", + } & normalized.keys(): + warnings = { + **warnings, + "key_vault_name": ( + "Microsoft 365 workflow connections are not ready. " + f"{m365_workflow_key['message']}" + ), + } + # The response reflects the draft back into the page's stored state, so a # resolved secret has to be re-redacted on the way out, which # _redact_admin_settings_for_v2 does below. diff --git a/application/single_app/route_frontend_admin_settings.py b/application/single_app/route_frontend_admin_settings.py index d12c889d8..4a82c3201 100644 --- a/application/single_app/route_frontend_admin_settings.py +++ b/application/single_app/route_frontend_admin_settings.py @@ -78,6 +78,7 @@ from functions_document_actions import normalize_document_action_capabilities from functions_model_capabilities import ModelTokenBudgetError, is_vision_capable_model from functions_m365_transport import M365ProviderError, normalize_m365_transport_settings +from functions_m365_connections import ensure_m365_workflow_encryption_key from functions_orchestration_registry import ( build_capability_client_projection, capabilities_for_contract, @@ -3296,6 +3297,22 @@ def is_valid_url(url): else: print("ERROR: Could not fetch settings after update to ensure logo/favicon files.") + # Saved Microsoft 365 workflow connections need the deployment's encryption key. + # Create it now rather than when someone first connects; failures are reported + # only when this save changed the Key Vault configuration. + m365_workflow_key = ensure_m365_workflow_encryption_key(updated_settings_for_file or new_settings) + key_vault_changed = any( + settings.get(key) != new_settings.get(key) + for key in ('enable_key_vault_secret_storage', 'key_vault_name', 'key_vault_identity') + ) + if m365_workflow_key['status'] == 'created': + flash(m365_workflow_key['message'], 'info') + elif m365_workflow_key['status'] == 'unavailable' and key_vault_changed: + flash( + f"Microsoft 365 workflow connections are not ready. {m365_workflow_key['message']}", + 'warning', + ) + if governance_toggle_changes: try: log_governance_change( diff --git a/application/single_app/route_frontend_authentication.py b/application/single_app/route_frontend_authentication.py index 76fba8f30..9cab91872 100644 --- a/application/single_app/route_frontend_authentication.py +++ b/application/single_app/route_frontend_authentication.py @@ -17,7 +17,7 @@ from functions_authentication import _build_msal_app, _load_cache, _save_cache, clear_requested_oauth_scopes, create_ci_bearer_session, get_graph_authority, get_graph_endpoint, get_requested_oauth_scopes from functions_debug import debug_print from functions_settings import get_settings, sanitize_settings_for_user -from functions_m365_connections import CHAT_AUTH_STATE_PREFIX +from functions_m365_connections import CHAT_AUTH_STATE_PREFIX, WORKFLOW_AUTH_STATE_PREFIX from swagger_wrapper import swagger_route, get_auth_security def build_front_door_urls(front_door_url): @@ -239,6 +239,10 @@ def authorized(): # The M365 flow owns state/nonce validation and preserves the current app session. from route_backend_m365 import complete_m365_chat_connection_callback return complete_m365_chat_connection_callback() + if (request.args.get('state') or '').startswith(WORKFLOW_AUTH_STATE_PREFIX): + # Saved workflow sign-in reuses this registered callback; its flow is validated server-side. + from route_backend_m365 import complete_m365_workflow_connection_callback + return complete_m365_workflow_connection_callback() # Check for errors passed back from Azure AD if request.args.get('error'): error = request.args.get('error') diff --git a/application/single_app/static/js/chat/chat-m365-connect.js b/application/single_app/static/js/chat/chat-m365-connect.js index b7248e909..81766142a 100644 --- a/application/single_app/static/js/chat/chat-m365-connect.js +++ b/application/single_app/static/js/chat/chat-m365-connect.js @@ -213,9 +213,14 @@ } } function onMessage(event) { - if (event.origin === window.location.origin && event.source === popup - && event.data?.type === 'm365-profile-reconnected') { + if (event.origin !== window.location.origin || event.source !== popup) { + return; + } + if (event.data?.type === 'm365-profile-reconnected') { finish(); + } else if (event.data?.type === 'm365-connect-failed' && event.data?.kind === 'chat') { + const detail = typeof event.data.message === 'string' ? event.data.message.slice(0, 500) : ''; + finish(new Error(`${detail || 'Microsoft 365 sign-in did not complete.'} No action was sent.`)); } } function onAbort() { @@ -242,8 +247,9 @@ return; } // Profile reconnect saves credentials only; the saved-request connect/resume path would rerun the agent. + // A popup completion ends on a small result page that reports back here and closes. const result = await api.requestJson('/api/m365/chat/connection/connect', { - method: 'POST', body: { sources: selectedSources }, + method: 'POST', body: { sources: selectedSources, completion: 'popup' }, }); if (!finished) { popup.location.replace(authorizationUrl(result.authorization_url)); diff --git a/application/single_app/static/js/profile/profile-m365-connection-result.js b/application/single_app/static/js/profile/profile-m365-connection-result.js new file mode 100644 index 000000000..eefc6aae5 --- /dev/null +++ b/application/single_app/static/js/profile/profile-m365-connection-result.js @@ -0,0 +1,83 @@ +// profile-m365-connection-result.js +// Ends a Microsoft 365 sign-in on the page the callback renders. +// +// A sign-in started in a popup reports its outcome to the same-origin window that opened it, +// then closes. The opener (V2 settings, a V2 chat notice, or the classic chat) shows the result, +// so the popup never strands the user on another interface. A full-page sign-in shows the +// outcome here and, after success, continues to the fixed settings page the server chose. + +(() => { + 'use strict'; + + // Only a same-origin absolute path; never another origin or a script URL. + const SAFE_PATH_PATTERN = /^\/(?![/\\])[^\s\\]*$/; + + function readResult() { + const element = document.getElementById('m365-connection-result-data'); + if (!element) { + return null; + } + try { + const value = JSON.parse(element.textContent || 'null'); + return value && typeof value === 'object' ? value : null; + } catch { + return null; + } + } + + function hasOpener() { + try { + return Boolean(window.opener) && window.opener !== window && !window.opener.closed; + } catch { + return false; + } + } + + function openerMessage(result) { + const message = { + type: typeof result.type === 'string' ? result.type : 'm365-connect-failed', + kind: result.kind === 'workflow' ? 'workflow' : 'chat', + outcome: result.outcome === 'connected' ? 'connected' : 'failed', + }; + if (message.outcome !== 'connected') { + message.code = typeof result.code === 'string' ? result.code : ''; + message.message = typeof result.message === 'string' ? result.message : ''; + } + return message; + } + + function initialize() { + const result = readResult(); + if (!result) { + return; + } + const closeButton = document.getElementById('m365-connection-result-close'); + const closingHint = document.getElementById('m365-connection-result-closing'); + closeButton?.addEventListener('click', () => window.close()); + + const popup = result.completion === 'popup' || (result.completion === 'auto' && hasOpener()); + if (popup) { + closeButton?.classList.remove('d-none'); + if (hasOpener()) { + window.opener.postMessage(openerMessage(result), window.location.origin); + closingHint?.classList.remove('d-none'); + window.close(); + } + return; + } + + if ( + result.outcome === 'connected' + && typeof result.continue_url === 'string' + && SAFE_PATH_PATTERN.test(result.continue_url) + ) { + window.location.replace(result.continue_url); + } + } + + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', initialize, { once: true }); + } else { + initialize(); + } +})(); diff --git a/application/single_app/static/js/profile/profile-m365.js b/application/single_app/static/js/profile/profile-m365.js index a42f12e20..58f14aea9 100644 --- a/application/single_app/static/js/profile/profile-m365.js +++ b/application/single_app/static/js/profile/profile-m365.js @@ -85,10 +85,12 @@ function showChatConnectionReturn() { const currentUrl = new URL(window.location.href); const result = currentUrl.searchParams.get('m365_chat_connection'); - if (result === null) { + const workflowResult = currentUrl.searchParams.get('m365_connection'); + if (result === null && workflowResult === null) { return; } currentUrl.searchParams.delete('m365_chat_connection'); + currentUrl.searchParams.delete('m365_connection'); window.history.replaceState(window.history.state, '', `${currentUrl.pathname}${currentUrl.search}${currentUrl.hash}`); if (result === 'connected') { window.opener?.postMessage({ type: 'm365-profile-reconnected' }, window.location.origin); @@ -96,6 +98,23 @@ 'Microsoft 365 sign-in completed. Return to your conversation and retry your original question. No past requests were retried.', 'success'); } + if (workflowResult === 'connected') { + showStatus('m365-connection-notice', + 'Microsoft 365 is connected for workflows. Each Run as workflow revision still needs your authorization.', + 'success'); + } + } + + function describeChatConnection(status, labels) { + if (status === 'available') { + return labels + ? `Signed in to Microsoft 365 for this session. Chat can use: ${labels}.` + : 'Signed in to Microsoft 365 for this session, but no source\'s permissions are granted yet. Connect the sources chat should use.'; + } + if (status === 'reconnect_required') { + return `Microsoft 365 needs you to sign in again before chat can use ${labels || 'your sources'}.`; + } + return 'Not signed in to Microsoft 365 in this session. Chat asks you to connect the first time it needs a source, or connect here.'; } async function loadChatConnection() { @@ -107,26 +126,25 @@ try { const response = await api.requestJson('/api/m365/chat/connection'); const chatConnection = response.connection; - const descriptions = { - available: 'Sign-in saved for this session', - not_connected: 'No Microsoft 365 sign-in is saved for this session', - reconnect_required: 'Reconnect Microsoft 365 before using these sources in chat' - }; - if (!Object.prototype.hasOwnProperty.call(descriptions, chatConnection?.status) + const statuses = ['available', 'not_connected', 'reconnect_required']; + if (!statuses.includes(chatConnection?.status) || !Array.isArray(chatConnection.sources) || !chatConnection.sources.every(source => typeof source === 'string' && Object.prototype.hasOwnProperty.call(api.sourceLabels, source))) { throw new Error('The chat sign-in status could not be verified. Refresh before trying again.'); } + const selected = chatConnection.sources; root.querySelectorAll('[data-m365-chat-connect-source]').forEach(checkbox => { - checkbox.checked = chatConnection.sources.includes(checkbox.dataset.m365ChatConnectSource); + checkbox.checked = selected.includes(checkbox.dataset.m365ChatConnectSource); }); - document.getElementById('m365-chat-connection-details').textContent = - `Sources saved for this session: ${chatConnection.sources.map(source => api.sourceLabels[source]).join(', ') || 'None'}.`; + const labels = chatConnection.sources.map(source => api.sourceLabels[source]).join(', '); + document.getElementById('m365-chat-connection-details').textContent = chatConnection.status === 'available' + ? 'Reconnect to renew this sign-in or to add sources.' + : ''; fields.disabled = false; - showStatus('m365-chat-connection-status', - `${descriptions[chatConnection.status]}. Access is checked when a source runs.`, - chatConnection.status === 'reconnect_required' ? 'warning' : 'info'); + showStatus('m365-chat-connection-status', describeChatConnection(chatConnection.status, labels), + chatConnection.status === 'reconnect_required' ? 'warning' + : chatConnection.status === 'available' ? 'success' : 'info'); } catch (error) { showStatus('m365-chat-connection-status', error.message, 'danger'); } @@ -202,8 +220,22 @@ root.querySelectorAll('[data-m365-connect-source]').forEach(checkbox => { checkbox.checked = (connection?.sources || []).includes(checkbox.dataset.m365ConnectSource); }); + const readiness = response.workflow_connections; + const blocked = readiness?.available === false; + const readinessElement = document.getElementById('m365-connection-readiness'); + if (blocked) { + showStatus('m365-connection-readiness', + typeof readiness.message === 'string' && readiness.message + ? readiness.message + : 'Workflow connections are not set up on this deployment yet. Ask an administrator.', + 'warning'); + } else { + readinessElement.classList.add('d-none'); + } fields.disabled = false; - document.getElementById('m365-connect-btn').textContent = connection?.id ? 'Reconnect Microsoft 365 for workflows' : 'Connect Microsoft 365 for workflows'; + const connectButton = document.getElementById('m365-connect-btn'); + connectButton.disabled = blocked; + connectButton.textContent = connection?.id && status !== 'disconnected' ? 'Reconnect Microsoft 365 for workflows' : 'Connect Microsoft 365 for workflows'; document.getElementById('m365-disconnect-btn').disabled = !connection?.id || status === 'disconnected'; showStatus('m365-connection-status', `Workflow connection: ${status.replaceAll('_', ' ')}. Connecting is separate from approving a workflow.`, status === 'connected' ? 'success' : 'info'); } catch (error) { diff --git a/application/single_app/templates/m365_connection_result.html b/application/single_app/templates/m365_connection_result.html new file mode 100644 index 000000000..ff9b3b996 --- /dev/null +++ b/application/single_app/templates/m365_connection_result.html @@ -0,0 +1,37 @@ + + + + + + + + {{ title }} + + + + +
+
+
+
+
+

{{ title }}

+

{{ message }}

+

+ SimpleChat has the result. You can close this window if it stays open. +

+
+ {{ continue_label }} + +
+
+
+
+
+
+ + + + + + diff --git a/application/single_app/templates/profile.html b/application/single_app/templates/profile.html index dab8f3e7e..d8dea5abc 100644 --- a/application/single_app/templates/profile.html +++ b/application/single_app/templates/profile.html @@ -1270,7 +1270,7 @@
Microsoft 365 chat connection

Reconnecting does not change sharing approvals or preferences, saved workflow credentials, or workflow Run as authorizations. It does not enable disabled action capabilities or fix unrelated model or storage configuration errors.

Loading chat sign-in status...
-

This status reflects local saved sign-in state, not a live test of Microsoft 365 access.

+

Sources listed are those this session's sign-in already has permission for, including consent given at sign-in or by an administrator. Microsoft still checks your access each time a source runs.

Sources to connect for chat @@ -1290,6 +1290,8 @@
Microsoft 365 chat connection
Connect Microsoft 365 for workflows

This is separate from ordinary sign-in, sharing preferences, and workflow approval. Each selected Run as workflow revision needs your explicit authorization. Disconnect stops future connected-account use and invalidates bindings; it does not sign you out of other applications.

This status is for saved workflow access. Interactive chat asks you to connect in the conversation when permission is needed; a disconnected workflow account does not disable chat.

+
+
Loading workflow connection...
@@ -1308,7 +1310,7 @@
Connect Microsoft 365 for workflows
-

Durable workflow sign-in requires the deployment's Key Vault-backed credential protection. If it is unavailable, connection fails explicitly; no plaintext or application-identity fallback is used.

+

A saved workflow sign-in is encrypted with this deployment's key in Key Vault, which SimpleChat creates the first time anyone connects. There is no plaintext or application-identity fallback.

Your workflow authorizations
diff --git a/application/v2_ui/src/App.tsx b/application/v2_ui/src/App.tsx index 64dfa48e5..72ca68a56 100644 --- a/application/v2_ui/src/App.tsx +++ b/application/v2_ui/src/App.tsx @@ -47,6 +47,9 @@ const AlertLabPage = import.meta.env.DEV ? lazy(() => import('./dev/AlertLabPage').then((lab) => ({ default: lab.AlertLabPage }))) : null; +/** Focus and visibility changes refresh bootstrap at most this often. */ +const FOCUS_REFRESH_MIN_INTERVAL_MS = 15_000; + function BootScreen() { return (
@@ -105,7 +108,7 @@ function BootError({ message, authExpired }: { message: string; authExpired: boo export function App() { const { data, loading, error, authExpired, load } = useBootstrapStore(); - const refreshBootstrap = useBootstrapStore((state) => state.refresh); + const refreshBootstrapWhenStale = useBootstrapStore((state) => state.refreshWhenStale); const loadUserSettings = useUserSettingsStore((state) => state.load); const fontSize = useUserSettingsStore( (state) => (state.settings.fontSizePreference as string) || 'm', @@ -159,7 +162,9 @@ export function App() { * Refreshing on re-focus closes that gap for every setting at once rather than for * whichever one is being complained about. `refresh` is advisory -- it leaves the page * alone if the request fails, and never shows the boot screen -- so the worst case of a - * spurious wake-up is one wasted request. + * spurious wake-up is one wasted request. Window switches arrive in bursts, though, and + * the payload takes seconds to build, so a refresh is skipped while one is running or + * one started in the last few seconds. */ useEffect(() => { if (standalonePage) { @@ -167,7 +172,7 @@ export function App() { } const onVisible = () => { if (document.visibilityState === 'visible') { - void refreshBootstrap(); + void refreshBootstrapWhenStale(FOCUS_REFRESH_MIN_INTERVAL_MS); } }; document.addEventListener('visibilitychange', onVisible); @@ -179,7 +184,7 @@ export function App() { document.removeEventListener('visibilitychange', onVisible); window.removeEventListener('focus', onVisible); }; - }, [refreshBootstrap, standalonePage]); + }, [refreshBootstrapWhenStale, standalonePage]); useEffect(() => { const title = data?.branding?.app_title; diff --git a/application/v2_ui/src/components/approvals/PausedRequestsPanel.tsx b/application/v2_ui/src/components/approvals/PausedRequestsPanel.tsx index 2dbfe6fe9..1b09aa0d6 100644 --- a/application/v2_ui/src/components/approvals/PausedRequestsPanel.tsx +++ b/application/v2_ui/src/components/approvals/PausedRequestsPanel.tsx @@ -3,7 +3,7 @@ // // Resuming only queues the saved request again; its answer appears in the original // conversation. Requests that need recovery cannot be resumed from here, and workflow -// requests are fixed by reconnecting the workflow's account in Profile. +// requests are fixed by reconnecting the workflow's account in Settings. import { useCallback, useEffect, useMemo, useState } from 'react'; import { Link, useNavigate } from 'react-router-dom'; @@ -17,7 +17,8 @@ import { type PausedRequest, } from '../../lib/approvalsApi'; import { chatHrefForConversation } from '../../lib/conversationUrl'; -import { M365_PROFILE_CONNECTION_HREF, connectMicrosoft365, m365Sources } from '../../lib/m365Connect'; +import { connectMicrosoft365, m365Sources } from '../../lib/m365Connect'; +import { M365_CONNECT_HREF } from '../../lib/m365Links'; import { GlassButton, Skeleton } from '../ui/primitives'; import { ApprovalSplit, @@ -230,12 +231,12 @@ function PausedRequestDetail({ item }: { item: PausedRequest }) { ) : null}
{item.workflow_id ? ( - Review Microsoft 365 connection - + ) : authSources && !queued ? ( void connectAndResume()} data-testid="v2-paused-request-connect"> {busy ? : } diff --git a/application/v2_ui/src/components/approvals/PendingActionsPanel.tsx b/application/v2_ui/src/components/approvals/PendingActionsPanel.tsx index abf522833..066e473c9 100644 --- a/application/v2_ui/src/components/approvals/PendingActionsPanel.tsx +++ b/application/v2_ui/src/components/approvals/PendingActionsPanel.tsx @@ -27,7 +27,8 @@ import { type PendingAction, } from '../../lib/approvalsApi'; import { chatHrefForConversation } from '../../lib/conversationUrl'; -import { M365_PROFILE_CONNECTION_HREF, connectMicrosoft365, m365Sources } from '../../lib/m365Connect'; +import { connectMicrosoft365, m365Sources } from '../../lib/m365Connect'; +import { M365_CHAT_CONNECTION_HREF, M365_CONNECT_HREF } from '../../lib/m365Links'; import { GlassButton, Skeleton } from '../ui/primitives'; import { ApprovalSplit, @@ -529,14 +530,14 @@ function PendingActionDetail({ Reconnect Microsoft 365 ) : null} - - {action.workflow_id ? 'Reconnect workflow account in Profile' : 'Open Profile connection settings'} - + {action.workflow_id ? 'Reconnect workflow account in Settings' : 'Open Microsoft 365 settings'} + ) : null} void refresh({ clearNotice: true })}> diff --git a/application/v2_ui/src/components/chat/OrchestrationM365Notice.tsx b/application/v2_ui/src/components/chat/OrchestrationM365Notice.tsx index 0feb51809..0a96a79ac 100644 --- a/application/v2_ui/src/components/chat/OrchestrationM365Notice.tsx +++ b/application/v2_ui/src/components/chat/OrchestrationM365Notice.tsx @@ -1,12 +1,9 @@ // OrchestrationM365Notice.tsx import { useState } from 'react'; +import { Link } from 'react-router-dom'; import { GlassButton } from '../ui/primitives'; -import { - M365_APPROVALS_HREF, - M365_PROFILE_CONNECTION_HREF, - connectMicrosoft365, - m365Sources, -} from '../../lib/m365Connect'; +import { connectMicrosoft365, m365Sources } from '../../lib/m365Connect'; +import { M365_APPROVALS_HREF, M365_CHAT_CONNECTION_HREF } from '../../lib/m365Links'; import type { OrchestrationFailure } from '../../lib/orchestration'; const SOURCE_LABELS: Record = { @@ -57,18 +54,18 @@ export function OrchestrationM365Notice({ failures }: { failures: OrchestrationF ) : null} {signIn && !sources.length ? (

- - Connect Microsoft 365 in Profile settings - + + Connect Microsoft 365 in Settings + , then select Retry from failed step.

) : null} {error ?

{error}

: null} {approval ? (

- + Review the Microsoft 365 approval - + , then select Retry from failed step.

) : null} diff --git a/application/v2_ui/src/components/chat/WorkflowProposalCard.tsx b/application/v2_ui/src/components/chat/WorkflowProposalCard.tsx index 76c3d23bc..47dfa8b17 100644 --- a/application/v2_ui/src/components/chat/WorkflowProposalCard.tsx +++ b/application/v2_ui/src/components/chat/WorkflowProposalCard.tsx @@ -15,7 +15,7 @@ import { GlassButton } from '../ui/primitives'; import { WorkflowEditorDialog } from '../workflows/WorkflowEditorDialog'; import { WorkflowProposalRunSummary } from './WorkflowProposalRunSummary'; import { ApiError } from '../../lib/apiClient'; -import { M365_CONNECT_HREF } from '../../lib/m365Links'; +import { M365_APPROVALS_HREF, M365_CONNECT_HREF } from '../../lib/m365Links'; import { fetchWorkflowEditorOptions, normalizeWorkflowDefinition, @@ -43,8 +43,6 @@ const PERSONAL_SCOPE: WorkflowScope = { type: 'personal' }; const CREATING_POLL_MS = 3000; const CREATING_POLL_LIMIT = 40; const CREATING_WINDOW_MS = 120_000; -// Classic pages: V2 has no Microsoft 365 connection or approval page of its own. -const M365_APPROVALS_HREF = '/approvals'; const URL_ACCESS_REFUSED = 'URL Access is not available for workflows created from chat.'; const STATE_LABELS: Record = { @@ -209,8 +207,8 @@ function Microsoft365({ proposal }: { proposal: WorkflowProposal }) { {runAsSelf && m365.required ? ( m365.approval_state === 'approved' ? ' Run as is approved.' : m365.approval_state === 'waiting' ? ( - <> A run is waiting for your approval. Review Run as approval + <> A run is waiting for your approval. Review Run as approval ) : m365.approval_state === 'self_authored' ? ( `${created ? ' You saved it' : ' You create it'}, so it needs no separate Run as approval. ` @@ -223,9 +221,9 @@ function Microsoft365({ proposal }: { proposal: WorkflowProposal }) { {m365.required && m365.connected === false ? (

Microsoft 365 is not connected for workflows.{' '} - + Connect Microsoft 365 for workflows - +

) : null} diff --git a/application/v2_ui/src/components/chat/WorkflowRunCard.tsx b/application/v2_ui/src/components/chat/WorkflowRunCard.tsx index 0b07e82a5..338d69de5 100644 --- a/application/v2_ui/src/components/chat/WorkflowRunCard.tsx +++ b/application/v2_ui/src/components/chat/WorkflowRunCard.tsx @@ -198,9 +198,9 @@ export function LiveRunRow({ ); } else if (controls?.reconnect) { actions.push( - + Reconnect Microsoft 365 - , + , ); } } else if (row?.phase === 'finished') { diff --git a/application/v2_ui/src/components/settings/M365Cards.tsx b/application/v2_ui/src/components/settings/M365Cards.tsx index 90a63bfe4..dd9616cf0 100644 --- a/application/v2_ui/src/components/settings/M365Cards.tsx +++ b/application/v2_ui/src/components/settings/M365Cards.tsx @@ -11,10 +11,10 @@ // already published to a conversation, and the dialogs say so. import { useCallback, useEffect, useState, type ReactNode } from 'react'; +import { Link } from 'react-router-dom'; import { clsx } from 'clsx'; import { Cable, - ExternalLink, KeyRound, Loader2, MessageSquare, @@ -25,12 +25,12 @@ import { } from 'lucide-react'; import { ApiError, request } from '../../lib/apiClient'; import { - normalizeAuthorizationUrl, connectMicrosoft365, + connectMicrosoft365Workflow, m365Sources, - M365_APPROVALS_HREF, type M365Source, } from '../../lib/m365Connect'; +import { M365_APPROVALS_HREF } from '../../lib/m365Links'; import { toast } from '../../stores/toastStore'; import { ConfirmDialog } from '../ui/ConfirmDialog'; import { SettingsCard } from './SettingsCard'; @@ -84,11 +84,31 @@ const ANALYSIS_DECISION_LABELS: Record = { }; const CHAT_STATUS_TEXT: Record = { - available: 'Sign-in saved for this session', - not_connected: 'No Microsoft 365 sign-in is saved for this session', - reconnect_required: 'Reconnect Microsoft 365 before using these sources in chat', + available: 'Signed in to Microsoft 365 for this session', + not_connected: 'Not signed in to Microsoft 365 in this session', + reconnect_required: 'Microsoft 365 needs you to sign in again', }; +/** + * What the chat sign-in can do now. For a working sign-in the server lists the sources whose + * permissions it already holds, including consent given at sign-in or by an administrator, so + * this matches what chat actually does rather than only what was reconnected here. + */ +export function describeChatConnection(status: string, sources: M365Source[]): string { + const labels = sources.map((source) => M365_SOURCE_LABELS[source]).join(', '); + if (status === 'available') { + return labels + ? `${CHAT_STATUS_TEXT.available}. Chat can use: ${labels}. Microsoft still checks your access each time a source runs.` + : `${CHAT_STATUS_TEXT.available}, but no source's permissions are granted yet. Connect the sources chat should use.`; + } + if (status === 'reconnect_required') { + return labels + ? `${CHAT_STATUS_TEXT.reconnect_required} before chat can use ${labels}.` + : `${CHAT_STATUS_TEXT.reconnect_required} before chat can use your sources.`; + } + return `${CHAT_STATUS_TEXT.not_connected}. Chat asks you to connect the first time it needs a source, or connect here.`; +} + const SOURCE_PERMISSIONS_HELP = 'Each source includes the permissions for all of its supported operations. Calendar includes reading events, creating invitations, mailbox timezone and recipient lookup. Email includes reading messages, managing drafts and read state, sending mail and recipient lookup. OneDrive and SPO include file discovery and reading. Microsoft shows the permissions before you consent.'; @@ -483,7 +503,7 @@ function ChatConnectionCard({ refreshKey }: { refreshKey: number }) { setConnecting(true); try { await connectMicrosoft365(selected); - toast.success('Microsoft 365 sign-in completed. Retry your original question in the conversation.'); + toast.success('Microsoft 365 is connected for chat. Retry your original question in the conversation.'); setReload((value) => value + 1); } catch (error) { toast.error(errorText(error, 'Microsoft 365 sign-in could not start.')); @@ -497,7 +517,7 @@ function ChatConnectionCard({ refreshKey }: { refreshKey: number }) { title="Chat connection" sectionId="m365-chat-connection" Icon={MessageSquare} - description="Renew your interactive Microsoft 365 sign-in for chat, even when no request is waiting. It uses your current session and needs neither Key Vault nor a saved workflow connection. Reconnecting leaves sharing approvals, workflow credentials and workflow authorizations unchanged." + description="Your Microsoft 365 sign-in for chat in this browser session. Chat uses it to read the sources you allow, and asks you to connect when one needs more permission. It needs neither Key Vault nor a saved workflow connection, and reconnecting leaves sharing approvals, workflow credentials and workflow authorizations unchanged." >
{loadError ? ( @@ -506,9 +526,7 @@ function ChatConnectionCard({ refreshKey }: { refreshKey: number }) { Loading chat sign-in statusโ€ฆ ) : ( - {CHAT_STATUS_TEXT[connection.status]}. Sources saved for this session:{' '} - {connection.sources.map((source) => M365_SOURCE_LABELS[source]).join(', ') || 'none'}. This is - the saved sign-in state, not a live test; access is checked when a source runs. + {describeChatConnection(connection.status, connection.sources)} )} void; + onConnected: () => void; }) { const [connection, setConnection] = useState(undefined); + const [availability, setAvailability] = useState({ available: true, message: '' }); const [selected, setSelected] = useState([]); const [loadError, setLoadError] = useState(null); const [connecting, setConnecting] = useState(false); useEffect(() => { const controller = new AbortController(); - m365Request<{ connection?: WorkflowConnection | null }>('/api/m365/connections', { signal: controller.signal }) + m365Request<{ connection?: WorkflowConnection | null; workflow_connections?: unknown }>( + '/api/m365/connections', + { signal: controller.signal }, + ) .then((response) => { if (!Object.prototype.hasOwnProperty.call(response, 'connection')) { throw new Error('The workflow connection status could not be verified.'); } setConnection(response.connection ?? null); + setAvailability(readWorkflowAvailability(response.workflow_connections)); setSelected(m365Sources(response.connection?.sources)); setLoadError(null); }) @@ -580,14 +623,13 @@ function WorkflowConnectionCard({ } setConnecting(true); try { - const result = await m365Request<{ authorization_url?: unknown }>('/api/m365/connections/connect', { - method: 'POST', - body: { sources: selected }, - }); - // The sign-in returns to the classic Profile page, which confirms the saved connection. - window.location.assign(normalizeAuthorizationUrl(result.authorization_url)); + // A popup, so the result comes back here rather than to a classic page. + await connectMicrosoft365Workflow(selected); + toast.success('Microsoft 365 is connected for workflows. Each Run as workflow still needs your authorization.'); + onConnected(); } catch (error) { toast.error(errorText(error, 'The workflow connection could not start.')); + } finally { setConnecting(false); } }; @@ -603,6 +645,7 @@ function WorkflowConnectionCard({ ['Authorized sources', m365Sources(connection?.sources).map((source) => M365_SOURCE_LABELS[source]).join(', ') || 'None'], ['Delegated permissions', scopes.join(', ') || 'None'], ]; + const blocked = !availability.available; return ( Loading workflow connectionโ€ฆ ) : ( <> + {blocked ? {availability.message} : null} Workflow connection: {spaced(status)}. @@ -635,18 +679,18 @@ function WorkflowConnectionCard({ legend="Sources to connect for workflows" value={selected} onChange={setSelected} - disabled={connection === undefined || connecting} + disabled={connection === undefined || connecting || blocked} idPrefix="m365-workflow-source" />

- A saved workflow sign-in needs the deployment's Key Vault-backed credential protection. Without - it, connecting fails; there is no plaintext or application-identity fallback. + A saved workflow sign-in is encrypted with this deployment's key in Key Vault, which SimpleChat + creates the first time anyone connects. There is no plaintext or application-identity fallback.

@@ -740,10 +784,9 @@ function WorkflowAuthorizationsCard({ Icon={KeyRound} description="The workflow revisions you have allowed to run as your Microsoft 365 account. Only your own authorizations are listed; decide pending requests from Approvals." actions={( - + Open Approvals - + )} > {loadError ? ( @@ -846,7 +889,11 @@ export function M365Cards() {
- + setRefreshKey((value) => value + 1)} + /> {pending && ( diff --git a/application/v2_ui/src/lib/m365Connect.ts b/application/v2_ui/src/lib/m365Connect.ts index 9e9bc065f..25304c3fe 100644 --- a/application/v2_ui/src/lib/m365Connect.ts +++ b/application/v2_ui/src/lib/m365Connect.ts @@ -1,26 +1,27 @@ // m365Connect.ts -// Connects Microsoft 365 for orchestration steps that stopped for sign-in. +// Signs in to Microsoft 365 from V2 in a popup, for chat or for saved workflows. // -// The flow is the classic Profile reconnect: the server starts an authorization code flow -// for the named sources and stores the result in the signed-in session. The sign-in runs in -// a popup that ends on the Profile page, which tells its opener it connected. That message -// can be lost when the popup visits Microsoft sign-in or the API is on another origin, so -// the saved connection is checked when the popup closes. +// The server starts an authorization code flow for the named sources and asks for a popup +// completion. The sign-in ends on a small result page that tells this window the outcome and +// closes, so the user stays in V2 and never lands on a classic page. That message can be lost +// when the popup visits Microsoft sign-in or the API is on another origin, so the saved +// connection is checked when the popup closes. import { apiUrl, request } from './apiClient'; export type M365Source = 'calendar' | 'email' | 'onedrive' | 'spo'; const M365_SOURCES: ReadonlySet = new Set(['calendar', 'email', 'onedrive', 'spo']); -const POPUP_NAME = 'simplechat-m365-orchestration-connect'; +const CHAT_POPUP_NAME = 'simplechat-m365-orchestration-connect'; +const WORKFLOW_POPUP_NAME = 'simplechat-m365-workflow-connect'; const POPUP_FEATURES = 'popup,width=720,height=780,resizable=yes,scrollbars=yes'; const CONNECT_TIMEOUT_MS = 300_000; const POPUP_POLL_MS = 500; +const MAX_FAILURE_MESSAGE_LENGTH = 500; const INVALID_URL_MESSAGE = 'The server did not return a valid HTTPS Microsoft 365 sign-in URL.'; - -/** Classic pages that own Microsoft 365 connections and approvals, on the Flask origin. */ -export const M365_PROFILE_CONNECTION_HREF = apiUrl('/profile?tab=settings#m365-chat-connection'); -export const M365_APPROVALS_HREF = apiUrl('/approvals'); +const CHAT_CONNECTED_TYPE = 'm365-profile-reconnected'; +const WORKFLOW_CONNECTED_TYPE = 'm365-workflow-connected'; +const CONNECT_FAILED_TYPE = 'm365-connect-failed'; interface ChatConnection { status?: string; @@ -28,11 +29,22 @@ interface ChatConnection { connected_at?: string; } +interface WorkflowConnection { + status?: string; + sources?: unknown; + connected_at?: string; +} + interface ChatConnectionResponse { connection?: ChatConnection; csrf_token?: string; } +interface WorkflowConnectionResponse { + connection?: WorkflowConnection | null; + csrf_token?: string; +} + interface ConnectResponse { authorization_url?: unknown; } @@ -61,34 +73,50 @@ export function normalizeAuthorizationUrl(value: unknown): string { return target.href; } -function connectionCovers(connection: ChatConnection | undefined, sources: M365Source[], before?: string): boolean { +function covers( + connection: ChatConnection | WorkflowConnection | null | undefined, + status: string, + sources: M365Source[], + before?: string, +): boolean { const connected = m365Sources(connection?.sources); - return connection?.status === 'available' + return connection?.status === status && typeof connection.connected_at === 'string' && connection.connected_at !== before && sources.every((source) => connected.includes(source)); } +/** The result page's own wording for a failed sign-in, as plain text. */ +function failureMessage(data: { message?: unknown }): string { + const text = typeof data.message === 'string' ? data.message.trim() : ''; + return text ? text.slice(0, MAX_FAILURE_MESSAGE_LENGTH) : 'Microsoft 365 sign-in did not complete. Connect again when you are ready.'; +} + +interface PopupSignIn { + kind: 'chat' | 'workflow'; + popupName: string; + connectedType: string; + /** Starts the sign-in and returns the authorization URL; records any state `confirm` needs. */ + start: () => Promise; + /** After the popup closed without a message: whether the connection was saved anyway. */ + confirm: () => Promise; +} + /** - * Sign in to Microsoft 365 for these sources. Resolves once the sign-in is saved for this - * session; rejects with a user-facing message otherwise. Call it from a click handler: the - * popup opens before anything is awaited, so the browser does not block it. + * Runs one popup sign-in. The popup opens before anything is awaited, so call this from a + * click handler or the browser blocks it. */ -export function connectMicrosoft365(sources: M365Source[]): Promise { - if (!sources.length) { - return Promise.reject(new Error('Open Profile settings to choose the Microsoft 365 sources to connect.')); - } - const popup = window.open('about:blank', POPUP_NAME, POPUP_FEATURES); +function runPopupSignIn(signIn: PopupSignIn): Promise { + const popup = window.open('about:blank', signIn.popupName, POPUP_FEATURES); if (!popup) { return Promise.reject(new Error( - 'The sign-in window was blocked. Allow pop-ups for this site, or connect Microsoft 365 in Profile settings.', + 'The sign-in window was blocked. Allow pop-ups for this site, then connect Microsoft 365 again.', )); } const apiOrigin = new URL(apiUrl('/'), window.location.origin).origin; return new Promise((resolve, reject) => { let finished = false; let checking = false; - let connectedBefore: string | undefined; const finish = (error?: Error) => { if (finished) return; finished = true; @@ -101,22 +129,26 @@ export function connectMicrosoft365(sources: M365Source[]): Promise { }; const onMessage = (event: MessageEvent) => { if ( - event.source === popup - && (event.origin === window.location.origin || event.origin === apiOrigin) - && (event.data as { type?: unknown } | null)?.type === 'm365-profile-reconnected' + event.source !== popup + || (event.origin !== window.location.origin && event.origin !== apiOrigin) ) { + return; + } + const data = (event.data ?? {}) as { type?: unknown; kind?: unknown; message?: unknown }; + if (data.type === signIn.connectedType) { finish(); + } else if (data.type === CONNECT_FAILED_TYPE && data.kind === signIn.kind) { + finish(new Error(failureMessage(data))); } }; const confirmAfterClose = async () => { if (checking || finished) return; checking = true; try { - const latest = await request('/api/m365/chat/connection'); - finish(connectionCovers(latest.connection, sources, connectedBefore) ? undefined + finish(await signIn.confirm() ? undefined : new Error('Microsoft 365 sign-in was not completed. Connect again when you are ready.')); } catch { - finish(new Error('Microsoft 365 sign-in could not be confirmed. Check Profile settings, then retry.')); + finish(new Error('Microsoft 365 sign-in could not be confirmed. Refresh Microsoft 365 status, then retry.')); } }; const poll = window.setInterval(() => { @@ -128,19 +160,9 @@ export function connectMicrosoft365(sources: M365Source[]): Promise { window.addEventListener('message', onMessage); void (async () => { try { - const current = await request('/api/m365/chat/connection'); - connectedBefore = current.connection?.connected_at; - const csrfToken = current.csrf_token; - if (typeof csrfToken !== 'string' || csrfToken.length < 32) { - throw new Error('Microsoft 365 controls are unavailable. Refresh the page and try again.'); - } - const started = await request('/api/m365/chat/connection/connect', { - method: 'POST', - body: { sources }, - headers: { 'X-M365-CSRF-Token': csrfToken }, - }); + const authorizationUrl = await signIn.start(); if (!finished) { - popup.location.replace(normalizeAuthorizationUrl(started.authorization_url)); + popup.location.replace(normalizeAuthorizationUrl(authorizationUrl)); popup.focus(); } } catch (error) { @@ -149,3 +171,70 @@ export function connectMicrosoft365(sources: M365Source[]): Promise { })(); }); } + +function requireCsrf(token: unknown): string { + if (typeof token !== 'string' || token.length < 32) { + throw new Error('Microsoft 365 controls are unavailable. Refresh the page and try again.'); + } + return token; +} + +/** + * Sign in to Microsoft 365 for chat with these sources. Resolves once the sign-in is saved for + * this session; rejects with a user-facing message otherwise. + */ +export function connectMicrosoft365(sources: M365Source[]): Promise { + if (!sources.length) { + return Promise.reject(new Error('Choose the Microsoft 365 sources to connect in Settings.')); + } + let connectedBefore: string | undefined; + return runPopupSignIn({ + kind: 'chat', + popupName: CHAT_POPUP_NAME, + connectedType: CHAT_CONNECTED_TYPE, + start: async () => { + const current = await request('/api/m365/chat/connection'); + connectedBefore = current.connection?.connected_at; + const started = await request('/api/m365/chat/connection/connect', { + method: 'POST', + body: { sources, completion: 'popup' }, + headers: { 'X-M365-CSRF-Token': requireCsrf(current.csrf_token) }, + }); + return started.authorization_url; + }, + confirm: async () => { + const latest = await request('/api/m365/chat/connection'); + return covers(latest.connection, 'available', sources, connectedBefore); + }, + }); +} + +/** + * Save a Microsoft 365 sign-in that workflows can run as, for these sources. Resolves once the + * workflow connection is saved; rejects with a user-facing message otherwise. + */ +export function connectMicrosoft365Workflow(sources: M365Source[]): Promise { + if (!sources.length) { + return Promise.reject(new Error('Select at least one source to connect for workflows.')); + } + let connectedBefore: string | undefined; + return runPopupSignIn({ + kind: 'workflow', + popupName: WORKFLOW_POPUP_NAME, + connectedType: WORKFLOW_CONNECTED_TYPE, + start: async () => { + const current = await request('/api/m365/connections'); + connectedBefore = current.connection?.connected_at; + const started = await request('/api/m365/connections/connect', { + method: 'POST', + body: { sources, completion: 'popup' }, + headers: { 'X-M365-CSRF-Token': requireCsrf(current.csrf_token) }, + }); + return started.authorization_url; + }, + confirm: async () => { + const latest = await request('/api/m365/connections'); + return covers(latest.connection, 'connected', sources, connectedBefore); + }, + }); +} \ No newline at end of file diff --git a/application/v2_ui/src/lib/m365Links.ts b/application/v2_ui/src/lib/m365Links.ts index 1cd9df9be..359662b04 100644 --- a/application/v2_ui/src/lib/m365Links.ts +++ b/application/v2_ui/src/lib/m365Links.ts @@ -1,7 +1,11 @@ // m365Links.ts -// Where V2 sends someone to connect or reconnect Microsoft 365. +// Where V2 sends someone to connect, reconnect or review Microsoft 365. // -// A classic page: V2 has no Microsoft 365 connection page of its own. The workflow proposal card -// and the workflow run card both link here. +// V2 Settings holds the Microsoft 365 cards and V2 Approvals the Microsoft 365 requests, so each +// link stays in this interface rather than opening the classic Profile or Approvals page. They +// are router paths under the /v2 basename: render them with Link, never a bare anchor, which +// would leave the app. `section` names the settings card to scroll to. -export const M365_CONNECT_HREF = '/profile?tab=settings#m365-connection-status'; +export const M365_CHAT_CONNECTION_HREF = '/settings?tab=preferences§ion=m365-chat-connection'; +export const M365_CONNECT_HREF = '/settings?tab=preferences§ion=m365-workflow-connection'; +export const M365_APPROVALS_HREF = '/approvals/m365'; \ No newline at end of file diff --git a/application/v2_ui/src/pages/SettingsPage.tsx b/application/v2_ui/src/pages/SettingsPage.tsx index b2be8537f..bfbcaaa61 100644 --- a/application/v2_ui/src/pages/SettingsPage.tsx +++ b/application/v2_ui/src/pages/SettingsPage.tsx @@ -98,6 +98,22 @@ export function SettingsPage() { document.getElementById(`${sectionId}-title`)?.focus({ preventScroll: true }); }, []); + // A link can name one card with `section`, such as the Microsoft 365 connection a chat + // notice points at. Jump to it once, when its card has registered on the open tab. + const requestedSection = searchParams.get('section'); + const jumpedSection = useRef(null); + useEffect(() => { + if (!requestedSection) { + jumpedSection.current = null; + return; + } + if (jumpedSection.current === requestedSection || !(requestedSection in sections)) { + return; + } + jumpedSection.current = requestedSection; + jumpToSection(requestedSection); + }, [requestedSection, sections, jumpToSection]); + if (!active) { return null; } diff --git a/application/v2_ui/src/stores/bootstrapStore.ts b/application/v2_ui/src/stores/bootstrapStore.ts index cfcb40c3f..6910ef296 100644 --- a/application/v2_ui/src/stores/bootstrapStore.ts +++ b/application/v2_ui/src/stores/bootstrapStore.ts @@ -17,6 +17,25 @@ import type { BootstrapPayload, PromptOption } from '../lib/types'; */ let refreshSequence = 0; +/** + * When a bootstrap read last started, and how many are in flight. + * + * Focus and visibility changes ask for a refresh, and switching windows -- to a Microsoft 365 + * sign-in popup and back, say -- fires them in bursts. The payload takes seconds to build, so + * `refreshWhenStale` skips a refresh while one is running or one started moments ago. + */ +let lastBootstrapStartedAt = 0; +let bootstrapReadsInFlight = 0; + +/** Note that a bootstrap read is starting; call the returned function once it settles. */ +function startBootstrapRead(): () => void { + lastBootstrapStartedAt = Date.now(); + bootstrapReadsInFlight += 1; + return () => { + bootstrapReadsInFlight -= 1; + }; +} + interface BootstrapState { data: BootstrapPayload | null; loading: boolean; @@ -33,6 +52,11 @@ interface BootstrapState { * those has to reload the browser before the change is visible without this. */ refresh: () => Promise; + /** + * `refresh`, unless a read is already running or one started within `minIntervalMs`. + * For focus and visibility changes, which arrive in bursts; a save should call `refresh`. + */ + refreshWhenStale: (minIntervalMs: number) => Promise; /** An identity-bound selection must not install a response from a different sign-in. */ refreshRequired: (expectedViewerId?: string) => Promise; /** @@ -55,6 +79,7 @@ export const useBootstrapStore = create((set, get) => ({ load: async () => { set({ loading: true, error: null, authExpired: false }); + const settle = startBootstrapRead(); try { const data = await fetchBootstrap(); set({ data, loading: false }); @@ -78,11 +103,14 @@ export const useBootstrapStore = create((set, get) => ({ ? error.message : 'Failed to load the application.', }); + } finally { + settle(); } }, refresh: async () => { const sequence = ++refreshSequence; + const settle = startBootstrapRead(); try { const data = await fetchBootstrap(); if (sequence === refreshSequence) { @@ -95,12 +123,27 @@ export const useBootstrapStore = create((set, get) => ({ // tear down the page being worked on -- unsaved edits included -- over a // refetch the reader never asked for. The caller's own write already // succeeded, so a briefly stale shell is cosmetic and the next load fixes it. + } finally { + settle(); + } + }, + + refreshWhenStale: async (minIntervalMs) => { + if (bootstrapReadsInFlight > 0 || Date.now() - lastBootstrapStartedAt < minIntervalMs) { + return; } + await get().refresh(); }, refreshRequired: async (expectedViewerId) => { const sequence = ++refreshSequence; - const data = await fetchBootstrap(); + const settle = startBootstrapRead(); + let data: BootstrapPayload; + try { + data = await fetchBootstrap(); + } finally { + settle(); + } if (sequence !== refreshSequence) { throw new Error('Application availability changed during refresh. Try again.'); } diff --git a/docs/_data/app_surface.yml b/docs/_data/app_surface.yml index 7fe3373bf..a3eda799d 100644 --- a/docs/_data/app_surface.yml +++ b/docs/_data/app_surface.yml @@ -8,7 +8,7 @@ counts: actions: 33 chat_controls: 47 sub_elements: 3 - app_pages: 30 + app_pages: 31 feature_surfaces: 25 capabilities: - key: enable_action_ai_assistant @@ -1234,6 +1234,8 @@ app_pages: - template: login.html slug: login route_url: /login +- template: m365_connection_result.html + slug: m365-connection-result - template: manage_group.html slug: manage-group route_url: /groups/ diff --git a/docs/admin/security.md b/docs/admin/security.md index af63ab06e..270f6906a 100644 --- a/docs/admin/security.md +++ b/docs/admin/security.md @@ -118,6 +118,8 @@ Assign **Key Vault Secrets Officer at the vault scope** to the application's sel **Permission test (0.261.125).** Before saving a vault change, run **Test Key Vault connection** in either interface. The test uses the draft vault and identity, lists secret properties, writes a uniquely named `simplechat-connection-test-*` secret containing a synthetic value, reads it back, and deletes it. Success requires every stage, including cleanup; it never changes existing secrets or purges deleted secrets. +**Microsoft 365 workflow key (0.261.302).** Saved Microsoft 365 workflow connections, the sign-ins that workflows run as, are encrypted with one key for the whole deployment, not one per user. SimpleChat keeps it in this vault as the secret `simplechat-m365-workflow-token-key` and creates it itself: the first time anyone connects for workflows, and on any settings save while Key Vault storage is on. There is nothing to generate by hand. Creating it needs the same Secrets Officer access as other secrets; if that is missing, the save reports it beside the Key Vault settings and users see that workflow connections are not ready. Do not delete the secret or its older versions: each saved connection can only be read with the version that encrypted it, so deleting it forces every user to reconnect. The optional `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME` app setting chooses a different secret name. + The temporary secret expires after ten minutes, but expiration is not deletion. If cleanup cannot be confirmed, the result identifies the generated secret for an administrator to remove. Soft-deleted test metadata remains under the vault's retention policy. Do not remove other application secrets when cleaning up a test. A denied-write result or save error names the missing permission rather than suggesting that a successful list operation proves write access. Check the identity, vault-scoped role or access policy, propagation of a recent role assignment, and network restrictions. A failed secret write does not fall back to storing the submitted credential as plaintext. See [Admin settings troubleshooting]({{ '/troubleshooting/#admin-settings-saves-and-connection-tests' | relative_url }}). diff --git a/docs/explanation/features/CHAT_ORCHESTRATION_WORKFLOWS_ROADMAP.md b/docs/explanation/features/CHAT_ORCHESTRATION_WORKFLOWS_ROADMAP.md index 041ec4e44..f120e89f8 100644 --- a/docs/explanation/features/CHAT_ORCHESTRATION_WORKFLOWS_ROADMAP.md +++ b/docs/explanation/features/CHAT_ORCHESTRATION_WORKFLOWS_ROADMAP.md @@ -1486,7 +1486,10 @@ tracker, running tag and posted-message footers. The last bullet of the **V2** l 3. **Vocabulary mismatch.** Workflows have no web search, deep research, image generation, or render_file except through agent actions. Phase 8 plan-replay addresses this. 4. **M365 setup.** - - Key Vault key `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME` and the `/api/m365/connections/callback` redirect + - Key Vault secret storage turned on. Since 0.261.302 the workflow encryption key creates itself in Key Vault + and workflow sign-in returns to the registered `/getAToken` callback, so neither the + `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME` setting nor a `/api/m365/connections/callback` redirect is required + ([connection session race fix](../fixes/M365_CONNECTION_SESSION_RACE_FIX.md)). - Profile โ†’ Connect Microsoft 365 with offline consent - Run as = self - Approval of the exact revision, unless the Run as user saved it. Since 0.261.229 (#1630), a revision the Run as diff --git a/docs/explanation/features/MICROSOFT_365_ACTIONS.md b/docs/explanation/features/MICROSOFT_365_ACTIONS.md index 826dea024..abe48f5da 100644 --- a/docs/explanation/features/MICROSOFT_365_ACTIONS.md +++ b/docs/explanation/features/MICROSOFT_365_ACTIONS.md @@ -32,8 +32,16 @@ Mail and calendar history search, and the agent stream context and interrupted-reply persistence repair, were added in **0.261.129**. See the [stream context and persistence fix](../fixes/M365_AGENT_STREAM_CONTEXT_PERSISTENCE_FIX.md). +In **0.261.302** the workflow encryption key creates itself in Key Vault, +workflow sign-in returns to the registered `/getAToken` callback, sign-in +results report to the window that opened them instead of showing raw JSON, +chat status lists the sources a session can actually use, and V2 links stay in +V2. Concurrent requests no longer erase each other's session writes, which had +broken sign-in callbacks. See the +[connection session race fix](../fixes/M365_CONNECTION_SESSION_RACE_FIX.md). + Related version update: `application/single_app/config.py`. -Associated issues: #1493 and #1523. Related future work: #954 and #956. +Associated issues: #1493, #1523, #1604 and #1714. Related future work: #954 and #956. ## Overview @@ -60,15 +68,29 @@ operations also pass through source-sharing checks. SharePoint `Sites.Selected` grants. - Existing chat Blob storage for retained evidence and resumable processing. This dependency is independent of whether enhanced-citation rendering is on. -- Key Vault for saved workflow connections. Set the server environment variable - `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME` to a dedicated Key Vault secret holding - a base64-encoded 32-byte encryption key. Preserve historical key versions - while rotating active connection caches. -- Register `/api/m365/connections/callback` as a redirect URI for workflow - connection. This connection is separate from ordinary interactive sign-in. +- Key Vault for saved workflow connections. Turn on Key Vault secret storage + in Admin Settings > Security > Secrets > Key Vault. Every user's saved + workflow sign-in is encrypted with one deployment-wide 256-bit key, bound to + that user's connection record. Since 0.261.302 SimpleChat creates the key + itself, as the secret `simplechat-m365-workflow-token-key`, the first time + anyone connects or an administrator saves settings; the application identity + needs Key Vault Secrets Officer, which Key Vault secret storage already + requires. The optional `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME` app setting + renames the secret. Preserve historical key versions while rotating active + connection caches. Without Key Vault secret storage, Profile and V2 Settings + explain that workflow connections are not set up rather than failing on + Connect. +- Workflow connection sign-in returns to the registered `/getAToken` callback + (since 0.261.302), routed by its `m365-workflow-` state prefix, so no extra + redirect URI is needed. `/api/m365/connections/callback` still completes + sign-ins for deployments that registered it. This connection is separate + from ordinary interactive sign-in. - Interactive chat reuses the registered `/getAToken` callback with a separate state/nonce/PKCE-protected flow and the existing server-side login session. It does not require a saved workflow connection or Key Vault. +- A sign-in started in a popup ends on a small result page that reports the + outcome to the window that opened it and closes, so V2 never lands on a + classic page. A full-page sign-in from classic Profile returns to Profile. Admin Settings, **Agents & Actions**, offers `m365_retrieval_provider` (`auto` or `graph`) and `m365_trusted_download_hosts`. These hosts control diff --git a/docs/explanation/fixes/M365_CONNECTION_SESSION_RACE_FIX.md b/docs/explanation/fixes/M365_CONNECTION_SESSION_RACE_FIX.md new file mode 100644 index 000000000..4c2a79bce --- /dev/null +++ b/docs/explanation/fixes/M365_CONNECTION_SESSION_RACE_FIX.md @@ -0,0 +1,227 @@ +# Microsoft 365 connection session race and setup fix (v0.261.302) + +Fixed in version: **0.261.302** + +Related version update: `application/single_app/config.py`, from `0.261.301` +to `0.261.302`. Associated issues: +[#1714](https://github.com/microsoft/simplechat/issues/1714) for the session +race, V2 links and chat connection status, and +[#1604](https://github.com/microsoft/simplechat/issues/1604) for the workflow +key, callback and readiness work. + +## Issue + +On a deployment where chat could already read the user's mail and SharePoint, +the Microsoft 365 settings misbehaved in several ways at once: + +- **Reconnect Microsoft 365 for chat** opened a popup that showed raw JSON: + `{"error":"m365_auth_state_invalid","message":"This sign-in request expired or changed. Connect again from chat."}`, + seconds after the user had signed in. +- **Connect for workflows** failed in both interfaces with "Configure Key Vault + and a dedicated workflow encryption-key secret before connecting Microsoft + 365", an error no end user can act on. +- The chat connection card said "Sources saved for this session: none" while + chat was successfully using Email, OneDrive and SharePoint. +- Links in V2 chat (Connect Microsoft 365, Review the approval, Reconnect for + workflows) opened the classic Profile and Approvals pages instead of V2. + +## Root cause + +### Lost session writes (the popup error) + +Flask-Session 0.8 writes the whole stored session back at the end of every +request, including requests that changed nothing +(`should_set_storage` returns `session.modified or SESSION_REFRESH_EACH_REQUEST`, +and Flask's default is `True`). The request that finishes last wins. + +The affected App Service runs three instances with ARR affinity off and +sessions in Redis. The V2 interface re-reads `GET /api/v2/bootstrap`, which +took 5.2 seconds at the median and 7.4 at p90, whenever the window regains +focus. Application Insights showed the same sequence for both failed attempts: + +| Attempt | Bootstrap started | Connect POST saved the sign-in | Bootstrap finished and saved its copy | `/getAToken` | +|---|---|---|---|---| +| 1 | 14:09:39.2 (4865 ms) | 14:09:41.6 | ~14:09:44.1 | 14:09:44.9 โ†’ 400 | +| 2 | 14:11:00.3 (4971 ms) | 14:11:04.5 | ~14:11:05.3 | 14:11:08.6 โ†’ 400 | + +Each bootstrap had loaded the session before the connect call stored the +pending sign-in, and wrote its stale copy back afterwards. The callback then +found no pending sign-in. The same race could silently drop any session +write: token-cache refreshes, the Microsoft 365 CSRF token, the workflow +connect binding, and it could even recreate a session that had just signed +out. + +### Workflow connections had no key and no callback + +Saved workflow sign-ins are encrypted with one deployment-wide AES-256 key in +Key Vault. Nothing created that key: an operator had to create the secret and +set `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME`, and the deployment had neither. The +workflow callback `/api/m365/connections/callback` was also not registered on +the app registration, so connecting would have failed at Microsoft next. + +### Chat status reported only explicit reconnects + +The chat card listed only sources recorded by an explicit Profile reconnect. +Chat itself gets tokens silently with the session's refresh token, using +consent granted at sign-in or by an administrator, so a working session could +show "none". + +### Hard-coded classic links + +`m365Links.ts`, `m365Connect.ts` and the workflow proposal card pointed at +`/profile?...` and `/approvals`. V2 workflow connect also navigated the whole +page to Microsoft and returned to classic Profile. + +## Fix + +### Session saves merge instead of overwrite + +`functions_session_store.py` replaces the Redis and filesystem session +interfaces right after every `Session(app)`. It records what a request loaded +and, on save: + +- writes nothing when the request changed nothing, only refreshing the expiry + (`EXPIRE`, which never recreates a deleted session); +- otherwise writes only the keys the request changed and removes only the keys + it removed, merged into the latest stored copy. Redis applies the merge with + `WATCH`/`MULTI`/`EXEC` and retries a conflict up to five times, then merges + without the lock rather than overwriting; +- writes new, cleared and re-keyed sessions whole, as before; +- leaves a session that was deleted during the request deleted. + +Two requests that change the same key still resolve last writer wins. The V2 +shell also skips a focus-triggered bootstrap refresh while one is running or +started in the last 15 seconds. + +### The workflow key creates itself + +`_default_key_provider` now uses the secret `simplechat-m365-workflow-token-key` +by default; the app setting only renames it. When a connection is encrypted and +the secret does not exist, SimpleChat generates 32 random bytes and stores +them with `set_secret`, tagged and typed, without logging the value. +Decryption pins the stored key version and never creates a key, so two +instances creating it at once leave two readable versions rather than a broken +connection. Admin settings saves in both interfaces also make sure the key +exists when Key Vault storage is on; a failure is reported beside the Key +Vault settings only when that save changed them, and never blocks the save. +Missing write permission and a soft-deleted secret produce specific, +actionable errors. + +`GET /api/m365/connections` now includes `workflow_connections` +(`available`, `reason`, `message`), computed from settings alone. Both +interfaces disable **Connect for workflows** and explain that an administrator +needs to turn on Key Vault secret storage, instead of failing on click. + +### Workflow sign-in uses the registered callback + +Workflow sign-in now returns to `/getAToken`, which is always registered, with +an `m365-workflow-` state prefix that routes it to the workflow completion. +`/api/m365/connections/callback` still completes sign-ins for deployments that +registered it. + +### Sign-in results report back instead of showing JSON + +Callbacks render `m365_connection_result.html` with a local script. A sign-in +started as a popup (`"completion": "popup"`, sent by V2 and the classic chat) +posts `m365-profile-reconnected`, `m365-workflow-connected` or +`m365-connect-failed` with the safe error message to the same-origin opener, +then closes. A full-page classic sign-in keeps its redirect on success and +shows a readable error with a link back on failure. V2 shows the failure +message where the user clicked. + +### Chat status matches what chat does + +`read_chat_connection` derives the usable sources from the session's token +cache: a refresh token for the user plus each source's permissions on the +cached Graph tokens. It makes no network call. Both interfaces now say, for +example, "Signed in to Microsoft 365 for this session. Chat can use: Email, +OneDrive, SharePoint Online (SPO)." + +### V2 links stay in V2 + +`m365Links.ts` holds router paths: `/settings?tab=preferences§ion=m365-chat-connection`, +`/settings?tab=preferences§ion=m365-workflow-connection` and `/approvals/m365`, +rendered with `Link`. The Settings page scrolls to the card named by `section`. +V2 workflow connect uses the popup flow. + +## Files modified + +- `application/single_app/functions_session_store.py` (new), + `application/single_app/app.py` +- `application/single_app/functions_m365_connections.py`, + `application/single_app/route_backend_m365.py`, + `application/single_app/route_frontend_authentication.py` +- `application/single_app/route_frontend_admin_settings.py`, + `application/single_app/route_backend_v2.py` +- `application/single_app/templates/m365_connection_result.html` (new), + `application/single_app/static/js/profile/profile-m365-connection-result.js` (new), + `application/single_app/templates/profile.html`, + `application/single_app/static/js/profile/profile-m365.js`, + `application/single_app/static/js/chat/chat-m365-connect.js` +- `application/v2_ui/src/lib/m365Connect.ts`, `m365Links.ts`, + `components/settings/M365Cards.tsx`, `pages/SettingsPage.tsx`, + `stores/bootstrapStore.ts`, `App.tsx`, and the chat and approvals components + that link to Microsoft 365 settings + +## Validation + +New and updated tests: + +- `functional_tests/test_session_store_merge.py` (new, 14 tests): the production + interleaving (a slow request against a concurrent sign-in write), expiry-only + saves, no resurrection after sign-out, removals beside concurrent additions, + nested in-place changes, `WATCH` retry and exhausted-retry fallback, whole + writes for new and cleared sessions, unreadable stored data, the filesystem + backend, real Flask requests and `session_transaction`, and that `app.py` + installs the interface after every `Session(app)`. +- `functional_tests/test_m365_connections.py`: default secret name, key creation + only when encrypting, specific 403/409 errors, the never-raising admin + helper, settings-only readiness, the `m365-workflow-` state on `/getAToken` + with the legacy callback still accepted, popup completion, and chat sources + derived from the session cache, including no sources without a refresh token. +- `functional_tests/test_m365_routes.py`: result pages instead of JSON for chat + and workflow callbacks, popup and full-page completion, readiness in the + status read, `/getAToken` as the workflow callback, and the legacy route. +- `functional_tests/test_v2_user_settings_memory_m365.py` and + `test_v2_workflow_run_tracking_xss_guardrail.py`: popup connect, V2-only + link constants, and the `section` deep link. +- UI (local Chromium): `ui_tests/test_m365_connection_result_page.py` (new, 6), + `ui_tests/test_v2_settings_m365_connections.py` (new, 5), and updated + `test_v2_orchestration_m365_recovery.py`, + `test_v2_orchestration_workflow_proposal_card.py`, + `test_m365_lifecycle_and_approvals.py` and `test_m365_pending_action_cards.py`. + +Results: + +- 131 related functional test files, about 1,490 tests, were run here and on + an unmodified checkout of the same commit. No test fails here that passes on + the baseline. 72 tests fail, and 6 files cannot be collected, identically on + both, because of this machine's environment: for example an older local + Flask, an incompatible pyOpenSSL, and modules that need live Azure + configuration. +- Route policy: 14/14, 9/9 and 3/3. Documentation quality 6/6 and coverage 7/7 + with the regenerated inventory. +- V2 `npm run typecheck` passed; the V2 `.mjs` logic suites passed, including + the real bootstrap store (8/8), apart from two workspace-action suites that + fail identically on the baseline. +- Playwright: classic Microsoft 365 suites 155 passed (one fixture fails to + start on the same pyOpenSSL mismatch), result page 6, V2 Settings 5, V2 + orchestration recovery and workflow proposal card 54. +- `scripts/check_xss_sinks.py --full-file` over every changed browser-facing + file reports only findings on unchanged `profile.html` lines. + +## Deployment + +Deploy **0.261.302**. With Key Vault secret storage on and the application +identity holding Key Vault Secrets Officer, as Key Vault storage already +requires, there is nothing else to configure: the key is created on the first +workflow connection or admin settings save, and `/getAToken` is already +registered. Users whose tenant blocks user consent still need an administrator +to grant the Microsoft 365 delegated permissions. + +## Follow-ups + +- `GET /api/v2/bootstrap` spends about five seconds in Python per call. The + merge removes the correctness impact, but the latency deserves profiling. +- `Initialize-EntraApplication.ps1` does not declare the Microsoft 365 + delegated permissions (#1604, item 3). diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index f4b604495..57e628a2f 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,31 @@ 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.302)** + +#### Bug Fixes + +* **Microsoft 365 Sign-In No Longer Fails With `m365_auth_state_invalid`** + * Reconnecting Microsoft 365 for chat could fail seconds after the user signed in, leaving raw JSON in the popup. The session store wrote the whole session back at the end of every request, so the request that finished last won. In V2, the startup-data refresh that runs when the window regains focus took about five seconds, loaded the session before the connect call saved the pending sign-in, and wrote its stale copy back afterwards. + * A request now saves only the session keys it changed, merged into the latest stored copy, and a request that changed nothing no longer writes its copy back. A session deleted during a request stays deleted. This also stops overlapping requests from dropping token-cache refreshes, the Microsoft 365 CSRF token and other session writes. + * V2 skips the focus refresh while one is running or one started in the last 15 seconds. + * Sign-in callbacks show a result page instead of JSON. In a popup it reports back to the page that opened it and closes; opened as a full page, it continues on success or explains the error with a way to try again. + * (Ref: `functions_session_store.py`, `app.py`, `m365_connection_result.html`, `route_backend_m365.py`, `bootstrapStore.ts`, [#1714](https://github.com/microsoft/simplechat/issues/1714), [Microsoft 365 Connection Session Race Fix](fixes/M365_CONNECTION_SESSION_RACE_FIX.md)) + +* **Workflow Connections Create Their Own Encryption Key** + * **Connect for workflows** failed with "Configure Key Vault and a dedicated workflow encryption-key secret" unless an operator had created a key and set `M365_WORKFLOW_TOKEN_KEY_SECRET_NAME`. SimpleChat now creates one deployment-wide key in the configured Key Vault the first time someone connects, and checks for it whenever an administrator saves settings. The app setting still overrides the secret name, so existing keys keep working. + * This needs Key Vault secret storage on and the application identity allowed to create secrets. **Connect for workflows** is disabled with an explanation until Key Vault secret storage and a Key Vault name are configured. + * Workflow sign-in now returns through `/getAToken`, which the app registration already lists, so no extra redirect URI is needed. + * (Ref: `functions_m365_connections.py` `ensure_m365_workflow_encryption_key`, `workflow_connection_readiness`, `route_frontend_authentication.py`, `route_frontend_admin_settings.py`, `route_backend_v2.py`, [#1604](https://github.com/microsoft/simplechat/issues/1604)) + +* **Chat Connection Status Matches What Chat Can Use** + * The chat connection card said "Sources saved for this session: none" while chat was reading mail, OneDrive and SharePoint, because it listed only sources from an explicit reconnect. It now lists the sources the current sign-in can use, in both V2 Settings and classic Profile. + * (Ref: `functions_m365_connections.py` `read_chat_connection`, `profile-m365.js`, `M365Cards.tsx`) + +* **Microsoft 365 Links Stay in V2** + * Connect, reconnect and approval links in V2 chat, workflow cards and Approvals opened the classic Profile and Approvals pages. They now open V2 Settings at the Microsoft 365 section and V2 Approvals, and connecting for workflows in V2 uses a popup, like chat, instead of leaving the page. + * (Ref: `m365Links.ts`, `m365Connect.ts`, `M365Cards.tsx`, `SettingsPage.tsx`, `OrchestrationM365Notice.tsx`, `WorkflowProposalCard.tsx`, `WorkflowRunCard.tsx`, `PendingActionsPanel.tsx`, `PausedRequestsPanel.tsx`) + ### **(v0.261.301)** #### New Features diff --git a/docs/reference/logging-tags.md b/docs/reference/logging-tags.md index b321d1988..4a44b964a 100644 --- a/docs/reference/logging-tags.md +++ b/docs/reference/logging-tags.md @@ -231,6 +231,7 @@ Last inventoried: 2026-08-10 - `[SEARCH_CACHE_DEBUG]` - `[SEARCH_SERVICE]` - `[SERVICE_HEALTH]` +- `[SESSION_STORE]` - `[SHARED_CACHE]` - `[SIMPLE_CHAT]` - `[SIMPLE_CHAT_DEBUG_TRACE]` diff --git a/functional_tests/test_ai_connection_embedding_defaults_api.py b/functional_tests/test_ai_connection_embedding_defaults_api.py index 3bf6822c5..09a82075f 100644 --- a/functional_tests/test_ai_connection_embedding_defaults_api.py +++ b/functional_tests/test_ai_connection_embedding_defaults_api.py @@ -253,6 +253,9 @@ def stage_secret(endpoint, _scope_value, *, scope, existing_endpoint, stage_new_ "_seed_connections_on_first_enable": lambda *_args: self.events.append("seed"), "_log_governance_setting_changes": lambda *_args: None, "_refresh_branding_static_files": lambda: None, + "ensure_m365_workflow_encryption_key": lambda _settings: { + "status": "not_configured", "reason": "key_vault_disabled", "message": "", + }, "_redact_admin_settings_for_v2": lambda updates: copy.deepcopy(updates), }) load_functions( diff --git a/functional_tests/test_content_screening_admin_settings_fix.py b/functional_tests/test_content_screening_admin_settings_fix.py index afc3e3297..8c439a06b 100644 --- a/functional_tests/test_content_screening_admin_settings_fix.py +++ b/functional_tests/test_content_screening_admin_settings_fix.py @@ -71,6 +71,9 @@ def patch_handler(settings, *, write_succeeds=True, validation_error=None): "validate_content_screening_settings": validator, "update_settings": writes, "_refresh_branding_static_files": Mock(), + "ensure_m365_workflow_encryption_key": Mock(return_value={ + "status": "not_configured", "reason": "key_vault_disabled", "message": "", + }), "_redact_admin_settings_for_v2": deepcopy, "log_event": Mock(), } diff --git a/functional_tests/test_m365_connections.py b/functional_tests/test_m365_connections.py index 1d26073e3..88ba24685 100644 --- a/functional_tests/test_m365_connections.py +++ b/functional_tests/test_m365_connections.py @@ -1,7 +1,7 @@ # test_m365_connections.py """ Functional tests for encrypted Microsoft 365 workflow connections. -Version: 0.261.034 +Version: 0.261.302 Implemented in: 0.261.029 Uses real MSAL authorization-code/cache logic with a scoped HTTP fake, real @@ -621,7 +621,8 @@ def test_profile_reconnect_repairs_interactive_cache_without_a_saved_request_or_ self.assertEqual(cache_before_callback, "broken-cache") self.assertEqual(completed, {"return_to": "profile"}) self.assertEqual(status_after["status"], "available") - self.assertEqual(status_after["sources"], ["spo"]) + # OneDrive and SPO need the same permissions, so connecting SPO makes both usable. + self.assertEqual(status_after["sources"], ["onedrive", "spo"]) self.assertIn("connected_at", status_after) self.assertEqual(original_user, user_after) self.assertNotIn("request_id", metadata) @@ -631,6 +632,51 @@ def test_profile_reconnect_repairs_interactive_cache_without_a_saved_request_or_ self.assertNotIn("refresh-token", json.dumps(status_after)) self.service.key_provider.assert_not_called() + def test_chat_status_reports_sources_granted_to_the_session_without_explicit_reconnect(self): + """Chat could read mail although Settings said 'Sources saved for this session: none'.""" + app = Flask(__name__) + app.secret_key = "unit-test-only" + # Microsoft returns every scope consented for the resource on each token. + self.http.scope_transform = lambda scopes: " ".join([ + scopes, "Mail.Read Mail.ReadWrite Mail.Send User.ReadBasic.All", + "Calendars.Read MailboxSettings.Read", + ]) + with app.test_request_context(): + _begin, query = self.begin_chat() + self.service.complete_chat_connection("user-a", "tenant-a", {"state": query["state"][0], "code": "chat"}) + session.pop(connections.CHAT_CONNECTION_SESSION_KEY) + status = self.service.read_chat_connection("user-a", "tenant-a") + cache = connections.deserialize_m365_cache(session["token_cache"]) + for token in list(cache.search(msal.TokenCache.CredentialType.REFRESH_TOKEN)): + cache.remove_rt(token) + session["token_cache"] = cache.serialize() + without_refresh = self.service.read_chat_connection("user-a", "tenant-a") + self.assertEqual(status["status"], "available") + self.assertEqual(status["sources"], ["email", "onedrive", "spo"]) + self.assertNotIn("calendar", status["sources"]) + self.assertEqual(without_refresh["sources"], []) + + def test_profile_chat_reconnect_reports_popup_completion(self): + app = Flask(__name__) + app.secret_key = "unit-test-only" + with app.test_request_context(): + session["user"] = {"oid": "user-a", "tid": "tenant-a", "roles": ["User"]} + begin = self.service.start_profile_chat_connection( + "user-a", "tenant-a", ["email"], f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}", + completion="popup", + ) + query = parse_qs(urlsplit(begin["authorization_url"]).query) + self.http.nonce = query["nonce"][0] + completed = self.service.complete_chat_connection( + "user-a", "tenant-a", {"state": query["state"][0], "code": "popup-code"}, + ) + with self.assertRaises(ValueError): + self.service.start_profile_chat_connection( + "user-a", "tenant-a", ["email"], + f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}", completion="tab", + ) + self.assertEqual(completed, {"return_to": "profile", "completion": "popup"}) + def test_failed_profile_reconnect_preserves_previous_sign_in_and_reconnect_marker(self): app = Flask(__name__) app.secret_key = "unit-test-only" @@ -1003,37 +1049,186 @@ def test_default_msal_factory_uses_pinned_graph_authority_without_public_discove ) self.assertIs(client, constructor.return_value) - def test_default_key_provider_requires_explicit_key_vault_reference(self): + def key_vault_modules(self, settings=None): config_stub = types.ModuleType("config") config_stub.KEY_VAULT_DOMAIN = ".vault.azure.net" settings_stub = types.ModuleType("functions_settings") - settings_stub.get_settings = lambda: { + settings_stub.get_settings = lambda: dict(settings or { "enable_key_vault_secret_storage": True, "key_vault_name": "test-vault", - } + }) vault_stub = types.ModuleType("functions_keyvault") vault_stub.get_keyvault_credential = lambda settings: "test-credential" - properties = types.SimpleNamespace( - enabled=True, expires_on=None, not_before=None, version="version1", - ) - secret = types.SimpleNamespace( - value=base64.b64encode(b"a" * 32).decode("ascii"), properties=properties, - ) - client = types.SimpleNamespace(get_secret=lambda name, version=None: secret) - with patch.dict(sys.modules, { - "config": config_stub, "functions_settings": settings_stub, "functions_keyvault": vault_stub, - }), patch.dict(os.environ, {connections.KEY_SECRET_ENV: ""}), patch( + return {"config": config_stub, "functions_settings": settings_stub, "functions_keyvault": vault_stub} + + @staticmethod + def vault_secret(value, version): + properties = types.SimpleNamespace(enabled=True, expires_on=None, not_before=None, version=version) + return types.SimpleNamespace(value=value, properties=properties) + + def test_default_key_provider_requires_key_vault_storage_and_names_the_secret_itself(self): + secret = self.vault_secret(base64.b64encode(b"a" * 32).decode("ascii"), "version1") + client = Mock() + client.get_secret.return_value = secret + disabled = self.key_vault_modules({"enable_key_vault_secret_storage": False, "key_vault_name": "test-vault"}) + with patch.dict(sys.modules, disabled), patch.dict(os.environ, {connections.KEY_SECRET_ENV: ""}), patch( "azure.keyvault.secrets.SecretClient", return_value=client, ) as constructor: - with self.assertRaises(connections.M365ConnectionError): + with self.assertRaises(connections.M365ConnectionError) as raised: connections._default_key_provider() constructor.assert_not_called() - os.environ[connections.KEY_SECRET_ENV] = "workflow-token-key" - key = connections._default_key_provider("version1", "workflow-token-key") - self.assertEqual(key.key, b"a" * 32) - self.assertEqual(key.version, "version1") + self.assertEqual(raised.exception.code, "m365_key_vault_required") + self.assertIn("Admin Settings > Security > Secrets > Key Vault", raised.exception.payload["message"]) + + with patch.dict(sys.modules, self.key_vault_modules()), patch.dict(os.environ, {connections.KEY_SECRET_ENV: ""}), patch( + "azure.keyvault.secrets.SecretClient", return_value=client, + ) as constructor: + key = connections._default_key_provider() constructor.assert_called_once_with( vault_url="https://test-vault.vault.azure.net", credential="test-credential", ) + client.get_secret.assert_called_with(connections.DEFAULT_KEY_SECRET_NAME, version=None) + self.assertEqual((key.key, key.version, key.name), (b"a" * 32, "version1", connections.DEFAULT_KEY_SECRET_NAME)) + + with patch.dict(sys.modules, self.key_vault_modules()), patch.dict( + os.environ, {connections.KEY_SECRET_ENV: "workflow-token-key"}, + ), patch("azure.keyvault.secrets.SecretClient", return_value=client): + pinned = connections._default_key_provider("version1", "workflow-token-key") + with self.assertRaises(connections.M365ConnectionError) as renamed: + connections._default_key_provider("version1", connections.DEFAULT_KEY_SECRET_NAME) + client.get_secret.assert_called_with("workflow-token-key", version="version1") + self.assertEqual(pinned.name, "workflow-token-key") + self.assertEqual(renamed.exception.code, "m365_key_unavailable") + + def test_missing_key_is_created_once_when_encrypting_and_never_when_decrypting(self): + from azure.core.exceptions import ResourceNotFoundError + client = Mock() + client.get_secret.side_effect = ResourceNotFoundError("SecretNotFound") + client.set_secret.side_effect = lambda name, value, **kwargs: types.SimpleNamespace( + value=None, properties=types.SimpleNamespace( + enabled=True, expires_on=None, not_before=None, version="created-version", + ), + ) + logged = [] + with patch.dict(sys.modules, self.key_vault_modules()), patch.dict(os.environ, {connections.KEY_SECRET_ENV: ""}), patch( + "azure.keyvault.secrets.SecretClient", return_value=client, + ), patch.object(connections, "_log_key_created", lambda name, version: logged.append((name, version))): + with self.assertRaises(connections.M365ConnectionError) as decrypting: + connections._default_key_provider("old-version", connections.DEFAULT_KEY_SECRET_NAME) + client.set_secret.assert_not_called() + key = connections._default_key_provider() + + self.assertEqual(decrypting.exception.code, "m365_key_unavailable") + client.set_secret.assert_called_once() + name, value = client.set_secret.call_args.args + self.assertEqual(name, connections.DEFAULT_KEY_SECRET_NAME) + self.assertEqual(len(base64.b64decode(value, validate=True)), 32) + self.assertEqual(client.set_secret.call_args.kwargs, { + "content_type": connections.KEY_SECRET_CONTENT_TYPE, "tags": connections.KEY_SECRET_TAGS, + }) + self.assertEqual(key.key, base64.b64decode(value)) + self.assertEqual((key.version, key.name), ("created-version", connections.DEFAULT_KEY_SECRET_NAME)) + self.assertEqual(logged, [(connections.DEFAULT_KEY_SECRET_NAME, "created-version")]) + self.assertNotIn(value, repr(logged)) + + def test_key_creation_failures_name_the_missing_permission_or_deleted_secret(self): + from azure.core.exceptions import HttpResponseError, ResourceNotFoundError + cases = {403: "m365_key_provision_forbidden", 409: "m365_key_deleted", 500: "m365_key_unavailable"} + for status, code in cases.items(): + with self.subTest(status=status): + failure = HttpResponseError(message="private provider detail") + failure.status_code = status + client = Mock() + client.get_secret.side_effect = ResourceNotFoundError("SecretNotFound") + client.set_secret.side_effect = failure + with patch.dict(sys.modules, self.key_vault_modules()), patch.dict( + os.environ, {connections.KEY_SECRET_ENV: ""}, + ), patch("azure.keyvault.secrets.SecretClient", return_value=client): + with self.assertRaises(connections.M365ConnectionError) as raised: + connections._default_key_provider() + self.assertEqual(raised.exception.code, code) + self.assertNotIn("private provider detail", raised.exception.payload["message"]) + + def test_ensure_workflow_key_reports_status_and_never_raises(self): + from azure.core.exceptions import ResourceNotFoundError + modules = self.key_vault_modules() + settings = modules["functions_settings"].get_settings() + self.assertEqual( + connections.ensure_m365_workflow_encryption_key({"enable_key_vault_secret_storage": False})["status"], + "not_configured", + ) + present = Mock() + present.get_secret.return_value = self.vault_secret(base64.b64encode(b"a" * 32).decode("ascii"), "v1") + missing = Mock() + missing.get_secret.side_effect = ResourceNotFoundError("SecretNotFound") + missing.set_secret.return_value = self.vault_secret(None, "v2") + broken = Mock() + broken.get_secret.side_effect = RuntimeError("unexpected SDK failure") + results = {} + for label, client in (("present", present), ("missing", missing), ("broken", broken)): + with patch.dict(sys.modules, modules), patch.dict(os.environ, {connections.KEY_SECRET_ENV: ""}), patch( + "azure.keyvault.secrets.SecretClient", return_value=client, + ), patch.object(connections, "_log_key_created", lambda *args: None): + results[label] = connections.ensure_m365_workflow_encryption_key(settings) + self.assertEqual(results["present"]["status"], "ready") + self.assertEqual(results["missing"]["status"], "created") + self.assertIn(connections.DEFAULT_KEY_SECRET_NAME, results["missing"]["message"]) + self.assertEqual(results["broken"]["status"], "unavailable") + self.assertNotIn("unexpected SDK failure", results["broken"]["message"]) + + def test_workflow_readiness_comes_from_settings_alone(self): + readiness = connections.workflow_connection_readiness + self.assertEqual(readiness({})["reason"], "key_vault_disabled") + self.assertEqual( + readiness({"enable_key_vault_secret_storage": True, "key_vault_name": ""})["reason"], + "key_vault_name_missing", + ) + ready = {"enable_key_vault_secret_storage": True, "key_vault_name": "test-vault"} + with patch.dict(os.environ, {connections.KEY_SECRET_ENV: ""}): + self.assertEqual(readiness(ready), {"available": True, "reason": None, "message": ""}) + with patch.dict(os.environ, {connections.KEY_SECRET_ENV: "not a valid/name"}): + self.assertEqual(readiness(ready)["reason"], "key_secret_name_invalid") + + def test_workflow_sign_in_uses_registered_callback_prefix_and_remembers_completion(self): + for completion in ("page", "popup"): + with self.subTest(completion=completion): + result = self.service.start_connection( + "user-a", "tenant-a", ["email"], + f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}", + "session-binding-for-user-a", completion=completion, + ) + query = parse_qs(urlsplit(result["authorization_url"]).query) + state = query["state"][0] + self.assertTrue(state.startswith(connections.WORKFLOW_AUTH_STATE_PREFIX)) + self.assertEqual(query["redirect_uri"], [f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}"]) + self.assertEqual(self.service.connection_flow_completion("user-a", state), completion) + self.assertEqual(self.service.connection_flow_completion("user-b", state), "auto") + self.assertEqual(self.service.connection_flow_completion("user-a", "unknown-state-value-123456"), "auto") + with self.assertRaises(ValueError): + self.service.start_connection( + "user-a", "tenant-a", ["email"], + f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}", + "session-binding-for-user-a", completion="redirect", + ) + with self.assertRaises(connections.M365ConnectionError): + self.service.start_connection( + "user-a", "tenant-a", ["email"], "https://simplechat.example.test/elsewhere", + "session-binding-for-user-a", + ) + + def test_workflow_callback_through_registered_callback_completes(self): + result = self.service.start_connection( + "user-a", "tenant-a", ["email"], + f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}", + "session-binding-for-user-a", + ) + query = parse_qs(urlsplit(result["authorization_url"]).query) + self.http.nonce = query["nonce"][0] + connected = self.service.complete_connection( + "user-a", "tenant-a", {"state": query["state"][0], "code": "one-use-code"}, + "session-binding-for-user-a", + ) + self.assertEqual(connected["status"], "connected") + self.assertEqual(self.http.posts[-1]["redirect_uri"], f"https://simplechat.example.test{connections.CHAT_CALLBACK_PATH}") if __name__ == "__main__": diff --git a/functional_tests/test_m365_routes.py b/functional_tests/test_m365_routes.py index d1d0a18bd..275951f16 100644 --- a/functional_tests/test_m365_routes.py +++ b/functional_tests/test_m365_routes.py @@ -1,7 +1,7 @@ # test_m365_routes.py """ Functional tests for Microsoft 365 Profile, approval, and audit routes. -Version: 0.261.034 +Version: 0.261.302 Implemented in: 0.261.029 Imports the real route module with scoped authentication/logging I/O seams. @@ -110,7 +110,10 @@ def setUp(self): } with patch.dict(sys.modules, dependencies): spec.loader.exec_module(self.routes) - self.app = Flask(__name__) + # The real templates and static files, so callbacks render the actual result page. + self.app = Flask( + __name__, template_folder=str(APP_DIR / "templates"), static_folder=str(APP_DIR / "static"), + ) self.app.secret_key = "unit-test-only" blueprint = Blueprint("backend_m365", __name__) self.routes.register_route_backend_m365(blueprint) @@ -497,6 +500,144 @@ def test_registered_chat_callback_returns_no_store_for_success_and_failure(self) self.assertEqual(response.headers["Cache-Control"], "private, no-store") self.assertEqual(response.headers["Pragma"], "no-cache") self.assertEqual(response.status_code, 302 if result == "connected" else 400) + if result == "invalid": + # A readable page that can report to an opener, not raw JSON in the popup. + page = response.get_data(as_text=True) + self.assertEqual(response.mimetype, "text/html") + self.assertIn("Microsoft 365 was not connected", page) + self.assertIn("Connect again.", page) + self.assertIn('"type": "m365-connect-failed"', page) + self.assertIn('"code": "m365_auth_state_invalid"', page) + self.assertIn("js/profile/profile-m365-connection-result.js", page) + + def test_chat_callback_answers_the_popup_that_started_it(self): + dependencies = self.chat_request_dependencies(CosmosContainer("user_id")) + dependencies["functions_m365_runtime"] = module_stub( + "functions_m365_runtime", _conversation_access=Mock(side_effect=AssertionError("No request is resumed.")), + ) + with patch.dict(sys.modules, dependencies), self.app.test_request_context( + "/getAToken?state=m365-chat-popupstate&code=opaque", + ): + session["user"] = {"oid": "user-a", "tid": "tenant-a", "roles": ["User"]} + session[connections.CHAT_AUTH_SESSION_KEY] = { + "flow": {"state": "m365-chat-popupstate"}, "completion": "popup", "purpose": "profile_reconnect", + } + with patch.object( + self.connection_service, "complete_chat_connection", + return_value={"return_to": "profile", "completion": "popup"}, + ): + connected = self.routes.complete_m365_chat_connection_callback() + with patch.object( + self.connection_service, "complete_chat_connection", + side_effect=connections.M365ConnectionError("m365_consent_required", "Consent was not completed."), + ): + session[connections.CHAT_AUTH_SESSION_KEY] = { + "flow": {"state": "m365-chat-popupstate"}, "completion": "popup", "purpose": "profile_reconnect", + } + failed = self.routes.complete_m365_chat_connection_callback() + self.assertEqual(connected.status_code, 200) + self.assertIn('"type": "m365-profile-reconnected"', connected.get_data(as_text=True)) + self.assertIn('"completion": "popup"', connected.get_data(as_text=True)) + self.assertEqual(failed.status_code, 400) + self.assertIn('"completion": "popup"', failed.get_data(as_text=True)) + self.assertIn("Consent was not completed.", failed.get_data(as_text=True)) + + def test_workflow_connect_uses_registered_callback_and_reports_readiness(self): + dependencies = { + "config": module_stub("config", LOGIN_REDIRECT_URL=None), + "functions_settings": module_stub("functions_settings", get_settings=lambda: { + "enable_key_vault_secret_storage": False, + }), + } + begin = Mock(return_value={"authorization_url": "https://login.microsoftonline.com/tenant-a/authorize"}) + with patch.dict(sys.modules, dependencies), patch.object(self.connection_service, "start_connection", begin): + status = self.client.get("/api/m365/connections") + page = self.client.post( + "/api/m365/connections/connect", json={"sources": ["email"]}, + headers={"X-M365-CSRF-Token": self.csrf}, + ) + popup = self.client.post( + "/api/m365/connections/connect", json={"sources": ["email"], "completion": "popup"}, + headers={"X-M365-CSRF-Token": self.csrf}, + ) + invalid = self.client.post( + "/api/m365/connections/connect", json={"sources": ["email"], "completion": "window"}, + headers={"X-M365-CSRF-Token": self.csrf}, + ) + readiness = status.get_json()["workflow_connections"] + self.assertEqual(readiness["available"], False) + self.assertEqual(readiness["reason"], "key_vault_disabled") + self.assertIn("Admin Settings", readiness["message"]) + self.assertEqual((page.status_code, popup.status_code, invalid.status_code), (200, 200, 400)) + self.assertEqual(begin.call_count, 2) + for call, completion in zip(begin.call_args_list, ("page", "popup")): + self.assertEqual(call.args[3], "http://localhost/getAToken") + self.assertEqual(call.kwargs["completion"], completion) + + def workflow_callback(self, path, *, completion, binding=True, failure=None): + with self.app.test_request_context(path): + session["user"] = {"oid": "user-a", "tid": "tenant-a", "roles": ["User"]} + if binding: + session["m365_workflow_oauth_binding"] = "binding" + with patch.object( + self.connection_service, "connection_flow_completion", return_value=completion, + ), patch.object( + self.connection_service, "complete_connection", + side_effect=failure, return_value={"status": "connected"}, + ) as complete: + response = self.routes.complete_m365_workflow_connection_callback() + binding_left = session.get("m365_workflow_oauth_binding") + return response, complete, binding_left + + def test_workflow_callback_answers_popups_redirects_pages_and_explains_failures(self): + path = "/getAToken?state=m365-workflow-abcdefghijklmnopqrstuvwxyz&code=opaque" + popup, complete, binding_left = self.workflow_callback(path, completion="popup") + self.assertEqual(popup.status_code, 200) + self.assertIn('"type": "m365-workflow-connected"', popup.get_data(as_text=True)) + self.assertEqual(complete.call_args.args[:4], ( + "user-a", "tenant-a", {"state": "m365-workflow-abcdefghijklmnopqrstuvwxyz", "code": "opaque"}, "binding", + )) + self.assertIsNone(binding_left) + + page, _complete, _binding = self.workflow_callback(path, completion="page") + self.assertEqual(page.status_code, 302) + self.assertEqual(page.headers["Location"], self.routes.CLASSIC_WORKFLOW_CONNECTED_URL) + + expired, complete, _binding = self.workflow_callback(path, completion="popup", binding=False) + self.assertEqual(expired.status_code, 400) + self.assertIn('"code": "m365_auth_state_invalid"', expired.get_data(as_text=True)) + self.assertIn("Start Connect for workflows again.", expired.get_data(as_text=True)) + complete.assert_not_called() + + key_error = connections.M365ConnectionError("m365_key_provision_forbidden", "Grant Key Vault Secrets Officer.") + forbidden, _complete, _binding = self.workflow_callback(path, completion="auto", failure=key_error) + self.assertEqual(forbidden.status_code, 503) + self.assertEqual(forbidden.headers["Cache-Control"], "private, no-store") + self.assertIn('"completion": "auto"', forbidden.get_data(as_text=True)) + self.assertIn("Grant Key Vault Secrets Officer.", forbidden.get_data(as_text=True)) + + def test_legacy_workflow_callback_still_completes_existing_registrations(self): + with patch.object(self.routes, "complete_m365_workflow_connection_callback", return_value="workflow-done") as workflow, \ + patch.object(self.routes, "complete_m365_chat_connection_callback", return_value="chat-done") as chat: + workflow_response = self.client.get("/api/m365/connections/callback?state=legacy-state-value&code=x") + chat_response = self.client.get("/api/m365/connections/callback?state=m365-chat-value&code=x") + self.assertEqual(workflow_response.get_data(as_text=True), "workflow-done") + self.assertEqual(chat_response.get_data(as_text=True), "chat-done") + workflow.assert_called_once() + chat.assert_called_once() + + def test_sign_in_callback_routes_workflow_state_to_the_workflow_completion(self): + source = (APP_DIR / "route_frontend_authentication.py").read_text(encoding="utf-8") + tree = ast.parse(source) + authorized = next( + node for node in ast.walk(tree) if isinstance(node, ast.FunctionDef) and node.name == "authorized" + ) + body = ast.get_source_segment(source, authorized) + workflow_branch = body.index("startswith(WORKFLOW_AUTH_STATE_PREFIX)") + self.assertLess(body.index("startswith(CHAT_AUTH_STATE_PREFIX)"), workflow_branch) + self.assertLess(workflow_branch, body.index("complete_m365_workflow_connection_callback()")) + # Both M365 branches run before the login token exchange. + self.assertLess(body.index("complete_m365_workflow_connection_callback()"), body.index("request.args.get('code')")) def test_chat_callback_returns_to_original_visible_conversation_without_running_a_request(self): jobs = CosmosContainer("user_id") @@ -550,9 +691,22 @@ def test_profile_chat_reconnect_uses_current_subject_sources_and_fixed_callback( "/api/m365/chat/connection/connect", json={"sources": ["spo", "email"]}, headers={"X-M365-CSRF-Token": self.csrf}, ) + popup = self.client.post( + "/api/m365/chat/connection/connect", json={"sources": ["email"], "completion": "popup"}, + headers={"X-M365-CSRF-Token": self.csrf}, + ) + invalid_completion = self.client.post( + "/api/m365/chat/connection/connect", json={"sources": ["email"], "completion": "tab"}, + headers={"X-M365-CSRF-Token": self.csrf}, + ) self.assertEqual(missing.status_code, 403) self.assertEqual(valid.status_code, 200) - begin.assert_called_once_with("user-a", "tenant-a", ["spo", "email"], "http://localhost/getAToken") + self.assertEqual(popup.status_code, 200) + self.assertEqual(invalid_completion.status_code, 400) + self.assertEqual(begin.call_args_list[0].args, ("user-a", "tenant-a", ["spo", "email"], "http://localhost/getAToken")) + self.assertEqual(begin.call_args_list[0].kwargs, {"completion": "page"}) + self.assertEqual(begin.call_args_list[1].kwargs, {"completion": "popup"}) + self.assertEqual(begin.call_count, 2) self.assertEqual(self.connection_container.items, {}) def test_profile_callback_does_not_read_or_replay_any_chat_request(self): diff --git a/functional_tests/test_session_store_merge.py b/functional_tests/test_session_store_merge.py new file mode 100644 index 000000000..c04f2a805 --- /dev/null +++ b/functional_tests/test_session_store_merge.py @@ -0,0 +1,525 @@ +#!/usr/bin/env python3 +# test_session_store_merge.py +""" +Functional test for the merging server-side session store. +Version: 0.261.302 +Implemented in: 0.261.302 + +Flask-Session 0.8 writes the whole stored session at the end of every request, so a slow +request that loaded the session before another request saved a change used to write its stale +copy over that change. In production a ~5 second V2 bootstrap request erased a pending +Microsoft 365 sign-in that a concurrent request had just saved, and the sign-in callback then +failed with m365_auth_state_invalid. + +This test ensures that saves write only the keys a request changed, merged into the latest +stored copy (atomically for Redis), that unchanged sessions only refresh their expiry, that +sessions deleted mid-request are not recreated, and that app.py installs the merging interface +after every Session(app). +""" + +import ast +import sys +import tempfile +from datetime import timedelta +from importlib.metadata import version as package_version +from pathlib import Path +from unittest.mock import patch + +import flask +import redis +import werkzeug +from flask import Flask +from flask_session import Session +from redis.exceptions import WatchError + +ROOT = Path(__file__).resolve().parents[1] +APP_DIR = ROOT / "application" / "single_app" +sys.path.insert(0, str(APP_DIR)) +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import functions_session_store as store # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +class FakePipeline: + """The subset of a redis-py transaction pipeline the session store uses.""" + + def __init__(self, client): + self.client = client + self.reset() + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + self.reset() + + def reset(self): + self.watched = None + self.watched_version = None + self.commands = [] + + def watch(self, name): + self.watched = name + self.watched_version = self.client.versions.get(name, 0) + + def get(self, name): + return self.client.get(name) + + def multi(self): + self.commands = [] + + def set(self, name, value, ex=None): + self.commands.append(("set", name, value, ex)) + + def delete(self, name): + self.commands.append(("delete", name)) + + def execute(self): + self.client.transactions += 1 + if self.client.before_execute is not None: + self.client.before_execute() + if self.watched is not None and self.client.versions.get(self.watched, 0) != self.watched_version: + self.reset() + raise WatchError("Watched variable changed.") + for command in self.commands: + if command[0] == "set": + self.client.set(command[1], command[2], ex=command[3]) + else: + self.client.delete(command[1]) + self.reset() + return [] + + +class FakeRedis(redis.Redis): + """An in-memory Redis that records versions so WATCH conflicts can be simulated.""" + + def __init__(self): + self.data = {} + self.ttls = {} + self.versions = {} + self.expire_calls = [] + self.transactions = 0 + self.before_execute = None + + def __del__(self): + pass + + def close(self): + pass + + def _bump(self, name): + self.versions[name] = self.versions.get(name, 0) + 1 + + def get(self, name): + return self.data.get(name) + + def set(self, name, value, ex=None, **_kwargs): + self.data[name] = value + self.ttls[name] = ex + self._bump(name) + return True + + def expire(self, name, time, **_kwargs): + self.expire_calls.append((name, time)) + if name not in self.data: + return False + self.ttls[name] = time + return True + + def delete(self, *names): + removed = 0 + for name in names: + if name in self.data: + del self.data[name] + self.ttls.pop(name, None) + self._bump(name) + removed += 1 + return removed + + def pipeline(self, transaction=True, shard_hint=None): + return FakePipeline(self) + + +def build_redis_app(client=None): + app = Flask(__name__) + app.config.update( + SECRET_KEY="test-secret", + SESSION_TYPE="redis", + SESSION_REDIS=client or FakeRedis(), + PERMANENT_SESSION_LIFETIME=timedelta(hours=2), + ) + Session(app) + store.install_merging_session_interface(app) + return app + + +def build_filesystem_app(directory): + app = Flask(__name__) + app.config.update( + SECRET_KEY="test-secret", + SESSION_TYPE="filesystem", + SESSION_FILE_DIR=directory, + PERMANENT_SESSION_LIFETIME=timedelta(hours=2), + ) + Session(app) + store.install_merging_session_interface(app) + return app + + +def open_request(app, sid=None): + """Open the session as a request carrying this session cookie would.""" + headers = {"Cookie": f"{app.config['SESSION_COOKIE_NAME']}={sid}"} if sid else {} + with app.test_request_context("/", headers=headers): + return app.session_interface.open_session(app, flask.request) + + +def save(app, session): + response = app.response_class() + with app.test_request_context("/"): + app.session_interface.save_session(app, session, response) + return response + + +def stored(app, sid): + interface = app.session_interface + store_id = interface._get_store_id(sid) + if isinstance(interface, store.MergingRedisSessionInterface): + raw = interface.client.get(store_id) + return None if raw is None else interface.serializer.decode(raw) + return interface.cache.get(store_id) + + +def create_session(app, **values): + session = open_request(app) + session.update(values) + save(app, session) + return session.sid + + +def test_install_replaces_both_backends(): + """Session(app) is followed by the merging interface for Redis and the filesystem.""" + print("๐Ÿ” Testing merging interface installation...") + assert_app_version_at_least("0.261.302") + client = FakeRedis() + redis_app = build_redis_app(client) + assert isinstance(redis_app.session_interface, store.MergingRedisSessionInterface) + assert redis_app.session_interface.client is client + assert store.install_merging_session_interface(redis_app) is redis_app.session_interface + with tempfile.TemporaryDirectory() as directory: + file_app = build_filesystem_app(directory) + assert isinstance(file_app.session_interface, store.MergingFileSystemSessionInterface) + print("โœ… Merging interfaces installed.") + return True + + +def test_slow_request_does_not_erase_concurrent_write(): + """The production failure: a bootstrap that started first must not erase the sign-in flow.""" + print("๐Ÿ” Testing a slow request against a concurrent write...") + app = build_redis_app() + sid = create_session(app, user={"oid": "user-a"}, last_activity_epoch=100) + + bootstrap = open_request(app, sid) + connect = open_request(app, sid) + connect["m365_chat_auth_flow"] = {"state": "m365-chat-abc", "expires_at": "later"} + save(app, connect) + + # The bootstrap read the session before the connect call saved and finishes after it. + save(app, bootstrap) + after_unchanged = stored(app, sid) + assert after_unchanged["m365_chat_auth_flow"]["state"] == "m365-chat-abc" + + bootstrap_writer = open_request(app, sid) + stale = open_request(app, sid) + bootstrap_writer["m365_chat_auth_flow"] = {"state": "m365-chat-def", "expires_at": "later"} + save(app, bootstrap_writer) + stale["last_activity_epoch"] = 200 + save(app, stale) + merged = stored(app, sid) + assert merged["m365_chat_auth_flow"]["state"] == "m365-chat-def" + assert merged["last_activity_epoch"] == 200 + assert merged["user"] == {"oid": "user-a"} + print("โœ… Concurrent write survived.") + return True + + +def test_unchanged_request_only_refreshes_expiry(): + print("๐Ÿ” Testing that an unchanged session is not rewritten...") + client = FakeRedis() + app = build_redis_app(client) + sid = create_session(app, user={"oid": "user-a"}) + store_id = app.session_interface._get_store_id(sid) + version_before = client.versions[store_id] + + session = open_request(app, sid) + assert session.get("user") == {"oid": "user-a"} + save(app, session) + + assert client.versions[store_id] == version_before + assert client.expire_calls[-1] == (store_id, 7200) + print("โœ… Only the expiry was refreshed.") + return True + + +def test_session_deleted_mid_request_is_not_recreated(): + print("๐Ÿ” Testing that a signed-out session stays signed out...") + app = build_redis_app() + sid = create_session(app, user={"oid": "user-a"}, token_cache="cache") + + long_request = open_request(app, sid) + idle_request = open_request(app, sid) + logout = open_request(app, sid) + logout.clear() + save(app, logout) + assert stored(app, sid) is None + + long_request["last_activity_epoch"] = 300 + save(app, long_request) + save(app, idle_request) + assert stored(app, sid) is None + print("โœ… Deleted session was not recreated.") + return True + + +def test_removals_keep_concurrent_additions(): + print("๐Ÿ” Testing removals alongside concurrent additions...") + app = build_redis_app() + sid = create_session(app, user={"oid": "user-a"}, m365_workflow_oauth_binding="binding") + + callback = open_request(app, sid) + other = open_request(app, sid) + other["m365_csrf_token"] = "csrf" + save(app, other) + callback.pop("m365_workflow_oauth_binding") + save(app, callback) + + result = stored(app, sid) + assert "m365_workflow_oauth_binding" not in result + assert result["m365_csrf_token"] == "csrf" + assert result["user"] == {"oid": "user-a"} + print("โœ… Removal applied without dropping the concurrent key.") + return True + + +def test_same_key_is_last_writer_wins(): + app = build_redis_app() + sid = create_session(app, user={"oid": "user-a"}, token_cache="v0") + first = open_request(app, sid) + second = open_request(app, sid) + first["token_cache"] = "v1" + second["token_cache"] = "v2" + save(app, first) + save(app, second) + assert stored(app, sid)["token_cache"] == "v2" + return True + + +def test_nested_in_place_change_is_saved(): + """A nested value changed in place is still detected and written.""" + app = build_redis_app() + sid = create_session(app, user={"oid": "user-a", "roles": ["User"]}) + session = open_request(app, sid) + session["user"]["roles"].append("Admin") + save(app, session) + assert stored(app, sid)["user"]["roles"] == ["User", "Admin"] + return True + + +def test_watch_conflict_retries_and_merges(): + print("๐Ÿ” Testing optimistic-lock retry...") + client = FakeRedis() + app = build_redis_app(client) + sid = create_session(app, user={"oid": "user-a"}) + store_id = app.session_interface._get_store_id(sid) + session = open_request(app, sid) + session["a"] = 1 + + def concurrent_write(): + client.before_execute = None + current = app.session_interface.serializer.decode(client.get(store_id)) + current["b"] = 2 + client.set(store_id, app.session_interface.serializer.encode(current), ex=7200) + + client.before_execute = concurrent_write + save(app, session) + + result = stored(app, sid) + assert result["a"] == 1 and result["b"] == 2 + assert client.transactions == 2 + print("โœ… Retried once and merged both writes.") + return True + + +def test_contention_falls_back_to_lock_free_merge(): + client = FakeRedis() + app = build_redis_app(client) + sid = create_session(app, user={"oid": "user-a"}) + store_id = app.session_interface._get_store_id(sid) + session = open_request(app, sid) + session["mine"] = "kept" + counter = {"writes": 0} + + def always_conflict(): + counter["writes"] += 1 + current = app.session_interface.serializer.decode(client.get(store_id)) + current[f"other_{counter['writes']}"] = counter["writes"] + client.set(store_id, app.session_interface.serializer.encode(current), ex=7200) + + client.before_execute = always_conflict + save(app, session) + client.before_execute = None + + result = stored(app, sid) + assert counter["writes"] == store.MERGE_ATTEMPTS + assert result["mine"] == "kept" + assert all(result[f"other_{index}"] == index for index in range(1, store.MERGE_ATTEMPTS + 1)) + return True + + +def test_new_and_cleared_sessions_are_written_whole(): + app = build_redis_app() + sid = create_session(app, user={"oid": "user-a"}, stale="value") + replacing = open_request(app, sid) + concurrent = open_request(app, sid) + concurrent["added_meanwhile"] = True + save(app, concurrent) + replacing.clear() + replacing["user"] = {"oid": "user-a", "fresh": True} + save(app, replacing) + result = stored(app, sid) + assert result["user"] == {"oid": "user-a", "fresh": True} + assert "stale" not in result and "added_meanwhile" not in result + return True + + +def test_unreadable_stored_session_is_written_whole(): + client = FakeRedis() + app = build_redis_app(client) + sid = create_session(app, user={"oid": "user-a"}) + store_id = app.session_interface._get_store_id(sid) + session = open_request(app, sid) + session["after"] = True + client.data[store_id] = b"\x00not-a-session" + save(app, session) + result = stored(app, sid) + assert result["user"] == {"oid": "user-a"} and result["after"] is True + return True + + +def test_filesystem_backend_merges_and_respects_deletion(): + print("๐Ÿ” Testing the filesystem backend...") + with tempfile.TemporaryDirectory() as directory: + app = build_filesystem_app(directory) + sid = create_session(app, user={"oid": "user-a"}) + + slow = open_request(app, sid) + fast = open_request(app, sid) + fast["m365_chat_auth_flow"] = {"state": "m365-chat-xyz"} + save(app, fast) + slow["last_activity_epoch"] = 5 + save(app, slow) + result = stored(app, sid) + assert result["m365_chat_auth_flow"] == {"state": "m365-chat-xyz"} + assert result["last_activity_epoch"] == 5 + + late = open_request(app, sid) + logout = open_request(app, sid) + logout.clear() + save(app, logout) + late["after_logout"] = True + save(app, late) + assert stored(app, sid) is None + print("โœ… Filesystem backend merged and stayed signed out.") + return True + + +def _flask_requests_round_trip(): + app = build_redis_app() + + @app.route("/set//") + def set_value(key, value): + flask.session[key] = value + return "ok" + + @app.route("/pop/") + def pop_value(key): + flask.session.pop(key, None) + return "ok" + + @app.route("/read") + def read_values(): + return flask.jsonify({key: value for key, value in flask.session.items() if key != "_permanent"}) + + client = app.test_client() + assert client.get("/set/user/alice").status_code == 200 + assert client.get("/set/flow/one").status_code == 200 + assert client.get("/pop/flow").status_code == 200 + assert client.get("/read").get_json() == {"user": "alice"} + with client.session_transaction() as session: + session["transaction"] = "yes" + assert client.get("/read").get_json() == {"user": "alice", "transaction": "yes"} + return True + + +def test_flask_requests_round_trip(): + """Ordinary requests and the test client's session_transaction still work.""" + # Flask 2.x test clients read werkzeug.__version__, which Werkzeug 3 no longer defines. + with patch.object(werkzeug, "__version__", package_version("werkzeug"), create=True): + return _flask_requests_round_trip() + + +def test_app_installs_merging_interface_after_every_session_init(): + source = (APP_DIR / "app.py").read_text(encoding="utf-8") + tree = ast.parse(source) + + def is_call(statement, name): + return ( + isinstance(statement, ast.Expr) and isinstance(statement.value, ast.Call) + and isinstance(statement.value.func, ast.Name) and statement.value.func.id == name + ) + + session_inits = 0 + for node in ast.walk(tree): + body = getattr(node, "body", None) + if not isinstance(body, list): + continue + for index, statement in enumerate(body): + if is_call(statement, "Session"): + session_inits += 1 + assert index + 1 < len(body) and is_call(body[index + 1], "install_merging_session_interface"), ( + f"Session(app) on line {statement.lineno} is not followed by install_merging_session_interface(app)." + ) + assert session_inits >= 2 + assert "from functions_session_store import install_merging_session_interface" in source + return True + + +if __name__ == "__main__": + tests = [ + test_install_replaces_both_backends, + test_slow_request_does_not_erase_concurrent_write, + test_unchanged_request_only_refreshes_expiry, + test_session_deleted_mid_request_is_not_recreated, + test_removals_keep_concurrent_additions, + test_same_key_is_last_writer_wins, + test_nested_in_place_change_is_saved, + test_watch_conflict_retries_and_merges, + test_contention_falls_back_to_lock_free_merge, + test_new_and_cleared_sessions_are_written_whole, + test_unreadable_stored_session_is_written_whole, + test_filesystem_backend_merges_and_respects_deletion, + test_flask_requests_round_trip, + test_app_installs_merging_interface_after_every_session_init, + ] + results = [] + for test in tests: + print(f"\n๐Ÿงช Running {test.__name__}...") + try: + results.append(test() is not False) + except Exception as exc: + import traceback + print(f"โŒ {test.__name__} failed: {exc}") + traceback.print_exc() + results.append(False) + print(f"\n๐Ÿ“Š Results: {sum(results)}/{len(results)} tests passed") + sys.exit(0 if all(results) else 1) diff --git a/functional_tests/test_v2_user_settings_memory_m365.py b/functional_tests/test_v2_user_settings_memory_m365.py index 688a71109..378310dc3 100644 --- a/functional_tests/test_v2_user_settings_memory_m365.py +++ b/functional_tests/test_v2_user_settings_memory_m365.py @@ -2,9 +2,10 @@ # test_v2_user_settings_memory_m365.py """ Functional test for V2 User Settings fact memory and Microsoft 365 parity. -Version: 0.261.281 +Version: 0.261.302 Implemented in: 0.261.279 Authorization URL validation updated in: 0.261.281 +Popup workflow connect and V2-only links updated in: 0.261.302 V2 Preferences now carries the classic profile page's Fact Memory section, as a workbench for adding, searching, editing and deleting memories, and its Microsoft 365 sharing and @@ -147,13 +148,17 @@ def test_m365_cards_send_and_refresh_the_csrf_token(): "'/api/m365/preferences'", "'/api/m365/chat/connection'", "'/api/m365/connections'", - "'/api/m365/connections/connect'", "'/api/m365/connections/disconnect'", "`/api/m365/sources/${encodeURIComponent(source)}/revoke`", "`/api/m365/bindings/${encodeURIComponent(binding.id)}/revoke`", "`/api/m365/bindings?${query}`", ): assert path in source, f"M365 cards no longer call {path}" + # Connecting runs in the shared popup helper, which sends the token it just read. + connect_source = _read(M365_CONNECT_TS) + for path in ("'/api/m365/chat/connection/connect'", "'/api/m365/connections/connect'"): + assert path in connect_source, f"The connect helper no longer calls {path}" + assert connect_source.count("headers: { 'X-M365-CSRF-Token': requireCsrf(current.csrf_token) }") == 2 def test_m365_cards_mirror_classic_choices(): @@ -182,13 +187,39 @@ def test_m365_revocations_are_confirmed_and_redirects_validated(): assert "onRevoke({" in preceding, f"{path} is not behind the confirmation dialog" assert " None: assert found[RUN_LINKS.name] == ['workflowRunHref(item.run.workflowId, item.run.runId)'] builder_import = "import { workflowRunHref } from '../../lib/workflowRunLink';" - constant_import = "import { M365_CONNECT_HREF } from '../../lib/m365Links';" + constant_import = re.compile( + r"^import \{[^}]*\bM365_CONNECT_HREF\b[^}]*\} from '\.\./\.\./lib/m365Links';$", re.MULTILINE, + ) for path in (RUN_CARD, DELIVERY_FOOTER, PROPOSAL_RUN_SUMMARY, RUN_LINKS): assert builder_import in read_text(path), f'{path.name} does not import workflowRunHref' # The proposal card shares the run card's reconnect path instead of keeping its own copy. for path in (RUN_CARD, PROPOSAL_CARD): source = read_text(path) - assert constant_import in source, f'{path.name} does not import M365_CONNECT_HREF' + assert constant_import.search(source), f'{path.name} does not import M365_CONNECT_HREF' assert 'const M365_CONNECT_HREF' not in source, f'{path.name} defines its own M365_CONNECT_HREF' + assert 'const M365_APPROVALS_HREF' not in source, f'{path.name} defines its own M365_APPROVALS_HREF' def test_fixed_link_values_stay_same_origin() -> None: - """The constant the checker trusts by name is a fixed same-origin path, and notices reuse the builder.""" + """The constants the checker trusts by name are fixed V2 router paths, and notices reuse the builder.""" m365_source = read_text(M365_LINKS) - assert "export const M365_CONNECT_HREF = '/profile?tab=settings#m365-connection-status';" in m365_source - assert m365_source.count('export ') == 1 + exported = dict(re.findall(r"^export const (M365_[A-Z_]+_HREF) = '([^']*)';$", m365_source, re.MULTILINE)) + # V2 routes under the /v2 basename, so a Microsoft 365 link never leaves V2 for a classic page. + assert exported == { + 'M365_CHAT_CONNECTION_HREF': '/settings?tab=preferences§ion=m365-chat-connection', + 'M365_CONNECT_HREF': '/settings?tab=preferences§ion=m365-workflow-connection', + 'M365_APPROVALS_HREF': '/approvals/m365', + } + assert m365_source.count('export ') == len(exported) notification_source = read_text(NOTIFICATION_LINKS) assert "import { workflowRunHref } from './workflowRunLink';" in notification_source diff --git a/ui_tests/fixtures/orchestration/harness_entry.tsx b/ui_tests/fixtures/orchestration/harness_entry.tsx index d950e00c9..dd46f3826 100644 --- a/ui_tests/fixtures/orchestration/harness_entry.tsx +++ b/ui_tests/fixtures/orchestration/harness_entry.tsx @@ -42,6 +42,7 @@ import { ChatPage } from '../../../application/v2_ui/src/pages/ChatPage'; import { HomePage } from '../../../application/v2_ui/src/pages/HomePage'; import { ModelCatalogManager } from '../../../application/v2_ui/src/components/admin/ModelCatalogManager'; import { OrchestrationPlannerModelPicker } from '../../../application/v2_ui/src/components/admin/OrchestrationPlannerModelPicker'; +import { M365Cards } from '../../../application/v2_ui/src/components/settings/M365Cards'; import { PLANNER_MODEL_KEYS } from '../../../application/v2_ui/src/lib/orchestrationPlannerModel'; import type { AdminField } from '../../../application/v2_ui/src/lib/adminFields'; @@ -171,6 +172,7 @@ type ComponentName = | 'OrchestrationPlanEditorHost' | 'ContextWorkflow' | 'ParticipantsPanel' + | 'M365Cards' | 'Toaster'; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -193,6 +195,7 @@ const components: Record ReactElement | null> = { OrchestrationPlanEditorHost, ContextWorkflow, ParticipantsPanel, + M365Cards, Toaster, }; diff --git a/ui_tests/test_m365_connection_result_page.py b/ui_tests/test_m365_connection_result_page.py new file mode 100644 index 000000000..2e246592e --- /dev/null +++ b/ui_tests/test_m365_connection_result_page.py @@ -0,0 +1,229 @@ +# test_m365_connection_result_page.py +""" +UI test for the Microsoft 365 sign-in result page. +Version: 0.261.302 +Implemented in: 0.261.302 + +The sign-in callbacks used to answer a failed sign-in with raw JSON, which the V2 popup showed +as {"error":"m365_auth_state_invalid",...}. They now render templates/m365_connection_result.html. +In a popup the page reports the outcome to the same-origin window that opened it and closes; on a +full page it continues to the fixed settings page after success, and shows the error, as text, +with a link back after a failure. + +The page is rendered by the real route helper, route_backend_m365._connection_result_page, with +the real template, Bootstrap and the page's local script, in local Chromium with same-origin +fixtures. No server, Microsoft sign-in or network is used. +Run: python -m pytest .\\ui_tests\\test_m365_connection_result_page.py -q +""" + +import functools +import importlib.util +import mimetypes +import sys +import types +from pathlib import Path +from unittest.mock import patch +from urllib.parse import unquote, urlsplit + +import pytest +from flask import Flask +from playwright.sync_api import expect + + +APP_ROOT = Path(__file__).resolve().parents[1] / "application" / "single_app" +ORIGIN = "https://simplechat.test" +CLASSIC_CHAT_CONNECTION_URL = "/profile?tab=settings#m365-chat-connection" +CLASSIC_WORKFLOW_CONNECTED_URL = "/profile?tab=settings&m365_connection=connected#m365-connection-status" +UNSAFE_MESSAGE = 'Consent was not completed. ' + +pytestmark = pytest.mark.ui + + +def _module_stub(name, **values): + module = types.ModuleType(name) + module.__dict__.update(values) + return module + + +@functools.lru_cache(maxsize=1) +def _routes_module(): + """The real route module, with its authentication and logging seams stubbed.""" + if str(APP_ROOT) not in sys.path: + sys.path.insert(0, str(APP_ROOT)) + spec = importlib.util.spec_from_file_location("m365_result_page_routes", APP_ROOT / "route_backend_m365.py") + module = importlib.util.module_from_spec(spec) + stubs = { + "functions_appinsights": _module_stub("functions_appinsights", log_event=lambda *args, **kwargs: None), + "functions_authentication": _module_stub( + "functions_authentication", login_required=lambda function: function, + user_required=lambda function: function, user_required_blueprint=lambda: (lambda: None), + ), + "swagger_wrapper": _module_stub( + "swagger_wrapper", swagger_route=lambda **kwargs: (lambda function: function), + get_auth_security=lambda: [], + ), + } + with patch.dict(sys.modules, stubs): + spec.loader.exec_module(module) + return module + + +def render_result_page(**options): + """The callback's result page exactly as route_backend_m365 renders it.""" + app = Flask(__name__, template_folder=str(APP_ROOT / "templates"), static_folder=str(APP_ROOT / "static")) + with app.test_request_context("/getAToken"): + response = _routes_module()._connection_result_page(**options) + return response.status_code, response.get_data(as_text=True) + + +def serve_static(route, path): + """Serve a file from the application's static folder, refusing anything outside it.""" + root = (APP_ROOT / "static").resolve() + asset = (root / unquote(path[len("/static/"):])).resolve() + if not asset.is_relative_to(root) or not asset.is_file(): + route.fulfill(status=404, body="") + return + route.fulfill(body=asset.read_bytes(), content_type=mimetypes.guess_type(asset.name)[0] or "application/octet-stream") + + +OPENER_HTML = """Opener + +""" + + +class ResultFixture: + def __init__(self): + self.result_html = "" + self.unexpected = [] + + def handle(self, route): + parsed = urlsplit(route.request.url) + path = parsed.path + if f"{parsed.scheme}://{parsed.netloc}" != ORIGIN: + self.unexpected.append(route.request.url) + route.abort() + return + if path == "/opener": + route.fulfill(content_type="text/html", body=OPENER_HTML) + elif path == "/getAToken": + route.fulfill(content_type="text/html", body=self.result_html) + elif path == "/profile": + route.fulfill(content_type="text/html", body="Profile

Profile fixture

") + elif path.startswith("/static/"): + serve_static(route, path) + else: + self.unexpected.append(route.request.url) + route.fulfill(status=404, body="") + + +@pytest.fixture +def result_ui(playwright): + browser = playwright.chromium.launch(headless=True) + context = browser.new_context(viewport={"width": 1280, "height": 900}) + fixture = ResultFixture() + errors = [] + context.route("**/*", fixture.handle) + page = context.new_page() + page.on("pageerror", lambda error: errors.append(str(error))) + try: + yield page, fixture + finally: + context.close() + browser.close() + assert not fixture.unexpected, fixture.unexpected + assert not errors, errors + + +def open_result_popup(page): + page.goto(f"{ORIGIN}/opener") + with page.expect_popup() as opened: + page.locator("#open").click() + popup = opened.value + if not popup.is_closed(): + popup.wait_for_event("close") + return page.evaluate("window.received") + + +def test_popup_reports_a_workflow_connection_to_its_opener_and_closes(result_ui): + page, fixture = result_ui + status, fixture.result_html = render_result_page( + kind="workflow", outcome="connected", completion="popup", continue_url=CLASSIC_WORKFLOW_CONNECTED_URL, + ) + assert status == 200 + received = open_result_popup(page) + assert received == [{ + "origin": ORIGIN, + "data": {"type": "m365-workflow-connected", "kind": "workflow", "outcome": "connected"}, + }] + + +def test_popup_reports_a_failure_with_its_code_and_message_as_text(result_ui): + page, fixture = result_ui + status, fixture.result_html = render_result_page( + kind="chat", outcome="failed", completion="popup", status=400, + code="m365_consent_required", message=UNSAFE_MESSAGE, continue_url=CLASSIC_CHAT_CONNECTION_URL, + ) + assert status == 400 + received = open_result_popup(page) + assert received == [{ + "origin": ORIGIN, + "data": { + "type": "m365-connect-failed", "kind": "chat", "outcome": "failed", + "code": "m365_consent_required", "message": UNSAFE_MESSAGE, + }, + }] + assert page.evaluate("window.injected === undefined") + + +@pytest.mark.parametrize("viewport", [{"width": 1280, "height": 900}, {"width": 390, "height": 844}]) +def test_failure_without_an_opener_stays_readable_with_a_link_back(result_ui, viewport): + page, fixture = result_ui + page.set_viewport_size(viewport) + _status, fixture.result_html = render_result_page( + kind="chat", outcome="failed", completion="auto", status=400, + code="m365_auth_state_invalid", message=UNSAFE_MESSAGE, continue_url=CLASSIC_CHAT_CONNECTION_URL, + ) + page.goto(f"{ORIGIN}/getAToken?code=ui&state=fixture") + expect(page.get_by_role("heading", name="Microsoft 365 was not connected")).to_be_visible() + expect(page.get_by_role("status")).to_have_text(UNSAFE_MESSAGE) + expect(page.locator("#m365-connection-result img")).to_have_count(0) + link = page.get_by_role("link", name="Return to settings") + expect(link).to_have_attribute("href", CLASSIC_CHAT_CONNECTION_URL) + expect(page.get_by_role("button", name="Close window")).to_be_hidden() + assert urlsplit(page.url).path == "/getAToken" + layout = page.evaluate("""() => ({ + content: document.documentElement.scrollWidth, viewport: window.innerWidth, + })""") + assert layout["content"] <= layout["viewport"] + assert page.evaluate("window.injected === undefined") + + +def test_full_page_success_continues_to_the_fixed_settings_page(result_ui): + page, fixture = result_ui + _status, fixture.result_html = render_result_page( + kind="workflow", outcome="connected", completion="auto", continue_url=CLASSIC_WORKFLOW_CONNECTED_URL, + ) + page.goto(f"{ORIGIN}/getAToken?code=ui&state=fixture") + expect(page.get_by_role("heading", name="Profile fixture")).to_be_visible() + returned = urlsplit(page.url) + assert (returned.path, returned.query, returned.fragment) == ( + "/profile", "tab=settings&m365_connection=connected", "m365-connection-status", + ) + + +def test_popup_that_lost_its_opener_offers_to_close_instead_of_navigating(result_ui): + page, fixture = result_ui + _status, fixture.result_html = render_result_page( + kind="chat", outcome="connected", completion="popup", continue_url=CLASSIC_CHAT_CONNECTION_URL, + ) + page.goto(f"{ORIGIN}/getAToken?code=ui&state=fixture") + expect(page.get_by_role("heading", name="Microsoft 365 is connected")).to_be_visible() + expect(page.get_by_role("button", name="Close window")).to_be_visible() + expect(page.locator("#m365-connection-result-closing")).to_be_hidden() + assert urlsplit(page.url).path == "/getAToken" diff --git a/ui_tests/test_m365_lifecycle_and_approvals.py b/ui_tests/test_m365_lifecycle_and_approvals.py index 62792d4f2..60c25e265 100644 --- a/ui_tests/test_m365_lifecycle_and_approvals.py +++ b/ui_tests/test_m365_lifecycle_and_approvals.py @@ -992,9 +992,9 @@ def test_workflow_delivery_controls_follow_the_run_as_viewer(ui): @pytest.mark.ui @pytest.mark.parametrize("viewport", [{"width": 1440, "height": 900}, {"width": 390, "height": 844}]) @pytest.mark.parametrize("status,sources,message", [ - ("available", list(SOURCES), "Sign-in saved for this session"), - ("not_connected", [], "No Microsoft 365 sign-in is saved for this session"), - ("reconnect_required", ["email"], "Reconnect Microsoft 365 before using these sources in chat"), + ("available", list(SOURCES), "Signed in to Microsoft 365 for this session. Chat can use: Calendar, Email"), + ("not_connected", [], "Not signed in to Microsoft 365 in this session"), + ("reconnect_required", ["email"], "Microsoft 365 needs you to sign in again before chat can use Email"), ]) def test_profile_chat_reconnect_is_available_without_a_pending_request(ui, viewport, status, sources, message): page, api = ui @@ -1004,7 +1004,7 @@ def test_profile_chat_reconnect_is_available_without_a_pending_request(ui, viewp region = page.get_by_role("region", name="Microsoft 365 chat connection", exact=True) expect(region.get_by_role("button", name="Reconnect Microsoft 365 for chat", exact=True)).to_be_enabled() expect(region.locator("#m365-chat-connection-status")).to_contain_text(message) - expect(region).to_contain_text("Access is checked when a source runs") + expect(region).to_contain_text("Microsoft still checks your access each time a source runs") expect(region).to_contain_text("does not require Key Vault") expect(region).to_contain_text("sharing approvals") expect(region).to_contain_text("saved workflow credentials") @@ -1226,7 +1226,7 @@ def test_profile_chat_invalid_status_is_visible_without_blocking_workflow_contro api.chat_connection = {"status": "not_connected", "sources": []} page.locator("#m365-profile-refresh").click() expect(page.locator("#m365-chat-connect-btn")).to_be_enabled() - expect(page.locator("#m365-chat-connection-status")).to_contain_text("No Microsoft 365 sign-in is saved") + expect(page.locator("#m365-chat-connection-status")).to_contain_text("Not signed in to Microsoft 365 in this session") assert not api.m365_posts assert not api.errors @@ -1281,7 +1281,7 @@ def test_profile_chat_callback_reports_success_preserves_navigation_and_never_re expect(notice).to_contain_text("Microsoft 365 sign-in completed") expect(notice).to_contain_text("retry your original question") expect(notice).to_contain_text("No past requests were retried") - expect(page.locator("#m365-chat-connection-status")).to_contain_text("Sign-in saved for this session") + expect(page.locator("#m365-chat-connection-status")).to_contain_text("Signed in to Microsoft 365 for this session") expect(page.locator("#m365-chat-connect-btn")).to_be_enabled() returned_url = urlsplit(page.url) assert returned_url.path == "/profile" diff --git a/ui_tests/test_m365_pending_action_cards.py b/ui_tests/test_m365_pending_action_cards.py index 9f8a3767a..83db2b5e9 100644 --- a/ui_tests/test_m365_pending_action_cards.py +++ b/ui_tests/test_m365_pending_action_cards.py @@ -977,7 +977,7 @@ def test_expired_sign_in_can_cancel_or_reconnect_without_resuming_agent(pending_ expect(card(page)).to_contain_text("Current server status loaded") expect(card(page).get_by_role("button", name="Send", exact=True)).to_be_enabled() assert len(pending.writes) == 1 - assert api.profile_chat_connect_requests == [{"sources": ["calendar"]}] + assert api.profile_chat_connect_requests == [{"sources": ["calendar"], "completion": "popup"}] card(page).get_by_role("button", name="Send", exact=True).click() expect(card(page)).to_contain_text("Sent โ€” calendar invitation created") assert len(pending.writes) == 2 @@ -1041,7 +1041,7 @@ def test_reconnect_acknowledgement_survives_stored_error_echoes_but_not_new_revi expect(card(page).get_by_role("button", name="Reconnect Microsoft 365", exact=True)).to_be_visible() assert not pending.writes assert len(api.chat_requests) == 1 - assert api.profile_chat_connect_requests == [{"sources": ["calendar"]}] + assert api.profile_chat_connect_requests == [{"sources": ["calendar"], "completion": "popup"}] assert not api.resume_requests assert not api.errors diff --git a/ui_tests/test_v2_orchestration_m365_recovery.py b/ui_tests/test_v2_orchestration_m365_recovery.py index f211a9268..2a37bc338 100644 --- a/ui_tests/test_v2_orchestration_m365_recovery.py +++ b/ui_tests/test_v2_orchestration_m365_recovery.py @@ -1,14 +1,17 @@ # test_v2_orchestration_m365_recovery.py """ Real-component browser coverage for plan steps that stopped for Microsoft 365. -Version: 0.261.238 +Version: 0.261.302 Implemented in: 0.261.238 A step that stopped for Microsoft 365 sign-in offers Connect Microsoft 365 in its run's recovery notice, and a step that stopped for approval links to Approvals. The production notice, connect flow, controller, stores, and message list run in Chromium. Only HTTP responses are deterministic, including the sign-in window's last page: a stand-in for the -Profile page, which either tells its opener it connected or closes without saying so. +/getAToken result page, which tells its opener it connected or failed, or closes without +saying so. Since 0.261.302 the sign-in asks for a popup completion, a failure reported by the +result page is shown in the notice, and the notice's links stay in V2: Settings for the chat +connection and V2 Approvals, never the classic Profile or Approvals pages. Build CSS with the existing V2 build, keeping outputs in UI test artifacts: npm --prefix .\\application\\v2_ui run build -- --outDir ..\\..\\ui_tests\\artifacts\\orchestration-plan-editor @@ -41,10 +44,9 @@ CONNECTED = "Microsoft 365 is connected. Select Retry from failed step to continue." NOT_COMPLETED = "Microsoft 365 sign-in was not completed. Connect again when you are ready." CSRF_TOKEN = "m365-recovery-test-csrf-token-000000000001" -PROFILE_PATH = "/profile" -SIGN_IN_URL = ( - "https://simplechat.test/profile?tab=settings&m365_chat_connection=connected#m365-chat-connection" -) +RESULT_PATH = "/getAToken" +SIGN_IN_URL = "https://simplechat.test/getAToken?code=ui-recovery-code&state=m365-chat-ui-recovery-state" +FAILED_MESSAGE = "Microsoft 365 sign-in or consent was not completed." EARLIER = "2026-09-01T08:00:00+00:00" LATER = "2026-09-08T12:02:00+00:00" @@ -88,24 +90,31 @@ def m365_ui(editor_browser, editor_assets): assert not api.errors, api.errors -def sign_in_window(page, api, *, reports, saves=True): +def sign_in_window(page, api, *, reports, saves=True, fails=False): """Serve the sign-in window's last page, which only the popup ever requests. - ``reports`` posts the Profile page's message to the opener. Without it the window - closes without a word, as when the message is lost; ``saves`` decides whether the - sign-in was saved first. + ``reports`` posts the result page's message to the opener: connected, or failed with the + server's message when ``fails``. Without it the window closes without a word, as when the + message is lost; ``saves`` decides whether the sign-in was saved first. """ def finish(route): - if urlsplit(route.request.url).path != PROFILE_PATH: + if urlsplit(route.request.url).path != RESULT_PATH: route.fulfill(status=204) return - if saves: + if saves and not fails: api.connection = {"status": "available", "sources": ["calendar", "email"], "connected_at": LATER} + if fails: + message = ( + "{ type: 'm365-connect-failed', kind: 'chat', outcome: 'failed', " + f"code: 'm365_consent_required', message: '{FAILED_MESSAGE}' }}" + ) + else: + message = "{ type: 'm365-profile-reconnected', kind: 'chat', outcome: 'connected' }" script = ( - "window.opener.postMessage({ type: 'm365-profile-reconnected' }, window.location.origin);" + f"window.opener.postMessage({message}, window.location.origin);" if reports else "window.close();" ) - route.fulfill(content_type="text/html", body=f"Profile") + route.fulfill(content_type="text/html", body=f"Result") # Context routes reach the popup; the page's own route still answers the chat page first. page.context.route("**/*", finish) @@ -139,7 +148,7 @@ def test_sign_in_stop_connects_only_known_sources_then_waits_for_retry(m365_ui): expect(notice.get_by_role("status")).to_have_text(CONNECTED) wait_closed(opened.value) - assert api.connects == [{"body": {"sources": ["email"]}, "csrf": CSRF_TOKEN}] + assert api.connects == [{"body": {"sources": ["email"], "completion": "popup"}, "csrf": CSRF_TOKEN}] expect(notice.get_by_role("button", name="Connect Microsoft 365")).to_have_count(0) expect(page.get_by_role("button", name="Retry from failed step").first).to_be_enabled() # Connecting saves the sign-in only; the user retries the failed step. @@ -162,7 +171,7 @@ def test_closed_sign_in_window_without_a_saved_sign_in_can_connect_again(m365_ui def test_saved_sign_in_is_confirmed_when_the_window_closes_without_reporting(m365_ui): - # A Profile page on another origin can't message the chat page, so the saved + # A result page on another origin can't message the chat page, so the saved # connection confirms the sign-in once the window closes. page, api = m365_ui api.failure = {"code": "m365_sign_in_required", "message": SIGN_IN_MESSAGE, "m365_sources": ["email"]} @@ -176,22 +185,37 @@ def test_saved_sign_in_is_confirmed_when_the_window_closes_without_reporting(m36 expect(notice.get_by_role("alert")).to_have_count(0) -def test_sign_in_stop_without_sources_links_to_profile(m365_ui): +def test_failed_sign_in_shows_the_result_pages_message_and_can_connect_again(m365_ui): + page, api = m365_ui + api.failure = {"code": "m365_sign_in_required", "message": SIGN_IN_MESSAGE, "m365_sources": ["email"]} + sign_in_window(page, api, reports=True, fails=True) + notice = start(page, api, SIGN_IN_MESSAGE) + + with page.expect_popup() as opened: + notice.get_by_role("button", name="Connect Microsoft 365").click() + wait_closed(opened.value) + expect(notice.get_by_role("alert")).to_have_text(FAILED_MESSAGE) + expect(notice.get_by_role("button", name="Connect Microsoft 365")).to_be_enabled() + expect(notice.get_by_role("status")).to_have_count(0) + assert len(api.connects) == 1 and not api.calls("/retry") + + +def test_sign_in_stop_without_sources_links_to_v2_settings(m365_ui): page, api = m365_ui api.failure = {"code": "m365_sign_in_required", "message": SIGN_IN_MESSAGE} notice = start(page, api, SIGN_IN_MESSAGE) - expect(notice.get_by_role("link", name="Connect Microsoft 365 in Profile settings")).to_have_attribute( - "href", "/profile?tab=settings#m365-chat-connection", + expect(notice.get_by_role("link", name="Connect Microsoft 365 in Settings")).to_have_attribute( + "href", "/settings?tab=preferences§ion=m365-chat-connection", ) expect(notice.get_by_role("button", name="Connect Microsoft 365")).to_have_count(0) -def test_approval_stop_links_to_approvals_without_connecting(m365_ui): +def test_approval_stop_links_to_v2_approvals_without_connecting(m365_ui): page, api = m365_ui api.failure = {"code": "m365_approval_required", "message": APPROVAL_MESSAGE, "m365_sources": ["onedrive"]} notice = start(page, api, APPROVAL_MESSAGE) expect(notice.get_by_role("link", name="Review the Microsoft 365 approval")).to_have_attribute( - "href", "/approvals", + "href", "/approvals/m365", ) expect(notice.get_by_role("button", name="Connect Microsoft 365")).to_have_count(0) assert not api.connects diff --git a/ui_tests/test_v2_orchestration_workflow_proposal_card.py b/ui_tests/test_v2_orchestration_workflow_proposal_card.py index 11937fd74..9e40e4c6d 100644 --- a/ui_tests/test_v2_orchestration_workflow_proposal_card.py +++ b/ui_tests/test_v2_orchestration_workflow_proposal_card.py @@ -1,9 +1,10 @@ # test_v2_orchestration_workflow_proposal_card.py """ Real-component browser tests for the workflow proposal card under an orchestration answer. -Version: 0.261.251 +Version: 0.261.302 Implemented in: 0.261.207; merge task wording added in 0.261.241; merge kinds in 0.261.242; Word in 0.261.243; PowerPoint in 0.261.244 Next and last run on a created card: 0.261.251 (microsoft/simplechat#1546) +Microsoft 365 links stay in V2 Settings and V2 Approvals: 0.261.302 Refs: microsoft/simplechat#1547, microsoft/simplechat#1619 The production MessageList, WorkflowProposalCards, ConfirmDialog and WorkflowEditorDialog run in @@ -65,7 +66,9 @@ ) HOSTILE_DESCRIPTION = "Reviews the week's email." HOSTILE_TITLE = "Review email" -CONNECT_HREF = "/profile?tab=settings#m365-connection-status" +# V2 router paths (the harness router has no /v2 basename), never the classic Profile or Approvals. +CONNECT_HREF = "/settings?tab=preferences§ion=m365-workflow-connection" +APPROVALS_HREF = "/approvals/m365" CLOCK_START = datetime(2026, 9, 28, 12, 0, tzinfo=timezone.utc) URL_ACCESS_NOTE = "Create the workflow first, then turn on URL Access in the workflow editor." RUN_AS_SELF = "You. Microsoft 365 steps use your account." @@ -607,6 +610,8 @@ def test_run_as_line_follows_the_servers_approval_state(card_ui, state, approval article = card(page) run_as = article.locator("div:has(> dt:text-is('Run as')) > dd") expect(run_as).to_have_text(expected) + if approval_state == "waiting": + expect(article.get_by_role("link", name="Review Run as approval")).to_have_attribute("href", APPROVALS_HREF) # A revision the requester saved never claims its first run waits for approval. if approval_state == "self_authored": expect(run_as).not_to_contain_text("The first run will wait") diff --git a/ui_tests/test_v2_settings_m365_connections.py b/ui_tests/test_v2_settings_m365_connections.py new file mode 100644 index 000000000..92da9ef4b --- /dev/null +++ b/ui_tests/test_v2_settings_m365_connections.py @@ -0,0 +1,233 @@ +# test_v2_settings_m365_connections.py +""" +Real-component browser tests for the Microsoft 365 connection cards in V2 Settings. +Version: 0.261.302 +Implemented in: 0.261.302 + +The production M365Cards, Toaster and connect flow run in Chromium with the production CSS; +only HTTP responses are deterministic. The sign-in window ends on the real callback result +page, rendered by route_backend_m365 with its local script. These tests pin what the user saw +go wrong: + +- the chat card said "Sources saved for this session: none" while chat was reading mail; it + now names the sources the session can use; +- Connect for workflows failed with a Key Vault error no user can act on; the card now + explains that an administrator has to turn on Key Vault storage and keeps Connect disabled; +- workflow connect navigated the whole page away and came back to classic Profile; it now + signs in through a popup that reports back, so the user stays in V2, and a failure reported + by the result page is shown; +- Open Approvals opened the classic Approvals page; it is now the V2 route. + +Build CSS with the existing V2 build, keeping outputs in UI test artifacts: +npm --prefix .\\application\\v2_ui run build -- --outDir ..\\..\\ui_tests\\artifacts\\orchestration-plan-editor +Run: python -m pytest .\\ui_tests\\test_v2_settings_m365_connections.py -q +""" + +import copy +from urllib.parse import urlsplit + +import pytest +from playwright.sync_api import expect + +from test_m365_connection_result_page import render_result_page, serve_static +from test_v2_orchestration_plan_editor import ( # noqa: F401 + HARNESS, + ORIGIN, + connect_options, + editor_assets, + editor_browser, +) + + +pytestmark = pytest.mark.ui +CSRF_TOKEN = "m365-settings-test-csrf-token-000000000001" +SIGN_IN_URL = f"{ORIGIN}/getAToken?code=ui-settings-code&state=m365-workflow-ui-settings-state" +CLASSIC_WORKFLOW_CONNECTED_URL = "/profile?tab=settings&m365_connection=connected#m365-connection-status" +NOT_READY = ( + "Workflow connections are not set up on this deployment yet: they need Key Vault secret storage. " + "An administrator can turn it on in Admin Settings > Security > Secrets > Key Vault." +) +KEY_FORBIDDEN = ( + "SimpleChat could not create the workflow encryption key because its identity cannot write " + "Key Vault secrets. An administrator must grant it Key Vault Secrets Officer on the vault." +) +PREFERENCES = { + "sources": {"calendar": "ask", "email": "ask", "onedrive": "ask", "spo": "ask"}, + "extended_analysis": {"onedrive": "ask", "spo": "ask"}, +} +CONNECTED = { + "id": "m365-connection-ui", "status": "connected", "tenant_id": "tenant-ui", "cloud": "public", + "account_username": "tester@contoso.test", "sources": ["email"], "authorized_scopes": ["Mail.Read"], + "connected_at": "2026-10-08T15:00:00+00:00", +} + + +class SettingsApi: + """The Microsoft 365 routes the cards call, plus the sign-in window's last page.""" + + def __init__(self, assets): + self.assets = assets + self.chat_connection = {"status": "available", "sources": ["email", "onedrive", "spo"]} + self.workflow_connection = None + self.readiness = {"available": True, "reason": None, "message": ""} + self.sign_in_fails = False + self.connects = [] + self.unexpected = [] + + def handle(self, route): + request = route.request + parsed = urlsplit(request.url) + path = parsed.path + if f"{parsed.scheme}://{parsed.netloc}" != ORIGIN: + self.unexpected.append(request.url) + route.abort() + return + if request.method == "GET" and path in self.assets: + route.fulfill(path=str(self.assets[path])) + return + if path.startswith("/static/"): + serve_static(route, path) + return + if request.method == "GET" and path == "/api/m365/preferences": + route.fulfill(json={"success": True, "preferences": PREFERENCES, "csrf_token": CSRF_TOKEN}) + elif request.method == "GET" and path == "/api/m365/chat/connection": + route.fulfill(json={"success": True, "connection": self.chat_connection, "csrf_token": CSRF_TOKEN}) + elif request.method == "GET" and path == "/api/m365/connections": + route.fulfill(json={ + "success": True, "connection": copy.deepcopy(self.workflow_connection), + "workflow_connections": self.readiness, "csrf_token": CSRF_TOKEN, + }) + elif request.method == "GET" and path == "/api/m365/bindings": + route.fulfill(json={"items": [], "continuation_token": None}) + elif request.method == "POST" and path == "/api/m365/connections/connect": + self.connects.append({"body": request.post_data_json, "csrf": request.headers.get("x-m365-csrf-token")}) + route.fulfill(json={"success": True, "authorization_url": SIGN_IN_URL}) + elif request.method == "GET" and path == "/getAToken": + self.finish_sign_in(route) + else: + self.unexpected.append(f"{request.method} {path}") + route.fulfill(status=404, json={"error": "Unmocked request"}) + + def finish_sign_in(self, route): + if self.sign_in_fails: + status, html = render_result_page( + kind="workflow", outcome="failed", completion="popup", status=503, + code="m365_key_provision_forbidden", message=KEY_FORBIDDEN, + continue_url="/profile?tab=settings#m365-connection-status", + ) + else: + self.workflow_connection = copy.deepcopy(CONNECTED) + status, html = render_result_page( + kind="workflow", outcome="connected", completion="popup", continue_url=CLASSIC_WORKFLOW_CONNECTED_URL, + ) + route.fulfill(status=status, content_type="text/html", body=html) + + +@pytest.fixture +def settings_ui(editor_browser, editor_assets): + context = editor_browser.new_context(viewport={"width": 1440, "height": 900}) + api = SettingsApi(editor_assets) + errors = [] + # Context routes reach the sign-in popup as well as the page. + context.route("**/*", api.handle) + page = context.new_page() + page.on("pageerror", lambda error: errors.append(str(error))) + try: + yield page, api + finally: + context.close() + assert not api.unexpected, f"Unexpected browser requests: {api.unexpected}" + assert not errors, f"Unexpected browser errors: {errors}" + + +def mount(page, api): + page.goto(ORIGIN + HARNESS) + page.wait_for_function("() => Boolean(window.OrchHarness)") + for url in api.assets: + if url.endswith(".css"): + page.add_style_tag(url=ORIGIN + url) + page.evaluate( + """() => { + const H = window.OrchHarness; + H.reset(); + H.mount('mount-a', 'M365Cards', {}); + H.mount('mount-b', 'Toaster', {}); + }""" + ) + return ( + page.get_by_role("region", name="Chat connection", exact=True), + page.get_by_role("region", name="Workflow connection", exact=True), + ) + + +def test_chat_card_names_the_sources_this_session_can_use(settings_ui): + page, api = settings_ui + chat, _workflow = mount(page, api) + expect(chat.get_by_role("status")).to_have_text( + "Signed in to Microsoft 365 for this session. Chat can use: Email, OneDrive, SharePoint Online (SPO). " + "Microsoft still checks your access each time a source runs." + ) + expect(chat).not_to_contain_text("Sources saved for this session") + for source, checked in (("calendar", False), ("email", True), ("onedrive", True), ("spo", True)): + checkbox = chat.locator(f"#m365-chat-source-{source}") + if checked: + expect(checkbox).to_be_checked() + else: + expect(checkbox).not_to_be_checked() + expect(chat.get_by_role("button", name="Reconnect Microsoft 365 for chat")).to_be_enabled() + + +def test_unready_deployment_is_explained_instead_of_failing_on_connect(settings_ui): + page, api = settings_ui + api.readiness = {"available": False, "reason": "key_vault_disabled", "message": NOT_READY} + _chat, workflow = mount(page, api) + expect(workflow.get_by_text(NOT_READY, exact=True)).to_be_visible() + expect(workflow.get_by_role("button", name="Connect for workflows")).to_be_disabled() + expect(workflow.locator("#m365-workflow-source-email")).to_be_disabled() + expect(workflow).to_contain_text("which SimpleChat creates the first time anyone connects") + assert not api.connects + + +def test_workflow_connect_signs_in_through_a_popup_and_stays_in_v2(settings_ui): + page, api = settings_ui + _chat, workflow = mount(page, api) + expect(workflow.get_by_role("status").first).to_have_text("Workflow connection: disconnected.") + workflow.locator("#m365-workflow-source-email").check() + with page.expect_popup() as opened: + workflow.get_by_role("button", name="Connect for workflows").click() + popup = opened.value + if not popup.is_closed(): + popup.wait_for_event("close") + + expect(page.get_by_text("Microsoft 365 is connected for workflows.", exact=False)).to_be_visible() + expect(workflow.get_by_text("Workflow connection: connected.", exact=True)).to_be_visible() + expect(workflow).to_contain_text("tester@contoso.test") + expect(workflow.get_by_role("button", name="Reconnect for workflows")).to_be_enabled() + assert api.connects == [{"body": {"sources": ["email"], "completion": "popup"}, "csrf": CSRF_TOKEN}] + # The page never left V2 for the sign-in or for classic Profile. + assert urlsplit(page.url).path == HARNESS + + +def test_failed_workflow_sign_in_shows_the_result_pages_message(settings_ui): + page, api = settings_ui + api.sign_in_fails = True + _chat, workflow = mount(page, api) + workflow.locator("#m365-workflow-source-email").check() + with page.expect_popup() as opened: + workflow.get_by_role("button", name="Connect for workflows").click() + popup = opened.value + if not popup.is_closed(): + popup.wait_for_event("close") + + expect(page.get_by_text(KEY_FORBIDDEN, exact=True)).to_be_visible() + expect(workflow.get_by_role("button", name="Connect for workflows")).to_be_enabled() + expect(workflow.get_by_text("Workflow connection: disconnected.", exact=True)).to_be_visible() + assert len(api.connects) == 1 + assert urlsplit(page.url).path == HARNESS + + +def test_open_approvals_stays_in_v2(settings_ui): + page, api = settings_ui + mount(page, api) + authorizations = page.get_by_role("region", name="Workflow authorizations") + expect(authorizations.get_by_role("link", name="Open Approvals")).to_have_attribute("href", "/approvals/m365") From fd690f8b8efa43457b52e18de7f3fb7815152622 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Thu, 8 Oct 2026 13:04:03 -0400 Subject: [PATCH 2/2] Check the Key Vault name where the workflow key request is built, and assert result page statuses Code scanning reported the workflow key's Key Vault URL as full server-side request forgery: the vault name, which an admin settings save can supply, became the request's hostname. The readiness check already fullmatched the name, but in another function, so the request site did not show it. _load_workflow_key now fullmatches the exact stripped name it puts in the URL, so every request stays on a vault in the configured Key Vault domain. A new test bypasses readiness and confirms that invalid names never reach SecretClient. The result page tests now assert the HTTP status they were discarding. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../single_app/functions_m365_connections.py | 10 +++++++++- functional_tests/test_m365_connections.py | 16 ++++++++++++++++ ui_tests/test_m365_connection_result_page.py | 9 ++++++--- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/application/single_app/functions_m365_connections.py b/application/single_app/functions_m365_connections.py index 53e8aafe7..41c4d2760 100644 --- a/application/single_app/functions_m365_connections.py +++ b/application/single_app/functions_m365_connections.py @@ -510,6 +510,14 @@ def _load_workflow_key(settings, version=None, name=None, *, create_missing=Fals readiness = workflow_connection_readiness(settings) if not readiness["available"]: raise M365ConnectionError("m365_key_vault_required", readiness["message"]) + # The name becomes the request's hostname, so validate the exact string used here: a valid + # Key Vault name can only address a vault in the configured Key Vault domain. + vault_name = settings["key_vault_name"].strip() + if not re.fullmatch(_KEY_VAULT_NAME_PATTERN, vault_name): + raise M365ConnectionError( + "m365_key_vault_required", + f"The Key Vault name is not valid. An administrator can correct it in {_KEY_VAULT_SETTINGS_LOCATION}.", + ) configured_name = workflow_key_secret_name() if name is not None and name != configured_name: raise M365ConnectionError( @@ -518,7 +526,7 @@ def _load_workflow_key(settings, version=None, name=None, *, create_missing=Fals "Reconnect Microsoft 365 for workflows.", ) client = SecretClient( - vault_url=f"https://{settings['key_vault_name'].strip()}{app_config.KEY_VAULT_DOMAIN}", + vault_url=f"https://{vault_name}{app_config.KEY_VAULT_DOMAIN}", credential=get_keyvault_credential(settings=settings), ) created = False diff --git a/functional_tests/test_m365_connections.py b/functional_tests/test_m365_connections.py index 88ba24685..2740d91d6 100644 --- a/functional_tests/test_m365_connections.py +++ b/functional_tests/test_m365_connections.py @@ -1099,6 +1099,22 @@ def test_default_key_provider_requires_key_vault_storage_and_names_the_secret_it self.assertEqual(pinned.name, "workflow-token-key") self.assertEqual(renamed.exception.code, "m365_key_unavailable") + def test_key_vault_hostname_comes_only_from_a_valid_vault_name(self): + # Readiness rejects these names too; bypassing it proves the hostname check stands alone. + for vault_name in ("evil.example.com/x", "test-vault.evil.example", "user@test-vault", "test-vault:8443"): + with self.subTest(vault_name=vault_name): + settings = {"enable_key_vault_secret_storage": True, "key_vault_name": vault_name} + with patch.dict(sys.modules, self.key_vault_modules(settings)), patch.dict( + os.environ, {connections.KEY_SECRET_ENV: ""}, + ), patch.object( + connections, "workflow_connection_readiness", + return_value={"available": True, "reason": None, "message": ""}, + ), patch("azure.keyvault.secrets.SecretClient") as constructor: + with self.assertRaises(connections.M365ConnectionError) as raised: + connections._default_key_provider() + constructor.assert_not_called() + self.assertEqual(raised.exception.code, "m365_key_vault_required") + def test_missing_key_is_created_once_when_encrypting_and_never_when_decrypting(self): from azure.core.exceptions import ResourceNotFoundError client = Mock() diff --git a/ui_tests/test_m365_connection_result_page.py b/ui_tests/test_m365_connection_result_page.py index 2e246592e..d33a21453 100644 --- a/ui_tests/test_m365_connection_result_page.py +++ b/ui_tests/test_m365_connection_result_page.py @@ -185,10 +185,11 @@ def test_popup_reports_a_failure_with_its_code_and_message_as_text(result_ui): def test_failure_without_an_opener_stays_readable_with_a_link_back(result_ui, viewport): page, fixture = result_ui page.set_viewport_size(viewport) - _status, fixture.result_html = render_result_page( + status, fixture.result_html = render_result_page( kind="chat", outcome="failed", completion="auto", status=400, code="m365_auth_state_invalid", message=UNSAFE_MESSAGE, continue_url=CLASSIC_CHAT_CONNECTION_URL, ) + assert status == 400 page.goto(f"{ORIGIN}/getAToken?code=ui&state=fixture") expect(page.get_by_role("heading", name="Microsoft 365 was not connected")).to_be_visible() expect(page.get_by_role("status")).to_have_text(UNSAFE_MESSAGE) @@ -206,9 +207,10 @@ def test_failure_without_an_opener_stays_readable_with_a_link_back(result_ui, vi def test_full_page_success_continues_to_the_fixed_settings_page(result_ui): page, fixture = result_ui - _status, fixture.result_html = render_result_page( + status, fixture.result_html = render_result_page( kind="workflow", outcome="connected", completion="auto", continue_url=CLASSIC_WORKFLOW_CONNECTED_URL, ) + assert status == 200 page.goto(f"{ORIGIN}/getAToken?code=ui&state=fixture") expect(page.get_by_role("heading", name="Profile fixture")).to_be_visible() returned = urlsplit(page.url) @@ -219,9 +221,10 @@ def test_full_page_success_continues_to_the_fixed_settings_page(result_ui): def test_popup_that_lost_its_opener_offers_to_close_instead_of_navigating(result_ui): page, fixture = result_ui - _status, fixture.result_html = render_result_page( + status, fixture.result_html = render_result_page( kind="chat", outcome="connected", completion="popup", continue_url=CLASSIC_CHAT_CONNECTION_URL, ) + assert status == 200 page.goto(f"{ORIGIN}/getAToken?code=ui&state=fixture") expect(page.get_by_role("heading", name="Microsoft 365 is connected")).to_be_visible() expect(page.get_by_role("button", name="Close window")).to_be_visible()