mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 08:02:40 +00:00
fix: guest tokens are not rescopable and guest embed tokens keep the sentinel
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5
This commit is contained in:
co-authored by
Claude Opus 5
parent
4766f979dc
commit
d78f7f6709
-29
@@ -1,29 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT path, policy->>'execution_mode' as execution_mode\n FROM app WHERE id = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "path",
|
||||
"type_info": "Varchar"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "execution_mode",
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false,
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "126f72d28d1bfcea764311f3aedb2d1bbe51c9d5ba2cd12890de867ac51573ea"
|
||||
}
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE token SET label = $1\n WHERE email = $2 AND token_prefix = $3\n AND (label IS NULL OR (\n label <> 'session'\n AND lower(label) NOT LIKE 'ephemeral%'\n AND label <> 'debugger-token'\n AND label NOT LIKE 'mcp-oauth-%'\n ))\n RETURNING token_prefix",
|
||||
"query": "UPDATE token SET label = $1\n WHERE email = $2 AND token_prefix = $3\n AND (label IS NULL OR (\n label <> 'session'\n AND label <> 'guest_session'\n AND lower(label) NOT LIKE 'ephemeral%'\n AND label <> 'debugger-token'\n AND label NOT LIKE 'mcp-oauth-%'\n ))\n RETURNING token_prefix",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -20,5 +20,5 @@
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "c0da3f1f2c55900dbdf92b16ebbfdb7b4cc11a648460f175e4f57d080a0005a5"
|
||||
"hash": "31ed2fb85c0c726e3cf6392be2a73c62bae004b2842a4828c6807232570f83a1"
|
||||
}
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT path FROM app WHERE id = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "path",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "391139a04bd48319a5512e7859b63e81438c7483ac892b971fe8a20709555cc1"
|
||||
}
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE token SET scopes = $1\n WHERE email = $2 AND token_prefix = $3\n RETURNING token_prefix",
|
||||
"query": "UPDATE token SET scopes = $1\n WHERE email = $2 AND token_prefix = $3\n AND (label IS NULL OR label <> 'guest_session')\n RETURNING token_prefix",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -20,5 +20,5 @@
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "a7a20412e303568b271f949642de55e9880ef05786fe59f05de5e025ef315726"
|
||||
"hash": "d17645b5001d7f8da1dc451c5d35ea3c9346271b8404863256071cfdf884036a"
|
||||
}
|
||||
@@ -314,14 +314,16 @@ async fn a_self_declared_guest_scope_grants_nothing(db: Pool<Postgres>) -> anyho
|
||||
}
|
||||
|
||||
/// A guest-mode policy that names one runnable, so an `execute_component` request
|
||||
/// gets past the triggerables lookup and reaches the guest gate.
|
||||
fn guest_app_with_runnable(path: &str) -> serde_json::Value {
|
||||
/// gets past the triggerables lookup and reaches the guest gate. `sandbox` is what
|
||||
/// makes the embed-token endpoint actually mint a token.
|
||||
fn guest_app_with_runnable(path: &str, sandbox: bool) -> serde_json::Value {
|
||||
json!({
|
||||
"path": path,
|
||||
"summary": "Guest app",
|
||||
"value": {},
|
||||
"policy": {
|
||||
"execution_mode": "guest",
|
||||
"sandbox": sandbox,
|
||||
"triggerables_v2": {
|
||||
"script/u/test-user/noop": { "static_inputs": {}, "one_of_inputs": {} }
|
||||
}
|
||||
@@ -356,7 +358,7 @@ async fn execute_component_re_checks_the_workspace_switch(
|
||||
let ws = format!("http://localhost:{port}/api/w/test-workspace");
|
||||
|
||||
let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
|
||||
.json(&guest_app_with_runnable(APP_PATH))
|
||||
.json(&guest_app_with_runnable(APP_PATH, false))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201, "{}", resp.text().await?);
|
||||
@@ -413,7 +415,7 @@ async fn guest_cannot_run_another_guest_app(db: Pool<Postgres>) -> anyhow::Resul
|
||||
.await?;
|
||||
let other = "u/test-user/other_guest_app";
|
||||
let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
|
||||
.json(&guest_app_with_runnable(other))
|
||||
.json(&guest_app_with_runnable(other, false))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201, "{}", resp.text().await?);
|
||||
@@ -430,3 +432,107 @@ async fn guest_cannot_run_another_guest_app(db: Pool<Postgres>) -> anyhow::Resul
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The embed token a guest mints for a sandboxed app is the one credential handed to
|
||||
/// untrusted app JS. It must be a guest twice over — resolve like its minter (the
|
||||
/// label) and be governed like its minter (the sentinel) — or every guest control
|
||||
/// silently skips the most exposed credential there is.
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn a_guest_minted_embed_token_stays_a_guest(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let ws = format!("http://localhost:{port}/api/w/test-workspace");
|
||||
|
||||
authed(
|
||||
client().post(format!("{ws}/workspaces/edit_guest_access")),
|
||||
ADMIN_TOKEN,
|
||||
)
|
||||
.json(&json!({ "guest_access_enabled": true }))
|
||||
.send()
|
||||
.await?;
|
||||
let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
|
||||
.json(&guest_app_with_runnable(APP_PATH, true))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201, "{}", resp.text().await?);
|
||||
let secret: String = authed(
|
||||
client().get(format!("{ws}/apps/secret_of/{APP_PATH}")),
|
||||
ADMIN_TOKEN,
|
||||
)
|
||||
.send()
|
||||
.await?
|
||||
.text()
|
||||
.await?;
|
||||
insert_guest_token(&db, "test-workspace").await?;
|
||||
|
||||
// The guest page mints the iframe's token from the guest session.
|
||||
let resp = authed(
|
||||
client().get(format!("{ws}/apps_u/embed_token/{secret}")),
|
||||
GUEST_TOKEN,
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "a guest must be able to mint: {}", resp.text().await?);
|
||||
let body: serde_json::Value = resp.json().await?;
|
||||
let embed = body["token"]
|
||||
.as_str()
|
||||
.expect("mint must return a token for an authenticated guest")
|
||||
.to_string();
|
||||
|
||||
// Resolves — and as a guest, not as the non-member superadmin shape.
|
||||
let resp = authed(client().get(format!("{ws}/users/whoami")), &embed)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "the minted token must authenticate");
|
||||
let me: serde_json::Value = resp.json().await?;
|
||||
assert_eq!(me["role"], json!("guest"));
|
||||
|
||||
// Governed: the workspace switch closes it, iframe or not.
|
||||
authed(
|
||||
client().post(format!("{ws}/workspaces/edit_guest_access")),
|
||||
ADMIN_TOKEN,
|
||||
)
|
||||
.json(&json!({ "guest_access_enabled": false }))
|
||||
.send()
|
||||
.await?;
|
||||
let resp = execute(port, "test-workspace", APP_PATH, &embed)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
403,
|
||||
"turning guests off must stop a guest's embed token running components"
|
||||
);
|
||||
|
||||
// And its scopes are not something the guest's email can later rewrite. The
|
||||
// guest session itself cannot reach `/users/*` (workspace pin), so model the real
|
||||
// threat: the same email after promotion, holding an ordinary unpinned session.
|
||||
sqlx::query(
|
||||
"INSERT INTO token (token_hash, token_prefix, token, email, label)
|
||||
VALUES (encode(sha256($1::bytea), 'hex'), 'PROMOTED_S', $2, 'guest@example.com',
|
||||
'session')",
|
||||
)
|
||||
.bind(b"PROMOTED_SESSION".as_slice())
|
||||
.bind("PROMOTED_SESSION")
|
||||
.execute(&db)
|
||||
.await?;
|
||||
for prefix in [&embed[..10], &GUEST_TOKEN[..10]] {
|
||||
let resp = authed(
|
||||
client().post(format!(
|
||||
"http://localhost:{port}/api/users/tokens/update_scopes/{prefix}"
|
||||
)),
|
||||
"PROMOTED_SESSION",
|
||||
)
|
||||
.json(&json!({ "scopes": null }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
404,
|
||||
"a promoted account must not be able to rescope its old guest credentials"
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2931,7 +2931,7 @@ lazy_static::lazy_static! {
|
||||
/// there is nothing to disable when the workspace revokes guest access or the
|
||||
/// identity provider removes them — the expiry is the revocation. Much shorter
|
||||
/// than a member session for that reason.
|
||||
static ref GUEST_SESSION_VALIDITY_SECONDS: i64 = std::env::var("GUEST_SESSION_VALIDITY_SECONDS")
|
||||
pub static ref GUEST_SESSION_VALIDITY_SECONDS: i64 = std::env::var("GUEST_SESSION_VALIDITY_SECONDS")
|
||||
.ok()
|
||||
.and_then(|x| x.parse::<i64>().ok())
|
||||
.unwrap_or(8 * 60 * 60);
|
||||
@@ -3311,9 +3311,15 @@ async fn update_token_scopes(
|
||||
|
||||
let mut tx = db.begin().await?;
|
||||
|
||||
// A guest session's scopes are its entire confinement — the label grants the
|
||||
// identity, the scopes bound it to one app. Once the same email holds a real
|
||||
// account (promotion), that account could otherwise rescope the still-valid
|
||||
// guest token into an unconfined non-member credential, so a guest-labelled
|
||||
// token is not rescoped by anyone. Same shape as the relabel guard.
|
||||
let updated: Option<String> = sqlx::query_scalar!(
|
||||
"UPDATE token SET scopes = $1
|
||||
WHERE email = $2 AND token_prefix = $3
|
||||
AND (label IS NULL OR label <> 'guest_session')
|
||||
RETURNING token_prefix",
|
||||
req.scopes.as_deref(),
|
||||
&authed.email,
|
||||
@@ -3324,7 +3330,7 @@ async fn update_token_scopes(
|
||||
|
||||
let prefix = updated.ok_or_else(|| {
|
||||
Error::NotFound(format!(
|
||||
"token {token_prefix} not found or not owned by user"
|
||||
"token {token_prefix} not found, not owned by user, or not rescopable"
|
||||
))
|
||||
})?;
|
||||
|
||||
|
||||
@@ -4610,11 +4610,11 @@ struct EditGuestAccess {
|
||||
}
|
||||
|
||||
/// Turn guest sessions on or off for this workspace. Off by default, and off is
|
||||
/// authoritative: an app whose policy already says `guest` stops admitting them at
|
||||
/// the next sign-in, because the switch is checked where the session is minted.
|
||||
///
|
||||
/// Sessions already handed out are not revoked — a guest has no account to disable —
|
||||
/// and run out on their own (`GUEST_SESSION_VALIDITY_SECONDS`).
|
||||
/// authoritative and immediate: the switch is re-read where a guest session is
|
||||
/// minted, where a guest reads an app and where a guest runs its components
|
||||
/// (`guest_app_admits` / `is_guest_access_enabled`), so an app whose policy already
|
||||
/// says `guest` — pushed by git-sync, say — closes to guests on the next request,
|
||||
/// sessions already issued included.
|
||||
async fn edit_guest_access(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
|
||||
@@ -1654,8 +1654,18 @@ pub async fn mint_app_embed_token(
|
||||
"App embed tokens cannot mint or renew embed tokens".to_string(),
|
||||
));
|
||||
}
|
||||
let expiration =
|
||||
chrono::Utc::now() + chrono::Duration::hours(APP_EMBED_TOKEN_VALIDITY_HOURS);
|
||||
let is_guest_minter =
|
||||
windmill_api_auth::scopes::has_guest_sentinel(authed.scopes.as_deref());
|
||||
// A guest's embed token must not outlive the kind of session that minted it.
|
||||
let validity = if is_guest_minter {
|
||||
chrono::Duration::seconds(
|
||||
(*windmill_api_users::users::GUEST_SESSION_VALIDITY_SECONDS)
|
||||
.min(APP_EMBED_TOKEN_VALIDITY_HOURS * 3600),
|
||||
)
|
||||
} else {
|
||||
chrono::Duration::hours(APP_EMBED_TOKEN_VALIDITY_HOURS)
|
||||
};
|
||||
let expiration = chrono::Utc::now() + validity;
|
||||
let mut scopes: Vec<String> = APP_EMBED_SCOPES
|
||||
.iter()
|
||||
.filter(|s| **s != windmill_api_auth::scopes::APP_EMBED_SENTINEL)
|
||||
@@ -1677,12 +1687,15 @@ pub async fn mint_app_embed_token(
|
||||
// guest session is. `mint_raw_app_sdk_token` has the same shape.
|
||||
ensure_scopes_within_caller(authed, Some(&scopes))?;
|
||||
scopes.push(windmill_api_auth::scopes::APP_EMBED_SENTINEL.to_string());
|
||||
// An embed token minted by a guest has to resolve the same way the guest's
|
||||
// own session does — through the label, since there is no `usr` row behind
|
||||
// the email. With the ordinary label the iframe's every request would 401.
|
||||
// The `app_embed` sentinel pushed above still confines it more tightly than
|
||||
// the session that minted it.
|
||||
let label = if windmill_api_auth::scopes::has_guest_sentinel(authed.scopes.as_deref()) {
|
||||
// A guest's embed token is a guest twice over. The label is what lets it
|
||||
// resolve at all — there is no `usr` row behind the email, so `AuthCache`
|
||||
// admits it the same way it admits the session that minted it. The `guest`
|
||||
// sentinel is what keeps every guest control on it: the workspace switch, the
|
||||
// rate limit, the `whoami` role all key on the sentinel, and this is the one
|
||||
// credential handed to untrusted app JS, so it must be at least as confined
|
||||
// as its minter. Both sentinels compose — each is a default-deny allowlist.
|
||||
let label = if is_guest_minter {
|
||||
scopes.push(windmill_api_auth::scopes::GUEST_SENTINEL.to_string());
|
||||
windmill_common::auth::GUEST_SESSION_LABEL.to_string()
|
||||
} else {
|
||||
format!("embed_app:{app_path}")
|
||||
@@ -1738,17 +1751,14 @@ async fn get_guest_entry(
|
||||
) -> JsonResult<GuestEntry> {
|
||||
let id = get_id_from_secret(&db, &w_id, secret, None).await?;
|
||||
let app = sqlx::query!(
|
||||
"SELECT path, policy->>'execution_mode' as execution_mode
|
||||
FROM app WHERE id = $1 AND workspace_id = $2",
|
||||
"SELECT path FROM app WHERE id = $1 AND workspace_id = $2",
|
||||
id,
|
||||
&w_id
|
||||
)
|
||||
.fetch_optional(&db)
|
||||
.await?;
|
||||
let app = not_found_if_none(app, "App", id.to_string())?;
|
||||
if app.execution_mode.as_deref() != Some(ExecutionMode::Guest.as_str())
|
||||
|| !windmill_common::workspaces::is_guest_access_enabled(&db, &w_id).await?
|
||||
{
|
||||
if !windmill_common::workspaces::guest_app_admits(&db, &w_id, &app.path).await? {
|
||||
return Err(Error::NotFound("App is not open to guests".to_string()));
|
||||
}
|
||||
Ok(Json(GuestEntry { workspace_id: w_id, app_path: app.path }))
|
||||
|
||||
@@ -606,16 +606,17 @@
|
||||
}, 1500)
|
||||
}
|
||||
|
||||
/** Mirrors the server-side write in the OAuth `login` handler, including clearing
|
||||
* it when this sign-in is not a guest entry. `login_externally` consumes it.
|
||||
* `Secure` only over https, as the backend does with its own cookies: a browser
|
||||
* drops a `Secure` cookie set from an http origin, and a SAML guest sign-in there
|
||||
* would then silently provision a real account instead. Lax suffices, since the
|
||||
* ACS redirect that consumes it is a same-site top-level navigation. */
|
||||
/** Mirrors the server-side write in the OAuth `login` handler (`set_unsensitive_cookie`)
|
||||
* attribute for attribute, including clearing it when this sign-in is not a guest
|
||||
* entry. `login_externally` consumes it. `SameSite=None` is required, not a choice:
|
||||
* the SAML ACS is a cross-site POST from the IdP, and a Lax cookie is not sent on
|
||||
* those. `None` needs `Secure`, so — exactly like the backend's own cookie — this
|
||||
* only survives the round trip over https; over plain http the browser drops it
|
||||
* and a guest sign-in falls through to ordinary provisioning. */
|
||||
function setGuestAppCookie(value: string | undefined) {
|
||||
try {
|
||||
const secure = window.location.protocol === 'https:' ? '; Secure' : ''
|
||||
document.cookie = `guest_app=${encodeURIComponent(value ?? '')}; path=/; SameSite=Lax${secure}`
|
||||
document.cookie = `guest_app=${encodeURIComponent(value ?? '')}; path=/; SameSite=None${secure}`
|
||||
} catch (e) {
|
||||
console.error('Could not set the guest app cookie', e)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user