From bf57c3a628d78af18bcc4c4051e2425313d2d6f7 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 1 Aug 2023 17:02:23 +0200 Subject: [PATCH] fix: fix draft permissions (require writer instead of owner) --- backend/windmill-api/src/apps.rs | 12 +++ backend/windmill-api/src/drafts.rs | 22 ++++- backend/windmill-api/src/flows.rs | 12 +++ backend/windmill-api/src/scripts.rs | 14 +++ backend/windmill-api/src/users.rs | 86 +++++++++++++++++++ .../src/lib/components/GroupEditor.svelte | 6 +- 6 files changed, 150 insertions(+), 2 deletions(-) diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 8f755fbdaf..b26c7057a4 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -840,6 +840,18 @@ fn get_on_behalf_of(policy: &Policy) -> Result<(String, String)> { Ok((permissioned_as, email)) } +pub async fn require_is_writer(authed: &Authed, path: &str, w_id: &str, db: DB) -> Result<()> { + return crate::users::require_is_writer( + authed, + path, + w_id, + db, + "SELECT extra_perms FROM app WHERE path = $1 AND workspace_id = $2", + "app", + ) + .await; +} + async fn exists_app( Extension(db): Extension, Path((w_id, path)): Path<(String, StripPath)>, diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index d9dc563863..0220cf939d 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -42,6 +42,26 @@ pub struct Draft { pub typ: DraftType, } +pub async fn require_writer_of_path( + authed: &Authed, + path: &str, + w_id: &str, + db: DB, + kind: &DraftType, +) -> Result<()> { + if authed.is_admin { + return Ok(()); + } else if require_owner_of_path(authed, path).is_ok() { + return Ok(()); + } else { + match kind { + DraftType::Script => crate::scripts::require_is_writer(authed, path, w_id, db).await, + DraftType::Flow => crate::flows::require_is_writer(authed, path, w_id, db).await, + DraftType::App => crate::apps::require_is_writer(authed, path, w_id, db).await, + } + } +} + async fn create_draft( authed: Authed, Extension(db): Extension, @@ -53,7 +73,7 @@ async fn create_draft( let mut tx = user_db.begin(&authed).await?; - require_owner_of_path(&authed, &draft.path)?; + require_writer_of_path(&authed, &draft.path, &w_id, db, &draft.typ).await?; sqlx::query!( "INSERT INTO draft diff --git a/backend/windmill-api/src/flows.rs b/backend/windmill-api/src/flows.rs index efea8b3461..796b333b51 100644 --- a/backend/windmill-api/src/flows.rs +++ b/backend/windmill-api/src/flows.rs @@ -304,6 +304,18 @@ async fn check_schedule_conflict<'c>( Ok(()) } +pub async fn require_is_writer(authed: &Authed, path: &str, w_id: &str, db: DB) -> Result<()> { + return crate::users::require_is_writer( + authed, + path, + w_id, + db, + "SELECT extra_perms FROM flow WHERE path = $1 AND workspace_id = $2", + "flow", + ) + .await; +} + async fn update_flow( authed: Authed, Extension(user_db): Extension, diff --git a/backend/windmill-api/src/scripts.rs b/backend/windmill-api/src/scripts.rs index f21cd2671d..afea487f9b 100644 --- a/backend/windmill-api/src/scripts.rs +++ b/backend/windmill-api/src/scripts.rs @@ -738,6 +738,20 @@ async fn get_deployment_status( Ok(Json(status)) } +pub async fn require_is_writer(authed: &Authed, path: &str, w_id: &str, db: DB) -> Result<()> { + return crate::users::require_is_writer( + authed, + path, + w_id, + db, + "SELECT extra_perms FROM script WHERE path = $1 AND workspace_id = $2 \ + AND created_at = (SELECT max(created_at) FROM script WHERE path = $1 AND \ + workspace_id = $2)", + "script", + ) + .await; +} + async fn archive_script_by_path( authed: Authed, Extension(webhook): Extension, diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index e78506019c..431d84631a 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -1041,6 +1041,92 @@ pub fn require_owner_of_path(authed: &Authed, path: &str) -> Result<()> { } } +pub fn get_perm_in_extra_perms_for_authed(v: serde_json::Value, authed: &Authed) -> Option { + match v { + serde_json::Value::Object(obj) => { + let mut keys = vec![format!("u/{}", authed.username)]; + for g in authed.groups.iter() { + keys.push(format!("g/{}", g)); + } + let mut res = None; + for k in keys { + if let Some(v) = obj.get(&k) { + if let Some(v) = v.as_bool() { + if v { + return Some(true); + } + res = Some(v); + } + } + } + res + } + _ => None, + } +} + +pub async fn require_is_writer( + authed: &Authed, + path: &str, + w_id: &str, + db: DB, + query: &str, + kind: &str, +) -> Result<()> { + if authed.is_admin { + return Ok(()); + } + if !path.is_empty() { + if require_owner_of_path(authed, path).is_ok() { + return Ok(()); + } + if path.starts_with("f/") && path.split('/').count() >= 2 { + let folder = path.split('/').nth(1).unwrap(); + let extra_perms = sqlx::query_scalar!( + "SELECT extra_perms FROM folder WHERE name = $1 AND workspace_id = $2", + folder, + w_id + ) + .fetch_optional(&db) + .await?; + if let Some(perms) = extra_perms { + let is_folder_writer = + get_perm_in_extra_perms_for_authed(perms, authed).unwrap_or(false); + if is_folder_writer { + return Ok(()); + } + } + } + let extra_perms = sqlx::query_scalar(query) + .bind(path) + .bind(w_id) + .fetch_optional(&db) + .await?; + if let Some(perms) = extra_perms { + let perm = get_perm_in_extra_perms_for_authed(perms, authed); + match perm { + Some(true) => Ok(()), + Some(false) => Err(Error::BadRequest(format!( + "User {} is not a writer of {kind} path {path}", + authed.username + ))), + None => Err(Error::BadRequest(format!( + "User {} has neither read or write permission on {kind} {path}", + authed.username + ))), + } + } else { + Err(Error::BadRequest(format!( + "{path} does not exist yet and user {} is not an owner of the parent folder", + authed.username + ))) + } + } else { + Err(Error::BadRequest(format!( + "Cannot be writer of an empty path" + ))) + } +} async fn whois( Extension(db): Extension, Path((w_id, username)): Path<(String, String)>, diff --git a/frontend/src/lib/components/GroupEditor.svelte b/frontend/src/lib/components/GroupEditor.svelte index 191a9a87c9..913f75fed4 100644 --- a/frontend/src/lib/components/GroupEditor.svelte +++ b/frontend/src/lib/components/GroupEditor.svelte @@ -53,7 +53,11 @@ } async function loadInstanceGroup(): Promise { - instance_group = await GroupService.getInstanceGroup({ name }) + try { + instance_group = await GroupService.getInstanceGroup({ name }) + } catch (e) { + instance_group = undefined + } } async function loadGroup(): Promise {