From 7639655883f1d3abc6dcb3440cd72152ee314fbf Mon Sep 17 00:00:00 2001 From: Brad Ison Date: Mon, 3 Aug 2026 12:26:38 +0200 Subject: [PATCH 1/5] [management] Generic gRPC extension seam for external modules (#6894) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Describe your changes This adds an extension point to the management server for registering additional gRPC services. We already have a generic integrations system and dependency injection for server components. This closes the gap on being able to also extend the gRPC API cleanly. ## Issue ticket number and link N/A ## Stack ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [x] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) No docs needed. This is strictly a small internal plumbing enhancement / refactor. --- View with [code]smith Autofix with [code]smith Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled. ## Summary by CodeRabbit * **New Features** * Added a gRPC extension mechanism to contribute additional services and automatically chain extra unary and stream interceptors. * Extension shutdown hooks now run as part of server stop. * Added exported proxy token generation via `GenerateProxyToken()` for external integrations. * **Tests** * Added coverage for extension interceptor/service wiring, extension shutdown execution, and proxy token generation validation (including hash consistency and prefix). --- management/internals/server/boot.go | 9 +- management/internals/server/grpc_extension.go | 74 ++++++++ .../internals/server/grpc_extension_test.go | 160 ++++++++++++++++++ management/internals/server/server.go | 6 + management/server/types/proxy_access_token.go | 7 +- .../server/types/proxy_access_token_test.go | 17 ++ 6 files changed, 270 insertions(+), 3 deletions(-) create mode 100644 management/internals/server/grpc_extension.go create mode 100644 management/internals/server/grpc_extension_test.go diff --git a/management/internals/server/boot.go b/management/internals/server/boot.go index 1c78af9d0..2a1e521aa 100644 --- a/management/internals/server/boot.go +++ b/management/internals/server/boot.go @@ -24,13 +24,13 @@ import ( "github.com/netbirdio/netbird/encryption" "github.com/netbirdio/netbird/formatter/hook" + "github.com/netbirdio/netbird/management/internals/modules/agentnetwork" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs" accesslogsmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs/manager" rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc" "github.com/netbirdio/netbird/management/server/activity" activitystore "github.com/netbirdio/netbird/management/server/activity/store" - "github.com/netbirdio/netbird/management/internals/modules/agentnetwork" nbcache "github.com/netbirdio/netbird/management/server/cache" nbContext "github.com/netbirdio/netbird/management/server/context" nbhttp "github.com/netbirdio/netbird/management/server/http" @@ -184,6 +184,10 @@ func (s *BaseServer) GRPCServer() *grpc.Server { grpc.ChainStreamInterceptor(realip.StreamServerInterceptorOpts(realipOpts...), streamInterceptor, proxyStream), } + // Append interceptors contributed by registered gRPC extensions. These + // run after the built-in chain (ChainUnaryInterceptor is additive). + gRPCOpts = appendExtensionInterceptors(gRPCOpts, s.grpcExtensions) + if s.Config.HttpConfig.LetsEncryptDomain != "" { certManager, err := encryption.CreateCertManager(s.Config.Datadir, s.Config.HttpConfig.LetsEncryptDomain) if err != nil { @@ -215,6 +219,9 @@ func (s *BaseServer) GRPCServer() *grpc.Server { mgmtProto.RegisterProxyServiceServer(gRPCAPIHandler, s.ReverseProxyGRPCServer()) log.Info("ProxyService registered on gRPC server") + // Register services contributed by external modules via the extension seam. + registerExtensions(gRPCAPIHandler, s.grpcExtensions) + return gRPCAPIHandler }) } diff --git a/management/internals/server/grpc_extension.go b/management/internals/server/grpc_extension.go new file mode 100644 index 000000000..3f257c75e --- /dev/null +++ b/management/internals/server/grpc_extension.go @@ -0,0 +1,74 @@ +package server + +import ( + "context" + + "google.golang.org/grpc" +) + +// GRPCExtension bundles an external module's contribution to the management +// gRPC server: the registration of one or more services onto the shared +// grpc.Server, any server-wide interceptors those services require, and an +// optional shutdown hook. It is a generic extension point with no knowledge of +// any specific service. +type GRPCExtension struct { + // Register is invoked with the shared grpc.Server (as a ServiceRegistrar) + // after the built-in services are registered. It may register any number of + // services. May be nil. + Register func(grpc.ServiceRegistrar) + // UnaryInterceptors are appended to the server's unary interceptor chain, + // running after the built-in interceptors. May be empty. + UnaryInterceptors []grpc.UnaryServerInterceptor + // StreamInterceptors are appended to the server's stream interceptor chain, + // running after the built-in interceptors. May be empty. + StreamInterceptors []grpc.StreamServerInterceptor + // Shutdown, if non-nil, is called once during Stop() with the context + // governing server shutdown, which carries a deadline. The hook MUST + // return promptly and MUST abandon its work once that context is + // cancelled or expires: it runs before the rest of Stop()'s cleanup + // (store, event store, embedded IdP) and before Stop() itself checks the + // context's deadline, so a hook that ignores the context will delay all + // of that cleanup and prevent Stop() from returning on time. May be nil. + Shutdown func(ctx context.Context) +} + +// RegisterGRPCExtension registers a gRPC extension. Call before the gRPC server +// is first built (i.e. before Start); registrations after that have no effect. +func (s *BaseServer) RegisterGRPCExtension(ext GRPCExtension) { + s.grpcExtensions = append(s.grpcExtensions, ext) +} + +// appendExtensionInterceptors appends each extension's interceptors to the gRPC +// server options as additional chained interceptors. grpc.ChainUnaryInterceptor +// and grpc.ChainStreamInterceptor are additive, so the returned options run the +// extension interceptors after any interceptors already present in opts. +func appendExtensionInterceptors(opts []grpc.ServerOption, exts []GRPCExtension) []grpc.ServerOption { + for _, ext := range exts { + if len(ext.UnaryInterceptors) > 0 { + opts = append(opts, grpc.ChainUnaryInterceptor(ext.UnaryInterceptors...)) + } + if len(ext.StreamInterceptors) > 0 { + opts = append(opts, grpc.ChainStreamInterceptor(ext.StreamInterceptors...)) + } + } + return opts +} + +// registerExtensions registers each extension's services onto reg. +func registerExtensions(reg grpc.ServiceRegistrar, exts []GRPCExtension) { + for _, ext := range exts { + if ext.Register != nil { + ext.Register(reg) + } + } +} + +// runExtensionShutdownHooks calls each extension's shutdown hook, if set, +// passing ctx through so hooks can honor its deadline/cancellation. +func runExtensionShutdownHooks(ctx context.Context, exts []GRPCExtension) { + for _, ext := range exts { + if ext.Shutdown != nil { + ext.Shutdown(ctx) + } + } +} diff --git a/management/internals/server/grpc_extension_test.go b/management/internals/server/grpc_extension_test.go new file mode 100644 index 000000000..8f444ca72 --- /dev/null +++ b/management/internals/server/grpc_extension_test.go @@ -0,0 +1,160 @@ +package server + +import ( + "context" + "net" + "sync/atomic" + "testing" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/health" + healthgrpc "google.golang.org/grpc/health/grpc_health_v1" + "google.golang.org/grpc/test/bufconn" +) + +// Test that an extension's interceptors and service registration are actually +// wired onto a real in-process gRPC server via the helpers, and that shutdown +// hooks run. This validates the load-bearing assumption that +// grpc.ChainUnaryInterceptor is additive (extension interceptors run in +// addition to any base chain). +func TestGRPCExtensionAppliedToServer(t *testing.T) { + var unaryCalls atomic.Int32 + var streamShutdownCalled atomic.Bool + + ext := GRPCExtension{ + Register: func(reg grpc.ServiceRegistrar) { + healthgrpc.RegisterHealthServer(reg, health.NewServer()) + }, + UnaryInterceptors: []grpc.UnaryServerInterceptor{ + func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { + unaryCalls.Add(1) + return handler(ctx, req) + }, + }, + Shutdown: func(ctx context.Context) { streamShutdownCalled.Store(true) }, + } + exts := []GRPCExtension{ext} + + // Base options mimic GRPCServer(): a pre-existing chain the extension appends to. + var baseUnaryCalls atomic.Int32 + opts := []grpc.ServerOption{ + grpc.ChainUnaryInterceptor(func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { + baseUnaryCalls.Add(1) + return handler(ctx, req) + }), + } + opts = appendExtensionInterceptors(opts, exts) + + srv := grpc.NewServer(opts...) + registerExtensions(srv, exts) + + lis := bufconn.Listen(1024 * 1024) + go func() { _ = srv.Serve(lis) }() + t.Cleanup(srv.Stop) + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return lis.DialContext(ctx) }), + grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = conn.Close() }) + + _, err = healthgrpc.NewHealthClient(conn).Check(context.Background(), &healthgrpc.HealthCheckRequest{}) + if err != nil { + t.Fatalf("health check via extension-registered service failed: %v", err) + } + if baseUnaryCalls.Load() != 1 { + t.Errorf("base interceptor calls = %d, want 1 (base chain must be preserved)", baseUnaryCalls.Load()) + } + if unaryCalls.Load() != 1 { + t.Errorf("extension interceptor calls = %d, want 1", unaryCalls.Load()) + } + + runExtensionShutdownHooks(context.Background(), exts) + if !streamShutdownCalled.Load() { + t.Error("extension shutdown hook was not called") + } +} + +// TestGRPCExtensionShutdownHookReceivesCallerContext asserts that each hook receives +// a non-nil context and that it is the very same context the caller passed +// in, so hooks can rely on values/deadlines placed on it by Stop(). +func TestGRPCExtensionShutdownHookReceivesCallerContext(t *testing.T) { + type sentinelKey struct{} + want := "shutdown-ctx-sentinel" + ctx := context.WithValue(context.Background(), sentinelKey{}, want) + + var called bool + ext := GRPCExtension{ + Shutdown: func(hookCtx context.Context) { + called = true + if hookCtx == nil { + t.Fatal("hook received a nil context") + } + got, _ := hookCtx.Value(sentinelKey{}).(string) + if got != want { + t.Errorf("hook context sentinel = %q, want %q (not the caller's context)", got, want) + } + }, + } + + runExtensionShutdownHooks(ctx, []GRPCExtension{ext}) + if !called { + t.Fatal("shutdown hook was not called") + } +} + +// TestGRPCExtensionShutdownHookObservesCancellation documents, by test, that +// hooks can honor cancellation/deadlines: a hook given an already-cancelled +// context must see ctx.Err() != nil and a closed Done() channel. +func TestGRPCExtensionShutdownHookObservesCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + var called bool + ext := GRPCExtension{ + Shutdown: func(hookCtx context.Context) { + called = true + if hookCtx.Err() == nil { + t.Error("hook context Err() = nil, want non-nil for a cancelled context") + } + select { + case <-hookCtx.Done(): + default: + t.Error("hook context Done() channel is not closed for a cancelled context") + } + }, + } + + runExtensionShutdownHooks(ctx, []GRPCExtension{ext}) + if !called { + t.Fatal("shutdown hook was not called") + } +} + +// TestGRPCExtensionShutdownHookNilSkipped asserts that an extension +// with a nil Shutdown hook is skipped without panicking, and that hooks for +// other extensions still run. +func TestGRPCExtensionShutdownHookNilSkipped(t *testing.T) { + var called atomic.Bool + exts := []GRPCExtension{ + {Shutdown: nil}, + {Shutdown: func(context.Context) { called.Store(true) }}, + } + + runExtensionShutdownHooks(context.Background(), exts) + if !called.Load() { + t.Error("shutdown hook for non-nil extension was not called") + } +} + +func TestRegisterGRPCExtensionAccumulates(t *testing.T) { + s := &BaseServer{} + s.RegisterGRPCExtension(GRPCExtension{}) + s.RegisterGRPCExtension(GRPCExtension{}) + if len(s.grpcExtensions) != 2 { + t.Fatalf("grpcExtensions len = %d, want 2", len(s.grpcExtensions)) + } +} diff --git a/management/internals/server/server.go b/management/internals/server/server.go index 7fd06d947..22a61bada 100644 --- a/management/internals/server/server.go +++ b/management/internals/server/server.go @@ -68,6 +68,11 @@ type BaseServer struct { proxyAuthClose func() + // grpcExtensions holds additional gRPC services, interceptors, and shutdown + // hooks registered by external modules via RegisterGRPCExtension. Populated + // during boot (single-threaded), consumed by GRPCServer() and Stop(). + grpcExtensions []GRPCExtension + listener net.Listener certManager *autocert.Manager update *version.Update @@ -257,6 +262,7 @@ func (s *BaseServer) Stop() error { s.proxyAuthClose() s.proxyAuthClose = nil } + runExtensionShutdownHooks(ctx, s.grpcExtensions) _ = s.Store().Close(ctx) _ = s.EventStore().Close(ctx) if s.update != nil { diff --git a/management/server/types/proxy_access_token.go b/management/server/types/proxy_access_token.go index b20b83bc1..9bb27ef02 100644 --- a/management/server/types/proxy_access_token.go +++ b/management/server/types/proxy_access_token.go @@ -68,7 +68,7 @@ type ProxyAccessTokenGenerated struct { // CreateNewProxyAccessToken generates a new proxy access token. // Returns the token with hashed value stored and plain token for one-time display. func CreateNewProxyAccessToken(name string, expiresIn time.Duration, accountID *string, createdBy string) (*ProxyAccessTokenGenerated, error) { - hashedToken, plainToken, err := generateProxyToken() + hashedToken, plainToken, err := GenerateProxyToken() if err != nil { return nil, err } @@ -94,7 +94,10 @@ func CreateNewProxyAccessToken(name string, expiresIn time.Duration, accountID * }, nil } -func generateProxyToken() (HashedProxyToken, PlainProxyToken, error) { +// GenerateProxyToken generates a new random proxy token, returning its SHA-256 +// hash (for storage) and the one-time plaintext. Exported so external modules +// can mint tokens in the canonical proxy-token format. +func GenerateProxyToken() (HashedProxyToken, PlainProxyToken, error) { secret, err := b.Random(ProxyTokenSecretLength) if err != nil { return "", "", err diff --git a/management/server/types/proxy_access_token_test.go b/management/server/types/proxy_access_token_test.go index aa1a4d2dd..740b87c2f 100644 --- a/management/server/types/proxy_access_token_test.go +++ b/management/server/types/proxy_access_token_test.go @@ -1,6 +1,7 @@ package types import ( + "strings" "testing" "time" @@ -123,6 +124,22 @@ func TestCreateNewProxyAccessToken(t *testing.T) { }) } +func TestGenerateProxyToken(t *testing.T) { + hashed, plain, err := GenerateProxyToken() + if err != nil { + t.Fatal(err) + } + if err := plain.Validate(); err != nil { + t.Errorf("generated token failed Validate(): %v", err) + } + if plain.Hash() != hashed { + t.Error("returned hashed token does not match Hash(plain)") + } + if !strings.HasPrefix(string(plain), ProxyTokenPrefix) { + t.Errorf("token %q missing prefix %q", plain, ProxyTokenPrefix) + } +} + func TestProxyAccessToken_IsExpired(t *testing.T) { past := time.Now().Add(-1 * time.Hour) future := time.Now().Add(1 * time.Hour) From 2bfd9fcffe40acdf3d7ae7bf285f7f14a3931fb3 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 3 Aug 2026 19:45:17 +0900 Subject: [PATCH 2/5] [management] Resolve agent network permissions per submodule (#7030) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Describe your changes Agent Network gates providers, policies, guardrails, budgets, usage, access logs, and settings behind the single `agent_network` permission module, so access is all-or-nothing: a future delegated role cannot be scoped to a subset of the area (for example usage-only visibility). This introduces dotted submodules (`agent_network.providers`, `.policies`, `.guardrails`, `.budgets`, `.usage`, `.logs`, `.settings`) and resolves grants with a cascade: exact module first, then its parent, then the role's `AutoAllowNew` default. The agent network manager now validates each operation against its matching submodule. `usage` (aggregated counters, overview) is deliberately separate from `logs` (request-level entries, which can contain captured prompts). No role definitions change. No built-in role carries an explicit `agent_network` entry, so every role resolves the submodules exactly as it resolved the parent module before — pinned by a test that compares each built-in role's answer on every submodule against its answer on `agent_network`. Role additions that use these submodules come separately. --- .../internals/modules/agentnetwork/manager.go | 77 ++++++---- .../agentnetwork/provider_bootstrap_test.go | 134 +++++++++++++++++ management/server/permissions/manager.go | 22 ++- management/server/permissions/manager_test.go | 139 ++++++++++++++++++ .../server/permissions/modules/module.go | 30 ++++ 5 files changed, 372 insertions(+), 30 deletions(-) create mode 100644 management/internals/modules/agentnetwork/provider_bootstrap_test.go create mode 100644 management/server/permissions/manager_test.go diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go index 77c77ce44..2687e4534 100644 --- a/management/internals/modules/agentnetwork/manager.go +++ b/management/internals/modules/agentnetwork/manager.go @@ -157,14 +157,14 @@ func NewManager( } func (m *managerImpl) GetAllProviders(ctx context.Context, accountID, userID string) ([]*types.Provider, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Read); err != nil { return nil, err } return m.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID) } func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, providerID string) (*types.Provider, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Read); err != nil { return nil, err } return m.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, accountID, providerID) @@ -175,9 +175,14 @@ func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, provid // been created yet; otherwise it is ignored (the cluster is pinned on // Settings and every provider in the account routes through it). func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provider *types.Provider, bootstrapCluster string) (*types.Provider, error) { - if err := m.requirePermission(ctx, provider.AccountID, userID, operations.Create); err != nil { + if err := m.requirePermission(ctx, provider.AccountID, userID, modules.AgentNetworkProviders, operations.Create); err != nil { return nil, err } + if strings.TrimSpace(bootstrapCluster) != "" { + if err := m.requireSettingsBootstrapPermission(ctx, provider.AccountID, userID); err != nil { + return nil, err + } + } // An empty api_key would silently produce a synthesised service // that 401s on every upstream request. Surface the misconfiguration @@ -218,7 +223,7 @@ func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provide } func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provider *types.Provider) (*types.Provider, error) { - if err := m.requirePermission(ctx, provider.AccountID, userID, operations.Update); err != nil { + if err := m.requirePermission(ctx, provider.AccountID, userID, modules.AgentNetworkProviders, operations.Update); err != nil { return nil, err } @@ -257,7 +262,7 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide } func (m *managerImpl) DeleteProvider(ctx context.Context, accountID, userID, providerID string) error { - if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkProviders, operations.Delete); err != nil { return err } @@ -306,21 +311,21 @@ func pluralize(n int, singular, plural string) string { } func (m *managerImpl) GetAllPolicies(ctx context.Context, accountID, userID string) ([]*types.Policy, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkPolicies, operations.Read); err != nil { return nil, err } return m.store.GetAccountAgentNetworkPolicies(ctx, store.LockingStrengthNone, accountID) } func (m *managerImpl) GetPolicy(ctx context.Context, accountID, userID, policyID string) (*types.Policy, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkPolicies, operations.Read); err != nil { return nil, err } return m.store.GetAgentNetworkPolicyByID(ctx, store.LockingStrengthNone, accountID, policyID) } func (m *managerImpl) CreatePolicy(ctx context.Context, userID string, policy *types.Policy) (*types.Policy, error) { - if err := m.requirePermission(ctx, policy.AccountID, userID, operations.Create); err != nil { + if err := m.requirePermission(ctx, policy.AccountID, userID, modules.AgentNetworkPolicies, operations.Create); err != nil { return nil, err } @@ -346,7 +351,7 @@ func (m *managerImpl) CreatePolicy(ctx context.Context, userID string, policy *t } func (m *managerImpl) UpdatePolicy(ctx context.Context, userID string, policy *types.Policy) (*types.Policy, error) { - if err := m.requirePermission(ctx, policy.AccountID, userID, operations.Update); err != nil { + if err := m.requirePermission(ctx, policy.AccountID, userID, modules.AgentNetworkPolicies, operations.Update); err != nil { return nil, err } @@ -373,7 +378,7 @@ func (m *managerImpl) UpdatePolicy(ctx context.Context, userID string, policy *t } func (m *managerImpl) DeletePolicy(ctx context.Context, accountID, userID, policyID string) error { - if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkPolicies, operations.Delete); err != nil { return err } @@ -393,21 +398,21 @@ func (m *managerImpl) DeletePolicy(ctx context.Context, accountID, userID, polic } func (m *managerImpl) GetAllGuardrails(ctx context.Context, accountID, userID string) ([]*types.Guardrail, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkGuardrails, operations.Read); err != nil { return nil, err } return m.store.GetAccountAgentNetworkGuardrails(ctx, store.LockingStrengthNone, accountID) } func (m *managerImpl) GetGuardrail(ctx context.Context, accountID, userID, guardrailID string) (*types.Guardrail, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkGuardrails, operations.Read); err != nil { return nil, err } return m.store.GetAgentNetworkGuardrailByID(ctx, store.LockingStrengthNone, accountID, guardrailID) } func (m *managerImpl) CreateGuardrail(ctx context.Context, userID string, guardrail *types.Guardrail) (*types.Guardrail, error) { - if err := m.requirePermission(ctx, guardrail.AccountID, userID, operations.Create); err != nil { + if err := m.requirePermission(ctx, guardrail.AccountID, userID, modules.AgentNetworkGuardrails, operations.Create); err != nil { return nil, err } @@ -429,7 +434,7 @@ func (m *managerImpl) CreateGuardrail(ctx context.Context, userID string, guardr } func (m *managerImpl) UpdateGuardrail(ctx context.Context, userID string, guardrail *types.Guardrail) (*types.Guardrail, error) { - if err := m.requirePermission(ctx, guardrail.AccountID, userID, operations.Update); err != nil { + if err := m.requirePermission(ctx, guardrail.AccountID, userID, modules.AgentNetworkGuardrails, operations.Update); err != nil { return nil, err } @@ -452,7 +457,7 @@ func (m *managerImpl) UpdateGuardrail(ctx context.Context, userID string, guardr } func (m *managerImpl) DeleteGuardrail(ctx context.Context, accountID, userID, guardrailID string) error { - if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkGuardrails, operations.Delete); err != nil { return err } @@ -473,7 +478,7 @@ func (m *managerImpl) DeleteGuardrail(ctx context.Context, accountID, userID, gu // GetAllBudgetRules returns every account-level budget rule for the account. func (m *managerImpl) GetAllBudgetRules(ctx context.Context, accountID, userID string) ([]*types.AccountBudgetRule, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkBudgets, operations.Read); err != nil { return nil, err } return m.store.GetAccountAgentNetworkBudgetRules(ctx, store.LockingStrengthNone, accountID) @@ -481,7 +486,7 @@ func (m *managerImpl) GetAllBudgetRules(ctx context.Context, accountID, userID s // GetBudgetRule returns a single account-level budget rule. func (m *managerImpl) GetBudgetRule(ctx context.Context, accountID, userID, ruleID string) (*types.AccountBudgetRule, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkBudgets, operations.Read); err != nil { return nil, err } return m.store.GetAgentNetworkBudgetRuleByID(ctx, store.LockingStrengthNone, accountID, ruleID) @@ -491,7 +496,7 @@ func (m *managerImpl) GetBudgetRule(ctx context.Context, accountID, userID, rule // enforced at request time (CheckLLMPolicyLimits), not baked into the synth // proxy config, so no reconcile is needed. func (m *managerImpl) CreateBudgetRule(ctx context.Context, userID string, rule *types.AccountBudgetRule) (*types.AccountBudgetRule, error) { - if err := m.requirePermission(ctx, rule.AccountID, userID, operations.Create); err != nil { + if err := m.requirePermission(ctx, rule.AccountID, userID, modules.AgentNetworkBudgets, operations.Create); err != nil { return nil, err } @@ -513,7 +518,7 @@ func (m *managerImpl) CreateBudgetRule(ctx context.Context, userID string, rule // UpdateBudgetRule updates an existing account-level budget rule. func (m *managerImpl) UpdateBudgetRule(ctx context.Context, userID string, rule *types.AccountBudgetRule) (*types.AccountBudgetRule, error) { - if err := m.requirePermission(ctx, rule.AccountID, userID, operations.Update); err != nil { + if err := m.requirePermission(ctx, rule.AccountID, userID, modules.AgentNetworkBudgets, operations.Update); err != nil { return nil, err } @@ -536,7 +541,7 @@ func (m *managerImpl) UpdateBudgetRule(ctx context.Context, userID string, rule // DeleteBudgetRule removes an account-level budget rule. func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, ruleID string) error { - if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkBudgets, operations.Delete); err != nil { return err } @@ -561,7 +566,7 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r // gating, access-log emission), a reconcile is triggered so the proxy and peer // network maps converge on the new state. func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) { - if err := m.requirePermission(ctx, settings.AccountID, userID, operations.Update); err != nil { + if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Update); err != nil { return nil, err } @@ -615,7 +620,7 @@ func (m *managerImpl) validateProviderRefs(ctx context.Context, accountID string // Returns the underlying status.NotFound when no row has been // bootstrapped yet (i.e. the account has no providers). func (m *managerImpl) GetSettings(ctx context.Context, accountID, userID string) (*types.Settings, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkSettings, operations.Read); err != nil { return nil, err } return m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID) @@ -627,6 +632,22 @@ func (m *managerImpl) GetSettings(ctx context.Context, accountID, userID string) // the subdomain is picked from the curated wordlist avoiding // collisions on the same cluster. Idempotent: if a row already exists // it is returned untouched and the hint is ignored. +// requireSettingsBootstrapPermission gates the one-time settings bootstrap a +// first provider create performs. Pinning the account's cluster and subdomain +// is a settings write, so it needs the settings permission on top of the +// provider one. No-op once the settings row exists. +func (m *managerImpl) requireSettingsBootstrapPermission(ctx context.Context, accountID, userID string) error { + _, err := m.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID) + if err == nil { + return nil + } + var sErr *status.Error + if !errors.As(err, &sErr) || sErr.Type() != status.NotFound { + return fmt.Errorf("get agent network settings: %w", err) + } + return m.requirePermission(ctx, accountID, userID, modules.AgentNetworkSettings, operations.Create) +} + func (m *managerImpl) bootstrapSettingsIfNeeded(ctx context.Context, accountID, providerCluster string) (*types.Settings, error) { if accountID == "" { return nil, fmt.Errorf("bootstrap settings: account id is required") @@ -685,7 +706,7 @@ func (m *managerImpl) bootstrapSettingsIfNeeded(ctx context.Context, accountID, // counter view; permission gate is the same Read role that gates // every other agent-network surface. func (m *managerImpl) ListConsumption(ctx context.Context, accountID, userID string) ([]*types.Consumption, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkUsage, operations.Read); err != nil { return nil, err } return m.store.ListAgentNetworkConsumption(ctx, store.LockingStrengthNone, accountID) @@ -694,7 +715,7 @@ func (m *managerImpl) ListConsumption(ctx context.Context, accountID, userID str // ListAccessLogs returns a paginated, server-side-filtered page of // agent-network access logs plus the total count matching the filter. func (m *managerImpl) ListAccessLogs(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLog, int64, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkLogs, operations.Read); err != nil { return nil, 0, err } return m.store.GetAgentNetworkAccessLogs(ctx, store.LockingStrengthNone, accountID, filter) @@ -704,7 +725,7 @@ func (m *managerImpl) ListAccessLogs(ctx context.Context, accountID, userID stri // agent-network access logs grouped by session, plus the total number of // sessions matching the filter. func (m *managerImpl) ListAccessLogSessions(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLogSession, int64, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkLogs, operations.Read); err != nil { return nil, 0, err } return m.store.GetAgentNetworkAccessLogSessions(ctx, store.LockingStrengthNone, accountID, filter) @@ -713,7 +734,7 @@ func (m *managerImpl) ListAccessLogSessions(ctx context.Context, accountID, user // GetUsageOverview returns the filtered usage rows aggregated into time buckets // at the requested granularity, oldest-first. func (m *managerImpl) GetUsageOverview(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter, granularity types.UsageGranularity) ([]*types.AgentNetworkUsageBucket, error) { - if err := m.requirePermission(ctx, accountID, userID, operations.Read); err != nil { + if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkUsage, operations.Read); err != nil { return nil, err } rows, err := m.store.GetAgentNetworkUsageRows(ctx, store.LockingStrengthNone, accountID, filter) @@ -787,8 +808,8 @@ func (m *managerImpl) RecordConsumption(ctx context.Context, accountID string, k return m.store.IncrementAgentNetworkConsumption(ctx, accountID, kind, dimID, windowSeconds, windowStart, tokensIn, tokensOut, costUSD) } -func (m *managerImpl) requirePermission(ctx context.Context, accountID, userID string, op operations.Operation) error { - ok, _, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.AgentNetwork, op) +func (m *managerImpl) requirePermission(ctx context.Context, accountID, userID string, module modules.Module, op operations.Operation) error { + ok, _, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, module, op) if err != nil { return status.NewPermissionValidationError(err) } diff --git a/management/internals/modules/agentnetwork/provider_bootstrap_test.go b/management/internals/modules/agentnetwork/provider_bootstrap_test.go new file mode 100644 index 000000000..1a2904c51 --- /dev/null +++ b/management/internals/modules/agentnetwork/provider_bootstrap_test.go @@ -0,0 +1,134 @@ +package agentnetwork + +import ( + "context" + "runtime" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types" + "github.com/netbirdio/netbird/management/server/account" + "github.com/netbirdio/netbird/management/server/permissions" + "github.com/netbirdio/netbird/management/server/permissions/modules" + "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/server/store" + nbtypes "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/shared/management/status" +) + +// bootstrapFixture wires a real sqlite store to a gomock permissions manager +// so tests can grant the provider permission while denying (or never +// expecting) the settings one. +type bootstrapFixture struct { + manager Manager + store store.Store + perms *permissions.MockManager +} + +func newBootstrapFixture(t *testing.T) *bootstrapFixture { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("sqlite store not properly supported on Windows yet") + } + t.Setenv("NETBIRD_STORE_ENGINE", string(nbtypes.SqliteStoreEngine)) + + st, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir()) + require.NoError(t, err, "test store setup must succeed") + t.Cleanup(cleanUp) + + ctrl := gomock.NewController(t) + perms := permissions.NewMockManager(ctrl) + + accounts := account.NewMockManager(ctrl) + accounts.EXPECT().StoreEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + accounts.EXPECT().UpdateAccountPeers(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + accounts.EXPECT().BufferUpdateAccountPeers(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + + return &bootstrapFixture{ + manager: NewManager(st, perms, accounts, nil), + store: st, + perms: perms, + } +} + +func (f *bootstrapFixture) expectPermission(accountID, userID string, module modules.Module, op operations.Operation, allowed bool) { + f.perms.EXPECT(). + ValidateUserPermissions(gomock.Any(), accountID, userID, module, op). + Return(allowed, context.Background(), nil) +} + +func newBootstrapProvider(accountID string) *types.Provider { + p := types.NewProvider(accountID) + p.Name = "openai" + p.UpstreamURL = "https://api.openai.com" + p.APIKey = "sk-test" + p.Enabled = true + return p +} + +// TestCreateProviderBootstrapRequiresSettingsPermission pins the gate on the +// one-time settings bootstrap: creating the first provider with a +// bootstrap_cluster pins the account's cluster and subdomain, which is a +// settings write and must not ride on the providers permission alone. +func TestCreateProviderBootstrapRequiresSettingsPermission(t *testing.T) { + ctx := context.Background() + + t.Run("denied without settings permission", func(t *testing.T) { + f := newBootstrapFixture(t) + f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true) + f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, false) + + _, err := f.manager.CreateProvider(ctx, "user1", newBootstrapProvider("account1"), "cluster1.example.com") + require.Error(t, err, "bootstrap without settings permission must fail") + var sErr *status.Error + require.ErrorAs(t, err, &sErr) + assert.Equal(t, status.PermissionDenied, sErr.Type(), "denial should surface as permission denied") + + providers, err := f.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, "account1") + require.NoError(t, err) + assert.Empty(t, providers, "provider must not be persisted when bootstrap is denied") + _, err = f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "account1") + assert.Error(t, err, "settings row must not be created when bootstrap is denied") + }) + + t.Run("allowed with settings permission", func(t *testing.T) { + f := newBootstrapFixture(t) + f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true) + f.expectPermission("account1", "user1", modules.AgentNetworkSettings, operations.Create, true) + + created, err := f.manager.CreateProvider(ctx, "user1", newBootstrapProvider("account1"), "cluster1.example.com") + require.NoError(t, err, "bootstrap with both permissions must succeed") + require.NotNil(t, created) + + settings, err := f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, "account1") + require.NoError(t, err, "bootstrap must create the settings row") + assert.Equal(t, "cluster1.example.com", settings.Cluster, "settings should pin the bootstrap cluster") + }) + + t.Run("existing settings need no settings permission", func(t *testing.T) { + f := newBootstrapFixture(t) + require.NoError(t, f.store.SaveAgentNetworkSettings(ctx, &types.Settings{ + AccountID: "account1", + Cluster: "cluster1.example.com", + Subdomain: "existing", + }), "pre-existing settings row setup must succeed") + + // Only the providers permission may be consulted: gomock fails the + // test on any unexpected settings-permission call. + f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true) + + _, err := f.manager.CreateProvider(ctx, "user1", newBootstrapProvider("account1"), "cluster1.example.com") + require.NoError(t, err, "create with existing settings must not require the settings permission") + }) + + t.Run("no bootstrap cluster needs no settings permission", func(t *testing.T) { + f := newBootstrapFixture(t) + f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true) + + _, err := f.manager.CreateProvider(ctx, "user1", newBootstrapProvider("account1"), "") + require.NoError(t, err, "create without bootstrap must not require the settings permission") + }) +} diff --git a/management/server/permissions/manager.go b/management/server/permissions/manager.go index 995f234d8..6b9977a86 100644 --- a/management/server/permissions/manager.go +++ b/management/server/permissions/manager.go @@ -82,6 +82,9 @@ func (m *managerImpl) ValidateUserPermissions( return m.ValidateRoleModuleAccess(ctx, accountID, role, module, operation), ctxEnriched, nil } +// ValidateRoleModuleAccess resolves an operation against the role's explicit +// grant for the module, then the grant for its parent module when the module +// is a dotted submodule, and finally the role's AutoAllowNew default. func (m *managerImpl) ValidateRoleModuleAccess( ctx context.Context, accountID string, @@ -89,7 +92,7 @@ func (m *managerImpl) ValidateRoleModuleAccess( module modules.Module, operation operations.Operation, ) bool { - if permissions, ok := role.Permissions[module]; ok { + if permissions, ok := lookupModulePermissions(role, module); ok { if allowed, exists := permissions[operation]; exists { return allowed } @@ -100,6 +103,21 @@ func (m *managerImpl) ValidateRoleModuleAccess( return role.AutoAllowNew[operation] } +// lookupModulePermissions returns the role's explicit permission set for the +// module, falling back to the parent module's set for dotted submodules. The +// second return reports whether any explicit set was found. +func lookupModulePermissions(role roles.RolePermissions, module modules.Module) (map[operations.Operation]bool, bool) { + if permissions, ok := role.Permissions[module]; ok { + return permissions, true + } + if parent, hasParent := module.Parent(); hasParent { + if permissions, ok := role.Permissions[parent]; ok { + return permissions, true + } + } + return nil, false +} + func (m *managerImpl) ValidateAccountAccess(ctx context.Context, accountID string, user *types.User, allowOwnerAndAdmin bool) (context.Context, error) { if user.AccountID != accountID { return ctx, status.NewUserNotPartOfAccountError() @@ -119,7 +137,7 @@ func (m *managerImpl) GetPermissionsByRole(ctx context.Context, role types.UserR permissions := roles.Permissions{} for k := range modules.All { - if rolePermissions, ok := roleMap.Permissions[k]; ok { + if rolePermissions, ok := lookupModulePermissions(roleMap, k); ok { permissions[k] = rolePermissions continue } diff --git a/management/server/permissions/manager_test.go b/management/server/permissions/manager_test.go new file mode 100644 index 000000000..345212f43 --- /dev/null +++ b/management/server/permissions/manager_test.go @@ -0,0 +1,139 @@ +package permissions + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/server/permissions/modules" + "github.com/netbirdio/netbird/management/server/permissions/operations" + "github.com/netbirdio/netbird/management/server/permissions/roles" + "github.com/netbirdio/netbird/management/server/types" +) + +func TestValidateRoleModuleAccessSubmoduleCascade(t *testing.T) { + manager := NewManager(nil) + ctx := context.Background() + + fullAccess := map[operations.Operation]bool{ + operations.Read: true, + operations.Create: true, + operations.Update: true, + operations.Delete: true, + } + readOnly := map[operations.Operation]bool{ + operations.Read: true, + operations.Create: false, + operations.Update: false, + operations.Delete: false, + } + denyAll := map[operations.Operation]bool{ + operations.Read: false, + operations.Create: false, + operations.Update: false, + operations.Delete: false, + } + + t.Run("parent grant covers submodules", func(t *testing.T) { + role := roles.RolePermissions{ + AutoAllowNew: denyAll, + Permissions: roles.Permissions{modules.AgentNetwork: fullAccess}, + } + assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Create), + "parent full grant should allow create on a submodule") + assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkLogs, operations.Read), + "parent full grant should allow read on a submodule") + }) + + t.Run("submodule grant does not leak to parent or siblings", func(t *testing.T) { + role := roles.RolePermissions{ + AutoAllowNew: denyAll, + Permissions: roles.Permissions{modules.AgentNetworkUsage: readOnly}, + } + assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkUsage, operations.Read), + "explicit submodule read should be allowed") + assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkUsage, operations.Create), + "read-only submodule grant should not allow create") + assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetwork, operations.Read), + "submodule grant should not grant the parent module") + assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Read), + "submodule grant should not grant a sibling submodule") + }) + + t.Run("explicit submodule entry wins over parent grant", func(t *testing.T) { + role := roles.RolePermissions{ + AutoAllowNew: denyAll, + Permissions: roles.Permissions{ + modules.AgentNetwork: fullAccess, + modules.AgentNetworkLogs: denyAll, + }, + } + assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkLogs, operations.Read), + "explicit submodule deny should override the parent grant") + assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkUsage, operations.Read), + "sibling submodules should still resolve through the parent grant") + }) + + t.Run("auto allow applies when neither submodule nor parent is granted", func(t *testing.T) { + role := roles.RolePermissions{ + AutoAllowNew: readOnly, + } + assert.True(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Read), + "auto-allow read should apply to submodules") + assert.False(t, manager.ValidateRoleModuleAccess(ctx, "account", role, modules.AgentNetworkProviders, operations.Delete), + "auto-allow should not grant unlisted operations") + }) +} + +// TestExistingRolesKeepAgentNetworkBehaviorOnSubmodules pins the behavior the +// submodule split must not change: every built-in role resolves the new +// submodules exactly as it resolved the agent_network module before. +func TestExistingRolesKeepAgentNetworkBehaviorOnSubmodules(t *testing.T) { + manager := NewManager(nil) + ctx := context.Background() + + submodules := []modules.Module{ + modules.AgentNetworkProviders, + modules.AgentNetworkPolicies, + modules.AgentNetworkGuardrails, + modules.AgentNetworkBudgets, + modules.AgentNetworkUsage, + modules.AgentNetworkLogs, + modules.AgentNetworkSettings, + } + allOperations := []operations.Operation{operations.Read, operations.Create, operations.Update, operations.Delete} + + for _, role := range []types.UserRole{types.UserRoleOwner, types.UserRoleAdmin, types.UserRoleAuditor, types.UserRoleNetworkAdmin, types.UserRoleUser} { + rolePermissions, ok := roles.RolesMap[role] + require.True(t, ok, "role %s must exist in RolesMap", role) + + for _, sub := range submodules { + for _, op := range allOperations { + expected := manager.ValidateRoleModuleAccess(ctx, "account", rolePermissions, modules.AgentNetwork, op) + actual := manager.ValidateRoleModuleAccess(ctx, "account", rolePermissions, sub, op) + assert.Equal(t, expected, actual, "role %s: %s on %s should match the agent_network module", role, op, sub) + } + } + } +} + +func TestGetPermissionsByRoleIncludesSubmodules(t *testing.T) { + manager := NewManager(nil) + ctx := context.Background() + + permissions, err := manager.GetPermissionsByRole(ctx, types.UserRoleAuditor) + require.NoError(t, err, "auditor role must resolve") + + usage, ok := permissions[modules.AgentNetworkUsage] + require.True(t, ok, "permissions map should contain the usage submodule") + assert.True(t, usage[operations.Read], "auditor should read the usage submodule") + assert.False(t, usage[operations.Update], "auditor should not update the usage submodule") + + adminPermissions, err := manager.GetPermissionsByRole(ctx, types.UserRoleAdmin) + require.NoError(t, err, "admin role must resolve") + providers, ok := adminPermissions[modules.AgentNetworkProviders] + require.True(t, ok, "permissions map should contain the providers submodule") + assert.True(t, providers[operations.Delete], "admin should delete on the providers submodule") +} diff --git a/management/server/permissions/modules/module.go b/management/server/permissions/modules/module.go index a3a9c554d..8a2a1a52d 100644 --- a/management/server/permissions/modules/module.go +++ b/management/server/permissions/modules/module.go @@ -1,5 +1,7 @@ package modules +import "strings" + type Module string const ( @@ -20,6 +22,17 @@ const ( IdentityProviders Module = "identity_providers" Services Module = "services" AgentNetwork Module = "agent_network" + + // Agent Network submodules. A role may grant one of these directly + // or grant the AgentNetwork parent, which covers all of them (see + // permissions.Manager cascade resolution). + AgentNetworkProviders Module = "agent_network.providers" + AgentNetworkPolicies Module = "agent_network.policies" + AgentNetworkGuardrails Module = "agent_network.guardrails" + AgentNetworkBudgets Module = "agent_network.budgets" + AgentNetworkUsage Module = "agent_network.usage" + AgentNetworkLogs Module = "agent_network.logs" + AgentNetworkSettings Module = "agent_network.settings" ) var All = map[Module]struct{}{ @@ -40,4 +53,21 @@ var All = map[Module]struct{}{ IdentityProviders: {}, Services: {}, AgentNetwork: {}, + + AgentNetworkProviders: {}, + AgentNetworkPolicies: {}, + AgentNetworkGuardrails: {}, + AgentNetworkBudgets: {}, + AgentNetworkUsage: {}, + AgentNetworkLogs: {}, + AgentNetworkSettings: {}, +} + +// Parent returns the module owning a dotted submodule name and true, or the +// module itself and false when it has no parent. +func (m Module) Parent() (Module, bool) { + if i := strings.IndexByte(string(m), '.'); i > 0 { + return Module(string(m)[:i]), true + } + return m, false } From f9b412228e19b525123e10d879cadb08dfeec645 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:20:53 +0200 Subject: [PATCH 3/5] [management] fix handling of empty network map during decode and encode (#6987) --- .../shared/grpc/components_encoder.go | 2 + .../shared/grpc/components_encoder_test.go | 11 +++- shared/management/networkmap/decode.go | 14 +++-- shared/management/networkmap/envelope_test.go | 60 +++++++++++++++++++ 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/management/internals/shared/grpc/components_encoder.go b/management/internals/shared/grpc/components_encoder.go index 7e43cf478..e1a5cae48 100644 --- a/management/internals/shared/grpc/components_encoder.go +++ b/management/internals/shared/grpc/components_encoder.go @@ -61,6 +61,8 @@ func EncodeNetworkMapEnvelope(in ComponentsEnvelopeInput) *proto.NetworkMapEnvel return &proto.NetworkMapEnvelope{ Payload: &proto.NetworkMapEnvelope_Full{ Full: &proto.NetworkMapComponentsFull{ + Serial: networkSerial(c.Network), + Network: toAccountNetwork(c.Network), PeerConfig: in.PeerConfig, // components.Peers always contains the target peer Peers: []*proto.PeerCompact{toPeerCompact(c.Peers[c.PeerID])}, diff --git a/management/internals/shared/grpc/components_encoder_test.go b/management/internals/shared/grpc/components_encoder_test.go index 100ab0948..f7df82f2f 100644 --- a/management/internals/shared/grpc/components_encoder_test.go +++ b/management/internals/shared/grpc/components_encoder_test.go @@ -758,6 +758,9 @@ func TestEncodeNetworkMapEnvelope_NilComponentsGracefulDegrade(t *testing.T) { assert.Equal(t, "netbird.cloud", full.DnsDomain) assert.Len(t, full.Peers, 1) assert.Empty(t, full.Policies) + require.NotNil(t, full.Network, "client runs Calculate() over the envelope and dereferences Network unconditionally; a nil here would crash the receiver") + assert.Equal(t, "net-empty", full.Network.Identifier) + assert.Equal(t, uint64(9), full.Serial) } func TestEncodeNetworkMapEnvelope_AccountSettingsAlwaysEmitted(t *testing.T) { @@ -776,6 +779,12 @@ func TestEncodeNetworkMapEnvelope_AccountSettingsAlwaysEmitted(t *testing.T) { func emptyNetworkMapComponents() *types.NetworkMapComponents { return types.EmptyNetworkMapComponents( &types.NetworkMapComponents{ - PeerID: "peer-id", Peers: map[string]*types.ComponentPeer{"peer-id": {}}}, + PeerID: "peer-id", Peers: map[string]*types.ComponentPeer{"peer-id": {}}, + Network: &types.Network{ + Identifier: "net-empty", + Net: net.IPNet{IP: net.IP{100, 64, 0, 0}, Mask: net.CIDRMask(10, 32)}, + Serial: 9, + }, + }, ) } diff --git a/shared/management/networkmap/decode.go b/shared/management/networkmap/decode.go index d15117b6e..4864a9dff 100644 --- a/shared/management/networkmap/decode.go +++ b/shared/management/networkmap/decode.go @@ -228,15 +228,17 @@ func DecodeEnvelope(env *proto.NetworkMapEnvelope) (*types.NetworkMapComponents, return c, nil } +// decodeAccountNetwork never returns nil — Calculate() dereferences +// c.Network unconditionally, and servers that predate the fix omit the field +// entirely from the empty-components envelope. func decodeAccountNetwork(an *proto.AccountNetwork) *types.Network { + n := &types.Network{} if an == nil { - return nil - } - n := &types.Network{ - Identifier: an.Identifier, - Dns: an.Dns, - Serial: an.Serial, + return n } + n.Identifier = an.Identifier + n.Dns = an.Dns + n.Serial = an.Serial if an.NetCidr != "" { if _, ipnet, err := net.ParseCIDR(an.NetCidr); err == nil && ipnet != nil { n.Net = *ipnet diff --git a/shared/management/networkmap/envelope_test.go b/shared/management/networkmap/envelope_test.go index a81478aff..92e1916da 100644 --- a/shared/management/networkmap/envelope_test.go +++ b/shared/management/networkmap/envelope_test.go @@ -221,6 +221,66 @@ func TestEnvelopeRoundTrip_AllGroupShortCircuitParity(t *testing.T) { "client-side Calculate must connect the same remote peers as the server") } +// TestEnvelopeToNetworkMap_EmptyComponents covers the graceful-degrade path +// the server takes for a peer that is missing from the account or absent from +// the validated-peers map. The legacy server short-circuited before +// Calculate() and shipped a NetworkMap carrying only the account Network; the +// components path runs Calculate() on the client instead, so the envelope must +// carry Network or the client panics dereferencing a nil *types.Network. +func TestEnvelopeToNetworkMap_EmptyComponents(t *testing.T) { + localPeerKey := randomWgKey(t) + c := types.EmptyNetworkMapComponents(&types.NetworkMapComponents{ + PeerID: "peer-A", + Network: &types.Network{ + Identifier: "net-empty", + Net: net.IPNet{IP: net.IP{100, 64, 0, 0}, Mask: net.CIDRMask(10, 32)}, + Serial: 7, + }, + Peers: map[string]*types.ComponentPeer{ + "peer-A": {ID: "peer-A", Key: localPeerKey, IP: netip.AddrFrom4([4]byte{100, 64, 0, 1})}, + }, + }) + + envelope := mgmtgrpc.EncodeNetworkMapEnvelope(mgmtgrpc.ComponentsEnvelopeInput{ + Components: c, + DNSDomain: "netbird.cloud", + }) + require.NotNil(t, envelope.GetFull().Network, "empty envelope must carry the account Network") + + wire, err := goproto.Marshal(envelope) + require.NoError(t, err, "marshal envelope") + var decoded proto.NetworkMapEnvelope + require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") + + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + require.NoError(t, err, "EnvelopeToNetworkMap must degrade gracefully on empty components") + require.Equal(t, uint64(7), result.NetworkMap.Serial) + require.Empty(t, result.NetworkMap.RemotePeers, "unvalidated peer connects to nobody") +} + +// TestEnvelopeToNetworkMap_MissingNetwork simulates a server that omits +// AccountNetwork from the envelope. Clients must degrade rather than panic, so +// they survive talking to a management server that predates the encoder fix. +func TestEnvelopeToNetworkMap_MissingNetwork(t *testing.T) { + c, localPeerKey := buildSmokeComponents(t) + + envelope := mgmtgrpc.EncodeNetworkMapEnvelope(mgmtgrpc.ComponentsEnvelopeInput{ + Components: c, + DNSDomain: "netbird.cloud", + }) + envelope.GetFull().Network = nil + + wire, err := goproto.Marshal(envelope) + require.NoError(t, err, "marshal envelope") + var decoded proto.NetworkMapEnvelope + require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope") + + result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud") + require.NoError(t, err, "a missing AccountNetwork must not panic the client") + require.NotNil(t, result.Components.Network) + require.NotEmpty(t, result.NetworkMap.RemotePeers, "the rest of the snapshot stays usable") +} + // buildSmokeComponents returns a minimal NetworkMapComponents (2 peers, 1 // group, 1 allow policy) plus the receiving peer's WG public key. Sufficient // to validate the encode → marshal → decode → Calculate pipeline produces From 2f721ec0d5d213e59d33aeedf1a9fd90370d2535 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Mon, 3 Aug 2026 14:21:40 +0200 Subject: [PATCH 4/5] [client] Keep the account email backing the SSO login hint correct (#6986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Describe your changes Three fixes to how the desktop GUI keeps the account email that backs the SSO `login_hint`. Each is independent and reviewable on its own. **1. Store the email after a GUI SSO login** The daemon returns the authenticated user's email from `WaitSSOLogin` but cannot persist it: it runs as root while the per-profile state file is user-owned. The CLI's `handleSSOLogin` writes it after its own `WaitSSOLogin`; the GUI path read the value and dropped it. The profile was therefore left with no email, so `Profiles.List` showed no account for it, and later logins and session extends went out with no `login_hint` — leaving the IdP to pick an account instead of reusing the one the profile belongs to. Mirror the CLI and store it, next to the `Logout` path that already clears the same file for the same reason. **2. File the email against the profile the login ran for** `SetActiveProfileState` resolves the target itself, so it writes to whichever profile is active when it is called. A GUI SSO login spans seconds of user interaction in the browser, and the tray stays clickable throughout: switching profiles in that window left the email filed under the profile that happened to be active when the flow returned. The wrong profile then advertised an account it does not own, and offered it as the `login_hint` next time. Adds `SetProfileState(id, state)`, the write-side counterpart of the existing `GetProfileState(id)`, and keeps `SetActiveProfileState` as a wrapper for callers with no particular profile in mind. `Login` now reports the profile it resolved so the frontend can hand it back with the SSO wait, which closes the window. **3. Delete the email when a profile is removed** Removing a profile left its state file behind: the daemon deletes what it owns, but the email file is user-owned and out of reach for a root daemon — the same split that already puts the `Logout` cleanup on the UI side. Beyond the stray file, legacy profiles are keyed by name rather than by a generated ID, so recreating a profile under a removed one's name inherited its email — shown as the account in the profile list and sent as the `login_hint` on the next login. ## Issue ticket number and link ## Stack ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ ## Summary by CodeRabbit - **New Features** - SSO login details can now be saved to the specific profile selected during sign-in. - Profile state can be managed independently for different profiles. - **Bug Fixes** - Removing a profile now also cleans up its associated saved state. - Cleanup issues no longer prevent successful profile removal and are handled gracefully. - Existing active-profile behavior remains unchanged. --- client/internal/profilemanager/state.go | 38 ++++++++++++++-------- client/ui/frontend/src/lib/connection.ts | 9 ++++-- client/ui/services/connection.go | 40 ++++++++++++++++++++++++ client/ui/services/profile.go | 26 +++++++++++++-- 4 files changed, 96 insertions(+), 17 deletions(-) diff --git a/client/internal/profilemanager/state.go b/client/internal/profilemanager/state.go index 9e9577796..fcd1c384c 100644 --- a/client/internal/profilemanager/state.go +++ b/client/internal/profilemanager/state.go @@ -45,12 +45,35 @@ func (pm *ProfileManager) GetProfileState(id ID) (*ProfileState, error) { return &state, nil } -func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error { +// SetProfileState writes the state file of the profile identified by id. Prefer +// it over SetActiveProfileState whenever the caller knows which profile the data +// belongs to: an SSO login spans seconds of user interaction, and the active +// profile can change during it, which would file the account email under +// whichever profile happened to be active when the flow returned. +func (pm *ProfileManager) SetProfileState(id ID, state *ProfileState) error { configDir, err := getConfigDir() if err != nil { return fmt.Errorf("get config directory: %w", err) } + if id == "" { + return fmt.Errorf("empty profile ID") + } + if id != defaultProfileName && !IsValidProfileFilenameStem(id) { + return fmt.Errorf("invalid profile ID: %q", id) + } + + stateFile := filepath.Join(configDir, id.String()+".state.json") + if err := util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state); err != nil { + return fmt.Errorf("write profile state: %w", err) + } + + return nil +} + +// SetActiveProfileState writes the state file of whichever profile is active at +// call time. Use SetProfileState when the target profile is known. +func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error { activeProf, err := pm.GetActiveProfile() if err != nil { if errors.Is(err, ErrNoActiveProfile) { @@ -59,18 +82,7 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error { return fmt.Errorf("get active profile: %w", err) } - id := activeProf.ID - if id != defaultProfileName && !IsValidProfileFilenameStem(id) { - return fmt.Errorf("invalid active profile ID: %q", id) - } - - stateFile := filepath.Join(configDir, id.String()+".state.json") - err = util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state) - if err != nil { - return fmt.Errorf("write profile state: %w", err) - } - - return nil + return pm.SetProfileState(activeProf.ID, state) } // RemoveProfileState deletes the per-profile state file (which holds the diff --git a/client/ui/frontend/src/lib/connection.ts b/client/ui/frontend/src/lib/connection.ts index fca03fc87..cc0e67cb3 100644 --- a/client/ui/frontend/src/lib/connection.ts +++ b/client/ui/frontend/src/lib/connection.ts @@ -43,7 +43,12 @@ function buildSsoCancelPromise(state: SsoState, signal?: AbortSignal): Promise { @@ -56,7 +61,7 @@ async function runSsoLogin( // suspended, so a frontend-driven Up (a promise continuation) would not // fire until the user woke the window (e.g. hovering the tray icon). const waitPromise = Connection.WaitSSOLoginAndUp( - { userCode: result.userCode, hostname: "" }, + { userCode: result.userCode, hostname: "", profileId: result.profileId }, { profileName: "", username: "" }, ); diff --git a/client/ui/services/connection.go b/client/ui/services/connection.go index fae7ddd23..1069f8754 100644 --- a/client/ui/services/connection.go +++ b/client/ui/services/connection.go @@ -33,12 +33,21 @@ type LoginResult struct { UserCode string `json:"userCode"` VerificationURI string `json:"verificationUri"` VerificationURIComplete string `json:"verificationUriComplete"` + // ProfileID is the ID of the profile this login ran against, or "" when the + // caller named the profile itself and no ID was resolved. Pass it back in + // WaitSSOParams so the account email lands on this profile even if the + // active one changes during SSO. + ProfileID string `json:"profileId"` } // WaitSSOParams are the inputs to waitSSOLogin. type WaitSSOParams struct { UserCode string `json:"userCode"` Hostname string `json:"hostname"` + // ProfileID is the profile the login was started for, used to file the + // account email against it rather than against whichever profile is active + // when the flow returns. Optional: empty falls back to the active profile. + ProfileID string `json:"profileId"` } // UpParams selects the profile to bring up. @@ -77,11 +86,16 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err // Fall back to the daemon's active profile and the current OS user. profileName := p.ProfileName username := p.Username + // Only set when the daemon told us the ID. A caller-supplied ProfileName is + // a handle — a display name or an ID prefix resolve too — and the state file + // is named after the ID, so passing a handle on would name the wrong file. + profileID := "" if profileName == "" { if active, aerr := cli.GetActiveProfile(ctx, &proto.GetActiveProfileRequest{}); aerr == nil { // Address the active profile by ID (the daemon resolves it as a // handle); names can collide, the ID cannot. profileName = active.GetId() + profileID = profileName if username == "" { username = active.GetUsername() } @@ -122,6 +136,7 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err UserCode: resp.GetUserCode(), VerificationURI: resp.GetVerificationURI(), VerificationURIComplete: resp.GetVerificationURIComplete(), + ProfileID: profileID, }, nil } @@ -242,6 +257,31 @@ func (s *Connection) waitSSOLogin(ctx context.Context, p WaitSSOParams) (string, return "", s.classifyDaemonError(err) } log.Infof("SSO login completed, daemon reported success") + + // Persist the account email the same way the CLI does after its own + // WaitSSOLogin: the daemon returns it but cannot store it, since it runs as + // root and the per-profile state file is user-owned (see Logout below). + // Without this the profile has no email, so Profiles.List shows no account + // and later logins and session extends go out without a login_hint — + // leaving the IdP to guess which account was meant. + if email := resp.GetEmail(); email != "" { + state := &profilemanager.ProfileState{Email: email} + pm := profilemanager.NewProfileManager() + + // Against the profile the login was started for: SSO spans seconds of + // user interaction, and a profile switch in that window would otherwise + // file the email under the wrong profile. + if p.ProfileID != "" { + err = pm.SetProfileState(profilemanager.ID(p.ProfileID), state) + } else { + err = pm.SetActiveProfileState(state) + } + if err != nil { + // Non-fatal: the login itself succeeded. + log.Warnf("failed to store account email: %v", err) + } + } + return resp.GetEmail(), nil } diff --git a/client/ui/services/profile.go b/client/ui/services/profile.go index 09468c9df..5a9a0e68d 100644 --- a/client/ui/services/profile.go +++ b/client/ui/services/profile.go @@ -6,6 +6,8 @@ import ( "context" "os/user" + log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/proto" ) @@ -151,11 +153,31 @@ func (s *Profiles) Remove(ctx context.Context, p ProfileRef) error { if err != nil { return err } - _, err = cli.RemoveProfile(ctx, &proto.RemoveProfileRequest{ + resp, err := cli.RemoveProfile(ctx, &proto.RemoveProfileRequest{ ProfileName: p.ProfileName, Username: p.Username, }) - return err + if err != nil { + return err + } + + // The daemon deletes what it owns but runs as root, so it leaves the + // user-owned state file holding the account email behind (same split as + // Connection.Logout). Legacy profiles are keyed by name rather than by a + // generated ID, so a recreated profile of the same name would inherit the + // deleted one's email and offer it as the login_hint. + // + // Keyed on the ID the daemon resolved, not on the request handle: that may + // have been a display name or an ID prefix, which would name a different + // file (or none). + if id := resp.GetId(); id != "" { + if err := profilemanager.NewProfileManager().RemoveProfileState(id); err != nil { + // Non-fatal: the profile itself is gone. + log.Warnf("failed to remove profile state for %s: %v", id, err) + } + } + + return nil } // Rename changes a profile's display name. The on-disk ID is unaffected, so From 28197e6504418622436425e584d43eedeeb8ce11 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Mon, 3 Aug 2026 14:30:41 +0200 Subject: [PATCH 5/5] [client] Android - Create the Android fake IP manager lazily on DNS flag enable (#6989) The fake IP manager was only created at route manager construction, from the DNS feature flag fetched by the initial GetNetworkMap call. When the flag flipped to true mid-session, UpdateRoutes set useNewDNSRoute but never created the manager, so domain routes added after the flip got a DNS interceptor with a nil fake IP manager. internalDnatFw only checked for a firewall and GOOS, so the interceptor took the DNAT path and called GetFakeIP/AllocateFakeIP on the nil *fakeip.Manager. These methods lock m.mu first, which is a nil pointer dereference: the first DNS answer for such a route panicked and crashed the VPN service. The fake IP blocks (240.0.0.0/8 and its v6 pair) also never reached the TUN, since only the constructor registered them. Create the manager and its TUN routes from UpdateRoutes when the flag turns on, notify so the fake IP blocks get into the TUN without a client route change, and treat a nil manager as no internal DNAT. This is groundwork for removing the initial GetNetworkMap fetch, after which every startup goes through the flag-off-to-on transition. --- .../routemanager/dnsinterceptor/handler.go | 2 +- client/internal/routemanager/manager.go | 48 +++++++++++-------- .../routemanager/notifier/notifier_android.go | 1 + 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/client/internal/routemanager/dnsinterceptor/handler.go b/client/internal/routemanager/dnsinterceptor/handler.go index f92300bfd..d20f4b944 100644 --- a/client/internal/routemanager/dnsinterceptor/handler.go +++ b/client/internal/routemanager/dnsinterceptor/handler.go @@ -479,7 +479,7 @@ func (d *DnsInterceptor) removeDNATMappings(realPrefixes []netip.Prefix, logger // internalDnatFw checks if the firewall supports internal DNAT func (d *DnsInterceptor) internalDnatFw() (internalDNATer, bool) { - if d.firewall == nil || runtime.GOOS != "android" { + if d.firewall == nil || d.fakeIPManager == nil || runtime.GOOS != "android" { return nil, false } fw, ok := d.firewall.(internalDNATer) diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index 2ab7e2a85..0cb74fd45 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -165,31 +165,36 @@ func (m *DefaultManager) setupAndroidRoutes(config ManagerConfig) { routesForComparison := slices.Clone(cr) if config.DNSFeatureFlag { - m.fakeIPManager = fakeip.NewManager() - - v4ID := uuid.NewString() - fakeIPRoute := &route.Route{ - ID: route.ID(v4ID), - Network: m.fakeIPManager.GetFakeIPBlock(), - NetID: route.NetID(v4ID), - Peer: m.pubKey, - NetworkType: route.IPv4Network, - } - v6ID := uuid.NewString() - fakeIPv6Route := &route.Route{ - ID: route.ID(v6ID), - Network: m.fakeIPManager.GetFakeIPv6Block(), - NetID: route.NetID(v6ID), - Peer: m.pubKey, - NetworkType: route.IPv6Network, - } - cr = append(cr, fakeIPRoute, fakeIPv6Route) - m.notifier.SetFakeIPRoutes([]*route.Route{fakeIPRoute, fakeIPv6Route}) + cr = append(cr, m.enableFakeIPRoutes()...) } m.notifier.SetInitialClientRoutes(cr, routesForComparison) } +func (m *DefaultManager) enableFakeIPRoutes() []*route.Route { + m.fakeIPManager = fakeip.NewManager() + + v4ID := uuid.NewString() + fakeIPRoute := &route.Route{ + ID: route.ID(v4ID), + Network: m.fakeIPManager.GetFakeIPBlock(), + NetID: route.NetID(v4ID), + Peer: m.pubKey, + NetworkType: route.IPv4Network, + } + v6ID := uuid.NewString() + fakeIPv6Route := &route.Route{ + ID: route.ID(v6ID), + Network: m.fakeIPManager.GetFakeIPv6Block(), + NetID: route.NetID(v6ID), + Peer: m.pubKey, + NetworkType: route.IPv6Network, + } + fakeRoutes := []*route.Route{fakeIPRoute, fakeIPv6Route} + m.notifier.SetFakeIPRoutes(fakeRoutes) + return fakeRoutes +} + func (m *DefaultManager) setupRefCounters(useNoop bool) { var once sync.Once var wgIface *net.Interface @@ -464,6 +469,9 @@ func (m *DefaultManager) UpdateRoutes( var merr *multierror.Error if !m.disableClientRoutes { + if runtime.GOOS == "android" && useNewDNSRoute && m.fakeIPManager == nil { + m.enableFakeIPRoutes() + } // Update route selector based on management server's isSelected status m.updateRouteSelectorFromManagement(clientRoutes) diff --git a/client/internal/routemanager/notifier/notifier_android.go b/client/internal/routemanager/notifier/notifier_android.go index 49300dbb2..31f2a7aef 100644 --- a/client/internal/routemanager/notifier/notifier_android.go +++ b/client/internal/routemanager/notifier/notifier_android.go @@ -41,6 +41,7 @@ func (n *Notifier) SetInitialClientRoutes(initialRoutes []*route.Route, routesFo // SetFakeIPRoutes stores the fake IP routes to be included in every TUN rebuild. func (n *Notifier) SetFakeIPRoutes(routes []*route.Route) { n.fakeIPRoutes = routes + n.notify() } func (n *Notifier) OnNewRoutes(idMap route.HAMap) {