From 9e9e33ae688bc1c167dcad78c57f87df550d361d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Mon, 17 Aug 2026 10:12:42 +0200 Subject: [PATCH] [client] Reduce cognitive complexity of Server.Login Login sat at cognitive complexity 27, over the 25 the linter allows. Extract the interactive SSO branch into startSSOLogin, and split the nested in-flight-flow reuse check out of it into reuseOAuthFlow, which flattens the original if/else into early returns: it returns the cached auth info when the previous flow targets the same client and still has more than 90s left, otherwise cancels the stale wait and returns nil so the caller requests a fresh flow. The helpers take the contextState through a small statusSetter interface, since internal.contextState is unexported and re-deriving it with CtxGetState inside the helper would resolve against callerCtx rather than rootCtx. No behavior change: same ordering of state transitions, same mutex scope around the oauthAuthFlow write, same error paths. Login is now at 21. --- client/server/server.go | 119 ++++++++++++++++++++++++---------------- 1 file changed, 71 insertions(+), 48 deletions(-) diff --git a/client/server/server.go b/client/server/server.go index 286f011b9..18a850735 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -61,6 +61,10 @@ const ( var ErrServiceNotUp = errors.New("service is not up") +type statusSetter interface { + Set(update internal.StatusType) +} + // Server for service control. type Server struct { rootCtx context.Context @@ -675,54 +679,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro } if msg.SetupKey == "" { - hint := "" - if msg.Hint != nil { - hint = *msg.Hint - } - oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint, false) - if err != nil { - state.Set(internal.StatusLoginFailed) - return nil, err - } - - if s.oauthAuthFlow.flow != nil && s.oauthAuthFlow.flow.GetClientID(ctx) == oAuthFlow.GetClientID(ctx) { - if s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) { - log.Debugf("using previous oauth flow info") - state.Set(internal.StatusNeedsLogin) - return &proto.LoginResponse{ - NeedsSSOLogin: true, - VerificationURI: s.oauthAuthFlow.info.VerificationURI, - VerificationURIComplete: s.oauthAuthFlow.info.VerificationURIComplete, - UserCode: s.oauthAuthFlow.info.UserCode, - }, nil - } else { - log.Warnf("canceling previous waiting execution") - if s.oauthAuthFlow.waitCancel != nil { - s.oauthAuthFlow.waitCancel() - } - } - } - - authInfo, err := oAuthFlow.RequestAuthInfo(ctx) - if err != nil { - log.Errorf("getting a request OAuth flow failed: %v", err) - return nil, err - } - - s.mutex.Lock() - s.oauthAuthFlow.flow = oAuthFlow - s.oauthAuthFlow.info = authInfo - s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second) - s.mutex.Unlock() - - state.Set(internal.StatusNeedsLogin) - - return &proto.LoginResponse{ - NeedsSSOLogin: true, - VerificationURI: authInfo.VerificationURI, - VerificationURIComplete: authInfo.VerificationURIComplete, - UserCode: authInfo.UserCode, - }, nil + return s.startSSOLogin(ctx, msg, config, state) } // Setup-key path: we are about to dial Management with the key, so the @@ -738,6 +695,72 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro return &proto.LoginResponse{}, nil } +// startSSOLogin opens the interactive leg of a login: it reuses the in-flight +// OAuth flow when one is still valid for the same client, and otherwise +// requests fresh auth info and parks the daemon on StatusNeedsLogin. +func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, config *profilemanager.Config, state statusSetter) (*proto.LoginResponse, error) { + hint := "" + if msg.Hint != nil { + hint = *msg.Hint + } + oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint, false) + if err != nil { + state.Set(internal.StatusLoginFailed) + return nil, err + } + + if resp := s.reuseOAuthFlow(ctx, oAuthFlow, state); resp != nil { + return resp, nil + } + + authInfo, err := oAuthFlow.RequestAuthInfo(ctx) + if err != nil { + log.Errorf("getting a request OAuth flow failed: %v", err) + return nil, err + } + + s.mutex.Lock() + s.oauthAuthFlow.flow = oAuthFlow + s.oauthAuthFlow.info = authInfo + s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second) + s.mutex.Unlock() + + state.Set(internal.StatusNeedsLogin) + + return &proto.LoginResponse{ + NeedsSSOLogin: true, + VerificationURI: authInfo.VerificationURI, + VerificationURIComplete: authInfo.VerificationURIComplete, + UserCode: authInfo.UserCode, + }, nil +} + +// reuseOAuthFlow returns the cached auth info when the previous flow targets +// the same client and still has enough life left, and otherwise cancels the +// stale wait and returns nil so the caller requests a fresh flow. +func (s *Server) reuseOAuthFlow(ctx context.Context, oAuthFlow auth.OAuthFlow, state statusSetter) *proto.LoginResponse { + if s.oauthAuthFlow.flow == nil || s.oauthAuthFlow.flow.GetClientID(ctx) != oAuthFlow.GetClientID(ctx) { + return nil + } + + if !s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) { + log.Warnf("canceling previous waiting execution") + if s.oauthAuthFlow.waitCancel != nil { + s.oauthAuthFlow.waitCancel() + } + return nil + } + + log.Debugf("using previous oauth flow info") + state.Set(internal.StatusNeedsLogin) + return &proto.LoginResponse{ + NeedsSSOLogin: true, + VerificationURI: s.oauthAuthFlow.info.VerificationURI, + VerificationURIComplete: s.oauthAuthFlow.info.VerificationURIComplete, + UserCode: s.oauthAuthFlow.info.UserCode, + } +} + // WaitSSOLogin validates the supplied userCode against the in-flight OAuth // device/PKCE flow and blocks until the user finishes the browser leg. //