[management] Prevent deleting custom domains used by services

Keep domain registrations reserved while services use the domain or its subdomains. Reject deletion with a precondition error and recheck domain authorization under database locks before committing service writes.

docs/custom-domain-validation.md

@@ -26,3 +26,8 @@ window. Restarting management does not extend a previously assigned deadline.
 Registrations with existing services, including services using subdomains, are
 retained for operator review. Management logs their account and domain IDs so
 an operator can identify and resolve those dependencies before cleanup.
+
+Manual deletion is also refused while any service uses the domain or a subdomain,
+including disabled services. Delete those services or move them to another domain
+before removing the registration. A refused deletion returns HTTP 412 and leaves
+the domain and its services unchanged; no deletion activity event is recorded.

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

@@ -0,0 +1,78 @@
+package manager
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gorilla/mux"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+	"github.com/netbirdio/netbird/management/server/activity"
+	nbcontext "github.com/netbirdio/netbird/management/server/context"
+	nbstore "github.com/netbirdio/netbird/management/server/store"
+	"github.com/netbirdio/netbird/shared/auth"
+)
+
+func TestDeleteDomain_ServiceDependencies(t *testing.T) {
+	for _, tt := range []struct {
+		name        string
+		domainName  string
+		serviceHost string
+		accountID   string
+		enabled     bool
+		protected   bool
+	}{
+		{"exact", "example.com", "example.com", accountA, true, true},
+		{"subdomain", "example.com", "deep.app.example.com", accountA, true, true},
+		{"disabled", "example.com", "app.example.com", accountA, false, true},
+		{"other account", "example.com", "app.example.com", accountB, true, true},
+		{"case and trailing dot", "example.com", "APP.EXAMPLE.COM.", accountA, true, true},
+		{"suffix boundary", "example.com", "notexample.com", accountA, true, false},
+		{"literal underscore", "a_b.example.com", "app.a_b.example.com", accountA, true, true},
+		{"underscore wildcard", "a_b.example.com", "app.axb.example.com", accountA, true, false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			env := setupDomainTest(t)
+			events := captureDomainEvents(env)
+			d, err := env.store.CreateCustomDomain(ctx, accountA, tt.domainName, testCluster, true)
+			require.NoError(t, err)
+			svc := &rpservice.Service{
+				ID: "dependent", AccountID: tt.accountID, Domain: tt.serviceHost,
+				Enabled: tt.enabled, ProxyCluster: testCluster,
+			}
+			require.NoError(t, env.store.CreateService(ctx, svc))
+			router := mux.NewRouter()
+			RegisterEndpoints(router, env.manager)
+			deleteDomain := func() *httptest.ResponseRecorder {
+				req := httptest.NewRequest(http.MethodDelete, "/domains/"+d.ID, nil)
+				req = nbcontext.SetUserAuthInRequest(req, auth.UserAuth{AccountId: accountA, UserId: accountAUser})
+				response := httptest.NewRecorder()
+				router.ServeHTTP(response, req)
+				return response
+			}
+
+			response := deleteDomain()
+			if tt.protected {
+				require.Equal(t, http.StatusPreconditionFailed, response.Code, "dependent services must block deletion: %s", response.Body.String())
+				assert.NotContains(t, response.Body.String(), tt.accountID, "the error must not reveal the service's account")
+				assert.NotNil(t, storedDomain(t, env.store, accountA, d.Domain), "the namespace must remain reserved")
+				assert.Empty(t, events.get(), "rejected deletion must not emit DomainDeleted")
+				stored, err := env.store.GetServiceByID(ctx, nbstore.LockingStrengthNone, tt.accountID, svc.ID)
+				require.NoError(t, err)
+				assert.Equal(t, svc.Enabled, stored.Enabled, "rejected deletion must preserve the service")
+				require.NoError(t, env.store.DeleteService(ctx, tt.accountID, svc.ID))
+				response = deleteDomain()
+			}
+			require.Equal(t, http.StatusNoContent, response.Code, "deletion must succeed without dependencies: %s", response.Body.String())
+			assert.Nil(t, storedDomain(t, env.store, accountA, d.Domain), "the registration must be deleted")
+			captured := events.get()
+			require.Len(t, captured, 1, "only successful deletion may emit an event")
+			assert.Equal(t, activity.DomainDeleted, captured[0].Activity, "the event must describe the successful deletion")
+		})
+	}
+}

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

@@ -357,6 +357,22 @@ func (m Manager) DeriveClusterFromDomain(ctx context.Context, accountID, domain
 	return "", fmt.Errorf("domain %s does not match any available proxy cluster", domain)
 }
 
+// ValidateServiceDomain holds custom domain authorization through a service write transaction.
+func (m Manager) ValidateServiceDomain(ctx context.Context, tx nbstore.Store, accountID, serviceDomain, cluster string) error {
+	if _, ok := ExtractClusterFromFreeDomain(serviceDomain, []string{cluster}); ok {
+		return nil
+	}
+	customDomains, err := tx.LockCustomDomains(ctx, accountID)
+	if err != nil {
+		return err
+	}
+	target, match := extractClusterFromCustomDomains(serviceDomain, customDomains)
+	if match != customDomainValidated || target != cluster {
+		return status.Errorf(status.PreconditionFailed, "custom domain authorization changed; retry the service operation")
+	}
+	return nil
+}
+
 func (m Manager) getClusterAllowList(ctx context.Context, accountID string) ([]string, error) {
 	byopAddresses, err := m.proxyManager.GetActiveClusterAddressesForAccount(ctx, accountID)
 	if err != nil {

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

@@ -125,3 +125,53 @@ func TestUpdateService_RefusesMoveToUnvalidatedDomain(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "app.proven.example.com", stored.Domain, "the service must keep its original domain")
 }
+
+func TestCreateService_DomainDeletedBeforeWrite(t *testing.T) {
+	ctx := context.Background()
+	mgr, testStore := setupIntegrationTest(t)
+	withRealDomainManager(t, mgr, testStore)
+
+	d, err := testStore.CreateCustomDomain(ctx, testAccountID, "proven.example.com", validationTestCluster, true)
+	require.NoError(t, err)
+	svc := newTestService("app.proven.example.com")
+	require.NoError(t, mgr.initializeServiceForCreate(ctx, testAccountID, svc))
+
+	// Delete after the initial authorization check, before the service transaction starts.
+	require.NoError(t, testStore.DeleteCustomDomain(ctx, testAccountID, d.ID))
+	err = mgr.persistNewService(ctx, testAccountID, svc)
+	require.Error(t, err, "an earlier validation result must not authorize a deleted registration")
+	sErr, ok := status.FromError(err)
+	require.True(t, ok, "the caller must receive a typed precondition error")
+	assert.Equal(t, status.PreconditionFailed, sErr.Type(), "the service must require current domain authorization")
+	services, err := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
+	require.NoError(t, err)
+	assert.Empty(t, services, "the failed write must not leave a service")
+}
+
+func TestUpdateService_DomainDeletedBeforeWrite(t *testing.T) {
+	ctx := context.Background()
+	mgr, testStore := setupIntegrationTest(t)
+	withRealDomainManager(t, mgr, testStore)
+	_, err := testStore.CreateCustomDomain(ctx, testAccountID, "original.example.com", validationTestCluster, true)
+	require.NoError(t, err)
+	d, err := testStore.CreateCustomDomain(ctx, testAccountID, "destination.example.com", validationTestCluster, true)
+	require.NoError(t, err)
+	svc, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.original.example.com"))
+	require.NoError(t, err)
+	moved := svc.Copy()
+	moved.Domain = "app.destination.example.com"
+	cluster, err := mgr.resolveEffectiveCluster(ctx, testAccountID, moved)
+	require.NoError(t, err)
+
+	require.NoError(t, testStore.DeleteCustomDomain(ctx, testAccountID, d.ID))
+	err = testStore.ExecuteInTransaction(ctx, func(tx store.Store) error {
+		return mgr.executeServiceUpdate(ctx, tx, testAccountID, moved, &serviceUpdateInfo{}, nil, cluster)
+	})
+	require.Error(t, err, "a domain deleted after cluster resolution must reject the update")
+	sErr, ok := status.FromError(err)
+	require.True(t, ok, "the caller must receive a typed precondition error")
+	assert.Equal(t, status.PreconditionFailed, sErr.Type(), "the move must require current domain authorization")
+	stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, svc.ID)
+	require.NoError(t, err)
+	assert.Equal(t, svc.Domain, stored.Domain, "the service must retain its authorized domain")
+}

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

@@ -74,6 +74,7 @@ const unknownHostPlaceholder = "unknown"
 // ClusterDeriver derives the proxy cluster from a domain.
 type ClusterDeriver interface {
 	DeriveClusterFromDomain(ctx context.Context, accountID, domain string) (string, error)
+	ValidateServiceDomain(ctx context.Context, tx store.Store, accountID, domain, cluster string) error
 	GetClusterDomains() []string
 }
 
@@ -332,6 +333,9 @@ func (m *Manager) persistNewService(ctx context.Context, accountID string, svc *
 	}
 
 	return m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
+		if err := m.validateServiceDomain(ctx, transaction, accountID, svc, svc.ProxyCluster); err != nil {
+			return err
+		}
 		if svc.Domain != "" {
 			if err := m.checkDomainAvailable(ctx, transaction, svc.Domain, ""); err != nil {
 				return err
@@ -461,6 +465,9 @@ func (m *Manager) persistNewEphemeralService(ctx context.Context, accountID, pee
 	}
 
 	return m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
+		if err := m.validateServiceDomain(ctx, transaction, accountID, svc, svc.ProxyCluster); err != nil {
+			return err
+		}
 		if err := m.validateEphemeralPreconditions(ctx, transaction, accountID, peerID, svc); err != nil {
 			return err
 		}
@@ -622,6 +629,9 @@ func (m *Manager) resolveEffectiveCluster(ctx context.Context, accountID string,
 }
 
 func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.Store, accountID string, service *service.Service, updateInfo *serviceUpdateInfo, customPorts *bool, effectiveCluster string) error {
+	if err := m.validateServiceDomain(ctx, transaction, accountID, service, effectiveCluster); err != nil {
+		return err
+	}
 	existingService, err := transaction.GetServiceByID(ctx, store.LockingStrengthUpdate, accountID, service.ID)
 	if err != nil {
 		return err
@@ -677,6 +687,13 @@ func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.St
 	return nil
 }
 
+func (m *Manager) validateServiceDomain(ctx context.Context, tx store.Store, accountID string, svc *service.Service, cluster string) error {
+	if m.clusterDeriver == nil {
+		return nil
+	}
+	return m.clusterDeriver.ValidateServiceDomain(ctx, tx, accountID, svc.Domain, cluster)
+}
+
 // validateL4PortDiffOnClusterDiff checks if custom L4 ports are configured and validates port changes across clusters.
 // It ensures no port changes if custom ports are unsupported for a given cluster and protocol mode.
 // Returns an error if validation fails, otherwise returns nil.

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

@@ -655,6 +655,10 @@ func (d *testClusterDeriver) GetClusterDomains() []string {
 	return d.domains
 }
 
+func (d *testClusterDeriver) ValidateServiceDomain(context.Context, store.Store, string, string, string) error {
+	return nil
+}
+
 const (
 	testAccountID = "test-account"
 	testPeerID    = "test-peer-1"

management/server/store/sql_store.go

@@ -5757,20 +5757,6 @@ func (s *SqlStore) UpdateCustomDomain(ctx context.Context, accountID string, d *
 	return d, nil
 }
 
-func (s *SqlStore) DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error {
-	result := s.db.Delete(domain.Domain{}, accountAndIDQueryCondition, accountID, domainID)
-	if result.Error != nil {
-		log.WithContext(ctx).Errorf("failed to delete reverse proxy custom domain from store: %v", result.Error)
-		return status.Errorf(status.Internal, "failed to delete reverse proxy custom domain from store")
-	}
-
-	if result.RowsAffected == 0 {
-		return status.Errorf(status.NotFound, "reverse proxy custom domain %s not found", domainID)
-	}
-
-	return nil
-}
-
 // CreateAccessLog creates a new access log entry in the database
 func (s *SqlStore) CreateAccessLog(ctx context.Context, logEntry *accesslogs.AccessLogEntry) error {
 	result := s.db.Create(logEntry)

management/server/store/sql_store_domain_deletion.go

@@ -0,0 +1,49 @@
+package store
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	"gorm.io/gorm"
+	"gorm.io/gorm/clause"
+
+	"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+// LockCustomDomains locks an account's registrations until the caller's transaction ends.
+func (s *SqlStore) LockCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error) {
+	var domains []*domain.Domain
+	if err := s.db.WithContext(ctx).Clauses(clause.Locking{Strength: string(LockingStrengthUpdate)}).
+		Where(accountIDCondition, accountID).Order("id").Find(&domains).Error; err != nil {
+		return nil, fmt.Errorf("lock custom domains: %w", err)
+	}
+	return domains, nil
+}
+
+// DeleteCustomDomain removes a registration only when no service uses its namespace.
+func (s *SqlStore) DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error {
+	return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+		var d domain.Domain
+		// Service writes take the same lock before checking validation, so neither
+		// operation can commit against the other's outdated view of the domain.
+		if err := tx.Clauses(clause.Locking{Strength: string(LockingStrengthUpdate)}).
+			Take(&d, accountAndIDQueryCondition, accountID, domainID).Error; err != nil {
+			if errors.Is(err, gorm.ErrRecordNotFound) {
+				return status.Errorf(status.NotFound, "custom domain not found")
+			}
+			return fmt.Errorf("lock custom domain for deletion: %w", err)
+		}
+
+		result := tx.Where(accountAndIDQueryCondition, accountID, domainID).
+			Where("NOT EXISTS (?)", customDomainServices(tx, &d).Select("1")).Delete(&domain.Domain{})
+		if result.Error != nil {
+			return fmt.Errorf("delete custom domain: %w", result.Error)
+		}
+		if result.RowsAffected == 0 {
+			return status.Errorf(status.PreconditionFailed, "custom domain has dependent services; delete or move them before deleting the domain")
+		}
+		return nil
+	})
+}

management/server/store/sql_store_domain_deletion_test.go

@@ -0,0 +1,96 @@
+package store
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+func TestDeleteCustomDomain_ServiceDependencies(t *testing.T) {
+	runTestForAllEngines(t, "", func(t *testing.T, store Store) {
+		ctx := context.Background()
+		require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
+		d, err := store.CreateCustomDomain(ctx, "owner", "example.com", "cluster", true)
+		require.NoError(t, err)
+		svc := &rpservice.Service{ID: "service", AccountID: "owner", Domain: "APP.EXAMPLE.COM."}
+		require.NoError(t, store.CreateService(ctx, svc))
+
+		err = store.DeleteCustomDomain(ctx, "other", d.ID)
+		require.Error(t, err)
+		sErr, ok := status.FromError(err)
+		require.True(t, ok, "cross-account deletion must return a typed error")
+		assert.Equal(t, status.NotFound, sErr.Type(), "cross-account deletion must not reveal dependencies")
+
+		err = store.DeleteCustomDomain(ctx, "owner", d.ID)
+		require.Error(t, err)
+		sErr, ok = status.FromError(err)
+		require.True(t, ok, "dependent services must return a typed error")
+		assert.Equal(t, status.PreconditionFailed, sErr.Type(), "deletion must fail until services are removed")
+		stored, err := store.GetCustomDomain(ctx, "owner", d.ID)
+		require.NoError(t, err)
+		assert.True(t, stored.Validated, "rejected deletion must preserve validation")
+
+		require.NoError(t, store.DeleteService(ctx, "owner", svc.ID))
+		require.NoError(t, store.DeleteCustomDomain(ctx, "owner", d.ID))
+		_, err = store.GetCustomDomain(ctx, "owner", d.ID)
+		require.Error(t, err, "the registration must be gone after successful deletion")
+	})
+}
+
+func TestDeleteCustomDomain_ConcurrentServiceCreation(t *testing.T) {
+	runTestForAllEngines(t, "", func(t *testing.T, store Store) {
+		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+		defer cancel()
+		require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
+		for i := range 10 {
+			d, err := store.CreateCustomDomain(ctx, "owner", fmt.Sprintf("app%d.example.com", i), "cluster", true)
+			require.NoError(t, err)
+			svc := &rpservice.Service{ID: fmt.Sprintf("service-%d", i), AccountID: "owner", Domain: d.Domain}
+			start := make(chan struct{})
+			created := make(chan error, 1)
+			deleted := make(chan error, 1)
+			go func() {
+				<-start
+				created <- store.ExecuteInTransaction(ctx, func(tx Store) error {
+					domains, err := tx.LockCustomDomains(ctx, "owner")
+					if err != nil {
+						return err
+					}
+					for _, candidate := range domains {
+						if candidate.ID == d.ID && candidate.Validated {
+							return tx.CreateService(ctx, svc)
+						}
+					}
+					return status.Errorf(status.PreconditionFailed, "registration was deleted")
+				})
+			}()
+			go func() {
+				<-start
+				deleted <- store.DeleteCustomDomain(ctx, "owner", d.ID)
+			}()
+			close(start)
+			createErr, deleteErr := <-created, <-deleted
+			require.True(t, createErr == nil || deleteErr == nil, "one operation must succeed: create=%v, delete=%v", createErr, deleteErr)
+			if createErr == nil {
+				require.Error(t, deleteErr, "a committed service must block deletion")
+				stored, err := store.GetCustomDomain(ctx, "owner", d.ID)
+				require.NoError(t, err)
+				assert.True(t, stored.Validated, "the service must retain its authorization")
+				require.NoError(t, store.DeleteService(ctx, "owner", svc.ID))
+				require.NoError(t, store.DeleteCustomDomain(ctx, "owner", d.ID))
+				continue
+			}
+			require.NoError(t, deleteErr)
+			services, err := store.GetAccountServices(ctx, LockingStrengthNone, "owner")
+			require.NoError(t, err)
+			assert.Empty(t, services, "a deleted registration must not leave a new service")
+		}
+	})
+}

management/server/store/store.go

@@ -302,6 +302,7 @@ type Store interface {
 	GetCustomDomain(ctx context.Context, accountID string, domainID string) (*domain.Domain, error)
 	ListFreeDomains(ctx context.Context, accountID string) ([]string, error)
 	ListCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
+	LockCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
 	GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error)
 	CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
 	UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error)

management/server/store/store_mock.go

@@ -3212,6 +3212,21 @@ func (mr *MockStoreMockRecorder) ListFreeDomains(ctx, accountID any) *gomock.Cal
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListFreeDomains", reflect.TypeOf((*MockStore)(nil).ListFreeDomains), ctx, accountID)
 }
 
+// LockCustomDomains mocks base method.
+func (m *MockStore) LockCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "LockCustomDomains", ctx, accountID)
+	ret0, _ := ret[0].([]*domain.Domain)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// LockCustomDomains indicates an expected call of LockCustomDomains.
+func (mr *MockStoreMockRecorder) LockCustomDomains(ctx, accountID any) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockCustomDomains", reflect.TypeOf((*MockStore)(nil).LockCustomDomains), ctx, accountID)
+}
+
 // MarkAccountPrimary mocks base method.
 func (m *MockStore) MarkAccountPrimary(ctx context.Context, accountID string) error {
 	m.ctrl.T.Helper()

shared/management/http/api/openapi.yml

@@ -13347,7 +13347,7 @@ paths:
   /api/reverse-proxies/domains/{domainId}:
     delete:
       summary: Delete a Custom domain
-      description: Delete an existing service custom domain
+      description: Delete an existing service custom domain after removing or moving all services that use it or its subdomains, including disabled services.
       tags: [ Services ]
       security:
         - BearerAuth: [ ]
@@ -13370,6 +13370,9 @@ paths:
           "$ref": "#/components/responses/forbidden"
         '404':
           "$ref": "#/components/responses/not_found"
+        '412':
+          description: The domain or one of its subdomains is still used by a service
+          content: { }
         '500':
           "$ref": "#/components/responses/internal_error"
   /api/reverse-proxies/domains/{domainId}/validate: