From cd22a9d4f1182e67934649b8e6403fffe389624e Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 12 Oct 2026 12:56:17 +0200 Subject: [PATCH] [management] Configure per-target HTTP access actions (#8116) Let HTTP targets inherit service authentication, bypass it, or block access. Validate and persist the action, expose it through the API, and deliver it to the proxy. Private services reject bypass. Validation: service/manager, store, and management-to-proxy integration race tests passed; scoped lint passed. --- .../reverseproxy/service/manager/manager.go | 71 ++++ .../service/manager/target_access_test.go | 196 +++++++++++ .../modules/reverseproxy/service/service.go | 159 ++++++--- .../reverseproxy/service/service_test.go | 312 ++++++++++++++++++ .../reverseproxy/service/target_access.go | 115 +++++++ .../server/store/sql_store_service_target.go | 9 +- .../server/store/sql_store_service_test.go | 2 + .../target_access_control_integration_test.go | 270 +++++++++++++++ shared/management/http/api/openapi.yml | 6 + shared/management/http/api/types.gen.go | 27 ++ 10 files changed, 1111 insertions(+), 56 deletions(-) create mode 100644 management/internals/modules/reverseproxy/service/manager/target_access_test.go create mode 100644 management/internals/modules/reverseproxy/service/target_access.go create mode 100644 proxy/target_access_control_integration_test.go diff --git a/management/internals/modules/reverseproxy/service/manager/manager.go b/management/internals/modules/reverseproxy/service/manager/manager.go index d1ed8d078..ebb52c74b 100644 --- a/management/internals/modules/reverseproxy/service/manager/manager.go +++ b/management/internals/modules/reverseproxy/service/manager/manager.go @@ -339,6 +339,9 @@ func (m *Manager) persistNewService(ctx context.Context, accountID string, svc * } return m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error { + if err := validateTargetAccessControl(svc); err != nil { + return err + } if err := m.validateServiceDomain(ctx, transaction, accountID, svc, svc.ProxyCluster); err != nil { return err } @@ -708,6 +711,12 @@ func (m *Manager) executeServiceUpdate(ctx context.Context, transaction store.St } m.preserveExistingAuthSecrets(service, existingService) + if err := preserveTargetAccessActions(service, existingService); err != nil { + return err + } + if err := validateTargetAccessControl(service); err != nil { + return err + } if err := validateHeaderAuthValues(service.Auth.HeaderAuths); err != nil { return err } @@ -745,6 +754,68 @@ func (m *Manager) validateServiceDomain(ctx context.Context, tx store.Store, acc return m.clusterDeriver.ValidateServiceDomain(ctx, tx, accountID, svc.Domain, cluster) } +func validateTargetAccessControl(svc *service.Service) error { + if !svc.HasTargetAccessControl() { + return nil + } + // Legacy requests may acquire an existing action during the update merge. + if err := svc.Validate(); err != nil { + return status.Errorf(status.InvalidArgument, "%s", err) + } + return nil +} + +func preserveTargetAccessActions(updated, existing *service.Service) error { + if !existing.HasTargetAccessControl() { + return nil + } + if slices.ContainsFunc(updated.Targets, func(target *service.Target) bool { + return target == nil + }) { + return status.Errorf(status.InvalidArgument, "target must not be nil") + } + if len(updated.Targets) > 0 && !slices.ContainsFunc(updated.Targets, func(target *service.Target) bool { + return !target.AccessActionProvided + }) { + return nil + } + byPath := make(map[string]*service.Target, len(existing.Targets)) + for _, target := range existing.Targets { + location := effectiveTargetPath(target) + if _, duplicate := byPath[location]; duplicate { + return status.Errorf(status.InvalidArgument, + "cannot preserve target access control for duplicate location %q", location) + } + byPath[location] = target + } + actionsProvided := false + for _, target := range updated.Targets { + actionsProvided = actionsProvided || target.AccessActionProvided + location := effectiveTargetPath(target) + if previous := byPath[location]; previous != nil && !target.AccessActionProvided { + target.AccessAction = previous.AccessAction + } + delete(byPath, location) + } + if actionsProvided { + return nil + } + for location, target := range byPath { + if target.AccessAction != "" && target.AccessAction != service.TargetAccessActionInherit { + return status.Errorf(status.InvalidArgument, + "include access_action when removing or changing controlled target location %q", location) + } + } + return nil +} + +func effectiveTargetPath(target *service.Target) string { + if target.Path == nil || *target.Path == "" { + return "/" + } + return *target.Path +} + // validateL4PortDiffOnClusterDiff checks if custom L4 ports are configured and validates port changes across clusters. // It ensures no port changes if custom ports are unsupported for a given cluster and protocol mode. // Returns an error if validation fails, otherwise returns nil. diff --git a/management/internals/modules/reverseproxy/service/manager/target_access_test.go b/management/internals/modules/reverseproxy/service/manager/target_access_test.go new file mode 100644 index 000000000..e87d0c64c --- /dev/null +++ b/management/internals/modules/reverseproxy/service/manager/target_access_test.go @@ -0,0 +1,196 @@ +package manager + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" + "github.com/netbirdio/netbird/management/server/store" +) + +func TestCreateServiceTargetAccessWithoutActiveProxies(t *testing.T) { + for _, action := range []rpservice.TargetAccessAction{ + rpservice.TargetAccessActionBlock, + rpservice.TargetAccessActionBypass, + } { + t.Run(string(action), func(t *testing.T) { + ctx := context.Background() + mgr, testStore, _ := setupL4Test(t, nil) + svc := &rpservice.Service{ + Name: "target access", Domain: "target.test.netbird.io", Mode: rpservice.ModeHTTP, + Enabled: true, + Targets: []*rpservice.Target{{ + AccountID: testAccountID, TargetId: testPeerID, + TargetType: rpservice.TargetTypePeer, Protocol: "http", Port: 8080, + Enabled: true, AccessAction: action, + }}, + } + created, err := mgr.CreateService(ctx, testAccountID, testUserID, svc) + require.NoError(t, err) + stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, created.ID) + require.NoError(t, err) + require.Len(t, stored.Targets, 1) + assert.Equal(t, action, stored.Targets[0].AccessAction, + "accepted target actions must survive persistence") + }) + } +} + +func TestUpdateServicePreservesOmittedTargetAccessAction(t *testing.T) { + ctx := context.Background() + mgr, testStore, _ := setupL4Test(t, nil) + existing := seedService(t, testStore, "protected", "http", "target.test.netbird.io", testCluster, 0) + existing.Targets[0].AccessAction = rpservice.TargetAccessActionBlock + require.NoError(t, testStore.UpdateService(ctx, existing)) + + updated := existing.Copy() + updated.Name = "renamed by an older client" + updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit + updated.Targets[0].AccessActionProvided = false + rootPath := "/" + updated.Targets[0].Path = &rootPath + _, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.NoError(t, err) + stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + assert.Equal(t, updated.Name, stored.Name, "legacy updates must still apply unrelated edits") + assert.Equal(t, rpservice.TargetAccessActionBlock, stored.Targets[0].AccessAction, + "an omitted action must preserve the stored block across equivalent root locations") + + emptyPath := "" + updated.Targets[0].Path = &emptyPath + updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit + _, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.NoError(t, err) + stored, err = testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + assert.Equal(t, rpservice.TargetAccessActionBlock, stored.Targets[0].AccessAction, + "an older client's empty root location must preserve the stored block") + + updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit + updated.Targets[0].AccessActionProvided = true + _, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.NoError(t, err) + stored, err = testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + assert.Equal(t, rpservice.TargetAccessActionInherit, stored.Targets[0].AccessAction, + "explicit inherit must clear the stored target block") +} + +func TestUpdateServiceActivatesPreservedTargetAccess(t *testing.T) { + ctx := context.Background() + mgr, testStore, _ := setupL4Test(t, nil) + existing := seedService(t, testStore, "protected", "http", "target.test.netbird.io", testCluster, 0) + existing.Enabled = false + existing.Targets[0].AccessAction = rpservice.TargetAccessActionBlock + require.NoError(t, testStore.UpdateService(ctx, existing)) + + updated := existing.Copy() + updated.Enabled = true + updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit + updated.Targets[0].AccessActionProvided = false + _, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.NoError(t, err) + stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + assert.True(t, stored.Enabled, "activating target access must not require an active proxy") + assert.Equal(t, rpservice.TargetAccessActionBlock, stored.Targets[0].AccessAction, + "activation must retain an omitted stored access action") +} + +func TestUpdateServiceValidatesPreservedTargetAccess(t *testing.T) { + ctx := context.Background() + mgr, testStore, _ := setupL4Test(t, nil) + existing := seedService(t, testStore, "public", "http", "target.test.netbird.io", testCluster, 0) + existing.Targets[0].AccessAction = rpservice.TargetAccessActionBypass + require.NoError(t, testStore.UpdateService(ctx, existing)) + + updated := existing.Copy() + updated.Private = true + updated.AccessGroups = []string{testGroupID} + updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit + updated.Targets[0].AccessActionProvided = false + _, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.ErrorContains(t, err, "bypass access_action is not supported for private services") + + stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + assert.False(t, stored.Private, "an invalid preserved bypass must prevent the private-service update") + assert.Equal(t, rpservice.TargetAccessActionBypass, stored.Targets[0].AccessAction, + "a rejected update must retain the existing target action") +} + +func TestUpdateServiceLegacyClientCannotRemoveControlledPath(t *testing.T) { + for _, operation := range []string{"rename", "delete"} { + t.Run(operation, func(t *testing.T) { + ctx := context.Background() + mgr, testStore, _ := setupL4Test(t, nil) + existing := seedService(t, testStore, "protected", "http", "target.test.netbird.io", testCluster, 0) + blockedPath := "/blocked" + existing.Targets = append(existing.Targets, &rpservice.Target{ + AccountID: testAccountID, TargetId: testPeerID, TargetType: rpservice.TargetTypePeer, + Protocol: "http", Port: 8080, Enabled: true, Path: &blockedPath, + AccessAction: rpservice.TargetAccessActionBlock, + }) + require.NoError(t, testStore.UpdateService(ctx, existing)) + + updated := existing.Copy() + updated.Targets[1].AccessAction = rpservice.TargetAccessActionInherit + if operation == "rename" { + renamedPath := "/renamed" + updated.Targets[1].Path = &renamedPath + } else { + updated.Targets = updated.Targets[:1] + } + _, err := mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.Error(t, err) + stored, err := testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + require.Len(t, stored.Targets, 2, "a legacy structural edit must retain the controlled target") + var blocked *rpservice.Target + for _, target := range stored.Targets { + if effectiveTargetPath(target) == blockedPath { + blocked = target + } + } + require.NotNil(t, blocked, "the original controlled location must still exist") + assert.Equal(t, rpservice.TargetAccessActionBlock, blocked.AccessAction, + "a rejected legacy edit must not erase the block action") + + updated.Targets[0].AccessAction = rpservice.TargetAccessActionInherit + updated.Targets[0].AccessActionProvided = true + _, err = mgr.UpdateService(ctx, testAccountID, testUserID, updated) + require.NoError(t, err) + stored, err = testStore.GetServiceByID(ctx, store.LockingStrengthNone, testAccountID, existing.ID) + require.NoError(t, err) + assert.False(t, stored.HasTargetAccessControl(), + "a feature-aware client must be able to intentionally remove or rename the controlled path") + }) + } +} + +func TestPreserveTargetAccessActionsExplicitUpdate(t *testing.T) { + existing := &rpservice.Service{Targets: []*rpservice.Target{ + {Enabled: true, AccessAction: rpservice.TargetAccessActionBlock}, + {Enabled: false, AccessAction: rpservice.TargetAccessActionInherit}, + }} + updated := &rpservice.Service{Targets: []*rpservice.Target{ + {Enabled: true, AccessAction: rpservice.TargetAccessActionBypass, AccessActionProvided: true}, + {Enabled: false, AccessAction: rpservice.TargetAccessActionInherit, AccessActionProvided: true}, + }} + require.NoError(t, preserveTargetAccessActions(updated, existing)) + assert.Equal(t, rpservice.TargetAccessActionBypass, updated.Targets[0].AccessAction, + "explicit actions must not need an ambiguous location-based legacy merge") + + updated.Targets[0].AccessActionProvided = false + assert.Error(t, preserveTargetAccessActions(updated, existing), + "an omitted action must be rejected when its stored location is ambiguous") + + existing.Targets = existing.Targets[:1] + updated.Targets = nil + assert.Error(t, preserveTargetAccessActions(updated, existing), + "an empty target list must not count as an explicit removal of the stored block") +} diff --git a/management/internals/modules/reverseproxy/service/service.go b/management/internals/modules/reverseproxy/service/service.go index ef92fbbf4..de47f1239 100644 --- a/management/internals/modules/reverseproxy/service/service.go +++ b/management/internals/modules/reverseproxy/service/service.go @@ -116,18 +116,20 @@ type MiddlewareConfig struct { } type Target struct { - ID uint `gorm:"primaryKey" json:"-"` - AccountID string `gorm:"index:idx_target_account;not null" json:"-"` - ServiceID string `gorm:"index:idx_service_targets;not null" json:"-"` - Path *string `json:"path,omitempty"` - Host string `json:"host"` - Port uint16 `gorm:"index:idx_target_port" json:"port"` - Protocol string `gorm:"index:idx_target_protocol" json:"protocol"` - TargetId string `gorm:"index:idx_target_id" json:"target_id"` - TargetType TargetType `gorm:"index:idx_target_type" json:"target_type"` - Enabled bool `gorm:"index:idx_target_enabled" json:"enabled"` - Options TargetOptions `gorm:"embedded" json:"options"` - ProxyProtocol bool `json:"proxy_protocol"` + ID uint `gorm:"primaryKey" json:"-"` + AccountID string `gorm:"index:idx_target_account;not null" json:"-"` + ServiceID string `gorm:"index:idx_service_targets;not null" json:"-"` + Path *string `json:"path,omitempty"` + Host string `json:"host"` + Port uint16 `gorm:"index:idx_target_port" json:"port"` + Protocol string `gorm:"index:idx_target_protocol" json:"protocol"` + TargetId string `gorm:"index:idx_target_id" json:"target_id"` + TargetType TargetType `gorm:"index:idx_target_type" json:"target_type"` + Enabled bool `gorm:"index:idx_target_enabled" json:"enabled"` + AccessAction TargetAccessAction `gorm:"type:varchar(16);not null;default:'inherit'" json:"access_action"` + AccessActionProvided bool `gorm:"-" json:"-"` + Options TargetOptions `gorm:"embedded" json:"options"` + ProxyProtocol bool `json:"proxy_protocol"` } type PasswordAuthConfig struct { @@ -309,14 +311,16 @@ func (s *Service) ToAPIResponse() *api.Service { // Convert internal targets to API targets apiTargets := make([]api.ServiceTarget, 0, len(s.Targets)) for _, target := range s.Targets { + accessAction := api.ServiceTargetAccessAction(target.effectiveAccessAction()) st := api.ServiceTarget{ - Path: target.Path, - Host: &target.Host, - Port: int(target.Port), - Protocol: api.ServiceTargetProtocol(target.Protocol), - TargetId: target.TargetId, - TargetType: api.ServiceTargetTargetType(target.TargetType), - Enabled: target.Enabled && !s.Terminated, + Path: target.Path, + Host: &target.Host, + Port: int(target.Port), + Protocol: api.ServiceTargetProtocol(target.Protocol), + TargetId: target.TargetId, + TargetType: api.ServiceTargetTargetType(target.TargetType), + Enabled: target.Enabled && !s.Terminated, + AccessAction: &accessAction, } opts := targetOptionsToAPI(target.Options) if opts == nil { @@ -463,8 +467,9 @@ func (s *Service) buildPathMappings() []*proto.PathMapping { } pm := &proto.PathMapping{ - Path: path, - Target: targetURL.String(), + Path: path, + Target: targetURL.String(), + AccessAction: targetAccessActionToProto(target.effectiveAccessAction()), } pm.Options = targetOptionsToProto(target.Options) pathMappings = append(pathMappings, pm) @@ -727,13 +732,18 @@ func targetsFromAPI(accountID string, apiTargetsPtr *[]api.ServiceTarget) ([]*Ta targets := make([]*Target, 0, len(apiTargets)) for i, apiTarget := range apiTargets { target := &Target{ - AccountID: accountID, - Path: apiTarget.Path, - Port: uint16(apiTarget.Port), //nolint:gosec // validated by API layer - Protocol: string(apiTarget.Protocol), - TargetId: apiTarget.TargetId, - TargetType: TargetType(apiTarget.TargetType), - Enabled: apiTarget.Enabled, + AccountID: accountID, + Path: apiTarget.Path, + Port: uint16(apiTarget.Port), //nolint:gosec // validated by API layer + Protocol: string(apiTarget.Protocol), + TargetId: apiTarget.TargetId, + TargetType: TargetType(apiTarget.TargetType), + Enabled: apiTarget.Enabled, + AccessAction: TargetAccessActionInherit, + } + if apiTarget.AccessAction != nil { + target.AccessAction = TargetAccessAction(*apiTarget.AccessAction) + target.AccessActionProvided = true } if apiTarget.Host != nil { target.Host = *apiTarget.Host @@ -953,41 +963,70 @@ func (s *Service) validateTLSMode() error { } func (s *Service) validateHTTPTargets() error { + strictPaths := false for i, target := range s.Targets { - switch target.TargetType { - case TargetTypePeer, TargetTypeHost, TargetTypeDomain: - // Host is normally overwritten by replaceHostByLookup with the - // resolved peer IP / resource address; operator-supplied values - // are honored only when DirectUpstream is set. Validate the - // override here so misconfigured hosts fail fast at API time. - if err := validateDirectUpstreamHost(i, target); err != nil { - return err - } - case TargetTypeSubnet: - if err := validateSubnetTarget(i, target); err != nil { - return err - } - case TargetTypeCluster: - if err := validateClusterTarget(i, target); err != nil { - return err - } - default: - return fmt.Errorf("target %d has invalid target_type %q", i, target.TargetType) - } - if target.TargetId == "" { - return fmt.Errorf("target %d has empty target_id", i) - } - if target.ProxyProtocol { - return fmt.Errorf("target %d: proxy_protocol is not supported for HTTP services", i) - } - if err := validateTargetOptions(i, &target.Options); err != nil { + if err := s.validateHTTPTarget(i, target); err != nil { return err } + if target.Enabled && target.effectiveAccessAction() != TargetAccessActionInherit { + strictPaths = true + } + } + if !strictPaths { + return nil + } + + paths := make(map[string]int, len(s.Targets)) + for i, target := range s.Targets { + if !target.Enabled { + continue + } + normalizedPath := normalizedTargetPath(target.Path) + if previous, ok := paths[normalizedPath]; ok { + return fmt.Errorf("targets %d and %d have duplicate path %q", previous, i, normalizedPath) + } + paths[normalizedPath] = i } return nil } +func (s *Service) validateHTTPTarget(idx int, target *Target) error { + if target == nil { + return fmt.Errorf("target %d is nil", idx) + } + if err := validateTargetAccessAction(idx, target, s.Private); err != nil { + return err + } + switch target.TargetType { + case TargetTypePeer, TargetTypeHost, TargetTypeDomain: + // Host is normally overwritten by replaceHostByLookup with the + // resolved peer IP / resource address; operator-supplied values + // are honored only when DirectUpstream is set. Validate the + // override here so misconfigured hosts fail fast at API time. + if err := validateDirectUpstreamHost(idx, target); err != nil { + return err + } + case TargetTypeSubnet: + if err := validateSubnetTarget(idx, target); err != nil { + return err + } + case TargetTypeCluster: + if err := validateClusterTarget(idx, target); err != nil { + return err + } + default: + return fmt.Errorf("target %d has invalid target_type %q", idx, target.TargetType) + } + if target.TargetId == "" { + return fmt.Errorf("target %d has empty target_id", idx) + } + if target.ProxyProtocol { + return fmt.Errorf("target %d: proxy_protocol is not supported for HTTP services", idx) + } + return validateTargetOptions(idx, &target.Options) +} + func validateSubnetTarget(idx int, target *Target) error { host := strings.TrimSpace(target.Host) if host == "" { @@ -1063,10 +1102,20 @@ func validateDirectUpstreamHost(idx int, target *Target) error { } func (s *Service) validateL4Target(target *Target) error { + if target == nil { + return errors.New("target 0 is nil") + } // L4 services have a single target; per-target disable is meaningless // (use the service-level Enabled flag instead). Force it on so that // buildPathMappings always includes the target in the proto. target.Enabled = true + action, err := validatedTargetAccessAction(0, target) + if err != nil { + return err + } + if action != TargetAccessActionInherit { + return errors.New("access_action is only supported for HTTP services") + } if target.TargetId == "" { return errors.New("target_id is required for L4 services") diff --git a/management/internals/modules/reverseproxy/service/service_test.go b/management/internals/modules/reverseproxy/service/service_test.go index 84343bcdd..756a5a915 100644 --- a/management/internals/modules/reverseproxy/service/service_test.go +++ b/management/internals/modules/reverseproxy/service/service_test.go @@ -62,9 +62,11 @@ func TestValidate_InvalidTargetType(t *testing.T) { func TestValidate_ResourceTarget(t *testing.T) { rp := validProxy() + resourcePath := "/resource" rp.Targets = append(rp.Targets, &Target{ TargetId: "resource-1", TargetType: TargetTypeHost, + Path: &resourcePath, Host: "example.org", Port: 443, Protocol: "https", @@ -73,6 +75,232 @@ func TestValidate_ResourceTarget(t *testing.T) { require.NoError(t, rp.Validate()) } +func TestValidateTargetAccessAction(t *testing.T) { + t.Run("valid actions", func(t *testing.T) { + for _, action := range []TargetAccessAction{ + TargetAccessActionInherit, + TargetAccessActionBypass, + TargetAccessActionBlock, + } { + rp := validProxy() + rp.Targets[0].AccessAction = action + require.NoError(t, rp.Validate(), "action %q should be valid", action) + } + }) + + t.Run("unknown action", func(t *testing.T) { + rp := validProxy() + rp.Targets[0].AccessAction = "allow" + assert.ErrorContains(t, rp.Validate(), `unknown access_action "allow"`) + }) + + t.Run("explicit empty action", func(t *testing.T) { + rp := validProxy() + rp.Targets[0].AccessActionProvided = true + assert.ErrorContains(t, rp.Validate(), `unknown access_action ""`) + }) + + t.Run("private bypass", func(t *testing.T) { + rp := validProxy() + rp.Private = true + rp.AccessGroups = []string{"group-1"} + rp.Targets[0].AccessAction = TargetAccessActionBypass + assert.ErrorContains(t, rp.Validate(), "not supported for private services") + }) + + t.Run("Agent Network bypass", func(t *testing.T) { + rp := validProxy() + rp.Targets[0].AccessAction = TargetAccessActionBypass + rp.Targets[0].Options.AgentNetwork = true + assert.ErrorContains(t, rp.Validate(), "not supported for Agent Network targets") + }) + + t.Run("block remains available to private services", func(t *testing.T) { + rp := validProxy() + rp.Private = true + rp.AccessGroups = []string{"group-1"} + rp.Targets[0].AccessAction = TargetAccessActionBlock + require.NoError(t, rp.Validate()) + }) + + t.Run("non HTTP service", func(t *testing.T) { + rp := validProxy() + rp.Mode = ModeTCP + rp.ListenPort = 8080 + rp.Targets[0].Protocol = TargetProtoTCP + rp.Targets[0].AccessAction = TargetAccessActionBlock + assert.ErrorContains(t, rp.Validate(), "only supported for HTTP services") + }) + + t.Run("nil non HTTP target", func(t *testing.T) { + rp := validProxy() + rp.Mode = ModeTCP + rp.ListenPort = 8080 + rp.Targets[0] = nil + assert.ErrorContains(t, rp.Validate(), "target 0 is nil") + }) +} + +func TestValidateTargetAccessActionPath(t *testing.T) { + for _, value := range []string{"api", "/api/../admin", "/api/./admin", "/api//admin", "/api%2Fadmin", `/api\admin`, "/api?x=1", "/api#fragment", "/api;admin", "/api/\x00"} { + t.Run(fmt.Sprintf("path_%q", value), func(t *testing.T) { + rp := validProxy() + rp.Targets[0].Path = &value + rp.Targets[0].AccessAction = TargetAccessActionBypass + assert.ErrorContains(t, rp.Validate(), "access_action path") + }) + } + + t.Run("nil path is canonical root", func(t *testing.T) { + rp := validProxy() + rp.Targets[0].AccessAction = TargetAccessActionBlock + require.NoError(t, rp.Validate()) + }) + + t.Run("empty path is legacy root", func(t *testing.T) { + value := "" + rp := validProxy() + rp.Targets[0].Path = &value + rp.Targets[0].AccessAction = TargetAccessActionBlock + require.NoError(t, rp.Validate()) + }) + + t.Run("trailing slash is a distinct valid prefix", func(t *testing.T) { + value := "/api/" + rp := validProxy() + rp.Targets[0].Path = &value + rp.Targets[0].AccessAction = TargetAccessActionBypass + require.NoError(t, rp.Validate()) + }) +} + +func TestValidateDuplicateEnabledTargetPaths(t *testing.T) { + emptyPath := "" + rootPath := "/" + rootAliases := []struct { + name string + path *string + }{ + {name: "nil", path: nil}, + {name: "empty", path: &emptyPath}, + {name: "slash", path: &rootPath}, + } + + for _, alias := range rootAliases { + t.Run("legacy all-inherit "+alias.name, func(t *testing.T) { + rp := validProxy() + rp.Targets = append(rp.Targets, &Target{ + TargetId: "peer-2", + TargetType: TargetTypePeer, + Path: alias.path, + Host: "10.0.0.2", + Port: 443, + Protocol: TargetProtoHTTPS, + Enabled: true, + }) + require.NoError(t, rp.Validate(), "legacy all-inherit path and protocol twins must remain valid") + }) + + t.Run("access-controlled "+alias.name, func(t *testing.T) { + rp := validProxy() + rp.Targets = append(rp.Targets, &Target{ + TargetId: "peer-2", + TargetType: TargetTypePeer, + Path: alias.path, + Host: "10.0.0.2", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: true, + AccessAction: TargetAccessActionBlock, + }) + assert.ErrorContains(t, rp.Validate(), `duplicate path "/"`) + }) + } + + t.Run("access action elsewhere enables strict paths", func(t *testing.T) { + publicPath := "/public" + rp := validProxy() + rp.Targets = append(rp.Targets, + &Target{ + TargetId: "peer-2", + TargetType: TargetTypePeer, + Host: "10.0.0.2", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: true, + }, + &Target{ + TargetId: "peer-3", + TargetType: TargetTypePeer, + Path: &publicPath, + Host: "10.0.0.3", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: true, + AccessAction: TargetAccessActionBypass, + }, + ) + assert.ErrorContains(t, rp.Validate(), `duplicate path "/"`) + }) + + t.Run("disabled action preserves legacy validation", func(t *testing.T) { + publicPath := "/public" + rp := validProxy() + rp.Targets = append(rp.Targets, + &Target{ + TargetId: "peer-2", + TargetType: TargetTypePeer, + Host: "10.0.0.2", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: true, + }, + &Target{ + TargetId: "peer-3", + TargetType: TargetTypePeer, + Path: &publicPath, + Host: "10.0.0.3", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: false, + AccessAction: TargetAccessActionBlock, + }, + ) + require.NoError(t, rp.Validate()) + }) + + t.Run("disabled target does not collide", func(t *testing.T) { + rp := validProxy() + rp.Targets = append(rp.Targets, &Target{ + TargetId: "peer-2", + TargetType: TargetTypePeer, + Host: "10.0.0.2", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: false, + }) + require.NoError(t, rp.Validate()) + }) + + t.Run("trailing slash remains distinct", func(t *testing.T) { + apiPath := "/api" + apiSlashPath := "/api/" + rp := validProxy() + rp.Targets[0].Path = &apiPath + rp.Targets = append(rp.Targets, &Target{ + TargetId: "peer-2", + TargetType: TargetTypePeer, + Path: &apiSlashPath, + Host: "10.0.0.2", + Port: 80, + Protocol: TargetProtoHTTP, + Enabled: true, + AccessAction: TargetAccessActionBlock, + }) + require.NoError(t, rp.Validate()) + }) +} + func TestValidate_MultipleTargetsOneInvalid(t *testing.T) { rp := validProxy() rp.Targets = append(rp.Targets, &Target{ @@ -308,6 +536,90 @@ func TestToProtoMapping_TargetOptions(t *testing.T) { assert.Equal(t, int64(30), opts.RequestTimeout.Seconds) } +func TestTargetAccessActionConversions(t *testing.T) { + t.Run("API presence", func(t *testing.T) { + bypass := api.ServiceTargetAccessActionBypass + empty := api.ServiceTargetAccessAction("") + apiTargets := []api.ServiceTarget{ + { + TargetId: "peer-1", + TargetType: api.ServiceTargetTargetTypePeer, + Protocol: api.ServiceTargetProtocolHttp, + Port: 80, + Enabled: true, + }, + { + TargetId: "peer-2", + TargetType: api.ServiceTargetTargetTypePeer, + Protocol: api.ServiceTargetProtocolHttp, + Port: 80, + Enabled: true, + AccessAction: &bypass, + }, + { + TargetId: "peer-3", + TargetType: api.ServiceTargetTargetTypePeer, + Protocol: api.ServiceTargetProtocolHttp, + Port: 80, + Enabled: true, + AccessAction: &empty, + }, + } + + targets, err := targetsFromAPI("account-1", &apiTargets) + require.NoError(t, err) + require.Len(t, targets, 3) + assert.Equal(t, TargetAccessActionInherit, targets[0].AccessAction) + assert.False(t, targets[0].AccessActionProvided) + assert.Equal(t, TargetAccessActionBypass, targets[1].AccessAction) + assert.True(t, targets[1].AccessActionProvided) + assert.Empty(t, targets[2].AccessAction) + assert.True(t, targets[2].AccessActionProvided) + + rp := &Service{Name: "test", Domain: "example.com", Targets: []*Target{targets[2]}} + assert.ErrorContains(t, rp.Validate(), `unknown access_action ""`) + }) + + t.Run("API response", func(t *testing.T) { + rp := validProxy() + rp.Targets[0].AccessAction = TargetAccessActionBlock + response := rp.ToAPIResponse() + require.Len(t, response.Targets, 1) + require.NotNil(t, response.Targets[0].AccessAction) + assert.Equal(t, api.ServiceTargetAccessActionBlock, *response.Targets[0].AccessAction) + }) + + t.Run("proto mapping", func(t *testing.T) { + root := "/" + bypassPath := "/bypass" + blockPath := "/block" + rp := &Service{ + ID: "svc-1", + AccountID: "acc-1", + Domain: "example.com", + Targets: []*Target{ + {Path: &root, Host: "10.0.0.1", Port: 80, Protocol: TargetProtoHTTP, Enabled: true}, + {Path: &bypassPath, Host: "10.0.0.1", Port: 80, Protocol: TargetProtoHTTP, Enabled: true, AccessAction: TargetAccessActionBypass}, + {Path: &blockPath, Host: "10.0.0.1", Port: 80, Protocol: TargetProtoHTTP, Enabled: true, AccessAction: TargetAccessActionBlock}, + }, + } + + mapping := rp.ToProtoMapping(Create, "token", proxy.OIDCValidationConfig{}) + require.Len(t, mapping.Path, 3) + assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_INHERIT, mapping.Path[0].GetAccessAction()) + assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_BYPASS, mapping.Path[1].GetAccessAction()) + assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK, mapping.Path[2].GetAccessAction()) + }) +} + +func TestHasTargetAccessControl(t *testing.T) { + rp := validProxy() + assert.False(t, rp.HasTargetAccessControl()) + + rp.Targets[0].AccessAction = TargetAccessActionBypass + assert.True(t, rp.HasTargetAccessControl()) +} + // TestToProtoMapping_AllowedGroupIds covers the list the proxy gates session // cookies on: without it the proxy can only check a cookie's signature, which // makes a token minted for a user outside the groups a bearer credential. diff --git a/management/internals/modules/reverseproxy/service/target_access.go b/management/internals/modules/reverseproxy/service/target_access.go new file mode 100644 index 000000000..bd2d717fe --- /dev/null +++ b/management/internals/modules/reverseproxy/service/target_access.go @@ -0,0 +1,115 @@ +package service + +import ( + "fmt" + "strings" + "unicode" + "unicode/utf8" + + "github.com/netbirdio/netbird/shared/management/proto" +) + +// TargetAccessAction controls authentication for a selected HTTP target. +type TargetAccessAction string + +const ( + TargetAccessActionInherit TargetAccessAction = "inherit" + TargetAccessActionBypass TargetAccessAction = "bypass" + TargetAccessActionBlock TargetAccessAction = "block" +) + +func (t *Target) effectiveAccessAction() TargetAccessAction { + if t.AccessAction == "" { + return TargetAccessActionInherit + } + return t.AccessAction +} + +func targetAccessActionToProto(action TargetAccessAction) proto.TargetAccessAction { + switch action { + case TargetAccessActionInherit: + return proto.TargetAccessAction_TARGET_ACCESS_ACTION_INHERIT + case TargetAccessActionBypass: + return proto.TargetAccessAction_TARGET_ACCESS_ACTION_BYPASS + case TargetAccessActionBlock: + return proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK + default: + return proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK + } +} + +// HasTargetAccessControl reports whether any target overrides service authentication. +func (s *Service) HasTargetAccessControl() bool { + for _, target := range s.Targets { + if target != nil && target.effectiveAccessAction() != TargetAccessActionInherit { + return true + } + } + return false +} + +func validateTargetAccessAction(idx int, target *Target, private bool) error { + action, err := validatedTargetAccessAction(idx, target) + if err != nil { + return err + } + if action == TargetAccessActionBypass { + if private { + return fmt.Errorf("target %d: bypass access_action is not supported for private services", idx) + } + if target.Options.AgentNetwork { + return fmt.Errorf("target %d: bypass access_action is not supported for Agent Network targets", idx) + } + } + + if action == TargetAccessActionInherit { + return nil + } + return validateAccessActionPath(idx, target.Path) +} + +func validatedTargetAccessAction(idx int, target *Target) (TargetAccessAction, error) { + if target.AccessActionProvided && target.AccessAction == "" { + return "", fmt.Errorf("target %d: unknown access_action %q", idx, target.AccessAction) + } + action := target.effectiveAccessAction() + switch action { + case TargetAccessActionInherit, TargetAccessActionBypass, TargetAccessActionBlock: + return action, nil + default: + return "", fmt.Errorf("target %d: unknown access_action %q", idx, target.AccessAction) + } +} + +func normalizedTargetPath(configured *string) string { + if configured == nil || *configured == "" { + return "/" + } + return *configured +} + +func validateAccessActionPath(idx int, configured *string) error { + if configured == nil { + return nil + } + value := *configured + if value == "" { + return nil + } + if !strings.HasPrefix(value, "/") { + return fmt.Errorf("target %d: access_action path %q must start with /", idx, value) + } + if !utf8.ValidString(value) || strings.ContainsAny(value, "%\\?#;") || strings.IndexFunc(value, unicode.IsControl) >= 0 { + return fmt.Errorf("target %d: access_action path %q contains invalid characters", idx, value) + } + segments := strings.Split(strings.TrimPrefix(value, "/"), "/") + for i, segment := range segments { + if segment == "." || segment == ".." { + return fmt.Errorf("target %d: access_action path %q is not canonical", idx, value) + } + if segment == "" && value != "/" && i != len(segments)-1 { + return fmt.Errorf("target %d: access_action path %q is not canonical", idx, value) + } + } + return nil +} diff --git a/management/server/store/sql_store_service_target.go b/management/server/store/sql_store_service_target.go index 5c2b99510..0f7429880 100644 --- a/management/server/store/sql_store_service_target.go +++ b/management/server/store/sql_store_service_target.go @@ -18,7 +18,7 @@ import ( ) const targetSelectColumns = `id, account_id, service_id, path, host, port, protocol, - target_id, target_type, enabled, proxy_protocol, + target_id, target_type, enabled, access_action, proxy_protocol, skip_tls_verify, request_timeout, session_idle_timeout, path_rewrite, custom_headers, direct_upstream, middlewares, capture_max_request_bytes, capture_max_response_bytes, capture_content_types, agent_network, disable_access_log` @@ -37,6 +37,7 @@ func (s *SqlStore) getServiceTargets(ctx context.Context, serviceIDs []string) ( func scanTarget(row pgx.CollectableRow) (*rpservice.Target, error) { var t rpservice.Target var path sql.NullString + var accessAction sql.NullString var pathRewrite sql.NullString var proxyProtocol, skipTLSVerify, directUpstream, agentNetwork, disableAccessLog sql.NullBool var requestTimeout, sessionIdleTimeout, captureMaxRequestBytes, captureMaxResponseBytes sql.NullInt64 @@ -52,6 +53,7 @@ func scanTarget(row pgx.CollectableRow) (*rpservice.Target, error) { &t.TargetId, &t.TargetType, &t.Enabled, + &accessAction, &proxyProtocol, &skipTLSVerify, &requestTimeout, @@ -72,6 +74,11 @@ func scanTarget(row pgx.CollectableRow) (*rpservice.Target, error) { if path.Valid { t.Path = &path.String } + if accessAction.Valid && accessAction.String != "" { + t.AccessAction = rpservice.TargetAccessAction(accessAction.String) + } else { + t.AccessAction = rpservice.TargetAccessActionInherit + } t.ProxyProtocol = proxyProtocol.Bool t.Options.SkipTLSVerify = skipTLSVerify.Bool diff --git a/management/server/store/sql_store_service_test.go b/management/server/store/sql_store_service_test.go index 0e14fbdab..f71b8c97a 100644 --- a/management/server/store/sql_store_service_test.go +++ b/management/server/store/sql_store_service_test.go @@ -87,6 +87,7 @@ func TestSqlStore_GetAccount_ServiceTargetOptionsRoundtrip(t *testing.T) { Protocol: "http", TargetId: "tgt-1", Enabled: true, + AccessAction: rpservice.TargetAccessActionBlock, ProxyProtocol: true, Options: rpservice.TargetOptions{ SkipTLSVerify: true, @@ -119,6 +120,7 @@ func TestSqlStore_GetAccount_ServiceTargetOptionsRoundtrip(t *testing.T) { require.Len(t, got.Targets, 1) tg := got.Targets[0] + assert.Equal(t, rpservice.TargetAccessActionBlock, tg.AccessAction, "target access action") assert.True(t, tg.ProxyProtocol, "target proxy protocol") assert.True(t, tg.Options.SkipTLSVerify, "options skip TLS verify") assert.Equal(t, 30*time.Second, tg.Options.RequestTimeout, "options request timeout") diff --git a/proxy/target_access_control_integration_test.go b/proxy/target_access_control_integration_test.go new file mode 100644 index 000000000..82fc6d67a --- /dev/null +++ b/proxy/target_access_control_integration_test.go @@ -0,0 +1,270 @@ +package proxy + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "encoding/base64" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + + nbproxy "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" + "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" + "github.com/netbirdio/netbird/management/server/store" + "github.com/netbirdio/netbird/shared/hash/argon2id" + "github.com/netbirdio/netbird/shared/management/proto" +) + +func targetAccessService(t *testing.T, upstreamURL string) *service.Service { + t.Helper() + + u, err := url.Parse(upstreamURL) + require.NoError(t, err) + host, portText, err := net.SplitHostPort(u.Host) + require.NoError(t, err) + port, err := strconv.ParseUint(portText, 10, 16) + require.NoError(t, err) + + headerHash, err := argon2id.Hash(targetAccessSecret) + require.NoError(t, err) + publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + + newTarget := func(path string, action service.TargetAccessAction) *service.Target { + return &service.Target{ + Path: strPtr(path), + Host: host, + Port: uint16(port), + Protocol: u.Scheme, + TargetId: "target-access-upstream", + TargetType: service.TargetTypeCluster, + Enabled: true, + AccessAction: action, + Options: service.TargetOptions{ + DirectUpstream: true, + }, + } + } + + return &service.Service{ + ID: "target-access-service", + AccountID: targetAccessAccountID, + Name: "Target Access Control", + Domain: targetAccessDomain, + ProxyCluster: targetAccessCluster, + Enabled: true, + SessionPrivateKey: base64.StdEncoding.EncodeToString(privateKey), + SessionPublicKey: base64.StdEncoding.EncodeToString(publicKey), + Targets: []*service.Target{ + newTarget("/", service.TargetAccessActionInherit), + newTarget("/public", service.TargetAccessActionBypass), + newTarget("/blocked", service.TargetAccessActionBlock), + }, + Auth: service.AuthConfig{HeaderAuths: []*service.HeaderAuthConfig{{ + Enabled: true, + Header: targetAccessHeader, + Value: headerHash, + }}}, + Restrictions: service.AccessRestrictions{ + BlockedCIDRs: []string{"203.0.113.0/24"}, + }, + } +} + +func privateServiceCapabilities() *proto.ProxyCapabilities { + supported := true + return &proto.ProxyCapabilities{ + SupportsPrivateService: &supported, + } +} + +func targetAccessMappingStream(t *testing.T, setup *integrationTestSetup, proxyID string, caps *proto.ProxyCapabilities) proto.ProxyService_GetMappingUpdateClient { + t.Helper() + + conn, err := grpc.NewClient(setup.grpcAddr, grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + t.Cleanup(cancel) + stream, err := proto.NewProxyServiceClient(conn).GetMappingUpdate(ctx, &proto.GetMappingUpdateRequest{ + ProxyId: proxyID, + Version: "target-access-test", + Address: targetAccessCluster, + Capabilities: caps, + }) + require.NoError(t, err) + return stream +} + +func receiveTargetAccessSnapshot(t *testing.T, stream proto.ProxyService_GetMappingUpdateClient) map[string]*proto.ProxyMapping { + t.Helper() + + mappings := make(map[string]*proto.ProxyMapping) + for { + msg, err := stream.Recv() + require.NoError(t, err) + for _, mapping := range msg.GetMapping() { + mappings[mapping.GetId()] = mapping + } + if msg.GetInitialSyncComplete() { + return mappings + } + } +} + +func receiveTargetAccessUpdate(t *testing.T, stream proto.ProxyService_GetMappingUpdateClient, serviceID string) *proto.ProxyMapping { + t.Helper() + + for { + msg, err := stream.Recv() + require.NoError(t, err) + for _, mapping := range msg.GetMapping() { + if mapping.GetId() == serviceID { + return mapping + } + } + } +} + +func TestIntegration_TargetAccessControl_ManagementToUpstream(t *testing.T) { + setup := setupIntegrationTest(t) + t.Cleanup(setup.cleanup) + + upstreamRecorder := &targetAccessUpstream{} + upstream := httptest.NewServer(upstreamRecorder) + t.Cleanup(upstream.Close) + + svc := targetAccessService(t, upstream.URL) + require.NoError(t, setup.store.CreateService(t.Context(), svc)) + + stream := targetAccessMappingStream(t, setup, "target-access-proxy", nil) + snapshot := receiveTargetAccessSnapshot(t, stream) + mapping := snapshot[svc.ID] + require.NotNil(t, mapping, "proxy must receive the target-access service") + + actions := make(map[string]proto.TargetAccessAction) + for _, pathMapping := range mapping.GetPath() { + actions[pathMapping.GetPath()] = pathMapping.GetAccessAction() + } + assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_INHERIT, actions["/"]) + assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_BYPASS, actions["/public"]) + assert.Equal(t, proto.TargetAccessAction_TARGET_ACCESS_ACTION_BLOCK, actions["/blocked"]) + + runtime, handler := newTargetAccessRuntime(t) + applyTargetAccessMapping(t, runtime, mapping) + + before := len(upstreamRecorder.snapshot()) + res := targetAccessRequestTo(handler, "/private", "198.51.100.10:12000", nil) + assert.Equal(t, http.StatusUnauthorized, res.Code, "inherit must retain service authentication") + assert.Len(t, upstreamRecorder.snapshot(), before, "unauthenticated inherited request must not reach upstream") + + validHeader := http.Header{targetAccessHeader: []string{targetAccessSecret}} + res = targetAccessRequestTo(handler, "/private", "198.51.100.10:12000", validHeader) + assert.Equal(t, http.StatusOK, res.Code, "valid service authentication must pass an inherited target") + inheritedRequests := upstreamRecorder.snapshot() + require.Len(t, inheritedRequests, before+1) + assert.Empty(t, inheritedRequests[len(inheritedRequests)-1].headers.Get(targetAccessHeader), + "proxy authentication credential must not be forwarded upstream") + + bypassHeaders := http.Header{ + targetAccessHeader: []string{"spoofed-auth"}, + "X-NetBird-User": []string{"spoofed@example.com"}, + "X-NetBird-Groups": []string{"administrators"}, + } + res = targetAccessRequestTo(handler, "/public/ready", "198.51.100.10:12000", bypassHeaders) + assert.Equal(t, http.StatusOK, res.Code, "bypass target must reach upstream without authenticating") + bypassRequests := upstreamRecorder.snapshot() + require.Len(t, bypassRequests, before+2) + bypassed := bypassRequests[len(bypassRequests)-1] + assert.Equal(t, "/ready", bypassed.path, "routing must retain the existing literal-prefix rewrite") + assert.Empty(t, bypassed.headers.Get(targetAccessHeader), "configured auth header must be stripped on bypass") + assert.Empty(t, bypassed.headers.Get("X-NetBird-User"), "bypass must not permit spoofed identity") + assert.Empty(t, bypassed.headers.Get("X-NetBird-Groups"), "bypass must not permit spoofed groups") + + before = len(bypassRequests) + res = targetAccessRequestTo(handler, "/blocked/secret", "198.51.100.10:12000", validHeader) + assert.Equal(t, http.StatusForbidden, res.Code, "block must win even when service authentication is valid") + assert.Len(t, upstreamRecorder.snapshot(), before, "blocked request must not reach upstream") + + res = targetAccessRequestTo(handler, "/public/ready", "203.0.113.8:12000", nil) + assert.Equal(t, http.StatusForbidden, res.Code, "connection restrictions must run before bypass") + assert.Len(t, upstreamRecorder.snapshot(), before, "restricted bypass request must not reach upstream") + + for _, target := range svc.Targets { + switch *target.Path { + case "/public": + target.AccessAction = service.TargetAccessActionBlock + case "/blocked": + target.AccessAction = service.TargetAccessActionBypass + } + } + require.NoError(t, setup.store.UpdateService(t.Context(), svc)) + persisted, err := setup.store.GetServiceByID(t.Context(), store.LockingStrengthNone, targetAccessAccountID, svc.ID) + require.NoError(t, err) + update := persisted.ToProtoMapping(service.Update, "", nbproxy.OIDCValidationConfig{}) + setup.proxyService.SendServiceUpdate(&proto.GetMappingUpdateResponse{Mapping: []*proto.ProxyMapping{update}}) + receivedUpdate := receiveTargetAccessUpdate(t, stream, svc.ID) + applyTargetAccessMapping(t, runtime, receivedUpdate) + + res = targetAccessRequestTo(handler, "/public/ready", "198.51.100.10:12000", validHeader) + assert.Equal(t, http.StatusForbidden, res.Code, "modified action must replace bypass with block") + assert.Len(t, upstreamRecorder.snapshot(), before, "newly blocked request must not reach upstream") + + res = targetAccessRequestTo(handler, "/blocked/ready", "198.51.100.10:12000", nil) + assert.Equal(t, http.StatusOK, res.Code, "modified action must replace block with bypass") + assert.Len(t, upstreamRecorder.snapshot(), before+1, "newly bypassed request must reach upstream") + + require.NoError(t, setup.store.DeleteService(t.Context(), targetAccessAccountID, svc.ID)) + removed := persisted.ToProtoMapping(service.Delete, "", nbproxy.OIDCValidationConfig{}) + setup.proxyService.SendServiceUpdate(&proto.GetMappingUpdateResponse{Mapping: []*proto.ProxyMapping{removed}}) + receivedRemoval := receiveTargetAccessUpdate(t, stream, svc.ID) + runtime.removeMapping(t.Context(), receivedRemoval) + + res = targetAccessRequestTo(handler, "/blocked/ready", "198.51.100.10:12000", nil) + assert.Equal(t, http.StatusNotFound, res.Code, "removed mapping must no longer route") + assert.Len(t, upstreamRecorder.snapshot(), before+1, "request after removal must not reach upstream") +} + +func TestIntegration_TargetAccessControl_PrivateServiceRejectsBypass(t *testing.T) { + setup := setupIntegrationTest(t) + t.Cleanup(setup.cleanup) + + upstreamRecorder := &targetAccessUpstream{} + upstream := httptest.NewServer(upstreamRecorder) + t.Cleanup(upstream.Close) + + svc := targetAccessService(t, upstream.URL) + svc.ID = "target-access-private" + svc.Domain = "target-access-private.test.proxy.io" + svc.Private = true + svc.Targets = svc.Targets[1:2] + require.NoError(t, setup.store.CreateService(t.Context(), svc)) + + stream := targetAccessMappingStream(t, setup, "target-access-private", privateServiceCapabilities()) + mapping := receiveTargetAccessSnapshot(t, stream)[svc.ID] + require.NotNil(t, mapping, "proxy supporting private services must receive private target-access mapping") + + runtime, handler := newTargetAccessRuntime(t) + err := runtime.updateMapping(t.Context(), mapping) + require.Error(t, err, "private service with a bypass target must be rejected") + + req := httptest.NewRequest(http.MethodGet, "http://"+svc.Domain+"/public/ready", nil) + req.Host = svc.Domain + req.RemoteAddr = "100.64.0.20:12000" + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusNotFound, recorder.Code, "rejected private mapping must not be published") + assert.Empty(t, upstreamRecorder.snapshot(), "rejected private mapping must not reach upstream") +} diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index 7060aee4e..68de0d1f5 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -3104,6 +3104,12 @@ components: type: string description: URL path prefix for this target (HTTP only) example: "/" + access_action: + type: string + description: Access action applied after this target's HTTP path prefix is selected. "inherit" uses the service authentication configuration, "bypass" skips service authentication, and "block" denies access. HTTP services only. + enum: [inherit, bypass, block] + default: inherit + example: "inherit" protocol: type: string description: Protocol to use when connecting to the backend diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index dea9f4a14..a1f4bc530 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -1037,6 +1037,27 @@ func (e ServiceRequestMode) Valid() bool { } } +// Defines values for ServiceTargetAccessAction. +const ( + ServiceTargetAccessActionBlock ServiceTargetAccessAction = "block" + ServiceTargetAccessActionBypass ServiceTargetAccessAction = "bypass" + ServiceTargetAccessActionInherit ServiceTargetAccessAction = "inherit" +) + +// Valid indicates whether the value is a known member of the ServiceTargetAccessAction enum. +func (e ServiceTargetAccessAction) Valid() bool { + switch e { + case ServiceTargetAccessActionBlock: + return true + case ServiceTargetAccessActionBypass: + return true + case ServiceTargetAccessActionInherit: + return true + default: + return false + } +} + // Defines values for ServiceTargetProtocol. const ( ServiceTargetProtocolHttp ServiceTargetProtocol = "http" @@ -5113,6 +5134,9 @@ type ServiceRequestMode string // ServiceTarget defines model for ServiceTarget. type ServiceTarget struct { + // AccessAction Access action applied after this target's HTTP path prefix is selected. "inherit" uses the service authentication configuration, "bypass" skips service authentication, and "block" denies access. HTTP services only. + AccessAction *ServiceTargetAccessAction `json:"access_action,omitempty"` + // Enabled Whether this target is enabled Enabled bool `json:"enabled"` @@ -5136,6 +5160,9 @@ type ServiceTarget struct { TargetType ServiceTargetTargetType `json:"target_type"` } +// ServiceTargetAccessAction Access action applied after this target's HTTP path prefix is selected. "inherit" uses the service authentication configuration, "bypass" skips service authentication, and "block" denies access. HTTP services only. +type ServiceTargetAccessAction string + // ServiceTargetProtocol Protocol to use when connecting to the backend type ServiceTargetProtocol string