Support multiple dataset ids in BigQuery rules (#1986)

* support multiple dataset ids in bq rules * fix test name * update rules file
forseti-security · Sep 6, 2018 · 6c6de3f · 6c6de3f
1 parent bf9b667
commit 6c6de3f
Show file tree

Hide file tree

Showing 12 changed files with 71 additions and 18 deletions.
diff --git a/google/cloud/forseti/scanner/audit/bigquery_rules_engine.py b/google/cloud/forseti/scanner/audit/bigquery_rules_engine.py
@@ -37,7 +37,7 @@ class Mode(enum.Enum):
 # Rule definition wrappers.
 # TODO: allow for multiple dataset ids.
 RuleReference = collections.namedtuple(
-    'RuleReference', ['mode', 'dataset_id', 'bindings'])
+    'RuleReference', ['mode', 'dataset_ids', 'bindings'])
 Binding = collections.namedtuple('Binding', ['role', 'members'])
 Member = collections.namedtuple(
     'Member', ['domain', 'group_email', 'user_email', 'special_group'],
@@ -139,11 +139,20 @@ def _build_rule(cls, rule_def, rule_index):
         Returns:
             Rule: rule for the given definition.
         """
-        if 'dataset_id' not in rule_def:
-            raise audit_errors.InvalidRulesSchemaError(
-                'Missing dataset_id in rule {}'.format(rule_index))
+        dataset_ids = []
+        for dataset_id in rule_def.get('dataset_ids', []):
+            dataset_ids.append(regular_exp.escape_and_globify(dataset_id))
 
-        dataset_id = regular_exp.escape_and_globify(rule_def['dataset_id'])
+        # Check `dataset_id` for backwards compatibility.
+        # TODO: stop supporting this.
+        if 'dataset_id' in rule_def:
+            dataset_ids.append(
+                regular_exp.escape_and_globify(rule_def['dataset_id'])
+            )
+
+        if not dataset_ids:
+            raise audit_errors.InvalidRulesSchemaError(
+                'Missing dataset_ids in rule {}'.format(rule_index))
 
         bindings = []
 
@@ -195,7 +204,7 @@ def _build_rule(cls, rule_def, rule_index):
         rule = Rule(rule_name=rule_def.get('name'),
                     rule_index=rule_index,
                     rule_reference=RuleReference(
-                        dataset_id=dataset_id,
+                        dataset_ids=dataset_ids,
                         bindings=bindings,
                         mode=mode))
 
@@ -383,7 +392,9 @@ def _is_binding_applicable(self, binding, bigquery_acl):
                 otherwise.
         """
         rule_regex_to_val = {
-            self.rule_reference.dataset_id: bigquery_acl.dataset_id,
+            # only one dataset needs to match, so union all dataset ids into one
+            # regex expression
+            '|'.join(self.rule_reference.dataset_ids): bigquery_acl.dataset_id,
             binding.role: bigquery_acl.role,
         }
 

diff --git a/rules/bigquery_rules.yaml b/rules/bigquery_rules.yaml
@@ -24,7 +24,7 @@ rules:
       - type: organization
         resource_ids:
           - {ORGANIZATION_ID}
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: '*'
         members:
@@ -35,7 +35,7 @@ rules:
       - type: organization
         resource_ids:
           - {ORGANIZATION_ID}
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: '*'
         members:
@@ -46,7 +46,7 @@ rules:
       - type: organization
         resource_ids:
           - {ORGANIZATION_ID}
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: '*'
         members:

diff --git a/tests/scanner/audit/bigquery_rules_engine_test.py b/tests/scanner/audit/bigquery_rules_engine_test.py
@@ -262,6 +262,20 @@ def test_find_violations_old_style_rules(self):
             fake_bigquery_scanner_data.BIGQUERY_EXPECTED_VIOLATION_LIST,
             actual_violations_list)
 
+    def test_find_violations_multiple_dataset_ids(self):
+        rules_local_path = get_datafile_path(
+            __file__,
+            'bigquery_test_rules_10.yaml')
+        rules_engine = bqe.BigqueryRulesEngine(rules_local_path)
+        rules_engine.build_rule_book()
+        fake_bq_acls_data = create_list_of_bq_objects_from_data()
+        actual_violations_list = []
+        for bqt in fake_bq_acls_data:
+            violation = rules_engine.find_policy_violations(self.project, bqt)
+            actual_violations_list.extend(violation)
+        self.assertEqual(
+            [fake_bigquery_scanner_data.BIGQUERY_EXPECTED_VIOLATION_LIST[0]],
+            actual_violations_list)
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/scanner/audit/data/bigquery_test_rules_1.yaml b/tests/scanner/audit/data/bigquery_test_rules_1.yaml
@@ -18,7 +18,7 @@ rules:
       - type: organization
         resource_ids:
           - 234
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'OWNER'
         members: []
diff --git a/tests/scanner/audit/data/bigquery_test_rules_10.yaml b/tests/scanner/audit/data/bigquery_test_rules_10.yaml
@@ -0,0 +1,28 @@
+# Copyright 2017 The Forseti Security Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+rules:
+  - name: BigQuery test rule
+    mode: blacklist
+    resource:
+      - type: organization
+        resource_ids:
+          - 234
+    dataset_ids:
+      - 'd1'
+      - 'dne'
+    bindings:
+      - role: 'OWNER'
+        members:
+          - user_email: '*'
diff --git a/tests/scanner/audit/data/bigquery_test_rules_2.yaml b/tests/scanner/audit/data/bigquery_test_rules_2.yaml
@@ -17,6 +17,6 @@ rules:
     resource:
       - type: organization
           - 234
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'OWNER'
diff --git a/tests/scanner/audit/data/bigquery_test_rules_3.yaml b/tests/scanner/audit/data/bigquery_test_rules_3.yaml
@@ -18,7 +18,7 @@ rules:
       - type: organization
         resource_ids:
           - 234
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'READER'
         members:

diff --git a/tests/scanner/audit/data/bigquery_test_rules_4.yaml b/tests/scanner/audit/data/bigquery_test_rules_4.yaml
@@ -19,7 +19,7 @@ rules:
       - type: organization
         resource_ids:
           - 234
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'OWNER'
         members:

diff --git a/tests/scanner/audit/data/bigquery_test_rules_5.yaml b/tests/scanner/audit/data/bigquery_test_rules_5.yaml
@@ -18,7 +18,7 @@
       - type: organization
         resource_ids:
           - 234
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'OWNER'
         members:

diff --git a/tests/scanner/audit/data/bigquery_test_rules_6.yaml b/tests/scanner/audit/data/bigquery_test_rules_6.yaml
@@ -18,7 +18,7 @@
       - type: organization
         resource_ids:
           - 234
-    dataset_id: 'dne'
+    dataset_ids: ['dne']
     bindings:
       - role: 'READER'
         members:

diff --git a/tests/scanner/audit/data/bigquery_test_rules_7.yaml b/tests/scanner/audit/data/bigquery_test_rules_7.yaml
@@ -18,7 +18,7 @@
       - type: organization
         resource_ids:
           - 111
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'READER'
         members:

diff --git a/tests/scanner/audit/data/bigquery_test_rules_8.yaml b/tests/scanner/audit/data/bigquery_test_rules_8.yaml
@@ -18,7 +18,7 @@
       - type: organization
         resource_ids:
           - 234
-    dataset_id: '*'
+    dataset_ids: ['*']
     bindings:
       - role: 'OWNER'
         members: