Merge branch 'main' into poc/certificate-posture

management/internals/modules/reverseproxy/domain/manager/domain_test.go

@@ -66,8 +66,8 @@ func TestExtractClusterFromFreeDomain(t *testing.T) {
 
 func TestExtractClusterFromCustomDomains(t *testing.T) {
 	customDomains := []*domain.Domain{
-		{Domain: "example.com", TargetCluster: "eu1.proxy.netbird.io"},
-		{Domain: "proxy.corp.io", TargetCluster: "us1.proxy.netbird.io"},
+		{Domain: "example.com", TargetCluster: "eu1.proxy.netbird.io", Validated: true},
+		{Domain: "proxy.corp.io", TargetCluster: "us1.proxy.netbird.io", Validated: true},
 	}
 
 	tests := []struct {
@@ -120,19 +120,49 @@ func TestExtractClusterFromCustomDomains(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			cluster, ok := extractClusterFromCustomDomains(tc.domain, customDomains)
-			assert.Equal(t, tc.wantOK, ok)
-			if ok {
-				assert.Equal(t, tc.wantVal, cluster)
+			cluster, match := extractClusterFromCustomDomains(tc.domain, customDomains)
+			if !tc.wantOK {
+				assert.Equal(t, customDomainNoMatch, match, "unrelated domain should not match any custom domain")
+				return
 			}
+			assert.Equal(t, customDomainValidated, match, "validated custom domain should resolve a cluster")
+			assert.Equal(t, tc.wantVal, cluster)
 		})
 	}
 }
 
+// An unvalidated row must never yield a cluster: the account has not shown it
+// controls the name, so no service may be bound to it.
+func TestExtractClusterFromCustomDomains_UnvalidatedDomainRefused(t *testing.T) {
+	customDomains := []*domain.Domain{
+		{Domain: "example.com", TargetCluster: "eu1.proxy.netbird.io", Validated: false},
+	}
+
+	for _, serviceDomain := range []string{"example.com", "app.example.com"} {
+		t.Run(serviceDomain, func(t *testing.T) {
+			cluster, match := extractClusterFromCustomDomains(serviceDomain, customDomains)
+			assert.Equal(t, customDomainUnvalidated, match, "unvalidated row must be reported as such")
+			assert.Empty(t, cluster, "unvalidated row must not resolve a cluster")
+		})
+	}
+}
+
+// A more specific unvalidated row must not shadow a validated parent domain.
+func TestExtractClusterFromCustomDomains_ValidatedParentWinsOverUnvalidatedChild(t *testing.T) {
+	customDomains := []*domain.Domain{
+		{Domain: "example.com", TargetCluster: "cluster-generic", Validated: true},
+		{Domain: "app.example.com", TargetCluster: "cluster-app", Validated: false},
+	}
+
+	cluster, match := extractClusterFromCustomDomains("app.example.com", customDomains)
+	assert.Equal(t, customDomainValidated, match)
+	assert.Equal(t, "cluster-generic", cluster, "validated parent domain should provide the cluster")
+}
+
 func TestExtractClusterFromCustomDomains_OverlappingDomains(t *testing.T) {
 	customDomains := []*domain.Domain{
-		{Domain: "example.com", TargetCluster: "cluster-generic"},
-		{Domain: "app.example.com", TargetCluster: "cluster-app"},
+		{Domain: "example.com", TargetCluster: "cluster-generic", Validated: true},
+		{Domain: "app.example.com", TargetCluster: "cluster-app", Validated: true},
 	}
 
 	tests := []struct {
@@ -164,8 +194,8 @@ func TestExtractClusterFromCustomDomains_OverlappingDomains(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			cluster, ok := extractClusterFromCustomDomains(tc.domain, customDomains)
-			assert.True(t, ok)
+			cluster, match := extractClusterFromCustomDomains(tc.domain, customDomains)
+			assert.Equal(t, customDomainValidated, match)
 			assert.Equal(t, tc.wantVal, cluster)
 		})
 	}

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

@@ -26,6 +26,7 @@ type store interface {
 	GetAgentNetworkSettings(ctx context.Context, lockStrength nbstore.LockingStrength, accountID string) (*agentnetworkTypes.Settings, error)
 
 	GetCustomDomain(ctx context.Context, accountID string, domainID string) (*domain.Domain, error)
+	GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error)
 	ListFreeDomains(ctx context.Context, accountID string) ([]string, error)
 	ListCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
 	CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
@@ -150,6 +151,10 @@ func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName
 		return nil, fmt.Errorf("target cluster %s is not available", targetCluster)
 	}
 
+	if err := m.checkDomainAvailable(ctx, domainName); err != nil {
+		return nil, err
+	}
+
 	// Attempt an initial validation against the specified cluster only
 	var validated bool
 	if m.validator.IsValid(ctx, domainName, []string{targetCluster}) {
@@ -166,6 +171,23 @@ func (m Manager) CreateDomain(ctx context.Context, accountID, userID, domainName
 	return d, nil
 }
 
+// checkDomainAvailable reports whether the domain is free to claim. The unique
+// index on the column is the real guard; this turns the violation into a
+// conflict the caller can act on instead of a database error, and says nothing
+// about which account holds the domain.
+func (m Manager) checkDomainAvailable(ctx context.Context, domainName string) error {
+	_, err := m.store.GetCustomDomainByName(ctx, domainName)
+	if err == nil {
+		return status.Errorf(status.AlreadyExists, "domain %s is already registered", domainName)
+	}
+
+	if sErr, ok := status.FromError(err); ok && sErr.Type() == status.NotFound {
+		return nil
+	}
+
+	return fmt.Errorf("look up domain: %w", err)
+}
+
 func (m Manager) DeleteDomain(ctx context.Context, accountID, userID, domainID string) error {
 	ok, ctx, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete)
 	if err != nil {
@@ -203,7 +225,9 @@ func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID
 		log.WithFields(log.Fields{
 			"accountID": accountID,
 			"domainID":  domainID,
-		}).WithError(err).Error("validate domain")
+			"userID":    userID,
+		}).Error("validate domain: permission denied")
+		return
 	}
 
 	log.WithFields(log.Fields{
@@ -298,9 +322,12 @@ func (m Manager) DeriveClusterFromDomain(ctx context.Context, accountID, domain
 		return "", fmt.Errorf("list custom domains: %w", err)
 	}
 
-	targetCluster, valid := extractClusterFromCustomDomains(domain, customDomains)
-	if valid {
+	targetCluster, match := extractClusterFromCustomDomains(domain, customDomains)
+	switch match {
+	case customDomainValidated:
 		return targetCluster, nil
+	case customDomainUnvalidated:
+		return "", status.Errorf(status.PreconditionFailed, "domain %s is not validated", domain)
 	}
 
 	return "", fmt.Errorf("domain %s does not match any available proxy cluster", domain)
@@ -363,19 +390,46 @@ func (m Manager) reservedGatewayAddress(ctx context.Context, accountID string) (
 	return settings.ProxyAddress, nil
 }
 
-func extractClusterFromCustomDomains(serviceDomain string, customDomains []*domain.Domain) (string, bool) {
+// customDomainMatch describes how a service domain relates to the account's
+// custom domain rows.
+type customDomainMatch int
+
+const (
+	customDomainNoMatch customDomainMatch = iota
+	customDomainUnvalidated
+	customDomainValidated
+)
+
+// extractClusterFromCustomDomains finds the longest custom domain covering the
+// service domain and reports its target cluster. Only a validated row yields a
+// cluster: until the CNAME check has passed the account has not shown it
+// controls the name, so no traffic may be routed for it.
+func extractClusterFromCustomDomains(serviceDomain string, customDomains []*domain.Domain) (string, customDomainMatch) {
 	bestCluster := ""
 	bestLen := -1
+	matched := false
 	for _, cd := range customDomains {
 		if serviceDomain != cd.Domain && !strings.HasSuffix(serviceDomain, "."+cd.Domain) {
 			continue
 		}
+		matched = true
+		if !cd.Validated {
+			continue
+		}
 		if l := len(cd.Domain); l > bestLen {
 			bestLen = l
 			bestCluster = cd.TargetCluster
 		}
 	}
-	return bestCluster, bestLen >= 0
+
+	switch {
+	case bestLen >= 0:
+		return bestCluster, customDomainValidated
+	case matched:
+		return "", customDomainUnvalidated
+	default:
+		return "", customDomainNoMatch
+	}
 }
 
 // ExtractClusterFromFreeDomain extracts the cluster address from a free domain.

management/internals/modules/reverseproxy/domain/manager/manager_realstore_test.go

@@ -0,0 +1,326 @@
+package manager
+
+import (
+	"context"
+	"fmt"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel/metric/noop"
+
+	"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+	proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager"
+	"github.com/netbirdio/netbird/management/server/activity"
+	"github.com/netbirdio/netbird/management/server/mock_server"
+	"github.com/netbirdio/netbird/management/server/permissions"
+	nbstore "github.com/netbirdio/netbird/management/server/store"
+	"github.com/netbirdio/netbird/management/server/types"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+const (
+	testCluster    = "eu.proxy.test"
+	accountA       = "account-a"
+	accountAUser   = "account-a-admin"
+	accountB       = "account-b"
+	accountBUser   = "account-b-admin"
+	accountAMember = "account-a-member"
+)
+
+// stubResolver answers CNAME lookups from a table the test controls, so a
+// domain can point at the cluster or nowhere without touching a real resolver.
+type stubResolver struct {
+	mu     sync.Mutex
+	cnames map[string]string
+}
+
+func (r *stubResolver) LookupCNAME(_ context.Context, host string) (string, error) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	cname, ok := r.cnames[host]
+	if !ok {
+		return "", fmt.Errorf("lookup %s: no such host", host)
+	}
+	return cname + ".", nil
+}
+
+func (r *stubResolver) set(host, cname string) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	r.cnames[host] = cname
+}
+
+type domainTestEnv struct {
+	manager  Manager
+	store    nbstore.Store
+	resolver *stubResolver
+}
+
+// setupDomainTest builds the domain manager on a real SQLite store with two
+// accounts and one active public proxy cluster.
+func setupDomainTest(t *testing.T) *domainTestEnv {
+	t.Helper()
+
+	ctx := context.Background()
+	testStore, cleanup, err := nbstore.NewTestStoreFromSQL(ctx, "", t.TempDir())
+	require.NoError(t, err)
+	t.Cleanup(cleanup)
+
+	for accountID, userID := range map[string]string{accountA: accountAUser, accountB: accountBUser} {
+		users := map[string]*types.User{
+			userID: {
+				Id:        userID,
+				AccountID: accountID,
+				Role:      types.UserRoleAdmin,
+			},
+		}
+		if accountID == accountA {
+			// A real member of the account whose role denies Services:Create, so
+			// permission denial is exercised as ok=false rather than as a lookup
+			// error for a user who is not in the account at all.
+			users[accountAMember] = &types.User{
+				Id:        accountAMember,
+				AccountID: accountID,
+				Role:      types.UserRoleUser,
+			}
+		}
+
+		require.NoError(t, testStore.SaveAccount(ctx, &types.Account{
+			Id:        accountID,
+			CreatedBy: userID,
+			Settings:  &types.Settings{},
+			Users:     users,
+		}))
+	}
+
+	proxyMgr, err := proxymanager.NewManager(testStore, noop.NewMeterProvider().Meter(""))
+	require.NoError(t, err)
+
+	_, err = proxyMgr.Connect(ctx, "proxy-1", "session-1", testCluster, "127.0.0.1", nil, nil)
+	require.NoError(t, err)
+
+	resolver := &stubResolver{cnames: make(map[string]string)}
+
+	mgr := Manager{
+		store:              testStore,
+		proxyManager:       proxyMgr,
+		validator:          domain.Validator{Resolver: resolver},
+		permissionsManager: permissions.NewManager(testStore),
+		accountManager: &mock_server.MockAccountManager{
+			StoreEventFunc: func(context.Context, string, string, string, activity.ActivityDescriber, map[string]any) {},
+		},
+	}
+
+	return &domainTestEnv{manager: mgr, store: testStore, resolver: resolver}
+}
+
+// storedDomain reads a domain row back through the store so assertions are made
+// on what was persisted rather than on the value the manager returned.
+func storedDomain(t *testing.T, s nbstore.Store, accountID, domainName string) *domain.Domain {
+	t.Helper()
+
+	domains, err := s.ListCustomDomains(context.Background(), accountID)
+	require.NoError(t, err)
+	for _, d := range domains {
+		if d.Domain == domainName {
+			return d
+		}
+	}
+	return nil
+}
+
+// A domain whose CNAME check fails is stored unvalidated and must not resolve a
+// cluster, which is what service creation gates on.
+func TestCreateDomain_FailedLookupIsNotServable(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "apps.example.com", testCluster)
+	require.NoError(t, err)
+	assert.False(t, created.Validated, "a domain whose CNAME lookup fails must not be created validated")
+
+	stored := storedDomain(t, env.store, accountA, "apps.example.com")
+	require.NotNil(t, stored, "domain row should exist")
+	assert.False(t, stored.Validated, "persisted row must be unvalidated")
+
+	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "apps.example.com")
+	require.Error(t, err, "an unvalidated domain must not resolve a cluster")
+	assert.Empty(t, cluster)
+	assert.Contains(t, err.Error(), "not validated", "error should tell the caller what to fix")
+
+	sErr, ok := status.FromError(err)
+	require.True(t, ok, "error should be a typed status error")
+	assert.Equal(t, status.PreconditionFailed, sErr.Type())
+
+	_, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "sub.apps.example.com")
+	assert.Error(t, err, "subdomains of an unvalidated custom domain are not servable either")
+}
+
+// A second account claiming a registered domain gets a clean conflict, not a
+// database error surfaced as a 500.
+func TestCreateDomain_DuplicateIsAConflict(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	_, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "shared.example.com", testCluster)
+	require.NoError(t, err)
+
+	_, err = env.manager.CreateDomain(ctx, accountB, accountBUser, "shared.example.com", testCluster)
+	require.Error(t, err)
+
+	sErr, ok := status.FromError(err)
+	require.True(t, ok, "conflict must be a typed status error, not a raw database error")
+	assert.Equal(t, status.AlreadyExists, sErr.Type(), "conflict should map to 409, not 500")
+	assert.NotContains(t, sErr.Message, accountA, "the response must not reveal the holding account")
+
+	assert.Nil(t, storedDomain(t, env.store, accountB, "shared.example.com"), "no row should be written on conflict")
+}
+
+// The same account re-adding one of its own domains is a conflict too.
+func TestCreateDomain_SameAccountDuplicateIsAConflict(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	_, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "dup.example.com", testCluster)
+	require.NoError(t, err)
+
+	_, err = env.manager.CreateDomain(ctx, accountA, accountAUser, "dup.example.com", testCluster)
+	require.Error(t, err)
+
+	sErr, ok := status.FromError(err)
+	require.True(t, ok)
+	assert.Equal(t, status.AlreadyExists, sErr.Type())
+}
+
+// The negative control: a validated domain still derives its cluster, for the
+// bare name and for subdomains, exactly as before.
+func TestCreateDomain_ValidatedDomainDerivesCluster(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+	env.resolver.set("validation.valid.example.com", testCluster)
+
+	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "valid.example.com", testCluster)
+	require.NoError(t, err)
+	require.True(t, created.Validated, "a matching CNAME should validate on create")
+
+	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "valid.example.com")
+	require.NoError(t, err)
+	assert.Equal(t, testCluster, cluster)
+
+	cluster, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "app.valid.example.com")
+	require.NoError(t, err)
+	assert.Equal(t, testCluster, cluster, "subdomains of a validated custom domain resolve too")
+}
+
+// Validating a domain flips the gate: the same lookup that failed before now
+// resolves a cluster.
+func TestValidateDomain_UnlocksClusterDerivation(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "later.example.com", testCluster)
+	require.NoError(t, err)
+	require.False(t, created.Validated)
+
+	_, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "later.example.com")
+	require.Error(t, err)
+
+	env.resolver.set("validation.later.example.com", testCluster)
+	env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID)
+
+	require.True(t, storedDomain(t, env.store, accountA, "later.example.com").Validated)
+
+	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "later.example.com")
+	require.NoError(t, err)
+	assert.Equal(t, testCluster, cluster)
+}
+
+// Free cluster domains are unaffected by the custom domain gate.
+func TestDeriveClusterFromDomain_FreeDomainUnaffected(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	cluster, err := env.manager.DeriveClusterFromDomain(ctx, accountA, "myapp.abc123."+testCluster)
+	require.NoError(t, err)
+	assert.Equal(t, testCluster, cluster)
+}
+
+// The manager pre-check exists to turn a conflict into a 409, but the unique
+// index on the column is what actually guarantees the domain is claimed once.
+//
+// Two requests can clear the pre-check concurrently and race to the insert.
+// Inserting twice through the store reaches the same code path the loser of
+// that race takes, without the nondeterminism of driving it from goroutines,
+// and the loser must still see a conflict rather than an internal error.
+func TestStore_DuplicateDomainRejectedByIndexAsConflict(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	_, err := env.store.CreateCustomDomain(ctx, accountA, "indexed.example.com", testCluster, false)
+	require.NoError(t, err)
+
+	_, err = env.store.CreateCustomDomain(ctx, accountB, "indexed.example.com", testCluster, false)
+	require.Error(t, err, "the unique index must reject the same domain in a second account")
+
+	sErr, ok := status.FromError(err)
+	require.True(t, ok, "the losing insert must return a typed status error")
+	assert.Equal(t, status.AlreadyExists, sErr.Type(), "a lost race is a 409, not a 500")
+}
+
+// Validation is what decides whether a domain routes traffic, so a caller
+// without permission to it must not be able to flip the flag. The check logged
+// the denial and then carried on, which was inert while nothing read Validated
+// and is not once cluster derivation gates on it.
+func TestValidateDomain_PermissionDeniedDoesNotValidate(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "guarded.example.com", testCluster)
+	require.NoError(t, err)
+	require.False(t, created.Validated)
+
+	// The CNAME is in place, so the only thing standing between this caller and
+	// a validated domain is the permission check.
+	env.resolver.set("validation.guarded.example.com", testCluster)
+
+	env.manager.ValidateDomain(ctx, accountA, accountAMember, created.ID)
+
+	stored := storedDomain(t, env.store, accountA, "guarded.example.com")
+	require.NotNil(t, stored)
+	assert.False(t, stored.Validated, "a caller without permission must not validate the domain")
+
+	_, err = env.manager.DeriveClusterFromDomain(ctx, accountA, "guarded.example.com")
+	assert.Error(t, err, "the domain must still be unservable")
+}
+
+// Validation runs asynchronously, so it can finish after the domain was
+// deleted and then write a stale row back. gorm's Save falls back to an insert
+// when an update affects no rows, which would resurrect the domain as
+// validated; UpdateCustomDomain avoids that by selecting explicit columns.
+// This pins that behaviour, since dropping the Select would reintroduce it.
+func TestUpdateCustomDomain_DoesNotResurrectDeletedDomain(t *testing.T) {
+	ctx := context.Background()
+	env := setupDomainTest(t)
+
+	created, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "racy.example.com", testCluster)
+	require.NoError(t, err)
+
+	stale := storedDomain(t, env.store, accountA, "racy.example.com")
+	require.NotNil(t, stale)
+
+	require.NoError(t, env.manager.DeleteDomain(ctx, accountA, accountAUser, created.ID))
+	require.Nil(t, storedDomain(t, env.store, accountA, "racy.example.com"), "the domain should be gone")
+
+	// What an in-flight validation would write once its CNAME check succeeded.
+	// The write has to succeed for the assertion below to mean anything: a
+	// rejected write would leave the domain absent for the wrong reason.
+	stale.Validated = true
+	_, err = env.store.UpdateCustomDomain(ctx, accountA, stale)
+	require.NoError(t, err, "the update itself must succeed, so absence is not just a failed write")
+
+	assert.Nil(t, storedDomain(t, env.store, accountA, "racy.example.com"),
+		"a late validation write must not recreate a deleted domain")
+}

management/internals/modules/reverseproxy/domain/manager/manager_test.go

@@ -184,6 +184,10 @@ func (s *stubStore) GetCustomDomain(context.Context, string, string) (*domain.Do
 	panic("not used in allow-list tests")
 }
 
+func (s *stubStore) GetCustomDomainByName(context.Context, string) (*domain.Domain, error) {
+	panic("not used in allow-list tests")
+}
+
 func (s *stubStore) ListFreeDomains(context.Context, string) ([]string, error) {
 	panic("not used in allow-list tests")
 }

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

@@ -0,0 +1,127 @@
+package manager
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel/metric/noop"
+
+	domainmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain/manager"
+	proxymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy/manager"
+	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+	"github.com/netbirdio/netbird/management/server/activity"
+	"github.com/netbirdio/netbird/management/server/mock_server"
+	"github.com/netbirdio/netbird/management/server/permissions"
+	"github.com/netbirdio/netbird/management/server/store"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+const validationTestCluster = "eu.proxy.test"
+
+// withRealDomainManager swaps the stub cluster deriver for the real domain
+// manager backed by the same store, so service creation is gated by the actual
+// domain rows rather than by a test double that always agrees.
+func withRealDomainManager(t *testing.T, mgr *Manager, testStore store.Store) {
+	t.Helper()
+
+	ctx := context.Background()
+	proxyMgr, err := proxymanager.NewManager(testStore, noop.NewMeterProvider().Meter(""))
+	require.NoError(t, err)
+
+	_, err = proxyMgr.Connect(ctx, "proxy-1", "session-1", validationTestCluster, "127.0.0.1", nil, nil)
+	require.NoError(t, err)
+
+	accountMgr := &mock_server.MockAccountManager{
+		StoreEventFunc: func(context.Context, string, string, string, activity.ActivityDescriber, map[string]any) {},
+	}
+	mgr.clusterDeriver = domainmanager.NewManager(testStore, proxyMgr, permissions.NewManager(testStore), accountMgr)
+}
+
+func newTestService(domain string) *rpservice.Service {
+	return &rpservice.Service{
+		Name:    "test-service",
+		Domain:  domain,
+		Enabled: true,
+		Mode:    rpservice.ModeHTTP,
+		Targets: []*rpservice.Target{{
+			Host:       "10.0.0.1",
+			Port:       8080,
+			Protocol:   "http",
+			TargetId:   testPeerID,
+			TargetType: "peer",
+			Enabled:    true,
+		}},
+	}
+}
+
+// A service must not bind to a domain the account has not validated, and
+// nothing may be persisted for the attempt.
+func TestCreateService_RefusesUnvalidatedDomain(t *testing.T) {
+	ctx := context.Background()
+	mgr, testStore := setupIntegrationTest(t)
+	withRealDomainManager(t, mgr, testStore)
+
+	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "unproven.example.com", validationTestCluster, false)
+	require.NoError(t, err)
+
+	_, err = mgr.CreateService(ctx, testAccountID, testUserID, newTestService("unproven.example.com"))
+	require.Error(t, err, "an unvalidated domain must not bind a service")
+	assert.Contains(t, err.Error(), "not validated", "the API error should name the actual problem")
+
+	sErr, ok := status.FromError(err)
+	require.True(t, ok, "error should be a typed status error")
+	assert.Equal(t, status.PreconditionFailed, sErr.Type())
+
+	services, err := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
+	require.NoError(t, err)
+	assert.Empty(t, services, "no service row should be written for a refused domain")
+}
+
+// The negative control: a validated domain still binds a service and derives
+// its cluster exactly as before.
+func TestCreateService_ValidatedDomainBindsService(t *testing.T) {
+	ctx := context.Background()
+	mgr, testStore := setupIntegrationTest(t)
+	withRealDomainManager(t, mgr, testStore)
+
+	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "proven.example.com", validationTestCluster, true)
+	require.NoError(t, err)
+
+	created, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.proven.example.com"))
+	require.NoError(t, err)
+	assert.Equal(t, validationTestCluster, created.ProxyCluster, "service should bind to the domain's target cluster")
+
+	services, err := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, services, 1, "the service should be persisted")
+	assert.Equal(t, "app.proven.example.com", services[0].Domain)
+}
+
+// An update must not be a way around the creation gate: moving a live service
+// onto an unvalidated domain has to fail rather than silently keep the old
+// cluster and start serving the new hostname.
+func TestUpdateService_RefusesMoveToUnvalidatedDomain(t *testing.T) {
+	ctx := context.Background()
+	mgr, testStore := setupIntegrationTest(t)
+	withRealDomainManager(t, mgr, testStore)
+
+	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "proven.example.com", validationTestCluster, true)
+	require.NoError(t, err)
+	_, err = testStore.CreateCustomDomain(ctx, testAccountID, "unproven.example.com", validationTestCluster, false)
+	require.NoError(t, err)
+
+	created, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.proven.example.com"))
+	require.NoError(t, err)
+
+	moved := *created
+	moved.Domain = "app.unproven.example.com"
+	_, err = mgr.UpdateService(ctx, testAccountID, testUserID, &moved)
+	require.Error(t, err, "moving to an unvalidated domain must fail")
+	assert.Contains(t, err.Error(), "not validated")
+
+	stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, created.ID)
+	require.NoError(t, err)
+	assert.Equal(t, "app.proven.example.com", stored.Domain, "the service must keep its original domain")
+}

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

@@ -606,16 +606,19 @@ func (m *Manager) resolveEffectiveCluster(ctx context.Context, accountID string,
 		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
-		}
+	if m.clusterDeriver == nil {
+		return existing.ProxyCluster, nil
 	}
 
-	return existing.ProxyCluster, nil
+	// Falling back to the old cluster here would let an update move a service
+	// onto a domain the account has not validated, bypassing the check that
+	// creation makes.
+	derived, err := m.clusterDeriver.DeriveClusterFromDomain(ctx, accountID, svc.Domain)
+	if err != nil {
+		return "", status.Errorf(status.PreconditionFailed, "could not derive cluster from domain %s: %v", svc.Domain, err)
+	}
+
+	return derived, nil
 }
 
 func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.Store, accountID string, service *service.Service, updateInfo *serviceUpdateInfo, customPorts *bool, effectiveCluster string) error {

management/internals/server/boot.go

@@ -31,6 +31,7 @@ import (
 	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
 	networkmapdb "github.com/netbirdio/netbird/management/internals/network_map_db"
 	networkmapdbfactory "github.com/netbirdio/netbird/management/internals/network_map_db/factory"
+	nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
 	nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
 	"github.com/netbirdio/netbird/management/server/activity"
 	activitystore "github.com/netbirdio/netbird/management/server/activity/store"
@@ -111,7 +112,8 @@ func (s *BaseServer) NetworkMapStore() *networkmapdb.NetworkMapDBStoreImpl {
 			s.Config.StoreConfig.Engine,
 			s.Config.Datadir,
 			s.IntegratedValidator(),
-			s.SettingsManager())
+			s.SettingsManager(),
+		)
 		// networkmap db store supports postgres and sqlite backends only
 		// for other backends a fallback is used, so NotSupportedStoreEngineError
 		// is not a fatal error
@@ -180,24 +182,7 @@ func (s *BaseServer) RateLimiter() *middleware.APIRateLimiter {
 
 func (s *BaseServer) GRPCServer() *grpc.Server {
 	return Create(s, func() *grpc.Server {
-		trustedPeers := s.Config.ReverseProxy.TrustedPeers
-		defaultTrustedPeers := []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0")}
-		if len(trustedPeers) == 0 || slices.Equal[[]netip.Prefix](trustedPeers, defaultTrustedPeers) {
-			log.WithContext(context.Background()).Warn("TrustedPeers are configured to default value '0.0.0.0/0', '::/0'. This allows connection IP spoofing.")
-			trustedPeers = defaultTrustedPeers
-		}
-		trustedHTTPProxies := s.Config.ReverseProxy.TrustedHTTPProxies
-		trustedProxiesCount := s.Config.ReverseProxy.TrustedHTTPProxiesCount
-		if len(trustedHTTPProxies) > 0 && trustedProxiesCount > 0 {
-			log.WithContext(context.Background()).Warn("TrustedHTTPProxies and TrustedHTTPProxiesCount both are configured. " +
-				"This is not recommended way to extract X-Forwarded-For. Consider using one of these options.")
-		}
-		realipOpts := []realip.Option{
-			realip.WithTrustedPeers(trustedPeers),
-			realip.WithTrustedProxies(trustedHTTPProxies),
-			realip.WithTrustedProxiesCount(trustedProxiesCount),
-			realip.WithHeaders([]string{realip.XForwardedFor, realip.XRealIp}),
-		}
+		realipOpts := realIPOptions(s.Config.ReverseProxy)
 		proxyUnary, proxyStream, proxyAuthClose := nbgrpc.NewProxyAuthInterceptors(s.Store())
 		s.proxyAuthClose = proxyAuthClose
 		gRPCOpts := []grpc.ServerOption{
@@ -333,7 +318,7 @@ func (s *BaseServer) AccessLogsManager() accesslogs.Manager {
 	})
 }
 
-func loadTLSConfig(certFile string, certKey string) (*tls.Config, error) {
+func loadTLSConfig(certFile, certKey string) (*tls.Config, error) {
 	// Load server's certificate and private key
 	serverCert, err := tls.LoadX509KeyPair(certFile, certKey)
 	if err != nil {
@@ -380,3 +365,34 @@ func streamInterceptor(
 	wrapped.WrappedContext = context.WithValue(ctx, nbContext.RequestIDKey, reqID)
 	return handler(srv, wrapped)
 }
+
+// realIPOptions builds the real-IP middleware options from the reverse proxy config.
+//
+// TrustedPeers controls which transport peers are allowed to supply forwarded-IP
+// headers. If empty, forwarded headers are ignored and the transport peer address
+// is used directly. Operators terminating connections at a reverse proxy should
+// configure TrustedPeers with that proxy's address or network.
+//
+// Only X-Forwarded-For is trusted. X-Real-IP contains a single client-supplied
+// address with no proxy chain to validate, and none of the reverse proxies we ship
+// use it on the gRPC path.
+func realIPOptions(cfg nbconfig.ReverseProxy) []realip.Option {
+	if idx := slices.IndexFunc(cfg.TrustedPeers, func(p netip.Prefix) bool { return p.Bits() == 0 }); idx >= 0 {
+		log.WithContext(context.Background()).Warnf("TrustedPeers contains the default route %s, which trusts "+
+			"X-Forwarded-For from every client and allows connection IP spoofing. Set TrustedPeers to the address "+
+			"of your reverse proxy, or leave it empty to use the connection's source address.", cfg.TrustedPeers[idx])
+	}
+	if cfg.TrustedHTTPProxiesCount > 0 {
+		log.WithContext(context.Background()).Warn(
+			"TrustedHTTPProxiesCount skips X-Forwarded-For entries by position before TrustedHTTPProxies filters by address. " +
+				"An incorrect count may skip the real client IP and produce an incorrect source address.",
+		)
+	}
+
+	return []realip.Option{
+		realip.WithTrustedPeers(cfg.TrustedPeers),
+		realip.WithTrustedProxies(cfg.TrustedHTTPProxies),
+		realip.WithTrustedProxiesCount(cfg.TrustedHTTPProxiesCount),
+		realip.WithHeaders([]string{realip.XForwardedFor}),
+	}
+}

management/internals/server/realip_test.go

@@ -0,0 +1,171 @@
+package server
+
+import (
+	"context"
+	"io"
+	"net"
+	"net/netip"
+	"testing"
+	"time"
+
+	"github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/realip"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/metadata"
+	"google.golang.org/protobuf/types/known/emptypb"
+
+	nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
+)
+
+const (
+	realIPProbeMethod       = "/netbird.test.RealIPProbe/Probe"
+	realIPProbeStreamMethod = "/netbird.test.RealIPProbe/ProbeStream"
+)
+
+// realIPProbe records the real IP the middleware derived for each call.
+type realIPProbe struct {
+	got chan string
+}
+
+func (p *realIPProbe) record(ctx context.Context) {
+	addr, _ := realip.FromContext(ctx)
+	p.got <- addr.String()
+}
+
+func (p *realIPProbe) wait(t *testing.T) string {
+	t.Helper()
+
+	select {
+	case got := <-p.got:
+		return got
+	case <-time.After(5 * time.Second):
+		t.Fatal("timed out waiting for probe")
+		return ""
+	}
+}
+
+func startProbeServer(t *testing.T, cfg nbconfig.ReverseProxy) (*grpc.ClientConn, *realIPProbe) {
+	t.Helper()
+
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	probe := &realIPProbe{got: make(chan string, 1)}
+	opts := realIPOptions(cfg)
+	srv := grpc.NewServer(
+		grpc.ChainUnaryInterceptor(realip.UnaryServerInterceptorOpts(opts...)),
+		grpc.ChainStreamInterceptor(realip.StreamServerInterceptorOpts(opts...)),
+	)
+	srv.RegisterService(&grpc.ServiceDesc{
+		ServiceName: "netbird.test.RealIPProbe",
+		HandlerType: (*any)(nil),
+		Methods: []grpc.MethodDesc{{
+			MethodName: "Probe",
+			Handler: func(_ any, ctx context.Context, dec func(any) error, interceptor grpc.UnaryServerInterceptor) (any, error) {
+				req := new(emptypb.Empty)
+				if err := dec(req); err != nil {
+					return nil, err
+				}
+				handler := func(ctx context.Context, _ any) (any, error) {
+					probe.record(ctx)
+					return &emptypb.Empty{}, nil
+				}
+				if interceptor == nil {
+					return handler(ctx, req)
+				}
+				return interceptor(ctx, req, &grpc.UnaryServerInfo{FullMethod: realIPProbeMethod}, handler)
+			},
+		}},
+		Streams: []grpc.StreamDesc{{
+			StreamName:    "ProbeStream",
+			ServerStreams: true,
+			Handler: func(_ any, stream grpc.ServerStream) error {
+				probe.record(stream.Context())
+				return nil
+			},
+		}},
+	}, probe)
+
+	go func() { _ = srv.Serve(listener) }()
+	t.Cleanup(srv.Stop)
+
+	conn, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = conn.Close() })
+
+	return conn, probe
+}
+
+func callUnary(t *testing.T, conn *grpc.ClientConn, probe *realIPProbe, kv ...string) string {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ctx = metadata.AppendToOutgoingContext(ctx, kv...)
+	require.NoError(t, conn.Invoke(ctx, realIPProbeMethod, &emptypb.Empty{}, &emptypb.Empty{}))
+
+	return probe.wait(t)
+}
+
+func callStream(t *testing.T, conn *grpc.ClientConn, probe *realIPProbe, kv ...string) string {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ctx = metadata.AppendToOutgoingContext(ctx, kv...)
+	desc := &grpc.StreamDesc{StreamName: "ProbeStream", ServerStreams: true}
+	stream, err := conn.NewStream(ctx, desc, realIPProbeStreamMethod)
+	require.NoError(t, err)
+	require.NoError(t, stream.CloseSend())
+	require.ErrorIs(t, stream.RecvMsg(&emptypb.Empty{}), io.EOF)
+
+	return probe.wait(t)
+}
+
+func assertRealIP(t *testing.T, cfg nbconfig.ReverseProxy, want string, kv ...string) {
+	t.Helper()
+
+	conn, probe := startProbeServer(t, cfg)
+	t.Run("unary", func(t *testing.T) {
+		assert.Equal(t, want, callUnary(t, conn, probe, kv...))
+	})
+	t.Run("stream", func(t *testing.T) {
+		assert.Equal(t, want, callStream(t, conn, probe, kv...))
+	})
+}
+
+func TestRealIPDefaultIgnoresClientForwardedHeaders(t *testing.T) {
+	assertRealIP(t, nbconfig.ReverseProxy{}, "127.0.0.1",
+		realip.XForwardedFor, "203.0.113.44",
+		realip.XRealIp, "203.0.113.44",
+	)
+}
+
+func TestRealIPUntrustedPeerIgnoresForwardedHeaders(t *testing.T) {
+	cfg := nbconfig.ReverseProxy{TrustedPeers: []netip.Prefix{netip.MustParsePrefix("10.9.8.7/32")}}
+
+	assertRealIP(t, cfg, "127.0.0.1",
+		realip.XForwardedFor, "203.0.113.44",
+		realip.XRealIp, "203.0.113.44",
+	)
+}
+
+func TestRealIPTrustedPeerHonoursForwardedHeaders(t *testing.T) {
+	cfg := nbconfig.ReverseProxy{TrustedPeers: []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}}
+
+	assertRealIP(t, cfg, "203.0.113.44",
+		realip.XForwardedFor, "203.0.113.44",
+		realip.XRealIp, "203.0.113.44",
+	)
+}
+
+func TestRealIPIgnoresXRealIPWhenProxyCountIsSet(t *testing.T) {
+	cfg := nbconfig.ReverseProxy{
+		TrustedPeers:            []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")},
+		TrustedHTTPProxiesCount: 1,
+	}
+
+	assertRealIP(t, cfg, "127.0.0.1", realip.XRealIp, "203.0.113.44")
+}

management/internals/server/server.go

@@ -74,6 +74,7 @@ type BaseServer struct {
 	grpcExtensions []GRPCExtension
 
 	listener    net.Listener
+	tlsConfig   *tls.Config
 	certManager *autocert.Manager
 	update      *version.Update
 
@@ -94,6 +95,7 @@ type Config struct {
 	DisableGeoliteUpdate        bool
 	UserDeleteFromIDPEnabled    bool
 	AutoResolveDomains          bool
+	TLSConfig                   *tls.Config
 }
 
 // NewServer initializes and configures a new Server instance
@@ -110,6 +112,7 @@ func NewServer(cfg *Config) *BaseServer {
 		disableLegacyManagementPort: cfg.DisableLegacyManagementPort,
 		mgmtMetricsPort:             cfg.MgmtMetricsPort,
 		autoResolveDomains:          cfg.AutoResolveDomains,
+		tlsConfig:                   cfg.TLSConfig,
 	}
 	s.container[ContainerKeyBaseServer] = s
 
@@ -139,21 +142,9 @@ func (s *BaseServer) Start(ctx context.Context) error {
 	}
 	s.EphemeralManager().LoadInitialPeers(srvCtx)
 
-	var tlsConfig *tls.Config
-	tlsEnabled := false
-	if s.Config.HttpConfig.LetsEncryptDomain != "" {
-		s.certManager, err = encryption.CreateCertManager(s.Config.Datadir, s.Config.HttpConfig.LetsEncryptDomain)
-		if err != nil {
-			return fmt.Errorf("failed creating LetsEncrypt cert manager: %v", err)
-		}
-		tlsEnabled = true
-	} else if s.Config.HttpConfig.CertFile != "" && s.Config.HttpConfig.CertKey != "" {
-		tlsConfig, err = loadTLSConfig(s.Config.HttpConfig.CertFile, s.Config.HttpConfig.CertKey)
-		if err != nil {
-			log.WithContext(srvCtx).Errorf("cannot load TLS credentials: %v", err)
-			return err
-		}
-		tlsEnabled = true
+	tlsEnabled, err := s.setupTLS(srvCtx)
+	if err != nil {
+		return err
 	}
 
 	installationID, err := getInstallationID(srvCtx, s.Store())
@@ -215,8 +206,8 @@ func (s *BaseServer) Start(ctx context.Context) error {
 			log.WithContext(ctx).Infof("running HTTP server (LetsEncrypt challenge handler): %s", cml.Addr().String())
 			s.serveHTTP(ctx, cml, s.certManager.HTTPHandler(nil))
 		}
-	case tlsConfig != nil:
-		s.listener, err = tls.Listen("tcp", fmt.Sprintf(":%d", s.mgmtPort), tlsConfig)
+	case s.tlsConfig != nil:
+		s.listener, err = tls.Listen("tcp", fmt.Sprintf(":%d", s.mgmtPort), s.tlsConfig)
 		if err != nil {
 			return fmt.Errorf("failed creating TLS listener on port %d: %v", s.mgmtPort, err)
 		}
@@ -240,6 +231,31 @@ func (s *BaseServer) Start(ctx context.Context) error {
 	return nil
 }
 
+// setupTLS resolves the listener's TLS source: an injected config wins over the HttpConfig certificate settings
+func (s *BaseServer) setupTLS(ctx context.Context) (bool, error) {
+	switch {
+	case s.tlsConfig != nil:
+		return true, nil
+	case s.Config.HttpConfig.LetsEncryptDomain != "":
+		certManager, err := encryption.CreateCertManager(s.Config.Datadir, s.Config.HttpConfig.LetsEncryptDomain)
+		if err != nil {
+			return false, fmt.Errorf("failed creating LetsEncrypt cert manager: %v", err)
+		}
+		s.certManager = certManager
+		return true, nil
+	case s.Config.HttpConfig.CertFile != "" && s.Config.HttpConfig.CertKey != "":
+		tlsConfig, err := loadTLSConfig(s.Config.HttpConfig.CertFile, s.Config.HttpConfig.CertKey)
+		if err != nil {
+			log.WithContext(ctx).Errorf("cannot load TLS credentials: %v", err)
+			return false, err
+		}
+		s.tlsConfig = tlsConfig
+		return true, nil
+	default:
+		return false, nil
+	}
+}
+
 // Stop attempts a graceful shutdown, waiting up to 5 seconds for active connections to finish
 func (s *BaseServer) Stop() error {
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)