mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-20 20:39:08 +02:00
client: hold NeedsLogin during SSO browser wait and tie it to the caller
WaitSSOLogin set StatusConnecting on entry and ran the browser wait on rootCtx. If the client that drove the login went away mid-wait (UI restart, CLI Ctrl+C), the wait orphaned on rootCtx until the OAuth device-code window expired, and the daemon stayed stuck reporting Connecting — a reattaching client saw a spinner that never resolved instead of a login prompt. Hold StatusNeedsLogin for the whole browser wait (also in the Login cached-flow path) so any client attaching mid-wait reads 'login required', and bridge the wait to callerCtx so a departing client cancels it. On that cancel the defer leaves NeedsLogin in place, so the next client shows the login affordance instead of a stale Connecting.
This commit is contained in:
+39
-11
@@ -555,6 +555,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
|||||||
if s.oauthAuthFlow.flow != nil && s.oauthAuthFlow.flow.GetClientID(ctx) == oAuthFlow.GetClientID(ctx) {
|
if s.oauthAuthFlow.flow != nil && s.oauthAuthFlow.flow.GetClientID(ctx) == oAuthFlow.GetClientID(ctx) {
|
||||||
if s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) {
|
if s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) {
|
||||||
log.Debugf("using previous oauth flow info")
|
log.Debugf("using previous oauth flow info")
|
||||||
|
state.Set(internal.StatusNeedsLogin)
|
||||||
return &proto.LoginResponse{
|
return &proto.LoginResponse{
|
||||||
NeedsSSOLogin: true,
|
NeedsSSOLogin: true,
|
||||||
VerificationURI: s.oauthAuthFlow.info.VerificationURI,
|
VerificationURI: s.oauthAuthFlow.info.VerificationURI,
|
||||||
@@ -602,21 +603,29 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
|||||||
// WaitSSOLogin validates the supplied userCode against the in-flight OAuth
|
// WaitSSOLogin validates the supplied userCode against the in-flight OAuth
|
||||||
// device/PKCE flow and blocks until the user finishes the browser leg.
|
// device/PKCE flow and blocks until the user finishes the browser leg.
|
||||||
//
|
//
|
||||||
|
// The daemon holds StatusNeedsLogin for the whole browser wait (set on
|
||||||
|
// entry): the login is not done until the token returns, so a client that
|
||||||
|
// (re)attaches mid-wait — a restarted UI, a second `netbird up` — reads
|
||||||
|
// "login required" and offers the affordance, instead of a Connecting that
|
||||||
|
// never resolves. The wait is also tied to the caller's context (see the
|
||||||
|
// goroutine below), so a client that goes away cancels the wait instead of
|
||||||
|
// orphaning it on rootCtx until the device-code window expires.
|
||||||
|
//
|
||||||
// State transitions on exit:
|
// State transitions on exit:
|
||||||
//
|
//
|
||||||
// ┌──────────────────────────────────────────┬──────────────────────────────────┐
|
// ┌──────────────────────────────────────────┬──────────────────────────────────┐
|
||||||
// │ Outcome │ contextState │
|
// │ Outcome │ contextState │
|
||||||
// ├──────────────────────────────────────────┼──────────────────────────────────┤
|
// ├──────────────────────────────────────────┼──────────────────────────────────┤
|
||||||
// │ Success → loginAttempt → Connected │ StatusConnected (loginAttempt) │
|
// │ Success → loginAttempt ok │ NeedsLogin held; the caller's Up │
|
||||||
|
// │ │ drives Connecting → Connected │
|
||||||
// │ Success → loginAttempt → still-NeedsLogin│ StatusNeedsLogin (loginAttempt) │
|
// │ Success → loginAttempt → still-NeedsLogin│ StatusNeedsLogin (loginAttempt) │
|
||||||
// │ Success → loginAttempt error │ StatusLoginFailed (loginAttempt) │
|
// │ Success → loginAttempt error │ StatusLoginFailed (loginAttempt) │
|
||||||
// │ UserCode mismatch │ StatusLoginFailed │
|
// │ UserCode mismatch │ StatusLoginFailed │
|
||||||
// │ WaitToken: context.Canceled (external │ defer runs: status untouched if │
|
// │ WaitToken: context.Canceled │ NeedsLogin held. Caller gone │
|
||||||
// │ abort — profile switch invokes │ already NeedsLogin/LoginFailed,│
|
// │ (caller went away — UI restart / │ (UI/CLI) → a fresh client │
|
||||||
// │ actCancel/waitCancel, app quit, │ else StatusIdle. Keeps the │
|
// │ Ctrl+C — or internal abort: profile │ shows the login affordance; │
|
||||||
// │ another WaitSSOLogin started) │ cancel from leaking as a │
|
// │ switch / app quit / another │ internal aborts are │
|
||||||
// │ │ spurious LoginFailed on the │
|
// │ WaitSSOLogin via actCancel/waitCancel) │ overwritten by the next Up. │
|
||||||
// │ │ next profile's Up. │
|
|
||||||
// │ WaitToken: context.DeadlineExceeded │ StatusNeedsLogin │
|
// │ WaitToken: context.DeadlineExceeded │ StatusNeedsLogin │
|
||||||
// │ (OAuth device-code window expired │ (retryable; the UI's "Connect" │
|
// │ (OAuth device-code window expired │ (retryable; the UI's "Connect" │
|
||||||
// │ while waiting on the browser leg) │ re-enters the Login flow) │
|
// │ while waiting on the browser leg) │ re-enters the Login flow) │
|
||||||
@@ -625,9 +634,9 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
|||||||
// │ failure, token validation rejection) │ surfaced verbatim to caller) │
|
// │ failure, token validation rejection) │ surfaced verbatim to caller) │
|
||||||
// └──────────────────────────────────────────┴──────────────────────────────────┘
|
// └──────────────────────────────────────────┴──────────────────────────────────┘
|
||||||
//
|
//
|
||||||
// The defer at the top of the function applies the Idle fallback so callers
|
// The defer still applies a StatusIdle fallback for the early
|
||||||
// that bypass the explicit Set calls (the Canceled branch above, the success
|
// oauth-flow-not-initialized return (before the entry Set), so a half state
|
||||||
// path before loginAttempt) still land on a sensible terminal status.
|
// doesn't leak when there is nothing to wait on.
|
||||||
func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLoginRequest) (*proto.WaitSSOLoginResponse, error) {
|
func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLoginRequest) (*proto.WaitSSOLoginResponse, error) {
|
||||||
s.mutex.Lock()
|
s.mutex.Lock()
|
||||||
if s.actCancel != nil {
|
if s.actCancel != nil {
|
||||||
@@ -635,6 +644,21 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
|
|||||||
}
|
}
|
||||||
ctx, cancel := context.WithCancel(s.rootCtx)
|
ctx, cancel := context.WithCancel(s.rootCtx)
|
||||||
|
|
||||||
|
// Tie the in-flight browser wait to the caller. ctx stays rooted in
|
||||||
|
// rootCtx so CtxGetState resolves the daemon's contextState, but if the
|
||||||
|
// UI window or CLI that drove the login goes away mid-flow (restart,
|
||||||
|
// Ctrl+C) the gRPC callerCtx cancels and we cancel the wait instead of
|
||||||
|
// orphaning it on rootCtx until the OAuth device-code window expires.
|
||||||
|
// The goroutine exits as soon as either context completes, so it can't
|
||||||
|
// outlive the RPC.
|
||||||
|
go func() {
|
||||||
|
select {
|
||||||
|
case <-callerCtx.Done():
|
||||||
|
cancel()
|
||||||
|
case <-ctx.Done():
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
|
||||||
md, ok := metadata.FromIncomingContext(callerCtx)
|
md, ok := metadata.FromIncomingContext(callerCtx)
|
||||||
if ok {
|
if ok {
|
||||||
ctx = metadata.NewOutgoingContext(ctx, md)
|
ctx = metadata.NewOutgoingContext(ctx, md)
|
||||||
@@ -660,7 +684,11 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
|
|||||||
}
|
}
|
||||||
}()
|
}()
|
||||||
|
|
||||||
state.Set(internal.StatusConnecting)
|
// Hold NeedsLogin for the whole browser wait — the login is not done
|
||||||
|
// until the token returns, so a client that (re)attaches mid-wait
|
||||||
|
// (restarted UI, second `netbird up`) reads "login required" and offers
|
||||||
|
// the affordance instead of a Connecting that never resolves.
|
||||||
|
state.Set(internal.StatusNeedsLogin)
|
||||||
|
|
||||||
s.mutex.Lock()
|
s.mutex.Lock()
|
||||||
flowInfo := s.oauthAuthFlow.info
|
flowInfo := s.oauthAuthFlow.info
|
||||||
|
|||||||
Reference in New Issue
Block a user