mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
feat: let a workspace withdraw operator schedule and trigger writes
Operators can create, edit and delete schedules and triggers today through the API, CLI and MCP, while the operator_settings flags beside them only hide those pages. An admin who wants operators to see what is scheduled without letting them change it cannot express that. Add manage_schedules and manage_triggers as enforced settings, gated at the schedule handlers and at the generic TriggerCrud routes so every trigger kind is covered by one check. They name capabilities operators already hold, so they are granted unless withdrawn, and absence has to mean "never configured" rather than a value. The read coalesces to true; the update endpoint merges into the stored jsonb with the two fields as Option<bool>, so an omitted key keeps what is stored. operator_settings is git-synced as a whole object, so a settings file written before these keys existed reaches the endpoint on every pull, and a serde or SQL default of either polarity would turn that pull into a silent withdrawal or restoration. The rights are read through a per-process cache, so withdrawing one publishes a notify_event that drops the entry on every replica. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dsf6VC4MVLisiEoeQkgbr4
This commit is contained in:
co-authored by
Claude Opus 5
parent
01b38320ea
commit
625116e378
@@ -30,6 +30,9 @@ Open-source platform for internal tools, workflows, API integrations, background
|
||||
reaches the DB only through the API, so `Connection::Http` paths are never taken by a plain
|
||||
`cargo run`; a normal build cannot start one at all.
|
||||
- **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow
|
||||
- **Operator write rights**: `docs/operator-write-rights.md` — which `operator_settings` flags are
|
||||
enforced rather than cosmetic, and why a right that is granted-unless-withdrawn needs `Option`
|
||||
fields and a jsonb merge rather than a serde default
|
||||
- **Auth surface**: `docs/auth-surface.md` — credential precedence, session/cache invalidation
|
||||
scope, which token labels email their owner at expiry, how OAuth login matches `login_type`, and
|
||||
that every superadmin route refuses `$WM_TOKEN`. Read before designing anything that creates
|
||||
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE workspace_settings\n SET operator_settings = COALESCE(operator_settings, '{}'::jsonb) || $1\n WHERE workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "4515bdeded3b40abbdcc554c9562cce3225d2b66dc9fe23b1fa59cfaa120cd30"
|
||||
}
|
||||
+28
@@ -0,0 +1,28 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT COALESCE((operator_settings->>'manage_schedules')::boolean, true) AS \"schedules!\",\n COALESCE((operator_settings->>'manage_triggers')::boolean, true) AS \"triggers!\"\n FROM workspace_settings WHERE workspace_id = $1",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "schedules!",
|
||||
"type_info": "Bool"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "triggers!",
|
||||
"type_info": "Bool"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null,
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "457575e0858db5747964d4b2a69d3506774e50f871963da9697c7035def2aedd"
|
||||
}
|
||||
@@ -0,0 +1,2 @@
|
||||
DROP TRIGGER IF EXISTS operator_settings_change_trigger ON workspace_settings;
|
||||
DROP FUNCTION IF EXISTS notify_operator_settings_change();
|
||||
@@ -0,0 +1,18 @@
|
||||
-- The write rights in `operator_settings` are authorization decisions read through a per-process
|
||||
-- cache. Without this, withdrawing one on a single API replica leaves every other replica
|
||||
-- authorizing writes until its own entry expires.
|
||||
CREATE OR REPLACE FUNCTION notify_operator_settings_change()
|
||||
RETURNS TRIGGER AS $$
|
||||
BEGIN
|
||||
INSERT INTO notify_event (channel, payload)
|
||||
VALUES ('notify_operator_settings_change', NEW.workspace_id);
|
||||
RETURN NEW;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
|
||||
DROP TRIGGER IF EXISTS operator_settings_change_trigger ON workspace_settings;
|
||||
CREATE TRIGGER operator_settings_change_trigger
|
||||
AFTER UPDATE OF operator_settings ON workspace_settings
|
||||
FOR EACH ROW
|
||||
WHEN (OLD.operator_settings IS DISTINCT FROM NEW.operator_settings)
|
||||
EXECUTE FUNCTION notify_operator_settings_change();
|
||||
+35
-32
@@ -46,26 +46,24 @@ use windmill_common::{
|
||||
CUSTOM_TAGS_SETTING, DEFAULT_TAGS_PER_WORKSPACE_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING,
|
||||
DISABLE_PASSWORD_LOGIN_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS,
|
||||
EXPOSE_DEBUG_METRICS_SETTING, EXPOSE_METRICS_SETTING, EXTRA_PIP_INDEX_URL_SETTING,
|
||||
FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING,
|
||||
HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING,
|
||||
HUB_API_SECRET_SETTING, HUB_BASE_URL_SETTING, INDEXER_SETTING,
|
||||
INSTANCE_EVENTS_WEBHOOK_SETTING, INSTANCE_PYTHON_VERSION_SETTING,
|
||||
FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING,
|
||||
HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_API_SECRET_SETTING, HUB_BASE_URL_SETTING,
|
||||
INDEXER_SETTING, INSTANCE_EVENTS_WEBHOOK_SETTING, INSTANCE_PYTHON_VERSION_SETTING,
|
||||
JOB_DEFAULT_TIMEOUT_SECS_SETTING, JOB_ISOLATION_SETTING, JWT_SECRET_SETTING,
|
||||
KEEP_JOB_DIR_SETTING, LICENSE_KEY_SETTING, MAVEN_REPOS_SETTING, MAVEN_SETTINGS_XML_SETTING,
|
||||
MCP_DISABLE_TOKEN_QUERY_PARAM_SETTING, MONITOR_LOGS_ON_OBJECT_STORE_SETTING,
|
||||
NO_DEFAULT_MAVEN_SETTING,
|
||||
NPM_CONFIG_REGISTRY_SETTING, NSJAIL_TMPFS_SIZE_MB_SETTING, NSJAIL_TMP_BACKING_SETTING,
|
||||
NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, OTEL_TRACES_RETENTION_SECS_SETTING,
|
||||
OTEL_TRACING_PROXY_SETTING, PIP_INDEX_URL_SETTING, POWERSHELL_REPO_PAT_SETTING,
|
||||
POWERSHELL_REPO_URL_SETTING, PREVIEW_TAGS_OVERRIDE_SETTING, REQUEST_SIZE_LIMIT_SETTING,
|
||||
REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, RESTART_COORDINATION_SETTING,
|
||||
RETENTION_PERIOD_SECS_OVERRIDES_SETTING, RETENTION_PERIOD_SECS_SETTING, RUBY_REPOS_SETTING,
|
||||
SAML_METADATA_SETTING, SANDBOX_IMAGE_CACHE_MAX_MB_SETTING,
|
||||
SANDBOX_IMAGE_DEFAULT_REGISTRY_SETTING, SANDBOX_IMAGE_MAX_SIZE_MB_SETTING,
|
||||
SANDBOX_IMAGE_PULL_POLICY_SETTING, SANDBOX_REGISTRY_AUTH_SETTING, SCIM_TOKEN_SETTING,
|
||||
SERVICE_LOG_RETENTION_SECS_SETTING, SMTP_SETTING, STORE_AUDIT_LOGS_S3_SETTING,
|
||||
TEAMS_SETTING, TIMEOUT_WAIT_RESULT_SETTING, UV_EXCLUDE_NEWER_SETTING,
|
||||
UV_INDEX_STRATEGY_SETTING, UV_PYTHON_INSTALL_MIRROR_SETTING,
|
||||
NO_DEFAULT_MAVEN_SETTING, NPM_CONFIG_REGISTRY_SETTING, NSJAIL_TMPFS_SIZE_MB_SETTING,
|
||||
NSJAIL_TMP_BACKING_SETTING, NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING,
|
||||
OTEL_TRACES_RETENTION_SECS_SETTING, OTEL_TRACING_PROXY_SETTING, PIP_INDEX_URL_SETTING,
|
||||
POWERSHELL_REPO_PAT_SETTING, POWERSHELL_REPO_URL_SETTING, PREVIEW_TAGS_OVERRIDE_SETTING,
|
||||
REQUEST_SIZE_LIMIT_SETTING, REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING,
|
||||
RESTART_COORDINATION_SETTING, RETENTION_PERIOD_SECS_OVERRIDES_SETTING,
|
||||
RETENTION_PERIOD_SECS_SETTING, RUBY_REPOS_SETTING, SAML_METADATA_SETTING,
|
||||
SANDBOX_IMAGE_CACHE_MAX_MB_SETTING, SANDBOX_IMAGE_DEFAULT_REGISTRY_SETTING,
|
||||
SANDBOX_IMAGE_MAX_SIZE_MB_SETTING, SANDBOX_IMAGE_PULL_POLICY_SETTING,
|
||||
SANDBOX_REGISTRY_AUTH_SETTING, SCIM_TOKEN_SETTING, SERVICE_LOG_RETENTION_SECS_SETTING,
|
||||
SMTP_SETTING, STORE_AUDIT_LOGS_S3_SETTING, TEAMS_SETTING, TIMEOUT_WAIT_RESULT_SETTING,
|
||||
UV_EXCLUDE_NEWER_SETTING, UV_INDEX_STRATEGY_SETTING, UV_PYTHON_INSTALL_MIRROR_SETTING,
|
||||
WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, WORKSPACE_FAIRNESS_ENABLED_SETTING,
|
||||
WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING,
|
||||
WORKSPACE_MAX_QUEUED_JOBS_SETTING, WORKSPACE_REGISTRIES_SETTING,
|
||||
@@ -128,22 +126,21 @@ use windmill_worker::{
|
||||
use crate::monitor::{
|
||||
initial_load, load_concurrency_key_max_queued, load_disable_password_login,
|
||||
load_fork_workspace_tag_append_fork_suffix, load_keep_job_dir,
|
||||
load_mcp_disable_token_query_param, load_metrics_debug_enabled,
|
||||
load_preview_tags_override, load_require_preexisting_user, load_retention_period_overrides,
|
||||
load_tag_per_workspace_enabled, load_tag_per_workspace_workspaces,
|
||||
load_workspace_fairness_duration_secs, load_workspace_fairness_enabled,
|
||||
load_workspace_fairness_max_percent, load_workspace_fairness_min_total,
|
||||
load_workspace_max_queued_jobs, monitor_db, reload_app_workspaced_route_setting,
|
||||
reload_audit_log_retention_days_setting, reload_base_url_setting,
|
||||
reload_bun_install_min_release_age_setting, reload_bunfig_install_scopes_setting,
|
||||
reload_critical_alert_mute_ui_setting, reload_critical_alert_mute_zombie_job_restart_setting,
|
||||
load_mcp_disable_token_query_param, load_metrics_debug_enabled, load_preview_tags_override,
|
||||
load_require_preexisting_user, load_retention_period_overrides, load_tag_per_workspace_enabled,
|
||||
load_tag_per_workspace_workspaces, load_workspace_fairness_duration_secs,
|
||||
load_workspace_fairness_enabled, load_workspace_fairness_max_percent,
|
||||
load_workspace_fairness_min_total, load_workspace_max_queued_jobs, monitor_db,
|
||||
reload_app_workspaced_route_setting, reload_audit_log_retention_days_setting,
|
||||
reload_base_url_setting, reload_bun_install_min_release_age_setting,
|
||||
reload_bunfig_install_scopes_setting, reload_critical_alert_mute_ui_setting,
|
||||
reload_critical_alert_mute_zombie_job_restart_setting,
|
||||
reload_critical_alerts_on_token_expiry_setting, reload_critical_error_channels_setting,
|
||||
reload_extra_pip_index_url_setting, reload_http_route_default_allowed_origins_setting,
|
||||
reload_http_route_workspaced_route_setting,
|
||||
reload_hub_api_secret_setting, reload_hub_base_url_setting,
|
||||
reload_instance_events_webhook_setting, reload_job_default_timeout_setting,
|
||||
reload_job_isolation_setting, reload_jwt_secret_setting, reload_license_key,
|
||||
reload_npm_config_registry_setting, reload_nsjail_tmp_backing_setting,
|
||||
reload_http_route_workspaced_route_setting, reload_hub_api_secret_setting,
|
||||
reload_hub_base_url_setting, reload_instance_events_webhook_setting,
|
||||
reload_job_default_timeout_setting, reload_job_isolation_setting, reload_jwt_secret_setting,
|
||||
reload_license_key, reload_npm_config_registry_setting, reload_nsjail_tmp_backing_setting,
|
||||
reload_nsjail_tmpfs_size_setting, reload_otel_traces_retention_secs_setting,
|
||||
reload_otel_tracing_proxy_setting, reload_pip_index_url_setting,
|
||||
reload_retention_period_setting, reload_sandbox_image_cache_max_setting,
|
||||
@@ -422,7 +419,6 @@ struct HubResourceTypeRaw {
|
||||
pub display_name: Option<Option<String>>,
|
||||
}
|
||||
|
||||
|
||||
/// Processed resource type with parsed schema
|
||||
#[derive(serde::Deserialize, serde::Serialize, Clone)]
|
||||
pub struct HubResourceType {
|
||||
@@ -1835,6 +1831,13 @@ async fn process_notify_event(
|
||||
);
|
||||
windmill_common::variables::CUSTOM_ENVS_CACHE.remove(payload);
|
||||
}
|
||||
"notify_operator_settings_change" => {
|
||||
tracing::info!(
|
||||
"Operator settings change detected, invalidating operator rights cache: {}",
|
||||
payload
|
||||
);
|
||||
windmill_common::workspaces::invalidate_operator_rights_cache(payload);
|
||||
}
|
||||
"notify_asset_producer_change" => {
|
||||
tracing::debug!(
|
||||
"Asset producer change for workspace {}, invalidating producer-writes cache",
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
use serde_json::json;
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_common::workspaces::invalidate_operator_rights_cache;
|
||||
use windmill_test_utils::*;
|
||||
|
||||
const WS: &str = "test-workspace";
|
||||
|
||||
fn operator_client() -> reqwest::Client {
|
||||
let mut headers = reqwest::header::HeaderMap::new();
|
||||
headers.insert(
|
||||
reqwest::header::AUTHORIZATION,
|
||||
reqwest::header::HeaderValue::from_str("Bearer OPERATOR_TOKEN_1").unwrap(),
|
||||
);
|
||||
reqwest::ClientBuilder::new()
|
||||
.default_headers(headers)
|
||||
.build()
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
fn new_schedule(path: &str) -> serde_json::Value {
|
||||
json!({
|
||||
"path": path,
|
||||
"schedule": "0 0 * * * *",
|
||||
"timezone": "UTC",
|
||||
"script_path": "u/operator/some_script",
|
||||
"is_flow": false,
|
||||
"enabled": false,
|
||||
})
|
||||
}
|
||||
|
||||
/// Schedules and triggers are rights operators hold until an admin withdraws them, so the stored
|
||||
/// setting has to survive a payload that never mentions it: this endpoint takes whole-object
|
||||
/// bodies from git-sync files written before the keys existed, and either default would let such a
|
||||
/// file silently flip the right on every pull.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "permissions_test"))]
|
||||
async fn test_operator_manage_rights(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let api = format!("http://localhost:{port}/api/w/{WS}");
|
||||
let admin = reqwest::Client::new();
|
||||
let c = operator_client();
|
||||
|
||||
let set_settings = async |body: serde_json::Value| -> anyhow::Result<u16> {
|
||||
let resp = admin
|
||||
.post(format!("{api}/workspaces/operator_settings"))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN")
|
||||
.json(&body)
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status().as_u16();
|
||||
invalidate_operator_rights_cache(WS);
|
||||
Ok(status)
|
||||
};
|
||||
|
||||
// Never configured: the right is held.
|
||||
let resp = c
|
||||
.post(format!("{api}/schedules/create"))
|
||||
.json(&new_schedule("u/operator/sched_default"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
|
||||
// An admin withdraws it.
|
||||
assert_eq!(set_settings(json!({"manage_schedules": false})).await?, 200);
|
||||
|
||||
let resp = c
|
||||
.post(format!("{api}/schedules/create"))
|
||||
.json(&new_schedule("u/operator/sched_withdrawn"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 401, "{}", resp.text().await?);
|
||||
|
||||
// A payload omitting the key must not restore it. This is what an older git-sync settings file
|
||||
// looks like, and what a serde or SQL default of either polarity would get wrong.
|
||||
assert_eq!(set_settings(json!({"runs": true})).await?, 200);
|
||||
|
||||
let resp = c
|
||||
.post(format!("{api}/schedules/create"))
|
||||
.json(&new_schedule("u/operator/sched_still_withdrawn"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 401, "{}", resp.text().await?);
|
||||
|
||||
// And the other direction: omitting the key must not withdraw a stored grant, which is what a
|
||||
// plain `bool` field would do by serializing its own default over it.
|
||||
assert_eq!(set_settings(json!({"manage_schedules": true})).await?, 200);
|
||||
assert_eq!(set_settings(json!({"runs": true})).await?, 200);
|
||||
|
||||
let resp = c
|
||||
.post(format!("{api}/schedules/create"))
|
||||
.json(&new_schedule("u/operator/sched_restored"))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -38,6 +38,7 @@ use windmill_common::{
|
||||
escape_ilike_pattern, not_found_if_none, paginate, Pagination, ScheduleType, StripPath,
|
||||
},
|
||||
worker::to_raw_value,
|
||||
workspaces::{check_operator_can_manage, ManageKind},
|
||||
};
|
||||
use windmill_git_sync::{handle_deployment_metadata, DeployedObject};
|
||||
use windmill_queue::schedule::push_scheduled_job;
|
||||
@@ -271,6 +272,14 @@ async fn create_schedule(
|
||||
Json(ns): Json<NewSchedule>,
|
||||
) -> Result<String> {
|
||||
check_scopes(&authed, || format!("schedules:write:{}", ns.path))?;
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&w_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Schedules,
|
||||
"create schedules",
|
||||
)
|
||||
.await?;
|
||||
reject_reserved_schedule_path(&ns.path)?;
|
||||
|
||||
let authed = maybe_refresh_folders(&ns.path, &w_id, authed, &db).await;
|
||||
@@ -542,6 +551,14 @@ async fn edit_schedule(
|
||||
) -> Result<String> {
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("schedules:write:{}", path))?;
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&w_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Schedules,
|
||||
"edit schedules",
|
||||
)
|
||||
.await?;
|
||||
reject_reserved_schedule_path(path)?;
|
||||
|
||||
let authed = maybe_refresh_folders(&path, &w_id, authed, &db).await;
|
||||
@@ -1124,9 +1141,20 @@ pub async fn set_enabled(
|
||||
Path((w_id, path)): Path<(String, StripPath)>,
|
||||
Json(payload): Json<SetEnabled>,
|
||||
) -> Result<String> {
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("schedules:write:{}", path))?;
|
||||
// Before the tx: this reads the root pool, and a second pooled connection taken while the RLS
|
||||
// tx is held self-deadlocks on a single-connection pool.
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&w_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Schedules,
|
||||
"enable or disable schedules",
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
reject_reserved_schedule_path(path)?;
|
||||
|
||||
// Block enabling a schedule in a fork when an ancestor has the same path
|
||||
@@ -1308,6 +1336,14 @@ async fn delete_schedule(
|
||||
) -> Result<String> {
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("schedules:write:{}", path))?;
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&w_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Schedules,
|
||||
"delete schedules",
|
||||
)
|
||||
.await?;
|
||||
reject_reserved_schedule_path(path)?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
|
||||
@@ -10488,6 +10488,14 @@ struct ChangeOperatorSettings {
|
||||
folders: bool,
|
||||
#[serde(default)]
|
||||
workers: bool,
|
||||
/// Writes operators may perform unless withdrawn, so `None` (key absent) must mean "leave as
|
||||
/// stored" rather than a value: the row is merged, not overwritten, and this endpoint takes
|
||||
/// whole-object payloads from git-sync files that predate the key. Defaulting either way here
|
||||
/// would make an older file silently withdraw or restore the right on every pull.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
manage_schedules: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
manage_triggers: Option<bool>,
|
||||
}
|
||||
|
||||
async fn update_operator_settings(
|
||||
@@ -10502,8 +10510,12 @@ async fn update_operator_settings(
|
||||
|
||||
let settings_json = serde_json::json!(settings);
|
||||
|
||||
// Merge rather than overwrite so an omitted key keeps its stored value. The visibility flags
|
||||
// are plain bools and always serialize, so this is a no-op for them.
|
||||
sqlx::query!(
|
||||
"UPDATE workspace_settings SET operator_settings = $1 WHERE workspace_id = $2",
|
||||
"UPDATE workspace_settings
|
||||
SET operator_settings = COALESCE(operator_settings, '{}'::jsonb) || $1
|
||||
WHERE workspace_id = $2",
|
||||
settings_json,
|
||||
&w_id
|
||||
)
|
||||
@@ -10512,6 +10524,8 @@ async fn update_operator_settings(
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
windmill_common::workspaces::invalidate_operator_rights_cache(&w_id);
|
||||
|
||||
// Trigger git sync for operator settings changes
|
||||
handle_deployment_metadata(
|
||||
&authed.email,
|
||||
|
||||
@@ -36584,6 +36584,12 @@ components:
|
||||
workers:
|
||||
type: boolean
|
||||
description: Whether operators can view workers page
|
||||
manage_schedules:
|
||||
type: boolean
|
||||
description: Whether operators can create, edit and delete schedules. Granted unless withdrawn; omitting the field leaves the stored value unchanged.
|
||||
manage_triggers:
|
||||
type: boolean
|
||||
description: Whether operators can create, edit and delete triggers. Granted unless withdrawn; omitting the field leaves the stored value unchanged.
|
||||
|
||||
WorkspaceComparison:
|
||||
type: object
|
||||
|
||||
@@ -1186,6 +1186,112 @@ pub fn invalidate_protection_rules_cache(workspace_id: &str) {
|
||||
PROTECTION_RULES_CACHE.remove(workspace_id);
|
||||
}
|
||||
|
||||
// Operator rights cache
|
||||
|
||||
lazy_static::lazy_static! {
|
||||
static ref OPERATOR_RIGHTS_CACHE: Cache<String, (OperatorManageRights, i64)> = Cache::new(1000);
|
||||
}
|
||||
|
||||
/// Writes an operator may perform unless the workspace withdraws them. Unlike the visibility
|
||||
/// flags beside them in `operator_settings`, these are enforced; unlike a right a workspace
|
||||
/// grants, they are held by default and cost no seat.
|
||||
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
|
||||
pub struct OperatorManageRights {
|
||||
pub schedules: bool,
|
||||
pub triggers: bool,
|
||||
}
|
||||
|
||||
/// Granted unless explicitly withdrawn. Every path that materializes these rights without a
|
||||
/// stored setting - a workspace with no `workspace_settings` row included - has to land here, or
|
||||
/// an upgrade silently revokes what operators could do the day before.
|
||||
impl Default for OperatorManageRights {
|
||||
fn default() -> Self {
|
||||
Self { schedules: true, triggers: true }
|
||||
}
|
||||
}
|
||||
|
||||
/// Which withdrawable capability a gate needs.
|
||||
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
|
||||
pub enum ManageKind {
|
||||
Schedules,
|
||||
Triggers,
|
||||
}
|
||||
|
||||
impl OperatorManageRights {
|
||||
pub fn has(&self, kind: ManageKind) -> bool {
|
||||
match kind {
|
||||
ManageKind::Schedules => self.schedules,
|
||||
ManageKind::Triggers => self.triggers,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// What operators of this workspace may still write. Per workspace, not per user.
|
||||
///
|
||||
/// Read on every gated write, so it is cached with a 60s TTL. This gates writes, so a withdrawal
|
||||
/// cannot wait out that TTL on the rest of the fleet: an `AFTER UPDATE OF operator_settings`
|
||||
/// trigger publishes `notify_operator_settings_change` and every server drops its entry through
|
||||
/// `process_notify_event`. [`invalidate_operator_rights_cache`] is the local half of that, and
|
||||
/// what a test flipping the setting directly has to call itself.
|
||||
///
|
||||
/// Call it before opening an RLS transaction: it takes a connection from the root pool, and a
|
||||
/// second pooled connection held alongside a transaction self-deadlocks on a one-connection pool.
|
||||
pub async fn operator_manage_rights(db: &DB, workspace_id: &str) -> Result<OperatorManageRights> {
|
||||
let now = chrono::Utc::now().timestamp();
|
||||
|
||||
if let Some((rights, expiry)) = OPERATOR_RIGHTS_CACHE.get(workspace_id) {
|
||||
if expiry > now {
|
||||
return Ok(rights);
|
||||
}
|
||||
}
|
||||
|
||||
// Coalesced to true, matching `OperatorManageRights::default`: an absent key means the
|
||||
// workspace never configured the right, not that it withdrew it.
|
||||
let row = sqlx::query!(
|
||||
"SELECT COALESCE((operator_settings->>'manage_schedules')::boolean, true) AS \"schedules!\",
|
||||
COALESCE((operator_settings->>'manage_triggers')::boolean, true) AS \"triggers!\"
|
||||
FROM workspace_settings WHERE workspace_id = $1",
|
||||
workspace_id
|
||||
)
|
||||
.fetch_optional(db)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
Error::internal_err(format!(
|
||||
"Failed to fetch operator settings for {workspace_id}: {e:#}"
|
||||
))
|
||||
})?;
|
||||
|
||||
let rights = row
|
||||
.map(|r| OperatorManageRights { schedules: r.schedules, triggers: r.triggers })
|
||||
.unwrap_or_default();
|
||||
|
||||
OPERATOR_RIGHTS_CACHE.insert(workspace_id.to_string(), (rights, now + 60));
|
||||
|
||||
Ok(rights)
|
||||
}
|
||||
|
||||
/// Invalidate the operator rights cache for a workspace
|
||||
pub fn invalidate_operator_rights_cache(workspace_id: &str) {
|
||||
OPERATOR_RIGHTS_CACHE.remove(workspace_id);
|
||||
}
|
||||
|
||||
/// Gate for a write operators may perform unless the workspace withdrew it. `action` completes
|
||||
/// "Operators cannot {action} in this workspace".
|
||||
pub async fn check_operator_can_manage(
|
||||
db: &DB,
|
||||
workspace_id: &str,
|
||||
is_operator: bool,
|
||||
kind: ManageKind,
|
||||
action: &str,
|
||||
) -> Result<()> {
|
||||
if is_operator && !operator_manage_rights(db, workspace_id).await?.has(kind) {
|
||||
return Err(Error::NotAuthorized(format!(
|
||||
"Operators cannot {action} in this workspace"
|
||||
)));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum RuleCheckResult {
|
||||
Allowed,
|
||||
|
||||
@@ -24,6 +24,7 @@ use windmill_common::{
|
||||
},
|
||||
utils::{paginate, Pagination, StripPath},
|
||||
worker::CLOUD_HOSTED,
|
||||
workspaces::{check_operator_can_manage, ManageKind},
|
||||
DB,
|
||||
};
|
||||
use windmill_git_sync::DeployedObject;
|
||||
@@ -548,6 +549,15 @@ async fn create_trigger<T: TriggerCrud>(
|
||||
)
|
||||
})?;
|
||||
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&workspace_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Triggers,
|
||||
"create triggers",
|
||||
)
|
||||
.await?;
|
||||
|
||||
if *CLOUD_HOSTED && !T::IS_ALLOWED_ON_CLOUD {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"{} triggers are not supported on multi-tenant cloud, use dedicated cloud or self-host",
|
||||
@@ -818,6 +828,15 @@ async fn update_trigger<T: TriggerCrud>(
|
||||
)
|
||||
})?;
|
||||
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&workspace_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Triggers,
|
||||
"edit triggers",
|
||||
)
|
||||
.await?;
|
||||
|
||||
edit_trigger.error_handling.validate()?;
|
||||
|
||||
handler
|
||||
@@ -972,6 +991,14 @@ async fn delete_trigger<T: TriggerCrud>(
|
||||
check_scopes(&authed, || {
|
||||
format!("{}:write:{}", T::scope_domain_name(), &path)
|
||||
})?;
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&workspace_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Triggers,
|
||||
"delete triggers",
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
@@ -1113,6 +1140,14 @@ async fn set_trigger_mode<T: TriggerCrud>(
|
||||
) -> Result<String> {
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("{}:write", T::scope_domain_name()))?;
|
||||
check_operator_can_manage(
|
||||
&db,
|
||||
&workspace_id,
|
||||
authed.is_operator,
|
||||
ManageKind::Triggers,
|
||||
"enable or disable triggers",
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
# Operator write rights
|
||||
|
||||
Most of `workspace_settings.operator_settings` is visibility: flags that hide pages from operators
|
||||
so the UI stays uncluttered. They are not enforced, and were never meant to be.
|
||||
|
||||
`manage_schedules` and `manage_triggers` are different. They are **enforced** on the write paths,
|
||||
because hiding the schedules page never stopped an operator creating a schedule through the API,
|
||||
the CLI or MCP. An admin who wants operators to see what is scheduled without letting them change
|
||||
it could not express that with a visibility flag alone.
|
||||
|
||||
Read them with `windmill_common::workspaces::operator_manage_rights` (60s cache) and gate a write
|
||||
with `check_operator_can_manage`, naming a `ManageKind`. Call it **before** opening an RLS
|
||||
transaction: it takes a connection from the root pool, and a second pooled connection held
|
||||
alongside a transaction self-deadlocks on a one-connection pool.
|
||||
|
||||
The cache is per process, so withdrawing a right has to reach every replica: an `AFTER UPDATE OF
|
||||
operator_settings` trigger writes a `notify_operator_settings_change` row and `process_notify_event`
|
||||
drops the entry. Keep both ends if you touch either, or a workspace that withdrew a right keeps
|
||||
authorizing writes on every other replica until its own entry expires.
|
||||
|
||||
## Granted unless withdrawn
|
||||
|
||||
These name capabilities operators already hold, so absence has to mean "never configured", not a
|
||||
value. That is easy to get wrong in two places, and both were wrong in the obvious first draft:
|
||||
|
||||
- The read coalesces to **true** (`operator_manage_rights`), including for a workspace with no
|
||||
`workspace_settings` row, which is what `OperatorManageRights::default` is for. Coalescing to
|
||||
false instead revokes the right on upgrade for every workspace that ever saved operator settings,
|
||||
since those rows carry explicit keys and none of them is this one.
|
||||
- The update endpoint **merges** into the stored jsonb and takes these two as `Option<bool>`, so an
|
||||
omitted key keeps its stored value. It has to: `operator_settings` is git-synced as a whole
|
||||
object (`cli/src/core/settings.ts` posts the file's contents verbatim), so a settings file
|
||||
written before these keys existed reaches the endpoint on every pull. A serde or SQL default of
|
||||
either polarity turns that pull into a silent withdrawal or a silent restoration.
|
||||
|
||||
The visibility flags are plain `bool` and always serialize, so the merge is a no-op for them and
|
||||
their behaviour is unchanged.
|
||||
|
||||
Do not model a right that an admin *grants* on these. Granted-by-default and granted-on-request
|
||||
are opposite polarities, and a gate written for one is wrong for the other.
|
||||
@@ -6,6 +6,7 @@
|
||||
import Section from '$lib/components/Section.svelte'
|
||||
import Head from '$lib/components/table/Head.svelte'
|
||||
import Cell from '$lib/components/table/Cell.svelte'
|
||||
import Toggle from '$lib/components/Toggle.svelte'
|
||||
import { WorkspaceService } from '$lib/gen'
|
||||
import { workspaceStore } from '$lib/stores'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
@@ -25,18 +26,33 @@
|
||||
workers: true
|
||||
})
|
||||
|
||||
let originalSettings = $state({ ...untrack(() => operatorWorkspaceSettings) })
|
||||
// Kept out of `operatorWorkspaceSettings` so the visibility table's "Enable all" never flips a
|
||||
// write right, and so these rows stay out of that table. Withdrawable rather than granted:
|
||||
// operators hold them until an admin turns them off.
|
||||
let manageSchedules = $state(true)
|
||||
let manageTriggers = $state(true)
|
||||
|
||||
let originalSettings = $state({
|
||||
...untrack(() => operatorWorkspaceSettings),
|
||||
manage_schedules: true,
|
||||
manage_triggers: true
|
||||
})
|
||||
let isChanged = $state(false)
|
||||
let currentWorkspace: string | null = $state(null)
|
||||
|
||||
const settingsPayload = $derived({
|
||||
...operatorWorkspaceSettings,
|
||||
manage_schedules: manageSchedules,
|
||||
manage_triggers: manageTriggers
|
||||
})
|
||||
|
||||
async function saveSettings() {
|
||||
console.log('Saving operator settings:', operatorWorkspaceSettings)
|
||||
try {
|
||||
await WorkspaceService.updateOperatorSettings({
|
||||
workspace: $workspaceStore!,
|
||||
requestBody: operatorWorkspaceSettings
|
||||
requestBody: settingsPayload
|
||||
})
|
||||
originalSettings = { ...operatorWorkspaceSettings }
|
||||
originalSettings = { ...settingsPayload }
|
||||
isChanged = false
|
||||
sendUserToast('Operator settings saved successfully!', false)
|
||||
} catch (error) {
|
||||
@@ -66,18 +82,26 @@
|
||||
workspace: $workspaceStore
|
||||
})
|
||||
if (settings.operator_settings !== null) {
|
||||
operatorWorkspaceSettings = {
|
||||
const {
|
||||
manage_schedules: remoteSchedules,
|
||||
manage_triggers: remoteTriggers,
|
||||
...remoteVisibility
|
||||
} = settings.operator_settings ?? {}
|
||||
operatorWorkspaceSettings = { ...operatorWorkspaceSettings, ...remoteVisibility }
|
||||
manageSchedules = remoteSchedules ?? true
|
||||
manageTriggers = remoteTriggers ?? true
|
||||
originalSettings = {
|
||||
...operatorWorkspaceSettings,
|
||||
...(settings.operator_settings ?? {})
|
||||
manage_schedules: manageSchedules,
|
||||
manage_triggers: manageTriggers
|
||||
}
|
||||
originalSettings = { ...operatorWorkspaceSettings }
|
||||
}
|
||||
})()
|
||||
}
|
||||
})
|
||||
|
||||
$effect(() => {
|
||||
isChanged = JSON.stringify(operatorWorkspaceSettings) !== JSON.stringify(originalSettings)
|
||||
isChanged = JSON.stringify(settingsPayload) !== JSON.stringify(originalSettings)
|
||||
})
|
||||
|
||||
const allDisabled = $derived(
|
||||
@@ -105,6 +129,24 @@
|
||||
</Button>
|
||||
{/snippet}
|
||||
|
||||
<div class="flex flex-col gap-y-1 mb-4">
|
||||
<span class="text-xs font-semibold text-emphasis">Schedules and triggers</span>
|
||||
<span class="text-xs font-normal text-secondary">
|
||||
Operators can create, edit and delete schedules and triggers wherever their folder permissions
|
||||
let them write. Turn these off to withdraw that.
|
||||
</span>
|
||||
<Toggle
|
||||
bind:checked={manageSchedules}
|
||||
options={{ right: 'Operators can manage schedules' }}
|
||||
size="xs"
|
||||
/>
|
||||
<Toggle
|
||||
bind:checked={manageTriggers}
|
||||
options={{ right: 'Operators can manage triggers' }}
|
||||
size="xs"
|
||||
/>
|
||||
</div>
|
||||
|
||||
<DataTable tableFixed={true} size="xs">
|
||||
<Head>
|
||||
<tr>
|
||||
|
||||
Reference in New Issue
Block a user