From aced44f051f8f71107b6f903e37a891ab490f358 Mon Sep 17 00:00:00 2001
From: Sergio Garcia <38561120+sergargar@users.noreply.github.com>
Date: Wed, 2 Aug 2023 11:45:27 +0200
Subject: [PATCH] fix(sns): handle topic policy conditions (#2660)

---
 .../sns_topics_not_publicly_accessible.py       | 17 +++++++++++------
 .../sns_topics_not_publicly_accessible_test.py  |  7 ++++---
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.py b/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.py
index e93a4be6..951644be 100644
--- a/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.py
+++ b/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.py
@@ -1,4 +1,7 @@
 from prowler.lib.check.models import Check, Check_Report_AWS
+from prowler.providers.aws.lib.policy_condition_parser.policy_condition_parser import (
+    is_account_only_allowed_in_condition,
+)
 from prowler.providers.aws.services.sns.sns_client import sns_client
 
 
@@ -28,14 +31,16 @@ class sns_topics_not_publicly_accessible(Check):
                                 and "*" in statement["Principal"]["CanonicalUser"]
                             )
                         ):
-                            if "Condition" not in statement:
-                                report.status = "FAIL"
-                                report.status_extended = (
-                                    f"SNS topic {topic.name} is publicly accesible"
+                            if (
+                                "Condition" in statement
+                                and is_account_only_allowed_in_condition(
+                                    statement["Condition"], sns_client.audited_account
                                 )
+                            ):
+                                report.status_extended = f"SNS topic {topic.name} is not public because its policy only allows access from the same account"
                             else:
-                                report.status = "PASS"
-                                report.status_extended = f"SNS topic {topic.name} is publicly accesible but has a Condition that could filter it"
+                                report.status = "FAIL"
+                                report.status_extended = f"SNS topic {topic.name} is public because its policy allows public access"
 
             findings.append(report)
 
diff --git a/tests/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible_test.py b/tests/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible_test.py
index afdc2441..4facdf68 100644
--- a/tests/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible_test.py
+++ b/tests/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible_test.py
@@ -27,7 +27,7 @@ test_policy_restricted_condition = {
             "Principal": {"AWS": "*"},
             "Action": ["sns:Publish"],
             "Resource": f"arn:aws:sns:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:{topic_name}",
-            "Condition": {"StringEquals": {"sns:Protocol": "https"}},
+            "Condition": {"StringEquals": {"aws:SourceAccount": AWS_ACCOUNT_NUMBER}},
         }
     ]
 }
@@ -121,6 +121,7 @@ class Test_sns_topics_not_publicly_accessible:
 
     def test_topic_public_with_condition(self):
         sns_client = mock.MagicMock
+        sns_client.audited_account = AWS_ACCOUNT_NUMBER
         sns_client.topics = []
         sns_client.topics.append(
             Topic(
@@ -144,7 +145,7 @@ class Test_sns_topics_not_publicly_accessible:
             assert result[0].status == "PASS"
             assert (
                 result[0].status_extended
-                == f"SNS topic {topic_name} is publicly accesible but has a Condition that could filter it"
+                == f"SNS topic {topic_name} is not public because its policy only allows access from the same account"
             )
             assert result[0].resource_id == topic_name
             assert result[0].resource_arn == topic_arn
@@ -176,7 +177,7 @@ class Test_sns_topics_not_publicly_accessible:
             assert result[0].status == "FAIL"
             assert (
                 result[0].status_extended
-                == f"SNS topic {topic_name} is publicly accesible"
+                == f"SNS topic {topic_name} is public because its policy allows public access"
             )
             assert result[0].resource_id == topic_name
             assert result[0].resource_arn == topic_arn