Merge branch 'main' into dependabot/npm_and_yarn/proxy/web/npm_and_yarn-b39864987c

client/server/login_outcome_test.go

@@ -8,8 +8,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"google.golang.org/grpc/codes"
-	gstatus "google.golang.org/grpc/status"
 
 	"github.com/netbirdio/netbird/client/internal"
 	"github.com/netbirdio/netbird/client/proto"
@@ -27,9 +25,9 @@ func TestLogin_ManagementUnreachableIsReturnedInsteadOfDemandingSSO(t *testing.T
 
 	unreachable := errors.New("create connection: dial context: context deadline exceeded")
 	attempts := 0
-	s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
+	s.isLoginRequiredFn = func(context.Context) (bool, error) {
 		attempts++
-		return internal.StatusLoginFailed, unreachable
+		return false, unreachable
 	}
 
 	resp, err := s.Login(userCtx(), &proto.LoginRequest{Username: &username})
@@ -55,15 +53,12 @@ func TestLogin_AuthRefusalStartsSSOFlow(t *testing.T) {
 	s.rootCtx = internal.CtxInitState(context.Background())
 	breakProfilePrivateKey(t, cfgPath)
 
-	refused := gstatus.Error(codes.PermissionDenied, "peer is not registered")
-	s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
-		return internal.StatusNeedsLogin, refused
+	s.isLoginRequiredFn = func(context.Context) (bool, error) {
+		return true, nil
 	}
 
 	_, err := s.Login(userCtx(), &proto.LoginRequest{Username: &username})
 	require.Error(t, err)
-	require.NotErrorIs(t, err, refused,
-		"the refusal was handed back to the caller instead of starting the SSO flow")
 
 	status, stateErr := internal.CtxGetState(s.rootCtx).Status()
 	require.NoError(t, stateErr)
@@ -71,6 +66,32 @@ func TestLogin_AuthRefusalStartsSSOFlow(t *testing.T) {
 		"the SSO flow setup was never reached with the broken key")
 }
 
+func TestLogin_SetupKeyStillRunsWhenPeerNeedsLogin(t *testing.T) {
+	s, _, _, username, _ := setupServerWithProfile(t)
+	s.rootCtx = internal.CtxInitState(context.Background())
+
+	s.isLoginRequiredFn = func(context.Context) (bool, error) {
+		return true, nil
+	}
+
+	var keysTried []string
+	s.loginAttemptFn = func(_ context.Context, setupKey, _ string) (internal.StatusType, error) {
+		keysTried = append(keysTried, setupKey)
+		return "", nil
+	}
+
+	setupKey := "A2C8E32F-AEB2-4B45-8FD3-8A0C1B2D3E4F"
+	resp, err := s.Login(userCtx(), &proto.LoginRequest{Username: &username, SetupKey: setupKey})
+	require.NoError(t, err, "the probe's outcome leaked out as the login result")
+	require.NotNil(t, resp)
+	require.Equal(t, []string{setupKey}, keysTried, "the setup key never reached the login attempt")
+	require.Nil(t, s.oauthAuthFlow.flow, "a setup-key login started an SSO flow")
+
+	status, err := internal.CtxGetState(s.rootCtx).Status()
+	require.NoError(t, err)
+	require.Equal(t, internal.StatusIdle, status)
+}
+
 // breakProfilePrivateKey replaces the profile's private key with an unparseable
 // one, which makes any attempt to build a Management client fail on the spot.
 func breakProfilePrivateKey(t *testing.T, cfgPath string) {

client/server/server.go

@@ -140,6 +140,8 @@ type Server struct {
 	// it to drive the login outcomes that need a server on the other end;
 	// production leaves it nil, and every login goes through loginAttempt.
 	loginAttemptFn func(ctx context.Context, setupKey, jwtToken string) (internal.StatusType, error)
+
+	isLoginRequiredFn func(ctx context.Context) (bool, error)
 }
 
 type oauthAuthFlow struct {
@@ -384,6 +386,21 @@ func (s *Server) attemptLogin(ctx context.Context, setupKey, jwtToken string) (i
 	return s.loginAttempt(ctx, setupKey, jwtToken)
 }
 
+func (s *Server) isLoginRequired(ctx context.Context) (bool, error) {
+	if s.isLoginRequiredFn != nil {
+		return s.isLoginRequiredFn(ctx)
+	}
+
+	authClient, err := auth.NewAuth(ctx, s.config.PrivateKey, s.config.ManagementURL, s.config)
+	if err != nil {
+		log.Errorf("failed to create auth client: %v", err)
+		return false, err
+	}
+	defer authClient.Close()
+
+	return authClient.IsLoginRequired(ctx)
+}
+
 // loginAttempt attempts to login using the provided information. It returns
 // StatusNeedsLogin when Management refused the peer's credentials and
 // StatusLoginFailed for every other failure, so callers can tell an
@@ -640,22 +657,22 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
 	s.config = config
 	s.mutex.Unlock()
 
-	loginStatus, err := s.attemptLogin(ctx, "", "")
-	if err == nil {
-		state.Set(internal.StatusIdle)
-		return &proto.LoginResponse{}, nil
-	}
-
-	// Only an authentication refusal means the peer has to (re-)authenticate.
-	// Any other failure leaves the login undecided: Management unreachable, a
+	// A probe that errors leaves the login undecided: Management unreachable, a
 	// restart mid-request, an internal error. Those are returned for the caller
 	// to retry, because turning them into an SSO prompt asks the user to solve
 	// something that is not theirs to solve, and a browser login cannot succeed
-	// while Management is unreachable anyway.
-	if loginStatus != internal.StatusNeedsLogin {
-		state.Set(loginStatus)
+	// while Management is unreachable anyway. Only Management refusing the
+	// peer's key is a decision, and IsLoginRequired reports that as
+	// needsLogin=true rather than an error.
+	needsLogin, err := s.isLoginRequired(ctx)
+	if err != nil {
+		state.Set(internal.StatusLoginFailed)
 		return nil, err
 	}
+	if !needsLogin {
+		state.Set(internal.StatusIdle)
+		return &proto.LoginResponse{}, nil
+	}
 
 	if msg.SetupKey == "" {
 		hint := ""