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