diff --git a/authentik/admin/files/tests/test_validation.py b/authentik/admin/files/tests/test_validation.py index 1c575f2d03d6..859e4ac36468 100644 --- a/authentik/admin/files/tests/test_validation.py +++ b/authentik/admin/files/tests/test_validation.py @@ -62,10 +62,10 @@ def test_sanitize_invalid_characters_raises(self): "test@file.png", # @ "test#file.png", # # "test$file.png", # $ - "test%file.png", # % + "test%file.png", # % (but %(theme)s is allowed) "test&file.png", # & "test*file.png", # * - "test(file).png", # parentheses + "test(file).png", # parentheses (but %(theme)s is allowed) "test[file].png", # brackets "test{file}.png", # braces ] @@ -108,3 +108,30 @@ def test_sanitize_too_long_component_raises(self): with self.assertRaises(ValidationError): validate_file_name(path) + + def test_sanitize_theme_variable_valid(self): + """Test sanitizing filename with %(theme)s variable""" + # These should all be valid + validate_file_name("logo-%(theme)s.png") + validate_file_name("brand/logo-%(theme)s.svg") + validate_file_name("images/icon-%(theme)s.png") + validate_file_name("%(theme)s/logo.png") + validate_file_name("brand/%(theme)s/logo.png") + + def test_sanitize_theme_variable_multiple(self): + """Test sanitizing filename with multiple %(theme)s variables""" + validate_file_name("%(theme)s/logo-%(theme)s.png") + + def test_sanitize_theme_variable_invalid_format(self): + """Test that partial or malformed theme variables are rejected""" + invalid_paths = [ + "test%(theme.png", # missing )s + "test%theme)s.png", # missing ( + "test%(themes).png", # wrong variable name + "test%(THEME)s.png", # wrong case + "test%()s.png", # empty variable name + ] + + for path in invalid_paths: + with self.assertRaises(ValidationError): + validate_file_name(path) diff --git a/authentik/admin/files/validation.py b/authentik/admin/files/validation.py index 6da0fa802031..333fa04b3416 100644 --- a/authentik/admin/files/validation.py +++ b/authentik/admin/files/validation.py @@ -12,6 +12,10 @@ MAX_FILE_NAME_LENGTH = 1024 MAX_PATH_COMPONENT_LENGTH = 255 +# Theme variable placeholder that can be used in file paths +# This allows for theme-specific files like logo-%(theme)s.png +THEME_VARIABLE = "%(theme)s" + def validate_file_name(name: str) -> None: if PassthroughBackend(FileUsage.MEDIA).supports_file(name) or StaticBackend( @@ -39,12 +43,17 @@ def validate_upload_file_name( if not name: raise ValidationError(_("File name cannot be empty")) - # Same regex is used in the frontend as well - if not re.match(r"^[a-zA-Z0-9._/-]+$", name): + # Allow %(theme)s placeholder for theme-specific files + # We temporarily replace it for validation, then check the result + name_for_validation = name.replace(THEME_VARIABLE, "theme") + + # Same regex is used in the frontend as well (without %(theme)s handling there) + if not re.match(r"^[a-zA-Z0-9._/-]+$", name_for_validation): raise ValidationError( _( "File name can only contain letters (a-z, A-Z), numbers (0-9), " - "dots (.), hyphens (-), underscores (_), and forward slashes (/)" + "dots (.), hyphens (-), underscores (_), forward slashes (/), " + "and the special placeholder %(theme)s for theme-specific files" ) ) diff --git a/internal/web/static.go b/internal/web/static.go index 1050bfa2c861..3e9b9671eca8 100644 --- a/internal/web/static.go +++ b/internal/web/static.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "fmt" "net/http" + "strings" "time" "github.com/go-http-utils/etag" @@ -17,11 +18,44 @@ import ( staticWeb "goauthentik.io/web" ) +// Theme variable placeholder that can be used in file paths +// This allows for theme-specific files like logo-%(theme)s.png +const themeVariable = "%(theme)s" + +// Valid themes that can be substituted for %(theme)s +var validThemes = []string{"light", "dark"} + type StorageClaims struct { jwt.RegisteredClaims Path string `json:"path,omitempty"` } +// pathMatchesWithTheme checks if the requested path matches the JWT path, +// accounting for theme variable substitution. +// If the JWT path contains %(theme)s, it will match the requested path +// if substituting %(theme)s with any valid theme produces the requested path. +func pathMatchesWithTheme(jwtPath, requestedPath string) bool { + // Direct match (no theme variable) + if jwtPath == requestedPath { + return true + } + + // Check if JWT path contains theme variable + if !strings.Contains(jwtPath, themeVariable) { + return false + } + + // Try substituting each valid theme and check for a match + for _, theme := range validThemes { + substituted := strings.ReplaceAll(jwtPath, themeVariable, theme) + if substituted == requestedPath { + return true + } + } + + return false +} + func storageTokenIsValid(usage string, r *http.Request) bool { tokenString := r.URL.Query().Get("token") if tokenString == "" { @@ -51,11 +85,8 @@ func storageTokenIsValid(usage string, r *http.Request) bool { return false } - if claims.Path != fmt.Sprintf("%s/%s", usage, r.URL.Path) { - return false - } - - return true + requestedPath := fmt.Sprintf("%s/%s", usage, r.URL.Path) + return pathMatchesWithTheme(claims.Path, requestedPath) } func (ws *WebServer) configureStatic() { diff --git a/internal/web/static_test.go b/internal/web/static_test.go new file mode 100644 index 000000000000..0d30b195180e --- /dev/null +++ b/internal/web/static_test.go @@ -0,0 +1,95 @@ +package web + +import "testing" + +func TestPathMatchesWithTheme(t *testing.T) { + tests := []struct { + name string + jwtPath string + requestedPath string + want bool + }{ + { + name: "exact match without theme variable", + jwtPath: "media/public/logo.png", + requestedPath: "media/public/logo.png", + want: true, + }, + { + name: "no match without theme variable", + jwtPath: "media/public/logo.png", + requestedPath: "media/public/other.png", + want: false, + }, + { + name: "theme variable matches light theme", + jwtPath: "media/public/logo-%(theme)s.png", + requestedPath: "media/public/logo-light.png", + want: true, + }, + { + name: "theme variable matches dark theme", + jwtPath: "media/public/logo-%(theme)s.png", + requestedPath: "media/public/logo-dark.png", + want: true, + }, + { + name: "theme variable does not match invalid theme", + jwtPath: "media/public/logo-%(theme)s.png", + requestedPath: "media/public/logo-blue.png", + want: false, + }, + { + name: "theme variable in directory path", + jwtPath: "media/%(theme)s/logo.png", + requestedPath: "media/light/logo.png", + want: true, + }, + { + name: "multiple theme variables", + jwtPath: "media/%(theme)s/logo-%(theme)s.png", + requestedPath: "media/light/logo-light.png", + want: true, + }, + { + name: "multiple theme variables with dark", + jwtPath: "media/%(theme)s/logo-%(theme)s.png", + requestedPath: "media/dark/logo-dark.png", + want: true, + }, + { + name: "multiple theme variables mixed themes should not match", + jwtPath: "media/%(theme)s/logo-%(theme)s.png", + requestedPath: "media/light/logo-dark.png", + want: false, + }, + { + name: "theme variable with nested path", + jwtPath: "media/public/brand/logo-%(theme)s.svg", + requestedPath: "media/public/brand/logo-dark.svg", + want: true, + }, + { + name: "empty paths", + jwtPath: "", + requestedPath: "", + want: true, + }, + { + name: "theme variable only", + jwtPath: "%(theme)s", + requestedPath: "light", + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := pathMatchesWithTheme(tt.jwtPath, tt.requestedPath) + if got != tt.want { + t.Errorf("pathMatchesWithTheme(%q, %q) = %v, want %v", + tt.jwtPath, tt.requestedPath, got, tt.want) + } + }) + } +} diff --git a/web/src/admin/files/FileUploadForm.ts b/web/src/admin/files/FileUploadForm.ts index 39f94bd11b11..c92cd06d1cc8 100644 --- a/web/src/admin/files/FileUploadForm.ts +++ b/web/src/admin/files/FileUploadForm.ts @@ -14,18 +14,26 @@ import { html } from "lit"; import { customElement, property, state } from "lit/decorators.js"; import { createRef, ref } from "lit/directives/ref.js"; -// Same regex is used in the backend as well +// Theme variable placeholder that can be used in file paths +// This allows for theme-specific files like logo-%(theme)s.png +const THEME_VARIABLE = "%(theme)s"; + +// Same regex is used in the backend as well (after replacing %(theme)s) const VALID_FILE_NAME_PATTERN = /^[a-zA-Z0-9._/-]+$/; // Note: browsers compile `pattern` using the new `v` RegExp flag (Unicode sets). Under `/v`, // both `/` and `-` must be escaped inside character classes. -const VALID_FILE_NAME_PATTERN_STRING = "^[a-zA-Z0-9._\\/\\-]+$"; +// This pattern allows %(theme)s by including %, (, and ) characters +const VALID_FILE_NAME_PATTERN_STRING = "^[a-zA-Z0-9._\\/\\-%()+]+$"; function assertValidFileName(fileName: string): void { - if (!VALID_FILE_NAME_PATTERN.test(fileName)) { + // Allow %(theme)s placeholder for theme-specific files + // We temporarily replace it for validation, then check the result + const nameForValidation = fileName.replaceAll(THEME_VARIABLE, "theme"); + if (!VALID_FILE_NAME_PATTERN.test(nameForValidation)) { throw new Error( msg( - "Filename can only contain letters, numbers, dots, hyphens, underscores, and slashes", + "Filename can only contain letters, numbers, dots, hyphens, underscores, slashes, and the special placeholder %(theme)s", ), ); }