diff --git a/apidef/api_definitions.go b/apidef/api_definitions.go index d8bc35864ec..c1a052ab4f9 100644 --- a/apidef/api_definitions.go +++ b/apidef/api_definitions.go @@ -746,6 +746,7 @@ type APIDefinition struct { AllowedIPs []string `mapstructure:"allowed_ips" bson:"allowed_ips" json:"allowed_ips"` EnableIpBlacklisting bool `mapstructure:"enable_ip_blacklisting" bson:"enable_ip_blacklisting" json:"enable_ip_blacklisting"` BlacklistedIPs []string `mapstructure:"blacklisted_ips" bson:"blacklisted_ips" json:"blacklisted_ips"` + IPAccessControlDisabled bool `mapstructure:"ip_access_control_disabled" bson:"ip_access_control_disabled" json:"ip_access_control_disabled"` DontSetQuotasOnCreate bool `mapstructure:"dont_set_quota_on_create" bson:"dont_set_quota_on_create" json:"dont_set_quota_on_create"` ExpireAnalyticsAfter int64 `mapstructure:"expire_analytics_after" bson:"expire_analytics_after" json:"expire_analytics_after"` // must have an expireAt TTL index set (http://docs.mongodb.org/manual/tutorial/expire-data/) ResponseProcessors []ResponseProcessor `bson:"response_processors" json:"response_processors"` diff --git a/apidef/migration.go b/apidef/migration.go index b25b2abd90e..9d9b6421177 100644 --- a/apidef/migration.go +++ b/apidef/migration.go @@ -248,6 +248,7 @@ func (a *APIDefinition) Migrate() (versions []APIDefinition, err error) { a.migrateScopeToPolicy() a.migrateResponseProcessors() a.migrateGlobalRateLimit() + a.migrateIPAccessControl() versions, err = a.MigrateVersioning() if err != nil { @@ -517,3 +518,17 @@ func (a *APIDefinition) migrateGlobalRateLimit() { a.GlobalRateLimit.Disabled = true } } + +func (a *APIDefinition) migrateIPAccessControl() { + a.IPAccessControlDisabled = false + + if a.EnableIpBlacklisting && len(a.BlacklistedIPs) > 0 { + return + } + + if a.EnableIpWhiteListing && len(a.AllowedIPs) > 0 { + return + } + + a.IPAccessControlDisabled = true +} diff --git a/apidef/migration_test.go b/apidef/migration_test.go index 332833f734e..7cbebd9d147 100644 --- a/apidef/migration_test.go +++ b/apidef/migration_test.go @@ -836,3 +836,78 @@ func TestAPIDefinition_migrateGlobalRateLimit(t *testing.T) { assert.False(t, base.GlobalRateLimit.Disabled) }) } + +func TestAPIDefinition_migrateIPAccessControl(t *testing.T) { + t.Run("whitelisting", func(t *testing.T) { + t.Run("EnableIpWhitelisting=true, no whitelist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpWhiteListing = true + _, err := base.Migrate() + assert.NoError(t, err) + assert.True(t, base.IPAccessControlDisabled) + }) + + t.Run("IPWhiteListEnabled=true, non-empty whitelist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpWhiteListing = true + base.AllowedIPs = []string{"127.0.0.1"} + _, err := base.Migrate() + assert.NoError(t, err) + assert.False(t, base.IPAccessControlDisabled) + }) + + t.Run("EnableIpWhitelisting=false, no whitelist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpWhiteListing = false + _, err := base.Migrate() + assert.NoError(t, err) + assert.True(t, base.IPAccessControlDisabled) + }) + + t.Run("IPWhiteListEnabled=false, non-empty whitelist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpWhiteListing = false + base.AllowedIPs = []string{"127.0.0.1"} + _, err := base.Migrate() + assert.NoError(t, err) + assert.True(t, base.IPAccessControlDisabled) + }) + }) + + t.Run("blacklisting", func(t *testing.T) { + t.Run("EnableIpBlacklisting=true, no blacklist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpBlacklisting = true + _, err := base.Migrate() + assert.NoError(t, err) + assert.True(t, base.IPAccessControlDisabled) + }) + + t.Run("EnableIpBlacklisting=true, non-empty blacklist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpBlacklisting = true + base.BlacklistedIPs = []string{"127.0.0.1"} + _, err := base.Migrate() + assert.NoError(t, err) + assert.False(t, base.IPAccessControlDisabled) + }) + + t.Run("EnableIpBlacklisting=false, no blacklist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpBlacklisting = false + _, err := base.Migrate() + assert.NoError(t, err) + assert.True(t, base.IPAccessControlDisabled) + }) + + t.Run("IPWhiteListEnabled=false, non-empty blacklist", func(t *testing.T) { + base := oldTestAPI() + base.EnableIpBlacklisting = false + base.BlacklistedIPs = []string{"127.0.0.1"} + _, err := base.Migrate() + assert.NoError(t, err) + assert.True(t, base.IPAccessControlDisabled) + }) + }) + +} diff --git a/apidef/oas/oas_test.go b/apidef/oas/oas_test.go index 9e0fd17a7ea..dc839f76cec 100644 --- a/apidef/oas/oas_test.go +++ b/apidef/oas/oas_test.go @@ -173,6 +173,7 @@ func TestOAS_ExtractTo_ResetAPIDefinition(t *testing.T) { a.EnableContextVars = false a.DisableRateLimit = false a.DoNotTrack = false + a.IPAccessControlDisabled = false // deprecated fields a.Auth = apidef.AuthConfig{} @@ -263,9 +264,7 @@ func TestOAS_ExtractTo_ResetAPIDefinition(t *testing.T) { "APIDefinition.SessionProvider.Meta[0]", "APIDefinition.EnableBatchRequestSupport", "APIDefinition.EnableIpWhiteListing", - "APIDefinition.AllowedIPs[0]", "APIDefinition.EnableIpBlacklisting", - "APIDefinition.BlacklistedIPs[0]", "APIDefinition.DontSetQuotasOnCreate", "APIDefinition.ExpireAnalyticsAfter", "APIDefinition.ResponseProcessors[0].Name", diff --git a/apidef/oas/schema/x-tyk-api-gateway.json b/apidef/oas/schema/x-tyk-api-gateway.json index 60a8eb71c15..76d4118829b 100644 --- a/apidef/oas/schema/x-tyk-api-gateway.json +++ b/apidef/oas/schema/x-tyk-api-gateway.json @@ -1244,6 +1244,9 @@ }, "contextVariables": { "$ref": "#/definitions/X-Tyk-ContextVariables" + }, + "ipAccessControl": { + "$ref": "#/definitions/X-Tyk-IPAccessControl" } }, "required": [ @@ -2159,6 +2162,26 @@ "type": "string" } } + }, + "X-Tyk-IPAccessControl": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "allow": { + "type": "array", + "items": { + "type": "string" + } + }, + "block": { + "type": "array", + "items": { + "type": "string" + } + } + } } } -} \ No newline at end of file +} diff --git a/apidef/oas/server.go b/apidef/oas/server.go index 6d1ec5c1f3c..b497dffbff4 100644 --- a/apidef/oas/server.go +++ b/apidef/oas/server.go @@ -38,6 +38,11 @@ type Server struct { // // Tyk classic API definition: `event_handlers` EventHandlers EventHandlers `bson:"eventHandlers,omitempty" json:"eventHandlers,omitempty"` + + // IPAccessControl configures IP access control for this API. + // + // Tyk classic API definition: `allowed_ips` and `blacklisted_ips`. + IPAccessControl *IPAccessControl `bson:"ipAccessControl,omitempty" json:"ipAccessControl,omitempty"` } // Fill fills *Server from apidef.APIDefinition. @@ -94,6 +99,8 @@ func (s *Server) Fill(api apidef.APIDefinition) { if ShouldOmit(s.EventHandlers) { s.EventHandlers = nil } + + s.fillIPAccessControl(api) } // ExtractTo extracts *Server into *apidef.APIDefinition. @@ -153,6 +160,8 @@ func (s *Server) ExtractTo(api *apidef.APIDefinition) { } s.EventHandlers.ExtractTo(api) + + s.extractIPAccessControlTo(api) } // ListenPath is the base path on Tyk to which requests for this API @@ -287,3 +296,53 @@ func (dt *DetailedTracing) Fill(api apidef.APIDefinition) { func (dt *DetailedTracing) ExtractTo(api *apidef.APIDefinition) { api.DetailedTracing = dt.Enabled } + +// IPAccessControl represents IP access control configuration. +type IPAccessControl struct { + // Enabled indicates whether IP access control is enabled. + Enabled bool `bson:"enabled" json:"enabled"` + + // Allow is a list of allowed IP addresses or CIDR blocks (e.g. "192.168.1.0/24"). + // Note that if an IP address is present in both Allow and Block, the Block rule will take precedence. + Allow []string `bson:"allow,omitempty" json:"allow,omitempty"` + + // Block is a list of blocked IP addresses or CIDR blocks (e.g. "192.168.1.100/32"). + // If an IP address is present in both Allow and Block, the Block rule will take precedence. + Block []string `bson:"block,omitempty" json:"block,omitempty"` +} + +// Fill fills *IPAccessControl from apidef.APIDefinition. +func (i *IPAccessControl) Fill(api apidef.APIDefinition) { + i.Enabled = !api.IPAccessControlDisabled + i.Block = api.BlacklistedIPs + i.Allow = api.AllowedIPs +} + +// ExtractTo extracts *IPAccessControl into *apidef.APIDefinition. +func (i *IPAccessControl) ExtractTo(api *apidef.APIDefinition) { + api.IPAccessControlDisabled = !i.Enabled + api.BlacklistedIPs = i.Block + api.AllowedIPs = i.Allow +} + +func (s *Server) fillIPAccessControl(api apidef.APIDefinition) { + if s.IPAccessControl == nil { + s.IPAccessControl = &IPAccessControl{} + } + + s.IPAccessControl.Fill(api) + if ShouldOmit(s.IPAccessControl) { + s.IPAccessControl = nil + } +} + +func (s *Server) extractIPAccessControlTo(api *apidef.APIDefinition) { + if s.IPAccessControl == nil { + s.IPAccessControl = &IPAccessControl{} + defer func() { + s.IPAccessControl = nil + }() + } + + s.IPAccessControl.ExtractTo(api) +} diff --git a/apidef/oas/server_test.go b/apidef/oas/server_test.go index c4b2463c33b..c331664ab61 100644 --- a/apidef/oas/server_test.go +++ b/apidef/oas/server_test.go @@ -323,3 +323,37 @@ func TestExportDetailedTracing(t *testing.T) { }) } } + +func TestIPAccessControl(t *testing.T) { + t.Run("empty", func(t *testing.T) { + var emptyIPAccessControl IPAccessControl + + var convertedAPI apidef.APIDefinition + convertedAPI.SetDisabledFlags() + emptyIPAccessControl.ExtractTo(&convertedAPI) + + var resultIPAccessControl IPAccessControl + resultIPAccessControl.Fill(convertedAPI) + + assert.Equal(t, emptyIPAccessControl, resultIPAccessControl) + }) + + t.Run("valid", func(t *testing.T) { + ipAccessControl := IPAccessControl{ + Enabled: true, + Allow: []string{"127.0.0.1"}, + Block: []string{"10.0.0.1"}, + } + + var convertedAPI apidef.APIDefinition + convertedAPI.SetDisabledFlags() + ipAccessControl.ExtractTo(&convertedAPI) + + assert.False(t, convertedAPI.IPAccessControlDisabled) + + var resultIPAccessControl IPAccessControl + resultIPAccessControl.Fill(convertedAPI) + + assert.Equal(t, ipAccessControl, resultIPAccessControl) + }) +} diff --git a/apidef/schema.json b/apidef/schema.json index 7f909bccfb6..91c1760d334 100644 --- a/apidef/schema.json +++ b/apidef/schema.json @@ -34,6 +34,9 @@ "tags_disabled": { "type": "boolean" }, + "ip_access_control_disabled": { + "type": "boolean" + }, "enable_ip_whitelisting": { "type": "boolean" }, diff --git a/docs/dev/apidef-oas.md b/docs/dev/apidef-oas.md index f6387c86dbe..4b4d8664d4a 100644 --- a/docs/dev/apidef-oas.md +++ b/docs/dev/apidef-oas.md @@ -6,6 +6,8 @@ To ensure feature parity between Tyk OAS APIs and Tyk classic API definitions, f Define the necessary structs or add the necessary fields in the `apidef/oas` package. +Make sure `json` and `bson` tags are added to the fields. + If an `enabled` flag is specified in the OAS contract, make sure a corresponding `disabled` or `enabled` flag is added in the classic API definition. Also make sure that `disabled`/`enabled` flag toggles the feature on or off. @@ -42,9 +44,11 @@ For fields that are required: For optional fields: - 1. Add the `omitempty` tag. + 1. Add the `omitempty` tag - 2. Use pointer types for structs. + 2. Use pointer types for structs. + + 3. Make sure that `omitempty` tag is added for slice fields that are optional. ## Add Go Doc Comments diff --git a/gateway/mw_ip_blacklist.go b/gateway/mw_ip_blacklist.go index de313402988..1b5407e5cb5 100644 --- a/gateway/mw_ip_blacklist.go +++ b/gateway/mw_ip_blacklist.go @@ -18,7 +18,9 @@ func (i *IPBlackListMiddleware) Name() string { } func (i *IPBlackListMiddleware) EnabledForSpec() bool { - return i.Spec.EnableIpBlacklisting && len(i.Spec.BlacklistedIPs) > 0 + enabled := !i.Spec.APIDefinition.IPAccessControlDisabled || i.Spec.APIDefinition.EnableIpBlacklisting + + return enabled && len(i.Spec.APIDefinition.BlacklistedIPs) > 0 } // ProcessRequest will run any checks on the request on the way through the system, return an error to have the chain fail diff --git a/gateway/mw_ip_blacklist_test.go b/gateway/mw_ip_blacklist_test.go index 1321e724d53..8b5436b5d29 100644 --- a/gateway/mw_ip_blacklist_test.go +++ b/gateway/mw_ip_blacklist_test.go @@ -5,6 +5,9 @@ import ( "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" + + "github.com/TykTechnologies/tyk/apidef" "github.com/TykTechnologies/tyk/header" ) @@ -27,6 +30,83 @@ func testPrepareIPBlacklistMiddleware() *APISpec { })[0] } +func TestIPBlackListMiddleware_EnabledForSpec(t *testing.T) { + tests := []struct { + name string + spec *APISpec + wantResult bool + }{ + { + name: "IpBlacklisting enabled and BlacklistedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpBlacklisting: true, + BlacklistedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: true, + }, + { + name: "IpBlacklisting disabled and IPAccessControl disabled and BlacklistedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpBlacklisting: false, + IPAccessControlDisabled: true, + BlacklistedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: false, + }, + { + name: "IpBlacklisting enabled and BlacklistedIPs empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpBlacklisting: true, + BlacklistedIPs: []string{}, + }, + }, + wantResult: false, + }, + { + name: "IpBlacklisting disabled and BlacklistedIPs empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpBlacklisting: false, + BlacklistedIPs: []string{}, + }, + }, + wantResult: false, + }, + { + name: "IPAccessControlDisabled true and BlacklistedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + IPAccessControlDisabled: true, + BlacklistedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: false, + }, + { + name: "IPAccessControlDisabled false and BlacklistedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + IPAccessControlDisabled: false, + BlacklistedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + middleware := &IPBlackListMiddleware{BaseMiddleware: &BaseMiddleware{Spec: tt.spec}} + gotResult := middleware.EnabledForSpec() + assert.Equal(t, tt.wantResult, gotResult) + }) + } +} func TestIPBlacklistMiddleware(t *testing.T) { spec := testPrepareIPBlacklistMiddleware() diff --git a/gateway/mw_ip_whitelist.go b/gateway/mw_ip_whitelist.go index 174cdda28c5..faebc169cf6 100644 --- a/gateway/mw_ip_whitelist.go +++ b/gateway/mw_ip_whitelist.go @@ -18,7 +18,9 @@ func (i *IPWhiteListMiddleware) Name() string { } func (i *IPWhiteListMiddleware) EnabledForSpec() bool { - return i.Spec.EnableIpWhiteListing && len(i.Spec.AllowedIPs) > 0 + enabled := !i.Spec.APIDefinition.IPAccessControlDisabled || i.Spec.APIDefinition.EnableIpWhiteListing + + return enabled && len(i.Spec.APIDefinition.AllowedIPs) > 0 } // ProcessRequest will run any checks on the request on the way through the system, return an error to have the chain fail diff --git a/gateway/mw_ip_whitelist_test.go b/gateway/mw_ip_whitelist_test.go index e011f216c40..7d7515b6005 100644 --- a/gateway/mw_ip_whitelist_test.go +++ b/gateway/mw_ip_whitelist_test.go @@ -5,6 +5,9 @@ import ( "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" + + "github.com/TykTechnologies/tyk/apidef" "github.com/TykTechnologies/tyk/header" ) @@ -27,6 +30,84 @@ func testPrepareIPMiddlewarePass() *APISpec { })[0] } +func TestIPWhiteListMiddleware_EnabledForSpec(t *testing.T) { + tests := []struct { + name string + spec *APISpec + wantResult bool + }{ + { + name: "IpWhiteListing enabled and AllowedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpWhiteListing: true, + AllowedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: true, + }, + { + name: "IpWhiteListing disabled and IPAccessControl disabled and AllowedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpWhiteListing: false, + IPAccessControlDisabled: true, + AllowedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: false, + }, + { + name: "IpWhiteListing enabled and AllowedIPs empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpWhiteListing: true, + AllowedIPs: []string{}, + }, + }, + wantResult: false, + }, + { + name: "IpWhiteListing disabled and AllowedIPs empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + EnableIpWhiteListing: false, + AllowedIPs: []string{}, + }, + }, + wantResult: false, + }, + { + name: "IPAccessControlDisabled true and AllowedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + IPAccessControlDisabled: true, + AllowedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: false, + }, + { + name: "IPAccessControlDisabled false and AllowedIPs not empty", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + IPAccessControlDisabled: false, + AllowedIPs: []string{"192.168.1.1"}, + }, + }, + wantResult: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + middleware := &IPWhiteListMiddleware{BaseMiddleware: &BaseMiddleware{Spec: tt.spec}} + gotResult := middleware.EnabledForSpec() + assert.Equal(t, tt.wantResult, gotResult) + }) + } +} + func TestIPMiddlewarePass(t *testing.T) { spec := testPrepareIPMiddlewarePass()