@@ -654,7 +654,7 @@ const ActionIconButton = ({
className={cn(
"inline-flex h-9 w-9 cursor-default items-center justify-center rounded-md outline-none",
"transition-colors duration-150",
- "focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940",
+ "focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940",
variant === "danger"
? "text-nb-gray-400 hover:bg-red-500/10 hover:text-red-500"
: "text-nb-gray-400 hover:bg-nb-gray-900 hover:text-nb-gray-100",
diff --git a/client/ui/frontend/src/modules/settings/SettingsAbout.tsx b/client/ui/frontend/src/modules/settings/SettingsAbout.tsx
index 8ba1009ba..c103d1d74 100644
--- a/client/ui/frontend/src/modules/settings/SettingsAbout.tsx
+++ b/client/ui/frontend/src/modules/settings/SettingsAbout.tsx
@@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next";
import { Browser } from "@wailsio/runtime";
import { BookOpen, MessageSquareText, MessagesSquare } from "lucide-react";
import netbirdFull from "@/assets/logos/netbird-full.svg";
+import netbirdFullLight from "@/assets/logos/netbird-full-light.svg";
// Brand glyphs from simpleicons.org (lucide deprecated its brand icons).
const GithubIcon = (props: SVGProps) => (
@@ -90,7 +91,16 @@ export function SettingsAbout() {
"mx-auto flex min-h-[calc(100vh-12rem)] max-w-2xl flex-col items-center justify-center gap-4"
}
>
-
+
+
openUrl(url)}
className={
- "inline-flex items-center gap-1.5 rounded-sm decoration-[0.5px] underline-offset-4 outline-none transition hover:text-nb-gray-100 hover:underline focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940"
+ "inline-flex items-center gap-1.5 rounded-sm decoration-[0.5px] underline-offset-4 outline-none transition hover:text-nb-gray-100 hover:underline focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940"
}
>
@@ -157,7 +167,7 @@ export function SettingsAbout() {
tabIndex={0}
onClick={() => openUrl(link.url)}
className={
- "rounded-sm decoration-[0.5px] underline-offset-4 outline-none transition hover:text-nb-gray-100 hover:underline focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940"
+ "rounded-sm decoration-[0.5px] underline-offset-4 outline-none transition hover:text-nb-gray-100 hover:underline focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940"
}
>
{link.label}
diff --git a/client/ui/frontend/src/modules/settings/SettingsGeneral.tsx b/client/ui/frontend/src/modules/settings/SettingsGeneral.tsx
index 71720aebe..5e0327648 100644
--- a/client/ui/frontend/src/modules/settings/SettingsGeneral.tsx
+++ b/client/ui/frontend/src/modules/settings/SettingsGeneral.tsx
@@ -10,6 +10,7 @@ import { useAutostartSetting, useSettings } from "@/contexts/SettingsContext.tsx
import { ManagementServerSwitch } from "@/components/ManagementServerSwitch.tsx";
import { ManagementMode, useManagementUrl } from "@/hooks/useManagementUrl.ts";
import { LanguagePicker } from "@/components/LanguagePicker.tsx";
+import { ThemePicker } from "@/components/ThemePicker.tsx";
import { useRestrictions } from "@/contexts/RestrictionsContext.tsx";
import { useKeepConnectedOnQuit } from "@/hooks/useKeepConnectedOnQuit.ts";
@@ -36,6 +37,7 @@ export function SettingsGeneral() {
<>
+
setField("disableNotifications", !v)}
diff --git a/client/ui/frontend/src/modules/settings/SettingsSection.tsx b/client/ui/frontend/src/modules/settings/SettingsSection.tsx
index adba65cdc..4b67fe7eb 100644
--- a/client/ui/frontend/src/modules/settings/SettingsSection.tsx
+++ b/client/ui/frontend/src/modules/settings/SettingsSection.tsx
@@ -16,7 +16,7 @@ export const SectionGroup = ({
{...(disabled ? { inert: "" } : {})}
className={cn(
"mb-8 rounded-md px-1 outline-none last:mb-1",
- "focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940",
+ "focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940",
disabled && "pointer-events-none opacity-30",
)}
>
@@ -33,7 +33,7 @@ export const SettingsBottomBar = ({ children }: { children: ReactNode }) => (
{children}
diff --git a/client/ui/frontend/src/modules/settings/SettingsTroubleshooting.tsx b/client/ui/frontend/src/modules/settings/SettingsTroubleshooting.tsx
index 8b1774ed6..9eac41d08 100644
--- a/client/ui/frontend/src/modules/settings/SettingsTroubleshooting.tsx
+++ b/client/ui/frontend/src/modules/settings/SettingsTroubleshooting.tsx
@@ -99,7 +99,7 @@ export function SettingsTroubleshooting() {
"rounded-md border bg-white dark:bg-nb-gray-900",
"border-neutral-200 dark:border-nb-gray-700",
"cursor-default text-xs font-semibold text-nb-gray-100 outline-none",
- "hover:border-nb-gray-600 data-[state=open]:border-nb-gray-600",
+ "hover:border-nb-gray-700 data-[state=open]:border-nb-gray-700 dark:hover:border-nb-gray-600 dark:data-[state=open]:border-nb-gray-600",
)}
>
{t(`settings.troubleshooting.anonymize.${anonymizeLevel}`)}
@@ -277,7 +277,10 @@ function DoneResult({
};
return (
-
+
@@ -326,7 +329,9 @@ function DoneResult({
@@ -339,7 +344,7 @@ function DoneResult({
{result.uploadFailureReason
diff --git a/client/ui/frontend/tailwind.config.ts b/client/ui/frontend/tailwind.config.ts
index 93ff39eea..da41ee111 100644
--- a/client/ui/frontend/tailwind.config.ts
+++ b/client/ui/frontend/tailwind.config.ts
@@ -10,29 +10,32 @@ const config: Config = {
},
extend: {
colors: {
+ // nb-gray is theme-aware: channels come from CSS variables in
+ // globals.css (:root = light ramp, .dark = original dark ramp). The
+ // rgb(var(...) /
) form keeps opacity modifiers working.
"nb-gray": {
- DEFAULT: "#181A1D",
- 50: "#f4f6f7",
- 100: "#e4e7e9",
- 200: "#cbd2d6",
- 250: "#b7c0c6",
- 300: "#a3adb5",
- 350: "#8f9ca8",
- 400: "#7c8994",
- 500: "#616e79",
- 600: "#535d67",
- 700: "#474e57",
- 800: "#3f444b",
- 850: "#363b40",
- 900: "#2e3238",
- 910: "#2b2f33",
- 920: "#25282d",
- 925: "#1e2123",
- 930: "#25282c",
- 935: "#1f2124",
- 940: "#1c1e21",
- 950: "#181a1d",
- 960: "#16181b",
+ DEFAULT: "rgb(var(--nb-gray-DEFAULT) / )",
+ 50: "rgb(var(--nb-gray-50) / )",
+ 100: "rgb(var(--nb-gray-100) / )",
+ 200: "rgb(var(--nb-gray-200) / )",
+ 250: "rgb(var(--nb-gray-250) / )",
+ 300: "rgb(var(--nb-gray-300) / )",
+ 350: "rgb(var(--nb-gray-350) / )",
+ 400: "rgb(var(--nb-gray-400) / )",
+ 500: "rgb(var(--nb-gray-500) / )",
+ 600: "rgb(var(--nb-gray-600) / )",
+ 700: "rgb(var(--nb-gray-700) / )",
+ 800: "rgb(var(--nb-gray-800) / )",
+ 850: "rgb(var(--nb-gray-850) / )",
+ 900: "rgb(var(--nb-gray-900) / )",
+ 910: "rgb(var(--nb-gray-910) / )",
+ 920: "rgb(var(--nb-gray-920) / )",
+ 925: "rgb(var(--nb-gray-925) / )",
+ 930: "rgb(var(--nb-gray-930) / )",
+ 935: "rgb(var(--nb-gray-935) / )",
+ 940: "rgb(var(--nb-gray-940) / )",
+ 950: "rgb(var(--nb-gray-950) / )",
+ 960: "rgb(var(--nb-gray-960) / )",
},
gray: {
50: "#F9FAFB",
diff --git a/client/ui/i18n/locales/de/common.json b/client/ui/i18n/locales/de/common.json
index 11e085927..c39584992 100644
--- a/client/ui/i18n/locales/de/common.json
+++ b/client/ui/i18n/locales/de/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Keine Sprachen gefunden."
},
+ "settings.general.theme.label": {
+ "message": "Erscheinungsbild"
+ },
+ "settings.general.theme.help": {
+ "message": "Hell, Dunkel oder die Systemeinstellung verwenden."
+ },
+ "settings.general.theme.system": {
+ "message": "System"
+ },
+ "settings.general.theme.light": {
+ "message": "Hell"
+ },
+ "settings.general.theme.dark": {
+ "message": "Dunkel"
+ },
"settings.general.management.label": {
"message": "Management-Server"
},
diff --git a/client/ui/i18n/locales/en/common.json b/client/ui/i18n/locales/en/common.json
index 36f00e4bd..e9ee26de4 100644
--- a/client/ui/i18n/locales/en/common.json
+++ b/client/ui/i18n/locales/en/common.json
@@ -759,6 +759,26 @@
"message": "No languages match.",
"description": "Shown when no languages match the search."
},
+ "settings.general.theme.label": {
+ "message": "Theme",
+ "description": "Label for the appearance/theme picker."
+ },
+ "settings.general.theme.help": {
+ "message": "Choose light or dark, or follow your system appearance.",
+ "description": "Helper text for the theme picker."
+ },
+ "settings.general.theme.system": {
+ "message": "System",
+ "description": "Theme option: follow the OS appearance."
+ },
+ "settings.general.theme.light": {
+ "message": "Light",
+ "description": "Theme option: light appearance."
+ },
+ "settings.general.theme.dark": {
+ "message": "Dark",
+ "description": "Theme option: dark appearance."
+ },
"settings.general.management.label": {
"message": "Management Server",
"description": "Label for the management-server selector."
diff --git a/client/ui/i18n/locales/es/common.json b/client/ui/i18n/locales/es/common.json
index 41872d7a0..245b5aa5f 100644
--- a/client/ui/i18n/locales/es/common.json
+++ b/client/ui/i18n/locales/es/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Ningún idioma coincide."
},
+ "settings.general.theme.label": {
+ "message": "Tema"
+ },
+ "settings.general.theme.help": {
+ "message": "Elija el tema claro u oscuro, o siga la apariencia del sistema."
+ },
+ "settings.general.theme.system": {
+ "message": "Sistema"
+ },
+ "settings.general.theme.light": {
+ "message": "Claro"
+ },
+ "settings.general.theme.dark": {
+ "message": "Oscuro"
+ },
"settings.general.management.label": {
"message": "Servidor de gestión"
},
diff --git a/client/ui/i18n/locales/fr/common.json b/client/ui/i18n/locales/fr/common.json
index 920ef8343..6da66a643 100644
--- a/client/ui/i18n/locales/fr/common.json
+++ b/client/ui/i18n/locales/fr/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Aucune langue ne correspond."
},
+ "settings.general.theme.label": {
+ "message": "Thème"
+ },
+ "settings.general.theme.help": {
+ "message": "Choisissez le thème clair ou sombre, ou suivez l'apparence du système."
+ },
+ "settings.general.theme.system": {
+ "message": "Système"
+ },
+ "settings.general.theme.light": {
+ "message": "Clair"
+ },
+ "settings.general.theme.dark": {
+ "message": "Sombre"
+ },
"settings.general.management.label": {
"message": "Serveur de gestion"
},
diff --git a/client/ui/i18n/locales/hu/common.json b/client/ui/i18n/locales/hu/common.json
index 82996e3d3..1b4d2fb9d 100644
--- a/client/ui/i18n/locales/hu/common.json
+++ b/client/ui/i18n/locales/hu/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Nincs találat."
},
+ "settings.general.theme.label": {
+ "message": "Téma"
+ },
+ "settings.general.theme.help": {
+ "message": "Válassza a világos vagy sötét témát, vagy kövesse a rendszer beállítását."
+ },
+ "settings.general.theme.system": {
+ "message": "Rendszer"
+ },
+ "settings.general.theme.light": {
+ "message": "Világos"
+ },
+ "settings.general.theme.dark": {
+ "message": "Sötét"
+ },
"settings.general.management.label": {
"message": "Felügyeleti szerver"
},
diff --git a/client/ui/i18n/locales/it/common.json b/client/ui/i18n/locales/it/common.json
index b8166aa6e..4cee0f842 100644
--- a/client/ui/i18n/locales/it/common.json
+++ b/client/ui/i18n/locales/it/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Nessuna lingua corrisponde."
},
+ "settings.general.theme.label": {
+ "message": "Tema"
+ },
+ "settings.general.theme.help": {
+ "message": "Scelga il tema chiaro o scuro, oppure segua l'aspetto del sistema."
+ },
+ "settings.general.theme.system": {
+ "message": "Sistema"
+ },
+ "settings.general.theme.light": {
+ "message": "Chiaro"
+ },
+ "settings.general.theme.dark": {
+ "message": "Scuro"
+ },
"settings.general.management.label": {
"message": "Server di gestione"
},
diff --git a/client/ui/i18n/locales/ja/common.json b/client/ui/i18n/locales/ja/common.json
index 6ffe05e1c..4fc81d283 100644
--- a/client/ui/i18n/locales/ja/common.json
+++ b/client/ui/i18n/locales/ja/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "一致する言語がありません。"
},
+ "settings.general.theme.label": {
+ "message": "テーマ"
+ },
+ "settings.general.theme.help": {
+ "message": "ライト、ダーク、またはシステムの外観に従います。"
+ },
+ "settings.general.theme.system": {
+ "message": "システム"
+ },
+ "settings.general.theme.light": {
+ "message": "ライト"
+ },
+ "settings.general.theme.dark": {
+ "message": "ダーク"
+ },
"settings.general.management.label": {
"message": "管理サーバー"
},
diff --git a/client/ui/i18n/locales/pt/common.json b/client/ui/i18n/locales/pt/common.json
index 123e7a042..cb4a542d0 100644
--- a/client/ui/i18n/locales/pt/common.json
+++ b/client/ui/i18n/locales/pt/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Nenhum idioma corresponde."
},
+ "settings.general.theme.label": {
+ "message": "Tema"
+ },
+ "settings.general.theme.help": {
+ "message": "Escolha claro, escuro ou siga a aparência do sistema."
+ },
+ "settings.general.theme.system": {
+ "message": "Sistema"
+ },
+ "settings.general.theme.light": {
+ "message": "Claro"
+ },
+ "settings.general.theme.dark": {
+ "message": "Escuro"
+ },
"settings.general.management.label": {
"message": "Servidor de gerenciamento"
},
diff --git a/client/ui/i18n/locales/ru/common.json b/client/ui/i18n/locales/ru/common.json
index 3881a3783..61ece03b8 100644
--- a/client/ui/i18n/locales/ru/common.json
+++ b/client/ui/i18n/locales/ru/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "Языки не найдены."
},
+ "settings.general.theme.label": {
+ "message": "Тема"
+ },
+ "settings.general.theme.help": {
+ "message": "Выберите светлую или тёмную тему либо следуйте системной."
+ },
+ "settings.general.theme.system": {
+ "message": "Системная"
+ },
+ "settings.general.theme.light": {
+ "message": "Светлая"
+ },
+ "settings.general.theme.dark": {
+ "message": "Тёмная"
+ },
"settings.general.management.label": {
"message": "Сервер управления"
},
diff --git a/client/ui/i18n/locales/uk/common.json b/client/ui/i18n/locales/uk/common.json
index 4e3f24102..f8fe71562 100644
--- a/client/ui/i18n/locales/uk/common.json
+++ b/client/ui/i18n/locales/uk/common.json
@@ -569,6 +569,21 @@
"settings.general.language.empty": {
"message": "Не знайдено жодної мови."
},
+ "settings.general.theme.label": {
+ "message": "Тема"
+ },
+ "settings.general.theme.help": {
+ "message": "Виберіть світлу чи темну тему або використовуйте системні налаштування."
+ },
+ "settings.general.theme.system": {
+ "message": "Системна"
+ },
+ "settings.general.theme.light": {
+ "message": "Світла"
+ },
+ "settings.general.theme.dark": {
+ "message": "Темна"
+ },
"settings.general.management.label": {
"message": "Сервер керування"
},
@@ -764,7 +779,7 @@
"settings.troubleshooting.anonymize.help": {
"message": "Приховує IP-адреси, домени та інші конфіденційні дані."
},
- "settings.troubleshooting.anonymize.info": {
+ "settings.troubleshooting.anonymize.info": {
"message": "«Стандартний» залишає внутрішні адреси IPv4 та імена пірів читабельними для служби підтримки. «Суворий» додатково анонімізує приватні (RFC 1918), CGNAT- та link-local-адреси, імена пірів і публічні ключі WireGuard. Однакові значення замінюються тим самим псевдонімом, тож піри залишаються розрізнюваними. Використовуйте «Суворий», якщо ділитеся архівом за межами організації."
},
"settings.troubleshooting.anonymize.none": {
@@ -1370,7 +1385,7 @@
"settings.ssh.privilege.oneWayInverted": {
"message": "Ви можете увімкнути це, але щоб вимкнути знову, знадобиться {actor}:"
},
- "settings.ssh.privilege.authorizePending": {
+ "settings.ssh.privilege.authorizePending": {
"message": "Очікування авторизації…"
}
}
diff --git a/client/ui/i18n/locales/zh-CN/common.json b/client/ui/i18n/locales/zh-CN/common.json
index b1ff3370d..126b11851 100644
--- a/client/ui/i18n/locales/zh-CN/common.json
+++ b/client/ui/i18n/locales/zh-CN/common.json
@@ -571,6 +571,21 @@
"settings.general.language.empty": {
"message": "没有匹配的语言。"
},
+ "settings.general.theme.label": {
+ "message": "主题"
+ },
+ "settings.general.theme.help": {
+ "message": "选择浅色、深色或跟随系统外观。"
+ },
+ "settings.general.theme.system": {
+ "message": "跟随系统"
+ },
+ "settings.general.theme.light": {
+ "message": "浅色"
+ },
+ "settings.general.theme.dark": {
+ "message": "深色"
+ },
"settings.general.management.label": {
"message": "管理服务器"
},
diff --git a/client/ui/main.go b/client/ui/main.go
index 5652efcf2..74a87b4df 100644
--- a/client/ui/main.go
+++ b/client/ui/main.go
@@ -77,6 +77,7 @@ func init() {
application.RegisterEvent[authsession.Warning](services.EventSessionWarning)
application.RegisterEvent[updater.State](updater.EventStateChanged)
application.RegisterEvent[preferences.UIPreferences](preferences.EventPreferencesChanged)
+ application.RegisterEvent[services.SystemTheme](services.EventSystemThemeChanged)
}
func main() {
@@ -100,7 +101,7 @@ func main() {
var tray *Tray
app := newApplication(func() {
if tray != nil {
- tray.ShowWindow()
+ go tray.ShowWindow()
}
})
@@ -123,6 +124,9 @@ func main() {
bundle, prefStore, localizer := buildI18n(app)
+ // Before any window exists so creation-time backgrounds are already themed.
+ app.RegisterService(application.NewService(services.NewTheme(app, prefStore)))
+
// After bundle + prefStore: both are used to localise daemon errors.
settings := services.NewSettings(conn, bundle, prefStore, daemonAddr)
connection := services.NewConnection(conn, bundle, prefStore)
@@ -354,6 +358,7 @@ func newMainWindow(app *application.App, prefStore *preferences.Store, wm *servi
if prefStore.Get().ViewMode == preferences.ViewModeAdvanced {
initialWidth = 900
}
+ appearance := services.CurrentAppearance()
window := app.Window.NewWithOptions(application.WebviewWindowOptions{
Name: "main",
Title: "NetBird",
@@ -363,13 +368,13 @@ func newMainWindow(app *application.App, prefStore *preferences.Store, wm *servi
// drop new windows top-left unless asked.
InitialPosition: application.WindowCentered,
Hidden: true,
- BackgroundColour: services.WindowBackgroundColour,
+ BackgroundColour: services.WindowBackgroundColour(appearance),
URL: startURL,
DisableResize: true,
MinimiseButtonState: application.ButtonHidden,
MaximiseButtonState: application.ButtonHidden,
- Mac: services.AppleMacOSAppearanceOptions(),
- Windows: services.MicrosoftWindowsAppearanceOptions(),
+ Mac: services.AppleMacOSAppearanceOptions(appearance),
+ Windows: services.MicrosoftWindowsAppearanceOptions(appearance),
Linux: application.LinuxWindow{
Icon: iconWindow,
},
diff --git a/client/ui/preferences/store.go b/client/ui/preferences/store.go
index 3b677016f..9f3f9297e 100644
--- a/client/ui/preferences/store.go
+++ b/client/ui/preferences/store.go
@@ -49,10 +49,34 @@ func (v ViewMode) IsValid() bool {
return false
}
+// Theme is the preferred UI appearance: follow the OS ("system") or force
+// "light"/"dark".
+type Theme string
+
+const (
+ ThemeSystem Theme = "system"
+ ThemeLight Theme = "light"
+ ThemeDark Theme = "dark"
+)
+
+// DefaultTheme applies when no file exists or its theme is empty/unknown.
+const DefaultTheme = ThemeSystem
+
+var ErrUnsupportedTheme = errors.New("unsupported theme")
+
+func (t Theme) IsValid() bool {
+ switch t {
+ case ThemeSystem, ThemeLight, ThemeDark:
+ return true
+ }
+ return false
+}
+
// UIPreferences is rewritten in full on every change; there are no partial updates.
type UIPreferences struct {
Language i18n.LanguageCode `json:"language"`
ViewMode ViewMode `json:"viewMode"`
+ Theme Theme `json:"theme"`
OnboardingCompleted bool `json:"onboardingCompleted"`
// AutostartInitialized records that the one-time autostart default
// decision has run for this OS user. It only ever transitions to true
@@ -105,7 +129,7 @@ func NewStore(validator LanguageValidator, emitter Emitter) (*Store, error) {
path: path,
validator: validator,
emitter: emitter,
- current: UIPreferences{ViewMode: DefaultViewMode},
+ current: UIPreferences{ViewMode: DefaultViewMode, Theme: DefaultTheme},
}
if err := s.load(); err != nil {
@@ -146,6 +170,30 @@ func (s *Store) SetViewMode(mode ViewMode) error {
return nil
}
+// SetTheme validates, persists, and broadcasts. No-op if unchanged.
+func (s *Store) SetTheme(theme Theme) error {
+ if !theme.IsValid() {
+ return fmt.Errorf("%w: %q", ErrUnsupportedTheme, theme)
+ }
+
+ s.mu.Lock()
+ if s.current.Theme == theme {
+ s.mu.Unlock()
+ return nil
+ }
+ next := s.current
+ next.Theme = theme
+ if err := s.persistLocked(next); err != nil {
+ s.mu.Unlock()
+ return fmt.Errorf("persist preferences: %w", err)
+ }
+ s.current = next
+ s.mu.Unlock()
+
+ s.broadcast(next)
+ return nil
+}
+
// SetOnboardingCompleted persists the welcome-window dismissal. No-op if unchanged.
func (s *Store) SetOnboardingCompleted(done bool) error {
s.mu.Lock()
@@ -288,6 +336,9 @@ func (s *Store) load() error {
if !loaded.ViewMode.IsValid() {
loaded.ViewMode = DefaultViewMode
}
+ if !loaded.Theme.IsValid() {
+ loaded.Theme = DefaultTheme
+ }
s.mu.Lock()
s.current = loaded
diff --git a/client/ui/services/appappearance_linux.go b/client/ui/services/appappearance_linux.go
new file mode 100644
index 000000000..0cfc46d5c
--- /dev/null
+++ b/client/ui/services/appappearance_linux.go
@@ -0,0 +1,99 @@
+//go:build linux && cgo && !android && !ios
+
+package services
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+)
+
+// setAppAppearance points GTK at the light or dark variant of the current theme
+// so the decorations match the webview. Without it a forced Light theme keeps
+// dark decorations on a dark desktop, and the reverse.
+//
+// The theme name is switched, not just gtk-application-prefer-dark-theme:
+// desktops such as Ubuntu implement dark mode as a separate theme (Yaru-dark),
+// which that flag cannot lighten. The flag is still set for themes that do
+// carry both variants under one name. Both are per-process settings, so this
+// changes only our own decorations; GTK re-reads the desktop value on a change,
+// which is why Theme.apply re-asserts. Must run on the main thread.
+//
+// GTK styling is app-wide, which is why this is separate from
+// setWindowAppearance: it must be applied even when no window exists yet, since
+// windows created later inherit it rather than carrying it in their options.
+func setAppAppearance(dark bool) {
+ target := baseGtkTheme(gtkThemeName())
+ if dark {
+ if variant, ok := darkGtkVariant(target); ok {
+ target = variant
+ }
+ }
+ // An unknown name would leave GTK with no theme at all, so fall back to
+ // changing nothing and let the prefer-dark flag do what it can.
+ if target != "" && !gtkThemeExists(target) {
+ target = ""
+ }
+ applyGtkTheme(target, dark)
+}
+
+// baseGtkTheme strips a dark-variant suffix, so "Yaru-dark" becomes "Yaru".
+func baseGtkTheme(name string) string {
+ for _, suffix := range []string{"-dark", "-Dark"} {
+ if len(name) > len(suffix) && strings.EqualFold(name[len(name)-len(suffix):], suffix) {
+ return name[:len(name)-len(suffix)]
+ }
+ }
+ return name
+}
+
+// darkGtkVariant reports the installed dark counterpart of a base theme name.
+// Themes that carry both variants under one name have none, and rely on
+// gtk-application-prefer-dark-theme instead.
+func darkGtkVariant(base string) (string, bool) {
+ if base == "" {
+ return "", false
+ }
+ for _, suffix := range []string{"-dark", "-Dark"} {
+ if candidate := base + suffix; gtkThemeExists(candidate) {
+ return candidate, true
+ }
+ }
+ return "", false
+}
+
+// gtkThemeExists reports whether a theme of that name is installed, searching
+// the same locations GTK does.
+func gtkThemeExists(name string) bool {
+ if name == "" {
+ return false
+ }
+ for _, dir := range gtkThemeDirs() {
+ if info, err := os.Stat(filepath.Join(dir, name)); err == nil && info.IsDir() {
+ return true
+ }
+ }
+ return false
+}
+
+func gtkThemeDirs() []string {
+ var dirs []string
+ if home, err := os.UserHomeDir(); err == nil {
+ dirs = append(dirs, filepath.Join(home, ".themes"))
+ }
+ if dataHome := os.Getenv("XDG_DATA_HOME"); dataHome != "" {
+ dirs = append(dirs, filepath.Join(dataHome, "themes"))
+ } else if home, err := os.UserHomeDir(); err == nil {
+ dirs = append(dirs, filepath.Join(home, ".local", "share", "themes"))
+ }
+ dataDirs := os.Getenv("XDG_DATA_DIRS")
+ if dataDirs == "" {
+ dataDirs = "/usr/local/share:/usr/share"
+ }
+ for _, dir := range strings.Split(dataDirs, ":") {
+ if dir != "" {
+ dirs = append(dirs, filepath.Join(dir, "themes"))
+ }
+ }
+ return dirs
+}
diff --git a/client/ui/services/appappearance_linux_gtk.go b/client/ui/services/appappearance_linux_gtk.go
new file mode 100644
index 000000000..2648b2808
--- /dev/null
+++ b/client/ui/services/appappearance_linux_gtk.go
@@ -0,0 +1,63 @@
+//go:build linux && cgo && !android && !ios
+
+package services
+
+/*
+// The GTK major version is the only difference between the two Linux builds, so
+// it is selected by these two directives rather than by keeping a second copy of
+// this file per version: the C below and the Go wrappers under it are identical
+// for GTK3 and GTK4, and both resolve through pkg-config.
+#cgo gtk3 pkg-config: gtk+-3.0
+#cgo !gtk3 pkg-config: gtk4
+#include
+#include
+
+static char *nbGetGtkThemeName(void) {
+ GtkSettings *settings = gtk_settings_get_default();
+ if (settings == NULL) {
+ return NULL;
+ }
+ char *name = NULL;
+ g_object_get(settings, "gtk-theme-name", &name, NULL);
+ return name;
+}
+
+// name may be NULL to leave the theme name untouched.
+static void nbSetGtkTheme(const char *name, int dark) {
+ GtkSettings *settings = gtk_settings_get_default();
+ if (settings == NULL) {
+ return;
+ }
+ if (name != NULL && name[0] != '\0') {
+ g_object_set(settings, "gtk-theme-name", name, NULL);
+ }
+ g_object_set(settings, "gtk-application-prefer-dark-theme", dark ? TRUE : FALSE, NULL);
+}
+
+static void nbFreeGtkString(char *s) { g_free(s); }
+*/
+import "C"
+
+import "unsafe"
+
+func gtkThemeName() string {
+ c := C.nbGetGtkThemeName()
+ if c == nil {
+ return ""
+ }
+ defer C.nbFreeGtkString(c)
+ return C.GoString(c)
+}
+
+func applyGtkTheme(name string, dark bool) {
+ var cName *C.char
+ if name != "" {
+ cName = C.CString(name)
+ defer C.free(unsafe.Pointer(cName))
+ }
+ var forced C.int
+ if dark {
+ forced = 1
+ }
+ C.nbSetGtkTheme(cName, forced)
+}
diff --git a/client/ui/services/appappearance_other.go b/client/ui/services/appappearance_other.go
new file mode 100644
index 000000000..aaad912b3
--- /dev/null
+++ b/client/ui/services/appappearance_other.go
@@ -0,0 +1,7 @@
+//go:build !(linux && cgo)
+
+package services
+
+// setAppAppearance is a no-op where the platform has no app-wide appearance to
+// set; macOS and Windows theme each window instead, via setWindowAppearance.
+func setAppAppearance(bool) {}
diff --git a/client/ui/services/preferences.go b/client/ui/services/preferences.go
index 77faa4ef6..592d52e7b 100644
--- a/client/ui/services/preferences.go
+++ b/client/ui/services/preferences.go
@@ -31,6 +31,10 @@ func (s *Preferences) SetViewMode(_ context.Context, mode preferences.ViewMode)
return s.store.SetViewMode(mode)
}
+func (s *Preferences) SetTheme(_ context.Context, theme preferences.Theme) error {
+ return s.store.SetTheme(theme)
+}
+
func (s *Preferences) SetOnboardingCompleted(_ context.Context, done bool) error {
return s.store.SetOnboardingCompleted(done)
}
diff --git a/client/ui/services/theme.go b/client/ui/services/theme.go
new file mode 100644
index 000000000..52215e34b
--- /dev/null
+++ b/client/ui/services/theme.go
@@ -0,0 +1,194 @@
+//go:build !android && !ios && !freebsd && !js
+
+package services
+
+import (
+ "context"
+ "sync"
+ "sync/atomic"
+
+ "github.com/wailsapp/wails/v3/pkg/application"
+ "github.com/wailsapp/wails/v3/pkg/events"
+
+ "github.com/netbirdio/netbird/client/ui/preferences"
+)
+
+// EventSystemThemeChanged fires when the OS appearance flips, payload SystemTheme.
+// The frontend resolves the "system" preference against it.
+const EventSystemThemeChanged = "netbird:system-theme:changed"
+
+// SystemTheme is the EventSystemThemeChanged payload.
+type SystemTheme struct {
+ Dark bool `json:"dark"`
+}
+
+// Theme keeps native window background colours in step with the persisted
+// theme preference so no window flashes the wrong surface before the webview
+// paints. The frontend applies the matching .dark class via ThemeContext.
+type Theme struct {
+ app *application.App
+ store *preferences.Store
+ // mu serializes apply: concurrent callers could otherwise enqueue a stale
+ // pref's native updates after a newer one's.
+ mu sync.Mutex
+ // started gates the main-thread dispatch in apply: Run installs the platform
+ // layer InvokeAsync needs, and the store subscription can fire before that.
+ started atomic.Bool
+}
+
+// NewTheme wires the store subscription and OS theme-change listener. Call
+// before any window is created so creation-time colours are already themed.
+func NewTheme(app *application.App, store *preferences.Store) *Theme {
+ t := &Theme{app: app, store: store}
+ pref := store.Get().Theme
+ setAppearance(pref, resolveDark(pref, app.Env.IsDarkMode()))
+
+ // Window creation resolves through this rather than the seed above, which
+ // is wrong until Run installs the platform layer: Env.IsDarkMode reports
+ // light before that, so a "system" launch on a dark OS would build the
+ // first window light. The ApplicationStarted apply below cannot be relied
+ // on to land first because Wails runs each listener in its own goroutine.
+ // One store read backs both fields, so the snapshot is always self-consistent.
+ setAppearanceResolver(func() Appearance {
+ p := t.store.Get().Theme
+ return Appearance{Pref: p, Dark: resolveDark(p, t.app.Env.IsDarkMode())}
+ })
+
+ ch, _ := store.Subscribe()
+ go func() {
+ var last preferences.Theme
+ for p := range ch {
+ if p.Theme == last {
+ continue
+ }
+ last = p.Theme
+ t.apply()
+ }
+ }()
+
+ // Re-apply on every OS flip, not just for ThemeSystem: Windows re-evaluates
+ // process-level theme state on WM_SETTINGCHANGE, so a forced theme has to be
+ // re-asserted or the native chrome drifts to the OS appearance. The event's
+ // own IsDarkMode is deliberately unused: Wails runs each application event
+ // handler in its own goroutine, so two rapid flips race, and apply re-reads
+ // the appearance under mu instead.
+ app.Event.OnApplicationEvent(events.Common.ThemeChanged, func(*application.ApplicationEvent) {
+ t.apply()
+ })
+
+ // Startup is split in two because Wails runs every application-event
+ // listener in its own goroutine, so a listener cannot be ordered against the
+ // one that opens the first-launch window. Hooks can: they run sequentially,
+ // in registration order, and all of them before any listener is spawned.
+ //
+ // The app-wide GTK theme goes in the hook because it is the part a window
+ // must not be created without. On Linux it draws the decorations and
+ // application.LinuxWindow carries no theme of its own, so a window built
+ // before it lands shows OS-coloured decorations until it does. It is applied
+ // synchronously for the same reason -- returning from the hook has to mean
+ // the theme is live. This relies on the listener below existing: Wails skips
+ // an event's hooks entirely when it has no listeners.
+ app.Event.RegisterApplicationEventHook(events.Common.ApplicationStarted, func(*application.ApplicationEvent) {
+ t.started.Store(true)
+ t.syncAppAppearance()
+ })
+
+ // The rest of the startup apply. Env.IsDarkMode is a stub until the platform
+ // layer is up, so re-resolve once the app has started or a "system" launch on
+ // a light OS stays seeded dark.
+ app.Event.OnApplicationEvent(events.Common.ApplicationStarted, func(*application.ApplicationEvent) {
+ t.apply()
+ })
+
+ return t
+}
+
+// syncAppAppearance applies the app-wide appearance and waits for the UI thread
+// to have done it. Use it where a window is about to be created and must not be
+// built against the OS appearance: apply dispatches its own native work
+// asynchronously, so on Linux the GTK theme behind the decorations can otherwise
+// land after the window exists.
+//
+// No-op before the app has started, where InvokeSync has no platform layer to
+// dispatch to. Reads the appearance under mu like apply, so the two cannot
+// interleave into a torn update.
+func (t *Theme) syncAppAppearance() {
+ if !t.started.Load() {
+ return
+ }
+ t.mu.Lock()
+ defer t.mu.Unlock()
+
+ pref := t.store.Get().Theme
+ dark := resolveDark(pref, t.app.Env.IsDarkMode())
+ setAppearance(pref, dark)
+ application.InvokeSync(func() { setAppAppearance(dark) })
+}
+
+// SystemDarkMode reports the OS appearance; bound so the frontend can resolve
+// the "system" preference from the same source as the native layer.
+func (t *Theme) SystemDarkMode(_ context.Context) (bool, error) {
+ return t.app.Env.IsDarkMode(), nil
+}
+
+// resolveDark maps a preference to an effective appearance against a system
+// reading the caller already took.
+func resolveDark(pref preferences.Theme, systemDark bool) bool {
+ switch pref {
+ case preferences.ThemeDark:
+ return true
+ case preferences.ThemeLight:
+ return false
+ default:
+ return systemDark
+ }
+}
+
+// apply recomputes the effective appearance, re-tints every live window
+// (including the macOS NSWindow appearance so the frame matches the webview)
+// and publishes the system appearance the frontend resolves "system" against.
+//
+// Everything runs under mu and reads the appearance here rather than taking it
+// from a caller, so a later apply always carries the fresher state and the
+// frontend event is ordered by the same lock as the native assignments. Emit
+// only appends to a FIFO mailbox, so holding mu across it cannot block.
+//
+// The OS is read exactly once per update and the resolved value is passed on to
+// the background and the native chrome, so those cannot land on either side of
+// an OS flip that happens mid-apply. The event carries the raw system reading,
+// not the resolved one, because the frontend resolves "system" itself.
+func (t *Theme) apply() {
+ t.mu.Lock()
+ defer t.mu.Unlock()
+
+ pref := t.store.Get().Theme
+ systemDark := t.app.Env.IsDarkMode()
+ dark := resolveDark(pref, systemDark)
+ setAppearance(pref, dark)
+ t.app.Event.Emit(EventSystemThemeChanged, SystemTheme{Dark: systemDark})
+
+ // Before Run there is no platform layer for InvokeAsync to dispatch to.
+ // Windows created later read the globals set above.
+ if !t.started.Load() {
+ return
+ }
+
+ colour := windowBackgroundColour(dark)
+ // Re-tint on the UI thread and resolve each native handle there. Window
+ // teardown (markAsDestroyed then impl.close) runs as UI-thread work too, so
+ // a window closed meanwhile is either gone from GetAll or yields a nil
+ // handle -- never a freed handle the OS may already have reused.
+ application.InvokeAsync(func() {
+ // App-wide first, and unconditionally: on Linux this is the GTK theme
+ // that draws the decorations, and it must be set even with no window
+ // open because later windows inherit it instead of carrying it.
+ setAppAppearance(dark)
+ for _, w := range t.app.Window.GetAll() {
+ if w == nil {
+ continue
+ }
+ w.SetBackgroundColour(colour)
+ setWindowAppearance(w.NativeWindow(), pref, dark)
+ }
+ })
+}
diff --git a/client/ui/services/windowappearance_darwin.go b/client/ui/services/windowappearance_darwin.go
new file mode 100644
index 000000000..3c0a28447
--- /dev/null
+++ b/client/ui/services/windowappearance_darwin.go
@@ -0,0 +1,53 @@
+package services
+
+/*
+#cgo CFLAGS: -x objective-c
+#cgo LDFLAGS: -framework AppKit
+
+#import
+
+// forced < 0: follow the OS (appearance nil); 0: light; 1: dark.
+//
+// Assigns directly rather than dispatching: Theme.apply already runs this on
+// the main thread. Deferring would outlive the caller's check that the window
+// is alive, and the __bridge cast does not retain it, so the block could touch
+// a freed NSWindow.
+static void nbSetWindowAppearance(void *nsWindow, int forced) {
+ NSWindow *window = (__bridge NSWindow *)nsWindow;
+ if (forced < 0) {
+ window.appearance = nil;
+ } else {
+ NSAppearanceName name = forced == 1 ? NSAppearanceNameDarkAqua : NSAppearanceNameAqua;
+ window.appearance = [NSAppearance appearanceNamed:name];
+ }
+}
+*/
+import "C"
+
+import (
+ "unsafe"
+
+ "github.com/netbirdio/netbird/client/ui/preferences"
+)
+
+// setWindowAppearance pins the NSWindow appearance to the forced theme, or
+// hands it back to the OS for ThemeSystem. Without this, a window created
+// under one OS appearance keeps its dark/light frame after a manual theme
+// flip, leaving a mismatched border around the webview. Must run on the main
+// thread, which Theme.apply guarantees.
+//
+// The resolved appearance is unused: for ThemeSystem a nil NSAppearance lets
+// AppKit track the OS itself, which cannot drift from a snapshot we took.
+func setWindowAppearance(nsWindow unsafe.Pointer, pref preferences.Theme, _ bool) {
+ if nsWindow == nil {
+ return
+ }
+ forced := C.int(-1)
+ switch pref {
+ case preferences.ThemeLight:
+ forced = 0
+ case preferences.ThemeDark:
+ forced = 1
+ }
+ C.nbSetWindowAppearance(nsWindow, forced)
+}
diff --git a/client/ui/services/windowappearance_other.go b/client/ui/services/windowappearance_other.go
new file mode 100644
index 000000000..c2e9fbf74
--- /dev/null
+++ b/client/ui/services/windowappearance_other.go
@@ -0,0 +1,16 @@
+//go:build !darwin && !windows && !android && !ios && !freebsd && !js
+
+package services
+
+import (
+ "unsafe"
+
+ "github.com/netbirdio/netbird/client/ui/preferences"
+)
+
+// setWindowAppearance is a no-op wherever there is no per-window appearance to
+// set, which is every target this file covers. On Linux the appearance is real
+// but app-wide, so setAppAppearance owns it instead; on the remaining Unix
+// targets there is no native theming to apply at all and setAppAppearance is
+// itself a stub (appappearance_other.go).
+func setWindowAppearance(unsafe.Pointer, preferences.Theme, bool) {}
diff --git a/client/ui/services/windowappearance_windows.go b/client/ui/services/windowappearance_windows.go
new file mode 100644
index 000000000..8d5c88e47
--- /dev/null
+++ b/client/ui/services/windowappearance_windows.go
@@ -0,0 +1,54 @@
+package services
+
+import (
+ "unsafe"
+
+ "github.com/wailsapp/wails/v3/pkg/w32"
+
+ "github.com/netbirdio/netbird/client/ui/preferences"
+)
+
+// setWindowAppearance re-themes a live window's chrome; Wails only does this
+// itself on OS flips for SystemDefault windows.
+//
+// Must run on the UI thread, which Theme.apply guarantees: the uxtheme and
+// repaint calls behind w32.SetTheme belong to the window's thread, and hwnd is
+// only known live while we hold that thread. Re-dispatching here would let the
+// window be destroyed first and hand these writes a reused handle.
+//
+// dark is the appearance Theme.apply already resolved. Re-reading the OS here
+// would let the chrome land on the other side of an OS flip from the window
+// background and the webview.
+func setWindowAppearance(hwnd unsafe.Pointer, _ preferences.Theme, dark bool) {
+ if hwnd == nil || !w32.SupportsThemes() || w32.IsCurrentlyHighContrastMode() {
+ return
+ }
+
+ h := uintptr(hwnd)
+ w32.SetTheme(h, dark)
+
+ // After SetTheme, not before: its menu helper regates dark on the
+ // process-level ShouldAppsUseDarkMode and rewrites the per-window opt-in
+ // with that gated value, so forcing Dark on a light OS would lose it --
+ // and builds below 18985 need the opt-in for the pre-20H1 dark frame. The
+ // gated menu theme name is left alone on purpose: these windows carry no
+ // native menu, and popup-menu text follows the process policy, so forcing
+ // it dark gives dark text on dark.
+ if w32.AllowDarkModeForWindow != nil {
+ w32.AllowDarkModeForWindow(h, dark)
+ }
+
+ chrome := microsoftWindowsLightTheme
+ if dark {
+ chrome = microsoftWindowsDarkTheme
+ }
+ if chrome.TitleBarColour != nil {
+ w32.SetTitleBarColour(h, *chrome.TitleBarColour)
+ }
+ if chrome.TitleTextColour != nil {
+ w32.SetTitleTextColour(h, *chrome.TitleTextColour)
+ }
+ if chrome.BorderColour != nil {
+ w32.SetBorderColour(h, *chrome.BorderColour)
+ }
+}
diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go
index 94dba6038..24319dae0 100644
--- a/client/ui/services/windowmanager.go
+++ b/client/ui/services/windowmanager.go
@@ -6,6 +6,7 @@ import (
"net/url"
"strconv"
"sync"
+ "sync/atomic"
"time"
log "github.com/sirupsen/logrus"
@@ -21,6 +22,10 @@ type LanguageSubscriber interface {
Subscribe() (<-chan preferences.UIPreferences, func())
}
+type windowOp func(w *application.WebviewWindow, created bool)
+
+type windowCloser func(w *application.WebviewWindow)
+
// EventTriggerLogin asks the frontend's startLogin() to begin an SSO flow.
const EventTriggerLogin = "trigger-login"
@@ -36,39 +41,133 @@ const paintedFallback = 2 * time.Second
const headlessTeardownDelay = 2 * time.Second
-var WindowBackgroundColour = application.NewRGB(24, 26, 29) // bg-nb-gray-950
+const (
+ windowMain = "main"
+ windowSettings = "settings"
+ windowBrowserLogin = "browser-login"
+ windowSessionExpiration = "session-expiration"
+ windowInstallProgress = "install-progress"
+ windowWelcome = "welcome"
+ windowError = "error"
+)
+
+// Window background per effective appearance. Both match the body background
+// (bg-nb-gray DEFAULT) in globals.css so opaque native pixels and the webview
+// paint the same surface; keep the three in sync.
+var (
+ windowBackgroundDark = application.NewRGB(24, 26, 29) // dark nb-gray DEFAULT
+ windowBackgroundLight = application.NewRGB(243, 243, 243) // light nb-gray DEFAULT
+)
+
+// Appearance is one view of the theme state: the preference and the appearance
+// it resolves to. Take it once per window with CurrentAppearance and pass the
+// same value to every option builder -- Pref drives the macOS frame while Dark
+// drives the background and the Windows chrome, so reading them separately can
+// build a window with a new background behind the previous native frame.
+type Appearance struct {
+ Pref preferences.Theme
+ Dark bool
+}
+
+// storedAppearance is the snapshot maintained by services.Theme, published as
+// one value so the pair can never tear. It is the fallback for window creation
+// until resolveAppearance is installed.
+var storedAppearance atomic.Value // Appearance
+
+// resolveAppearance re-resolves against the live OS state. Theme installs it so
+// window creation never reads a stale seed: app.Env.IsDarkMode reports light
+// until Run installs the platform layer, and Wails runs every
+// ApplicationStarted listener in its own goroutine, so a startup window can be
+// created before Theme's listener has corrected the seed.
+var resolveAppearance atomic.Value // func() Appearance
+
+func init() {
+ storedAppearance.Store(Appearance{Pref: preferences.DefaultTheme, Dark: true})
+}
+
+func setAppearance(pref preferences.Theme, dark bool) {
+ storedAppearance.Store(Appearance{Pref: pref, Dark: dark})
+}
+
+func setAppearanceResolver(f func() Appearance) { resolveAppearance.Store(f) }
+
+// CurrentAppearance returns the snapshot every window creation must build from.
+func CurrentAppearance() Appearance {
+ if f, _ := resolveAppearance.Load().(func() Appearance); f != nil {
+ return f()
+ }
+ a, _ := storedAppearance.Load().(Appearance)
+ return a
+}
+
+// WindowBackgroundColour returns the background for a snapshot; use it for
+// every WebviewWindowOptions.BackgroundColour.
+func WindowBackgroundColour(a Appearance) application.RGBA {
+ return windowBackgroundColour(a.Dark)
+}
+
+// windowBackgroundColour maps a resolved appearance to its window background.
+func windowBackgroundColour(dark bool) application.RGBA {
+ if dark {
+ return windowBackgroundDark
+ }
+ return windowBackgroundLight
+}
// WindowHeight is shared by the main and Settings windows.
const WindowHeight = 660
// Wails reads CustomTheme colours as 0x00BBGGRR (RGB byte order reversed).
-var microsoftWindowsTheme = &application.WindowTheme{
- BorderColour: u32ptr(0x00211E1C),
+var microsoftWindowsDarkTheme = &application.WindowTheme{
+ BorderColour: u32ptr(0x00211E1C), // #1C1E21 nb-gray-940
TitleBarColour: u32ptr(0x00211E1C),
- TitleTextColour: u32ptr(0x00E9E7E4),
+ TitleTextColour: u32ptr(0x00E9E7E4), // #E4E7E9 nb-gray-100
}
-// MicrosoftWindowsAppearanceOptions is the shared Windows chrome (Mica + dark + custom title bar).
-func MicrosoftWindowsAppearanceOptions() application.WindowsWindow {
+var microsoftWindowsLightTheme = &application.WindowTheme{
+ BorderColour: u32ptr(0x00F3F3F3), // #F3F3F3 light nb-gray DEFAULT
+ TitleBarColour: u32ptr(0x00F3F3F3),
+ TitleTextColour: u32ptr(0x00212121), // #212121 light nb-gray-100
+}
+
+// MicrosoftWindowsAppearanceOptions is the shared Windows chrome (Mica +
+// custom title bar), resolved at creation; setWindowAppearance re-themes live
+// windows on later changes. Never SystemDefault: Wails gives those windows a
+// SystemThemeChanged handler that re-themes chrome from the OS appearance,
+// which outlives a switch to a forced theme and fights it on the next OS flip.
+// Both CustomTheme slots hold one colour set for the same reason.
+func MicrosoftWindowsAppearanceOptions(a Appearance) application.WindowsWindow {
+ theme, chrome := application.Light, microsoftWindowsLightTheme
+ if a.Dark {
+ theme, chrome = application.Dark, microsoftWindowsDarkTheme
+ }
return application.WindowsWindow{
BackdropType: application.Mica,
- Theme: application.Dark,
+ Theme: theme,
CustomTheme: application.ThemeSettings{
- DarkModeActive: microsoftWindowsTheme,
- DarkModeInactive: microsoftWindowsTheme,
- LightModeActive: microsoftWindowsTheme,
- LightModeInactive: microsoftWindowsTheme,
+ DarkModeActive: chrome,
+ DarkModeInactive: chrome,
+ LightModeActive: chrome,
+ LightModeInactive: chrome,
},
}
}
// AppleMacOSAppearanceOptions is the shared macOS chrome; FullScreenNone keeps the fixed-size layout.
-func AppleMacOSAppearanceOptions() application.MacWindow {
+func AppleMacOSAppearanceOptions(a Appearance) application.MacWindow {
+ appearance := application.DefaultAppearance
+ switch a.Pref {
+ case preferences.ThemeLight:
+ appearance = application.NSAppearanceNameAqua
+ case preferences.ThemeDark:
+ appearance = application.NSAppearanceNameDarkAqua
+ }
return application.MacWindow{
InvisibleTitleBarHeight: 38,
Backdrop: application.MacBackdropNormal,
TitleBar: application.MacTitleBarHiddenInset,
CollectionBehavior: application.MacWindowCollectionBehaviorFullScreenNone,
+ Appearance: appearance,
}
}
@@ -82,6 +181,7 @@ func LinuxAppearanceOptions(icon []byte) application.LinuxWindow {
// DialogWindowOptions is the baseline for every auxiliary dialog window; callers override per-dialog.
func DialogWindowOptions(name, title, url string, linuxIcon []byte) application.WebviewWindowOptions {
+ a := CurrentAppearance()
return application.WebviewWindowOptions{
Name: name,
Title: title,
@@ -93,10 +193,10 @@ func DialogWindowOptions(name, title, url string, linuxIcon []byte) application.
MinimiseButtonState: application.ButtonHidden,
MaximiseButtonState: application.ButtonHidden,
CloseButtonState: application.ButtonEnabled,
- BackgroundColour: WindowBackgroundColour,
+ BackgroundColour: WindowBackgroundColour(a),
URL: url,
- Mac: AppleMacOSAppearanceOptions(),
- Windows: MicrosoftWindowsAppearanceOptions(),
+ Mac: AppleMacOSAppearanceOptions(a),
+ Windows: MicrosoftWindowsAppearanceOptions(a),
Linux: LinuxAppearanceOptions(linuxIcon),
}
}
@@ -116,8 +216,11 @@ type WindowManager struct {
// hiddenForLogin holds windows hidden while the BrowserLogin popup is open, restored on close.
hiddenForLogin []application.Window
mu sync.Mutex
- createMu sync.Mutex
newMain func(startURL string) *application.WebviewWindow
+ creating map[string]bool
+ pendingOps map[string][]windowOp
+ pendingClose map[string]windowCloser
+ restoreGen uint64
ready map[uint]bool
showPending map[uint]bool
pendingTab map[uint]string
@@ -137,6 +240,9 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo
translator: translator,
prefs: prefs,
linuxIcon: linuxIcon,
+ creating: map[string]bool{},
+ pendingOps: map[string][]windowOp{},
+ pendingClose: map[string]windowCloser{},
ready: map[uint]bool{},
showPending: map[uint]bool{},
pendingTab: map[uint]string{},
@@ -164,8 +270,9 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo
}
func (s *WindowManager) newSettingsWindow() *application.WebviewWindow {
+ a := CurrentAppearance()
w := s.app.Window.NewWithOptions(application.WebviewWindowOptions{
- Name: "settings",
+ Name: windowSettings,
Title: s.title("window.title.settings"),
Width: 900,
Height: WindowHeight,
@@ -174,10 +281,10 @@ func (s *WindowManager) newSettingsWindow() *application.WebviewWindow {
MinimiseButtonState: application.ButtonHidden,
MaximiseButtonState: application.ButtonHidden,
CloseButtonState: application.ButtonEnabled,
- BackgroundColour: WindowBackgroundColour,
+ BackgroundColour: WindowBackgroundColour(a),
URL: "/#/settings",
- Mac: AppleMacOSAppearanceOptions(),
- Windows: MicrosoftWindowsAppearanceOptions(),
+ Mac: AppleMacOSAppearanceOptions(a),
+ Windows: MicrosoftWindowsAppearanceOptions(a),
Linux: LinuxAppearanceOptions(s.linuxIcon),
})
w.RegisterHook(events.Common.WindowClosing, func(_ *application.WindowEvent) {
@@ -186,6 +293,7 @@ func (s *WindowManager) newSettingsWindow() *application.WebviewWindow {
s.forgetWindowLocked(w)
s.mu.Unlock()
})
+ s.armReady(w)
return w
}
@@ -197,63 +305,68 @@ func (s *WindowManager) OpenSettings(tab string) {
target = "general"
}
- w, _ := s.ensureWindow(&s.settings, s.newSettingsWindow)
+ s.withWindow(windowSettings, &s.settings, s.newSettingsWindow, func(w *application.WebviewWindow, _ bool) {
+ s.mu.Lock()
+ ready := s.ready[w.ID()]
+ if !ready {
+ s.pendingTab[w.ID()] = target
+ }
+ s.mu.Unlock()
- s.mu.Lock()
- ready := s.ready[w.ID()]
- if !ready {
- s.pendingTab[w.ID()] = target
- }
- s.mu.Unlock()
-
- if ready {
- s.app.Event.Emit(EventSettingsOpen, target)
- }
- s.showWhenReady(w)
+ if ready {
+ s.app.Event.Emit(EventSettingsOpen, target)
+ }
+ s.showWhenReady(w)
+ })
}
// OpenBrowserLogin shows the SSO popup, creating it on first use.
func (s *WindowManager) OpenBrowserLogin(uri string) {
- s.mu.Lock()
- defer s.mu.Unlock()
- if s.browserLogin == nil {
- startURL := "/#/dialog/browser-login"
- if uri != "" {
- startURL = "/#/dialog/browser-login?uri=" + url.QueryEscape(uri)
- }
- s.hideOtherWindowsLocked("browser-login")
- opts := DialogWindowOptions("browser-login", s.title("window.title.signIn"), startURL, s.linuxIcon)
- // Not always-on-top: it would obscure the browser tab the user logs in through.
- opts.AlwaysOnTop = false
- opts.InitialPosition = application.WindowCentered
- // Open on the active (where users cursor is) display, like the session-expiration dialog.
- opts.Screen = s.getScreenBasedOnCursorPosition()
- s.browserLogin = s.app.Window.NewWithOptions(opts)
- bl := s.browserLogin
- bl.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
- s.mu.Lock()
- // Only a live user red-X still has this registered; programmatic closers
- // nil s.browserLogin first and clean up themselves. Guarding here stops a
- // stale close event from wiping a replacement popup's state.
- userClosed := s.browserLogin == bl
- if userClosed {
- s.browserLogin = nil
- s.restoreHiddenWindowsLocked()
- }
- s.mu.Unlock()
- if userClosed {
- s.app.Event.Emit(EventBrowserLoginCancel)
- }
- })
- s.centerOnCursorScreen(s.browserLogin)
- return
- }
+ startURL := "/#/dialog/browser-login"
if uri != "" {
- s.browserLogin.SetURL("/#/dialog/browser-login?uri=" + url.QueryEscape(uri))
+ startURL = "/#/dialog/browser-login?uri=" + url.QueryEscape(uri)
}
- s.centerOnCursorScreen(s.browserLogin)
- s.browserLogin.Show()
- s.browserLogin.Focus()
+ s.withWindow(windowBrowserLogin, &s.browserLogin, func() *application.WebviewWindow {
+ return s.newBrowserLoginWindow(startURL)
+ }, func(w *application.WebviewWindow, created bool) {
+ if created {
+ s.centerOnCursorScreen(w)
+ return
+ }
+ if uri != "" {
+ w.SetURL(startURL)
+ }
+ s.centerOnCursorScreen(w)
+ w.Show()
+ w.Focus()
+ })
+}
+
+func (s *WindowManager) newBrowserLoginWindow(startURL string) *application.WebviewWindow {
+ s.hideOtherWindows(windowBrowserLogin)
+ opts := DialogWindowOptions(windowBrowserLogin, s.title("window.title.signIn"), startURL, s.linuxIcon)
+ // Not always-on-top: it would obscure the browser tab the user logs in through.
+ opts.AlwaysOnTop = false
+ opts.InitialPosition = application.WindowCentered
+ // Open on the active (where users cursor is) display, like the session-expiration dialog.
+ opts.Screen = s.getScreenBasedOnCursorPosition()
+ w := s.app.Window.NewWithOptions(opts)
+ w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
+ s.mu.Lock()
+ // Only a live user red-X still has this registered; programmatic closers
+ // nil s.browserLogin first and clean up themselves. Guarding here stops a
+ // stale close event from wiping a replacement popup's state.
+ userClosed := s.browserLogin == w
+ if userClosed {
+ s.browserLogin = nil
+ }
+ s.mu.Unlock()
+ if userClosed {
+ s.restoreHiddenWindows()
+ s.app.Event.Emit(EventBrowserLoginCancel)
+ }
+ })
+ return w
}
// BrowserLoginWindow returns the live SSO popup, or nil. While non-nil it is the
@@ -273,71 +386,62 @@ func (s *WindowManager) InstallProgressWindow() *application.WebviewWindow {
}
func (s *WindowManager) CloseBrowserLogin() {
- s.mu.Lock()
- w := s.browserLogin
- s.browserLogin = nil
// The WindowClosing hook no-ops on a programmatic close, so restore here —
// but only if a popup was actually open. The frontend calls this even when no
// popup was ever shown (e.g. resetDialog() after an early RequestExtend failure,
// or connection.ts's catch path), and hiddenForLogin is shared with
// OpenInstallProgress, so an unconditional restore could re-show windows a
// still-running install-progress is hiding.
- if w != nil {
- s.restoreHiddenWindowsLocked()
- }
- s.mu.Unlock()
- if w != nil {
- w.Close()
- }
+ s.closeWindow(windowBrowserLogin, &s.browserLogin, s.restoreAndClose)
}
// OpenSessionExpiration shows the countdown warning on the cursor's display; seconds seeds
// the countdown and deadlineUnixMilli (0 when unknown) is the absolute deadline the dialog
// compares renewal snapshots against. Singleton, destroyed on close.
func (s *WindowManager) OpenSessionExpiration(seconds int, deadlineUnixMilli int64) {
- s.mu.Lock()
- defer s.mu.Unlock()
startURL := "/#/dialog/session-expiration?seconds=" + strconv.Itoa(seconds)
if deadlineUnixMilli > 0 {
startURL += "&deadline=" + strconv.FormatInt(deadlineUnixMilli, 10)
}
- if s.sessionExpiration == nil {
- opts := DialogWindowOptions("session-expiration", s.title("window.title.sessionExpiration"), startURL, s.linuxIcon)
- opts.Screen = s.getScreenBasedOnCursorPosition()
- opts.InitialPosition = application.WindowCentered
- s.sessionExpiration = s.app.Window.NewWithOptions(opts)
- s.sessionExpiration.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
- s.mu.Lock()
+ s.withWindow(windowSessionExpiration, &s.sessionExpiration, func() *application.WebviewWindow {
+ return s.newSessionExpirationWindow(startURL)
+ }, func(w *application.WebviewWindow, created bool) {
+ if created {
+ s.centerOnCursorScreen(w)
+ return
+ }
+ w.SetURL(startURL)
+ s.centerOnCursorScreen(w)
+ w.Show()
+ w.Focus()
+ })
+}
+
+func (s *WindowManager) newSessionExpirationWindow(startURL string) *application.WebviewWindow {
+ opts := DialogWindowOptions(windowSessionExpiration, s.title("window.title.sessionExpiration"), startURL, s.linuxIcon)
+ opts.Screen = s.getScreenBasedOnCursorPosition()
+ opts.InitialPosition = application.WindowCentered
+ w := s.app.Window.NewWithOptions(opts)
+ w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
+ s.mu.Lock()
+ if s.sessionExpiration == w {
s.sessionExpiration = nil
- s.mu.Unlock()
- })
- s.centerOnCursorScreen(s.sessionExpiration)
- return
- }
- s.sessionExpiration.SetURL(startURL)
- s.centerOnCursorScreen(s.sessionExpiration)
- s.sessionExpiration.Show()
- s.sessionExpiration.Focus()
+ }
+ s.mu.Unlock()
+ })
+ return w
}
func (s *WindowManager) CloseSessionExpiration() {
- s.mu.Lock()
- w := s.sessionExpiration
- s.sessionExpiration = nil
- s.mu.Unlock()
- if w != nil {
- w.Close()
- }
+ s.closeWindow(windowSessionExpiration, &s.sessionExpiration, closeOnly)
}
// CloseRenewFlow tears down the SSO session-renewal UI in a single call: it
// closes the browser-login popup and the session-expiration window together.
func (s *WindowManager) CloseRenewFlow() {
s.mu.Lock()
- bl := s.browserLogin
- se := s.sessionExpiration
- s.browserLogin = nil
- s.sessionExpiration = nil
+ bl := s.takeWindowLocked(windowBrowserLogin, &s.browserLogin, s.restoreAndClose)
+ se := s.takeWindowLocked(windowSessionExpiration, &s.sessionExpiration, closeOnly)
if se != nil {
kept := s.hiddenForLogin[:0]
for _, w := range s.hiddenForLogin {
@@ -347,9 +451,9 @@ func (s *WindowManager) CloseRenewFlow() {
}
s.hiddenForLogin = kept
}
- s.restoreHiddenWindowsLocked()
s.mu.Unlock()
+ s.restoreHiddenWindows()
// Close after unlock so the re-entrant handlers can take s.mu.
if bl != nil {
bl.Close()
@@ -362,73 +466,70 @@ func (s *WindowManager) CloseRenewFlow() {
// OpenInstallProgress shows the install-progress window and hides the rest for the duration
// (restored on close). It owns its own result polling since the daemon restarts mid-install.
func (s *WindowManager) OpenInstallProgress(version string) {
- s.mu.Lock()
- defer s.mu.Unlock()
startURL := "/#/dialog/install-progress"
if version != "" {
startURL = "/#/dialog/install-progress?version=" + url.QueryEscape(version)
}
- if s.installProgress == nil {
- s.hideOtherWindowsLocked("install-progress")
- s.installProgress = s.app.Window.NewWithOptions(
- DialogWindowOptions("install-progress", s.title("window.title.updating"), startURL, s.linuxIcon),
- )
- s.installProgress.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
- s.mu.Lock()
+ s.withWindow(windowInstallProgress, &s.installProgress, func() *application.WebviewWindow {
+ return s.newInstallProgressWindow(startURL)
+ }, func(w *application.WebviewWindow, created bool) {
+ if !created {
+ w.SetURL(startURL)
+ w.Show()
+ w.Focus()
+ }
+ s.centerWhenReady(w)
+ })
+}
+
+func (s *WindowManager) newInstallProgressWindow(startURL string) *application.WebviewWindow {
+ s.hideOtherWindows(windowInstallProgress)
+ w := s.app.Window.NewWithOptions(
+ DialogWindowOptions(windowInstallProgress, s.title("window.title.updating"), startURL, s.linuxIcon),
+ )
+ w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
+ s.mu.Lock()
+ if s.installProgress == w {
s.installProgress = nil
- s.restoreHiddenWindowsLocked()
- s.mu.Unlock()
- })
- s.centerWhenReady(s.installProgress)
- return
- }
- s.installProgress.SetURL(startURL)
- s.installProgress.Show()
- s.installProgress.Focus()
- s.centerWhenReady(s.installProgress)
+ }
+ s.mu.Unlock()
+ s.restoreHiddenWindows()
+ })
+ return w
}
func (s *WindowManager) CloseInstallProgress() {
- s.mu.Lock()
- w := s.installProgress
- s.installProgress = nil
- s.mu.Unlock()
- if w != nil {
- w.Close()
- }
+ s.closeWindow(windowInstallProgress, &s.installProgress, closeOnly)
}
// OpenWelcome shows the first-launch onboarding window. Singleton, destroyed on close.
func (s *WindowManager) OpenWelcome() {
- s.mu.Lock()
- defer s.mu.Unlock()
- if s.welcome == nil {
- opts := DialogWindowOptions("welcome", s.title("window.title.welcome"), "/#/dialog/welcome", s.linuxIcon)
- opts.Width = 420
- opts.InitialPosition = application.WindowCentered
- s.welcome = s.app.Window.NewWithOptions(opts)
- w := s.welcome
- w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
- s.mu.Lock()
+ s.withWindow(windowWelcome, &s.welcome, s.newWelcomeWindow, func(w *application.WebviewWindow, created bool) {
+ if !created {
+ w.Show()
+ w.Focus()
+ }
+ s.centerWhenReady(w)
+ })
+}
+
+func (s *WindowManager) newWelcomeWindow() *application.WebviewWindow {
+ opts := DialogWindowOptions(windowWelcome, s.title("window.title.welcome"), "/#/dialog/welcome", s.linuxIcon)
+ opts.Width = 420
+ opts.InitialPosition = application.WindowCentered
+ w := s.app.Window.NewWithOptions(opts)
+ w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
+ s.mu.Lock()
+ if s.welcome == w {
s.welcome = nil
- s.mu.Unlock()
- })
- s.centerWhenReady(s.welcome)
- return
- }
- s.welcome.Show()
- s.welcome.Focus()
- s.centerWhenReady(s.welcome)
+ }
+ s.mu.Unlock()
+ })
+ return w
}
func (s *WindowManager) CloseWelcome() {
- s.mu.Lock()
- w := s.welcome
- s.welcome = nil
- s.mu.Unlock()
- if w != nil {
- w.Close()
- }
+ s.closeWindow(windowWelcome, &s.welcome, closeOnly)
}
// OpenError shows the custom error dialog; title/message/command are pre-localised
@@ -439,35 +540,35 @@ func (s *WindowManager) OpenError(title, message, command string) {
if ShuttingDown() {
return
}
- s.mu.Lock()
- defer s.mu.Unlock()
startURL := errorDialogURL(title, message, command)
- if s.errorDialog == nil {
- s.errorDialog = s.app.Window.NewWithOptions(
- DialogWindowOptions("error", s.title("window.title.error"), startURL, s.linuxIcon),
- )
- s.errorDialog.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
- s.mu.Lock()
+ s.withWindow(windowError, &s.errorDialog, func() *application.WebviewWindow {
+ return s.newErrorWindow(startURL)
+ }, func(w *application.WebviewWindow, created bool) {
+ if !created {
+ w.SetURL(startURL)
+ w.Show()
+ w.Focus()
+ }
+ s.centerWhenReady(w)
+ })
+}
+
+func (s *WindowManager) newErrorWindow(startURL string) *application.WebviewWindow {
+ w := s.app.Window.NewWithOptions(
+ DialogWindowOptions(windowError, s.title("window.title.error"), startURL, s.linuxIcon),
+ )
+ w.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) {
+ s.mu.Lock()
+ if s.errorDialog == w {
s.errorDialog = nil
- s.mu.Unlock()
- })
- s.centerWhenReady(s.errorDialog)
- return
- }
- s.errorDialog.SetURL(startURL)
- s.errorDialog.Show()
- s.errorDialog.Focus()
- s.centerWhenReady(s.errorDialog)
+ }
+ s.mu.Unlock()
+ })
+ return w
}
func (s *WindowManager) CloseError() {
- s.mu.Lock()
- w := s.errorDialog
- s.errorDialog = nil
- s.mu.Unlock()
- if w != nil {
- w.Close()
- }
+ s.closeWindow(windowError, &s.errorDialog, closeOnly)
}
// OpenMain brings the main window forward; the welcome handoff uses it instead of the tray.
@@ -478,65 +579,171 @@ func (s *WindowManager) OpenMain() {
// ShowMain brings the main window forward (re-centering on minimal WMs). The single entry
// point every surface (tray, SIGUSR1, welcome) should use so centering applies uniformly.
func (s *WindowManager) ShowMain() {
- s.showWhenReady(s.MainWindow())
+ s.ensureMain("/", func(w *application.WebviewWindow, _ bool) {
+ s.showWhenReady(w)
+ })
}
// ShowMainAndEmit brings the main window forward and emits event once its frontend is ready.
func (s *WindowManager) ShowMainAndEmit(event string) {
- w := s.MainWindow()
- if w == nil {
- return
- }
+ s.ensureMain("/", func(w *application.WebviewWindow, _ bool) {
+ id := w.ID()
+ s.mu.Lock()
+ ready := s.ready[id]
+ if !ready {
+ s.pendingEmits[id] = append(s.pendingEmits[id], event)
+ }
+ s.mu.Unlock()
- id := w.ID()
- s.mu.Lock()
- ready := s.ready[id]
- if !ready {
- s.pendingEmits[id] = append(s.pendingEmits[id], event)
- }
- s.mu.Unlock()
-
- s.showWhenReady(w)
- if ready {
- s.app.Event.Emit(event)
- }
+ s.showWhenReady(w)
+ if ready {
+ s.app.Event.Emit(event)
+ }
+ })
}
func (s *WindowManager) MainWindow() *application.WebviewWindow {
- w, _ := s.ensureMain("/")
- return w
+ s.mu.Lock()
+ defer s.mu.Unlock()
+ return s.mainWindow
}
-func (s *WindowManager) ensureMain(startURL string) (*application.WebviewWindow, bool) {
+func (s *WindowManager) ensureMain(startURL string, op windowOp) {
s.mu.Lock()
factory := s.newMain
s.mu.Unlock()
if factory == nil {
- return s.ensureWindow(&s.mainWindow, nil)
+ s.withWindow(windowMain, &s.mainWindow, nil, op)
+ return
}
- return s.ensureWindow(&s.mainWindow, func() *application.WebviewWindow {
- return factory(startURL)
- })
+ s.withWindow(windowMain, &s.mainWindow, func() *application.WebviewWindow {
+ w := factory(startURL)
+ s.armReady(w)
+ return w
+ }, op)
}
-func (s *WindowManager) ensureWindow(slot **application.WebviewWindow, factory func() *application.WebviewWindow) (*application.WebviewWindow, bool) {
- s.createMu.Lock()
- defer s.createMu.Unlock()
-
+func (s *WindowManager) withWindow(name string, slot **application.WebviewWindow, factory func() *application.WebviewWindow, op windowOp) {
s.mu.Lock()
- w := *slot
- s.mu.Unlock()
- if w != nil || factory == nil {
- return w, false
+ if s.creating[name] {
+ s.pendingOps[name] = append(s.pendingOps[name], op)
+ s.mu.Unlock()
+ return
}
+ if w := *slot; w != nil {
+ s.mu.Unlock()
+ op(w, false)
+ return
+ }
+ if factory == nil {
+ s.mu.Unlock()
+ return
+ }
+ s.creating[name] = true
+ s.mu.Unlock()
- w = factory()
- s.armReady(w)
+ w := s.createWindow(name, slot, factory)
+ if w == nil {
+ return
+ }
+ s.finishCreation(name, slot, w, op)
+}
+func (s *WindowManager) createWindow(name string, slot **application.WebviewWindow, factory func() *application.WebviewWindow) *application.WebviewWindow {
+ created := false
+ defer func() {
+ if created {
+ return
+ }
+ s.mu.Lock()
+ s.releaseCreationLocked(name)
+ s.mu.Unlock()
+ }()
+
+ w := factory()
+ if w == nil {
+ return nil
+ }
s.mu.Lock()
*slot = w
s.mu.Unlock()
- return w, true
+ created = true
+ return w
+}
+
+func (s *WindowManager) finishCreation(name string, slot **application.WebviewWindow, w *application.WebviewWindow, op windowOp) {
+ finished := false
+ defer func() {
+ if finished {
+ return
+ }
+ s.mu.Lock()
+ s.releaseCreationLocked(name)
+ s.mu.Unlock()
+ }()
+
+ created := true
+ for {
+ s.mu.Lock()
+ if closer := s.pendingClose[name]; closer != nil {
+ if *slot == w {
+ *slot = nil
+ }
+ s.releaseCreationLocked(name)
+ finished = true
+ s.mu.Unlock()
+ closer(w)
+ return
+ }
+ var next windowOp
+ switch {
+ case created:
+ next = op
+ case len(s.pendingOps[name]) > 0:
+ next = s.pendingOps[name][0]
+ s.pendingOps[name] = s.pendingOps[name][1:]
+ default:
+ s.releaseCreationLocked(name)
+ finished = true
+ s.mu.Unlock()
+ return
+ }
+ s.mu.Unlock()
+ next(w, created)
+ created = false
+ }
+}
+
+func (s *WindowManager) closeWindow(name string, slot **application.WebviewWindow, closer windowCloser) {
+ s.mu.Lock()
+ w := s.takeWindowLocked(name, slot, closer)
+ s.mu.Unlock()
+ if w != nil {
+ closer(w)
+ }
+}
+
+func (s *WindowManager) takeWindowLocked(name string, slot **application.WebviewWindow, closer windowCloser) *application.WebviewWindow {
+ if s.creating[name] {
+ if s.pendingClose[name] == nil {
+ s.pendingClose[name] = closer
+ }
+ return nil
+ }
+ w := *slot
+ *slot = nil
+ return w
+}
+
+func (s *WindowManager) releaseCreationLocked(name string) {
+ delete(s.creating, name)
+ delete(s.pendingOps, name)
+ delete(s.pendingClose, name)
+}
+
+func (s *WindowManager) restoreAndClose(w *application.WebviewWindow) {
+ s.restoreHiddenWindows()
+ w.Close()
}
func (s *WindowManager) armReady(w *application.WebviewWindow) {
@@ -576,24 +783,21 @@ func (s *WindowManager) watchTriggerLogin() {
return
}
- w, created := s.ensureMain("/")
- if w == nil {
- return
- }
+ s.ensureMain("/", func(w *application.WebviewWindow, created bool) {
+ s.mu.Lock()
+ if created {
+ s.headlessMain = true
+ }
+ pending := !s.ready[w.ID()]
+ if pending {
+ s.pendingEmits[w.ID()] = append(s.pendingEmits[w.ID()], EventTriggerLogin)
+ }
+ s.mu.Unlock()
- s.mu.Lock()
- if created {
- s.headlessMain = true
- }
- pending := !s.ready[w.ID()]
- if pending {
- s.pendingEmits[w.ID()] = append(s.pendingEmits[w.ID()], EventTriggerLogin)
- }
- s.mu.Unlock()
-
- if !pending {
- s.app.Event.Emit(EventTriggerLogin)
- }
+ if !pending {
+ s.app.Event.Emit(EventTriggerLogin)
+ }
+ })
})
s.app.Event.On(EventBrowserLoginCancel, func(_ *application.CustomEvent) {
@@ -664,9 +868,9 @@ func (s *WindowManager) windowByName(name string) *application.WebviewWindow {
s.mu.Lock()
defer s.mu.Unlock()
switch name {
- case "main":
+ case windowMain:
return s.mainWindow
- case "settings":
+ case windowSettings:
return s.settings
default:
return nil
@@ -741,14 +945,12 @@ func (s *WindowManager) showNow(w *application.WebviewWindow) {
}
func (s *WindowManager) ShowMainAt(url string) {
- w, created := s.ensureMain(url)
- if w == nil {
- return
- }
- if !created {
- w.SetURL(url)
- }
- s.showWhenReady(w)
+ s.ensureMain(url, func(w *application.WebviewWindow, created bool) {
+ if !created {
+ w.SetURL(url)
+ }
+ s.showWhenReady(w)
+ })
}
func (s *WindowManager) SetMainFactory(f func(startURL string) *application.WebviewWindow) {
@@ -868,39 +1070,61 @@ func (s *WindowManager) retitleAll() {
}
}
-// hideOtherWindowsLocked hides every visible window except keepName, recording
-// them in hiddenForLogin for restoreHiddenWindowsLocked. Caller must hold s.mu.
-func (s *WindowManager) hideOtherWindowsLocked(keepName string) {
+func (s *WindowManager) hideOtherWindows(keepName string) {
+ s.mu.Lock()
+ gen := s.restoreGen
+ s.mu.Unlock()
+
+ var hidden []application.Window
for _, w := range s.app.Window.GetAll() {
- if w == nil || w.Name() == keepName {
- continue
- }
- if !w.IsVisible() {
+ if w == nil || w.Name() == keepName || !w.IsVisible() {
continue
}
w.Hide()
- s.hiddenForLogin = append(s.hiddenForLogin, w)
+ hidden = append(hidden, w)
+ }
+ if len(hidden) == 0 {
+ return
+ }
+
+ s.mu.Lock()
+ restored := s.restoreGen != gen
+ if !restored {
+ s.hiddenForLogin = append(s.hiddenForLogin, hidden...)
+ }
+ s.mu.Unlock()
+ if !restored {
+ return
+ }
+ for _, w := range hidden {
+ w.Show()
}
}
-// restoreHiddenWindowsLocked re-shows windows hidden by hideOtherWindowsLocked
-// (caller holds s.mu). If the main window was among them, raiseToForeground
-// lifts it above the SSO browser, which still owns the foreground — a plain
-// Show/Focus would be demoted to a taskbar flash and leave it stranded behind.
-func (s *WindowManager) restoreHiddenWindowsLocked() {
+// restoreHiddenWindows re-shows windows hidden by hideOtherWindows. If the main
+// window was among them, raiseToForeground lifts it above the SSO browser, which
+// still owns the foreground — a plain Show/Focus would be demoted to a taskbar
+// flash and leave it stranded behind.
+func (s *WindowManager) restoreHiddenWindows() {
+ s.mu.Lock()
+ hidden := s.hiddenForLogin
+ s.hiddenForLogin = nil
+ s.restoreGen++
+ mainWindow := s.mainWindow
+ s.mu.Unlock()
+
mainRestored := false
- for _, w := range s.hiddenForLogin {
+ for _, w := range hidden {
if w == nil {
continue
}
w.Show()
- if w == s.mainWindow {
+ if w == mainWindow {
mainRestored = true
}
}
- s.hiddenForLogin = nil
- if mainRestored && s.mainWindow != nil {
- raiseToForeground(s.mainWindow)
+ if mainRestored && mainWindow != nil {
+ raiseToForeground(mainWindow)
}
}
@@ -915,8 +1139,11 @@ func (s *WindowManager) getScreenBasedOnCursorPosition() *application.Screen {
return sc
}
}
- if s.mainWindow != nil {
- if sc, err := s.mainWindow.GetScreen(); err == nil {
+ s.mu.Lock()
+ mainWindow := s.mainWindow
+ s.mu.Unlock()
+ if mainWindow != nil {
+ if sc, err := mainWindow.GetScreen(); err == nil {
return sc
}
}
@@ -944,3 +1171,5 @@ func errorDialogURL(title, message, command string) string {
// u32ptr returns a pointer to v, for the optional *uint32 Wails theme fields.
func u32ptr(v uint32) *uint32 { return &v }
+
+func closeOnly(w *application.WebviewWindow) { w.Close() }
diff --git a/client/ui/services/windowmanager_test.go b/client/ui/services/windowmanager_test.go
new file mode 100644
index 000000000..13c8548ab
--- /dev/null
+++ b/client/ui/services/windowmanager_test.go
@@ -0,0 +1,350 @@
+//go:build !android && !ios && !freebsd && !js
+
+package services
+
+import (
+ "fmt"
+ "sync/atomic"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/require"
+ "github.com/wailsapp/wails/v3/pkg/application"
+)
+
+func newTestWindowManager() *WindowManager {
+ return &WindowManager{
+ creating: map[string]bool{},
+ pendingOps: map[string][]windowOp{},
+ pendingClose: map[string]windowCloser{},
+ }
+}
+
+func waitDone(t *testing.T, done <-chan struct{}, msg string) {
+ t.Helper()
+ select {
+ case <-done:
+ case <-time.After(5 * time.Second):
+ t.Fatal(msg)
+ }
+}
+
+func TestWithWindowReusesExistingWindow(t *testing.T) {
+ s := newTestWindowManager()
+ existing := &application.WebviewWindow{}
+ slot := existing
+ factoryCalls := 0
+ var got *application.WebviewWindow
+ created := true
+ s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
+ factoryCalls++
+ return &application.WebviewWindow{}
+ }, func(w *application.WebviewWindow, c bool) {
+ got, created = w, c
+ })
+ require.Equal(t, 0, factoryCalls)
+ require.Same(t, existing, got)
+ require.False(t, created)
+}
+
+func TestWithWindowNilFactoryWithoutWindowSkipsOp(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ opCalls := 0
+ s.withWindow(windowMain, &slot, nil, func(*application.WebviewWindow, bool) {
+ opCalls++
+ })
+ require.Equal(t, 0, opCalls)
+ require.Nil(t, slot)
+}
+
+func TestWithWindowReentrantCallDuringCreationIsQueued(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ factoryCalls := 0
+ var order []string
+ var factory func() *application.WebviewWindow
+ factory = func() *application.WebviewWindow {
+ factoryCalls++
+ // Simulates the Windows message pump re-entering the tray click handler
+ // while WebView2 is still initialising the window being created.
+ s.withWindow(windowMain, &slot, factory, func(_ *application.WebviewWindow, created bool) {
+ order = append(order, fmt.Sprintf("reentrant:%v", created))
+ })
+ return &application.WebviewWindow{}
+ }
+
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ s.withWindow(windowMain, &slot, factory, func(_ *application.WebviewWindow, created bool) {
+ order = append(order, fmt.Sprintf("outer:%v", created))
+ })
+ }()
+ waitDone(t, done, "withWindow deadlocked on a re-entrant call during creation")
+
+ require.Equal(t, 1, factoryCalls)
+ require.Equal(t, []string{"outer:true", "reentrant:false"}, order)
+ require.NotNil(t, slot)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingOps)
+}
+
+func TestWithWindowConcurrentCallersShareOneCreation(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ factoryEntered := make(chan struct{})
+ release := make(chan struct{})
+ var factoryCalls, opCalls atomic.Int32
+ factory := func() *application.WebviewWindow {
+ factoryCalls.Add(1)
+ close(factoryEntered)
+ <-release
+ return &application.WebviewWindow{}
+ }
+ op := func(*application.WebviewWindow, bool) { opCalls.Add(1) }
+
+ first := make(chan struct{})
+ go func() {
+ defer close(first)
+ s.withWindow(windowSettings, &slot, factory, op)
+ }()
+ <-factoryEntered
+
+ second := make(chan struct{})
+ go func() {
+ defer close(second)
+ s.withWindow(windowSettings, &slot, factory, op)
+ }()
+ waitDone(t, second, "second caller blocked while the window was being created")
+ require.Equal(t, int32(0), opCalls.Load())
+
+ close(release)
+ waitDone(t, first, "creator did not finish")
+
+ require.Equal(t, int32(1), factoryCalls.Load())
+ require.Equal(t, int32(2), opCalls.Load())
+ require.NotNil(t, slot)
+}
+
+func TestWithWindowOpsQueuedDuringCreationRunInArrivalOrder(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ var order []string
+ record := func(label string) windowOp {
+ return func(_ *application.WebviewWindow, created bool) {
+ order = append(order, fmt.Sprintf("%s:%v", label, created))
+ }
+ }
+ var factory func() *application.WebviewWindow
+ factory = func() *application.WebviewWindow {
+ s.withWindow(windowMain, &slot, factory, func(w *application.WebviewWindow, created bool) {
+ record("a")(w, created)
+ // Arrives while the creator is still draining the queue: it must not
+ // jump ahead of "b" through the existing-window fast path.
+ s.withWindow(windowMain, &slot, factory, record("c"))
+ })
+ s.withWindow(windowMain, &slot, factory, record("b"))
+ return &application.WebviewWindow{}
+ }
+
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ s.withWindow(windowMain, &slot, factory, record("outer"))
+ }()
+ waitDone(t, done, "withWindow deadlocked while draining queued operations")
+
+ require.Equal(t, []string{"outer:true", "a:false", "b:false", "c:false"}, order)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingOps)
+}
+
+func TestWithWindowFactoryPanicReleasesCreation(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ func() {
+ defer func() { require.NotNil(t, recover()) }()
+ s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
+ panic("factory failed")
+ }, func(*application.WebviewWindow, bool) {})
+ }()
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingOps)
+ require.Nil(t, slot)
+
+ created := false
+ s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
+ return &application.WebviewWindow{}
+ }, func(_ *application.WebviewWindow, c bool) {
+ created = c
+ })
+ require.True(t, created)
+ require.NotNil(t, slot)
+}
+
+func TestWithWindowNilFromFactoryReleasesCreation(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ opCalls := 0
+ s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
+ return nil
+ }, func(*application.WebviewWindow, bool) {
+ opCalls++
+ })
+ require.Equal(t, 0, opCalls)
+ require.Empty(t, s.creating)
+ require.Nil(t, slot)
+}
+
+func TestCloseWindowDuringCreationDefersCloseAndSkipsOps(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ created := &application.WebviewWindow{}
+ opCalls, closeCalls := 0, 0
+ var closed *application.WebviewWindow
+ s.withWindow(windowError, &slot, func() *application.WebviewWindow {
+ s.closeWindow(windowError, &slot, func(w *application.WebviewWindow) {
+ closeCalls++
+ closed = w
+ })
+ require.Equal(t, 0, closeCalls)
+ return created
+ }, func(*application.WebviewWindow, bool) {
+ opCalls++
+ })
+ require.Equal(t, 0, opCalls)
+ require.Equal(t, 1, closeCalls)
+ require.Same(t, created, closed)
+ require.Nil(t, slot)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingOps)
+ require.Empty(t, s.pendingClose)
+
+ factoryCalls := 0
+ reopened := false
+ s.withWindow(windowError, &slot, func() *application.WebviewWindow {
+ factoryCalls++
+ return &application.WebviewWindow{}
+ }, func(_ *application.WebviewWindow, c bool) {
+ reopened = c
+ })
+ require.Equal(t, 1, factoryCalls)
+ require.True(t, reopened)
+ require.NotNil(t, slot)
+}
+
+func TestCloseWindowDuringDrainStopsRemainingOps(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ var order []string
+ closeCalls := 0
+ var factory func() *application.WebviewWindow
+ factory = func() *application.WebviewWindow {
+ s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
+ order = append(order, "a")
+ s.closeWindow(windowWelcome, &slot, func(*application.WebviewWindow) { closeCalls++ })
+ s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
+ order = append(order, "c")
+ })
+ })
+ s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
+ order = append(order, "b")
+ })
+ return &application.WebviewWindow{}
+ }
+ s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
+ order = append(order, "outer")
+ })
+
+ // "b" was queued before the close and "c" after it; a close supersedes both
+ // rather than showing a window that is about to be destroyed.
+ require.Equal(t, []string{"outer", "a"}, order)
+ require.Equal(t, 1, closeCalls)
+ require.Nil(t, slot)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingOps)
+ require.Empty(t, s.pendingClose)
+}
+
+func TestCloseWindowWithoutWindowSkipsCloser(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ calls := 0
+ s.closeWindow(windowBrowserLogin, &slot, func(*application.WebviewWindow) { calls++ })
+ require.Equal(t, 0, calls)
+ require.Nil(t, slot)
+ require.Empty(t, s.pendingClose)
+}
+
+func TestCloseWindowWithExistingWindowRunsCloser(t *testing.T) {
+ s := newTestWindowManager()
+ existing := &application.WebviewWindow{}
+ slot := existing
+ var got *application.WebviewWindow
+ s.closeWindow(windowError, &slot, func(w *application.WebviewWindow) { got = w })
+ require.Same(t, existing, got)
+ require.Nil(t, slot)
+ require.Empty(t, s.pendingClose)
+}
+
+func TestWithWindowNilFromFactoryDropsPendingClose(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ closeCalls := 0
+ s.withWindow(windowError, &slot, func() *application.WebviewWindow {
+ s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { closeCalls++ })
+ return nil
+ }, func(*application.WebviewWindow, bool) {})
+ require.Equal(t, 0, closeCalls)
+ require.Nil(t, slot)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingClose)
+}
+
+func TestWithWindowFactoryPanicDropsPendingClose(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ closeCalls := 0
+ func() {
+ defer func() { require.NotNil(t, recover()) }()
+ s.withWindow(windowError, &slot, func() *application.WebviewWindow {
+ s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { closeCalls++ })
+ panic("factory failed")
+ }, func(*application.WebviewWindow, bool) {})
+ }()
+ require.Equal(t, 0, closeCalls)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingClose)
+}
+
+func TestCloseWindowKeepsFirstDeferredCloser(t *testing.T) {
+ s := newTestWindowManager()
+ var slot *application.WebviewWindow
+ var ran []string
+ s.withWindow(windowError, &slot, func() *application.WebviewWindow {
+ s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { ran = append(ran, "first") })
+ s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { ran = append(ran, "second") })
+ return &application.WebviewWindow{}
+ }, func(*application.WebviewWindow, bool) {})
+ require.Equal(t, []string{"first"}, ran)
+ require.Nil(t, slot)
+ require.Empty(t, s.pendingClose)
+}
+
+func TestCloseRenewFlowDuringBrowserLoginCreationRestoresHiddenWindows(t *testing.T) {
+ s := newTestWindowManager()
+ s.withWindow(windowBrowserLogin, &s.browserLogin, func() *application.WebviewWindow {
+ s.CloseRenewFlow()
+ // Seeded after the call so the deferred closer, not CloseRenewFlow's own
+ // immediate restore, is what has to drain it. A nil entry is skipped by
+ // restoreHiddenWindows, so no Wails window is needed.
+ s.hiddenForLogin = []application.Window{nil}
+ return &application.WebviewWindow{}
+ }, func(*application.WebviewWindow, bool) {})
+
+ require.Nil(t, s.browserLogin)
+ require.Empty(t, s.hiddenForLogin)
+ require.Empty(t, s.creating)
+ require.Empty(t, s.pendingClose)
+}
diff --git a/client/ui/tray_click_windows.go b/client/ui/tray_click_windows.go
index 17a6dc5df..04c66dd8a 100644
--- a/client/ui/tray_click_windows.go
+++ b/client/ui/tray_click_windows.go
@@ -4,5 +4,5 @@ package main
// Open application window on left click, right click opens the tray menu
func bindTrayClick(t *Tray) {
- t.tray.OnClick(func() { t.ShowWindow() })
+ t.tray.OnClick(func() { go t.ShowWindow() })
}
diff --git a/client/ui/tray_theme_linux.go b/client/ui/tray_theme_linux.go
index a3249e57a..f9978a7fe 100644
--- a/client/ui/tray_theme_linux.go
+++ b/client/ui/tray_theme_linux.go
@@ -17,6 +17,13 @@ import (
log "github.com/sirupsen/logrus"
)
+// The two KDE files that decide the panel's appearance. Both live in the user
+// config dir, so one directory watch covers them (see watchKdeConfig).
+const (
+ kdeglobalsFile = "kdeglobals"
+ plasmarcFile = "plasmarc"
+)
+
// startTrayTheme seeds t.panelDark and repaints on colour-scheme flips. Must
// run before the first applyIcon so the initial paint uses the right silhouette.
func (t *Tray) startTrayTheme() {
@@ -35,50 +42,142 @@ func isKDE() bool {
return false
}
-// kdeglobalsPath returns the user kdeglobals path. We read only this file, not
-// the full XDG_CONFIG_DIRS cascade: Plasma writes the active scheme here, and a
-// missing Complementary group falls back to the portal.
-func kdeglobalsPath() string {
+// kdeConfigPath locates one of KDE's user config files. We read only the user
+// file, not the full XDG_CONFIG_DIRS cascade: Plasma writes the active scheme
+// and style there, and anything missing falls back to the portal.
+func kdeConfigPath(name string) string {
if dir := os.Getenv("XDG_CONFIG_HOME"); dir != "" {
- return filepath.Join(dir, "kdeglobals")
+ return filepath.Join(dir, name)
}
home, err := os.UserHomeDir()
if err != nil {
return ""
}
- return filepath.Join(home, ".config", "kdeglobals")
+ return filepath.Join(home, ".config", name)
}
-// kdePanelIsDark reports whether the KDE Plasma panel is dark by the luma of
-// its "Complementary" background (the colour Plasma paints the tray with). ok
-// is false when this isn't KDE or the colour can't be read, so the caller falls
-// through to the portal/GTK path.
+func kdeglobalsPath() string { return kdeConfigPath(kdeglobalsFile) }
+func plasmarcPath() string { return kdeConfigPath(plasmarcFile) }
+
+// kdePanelIsDark reports whether the KDE Plasma panel the tray icon sits on is
+// dark. ok is false when this isn't KDE or neither source was conclusive, so the
+// caller falls through to the portal/GTK path.
func kdePanelIsDark() (dark, ok bool) {
if !isKDE() {
return false, false
}
- path := kdeglobalsPath()
- if path == "" {
- return false, false
+ // A pinned Plasma style paints the panel itself, so it outranks the
+ // application colour scheme: a style with dark colours under a Light scheme
+ // is still a dark panel and still needs the white silhouette.
+ if dark, ok := plasmaStyleIsDark(); ok {
+ return dark, true
}
- rgb, ok := readKdeComplementaryBackground(path)
+ // The default style follows the colour scheme, so decide by the luma of the
+ // window background Plasma derives the panel from. Deliberately not
+ // Complementary: that group is dark under Breeze *and* BreezeLight
+ // (42,46,50 measured on Plasma 6.7.4), so reading it kept the white icon on
+ // a light panel, where it is all but invisible.
+ rgb, ok := readKdeColour(kdeglobalsPath(), "[Colors:Window]")
if !ok {
return false, false
}
return isDarkRGB(rgb[0], rgb[1], rgb[2]), true
}
-// readKdeComplementaryBackground parses kdeglobals for
-// [Colors:Complementary] BackgroundNormal and returns its R,G,B (0-255).
-func readKdeComplementaryBackground(path string) (rgb [3]uint8, ok bool) {
+// plasmaStyleIsDark reports the appearance the pinned Plasma style paints the
+// panel with, decided by the style's own colours rather than by its name. A
+// style that ships a colours file overrides the colour scheme for the shell, and
+// nothing requires the name to admit it: breeze-dark happens to, but a style
+// named neutrally can carry dark colours just as well (measured on Plasma 6.7.4:
+// panel luma 38 while kdeglobals and the portal both reported light).
+//
+// ok is false when no style is pinned, when the pinned style ships no colours --
+// the "default" style, which is exactly the case that follows the colour scheme
+// -- and when its colours cannot be read; all three fall through to kdeglobals.
+func plasmaStyleIsDark() (dark, ok bool) {
+ name, found := readIniValue(plasmarcPath(), "[Theme]", "name")
+ if !found || name == "" {
+ return false, false
+ }
+ if !isBareStyleName(name) {
+ log.Debugf("tray theme: ignoring plasma style name %q, not a bare directory name", name)
+ return false, false
+ }
+ for _, dir := range plasmaStyleDirs(name) {
+ rgb, found := readKdeColour(filepath.Join(dir, "colors"), "[Colors:Window]")
+ if !found {
+ continue
+ }
+ return isDarkRGB(rgb[0], rgb[1], rgb[2]), true
+ }
+ return false, false
+}
+
+// isBareStyleName reports whether name is safe to index a directory with. The
+// name comes from a config file and is joined into a path, so it has to be a
+// single ordinary element: "../" in it would point the read anywhere, and ".",
+// ".." and "/" all survive filepath.Base(filepath.Clean(name)) unchanged, so
+// they need rejecting by name -- ".." alone resolves a level above desktoptheme.
+func isBareStyleName(name string) bool {
+ if name == "" || name == "." || name == ".." || filepath.IsAbs(name) {
+ return false
+ }
+ return name == filepath.Base(filepath.Clean(name))
+}
+
+// plasmaStyleDirs lists where a Plasma style of that name may live, in the order
+// Plasma itself resolves them: the user data dir first, so a local style shadows
+// a system one of the same name.
+func plasmaStyleDirs(name string) []string {
+ var dirs []string
+ for _, base := range xdgDataDirs() {
+ dirs = append(dirs, filepath.Join(base, "plasma", "desktoptheme", name))
+ }
+ return dirs
+}
+
+// xdgDataDirs returns XDG_DATA_HOME (or its default) followed by XDG_DATA_DIRS.
+func xdgDataDirs() []string {
+ var dirs []string
+ if home := os.Getenv("XDG_DATA_HOME"); home != "" {
+ dirs = append(dirs, home)
+ } else if h, err := os.UserHomeDir(); err == nil {
+ dirs = append(dirs, filepath.Join(h, ".local", "share"))
+ }
+ system := os.Getenv("XDG_DATA_DIRS")
+ if system == "" {
+ system = "/usr/local/share:/usr/share"
+ }
+ for _, dir := range strings.Split(system, ":") {
+ if dir != "" {
+ dirs = append(dirs, dir)
+ }
+ }
+ return dirs
+}
+
+// readKdeColour reads group's BackgroundNormal as an R,G,B triple.
+func readKdeColour(path, group string) (rgb [3]uint8, ok bool) {
+ val, found := readIniValue(path, group, "BackgroundNormal")
+ if !found {
+ return rgb, false
+ }
+ return parseRGB(val)
+}
+
+// readIniValue returns key's value inside group from a KDE-style INI file.
+// group carries its own brackets, e.g. "[Colors:Window]".
+func readIniValue(path, group, key string) (value string, ok bool) {
+ if path == "" {
+ return "", false
+ }
f, err := os.Open(path)
if err != nil {
- log.Debugf("tray theme: kdeglobals open failed, using portal: %v", err)
- return rgb, false
+ log.Debugf("tray theme: %s open failed, using portal: %v", filepath.Base(path), err)
+ return "", false
}
defer func() { _ = f.Close() }()
- const group = "[Colors:Complementary]"
inGroup := false
scanner := bufio.NewScanner(f)
for scanner.Scan() {
@@ -90,13 +189,13 @@ func readKdeComplementaryBackground(path string) (rgb [3]uint8, ok bool) {
if !inGroup {
continue
}
- key, val, found := strings.Cut(line, "=")
- if !found || strings.TrimSpace(key) != "BackgroundNormal" {
+ k, v, found := strings.Cut(line, "=")
+ if !found || strings.TrimSpace(k) != key {
continue
}
- return parseRGB(strings.TrimSpace(val))
+ return strings.TrimSpace(v), true
}
- return rgb, false
+ return "", false
}
// parseRGB parses KDE's "r,g,b" colour triple into bytes.
diff --git a/client/ui/tray_theme_linux_test.go b/client/ui/tray_theme_linux_test.go
index f14f08d7f..e3476055d 100644
--- a/client/ui/tray_theme_linux_test.go
+++ b/client/ui/tray_theme_linux_test.go
@@ -8,49 +8,346 @@ import (
"testing"
)
-func TestReadKdeComplementaryBackground(t *testing.T) {
- // Mirrors the KDE test VM's kdeglobals: Window light, Complementary dark.
- // The tray sits on the panel, which Plasma paints from Complementary, so
- // the panel is dark even though the global color-scheme is Light.
- content := `[Colors:Window]
-BackgroundNormal=239,240,241
-
-[Colors:Complementary]
+// Values measured on Plasma 6.7.4 (Fedora 44) under each Breeze scheme. The
+// Complementary group is dark under both, which is why it can't decide the
+// panel; Window tracks it.
+const (
+ kdeglobalsLight = `[Colors:Complementary]
BackgroundAlternate=27,30,32
BackgroundNormal=42,46,50
+[Colors:Window]
+BackgroundNormal=239,240,241
+
[General]
ColorSchemeHash=0be804dba87e3512aeb4be3d78ed981f59f0f2f4
`
- path := filepath.Join(t.TempDir(), "kdeglobals")
- if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
+ kdeglobalsDark = `[Colors:Complementary]
+BackgroundNormal=32,35,38
+
+[Colors:Window]
+BackgroundNormal=32,35,38
+
+[General]
+ColorScheme=BreezeDark
+`
+)
+
+// plasmaStyle installs a Plasma style of that name under XDG_DATA_HOME. An
+// empty colours body installs the style without a colours file, which is what
+// the stock "default" style looks like.
+func plasmaStyle(t *testing.T, name, colours string) string {
+ t.Helper()
+ data := t.TempDir()
+ dir := filepath.Join(data, "plasma", "desktoptheme", name)
+ if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatal(err)
}
+ if colours != "" {
+ if err := os.WriteFile(filepath.Join(dir, "colors"), []byte(colours), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ }
+ t.Setenv("XDG_DATA_HOME", data)
+ // Keep the system dirs out of it so an installed breeze-dark cannot answer.
+ t.Setenv("XDG_DATA_DIRS", filepath.Join(data, "empty"))
+ return data
+}
- rgb, ok := readKdeComplementaryBackground(path)
- if !ok {
- t.Fatal("expected to find Complementary BackgroundNormal")
+// kdeConfig points the KDE readers at a temp dir holding the given files, and
+// makes isKDE report KDE. An empty body skips the file.
+func kdeConfig(t *testing.T, kdeglobals, plasmarc string) {
+ t.Helper()
+ dir := t.TempDir()
+ for name, body := range map[string]string{kdeglobalsFile: kdeglobals, plasmarcFile: plasmarc} {
+ if body == "" {
+ continue
+ }
+ if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil {
+ t.Fatal(err)
+ }
}
- if rgb != [3]uint8{42, 46, 50} {
- t.Fatalf("rgb = %v, want [42 46 50]", rgb)
- }
- if !isDarkRGB(rgb[0], rgb[1], rgb[2]) {
- t.Fatal("panel colour 42,46,50 should be dark")
- }
- // The Window background (what color-scheme reflects) is light — the bug
- // this fix addresses is picking the icon from that instead of the panel.
- if isDarkRGB(239, 240, 241) {
- t.Fatal("window colour 239,240,241 should be light")
+ t.Setenv("XDG_CONFIG_HOME", dir)
+ t.Setenv("XDG_CURRENT_DESKTOP", "KDE")
+ // Point the Plasma style lookup at empty dirs so a style installed on the
+ // host cannot answer for a test that did not install one itself. Tests that
+ // want a style call plasmaStyle, which overrides these.
+ t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "empty-data-home"))
+ t.Setenv("XDG_DATA_DIRS", filepath.Join(dir, "empty-data-dirs"))
+}
+
+// The reported bug: a Light global scheme left the panel reported dark, so the
+// tray kept the white silhouette on a light panel.
+func TestKdePanelIsDarkFollowsColourScheme(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ kdeglobals string
+ wantDark bool
+ }{
+ {"light scheme", kdeglobalsLight, false},
+ {"dark scheme", kdeglobalsDark, true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ kdeConfig(t, tc.kdeglobals, "")
+ dark, ok := kdePanelIsDark()
+ if !ok {
+ t.Fatal("expected a conclusive answer from kdeglobals")
+ }
+ if dark != tc.wantDark {
+ t.Fatalf("dark = %v, want %v", dark, tc.wantDark)
+ }
+ })
}
}
-func TestReadKdeComplementaryBackgroundMissingGroup(t *testing.T) {
- path := filepath.Join(t.TempDir(), "kdeglobals")
- if err := os.WriteFile(path, []byte("[Colors:Window]\nBackgroundNormal=1,2,3\n"), 0o600); err != nil {
+// A pinned Plasma style paints the panel regardless of the colour scheme, so it
+// has to outrank it in both directions. The styles are installed into the test's
+// own XDG_DATA_HOME: reading whatever the host happens to ship would make the
+// result depend on the machine.
+func TestKdePanelIsDarkPinnedStyleOutranksScheme(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ kdeglobals string
+ style string
+ styleColours string
+ wantDark bool
+ }{
+ {"dark style, light scheme", kdeglobalsLight, "breeze-dark", kdeglobalsDark, true},
+ {"light style, dark scheme", kdeglobalsDark, "breeze-light", kdeglobalsLight, false},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ kdeConfig(t, tc.kdeglobals, "[Theme]\nname="+tc.style+"\n")
+ plasmaStyle(t, tc.style, tc.styleColours)
+ dark, ok := kdePanelIsDark()
+ if !ok {
+ t.Fatal("expected a conclusive answer from the pinned style")
+ }
+ if dark != tc.wantDark {
+ t.Fatalf("dark = %v, want %v", dark, tc.wantDark)
+ }
+ })
+ }
+}
+
+// "default" fixes nothing, so the colour scheme still decides.
+func TestKdePanelIsDarkDefaultStyleDefersToScheme(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname=default\n")
+ dark, ok := kdePanelIsDark()
+ if !ok {
+ t.Fatal("expected the colour scheme to answer")
+ }
+ if dark {
+ t.Fatal("default style on a Light scheme is a light panel")
+ }
+}
+
+// No colours and no style: stay inconclusive so readDarkMode uses the portal
+// rather than guessing.
+func TestKdePanelIsDarkInconclusive(t *testing.T) {
+ t.Run("no window group", func(t *testing.T) {
+ kdeConfig(t, "[Colors:Complementary]\nBackgroundNormal=42,46,50\n", "")
+ if _, ok := kdePanelIsDark(); ok {
+ t.Fatal("expected not-ok without a Window group")
+ }
+ })
+ t.Run("no kde files", func(t *testing.T) {
+ kdeConfig(t, "", "")
+ if _, ok := kdePanelIsDark(); ok {
+ t.Fatal("expected not-ok with no kdeglobals at all")
+ }
+ })
+ t.Run("not kde", func(t *testing.T) {
+ kdeConfig(t, kdeglobalsDark, "")
+ t.Setenv("XDG_CURRENT_DESKTOP", "ubuntu:GNOME")
+ if _, ok := kdePanelIsDark(); ok {
+ t.Fatal("expected not-ok off KDE")
+ }
+ })
+}
+
+// The panel follows the pinned style's own colours, not its name. Reproduces
+// the measured case: a neutrally named style shipping dark colours while
+// kdeglobals reports light.
+func TestPlasmaStyleIsDarkUsesStyleColours(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ style string
+ colours string
+ wantDark bool
+ }{
+ {"neutral name, dark colours", "nbtestneutral", kdeglobalsDark, true},
+ {"neutral name, light colours", "nbtestneutral", kdeglobalsLight, false},
+ {"name says dark, colours are light", "midnight-dark", kdeglobalsLight, false},
+ {"name says light, colours are dark", "daylight", kdeglobalsDark, true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname="+tc.style+"\n")
+ plasmaStyle(t, tc.style, tc.colours)
+ dark, ok := plasmaStyleIsDark()
+ if !ok {
+ t.Fatal("a style shipping colours should be conclusive")
+ }
+ if dark != tc.wantDark {
+ t.Fatalf("dark = %v, want %v", dark, tc.wantDark)
+ }
+ })
+ }
+}
+
+// End to end through kdePanelIsDark: the style's colours must beat kdeglobals.
+func TestKdePanelIsDarkStyleColoursBeatColourScheme(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname=nbtestneutral\n")
+ plasmaStyle(t, "nbtestneutral", kdeglobalsDark)
+ dark, ok := kdePanelIsDark()
+ if !ok {
+ t.Fatal("expected a conclusive answer")
+ }
+ if !dark {
+ t.Fatal("a dark-coloured style on a Light scheme is a dark panel")
+ }
+}
+
+// A style with no colours file is the "default" case: it follows the scheme.
+func TestPlasmaStyleIsDarkNoColoursFile(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname=default\n")
+ plasmaStyle(t, "default", "")
+ if _, ok := plasmaStyleIsDark(); ok {
+ t.Fatal("a style without colours must defer to the colour scheme")
+ }
+ // and the whole resolution then lands on the light scheme
+ dark, ok := kdePanelIsDark()
+ if !ok || dark {
+ t.Fatalf("kdePanelIsDark() = (%v, %v), want (false, true)", dark, ok)
+ }
+}
+
+func TestPlasmaStyleIsDarkInconclusive(t *testing.T) {
+ t.Run("no plasmarc", func(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "")
+ plasmaStyle(t, "unused", kdeglobalsDark)
+ if _, ok := plasmaStyleIsDark(); ok {
+ t.Fatal("expected not-ok with no plasmarc")
+ }
+ })
+ t.Run("empty style name", func(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname=\n")
+ if _, ok := plasmaStyleIsDark(); ok {
+ t.Fatal("expected not-ok for an empty style name")
+ }
+ })
+ t.Run("style not installed", func(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname=absent\n")
+ plasmaStyle(t, "somethingelse", kdeglobalsDark)
+ if _, ok := plasmaStyleIsDark(); ok {
+ t.Fatal("expected not-ok when the style is not installed")
+ }
+ })
+ // ... but a leading dot in an ordinary name is fine.
+ for _, good := range []string{".hidden", "..."} {
+ t.Run("accepts "+good, func(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname="+good+"\n")
+ plasmaStyle(t, good, kdeglobalsDark)
+ dark, ok := plasmaStyleIsDark()
+ if !ok || !dark {
+ t.Fatalf("plasmaStyleIsDark() = (%v, %v) for %q, want (true, true)", dark, ok, good)
+ }
+ })
+ }
+}
+
+// A traversing name must be rejected outright, not merely fail to find a file.
+// Each case plants colours at exactly the path the unguarded lookup would read,
+// so removing the name check makes plasmaStyleIsDark answer from the planted
+// file and these fail. Without the planted file the test would pass either way.
+func TestPlasmaStyleIsDarkRejectsPlantedEscape(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ // where filepath.Join(data, "plasma", "desktoptheme", name) lands
+ escaped []string
+ }{
+ {"..", []string{"plasma"}},
+ {".", []string{"plasma", "desktoptheme"}},
+ {"/", []string{"plasma", "desktoptheme"}},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ kdeConfig(t, kdeglobalsLight, "[Theme]\nname="+tc.name+"\n")
+ data := plasmaStyle(t, "unused", kdeglobalsLight)
+
+ target := filepath.Join(append([]string{data}, tc.escaped...)...)
+ // Guard the fixture itself: if Join ever stops landing here the
+ // test would go quietly vacuous again.
+ want := filepath.Clean(filepath.Join(data, "plasma", "desktoptheme", tc.name))
+ if target != want {
+ t.Fatalf("fixture targets %q but the lookup resolves %q", target, want)
+ }
+ if err := os.WriteFile(filepath.Join(target, "colors"), []byte(kdeglobalsDark), 0o600); err != nil {
+ t.Fatal(err)
+ }
+
+ if dark, ok := plasmaStyleIsDark(); ok {
+ t.Fatalf("plasmaStyleIsDark() = (%v, true) for %q: the name must be rejected, "+
+ "not resolved against %s", dark, tc.name, target)
+ }
+ })
+ }
+}
+
+// The rejection condition on its own, so every case is checked whether or not a
+// file happens to exist at the path it would resolve to.
+func TestIsBareStyleName(t *testing.T) {
+ for _, bad := range []string{"", ".", "..", "/", "//", "/etc", "../../../../etc", "a/b", "a/", "./x", "../x"} {
+ if isBareStyleName(bad) {
+ t.Errorf("isBareStyleName(%q) = true, want false", bad)
+ }
+ }
+ for _, good := range []string{"breeze-dark", "default", ".hidden", "...", "Breeze Dark", "a.b"} {
+ if !isBareStyleName(good) {
+ t.Errorf("isBareStyleName(%q) = false, want true", good)
+ }
+ }
+}
+
+// A local style of the same name shadows the system one, as in Plasma.
+func TestPlasmaStyleDirsPreferUserData(t *testing.T) {
+ t.Setenv("XDG_DATA_HOME", "/home/someone/.local/share")
+ t.Setenv("XDG_DATA_DIRS", "/usr/local/share:/usr/share")
+ got := plasmaStyleDirs("breeze-dark")
+ want := []string{
+ "/home/someone/.local/share/plasma/desktoptheme/breeze-dark",
+ "/usr/local/share/plasma/desktoptheme/breeze-dark",
+ "/usr/share/plasma/desktoptheme/breeze-dark",
+ }
+ if len(got) != len(want) {
+ t.Fatalf("plasmaStyleDirs() = %v, want %v", got, want)
+ }
+ for i := range want {
+ if got[i] != want[i] {
+ t.Fatalf("plasmaStyleDirs()[%d] = %q, want %q", i, got[i], want[i])
+ }
+ }
+}
+
+func TestReadIniValue(t *testing.T) {
+ path := filepath.Join(t.TempDir(), kdeglobalsFile)
+ if err := os.WriteFile(path, []byte(kdeglobalsLight), 0o600); err != nil {
t.Fatal(err)
}
- if _, ok := readKdeComplementaryBackground(path); ok {
- t.Fatal("expected not-ok when Complementary group is absent")
+ // The same key exists in two groups, so a group-blind reader would return
+ // whichever came first.
+ if v, ok := readIniValue(path, "[Colors:Window]", "BackgroundNormal"); !ok || v != "239,240,241" {
+ t.Fatalf("Window BackgroundNormal = %q ok=%v, want \"239,240,241\" true", v, ok)
+ }
+ if v, ok := readIniValue(path, "[Colors:Complementary]", "BackgroundNormal"); !ok || v != "42,46,50" {
+ t.Fatalf("Complementary BackgroundNormal = %q ok=%v, want \"42,46,50\" true", v, ok)
+ }
+ if _, ok := readIniValue(path, "[Colors:Window]", "ColorSchemeHash"); ok {
+ t.Fatal("a key from another group should not be found")
+ }
+ if _, ok := readIniValue(filepath.Join(t.TempDir(), "absent"), "[Theme]", "name"); ok {
+ t.Fatal("a missing file should not be found")
+ }
+ if _, ok := readIniValue("", "[Theme]", "name"); ok {
+ t.Fatal("an empty path should not be found")
}
}
@@ -77,8 +374,8 @@ func TestIsDarkRGB(t *testing.T) {
if isDarkRGB(255, 255, 255) {
t.Fatal("white is light")
}
- if !isDarkRGB(42, 46, 50) {
- t.Fatal("Breeze panel grey is dark")
+ if !isDarkRGB(32, 35, 38) {
+ t.Fatal("BreezeDark window grey is dark")
}
if isDarkRGB(239, 240, 241) {
t.Fatal("Breeze window grey is light")
diff --git a/client/ui/tray_theme_watcher_linux.go b/client/ui/tray_theme_watcher_linux.go
index b9bafe30b..fa175e006 100644
--- a/client/ui/tray_theme_watcher_linux.go
+++ b/client/ui/tray_theme_watcher_linux.go
@@ -3,9 +3,10 @@
package main
// Sources: the freedesktop Settings portal's SettingChanged signal, and on KDE
-// the kdeglobals file (the portal's color-scheme doesn't track the panel's
-// Complementary colour — see readDarkMode). The dark/light decision lives in
-// tray_theme_linux.go; this file owns the session-bus connection and subscriptions.
+// the kdeglobals and plasmarc files (a pinned Plasma style fixes the panel's
+// appearance without touching the portal's color-scheme — see readDarkMode).
+// The dark/light decision lives in tray_theme_linux.go; this file owns the
+// session-bus connection and subscriptions.
import (
"path/filepath"
@@ -66,9 +67,9 @@ func startThemeWatcher(onChange func()) *themeWatcher {
// Keep the connection: the seeded darkMode value is still useful.
}
- // The portal's signal doesn't track KDE's panel Complementary colour.
+ // The portal's signal says nothing about a pinned Plasma style.
if isKDE() {
- w.watchKdeglobals()
+ w.watchKdeConfig()
}
log.Infof("tray theme: panel dark mode = %v", w.IsDark())
@@ -88,12 +89,11 @@ func (w *themeWatcher) IsDark() bool {
// readDarkMode resolves whether the panel the tray icon sits on is dark.
//
-// On KDE the freedesktop color-scheme is the application preference, not the
-// panel's: Plasma paints its panel from the Breeze "Complementary" group, which
-// stays dark even under a Light global scheme, so we read the panel background
-// from kdeglobals first and decide by its luma. Off KDE the color-scheme portal
-// is the source; on "no preference" (0) or when unavailable we fall back to
-// GTK_THEME (":dark" suffix ⇒ dark), then default to dark.
+// KDE goes first because a pinned Plasma style decides the panel on its own,
+// independently of the application colour scheme the portal reports; with no
+// style pinned that check defers to KDE's own colour files. Off KDE the
+// color-scheme portal is the source; on "no preference" (0) or when unavailable
+// we fall back to GTK_THEME (":dark" suffix ⇒ dark), then default to dark.
func (w *themeWatcher) readDarkMode() bool {
if dark, ok := kdePanelIsDark(); ok {
return dark
@@ -160,8 +160,8 @@ func (w *themeWatcher) loop(sigs chan *dbus.Signal) {
continue
}
- // Re-resolve via readDarkMode, not the signal value: under KDE the panel
- // colour comes from kdeglobals, so the signal value would be wrong.
+ // Re-resolve via readDarkMode, not the signal value: under KDE a pinned
+ // Plasma style overrides it, so the signal value would be wrong.
w.update()
}
}
@@ -179,22 +179,23 @@ func (w *themeWatcher) update() {
}
}
-// watchKdeglobals watches the parent directory, not the file: KDE rewrites
-// kdeglobals atomically (write-temp + rename), which would drop an inotify watch
-// on the original inode. Filtering by name re-arms implicitly.
-func (w *themeWatcher) watchKdeglobals() {
+// watchKdeConfig repaints on writes to either KDE file that decides the panel
+// appearance. It watches their parent directory, not the files: KDE rewrites
+// them atomically (write-temp + rename), which would drop an inotify watch on
+// the original inode. Filtering by name re-arms implicitly.
+func (w *themeWatcher) watchKdeConfig() {
path := kdeglobalsPath()
if path == "" {
return
}
- dir, name := filepath.Split(path)
+ dir := filepath.Dir(path)
fw, err := fsnotify.NewWatcher()
if err != nil {
- log.Debugf("tray theme: kdeglobals watcher unavailable, theme is static: %v", err)
+ log.Debugf("tray theme: KDE config watcher unavailable, theme is static: %v", err)
return
}
- if err := fw.Add(filepath.Clean(dir)); err != nil {
+ if err := fw.Add(dir); err != nil {
log.Debugf("tray theme: watching %s failed, theme is static: %v", dir, err)
_ = fw.Close()
return
@@ -208,10 +209,14 @@ func (w *themeWatcher) watchKdeglobals() {
if !ok {
return
}
- if filepath.Base(event.Name) != name {
+ switch filepath.Base(event.Name) {
+ case kdeglobalsFile, plasmarcFile:
+ default:
continue
}
- if event.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename) == 0 {
+ // Remove counts: deleting plasmarc unpins the Plasma style, which
+ // hands the decision back to the colour scheme and can flip it.
+ if event.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename|fsnotify.Remove) == 0 {
continue
}
w.update()
@@ -219,7 +224,7 @@ func (w *themeWatcher) watchKdeglobals() {
if !ok {
return
}
- log.Debugf("tray theme: kdeglobals watch error: %v", err)
+ log.Debugf("tray theme: KDE config watch error: %v", err)
}
}
}()
diff --git a/docs/custom-domain-validation.md b/docs/custom-domain-validation.md
new file mode 100644
index 000000000..a70388e4d
--- /dev/null
+++ b/docs/custom-domain-validation.md
@@ -0,0 +1,28 @@
+A new custom domain name is converted to lowercase ASCII (punycode), with a
+trailing dot removed, before availability and DNS validation checks. Invalid
+names and wildcard registrations are rejected before storage.
+
+A custom domain registration must complete validation within 48 hours of
+creation. Retrying validation does not extend this window. Once validation
+succeeds, the registration is exempt from this expiration policy.
+
+Management removes expired, unvalidated registrations at startup and every
+60 minutes. While management is running, removal normally occurs between
+48 and 49 hours after registration. Validation is refused after the 48-hour
+deadline even if cleanup has not yet removed the registration.
+
+Removal releases the name for a new registration. The new registration must
+complete its own validation. Its account does not inherit validation or
+services from the expired registration.
+
+The original account receives a system activity event named
+`CustomDomainValidationExpired`, displayed as "Unvalidated domain registration
+expired". The event includes the domain name, original registration ID, and
+validation deadline.
+
+On upgrade, existing unvalidated registrations receive a 48-hour validation
+window. Restarting management does not extend a previously assigned deadline.
+
+Registrations with existing services, including services using subdomains, are
+retained for operator review. Management logs their account and domain IDs so
+an operator can identify and resolve those dependencies before cleanup.
diff --git a/encryption/message.go b/encryption/message.go
index 6e4cd7391..2bf2c59dc 100644
--- a/encryption/message.go
+++ b/encryption/message.go
@@ -6,6 +6,16 @@ import (
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
)
+type Encrypter interface {
+ EncryptMessage(remotePubKey wgtypes.Key, ourPrivateKey wgtypes.Key, message pb.Message) ([]byte, error)
+}
+
+type DefaultEncrypter struct{}
+
+func (e DefaultEncrypter) EncryptMessage(remotePubKey wgtypes.Key, ourPrivateKey wgtypes.Key, message pb.Message) ([]byte, error) {
+ return EncryptMessage(remotePubKey, ourPrivateKey, message)
+}
+
// EncryptMessage encrypts a body of the given protobuf Message
func EncryptMessage(remotePubKey wgtypes.Key, ourPrivateKey wgtypes.Key, message pb.Message) ([]byte, error) {
byteResp, err := pb.Marshal(message)
diff --git a/go.mod b/go.mod
index a2fe1e55b..03879a58a 100644
--- a/go.mod
+++ b/go.mod
@@ -331,7 +331,7 @@ replace github.com/kardianos/service => github.com/netbirdio/service v0.0.0-2024
replace github.com/getlantern/systray => github.com/netbirdio/systray v0.0.0-20231030152038-ef1ed2a27949
-replace golang.zx2c4.com/wireguard => github.com/netbirdio/wireguard-go v0.0.0-20260628102922-2834bebf6c1a
+replace golang.zx2c4.com/wireguard => github.com/netbirdio/wireguard-go v0.0.0-20260914123147-8bf8fa968f1a
replace github.com/cloudflare/circl => codeberg.org/cunicu/circl v0.0.0-20230801113412-fec58fc7b5f6
@@ -343,6 +343,6 @@ replace github.com/dexidp/dex/api/v2 => github.com/netbirdio/dex/api/v2 v2.0.0-2
replace github.com/mailru/easyjson => github.com/netbirdio/easyjson v0.9.0
-replace github.com/wailsapp/wails/v3 => github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260825085513-5f07a01f7a78
+replace github.com/wailsapp/wails/v3 => github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260902163841-4a71f7b1d9e1
tool go.uber.org/mock/mockgen
diff --git a/go.sum b/go.sum
index 3e0b4f5dc..e9c666526 100644
--- a/go.sum
+++ b/go.sum
@@ -490,10 +490,10 @@ github.com/netbirdio/service v0.0.0-20240911161631-f62744f42502 h1:3tHlFmhTdX9ax
github.com/netbirdio/service v0.0.0-20240911161631-f62744f42502/go.mod h1:CIMRFEJVL+0DS1a3Nx06NaMn4Dz63Ng6O7dl0qH0zVM=
github.com/netbirdio/signal-dispatcher/dispatcher v0.0.0-20250805121659-6b4ac470ca45 h1:ujgviVYmx243Ksy7NdSwrdGPSRNE3pb8kEDSpH0QuAQ=
github.com/netbirdio/signal-dispatcher/dispatcher v0.0.0-20250805121659-6b4ac470ca45/go.mod h1:5/sjFmLb8O96B5737VCqhHyGRzNFIaN/Bu7ZodXc3qQ=
-github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260825085513-5f07a01f7a78 h1:B/jRv24jnFeoA+VccxoCx6K94PUgsqR9wnshpeu9M+8=
-github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260825085513-5f07a01f7a78/go.mod h1:/6QR46/nhGCSADHbS++XtDb9dkTnenTHlGskTPRo9S0=
-github.com/netbirdio/wireguard-go v0.0.0-20260628102922-2834bebf6c1a h1:3CWK+yTvRKOcC0Q8VCTGy4l60TEb27CQVS7LkMxwjmw=
-github.com/netbirdio/wireguard-go v0.0.0-20260628102922-2834bebf6c1a/go.mod h1:rpwXGsirqLqN2L0JDJQlwOboGHmptD5ZD6T2VmcqhTw=
+github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260902163841-4a71f7b1d9e1 h1:n5aXV/U6I9bLc+yWN088TyVR4OfF64Gy+L6Hrffc+n4=
+github.com/netbirdio/wails/v3 v3.0.0-beta.3.0.20260902163841-4a71f7b1d9e1/go.mod h1:/6QR46/nhGCSADHbS++XtDb9dkTnenTHlGskTPRo9S0=
+github.com/netbirdio/wireguard-go v0.0.0-20260914123147-8bf8fa968f1a h1:Nt8BgkTkI56LGBPPEBywM406MVKJDqeDIVdgsZyYs80=
+github.com/netbirdio/wireguard-go v0.0.0-20260914123147-8bf8fa968f1a/go.mod h1:rpwXGsirqLqN2L0JDJQlwOboGHmptD5ZD6T2VmcqhTw=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
github.com/nxadm/tail v1.4.11 h1:8feyoE3OzPrcshW5/MJ4sGESc5cqmGkGCWlco4l0bqY=
diff --git a/management/internals/modules/reverseproxy/domain/domain.go b/management/internals/modules/reverseproxy/domain/domain.go
index 08d7ad19b..74583c55f 100644
--- a/management/internals/modules/reverseproxy/domain/domain.go
+++ b/management/internals/modules/reverseproxy/domain/domain.go
@@ -1,5 +1,13 @@
package domain
+import "time"
+
+// ValidationTTL is the time available to validate a custom domain registration.
+const ValidationTTL = 48 * time.Hour
+
+// ID identifies a custom domain registration.
+type ID string
+
type Type string
const (
@@ -8,12 +16,13 @@ const (
)
type Domain struct {
- ID string `gorm:"unique;primaryKey;autoIncrement"`
- Domain string `gorm:"unique"` // Domain records must be unique, this avoids domain reuse across accounts.
- AccountID string `gorm:"index"`
- TargetCluster string // The proxy cluster this domain should be validated against
- Type Type `gorm:"-"`
- Validated bool
+ ID string `gorm:"unique;primaryKey;autoIncrement"`
+ Domain string `gorm:"unique"` // Domain records must be unique, this avoids domain reuse across accounts.
+ AccountID string `gorm:"index"`
+ TargetCluster string // The proxy cluster this domain should be validated against
+ Type Type `gorm:"-"`
+ Validated bool
+ ValidationExpiresAt *time.Time `gorm:"index"`
// SupportsCustomPorts is populated at query time for free domains from the
// proxy cluster capabilities. Not persisted.
SupportsCustomPorts *bool `gorm:"-"`
@@ -36,7 +45,12 @@ func (d *Domain) EventMeta() map[string]any {
}
}
+// Copy returns a copy with an independent validation deadline.
func (d *Domain) Copy() *Domain {
dCopy := *d
+ if d.ValidationExpiresAt != nil {
+ expiresAt := *d.ValidationExpiresAt
+ dCopy.ValidationExpiresAt = &expiresAt
+ }
return &dCopy
}
diff --git a/management/internals/modules/reverseproxy/domain/manager/expiration.go b/management/internals/modules/reverseproxy/domain/manager/expiration.go
new file mode 100644
index 000000000..381e3ca51
--- /dev/null
+++ b/management/internals/modules/reverseproxy/domain/manager/expiration.go
@@ -0,0 +1,73 @@
+package manager
+
+import (
+ "context"
+ "time"
+
+ log "github.com/sirupsen/logrus"
+
+ "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+ "github.com/netbirdio/netbird/management/server/activity"
+)
+
+const (
+ validationCleanupInterval = 60 * time.Minute
+ validationCleanupBatch = 100
+)
+
+// RunValidationCleanup removes expired registrations on startup and hourly until cancellation.
+func (m Manager) RunValidationCleanup(ctx context.Context) {
+ ticker := time.NewTicker(validationCleanupInterval)
+ defer ticker.Stop()
+ for {
+ m.cleanupExpiredDomains(ctx, time.Now().UTC())
+ select {
+ case <-ctx.Done():
+ return
+ case <-ticker.C:
+ }
+ }
+}
+
+func (m Manager) cleanupExpiredDomains(ctx context.Context, now time.Time) {
+ var afterID domain.ID
+ for ctx.Err() == nil {
+ domains, err := m.store.GetExpiredCustomDomains(ctx, now, afterID, validationCleanupBatch)
+ if err != nil {
+ if ctx.Err() == nil {
+ log.WithContext(ctx).WithError(err).Error("list expired custom domain registrations")
+ }
+ return
+ }
+ for _, d := range domains {
+ if ctx.Err() != nil {
+ return
+ }
+ m.deleteExpiredDomain(ctx, d, now)
+ afterID = domain.ID(d.ID)
+ }
+ if len(domains) < validationCleanupBatch {
+ return
+ }
+ }
+}
+
+func (m Manager) deleteExpiredDomain(ctx context.Context, d *domain.Domain, now time.Time) {
+ deleted, err := m.store.DeleteExpiredCustomDomain(ctx, d, now)
+ if err != nil {
+ if ctx.Err() == nil {
+ log.WithContext(ctx).WithFields(log.Fields{"accountID": d.AccountID, "domainID": d.ID}).
+ WithError(err).Warn("could not expire custom domain registration")
+ }
+ return
+ }
+ if !deleted {
+ return
+ }
+ meta := d.EventMeta()
+ if d.ValidationExpiresAt != nil {
+ meta["validation_expires_at"] = d.ValidationExpiresAt.UTC().Format(time.RFC3339)
+ }
+ m.accountManager.StoreEvent(ctx, activity.SystemInitiator, d.ID, d.AccountID,
+ activity.CustomDomainValidationExpired, meta)
+}
diff --git a/management/internals/modules/reverseproxy/domain/manager/expiration_test.go b/management/internals/modules/reverseproxy/domain/manager/expiration_test.go
new file mode 100644
index 000000000..e3b05efbb
--- /dev/null
+++ b/management/internals/modules/reverseproxy/domain/manager/expiration_test.go
@@ -0,0 +1,274 @@
+package manager
+
+import (
+ "context"
+ "fmt"
+ "sync"
+ "testing"
+ "testing/synctest"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+ rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+ "github.com/netbirdio/netbird/management/server/activity"
+ "github.com/netbirdio/netbird/management/server/mock_server"
+ nbstore "github.com/netbirdio/netbird/management/server/store"
+)
+
+func TestValidateDomain_ExpiredRegistration(t *testing.T) {
+ env := setupDomainTest(t)
+ ctx := context.Background()
+ d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "expired.example.com", testCluster)
+ require.NoError(t, err)
+ expiresAt := time.Now().Add(-time.Second)
+ db := env.store.(*nbstore.SqlStore).GetDB()
+ require.NoError(t, db.Model(&domain.Domain{}).Where("id = ?", d.ID).
+ Update("validation_expires_at", expiresAt).Error)
+ env.resolver.set("validation.expired.example.com", testCluster)
+
+ env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
+
+ stored := storedDomain(t, env.store, accountA, d.Domain)
+ require.NotNil(t, stored)
+ assert.False(t, stored.Validated, "an expired registration must not become usable before cleanup runs")
+}
+
+func TestCreateDomain_ValidationDeadline(t *testing.T) {
+ env := setupClockDomainTest(t)
+ synctest.Test(t, func(t *testing.T) {
+ ctx := context.Background()
+ createdAt := time.Now().UTC()
+ d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "pending.example.com", testCluster)
+ require.NoError(t, err)
+ require.NotNil(t, d.ValidationExpiresAt)
+ assert.Equal(t, createdAt.Add(48*time.Hour), *d.ValidationExpiresAt, "new registrations get 48 hours")
+
+ time.Sleep(time.Hour)
+ env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
+ stored := storedDomain(t, env.store, accountA, d.Domain)
+ require.NotNil(t, stored)
+ require.NotNil(t, stored.ValidationExpiresAt)
+ assert.WithinDuration(t, *d.ValidationExpiresAt, *stored.ValidationExpiresAt, 0, "failed validation must not extend the deadline")
+ })
+}
+
+func TestCleanupExpiredDomains_Boundaries(t *testing.T) {
+ env := setupDomainTest(t)
+ events := captureDomainEvents(env)
+ ctx := context.Background()
+ now := time.Now().UTC().Truncate(time.Second)
+ tests := []struct {
+ name string
+ expiresAt time.Time
+ validated bool
+ deleted bool
+ }{
+ {"expired", now.Add(-time.Second), false, true},
+ {"deadline", now, false, true},
+ {"pending", now.Add(time.Second), false, false},
+ {"validated", now.Add(-time.Hour), true, false},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ d := createExpiringDomain(t, env, tt.name+".example.com", tt.expiresAt)
+ if tt.validated {
+ require.NoError(t, env.store.(*nbstore.SqlStore).GetDB().Model(d).Update("validated", true).Error)
+ }
+ env.manager.cleanupExpiredDomains(ctx, now)
+ stored := storedDomain(t, env.store, accountA, d.Domain)
+ if !tt.deleted {
+ assert.NotNil(t, stored, "pending and validated registrations must survive cleanup")
+ return
+ }
+ assert.Nil(t, stored, "expired unused registrations must be removed")
+ replacement, err := env.manager.CreateDomain(ctx, accountB, accountBUser, d.Domain, testCluster)
+ require.NoError(t, err)
+ assert.NotEqual(t, d.ID, replacement.ID, "the released name must receive a fresh registration")
+ assert.False(t, replacement.Validated, "the new account must validate its own registration")
+ })
+ }
+ got := events.get()
+ require.Len(t, got, 2, "only successful expiration deletions emit events")
+ for _, event := range got {
+ assert.Equal(t, activity.CustomDomainValidationExpired, event.Activity, "use the requested expiration event")
+ assert.Equal(t, activity.SystemInitiator, event.InitiatorID, "cleanup is attributed to the system")
+ assert.Equal(t, accountA, event.AccountID, "expiration belongs to the original account")
+ assert.NotEmpty(t, event.TargetID, "retain the deleted domain ID")
+ assert.NotEmpty(t, event.Meta["domain"], "retain the deleted domain name")
+ assert.NotEmpty(t, event.Meta["validation_expires_at"], "include the validation deadline")
+ }
+}
+
+func TestCleanupExpiredDomains_ContinuesPastProtectedBatch(t *testing.T) {
+ env := setupDomainTest(t)
+ ctx := context.Background()
+ now := time.Now().UTC()
+ for i := range validationCleanupBatch {
+ d := createExpiringDomain(t, env, fmt.Sprintf("protected-%d.example.com", i), now.Add(-time.Hour))
+ require.NoError(t, env.store.CreateService(ctx, &rpservice.Service{
+ ID: fmt.Sprintf("service-%d", i), AccountID: accountA, Domain: "app." + d.Domain,
+ }))
+ }
+ unprotected := createExpiringDomain(t, env, "unused.example.com", now.Add(-time.Hour))
+ env.manager.cleanupExpiredDomains(ctx, now)
+ assert.Nil(t, storedDomain(t, env.store, accountA, unprotected.Domain), "protected registrations must not starve later batches")
+ remaining, err := env.store.ListCustomDomains(ctx, accountA)
+ require.NoError(t, err)
+ assert.Len(t, remaining, validationCleanupBatch, "all registrations with dependent services must survive")
+}
+
+func TestCleanupExpiredDomains_ConcurrentWorkers(t *testing.T) {
+ env := setupDomainTest(t)
+ events := captureDomainEvents(env)
+ now := time.Now().UTC()
+ d := createExpiringDomain(t, env, "concurrent.example.com", now.Add(-time.Hour))
+ var workers sync.WaitGroup
+ for range 2 {
+ workers.Go(func() { env.manager.cleanupExpiredDomains(context.Background(), now) })
+ }
+ workers.Wait()
+ assert.Nil(t, storedDomain(t, env.store, accountA, d.Domain), "one worker must remove the expired registration")
+ assert.Len(t, events.get(), 1, "only the worker that deletes the row may emit the event")
+}
+
+func TestRunValidationCleanup_HourlyAndRestart(t *testing.T) {
+ env := setupClockDomainTest(t)
+ synctest.Test(t, func(t *testing.T) {
+ events := captureDomainEvents(env)
+ now := time.Now().UTC()
+ startup := createExpiringDomain(t, env, "startup.example.com", now.Add(-time.Hour))
+ hourly := createExpiringDomain(t, env, "hourly.example.com", now.Add(time.Minute))
+ ctx, cancel := context.WithCancel(context.Background())
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ env.manager.RunValidationCleanup(ctx)
+ }()
+ synctest.Wait()
+ assert.Nil(t, storedDomain(t, env.store, accountA, startup.Domain), "startup must collect overdue registrations")
+ time.Sleep(59 * time.Minute)
+ synctest.Wait()
+ assert.NotNil(t, storedDomain(t, env.store, accountA, hourly.Domain), "cleanup must wait for the 60-minute interval")
+ time.Sleep(time.Minute)
+ synctest.Wait()
+ assert.Nil(t, storedDomain(t, env.store, accountA, hourly.Domain), "the hourly scan must collect expired registrations")
+ cancel()
+ <-done
+
+ offline := createExpiringDomain(t, env, "offline.example.com", time.Now().UTC().Add(time.Minute))
+ time.Sleep(2 * time.Hour)
+ assert.NotNil(t, storedDomain(t, env.store, accountA, offline.Domain), "a stopped worker must not continue deleting")
+ ctx, cancel = context.WithCancel(context.Background())
+ done = make(chan struct{})
+ go func() {
+ defer close(done)
+ env.manager.RunValidationCleanup(ctx)
+ }()
+ synctest.Wait()
+ assert.Nil(t, storedDomain(t, env.store, accountA, offline.Domain), "restart must use the persisted deadline")
+ cancel()
+ <-done
+ assert.Len(t, events.get(), 3, "each deletion should emit an expiration event")
+ })
+}
+
+type blockingDomainResolver struct {
+ started chan struct{}
+ release chan struct{}
+}
+
+func (r blockingDomainResolver) LookupCNAME(context.Context, string) (string, error) {
+ close(r.started)
+ <-r.release
+ return testCluster + ".", nil
+}
+
+func TestValidateDomain_DeadlinePassesDuringLookup(t *testing.T) {
+ for _, cleanup := range []bool{false, true} {
+ t.Run(fmt.Sprintf("cleanup=%t", cleanup), func(t *testing.T) {
+ env := setupClockDomainTest(t)
+ synctest.Test(t, func(t *testing.T) {
+ events := captureDomainEvents(env)
+ ctx := context.Background()
+ d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "late.example.com", testCluster)
+ require.NoError(t, err)
+ resolver := blockingDomainResolver{started: make(chan struct{}), release: make(chan struct{})}
+ env.manager.validator.Resolver = resolver
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
+ }()
+ <-resolver.started
+ time.Sleep(48 * time.Hour)
+ if cleanup {
+ env.manager.cleanupExpiredDomains(ctx, time.Now().UTC())
+ _, err = env.store.CreateCustomDomain(ctx, accountB, d.Domain, testCluster, false)
+ require.NoError(t, err)
+ }
+ close(resolver.release)
+ <-done
+ owner := accountA
+ if cleanup {
+ assert.Nil(t, storedDomain(t, env.store, accountA, d.Domain), "late validation must not restore the old claim")
+ owner = accountB
+ }
+ stored := storedDomain(t, env.store, owner, d.Domain)
+ require.NotNil(t, stored)
+ assert.False(t, stored.Validated, "late validation must not validate either claim")
+ for _, event := range events.get() {
+ assert.NotEqual(t, activity.DomainValidated, event.Activity, "a rejected write must not emit a validation event")
+ }
+ })
+ })
+ }
+}
+
+func setupClockDomainTest(t *testing.T) *domainTestEnv {
+ t.Helper()
+ // Network driver watchers cannot share cancellation channels across synctest bubbles.
+ // Store boundary and concurrency tests still exercise the selected database engine.
+ t.Setenv("NETBIRD_STORE_ENGINE", "sqlite")
+ return setupDomainTest(t)
+}
+
+func createExpiringDomain(t *testing.T, env *domainTestEnv, name string, expiresAt time.Time) *domain.Domain {
+ t.Helper()
+ d, err := env.store.CreateCustomDomain(context.Background(), accountA, name, testCluster, false)
+ require.NoError(t, err)
+ require.NoError(t, env.store.(*nbstore.SqlStore).GetDB().Model(d).Update("validation_expires_at", expiresAt).Error)
+ d.ValidationExpiresAt = &expiresAt
+ return d
+}
+
+type domainEvents struct {
+ mu sync.Mutex
+ events []*activity.Event
+}
+
+func captureDomainEvents(env *domainTestEnv) *domainEvents {
+ events := &domainEvents{}
+ env.manager.accountManager = &mock_server.MockAccountManager{
+ StoreEventFunc: func(_ context.Context, initiator, target, account string, code activity.ActivityDescriber, meta map[string]any) {
+ if code == activity.DomainAdded {
+ return
+ }
+ events.mu.Lock()
+ defer events.mu.Unlock()
+ events.events = append(events.events, &activity.Event{
+ InitiatorID: initiator, TargetID: target, AccountID: account,
+ Activity: code.(activity.Activity), Meta: meta,
+ })
+ },
+ }
+ return events
+}
+
+func (e *domainEvents) get() []*activity.Event {
+ e.mu.Lock()
+ defer e.mu.Unlock()
+ return append([]*activity.Event(nil), e.events...)
+}
diff --git a/management/internals/modules/reverseproxy/domain/manager/manager.go b/management/internals/modules/reverseproxy/domain/manager/manager.go
index 46e4ced83..c0fb12e9c 100644
--- a/management/internals/modules/reverseproxy/domain/manager/manager.go
+++ b/management/internals/modules/reverseproxy/domain/manager/manager.go
@@ -6,6 +6,7 @@ import (
"fmt"
"net"
"strings"
+ "time"
log "github.com/sirupsen/logrus"
@@ -18,6 +19,7 @@ import (
"github.com/netbirdio/netbird/management/server/permissions/operations"
nbstore "github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types"
+ nbdomain "github.com/netbirdio/netbird/shared/management/domain"
"github.com/netbirdio/netbird/shared/management/status"
)
@@ -32,6 +34,8 @@ type store interface {
CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error)
DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error
+ GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error)
+ DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error)
}
type proxyManager interface {
@@ -106,12 +110,13 @@ func (m Manager) GetDomains(ctx context.Context, accountID, userID string) ([]*d
// Add custom domains.
for _, d := range domains {
cd := &domain.Domain{
- ID: d.ID,
- Domain: d.Domain,
- AccountID: accountID,
- TargetCluster: d.TargetCluster,
- Type: domain.TypeCustom,
- Validated: d.Validated,
+ ID: d.ID,
+ Domain: d.Domain,
+ AccountID: accountID,
+ TargetCluster: d.TargetCluster,
+ Type: domain.TypeCustom,
+ Validated: d.Validated,
+ ValidationExpiresAt: d.ValidationExpiresAt,
}
if d.TargetCluster != "" {
cd.SupportsCustomPorts = m.proxyManager.ClusterSupportsCustomPorts(ctx, d.TargetCluster)
@@ -126,6 +131,7 @@ func (m Manager) GetDomains(ctx context.Context, accountID, userID string) ([]*d
return ret, nil
}
+// CreateDomain registers a normalized custom domain and attempts DNS validation.
func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName, targetCluster string) (*domain.Domain, error) {
ok, ctx, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Create)
if err != nil {
@@ -135,6 +141,15 @@ func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName
return nil, status.NewPermissionDeniedError()
}
+ parsed, err := nbdomain.FromString(strings.TrimSuffix(domainName, "."))
+ if err != nil {
+ return nil, status.Errorf(status.InvalidArgument, "invalid domain: %v", err)
+ }
+ domainName = parsed.PunycodeString()
+ if !nbdomain.IsValidDomainNoWildcard(domainName) {
+ return nil, status.Errorf(status.InvalidArgument, "invalid domain format")
+ }
+
// Verify the target cluster is in the available clusters for this account
allowList, err := m.getClusterAllowList(ctx, accountID)
if err != nil {
@@ -243,6 +258,14 @@ func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID
}).WithError(err).Error("get custom domain from store")
return
}
+ if d.Validated {
+ return
+ }
+ if d.ValidationExpiresAt == nil || !time.Now().Before(*d.ValidationExpiresAt) {
+ log.WithFields(log.Fields{"accountID": accountID, "domainID": domainID}).
+ Debug("custom domain validation window has expired")
+ return
+ }
// Validate only against the domain's target cluster
targetCluster := d.TargetCluster
@@ -263,20 +286,21 @@ func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID
}).Info("validating domain against target cluster")
if m.validator.IsValid(context.Background(), d.Domain, []string{targetCluster}) {
- log.WithFields(log.Fields{
- "accountID": accountID,
- "domainID": domainID,
- "domain": d.Domain,
- }).Info("domain validated successfully")
d.Validated = true
if _, err := m.store.UpdateCustomDomain(context.Background(), accountID, d); err != nil {
- log.WithFields(log.Fields{
+ entry := log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
- "domain": d.Domain,
- }).WithError(err).Error("update custom domain in store")
+ }).WithError(err)
+ if sErr, ok := status.FromError(err); ok && sErr.Type() == status.PreconditionFailed {
+ entry.Debug("custom domain registration is no longer pending validation")
+ return
+ }
+ entry.Error("update custom domain in store")
return
}
+ log.WithFields(log.Fields{"accountID": accountID, "domainID": domainID}).
+ Info("custom domain validated successfully")
m.accountManager.StoreEvent(context.Background(), userID, domainID, accountID, activity.DomainValidated, d.EventMeta())
} else {
diff --git a/management/internals/modules/reverseproxy/domain/manager/manager_realstore_test.go b/management/internals/modules/reverseproxy/domain/manager/manager_realstore_test.go
index 8a0b56171..5c973c40e 100644
--- a/management/internals/modules/reverseproxy/domain/manager/manager_realstore_test.go
+++ b/management/internals/modules/reverseproxy/domain/manager/manager_realstore_test.go
@@ -296,11 +296,8 @@ func TestValidateDomain_PermissionDeniedDoesNotValidate(t *testing.T) {
assert.Error(t, err, "the domain must still be unservable")
}
-// Validation runs asynchronously, so it can finish after the domain was
-// deleted and then write a stale row back. gorm's Save falls back to an insert
-// when an update affects no rows, which would resurrect the domain as
-// validated; UpdateCustomDomain avoids that by selecting explicit columns.
-// This pins that behaviour, since dropping the Select would reintroduce it.
+// A validation finishing after deletion must reject the stale write, without
+// restoring the registration or reporting successful validation.
func TestUpdateCustomDomain_DoesNotResurrectDeletedDomain(t *testing.T) {
ctx := context.Background()
env := setupDomainTest(t)
@@ -315,11 +312,9 @@ func TestUpdateCustomDomain_DoesNotResurrectDeletedDomain(t *testing.T) {
require.Nil(t, storedDomain(t, env.store, accountA, "racy.example.com"), "the domain should be gone")
// What an in-flight validation would write once its CNAME check succeeded.
- // The write has to succeed for the assertion below to mean anything: a
- // rejected write would leave the domain absent for the wrong reason.
stale.Validated = true
_, err = env.store.UpdateCustomDomain(ctx, accountA, stale)
- require.NoError(t, err, "the update itself must succeed, so absence is not just a failed write")
+ require.Error(t, err, "a deleted registration must reject a late validation")
assert.Nil(t, storedDomain(t, env.store, accountA, "racy.example.com"),
"a late validation write must not recreate a deleted domain")
diff --git a/management/internals/modules/reverseproxy/domain/manager/manager_test.go b/management/internals/modules/reverseproxy/domain/manager/manager_test.go
index 519f5efeb..1ca690130 100644
--- a/management/internals/modules/reverseproxy/domain/manager/manager_test.go
+++ b/management/internals/modules/reverseproxy/domain/manager/manager_test.go
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"testing"
+ "time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -208,6 +209,14 @@ func (s *stubStore) DeleteCustomDomain(context.Context, string, string) error {
panic("not used in allow-list tests")
}
+func (s *stubStore) GetExpiredCustomDomains(context.Context, time.Time, domain.ID, int) ([]*domain.Domain, error) {
+ panic("not used in allow-list tests")
+}
+
+func (s *stubStore) DeleteExpiredCustomDomain(context.Context, *domain.Domain, time.Time) (bool, error) {
+ panic("not used in allow-list tests")
+}
+
// TestGetClusterAllowList_DedicatedGatewayAddressExcluded pins invariant (B)'s
// chokepoint: a self-addressed settings pin reserves the account's gateway
// address, so it is dropped from the allow list — which, because the
diff --git a/management/internals/modules/reverseproxy/domain/manager/normalization_test.go b/management/internals/modules/reverseproxy/domain/manager/normalization_test.go
new file mode 100644
index 000000000..da5e4bdd1
--- /dev/null
+++ b/management/internals/modules/reverseproxy/domain/manager/normalization_test.go
@@ -0,0 +1,83 @@
+package manager
+
+import (
+ "context"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/netbirdio/netbird/shared/management/status"
+)
+
+func TestCreateDomain_NormalizesName(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ input string
+ canonical string
+ }{
+ {"mixed case", "Apps.Example.COM", "apps.example.com"},
+ {"unicode", "münchen.example.com", "xn--mnchen-3ya.example.com"},
+ {"trailing dot", "apps.example.com.", "apps.example.com"},
+ {"underscore", "My_App.example.com", "my_app.example.com"},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ ctx := context.Background()
+ env := setupDomainTest(t)
+ env.resolver.set("validation."+tt.canonical, testCluster)
+
+ created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, tt.input, testCluster)
+ require.NoError(t, err)
+ assert.Equal(t, tt.canonical, created.Domain, "the response must use the normalized name")
+ assert.True(t, created.Validated, "the CNAME lookup must use the normalized name")
+ stored, err := env.store.GetCustomDomain(ctx, accountA, created.ID)
+ require.NoError(t, err)
+ assert.Equal(t, tt.canonical, stored.Domain, "the database must retain the normalized name")
+
+ _, err = env.manager.CreateDomain(ctx, accountB, accountBUser, tt.canonical, testCluster)
+ require.Error(t, err)
+ sErr, ok := status.FromError(err)
+ require.True(t, ok, "an equivalent name must return a typed conflict")
+ assert.Equal(t, status.AlreadyExists, sErr.Type(), "normalization must precede the availability check")
+ })
+ }
+}
+
+func TestCreateDomain_NormalizedNameCanValidateLater(t *testing.T) {
+ ctx := context.Background()
+ env := setupDomainTest(t)
+ created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "Apps.Example.COM.", testCluster)
+ require.NoError(t, err)
+ require.False(t, created.Validated, "a missing CNAME must leave the normalized registration pending")
+
+ env.resolver.set("validation.apps.example.com", testCluster)
+ env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID)
+ stored, err := env.store.GetCustomDomain(ctx, accountA, created.ID)
+ require.NoError(t, err)
+ assert.Equal(t, "apps.example.com", stored.Domain, "retrying validation must retain the normalized name")
+ assert.True(t, stored.Validated, "later validation must look up the normalized name")
+}
+
+func TestCreateDomain_RejectsInvalidName(t *testing.T) {
+ ctx := context.Background()
+ env := setupDomainTest(t)
+ for _, name := range []string{
+ "", ".", "app..example.com", "app.example.com..", "-app.example.com",
+ "app%.example.com", "app!.example.com", "*.example.com", "app example.com",
+ "https://example.com", strings.Repeat("a", 64) + ".example.com",
+ } {
+ t.Run(name, func(t *testing.T) {
+ // A matching DNS response must not make a malformed name acceptable.
+ env.resolver.set("validation."+name, testCluster)
+ _, err := env.manager.CreateDomain(ctx, accountA, accountAUser, name, testCluster)
+ require.Error(t, err)
+ sErr, ok := status.FromError(err)
+ require.True(t, ok, "invalid names must return a typed client error")
+ assert.Equal(t, status.InvalidArgument, sErr.Type(), "malformed names must be rejected before storage")
+ })
+ }
+ stored, err := env.store.ListCustomDomains(ctx, accountA)
+ require.NoError(t, err)
+ assert.Empty(t, stored, "invalid registration attempts must not reserve any names")
+}
diff --git a/management/internals/server/server.go b/management/internals/server/server.go
index 9709d1099..a1b58fdf1 100644
--- a/management/internals/server/server.go
+++ b/management/internals/server/server.go
@@ -66,7 +66,8 @@ type BaseServer struct {
disableLegacyManagementPort bool
autoResolveDomains bool
- proxyAuthClose func()
+ proxyAuthClose func()
+ domainCleanupStop func()
// grpcExtensions holds additional gRPC services, interceptors, and shutdown
// hooks registered by external modules via RegisterGRPCExtension. Populated
@@ -227,9 +228,26 @@ func (s *BaseServer) Start(ctx context.Context) error {
s.update.SetOnUpdateListener(func() {
log.WithContext(ctx).Infof("your management version, \"%s\", is outdated, a new management version is available. Learn more here: https://github.com/netbirdio/netbird/releases", version.NetbirdVersion())
})
+ s.startDomainCleanup(srvCtx)
return nil
}
+func (s *BaseServer) startDomainCleanup(ctx context.Context) {
+ if s.domainCleanupStop != nil {
+ return
+ }
+ mgr := s.ReverseProxyDomainManager()
+ ctx, cancel := context.WithCancel(ctx)
+ done := make(chan struct{})
+ s.domainCleanupStop = func() {
+ cancel()
+ <-done
+ }
+ go func() {
+ defer close(done)
+ mgr.RunValidationCleanup(ctx)
+ }()
+}
// setupTLS resolves the listener's TLS source: an injected config wins over the HttpConfig certificate settings
func (s *BaseServer) setupTLS(ctx context.Context) (bool, error) {
@@ -260,6 +278,9 @@ func (s *BaseServer) setupTLS(ctx context.Context) (bool, error) {
func (s *BaseServer) Stop() error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
+ if s.domainCleanupStop != nil {
+ s.domainCleanupStop()
+ }
s.IntegratedValidator().Stop(ctx)
if s.GeoLocationManager() != nil {
diff --git a/management/internals/shared/grpc/peer_update_handler.go b/management/internals/shared/grpc/peer_update_handler.go
new file mode 100644
index 000000000..d3ff9b6af
--- /dev/null
+++ b/management/internals/shared/grpc/peer_update_handler.go
@@ -0,0 +1,136 @@
+package grpc
+
+import (
+ "context"
+ "time"
+
+ "github.com/netbirdio/netbird/encryption"
+ "github.com/netbirdio/netbird/management/internals/controllers/network_map"
+ "github.com/netbirdio/netbird/management/server/telemetry"
+ "github.com/netbirdio/netbird/shared/management/proto"
+ log "github.com/sirupsen/logrus"
+ "golang.zx2c4.com/wireguard/wgctrl/wgtypes"
+ "google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
+)
+
+func PeerUpdateHandlerFactory(
+ peerKey wgtypes.Key,
+ updates chan *network_map.UpdateMessage,
+ secretsManager SecretsManager,
+ srv proto.ManagementService_SyncServer,
+ cleanupfunc func()) *PeerUpdateHandler {
+ return &PeerUpdateHandler{
+ peerKey: peerKey,
+ updates: updates,
+ secretsManager: secretsManager,
+ srv: srv,
+ encrypter: encryption.DefaultEncrypter{},
+ debouncer: NewUpdateDebouncer(1000 * time.Millisecond),
+ cleanupFunc: cleanupfunc,
+ }
+}
+
+// PeerUpdateHandler sends updates to the connected peer until the updates channel is closed.
+// It implements a backpressure mechanism that sends the first update immediately,
+// then debounces subsequent rapid updates, ensuring only the latest update is sent
+// after a quiet period.
+type PeerUpdateHandler struct {
+ peerKey wgtypes.Key
+ updates chan *network_map.UpdateMessage
+ appMetrics telemetry.AppMetrics
+ secretsManager SecretsManager
+ srv syncSender
+ encrypter encryption.Encrypter
+ debouncer Debouncer
+ cleanupFunc func()
+}
+
+func (pu *PeerUpdateHandler) WithMetrics(appMetrics telemetry.AppMetrics) *PeerUpdateHandler {
+ pu.appMetrics = appMetrics
+ return pu
+}
+
+//go:generate go tool mockgen -source=./peer_update_handler.go -destination=./sync_sender_mock.go -package=grpc
+type syncSender interface {
+ Send(*proto.EncryptedMessage) error
+ Context() context.Context
+}
+
+func (pu *PeerUpdateHandler) HandleUpdates(ctx context.Context) error {
+ log.WithContext(ctx).Tracef("starting to handle updates for peer %s", pu.peerKey.String())
+
+ defer pu.debouncer.Stop()
+
+ for {
+ select {
+ // condition when there are some updates
+ // todo set the updates channel size to 1
+ case update, open := <-pu.updates:
+ if pu.appMetrics != nil {
+ pu.appMetrics.GRPCMetrics().UpdateChannelQueueLength(len(pu.updates) + 1)
+ }
+
+ if !open {
+ log.WithContext(ctx).Debugf("updates channel for peer %s was closed", pu.peerKey.String())
+ pu.cleanupFunc()
+ return nil
+ }
+
+ log.WithContext(ctx).Tracef("received an update for peer %s", pu.peerKey.String())
+ if pu.debouncer.ProcessUpdate(update) {
+ // Send immediately (first update or after quiet period)
+ if err := pu.SendUpdate(ctx, update); err != nil {
+ log.WithContext(ctx).Debugf("error while sending an update to peer %s: %v", pu.peerKey.String(), err)
+ return err
+ }
+ }
+
+ // Timer expired - quiet period reached, send pending updates if any
+ case <-pu.debouncer.TimerChannel():
+ pendingUpdates := pu.debouncer.GetPendingUpdates()
+ if len(pendingUpdates) == 0 {
+ continue
+ }
+ log.WithContext(ctx).Debugf("sending %d debounced update(s) for peer %s", len(pendingUpdates), pu.peerKey.String())
+ for _, pendingUpdate := range pendingUpdates {
+ if err := pu.SendUpdate(ctx, pendingUpdate); err != nil {
+ log.WithContext(ctx).Debugf("error while sending an update to peer %s: %v", pu.peerKey.String(), err)
+ return err
+ }
+ }
+
+ // condition when client <-> server connection has been terminated
+ case <-pu.srv.Context().Done():
+ // happens when connection drops, e.g. client disconnects
+ log.WithContext(ctx).Debugf("stream of peer %s has been closed", pu.peerKey.String())
+ pu.cleanupFunc()
+ return pu.srv.Context().Err()
+ }
+ }
+}
+
+func (pu *PeerUpdateHandler) SendUpdate(ctx context.Context, update *network_map.UpdateMessage) error {
+ key, err := pu.secretsManager.GetWGKey()
+ if err != nil {
+ pu.cleanupFunc()
+ return status.Errorf(codes.Internal, "failed processing update message")
+ }
+
+ stampCertificateChallenges(update.Update.GetChecks(), pu.peerKey, key)
+ encryptedResp, err := pu.encrypter.EncryptMessage(pu.peerKey, key, update.Update)
+ if err != nil {
+ pu.cleanupFunc()
+ return status.Errorf(codes.Internal, "failed processing update message")
+ }
+ err = pu.srv.Send(&proto.EncryptedMessage{
+ WgPubKey: key.PublicKey().String(),
+ Body: encryptedResp,
+ })
+ if err != nil {
+ pu.cleanupFunc()
+ return status.Errorf(codes.Internal, "failed sending update message")
+ }
+ log.WithContext(ctx).Tracef("sent an update to peer %s", pu.peerKey.String())
+ return nil
+}
diff --git a/management/internals/shared/grpc/peer_update_handler_test.go b/management/internals/shared/grpc/peer_update_handler_test.go
new file mode 100644
index 000000000..02de49c47
--- /dev/null
+++ b/management/internals/shared/grpc/peer_update_handler_test.go
@@ -0,0 +1,155 @@
+package grpc
+
+import (
+ "context"
+ "fmt"
+ "sync"
+ "testing"
+ "time"
+
+ pb "github.com/golang/protobuf/proto" //nolint
+ "github.com/netbirdio/netbird/management/internals/controllers/network_map"
+ "github.com/netbirdio/netbird/shared/management/proto"
+ "github.com/stretchr/testify/assert"
+ "go.uber.org/mock/gomock"
+ "golang.zx2c4.com/wireguard/wgctrl/wgtypes"
+)
+
+func TestSendPeerUpdates_FirstUpdate(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ secretsManager := NewMockSecretsManager(ctrl)
+ updateDebouncer := NewMockDebouncer(ctrl)
+ syncSender := NewMocksyncSender(ctrl)
+
+ pu := PeerUpdateHandler{
+ peerKey: mustGenerateKey(t),
+ updates: make(chan *network_map.UpdateMessage),
+ secretsManager: secretsManager,
+ encrypter: testEncrypter{},
+ debouncer: updateDebouncer,
+ srv: syncSender,
+ cleanupFunc: func() {},
+ }
+
+ msg := network_map.UpdateMessage{
+ Update: &proto.SyncResponse{Version: 1},
+ }
+
+ timeCh := make(chan time.Time)
+ srvCtx := context.TODO()
+ srvKey := mustGenerateKey(t)
+ // mock a first update, should send it right away
+ updateDebouncer.EXPECT().ProcessUpdate(gomock.Eq(&msg)).Return(true)
+ updateDebouncer.EXPECT().TimerChannel().AnyTimes().Return(timeCh)
+ syncSender.EXPECT().Context().AnyTimes().Return(srvCtx)
+ secretsManager.EXPECT().GetWGKey().Return(srvKey, nil)
+ syncSender.EXPECT().Send(pbMatcher{x: &proto.EncryptedMessage{WgPubKey: srvKey.PublicKey().String(), Body: mustMarshal(t, &msg)}})
+ updateDebouncer.EXPECT().Stop()
+
+ var wg sync.WaitGroup
+ wg.Go(func() { pu.HandleUpdates(context.TODO()) }) //nolint:errcheck
+ pu.updates <- &msg
+ close(pu.updates)
+ wg.Wait()
+}
+
+func TestSendPeerUpdates_TimerUpdate(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ secretsManager := NewMockSecretsManager(ctrl)
+ updateDebouncer := NewMockDebouncer(ctrl)
+ syncSender := NewMocksyncSender(ctrl)
+
+ pu := PeerUpdateHandler{
+ peerKey: mustGenerateKey(t),
+ updates: make(chan *network_map.UpdateMessage),
+ secretsManager: secretsManager,
+ encrypter: testEncrypter{},
+ debouncer: updateDebouncer,
+ srv: syncSender,
+ cleanupFunc: func() {},
+ }
+
+ msg := network_map.UpdateMessage{
+ Update: &proto.SyncResponse{Version: 1},
+ }
+
+ timeCh := make(chan time.Time)
+ srvCtx := context.TODO()
+ srvKey := mustGenerateKey(t)
+ updateDebouncer.EXPECT().GetPendingUpdates().Return([]*network_map.UpdateMessage{&msg})
+ updateDebouncer.EXPECT().TimerChannel().AnyTimes().Return(timeCh)
+ syncSender.EXPECT().Context().AnyTimes().Return(srvCtx)
+ secretsManager.EXPECT().GetWGKey().Return(srvKey, nil)
+ syncSender.EXPECT().Send(pbMatcher{x: &proto.EncryptedMessage{WgPubKey: srvKey.PublicKey().String(), Body: mustMarshal(t, &msg)}})
+ updateDebouncer.EXPECT().Stop()
+
+ var wg sync.WaitGroup
+ wg.Go(func() { pu.HandleUpdates(context.TODO()) }) //nolint:errcheck
+ timeCh <- time.Now()
+ close(pu.updates)
+ wg.Wait()
+}
+
+func TestSendPeerUpdates_ServerContextDone(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ secretsManager := NewMockSecretsManager(ctrl)
+ updateDebouncer := NewMockDebouncer(ctrl)
+ syncSender := NewMocksyncSender(ctrl)
+
+ pu := PeerUpdateHandler{
+ peerKey: mustGenerateKey(t),
+ updates: make(chan *network_map.UpdateMessage),
+ secretsManager: secretsManager,
+ encrypter: testEncrypter{},
+ debouncer: updateDebouncer,
+ srv: syncSender,
+ cleanupFunc: func() {},
+ }
+
+ timeCh := make(chan time.Time)
+ srvCtx, cancel := context.WithCancel(context.TODO())
+ updateDebouncer.EXPECT().TimerChannel().AnyTimes().Return(timeCh)
+ syncSender.EXPECT().Context().AnyTimes().Return(srvCtx)
+ updateDebouncer.EXPECT().Stop()
+
+ var wg sync.WaitGroup
+ wg.Go(func() { pu.HandleUpdates(context.TODO()) }) //nolint:errcheck
+ cancel()
+ wg.Wait()
+}
+
+func mustGenerateKey(t *testing.T) wgtypes.Key {
+ t.Helper()
+ k, err := wgtypes.GenerateKey()
+ assert.NoError(t, err)
+ return k
+}
+
+func mustMarshal(t *testing.T, msg *network_map.UpdateMessage) []byte {
+ t.Helper()
+ r, err := pb.Marshal(msg.Update)
+ assert.NoError(t, err)
+ return r
+}
+
+type testEncrypter struct{}
+
+func (testEncrypter) EncryptMessage(remotePubKey wgtypes.Key, ourPrivateKey wgtypes.Key, message pb.Message) ([]byte, error) {
+ return pb.Marshal(message)
+}
+
+type pbMatcher struct {
+ x pb.Message
+}
+
+func (pbm pbMatcher) Matches(x any) bool {
+ msg, ok := x.(pb.Message)
+ if !ok {
+ return false
+ }
+ return pb.Equal(pbm.x, msg)
+}
+
+func (pbm pbMatcher) String() string {
+ return fmt.Sprintf("is equal to %s (%T)", pbm.x, pbm.x)
+}
diff --git a/management/internals/shared/grpc/server.go b/management/internals/shared/grpc/server.go
index bebbb9ad3..82a765481 100644
--- a/management/internals/shared/grpc/server.go
+++ b/management/internals/shared/grpc/server.go
@@ -338,7 +338,8 @@ func (s *Server) Sync(req *proto.EncryptedMessage, srv proto.ManagementService_S
s.syncSem.Add(-1)
- return s.handleUpdates(ctx, accountID, peerKey, peer, updates, srv, syncStart)
+ return PeerUpdateHandlerFactory(peerKey, updates, s.secretsManager, srv, func() { s.cancelPeerRoutines(ctx, accountID, peer, syncStart) }).
+ WithMetrics(s.appMetrics).HandleUpdates(ctx)
}
func (s *Server) handleHandshake(ctx context.Context, srv proto.ManagementService_JobServer) (wgtypes.Key, error) {
@@ -405,92 +406,6 @@ func (s *Server) sendJobsLoop(ctx context.Context, accountID string, peerKey wgt
}
}
-// handleUpdates sends updates to the connected peer until the updates channel is closed.
-// It implements a backpressure mechanism that sends the first update immediately,
-// then debounces subsequent rapid updates, ensuring only the latest update is sent
-// after a quiet period.
-func (s *Server) handleUpdates(ctx context.Context, accountID string, peerKey wgtypes.Key, peer *nbpeer.Peer, updates chan *network_map.UpdateMessage, srv proto.ManagementService_SyncServer, streamStartTime time.Time) error {
- log.WithContext(ctx).Tracef("starting to handle updates for peer %s", peerKey.String())
-
- // Create a debouncer for this peer connection
- debouncer := NewUpdateDebouncer(1000 * time.Millisecond)
- defer debouncer.Stop()
-
- for {
- select {
- // condition when there are some updates
- // todo set the updates channel size to 1
- case update, open := <-updates:
- if s.appMetrics != nil {
- s.appMetrics.GRPCMetrics().UpdateChannelQueueLength(len(updates) + 1)
- }
-
- if !open {
- log.WithContext(ctx).Debugf("updates channel for peer %s was closed", peerKey.String())
- s.cancelPeerRoutines(ctx, accountID, peer, streamStartTime)
- return nil
- }
-
- log.WithContext(ctx).Tracef("received an update for peer %s", peerKey.String())
- if debouncer.ProcessUpdate(update) {
- // Send immediately (first update or after quiet period)
- if err := s.sendUpdate(ctx, accountID, peerKey, peer, update, srv, streamStartTime); err != nil {
- log.WithContext(ctx).Debugf("error while sending an update to peer %s: %v", peerKey.String(), err)
- return err
- }
- }
-
- // Timer expired - quiet period reached, send pending updates if any
- case <-debouncer.TimerChannel():
- pendingUpdates := debouncer.GetPendingUpdates()
- if len(pendingUpdates) == 0 {
- continue
- }
- log.WithContext(ctx).Debugf("sending %d debounced update(s) for peer %s", len(pendingUpdates), peerKey.String())
- for _, pendingUpdate := range pendingUpdates {
- if err := s.sendUpdate(ctx, accountID, peerKey, peer, pendingUpdate, srv, streamStartTime); err != nil {
- log.WithContext(ctx).Debugf("error while sending an update to peer %s: %v", peerKey.String(), err)
- return err
- }
- }
-
- // condition when client <-> server connection has been terminated
- case <-srv.Context().Done():
- // happens when connection drops, e.g. client disconnects
- log.WithContext(ctx).Debugf("stream of peer %s has been closed", peerKey.String())
- s.cancelPeerRoutines(ctx, accountID, peer, streamStartTime)
- return srv.Context().Err()
- }
- }
-}
-
-// sendUpdate encrypts the update message using the peer key and the server's wireguard key,
-// then sends the encrypted message to the connected peer via the sync server.
-func (s *Server) sendUpdate(ctx context.Context, accountID string, peerKey wgtypes.Key, peer *nbpeer.Peer, update *network_map.UpdateMessage, srv proto.ManagementService_SyncServer, streamStartTime time.Time) error {
- key, err := s.secretsManager.GetWGKey()
- if err != nil {
- s.cancelPeerRoutines(ctx, accountID, peer, streamStartTime)
- return status.Errorf(codes.Internal, "failed processing update message")
- }
-
- stampCertificateChallenges(update.Update.GetChecks(), peerKey, key)
- encryptedResp, err := encryption.EncryptMessage(peerKey, key, update.Update)
- if err != nil {
- s.cancelPeerRoutines(ctx, accountID, peer, streamStartTime)
- return status.Errorf(codes.Internal, "failed processing update message")
- }
- err = srv.Send(&proto.EncryptedMessage{
- WgPubKey: key.PublicKey().String(),
- Body: encryptedResp,
- })
- if err != nil {
- s.cancelPeerRoutines(ctx, accountID, peer, streamStartTime)
- return status.Errorf(codes.Internal, "failed sending update message")
- }
- log.WithContext(ctx).Tracef("sent an update to peer %s", peerKey.String())
- return nil
-}
-
// sendJob encrypts the update message using the peer key and the server's wireguard key,
// then sends the encrypted message to the connected peer via the sync server.
func (s *Server) sendJob(ctx context.Context, peerKey wgtypes.Key, job *job.Event, srv proto.ManagementService_JobServer) error {
diff --git a/management/internals/shared/grpc/sync_sender_mock.go b/management/internals/shared/grpc/sync_sender_mock.go
new file mode 100644
index 000000000..3d1696f59
--- /dev/null
+++ b/management/internals/shared/grpc/sync_sender_mock.go
@@ -0,0 +1,70 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: ./peer_update_handler.go
+//
+// Generated by this command:
+//
+// mockgen -source=./peer_update_handler.go -destination=./sync_sender_mock.go -package=grpc
+//
+
+// Package grpc is a generated GoMock package.
+package grpc
+
+import (
+ context "context"
+ reflect "reflect"
+
+ proto "github.com/netbirdio/netbird/shared/management/proto"
+ gomock "go.uber.org/mock/gomock"
+)
+
+// MocksyncSender is a mock of syncSender interface.
+type MocksyncSender struct {
+ ctrl *gomock.Controller
+ recorder *MocksyncSenderMockRecorder
+ isgomock struct{}
+}
+
+// MocksyncSenderMockRecorder is the mock recorder for MocksyncSender.
+type MocksyncSenderMockRecorder struct {
+ mock *MocksyncSender
+}
+
+// NewMocksyncSender creates a new mock instance.
+func NewMocksyncSender(ctrl *gomock.Controller) *MocksyncSender {
+ mock := &MocksyncSender{ctrl: ctrl}
+ mock.recorder = &MocksyncSenderMockRecorder{mock}
+ return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MocksyncSender) EXPECT() *MocksyncSenderMockRecorder {
+ return m.recorder
+}
+
+// Context mocks base method.
+func (m *MocksyncSender) Context() context.Context {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "Context")
+ ret0, _ := ret[0].(context.Context)
+ return ret0
+}
+
+// Context indicates an expected call of Context.
+func (mr *MocksyncSenderMockRecorder) Context() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Context", reflect.TypeOf((*MocksyncSender)(nil).Context))
+}
+
+// Send mocks base method.
+func (m *MocksyncSender) Send(arg0 *proto.EncryptedMessage) error {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "Send", arg0)
+ ret0, _ := ret[0].(error)
+ return ret0
+}
+
+// Send indicates an expected call of Send.
+func (mr *MocksyncSenderMockRecorder) Send(arg0 any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Send", reflect.TypeOf((*MocksyncSender)(nil).Send), arg0)
+}
diff --git a/management/internals/shared/grpc/token_mgr.go b/management/internals/shared/grpc/token_mgr.go
index fb2d83a9a..4dab5007f 100644
--- a/management/internals/shared/grpc/token_mgr.go
+++ b/management/internals/shared/grpc/token_mgr.go
@@ -25,6 +25,8 @@ import (
const defaultDuration = 12 * time.Hour
// SecretsManager used to manage TURN and relay secrets
+//
+//go:generate go tool mockgen -source=./token_mgr.go -destination=./token_mgr_mock.go -package=grpc
type SecretsManager interface {
GenerateTurnToken() (*Token, error)
GenerateRelayToken() (*Token, error)
diff --git a/management/internals/shared/grpc/token_mgr_mock.go b/management/internals/shared/grpc/token_mgr_mock.go
new file mode 100644
index 000000000..e7dde4e3a
--- /dev/null
+++ b/management/internals/shared/grpc/token_mgr_mock.go
@@ -0,0 +1,111 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: ./token_mgr.go
+//
+// Generated by this command:
+//
+// mockgen -source=./token_mgr.go -destination=./token_mgr_mock.go -package=grpc
+//
+
+// Package grpc is a generated GoMock package.
+package grpc
+
+import (
+ context "context"
+ reflect "reflect"
+
+ gomock "go.uber.org/mock/gomock"
+ wgtypes "golang.zx2c4.com/wireguard/wgctrl/wgtypes"
+)
+
+// MockSecretsManager is a mock of SecretsManager interface.
+type MockSecretsManager struct {
+ ctrl *gomock.Controller
+ recorder *MockSecretsManagerMockRecorder
+ isgomock struct{}
+}
+
+// MockSecretsManagerMockRecorder is the mock recorder for MockSecretsManager.
+type MockSecretsManagerMockRecorder struct {
+ mock *MockSecretsManager
+}
+
+// NewMockSecretsManager creates a new mock instance.
+func NewMockSecretsManager(ctrl *gomock.Controller) *MockSecretsManager {
+ mock := &MockSecretsManager{ctrl: ctrl}
+ mock.recorder = &MockSecretsManagerMockRecorder{mock}
+ return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockSecretsManager) EXPECT() *MockSecretsManagerMockRecorder {
+ return m.recorder
+}
+
+// CancelRefresh mocks base method.
+func (m *MockSecretsManager) CancelRefresh(peerKey string) {
+ m.ctrl.T.Helper()
+ m.ctrl.Call(m, "CancelRefresh", peerKey)
+}
+
+// CancelRefresh indicates an expected call of CancelRefresh.
+func (mr *MockSecretsManagerMockRecorder) CancelRefresh(peerKey any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CancelRefresh", reflect.TypeOf((*MockSecretsManager)(nil).CancelRefresh), peerKey)
+}
+
+// GenerateRelayToken mocks base method.
+func (m *MockSecretsManager) GenerateRelayToken() (*Token, error) {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GenerateRelayToken")
+ ret0, _ := ret[0].(*Token)
+ ret1, _ := ret[1].(error)
+ return ret0, ret1
+}
+
+// GenerateRelayToken indicates an expected call of GenerateRelayToken.
+func (mr *MockSecretsManagerMockRecorder) GenerateRelayToken() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateRelayToken", reflect.TypeOf((*MockSecretsManager)(nil).GenerateRelayToken))
+}
+
+// GenerateTurnToken mocks base method.
+func (m *MockSecretsManager) GenerateTurnToken() (*Token, error) {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GenerateTurnToken")
+ ret0, _ := ret[0].(*Token)
+ ret1, _ := ret[1].(error)
+ return ret0, ret1
+}
+
+// GenerateTurnToken indicates an expected call of GenerateTurnToken.
+func (mr *MockSecretsManagerMockRecorder) GenerateTurnToken() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateTurnToken", reflect.TypeOf((*MockSecretsManager)(nil).GenerateTurnToken))
+}
+
+// GetWGKey mocks base method.
+func (m *MockSecretsManager) GetWGKey() (wgtypes.Key, error) {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GetWGKey")
+ ret0, _ := ret[0].(wgtypes.Key)
+ ret1, _ := ret[1].(error)
+ return ret0, ret1
+}
+
+// GetWGKey indicates an expected call of GetWGKey.
+func (mr *MockSecretsManagerMockRecorder) GetWGKey() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWGKey", reflect.TypeOf((*MockSecretsManager)(nil).GetWGKey))
+}
+
+// SetupRefresh mocks base method.
+func (m *MockSecretsManager) SetupRefresh(ctx context.Context, accountID, peerKey string) {
+ m.ctrl.T.Helper()
+ m.ctrl.Call(m, "SetupRefresh", ctx, accountID, peerKey)
+}
+
+// SetupRefresh indicates an expected call of SetupRefresh.
+func (mr *MockSecretsManagerMockRecorder) SetupRefresh(ctx, accountID, peerKey any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetupRefresh", reflect.TypeOf((*MockSecretsManager)(nil).SetupRefresh), ctx, accountID, peerKey)
+}
diff --git a/management/internals/shared/grpc/update_debouncer.go b/management/internals/shared/grpc/update_debouncer.go
index 8af9c2656..9483007c6 100644
--- a/management/internals/shared/grpc/update_debouncer.go
+++ b/management/internals/shared/grpc/update_debouncer.go
@@ -6,6 +6,14 @@ import (
"github.com/netbirdio/netbird/management/internals/controllers/network_map"
)
+//go:generate go tool mockgen -source=./update_debouncer.go -destination=./update_debouncer_mock.go -package=grpc
+type Debouncer interface {
+ Stop()
+ TimerChannel() <-chan time.Time
+ ProcessUpdate(update *network_map.UpdateMessage) bool
+ GetPendingUpdates() []*network_map.UpdateMessage
+}
+
// UpdateDebouncer implements a backpressure mechanism that:
// - Sends the first update immediately
// - Coalesces rapid subsequent network map updates (only latest matters)
diff --git a/management/internals/shared/grpc/update_debouncer_mock.go b/management/internals/shared/grpc/update_debouncer_mock.go
new file mode 100644
index 000000000..4dff632b7
--- /dev/null
+++ b/management/internals/shared/grpc/update_debouncer_mock.go
@@ -0,0 +1,96 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: ./update_debouncer.go
+//
+// Generated by this command:
+//
+// mockgen -source=./update_debouncer.go -destination=./update_debouncer_mock.go -package=grpc
+//
+
+// Package grpc is a generated GoMock package.
+package grpc
+
+import (
+ reflect "reflect"
+ time "time"
+
+ network_map "github.com/netbirdio/netbird/management/internals/controllers/network_map"
+ gomock "go.uber.org/mock/gomock"
+)
+
+// MockDebouncer is a mock of Debouncer interface.
+type MockDebouncer struct {
+ ctrl *gomock.Controller
+ recorder *MockDebouncerMockRecorder
+ isgomock struct{}
+}
+
+// MockDebouncerMockRecorder is the mock recorder for MockDebouncer.
+type MockDebouncerMockRecorder struct {
+ mock *MockDebouncer
+}
+
+// NewMockDebouncer creates a new mock instance.
+func NewMockDebouncer(ctrl *gomock.Controller) *MockDebouncer {
+ mock := &MockDebouncer{ctrl: ctrl}
+ mock.recorder = &MockDebouncerMockRecorder{mock}
+ return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockDebouncer) EXPECT() *MockDebouncerMockRecorder {
+ return m.recorder
+}
+
+// GetPendingUpdates mocks base method.
+func (m *MockDebouncer) GetPendingUpdates() []*network_map.UpdateMessage {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GetPendingUpdates")
+ ret0, _ := ret[0].([]*network_map.UpdateMessage)
+ return ret0
+}
+
+// GetPendingUpdates indicates an expected call of GetPendingUpdates.
+func (mr *MockDebouncerMockRecorder) GetPendingUpdates() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPendingUpdates", reflect.TypeOf((*MockDebouncer)(nil).GetPendingUpdates))
+}
+
+// ProcessUpdate mocks base method.
+func (m *MockDebouncer) ProcessUpdate(update *network_map.UpdateMessage) bool {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "ProcessUpdate", update)
+ ret0, _ := ret[0].(bool)
+ return ret0
+}
+
+// ProcessUpdate indicates an expected call of ProcessUpdate.
+func (mr *MockDebouncerMockRecorder) ProcessUpdate(update any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProcessUpdate", reflect.TypeOf((*MockDebouncer)(nil).ProcessUpdate), update)
+}
+
+// Stop mocks base method.
+func (m *MockDebouncer) Stop() {
+ m.ctrl.T.Helper()
+ m.ctrl.Call(m, "Stop")
+}
+
+// Stop indicates an expected call of Stop.
+func (mr *MockDebouncerMockRecorder) Stop() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Stop", reflect.TypeOf((*MockDebouncer)(nil).Stop))
+}
+
+// TimerChannel mocks base method.
+func (m *MockDebouncer) TimerChannel() <-chan time.Time {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "TimerChannel")
+ ret0, _ := ret[0].(<-chan time.Time)
+ return ret0
+}
+
+// TimerChannel indicates an expected call of TimerChannel.
+func (mr *MockDebouncerMockRecorder) TimerChannel() *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TimerChannel", reflect.TypeOf((*MockDebouncer)(nil).TimerChannel))
+}
diff --git a/management/server/activity/codes.go b/management/server/activity/codes.go
index 4d50fee3e..dca043027 100644
--- a/management/server/activity/codes.go
+++ b/management/server/activity/codes.go
@@ -284,6 +284,9 @@ const (
// AgentNetworkSettingsDeleted indicates that a user deleted the Agent Network account settings, releasing the endpoint
AgentNetworkSettingsDeleted Activity = 142
+ // CustomDomainValidationExpired indicates that an unvalidated domain registration expired.
+ CustomDomainValidationExpired Activity = 143
+
AccountDeleted Activity = 99999
)
@@ -461,9 +464,10 @@ var activityMap = map[Activity]Code{
AccountMetricsPushEnabled: {"Account metrics push enabled", "account.setting.metrics.push.enable"},
AccountMetricsPushDisabled: {"Account metrics push disabled", "account.setting.metrics.push.disable"},
- DomainAdded: {"Domain added", "domain.add"},
- DomainDeleted: {"Domain deleted", "domain.delete"},
- DomainValidated: {"Domain validated", "domain.validate"},
+ DomainAdded: {"Domain added", "domain.add"},
+ DomainDeleted: {"Domain deleted", "domain.delete"},
+ DomainValidated: {"Domain validated", "domain.validate"},
+ CustomDomainValidationExpired: {"Unvalidated domain registration expired", "domain.validation.expire"},
}
// StringCode returns a string code of the activity
diff --git a/management/server/activity/store/sql_store.go b/management/server/activity/store/sql_store.go
index 73e8e295c..5b0f23dd0 100644
--- a/management/server/activity/store/sql_store.go
+++ b/management/server/activity/store/sql_store.go
@@ -165,16 +165,16 @@ func (store *Store) Get(ctx context.Context, accountID string, offset, limit int
return store.processResult(ctx, events)
}
-// Save an event in the SQLite events table end encrypt the "email" element in meta map
-func (store *Store) Save(_ context.Context, event *activity.Event) (*activity.Event, error) {
+// Save persists an activity event and encrypts deleted user details using the caller's context.
+func (store *Store) Save(ctx context.Context, event *activity.Event) (*activity.Event, error) {
eventCopy := event.Copy()
- meta, err := store.saveDeletedUserEmailAndNameInEncrypted(eventCopy)
+ meta, err := store.saveDeletedUserEmailAndNameInEncrypted(ctx, eventCopy)
if err != nil {
return nil, err
}
eventCopy.Meta = meta
- if err = store.db.Create(eventCopy).Error; err != nil {
+ if err = store.db.WithContext(ctx).Create(eventCopy).Error; err != nil {
return nil, err
}
@@ -183,7 +183,7 @@ func (store *Store) Save(_ context.Context, event *activity.Event) (*activity.Ev
// saveDeletedUserEmailAndNameInEncrypted if the meta contains email and name then store it in encrypted way and delete
// this item from meta map
-func (store *Store) saveDeletedUserEmailAndNameInEncrypted(event *activity.Event) (map[string]any, error) {
+func (store *Store) saveDeletedUserEmailAndNameInEncrypted(ctx context.Context, event *activity.Event) (map[string]any, error) {
email, ok := event.Meta["email"]
if !ok {
return event.Meta, nil
@@ -211,7 +211,7 @@ func (store *Store) saveDeletedUserEmailAndNameInEncrypted(event *activity.Event
}
deletedUser.Name = encryptedName
- err = store.db.Clauses(clause.OnConflict{
+ err = store.db.WithContext(ctx).Clauses(clause.OnConflict{
Columns: []clause.Column{{Name: "id"}},
DoUpdates: clause.AssignmentColumns([]string{"email", "name"}),
}).Create(deletedUser).Error
diff --git a/management/server/activity/store/sql_store_test.go b/management/server/activity/store/sql_store_test.go
index d723f1623..0a68858e4 100644
--- a/management/server/activity/store/sql_store_test.go
+++ b/management/server/activity/store/sql_store_test.go
@@ -7,11 +7,49 @@ import (
"time"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/util/crypt"
)
+func TestSave_CancellationWhileWaitingForConnection(t *testing.T) {
+ t.Setenv(storeEngineEnv, "sqlite")
+ key, err := crypt.GenerateKey()
+ require.NoError(t, err)
+ store, err := NewSqlStore(context.Background(), t.TempDir(), key)
+ require.NoError(t, err)
+ t.Cleanup(func() { assert.NoError(t, store.Close(context.Background())) })
+ db, err := store.db.DB()
+ require.NoError(t, err)
+ conn, err := db.Conn(context.Background())
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = conn.Close() })
+ ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+ defer cancel()
+ result := make(chan error, 1)
+ go func() {
+ _, err := store.Save(ctx, &activity.Event{
+ Timestamp: time.Now().UTC(), Activity: activity.CustomDomainValidationExpired,
+ AccountID: "account-id", TargetID: "domain-id", InitiatorID: activity.SystemInitiator,
+ })
+ result <- err
+ }()
+ select {
+ case err := <-result:
+ assert.ErrorIs(t, err, context.DeadlineExceeded)
+ require.NoError(t, conn.Close())
+ case <-time.After(time.Second):
+ // Release the connection so a regression cannot leave the writer running.
+ require.NoError(t, conn.Close())
+ assert.ErrorIs(t, <-result, context.DeadlineExceeded)
+ t.Error("activity writes must stop waiting when their deadline expires")
+ }
+ events, err := store.Get(context.Background(), "account-id", 0, 10, true)
+ require.NoError(t, err)
+ assert.Empty(t, events, "a timed-out write must not persist after the connection is released")
+}
+
func TestNewSqlStore(t *testing.T) {
dataDir := t.TempDir()
key, _ := crypt.GenerateKey()
diff --git a/management/server/event.go b/management/server/event.go
index 4211f2dda..2b1ac4987 100644
--- a/management/server/event.go
+++ b/management/server/event.go
@@ -61,23 +61,34 @@ func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userI
return filtered, nil
}
+// StoreEvent records an activity, waiting for expiration events before cleanup can stop.
func (am *DefaultAccountManager) StoreEvent(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) {
- if isEnabled() {
- go func() {
- _, err := am.eventStore.Save(ctx, &activity.Event{
- Timestamp: time.Now().UTC(),
- Activity: activityID.(activity.Activity),
- InitiatorID: initiatorID,
- TargetID: targetID,
- AccountID: accountID,
- Meta: meta,
- })
- if err != nil {
- // todo add metric
- log.WithContext(ctx).Errorf("received an error while storing an activity event, error: %s", err)
- }
- }()
+ if !isEnabled() {
+ return
}
+ eventStore := am.eventStore
+ save := func(ctx context.Context) {
+ _, err := eventStore.Save(ctx, &activity.Event{
+ Timestamp: time.Now().UTC(),
+ Activity: activityID.(activity.Activity),
+ InitiatorID: initiatorID,
+ TargetID: targetID,
+ AccountID: accountID,
+ Meta: meta,
+ })
+ if err != nil {
+ log.WithContext(ctx).Errorf("received an error while storing an activity event, error: %s", err)
+ }
+ }
+ if activityID == activity.CustomDomainValidationExpired {
+ // The domain is already deleted; shutdown must allow its audit write to finish.
+ ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
+ defer cancel()
+ save(ctx)
+ return
+ }
+ // Request cancellation must not discard the audit record of a completed operation.
+ go save(context.WithoutCancel(ctx))
}
type eventUserInfo struct {
diff --git a/management/server/event_test.go b/management/server/event_test.go
index 420e69866..6fb3f437f 100644
--- a/management/server/event_test.go
+++ b/management/server/event_test.go
@@ -6,10 +6,52 @@ import (
"time"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/activity"
+ activitystore "github.com/netbirdio/netbird/management/server/activity/store"
+ "github.com/netbirdio/netbird/util/crypt"
)
+func TestStoreEvent_CanceledContext(t *testing.T) {
+ t.Setenv("NB_EVENT_ACTIVITY_LOG_ENABLED", "true")
+ t.Setenv("NB_ACTIVITY_EVENT_STORE_ENGINE", "sqlite")
+ for _, code := range []activity.Activity{activity.CustomDomainValidationExpired, activity.DomainAdded} {
+ t.Run(code.StringCode(), func(t *testing.T) {
+ dir := t.TempDir()
+ key, err := crypt.GenerateKey()
+ require.NoError(t, err)
+ eventStore, err := activitystore.NewSqlStore(context.Background(), dir, key)
+ require.NoError(t, err)
+ t.Cleanup(func() { assert.NoError(t, eventStore.Close(context.Background())) })
+ manager := &DefaultAccountManager{eventStore: eventStore}
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+
+ // The operation already succeeded when shutdown or the request cancels its context.
+ manager.StoreEvent(ctx, activity.SystemInitiator, "domain-id", "account-id",
+ code, map[string]any{"domain": "expired.example.com"})
+ if code != activity.CustomDomainValidationExpired {
+ require.Eventually(t, func() bool {
+ events, err := eventStore.Get(context.Background(), "account-id", 0, 10, true)
+ return err == nil && len(events) == 1
+ }, time.Second, time.Millisecond, "asynchronous events must survive request cancellation")
+ }
+ require.NoError(t, eventStore.Close(context.Background()))
+
+ reopened, err := activitystore.NewSqlStore(context.Background(), dir, key)
+ require.NoError(t, err)
+ t.Cleanup(func() { assert.NoError(t, reopened.Close(context.Background())) })
+ events, err := reopened.Get(context.Background(), "account-id", 0, 10, true)
+ require.NoError(t, err)
+ require.Len(t, events, 1, "the event must be persisted before shutdown closes the store")
+ assert.Equal(t, code, events[0].Activity, "persist the requested activity")
+ assert.Equal(t, "domain-id", events[0].TargetID, "retain the registration ID")
+ assert.Equal(t, "expired.example.com", events[0].Meta["domain"], "retain the domain name")
+ })
+ }
+}
+
func generateAndStoreEvents(t *testing.T, manager *DefaultAccountManager, typ activity.Activity, initiatorID, targetID,
accountID string, count int) {
t.Helper()
diff --git a/management/server/group.go b/management/server/group.go
index ca20a6b08..88295e2f6 100644
--- a/management/server/group.go
+++ b/management/server/group.go
@@ -774,6 +774,14 @@ func validateDeleteGroup(ctx context.Context, transaction store.Store, group *ty
return &GroupLinkError{"agent network policy", linkedPolicy.Name}
}
+ isLinked, linkedRule, err := isGroupLinkedToAgentNetworkBudgetRule(ctx, transaction, group.AccountID, group.ID)
+ if err != nil {
+ return status.Errorf(status.Internal, "failed to check agent network budget rules")
+ }
+ if isLinked {
+ return &GroupLinkError{"agent network budget rule", linkedRule.Name}
+ }
+
return checkGroupLinkedToSettings(ctx, transaction, group)
}
@@ -945,6 +953,26 @@ func isGroupLinkedToAgentNetworkPolicy(ctx context.Context, transaction store.St
return false, nil
}
+// isGroupLinkedToAgentNetworkBudgetRule checks if a group is a target of any
+// account-level agent network budget rule.
+func isGroupLinkedToAgentNetworkBudgetRule(ctx context.Context, transaction store.Store, accountID string, groupID string) (bool, *agentNetworkTypes.AccountBudgetRule, error) {
+ rules, err := transaction.GetAccountAgentNetworkBudgetRules(ctx, store.LockingStrengthNone, accountID)
+ if err != nil {
+ log.WithContext(ctx).Errorf("error retrieving agent network budget rules while checking group linkage: %v", err)
+ return false, nil, err
+ }
+
+ for _, rule := range rules {
+ if rule == nil {
+ continue
+ }
+ if slices.Contains(rule.TargetGroups, groupID) {
+ return true, rule, nil
+ }
+ }
+ return false, nil, nil
+}
+
// areGroupChangesAffectPeers checks if any changes to the specified groups will affect peers.
// It fetches each collection once and checks all groupIDs against them in memory.
func areGroupChangesAffectPeers(ctx context.Context, transaction store.Store, accountID string, groupIDs []string) (bool, error) {
diff --git a/management/server/group_test.go b/management/server/group_test.go
index da056c8a9..fa351a43e 100644
--- a/management/server/group_test.go
+++ b/management/server/group_test.go
@@ -132,6 +132,11 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
"grp-for-agent-network-policy",
"agent network policy",
},
+ {
+ "agent network budget rule",
+ "grp-for-agent-network-budget-rule",
+ "agent network budget rule",
+ },
{
"reverse proxy private service access group",
"grp-for-rp-private",
@@ -152,6 +157,16 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
return
}
+ group, getErr := am.GetGroup(context.Background(), account.Id, testCase.groupID, groupAdminUserID)
+ if getErr != nil {
+ t.Errorf("group %s should still exist after failed deletion: %s", testCase.groupID, getErr)
+ return
+ }
+ if group == nil {
+ t.Errorf("group %s was deleted despite the failed deletion", testCase.groupID)
+ return
+ }
+
var sErr *status.Error
if errors.As(err, &sErr) {
if sErr.Message != testCase.expectedReason {
@@ -240,6 +255,12 @@ func TestDefaultAccountManager_DeleteGroups(t *testing.T) {
groupIDs: []string{"grp-for-agent-network-policy"},
expectedReasons: []string{"agent network policy"},
},
+ {
+ name: "agent network budget rule",
+ groupIDs: []string{"grp-for-agent-network-budget-rule"},
+ expectedReasons: []string{"agent network budget rule"},
+ expectedNotDeleted: []string{"grp-for-agent-network-budget-rule"},
+ },
{
name: "reverse proxy services",
groupIDs: []string{"grp-for-rp-private", "grp-for-rp-bearer"},
@@ -501,6 +522,14 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
Peers: make([]string, 0),
}
+ groupForAgentNetworkBudgetRule := &types.Group{
+ ID: "grp-for-agent-network-budget-rule",
+ AccountID: "account-id",
+ Name: "Group for agent network budget rules",
+ Issued: types.GroupIssuedAPI,
+ Peers: make([]string, 0),
+ }
+
groupForRPPrivate := &types.Group{
ID: "grp-for-rp-private",
AccountID: "account-id",
@@ -573,6 +602,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForUsers)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkPolicy)
+ _ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkBudgetRule)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForRPPrivate)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForRPBearer)
@@ -587,6 +617,20 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
return nil, nil, err
}
+ budgetRuleDecoy := agentNetworkTypes.NewAccountBudgetRule(accountID)
+ budgetRuleDecoy.Name = "Unrelated agent network budget rule"
+ budgetRuleDecoy.TargetGroups = []string{"unrelated-group"}
+ if err := am.Store.SaveAgentNetworkBudgetRule(context.Background(), budgetRuleDecoy); err != nil {
+ return nil, nil, err
+ }
+
+ budgetRule := agentNetworkTypes.NewAccountBudgetRule(accountID)
+ budgetRule.Name = "Example agent network budget rule"
+ budgetRule.TargetGroups = []string{groupForAgentNetworkBudgetRule.ID}
+ if err := am.Store.SaveAgentNetworkBudgetRule(context.Background(), budgetRule); err != nil {
+ return nil, nil, err
+ }
+
// The decoy services are created first so the linkage check has to scan
// past services that do not reference the groups under test.
rpServices := []*rpservice.Service{
diff --git a/management/server/http/testing/integration/events_handler_integration_test.go b/management/server/http/testing/integration/events_handler_integration_test.go
index 6611b60ee..a051aabcd 100644
--- a/management/server/http/testing/integration/events_handler_integration_test.go
+++ b/management/server/http/testing/integration/events_handler_integration_test.go
@@ -3,6 +3,7 @@
package integration
import (
+ "context"
"encoding/json"
"net/http"
"net/http/httptest"
@@ -10,6 +11,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/http/testing/testing_tools"
"github.com/netbirdio/netbird/management/server/http/testing/testing_tools/channel"
@@ -34,7 +36,7 @@ func Test_Events_GetAll(t *testing.T) {
for _, user := range users {
t.Run(user.name+" - Get all events", func(t *testing.T) {
- apiHandler, _, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/events.sql", nil, false)
+ apiHandler, accountManager, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/events.sql", nil, false)
// First, perform a mutation to generate an event (create a group as admin)
groupBody, err := json.Marshal(&api.GroupRequest{Name: "eventTestGroup"})
@@ -44,7 +46,14 @@ func Test_Events_GetAll(t *testing.T) {
createReq := testing_tools.BuildRequest(t, groupBody, http.MethodPost, "/api/groups", testing_tools.TestAdminId)
createRecorder := httptest.NewRecorder()
apiHandler.ServeHTTP(createRecorder, createReq)
- assert.Equal(t, http.StatusOK, createRecorder.Code, "Failed to create group to generate event")
+ require.Equal(t, http.StatusOK, createRecorder.Code, "Failed to create group to generate event")
+
+ // Group creation returns before its asynchronous audit write finishes.
+ require.EventuallyWithT(t, func(c *assert.CollectT) {
+ events, err := accountManager.GetEvents(context.Background(), testing_tools.TestAccountId, testing_tools.TestAdminId)
+ assert.NoError(c, err)
+ assert.NotEmpty(c, events, "wait for the group creation event before checking permissions")
+ }, time.Second, 10*time.Millisecond)
// Now query events
req := testing_tools.BuildRequest(t, []byte{}, http.MethodGet, "/api/events", user.userId)
diff --git a/management/server/identity_provider.go b/management/server/identity_provider.go
index 86bbcd893..764d8598b 100644
--- a/management/server/identity_provider.go
+++ b/management/server/identity_provider.go
@@ -23,6 +23,9 @@ import (
"github.com/netbirdio/netbird/shared/management/status"
)
+// maxDiscoveryDocumentSize caps the discovery document read at 1 MiB. Providers serve a few kilobytes.
+const maxDiscoveryDocumentSize = 1 << 20
+
// oidcProviderJSON represents the OpenID Connect discovery document
type oidcProviderJSON struct {
Issuer string `json:"issuer"`
@@ -35,6 +38,10 @@ func validateOIDCIssuer(ctx context.Context, issuer string) error {
httpClient := &http.Client{
Timeout: 10 * time.Second,
+ // An issuer that redirects its own discovery document is misconfigured.
+ CheckRedirect: func(*http.Request, []*http.Request) error {
+ return http.ErrUseLastResponse
+ },
}
req, err := http.NewRequestWithContext(ctx, http.MethodGet, wellKnown, nil)
@@ -48,22 +55,22 @@ func validateOIDCIssuer(ctx context.Context, issuer string) error {
}
defer resp.Body.Close()
- body, err := io.ReadAll(resp.Body)
- if err != nil {
- return fmt.Errorf("%w: unable to read response body: %v", types.ErrIdentityProviderIssuerUnreachable, err)
+ if resp.StatusCode != http.StatusOK {
+ return fmt.Errorf("%w: %s", types.ErrIdentityProviderIssuerUnreachable, resp.Status)
}
- if resp.StatusCode != http.StatusOK {
- return fmt.Errorf("%w: %s: %s", types.ErrIdentityProviderIssuerUnreachable, resp.Status, body)
+ body, err := io.ReadAll(io.LimitReader(resp.Body, maxDiscoveryDocumentSize+1))
+ if err != nil || len(body) > maxDiscoveryDocumentSize {
+ return fmt.Errorf("%w: failed to decode provider discovery object", types.ErrIdentityProviderIssuerUnreachable)
}
var p oidcProviderJSON
if err := json.Unmarshal(body, &p); err != nil {
- return fmt.Errorf("%w: failed to decode provider discovery object: %v", types.ErrIdentityProviderIssuerUnreachable, err)
+ return fmt.Errorf("%w: failed to decode provider discovery object", types.ErrIdentityProviderIssuerUnreachable)
}
if p.Issuer != issuer {
- return fmt.Errorf("%w: expected %q got %q", types.ErrIdentityProviderIssuerMismatch, issuer, p.Issuer)
+ return fmt.Errorf("%w: %q", types.ErrIdentityProviderIssuerMismatch, issuer)
}
return nil
@@ -151,15 +158,15 @@ func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, acc
return nil, status.NewPermissionDeniedError()
}
- if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
- return nil, err
- }
-
embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager)
if !ok {
return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP")
}
+ if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
+ return nil, err
+ }
+
// Generate ID if not provided
if idpConfig.ID == "" {
idpConfig.ID = generateIdentityProviderID(idpConfig.Type)
@@ -188,15 +195,15 @@ func (am *DefaultAccountManager) UpdateIdentityProvider(ctx context.Context, acc
return nil, status.NewPermissionDeniedError()
}
- if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
- return nil, err
- }
-
embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager)
if !ok {
return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP")
}
+ if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
+ return nil, err
+ }
+
idpConfig.ID = idpID
idpConfig.AccountID = accountID
diff --git a/management/server/identity_provider_test.go b/management/server/identity_provider_test.go
index ecc47337c..c7a8af1d2 100644
--- a/management/server/identity_provider_test.go
+++ b/management/server/identity_provider_test.go
@@ -7,6 +7,7 @@ import (
"net/http"
"net/http/httptest"
"path/filepath"
+ "strings"
"testing"
"time"
@@ -121,7 +122,7 @@ func createManagerWithEmbeddedIdPModeAndSetup(
}
func TestDefaultAccountManager_CreateIdentityProvider_Validation(t *testing.T) {
- manager, _, err := createManager(t)
+ manager, _, err := createManagerWithEmbeddedIdP(t)
require.NoError(t, err)
userID := "testingUser"
@@ -233,7 +234,7 @@ func TestUpdateUserAuthWithSingleModeKeepsConfiguredDomain(t *testing.T) {
}
func TestDefaultAccountManager_UpdateIdentityProvider_Validation(t *testing.T) {
- manager, _, err := createManager(t)
+ manager, _, err := createManagerWithEmbeddedIdP(t)
require.NoError(t, err)
userID := "testingUser"
@@ -355,3 +356,45 @@ func TestValidateOIDCIssuer_TrailingSlash(t *testing.T) {
require.Error(t, err)
assert.True(t, errors.Is(err, types.ErrIdentityProviderIssuerMismatch))
}
+
+func TestValidateOIDCIssuer_DoesNotFollowRedirects(t *testing.T) {
+ var reached bool
+ target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ reached = true
+ w.WriteHeader(http.StatusForbidden)
+ }))
+ t.Cleanup(target.Close)
+
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ http.Redirect(w, r, target.URL+"/redirect-target", http.StatusFound)
+ }))
+ t.Cleanup(srv.Close)
+
+ err := validateOIDCIssuer(context.Background(), srv.URL)
+ require.Error(t, err)
+ assert.False(t, reached, "Redirects are not followed")
+}
+
+func TestValidateOIDCIssuer_BoundsResponseSize(t *testing.T) {
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ _, _ = w.Write([]byte(`{"issuer":"` + strings.Repeat("a", maxDiscoveryDocumentSize) + `"}`))
+ }))
+ t.Cleanup(srv.Close)
+
+ err := validateOIDCIssuer(context.Background(), srv.URL)
+ require.ErrorIs(t, err, types.ErrIdentityProviderIssuerUnreachable)
+ assert.NotErrorIs(t, err, types.ErrIdentityProviderIssuerMismatch)
+}
+
+func TestValidateOIDCIssuer_RejectsTrailingContent(t *testing.T) {
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ _, _ = w.Write([]byte(`{"issuer":"http://` + r.Host + `"} {"issuer":"second"}`))
+ }))
+ t.Cleanup(srv.Close)
+
+ err := validateOIDCIssuer(context.Background(), srv.URL)
+ require.ErrorIs(t, err, types.ErrIdentityProviderIssuerUnreachable,
+ "Content after the first object is not a valid discovery document")
+}
diff --git a/management/server/migration/migration_custom_domain.go b/management/server/migration/migration_custom_domain.go
new file mode 100644
index 000000000..bac784358
--- /dev/null
+++ b/management/server/migration/migration_custom_domain.go
@@ -0,0 +1,22 @@
+package migration
+
+import (
+ "context"
+ "fmt"
+ "time"
+
+ "gorm.io/gorm"
+
+ "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+)
+
+// MigrateCustomDomainValidationExpiry gives existing pending registrations a validation window.
+func MigrateCustomDomainValidationExpiry(ctx context.Context, db *gorm.DB) error {
+ result := db.WithContext(ctx).Model(&domain.Domain{}).
+ Where("validated = ? AND validation_expires_at IS NULL", false).
+ Update("validation_expires_at", time.Now().UTC().Add(domain.ValidationTTL))
+ if result.Error != nil {
+ return fmt.Errorf("backfill custom domain validation expiry: %w", result.Error)
+ }
+ return nil
+}
diff --git a/management/server/migration/migration_custom_domain_test.go b/management/server/migration/migration_custom_domain_test.go
new file mode 100644
index 000000000..facf23c9f
--- /dev/null
+++ b/management/server/migration/migration_custom_domain_test.go
@@ -0,0 +1,44 @@
+package migration_test
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+ "github.com/netbirdio/netbird/management/server/migration"
+)
+
+func TestMigrateCustomDomainValidationExpiry(t *testing.T) {
+ db := setupDatabase(t)
+ require.NoError(t, db.AutoMigrate(&domain.Domain{}))
+ t.Cleanup(func() { require.NoError(t, db.Migrator().DropTable(&domain.Domain{})) })
+ ctx := context.Background()
+ existingDeadline := time.Now().UTC().Add(time.Hour).Truncate(time.Second)
+ rows := []domain.Domain{
+ {ID: "legacy", Domain: "legacy.example.com"},
+ {ID: "validated", Domain: "validated.example.com", Validated: true},
+ {ID: "pending", Domain: "pending.example.com", ValidationExpiresAt: &existingDeadline},
+ }
+ require.NoError(t, db.Create(&rows).Error)
+ before := time.Now().UTC()
+ require.NoError(t, migration.MigrateCustomDomainValidationExpiry(ctx, db))
+ after := time.Now().UTC()
+ var migrated domain.Domain
+ require.NoError(t, db.First(&migrated, "id = ?", "legacy").Error)
+ require.NotNil(t, migrated.ValidationExpiresAt)
+ assert.WithinRange(t, *migrated.ValidationExpiresAt, before.Truncate(time.Millisecond).Add(48*time.Hour), after.Add(48*time.Hour+time.Millisecond), "legacy pending registrations get a full window")
+ deadline := *migrated.ValidationExpiresAt
+ require.NoError(t, migration.MigrateCustomDomainValidationExpiry(ctx, db))
+ require.NoError(t, db.First(&migrated, "id = ?", "legacy").Error)
+ assert.Equal(t, deadline, *migrated.ValidationExpiresAt, "repeated migration must not extend the deadline")
+ var validated, pending domain.Domain
+ require.NoError(t, db.First(&validated, "id = ?", "validated").Error)
+ require.NoError(t, db.First(&pending, "id = ?", "pending").Error)
+ assert.Nil(t, validated.ValidationExpiresAt, "validated domains do not acquire an expiry")
+ require.NotNil(t, pending.ValidationExpiresAt)
+ assert.WithinDuration(t, existingDeadline, *pending.ValidationExpiresAt, 0, "existing deadlines must be preserved")
+}
diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go
index 351b25d6e..72eb0b01a 100644
--- a/management/server/store/sql_store.go
+++ b/management/server/store/sql_store.go
@@ -5715,6 +5715,10 @@ func (s *SqlStore) CreateCustomDomain(ctx context.Context, accountID string, dom
Type: domain.TypeCustom,
Validated: validated,
}
+ if !validated {
+ expiresAt := time.Now().UTC().Add(domain.ValidationTTL)
+ newDomain.ValidationExpiresAt = &expiresAt
+ }
result := s.db.Create(newDomain)
if result.Error != nil {
// The unique index is the last guard when two requests clear the
@@ -5736,12 +5740,21 @@ func (s *SqlStore) CreateCustomDomain(ctx context.Context, accountID string, dom
return newDomain, nil
}
+// UpdateCustomDomain completes validation only while the original registration is pending.
func (s *SqlStore) UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error) {
- d.AccountID = accountID
- result := s.db.Select("*").Save(d)
+ if !d.Validated {
+ return nil, status.Errorf(status.InvalidArgument, "custom domain update must complete validation")
+ }
+ result := s.db.WithContext(ctx).Model(&domain.Domain{}).
+ Where(accountAndIDQueryCondition, accountID, d.ID).
+ Where("domain = ? AND target_cluster = ?", d.Domain, d.TargetCluster).
+ Where("validated = ? AND validation_expires_at > ?", false, time.Now().UTC()).
+ Update("validated", true)
if result.Error != nil {
- log.WithContext(ctx).Errorf("failed to update reverse proxy custom domain to store: %v", result.Error)
- return nil, status.Errorf(status.Internal, "failed to update reverse proxy custom domain to store")
+ return nil, fmt.Errorf("validate custom domain in store: %w", result.Error)
+ }
+ if result.RowsAffected == 0 {
+ return nil, status.Errorf(status.PreconditionFailed, "custom domain registration is no longer pending validation")
}
return d, nil
diff --git a/management/server/store/sql_store_domain_expiration.go b/management/server/store/sql_store_domain_expiration.go
new file mode 100644
index 000000000..525c54084
--- /dev/null
+++ b/management/server/store/sql_store_domain_expiration.go
@@ -0,0 +1,60 @@
+package store
+
+import (
+ "context"
+ "fmt"
+ "strings"
+ "time"
+
+ "gorm.io/gorm"
+
+ "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+ rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+ "github.com/netbirdio/netbird/shared/management/status"
+)
+
+// GetExpiredCustomDomains lists pending registrations in stable batches across accounts.
+func (s *SqlStore) GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error) {
+ var domains []*domain.Domain
+ result := s.db.WithContext(ctx).
+ Where("validated = ? AND validation_expires_at <= ? AND id > ?", false, now, string(afterID)).
+ Order("id").Limit(limit).Find(&domains)
+ if result.Error != nil {
+ return nil, fmt.Errorf("list expired custom domains: %w", result.Error)
+ }
+ return domains, nil
+}
+
+// DeleteExpiredCustomDomain deletes an expired registration only if no service uses its namespace.
+func (s *SqlStore) DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error) {
+ db := s.db.WithContext(ctx)
+ services := customDomainServices(db, d)
+ result := db.Where(accountAndIDQueryCondition, d.AccountID, d.ID).
+ Where("domain = ? AND validated = ? AND validation_expires_at <= ?", d.Domain, false, now).
+ Where("NOT EXISTS (?)", services.Select("1")).Delete(&domain.Domain{})
+ if result.Error != nil {
+ return false, fmt.Errorf("delete expired custom domain: %w", result.Error)
+ }
+ if result.RowsAffected > 0 {
+ return true, nil
+ }
+ var count int64
+ if err := customDomainServices(db, d).Count(&count).Error; err != nil {
+ return false, fmt.Errorf("check expired custom domain services: %w", err)
+ }
+ if count > 0 {
+ return false, status.Errorf(status.PreconditionFailed, "expired custom domain still has dependent services")
+ }
+ return false, nil
+}
+
+func customDomainServices(db *gorm.DB, d *domain.Domain) *gorm.DB {
+ name := strings.ToLower(strings.TrimSuffix(d.Domain, "."))
+ // Shared domain validation permits underscores, and older rows may contain
+ // other LIKE metacharacters.
+ escaped := strings.NewReplacer("!", "!!", "%", "!%", "_", "!_").Replace(name)
+ return db.Model(&rpservice.Service{}).Where(
+ "LOWER(domain) IN ? OR LOWER(domain) LIKE ? ESCAPE '!' OR LOWER(domain) LIKE ? ESCAPE '!'",
+ []string{name, name + "."}, "%."+escaped, "%."+escaped+".",
+ )
+}
diff --git a/management/server/store/sql_store_domain_expiration_test.go b/management/server/store/sql_store_domain_expiration_test.go
new file mode 100644
index 000000000..7dcfb0562
--- /dev/null
+++ b/management/server/store/sql_store_domain_expiration_test.go
@@ -0,0 +1,78 @@
+package store
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+ rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+)
+
+func TestDeleteExpiredCustomDomain_ServiceDependencies(t *testing.T) {
+ runTestForAllEngines(t, "", func(t *testing.T, store Store) {
+ ctx := context.Background()
+ now := time.Now().UTC()
+ db := store.(*SqlStore).db
+ require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
+ for _, tt := range []struct {
+ name string
+ domainName string
+ serviceHost string
+ protected bool
+ }{
+ {"exact", "example.com", "example.com", true},
+ {"subdomain", "example.com", "deep.app.example.com", true},
+ {"case", "example.com", "APP.EXAMPLE.COM.", true},
+ {"suffix-boundary", "example.com", "notexample.com", false},
+ {"literal underscore", "a_b.example.com", "app.a_b.example.com", true},
+ {"underscore wildcard", "a_b.example.com", "app.axb.example.com", false},
+ {"legacy percent wildcard", "a%b.example.com", "app.axxb.example.com", false},
+ {"legacy escape character", "a!b.example.com", "app.ab.example.com", false},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ d, err := store.CreateCustomDomain(ctx, "owner", tt.domainName, "cluster", false)
+ require.NoError(t, err)
+ require.NoError(t, db.Model(d).Update("validation_expires_at", now.Add(-time.Hour)).Error)
+ svc := &rpservice.Service{ID: "legacy", AccountID: "owner", Domain: tt.serviceHost}
+ require.NoError(t, store.CreateService(ctx, svc))
+ deleted, err := store.DeleteExpiredCustomDomain(ctx, d, now)
+ if tt.protected {
+ require.Error(t, err)
+ assert.False(t, deleted, "service namespaces must remain reserved")
+ } else {
+ require.NoError(t, err)
+ assert.True(t, deleted, "a hostname outside the namespace must not prevent cleanup")
+ }
+ require.NoError(t, db.Delete(svc).Error)
+ require.NoError(t, db.Delete(d).Error)
+ })
+ }
+ })
+}
+
+func TestDeleteExpiredCustomDomain_RechecksValidation(t *testing.T) {
+ runTestForAllEngines(t, "", func(t *testing.T, store Store) {
+ ctx := context.Background()
+ require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
+ d, err := store.CreateCustomDomain(ctx, "owner", "validated.example.com", "cluster", false)
+ require.NoError(t, err)
+ d, err = store.GetCustomDomain(ctx, "owner", d.ID)
+ require.NoError(t, err)
+ stale := d.Copy()
+ d.Validated = true
+ _, err = store.UpdateCustomDomain(ctx, "owner", d)
+ require.NoError(t, err)
+ deleted, err := store.DeleteExpiredCustomDomain(ctx, stale, time.Now().Add(domain.ValidationTTL))
+ require.NoError(t, err)
+ assert.False(t, deleted, "a stale cleanup candidate must not delete a validated registration")
+ stored, err := store.GetCustomDomain(ctx, "owner", d.ID)
+ require.NoError(t, err)
+ assert.True(t, stored.Validated, "the validated registration must remain usable")
+ require.NotNil(t, stored.ValidationExpiresAt)
+ assert.Equal(t, stale.ValidationExpiresAt, stored.ValidationExpiresAt, "validation must preserve the original deadline")
+ })
+}
diff --git a/management/server/store/sql_store_get_account_test.go b/management/server/store/sql_store_get_account_test.go
index 686839b1f..aca8d8a12 100644
--- a/management/server/store/sql_store_get_account_test.go
+++ b/management/server/store/sql_store_get_account_test.go
@@ -64,7 +64,7 @@ func assertGetAccountLoadsCustomDomains(t *testing.T, store Store) {
_, err := store.CreateCustomDomain(ctx, accountID, "example.com", "eu.proxy.netbird.io", true)
require.NoError(t, err, "creating the first custom domain must succeed")
- _, err = store.CreateCustomDomain(ctx, accountID, "apps.acme.io", "us.proxy.netbird.io", false)
+ pending, err := store.CreateCustomDomain(ctx, accountID, "apps.acme.io", "us.proxy.netbird.io", false)
require.NoError(t, err, "creating the second custom domain must succeed")
account, err := store.GetAccount(ctx, accountID)
@@ -75,6 +75,10 @@ func assertGetAccountLoadsCustomDomains(t *testing.T, store Store) {
for _, d := range account.Domains {
require.NotNil(t, d)
byDomain[d.Domain] = d.TargetCluster
+ if d.ID == pending.ID {
+ require.NotNil(t, d.ValidationExpiresAt)
+ assert.WithinDuration(t, *pending.ValidationExpiresAt, *d.ValidationExpiresAt, time.Millisecond, "both account loaders must preserve the validation deadline")
+ }
}
assert.Equal(t, "eu.proxy.netbird.io", byDomain["example.com"], "custom domain must carry its target cluster")
assert.Equal(t, "us.proxy.netbird.io", byDomain["apps.acme.io"], "custom domain must carry its target cluster")
diff --git a/management/server/store/store.go b/management/server/store/store.go
index da2b3c6e0..6886536b9 100644
--- a/management/server/store/store.go
+++ b/management/server/store/store.go
@@ -305,6 +305,8 @@ type Store interface {
GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error)
CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error)
+ GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error)
+ DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error)
DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error
CreateAccessLog(ctx context.Context, log *accesslogs.AccessLogEntry) error
@@ -642,6 +644,9 @@ func migratePostAuto(ctx context.Context, db *gorm.DB) error {
func getMigrationsPostAuto(ctx context.Context) []migrationFunc {
return []migrationFunc{
+ func(db *gorm.DB) error {
+ return migration.MigrateCustomDomainValidationExpiry(ctx, db)
+ },
func(db *gorm.DB) error {
return migration.CreateIndexIfNotExists[nbpeer.Peer](ctx, db, "idx_account_ip", "account_id", "ip")
},
diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go
index 9bf49f076..04f79d30a 100644
--- a/management/server/store/store_mock.go
+++ b/management/server/store/store_mock.go
@@ -555,6 +555,21 @@ func (mr *MockStoreMockRecorder) DeleteDNSRecord(ctx, accountID, zoneID, recordI
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteDNSRecord", reflect.TypeOf((*MockStore)(nil).DeleteDNSRecord), ctx, accountID, zoneID, recordID)
}
+// DeleteExpiredCustomDomain mocks base method.
+func (m *MockStore) DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error) {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "DeleteExpiredCustomDomain", ctx, d, now)
+ ret0, _ := ret[0].(bool)
+ ret1, _ := ret[1].(error)
+ return ret0, ret1
+}
+
+// DeleteExpiredCustomDomain indicates an expected call of DeleteExpiredCustomDomain.
+func (mr *MockStoreMockRecorder) DeleteExpiredCustomDomain(ctx, d, now any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteExpiredCustomDomain", reflect.TypeOf((*MockStore)(nil).DeleteExpiredCustomDomain), ctx, d, now)
+}
+
// DeleteGroup mocks base method.
func (m *MockStore) DeleteGroup(ctx context.Context, accountID, groupID string) error {
m.ctrl.T.Helper()
@@ -2002,6 +2017,21 @@ func (mr *MockStoreMockRecorder) GetEmbeddedProxyPeerIDsByCluster(ctx, accountID
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEmbeddedProxyPeerIDsByCluster", reflect.TypeOf((*MockStore)(nil).GetEmbeddedProxyPeerIDsByCluster), ctx, accountID)
}
+// GetExpiredCustomDomains mocks base method.
+func (m *MockStore) GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error) {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GetExpiredCustomDomains", ctx, now, afterID, limit)
+ ret0, _ := ret[0].([]*domain.Domain)
+ ret1, _ := ret[1].(error)
+ return ret0, ret1
+}
+
+// GetExpiredCustomDomains indicates an expected call of GetExpiredCustomDomains.
+func (mr *MockStoreMockRecorder) GetExpiredCustomDomains(ctx, now, afterID, limit any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetExpiredCustomDomains", reflect.TypeOf((*MockStore)(nil).GetExpiredCustomDomains), ctx, now, afterID, limit)
+}
+
// GetExpiredEphemeralServices mocks base method.
func (m *MockStore) GetExpiredEphemeralServices(ctx context.Context, ttl time.Duration, limit int) ([]*service.Service, error) {
m.ctrl.T.Helper()
diff --git a/management/server/types/identity_provider.go b/management/server/types/identity_provider.go
index 0c1f9509c..f75b5319d 100644
--- a/management/server/types/identity_provider.go
+++ b/management/server/types/identity_provider.go
@@ -3,6 +3,7 @@ package types
import (
"errors"
"net/url"
+ "strings"
)
// Identity provider validation errors
@@ -99,7 +100,16 @@ func (idp *IdentityProvider) Validate() error {
}
if idp.Issuer != "" {
parsedURL, err := url.Parse(idp.Issuer)
- if err != nil || parsedURL.Scheme == "" || parsedURL.Host == "" {
+ if err != nil || parsedURL.Host == "" {
+ return ErrIdentityProviderIssuerInvalid
+ }
+ if parsedURL.Scheme != "https" {
+ return ErrIdentityProviderIssuerInvalid
+ }
+ if parsedURL.User != nil {
+ return ErrIdentityProviderIssuerInvalid
+ }
+ if strings.ContainsAny(idp.Issuer, "?#") {
return ErrIdentityProviderIssuerInvalid
}
}
diff --git a/management/server/types/identity_provider_test.go b/management/server/types/identity_provider_test.go
index 6ddc563f2..53385c5ac 100644
--- a/management/server/types/identity_provider_test.go
+++ b/management/server/types/identity_provider_test.go
@@ -135,3 +135,54 @@ func TestIdentityProvider_Validate(t *testing.T) {
})
}
}
+
+func TestIdentityProvider_ValidateRejectsNonOriginIssuers(t *testing.T) {
+ issuers := []string{
+ "https://idp.example.com/realms/nb?foo=bar",
+ "https://idp.example.com/realms/nb#section",
+ "https://user:pass@idp.example.com",
+ "ftp://idp.example.com",
+ "ldap://idp.example.com",
+ "http://idp.example.com",
+ }
+
+ for _, issuer := range issuers {
+ t.Run(issuer, func(t *testing.T) {
+ idp := &IdentityProvider{
+ Name: "test",
+ Type: IdentityProviderTypeOIDC,
+ Issuer: issuer,
+ ClientID: "client-id",
+ }
+ assert.ErrorIs(t, idp.Validate(), ErrIdentityProviderIssuerInvalid)
+ })
+ }
+}
+
+func TestIdentityProvider_ValidateAcceptsOriginAndPath(t *testing.T) {
+ for _, issuer := range []string{"https://idp.example.com", "https://idp.example.com/realms/nb", "https://127.0.0.1:5556/dex"} {
+ t.Run(issuer, func(t *testing.T) {
+ idp := &IdentityProvider{
+ Name: "test",
+ Type: IdentityProviderTypeOIDC,
+ Issuer: issuer,
+ ClientID: "client-id",
+ }
+ assert.NoError(t, idp.Validate())
+ })
+ }
+}
+
+func TestIdentityProviderValidateRejectsBareDelimiters(t *testing.T) {
+ for _, issuer := range []string{"https://idp.example.com/realms/nb?", "https://idp.example.com/realms/nb#"} {
+ t.Run(issuer, func(t *testing.T) {
+ idp := &IdentityProvider{
+ Name: "test",
+ Type: IdentityProviderTypeOIDC,
+ Issuer: issuer,
+ ClientID: "client-id",
+ }
+ assert.ErrorIs(t, idp.Validate(), ErrIdentityProviderIssuerInvalid)
+ })
+ }
+}
diff --git a/upload-server/server/s3_test.go b/upload-server/server/s3_test.go
index a72356409..110b1b780 100644
--- a/upload-server/server/s3_test.go
+++ b/upload-server/server/s3_test.go
@@ -29,7 +29,7 @@ func Test_S3HandlerGetUploadURL(t *testing.T) {
ctx := context.Background()
c, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ContainerRequest: testcontainers.ContainerRequest{
- Image: "minio/minio:RELEASE.2025-04-22T22-12-26Z",
+ Image: "quay.io/minio/minio:RELEASE.2025-04-22T22-12-26Z",
ExposedPorts: []string{"9000/tcp"},
Env: map[string]string{
"MINIO_ROOT_USER": "minioadmin",