Merge branch 'main' into embedded-vnc

# Conflicts:
#	client/ui/frontend/src/app.tsx
#	client/ui/frontend/src/modules/main/MainConnectionStatusSwitch.tsx
#	client/ui/i18n/locales/uk/common.json
#	go.sum
This commit is contained in:
Viktor Liu
2026-09-22 14:53:34 +02:00
205 changed files with 12118 additions and 1280 deletions
+5 -2
View File
@@ -719,8 +719,10 @@ func (am *DefaultAccountManager) schedulePeerLoginExpiration(ctx context.Context
log.WithContext(ctx).Tracef("peer login expiration job for account %s is already scheduled", accountID)
return
}
// The job outlives the request that arms it, so it must not inherit the request's cancellation.
jobCtx := context.WithoutCancel(ctx)
if nextRun, ok := am.getNextPeerExpiration(ctx, accountID); ok {
go am.peerLoginExpiry.Schedule(ctx, nextRun, accountID, am.peerLoginExpirationJob(ctx, accountID))
go am.peerLoginExpiry.Schedule(jobCtx, nextRun, accountID, am.peerLoginExpirationJob(jobCtx, accountID))
}
}
@@ -752,8 +754,9 @@ func (am *DefaultAccountManager) peerInactivityExpirationJob(ctx context.Context
// checkAndSchedulePeerInactivityExpiration periodically checks for inactive peers to end their sessions
func (am *DefaultAccountManager) checkAndSchedulePeerInactivityExpiration(ctx context.Context, accountID string) {
am.peerInactivityExpiry.Cancel(ctx, []string{accountID})
jobCtx := context.WithoutCancel(ctx)
if nextRun, ok := am.getNextInactivePeerExpiration(ctx, accountID); ok {
go am.peerInactivityExpiry.Schedule(ctx, nextRun, accountID, am.peerInactivityExpirationJob(ctx, accountID))
go am.peerInactivityExpiry.Schedule(jobCtx, nextRun, accountID, am.peerInactivityExpirationJob(jobCtx, accountID))
}
}
+183 -6
View File
@@ -1920,6 +1920,154 @@ func TestDefaultAccountManager_MarkPeerConnected_PeerLoginExpiration(t *testing.
}
}
func TestDefaultAccountManager_SchedulePeerLoginExpiration_IncludesOfflinePeers(t *testing.T) {
manager, updateManager, err := createManager(t)
require.NoError(t, err, "unable to create account manager")
accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID})
require.NoError(t, err, "unable to create an account")
connectedKey, offlineKey := addExpiringPeers(t, manager)
_, err = manager.UpdateAccountSettings(context.Background(), accountID, userID, &types.Settings{
PeerLoginExpiration: time.Hour,
PeerLoginExpirationEnabled: true,
Extra: &types.ExtraSettings{},
})
require.NoError(t, err, "expecting to update account settings successfully but got error")
manager.peerLoginExpiry.CancelAll(context.Background())
// The connected peer logged in just now, so a job computed from connected peers alone
// would be armed for an hour. The offline peer's login expires in two seconds; a
// reconnect of that peer must not have to wait for the connected peer's tick.
now := time.Now().UTC()
setPeerLogin(t, manager, accountID, connectedKey, true, now)
setPeerLogin(t, manager, accountID, offlineKey, false, now.Add(-time.Hour+2*time.Second))
offlinePeer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, offlineKey)
require.NoError(t, err)
updateManager.CreateChannel(context.Background(), offlinePeer.ID)
manager.peerLoginExpiry = NewDefaultScheduler()
t.Cleanup(func() { manager.peerLoginExpiry.CancelAll(context.Background()) })
manager.schedulePeerLoginExpiration(context.Background(), accountID)
// The flag is committed per peer before the disconnect fans out, so wait for both.
require.Eventually(t, func() bool {
peer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, offlineKey)
return err == nil && peer.Status.LoginExpired && !updateManager.HasChannel(offlinePeer.ID)
}, 10*time.Second, 100*time.Millisecond, "offline peer should be expired and disconnected at its own deadline")
connectedPeer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, connectedKey)
require.NoError(t, err)
assert.False(t, connectedPeer.Status.LoginExpired, "connected peer with a fresh login must not expire")
}
func TestDefaultAccountManager_SchedulePeerLoginExpiration_DetachesRequestContext(t *testing.T) {
manager, _, err := createManager(t)
require.NoError(t, err, "unable to create account manager")
accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID})
require.NoError(t, err, "unable to create an account")
connectedKey, _ := addExpiringPeers(t, manager)
setPeerLogin(t, manager, accountID, connectedKey, true, time.Now().UTC())
scheduled := make(chan context.Context, 1)
manager.peerLoginExpiry = &MockScheduler{
IsSchedulerRunningFunc: func(string) bool { return false },
ScheduleFunc: func(ctx context.Context, _ time.Duration, _ string, _ func() (time.Duration, bool)) {
scheduled <- ctx
},
}
requestCtx, cancel := context.WithCancel(context.Background())
manager.schedulePeerLoginExpiration(requestCtx, accountID)
cancel()
select {
case jobCtx := <-scheduled:
assert.NoError(t, jobCtx.Err(), "the expiration job must outlive the request that armed it")
case <-time.After(time.Second):
t.Fatal("timeout while waiting for the job to be scheduled")
}
}
func TestDefaultAccountManager_ExpireAndUpdatePeers_SkipsPeerThatLoggedInAgain(t *testing.T) {
manager, updateManager, err := createManager(t)
require.NoError(t, err, "unable to create account manager")
accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID})
require.NoError(t, err, "unable to create an account")
reloggedKey, staleKey := addExpiringPeers(t, manager)
_, err = manager.UpdateAccountSettings(context.Background(), accountID, userID, &types.Settings{
PeerLoginExpiration: time.Hour,
PeerLoginExpirationEnabled: true,
Extra: &types.ExtraSettings{},
})
require.NoError(t, err, "expecting to update account settings successfully but got error")
manager.peerLoginExpiry.CancelAll(context.Background())
expiredLogin := time.Now().UTC().Add(-2 * time.Hour)
setPeerLogin(t, manager, accountID, reloggedKey, true, expiredLogin)
setPeerLogin(t, manager, accountID, staleKey, true, expiredLogin)
expiredPeers, err := manager.getExpiredPeers(context.Background(), accountID)
require.NoError(t, err)
require.Len(t, expiredPeers, 2, "both peers should be due for expiration")
// The job holds the candidate list while one peer completes a fresh login, which
// moves its deadline into the future and must win over the stale candidate entry.
setPeerLogin(t, manager, accountID, reloggedKey, true, time.Now().UTC())
reloggedPeer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, reloggedKey)
require.NoError(t, err)
stalePeer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, staleKey)
require.NoError(t, err)
updateManager.CreateChannel(context.Background(), reloggedPeer.ID)
updateManager.CreateChannel(context.Background(), stalePeer.ID)
err = manager.expireAndUpdatePeers(context.Background(), accountID, expiredPeers, peerExpirationSessionExpired)
require.NoError(t, err)
reloggedPeer, err = manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, reloggedKey)
require.NoError(t, err)
assert.False(t, reloggedPeer.Status.LoginExpired, "a peer that logged in again must not be flagged from the stale candidate list")
assert.True(t, reloggedPeer.Status.Connected, "the re-logged peer must keep its connected status")
assert.True(t, updateManager.HasChannel(reloggedPeer.ID), "the re-logged peer's update channel must stay open")
stalePeer, err = manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, staleKey)
require.NoError(t, err)
assert.True(t, stalePeer.Status.LoginExpired, "a peer that is still due must be flagged")
assert.False(t, updateManager.HasChannel(stalePeer.ID), "the expired peer's update channel must be closed")
}
// addExpiringPeers registers two SSO peers with login expiration enabled and returns their public keys.
func addExpiringPeers(t *testing.T, manager *DefaultAccountManager) (string, string) {
t.Helper()
keys := make([]string, 0, 2)
for _, hostname := range []string{"connected-peer", "offline-peer"} {
key, err := wgtypes.GenerateKey()
require.NoError(t, err, "unable to generate WireGuard key")
_, _, _, _, err = manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{
Key: key.PublicKey().String(),
Meta: nbpeer.PeerSystemMeta{Hostname: hostname},
LoginExpirationEnabled: true,
}, false)
require.NoError(t, err, "unable to add peer")
keys = append(keys, key.PublicKey().String())
}
return keys[0], keys[1]
}
func setPeerLogin(t *testing.T, manager *DefaultAccountManager, accountID, peerKey string, connected bool, lastLogin time.Time) {
t.Helper()
peer, err := manager.Store.GetPeerByPeerPubKey(context.Background(), store.LockingStrengthNone, peerKey)
require.NoError(t, err)
peer.Status.Connected = connected
peer.LastLogin = &lastLogin
require.NoError(t, manager.Store.SavePeer(context.Background(), accountID, peer))
}
func TestDefaultAccountManager_MarkPeerDisconnected_SchedulesInactivityExpiration(t *testing.T) {
manager, _, err := createManager(t)
require.NoError(t, err, "unable to create account manager")
@@ -2702,7 +2850,7 @@ func TestAccount_GetNextPeerExpiration(t *testing.T) {
expectedNextExpiration: time.Duration(0),
},
{
name: "No connected peers, no expiration",
name: "Offline peer with expiration, return expiration",
peers: map[string]*nbpeer.Peer{
"peer-1": {
Status: &nbpeer.PeerStatus{
@@ -2721,8 +2869,33 @@ func TestAccount_GetNextPeerExpiration(t *testing.T) {
},
expiration: time.Second,
expirationEnabled: false,
expectedNextRun: false,
expectedNextExpiration: time.Duration(0),
expectedNextRun: true,
expectedNextExpiration: time.Second,
},
{
name: "Offline peer with the earliest deadline defines the next run",
peers: map[string]*nbpeer.Peer{
"peer-1": {
Status: &nbpeer.PeerStatus{
Connected: true,
},
LoginExpirationEnabled: true,
LastLogin: util.ToPtr(time.Now().UTC()),
UserID: userID,
},
"peer-2": {
Status: &nbpeer.PeerStatus{
Connected: false,
},
LoginExpirationEnabled: true,
LastLogin: util.ToPtr(time.Now().UTC().Add(-50 * time.Minute)),
UserID: userID,
},
},
expiration: time.Hour,
expirationEnabled: true,
expectedNextRun: true,
expectedNextExpiration: 10 * time.Minute,
},
{
name: "Connected peers with disabled expiration, no expiration",
@@ -3343,7 +3516,13 @@ func buildTestManager(t testing.TB, store store.Store, nmdataStore *networkmapdb
eventStore := &activity.InMemoryEventStore{}
metrics, err := telemetry.NewDefaultAppMetrics(context.Background())
// Everything built here watches this context; cancelling it on cleanup stops
// the metrics flushers, caches and controllers instead of leaking them for
// the rest of the package run.
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
metrics, err := telemetry.NewDefaultAppMetrics(ctx)
if err != nil {
return nil, nil, err
}
@@ -3362,8 +3541,6 @@ func buildTestManager(t testing.TB, store store.Store, nmdataStore *networkmapdb
Return(nil).
AnyTimes()
ctx := context.Background()
cacheStore, err := cache.NewStore(ctx, 100*time.Millisecond, 300*time.Millisecond, 100)
if err != nil {
return nil, nil, err
+7 -3
View File
@@ -284,6 +284,9 @@ const (
// AgentNetworkSettingsDeleted indicates that a user deleted the Agent Network account settings, releasing the endpoint
AgentNetworkSettingsDeleted Activity = 142
// CustomDomainValidationExpired indicates that an unvalidated domain registration expired.
CustomDomainValidationExpired Activity = 143
AccountDeleted Activity = 99999
)
@@ -461,9 +464,10 @@ var activityMap = map[Activity]Code{
AccountMetricsPushEnabled: {"Account metrics push enabled", "account.setting.metrics.push.enable"},
AccountMetricsPushDisabled: {"Account metrics push disabled", "account.setting.metrics.push.disable"},
DomainAdded: {"Domain added", "domain.add"},
DomainDeleted: {"Domain deleted", "domain.delete"},
DomainValidated: {"Domain validated", "domain.validate"},
DomainAdded: {"Domain added", "domain.add"},
DomainDeleted: {"Domain deleted", "domain.delete"},
DomainValidated: {"Domain validated", "domain.validate"},
CustomDomainValidationExpired: {"Unvalidated domain registration expired", "domain.validation.expire"},
}
// StringCode returns a string code of the activity
@@ -165,16 +165,16 @@ func (store *Store) Get(ctx context.Context, accountID string, offset, limit int
return store.processResult(ctx, events)
}
// Save an event in the SQLite events table end encrypt the "email" element in meta map
func (store *Store) Save(_ context.Context, event *activity.Event) (*activity.Event, error) {
// Save persists an activity event and encrypts deleted user details using the caller's context.
func (store *Store) Save(ctx context.Context, event *activity.Event) (*activity.Event, error) {
eventCopy := event.Copy()
meta, err := store.saveDeletedUserEmailAndNameInEncrypted(eventCopy)
meta, err := store.saveDeletedUserEmailAndNameInEncrypted(ctx, eventCopy)
if err != nil {
return nil, err
}
eventCopy.Meta = meta
if err = store.db.Create(eventCopy).Error; err != nil {
if err = store.db.WithContext(ctx).Create(eventCopy).Error; err != nil {
return nil, err
}
@@ -183,7 +183,7 @@ func (store *Store) Save(_ context.Context, event *activity.Event) (*activity.Ev
// saveDeletedUserEmailAndNameInEncrypted if the meta contains email and name then store it in encrypted way and delete
// this item from meta map
func (store *Store) saveDeletedUserEmailAndNameInEncrypted(event *activity.Event) (map[string]any, error) {
func (store *Store) saveDeletedUserEmailAndNameInEncrypted(ctx context.Context, event *activity.Event) (map[string]any, error) {
email, ok := event.Meta["email"]
if !ok {
return event.Meta, nil
@@ -211,7 +211,7 @@ func (store *Store) saveDeletedUserEmailAndNameInEncrypted(event *activity.Event
}
deletedUser.Name = encryptedName
err = store.db.Clauses(clause.OnConflict{
err = store.db.WithContext(ctx).Clauses(clause.OnConflict{
Columns: []clause.Column{{Name: "id"}},
DoUpdates: clause.AssignmentColumns([]string{"email", "name"}),
}).Create(deletedUser).Error
@@ -7,11 +7,49 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/util/crypt"
)
func TestSave_CancellationWhileWaitingForConnection(t *testing.T) {
t.Setenv(storeEngineEnv, "sqlite")
key, err := crypt.GenerateKey()
require.NoError(t, err)
store, err := NewSqlStore(context.Background(), t.TempDir(), key)
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, store.Close(context.Background())) })
db, err := store.db.DB()
require.NoError(t, err)
conn, err := db.Conn(context.Background())
require.NoError(t, err)
t.Cleanup(func() { _ = conn.Close() })
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
result := make(chan error, 1)
go func() {
_, err := store.Save(ctx, &activity.Event{
Timestamp: time.Now().UTC(), Activity: activity.CustomDomainValidationExpired,
AccountID: "account-id", TargetID: "domain-id", InitiatorID: activity.SystemInitiator,
})
result <- err
}()
select {
case err := <-result:
assert.ErrorIs(t, err, context.DeadlineExceeded)
require.NoError(t, conn.Close())
case <-time.After(time.Second):
// Release the connection so a regression cannot leave the writer running.
require.NoError(t, conn.Close())
assert.ErrorIs(t, <-result, context.DeadlineExceeded)
t.Error("activity writes must stop waiting when their deadline expires")
}
events, err := store.Get(context.Background(), "account-id", 0, 10, true)
require.NoError(t, err)
assert.Empty(t, events, "a timed-out write must not persist after the connection is released")
}
func TestNewSqlStore(t *testing.T) {
dataDir := t.TempDir()
key, _ := crypt.GenerateKey()
@@ -89,6 +89,7 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
// Bootstrap is an explicit settings create; providers have no settings
// side effects anymore.
seedPrivateProxyCluster(t, am.Store, clusterAddr)
before, err := mgr.CreateSettings(ctx, adminUserID, agenttypes.DefaultSettings(accountID), clusterAddr, "")
require.NoError(t, err, "CreateSettings must bootstrap the row")
require.Equal(t, clusterAddr, before.ProxyAddress, "proxy address pinned at bootstrap")
@@ -13,6 +13,7 @@ import (
networkmap "github.com/netbirdio/netbird/management/internals/controllers/network_map"
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
agenttypes "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
rpproxy "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/store"
@@ -92,6 +93,7 @@ func TestAgentNetwork_ProviderCRUD_FansOutToProxyAndClientPeers(t *testing.T) {
// UpdateAccountPeers, which is the path under test.
agentMgr := agentnetwork.NewManager(am.Store, permissions.NewManager(am.Store), am, nil)
seedPrivateProxyCluster(t, am.Store, clusterAddr)
_, err = agentMgr.CreateSettings(ctx, adminUserID, agenttypes.DefaultSettings(accountID), clusterAddr, "")
require.NoError(t, err, "CreateSettings must bootstrap the endpoint")
// The bootstrap itself reconciles and queues updates on both channels;
@@ -222,3 +224,22 @@ func synthZoneRData(sync *nbproto.SyncResponse, clusterAddr, fqdn string) string
}
return ""
}
// seedPrivateProxyCluster registers a connected proxy with private capabilities in a
// netbird client for clusterAddr, matching what a real deployment looks like
// when the account bootstraps: the agent-network gateway service is always
// private, so its cluster has to be one that can serve private services.
func seedPrivateProxyCluster(t *testing.T, st store.Store, clusterAddr string) {
t.Helper()
private := true
now := time.Now().UTC()
require.NoError(t, st.SaveProxy(context.Background(), &rpproxy.Proxy{
ID: "agent-net-proxy-" + clusterAddr,
SessionID: "agent-net-session",
ClusterAddress: clusterAddr,
LastSeen: now,
ConnectedAt: &now,
Status: rpproxy.StatusConnected,
Capabilities: rpproxy.Capabilities{Private: &private},
}), "seeding the proxy cluster must succeed")
}
+26 -15
View File
@@ -61,23 +61,34 @@ func (am *DefaultAccountManager) GetEvents(ctx context.Context, accountID, userI
return filtered, nil
}
// StoreEvent records an activity, waiting for expiration events before cleanup can stop.
func (am *DefaultAccountManager) StoreEvent(ctx context.Context, initiatorID, targetID, accountID string, activityID activity.ActivityDescriber, meta map[string]any) {
if isEnabled() {
go func() {
_, err := am.eventStore.Save(ctx, &activity.Event{
Timestamp: time.Now().UTC(),
Activity: activityID.(activity.Activity),
InitiatorID: initiatorID,
TargetID: targetID,
AccountID: accountID,
Meta: meta,
})
if err != nil {
// todo add metric
log.WithContext(ctx).Errorf("received an error while storing an activity event, error: %s", err)
}
}()
if !isEnabled() {
return
}
eventStore := am.eventStore
save := func(ctx context.Context) {
_, err := eventStore.Save(ctx, &activity.Event{
Timestamp: time.Now().UTC(),
Activity: activityID.(activity.Activity),
InitiatorID: initiatorID,
TargetID: targetID,
AccountID: accountID,
Meta: meta,
})
if err != nil {
log.WithContext(ctx).Errorf("received an error while storing an activity event, error: %s", err)
}
}
if activityID == activity.CustomDomainValidationExpired {
// The domain is already deleted; shutdown must allow its audit write to finish.
ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
defer cancel()
save(ctx)
return
}
// Request cancellation must not discard the audit record of a completed operation.
go save(context.WithoutCancel(ctx))
}
type eventUserInfo struct {
+42
View File
@@ -6,10 +6,52 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/activity"
activitystore "github.com/netbirdio/netbird/management/server/activity/store"
"github.com/netbirdio/netbird/util/crypt"
)
func TestStoreEvent_CanceledContext(t *testing.T) {
t.Setenv("NB_EVENT_ACTIVITY_LOG_ENABLED", "true")
t.Setenv("NB_ACTIVITY_EVENT_STORE_ENGINE", "sqlite")
for _, code := range []activity.Activity{activity.CustomDomainValidationExpired, activity.DomainAdded} {
t.Run(code.StringCode(), func(t *testing.T) {
dir := t.TempDir()
key, err := crypt.GenerateKey()
require.NoError(t, err)
eventStore, err := activitystore.NewSqlStore(context.Background(), dir, key)
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, eventStore.Close(context.Background())) })
manager := &DefaultAccountManager{eventStore: eventStore}
ctx, cancel := context.WithCancel(context.Background())
cancel()
// The operation already succeeded when shutdown or the request cancels its context.
manager.StoreEvent(ctx, activity.SystemInitiator, "domain-id", "account-id",
code, map[string]any{"domain": "expired.example.com"})
if code != activity.CustomDomainValidationExpired {
require.Eventually(t, func() bool {
events, err := eventStore.Get(context.Background(), "account-id", 0, 10, true)
return err == nil && len(events) == 1
}, time.Second, time.Millisecond, "asynchronous events must survive request cancellation")
}
require.NoError(t, eventStore.Close(context.Background()))
reopened, err := activitystore.NewSqlStore(context.Background(), dir, key)
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, reopened.Close(context.Background())) })
events, err := reopened.Get(context.Background(), "account-id", 0, 10, true)
require.NoError(t, err)
require.Len(t, events, 1, "the event must be persisted before shutdown closes the store")
assert.Equal(t, code, events[0].Activity, "persist the requested activity")
assert.Equal(t, "domain-id", events[0].TargetID, "retain the registration ID")
assert.Equal(t, "expired.example.com", events[0].Meta["domain"], "retain the domain name")
})
}
}
func generateAndStoreEvents(t *testing.T, manager *DefaultAccountManager, typ activity.Activity, initiatorID, targetID,
accountID string, count int) {
t.Helper()
+53 -5
View File
@@ -101,10 +101,8 @@ func (am *DefaultAccountManager) CreateGroup(ctx context.Context, accountID, use
return status.Errorf(status.Internal, "failed to create group: %v", err)
}
for _, peerID := range newGroup.Peers {
if err := transaction.AddPeerToGroup(ctx, accountID, peerID, newGroup.ID); err != nil {
return status.Errorf(status.Internal, "failed to add peer %s to group %s: %v", peerID, newGroup.ID, err)
}
if err = syncGroupMembership(ctx, transaction, accountID, newGroup.ID, newGroup.Peers, nil); err != nil {
return err
}
snap, err = affectedpeers.Load(ctx, transaction, accountID, change)
@@ -200,6 +198,9 @@ func (am *DefaultAccountManager) UpdateGroup(ctx context.Context, accountID, use
// syncGroupMembership applies the peer membership delta for a group within a transaction.
func syncGroupMembership(ctx context.Context, transaction store.Store, accountID, groupID string, peersToAdd, peersToRemove []string) error {
if err := validateGroupPeers(ctx, transaction, accountID, peersToAdd); err != nil {
return err
}
for _, peerID := range peersToAdd {
if err := transaction.AddPeerToGroup(ctx, accountID, peerID, groupID); err != nil {
return status.Errorf(status.Internal, "failed to add peer %s to group %s: %v", peerID, groupID, err)
@@ -213,6 +214,25 @@ func syncGroupMembership(ctx context.Context, transaction store.Store, accountID
return nil
}
func validateGroupPeers(ctx context.Context, transaction store.Store, accountID string, peerIDs []string) error {
if len(peerIDs) == 0 {
return nil
}
peers, err := transaction.GetPeersByIDs(ctx, store.LockingStrengthNone, accountID, peerIDs)
if err != nil {
return err
}
for _, peerID := range peerIDs {
if _, ok := peers[peerID]; !ok {
return status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID)
}
}
return nil
}
// CreateGroups adds new groups to the account.
// Note: This function does not acquire the global lock.
// It is the caller's responsibility to ensure proper locking is in place before invoking this method.
@@ -540,7 +560,7 @@ func (am *DefaultAccountManager) GroupAddPeer(ctx context.Context, accountID, gr
change := affectedpeers.Change{OutputPeerIDs: []string{peerID}, LinkGroups: []string{groupID}}
err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
if err := transaction.AddPeerToGroup(ctx, accountID, peerID, groupID); err != nil {
if err := syncGroupMembership(ctx, transaction, accountID, groupID, []string{peerID}, nil); err != nil {
return err
}
@@ -754,6 +774,14 @@ func validateDeleteGroup(ctx context.Context, transaction store.Store, group *ty
return &GroupLinkError{"agent network policy", linkedPolicy.Name}
}
isLinked, linkedRule, err := isGroupLinkedToAgentNetworkBudgetRule(ctx, transaction, group.AccountID, group.ID)
if err != nil {
return status.Errorf(status.Internal, "failed to check agent network budget rules")
}
if isLinked {
return &GroupLinkError{"agent network budget rule", linkedRule.Name}
}
return checkGroupLinkedToSettings(ctx, transaction, group)
}
@@ -925,6 +953,26 @@ func isGroupLinkedToAgentNetworkPolicy(ctx context.Context, transaction store.St
return false, nil
}
// isGroupLinkedToAgentNetworkBudgetRule checks if a group is a target of any
// account-level agent network budget rule.
func isGroupLinkedToAgentNetworkBudgetRule(ctx context.Context, transaction store.Store, accountID string, groupID string) (bool, *agentNetworkTypes.AccountBudgetRule, error) {
rules, err := transaction.GetAccountAgentNetworkBudgetRules(ctx, store.LockingStrengthNone, accountID)
if err != nil {
log.WithContext(ctx).Errorf("error retrieving agent network budget rules while checking group linkage: %v", err)
return false, nil, err
}
for _, rule := range rules {
if rule == nil {
continue
}
if slices.Contains(rule.TargetGroups, groupID) {
return true, rule, nil
}
}
return false, nil, nil
}
// areGroupChangesAffectPeers checks if any changes to the specified groups will affect peers.
// It fetches each collection once and checks all groupIDs against them in memory.
func areGroupChangesAffectPeers(ctx context.Context, transaction store.Store, accountID string, groupIDs []string) (bool, error) {
+124 -1
View File
@@ -11,10 +11,10 @@ import (
"testing"
"time"
"go.uber.org/mock/gomock"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"golang.org/x/exp/maps"
nbdns "github.com/netbirdio/netbird/dns"
@@ -132,6 +132,11 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
"grp-for-agent-network-policy",
"agent network policy",
},
{
"agent network budget rule",
"grp-for-agent-network-budget-rule",
"agent network budget rule",
},
{
"reverse proxy private service access group",
"grp-for-rp-private",
@@ -152,6 +157,16 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
return
}
group, getErr := am.GetGroup(context.Background(), account.Id, testCase.groupID, groupAdminUserID)
if getErr != nil {
t.Errorf("group %s should still exist after failed deletion: %s", testCase.groupID, getErr)
return
}
if group == nil {
t.Errorf("group %s was deleted despite the failed deletion", testCase.groupID)
return
}
var sErr *status.Error
if errors.As(err, &sErr) {
if sErr.Message != testCase.expectedReason {
@@ -240,6 +255,12 @@ func TestDefaultAccountManager_DeleteGroups(t *testing.T) {
groupIDs: []string{"grp-for-agent-network-policy"},
expectedReasons: []string{"agent network policy"},
},
{
name: "agent network budget rule",
groupIDs: []string{"grp-for-agent-network-budget-rule"},
expectedReasons: []string{"agent network budget rule"},
expectedNotDeleted: []string{"grp-for-agent-network-budget-rule"},
},
{
name: "reverse proxy services",
groupIDs: []string{"grp-for-rp-private", "grp-for-rp-bearer"},
@@ -501,6 +522,14 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
Peers: make([]string, 0),
}
groupForAgentNetworkBudgetRule := &types.Group{
ID: "grp-for-agent-network-budget-rule",
AccountID: "account-id",
Name: "Group for agent network budget rules",
Issued: types.GroupIssuedAPI,
Peers: make([]string, 0),
}
groupForRPPrivate := &types.Group{
ID: "grp-for-rp-private",
AccountID: "account-id",
@@ -573,6 +602,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForUsers)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkPolicy)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkBudgetRule)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForRPPrivate)
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForRPBearer)
@@ -587,6 +617,20 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
return nil, nil, err
}
budgetRuleDecoy := agentNetworkTypes.NewAccountBudgetRule(accountID)
budgetRuleDecoy.Name = "Unrelated agent network budget rule"
budgetRuleDecoy.TargetGroups = []string{"unrelated-group"}
if err := am.Store.SaveAgentNetworkBudgetRule(context.Background(), budgetRuleDecoy); err != nil {
return nil, nil, err
}
budgetRule := agentNetworkTypes.NewAccountBudgetRule(accountID)
budgetRule.Name = "Example agent network budget rule"
budgetRule.TargetGroups = []string{groupForAgentNetworkBudgetRule.ID}
if err := am.Store.SaveAgentNetworkBudgetRule(context.Background(), budgetRule); err != nil {
return nil, nil, err
}
// The decoy services are created first so the linkage check has to scan
// past services that do not reference the groups under test.
rpServices := []*rpservice.Service{
@@ -1236,3 +1280,82 @@ func Test_IncrementNetworkSerial(t *testing.T) {
assert.Equal(t, totalPeers, int(account.Network.Serial), "Expected %d serial increases in account %s, got %d", totalPeers, accountID, account.Network.Serial)
}
func TestDefaultAccountManager_GroupPeersMustBelongToAccount(t *testing.T) {
manager, _, account, peer1, _, _ := setupNetworkMapTest(t)
otherAccount, err := createAccount(manager, "other_account", "other_user", "")
require.NoError(t, err)
foreignPeer := &peer2.Peer{
ID: "foreign-peer",
AccountID: otherAccount.Id,
Key: "foreign-key",
DNSLabel: "foreign-peer",
IP: uint32ToIP(1),
}
require.NoError(t, manager.Store.AddPeerToAccount(context.Background(), foreignPeer))
assertRejected := func(t *testing.T, err error) {
t.Helper()
require.Error(t, err)
s, ok := status.FromError(err)
require.True(t, ok, "expected status error, got %v", err)
assert.Equal(t, status.InvalidArgument, s.Type(), "peer outside the account should be rejected as invalid argument")
}
t.Run("create rejects foreign peer", func(t *testing.T) {
err := manager.CreateGroup(context.Background(), account.Id, userID, &types.Group{
Name: "foreign",
Issued: types.GroupIssuedAPI,
Peers: []string{peer1.ID, foreignPeer.ID},
})
assertRejected(t, err)
_, err = manager.Store.GetGroupByName(context.Background(), store.LockingStrengthNone, account.Id, "foreign")
assert.Error(t, err, "rejected create must not persist the group")
})
t.Run("update rejects foreign and unknown peers", func(t *testing.T) {
group := &types.Group{ID: "own", Name: "own", Issued: types.GroupIssuedAPI, Peers: []string{peer1.ID}}
require.NoError(t, manager.CreateGroup(context.Background(), account.Id, userID, group))
group.Peers = []string{peer1.ID, foreignPeer.ID}
assertRejected(t, manager.UpdateGroup(context.Background(), account.Id, userID, group))
group.Peers = []string{peer1.ID, "does-not-exist"}
assertRejected(t, manager.UpdateGroup(context.Background(), account.Id, userID, group))
stored, err := manager.Store.GetGroupByID(context.Background(), store.LockingStrengthNone, account.Id, group.ID)
require.NoError(t, err)
assert.Equal(t, []string{peer1.ID}, stored.Peers, "rejected updates must not change membership")
})
t.Run("update tolerates and drops pre-existing dangling members", func(t *testing.T) {
group := &types.Group{ID: "polluted", Name: "polluted", Issued: types.GroupIssuedAPI, Peers: []string{peer1.ID}}
require.NoError(t, manager.CreateGroup(context.Background(), account.Id, userID, group))
require.NoError(t, manager.Store.AddPeerToGroup(context.Background(), account.Id, foreignPeer.ID, group.ID))
group.Peers = []string{peer1.ID, foreignPeer.ID}
assert.NoError(t, manager.UpdateGroup(context.Background(), account.Id, userID, group), "keeping an existing member must not be rejected")
group.Peers = []string{peer1.ID}
require.NoError(t, manager.UpdateGroup(context.Background(), account.Id, userID, group))
stored, err := manager.Store.GetGroupByID(context.Background(), store.LockingStrengthNone, account.Id, group.ID)
require.NoError(t, err)
assert.Equal(t, []string{peer1.ID}, stored.Peers, "dangling member should be removed once omitted")
})
t.Run("direct add rejects foreign and unknown peers", func(t *testing.T) {
group := &types.Group{ID: "direct", Name: "direct", Issued: types.GroupIssuedAPI, Peers: []string{peer1.ID}}
require.NoError(t, manager.CreateGroup(context.Background(), account.Id, userID, group))
assertRejected(t, manager.GroupAddPeer(context.Background(), account.Id, group.ID, foreignPeer.ID))
assertRejected(t, manager.GroupAddPeer(context.Background(), account.Id, group.ID, "does-not-exist"))
stored, err := manager.Store.GetGroupByID(context.Background(), store.LockingStrengthNone, account.Id, group.ID)
require.NoError(t, err)
assert.Equal(t, []string{peer1.ID}, stored.Peers, "rejected direct adds must not change membership")
})
}
@@ -148,13 +148,10 @@ func (h *handler) updateGroup(w http.ResponseWriter, r *http.Request) {
peers = *req.Peers
}
resources := make([]types.Resource, 0)
if req.Resources != nil {
for _, res := range *req.Resources {
resource := types.Resource{}
resource.FromAPIRequest(&res)
resources = append(resources, resource)
}
resources, err := resourcesFromAPIRequest(req.Resources)
if err != nil {
util.WriteError(r.Context(), err, w)
return
}
group := types.Group{
@@ -210,13 +207,10 @@ func (h *handler) createGroup(w http.ResponseWriter, r *http.Request) {
peers = *req.Peers
}
resources := make([]types.Resource, 0)
if req.Resources != nil {
for _, res := range *req.Resources {
resource := types.Resource{}
resource.FromAPIRequest(&res)
resources = append(resources, resource)
}
resources, err := resourcesFromAPIRequest(req.Resources)
if err != nil {
util.WriteError(r.Context(), err, w)
return
}
group := types.Group{
@@ -335,11 +329,30 @@ func toGroupResponse(peers []*nbpeer.Peer, group *types.Group) *api.Group {
gr.PeersCount = len(gr.Peers)
for _, res := range group.Resources {
resResp := res.ToAPIResponse()
gr.Resources = append(gr.Resources, *resResp)
if resResp := res.ToAPIResponse(); resResp != nil {
gr.Resources = append(gr.Resources, *resResp)
}
}
gr.ResourcesCount = len(gr.Resources)
return &gr
}
func resourcesFromAPIRequest(req *[]api.Resource) ([]types.Resource, error) {
resources := make([]types.Resource, 0)
if req == nil {
return resources, nil
}
for _, res := range *req {
if res.Id == "" || !types.ResourceType(res.Type).Valid() {
return nil, status.Errorf(status.InvalidArgument, "resource id shouldn't be empty and type must be one of: peer, domain, host, subnet")
}
resource := types.Resource{}
resource.FromAPIRequest(&res)
resources = append(resources, resource)
}
return resources, nil
}
@@ -8,8 +8,8 @@ import (
"fmt"
"io"
"net/http"
"net/netip"
"net/http/httptest"
"net/netip"
"strings"
"testing"
@@ -208,6 +208,33 @@ func TestWriteGroup(t *testing.T) {
expectedStatus: http.StatusUnprocessableEntity,
expectedBody: false,
},
{
name: "Write Group POST Empty Resource",
requestType: http.MethodPost,
requestPath: "/api/groups",
requestBody: bytes.NewBuffer(
[]byte(`{"name":"With Resource","resources":[{}]}`)),
expectedStatus: http.StatusUnprocessableEntity,
expectedBody: false,
},
{
name: "Write Group PUT Empty Resource",
requestType: http.MethodPut,
requestPath: "/api/groups/id-existed",
requestBody: bytes.NewBuffer(
[]byte(`{"name":"With Resource","resources":[{"id":"","type":"host"}]}`)),
expectedStatus: http.StatusUnprocessableEntity,
expectedBody: false,
},
{
name: "Write Group POST Unknown Resource Type",
requestType: http.MethodPost,
requestPath: "/api/groups",
requestBody: bytes.NewBuffer(
[]byte(`{"name":"With Resource","resources":[{"id":"res-1","type":"banana"}]}`)),
expectedStatus: http.StatusUnprocessableEntity,
expectedBody: false,
},
{
name: "Write Group PUT OK",
requestType: http.MethodPut,
@@ -376,6 +403,20 @@ func TestGetAllGroups(t *testing.T) {
}
}
func TestToGroupResponseSkipsEmptyResource(t *testing.T) {
group := &types.Group{
ID: "id-resources",
Name: "Resources",
Issued: types.GroupIssuedAPI,
Resources: []types.Resource{{}, {ID: "res-1", Type: types.ResourceTypeHost}},
}
got := toGroupResponse(nil, group)
assert.Equal(t, 1, got.ResourcesCount)
assert.Equal(t, []api.Resource{{Id: "res-1", Type: api.ResourceType(types.ResourceTypeHost)}}, got.Resources)
}
func TestDeleteGroup(t *testing.T) {
tt := []struct {
name string
@@ -3,6 +3,7 @@
package integration
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
@@ -10,6 +11,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/http/testing/testing_tools"
"github.com/netbirdio/netbird/management/server/http/testing/testing_tools/channel"
@@ -34,7 +36,7 @@ func Test_Events_GetAll(t *testing.T) {
for _, user := range users {
t.Run(user.name+" - Get all events", func(t *testing.T) {
apiHandler, _, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/events.sql", nil, false)
apiHandler, accountManager, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/events.sql", nil, false)
// First, perform a mutation to generate an event (create a group as admin)
groupBody, err := json.Marshal(&api.GroupRequest{Name: "eventTestGroup"})
@@ -44,7 +46,14 @@ func Test_Events_GetAll(t *testing.T) {
createReq := testing_tools.BuildRequest(t, groupBody, http.MethodPost, "/api/groups", testing_tools.TestAdminId)
createRecorder := httptest.NewRecorder()
apiHandler.ServeHTTP(createRecorder, createReq)
assert.Equal(t, http.StatusOK, createRecorder.Code, "Failed to create group to generate event")
require.Equal(t, http.StatusOK, createRecorder.Code, "Failed to create group to generate event")
// Group creation returns before its asynchronous audit write finishes.
require.EventuallyWithT(t, func(c *assert.CollectT) {
events, err := accountManager.GetEvents(context.Background(), testing_tools.TestAccountId, testing_tools.TestAdminId)
assert.NoError(c, err)
assert.NotEmpty(c, events, "wait for the group creation event before checking permissions")
}, time.Second, 10*time.Millisecond)
// Now query events
req := testing_tools.BuildRequest(t, []byte{}, http.MethodGet, "/api/events", user.userId)
+22 -15
View File
@@ -23,6 +23,9 @@ import (
"github.com/netbirdio/netbird/shared/management/status"
)
// maxDiscoveryDocumentSize caps the discovery document read at 1 MiB. Providers serve a few kilobytes.
const maxDiscoveryDocumentSize = 1 << 20
// oidcProviderJSON represents the OpenID Connect discovery document
type oidcProviderJSON struct {
Issuer string `json:"issuer"`
@@ -35,6 +38,10 @@ func validateOIDCIssuer(ctx context.Context, issuer string) error {
httpClient := &http.Client{
Timeout: 10 * time.Second,
// An issuer that redirects its own discovery document is misconfigured.
CheckRedirect: func(*http.Request, []*http.Request) error {
return http.ErrUseLastResponse
},
}
req, err := http.NewRequestWithContext(ctx, http.MethodGet, wellKnown, nil)
@@ -48,22 +55,22 @@ func validateOIDCIssuer(ctx context.Context, issuer string) error {
}
defer resp.Body.Close()
body, err := io.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("%w: unable to read response body: %v", types.ErrIdentityProviderIssuerUnreachable, err)
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("%w: %s", types.ErrIdentityProviderIssuerUnreachable, resp.Status)
}
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("%w: %s: %s", types.ErrIdentityProviderIssuerUnreachable, resp.Status, body)
body, err := io.ReadAll(io.LimitReader(resp.Body, maxDiscoveryDocumentSize+1))
if err != nil || len(body) > maxDiscoveryDocumentSize {
return fmt.Errorf("%w: failed to decode provider discovery object", types.ErrIdentityProviderIssuerUnreachable)
}
var p oidcProviderJSON
if err := json.Unmarshal(body, &p); err != nil {
return fmt.Errorf("%w: failed to decode provider discovery object: %v", types.ErrIdentityProviderIssuerUnreachable, err)
return fmt.Errorf("%w: failed to decode provider discovery object", types.ErrIdentityProviderIssuerUnreachable)
}
if p.Issuer != issuer {
return fmt.Errorf("%w: expected %q got %q", types.ErrIdentityProviderIssuerMismatch, issuer, p.Issuer)
return fmt.Errorf("%w: %q", types.ErrIdentityProviderIssuerMismatch, issuer)
}
return nil
@@ -151,15 +158,15 @@ func (am *DefaultAccountManager) CreateIdentityProvider(ctx context.Context, acc
return nil, status.NewPermissionDeniedError()
}
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager)
if !ok {
return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP")
}
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
// Generate ID if not provided
if idpConfig.ID == "" {
idpConfig.ID = generateIdentityProviderID(idpConfig.Type)
@@ -188,15 +195,15 @@ func (am *DefaultAccountManager) UpdateIdentityProvider(ctx context.Context, acc
return nil, status.NewPermissionDeniedError()
}
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
embeddedManager, ok := am.idpManager.(*idp.EmbeddedIdPManager)
if !ok {
return nil, status.Errorf(status.Internal, "identity provider management requires embedded IdP")
}
if err := validateIdentityProviderConfig(ctx, idpConfig); err != nil {
return nil, err
}
idpConfig.ID = idpID
idpConfig.AccountID = accountID
+45 -2
View File
@@ -7,6 +7,7 @@ import (
"net/http"
"net/http/httptest"
"path/filepath"
"strings"
"testing"
"time"
@@ -121,7 +122,7 @@ func createManagerWithEmbeddedIdPModeAndSetup(
}
func TestDefaultAccountManager_CreateIdentityProvider_Validation(t *testing.T) {
manager, _, err := createManager(t)
manager, _, err := createManagerWithEmbeddedIdP(t)
require.NoError(t, err)
userID := "testingUser"
@@ -233,7 +234,7 @@ func TestUpdateUserAuthWithSingleModeKeepsConfiguredDomain(t *testing.T) {
}
func TestDefaultAccountManager_UpdateIdentityProvider_Validation(t *testing.T) {
manager, _, err := createManager(t)
manager, _, err := createManagerWithEmbeddedIdP(t)
require.NoError(t, err)
userID := "testingUser"
@@ -355,3 +356,45 @@ func TestValidateOIDCIssuer_TrailingSlash(t *testing.T) {
require.Error(t, err)
assert.True(t, errors.Is(err, types.ErrIdentityProviderIssuerMismatch))
}
func TestValidateOIDCIssuer_DoesNotFollowRedirects(t *testing.T) {
var reached bool
target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
reached = true
w.WriteHeader(http.StatusForbidden)
}))
t.Cleanup(target.Close)
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, target.URL+"/redirect-target", http.StatusFound)
}))
t.Cleanup(srv.Close)
err := validateOIDCIssuer(context.Background(), srv.URL)
require.Error(t, err)
assert.False(t, reached, "Redirects are not followed")
}
func TestValidateOIDCIssuer_BoundsResponseSize(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"issuer":"` + strings.Repeat("a", maxDiscoveryDocumentSize) + `"}`))
}))
t.Cleanup(srv.Close)
err := validateOIDCIssuer(context.Background(), srv.URL)
require.ErrorIs(t, err, types.ErrIdentityProviderIssuerUnreachable)
assert.NotErrorIs(t, err, types.ErrIdentityProviderIssuerMismatch)
}
func TestValidateOIDCIssuer_RejectsTrailingContent(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"issuer":"http://` + r.Host + `"} {"issuer":"second"}`))
}))
t.Cleanup(srv.Close)
err := validateOIDCIssuer(context.Background(), srv.URL)
require.ErrorIs(t, err, types.ErrIdentityProviderIssuerUnreachable,
"Content after the first object is not a valid discovery document")
}
@@ -0,0 +1,22 @@
package migration
import (
"context"
"fmt"
"time"
"gorm.io/gorm"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
)
// MigrateCustomDomainValidationExpiry gives existing pending registrations a validation window.
func MigrateCustomDomainValidationExpiry(ctx context.Context, db *gorm.DB) error {
result := db.WithContext(ctx).Model(&domain.Domain{}).
Where("validated = ? AND validation_expires_at IS NULL", false).
Update("validation_expires_at", time.Now().UTC().Add(domain.ValidationTTL))
if result.Error != nil {
return fmt.Errorf("backfill custom domain validation expiry: %w", result.Error)
}
return nil
}
@@ -0,0 +1,44 @@
package migration_test
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
"github.com/netbirdio/netbird/management/server/migration"
)
func TestMigrateCustomDomainValidationExpiry(t *testing.T) {
db := setupDatabase(t)
require.NoError(t, db.AutoMigrate(&domain.Domain{}))
t.Cleanup(func() { require.NoError(t, db.Migrator().DropTable(&domain.Domain{})) })
ctx := context.Background()
existingDeadline := time.Now().UTC().Add(time.Hour).Truncate(time.Second)
rows := []domain.Domain{
{ID: "legacy", Domain: "legacy.example.com"},
{ID: "validated", Domain: "validated.example.com", Validated: true},
{ID: "pending", Domain: "pending.example.com", ValidationExpiresAt: &existingDeadline},
}
require.NoError(t, db.Create(&rows).Error)
before := time.Now().UTC()
require.NoError(t, migration.MigrateCustomDomainValidationExpiry(ctx, db))
after := time.Now().UTC()
var migrated domain.Domain
require.NoError(t, db.First(&migrated, "id = ?", "legacy").Error)
require.NotNil(t, migrated.ValidationExpiresAt)
assert.WithinRange(t, *migrated.ValidationExpiresAt, before.Truncate(time.Millisecond).Add(48*time.Hour), after.Add(48*time.Hour+time.Millisecond), "legacy pending registrations get a full window")
deadline := *migrated.ValidationExpiresAt
require.NoError(t, migration.MigrateCustomDomainValidationExpiry(ctx, db))
require.NoError(t, db.First(&migrated, "id = ?", "legacy").Error)
assert.Equal(t, deadline, *migrated.ValidationExpiresAt, "repeated migration must not extend the deadline")
var validated, pending domain.Domain
require.NoError(t, db.First(&validated, "id = ?", "validated").Error)
require.NoError(t, db.First(&pending, "id = ?", "pending").Error)
assert.Nil(t, validated.ValidationExpiresAt, "validated domains do not acquire an expiry")
require.NotNil(t, pending.ValidationExpiresAt)
assert.WithinDuration(t, existingDeadline, *pending.ValidationExpiresAt, 0, "existing deadlines must be preserved")
}
+10 -6
View File
@@ -1494,9 +1494,12 @@ func checkAuth(ctx context.Context, loginUserID string, peer *nbpeer.Peer) error
func peerLoginExpired(ctx context.Context, peer *nbpeer.Peer, settings *types.Settings) bool {
expired, expiresIn := peer.LoginExpired(settings.PeerLoginExpiration)
expired = settings.PeerLoginExpirationEnabled && expired
if expired || peer.Status.LoginExpired {
log.WithContext(ctx).Debugf("peer's %s login expired %v ago", peer.ID, expiresIn)
if settings.PeerLoginExpirationEnabled && expired {
log.WithContext(ctx).Debugf("peer's %s login expired %v ago", peer.ID, -expiresIn)
return true
}
if peer.Status.LoginExpired {
log.WithContext(ctx).Debugf("peer's %s login is marked as expired", peer.ID)
return true
}
return false
@@ -1643,7 +1646,9 @@ func (am *DefaultAccountManager) UpdateAccountPeer(ctx context.Context, accountI
// getNextPeerExpiration returns the minimum duration in which the next peer of the account will expire if it was found.
// If there is no peer that expires this function returns false and a duration of 0.
// This function only considers peers that haven't been expired yet and that are connected.
// This function only considers peers that haven't been expired yet. Offline peers count too:
// a running job is never re-armed on connect, so a peer that reconnects with an old login
// must already be part of the scheduled run.
func (am *DefaultAccountManager) getNextPeerExpiration(ctx context.Context, accountID string) (time.Duration, bool) {
peersWithExpiry, err := am.Store.GetAccountPeersWithExpiration(ctx, store.LockingStrengthNone, accountID)
if err != nil {
@@ -1663,8 +1668,7 @@ func (am *DefaultAccountManager) getNextPeerExpiration(ctx context.Context, acco
var nextExpiry *time.Duration
for _, peer := range peersWithExpiry {
// consider only connected peers because others will require login on connecting to the management server
if peer.Status.LoginExpired || !peer.Status.Connected {
if peer.Status.LoginExpired {
continue
}
_, duration := peer.LoginExpired(settings.PeerLoginExpiration)
+7 -2
View File
@@ -117,6 +117,7 @@ func (wm *DefaultScheduler) Schedule(ctx context.Context, in time.Duration, ID s
}
ticker := time.NewTicker(in)
period := in
wm.jobs[ID] = cancel
log.WithContext(ctx).Debugf("scheduled a job %s to run in %s. There are %d total jobs scheduled.", ID, in.String(), len(wm.jobs))
@@ -136,14 +137,18 @@ func (wm *DefaultScheduler) Schedule(ctx context.Context, in time.Duration, ID s
if !reschedule {
wm.mu.Lock()
defer wm.mu.Unlock()
delete(wm.jobs, ID)
// A Cancel during job() may have registered a replacement under this ID.
if current, ok := wm.jobs[ID]; ok && current == cancel {
delete(wm.jobs, ID)
}
log.WithContext(ctx).Debugf("job %s is not scheduled to run again", ID)
ticker.Stop()
return
}
// we need this comparison to avoid resetting the ticker with the same duration and missing the current elapsesed time
if runIn != in {
if runIn != period {
ticker.Reset(runIn)
period = runIn
}
case <-cancel:
log.WithContext(ctx).Debugf("job %s was canceled, stopping timer", ID)
+89
View File
@@ -6,10 +6,12 @@ import (
"math/rand"
"runtime"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestScheduler_Performance(t *testing.T) {
@@ -150,3 +152,90 @@ func TestScheduler_Schedule(t *testing.T) {
scheduler.cancel(context.Background(), jobID)
}
func TestScheduler_Schedule_ResetsTickerAfterReturningInitialInterval(t *testing.T) {
jobID := "test-scheduler-job-2"
scheduler := NewDefaultScheduler()
defer scheduler.Cancel(context.Background(), []string{jobID})
initial := 30 * time.Millisecond
stretched := 400 * time.Millisecond
runs := make(chan time.Time, 3)
count := 0
// The first run stretches the period; the second returns the initial interval again,
// which must shrink the period back instead of keeping the stretched one.
job := func() (nextRunIn time.Duration, reschedule bool) {
count++
runs <- time.Now()
switch count {
case 1:
return stretched, true
case 2:
return initial, true
default:
return 0, false
}
}
scheduler.Schedule(context.Background(), initial, jobID, job)
var stamps []time.Time
for len(stamps) < 3 {
select {
case ts := <-runs:
stamps = append(stamps, ts)
case <-time.After(2 * time.Second):
t.Fatalf("timed out after %d runs", len(stamps))
}
}
assert.Less(t, stamps[2].Sub(stamps[1]), stretched/2, "returning the initial interval must reset the stretched ticker")
}
func TestScheduler_Schedule_StaleCompletionKeepsReplacement(t *testing.T) {
jobID := "test-scheduler-job-3"
scheduler := NewDefaultScheduler()
defer scheduler.Cancel(context.Background(), []string{jobID})
started := make(chan struct{})
release := make(chan struct{})
staleJob := func() (nextRunIn time.Duration, reschedule bool) {
close(started)
<-release
return 0, false
}
scheduler.Schedule(context.Background(), 10*time.Millisecond, jobID, staleJob)
select {
case <-started:
case <-time.After(time.Second):
t.Fatal("timed out waiting for the first job to start")
}
// Cancel the job while it is still executing and register a replacement under the
// same ID, as the expiration paths do on a settings change.
scheduler.Cancel(context.Background(), []string{jobID})
var replacementRuns atomic.Int32
scheduler.Schedule(context.Background(), 20*time.Millisecond, jobID, func() (nextRunIn time.Duration, reschedule bool) {
replacementRuns.Add(1)
return 20 * time.Millisecond, true
})
require.True(t, scheduler.IsSchedulerRunning(jobID), "replacement must be registered")
// The stale job now completes without rescheduling; its cleanup must leave the
// replacement's entry in place.
close(release)
assert.Never(t, func() bool { return !scheduler.IsSchedulerRunning(jobID) }, 200*time.Millisecond, 10*time.Millisecond,
"stale completion must not drop the replacement job")
var duplicateRuns atomic.Int32
scheduler.Schedule(context.Background(), 10*time.Millisecond, jobID, func() (nextRunIn time.Duration, reschedule bool) {
duplicateRuns.Add(1)
return 10 * time.Millisecond, true
})
assert.Never(t, func() bool { return duplicateRuns.Load() > 0 }, 100*time.Millisecond, 10*time.Millisecond,
"a duplicate schedule must be refused while the replacement is registered")
scheduler.Cancel(context.Background(), []string{jobID})
assert.False(t, scheduler.IsSchedulerRunning(jobID), "cancel must find and remove the replacement")
runsAfterCancel := replacementRuns.Load()
assert.Never(t, func() bool { return replacementRuns.Load() > runsAfterCancel+1 }, 150*time.Millisecond, 10*time.Millisecond,
"the replacement must stop after cancel")
}
+114 -28
View File
@@ -3160,7 +3160,12 @@ func NewMysqlStore(ctx context.Context, dsn string, metrics telemetry.AppMetrics
return nil, err
}
return NewSqlStore(ctx, db, types.MysqlStoreEngine, metrics, skipMigration)
store, err := NewSqlStore(ctx, db, types.MysqlStoreEngine, metrics, skipMigration)
if err != nil {
closeGormDB(db)
return nil, err
}
return store, nil
}
func getGormConfig() *gorm.Config {
@@ -3219,23 +3224,20 @@ func NewSqliteStoreFromFileStore(ctx context.Context, fileStore *FileStore, data
// NewPostgresqlStoreFromSqlStore restores a store from SqlStore and stores Postgres DB.
func NewPostgresqlStoreFromSqlStore(ctx context.Context, sqliteStore *SqlStore, dsn string, metrics telemetry.AppMetrics) (*SqlStore, error) {
store, err := NewPostgresqlStoreForTests(ctx, dsn, metrics, false)
return newPostgresqlStoreFromSqlStore(ctx, sqliteStore, dsn, metrics, false)
}
func newPostgresqlStoreFromSqlStore(ctx context.Context, sqliteStore *SqlStore, dsn string, metrics telemetry.AppMetrics, skipMigration bool) (*SqlStore, error) {
store, err := NewPostgresqlStoreForTests(ctx, dsn, metrics, skipMigration)
if err != nil {
return nil, err
}
err = store.SaveInstallationID(ctx, sqliteStore.GetInstallationID())
if err != nil {
if err := seedFromSqliteStore(ctx, store, sqliteStore); err != nil {
closeStore(ctx, store)
return nil, err
}
for _, account := range sqliteStore.GetAllAccounts(ctx) {
err := store.SaveAccount(ctx, account)
if err != nil {
return nil, err
}
}
return store, nil
}
@@ -3247,11 +3249,14 @@ func NewPostgresqlStoreForTests(ctx context.Context, dsn string, metrics telemet
}
pool, err := connectToPgDbForTests(context.Background(), dsn)
if err != nil {
closeGormDB(db)
return nil, err
}
store, err := NewSqlStore(ctx, db, types.PostgresStoreEngine, metrics, skipMigration)
if err != nil {
// Release the sessions, or the caller cannot drop the database.
pool.Close()
closeGormDB(db)
return nil, err
}
store.pool = pool
@@ -3285,22 +3290,42 @@ func connectToPgDbForTests(ctx context.Context, dsn string) (*pgxpool.Pool, erro
// NewMysqlStoreFromSqlStore restores a store from SqlStore and stores MySQL DB.
func NewMysqlStoreFromSqlStore(ctx context.Context, sqliteStore *SqlStore, dsn string, metrics telemetry.AppMetrics) (*SqlStore, error) {
store, err := NewMysqlStore(ctx, dsn, metrics, false)
if err != nil {
return nil, err
}
return newMysqlStoreFromSqlStore(ctx, sqliteStore, dsn, metrics, false)
}
err = store.SaveInstallationID(ctx, sqliteStore.GetInstallationID())
if err != nil {
return nil, err
// seedFromSqliteStore copies the installation ID and the accounts of the
// sqlite seed store into a freshly created engine store.
func seedFromSqliteStore(ctx context.Context, store, sqliteStore *SqlStore) error {
if err := store.SaveInstallationID(ctx, sqliteStore.GetInstallationID()); err != nil {
return err
}
for _, account := range sqliteStore.GetAllAccounts(ctx) {
err := store.SaveAccount(ctx, account)
if err != nil {
return nil, err
if err := store.SaveAccount(ctx, account); err != nil {
return err
}
}
return nil
}
// closeStore releases a store that is not handed to the caller, so a failed
// seed does not leak its connection and pool.
func closeStore(ctx context.Context, store *SqlStore) {
store.Close(ctx)
if store.pool != nil {
store.pool.Close()
}
}
func newMysqlStoreFromSqlStore(ctx context.Context, sqliteStore *SqlStore, dsn string, metrics telemetry.AppMetrics, skipMigration bool) (*SqlStore, error) {
store, err := NewMysqlStore(ctx, dsn, metrics, skipMigration)
if err != nil {
return nil, err
}
if err := seedFromSqliteStore(ctx, store, sqliteStore); err != nil {
closeStore(ctx, store)
return nil, err
}
return store, nil
}
@@ -3479,7 +3504,7 @@ func (s *SqlStore) GetPeerGroups(ctx context.Context, lockStrength LockingStreng
var groups []*types.Group
query := tx.
Joins("JOIN group_peers ON group_peers.group_id = groups.id").
Where("group_peers.peer_id = ?", peerId).
Where("groups.account_id = ? AND group_peers.peer_id = ?", accountId, peerId).
Preload(clause.Associations).
Find(&groups)
@@ -5059,7 +5084,7 @@ func (s *SqlStore) GetPeersByGroupIDs(ctx context.Context, accountID string, gro
Select("DISTINCT peer_id").
Where("account_id = ? AND group_id IN ?", accountID, groupIDs)
result := s.db.Where("id IN (?)", peerIDsSubquery).Find(&peers)
result := s.db.Where("account_id = ? AND id IN (?)", accountID, peerIDsSubquery).Find(&peers)
if result.Error != nil {
log.WithContext(ctx).Errorf("failed to get peers by group IDs: %s", result.Error)
return nil, status.Errorf(status.Internal, "failed to get peers by group IDs")
@@ -5692,6 +5717,23 @@ func (s *SqlStore) ListCustomDomains(ctx context.Context, accountID string) ([]*
return domains, nil
}
// GetCustomDomainByName returns the custom domain row holding the given name,
// regardless of which account owns it.
func (s *SqlStore) GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error) {
customDomain := &domain.Domain{}
result := s.db.Take(customDomain, "domain = ?", domainName)
if result.Error != nil {
if errors.Is(result.Error, gorm.ErrRecordNotFound) {
return nil, status.Errorf(status.NotFound, "custom domain %s not found", domainName)
}
log.WithContext(ctx).Errorf("failed to get custom domain by name from store: %v", result.Error)
return nil, status.Errorf(status.Internal, "failed to get custom domain from store")
}
return customDomain, nil
}
func (s *SqlStore) CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error) {
newDomain := &domain.Domain{
ID: xid.New().String(), // Generate our own ID because gorm doesn't always configure the database to handle this for us.
@@ -5701,8 +5743,24 @@ func (s *SqlStore) CreateCustomDomain(ctx context.Context, accountID string, dom
Type: domain.TypeCustom,
Validated: validated,
}
if !validated {
expiresAt := time.Now().UTC().Add(domain.ValidationTTL)
newDomain.ValidationExpiresAt = &expiresAt
}
result := s.db.Create(newDomain)
if result.Error != nil {
// The unique index is the last guard when two requests clear the
// manager's availability check at the same time. The one that loses the
// insert is a conflict, not an internal failure.
var count int64
if err := s.db.Model(&domain.Domain{}).Where("domain = ?", domainName).Count(&count).Error; err == nil && count > 0 {
// The insert error is logged even on this path: the name being taken
// is what the caller has to act on, but if the insert also failed for
// an unrelated reason the operator still needs to see it.
log.WithContext(ctx).Warnf("create reverse proxy custom domain %s rejected, name already registered: %v", domainName, result.Error)
return nil, status.Errorf(status.AlreadyExists, "domain %s is already registered", domainName)
}
log.WithContext(ctx).Errorf("failed to create reverse proxy custom domain to store: %v", result.Error)
return nil, status.Errorf(status.Internal, "failed to create reverse proxy custom domain to store")
}
@@ -5710,12 +5768,21 @@ func (s *SqlStore) CreateCustomDomain(ctx context.Context, accountID string, dom
return newDomain, nil
}
// UpdateCustomDomain completes validation only while the original registration is pending.
func (s *SqlStore) UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error) {
d.AccountID = accountID
result := s.db.Select("*").Save(d)
if !d.Validated {
return nil, status.Errorf(status.InvalidArgument, "custom domain update must complete validation")
}
result := s.db.WithContext(ctx).Model(&domain.Domain{}).
Where(accountAndIDQueryCondition, accountID, d.ID).
Where("domain = ? AND target_cluster = ?", d.Domain, d.TargetCluster).
Where("validated = ? AND validation_expires_at > ?", false, time.Now().UTC()).
Update("validated", true)
if result.Error != nil {
log.WithContext(ctx).Errorf("failed to update reverse proxy custom domain to store: %v", result.Error)
return nil, status.Errorf(status.Internal, "failed to update reverse proxy custom domain to store")
return nil, fmt.Errorf("validate custom domain in store: %w", result.Error)
}
if result.RowsAffected == 0 {
return nil, status.Errorf(status.PreconditionFailed, "custom domain registration is no longer pending validation")
}
return d, nil
@@ -6410,6 +6477,25 @@ func (s *SqlStore) IsClusterAddressConflicting(ctx context.Context, clusterAddre
return count > 0, nil
}
// HasForeignAccountProxyAtHost reports whether a proxy owned by a different
// account declares this host. Shared proxies (account_id IS NULL) are not
// foreign: a shared cluster is what most accounts pin their agent network
// gateway to. The match folds case because proxies declare their address as
// the operator spelled it while the caller's host is normalised; that costs a
// scan of the proxies table, taken once per account when its gateway is
// bootstrapped, not on the per-connect path IsClusterAddressConflicting serves.
func (s *SqlStore) HasForeignAccountProxyAtHost(ctx context.Context, host, accountID string) (bool, error) {
var count int64
result := s.db.
Model(&proxy.Proxy{}).
Where("LOWER(cluster_address) = LOWER(?) AND account_id IS NOT NULL AND account_id != ?", host, accountID).
Count(&count)
if result.Error != nil {
return false, status.Errorf(status.Internal, "check proxy host ownership: %v", result.Error)
}
return count > 0, nil
}
func (s *SqlStore) DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error {
result := s.db.
Where("cluster_address = ? AND account_id = ?", clusterAddress, accountID).
@@ -315,6 +315,36 @@ func (s *SqlStore) GetAllAgentNetworkSettings(ctx context.Context, lockStrength
return settings, nil
}
// HasGatewayClusterPinnedByOtherAccount reports whether another account has a
// labeled agent network gateway pinned beneath host, making host its cluster.
// A self-addressed endpoint on the very same hostname is not counted: that
// collision is the domain unique index's to refuse, as a conflict. Case-folded,
// since a settings row written before hostnames were normalised may carry
// capitals; one row per account, so the scan is cheap.
func (s *SqlStore) HasGatewayClusterPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) {
return s.countGatewayRowsByOtherAccount(ctx, "LOWER(proxy_address) = LOWER(?) AND LOWER(domain) <> LOWER(proxy_address)", host, accountID)
}
// HasGatewayEndpointByOtherAccount reports whether host is another account's
// agent network endpoint hostname (domain). Case-folded for the same reason as
// HasGatewayClusterPinnedByOtherAccount.
func (s *SqlStore) HasGatewayEndpointByOtherAccount(ctx context.Context, host, accountID string) (bool, error) {
return s.countGatewayRowsByOtherAccount(ctx, "LOWER(domain) = LOWER(?)", host, accountID)
}
func (s *SqlStore) countGatewayRowsByOtherAccount(ctx context.Context, predicate, host, accountID string) (bool, error) {
var count int64
result := s.db.
Model(&agentNetworkTypes.Settings{}).
Where(predicate+" AND account_id != ?", host, accountID).
Count(&count)
if result.Error != nil {
log.WithContext(ctx).Errorf("failed to check agent network gateway claims at host: %v", result.Error)
return false, status.Errorf(status.Internal, "check agent network gateway claims at host")
}
return count > 0, nil
}
// GetAgentNetworkSettingsByProxyAddress returns every Settings row whose
// gateway is served by the proxy declaring the given cluster address. Used by
// cluster-scoped synthesis to find the accounts a shared proxy serves.
@@ -0,0 +1,60 @@
package store
import (
"context"
"fmt"
"strings"
"time"
"gorm.io/gorm"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
"github.com/netbirdio/netbird/shared/management/status"
)
// GetExpiredCustomDomains lists pending registrations in stable batches across accounts.
func (s *SqlStore) GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error) {
var domains []*domain.Domain
result := s.db.WithContext(ctx).
Where("validated = ? AND validation_expires_at <= ? AND id > ?", false, now, string(afterID)).
Order("id").Limit(limit).Find(&domains)
if result.Error != nil {
return nil, fmt.Errorf("list expired custom domains: %w", result.Error)
}
return domains, nil
}
// DeleteExpiredCustomDomain deletes an expired registration only if no service uses its namespace.
func (s *SqlStore) DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error) {
db := s.db.WithContext(ctx)
services := customDomainServices(db, d)
result := db.Where(accountAndIDQueryCondition, d.AccountID, d.ID).
Where("domain = ? AND validated = ? AND validation_expires_at <= ?", d.Domain, false, now).
Where("NOT EXISTS (?)", services.Select("1")).Delete(&domain.Domain{})
if result.Error != nil {
return false, fmt.Errorf("delete expired custom domain: %w", result.Error)
}
if result.RowsAffected > 0 {
return true, nil
}
var count int64
if err := customDomainServices(db, d).Count(&count).Error; err != nil {
return false, fmt.Errorf("check expired custom domain services: %w", err)
}
if count > 0 {
return false, status.Errorf(status.PreconditionFailed, "expired custom domain still has dependent services")
}
return false, nil
}
func customDomainServices(db *gorm.DB, d *domain.Domain) *gorm.DB {
name := strings.ToLower(strings.TrimSuffix(d.Domain, "."))
// Shared domain validation permits underscores, and older rows may contain
// other LIKE metacharacters.
escaped := strings.NewReplacer("!", "!!", "%", "!%", "_", "!_").Replace(name)
return db.Model(&rpservice.Service{}).Where(
"LOWER(domain) IN ? OR LOWER(domain) LIKE ? ESCAPE '!' OR LOWER(domain) LIKE ? ESCAPE '!'",
[]string{name, name + "."}, "%."+escaped, "%."+escaped+".",
)
}
@@ -0,0 +1,78 @@
package store
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/domain"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
)
func TestDeleteExpiredCustomDomain_ServiceDependencies(t *testing.T) {
runTestForAllEngines(t, "", func(t *testing.T, store Store) {
ctx := context.Background()
now := time.Now().UTC()
db := store.(*SqlStore).db
require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
for _, tt := range []struct {
name string
domainName string
serviceHost string
protected bool
}{
{"exact", "example.com", "example.com", true},
{"subdomain", "example.com", "deep.app.example.com", true},
{"case", "example.com", "APP.EXAMPLE.COM.", true},
{"suffix-boundary", "example.com", "notexample.com", false},
{"literal underscore", "a_b.example.com", "app.a_b.example.com", true},
{"underscore wildcard", "a_b.example.com", "app.axb.example.com", false},
{"legacy percent wildcard", "a%b.example.com", "app.axxb.example.com", false},
{"legacy escape character", "a!b.example.com", "app.ab.example.com", false},
} {
t.Run(tt.name, func(t *testing.T) {
d, err := store.CreateCustomDomain(ctx, "owner", tt.domainName, "cluster", false)
require.NoError(t, err)
require.NoError(t, db.Model(d).Update("validation_expires_at", now.Add(-time.Hour)).Error)
svc := &rpservice.Service{ID: "legacy", AccountID: "owner", Domain: tt.serviceHost}
require.NoError(t, store.CreateService(ctx, svc))
deleted, err := store.DeleteExpiredCustomDomain(ctx, d, now)
if tt.protected {
require.Error(t, err)
assert.False(t, deleted, "service namespaces must remain reserved")
} else {
require.NoError(t, err)
assert.True(t, deleted, "a hostname outside the namespace must not prevent cleanup")
}
require.NoError(t, db.Delete(svc).Error)
require.NoError(t, db.Delete(d).Error)
})
}
})
}
func TestDeleteExpiredCustomDomain_RechecksValidation(t *testing.T) {
runTestForAllEngines(t, "", func(t *testing.T, store Store) {
ctx := context.Background()
require.NoError(t, store.SaveAccount(ctx, newAccountWithId(ctx, "owner", "admin", "")))
d, err := store.CreateCustomDomain(ctx, "owner", "validated.example.com", "cluster", false)
require.NoError(t, err)
d, err = store.GetCustomDomain(ctx, "owner", d.ID)
require.NoError(t, err)
stale := d.Copy()
d.Validated = true
_, err = store.UpdateCustomDomain(ctx, "owner", d)
require.NoError(t, err)
deleted, err := store.DeleteExpiredCustomDomain(ctx, stale, time.Now().Add(domain.ValidationTTL))
require.NoError(t, err)
assert.False(t, deleted, "a stale cleanup candidate must not delete a validated registration")
stored, err := store.GetCustomDomain(ctx, "owner", d.ID)
require.NoError(t, err)
assert.True(t, stored.Validated, "the validated registration must remain usable")
require.NotNil(t, stored.ValidationExpiresAt)
assert.Equal(t, stale.ValidationExpiresAt, stored.ValidationExpiresAt, "validation must preserve the original deadline")
})
}
@@ -64,7 +64,7 @@ func assertGetAccountLoadsCustomDomains(t *testing.T, store Store) {
_, err := store.CreateCustomDomain(ctx, accountID, "example.com", "eu.proxy.netbird.io", true)
require.NoError(t, err, "creating the first custom domain must succeed")
_, err = store.CreateCustomDomain(ctx, accountID, "apps.acme.io", "us.proxy.netbird.io", false)
pending, err := store.CreateCustomDomain(ctx, accountID, "apps.acme.io", "us.proxy.netbird.io", false)
require.NoError(t, err, "creating the second custom domain must succeed")
account, err := store.GetAccount(ctx, accountID)
@@ -75,6 +75,10 @@ func assertGetAccountLoadsCustomDomains(t *testing.T, store Store) {
for _, d := range account.Domains {
require.NotNil(t, d)
byDomain[d.Domain] = d.TargetCluster
if d.ID == pending.ID {
require.NotNil(t, d.ValidationExpiresAt)
assert.WithinDuration(t, *pending.ValidationExpiresAt, *d.ValidationExpiresAt, time.Millisecond, "both account loaders must preserve the validation deadline")
}
}
assert.Equal(t, "eu.proxy.netbird.io", byDomain["example.com"], "custom domain must carry its target cluster")
assert.Equal(t, "us.proxy.netbird.io", byDomain["apps.acme.io"], "custom domain must carry its target cluster")
+14
View File
@@ -2847,6 +2847,14 @@ func TestSqlStore_GetPeerGroups(t *testing.T) {
groups, err = store.GetPeerGroups(context.Background(), LockingStrengthNone, accountID, peerID)
require.NoError(t, err)
assert.Len(t, groups, 2)
foreignPeerID := "foreign-peer"
err = store.AddPeerToGroup(context.Background(), accountID, foreignPeerID, "cfefqs706sqkneg59g4h")
require.NoError(t, err)
groups, err = store.GetPeerGroups(context.Background(), LockingStrengthNone, "other-account", foreignPeerID)
require.NoError(t, err)
assert.Empty(t, groups, "groups of another account must not be returned")
}
func TestSqlStore_GetAccountPeers(t *testing.T) {
@@ -4042,9 +4050,15 @@ func TestSqlStore_GetPeersByGroupIDs(t *testing.T) {
}
require.NoError(t, store.CreateGroups(ctx, accountID, groups))
otherAccount := newAccountWithId(ctx, "other-account", "other-user", "")
require.NoError(t, store.SaveAccount(ctx, otherAccount))
foreignPeer := &nbpeer.Peer{ID: "foreign-peer", AccountID: otherAccount.Id}
require.NoError(t, store.AddPeerToAccount(ctx, foreignPeer))
require.NoError(t, store.AddPeerToGroup(ctx, accountID, peer1, group1ID))
require.NoError(t, store.AddPeerToGroup(ctx, accountID, peer2, group1ID))
require.NoError(t, store.AddPeerToGroup(ctx, accountID, peer1, group2ID))
require.NoError(t, store.AddPeerToGroup(ctx, accountID, foreignPeer.ID, group1ID))
peers, err := store.GetPeersByGroupIDs(ctx, accountID, tt.groupIDs)
require.NoError(t, err)
+248 -14
View File
@@ -4,6 +4,7 @@ package store
import (
"context"
"database/sql"
"errors"
"fmt"
"net"
@@ -15,6 +16,7 @@ import (
"runtime"
"slices"
"strings"
"sync"
"time"
"github.com/google/uuid"
@@ -302,8 +304,11 @@ type Store interface {
GetCustomDomain(ctx context.Context, accountID string, domainID string) (*domain.Domain, error)
ListFreeDomains(ctx context.Context, accountID string) ([]string, error)
ListCustomDomains(ctx context.Context, accountID string) ([]*domain.Domain, error)
GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error)
CreateCustomDomain(ctx context.Context, accountID string, domainName string, targetCluster string, validated bool) (*domain.Domain, error)
UpdateCustomDomain(ctx context.Context, accountID string, d *domain.Domain) (*domain.Domain, error)
GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error)
DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error)
DeleteCustomDomain(ctx context.Context, accountID string, domainID string) error
CreateAccessLog(ctx context.Context, log *accesslogs.AccessLogEntry) error
@@ -337,6 +342,9 @@ type Store interface {
CountProxiesByAccountID(ctx context.Context, accountID string) (int64, error)
IsClusterAddressConflicting(ctx context.Context, clusterAddress, accountID string) (bool, error)
HasActiveProxyAtClusterAddress(ctx context.Context, clusterAddress string) (bool, error)
HasForeignAccountProxyAtHost(ctx context.Context, host, accountID string) (bool, error)
HasGatewayClusterPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error)
HasGatewayEndpointByOtherAccount(ctx context.Context, host, accountID string) (bool, error)
DeleteAccountCluster(ctx context.Context, clusterAddress, accountID string) error
GetCustomDomainsCounts(ctx context.Context) (total int64, validated int64, err error)
@@ -641,6 +649,9 @@ func migratePostAuto(ctx context.Context, db *gorm.DB) error {
func getMigrationsPostAuto(ctx context.Context) []migrationFunc {
return []migrationFunc{
func(db *gorm.DB) error {
return migration.MigrateCustomDomainValidationExpiry(ctx, db)
},
func(db *gorm.DB) error {
return migration.CreateIndexIfNotExists[nbpeer.Peer](ctx, db, "idx_account_ip", "account_id", "ip")
},
@@ -726,6 +737,7 @@ func NewTestStoreFromSQL(ctx context.Context, filename string, dataDir string) (
time.Sleep(100 * time.Millisecond)
}
}
store.Close(ctx)
return nil, nil, fmt.Errorf("failed to create test store after %d attempts: %v", maxRetries, err)
}
@@ -752,14 +764,15 @@ func addAllGroupToAccount(ctx context.Context, store Store) error {
return nil
}
func getSqlStoreEngine(ctx context.Context, store *SqlStore, kind types.Engine) (Store, func(), error) {
func getSqlStoreEngine(ctx context.Context, sqliteStore *SqlStore, kind types.Engine) (Store, func(), error) {
store := sqliteStore
var cleanup func()
var err error
switch kind {
case types.PostgresStoreEngine:
store, cleanup, err = newReusedPostgresStore(ctx, store, kind)
store, cleanup, err = newReusedPostgresStore(ctx, sqliteStore, kind)
case types.MysqlStoreEngine:
store, cleanup, err = newReusedMysqlStore(ctx, store, kind)
store, cleanup, err = newReusedMysqlStore(ctx, sqliteStore, kind)
default:
cleanup = func() {
// sqlite doesn't need to be cleaned up
@@ -775,6 +788,11 @@ func getSqlStoreEngine(ctx context.Context, store *SqlStore, kind types.Engine)
if store.pool != nil {
store.pool.Close()
}
if store != sqliteStore {
// The sqlite store only seeded the engine under test; without this
// every test leaks its connection and the opener goroutines.
sqliteStore.Close(ctx)
}
}
return store, closeConnection, nil
@@ -799,19 +817,23 @@ func newReusedPostgresStore(ctx context.Context, store *SqlStore, kind types.Eng
return nil, nil, fmt.Errorf("failed to open postgres connection: %v", err)
}
dsn, cleanup, err := createRandomDB(dsn, db, kind)
sqlDB, _ := db.DB()
if sqlDB != nil {
sqlDB.Close()
template, err := postgresSchemaTemplate(ctx, dsn, db)
if err != nil {
closeGormDB(db)
return nil, nil, err
}
dsn, cleanup, err := createRandomDB(dsn, db, kind, template)
closeGormDB(db)
if err != nil {
return nil, nil, err
}
store, err = NewPostgresqlStoreFromSqlStore(ctx, store, dsn, nil)
store, err = newPostgresqlStoreFromSqlStore(ctx, store, dsn, nil, true)
if err != nil {
cleanup()
return nil, nil, err
}
@@ -844,7 +866,13 @@ func newReusedMysqlStore(ctx context.Context, store *SqlStore, kind types.Engine
sqlDB.SetMaxOpenConns(1)
sqlDB.SetMaxIdleConns(1)
dsn, cleanup, err := createRandomDB(dsn, db, kind)
tableDDL, err := mysqlSchemaTemplate(ctx, dsn, db)
if err != nil {
sqlDB.Close()
return nil, nil, err
}
dsn, cleanup, err := createRandomDB(dsn, db, kind, "")
sqlDB.Close()
@@ -852,14 +880,200 @@ func newReusedMysqlStore(ctx context.Context, store *SqlStore, kind types.Engine
return nil, nil, err
}
store, err = NewMysqlStoreFromSqlStore(ctx, store, dsn, nil)
if err := cloneMysqlSchema(ctx, dsn, tableDDL); err != nil {
cleanup()
return nil, nil, err
}
store, err = newMysqlStoreFromSqlStore(ctx, store, dsn, nil, true)
if err != nil {
cleanup()
return nil, nil, err
}
return store, cleanup, nil
}
// schemaTemplates remembers, per engine and server, a database that went
// through the full migration once in this process. Every later test database
// is cloned from it, so a test pays for CREATE DATABASE and a schema copy
// instead of the 40-table AutoMigrate plus every pre and post migration, which
// is what made each MySQL test store cost well over a second in CI.
var (
schemaTemplatesMu sync.Mutex
schemaTemplates = map[string]*schemaTemplate{}
)
type schemaTemplate struct {
dbName string
// tableDDL holds the CREATE TABLE statements of the template. MySQL has no
// server-side database template, so the schema is replayed statement by
// statement into each test database.
tableDDL []string
}
func schemaTemplateKey(engine types.Engine, dsn string) string {
return string(engine) + "|" + dsn
}
func newTestDBName(prefix string) string {
return fmt.Sprintf("%s_%s", prefix, strings.ReplaceAll(uuid.New().String(), "-", "_"))
}
// postgresSchemaTemplate returns the name of a fully migrated database that
// CREATE DATABASE ... TEMPLATE can copy, creating it on first use.
func postgresSchemaTemplate(ctx context.Context, baseDSN string, admin *gorm.DB) (string, error) {
schemaTemplatesMu.Lock()
defer schemaTemplatesMu.Unlock()
key := schemaTemplateKey(types.PostgresStoreEngine, baseDSN)
if tpl, ok := schemaTemplates[key]; ok {
return tpl.dbName, nil
}
name := newTestDBName("test_template")
if err := admin.Exec(fmt.Sprintf("CREATE DATABASE %s", name)).Error; err != nil {
return "", fmt.Errorf("create postgres template database: %w", err)
}
tplStore, err := NewPostgresqlStoreForTests(ctx, replaceDBName(baseDSN, name), nil, false)
if err != nil {
dropDatabase(admin, name)
return "", fmt.Errorf("migrate postgres template database: %w", err)
}
// TEMPLATE refuses a source that still has sessions, so release both handles
// before the first clone.
tplStore.Close(ctx)
if tplStore.pool != nil {
tplStore.pool.Close()
}
schemaTemplates[key] = &schemaTemplate{dbName: name}
return name, nil
}
// mysqlSchemaTemplate returns the CREATE TABLE statements of a fully migrated
// database, migrating one on first use.
func mysqlSchemaTemplate(ctx context.Context, baseDSN string, admin *gorm.DB) ([]string, error) {
schemaTemplatesMu.Lock()
defer schemaTemplatesMu.Unlock()
key := schemaTemplateKey(types.MysqlStoreEngine, baseDSN)
if tpl, ok := schemaTemplates[key]; ok {
return tpl.tableDDL, nil
}
name := newTestDBName("test_template")
if err := admin.Exec(fmt.Sprintf("CREATE DATABASE %s", name)).Error; err != nil {
return nil, fmt.Errorf("create mysql template database: %w", err)
}
tplStore, err := NewMysqlStore(ctx, replaceDBName(baseDSN, name), nil, false)
if err != nil {
dropDatabase(admin, name)
return nil, fmt.Errorf("migrate mysql template database: %w", err)
}
tableDDL, err := mysqlTableDDL(ctx, tplStore.db, name)
tplStore.Close(ctx)
if err != nil {
dropDatabase(admin, name)
return nil, err
}
schemaTemplates[key] = &schemaTemplate{dbName: name, tableDDL: tableDDL}
return tableDDL, nil
}
func mysqlTableDDL(ctx context.Context, db *gorm.DB, dbName string) ([]string, error) {
sqlDB, err := db.DB()
if err != nil {
return nil, err
}
tables, err := mysqlTableNames(ctx, sqlDB, dbName)
if err != nil {
return nil, err
}
tableDDL := make([]string, 0, len(tables))
for _, table := range tables {
var name, createStmt string
row := sqlDB.QueryRowContext(ctx, fmt.Sprintf("SHOW CREATE TABLE %s.%s", dbName, table))
if err := row.Scan(&name, &createStmt); err != nil {
return nil, fmt.Errorf("read create statement of %s: %w", table, err)
}
tableDDL = append(tableDDL, createStmt)
}
return tableDDL, nil
}
func mysqlTableNames(ctx context.Context, sqlDB *sql.DB, dbName string) ([]string, error) {
rows, err := sqlDB.QueryContext(ctx, fmt.Sprintf("SHOW TABLES FROM %s", dbName))
if err != nil {
return nil, fmt.Errorf("list template tables: %w", err)
}
defer rows.Close()
var tables []string
for rows.Next() {
var table string
if err := rows.Scan(&table); err != nil {
return nil, fmt.Errorf("scan template table name: %w", err)
}
tables = append(tables, table)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("list template tables: %w", err)
}
return tables, nil
}
// cloneMysqlSchema replays the template's CREATE TABLE statements into the
// database the DSN points at.
func cloneMysqlSchema(ctx context.Context, dsn string, tableDDL []string) error {
db, err := gorm.Open(mysql.Open(dsn+"?charset=utf8&parseTime=True&loc=Local"), getGormConfig())
if err != nil {
return fmt.Errorf("connect to test database: %w", err)
}
sqlDB, err := db.DB()
if err != nil {
return err
}
defer sqlDB.Close()
// The statements come out of SHOW TABLES in name order, not dependency
// order, and their foreign keys reference tables of the session's default
// database. Pin a single connection so the session setting below covers
// every statement, and connect straight to the new database so unqualified
// references land there.
sqlDB.SetMaxOpenConns(1)
if _, err := sqlDB.ExecContext(ctx, "SET FOREIGN_KEY_CHECKS = 0"); err != nil {
return fmt.Errorf("disable foreign key checks: %w", err)
}
for _, stmt := range tableDDL {
if _, err := sqlDB.ExecContext(ctx, stmt); err != nil {
return fmt.Errorf("replay table definition: %w", err)
}
}
return nil
}
// dropDatabase removes a template that never became usable, so a failed setup
// does not leave it behind on a shared server. The server may still be tearing
// down the sessions the failed migration held, so the drop retries while
// Postgres reports the database as in use.
func dropDatabase(admin *gorm.DB, name string) {
if err := execWithTemplateRetry(admin, fmt.Sprintf("DROP DATABASE IF EXISTS %s", name)); err != nil {
log.Warnf("failed to drop template database %s: %v", name, err)
}
}
func closeGormDB(db *gorm.DB) {
if sqlDB, _ := db.DB(); sqlDB != nil {
sqlDB.Close()
}
}
func openDBWithRetry(dsn string, engine types.Engine, maxRetries int) (*gorm.DB, error) {
var db *gorm.DB
var err error
@@ -885,10 +1099,16 @@ func openDBWithRetry(dsn string, engine types.Engine, maxRetries int) (*gorm.DB,
return nil, err
}
func createRandomDB(dsn string, db *gorm.DB, engine types.Engine) (string, func(), error) {
dbName := fmt.Sprintf("test_db_%s", strings.ReplaceAll(uuid.New().String(), "-", "_"))
// createRandomDB creates a uniquely named database for one test. On postgres a
// non-empty template is copied server-side with CREATE DATABASE ... TEMPLATE.
func createRandomDB(dsn string, db *gorm.DB, engine types.Engine, template string) (string, func(), error) {
dbName := newTestDBName("test_db")
if err := db.Exec(fmt.Sprintf("CREATE DATABASE %s", dbName)).Error; err != nil {
createStmt := fmt.Sprintf("CREATE DATABASE %s", dbName)
if template != "" && engine == types.PostgresStoreEngine {
createStmt = fmt.Sprintf("CREATE DATABASE %s TEMPLATE %s", dbName, template)
}
if err := execWithTemplateRetry(db, createStmt); err != nil {
return "", nil, fmt.Errorf("failed to create database: %v", err)
}
@@ -954,6 +1174,20 @@ func createRandomDB(dsn string, db *gorm.DB, engine types.Engine) (string, func(
return replaceDBName(dsn, dbName), cleanup, nil
}
// execWithTemplateRetry runs a statement, retrying briefly when postgres still
// sees the template's just-closed sessions and refuses to copy it.
func execWithTemplateRetry(db *gorm.DB, stmt string) error {
var err error
for attempt := 0; attempt < 20; attempt++ {
err = db.Exec(stmt).Error
if err == nil || !strings.Contains(err.Error(), "is being accessed by other users") {
return err
}
time.Sleep(100 * time.Millisecond)
}
return err
}
func replaceDBName(dsn, newDBName string) string {
re := regexp.MustCompile(`(?P<pre>[:/@])(?P<dbname>[^/?]+)(?P<post>\?|$)`)
return re.ReplaceAllString(dsn, `${pre}`+newDBName+`${post}`)
+90
View File
@@ -555,6 +555,21 @@ func (mr *MockStoreMockRecorder) DeleteDNSRecord(ctx, accountID, zoneID, recordI
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteDNSRecord", reflect.TypeOf((*MockStore)(nil).DeleteDNSRecord), ctx, accountID, zoneID, recordID)
}
// DeleteExpiredCustomDomain mocks base method.
func (m *MockStore) DeleteExpiredCustomDomain(ctx context.Context, d *domain.Domain, now time.Time) (bool, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteExpiredCustomDomain", ctx, d, now)
ret0, _ := ret[0].(bool)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// DeleteExpiredCustomDomain indicates an expected call of DeleteExpiredCustomDomain.
func (mr *MockStoreMockRecorder) DeleteExpiredCustomDomain(ctx, d, now any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteExpiredCustomDomain", reflect.TypeOf((*MockStore)(nil).DeleteExpiredCustomDomain), ctx, d, now)
}
// DeleteGroup mocks base method.
func (m *MockStore) DeleteGroup(ctx context.Context, accountID, groupID string) error {
m.ctrl.T.Helper()
@@ -1941,6 +1956,21 @@ func (mr *MockStoreMockRecorder) GetCustomDomain(ctx, accountID, domainID any) *
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCustomDomain", reflect.TypeOf((*MockStore)(nil).GetCustomDomain), ctx, accountID, domainID)
}
// GetCustomDomainByName mocks base method.
func (m *MockStore) GetCustomDomainByName(ctx context.Context, domainName string) (*domain.Domain, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetCustomDomainByName", ctx, domainName)
ret0, _ := ret[0].(*domain.Domain)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetCustomDomainByName indicates an expected call of GetCustomDomainByName.
func (mr *MockStoreMockRecorder) GetCustomDomainByName(ctx, domainName any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCustomDomainByName", reflect.TypeOf((*MockStore)(nil).GetCustomDomainByName), ctx, domainName)
}
// GetCustomDomainsCounts mocks base method.
func (m *MockStore) GetCustomDomainsCounts(ctx context.Context) (int64, int64, error) {
m.ctrl.T.Helper()
@@ -1987,6 +2017,21 @@ func (mr *MockStoreMockRecorder) GetEmbeddedProxyPeerIDsByCluster(ctx, accountID
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEmbeddedProxyPeerIDsByCluster", reflect.TypeOf((*MockStore)(nil).GetEmbeddedProxyPeerIDsByCluster), ctx, accountID)
}
// GetExpiredCustomDomains mocks base method.
func (m *MockStore) GetExpiredCustomDomains(ctx context.Context, now time.Time, afterID domain.ID, limit int) ([]*domain.Domain, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetExpiredCustomDomains", ctx, now, afterID, limit)
ret0, _ := ret[0].([]*domain.Domain)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetExpiredCustomDomains indicates an expected call of GetExpiredCustomDomains.
func (mr *MockStoreMockRecorder) GetExpiredCustomDomains(ctx, now, afterID, limit any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetExpiredCustomDomains", reflect.TypeOf((*MockStore)(nil).GetExpiredCustomDomains), ctx, now, afterID, limit)
}
// GetExpiredEphemeralServices mocks base method.
func (m *MockStore) GetExpiredEphemeralServices(ctx context.Context, ttl time.Duration, limit int) ([]*service.Service, error) {
m.ctrl.T.Helper()
@@ -3020,6 +3065,51 @@ func (mr *MockStoreMockRecorder) HasActiveProxyAtClusterAddress(ctx, clusterAddr
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasActiveProxyAtClusterAddress", reflect.TypeOf((*MockStore)(nil).HasActiveProxyAtClusterAddress), ctx, clusterAddress)
}
// HasForeignAccountProxyAtHost mocks base method.
func (m *MockStore) HasForeignAccountProxyAtHost(ctx context.Context, host, accountID string) (bool, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "HasForeignAccountProxyAtHost", ctx, host, accountID)
ret0, _ := ret[0].(bool)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// HasForeignAccountProxyAtHost indicates an expected call of HasForeignAccountProxyAtHost.
func (mr *MockStoreMockRecorder) HasForeignAccountProxyAtHost(ctx, host, accountID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasForeignAccountProxyAtHost", reflect.TypeOf((*MockStore)(nil).HasForeignAccountProxyAtHost), ctx, host, accountID)
}
// HasGatewayClusterPinnedByOtherAccount mocks base method.
func (m *MockStore) HasGatewayClusterPinnedByOtherAccount(ctx context.Context, host, accountID string) (bool, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "HasGatewayClusterPinnedByOtherAccount", ctx, host, accountID)
ret0, _ := ret[0].(bool)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// HasGatewayClusterPinnedByOtherAccount indicates an expected call of HasGatewayClusterPinnedByOtherAccount.
func (mr *MockStoreMockRecorder) HasGatewayClusterPinnedByOtherAccount(ctx, host, accountID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasGatewayClusterPinnedByOtherAccount", reflect.TypeOf((*MockStore)(nil).HasGatewayClusterPinnedByOtherAccount), ctx, host, accountID)
}
// HasGatewayEndpointByOtherAccount mocks base method.
func (m *MockStore) HasGatewayEndpointByOtherAccount(ctx context.Context, host, accountID string) (bool, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "HasGatewayEndpointByOtherAccount", ctx, host, accountID)
ret0, _ := ret[0].(bool)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// HasGatewayEndpointByOtherAccount indicates an expected call of HasGatewayEndpointByOtherAccount.
func (mr *MockStoreMockRecorder) HasGatewayEndpointByOtherAccount(ctx, host, accountID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasGatewayEndpointByOtherAccount", reflect.TypeOf((*MockStore)(nil).HasGatewayEndpointByOtherAccount), ctx, host, accountID)
}
// IncrementAgentNetworkConsumption mocks base method.
func (m *MockStore) IncrementAgentNetworkConsumption(ctx context.Context, accountID string, kind types.ConsumptionDimension, dimID string, windowSeconds int64, windowStart time.Time, tokensIn, tokensOut int64, costUSD float64) error {
m.ctrl.T.Helper()
+12
View File
@@ -37,6 +37,18 @@ func CreateMysqlTestContainer() (func(), string, error) {
mysql.WithDatabase("testing"),
mysql.WithUsername("root"),
mysql.WithPassword("testing"),
// Every test creates and drops a database with about 40 tables, so with
// the server defaults the run is dominated by durability work: each
// CREATE TABLE fsyncs the redo log, the binary log and the doublewrite
// buffer. None of it protects anything in a container that is discarded
// after the run. Tables stay in per-table files on purpose: in the shared
// system tablespace the cost of every CREATE and DROP grew with the number
// of databases the run had already created.
testcontainers.WithCmd("mysqld",
"--innodb-flush-log-at-trx-commit=0",
"--innodb-doublewrite=OFF",
"--skip-log-bin",
),
testcontainers.WithWaitStrategy(
wait.ForLog("/usr/sbin/mysqld: ready for connections").
WithOccurrence(1).WithStartupTimeout(15*time.Second).WithPollInterval(100*time.Millisecond),
+2 -3
View File
@@ -404,7 +404,7 @@ func (a *Account) GetExpiredPeers() []*nbpeer.Peer {
// GetNextPeerExpiration returns the minimum duration in which the next peer of the account will expire if it was found.
// If there is no peer that expires this function returns false and a duration of 0.
// This function only considers peers that haven't been expired yet and that are connected.
// This function only considers peers that haven't been expired yet, whether connected or not.
func (a *Account) GetNextPeerExpiration() (time.Duration, bool) {
peersWithExpiry := a.GetPeersWithExpiration()
if len(peersWithExpiry) == 0 {
@@ -412,8 +412,7 @@ func (a *Account) GetNextPeerExpiration() (time.Duration, bool) {
}
var nextExpiry *time.Duration
for _, peer := range peersWithExpiry {
// consider only connected peers because others will require login on connecting to the management server
if peer.Status.LoginExpired || !peer.Status.Connected {
if peer.Status.LoginExpired {
continue
}
_, duration := peer.LoginExpired(a.Settings.PeerLoginExpiration)
+11 -1
View File
@@ -3,6 +3,7 @@ package types
import (
"errors"
"net/url"
"strings"
)
// Identity provider validation errors
@@ -99,7 +100,16 @@ func (idp *IdentityProvider) Validate() error {
}
if idp.Issuer != "" {
parsedURL, err := url.Parse(idp.Issuer)
if err != nil || parsedURL.Scheme == "" || parsedURL.Host == "" {
if err != nil || parsedURL.Host == "" {
return ErrIdentityProviderIssuerInvalid
}
if parsedURL.Scheme != "https" {
return ErrIdentityProviderIssuerInvalid
}
if parsedURL.User != nil {
return ErrIdentityProviderIssuerInvalid
}
if strings.ContainsAny(idp.Issuer, "?#") {
return ErrIdentityProviderIssuerInvalid
}
}
@@ -135,3 +135,54 @@ func TestIdentityProvider_Validate(t *testing.T) {
})
}
}
func TestIdentityProvider_ValidateRejectsNonOriginIssuers(t *testing.T) {
issuers := []string{
"https://idp.example.com/realms/nb?foo=bar",
"https://idp.example.com/realms/nb#section",
"https://user:pass@idp.example.com",
"ftp://idp.example.com",
"ldap://idp.example.com",
"http://idp.example.com",
}
for _, issuer := range issuers {
t.Run(issuer, func(t *testing.T) {
idp := &IdentityProvider{
Name: "test",
Type: IdentityProviderTypeOIDC,
Issuer: issuer,
ClientID: "client-id",
}
assert.ErrorIs(t, idp.Validate(), ErrIdentityProviderIssuerInvalid)
})
}
}
func TestIdentityProvider_ValidateAcceptsOriginAndPath(t *testing.T) {
for _, issuer := range []string{"https://idp.example.com", "https://idp.example.com/realms/nb", "https://127.0.0.1:5556/dex"} {
t.Run(issuer, func(t *testing.T) {
idp := &IdentityProvider{
Name: "test",
Type: IdentityProviderTypeOIDC,
Issuer: issuer,
ClientID: "client-id",
}
assert.NoError(t, idp.Validate())
})
}
}
func TestIdentityProviderValidateRejectsBareDelimiters(t *testing.T) {
for _, issuer := range []string{"https://idp.example.com/realms/nb?", "https://idp.example.com/realms/nb#"} {
t.Run(issuer, func(t *testing.T) {
idp := &IdentityProvider{
Name: "test",
Type: IdentityProviderTypeOIDC,
Issuer: issuer,
ClientID: "client-id",
}
assert.ErrorIs(t, idp.Validate(), ErrIdentityProviderIssuerInvalid)
})
}
}
+19
View File
@@ -285,6 +285,25 @@ func (u *User) EncryptSensitiveData(enc *crypt.FieldEncrypt) error {
return nil
}
func MaskEmail(email string) string {
local, domain, found := strings.Cut(email, "@")
if !found || local == "" || domain == "" {
return ""
}
// Runes, not bytes, so a non-ASCII local part is not cut mid-character.
runes := []rune(local)
// Keeping the first two and the last needs a local part of at least four to
// hide anything at all: at three or fewer those are the whole of it, and the
// address would be recoverable in full from what is meant to conceal it.
if len(runes) < 4 {
return "****@" + domain
}
return string(runes[:2]) + "****" + string(runes[len(runes)-1]) + "@" + domain
}
// DecryptSensitiveData decrypts the user's sensitive fields (Email and Name) in place.
func (u *User) DecryptSensitiveData(enc *crypt.FieldEncrypt) error {
if enc == nil {
+141
View File
@@ -296,3 +296,144 @@ func TestUser_EncryptDecryptRoundTrip(t *testing.T) {
})
}
}
func TestMaskEmail(t *testing.T) {
testCases := []struct {
name string
email string
expected string
}{
{
name: "ordinary address keeps the first two, the last, and the domain",
email: "admin@example.com",
expected: "ad****n@example.com",
},
{
name: "four characters is the shortest local part that reveals anything",
email: "abcd@example.com",
expected: "ab****d@example.com",
},
{
name: "three character local part is masked whole, since a lead and tail would be all of it",
email: "abc@example.com",
expected: "****@example.com",
},
{
name: "two character local part is masked whole",
email: "ab@example.com",
expected: "****@example.com",
},
{
name: "single character local part is masked whole",
email: "a@b.co",
expected: "****@b.co",
},
{
name: "mask width does not report the length it stands in for",
email: "a.very.long.local.part@example.com",
expected: "a.****t@example.com",
},
{
name: "a local part far longer than the mask is still reduced to three characters",
email: "finance.department.notifications.owner.account@example.com",
expected: "fi****t@example.com",
},
{
name: "plus addressing is masked along with the rest of the local part",
email: "admin+netbird@example.com",
expected: "ad****d@example.com",
},
{
name: "separators inside the local part are not treated specially",
email: "first.last-name_x@example.com",
expected: "fi****x@example.com",
},
{
name: "case is preserved rather than normalised",
email: "Admin@Example.COM",
expected: "Ad****n@Example.COM",
},
{
name: "subdomains stay intact",
email: "owner@mail.corp.example.com",
expected: "ow****r@mail.corp.example.com",
},
{
name: "german umlauts count as single characters",
email: "müller@example.de",
expected: "mü****r@example.de",
},
{
name: "cyrillic local part is cut on runes",
email: "иванов@example.ru",
expected: "ив****в@example.ru",
},
{
name: "cjk local part of three runes is masked whole, counted in runes not bytes",
email: "用户名@example.cn",
expected: "****@example.cn",
},
{
name: "cjk local part of four runes reveals the first two and the last",
email: "用户名字@example.cn",
expected: "用户****字@example.cn",
},
{
name: "arabic local part is cut on runes",
email: "مستخدم@example.sa",
expected: "مس****م@example.sa",
},
{
name: "two rune non-ascii local part is masked whole",
email: "ää@example.de",
expected: "****@example.de",
},
{
name: "astral plane runes are not split into surrogates",
email: "a🎉bc@example.com",
expected: "a🎉****c@example.com",
},
{
name: "a non-ascii domain is left alone",
email: "admin@münchen.example",
expected: "ad****n@münchen.example",
},
{
name: "only the first separator splits, so a second stays in the domain",
email: "a@b@example.com",
expected: "****@b@example.com",
},
{
name: "empty email has nothing to mask",
email: "",
expected: "",
},
{
name: "value without a separator is not an address",
email: "not-an-email",
expected: "",
},
{
name: "missing local part is not an address",
email: "@example.com",
expected: "",
},
{
name: "missing domain is not an address",
email: "admin@",
expected: "",
},
{
name: "a bare separator is not an address",
email: "@",
expected: "",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, MaskEmail(tc.email))
})
}
}
+88 -16
View File
@@ -1177,28 +1177,35 @@ func (am *DefaultAccountManager) expireAndUpdatePeers(ctx context.Context, accou
dnsDomain := am.networkMapController.GetDNSDomain(settings)
var peerIDs []string
for _, peer := range peers {
defer func() {
if len(peerIDs) == 0 {
return
}
// this will trigger peer disconnect from the management service
log.Debugf("Expiring %d peers for account %s", len(peerIDs), accountID)
am.networkMapController.DisconnectPeers(ctx, accountID, peerIDs)
}()
for _, candidate := range peers {
// nolint:staticcheck
ctx = context.WithValue(ctx, nbcontext.PeerIDKey, peer.Key)
peerCtx := context.WithValue(ctx, nbcontext.PeerIDKey, candidate.Key)
if peer.UserID == "" {
if candidate.UserID == "" {
// we do not want to expire peers that are added via setup key
continue
}
if peer.Status.LoginExpired {
peer, err := am.expirePeerIfStillDue(peerCtx, accountID, candidate.ID, settings, reason)
if err != nil {
return err
}
if peer == nil {
continue
}
peerIDs = append(peerIDs, peer.ID)
peer.MarkLoginExpired(true)
if err := am.Store.SavePeerStatus(ctx, accountID, peer.ID, *peer.Status); err != nil {
return err
}
meta := peer.EventMeta(dnsDomain)
meta["reason"] = string(reason)
am.StoreEvent(
ctx,
peerCtx,
peer.UserID, peer.ID, accountID,
activity.PeerLoginExpired, meta,
)
@@ -1215,15 +1222,53 @@ func (am *DefaultAccountManager) expireAndUpdatePeers(ctx context.Context, accou
if err != nil {
return fmt.Errorf("notify network map controller of peer update: %w", err)
}
if len(peerIDs) != 0 {
// this will trigger peer disconnect from the management service
log.Debugf("Expiring %d peers for account %s", len(peerIDs), accountID)
am.networkMapController.DisconnectPeers(ctx, accountID, peerIDs)
}
return nil
}
// expirePeerIfStillDue flags the peer as login-expired and returns its fresh copy, or nil
// when it no longer qualifies. The candidate list is read without a lock, so a login that
// landed in between would otherwise be overwritten with a stale expired status.
func (am *DefaultAccountManager) expirePeerIfStillDue(ctx context.Context, accountID, peerID string, settings *types.Settings, reason peerExpirationReason) (*nbpeer.Peer, error) {
var expired *nbpeer.Peer
err := am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
peer, err := transaction.GetPeerByID(ctx, store.LockingStrengthUpdate, accountID, peerID)
if err != nil {
if s, ok := status.FromError(err); ok && s.Type() == status.NotFound {
return nil
}
return err
}
if peer.Status.LoginExpired || !peerExpirationDue(peer, settings, reason) {
return nil
}
peer.MarkLoginExpired(true)
if err := transaction.SavePeerStatus(ctx, accountID, peer.ID, *peer.Status); err != nil {
return err
}
expired = peer
return nil
})
if err != nil {
return nil, err
}
return expired, nil
}
// peerExpirationDue re-evaluates a time-based expiry against the peer's current state.
// Administrative reasons expire the peer unconditionally.
func peerExpirationDue(peer *nbpeer.Peer, settings *types.Settings, reason peerExpirationReason) bool {
switch reason {
case peerExpirationSessionExpired:
expired, _ := peer.LoginExpired(settings.PeerLoginExpiration)
return settings.PeerLoginExpirationEnabled && expired
case peerExpirationInactivity:
expired, _ := peer.SessionExpired(settings.PeerInactivityExpiration)
return settings.PeerInactivityExpirationEnabled && expired
default:
return true
}
}
func (am *DefaultAccountManager) deleteUserFromIDP(ctx context.Context, targetUserID, accountID string) error {
if am.userDeleteFromIDPEnabled {
log.WithContext(ctx).Debugf("user %s deleted from IdP", targetUserID)
@@ -1403,6 +1448,25 @@ func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, accountI
return updateAccountPeers, nil
}
// pendingApprovalError refuses a user awaiting approval, naming the owner who
// can approve them when their address resolves. Failing to resolve one is not a
// reason to withhold the refusal, so the lookup is best effort.
func (am *DefaultAccountManager) pendingApprovalError(ctx context.Context, accountID string) error {
owner, err := am.GetOwnerInfo(ctx, accountID)
if err != nil {
log.WithContext(ctx).Debugf("pending approval refusal: owner of account %s did not resolve: %v", accountID, err)
return status.NewUserPendingApprovalError()
}
masked := types.MaskEmail(owner.Email)
if masked == "" {
log.WithContext(ctx).Debugf("pending approval refusal: no address found for the owner of account %s", accountID)
return status.NewUserPendingApprovalError()
}
return status.NewUserPendingApprovalByOwnerError(masked)
}
// GetOwnerInfo retrieves the owner information for a given account ID.
func (am *DefaultAccountManager) GetOwnerInfo(ctx context.Context, accountID string) (*types.UserInfo, error) {
owner, err := am.Store.GetAccountOwner(ctx, store.LockingStrengthNone, accountID)
@@ -1460,6 +1524,14 @@ func (am *DefaultAccountManager) GetCurrentUserInfo(ctx context.Context, userAut
return nil, err
}
// A user pending approval is blocked too, and the dashboard needs to tell
// the two apart: one is a dead end, the other resolves by itself once the
// owner acts. Naming that owner needs the address the IdP holds, which is
// why this is answered here rather than in the permission gate.
if user.IsBlocked() && user.PendingApproval {
return nil, am.pendingApprovalError(ctx, user.AccountID)
}
if user.IsBlocked() {
return nil, status.NewUserBlockedError()
}
+64
View File
@@ -1779,6 +1779,42 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) {
}
require.NoError(t, store.SaveAccount(context.Background(), account2))
account3 := newAccountWithId(context.Background(), "account3", "account3Owner", "", "owner@example.com", "", false)
account3.Users["pending-user"] = &types.User{
Id: "pending-user",
AccountID: account3.Id,
Role: types.UserRoleUser,
Blocked: true,
PendingApproval: true,
}
require.NoError(t, store.SaveAccount(context.Background(), account3))
// The owner has no address to name, so the refusal falls back to the generic one.
account4 := newAccountWithId(context.Background(), "account4", "account4Owner", "", "", "", false)
account4.Users["pending-user-without-owner-email"] = &types.User{
Id: "pending-user-without-owner-email",
AccountID: account4.Id,
Role: types.UserRoleUser,
Blocked: true,
PendingApproval: true,
}
require.NoError(t, store.SaveAccount(context.Background(), account4))
// No user holds the owner role, so the owner lookup itself fails.
account5 := newAccountWithId(context.Background(), "account5", "account5Admin", "", "", "", false)
account5.Users["account5Admin"].Role = types.UserRoleAdmin
account5.Users["pending-user-without-owner"] = &types.User{
Id: "pending-user-without-owner",
AccountID: account5.Id,
Role: types.UserRoleUser,
Blocked: true,
PendingApproval: true,
}
require.NoError(t, store.SaveAccount(context.Background(), account5))
account6 := newAccountWithId(context.Background(), "account6", "account6Owner", "", "stranger@example.com", "", false)
require.NoError(t, store.SaveAccount(context.Background(), account6))
permissionsManager := permissions.NewManager(store)
am := DefaultAccountManager{
Store: store,
@@ -1812,6 +1848,34 @@ func TestDefaultAccountManager_GetCurrentUserInfo(t *testing.T) {
userAuth: auth.UserAuth{AccountId: account1.Id, UserId: "service-user"},
expectedErr: status.NewPermissionDeniedError(),
},
{
name: "pending approval names the owner",
userAuth: auth.UserAuth{AccountId: account3.Id, UserId: "pending-user"},
expectedErr: status.NewUserPendingApprovalByOwnerError("ow****r@example.com"),
},
{
name: "pending approval without an owner address",
userAuth: auth.UserAuth{AccountId: account4.Id, UserId: "pending-user-without-owner-email"},
expectedErr: status.NewUserPendingApprovalError(),
},
{
name: "pending approval without an owner",
userAuth: auth.UserAuth{AccountId: account5.Id, UserId: "pending-user-without-owner"},
expectedErr: status.NewUserPendingApprovalError(),
},
{
// The account claim points at an account the caller is not in. The
// owner named has to be the one of the account holding the caller's
// own record, never the one the claim asks for.
name: "pending approval ignores a mismatched account claim",
userAuth: auth.UserAuth{AccountId: account6.Id, UserId: "pending-user"},
expectedErr: status.NewUserPendingApprovalByOwnerError("ow****r@example.com"),
},
{
name: "blocked user answers before the account claim is validated",
userAuth: auth.UserAuth{AccountId: account6.Id, UserId: "blocked-user"},
expectedErr: status.NewUserBlockedError(),
},
{
name: "owner user",
userAuth: auth.UserAuth{AccountId: account1.Id, UserId: "account1Owner"},