sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-05-16 21:59:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

[client/ui] Fix profile-submenu race, restore Connect re-auth flow

Browse Source 
Three tray fixes after the SubscribeStatus stream refactor:

* loadProfiles now serializes via a dedicated profileLoadMu and runs
  AFTER the SetHidden/SetEnabled writes inside applyStatus's iconChanged
  block. Previously the status-driven refresh fired before the menu-item
  writes finished, so processMenu's clearMenu/re-add NSMenu rebuild
  raced against SetHidden on darwin — the Disconnect entry could end
  up visible-but-disabled even when applyStatus had requested it hidden.

* The status row is no longer a hidden "Login" entry. It now renders
  as a plain enabled label (so the text isn't greyed out) but has no
  OnClick handler — clicks are no-ops, matching the legacy Fyne UI.
  All actionable transitions flow through Connect/Disconnect.

* handleConnect routes NeedsLogin/SessionExpired/LoginFailed to the
  frontend's /login route (which already runs Login → WaitSSOLogin →
  Up) instead of calling Up directly. The plain Up RPC errors with
  "up already in progress: current status NeedsLogin" in those
  states; the legacy Fyne UI drove the SSO dance from the Connect
  button as well.
This commit is contained in:
Zoltan Papp
2026-05-14 14:52:03 +02:00
parent d841a6aa07
commit fc1db63fc3
1 changed files with 60 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
client/ui/tray.go
View File

@@ -133,6 +133,13 @@ type Tray struct {
activeProfile string
activeUsername string
switchCancel context.CancelFunc
// profileLoadMu serializes loadProfiles so the daemon-status-driven
// refresh in applyStatus cannot race with the ApplicationStarted seed
// or the post-switchProfile reload — both manipulate profileSubmenu and
// SetMenu, which the Wails menu API is not safe against concurrent
// callers.
profileLoadMu sync.Mutex
}
func NewTray(app *application.App, window *application.WebviewWindow, svc TrayServices) *Tray {
@@ -189,13 +196,14 @@ func (t *Tray) ShowWindow() {
func (t *Tray) buildMenu() *application.Menu {
menu := application.NewMenu()
// statusItem doubles as the "Login" entry once the daemon reports
// NeedsLogin/SessionExpired — applyStatus toggles its enabled state and
// label. The click handler is harmless while disabled, so we wire it
// up unconditionally rather than swapping items at runtime.
// statusItem shows the daemon's current status. Kept enabled so the
// label renders in the normal foreground colour (NSMenuItem greys
// disabled items, which makes the live status read like inactive
// chrome), but no OnClick handler is wired — clicking is a no-op.
// The Connect entry below drives every actionable transition,
// including the SSO re-auth flow for NeedsLogin/SessionExpired
// (the daemon's Up RPC returns NeedsSSOLogin when applicable).
t.statusItem = menu.Add(menuStatusDisconnected).
OnClick(func(*application.Context) { t.openRoute("/login") }).
SetEnabled(false).
SetBitmap(iconMenuDotIdle)
menu.AddSeparator()
@@ -275,6 +283,20 @@ func (t *Tray) openRoute(route string) {
}
func (t *Tray) handleConnect() {
// NeedsLogin/SessionExpired/LoginFailed mean the daemon won't honor a
// plain Up RPC ("up already in progress: current status NeedsLogin") —
// it needs the Login → WaitSSOLogin → Up sequence instead. The
// frontend's /login route already drives that flow, so route the
// click there rather than re-implementing the SSO dance in the tray.
t.mu.Lock()
needsLogin := strings.EqualFold(t.lastStatus, services.StatusNeedsLogin) ||
strings.EqualFold(t.lastStatus, services.StatusSessionExpired) ||
strings.EqualFold(t.lastStatus, services.StatusLoginFailed)
t.mu.Unlock()
if needsLogin {
t.openRoute("/login")
return
}
t.upItem.SetEnabled(false)
go func() {
ctx, cancel := context.WithCancel(context.Background())
@@ -475,16 +497,13 @@ func (t *Tray) applyStatus(st services.Status) {
if iconChanged {
t.applyIcon()
needsLogin := strings.EqualFold(st.Status, services.StatusNeedsLogin) ||
strings.EqualFold(st.Status, services.StatusSessionExpired) ||
strings.EqualFold(st.Status, services.StatusLoginFailed)
daemonUnavailable := strings.EqualFold(st.Status, services.StatusDaemonUnavailable)
connecting := strings.EqualFold(st.Status, services.StatusConnecting)
if t.statusItem != nil {
// When the daemon needs re-authentication the status row turns
// into the actionable Login entry — Connect would only fail.
// When the daemon socket is unreachable, swap the label to make
// the cause obvious; Connect/Disconnect would just fail.
// Label-only: enabled so the text isn't greyed out, but no
// click handler is bound (see buildMenu). Swap the displayed
// text so the user sees a familiar phrase instead of the raw
// daemon enum.
label := st.Status
switch {
case daemonUnavailable:
@@ -493,16 +512,19 @@ func (t *Tray) applyStatus(st services.Status) {
label = menuStatusDisconnected
}
t.statusItem.SetLabel(label)
t.statusItem.SetEnabled(needsLogin)
t.statusItem.SetEnabled(true)
t.applyStatusIndicator(st.Status)
}
if t.upItem != nil {
// Hide Connect whenever an Up action would be a no-op or would
// only fail: tunnel already up, daemon mid-connect (Disconnect
// takes over the slot so the user can abort), login required,
// or daemon socket unreachable.
t.upItem.SetHidden(connected || connecting || needsLogin || daemonUnavailable)
t.upItem.SetEnabled(!connected && !connecting && !needsLogin && !daemonUnavailable)
// Connect stays visible/clickable in NeedsLogin/SessionExpired/
// LoginFailed too — the daemon's Up RPC kicks off the SSO flow
// when re-auth is required, mirroring the legacy Fyne client
// where the same button drove the initial and the re-login
// paths. Hidden only when the action would be a no-op (tunnel
// up, daemon mid-connect — Disconnect takes the slot) or
// would fail with no useful side effect (daemon unreachable).
t.upItem.SetHidden(connected || connecting || daemonUnavailable)
t.upItem.SetEnabled(!connected && !connecting && !daemonUnavailable)
}
if t.downItem != nil {
// Disconnect is the abort path while the daemon is still
@@ -526,6 +548,16 @@ func (t *Tray) applyStatus(st services.Status) {
if t.debugItem != nil {
t.debugItem.SetEnabled(!daemonUnavailable)
}
// Refresh the Profiles submenu on every status-text transition: the
// daemon does not emit an active-profile event, so the startup race
// (UI loads profiles before autoconnect picks the persisted profile)
// and a CLI "profile select && up" both surface here. Fired AFTER
// all SetHidden/SetEnabled writes on the static menu items above so
// loadProfiles' SetMenu rebuild (which clearMenu+processMenu the
// entire NSMenu and re-assigns item.impl) cannot race those
// writes — the Wails 3 alpha menu API is not goroutine-safe and
// reads item.disabled/item.hidden at NSMenuItem construction time.
go t.loadProfiles()
}
if exitNodesChanged {
t.rebuildExitNodes(exitNodes)
@@ -711,12 +743,18 @@ func (t *Tray) loadConfig() {
// loadProfiles refreshes the Profiles submenu from the daemon. Each
// entry is a checkbox showing the active profile and switches on click.
// Called once on ApplicationStarted and again after a successful switch
// so the checkmark moves to the new active profile.
// Called on ApplicationStarted, after a successful switchProfile, and
// from applyStatus whenever the daemon's status text changes — the
// last case catches profile flips driven by another channel (CLI
// "netbird profile select", autoconnect picking the persisted profile
// after the UI's first ListProfiles, etc.) since the daemon does not
// emit a dedicated active-profile event.
func (t *Tray) loadProfiles() {
if t.profileSubmenu == nil {
return
}
t.profileLoadMu.Lock()
defer t.profileLoadMu.Unlock()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()