Merge remote-tracking branch 'origin/main' into HEAD

# Conflicts:
#	shared/management/proto/proxy_service.pb.go
This commit is contained in:
Viktor Liu
2026-10-20 01:18:10 +09:00
815 changed files with 73921 additions and 25975 deletions
@@ -0,0 +1,76 @@
package agentnetwork
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
"github.com/netbirdio/netbird/management/server/store"
nbtypes "github.com/netbirdio/netbird/management/server/types"
)
// TestCleanupAccessLogs_RealStore_DeletedAccount covers a deleted account's access logs.
// The sweep is driven by settings rows, which go with the account, so without a fallback
// those logs would never expire. They get the default retention instead. A live account
// can delete its own settings row, so "no settings" must not be mistaken for "deleted":
// that account's logs are left alone, as are those of an account that keeps logs forever.
func TestCleanupAccessLogs_RealStore_DeletedAccount(t *testing.T) {
ctx := context.Background()
s, cleanup, err := store.NewTestStoreFromSQL(ctx, "", t.TempDir())
require.NoError(t, err, "real sqlite test store must come up")
defer cleanup()
const (
deletedAccountID = "acc-deleted"
keepAccountID = "acc-keep-forever"
noSettingsAccountID = "acc-live-no-settings"
)
old := time.Now().UTC().AddDate(0, 0, -(types.DefaultAccessLogRetentionDays + 10))
recent := time.Now().UTC().AddDate(0, 0, -1)
require.NoError(t, s.SaveAccount(ctx, &nbtypes.Account{Id: keepAccountID}))
require.NoError(t, s.SaveAccount(ctx, &nbtypes.Account{Id: noSettingsAccountID}))
keepSettings := types.DefaultSettings(keepAccountID)
keepSettings.Domain = "keep.gw.example.com"
keepSettings.AccessLogRetentionDays = 0
require.NoError(t, s.SaveAgentNetworkSettings(ctx, keepSettings))
mkLog := func(id, accountID string, ts time.Time) {
t.Helper()
entry := &types.AgentNetworkAccessLog{
ID: id, AccountID: accountID, ServiceID: "svc", Timestamp: ts, StatusCode: 200, Model: "gpt-4o",
}
groups := []types.AgentNetworkAccessLogGroup{{LogID: id, GroupID: "grp-eng", AccountID: accountID}}
require.NoError(t, s.CreateAgentNetworkAccessLog(ctx, entry, groups))
}
mkLog("deleted-old", deletedAccountID, old)
mkLog("deleted-recent", deletedAccountID, recent)
mkLog("keep-old", keepAccountID, old)
mkLog("no-settings-old", noSettingsAccountID, old)
m := &managerImpl{store: s}
m.cleanupAccessLogsOnce(ctx)
logIDs := func(accountID string) []string {
t.Helper()
logs, _, err := s.GetAgentNetworkAccessLogs(ctx, store.LockingStrengthNone, accountID,
types.AgentNetworkAccessLogFilter{Page: 1, PageSize: 50})
require.NoError(t, err)
ids := make([]string, 0, len(logs))
for _, l := range logs {
ids = append(ids, l.ID)
}
return ids
}
assert.Equal(t, []string{"deleted-recent"}, logIDs(deletedAccountID),
"a deleted account should have logs past the default retention swept")
assert.Equal(t, []string{"keep-old"}, logIDs(keepAccountID),
"an account with retention disabled should keep its old logs")
assert.Equal(t, []string{"no-settings-old"}, logIDs(noSettingsAccountID),
"a live account without a settings row should keep its old logs")
}
@@ -80,6 +80,9 @@ type Manager interface {
ListAccessLogSessions(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLogSession, int64, error)
GetUsageOverview(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter, granularity types.UsageGranularity) ([]*types.AgentNetworkUsageBucket, error)
StartAccessLogCleanup(ctx context.Context, cleanupIntervalHours int)
// RemoveAccountGateway drops the account's gateway mappings from the
// proxies. It runs as an account deletion hook.
RemoveAccountGateway(ctx context.Context, accountID string) error
RecordConsumption(ctx context.Context, accountID string, kind types.ConsumptionDimension, dimID string, windowSeconds, tokensIn, tokensOut int64, costUSD float64) error
RecordAccountBudgetUsage(ctx context.Context, accountID, userID string, groupIDs []string, tokensIn, tokensOut int64, costUSD float64) error
RecordUsage(ctx context.Context, in RecordUsageInput) error
@@ -1350,8 +1353,8 @@ func (m *managerImpl) scopeFilterToCaller(ctx context.Context, accountID, userID
// StartAccessLogCleanup launches a background sweep that periodically deletes
// each account's agent-network access-log rows older than that account's
// AccessLogRetentionDays. Usage records are never swept. A non-positive
// interval defaults to 24h.
// AccessLogRetentionDays, and the consumption counters of deleted accounts.
// Usage records are never swept. A non-positive interval defaults to 24h.
func (m *managerImpl) StartAccessLogCleanup(ctx context.Context, cleanupIntervalHours int) {
if cleanupIntervalHours <= 0 {
cleanupIntervalHours = 24
@@ -1362,21 +1365,40 @@ func (m *managerImpl) StartAccessLogCleanup(ctx context.Context, cleanupInterval
ticker := time.NewTicker(interval)
defer ticker.Stop()
m.cleanupAccessLogsOnce(ctx) // run once on startup
m.cleanupOnce(ctx) // run once on startup
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
m.cleanupAccessLogsOnce(ctx)
m.cleanupOnce(ctx)
}
}
}()
}
func (m *managerImpl) cleanupOnce(ctx context.Context) {
m.cleanupAccessLogsOnce(ctx)
m.cleanupDeletedAccountConsumption(ctx)
}
// cleanupDeletedAccountConsumption deletes the consumption counters of accounts
// that no longer exist. Best-effort: a failure is logged and retried next sweep.
func (m *managerImpl) cleanupDeletedAccountConsumption(ctx context.Context) {
deleted, err := m.store.DeleteAgentNetworkConsumptionOfDeletedAccounts(ctx)
if err != nil {
log.WithContext(ctx).Warnf("agent-network consumption cleanup: %v", err)
return
}
if deleted > 0 {
log.WithContext(ctx).Infof("agent-network consumption cleanup: deleted %d counters of deleted accounts", deleted)
}
}
// cleanupAccessLogsOnce sweeps every account's expired access-log rows against
// its configured retention. Best-effort: a per-account failure is logged and
// the sweep continues.
// its configured retention. Deleted accounts, whose settings rows went with
// them, get the default retention. Best-effort: a per-account failure is
// logged and the sweep continues.
func (m *managerImpl) cleanupAccessLogsOnce(ctx context.Context) {
settings, err := m.store.GetAllAgentNetworkSettings(ctx, store.LockingStrengthNone)
if err != nil {
@@ -1384,18 +1406,31 @@ func (m *managerImpl) cleanupAccessLogsOnce(ctx context.Context) {
return
}
for _, s := range settings {
if s.AccessLogRetentionDays <= 0 {
continue // keep indefinitely
}
cutoff := time.Now().UTC().AddDate(0, 0, -s.AccessLogRetentionDays)
deleted, err := m.store.DeleteOldAgentNetworkAccessLogs(ctx, s.AccountID, cutoff)
if err != nil {
log.WithContext(ctx).Warnf("agent-network access-log cleanup for account %s: %v", s.AccountID, err)
continue
}
if deleted > 0 {
log.WithContext(ctx).Infof("agent-network access-log cleanup: deleted %d rows for account %s (retention %d days)", deleted, s.AccountID, s.AccessLogRetentionDays)
}
m.cleanupAccountAccessLogs(ctx, s.AccountID, s.AccessLogRetentionDays)
}
deleted, err := m.store.GetDeletedAccountIDsWithAgentNetworkAccessLogs(ctx)
if err != nil {
log.WithContext(ctx).Errorf("agent-network access-log cleanup: list deleted accounts: %v", err)
return
}
for _, accountID := range deleted {
m.cleanupAccountAccessLogs(ctx, accountID, types.DefaultAccessLogRetentionDays)
}
}
func (m *managerImpl) cleanupAccountAccessLogs(ctx context.Context, accountID string, retentionDays int) {
if retentionDays <= 0 {
return // keep indefinitely
}
cutoff := time.Now().UTC().AddDate(0, 0, -retentionDays)
deleted, err := m.store.DeleteOldAgentNetworkAccessLogs(ctx, accountID, cutoff)
if err != nil {
log.WithContext(ctx).Warnf("agent-network access-log cleanup for account %s: %v", accountID, err)
return
}
if deleted > 0 {
log.WithContext(ctx).Infof("agent-network access-log cleanup: deleted %d rows for account %s (retention %d days)", deleted, accountID, retentionDays)
}
}
@@ -1545,6 +1580,8 @@ func (*mockManager) GetUsageOverview(_ context.Context, _, _ string, _ types.Age
func (*mockManager) StartAccessLogCleanup(_ context.Context, _ int) {}
func (*mockManager) RemoveAccountGateway(_ context.Context, _ string) error { return nil }
func (*mockManager) RecordConsumption(_ context.Context, _ string, _ types.ConsumptionDimension, _ string, _, _, _ int64, _ float64) error {
return nil
}
@@ -2,8 +2,10 @@ package agentnetwork
import (
"context"
"fmt"
log "github.com/sirupsen/logrus"
goproto "google.golang.org/protobuf/proto"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
"github.com/netbirdio/netbird/management/server/types"
@@ -81,18 +83,66 @@ func (m *managerImpl) reconcile(ctx context.Context, accountID string) {
}
m.reconcileMu.Unlock()
for _, entry := range creates {
entry.mapping.Type = proto.ProxyMappingUpdateType_UPDATE_TYPE_CREATED
m.proxyController.SendServiceUpdateToCluster(ctx, accountID, entry.mapping, entry.cluster)
m.sendMappings(ctx, accountID, creates, proto.ProxyMappingUpdateType_UPDATE_TYPE_CREATED)
m.sendMappings(ctx, accountID, updates, proto.ProxyMappingUpdateType_UPDATE_TYPE_MODIFIED)
m.sendMappings(ctx, accountID, deletes, proto.ProxyMappingUpdateType_UPDATE_TYPE_REMOVED)
}
// sendMappings sends each entry as updateType. It sends a copy: the entries'
// mappings are shared with reconcileCache, which another reconcile or
// RemoveAccountGateway may be reading, so they are never written.
func (m *managerImpl) sendMappings(ctx context.Context, accountID string, entries []syntheticMapping, updateType proto.ProxyMappingUpdateType) {
for _, entry := range entries {
update := goproto.Clone(entry.mapping).(*proto.ProxyMapping)
update.Type = updateType
m.proxyController.SendServiceUpdateToCluster(ctx, accountID, update, entry.cluster)
}
for _, entry := range updates {
entry.mapping.Type = proto.ProxyMappingUpdateType_UPDATE_TYPE_MODIFIED
m.proxyController.SendServiceUpdateToCluster(ctx, accountID, entry.mapping, entry.cluster)
}
// RemoveAccountGateway tells the proxies to drop every mapping of the account's
// gateway, so a deleted account's proxy config, provider API keys included, does
// not linger in proxy memory until the next resync. It is an account deletion
// hook: it runs before the account's data is removed, the last point at which
// the mappings can be synthesised from the store. The cache alone would miss
// them, since it is per instance and empty after a restart. If the deletion
// then fails, the gateway stays down until the account's next change reconciles
// it back.
func (m *managerImpl) RemoveAccountGateway(ctx context.Context, accountID string) error {
if m.proxyController == nil {
return nil
}
for _, entry := range deletes {
entry.mapping.Type = proto.ProxyMappingUpdateType_UPDATE_TYPE_REMOVED
m.proxyController.SendServiceUpdateToCluster(ctx, accountID, entry.mapping, entry.cluster)
services, err := SynthesizeServices(ctx, m.store, accountID)
if err != nil {
return fmt.Errorf("synthesise agent network services: %w", err)
}
oidcCfg := m.proxyController.GetOIDCValidationConfig()
removed := make(map[string]syntheticMapping, len(services))
for _, svc := range services {
if svc == nil || svc.ID == "" {
continue
}
removed[svc.ID] = syntheticMapping{
mapping: svc.ToProtoMapping(rpservice.Delete, "", oidcCfg),
cluster: svc.ProxyCluster,
}
}
m.reconcileMu.Lock()
for id, entry := range m.reconcileCache[accountID] {
if _, ok := removed[id]; !ok {
removed[id] = entry
}
}
delete(m.reconcileCache, accountID)
m.reconcileMu.Unlock()
entries := make([]syntheticMapping, 0, len(removed))
for _, entry := range removed {
entries = append(entries, entry)
}
m.sendMappings(ctx, accountID, entries, proto.ProxyMappingUpdateType_UPDATE_TYPE_REMOVED)
return nil
}
// diffMappings classifies the previous→current transition for a single
@@ -2,6 +2,8 @@ package agentnetwork
import (
"context"
"sync"
"sync/atomic"
"testing"
"go.uber.org/mock/gomock"
@@ -12,6 +14,7 @@ import (
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/shared/management/proto"
"github.com/netbirdio/netbird/shared/management/status"
)
func newReconcileMgr(t *testing.T, ctrl *gomock.Controller) (*managerImpl, *store.MockStore, *proxy.MockController) {
@@ -287,3 +290,154 @@ func TestDiffMappings_RemovedServiceIsDeletedOnItsOwnCluster(t *testing.T) {
assert.Equal(t, "brave-otter.gateway.example.com", deletes[0].cluster)
}
}
// TestRemoveAccountGateway_EmitsRemovedFromStore — account deletion runs on an
// instance that may never have reconciled the account, so its cache is empty.
// The mappings are synthesised from the store, still intact before the delete,
// and each is sent as REMOVED to the cluster that serves it.
func TestRemoveAccountGateway_EmitsRemovedFromStore(t *testing.T) {
ctx := context.Background()
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mgr, mockStore, mockProxy := newReconcileMgr(t, ctrl)
provider := newReconcileTestProvider()
policy := newReconcileTestPolicy(provider.ID, "grp-eng")
expectReconcileSynthInputs(mockStore, ctx, []*types.Provider{provider}, []*types.Policy{policy}, []*types.Guardrail{})
mockProxy.EXPECT().GetOIDCValidationConfig().Return(proxy.OIDCValidationConfig{})
var sent []*proto.ProxyMapping
mockProxy.EXPECT().
SendServiceUpdateToCluster(ctx, "acct-1", gomock.Any(), "eu.proxy.netbird.io").
Do(func(_ context.Context, _ string, m *proto.ProxyMapping, _ string) {
sent = append(sent, m)
})
require.NoError(t, mgr.RemoveAccountGateway(ctx, "acct-1"))
require.Len(t, sent, 1, "the account's one gateway mapping must be removed")
assert.Equal(t, proto.ProxyMappingUpdateType_UPDATE_TYPE_REMOVED, sent[0].Type, "the update must be a removal")
assert.Equal(t, "agent-net-svc-acct-1", sent[0].Id, "the removal must name the account's gateway service")
}
// TestRemoveAccountGateway_AlsoRemovesCachedMappings — a mapping this instance
// last sent but the store no longer synthesises (here, one on another cluster)
// is removed too, and the account's cache entry is cleared.
func TestRemoveAccountGateway_AlsoRemovesCachedMappings(t *testing.T) {
ctx := context.Background()
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mgr, mockStore, mockProxy := newReconcileMgr(t, ctrl)
mgr.reconcileCache["acct-1"] = map[string]syntheticMapping{
"stale-svc": {mapping: &proto.ProxyMapping{Id: "stale-svc"}, cluster: "us.proxy.netbird.io"},
}
// Settings but no providers: the store synthesises nothing.
mockStore.EXPECT().
GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "acct-1").
Return(newReconcileTestSettings(), nil)
mockStore.EXPECT().
GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, "acct-1").
Return([]*types.Provider{}, nil)
mockProxy.EXPECT().GetOIDCValidationConfig().Return(proxy.OIDCValidationConfig{})
var sent []*proto.ProxyMapping
mockProxy.EXPECT().
SendServiceUpdateToCluster(ctx, "acct-1", gomock.Any(), "us.proxy.netbird.io").
Do(func(_ context.Context, _ string, m *proto.ProxyMapping, _ string) {
sent = append(sent, m)
})
require.NoError(t, mgr.RemoveAccountGateway(ctx, "acct-1"))
require.Len(t, sent, 1, "the cached mapping must be removed from its own cluster")
assert.Equal(t, "stale-svc", sent[0].Id)
assert.Equal(t, proto.ProxyMappingUpdateType_UPDATE_TYPE_REMOVED, sent[0].Type)
mgr.reconcileMu.Lock()
_, present := mgr.reconcileCache["acct-1"]
mgr.reconcileMu.Unlock()
assert.False(t, present, "the deleted account's cache entry must be cleared")
}
// TestRemoveAccountGateway_SynthFailureAbortsDeletion — if the mappings cannot
// be read, nothing is sent and the error is returned, which as an account
// deletion hook keeps the account rather than leaving its gateway running.
func TestRemoveAccountGateway_SynthFailureAbortsDeletion(t *testing.T) {
ctx := context.Background()
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mgr, mockStore, _ := newReconcileMgr(t, ctrl)
mockStore.EXPECT().
GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "acct-1").
Return(nil, status.Errorf(status.Internal, "store unavailable"))
assert.Error(t, mgr.RemoveAccountGateway(ctx, "acct-1"), "a failed synthesis must fail the hook")
}
func TestRemoveAccountGateway_NilProxyController_NoOp(t *testing.T) {
mgr := &managerImpl{reconcileCache: make(map[string]map[string]syntheticMapping)}
// Must not panic and must not query the store.
assert.NoError(t, mgr.RemoveAccountGateway(context.Background(), "acct-1"))
}
// TestReconcile_ConcurrentWithGatewayChanges — while an account's gateway
// flaps (its policy is removed and re-added between reads), concurrent
// reconciles and RemoveAccountGateway share the cached mappings: one caches a
// mapping and sends it, another finds it gone and sends its removal. Run under
// -race: neither path may write a cached mapping, only copies of it.
func TestReconcile_ConcurrentWithGatewayChanges(t *testing.T) {
ctx := context.Background()
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mgr, mockStore, mockProxy := newReconcileMgr(t, ctrl)
// gomock serialises every call on the controller's mutex, which would give
// the race detector the ordering the code under test lacks. The sends go
// through a fake that takes no lock.
mgr.proxyController = unsyncedSender{MockController: mockProxy}
provider := newReconcileTestProvider()
policy := newReconcileTestPolicy(provider.ID, "grp-eng")
var reads atomic.Int64
mockStore.EXPECT().GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "acct-1").Return(newReconcileTestSettings(), nil).AnyTimes()
mockStore.EXPECT().GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, "acct-1").Return([]*types.Provider{provider}, nil).AnyTimes()
mockStore.EXPECT().GetAccountAgentNetworkPolicies(ctx, store.LockingStrengthNone, "acct-1").
DoAndReturn(func(context.Context, store.LockingStrength, string) ([]*types.Policy, error) {
if reads.Add(1)%2 == 0 {
return []*types.Policy{}, nil
}
return []*types.Policy{policy}, nil
}).AnyTimes()
mockStore.EXPECT().GetAccountAgentNetworkGuardrails(ctx, store.LockingStrengthNone, "acct-1").Return([]*types.Guardrail{}, nil).AnyTimes()
var wg sync.WaitGroup
for i := 0; i < 8; i++ {
wg.Add(1)
go func(remove bool) {
defer wg.Done()
for j := 0; j < 50; j++ {
if remove && j%10 == 0 {
_ = mgr.RemoveAccountGateway(ctx, "acct-1")
continue
}
mgr.reconcile(ctx, "acct-1")
}
}(i == 0)
}
wg.Wait()
}
// unsyncedSender answers the calls reconcile makes on every pass without any
// locking, so concurrent callers are not ordered by the fake itself.
type unsyncedSender struct {
*proxy.MockController
}
func (unsyncedSender) GetOIDCValidationConfig() proxy.OIDCValidationConfig {
return proxy.OIDCValidationConfig{}
}
func (unsyncedSender) SendServiceUpdateToCluster(context.Context, string, *proto.ProxyMapping, string) {}
@@ -97,7 +97,7 @@ func (m *managerImpl) GetAllPeers(ctx context.Context, accountID, userID string)
return m.store.GetUserPeers(ctx, store.LockingStrengthNone, accountID, userID)
}
return m.store.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, "", "")
return m.store.GetAccountPeers(ctx, store.LockingStrengthNone, accountID, "", "", "")
}
func (m *managerImpl) GetPeerAccountID(ctx context.Context, peerID string) (string, error) {
@@ -9,6 +9,7 @@ import (
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
"github.com/netbirdio/netbird/management/internals/shared/db"
"github.com/netbirdio/netbird/management/server/geolocation"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/permissions/modules"
@@ -18,14 +19,16 @@ import (
)
type managerImpl struct {
repo accesslogs.Repository
store store.Store
permissionsManager permissions.Manager
geo geolocation.Geolocation
cleanupCancel context.CancelFunc
}
func NewManager(store store.Store, permissionsManager permissions.Manager, geo geolocation.Geolocation) accesslogs.Manager {
func NewManager(repo accesslogs.Repository, store store.Store, permissionsManager permissions.Manager, geo geolocation.Geolocation) accesslogs.Manager {
return &managerImpl{
repo: repo,
store: store,
permissionsManager: permissionsManager,
geo: geo,
@@ -54,7 +57,7 @@ func (m *managerImpl) SaveAccessLog(ctx context.Context, logEntry *accesslogs.Ac
}
}
if err := m.store.CreateAccessLog(ctx, logEntry); err != nil {
if err := m.repo.Create(ctx, logEntry); err != nil {
log.WithContext(ctx).WithFields(log.Fields{
"service_id": logEntry.ServiceID,
"method": logEntry.Method,
@@ -82,7 +85,7 @@ func (m *managerImpl) GetAllAccessLogs(ctx context.Context, accountID, userID st
log.WithContext(ctx).Warnf("failed to resolve user filters: %v", err)
}
logs, totalCount, err := m.store.GetAccountAccessLogs(ctx, store.LockingStrengthNone, accountID, *filter)
logs, totalCount, err := m.repo.ListByAccount(ctx, db.LockingStrengthNone, accountID, *filter)
if err != nil {
return nil, 0, err
}
@@ -98,7 +101,7 @@ func (m *managerImpl) CleanupOldAccessLogs(ctx context.Context, retentionDays in
}
cutoffTime := time.Now().AddDate(0, 0, -retentionDays)
deletedCount, err := m.store.DeleteOldAccessLogs(ctx, cutoffTime)
deletedCount, err := m.repo.DeleteOlderThan(ctx, cutoffTime)
if err != nil {
log.WithContext(ctx).Errorf("failed to cleanup old access logs: %v", err)
return 0, err
@@ -5,27 +5,27 @@ import (
"testing"
"time"
"go.uber.org/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
)
func TestCleanupOldAccessLogs(t *testing.T) {
tests := []struct {
name string
retentionDays int
setupMock func(*store.MockStore)
setupMock func(*accesslogs.MockRepository)
expectedCount int64
expectedError bool
}{
{
name: "cleanup logs older than retention period",
retentionDays: 30,
setupMock: func(mockStore *store.MockStore) {
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
setupMock: func(mockRepo *accesslogs.MockRepository) {
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
DoAndReturn(func(ctx context.Context, olderThan time.Time) (int64, error) {
expectedCutoff := time.Now().AddDate(0, 0, -30)
timeDiff := olderThan.Sub(expectedCutoff)
@@ -41,9 +41,9 @@ func TestCleanupOldAccessLogs(t *testing.T) {
{
name: "no logs to cleanup",
retentionDays: 30,
setupMock: func(mockStore *store.MockStore) {
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
setupMock: func(mockRepo *accesslogs.MockRepository) {
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
Return(int64(0), nil)
},
expectedCount: 0,
@@ -52,8 +52,8 @@ func TestCleanupOldAccessLogs(t *testing.T) {
{
name: "zero retention days skips cleanup",
retentionDays: 0,
setupMock: func(mockStore *store.MockStore) {
// No expectations - DeleteOldAccessLogs should not be called
setupMock: func(mockRepo *accesslogs.MockRepository) {
// No expectations - DeleteOlderThan should not be called
},
expectedCount: 0,
expectedError: false,
@@ -61,8 +61,8 @@ func TestCleanupOldAccessLogs(t *testing.T) {
{
name: "negative retention days skips cleanup",
retentionDays: -10,
setupMock: func(mockStore *store.MockStore) {
// No expectations - DeleteOldAccessLogs should not be called
setupMock: func(mockRepo *accesslogs.MockRepository) {
// No expectations - DeleteOlderThan should not be called
},
expectedCount: 0,
expectedError: false,
@@ -74,11 +74,11 @@ func TestCleanupOldAccessLogs(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
tt.setupMock(mockStore)
mockRepo := accesslogs.NewMockRepository(ctrl)
tt.setupMock(mockRepo)
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx := context.Background()
@@ -98,10 +98,10 @@ func TestCleanupWithExactBoundary(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
DoAndReturn(func(ctx context.Context, olderThan time.Time) (int64, error) {
expectedCutoff := time.Now().AddDate(0, 0, -30)
timeDiff := olderThan.Sub(expectedCutoff)
@@ -110,7 +110,7 @@ func TestCleanupWithExactBoundary(t *testing.T) {
})
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx := context.Background()
@@ -125,11 +125,11 @@ func TestStartPeriodicCleanup(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
// No expectations - cleanup should not run
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx, cancel := context.WithCancel(context.Background())
@@ -139,22 +139,22 @@ func TestStartPeriodicCleanup(t *testing.T) {
time.Sleep(100 * time.Millisecond)
// If DeleteOldAccessLogs was called, the test will fail due to unexpected call
// If DeleteOlderThan was called, the test will fail due to unexpected call
})
t.Run("periodic cleanup runs immediately on start", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
Return(int64(2), nil).
Times(1)
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx, cancel := context.WithCancel(context.Background())
@@ -171,15 +171,15 @@ func TestStartPeriodicCleanup(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
Return(int64(1), nil).
Times(1)
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx, cancel := context.WithCancel(context.Background())
@@ -198,15 +198,15 @@ func TestStartPeriodicCleanup(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
Return(int64(0), nil).
Times(1)
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx, cancel := context.WithCancel(context.Background())
@@ -223,15 +223,15 @@ func TestStartPeriodicCleanup(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
Return(int64(3), nil).
Times(1)
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx, cancel := context.WithCancel(context.Background())
@@ -249,15 +249,15 @@ func TestStopPeriodicCleanup(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStore := store.NewMockStore(ctrl)
mockRepo := accesslogs.NewMockRepository(ctrl)
mockStore.EXPECT().
DeleteOldAccessLogs(gomock.Any(), gomock.Any()).
mockRepo.EXPECT().
DeleteOlderThan(gomock.Any(), gomock.Any()).
Return(int64(1), nil).
Times(1)
manager := &managerImpl{
store: mockStore,
repo: mockRepo,
}
ctx := context.Background()
@@ -0,0 +1,135 @@
package manager
import (
"context"
"strings"
"time"
log "github.com/sirupsen/logrus"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
"github.com/netbirdio/netbird/management/internals/shared/db"
"github.com/netbirdio/netbird/shared/management/status"
)
type sqlRepository struct {
conn *db.Conn
db *gorm.DB
}
// NewRepository returns the access log repository backed by conn.
func NewRepository(conn *db.Conn) accesslogs.Repository {
return &sqlRepository{conn: conn, db: conn.DB(nil)}
}
func (r *sqlRepository) WithTx(tx *db.Tx) accesslogs.Repository {
return &sqlRepository{conn: r.conn, db: r.conn.DB(tx)}
}
func (r *sqlRepository) Create(ctx context.Context, entry *accesslogs.AccessLogEntry) error {
if err := r.db.Create(entry).Error; err != nil {
log.WithContext(ctx).WithFields(log.Fields{
"service_id": entry.ServiceID,
"method": entry.Method,
"host": entry.Host,
"path": entry.Path,
}).Errorf("failed to create access log entry in store: %v", err)
return status.Errorf(status.Internal, "failed to create access log entry in store")
}
return nil
}
// ListByAccount returns one page of an account's access logs together with the
// total number of entries matching the filter.
func (r *sqlRepository) ListByAccount(ctx context.Context, lockStrength db.LockingStrength, accountID string, filter accesslogs.AccessLogFilter) ([]*accesslogs.AccessLogEntry, int64, error) {
var totalCount int64
countQuery := applyFilters(r.db.Model(&accesslogs.AccessLogEntry{}).Where("account_id = ?", accountID), filter)
if err := countQuery.Count(&totalCount).Error; err != nil {
log.WithContext(ctx).Errorf("failed to count access logs: %v", err)
return nil, 0, status.Errorf(status.Internal, "failed to count access logs")
}
query := applyFilters(r.db.Where("account_id = ?", accountID), filter)
sortOrder := strings.ToUpper(filter.GetSortOrder())
for _, column := range strings.Split(filter.GetSortColumn(), ",") {
if column = strings.TrimSpace(column); column != "" {
query = query.Order(column + " " + sortOrder)
}
}
query = query.Limit(filter.GetLimit()).Offset(filter.GetOffset())
if lockStrength != db.LockingStrengthNone {
query = query.Clauses(clause.Locking{Strength: string(lockStrength)})
}
var logs []*accesslogs.AccessLogEntry
if err := query.Find(&logs).Error; err != nil {
log.WithContext(ctx).Errorf("failed to get access logs from store: %v", err)
return nil, 0, status.Errorf(status.Internal, "failed to get access logs from store")
}
return logs, totalCount, nil
}
func (r *sqlRepository) DeleteOlderThan(ctx context.Context, olderThan time.Time) (int64, error) {
result := r.db.Where("timestamp < ?", olderThan).Delete(&accesslogs.AccessLogEntry{})
if result.Error != nil {
log.WithContext(ctx).Errorf("failed to delete old access logs: %v", result.Error)
return 0, status.Errorf(status.Internal, "failed to delete old access logs")
}
return result.RowsAffected, nil
}
func applyFilters(query *gorm.DB, filter accesslogs.AccessLogFilter) *gorm.DB {
if filter.Search != nil {
searchPattern := "%" + *filter.Search + "%"
query = query.Where(
"id LIKE ? OR location_connection_ip LIKE ? OR host LIKE ? OR path LIKE ? OR CONCAT(host, path) LIKE ? OR user_id IN (SELECT id FROM users WHERE email LIKE ? OR name LIKE ?)",
searchPattern, searchPattern, searchPattern, searchPattern, searchPattern, searchPattern, searchPattern,
)
}
if filter.SourceIP != nil {
query = query.Where("location_connection_ip = ?", *filter.SourceIP)
}
if filter.Host != nil {
query = query.Where("host = ?", *filter.Host)
}
if filter.Path != nil {
query = query.Where("path LIKE ?", "%"+*filter.Path+"%")
}
if filter.UserID != nil {
query = query.Where("user_id = ?", *filter.UserID)
}
if filter.Method != nil {
query = query.Where("method = ?", *filter.Method)
}
if filter.Status != nil {
switch *filter.Status {
case "success":
query = query.Where("(status_code >= ? AND status_code < ?)", 200, 400)
case "failed":
query = query.Where("((status_code >= ? AND status_code < ?) OR status_code >= ?)", 100, 200, 400)
}
}
if filter.StatusCode != nil {
query = query.Where("status_code = ?", *filter.StatusCode)
}
if filter.StartDate != nil {
query = query.Where("timestamp >= ?", *filter.StartDate)
}
if filter.EndDate != nil {
query = query.Where("timestamp <= ?", *filter.EndDate)
}
return query
}
@@ -0,0 +1,125 @@
package manager
import (
"context"
"errors"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
"github.com/netbirdio/netbird/management/internals/shared/db"
"github.com/netbirdio/netbird/management/internals/shared/db/dbtest"
)
func newTestRepository(t *testing.T) (accesslogs.Repository, *db.Conn) {
conn := dbtest.NewConn(t, &accesslogs.AccessLogEntry{})
return NewRepository(conn), conn
}
func newEntry(id, accountID, method string, age time.Duration) *accesslogs.AccessLogEntry {
return &accesslogs.AccessLogEntry{
ID: id,
AccountID: accountID,
Method: method,
Host: "app.example.com",
Path: "/",
StatusCode: 200,
Timestamp: time.Now().Add(-age),
}
}
func TestSqlRepository_ListByAccount(t *testing.T) {
repo, _ := newTestRepository(t)
ctx := context.Background()
for _, entry := range []*accesslogs.AccessLogEntry{
newEntry("a1", "acc-a", "GET", 3*time.Hour),
newEntry("a2", "acc-a", "POST", 2*time.Hour),
newEntry("a3", "acc-a", "GET", time.Hour),
newEntry("b1", "acc-b", "GET", time.Hour),
} {
require.NoError(t, repo.Create(ctx, entry))
}
logs, total, err := repo.ListByAccount(ctx, db.LockingStrengthNone, "acc-a", accesslogs.AccessLogFilter{Page: 1, PageSize: 2})
require.NoError(t, err)
assert.EqualValues(t, 3, total)
require.Len(t, logs, 2)
assert.Equal(t, "a3", logs[0].ID)
assert.Equal(t, "a2", logs[1].ID)
method := "GET"
logs, total, err = repo.ListByAccount(ctx, db.LockingStrengthNone, "acc-a", accesslogs.AccessLogFilter{Page: 1, PageSize: 10, Method: &method, SortOrder: "asc"})
require.NoError(t, err)
assert.EqualValues(t, 2, total)
require.Len(t, logs, 2)
assert.Equal(t, "a1", logs[0].ID)
assert.Equal(t, "a3", logs[1].ID)
}
func TestSqlRepository_DeleteOlderThan(t *testing.T) {
repo, _ := newTestRepository(t)
ctx := context.Background()
require.NoError(t, repo.Create(ctx, newEntry("old", "acc", "GET", 48*time.Hour)))
require.NoError(t, repo.Create(ctx, newEntry("new", "acc", "GET", time.Hour)))
deleted, err := repo.DeleteOlderThan(ctx, time.Now().Add(-24*time.Hour))
require.NoError(t, err)
assert.EqualValues(t, 1, deleted)
logs, total, err := repo.ListByAccount(ctx, db.LockingStrengthNone, "acc", accesslogs.AccessLogFilter{Page: 1, PageSize: 10})
require.NoError(t, err)
assert.EqualValues(t, 1, total)
require.Len(t, logs, 1)
assert.Equal(t, "new", logs[0].ID)
}
func TestSqlRepository_CreateInsideTransactionRollsBack(t *testing.T) {
repo, conn := newTestRepository(t)
ctx := context.Background()
failure := errors.New("abort")
err := conn.RunInTx(ctx, func(tx *db.Tx) error {
txRepo := repo.WithTx(tx)
require.NoError(t, txRepo.Create(ctx, newEntry("tx", "acc", "GET", 0)))
_, total, err := txRepo.ListByAccount(ctx, db.LockingStrengthNone, "acc", accesslogs.AccessLogFilter{Page: 1, PageSize: 10})
require.NoError(t, err)
assert.EqualValues(t, 1, total)
return failure
})
require.ErrorIs(t, err, failure)
_, total, err := repo.ListByAccount(ctx, db.LockingStrengthNone, "acc", accesslogs.AccessLogFilter{Page: 1, PageSize: 10})
require.NoError(t, err)
assert.Zero(t, total)
}
func TestSqlRepository_ListByAccount_StatusFilter(t *testing.T) {
repo, _ := newTestRepository(t)
ctx := context.Background()
statusCodes := map[string]int{"l4": 0, "info": 101, "ok": 200, "notfound": 404}
for id, code := range statusCodes {
entry := newEntry(id, "acc", "GET", time.Hour)
entry.StatusCode = code
require.NoError(t, repo.Create(ctx, entry))
}
foreign := newEntry("foreign", "other", "GET", time.Hour)
foreign.StatusCode = 500
require.NoError(t, repo.Create(ctx, foreign))
listIDs := func(status string) []string {
logs, total, err := repo.ListByAccount(ctx, db.LockingStrengthNone, "acc", accesslogs.AccessLogFilter{Page: 1, PageSize: 10, Status: &status, SortBy: "status_code", SortOrder: "asc"})
require.NoError(t, err)
require.EqualValues(t, len(logs), total)
ids := make([]string, 0, len(logs))
for _, entry := range logs {
ids = append(ids, entry.ID)
}
return ids
}
assert.Equal(t, []string{"info", "notfound"}, listIDs("failed"))
assert.Equal(t, []string{"ok"}, listIDs("success"))
}
@@ -0,0 +1,18 @@
package accesslogs
import (
"context"
"time"
"github.com/netbirdio/netbird/management/internals/shared/db"
)
//go:generate go tool mockgen -package accesslogs -destination=repository_mock.go -source=./repository.go -build_flags=-mod=mod
// Repository persists reverse proxy access log entries.
type Repository interface {
WithTx(tx *db.Tx) Repository
Create(ctx context.Context, entry *AccessLogEntry) error
ListByAccount(ctx context.Context, lockStrength db.LockingStrength, accountID string, filter AccessLogFilter) ([]*AccessLogEntry, int64, error)
DeleteOlderThan(ctx context.Context, olderThan time.Time) (int64, error)
}
@@ -0,0 +1,102 @@
// Code generated by MockGen. DO NOT EDIT.
// Source: ./repository.go
//
// Generated by this command:
//
// mockgen -package accesslogs -destination=repository_mock.go -source=./repository.go -build_flags=-mod=mod
//
// Package accesslogs is a generated GoMock package.
package accesslogs
import (
context "context"
reflect "reflect"
time "time"
db "github.com/netbirdio/netbird/management/internals/shared/db"
gomock "go.uber.org/mock/gomock"
)
// MockRepository is a mock of Repository interface.
type MockRepository struct {
ctrl *gomock.Controller
recorder *MockRepositoryMockRecorder
isgomock struct{}
}
// MockRepositoryMockRecorder is the mock recorder for MockRepository.
type MockRepositoryMockRecorder struct {
mock *MockRepository
}
// NewMockRepository creates a new mock instance.
func NewMockRepository(ctrl *gomock.Controller) *MockRepository {
mock := &MockRepository{ctrl: ctrl}
mock.recorder = &MockRepositoryMockRecorder{mock}
return mock
}
// EXPECT returns an object that allows the caller to indicate expected use.
func (m *MockRepository) EXPECT() *MockRepositoryMockRecorder {
return m.recorder
}
// Create mocks base method.
func (m *MockRepository) Create(ctx context.Context, entry *AccessLogEntry) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Create", ctx, entry)
ret0, _ := ret[0].(error)
return ret0
}
// Create indicates an expected call of Create.
func (mr *MockRepositoryMockRecorder) Create(ctx, entry any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockRepository)(nil).Create), ctx, entry)
}
// DeleteOlderThan mocks base method.
func (m *MockRepository) DeleteOlderThan(ctx context.Context, olderThan time.Time) (int64, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteOlderThan", ctx, olderThan)
ret0, _ := ret[0].(int64)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// DeleteOlderThan indicates an expected call of DeleteOlderThan.
func (mr *MockRepositoryMockRecorder) DeleteOlderThan(ctx, olderThan any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOlderThan", reflect.TypeOf((*MockRepository)(nil).DeleteOlderThan), ctx, olderThan)
}
// ListByAccount mocks base method.
func (m *MockRepository) ListByAccount(ctx context.Context, lockStrength db.LockingStrength, accountID string, filter AccessLogFilter) ([]*AccessLogEntry, int64, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByAccount", ctx, lockStrength, accountID, filter)
ret0, _ := ret[0].([]*AccessLogEntry)
ret1, _ := ret[1].(int64)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
}
// ListByAccount indicates an expected call of ListByAccount.
func (mr *MockRepositoryMockRecorder) ListByAccount(ctx, lockStrength, accountID, filter any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListByAccount", reflect.TypeOf((*MockRepository)(nil).ListByAccount), ctx, lockStrength, accountID, filter)
}
// WithTx mocks base method.
func (m *MockRepository) WithTx(tx *db.Tx) Repository {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "WithTx", tx)
ret0, _ := ret[0].(Repository)
return ret0
}
// WithTx indicates an expected call of WithTx.
func (mr *MockRepositoryMockRecorder) WithTx(tx any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithTx", reflect.TypeOf((*MockRepository)(nil).WithTx), tx)
}
@@ -8,6 +8,6 @@ type Manager interface {
GetDomains(ctx context.Context, accountID, userID string) ([]*Domain, error)
CreateDomain(ctx context.Context, accountID, userID, domainName, targetCluster string) (*Domain, error)
DeleteDomain(ctx context.Context, accountID, userID, domainID string) error
ValidateDomain(ctx context.Context, accountID, userID, domainID string)
ValidateDomain(ctx context.Context, accountID, userID, domainID string) error
GetClusterDomains() []string
}
@@ -134,7 +134,10 @@ func (h *handler) triggerCustomDomainValidation(w http.ResponseWriter, r *http.R
return
}
go h.manager.ValidateDomain(r.Context(), userAuth.AccountId, userAuth.UserId, domainID)
if err := h.manager.ValidateDomain(r.Context(), userAuth.AccountId, userAuth.UserId, domainID); err != nil {
util.WriteError(r.Context(), err, w)
return
}
w.WriteHeader(http.StatusAccepted)
w.WriteHeader(http.StatusNoContent)
}
@@ -0,0 +1,80 @@
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},
// A service is authorized by its own account's registration, so another
// account's service under this namespace is not a dependency of it.
{"other account", "example.com", "app.example.com", accountB, true, false},
{"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")
})
}
}
@@ -29,7 +29,7 @@ func TestValidateDomain_ExpiredRegistration(t *testing.T) {
Update("validation_expires_at", expiresAt).Error)
env.resolver.set("validation.expired.example.com", testCluster)
env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
assert.Error(t, env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID), "an expired registration must be refused")
stored := storedDomain(t, env.store, accountA, d.Domain)
require.NotNil(t, stored)
@@ -47,7 +47,7 @@ func TestCreateDomain_ValidationDeadline(t *testing.T) {
assert.Equal(t, createdAt.Add(48*time.Hour), *d.ValidationExpiresAt, "new registrations get 48 hours")
time.Sleep(time.Hour)
env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
assert.Error(t, env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID), "a missing CNAME must fail validation")
stored := storedDomain(t, env.store, accountA, d.Domain)
require.NotNil(t, stored)
require.NotNil(t, stored.ValidationExpiresAt)
@@ -198,9 +198,10 @@ func TestValidateDomain_DeadlinePassesDuringLookup(t *testing.T) {
resolver := blockingDomainResolver{started: make(chan struct{}), release: make(chan struct{})}
env.manager.validator.Resolver = resolver
done := make(chan struct{})
var validateErr error
go func() {
defer close(done)
env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
validateErr = env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
}()
<-resolver.started
time.Sleep(48 * time.Hour)
@@ -211,6 +212,7 @@ func TestValidateDomain_DeadlinePassesDuringLookup(t *testing.T) {
}
close(resolver.release)
<-done
assert.Error(t, validateErr, "a validation finishing after the deadline must be refused")
owner := accountA
if cleanup {
assert.Nil(t, storedDomain(t, env.store, accountA, d.Domain), "late validation must not restore the old claim")
@@ -227,22 +227,19 @@ func (m Manager) DeleteDomain(ctx context.Context, accountID, userID, domainID s
return nil
}
func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID string) {
ok, _, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Create)
// validationTimeout bounds the DNS check so it ends, and reports why, before a proxy in
// front of management times the request out.
const validationTimeout = 10 * time.Second
// ValidateDomain checks the domain's validation CNAME and marks it validated. A failure
// the user can act on is returned as PreconditionFailed carrying the reason.
func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID string) error {
ok, ctx, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Create)
if err != nil {
log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
}).WithError(err).Error("validate domain")
return
return status.NewPermissionValidationError(err)
}
if !ok {
log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
"userID": userID,
}).Error("validate domain: permission denied")
return
return status.NewPermissionDeniedError()
}
log.WithFields(log.Fields{
@@ -250,32 +247,23 @@ func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID
"domainID": domainID,
}).Info("starting domain validation")
d, err := m.store.GetCustomDomain(context.Background(), accountID, domainID)
d, err := m.store.GetCustomDomain(ctx, accountID, domainID)
if err != nil {
log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
}).WithError(err).Error("get custom domain from store")
return
return fmt.Errorf("get domain from store: %w", err)
}
if d.Validated {
return
return nil
}
if d.ValidationExpiresAt == nil || !time.Now().Before(*d.ValidationExpiresAt) {
log.WithFields(log.Fields{"accountID": accountID, "domainID": domainID}).
Debug("custom domain validation window has expired")
return
return validationFailed(domain.NewValidationError(domain.ValidationReasonExpired,
"custom domain %s validation window has expired; delete and add the domain again", d.Domain))
}
// Validate only against the domain's target cluster
targetCluster := d.TargetCluster
if targetCluster == "" {
log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
"domain": d.Domain,
}).Warn("domain has no target cluster set, skipping validation")
return
return validationFailed(domain.NewValidationError(domain.ValidationReasonNoTargetCluster,
"custom domain %s has no target cluster", d.Domain))
}
log.WithFields(log.Fields{
@@ -285,32 +273,35 @@ func (m Manager) ValidateDomain(ctx context.Context, accountID, userID, domainID
"targetCluster": targetCluster,
}).Info("validating domain against target cluster")
if m.validator.IsValid(context.Background(), d.Domain, []string{targetCluster}) {
lookupCtx, cancel := context.WithTimeout(ctx, validationTimeout)
_, err = m.validator.Validate(lookupCtx, d.Domain, []string{targetCluster})
cancel()
if err == nil {
d.Validated = true
if _, err := m.store.UpdateCustomDomain(context.Background(), accountID, d); err != nil {
entry := log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
}).WithError(err)
if sErr, ok := status.FromError(err); ok && sErr.Type() == status.PreconditionFailed {
entry.Debug("custom domain registration is no longer pending validation")
return
if _, err := m.store.UpdateCustomDomain(ctx, accountID, d); err != nil {
// A concurrent request may have validated the domain first.
if current, getErr := m.store.GetCustomDomain(ctx, accountID, domainID); getErr == nil && current.Validated {
return nil
}
entry.Error("update custom domain in store")
return
return fmt.Errorf("update domain in store: %w", err)
}
log.WithFields(log.Fields{"accountID": accountID, "domainID": domainID}).
Info("custom domain validated successfully")
m.accountManager.StoreEvent(context.Background(), userID, domainID, accountID, activity.DomainValidated, d.EventMeta())
} else {
log.WithFields(log.Fields{
"accountID": accountID,
"domainID": domainID,
"domain": d.Domain,
"targetCluster": targetCluster,
}).Warn("domain validation failed - CNAME does not match target cluster")
m.accountManager.StoreEvent(ctx, userID, domainID, accountID, activity.DomainValidated, d.EventMeta())
return nil
}
return validationFailed(err)
}
// validationFailed shows a classified failure to the user and keeps anything else internal.
func validationFailed(err error) error {
var vErr *domain.ValidationError
if errors.As(err, &vErr) {
return status.Errorf(status.PreconditionFailed, "%s", vErr.Message)
}
return fmt.Errorf("validate domain: %w", err)
}
// GetClusterDomains returns a list of proxy cluster domains.
@@ -357,6 +348,26 @@ 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
}
name, err := nbdomain.FromString(serviceDomain)
if err != nil {
return status.Errorf(status.InvalidArgument, "invalid service domain: %v", err)
}
customDomains, err := tx.LockCustomDomains(ctx, accountID, name)
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 {
@@ -99,7 +99,7 @@ func setupDomainTest(t *testing.T) *domainTestEnv {
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)
_, 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)}
@@ -229,7 +229,7 @@ func TestValidateDomain_UnlocksClusterDerivation(t *testing.T) {
require.Error(t, err)
env.resolver.set("validation.later.example.com", testCluster)
env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID)
require.NoError(t, env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID))
require.True(t, storedDomain(t, env.store, accountA, "later.example.com").Validated)
@@ -286,7 +286,7 @@ func TestValidateDomain_PermissionDeniedDoesNotValidate(t *testing.T) {
// a validated domain is the permission check.
env.resolver.set("validation.guarded.example.com", testCluster)
env.manager.ValidateDomain(ctx, accountA, accountAMember, created.ID)
assert.Error(t, env.manager.ValidateDomain(ctx, accountA, accountAMember, created.ID), "a caller without permission must be refused")
stored := storedDomain(t, env.store, accountA, "guarded.example.com")
require.NotNil(t, stored)
@@ -52,7 +52,7 @@ func TestCreateDomain_NormalizedNameCanValidateLater(t *testing.T) {
require.False(t, created.Validated, "a missing CNAME must leave the normalized registration pending")
env.resolver.set("validation.apps.example.com", testCluster)
env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID)
require.NoError(t, env.manager.ValidateDomain(ctx, accountA, accountAUser, created.ID))
stored, err := env.store.GetCustomDomain(ctx, accountA, created.ID)
require.NoError(t, err)
assert.Equal(t, "apps.example.com", stored.Domain, "retrying validation must retain the normalized name")
@@ -0,0 +1,151 @@
package manager
import (
"context"
"testing"
"testing/synctest"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
nbstore "github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/shared/management/status"
)
// The validator tests cover each DNS outcome. These cover what the manager adds on top:
// the status type the API turns into a response code, and the checks before the lookup.
func TestValidateDomain_ReturnsReason(t *testing.T) {
ctx := context.Background()
tests := []struct {
name string
setup func(t *testing.T, env *domainTestEnv, d *domain.Domain)
user string
want status.Type
message string
}{
{
name: "cname mismatch",
setup: func(_ *testing.T, env *domainTestEnv, d *domain.Domain) {
env.resolver.set("validation."+d.Domain, "other.example.net")
},
want: status.PreconditionFailed,
message: "CNAME record validation.test.example.com points to other.example.net; point it to " + testCluster,
},
{
name: "expired window",
setup: func(t *testing.T, env *domainTestEnv, d *domain.Domain) {
setDomainColumn(t, env, d, "validation_expires_at", time.Now().Add(-time.Second))
},
want: status.PreconditionFailed,
message: "custom domain test.example.com validation window has expired; delete and add the domain again",
},
{
name: "no target cluster",
setup: func(t *testing.T, env *domainTestEnv, d *domain.Domain) {
setDomainColumn(t, env, d, "target_cluster", "")
},
want: status.PreconditionFailed,
message: "custom domain test.example.com has no target cluster",
},
{
name: "permission denied",
setup: func(_ *testing.T, env *domainTestEnv, d *domain.Domain) {
env.resolver.set("validation."+d.Domain, testCluster)
},
user: accountAMember,
want: status.PermissionDenied,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
env := setupDomainTest(t)
d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "test.example.com", testCluster)
require.NoError(t, err)
tt.setup(t, env, d)
user := accountAUser
if tt.user != "" {
user = tt.user
}
err = env.manager.ValidateDomain(ctx, accountA, user, d.ID)
sErr, ok := status.FromError(err)
require.True(t, ok, "expected a status error, got %v", err)
assert.Equal(t, tt.want, sErr.Type(), "status type")
if tt.message != "" {
assert.Equal(t, tt.message, sErr.Message, "user-facing message")
}
assert.False(t, storedDomain(t, env.store, accountA, d.Domain).Validated, "a failed check must not validate the domain")
})
}
}
func TestValidateDomain_SuccessIsIdempotent(t *testing.T) {
ctx := context.Background()
env := setupDomainTest(t)
d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "ok.example.com", testCluster)
require.NoError(t, err)
env.resolver.set("validation.ok.example.com", testCluster)
require.NoError(t, env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID))
assert.True(t, storedDomain(t, env.store, accountA, d.Domain).Validated, "domain should be validated")
assert.NoError(t, env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID), "an already validated domain is a success")
}
func setDomainColumn(t *testing.T, env *domainTestEnv, d *domain.Domain, column string, value any) {
t.Helper()
db := env.store.(*nbstore.SqlStore).GetDB()
require.NoError(t, db.Model(&domain.Domain{}).Where("id = ?", d.ID).Update(column, value).Error)
}
// Two verify requests can both read the domain as pending. The one that loses the store
// update must still report success when the other one validated the domain.
func TestValidateDomain_ConcurrentValidationSucceeds(t *testing.T) {
ctx := context.Background()
env := setupDomainTest(t)
d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "twice.example.com", testCluster)
require.NoError(t, err)
resolver := blockingDomainResolver{started: make(chan struct{}), release: make(chan struct{})}
env.manager.validator.Resolver = resolver
done := make(chan error, 1)
go func() {
done <- env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
}()
<-resolver.started
// The other request completes while this one is still looking up the CNAME.
setDomainColumn(t, env, d, "validated", true)
close(resolver.release)
assert.NoError(t, <-done, "a domain validated by a concurrent request is a success")
}
// hangingResolver never answers, so only a deadline on the context ends the lookup.
type hangingResolver struct{}
func (hangingResolver) LookupCNAME(ctx context.Context, _ string) (string, error) {
<-ctx.Done()
return "", ctx.Err()
}
// A lookup that hangs must end on the validation timeout and still report the reason,
// rather than leaving the request to whatever timeout sits in front of management.
func TestValidateDomain_LookupTimeout(t *testing.T) {
env := setupClockDomainTest(t)
synctest.Test(t, func(t *testing.T) {
ctx := context.Background()
d, err := env.manager.CreateDomain(ctx, accountA, accountAUser, "slow.example.com", testCluster)
require.NoError(t, err)
env.manager.validator.Resolver = hangingResolver{}
start := time.Now()
err = env.manager.ValidateDomain(ctx, accountA, accountAUser, d.ID)
sErr, ok := status.FromError(err)
require.True(t, ok, "expected a status error, got %v", err)
assert.Equal(t, status.PreconditionFailed, sErr.Type(), "status type")
assert.Equal(t, "DNS lookup for validation.slow.example.com failed; retry the validation", sErr.Message, "user-facing message")
assert.Equal(t, validationTimeout, time.Since(start), "the lookup must end on the validation timeout")
})
}
@@ -2,6 +2,8 @@ package domain
import (
"context"
"errors"
"fmt"
"net"
"strings"
@@ -16,6 +18,33 @@ type Validator struct {
Resolver resolver
}
// ValidationReason classifies why a custom domain failed validation.
type ValidationReason string
const (
ValidationReasonCNAMENotFound ValidationReason = "cname_not_found"
ValidationReasonCNAMEMismatch ValidationReason = "cname_mismatch"
ValidationReasonLookupFailed ValidationReason = "lookup_failed"
ValidationReasonExpired ValidationReason = "validation_expired"
ValidationReasonNoTargetCluster ValidationReason = "no_target_cluster"
)
// ValidationError is a validation failure whose message is safe to show to the user.
type ValidationError struct {
Reason ValidationReason
Message string
}
// Error returns the user-facing message.
func (e *ValidationError) Error() string {
return e.Message
}
// NewValidationError returns a ValidationError with a formatted message.
func NewValidationError(reason ValidationReason, format string, args ...any) *ValidationError {
return &ValidationError{Reason: reason, Message: fmt.Sprintf(format, args...)}
}
// NewValidator initializes a validator with a specific DNS Resolver.
// If a Validator is used without specifying a Resolver, then it will
// use the net.DefaultResolver.
@@ -39,6 +68,13 @@ func (v *Validator) IsValid(ctx context.Context, domain string, accept []string)
// ValidateWithCluster validates a custom domain and returns the matched cluster address.
// Returns the cluster address and true if valid, or empty string and false if invalid.
func (v *Validator) ValidateWithCluster(ctx context.Context, domain string, accept []string) (string, bool) {
cluster, err := v.Validate(ctx, domain, accept)
return cluster, err == nil
}
// Validate validates a custom domain and returns the matched cluster address.
// On failure it returns a *ValidationError whose message is safe to show to the user.
func (v *Validator) Validate(ctx context.Context, domain string, accept []string) (string, error) {
if v.Resolver == nil {
v.Resolver = net.DefaultResolver
}
@@ -56,10 +92,19 @@ func (v *Validator) ValidateWithCluster(ctx context.Context, domain string, acce
"domain": domain,
"lookupDomain": lookupDomain,
}).WithError(err).Warn("CNAME lookup failed for domain validation")
return "", false
// Never forward the resolver error: its text includes the resolver address.
var dnsErr *net.DNSError
if errors.As(err, &dnsErr) && dnsErr.IsNotFound {
return "", cnameNotFound(lookupDomain, accept)
}
return "", NewValidationError(ValidationReasonLookupFailed, "DNS lookup for %s failed; retry the validation", lookupDomain)
}
nakedCNAME := strings.TrimSuffix(cname, ".")
// A name without a CNAME record resolves to itself.
if strings.EqualFold(nakedCNAME, lookupDomain) {
return "", cnameNotFound(lookupDomain, accept)
}
log.WithFields(log.Fields{
"domain": domain,
"cname": cname,
@@ -69,13 +114,13 @@ func (v *Validator) ValidateWithCluster(ctx context.Context, domain string, acce
for _, acceptDomain := range accept {
normalizedAccept := strings.TrimSuffix(acceptDomain, ".")
if nakedCNAME == normalizedAccept {
if strings.EqualFold(nakedCNAME, normalizedAccept) {
log.WithFields(log.Fields{
"domain": domain,
"cname": nakedCNAME,
"cluster": acceptDomain,
}).Info("domain CNAME matched cluster")
return acceptDomain, true
return acceptDomain, nil
}
}
@@ -84,5 +129,11 @@ func (v *Validator) ValidateWithCluster(ctx context.Context, domain string, acce
"cname": nakedCNAME,
"acceptList": accept,
}).Warn("domain CNAME does not match any accepted cluster")
return "", false
return "", NewValidationError(ValidationReasonCNAMEMismatch, "CNAME record %s points to %s; point it to %s",
lookupDomain, nakedCNAME, strings.Join(accept, ", "))
}
func cnameNotFound(lookupDomain string, accept []string) *ValidationError {
return NewValidationError(ValidationReasonCNAMENotFound, "no CNAME record found for %s; point it to %s",
lookupDomain, strings.Join(accept, ", "))
}
@@ -2,6 +2,8 @@ package domain_test
import (
"context"
"errors"
"net"
"testing"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
@@ -36,6 +38,12 @@ func TestIsValid(t *testing.T) {
accept: []string{"bar.example.com"},
expect: false,
},
"match in other case": {
resolver: resolver{"Bar.Example.COM."},
domain: "foo.example.com",
accept: []string{"bar.example.com"},
expect: true,
},
"accept trailing dot": {
resolver: resolver{"bar.example.com."},
domain: "foo.example.com",
@@ -54,3 +62,61 @@ func TestIsValid(t *testing.T) {
})
}
}
type errResolver struct {
err error
}
func (r errResolver) LookupCNAME(context.Context, string) (string, error) {
return "", r.err
}
func TestValidate_Reason(t *testing.T) {
const notFound = "no CNAME record found for validation.foo.example.com; point it to eu.proxy.example.com"
tests := map[string]struct {
resolver interface {
LookupCNAME(context.Context, string) (string, error)
}
reason domain.ValidationReason
message string
}{
"mismatch": {
resolver: resolver{"other.example.net."},
reason: domain.ValidationReasonCNAMEMismatch,
message: "CNAME record validation.foo.example.com points to other.example.net; point it to eu.proxy.example.com",
},
"no cname resolves to itself": {
resolver: resolver{"validation.foo.example.com."},
reason: domain.ValidationReasonCNAMENotFound,
message: notFound,
},
"no cname resolves to itself in other case": {
resolver: resolver{"Validation.Foo.example.com."},
reason: domain.ValidationReasonCNAMENotFound,
message: notFound,
},
"not found": {
resolver: errResolver{&net.DNSError{Err: "no such host", Name: "validation.foo.example.com", Server: "10.0.0.2:53", IsNotFound: true}},
reason: domain.ValidationReasonCNAMENotFound,
message: notFound,
},
"other error is not forwarded": {
resolver: errResolver{errors.New("dial udp 10.0.0.2:53: connect: network is unreachable")},
reason: domain.ValidationReasonLookupFailed,
message: "DNS lookup for validation.foo.example.com failed; retry the validation",
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
_, err := domain.NewValidator(test.resolver).Validate(t.Context(), "foo.example.com", []string{"eu.proxy.example.com"})
var vErr *domain.ValidationError
if !errors.As(err, &vErr) {
t.Fatalf("expected a *domain.ValidationError, got %v", err)
}
if vErr.Reason != test.reason || vErr.Message != test.message {
t.Errorf("expect %q %q, actual %q %q", test.reason, test.message, vErr.Reason, vErr.Message)
}
})
}
}
@@ -11,7 +11,7 @@ import (
// Manager defines the interface for proxy operations
type Manager interface {
Connect(ctx context.Context, proxyID, sessionID, clusterAddress, ipAddress string, accountID *string, capabilities *Capabilities) (*Proxy, error)
Connect(ctx context.Context, proxyID, sessionID, clusterAddress, ipAddress, version string, accountID *string, capabilities *Capabilities) (*Proxy, error)
Disconnect(ctx context.Context, proxyID, sessionID string) error
Heartbeat(ctx context.Context, p *Proxy) error
GetActiveClusterAddresses(ctx context.Context) ([]string, error)
@@ -20,6 +20,8 @@ type Manager interface {
ClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool
ClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool
ClusterSupportsPrivate(ctx context.Context, clusterAddr string) *bool
ClusterAllProxiesPrivate(ctx context.Context, clusterAddr string) *bool
ClusterSupportsSessionCode(ctx context.Context, clusterAddr string) bool
CleanupStale(ctx context.Context, inactivityDuration time.Duration) error
GetAccountProxy(ctx context.Context, accountID string) (*Proxy, error)
CountAccountProxies(ctx context.Context, accountID string) (int64, error)
@@ -8,6 +8,7 @@ import (
"go.opentelemetry.io/otel/metric"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
nbversion "github.com/netbirdio/netbird/version"
)
// store defines the interface for proxy persistence operations
@@ -22,6 +23,8 @@ type store interface {
GetClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool
GetClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool
GetClusterSupportsPrivate(ctx context.Context, clusterAddr string) *bool
GetClusterAllProxiesPrivate(ctx context.Context, clusterAddr string) *bool
GetActiveProxyVersions(ctx context.Context, clusterAddr string) ([]string, error)
CleanupStaleProxies(ctx context.Context, inactivityDuration time.Duration) error
GetProxyByAccountID(ctx context.Context, accountID string) (*proxy.Proxy, error)
CountProxiesByAccountID(ctx context.Context, accountID string) (int64, error)
@@ -29,6 +32,8 @@ type store interface {
DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error
}
const minSessionCodeVersion = "0.81.0"
// Manager handles all proxy operations
type Manager struct {
store store
@@ -50,7 +55,7 @@ func NewManager(store store, meter metric.Meter) (*Manager, error) {
// Connect registers a new proxy connection in the database.
// capabilities may be nil for old proxies that do not report them.
func (m *Manager) Connect(ctx context.Context, proxyID, sessionID, clusterAddress, ipAddress string, accountID *string, capabilities *proxy.Capabilities) (*proxy.Proxy, error) {
func (m *Manager) Connect(ctx context.Context, proxyID, sessionID, clusterAddress, ipAddress, version string, accountID *string, capabilities *proxy.Capabilities) (*proxy.Proxy, error) {
now := time.Now()
var caps proxy.Capabilities
if capabilities != nil {
@@ -61,6 +66,7 @@ func (m *Manager) Connect(ctx context.Context, proxyID, sessionID, clusterAddres
SessionID: sessionID,
ClusterAddress: clusterAddress,
IPAddress: ipAddress,
Version: truncateVersion(version),
AccountID: accountID,
LastSeen: now,
ConnectedAt: &now,
@@ -78,6 +84,7 @@ func (m *Manager) Connect(ctx context.Context, proxyID, sessionID, clusterAddres
"sessionID": sessionID,
"clusterAddress": clusterAddress,
"ipAddress": ipAddress,
"version": p.Version,
}).Info("proxy connected")
return p, nil
@@ -143,6 +150,27 @@ func (m Manager) ClusterSupportsPrivate(ctx context.Context, clusterAddr string)
return m.store.GetClusterSupportsPrivate(ctx, clusterAddr)
}
// ClusterAllProxiesPrivate reports whether every active proxy claims the private capability (nil = unreported).
func (m Manager) ClusterAllProxiesPrivate(ctx context.Context, clusterAddr string) *bool {
return m.store.GetClusterAllProxiesPrivate(ctx, clusterAddr)
}
// ClusterSupportsSessionCode reports whether all active proxies support session codes.
func (m Manager) ClusterSupportsSessionCode(ctx context.Context, clusterAddr string) bool {
versions, err := m.store.GetActiveProxyVersions(ctx, clusterAddr)
if err != nil || len(versions) == 0 {
return false
}
for _, version := range versions {
if supported, err := nbversion.MeetsMinVersion(minSessionCodeVersion, version); err != nil || !supported {
return false
}
}
return true
}
// CleanupStale removes proxies that haven't sent heartbeat in the specified duration
func (m *Manager) CleanupStale(ctx context.Context, inactivityDuration time.Duration) error {
if err := m.store.CleanupStaleProxies(ctx, inactivityDuration); err != nil {
@@ -184,3 +212,13 @@ func (m *Manager) DeleteAccountCluster(ctx context.Context, clusterAddress, acco
}
return nil
}
// truncateVersion cuts a proxy-reported version to the column width so an
// oversized value cannot fail the save and block the connect.
func truncateVersion(version string) string {
runes := []rune(version)
if len(runes) <= proxy.MaxVersionLength {
return version
}
return string(runes[:proxy.MaxVersionLength])
}
@@ -4,8 +4,10 @@ import (
"context"
"errors"
"fmt"
"strings"
"testing"
"time"
"unicode/utf8"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -20,6 +22,7 @@ type mockStore struct {
updateProxyHeartbeatFunc func(ctx context.Context, p *proxy.Proxy) error
getActiveProxyClusterAddressesFunc func(ctx context.Context) ([]string, error)
getActiveProxyClusterAddressesForAccFunc func(ctx context.Context, accountID string) ([]string, error)
getActiveProxyVersionsFunc func(ctx context.Context, clusterAddress string) ([]string, error)
cleanupStaleProxiesFunc func(ctx context.Context, d time.Duration) error
getProxyByAccountIDFunc func(ctx context.Context, accountID string) (*proxy.Proxy, error)
countProxiesByAccountIDFunc func(ctx context.Context, accountID string) (int64, error)
@@ -102,6 +105,15 @@ func (m *mockStore) GetClusterSupportsCrowdSec(_ context.Context, _ string) *boo
func (m *mockStore) GetClusterSupportsPrivate(_ context.Context, _ string) *bool {
return nil
}
func (m *mockStore) GetClusterAllProxiesPrivate(_ context.Context, _ string) *bool {
return nil
}
func (m *mockStore) GetActiveProxyVersions(ctx context.Context, clusterAddress string) ([]string, error) {
if m.getActiveProxyVersionsFunc != nil {
return m.getActiveProxyVersionsFunc(ctx, clusterAddress)
}
return nil, nil
}
func newTestManager(s store) *Manager {
meter := noop.NewMeterProvider().Meter("test")
@@ -112,6 +124,34 @@ func newTestManager(s store) *Manager {
return m
}
func TestClusterSupportsSessionCode(t *testing.T) {
tests := []struct {
name string
versions []string
storeErr error
want bool
}{
{name: "all supported", versions: []string{"0.81.0", "0.81.2"}, want: true},
{name: "one old proxy", versions: []string{"0.81.0", "0.80.0"}},
{name: "missing version", versions: []string{"0.81.0", ""}},
{name: "no active proxies"},
{name: "store error", storeErr: errors.New("db error")},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := &mockStore{
getActiveProxyVersionsFunc: func(_ context.Context, _ string) ([]string, error) {
return tt.versions, tt.storeErr
},
}
got := newTestManager(s).ClusterSupportsSessionCode(context.Background(), "cluster.example.com")
assert.Equal(t, tt.want, got)
})
}
}
func TestConnect_WithAccountID(t *testing.T) {
accountID := "acc-123"
@@ -124,7 +164,7 @@ func TestConnect_WithAccountID(t *testing.T) {
}
mgr := newTestManager(s)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "cluster.example.com", "10.0.0.1", &accountID, nil)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "cluster.example.com", "10.0.0.1", "0.60.0", &accountID, nil)
require.NoError(t, err)
require.NotNil(t, savedProxy)
@@ -132,6 +172,7 @@ func TestConnect_WithAccountID(t *testing.T) {
assert.Equal(t, "session-1", savedProxy.SessionID)
assert.Equal(t, "cluster.example.com", savedProxy.ClusterAddress)
assert.Equal(t, "10.0.0.1", savedProxy.IPAddress)
assert.Equal(t, "0.60.0", savedProxy.Version, "reported proxy version should be stored")
assert.Equal(t, &accountID, savedProxy.AccountID)
assert.Equal(t, proxy.StatusConnected, savedProxy.Status)
assert.NotNil(t, savedProxy.ConnectedAt)
@@ -147,7 +188,7 @@ func TestConnect_WithoutAccountID(t *testing.T) {
}
mgr := newTestManager(s)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "eu.proxy.netbird.io", "10.0.0.1", nil, nil)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "eu.proxy.netbird.io", "10.0.0.1", "", nil, nil)
require.NoError(t, err)
require.NotNil(t, savedProxy)
@@ -155,6 +196,29 @@ func TestConnect_WithoutAccountID(t *testing.T) {
assert.Equal(t, proxy.StatusConnected, savedProxy.Status)
}
func TestConnect_TruncatesOversizedVersion(t *testing.T) {
var savedProxy *proxy.Proxy
s := &mockStore{
saveProxyFunc: func(_ context.Context, p *proxy.Proxy) error {
savedProxy = p
return nil
},
}
// Multi-byte runes make sure the cut counts characters, as varchar does,
// and never splits a rune into invalid UTF-8.
version := strings.Repeat("ü", proxy.MaxVersionLength+10)
mgr := newTestManager(s)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "cluster.example.com", "10.0.0.1", version, nil, nil)
require.NoError(t, err)
require.NotNil(t, savedProxy)
assert.Equal(t, proxy.MaxVersionLength, utf8.RuneCountInString(savedProxy.Version), "stored version should be cut to the column width")
assert.True(t, utf8.ValidString(savedProxy.Version), "stored version should remain valid UTF-8")
assert.True(t, strings.HasPrefix(version, savedProxy.Version), "stored version should be a prefix of the reported one")
}
func TestConnect_StoreError(t *testing.T) {
s := &mockStore{
saveProxyFunc: func(_ context.Context, _ *proxy.Proxy) error {
@@ -163,7 +227,7 @@ func TestConnect_StoreError(t *testing.T) {
}
mgr := newTestManager(s)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "cluster.example.com", "10.0.0.1", nil, nil)
_, err := mgr.Connect(context.Background(), "proxy-1", "session-1", "cluster.example.com", "10.0.0.1", "", nil, nil)
assert.Error(t, err)
}
@@ -56,6 +56,20 @@ func (mr *MockManagerMockRecorder) CleanupStale(ctx, inactivityDuration any) *go
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanupStale", reflect.TypeOf((*MockManager)(nil).CleanupStale), ctx, inactivityDuration)
}
// ClusterAllProxiesPrivate mocks base method.
func (m *MockManager) ClusterAllProxiesPrivate(ctx context.Context, clusterAddr string) *bool {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ClusterAllProxiesPrivate", ctx, clusterAddr)
ret0, _ := ret[0].(*bool)
return ret0
}
// ClusterAllProxiesPrivate indicates an expected call of ClusterAllProxiesPrivate.
func (mr *MockManagerMockRecorder) ClusterAllProxiesPrivate(ctx, clusterAddr any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterAllProxiesPrivate", reflect.TypeOf((*MockManager)(nil).ClusterAllProxiesPrivate), ctx, clusterAddr)
}
// ClusterRequireSubdomain mocks base method.
func (m *MockManager) ClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool {
m.ctrl.T.Helper()
@@ -112,19 +126,33 @@ func (mr *MockManagerMockRecorder) ClusterSupportsPrivate(ctx, clusterAddr any)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterSupportsPrivate", reflect.TypeOf((*MockManager)(nil).ClusterSupportsPrivate), ctx, clusterAddr)
}
// Connect mocks base method.
func (m *MockManager) Connect(ctx context.Context, proxyID, sessionID, clusterAddress, ipAddress string, accountID *string, capabilities *Capabilities) (*Proxy, error) {
// ClusterSupportsSessionCode mocks base method.
func (m *MockManager) ClusterSupportsSessionCode(ctx context.Context, clusterAddr string) bool {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Connect", ctx, proxyID, sessionID, clusterAddress, ipAddress, accountID, capabilities)
ret := m.ctrl.Call(m, "ClusterSupportsSessionCode", ctx, clusterAddr)
ret0, _ := ret[0].(bool)
return ret0
}
// ClusterSupportsSessionCode indicates an expected call of ClusterSupportsSessionCode.
func (mr *MockManagerMockRecorder) ClusterSupportsSessionCode(ctx, clusterAddr any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterSupportsSessionCode", reflect.TypeOf((*MockManager)(nil).ClusterSupportsSessionCode), ctx, clusterAddr)
}
// Connect mocks base method.
func (m *MockManager) Connect(ctx context.Context, proxyID, sessionID, clusterAddress, ipAddress, version string, accountID *string, capabilities *Capabilities) (*Proxy, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Connect", ctx, proxyID, sessionID, clusterAddress, ipAddress, version, accountID, capabilities)
ret0, _ := ret[0].(*Proxy)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// Connect indicates an expected call of Connect.
func (mr *MockManagerMockRecorder) Connect(ctx, proxyID, sessionID, clusterAddress, ipAddress, accountID, capabilities any) *gomock.Call {
func (mr *MockManagerMockRecorder) Connect(ctx, proxyID, sessionID, clusterAddress, ipAddress, version, accountID, capabilities any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Connect", reflect.TypeOf((*MockManager)(nil).Connect), ctx, proxyID, sessionID, clusterAddress, ipAddress, accountID, capabilities)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Connect", reflect.TypeOf((*MockManager)(nil).Connect), ctx, proxyID, sessionID, clusterAddress, ipAddress, version, accountID, capabilities)
}
// CountAccountProxies mocks base method.
@@ -9,6 +9,9 @@ const (
StatusDisconnected = "disconnected"
)
// MaxVersionLength is the width of the Version column, in characters.
const MaxVersionLength = 255
// Capabilities describes what a proxy can handle, as reported via gRPC.
// Nil fields mean the proxy never reported this capability.
type Capabilities struct {
@@ -31,6 +34,7 @@ type Proxy struct {
SessionID string `gorm:"type:varchar(36)"`
ClusterAddress string `gorm:"type:varchar(255);not null;index:idx_proxy_cluster_status"`
IPAddress string `gorm:"type:varchar(45)"`
Version string `gorm:"type:varchar(255)"`
AccountID *string `gorm:"type:varchar(255);index:idx_proxy_account_id"`
LastSeen time.Time `gorm:"not null;index:idx_proxy_last_seen"`
ConnectedAt *time.Time
@@ -1,6 +1,7 @@
package proxytoken
import (
"context"
"encoding/json"
"net/http"
"time"
@@ -18,13 +19,29 @@ import (
"github.com/netbirdio/netbird/shared/management/status"
)
// RevocationGuard vetoes the tenant-facing revocation of a proxy access
// token. Implementations are supplied by integrations; none is installed by
// default, so every token the caller's account owns may be revoked. It is
// consulted after the ownership check and before the token is revoked. A
// returned status error is written with util.WriteError: its type selects the
// HTTP status and its message is shown to the caller, so it must not carry
// internal detail. Any other error is reported as a generic internal error.
type RevocationGuard interface {
CheckProxyAccessTokenRevocation(ctx context.Context, token *types.ProxyAccessToken) error
}
type handler struct {
store store.Store
permissionsManager permissions.Manager
// revocationGuard vetoes revocations. Optional — when nil every owned
// token may be revoked.
revocationGuard RevocationGuard
}
func RegisterEndpoints(s store.Store, permissionsManager permissions.Manager, router *mux.Router) {
h := &handler{store: s, permissionsManager: permissionsManager}
// RegisterEndpoints registers the proxy token endpoints. revocationGuard is
// optional; pass nil for no revocation policy.
func RegisterEndpoints(s store.Store, permissionsManager permissions.Manager, revocationGuard RevocationGuard, router *mux.Router) {
h := &handler{store: s, permissionsManager: permissionsManager, revocationGuard: revocationGuard}
router.HandleFunc("/reverse-proxies/proxy-tokens", h.listTokens).Methods("GET", "OPTIONS")
router.HandleFunc("/reverse-proxies/proxy-tokens", h.createToken).Methods("POST", "OPTIONS")
router.HandleFunc("/reverse-proxies/proxy-tokens/{tokenId}", h.revokeToken).Methods("DELETE", "OPTIONS")
@@ -154,6 +171,13 @@ func (h *handler) revokeToken(w http.ResponseWriter, r *http.Request) {
return
}
if h.revocationGuard != nil {
if err := h.revocationGuard.CheckProxyAccessTokenRevocation(ctx, token); err != nil {
util.WriteError(ctx, err, w)
return
}
}
if err := h.store.RevokeProxyAccessToken(ctx, tokenID); err != nil {
util.WriteErrorResponse("failed to revoke token", http.StatusInternalServerError, w)
return
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"testing"
@@ -22,6 +23,7 @@ import (
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/shared/auth"
"github.com/netbirdio/netbird/shared/management/http/api"
"github.com/netbirdio/netbird/shared/management/status"
)
func authContext(accountID, userID string) context.Context {
@@ -273,3 +275,152 @@ func TestRevokeToken_ManagementWideToken(t *testing.T) {
h.revokeToken(w, req)
assert.Equal(t, http.StatusNotFound, w.Code)
}
type revocationGuardFunc func(ctx context.Context, token *types.ProxyAccessToken) error
func (f revocationGuardFunc) CheckProxyAccessTokenRevocation(ctx context.Context, token *types.ProxyAccessToken) error {
return f(ctx, token)
}
func TestRevokeToken_GuardRefuses(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
accountID := "acc-123"
// No RevokeProxyAccessToken expectation: a refused revocation must not
// reach the store.
mockStore := store.NewMockStore(ctrl)
mockStore.EXPECT().GetProxyAccessTokenByID(gomock.Any(), store.LockingStrengthNone, "tok-1").Return(&types.ProxyAccessToken{
ID: "tok-1",
AccountID: &accountID,
}, nil)
permsMgr := permissions.NewMockManager(ctrl)
permsMgr.EXPECT().ValidateUserPermissions(gomock.Any(), accountID, "user-1", modules.Services, operations.Delete).Return(true, context.Background(), nil)
var checked *types.ProxyAccessToken
h := &handler{
store: mockStore,
permissionsManager: permsMgr,
revocationGuard: revocationGuardFunc(func(_ context.Context, token *types.ProxyAccessToken) error {
checked = token
return status.Errorf(status.PreconditionFailed, "token is in use")
}),
}
req := httptest.NewRequest("DELETE", "/reverse-proxies/proxy-tokens/tok-1", nil)
req = req.WithContext(authContext(accountID, "user-1"))
req = mux.SetURLVars(req, map[string]string{"tokenId": "tok-1"})
w := httptest.NewRecorder()
h.revokeToken(w, req)
assert.Equal(t, http.StatusPreconditionFailed, w.Code)
assert.Contains(t, w.Body.String(), "token is in use")
require.NotNil(t, checked)
assert.Equal(t, "tok-1", checked.ID)
}
func TestRevokeToken_GuardAllows(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
accountID := "acc-123"
mockStore := store.NewMockStore(ctrl)
mockStore.EXPECT().GetProxyAccessTokenByID(gomock.Any(), store.LockingStrengthNone, "tok-1").Return(&types.ProxyAccessToken{
ID: "tok-1",
AccountID: &accountID,
}, nil)
mockStore.EXPECT().RevokeProxyAccessToken(gomock.Any(), "tok-1").Return(nil)
permsMgr := permissions.NewMockManager(ctrl)
permsMgr.EXPECT().ValidateUserPermissions(gomock.Any(), accountID, "user-1", modules.Services, operations.Delete).Return(true, context.Background(), nil)
h := &handler{
store: mockStore,
permissionsManager: permsMgr,
revocationGuard: revocationGuardFunc(func(context.Context, *types.ProxyAccessToken) error {
return nil
}),
}
req := httptest.NewRequest("DELETE", "/reverse-proxies/proxy-tokens/tok-1", nil)
req = req.WithContext(authContext(accountID, "user-1"))
req = mux.SetURLVars(req, map[string]string{"tokenId": "tok-1"})
w := httptest.NewRecorder()
h.revokeToken(w, req)
assert.Equal(t, http.StatusOK, w.Code)
}
func TestRevokeToken_GuardFailure(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
accountID := "acc-123"
// No RevokeProxyAccessToken expectation: a guard that cannot decide must
// not let the revocation through.
mockStore := store.NewMockStore(ctrl)
mockStore.EXPECT().GetProxyAccessTokenByID(gomock.Any(), store.LockingStrengthNone, "tok-1").Return(&types.ProxyAccessToken{
ID: "tok-1",
AccountID: &accountID,
}, nil)
permsMgr := permissions.NewMockManager(ctrl)
permsMgr.EXPECT().ValidateUserPermissions(gomock.Any(), accountID, "user-1", modules.Services, operations.Delete).Return(true, context.Background(), nil)
h := &handler{
store: mockStore,
permissionsManager: permsMgr,
revocationGuard: revocationGuardFunc(func(context.Context, *types.ProxyAccessToken) error {
return errors.New("connection refused")
}),
}
req := httptest.NewRequest("DELETE", "/reverse-proxies/proxy-tokens/tok-1", nil)
req = req.WithContext(authContext(accountID, "user-1"))
req = mux.SetURLVars(req, map[string]string{"tokenId": "tok-1"})
w := httptest.NewRecorder()
h.revokeToken(w, req)
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Contains(t, w.Body.String(), "internal server error")
assert.NotContains(t, w.Body.String(), "connection refused")
}
func TestRevokeToken_GuardNotConsultedForForeignToken(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
otherAccount := "acc-other"
mockStore := store.NewMockStore(ctrl)
mockStore.EXPECT().GetProxyAccessTokenByID(gomock.Any(), store.LockingStrengthNone, "tok-1").Return(&types.ProxyAccessToken{
ID: "tok-1",
AccountID: &otherAccount,
}, nil)
permsMgr := permissions.NewMockManager(ctrl)
permsMgr.EXPECT().ValidateUserPermissions(gomock.Any(), "acc-123", "user-1", modules.Services, operations.Delete).Return(true, context.Background(), nil)
// A foreign token must read as not found, not reveal through the guard's
// answer that it belongs to some account's managed proxy.
h := &handler{
store: mockStore,
permissionsManager: permsMgr,
revocationGuard: revocationGuardFunc(func(context.Context, *types.ProxyAccessToken) error {
t.Fatal("guard consulted for a token the caller does not own")
return nil
}),
}
req := httptest.NewRequest("DELETE", "/reverse-proxies/proxy-tokens/tok-1", nil)
req = req.WithContext(authContext("acc-123", "user-1"))
req = mux.SetURLVars(req, map[string]string{"tokenId": "tok-1"})
w := httptest.NewRecorder()
h.revokeToken(w, req)
assert.Equal(t, http.StatusNotFound, w.Code)
}
@@ -30,7 +30,7 @@ func withRealDomainManager(t *testing.T, mgr *Manager, testStore store.Store) {
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)
_, err = proxyMgr.Connect(ctx, "proxy-1", "session-1", validationTestCluster, "127.0.0.1", "", nil, nil)
require.NoError(t, err)
accountMgr := &mock_server.MockAccountManager{
@@ -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")
}
@@ -2,6 +2,7 @@ package manager
import (
"context"
"errors"
"fmt"
"math/rand/v2"
"net"
@@ -74,6 +75,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
}
@@ -83,6 +85,7 @@ type CapabilityProvider interface {
ClusterRequireSubdomain(ctx context.Context, clusterAddr string) *bool
ClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool
ClusterSupportsPrivate(ctx context.Context, clusterAddr string) *bool
ClusterAllProxiesPrivate(ctx context.Context, clusterAddr string) *bool
}
type Manager struct {
@@ -331,7 +334,17 @@ func (m *Manager) persistNewService(ctx context.Context, accountID string, svc *
return err
}
if err := m.validatePrivateClusterTargets(ctx, svc.Targets, svc.ProxyCluster); err != nil {
return err
}
return m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
if err := validateTargetAccessControl(svc); err != nil {
return err
}
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
@@ -365,6 +378,43 @@ func (m *Manager) clusterCustomPorts(ctx context.Context, svc *service.Service)
return m.capabilities.ClusterSupportsCustomPorts(ctx, svc.ProxyCluster)
}
// validatePrivateClusterTargets rejects cluster and direct upstream targets unless
// every active proxy in the service's cluster reports the private capability. The
// mapping reaches all proxies in the cluster, so one non-private proxy would serve
// these targets too. An unreported capability is treated as unsupported. Must be
// called outside a transaction, like clusterCustomPorts.
func (m *Manager) validatePrivateClusterTargets(ctx context.Context, targets []*service.Target, cluster string) error {
target := firstPrivateClusterTarget(targets)
if target == nil {
return nil
}
if private := m.capabilities.ClusterAllProxiesPrivate(ctx, cluster); private != nil && *private {
return nil
}
if target.TargetType == service.TargetTypeCluster {
return status.Errorf(status.InvalidArgument,
"target_type %q requires a proxy cluster with private mode enabled, cluster %s does not support it",
service.TargetTypeCluster, cluster)
}
return status.Errorf(status.InvalidArgument,
"direct_upstream requires a proxy cluster with private mode enabled, cluster %s does not support it", cluster)
}
// firstPrivateClusterTarget returns the first target that only a private cluster may serve.
func firstPrivateClusterTarget(targets []*service.Target) *service.Target {
for _, target := range targets {
if target == nil {
continue
}
if target.TargetType == service.TargetTypeCluster || target.Options.DirectUpstream {
return target
}
}
return nil
}
// ensureL4Port auto-assigns a listen port when needed and validates cluster support.
// customPorts must be pre-computed via clusterCustomPorts before entering a transaction.
func (m *Manager) ensureL4Port(ctx context.Context, tx store.Store, svc *service.Service, customPorts *bool, serviceUpdate bool) error {
@@ -460,7 +510,14 @@ func (m *Manager) persistNewEphemeralService(ctx context.Context, accountID, pee
return err
}
if err := m.validatePrivateClusterTargets(ctx, svc.Targets, svc.ProxyCluster); err != nil {
return err
}
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
}
@@ -577,6 +634,10 @@ func (m *Manager) persistServiceUpdate(ctx context.Context, accountID string, se
return nil, err
}
if err := m.validatePrivateClusterTargets(ctx, service.Targets, effectiveCluster); err != nil {
return nil, err
}
// Validate subdomain requirement *before* the transaction: the underlying
// capability lookup talks to the main DB pool, and SQLite's single-connection
// pool would self-deadlock if this ran while the tx already held the only
@@ -622,6 +683,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
@@ -647,6 +711,12 @@ func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.St
}
m.preserveExistingAuthSecrets(service, existingService)
if err := preserveTargetAccessActions(service, existingService); err != nil {
return err
}
if err := validateTargetAccessControl(service); err != nil {
return err
}
if err := validateHeaderAuthValues(service.Auth.HeaderAuths); err != nil {
return err
}
@@ -677,6 +747,75 @@ 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)
}
func validateTargetAccessControl(svc *service.Service) error {
if !svc.HasTargetAccessControl() {
return nil
}
// Legacy requests may acquire an existing action during the update merge.
if err := svc.Validate(); err != nil {
return status.Errorf(status.InvalidArgument, "%s", err)
}
return nil
}
func preserveTargetAccessActions(updated, existing *service.Service) error {
if !existing.HasTargetAccessControl() {
return nil
}
if slices.ContainsFunc(updated.Targets, func(target *service.Target) bool {
return target == nil
}) {
return status.Errorf(status.InvalidArgument, "target must not be nil")
}
if len(updated.Targets) > 0 && !slices.ContainsFunc(updated.Targets, func(target *service.Target) bool {
return !target.AccessActionProvided
}) {
return nil
}
byPath := make(map[string]*service.Target, len(existing.Targets))
for _, target := range existing.Targets {
location := effectiveTargetPath(target)
if _, duplicate := byPath[location]; duplicate {
return status.Errorf(status.InvalidArgument,
"cannot preserve target access control for duplicate location %q", location)
}
byPath[location] = target
}
actionsProvided := false
for _, target := range updated.Targets {
actionsProvided = actionsProvided || target.AccessActionProvided
location := effectiveTargetPath(target)
if previous := byPath[location]; previous != nil && !target.AccessActionProvided {
target.AccessAction = previous.AccessAction
}
delete(byPath, location)
}
if actionsProvided {
return nil
}
for location, target := range byPath {
if target.AccessAction != "" && target.AccessAction != service.TargetAccessActionInherit {
return status.Errorf(status.InvalidArgument,
"include access_action when removing or changing controlled target location %q", location)
}
}
return nil
}
func effectiveTargetPath(target *service.Target) string {
if target.Path == nil || *target.Path == "" {
return "/"
}
return *target.Path
}
// 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.
@@ -1012,15 +1151,16 @@ func (m *Manager) ReloadAllServicesForAccount(ctx context.Context, accountID str
oidcCfg := m.proxyController.GetOIDCValidationConfig()
var errs []error
for _, s := range services {
err = m.replaceHostByLookup(ctx, accountID, s)
if err != nil {
return fmt.Errorf("failed to replace host by lookup for service %s: %w", s.ID, err)
if err := m.replaceHostByLookup(ctx, accountID, s); err != nil {
errs = append(errs, fmt.Errorf("replace host by lookup for service %s: %w", s.ID, err))
continue
}
m.proxyController.SendServiceUpdateToCluster(ctx, accountID, s.ToProtoMapping(service.Update, "", oidcCfg), s.ProxyCluster)
}
return nil
return errors.Join(errs...)
}
func (m *Manager) GetGlobalServices(ctx context.Context) ([]*service.Service, error) {
@@ -27,6 +27,7 @@ import (
"github.com/netbirdio/netbird/management/server/permissions/operations"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/shared/management/proto"
"github.com/netbirdio/netbird/shared/management/status"
)
@@ -433,8 +434,8 @@ func TestDeletePeerService_SourcePeerValidation(t *testing.T) {
newProxyServer := func(t *testing.T) *nbgrpc.ProxyServiceServer {
t.Helper()
tokenStore := nbgrpc.NewOneTimeTokenStore(context.Background(), testCacheStore(t))
pkceStore := nbgrpc.NewPKCEVerifierStore(context.Background(), testCacheStore(t))
srv := nbgrpc.NewProxyServiceServer(nil, tokenStore, pkceStore, nbgrpc.ProxyOIDCConfig{}, nil, nil, nil, nil, nil)
singleUseStore := nbgrpc.NewSingleUseStore(context.Background(), testCacheStore(t))
srv := nbgrpc.NewProxyServiceServer(nil, tokenStore, singleUseStore, nbgrpc.ProxyOIDCConfig{}, nil, nil, nil, nil, nil)
return srv
}
@@ -655,6 +656,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"
@@ -722,8 +727,8 @@ func setupIntegrationTest(t *testing.T) (*Manager, store.Store) {
}
tokenStore := nbgrpc.NewOneTimeTokenStore(ctx, testCacheStore(t))
pkceStore := nbgrpc.NewPKCEVerifierStore(ctx, testCacheStore(t))
proxySrv := nbgrpc.NewProxyServiceServer(nil, tokenStore, pkceStore, nbgrpc.ProxyOIDCConfig{}, nil, nil, nil, nil, nil)
singleUseStore := nbgrpc.NewSingleUseStore(ctx, testCacheStore(t))
proxySrv := nbgrpc.NewProxyServiceServer(nil, tokenStore, singleUseStore, nbgrpc.ProxyOIDCConfig{}, nil, nil, nil, nil, nil)
proxyController, err := proxymanager.NewGRPCController(proxySrv, noop.NewMeterProvider().Meter(""))
require.NoError(t, err)
@@ -1146,8 +1151,8 @@ func TestDeleteService_DeletesTargets(t *testing.T) {
mockAcct := account.NewMockManager(ctrl)
tokenStore := nbgrpc.NewOneTimeTokenStore(ctx, testCacheStore(t))
pkceStore := nbgrpc.NewPKCEVerifierStore(ctx, testCacheStore(t))
proxySrv := nbgrpc.NewProxyServiceServer(nil, tokenStore, pkceStore, nbgrpc.ProxyOIDCConfig{}, nil, nil, nil, nil, nil)
singleUseStore := nbgrpc.NewSingleUseStore(ctx, testCacheStore(t))
proxySrv := nbgrpc.NewProxyServiceServer(nil, tokenStore, singleUseStore, nbgrpc.ProxyOIDCConfig{}, nil, nil, nil, nil, nil)
proxyController, err := proxymanager.NewGRPCController(proxySrv, noop.NewMeterProvider().Meter(""))
require.NoError(t, err)
@@ -1477,3 +1482,42 @@ func TestReplaceHostByLookup_SkipsClusterTarget(t *testing.T) {
require.NoError(t, mgr.replaceHostByLookup(ctx, accountID, svc), "cluster target must not trigger peer/resource lookup")
assert.Equal(t, "127.0.0.1", svc.Targets[0].Host, "operator-supplied host must be preserved for cluster target")
}
func TestReloadAllServicesForAccount_ContinuesAfterFailedService(t *testing.T) {
ctx := context.Background()
ctrl := gomock.NewController(t)
mockStore := store.NewMockStore(ctrl)
proxyController := proxy.NewMockController(ctrl)
accountID := "test-account"
broken := &rpservice.Service{
ID: "svc-broken",
AccountID: accountID,
ProxyCluster: "eu.proxy.netbird.io",
Targets: []*rpservice.Target{{TargetId: "x", TargetType: "unknown"}},
}
healthy := &rpservice.Service{
ID: "svc-healthy",
AccountID: accountID,
ProxyCluster: "eu.proxy.netbird.io",
Targets: []*rpservice.Target{{TargetId: "peer-1", TargetType: rpservice.TargetTypePeer, Host: "100.64.0.1", Protocol: "http", Port: 8080, Enabled: true}},
}
mockStore.EXPECT().GetAccountServices(ctx, store.LockingStrengthNone, accountID).Return([]*rpservice.Service{broken, healthy}, nil)
mockStore.EXPECT().GetPeerByID(ctx, store.LockingStrengthNone, accountID, "peer-1").
Return(&nbpeer.Peer{ID: "peer-1", IP: netip.MustParseAddr("100.70.0.9")}, nil)
proxyController.EXPECT().GetOIDCValidationConfig().Return(proxy.OIDCValidationConfig{})
proxyController.EXPECT().SendServiceUpdateToCluster(ctx, accountID, gomock.Any(), "eu.proxy.netbird.io").
Do(func(_ context.Context, _ string, mapping *proto.ProxyMapping, _ string) {
assert.Equal(t, "svc-healthy", mapping.GetId())
require.Len(t, mapping.GetPath(), 1)
assert.Contains(t, mapping.GetPath()[0].GetTarget(), "100.70.0.9", "mapping must carry the peer's current IP")
})
mgr := &Manager{store: mockStore, proxyController: proxyController}
err := mgr.ReloadAllServicesForAccount(ctx, accountID)
require.Error(t, err)
assert.Contains(t, err.Error(), "svc-broken")
assert.NotContains(t, err.Error(), "svc-healthy")
}
@@ -0,0 +1,218 @@
package manager
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/metric/noop"
"go.uber.org/mock/gomock"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
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/store"
"github.com/netbirdio/netbird/shared/management/status"
)
// setupPrivateClusterTest wires the real proxy manager as the capability
// provider and connects one proxy to testCluster reporting the given private
// capability. A nil private connects no proxy, so the capability is unreported.
func setupPrivateClusterTest(t *testing.T, private *bool) (*Manager, store.Store) {
t.Helper()
mgr, testStore := setupIntegrationTest(t)
proxyMgr, err := proxymanager.NewManager(testStore, noop.NewMeterProvider().Meter(""))
require.NoError(t, err)
mgr.capabilities = proxyMgr
if private != nil {
connectTestProxy(t, proxyMgr, "proxy-1", &proxy.Capabilities{Private: private})
}
return mgr, testStore
}
func connectTestProxy(t *testing.T, proxyMgr *proxymanager.Manager, proxyID string, caps *proxy.Capabilities) {
t.Helper()
_, err := proxyMgr.Connect(context.Background(), proxyID, "session-"+proxyID, testCluster, "127.0.0.1", "", nil, caps)
require.NoError(t, err)
}
func clusterTarget() *rpservice.Target {
return &rpservice.Target{
TargetId: testCluster,
TargetType: rpservice.TargetTypeCluster,
Host: "backend.lan",
Port: 8080,
Protocol: "http",
Enabled: true,
Options: rpservice.TargetOptions{DirectUpstream: true},
}
}
func directUpstreamPeerTarget() *rpservice.Target {
return &rpservice.Target{
TargetId: testPeerID,
TargetType: rpservice.TargetTypePeer,
Host: "backend.lan",
Port: 8080,
Protocol: "http",
Enabled: true,
Options: rpservice.TargetOptions{DirectUpstream: true},
}
}
func TestCreateService_PrivateClusterTargets(t *testing.T) {
tests := []struct {
name string
private *bool
target *rpservice.Target
wantErr string
}{
{name: "cluster target on private cluster", private: boolPtr(true), target: clusterTarget()},
{name: "direct upstream on private cluster", private: boolPtr(true), target: directUpstreamPeerTarget()},
{name: "cluster target on non-private cluster", private: boolPtr(false), target: clusterTarget(), wantErr: `target_type "cluster" requires a proxy cluster with private mode enabled`},
{name: "direct upstream on non-private cluster", private: boolPtr(false), target: directUpstreamPeerTarget(), wantErr: "direct_upstream requires a proxy cluster with private mode enabled"},
{name: "cluster target with unreported capability", private: nil, target: clusterTarget(), wantErr: `target_type "cluster" requires a proxy cluster with private mode enabled`},
{name: "direct upstream with unreported capability", private: nil, target: directUpstreamPeerTarget(), wantErr: "direct_upstream requires a proxy cluster with private mode enabled"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
mgr, testStore := setupPrivateClusterTest(t, tc.private)
svc := newTestService("app.test.netbird.io")
svc.Targets = []*rpservice.Target{tc.target}
_, err := mgr.CreateService(ctx, testAccountID, testUserID, svc)
services, listErr := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
require.NoError(t, listErr)
if tc.wantErr == "" {
require.NoError(t, err)
assert.Len(t, services, 1, "the service should be persisted")
return
}
require.Error(t, err)
assert.Contains(t, err.Error(), tc.wantErr)
sErr, ok := status.FromError(err)
require.True(t, ok, "the caller must receive a typed error")
assert.Equal(t, status.InvalidArgument, sErr.Type(), "the rejection should be an invalid argument")
assert.Empty(t, services, "a rejected service must not be persisted")
})
}
}
// A cluster where only some proxies run in private mode must not accept these
// targets: the mapping is delivered to every proxy in the cluster, so the
// non-private ones would serve the target from their host network as well.
func TestCreateService_MixedClusterRejectsPrivateTargets(t *testing.T) {
tests := []struct {
name string
secondCaps *proxy.Capabilities
}{
{name: "second proxy reports not private", secondCaps: &proxy.Capabilities{Private: boolPtr(false)}},
{name: "second proxy predates capability reporting", secondCaps: nil},
}
for _, tc := range tests {
for _, target := range []*rpservice.Target{clusterTarget(), directUpstreamPeerTarget()} {
t.Run(tc.name+"/"+string(target.TargetType), func(t *testing.T) {
ctx := context.Background()
mgr, testStore := setupPrivateClusterTest(t, boolPtr(true))
connectTestProxy(t, mgr.capabilities.(*proxymanager.Manager), "proxy-2", tc.secondCaps)
svc := newTestService("app.test.netbird.io")
svc.Targets = []*rpservice.Target{target}
_, err := mgr.CreateService(ctx, testAccountID, testUserID, svc)
require.Error(t, err, "a cluster with a non-private proxy must not accept the target")
assert.Contains(t, err.Error(), "requires a proxy cluster with private mode enabled")
services, err := testStore.GetAccountServices(ctx, store.LockingStrengthNone, testAccountID)
require.NoError(t, err)
assert.Empty(t, services, "a rejected service must not be persisted")
})
}
}
}
func TestCreateService_RegularTargetIgnoresPrivateCapability(t *testing.T) {
ctx := context.Background()
mgr, _ := setupPrivateClusterTest(t, boolPtr(false))
_, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.test.netbird.io"))
require.NoError(t, err, "a peer target without direct upstream must not need a private cluster")
}
func TestUpdateService_PrivateClusterTargets(t *testing.T) {
tests := []struct {
name string
target *rpservice.Target
wantErr string
}{
{name: "switch to cluster target", target: clusterTarget(), wantErr: `target_type "cluster" requires a proxy cluster with private mode enabled`},
{name: "enable direct upstream", target: directUpstreamPeerTarget(), wantErr: "direct_upstream requires a proxy cluster with private mode enabled"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
mgr, testStore := setupPrivateClusterTest(t, boolPtr(false))
created, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.test.netbird.io"))
require.NoError(t, err)
updated := newTestService("app.test.netbird.io")
updated.ID = created.ID
updated.AccountID = testAccountID
updated.Targets = []*rpservice.Target{tc.target}
_, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.Error(t, err)
assert.Contains(t, err.Error(), tc.wantErr)
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, created.ID)
require.NoError(t, err)
require.Len(t, stored.Targets, 1)
assert.Equal(t, rpservice.TargetTypePeer, stored.Targets[0].TargetType, "the stored target must be unchanged")
assert.False(t, stored.Targets[0].Options.DirectUpstream, "the stored target must keep direct upstream disabled")
})
}
}
func TestUpdateService_PrivateClusterAllowsClusterTarget(t *testing.T) {
ctx := context.Background()
mgr, testStore := setupPrivateClusterTest(t, boolPtr(true))
created, err := mgr.CreateService(ctx, testAccountID, testUserID, newTestService("app.test.netbird.io"))
require.NoError(t, err)
updated := newTestService("app.test.netbird.io")
updated.ID = created.ID
updated.AccountID = testAccountID
updated.Targets = []*rpservice.Target{clusterTarget()}
_, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, created.ID)
require.NoError(t, err)
require.Len(t, stored.Targets, 1)
assert.Equal(t, rpservice.TargetTypeCluster, stored.Targets[0].TargetType, "the cluster target should be stored")
}
func TestValidatePrivateClusterTargets_NoLookupWithoutPrivateTargets(t *testing.T) {
ctrl := gomock.NewController(t)
// No ClusterAllProxiesPrivate expectation: a lookup would fail the test.
mgr := &Manager{capabilities: proxy.NewMockManager(ctrl)}
targets := []*rpservice.Target{{TargetId: testPeerID, TargetType: rpservice.TargetTypePeer}}
require.NoError(t, mgr.validatePrivateClusterTargets(context.Background(), targets, testCluster))
}
@@ -0,0 +1,196 @@
package manager
import (
"context"
"testing"
"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/store"
)
func TestCreateServiceTargetAccessWithoutActiveProxies(t *testing.T) {
for _, action := range []rpservice.TargetAccessAction{
rpservice.TargetAccessActionBlock,
rpservice.TargetAccessActionBypass,
} {
t.Run(string(action), func(t *testing.T) {
ctx := context.Background()
mgr, testStore, _ := setupL4Test(t, nil)
svc := &rpservice.Service{
Name: "target access", Domain: "target.test.netbird.io", Mode: rpservice.ModeHTTP,
Enabled: true,
Targets: []*rpservice.Target{{
AccountID: testAccountID, TargetId: testPeerID,
TargetType: rpservice.TargetTypePeer, Protocol: "http", Port: 8080,
Enabled: true, AccessAction: action,
}},
}
created, err := mgr.CreateService(ctx, testAccountID, testUserID, svc)
require.NoError(t, err)
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, created.ID)
require.NoError(t, err)
require.Len(t, stored.Targets, 1)
assert.Equal(t, action, stored.Targets[0].AccessAction,
"accepted target actions must survive persistence")
})
}
}
func TestUpdateServicePreservesOmittedTargetAccessAction(t *testing.T) {
ctx := context.Background()
mgr, testStore, _ := setupL4Test(t, nil)
existing := seedService(t, testStore, "protected", "http", "target.test.netbird.io", testCluster, 0)
existing.Targets[0].AccessAction = rpservice.TargetAccessActionBlock
require.NoError(t, testStore.UpdateService(ctx, existing))
updated := existing.Copy()
updated.Name = "renamed by an older client"
updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit
updated.Targets[0].AccessActionProvided = false
rootPath := "/"
updated.Targets[0].Path = &rootPath
_, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
assert.Equal(t, updated.Name, stored.Name, "legacy updates must still apply unrelated edits")
assert.Equal(t, rpservice.TargetAccessActionBlock, stored.Targets[0].AccessAction,
"an omitted action must preserve the stored block across equivalent root locations")
emptyPath := ""
updated.Targets[0].Path = &emptyPath
updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit
_, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
stored, err = testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
assert.Equal(t, rpservice.TargetAccessActionBlock, stored.Targets[0].AccessAction,
"an older client's empty root location must preserve the stored block")
updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit
updated.Targets[0].AccessActionProvided = true
_, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
stored, err = testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
assert.Equal(t, rpservice.TargetAccessActionInherit, stored.Targets[0].AccessAction,
"explicit inherit must clear the stored target block")
}
func TestUpdateServiceActivatesPreservedTargetAccess(t *testing.T) {
ctx := context.Background()
mgr, testStore, _ := setupL4Test(t, nil)
existing := seedService(t, testStore, "protected", "http", "target.test.netbird.io", testCluster, 0)
existing.Enabled = false
existing.Targets[0].AccessAction = rpservice.TargetAccessActionBlock
require.NoError(t, testStore.UpdateService(ctx, existing))
updated := existing.Copy()
updated.Enabled = true
updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit
updated.Targets[0].AccessActionProvided = false
_, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
assert.True(t, stored.Enabled, "activating target access must not require an active proxy")
assert.Equal(t, rpservice.TargetAccessActionBlock, stored.Targets[0].AccessAction,
"activation must retain an omitted stored access action")
}
func TestUpdateServiceValidatesPreservedTargetAccess(t *testing.T) {
ctx := context.Background()
mgr, testStore, _ := setupL4Test(t, nil)
existing := seedService(t, testStore, "public", "http", "target.test.netbird.io", testCluster, 0)
existing.Targets[0].AccessAction = rpservice.TargetAccessActionBypass
require.NoError(t, testStore.UpdateService(ctx, existing))
updated := existing.Copy()
updated.Private = true
updated.AccessGroups = []string{testGroupID}
updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit
updated.Targets[0].AccessActionProvided = false
_, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.ErrorContains(t, err, "bypass access_action is not supported for private services")
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
assert.False(t, stored.Private, "an invalid preserved bypass must prevent the private-service update")
assert.Equal(t, rpservice.TargetAccessActionBypass, stored.Targets[0].AccessAction,
"a rejected update must retain the existing target action")
}
func TestUpdateServiceLegacyClientCannotRemoveControlledPath(t *testing.T) {
for _, operation := range []string{"rename", "delete"} {
t.Run(operation, func(t *testing.T) {
ctx := context.Background()
mgr, testStore, _ := setupL4Test(t, nil)
existing := seedService(t, testStore, "protected", "http", "target.test.netbird.io", testCluster, 0)
blockedPath := "/blocked"
existing.Targets = append(existing.Targets, &rpservice.Target{
AccountID: testAccountID, TargetId: testPeerID, TargetType: rpservice.TargetTypePeer,
Protocol: "http", Port: 8080, Enabled: true, Path: &blockedPath,
AccessAction: rpservice.TargetAccessActionBlock,
})
require.NoError(t, testStore.UpdateService(ctx, existing))
updated := existing.Copy()
updated.Targets[1].AccessAction = rpservice.TargetAccessActionInherit
if operation == "rename" {
renamedPath := "/renamed"
updated.Targets[1].Path = &renamedPath
} else {
updated.Targets = updated.Targets[:1]
}
_, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.Error(t, err)
stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
require.Len(t, stored.Targets, 2, "a legacy structural edit must retain the controlled target")
var blocked *rpservice.Target
for _, target := range stored.Targets {
if effectiveTargetPath(target) == blockedPath {
blocked = target
}
}
require.NotNil(t, blocked, "the original controlled location must still exist")
assert.Equal(t, rpservice.TargetAccessActionBlock, blocked.AccessAction,
"a rejected legacy edit must not erase the block action")
updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit
updated.Targets[0].AccessActionProvided = true
_, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated)
require.NoError(t, err)
stored, err = testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID)
require.NoError(t, err)
assert.False(t, stored.HasTargetAccessControl(),
"a feature-aware client must be able to intentionally remove or rename the controlled path")
})
}
}
func TestPreserveTargetAccessActionsExplicitUpdate(t *testing.T) {
existing := &rpservice.Service{Targets: []*rpservice.Target{
{Enabled: true, AccessAction: rpservice.TargetAccessActionBlock},
{Enabled: false, AccessAction: rpservice.TargetAccessActionInherit},
}}
updated := &rpservice.Service{Targets: []*rpservice.Target{
{Enabled: true, AccessAction: rpservice.TargetAccessActionBypass, AccessActionProvided: true},
{Enabled: false, AccessAction: rpservice.TargetAccessActionInherit, AccessActionProvided: true},
}}
require.NoError(t, preserveTargetAccessActions(updated, existing))
assert.Equal(t, rpservice.TargetAccessActionBypass, updated.Targets[0].AccessAction,
"explicit actions must not need an ambiguous location-based legacy merge")
updated.Targets[0].AccessActionProvided = false
assert.Error(t, preserveTargetAccessActions(updated, existing),
"an omitted action must be rejected when its stored location is ambiguous")
existing.Targets = existing.Targets[:1]
updated.Targets = nil
assert.Error(t, preserveTargetAccessActions(updated, existing),
"an empty target list must not count as an explicit removal of the stored block")
}
@@ -116,18 +116,20 @@ type MiddlewareConfig struct {
}
type Target struct {
ID uint `gorm:"primaryKey" json:"-"`
AccountID string `gorm:"index:idx_target_account;not null" json:"-"`
ServiceID string `gorm:"index:idx_service_targets;not null" json:"-"`
Path *string `json:"path,omitempty"`
Host string `json:"host"`
Port uint16 `gorm:"index:idx_target_port" json:"port"`
Protocol string `gorm:"index:idx_target_protocol" json:"protocol"`
TargetId string `gorm:"index:idx_target_id" json:"target_id"`
TargetType TargetType `gorm:"index:idx_target_type" json:"target_type"`
Enabled bool `gorm:"index:idx_target_enabled" json:"enabled"`
Options TargetOptions `gorm:"embedded" json:"options"`
ProxyProtocol bool `json:"proxy_protocol"`
ID uint `gorm:"primaryKey" json:"-"`
AccountID string `gorm:"index:idx_target_account;not null" json:"-"`
ServiceID string `gorm:"index:idx_service_targets;not null" json:"-"`
Path *string `json:"path,omitempty"`
Host string `json:"host"`
Port uint16 `gorm:"index:idx_target_port" json:"port"`
Protocol string `gorm:"index:idx_target_protocol" json:"protocol"`
TargetId string `gorm:"index:idx_target_id" json:"target_id"`
TargetType TargetType `gorm:"index:idx_target_type" json:"target_type"`
Enabled bool `gorm:"index:idx_target_enabled" json:"enabled"`
AccessAction TargetAccessAction `gorm:"type:varchar(16);not null;default:'inherit'" json:"access_action"`
AccessActionProvided bool `gorm:"-" json:"-"`
Options TargetOptions `gorm:"embedded" json:"options"`
ProxyProtocol bool `json:"proxy_protocol"`
}
type PasswordAuthConfig struct {
@@ -314,14 +316,16 @@ func (s *Service) ToAPIResponse() *api.Service {
// Convert internal targets to API targets
apiTargets := make([]api.ServiceTarget, 0, len(s.Targets))
for _, target := range s.Targets {
accessAction := api.ServiceTargetAccessAction(target.effectiveAccessAction())
st := api.ServiceTarget{
Path: target.Path,
Host: &target.Host,
Port: int(target.Port),
Protocol: api.ServiceTargetProtocol(target.Protocol),
TargetId: target.TargetId,
TargetType: api.ServiceTargetTargetType(target.TargetType),
Enabled: target.Enabled && !s.Terminated,
Path: target.Path,
Host: &target.Host,
Port: int(target.Port),
Protocol: api.ServiceTargetProtocol(target.Protocol),
TargetId: target.TargetId,
TargetType: api.ServiceTargetTargetType(target.TargetType),
Enabled: target.Enabled && !s.Terminated,
AccessAction: &accessAction,
}
opts := targetOptionsToAPI(target.Options)
if opts == nil {
@@ -468,8 +472,9 @@ func (s *Service) buildPathMappings() []*proto.PathMapping {
}
pm := &proto.PathMapping{
Path: path,
Target: targetURL.String(),
Path: path,
Target: targetURL.String(),
AccessAction: targetAccessActionToProto(target.effectiveAccessAction()),
}
pm.Options = targetOptionsToProto(target.Options)
pathMappings = append(pathMappings, pm)
@@ -732,13 +737,18 @@ func targetsFromAPI(accountID string, apiTargetsPtr *[]api.ServiceTarget) ([]*Ta
targets := make([]*Target, 0, len(apiTargets))
for i, apiTarget := range apiTargets {
target := &Target{
AccountID: accountID,
Path: apiTarget.Path,
Port: uint16(apiTarget.Port), //nolint:gosec // validated by API layer
Protocol: string(apiTarget.Protocol),
TargetId: apiTarget.TargetId,
TargetType: TargetType(apiTarget.TargetType),
Enabled: apiTarget.Enabled,
AccountID: accountID,
Path: apiTarget.Path,
Port: uint16(apiTarget.Port), //nolint:gosec // validated by API layer
Protocol: string(apiTarget.Protocol),
TargetId: apiTarget.TargetId,
TargetType: TargetType(apiTarget.TargetType),
Enabled: apiTarget.Enabled,
AccessAction: TargetAccessActionInherit,
}
if apiTarget.AccessAction != nil {
target.AccessAction = TargetAccessAction(*apiTarget.AccessAction)
target.AccessActionProvided = true
}
if apiTarget.Host != nil {
target.Host = *apiTarget.Host
@@ -970,41 +980,70 @@ func (s *Service) validateTLSMode() error {
}
func (s *Service) validateHTTPTargets() error {
strictPaths := false
for i, target := range s.Targets {
switch target.TargetType {
case TargetTypePeer, TargetTypeHost, TargetTypeDomain:
// Host is normally overwritten by replaceHostByLookup with the
// resolved peer IP / resource address; operator-supplied values
// are honored only when DirectUpstream is set. Validate the
// override here so misconfigured hosts fail fast at API time.
if err := validateDirectUpstreamHost(i, target); err != nil {
return err
}
case TargetTypeSubnet:
if err := validateSubnetTarget(i, target); err != nil {
return err
}
case TargetTypeCluster:
if err := validateClusterTarget(i, target); err != nil {
return err
}
default:
return fmt.Errorf("target %d has invalid target_type %q", i, target.TargetType)
}
if target.TargetId == "" {
return fmt.Errorf("target %d has empty target_id", i)
}
if target.ProxyProtocol {
return fmt.Errorf("target %d: proxy_protocol is not supported for HTTP services", i)
}
if err := validateTargetOptions(i, &target.Options); err != nil {
if err := s.validateHTTPTarget(i, target); err != nil {
return err
}
if target.Enabled && target.effectiveAccessAction() != TargetAccessActionInherit {
strictPaths = true
}
}
if !strictPaths {
return nil
}
paths := make(map[string]int, len(s.Targets))
for i, target := range s.Targets {
if !target.Enabled {
continue
}
normalizedPath := normalizedTargetPath(target.Path)
if previous, ok := paths[normalizedPath]; ok {
return fmt.Errorf("targets %d and %d have duplicate path %q", previous, i, normalizedPath)
}
paths[normalizedPath] = i
}
return nil
}
func (s *Service) validateHTTPTarget(idx int, target *Target) error {
if target == nil {
return fmt.Errorf("target %d is nil", idx)
}
if err := validateTargetAccessAction(idx, target, s.Private); err != nil {
return err
}
switch target.TargetType {
case TargetTypePeer, TargetTypeHost, TargetTypeDomain:
// Host is normally overwritten by replaceHostByLookup with the
// resolved peer IP / resource address; operator-supplied values
// are honored only when DirectUpstream is set. Validate the
// override here so misconfigured hosts fail fast at API time.
if err := validateDirectUpstreamHost(idx, target); err != nil {
return err
}
case TargetTypeSubnet:
if err := validateSubnetTarget(idx, target); err != nil {
return err
}
case TargetTypeCluster:
if err := validateClusterTarget(idx, target); err != nil {
return err
}
default:
return fmt.Errorf("target %d has invalid target_type %q", idx, target.TargetType)
}
if target.TargetId == "" {
return fmt.Errorf("target %d has empty target_id", idx)
}
if target.ProxyProtocol {
return fmt.Errorf("target %d: proxy_protocol is not supported for HTTP services", idx)
}
return validateTargetOptions(idx, &target.Options)
}
func validateSubnetTarget(idx int, target *Target) error {
host := strings.TrimSpace(target.Host)
if host == "" {
@@ -1080,10 +1119,20 @@ func validateDirectUpstreamHost(idx int, target *Target) error {
}
func (s *Service) validateL4Target(target *Target) error {
if target == nil {
return errors.New("target 0 is nil")
}
// L4 services have a single target; per-target disable is meaningless
// (use the service-level Enabled flag instead). Force it on so that
// buildPathMappings always includes the target in the proto.
target.Enabled = true
action, err := validatedTargetAccessAction(0, target)
if err != nil {
return err
}
if action != TargetAccessActionInherit {
return errors.New("access_action is only supported for HTTP services")
}
if target.TargetId == "" {
return errors.New("target_id is required for L4 services")
@@ -62,9 +62,11 @@ func TestValidate_InvalidTargetType(t *testing.T) {
func TestValidate_ResourceTarget(t *testing.T) {
rp := validProxy()
resourcePath := "/resource"
rp.Targets = append(rp.Targets, &Target{
TargetId: "resource-1",
TargetType: TargetTypeHost,
Path: &resourcePath,
Host: "example.org",
Port: 443,
Protocol: "https",
@@ -73,6 +75,232 @@ func TestValidate_ResourceTarget(t *testing.T) {
require.NoError(t, rp.Validate())
}
func TestValidateTargetAccessAction(t *testing.T) {
t.Run("valid actions", func(t *testing.T) {
for _, action := range []TargetAccessAction{
TargetAccessActionInherit,
TargetAccessActionBypass,
TargetAccessActionBlock,
} {
rp := validProxy()
rp.Targets[0].AccessAction = action
require.NoError(t, rp.Validate(), "action %q should be valid", action)
}
})
t.Run("unknown action", func(t *testing.T) {
rp := validProxy()
rp.Targets[0].AccessAction = "allow"
assert.ErrorContains(t, rp.Validate(), `unknown access_action "allow"`)
})
t.Run("explicit empty action", func(t *testing.T) {
rp := validProxy()
rp.Targets[0].AccessActionProvided = true
assert.ErrorContains(t, rp.Validate(), `unknown access_action ""`)
})
t.Run("private bypass", func(t *testing.T) {
rp := validProxy()
rp.Private = true
rp.AccessGroups = []string{"group-1"}
rp.Targets[0].AccessAction = TargetAccessActionBypass
assert.ErrorContains(t, rp.Validate(), "not supported for private services")
})
t.Run("Agent Network bypass", func(t *testing.T) {
rp := validProxy()
rp.Targets[0].AccessAction = TargetAccessActionBypass
rp.Targets[0].Options.AgentNetwork = true
assert.ErrorContains(t, rp.Validate(), "not supported for Agent Network targets")
})
t.Run("block remains available to private services", func(t *testing.T) {
rp := validProxy()
rp.Private = true
rp.AccessGroups = []string{"group-1"}
rp.Targets[0].AccessAction = TargetAccessActionBlock
require.NoError(t, rp.Validate())
})
t.Run("non HTTP service", func(t *testing.T) {
rp := validProxy()
rp.Mode = ModeTCP
rp.ListenPort = 8080
rp.Targets[0].Protocol = TargetProtoTCP
rp.Targets[0].AccessAction = TargetAccessActionBlock
assert.ErrorContains(t, rp.Validate(), "only supported for HTTP services")
})
t.Run("nil non HTTP target", func(t *testing.T) {
rp := validProxy()
rp.Mode = ModeTCP
rp.ListenPort = 8080
rp.Targets[0] = nil
assert.ErrorContains(t, rp.Validate(), "target 0 is nil")
})
}
func TestValidateTargetAccessActionPath(t *testing.T) {
for _, value := range []string{"api", "/api/../admin", "/api/./admin", "/api//admin", "/api%2Fadmin", `/api\admin`, "/api?x=1", "/api#fragment", "/api;admin", "/api/\x00"} {
t.Run(fmt.Sprintf("path_%q", value), func(t *testing.T) {
rp := validProxy()
rp.Targets[0].Path = &value
rp.Targets[0].AccessAction = TargetAccessActionBypass
assert.ErrorContains(t, rp.Validate(), "access_action path")
})
}
t.Run("nil path is canonical root", func(t *testing.T) {
rp := validProxy()
rp.Targets[0].AccessAction = TargetAccessActionBlock
require.NoError(t, rp.Validate())
})
t.Run("empty path is legacy root", func(t *testing.T) {
value := ""
rp := validProxy()
rp.Targets[0].Path = &value
rp.Targets[0].AccessAction = TargetAccessActionBlock
require.NoError(t, rp.Validate())
})
t.Run("trailing slash is a distinct valid prefix", func(t *testing.T) {
value := "/api/"
rp := validProxy()
rp.Targets[0].Path = &value
rp.Targets[0].AccessAction = TargetAccessActionBypass
require.NoError(t, rp.Validate())
})
}
func TestValidateDuplicateEnabledTargetPaths(t *testing.T) {
emptyPath := ""
rootPath := "/"
rootAliases := []struct {
name string
path *string
}{
{name: "nil", path: nil},
{name: "empty", path: &emptyPath},
{name: "slash", path: &rootPath},
}
for _, alias := range rootAliases {
t.Run("legacy all-inherit "+alias.name, func(t *testing.T) {
rp := validProxy()
rp.Targets = append(rp.Targets, &Target{
TargetId: "peer-2",
TargetType: TargetTypePeer,
Path: alias.path,
Host: "10.0.0.2",
Port: 443,
Protocol: TargetProtoHTTPS,
Enabled: true,
})
require.NoError(t, rp.Validate(), "legacy all-inherit path and protocol twins must remain valid")
})
t.Run("access-controlled "+alias.name, func(t *testing.T) {
rp := validProxy()
rp.Targets = append(rp.Targets, &Target{
TargetId: "peer-2",
TargetType: TargetTypePeer,
Path: alias.path,
Host: "10.0.0.2",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: true,
AccessAction: TargetAccessActionBlock,
})
assert.ErrorContains(t, rp.Validate(), `duplicate path "/"`)
})
}
t.Run("access action elsewhere enables strict paths", func(t *testing.T) {
publicPath := "/public"
rp := validProxy()
rp.Targets = append(rp.Targets,
&Target{
TargetId: "peer-2",
TargetType: TargetTypePeer,
Host: "10.0.0.2",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: true,
},
&Target{
TargetId: "peer-3",
TargetType: TargetTypePeer,
Path: &publicPath,
Host: "10.0.0.3",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: true,
AccessAction: TargetAccessActionBypass,
},
)
assert.ErrorContains(t, rp.Validate(), `duplicate path "/"`)
})
t.Run("disabled action preserves legacy validation", func(t *testing.T) {
publicPath := "/public"
rp := validProxy()
rp.Targets = append(rp.Targets,
&Target{
TargetId: "peer-2",
TargetType: TargetTypePeer,
Host: "10.0.0.2",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: true,
},
&Target{
TargetId: "peer-3",
TargetType: TargetTypePeer,
Path: &publicPath,
Host: "10.0.0.3",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: false,
AccessAction: TargetAccessActionBlock,
},
)
require.NoError(t, rp.Validate())
})
t.Run("disabled target does not collide", func(t *testing.T) {
rp := validProxy()
rp.Targets = append(rp.Targets, &Target{
TargetId: "peer-2",
TargetType: TargetTypePeer,
Host: "10.0.0.2",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: false,
})
require.NoError(t, rp.Validate())
})
t.Run("trailing slash remains distinct", func(t *testing.T) {
apiPath := "/api"
apiSlashPath := "/api/"
rp := validProxy()
rp.Targets[0].Path = &apiPath
rp.Targets = append(rp.Targets, &Target{
TargetId: "peer-2",
TargetType: TargetTypePeer,
Path: &apiSlashPath,
Host: "10.0.0.2",
Port: 80,
Protocol: TargetProtoHTTP,
Enabled: true,
AccessAction: TargetAccessActionBlock,
})
require.NoError(t, rp.Validate())
})
}
func TestValidate_MultipleTargetsOneInvalid(t *testing.T) {
rp := validProxy()
rp.Targets = append(rp.Targets, &Target{
@@ -308,6 +536,90 @@ func TestToProtoMapping_TargetOptions(t *testing.T) {
assert.Equal(t, int64(30), opts.RequestTimeout.Seconds)
}
func TestTargetAccessActionConversions(t *testing.T) {
t.Run("API presence", func(t *testing.T) {
bypass := api.ServiceTargetAccessActionBypass
empty := api.ServiceTargetAccessAction("")
apiTargets := []api.ServiceTarget{
{
TargetId: "peer-1",
TargetType: api.ServiceTargetTargetTypePeer,
Protocol: api.ServiceTargetProtocolHttp,
Port: 80,
Enabled: true,
},
{
TargetId: "peer-2",
TargetType: api.ServiceTargetTargetTypePeer,
Protocol: api.ServiceTargetProtocolHttp,
Port: 80,
Enabled: true,
AccessAction: &bypass,
},
{
TargetId: "peer-3",
TargetType: api.ServiceTargetTargetTypePeer,
Protocol: api.ServiceTargetProtocolHttp,
Port: 80,
Enabled: true,
AccessAction: &empty,
},
}
targets, err := targetsFromAPI("account-1", &apiTargets)
require.NoError(t, err)
require.Len(t, targets, 3)
assert.Equal(t, TargetAccessActionInherit, targets[0].AccessAction)
assert.False(t, targets[0].AccessActionProvided)
assert.Equal(t, TargetAccessActionBypass, targets[1].AccessAction)
assert.True(t, targets[1].AccessActionProvided)
assert.Empty(t, targets[2].AccessAction)
assert.True(t, targets[2].AccessActionProvided)
rp := &Service{Name: "test", Domain: "example.com", Targets: []*Target{targets[2]}}
assert.ErrorContains(t, rp.Validate(), `unknown access_action ""`)
})
t.Run("API response", func(t *testing.T) {
rp := validProxy()
rp.Targets[0].AccessAction = TargetAccessActionBlock
response := rp.ToAPIResponse()
require.Len(t, response.Targets, 1)
require.NotNil(t, response.Targets[0].AccessAction)
assert.Equal(t, api.ServiceTargetAccessActionBlock, *response.Targets[0].AccessAction)
})
t.Run("proto mapping", func(t *testing.T) {
root := "/"
bypassPath := "/bypass"
blockPath := "/block"
rp := &Service{
ID: "svc-1",
AccountID: "acc-1",
Domain: "example.com",
Targets: []*Target{
{Path: &root, Host: "10.0.0.1", Port: 80, Protocol: TargetProtoHTTP, Enabled: true},
{Path: &bypassPath, Host: "10.0.0.1", Port: 80, Protocol: TargetProtoHTTP, Enabled: true, AccessAction: TargetAccessActionBypass},
{Path: &blockPath, Host: "10.0.0.1", Port: 80, Protocol: TargetProtoHTTP, Enabled: true, AccessAction: TargetAccessActionBlock},
},
}
mapping := rp.ToProtoMapping(Create, "token", proxy.OIDCValidationConfig{})
require.Len(t, mapping.Path, 3)
assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_INHERIT, mapping.Path[0].GetAccessAction())
assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_BYPASS, mapping.Path[1].GetAccessAction())
assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK, mapping.Path[2].GetAccessAction())
})
}
func TestHasTargetAccessControl(t *testing.T) {
rp := validProxy()
assert.False(t, rp.HasTargetAccessControl())
rp.Targets[0].AccessAction = TargetAccessActionBypass
assert.True(t, rp.HasTargetAccessControl())
}
// TestToProtoMapping_AllowedGroupIds covers the list the proxy gates session
// cookies on: without it the proxy can only check a cookie's signature, which
// makes a token minted for a user outside the groups a bearer credential.
@@ -0,0 +1,115 @@
package service
import (
"fmt"
"strings"
"unicode"
"unicode/utf8"
"github.com/netbirdio/netbird/shared/management/proto"
)
// TargetAccessAction controls authentication for a selected HTTP target.
type TargetAccessAction string
const (
TargetAccessActionInherit TargetAccessAction = "inherit"
TargetAccessActionBypass TargetAccessAction = "bypass"
TargetAccessActionBlock TargetAccessAction = "block"
)
func (t *Target) effectiveAccessAction() TargetAccessAction {
if t.AccessAction == "" {
return TargetAccessActionInherit
}
return t.AccessAction
}
func targetAccessActionToProto(action TargetAccessAction) proto.TargetAccessAction {
switch action {
case TargetAccessActionInherit:
return proto.TargetAccessAction_TARGET_ACCESS_ACTION_INHERIT
case TargetAccessActionBypass:
return proto.TargetAccessAction_TARGET_ACCESS_ACTION_BYPASS
case TargetAccessActionBlock:
return proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK
default:
return proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK
}
}
// HasTargetAccessControl reports whether any target overrides service authentication.
func (s *Service) HasTargetAccessControl() bool {
for _, target := range s.Targets {
if target != nil && target.effectiveAccessAction() != TargetAccessActionInherit {
return true
}
}
return false
}
func validateTargetAccessAction(idx int, target *Target, private bool) error {
action, err := validatedTargetAccessAction(idx, target)
if err != nil {
return err
}
if action == TargetAccessActionBypass {
if private {
return fmt.Errorf("target %d: bypass access_action is not supported for private services", idx)
}
if target.Options.AgentNetwork {
return fmt.Errorf("target %d: bypass access_action is not supported for Agent Network targets", idx)
}
}
if action == TargetAccessActionInherit {
return nil
}
return validateAccessActionPath(idx, target.Path)
}
func validatedTargetAccessAction(idx int, target *Target) (TargetAccessAction, error) {
if target.AccessActionProvided && target.AccessAction == "" {
return "", fmt.Errorf("target %d: unknown access_action %q", idx, target.AccessAction)
}
action := target.effectiveAccessAction()
switch action {
case TargetAccessActionInherit, TargetAccessActionBypass, TargetAccessActionBlock:
return action, nil
default:
return "", fmt.Errorf("target %d: unknown access_action %q", idx, target.AccessAction)
}
}
func normalizedTargetPath(configured *string) string {
if configured == nil || *configured == "" {
return "/"
}
return *configured
}
func validateAccessActionPath(idx int, configured *string) error {
if configured == nil {
return nil
}
value := *configured
if value == "" {
return nil
}
if !strings.HasPrefix(value, "/") {
return fmt.Errorf("target %d: access_action path %q must start with /", idx, value)
}
if !utf8.ValidString(value) || strings.ContainsAny(value, "%\\?#;") || strings.IndexFunc(value, unicode.IsControl) >= 0 {
return fmt.Errorf("target %d: access_action path %q contains invalid characters", idx, value)
}
segments := strings.Split(strings.TrimPrefix(value, "/"), "/")
for i, segment := range segments {
if segment == "." || segment == ".." {
return fmt.Errorf("target %d: access_action path %q is not canonical", idx, value)
}
if segment == "" && value != "/" && i != len(segments)-1 {
return fmt.Errorf("target %d: access_action path %q is not canonical", idx, value)
}
}
return nil
}
@@ -3,15 +3,16 @@ package manager
import (
"context"
"fmt"
"slices"
"github.com/netbirdio/netbird/management/internals/modules/zones"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/affectedpeers"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/permissions/modules"
"github.com/netbirdio/netbird/management/server/permissions/operations"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/shared/management/status"
)
@@ -69,6 +70,9 @@ func (m *managerImpl) CreateZone(ctx context.Context, accountID, userID string,
}
zone = zones.NewZone(accountID, zone.Name, zone.Domain, zone.Enabled, zone.EnableSearchDomain, zone.DistributionGroups)
var snap *affectedpeers.Snapshot
change := affectedpeers.Change{DistributionGroupIDs: zone.DistributionGroups}
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
existingZone, err := transaction.GetZoneByDomain(ctx, accountID, zone.Domain)
if err != nil {
@@ -88,7 +92,15 @@ func (m *managerImpl) CreateZone(ctx context.Context, accountID, userID string,
}
if err = transaction.CreateZone(ctx, zone); err != nil {
return fmt.Errorf("failed to create zone: %w", err)
return fmt.Errorf("create zone: %w", err)
}
if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil {
return fmt.Errorf("load affected peers: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
@@ -99,6 +111,8 @@ func (m *managerImpl) CreateZone(ctx context.Context, accountID, userID string,
m.accountManager.StoreEvent(ctx, userID, zone.ID, accountID, activity.DNSZoneCreated, zone.EventMeta())
m.accountManager.ExpandAndUpdateAffected(ctx, accountID, snap, change)
return zone, nil
}
@@ -111,21 +125,26 @@ func (m *managerImpl) UpdateZone(ctx context.Context, accountID, userID string,
return nil, status.NewPermissionDeniedError()
}
zone, err := m.store.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, updatedZone.ID)
if err != nil {
return nil, fmt.Errorf("failed to get zone: %w", err)
}
if zone.Domain != updatedZone.Domain {
return nil, status.Errorf(status.InvalidArgument, "zone domain cannot be updated")
}
zone.Name = updatedZone.Name
zone.Enabled = updatedZone.Enabled
zone.EnableSearchDomain = updatedZone.EnableSearchDomain
zone.DistributionGroups = updatedZone.DistributionGroups
var zone *zones.Zone
var snap *affectedpeers.Snapshot
var change affectedpeers.Change
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, updatedZone.ID)
if err != nil {
return fmt.Errorf("get zone: %w", err)
}
if zone.Domain != updatedZone.Domain {
return status.Errorf(status.InvalidArgument, "zone domain cannot be updated")
}
oldGroups := zone.DistributionGroups
zone.Name = updatedZone.Name
zone.Enabled = updatedZone.Enabled
zone.EnableSearchDomain = updatedZone.EnableSearchDomain
zone.DistributionGroups = updatedZone.DistributionGroups
for _, groupID := range zone.DistributionGroups {
_, err = transaction.GetGroupByID(ctx, store.LockingStrengthNone, accountID, groupID)
if err != nil {
@@ -134,7 +153,16 @@ func (m *managerImpl) UpdateZone(ctx context.Context, accountID, userID string,
}
if err = transaction.UpdateZone(ctx, zone); err != nil {
return fmt.Errorf("failed to update zone: %w", err)
return fmt.Errorf("update zone: %w", err)
}
change = affectedpeers.Change{DistributionGroupIDs: slices.Concat(zone.DistributionGroups, oldGroups)}
if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil {
return fmt.Errorf("load affected peers: %w", err)
}
if err = transaction.IncrementNetworkSerial(ctx, accountID); err != nil {
return fmt.Errorf("increment network serial: %w", err)
}
return nil
@@ -145,7 +173,7 @@ func (m *managerImpl) UpdateZone(ctx context.Context, accountID, userID string,
m.accountManager.StoreEvent(ctx, userID, zone.ID, accountID, activity.DNSZoneUpdated, zone.EventMeta())
go m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceZone, Operation: types.UpdateOperationUpdate})
m.accountManager.ExpandAndUpdateAffected(ctx, accountID, snap, change)
return zone, nil
}
@@ -159,13 +187,23 @@ func (m *managerImpl) DeleteZone(ctx context.Context, accountID, userID, zoneID
return status.NewPermissionDeniedError()
}
zone, err := m.store.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID)
if err != nil {
return fmt.Errorf("failed to get zone: %w", err)
}
var zone *zones.Zone
var snap *affectedpeers.Snapshot
var change affectedpeers.Change
var eventsToStore []func()
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID)
if err != nil {
return fmt.Errorf("get zone: %w", err)
}
// Load before delete: the post-delete state no longer references the groups.
change = affectedpeers.Change{DistributionGroupIDs: zone.DistributionGroups}
if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil {
return fmt.Errorf("load affected peers: %w", err)
}
records, err := transaction.GetZoneDNSRecords(ctx, store.LockingStrengthNone, accountID, zoneID)
if err != nil {
return fmt.Errorf("failed to get records: %w", err)
@@ -207,7 +245,7 @@ func (m *managerImpl) DeleteZone(ctx context.Context, accountID, userID, zoneID
event()
}
go m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceZone, Operation: types.UpdateOperationDelete})
m.accountManager.ExpandAndUpdateAffected(ctx, accountID, snap, change)
return nil
}
@@ -9,11 +9,11 @@ import (
"github.com/netbirdio/netbird/management/internals/modules/zones/records"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/affectedpeers"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/permissions/modules"
"github.com/netbirdio/netbird/management/server/permissions/operations"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/shared/management/status"
)
@@ -65,6 +65,8 @@ func (m *managerImpl) CreateRecord(ctx context.Context, accountID, userID, zoneI
}
var zone *zones.Zone
var snap *affectedpeers.Snapshot
var change affectedpeers.Change
record = records.NewRecord(accountID, zoneID, record.Name, record.Type, record.Content, record.TTL)
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
@@ -82,6 +84,11 @@ func (m *managerImpl) CreateRecord(ctx context.Context, accountID, userID, zoneI
return fmt.Errorf("failed to create dns record: %w", err)
}
change = affectedpeers.Change{DistributionGroupIDs: zone.DistributionGroups}
if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil {
return fmt.Errorf("load affected peers: %w", err)
}
err = transaction.IncrementNetworkSerial(ctx, accountID)
if err != nil {
return fmt.Errorf("failed to increment network serial: %w", err)
@@ -96,7 +103,7 @@ func (m *managerImpl) CreateRecord(ctx context.Context, accountID, userID, zoneI
meta := record.EventMeta(zone.ID, zone.Name)
m.accountManager.StoreEvent(ctx, userID, record.ID, accountID, activity.DNSRecordCreated, meta)
go m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceZoneRecord, Operation: types.UpdateOperationCreate})
m.accountManager.ExpandAndUpdateAffected(ctx, accountID, snap, change)
return record, nil
}
@@ -112,6 +119,8 @@ func (m *managerImpl) UpdateRecord(ctx context.Context, accountID, userID, zoneI
var zone *zones.Zone
var record *records.Record
var snap *affectedpeers.Snapshot
var change affectedpeers.Change
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID)
@@ -141,6 +150,11 @@ func (m *managerImpl) UpdateRecord(ctx context.Context, accountID, userID, zoneI
return fmt.Errorf("failed to update dns record: %w", err)
}
change = affectedpeers.Change{DistributionGroupIDs: zone.DistributionGroups}
if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil {
return fmt.Errorf("load affected peers: %w", err)
}
err = transaction.IncrementNetworkSerial(ctx, accountID)
if err != nil {
return fmt.Errorf("failed to increment network serial: %w", err)
@@ -155,7 +169,7 @@ func (m *managerImpl) UpdateRecord(ctx context.Context, accountID, userID, zoneI
meta := record.EventMeta(zone.ID, zone.Name)
m.accountManager.StoreEvent(ctx, userID, record.ID, accountID, activity.DNSRecordUpdated, meta)
go m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceZoneRecord, Operation: types.UpdateOperationUpdate})
m.accountManager.ExpandAndUpdateAffected(ctx, accountID, snap, change)
return record, nil
}
@@ -171,6 +185,8 @@ func (m *managerImpl) DeleteRecord(ctx context.Context, accountID, userID, zoneI
var record *records.Record
var zone *zones.Zone
var snap *affectedpeers.Snapshot
var change affectedpeers.Change
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
zone, err = transaction.GetZoneByID(ctx, store.LockingStrengthUpdate, accountID, zoneID)
@@ -188,6 +204,11 @@ func (m *managerImpl) DeleteRecord(ctx context.Context, accountID, userID, zoneI
return fmt.Errorf("failed to delete dns record: %w", err)
}
change = affectedpeers.Change{DistributionGroupIDs: zone.DistributionGroups}
if snap, err = affectedpeers.Load(ctx, transaction, accountID, change); err != nil {
return fmt.Errorf("load affected peers: %w", err)
}
err = transaction.IncrementNetworkSerial(ctx, accountID)
if err != nil {
return fmt.Errorf("failed to increment network serial: %w", err)
@@ -202,7 +223,7 @@ func (m *managerImpl) DeleteRecord(ctx context.Context, accountID, userID, zoneI
meta := record.EventMeta(zone.ID, zone.Name)
m.accountManager.StoreEvent(ctx, userID, recordID, accountID, activity.DNSRecordDeleted, meta)
go m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceZoneRecord, Operation: types.UpdateOperationDelete})
m.accountManager.ExpandAndUpdateAffected(ctx, accountID, snap, change)
return nil
}