aws-solutions / account-assessment-for-aws-organizations

Account Assessment for AWS Organizations programmatically scans all AWS accounts in an AWS Organization for identity-based and resource-based policies with Organization-based conditions.
Apache License 2.0
25 stars 10 forks source link

Potential Failed-by-Default ECR policy test case for resource-based policy scan #18

Closed GohEeEn closed 9 months ago

GohEeEn commented 11 months ago

Describe the bug The provided mock_data provides a failed-by-default test cases for _tests/test_resource_based_policy/test_ecr_policy_for_organizationsdependency.py, due to the limitation of ECR repository name, that are not supposed to contain any capital letter. The following mock data is the cause of this bug on file ./source/lambda/tests/test_resource_based_policy/mock_data.py:189-191 :

    {
        "MockResourceName": "ResourceWithNoPolicy",
    }

To Reproduce Run the ./source/run-all-tests.sh for custom-build of this solution, without an change to the repository code after exporting a valid AWS_REGION environment value (eg. us-west-1).

Expected behavior There should not have a false-by-default test case from the given mock_data, but fail by misconfiguration or wrongly modified IaC code.

Example Patch Add the .lower() function on the following code block to make the resource name into non-capital letter.

# ./source/lambda/tests/test_resource_based_policy/test_ecr_policy_for_organizations_dependency.py:35-37
ecr_client.create_repository(
    repositoryName=policy_object.get('MockResourceName').lower()
)

Please complete the following information about the solution:

To get the version of the solution, you can look at the description of the created CloudFormation stack.

For example, "(SO0217) - The AWS CloudFormation hub template for deployment of the Account Assessment for AWS Organisations, Version: v1.0.0".

Screenshots Screenshot 2023-08-30 at 14 49 23 Screenshot 2023-08-30 at 14 50 30

Additional context

gockle commented 10 months ago

Hi @GohEeEn Thanks for reporting the issue, we were able to replicate this issue, and we will push a fix in the in the next release.

gockle commented 9 months ago

This issue has been resolved in v1.0.5, closing this issue.