mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-25 16:03:47 +00:00
fix: link a variable to its OAuth account only in the connect flow (#11361)
* fix: only let an oauth account's creator or readers link it to a variable Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: drop an oauth account link the caller may not hold instead of failing Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: cover relinking an oauth account through a variable update Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: cover a permitted relink through a variable update Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep an oauth account's creator in step with the user's username Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: follow a non-member superadmin's instance username on the OAuth accounts they created Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: link a variable to its OAuth account only in the connect flow The variable endpoints no longer take an account link from the caller. The connect flows create the token variable first and pass its path to create_account, which links it server-side. This replaces the creator check and the created_by bookkeeping, along with both migrations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: keep the MCP secret variable test's formatting Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: link the account to the token variable AppConnect created, even if the path is edited mid-save Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: stop exporting the OAuth account a variable links, and regenerate the chat tool schema Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: pin the EE refresh and linking checks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: delete AppConnect's token variable when linking its account fails, so Save can be retried Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep the resource AppConnect fills when linking its account fails Deleting a variable also deletes the resource at its path, so the rollback is skipped when filling an existing resource. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: create an OAuth token variable and its account in one request 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> * fix: stop listing the ignored account input in the chat's write_variable instructions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: drop the account visibility helper the oauth refresh route no longer uses Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref to da49c12e3006fc77de789277f1462a4d8d7d92ce This commit updates the EE repository reference after PR #829 was merged in windmill-ee-private. Previous ee-repo-ref: 58681a2e7eafb5689ed739cd2560fa0f5d82c3dc New ee-repo-ref: da49c12e3006fc77de789277f1462a4d8d7d92ce Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
hugocasa
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
Claude Opus 5.5
windmill-internal-app[bot]
parent
34de8bc06d
commit
d6adac8b75
+4
-3
@@ -1,16 +1,17 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT EXISTS (SELECT 1 FROM account WHERE workspace_id = $1 AND id = $2) AS \"exists!\"",
|
||||
"query": "SELECT EXISTS(SELECT 1 FROM variable WHERE workspace_id = $1 AND path = $2 AND account = $3)",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "exists!",
|
||||
"name": "exists",
|
||||
"type_info": "Bool"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text",
|
||||
"Int4"
|
||||
]
|
||||
@@ -19,5 +20,5 @@
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "e910ae81fb98642e76644e9691157c3d02adce3ba55324375a2035231c640ac0"
|
||||
"hash": "086866581c8f253a611d85cc9464e63767da94af0fcb6d660cd6fb0479b9a99d"
|
||||
}
|
||||
-23
@@ -1,23 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT EXISTS(SELECT 1 FROM variable WHERE account = $1 AND workspace_id = $2)",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "exists",
|
||||
"type_info": "Bool"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int4",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "7aa589db3199d7f727cc69e63e1281b7ed329ff0c9d1617747f4ccd6014720cf"
|
||||
}
|
||||
-23
@@ -1,23 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT account FROM variable WHERE path = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "account",
|
||||
"type_info": "Int4"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
true
|
||||
]
|
||||
},
|
||||
"hash": "c925264b7b0fd44ea7ab01c9af1514b9a9f2200e5a5db0a741697b28cd8b505f"
|
||||
}
|
||||
@@ -1 +1 @@
|
||||
c8d562aa9d75a7e99bf0ecd1ac224f532452a07e
|
||||
da49c12e3006fc77de789277f1462a4d8d7d92ce
|
||||
|
||||
@@ -338,3 +338,65 @@ async fn test_variables_are_cached(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Reading a variable linked to an OAuth account refreshes through it and returns the fresh token,
|
||||
/// so the variable endpoints must not let a caller choose the link.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn test_variable_endpoints_ignore_account_link(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace/variables");
|
||||
|
||||
let account: i32 = sqlx::query_scalar(
|
||||
"INSERT INTO account (workspace_id, client, expires_at, refresh_token)
|
||||
VALUES ('test-workspace', 'github', now(), 'r') RETURNING id",
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
let link = |path: &'static str| {
|
||||
let db = db.clone();
|
||||
async move {
|
||||
sqlx::query_scalar::<_, Option<i32>>(
|
||||
"SELECT account FROM variable WHERE workspace_id = 'test-workspace' AND path = $1",
|
||||
)
|
||||
.bind(path)
|
||||
.fetch_one(&db)
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
};
|
||||
|
||||
let path = "u/test-user/oauth_probe";
|
||||
let resp = authed(client().post(format!("{base}/create")))
|
||||
.json(
|
||||
&json!({ "path": path, "value": "t", "is_secret": true, "description": "",
|
||||
"is_oauth": true, "account": account }),
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201);
|
||||
assert_eq!(link(path).await, None);
|
||||
|
||||
let other: i32 = sqlx::query_scalar(
|
||||
"INSERT INTO account (workspace_id, client, expires_at, refresh_token)
|
||||
VALUES ('test-workspace', 'github', now(), 'r') RETURNING id",
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
sqlx::query(
|
||||
"UPDATE variable SET account = $1 WHERE workspace_id = 'test-workspace' AND path = $2",
|
||||
)
|
||||
.bind(account)
|
||||
.bind(path)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
let resp = authed(client().post(variable_url(port, "update", path)))
|
||||
.json(&json!({ "description": "d", "account": other }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200);
|
||||
assert_eq!(link(path).await, Some(account));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -8894,6 +8894,9 @@ paths:
|
||||
items:
|
||||
type: string
|
||||
description: "OAuth scopes to use for token refresh. Overrides instance-level scopes."
|
||||
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
|
||||
@@ -32523,7 +32526,8 @@ components:
|
||||
description: The description of the variable
|
||||
account:
|
||||
type: integer
|
||||
description: The account identifier
|
||||
deprecated: true
|
||||
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
|
||||
|
||||
@@ -968,11 +968,13 @@ pub(crate) async fn tarball_workspace(
|
||||
}
|
||||
|
||||
if !skip_variables.unwrap_or(false) {
|
||||
// `account` is left out: the variable endpoints ignore it, so a pushed spec carrying it
|
||||
// would never match the variable it created and every sync would report it changed.
|
||||
let variables =
|
||||
sqlx::query_as::<_, ExportableListableVariable>(if !skip_secrets.unwrap_or(false) {
|
||||
"SELECT workspace_id, path, value, is_secret, description, extra_perms, account, is_oauth, expires_at, labels FROM variable WHERE workspace_id = $1 AND expires_at IS NULL"
|
||||
"SELECT workspace_id, path, value, is_secret, description, extra_perms, NULL::int AS account, is_oauth, expires_at, labels FROM variable WHERE workspace_id = $1 AND expires_at IS NULL"
|
||||
} else {
|
||||
"SELECT workspace_id, path, value, is_secret, description, extra_perms, account, is_oauth, expires_at, labels FROM variable WHERE workspace_id = $1 AND is_secret = false AND expires_at IS NULL"
|
||||
"SELECT workspace_id, path, value, is_secret, description, extra_perms, NULL::int AS account, is_oauth, expires_at, labels FROM variable WHERE workspace_id = $1 AND is_secret = false AND expires_at IS NULL"
|
||||
})
|
||||
.bind(&w_id)
|
||||
.fetch_all(&mut *tx)
|
||||
|
||||
@@ -180,23 +180,6 @@ where
|
||||
Ok(token.access_token)
|
||||
}
|
||||
|
||||
/// Whether account `id` exists in `w_id`, read through the caller's user transaction. `account`
|
||||
/// carries no RLS, so this gates nothing beyond the workspace the caller is already authorized
|
||||
/// for: a refresh asked for by account id is scoped by that membership, not by ownership.
|
||||
pub async fn account_visible(
|
||||
user_conn: &mut sqlx::PgConnection,
|
||||
w_id: &str,
|
||||
id: i32,
|
||||
) -> error::Result<bool> {
|
||||
Ok(sqlx::query_scalar!(
|
||||
"SELECT EXISTS (SELECT 1 FROM account WHERE workspace_id = $1 AND id = $2) AS \"exists!\"",
|
||||
w_id,
|
||||
id
|
||||
)
|
||||
.fetch_one(user_conn)
|
||||
.await?)
|
||||
}
|
||||
|
||||
async fn current_token(db: &DB, w_id: &str, path: &str) -> error::Result<String> {
|
||||
let value = sqlx::query_scalar!(
|
||||
"SELECT value FROM variable WHERE workspace_id = $1 AND path = $2",
|
||||
|
||||
@@ -594,6 +594,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(
|
||||
@@ -625,7 +668,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) {
|
||||
@@ -646,6 +689,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)
|
||||
@@ -655,7 +719,7 @@ async fn create_variable(
|
||||
value,
|
||||
variable.is_secret,
|
||||
variable.description,
|
||||
variable.account,
|
||||
account_id,
|
||||
variable.is_oauth.unwrap_or(false),
|
||||
variable.expires_at,
|
||||
variable.labels.as_deref() as Option<&[String]>,
|
||||
@@ -705,10 +769,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(
|
||||
@@ -1059,7 +1120,6 @@ struct EditVariable {
|
||||
value: Option<String>,
|
||||
is_secret: Option<bool>,
|
||||
description: Option<String>,
|
||||
account: Option<i32>,
|
||||
labels: Option<Vec<String>>,
|
||||
ws_specific: Option<bool>,
|
||||
}
|
||||
@@ -1156,11 +1216,6 @@ async fn update_variable(
|
||||
has_sql_updates = true;
|
||||
}
|
||||
|
||||
if let Some(account_id) = ns.account {
|
||||
sqlb.set_str("account", account_id);
|
||||
has_sql_updates = true;
|
||||
}
|
||||
|
||||
if let Some(nbool) = ns.is_secret {
|
||||
let old_secret = sqlx::query_scalar!(
|
||||
"SELECT is_secret from variable WHERE path = $1 AND workspace_id = $2",
|
||||
@@ -1179,20 +1234,6 @@ async fn update_variable(
|
||||
has_sql_updates = true;
|
||||
}
|
||||
|
||||
// Get old account_id if we're updating the account field
|
||||
let old_account_id = if ns.account.is_some() {
|
||||
sqlx::query_scalar!(
|
||||
"SELECT account FROM variable WHERE path = $1 AND workspace_id = $2",
|
||||
&path,
|
||||
&w_id
|
||||
)
|
||||
.fetch_optional(&db)
|
||||
.await?
|
||||
.flatten()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let mut tx: Transaction<'_, Postgres> = user_db.begin(&authed).await?;
|
||||
|
||||
if let Some(npath) = ns.path.clone() {
|
||||
@@ -1366,32 +1407,6 @@ async fn update_variable(
|
||||
)
|
||||
.await?;
|
||||
|
||||
// Clean up old account if it's no longer referenced and different from new account
|
||||
if let Some(old_acc_id) = old_account_id {
|
||||
if ns.account.is_some() && ns.account != Some(old_acc_id) {
|
||||
// Check if old account is still referenced by other variables or resources
|
||||
let account_still_used = sqlx::query_scalar!(
|
||||
"SELECT EXISTS(SELECT 1 FROM variable WHERE account = $1 AND workspace_id = $2)",
|
||||
old_acc_id,
|
||||
&w_id
|
||||
)
|
||||
.fetch_one(&mut *tx)
|
||||
.await?
|
||||
.unwrap_or(true);
|
||||
|
||||
if !account_still_used {
|
||||
// Delete the orphaned account
|
||||
sqlx::query!(
|
||||
"DELETE FROM account WHERE id = $1 AND workspace_id = $2",
|
||||
old_acc_id,
|
||||
&w_id
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
// Detect if this was a rename operation
|
||||
|
||||
@@ -939,9 +939,9 @@
|
||||
args['realmId'] = responseExtra['realmId']
|
||||
}
|
||||
|
||||
let account: number | undefined = undefined
|
||||
let accountData: any = undefined
|
||||
if (valueToken?.expires_in != undefined) {
|
||||
const accountData: any = {
|
||||
accountData = {
|
||||
refresh_token: valueToken.refresh_token ?? '',
|
||||
expires_in: valueToken.expires_in,
|
||||
client: connectClient,
|
||||
@@ -970,13 +970,6 @@
|
||||
accountData.cc_token_url = tokenUrl.trim()
|
||||
}
|
||||
}
|
||||
|
||||
account = Number(
|
||||
await OauthService.createAccount({
|
||||
workspace: effectiveWorkspace,
|
||||
requestBody: accountData
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
// A copy: the form is still mounted and bound to `args` across the awaits below, and
|
||||
@@ -995,21 +988,27 @@
|
||||
// OAuth flow: single secret variable for the token
|
||||
if (typeof value == 'string' && value != '' && !value.startsWith('$var:')) {
|
||||
savedVariableCount++
|
||||
await VariableService.createVariable({
|
||||
workspace: effectiveWorkspace,
|
||||
requestBody: {
|
||||
path,
|
||||
value: value,
|
||||
is_secret: true,
|
||||
description: emptyString(description)
|
||||
? `OAuth token for ${resourceType}`
|
||||
: description,
|
||||
is_oauth: true,
|
||||
account: account,
|
||||
ws_specific: wsSpecific
|
||||
}
|
||||
})
|
||||
resourceValue['token'] = `$var:${path}`
|
||||
const tokenVariable = {
|
||||
path,
|
||||
value: value,
|
||||
is_secret: true,
|
||||
description: emptyString(description) ? `OAuth token for ${resourceType}` : description,
|
||||
is_oauth: true,
|
||||
ws_specific: wsSpecific
|
||||
}
|
||||
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)
|
||||
|
||||
@@ -2854,7 +2854,7 @@ function getResourceInstructions(): string {
|
||||
- Global mode writes complete draft payloads only; it does not save, deploy, run, scaffold local files, or generate metadata.
|
||||
- A resource draft is a workspace item: \`{ type: 'resource', path, summary?, value, isDraft }\`. \`value\` is a CreateResource body: \`{ path, value, description?, resource_type, labels? }\` where the inner \`value\` is the resource type's data shape.
|
||||
- Reading a variable returns \`{ type: 'variable', path, summary?, isSecret, isDraft }\` — never its value, secret or not. \`isSecret\` tells you whether the value is encrypted.
|
||||
- \`write_variable\` takes \`{ path, value?, is_secret?, description?, account?, is_oauth?, expires_at?, labels? }\`. Creating a variable needs \`value\` and \`is_secret\`; editing one needs only the fields you are changing. Omitting \`value\` keeps the stored value, which is the only way to edit a secret variable — you cannot read its value, so passing any \`value\` you did not get from the user destroys it.
|
||||
- \`write_variable\` takes \`{ path, value?, is_secret?, description?, is_oauth?, expires_at?, labels? }\`. Creating a variable needs \`value\` and \`is_secret\`; editing one needs only the fields you are changing. Omitting \`value\` keeps the stored value, which is the only way to edit a secret variable — you cannot read its value, so passing any \`value\` you did not get from the user destroys it.
|
||||
- For secret fields in a resource value, create the variable with \`write_variable\` and \`is_secret: true\`, and deploy it before the resource (see "Secrets" in the reference below).
|
||||
- Reference formats inside resource values: \`$var:g/all/name\` (global), \`$var:u/user/name\` (user), \`$var:f/folder/name\` (folder). Reference another resource with \`$res:path/to/resource\`. The same strings are also how a resource or variable is passed as a run argument (see the run-argument rule in the resource reference below); what they are never valid as is a variable's own value.
|
||||
- Use \`search_resource_types\` to discover valid \`resource_type\` names and their JSON Schemas. Match the resource value to that schema.
|
||||
|
||||
@@ -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("The account identifier").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(),
|
||||
|
||||
@@ -278,27 +278,21 @@
|
||||
if (!t) return
|
||||
const { workspace, path } = t
|
||||
try {
|
||||
let account: number | undefined = undefined
|
||||
if (res?.expires_in != undefined) {
|
||||
account = Number(
|
||||
await OauthService.createAccount({
|
||||
workspace,
|
||||
requestBody: {
|
||||
refresh_token: res.refresh_token ?? '',
|
||||
expires_in: res.expires_in,
|
||||
client: t.client,
|
||||
scopes: t.scopes
|
||||
}
|
||||
})
|
||||
)
|
||||
}
|
||||
await upsertSecretVariable({
|
||||
workspace,
|
||||
path,
|
||||
value: res.access_token,
|
||||
resourcePath: path,
|
||||
isOauth: true,
|
||||
account
|
||||
account:
|
||||
res?.expires_in != undefined
|
||||
? {
|
||||
refresh_token: res.refresh_token ?? '',
|
||||
expires_in: res.expires_in,
|
||||
client: t.client,
|
||||
scopes: t.scopes
|
||||
}
|
||||
: undefined
|
||||
})
|
||||
await createMcpResource(t, `$var:${path}`)
|
||||
sendUserToast(`Connected ${t.label}`)
|
||||
|
||||
@@ -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'
|
||||
@@ -136,27 +131,21 @@
|
||||
if (!target) return
|
||||
const { workspace, path } = target
|
||||
try {
|
||||
let accountId: number | undefined
|
||||
if (data.expires_in && data.refresh_token) {
|
||||
const accountIdStr = await OauthService.createAccount({
|
||||
workspace,
|
||||
requestBody: {
|
||||
refresh_token: data.refresh_token,
|
||||
expires_in: data.expires_in,
|
||||
client: 'mcp',
|
||||
mcp_server_url: data.mcp_server_url
|
||||
}
|
||||
})
|
||||
accountId = Number(accountIdStr)
|
||||
}
|
||||
|
||||
await upsertSecretVariable({
|
||||
workspace,
|
||||
path,
|
||||
value: data.access_token,
|
||||
resourcePath: path,
|
||||
isOauth: true,
|
||||
account: accountId
|
||||
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
|
||||
})
|
||||
|
||||
await ResourceService.createResource({
|
||||
|
||||
@@ -71,7 +71,7 @@ describe('upsertSecretVariable', () => {
|
||||
existsVariable.mockResolvedValue(true)
|
||||
getVariable.mockResolvedValue({ description: OURS })
|
||||
|
||||
await upsertSecretVariable({ ...ARGS, isOauth: true, account: 7 })
|
||||
await upsertSecretVariable({ ...ARGS, isOauth: true })
|
||||
|
||||
expect(deleteVariable).toHaveBeenCalled()
|
||||
expect(createVariable).toHaveBeenCalledWith({
|
||||
@@ -81,7 +81,6 @@ describe('upsertSecretVariable', () => {
|
||||
value: 'token',
|
||||
is_secret: true,
|
||||
is_oauth: true,
|
||||
account: 7,
|
||||
description: OURS
|
||||
}
|
||||
})
|
||||
|
||||
@@ -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,7 +33,14 @@ export async function upsertSecretVariable(args: {
|
||||
/** The MCP resource this token belongs to; stamped into the description. */
|
||||
resourcePath: string
|
||||
isOauth?: boolean
|
||||
account?: number
|
||||
/** 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, account } = args
|
||||
const description = mcpTokenDescription(resourcePath)
|
||||
@@ -64,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, account, 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 })
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user