From 3db4e3bab1de6a6789368350f03c5fc110adb943 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 27 May 2024 07:51:49 +0200 Subject: [PATCH] feat: allow user resources in apps with a toggle (#3821) * all * all * all * all * all * all * all * all * all * all * nits * nits --- backend/Cargo.lock | 1 + backend/windmill-api/Cargo.toml | 3 +- backend/windmill-api/openapi.yaml | 51 +- backend/windmill-api/src/apps.rs | 143 +++- backend/windmill-api/src/variables.rs | 3 +- backend/windmill-api/src/workspaces.rs | 4 +- backend/windmill-common/src/variables.rs | 27 +- backend/windmill-worker/src/common.rs | 17 +- .../src/lib/components/ApiConnectForm.svelte | 5 +- frontend/src/lib/components/AppConnect.svelte | 631 ------------------ .../lib/components/AppConnectDrawer.svelte | 82 +++ .../src/lib/components/AppConnectInner.svelte | 550 +++++++++++++++ frontend/src/lib/components/ArgInput.svelte | 1 - .../src/lib/components/CustomOauth.svelte | 4 +- .../components/FlowStatusViewerInner.svelte | 2 +- .../lib/components/LightweightArgInput.svelte | 569 ++++++++-------- .../LightweightObjectResourceInput.svelte | 26 +- .../LightweightResourcePicker.svelte | 100 ++- .../components/LightweightSchemaForm.svelte | 4 - .../lib/components/PasswordArgInput.svelte | 2 +- .../src/lib/components/ResourcePicker.svelte | 10 +- .../src/lib/components/SchemaEditor.svelte | 3 + .../src/lib/components/SchemaModal.svelte | 46 +- frontend/src/lib/components/app_connect.ts | 84 +++ .../components/buttons/AppSchemaForm.svelte | 4 +- .../helpers/RunnableComponent.svelte | 25 +- .../apps/editor/AppEditorHeader.svelte | 46 +- .../lib/components/apps/editor/appUtils.ts | 3 +- .../settingsPanel/ComponentControl.svelte | 2 +- .../settingsPanel/InputsSpecEditor.svelte | 17 +- frontend/src/lib/components/apps/inputType.ts | 3 + frontend/src/lib/components/apps/utils.ts | 2 +- .../components/details/createAppFromScript.ts | 4 + .../oauth/callback/[client_name]/+page.svelte | 29 +- .../oauth/callback_slack/+page.svelte | 5 +- .../(root)/(logged)/resources/+page.svelte | 11 +- .../src/routes/(root)/embed_connect/+page.js | 5 + .../routes/(root)/embed_connect/+page.svelte | 67 ++ 38 files changed, 1521 insertions(+), 1070 deletions(-) delete mode 100644 frontend/src/lib/components/AppConnect.svelte create mode 100644 frontend/src/lib/components/AppConnectDrawer.svelte create mode 100644 frontend/src/lib/components/AppConnectInner.svelte create mode 100644 frontend/src/lib/components/app_connect.ts create mode 100644 frontend/src/routes/(root)/embed_connect/+page.js create mode 100644 frontend/src/routes/(root)/embed_connect/+page.svelte diff --git a/backend/Cargo.lock b/backend/Cargo.lock index b61d27d8c4..d62751db9c 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -9802,6 +9802,7 @@ dependencies = [ "tower-http", "tracing", "tracing-subscriber", + "ulid", "urlencoding", "uuid 1.8.0", "windmill-audit", diff --git a/backend/windmill-api/Cargo.toml b/backend/windmill-api/Cargo.toml index 0769d3db34..b2dff318a7 100644 --- a/backend/windmill-api/Cargo.toml +++ b/backend/windmill-api/Cargo.toml @@ -89,4 +89,5 @@ openidconnect = { workspace = true, optional = true} pin-project.workspace = true crc.workspace = true http.workspace = true -async-stream.workspace = true \ No newline at end of file +async-stream.workspace = true +ulid.workspace = true \ No newline at end of file diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index d3c8269f20..f68eeddb8c 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -2578,16 +2578,41 @@ paths: content: application/json: schema: - additionalProperties: - type: object - properties: - extra_params: - additionalProperties: - type: string - scopes: - type: array - items: - type: string + type: array + items: + type: string + + /oauth/get_connect/{client}: + get: + summary: get oauth connect + operationId: getOAuthConnect + tags: + - oauth + parameters: + - name: client + description: client name + in: path + required: true + schema: + type: string + responses: + "200": + description: get + content: + application/json: + schema: + type: object + properties: + extra_params: + additionalProperties: + type: string + scopes: + type: array + items: + type: string + + + /w/{workspace}/resources/create: post: @@ -4921,6 +4946,8 @@ paths: type: string path: type: string + lock: + type: string cache_ttl: type: integer required: @@ -4930,6 +4957,10 @@ paths: type: object force_viewer_one_of_fields: type: object + force_viewer_allow_user_resources: + type: array + items: + type: string required: - args - component diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 91d4bb9996..1b986f0d59 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -9,7 +9,9 @@ use std::collections::HashMap; */ use crate::{ db::{ApiAuthed, DB}, + resources::get_resource_value_interpolated_internal, users::{require_owner_of_path, OptAuthed}, + variables::encrypt, webhook_util::{WebhookMessage, WebhookShared}, HTTP_CLIENT, }; @@ -38,7 +40,7 @@ use windmill_common::{ utils::{ http_get_from_hub, not_found_if_none, paginate, query_elems_from_hub, Pagination, StripPath, }, - variables::build_crypt, + variables::{build_crypt, build_crypt_with_key_suffix}, worker::to_raw_value, HUB_BASE_URL, }; @@ -145,6 +147,7 @@ pub struct AppHistoryUpdate { pub type StaticFields = HashMap>; pub type OneOfFields = HashMap>>; +pub type AllowUserResources = Vec; #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(rename_all = "lowercase")] @@ -158,6 +161,7 @@ pub enum ExecutionMode { pub struct PolicyTriggerableInputs { static_inputs: StaticFields, one_of_inputs: OneOfFields, + allow_user_resources: AllowUserResources, } #[derive(Serialize, Deserialize, Debug, Clone)] @@ -940,6 +944,7 @@ pub struct ExecuteApp { // if set, the app is executed as viewer with the given static fields pub force_viewer_static_fields: Option, pub force_viewer_one_of_fields: Option, + pub force_viewer_allow_user_resources: Option, } fn digest(code: &str) -> String { @@ -952,6 +957,7 @@ fn digest(code: &str) -> String { async fn execute_component( OptAuthed(opt_authed): OptAuthed, Extension(db): Extension, + Extension(user_db): Extension, Extension(rsmq): Extension>, Path((w_id, path)): Path<(String, StripPath)>, Json(payload): Json, @@ -976,6 +982,7 @@ async fn execute_component( ExecuteApp { force_viewer_static_fields: Some(static_fields), force_viewer_one_of_fields: Some(one_of_fields), + force_viewer_allow_user_resources: Some(allow_user_resources), .. } => { let mut hm = HashMap::new(); @@ -986,6 +993,7 @@ async fn execute_component( PolicyTriggerableInputs { static_inputs: static_fields, one_of_inputs: one_of_fields, + allow_user_resources, }, ); } else { @@ -998,6 +1006,7 @@ async fn execute_component( PolicyTriggerableInputs { static_inputs: static_fields, one_of_inputs: one_of_fields, + allow_user_resources, }, ); } @@ -1027,22 +1036,31 @@ async fn execute_component( let (username, permissioned_as, email) = match policy.execution_mode { ExecutionMode::Anonymous => { let username = opt_authed - .map(|a| a.username) + .as_ref() + .map(|a| a.username.clone()) .unwrap_or_else(|| "anonymous".to_string()); let (permissioned_as, email) = get_on_behalf_of(&policy)?; (username, permissioned_as, email) } ExecutionMode::Publisher => { - let username = opt_authed.map(|a| a.username).ok_or_else(|| { - Error::BadRequest("publisher execution mode requires authentication".to_string()) - })?; + let username = opt_authed + .as_ref() + .map(|a| a.username.clone()) + .ok_or_else(|| { + Error::BadRequest( + "publisher execution mode requires authentication".to_string(), + ) + })?; let (permissioned_as, email) = get_on_behalf_of(&policy)?; (username, permissioned_as, email) } ExecutionMode::Viewer => { - let (username, email) = opt_authed.map(|a| (a.username, a.email)).ok_or_else(|| { - Error::BadRequest("Required to be authed in viewer mode".to_string()) - })?; + let (username, email) = opt_authed + .as_ref() + .map(|a| (a.username.clone(), a.email.clone())) + .ok_or_else(|| { + Error::BadRequest("Required to be authed in viewer mode".to_string()) + })?; ( username.clone(), username_to_permissioned_as(&username), @@ -1051,17 +1069,37 @@ async fn execute_component( } }; - let (job_payload, args, tag) = match payload { + let (job_payload, (args, job_id), tag) = match payload { ExecuteApp { args, component, raw_code: Some(raw_code), path: None, .. } => { let content = &raw_code.content; let payload = JobPayload::Code(raw_code.clone()); let path = digest(content); - let args = build_args(policy, &component, path, args)?; + let args = build_args( + policy, + &component, + path, + args, + opt_authed.as_ref(), + &user_db, + &db, + &w_id, + ) + .await?; (payload, args, None) } ExecuteApp { args, component, raw_code: None, path: Some(path), .. } => { let (payload, tag) = get_payload_tag_from_prefixed_path(&path, &db, &w_id).await?; - let args = build_args(policy, &component, path.to_string(), args)?; + let args = build_args( + policy, + &component, + path.to_string(), + args, + opt_authed.as_ref(), + &user_db, + &db, + &w_id, + ) + .await?; (payload, args, tag) } _ => unreachable!(), @@ -1081,7 +1119,7 @@ async fn execute_component( None, None, None, - None, + job_id, false, false, None, @@ -1150,16 +1188,21 @@ async fn exists_app( Ok(Json(exists)) } -fn build_args( +async fn build_args( policy: Policy, component: &str, path: String, args: HashMap>, -) -> Result { + authed: Option<&ApiAuthed>, + user_db: &UserDB, + db: &DB, + w_id: &str, +) -> Result<(PushArgs, Option)> { + let mut job_id: Option = None; let key = format!("{}:{}", component, &path); - let (static_inputs, one_of_inputs) = match policy { + let (static_inputs, one_of_inputs, allow_user_resources) = match policy { Policy { triggerables_v2: Some(t), .. } => { - let PolicyTriggerableInputs { static_inputs, one_of_inputs } = t + let PolicyTriggerableInputs { static_inputs, one_of_inputs, allow_user_resources } = t .get(&key) .or_else(|| t.get(&path)) .map(|x| x.clone()) @@ -1168,6 +1211,7 @@ fn build_args( Some(PolicyTriggerableInputs { static_inputs: HashMap::new(), one_of_inputs: HashMap::new(), + allow_user_resources: Vec::new(), }) } else { None @@ -1177,7 +1221,7 @@ fn build_args( Error::BadRequest(format!("path {} is not allowed in the app policy", path)) })?; - (static_inputs, one_of_inputs) + (static_inputs, one_of_inputs, allow_user_resources) } Policy { triggerables: Some(t), .. } => { let static_inputs = t @@ -1195,7 +1239,7 @@ fn build_args( Error::BadRequest(format!("path {} is not allowed in the app policy", path)) })?; - (static_inputs, HashMap::new()) + (static_inputs, HashMap::new(), Vec::new()) } _ => Err(Error::BadRequest(format!( "Policy is missing triggerables for {}", @@ -1206,7 +1250,58 @@ fn build_args( let mut args = args.clone(); let mut safe_args = HashMap::>::new(); + // tracing::error!("{:?}", allow_user_resources); + for k in allow_user_resources.iter() { + if let Some(arg_val) = args.get(k) { + let key = serde_json::from_str::(arg_val.get()).ok(); + if let Some(path) = + key.and_then(|x| x.clone().strip_prefix("$res:").map(|x| x.to_string())) + { + if let Some(authed) = authed { + let res = get_resource_value_interpolated_internal( + authed, + Some(user_db.clone()), + db, + w_id, + &path, + None, + "", + ) + .await?; + if res.is_none() { + return Err(Error::BadRequest(format!( + "Resource {} not found or not allowed for viewer", + path + ))); + } + let job_id = if let Some(job_id) = job_id { + job_id + } else { + job_id = Some(ulid::Ulid::new().into()); + job_id.unwrap() + }; + let mut tx = db.begin().await?; + let mc = + build_crypt_with_key_suffix(&mut tx, &w_id, &job_id.to_string()).await?; + let encrypted = encrypt(&mc, to_raw_value(&res.unwrap()).get()); + tx.commit().await?; + safe_args.insert( + k.to_string(), + to_raw_value(&format!("$encrypted:{encrypted}")), + ); + } else { + return Err(Error::BadRequest( + "User resources are not allowed without being logged in".to_string(), + )); + } + } + } + } + for (k, v) in one_of_inputs { + if safe_args.contains_key(&k) { + continue; + } if let Some(arg_val) = args.get(&k) { let arg_str = arg_val.get(); @@ -1239,9 +1334,13 @@ fn build_args( } for (k, v) in args { - let arg_str = serde_json::to_string(&v).unwrap_or_else(|_| "".to_string()); + if safe_args.contains_key(&k) { + continue; + } - if !arg_str.contains("$var:") && !arg_str.contains("$res:") { + let arg_str = v.get(); + + if !arg_str.contains("\"$var:") && !arg_str.contains("\"$res:") { safe_args.insert(k.to_string(), v); } else { safe_args.insert( @@ -1254,7 +1353,7 @@ fn build_args( ) .replace( "$res:", - "The following resource has been omitted for security reasons: ", + "The following resource has been omitted for security reasons, to allow it, toggle: 'Allow resources from users' on that field input: ", ), ) .map_err(|e| { @@ -1270,5 +1369,5 @@ fn build_args( for (k, v) in static_inputs { extra.insert(k.to_string(), v.to_owned()); } - Ok(PushArgs { extra, args: safe_args }) + Ok((PushArgs { extra, args: safe_args }, job_id)) } diff --git a/backend/windmill-api/src/variables.rs b/backend/windmill-api/src/variables.rs index 1733d8d539..e7e6e5c36c 100644 --- a/backend/windmill-api/src/variables.rs +++ b/backend/windmill-api/src/variables.rs @@ -97,9 +97,10 @@ async fn list_variables( from variable LEFT JOIN account ON variable.account = account.id AND account.workspace_id = $1 LEFT JOIN resource ON resource.path = variable.path AND resource.workspace_id = $1 - WHERE variable.workspace_id = $1 ORDER BY path", + WHERE variable.workspace_id = $1 AND variable.path NOT LIKE 'u/' || $2 || '/secret_arg/%' ORDER BY path", ) .bind(&w_id) + .bind(&authed.username) .fetch_all(&mut *tx) .await?; diff --git a/backend/windmill-api/src/workspaces.rs b/backend/windmill-api/src/workspaces.rs index 301ed36dc6..8efe435e60 100644 --- a/backend/windmill-api/src/workspaces.rs +++ b/backend/windmill-api/src/workspaces.rs @@ -2578,9 +2578,9 @@ async fn tarball_workspace( if !skip_variables.unwrap_or(false) { let variables = sqlx::query_as::<_, ExportableListableVariable>(if !skip_secrets.unwrap_or(false) { - "SELECT * FROM variable WHERE workspace_id = $1" + "SELECT * FROM variable WHERE workspace_id = $1 AND path NOT LIKE 'u/%/secret_arg/%'" } else { - "SELECT * FROM variable WHERE workspace_id = $1 AND is_secret = false" + "SELECT * FROM variable WHERE workspace_id = $1 AND is_secret = false AND path NOT LIKE 'u/%/secret_arg/%'" }) .bind(&w_id) .fetch_all(&mut *tx) diff --git a/backend/windmill-common/src/variables.rs b/backend/windmill-common/src/variables.rs index 48df1cca8b..6d2ae74f48 100644 --- a/backend/windmill-common/src/variables.rs +++ b/backend/windmill-common/src/variables.rs @@ -78,6 +78,20 @@ pub async fn build_crypt<'c>( Ok(magic_crypt::new_magic_crypt!(crypt_key, 256)) } +pub async fn build_crypt_with_key_suffix<'c>( + db: &mut Transaction<'c, Postgres>, + w_id: &str, + key_suffix: &str, +) -> crate::error::Result { + let key = get_workspace_key(w_id, db).await?; + let crypt_key = if let Some(ref salt) = SECRET_SALT.as_ref() { + format!("{}{}{}", key, salt, key_suffix) + } else { + format!("{}{}", key, key_suffix) + }; + Ok(magic_crypt::new_magic_crypt!(crypt_key, 256)) +} + pub async fn get_workspace_key<'c>( w_id: &str, db: &mut Transaction<'c, Postgres>, @@ -118,9 +132,7 @@ pub async fn get_secret_value_as_admin( let mut tx = db.begin().await?; let mc = build_crypt(&mut tx, &w_id).await?; tx.commit().await?; - - mc.decrypt_base64_to_string(value) - .map_err(|e| crate::Error::InternalErr(e.to_string()))? + decrypt_value_with_mc(value, mc).await? } else { "".to_string() } @@ -131,6 +143,15 @@ pub async fn get_secret_value_as_admin( Ok(r) } +pub async fn decrypt_value_with_mc( + value: String, + mc: MagicCrypt256, +) -> Result { + Ok(mc + .decrypt_base64_to_string(value) + .map_err(|e| crate::Error::InternalErr(e.to_string()))?) +} + pub async fn get_reserved_variables( db: &DB, w_id: &str, diff --git a/backend/windmill-worker/src/common.rs b/backend/windmill-worker/src/common.rs index bf8505d86c..fe73ef0116 100644 --- a/backend/windmill-worker/src/common.rs +++ b/backend/windmill-worker/src/common.rs @@ -26,6 +26,7 @@ use windmill_common::s3_helpers::OBJECT_STORE_CACHE_SETTINGS; use windmill_common::s3_helpers::{ get_etag_or_empty, LargeFileStorage, ObjectStoreResource, S3Object, }; +use windmill_common::variables::{build_crypt_with_key_suffix, decrypt_value_with_mc}; use windmill_common::worker::{CLOUD_HOSTED, TMP_DIR, WORKER_CONFIG}; use windmill_common::{ error::{self, Error}, @@ -146,7 +147,7 @@ pub async fn write_file_binary(dir: &str, path: &str, content: &[u8]) -> error:: } lazy_static::lazy_static! { - static ref RE_RES_VAR: Regex = Regex::new(r#"\$(?:var|res)\:"#).unwrap(); + static ref RE_RES_VAR: Regex = Regex::new(r#"\$(?:var|res|encrypted)\:"#).unwrap(); } pub async fn transform_json<'a>( @@ -274,6 +275,20 @@ pub async fn transform_json_value( Error::NotFound(format!("Resource {path} not found for `{name}`: {e:#}")) }) } + Value::String(y) if y.starts_with("$encrypted:") => { + let encrypted = y.strip_prefix("$encrypted:").unwrap(); + let mut tx = db.begin().await?; + let mc = build_crypt_with_key_suffix(&mut tx, &job.workspace_id, &job.id.to_string()) + .await?; + tx.commit().await?; + decrypt_value_with_mc(encrypted.to_string(), mc) + .await + .and_then(|x| { + serde_json::from_str(&x).map_err(|e| Error::InternalErr(e.to_string())) + }) + + // let path = y.strip_prefix("$res:").unwrap(); + } Value::String(y) if y.starts_with("$") => { let flow_path = if let Some(uuid) = job.parent_job { sqlx::query_scalar!("SELECT script_path FROM queue WHERE id = $1", uuid) diff --git a/frontend/src/lib/components/ApiConnectForm.svelte b/frontend/src/lib/components/ApiConnectForm.svelte index 66ae52fa31..edc7bfc14e 100644 --- a/frontend/src/lib/components/ApiConnectForm.svelte +++ b/frontend/src/lib/components/ApiConnectForm.svelte @@ -37,9 +37,8 @@ notFound = true } } - $: { - $workspaceStore && loadSchema() - } + $: $workspaceStore && loadSchema() + $: notFound && rawCode && parseJson() function parseJson() { diff --git a/frontend/src/lib/components/AppConnect.svelte b/frontend/src/lib/components/AppConnect.svelte deleted file mode 100644 index dc127172d6..0000000000 --- a/frontend/src/lib/components/AppConnect.svelte +++ /dev/null @@ -1,631 +0,0 @@ - - - - - - - a[0].localeCompare(b[0])) : undefined} - bind:filteredItems={filteredConnects} - f={(x) => x[0]} -/> - a[0].localeCompare(b[0]))} - bind:filteredItems={filteredConnectsManual} - f={(x) => x[0]} -/> - - { - dispatch('close') - }} - on:open={() => { - loadResources() - }} - size="800px" -> - - {#if step == 1} -
- -
- -

OAuth APIs

-
- {#if filteredConnects} - {#each filteredConnects as [key, values]} - - {/each} - {:else} - {#each new Array(3) as _} - - {/each} - {/if} -
- {#if connects && Object.keys(connects).length == 0} -
No OAuth APIs has been setup on the instance. To add oauth APIs, first sync the resource - types with the hub, then add oauth configuration. See documentation -
- {/if} - {#if manual == false && resourceType != ''} -

Scopes

- {#if !manual && resourceType != ''} - - {/if} - {/if} - -

Others

- - {#if connectsManual && connectsManual?.length < 10} -
- Resource Types have not been synced with the hub. Go to the admins workspace to sync them - (and add a schedule to do daily): -

1. Go to the "admins" workspaces: - sync resource types -

-

- 2: Run the synchronization script: - sync resource types -

-
- {/if} - -
- {#if filteredConnectsManual} - {#each filteredConnectsManual as [key, _]} - {#if nativeLanguagesCategory.includes(key)} - - {/if} - {/each} - {/if} -
- -
-
- {#if filteredConnectsManual} - {#each filteredConnectsManual as [key, _]} - {#if !nativeLanguagesCategory.includes(key)} - - - {/if} - {/each} - {:else} - {#each new Array(9) as _} - - {/each} - {/if} -
- {:else if step == 2 && manual} - - - {#if apiTokenApps[resourceType]} -

Instructions

-
-
    - {#each apiTokenApps[resourceType].instructions as step} -
  1. - {@html step} -
    2. - {/each} -
-
- {#if apiTokenApps[resourceType].img} -
- connect -
- {/if} - {:else if !emptyString(resourceTypeInfo?.description)} -

{resourceTypeInfo?.name} description

-
- -
- {/if} - {#if resourceType == 'postgresql' || resourceType == 'mysql' || resourceType == 'mongodb'} - - {/if} - -

Resource description -
- - -
-

- {#if renderDescription} -
-
GH Markdown
-