starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-20 00:02:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: improve error message for unauthorized variables/resources

Browse Source
This commit is contained in:
Ruben Fiszel
2023-11-18 16:27:09 +01:00
parent ce08b21133
commit efac70453e
7 changed files with 207 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231.json
Generated
+23
View File
@@ -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"
}
backend/.sqlx/query-7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54.json
Generated
+23
View File
@@ -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"
}
backend/.sqlx/query-a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419.json
Generated
+23
View File
@@ -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"
}
backend/windmill-api/src/job_helpers.rs
+9 -3
View File
@@ -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<UserDB>,
Extension(db): Extension<DB>,
Tokened { token }: Tokened,
Path(w_id): Path<String>,
) -> 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<UserDB>,
Extension(db): Extension<DB>,
Tokened { token }: Tokened,
Path(w_id): Path<String>,
) -> error::JsonResult<ListStoredDatasetsResponse> {
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<Option<S3Resource>> {
@@ -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,
backend/windmill-api/src/resources.rs
+67 -6
View File
@@ -247,6 +247,7 @@ async fn list_resources(
async fn get_resource(
authed: ApiAuthed,
Extension(user_db): Extension<UserDB>,
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
) -> JsonResult<ListableResource> {
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<UserDB>,
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
) -> JsonResult<Option<serde_json::Value>> {
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<Postgres>,
) -> 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<UserDB>,
@@ -347,6 +394,7 @@ struct JobInfo {
async fn get_resource_value_interpolated(
authed: ApiAuthed,
Extension(user_db): Extension<UserDB>,
Extension(db): Extension<DB>,
Tokened { token }: Tokened,
Path((w_id, path)): Path<(String, StripPath)>,
Query(job_info): Query<JobInfo>,
@@ -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<Uuid>,
@@ -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))
backend/windmill-api/src/variables.rs
+56 -3
View File
@@ -107,6 +107,7 @@ struct GetVariableQuery {
async fn get_variable(
authed: ApiAuthed,
Extension(user_db): Extension<UserDB>,
Extension(db): Extension<DB>,
Query(q): Query<GetVariableQuery>,
Path((w_id, path)): Path<(String, StripPath)>,
) -> JsonResult<ListableVariable> {
@@ -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<UserDB>,
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
) -> JsonResult<String> {
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<Postgres>,
) -> 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<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
typescript-client/client.ts
+6 -2
View File
@@ -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<string> {
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}`
);
}
}