mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-13 08:05:23 +00:00
fix(auth): filter resource/variable listings by token scope (WIN-1981) (#9302)
A token scoped to a single resource (e.g. `resources:read:u/alice/foo`)
could call `GET /api/w/{w}/resources/list_search` and receive `path` and
`value` for unrelated resources in the workspace. Route-level scope
checks only validate `domain:action`; per-resource handlers do a
`check_scopes` against the path, but the listing endpoints did not —
leaking integration credentials, API keys, and other secrets stored as
resource values to narrowly-scoped tokens.
Add `build_scope_path_predicate` to `windmill-api-auth` (mirrors
`check_scopes` semantics but parses the token's scopes once, suitable
for filtering many rows). Apply it to `list_search_resources`,
`list_resources`, `list_names` (resources) and `list_variables`
(non-secret value leak), so a scope-restricted token only ever sees the
paths it is authorized to read. Unscoped tokens and tokens whose only
scopes are `if_jobs:filter_tags:*` are unaffected.
Includes regression tests covering: unscoped, tag-filter-only,
single-resource, wildcard, wrong-domain, and write-implies-read.
Fixes WIN-1981
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
tristantr
co-authored by
Claude Opus 4.7
parent
2df3dd09dd
commit
b5a17bdcb7
@@ -235,6 +235,50 @@ where
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Returns a predicate that checks whether `path` is within the token's
|
||||
/// scope for `{domain}:{action}:{path}`. For tokens without scope
|
||||
/// restrictions (no scopes at all, or only `if_jobs:filter_tags:*` scopes),
|
||||
/// the predicate always returns `true`.
|
||||
///
|
||||
/// Pre-parses the token's scopes once so the returned closure can cheaply
|
||||
/// filter large listings without re-parsing on each call.
|
||||
pub fn build_scope_path_predicate(
|
||||
authed: &ApiAuthed,
|
||||
domain: &str,
|
||||
action: &str,
|
||||
) -> impl Fn(&str) -> bool {
|
||||
// Mirror check_scopes semantics: a token is "scope-restricted" iff it has
|
||||
// at least one non-`if_jobs:filter_tags:` scope. Unparseable scopes still
|
||||
// count as restrictive — they just match nothing.
|
||||
let (is_scoped_token, parsed): (bool, Vec<ScopeDefinition>) = match authed.scopes.as_ref() {
|
||||
Some(scopes) => {
|
||||
let mut is_scoped = false;
|
||||
let parsed = scopes
|
||||
.iter()
|
||||
.filter(|s| !s.starts_with("if_jobs:filter_tags:"))
|
||||
.inspect(|_| is_scoped = true)
|
||||
.filter_map(|s| ScopeDefinition::from_scope_string(s).ok())
|
||||
.collect();
|
||||
(is_scoped, parsed)
|
||||
}
|
||||
None => (false, Vec::new()),
|
||||
};
|
||||
let domain = domain.to_string();
|
||||
let action = action.to_string();
|
||||
|
||||
move |path: &str| -> bool {
|
||||
if !is_scoped_token {
|
||||
return true;
|
||||
}
|
||||
let required =
|
||||
match ScopeDefinition::from_scope_string(&format!("{}:{}:{}", domain, action, path)) {
|
||||
Ok(r) => r,
|
||||
Err(_) => return false,
|
||||
};
|
||||
parsed.iter().any(|s| s.includes(&required))
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn require_devops_role(db: &DB, email: &str) -> error::Result<()> {
|
||||
let is_devops = is_devops_email(db, email).await?;
|
||||
|
||||
@@ -803,3 +847,65 @@ pub fn require_path_read_access_for_preview(
|
||||
))),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn authed_with_scopes(scopes: Option<Vec<&str>>) -> ApiAuthed {
|
||||
ApiAuthed {
|
||||
scopes: scopes.map(|v| v.into_iter().map(String::from).collect()),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn predicate_no_scopes_allows_all() {
|
||||
let authed = authed_with_scopes(None);
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
assert!(allowed("u/alice/anything"));
|
||||
assert!(allowed("u/bob/other"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn predicate_tag_filter_only_allows_all() {
|
||||
let authed = authed_with_scopes(Some(vec!["if_jobs:filter_tags:default"]));
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
assert!(allowed("u/alice/foo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn predicate_single_resource_scope_filters_others() {
|
||||
// Regression test for WIN-1981: a token scoped to one resource must
|
||||
// not match unrelated paths in listings (e.g. /resources/list_search).
|
||||
let authed = authed_with_scopes(Some(vec!["resources:read:u/alice/allowed_resource"]));
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
assert!(allowed("u/alice/allowed_resource"));
|
||||
assert!(!allowed("u/alice/other_resource"));
|
||||
assert!(!allowed("u/bob/foo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn predicate_wildcard_scope_matches_subtree() {
|
||||
let authed = authed_with_scopes(Some(vec!["resources:read:f/team/*"]));
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
assert!(allowed("f/team/db"));
|
||||
assert!(allowed("f/team/sub/nested"));
|
||||
assert!(!allowed("f/other/db"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn predicate_wrong_domain_is_rejected() {
|
||||
let authed = authed_with_scopes(Some(vec!["variables:read:u/alice/secret"]));
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
assert!(!allowed("u/alice/secret"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn predicate_write_implies_read() {
|
||||
let authed = authed_with_scopes(Some(vec!["resources:write:u/alice/foo"]));
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
assert!(allowed("u/alice/foo"));
|
||||
assert!(!allowed("u/alice/bar"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,8 +10,8 @@ use std::collections::HashMap;
|
||||
use std::net::IpAddr;
|
||||
|
||||
use windmill_api_auth::{
|
||||
check_scopes, maybe_refresh_folders, require_owner_of_path, require_super_admin, ApiAuthed,
|
||||
Tokened,
|
||||
build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path,
|
||||
require_super_admin, ApiAuthed, Tokened,
|
||||
};
|
||||
use windmill_common::db::DB;
|
||||
use windmill_common::workspaces::{check_deploy_rules, RuleCheckResult};
|
||||
@@ -194,6 +194,7 @@ async fn list_names(
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
) -> JsonResult<Vec<NamePath>> {
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
let rows = sqlx::query!(
|
||||
"SELECT value->>'name' as name, path from resource WHERE resource_type = $1 AND workspace_id = $2",
|
||||
rt,
|
||||
@@ -203,6 +204,7 @@ async fn list_names(
|
||||
.await?
|
||||
.into_iter()
|
||||
.filter_map(|x| x.name.map(|name| NamePath { name, path: x.path }))
|
||||
.filter(|np| allowed(&np.path))
|
||||
.collect::<Vec<_>>();
|
||||
tx.commit().await?;
|
||||
Ok(Json(rows))
|
||||
@@ -225,6 +227,7 @@ async fn list_search_resources(
|
||||
#[cfg(not(feature = "enterprise"))]
|
||||
let n = 3;
|
||||
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
let rows = sqlx::query_as!(
|
||||
SearchResource,
|
||||
"SELECT path, value from resource WHERE workspace_id = $1 LIMIT $2",
|
||||
@@ -234,6 +237,7 @@ async fn list_search_resources(
|
||||
.fetch_all(&mut *tx)
|
||||
.await?
|
||||
.into_iter()
|
||||
.filter(|r| allowed(&r.path))
|
||||
.collect::<Vec<_>>();
|
||||
tx.commit().await?;
|
||||
Ok(Json(rows))
|
||||
@@ -338,9 +342,13 @@ async fn list_resources(
|
||||
|
||||
let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
let allowed = build_scope_path_predicate(&authed, "resources", "read");
|
||||
let rows = sqlx::query_as::<_, ListableResource>(&sql)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
.await?
|
||||
.into_iter()
|
||||
.filter(|r| allowed(&r.path))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
|
||||
@@ -6,7 +6,10 @@
|
||||
* LICENSE-AGPL for a copy of the license.
|
||||
*/
|
||||
|
||||
use windmill_api_auth::{check_scopes, maybe_refresh_folders, require_owner_of_path, ApiAuthed};
|
||||
use windmill_api_auth::{
|
||||
build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path,
|
||||
ApiAuthed,
|
||||
};
|
||||
use windmill_common::db::DB;
|
||||
use windmill_common::workspaces::{check_deploy_rules, RuleCheckResult};
|
||||
|
||||
@@ -188,9 +191,13 @@ async fn list_variables(
|
||||
|
||||
let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
let allowed = build_scope_path_predicate(&authed, "variables", "read");
|
||||
let rows = sqlx::query_as::<_, ListableVariable>(&sql)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
.await?
|
||||
.into_iter()
|
||||
.filter(|r| allowed(&r.path))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
tx.commit().await?;
|
||||
Ok(Json(rows))
|
||||
|
||||
Reference in New Issue
Block a user