From 39e7fc6c28cc13aba0bb3a52fe651b85cbcd0c36 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 2 Jan 2023 18:53:10 +0100 Subject: [PATCH] tighten public apps security using triggerable policy --- backend/sqlx-data.json | 139 ++++++++++-------- backend/windmill-api/openapi.yaml | 2 + backend/windmill-api/src/apps.rs | 2 +- backend/windmill-api/src/jobs.rs | 2 +- backend/windmill-api/src/resources.rs | 7 +- .../src/lib/components/TestJobLoader.svelte | 12 +- .../helpers/RunnableComponent.svelte | 115 +++++---------- .../components/apps/editor/AppEditor.svelte | 5 +- .../apps/editor/AppEditorHeader.svelte | 53 +++++-- .../componentsPanel/ComponentList.svelte | 6 +- frontend/src/lib/logout.ts | 1 - frontend/src/lib/utils.ts | 4 +- .../(root)/(logged)/resources/+page.svelte | 2 +- .../(root)/(logged)/schedules/+page.svelte | 2 +- 14 files changed, 180 insertions(+), 172 deletions(-) diff --git a/backend/sqlx-data.json b/backend/sqlx-data.json index c12ced4e42..dd54e84820 100644 --- a/backend/sqlx-data.json +++ b/backend/sqlx-data.json @@ -317,6 +317,45 @@ }, "query": "DELETE FROM account WHERE id = $1 AND workspace_id = $2" }, + "0f6af40e79a3f44ce84bd127a8af9216c82e71acf8bf4df12b078020c90d7d9d": { + "describe": { + "columns": [ + { + "name": "workspace_id", + "ordinal": 0, + "type_info": "Varchar" + }, + { + "name": "name", + "ordinal": 1, + "type_info": "Varchar" + }, + { + "name": "schema", + "ordinal": 2, + "type_info": "Jsonb" + }, + { + "name": "description", + "ordinal": 3, + "type_info": "Text" + } + ], + "nullable": [ + false, + false, + true, + true + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT * from resource_type WHERE name = $1 AND (workspace_id = $2 OR workspace_id = 'starter' OR workspace_id = 'admins')" + }, "11b1586acdfc180c5a077861ee1f7201fcbcec9d0ebada464f9d952c9c3e400d": { "describe": { "columns": [], @@ -1839,6 +1878,27 @@ }, "query": "SELECT is_flow FROM schedule WHERE path = $1 AND workspace_id = $2" }, + "5767e6a8f94f571a78bed0c0423882060c7d417fdabb30cff6c86cb1c5f46df9": { + "describe": { + "columns": [ + { + "name": "exists", + "ordinal": 0, + "type_info": "Bool" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT EXISTS(SELECT 1 FROM resource_type WHERE name = $1 AND (workspace_id = $2 OR workspace_id = 'starter' OR workspace_id = 'admins'))" + }, "576b00c515ee7cbf628b1881596d8a03c1c506b309e39a0048a3a8fe18f37070": { "describe": { "columns": [], @@ -1873,45 +1933,6 @@ }, "query": "SELECT EXISTS(SELECT 1 FROM app WHERE path = $1 AND workspace_id = $2)" }, - "58efbf34ba014b4853ef20ae400929b57c1d9de4262189274badf100d29e0649": { - "describe": { - "columns": [ - { - "name": "workspace_id", - "ordinal": 0, - "type_info": "Varchar" - }, - { - "name": "name", - "ordinal": 1, - "type_info": "Varchar" - }, - { - "name": "schema", - "ordinal": 2, - "type_info": "Jsonb" - }, - { - "name": "description", - "ordinal": 3, - "type_info": "Text" - } - ], - "nullable": [ - false, - false, - true, - true - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - } - }, - "query": "SELECT * from resource_type WHERE name = $1 AND (workspace_id = $2 OR workspace_id = 'starter')" - }, "59fc51efa01e823ec63f55a8081f282db0d7a40716c04f4043925eb84fe4429b": { "describe": { "columns": [], @@ -2108,26 +2129,6 @@ }, "query": "SELECT EXISTS(SELECT 1 FROM usr WHERE username = $1 AND workspace_id = $2)" }, - "63c4b9320681fac84ea92c25c0f6da5c9ac154dfccf575cea8145692246205c4": { - "describe": { - "columns": [ - { - "name": "name", - "ordinal": 0, - "type_info": "Varchar" - } - ], - "nullable": [ - false - ], - "parameters": { - "Left": [ - "Text" - ] - } - }, - "query": "SELECT name from resource_type WHERE (workspace_id = $1 OR workspace_id = 'starter') ORDER BY name" - }, "63f330e98051ae9d0cf5617a553f769cc98b05fdc8643839b504023b26f38aab": { "describe": { "columns": [], @@ -5114,6 +5115,26 @@ }, "query": "INSERT INTO usr_to_group\n VALUES ($1, 'all', $2)" }, + "e3fbad80527771f63390fb1ab203bd3d0c058627b92f32f5c6f3e2856ab24e47": { + "describe": { + "columns": [ + { + "name": "name", + "ordinal": 0, + "type_info": "Varchar" + } + ], + "nullable": [ + false + ], + "parameters": { + "Left": [ + "Text" + ] + } + }, + "query": "SELECT name from resource_type WHERE (workspace_id = $1 OR workspace_id = 'starter' OR workspace_id = 'admins') ORDER BY name" + }, "e5231634fbb37d45e5cf3e5dfb39cb829d224caad1c3ca1712d0fc4c495aa4fc": { "describe": { "columns": [ diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 741cd5dea5..c8387b7fdc 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -5399,6 +5399,8 @@ components: enum: [viewer, publisher, anonymous] on_behalf_of: type: string + on_behalf_of_email: + type: string ListableApp: type: object diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index afb7e5c97c..d5a99263d6 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -631,7 +631,7 @@ fn get_on_behalf_of(policy: &Policy) -> Result<(String, String)> { .as_ref() .ok_or_else(|| { Error::BadRequest( - "on_behalf_of is missing in the app policy and is required for anonymous execution" + "on_behalf_of_email is missing in the app policy and is required for anonymous execution" .to_string(), ) })? diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index e61b5cc5a9..03adbee7d1 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -62,7 +62,6 @@ pub fn workspaced_service() -> Router { .route("/completed/get/:id", get(get_completed_job)) .route("/completed/get_result/:id", get(get_completed_job_result)) .route("/completed/delete/:id", post(delete_completed_job)) - .route("/get/:id", get(get_job)) .route("/flow/resume/:id", post(resume_suspended_flow_as_owner)) .route("/getupdate/:id", get(get_job_update)) .route( @@ -95,6 +94,7 @@ pub fn global_service() -> Router { "/get_flow/:job_id/:resume_id/:secret", get(get_suspended_job_flow), ) + .route("/get/:id", get(get_job)) } async fn get_result_by_id( diff --git a/backend/windmill-api/src/resources.rs b/backend/windmill-api/src/resources.rs index 5219d632f8..b41daad60f 100644 --- a/backend/windmill-api/src/resources.rs +++ b/backend/windmill-api/src/resources.rs @@ -461,7 +461,7 @@ async fn list_resource_types_names( Path(w_id): Path, ) -> JsonResult> { let rows = sqlx::query_scalar!( - "SELECT name from resource_type WHERE (workspace_id = $1 OR workspace_id = 'starter') \ + "SELECT name from resource_type WHERE (workspace_id = $1 OR workspace_id = 'starter' OR workspace_id = 'admins') \ ORDER BY name", &w_id ) @@ -481,7 +481,7 @@ async fn get_resource_type( let resource_type_o = sqlx::query_as!( ResourceType, "SELECT * from resource_type WHERE name = $1 AND (workspace_id = $2 OR workspace_id = \ - 'starter')", + 'starter' OR workspace_id = 'admins')", &name, &w_id ) @@ -498,7 +498,8 @@ async fn exists_resource_type( Path((w_id, name)): Path<(String, String)>, ) -> JsonResult { let exists = sqlx::query_scalar!( - "SELECT EXISTS(SELECT 1 FROM resource_type WHERE name = $1 AND workspace_id = $2)", + "SELECT EXISTS(SELECT 1 FROM resource_type WHERE name = $1 AND (workspace_id = $2 OR workspace_id = \ + 'starter' OR workspace_id = 'admins'))", name, w_id ) diff --git a/frontend/src/lib/components/TestJobLoader.svelte b/frontend/src/lib/components/TestJobLoader.svelte index ad924ef93d..59f37ec6ef 100644 --- a/frontend/src/lib/components/TestJobLoader.svelte +++ b/frontend/src/lib/components/TestJobLoader.svelte @@ -108,11 +108,13 @@ intervalId = undefined stopCurrentIteration = true if (isLoading && job) { - await JobService.cancelQueuedJob({ - workspace: workspace!, - id: job.id, - requestBody: {} - }) + try { + await JobService.cancelQueuedJob({ + workspace: workspace!, + id: job.id, + requestBody: {} + }) + } catch {} } await clearIntervalAsync(interval) } diff --git a/frontend/src/lib/components/apps/components/helpers/RunnableComponent.svelte b/frontend/src/lib/components/apps/components/helpers/RunnableComponent.svelte index 461f36135f..4e0d9217d6 100644 --- a/frontend/src/lib/components/apps/components/helpers/RunnableComponent.svelte +++ b/frontend/src/lib/components/apps/components/helpers/RunnableComponent.svelte @@ -11,7 +11,6 @@ import type { AppEditorContext } from '../../types' import { fieldTypeToTsType, schemaToInputsSpec } from '../../utils' import InputValue from './InputValue.svelte' - import MissingConnectionWarning from './MissingConnectionWarning.svelte' import RefreshButton from './RefreshButton.svelte' // Component props @@ -23,7 +22,7 @@ export let result: any = undefined export let forceSchemaDisplay: boolean = false - const { worldStore, runnableComponents, workspace, appPath } = + const { worldStore, runnableComponents, workspace, appPath, mode } = getContext('AppEditorContext') onMount(() => { @@ -36,65 +35,19 @@ }) let args: Record = {} - let debouncedArgs: Record = args let testIsLoading = false let runnableInputValues: Record = {} - let argsTimeout: NodeJS.Timeout | undefined + let executeTimeout: NodeJS.Timeout | undefined = undefined - function setDebouncedArgs() { - argsTimeout && clearTimeout(argsTimeout) - argsTimeout = setTimeout(() => { - Object.assign(args, debouncedArgs) - args = args + function setDebouncedExecute() { + executeTimeout && clearTimeout(executeTimeout) + executeTimeout = setTimeout(() => { + executeComponent() }, 200) } - $: debouncedArgs && setDebouncedArgs() - let previousStaticArgs: any = {} - function setStaticInputsToArgs() { - let nargs = {} - Object.entries(fields ?? {}).forEach(([key, value]) => { - if (value.type === 'static') { - nargs[key] = value.value - } - }) - - if (JSON.stringify(previousStaticArgs) != JSON.stringify(nargs)) { - previousStaticArgs = nargs - Object.assign(args, nargs) - args = args - } - } - - $: fields && setStaticInputsToArgs() - - function argMergedArgsValid(mergedArgs: Record, testJobLoader) { - if (!fields) { - return false - } - - if ( - Object.keys(fields).length !== - Object.keys(mergedArgs).length - Object.keys(extraQueryParams).length - ) { - return false - } - - const areAllArgsValid = Object.values(mergedArgs).every( - (arg) => arg !== undefined && arg !== null - ) - - if (areAllArgsValid && autoRefresh && testJobLoader) { - executeComponent() - } - - return areAllArgsValid - } - - $: isValid = - Object.keys(fields ?? {}).length == 0 || - argMergedArgsValid({ ...extraQueryParams, ...runnableInputValues, ...args }, testJobLoader) + $: fields && runnableInputValues && args && autoRefresh && testJobLoader && setDebouncedExecute() // Test job internal state let testJob: CompletedJob | undefined = undefined @@ -109,6 +62,8 @@ outputs.loading.set(false, true) } + $: outputs?.loading?.set(testIsLoading) + $: runnable?.type === 'runnableByName' && loadSchemaAndInputsByName() async function loadSchemaAndInputsByName() { @@ -132,10 +87,10 @@ // When the schema is loaded, we need to update the inputs spec // in order to render the inputs the component panel function reloadInputs(schema: Schema) { - let schemaWithoutExtraQueries: Schema = JSON.parse(JSON.stringify(schema)) + let schemaCopy: Schema = JSON.parse(JSON.stringify(schema)) const result = {} - const newInputs = schemaToInputsSpec(schemaWithoutExtraQueries) + const newInputs = schemaToInputsSpec(schemaCopy) if (!fields) { return newInputs } @@ -207,9 +162,19 @@ outputs?.loading?.set(true) await testJobLoader?.abstractRun(() => { + const nonStaticRunnableInputs = {} + const staticRunnableInputs = {} + Object.keys(fields ?? {}).forEach(([k, v]) => { + let field = fields[k] + if (field.type == 'static') { + staticRunnableInputs[k] = field.value + } else { + nonStaticRunnableInputs[k] = runnableInputValues[k] + } + }, {}) const requestBody = { - args: { ...args, ...runnableInputValues }, - force_viewer_static_fields: {} + args: { ...nonStaticRunnableInputs, ...args }, + force_viewer_static_fields: $mode == 'preview' ? undefined : staticRunnableInputs } if (runnable?.type === 'runnableByName') { @@ -240,13 +205,15 @@ } -{#each Object.keys(fields ?? {}) as key} - +{#each Object.entries(fields ?? {}) as [key, v]} + {#if v.type != 'static' && v.type != 'user'} + + {/if} {/each} { if (testJob && outputs) { outputs.result?.set(testJob?.result) - outputs.loading?.set(false) result = testJob.result } }} @@ -268,8 +234,7 @@
- {#if isValid} - - {:else} - - Please fill in all the inputs - - {#each Object.keys(fields ?? {}) as key} - {#if fields[key].type === 'connected'} - - {/if} - {/each} - - {/if} + {:else} {/if} diff --git a/frontend/src/lib/components/apps/editor/AppEditor.svelte b/frontend/src/lib/components/apps/editor/AppEditor.svelte index 97c4209e86..948b4ee92d 100644 --- a/frontend/src/lib/components/apps/editor/AppEditor.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditor.svelte @@ -74,7 +74,6 @@ $: $appStore && saveDraft() function saveDraft() { - console.log('save') timeout && clearTimeout(timeout) timeout = setTimeout(() => localStorage.setItem('app', encodeState($appStore)), 500) } @@ -121,7 +120,9 @@
{#if $appStore.grid}
diff --git a/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte b/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte index 5f67351c6b..b24d8885d9 100644 --- a/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditorHeader.svelte @@ -12,13 +12,21 @@ import { AppService, Policy } from '$lib/gen' import { userStore, workspaceStore } from '$lib/stores' import { faClipboard, faExternalLink, faSave } from '@fortawesome/free-solid-svg-icons' - import { Eye, Laptop2, Pencil, PenTool, Smartphone } from 'lucide-svelte' + import { Eye, Laptop2, Pencil, Smartphone } from 'lucide-svelte' import { getContext } from 'svelte' import { Icon } from 'svelte-awesome' import { copyToClipboard, sendUserToast } from '../../../utils' - import type { AppEditorContext, EditorBreakpoint, EditorMode } from '../types' + import type { AppComponent, AppEditorContext } from '../types' import AppExportButton from './AppExportButton.svelte' + async function hash(message) { + const msgUint8 = new TextEncoder().encode(message) // encode as (utf-8) Uint8Array + const hashBuffer = await crypto.subtle.digest('SHA-256', msgUint8) // hash the message + const hashArray = Array.from(new Uint8Array(hashBuffer)) // convert buffer to byte array + const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('') // convert bytes to hex string + return hashHex + } + export let policy: Policy const { app, summary, mode, breakpoint, appPath } = @@ -38,12 +46,35 @@ saveDrawerOpen = false } + async function computeTriggerables() { + const allTriggers = await Promise.all( + $app.grid.map(async (x) => { + let c = x.data as AppComponent + if (c.componentInput?.type == 'runnable') { + const staticInputs = Object.fromEntries( + Object.entries(c.componentInput.fields ?? {}) + .filter(([k, v]) => v.type == 'static') + .map(([k, v]) => { + return [k, v['value']] + }) + ) + if (c.componentInput.runnable?.type == 'runnableByName') { + let hex = await hash(c.componentInput.runnable.inlineScript?.content) + console.log(staticInputs) + return [`rawscript/${hex}`, staticInputs] + } else if (c.componentInput.runnable?.type == 'runnableByPath') { + return [c.componentInput.runnable.path, staticInputs] + } + } + return [] + }) + ) + policy.triggerables = Object.fromEntries(allTriggers) + policy.on_behalf_of = `u/${$userStore?.username}` + policy.on_behalf_of_email = $userStore?.email + } async function createApp(path: string) { - const policy = { - triggerables: {}, - execution_mode: Policy.execution_mode.PUBLISHER, - on_behalf_of: `u/${$userStore?.username}` - } + await computeTriggerables() try { const appId = await AppService.createApp({ workspace: $workspaceStore!, @@ -77,6 +108,7 @@ path: appPath, requestBody: { policy } }) + console.log(policy) } async function save() { @@ -86,17 +118,14 @@ return } loading.save = true + await computeTriggerables() await AppService.updateApp({ workspace: $workspaceStore!, path: $page.params.path, requestBody: { value: $app!, summary: $summary, - policy: { - triggerables: {}, - execution_mode: Policy.execution_mode.PUBLISHER, - on_behalf_of: `u/${$userStore?.username}` - } + policy } }) loading.save = false diff --git a/frontend/src/lib/components/apps/editor/componentsPanel/ComponentList.svelte b/frontend/src/lib/components/apps/editor/componentsPanel/ComponentList.svelte index 43959c5b74..6a7c5416ba 100644 --- a/frontend/src/lib/components/apps/editor/componentsPanel/ComponentList.svelte +++ b/frontend/src/lib/components/apps/editor/componentsPanel/ComponentList.svelte @@ -19,7 +19,7 @@ function getMinDimensionsByComponent(componentType: AppComponent['type'], column: number): Size { // Dimensions key formula: :-: const dimensions: Record<`${number}:${number}-${number}:${number}`, AppComponent['type'][]> = { - '1:2-2:2': [ + '4:2-4:2': [ 'buttoncomponent', 'textcomponent', 'checkboxcomponent', @@ -29,7 +29,7 @@ 'passwordinputcomponent', 'dateinputcomponent' ], - '2:12-4:12': ['barchartcomponent', 'piechartcomponent', 'formcomponent', 'displaycomponent'], + '4:12-4:12': ['barchartcomponent', 'piechartcomponent', 'formcomponent', 'displaycomponent'], '3:10-6:12': ['tablecomponent'] } // Finds the key that is associated with the component type and extracts the dimensions from it @@ -52,7 +52,7 @@ 'buttoncomponent' ].includes(componentType) ) { - return { w: column, h: 1 } + return { w: column, h: 2 } } return { w: column, h: 80 } } diff --git a/frontend/src/lib/logout.ts b/frontend/src/lib/logout.ts index dee90456f9..3ea25ecd55 100644 --- a/frontend/src/lib/logout.ts +++ b/frontend/src/lib/logout.ts @@ -4,7 +4,6 @@ import { clearStores } from './stores.js' import { sendUserToast } from './utils.js' export async function logoutWithRedirect(rd?: string): Promise { - console.log('logoutWithRedirect', rd) await clearUser() if (rd && rd?.split('?')[0] != '/user/login') { diff --git a/frontend/src/lib/utils.ts b/frontend/src/lib/utils.ts index ebbdcb838d..1ecaff8d08 100644 --- a/frontend/src/lib/utils.ts +++ b/frontend/src/lib/utils.ts @@ -184,9 +184,9 @@ export function removeItemAll(arr: T[], value: T) { } export async function isOwner(path: string, user: UserExt, workspace: string): Promise { - if (user.is_admin && (workspace != 'starter' || user.is_super_admin)) { + if (user.is_admin && ((workspace == 'starter' || workspace == 'admin') && user.is_super_admin)) { return true - } else if (workspace == 'starter') { + } else if (workspace == 'starter' || workspace == 'admin') { return false } else { return await UserService.isOwnerOfPath({ path: path, workspace: workspace }) diff --git a/frontend/src/routes/(root)/(logged)/resources/+page.svelte b/frontend/src/routes/(root)/(logged)/resources/+page.svelte index 4e75c1ef33..b52c44d609 100644 --- a/frontend/src/routes/(root)/(logged)/resources/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/resources/+page.svelte @@ -554,7 +554,7 @@ {#if !canWrite} Shared globallyThis resource type is from the 'starter' workspace shared with all + >This resource type is from the 'admins' workspace shared with all workspaces diff --git a/frontend/src/routes/(root)/(logged)/schedules/+page.svelte b/frontend/src/routes/(root)/(logged)/schedules/+page.svelte index d714c4d582..37a5c9a922 100644 --- a/frontend/src/routes/(root)/(logged)/schedules/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/schedules/+page.svelte @@ -89,7 +89,7 @@