From bee719813e61bcc6ce1c49693fac643e18f41cd7 Mon Sep 17 00:00:00 2001 From: HugoCasa Date: Thu, 25 Apr 2024 17:21:00 +0200 Subject: [PATCH] feat: app select policy (#3610) * feat: app select policy * fix: question marks --- backend/windmill-api/openapi.yaml | 6 + backend/windmill-api/src/apps.rs | 206 +++++-- .../lib/components/LightweightArgInput.svelte | 557 +++++++++--------- .../LightweightObjectResourceInput.svelte | 26 +- .../components/LightweightSchemaForm.svelte | 4 + .../components/buttons/AppSchemaForm.svelte | 4 +- .../helpers/RunnableComponent.svelte | 7 +- .../apps/editor/AppEditorHeader.svelte | 12 +- .../lib/components/apps/editor/appUtils.ts | 60 +- .../apps/editor/component/components.ts | 1 + .../settingsPanel/InputsSpecsEditor.svelte | 1 + 11 files changed, 539 insertions(+), 345 deletions(-) diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 2bf7b865cc..ac1642b598 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -4907,6 +4907,8 @@ paths: - language force_viewer_static_fields: type: object + force_viewer_one_of_fields: + type: object required: - args - component @@ -9888,6 +9890,10 @@ components: type: object additionalProperties: type: object + triggerables_v2: + type: object + additionalProperties: + type: object execution_mode: type: string enum: [viewer, publisher, anonymous] diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index d125245d99..cb7becda60 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -143,6 +143,7 @@ pub struct AppHistoryUpdate { } pub type StaticFields = HashMap>; +pub type OneOfFields = HashMap>>; #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(rename_all = "lowercase")] @@ -152,6 +153,12 @@ pub enum ExecutionMode { Viewer, } +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct PolicyTriggerableInputs { + static_inputs: StaticFields, + one_of_inputs: OneOfFields, +} + #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Policy { pub on_behalf_of: Option, @@ -160,7 +167,10 @@ pub struct Policy { // - script/ // - flow/ // - rawscript/ - pub triggerables: HashMap, + #[serde(skip_serializing_if = "Option::is_none")] + pub triggerables: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub triggerables_v2: Option>, pub execution_mode: ExecutionMode, } @@ -934,6 +944,7 @@ pub struct ExecuteApp { pub raw_code: Option, // 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, } fn digest(code: &str) -> String { @@ -966,39 +977,56 @@ async fn execute_component( let path = path.to_path(); - let policy = if let Some(static_fields) = payload.clone().force_viewer_static_fields { - let mut hm = HashMap::new(); + let policy = match payload.clone() { + ExecuteApp { + force_viewer_static_fields: Some(static_fields), + force_viewer_one_of_fields: Some(one_of_fields), + .. + } => { + let mut hm = HashMap::new(); - if let Some(path) = payload.path.clone() { - hm.insert(format!("{}:{path}", payload.component), static_fields); - } else { - hm.insert( - format!( - "{}:{}", - payload.component, - digest(payload.raw_code.clone().unwrap().content.as_str()) - ), - static_fields, - ); + if let Some(path) = payload.path.clone() { + hm.insert( + format!("{}:{path}", payload.component), + PolicyTriggerableInputs { + static_inputs: static_fields, + one_of_inputs: one_of_fields, + }, + ); + } else { + hm.insert( + format!( + "{}:{}", + payload.component, + digest(payload.raw_code.clone().unwrap().content.as_str()) + ), + PolicyTriggerableInputs { + static_inputs: static_fields, + one_of_inputs: one_of_fields, + }, + ); + } + Policy { + execution_mode: ExecutionMode::Viewer, + triggerables: None, + triggerables_v2: Some(hm), + on_behalf_of: None, + on_behalf_of_email: None, + } } - Policy { - execution_mode: ExecutionMode::Viewer, - triggerables: hm, - on_behalf_of: None, - on_behalf_of_email: None, + _ => { + let policy_o = sqlx::query_scalar!( + "SELECT policy from app WHERE path = $1 AND workspace_id = $2", + path, + &w_id + ) + .fetch_optional(&db) + .await?; + + let policy = not_found_if_none(policy_o, "App", path)?; + + serde_json::from_value::(policy).map_err(to_anyhow)? } - } else { - let policy_o = sqlx::query_scalar!( - "SELECT policy from app WHERE path = $1 AND workspace_id = $2", - path, - &w_id - ) - .fetch_optional(&db) - .await?; - - let policy = not_found_if_none(policy_o, "App", path)?; - - serde_json::from_value::(policy).map_err(to_anyhow)? }; let (username, permissioned_as, email) = match policy.execution_mode { @@ -1133,22 +1161,106 @@ fn build_args( path: String, args: HashMap>, ) -> Result>>> { - // disallow var and res access in args coming from the user for security reasons - let mut safe_args: HashMap> = args.clone(); + + let key = format!("{}:{}", component, &path); + let (static_inputs, one_of_inputs) = match policy { + Policy { triggerables_v2: Some(t), .. } => { + let PolicyTriggerableInputs { static_inputs, one_of_inputs } = t + .get(&key) + .or_else(|| t.get(&path)) + .map(|x| x.clone()) + .or_else(|| { + if matches!(policy.execution_mode, ExecutionMode::Viewer) { + Some(PolicyTriggerableInputs { + static_inputs: HashMap::new(), + one_of_inputs: HashMap::new(), + }) + } else { + None + } + }) + .ok_or_else(|| { + Error::BadRequest(format!("path {} is not allowed in the app policy", path)) + })?; + + (static_inputs, one_of_inputs) + } + Policy { triggerables: Some(t), .. } => { + let static_inputs = t + .get(&key) + .or_else(|| t.get(&path)) + .map(|x| x.clone()) + .or_else(|| { + if matches!(policy.execution_mode, ExecutionMode::Viewer) { + Some(HashMap::new()) + } else { + None + } + }) + .ok_or_else(|| { + Error::BadRequest(format!("path {} is not allowed in the app policy", path)) + })?; + + (static_inputs, HashMap::new()) + } + _ => Err(Error::BadRequest(format!( + "Policy is missing triggerables for {}", + key + )))?, + }; + + let mut args = args.clone(); + let mut safe_args = HashMap::>::new(); + + for (k, v) in one_of_inputs { + if let Some(arg_val) = args.get(&k) { + let arg_str = arg_val.get(); + + let options_str_vec = v.iter().map(|x| x.get()).collect::>(); + if options_str_vec.contains(&arg_str) { + safe_args.insert(k.to_string(), arg_val.clone()); + args.remove(&k); + continue; + } + + // check if multiselect + if let Ok(args_str_vec) = serde_json::from_str::>>(arg_val.get()) { + if args_str_vec + .iter() + .all(|x| options_str_vec.contains(&x.get())) + { + safe_args.insert(k.to_string(), arg_val.clone()); + args.remove(&k); + continue; + } + } + + return Err(Error::BadRequest(format!( + "argument {} with value {} must be one of [{}]", + k, + arg_str, + options_str_vec.join(",") + ))); + } + } + for (k, v) in args { - let args_str = serde_json::to_string(&v).unwrap_or_else(|_| "".to_string()); - if args_str.contains("$var:") || args_str.contains("$res:") { + let arg_str = serde_json::to_string(&v).unwrap_or_else(|_| "".to_string()); + + if !arg_str.contains("$var:") && !arg_str.contains("$res:") { + safe_args.insert(k.to_string(), v); + } else { safe_args.insert( k.to_string(), RawValue::from_string( - args_str + arg_str .replace( "$var:", - "The following variable has been ommited for security reasons: ", + "The following variable has been omitted for security reasons: ", ) .replace( "$res:", - "The following resource has been ommited for security reasons: ", + "The following resource has been omitted for security reasons: ", ), ) .map_err(|e| { @@ -1160,24 +1272,8 @@ fn build_args( ); } } - let key = format!("{}:{}", component, &path); - let static_args = policy - .triggerables - .get(&key) - .or_else(|| policy.triggerables.get(&path)) - .map(|x| x.clone()) - .or_else(|| { - if matches!(policy.execution_mode, ExecutionMode::Viewer) { - Some(HashMap::new()) - } else { - None - } - }) - .ok_or_else(|| { - Error::BadRequest(format!("path {} is not allowed in the app policy", path)) - })?; let mut extra = HashMap::new(); - for (k, v) in static_args { + for (k, v) in static_inputs { extra.insert(k.to_string(), v.to_owned()); } Ok(PushArgs { extra, args: sqlx::types::Json(safe_args) }) diff --git a/frontend/src/lib/components/LightweightArgInput.svelte b/frontend/src/lib/components/LightweightArgInput.svelte index 4feaf1df4f..a28281e34c 100644 --- a/frontend/src/lib/components/LightweightArgInput.svelte +++ b/frontend/src/lib/components/LightweightArgInput.svelte @@ -47,6 +47,8 @@ export let extra: Record = {} export let displayType: boolean = true export let customErrorMessage: string | undefined = undefined + export let hideResourceInput: boolean = false + export let resourceInputUnsupported: boolean = false const dispatch = createEventDispatcher() @@ -158,296 +160,303 @@ } -
-
- {#if displayHeader} - - {/if} - - {#if description} -
- 
{description}
-
- {/if} - -
- {#if inputCat == 'number'} - {#if extra['min'] != undefined && extra['max'] != undefined} -
- {extra['min']} -
- -
- {extra['max']} - {value} -
- {:else if extra?.currency} - - {:else} - { - dispatch('focus') - }} - type="number" - class={twMerge( - valid && error == '' - ? '' - : 'border !border-red-700 !border-opacity-70 focus:!border-red-700 focus:!border-opacity-30' - )} - placeholder={defaultValue ?? ''} - bind:value - min={extra['min']} - max={extra['max']} - /> - {/if} - {:else if inputCat == 'boolean'} - { - e?.stopPropagation() - }} - class={valid && error == '' - ? '' - : 'border !border-red-700 !border-opacity-70 focus:!border-red-700 focus:!border-opacity-30'} - bind:checked={value} +{#if !(hideResourceInput && inputCat === 'resource-object')} +
+
+ {#if displayHeader} + - {#if type == 'boolean' && value == undefined} -   Not set - {/if} - {:else if inputCat == 'list'} -
- {#if Array.isArray(itemsType?.multiselect) && Array.isArray(value)} -
- + {/if} + + {#if description} +
+ 
{description}
+
+ {/if} + +
+ {#if inputCat == 'number'} + {#if extra['min'] != undefined && extra['max'] != undefined} +
+ {extra['min']} +
+ +
+ {extra['max']} + {value}
- {:else if Array.isArray(itemsType?.enum) && Array.isArray(value)} -
- -
- {:else if Array.isArray(enum_) && Array.isArray(value)} -
- + {:else} + { + dispatch('focus') + }} + type="number" + class={twMerge( + valid && error == '' + ? '' + : 'border !border-red-700 !border-opacity-70 focus:!border-red-700 focus:!border-opacity-30' + )} + placeholder={defaultValue ?? ''} + bind:value + min={extra['min']} + max={extra['max']} + /> + {/if} + {:else if inputCat == 'boolean'} + { + e?.stopPropagation() + }} + class={valid && error == '' + ? '' + : 'border !border-red-700 !border-opacity-70 focus:!border-red-700 focus:!border-opacity-30'} + bind:checked={value} + /> + {#if type == 'boolean' && value == undefined} +   Not set + {/if} + {:else if inputCat == 'list'} +
+ {#if Array.isArray(itemsType?.multiselect) && Array.isArray(value)} +
+ +
+ {:else if Array.isArray(itemsType?.enum) && Array.isArray(value)} +
+ +
+ {:else if Array.isArray(enum_) && Array.isArray(value)} +
+ +
+ {:else} +
+ {#if Array.isArray(value)} + {#each value ?? [] as v, i} +
+ {#if itemsType?.type == 'number'} + + {:else if itemsType?.type == 'string' && itemsType?.contentEncoding == 'base64'} + fileChanged(x, (val) => (value[i] = val))} + multiple={false} + /> + {:else if Array.isArray(itemsType?.enum)} + + {:else} + + {/if} + +
+ {/each} + {:else if value != undefined} + List is not an array + {/if} +
+
+ +
+ + {(value ?? []).length} item{(value ?? []).length != 1 ? 's' : ''} + + {/if} +
+ {:else if inputCat == 'resource-object'} + + {:else if inputCat == 'object'} + {#if properties && Object.keys(properties).length > 0} +
+
{:else} -
- {#if Array.isArray(value)} - {#each value ?? [] as v, i} -
- {#if itemsType?.type == 'number'} - - {:else if itemsType?.type == 'string' && itemsType?.contentEncoding == 'base64'} - fileChanged(x, (val) => (value[i] = val))} - multiple={false} - /> - {:else if Array.isArray(itemsType?.enum)} - - {:else} - - {/if} - -
- {/each} - {:else if value != undefined} - List is not an array - {/if} -
-
- -
- - {(value ?? []).length} item{(value ?? []).length != 1 ? 's' : ''} - - {/if} -
- {:else if inputCat == 'resource-object'} - - {:else if inputCat == 'object'} - {#if properties && Object.keys(properties).length > 0} -
- { + dispatch('focus') + }} + use:autosize + style="min-height: 5px;" + class="col-span-10 {valid && error == '' + ? '' + : 'border !border-red-700 !border-opacity-70 focus:!border-red-700 focus:!border-opacity-30'}" + placeholder={defaultValue ? JSON.stringify(defaultValue, null, 4) : ''} + bind:value={rawValue} /> -
- {:else} -