[TT-11896] Add OAS IPAccessControl

[jeffy-mathew]

User description

Description

This PR adds IP Access control configurations to OAS API definition.

Related Issue

https://tyktech.atlassian.net/browse/TT-11896

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

---

PR Type

Enhancement, Tests

---

Description

• Introduced IPAccessControl feature for API definitions.

• Added migration logic for IP access control settings.

• Enhanced middleware logic for IP whitelisting and blacklisting.

• Comprehensive test coverage for new IP access control functionality.

---

Changes walkthrough 📝

	Relevant files
Enhancement	6 files api_definitions.go Added `IPAccessControlDisabled` field to API definitions. +1/-0      migration.go Added migration logic for IP access control settings.        +14/-0    server.go Added `IPAccessControl` struct and integration with API definitions. +48/-0    mw_ip_blacklist.go Updated blacklist middleware to support `IPAccessControl`. +6/-1      mw_ip_whitelist.go Updated whitelist middleware to support `IPAccessControl`. +6/-1      x-tyk-api-gateway.json Updated OAS schema to include `IPAccessControl` definition. +23/-0
Tests	5 files migration_test.go Added tests for IP access control migration logic.              +75/-0    oas_test.go Updated OAS tests to include `IPAccessControlDisabled`.    +1/-2      server_test.go Added tests for `IPAccessControl` functionality in server logic. +34/-0    mw_ip_blacklist_test.go Added tests for blacklist middleware with `IPAccessControl`. +81/-0    mw_ip_whitelist_test.go Added tests for whitelist middleware with `IPAccessControl`. +82/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[buger]

💔 The detected issue is not in one of the allowed statuses 💔

Detected Status	Open	❌
Allowed Statuses	In Dev,In Code Review,Ready for Testing,In Test,In Progress,In Review	✔️

Please ensure your jira story is in one of the allowed statuses

[github-actions]

API Changes

--- prev.txt	2025-01-10 12:18:43.159942886 +0000
+++ current.txt	2025-01-10 12:18:38.325952713 +0000
@@ -256,6 +256,7 @@
 	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"`
@@ -2835,6 +2836,26 @@
 func (id *IDExtractorConfig) Fill(api apidef.APIDefinition)
     Fill fills IDExtractorConfig from supplied classic APIDefinition.
 
+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"`
+}
+    IPAccessControl represents IP access control configuration.
+
+func (i *IPAccessControl) ExtractTo(api *apidef.APIDefinition)
+    ExtractTo extracts *IPAccessControl into *apidef.APIDefinition.
+
+func (i *IPAccessControl) Fill(api apidef.APIDefinition)
+    Fill fills *IPAccessControl from apidef.APIDefinition.
+
 type Info struct {
 	// ID is the unique identifier of the API within Tyk.
 	// Tyk classic API definition: `api_id`
@@ -3681,6 +3702,11 @@
 	//
 	// 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"`
 }
     Server contains the configuration that sets Tyk up to receive requests from
     the client applications.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Misconfiguration The new IPAccessControlDisabled field might lead to confusion when combined with EnableIpWhiteListing and EnableIpBlacklisting. Ensure that the logic for determining the effective state of IP access control is clear and consistent. IPAccessControlDisabled bool `mapstructure:"ip_access_control_disabled" bson:"ip_access_control_disabled" json:"ip_access_control_disabled"` Middleware Logic The EnabledForSpec method now checks both EnableIpBlacklisting and IPAccessControlDisabled. Ensure this logic is correctly handling all edge cases and does not introduce unintended behavior. enabled := i.Spec.APIDefinition.EnableIpBlacklisting || !i.Spec.APIDefinition.IPAccessControlDisabled if !enabled { return false } return len(i.Spec.APIDefinition.BlacklistedIPs) > 0 Middleware Logic The EnabledForSpec method now checks both EnableIpWhiteListing and IPAccessControlDisabled. Validate that this logic aligns with the intended behavior and does not conflict with existing configurations. enabled := i.Spec.APIDefinition.EnableIpWhiteListing || !i.Spec.APIDefinition.IPAccessControlDisabled if !enabled { return false } return len(i.Spec.APIDefinition.AllowedIPs) > 0

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
General	Enforce IP address format validation in the schema for better data integrity Add a format validation for the allow and block properties in the X-Tyk-IPAccessControl schema to ensure they contain valid IP addresses or CIDR blocks. apidef/oas/schema/x-tyk-api-gateway.json [2166-2182] "allow": { "type": "array", "items": { - "type": "string" + "type": "string", + "format": "ipv4" } }, "block": { "type": "array", "items": { - "type": "string" + "type": "string", + "format": "ipv4" } } Suggestion importance[1-10]: 9 Why: Adding format validation for IP addresses in the schema ensures data integrity and prevents invalid IP addresses from being accepted. This is a critical improvement for maintaining consistency and correctness in the API definition.	9
	Validate IP addresses in the blacklist to prevent invalid entries Add a check in EnabledForSpec to ensure that BlacklistedIPs contains valid IP addresses or CIDR blocks to prevent runtime errors. gateway/mw_ip_blacklist.go [26] -return len(i.Spec.APIDefinition.BlacklistedIPs) > 0 +return len(i.Spec.APIDefinition.BlacklistedIPs) > 0 && validateIPList(i.Spec.APIDefinition.BlacklistedIPs) Suggestion importance[1-10]: 7 Why: Adding validation for IP addresses in the blacklist ensures that only valid entries are processed, reducing the risk of runtime errors and misconfigurations. This enhances the reliability of the middleware.	7
	Ensure that whitelisted IPs are valid to prevent configuration errors Add a validation step in EnabledForSpec to ensure that AllowedIPs contains valid IP addresses or CIDR blocks to avoid misconfigurations. gateway/mw_ip_whitelist.go [26] -return len(i.Spec.APIDefinition.AllowedIPs) > 0 +return len(i.Spec.APIDefinition.AllowedIPs) > 0 && validateIPList(i.Spec.APIDefinition.AllowedIPs) Suggestion importance[1-10]: 7 Why: Validating the whitelisted IPs prevents misconfigurations and ensures that only valid IP addresses or CIDR blocks are used. This improves the middleware's reliability and prevents potential issues during runtime.	7
Possible issue	Handle potential conflicts when both whitelisting and blacklisting are enabled simultaneously Ensure that the migrateIPAccessControl function handles cases where both EnableIpWhiteListing and EnableIpBlacklisting are true simultaneously, as this could lead to unexpected behavior or conflicts. apidef/migration.go [524-530] -if a.EnableIpBlacklisting && len(a.BlacklistedIPs) > 0 { +if (a.EnableIpBlacklisting && len(a.BlacklistedIPs) > 0) || (a.EnableIpWhiteListing && len(a.AllowedIPs) > 0) { return } -if a.EnableIpWhiteListing && len(a.AllowedIPs) > 0 { - return -} - Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential logical conflict in the migrateIPAccessControl function when both whitelisting and blacklisting are enabled. Combining the conditions ensures that the function handles these cases correctly, improving the robustness of the migration logic.	8

[titpetric]

do we need bson tags?

[jeffy-mathew]

to follow the pattern, yes.
but when we think about how we're handling the db interaction, it's probably a no since we're storing the json blob in db.
that can be a separate discussion. I'll add the bson tag

[sonarqubecloud]

Quality Gate failed

Failed conditions
1 Security Hotspot
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE