From 3331de890aa0aca7889087012c5aa2b922ba8515 Mon Sep 17 00:00:00 2001 From: Praveen Sampath Date: Tue, 28 Jul 2026 15:49:04 +0000 Subject: [PATCH 1/4] feat(google): add shared Drive folder filtering - discover shared drives and top-level folders using stored or transient DWD credentials - persist folder selections in Drive settings and apply cached ancestry filtering during sync - secure preview actions with strict validation, request limits, and focused tests --- connectors/google/src/connector.rs | 182 ++++- connectors/google/src/drive.rs | 203 +++++ connectors/google/src/models.rs | 383 ++++++++- connectors/google/src/sync.rs | 745 +++++++++++++++++- services/connector-manager/src/handlers.rs | 168 ++++ services/connector-manager/src/lib.rs | 8 + services/connector-manager/src/models.rs | 22 + .../google-drive-folder-selector.svelte | 420 ++++++++++ .../components/google-workspace-setup.svelte | 70 +- web/src/lib/types.ts | 11 +- web/src/lib/types/search.ts | 12 + .../drive/[sourceId]/+page.server.ts | 113 ++- .../drive/[sourceId]/+page.svelte | 44 ++ web/src/routes/api/preview-action/+server.ts | 221 ++++++ .../api/preview-action/preview-action.test.ts | 150 ++++ 15 files changed, 2699 insertions(+), 53 deletions(-) create mode 100644 web/src/lib/components/google-drive-folder-selector.svelte create mode 100644 web/src/routes/api/preview-action/+server.ts create mode 100644 web/src/routes/api/preview-action/preview-action.test.ts diff --git a/connectors/google/src/connector.rs b/connectors/google/src/connector.rs index f560739c9..b6acab634 100644 --- a/connectors/google/src/connector.rs +++ b/connectors/google/src/connector.rs @@ -808,6 +808,79 @@ impl GoogleConnector { Ok(ActionResponse::success(serde_json::to_value(result)?).into_response()) } + + async fn execute_discover_folders( + &self, + _params: JsonValue, + creds: &ServiceCredential, + ) -> Result { + // Only JWT secrets are supported for shared-drive discovery. + if creds.auth_type != AuthType::Jwt { + return Ok(ActionResponse::failure( + "discover_folders requires JWT credentials".to_string(), + ) + .into_response()); + } + + let principal_email = creds + .principal_email + .as_deref() + .ok_or_else(|| anyhow!("Missing principal_email in credentials"))?; + + let auth = crate::auth::create_service_auth(creds, SourceType::GoogleDrive)?; + + let google_auth = crate::auth::GoogleAuth::ServiceAccount(auth); + + // 1. List all shared drives + let drives_response = self + .sync_manager + .drive_client() + .list_drives(&google_auth, principal_email) + .await?; + + let mut items: Vec = Vec::new(); + + // 2. Add each shared drive as a selectable root + for drive in &drives_response.drives { + items.push(crate::models::DriveFolderDiscoveryEntry { + id: drive.id.clone(), + name: drive.name.clone(), + path: format!("/{} (Shared Drive)", drive.name), + drive_id: drive.id.clone(), + kind: "shared_drive_root".to_string(), + }); + } + + // 3. For each shared drive, list top-level folder children. + // Any failure propagates so the admin gets an actionable error. + for drive in &drives_response.drives { + let folders = self + .sync_manager + .drive_client() + .list_folder_children(&google_auth, principal_email, &drive.id, &drive.id) + .await + .with_context(|| { + format!( + "Failed to list folder children for shared drive '{}' ({})", + drive.name, drive.id + ) + })?; + + for folder in &folders.files { + items.push(crate::models::DriveFolderDiscoveryEntry { + id: folder.id.clone(), + name: folder.name.clone(), + path: format!("/{}/{}", drive.name, folder.name), + drive_id: drive.id.clone(), + kind: "folder".to_string(), + }); + } + } + + let result = crate::models::DriveFolderDiscoveryResponse { items }; + + Ok(ActionResponse::success(serde_json::to_value(result)?).into_response()) + } } #[async_trait] @@ -907,6 +980,21 @@ impl Connector for GoogleConnector { admin_only: false, hidden: false, }, + ActionDefinition { + name: "discover_folders".to_string(), + description: + "List accessible shared drives and their top-level folders for folder-path filter selection." + .to_string(), + mode: omni_connector_sdk::ActionMode::Read, + input_schema: json!({ + "type": "object", + "properties": {}, + "required": [] + }), + source_types: vec![SourceType::GoogleDrive], + admin_only: true, + hidden: true, + }, ActionDefinition { name: "google_workspace_call".to_string(), description: "Call a Google Workspace API through the installed gws CLI" @@ -1156,6 +1244,7 @@ impl Connector for GoogleConnector { match action { "fetch_file" => self.execute_fetch_file(params, &creds).await, "search_users" => self.execute_search_users(params, &creds).await, + "discover_folders" => self.execute_discover_folders(params, &creds).await, "google_workspace_schema" => self.execute_gws_schema(params).await, "google_workspace_call" => self.execute_gws_call(params, &creds).await, _ => { @@ -1177,7 +1266,9 @@ impl Connector for GoogleConnector { mod tests { use std::sync::Arc; - use omni_connector_sdk::{AuthType, Connector, SdkClient, ServiceCredential, ServiceProvider}; + use omni_connector_sdk::{ + AuthType, Connector, SdkClient, ServiceCredential, ServiceProvider, SourceType, + }; use serde_json::json; use crate::admin::AdminClient; @@ -1573,4 +1664,93 @@ mod tests { // Empty size assert!(parse_attachment_doc_id("CABc123%40mail.example.test:att:report.pdf:").is_err()); } + + // ======================================================================== + // discover_folders action tests + // ======================================================================== + + #[test] + fn discover_folders_action_registered_in_manifest() { + let connector = test_connector(); + let actions = connector.actions(); + let action = actions.iter().find(|a| a.name == "discover_folders"); + assert!( + action.is_some(), + "discover_folders action must be registered" + ); + let action = action.unwrap(); + assert!(action.admin_only, "discover_folders must be admin_only"); + assert!(action.hidden, "discover_folders must be hidden"); + assert_eq!( + action.source_types, + vec![SourceType::GoogleDrive], + "discover_folders must only accept google_drive source type" + ); + } + + #[test] + fn discover_folders_rejects_oauth_credentials() { + let connector = test_connector(); + let cred = test_service_credential(AuthType::OAuth, json!({})); + + let rt = tokio::runtime::Runtime::new().unwrap(); + let result = rt.block_on(async { + connector + .execute_action("discover_folders", json!({}), Some(cred), None, None) + .await + }); + + assert!(result.is_ok(), "execute_action should return OK response"); + let response = result.unwrap(); + let status = response.status(); + assert_eq!( + status, + axum::http::StatusCode::BAD_REQUEST, + "OAuth credentials should be rejected with 400" + ); + } + + #[test] + fn discover_folders_invalid_action_returns_not_supported() { + let connector = test_connector(); + let cred = test_service_credential(AuthType::Jwt, json!({})); + + let rt = tokio::runtime::Runtime::new().unwrap(); + let result = rt.block_on(async { + connector + .execute_action("nonexistent_action", json!({}), Some(cred), None, None) + .await + }); + + assert!(result.is_ok()); + let response = result.unwrap(); + assert_eq!(response.status(), axum::http::StatusCode::NOT_FOUND); + } + + #[test] + fn discover_folders_missing_principal_email_returns_500() { + let connector = test_connector(); + let mut cred = test_service_credential(AuthType::Jwt, json!({})); + cred.principal_email = None; + + let rt = tokio::runtime::Runtime::new().unwrap(); + let result = rt.block_on(async { + connector + .execute_action("discover_folders", json!({}), Some(cred), None, None) + .await + }); + + // Missing principal_email propagates as Err (anyhow) to the SDK layer + assert!( + result.is_err(), + "missing principal_email should produce an Err" + ); + let err = result.unwrap_err(); + let err_msg = format!("{:?}", err); + assert!( + err_msg.contains("principal_email") || err_msg.contains("principal"), + "error should mention missing principal_email: {}", + err_msg + ); + } } diff --git a/connectors/google/src/drive.rs b/connectors/google/src/drive.rs index 51e613b46..a775fa6bb 100644 --- a/connectors/google/src/drive.rs +++ b/connectors/google/src/drive.rs @@ -1080,6 +1080,203 @@ impl DriveClient { ) }) } + + /// List shared drives visible to the delegated principal's credentials. + /// + /// Paginates through all pages and returns the full list, sorted by drive name. + /// Only shared drives that the authenticated principal can see are returned; + /// does NOT use `useDomainAdminAccess` so drives the principal explicitly has + /// access to (e.g. shared drives they are members of) are surfaced. + pub async fn list_drives( + &self, + auth: &GoogleAuth, + user_email: &str, + ) -> Result { + let user_email_owned = user_email.to_string(); + let rate_limiter = self.rate_limiter.clone(); + let client = self.client.clone(); + + let mut all_drives: Vec = Vec::new(); + let mut page_token: Option = None; + + loop { + let page: DrivesListResponse = + execute_with_auth_retry(auth, &user_email_owned, rate_limiter.clone(), |token| { + let page_token = page_token.clone(); + let client = client.clone(); + async move { + let url = format!("{}/drives", drive_api_base().as_str()); + let mut params: Vec<(&str, &str)> = vec![ + ("pageSize", "100"), + ("fields", "nextPageToken,drives(id,name)"), + ]; + if let Some(ref pt) = page_token { + params.push(("pageToken", pt)); + } + + let response = client + .get(&url) + .bearer_auth(&token) + .query(¶ms) + .send() + .await?; + + let status = response.status(); + if !status.is_success() { + return classify_google_api_error( + response, + "Failed to list shared drives".to_string(), + ) + .await; + } + + let response_text = response.text().await?; + let parsed: DrivesListResponse = serde_json::from_str(&response_text) + .map_err(|e| { + anyhow!( + "Failed to parse drives list response: {}. Raw: {}", + e, + response_text + ) + })?; + + Ok(ApiResult::Success(parsed)) + } + }) + .await?; + + all_drives.extend(page.drives); + + if let Some(next) = page.next_page_token { + page_token = Some(next); + } else { + break; + } + } + + // Deterministic sort by name for stable UI ordering. + all_drives.sort_by(|a, b| a.name.cmp(&b.name)); + + Ok(DrivesListResponse { + drives: all_drives, + next_page_token: None, + }) + } + + /// List immediate children (sub-folders only) of a given folder in any drive. + /// + /// Paginates through all pages and returns the full list, sorted by name. + /// Uses `corpora=drive` and `driveId` scoping for shared-drive children. + pub async fn list_folder_children( + &self, + auth: &GoogleAuth, + user_email: &str, + folder_id: &str, + drive_id: &str, + ) -> Result { + let folder_id_owned = folder_id.to_string(); + let drive_id_owned = drive_id.to_string(); + let user_email_owned = user_email.to_string(); + let rate_limiter = self.rate_limiter.clone(); + let client = self.client.clone(); + + let mut all_files: Vec = Vec::new(); + let mut page_token: Option = None; + + loop { + let page: FilesListResponse = execute_with_auth_retry( + auth, + &user_email_owned, + rate_limiter.clone(), + |token| { + let folder_id = folder_id_owned.clone(); + let drive_id = drive_id_owned.clone(); + let page_token = page_token.clone(); + let client = client.clone(); + async move { + let url = format!("{}/files", drive_api_base().as_str()); + + let query = format!( + "trashed=false and mimeType='application/vnd.google-apps.folder' and '{}' in parents", + folder_id + ); + + let mut params: Vec<(&str, &str)> = vec![ + ("pageSize", "100"), + ("fields", "nextPageToken,files(id,name,mimeType,parents,createdTime)"), + ("q", query.as_str()), + ("supportsAllDrives", "true"), + ("includeItemsFromAllDrives", "true"), + ("corpora", "drive"), + ("driveId", &drive_id), + ]; + if let Some(ref pt) = page_token { + params.push(("pageToken", pt)); + } + + let response = client + .get(&url) + .bearer_auth(&token) + .query(¶ms) + .send() + .await?; + + let status = response.status(); + if !status.is_success() { + return classify_google_api_error( + response, + format!("Failed to list folder children for {folder_id}"), + ) + .await; + } + + let response_text = response.text().await?; + let parsed: FilesListResponse = + serde_json::from_str(&response_text).map_err(|e| { + anyhow!( + "Failed to parse folder children response: {}. Raw: {}", + e, + response_text + ) + })?; + + Ok(ApiResult::Success(parsed)) + } + }, + ) + .await?; + + all_files.extend(page.files); + + if let Some(next) = page.next_page_token { + page_token = Some(next); + } else { + break; + } + } + + // Deterministic sort by name for stable UI ordering. + all_files.sort_by(|a, b| a.name.cmp(&b.name)); + + Ok(FilesListResponse { + files: all_files, + next_page_token: None, + }) + } +} + +/// Response from the Drive API v3 `/drives` endpoint. +#[derive(Debug, Deserialize)] +pub struct DrivesListResponse { + pub drives: Vec, + #[serde(rename = "nextPageToken")] + pub next_page_token: Option, +} + +#[derive(Debug, Deserialize, Clone)] +pub struct DriveMetadata { + pub id: String, + pub name: String, } #[derive(Debug, Deserialize)] @@ -1340,6 +1537,12 @@ fn extract_text_from_presentation(presentation: &GooglePresentation) -> String { mod tests { use super::*; + #[test] + fn files_query_no_cutoff_returns_trashed_false_only() { + let query = build_files_query(None); + assert_eq!(query, "trashed=false"); + } + #[test] fn files_query_uses_modified_or_created_time_cutoff() { let query = build_files_query(Some("2025-01-01T00:00:00Z")); diff --git a/connectors/google/src/models.rs b/connectors/google/src/models.rs index 6205a36c3..7f19f81c6 100644 --- a/connectors/google/src/models.rs +++ b/connectors/google/src/models.rs @@ -23,6 +23,121 @@ pub struct GoogleDirectoryUser { pub is_admin: bool, } +/// A single entry in the persisted folder-path filter list in source config. +/// Stored under `sources.config.folder_path_filters`. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct FolderPathFilterEntry { + pub id: String, + pub name: String, + pub path: String, + #[serde(rename = "driveId")] + pub drive_id: String, + pub kind: FolderPathFilterKind, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FolderPathFilterKind { + SharedDriveRoot, + Folder, +} + +/// Result entry returned by the `discover_folders` connector action. +#[derive(Debug, Clone, Serialize)] +pub struct DriveFolderDiscoveryEntry { + pub id: String, + pub name: String, + pub path: String, + #[serde(rename = "driveId")] + pub drive_id: String, + pub kind: String, +} + +/// Response from the `discover_folders` connector action. +#[derive(Debug, Clone, Serialize)] +pub struct DriveFolderDiscoveryResponse { + pub items: Vec, +} + +/// Helper to parse folder_path_filters from a source config. +/// +/// Returns: +/// - `Ok(Some(filters))` — a valid non-empty, deduplicated filter list was found +/// - `Ok(None)` — the key is absent or explicitly an empty array (index all) +/// - `Err(msg)` — the key is present but malformed (fail closed) +pub fn parse_folder_path_filters( + config: &serde_json::Value, +) -> Result>, String> { + let raw = match config.get("folder_path_filters") { + Some(v) => v, + None => return Ok(None), + }; + + // Null is malformed — fail closed. + if raw.is_null() { + return Err( + "folder_path_filters must not be null; omit the key or use [] to index all." + .to_string(), + ); + } + + let arr = raw.as_array().ok_or_else(|| { + format!( + "folder_path_filters must be an array, got {}", + serde_json::to_string(raw).unwrap_or_default() + ) + })?; + + // Empty array → index all. + if arr.is_empty() { + return Ok(None); + } + + // Parse each entry, rejecting null and validating non-empty required fields. + let mut entries: Vec = Vec::new(); + let mut seen_ids: std::collections::HashSet = std::collections::HashSet::new(); + + for (i, item) in arr.iter().enumerate() { + let entry: FolderPathFilterEntry = serde_json::from_value(item.clone()).map_err(|e| { + format!( + "folder_path_filters[{}] is invalid: {} (value={})", + i, + e, + serde_json::to_string(item).unwrap_or_default() + ) + })?; + + // Validate non-empty required fields. + if entry.id.is_empty() { + return Err(format!("folder_path_filters[{}].id must not be empty", i)); + } + if entry.name.is_empty() { + return Err(format!("folder_path_filters[{}].name must not be empty", i)); + } + if entry.path.is_empty() { + return Err(format!("folder_path_filters[{}].path must not be empty", i)); + } + if entry.drive_id.is_empty() { + return Err(format!( + "folder_path_filters[{}].drive_id must not be empty", + i + )); + } + + // Deduplicate by stable ID — first occurrence wins. + if seen_ids.insert(entry.id.clone()) { + entries.push(entry); + } + } + + if entries.is_empty() { + Ok(None) + } else { + Ok(Some(entries)) + } +} + #[derive(Debug, Clone, Serialize)] pub struct SearchUsersResponse { pub users: Vec, @@ -1411,11 +1526,9 @@ mod tests { match event { ConnectorEvent::DocumentCreated { permissions, .. } => { assert!(permissions.users.contains(&"owner@example.com".to_string())); - assert!( - permissions - .users - .contains(&"viewer@example.com".to_string()) - ); + assert!(permissions + .users + .contains(&"viewer@example.com".to_string())); assert_eq!(permissions.users.len(), 2); } _ => panic!("Expected DocumentCreated event"), @@ -1588,11 +1701,9 @@ mod tests { ConnectorEvent::DocumentCreated { permissions, .. } => { assert!(permissions.users.contains(&"alice@example.com".to_string())); assert!(permissions.users.contains(&"owner@example.com".to_string())); - assert!( - permissions - .users - .contains(&"sender@example.com".to_string()) - ); + assert!(permissions + .users + .contains(&"sender@example.com".to_string())); } _ => panic!("Expected DocumentCreated event"), } @@ -1673,4 +1784,256 @@ mod tests { _ => panic!("Expected DocumentCreated event"), } } + + // ======================================================================== + // Folder path filter parse tests + // ======================================================================== + + #[test] + fn test_parse_folder_path_filters_absent_key() { + // Key absent → Ok(None) → index everything. + let config = serde_json::json!({"domain": "example.com"}); + let result = parse_folder_path_filters(&config); + assert!(result.is_ok(), "absent key should be Ok"); + assert!(result.unwrap().is_none(), "absent key → None"); + } + + #[test] + fn test_parse_folder_path_filters_null_value() { + // Explicit null → Err (fail closed). + let config = serde_json::json!({"folder_path_filters": null}); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "null should be rejected"); + assert!( + result.unwrap_err().contains("must not be null"), + "error should explain null is not allowed" + ); + } + + #[test] + fn test_parse_folder_path_filters_empty_array() { + // Empty array [] → Ok(None) → index everything (valid, intentional). + let config = serde_json::json!({"folder_path_filters": []}); + let result = parse_folder_path_filters(&config); + assert!(result.is_ok(), "empty array should be Ok"); + assert!(result.unwrap().is_none(), "empty array → None"); + } + + #[test] + fn test_parse_folder_path_filters_valid_shared_drive_root() { + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "0AK123456789", + "name": "Marketing", + "path": "/Marketing (Shared Drive)", + "driveId": "0AK123456789", + "kind": "shared_drive_root" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_ok()); + let filters = result.unwrap(); + assert!(filters.is_some()); + let filters = filters.unwrap(); + assert_eq!(filters.len(), 1); + assert_eq!(filters[0].id, "0AK123456789"); + assert!(matches!( + filters[0].kind, + FolderPathFilterKind::SharedDriveRoot + )); + } + + #[test] + fn test_parse_folder_path_filters_valid_folder() { + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "1ABC123DEF", + "name": "Reports", + "path": "/Marketing/Reports", + "driveId": "0AK123456789", + "kind": "folder" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_ok()); + let filters = result.unwrap().unwrap(); + assert_eq!(filters[0].name, "Reports"); + assert!(matches!(filters[0].kind, FolderPathFilterKind::Folder)); + } + + #[test] + fn test_parse_folder_path_filters_malformed_string() { + // String value instead of array → Err (fail closed). + let config = serde_json::json!({"folder_path_filters": "not-an-array"}); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "malformed string should be Err"); + let err = result.unwrap_err(); + assert!(!err.is_empty(), "error message should be non-empty"); + } + + #[test] + fn test_parse_folder_path_filters_malformed_array_with_primitives() { + // Array of primitives fails serde → Err (fail closed). + let config = serde_json::json!({"folder_path_filters": ["invalid"]}); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "array of non-objects should be Err"); + } + + #[test] + fn test_parse_folder_path_filters_missing_required_fields() { + // Entry with missing name/path/driveId/kind fails serde → Err. + let config = serde_json::json!({ + "folder_path_filters": [{"id": "only-id"}] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "entry with missing fields should be Err"); + } + + #[test] + fn test_parse_folder_path_filters_unknown_kind() { + // Unknown kind variant fails serde → Err (fail closed). + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "1", + "name": "Test", + "path": "/Test", + "driveId": "1", + "kind": "unknown_kind" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "unknown kind should be Err"); + } + + #[test] + fn test_folder_path_filter_serialization_roundtrip() { + let entry = FolderPathFilterEntry { + id: "0ATEST123".to_string(), + name: "Test Drive".to_string(), + path: "/Test Drive (Shared Drive)".to_string(), + drive_id: "0ATEST123".to_string(), + kind: FolderPathFilterKind::SharedDriveRoot, + }; + let json = serde_json::to_value(&entry).unwrap(); + assert_eq!(json["id"], "0ATEST123"); + assert_eq!(json["kind"], "shared_drive_root"); + let deserialized: FolderPathFilterEntry = serde_json::from_value(json).unwrap(); + assert_eq!(deserialized.id, entry.id); + } + + #[test] + fn test_drive_folder_discovery_response_serialization() { + let response = DriveFolderDiscoveryResponse { + items: vec![DriveFolderDiscoveryEntry { + id: "drive1".to_string(), + name: "Company Drive".to_string(), + path: "/Company Drive (Shared Drive)".to_string(), + drive_id: "drive1".to_string(), + kind: "shared_drive_root".to_string(), + }], + }; + let json = serde_json::to_value(&response).unwrap(); + assert_eq!(json["items"][0]["kind"], "shared_drive_root"); + } + + #[test] + fn test_parse_folder_path_filters_deny_unknown_fields() { + // deny_unknown_fields on FolderPathFilterEntry rejects extra keys. + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "1", + "name": "T", + "path": "/T", + "driveId": "1", + "kind": "folder", + "extra_field": "should_not_exist" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "unknown fields should be rejected"); + let err = result.unwrap_err(); + assert!( + err.contains("unknown field") || err.contains("extra_field"), + "error should mention the unknown field: {}", + err + ); + } + + #[test] + fn test_parse_folder_path_filters_empty_id_rejected() { + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "", + "name": "Test", + "path": "/Test", + "driveId": "1", + "kind": "folder" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "empty id should be rejected"); + } + + #[test] + fn test_parse_folder_path_filters_empty_name_rejected() { + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "1", + "name": "", + "path": "/Test", + "driveId": "1", + "kind": "folder" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "empty name should be rejected"); + } + + #[test] + fn test_parse_folder_path_filters_empty_drive_id_rejected() { + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "1", + "name": "Test", + "path": "/Test", + "driveId": "", + "kind": "folder" + }] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_err(), "empty driveId should be rejected"); + } + + #[test] + fn test_parse_folder_path_filters_duplicate_id_rejected() { + let config = serde_json::json!({ + "folder_path_filters": [ + { + "id": "dup-id", + "name": "First", + "path": "/First", + "driveId": "d1", + "kind": "folder" + }, + { + "id": "dup-id", + "name": "Second", + "path": "/Second", + "driveId": "d2", + "kind": "folder" + } + ] + }); + let result = parse_folder_path_filters(&config); + assert!(result.is_ok(), "duplicate id should be deduplicated"); + let filters = result.unwrap(); + assert!(filters.is_some()); + // Deduplication means only the first occurrence is kept. + assert_eq!( + filters.as_ref().unwrap().len(), + 1, + "duplicate id → deduped to 1" + ); + assert_eq!(filters.unwrap()[0].name, "First", "first occurrence wins"); + } } diff --git a/connectors/google/src/sync.rs b/connectors/google/src/sync.rs index 098c536a7..e9ffad1b4 100644 --- a/connectors/google/src/sync.rs +++ b/connectors/google/src/sync.rs @@ -1,11 +1,12 @@ -use anyhow::{Context, Result, anyhow}; +use anyhow::{anyhow, Context, Result}; use dashmap::DashMap; -use futures::{StreamExt, stream}; +use futures::{stream, StreamExt}; use omni_connector_sdk::SyncContext; use std::collections::{HashMap, HashSet}; use std::future::Future; -use std::sync::Arc; +use std::pin::Pin; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, RwLock}; use std::time::{Duration, Instant}; use time::{self, OffsetDateTime}; use tokio::sync::{Mutex, Notify, OwnedSemaphorePermit, Semaphore}; @@ -100,6 +101,105 @@ struct DriveContentCache { locks: DashMap>>, } +/// Invocation-local cache for folder-metadata lookups during folder-path filter +/// ancestry resolution. Shared via `Arc` across parallel user tasks within a +/// single sync run so repeated lookups for the same folder ID are served from +/// memory rather than re-fetched from the Drive API. +pub(crate) struct FolderAncestryCache { + cache: RwLock)>>, + pub hits: AtomicU64, +} + +impl FolderAncestryCache { + pub fn new() -> Self { + Self { + cache: RwLock::new(HashMap::new()), + hits: AtomicU64::new(0), + } + } + + /// Look up a folder's metadata, using the cache if available. + /// Returns `(name, Option)`. + pub async fn get_or_fetch( + &self, + drive_client: &DriveClient, + auth: &GoogleAuth, + user_email: &str, + folder_id: &str, + ) -> Result<(String, Option)> { + // Fast path: check cache under read lock. + { + let cache = self + .cache + .read() + .map_err(|e| anyhow!("FolderAncestryCache read lock error: {:?}", e))?; + if let Some(entry) = cache.get(folder_id) { + self.hits.fetch_add(1, Ordering::Relaxed); + return Ok(entry.clone()); + } + } + // Miss: fetch from API. + let meta = drive_client + .get_folder_metadata(auth, user_email, folder_id) + .await + .with_context(|| { + format!( + "Failed to fetch folder metadata for {} while evaluating folder-path filter", + folder_id + ) + })?; + let result = ( + meta.name.clone(), + meta.parents.as_ref().and_then(|p| p.first()).cloned(), + ); + // Store under write lock (first-wins silently if another task arrived first). + { + let mut cache = self + .cache + .write() + .map_err(|e| anyhow!("FolderAncestryCache write lock error: {:?}", e))?; + cache + .entry(folder_id.to_string()) + .or_insert_with(|| result.clone()); + } + Ok(result) + } + + /// Test-only: lookup with a custom async fetcher instead of DriveClient. + #[cfg(test)] + pub(crate) async fn get_or_fetch_test( + &self, + folder_id: &str, + fetcher: F, + ) -> Result<(String, Option)> + where + F: Fn() -> Fut, + Fut: Future)>> + Send, + { + { + let cache = self + .cache + .read() + .map_err(|e| anyhow!("FolderAncestryCache read lock error: {:?}", e))?; + if let Some(entry) = cache.get(folder_id) { + self.hits.fetch_add(1, Ordering::Relaxed); + return Ok(entry.clone()); + } + } + let result = fetcher().await?; + { + let mut cache = self + .cache + .write() + .map_err(|e| anyhow!("FolderAncestryCache write lock error: {:?}", e))?; + cache + .entry(folder_id.to_string()) + .or_insert_with(|| result.clone()); + } + Ok(result) + } +} + impl DriveContentCache { fn get_content_id(&self, file_id: &str) -> Option { self.content_ids @@ -266,7 +366,7 @@ async fn emit_metadata_only_drive_event( } use crate::admin::AdminClient; -use crate::auth::{GoogleAuth, GoogleOAuthCredentials, OAuthAuth, google_max_retries}; +use crate::auth::{google_max_retries, GoogleAuth, GoogleOAuthCredentials, OAuthAuth}; use crate::cache::LruFolderCache; use crate::chat::{ ChatClient, GoogleChatAttachmentSource, GoogleChatMessage, GoogleChatSpace, @@ -276,9 +376,9 @@ use crate::connector::build_attachment_doc_id; use crate::drive::{DriveClient, FileContent}; use crate::gmail::{BatchThreadResult, ExtractedAttachment, GmailClient, MessageFormat}; use crate::models::{ - AttachmentPointer, GmailThread, GoogleChatSegmentCheckpoint, GoogleChatSpaceCheckpoint, - GoogleConnectorState, GoogleSyncCheckpoint, UserFile, WebhookChannel, WebhookChannelResponse, - WebhookNotification, mime_type_to_content_type, + mime_type_to_content_type, AttachmentPointer, GmailThread, GoogleChatSegmentCheckpoint, + GoogleChatSpaceCheckpoint, GoogleConnectorState, GoogleSyncCheckpoint, UserFile, + WebhookChannel, WebhookChannelResponse, WebhookNotification, }; use omni_connector_sdk::RateLimiter; use omni_connector_sdk::SdkClient; @@ -444,12 +544,11 @@ impl GoogleChatSegment { extra.insert("message_count".to_string(), json!(self.messages.len())); extra.insert( "message_names".to_string(), - json!( - self.messages - .iter() - .map(|m| m.name.clone()) - .collect::>() - ), + json!(self + .messages + .iter() + .map(|m| m.name.clone()) + .collect::>()), ); extra.insert("thread_names".to_string(), json!(self.thread_names())); extra.insert( @@ -499,11 +598,10 @@ impl GoogleChatSegment { let mut attrs = HashMap::new(); attrs.insert( "space".to_string(), - json!( - self.space_display_name - .as_deref() - .unwrap_or(&self.space_name) - ), + json!(self + .space_display_name + .as_deref() + .unwrap_or(&self.space_name)), ); attrs.insert("space_id".to_string(), json!(self.space_name)); attrs.insert("threads".to_string(), json!(self.thread_names())); @@ -1059,6 +1157,79 @@ impl SyncManager { Ok((drive_format, gmail_format)) } + // TODO: When folder-path filters are narrowed or changed, documents that were + // previously indexed but are no longer in scope are NOT automatically pruned from + // the search index. A future reconciliation step should delete documents whose + // parent ancestry no longer intersects the configured filter set. + + /// Check whether a file should be included based on configured folder-path filters. + /// Delegates to the module-level `check_ancestry_static` production walker with + /// a closure that calls the Drive API via the client (no caching during the walk; + /// the folder cache is still used by other production code). + /// + /// Returns `Ok(true)` if no filters are configured (include-everything mode). + /// Returns `Ok(false)` if the file's ancestry is outside the configured scope. + /// Returns `Err(...)` if ancestry metadata could not be determined due to a + /// transient API error — the caller should fail/retry the sync rather than + /// silently including or excluding the file. + async fn is_file_in_allowed_folder( + &self, + ancestry_cache: &Option>, + auth: &GoogleAuth, + user_email: &str, + file: &crate::models::GoogleDriveFile, + allowed_folder_ids: &HashSet, + ) -> Result { + if allowed_folder_ids.is_empty() { + return Ok(true); + } + + let drive_client = self.drive_client.clone(); + let google_auth = auth.clone(); + let user_email_owned = user_email.to_string(); + let ancestry_cache_ref = ancestry_cache.clone(); + + let lookup = move |folder_id: String| -> Pin< + Box)>> + Send>, + > { + let drive_client = drive_client.clone(); + let google_auth = google_auth.clone(); + let user_email_owned = user_email_owned.clone(); + let ancestry_cache_ref = ancestry_cache_ref.clone(); + Box::pin(async move { + match ancestry_cache_ref { + Some(cache) => { + cache + .get_or_fetch( + &drive_client, + &google_auth, + &user_email_owned, + &folder_id, + ) + .await + } + None => { + // Fallback: fetch directly (no cache available — should not + // happen when filters are configured). + let meta = drive_client + .get_folder_metadata(&google_auth, &user_email_owned, &folder_id) + .await + .with_context(|| { + format!( + "Failed to fetch folder metadata for {} while evaluating folder-path filter", + folder_id + ) + })?; + let parent = meta.parents.as_ref().and_then(|p| p.first()).cloned(); + Ok((meta.name, parent)) + } + } + }) + }; + + check_ancestry_static(&file.parents, allowed_folder_ids, &lookup).await + } + async fn sync_drive_for_user( &self, user_email: &str, @@ -1068,6 +1239,8 @@ impl SyncManager { ctx: &SyncContext, created_after: Option<&str>, content_cache: Arc, + folder_filter_ids: Option>>, + folder_ancestry_cache: Option>, ) -> Result<(usize, usize)> { info!("Processing Drive files for user: {}", user_email); @@ -1109,6 +1282,26 @@ impl SyncManager { // we always emit and let the indexer skip unchanged docs. for file in response.files { if self.should_index_file(&file) { + // Apply folder-path filter if configured. + // Errors propagate upward so the sync fails and can be retried. + if let Some(ref allowed_ids) = folder_filter_ids { + if !self + .is_file_in_allowed_folder( + &folder_ancestry_cache, + &service_auth, + user_email, + &file, + allowed_ids, + ) + .await? + { + debug!( + "Skipping file {} ({}) — outside configured folder scope", + file.name, file.id + ); + continue; + } + } file_batch.push(UserFile { user_email: Arc::new(user_email.to_string()), file, @@ -1182,6 +1375,8 @@ impl SyncManager { ctx: &SyncContext, start_page_token: &str, content_cache: Arc, + folder_filter_ids: Option>>, + folder_ancestry_cache: Option>, ) -> Result<(usize, usize)> { info!( "Processing incremental Drive sync for user {} from pageToken {}", @@ -1245,6 +1440,27 @@ impl SyncManager { continue; } + // Apply folder-path filter if configured. + // Errors propagate upward so the sync fails and can be retried. + if let Some(ref allowed_ids) = folder_filter_ids { + if !self + .is_file_in_allowed_folder( + &folder_ancestry_cache, + &service_auth, + user_email, + &file, + allowed_ids, + ) + .await? + { + debug!( + "Skipping change for file {} ({}) — outside configured folder scope", + file.name, file.id + ); + continue; + } + } + file_batch.push(UserFile { user_email: Arc::new(user_email.to_string()), file, @@ -1739,10 +1955,33 @@ impl SyncManager { let mut successful_users = 0; let mut errors = 0; let mut last_error: Option = None; + // Parse folder-path filters from source config. + // Malformed config is treated as a sync error (fail closed). + let folder_filter_ids: Option>> = + match crate::models::parse_folder_path_filters(&source.config) { + Ok(Some(filters)) => { + let ids: HashSet = filters.into_iter().map(|f| f.id).collect(); + Some(Arc::new(ids)) + } + Ok(None) => None, + Err(e) => { + // Fail the sync — malformed config must not silently index everything. + return Err(anyhow::anyhow!( + "Invalid folder path filter configuration: {}", + e + )); + } + }; let content_cache = Arc::new(DriveContentCache::default()); + let ancestry_cache: Option> = folder_filter_ids + .as_ref() + .map(|_| Arc::new(FolderAncestryCache::new())); let parallel_users = google_drive_parallel_users(); info!("Processing Drive users with concurrency {}", parallel_users); + let folder_filter_ids_clone = folder_filter_ids.clone(); + let ancestry_cache_clone = ancestry_cache.clone(); + let user_tasks = stream::iter(user_emails.iter().cloned()).map(|cur_user_email| { let service_auth = service_auth.clone(); let source_id = source.id.clone(); @@ -1751,6 +1990,8 @@ impl SyncManager { let ctx = ctx.clone(); let content_cache = content_cache.clone(); let stored_page_token = old_page_tokens.get(cur_user_email.as_str()).cloned(); + let folder_filter_ids = folder_filter_ids_clone.clone(); + let folder_ancestry_cache = ancestry_cache_clone.clone(); async move { if can_resume_full && stored_page_token.is_some() { @@ -1801,6 +2042,8 @@ impl SyncManager { &ctx, start_token, content_cache.clone(), + folder_filter_ids.clone(), + folder_ancestry_cache.clone(), ) .await { @@ -1828,6 +2071,8 @@ impl SyncManager { &ctx, Some(&drive_cutoff_date), content_cache.clone(), + folder_filter_ids.clone(), + folder_ancestry_cache.clone(), ) .await }; @@ -3481,6 +3726,11 @@ impl SyncManager { Ok(webhook_response) } + /// Public accessor for the Drive client (used by connector actions). + pub fn drive_client(&self) -> &DriveClient { + &self.drive_client + } + pub async fn stop_webhook_for_source( &self, source_id: &str, @@ -3542,24 +3792,30 @@ impl SyncManager { let mut path_components = vec![file_name.to_string()]; let mut current_folder_id = folder_id.to_string(); - // Build path by traversing up the folder hierarchy - let mut depth = 0; + // Build path by traversing up the folder hierarchy. + // Uses cycle detection via visited set; the iteration cap is only a safety net. + let mut visited = HashSet::new(); + let mut _iteration: usize = 0; + const MAX_ITERATIONS: usize = 10_000; loop { - depth += 1; - debug!( - "Path building depth: {}, current folder: {}", - depth, current_folder_id - ); - - // TODO: Remove this - if depth > 50 { + _iteration += 1; + if _iteration >= MAX_ITERATIONS { warn!( - "Path building depth exceeded 50 levels for file: {}, folder: {}", + "Path building exceeded maximum iterations for file: {}, folder: {}", file_name, folder_id ); break; } + // Check for cycles before processing. + if !visited.insert(current_folder_id.clone()) { + warn!( + "Cycle detected in folder ancestry for file: {}, folder: {}", + file_name, current_folder_id + ); + break; + } + let cached_folder = self.folder_cache.get(¤t_folder_id); let parent_folder_id: Option = match cached_folder { @@ -3580,7 +3836,7 @@ impl SyncManager { ); let folder_metadata = self .drive_client - .get_folder_metadata(&auth, &user_email, &folder_id) + .get_folder_metadata(&auth, &user_email, ¤t_folder_id) .await; match folder_metadata { @@ -4347,6 +4603,60 @@ impl SyncManager { } } +/// Core ancestry-check logic — the production walker used by `is_file_in_allowed_folder` +/// and tested directly with mock lookups. +/// +/// Given a file's parent IDs, an allowed set, and a function to fetch folder +/// metadata by ID, walks up the parent chain and returns true if any ancestor +/// is in the allowed set. +/// +/// `lookup` returns `(name, Option)` for a folder ID. +/// Returns Err on error, Ok(true) if found in allowed set, Ok(false) if outside. +/// Has a safety cap of 10_000 iterations to prevent infinite loops. +pub(crate) async fn check_ancestry_static( + file_parents: &Option>, + allowed_ids: &HashSet, + lookup: &(dyn Fn(String) -> Pin)>> + Send>> + + Send + + Sync), +) -> Result { + if allowed_ids.is_empty() { + return Ok(true); + } + + let mut stack: Vec = Vec::new(); + if let Some(parents) = file_parents { + for p in parents { + stack.push(p.clone()); + } + } + + let mut visited = HashSet::new(); + let mut iterations: usize = 0; + const MAX_ITERATIONS: usize = 10_000; + + while let Some(cur) = stack.pop() { + iterations += 1; + if iterations > MAX_ITERATIONS { + // Safety cap — treat as outside rather than infinite loop. + return Ok(false); + } + if !visited.insert(cur.clone()) { + // Cycle detected — skip, don't include. + continue; + } + if allowed_ids.contains(&cur) { + return Ok(true); + } + let (_name, parent) = lookup(cur).await?; + if let Some(p) = parent { + stack.push(p); + } + } + + Ok(false) +} + #[cfg(test)] mod tests { use super::*; @@ -4523,4 +4833,379 @@ mod tests { assert!(semaphore.try_acquire_many_owned(2).is_ok()); } + + // ======================================================================== + // Folder path filter logic tests (production helper check_ancestry_static is at module level above) + // ======================================================================== + + #[tokio::test] + async fn test_ancestry_empty_allowed_set_returns_true() { + let allowed: HashSet = HashSet::new(); + let parents = Some(vec!["p1".to_string()]); + let lookup = + |id: String| -> Pin)>> + Send>> { + Box::pin(async move { Ok(("name".to_string(), Some(format!("parent-of-{}", id)))) }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(result, "empty allowed set → include all"); + } + + #[tokio::test] + async fn test_ancestry_direct_parent_match() { + let allowed: HashSet = ["p1".to_string()].into_iter().collect(); + let lookup = |_id: String| -> Pin)>> + Send>> { + Box::pin(async move { Ok(("name".to_string(), None)) }) + }; + let result = check_ancestry_static(&Some(vec!["p1".to_string()]), &allowed, &lookup) + .await + .unwrap(); + assert!(result, "direct parent match → true"); + } + + #[tokio::test] + async fn test_ancestry_no_match_no_parents() { + let allowed: HashSet = ["p999".to_string()].into_iter().collect(); + let lookup = |_id: String| -> Pin)>> + Send>> { + Box::pin(async move { Ok(("name".to_string(), None)) }) + }; + let result = check_ancestry_static(&Some(Vec::new()), &allowed, &lookup) + .await + .unwrap(); + assert!(!result, "empty parents + no match → false"); + } + + #[tokio::test] + async fn test_ancestry_nested_match_via_grandparent() { + let allowed: HashSet = ["root-allowed".to_string()].into_iter().collect(); + let parents = Some(vec!["child".to_string()]); + let lookup = + |id: String| -> Pin)>> + Send>> { + Box::pin(async move { + match id.as_str() { + "child" => { + Ok(("Child Folder".to_string(), Some("root-allowed".to_string()))) + } + _ => Ok(("unknown".to_string(), None)), + } + }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(result, "nested grandparent match → true"); + } + + #[tokio::test] + async fn test_ancestry_outside_scope() { + let allowed: HashSet = ["allowed-dir".to_string()].into_iter().collect(); + let parents = Some(vec!["other-dir".to_string()]); + let lookup = + |id: String| -> Pin)>> + Send>> { + Box::pin(async move { + match id.as_str() { + "other-dir" => Ok(("Other".to_string(), Some("grandparent".to_string()))), + "grandparent" => Ok(("GP".to_string(), None)), + _ => Ok(("unknown".to_string(), None)), + } + }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(!result, "ancestry outside scope → false"); + } + + #[tokio::test] + async fn test_ancestry_propagates_lookup_error() { + let allowed: HashSet = ["allowed".to_string()].into_iter().collect(); + let parents = Some(vec!["broken".to_string()]); + let lookup = + |id: String| -> Pin)>> + Send>> { + Box::pin(async move { Err(anyhow::anyhow!("API error for {}", id)) }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup).await; + assert!(result.is_err(), "lookup error should propagate"); + } + + #[tokio::test] + async fn test_ancestry_cycle_does_not_loop() { + let allowed: HashSet = ["real-allowed".to_string()].into_iter().collect(); + let parents = Some(vec!["p1".to_string()]); + let lookup = + |id: String| -> Pin)>> + Send>> { + Box::pin(async move { + match id.as_str() { + "p1" => Ok(("Folder 1".to_string(), Some("p2".to_string()))), + "p2" => Ok(("Folder 2".to_string(), Some("p1".to_string()))), + _ => Ok(("unknown".to_string(), None)), + } + }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(!result, "cycle without allowed ID → false"); + } + + #[tokio::test] + async fn test_ancestry_max_iterations_safety_net() { + let allowed: HashSet = ["target-never-found".to_string()].into_iter().collect(); + let parents = Some(vec!["level0".to_string()]); + let counter = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let counter_clone = counter.clone(); + let lookup = move |_id: String| -> Pin< + Box)>> + Send>, + > { + let counter = counter_clone.clone(); + Box::pin(async move { + let next = counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(("x".to_string(), Some(format!("level{}", next + 1)))) + }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(!result, "safety cap reached → false"); + let count = counter.load(std::sync::atomic::Ordering::SeqCst); + assert!( + count >= 10_000, + "should have iterated at least 10k times, got {}", + count + ); + } + + #[tokio::test] + async fn test_ancestry_multiple_parents_any_match() { + let allowed: HashSet = ["allowed-p1".to_string()].into_iter().collect(); + let parents = Some(vec!["other-p".to_string(), "allowed-p1".to_string()]); + let lookup = |_id: String| -> Pin)>> + Send>> { + Box::pin(async move { Ok(("name".to_string(), None)) }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(result, "any parent match → true"); + } + + #[test] + fn test_folder_filter_ids_generated_from_parse() { + // Parse a config with two filters and verify the ID set. + let config = serde_json::json!({ + "folder_path_filters": [ + { + "id": "0ADRI000001", + "name": "Engineering", + "path": "/Engineering (Shared Drive)", + "driveId": "0ADRI000001", + "kind": "shared_drive_root" + }, + { + "id": "1FOLDER0001", + "name": "Docs", + "path": "/Engineering/Docs", + "driveId": "0ADRI000001", + "kind": "folder" + } + ] + }); + + let result = crate::models::parse_folder_path_filters(&config); + assert!(result.is_ok()); + let filters = result.unwrap().unwrap(); + assert_eq!(filters.len(), 2); + + let ids: HashSet = filters.into_iter().map(|f| f.id).collect(); + assert!(ids.contains("0ADRI000001")); + assert!(ids.contains("1FOLDER0001")); + assert_eq!(ids.len(), 2); + } + + #[test] + fn test_config_parse_to_folder_filter_ids_chain() { + // End-to-end production-like path: parse config → extract IDs → HashSet. + let config = serde_json::json!({ + "folder_path_filters": [{ + "id": "0AMYDRIVE", + "name": "Company", + "path": "/Company (Shared Drive)", + "driveId": "0AMYDRIVE", + "kind": "shared_drive_root" + }] + }); + let result = crate::models::parse_folder_path_filters(&config); + assert!(result.is_ok()); + let ids_opt = result + .unwrap() + .map(|f| f.into_iter().map(|e| e.id).collect::>()); + assert!(ids_opt.is_some()); + let ids = ids_opt.unwrap(); + assert!(ids.contains("0AMYDRIVE")); + assert_eq!(ids.len(), 1); + + // If we got None (no filters), that's also index-all. + let nonexistent = crate::models::parse_folder_path_filters(&serde_json::json!({})); + assert!(nonexistent.is_ok_and(|v| v.is_none())); + } + + // ======================================================================== + // FolderAncestryCache tests + // ======================================================================== + + #[tokio::test] + async fn test_folder_ancestry_cache_miss_then_hit() { + // Verify that the cache populates on first call and serves a second + // call without re-invoking the fetcher (cache hit). + let cache = Arc::new(FolderAncestryCache::new()); + let fetch_count = Arc::new(AtomicU64::new(0)); + + // First call — should invoke fetcher once. + { + let fetch_count = fetch_count.clone(); + let result = cache + .get_or_fetch_test("folder-1", move || { + let fetch_count = fetch_count.clone(); + async move { + fetch_count.fetch_add(1, Ordering::Relaxed); + Ok(("Folder 1".to_string(), Some("parent-1".to_string()))) + } + }) + .await + .unwrap(); + assert_eq!(result.0, "Folder 1"); + assert_eq!(result.1, Some("parent-1".to_string())); + } + assert_eq!( + fetch_count.load(Ordering::Relaxed), + 1, + "first call fetched once" + ); + + // Second call — same folder_id, should hit cache without fetching. + { + let result = cache + .get_or_fetch_test("folder-1", || async move { + panic!("fetcher should not be called on cache hit"); + }) + .await + .unwrap(); + assert_eq!(result.0, "Folder 1"); + } + + // fetch_count still 1 (fetcher not called again). + assert_eq!(fetch_count.load(Ordering::Relaxed), 1); + assert!( + cache.hits.load(Ordering::Relaxed) > 0, + "cache should have recorded at least one hit" + ); + } + + #[tokio::test] + async fn test_folder_ancestry_cache_no_cache_when_no_filters() { + // When no filters are configured, the is_file_in_allowed_folder path + // short-circuits at the top-level empty-check without creating a cache. + // Verify that short-circuit with empty allowed set. + let allowed: HashSet = HashSet::new(); + let parents = Some(vec!["child".to_string()]); + let lookup = |_id: String| -> Pin< + Box)>> + Send>, + > { + Box::pin(async move { Ok(("ignored".to_string(), None)) }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(result, "empty allowed set → include all"); + } + + #[tokio::test] + async fn test_check_ancestry_static_cycle_detection() { + // A cycle in the parent chain should not cause infinite loop. + let allowed: HashSet = ["root-folder".to_string()].into_iter().collect(); + let parents = Some(vec!["a".to_string()]); + let lookup = + |id: String| -> Pin)>> + Send>> { + Box::pin(async move { + match id.as_str() { + "a" => Ok(("A".to_string(), Some("b".to_string()))), + "b" => Ok(("B".to_string(), Some("a".to_string()))), // cycle back to a + _ => Ok(("unknown".to_string(), None)), + } + }) + }; + let result = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(!result, "cycle should not match allowed folder"); + } + + #[tokio::test] + async fn test_check_ancestry_static_ancestry_cache_together() { + // FolderAncestryCache used with check_ancestry_static to verify both + // the ancestry walking and caching integration. + let cache = Arc::new(FolderAncestryCache::new()); + let fetch_count = Arc::new(AtomicU64::new(0)); + + let lookup = { + let cache = cache.clone(); + let fetch_count = fetch_count.clone(); + move |folder_id: String| -> Pin< + Box)>> + Send>, + > { + let cache = cache.clone(); + let fetch_count = fetch_count.clone(); + Box::pin(async move { + let fid = folder_id.clone(); + cache + .get_or_fetch_test(&folder_id, { + let fid = fid.clone(); + move || { + let fid = fid.clone(); + let fetch_count = fetch_count.clone(); + async move { + fetch_count.fetch_add(1, Ordering::Relaxed); + match fid.as_str() { + "child" => Ok(("Child Folder".to_string(), Some("root-allowed".to_string()))), + "root-allowed" => Ok(("Allowed Root".to_string(), None)), + _ => Ok(("unknown".to_string(), None)), + } + } + } + }) + .await + }) + } + }; + + let allowed: HashSet = ["root-allowed".to_string()].into_iter().collect(); + let parents = Some(vec!["child".to_string()]); + + // First call — should fetch child only. The root-allowed is in allowed_ids, + // so check_ancestry_static returns true without fetching its metadata. + let r1 = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(r1, "child should resolve to root-allowed"); + assert_eq!( + fetch_count.load(Ordering::Relaxed), + 1, + "one fetch (child only; root-allowed is in allowed set directly)" + ); + + // Second call — should NOT re-fetch because cache already populated. + let r2 = check_ancestry_static(&parents, &allowed, &lookup) + .await + .unwrap(); + assert!(r2, "second call same result"); + assert_eq!( + fetch_count.load(Ordering::Relaxed), + 1, + "no additional fetches on second call (cache hit)" + ); + assert!( + cache.hits.load(Ordering::Relaxed) >= 1, + "cache recorded at least one hit for reused entry" + ); + } } diff --git a/services/connector-manager/src/handlers.rs b/services/connector-manager/src/handlers.rs index 546e4e16f..6df0f9444 100644 --- a/services/connector-manager/src/handlers.rs +++ b/services/connector-manager/src/handlers.rs @@ -555,6 +555,174 @@ pub async fn execute_action( Ok(builder.body(axum::body::Body::from(bytes)).unwrap()) } +/// Preview-action endpoint similar to `execute_action` but accepts inline credentials +/// instead of resolving from the database. This is used for admin preview/discovery +/// actions (like listing shared drives) before the admin has saved their credentials. +/// +/// SECURITY: Strictly allowlisted — only action="discover_folders" for +/// google_drive source type with Google JWT credentials is accepted. +/// Request body size limit: 128KB enforced before any parsing. +pub async fn execute_action_preview( + State(state): State, + body_bytes: axum::body::Bytes, +) -> Result { + // Enforce 128KB body limit BEFORE any parsing. + if body_bytes.len() > 128 * 1024 { + return Err(ApiError::BadRequest( + "Preview request body too large (max 128KB)".to_string(), + )); + } + + let request: crate::models::PreviewActionRequest = serde_json::from_slice(&body_bytes) + .map_err(|e| ApiError::BadRequest(format!("Invalid preview request JSON: {}", e)))?; + + info!( + "Preview action '{}' (source_type={:?}, source_id={:?}, params keys: {:?})", + request.action, + request.source_type, + request.source_id, + request + .params + .as_object() + .map(|m| m.keys().cloned().collect::>()) + .unwrap_or_default() + ); + + // ======== STRICT ALLOWLIST ======== + // Only allow discover_folders action for google_drive with Google JWT credentials. + if request.action != "discover_folders" { + return Err(ApiError::BadRequest(format!( + "Preview action '{}' is not allowed", + request.action + ))); + } + + // source_type must be google_drive; if not present, reject. + let source_type = request.source_type.as_ref().ok_or_else(|| { + ApiError::BadRequest("source_type is required for preview actions".to_string()) + })?; + + if *source_type != SourceType::GoogleDrive { + return Err(ApiError::BadRequest(format!( + "Preview action only supports google_drive, got {:?}", + source_type + ))); + } + + // Validate credential is Google JWT. + let creds = &request.credentials; + if creds.provider != shared::models::ServiceProvider::Google { + return Err(ApiError::BadRequest(format!( + "Preview action requires Google credentials, got provider: {:?}", + creds.provider + ))); + } + if creds.auth_type != shared::models::AuthType::Jwt { + return Err(ApiError::BadRequest(format!( + "Preview action requires JWT credentials, got auth_type: {:?}", + creds.auth_type + ))); + } + if creds.principal_email.as_deref().unwrap_or("").is_empty() { + return Err(ApiError::BadRequest( + "Preview action requires a non-empty principal_email".to_string(), + )); + } + + // Validate credential shape: credentials must be a JSON object with service_account_key. + let creds_obj = creds.credentials.as_object().ok_or_else(|| { + ApiError::BadRequest( + "Preview credential 'credentials' field must be a JSON object".to_string(), + ) + })?; + let sa_key = creds_obj + .get("service_account_key") + .and_then(|v| v.as_str()) + .ok_or_else(|| { + ApiError::BadRequest( + "Preview JWT credentials must contain 'service_account_key' as a string" + .to_string(), + ) + })?; + if sa_key.is_empty() { + return Err(ApiError::BadRequest( + "Preview service_account_key must not be empty".to_string(), + )); + } + // Validate that service_account_key is valid JSON (it's a full SA key blob). + serde_json::from_str::(sa_key).map_err(|e| { + ApiError::BadRequest(format!( + "Preview service_account_key is not valid JSON: {}", + e + )) + })?; + + // Validate config contains domain. + let config_obj = creds.config.as_object().ok_or_else(|| { + ApiError::BadRequest("Preview credential 'config' field must be a JSON object".to_string()) + })?; + let domain = config_obj.get("domain").and_then(|v| v.as_str()); + if domain.is_none_or(|d| d.is_empty()) { + return Err(ApiError::BadRequest( + "Preview credential config must contain a non-empty 'domain'".to_string(), + )); + } + + let manifests = get_registered_manifests(&state.redis_client).await; + + // Source type is the only lookup key — must match exactly. + let connector_url = manifests + .iter() + .find(|m| m.source_types.contains(source_type)) + .map(|m| m.connector_url.clone()) + .ok_or_else(|| { + ApiError::NotFound(format!( + "Connector not registered for type: {:?}", + source_type + )) + })?; + + // Use the inline credential directly — no DB resolution. + let client = ConnectorClient::new(); + let action_request = ActionRequest { + action: request.action, + params: request.params, + credentials: Some(creds.clone()), + source: None, + actor_email: None, + }; + + let response = client + .execute_action_raw(&connector_url, &action_request) + .await + .map_err(|e| ApiError::Internal(e.to_string()))?; + + let status = response.status(); + let mut builder = axum::response::Response::builder().status(status); + + let hop_by_hop = [ + "connection", + "keep-alive", + "transfer-encoding", + "te", + "trailer", + "upgrade", + ]; + for (key, value) in response.headers() { + let key_str = key.as_str(); + if !hop_by_hop.contains(&key_str) { + builder = builder.header(key, value); + } + } + + let bytes = response + .bytes() + .await + .map_err(|e| ApiError::Internal(e.to_string()))?; + + Ok(builder.body(axum::body::Body::from(bytes)).unwrap()) +} + /// Outcome of resolving credentials for a tool/action invocation. enum CredentialResolution { Resolved(shared::models::ServiceCredential), diff --git a/services/connector-manager/src/lib.rs b/services/connector-manager/src/lib.rs index ccb0d9f18..e638c5a3c 100644 --- a/services/connector-manager/src/lib.rs +++ b/services/connector-manager/src/lib.rs @@ -51,6 +51,14 @@ pub fn create_app(state: AppState) -> Router { .route("/sources/:source_id", get(handlers::get_source)) .route("/connectors", get(handlers::list_connectors)) .route("/action", post(handlers::execute_action)) + // /action-preview has a per-route 128KB body limit, applied BEFORE extraction. + // Merge a nested sub-router so the limit layer binds to this route alone, + // independent of the global DefaultBodyLimit::disable() below. + .merge( + Router::new() + .route("/action-preview", post(handlers::execute_action_preview)) + .layer(DefaultBodyLimit::max(128 * 1024)), + ) .route("/actions", get(handlers::list_actions)) .route("/resource", post(handlers::read_resource)) .route("/resources", get(handlers::list_resources)) diff --git a/services/connector-manager/src/models.rs b/services/connector-manager/src/models.rs index 04e65e733..e9f5483c0 100644 --- a/services/connector-manager/src/models.rs +++ b/services/connector-manager/src/models.rs @@ -275,3 +275,25 @@ pub struct ExecuteSkillRequest { #[serde(default)] pub arguments: Option, } + +// ============================================================================ +// Preview Action (transient credentials, no DB dependency) +// ============================================================================ + +/// Like `ExecuteActionRequest` but accepts inline credentials and a source_type +/// hint so the action can be dispatched without a persisted source or credential row. +/// +/// SECURITY: `deny_unknown_fields` ensures no extra fields are smuggled through. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct PreviewActionRequest { + /// Source type to select the connector (used when source_id is not available). + pub source_type: Option, + /// Existing source id (optional, used for source config merging). + pub source_id: Option, + pub action: String, + #[serde(default)] + pub params: JsonValue, + /// Inline credentials (not persisted). Must have provider/google fields populated. + pub credentials: shared::models::ServiceCredential, +} diff --git a/web/src/lib/components/google-drive-folder-selector.svelte b/web/src/lib/components/google-drive-folder-selector.svelte new file mode 100644 index 000000000..af7658985 --- /dev/null +++ b/web/src/lib/components/google-drive-folder-selector.svelte @@ -0,0 +1,420 @@ + + +
+ +

{description}

+ + + {#if isLoading && !hasLoaded} +
+ + Discovering shared drives and folders... +
+ {/if} + + + {#if errorMessage && !isLoading} + + + Discovery Failed + +

{errorMessage}

+ +
+
+ {/if} + + + {#if hasLoaded && allItems.length === 0 && !isLoading && !errorMessage} +

+ No shared drives found. Make sure the service account has access to shared drives in + this domain. +

+ {/if} + + +
+ + + {#if isLoading} + + {/if} + + + {#if showDropdown && filteredItems.length > 0} +
+ {#each filteredItems as item (item.id)} + + {/each} +
+ {:else if showDropdown && searchQuery.trim().length > 0 && !isLoading} +
+ No matching folders +
+ {/if} +
+ + + {#if selected.length > 0} +
+ {#each selected as item (item.id)} +
+ {item.name} + {#if !disabled} + + {/if} +
+ {/each} +
+ {:else if !isLoading && hasLoaded} +

+ No folders selected — all accessible files will be indexed. +

+ {/if} +
diff --git a/web/src/lib/components/google-workspace-setup.svelte b/web/src/lib/components/google-workspace-setup.svelte index ad59045b4..29f52a48d 100644 --- a/web/src/lib/components/google-workspace-setup.svelte +++ b/web/src/lib/components/google-workspace-setup.svelte @@ -3,13 +3,16 @@ import { Button } from '$lib/components/ui/button' import { Label } from '$lib/components/ui/label' import { Checkbox } from '$lib/components/ui/checkbox' + import * as Card from '$lib/components/ui/card' import { AuthType } from '$lib/types' + import type { FolderPathFilter } from '$lib/types' import { toast } from 'svelte-sonner' import { goto } from '$app/navigation' import googleDriveLogo from '$lib/images/icons/google-drive.svg' import gmailLogo from '$lib/images/icons/gmail.svg' import googleChatLogo from '$lib/images/icons/google-chat.svg' import GoogleServiceAccountForm from '$lib/components/google-service-account-form.svelte' + import GoogleDriveFolderSelector from '$lib/components/google-drive-folder-selector.svelte' interface Props { open: boolean @@ -27,6 +30,30 @@ let connectGmail = $state(true) let connectChat = $state(false) let isSubmitting = $state(false) + let driveFolderFilters = $state([]) + + // Credential-context guard: clear selected folder filters whenever the + // credential inputs change after initialization so filters from one set + // of credentials cannot be silently submitted with another. + let prevCredContext = $state('') + let credInitialized = $state(false) + + $effect(() => { + const token = JSON.stringify({ + sa: serviceAccountJson, + pe: principalEmail, + dm: domain, + }) + if (!credInitialized) { + credInitialized = true + prevCredContext = token + return + } + if (token !== prevCredContext) { + prevCredContext = token + driveFolderFilters = [] + } + }) async function handleSubmit() { isSubmitting = true @@ -55,9 +82,20 @@ } const credentials = { service_account_key: serviceAccountJson } - const config = { + + // Base config shared by all Google services (domain only). + const baseConfig: Record = { domain: domain || null, } + + // Drive-specific config with optional folder filters. + const driveConfig: Record = { + domain: domain || null, + } + if (driveFolderFilters.length > 0) { + driveConfig.folder_path_filters = driveFolderFilters + } + const authType = AuthType.JWT const provider = 'google' @@ -69,7 +107,7 @@ scope: 'org', name: 'Google Drive', sourceType: 'google_drive', - config, + config: driveConfig, }), }) @@ -88,7 +126,7 @@ authType: authType, principalEmail: principalEmail || null, credentials, - config, + config: driveConfig, }), }) @@ -105,7 +143,7 @@ scope: 'org', name: 'Gmail', sourceType: 'gmail', - config, + config: baseConfig, }), }) @@ -124,7 +162,7 @@ authType: authType, principalEmail: principalEmail || null, credentials: credentials, - config, + config: baseConfig, }), }) @@ -141,7 +179,7 @@ scope: 'org', name: 'Google Chat', sourceType: 'google_chat', - config, + config: baseConfig, }), }) @@ -160,7 +198,7 @@ authType: authType, principalEmail: principalEmail || null, credentials: credentials, - config, + config: baseConfig, }), }) @@ -240,6 +278,24 @@ + + {#if connectDrive} + + + Drive Folder Filters + + Optionally restrict indexing to specific shared drives and folders. + + + + + + + {/if} diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index 3371ae6c8..2d75d4376 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -74,8 +74,17 @@ export interface ClickUpSourceConfig { space_filters?: string[] } +export interface FolderPathFilter { + id: string + name: string + path: string + driveId: string + kind: 'shared_drive_root' | 'folder' +} + export interface GoogleDriveSourceConfig { - // Future: shared_drive_filters, mime_type_filters, folder_path_filters, etc. + folder_path_filters?: FolderPathFilter[] + // Future: shared_drive_filters, mime_type_filters, etc. } export interface GmailSourceConfig { diff --git a/web/src/lib/types/search.ts b/web/src/lib/types/search.ts index 685d73f08..643276f01 100644 --- a/web/src/lib/types/search.ts +++ b/web/src/lib/types/search.ts @@ -158,6 +158,18 @@ export interface SearchUsersResponse { hasMore: boolean } +export interface DriveFolderDiscoveryEntry { + id: string + name: string + path: string + driveId: string + kind: 'shared_drive_root' | 'folder' +} + +export interface DriveFolderDiscoveryResponse { + items: DriveFolderDiscoveryEntry[] +} + export interface ConnectorActionResponse { status: string result?: T diff --git a/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.server.ts b/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.server.ts index 892ef3f9c..2bea3ba4c 100644 --- a/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.server.ts +++ b/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.server.ts @@ -7,6 +7,7 @@ import { serviceCredentialsRepository } from '$lib/server/repositories/service-c import { userRepository } from '$lib/server/db/users' import { getConfig } from '$lib/server/config' import { AuthType, SourceType } from '$lib/types' +import type { FolderPathFilter } from '$lib/types' export const load: PageServerLoad = async ({ params, locals }) => { requireAdmin(locals) @@ -29,7 +30,12 @@ export const load: PageServerLoad = async ({ params, locals }) => { const creds = await serviceCredentialsRepository.getOrgCredsBySourceId(source.id) const credsConfig = (creds?.config as { domain?: string } | null) ?? {} - const sourceConfig = (source.config as { domain?: string } | null) ?? {} + const sourceConfig = + (source.config as { domain?: string; folder_path_filters?: unknown } | null) ?? {} + + const folderPathFilters = Array.isArray(sourceConfig?.folder_path_filters) + ? sourceConfig.folder_path_filters + : [] const gmailSibling = await sourcesRepository.findActiveByTypeAndCreator( SourceType.GMAIL, @@ -43,9 +49,74 @@ export const load: PageServerLoad = async ({ params, locals }) => { principalEmail: creds?.principalEmail ?? '', domain: credsConfig.domain ?? sourceConfig.domain ?? '', gmailSiblingId: gmailSibling?.id ?? null, + folderPathFilters: folderPathFilters as FolderPathFilter[], } } +function parseFolderPathFilters(formData: FormData): Record[] { + const raw = formData.get('folder_path_filters') as string | null + if (!raw) return [] + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + throw error(400, 'folder_path_filters is not valid JSON') + } + if (!Array.isArray(parsed)) { + throw error(400, 'folder_path_filters must be a JSON array') + } + // Each entry must have all required fields with correct types and no unknown fields. + const allowedEntryKeys = ['id', 'name', 'path', 'driveId', 'kind'] + const seenIds = new Set() + for (const entry of parsed) { + if (!entry || typeof entry !== 'object') { + throw error(400, 'Each folder filter entry must be a non-null object') + } + const e = entry as Record + // Reject unknown fields. + for (const key of Object.keys(e)) { + if (!allowedEntryKeys.includes(key)) { + throw error(400, `Unknown field '${key}' in folder filter entry`) + } + } + // Validate required string fields. + if (typeof e.id !== 'string' || !e.id) { + throw error(400, 'Each folder filter entry must have a non-empty id') + } + if (typeof e.name !== 'string' || !e.name) { + throw error(400, 'Each folder filter entry must have a non-empty name') + } + if (typeof e.path !== 'string' || !e.path) { + throw error(400, 'Each folder filter entry must have a non-empty path') + } + if (typeof e.driveId !== 'string' || !e.driveId) { + throw error(400, 'Each folder filter entry must have a non-empty driveId') + } + if (e.kind !== 'shared_drive_root' && e.kind !== 'folder') { + throw error( + 400, + "Each folder filter entry must have kind 'shared_drive_root' or 'folder'", + ) + } + // Deduplicate by stable ID (first-wins). + if (seenIds.has(e.id)) { + continue + } + seenIds.add(e.id) + } + // Return deduplicated array — first-wins. + const deduplicated: Record[] = [] + seenIds.clear() + for (const entry of parsed) { + const e = entry as Record + const id = e.id as string + if (seenIds.has(id)) continue + seenIds.add(id) + deduplicated.push(e) + } + return deduplicated +} + export const actions: Actions = { default: async ({ request, params, locals, fetch }) => { const user = locals.user @@ -109,9 +180,30 @@ export const actions: Actions = { } } + const folderPathFilters = parseFolderPathFilters(formData) + + // Merge folder_path_filters into config while PRESERVING all existing source config keys. + const existingConfig: Record = + source.config && + typeof source.config === 'object' && + !Array.isArray(source.config) + ? (source.config as Record) + : {} + + const mergedConfig: Record = { ...existingConfig, domain } + mergedConfig.folder_path_filters = folderPathFilters + + // Preserve existing credential config as well. + const existingCredConfig: Record = + existingCreds?.config && + typeof existingCreds.config === 'object' && + !Array.isArray(existingCreds.config) + ? (existingCreds.config as Record) + : {} + await serviceCredentialsRepository.updateBySourceId(source.id, { principalEmail, - config: { domain }, + config: { ...existingCredConfig, domain }, credentials: serviceAccountJson ? { service_account_key: serviceAccountJson } : null, @@ -122,11 +214,24 @@ export const actions: Actions = { userFilterMode, userWhitelist, userBlacklist, - config: { domain }, + config: mergedConfig, }) } else { // OAuth or other auth types — admin can only toggle enabled. - await updateSourceById(source.id, { isActive }) + // Still merge any config changes without destroying existing keys. + const existingConfig: Record = + source.config && + typeof source.config === 'object' && + !Array.isArray(source.config) + ? (source.config as Record) + : {} + const folderPathFilters = parseFolderPathFilters(formData) + const mergedConfig: Record = { ...existingConfig } + mergedConfig.folder_path_filters = folderPathFilters + await updateSourceById(source.id, { + isActive, + config: mergedConfig, + }) } if (isActive) { diff --git a/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.svelte b/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.svelte index aa8d23f7b..51a04c76a 100644 --- a/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.svelte +++ b/web/src/routes/(admin)/admin/settings/integrations/drive/[sourceId]/+page.svelte @@ -10,6 +10,7 @@ import { Badge } from '$lib/components/ui/badge' import { Search, X, AlertCircle, Info, Loader2 } from '@lucide/svelte' import GoogleServiceAccountForm from '$lib/components/google-service-account-form.svelte' + import GoogleDriveFolderSelector from '$lib/components/google-drive-folder-selector.svelte' import { onMount } from 'svelte' import { beforeNavigate } from '$app/navigation' import type { PageProps } from './$types' @@ -19,6 +20,7 @@ ConnectorActionResponse, } from '$lib/types/search' import { AuthType } from '$lib/types' + import type { FolderPathFilter } from '$lib/types' import googleDriveLogo from '$lib/images/icons/google-drive.svg' let { data }: PageProps = $props() @@ -33,6 +35,10 @@ let principalEmail = $state(data.principalEmail) let domain = $state(data.domain) + let folderFilters = $state( + (Array.isArray(data.folderPathFilters) ? data.folderPathFilters : []) as FolderPathFilter[], + ) + let searchQuery = $state('') let searchResults = $state([]) let isSearching = $state(false) @@ -50,6 +56,7 @@ let originalSelectedUsers: string[] = [] let originalPrincipalEmail = data.principalEmail let originalDomain = data.domain + let originalFolderFilters = $state([...folderFilters]) async function searchUsers() { if (searchQuery.trim().length < 2) { @@ -196,10 +203,15 @@ const usersChanged = JSON.stringify(selectedUsers.sort()) !== JSON.stringify(originalSelectedUsers.sort()) + const foldersChanged = + JSON.stringify(folderFilters.map((f) => f.id).sort()) !== + JSON.stringify(originalFolderFilters.map((f) => f.id).sort()) + hasUnsavedChanges = enabled !== originalEnabled || userFilterMode !== originalUserFilterMode || usersChanged || + foldersChanged || principalEmail !== originalPrincipalEmail || domain !== originalDomain || serviceAccountJson.trim().length > 0 @@ -286,6 +298,15 @@ Integrations settings. + + + Folder-level filtering unavailable + + To select specific folders or shared drives for indexing, switch to a + domain-wide delegation (service account) connection. OAuth-connected + sources always index all accessible files. + + {:else if isJwt}
@@ -314,6 +335,29 @@ {/if}
+ +
+
+

Drive Folder Filters

+

+ Optionally restrict indexing to specific shared drives and top-level + folders. All sub-folders within a selected item are included. +

+
+ + + +
+

User Access Control

diff --git a/web/src/routes/api/preview-action/+server.ts b/web/src/routes/api/preview-action/+server.ts new file mode 100644 index 000000000..5657c8a8d --- /dev/null +++ b/web/src/routes/api/preview-action/+server.ts @@ -0,0 +1,221 @@ +import { json, error } from '@sveltejs/kit' +import type { RequestHandler } from './$types' +import { getConfig } from '$lib/server/config' +import { logger } from '$lib/server/logger' +import { SourceType } from '$lib/types' + +/** + * POST /api/preview-action + * + * Admin-only endpoint that forwards a transient credential (not yet persisted) to + * the connector for preview/discovery actions such as listing shared drives and + * top-level folders. The credential is never stored or returned; it flows straight + * to the connector-manager's /action-preview endpoint and then to the connector. + * + * Request body size is limited to 64KB at the web boundary. + * + * SECURITY: Strictly allowlisted — only action='discover_folders', + * sourceType='google_drive', and JWT credentials are accepted. + * + * Request body: + * { + * sourceType: 'google_drive', + * action: 'discover_folders', + * params: {}, + * serviceAccountJson: string, // full service-account JSON key + * principalEmail: string, // delegated admin email + * domain: string // Google Workspace domain + * } + */ +export const POST: RequestHandler = async ({ request, locals }) => { + // Admin-only + if (!locals.user) { + throw error(401, 'Unauthorized') + } + if (locals.user.role !== 'admin') { + throw error(403, 'Admin access required') + } + + // Enforce body size limit BEFORE reading — check Content-Length header first. + const contentLength = request.headers.get('content-length') + if (contentLength && parseInt(contentLength, 10) > 64 * 1024) { + throw error(413, 'Request body too large') + } + // Read body via streaming reader; stop and cancel once 64KiB is exceeded. + // Never call arrayBuffer/text first which would buffer the full body. + let bodyBytes = new Uint8Array(0) + const reader = request.body?.getReader() + const MAX_BYTES = 64 * 1024 + if (reader) { + try { + while (true) { + const { done, value } = await reader.read() + if (done) break + if (bodyBytes.length + value.length > MAX_BYTES) { + await reader.cancel() + throw error(413, 'Request body too large') + } + const newBody = new Uint8Array(bodyBytes.length + value.length) + newBody.set(bodyBytes, 0) + newBody.set(value, bodyBytes.length) + bodyBytes = newBody + } + } catch (err) { + // If we already threw a 413, re-throw it directly rather than wrapping. + if (err && typeof err === 'object' && 'status' in err) { + throw err + } + // Otherwise wrap as parse error — empty/null body will land here. + throw error(400, 'Failed to read request body') + } + } + const text = new TextDecoder().decode(bodyBytes) + + let body: Record + try { + const parsed = JSON.parse(text) + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + throw error(400, 'Request body must be a JSON object') + } + body = parsed as Record + } catch (err) { + if (err && typeof err === 'object' && 'status' in err) { + throw err + } + throw error(400, 'Invalid JSON body') + } + + // ======== STRICT ALLOWLIST ======== + // Reject unknown top-level fields. + const allowedFields = [ + 'sourceType', + 'action', + 'params', + 'serviceAccountJson', + 'principalEmail', + 'domain', + ] + for (const key of Object.keys(body)) { + if (!allowedFields.includes(key)) { + throw error(400, `Unknown field: '${key}'`) + } + } + + const sourceType = body.sourceType as string | undefined + const action = body.action as string | undefined + const params = body.params as Record | undefined + const serviceAccountJson = body.serviceAccountJson as string | undefined + const principalEmail = body.principalEmail as string | undefined + const domain = body.domain as string | undefined + + if (!action) { + throw error(400, 'Action is required') + } + if (action !== 'discover_folders') { + throw error(400, `Preview action '${action}' is not supported`) + } + if (!sourceType || sourceType !== SourceType.GOOGLE_DRIVE) { + throw error(400, 'Preview action only supports source_type: google_drive') + } + + // Only allow empty params object for discover_folders. + if (params !== undefined) { + if (typeof params !== 'object' || params === null || Array.isArray(params)) { + throw error(400, 'params must be a JSON object') + } + if (Object.keys(params).length > 0) { + throw error(400, 'discover_folders does not accept any params') + } + } + + // Transient credentials must be provided for preview (not optional). + if (!serviceAccountJson || !principalEmail || !domain) { + throw error(400, 'serviceAccountJson, principalEmail, and domain are required for preview') + } + + // Validate types are strings. + if (typeof serviceAccountJson !== 'string') { + throw error(400, 'serviceAccountJson must be a string') + } + if (typeof principalEmail !== 'string') { + throw error(400, 'principalEmail must be a string') + } + if (typeof domain !== 'string') { + throw error(400, 'domain must be a string') + } + + // Validate service account JSON + try { + JSON.parse(serviceAccountJson) + } catch { + throw error(400, 'Invalid service account JSON') + } + + try { + const config = getConfig() + const connectorManagerUrl = config.services.connectorManagerUrl + + // Build a transient ServiceCredential payload for the connector-manager + const transientCredential = { + id: 'preview', + source_id: 'preview', + user_id: null as string | null, + provider: 'google', + auth_type: 'jwt', + principal_email: principalEmail, + credentials: { + service_account_key: serviceAccountJson, + }, + config: { + domain: domain, + }, + expires_at: null, + last_validated_at: null, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + } + + const response = await fetch(`${connectorManagerUrl}/action-preview`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + source_type: 'google_drive', + source_id: null, + action: 'discover_folders', + params: params || {}, + credentials: transientCredential, + }), + // Limit connector-manager communication too + signal: AbortSignal.timeout(30_000), + }) + + if (!response.ok) { + let errorMessage = 'Failed to discover folders' + const contentType = response.headers.get('content-type') || '' + if (contentType.includes('application/json')) { + try { + const errorBody = await response.json() + errorMessage = errorBody.error || errorBody.message || errorMessage + } catch { + // ignore parse errors + } + } else { + errorMessage = (await response.text()) || errorMessage + } + logger.error(`Preview action failed`, { + status: response.status, + error: errorMessage, + }) + throw error(response.status, errorMessage) + } + + const result = await response.json() + return json(result) + } catch (err) { + if (err && typeof err === 'object' && 'status' in err) { + throw err + } + logger.error('Error executing preview action:', err) + throw error(500, 'Internal server error') + } +} diff --git a/web/src/routes/api/preview-action/preview-action.test.ts b/web/src/routes/api/preview-action/preview-action.test.ts new file mode 100644 index 000000000..4759fcf76 --- /dev/null +++ b/web/src/routes/api/preview-action/preview-action.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' + +// Mock server dependencies before importing the handler. +vi.mock('$lib/server/config', () => ({ + getConfig: vi.fn(() => ({ + database: { url: 'postgresql://test:test@localhost:5432/test' }, + redis: { url: 'redis://localhost:6379' }, + services: { + searcherUrl: 'http://searcher.test', + indexerUrl: 'http://indexer.test', + aiServiceUrl: 'http://ai.test', + connectorManagerUrl: 'http://cm.test', + }, + session: { secret: 'test-secret', cookieName: 'test-cookie', durationDays: 30 }, + app: { publicUrl: 'http://localhost:3000' }, + })), +})) + +vi.mock('$lib/server/logger', () => ({ + logger: { error: vi.fn() }, +})) + +const { POST } = await import('./+server') + +function mockRequest(body: unknown, headers: Record = {}): Request { + return new Request('http://localhost/api/preview-action', { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...headers }, + body: JSON.stringify(body), + }) +} + +function mockLocals(): Record { + return { user: { id: 'admin-1', role: 'admin' } } +} + +/** + * Helper that calls POST and converts both success and thrown HttpError + * into a consistent { status, body? } shape so tests don't need .catch(). + */ +async function callPost( + body: unknown, + locals?: Record, +): Promise<{ status: number; body?: unknown }> { + try { + const response = await POST({ + request: mockRequest(body), + locals: locals ?? mockLocals(), + } as unknown as Parameters[0]) + return { status: response.status, body: await response.json().catch(() => undefined) } + } catch (err: unknown) { + // SvelteKit error() throws HttpError { status, body } + const httpError = err as { status?: number; body?: unknown } + if (httpError.status !== undefined) { + return { status: httpError.status, body: httpError.body } + } + throw err + } +} + +describe('POST /api/preview-action', () => { + beforeEach(() => { + vi.restoreAllMocks() + }) + + it('rejects non-admin users with 403', async () => { + const response = await callPost( + { + sourceType: 'google_drive', + action: 'discover_folders', + serviceAccountJson: '{}', + principalEmail: 'a@b.com', + domain: 'b.com', + }, + { user: null }, + ) + expect(response.status).toBe(401) + }) + + it('rejects non-admin role with 403', async () => { + const response = await callPost( + { + sourceType: 'google_drive', + action: 'discover_folders', + serviceAccountJson: '{}', + principalEmail: 'a@b.com', + domain: 'b.com', + }, + { user: { id: 'u1', role: 'member' } }, + ) + expect(response.status).toBe(403) + }) + + it('rejects unknown top-level fields', async () => { + const response = await callPost({ + sourceType: 'google_drive', + action: 'discover_folders', + unknownField: 'x', + serviceAccountJson: '{}', + principalEmail: 'a@b.com', + domain: 'b.com', + }) + expect(response.status).toBe(400) + const body = response.body as { message?: string } | undefined + expect(body?.message || '').toContain('Unknown field') + }) + + it('rejects unsupported action', async () => { + const response = await callPost({ + sourceType: 'google_drive', + action: 'delete_all_files', + serviceAccountJson: '{}', + principalEmail: 'a@b.com', + domain: 'b.com', + }) + expect(response.status).toBe(400) + }) + + it('rejects non-google_drive sourceType', async () => { + const response = await callPost({ + sourceType: 'gmail', + action: 'discover_folders', + serviceAccountJson: '{}', + principalEmail: 'a@b.com', + domain: 'b.com', + }) + expect(response.status).toBe(400) + }) + + it('rejects missing required credential fields', async () => { + const response = await callPost({ + sourceType: 'google_drive', + action: 'discover_folders', + principalEmail: 'a@b.com', + domain: 'b.com', + }) + expect(response.status).toBe(400) + }) + + it('rejects invalid service account JSON', async () => { + const response = await callPost({ + sourceType: 'google_drive', + action: 'discover_folders', + serviceAccountJson: '{invalid}', + principalEmail: 'a@b.com', + domain: 'b.com', + }) + expect(response.status).toBe(400) + }) +}) From e2089c33968a5055e6c2399c3daeeea90314a490 Mon Sep 17 00:00:00 2001 From: Praveen Sampath Date: Wed, 29 Jul 2026 04:03:53 +0000 Subject: [PATCH 2/4] feat(google): optimize filtered Drive sync --- connectors/google/src/connector.rs | 106 +- connectors/google/src/drive.rs | 206 ++- connectors/google/src/models.rs | 406 +++--- connectors/google/src/sync.rs | 1223 ++++++++++------- services/connector-manager/src/handlers.rs | 536 ++++---- services/connector-manager/src/lib.rs | 8 - services/connector-manager/src/models.rs | 49 +- .../google-drive-folder-selector.svelte | 2 +- web/src/routes/api/preview-action/+server.ts | 55 +- .../api/preview-action/preview-action.test.ts | 113 +- 10 files changed, 1422 insertions(+), 1282 deletions(-) diff --git a/connectors/google/src/connector.rs b/connectors/google/src/connector.rs index b6acab634..829cea6ed 100644 --- a/connectors/google/src/connector.rs +++ b/connectors/google/src/connector.rs @@ -8,7 +8,10 @@ use crate::auth::{ }; use crate::drive::DriveClient; use crate::gmail::{MessageFormat, MessagePart}; -use crate::models::{GoogleDirectoryUser, GoogleSyncCheckpoint, SearchUsersResponse}; +use crate::models::{ + DriveFolderDiscoveryEntry, DriveFolderDiscoveryResponse, GoogleDirectoryUser, + GoogleSyncCheckpoint, SearchUsersResponse, +}; use crate::sync::SyncManager; use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; @@ -838,11 +841,11 @@ impl GoogleConnector { .list_drives(&google_auth, principal_email) .await?; - let mut items: Vec = Vec::new(); + let mut items: Vec = Vec::new(); // 2. Add each shared drive as a selectable root for drive in &drives_response.drives { - items.push(crate::models::DriveFolderDiscoveryEntry { + items.push(DriveFolderDiscoveryEntry { id: drive.id.clone(), name: drive.name.clone(), path: format!("/{} (Shared Drive)", drive.name), @@ -867,7 +870,7 @@ impl GoogleConnector { })?; for folder in &folders.files { - items.push(crate::models::DriveFolderDiscoveryEntry { + items.push(DriveFolderDiscoveryEntry { id: folder.id.clone(), name: folder.name.clone(), path: format!("/{}/{}", drive.name, folder.name), @@ -877,7 +880,7 @@ impl GoogleConnector { } } - let result = crate::models::DriveFolderDiscoveryResponse { items }; + let result = DriveFolderDiscoveryResponse { items }; Ok(ActionResponse::success(serde_json::to_value(result)?).into_response()) } @@ -1266,9 +1269,7 @@ impl Connector for GoogleConnector { mod tests { use std::sync::Arc; - use omni_connector_sdk::{ - AuthType, Connector, SdkClient, ServiceCredential, ServiceProvider, SourceType, - }; + use omni_connector_sdk::{AuthType, Connector, SdkClient, ServiceCredential, ServiceProvider}; use serde_json::json; use crate::admin::AdminClient; @@ -1664,93 +1665,4 @@ mod tests { // Empty size assert!(parse_attachment_doc_id("CABc123%40mail.example.test:att:report.pdf:").is_err()); } - - // ======================================================================== - // discover_folders action tests - // ======================================================================== - - #[test] - fn discover_folders_action_registered_in_manifest() { - let connector = test_connector(); - let actions = connector.actions(); - let action = actions.iter().find(|a| a.name == "discover_folders"); - assert!( - action.is_some(), - "discover_folders action must be registered" - ); - let action = action.unwrap(); - assert!(action.admin_only, "discover_folders must be admin_only"); - assert!(action.hidden, "discover_folders must be hidden"); - assert_eq!( - action.source_types, - vec![SourceType::GoogleDrive], - "discover_folders must only accept google_drive source type" - ); - } - - #[test] - fn discover_folders_rejects_oauth_credentials() { - let connector = test_connector(); - let cred = test_service_credential(AuthType::OAuth, json!({})); - - let rt = tokio::runtime::Runtime::new().unwrap(); - let result = rt.block_on(async { - connector - .execute_action("discover_folders", json!({}), Some(cred), None, None) - .await - }); - - assert!(result.is_ok(), "execute_action should return OK response"); - let response = result.unwrap(); - let status = response.status(); - assert_eq!( - status, - axum::http::StatusCode::BAD_REQUEST, - "OAuth credentials should be rejected with 400" - ); - } - - #[test] - fn discover_folders_invalid_action_returns_not_supported() { - let connector = test_connector(); - let cred = test_service_credential(AuthType::Jwt, json!({})); - - let rt = tokio::runtime::Runtime::new().unwrap(); - let result = rt.block_on(async { - connector - .execute_action("nonexistent_action", json!({}), Some(cred), None, None) - .await - }); - - assert!(result.is_ok()); - let response = result.unwrap(); - assert_eq!(response.status(), axum::http::StatusCode::NOT_FOUND); - } - - #[test] - fn discover_folders_missing_principal_email_returns_500() { - let connector = test_connector(); - let mut cred = test_service_credential(AuthType::Jwt, json!({})); - cred.principal_email = None; - - let rt = tokio::runtime::Runtime::new().unwrap(); - let result = rt.block_on(async { - connector - .execute_action("discover_folders", json!({}), Some(cred), None, None) - .await - }); - - // Missing principal_email propagates as Err (anyhow) to the SDK layer - assert!( - result.is_err(), - "missing principal_email should produce an Err" - ); - let err = result.unwrap_err(); - let err_msg = format!("{:?}", err); - assert!( - err_msg.contains("principal_email") || err_msg.contains("principal"), - "error should mention missing principal_email: {}", - err_msg - ); - } } diff --git a/connectors/google/src/drive.rs b/connectors/google/src/drive.rs index a775fa6bb..8d37cff96 100644 --- a/connectors/google/src/drive.rs +++ b/connectors/google/src/drive.rs @@ -1042,19 +1042,79 @@ impl DriveClient { &self, token: &str, page_token: &str, + ) -> Result { + self.list_changes_internal(token, page_token, None).await + } + + /// Like `list_changes` but scoped to a single shared drive. + pub async fn list_changes_for_drive( + &self, + token: &str, + page_token: &str, + drive_id: &str, + ) -> Result { + self.list_changes_internal(token, page_token, Some(drive_id)) + .await + } + + /// Drive-scoped start page token. Required for per-drive incremental sync. + pub async fn get_start_page_token_for_drive( + &self, + token: &str, + drive_id: &str, + ) -> Result { + let url = format!("{}/changes/startPageToken", drive_api_base().as_str()); + let drive_id_owned = drive_id.to_string(); + + let params = vec![("supportsAllDrives", "true"), ("driveId", &drive_id_owned)]; + + let response = self + .client + .get(&url) + .bearer_auth(token) + .query(¶ms) + .send() + .await?; + + if !response.status().is_success() { + let error_text = response.text().await?; + return Err(anyhow!( + "Failed to get start page token for drive {}: {}", + drive_id, + error_text + )); + } + + let response_json: serde_json::Value = response.json().await?; + let start_page_token = response_json["startPageToken"] + .as_str() + .ok_or_else(|| anyhow!("Missing startPageToken in drive-scoped response"))?; + + Ok(start_page_token.to_string()) + } + + /// Internal helper that optionally scopes to a drive. + async fn list_changes_internal( + &self, + token: &str, + page_token: &str, + drive_id: Option<&str>, ) -> Result { let url = format!("{}/changes", drive_api_base().as_str()); - let params = vec![ + let mut params: Vec<(&str, &str)> = vec![ ("pageToken", page_token), ("includeItemsFromAllDrives", "true"), ("supportsAllDrives", "true"), ("includeRemoved", "true"), ( "fields", - "nextPageToken,changes(changeType,removed,file(id,name,mimeType,webViewLink,createdTime,modifiedTime,size,parents,shared,permissions(id,type,emailAddress,role),owners(emailAddress)),fileId,time)", + "nextPageToken,newStartPageToken,changes(changeType,removed,file(id,name,mimeType,webViewLink,createdTime,modifiedTime,size,parents,shared,permissions(id,type,emailAddress,domain,role,allowFileDiscovery,permissionDetails),owners(emailAddress)),fileId,time)", ), ]; + if let Some(did) = drive_id { + params.push(("driveId", did)); + } let response = self .client @@ -1163,6 +1223,142 @@ impl DriveClient { }) } + /// List all files (not just folders) in a single shared drive with the given cut-off. + /// Uses `corpora=drive` and `driveId` to keep the query scoped. + /// Paginates fully and returns all files. + pub async fn list_files_in_drive( + &self, + auth: &GoogleAuth, + user_email: &str, + drive_id: &str, + page_token: Option<&str>, + modified_after: Option<&str>, + ) -> Result { + let drive_id_owned = drive_id.to_string(); + let page_token = page_token.map(|s| s.to_string()); + let modified_after = modified_after.map(|s| s.to_string()); + + execute_with_auth_retry(auth, user_email, self.rate_limiter.clone(), |token| { + let drive_id = drive_id_owned.clone(); + let page_token = page_token.clone(); + let modified_after = modified_after.clone(); + async move { + let url = format!("{}/files", drive_api_base().as_str()); + let query = build_files_query(modified_after.as_deref()); + + let mut params: Vec<(&str, &str)> = vec![ + ("pageSize", "100"), + ("fields", "nextPageToken,files(id,name,mimeType,webViewLink,createdTime,modifiedTime,size,parents,shared,permissions(id,type,emailAddress,domain,role,allowFileDiscovery,permissionDetails),owners(emailAddress))"), + ("q", query.as_str()), + ("orderBy", "modifiedTime desc"), + ("supportsAllDrives", "true"), + ("includeItemsFromAllDrives", "true"), + ("corpora", "drive"), + ("driveId", &drive_id), + ]; + if let Some(ref pt) = page_token { + params.push(("pageToken", pt)); + } + + debug!("[GOOGLE API CALL] list_files_in_drive drive={}, page_token={:?}", drive_id, page_token); + let response = self + .client + .get(&url) + .bearer_auth(&token) + .query(¶ms) + .send() + .await + .with_context(|| format!("Failed to list files in drive {}", drive_id))?; + + let status = response.status(); + if !status.is_success() { + return classify_google_api_error(response, "Failed to list files in drive").await; + } + + let response_text = response.text().await?; + let parsed: FilesListResponse = serde_json::from_str(&response_text).map_err(|e| { + anyhow!( + "Failed to parse drive-scoped file list response: {}. Raw: {}", + e, response_text + ) + })?; + + Ok(ApiResult::Success(parsed)) + } + }).await + } + + /// List all files under a given folder (non-recursive, paginated). + /// + /// Lists ALL immediate children (files and sub-folders) regardless of + /// modification time so that old folders containing new files are never + /// missed. Date-based filtering is applied client-side in the caller. + /// Used by folder-subtree traversal in scoped filtered sync. + pub async fn list_files_in_folder( + &self, + auth: &GoogleAuth, + user_email: &str, + folder_id: &str, + drive_id: &str, + page_token: Option<&str>, + ) -> Result { + let folder_id_owned = folder_id.to_string(); + let drive_id_owned = drive_id.to_string(); + let page_token = page_token.map(|s| s.to_string()); + + execute_with_auth_retry(auth, user_email, self.rate_limiter.clone(), |token| { + let folder_id = folder_id_owned.clone(); + let drive_id = drive_id_owned.clone(); + let page_token = page_token.clone(); + async move { + let url = format!("{}/files", drive_api_base().as_str()); + + // No date-cutoff in the query — we must discover old folders + // that may contain new files. The caller applies the cutoff + // client-side to non-folder file types only. + let query = format!("trashed=false and '{}' in parents", folder_id); + + let mut params: Vec<(&str, &str)> = vec![ + ("pageSize", "100"), + ("fields", "nextPageToken,files(id,name,mimeType,webViewLink,createdTime,modifiedTime,size,parents,shared,permissions(id,type,emailAddress,domain,role,allowFileDiscovery,permissionDetails),owners(emailAddress))"), + ("q", query.as_str()), + ("supportsAllDrives", "true"), + ("includeItemsFromAllDrives", "true"), + ("corpora", "drive"), + ("driveId", &drive_id), + ]; + if let Some(ref pt) = page_token { + params.push(("pageToken", pt)); + } + + debug!("[GOOGLE API CALL] list_files_in_folder folder={}, drive={}, page_token={:?}", folder_id, drive_id, page_token); + let response = self + .client + .get(&url) + .bearer_auth(&token) + .query(¶ms) + .send() + .await + .with_context(|| format!("Failed to list files in folder {}", folder_id))?; + + let status = response.status(); + if !status.is_success() { + return classify_google_api_error(response, "Failed to list files in folder").await; + } + + let response_text = response.text().await?; + let parsed: FilesListResponse = serde_json::from_str(&response_text).map_err(|e| { + anyhow!( + "Failed to parse folder-scoped file list response: {}. Raw: {}", + e, response_text + ) + })?; + + Ok(ApiResult::Success(parsed)) + } + }).await + } + /// List immediate children (sub-folders only) of a given folder in any drive. /// /// Paginates through all pages and returns the full list, sorted by name. @@ -1537,12 +1733,6 @@ fn extract_text_from_presentation(presentation: &GooglePresentation) -> String { mod tests { use super::*; - #[test] - fn files_query_no_cutoff_returns_trashed_false_only() { - let query = build_files_query(None); - assert_eq!(query, "trashed=false"); - } - #[test] fn files_query_uses_modified_or_created_time_cutoff() { let query = build_files_query(Some("2025-01-01T00:00:00Z")); diff --git a/connectors/google/src/models.rs b/connectors/google/src/models.rs index 7f19f81c6..6df206c6f 100644 --- a/connectors/google/src/models.rs +++ b/connectors/google/src/models.rs @@ -154,10 +154,54 @@ pub struct GoogleConnectorState { pub webhook_expires_at: Option, } +/// Normalised scope fingerprint for shared-drive/folder filtered mode. +/// `"all"` means no folder filters (index everything). +/// Otherwise it is a deterministic representation of the effective drive/folder scope. +pub fn compute_scope_fingerprint(filters: Option<&[FolderPathFilterEntry]>) -> String { + match filters { + None | Some([]) => "all".to_string(), + Some(list) => { + let entire_drives: HashSet<&str> = list + .iter() + .filter(|filter| matches!(filter.kind, FolderPathFilterKind::SharedDriveRoot)) + .map(|filter| filter.drive_id.as_str()) + .collect(); + let mut parts: Vec = list + .iter() + .filter(|filter| { + matches!(filter.kind, FolderPathFilterKind::SharedDriveRoot) + || !entire_drives.contains(filter.drive_id.as_str()) + }) + .map(|filter| format!("{}:{}:{}", filter.drive_id, filter.kind.as_str(), filter.id)) + .collect(); + parts.sort(); + parts.dedup(); + parts.join(";") + } + } +} + +impl FolderPathFilterKind { + pub fn as_str(&self) -> &'static str { + match self { + FolderPathFilterKind::SharedDriveRoot => "shared_drive_root", + FolderPathFilterKind::Folder => "folder", + } + } +} + #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct GoogleSyncCheckpoint { pub gmail_history_ids: Option>, pub drive_page_tokens: Option>, + /// Per-drive change tokens used in scoped (filtered) incremental sync. + /// Keyed by shared-drive ID; absent for unfiltered mode. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub drive_change_tokens: Option>, + /// Scope fingerprint for detecting filter transitions. + /// `"all"` or a sorted `;`-joined list of selected drive/kind/ID tuples. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub drive_scope_fingerprint: Option, pub chat: Option, } @@ -535,6 +579,10 @@ impl WebhookNotification { pub struct DriveChangesResponse { #[serde(rename = "nextPageToken")] pub next_page_token: Option, + /// The starting page token for future incremental changes. + /// Returned on the last page of a changes.list response. + #[serde(rename = "newStartPageToken", default)] + pub new_start_page_token: Option, pub changes: Vec, } @@ -1787,253 +1835,155 @@ mod tests { // ======================================================================== // Folder path filter parse tests + // + // Focus on behaviour rather than Serde mechanics: + // 1. absent / null / empty — the three filter knob states + // 2. successful parse + first-wins deduplication + // 3. a single table of malformed inputs that must all fail closed // ======================================================================== #[test] - fn test_parse_folder_path_filters_absent_key() { - // Key absent → Ok(None) → index everything. - let config = serde_json::json!({"domain": "example.com"}); - let result = parse_folder_path_filters(&config); - assert!(result.is_ok(), "absent key should be Ok"); - assert!(result.unwrap().is_none(), "absent key → None"); - } - - #[test] - fn test_parse_folder_path_filters_null_value() { - // Explicit null → Err (fail closed). - let config = serde_json::json!({"folder_path_filters": null}); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "null should be rejected"); - assert!( - result.unwrap_err().contains("must not be null"), - "error should explain null is not allowed" - ); - } - - #[test] - fn test_parse_folder_path_filters_empty_array() { - // Empty array [] → Ok(None) → index everything (valid, intentional). - let config = serde_json::json!({"folder_path_filters": []}); - let result = parse_folder_path_filters(&config); - assert!(result.is_ok(), "empty array should be Ok"); - assert!(result.unwrap().is_none(), "empty array → None"); - } + fn test_folder_filter_absent_or_empty_is_none() { + // Absent key. + let r = parse_folder_path_filters(&json!({"domain": "x"})); + assert!(r.is_ok() && r.unwrap().is_none()); - #[test] - fn test_parse_folder_path_filters_valid_shared_drive_root() { - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "0AK123456789", - "name": "Marketing", - "path": "/Marketing (Shared Drive)", - "driveId": "0AK123456789", - "kind": "shared_drive_root" - }] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_ok()); - let filters = result.unwrap(); - assert!(filters.is_some()); - let filters = filters.unwrap(); - assert_eq!(filters.len(), 1); - assert_eq!(filters[0].id, "0AK123456789"); - assert!(matches!( - filters[0].kind, - FolderPathFilterKind::SharedDriveRoot - )); - } - - #[test] - fn test_parse_folder_path_filters_valid_folder() { - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "1ABC123DEF", - "name": "Reports", - "path": "/Marketing/Reports", - "driveId": "0AK123456789", - "kind": "folder" - }] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_ok()); - let filters = result.unwrap().unwrap(); - assert_eq!(filters[0].name, "Reports"); - assert!(matches!(filters[0].kind, FolderPathFilterKind::Folder)); + // Empty array (explicit “no filter”). + let r = parse_folder_path_filters(&json!({"folder_path_filters": []})); + assert!(r.is_ok() && r.unwrap().is_none()); } #[test] - fn test_parse_folder_path_filters_malformed_string() { - // String value instead of array → Err (fail closed). - let config = serde_json::json!({"folder_path_filters": "not-an-array"}); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "malformed string should be Err"); - let err = result.unwrap_err(); - assert!(!err.is_empty(), "error message should be non-empty"); - } - - #[test] - fn test_parse_folder_path_filters_malformed_array_with_primitives() { - // Array of primitives fails serde → Err (fail closed). - let config = serde_json::json!({"folder_path_filters": ["invalid"]}); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "array of non-objects should be Err"); - } - - #[test] - fn test_parse_folder_path_filters_missing_required_fields() { - // Entry with missing name/path/driveId/kind fails serde → Err. - let config = serde_json::json!({ - "folder_path_filters": [{"id": "only-id"}] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "entry with missing fields should be Err"); - } - - #[test] - fn test_parse_folder_path_filters_unknown_kind() { - // Unknown kind variant fails serde → Err (fail closed). - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "1", - "name": "Test", - "path": "/Test", - "driveId": "1", - "kind": "unknown_kind" - }] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "unknown kind should be Err"); - } - - #[test] - fn test_folder_path_filter_serialization_roundtrip() { - let entry = FolderPathFilterEntry { - id: "0ATEST123".to_string(), - name: "Test Drive".to_string(), - path: "/Test Drive (Shared Drive)".to_string(), - drive_id: "0ATEST123".to_string(), - kind: FolderPathFilterKind::SharedDriveRoot, - }; - let json = serde_json::to_value(&entry).unwrap(); - assert_eq!(json["id"], "0ATEST123"); - assert_eq!(json["kind"], "shared_drive_root"); - let deserialized: FolderPathFilterEntry = serde_json::from_value(json).unwrap(); - assert_eq!(deserialized.id, entry.id); - } - - #[test] - fn test_drive_folder_discovery_response_serialization() { - let response = DriveFolderDiscoveryResponse { - items: vec![DriveFolderDiscoveryEntry { - id: "drive1".to_string(), - name: "Company Drive".to_string(), - path: "/Company Drive (Shared Drive)".to_string(), - drive_id: "drive1".to_string(), - kind: "shared_drive_root".to_string(), - }], - }; - let json = serde_json::to_value(&response).unwrap(); - assert_eq!(json["items"][0]["kind"], "shared_drive_root"); - } - - #[test] - fn test_parse_folder_path_filters_deny_unknown_fields() { - // deny_unknown_fields on FolderPathFilterEntry rejects extra keys. - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "1", - "name": "T", - "path": "/T", - "driveId": "1", - "kind": "folder", - "extra_field": "should_not_exist" - }] + fn test_folder_filter_valid_parse_with_dedup() { + let config = json!({ + "folder_path_filters": [ + { + "id": "0ADRI000001", + "name": "Engineering", + "path": "/Engineering (Shared Drive)", + "driveId": "0ADRI000001", + "kind": "shared_drive_root" + }, + { + "id": "1FOLDER0001", + "name": "Docs", + "path": "/Engineering/Docs", + "driveId": "0ADRI000001", + "kind": "folder" + }, + // duplicate of the first – must be silently dropped + { + "id": "0ADRI000001", + "name": "Duplicate", + "path": "/Engineering (Shared Drive)", + "driveId": "0ADRI000001", + "kind": "shared_drive_root" + } + ] }); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "unknown fields should be rejected"); - let err = result.unwrap_err(); - assert!( - err.contains("unknown field") || err.contains("extra_field"), - "error should mention the unknown field: {}", - err - ); + let r = parse_folder_path_filters(&config).unwrap().unwrap(); + assert_eq!(r.len(), 2, "duplicate id deduped away"); + assert_eq!(r[0].name, "Engineering", "first occurrence wins"); + assert_eq!(r[1].name, "Docs"); + assert!(matches!(r[0].kind, FolderPathFilterKind::SharedDriveRoot)); + assert!(matches!(r[1].kind, FolderPathFilterKind::Folder)); } #[test] - fn test_parse_folder_path_filters_empty_id_rejected() { - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "", - "name": "Test", - "path": "/Test", - "driveId": "1", - "kind": "folder" - }] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "empty id should be rejected"); + fn test_folder_filter_malformed_rejected() { + // Table of invalid configurations — every row must produce Err. + let cases: Vec<(&str, serde_json::Value)> = vec![ + ("null", serde_json::Value::Null), + ("not an array", json!("not-an-array")), + ("array of primitives", json!(["invalid"])), + ("missing required fields", json!([{"id": "only-id"}])), + ( + "unknown kind", + json!([{ + "id":"1", "name":"T", "path":"/T", "driveId":"1", "kind":"nope" + }]), + ), + ( + "unknown extra field", + json!([{ + "id":"1", "name":"T", "path":"/T", "driveId":"1", "kind":"folder", + "extra":"x" + }]), + ), + ( + "empty id", + json!([{ + "id":"", "name":"T", "path":"/T", "driveId":"1", "kind":"folder" + }]), + ), + ( + "empty name", + json!([{ + "id":"1", "name":"", "path":"/T", "driveId":"1", "kind":"folder" + }]), + ), + ( + "empty driveId", + json!([{ + "id":"1", "name":"T", "path":"/T", "driveId":"", "kind":"folder" + }]), + ), + ]; + for (label, val) in &cases { + let config = json!({"folder_path_filters": val}); + assert!( + parse_folder_path_filters(&config).is_err(), + "should reject: {}", + label + ); + } } - #[test] - fn test_parse_folder_path_filters_empty_name_rejected() { - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "1", - "name": "", - "path": "/Test", - "driveId": "1", - "kind": "folder" - }] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "empty name should be rejected"); - } + // ======================================================================== + // Scope fingerprint tests + // ======================================================================== #[test] - fn test_parse_folder_path_filters_empty_drive_id_rejected() { - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "1", - "name": "Test", - "path": "/Test", - "driveId": "", - "kind": "folder" - }] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_err(), "empty driveId should be rejected"); + fn scope_fingerprint_none_returns_all() { + assert_eq!(compute_scope_fingerprint(None), "all"); + assert_eq!(compute_scope_fingerprint(Some(&[])), "all"); } #[test] - fn test_parse_folder_path_filters_duplicate_id_rejected() { - let config = serde_json::json!({ - "folder_path_filters": [ - { - "id": "dup-id", - "name": "First", - "path": "/First", - "driveId": "d1", - "kind": "folder" - }, - { - "id": "dup-id", - "name": "Second", - "path": "/Second", - "driveId": "d2", - "kind": "folder" - } - ] - }); - let result = parse_folder_path_filters(&config); - assert!(result.is_ok(), "duplicate id should be deduplicated"); - let filters = result.unwrap(); - assert!(filters.is_some()); - // Deduplication means only the first occurrence is kept. + fn scope_fingerprint_deterministic_sorted() { + let filters = vec![ + FolderPathFilterEntry { + id: "drive-b".to_string(), + name: "B".to_string(), + path: "/B".to_string(), + drive_id: "b".to_string(), + kind: FolderPathFilterKind::SharedDriveRoot, + }, + FolderPathFilterEntry { + id: "drive-a".to_string(), + name: "A".to_string(), + path: "/A".to_string(), + drive_id: "a".to_string(), + kind: FolderPathFilterKind::SharedDriveRoot, + }, + FolderPathFilterEntry { + id: "redundant-folder".to_string(), + name: "Folder".to_string(), + path: "/A/Folder".to_string(), + drive_id: "a".to_string(), + kind: FolderPathFilterKind::Folder, + }, + ]; + let fp = compute_scope_fingerprint(Some(&filters)); + // Fingerprint reflects effective scope: the selected drive root supersedes + // its redundant folder selection, and remaining entries are sorted. assert_eq!( - filters.as_ref().unwrap().len(), - 1, - "duplicate id → deduped to 1" + fp, + "a:shared_drive_root:drive-a;b:shared_drive_root:drive-b" ); - assert_eq!(filters.unwrap()[0].name, "First", "first occurrence wins"); + + // Reverse input order should produce same result. + let filters_rev: Vec = filters.into_iter().rev().collect(); + assert_eq!(compute_scope_fingerprint(Some(&filters_rev)), fp); } } diff --git a/connectors/google/src/sync.rs b/connectors/google/src/sync.rs index e9ffad1b4..026a1e722 100644 --- a/connectors/google/src/sync.rs +++ b/connectors/google/src/sync.rs @@ -2,11 +2,11 @@ use anyhow::{anyhow, Context, Result}; use dashmap::DashMap; use futures::{stream, StreamExt}; use omni_connector_sdk::SyncContext; -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::future::Future; use std::pin::Pin; use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::{Arc, RwLock}; +use std::sync::Arc; use std::time::{Duration, Instant}; use time::{self, OffsetDateTime}; use tokio::sync::{Mutex, Notify, OwnedSemaphorePermit, Semaphore}; @@ -85,6 +85,14 @@ fn google_drive_max_download_bytes() -> usize { .unwrap_or(DEFAULT_GOOGLE_DRIVE_MAX_DOWNLOAD_BYTES) } +fn drive_scope_requires_full_sync( + sync_type: SyncType, + stored_fingerprint: Option<&str>, + current_fingerprint: &str, +) -> bool { + sync_type != SyncType::Incremental || stored_fingerprint != Some(current_fingerprint) +} + fn google_webhook_debounce_duration_ms() -> u64 { std::env::var("GOOGLE_WEBHOOK_DEBOUNCE_SECONDS") .ok() @@ -101,105 +109,6 @@ struct DriveContentCache { locks: DashMap>>, } -/// Invocation-local cache for folder-metadata lookups during folder-path filter -/// ancestry resolution. Shared via `Arc` across parallel user tasks within a -/// single sync run so repeated lookups for the same folder ID are served from -/// memory rather than re-fetched from the Drive API. -pub(crate) struct FolderAncestryCache { - cache: RwLock)>>, - pub hits: AtomicU64, -} - -impl FolderAncestryCache { - pub fn new() -> Self { - Self { - cache: RwLock::new(HashMap::new()), - hits: AtomicU64::new(0), - } - } - - /// Look up a folder's metadata, using the cache if available. - /// Returns `(name, Option)`. - pub async fn get_or_fetch( - &self, - drive_client: &DriveClient, - auth: &GoogleAuth, - user_email: &str, - folder_id: &str, - ) -> Result<(String, Option)> { - // Fast path: check cache under read lock. - { - let cache = self - .cache - .read() - .map_err(|e| anyhow!("FolderAncestryCache read lock error: {:?}", e))?; - if let Some(entry) = cache.get(folder_id) { - self.hits.fetch_add(1, Ordering::Relaxed); - return Ok(entry.clone()); - } - } - // Miss: fetch from API. - let meta = drive_client - .get_folder_metadata(auth, user_email, folder_id) - .await - .with_context(|| { - format!( - "Failed to fetch folder metadata for {} while evaluating folder-path filter", - folder_id - ) - })?; - let result = ( - meta.name.clone(), - meta.parents.as_ref().and_then(|p| p.first()).cloned(), - ); - // Store under write lock (first-wins silently if another task arrived first). - { - let mut cache = self - .cache - .write() - .map_err(|e| anyhow!("FolderAncestryCache write lock error: {:?}", e))?; - cache - .entry(folder_id.to_string()) - .or_insert_with(|| result.clone()); - } - Ok(result) - } - - /// Test-only: lookup with a custom async fetcher instead of DriveClient. - #[cfg(test)] - pub(crate) async fn get_or_fetch_test( - &self, - folder_id: &str, - fetcher: F, - ) -> Result<(String, Option)> - where - F: Fn() -> Fut, - Fut: Future)>> + Send, - { - { - let cache = self - .cache - .read() - .map_err(|e| anyhow!("FolderAncestryCache read lock error: {:?}", e))?; - if let Some(entry) = cache.get(folder_id) { - self.hits.fetch_add(1, Ordering::Relaxed); - return Ok(entry.clone()); - } - } - let result = fetcher().await?; - { - let mut cache = self - .cache - .write() - .map_err(|e| anyhow!("FolderAncestryCache write lock error: {:?}", e))?; - cache - .entry(folder_id.to_string()) - .or_insert_with(|| result.clone()); - } - Ok(result) - } -} - impl DriveContentCache { fn get_content_id(&self, file_id: &str) -> Option { self.content_ids @@ -1163,9 +1072,8 @@ impl SyncManager { // parent ancestry no longer intersects the configured filter set. /// Check whether a file should be included based on configured folder-path filters. - /// Delegates to the module-level `check_ancestry_static` production walker with - /// a closure that calls the Drive API via the client (no caching during the walk; - /// the folder cache is still used by other production code). + /// Uses `self.folder_cache` (the same LRU cache used by `build_full_path`) for + /// folder-metadata lookups during ancestry resolution. /// /// Returns `Ok(true)` if no filters are configured (include-everything mode). /// Returns `Ok(false)` if the file's ancestry is outside the configured scope. @@ -1174,7 +1082,6 @@ impl SyncManager { /// silently including or excluding the file. async fn is_file_in_allowed_folder( &self, - ancestry_cache: &Option>, auth: &GoogleAuth, user_email: &str, file: &crate::models::GoogleDriveFile, @@ -1184,46 +1091,38 @@ impl SyncManager { return Ok(true); } + let folder_cache = self.folder_cache.clone(); let drive_client = self.drive_client.clone(); let google_auth = auth.clone(); let user_email_owned = user_email.to_string(); - let ancestry_cache_ref = ancestry_cache.clone(); let lookup = move |folder_id: String| -> Pin< Box)>> + Send>, > { + let folder_cache = folder_cache.clone(); let drive_client = drive_client.clone(); let google_auth = google_auth.clone(); let user_email_owned = user_email_owned.clone(); - let ancestry_cache_ref = ancestry_cache_ref.clone(); Box::pin(async move { - match ancestry_cache_ref { - Some(cache) => { - cache - .get_or_fetch( - &drive_client, - &google_auth, - &user_email_owned, - &folder_id, - ) - .await - } - None => { - // Fallback: fetch directly (no cache available — should not - // happen when filters are configured). - let meta = drive_client - .get_folder_metadata(&google_auth, &user_email_owned, &folder_id) - .await - .with_context(|| { - format!( - "Failed to fetch folder metadata for {} while evaluating folder-path filter", - folder_id - ) - })?; - let parent = meta.parents.as_ref().and_then(|p| p.first()).cloned(); - Ok((meta.name, parent)) - } + // Try the LRU folder cache first (same cache used by build_full_path). + if let Some(cached) = folder_cache.get(&folder_id) { + let parent = cached.parents.as_ref().and_then(|p| p.first()).cloned(); + return Ok((cached.name, parent)); } + // Cache miss: fetch from API and populate cache. + let meta = drive_client + .get_folder_metadata(&google_auth, &user_email_owned, &folder_id) + .await + .with_context(|| { + format!( + "Failed to fetch folder metadata for {} while evaluating folder-path filter", + folder_id + ) + })?; + let name = meta.name.clone(); + let parent = meta.parents.as_ref().and_then(|p| p.first()).cloned(); + folder_cache.insert(folder_id.clone(), meta.into()); + Ok((name, parent)) }) }; @@ -1240,7 +1139,6 @@ impl SyncManager { created_after: Option<&str>, content_cache: Arc, folder_filter_ids: Option>>, - folder_ancestry_cache: Option>, ) -> Result<(usize, usize)> { info!("Processing Drive files for user: {}", user_email); @@ -1287,7 +1185,6 @@ impl SyncManager { if let Some(ref allowed_ids) = folder_filter_ids { if !self .is_file_in_allowed_folder( - &folder_ancestry_cache, &service_auth, user_email, &file, @@ -1376,7 +1273,6 @@ impl SyncManager { start_page_token: &str, content_cache: Arc, folder_filter_ids: Option>>, - folder_ancestry_cache: Option>, ) -> Result<(usize, usize)> { info!( "Processing incremental Drive sync for user {} from pageToken {}", @@ -1444,13 +1340,7 @@ impl SyncManager { // Errors propagate upward so the sync fails and can be retried. if let Some(ref allowed_ids) = folder_filter_ids { if !self - .is_file_in_allowed_folder( - &folder_ancestry_cache, - &service_auth, - user_email, - &file, - allowed_ids, - ) + .is_file_in_allowed_folder(&service_auth, user_email, &file, allowed_ids) .await? { debug!( @@ -1884,6 +1774,512 @@ impl SyncManager { Ok((scanned, updated)) } + /// Build a scope-aware Drive checkpoint that preserves Gmail and Chat state. + fn build_drive_checkpoint( + &self, + existing: &GoogleSyncCheckpoint, + drive_page_tokens: Option>, + drive_change_tokens: Option>, + drive_scope_fingerprint: Option, + ) -> GoogleSyncCheckpoint { + GoogleSyncCheckpoint { + gmail_history_ids: existing.gmail_history_ids.clone(), + drive_page_tokens, + drive_change_tokens, + drive_scope_fingerprint, + chat: existing.chat.clone(), + } + } + + /// Helper: apply the date cutoff to a single Google Drive file. + /// Returns `true` if the file passes the cutoff (or no cutoff is set). + /// Folders are never filtered by the cutoff — they must be discovered + /// regardless of age so their contents can be indexed. + fn pass_cutoff(file: &crate::models::GoogleDriveFile, cutoff: OffsetDateTime) -> bool { + if file.mime_type == "application/vnd.google-apps.folder" { + return true; + } + file.modified_time + .as_deref() + .and_then(|value| parse_google_time(Some(value))) + .is_some_and(|modified| modified > cutoff) + || file + .created_time + .as_deref() + .and_then(|value| parse_google_time(Some(value))) + .is_some_and(|created| created > cutoff) + } + + /// Flush a batch of user files through `process_file_batch` and update totals. + async fn flush_batch( + &self, + batch: &mut Vec, + source_id: &str, + sync_run_id: &str, + ctx: &SyncContext, + service_auth: Arc, + content_cache: Arc, + total_scanned: &mut usize, + total_updated: &mut usize, + ) -> Result<()> { + if batch.is_empty() { + return Ok(()); + } + let (s, u) = self + .process_file_batch( + std::mem::take(batch), + source_id, + sync_run_id, + ctx, + service_auth, + content_cache, + ) + .await?; + *total_scanned += s; + *total_updated += u; + Ok(()) + } + + /// Scoped full sync: crawl selected shared drives and/or folder subtrees only, + /// using the configured DWD principal (no user impersonation/iteration). + async fn sync_drive_scoped( + &self, + source: &Source, + service_creds: &ServiceCredential, + sync_type: SyncType, + existing_state: &GoogleSyncCheckpoint, + ctx: &SyncContext, + ) -> Result { + let sync_run_id = ctx.sync_run_id(); + let service_auth = Arc::new(self.create_auth(service_creds, source.source_type).await?); + let (drive_cutoff_date, _gmail_cutoff_date) = self.get_cutoff_date()?; + let drive_cutoff = parse_google_time(Some(&drive_cutoff_date)) + .ok_or_else(|| anyhow!("Invalid Drive cutoff time: {}", drive_cutoff_date))?; + + // Parse filters; will error on malformed config. + let filter_entries = match crate::models::parse_folder_path_filters(&source.config) { + Ok(Some(f)) => f, + Ok(None) => { + // Should not happen — caller guards this branch — but be safe. + return Err(anyhow!("sync_drive_scoped called without filters")); + } + Err(e) => { + return Err(anyhow!("Invalid folder path filter configuration: {}", e)); + } + }; + + let current_fingerprint = crate::models::compute_scope_fingerprint(Some(&filter_entries)); + + // Detect scope transition. If the fingerprint differs from the + // checkpoint, force a full sync regardless of requested mode. + let stored_fingerprint = existing_state.drive_scope_fingerprint.as_deref(); + let scope_changed = stored_fingerprint != Some(¤t_fingerprint); + + if scope_changed { + info!( + "Drive scope transition detected: {:?} -> {} — forcing full sync", + stored_fingerprint, current_fingerprint + ); + } + let effective_full = + drive_scope_requires_full_sync(sync_type, stored_fingerprint, ¤t_fingerprint); + + // Build drive groups: driveId → (selected folder IDs, whether entire drive is selected) + struct DriveGroup { + drive_id: String, + entire_drive: bool, + folder_ids: Vec, + } + + let mut drive_groups: HashMap = HashMap::new(); + for entry in &filter_entries { + let group = drive_groups + .entry(entry.drive_id.clone()) + .or_insert(DriveGroup { + drive_id: entry.drive_id.clone(), + entire_drive: false, + folder_ids: Vec::new(), + }); + match entry.kind { + crate::models::FolderPathFilterKind::SharedDriveRoot => { + group.entire_drive = true; + group.folder_ids.clear(); // entire-drive supersedes per-folder. + } + crate::models::FolderPathFilterKind::Folder => { + if !group.entire_drive { + group.folder_ids.push(entry.id.clone()); + } + } + } + } + + let drives: Vec = drive_groups.into_values().collect(); + if drives.is_empty() { + info!("No valid drive scopes found — indexing nothing"); + return Ok(self.build_drive_checkpoint( + existing_state, + None, + None, + Some(current_fingerprint), + )); + } + + // Resolve the delegated principal email. + let principal_email = service_creds + .principal_email + .as_deref() + .ok_or_else(|| anyhow!("Missing principal_email for scoped Drive sync"))? + .to_string(); + + // Shared-drive operations need a single access token (no impersonation per user). + let access_token = service_auth.get_access_token(&principal_email).await?; + + let content_cache = Arc::new(DriveContentCache::default()); + let mut total_scanned = 0; + let mut total_updated = 0; + + let mut new_change_tokens: HashMap = HashMap::new(); + let old_change_tokens = existing_state + .drive_change_tokens + .clone() + .unwrap_or_default(); + const BATCH_SIZE: usize = 200; + + for drive_group in &drives { + if ctx.is_cancelled() { + info!("Sync {} cancelled during drive-scoped sync", sync_run_id); + break; + } + + info!( + "Processing drive {} (entire={}, folders={:?})", + drive_group.drive_id, drive_group.entire_drive, drive_group.folder_ids + ); + + let stored_token = old_change_tokens.get(&drive_group.drive_id); + let use_incremental = !effective_full && stored_token.is_some(); + + if use_incremental { + // Incremental per-drive changes path. Use newStartPageToken + // from the API response to advance the token. + let start_token = stored_token.unwrap(); + let mut current_token = start_token.clone(); + + loop { + let response = self + .drive_client + .list_changes_for_drive( + &access_token, + ¤t_token, + &drive_group.drive_id, + ) + .await?; + + if ctx.is_cancelled() { + break; + } + + let mut file_batch: Vec = Vec::new(); + + for change in &response.changes { + let is_removed = change.removed.unwrap_or(false); + + if is_removed { + if let Some(file_id) = &change.file_id { + info!( + "File {} was removed (scoped incremental), publishing deletion", + file_id + ); + self.publish_deletion_event(ctx, file_id).await?; + } + continue; + } + + if let Some(file) = &change.file { + if !self.should_index_file(file) { + continue; + } + + // For folder-only selections, ancestry-check the change. + if !drive_group.entire_drive { + let allowed: HashSet = + drive_group.folder_ids.iter().cloned().collect(); + if !self + .is_file_in_allowed_folder( + &service_auth, + &principal_email, + file, + &allowed, + ) + .await? + { + debug!( + "Skipping changed file {} ({}) — outside folder scope", + file.name, file.id + ); + continue; + } + } + + file_batch.push(UserFile { + user_email: Arc::new(principal_email.clone()), + file: file.clone(), + }); + + if file_batch.len() >= BATCH_SIZE { + self.flush_batch( + &mut file_batch, + &source.id, + &sync_run_id, + ctx, + service_auth.clone(), + content_cache.clone(), + &mut total_scanned, + &mut total_updated, + ) + .await?; + } + } + } + + // Flush remaining changes for this page. + self.flush_batch( + &mut file_batch, + &source.id, + &sync_run_id, + ctx, + service_auth.clone(), + content_cache.clone(), + &mut total_scanned, + &mut total_updated, + ) + .await?; + + // Use newStartPageToken from response if present. + if let Some(new_token) = &response.new_start_page_token { + new_change_tokens.insert(drive_group.drive_id.clone(), new_token.clone()); + } + + match response.next_page_token { + Some(token) => current_token = token, + None => break, + } + } + + // Fallback: if no newStartPageToken was received, fetch one. + if !new_change_tokens.contains_key(&drive_group.drive_id) { + match self + .drive_client + .get_start_page_token_for_drive(&access_token, &drive_group.drive_id) + .await + { + Ok(token) => { + new_change_tokens.insert(drive_group.drive_id.clone(), token); + } + Err(e) => { + warn!( + "Failed to get start page token for drive {}: {}", + drive_group.drive_id, e + ); + } + } + } + } else { + // Capture the drive-scoped token before the full crawl so changes + // made while pagination is in progress are replayed next time. + let next_change_token = match self + .drive_client + .get_start_page_token_for_drive(&access_token, &drive_group.drive_id) + .await + { + Ok(token) => Some(token), + Err(error) => { + warn!( + "Failed to get start page token for drive {}: {}", + drive_group.drive_id, error + ); + None + } + }; + + // Full scoped sync for this drive. Stream directly into batches. + let mut file_batch: Vec = Vec::new(); + + if drive_group.entire_drive { + // Entire-drive selection: list all files with corpora=drive + let mut page_token: Option = None; + loop { + let response = self + .drive_client + .list_files_in_drive( + &service_auth, + &principal_email, + &drive_group.drive_id, + page_token.as_deref(), + Some(&drive_cutoff_date), + ) + .await?; + + for file in response.files { + if self.should_index_file(&file) { + file_batch.push(UserFile { + user_email: Arc::new(principal_email.clone()), + file, + }); + if file_batch.len() >= BATCH_SIZE { + self.flush_batch( + &mut file_batch, + &source.id, + &sync_run_id, + ctx, + service_auth.clone(), + content_cache.clone(), + &mut total_scanned, + &mut total_updated, + ) + .await?; + } + } + } + + if ctx.is_cancelled() { + break; + } + + page_token = response.next_page_token; + if page_token.is_none() { + break; + } + } + } else { + // Folder-level selection: traverse each selected subtree. + let mut seen_file_ids: HashSet = HashSet::new(); + let mut visited_folders: HashSet = HashSet::new(); + + for folder_id in &drive_group.folder_ids { + if ctx.is_cancelled() { + break; + } + if !visited_folders.insert(folder_id.clone()) { + continue; // already traversed via overlapping selection + } + + let mut queue = VecDeque::from([folder_id.clone()]); + while let Some(current_folder_id) = queue.pop_front() { + if ctx.is_cancelled() { + break; + } + + // Mark visited to avoid re-traversal via + // overlapping folder selections. + visited_folders.insert(current_folder_id.clone()); + + let mut page_token: Option = None; + loop { + // List ALL children — no date cutoff so old + // folders containing new files are never missed. + let response = self + .drive_client + .list_files_in_folder( + &service_auth, + &principal_email, + ¤t_folder_id, + &drive_group.drive_id, + page_token.as_deref(), + ) + .await?; + + for file in response.files { + if seen_file_ids.contains(&file.id) { + continue; + } + seen_file_ids.insert(file.id.clone()); + + // Folders are enqueued unconditionally. + if file.mime_type == "application/vnd.google-apps.folder" + && !visited_folders.contains(&file.id) + { + queue.push_back(file.id.clone()); + } + + // Non-folder files: apply cutoff locally. + if !self.should_index_file(&file) { + continue; + } + if !Self::pass_cutoff(&file, drive_cutoff) { + continue; + } + + file_batch.push(UserFile { + user_email: Arc::new(principal_email.clone()), + file, + }); + if file_batch.len() >= BATCH_SIZE { + self.flush_batch( + &mut file_batch, + &source.id, + &sync_run_id, + ctx, + service_auth.clone(), + content_cache.clone(), + &mut total_scanned, + &mut total_updated, + ) + .await?; + } + } + + if ctx.is_cancelled() { + break; + } + + page_token = response.next_page_token; + if page_token.is_none() { + break; + } + } + } + } + } + + // Flush any remaining files for this drive. + self.flush_batch( + &mut file_batch, + &source.id, + &sync_run_id, + ctx, + service_auth.clone(), + content_cache.clone(), + &mut total_scanned, + &mut total_updated, + ) + .await?; + + if ctx.is_cancelled() { + break; + } + + if let Some(token) = next_change_token { + new_change_tokens.insert(drive_group.drive_id.clone(), token); + } + } + } + + info!( + "Scoped Drive sync completed for source {}: {} scanned, {} updated", + source.id, total_scanned, total_updated + ); + + self.folder_cache.clear(); + + Ok(self.build_drive_checkpoint( + existing_state, + None, // No per-user page tokens in scoped mode. + Some(new_change_tokens), + Some(current_fingerprint), + )) + } + async fn sync_drive_source_internal( &self, source: &Source, @@ -1893,10 +2289,46 @@ impl SyncManager { ctx: &SyncContext, ) -> Result { let sync_run_id = ctx.sync_run_id(); - let service_auth = Arc::new(self.create_auth(service_creds, source.source_type).await?); - // Calculate cutoff date for filtering + // Parse folder-path filters. Malformed config is a sync error. + let parsed_filters = match crate::models::parse_folder_path_filters(&source.config) { + Ok(f) => f, + Err(e) => { + return Err(anyhow!("Invalid folder path filter configuration: {}", e)); + } + }; + + // Scoped mode (filters present, non-OAuth): delegate to sync_drive_scoped. + if parsed_filters.is_some() && !service_auth.is_oauth() { + return self + .sync_drive_scoped(source, service_creds, sync_type, &existing_state, ctx) + .await; + } + + // === Unfiltered / OAuth path: existing all-user listing === + + // Compute fingerprint. For OAuth sources, filters are ignored — + // folder filtering is DWD-only — so the fingerprint is always "all". + let current_fingerprint = if service_auth.is_oauth() { + "all".to_string() + } else { + match &parsed_filters { + Some(f) => crate::models::compute_scope_fingerprint(Some(f)), + None => "all".to_string(), + } + }; + + // Detect scope transition and force full sync if needed. + let stored_fingerprint = existing_state.drive_scope_fingerprint.as_deref(); + let scope_changed = stored_fingerprint != Some(¤t_fingerprint); + if scope_changed { + info!( + "Drive scope transition detected: {:?} -> {} — forcing full sync", + stored_fingerprint, current_fingerprint + ); + } + let (drive_cutoff_date, _gmail_cutoff_date) = self.get_cutoff_date()?; info!("Using Drive cutoff date: {}", drive_cutoff_date); @@ -1932,12 +2364,14 @@ impl SyncManager { filtered }; - let is_incremental = matches!(sync_type, SyncType::Incremental); + // Determine effective incremental mode — scope transitions force full sync. + let is_incremental = + !drive_scope_requires_full_sync(sync_type, stored_fingerprint, ¤t_fingerprint); let gmail_history_ids = existing_state.gmail_history_ids.clone(); let chat_checkpoint = existing_state.chat.clone(); let old_page_tokens = existing_state.drive_page_tokens.unwrap_or_default(); - let can_resume_full = sync_type == SyncType::Full && ctx.is_resume(); + let can_resume_full = sync_type == SyncType::Full && ctx.is_resume() && !scope_changed; let mut new_page_tokens: HashMap = if can_resume_full { old_page_tokens.clone() } else { @@ -1955,33 +2389,17 @@ impl SyncManager { let mut successful_users = 0; let mut errors = 0; let mut last_error: Option = None; - // Parse folder-path filters from source config. - // Malformed config is treated as a sync error (fail closed). - let folder_filter_ids: Option>> = - match crate::models::parse_folder_path_filters(&source.config) { - Ok(Some(filters)) => { - let ids: HashSet = filters.into_iter().map(|f| f.id).collect(); - Some(Arc::new(ids)) - } - Ok(None) => None, - Err(e) => { - // Fail the sync — malformed config must not silently index everything. - return Err(anyhow::anyhow!( - "Invalid folder path filter configuration: {}", - e - )); - } - }; + let folder_filter_ids: Option>> = if service_auth.is_oauth() { + None + } else { + parsed_filters + .as_ref() + .map(|filters| Arc::new(filters.iter().map(|filter| filter.id.clone()).collect())) + }; let content_cache = Arc::new(DriveContentCache::default()); - let ancestry_cache: Option> = folder_filter_ids - .as_ref() - .map(|_| Arc::new(FolderAncestryCache::new())); let parallel_users = google_drive_parallel_users(); info!("Processing Drive users with concurrency {}", parallel_users); - let folder_filter_ids_clone = folder_filter_ids.clone(); - let ancestry_cache_clone = ancestry_cache.clone(); - let user_tasks = stream::iter(user_emails.iter().cloned()).map(|cur_user_email| { let service_auth = service_auth.clone(); let source_id = source.id.clone(); @@ -1990,8 +2408,7 @@ impl SyncManager { let ctx = ctx.clone(); let content_cache = content_cache.clone(); let stored_page_token = old_page_tokens.get(cur_user_email.as_str()).cloned(); - let folder_filter_ids = folder_filter_ids_clone.clone(); - let folder_ancestry_cache = ancestry_cache_clone.clone(); + let folder_filter_ids = folder_filter_ids.clone(); async move { if can_resume_full && stored_page_token.is_some() { @@ -2043,7 +2460,6 @@ impl SyncManager { start_token, content_cache.clone(), folder_filter_ids.clone(), - folder_ancestry_cache.clone(), ) .await { @@ -2072,7 +2488,6 @@ impl SyncManager { Some(&drive_cutoff_date), content_cache.clone(), folder_filter_ids.clone(), - folder_ancestry_cache.clone(), ) .await }; @@ -2123,6 +2538,8 @@ impl SyncManager { } else { Some(new_page_tokens.clone()) }, + drive_change_tokens: None, + drive_scope_fingerprint: Some(current_fingerprint.clone()), chat: chat_checkpoint.clone(), }; ctx.save_checkpoint(serde_json::to_value(&checkpoint_state)?) @@ -2178,6 +2595,8 @@ impl SyncManager { } else { Some(new_page_tokens) }, + drive_change_tokens: None, + drive_scope_fingerprint: Some(current_fingerprint), chat: chat_checkpoint, }) } @@ -2382,6 +2801,8 @@ impl SyncManager { Some(new_history_ids.clone()) }, drive_page_tokens: drive_page_tokens.clone(), + drive_change_tokens: existing_state.drive_change_tokens.clone(), + drive_scope_fingerprint: existing_state.drive_scope_fingerprint.clone(), chat: chat_checkpoint.clone(), }; ctx.save_checkpoint(serde_json::to_value(&checkpoint_state)?) @@ -2430,6 +2851,8 @@ impl SyncManager { Some(new_history_ids) }, drive_page_tokens, + drive_change_tokens: existing_state.drive_change_tokens.clone(), + drive_scope_fingerprint: existing_state.drive_scope_fingerprint.clone(), chat: chat_checkpoint, }) } @@ -2585,6 +3008,8 @@ impl SyncManager { let checkpoint_state = GoogleSyncCheckpoint { gmail_history_ids: existing_state.gmail_history_ids.clone(), drive_page_tokens: existing_state.drive_page_tokens.clone(), + drive_change_tokens: existing_state.drive_change_tokens.clone(), + drive_scope_fingerprint: existing_state.drive_scope_fingerprint.clone(), chat: Some(chat_checkpoint.clone()), }; ctx.save_checkpoint(serde_json::to_value(&checkpoint_state)?) @@ -2594,6 +3019,8 @@ impl SyncManager { Ok(GoogleSyncCheckpoint { gmail_history_ids: existing_state.gmail_history_ids, drive_page_tokens: existing_state.drive_page_tokens, + drive_change_tokens: existing_state.drive_change_tokens, + drive_scope_fingerprint: existing_state.drive_scope_fingerprint, chat: Some(chat_checkpoint), }) } @@ -4835,377 +5262,137 @@ mod tests { } // ======================================================================== - // Folder path filter logic tests (production helper check_ancestry_static is at module level above) + // Folder path filter logic // ======================================================================== #[tokio::test] - async fn test_ancestry_empty_allowed_set_returns_true() { - let allowed: HashSet = HashSet::new(); - let parents = Some(vec!["p1".to_string()]); - let lookup = - |id: String| -> Pin)>> + Send>> { - Box::pin(async move { Ok(("name".to_string(), Some(format!("parent-of-{}", id)))) }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(result, "empty allowed set → include all"); - } - - #[tokio::test] - async fn test_ancestry_direct_parent_match() { - let allowed: HashSet = ["p1".to_string()].into_iter().collect(); - let lookup = |_id: String| -> Pin)>> + Send>> { - Box::pin(async move { Ok(("name".to_string(), None)) }) - }; - let result = check_ancestry_static(&Some(vec!["p1".to_string()]), &allowed, &lookup) - .await - .unwrap(); - assert!(result, "direct parent match → true"); - } - - #[tokio::test] - async fn test_ancestry_no_match_no_parents() { - let allowed: HashSet = ["p999".to_string()].into_iter().collect(); - let lookup = |_id: String| -> Pin)>> + Send>> { - Box::pin(async move { Ok(("name".to_string(), None)) }) - }; - let result = check_ancestry_static(&Some(Vec::new()), &allowed, &lookup) - .await - .unwrap(); - assert!(!result, "empty parents + no match → false"); - } - - #[tokio::test] - async fn test_ancestry_nested_match_via_grandparent() { - let allowed: HashSet = ["root-allowed".to_string()].into_iter().collect(); - let parents = Some(vec!["child".to_string()]); - let lookup = - |id: String| -> Pin)>> + Send>> { - Box::pin(async move { - match id.as_str() { - "child" => { - Ok(("Child Folder".to_string(), Some("root-allowed".to_string()))) - } - _ => Ok(("unknown".to_string(), None)), - } - }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(result, "nested grandparent match → true"); - } - - #[tokio::test] - async fn test_ancestry_outside_scope() { - let allowed: HashSet = ["allowed-dir".to_string()].into_iter().collect(); - let parents = Some(vec!["other-dir".to_string()]); - let lookup = - |id: String| -> Pin)>> + Send>> { - Box::pin(async move { - match id.as_str() { - "other-dir" => Ok(("Other".to_string(), Some("grandparent".to_string()))), - "grandparent" => Ok(("GP".to_string(), None)), - _ => Ok(("unknown".to_string(), None)), - } - }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(!result, "ancestry outside scope → false"); - } - - #[tokio::test] - async fn test_ancestry_propagates_lookup_error() { + async fn folder_filter_matches_direct_and_nested_ancestors() { let allowed: HashSet = ["allowed".to_string()].into_iter().collect(); - let parents = Some(vec!["broken".to_string()]); - let lookup = - |id: String| -> Pin)>> + Send>> { - Box::pin(async move { Err(anyhow::anyhow!("API error for {}", id)) }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup).await; - assert!(result.is_err(), "lookup error should propagate"); - } - - #[tokio::test] - async fn test_ancestry_cycle_does_not_loop() { - let allowed: HashSet = ["real-allowed".to_string()].into_iter().collect(); - let parents = Some(vec!["p1".to_string()]); let lookup = |id: String| -> Pin)>> + Send>> { Box::pin(async move { - match id.as_str() { - "p1" => Ok(("Folder 1".to_string(), Some("p2".to_string()))), - "p2" => Ok(("Folder 2".to_string(), Some("p1".to_string()))), - _ => Ok(("unknown".to_string(), None)), - } + let parent = match id.as_str() { + "child" => Some("allowed".to_string()), + "outside" => Some("root".to_string()), + "cycle-a" => Some("cycle-b".to_string()), + "cycle-b" => Some("cycle-a".to_string()), + _ => None, + }; + Ok((id, parent)) }) }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(!result, "cycle without allowed ID → false"); - } - #[tokio::test] - async fn test_ancestry_max_iterations_safety_net() { - let allowed: HashSet = ["target-never-found".to_string()].into_iter().collect(); - let parents = Some(vec!["level0".to_string()]); - let counter = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); - let counter_clone = counter.clone(); - let lookup = move |_id: String| -> Pin< - Box)>> + Send>, - > { - let counter = counter_clone.clone(); - Box::pin(async move { - let next = counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); - Ok(("x".to_string(), Some(format!("level{}", next + 1)))) - }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(!result, "safety cap reached → false"); - let count = counter.load(std::sync::atomic::Ordering::SeqCst); assert!( - count >= 10_000, - "should have iterated at least 10k times, got {}", - count + check_ancestry_static(&Some(vec!["allowed".to_string()]), &allowed, &lookup) + .await + .unwrap() ); - } - - #[tokio::test] - async fn test_ancestry_multiple_parents_any_match() { - let allowed: HashSet = ["allowed-p1".to_string()].into_iter().collect(); - let parents = Some(vec!["other-p".to_string(), "allowed-p1".to_string()]); - let lookup = |_id: String| -> Pin)>> + Send>> { - Box::pin(async move { Ok(("name".to_string(), None)) }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(result, "any parent match → true"); - } - - #[test] - fn test_folder_filter_ids_generated_from_parse() { - // Parse a config with two filters and verify the ID set. - let config = serde_json::json!({ - "folder_path_filters": [ - { - "id": "0ADRI000001", - "name": "Engineering", - "path": "/Engineering (Shared Drive)", - "driveId": "0ADRI000001", - "kind": "shared_drive_root" - }, - { - "id": "1FOLDER0001", - "name": "Docs", - "path": "/Engineering/Docs", - "driveId": "0ADRI000001", - "kind": "folder" - } - ] - }); - - let result = crate::models::parse_folder_path_filters(&config); - assert!(result.is_ok()); - let filters = result.unwrap().unwrap(); - assert_eq!(filters.len(), 2); - - let ids: HashSet = filters.into_iter().map(|f| f.id).collect(); - assert!(ids.contains("0ADRI000001")); - assert!(ids.contains("1FOLDER0001")); - assert_eq!(ids.len(), 2); - } - - #[test] - fn test_config_parse_to_folder_filter_ids_chain() { - // End-to-end production-like path: parse config → extract IDs → HashSet. - let config = serde_json::json!({ - "folder_path_filters": [{ - "id": "0AMYDRIVE", - "name": "Company", - "path": "/Company (Shared Drive)", - "driveId": "0AMYDRIVE", - "kind": "shared_drive_root" - }] - }); - let result = crate::models::parse_folder_path_filters(&config); - assert!(result.is_ok()); - let ids_opt = result - .unwrap() - .map(|f| f.into_iter().map(|e| e.id).collect::>()); - assert!(ids_opt.is_some()); - let ids = ids_opt.unwrap(); - assert!(ids.contains("0AMYDRIVE")); - assert_eq!(ids.len(), 1); - - // If we got None (no filters), that's also index-all. - let nonexistent = crate::models::parse_folder_path_filters(&serde_json::json!({})); - assert!(nonexistent.is_ok_and(|v| v.is_none())); - } - - // ======================================================================== - // FolderAncestryCache tests - // ======================================================================== - - #[tokio::test] - async fn test_folder_ancestry_cache_miss_then_hit() { - // Verify that the cache populates on first call and serves a second - // call without re-invoking the fetcher (cache hit). - let cache = Arc::new(FolderAncestryCache::new()); - let fetch_count = Arc::new(AtomicU64::new(0)); - - // First call — should invoke fetcher once. - { - let fetch_count = fetch_count.clone(); - let result = cache - .get_or_fetch_test("folder-1", move || { - let fetch_count = fetch_count.clone(); - async move { - fetch_count.fetch_add(1, Ordering::Relaxed); - Ok(("Folder 1".to_string(), Some("parent-1".to_string()))) - } - }) + assert!( + check_ancestry_static(&Some(vec!["child".to_string()]), &allowed, &lookup) .await - .unwrap(); - assert_eq!(result.0, "Folder 1"); - assert_eq!(result.1, Some("parent-1".to_string())); - } - assert_eq!( - fetch_count.load(Ordering::Relaxed), - 1, - "first call fetched once" + .unwrap() ); - - // Second call — same folder_id, should hit cache without fetching. - { - let result = cache - .get_or_fetch_test("folder-1", || async move { - panic!("fetcher should not be called on cache hit"); - }) + assert!( + !check_ancestry_static(&Some(vec!["outside".to_string()]), &allowed, &lookup) .await - .unwrap(); - assert_eq!(result.0, "Folder 1"); - } - - // fetch_count still 1 (fetcher not called again). - assert_eq!(fetch_count.load(Ordering::Relaxed), 1); + .unwrap() + ); assert!( - cache.hits.load(Ordering::Relaxed) > 0, - "cache should have recorded at least one hit" + !check_ancestry_static(&Some(vec!["cycle-a".to_string()]), &allowed, &lookup) + .await + .unwrap() ); } #[tokio::test] - async fn test_folder_ancestry_cache_no_cache_when_no_filters() { - // When no filters are configured, the is_file_in_allowed_folder path - // short-circuits at the top-level empty-check without creating a cache. - // Verify that short-circuit with empty allowed set. - let allowed: HashSet = HashSet::new(); - let parents = Some(vec!["child".to_string()]); - let lookup = |_id: String| -> Pin< - Box)>> + Send>, - > { - Box::pin(async move { Ok(("ignored".to_string(), None)) }) - }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(result, "empty allowed set → include all"); - } - - #[tokio::test] - async fn test_check_ancestry_static_cycle_detection() { - // A cycle in the parent chain should not cause infinite loop. - let allowed: HashSet = ["root-folder".to_string()].into_iter().collect(); - let parents = Some(vec!["a".to_string()]); + async fn folder_filter_propagates_lookup_errors() { + let allowed: HashSet = ["allowed".to_string()].into_iter().collect(); let lookup = |id: String| -> Pin)>> + Send>> { - Box::pin(async move { - match id.as_str() { - "a" => Ok(("A".to_string(), Some("b".to_string()))), - "b" => Ok(("B".to_string(), Some("a".to_string()))), // cycle back to a - _ => Ok(("unknown".to_string(), None)), - } - }) + Box::pin(async move { Err(anyhow!("API error for {id}")) }) }; - let result = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(!result, "cycle should not match allowed folder"); + + assert!( + check_ancestry_static(&Some(vec!["broken".to_string()]), &allowed, &lookup) + .await + .is_err() + ); } - #[tokio::test] - async fn test_check_ancestry_static_ancestry_cache_together() { - // FolderAncestryCache used with check_ancestry_static to verify both - // the ancestry walking and caching integration. - let cache = Arc::new(FolderAncestryCache::new()); - let fetch_count = Arc::new(AtomicU64::new(0)); - - let lookup = { - let cache = cache.clone(); - let fetch_count = fetch_count.clone(); - move |folder_id: String| -> Pin< - Box)>> + Send>, - > { - let cache = cache.clone(); - let fetch_count = fetch_count.clone(); - Box::pin(async move { - let fid = folder_id.clone(); - cache - .get_or_fetch_test(&folder_id, { - let fid = fid.clone(); - move || { - let fid = fid.clone(); - let fetch_count = fetch_count.clone(); - async move { - fetch_count.fetch_add(1, Ordering::Relaxed); - match fid.as_str() { - "child" => Ok(("Child Folder".to_string(), Some("root-allowed".to_string()))), - "root-allowed" => Ok(("Allowed Root".to_string(), None)), - _ => Ok(("unknown".to_string(), None)), - } - } - } - }) - .await - }) + #[test] + fn drive_scope_changes_force_full_sync() { + assert!(drive_scope_requires_full_sync( + SyncType::Full, + Some("all"), + "all" + )); + assert!(drive_scope_requires_full_sync( + SyncType::Incremental, + None, + "all" + )); + assert!(drive_scope_requires_full_sync( + SyncType::Incremental, + Some("all"), + "drive:folder:id", + )); + assert!(!drive_scope_requires_full_sync( + SyncType::Incremental, + Some("drive:folder:id"), + "drive:folder:id", + )); + } + + #[test] + fn folder_scope_cutoff_uses_parsed_created_or_modified_times() { + let cutoff = parse_google_time(Some("2025-01-01T00:00:00Z")).unwrap(); + let file = |mime_type: &str, created_time: &str, modified_time: &str| { + crate::models::GoogleDriveFile { + id: "file".to_string(), + name: "File".to_string(), + mime_type: mime_type.to_string(), + web_view_link: None, + created_time: Some(created_time.to_string()), + modified_time: Some(modified_time.to_string()), + size: None, + parents: None, + shared: None, + permissions: None, + owners: None, } }; - let allowed: HashSet = ["root-allowed".to_string()].into_iter().collect(); - let parents = Some(vec!["child".to_string()]); - - // First call — should fetch child only. The root-allowed is in allowed_ids, - // so check_ancestry_static returns true without fetching its metadata. - let r1 = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(r1, "child should resolve to root-allowed"); - assert_eq!( - fetch_count.load(Ordering::Relaxed), - 1, - "one fetch (child only; root-allowed is in allowed set directly)" - ); - - // Second call — should NOT re-fetch because cache already populated. - let r2 = check_ancestry_static(&parents, &allowed, &lookup) - .await - .unwrap(); - assert!(r2, "second call same result"); - assert_eq!( - fetch_count.load(Ordering::Relaxed), - 1, - "no additional fetches on second call (cache hit)" - ); - assert!( - cache.hits.load(Ordering::Relaxed) >= 1, - "cache recorded at least one hit for reused entry" - ); + assert!(SyncManager::pass_cutoff( + &file( + "application/pdf", + "2024-01-01T00:00:00Z", + "2025-01-02T00:00:00Z" + ), + cutoff, + )); + assert!(SyncManager::pass_cutoff( + &file( + "application/pdf", + "2025-01-02T01:00:00+01:00", + "2024-01-01T00:00:00Z" + ), + cutoff, + )); + assert!(!SyncManager::pass_cutoff( + &file( + "application/pdf", + "2024-01-01T00:00:00Z", + "2024-12-31T23:59:59Z" + ), + cutoff, + )); + assert!(SyncManager::pass_cutoff( + &file( + "application/vnd.google-apps.folder", + "2020-01-01T00:00:00Z", + "2020-01-01T00:00:00Z", + ), + cutoff, + )); } } diff --git a/services/connector-manager/src/handlers.rs b/services/connector-manager/src/handlers.rs index 6df0f9444..754e76ff4 100644 --- a/services/connector-manager/src/handlers.rs +++ b/services/connector-manager/src/handlers.rs @@ -358,340 +358,294 @@ pub async fn execute_action( _headers: HeaderMap, Json(request): Json, ) -> Result { - info!( - "Executing action '{}' for source {} (user {:?}, params keys: {:?})", - request.action, - request.source_id, - request.user_id, - request - .params - .as_object() - .map(|m| m.keys().cloned().collect::>()) - .unwrap_or_default() - ); - - let source_repo = SourceRepository::new(state.db_pool.pool()); - let source = source_repo - .find_by_id(request.source_id.clone()) - .await - .map_err(|e| ApiError::Internal(e.to_string()))? - .ok_or_else(|| ApiError::NotFound(format!("Source not found: {}", request.source_id)))?; - - // Look up the connector manifest to get connector_url and action metadata + let is_transient = request.transient_credentials.is_some(); + let source_type: SourceType; + let source: Option; + let creds: shared::models::ServiceCredential; + let mut params = request.params.clone(); + let mut transient_actor_email = None; let manifests = get_registered_manifests(&state.redis_client).await; - let manifest = manifests - .iter() - .find(|m| m.source_types.contains(&source.source_type)); - let connector_url = manifest - .as_ref() - .map(|m| m.connector_url.clone()) - .ok_or_else(|| { - ApiError::NotFound(format!( - "Connector not registered for type: {:?}", - source.source_type - )) + let (connector_url, action_admin_only) = if is_transient { + // ======== TRANSIENT MODE ======== + // Transient mode: source_type + transient_credentials required, no source/credential DB. + let tc = request.transient_credentials.as_ref().unwrap(); + source_type = request.source_type.ok_or_else(|| { + ApiError::BadRequest( + "source_type is required when transient_credentials are provided".to_string(), + ) })?; + if request.source_id.is_some() { + return Err(ApiError::BadRequest( + "source_id must not be set when transient_credentials are provided".to_string(), + )); + } - let action_def = manifest.and_then(|m| m.actions.iter().find(|a| a.name == request.action)); - let action_mode = action_def.map(|a| a.mode).unwrap_or_default(); - // Reject unknown action names — they must exist in the manifest - let action_def = action_def.ok_or_else(|| { - ApiError::BadRequest(format!( - "Unknown action '{}' for source type {:?}", - request.action, source.source_type - )) - })?; - let action_admin_only = action_def.admin_only; - - // Generic read_only enforcement — the source config is the authority. - let source_read_only = source - .config - .get("read_only") - .and_then(|v| v.as_bool()) - .unwrap_or(false); - if let Some(m) = manifest.as_ref() { - if (m.read_only || source_read_only) && action_mode == ActionMode::Write { + // Resolve user/admin from user_id (required in transient mode). + let user_id = request.user_id.as_ref().ok_or_else(|| { + ApiError::BadRequest("user_id is required in transient mode".to_string()) + })?; + + // Look up the connector manifest by source_type. + let manifest = manifests + .iter() + .find(|m| m.source_types.contains(&source_type)) + .ok_or_else(|| { + ApiError::NotFound(format!( + "Connector not registered for type: {:?}", + source_type + )) + })?; + + let connector_url = manifest.connector_url.clone(); + let action_def = manifest + .actions + .iter() + .find(|a| a.name == request.action) + .ok_or_else(|| { + ApiError::BadRequest(format!( + "Unknown action '{}' for source type {:?}", + request.action, source_type + )) + })?; + if !action_def.source_types.contains(&source_type) { return Err(ApiError::BadRequest(format!( - "Action '{}' is not allowed: source is read-only", - request.action + "Action '{}' does not support source type {:?}", + request.action, source_type ))); } - } + let action_admin_only = action_def.admin_only; + let action_mode = action_def.mode; - let creds_repo = ServiceCredentialsRepo::new(state.db_pool.pool().clone()) - .map_err(|e| ApiError::Internal(e.to_string()))?; - let creds = match resolve_credentials( - &creds_repo, - &request.source_id, - request.user_id.as_deref(), - action_admin_only, - ) - .await? - { - CredentialResolution::Resolved(c) => c, - CredentialResolution::NeedsUserAuth { provider } => { - return Ok(needs_user_auth_response( - &request.source_id, - source.source_type, - provider, - )?); - } - CredentialResolution::NoCredentials => { - return Err(ApiError::NotFound(format!( - "Credentials not found for source: {}", - request.source_id + if action_mode != ActionMode::Read { + return Err(ApiError::BadRequest(format!( + "Transient action '{}' must be read-only", + request.action ))); } - }; - - // Resolve Omni document ID -> source external_id. - // TODO: replace hard-coded param names with a connector-declared resolve_params list. - let mut params = request.params.clone(); - let doc_id = params - .get("document_id") - .or_else(|| params.get("file_id")) - .and_then(|v| v.as_str()) - .map(|s| s.to_string()); - if let Some(doc_id) = doc_id { - let doc_repo = DocumentRepository::new(state.db_pool.pool()); - if let Ok(Some(doc)) = doc_repo.find_by_id(&doc_id).await { - info!( - "Resolved document/file ID {} -> external_id {}", - doc_id, doc.external_id - ); - if let Some(obj) = params.as_object_mut() { - obj.remove("document_id"); - obj.remove("file_id"); - obj.insert( - "file_id".to_string(), - serde_json::Value::String(doc.external_id), - ); - } - } - // If not found, assume the ID is already a source-native ID and pass through - } - - // Merge source config into params for legacy connector compatibility. - // Connectors that use the typed context (e.g. Darwinbox) ignore these. - if params.is_null() { - params = serde_json::Value::Object(serde_json::Map::new()); - } - if let (Some(src_obj), Some(params_obj)) = (source.config.as_object(), params.as_object_mut()) { - for (k, v) in src_obj { - params_obj.entry(k.clone()).or_insert_with(|| v.clone()); - } - } - // Resolve actor email for the execution context - let actor_email = if let Some(uid) = request.user_id.as_ref() { let user_repo = UserRepository::new(state.db_pool.pool()); let user = user_repo - .find_by_id(uid.clone()) + .find_by_id(user_id.clone()) .await .map_err(|e| ApiError::Internal(e.to_string()))? - .ok_or_else(|| ApiError::NotFound(format!("User not found: {uid}")))?; - // Enforce admin_only action authorization + .ok_or_else(|| ApiError::NotFound(format!("User not found: {user_id}")))?; if action_admin_only && user.role != shared::models::UserRole::Admin { return Err(ApiError::BadRequest(format!( "Action '{}' requires admin privileges", request.action ))); } - Some(user.email) - } else { - None - }; - - info!( - "Dispatching action '{}' to connector {} with credential {} (provider={:?}, auth_type={:?}, principal={:?})", - request.action, - connector_url, - creds.id, - creds.provider, - creds.auth_type, - creds.principal_email, - ); + transient_actor_email = Some(user.email); + + // Adapt the transient payload to the connector SDK's credential model. + let tc = tc.clone(); + creds = shared::models::ServiceCredential { + id: "transient".to_string(), + source_id: "transient".to_string(), + user_id: Some(user_id.clone()), + provider: tc.provider, + auth_type: tc.auth_type, + principal_email: tc.principal_email, + credentials: tc.credentials, + config: tc.config, + expires_at: None, + last_validated_at: None, + created_at: time::OffsetDateTime::now_utc(), + updated_at: time::OffsetDateTime::now_utc(), + }; + source = None; - let client = ConnectorClient::new(); - let action_request = ActionRequest { - action: request.action, - params, - credentials: Some(creds), - source: Some(source), - actor_email, - }; + info!( + "Executing transient action '{}' for source_type {:?} (user {:?})", + request.action, source_type, request.user_id + ); - // Proxy the connector's full HTTP response (status, headers, body) verbatim. - let response = client - .execute_action_raw(&connector_url, &action_request) - .await - .map_err(|e| ApiError::Internal(e.to_string()))?; + (connector_url, action_admin_only) + } else { + // ======== PERSISTED MODE (unchanged) ======== + if request.source_type.is_some() { + return Err(ApiError::BadRequest( + "source_type is only valid with transient_credentials".to_string(), + )); + } + let source_id = request + .source_id + .as_deref() + .filter(|id| !id.is_empty()) + .ok_or_else(|| ApiError::BadRequest("source_id is required".to_string()))? + .to_string(); - let status = response.status(); - let mut builder = axum::response::Response::builder().status(status); + let source_repo = SourceRepository::new(state.db_pool.pool()); + let db_source = source_repo + .find_by_id(source_id.clone()) + .await + .map_err(|e| ApiError::Internal(e.to_string()))? + .ok_or_else(|| ApiError::NotFound(format!("Source not found: {source_id}")))?; - // Forward all headers except hop-by-hop connection headers. - let hop_by_hop = [ - "connection", - "keep-alive", - "transfer-encoding", - "te", - "trailer", - "upgrade", - ]; - for (key, value) in response.headers() { - let key_str = key.as_str(); - if !hop_by_hop.contains(&key_str) { - builder = builder.header(key, value); - } - } + source_type = db_source.source_type; - let bytes = response - .bytes() - .await - .map_err(|e| ApiError::Internal(e.to_string()))?; + let manifest = manifests + .iter() + .find(|m| m.source_types.contains(&source_type)) + .ok_or_else(|| { + ApiError::NotFound(format!( + "Connector not registered for type: {:?}", + source_type + )) + })?; + let connector_url = manifest.connector_url.clone(); - Ok(builder.body(axum::body::Body::from(bytes)).unwrap()) -} + let action_def = manifest + .actions + .iter() + .find(|a| a.name == request.action) + .ok_or_else(|| { + ApiError::BadRequest(format!( + "Unknown action '{}' for source type {:?}", + request.action, source_type + )) + })?; + if !action_def.source_types.contains(&source_type) { + return Err(ApiError::BadRequest(format!( + "Action '{}' does not support source type {:?}", + request.action, source_type + ))); + } + let action_admin_only = action_def.admin_only; + let action_mode = action_def.mode; + + // Generic read_only enforcement — the source config is the authority. + let source_read_only = db_source + .config + .get("read_only") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + if (manifest.read_only || source_read_only) && action_mode == ActionMode::Write { + return Err(ApiError::BadRequest(format!( + "Action '{}' is not allowed: source is read-only", + request.action + ))); + } -/// Preview-action endpoint similar to `execute_action` but accepts inline credentials -/// instead of resolving from the database. This is used for admin preview/discovery -/// actions (like listing shared drives) before the admin has saved their credentials. -/// -/// SECURITY: Strictly allowlisted — only action="discover_folders" for -/// google_drive source type with Google JWT credentials is accepted. -/// Request body size limit: 128KB enforced before any parsing. -pub async fn execute_action_preview( - State(state): State, - body_bytes: axum::body::Bytes, -) -> Result { - // Enforce 128KB body limit BEFORE any parsing. - if body_bytes.len() > 128 * 1024 { - return Err(ApiError::BadRequest( - "Preview request body too large (max 128KB)".to_string(), - )); - } + let creds_repo = ServiceCredentialsRepo::new(state.db_pool.pool().clone()) + .map_err(|e| ApiError::Internal(e.to_string()))?; + creds = match resolve_credentials( + &creds_repo, + &source_id, + request.user_id.as_deref(), + action_admin_only, + ) + .await? + { + CredentialResolution::Resolved(c) => c, + CredentialResolution::NeedsUserAuth { provider } => { + return Ok(needs_user_auth_response(&source_id, source_type, provider)?); + } + CredentialResolution::NoCredentials => { + return Err(ApiError::NotFound(format!( + "Credentials not found for source: {source_id}" + ))); + } + }; - let request: crate::models::PreviewActionRequest = serde_json::from_slice(&body_bytes) - .map_err(|e| ApiError::BadRequest(format!("Invalid preview request JSON: {}", e)))?; + // Resolve Omni document ID -> source external_id (persisted mode only). + let doc_id = params + .get("document_id") + .or_else(|| params.get("file_id")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); + if let Some(doc_id) = doc_id { + let doc_repo = DocumentRepository::new(state.db_pool.pool()); + if let Ok(Some(doc)) = doc_repo.find_by_id(&doc_id).await { + info!( + "Resolved document/file ID {} -> external_id {}", + doc_id, doc.external_id + ); + if let Some(obj) = params.as_object_mut() { + obj.remove("document_id"); + obj.remove("file_id"); + obj.insert( + "file_id".to_string(), + serde_json::Value::String(doc.external_id), + ); + } + } + } - info!( - "Preview action '{}' (source_type={:?}, source_id={:?}, params keys: {:?})", - request.action, - request.source_type, - request.source_id, - request - .params - .as_object() - .map(|m| m.keys().cloned().collect::>()) - .unwrap_or_default() - ); + // Merge source config into params for legacy connector compatibility. + if params.is_null() { + params = serde_json::Value::Object(serde_json::Map::new()); + } + if let (Some(src_obj), Some(params_obj)) = + (db_source.config.as_object(), params.as_object_mut()) + { + for (k, v) in src_obj { + params_obj.entry(k.clone()).or_insert_with(|| v.clone()); + } + } - // ======== STRICT ALLOWLIST ======== - // Only allow discover_folders action for google_drive with Google JWT credentials. - if request.action != "discover_folders" { - return Err(ApiError::BadRequest(format!( - "Preview action '{}' is not allowed", - request.action - ))); - } + source = Some(db_source); - // source_type must be google_drive; if not present, reject. - let source_type = request.source_type.as_ref().ok_or_else(|| { - ApiError::BadRequest("source_type is required for preview actions".to_string()) - })?; + info!( + "Executing action '{}' for source {} (user {:?}, params keys: {:?})", + request.action, + source_id, + request.user_id, + request + .params + .as_object() + .map(|m| m.keys().cloned().collect::>()) + .unwrap_or_default() + ); - if *source_type != SourceType::GoogleDrive { - return Err(ApiError::BadRequest(format!( - "Preview action only supports google_drive, got {:?}", - source_type - ))); - } + (connector_url, action_admin_only) + }; - // Validate credential is Google JWT. - let creds = &request.credentials; - if creds.provider != shared::models::ServiceProvider::Google { - return Err(ApiError::BadRequest(format!( - "Preview action requires Google credentials, got provider: {:?}", - creds.provider - ))); - } - if creds.auth_type != shared::models::AuthType::Jwt { - return Err(ApiError::BadRequest(format!( - "Preview action requires JWT credentials, got auth_type: {:?}", - creds.auth_type - ))); - } - if creds.principal_email.as_deref().unwrap_or("").is_empty() { - return Err(ApiError::BadRequest( - "Preview action requires a non-empty principal_email".to_string(), - )); - } + // ==== Common path for both modes ==== + // Resolve the actor email; transient mode already loaded the actor above. + let actor_email = if !is_transient { + if let Some(uid) = request.user_id.as_ref() { + let user_repo = UserRepository::new(state.db_pool.pool()); + let user = user_repo + .find_by_id(uid.clone()) + .await + .map_err(|e| ApiError::Internal(e.to_string()))? + .ok_or_else(|| ApiError::NotFound(format!("User not found: {uid}")))?; + // Enforce admin_only action authorization (user already validated for transient mode above). + if action_admin_only && user.role != shared::models::UserRole::Admin { + return Err(ApiError::BadRequest(format!( + "Action '{}' requires admin privileges", + request.action + ))); + } + Some(user.email) + } else { + None + } + } else { + transient_actor_email + }; - // Validate credential shape: credentials must be a JSON object with service_account_key. - let creds_obj = creds.credentials.as_object().ok_or_else(|| { - ApiError::BadRequest( - "Preview credential 'credentials' field must be a JSON object".to_string(), - ) - })?; - let sa_key = creds_obj - .get("service_account_key") - .and_then(|v| v.as_str()) - .ok_or_else(|| { - ApiError::BadRequest( - "Preview JWT credentials must contain 'service_account_key' as a string" - .to_string(), - ) - })?; - if sa_key.is_empty() { - return Err(ApiError::BadRequest( - "Preview service_account_key must not be empty".to_string(), - )); - } - // Validate that service_account_key is valid JSON (it's a full SA key blob). - serde_json::from_str::(sa_key).map_err(|e| { - ApiError::BadRequest(format!( - "Preview service_account_key is not valid JSON: {}", - e - )) - })?; - - // Validate config contains domain. - let config_obj = creds.config.as_object().ok_or_else(|| { - ApiError::BadRequest("Preview credential 'config' field must be a JSON object".to_string()) - })?; - let domain = config_obj.get("domain").and_then(|v| v.as_str()); - if domain.is_none_or(|d| d.is_empty()) { - return Err(ApiError::BadRequest( - "Preview credential config must contain a non-empty 'domain'".to_string(), - )); + if params.is_null() { + params = serde_json::Value::Object(serde_json::Map::new()); } - let manifests = get_registered_manifests(&state.redis_client).await; - - // Source type is the only lookup key — must match exactly. - let connector_url = manifests - .iter() - .find(|m| m.source_types.contains(source_type)) - .map(|m| m.connector_url.clone()) - .ok_or_else(|| { - ApiError::NotFound(format!( - "Connector not registered for type: {:?}", - source_type - )) - })?; + info!( + "Dispatching action '{}' to connector {} (provider={:?}, auth_type={:?}, principal={:?})", + request.action, connector_url, creds.provider, creds.auth_type, creds.principal_email, + ); - // Use the inline credential directly — no DB resolution. let client = ConnectorClient::new(); let action_request = ActionRequest { action: request.action, - params: request.params, - credentials: Some(creds.clone()), - source: None, - actor_email: None, + params, + credentials: Some(creds), + source, + actor_email, }; + // Proxy the connector's full HTTP response (status, headers, body) verbatim. let response = client .execute_action_raw(&connector_url, &action_request) .await diff --git a/services/connector-manager/src/lib.rs b/services/connector-manager/src/lib.rs index e638c5a3c..ccb0d9f18 100644 --- a/services/connector-manager/src/lib.rs +++ b/services/connector-manager/src/lib.rs @@ -51,14 +51,6 @@ pub fn create_app(state: AppState) -> Router { .route("/sources/:source_id", get(handlers::get_source)) .route("/connectors", get(handlers::list_connectors)) .route("/action", post(handlers::execute_action)) - // /action-preview has a per-route 128KB body limit, applied BEFORE extraction. - // Merge a nested sub-router so the limit layer binds to this route alone, - // independent of the global DefaultBodyLimit::disable() below. - .merge( - Router::new() - .route("/action-preview", post(handlers::execute_action_preview)) - .layer(DefaultBodyLimit::max(128 * 1024)), - ) .route("/actions", get(handlers::list_actions)) .route("/resource", post(handlers::read_resource)) .route("/resources", get(handlers::list_resources)) diff --git a/services/connector-manager/src/models.rs b/services/connector-manager/src/models.rs index e9f5483c0..e11098244 100644 --- a/services/connector-manager/src/models.rs +++ b/services/connector-manager/src/models.rs @@ -91,9 +91,26 @@ pub struct TriggerSyncResponse { pub status: String, } +/// Transient credentials accepted during setup/discovery before a source or +/// credential row exists in the database. Used by the setup UI to preview +/// connector capabilities (e.g. list shared drives) with an inline credential +/// that has not yet been persisted. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct TransientCredentials { + pub provider: shared::models::ServiceProvider, + pub auth_type: shared::models::AuthType, + pub principal_email: Option, + pub credentials: JsonValue, + #[serde(default)] + pub config: JsonValue, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ExecuteActionRequest { - pub source_id: String, + /// Source ID, required when persisted credentials should be resolved. + #[serde(default)] + pub source_id: Option, /// Acting user. `None` for org-level / system-initiated calls (sync, /// future org-level agents); `Some` for chat tool dispatch and other /// user-context invocations. @@ -102,6 +119,14 @@ pub struct ExecuteActionRequest { pub action: String, #[serde(default)] pub params: JsonValue, + /// Source type hint, required when `transient_credentials` are provided. + #[serde(default)] + pub source_type: Option, + /// Inline credentials for setup/preview actions. When present, the request + /// is handled in **transient mode**: `source_id` and DB credentials are + /// not used, and `source_type` must identify the connector. + #[serde(default)] + pub transient_credentials: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -275,25 +300,3 @@ pub struct ExecuteSkillRequest { #[serde(default)] pub arguments: Option, } - -// ============================================================================ -// Preview Action (transient credentials, no DB dependency) -// ============================================================================ - -/// Like `ExecuteActionRequest` but accepts inline credentials and a source_type -/// hint so the action can be dispatched without a persisted source or credential row. -/// -/// SECURITY: `deny_unknown_fields` ensures no extra fields are smuggled through. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct PreviewActionRequest { - /// Source type to select the connector (used when source_id is not available). - pub source_type: Option, - /// Existing source id (optional, used for source config merging). - pub source_id: Option, - pub action: String, - #[serde(default)] - pub params: JsonValue, - /// Inline credentials (not persisted). Must have provider/google fields populated. - pub credentials: shared::models::ServiceCredential, -} diff --git a/web/src/lib/components/google-drive-folder-selector.svelte b/web/src/lib/components/google-drive-folder-selector.svelte index af7658985..c17d33d5c 100644 --- a/web/src/lib/components/google-drive-folder-selector.svelte +++ b/web/src/lib/components/google-drive-folder-selector.svelte @@ -68,7 +68,7 @@ ) // Track the previous context token to detect changes. - let prevContextToken = $state(contextToken) + let prevContextToken = $state('') // Reactive effect: detect credential-context change and re-discover. $effect(() => { diff --git a/web/src/routes/api/preview-action/+server.ts b/web/src/routes/api/preview-action/+server.ts index 5657c8a8d..f8a46ec1d 100644 --- a/web/src/routes/api/preview-action/+server.ts +++ b/web/src/routes/api/preview-action/+server.ts @@ -8,24 +8,13 @@ import { SourceType } from '$lib/types' * POST /api/preview-action * * Admin-only endpoint that forwards a transient credential (not yet persisted) to - * the connector for preview/discovery actions such as listing shared drives and - * top-level folders. The credential is never stored or returned; it flows straight - * to the connector-manager's /action-preview endpoint and then to the connector. - * - * Request body size is limited to 64KB at the web boundary. + * the connector-manager's normal /action endpoint in **transient mode**. + * The credential is never stored or returned. * * SECURITY: Strictly allowlisted — only action='discover_folders', * sourceType='google_drive', and JWT credentials are accepted. * - * Request body: - * { - * sourceType: 'google_drive', - * action: 'discover_folders', - * params: {}, - * serviceAccountJson: string, // full service-account JSON key - * principalEmail: string, // delegated admin email - * domain: string // Google Workspace domain - * } + * Request body size is limited to 64KB at the web boundary. */ export const POST: RequestHandler = async ({ request, locals }) => { // Admin-only @@ -155,37 +144,27 @@ export const POST: RequestHandler = async ({ request, locals }) => { const config = getConfig() const connectorManagerUrl = config.services.connectorManagerUrl - // Build a transient ServiceCredential payload for the connector-manager - const transientCredential = { - id: 'preview', - source_id: 'preview', - user_id: null as string | null, - provider: 'google', - auth_type: 'jwt', - principal_email: principalEmail, - credentials: { - service_account_key: serviceAccountJson, - }, - config: { - domain: domain, - }, - expires_at: null, - last_validated_at: null, - created_at: new Date().toISOString(), - updated_at: new Date().toISOString(), - } - - const response = await fetch(`${connectorManagerUrl}/action-preview`, { + // Forward the setup credential through connector-manager's generic transient action mode. + const response = await fetch(`${connectorManagerUrl}/action`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ source_type: 'google_drive', - source_id: null, + user_id: locals.user.id, action: 'discover_folders', params: params || {}, - credentials: transientCredential, + transient_credentials: { + provider: 'google', + auth_type: 'jwt', + principal_email: principalEmail, + credentials: { + service_account_key: serviceAccountJson, + }, + config: { + domain: domain, + }, + }, }), - // Limit connector-manager communication too signal: AbortSignal.timeout(30_000), }) diff --git a/web/src/routes/api/preview-action/preview-action.test.ts b/web/src/routes/api/preview-action/preview-action.test.ts index 4759fcf76..6e010d59c 100644 --- a/web/src/routes/api/preview-action/preview-action.test.ts +++ b/web/src/routes/api/preview-action/preview-action.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' // Mock server dependencies before importing the handler. vi.mock('$lib/server/config', () => ({ @@ -59,92 +59,65 @@ async function callPost( } describe('POST /api/preview-action', () => { - beforeEach(() => { - vi.restoreAllMocks() + afterEach(() => { + vi.unstubAllGlobals() }) - it('rejects non-admin users with 403', async () => { - const response = await callPost( - { - sourceType: 'google_drive', - action: 'discover_folders', - serviceAccountJson: '{}', - principalEmail: 'a@b.com', - domain: 'b.com', - }, - { user: null }, - ) - expect(response.status).toBe(401) - }) - - it('rejects non-admin role with 403', async () => { - const response = await callPost( - { - sourceType: 'google_drive', - action: 'discover_folders', - serviceAccountJson: '{}', - principalEmail: 'a@b.com', - domain: 'b.com', - }, - { user: { id: 'u1', role: 'member' } }, - ) - expect(response.status).toBe(403) - }) - - it('rejects unknown top-level fields', async () => { - const response = await callPost({ + it('requires an admin', async () => { + const request = { sourceType: 'google_drive', action: 'discover_folders', - unknownField: 'x', serviceAccountJson: '{}', principalEmail: 'a@b.com', domain: 'b.com', - }) - expect(response.status).toBe(400) - const body = response.body as { message?: string } | undefined - expect(body?.message || '').toContain('Unknown field') - }) + } - it('rejects unsupported action', async () => { - const response = await callPost({ - sourceType: 'google_drive', - action: 'delete_all_files', - serviceAccountJson: '{}', - principalEmail: 'a@b.com', - domain: 'b.com', - }) - expect(response.status).toBe(400) + expect((await callPost(request, { user: null })).status).toBe(401) + expect((await callPost(request, { user: { id: 'u1', role: 'member' } })).status).toBe(403) }) - it('rejects non-google_drive sourceType', async () => { - const response = await callPost({ - sourceType: 'gmail', - action: 'discover_folders', - serviceAccountJson: '{}', - principalEmail: 'a@b.com', - domain: 'b.com', - }) - expect(response.status).toBe(400) - }) + it('forwards validated setup credentials through the normal action endpoint', async () => { + const connectorResponse = { + status: 'success', + result: { items: [] }, + } + const connectorFetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify(connectorResponse), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ) + vi.stubGlobal('fetch', connectorFetch) - it('rejects missing required credential fields', async () => { const response = await callPost({ sourceType: 'google_drive', action: 'discover_folders', - principalEmail: 'a@b.com', - domain: 'b.com', + serviceAccountJson: '{"client_email":"service@example.com"}', + principalEmail: 'admin@example.com', + domain: 'example.com', + params: {}, }) - expect(response.status).toBe(400) - }) - it('rejects invalid service account JSON', async () => { - const response = await callPost({ - sourceType: 'google_drive', + expect(response).toEqual({ status: 200, body: connectorResponse }) + expect(connectorFetch).toHaveBeenCalledOnce() + const [url, init] = connectorFetch.mock.calls[0] as [string, RequestInit] + expect(url).toBe('http://cm.test/action') + const forwarded = JSON.parse(String(init.body)) as Record + expect(forwarded).not.toHaveProperty('source_id') + expect(forwarded).toMatchObject({ + source_type: 'google_drive', + user_id: 'admin-1', action: 'discover_folders', - serviceAccountJson: '{invalid}', - principalEmail: 'a@b.com', - domain: 'b.com', + params: {}, + transient_credentials: { + provider: 'google', + auth_type: 'jwt', + principal_email: 'admin@example.com', + credentials: { + service_account_key: '{"client_email":"service@example.com"}', + }, + config: { domain: 'example.com' }, + }, }) - expect(response.status).toBe(400) }) }) From 7f241f819f037b822eb7d815cf332ff2361666ba Mon Sep 17 00:00:00 2001 From: Praveen Sampath Date: Wed, 29 Jul 2026 04:27:25 +0000 Subject: [PATCH 3/4] refactor(web): route transient actions through connectors API - Move pre-source actions under /api/connectors/[sourceType]/action. - Derive source type from the route while retaining strict action and credential validation. - Update Google Drive folder discovery and focused endpoint tests. --- services/connector-manager/src/models.rs | 4 +- .../google-drive-folder-selector.svelte | 5 +-- .../[sourceType]/action}/+server.ts | 43 ++++++++----------- .../[sourceType]/action/action.test.ts} | 23 ++++++++-- 4 files changed, 41 insertions(+), 34 deletions(-) rename web/src/routes/api/{preview-action => connectors/[sourceType]/action}/+server.ts (83%) rename web/src/routes/api/{preview-action/preview-action.test.ts => connectors/[sourceType]/action/action.test.ts} (86%) diff --git a/services/connector-manager/src/models.rs b/services/connector-manager/src/models.rs index e11098244..0f4cac67d 100644 --- a/services/connector-manager/src/models.rs +++ b/services/connector-manager/src/models.rs @@ -92,7 +92,7 @@ pub struct TriggerSyncResponse { } /// Transient credentials accepted during setup/discovery before a source or -/// credential row exists in the database. Used by the setup UI to preview +/// credential row exists in the database. Used by the setup UI to discover /// connector capabilities (e.g. list shared drives) with an inline credential /// that has not yet been persisted. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -122,7 +122,7 @@ pub struct ExecuteActionRequest { /// Source type hint, required when `transient_credentials` are provided. #[serde(default)] pub source_type: Option, - /// Inline credentials for setup/preview actions. When present, the request + /// Inline credentials for setup/discovery actions. When present, the request /// is handled in **transient mode**: `source_id` and DB credentials are /// not used, and `source_type` must identify the connector. #[serde(default)] diff --git a/web/src/lib/components/google-drive-folder-selector.svelte b/web/src/lib/components/google-drive-folder-selector.svelte index c17d33d5c..f52455050 100644 --- a/web/src/lib/components/google-drive-folder-selector.svelte +++ b/web/src/lib/components/google-drive-folder-selector.svelte @@ -147,12 +147,11 @@ let response: Response if (serviceAccountJson && principalEmail && domain) { - // Use preview API with transient credentials - response = await fetch('/api/preview-action', { + // Invoke the connector directly with transient credentials. + response = await fetch('/api/connectors/google_drive/action', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - sourceType: 'google_drive', action: 'discover_folders', params: {}, serviceAccountJson, diff --git a/web/src/routes/api/preview-action/+server.ts b/web/src/routes/api/connectors/[sourceType]/action/+server.ts similarity index 83% rename from web/src/routes/api/preview-action/+server.ts rename to web/src/routes/api/connectors/[sourceType]/action/+server.ts index f8a46ec1d..386e61101 100644 --- a/web/src/routes/api/preview-action/+server.ts +++ b/web/src/routes/api/connectors/[sourceType]/action/+server.ts @@ -5,18 +5,15 @@ import { logger } from '$lib/server/logger' import { SourceType } from '$lib/types' /** - * POST /api/preview-action + * Invokes an action directly on a connector before a source exists. * - * Admin-only endpoint that forwards a transient credential (not yet persisted) to - * the connector-manager's normal /action endpoint in **transient mode**. - * The credential is never stored or returned. + * The transient credential is forwarded to connector-manager's normal /action + * endpoint and is never stored or returned. * - * SECURITY: Strictly allowlisted — only action='discover_folders', - * sourceType='google_drive', and JWT credentials are accepted. - * - * Request body size is limited to 64KB at the web boundary. + * SECURITY: Strictly allowlisted — currently only google_drive's + * discover_folders action with JWT credentials is accepted. */ -export const POST: RequestHandler = async ({ request, locals }) => { +export const POST: RequestHandler = async ({ params: routeParams, request, locals }) => { // Admin-only if (!locals.user) { throw error(401, 'Unauthorized') @@ -76,21 +73,14 @@ export const POST: RequestHandler = async ({ request, locals }) => { // ======== STRICT ALLOWLIST ======== // Reject unknown top-level fields. - const allowedFields = [ - 'sourceType', - 'action', - 'params', - 'serviceAccountJson', - 'principalEmail', - 'domain', - ] + const allowedFields = ['action', 'params', 'serviceAccountJson', 'principalEmail', 'domain'] for (const key of Object.keys(body)) { if (!allowedFields.includes(key)) { throw error(400, `Unknown field: '${key}'`) } } - const sourceType = body.sourceType as string | undefined + const sourceType = routeParams.sourceType const action = body.action as string | undefined const params = body.params as Record | undefined const serviceAccountJson = body.serviceAccountJson as string | undefined @@ -101,10 +91,10 @@ export const POST: RequestHandler = async ({ request, locals }) => { throw error(400, 'Action is required') } if (action !== 'discover_folders') { - throw error(400, `Preview action '${action}' is not supported`) + throw error(400, `Connector action '${action}' is not supported`) } - if (!sourceType || sourceType !== SourceType.GOOGLE_DRIVE) { - throw error(400, 'Preview action only supports source_type: google_drive') + if (sourceType !== SourceType.GOOGLE_DRIVE) { + throw error(400, `Connector actions are not supported for source type '${sourceType}'`) } // Only allow empty params object for discover_folders. @@ -119,7 +109,10 @@ export const POST: RequestHandler = async ({ request, locals }) => { // Transient credentials must be provided for preview (not optional). if (!serviceAccountJson || !principalEmail || !domain) { - throw error(400, 'serviceAccountJson, principalEmail, and domain are required for preview') + throw error( + 400, + 'serviceAccountJson, principalEmail, and domain are required for connector actions', + ) } // Validate types are strings. @@ -149,7 +142,7 @@ export const POST: RequestHandler = async ({ request, locals }) => { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - source_type: 'google_drive', + source_type: sourceType, user_id: locals.user.id, action: 'discover_folders', params: params || {}, @@ -181,7 +174,7 @@ export const POST: RequestHandler = async ({ request, locals }) => { } else { errorMessage = (await response.text()) || errorMessage } - logger.error(`Preview action failed`, { + logger.error(`Connector action failed`, { status: response.status, error: errorMessage, }) @@ -194,7 +187,7 @@ export const POST: RequestHandler = async ({ request, locals }) => { if (err && typeof err === 'object' && 'status' in err) { throw err } - logger.error('Error executing preview action:', err) + logger.error('Error executing connector action:', err) throw error(500, 'Internal server error') } } diff --git a/web/src/routes/api/preview-action/preview-action.test.ts b/web/src/routes/api/connectors/[sourceType]/action/action.test.ts similarity index 86% rename from web/src/routes/api/preview-action/preview-action.test.ts rename to web/src/routes/api/connectors/[sourceType]/action/action.test.ts index 6e010d59c..46e99e921 100644 --- a/web/src/routes/api/preview-action/preview-action.test.ts +++ b/web/src/routes/api/connectors/[sourceType]/action/action.test.ts @@ -23,7 +23,7 @@ vi.mock('$lib/server/logger', () => ({ const { POST } = await import('./+server') function mockRequest(body: unknown, headers: Record = {}): Request { - return new Request('http://localhost/api/preview-action', { + return new Request('http://localhost/api/connectors/google_drive/action', { method: 'POST', headers: { 'Content-Type': 'application/json', ...headers }, body: JSON.stringify(body), @@ -41,9 +41,11 @@ function mockLocals(): Record { async function callPost( body: unknown, locals?: Record, + sourceType = 'google_drive', ): Promise<{ status: number; body?: unknown }> { try { const response = await POST({ + params: { sourceType }, request: mockRequest(body), locals: locals ?? mockLocals(), } as unknown as Parameters[0]) @@ -58,14 +60,13 @@ async function callPost( } } -describe('POST /api/preview-action', () => { +describe('POST /api/connectors/[sourceType]/action', () => { afterEach(() => { vi.unstubAllGlobals() }) it('requires an admin', async () => { const request = { - sourceType: 'google_drive', action: 'discover_folders', serviceAccountJson: '{}', principalEmail: 'a@b.com', @@ -76,6 +77,21 @@ describe('POST /api/preview-action', () => { expect((await callPost(request, { user: { id: 'u1', role: 'member' } })).status).toBe(403) }) + it('rejects connector source types without an allowlisted transient action', async () => { + const response = await callPost( + { + action: 'discover_folders', + serviceAccountJson: '{}', + principalEmail: 'a@b.com', + domain: 'b.com', + }, + undefined, + 'slack', + ) + + expect(response.status).toBe(400) + }) + it('forwards validated setup credentials through the normal action endpoint', async () => { const connectorResponse = { status: 'success', @@ -90,7 +106,6 @@ describe('POST /api/preview-action', () => { vi.stubGlobal('fetch', connectorFetch) const response = await callPost({ - sourceType: 'google_drive', action: 'discover_folders', serviceAccountJson: '{"client_email":"service@example.com"}', principalEmail: 'admin@example.com', From cae3912ebb0297c82dd80201a7d29923f1191cf9 Mon Sep 17 00:00:00 2001 From: Praveen Sampath Date: Wed, 29 Jul 2026 07:49:05 +0000 Subject: [PATCH 4/4] fix(web): repair Drive setup interactions - Render Drive folder suggestions in a portal so dialogs do not clip them. - Restore source creation by using the submitted config instead of an undefined variable. - Remove the unused environment import and preserve authenticated-user narrowing. --- .../google-drive-folder-selector.svelte | 56 ++++++++++--------- web/src/routes/api/sources/+server.ts | 12 ++-- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/web/src/lib/components/google-drive-folder-selector.svelte b/web/src/lib/components/google-drive-folder-selector.svelte index f52455050..4106fc93f 100644 --- a/web/src/lib/components/google-drive-folder-selector.svelte +++ b/web/src/lib/components/google-drive-folder-selector.svelte @@ -3,6 +3,7 @@ import { Input } from '$lib/components/ui/input' import { X, Loader2, Search, AlertCircle, RefreshCw } from '@lucide/svelte' import * as Alert from '$lib/components/ui/alert' + import * as Popover from '$lib/components/ui/popover' import { onDestroy } from 'svelte' import type { FolderPathFilter } from '$lib/types' import type { DriveFolderDiscoveryEntry, DriveFolderDiscoveryResponse } from '$lib/types/search' @@ -36,6 +37,7 @@ let hasLoaded = $state(false) let errorMessage = $state('') let inputRef = $state(null) + let dropdownAnchor = $state(null) // Guard: only start auto-discovery after mount, and only when credential // context actually changes (not on first mount where persisted selected @@ -349,30 +351,39 @@ {/if} -
- - - {#if isLoading} - - {/if} + +
+ + + {#if isLoading} + + {/if} +
- {#if showDropdown && filteredItems.length > 0} -
+ event.preventDefault()} + onCloseAutoFocus={(event) => event.preventDefault()}> {#each filteredItems as item (item.id)} {/each} -
- {:else if showDropdown && searchQuery.trim().length > 0 && !isLoading} -
- No matching folders -
+ {/if} -
+ {#if selected.length > 0} diff --git a/web/src/routes/api/sources/+server.ts b/web/src/routes/api/sources/+server.ts index 7fb15629b..4078996d8 100644 --- a/web/src/routes/api/sources/+server.ts +++ b/web/src/routes/api/sources/+server.ts @@ -7,7 +7,6 @@ import { ulid } from 'ulid' import { logger } from '$lib/server/logger' import { SourceType, DEFAULT_SYNC_INTERVAL_SECONDS } from '$lib/types' import { getSourcesByType } from '$lib/server/db/sources' -import { env } from '$env/dynamic/private' export const GET: RequestHandler = async ({ locals }) => { if (!locals.user) { @@ -77,7 +76,8 @@ export const GET: RequestHandler = async ({ locals }) => { } export const POST: RequestHandler = async ({ request, locals }) => { - if (!locals.user) { + const user = locals.user + if (!user) { throw error(401, 'Unauthorized') } @@ -92,7 +92,7 @@ export const POST: RequestHandler = async ({ request, locals }) => { throw error(400, 'Name and sourceType are required') } - if (scope === 'org' && locals.user.role !== 'admin') { + if (scope === 'org' && user.role !== 'admin') { throw error(403, 'Only admins can create org-wide sources') } const sourcesOfType = await getSourcesByType(sourceType) @@ -104,7 +104,7 @@ export const POST: RequestHandler = async ({ request, locals }) => { const uniqueSourceTypes: string[] = [SourceType.GOOGLE_DRIVE, SourceType.GMAIL] if (uniqueSourceTypes.includes(sourceType)) { const existingForUser = sourcesOfType.find( - (s) => s.scope === 'user' && s.createdBy === locals.user.id, + (s) => s.scope === 'user' && s.createdBy === user.id, ) if (existingForUser) { throw error(409, `A ${sourceType} source already exists`) @@ -119,8 +119,8 @@ export const POST: RequestHandler = async ({ request, locals }) => { name, sourceType, scope, - config: validatedConfig || {}, - createdBy: locals.user.id, + config: config || {}, + createdBy: user.id, isActive: isActive ?? false, syncIntervalSeconds: DEFAULT_SYNC_INTERVAL_SECONDS[sourceType as SourceType], })