Merge remote-tracking branch 'origin/main' into refactor/permissions-manager

management/server/store/sql_store.go

@@ -3496,7 +3496,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)
 
@@ -5076,7 +5076,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")
@@ -5735,6 +5735,10 @@ 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
@@ -5756,12 +5760,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

management/server/store/sql_store_domain_expiration.go

@@ -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+".",
+	)
+}

management/server/store/sql_store_domain_expiration_test.go

@@ -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")
+	})
+}

management/server/store/sql_store_get_account_test.go

@@ -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")

management/server/store/sql_store_test.go

@@ -2844,6 +2844,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) {
@@ -4039,9 +4047,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)

management/server/store/store.go

@@ -305,6 +305,8 @@ type Store interface {
 	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
@@ -642,6 +644,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")
 		},

management/server/store/store_mock.go

@@ -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()
@@ -2002,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()