mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-03 08:02:19 +00:00
fix: enforce token path scopes on GET /raw_apps/list (#11284)
* fix: enforce token path scopes on GET /raw_apps/list Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: apply the raw app scope filter before the page limit Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: cover the bare prefix path in the raw app scope test Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
58217006ec
commit
9ad2c91ddb
@@ -0,0 +1,76 @@
|
||||
//! `GET /raw_apps/list` must honor a path-scoped token. The route layer only checks
|
||||
//! the scope domain and RLS knows nothing of token scopes, so the per-path filter
|
||||
//! lives in the handler, ahead of the page's LIMIT.
|
||||
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_test_utils::*;
|
||||
|
||||
async fn list_paths(port: u16, token: &str, query: &str) -> anyhow::Result<Vec<String>> {
|
||||
let resp = reqwest::Client::new()
|
||||
.get(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/raw_apps/list?{query}"
|
||||
))
|
||||
.header("Authorization", format!("Bearer {token}"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "list: {}", resp.text().await?);
|
||||
let body: Vec<serde_json::Value> = resp.json().await?;
|
||||
let mut paths: Vec<String> = body
|
||||
.iter()
|
||||
.map(|a| a["path"].as_str().unwrap().to_string())
|
||||
.collect();
|
||||
paths.sort();
|
||||
Ok(paths)
|
||||
}
|
||||
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn test_raw_apps_list_filters_by_token_scope(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
|
||||
// The list sorts by edited_at desc, so the out-of-scope apps fill the first page
|
||||
// unless the scope is applied before the LIMIT.
|
||||
for (path, age_minutes) in [
|
||||
("f/other/c", 0),
|
||||
("u/test-userx/d", 1),
|
||||
("u/test-user/a", 2),
|
||||
("u/test-user/sub/b", 3),
|
||||
// A `prefix/*` grant also covers the path at the prefix itself.
|
||||
("u/test-user", 4),
|
||||
] {
|
||||
sqlx::query(
|
||||
"INSERT INTO raw_app (path, workspace_id, data, edited_at)
|
||||
VALUES ($1, 'test-workspace', '', now() - make_interval(mins => $2))",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(age_minutes)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
}
|
||||
sqlx::query(
|
||||
"INSERT INTO token (token_hash, token_prefix, token, email, label, scopes)
|
||||
VALUES (encode(sha256('SCOPED_TOKEN'::bytea), 'hex'), 'SCOPED_TOK', 'SCOPED_TOKEN',
|
||||
'test@windmill.dev', 'scoped', ARRAY['raw_apps:read:u/test-user/*'])",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
|
||||
// The same user through an unscoped token sees every row.
|
||||
assert_eq!(
|
||||
list_paths(port, "SECRET_TOKEN", "").await?,
|
||||
[
|
||||
"f/other/c",
|
||||
"u/test-user",
|
||||
"u/test-user/a",
|
||||
"u/test-user/sub/b",
|
||||
"u/test-userx/d"
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
list_paths(port, "SCOPED_TOKEN", "per_page=3").await?,
|
||||
["u/test-user", "u/test-user/a", "u/test-user/sub/b"]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -5,7 +5,10 @@
|
||||
* Please see the included NOTICE for copyright information and
|
||||
* LICENSE-AGPL for a copy of the license.
|
||||
*/
|
||||
use crate::{db::ApiAuthed, utils::check_scopes};
|
||||
use crate::{
|
||||
db::ApiAuthed,
|
||||
utils::{build_scope_path_filter, check_scopes, ScopePathFilter},
|
||||
};
|
||||
use axum::{
|
||||
body::Body,
|
||||
extract::{Extension, Json, Path, Query},
|
||||
@@ -103,6 +106,31 @@ async fn list_apps(
|
||||
}
|
||||
}
|
||||
|
||||
// In the WHERE, not a retain after the fetch: the result is paginated, and a
|
||||
// post-fetch filter would return short pages and let a page's size report how
|
||||
// many raw apps the token may not read. One `?` per term, since a chained `.bind`
|
||||
// substitutes into a `?` inside the value an earlier one inserted; `starts_with`,
|
||||
// since LIKE reads a `_` in the path as a wildcard.
|
||||
if let ScopePathFilter::Restricted { exact, prefix } =
|
||||
build_scope_path_filter(&authed, "raw_apps", "read")
|
||||
{
|
||||
let terms: Vec<String> = exact
|
||||
.iter()
|
||||
.chain(&prefix)
|
||||
.map(|p| "app.path = ?".bind(p))
|
||||
.chain(
|
||||
prefix
|
||||
.iter()
|
||||
.map(|p| "starts_with(app.path, ?)".bind(&format!("{p}/"))),
|
||||
)
|
||||
.collect();
|
||||
sqlb.and_where(if terms.is_empty() {
|
||||
"false".to_string()
|
||||
} else {
|
||||
format!("({})", terms.join(" OR "))
|
||||
});
|
||||
}
|
||||
|
||||
let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
let rows = sqlx::query_as::<_, ListableApp>(&sql)
|
||||
|
||||
Reference in New Issue
Block a user