From f6cfa775f87a6fb04beba787377326c0abdb8693 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:01:37 +0200 Subject: [PATCH] [management] setting to return error on name collision during AddPeer (#8154) --- management/server/account.go | 11 + management/server/activity/codes.go | 11 +- .../handlers/accounts/accounts_handler.go | 4 + .../accounts/accounts_handler_test.go | 9 + management/server/peer.go | 101 +++++++-- management/server/peer_test.go | 211 ++++++++++++++++++ management/server/store/sql_store_account.go | 6 + .../server/store/sql_store_account_test.go | 2 +- management/server/store/sql_store_peer.go | 5 +- management/server/types/settings.go | 5 + shared/management/http/api/openapi.yml | 4 + shared/management/http/api/types.gen.go | 3 + 12 files changed, 344 insertions(+), 28 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 946b9ad1c..0a8ec9a59 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -459,6 +459,7 @@ func (am *DefaultAccountManager) UpdateAccountSettings(ctx context.Context, acco am.handleAutoUpdateAlwaysSettings(ctx, oldSettings, newSettings, userID, accountID) am.handlePeerExposeSettings(ctx, oldSettings, newSettings, userID, accountID) am.handleMetricsPushSettings(ctx, oldSettings, newSettings, userID, accountID) + am.handlePeerHostnameCollisionSettings(ctx, oldSettings, newSettings, userID, accountID) if err = am.handleInactivityExpirationSettings(ctx, oldSettings, newSettings, userID, accountID); err != nil { return nil, err } @@ -644,6 +645,16 @@ func (am *DefaultAccountManager) handleMetricsPushSettings(ctx context.Context, } } +func (am *DefaultAccountManager) handlePeerHostnameCollisionSettings(ctx context.Context, oldSettings, newSettings *types.Settings, userID, accountID string) { + if oldSettings.PeerHostnameCollisionRejected != newSettings.PeerHostnameCollisionRejected { + if newSettings.PeerHostnameCollisionRejected { + am.StoreEvent(ctx, userID, accountID, accountID, activity.AccountPeerHostnameCollisionRejectionEnabled, nil) + } else { + am.StoreEvent(ctx, userID, accountID, accountID, activity.AccountPeerHostnameCollisionRejectionDisabled, nil) + } + } +} + func (am *DefaultAccountManager) handlePeerLoginExpirationSettings(ctx context.Context, oldSettings, newSettings *types.Settings, userID, accountID string) { if oldSettings.PeerLoginExpirationEnabled != newSettings.PeerLoginExpirationEnabled { event := activity.AccountPeerLoginExpirationEnabled diff --git a/management/server/activity/codes.go b/management/server/activity/codes.go index dca043027..11c309ef4 100644 --- a/management/server/activity/codes.go +++ b/management/server/activity/codes.go @@ -287,6 +287,11 @@ const ( // CustomDomainValidationExpired indicates that an unvalidated domain registration expired. CustomDomainValidationExpired Activity = 143 + // AccountPeerHostnameCollisionRejectionEnabled indicates that a user enabled rejecting peer hostname collisions for the account + AccountPeerHostnameCollisionRejectionEnabled Activity = 144 + // AccountPeerHostnameCollisionRejectionDisabled indicates that a user disabled rejecting peer hostname collisions for the account + AccountPeerHostnameCollisionRejectionDisabled Activity = 145 + AccountDeleted Activity = 99999 ) @@ -461,8 +466,10 @@ var activityMap = map[Activity]Code{ AgentNetworkSettingsUpdated: {"Agent Network settings updated", "agent_network.settings.update"}, AgentNetworkSettingsDeleted: {"Agent Network settings deleted", "agent_network.settings.delete"}, - AccountMetricsPushEnabled: {"Account metrics push enabled", "account.setting.metrics.push.enable"}, - AccountMetricsPushDisabled: {"Account metrics push disabled", "account.setting.metrics.push.disable"}, + AccountMetricsPushEnabled: {"Account metrics push enabled", "account.setting.metrics.push.enable"}, + AccountMetricsPushDisabled: {"Account metrics push disabled", "account.setting.metrics.push.disable"}, + AccountPeerHostnameCollisionRejectionEnabled: {"Account peer hostname collision rejection enabled", "account.setting.peer.hostname.collision.rejection.enable"}, + AccountPeerHostnameCollisionRejectionDisabled: {"Account peer hostname collision rejection disabled", "account.setting.peer.hostname.collision.rejection.disable"}, DomainAdded: {"Domain added", "domain.add"}, DomainDeleted: {"Domain deleted", "domain.delete"}, diff --git a/management/server/http/handlers/accounts/accounts_handler.go b/management/server/http/handlers/accounts/accounts_handler.go index e5d75007f..797815685 100644 --- a/management/server/http/handlers/accounts/accounts_handler.go +++ b/management/server/http/handlers/accounts/accounts_handler.go @@ -286,6 +286,9 @@ func (h *handler) updateAccountRequestSettings(req api.PutApiAccountsAccountIdJS if req.Settings.MetricsPushEnabled != nil { returnSettings.MetricsPushEnabled = *req.Settings.MetricsPushEnabled } + if req.Settings.PeerHostnameCollisionRejected != nil { + returnSettings.PeerHostnameCollisionRejected = *req.Settings.PeerHostnameCollisionRejected + } if req.Settings.AgentNetworkOnly != nil { returnSettings.AgentNetworkOnly = *req.Settings.AgentNetworkOnly } @@ -432,6 +435,7 @@ func toAccountResponse(accountID string, settings *types.Settings, meta *types.A AutoUpdateAlways: &settings.AutoUpdateAlways, Ipv6EnabledGroups: &settings.IPv6EnabledGroups, MetricsPushEnabled: &settings.MetricsPushEnabled, + PeerHostnameCollisionRejected: &settings.PeerHostnameCollisionRejected, AgentNetworkOnly: &settings.AgentNetworkOnly, EmbeddedIdpEnabled: &settings.EmbeddedIdpEnabled, LocalAuthDisabled: &settings.LocalAuthDisabled, diff --git a/management/server/http/handlers/accounts/accounts_handler_test.go b/management/server/http/handlers/accounts/accounts_handler_test.go index 1ccac2b04..588258003 100644 --- a/management/server/http/handlers/accounts/accounts_handler_test.go +++ b/management/server/http/handlers/accounts/accounts_handler_test.go @@ -130,6 +130,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), @@ -159,6 +160,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), @@ -188,6 +190,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr("latest"), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), @@ -217,6 +220,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), @@ -246,6 +250,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), @@ -275,6 +280,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), @@ -304,6 +310,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(true), DashboardFeatures: &api.AccountDashboardFeatures{ AgentNetwork: br(true), @@ -345,6 +352,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), DashboardFeatures: &api.AccountDashboardFeatures{ AgentNetwork: br(true), @@ -377,6 +385,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { AutoUpdateAlways: br(false), AutoUpdateVersion: sr(""), MetricsPushEnabled: br(false), + PeerHostnameCollisionRejected: br(false), AgentNetworkOnly: br(false), EmbeddedIdpEnabled: br(false), LocalAuthDisabled: br(false), diff --git a/management/server/peer.go b/management/server/peer.go index f3a5ebf46..c60737535 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" b64 "encoding/base64" + "errors" "fmt" "net" "net/netip" @@ -14,6 +15,7 @@ import ( "github.com/rs/xid" log "github.com/sirupsen/logrus" "golang.org/x/exp/maps" + "gorm.io/gorm" nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/server/idp" @@ -288,19 +290,34 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user var newLabel string newLabel, err = nbdns.GetParsedDomainLabel(update.Name) - if err != nil { - newLabel = "" - } else { - _, err := transaction.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, newLabel) - if err == nil { - newLabel = "" - } - } - - if newLabel == "" { - newLabel, err = getPeerIPDNSLabel(peer.IP, update.Name) + if settings.PeerHostnameCollisionRejected { if err != nil { - return fmt.Errorf("failed to get free DNS label: %w", err) + return status.Errorf(status.InvalidArgument, "invalid peer name: %v", err) + } + if newLabel != peer.DNSLabel { + taken, err := peerLabelTaken(ctx, transaction, accountID, newLabel) + if err != nil { + return err + } + if taken { + return errPeerHostnameInUse(update.Name) + } + } + } else { + if err != nil { + newLabel = "" + } else { + _, err := transaction.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, newLabel) + if err == nil { + newLabel = "" + } + } + + if newLabel == "" { + newLabel, err = getPeerIPDNSLabel(peer.IP, update.Name) + if err != nil { + return fmt.Errorf("failed to get free DNS label: %w", err) + } } } peer.Name = update.Name @@ -336,6 +353,9 @@ func (am *DefaultAccountManager) UpdatePeer(ctx context.Context, accountID, user return transaction.SavePeer(ctx, accountID, peer) }) if err != nil { + if peerLabelChanged && isUniqueConstraintError(err) { + return nil, errPeerHostnameInUse(peer.Name) + } return nil, err } @@ -832,6 +852,14 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe return nil, nil, nil, false, fmt.Errorf("failed getting network: %w", err) } + hostLabel, err := nbdns.GetParsedDomainLabel(peer.Meta.Hostname) + if err != nil { + return nil, nil, nil, false, status.Errorf(status.InvalidArgument, "invalid peer hostname: %v", err) + } + rejectLabelCollision := settings.PeerHostnameCollisionRejected + labelTaken := false + + var insertErr error maxAttempts := 10 for attempt := 1; attempt <= maxAttempts; attempt++ { netPrefix, err := netip.ParsePrefix(network.Net.String()) @@ -843,17 +871,12 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe return nil, nil, nil, false, fmt.Errorf("failed to get free IP: %w", err) } - var freeLabel string - if ephemeral || attempt > 1 { + freeLabel := hostLabel + if !rejectLabelCollision && (ephemeral || labelTaken) { freeLabel, err = getPeerIPDNSLabel(freeIP, peer.Meta.Hostname) if err != nil { return nil, nil, nil, false, fmt.Errorf("failed to get free DNS label: %w", err) } - } else { - freeLabel, err = nbdns.GetParsedDomainLabel(peer.Meta.Hostname) - if err != nil { - return nil, nil, nil, false, fmt.Errorf("failed to get free DNS label: %w", err) - } } newPeer.DNSLabel = freeLabel newPeer.IP = freeIP @@ -884,7 +907,7 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe } } - err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + insertErr = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error { err = transaction.AddPeerToAccount(ctx, newPeer) if err != nil { return err @@ -942,16 +965,28 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe log.WithContext(ctx).Debugf("Peer %s added to account %s", newPeer.ID, accountID) return nil }) - if err == nil { + if insertErr == nil { break } - if isUniqueConstraintError(err) { - log.WithContext(ctx).WithFields(log.Fields{"dns_label": freeLabel, "ip": freeIP}).Tracef("Failed to add peer in attempt %d, retrying: %v", attempt, err) + if isUniqueConstraintError(insertErr) { + if freeLabel == hostLabel { + labelTaken, err = peerLabelTaken(ctx, am.Store, accountID, hostLabel) + if err != nil { + return nil, nil, nil, false, err + } + if labelTaken && rejectLabelCollision { + return nil, nil, nil, false, errPeerHostnameInUse(peer.Meta.Hostname) + } + } + log.WithContext(ctx).WithFields(log.Fields{"dns_label": freeLabel, "ip": freeIP}).Tracef("Failed to add peer in attempt %d, retrying: %v", attempt, insertErr) continue } - return nil, nil, nil, false, fmt.Errorf("failed to add peer to database: %w", err) + return nil, nil, nil, false, fmt.Errorf("failed to add peer to database: %w", insertErr) + } + if insertErr != nil { + return nil, nil, nil, false, fmt.Errorf("add peer after %d attempts: %w", maxAttempts, insertErr) } if newPeer == nil { return nil, nil, nil, false, fmt.Errorf("new peer is nil") @@ -985,6 +1020,24 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, accountID, setupKe return newPeer, network, postureChecks, enableSSH, nil } +// peerLabelTaken reports whether a peer of the account already uses the DNS label. +func peerLabelTaken(ctx context.Context, s store.Store, accountID, label string) (bool, error) { + _, err := s.GetPeerIdByLabel(ctx, store.LockingStrengthNone, accountID, label) + switch { + case err == nil: + return true, nil + case errors.Is(err, gorm.ErrRecordNotFound): + return false, nil + default: + return false, fmt.Errorf("check peer DNS label: %w", err) + } +} + +// errPeerHostnameInUse is a PermissionDenied error because the client treats only that gRPC code as a permanent login failure instead of retrying. +func errPeerHostnameInUse(hostname string) error { + return status.Errorf(status.PermissionDenied, "peer with hostname %q is already registered", hostname) +} + func getPeerIPDNSLabel(ip netip.Addr, peerHostName string) (string, error) { if !ip.Is4() { return "", fmt.Errorf("DNS label generation requires an IPv4 address, got %s", ip) diff --git a/management/server/peer_test.go b/management/server/peer_test.go index fa3be9a58..93f38f25e 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -3050,3 +3050,214 @@ func TestResolvePeerLocation(t *testing.T) { }) } } + +func TestAddPeer_HostnameCollisionRejected(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err) + + account := newAccountWithId(context.Background(), "test-account", "owner", "", "", "", false) + account.Settings.PeerHostnameCollisionRejected = true + require.NoError(t, manager.Store.SaveAccount(context.Background(), account)) + + setupKey, err := manager.CreateSetupKey(context.Background(), account.Id, "key", types.SetupKeyReusable, time.Hour, nil, 100, "owner", false, false) + require.NoError(t, err) + ephemeralKey, err := manager.CreateSetupKey(context.Background(), account.Id, "ephemeral", types.SetupKeyReusable, time.Hour, nil, 100, "owner", true, false) + require.NoError(t, err) + + addPeer := func(hostname, key string) (*nbpeer.Peer, error) { + wgKey, err := wgtypes.GenerateKey() + require.NoError(t, err) + peer := &nbpeer.Peer{ + Key: wgKey.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: hostname, OS: "linux"}, + } + added, _, _, _, err := manager.AddPeer(context.Background(), "", key, "", peer, false) + return added, err + } + + first, err := addPeer("my-host", setupKey.Key) + require.NoError(t, err) + assert.Equal(t, "my-host", first.DNSLabel) + + _, err = addPeer("My-Host", setupKey.Key) + require.Error(t, err) + e, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, status.PermissionDenied, e.Type()) + + other, err := addPeer("other-host", setupKey.Key) + require.NoError(t, err) + assert.Equal(t, "other-host", other.DNSLabel) + + _, err = addPeer("my-host", ephemeralKey.Key) + require.Error(t, err) + e, ok = status.FromError(err) + require.True(t, ok) + assert.Equal(t, status.PermissionDenied, e.Type()) + + ephemeral, err := addPeer("ephemeral-host", ephemeralKey.Key) + require.NoError(t, err) + assert.Equal(t, "ephemeral-host", ephemeral.DNSLabel) + + account.Settings.PeerHostnameCollisionRejected = false + require.NoError(t, manager.Store.SaveAccountSettings(context.Background(), account.Id, account.Settings)) + + suffixed, err := addPeer("my-host", setupKey.Key) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(suffixed.DNSLabel, "my-host-")) +} + +func TestUpdatePeer_HostnameCollisionRejected(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err) + + account := newAccountWithId(context.Background(), "test-account", "owner", "", "", "", false) + account.Settings.PeerHostnameCollisionRejected = true + require.NoError(t, manager.Store.SaveAccount(context.Background(), account)) + + addPeer := func(hostname string) *nbpeer.Peer { + wgKey, err := wgtypes.GenerateKey() + require.NoError(t, err) + added, _, _, _, err := manager.AddPeer(context.Background(), "", "", "owner", &nbpeer.Peer{ + Key: wgKey.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: hostname, OS: "linux"}, + }, false) + require.NoError(t, err) + return added + } + first := addPeer("web-server") + second := addPeer("old-name") + + update := second.Copy() + update.Name = "Web-Server" + _, err = manager.UpdatePeer(context.Background(), account.Id, "owner", update) + require.Error(t, err) + e, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, status.PermissionDenied, e.Type()) + + update = second.Copy() + update.Name = "api-server.example.com" + updated, err := manager.UpdatePeer(context.Background(), account.Id, "owner", update) + require.NoError(t, err) + assert.Equal(t, "api-server", updated.DNSLabel) + + update = first.Copy() + update.Name = "WEB-SERVER" + updated, err = manager.UpdatePeer(context.Background(), account.Id, "owner", update) + require.NoError(t, err, "renaming to a different spelling of the own label must not collide with itself") + assert.Equal(t, "web-server", updated.DNSLabel) +} + +func TestAddPeer_HostnameCollisionRejectedConcurrent(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err) + + account := newAccountWithId(context.Background(), "test-account", "owner", "", "", "", false) + account.Settings.PeerHostnameCollisionRejected = true + require.NoError(t, manager.Store.SaveAccount(context.Background(), account)) + + setupKey, err := manager.CreateSetupKey(context.Background(), account.Id, "key", types.SetupKeyReusable, time.Hour, nil, 100, "owner", false, false) + require.NoError(t, err) + + const attempts = 20 + start := make(chan struct{}) + errs := make(chan error, attempts) + var wg sync.WaitGroup + for i := 0; i < attempts; i++ { + wg.Add(1) + go func() { + defer wg.Done() + wgKey, err := wgtypes.GenerateKey() + if err != nil { + errs <- err + return + } + <-start + _, _, _, _, err = manager.AddPeer(context.Background(), "", setupKey.Key, "", &nbpeer.Peer{ + Key: wgKey.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: "same-host", OS: "linux"}, + }, false) + errs <- err + }() + } + close(start) + wg.Wait() + close(errs) + + succeeded := 0 + for err := range errs { + if err == nil { + succeeded++ + continue + } + e, ok := status.FromError(err) + require.True(t, ok, "unexpected error: %v", err) + assert.Equal(t, status.PermissionDenied, e.Type(), "unexpected error: %v", err) + } + assert.Equal(t, 1, succeeded) + + peers, err := manager.Store.GetAccountPeers(context.Background(), store.LockingStrengthNone, account.Id, "", "", "") + require.NoError(t, err) + require.Len(t, peers, 1) + assert.Equal(t, "same-host", peers[0].DNSLabel) +} + +func TestAddPeer_IPCollisionKeepsLabel(t *testing.T) { + for _, rejectCollision := range []bool{false, true} { + t.Run(fmt.Sprintf("collision rejected %t", rejectCollision), func(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err) + + account := newAccountWithId(context.Background(), "test-account", "owner", "", "", "", false) + account.Settings.PeerHostnameCollisionRejected = rejectCollision + _, smallNet, err := net.ParseCIDR("100.64.0.0/26") + require.NoError(t, err) + account.Network.Net = *smallNet + require.NoError(t, manager.Store.SaveAccount(context.Background(), account)) + + const totalPeers = 20 + for i := 0; i < totalPeers; i++ { + wgKey, err := wgtypes.GenerateKey() + require.NoError(t, err) + hostname := fmt.Sprintf("host-%d", i) + added, _, _, _, err := manager.AddPeer(context.Background(), "", "", "owner", &nbpeer.Peer{ + Key: wgKey.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: hostname, OS: "linux"}, + }, false) + require.NoError(t, err) + assert.Equal(t, hostname, added.DNSLabel, "an IP collision must not change the DNS label") + } + }) + } +} + +func TestAddPeer_ExhaustedIPAllocationFails(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err) + + account := newAccountWithId(context.Background(), "test-account", "owner", "", "", "", false) + _, tinyNet, err := net.ParseCIDR("100.64.0.0/30") + require.NoError(t, err) + account.Network.Net = *tinyNet + require.NoError(t, manager.Store.SaveAccount(context.Background(), account)) + + addPeer := func(hostname string) error { + wgKey, err := wgtypes.GenerateKey() + require.NoError(t, err) + _, _, _, _, err = manager.AddPeer(context.Background(), "", "", "owner", &nbpeer.Peer{ + Key: wgKey.PublicKey().String(), + Meta: nbpeer.PeerSystemMeta{Hostname: hostname, OS: "linux"}, + }, false) + return err + } + require.NoError(t, addPeer("host-1")) + require.NoError(t, addPeer("host-2")) + + err = addPeer("host-3") + require.Error(t, err, "no free IP left, registration must fail") + + peers, err := manager.Store.GetAccountPeers(context.Background(), store.LockingStrengthNone, account.Id, "", "", "") + require.NoError(t, err) + assert.Len(t, peers, 2) +} diff --git a/management/server/store/sql_store_account.go b/management/server/store/sql_store_account.go index 43fe09861..3780de77d 100644 --- a/management/server/store/sql_store_account.go +++ b/management/server/store/sql_store_account.go @@ -746,6 +746,7 @@ func (s *SqlStore) getAccount(ctx context.Context, accountID string) (*types.Acc settings_routing_peer_dns_resolution_enabled, settings_dns_domain, settings_network_range, settings_network_range_v6, settings_ipv6_enabled_groups, settings_lazy_connection_enabled, settings_local_mfa_enabled, settings_metrics_push_enabled, settings_agent_network_only, + settings_peer_hostname_collision_rejected, settings_dashboard_features, settings_auto_update_version, settings_auto_update_always, settings_peer_expose_enabled, settings_peer_expose_groups, -- Embedded ExtraSettings @@ -771,6 +772,7 @@ func (s *SqlStore) getAccount(ctx context.Context, accountID string) (*types.Acc sLazyConnectionEnabled sql.NullBool sLocalMFAEnabled sql.NullBool sMetricsPushEnabled sql.NullBool + sPeerHostnameCollisionRejected sql.NullBool sAgentNetworkOnly sql.NullBool sDashboardFeatures sql.NullString autoUpdateVersion sql.NullString @@ -800,6 +802,7 @@ func (s *SqlStore) getAccount(ctx context.Context, accountID string) (*types.Acc &sRoutingPeerDNSResolutionEnabled, &sDNSDomain, &sNetworkRange, &sNetworkRangeV6, &sIPv6EnabledGroups, &sLazyConnectionEnabled, &sLocalMFAEnabled, &sMetricsPushEnabled, &sAgentNetworkOnly, + &sPeerHostnameCollisionRejected, &sDashboardFeatures, &autoUpdateVersion, &autoUpdateAlways, &peerExposeEnabled, &peerExposeGroups, &sExtraPeerApprovalEnabled, &sExtraUserApprovalRequired, @@ -870,6 +873,9 @@ func (s *SqlStore) getAccount(ctx context.Context, accountID string) (*types.Acc if sMetricsPushEnabled.Valid { account.Settings.MetricsPushEnabled = sMetricsPushEnabled.Bool } + if sPeerHostnameCollisionRejected.Valid { + account.Settings.PeerHostnameCollisionRejected = sPeerHostnameCollisionRejected.Bool + } if sAgentNetworkOnly.Valid { account.Settings.AgentNetworkOnly = sAgentNetworkOnly.Bool } diff --git a/management/server/store/sql_store_account_test.go b/management/server/store/sql_store_account_test.go index ec5976982..ba904bbe4 100644 --- a/management/server/store/sql_store_account_test.go +++ b/management/server/store/sql_store_account_test.go @@ -288,7 +288,7 @@ func Test_AccountSettings_SaveAndRetrieve(t *testing.T) { settings := types.Settings{} numOfExportedFields, err := populateFields.PopulateAll(reflect.ValueOf(&settings).Elem()) assert.NoError(t, err) - assert.Equal(t, 27, numOfExportedFields) + assert.Equal(t, 28, numOfExportedFields) account.Settings = &settings err = store.SaveAccount(context.Background(), account) diff --git a/management/server/store/sql_store_peer.go b/management/server/store/sql_store_peer.go index 9950069fd..ec5e1993c 100644 --- a/management/server/store/sql_store_peer.go +++ b/management/server/store/sql_store_peer.go @@ -715,12 +715,15 @@ func (s *SqlStore) GetPeerIdByLabel(ctx context.Context, lockStrength LockingStr Where("account_id = ? AND dns_label = ?", accountID, hostname). Limit(1). Scan(&peerID) + if result.Error != nil { + return "", result.Error + } if peerID == "" { return "", gorm.ErrRecordNotFound } - return peerID, result.Error + return peerID, nil } // GetEmbeddedProxyPeerIDsByCluster returns peer IDs of all embedded proxy peers diff --git a/management/server/types/settings.go b/management/server/types/settings.go index 815c93ce6..8aa1c7e4c 100644 --- a/management/server/types/settings.go +++ b/management/server/types/settings.go @@ -76,6 +76,10 @@ type Settings struct { // MetricsPushEnabled globally enables or disables client metrics push for the account MetricsPushEnabled bool `gorm:"default:false"` + // PeerHostnameCollisionRejected rejects registering or renaming a peer to a hostname + // already used by another peer of the account instead of suffixing it + PeerHostnameCollisionRejected bool `gorm:"default:false"` + // AgentNetworkOnly limits the dashboard to the Agent Network surface for this account. // Set for accounts created via netbird.ai signups; users can disable it later. AgentNetworkOnly bool `gorm:"default:false"` @@ -123,6 +127,7 @@ func (s *Settings) Copy() *Settings { AutoUpdateAlways: s.AutoUpdateAlways, IPv6EnabledGroups: slices.Clone(s.IPv6EnabledGroups), MetricsPushEnabled: s.MetricsPushEnabled, + PeerHostnameCollisionRejected: s.PeerHostnameCollisionRejected, AgentNetworkOnly: s.AgentNetworkOnly, EmbeddedIdpEnabled: s.EmbeddedIdpEnabled, LocalAuthDisabled: s.LocalAuthDisabled, diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index 68de0d1f5..8b97d5f47 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -380,6 +380,10 @@ components: description: Enables or disables client metrics push for all peers in the account type: boolean example: false + peer_hostname_collision_rejected: + description: When enabled, registering or renaming a peer to a hostname already used by another peer in the account fails instead of receiving an IP-based suffix. Applies to ephemeral peers as well. + type: boolean + example: false agent_network_only: description: Limits the dashboard to the Agent Network surface for this account. Set for accounts created via netbird.ai signups and can be disabled later. Enabling this requires dashboard_features.agent_network to be true in the same request. type: boolean diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index a1f4bc530..1595553ed 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -1693,6 +1693,9 @@ type AccountSettings struct { // PeerExposeGroups Limits which peer groups are allowed to expose services. If empty, all peers are allowed when peer expose is enabled. PeerExposeGroups []string `json:"peer_expose_groups"` + // PeerHostnameCollisionRejected When enabled, registering or renaming a peer to a hostname already used by another peer in the account fails instead of receiving an IP-based suffix. Applies to ephemeral peers as well. + PeerHostnameCollisionRejected *bool `json:"peer_hostname_collision_rejected,omitempty"` + // PeerInactivityExpiration Period of time of inactivity after which peer session expires (seconds). PeerInactivityExpiration int `json:"peer_inactivity_expiration"`