diff --git a/crates/yaak-commands/src/auth.rs b/crates/yaak-commands/src/auth.rs index b171c43f..8cdfae5f 100644 --- a/crates/yaak-commands/src/auth.rs +++ b/crates/yaak-commands/src/auth.rs @@ -6,14 +6,11 @@ //! own environment chain before a plugin sees them, or an auth plugin receives //! `${[ api_key ]}` where it expected a key. -use crate::error::{Error, Result}; +use crate::error::Result; use crate::host::PluginHost; -use crate::render::render_json_value; -use std::collections::HashMap; -use yaak_models::models::AnyModel; +use crate::render::render_form_values; use yaak_plugins::events::{ - GetHttpAuthenticationConfigResponse, GetHttpAuthenticationSummaryResponse, JsonPrimitive, - RenderPurpose, + GetHttpAuthenticationConfigResponse, GetHttpAuthenticationSummaryResponse, RenderPurpose, }; use yaak_rpc_schema::*; use yaak_templates::RenderOptions; @@ -31,7 +28,7 @@ pub async fn cmd_get_http_authentication_config( ) -> Result { // A config form is being displayed, so a template that cannot resolve // should show as blank rather than refuse to open the form. - let values = render_auth_values( + let values = render_form_values( &host, &req.model, req.environment_id.as_deref(), @@ -50,7 +47,7 @@ pub async fn cmd_call_http_authentication_action( ) -> Result<()> { // An action actually uses these values, so an unresolvable template is an // error rather than an empty string that would silently authenticate wrong. - let values = render_auth_values( + let values = render_form_values( &host, &req.model, req.environment_id.as_deref(), @@ -63,40 +60,3 @@ pub async fn cmd_call_http_authentication_action( host.call_http_authentication_action(&req.auth_name, req.action_index, values, req.model.id()) .await } - -/// Render the form's values against the environment chain the model sits in. -/// -/// The chain depends on where the model lives — a request inherits through its -/// folder, a workspace has only its own — so the model is what decides which -/// variables are in scope. -async fn render_auth_values( - host: &H, - model: &AnyModel, - environment_id: Option<&str>, - values: HashMap, - purpose: RenderPurpose, - options: &RenderOptions, -) -> Result> { - let (workspace_id, folder_id) = match model { - AnyModel::HttpRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()), - AnyModel::GrpcRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()), - AnyModel::WebsocketRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()), - AnyModel::Folder(f) => (f.workspace_id.clone(), f.folder_id.clone()), - AnyModel::Workspace(w) => (w.id.clone(), None), - other => { - return Err(Error::Generic(format!( - "Cannot resolve authentication for a {}", - other.model() - ))); - } - }; - - let environment_chain = - host.db().resolve_environments(&workspace_id, folder_id.as_deref(), environment_id)?; - - let cb = host.template_callback(purpose); - let rendered = - render_json_value(serde_json::to_value(&values)?, environment_chain, &cb, options).await?; - - Ok(serde_json::from_value(rendered)?) -} diff --git a/crates/yaak-commands/src/host.rs b/crates/yaak-commands/src/host.rs index 2ed259ee..fb78df5a 100644 --- a/crates/yaak-commands/src/host.rs +++ b/crates/yaak-commands/src/host.rs @@ -130,6 +130,7 @@ pub trait PluginHost: Host { ) -> impl Future>>; /// The form a template function wants to show for the given values. + /// `values` arrive already rendered. fn template_function_config( &self, function_name: &str, diff --git a/crates/yaak-commands/src/render.rs b/crates/yaak-commands/src/render.rs index 3e6d794f..5d652700 100644 --- a/crates/yaak-commands/src/render.rs +++ b/crates/yaak-commands/src/render.rs @@ -1,12 +1,19 @@ //! Rendering a template against an environment chain. //! //! The variables come from the chain, the functions come from the host's -//! template callback. Neither of these knows which host it is running under — -//! that is the whole point of taking the callback as a parameter. +//! template callback. `render_template` and `render_json_value` know nothing +//! about which host they run under — that is the whole point of taking the +//! callback as a parameter. `render_form_values` sits one level up: resolving +//! the chain a model sits in is an ordinary database read, so it takes the +//! host and does that read before rendering. +use crate::error::{Error, Result}; +use crate::host::PluginHost; use serde_json::Value; -use yaak_models::models::Environment; +use std::collections::HashMap; +use yaak_models::models::{AnyModel, Environment}; use yaak_models::render::make_vars_hashmap; +use yaak_plugins::events::{JsonPrimitive, RenderPurpose}; use yaak_templates::{RenderOptions, TemplateCallback, parse_and_render, render_json_value_raw}; pub async fn render_template( @@ -28,3 +35,40 @@ pub async fn render_json_value( let vars = &make_vars_hashmap(environment_chain); render_json_value_raw(value, vars, cb, opt).await } + +/// Render a config form's values against the environment chain the model sits in. +/// +/// The chain depends on where the model lives — a request inherits through its +/// folder, a workspace has only its own — so the model is what decides which +/// variables are in scope. +pub(crate) async fn render_form_values( + host: &H, + model: &AnyModel, + environment_id: Option<&str>, + values: HashMap, + purpose: RenderPurpose, + options: &RenderOptions, +) -> Result> { + let (workspace_id, folder_id) = match model { + AnyModel::HttpRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()), + AnyModel::GrpcRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()), + AnyModel::WebsocketRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()), + AnyModel::Folder(f) => (f.workspace_id.clone(), f.folder_id.clone()), + AnyModel::Workspace(w) => (w.id.clone(), None), + other => { + return Err(Error::Generic(format!( + "Cannot resolve environments for a {}", + other.model() + ))); + } + }; + + let environment_chain = + host.db().resolve_environments(&workspace_id, folder_id.as_deref(), environment_id)?; + + let cb = host.template_callback(purpose); + let rendered = + render_json_value(serde_json::to_value(&values)?, environment_chain, &cb, options).await?; + + Ok(serde_json::from_value(rendered)?) +} diff --git a/crates/yaak-commands/src/templates.rs b/crates/yaak-commands/src/templates.rs index 1664e2ad..926090a5 100644 --- a/crates/yaak-commands/src/templates.rs +++ b/crates/yaak-commands/src/templates.rs @@ -7,7 +7,7 @@ use crate::error::Result; use crate::host::PluginHost; -use crate::render::render_template; +use crate::render::{render_form_values, render_template}; use yaak_plugins::events::{ GetTemplateFunctionConfigResponse, GetTemplateFunctionSummaryResponse, GetThemesResponse, RenderPurpose, @@ -56,7 +56,19 @@ pub async fn cmd_template_function_config( host: H, req: CmdTemplateFunctionConfigReq, ) -> Result { - host.template_function_config(&req.function_name, req.values, req.model.id()).await + // A config form is being displayed, so a template that cannot resolve + // should show as blank rather than refuse to open the form. + let values = render_form_values( + &host, + &req.model, + req.environment_id.as_deref(), + req.values, + RenderPurpose::Preview, + &RenderOptions::return_empty(), + ) + .await?; + + host.template_function_config(&req.function_name, values, req.model.id()).await } pub async fn cmd_get_themes( diff --git a/crates/yaak-commands/tests/test_host.rs b/crates/yaak-commands/tests/test_host.rs index 1bfc3995..6a485d1f 100644 --- a/crates/yaak-commands/tests/test_host.rs +++ b/crates/yaak-commands/tests/test_host.rs @@ -18,7 +18,7 @@ use yaak_commands::models::{ cmd_default_headers, cmd_get_workspace_meta, models_delete, models_upsert, models_workspace_models, }; -use yaak_commands::templates::cmd_render_template; +use yaak_commands::templates::{cmd_render_template, cmd_template_function_config}; use yaak_commands::{Host, PluginHost}; use yaak_core::WorkspaceContext; use yaak_crypto::manager::EncryptionManager; @@ -172,6 +172,8 @@ struct SingleThreadedHost { /// The values the last auth-config call arrived with, so a test can check /// they were rendered before the host ever saw them. auth_values: Rc>>>, + /// Same, for the last template-function-config call. + fn_values: Rc>>>, } impl Host for SingleThreadedHost { @@ -261,9 +263,10 @@ impl PluginHost for SingleThreadedHost { async fn template_function_config( &self, function_name: &str, - _values: HashMap, + values: HashMap, _model_id: &str, ) -> yaak_commands::Result { + *self.fn_values.borrow_mut() = Some(values); Err(yaak_commands::Error::Generic(format!("no plugin provides {function_name}()"))) } @@ -376,6 +379,7 @@ async fn a_single_threaded_host_can_implement_the_trait() { let host = SingleThreadedHost { inner: Rc::new(Arc::into_inner(inner).expect("sole owner")), auth_values: Rc::new(RefCell::new(None)), + fn_values: Rc::new(RefCell::new(None)), }; let workspace = Workspace { name: "From one thread".to_string(), ..Default::default() }; @@ -448,6 +452,7 @@ async fn auth_values_are_rendered_before_the_host_sees_them() { let host = SingleThreadedHost { inner: Rc::new(Arc::into_inner(inner).expect("sole owner")), auth_values: Rc::new(RefCell::new(None)), + fn_values: Rc::new(RefCell::new(None)), }; let workspace = host @@ -499,3 +504,65 @@ async fn auth_values_are_rendered_before_the_host_sees_them() { seen.get("password"), ); } + +/// Same contract as auth: template function argument values may contain +/// templates (the 1Password token argument defaults to `${[1PASSWORD_TOKEN]}`), +/// and the shared handler renders them before the host is called. +#[tokio::test] +async fn template_function_values_are_rendered_before_the_host_sees_them() { + let TestHost { inner } = TestHost::new(); + let host = SingleThreadedHost { + inner: Rc::new(Arc::into_inner(inner).expect("sole owner")), + auth_values: Rc::new(RefCell::new(None)), + fn_values: Rc::new(RefCell::new(None)), + }; + + let workspace = host + .db() + .upsert_workspace( + &Workspace { name: "Functions".to_string(), ..Default::default() }, + &host.update_source(), + ) + .expect("workspace"); + host.db() + .upsert_environment( + &Environment { + workspace_id: workspace.id.clone(), + name: "Env".to_string(), + variables: vec![EnvironmentVariable { + enabled: true, + name: "1PASSWORD_TOKEN".to_string(), + value: "ops_abc123".to_string(), + id: None, + }], + ..Default::default() + }, + &host.update_source(), + ) + .expect("environment"); + let environment = + host.db().list_environments_ensure_base(&workspace.id).expect("list").remove(0); + + let mut values = HashMap::new(); + values.insert("token".to_string(), JsonPrimitive::String("${[1PASSWORD_TOKEN]}".to_string())); + + // The host refuses the call itself — it has no plugins — but only after the + // handler has rendered and handed over the values, which is what matters. + let _ = cmd_template_function_config( + host.clone(), + yaak_rpc_schema::CmdTemplateFunctionConfigReq { + function_name: "1password.item".to_string(), + values, + model: AnyModel::Workspace(workspace), + environment_id: Some(environment.id), + }, + ) + .await; + + let seen = host.fn_values.borrow().clone().expect("the host should have been called"); + assert!( + matches!(seen.get("token"), Some(JsonPrimitive::String(v)) if v == "ops_abc123"), + "the template should have been rendered before reaching the host, got {:?}", + seen.get("token"), + ); +}