diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 6230069633..1d083b6f75 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -cf48cb048f413cc922cce08b303e40dfebb6b897 +6f03a04595174228a4609b76725394c7feedea00 diff --git a/docs/git-sync-gitlab-setup.md b/docs/git-sync-gitlab-setup.md index e3d03e2e64..14bbdae34b 100644 --- a/docs/git-sync-gitlab-setup.md +++ b/docs/git-sync-gitlab-setup.md @@ -68,8 +68,15 @@ Give the resource its final path before picking a project. The token is filed under that path, so renaming afterwards leaves it behind. Forks of the workspace read this one copy rather than getting their own, so -renewal reaches all of them at once and no fork holds a credential a fork admin -could read. +renewal reaches all of them at once and the token is not duplicated into every +descendant workspace. + +Treat workspace admin as equivalent to holding the token. An admin of the +workspace, or of any fork below it, can point a repository at a sync script they +wrote and have that job request the credential, exactly as they can for a GitHub +App installation token. Storing it this way keeps it out of the variables API and +out of every fork's own storage; it is not a boundary against the admins of those +workspaces. A URL with the token written into it keeps working, whether it sits in the resource or in a secret variable the resource points at (`"url": "$var:..."`), diff --git a/frontend/src/lib/components/ApiConnectForm.svelte b/frontend/src/lib/components/ApiConnectForm.svelte index d339c779a2..2deb075e34 100644 --- a/frontend/src/lib/components/ApiConnectForm.svelte +++ b/frontend/src/lib/components/ApiConnectForm.svelte @@ -29,13 +29,13 @@ isValid?: boolean linkedSecretCandidates?: string[] | undefined description?: string | undefined - /** Path the resource is being saved at, passed through to the GitLab picker - * so the credential it stores is keyed by the resource's own path. */ - resourcePath?: string /** Workspace the resource is being saved into, which is not always the one * being navigated. The GitLab picker has to store the credential where the * resource will look for it. */ workspace?: string + /** A git credential the picker chose, for the drawer to store once it has + * saved the resource and its path is final. */ + onCredentialSelected?: (credential: { token: string; repoUrl: string }) => void onSynced?: () => void } @@ -47,8 +47,8 @@ isValid = $bindable(true), linkedSecretCandidates = undefined, description = $bindable(undefined), - resourcePath = undefined, workspace = undefined, + onCredentialSelected, onSynced = undefined }: Props = $props() @@ -264,8 +264,8 @@ { args = newArgs rawCode = JSON.stringify(args, null, 2) diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index 4beb3be850..a9ffd38d00 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -12,6 +12,7 @@ sortResourceTypesByMatch } from './resourceTypeDisplay' import { + GitSyncService, OauthService, ResourceService, WorkspaceService, @@ -75,6 +76,7 @@ }: Props = $props() let effectiveWorkspace = $derived(workspace ?? $workspaceStore!) + let pendingGitCredential: { token: string; repoUrl: string } | undefined = $state(undefined) let isValid = $state(true) @@ -952,6 +954,19 @@ } }) } + // After the resource exists, so the path the credential is filed under is + // the one that was actually saved and a cancelled form writes nothing. + if (pendingGitCredential) { + await GitSyncService.setGitCredential({ + workspace: effectiveWorkspace, + requestBody: { + repo_path: path, + repo_url: pendingGitCredential.repoUrl, + token: pendingGitCredential.token + } + }) + pendingGitCredential = undefined + } dispatch('refresh', path) dispatch('close') sendUserToast( @@ -1388,8 +1403,16 @@ {linkedSecretCandidates} {resourceType} {resourceTypeInfo} - resourcePath={path} workspace={effectiveWorkspace} + onCredentialSelected={(c) => { + pendingGitCredential = c + // `forceSecretValue` files a git_repository's `url` in a secret + // variable, for the URLs that carry a token in them. The picker's + // does not — the token is stored separately — so that variable + // would hold nothing secret and add a second place to keep in + // step with the resource. + linkedSecrets = linkedSecrets.filter((f) => f !== 'url') + }} bind:args bind:isValid onSynced={getResourceTypeInfo} diff --git a/frontend/src/lib/components/GitLabIntegration.svelte b/frontend/src/lib/components/GitLabIntegration.svelte index 150d1a5ee6..bb0c4fd605 100644 --- a/frontend/src/lib/components/GitLabIntegration.svelte +++ b/frontend/src/lib/components/GitLabIntegration.svelte @@ -16,9 +16,9 @@ * one being navigated: the credential has to land where the resource will * look for it. */ workspace?: string - /** Path the resource is being saved at. The stored credential is keyed by - * it, so the picker cannot run before the resource has a path. */ - resourcePath?: string + /** The picked project's token, handed over for the form to store once the + * resource is saved and its path is final. */ + onCredentialSelected?: (credential: { token: string; repoUrl: string }) => void onArgsUpdate?: (args: Record) => void } @@ -26,7 +26,7 @@ resourceType, args = {}, workspace = undefined, - resourcePath = undefined, + onCredentialSelected, onArgsUpdate }: Props = $props() @@ -38,7 +38,6 @@ let projects: GitlabProject[] = $state([]) let selectedProject: string | undefined = $state(undefined) let loading = $state(false) - let applying = $state(false) let listError: string | undefined = $state(undefined) // Shown alongside the GitHub App button and on the same terms, so the two @@ -55,7 +54,6 @@ let enabled = $derived(!!$enterpriseLicense) let project = $derived(projects.find((p) => p.path_with_namespace === selectedProject)) - let hasPath = $derived(!!resourcePath && resourcePath !== '') async function listProjects() { if (!ws) return @@ -79,37 +77,26 @@ } } - async function apply(close: (_: any) => void) { - if (!ws || !project || !token || !resourcePath) return - // Everything this writes is read once, here, before the first await. The - // selector stays live while the request is in flight, so re-reading it - // later could store one project's token against another's URL. - const workspace = ws + function apply(close: (_: any) => void) { + if (!project || !token) return const chosen = project - const repoPath = resourcePath const url = chosen.http_url_to_repo - applying = true - try { - await GitSyncService.setGitCredential({ - workspace, - requestBody: { repo_path: repoPath, repo_url: url, token } - }) - onArgsUpdate?.({ - ...args, - url, - is_github_app: false, - branch: args.branch || chosen.default_branch || undefined - }) - token = '' - projects = [] - selectedProject = undefined - sendUserToast(`Windmill stored the token for ${chosen.path_with_namespace}`) - close(null) - } catch (err) { - sendUserToast(`Could not store the token: ${err?.body ?? err?.message}`, true) - } finally { - applying = false - } + // Handed to the form instead of stored now. The credential is filed under + // the resource's path, which is not settled until the resource is saved, + // and writing here would outlive an edit the user then cancels: picking a + // different project and backing out would have replaced a working token. + onCredentialSelected?.({ token, repoUrl: url }) + onArgsUpdate?.({ + ...args, + url, + is_github_app: false, + branch: args.branch || chosen.default_branch || undefined + }) + token = '' + projects = [] + selectedProject = undefined + sendUserToast(`${chosen.path_with_namespace} selected. Its token is stored when you save.`) + close(null) } @@ -151,8 +138,8 @@
- Windmill keeps it for this repository and hands it only to this workspace's sync jobs. - Forks of this workspace use it without holding a copy. + Windmill keeps it for this repository and hands it to this workspace's sync jobs. + Forks read this one copy instead of storing their own, so renewal reaches them all.
@@ -186,29 +173,17 @@ }))} bind:value={selectedProject} clearable={false} - disabled={applying} />
- {#if hasPath} -
- Stored for the resource at {resourcePath}. Give the resource its final path before - applying, so the token stays with it. -
- {:else} - - The token is kept against the resource's path. Name the resource, then pick the - project. - - {/if} +
+ The token is stored when you save the resource, under whatever path you save it at. +