fix: tighten delete permissions

This commit is contained in:
Ruben Fiszel
2024-04-16 10:41:08 +02:00
parent 86869ff61a
commit 0a9e1326fe
5 changed files with 262 additions and 14 deletions
@@ -0,0 +1 @@
-- Add down migration script here
@@ -0,0 +1,228 @@
DO
$do$
DECLARE
i text;
arr text[] := array['resource', 'script', 'variable', 'schedule', 'flow', 'app', 'raw_app'];
BEGIN
FOREACH i IN ARRAY arr
LOOP
EXECUTE FORMAT(
$$
DROP POLICY IF EXISTS see_folder_extra_perms_user ON %1$I;
DROP POLICY IF EXISTS see_folder_extra_perms_user_delete ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_user ON %1$I;
DROP POLICY IF EXISTS see_member ON %1$I;
DROP POLICY IF EXISTS see_own ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_user_delete ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_groups ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_groups_delete ON %1$I;
-- New policies for select, insert, update
DROP POLICY IF EXISTS see_folder_extra_perms_user_select ON %1$I;
DROP POLICY IF EXISTS see_folder_extra_perms_user_insert ON %1$I;
DROP POLICY IF EXISTS see_folder_extra_perms_user_update ON %1$I;
DROP POLICY IF EXISTS see_own ON %1$I;
DROP POLICY IF EXISTS see_member ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_user_select ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_user_insert ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_user_update ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_groups_select ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_groups_insert ON %1$I;
DROP POLICY IF EXISTS see_extra_perms_groups_update ON %1$I;
-- Folder permissions split into select, insert, and update
CREATE POLICY see_folder_extra_perms_user_select ON %1$I FOR SELECT TO windmill_user
USING (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_read'), ',')::text[]));
CREATE POLICY see_folder_extra_perms_user_insert ON %1$I FOR INSERT TO windmill_user
WITH CHECK (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_write'), ',')::text[]));
CREATE POLICY see_folder_extra_perms_user_update ON %1$I FOR UPDATE TO windmill_user
USING (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_write'), ',')::text[]));
CREATE POLICY see_folder_extra_perms_user_delete ON %1$I FOR UPDATE TO windmill_user
USING (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_write'), ',')::text[]));
CREATE POLICY see_own ON %1$I FOR ALL TO windmill_user
USING (SPLIT_PART(%1$I.path, '/', 1) = 'u' AND SPLIT_PART(%1$I.path, '/', 2) = current_setting('session.user'));
CREATE POLICY see_member ON %1$I FOR ALL TO windmill_user
USING (SPLIT_PART(%1$I.path, '/', 1) = 'g' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.groups'), ',')::text[]));
CREATE POLICY see_extra_perms_user_select ON %1$I FOR SELECT TO windmill_user
USING (extra_perms ? CONCAT('u/', current_setting('session.user')));
CREATE POLICY see_extra_perms_user_insert ON %1$I FOR INSERT TO windmill_user
WITH CHECK ((extra_perms ->> CONCAT('u/', current_setting('session.user')))::boolean);
CREATE POLICY see_extra_perms_user_update ON %1$I FOR UPDATE TO windmill_user
USING ((extra_perms ->> CONCAT('u/', current_setting('session.user')))::boolean);
CREATE POLICY see_extra_perms_user_delete ON %1$I FOR DELETE TO windmill_user
USING ((extra_perms ->> CONCAT('u/', current_setting('session.user')))::boolean);
CREATE POLICY see_extra_perms_groups_select ON %1$I FOR SELECT TO windmill_user
USING (extra_perms ?| regexp_split_to_array(current_setting('session.pgroups'), ',')::text[]);
CREATE POLICY see_extra_perms_groups_insert ON %1$I FOR INSERT TO windmill_user
WITH CHECK (exists(
SELECT key, value FROM jsonb_each_text(extra_perms)
WHERE SPLIT_PART(key, '/', 1) = 'g' AND key = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])
AND value::boolean));
CREATE POLICY see_extra_perms_groups_update ON %1$I FOR UPDATE TO windmill_user
USING (exists(
SELECT key, value FROM jsonb_each_text(extra_perms)
WHERE SPLIT_PART(key, '/', 1) = 'g' AND key = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])
AND value::boolean));
CREATE POLICY see_extra_perms_groups_delete ON %1$I FOR DELETE TO windmill_user
USING (exists(
SELECT key, value FROM jsonb_each_text(extra_perms)
WHERE SPLIT_PART(key, '/', 1) = 'g' AND key = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])
AND value::boolean));
$$,
i
);
END LOOP;
END
$do$;
DROP POLICY IF EXISTS see_extra_perms_user ON folder;
DROP POLICY IF EXISTS see_extra_perms_user_select ON folder;
DROP POLICY IF EXISTS see_extra_perms_user_insert ON folder;
DROP POLICY IF EXISTS see_extra_perms_user_update ON folder;
DROP POLICY IF EXISTS see_extra_perms_user_delete ON folder;
DROP POLICY IF EXISTS see_extra_perms_groups ON folder;
DROP POLICY IF EXISTS see_extra_perms_groups_select ON folder;
DROP POLICY IF EXISTS see_extra_perms_groups_insert ON folder;
DROP POLICY IF EXISTS see_extra_perms_groups_update ON folder;
DROP POLICY IF EXISTS see_extra_perms_groups_delete ON folder;
-- Existing CREATE POLICY statements updated to reflect policy splitting for 'folder' table
CREATE POLICY see_extra_perms_user_select ON folder FOR SELECT TO windmill_user
USING (extra_perms ? CONCAT('u/', current_setting('session.user')) OR CONCAT('u/', current_setting('session.user')) = ANY(owners));
CREATE POLICY see_extra_perms_user_insert ON folder FOR INSERT TO windmill_user
WITH CHECK ((CONCAT('u/', current_setting('session.user')) = ANY(owners)));
CREATE POLICY see_extra_perms_user_update ON folder FOR UPDATE TO windmill_user
USING ((CONCAT('u/', current_setting('session.user')) = ANY(owners)));
CREATE POLICY see_extra_perms_user_delete ON folder FOR DELETE TO windmill_user
USING ((CONCAT('u/', current_setting('session.user')) = ANY(owners)));
CREATE POLICY see_extra_perms_groups_select ON folder FOR SELECT TO windmill_user
USING (extra_perms ?| regexp_split_to_array(current_setting('session.pgroups'), ',')::text[] OR EXISTS (
SELECT o FROM unnest(owners) AS o
WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])));
CREATE POLICY see_extra_perms_groups_insert ON folder FOR INSERT TO windmill_user
WITH CHECK (EXISTS (
SELECT o FROM unnest(owners) AS o
WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])));
CREATE POLICY see_extra_perms_groups_update ON folder FOR UPDATE TO windmill_user
USING (EXISTS (
SELECT o FROM unnest(owners) AS o
WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])));
CREATE POLICY see_extra_perms_groups_delete ON folder FOR DELETE TO windmill_user
USING (EXISTS (
SELECT o FROM unnest(owners) AS o
WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])));
-- DO
-- $do$
-- DECLARE
-- i text;
-- arr text[] := array['resource', 'script', 'variable', 'schedule', 'flow', 'app', 'raw_app'];
-- BEGIN
-- FOREACH i IN ARRAY arr
-- LOOP
-- EXECUTE FORMAT(
-- $$
-- CREATE POLICY see_folder_extra_perms_user ON %1$I FOR ALL TO windmill_user
-- USING (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_read'), ',')::text[]))
-- WITH CHECK (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_write'), ',')::text[]));
-- CREATE POLICY see_folder_extra_perms_user_delete ON %1$I AS RESTRICTIVE FOR DELETE TO windmill_user
-- USING (SPLIT_PART(%1$I.path, '/', 1) = 'f' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.folders_write'), ',')::text[]));
-- CREATE POLICY see_own ON %1$I FOR ALL TO windmill_user
-- USING (SPLIT_PART(%1$I.path, '/', 1) = 'u' AND SPLIT_PART(%1$I.path, '/', 2) = current_setting('session.user'));
-- CREATE POLICY see_member ON %1$I FOR ALL TO windmill_user
-- USING (SPLIT_PART(%1$I.path, '/', 1) = 'g' AND SPLIT_PART(%1$I.path, '/', 2) = any(regexp_split_to_array(current_setting('session.groups'), ',')::text[]));
-- CREATE POLICY see_extra_perms_user ON %1$I FOR ALL TO windmill_user
-- USING (extra_perms ? CONCAT('u/', current_setting('session.user')))
-- WITH CHECK ((extra_perms ->> CONCAT('u/', current_setting('session.user')))::boolean);
-- CREATE POLICY see_extra_perms_user_delete ON %1$I FOR DELETE TO windmill_user
-- USING ((extra_perms ->> CONCAT('u/', current_setting('session.user')))::boolean);
-- CREATE POLICY see_extra_perms_groups ON %1$I FOR ALL TO windmill_user
-- USING (extra_perms ?| regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])
-- WITH CHECK (exists(
-- SELECT key, value FROM jsonb_each_text(extra_perms)
-- WHERE SPLIT_PART(key, '/', 1) = 'g' AND key = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])
-- AND value::boolean));
-- CREATE POLICY see_extra_perms_groups_delete ON %1$I FOR DELETE TO windmill_user
-- USING (exists(
-- SELECT key, value FROM jsonb_each_text(extra_perms)
-- WHERE SPLIT_PART(key, '/', 1) = 'g' AND key = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])
-- AND value::boolean));
-- $$,
-- i
-- );
-- END LOOP;
-- END
-- $do$;
-- DROP POLICY see_extra_perms_user ON folder;
-- DROP POLICY see_extra_perms_groups ON folder;
-- CREATE POLICY see_extra_perms_user ON folder FOR ALL to windmill_user
-- USING (extra_perms ? CONCAT('u/', current_setting('session.user')) or (CONCAT('u/', current_setting('session.user')) = ANY(owners)))
-- WITH CHECK ((CONCAT('u/', current_setting('session.user')) = ANY(owners)));
-- DROP POLICY IF EXISTS see_extra_perms_user_delete ON folder;
-- CREATE POLICY see_extra_perms_user_delete ON folder AS RESTRICTIVE FOR DELETE to windmill_user
-- USING ((CONCAT('u/', current_setting('session.user')) = ANY(owners)));
-- CREATE POLICY see_extra_perms_groups ON folder FOR ALL to windmill_user
-- USING (extra_perms ?| regexp_split_to_array(current_setting('session.pgroups'), ',')::text[] or (exists(
-- SELECT o FROM unnest(owners) as o
-- WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[]))))
-- WITH CHECK (exists(
-- SELECT o FROM unnest(owners) as o
-- WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])));
-- DROP POLICY IF EXISTS see_extra_perms_groups_delete ON folder;
-- CREATE POLICY see_extra_perms_groups_delete ON folder AS RESTRICTIVE FOR DELETE to windmill_user
-- USING (exists(
-- SELECT o FROM unnest(owners) as o
-- WHERE o = ANY(regexp_split_to_array(current_setting('session.pgroups'), ',')::text[])));
+13 -5
View File
@@ -477,15 +477,23 @@ async fn delete_folder(
let mut tx = user_db.begin(&authed).await?;
not_found_if_none(get_folderopt(&mut tx, &w_id, &name).await?, "Folder", &name)?;
require_is_owner(&authed, &name)?;
sqlx::query!(
"DELETE FROM folder WHERE name = $1 AND workspace_id = $2",
let del = sqlx::query_scalar!(
"DELETE FROM folder WHERE name = $1 AND workspace_id = $2 RETURNING 1",
name,
w_id
)
.execute(&mut *tx)
.await?;
.fetch_optional(&mut *tx)
.await?
.flatten();
if del.is_none() {
return Err(windmill_common::error::Error::NotAuthorized(format!(
"Not authorized to delete folder {}",
name
)));
}
audit_log(
&mut *tx,
&authed.username,
+1 -1
View File
@@ -579,7 +579,7 @@ export function canWrite(
if (user.pgroups.findIndex((x) => keys.includes(x) && extra_perms[x]) != -1) {
return true
}
if (user.folders.findIndex((x) => path.startsWith('f/' + x)) != -1) {
if (user.folders.findIndex((x) => path.startsWith('f/' + x + '/') && user.folders[x]) != -1) {
return true
}
@@ -10,7 +10,7 @@
import { Button, Drawer, DrawerContent, Popup, Skeleton } from '$lib/components/common'
import FolderInfo from '$lib/components/FolderInfo.svelte'
import FolderUsageInfo from '$lib/components/FolderUsageInfo.svelte'
import { canWrite } from '$lib/utils'
import { sendUserToast } from '$lib/utils'
import DataTable from '$lib/components/table/DataTable.svelte'
import Cell from '$lib/components/table/Cell.svelte'
import { Pen, Trash, Plus } from 'lucide-svelte'
@@ -25,7 +25,14 @@
async function loadFolders(): Promise<void> {
folders = (await FolderService.listFolders({ workspace: $workspaceStore! })).map((x) => {
return { canWrite: canWrite('f/' + x.name, x.extra_perms ?? {}, $userStore), ...x }
return {
canWrite:
$userStore != undefined &&
($userStore?.is_admin ||
$userStore?.is_super_admin ||
$userStore?.folders_owners.includes(x.name)),
...x
}
})
}
@@ -170,16 +177,20 @@
}
},
{
displayName: 'Delete',
displayName: `Delete${canWrite ? '' : ' (require owner permissions)'}`,
icon: Trash,
type: 'delete',
disabled: !canWrite,
action: async () => {
await FolderService.deleteFolder({
workspace: $workspaceStore ?? '',
name
})
loadFolders()
try {
await FolderService.deleteFolder({
workspace: $workspaceStore ?? '',
name
})
} catch (e) {
sendUserToast(e.body, true)
loadFolders()
}
}
}
]}