Skip to content

Commit

Permalink
FIX: CrossAccountCheckingRule checking resources without PROPERTY_WIT…
Browse files Browse the repository at this point in the history
…H_POLICYDOCUMENT (#201)

* fix - cross account rule checking resources without policydocument

* bumping version

* fix format

Co-authored-by: Ramon <ramon.guimera@skyscanner.net>
  • Loading branch information
w0rmr1d3r and Ramon committed Jan 19, 2022
1 parent 19dfee4 commit 407401b
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Changelog
All notable changes to this project will be documented in this file.

## [1.3.1] - 2022-1-17
### Fixes
- Fixes `CrossAccountCheckingRule` when checking resources without `PROPERTY_WITH_POLICYDOCUMENT`.

## [1.3.0] - 2022-1-17
### Improvements
- Add `ElasticsearchDomainCrossAccountTrustRule` and `OpenSearchDomainCrossAccountTrustRule`
Expand Down
2 changes: 1 addition & 1 deletion cfripper/__version__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
VERSION = (1, 3, 0)
VERSION = (1, 3, 1)

__version__ = ".".join(map(str, VERSION))
19 changes: 10 additions & 9 deletions cfripper/rules/cross_account_trust.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,16 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
if isinstance(resource, self.RESOURCE_TYPE):
properties = resource.Properties
policy_document = getattr(properties, self.PROPERTY_WITH_POLICYDOCUMENT)
for statement in policy_document._statement_as_list():
filters_available_context = {
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
"statement": statement,
}
self._do_statement_check(result, logical_id, statement, filters_available_context)
if policy_document:
for statement in policy_document._statement_as_list():
filters_available_context = {
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
"statement": statement,
}
self._do_statement_check(result, logical_id, statement, filters_available_context)
return result

def _do_statement_check(
Expand Down
26 changes: 26 additions & 0 deletions tests/rules/test_CrossAccountTrustRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ def template_invalid_with_sts_opensearch_domain():
return get_cfmodel_from("rules/CrossAccountTrustRule/invalid_with_sts_opensearch_domain.yml").resolve()


@pytest.fixture()
def template_es_domain_without_access_policies():
return get_cfmodel_from("rules/CrossAccountTrustRule/es_domain_without_access_policies.yml").resolve()


@pytest.fixture()
def template_opensearch_domain_without_access_policies():
return get_cfmodel_from("rules/CrossAccountTrustRule/opensearch_domain_without_access_policies.yml").resolve()


@pytest.fixture()
def expected_result_two_roles():
return [
Expand Down Expand Up @@ -367,6 +377,14 @@ def test_sts_failure_es_domain(template_invalid_with_sts_es_domain):
)


def test_es_domain_without_access_policies(template_es_domain_without_access_policies):
rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
result = rule.invoke(template_es_domain_without_access_policies)

assert result.valid
assert compare_lists_of_failures(result.failures, [])


@pytest.mark.parametrize(
"principal",
[
Expand Down Expand Up @@ -442,3 +460,11 @@ def test_sts_failure_opensearch_domain(template_invalid_with_sts_opensearch_doma
)
],
)


def test_opensearch_domain_without_access_policies(template_opensearch_domain_without_access_policies):
rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"]))
result = rule.invoke(template_opensearch_domain_without_access_policies)

assert result.valid
assert compare_lists_of_failures(result.failures, [])
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
AWSTemplateFormatVersion: "2010-09-09"
Description: Testing ES Domain

Parameters:
Principal:
Type: String

Resources:
TestDomain:
Type: AWS::Elasticsearch::Domain
Properties:
DomainName: "test"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
AWSTemplateFormatVersion: "2010-09-09"
Description: Testing OpenSearchService Domain

Parameters:
Principal:
Type: String

Resources:
TestDomain:
Type: AWS::OpenSearchService::Domain
Properties:
DomainName: "test"

0 comments on commit 407401b

Please sign in to comment.