sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-07-20 15:31:30 +02:00
Code Issues Packages Projects Releases Wiki Activity

client+ui: remove SSO handoff flicker and clean up abandoned login via context

Browse Source 
Two follow-ups to the "hold NeedsLogin during the SSO browser wait" change.
Both target the visible state churn the tray showed during the auto-login
handoff (Connect / profile-switch lands on NeedsLogin -> the UI's startLogin
kicks off the SSO flow) and the broken recovery after the user dismisses the
browser-login popup with the window's X.

Background
----------
When a connect attempt lands on NeedsLogin, the UI's startLogin() drives the
SSO flow: Connection.Login() -> (NeedsSSOLogin) open the browser-login popup
-> Connection.WaitSSOLogin() blocks until the browser leg completes. The tray
and the React status page both paint the raw daemon status, so any transient
state the daemon publishes during this handoff is visible as a flicker.

Previously the handoff churned the daemon status through
  NeedsLogin -> Idle -> Connecting -> NeedsLogin
which read as a flicker on the tray icon and the status dot. Two distinct
sources produced the two intermediate states:

  * Idle       came from the UI's defensive cli.Down() at the top of
                Connection.Login (services/connection.go): it tore the engine
                down before every login to dislodge a possibly-parked
                WaitSSOLogin, emitting a StatusIdle on the way.
  * Connecting  came from server.go Login() unconditionally setting
                StatusConnecting before deciding whether the request is an
                SSO flow (which immediately returns NeedsLogin) or a
                setup-key flow (which actually dials Management).

Changes
-------
1. server.go Login(): only set StatusConnecting on the setup-key path, where
   we are about to dial Management with the key and the Connecting paint is
   meaningful. The SSO path returns NeedsLogin and parks on the browser leg,
   so it no longer flashes Connecting first. Removes the Connecting blip.

2. services/connection.go Login(): drop the pre-Login cli.Down(). The daemon
   already dislodges a pending WaitSSOLogin at Login entry (actCancel), and an
   abandoned browser leg is now torn down by cancelling the WaitSSOLogin RPC
   (see 3/4). Removing the Down removes the Idle blip on every login.

3. MainConnectionStatusSwitch.tsx startLogin(): on cancel (the browser-login
   popup's Cancel button or its window X, both routed through
   EventBrowserLoginCancel), cancel the in-flight WaitSSOLogin gRPC call via
   waitPromise.cancel() instead of issuing a heavy Connection.Down(). The
   daemon ties the wait to this call's context, so cancelling the call ends
   the wait cleanly with no engine teardown and no Idle paint.

4. server.go WaitSSOLogin(): when the wait unblocks with context.Canceled and
   the cancellation came from our caller (callerCtx.Err() != nil — the client
   cancelled the RPC or went away), clear the cached oauthAuthFlow so a fresh
   Login starts a new device code instead of reusing the abandoned one. The
   entry NeedsLogin stays in place, so a reattaching client still shows the
   login affordance. An internal abort (actCancel fired by a newer
   Login/WaitSSOLogin while our callerCtx is still live) is left untouched so
   the new owner's flow is not clobbered.

Effect
------
The auto-login handoff now goes Connected -> Connecting -> NeedsLogin and
holds, with no Idle/Connecting flicker in between. Dismissing the browser-login
popup with X now recovers the same way as the Cancel button: the WaitSSOLogin
RPC is cancelled, the stale OAuth flow is cleared, and the next connect opens a
fresh browser-login window instead of getting stuck.
This commit is contained in:
Zoltan Papp
2026-05-31 04:26:15 +02:00
parent 9569ac2081
commit 18348e1491
3 changed files with 33 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
client/server/server.go
View File

@@ -539,8 +539,6 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
return &proto.LoginResponse{}, nil
}
state.Set(internal.StatusConnecting)
if msg.SetupKey == "" {
hint := ""
if msg.Hint != nil {
@@ -592,6 +590,11 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
}, nil
}
// Setup-key path: we are about to dial Management with the key, so the
// Connecting paint is meaningful here — unlike the SSO branch above,
// which returns NeedsLogin and parks on the browser leg.
state.Set(internal.StatusConnecting)
if loginStatus, err := s.loginAttempt(ctx, msg.SetupKey, ""); err != nil {
state.Set(loginStatus)
return nil, err
@@ -717,10 +720,19 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
s.mutex.Unlock()
switch {
case errors.Is(err, context.Canceled):
// External abort (profile switch, app quit, another
// WaitSSOLogin started). Not a login failure — let the
// top-level defer fall through to StatusIdle so the next
// flow starts from a clean state.
// External abort. If our caller cancelled (the client closed
// the browser-login popup, or the UI went away — callerCtx is
// done), clear the abandoned OAuth flow so a fresh Login starts
// a new device code instead of reusing this one. The entry
// NeedsLogin stays in place, so a reattaching client shows the
// login affordance. An internal abort (actCancel from a new
// Login/WaitSSOLogin, callerCtx still live) leaves the flow for
// the new owner — don't clobber it.
if callerCtx.Err() != nil {
s.mutex.Lock()
s.oauthAuthFlow = oauthAuthFlow{}
s.mutex.Unlock()
}
case errors.Is(err, context.DeadlineExceeded):
// OAuth device-code window expired with no user action.
// Retryable — leave the daemon in NeedsLogin so the UI

15
client/ui/frontend/src/modules/main/MainConnectionStatusSwitch.tsx
View File

@@ -89,13 +89,14 @@ async function startLogin(): Promise<void> {
}
if (cancelled) {
// Tell the daemon to drop the in-flight WaitSSOLogin so a
// future Login starts fresh; see services/connection.go:74.
try {
await Connection.Down();
} catch (e) {
console.error(e);
}
// Cancel the in-flight WaitSSOLogin gRPC instead of a heavy
// Down. The daemon ties the wait to this call's context
// (server.go WaitSSOLogin), so cancelling ends the wait and
// clears the abandoned OAuth flow — a fresh Login then starts
// a new device code, with no Idle blink on the tray. Swallow
// the cancellation rejection on the abandoned promise.
waitPromise.cancel?.();
void waitPromise.catch(() => {});
return;
}
}

17
client/ui/services/connection.go
View File

@@ -199,16 +199,13 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
return LoginResult{}, err
}
// Reset the daemon's connection loop before kicking off a new login.
// If a previous Login left a WaitSSOLogin pending (user closed the
// browser without completing the flow), the daemon stays parked on the
// old UserCode and replies with "invalid setup-key or no sso information
// provided" to a fresh Login. Calling Down first dislodges that state;
// we ignore the error since Down on an already-idle daemon is a no-op.
if _, derr := cli.Down(ctx, &proto.DownRequest{}); derr != nil {
// Down failed — likely because the daemon is already idle. Continue.
_ = derr
}
// No pre-Login Down: the daemon's Login dislodges a pending WaitSSOLogin
// itself (server.go cancels the in-flight wait via actCancel), and an
// abandoned browser leg is torn down by startLogin cancelling the
// WaitSSOLogin RPC, which the daemon reacts to by clearing the stale
// OAuth flow. A defensive Down here would only add a visible Idle blink
// to the tray during the SSO handoff (Connect/profile-switch →
// NeedsLogin → auto-login) for no gain.
// Mirror the Fyne client's defaulting: when the frontend doesn't supply
// profile / username, fall back to the daemon's active profile and the