From 318e1d1d715236ca3888ee6ac3cb71ebe5050bc9 Mon Sep 17 00:00:00 2001 From: "Andoni A." <14891798+andoniaf@users.noreply.github.com> Date: Wed, 15 Apr 2026 09:03:45 +0200 Subject: [PATCH] fix(cloudflare): address review feedback for api_token_ip_restriction_enabled check Apply PR review fixes: use continue instead of break for duplicate guard, add None ID guard, restore execute() docstring, rewrite metadata fields to comply with guidelines, add service-level tests, and update README stats. --- README.md | 2 +- .../cloudflare/services/api/api_service.py | 11 +- ...token_ip_restriction_enabled.metadata.json | 14 +- .../api_token_ip_restriction_enabled.py | 17 +- .../api/cloudflare_api_token_service_test.py | 158 ++++++++++++++++++ 5 files changed, 175 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 79b336bfbd..13ec785662 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ Every AWS provider scan will enqueue an Attack Paths ingestion job automatically | M365 | 89 | 9 | 4 | 5 | Official | UI, API, CLI | | OCI | 48 | 13 | 3 | 10 | Official | UI, API, CLI | | Alibaba Cloud | 61 | 9 | 3 | 9 | Official | UI, API, CLI | -| Cloudflare | 29 | 2 | 0 | 5 | Official | UI, API, CLI | +| Cloudflare | 30 | 3 | 0 | 5 | Official | UI, API, CLI | | IaC | [See `trivy` docs.](https://trivy.dev/latest/docs/coverage/iac/) | N/A | N/A | N/A | Official | UI, API, CLI | | MongoDB Atlas | 10 | 3 | 0 | 8 | Official | UI, API, CLI | | LLM | [See `promptfoo` docs.](https://www.promptfoo.dev/docs/red-team/plugins/) | N/A | N/A | N/A | Official | CLI | diff --git a/prowler/providers/cloudflare/services/api/api_service.py b/prowler/providers/cloudflare/services/api/api_service.py index 93a3d9627a..e4527e4d67 100644 --- a/prowler/providers/cloudflare/services/api/api_service.py +++ b/prowler/providers/cloudflare/services/api/api_service.py @@ -1,7 +1,7 @@ from datetime import datetime from typing import Optional -from pydantic import BaseModel +from pydantic import BaseModel, Field from prowler.lib.logger import logger from prowler.providers.cloudflare.lib.service.service import CloudflareService @@ -22,9 +22,10 @@ class API(CloudflareService): seen_token_ids: set[str] = set() for token in self.client.user.tokens.list(): token_id = getattr(token, "id", None) - # Prevent infinite loop + if not token_id: + continue if token_id in seen_token_ids: - break + continue seen_token_ids.add(token_id) # Extract IP condition details @@ -58,8 +59,8 @@ class CloudflareAPIToken(BaseModel): id: str name: Optional[str] = None status: Optional[str] = None - ip_allow_list: list[str] = [] - ip_deny_list: list[str] = [] + ip_allow_list: list[str] = Field(default_factory=list) + ip_deny_list: list[str] = Field(default_factory=list) expires_on: Optional[datetime] = None issued_on: Optional[datetime] = None last_used_on: Optional[datetime] = None diff --git a/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.metadata.json b/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.metadata.json index 613b309972..cdc3e0c9f8 100644 --- a/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.metadata.json +++ b/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.metadata.json @@ -1,16 +1,16 @@ { "Provider": "cloudflare", "CheckID": "api_token_ip_restriction_enabled", - "CheckTitle": "IP address filtering restricts API token usage to authorized networks", + "CheckTitle": "API token has client IP address filtering configured", "CheckType": [], "ServiceName": "api", "SubServiceName": "", "ResourceIdTemplate": "", "Severity": "medium", - "ResourceType": "Other", + "ResourceType": "APIToken", "ResourceGroup": "IAM", - "Description": "**Cloudflare API tokens** are assessed for **client IP address filtering** by checking whether an allow list or deny list of IP addresses/CIDR ranges is configured to restrict which networks can use the token.", - "Risk": "Without **IP address filtering**, a compromised API token can be used from **any network location**.\n- **Confidentiality**: attackers with a stolen token can access account resources from any IP\n- **Integrity**: unauthorized modifications can be made from uncontrolled networks\n- Restricting tokens to known IP ranges limits the blast radius of credential theft", + "Description": "**Cloudflare API tokens** with **client IP address filtering** restrict token usage to requests originating from trusted network locations, reducing exposure if a token is compromised.", + "Risk": "Without **IP address filtering**, a compromised API token can be used from **any network location**.\n- **Confidentiality**: attackers with a stolen token can access account resources from any IP.\n- **Integrity**: unauthorized modifications can be made from uncontrolled networks.", "RelatedUrl": "", "AdditionalURLs": [ "https://developers.cloudflare.com/fundamentals/api/get-started/create-token/" @@ -19,11 +19,11 @@ "Code": { "CLI": "", "NativeIaC": "", - "Other": "1. Log in to the Cloudflare dashboard\n2. Go to My Profile > API Tokens\n3. Select the token to edit and click the three-dot menu > Edit\n4. Under Client IP Address Filtering, add the IP addresses or CIDR ranges that should be allowed or denied\n5. Save the token", + "Other": "1. Log in to the Cloudflare dashboard.\n2. Go to My Profile > API Tokens.\n3. Select the token to edit and click the three-dot menu > Edit.\n4. Under Client IP Address Filtering, add the IP addresses or CIDR ranges that should be allowed or denied.\n5. Save the token.", "Terraform": "```hcl\nresource \"cloudflare_api_token\" \"example_resource\" {\n name = \"example_resource\"\n\n policy {\n permission_groups = [\"\"]\n resources = { \"com.cloudflare.api.account.*\" = \"*\" }\n }\n\n # Restrict token to specific IP ranges\n condition {\n request_ip {\n in = [\"192.0.2.0/24\", \"198.51.100.0/24\"]\n }\n }\n}\n```" }, "Recommendation": { - "Text": "Configure **client IP address filtering** on all API tokens following the **principle of least privilege**.\n- Restrict tokens to known corporate or CI/CD IP ranges\n- Use CIDR notation to allow specific networks rather than broad ranges\n- Regularly review and update allowed IP lists as infrastructure changes", + "Text": "Configure **client IP address filtering** following the **principle of least privilege**. Restricting credentials to known IP ranges reduces the impact of credential compromise by ensuring they cannot be used from unauthorized networks.", "Url": "https://hub.prowler.com/checks/cloudflare/api_token_ip_restriction_enabled" } }, @@ -32,5 +32,5 @@ ], "DependsOn": [], "RelatedTo": [], - "Notes": "API tokens can have both allow lists and deny lists for IP filtering. This check verifies that at least one form of IP restriction is configured. Tokens used by CI/CD pipelines should be restricted to the IP ranges of the pipeline runners." + "Notes": "API tokens can have both allow lists and deny lists for IP filtering." } diff --git a/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.py b/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.py index cfbfe717f2..212d3bfced 100644 --- a/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.py +++ b/prowler/providers/cloudflare/services/api/api_token_ip_restriction_enabled/api_token_ip_restriction_enabled.py @@ -8,9 +8,6 @@ class api_token_ip_restriction_enabled(Check): API tokens with IP address filtering restrict token usage to requests originating from specific IP addresses or CIDR ranges. Without IP filtering, a compromised token can be used from any network location. - - - PASS: The API token has at least one IP address restriction configured. - - FAIL: The API token has no IP address filtering configured. """ def execute(self) -> list[CheckReportCloudflare]: @@ -30,19 +27,11 @@ class api_token_ip_restriction_enabled(Check): resource=token, ) - has_ip_restriction = bool(token.ip_allow_list) or bool( - token.ip_deny_list - ) - - if has_ip_restriction: + if token.ip_allow_list or token.ip_deny_list: report.status = "PASS" - report.status_extended = ( - f"API token {token.name} has client IP address filtering configured." - ) + report.status_extended = f"API token {token.name or token.id} has client IP address filtering configured." else: report.status = "FAIL" - report.status_extended = ( - f"API token {token.name} does not have client IP address filtering configured." - ) + report.status_extended = f"API token {token.name or token.id} does not have client IP address filtering configured." findings.append(report) return findings diff --git a/tests/providers/cloudflare/services/api/cloudflare_api_token_service_test.py b/tests/providers/cloudflare/services/api/cloudflare_api_token_service_test.py index 994f0e4bf0..32988b33c6 100644 --- a/tests/providers/cloudflare/services/api/cloudflare_api_token_service_test.py +++ b/tests/providers/cloudflare/services/api/cloudflare_api_token_service_test.py @@ -1,8 +1,14 @@ from datetime import datetime, timezone +from unittest import mock +from unittest.mock import MagicMock from prowler.providers.cloudflare.services.api.api_service import ( + API, CloudflareAPIToken, ) +from tests.providers.cloudflare.cloudflare_fixtures import ( + set_mocked_cloudflare_provider, +) class Test_CloudflareAPIToken_Model: @@ -77,3 +83,155 @@ class Test_CloudflareAPIToken_Model: ) assert token.status == "expired" assert token.expires_on == datetime(2025, 1, 1, tzinfo=timezone.utc) + + +class Test_API_Service: + """Tests for the API service _list_api_tokens method.""" + + def _make_sdk_token(self, **kwargs): + """Create a mock Cloudflare SDK token object.""" + token = MagicMock() + for key, value in kwargs.items(): + setattr(token, key, value) + return token + + def test_list_api_tokens_with_ip_conditions(self): + mock_provider = set_mocked_cloudflare_provider() + request_ip = MagicMock() + request_ip.in_ = ["192.0.2.0/24"] + request_ip.not_in = ["10.0.0.0/8"] + condition = MagicMock() + condition.request_ip = request_ip + + sdk_token = self._make_sdk_token( + id="token-1", + name="Test Token", + status="active", + condition=condition, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + mock_provider.session.client.user.tokens.list.return_value = [sdk_token] + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=mock_provider, + ): + service = API(mock_provider) + + assert len(service.tokens) == 1 + assert service.tokens[0].id == "token-1" + assert service.tokens[0].name == "Test Token" + assert service.tokens[0].ip_allow_list == ["192.0.2.0/24"] + assert service.tokens[0].ip_deny_list == ["10.0.0.0/8"] + + def test_list_api_tokens_without_condition(self): + mock_provider = set_mocked_cloudflare_provider() + sdk_token = self._make_sdk_token( + id="token-2", + name="No Condition Token", + status="active", + condition=None, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + mock_provider.session.client.user.tokens.list.return_value = [sdk_token] + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=mock_provider, + ): + service = API(mock_provider) + + assert len(service.tokens) == 1 + assert service.tokens[0].ip_allow_list == [] + assert service.tokens[0].ip_deny_list == [] + + def test_list_api_tokens_skips_duplicates(self): + mock_provider = set_mocked_cloudflare_provider() + sdk_token_1 = self._make_sdk_token( + id="token-dup", + name="First", + status="active", + condition=None, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + sdk_token_2 = self._make_sdk_token( + id="token-dup", + name="Duplicate", + status="active", + condition=None, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + sdk_token_3 = self._make_sdk_token( + id="token-other", + name="Other", + status="active", + condition=None, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + mock_provider.session.client.user.tokens.list.return_value = [ + sdk_token_1, + sdk_token_2, + sdk_token_3, + ] + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=mock_provider, + ): + service = API(mock_provider) + + assert len(service.tokens) == 2 + assert service.tokens[0].id == "token-dup" + assert service.tokens[0].name == "First" + assert service.tokens[1].id == "token-other" + + def test_list_api_tokens_skips_none_id(self): + mock_provider = set_mocked_cloudflare_provider() + sdk_token_no_id = self._make_sdk_token( + id=None, + name="No ID", + status="active", + condition=None, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + sdk_token_valid = self._make_sdk_token( + id="token-valid", + name="Valid", + status="active", + condition=None, + expires_on=None, + issued_on=None, + last_used_on=None, + modified_on=None, + ) + mock_provider.session.client.user.tokens.list.return_value = [ + sdk_token_no_id, + sdk_token_valid, + ] + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=mock_provider, + ): + service = API(mock_provider) + + assert len(service.tokens) == 1 + assert service.tokens[0].id == "token-valid"