mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-24 16:01:42 +00:00
fix(auth): enforce monotonic privilege on user token lifecycle endpoints (#9371)
This commit is contained in:
+22
@@ -0,0 +1,22 @@
|
|||||||
|
{
|
||||||
|
"db_name": "PostgreSQL",
|
||||||
|
"query": "INSERT INTO token\n (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, read_only)\n VALUES ($1, $2, $3, $4, $5, now() + ($6 || ' seconds')::interval, $7, $8, $9)",
|
||||||
|
"describe": {
|
||||||
|
"columns": [],
|
||||||
|
"parameters": {
|
||||||
|
"Left": [
|
||||||
|
"Varchar",
|
||||||
|
"Varchar",
|
||||||
|
"Varchar",
|
||||||
|
"Varchar",
|
||||||
|
"Varchar",
|
||||||
|
"Text",
|
||||||
|
"Bool",
|
||||||
|
"TextArray",
|
||||||
|
"Bool"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"nullable": []
|
||||||
|
},
|
||||||
|
"hash": "7f832370916794ab0e5645053688c24678f1519d49ee7263a86dba71d45b8e8c"
|
||||||
|
}
|
||||||
@@ -1 +1 @@
|
|||||||
08e3b9b818f9f1b439e9a6ba477fd5fdbcab944e
|
3742e0659c5e97aab03b9efeea14cd94a3ac658a
|
||||||
|
|||||||
@@ -235,6 +235,202 @@ where
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns the caller's "real" scope restrictions: every scope other than
|
||||||
|
/// `if_jobs:filter_tags:` tag filters. `None` means the token is unscoped and
|
||||||
|
/// has the full privileges of its user; `Some` means it is restricted to the
|
||||||
|
/// returned scopes. An empty or filter-tags-only scope list is treated as
|
||||||
|
/// unscoped, mirroring `check_scopes`/`check_route_access`.
|
||||||
|
fn scope_restrictions(scopes: Option<&[String]>) -> Option<Vec<&String>> {
|
||||||
|
let restrictions: Vec<&String> = scopes?
|
||||||
|
.iter()
|
||||||
|
.filter(|s| !s.starts_with("if_jobs:filter_tags:"))
|
||||||
|
.collect();
|
||||||
|
(!restrictions.is_empty()).then_some(restrictions)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Enforce monotonic privilege when a token lifecycle endpoint mints or rescopes
|
||||||
|
/// a credential on behalf of `authed`: the resulting credential must never be
|
||||||
|
/// more privileged than the caller's own token.
|
||||||
|
///
|
||||||
|
/// - An unscoped caller may grant any scopes (this is the existing UI/CLI flow).
|
||||||
|
/// - A scope-restricted caller may only grant scopes that are a subset of its
|
||||||
|
/// own, and may never produce an unscoped credential.
|
||||||
|
///
|
||||||
|
/// Without this, a `users:write` token could create or rescope a token to be
|
||||||
|
/// unscoped, and a `users:read` token could refresh into an unscoped session —
|
||||||
|
/// escaping its own restrictions.
|
||||||
|
pub fn ensure_scopes_within_caller(
|
||||||
|
authed: &ApiAuthed,
|
||||||
|
requested_scopes: Option<&[String]>,
|
||||||
|
) -> error::Result<()> {
|
||||||
|
if let Some(caller_restrictions) = scope_restrictions(authed.scopes.as_deref()) {
|
||||||
|
let Some(requested_restrictions) = scope_restrictions(requested_scopes) else {
|
||||||
|
return Err(Error::PermissionDenied(
|
||||||
|
"A scope-restricted token cannot create or update a token with broader (unscoped) \
|
||||||
|
privileges"
|
||||||
|
.to_string(),
|
||||||
|
));
|
||||||
|
};
|
||||||
|
|
||||||
|
// MCP scopes (`mcp:all`, `mcp:favorites`, `mcp:scripts:*`, etc.) use a
|
||||||
|
// custom format that ScopeDefinition::from_scope_string parses
|
||||||
|
// permissively but the MCP runtime interprets via its own parser
|
||||||
|
// (parse_mcp_scopes). The two views disagree — e.g. the generic parser
|
||||||
|
// accepts `mcp:scripts` as an unrestricted-resource scope, while the
|
||||||
|
// MCP runtime ignores it as unrecognized but interprets `mcp:scripts:*`
|
||||||
|
// as granting all scripts. So generic containment would silently allow
|
||||||
|
// `mcp:scripts` → `mcp:scripts:*` (a widening). Legitimate MCP token
|
||||||
|
// issuance goes through the OAuth gateway (mcp/oauth_server.rs), not
|
||||||
|
// these user-token endpoints, so require byte-identical match for MCP
|
||||||
|
// scopes here rather than trying to mirror MCP semantics in two places.
|
||||||
|
// Unparseable non-MCP caller scopes are intentionally dropped
|
||||||
|
// (fail-closed): a caller scope that fails to parse can only narrow
|
||||||
|
// the set of requested scopes that get covered, never widen it.
|
||||||
|
// Unparseable requested scopes surface as `BadRequest`, which is what
|
||||||
|
// we want — the client is sending garbage.
|
||||||
|
let parsed_caller: Vec<ScopeDefinition> = caller_restrictions
|
||||||
|
.iter()
|
||||||
|
.filter(|s| !s.starts_with("mcp:"))
|
||||||
|
.filter_map(|s| ScopeDefinition::from_scope_string(s).ok())
|
||||||
|
.collect();
|
||||||
|
let caller_mcp: std::collections::HashSet<&str> = caller_restrictions
|
||||||
|
.iter()
|
||||||
|
.filter(|s| s.starts_with("mcp:"))
|
||||||
|
.map(|s| s.as_str())
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
for requested in requested_restrictions {
|
||||||
|
if requested.starts_with("mcp:") {
|
||||||
|
if !caller_mcp.contains(requested.as_str()) {
|
||||||
|
return Err(Error::PermissionDenied(format!(
|
||||||
|
"A scope-restricted token cannot grant MCP scope '{requested}' unless the \
|
||||||
|
caller holds the same scope verbatim"
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let requested_scope = ScopeDefinition::from_scope_string(requested)?;
|
||||||
|
let covered = parsed_caller
|
||||||
|
.iter()
|
||||||
|
.any(|caller_scope| scope_contains(caller_scope, &requested_scope));
|
||||||
|
if !covered {
|
||||||
|
return Err(Error::PermissionDenied(format!(
|
||||||
|
"A scope-restricted token cannot grant scope '{requested}' which exceeds its \
|
||||||
|
own scopes"
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// `if_jobs:filter_tags:` fences which job tags a token can run on (enforced
|
||||||
|
// at job operations as `v2_job.tag = ANY(...)`), and is checked independently
|
||||||
|
// of domain/action/resource subset. A caller restricted by filter_tags must
|
||||||
|
// not be able to mint or rescope a credential that drops or widens the fence
|
||||||
|
// — even if the caller has no other scope restrictions (filter_tags-only
|
||||||
|
// tokens otherwise look "unscoped" to `scope_restrictions`).
|
||||||
|
if let Some(caller_tags) = first_filter_tags(authed.scopes.as_deref()) {
|
||||||
|
let Some(requested_tags) = first_filter_tags(requested_scopes) else {
|
||||||
|
return Err(Error::PermissionDenied(
|
||||||
|
"A token restricted by if_jobs:filter_tags cannot mint or rescope a token that \
|
||||||
|
drops the tag restriction"
|
||||||
|
.to_string(),
|
||||||
|
));
|
||||||
|
};
|
||||||
|
let caller_set: std::collections::HashSet<&str> = caller_tags.iter().copied().collect();
|
||||||
|
for tag in &requested_tags {
|
||||||
|
if !caller_set.contains(tag) {
|
||||||
|
return Err(Error::PermissionDenied(format!(
|
||||||
|
"A token restricted by if_jobs:filter_tags cannot grant tag '{tag}' which is \
|
||||||
|
not within its own filter_tags"
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Tags from the first `if_jobs:filter_tags:<a,b,...>` scope, matching the
|
||||||
|
/// semantics of [`get_scope_tags`] (which is what the job runtime consults).
|
||||||
|
/// Returns `None` if no such scope is present.
|
||||||
|
fn first_filter_tags(scopes: Option<&[String]>) -> Option<Vec<&str>> {
|
||||||
|
scopes?.iter().find_map(|s| {
|
||||||
|
s.strip_prefix("if_jobs:filter_tags:")
|
||||||
|
.map(|tags| tags.split(',').collect())
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether `caller` grants at least everything `requested` grants (directional
|
||||||
|
/// containment).
|
||||||
|
///
|
||||||
|
/// This is intentionally NOT `ScopeDefinition::includes`: that method answers
|
||||||
|
/// "does this scope grant access to a required action" using OR semantics over
|
||||||
|
/// resources (any overlap counts, and a `*` on either side matches), which is
|
||||||
|
/// correct for access checks but unsafe for subset checks — it would let a
|
||||||
|
/// token scoped to `scripts:read:f/team/a` mint `scripts:read:*` or
|
||||||
|
/// `scripts:read:f/team/a,f/other/b`. Subset containment instead requires that
|
||||||
|
/// EVERY requested resource is covered by SOME caller resource.
|
||||||
|
fn scope_contains(caller: &ScopeDefinition, requested: &ScopeDefinition) -> bool {
|
||||||
|
if caller.domain != requested.domain {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// write subsumes read; otherwise the action must match exactly.
|
||||||
|
match (caller.action.as_str(), requested.action.as_str()) {
|
||||||
|
(c, r) if c == r || (c == "write" && r == "read") => {}
|
||||||
|
_ => return false,
|
||||||
|
}
|
||||||
|
|
||||||
|
if caller.domain == "jobs" && caller.action == "run" {
|
||||||
|
match (&caller.kind, &requested.kind) {
|
||||||
|
(Some(caller_kind), Some(requested_kind)) if caller_kind != requested_kind => {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
// Caller pinned to a kind, but the request covers any kind.
|
||||||
|
(Some(_), None) => return false,
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
match (&caller.resource, &requested.resource) {
|
||||||
|
// Caller is unrestricted on resources: covers everything.
|
||||||
|
(None, _) => true,
|
||||||
|
// Caller is resource-restricted but the request is not: broader.
|
||||||
|
(Some(_), None) => false,
|
||||||
|
(Some(caller_resources), Some(requested_resources)) => {
|
||||||
|
resource_set_contains(caller_resources, requested_resources)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Every resource in `requested` must be covered by some resource in `caller`.
|
||||||
|
fn resource_set_contains(caller: &[String], requested: &[String]) -> bool {
|
||||||
|
if caller.iter().any(|r| r == "*") {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
requested
|
||||||
|
.iter()
|
||||||
|
.all(|req| req != "*" && caller.iter().any(|c| resource_covers(c, req)))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Directional: does the single caller resource pattern cover `requested`?
|
||||||
|
/// `caller` may be an exact path or a `<prefix>/*` subtree wildcard; `requested`
|
||||||
|
/// may itself be a subtree wildcard, in which case the whole requested subtree
|
||||||
|
/// must fall within the caller's subtree.
|
||||||
|
fn resource_covers(caller: &str, requested: &str) -> bool {
|
||||||
|
if caller == requested {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
let Some(prefix) = caller.strip_suffix("/*") else {
|
||||||
|
// An exact caller resource only covers itself (handled above).
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
let requested_base = requested.strip_suffix("/*").unwrap_or(requested);
|
||||||
|
requested_base == prefix
|
||||||
|
|| (requested_base.starts_with(prefix)
|
||||||
|
&& requested_base.as_bytes().get(prefix.len()) == Some(&b'/'))
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns a predicate that checks whether `path` is within the token's
|
/// Returns a predicate that checks whether `path` is within the token's
|
||||||
/// scope for `{domain}:{action}:{path}`. For tokens without scope
|
/// scope for `{domain}:{action}:{path}`. For tokens without scope
|
||||||
/// restrictions (no scopes at all, or only `if_jobs:filter_tags:*` scopes),
|
/// restrictions (no scopes at all, or only `if_jobs:filter_tags:*` scopes),
|
||||||
@@ -574,6 +770,13 @@ impl NewToken {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Low-level token mint shared by trusted callers (the user-facing
|
||||||
|
/// `tokens/create` handler and internal mints such as native-trigger webhook
|
||||||
|
/// tokens). It does NOT enforce that `token_config.scopes` is within the
|
||||||
|
/// caller's own scopes — callers exposed to untrusted input must call
|
||||||
|
/// [`ensure_scopes_within_caller`] first (internal narrowing mints intentionally
|
||||||
|
/// skip it, since their scopes derive from the action being authorized, not the
|
||||||
|
/// caller's token).
|
||||||
pub async fn create_token_internal(
|
pub async fn create_token_internal(
|
||||||
tx: &mut sqlx::PgConnection,
|
tx: &mut sqlx::PgConnection,
|
||||||
db: &DB,
|
db: &DB,
|
||||||
@@ -908,4 +1111,278 @@ mod tests {
|
|||||||
assert!(allowed("u/alice/foo"));
|
assert!(allowed("u/alice/foo"));
|
||||||
assert!(!allowed("u/alice/bar"));
|
assert!(!allowed("u/alice/bar"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn opt_scopes(scopes: Option<Vec<&str>>) -> Option<Vec<String>> {
|
||||||
|
scopes.map(|v| v.into_iter().map(String::from).collect())
|
||||||
|
}
|
||||||
|
|
||||||
|
// Regression tests for WIN-1999: scoped user tokens must not be able to
|
||||||
|
// mint or rescope credentials with broader privileges than themselves.
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unscoped_caller_can_grant_anything() {
|
||||||
|
let authed = authed_with_scopes(None);
|
||||||
|
assert!(ensure_scopes_within_caller(&authed, None).is_ok());
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["jobs:run:scripts"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn filter_tags_only_caller_is_unrestricted_on_domain_action_dimension() {
|
||||||
|
// The domain/action/resource subset check treats filter-tags-only as
|
||||||
|
// unrestricted, mirroring check_scopes/check_route_access. The tag
|
||||||
|
// dimension is checked separately (see filter_tags_dimension_is_monotonic).
|
||||||
|
let authed = authed_with_scopes(Some(vec!["if_jobs:filter_tags:default"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["users:write", "if_jobs:filter_tags:default"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn filter_tags_dimension_is_monotonic() {
|
||||||
|
// Caller restricted to tag fence "a" cannot drop the fence …
|
||||||
|
let single = authed_with_scopes(Some(vec!["if_jobs:filter_tags:a"]));
|
||||||
|
assert!(ensure_scopes_within_caller(&single, None).is_err());
|
||||||
|
assert!(
|
||||||
|
ensure_scopes_within_caller(&single, opt_scopes(Some(vec!["users:read"])).as_deref())
|
||||||
|
.is_err(),
|
||||||
|
"minting a token without filter_tags must be rejected"
|
||||||
|
);
|
||||||
|
// … cannot widen to a tag it lacks …
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&single,
|
||||||
|
opt_scopes(Some(vec!["if_jobs:filter_tags:a,b"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
// … and cannot mint a token fenced on a disjoint tag.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&single,
|
||||||
|
opt_scopes(Some(vec!["if_jobs:filter_tags:b"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
// Narrowing or matching the tag fence is allowed.
|
||||||
|
let multi = authed_with_scopes(Some(vec!["if_jobs:filter_tags:a,b"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&multi,
|
||||||
|
opt_scopes(Some(vec!["if_jobs:filter_tags:a"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&multi,
|
||||||
|
opt_scopes(Some(vec!["if_jobs:filter_tags:a,b"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// A caller with a real scope plus a tag fence cannot drop just the fence.
|
||||||
|
let mixed = authed_with_scopes(Some(vec!["jobs:run:scripts", "if_jobs:filter_tags:a"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&mixed,
|
||||||
|
opt_scopes(Some(vec!["jobs:run:scripts"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&mixed,
|
||||||
|
opt_scopes(Some(vec!["jobs:run:scripts", "if_jobs:filter_tags:a"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// An unrestricted caller may grant filter_tags freely.
|
||||||
|
let unscoped = authed_with_scopes(None);
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&unscoped,
|
||||||
|
opt_scopes(Some(vec!["if_jobs:filter_tags:x"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoped_caller_cannot_mint_unscoped_token() {
|
||||||
|
// Primitive 2 in the report: a users:write token minting an unscoped token.
|
||||||
|
let authed = authed_with_scopes(Some(vec!["users:write"]));
|
||||||
|
assert!(ensure_scopes_within_caller(&authed, None).is_err());
|
||||||
|
// Empty scope list is effectively unscoped and must also be rejected.
|
||||||
|
assert!(ensure_scopes_within_caller(&authed, Some(&[])).is_err());
|
||||||
|
// A scope list of only tag filters is effectively unscoped too.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["if_jobs:filter_tags:default"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoped_caller_cannot_remove_its_own_scopes() {
|
||||||
|
// Primitive 3 in the report: a users:write token setting its scopes to null.
|
||||||
|
let authed = authed_with_scopes(Some(vec!["users:write"]));
|
||||||
|
assert!(ensure_scopes_within_caller(&authed, None).is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoped_caller_cannot_grant_scope_it_lacks() {
|
||||||
|
let authed = authed_with_scopes(Some(vec!["users:write"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["jobs:run:scripts"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoped_caller_can_grant_subset_of_own_scopes() {
|
||||||
|
let authed = authed_with_scopes(Some(vec!["users:write", "jobs:run:scripts"]));
|
||||||
|
// Equal scope is allowed.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["jobs:run:scripts"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// write implies read, so a narrower read scope is allowed.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["users:read"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// Tag filters narrow further and are always permitted.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["users:read", "if_jobs:filter_tags:default"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoped_caller_cannot_broaden_resource_scope() {
|
||||||
|
let authed = authed_with_scopes(Some(vec!["scripts:read:f/team/*"]));
|
||||||
|
// Narrower resource within the subtree is allowed.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team/sub"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// A nested subtree within the caller's subtree is allowed.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team/sub/*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// The subtree root itself is allowed.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
// A path outside the subtree is rejected.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/other/x"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
// read caller cannot grant write.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&authed,
|
||||||
|
opt_scopes(Some(vec!["scripts:write:f/team/db"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn mcp_scopes_require_byte_identical_match() {
|
||||||
|
// Regression for the access-grant-OR vs runtime-MCP-parser confusion:
|
||||||
|
// ScopeDefinition treats `mcp:scripts` as an unrestricted-resource scope
|
||||||
|
// and `mcp:scripts:*` as a strictly narrower one, so generic containment
|
||||||
|
// would silently allow widening. The MCP runtime however ignores
|
||||||
|
// `mcp:scripts` (unrecognized) while `mcp:scripts:*` grants all scripts.
|
||||||
|
// Legitimate MCP token issuance is the OAuth gateway, not these
|
||||||
|
// user-token endpoints, so MCP scopes must match the caller verbatim.
|
||||||
|
|
||||||
|
// The bypass the reviewer flagged: malformed `mcp:scripts` would widen
|
||||||
|
// into the real `mcp:scripts:*` under generic containment.
|
||||||
|
let bypass = authed_with_scopes(Some(vec!["users:write", "mcp:scripts"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&bypass,
|
||||||
|
opt_scopes(Some(vec!["users:write", "mcp:scripts:*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
|
||||||
|
// A caller without any MCP scope cannot grant one (widening on the MCP
|
||||||
|
// dimension), even if the rest of the requested scopes are within reach.
|
||||||
|
let no_mcp = authed_with_scopes(Some(vec!["users:write"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&no_mcp,
|
||||||
|
opt_scopes(Some(vec!["users:write", "mcp:scripts:*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
|
||||||
|
// Byte-identical MCP scope passes; an additional non-matching MCP scope
|
||||||
|
// alongside it does not.
|
||||||
|
let mcp_caller = authed_with_scopes(Some(vec!["mcp:scripts:*"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&mcp_caller,
|
||||||
|
opt_scopes(Some(vec!["mcp:scripts:*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&mcp_caller,
|
||||||
|
opt_scopes(Some(vec!["mcp:scripts:*", "mcp:flows:*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
|
||||||
|
// Even a narrowing within MCP semantics (`mcp:all` → `mcp:scripts:*`)
|
||||||
|
// is rejected by the byte-identical rule. This is intentional — these
|
||||||
|
// endpoints are not the legitimate path for narrowing MCP tokens.
|
||||||
|
let mcp_all = authed_with_scopes(Some(vec!["mcp:all"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&mcp_all,
|
||||||
|
opt_scopes(Some(vec!["mcp:scripts:*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn scoped_caller_cannot_escalate_to_wildcard_or_superset() {
|
||||||
|
// Regression for the access-grant-OR vs subset-containment confusion:
|
||||||
|
// ScopeDefinition::includes would (incorrectly) allow all of these.
|
||||||
|
let star = authed_with_scopes(Some(vec!["scripts:read:f/team/a"]));
|
||||||
|
// Minting `*` from a single-path scope must be rejected.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&star,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
// Minting a broader subtree must be rejected.
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&star,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team/*"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
|
||||||
|
// A comma-separated list that adds an uncovered resource must be rejected,
|
||||||
|
// even though one element overlaps the caller's scope.
|
||||||
|
let list = authed_with_scopes(Some(vec!["scripts:read:f/team/a"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&list,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team/a,f/other/b"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_err());
|
||||||
|
|
||||||
|
// A subset of a multi-resource caller scope is allowed.
|
||||||
|
let multi = authed_with_scopes(Some(vec!["scripts:read:f/team/a,f/team/b"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&multi,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team/a"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
|
||||||
|
// A wildcard caller covers any subset, but not `*`-less escalation rules apply
|
||||||
|
// only when the caller itself lacks `*`.
|
||||||
|
let wildcard = authed_with_scopes(Some(vec!["scripts:read:*"]));
|
||||||
|
assert!(ensure_scopes_within_caller(
|
||||||
|
&wildcard,
|
||||||
|
opt_scopes(Some(vec!["scripts:read:f/team/a"])).as_deref()
|
||||||
|
)
|
||||||
|
.is_ok());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1964,7 +1964,8 @@ async fn login(
|
|||||||
windmill_common::login_rate_limit::record_login_failure(&email);
|
windmill_common::login_rate_limit::record_login_failure(&email);
|
||||||
Err(Error::BadRequest("Invalid login".to_string()))
|
Err(Error::BadRequest("Invalid login".to_string()))
|
||||||
} else {
|
} else {
|
||||||
let token = create_session_token(&email, super_admin, &mut tx, cookies).await?;
|
let token =
|
||||||
|
create_session_token(&email, super_admin, None, false, &mut tx, cookies).await?;
|
||||||
|
|
||||||
let audit_author = AuditAuthor {
|
let audit_author = AuditAuthor {
|
||||||
email: email.clone(),
|
email: email.clone(),
|
||||||
@@ -2036,7 +2037,15 @@ async fn refresh_token(
|
|||||||
.await?
|
.await?
|
||||||
.unwrap_or(false);
|
.unwrap_or(false);
|
||||||
|
|
||||||
let new_token = create_session_token(&authed.email, super_admin, &mut tx, cookies).await?;
|
let new_token = create_session_token(
|
||||||
|
&authed.email,
|
||||||
|
super_admin,
|
||||||
|
authed.scopes.as_deref(),
|
||||||
|
authed.read_only,
|
||||||
|
&mut tx,
|
||||||
|
cookies,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
|
||||||
audit_log(
|
audit_log(
|
||||||
&mut *tx,
|
&mut *tx,
|
||||||
@@ -2066,6 +2075,8 @@ lazy_static::lazy_static! {
|
|||||||
pub async fn create_session_token<'c>(
|
pub async fn create_session_token<'c>(
|
||||||
email: &str,
|
email: &str,
|
||||||
super_admin: bool,
|
super_admin: bool,
|
||||||
|
scopes: Option<&[String]>,
|
||||||
|
read_only: bool,
|
||||||
tx: &mut sqlx::Transaction<'c, sqlx::Postgres>,
|
tx: &mut sqlx::Transaction<'c, sqlx::Postgres>,
|
||||||
cookies: Cookies,
|
cookies: Cookies,
|
||||||
) -> Result<String> {
|
) -> Result<String> {
|
||||||
@@ -2108,15 +2119,17 @@ pub async fn create_session_token<'c>(
|
|||||||
|
|
||||||
sqlx::query!(
|
sqlx::query!(
|
||||||
"INSERT INTO token
|
"INSERT INTO token
|
||||||
(token_hash, token_prefix, token, email, label, expiration, super_admin)
|
(token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, read_only)
|
||||||
VALUES ($1, $2, $3, $4, $5, now() + ($6 || ' seconds')::interval, $7)",
|
VALUES ($1, $2, $3, $4, $5, now() + ($6 || ' seconds')::interval, $7, $8, $9)",
|
||||||
t_hash,
|
t_hash,
|
||||||
t_prefix,
|
t_prefix,
|
||||||
plaintext as Option<&str>,
|
plaintext as Option<&str>,
|
||||||
email,
|
email,
|
||||||
"session",
|
"session",
|
||||||
&MAX_SESSION_VALIDITY_SECONDS.to_string(),
|
&MAX_SESSION_VALIDITY_SECONDS.to_string(),
|
||||||
super_admin
|
super_admin,
|
||||||
|
scopes,
|
||||||
|
read_only,
|
||||||
)
|
)
|
||||||
.execute(&mut **tx)
|
.execute(&mut **tx)
|
||||||
.await?;
|
.await?;
|
||||||
@@ -2146,6 +2159,8 @@ async fn create_token(
|
|||||||
) -> Result<(StatusCode, String)> {
|
) -> Result<(StatusCode, String)> {
|
||||||
check_token_create_rate_limit(&authed.username)?;
|
check_token_create_rate_limit(&authed.username)?;
|
||||||
|
|
||||||
|
windmill_api_auth::ensure_scopes_within_caller(&authed, token_config.scopes.as_deref())?;
|
||||||
|
|
||||||
let mut tx = db.begin().await?;
|
let mut tx = db.begin().await?;
|
||||||
|
|
||||||
let token = create_token_internal(&mut *tx, &db, &authed, token_config).await?;
|
let token = create_token_internal(&mut *tx, &db, &authed, token_config).await?;
|
||||||
@@ -2353,6 +2368,8 @@ async fn update_token_scopes(
|
|||||||
Path(token_prefix): Path<String>,
|
Path(token_prefix): Path<String>,
|
||||||
Json(req): Json<UpdateTokenScopesRequest>,
|
Json(req): Json<UpdateTokenScopesRequest>,
|
||||||
) -> Result<String> {
|
) -> Result<String> {
|
||||||
|
windmill_api_auth::ensure_scopes_within_caller(&authed, req.scopes.as_deref())?;
|
||||||
|
|
||||||
let mut tx = db.begin().await?;
|
let mut tx = db.begin().await?;
|
||||||
|
|
||||||
let updated: Option<String> = sqlx::query_scalar!(
|
let updated: Option<String> = sqlx::query_scalar!(
|
||||||
|
|||||||
Reference in New Issue
Block a user