[client] Fix flaky TestServiceLifecycle/Restart on FreeBSD (#5786)

* This reverts commit e5914e4e8b

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>

* Deprecate client secret in proto

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>

* Fix lint

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>

---------

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>

client/cmd/service_test.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/signal"
 	"runtime"
+	"syscall"
 	"testing"
 	"time"
 
@@ -13,6 +15,22 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// TestMain intercepts when this test binary is run as a daemon subprocess.
+// On FreeBSD, the rc.d service script runs the binary via daemon(8) -r with
+// "service run ..." arguments. Since the test binary can't handle cobra CLI
+// args, it exits immediately, causing daemon -r to respawn rapidly until
+// hitting the rate limit and exiting. This makes service restart unreliable.
+// Blocking here keeps the subprocess alive until the init system sends SIGTERM.
+func TestMain(m *testing.M) {
+	if len(os.Args) > 2 && os.Args[1] == "service" && os.Args[2] == "run" {
+		sig := make(chan os.Signal, 1)
+		signal.Notify(sig, syscall.SIGTERM, os.Interrupt)
+		<-sig
+		return
+	}
+	os.Exit(m.Run())
+}
+
 const (
 	serviceStartTimeout = 10 * time.Second
 	serviceStopTimeout  = 5 * time.Second
@@ -79,6 +97,34 @@ func TestServiceLifecycle(t *testing.T) {
 	logLevel = "info"
 	daemonAddr = fmt.Sprintf("unix://%s/netbird-test.sock", tempDir)
 
+	// Ensure cleanup even if a subtest fails and Stop/Uninstall subtests don't run.
+	t.Cleanup(func() {
+		cfg, err := newSVCConfig()
+		if err != nil {
+			t.Errorf("cleanup: create service config: %v", err)
+			return
+		}
+		ctxSvc, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		s, err := newSVC(newProgram(ctxSvc, cancel), cfg)
+		if err != nil {
+			t.Errorf("cleanup: create service: %v", err)
+			return
+		}
+
+		// If the subtests already cleaned up, there's nothing to do.
+		if _, err := s.Status(); err != nil {
+			return
+		}
+
+		if err := s.Stop(); err != nil {
+			t.Errorf("cleanup: stop service: %v", err)
+		}
+		if err := s.Uninstall(); err != nil {
+			t.Errorf("cleanup: uninstall service: %v", err)
+		}
+	})
+
 	ctx := context.Background()
 
 	t.Run("Install", func(t *testing.T) {

client/internal/auth/auth.go

@@ -221,6 +221,7 @@ func (a *Auth) getPKCEFlow(client *mgm.GrpcClient) (*PKCEAuthorizationFlow, erro
 	config := &PKCEAuthProviderConfig{
 		Audience:              protoConfig.GetAudience(),
 		ClientID:              protoConfig.GetClientID(),
+		ClientSecret:          protoConfig.GetClientSecret(), //nolint:staticcheck
 		TokenEndpoint:         protoConfig.GetTokenEndpoint(),
 		AuthorizationEndpoint: protoConfig.GetAuthorizationEndpoint(),
 		Scope:                 protoConfig.GetScope(),
@@ -265,6 +266,7 @@ func (a *Auth) getDeviceFlow(client *mgm.GrpcClient) (*DeviceAuthorizationFlow,
 	config := &DeviceAuthProviderConfig{
 		Audience:           protoConfig.GetAudience(),
 		ClientID:           protoConfig.GetClientID(),
+		ClientSecret:       protoConfig.GetClientSecret(), //nolint:staticcheck
 		Domain:             protoConfig.Domain,
 		TokenEndpoint:      protoConfig.GetTokenEndpoint(),
 		DeviceAuthEndpoint: protoConfig.GetDeviceAuthEndpoint(),

client/internal/auth/device_flow.go

@@ -29,6 +29,8 @@ var _ OAuthFlow = &DeviceAuthorizationFlow{}
 type DeviceAuthProviderConfig struct {
 	// ClientID An IDP application client id
 	ClientID string
+	// ClientSecret An IDP application client secret
+	ClientSecret string
 	// Domain An IDP API domain
 	// Deprecated. Use OIDCConfigEndpoint instead
 	Domain string

client/internal/auth/pkce_flow.go

@@ -38,6 +38,8 @@ const (
 type PKCEAuthProviderConfig struct {
 	// ClientID An IDP application client id
 	ClientID string
+	// ClientSecret An IDP application client secret
+	ClientSecret string
 	// Audience An Audience for to authorization validation
 	Audience string
 	// TokenEndpoint is the endpoint of an IDP manager where clients can obtain access token
@@ -109,7 +111,8 @@ func NewPKCEAuthorizationFlow(config PKCEAuthProviderConfig) (*PKCEAuthorization
 	}
 
 	cfg := &oauth2.Config{
-		ClientID: config.ClientID,
+		ClientID:     config.ClientID,
+		ClientSecret: config.ClientSecret,
 		Endpoint: oauth2.Endpoint{
 			AuthURL:  config.AuthorizationEndpoint,
 			TokenURL: config.TokenEndpoint,

management/internals/modules/reverseproxy/service/manager/manager.go

@@ -288,6 +288,8 @@ func (m *Manager) validateSubdomainRequirement(ctx context.Context, domain, clus
 }
 
 func (m *Manager) persistNewService(ctx context.Context, accountID string, svc *service.Service) error {
+	customPorts := m.clusterCustomPorts(ctx, svc)
+
 	return m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
 		if svc.Domain != "" {
 			if err := m.checkDomainAvailable(ctx, transaction, svc.Domain, ""); err != nil {
@@ -295,7 +297,7 @@ func (m *Manager) persistNewService(ctx context.Context, accountID string, svc *
 			}
 		}
 
-		if err := m.ensureL4Port(ctx, transaction, svc); err != nil {
+		if err := m.ensureL4Port(ctx, transaction, svc, customPorts); err != nil {
 			return err
 		}
 
@@ -315,12 +317,23 @@ func (m *Manager) persistNewService(ctx context.Context, accountID string, svc *
 	})
 }
 
-// ensureL4Port auto-assigns a listen port when needed and validates cluster support.
-func (m *Manager) ensureL4Port(ctx context.Context, tx store.Store, svc *service.Service) error {
+// clusterCustomPorts queries whether the cluster supports custom ports.
+// Must be called before entering a transaction: the underlying query uses
+// the main DB handle, which deadlocks when called inside a transaction
+// that already holds the connection.
+func (m *Manager) clusterCustomPorts(ctx context.Context, svc *service.Service) *bool {
+	if !service.IsL4Protocol(svc.Mode) {
+		return nil
+	}
+	return m.capabilities.ClusterSupportsCustomPorts(ctx, svc.ProxyCluster)
+}
+
+// ensureL4Port auto-assigns a listen port when needed and validates cluster support.
+// customPorts must be pre-computed via clusterCustomPorts before entering a transaction.
+func (m *Manager) ensureL4Port(ctx context.Context, tx store.Store, svc *service.Service, customPorts *bool) error {
 	if !service.IsL4Protocol(svc.Mode) {
 		return nil
 	}
-	customPorts := m.capabilities.ClusterSupportsCustomPorts(ctx, svc.ProxyCluster)
 	if service.IsPortBasedProtocol(svc.Mode) && svc.ListenPort > 0 && (customPorts == nil || !*customPorts) {
 		if svc.Source != service.SourceEphemeral {
 			return status.Errorf(status.InvalidArgument, "custom ports not supported on cluster %s", svc.ProxyCluster)
@@ -404,12 +417,14 @@ func (m *Manager) assignPort(ctx context.Context, tx store.Store, cluster string
 // The count and exists queries use FOR UPDATE locking to serialize concurrent creates
 // for the same peer, preventing the per-peer limit from being bypassed.
 func (m *Manager) persistNewEphemeralService(ctx context.Context, accountID, peerID string, svc *service.Service) error {
+	customPorts := m.clusterCustomPorts(ctx, svc)
+
 	return m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
 		if err := m.validateEphemeralPreconditions(ctx, transaction, accountID, peerID, svc); err != nil {
 			return err
 		}
 
-		if err := m.ensureL4Port(ctx, transaction, svc); err != nil {
+		if err := m.ensureL4Port(ctx, transaction, svc, customPorts); err != nil {
 			return err
 		}
 
@@ -512,16 +527,49 @@ type serviceUpdateInfo struct {
 }
 
 func (m *Manager) persistServiceUpdate(ctx context.Context, accountID string, service *service.Service) (*serviceUpdateInfo, error) {
+	effectiveCluster, err := m.resolveEffectiveCluster(ctx, accountID, service)
+	if err != nil {
+		return nil, err
+	}
+
+	svcForCaps := *service
+	svcForCaps.ProxyCluster = effectiveCluster
+	customPorts := m.clusterCustomPorts(ctx, &svcForCaps)
+
 	var updateInfo serviceUpdateInfo
 
-	err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
-		return m.executeServiceUpdate(ctx, transaction, accountID, service, &updateInfo)
+	err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
+		return m.executeServiceUpdate(ctx, transaction, accountID, service, &updateInfo, customPorts)
 	})
 
 	return &updateInfo, err
 }
 
-func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.Store, accountID string, service *service.Service, updateInfo *serviceUpdateInfo) error {
+// resolveEffectiveCluster determines the cluster that will be used after the update.
+// It reads the existing service without locking and derives the new cluster if the domain changed.
+func (m *Manager) resolveEffectiveCluster(ctx context.Context, accountID string, svc *service.Service) (string, error) {
+	existing, err := m.store.GetServiceByID(ctx, store.LockingStrengthNone, accountID, svc.ID)
+	if err != nil {
+		return "", err
+	}
+
+	if existing.Domain == svc.Domain {
+		return existing.ProxyCluster, nil
+	}
+
+	if m.clusterDeriver != nil {
+		derived, err := m.clusterDeriver.DeriveClusterFromDomain(ctx, accountID, svc.Domain)
+		if err != nil {
+			log.WithError(err).Warnf("could not derive cluster from domain %s", svc.Domain)
+		} else {
+			return derived, nil
+		}
+	}
+
+	return existing.ProxyCluster, nil
+}
+
+func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.Store, accountID string, service *service.Service, updateInfo *serviceUpdateInfo, customPorts *bool) error {
 	existingService, err := transaction.GetServiceByID(ctx, store.LockingStrengthUpdate, accountID, service.ID)
 	if err != nil {
 		return err
@@ -558,7 +606,7 @@ func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.St
 	m.preserveListenPort(service, existingService)
 	updateInfo.serviceEnabledChanged = existingService.Enabled != service.Enabled
 
-	if err := m.ensureL4Port(ctx, transaction, service); err != nil {
+	if err := m.ensureL4Port(ctx, transaction, service, customPorts); err != nil {
 		return err
 	}
 	if err := m.checkPortConflict(ctx, transaction, service); err != nil {

management/internals/modules/reverseproxy/service/service.go

@@ -787,6 +787,11 @@ func (s *Service) validateHTTPTargets() error {
 }
 
 func (s *Service) validateL4Target(target *Target) error {
+	// L4 services have a single target; per-target disable is meaningless
+	// (use the service-level Enabled flag instead). Force it on so that
+	// buildPathMappings always includes the target in the proto.
+	target.Enabled = true
+
 	if target.Port == 0 {
 		return errors.New("target port is required for L4 services")
 	}

management/internals/shared/grpc/server.go

@@ -966,6 +966,7 @@ func (s *Server) GetDeviceAuthorizationFlow(ctx context.Context, req *proto.Encr
 			Provider: proto.DeviceAuthorizationFlowProvider(provider),
 			ProviderConfig: &proto.ProviderConfig{
 				ClientID:           s.config.DeviceAuthorizationFlow.ProviderConfig.ClientID,
+				ClientSecret:       s.config.DeviceAuthorizationFlow.ProviderConfig.ClientSecret,
 				Domain:             s.config.DeviceAuthorizationFlow.ProviderConfig.Domain,
 				Audience:           s.config.DeviceAuthorizationFlow.ProviderConfig.Audience,
 				DeviceAuthEndpoint: s.config.DeviceAuthorizationFlow.ProviderConfig.DeviceAuthEndpoint,
@@ -1036,6 +1037,7 @@ func (s *Server) GetPKCEAuthorizationFlow(ctx context.Context, req *proto.Encryp
 			ProviderConfig: &proto.ProviderConfig{
 				Audience:              s.config.PKCEAuthorizationFlow.ProviderConfig.Audience,
 				ClientID:              s.config.PKCEAuthorizationFlow.ProviderConfig.ClientID,
+				ClientSecret:          s.config.PKCEAuthorizationFlow.ProviderConfig.ClientSecret,
 				TokenEndpoint:         s.config.PKCEAuthorizationFlow.ProviderConfig.TokenEndpoint,
 				AuthorizationEndpoint: s.config.PKCEAuthorizationFlow.ProviderConfig.AuthorizationEndpoint,
 				Scope:                 s.config.PKCEAuthorizationFlow.ProviderConfig.Scope,

management/server/user.go

@@ -780,9 +780,15 @@ func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transact
 	updatedUser.Role = update.Role
 	updatedUser.Blocked = update.Blocked
 	updatedUser.AutoGroups = update.AutoGroups
-	// these two fields can't be set via API, only via direct call to the method
+	// these fields can't be set via API, only via direct call to the method
 	updatedUser.Issued = update.Issued
 	updatedUser.IntegrationReference = update.IntegrationReference
+	if update.Name != "" {
+		updatedUser.Name = update.Name
+	}
+	if update.Email != "" {
+		updatedUser.Email = update.Email
+	}
 
 	var transferredOwnerRole bool
 	result, err := handleOwnerRoleTransfer(ctx, transaction, initiatorUser, update)

shared/management/client/client_test.go

@@ -545,7 +545,8 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) {
 
 	expectedFlowInfo := &mgmtProto.PKCEAuthorizationFlow{
 		ProviderConfig: &mgmtProto.ProviderConfig{
-			ClientID: "client",
+			ClientID:     "client",
+			ClientSecret: "secret",
 		},
 	}
 
@@ -568,4 +569,5 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) {
 	}
 
 	assert.Equal(t, expectedFlowInfo.ProviderConfig.ClientID, flowInfo.ProviderConfig.ClientID, "provider configured client ID should match")
+	assert.Equal(t, expectedFlowInfo.ProviderConfig.ClientSecret, flowInfo.ProviderConfig.ClientSecret, "provider configured client secret should match") //nolint:staticcheck
 }

shared/management/http/api/openapi.yml

@@ -4414,6 +4414,9 @@ components:
           items:
             type: string
           example: [ "Users" ]
+        connector_id:
+          type: string
+          description: DEX connector ID for embedded IDP setups
     IntegrationEnabled:
       type: object
       properties:

shared/management/http/api/types.gen.go

@@ -1492,6 +1492,9 @@ type AzureIntegration struct {
 	// ClientId Azure AD application (client) ID
 	ClientId string `json:"client_id"`
 
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// Enabled Whether the integration is enabled
 	Enabled bool `json:"enabled"`
 
@@ -1632,6 +1635,9 @@ type CreateAzureIntegrationRequest struct {
 	// ClientSecret Base64-encoded Azure AD client secret
 	ClientSecret string `json:"client_secret"`
 
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// GroupPrefixes List of start_with string patterns for groups to sync
 	GroupPrefixes *[]string `json:"group_prefixes,omitempty"`
 
@@ -1653,6 +1659,9 @@ type CreateAzureIntegrationRequestHost string
 
 // CreateGoogleIntegrationRequest defines model for CreateGoogleIntegrationRequest.
 type CreateGoogleIntegrationRequest struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// CustomerId Customer ID from Google Workspace Account Settings
 	CustomerId string `json:"customer_id"`
 
@@ -1689,6 +1698,9 @@ type CreateOktaScimIntegrationRequest struct {
 	// ConnectionName The Okta enterprise connection name on Auth0
 	ConnectionName string `json:"connection_name"`
 
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// GroupPrefixes List of start_with string patterns for groups to sync
 	GroupPrefixes *[]string `json:"group_prefixes,omitempty"`
 
@@ -1698,6 +1710,9 @@ type CreateOktaScimIntegrationRequest struct {
 
 // CreateScimIntegrationRequest defines model for CreateScimIntegrationRequest.
 type CreateScimIntegrationRequest struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// GroupPrefixes List of start_with string patterns for groups to sync
 	GroupPrefixes *[]string `json:"group_prefixes,omitempty"`
 
@@ -2154,6 +2169,9 @@ type GetTenantsResponse = []TenantResponse
 
 // GoogleIntegration defines model for GoogleIntegration.
 type GoogleIntegration struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// CustomerId Customer ID from Google Workspace
 	CustomerId string `json:"customer_id"`
 
@@ -2502,6 +2520,9 @@ type IntegrationResponsePlatform string
 
 // IntegrationSyncFilters defines model for IntegrationSyncFilters.
 type IntegrationSyncFilters struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// GroupPrefixes List of start_with string patterns for groups to sync
 	GroupPrefixes *[]string `json:"group_prefixes,omitempty"`
 
@@ -2994,6 +3015,9 @@ type OktaScimIntegration struct {
 	// AuthToken SCIM API token (full on creation/regeneration, masked on retrieval)
 	AuthToken string `json:"auth_token"`
 
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// Enabled Whether the integration is enabled
 	Enabled bool `json:"enabled"`
 
@@ -3864,6 +3888,9 @@ type ScimIntegration struct {
 	// AuthToken SCIM API token (full on creation, masked otherwise)
 	AuthToken string `json:"auth_token"`
 
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// Enabled Whether the integration is enabled
 	Enabled bool `json:"enabled"`
 
@@ -4341,6 +4368,9 @@ type UpdateAzureIntegrationRequest struct {
 	// ClientSecret Base64-encoded Azure AD client secret
 	ClientSecret *string `json:"client_secret,omitempty"`
 
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// Enabled Whether the integration is enabled
 	Enabled *bool `json:"enabled,omitempty"`
 
@@ -4359,6 +4389,9 @@ type UpdateAzureIntegrationRequest struct {
 
 // UpdateGoogleIntegrationRequest defines model for UpdateGoogleIntegrationRequest.
 type UpdateGoogleIntegrationRequest struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// CustomerId Customer ID from Google Workspace Account Settings
 	CustomerId *string `json:"customer_id,omitempty"`
 
@@ -4380,6 +4413,9 @@ type UpdateGoogleIntegrationRequest struct {
 
 // UpdateOktaScimIntegrationRequest defines model for UpdateOktaScimIntegrationRequest.
 type UpdateOktaScimIntegrationRequest struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// Enabled Whether the integration is enabled
 	Enabled *bool `json:"enabled,omitempty"`
 
@@ -4392,6 +4428,9 @@ type UpdateOktaScimIntegrationRequest struct {
 
 // UpdateScimIntegrationRequest defines model for UpdateScimIntegrationRequest.
 type UpdateScimIntegrationRequest struct {
+	// ConnectorId DEX connector ID for embedded IDP setups
+	ConnectorId *string `json:"connector_id,omitempty"`
+
 	// Enabled Whether the integration is enabled
 	Enabled *bool `json:"enabled,omitempty"`