diff --git a/backend/.sqlx/query-108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231.json b/backend/.sqlx/query-108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231.json new file mode 100644 index 0000000000..8f22fed492 --- /dev/null +++ b/backend/.sqlx/query-108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT extra_perms from resource WHERE path = $1 AND workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "extra_perms", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231" +} diff --git a/backend/.sqlx/query-7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54.json b/backend/.sqlx/query-7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54.json new file mode 100644 index 0000000000..a2cd73e737 --- /dev/null +++ b/backend/.sqlx/query-7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT extra_perms from folder WHERE name = $1 AND workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "extra_perms", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54" +} diff --git a/backend/.sqlx/query-a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419.json b/backend/.sqlx/query-a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419.json new file mode 100644 index 0000000000..2eb0b4cfc3 --- /dev/null +++ b/backend/.sqlx/query-a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT extra_perms from variable WHERE path = $1 AND workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "extra_perms", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419" +} diff --git a/backend/windmill-api/src/job_helpers.rs b/backend/windmill-api/src/job_helpers.rs index 2c20d97381..958f8759ba 100644 --- a/backend/windmill-api/src/job_helpers.rs +++ b/backend/windmill-api/src/job_helpers.rs @@ -1,4 +1,6 @@ -use crate::{resources::transform_json_value, users::Tokened, workspaces::LargeFileStorage}; +use crate::{ + db::DB, resources::transform_json_value, users::Tokened, workspaces::LargeFileStorage, +}; use aws_sdk_s3::config::{Credentials, Region}; use axum::{ extract::Path, @@ -143,10 +145,11 @@ struct WindmillLargeFile { async fn test_connection( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, Tokened { token }: Tokened, Path(w_id): Path, ) -> error::JsonResult<()> { - let s3_resource_opt = get_workspace_s3_resource(&authed, &user_db, &token, &w_id).await?; + let s3_resource_opt = get_workspace_s3_resource(&authed, &user_db, &db, &token, &w_id).await?; if s3_resource_opt.is_none() { return Err(error::Error::NotFound( "No datasets storage resource defined at the workspace level".to_string(), @@ -168,10 +171,11 @@ async fn test_connection( async fn list_stored_datasets( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, Tokened { token }: Tokened, Path(w_id): Path, ) -> error::JsonResult { - let s3_resource_opt = get_workspace_s3_resource(&authed, &user_db, &token, &w_id).await?; + let s3_resource_opt = get_workspace_s3_resource(&authed, &user_db, &db, &token, &w_id).await?; let s3_resource = s3_resource_opt.unwrap(); let s3_client = build_s3_client(&s3_resource); @@ -223,6 +227,7 @@ async fn list_stored_datasets( async fn get_workspace_s3_resource<'c>( authed: &ApiAuthed, user_db: &UserDB, + db: &DB, token: &str, w_id: &str, ) -> error::Result> { @@ -261,6 +266,7 @@ async fn get_workspace_s3_resource<'c>( let interpolated_value = transform_json_value( authed, user_db, + db, &w_id, resource_path_json_value, &Option::None, diff --git a/backend/windmill-api/src/resources.rs b/backend/windmill-api/src/resources.rs index bda0e34d4b..01e04ad12e 100644 --- a/backend/windmill-api/src/resources.rs +++ b/backend/windmill-api/src/resources.rs @@ -247,6 +247,7 @@ async fn list_resources( async fn get_resource( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, Path((w_id, path)): Path<(String, StripPath)>, ) -> JsonResult { let path = path.to_path(); @@ -269,7 +270,9 @@ async fn get_resource( .fetch_optional(&mut *tx) .await?; tx.commit().await?; - + if resource_o.is_none() { + explain_resource_perm_error(&path, &w_id, &db).await?; + } let resource = not_found_if_none(resource_o, "Resource", path)?; Ok(Json(resource)) } @@ -295,6 +298,7 @@ async fn exists_resource( async fn get_resource_value( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, Path((w_id, path)): Path<(String, StripPath)>, ) -> JsonResult> { let path = path.to_path(); @@ -307,12 +311,55 @@ async fn get_resource_value( ) .fetch_optional(&mut *tx) .await?; + tx.commit().await?; + if value_o.is_none() { + explain_resource_perm_error(&path, &w_id, &db).await?; + } let value = not_found_if_none(value_o, "Resource", path)?; Ok(Json(value)) } +async fn explain_resource_perm_error( + path: &str, + w_id: &str, + db: &sqlx::Pool, +) -> windmill_common::error::Result<()> { + let extra_perms = sqlx::query_scalar!( + "SELECT extra_perms from resource WHERE path = $1 AND workspace_id = $2", + path, + w_id + ) + .fetch_optional(db) + .await? + .ok_or_else(|| Error::NotFound(format!("Resource {} not found", path)))?; + if path.starts_with("f/") { + let folder = path.split("/").nth(1).ok_or_else(|| { + Error::BadRequest(format!( + "path {} should have at least 2 components separated by /", + path + )) + })?; + let folder_extra_perms = sqlx::query_scalar!( + "SELECT extra_perms from folder WHERE name = $1 AND workspace_id = $2", + folder, + w_id + ) + .fetch_optional(db) + .await?; + return Err(Error::NotAuthorized(format!( + "Resource exists but you don't have access to it:\nresource perms: {}\nfolder perms: {}", + serde_json::to_string_pretty(&extra_perms).unwrap_or_default(), serde_json::to_string_pretty(&folder_extra_perms).unwrap_or_default() + ))); + } else { + return Err(Error::NotAuthorized(format!( + "Resource exists but you don't have access to it:\nresource perms: {}", + serde_json::to_string_pretty(&extra_perms).unwrap_or_default() + ))); + } +} + async fn custom_component( authed: ApiAuthed, Extension(user_db): Extension, @@ -347,6 +394,7 @@ struct JobInfo { async fn get_resource_value_interpolated( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, Tokened { token }: Tokened, Path((w_id, path)): Path<(String, StripPath)>, Query(job_info): Query, @@ -362,11 +410,23 @@ async fn get_resource_value_interpolated( .fetch_optional(&mut *tx) .await?; tx.commit().await?; + if value_o.is_none() { + explain_resource_perm_error(&path, &w_id, &db).await?; + } let value = not_found_if_none(value_o, "Resource", path)?; if let Some(value) = value { Ok(Json(Some( - transform_json_value(&authed, &user_db, &w_id, value, &job_info.job_id, &token).await?, + transform_json_value( + &authed, + &user_db, + &db, + &w_id, + value, + &job_info.job_id, + &token, + ) + .await?, ))) } else { Ok(Json(None)) @@ -379,6 +439,7 @@ use async_recursion::async_recursion; pub async fn transform_json_value<'c>( authed: &ApiAuthed, user_db: &UserDB, + db: &DB, workspace: &str, v: Value, job_id: &Option, @@ -388,8 +449,8 @@ pub async fn transform_json_value<'c>( Value::String(y) if y.starts_with("$var:") => { let path = y.strip_prefix("$var:").unwrap(); let tx: Transaction<'_, Postgres> = user_db.clone().begin(authed).await?; - let v = - crate::variables::get_value_internal(tx, workspace, path, &authed.username).await?; + let v = crate::variables::get_value_internal(tx, db, workspace, path, &authed.username) + .await?; Ok(Value::String(v)) } Value::String(y) if y.starts_with("$res:") => { @@ -408,7 +469,7 @@ pub async fn transform_json_value<'c>( tx.commit().await?; let v = not_found_if_none(v, "Resource", path)?; if let Some(v) = v { - transform_json_value(authed, user_db, workspace, v, job_id, token).await + transform_json_value(authed, user_db, db, workspace, v, job_id, token).await } else { Ok(Value::Null) } @@ -466,7 +527,7 @@ pub async fn transform_json_value<'c>( for (a, b) in m.clone().into_iter() { m.insert( a.clone(), - transform_json_value(authed, user_db, workspace, b, job_id, token).await?, + transform_json_value(authed, user_db, db, workspace, b, job_id, token).await?, ); } Ok(Value::Object(m)) diff --git a/backend/windmill-api/src/variables.rs b/backend/windmill-api/src/variables.rs index 99a8987cc9..c125934053 100644 --- a/backend/windmill-api/src/variables.rs +++ b/backend/windmill-api/src/variables.rs @@ -107,6 +107,7 @@ struct GetVariableQuery { async fn get_variable( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, Query(q): Query, Path((w_id, path)): Path<(String, StripPath)>, ) -> JsonResult { @@ -128,7 +129,12 @@ async fn get_variable( .fetch_optional(&mut *tx) .await?; - let variable = not_found_if_none(variable_o, "Variable", &path)?; + let variable = if let Some(variable) = variable_o { + variable + } else { + explain_variable_perm_error(&path, &w_id, &db).await?; + unreachable!() + }; let decrypt_secret = q.decrypt_secret.unwrap_or(true); @@ -172,17 +178,20 @@ async fn get_variable( async fn get_value( authed: ApiAuthed, Extension(user_db): Extension, + Extension(db): Extension, + Path((w_id, path)): Path<(String, StripPath)>, ) -> JsonResult { let path = path.to_path(); let tx = user_db.begin(&authed).await?; - return get_value_internal(tx, &w_id, &path, &authed.username) + return get_value_internal(tx, &db, &w_id, &path, &authed.username) .await .map(Json); } pub async fn get_value_internal<'c>( mut tx: Transaction<'c, Postgres>, + db: &DB, w_id: &str, path: &str, username: &str, @@ -194,7 +203,12 @@ pub async fn get_value_internal<'c>( .fetch_optional(&mut *tx) .await?; - let variable = not_found_if_none(variable_o, "Variable", &path)?; + let variable = if let Some(variable) = variable_o { + variable + } else { + explain_variable_perm_error(path, w_id, db).await?; + unreachable!() + }; let r = if variable.is_secret { audit_log( @@ -226,6 +240,45 @@ pub async fn get_value_internal<'c>( Ok(r) } +async fn explain_variable_perm_error( + path: &str, + w_id: &str, + db: &sqlx::Pool, +) -> windmill_common::error::Result<()> { + let extra_perms = sqlx::query_scalar!( + "SELECT extra_perms from variable WHERE path = $1 AND workspace_id = $2", + path, + w_id + ) + .fetch_optional(db) + .await? + .ok_or_else(|| Error::NotFound(format!("Variable {} not found", path)))?; + if path.starts_with("f/") { + let folder = path.split("/").nth(1).ok_or_else(|| { + Error::BadRequest(format!( + "path {} should have at least 2 components separated by /", + path + )) + })?; + let folder_extra_perms = sqlx::query_scalar!( + "SELECT extra_perms from folder WHERE name = $1 AND workspace_id = $2", + folder, + w_id + ) + .fetch_optional(db) + .await?; + return Err(Error::NotAuthorized(format!( + "Variable exists but you don't have access to it:\nvariable perms: {}\nfolder perms: {}", + serde_json::to_string_pretty(&extra_perms).unwrap_or_default(), serde_json::to_string_pretty(&folder_extra_perms).unwrap_or_default() + ))); + } else { + return Err(Error::NotAuthorized(format!( + "Variable exists but you don't have access to it:\nvariable perms: {}", + serde_json::to_string_pretty(&extra_perms).unwrap_or_default() + ))); + } +} + async fn exists_variable( Extension(db): Extension, Path((w_id, path)): Path<(String, StripPath)>, diff --git a/typescript-client/client.ts b/typescript-client/client.ts index 0e012f3b73..5d7de23625 100644 --- a/typescript-client/client.ts +++ b/typescript-client/client.ts @@ -80,7 +80,9 @@ export async function getResource( if (undefinedIfEmpty && e.status === 404) { return undefined; } else { - throw Error(`Resource not found at ${path} or not visible to you`); + throw Error( + `Resource not found at ${path} or not visible to you: ${e.body}` + ); } } } @@ -200,7 +202,9 @@ export async function getVariable(path: string): Promise { try { return await VariableService.getVariableValue({ workspace, path }); } catch (e: any) { - throw Error(`Variable not found at ${path} or not visible to you`); + throw Error( + `Variable not found at ${path} or not visible to you: ${e.body}` + ); } }