mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
feat: treat an empty allowlist as unset at both levels
This commit is contained in:
@@ -231,12 +231,12 @@ pub fn effective_allowed_origins<'a>(
|
||||
route_allowed_origins: Option<&'a [String]>,
|
||||
instance_default: &'a [String],
|
||||
) -> Option<&'a [String]> {
|
||||
match route_allowed_origins {
|
||||
// An empty list is not a configuration. It reads exactly as never having set
|
||||
// one, so such a route still inherits the instance default rather than
|
||||
// skipping it, which is what would make `[]` more permissive than `NULL`.
|
||||
match route_allowed_origins.filter(|list| !list.is_empty()) {
|
||||
// `*` is the opt-out, including out of a stricter instance default.
|
||||
Some(list) if allows_any_origin(list) => None,
|
||||
// Any other stored list restricts, an empty one included: it allows no
|
||||
// origin at all. Falling back to the default here would make `[]` more
|
||||
// permissive than `NULL`, which is the wrong direction to fail in.
|
||||
Some(list) => Some(list),
|
||||
None => (!instance_default.is_empty() && !allows_any_origin(instance_default))
|
||||
.then_some(instance_default),
|
||||
@@ -708,14 +708,14 @@ mod tests {
|
||||
effective_allowed_origins(Some(&["*".to_string()]), &default),
|
||||
None
|
||||
);
|
||||
// An empty route list is a restriction that matches nothing, distinct
|
||||
// from `NULL` which inherits the instance default. It must never come
|
||||
// back as `None`, which the middleware reads as "any origin".
|
||||
// An empty route list is not a configuration: it resolves exactly as
|
||||
// `NULL` does, so it inherits the instance default rather than skipping
|
||||
// it and becoming more permissive than an unset one.
|
||||
assert_eq!(
|
||||
effective_allowed_origins(Some(&[]), &default),
|
||||
Some(&[][..])
|
||||
Some(&default[..])
|
||||
);
|
||||
assert_eq!(match_origin(&[], Some(&origin("https://a.com"))), None);
|
||||
assert_eq!(effective_allowed_origins(Some(&[]), &[]), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -266,7 +266,7 @@ export const settings: Record<string, Setting[]> = {
|
||||
{
|
||||
label: 'HTTP route default allowed origins',
|
||||
description:
|
||||
'Origins that HTTP routes allow to call them from a browser when the route sets none of its own. A route overrides this with its own list, and opts out entirely by setting its allowed origins to *. Leave empty to let every route be called from any origin.',
|
||||
'Origins that HTTP routes allow to call them from a browser when the route sets none of its own. A route overrides this with its own list, and opts out entirely by setting its allowed origins to *. Leave unset for no instance-wide default, so every route is callable from any origin unless it restricts itself.',
|
||||
key: 'http_route_default_allowed_origins',
|
||||
fieldType: 'text',
|
||||
placeholder: 'https://app.example.com, https://admin.example.com',
|
||||
|
||||
@@ -107,16 +107,19 @@
|
||||
error={error !== undefined}
|
||||
/>
|
||||
<!-- One line, per the form guideline's single Input -> Validation/Hint
|
||||
slot. Red is what the API refuses, yellow is what saves but will
|
||||
never match, and an empty list is neither: it allows no origin at
|
||||
all, which is a state a route can legitimately be in. -->
|
||||
slot. Red is what the API refuses, yellow is what saves but can never
|
||||
match, and an empty field is neither: it reads as no list at all. -->
|
||||
{#if error}
|
||||
<div class="text-2xs text-red-600 dark:text-red-400">{error}</div>
|
||||
{:else if warning}
|
||||
<div class="text-2xs text-yellow-600 dark:text-yellow-400">{warning}</div>
|
||||
{:else if origins.length === 0 && hasInstanceDefault}
|
||||
<div class="text-2xs text-secondary">
|
||||
Empty, so the instance default still applies: {instanceDefaultOrigins.join(', ')}.
|
||||
</div>
|
||||
{:else if origins.length === 0}
|
||||
<div class="text-2xs text-secondary">
|
||||
Allows no origin. Add one, comma-separated, or * to allow any.
|
||||
Empty, so any origin may call this route. Add one, comma-separated.
|
||||
</div>
|
||||
{:else}
|
||||
<div class="text-2xs text-secondary">Comma-separated. Use * to allow any.</div>
|
||||
|
||||
@@ -102,16 +102,17 @@ export function parseAllowedOriginsSetting(setting: unknown): string[] {
|
||||
|
||||
/**
|
||||
* Whether a route is restricted to specific origins, mirroring
|
||||
* `effective_allowed_origins` in windmill-trigger-http: a route with its own
|
||||
* list restricts, an empty one included since it then matches no origin at all;
|
||||
* only a route without one falls back to the instance default, and `*` in
|
||||
* either is the opt-out.
|
||||
* `effective_allowed_origins` in windmill-trigger-http: a route with a non-empty
|
||||
* list of its own restricts, `*` in it is the opt-out, and anything else — an
|
||||
* empty list included, since that is not a configuration — falls back to the
|
||||
* instance default.
|
||||
*/
|
||||
export function isOriginRestricted(
|
||||
allowed_origins: string[] | undefined,
|
||||
instanceDefaultOrigins: string[]
|
||||
): boolean {
|
||||
if (allowed_origins !== undefined) return !allowed_origins.includes('*')
|
||||
if (allowed_origins !== undefined && allowed_origins.length > 0)
|
||||
return !allowed_origins.includes('*')
|
||||
return instanceDefaultOrigins.length > 0 && !instanceDefaultOrigins.includes('*')
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user