mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-26 00:01:37 +00:00
fix: persist on-behalf-of user across app deploy paths (#9773)
* fix(frontend): persist on-behalf-of user when redeploying raw apps The raw-app deploy drawer reused AppEditorHeaderDeploy but never wired up the `preserveOnBehalfOf` bindable nor forwarded `preserve_on_behalf_of` in the createAppRaw/updateAppRaw request bodies. Without that flag, the shared backend handler (create_app_internal/update_app_internal) resets the policy's on_behalf_of to the deploying user on every deploy. So a publisher who set "App executed on behalf of <other user>" would silently lose it on the next deploy, unlike every other setting on the deploy page. Mirror the classic (low-code) app header: declare `preserveOnBehalfOf`, bind it to the deploy component, and send `preserve_on_behalf_of` on both create and update. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(frontend): preserve on-behalf-of in the draft-deploy path The draft-deploy path (deployDraft → AppService.createApp/updateApp for visual apps, deployRawAppDraft → createAppRaw/updateAppRaw for raw apps) carries the deployed app's policy forward but never sent preserve_on_behalf_of. So deploying a draft via the "Review & deploy drafts" UI silently reset the policy's on_behalf_of to the deploying user — the same backend reset behind the deploy-drawer bug, on a surface that has no on-behalf-of selector to re-set it. Send preserve_on_behalf_of whenever the carried policy has an on_behalf_of, for both app types. The backend still gates actual preservation on can_preserve_on_behalf_of, so a non-deployer cannot escalate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(frontend): preserve on-behalf-of in the AI-chat raw-app deploy The global AI-chat deploy path (`deploy_workspace_item` → createAppRaw/ updateAppRaw in copilot/chat/global/core.ts) carried the recomputed policy forward but omitted preserve_on_behalf_of, so deploying a raw app via chat reset the policy's on_behalf_of to the deploying user — the last of the deploy surfaces with this gap. Send the flag when the policy has an on_behalf_of, mirroring the editor and draft-deploy paths; the backend still gates preservation on can_preserve_on_behalf_of. Add a regression test asserting the flag is forwarded when the deployed policy carries an on_behalf_of. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2135,6 +2135,40 @@ describe('global AI tools', () => {
|
||||
expect(getBackendDraft('raw_app', 'f/apps/report', { workspace: WORKSPACE })).toBeUndefined()
|
||||
})
|
||||
|
||||
it('forwards preserve_on_behalf_of when the deployed policy carries an on_behalf_of', async () => {
|
||||
// Without the flag the backend resets the policy's on_behalf_of to the
|
||||
// deploying user; this chat path has no on-behalf-of selector, so it must
|
||||
// preserve whatever the carried policy already holds.
|
||||
vi.mocked(AppService.existsApp).mockResolvedValueOnce(true)
|
||||
seedBackendDraft(
|
||||
'raw_app',
|
||||
'f/apps/obo',
|
||||
{
|
||||
summary: 'On-behalf app',
|
||||
files: { '/index.tsx': 'console.log("obo")' },
|
||||
runnables: {},
|
||||
data: { tables: [] },
|
||||
policy: {
|
||||
execution_mode: 'publisher',
|
||||
on_behalf_of: 'u/alice',
|
||||
on_behalf_of_email: 'alice@windmill.dev'
|
||||
}
|
||||
},
|
||||
{ workspace: WORKSPACE }
|
||||
)
|
||||
vi.mocked(AppService.getAppByPath).mockResolvedValueOnce({} as any)
|
||||
|
||||
await callGlobalTool('deploy_workspace_item', { type: 'app', path: 'f/apps/obo' })
|
||||
|
||||
expect(AppService.updateAppRaw).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
formData: expect.objectContaining({
|
||||
app: expect.objectContaining({ preserve_on_behalf_of: true })
|
||||
})
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('notifies the session preview (as raw_app) after deploying a raw app', async () => {
|
||||
const onDeployed = vi.fn()
|
||||
setDeployedInSessionHandler(onDeployed)
|
||||
|
||||
@@ -3932,7 +3932,11 @@ async function deployDraft(
|
||||
value: rawAppValue,
|
||||
summary,
|
||||
policy,
|
||||
deployment_message: deploymentMessage
|
||||
deployment_message: deploymentMessage,
|
||||
// Preserve the policy's on_behalf_of: this chat deploy path has no
|
||||
// on-behalf-of selector, so without the flag the backend resets it to
|
||||
// the deploying user (gated server-side by can_preserve_on_behalf_of).
|
||||
preserve_on_behalf_of: policy.on_behalf_of ? true : undefined
|
||||
},
|
||||
js: bundle.js,
|
||||
css: bundle.css
|
||||
@@ -3948,7 +3952,9 @@ async function deployDraft(
|
||||
summary,
|
||||
policy,
|
||||
deployment_message: deploymentMessage,
|
||||
custom_path: appValue.custom_path
|
||||
custom_path: appValue.custom_path,
|
||||
// Preserve the policy's on_behalf_of (see update branch above).
|
||||
preserve_on_behalf_of: policy.on_behalf_of ? true : undefined
|
||||
},
|
||||
js: bundle.js,
|
||||
css: bundle.css
|
||||
|
||||
@@ -187,6 +187,11 @@
|
||||
onOpenOthersDrafts
|
||||
}: Props = $props()
|
||||
|
||||
// Set by the on-behalf-of selector when the publisher picks a user other than
|
||||
// themselves. Forwarded as `preserve_on_behalf_of` so the backend keeps the
|
||||
// policy's on_behalf_of instead of resetting it to the deploying user.
|
||||
let preserveOnBehalfOf = $state(false)
|
||||
|
||||
// The AutosaveIndicator watches these; in the sessions preview they're the
|
||||
// session's (workspace, path), else the full-page editor's own values.
|
||||
const indicatorWorkspace = $derived(autosaveWorkspace ?? $workspaceStore)
|
||||
@@ -315,7 +320,8 @@
|
||||
summary: summary,
|
||||
policy,
|
||||
deployment_message: deploymentMsg,
|
||||
custom_path: customPath
|
||||
custom_path: customPath,
|
||||
preserve_on_behalf_of: preserveOnBehalfOf || undefined
|
||||
},
|
||||
js,
|
||||
css
|
||||
@@ -434,6 +440,7 @@
|
||||
policy,
|
||||
path: npath,
|
||||
deployment_message: deploymentMsg,
|
||||
preserve_on_behalf_of: preserveOnBehalfOf || undefined,
|
||||
// custom_path requires admin so to accept update without it, we need to send as undefined when non-admin (when undefined, it will be ignored)
|
||||
// it also means that customPath needs to be set to '' instead of undefined to unset it (when admin)
|
||||
custom_path:
|
||||
@@ -674,6 +681,7 @@
|
||||
bind:customPathError
|
||||
bind:pathError
|
||||
bind:newEditedPath
|
||||
bind:preserveOnBehalfOf
|
||||
/>
|
||||
</DrawerContent>
|
||||
</Drawer>
|
||||
|
||||
@@ -75,7 +75,11 @@ export async function deployRawAppDraft(
|
||||
summary,
|
||||
policy,
|
||||
deployment_message: deploymentMessage,
|
||||
custom_path: isAdmin ? (value.custom_path ?? '') : undefined
|
||||
custom_path: isAdmin ? (value.custom_path ?? '') : undefined,
|
||||
// Preserve the policy's on_behalf_of: this draft-deploy path has no
|
||||
// on-behalf-of selector, so without the flag the backend resets it to
|
||||
// the deploying user (gated server-side by can_preserve_on_behalf_of).
|
||||
preserve_on_behalf_of: policy.on_behalf_of ? true : undefined
|
||||
},
|
||||
js: bundle.js,
|
||||
css: bundle.css
|
||||
@@ -91,7 +95,9 @@ export async function deployRawAppDraft(
|
||||
summary,
|
||||
policy,
|
||||
deployment_message: deploymentMessage,
|
||||
custom_path: value.custom_path
|
||||
custom_path: value.custom_path,
|
||||
// Preserve the policy's on_behalf_of (see update branch above).
|
||||
preserve_on_behalf_of: policy.on_behalf_of ? true : undefined
|
||||
},
|
||||
js: bundle.js,
|
||||
css: bundle.css
|
||||
|
||||
@@ -370,15 +370,20 @@ export async function deployDraft(
|
||||
// undefined so the backend preserves the existing route. The draft has no
|
||||
// custom_path, so admins fall back to the deployed route (`''` when none).
|
||||
const isAdmin = !!(get(userStore)?.is_admin || get(userStore)?.is_super_admin)
|
||||
const policy = r.policy ?? { execution_mode: 'publisher' }
|
||||
const requestBody = {
|
||||
value: appValue,
|
||||
summary: draftSummary ?? r.summary ?? '',
|
||||
policy: r.policy ?? { execution_mode: 'publisher' },
|
||||
policy,
|
||||
// Honor the draft's intended path; `draft_path` holds the user-typed path
|
||||
// for a never-deployed app parked at a `u/{user}/draft_{uuid}` storage key.
|
||||
path: draftPath ?? r.path ?? path,
|
||||
custom_path: isAdmin ? (r.custom_path ?? '') : undefined,
|
||||
deployment_message: deploymentMessage
|
||||
deployment_message: deploymentMessage,
|
||||
// The draft carries no on-behalf-of selector — the policy comes straight
|
||||
// from the deployed app. Preserve its on_behalf_of (the backend resets it
|
||||
// to the deploying user without this flag, gated by can_preserve_on_behalf_of).
|
||||
preserve_on_behalf_of: policy?.on_behalf_of ? true : undefined
|
||||
}
|
||||
// Same as flows: draft-only apps have no app row → create;
|
||||
// drafts on a deployed app update it.
|
||||
|
||||
Reference in New Issue
Block a user