mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 08:02:18 +00:00
fix: give the guest JWKS https-required refusal its own clear error
An http:// JWKS URL without the opt-in returned the shared DisallowedScheme,
whose message says "only http and https are permitted" — rejecting http while
saying it is allowed. Add a distinct HttpsRequired variant ("URL must use
https"), correct validate_guest_jwks_url's doc (the flag now opts in the scheme
too, not just private ranges), and note in the Guests card that a JWKS URL must
be https.
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
773e80a20a
commit
ea8734a5b6
@@ -889,7 +889,7 @@ y9rTR828ADcaZ63Ej1oL4GcqmGhODxCLy1YKKcy0FHzChqPMV6g=\n\
|
||||
// a plaintext URL (which an on-path attacker could replace) is refused for its scheme.
|
||||
assert!(matches!(
|
||||
crate::ssrf::validate_guest_jwks_url("http://issuer.example.com/jwks.json").await,
|
||||
Err(crate::ssrf::SsrfValidationError::DisallowedScheme(_))
|
||||
Err(crate::ssrf::SsrfValidationError::HttpsRequired)
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,6 +20,9 @@ pub enum SsrfValidationError {
|
||||
InvalidUrl(String),
|
||||
/// Scheme is not `http`/`https`.
|
||||
DisallowedScheme(String),
|
||||
/// The URL uses `http` where `https` is required (guest JWKS). The private-host opt-in
|
||||
/// also permits `http`, so, unlike the other scheme errors, this one the flag can fix.
|
||||
HttpsRequired,
|
||||
/// No host in the URL.
|
||||
MissingHost,
|
||||
/// DNS resolution failed for the host.
|
||||
@@ -39,6 +42,9 @@ impl std::fmt::Display for SsrfValidationError {
|
||||
f,
|
||||
"URL scheme '{s}' is not allowed, only http and https are permitted"
|
||||
),
|
||||
SsrfValidationError::HttpsRequired => {
|
||||
write!(f, "URL must use https")
|
||||
}
|
||||
SsrfValidationError::MissingHost => write!(f, "URL must have a host"),
|
||||
SsrfValidationError::ResolutionFailed { host, source } => {
|
||||
write!(f, "Failed to resolve host '{host}': {source}")
|
||||
@@ -216,8 +222,8 @@ pub async fn validate_saml_metadata_url(url: &str) -> Result<ValidatedTarget, Ss
|
||||
}
|
||||
|
||||
/// Validate a workspace admin's guest-JWKS URL and return the [`ValidatedTarget`] so
|
||||
/// the fetch can pin the connect. Same shape as [`validate_mcp_server_url`]: a private
|
||||
/// range is refused unless the operator opts in with `ALLOW_PRIVATE_GUEST_JWKS_URLS`.
|
||||
/// the fetch can pin the connect. `https` is required (the JWKS authenticates guest JWTs);
|
||||
/// `ALLOW_PRIVATE_GUEST_JWKS_URLS` opts a private range AND plaintext `http` in, for dev.
|
||||
pub async fn validate_guest_jwks_url(url: &str) -> Result<ValidatedTarget, SsrfValidationError> {
|
||||
let parsed =
|
||||
url::Url::parse(url).map_err(|e| SsrfValidationError::InvalidUrl(e.to_string()))?;
|
||||
@@ -232,6 +238,7 @@ pub async fn validate_guest_jwks_url(url: &str) -> Result<ValidatedTarget, SsrfV
|
||||
// hosts (dev/loopback): the JWKS supplies the keys that authenticate guest JWTs, so an
|
||||
// on-path attacker who could replace an http response could forge accepted tokens.
|
||||
"http" if allow_private => {}
|
||||
"http" => return Err(SsrfValidationError::HttpsRequired),
|
||||
scheme => return Err(SsrfValidationError::DisallowedScheme(scheme.to_string())),
|
||||
}
|
||||
|
||||
|
||||
@@ -2265,9 +2265,9 @@ export async function main(
|
||||
<code>exp</code> (lifetime capped at 24h); it opens only the app named by
|
||||
<code>app_path</code>. Accepted algorithms: RS256/384/512, PS256/384/512,
|
||||
ES256/384. Symmetric algorithms (HS*) are refused. Configure one key, a PEM
|
||||
public key or a JWKS URL. Point it at an issuer you control: any token that key
|
||||
signs carrying these claims is accepted, so a shared multi-tenant issuer is not
|
||||
a good fit.
|
||||
public key or a JWKS URL (which must be https). Point it at an issuer you
|
||||
control: any token that key signs carrying these claims is accepted, so a shared
|
||||
multi-tenant issuer is not a good fit.
|
||||
</div>
|
||||
<ToggleButtonGroup bind:selected={guestJwtKeyType}>
|
||||
{#snippet children({ item })}
|
||||
|
||||
Reference in New Issue
Block a user