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>
This commit is contained in:
hugocasa
2026-09-28 09:01:13 +02:00
co-authored by Claude Opus 5.5
parent 3838cd6ee0
commit 35b6dedaa1
9 changed files with 110 additions and 9 deletions
@@ -0,0 +1,23 @@
{
"db_name": "PostgreSQL",
"query": "SELECT EXISTS(SELECT 1 FROM variable WHERE workspace_id = $1 AND account = $2)",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "exists",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Text",
"Int4"
]
},
"nullable": [
null
]
},
"hash": "1f35a009e0740cbd847631ecfdeb88de508c463d11b4dce75f9a0b62c88e7a52"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO account (workspace_id, client, expires_at, refresh_token, is_workspace_integration)\n VALUES ($1, $2, now() + ($3 || ' seconds')::interval, $4, true)\n RETURNING id",
"query": "INSERT INTO account (workspace_id, client, expires_at, refresh_token, is_workspace_integration, created_by)\n VALUES ($1, $2, now() + ($3 || ' seconds')::interval, $4, true, $5)\n RETURNING id",
"describe": {
"columns": [
{
@@ -14,6 +14,7 @@
"Varchar",
"Varchar",
"Text",
"Varchar",
"Varchar"
]
},
@@ -21,5 +22,5 @@
false
]
},
"hash": "26beff5e94b68703ad81ef9dd2d08869eb3bb7659efd9bac04cdf98ae963063d"
"hash": "3883333dd076b66a2329afbfb510339c2b3b6f39f872e0315e3e3f74908d2e4b"
}
@@ -0,0 +1,23 @@
{
"db_name": "PostgreSQL",
"query": "SELECT created_by FROM account WHERE workspace_id = $1 AND id = $2",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "created_by",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text",
"Int4"
]
},
"nullable": [
true
]
},
"hash": "61fb1a18aaec9ca699fa17245cc190eb69010b1dd732a8ae848b705e2f565bd8"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "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",
"query": "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, created_by) VALUES ($1, $2, now() + ($3 || ' seconds')::interval, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id",
"describe": {
"columns": [
{
@@ -20,12 +20,13 @@
"Varchar",
"Varchar",
"Text",
"TextArray"
"TextArray",
"Varchar"
]
},
"nullable": [
false
]
},
"hash": "870e1c3f0dc1aaa07ac74a2e37721ce352ad4fb67d36c19dce09d841e36f85dd"
"hash": "d5b3132a3bea036abbbf806cc1d1a70e79c624371c25208ba31e23dbd5d48ed6"
}
+1 -1
View File
@@ -1 +1 @@
7a256cf353db7cf64a60a09fa0de7f3a8b27f626
bdee9dd947448d16244e4941f1ba3847da0ff9c5
@@ -0,0 +1 @@
ALTER TABLE account DROP COLUMN IF EXISTS created_by;
@@ -0,0 +1,4 @@
-- Who connected an OAuth account. A variable may link an account only if its caller created it
-- or can already read a variable linking it. Existing accounts stay NULL: they can still be
-- linked through a readable variable, never claimed as unlinked.
ALTER TABLE account ADD COLUMN IF NOT EXISTS created_by VARCHAR(255);
@@ -588,13 +588,14 @@ async fn oauth_callback(
// 1. Create account record for token refresh
let account_id = sqlx::query_scalar!(
"INSERT INTO account (workspace_id, client, expires_at, refresh_token, is_workspace_integration)
VALUES ($1, $2, now() + ($3 || ' seconds')::interval, $4, true)
"INSERT INTO account (workspace_id, client, expires_at, refresh_token, is_workspace_integration, created_by)
VALUES ($1, $2, now() + ($3 || ' seconds')::interval, $4, true, $5)
RETURNING id",
workspace_id,
service_name.as_str(),
expires_in.to_string(),
token_response.refresh_token.as_deref().unwrap_or(""),
authed.username,
)
.fetch_one(&mut *tx)
.await
+48 -1
View File
@@ -55,7 +55,7 @@ use crate::var_resource_cache::{
};
use lazy_static::lazy_static;
use serde::Deserialize;
use sqlx::{Acquire, Postgres, Transaction};
use sqlx::{Acquire, PgConnection, Postgres, Transaction};
use windmill_common::variables::encrypt;
use windmill_git_sync::{handle_deployment_metadata, DeployedObject};
@@ -585,6 +585,47 @@ fn validate_already_encrypted_secret(path: &str, value: &str) -> Result<()> {
Ok(())
}
/// Reading a variable linked to an OAuth account refreshes through that account and hands the
/// fresh token to the reader. So only the account's creator may link it, or a caller who can
/// already read a variable that does (`tx` is the caller's transaction, so RLS decides that).
async fn require_account_link_allowed(
db: &DB,
tx: &mut PgConnection,
w_id: &str,
account: Option<i32>,
username: &str,
) -> Result<()> {
let Some(account) = account else {
return Ok(());
};
let created_by = sqlx::query_scalar!(
"SELECT created_by FROM account WHERE workspace_id = $1 AND id = $2",
w_id,
account
)
.fetch_optional(db)
.await?
.flatten();
if created_by.as_deref() == Some(username) {
return Ok(());
}
let readable = sqlx::query_scalar!(
"SELECT EXISTS(SELECT 1 FROM variable WHERE workspace_id = $1 AND account = $2)",
w_id,
account
)
.fetch_one(&mut *tx)
.await?
.unwrap_or(false);
if readable {
Ok(())
} else {
Err(Error::NotAuthorized(format!(
"OAuth account {account} was connected by someone else"
)))
}
}
async fn create_variable(
authed: ApiAuthed,
Extension(db): Extension<DB>,
@@ -646,6 +687,8 @@ async fn create_variable(
let mut tx = user_db.begin(&authed).await?;
require_account_link_allowed(&db, &mut tx, &w_id, variable.account, &authed.username).await?;
sqlx::query!(
"INSERT INTO variable
(workspace_id, path, value, is_secret, description, account, is_oauth, expires_at, labels, edited_by)
@@ -1195,6 +1238,10 @@ async fn update_variable(
let mut tx: Transaction<'_, Postgres> = user_db.begin(&authed).await?;
if ns.account != old_account_id {
require_account_link_allowed(&db, &mut tx, &w_id, ns.account, &authed.username).await?;
}
if let Some(npath) = ns.path.clone() {
if npath != path {
check_path_conflict(&db, &w_id, &npath).await?;