starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-17 00:02:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: create an OAuth token variable and its account in one request

Browse Source 
createAccount takes the variable and the store creates both in one
transaction, so a connect is either fully saved or not at all. The connect
flows make that single call instead of creating the variable and linking it
afterwards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
hugocasa
2026-10-16 13:08:47 +02:00
co-authored by Claude Opus 5.5
parent 01c45104fd
commit e03ff93280
10 changed files with 130 additions and 134 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-92d2202c8752ab24777daa5c46b14ae645076a85940cef890083a45301a7cd62.json
Generated
-24
View File
@@ -1,24 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "SELECT EXISTS(SELECT 1 FROM variable WHERE account = $1 AND workspace_id = $2 AND path <> $3)",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "exists",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Int4",
"Text",
"Text"
]
},
"nullable": [
null
]
},
"hash": "92d2202c8752ab24777daa5c46b14ae645076a85940cef890083a45301a7cd62"
}
backend/.sqlx/query-fca09e2895680f5eee6e89bcecf6595e3786e4489ceff5ad2e301635c4606f2e.json
Generated
-24
View File
@@ -1,24 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "WITH previous AS (SELECT account FROM variable WHERE workspace_id = $1 AND path = $2)\n UPDATE variable SET account = $3 WHERE workspace_id = $1 AND path = $2\n RETURNING (SELECT account FROM previous)",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "account",
"type_info": "Int4"
}
],
"parameters": {
"Left": [
"Text",
"Text",
"Int4"
]
},
"nullable": [
null
]
},
"hash": "fca09e2895680f5eee6e89bcecf6595e3786e4489ceff5ad2e301635c4606f2e"
}
backend/ee-repo-ref.txt
+1 -1
View File
@@ -1 +1 @@
fa84be43efe17ec2f55a6026d48c48c064d97254
8ded9be3d5c9b19cc84dd5d2b3106b2ac00d2ac5
backend/windmill-api/openapi.yaml
+4 -4
View File
@@ -8811,9 +8811,9 @@ paths:
items:
type: string
description: "OAuth scopes to use for token refresh. Overrides instance-level scopes."
variable_path:
type: string
description: "Path of an existing variable, writable by the caller, to link to this account so it refreshes through it. Its previous account is deleted when nothing else uses it."
variable:
$ref: "#/components/schemas/CreateVariable"
description: "The variable holding this account's access token. It is created with the account, in one transaction and with the same checks as createVariable, and linked to it so it refreshes through it."
required:
- refresh_token
- expires_in
@@ -31994,7 +31994,7 @@ components:
account:
type: integer
deprecated: true
description: Ignored. A variable is linked to an OAuth account only by `createAccount`'s `variable_path`.
description: Ignored. A variable is linked to an OAuth account only when `createAccount` creates both.
is_oauth:
type: boolean
description: Whether the variable is an OAuth variable
backend/windmill-store/src/variables.rs
+67 -8
View File
@@ -596,6 +596,49 @@ async fn create_variable(
Query(AlreadyEncrypted { already_encrypted }): Query<AlreadyEncrypted>,
Json(variable): Json<CreateVariable>,
) -> Result<(StatusCode, String)> {
let path = variable.path.clone();
create_variable_with_account(
authed,
&db,
user_db,
&webhook,
w_id,
variable,
already_encrypted.unwrap_or(false),
None,
)
.await?;
Ok((StatusCode::CREATED, format!("variable {path} created")))
}
/// An OAuth account to create with the variable that holds its access token.
pub struct NewOAuthAccount {
pub client: String,
pub expires_in: i64,
pub refresh_token: Option<String>,
pub grant_type: String,
pub cc_client_id: Option<String>,
pub cc_client_secret: Option<String>,
pub cc_token_url: Option<String>,
pub mcp_server_url: Option<String>,
pub scopes: Option<Vec<String>>,
}
/// Creates a variable and, given `account`, the OAuth account it refreshes through, linked in the
/// same transaction. Reading a linked variable refreshes through its account and hands the reader
/// the fresh token, so this is the only way a variable gets a link: `variable.account` is ignored.
/// Returns the new account's id.
pub async fn create_variable_with_account(
authed: ApiAuthed,
db: &DB,
user_db: UserDB,
webhook: &WebhookShared,
w_id: String,
variable: CreateVariable,
already_encrypted: bool,
account: Option<NewOAuthAccount>,
) -> Result<Option<i32>> {
let db = db.clone();
check_scopes(&authed, || format!("variables:write:{}", variable.path))?;
check_proper_path(&variable.path)?;
if let RuleCheckResult::Blocked(msg) = check_deploy_rules(
@@ -627,7 +670,7 @@ async fn create_variable(
let authed = maybe_refresh_folders(&variable.path, &w_id, authed, &db).await;
check_path_conflict(&db, &w_id, &variable.path).await?;
let value = if variable.is_secret && !already_encrypted.unwrap_or(false) {
let value = if variable.is_secret && !already_encrypted {
// A restored draft sends the `$encrypted:` marker as-is; decrypt it back
// (validating against the workspace key) before the secret backend re-stores it.
let plain = if variable.value.starts_with(ENCRYPTED_DRAFT_PREFIX) {
@@ -648,6 +691,27 @@ async fn create_variable(
let mut tx = user_db.begin(&authed).await?;
let account_id = match account {
Some(a) => Some(
sqlx::query_scalar!(
"INSERT INTO account (workspace_id, client, expires_at, refresh_token, grant_type, cc_client_id, cc_client_secret, cc_token_url, mcp_server_url, scopes) VALUES ($1, $2, now() + ($3 || ' seconds')::interval, $4, $5, $6, $7, $8, $9, $10) RETURNING id",
&w_id,
a.client,
a.expires_in.to_string(),
a.refresh_token,
a.grant_type,
a.cc_client_id,
a.cc_client_secret,
a.cc_token_url,
a.mcp_server_url,
a.scopes.as_deref()
)
.fetch_one(&mut *tx)
.await?,
),
None => None,
};
sqlx::query!(
"INSERT INTO variable
(workspace_id, path, value, is_secret, description, account, is_oauth, expires_at, labels, edited_by)
@@ -657,9 +721,7 @@ async fn create_variable(
value,
variable.is_secret,
variable.description,
// Reading a variable linked to an OAuth account refreshes through it and hands the reader
// the fresh token, so a caller never chooses the link: only the connect flow sets it.
None::<i32>,
account_id,
variable.is_oauth.unwrap_or(false),
variable.expires_at,
variable.labels.as_deref() as Option<&[String]>,
@@ -709,10 +771,7 @@ async fn create_variable(
WebhookMessage::CreateVariable { workspace: w_id, path: variable.path.clone() },
);
Ok((
StatusCode::CREATED,
format!("variable {} created", variable.path),
))
Ok(account_id)
}
async fn encrypt_value(
frontend/src/lib/components/AppConnectInner.svelte
+20 -34
View File
@@ -988,41 +988,27 @@
// OAuth flow: single secret variable for the token
if (typeof value == 'string' && value != '' && !value.startsWith('$var:')) {
savedVariableCount++
// Read once: the path field stays editable while this awaits, and the account
// must link the variable this call created.
const tokenPath = path
await VariableService.createVariable({
workspace: effectiveWorkspace,
requestBody: {
path: tokenPath,
value: value,
is_secret: true,
description: emptyString(description)
? `OAuth token for ${resourceType}`
: description,
is_oauth: true,
ws_specific: wsSpecific
}
})
if (accountData) {
try {
await OauthService.createAccount({
workspace: effectiveWorkspace,
requestBody: { ...accountData, variable_path: tokenPath }
})
} catch (e) {
// An unlinked token never refreshes, and its path would block a retry. Not when
// filling: deleting a variable also deletes the resource at its path.
if (!filling) {
await VariableService.deleteVariable({
workspace: effectiveWorkspace,
path: tokenPath
}).catch(() => {})
}
throw e
}
const tokenVariable = {
path,
value: value,
is_secret: true,
description: emptyString(description) ? `OAuth token for ${resourceType}` : description,
is_oauth: true,
ws_specific: wsSpecific
}
resourceValue['token'] = `$var:${tokenPath}`
if (accountData) {
// Creates the variable with the account it refreshes through, in one transaction.
await OauthService.createAccount({
workspace: effectiveWorkspace,
requestBody: { ...accountData, variable: tokenVariable }
})
} else {
await VariableService.createVariable({
workspace: effectiveWorkspace,
requestBody: tokenVariable
})
}
resourceValue['token'] = `$var:${tokenVariable.path}`
}
} else if (linkedSecrets.length === 1) {
// Single secret: use the resource path as variable name (original behavior)
frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts
Generated
+1 -1
View File
@@ -488,7 +488,7 @@ export const variableRequestSchema = z.object({
"value": z.string().describe("The value of the variable"),
"is_secret": z.boolean().describe("Whether the variable is a secret"),
"description": z.string().describe("The description of the variable"),
"account": z.number().int().describe("Ignored. A variable is linked to an OAuth account only by `createAccount`'s `variable_path`.").optional(),
"account": z.number().int().describe("Ignored. A variable is linked to an OAuth account only when `createAccount` creates both.").optional(),
"is_oauth": z.boolean().describe("Whether the variable is an OAuth variable").optional(),
"expires_at": z.string().datetime({ offset: true }).describe("The expiration date of the variable").optional(),
"labels": z.array(z.string()).optional(),
frontend/src/lib/components/mcp/McpConnect.svelte
+10 -13
View File
@@ -283,20 +283,17 @@
path,
value: res.access_token,
resourcePath: path,
isOauth: true
isOauth: true,
account:
res?.expires_in != undefined
? {
refresh_token: res.refresh_token ?? '',
expires_in: res.expires_in,
client: t.client,
scopes: t.scopes
}
: undefined
})
if (res?.expires_in != undefined) {
await OauthService.createAccount({
workspace,
requestBody: {
refresh_token: res.refresh_token ?? '',
expires_in: res.expires_in,
client: t.client,
scopes: t.scopes,
variable_path: path
}
})
}
await createMcpResource(t, `$var:${path}`)
sendUserToast(`Connected ${t.label}`)
} catch (e) {
frontend/src/lib/components/mcp/McpServerOAuthConnect.svelte
+11 -19
View File
@@ -1,10 +1,5 @@
<script lang="ts">
import {
McpOauthService,
OauthService,
ResourceService,
type DiscoverMcpOauthResponse
} from '$lib/gen'
import { McpOauthService, ResourceService, type DiscoverMcpOauthResponse } from '$lib/gen'
import { Button } from '$lib/components/common'
import Label from '$lib/components/Label.svelte'
import { sendUserToast } from '$lib/toast'
@@ -141,20 +136,17 @@
path,
value: data.access_token,
resourcePath: path,
isOauth: true
isOauth: true,
account:
data.expires_in && data.refresh_token
? {
refresh_token: data.refresh_token,
expires_in: data.expires_in,
client: 'mcp',
mcp_server_url: data.mcp_server_url
}
: undefined
})
if (data.expires_in && data.refresh_token) {
await OauthService.createAccount({
workspace,
requestBody: {
refresh_token: data.refresh_token,
expires_in: data.expires_in,
client: 'mcp',
mcp_server_url: data.mcp_server_url,
variable_path: path
}
})
}
await ResourceService.createResource({
workspace,
frontend/src/lib/components/mcp/secretVariable.ts
+16 -6
View File
@@ -1,4 +1,4 @@
import { ResourceService, VariableService } from '$lib/gen'
import { OauthService, ResourceService, VariableService } from '$lib/gen'
/**
* What this connection writes in its token variable's description, and the only
@@ -33,8 +33,16 @@ export async function upsertSecretVariable(args: {
/** The MCP resource this token belongs to; stamped into the description. */
resourcePath: string
isOauth?: boolean
/** The account an expiring token refreshes through, created with the variable and linked to it. */
account?: {
refresh_token: string
expires_in: number
client: string
scopes?: string[]
mcp_server_url?: string
}
}): Promise<void> {
const { workspace, path, value, resourcePath, isOauth } = args
const { workspace, path, value, resourcePath, isOauth, account } = args
const description = mcpTokenDescription(resourcePath)
// The path picker rejects an occupied path, but it validates on a debounce and
@@ -63,8 +71,10 @@ export async function upsertSecretVariable(args: {
await VariableService.deleteVariable({ workspace, path })
}
await VariableService.createVariable({
workspace,
requestBody: { path, value, is_secret: true, is_oauth: isOauth, description }
})
const variable = { path, value, is_secret: true, is_oauth: isOauth, description }
if (account) {
await OauthService.createAccount({ workspace, requestBody: { ...account, variable } })
} else {
await VariableService.createVariable({ workspace, requestBody: variable })
}
}