mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: skip expiry notifications for app embed and SDK tokens (#11169)
* fix: skip expiry notifications for app embed and SDK tokens Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: share app token label prefixes between mint sites and the check Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: skip expiry alerts for impersonation and test-connection tokens Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
02e47de8b4
commit
9d348f84c7
@@ -31,8 +31,9 @@ Open-source platform for internal tools, workflows, API integrations, background
|
||||
`cargo run`; a normal build cannot start one at all.
|
||||
- **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow
|
||||
- **Auth surface**: `docs/auth-surface.md` — credential precedence, session/cache invalidation
|
||||
scope, how OAuth login matches `login_type`, and that every superadmin route refuses `$WM_TOKEN`.
|
||||
Read before designing anything that creates users, tokens or sessions.
|
||||
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
|
||||
users, tokens or sessions.
|
||||
- **Product telemetry**: `docs/feature-telemetry.md` — when to instrument a new feature with
|
||||
`feature_usage`, and the four-step recipe. An unregistered `(feature, kind)` pair is dropped
|
||||
silently, so frontend-only instrumentation records nothing.
|
||||
|
||||
+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 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",
|
||||
"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 AND NOT starts_with(label, 'embed_app:')\n AND NOT starts_with(label, 'sdk_app:')\n AND NOT starts_with(label, 'impersonation:')\n ))\n RETURNING token_prefix",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -20,5 +20,5 @@
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "31ed2fb85c0c726e3cf6392be2a73c62bae004b2842a4828c6807232570f83a1"
|
||||
"hash": "d631a26e5531589ff37e677f91a4f1d9f850e3e46c17130dd580426cda7a9f65"
|
||||
}
|
||||
@@ -1141,6 +1141,9 @@ impl NewToken {
|
||||
/// [`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).
|
||||
///
|
||||
/// A token the system mints for itself with an `expiration` needs a label reserved in
|
||||
/// `windmill_common::auth::is_user_token`, or its expiry alerts its owner (docs/auth-surface.md).
|
||||
pub async fn create_token_internal(
|
||||
tx: &mut sqlx::PgConnection,
|
||||
db: &DB,
|
||||
|
||||
@@ -179,9 +179,11 @@ async fn test_trigger_token_labels_still_creatable(db: Pool<Postgres>) -> anyhow
|
||||
"http-test-user-2-cd34",
|
||||
"email-test-user-2-ef56",
|
||||
"my-ci-token",
|
||||
// Minted client-side by the editor (every TypeScript editor load) and the debugger.
|
||||
// Minted client-side by the editor (every TypeScript editor load), the debugger and
|
||||
// the object-storage "Test from a worker" button.
|
||||
"Ephemeral lsp token",
|
||||
"debugger-token",
|
||||
"ephemeral-test-connection: s3_bucket",
|
||||
] {
|
||||
let resp = create_token_with_label(port, "SECRET_TOKEN_2", label).await;
|
||||
assert_eq!(
|
||||
|
||||
@@ -3882,8 +3882,8 @@ async fn update_token_label(
|
||||
Path(token_prefix): Path<String>,
|
||||
Json(req): Json<UpdateTokenLabelRequest>,
|
||||
) -> Result<String> {
|
||||
// The new label must not collide with a system-token namespace (`session`,
|
||||
// `ephemeral*`, `debugger-token`, `mcp-oauth-*`): those labels are
|
||||
// The new label must not collide with a system-token namespace (see
|
||||
// `windmill_common::auth::is_user_token`): those labels are
|
||||
// load-bearing, and a user-set collision would orphan the token — hidden
|
||||
// from the UI (`isUserToken`) and rejected by the editability guard below —
|
||||
// while it still authenticates. (`is_user_token(None)` is true, so clearing
|
||||
@@ -3922,6 +3922,9 @@ async fn update_token_label(
|
||||
AND lower(label) NOT LIKE 'ephemeral%'
|
||||
AND label <> 'debugger-token'
|
||||
AND label NOT LIKE 'mcp-oauth-%'
|
||||
AND NOT starts_with(label, 'embed_app:')
|
||||
AND NOT starts_with(label, 'sdk_app:')
|
||||
AND NOT starts_with(label, 'impersonation:')
|
||||
))
|
||||
RETURNING token_prefix",
|
||||
req.label.as_deref(),
|
||||
|
||||
@@ -57,7 +57,7 @@ use windmill_audit::audit_oss::{audit_log, AuditAuthorable};
|
||||
use windmill_audit::ActionKind;
|
||||
use windmill_common::{
|
||||
apps::{AppScriptId, ListAppQuery, APP_WORKSPACED_ROUTE},
|
||||
auth::TOKEN_PREFIX_LEN,
|
||||
auth::{APP_EMBED_TOKEN_LABEL_PREFIX, RAW_APP_SDK_TOKEN_LABEL_PREFIX, TOKEN_PREFIX_LEN},
|
||||
cache::{self, future::FutureCachedExt},
|
||||
db::{DbWithOptAuthed, UserDB},
|
||||
error::{to_anyhow, Error, JsonResult, Result},
|
||||
@@ -1522,7 +1522,10 @@ async fn mint_raw_app_sdk_token(
|
||||
scopes.push(windmill_api_auth::scopes::GUEST_SENTINEL.to_string());
|
||||
(label, exp)
|
||||
}
|
||||
None => (format!("sdk_app:{app_path}"), requested_exp),
|
||||
None => (
|
||||
format!("{RAW_APP_SDK_TOKEN_LABEL_PREFIX}{app_path}"),
|
||||
requested_exp,
|
||||
),
|
||||
};
|
||||
let token_config = NewToken::new(
|
||||
Some(label),
|
||||
@@ -1804,7 +1807,10 @@ pub async fn mint_app_embed_token(
|
||||
scopes.push(windmill_api_auth::scopes::GUEST_SENTINEL.to_string());
|
||||
(label, exp)
|
||||
}
|
||||
None => (format!("embed_app:{app_path}"), requested_exp),
|
||||
None => (
|
||||
format!("{APP_EMBED_TOKEN_LABEL_PREFIX}{app_path}"),
|
||||
requested_exp,
|
||||
),
|
||||
};
|
||||
let token_config = NewToken::new(
|
||||
Some(label),
|
||||
|
||||
@@ -19,10 +19,11 @@ use crate::{
|
||||
};
|
||||
|
||||
/// Whether `label` denotes a user-created token rather than a system token
|
||||
/// (`session`, `guest_session`, `ephemeral*`, `debugger-token`, `mcp-oauth-*`). System-token
|
||||
/// labels are load-bearing — session cleanup, super_admin propagation, expiry
|
||||
/// notifications and username overrides all key off them — so they must not be
|
||||
/// user-editable. `None` (no label) is treated as a user token.
|
||||
/// (`session`, `guest_session`, `ephemeral*`, `debugger-token`, `mcp-oauth-*`,
|
||||
/// `embed_app:*`, `sdk_app:*`, `impersonation:*`). System-token labels are load-bearing —
|
||||
/// session cleanup, super_admin propagation, expiry notifications and username overrides
|
||||
/// all key off them — so they must not be user-editable. `None` (no label) is treated as
|
||||
/// a user token.
|
||||
///
|
||||
/// This is the canonical copy. When updating it, also update its mirrors:
|
||||
/// - the `update_token_label` editability guard (SQL `WHERE`) in
|
||||
@@ -40,15 +41,30 @@ pub fn is_user_token(label: Option<&str>) -> bool {
|
||||
&& !l.to_lowercase().starts_with("ephemeral")
|
||||
&& l != "debugger-token"
|
||||
&& !l.starts_with("mcp-oauth-")
|
||||
// Short-lived tokens the server mints per app open or per service-account
|
||||
// impersonation (EE `users_ee.rs`) and nobody manages, so an expiry warning
|
||||
// for one is noise.
|
||||
&& !l.starts_with(APP_EMBED_TOKEN_LABEL_PREFIX)
|
||||
&& !l.starts_with(RAW_APP_SDK_TOKEN_LABEL_PREFIX)
|
||||
&& !l.starts_with("impersonation:")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Label prefix, followed by the app path, of the token an app viewer's sandboxed iframe
|
||||
/// runs with. Reserved in [`is_user_token`], whose SQL and frontend mirrors spell it out.
|
||||
pub const APP_EMBED_TOKEN_LABEL_PREFIX: &str = "embed_app:";
|
||||
|
||||
/// Label prefix, followed by the app path, of the token a raw app's bundle uses for the
|
||||
/// frontend SDK. Reserved in [`is_user_token`], whose SQL and frontend mirrors spell it out.
|
||||
pub const RAW_APP_SDK_TOKEN_LABEL_PREFIX: &str = "sdk_app:";
|
||||
|
||||
/// Whether `label` belongs to a namespace only the server mints, and which therefore must be
|
||||
/// rejected by `create_token`. Narrower than [`is_user_token`], which also drives label
|
||||
/// editability and expiry notifications and can afford to reserve more: `Ephemeral lsp token`
|
||||
/// and `debugger-token` are minted by the editor and the debugger through that same handler,
|
||||
/// so reserving them would break those features.
|
||||
/// editability and expiry notifications and can afford to reserve more: `Ephemeral lsp token`,
|
||||
/// `debugger-token` and `ephemeral-test-connection: *` are minted by the editor, the debugger
|
||||
/// and object-storage connection tests through that same handler, so reserving them would
|
||||
/// break those features.
|
||||
///
|
||||
/// `username_override_from_label` trusts a label to name the entity acting only if it is in
|
||||
/// here, so anything added must be unmintable by a member.
|
||||
@@ -961,6 +977,9 @@ mod tests {
|
||||
assert!(!is_user_token(Some("Ephemeral lsp token")));
|
||||
assert!(!is_user_token(Some("debugger-token")));
|
||||
assert!(!is_user_token(Some("mcp-oauth-client")));
|
||||
assert!(!is_user_token(Some("embed_app:f/team/dashboard")));
|
||||
assert!(!is_user_token(Some("sdk_app:u/admin/raw app")));
|
||||
assert!(!is_user_token(Some("impersonation:admin@windmill.dev")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -12,6 +12,14 @@ Symbols, not line numbers, are cited: they drift less.
|
||||
by `create_session_token` (`windmill-api-users/src/users.rs`). `GET /api/users/refresh_token`
|
||||
mints one for any non-job token but returns plain text, no redirect.
|
||||
- **`tokens/impersonate`** (superadmin) returns a multi-use token and sets no cookie.
|
||||
- **A token's label decides whether its expiry raises alerts.** When `delete_expired_items`
|
||||
removes an expired `token` row, the monitor emails the owner and raises a critical alert (if
|
||||
enabled); rows registered by `register_token_expiry_notification` also get an "expiring soon"
|
||||
warning first. Neither happens when `is_user_token` (`windmill-common/src/auth.rs`) reserves
|
||||
the label, so a token the system mints for itself, whether from the backend or from the frontend
|
||||
through `tokens/create`, needs a reserved label. An `ephemeral-` prefix needs no other change
|
||||
(keep it clear of `is_server_minted_label` if minted through `tokens/create`); a new prefix
|
||||
also goes into the SQL and Svelte mirrors that function's doc lists.
|
||||
- **Every superadmin route refuses a job token**: `require_super_admin`
|
||||
(`windmill-api-auth/src/lib.rs`) errors on `authed.job_id.is_some()`. A script that needs
|
||||
`users/create`, `tokens/impersonate`, `set_login_type`, … must use a dedicated superadmin user
|
||||
|
||||
@@ -174,7 +174,7 @@ export async function main(bucket: any, api_token: string) {
|
||||
async function mintApiToken(): Promise<string> {
|
||||
return await UserService.createToken({
|
||||
requestBody: {
|
||||
label: `test connection: ${resourceType}`,
|
||||
label: `ephemeral-test-connection: ${resourceType}`,
|
||||
expiration: new Date(Date.now() + API_TOKEN_TTL_MS).toISOString(),
|
||||
scopes: ['settings:write']
|
||||
}
|
||||
|
||||
@@ -59,7 +59,10 @@
|
||||
label !== 'guest_session' &&
|
||||
!label.toLowerCase().startsWith('ephemeral') &&
|
||||
label !== 'debugger-token' &&
|
||||
!label.startsWith('mcp-oauth-')
|
||||
!label.startsWith('mcp-oauth-') &&
|
||||
!label.startsWith('embed_app:') &&
|
||||
!label.startsWith('sdk_app:') &&
|
||||
!label.startsWith('impersonation:')
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user