mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 00:01:49 +00:00
fix: single-flight the JWKS refresh; design-system textarea; doc placement
- Refresh a workspace's JWKS under a per-URL lock, so a burst against a cold or stale entry triggers one fetch, not one per request (single-flight). The "at most once per interval" bound now holds while a fetch is in flight, not only after it lands. - A cached entry with no keys is a remembered failure; serving it reported an unreachable issuer as an unknown `kid`. Map an empty entry to an issuer-unreachable error instead. - Use the design-system TextInput (textarea variant) for the PEM key field rather than a raw <textarea>. - Move `has_any_account` above `username_to_permissioned_as` so it no longer sits between that function's doc comment and its body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
77b24b20cd
commit
98e8a594f2
@@ -174,6 +174,9 @@ struct JwksEntry {
|
||||
|
||||
lazy_static::lazy_static! {
|
||||
static ref JWKS_CACHE: Cache<String, Arc<JwksEntry>> = Cache::new(200);
|
||||
/// Per-URL fetch lock: only one refresh per URL is in flight at a time, so a cold
|
||||
/// or stale entry under a burst triggers one fetch, not one per request.
|
||||
static ref JWKS_FETCH_LOCKS: Cache<String, Arc<tokio::sync::Mutex<()>>> = Cache::new(200);
|
||||
}
|
||||
|
||||
/// How long a good key set is served before a refresh; also the lag before a
|
||||
@@ -236,15 +239,40 @@ pub async fn fetch_jwks(url: &str) -> Result<HashMap<String, Jwk>> {
|
||||
Ok(keys)
|
||||
}
|
||||
|
||||
/// A cached entry that holds no keys is a remembered failure; serving it would report
|
||||
/// an unreachable issuer as an unknown `kid`. Map it to an issuer-unreachable error.
|
||||
fn servable(entry: Arc<JwksEntry>) -> Result<Arc<JwksEntry>> {
|
||||
if entry.keys.is_empty() {
|
||||
Err(Error::NotAuthorized(
|
||||
"guest JWT refused: the JWKS issuer is unreachable".to_string(),
|
||||
))
|
||||
} else {
|
||||
Ok(entry)
|
||||
}
|
||||
}
|
||||
|
||||
/// The workspace's JWKS, from cache when fresh. A cold or stale entry is refetched
|
||||
/// once; a failed refetch serves the last good keys (or an empty set) for
|
||||
/// `JWKS_NEGATIVE_TTL`, so an unreachable issuer is fetched at most once per that
|
||||
/// interval whatever the guest-JWT traffic. Fetches thus follow a schedule, never a
|
||||
/// per-request, attacker-chosen `kid`.
|
||||
/// under a per-URL lock, so a burst triggers one fetch, not one per request; a failed
|
||||
/// refetch serves the last good keys, or a short-lived empty entry that reads as
|
||||
/// "issuer unreachable" rather than "kid not found", so an unreachable issuer is hit at
|
||||
/// most once per `JWKS_NEGATIVE_TTL`. Fetches follow a schedule, never a per-request,
|
||||
/// attacker-chosen `kid`.
|
||||
async fn cached_jwks(url: &str) -> Result<Arc<JwksEntry>> {
|
||||
if let Some(entry) = JWKS_CACHE.get(url) {
|
||||
if entry.expires_at > Instant::now() {
|
||||
return Ok(entry);
|
||||
return servable(entry);
|
||||
}
|
||||
}
|
||||
// Single-flight: hold the per-URL lock across the fetch. `get_or_insert_with`
|
||||
// creates the lock atomically, so two cold requests share one.
|
||||
let lock = JWKS_FETCH_LOCKS
|
||||
.get_or_insert_with(url, || Ok::<_, ()>(Arc::new(tokio::sync::Mutex::new(()))))
|
||||
.unwrap();
|
||||
let _guard = lock.lock().await;
|
||||
// Another task may have refreshed while we waited for the lock.
|
||||
if let Some(entry) = JWKS_CACHE.get(url) {
|
||||
if entry.expires_at > Instant::now() {
|
||||
return servable(entry);
|
||||
}
|
||||
}
|
||||
match fetch_jwks(url).await {
|
||||
@@ -256,13 +284,11 @@ async fn cached_jwks(url: &str) -> Result<Arc<JwksEntry>> {
|
||||
}
|
||||
Err(e) => {
|
||||
let keys = JWKS_CACHE.get(url).map(|stale| stale.keys.clone()).unwrap_or_default();
|
||||
let empty = keys.is_empty();
|
||||
let entry =
|
||||
Arc::new(JwksEntry { keys, expires_at: Instant::now() + JWKS_NEGATIVE_TTL });
|
||||
JWKS_CACHE.insert(url.to_string(), entry.clone());
|
||||
// Nothing good was ever cached: surface the fetch error rather than an
|
||||
// empty key set that reads as "kid not found".
|
||||
if empty {
|
||||
// Nothing good was ever cached: surface the fetch error itself.
|
||||
if entry.keys.is_empty() {
|
||||
return Err(e);
|
||||
}
|
||||
Ok(entry)
|
||||
|
||||
@@ -31,11 +31,6 @@ pub const USERNAME_GROUP_PREFIX: &str = "group-";
|
||||
/// columns runnables and triggers store one in.
|
||||
pub const PERMISSIONED_AS_MAX_LEN: usize = 55;
|
||||
|
||||
/// An email-shaped username is its own principal, which is how a superadmin acting without a
|
||||
/// `usr` row is named (`usr.username` is constrained to `[\w-]+`, so a member never is). It is
|
||||
/// decided before the group convention — an address is never a group's username — and one
|
||||
/// containing `/` is prefixed, since readers split on the first `/` and would otherwise take
|
||||
/// `g/alice@example.com` for a group.
|
||||
/// Whether any account exists for `email`: a `password` row (deactivated ones
|
||||
/// included, since the sign-in path filters `disabled = false` and a re-enabled
|
||||
/// account must not read as absent) or a `usr` row in any workspace (what a service
|
||||
@@ -59,6 +54,11 @@ pub async fn has_any_account<'c, E: sqlx::Executor<'c, Database = sqlx::Postgres
|
||||
.map_err(|e| crate::error::Error::internal_err(format!("checking account for {email}: {e:#}")))
|
||||
}
|
||||
|
||||
/// An email-shaped username is its own principal, which is how a superadmin acting without a
|
||||
/// `usr` row is named (`usr.username` is constrained to `[\w-]+`, so a member never is). It is
|
||||
/// decided before the group convention — an address is never a group's username — and one
|
||||
/// containing `/` is prefixed, since readers split on the first `/` and would otherwise take
|
||||
/// `g/alice@example.com` for a group.
|
||||
pub fn username_to_permissioned_as(user: &str) -> String {
|
||||
if user.contains('@') {
|
||||
return if user.contains('/') {
|
||||
|
||||
@@ -2258,11 +2258,15 @@ export async function main(
|
||||
{/snippet}
|
||||
</ToggleButtonGroup>
|
||||
{#if guestJwtKeyType === 'pem'}
|
||||
<textarea
|
||||
class="w-full h-32 font-mono text-xs p-2 border rounded resize-y bg-surface text-primary"
|
||||
placeholder={'-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----'}
|
||||
<TextInput
|
||||
underlyingInputEl="textarea"
|
||||
class="font-mono text-xs"
|
||||
autosizeParams={{ minHeight: 128 }}
|
||||
inputProps={{
|
||||
placeholder: '-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----'
|
||||
}}
|
||||
bind:value={guestJwtPublicKey}
|
||||
></textarea>
|
||||
/>
|
||||
{:else}
|
||||
<TextInput
|
||||
inputProps={{ placeholder: 'https://issuer.example.com/.well-known/jwks.json' }}
|
||||
|
||||
Reference in New Issue
Block a user