[management] Refuse services on unvalidated custom domains (#7341)

Require validated custom domains when creating or updating reverse proxy services. 
Propagate validation errors during updates and return HTTP 409 for duplicate domain claims.

Add regression tests for domain validation, ownership, and service creation and updates.
This commit is contained in:
Maycon Santos
2026-09-10 11:57:14 +02:00
committed by GitHub
parent 27991aab98
commit 21b4a83cea
9 changed files with 612 additions and 23 deletions
@@ -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")
}
@@ -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 {