mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-06 22:32:16 +00:00
Merge commit from fork
* Inject DenyList from config to `StartCommands()` * Implementation draft * Move address checker to separate package * Migrate usages of actions.AddressChecker to denylist.AddressChecker * Rename denylist package files * Pass []denylist.AddressChecker to StartCommand * Add DenyList to defaults.yaml and add custom config parser * net: add HostnameToIPList function * denylist: Add IsHostBlocked() * actions: use denylist.IsHostBlocked() * command: Inject ip lookup function + extend add target validationt test * command: Unexport ChangeTarget.IsValid() * command: Add denyList check on ChangeTarget validation * command: Export ActionsV2DenyList and IPLookupFunction params * Lint fix * Check denylist during action execution * Fix integration tests * Apply suggestions * Add `mapstructure.StringToSliceHookFunc()` to decode `HTTPConfigDecodeHook`
This commit is contained in:
@@ -7,9 +7,11 @@ import (
|
||||
|
||||
"github.com/zitadel/zitadel/internal/command/preparation"
|
||||
"github.com/zitadel/zitadel/internal/crypto"
|
||||
"github.com/zitadel/zitadel/internal/denylist"
|
||||
"github.com/zitadel/zitadel/internal/domain"
|
||||
"github.com/zitadel/zitadel/internal/eventstore/v1/models"
|
||||
target_domain "github.com/zitadel/zitadel/internal/execution/target"
|
||||
internal_net "github.com/zitadel/zitadel/internal/net"
|
||||
"github.com/zitadel/zitadel/internal/repository/target"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
@@ -27,18 +29,22 @@ type AddTarget struct {
|
||||
SigningKey string
|
||||
}
|
||||
|
||||
func (a *AddTarget) IsValid() error {
|
||||
func (a *AddTarget) isValid(inputDenyList []denylist.AddressChecker, lookupFunc internal_net.IPLookupFunc) error {
|
||||
if a.Name == "" {
|
||||
return zerrors.ThrowInvalidArgument(nil, "COMMAND-ddqbm9us5p", "Errors.Target.Invalid")
|
||||
}
|
||||
if a.Timeout == 0 {
|
||||
return zerrors.ThrowInvalidArgument(nil, "COMMAND-39f35d8uri", "Errors.Target.NoTimeout")
|
||||
}
|
||||
_, err := url.Parse(a.Endpoint)
|
||||
parsedURL, err := url.Parse(a.Endpoint)
|
||||
if err != nil || a.Endpoint == "" {
|
||||
return zerrors.ThrowInvalidArgument(err, "COMMAND-1r2k6qo6wg", "Errors.Target.InvalidURL")
|
||||
}
|
||||
|
||||
if err := denylist.IsHostBlocked(inputDenyList, parsedURL, lookupFunc); err != nil {
|
||||
return zerrors.ThrowInvalidArgument(err, "COMMAND-NcJUKo", "Errors.Target.DeniedURL")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -47,7 +53,7 @@ func (c *Commands) AddTarget(ctx context.Context, add *AddTarget, resourceOwner
|
||||
return time.Time{}, zerrors.ThrowInvalidArgument(nil, "COMMAND-brml926e2d", "Errors.IDMissing")
|
||||
}
|
||||
|
||||
if err := add.IsValid(); err != nil {
|
||||
if err := add.isValid(c.ActionsV2DenyList, c.IPLookupFunction); err != nil {
|
||||
return time.Time{}, err
|
||||
}
|
||||
|
||||
@@ -103,7 +109,7 @@ type ChangeTarget struct {
|
||||
SigningKey *string
|
||||
}
|
||||
|
||||
func (a *ChangeTarget) IsValid() error {
|
||||
func (a *ChangeTarget) isValid(inputDenyList []denylist.AddressChecker, lookupFunc internal_net.IPLookupFunc) error {
|
||||
if a.AggregateID == "" {
|
||||
return zerrors.ThrowInvalidArgument(nil, "COMMAND-1l6ympeagp", "Errors.IDMissing")
|
||||
}
|
||||
@@ -114,11 +120,15 @@ func (a *ChangeTarget) IsValid() error {
|
||||
return zerrors.ThrowInvalidArgument(nil, "COMMAND-08b39vdi57", "Errors.Target.NoTimeout")
|
||||
}
|
||||
if a.Endpoint != nil {
|
||||
_, err := url.Parse(*a.Endpoint)
|
||||
parsedURL, err := url.Parse(*a.Endpoint)
|
||||
if err != nil || *a.Endpoint == "" {
|
||||
return zerrors.ThrowInvalidArgument(err, "COMMAND-jsbaera7b6", "Errors.Target.InvalidURL")
|
||||
}
|
||||
if err := denylist.IsHostBlocked(inputDenyList, parsedURL, lookupFunc); err != nil {
|
||||
return zerrors.ThrowInvalidArgument(err, "COMMAND-jKbbu2", "Errors.Target.DeniedURL")
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -126,7 +136,7 @@ func (c *Commands) ChangeTarget(ctx context.Context, change *ChangeTarget, resou
|
||||
if resourceOwner == "" {
|
||||
return time.Time{}, zerrors.ThrowInvalidArgument(nil, "COMMAND-zqibgg0wwh", "Errors.IDMissing")
|
||||
}
|
||||
if err := change.IsValid(); err != nil {
|
||||
if err := change.isValid(c.ActionsV2DenyList, c.IPLookupFunction); err != nil {
|
||||
return time.Time{}, err
|
||||
}
|
||||
existing, err := c.getTargetWriteModelByID(ctx, change.AggregateID, resourceOwner)
|
||||
|
||||
@@ -2,28 +2,37 @@ package command
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/muhlemmer/gu"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/crypto"
|
||||
"github.com/zitadel/zitadel/internal/denylist"
|
||||
"github.com/zitadel/zitadel/internal/eventstore"
|
||||
"github.com/zitadel/zitadel/internal/eventstore/v1/models"
|
||||
target_domain "github.com/zitadel/zitadel/internal/execution/target"
|
||||
"github.com/zitadel/zitadel/internal/id"
|
||||
"github.com/zitadel/zitadel/internal/id/mock"
|
||||
internal_net "github.com/zitadel/zitadel/internal/net"
|
||||
"github.com/zitadel/zitadel/internal/repository/target"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
func TestCommands_AddTarget(t *testing.T) {
|
||||
localhostAddrChecker, err := denylist.NewHostChecker("localhost")
|
||||
require.NoError(t, err)
|
||||
|
||||
type fields struct {
|
||||
eventstore func(t *testing.T) *eventstore.Eventstore
|
||||
idGenerator id.Generator
|
||||
newEncryptedCodeWithDefault encryptedCodeWithDefaultFunc
|
||||
defaultSecretGenerators *SecretGenerators
|
||||
denyList []denylist.AddressChecker
|
||||
lookupFunc internal_net.IPLookupFunc
|
||||
}
|
||||
type args struct {
|
||||
ctx context.Context
|
||||
@@ -120,6 +129,28 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
err: zerrors.IsErrorInvalidArgument,
|
||||
},
|
||||
},
|
||||
{
|
||||
"ip not allowed, error",
|
||||
fields{
|
||||
eventstore: expectEventstore(),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("127.0.0.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
add: &AddTarget{
|
||||
Name: "name",
|
||||
Timeout: time.Second,
|
||||
Endpoint: "http://localhost",
|
||||
},
|
||||
resourceOwner: "instance",
|
||||
},
|
||||
res{
|
||||
err: zerrors.IsErrorInvalidArgument,
|
||||
},
|
||||
},
|
||||
{
|
||||
"unique constraint failed, error",
|
||||
fields{
|
||||
@@ -147,6 +178,10 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
idGenerator: mock.ExpectID(t, "id1"),
|
||||
newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour),
|
||||
defaultSecretGenerators: &SecretGenerators{},
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -174,6 +209,10 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
),
|
||||
),
|
||||
idGenerator: mock.ExpectID(t, "id1"),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -201,6 +240,10 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
idGenerator: mock.ExpectID(t, "id1"),
|
||||
newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour),
|
||||
defaultSecretGenerators: &SecretGenerators{},
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -233,6 +276,10 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
idGenerator: mock.ExpectID(t, "id1"),
|
||||
newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour),
|
||||
defaultSecretGenerators: &SecretGenerators{},
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -258,6 +305,7 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
idGenerator: tt.fields.idGenerator,
|
||||
newEncryptedCodeWithDefault: tt.fields.newEncryptedCodeWithDefault,
|
||||
defaultSecretGenerators: tt.fields.defaultSecretGenerators,
|
||||
ActionsV2DenyList: tt.fields.denyList,
|
||||
}
|
||||
_, err := c.AddTarget(tt.args.ctx, tt.args.add, tt.args.resourceOwner)
|
||||
if tt.res.err == nil {
|
||||
@@ -274,10 +322,15 @@ func TestCommands_AddTarget(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestCommands_ChangeTarget(t *testing.T) {
|
||||
localhostAddrChecker, err := denylist.NewHostChecker("localhost")
|
||||
require.NoError(t, err)
|
||||
|
||||
type fields struct {
|
||||
eventstore func(t *testing.T) *eventstore.Eventstore
|
||||
newEncryptedCodeWithDefault encryptedCodeWithDefaultFunc
|
||||
defaultSecretGenerators *SecretGenerators
|
||||
denyList []denylist.AddressChecker
|
||||
lookupFunc internal_net.IPLookupFunc
|
||||
}
|
||||
type args struct {
|
||||
ctx context.Context
|
||||
@@ -385,12 +438,39 @@ func TestCommands_ChangeTarget(t *testing.T) {
|
||||
err: zerrors.IsErrorInvalidArgument,
|
||||
},
|
||||
},
|
||||
{
|
||||
"ip not allowed, error",
|
||||
fields{
|
||||
eventstore: expectEventstore(),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("127.0.0.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
change: &ChangeTarget{
|
||||
ObjectRoot: models.ObjectRoot{
|
||||
AggregateID: "id1",
|
||||
},
|
||||
Endpoint: gu.Ptr("http://localhost"),
|
||||
},
|
||||
resourceOwner: "instance",
|
||||
},
|
||||
res{
|
||||
err: zerrors.IsErrorInvalidArgument,
|
||||
},
|
||||
},
|
||||
{
|
||||
"not found, error",
|
||||
fields{
|
||||
eventstore: expectEventstore(
|
||||
expectFilter(),
|
||||
),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -416,6 +496,10 @@ func TestCommands_ChangeTarget(t *testing.T) {
|
||||
),
|
||||
),
|
||||
),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -448,6 +532,10 @@ func TestCommands_ChangeTarget(t *testing.T) {
|
||||
),
|
||||
),
|
||||
),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -481,6 +569,10 @@ func TestCommands_ChangeTarget(t *testing.T) {
|
||||
),
|
||||
),
|
||||
),
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -525,6 +617,10 @@ func TestCommands_ChangeTarget(t *testing.T) {
|
||||
),
|
||||
newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("12345678", time.Hour),
|
||||
defaultSecretGenerators: &SecretGenerators{},
|
||||
denyList: []denylist.AddressChecker{localhostAddrChecker},
|
||||
lookupFunc: func(_ string) ([]net.IP, error) {
|
||||
return []net.IP{[]byte("192.168.1.1")}, nil
|
||||
},
|
||||
},
|
||||
args{
|
||||
ctx: context.Background(),
|
||||
@@ -551,6 +647,8 @@ func TestCommands_ChangeTarget(t *testing.T) {
|
||||
eventstore: tt.fields.eventstore(t),
|
||||
newEncryptedCodeWithDefault: tt.fields.newEncryptedCodeWithDefault,
|
||||
defaultSecretGenerators: tt.fields.defaultSecretGenerators,
|
||||
ActionsV2DenyList: tt.fields.denyList,
|
||||
IPLookupFunction: tt.fields.lookupFunc,
|
||||
}
|
||||
_, err := c.ChangeTarget(tt.args.ctx, tt.args.change, tt.args.resourceOwner)
|
||||
if tt.res.err == nil {
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"encoding/pem"
|
||||
"fmt"
|
||||
"math/big"
|
||||
"net"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strconv"
|
||||
@@ -24,9 +25,11 @@ import (
|
||||
"github.com/zitadel/zitadel/internal/command/preparation"
|
||||
sd "github.com/zitadel/zitadel/internal/config/systemdefaults"
|
||||
"github.com/zitadel/zitadel/internal/crypto"
|
||||
"github.com/zitadel/zitadel/internal/denylist"
|
||||
"github.com/zitadel/zitadel/internal/domain"
|
||||
"github.com/zitadel/zitadel/internal/eventstore"
|
||||
"github.com/zitadel/zitadel/internal/id"
|
||||
internal_net "github.com/zitadel/zitadel/internal/net"
|
||||
"github.com/zitadel/zitadel/internal/notification/senders"
|
||||
"github.com/zitadel/zitadel/internal/static"
|
||||
"github.com/zitadel/zitadel/internal/telemetry/tracing"
|
||||
@@ -101,7 +104,9 @@ type Commands struct {
|
||||
// so the query and cache overhead can be completely eliminated.
|
||||
milestonesCompleted sync.Map
|
||||
|
||||
loginPaths LoginPaths
|
||||
loginPaths LoginPaths
|
||||
ActionsV2DenyList []denylist.AddressChecker
|
||||
IPLookupFunction internal_net.IPLookupFunc
|
||||
}
|
||||
|
||||
//go:generate mockgen -package command -destination ./mock_login_paths.go . LoginPaths
|
||||
@@ -130,6 +135,7 @@ func StartCommands(
|
||||
defaultAccessTokenLifetime, defaultRefreshTokenLifetime, defaultRefreshTokenIdleLifetime time.Duration,
|
||||
defaultSecretGenerators *SecretGenerators,
|
||||
loginPaths LoginPaths,
|
||||
actionsDeniedHostList []denylist.AddressChecker,
|
||||
) (repo *Commands, err error) {
|
||||
if externalDomain == "" {
|
||||
return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-Df21s", "no external domain specified")
|
||||
@@ -215,9 +221,11 @@ func StartCommands(
|
||||
WithHyphen: defaults.Multifactors.RecoveryCodes.WithHyphen,
|
||||
},
|
||||
},
|
||||
GenerateDomain: domain.NewGeneratedInstanceDomain,
|
||||
caches: caches,
|
||||
loginPaths: loginPaths,
|
||||
GenerateDomain: domain.NewGeneratedInstanceDomain,
|
||||
caches: caches,
|
||||
loginPaths: loginPaths,
|
||||
ActionsV2DenyList: actionsDeniedHostList,
|
||||
IPLookupFunction: net.LookupIP,
|
||||
}
|
||||
|
||||
if defaultSecretGenerators != nil && defaultSecretGenerators.ClientSecret != nil {
|
||||
|
||||
Reference in New Issue
Block a user