Merge remote-tracking branch 'origin/main' into dmitri-catch-disconnected-peer

Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io>
This commit is contained in:
Dmitri Dolguikh
2026-09-09 14:46:19 +02:00
143 changed files with 5761 additions and 1636 deletions
+17 -7
View File
@@ -14,10 +14,6 @@ import (
"sync"
"time"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
"github.com/netbirdio/netbird/management/server/job"
"github.com/netbirdio/netbird/shared/auth"
cacheStore "github.com/eko/gocache/lib/v4/store"
"github.com/eko/gocache/store/redis/v4"
"github.com/rs/xid"
@@ -29,6 +25,7 @@ import (
"github.com/netbirdio/netbird/formatter/hook"
"github.com/netbirdio/netbird/idp/dex"
"github.com/netbirdio/netbird/management/internals/controllers/network_map"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/activity"
@@ -39,6 +36,7 @@ import (
"github.com/netbirdio/netbird/management/server/idp"
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/job"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/permissions/modules"
@@ -50,6 +48,7 @@ import (
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/management/server/util"
"github.com/netbirdio/netbird/route"
"github.com/netbirdio/netbird/shared/auth"
nbdomain "github.com/netbirdio/netbird/shared/management/domain"
"github.com/netbirdio/netbird/shared/management/networkmap/nmdata"
"github.com/netbirdio/netbird/shared/management/status"
@@ -238,6 +237,10 @@ func BuildManager(
log.WithContext(ctx).Error(err)
}
if IsEmbeddedIdp(idpManager) && accountsCounter > 1 {
log.WithContext(ctx).Warnf("embedded IdP requires a single account, found %d", accountsCounter)
}
// enable single account mode only if configured by user and number of existing accounts is not grater than 1
am.singleAccountMode = singleAccountModeDomain != "" && accountsCounter <= 1
if am.singleAccountMode {
@@ -716,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))
}
}
@@ -749,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))
}
}
@@ -1592,7 +1598,10 @@ func (am *DefaultAccountManager) updateUserAuthWithSingleMode(ctx context.Contex
if err != nil {
return err
}
userAuth.Domain = domain
// Keep the configured single account domain when the existing account has none
if domain != "" {
userAuth.Domain = domain
}
log.WithContext(ctx).Debugf("overriding JWT Domain and DomainCategory claims since single account mode is enabled")
return nil
@@ -1837,6 +1846,7 @@ func (am *DefaultAccountManager) getAccountIDWithAuthorizationClaims(ctx context
return am.addNewPrivateAccount(ctx, domainAccountID, userAuth)
}
func (am *DefaultAccountManager) getPrivateDomainWithGlobalLock(ctx context.Context, domain string) (string, context.CancelFunc, error) {
domainAccountID, err := am.Store.GetAccountIDByPrivateDomain(ctx, store.LockingStrengthNone, domain)
if handleNotFound(err) != nil {
+176 -3
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",
+14 -18
View File
@@ -7,9 +7,6 @@ import (
"errors"
"fmt"
"time"
"github.com/eko/gocache/lib/v4/cache"
"github.com/eko/gocache/lib/v4/store"
)
const (
@@ -22,12 +19,17 @@ var (
ErrTokenExpired = errors.New("JWT expired")
)
type SessionStore struct {
cache *cache.Cache[string]
// TokenCache atomically records used JWTs until their expiration.
type TokenCache interface {
SetNX(ctx context.Context, key, value string, ttl time.Duration) (bool, error)
}
func NewSessionStore(cacheStore store.StoreInterface) *SessionStore {
return &SessionStore{cache: cache.New[string](cacheStore)}
type SessionStore struct {
cache TokenCache
}
func NewSessionStore(cacheStore TokenCache) *SessionStore {
return &SessionStore{cache: cacheStore}
}
// RegisterToken records a JWT until its exp time and rejects reuse.
@@ -38,20 +40,14 @@ func (s *SessionStore) RegisterToken(ctx context.Context, token string, expiresA
}
key := usedTokenKeyPrefix + hashToken(token)
_, err := s.cache.Get(ctx, key)
if err == nil {
created, err := s.cache.SetNX(ctx, key, usedTokenMarker, ttl)
if err != nil {
return fmt.Errorf("store used token entry: %w", err)
}
if !created {
return ErrTokenAlreadyUsed
}
var notFound *store.NotFound
if !errors.As(err, &notFound) {
return fmt.Errorf("failed to lookup used token entry: %w", err)
}
if err := s.cache.Set(ctx, key, usedTokenMarker, store.WithExpiration(ttl)); err != nil {
return fmt.Errorf("failed to store used token entry: %w", err)
}
return nil
}
+51
View File
@@ -2,6 +2,7 @@ package auth
import (
"context"
"errors"
"testing"
"time"
@@ -38,6 +39,39 @@ func TestSessionStore_RegisterSameTokenTwiceIsRejected(t *testing.T) {
assert.ErrorIs(t, err, ErrTokenAlreadyUsed)
}
func TestSessionStore_ConcurrentRegistrationAllowsOneCaller(t *testing.T) {
s := newTestSessionStore(t)
ctx := context.Background()
const attempts = 100
start := make(chan struct{})
results := make(chan error, attempts)
for range attempts {
go func() {
<-start
results <- s.RegisterToken(ctx, "token", time.Now().Add(time.Hour))
}()
}
close(start)
succeeded := 0
alreadyUsed := 0
for range attempts {
err := <-results
switch {
case err == nil:
succeeded++
case errors.Is(err, ErrTokenAlreadyUsed):
alreadyUsed++
default:
require.NoError(t, err, "concurrent registration returned an unexpected error")
}
}
assert.Equal(t, 1, succeeded, "exactly one concurrent caller should register the token")
assert.Equal(t, attempts-1, alreadyUsed, "every other caller should be rejected as already used")
}
func TestSessionStore_RegisterDifferentTokensAreIndependent(t *testing.T) {
s := newTestSessionStore(t)
ctx := context.Background()
@@ -72,6 +106,23 @@ func TestSessionStore_EntryEvictsAtTTLAndAllowsReRegistration(t *testing.T) {
require.NoError(t, s.RegisterToken(ctx, token, time.Now().Add(time.Hour)))
}
type failingTokenCache struct {
err error
}
func (f failingTokenCache) SetNX(context.Context, string, string, time.Duration) (bool, error) {
return false, f.err
}
func TestSessionStore_CacheErrorIsReturned(t *testing.T) {
cacheErr := errors.New("cache unavailable")
s := NewSessionStore(failingTokenCache{err: cacheErr})
err := s.RegisterToken(context.Background(), "token", time.Now().Add(time.Hour))
require.Error(t, err, "cache failure should be surfaced to the caller")
assert.ErrorIs(t, err, cacheErr, "cache error should be wrapped, not replaced")
}
func TestHashToken_StableAndDoesNotLeak(t *testing.T) {
a := hashToken("tokenA")
b := hashToken("tokenB")
+57
View File
@@ -0,0 +1,57 @@
package cache
import (
"context"
"fmt"
"sync"
"time"
"github.com/eko/gocache/lib/v4/store"
gocachestore "github.com/eko/gocache/store/go_cache/v4"
gocache "github.com/patrickmn/go-cache"
)
type goCacheStore struct {
store.StoreInterface
client *gocache.Cache
mu sync.Mutex
}
func newMemoryStore(maxTimeout, cleanupInterval time.Duration) Store {
client := gocache.New(maxTimeout, cleanupInterval)
return &goCacheStore{
StoreInterface: gocachestore.NewGoCache(client),
client: client,
}
}
func (s *goCacheStore) SetNX(_ context.Context, key, value string, ttl time.Duration) (bool, error) {
// Add only returns an error when a non-expired entry already exists.
if err := s.client.Add(key, value, ttl); err != nil {
return false, nil //nolint:nilerr
}
return true, nil
}
// GetDel reads the value under key and removes it. go-cache has no native read-and-delete
// and releases its own lock between the two calls, so mu holds the pair together and no
// value is consumed twice.
//
// Writes do not take mu: a Set landing mid-pair is lost, since GetDel returns the prior
// value and deletes the new one. Callers must write a consumed key only once.
func (s *goCacheStore) GetDel(_ context.Context, key string) (string, bool, error) {
s.mu.Lock()
defer s.mu.Unlock()
value, found := s.client.Get(key)
if !found {
return "", false, nil
}
s.client.Delete(key)
str, ok := value.(string)
if !ok {
return "", false, fmt.Errorf("cached value is %T, not a string", value)
}
return str, true, nil
}
+76
View File
@@ -0,0 +1,76 @@
package cache_test
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/cache"
)
func TestMemoryStore(t *testing.T) {
memStore, err := cache.NewStore(context.Background(), 100*time.Millisecond, 300*time.Millisecond, 100)
require.NoError(t, err, "couldn't create memory store")
ctx := context.Background()
key, value := "testing", "tested"
err = memStore.Set(ctx, key, value)
assert.NoError(t, err, "couldn't set testing data")
result, err := memStore.Get(ctx, key)
assert.NoError(t, err, "couldn't get testing data")
assert.Equal(t, value, result, "value returned doesn't match testing data")
created, err := memStore.SetNX(ctx, "conditional", value, 100*time.Millisecond)
require.NoError(t, err, "couldn't conditionally set testing data")
require.True(t, created, "first conditional set should create the entry")
created, err = memStore.SetNX(ctx, "conditional", value, 100*time.Millisecond)
require.NoError(t, err, "couldn't conditionally check testing data")
require.False(t, created, "second conditional set should not replace the entry")
// test expiration
time.Sleep(300 * time.Millisecond)
_, err = memStore.Get(ctx, key)
assert.Error(t, err, "value should not be found")
}
func TestMemoryStoreGetDel(t *testing.T) {
ctx := context.Background()
newStore := func(t *testing.T) cache.Store {
t.Helper()
memStore, err := cache.NewStore(ctx, time.Minute, time.Minute, 100)
require.NoError(t, err, "couldn't create memory store")
return memStore
}
const (
key = "consume"
value = "verifier"
)
t.Run("exactly one concurrent caller consumes the key", func(t *testing.T) {
memStore := newStore(t)
require.NoError(t, memStore.Set(ctx, key, value), "couldn't set testing data")
assertGetDelConsumedOnce(ctx, t, []cache.Store{memStore}, key, value)
assertGetDelMisses(ctx, t, memStore, key)
})
t.Run("missing key is not an error", func(t *testing.T) {
assertGetDelMisses(ctx, t, newStore(t), "never-set")
})
t.Run("expired key is not found", func(t *testing.T) {
memStore := newStore(t)
_, err := memStore.SetNX(ctx, key, value, 50*time.Millisecond)
require.NoError(t, err, "couldn't set testing data")
time.Sleep(100 * time.Millisecond)
assertGetDelMisses(ctx, t, memStore, key)
})
}
+63
View File
@@ -0,0 +1,63 @@
package cache
import (
"context"
"errors"
"fmt"
"math"
"time"
"github.com/eko/gocache/lib/v4/store"
redisstore "github.com/eko/gocache/store/redis/v4"
"github.com/redis/go-redis/v9"
log "github.com/sirupsen/logrus"
)
type redisStore struct {
store.StoreInterface
client *redis.Client
}
func getRedisStore(ctx context.Context, redisEnvAddr string, maxConn int) (Store, error) {
options, err := redis.ParseURL(redisEnvAddr)
if err != nil {
return nil, fmt.Errorf("parsing redis cache url: %s", err)
}
options.MaxIdleConns = int(math.Ceil(float64(maxConn) * 0.5)) // 50% of max conns
options.MinIdleConns = int(math.Ceil(float64(maxConn) * 0.1)) // 10% of max conns
options.MaxActiveConns = maxConn
options.ConnMaxIdleTime = 30 * time.Minute
options.ConnMaxLifetime = 0
options.PoolTimeout = 10 * time.Second
redisClient := redis.NewClient(options)
subCtx, cancel := context.WithTimeout(ctx, 2*time.Second)
defer cancel()
_, err = redisClient.Ping(subCtx).Result()
if err != nil {
return nil, err
}
log.WithContext(subCtx).Infof("using redis cache at %s", redisEnvAddr)
return &redisStore{
StoreInterface: redisstore.NewRedis(redisClient),
client: redisClient,
}, nil
}
func (s *redisStore) SetNX(ctx context.Context, key, value string, ttl time.Duration) (bool, error) {
return s.client.SetNX(ctx, key, value, ttl).Result()
}
func (s *redisStore) GetDel(ctx context.Context, key string) (string, bool, error) {
value, err := s.client.GetDel(ctx, key).Result()
if errors.Is(err, redis.Nil) {
return "", false, nil
}
if err != nil {
return "", false, err
}
return value, true, nil
}
+153
View File
@@ -0,0 +1,153 @@
package cache_test
import (
"context"
"testing"
"time"
"github.com/eko/gocache/lib/v4/store"
"github.com/redis/go-redis/v9"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
testcontainersredis "github.com/testcontainers/testcontainers-go/modules/redis"
"github.com/netbirdio/netbird/management/server/cache"
)
func startRedis(t *testing.T) string {
t.Helper()
ctx := context.Background()
redisContainer, err := testcontainersredis.Run(ctx, "redis:7")
require.NoError(t, err, "couldn't start redis container")
t.Cleanup(func() {
if err := redisContainer.Terminate(ctx); err != nil {
t.Logf("failed to terminate container: %s", err)
}
})
redisURL, err := redisContainer.ConnectionString(ctx)
require.NoError(t, err, "couldn't get connection string")
t.Setenv(cache.RedisStoreEnvVar, redisURL)
return redisURL
}
func newRedisStore(t *testing.T) cache.Store {
t.Helper()
redisStore, err := cache.NewStore(context.Background(), 100*time.Millisecond, 300*time.Millisecond, 100)
require.NoError(t, err)
return redisStore
}
func TestRedisStoreConnectionFailure(t *testing.T) {
t.Setenv(cache.RedisStoreEnvVar, "redis://127.0.0.1:6379")
_, err := cache.NewStore(context.Background(), 10*time.Millisecond, 30*time.Millisecond, 100)
require.Error(t, err, "getting redis cache store should return error")
}
func TestRedisStoreConnectionSuccess(t *testing.T) {
ctx := context.Background()
redisURL := startRedis(t)
redisStore := newRedisStore(t)
key, value := "testing", "tested"
err := redisStore.Set(ctx, key, value, store.WithExpiration(100*time.Millisecond))
assert.NoError(t, err, "couldn't set testing data")
result, err := redisStore.Get(ctx, key)
assert.NoError(t, err, "couldn't get testing data")
assert.Equal(t, value, result, "value returned doesn't match testing data")
options, err := redis.ParseURL(redisURL)
require.NoError(t, err, "parsing redis cache url")
redisClient := redis.NewClient(options)
r, err := redisClient.Get(ctx, key).Result()
assert.NoError(t, err, "couldn't get testing data from redis")
assert.Equal(t, value, r, "value returned from redis doesn't match testing data")
// test expiration
time.Sleep(300 * time.Millisecond)
_, err = redisStore.Get(ctx, key)
assert.Error(t, err, "value should not be found")
}
func TestRedisStoreSetNX(t *testing.T) {
ctx := context.Background()
redisURL := startRedis(t)
redisStore, secondRedisStore := newRedisStore(t), newRedisStore(t)
const (
key = "conditional"
value = "tested"
)
start := make(chan struct{})
type setResult struct {
created bool
err error
}
results := make(chan setResult, 2)
for _, cacheStore := range []cache.Store{redisStore, secondRedisStore} {
go func() {
<-start
created, err := cacheStore.SetNX(ctx, key, value, time.Minute)
results <- setResult{created: created, err: err}
}()
}
close(start)
created := 0
for range 2 {
result := <-results
require.NoError(t, result.err, "conditional redis set failed")
if result.created {
created++
}
}
require.Equal(t, 1, created, "expected exactly one redis client to create the entry")
options, err := redis.ParseURL(redisURL)
require.NoError(t, err, "parsing redis cache url")
ttl, err := redis.NewClient(options).PTTL(ctx, key).Result()
require.NoError(t, err, "couldn't read entry TTL")
require.Positive(t, ttl, "created entry should have a positive TTL")
}
func TestRedisStoreGetDel(t *testing.T) {
ctx := context.Background()
startRedis(t)
redisStore, secondRedisStore := newRedisStore(t), newRedisStore(t)
const (
key = "consume"
value = "verifier"
)
t.Run("exactly one caller across independent clients consumes the key", func(t *testing.T) {
// A generous TTL: the key is consumed explicitly, so expiry racing the
// concurrent callers would only make the test flaky on a loaded runner.
err := redisStore.Set(ctx, key, value, store.WithExpiration(time.Minute))
require.NoError(t, err, "couldn't set value to consume")
assertGetDelConsumedOnce(ctx, t, []cache.Store{redisStore, secondRedisStore}, key, value)
assertGetDelMisses(ctx, t, secondRedisStore, key)
})
t.Run("missing key is not an error", func(t *testing.T) {
assertGetDelMisses(ctx, t, redisStore, "never-set")
})
t.Run("expired key is not found", func(t *testing.T) {
err := redisStore.Set(ctx, key, value, store.WithExpiration(50*time.Millisecond))
require.NoError(t, err, "couldn't set value to consume")
time.Sleep(100 * time.Millisecond)
assertGetDelMisses(ctx, t, redisStore, key)
})
}
+11 -36
View File
@@ -2,17 +2,10 @@ package cache
import (
"context"
"fmt"
"math"
"os"
"time"
"github.com/eko/gocache/lib/v4/store"
gocache_store "github.com/eko/gocache/store/go_cache/v4"
redis_store "github.com/eko/gocache/store/redis/v4"
gocache "github.com/patrickmn/go-cache"
"github.com/redis/go-redis/v9"
log "github.com/sirupsen/logrus"
)
// RedisStoreEnvVar is the environment variable that determines if a redis store should be used.
@@ -31,15 +24,23 @@ const (
DefaultStoreMaxConn = 1000
)
// Store extends the shared cache interface with conditional and consuming operations.
type Store interface {
store.StoreInterface
// SetNX stores a value with a TTL only when the key does not exist.
SetNX(ctx context.Context, key, value string, ttl time.Duration) (bool, error)
// GetDel reads a value and removes it, so only one caller can consume a key.
GetDel(ctx context.Context, key string) (value string, found bool, err error)
}
// NewStore creates a new cache store with the given max timeout and cleanup interval. It checks for the environment Variable RedisStoreEnvVar
// to determine if a redis store should be used. If the environment variable is set, it will attempt to connect to the redis store.
func NewStore(ctx context.Context, maxTimeout, cleanupInterval time.Duration, maxConn int) (store.StoreInterface, error) {
func NewStore(ctx context.Context, maxTimeout, cleanupInterval time.Duration, maxConn int) (Store, error) {
redisAddr := GetAddrFromEnv()
if redisAddr != "" {
return getRedisStore(ctx, redisAddr, maxConn)
}
goc := gocache.New(maxTimeout, cleanupInterval)
return gocache_store.NewGoCache(goc), nil
return newMemoryStore(maxTimeout, cleanupInterval), nil
}
// GetAddrFromEnv returns the redis address from the environment variable RedisStoreEnvVar or its legacy counterpart.
@@ -50,29 +51,3 @@ func GetAddrFromEnv() string {
}
return addr
}
func getRedisStore(ctx context.Context, redisEnvAddr string, maxConn int) (store.StoreInterface, error) {
options, err := redis.ParseURL(redisEnvAddr)
if err != nil {
return nil, fmt.Errorf("parsing redis cache url: %s", err)
}
options.MaxIdleConns = int(math.Ceil(float64(maxConn) * 0.5)) // 50% of max conns
options.MinIdleConns = int(math.Ceil(float64(maxConn) * 0.1)) // 10% of max conns
options.MaxActiveConns = maxConn
options.ConnMaxIdleTime = 30 * time.Minute
options.ConnMaxLifetime = 0
options.PoolTimeout = 10 * time.Second
redisClient := redis.NewClient(options)
subCtx, cancel := context.WithTimeout(ctx, 2*time.Second)
defer cancel()
_, err = redisClient.Ping(subCtx).Result()
if err != nil {
return nil, err
}
log.WithContext(subCtx).Infof("using redis cache at %s", redisEnvAddr)
return redis_store.NewRedis(redisClient), nil
}
+39 -87
View File
@@ -3,101 +3,53 @@ package cache_test
import (
"context"
"testing"
"time"
"github.com/eko/gocache/lib/v4/store"
"github.com/redis/go-redis/v9"
testcontainersredis "github.com/testcontainers/testcontainers-go/modules/redis"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/server/cache"
)
func TestMemoryStore(t *testing.T) {
memStore, err := cache.NewStore(context.Background(), 100*time.Millisecond, 300*time.Millisecond, 100)
if err != nil {
t.Fatalf("couldn't create memory store: %s", err)
}
ctx := context.Background()
key, value := "testing", "tested"
err = memStore.Set(ctx, key, value)
if err != nil {
t.Errorf("couldn't set testing data: %s", err)
}
result, err := memStore.Get(ctx, key)
if err != nil {
t.Errorf("couldn't get testing data: %s", err)
}
if value != result.(string) {
t.Errorf("value returned doesn't match testing data, got %s, expected %s", result, value)
}
// test expiration
time.Sleep(300 * time.Millisecond)
_, err = memStore.Get(ctx, key)
if err == nil {
t.Error("value should not be found")
}
}
func assertGetDelConsumedOnce(ctx context.Context, t *testing.T, stores []cache.Store, key, value string) {
t.Helper()
func TestRedisStoreConnectionFailure(t *testing.T) {
t.Setenv(cache.RedisStoreEnvVar, "redis://127.0.0.1:6379")
_, err := cache.NewStore(context.Background(), 10*time.Millisecond, 30*time.Millisecond, 100)
if err == nil {
t.Fatal("getting redis cache store should return error")
}
}
const getDelAttempts = 64
func TestRedisStoreConnectionSuccess(t *testing.T) {
ctx := context.Background()
redisContainer, err := testcontainersredis.Run(ctx, "redis:7")
if err != nil {
t.Fatalf("couldn't start redis container: %s", err)
type getDelResult struct {
value string
found bool
err error
}
defer func() {
if err := redisContainer.Terminate(ctx); err != nil {
t.Logf("failed to terminate container: %s", err)
start := make(chan struct{})
results := make(chan getDelResult, getDelAttempts)
for i := range getDelAttempts {
cacheStore := stores[i%len(stores)]
go func() {
<-start
value, found, err := cacheStore.GetDel(ctx, key)
results <- getDelResult{value: value, found: found, err: err}
}()
}
close(start)
consumers := 0
for range getDelAttempts {
result := <-results
require.NoError(t, result.err, "concurrent GetDel failed")
if !result.found {
continue
}
}()
redisURL, err := redisContainer.ConnectionString(ctx)
if err != nil {
t.Fatalf("couldn't get connection string: %s", err)
}
t.Setenv(cache.RedisStoreEnvVar, redisURL)
redisStore, err := cache.NewStore(context.Background(), 100*time.Millisecond, 300*time.Millisecond, 100)
if err != nil {
t.Fatalf("couldn't create redis store: %s", err)
}
key, value := "testing", "tested"
err = redisStore.Set(ctx, key, value, store.WithExpiration(100*time.Millisecond))
if err != nil {
t.Errorf("couldn't set testing data: %s", err)
}
result, err := redisStore.Get(ctx, key)
if err != nil {
t.Errorf("couldn't get testing data: %s", err)
}
if value != result.(string) {
t.Errorf("value returned doesn't match testing data, got %s, expected %s", result, value)
}
options, err := redis.ParseURL(redisURL)
if err != nil {
t.Errorf("parsing redis cache url: %s", err)
}
redisClient := redis.NewClient(options)
r, e := redisClient.Get(ctx, key).Result()
if e != nil {
t.Errorf("couldn't get testing data from redis: %s", e)
}
if value != r {
t.Errorf("value returned from redis doesn't match testing data, got %s, expected %s", r, value)
}
// test expiration
time.Sleep(300 * time.Millisecond)
_, err = redisStore.Get(ctx, key)
if err == nil {
t.Error("value should not be found")
consumers++
require.Equal(t, value, result.value, "consumed value doesn't match testing data")
}
require.Equal(t, 1, consumers, "expected exactly one consumer")
}
func assertGetDelMisses(ctx context.Context, t *testing.T, cacheStore cache.Store, key string) {
t.Helper()
value, found, err := cacheStore.GetDel(ctx, key)
require.NoError(t, err, "GetDel on a missing key should not error")
require.False(t, found, "GetDel should not find key %q, got value %q", key, value)
require.Empty(t, value, "GetDel should return an empty value when not found")
}
@@ -100,9 +100,10 @@ func (h *AuthCallbackHandler) handleCallback(w http.ResponseWriter, r *http.Requ
return
}
// Group validation is performed by the proxy via ValidateSession gRPC call.
// This allows the proxy to show 403 pages directly without redirect dance.
// GenerateSessionToken applies the service's group and account-status gates,
// so a user without access never receives a token. The proxy re-checks the
// installed cookie against the service's allowed groups, and renders the
// denial page from the error carried back in the redirect.
sessionToken, err := h.proxyService.GenerateSessionToken(r.Context(), redirectURL.Hostname(), userID, auth.MethodOIDC)
if err != nil {
log.WithError(err).Error("Failed to create session token")
@@ -136,6 +137,9 @@ func sessionTokenErrorDescription(err error) string {
if errors.Is(err, nbgrpc.ErrUserBlocked) {
return "Your account is blocked"
}
if errors.Is(err, nbgrpc.ErrUserNotInGroup) {
return "You are not authorized to access this service"
}
return "Service configuration error"
}
+38 -2
View File
@@ -10,9 +10,9 @@ import (
"testing"
"time"
"go.uber.org/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel"
@@ -34,6 +34,20 @@ import (
func createManagerWithEmbeddedIdP(t testing.TB) (*DefaultAccountManager, *update_channel.PeersUpdateManager, error) {
t.Helper()
return createManagerWithEmbeddedIdPMode(t, "netbird.selfhosted")
}
func createManagerWithEmbeddedIdPMode(t testing.TB, singleAccountModeDomain string) (*DefaultAccountManager, *update_channel.PeersUpdateManager, error) {
t.Helper()
return createManagerWithEmbeddedIdPModeAndSetup(t, singleAccountModeDomain, nil)
}
func createManagerWithEmbeddedIdPModeAndSetup(
t testing.TB,
singleAccountModeDomain string,
setupStore func(context.Context, store.Store) error,
) (*DefaultAccountManager, *update_channel.PeersUpdateManager, error) {
t.Helper()
ctx := context.Background()
@@ -43,6 +57,11 @@ func createManagerWithEmbeddedIdP(t testing.TB) (*DefaultAccountManager, *update
return nil, nil, err
}
t.Cleanup(cleanUp)
if setupStore != nil {
if err := setupStore(ctx, testStore); err != nil {
return nil, nil, err
}
}
// Create embedded IdP manager
embeddedConfig := &idp.EmbeddedIdPConfig{
@@ -93,7 +112,7 @@ func createManagerWithEmbeddedIdP(t testing.TB) (*DefaultAccountManager, *update
updateManager := update_channel.NewPeersUpdateManager(metrics)
requestBuffer := NewAccountRequestBuffer(ctx, testStore)
networkMapController := controller.NewController(ctx, testStore, metrics, updateManager, requestBuffer, MockIntegratedValidator{}, settingsMockManager, "netbird.cloud", port_forwarding.NewControllerMock(), ephemeral_manager.NewEphemeralManager(testStore, peersManager), &config.Config{}, nil)
manager, err := BuildManager(ctx, &config.Config{}, testStore, networkMapController, job.NewJobManager(nil, testStore, peersManager), idpManager, "", eventStore, nil, false, MockIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false, cacheStore)
manager, err := BuildManager(ctx, &config.Config{}, testStore, networkMapController, job.NewJobManager(nil, testStore, peersManager), idpManager, singleAccountModeDomain, eventStore, nil, false, MockIntegratedValidator{}, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false, cacheStore)
if err != nil {
return nil, nil, err
}
@@ -196,6 +215,23 @@ func TestDefaultAccountManager_GetIdentityProvider_NotFound(t *testing.T) {
assert.Contains(t, err.Error(), "not found")
}
func TestUpdateUserAuthWithSingleModeKeepsConfiguredDomain(t *testing.T) {
ctx := context.Background()
manager, _, err := createManagerWithEmbeddedIdPModeAndSetup(t, "netbird.selfhosted", func(ctx context.Context, testStore store.Store) error {
// An account with no domain, as left behind by an IdP that emitted no domain claims.
return testStore.SaveAccount(ctx, newAccountWithId(ctx, "account-1", "user-1", "", "", "", false))
})
require.NoError(t, err)
require.True(t, manager.singleAccountMode)
userAuth := auth.UserAuth{UserId: "user-2"}
require.NoError(t, manager.updateUserAuthWithSingleMode(ctx, &userAuth))
assert.Equal(t, "netbird.selfhosted", userAuth.Domain,
"An empty account domain must not clear the configured single account domain")
assert.Equal(t, types.PrivateCategory, userAuth.DomainCategory)
}
func TestDefaultAccountManager_UpdateIdentityProvider_Validation(t *testing.T) {
manager, _, err := createManager(t)
require.NoError(t, err)
+166 -2
View File
@@ -10,6 +10,8 @@ import (
"errors"
"fmt"
"os"
"regexp"
"strings"
log "github.com/sirupsen/logrus"
@@ -25,8 +27,10 @@ type Server interface {
EventStore() EventStore // may return nil
}
const idpSeedInfoKey = "IDP_SEED_INFO"
const dryRunEnvKey = "NB_IDP_MIGRATION_DRY_RUN"
const (
idpSeedInfoKey = "IDP_SEED_INFO"
dryRunEnvKey = "NB_IDP_MIGRATION_DRY_RUN"
)
func isDryRun() bool {
return os.Getenv(dryRunEnvKey) == "true"
@@ -233,3 +237,163 @@ func PopulateUserInfo(s Server, idpManager idp.Manager, dryRun bool) error {
return nil
}
const DefaultSingleAccountDomain = "netbird.selfhosted"
var (
ErrMultipleAccounts = errors.New("the embedded IdP supports a single account only")
ErrUnusableDomain = errors.New("domain cannot be resolved in single account mode")
ErrDomainConflict = errors.New("requested domain conflicts with the account domain")
)
var resolvableDomainRegexp = regexp.MustCompile(`^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$`)
// RequireSingleAccount refuses to migrate an instance that holds more than one account.
func RequireSingleAccount(s Server) error {
accountsCounter, err := s.Store().GetAccountsCounter(context.Background())
if err != nil {
return fmt.Errorf("failed to count accounts: %w", err)
}
if accountsCounter > 1 {
return errMultipleAccounts(accountsCounter)
}
return nil
}
func errMultipleAccounts(accountsCounter int64) error {
return fmt.Errorf("%w: this instance has %d accounts. Identity provider connectors are stored without "+
"an account scope, so every account would share and be able to manage the same connectors. "+
"Consolidate this instance to a single account, or keep using an external IdP, before migrating",
ErrMultipleAccounts, accountsCounter)
}
func NormalizeSingleAccountDomain(singleAccountDomain string) (string, error) {
if singleAccountDomain == "" {
singleAccountDomain = DefaultSingleAccountDomain
}
singleAccountDomain = strings.ToLower(singleAccountDomain)
if !resolvableDomainRegexp.MatchString(singleAccountDomain) {
return "", fmt.Errorf("%w: %q must contain at least one dot and only lowercase letters, digits and "+
"hyphens, otherwise users cannot join the existing account", ErrUnusableDomain, singleAccountDomain)
}
return singleAccountDomain, nil
}
// resolveAccountDomain picks the domain the account should end up with. The account keeps a usable
// domain of its own, the configured one only fills a blank. Anything else is a conflict to report.
func resolveAccountDomain(accountID, accountDomain, singleAccountDomain string, requested bool) (string, error) {
accountDomain = strings.ToLower(accountDomain)
if accountDomain == "" {
return singleAccountDomain, nil
}
if !resolvableDomainRegexp.MatchString(accountDomain) {
return "", fmt.Errorf("%w: account %s has domain %q, which must contain at least one dot and only "+
"lowercase letters, digits and hyphens. Correct the account domain before migrating",
ErrUnusableDomain, accountID, accountDomain)
}
if requested && accountDomain != singleAccountDomain {
return "", fmt.Errorf("%w: account %s already uses domain %q but %q was requested. Re-run without "+
"--single-account-mode-domain to keep %q, or correct the account domain first",
ErrDomainConflict, accountID, accountDomain, singleAccountDomain, accountDomain)
}
return accountDomain, nil
}
// EnsureSingleAccountDomain gives the remaining account the domain attributes single account mode
// resolves against, so users can still join it after the migration.
func EnsureSingleAccountDomain(s Server, singleAccountDomain string) error {
plan, err := planSingleAccountDomain(s, singleAccountDomain)
if err != nil {
return err
}
if plan.skip {
return nil
}
if isDryRun() {
log.Infof("[DRY RUN] would set account %s domain to %q, category to %q and mark it as the primary domain account "+
"(currently domain=%q primary=%v)", plan.accountID, plan.domain, types.PrivateCategory,
plan.currentDomain, plan.isPrimary)
return nil
}
if err := s.Store().UpdateAccountDomainAttributes(context.Background(), plan.accountID, plan.domain,
types.PrivateCategory, true); err != nil {
return fmt.Errorf("failed to update domain attributes of account %s: %w", plan.accountID, err)
}
log.Infof("account %s now resolves in single account mode with domain %q", plan.accountID, plan.domain)
return nil
}
// CheckSingleAccountDomain reports whether EnsureSingleAccountDomain would succeed, without writing.
func CheckSingleAccountDomain(s Server, singleAccountDomain string) error {
_, err := planSingleAccountDomain(s, singleAccountDomain)
return err
}
type singleAccountDomainPlan struct {
accountID string
domain string
currentDomain string
isPrimary bool
skip bool
}
// planSingleAccountDomain decides what the account's domain attributes should become. It reads
// only, so it can run both as a preflight and as the first half of the update.
func planSingleAccountDomain(s Server, singleAccountDomain string) (singleAccountDomainPlan, error) {
ctx := context.Background()
// An empty value means the operator did not pick a domain, so the default is only a fallback.
requested := singleAccountDomain != ""
singleAccountDomain, err := NormalizeSingleAccountDomain(singleAccountDomain)
if err != nil {
return singleAccountDomainPlan{}, err
}
accountsCounter, err := s.Store().GetAccountsCounter(ctx)
if err != nil {
return singleAccountDomainPlan{}, fmt.Errorf("failed to count accounts: %w", err)
}
// The count is checked again here: it is read long after RequireSingleAccount, and marking an
// arbitrary account as the primary one for the domain would be wrong.
switch {
case accountsCounter == 0:
log.Info("no accounts yet, nothing to prepare for single account mode")
return singleAccountDomainPlan{skip: true}, nil
case accountsCounter > 1:
return singleAccountDomainPlan{}, errMultipleAccounts(accountsCounter)
}
accountID, err := s.Store().GetAnyAccountID(ctx)
if err != nil {
return singleAccountDomainPlan{}, fmt.Errorf("failed to get the existing account: %w", err)
}
isPrimary, accountDomain, err := s.Store().IsPrimaryAccount(ctx, accountID)
if err != nil {
return singleAccountDomainPlan{}, fmt.Errorf("failed to read domain attributes of account %s: %w", accountID, err)
}
domain, err := resolveAccountDomain(accountID, accountDomain, singleAccountDomain, requested)
if err != nil {
return singleAccountDomainPlan{}, err
}
return singleAccountDomainPlan{
accountID: accountID,
domain: domain,
currentDomain: accountDomain,
isPrimary: isPrimary,
}, nil
}
@@ -24,6 +24,17 @@ type testStore struct {
checkSchemaFunc func(checks []SchemaCheck) []SchemaError
updateCalls []updateUserIDCall
updateInfoCalls []updateUserInfoCall
accountsCounter int64
accounts map[string]*types.Account
domainAttrCalls []domainAttrCall
}
type domainAttrCall struct {
AccountID string
Domain string
Category string
IsPrimary bool
}
type updateUserIDCall struct {
@@ -38,6 +49,35 @@ type updateUserInfoCall struct {
Name string
}
func (s *testStore) GetAccountsCounter(context.Context) (int64, error) {
return s.accountsCounter, nil
}
func (s *testStore) GetAnyAccountID(context.Context) (string, error) {
for id := range s.accounts {
return id, nil
}
return "", fmt.Errorf("no accounts")
}
func (s *testStore) IsPrimaryAccount(_ context.Context, accountID string) (bool, string, error) {
account, ok := s.accounts[accountID]
if !ok {
return false, "", fmt.Errorf("account %s not found", accountID)
}
return account.IsDomainPrimaryAccount, account.Domain, nil
}
func (s *testStore) UpdateAccountDomainAttributes(_ context.Context, accountID, domain, category string, isPrimaryDomain bool) error {
s.domainAttrCalls = append(s.domainAttrCalls, domainAttrCall{accountID, domain, category, isPrimaryDomain})
if account, ok := s.accounts[accountID]; ok {
account.Domain = domain
account.DomainCategory = category
account.IsDomainPrimaryAccount = isPrimaryDomain
}
return nil
}
func (s *testStore) ListUsers(ctx context.Context) ([]*types.User, error) {
return s.listUsersFunc(ctx)
}
@@ -826,3 +866,212 @@ func TestCheckSchema_MockStore(t *testing.T) {
assert.Equal(t, "email", errs[0].Column)
})
}
func TestRequireSingleAccount(t *testing.T) {
tests := []struct {
name string
accounts int64
expectErr bool
}{
{name: "fresh install", accounts: 0},
{name: "single account", accounts: 1},
{name: "multiple accounts", accounts: 3, expectErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
srv := &testServer{store: &testStore{accountsCounter: tt.accounts}}
err := RequireSingleAccount(srv)
if !tt.expectErr {
require.NoError(t, err)
return
}
require.Error(t, err)
assert.ErrorIs(t, err, ErrMultipleAccounts)
})
}
}
func TestEnsureSingleAccountDomain(t *testing.T) {
tests := []struct {
name string
account *types.Account
requestedDomain string
expectedDomain string
}{
{
name: "account migrated from an IdP without domain claims",
account: &types.Account{Id: "account-1"},
expectedDomain: DefaultSingleAccountDomain,
},
{
name: "requested domain is applied to an account without one",
account: &types.Account{Id: "account-1"},
requestedDomain: "corp.example.com",
expectedDomain: "corp.example.com",
},
{
name: "account keeps its own domain",
account: &types.Account{Id: "account-1", Domain: "acme.com"},
expectedDomain: "acme.com",
},
{
name: "requesting the domain the account already has is not a conflict",
account: &types.Account{Id: "account-1", Domain: "acme.com"},
requestedDomain: "acme.com",
expectedDomain: "acme.com",
},
{
name: "already resolvable account is rewritten with the same values",
account: &types.Account{
Id: "account-1",
Domain: "acme.com",
DomainCategory: types.PrivateCategory,
IsDomainPrimaryAccount: true,
},
expectedDomain: "acme.com",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
store := &testStore{
accountsCounter: 1,
accounts: map[string]*types.Account{tt.account.Id: tt.account},
}
require.NoError(t, EnsureSingleAccountDomain(&testServer{store: store}, tt.requestedDomain))
require.Len(t, store.domainAttrCalls, 1)
assert.Equal(t, domainAttrCall{
AccountID: tt.account.Id,
Domain: tt.expectedDomain,
Category: types.PrivateCategory,
IsPrimary: true,
}, store.domainAttrCalls[0])
})
}
}
func TestEnsureSingleAccountDomainDryRun(t *testing.T) {
t.Setenv(dryRunEnvKey, "true")
store := &testStore{
accountsCounter: 1,
accounts: map[string]*types.Account{"account-1": {Id: "account-1"}},
}
require.NoError(t, EnsureSingleAccountDomain(&testServer{store: store}, ""))
assert.Empty(t, store.domainAttrCalls, "Dry run must not write anything")
}
func TestEnsureSingleAccountDomainRejectsUnresolvableDomains(t *testing.T) {
t.Run("account domain that cannot resolve is reported", func(t *testing.T) {
account := &types.Account{Id: "account-1", Domain: "corp"}
store := &testStore{
accountsCounter: 1,
accounts: map[string]*types.Account{account.Id: account},
}
err := EnsureSingleAccountDomain(&testServer{store: store}, "")
require.Error(t, err)
assert.ErrorIs(t, err, ErrUnusableDomain)
assert.Empty(t, store.domainAttrCalls, "A broken account domain must not be replaced silently")
})
t.Run("requested domain conflicting with the account domain is reported", func(t *testing.T) {
account := &types.Account{Id: "account-1", Domain: "acme.com"}
store := &testStore{
accountsCounter: 1,
accounts: map[string]*types.Account{account.Id: account},
}
err := EnsureSingleAccountDomain(&testServer{store: store}, "corp.example.com")
require.Error(t, err)
assert.ErrorIs(t, err, ErrDomainConflict)
assert.Empty(t, store.domainAttrCalls, "A conflict must not overwrite the account domain")
})
t.Run("configured domain that cannot resolve is rejected", func(t *testing.T) {
store := &testStore{
accountsCounter: 1,
accounts: map[string]*types.Account{"account-1": {Id: "account-1"}},
}
err := EnsureSingleAccountDomain(&testServer{store: store}, "corp")
require.Error(t, err)
assert.ErrorIs(t, err, ErrUnusableDomain)
assert.Empty(t, store.domainAttrCalls)
})
t.Run("account appearing after the preflight is rejected", func(t *testing.T) {
store := &testStore{
accountsCounter: 2,
accounts: map[string]*types.Account{
"account-1": {Id: "account-1"},
"account-2": {Id: "account-2"},
},
}
err := EnsureSingleAccountDomain(&testServer{store: store}, "")
require.Error(t, err)
assert.ErrorIs(t, err, ErrMultipleAccounts)
assert.Empty(t, store.domainAttrCalls, "No account may be marked primary when several exist")
})
t.Run("fresh install with no accounts is a no-op", func(t *testing.T) {
store := &testStore{accountsCounter: 0, accounts: map[string]*types.Account{}}
require.NoError(t, EnsureSingleAccountDomain(&testServer{store: store}, ""))
assert.Empty(t, store.domainAttrCalls)
})
}
func TestCheckSingleAccountDomain(t *testing.T) {
tests := []struct {
name string
account *types.Account
requested string
expectErr error
}{
{
name: "usable account domain passes",
account: &types.Account{Id: "account-1", Domain: "acme.com"},
},
{
name: "empty account domain passes",
account: &types.Account{Id: "account-1"},
},
{
name: "unresolvable account domain fails",
account: &types.Account{Id: "account-1", Domain: "corp"},
expectErr: ErrUnusableDomain,
},
{
name: "conflicting request fails",
account: &types.Account{Id: "account-1", Domain: "acme.com"},
requested: "corp.example.com",
expectErr: ErrDomainConflict,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
store := &testStore{
accountsCounter: 1,
accounts: map[string]*types.Account{tt.account.Id: tt.account},
}
err := CheckSingleAccountDomain(&testServer{store: store}, tt.requested)
if tt.expectErr == nil {
require.NoError(t, err)
} else {
require.ErrorIs(t, err, tt.expectErr)
}
assert.Empty(t, store.domainAttrCalls, "The preflight must not write anything")
})
}
}
+14
View File
@@ -60,6 +60,20 @@ type Store interface {
// CheckSchema verifies that all tables and columns required by the migration
// exist in the database. Returns a list of problems; an empty slice means OK.
CheckSchema(checks []SchemaCheck) []SchemaError
// GetAccountsCounter returns the total number of accounts in the store.
GetAccountsCounter(ctx context.Context) (int64, error)
// GetAnyAccountID returns the ID of one of the existing accounts.
GetAnyAccountID(ctx context.Context) (string, error)
// IsPrimaryAccount returns whether the account is the primary account for its domain,
// along with that domain.
IsPrimaryAccount(ctx context.Context, accountID string) (bool, string, error)
// UpdateAccountDomainAttributes sets the domain, domain category and primary
// domain flag of an account.
UpdateAccountDomainAttributes(ctx context.Context, accountID string, domain string, category string, isPrimaryDomain bool) error
}
// RequiredEventSchema lists all tables and columns that the migration tool needs
@@ -13,7 +13,7 @@ type ManagementServiceServerMock struct {
proto.UnimplementedManagementServiceServer
LoginFunc func(context.Context, *proto.EncryptedMessage) (*proto.EncryptedMessage, error)
SyncFunc func(*proto.EncryptedMessage, proto.ManagementService_SyncServer)
SyncFunc func(*proto.EncryptedMessage, proto.ManagementService_SyncServer) error
GetServerKeyFunc func(context.Context, *proto.Empty) (*proto.ServerKeyResponse, error)
IsHealthyFunc func(context.Context, *proto.Empty) (*proto.Empty, error)
GetDeviceAuthorizationFlowFunc func(ctx context.Context, req *proto.EncryptedMessage) (*proto.EncryptedMessage, error)
@@ -30,7 +30,7 @@ func (m ManagementServiceServerMock) Login(ctx context.Context, req *proto.Encry
func (m ManagementServiceServerMock) Sync(msg *proto.EncryptedMessage, sync proto.ManagementService_SyncServer) error {
if m.SyncFunc != nil {
return m.Sync(msg, sync)
return m.SyncFunc(msg, sync)
}
return status.Errorf(codes.Unimplemented, "method Sync not implemented")
}
+10 -6
View File
@@ -1499,9 +1499,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
@@ -1648,7 +1651,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 {
@@ -1668,8 +1673,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")
}
+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)
+65 -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)
@@ -1337,6 +1382,10 @@ func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, accountI
return fmt.Errorf("failed to get user to delete: %w", err)
}
if targetUser.Role == types.UserRoleOwner && targetUser.Id != initiatorUserID {
return status.NewOwnerDeletePermissionError()
}
settings, err = transaction.GetAccountSettings(ctx, store.LockingStrengthNone, accountID)
if err != nil {
return fmt.Errorf("failed to get account settings: %w", err)
+43
View File
@@ -942,6 +942,49 @@ func TestUser_DeleteUser_regularUser(t *testing.T) {
}
func TestUser_deleteRegularUser_RejectsOwner(t *testing.T) {
s, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir())
require.NoError(t, err)
t.Cleanup(cleanup)
account := newAccountWithId(context.Background(), mockAccountID, mockUserID, "", "", "", false)
account.Users[mockTargetUserId] = &types.User{
Id: mockTargetUserId,
Issued: types.UserIssuedAPI,
Role: types.UserRoleOwner,
}
require.NoError(t, s.SaveAccount(context.Background(), account))
am := DefaultAccountManager{Store: s}
_, err = am.deleteRegularUser(context.Background(), mockAccountID, mockUserID, &types.UserInfo{ID: mockTargetUserId})
assert.EqualError(t, err, status.NewOwnerDeletePermissionError().Error())
}
func TestUser_deleteRegularUser_InitiatorOwnerDeletesThemself(t *testing.T) {
s, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir())
require.NoError(t, err)
t.Cleanup(cleanup)
account := newAccountWithId(context.Background(), mockAccountID, mockUserID, "", "", "", false)
require.NoError(t, s.SaveAccount(context.Background(), account))
networkMapControllerMock := network_map.NewMockController(gomock.NewController(t))
networkMapControllerMock.EXPECT().OnPeersDeleted(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
am := DefaultAccountManager{
Store: s,
eventStore: &activity.InMemoryEventStore{},
networkMapController: networkMapControllerMock,
}
_, err = am.deleteRegularUser(context.Background(), mockAccountID, mockUserID, &types.UserInfo{ID: mockUserID})
require.NoError(t, err)
_, err = s.GetUserByUserID(context.Background(), store.LockingStrengthNone, mockUserID)
assert.Equal(t, status.NewUserNotFoundError(mockUserID), err)
}
func TestUser_DeleteUser_RegularUsers(t *testing.T) {
store, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir())
if err != nil {