From b3f33cb75f5cb1ba0dcc32f770f6bc016563df97 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Fri, 9 Oct 2026 07:37:59 -0400 Subject: [PATCH 1/2] Add native V2 file source configuration across workspaces Reuse the shared editor for personal, group and public file-source setup without classic page hand-offs. Add authorized personal editor reads and conflict-safe writes while preserving stored secrets, existing connector availability and classic API compatibility. Include API, browser and transport regressions plus versioned documentation. Refs #1722 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- application/single_app/functions_file_sync.py | 1 + .../single_app/route_backend_file_sync.py | 135 ++++++-- .../fileSources/FileSourceEditorDialog.tsx | 125 ++++++-- application/v2_ui/src/components/ui/Modal.tsx | 5 +- application/v2_ui/src/lib/fileSourceFields.ts | 46 ++- .../v2_ui/src/lib/fileSourceWorkbench.ts | 151 ++++++--- application/v2_ui/src/lib/types.ts | 3 +- .../pages/workspace/FileSourcesSection.tsx | 231 +------------- .../workspace/GroupFileSourcesSection.tsx | 169 ++++++++-- .../features/V2_FILE_SOURCE_CONFIGURATION.md | 96 ++++++ .../features/V2_GROUP_FILE_SOURCES.md | 6 + docs/explanation/features/V2_MY_WORKSPACE.md | 8 +- .../features/V2_PUBLIC_CONNECTIONS.md | 6 + docs/guides/create-a-file-sync.md | 38 ++- .../test_route_blueprint_policy_inventory.py | 4 +- .../test_group_file_source_fixture_parity.py | 4 +- .../test_personal_file_source_apis.py | 295 +++++++++++++++++ .../test_public_file_source_fixture_parity.py | 16 +- .../file_source_configuration_probe.ts | 206 ++++++++++++ .../personal_file_source_harness.py | 62 ++++ .../test_v2_personal_file_sources.py | 35 +++ ui_tests/fixtures/group_workspace.py | 1 + ui_tests/fixtures/personal_file_sources.py | 195 ++++++++++++ ui_tests/fixtures/public_file_sources.py | 46 ++- ui_tests/test_v2_group_file_sources.py | 8 +- ui_tests/test_v2_group_file_sources.ts | 36 +-- ui_tests/test_v2_personal_file_sources.py | 297 ++++++++++++++++++ ui_tests/test_v2_public_file_sources.py | 49 ++- 29 files changed, 1852 insertions(+), 424 deletions(-) create mode 100644 docs/explanation/features/V2_FILE_SOURCE_CONFIGURATION.md create mode 100644 functional_tests/test_personal_file_source_apis.py create mode 100644 functional_tests/test_support/file_source_configuration_probe.ts create mode 100644 functional_tests/test_support/personal_file_source_harness.py create mode 100644 functional_tests/test_v2_personal_file_sources.py create mode 100644 ui_tests/fixtures/personal_file_sources.py create mode 100644 ui_tests/test_v2_personal_file_sources.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 55eb38109..39190f940 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.309" +VERSION = "0.261.310" 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_file_sync.py b/application/single_app/functions_file_sync.py index d4a41d505..f2d202d19 100644 --- a/application/single_app/functions_file_sync.py +++ b/application/single_app/functions_file_sync.py @@ -867,6 +867,7 @@ def build_file_sync_source_options(scope_type: str, scope_id: str, settings: Opt }, "limits": {"max_sources": config["file_sync_max_sources_per_scope"]}, "recursive_allowed": bool(config["file_sync_allow_recursive_sources"]), + "default_remote_delete_policy": config["file_sync_default_remote_delete_policy"], } diff --git a/application/single_app/route_backend_file_sync.py b/application/single_app/route_backend_file_sync.py index 790a92519..fd20d5a98 100644 --- a/application/single_app/route_backend_file_sync.py +++ b/application/single_app/route_backend_file_sync.py @@ -2,7 +2,7 @@ import logging -from flask import jsonify, request +from flask import jsonify, request, session from functions_appinsights import log_event from functions_authentication import admin_required, enabled_required, get_current_user_id, get_current_user_info, login_required, user_required @@ -12,10 +12,15 @@ FILE_SYNC_SCOPE_PERSONAL, FILE_SYNC_SCOPE_PUBLIC, FILE_SYNC_SOURCE_TYPE_SMB, + FileSyncConfigConflict, + FileSyncDeleteIncomplete, FileSyncPublicValidationError, + FileSyncSourceBusy, FileSyncWriteConflict, assert_public_workspace_role, browse_file_sync_source_path, + build_file_sync_source_options, + compute_file_sync_config_revision, create_file_sync_source, delete_file_sync_source, get_authorized_sync_source, @@ -51,7 +56,9 @@ def _current_user(): user_id = get_current_user_id() if not user_id: return None, None - return user_id, get_current_user_info() or {} + user_info = get_current_user_info() or {} + user_info["roles"] = (session.get("user") or {}).get("roles", []) + return user_id, user_info def _require_personal_context(): user_id, user_info = _current_user() @@ -121,7 +128,10 @@ def _serialize_public_workspace_target(workspace): } def _map_exception(error): - expected_error = isinstance(error, (PermissionError, LookupError, ValueError, FileSyncWriteConflict)) + expected_error = isinstance(error, ( + PermissionError, LookupError, ValueError, FileSyncWriteConflict, + FileSyncConfigConflict, FileSyncSourceBusy, + )) log_event( "[FILE_SYNC] Request failed.", level=logging.WARNING if expected_error else logging.ERROR, @@ -129,49 +139,107 @@ def _map_exception(error): "endpoint": request.endpoint or "", "method": request.method, "exception_type": type(error).__name__, - "error": str(error), }, exceptionTraceback=not expected_error, ) if isinstance(error, FileSyncPublicValidationError): return _error(error.public_message, 400) + partial_result = getattr(error, "delete_result", None) + partial_fields = ( + {"partial": bool(getattr(error, "partial", False)), "delete_result": partial_result} + if partial_result is not None else {} + ) + if isinstance(error, FileSyncDeleteIncomplete): + return jsonify({ + "error": error.public_message, + "error_code": "delete_incomplete", + "delete_result": error.delete_result, + "partial": error.delete_result.get("documents_deleted", 0) > 0, + }), 409 + if isinstance(error, FileSyncConfigConflict): + return jsonify({ + "error": "This file source changed while it was being saved. Reload it and try again.", + "error_code": "config_conflict", **partial_fields, + }), 409 + if isinstance(error, FileSyncSourceBusy): + return jsonify({ + "error": "Wait for the running sync to finish, then delete the source.", + "error_code": "source_busy", **partial_fields, + }), 409 if isinstance(error, FileSyncWriteConflict): - return _error("This File Sync item changed while it was being saved. Reload it and try again.", 409) + return jsonify({ + "error": "This File Sync item changed while it was being saved. Try saving again.", + "error_code": "write_conflict", **partial_fields, + }), 409 if isinstance(error, PermissionError): return _error("You do not have permission to perform this File Sync operation.", 403) if isinstance(error, LookupError): + if partial_fields: + return jsonify({ + "error": "The requested File Sync resource was not found.", **partial_fields, + }), 404 return _error("The requested File Sync resource was not found.", 404) if isinstance(error, ValueError): return _error("The File Sync request could not be completed. Verify the source configuration and try again.", 400) return _error("An unexpected error occurred while processing the File Sync request.", 500) + def _project_source(scope_type, source): + projected = sanitize_file_sync_source(source) + if scope_type == FILE_SYNC_SCOPE_PERSONAL: + for field in ("_etag", "_rid", "_self", "_attachments", "_ts"): + projected.pop(field, None) + projected["config_revision"] = compute_file_sync_config_revision(source) + return projected + def _list_sources(scope_type, scope_id): - sources = [sanitize_file_sync_source(source) for source in list_file_sync_sources(scope_type, scope_id)] + sources = [_project_source(scope_type, source) for source in list_file_sync_sources(scope_type, scope_id)] return jsonify({"sources": sources}), 200 + def _expected_revision(payload): + if "expected_config_revision" not in payload: + return None + revision = payload.pop("expected_config_revision") + if not isinstance(revision, str) or not revision.strip(): + raise FileSyncPublicValidationError("Provide a valid configuration revision.") + return revision.strip() + + def _personal_write_payload(): + payload = _payload() + if not isinstance(payload, dict): + raise FileSyncPublicValidationError("A JSON object is required for this action.") + return dict(payload) + def _assert_new_source_type_visible(payload): source_type = str(payload.get("source_type") or FILE_SYNC_SOURCE_TYPE_SMB).strip().lower() if not is_file_sync_source_type_visible(get_settings(), source_type): raise PermissionError("This File Sync source type is not available") - def _create_source(scope_type, scope_id, user_id): - payload = _payload() + def _create_source(scope_type, scope_id, user_id, *, personal_editor=False): + payload = _personal_write_payload() if personal_editor else _payload() _assert_new_source_type_visible(payload) - source = create_file_sync_source(scope_type, scope_id, payload, user_id) - return jsonify({"source": sanitize_file_sync_source(source)}), 201 - - def _update_source(scope_type, scope_id, source_id, user_id): - source = update_file_sync_source(scope_type, scope_id, source_id, _payload(), user_id) - return jsonify({"source": sanitize_file_sync_source(source)}), 200 + source = create_file_sync_source(scope_type, scope_id, payload, user_id, stage_secrets=personal_editor) + return jsonify({"source": _project_source(scope_type, source)}), 201 + + def _update_source(scope_type, scope_id, source_id, user_id, *, personal_editor=False): + payload = _personal_write_payload() if personal_editor else _payload() + revision = _expected_revision(payload) if personal_editor else None + source = update_file_sync_source( + scope_type, scope_id, source_id, payload, user_id, + expected_config_revision=revision, stage_secrets=revision is not None, + ) + return jsonify({"source": _project_source(scope_type, source)}), 200 - def _delete_source(scope_type, scope_id, source_id, user_id): - payload = _payload() + def _delete_source(scope_type, scope_id, source_id, user_id, *, personal_editor=False): + payload = _personal_write_payload() if personal_editor else _payload() + revision = _expected_revision(payload) if personal_editor else None delete_result = delete_file_sync_source( scope_type, scope_id, source_id, user_id, delete_associated_files=bool(payload.get("delete_associated_files")), + expected_config_revision=revision, + refuse_active_run=revision is not None, ) return jsonify({"message": "File Sync source deleted", "delete_result": delete_result}), 200 @@ -571,6 +639,35 @@ def api_file_sync_personal_sources_list(): except Exception as error: return _map_exception(error) + @bp.route('/api/file-sync/personal/source-options', methods=['GET']) + @swagger_route(security=get_auth_security()) + @login_required + @user_required + @enabled_required("enable_file_sync") + def api_file_sync_personal_source_options(): + try: + user_id = _require_personal_context() + if request.args or request.get_data(): + raise FileSyncPublicValidationError("This request does not accept parameters or a body.") + return jsonify(build_file_sync_source_options(FILE_SYNC_SCOPE_PERSONAL, user_id, get_settings())), 200 + except Exception as error: + return _map_exception(error) + + @bp.route('/api/file-sync/personal/sources/', methods=['GET']) + @swagger_route(security=get_auth_security()) + @login_required + @user_required + @enabled_required("enable_file_sync") + def api_file_sync_personal_source_read(source_id): + try: + user_id = _require_personal_context() + if request.args or request.get_data(): + raise FileSyncPublicValidationError("This request does not accept parameters or a body.") + source = get_authorized_sync_source(FILE_SYNC_SCOPE_PERSONAL, source_id, user_id, scope_id=user_id) + return jsonify({"source": _project_source(FILE_SYNC_SCOPE_PERSONAL, source)}), 200 + except Exception as error: + return _map_exception(error) + @bp.route('/api/file-sync//sources/browse', methods=['POST']) @swagger_route(security=get_auth_security()) @login_required @@ -637,7 +734,7 @@ def api_file_sync_public_source_browse_existing(public_workspace_id, source_id): def api_file_sync_personal_sources_create(): try: user_id = _require_personal_context() - return _create_source(FILE_SYNC_SCOPE_PERSONAL, user_id, user_id) + return _create_source(FILE_SYNC_SCOPE_PERSONAL, user_id, user_id, personal_editor=True) except Exception as error: return _map_exception(error) @@ -661,7 +758,7 @@ def api_file_sync_personal_source_test_connection_new(): def api_file_sync_personal_source_update(source_id): try: user_id = _require_personal_context() - return _update_source(FILE_SYNC_SCOPE_PERSONAL, user_id, source_id, user_id) + return _update_source(FILE_SYNC_SCOPE_PERSONAL, user_id, source_id, user_id, personal_editor=True) except Exception as error: return _map_exception(error) @@ -685,7 +782,7 @@ def api_file_sync_personal_source_test_connection_existing(source_id): def api_file_sync_personal_source_delete(source_id): try: user_id = _require_personal_context() - return _delete_source(FILE_SYNC_SCOPE_PERSONAL, user_id, source_id, user_id) + return _delete_source(FILE_SYNC_SCOPE_PERSONAL, user_id, source_id, user_id, personal_editor=True) except Exception as error: return _map_exception(error) diff --git a/application/v2_ui/src/components/fileSources/FileSourceEditorDialog.tsx b/application/v2_ui/src/components/fileSources/FileSourceEditorDialog.tsx index af72f0125..6034db42b 100644 --- a/application/v2_ui/src/components/fileSources/FileSourceEditorDialog.tsx +++ b/application/v2_ui/src/components/fileSources/FileSourceEditorDialog.tsx @@ -29,11 +29,13 @@ import { authUsesSecret, authUsesUsername, connectionDescriptor, + emptyFileSourceDraft, eligibleIdentities, isPathSelected, normalizeFixedTag, parentBrowsePath, secretFieldLabel, + sourceTypeLabel, visibleSourceTypes, withFixedTag, withSelectedPath, @@ -90,6 +92,7 @@ export function FileSourceEditorDialog({ tagSuggestions = [], tagSuggestionsFailed = false, saving, + saveBlocked = false, error, onChange, onSave, @@ -98,6 +101,7 @@ export function FileSourceEditorDialog({ onTest, onBrowse, onIgnore, + returnFocusTo, }: { draft: FileSourceDraft; options: FileSourceOptions | null; @@ -109,6 +113,7 @@ export function FileSourceEditorDialog({ /** The existing tags could not be read, so none are offered; a tag can still be typed. */ tagSuggestionsFailed?: boolean; saving: boolean; + saveBlocked?: boolean; error: string | null; onChange: (next: FileSourceDraft) => void; onSave: () => void; @@ -123,8 +128,14 @@ export function FileSourceEditorDialog({ * tracks it per path for the session. */ onIgnore?: (remotePath: string, ignored: boolean) => Promise; + returnFocusTo?: HTMLElement | null; }) { const nameRef = useRef(null); + const sourceTypeLabelId = useId(); + const identityLabelId = useId(); + const authenticationLabelId = useId(); + const secretLabelId = useId(); + const secretHintId = useId(); const folderTagsId = useId(); const deletePolicyId = useId(); const [confirmingDiscard, setConfirmingDiscard] = useState(false); @@ -136,6 +147,14 @@ export function FileSourceEditorDialog({ const [tagInput, setTagInput] = useState(''); const [tagError, setTagError] = useState(null); const dirty = JSON.stringify(draft) !== original || pathInput.trim() !== '' || tagInput.trim() !== ''; + const mounted = useRef(true); + const connectionKey = JSON.stringify({ + sourceType: draft.sourceType, connection: draft.connection, + credentialMode: draft.credentialMode, identityId: draft.identityId, + credentials: draft.credentials, + }); + const latestConnectionKey = useRef(connectionKey); + latestConnectionKey.current = connectionKey; const [testing, setTesting] = useState(false); const [testResult, setTestResult] = useState(null); @@ -155,13 +174,18 @@ export function FileSourceEditorDialog({ [identities, options, draft.sourceType], ); const authTypes = descriptor.authTypes; - const canSave = draft.name.trim().length > 0 - && (draft.credentialMode === 'inline' || draft.identityId.trim().length > 0); const scheduleRange = options?.schedule ?? { min_interval_minutes: 1, max_interval_minutes: 10080 }; const recursiveAllowed = options?.recursive_allowed !== false; + const canSave = !saveBlocked && draft.name.trim().length > 0 + && (draft.credentialMode === 'inline' || eligible.some((identity) => identity.id === draft.identityId)) + && (!draft.scheduleEnabled || (Number.isInteger(draft.intervalMinutes) + && draft.intervalMinutes >= scheduleRange.min_interval_minutes + && draft.intervalMinutes <= scheduleRange.max_interval_minutes)); useEffect(() => { + mounted.current = true; nameRef.current?.focus(); + return () => { mounted.current = false; }; }, []); // Keep the inline auth type valid for the current source type: a type change can drop the @@ -177,9 +201,12 @@ export function FileSourceEditorDialog({ useEffect(() => { setBrowseResult(null); setBrowseError(null); - }, [draft.sourceType]); + setTestResult(null); + setTestError(null); + }, [connectionKey]); const requestClose = () => { + if (saving) return; if (dirty && !confirmingDiscard) { setConfirmingDiscard(true); return; @@ -206,27 +233,35 @@ export function FileSourceEditorDialog({ onChange({ ...draft, credentials: { ...draft.credentials, ...patch } }); const runTest = async () => { + const key = connectionKey; setTesting(true); setTestError(null); setTestResult(null); try { - setTestResult(await onTest()); + const result = await onTest(); + if (mounted.current && latestConnectionKey.current === key) setTestResult(result); } catch (cause) { - setTestError(cause instanceof Error ? cause.message : 'The connection test failed.'); + if (mounted.current && latestConnectionKey.current === key) { + setTestError(cause instanceof Error ? cause.message : 'The connection test failed.'); + } } finally { - setTesting(false); + if (mounted.current) setTesting(false); } }; const runBrowse = async (browsePath: string) => { + const key = connectionKey; setBrowsing(true); setBrowseError(null); try { - setBrowseResult(await onBrowse(browsePath)); + const result = await onBrowse(browsePath); + if (mounted.current && latestConnectionKey.current === key) setBrowseResult(result); } catch (cause) { - setBrowseError(cause instanceof Error ? cause.message : 'Could not browse this location.'); + if (mounted.current && latestConnectionKey.current === key) { + setBrowseError(cause instanceof Error ? cause.message : 'Could not browse this location.'); + } } finally { - setBrowsing(false); + if (mounted.current) setBrowsing(false); } }; @@ -288,6 +323,7 @@ export function FileSourceEditorDialog({ if (!onIgnore || !remotePath) { return; } + const key = connectionKey; const next = !ignoredPaths[remotePath]; setIgnoreError(null); try { @@ -296,9 +332,13 @@ export function FileSourceEditorDialog({ // is authoritative; browse can't reflect the change, so the per-path map is updated from // the response rather than by re-browsing. const applied = await onIgnore(remotePath, next); - setIgnoredPaths((current) => ({ ...current, [remotePath]: applied })); + if (mounted.current && latestConnectionKey.current === key) { + setIgnoredPaths((current) => ({ ...current, [remotePath]: applied })); + } } catch (cause) { - setIgnoreError(cause instanceof Error ? cause.message : 'Could not update the ignore list.'); + if (mounted.current && latestConnectionKey.current === key) { + setIgnoreError(cause instanceof Error ? cause.message : 'Could not update the ignore list.'); + } } }; @@ -307,8 +347,9 @@ export function FileSourceEditorDialog({ return ( ) : ( <> - {error ? {error} : null} + {error ? {error} : null} Cancel @@ -339,7 +380,7 @@ export function FileSourceEditorDialog({ ) } > -
+
-
+
} - {draft.credentialMode === 'identity' ? ( + {draft.sourceType === 'onedrive' ? null : draft.credentialMode === 'identity' ? ( ); } diff --git a/application/v2_ui/src/components/ui/Modal.tsx b/application/v2_ui/src/components/ui/Modal.tsx index 3631f5234..9c798c477 100644 --- a/application/v2_ui/src/components/ui/Modal.tsx +++ b/application/v2_ui/src/components/ui/Modal.tsx @@ -95,6 +95,7 @@ export function Modal({ banner, panelRef: externalPanelRef, placement = 'center', + returnFocusTo, }: { title: string; description?: string; @@ -120,10 +121,12 @@ export function Modal({ /** The dialog's panel, for a caller that animates it into place. */ panelRef?: Ref; placement?: 'center' | 'drawer'; + /** Preserve the original opener across loading/editor dialog handoffs. */ + returnFocusTo?: HTMLElement | null; }) { // Read while rendering: by the time an effect runs, a field's autoFocus has already moved // focus into the dialog, and it is the control that opened it that focus must return to. - const [opener] = useState(currentOpener); + const [opener] = useState(() => returnFocusTo ?? currentOpener()); const panelRef = useRef(null); const setPanelRef = useCallback((node: HTMLDivElement | null) => { panelRef.current = node; diff --git a/application/v2_ui/src/lib/fileSourceFields.ts b/application/v2_ui/src/lib/fileSourceFields.ts index 2b5eb006a..4d688a5da 100644 --- a/application/v2_ui/src/lib/fileSourceFields.ts +++ b/application/v2_ui/src/lib/fileSourceFields.ts @@ -67,6 +67,19 @@ export const FILE_SOURCE_REBASE_FIELDS: RebaseField[] = [ export const FILE_SOURCE_TYPE_SMB = 'smb'; export const FILE_SOURCE_TYPE_AZURE_FILES = 'azure_files'; export const FILE_SOURCE_TYPE_AZURE_BLOB = 'azure_blob'; +export const FILE_SOURCE_TYPE_ONEDRIVE = 'onedrive'; + +const SOURCE_TYPE_LABELS: Record = { + smb: 'Network share', + azure_files: 'Azure Files', + azure_blob: 'Azure Blob Storage', + onedrive: 'OneDrive', +}; + +export function sourceTypeLabel(sourceType: unknown): string { + const raw = String(sourceType ?? '').trim(); + return SOURCE_TYPE_LABELS[raw] ?? (raw ? raw.replace(/[_-]+/g, ' ') : 'Unknown'); +} /** * How synced files are tagged from their folders, as `FILE_SYNC_FOLDER_TAG_MODES` defines them. The @@ -114,9 +127,8 @@ export interface ConnectionDescriptor { /** * The connection shape per source type, mirroring `_normalize_connection_payload` and - * `FILE_SYNC_IDENTITY_AUTH_TYPES_BY_SOURCE`. Only the group-eligible storage types are modelled; a - * type the options endpoint does not mark visible is never offered, so an unmodelled connector can - * never be reached from here. + * `FILE_SYNC_IDENTITY_AUTH_TYPES_BY_SOURCE`. The options endpoint controls new-source availability. + * OneDrive is modelled only so an existing personal connector can be edited without changing auth. */ export const CONNECTION_DESCRIPTORS: Record = { [FILE_SOURCE_TYPE_SMB]: { @@ -141,10 +153,18 @@ export const CONNECTION_DESCRIPTORS: Record = { ], authTypes: ['managed_identity', 'client_secret', 'connection_string'], }, + [FILE_SOURCE_TYPE_ONEDRIVE]: { + fields: [], + authTypes: ['global_identity'], + }, }; export function connectionDescriptor(sourceType: string): ConnectionDescriptor { - return CONNECTION_DESCRIPTORS[sourceType] ?? CONNECTION_DESCRIPTORS[FILE_SOURCE_TYPE_SMB]; + const descriptor = CONNECTION_DESCRIPTORS[sourceType]; + if (!descriptor) { + throw new Error('This source type cannot be configured in this editor.'); + } + return descriptor; } export const AUTH_TYPE_LABELS: Record = { @@ -153,6 +173,7 @@ export const AUTH_TYPE_LABELS: Record = { managed_identity: 'Managed identity', client_secret: 'Service principal (client secret)', connection_string: 'Connection string or SAS', + global_identity: 'Administrator-managed connector', }; export function authTypeLabel(authType: unknown): string { @@ -164,7 +185,7 @@ export const authUsesUsername = (authType: string): boolean => authType === 'use export const authUsesClientId = (authType: string): boolean => authType === 'client_secret'; /** Managed identity and anonymous carry no stored secret; every other method does. */ export const authUsesSecret = (authType: string): boolean => - authType !== 'anonymous' && authType !== 'managed_identity'; + !['anonymous', 'managed_identity', 'global_identity'].includes(authType); export function secretFieldLabel(authType: string, stored: boolean): string { if (authType === 'username_password') { @@ -322,7 +343,7 @@ export function draftFromSource(source: WorkspaceSyncSource, minInterval: number remoteDeletePolicy: storedChoice(source.remote_delete_policy, REMOTE_DELETE_POLICIES, DEFAULT_REMOTE_DELETE_POLICY), scheduleEnabled: Boolean((schedule as Record).enabled), intervalMinutes: Number((schedule as Record).interval_minutes ?? minInterval) || minInterval || 60, - credentialMode: identityId ? 'identity' : 'inline', + credentialMode: sourceType !== FILE_SOURCE_TYPE_ONEDRIVE && identityId ? 'identity' : 'inline', identityId, credentials: { authType: String(credentials.auth_type ?? connectionDescriptor(sourceType).authTypes[0] ?? 'username_password'), @@ -345,6 +366,9 @@ export function draftFromSource(source: WorkspaceSyncSource, minInterval: number function buildConnection(draft: FileSourceDraft): Record { const connection = draft.connection; const selectedPaths = [...draft.selectedPaths]; + if (draft.sourceType === FILE_SOURCE_TYPE_ONEDRIVE) { + return { selected_paths: selectedPaths }; + } if (draft.sourceType === FILE_SOURCE_TYPE_AZURE_FILES) { return { account_url: connection.accountUrl.trim(), @@ -371,7 +395,7 @@ function buildConnection(draft: FileSourceDraft): Record { */ function buildCredentials(credentials: FileSourceCredentialsDraft): Record { const authType = credentials.authType; - if (authType === 'anonymous') { + if (authType === 'anonymous' || authType === 'global_identity') { return { auth_type: authType }; } if (authType === 'managed_identity') { @@ -426,7 +450,10 @@ export function buildFileSourceWrite(draft: FileSourceDraft): FileSourceWrite { schedule: { enabled: draft.scheduleEnabled, interval_minutes: draft.intervalMinutes }, remote_delete_policy: draft.remoteDeletePolicy, }; - if (draft.credentialMode === 'identity') { + if (draft.sourceType === FILE_SOURCE_TYPE_ONEDRIVE) { + write.identity_id = ''; + write.credentials = { auth_type: 'global_identity' }; + } else if (draft.credentialMode === 'identity') { write.identity_id = draft.identityId; } else { write.identity_id = ''; @@ -439,6 +466,9 @@ export function buildFileSourceWrite(draft: FileSourceDraft): FileSourceWrite { export function sourcePathText(source: WorkspaceSyncSource): string { const connection = readConnection(source); const sourceType = String(source.source_type ?? ''); + if (sourceType === FILE_SOURCE_TYPE_ONEDRIVE) { + return 'Your OneDrive'; + } if (sourceType === FILE_SOURCE_TYPE_AZURE_FILES) { const share = String(connection.share_name ?? ''); const directory = String(connection.directory_path ?? ''); diff --git a/application/v2_ui/src/lib/fileSourceWorkbench.ts b/application/v2_ui/src/lib/fileSourceWorkbench.ts index 28cf404f7..1431306f8 100644 --- a/application/v2_ui/src/lib/fileSourceWorkbench.ts +++ b/application/v2_ui/src/lib/fileSourceWorkbench.ts @@ -1,12 +1,8 @@ // fileSourceWorkbench.ts // Scope-aware file source reads and writes for the workspace "Sync" (file sources) section. // -// The file sources section shipped personal-only: it called the /api/file-sync/personal -// functions in workspaceApi.ts directly, and stayed list-and-delete. This module is the seam that -// lets the section serve a group workspace natively, the way identityWorkbench.ts did for -// identities, and from M10B a public workspace too. A FileSourceScope selects which URLs are used -// and which per-operation gates apply. The personal adapter is a thin pass-through so its behaviour -// stays byte-identical in effect: same list URL, same runs/sync/delete URLs. +// Each scope selects its own URLs and envelopes. Personal sources keep their existing API family; +// shared sources use immutable workspace IDs and server-advertised operation gates. // // The group and public paths never fall back to personal behaviour. An absent or unrecognised // file_source_management hint yields an empty operation set, which leaves every write gate refusing @@ -16,7 +12,7 @@ // check (§9), so the envelope is validated strictly instead: a malformed response throws rather // than rendering as empty. // -// Group and public scope carry a conditional write over config_revision, not an etag: PATCH and +// All native editors carry a conditional write over config_revision, not an etag: PATCH and // DELETE send expected_config_revision (missing is 400, stale is 409). A 409 becomes a typed error // the section turns into "your draft is kept, reload and retry". Delete is richer: the associated // documents may already be gone when the removal is refused, so a refusal carrying partial:true @@ -25,12 +21,11 @@ import { ApiError, api, requestWithStatus } from './apiClient'; import { fetchScopedGroupDocumentTags, fetchScopedPublicDocumentTags } from './documentReadAdapter'; +import { fetchPersonalDocumentTags } from './endpoints'; import { isRecord } from './workspaceAuthoring'; import { requireWorkspaceId } from './workspaceContext'; import { - deleteSyncSource as deletePersonalSyncSource, fetchSyncRuns as fetchPersonalSyncRuns, - fetchSyncSources as fetchPersonalSyncSources, startSyncRun as startPersonalSyncRun, } from './workspaceApi'; import type { @@ -162,13 +157,13 @@ export interface FileSourceWorkbenchAdapter { create: (write: FileSourceWrite) => Promise; update: (source: WorkspaceSyncSource, write: FileSourceWrite) => Promise; remove: (source: WorkspaceSyncSource, deleteAssociatedFiles: boolean) => Promise; - /** The server-decided editor options; null when the scope has none (personal). */ + /** The server-decided editor options. */ options: (signal?: AbortSignal) => Promise; - /** The group identities that back the credential picker; empty for personal. */ + /** Same-scope identities backing the credential picker. */ identities: (signal?: AbortSignal) => Promise; /** * The workspace's existing tag names, offered as fixed-tag suggestions, most used first; empty - * for personal. A failed read only costs the suggestions, so the section treats it as optional. + * A failed read only costs the suggestions, so the section treats it as optional. */ tags: (signal?: AbortSignal) => Promise; runs: (sourceId: string, signal?: AbortSignal) => Promise; @@ -176,8 +171,6 @@ export interface FileSourceWorkbenchAdapter { testConnection: (source: WorkspaceSyncSource | null, write: FileSourceWrite | null) => Promise; browse: (source: WorkspaceSyncSource | null, write: FileSourceWrite | null, browsePath: string) => Promise; ignorePath: (sourceId: string, remotePath: string, ignored: boolean) => Promise; - /** Where "adding one is still done in the classic workspace" points, for personal scope. */ - classicPath: string; } /** @@ -225,48 +218,85 @@ export function fileSourceOperationAllowed( return Array.isArray(source.source_actions) && source.source_actions.includes(operation); } -const NOT_IN_PERSONAL = 'This action is only available in a group workspace.'; +function personalFileSourcesUrl(sourceId?: string, suffix?: string): string { + const base = '/api/file-sync/personal/sources'; + const path = sourceId ? `${base}/${encodeURIComponent(requireWorkspaceId(sourceId))}` : base; + return suffix ? `${path}/${suffix}` : path; +} + +function personalSourceFromResponse(value: unknown): WorkspaceSyncSource { + if (isRecord(value) && isRecord(value.source) && typeof value.source.id === 'string' + && value.source.id && typeof value.source.config_revision === 'string' + && value.source.config_revision) { + return value.source as WorkspaceSyncSource; + } + throw new Error('The file source response was malformed. Refresh and try again.'); +} export const PERSONAL_FILE_SOURCE_WORKBENCH: FileSourceWorkbenchAdapter = { scope: { kind: 'personal' }, - manageable: false, - supported: new Set(), + manageable: true, + supported: new Set(FILE_SOURCE_OPERATIONS), allows: () => true, - list: (signal) => fetchPersonalSyncSources(signal), - read: async (source) => source, - create: () => { - throw new Error('Adding a file source is done in the classic workspace.'); + list: async (signal) => { + const response = await api.get(personalFileSourcesUrl(), signal); + if (!isRecord(response) || !Array.isArray(response.sources)) { + throw new Error('The file sources response was malformed. Refresh and try again.'); + } + return response.sources.map((source) => personalSourceFromResponse({ source })); }, - update: () => { - throw new Error('Editing a file source is done in the classic workspace.'); + read: async (source, signal) => + personalSourceFromResponse(await api.get(personalFileSourcesUrl(source.id), signal)), + create: async (write) => + personalSourceFromResponse(await api.post(personalFileSourcesUrl(), groupWriteBody(write))), + update: async (source, write) => { + const response = await conditionalGroupUpdate(personalFileSourcesUrl(source.id), { + ...groupWriteBody(write), expected_config_revision: requiredConfigRevision(source), + }); + return personalSourceFromResponse(response); }, - remove: async (source, deleteAssociatedFiles) => { - await deletePersonalSyncSource(source.id, deleteAssociatedFiles); - return { - associated_files_requested: deleteAssociatedFiles, - documents_deleted: 0, - documents_skipped: 0, - documents_failed: 0, - }; + remove: (source, deleteAssociatedFiles) => deleteGroupSource(personalFileSourcesUrl(source.id), { + expected_config_revision: requiredConfigRevision(source), + delete_associated_files: deleteAssociatedFiles, + }), + options: async (signal) => + optionsFromResponse(await api.get('/api/file-sync/personal/source-options', signal)), + identities: async (signal) => + identitiesFromResponse(await api.get('/api/workspace-identities/personal/identities', signal)), + tags: async (signal) => { + const response = await fetchPersonalDocumentTags(signal); + if (!Array.isArray(response.tags)) { + throw new Error('The document tags response was malformed.'); + } + return [...response.tags] + .sort((left, right) => (Number(right.count) - Number(left.count)) || left.name.localeCompare(right.name)) + .map((tag) => tag.name); }, - options: async () => null, - identities: async () => [], - tags: async () => [], runs: (sourceId, signal) => fetchPersonalSyncRuns(sourceId, signal), sync: async (sourceId) => { const response = await startPersonalSyncRun(sourceId); return response.run ?? null; }, - testConnection: () => { - throw new Error(NOT_IN_PERSONAL); + testConnection: async (source, write) => { + const response = await api.post( + personalFileSourcesUrl(source?.id, 'test-connection'), + write ? groupWriteBody(write) : undefined, + ); + return connectionFromResponse(response); }, - browse: () => { - throw new Error(NOT_IN_PERSONAL); + browse: async (source, write, browsePath) => { + const response = await api.post( + personalFileSourcesUrl(source?.id, 'browse'), + { ...(write ? groupWriteBody(write) : {}), browse_path: browsePath }, + ); + return browseFromResponse(response); }, - ignorePath: () => { - throw new Error(NOT_IN_PERSONAL); + ignorePath: async (sourceId, remotePath, ignored) => { + const response = await api.post( + personalFileSourcesUrl(sourceId, 'ignore-path'), { remote_path: remotePath, ignored }, + ); + return ignoreItemFromResponse(response); }, - classicPath: '/workspace', }; function groupFileSourcesUrl(groupId: string, sourceId?: string, suffix?: string): string { @@ -323,7 +353,20 @@ function sourceFromResponse(value: unknown): WorkspaceSyncSource { function optionsFromResponse(value: unknown): FileSourceOptions { if (!isRecord(value) || !Array.isArray(value.source_types) || !isRecord(value.eligible_identity_ids) - || !isRecord(value.schedule) || !isRecord(value.limits)) { + || !value.source_types.every((type) => isRecord(type) && typeof type.value === 'string' + && typeof type.label === 'string' && typeof type.visible === 'boolean') + || !Object.values(value.eligible_identity_ids).every((ids) => + Array.isArray(ids) && ids.every((id) => typeof id === 'string')) + || !isRecord(value.schedule) || !isRecord(value.limits) + || typeof value.schedule.min_interval_minutes !== 'number' + || !Number.isInteger(value.schedule.min_interval_minutes) || value.schedule.min_interval_minutes < 1 + || typeof value.schedule.max_interval_minutes !== 'number' + || !Number.isInteger(value.schedule.max_interval_minutes) + || value.schedule.max_interval_minutes < value.schedule.min_interval_minutes + || typeof value.limits.max_sources !== 'number' || !Number.isInteger(value.limits.max_sources) + || value.limits.max_sources < 1 || typeof value.recursive_allowed !== 'boolean' + || (value.default_remote_delete_policy !== undefined + && !['ignore', 'hard_delete'].includes(String(value.default_remote_delete_policy)))) { throw new Error('The file source options response was malformed. Refresh and try again.'); } return value as unknown as FileSourceOptions; @@ -345,7 +388,7 @@ function runsFromResponse(value: unknown): WorkspaceSyncRun[] { } function connectionFromResponse(value: unknown): FileSourceConnectionResult { - if (isRecord(value) && isRecord(value.connection)) { + if (isRecord(value) && isRecord(value.connection) && value.connection.success === true) { return value.connection as FileSourceConnectionResult; } throw new Error('The connection test response was malformed. Try again.'); @@ -362,7 +405,7 @@ function browseFromResponse(value: unknown): FileSourceBrowseResult { } function ignoreItemFromResponse(value: unknown): FileSourceIgnoreItem { - if (isRecord(value) && isRecord(value.item)) { + if (isRecord(value) && isRecord(value.item) && typeof value.item.ignored === 'boolean') { return value.item as FileSourceIgnoreItem; } throw new Error('The ignore-path response was malformed. Try again.'); @@ -394,12 +437,20 @@ function groupWriteBody(write: FileSourceWrite): Record { } function deleteOutcome(value: unknown): FileSourceDeleteOutcome { - const result = isRecord(value) ? value : {}; + if (!isRecord(value) || typeof value.associated_files_requested !== 'boolean' + || typeof value.documents_deleted !== 'number' || !Number.isInteger(value.documents_deleted) + || value.documents_deleted < 0 + || typeof value.documents_skipped !== 'number' || !Number.isInteger(value.documents_skipped) + || value.documents_skipped < 0 + || typeof value.documents_failed !== 'number' || !Number.isInteger(value.documents_failed) + || value.documents_failed < 0) { + throw new Error('The deletion response could not be verified. Refresh to confirm the source and document status.'); + } return { - associated_files_requested: Boolean(result.associated_files_requested), - documents_deleted: Number(result.documents_deleted ?? 0), - documents_skipped: Number(result.documents_skipped ?? 0), - documents_failed: Number(result.documents_failed ?? 0), + associated_files_requested: value.associated_files_requested, + documents_deleted: value.documents_deleted, + documents_skipped: value.documents_skipped, + documents_failed: value.documents_failed, }; } @@ -553,7 +604,6 @@ export function createGroupFileSourceWorkbench( ); return ignoreItemFromResponse(response); }, - classicPath: '/group_workspaces', }; } @@ -662,6 +712,5 @@ export function createPublicFileSourceWorkbench( ); return ignoreItemFromResponse(response); }, - classicPath: '/public_workspaces', }; } diff --git a/application/v2_ui/src/lib/types.ts b/application/v2_ui/src/lib/types.ts index 7032a1db6..2d53cc486 100644 --- a/application/v2_ui/src/lib/types.ts +++ b/application/v2_ui/src/lib/types.ts @@ -502,7 +502,7 @@ export interface FileSourceTypeOption { visible: boolean; } -/** The server-decided options envelope for the group file source editor. */ +/** Server-decided editor options in any workspace scope. */ export interface FileSourceOptions { source_types: FileSourceTypeOption[]; /** Identity ids eligible per source type; the picker filters against this. */ @@ -510,6 +510,7 @@ export interface FileSourceOptions { schedule: { min_interval_minutes: number; max_interval_minutes: number }; limits: { max_sources: number }; recursive_allowed: boolean; + default_remote_delete_policy?: 'ignore' | 'hard_delete'; } /** diff --git a/application/v2_ui/src/pages/workspace/FileSourcesSection.tsx b/application/v2_ui/src/pages/workspace/FileSourcesSection.tsx index 7127a18a0..2790e12e3 100644 --- a/application/v2_ui/src/pages/workspace/FileSourcesSection.tsx +++ b/application/v2_ui/src/pages/workspace/FileSourcesSection.tsx @@ -1,231 +1,16 @@ // FileSourcesSection.tsx -// External file sources that feed the documents section. -// -// Called "Sync" in the classic interface, which describes the mechanism rather than the -// purpose. What a user is doing here is pointing the workspace at somewhere their files -// already live, so the section is named for that and says where the files end up. +// Personal file sources use the same native editor as shared workspaces. -import { useMemo, useState } from 'react'; -import { Link } from 'react-router-dom'; -import { ChevronDown, ChevronRight, FolderSync, Play, Trash2 } from 'lucide-react'; -import { - ConfirmAction, - Pill, - ResourceRow, - RowAction, - SectionIntro, - SectionList, - SectionSearch, -} from '../../components/workspace/primitives'; -import { - errorMessage, - useSectionResource, -} from '../../components/workspace/useSectionResource'; -import { - deleteSyncSource, - fetchSyncRuns, - fetchSyncSources, - startSyncRun, -} from '../../lib/workspaceApi'; -import { statusTone } from './WorkflowsSection'; -import type { WorkspaceSyncRun, WorkspaceSyncSource } from '../../lib/types'; +import { PERSONAL_FILE_SOURCE_WORKBENCH } from '../../lib/fileSourceWorkbench'; +import { FileSourcesWorkbenchSection } from './GroupFileSourcesSection'; -const SOURCE_TYPE_LABELS: Record = { - smb: 'Network share', - azure_files: 'Azure Files', - azure_blob: 'Azure Blob Storage', - onedrive: 'OneDrive', - google_drive: 'Google Drive', - google_shared_drive: 'Google shared drive', -}; - -export function sourceTypeLabel(sourceType: unknown): string { - const raw = String(sourceType ?? '').trim(); - return SOURCE_TYPE_LABELS[raw] ?? (raw ? raw.replace(/[_-]+/g, ' ') : 'Unknown'); -} - -function formatTimestamp(value: unknown): string { - const raw = String(value ?? ''); - if (!raw) { - return ''; - } - const parsed = new Date(raw); - return Number.isNaN(parsed.valueOf()) ? raw : parsed.toLocaleString(); -} - -function SourceRuns({ sourceId }: { sourceId: string }) { - const { items, loading, error } = useSectionResource( - (signal) => fetchSyncRuns(sourceId, signal), - 'Failed to load sync history.', - ); - - if (loading) { - return

Loading history…

; - } - if (error) { - return

{error}

; - } - if (items.length === 0) { - return

This source has not run yet.

; - } - - return ( -
    - {items.slice(0, 10).map((run) => ( -
  • - {String(run.status ?? 'unknown')} - - {formatTimestamp(run.started_at) || 'Not started'} - {run.completed_at ? ` → ${formatTimestamp(run.completed_at)}` : ''} - -
    • - ))} -
- ); -} +export { sourceTypeLabel } from '../../lib/fileSourceFields'; export function FileSourcesSection() { - const { items, loading, error, refresh, setItems, setError } = - useSectionResource( - fetchSyncSources, - 'Failed to load file sources.', - ); - - const [query, setQuery] = useState(''); - const [busyId, setBusyId] = useState(null); - const [expandedId, setExpandedId] = useState(null); - - const visible = useMemo(() => { - const needle = query.trim().toLowerCase(); - if (!needle) { - return items; - } - return items.filter((source) => - `${source.name ?? ''} ${source.remote_path ?? ''}`.toLowerCase().includes(needle), - ); - }, [items, query]); - - const onRun = async (source: WorkspaceSyncSource) => { - setBusyId(source.id); - setError(null); - try { - await startSyncRun(source.id); - await refresh(); - } catch (runError) { - setError(errorMessage(runError, 'Could not start the sync.')); - } finally { - setBusyId(null); - } - }; - - const onDelete = async (source: WorkspaceSyncSource) => { - const previous = items; - setBusyId(source.id); - setItems(items.filter((item) => item.id !== source.id)); - try { - // The documents this source produced are deliberately kept. Removing a - // connection and discarding everything it ever brought in are different - // intentions, and only the first one was expressed here. - await deleteSyncSource(source.id, false); - } catch (deleteError) { - setItems(previous); - setError(errorMessage(deleteError, 'Could not delete the file source.')); - } finally { - setBusyId(null); - } - }; - return ( -
- - -

- Files land in{' '} - - Documents - - . Adding and configuring a source is still done in the{' '} - - classic workspace - - . -

- - - - } - emptyTitle={ - items.length === 0 ? 'No file sources yet' : 'No sources match your search' - } - emptyDescription={ - items.length === 0 - ? 'Connect a source to bring documents in without uploading them one by one.' - : undefined - } - getKey={(source, index) => String(source.id ?? index)} - renderItem={(source) => { - const expanded = expandedId === source.id; - return ( -
- } - title={String(source.name ?? 'Untitled source')} - subtitle={String(source.remote_path ?? '')} - meta={ - <> - {sourceTypeLabel(source.source_type)} - - {source.enabled ? 'Enabled' : 'Paused'} - - - } - actions={ - <> - - ) : ( - - ) - } - label={ - expanded - ? 'Hide sync history' - : 'Show sync history' - } - onClick={() => - setExpandedId(expanded ? null : source.id) - } - /> - } - label={`Sync ${source.name ?? 'source'} now`} - busy={busyId === source.id} - onClick={() => void onRun(source)} - /> - } - label={`Delete ${source.name ?? 'source'}`} - confirmLabel="Delete" - busy={busyId === source.id} - onConfirm={() => void onDelete(source)} - /> - - } - /> - {expanded ? : null} -
- ); - }} - /> -
+ ); } diff --git a/application/v2_ui/src/pages/workspace/GroupFileSourcesSection.tsx b/application/v2_ui/src/pages/workspace/GroupFileSourcesSection.tsx index 42a300463..e8157722b 100644 --- a/application/v2_ui/src/pages/workspace/GroupFileSourcesSection.tsx +++ b/application/v2_ui/src/pages/workspace/GroupFileSourcesSection.tsx @@ -14,7 +14,8 @@ // chooses whether to remove them, the counts are shown, and a refusal that already removed the // documents says so instead of claiming nothing changed. -import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { Link } from 'react-router-dom'; import { ChevronDown, ChevronRight, FolderSync, Pencil, Play, Plus, Trash2 } from 'lucide-react'; import { Pill, @@ -40,17 +41,19 @@ import { } from '../../lib/fileSourceWorkbench'; import { buildFileSourceWrite, + connectionDescriptor, draftFromSource, emptyFileSourceDraft, sourcePathText, + sourceTypeLabel, visibleSourceTypes, FILE_SOURCE_REBASE_FIELDS, type FileSourceDraft, } from '../../lib/fileSourceFields'; import { rebaseDraft, rebaseNotice, REBASE_DELETED_NOTICE } from '../../lib/rebaseDraft'; -import { sourceTypeLabel } from './FileSourcesSection'; import { statusTone } from './WorkflowsSection'; import { toast } from '../../stores/toastStore'; +import { ApiError, safeSameOriginUrl } from '../../lib/apiClient'; import type { FileSourceOptions, WorkspaceIdentity, @@ -123,7 +126,7 @@ interface DeleteState { result: FileSourceDeleteOutcome | null; } -export function GroupFileSourcesSection({ +export function FileSourcesWorkbenchSection({ adapter, scopeNoun = 'group', }: { @@ -153,9 +156,15 @@ export function GroupFileSourcesSection({ // A conditional-write conflict on save: the draft stays open and a reload is offered. const [saveConflict, setSaveConflict] = useState(false); const [editorLoading, setEditorLoading] = useState(false); + const [editorOpen, setEditorOpen] = useState(false); + const [editorTarget, setEditorTarget] = useState(null); + const [editingSource, setEditingSource] = useState(null); + const [sourceDeleted, setSourceDeleted] = useState(false); + const editorRequest = useRef(null); + const editorOpener = useRef(null); const [options, setOptions] = useState(null); const [identities, setIdentities] = useState([]); - // The group's existing tags, offered as fixed-tag suggestions. They are optional: a failed read + // Existing workspace tags are optional suggestions; a failed read // only costs the suggestions, so the editor still opens and a tag can still be typed. const [tagSuggestions, setTagSuggestions] = useState([]); const [tagSuggestionsFailed, setTagSuggestionsFailed] = useState(false); @@ -163,6 +172,14 @@ export function GroupFileSourcesSection({ const [deleteState, setDeleteState] = useState(null); const canCreate = adapter.allows('create'); + const documentsHref = adapter.scope.kind === 'personal' + ? '/workspace/documents' + : adapter.scope.kind === 'group' + ? `/groups/${encodeURIComponent(adapter.scope.id)}/documents` + : `/public/${encodeURIComponent(adapter.scope.id)}/documents`; + + // Scope wrappers remount for another target; same-scope permission refreshes keep the draft. + useEffect(() => () => editorRequest.current?.abort(), []); const visible = useMemo(() => { const needle = query.trim().toLowerCase(); @@ -175,60 +192,92 @@ export function GroupFileSourcesSection({ }, [items, query]); const openEditor = async (source: WorkspaceSyncSource | null) => { + if (!editorOpen) { + editorOpener.current = document.activeElement instanceof HTMLElement ? document.activeElement : null; + } + editorRequest.current?.abort(); + const controller = new AbortController(); + editorRequest.current = controller; + setEditorOpen(true); + setEditorTarget(source); + setDraft(null); + setBaseline(null); + setEditingSource(null); + setSaving(false); + setSourceDeleted(false); setSaveError(null); setSaveConflict(false); setEditingId(source ? source.id : null); setEditorLoading(true); - // A blank draft while options load, so the dialog can open immediately. - setDraft(source ? draftFromSource(source, 1) : emptyFileSourceDraft('smb', 1)); + setTagSuggestions([]); setTagSuggestionsFailed(false); - void adapter.tags().then(setTagSuggestions, () => { - setTagSuggestions([]); - setTagSuggestionsFailed(true); + void adapter.tags(controller.signal).then((tags) => { + if (!controller.signal.aborted) setTagSuggestions(tags); + }, () => { + if (!controller.signal.aborted) setTagSuggestionsFailed(true); }); try { - const [loadedOptions, loadedIdentities] = await Promise.all([ - adapter.options(), - adapter.identities(), + const [loadedOptions, loadedIdentities, loadedSource] = await Promise.all([ + adapter.options(controller.signal), + adapter.identities(controller.signal), + source ? adapter.read(source, controller.signal) : Promise.resolve(null), ]); + if (controller.signal.aborted) return; + if (!loadedOptions) { + throw new Error('The editor options are unavailable. Retry to load them.'); + } setOptions(loadedOptions); setIdentities(loadedIdentities); const minInterval = loadedOptions?.schedule?.min_interval_minutes ?? 1; - if (source) { - setDraft(draftFromSource(source, minInterval)); - setBaseline(draftFromSource(source, minInterval)); + if (loadedSource) { + connectionDescriptor(String(loadedSource.source_type)); + const loadedDraft = draftFromSource(loadedSource, minInterval); + setDraft(loadedDraft); + setBaseline(loadedDraft); + setEditingSource(loadedSource); } else { - const firstType = visibleSourceTypes(loadedOptions)[0]?.value ?? 'smb'; - setDraft(emptyFileSourceDraft(firstType, minInterval)); + const firstType = visibleSourceTypes(loadedOptions)[0]?.value; + if (!firstType) { + throw new Error('No source types are available. Ask an administrator to review File Sync settings.'); + } + connectionDescriptor(firstType); + setDraft({ + ...emptyFileSourceDraft(firstType, minInterval), + recursive: loadedOptions.recursive_allowed, + remoteDeletePolicy: loadedOptions.default_remote_delete_policy ?? 'ignore', + }); } } catch (loadError) { - setSaveError(errorMessage(loadError, 'Could not load the editor options.')); + if (!controller.signal.aborted) setSaveError(errorMessage(loadError, 'Could not load the editor options.')); } finally { - setEditorLoading(false); + if (!controller.signal.aborted) setEditorLoading(false); } }; const closeEditor = () => { + editorRequest.current?.abort(); + setEditorOpen(false); setDraft(null); setBaseline(null); setEditingId(null); + setEditingSource(null); + setSaving(false); setSaveError(null); setSaveConflict(false); }; const onSave = async () => { - if (!draft) { + if (!draft || sourceDeleted) { return; } + const controller = editorRequest.current; setSaving(true); setSaveError(null); setSaveConflict(false); try { const write = buildFileSourceWrite(draft); if (editingId) { - // The config_revision rides on the current list row, so a save retried after a - // reload picks up the fresh marker without the draft having to carry it. - const current = items.find((source) => source.id === editingId); + const current = editingSource; if (!current) { throw new Error('This file source is no longer available. Reload and try again.'); } @@ -236,10 +285,12 @@ export function GroupFileSourcesSection({ } else { await adapter.create(write); } + if (controller?.signal.aborted) return; closeEditor(); await refresh(); toast.success(editingId ? 'File source saved' : 'File source created'); } catch (writeError) { + if (controller?.signal.aborted) return; if (writeError instanceof FileSourceConflictError) { // The config revision moved. Keep the draft open. Reload brings the new // config_revision, and saving again applies the edit on top of it. @@ -256,7 +307,7 @@ export function GroupFileSourcesSection({ setSaveError(errorMessage(writeError, 'Could not save the file source.')); } } finally { - setSaving(false); + if (!controller?.signal.aborted) setSaving(false); } }; @@ -267,47 +318,60 @@ export function GroupFileSourcesSection({ await refresh(); return; } + const controller = editorRequest.current; + setSaving(true); try { - const fresh = await adapter.list(new AbortController().signal); - setItems(fresh); - const current = fresh.find((source) => source.id === editingId) ?? null; - if (!current) { - setSaveConflict(false); - setSaveError(REBASE_DELETED_NOTICE); - return; - } + if (!editingSource) throw new Error(REBASE_DELETED_NOTICE); + const current = await adapter.read(editingSource, controller?.signal); + if (controller?.signal.aborted) return; + setItems(items.map((source) => source.id === current.id ? current : source)); const minInterval = options?.schedule?.min_interval_minutes ?? 1; const freshDraft = draftFromSource(current, minInterval); const { draft: rebased, conflicts } = rebaseDraft(baseline, freshDraft, draft, FILE_SOURCE_REBASE_FIELDS); setDraft(rebased); setBaseline(freshDraft); + setEditingSource(current); setSaveConflict(false); setSaveError(rebaseNotice(conflicts)); } catch (reloadError) { + if (controller?.signal.aborted) return; + if (reloadError instanceof ApiError && reloadError.status === 404) { + setSourceDeleted(true); + setSaveConflict(false); + setSaveError(REBASE_DELETED_NOTICE); + return; + } setSaveError(errorMessage(reloadError, 'Could not reload the latest version.')); + } finally { + if (!controller?.signal.aborted) setSaving(false); } }; const onTest = async () => { + if (sourceDeleted) throw new Error(REBASE_DELETED_NOTICE); if (!draft) { throw new Error('There is nothing to test yet.'); } const write = buildFileSourceWrite(draft); - const saved = editingId ? items.find((source) => source.id === editingId) ?? null : null; + const saved = editingSource; + if (editingId && !saved) throw new Error(REBASE_DELETED_NOTICE); return adapter.testConnection(saved, write); }; const onBrowse = async (browsePath: string) => { + if (sourceDeleted) throw new Error(REBASE_DELETED_NOTICE); if (!draft) { throw new Error('There is nothing to browse yet.'); } const write = buildFileSourceWrite(draft); - const saved = editingId ? items.find((source) => source.id === editingId) ?? null : null; + const saved = editingSource; + if (editingId && !saved) throw new Error(REBASE_DELETED_NOTICE); return adapter.browse(saved, write, browsePath); }; const onIgnore = editingId ? async (remotePath: string, ignored: boolean) => { + if (sourceDeleted) throw new Error(REBASE_DELETED_NOTICE); const item = await adapter.ignorePath(editingId, remotePath, ignored); return Boolean(item.ignored); } @@ -411,6 +475,8 @@ export function GroupFileSourcesSection({ {canCreate ? 'Connect a source to bring documents in without uploading them one by one.' : `A workspace manager can connect a source for this ${scopeNoun}.`} + {' '}Files appear in{' '} + Documents.

@@ -493,15 +559,38 @@ export function GroupFileSourcesSection({ }} /> + {editorOpen && !draft ? ( + + Cancel + {!editorLoading ? ( + void openEditor(editorTarget)}> + Retry editor + + ) : null} + + } + > + {editorLoading ?

Loading file source configuration…

+ :

{saveError}

} + + ) : null} + {draft ? ( void onSave()} @@ -548,6 +637,7 @@ export function GroupFileSourcesSection({ } + >

@@ -571,6 +661,17 @@ export function GroupFileSourcesSection({ ); } +export function GroupFileSourcesSection({ + adapter, + scopeNoun = 'group', +}: { + adapter: FileSourceWorkbenchAdapter; + scopeNoun?: string; +}) { + const key = adapter.scope.kind === 'personal' ? 'personal' : `${adapter.scope.kind}:${adapter.scope.id}`; + return ; +} + /** * The file sources workbench bound to a public workspace (M10B). * diff --git a/docs/explanation/features/V2_FILE_SOURCE_CONFIGURATION.md b/docs/explanation/features/V2_FILE_SOURCE_CONFIGURATION.md new file mode 100644 index 000000000..bfd5e2607 --- /dev/null +++ b/docs/explanation/features/V2_FILE_SOURCE_CONFIGURATION.md @@ -0,0 +1,96 @@ +# V2 File Source Configuration - 0.261.310 + +## Overview + +Implemented in version: **0.261.310**, recorded in +`application/single_app/config.py`. Refs +[#1722](https://github.com/microsoft/simplechat/issues/1722), row 4. + +Personal, group and public workspaces create and configure file sources +without opening a classic page. This removes the personal workspace's +configuration hand-off and reuses the already-native group/public editor. +File-source workflows no longer depend on legacy-page access in New UI only +mode; this change does not implement the separate legacy routing policy. + +Dependencies are the existing File Sync engine, Redis readiness, workspace +permissions, approved connectors and stored credentials or eligible identities. +No browser libraries, admin settings or connector types are added. + +## Technical specifications + +`FileSourcesWorkbenchSection` supplies the shared list and editor lifecycle. +`FileSourceEditorDialog` and `fileSourceFields.ts` keep connection, credential, +selection and filtering behavior identical across scopes. Each adapter uses +its scope's routes and response envelopes: + +| Scope | Sources | Options | Response keys | +|---|---|---|---| +| Personal | `/api/file-sync/personal/sources` | `/api/file-sync/personal/source-options` | `sources`, `source` | +| Group | `/api/groups//file-sources` | `/api/groups//file-source-options` | `file_sources`, `file_source` | +| Public | `/api/public-workspaces//file-sources` | `/api/public-workspaces//file-source-options` | `file_sources`, `file_source` | + +The personal source family adds a single-source GET for a fresh editor +baseline. Options return visible implemented connectors, same-scope identity +IDs, recursion policy, schedule limits, source limits and the new-source remote +delete default. They never return raw settings or secrets. + +Personal writes include the additive `expected_config_revision` token; +PATCH/DELETE callers without it remain compatible. The revision covers +configuration, not sync-run progress. A configuration conflict keeps the +draft and offers reload/merge; an etag-only write conflict permits retry. +Fresh secret references are staged, discarded on refused writes, and replace +old secrets only after a committed save. Browser projections expose stored +secret flags, never stored credentials or Key Vault references. + +Personal targets come from the authenticated user, including session roles +for admin-only/app-role policies. Group/public requests remain bound to their +immutable workspace ID and require management hints and row actions. Locked +or upload-disabled shared workspaces remain read-only. + +## Usage + +Choose **File sources → New file source**, or use **Edit** on a saved source. +Use a same-workspace identity or source-local credentials. SMB supports +anonymous and username/password; Azure storage supports managed identity, +service principal and connection string/SAS authentication. + +**Test connection** and **Browse the source** use the unsaved draft. Select +root-relative folders/files, or leave the selection empty to import the root. +Configure recursion, filename patterns, extensions, fixed/folder tags, remote +deletions, enabled state and scheduling. Creation saves without starting a sync. +Ignore/Restore is an immediate operation on a saved source and uses the +server's canonical remote file path, not the browse-relative path. + +Editing retains blank stored secrets. Reload after a conflict preserves locally +edited fields and adopts untouched remote changes. A deleted source cannot be +saved, tested, browsed or ignored. Loading failures offer retry; optional tag +suggestions can fail without preventing a typed tag. + +Connection changes discard stale test/browse feedback. Cancel/Escape protects +unsaved work, and focus returns to the original action after the dialog closes. +The editor uses the existing light/dark theme and scrollable mobile modal. + +See [Create a file sync](../../guides/create-a-file-sync.md) for the workflow. + +## Testing and limitations + +- `functional_tests/test_personal_file_source_apis.py` exercises real routes, + ownership, capability/role policies, connector/auth round trips, conditional + writes, secret staging, delete outcomes and classic compatibility. +- `functional_tests/test_v2_personal_file_sources.py` executes the real adapter + and shared serializers, including exact transport, malformed responses and + existing OneDrive configuration safety. +- `ui_tests/test_v2_personal_file_sources.py` exercises the production SPA; + existing group/public suites cover shared-scope gates, editing and deletion. +- Route policy tests cover both new personal reads. Fixture parity checks + keep options aligned with the actual backend. + +New connector availability stays administrator-controlled: SMB, Azure Files +and Azure Blob are the implemented selectable types. An existing personal +OneDrive source retains selected paths and the administrator-managed global +connector; V2 does not enable new OneDrive or coming-soon sources. Identity +creation is separate work. + +The checked-out app still uses the `/v2` basename. Live root-hosted/Teams +validation and New UI only enforcement depend on the companion routing work +and a configured deployment; synthetic tests do not claim those checks. diff --git a/docs/explanation/features/V2_GROUP_FILE_SOURCES.md b/docs/explanation/features/V2_GROUP_FILE_SOURCES.md index eeaeec7d9..336712d0b 100644 --- a/docs/explanation/features/V2_GROUP_FILE_SOURCES.md +++ b/docs/explanation/features/V2_GROUP_FILE_SOURCES.md @@ -5,6 +5,12 @@ Implemented in version: **0.261.147**, tracked in `application/single_app/config.py`. +**Shared configuration expanded in version: 0.261.310.** The same native +workbench now serves personal sources too, without changing the group's route +or authorization rules. Edit loads a fresh source and options before opening +the form; a failed load offers retry rather than a guessed configuration. +See [V2 File Source Configuration](V2_FILE_SOURCE_CONFIGURATION.md). + File sources are the connections a group syncs documents from: an SMB share, an Azure Files share, or an Azure Blob Storage container. The documents they bring in are processed like uploads and appear in the group's documents. The native V2 diff --git a/docs/explanation/features/V2_MY_WORKSPACE.md b/docs/explanation/features/V2_MY_WORKSPACE.md index 787679469..560a6fcc0 100644 --- a/docs/explanation/features/V2_MY_WORKSPACE.md +++ b/docs/explanation/features/V2_MY_WORKSPACE.md @@ -15,6 +15,10 @@ serve file sources and actions. **Implemented in version:** 0.261.042 +**File-source configuration implemented in version: 0.261.310**, recorded in +`application/single_app/config.py`. Personal, group and public sources now use +the native shared editor; see [V2 File Source Configuration](V2_FILE_SOURCE_CONFIGURATION.md). + **Agent/action authoring expanded in version:** 0.261.096, recorded in `application/single_app/config.py`. @@ -169,7 +173,7 @@ What each section supports in this release: | Section | Available now | |---|---| | Documents | Upload, search, filter by tag, delete | -| File sources | List, sync now, run history, delete | +| File sources | Native create/edit, credentials or saved identities, test/browse, selected paths, filters, tags, schedules, sync now, run history, delete | | Prompts | Full create, edit and delete | | Agents | Full-page authoring for all permitted agent types, models, knowledge, actions, instructions, and templates; use in chat | | Actions | Unified list and full-page create/edit/delete, with native type-specific configuration | @@ -187,7 +191,7 @@ The following separate management surfaces still use their existing editors: - The workflow designer: tasks, document actions and scheduling. - Model endpoint connection details: provider, API versions, authentication, model list. -- File source configuration, and identity creation with its auth-type-specific fields. +- Identity creation with its auth-type-specific fields is separate from file-source configuration. ## Testing and validation diff --git a/docs/explanation/features/V2_PUBLIC_CONNECTIONS.md b/docs/explanation/features/V2_PUBLIC_CONNECTIONS.md index 08fd9df6f..fd4f3669f 100644 --- a/docs/explanation/features/V2_PUBLIC_CONNECTIONS.md +++ b/docs/explanation/features/V2_PUBLIC_CONNECTIONS.md @@ -11,6 +11,12 @@ no actions, so File Sync is what both sections are for. Implemented in version: **0.261.182**. The routes are in [Public Connection APIs](PUBLIC_CONNECTION_APIS.md). +**Shared source editor expanded in version: 0.261.310**, recorded in +`application/single_app/config.py`. Personal, group and public source editors +share loading, connection validation and draft recovery while keeping +scope-specific routes and permissions. See +[V2 File Source Configuration](V2_FILE_SOURCE_CONFIGURATION.md). + ## Who can do what | | Owner, Admin, DocumentManager | Reader | diff --git a/docs/guides/create-a-file-sync.md b/docs/guides/create-a-file-sync.md index fb4519825..a9a48b245 100644 --- a/docs/guides/create-a-file-sync.md +++ b/docs/guides/create-a-file-sync.md @@ -25,7 +25,41 @@ Use File Sync when important documents already live in a share or storage contai - The workspace scope must be enabled on [Workspaces settings]({{ '/admin/workspaces/' | relative_url }}). - You need permission to manage sync sources and a readable credential or **Reusable identity**. -## Steps +## V2 workflow + +Native file-source configuration is available from version **0.261.310** in +personal, group and public workspaces. This flow does not open a classic page. + +1. Open the destination workspace's **File sources** section and choose + **New file source**. In shared workspaces, management requires Owner, Admin + or DocumentManager permissions and an active workspace. +2. Name the source and choose an administrator-approved **Network share**, + **Azure Files**, or **Azure Blob Storage** connection. Enter its root path, + service URL, share or container, and optional directory or prefix. +3. Select a saved identity from this workspace or enter credentials directly. + SMB accepts anonymous or username/password access; Azure storage accepts + managed identity, a service principal, or a connection string/SAS. +4. Use **Test connection** to check the unsaved connection. **Browse the source** + lets you choose folders and files relative to its root. An empty selection + syncs the root; select paths to limit the import. +5. Configure subfolders, include/exclude patterns, extensions, fixed tags and + folder tags. Choose whether a later remote deletion keeps or deletes the + SimpleChat copy. Enable a schedule only when regular refresh is useful; + the interval must meet the administrator's limits. +6. Choose **Create source**. Creation saves the configuration; it does not start + a sync. Use the row's **Sync now** action when ready to import documents. + +Use a source's **Edit** action to change its configuration. Stored passwords +and secrets open blank: leave them blank to retain the stored value, or type +a replacement. If another writer changed the source, **Reload** adopts their +untouched fields while keeping your edits for review before saving again. +Ignore/Restore on a saved source changes its ignore list immediately, even +before the configuration is saved. + +Coming-soon connectors are not newly enabled by V2. Creating an identity is a +separate workflow; the source editor selects an existing eligible identity. + +## Classic workflow 1. Open the workspace that should receive synced documents. 2. Choose **Sync** from **Section** or the workspace tabs. @@ -36,7 +70,7 @@ Use File Sync when important documents already live in a share or storage contai title="Create a file sync step 3" capture="Capture the create a file sync task at this step in SimpleChat with realistic sample data and redact secrets." %} -4. On **1. Source Type**, choose **SMB Share**, **Azure Files**, **Azure Blob Storage**, or another available card. +4. On **1. Source Type**, choose an available **SMB Share**, **Azure Files**, or **Azure Blob Storage** card. 5. Select **Configure Source**. 6. In **General**, enter **Source name** and connection fields such as **UNC path**, **File service URL**, **Share name**, **Container name**, or **Blob prefix**. 7. In **Identity and Authentication**, choose **Reusable identity** or source-local credentials. diff --git a/functional_tests/route_tests/test_route_blueprint_policy_inventory.py b/functional_tests/route_tests/test_route_blueprint_policy_inventory.py index ed8436332..9ff9d61ce 100644 --- a/functional_tests/route_tests/test_route_blueprint_policy_inventory.py +++ b/functional_tests/route_tests/test_route_blueprint_policy_inventory.py @@ -2,7 +2,7 @@ #!/usr/bin/env python3 """ Functional test for route blueprint policy inventory. -Version: 0.261.305 +Version: 0.261.310 Implemented in: 0.242.069 Plan editor policy coverage: 0.261.102 Selected-group context policy coverage: 0.261.126 @@ -160,6 +160,8 @@ } SENSITIVE_ROUTE_POLICIES = { + ("route_backend_file_sync.py", "api_file_sync_personal_source_options"): ("login_required", "user_required", "enabled_required"), + ("route_backend_file_sync.py", "api_file_sync_personal_source_read"): ("login_required", "user_required", "enabled_required"), ("route_backend_control_center.py", "api_v2_control_center_dashboard_summary"): ("login_required", "control_center_required"), ("route_backend_control_center.py", "api_v2_control_center_dashboard_insights"): ("login_required", "control_center_required"), ("route_backend_control_center.py", "api_v2_control_center_dashboard_chat_readiness"): ("login_required", "control_center_required"), diff --git a/functional_tests/test_group_file_source_fixture_parity.py b/functional_tests/test_group_file_source_fixture_parity.py index ddd6d45d3..1facb0279 100644 --- a/functional_tests/test_group_file_source_fixture_parity.py +++ b/functional_tests/test_group_file_source_fixture_parity.py @@ -1,7 +1,7 @@ # test_group_file_source_fixture_parity.py """ Per-route shape parity between the M5B group file source UI fixture and the real routes. -Version: 0.261.172 +Version: 0.261.310 Implemented in: 0.261.147 Credentials block compared: 0.261.156 Sync fields and browse paths compared by value: 0.261.171 @@ -85,7 +85,7 @@ FILE_ENTRY_UI_KEYS = ENTRY_UI_KEYS | {"remote_path"} IGNORE_ITEM_UI_KEYS = {"id", "remote_path", "status", "ignored"} CONNECTION_UI_KEYS = {"success", "entries_checked", "files_seen", "folders_seen"} -OPTIONS_UI_KEYS = {"source_types", "eligible_identity_ids", "schedule", "limits", "recursive_allowed"} +OPTIONS_UI_KEYS = {"source_types", "eligible_identity_ids", "schedule", "limits", "recursive_allowed", "default_remote_delete_policy"} # The credential fields the editor's draftFromSource reads. A key the server omits would open blank # and be sent back blank, which clears the stored value (the 0.261.156 tenant fix). CREDENTIALS_UI_KEYS = { diff --git a/functional_tests/test_personal_file_source_apis.py b/functional_tests/test_personal_file_source_apis.py new file mode 100644 index 000000000..034dab89b --- /dev/null +++ b/functional_tests/test_personal_file_source_apis.py @@ -0,0 +1,295 @@ +# test_personal_file_source_apis.py +""" +Functional tests for native personal File Sync configuration. +Version: 0.261.310 +Implemented in: 0.261.310 + +Real routes and the real storage/credential engine run against isolated Cosmos and +Key Vault doubles, with network access blocked by the shared file-source harness. +""" + +from copy import deepcopy +from unittest.mock import Mock + +import pytest + +from test_support.group_file_source_harness import as_user, environment, smb_payload +from test_support.personal_file_source_harness import ( + LIST_PATH, OPTIONS_PATH, create_personal_source, personal_environment, +) + + +def stored(env, source): + return env.sources_container.get("owner", source["id"]) + + +def test_personal_options_are_safe_and_authoritative(personal_environment): + env = personal_environment + env.settings.update({ + "file_sync_visible_source_types": ["smb", "azure_blob", "onedrive", "google_workspace"], + "file_sync_min_schedule_interval_minutes": 17, + "file_sync_allow_recursive_sources": False, + "file_sync_default_remote_delete_policy": "hard_delete", + }) + response = env.client.get(OPTIONS_PATH) + assert response.status_code == 200 + options = response.get_json() + assert [item["value"] for item in options["source_types"] if item["visible"]] == ["smb", "azure_blob"] + assert options["schedule"] == {"min_interval_minutes": 17, "max_interval_minutes": 10080} + assert options["recursive_allowed"] is False + assert options["default_remote_delete_policy"] == "hard_delete" + assert "key_vault_name" not in options and "redis_url" not in options + + +@pytest.mark.parametrize("path", [LIST_PATH, OPTIONS_PATH, f"{LIST_PATH}/foreign"]) +@pytest.mark.parametrize("setting", ["enable_file_sync", "enable_file_sync_personal", "file_sync_personal_admin_only", "file_sync_personal_require_app_role"]) +def test_personal_capability_is_rechecked(personal_environment, path, setting): + env = personal_environment + env.settings[setting] = setting.endswith("admin_only") or setting.endswith("require_app_role") + response = env.client.get(path) + assert response.status_code == (400 if setting == "enable_file_sync" else 403) + + +def test_app_role_allows_options(personal_environment): + env = personal_environment + env.settings["file_sync_personal_require_app_role"] = True + as_user(env, "owner", roles=("User", env.filesync.FILE_SYNC_PERSONAL_APP_ROLE)) + response = env.client.get(OPTIONS_PATH) + assert response.status_code == 200 + + +def test_sources_are_owner_bound_and_masked(personal_environment): + env = personal_environment + source = create_personal_source(env) + assert source["config_revision"] and "_etag" not in source and "auth" not in source + read = env.client.get(f"{LIST_PATH}/{source['id']}") + assert read.get_json()["source"]["config_revision"] == source["config_revision"] + assert read.get_json()["source"]["credentials"]["password_stored"] is True + as_user(env, "stranger") + listing = env.client.get(LIST_PATH) + read = env.client.get(f"{LIST_PATH}/{source['id']}") + update = env.client.patch(f"{LIST_PATH}/{source['id']}", json={"name": "Stolen"}) + assert listing.get_json()["sources"] == [] + assert read.status_code == 404 and update.status_code == 404 + + +@pytest.mark.parametrize("source_type,connection,credentials", [ + ("smb", {"unc_path": "\\\\files\\share"}, {"auth_type": "anonymous"}), + ("azure_files", {"account_url": "https://storage.file.core.windows.net", "share_name": "reports", "directory_path": "2026"}, {"auth_type": "managed_identity", "managed_identity_client_id": "managed-client"}), + ("azure_blob", {"account_url": "https://storage.blob.core.windows.net", "container_name": "reports", "blob_prefix": "2026"}, {"auth_type": "managed_identity"}), + ("azure_files", {"account_url": "https://storage.file.core.windows.net", "share_name": "reports"}, {"auth_type": "client_secret", "identity": "client", "tenant_id": "tenant", "secret": "fixture-client-secret"}), + ("azure_files", {"account_url": "https://storage.file.core.windows.net", "share_name": "reports"}, {"auth_type": "connection_string", "connection_string": "fixture-connection-string"}), + ("azure_blob", {"account_url": "https://storage.blob.core.windows.net", "container_name": "reports"}, {"auth_type": "client_secret", "identity": "client", "tenant_id": "tenant", "secret": "fixture-client-secret"}), + ("azure_blob", {"account_url": "https://storage.blob.core.windows.net", "container_name": "reports"}, {"auth_type": "connection_string", "connection_string": "DefaultEndpointsProtocol=https;AccountName=storage;AccountKey=Zml4dHVyZS1vbmx5;EndpointSuffix=core.windows.net"}), +]) +def test_connector_configuration_round_trip(personal_environment, source_type, connection, credentials): + env = personal_environment + write = { + "name": f"{source_type} reports", "source_type": source_type, "connection": { + **connection, "selected_paths": ["Quarterly/report.pdf"], + }, "credentials": credentials, "recursive": True, + "filters": {"fixed_tags": ["finance"], "folder_tag_mode": "full_path", "include_patterns": ["*.pdf"], "exclude_patterns": ["drafts/*"], "allowed_extensions": ["pdf"]}, + "schedule": {"enabled": True, "interval_minutes": 60}, "remote_delete_policy": "hard_delete", + } + source = create_personal_source(env, write) + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": source["config_revision"], "name": "Renamed", + }) + assert response.status_code == 200, response.get_json() + reopened = response.get_json()["source"] + for field in ("connection", "filters", "remote_delete_policy", "credentials"): + assert reopened[field] == source[field] + assert reopened["schedule"]["interval_minutes"] == 60 + assert reopened["config_revision"] != source["config_revision"] + + +def test_blank_secret_is_kept_and_rotation_is_committed(personal_environment): + env = personal_environment + source = create_personal_source(env) + old_name = stored(env, source)["auth"]["password_secret_name"] + old_secret = env.state.vault[old_name] + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": source["config_revision"], + "credentials": {"auth_type": "username_password", "username": "svc", "password": ""}, + }) + assert response.status_code == 200 + assert stored(env, source)["auth"]["password_secret_name"] == old_name + revision = response.get_json()["source"]["config_revision"] + rotated = env.client.patch(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": revision, + "credentials": {"auth_type": "username_password", "username": "svc", "password": "fixture-rotated-password"}, + }) + assert rotated.status_code == 200 + new_name = stored(env, source)["auth"]["password_secret_name"] + assert new_name != old_name and old_name not in env.state.vault + assert env.state.vault[new_name] == "fixture-rotated-password" != old_secret + + +def test_conflict_preserves_live_secret_and_discards_staged_secret(personal_environment): + env = personal_environment + source = create_personal_source(env) + before = deepcopy(env.state.vault) + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": "stale", + "credentials": {"auth_type": "username_password", "password": "fixture-refused-password"}, + }) + assert response.status_code == 409 and response.get_json()["error_code"] == "config_conflict" + assert env.state.vault == before + assert stored(env, source)["name"] == source["name"] + assert env.state.secret_writes[-1][0] in env.state.secret_deletes + + +def test_engine_progress_does_not_conflict_with_edit(personal_environment): + env = personal_environment + source = create_personal_source(env) + def progress_underneath(): + record = env.sources_container.records[("owner", source["id"])] + record["last_run_status"] = "completed" + record["_etag"] = '"engine-progress"' + + env.sources_container.before_replace.append(progress_underneath) + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": source["config_revision"], "name": "Renamed", + }) + assert response.status_code == 200 + assert stored(env, source)["last_run_status"] == "completed" + + +def test_classic_no_token_update_and_delete_remain_supported(personal_environment): + env = personal_environment + source = create_personal_source(env) + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={"name": "Classic edit"}) + deleted = env.client.delete(f"{LIST_PATH}/{source['id']}", json={"delete_associated_files": False}) + assert response.status_code == 200 and response.get_json()["source"]["name"] == "Classic edit" + assert deleted.status_code == 200 + assert deleted.get_json()["delete_result"]["associated_files_requested"] is False + + +def test_hidden_type_is_not_created_and_bad_revision_is_refused(personal_environment): + env = personal_environment + env.settings["file_sync_visible_source_types"] = ["azure_files"] + response = env.client.post(LIST_PATH, json=smb_payload()) + assert response.status_code == 403 + env.settings["file_sync_visible_source_types"] = ["smb"] + source = create_personal_source(env) + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={"expected_config_revision": "", "name": "No"}) + assert response.status_code == 400 and stored(env, source)["name"] == source["name"] + + +def test_personal_identity_options_and_binding_reject_other_owner(personal_environment): + env = personal_environment + identities = env.identities._get_identities_container("personal") + for user_id, identifier in [("owner", "own-identity"), ("stranger", "foreign-identity")]: + identities.seed({ + "id": identifier, "user_id": user_id, "scope_type": "personal", "name": identifier, + "provider": "smb", "usage_contexts": ["file_sync"], "supported_source_types": ["smb"], + "auth": {"auth_type": "username_password", "username": "svc", "password": "fixture-identity-password"}, + }) + options = env.client.get(OPTIONS_PATH).get_json() + assert options["eligible_identity_ids"]["smb"] == ["own-identity"] + write = {**smb_payload(), "identity_id": "foreign-identity"} + response = env.client.post(LIST_PATH, json=write) + assert response.status_code in (400, 404) + write["identity_id"] = "own-identity" + source = create_personal_source(env, write) + assert source["identity_name"] == "own-identity" + + +def test_native_delete_conflict_and_busy_are_explicit(personal_environment): + env = personal_environment + source = create_personal_source(env) + response = env.client.delete(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": "stale", "delete_associated_files": False, + }) + assert response.status_code == 409 and response.get_json()["error_code"] == "config_conflict" + env.runs_container.seed({"id": "queued-run", "source_id": source["id"], "status": "queued"}) + response = env.client.delete(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": source["config_revision"], "delete_associated_files": False, + }) + assert response.status_code == 409 and response.get_json()["error_code"] == "source_busy" + assert stored(env, source) is not None + + +@pytest.mark.parametrize("gate", ["file_sync_personal_admin_only", "file_sync_personal_require_app_role"]) +def test_authorized_roles_are_taken_from_the_authenticated_session(personal_environment, gate): + env = personal_environment + env.settings[gate] = True + role = "Admin" if gate.endswith("admin_only") else env.filesync.FILE_SYNC_PERSONAL_APP_ROLE + as_user(env, "owner", roles=("User", role)) + response = env.client.get(OPTIONS_PATH) + assert response.status_code == 200 + + +def test_new_reads_refuse_targets_and_request_bodies(personal_environment): + env = personal_environment + source = create_personal_source(env) + for path in (OPTIONS_PATH, f"{LIST_PATH}/{source['id']}"): + targeted = env.client.get(path + "?user_id=stranger") + body = env.client.get(path, json={"user_id": "stranger"}) + assert targeted.status_code == 400 and body.status_code == 400 + + +def test_edit_race_discards_new_secret_without_overwriting_other_writer(personal_environment): + env = personal_environment + source = create_personal_source(env) + before = deepcopy(env.state.vault) + + def edit_underneath(): + record = env.sources_container.records[("owner", source["id"])] + record["name"] = "Other writer" + record["_etag"] = '"other-writer"' + + env.sources_container.before_replace.append(edit_underneath) + response = env.client.patch(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": source["config_revision"], + "credentials": {"auth_type": "username_password", "password": "fixture-racing-password"}, + }) + assert response.status_code == 409 and response.get_json()["error_code"] == "config_conflict" + assert stored(env, source)["name"] == "Other writer" and env.state.vault == before + + +@pytest.mark.parametrize("document_failure", [False, True]) +def test_native_delete_reports_real_counts_and_partial_refusals(personal_environment, document_failure): + env = personal_environment + source = create_personal_source(env) + for document_id in ("doc-one", "doc-two"): + env.items_container.seed({ + "id": document_id, "type": "file_sync_item", "scope_type": "personal", + "source_id": source["id"], "document_id": document_id, "user_id": "owner", + "remote_path": f"\\\\files\\reports\\{document_id}.pdf", "status": "synced", + }) + def delete_document(*_args, document_id=None, **_kwargs): + if document_failure and document_id == "doc-two": + raise RuntimeError("fixture delete refused") + + env.filesync.delete_document_revision = Mock(side_effect=delete_document) + response = env.client.delete(f"{LIST_PATH}/{source['id']}", json={ + "expected_config_revision": source["config_revision"], "delete_associated_files": True, + }) + result = response.get_json()["delete_result"] + assert result["documents_deleted"] == (1 if document_failure else 2) + assert result["documents_failed"] == (1 if document_failure else 0) + assert response.status_code == (409 if document_failure else 200) + if document_failure: + assert response.get_json()["error_code"] == "delete_incomplete" and response.get_json()["partial"] is True + + +def test_unsaved_and_saved_test_browse_use_the_draft(personal_environment, monkeypatch): + env = personal_environment + source = create_personal_source(env) + test = Mock(return_value=Mock(scandir=Mock(return_value=[]))) + browse = Mock(return_value=[]) + monkeypatch.setattr(env.filesync, "_register_smb_session", test) + monkeypatch.setattr(env.filesync, "_browse_smb_path", browse) + for path in (LIST_PATH, f"{LIST_PATH}/{source['id']}"): + tested = env.client.post(f"{path}/test-connection", json=smb_payload(unc_path="\\\\files\\changed")) + browsed = env.client.post(f"{path}/browse", json={ + **smb_payload(unc_path="\\\\files\\changed"), "browse_path": "Reports", + }) + assert tested.status_code == 200 and browsed.status_code == 200 + assert test.call_args.args[0]["connection"]["unc_path"] == "\\\\files\\changed" + assert browse.call_args.args[1] == "Reports" + + +if __name__ == "__main__": + raise SystemExit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_public_file_source_fixture_parity.py b/functional_tests/test_public_file_source_fixture_parity.py index 9a294211d..c0744a8d3 100644 --- a/functional_tests/test_public_file_source_fixture_parity.py +++ b/functional_tests/test_public_file_source_fixture_parity.py @@ -1,7 +1,7 @@ # test_public_file_source_fixture_parity.py """ Per-route shape parity between the M10B public file source UI fixture and the real routes. -Version: 0.261.182 +Version: 0.261.310 Implemented in: 0.261.182 M10B: the V2 public file sources browser suite mocks the network with the closed HTTP fixture @@ -63,7 +63,7 @@ } SOURCE_CONNECTION_UI_KEYS = {"unc_path", "selected_paths"} FILTERS_UI_KEYS = {"include_patterns", "exclude_patterns", "allowed_extensions", "fixed_tags", "folder_tag_mode"} -OPTIONS_UI_KEYS = {"source_types", "eligible_identity_ids", "schedule", "limits", "recursive_allowed"} +OPTIONS_UI_KEYS = {"source_types", "eligible_identity_ids", "schedule", "limits", "recursive_allowed", "default_remote_delete_policy"} # The credential fields the editor's draftFromSource reads. A key the server omits would open blank and # be sent back blank, which clears the stored value (the identity credential round-trip fix). CREDENTIALS_UI_KEYS = { @@ -277,6 +277,18 @@ def test_options_shape_parity(environment): real_payload = real.get_json() assert_no_invented_keys("options", payload, real_payload) assert_shared_keys("options", payload, real_payload, OPTIONS_UI_KEYS) + assert_shared_keys("schedule", payload["schedule"], real_payload["schedule"], { + "min_interval_minutes", "max_interval_minutes", + }) + assert_shared_keys("limits", payload["limits"], real_payload["limits"], {"max_sources"}) + real_types = {item["value"]: item for item in real_payload["source_types"]} + for item in payload["source_types"]: + assert isinstance(item, dict), "The native editor requires source-type descriptors, not strings." + assert_shared_keys("source type", item, real_types[item["value"]], {"value", "label", "visible"}) + assert isinstance(item["visible"], bool) + assert set(payload["eligible_identity_ids"]) == set(real_payload["eligible_identity_ids"]) + assert payload["eligible_identity_ids"]["azure_files"] == [] + assert payload["eligible_identity_ids"]["azure_blob"] == [] # -------------------------------------------------------------------------- diff --git a/functional_tests/test_support/file_source_configuration_probe.ts b/functional_tests/test_support/file_source_configuration_probe.ts new file mode 100644 index 000000000..e8f45e057 --- /dev/null +++ b/functional_tests/test_support/file_source_configuration_probe.ts @@ -0,0 +1,206 @@ +// file_source_configuration_probe.ts +// Version: 0.261.310 +// Implemented in: 0.261.310 +// Executes the real personal transport and shared credential/field serializers. + +import assert from 'node:assert/strict'; +import { + PERSONAL_FILE_SOURCE_WORKBENCH as adapter, + FileSourceConflictError, + FileSourceWriteConflictError, + FileSourceBusyError, + FileSourceDeleteIncompleteError, + FileSourcePartialDeleteError, +} from '../../application/v2_ui/src/lib/fileSourceWorkbench'; +import { + buildFileSourceWrite, + connectionDescriptor, + draftFromSource, + emptyFileSourceDraft, + visibleSourceTypes, +} from '../../application/v2_ui/src/lib/fileSourceFields'; +import type { FileSourceOptions, WorkspaceSyncSource } from '../../application/v2_ui/src/lib/types'; + +interface Call { method: string; path: string; body?: unknown; } +const calls: Call[] = []; +let payload: unknown; +let status = 200; +const options: FileSourceOptions = { + source_types: ['smb', 'azure_files', 'azure_blob'].map((value) => ({ value, label: value, visible: true })), + eligible_identity_ids: { smb: ['identity'], azure_files: [], azure_blob: [] }, + schedule: { min_interval_minutes: 5, max_interval_minutes: 10080 }, + limits: { max_sources: 25 }, + recursive_allowed: false, + default_remote_delete_policy: 'hard_delete', +}; +const source: WorkspaceSyncSource = { + id: 'source-id', config_revision: 'revision-1', name: 'Reports', source_type: 'smb', + credentials: { auth_type: 'username_password', username: 'svc', password_stored: true }, +}; + +globalThis.fetch = async (input, init) => { + calls.push({ + method: init?.method ?? 'GET', path: String(input), + body: typeof init?.body === 'string' ? JSON.parse(init.body) : undefined, + }); + return new Response(JSON.stringify(payload), { + status, headers: { 'content-type': 'application/json' }, + }); +}; + +function respond(value: unknown, responseStatus = 200): void { + calls.length = 0; + payload = value; + status = responseStatus; +} + +function last(method: string, path: string, body?: unknown): void { + assert.deepEqual(calls, [{ method, path, body }]); +} + +async function transport(): Promise { + const base = '/api/file-sync/personal/sources'; + const saved = `${base}/source-id`; + const write = { name: 'Draft', source_type: 'smb', connection: { unc_path: '\\\\files\\reports' } }; + respond({ sources: [source] }); + const listed = await adapter.list(); + assert.deepEqual(listed, [source]); + last('GET', base); + respond({ source }); + await adapter.read(source); + last('GET', saved); + await adapter.create(write); + assert.equal(calls.at(-1)?.method, 'POST'); + assert.deepEqual(calls.at(-1)?.body, write); + respond({ source }); + await adapter.update(source, write); + last('PATCH', saved, { ...write, expected_config_revision: 'revision-1' }); + respond(options); + const loaded = await adapter.options(); + assert.deepEqual(loaded, options); + last('GET', '/api/file-sync/personal/source-options'); + respond({ identities: [{ id: 'identity' }] }); + await adapter.identities(); + last('GET', '/api/workspace-identities/personal/identities'); + respond({ tags: [{ name: 'legal', count: 2 }, { name: 'finance', count: 4 }] }); + const tags = await adapter.tags(); + assert.deepEqual(tags, ['finance', 'legal']); + last('GET', '/api/documents/tags'); + for (const target of [null, source]) { + const root = target ? saved : base; + respond({ connection: { success: true } }); + await adapter.testConnection(target, write); + last('POST', `${root}/test-connection`, write); + respond({ browse: { path: 'Reports', entries: [] } }); + await adapter.browse(target, write, 'Reports'); + last('POST', `${root}/browse`, { ...write, browse_path: 'Reports' }); + } + respond({ item: { ignored: true } }); + await adapter.ignorePath(source.id, '\\\\files\\reports\\report.pdf', true); + last('POST', `${saved}/ignore-path`, { remote_path: '\\\\files\\reports\\report.pdf', ignored: true }); + const result = { associated_files_requested: true, documents_deleted: 3, documents_failed: 0, documents_skipped: 1 }; + respond({ delete_result: result }); + const removed = await adapter.remove(source, true); + assert.deepEqual(removed, result); + last('DELETE', saved, { expected_config_revision: 'revision-1', delete_associated_files: true }); + + for (const [code, errorType] of [ + ['config_conflict', FileSourceConflictError], ['write_conflict', FileSourceWriteConflictError], + ] as const) { + respond({ error: 'Refused', error_code: code }, 409); + await assert.rejects(() => adapter.update(source, write), errorType); + } + for (const [errorPayload, errorType] of [ + [{ error_code: 'source_busy' }, FileSourceBusyError], + [{ error_code: 'config_conflict' }, FileSourceConflictError], + [{ error_code: 'delete_incomplete', delete_result: result }, FileSourceDeleteIncompleteError], + [{ partial: true, delete_result: result }, FileSourcePartialDeleteError], + ] as const) { + respond({ error: 'Refused', ...errorPayload }, 409); + await assert.rejects(() => adapter.remove(source, true), errorType); + } + respond({}); + await assert.rejects(() => adapter.read(source), /malformed/); + await assert.rejects(() => adapter.list(), /malformed/); + await assert.rejects(() => adapter.options(), /malformed/); + await assert.rejects(() => adapter.identities(), /malformed/); + await assert.rejects(() => adapter.remove(source, true), /could not be verified/); + respond({ connection: { success: false } }); + await assert.rejects(() => adapter.testConnection(null, {}), /malformed/); + respond({ item: {} }); + await assert.rejects(() => adapter.ignorePath(source.id, 'remote-path', true), /malformed/); + respond({ sources: [{ id: 'missing-revision' }] }); + await assert.rejects(() => adapter.list(), /malformed/); + respond({ ...options, recursive_allowed: 'false' }); + await assert.rejects(() => adapter.options(), /malformed/); + respond({ ...options, limits: { max_sources: 1.5 } }); + await assert.rejects(() => adapter.options(), /malformed/); + respond({}); + await assert.rejects(() => adapter.update({ id: 'no-revision' }, write), /version marker/); + assert.equal(calls.length, 0, 'No write may occur without the revision.'); +} + +function fields(): void { + for (const sourceType of ['smb', 'azure_files', 'azure_blob']) { + for (const authType of connectionDescriptor(sourceType).authTypes) { + const draft = emptyFileSourceDraft(sourceType, 5); + draft.credentialMode = 'inline'; + Object.assign(draft, { + name: 'Reports', selectedPaths: ['Q1/report.pdf'], fixedTags: ['finance'], + folderTagMode: 'full_path', remoteDeletePolicy: 'hard_delete', + includePatterns: '*.pdf, *.docx', excludePatterns: 'drafts/*', + allowedExtensions: 'pdf', scheduleEnabled: true, intervalMinutes: 30, + }); + Object.assign(draft.credentials, { + authType, username: 'svc', domain: 'CORP', clientId: 'client-id', tenantId: 'tenant-id', + secret: 'fixture-new-secret', + }); + const write = buildFileSourceWrite(draft); + assert.deepEqual(write.connection?.selected_paths, ['Q1/report.pdf']); + assert.deepEqual(write.filters?.fixed_tags, ['finance']); + assert.equal(write.filters?.folder_tag_mode, 'full_path'); + assert.equal(write.remote_delete_policy, 'hard_delete'); + assert.deepEqual(write.schedule, { enabled: true, interval_minutes: 30 }); + assert.equal(write.credentials?.auth_type, authType); + if (authType === 'managed_identity') assert.equal(write.credentials?.managed_identity_client_id, 'client-id'); + if (authType === 'client_secret') { + assert.equal(write.credentials?.identity, 'client-id'); + assert.equal(write.credentials?.tenant_id, 'tenant-id'); + assert.equal(write.credentials?.secret, 'fixture-new-secret'); + } + if (authType === 'connection_string') assert.equal(write.credentials?.connection_string, 'fixture-new-secret'); + if (authType === 'username_password') assert.equal(write.credentials?.password, 'fixture-new-secret'); + draft.credentialMode = 'identity'; + draft.identityId = 'identity'; + const bound = buildFileSourceWrite(draft); + assert.equal(bound.identity_id, 'identity'); + assert.equal(bound.credentials, undefined); + } + } + const masked = draftFromSource({ + ...source, credentials: { auth_type: 'username_password', password: '********', password_stored: true }, + }, 5); + assert.equal(masked.credentials.secret, ''); + assert.equal(buildFileSourceWrite(masked).credentials?.password, ''); + const existing = draftFromSource({ + id: 'onedrive-existing', source_type: 'onedrive', identity_id: 'old-global', + connection: { selected_paths: ['Reports'] }, credentials: { auth_type: 'global_identity' }, + }, 5); + const oneDriveWrite = buildFileSourceWrite(existing); + assert.deepEqual(oneDriveWrite.connection, { selected_paths: ['Reports'] }); + assert.deepEqual(oneDriveWrite.credentials, { auth_type: 'global_identity' }); + assert.equal(oneDriveWrite.identity_id, ''); + assert.ok(!visibleSourceTypes(options).some((item) => item.value === 'onedrive')); + assert.throws(() => connectionDescriptor('google_workspace'), /cannot be configured/); +} + +async function main(): Promise { + await transport(); + fields(); + console.log('native personal configuration transport and field checks passed'); +} + +main().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/functional_tests/test_support/personal_file_source_harness.py b/functional_tests/test_support/personal_file_source_harness.py new file mode 100644 index 000000000..fc796e3e0 --- /dev/null +++ b/functional_tests/test_support/personal_file_source_harness.py @@ -0,0 +1,62 @@ +# personal_file_source_harness.py +""" +Isolated personal File Sync routes using the existing real engine/Key Vault harness. +Version: 0.261.310 +Implemented in: 0.261.310 +""" + +import importlib.util +import sys +from unittest.mock import Mock + +import pytest +from flask import Blueprint, Flask + +from test_support.agent_delegation import APP_ROOT, execute_functions, module_stub +from test_support.group_file_source_harness import as_user, environment, smb_payload + + +LIST_PATH = "/api/file-sync/personal/sources" +OPTIONS_PATH = "/api/file-sync/personal/source-options" + + +@pytest.fixture +def personal_environment(environment, monkeypatch): + """Run the real personal route module with cloud I/O replaced by the shared harness.""" + env = environment + auth = sys.modules["functions_authentication"] + namespace = dict(vars(auth)) + execute_functions("functions_authentication.py", {"admin_required"}, namespace) + monkeypatch.setattr(auth, "admin_required", namespace["admin_required"], raising=False) + monkeypatch.setattr(auth, "enabled_required", sys.modules["functions_settings"].enabled_required, raising=False) + groups = sys.modules["functions_group"] + monkeypatch.setattr(groups, "require_active_group", Mock(), raising=False) + monkeypatch.setattr(groups, "search_all_groups", Mock(), raising=False) + monkeypatch.setattr(sys.modules["functions_public_workspaces"], "search_all_public_workspaces", Mock(), raising=False) + monkeypatch.setitem(sys.modules, "functions_simplechat_operations", module_stub( + "functions_simplechat_operations", search_directory_users=Mock(), + )) + spec = importlib.util.spec_from_file_location("route_backend_file_sync", APP_ROOT / "route_backend_file_sync.py") + routes = importlib.util.module_from_spec(spec) + monkeypatch.setitem(sys.modules, "route_backend_file_sync", routes) + spec.loader.exec_module(routes) + app = Flask("personal_file_source_contract") + app.config.update(TESTING=True, SECRET_KEY="test-only-session-key") + blueprint = Blueprint("backend_file_sync", __name__) + blueprint.before_request(auth.user_required_blueprint()) + routes.register_route_backend_file_sync(blueprint) + app.register_blueprint(blueprint) + app.extensions["executor"] = env.executor + env.app, env.client = app, app.test_client() + env.sources_container = env.filesync._get_sources_container("personal") + env.items_container = env.filesync._get_items_container("personal") + env.runs_container = env.filesync._get_runs_container("personal") + env.routes = routes + as_user(env, "owner") + yield env + + +def create_personal_source(env, payload=None): + response = env.client.post(LIST_PATH, json=payload or smb_payload()) + assert response.status_code == 201, response.get_data(as_text=True) + return response.get_json()["source"] diff --git a/functional_tests/test_v2_personal_file_sources.py b/functional_tests/test_v2_personal_file_sources.py new file mode 100644 index 000000000..0e3c02eb0 --- /dev/null +++ b/functional_tests/test_v2_personal_file_sources.py @@ -0,0 +1,35 @@ +# test_v2_personal_file_sources.py +""" +Execute the real native personal adapter and shared file-source field rules. +Version: 0.261.310 +Implemented in: 0.261.310 +""" + +import subprocess +import uuid +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +V2 = ROOT / "application" / "v2_ui" + + +def test_native_personal_transport_and_fields(): + bundle = V2 / "node_modules" / f".cache-personal-file-sources-{uuid.uuid4().hex}.mjs" + try: + subprocess.run([ + "node", str(V2 / "node_modules" / "esbuild" / "bin" / "esbuild"), + str(ROOT / "functional_tests" / "test_support" / "file_source_configuration_probe.ts"), + "--bundle", "--platform=node", "--format=esm", "--packages=external", + "--define:import.meta.env={}", f"--outfile={bundle}", "--log-level=error", + ], check=True, capture_output=True, text=True) + result = subprocess.run(["node", str(bundle)], capture_output=True, text=True) + assert result.returncode == 0, result.stdout + result.stderr + finally: + bundle.unlink(missing_ok=True) + + +if __name__ == "__main__": + raise SystemExit(pytest.main([__file__, "-q"])) diff --git a/ui_tests/fixtures/group_workspace.py b/ui_tests/fixtures/group_workspace.py index b56bab5cc..9d7d32f47 100644 --- a/ui_tests/fixtures/group_workspace.py +++ b/ui_tests/fixtures/group_workspace.py @@ -3252,6 +3252,7 @@ def _file_source_options_payload(self, group_id): "schedule": {"min_interval_minutes": 5, "max_interval_minutes": 10080}, "limits": {"max_sources": 25}, "recursive_allowed": True, + "default_remote_delete_policy": "ignore", } def _file_source_guard(self, route, entry, group_id): diff --git a/ui_tests/fixtures/personal_file_sources.py b/ui_tests/fixtures/personal_file_sources.py new file mode 100644 index 000000000..2f05dc265 --- /dev/null +++ b/ui_tests/fixtures/personal_file_sources.py @@ -0,0 +1,195 @@ +# personal_file_sources.py +""" +Closed production-SPA HTTP fixture for native personal source configuration. +Version: 0.261.310 +Implemented in: 0.261.310 +""" + +import copy +import re +from urllib.parse import urlsplit + +import pytest + +from ui_tests.fixtures.workspace_authoring import ORIGIN, SPA_INDEX, WorkspaceAuthoringFixture + + +BASE = "/api/file-sync/personal/sources" +SOURCE_ID = "personal-source" +SOURCE_NAME = "Personal reports" +STORED_PASSWORD = "fixture-personal-stored-password" + + +class PersonalFileSourcesFixture(WorkspaceAuthoringFixture): + def __init__(self, page): + super().__init__(page) + self.sources = { + SOURCE_ID: { + "id": SOURCE_ID, "name": SOURCE_NAME, "source_type": "smb", "enabled": True, + "recursive": True, "connection": {"unc_path": "\\\\files\\reports", "selected_paths": ["Reports"]}, + "filters": {"include_patterns": ["*.pdf"], "exclude_patterns": [], "allowed_extensions": ["pdf"], + "fixed_tags": ["finance"], "folder_tag_mode": "parent"}, + "schedule": {"enabled": False, "interval_minutes": 30}, "remote_delete_policy": "ignore", + "identity_id": "", "identity_name": "", + "credentials": {"auth_type": "username_password", "username": "svc-reports", "domain": "CORP", + "password_stored": True, "secret_stored": False, "password": "", "secret": ""}, + "config_revision": "revision-1", + }, + } + self.source_secrets = {SOURCE_ID: STORED_PASSWORD} + self.source_versions = {SOURCE_ID: 1} + self.private_values.add(STORED_PASSWORD) + self.source_options = { + "source_types": [ + {"value": value, "label": label, "visible": True} + for value, label in [("smb", "Network share"), ("azure_files", "Azure Files"), ("azure_blob", "Azure Blob Storage")] + ], + "eligible_identity_ids": {"smb": ["personal-smb-identity"], "azure_files": [], "azure_blob": []}, + "schedule": {"min_interval_minutes": 5, "max_interval_minutes": 10080}, + "limits": {"max_sources": 25}, "recursive_allowed": True, + "default_remote_delete_policy": "ignore", + } + self.source_identities = [{ + "id": "personal-smb-identity", "name": "Personal share account", "provider": "smb", + "is_enabled": True, "usage_contexts": ["file_sync"], "supported_source_types": ["smb"], + }] + + def _bootstrap(self): + bootstrap = super()._bootstrap() + bootstrap["workspace"]["sections"]["sync"] = { + "enabled": "sync" not in self.disabled_sections, + "reason": self.disabled_sections.get("sync"), "group": "connections", + } + return bootstrap + + def _route(self, route): + parsed = urlsplit(route.request.url) + if route.request.method == "GET" and f"{parsed.scheme}://{parsed.netloc}" == ORIGIN and parsed.path == "/v2/workspace/sync": + route.fulfill(path=str(SPA_INDEX), content_type="text/html") + return + super()._route(route) + + def change_source(self, identifier, **changes): + self.sources[identifier].update(copy.deepcopy(changes)) + self.source_versions[identifier] += 1 + self.sources[identifier]["config_revision"] = f"revision-{self.source_versions[identifier]}" + + def _apply(self, identifier, write): + source = self.sources[identifier] + for field in ("name", "source_type", "enabled", "recursive", "connection", "filters", "schedule", "remote_delete_policy", "identity_id"): + if field in write: + source[field] = copy.deepcopy(write[field]) + if source.get("identity_id"): + identity = next(item for item in self.source_identities if item["id"] == source["identity_id"]) + source["identity_name"] = identity["name"] + else: + source["identity_name"] = "" + if "credentials" in write: + incoming = write["credentials"] + auth_type = incoming["auth_type"] + old = source.get("credentials", {}) + secret = incoming.get("password") or incoming.get("secret") or incoming.get("connection_string") + if secret: + self.source_secrets[identifier] = secret + self.private_values.add(secret) + elif old.get("auth_type") != auth_type: + self.source_secrets.pop(identifier, None) + source["credentials"] = { + **{key: copy.deepcopy(incoming[key]) for key in ("auth_type", "username", "domain", "identity", "tenant_id", "managed_identity_client_id") if key in incoming}, + "password_stored": bool(self.source_secrets.get(identifier)) and auth_type == "username_password", + "secret_stored": bool(self.source_secrets.get(identifier)) and auth_type in ("client_secret", "connection_string"), + "password": "", "secret": "", + } + self.change_source(identifier) + + def _dispatch(self, route, entry): + path, method = entry.path, entry.method + if path == "/api/file-sync/personal/source-options" and method == "GET": + self._json(route, self.source_options) + return + if path == "/api/workspace-identities/personal/identities" and method == "GET": + self._json(route, {"identities": self.source_identities}) + return + if path == "/api/documents/tags" and method == "GET": + self._json(route, {"tags": [{"name": "finance", "count": 3}, {"name": "legal", "count": 1}]}) + return + if path == BASE: + if method == "GET": + self._json(route, {"sources": list(self.sources.values())}) + return + if method == "POST": + identifier = f"created-source-{len(self.sources)}" + self.sources[identifier] = {"id": identifier, "credentials": {}} + self.source_versions[identifier] = 0 + self._apply(identifier, entry.body) + self._json(route, {"source": self.sources[identifier]}, 201) + return + match = re.fullmatch(re.escape(BASE) + r"(?:/([^/]+))?/(test-connection|browse|ignore-path|sync|runs)", path) + if match: + identifier, operation = match.groups() + if identifier and identifier not in self.sources: + self._json(route, {"error": "File source not found."}, 404) + return + if operation == "test-connection" and method == "POST": + self._json(route, {"connection": { + "success": True, "entries_checked": 1, "folders_seen": 0, "files_seen": 1, + }}) + return + if operation == "browse" and method == "POST": + browse_path = entry.body.get("browse_path", "") + entries = [ + {"name": "Reports", "path": "Reports", "is_dir": True}, + {"name": "report.pdf", "path": "report.pdf", "is_dir": False, "remote_path": "\\\\files\\reports\\report.pdf"}, + ] if not browse_path else [ + {"name": "Q1.pdf", "path": "Reports/Q1.pdf", "is_dir": False, "remote_path": "\\\\files\\reports\\Reports\\Q1.pdf"}, + ] + self._json(route, {"browse": {"path": browse_path, "entries": entries}}) + return + if operation == "ignore-path" and method == "POST": + self._json(route, {"item": {"remote_path": entry.body["remote_path"], "ignored": entry.body["ignored"]}}) + return + if operation == "sync" and method == "POST": + self._json(route, {"run": {"id": "queued-run", "status": "queued"}}, 202) + return + if operation == "runs" and method == "GET": + self._json(route, {"runs": [{"id": "completed-run", "status": "completed"}]}) + return + match = re.fullmatch(re.escape(BASE) + r"/([^/]+)", path) + if match: + identifier = match.group(1) + source = self.sources.get(identifier) + if not source: + self._json(route, {"error": "File source not found."}, 404) + return + if method == "GET": + self._json(route, {"source": source}) + return + if method in ("PATCH", "DELETE") and entry.body["expected_config_revision"] != source["config_revision"]: + self._json(route, {"error": "This file source changed while you were editing.", "error_code": "config_conflict"}, 409) + return + if method == "PATCH": + self._apply(identifier, entry.body) + self._json(route, {"source": self.sources[identifier]}) + return + if method == "DELETE": + del self.sources[identifier] + self._json(route, {"delete_result": { + "associated_files_requested": entry.body["delete_associated_files"], + "documents_deleted": 3 if entry.body["delete_associated_files"] else 0, + "documents_skipped": 0, "documents_failed": 0, + }}) + return + super()._dispatch(route, entry) + + def assert_clean(self): + super().assert_clean() + for _, payload in self.responses: + assert STORED_PASSWORD not in str(payload), "Stored personal credentials must never cross HTTP." + assert not [entry for entry in self.requests if entry.path.startswith(("/api/groups/", "/api/public-workspaces/"))] + + +@pytest.fixture +def personal_file_sources_ui(page): + fixture = PersonalFileSourcesFixture(page) + yield fixture + fixture.assert_clean() diff --git a/ui_tests/fixtures/public_file_sources.py b/ui_tests/fixtures/public_file_sources.py index 67d2bf4b9..90e0debb5 100644 --- a/ui_tests/fixtures/public_file_sources.py +++ b/ui_tests/fixtures/public_file_sources.py @@ -1,7 +1,7 @@ # public_file_sources.py """ Closed M10B public file source HTTP fixtures for the real production V2 SPA. -Version: 0.261.182 +Version: 0.261.310 Implemented in: 0.261.182 The fixture serves the immutable `/api/public-workspaces//file-sources[...]` family and @@ -237,14 +237,21 @@ def _options_payload(self, workspace_id): # Sync identity is offered for every type it supports. pub-a carries one; pub-b none. eligible_id = FILE_SYNC_IDENTITY_ID if workspace_id == "pub-a" else None eligible_identity_ids = { - source_type: ([eligible_id] if eligible_id else []) for source_type in source_types + source_type: ([eligible_id] if eligible_id and source_type == "smb" else []) for source_type in source_types } return { - "source_types": source_types, + "source_types": [ + {"value": source_type, "label": label, "visible": True} + for source_type, label in ( + ("smb", "Network share"), ("azure_files", "Azure Files"), + ("azure_blob", "Azure Blob Storage"), + ) + ], "eligible_identity_ids": eligible_identity_ids, - "schedule": {"min_interval_minutes": 15, "default_interval_minutes": 60}, - "limits": {"max_sources": 10, "current_count": len(self.file_sources.get(workspace_id, []))}, + "schedule": {"min_interval_minutes": 15, "max_interval_minutes": 10080}, + "limits": {"max_sources": 10}, "recursive_allowed": True, + "default_remote_delete_policy": "ignore", } def _dispatch(self, route, entry): @@ -313,6 +320,35 @@ def _file_sources(self, route, entry): if policy is None: return allowed = set(policy["operations"]) + operation = parts[-1] + if method == "POST" and operation in ("test-connection", "browse", "ignore-path"): + source_id = parts[5] if len(parts) == 7 else None + record = next((row for row in self.file_sources.get(workspace_id, []) if row["id"] == source_id), None) + assert "test" in allowed and policy["status"] == "active", f"Tool reached a read-only workspace: {entry}" + if source_id and record is None: + self._json(route, copy.deepcopy(FILE_SOURCE_NOT_FOUND_BODY), 404) + return + if record: + assert "test" in record["source_actions"], f"Tool reached a withheld source: {entry}" + if operation == "test-connection": + self._json(route, {"connection": {"success": True, "entries_checked": 1}}) + elif operation == "browse": + connection = (entry.body or {}).get("connection") or (record or {}).get("connection") or {} + root = connection.get("unc_path", "\\\\files.example.test\\reports") + self._json(route, {"browse": { + "path": (entry.body or {}).get("browse_path", ""), + "entries": [{ + "name": "budget.xlsx", "path": "budget.xlsx", "is_dir": False, + "remote_path": f"{root}\\budget.xlsx", + }], + }}) + else: + assert record, "Ignore requires a saved source." + self._json(route, {"item": { + "source_id": source_id, "remote_path": entry.body["remote_path"], + "ignored": entry.body["ignored"], + }}) + return if identifier is None: if method == "GET": if entry.query: diff --git a/ui_tests/test_v2_group_file_sources.py b/ui_tests/test_v2_group_file_sources.py index fe4a25481..f806427ba 100644 --- a/ui_tests/test_v2_group_file_sources.py +++ b/ui_tests/test_v2_group_file_sources.py @@ -1,7 +1,7 @@ # test_v2_group_file_sources.py """ Production-SPA coverage for the native scope-aware V2 group file sources section. -Version: 0.261.188 +Version: 0.261.310 Implemented in: 0.261.147 Credential identifiers kept on edit: 0.261.156 Selected paths, fixed tags, folder tags, remote delete policy and root-relative browse: 0.261.171 @@ -388,7 +388,7 @@ def test_config_conflict_reload_rebases_concurrent_subfolder_scope_and_local_nam assert conflict.value.status == 409 with page.expect_response( lambda response: response.request.method == "GET" - and urlsplit(response.url).path == "/api/groups/group-a/file-sources" + and urlsplit(response.url).path == f"/api/groups/group-a/file-sources/{EDITABLE_SOURCE_ID}" ): page.get_by_role("button", name="Reload", exact=True).click() expect(page.get_by_label("Include subfolders", exact=True)).not_to_be_checked() @@ -421,7 +421,7 @@ def test_config_conflict_reload_reports_name_conflict_and_keeps_local_name(group assert conflict.value.status == 409 with page.expect_response( lambda response: response.request.method == "GET" - and urlsplit(response.url).path == "/api/groups/group-a/file-sources" + and urlsplit(response.url).path == f"/api/groups/group-a/file-sources/{EDITABLE_SOURCE_ID}" ): page.get_by_role("button", name="Reload", exact=True).click() expect(page.get_by_text(f"{REBASE_NOTICE} You and someone else both changed: Name. Your values are shown.", exact=True)).to_be_visible() @@ -450,7 +450,7 @@ def test_config_conflict_reload_reports_deleted_source_without_resaving(group_fi assert conflict.value.status == 409 with page.expect_response( lambda response: response.request.method == "GET" - and urlsplit(response.url).path == "/api/groups/group-a/file-sources" + and urlsplit(response.url).path == f"/api/groups/group-a/file-sources/{EDITABLE_SOURCE_ID}" ): page.get_by_role("button", name="Reload", exact=True).click() expect(page.get_by_text(REBASE_DELETED_NOTICE, exact=True)).to_be_visible() diff --git a/ui_tests/test_v2_group_file_sources.ts b/ui_tests/test_v2_group_file_sources.ts index 875dd7b65..56d94b32d 100644 --- a/ui_tests/test_v2_group_file_sources.ts +++ b/ui_tests/test_v2_group_file_sources.ts @@ -1,13 +1,12 @@ // test_v2_group_file_sources.ts // // Runtime pin for the scope seam in fileSourceWorkbench.ts. -// Version: 0.261.171 +// Version: 0.261.310 // Implemented in: 0.261.147 // Tag suggestions read by explicit group: 0.261.171 // // The browser suite proves the group editor, gating, conflict and delete behaviour against a -// mocked backend. It cannot prove the one property the contract calls the floor: that the personal -// adapter stays byte-identical in transport to the file-sync functions the section shipped with, +// mocked backend. This probe proves that the personal list/sync/history keep their existing URLs, // and that the group adapter never reaches for a personal URL (or the reverse). That isolation // lives entirely in the scope branch, so exercising both adapters against a stubbed api client pins // it decisively, with a positive control beside every negative one. @@ -48,9 +47,9 @@ async function testPersonalTransportIsUnchanged(): Promise { const adapter = PERSONAL_FILE_SOURCE_WORKBENCH; // The personal section can still do everything it shipped able to do: the gate never refuses. assert.equal(adapter.allows('create'), true, 'Personal scope never refuses an operation.'); - assert.equal(adapter.manageable, false, 'Personal scope advertises no group-style create control.'); + assert.equal(adapter.manageable, true, 'Personal scope offers native creation.'); - stub({ sources: [{ id: 's1' }] }); + stub({ sources: [{ id: 's1', config_revision: 'r1' }] }); await adapter.list(); assert.deepEqual(calls, [{ method: 'GET', path: '/api/file-sync/personal/sources' }], 'The personal list must hit the shipped personal sources URL, unchanged.'); @@ -65,32 +64,7 @@ async function testPersonalTransportIsUnchanged(): Promise { assert.deepEqual(calls, [{ method: 'POST', path: '/api/file-sync/personal/sources/s1/sync', body: undefined }], 'The personal sync must hit the shipped personal sync URL, unchanged.'); - stub({ message: 'ok' }); - await adapter.remove({ id: 's1' } as never, true); - assert.deepEqual(calls, [{ - method: 'DELETE', path: '/api/file-sync/personal/sources/s1', - body: { delete_associated_files: true }, - }], 'The personal delete must send the documents choice on the shipped personal URL, unchanged.'); - - // Options, identities and tag suggestions are group-only concepts, so the personal path answers - // them inertly and never issues a request for them. - stub({}); - assert.equal(await adapter.options(), null, 'Personal scope has no options endpoint.'); - assert.deepEqual(await adapter.identities(), [], 'Personal scope has no identities endpoint.'); - assert.deepEqual(await adapter.tags(), [], 'Personal scope offers no tag suggestions.'); - assert.equal(calls.length, 0, 'Personal options, identities and tags must issue no request.'); - - // The writes the section only ever offered in the classic workspace stay refused here, so the - // group affordances can never be driven against a personal scope. - for (const attempt of [ - () => adapter.create({ name: 'x' }), - () => adapter.update({ id: 's1' } as never, { name: 'x' }), - () => adapter.testConnection({ id: 's1' } as never, { name: 'x' }), - () => adapter.browse({ id: 's1' } as never, { name: 'x' }, ''), - () => adapter.ignorePath('s1', 'p', true), - ]) { - await assert.rejects(async () => attempt(), 'A personal file source write must be refused.'); - } + // Full native personal configuration is exercised by file_source_configuration_probe.ts. } async function testGroupTransportNeverTouchesPersonal(): Promise { diff --git a/ui_tests/test_v2_personal_file_sources.py b/ui_tests/test_v2_personal_file_sources.py new file mode 100644 index 000000000..19fb37bf1 --- /dev/null +++ b/ui_tests/test_v2_personal_file_sources.py @@ -0,0 +1,297 @@ +# test_v2_personal_file_sources.py +""" +Production-SPA tests for personal native source creation and configuration. +Version: 0.261.310 +Implemented in: 0.261.310 +""" + +import re + +import pytest +from playwright.sync_api import expect + +from ui_tests.fixtures.personal_file_sources import ( + BASE, SOURCE_ID, SOURCE_NAME, STORED_PASSWORD, personal_file_sources_ui, +) + + +pytestmark = pytest.mark.ui + + +def source_row(ui, name=SOURCE_NAME): + return ui.page.get_by_role("listitem").filter(has_text=name) + + +def open_sources(ui, **layout): + ui.open("/workspace/sync", **layout) + expect(ui.page.get_by_role("heading", name="File sources", exact=True)).to_be_visible() + expect(source_row(ui)).to_be_visible() + + +def edit_source(ui, name=SOURCE_NAME): + source_row(ui, name).get_by_role("button", name=f"Edit {name}", exact=True).click() + expect(ui.page.get_by_role("button", name="Save changes", exact=True)).to_be_enabled() + return ui.page.get_by_role("dialog") + + +def last_write(ui, method, path): + writes = [entry for entry in ui.writes if entry.method == method and entry.path == path] + assert writes, f"No {method} to {path}." + return writes[-1] + + +@pytest.mark.parametrize("theme,width,height", [ + ("light", 1440, 900), ("dark", 1440, 900), ("light", 390, 844), ("dark", 390, 844), +]) +def test_native_personal_layout_and_focus(personal_file_sources_ui, theme, width, height): + ui = personal_file_sources_ui + open_sources(ui, theme=theme, width=width, height=height) + dialog = edit_source(ui) + expect(dialog.get_by_label("Name", exact=True)).to_be_focused() + expect(ui.page.get_by_role("link", name=re.compile("classic", re.I))).to_have_count(0) + ui.assert_no_overflow() + dialog.get_by_role("button", name="Cancel", exact=True).click() + expect(source_row(ui).get_by_role("button", name=f"Edit {SOURCE_NAME}", exact=True)).to_be_focused() + + +@pytest.mark.parametrize("source_type,auth_type,fields", [ + ("smb", "username_password", {"Network path": "\\\\files\\new", "Username": "svc-new", "Password": "fixture-draft-password"}), + ("azure_files", "client_secret", {"File service URL": "https://storage.file.core.windows.net", "Share name": "reports", "Directory": "2026", + "Client ID": "client-new", "Tenant ID": "tenant-new", "Client secret": "fixture-draft-client-secret"}), + ("azure_blob", "connection_string", {"Blob service URL": "https://storage.blob.core.windows.net", "Container": "reports", "Prefix": "2026", + "Connection string": "fixture-draft-connection-string"}), +]) +def test_create_reopen_and_edit_each_connector(personal_file_sources_ui, source_type, auth_type, fields): + ui, page = personal_file_sources_ui, personal_file_sources_ui.page + open_sources(ui) + page.get_by_role("button", name="New file source", exact=True).click() + dialog = page.get_by_role("dialog") + expect(dialog.get_by_label("Name", exact=True)).to_be_visible() + dialog.get_by_label("Name", exact=True).fill(f"New {source_type}") + dialog.get_by_label("Source type", exact=True).select_option(source_type) + dialog.get_by_label("Enter credentials directly", exact=True).check() + dialog.get_by_label("Authentication method", exact=True).select_option(auth_type) + for label, value in fields.items(): + dialog.get_by_label(re.compile(rf"^{re.escape(label)}")).fill(value) + dialog.get_by_role("button", name="Test connection", exact=True).click() + expect(dialog.get_by_text("Connected. Checked 1 entry: 0 folders, 1 file.", exact=True)).to_be_visible() + dialog.get_by_role("button", name="Create source", exact=True).click() + expect(page.get_by_role("dialog")).to_have_count(0) + write = last_write(ui, "POST", BASE) + assert write.body["source_type"] == source_type and write.body["credentials"]["auth_type"] == auth_type + assert not [entry for entry in ui.writes if entry.path.endswith("/sync")] + dialog = edit_source(ui, f"New {source_type}") + secret_label = {"username_password": "Password", "client_secret": "Client secret", "connection_string": "Connection string"}[auth_type] + expect(dialog.get_by_label(re.compile(rf"^{re.escape(secret_label)}(?: \(stored\))?$"))).to_have_value("") + dialog.get_by_label("Name", exact=True).fill(f"Saved {source_type}") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(source_row(ui, f"Saved {source_type}")).to_be_visible() + assert last_write(ui, "PATCH", f"{BASE}/created-source-1").body["expected_config_revision"] + ui.assert_no_secret_storage(*[value for label, value in fields.items() if label == secret_label]) + + +def test_all_fields_browse_selection_tags_and_ignore(personal_file_sources_ui): + ui, page = personal_file_sources_ui, personal_file_sources_ui.page + open_sources(ui) + dialog = edit_source(ui) + dialog.get_by_role("button", name="Browse the source", exact=True).click() + expect(dialog.get_by_role("button", name="Ignore report.pdf", exact=True)).to_be_visible() + dialog.get_by_role("button", name="Ignore report.pdf", exact=True).click() + expect(dialog.get_by_role("button", name="Restore report.pdf", exact=True)).to_be_visible() + ignored = last_write(ui, "POST", f"{BASE}/{SOURCE_ID}/ignore-path") + assert ignored.body == {"remote_path": "\\\\files\\reports\\report.pdf", "ignored": True} + dialog.get_by_role("button", name="Restore report.pdf", exact=True).click() + expect(dialog.get_by_role("button", name="Ignore report.pdf", exact=True)).to_be_visible() + dialog.get_by_role("button", name="Select report.pdf", exact=True).click() + dialog.get_by_label("Include patterns", exact=True).fill("*.pdf, *.docx") + dialog.get_by_label("Exclude patterns", exact=True).fill("drafts/*") + dialog.get_by_label("Allowed file types", exact=True).fill("pdf, docx") + dialog.get_by_label("Folder tags", exact=True).select_option("full_path") + dialog.get_by_label("When a source file is deleted", exact=True).select_option("hard_delete") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(page.get_by_role("dialog")).to_have_count(0) + write = last_write(ui, "PATCH", f"{BASE}/{SOURCE_ID}").body + assert write["connection"]["selected_paths"] == ["Reports", "report.pdf"] + assert write["filters"] == { + "include_patterns": ["*.pdf", "*.docx"], "exclude_patterns": ["drafts/*"], + "allowed_extensions": ["pdf", "docx"], "fixed_tags": ["finance"], "folder_tag_mode": "full_path", + } + assert write["remote_delete_policy"] == "hard_delete" + assert write["credentials"]["password"] == "" and ui.source_secrets[SOURCE_ID] == STORED_PASSWORD + dialog = edit_source(ui) + expect(dialog.get_by_label("Folder tags", exact=True)).to_have_value("full_path") + + +def test_saved_identity_is_selected_without_inline_credentials(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + dialog = edit_source(ui) + dialog.get_by_label("Use a saved workspace identity", exact=True).check() + dialog.get_by_label("Identity", exact=True).select_option("personal-smb-identity") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(ui.page.get_by_role("dialog")).to_have_count(0) + write = last_write(ui, "PATCH", f"{BASE}/{SOURCE_ID}").body + assert write["identity_id"] == "personal-smb-identity" and "credentials" not in write + dialog = edit_source(ui) + expect(dialog.get_by_label("Identity", exact=True)).to_have_value("personal-smb-identity") + + +def test_conflict_reload_preserves_local_edits_and_adopts_remote_fields(personal_file_sources_ui): + ui, page = personal_file_sources_ui, personal_file_sources_ui.page + open_sources(ui) + dialog = edit_source(ui) + dialog.get_by_label("Name", exact=True).fill("Local reports") + ui.change_source(SOURCE_ID, recursive=False, remote_delete_policy="hard_delete") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(dialog.get_by_text("This file source changed while you were editing.", exact=True)).to_be_visible() + dialog.get_by_role("button", name="Reload", exact=True).click() + expect(dialog.get_by_label("Name", exact=True)).to_have_value("Local reports") + expect(dialog.get_by_label("Include subfolders", exact=True)).not_to_be_checked() + expect(dialog.get_by_label("When a source file is deleted", exact=True)).to_have_value("hard_delete") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(source_row(ui, "Local reports")).to_be_visible() + assert last_write(ui, "PATCH", f"{BASE}/{SOURCE_ID}").body["expected_config_revision"] == "revision-2" + + +def test_options_failure_is_explicit_and_retry_does_not_guess(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + ui.reject_next("GET", "/api/file-sync/personal/source-options", error="Options unavailable.") + ui.page.get_by_role("button", name="New file source", exact=True).click() + dialog = ui.page.get_by_role("dialog") + expect(dialog.get_by_role("alert")).to_contain_text("Options unavailable.") + expect(dialog.get_by_role("button", name="Create source", exact=True)).to_have_count(0) + dialog.get_by_role("button", name="Retry editor", exact=True).click() + expect(dialog.get_by_role("button", name="Create source", exact=True)).to_be_visible() + + +def test_late_test_response_is_not_applied_to_changed_connection(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + dialog = edit_source(ui) + ui.defer_next("POST", f"{BASE}/{SOURCE_ID}/test-connection") + with ui.page.expect_request(f"**{BASE}/{SOURCE_ID}/test-connection"): + dialog.get_by_role("button", name="Test connection", exact=True).click() + expect(dialog.get_by_role("button", name="Test connection", exact=True)).to_be_disabled() + assert ui.pending_responses + dialog.get_by_label("Network path", exact=True).fill("\\\\files\\changed") + ui.release_responses() + expect(dialog.get_by_text("Connected. Checked 1 entry: 0 folders, 1 file.", exact=True)).to_have_count(0) + dialog.get_by_role("button", name="Cancel", exact=True).click() + expect(dialog.get_by_text("Discard your unsaved changes?", exact=True)).to_be_visible() + dialog.get_by_role("button", name="Keep editing", exact=True).click() + expect(dialog.get_by_label("Network path", exact=True)).to_have_value("\\\\files\\changed") + + +def test_server_defaults_and_schedule_limits(personal_file_sources_ui): + ui = personal_file_sources_ui + ui.source_options["recursive_allowed"] = False + ui.source_options["default_remote_delete_policy"] = "hard_delete" + open_sources(ui) + ui.page.get_by_role("button", name="New file source", exact=True).click() + dialog = ui.page.get_by_role("dialog") + expect(dialog.get_by_label("Name", exact=True)).to_be_visible() + expect(dialog.get_by_label("Include subfolders", exact=True)).to_have_count(0) + expect(dialog.get_by_label("When a source file is deleted", exact=True)).to_have_value("hard_delete") + dialog.get_by_label("Name", exact=True).fill("Scheduled identity source") + dialog.get_by_label("Identity", exact=True).select_option("personal-smb-identity") + dialog.get_by_label("Network path", exact=True).fill("\\\\files\\scheduled") + dialog.get_by_label("Sync on a schedule", exact=True).check() + interval = dialog.get_by_role("spinbutton") + interval.fill("4") + expect(dialog.get_by_role("button", name="Create source", exact=True)).to_be_disabled() + interval.fill("12.5") + expect(dialog.get_by_role("button", name="Create source", exact=True)).to_be_disabled() + interval.fill("30") + dialog.get_by_role("button", name="Create source", exact=True).click() + expect(ui.page.get_by_role("dialog")).to_have_count(0) + write = last_write(ui, "POST", BASE).body + assert write["schedule"] == {"enabled": True, "interval_minutes": 30} + assert write["recursive"] is False and write["remote_delete_policy"] == "hard_delete" + + +def test_deleted_source_conflict_blocks_all_dependent_actions(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + dialog = edit_source(ui) + dialog.get_by_label("Name", exact=True).fill("Kept draft") + ui.reject_next("PATCH", f"{BASE}/{SOURCE_ID}", status=409, error="Source changed.", error_code="config_conflict") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(dialog.get_by_text("Source changed.", exact=True)).to_be_visible() + del ui.sources[SOURCE_ID] + dialog.get_by_role("button", name="Reload", exact=True).click() + expect(dialog.get_by_text("This item was deleted. Copy anything you need, then close.", exact=True)).to_be_visible() + expect(dialog.get_by_role("button", name="Save changes", exact=True)).to_be_disabled() + expect(dialog.get_by_role("button", name="Test connection", exact=True)).to_be_disabled() + expect(dialog.get_by_role("button", name="Browse the source", exact=True)).to_be_disabled() + expect(dialog.get_by_label("Name", exact=True)).to_have_value("Kept draft") + + +def test_failed_save_retains_typed_secret_and_retry_commits_it(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + dialog = edit_source(ui) + secret = "fixture-personal-replacement-password" + dialog.get_by_label("Password (stored)", exact=True).fill(secret) + ui.reject_next("PATCH", f"{BASE}/{SOURCE_ID}", error="Save unavailable.") + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(dialog.get_by_role("alert")).to_contain_text("Save unavailable.") + expect(dialog.get_by_label("Password (stored)", exact=True)).to_have_value(secret) + dialog.get_by_role("button", name="Save changes", exact=True).click() + expect(ui.page.get_by_role("dialog")).to_have_count(0) + assert ui.source_secrets[SOURCE_ID] == secret + ui.assert_no_secret_storage(secret) + + +def test_sync_history_and_conditional_delete_remain_native(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + source_row(ui).get_by_role("button", name="Show sync history", exact=True).click() + expect(source_row(ui).get_by_text("completed", exact=True)).to_be_visible() + source_row(ui).get_by_role("button", name=f"Sync {SOURCE_NAME} now", exact=True).click() + expect(ui.page.get_by_text(f"Sync started for {SOURCE_NAME}", exact=True)).to_be_visible() + source_row(ui).get_by_role("button", name=f"Delete {SOURCE_NAME}", exact=True).click() + ui.page.get_by_role("button", name="Delete documents too", exact=True).click() + expect(source_row(ui)).to_have_count(0) + write = last_write(ui, "DELETE", f"{BASE}/{SOURCE_ID}").body + assert write == {"expected_config_revision": "revision-1", "delete_associated_files": True} + expect(ui.page.get_by_text("File source deleted (3 deleted)", exact=True)).to_be_visible() + + +def test_disabled_personal_sources_do_not_make_source_requests(personal_file_sources_ui): + ui = personal_file_sources_ui + ui.disabled_sections["sync"] = "File Sync is unavailable." + ui.open("/workspace/sync") + expect(ui.page.get_by_text("File Sync is unavailable.", exact=True)).to_be_visible() + assert not [entry for entry in ui.requests if entry.path.startswith(BASE)] + + +def test_cancelled_options_cannot_overwrite_a_reopened_source(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + ui.defer_next("GET", "/api/file-sync/personal/source-options") + ui.page.get_by_role("button", name="New file source", exact=True).click() + dialog = ui.page.get_by_role("dialog") + expect(dialog.get_by_role("status")).to_contain_text("Loading file source configuration") + assert ui.pending_responses + dialog.get_by_role("button", name="Cancel", exact=True).click() + expect(ui.page.get_by_role("dialog")).to_have_count(0) + dialog = edit_source(ui) + ui.release_responses() + expect(dialog.get_by_label("Name", exact=True)).to_have_value(SOURCE_NAME) + expect(dialog.get_by_role("button", name="Save changes", exact=True)).to_be_enabled() + + +def test_late_browse_does_not_offer_paths_for_a_changed_connection(personal_file_sources_ui): + ui = personal_file_sources_ui + open_sources(ui) + dialog = edit_source(ui) + ui.defer_next("POST", f"{BASE}/{SOURCE_ID}/browse") + with ui.page.expect_request(f"**{BASE}/{SOURCE_ID}/browse"): + dialog.get_by_role("button", name="Browse the source", exact=True).click() + expect(dialog.get_by_role("button", name="Browse the source", exact=True)).to_be_disabled() + assert ui.pending_responses + dialog.get_by_label("Network path", exact=True).fill("\\\\files\\changed") + ui.release_responses() + expect(dialog.get_by_role("button", name="Browse the source", exact=True)).to_be_enabled() + expect(dialog.get_by_role("button", name="Select report.pdf", exact=True)).to_have_count(0) diff --git a/ui_tests/test_v2_public_file_sources.py b/ui_tests/test_v2_public_file_sources.py index 6661c23d2..828190b21 100644 --- a/ui_tests/test_v2_public_file_sources.py +++ b/ui_tests/test_v2_public_file_sources.py @@ -1,7 +1,7 @@ # test_v2_public_file_sources.py """ Production-SPA coverage for the native V2 public workspace file sources section (M10B). -Version: 0.261.188 +Version: 0.261.310 Implemented in: 0.261.182 A failed read offers a retry and never the empty state: 0.261.188 @@ -21,10 +21,8 @@ read-only section; and a reader's and a File-Sync-off manager's manager-only section is unavailable with the server's own reason. -The live File Sync engine, run history, connection tests, browse and ignore are exercised -byte-for-byte by the group M5B suite because the public routes drive the same scope-generic engine, -so this suite does not repeat them; the public envelope, projection, credentials, conflict codes and -management gating are what differ, and those are what it covers. +The group suite covers the scope-generic engine. This suite additionally exercises public test, +browse and canonical ignore/restore transport, including an unsaved source's tools. """ import re @@ -293,7 +291,7 @@ def test_config_conflict_reload_rebases_concurrent_scope_and_keeps_local_name(pu assert conflict.value.status == 409 with page.expect_response( lambda response: response.request.method == "GET" - and urlsplit(response.url).path == "/api/public-workspaces/pub-a/file-sources" + and urlsplit(response.url).path == f"/api/public-workspaces/pub-a/file-sources/{EDITABLE_SOURCE_ID}" ): page.get_by_role("button", name="Reload", exact=True).click() expect(page.get_by_label("Include subfolders", exact=True)).not_to_be_checked() @@ -408,6 +406,45 @@ def test_editor_offers_only_eligible_public_identities(public_file_sources_ui): assert_no_personal_or_group_reads(ui) +def test_public_test_browse_and_ignore_stay_in_the_workspace(public_file_sources_ui): + ui, page = public_file_sources_ui, public_file_sources_ui.page + open_sources(ui) + row(ui, EDITABLE_NAME).get_by_role("button", name=f"Edit {EDITABLE_NAME}", exact=True).click() + expect(page.get_by_role("button", name="Save changes", exact=True)).to_be_enabled() + base = f"/api/public-workspaces/pub-a/file-sources/{EDITABLE_SOURCE_ID}" + with page.expect_response(lambda response: urlsplit(response.url).path == base + "/test-connection"): + page.get_by_role("button", name="Test connection", exact=True).click() + with page.expect_response(lambda response: urlsplit(response.url).path == base + "/browse"): + page.get_by_role("button", name="Browse the source", exact=True).click() + expect(page.get_by_role("button", name="Ignore budget.xlsx", exact=True)).to_be_visible() + with page.expect_response(lambda response: urlsplit(response.url).path == base + "/ignore-path") as ignored: + page.get_by_role("button", name="Ignore budget.xlsx", exact=True).click() + assert ignored.value.request.post_data_json == { + "remote_path": "\\\\files.example.test\\reports\\budget.xlsx", "ignored": True, + } + expect(page.get_by_role("button", name="Restore budget.xlsx", exact=True)).to_be_visible() + with page.expect_response(lambda response: urlsplit(response.url).path == base + "/ignore-path") as restored: + page.get_by_role("button", name="Restore budget.xlsx", exact=True).click() + assert restored.value.request.post_data_json["ignored"] is False + assert_no_personal_or_group_reads(ui) + + +def test_public_unsaved_source_tools_do_not_create_a_source(public_file_sources_ui): + ui, page = public_file_sources_ui, public_file_sources_ui.page + open_sources(ui) + page.get_by_role("button", name="New file source", exact=True).click() + page.get_by_role("radio", name="Enter credentials directly").check() + page.get_by_label("Network path", exact=True).fill("\\\\files.example.test\\draft") + base = "/api/public-workspaces/pub-a/file-sources" + with page.expect_response(lambda response: urlsplit(response.url).path == base + "/test-connection"): + page.get_by_role("button", name="Test connection", exact=True).click() + with page.expect_response(lambda response: urlsplit(response.url).path == base + "/browse"): + page.get_by_role("button", name="Browse the source", exact=True).click() + expect(page.get_by_text("File: budget.xlsx", exact=True)).to_be_visible() + assert not [entry for entry in ui.writes if entry.method == "POST" and entry.path == base] + assert_no_personal_or_group_reads(ui) + + def test_create_editor_fits_mobile(public_file_sources_ui): """The create editor and its credential fields fit the narrow breakpoint without sideways scroll.""" ui, page = public_file_sources_ui, public_file_sources_ui.page From 7548e328a464cbd8dac920951bc1d93ffef189f6 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Fri, 9 Oct 2026 07:57:23 -0400 Subject: [PATCH 2/2] Register personal file source test fixtures explicitly Use module-qualified fixture aliases so pytest discovers both the personal fixture and its shared dependency without unused imports. Remove the redundant fixture import from the harness and bump the application patch version to 0.261.311. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- functional_tests/test_personal_file_source_apis.py | 12 +++++++++--- .../test_support/personal_file_source_harness.py | 4 ++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/application/single_app/config.py b/application/single_app/config.py index 39190f940..bd171fbe2 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.310" +VERSION = "0.261.311" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/functional_tests/test_personal_file_source_apis.py b/functional_tests/test_personal_file_source_apis.py index 034dab89b..1b807c6b2 100644 --- a/functional_tests/test_personal_file_source_apis.py +++ b/functional_tests/test_personal_file_source_apis.py @@ -1,7 +1,7 @@ # test_personal_file_source_apis.py """ Functional tests for native personal File Sync configuration. -Version: 0.261.310 +Version: 0.261.311 Implemented in: 0.261.310 Real routes and the real storage/credential engine run against isolated Cosmos and @@ -13,12 +13,18 @@ import pytest -from test_support.group_file_source_harness import as_user, environment, smb_payload +from test_support import group_file_source_harness, personal_file_source_harness +from test_support.group_file_source_harness import as_user, smb_payload from test_support.personal_file_source_harness import ( - LIST_PATH, OPTIONS_PATH, create_personal_source, personal_environment, + LIST_PATH, OPTIONS_PATH, create_personal_source, ) +# Register both fixtures in this test module, including the personal fixture's dependency. +environment = group_file_source_harness.environment +personal_environment = personal_file_source_harness.personal_environment + + def stored(env, source): return env.sources_container.get("owner", source["id"]) diff --git a/functional_tests/test_support/personal_file_source_harness.py b/functional_tests/test_support/personal_file_source_harness.py index fc796e3e0..834883898 100644 --- a/functional_tests/test_support/personal_file_source_harness.py +++ b/functional_tests/test_support/personal_file_source_harness.py @@ -1,7 +1,7 @@ # personal_file_source_harness.py """ Isolated personal File Sync routes using the existing real engine/Key Vault harness. -Version: 0.261.310 +Version: 0.261.311 Implemented in: 0.261.310 """ @@ -13,7 +13,7 @@ from flask import Blueprint, Flask from test_support.agent_delegation import APP_ROOT, execute_functions, module_stub -from test_support.group_file_source_harness import as_user, environment, smb_payload +from test_support.group_file_source_harness import as_user, smb_payload LIST_PATH = "/api/file-sync/personal/sources"