Repository: bridgecrewio/checkov
Pull request: [OCI TF] administrator users are associated with api key
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

resolves #1976 

Added an OCI terraform check that will enforce that administrator users will be associated with an API key.

* I do need an ID to assign to this check

Problem statement:
Add OCI TF check - OCI tenancy administrator users are associated with API keys
Equivalent RQL query

config from cloud.resource where cloud.type = 'oci' AND api.name = 'oci-iam-group' as X; config from cloud.resource where api.name = 'oci-iam-user' as Y; filter '($.X.name equals Administrators) and ($.X.groupMembers[*].userId contains $.Y.id) and ($.Y.apiKeys[*] size greater than 0)';show Y;

Hints from issue discussion:

Commit message: modify test case tldr

Patch to review:
diff --git a/checkov/terraform/checks/graph_checks/oci/AdministratorUserAssociatedWithAPIKey.yaml b/checkov/terraform/checks/graph_checks/oci/AdministratorUserAssociatedWithAPIKey.yaml
new file mode 100644
index 0000000000..cafed3cdf2
--- /dev/null
+++ b/checkov/terraform/checks/graph_checks/oci/AdministratorUserAssociatedWithAPIKey.yaml
@@ -0,0 +1,65 @@
+metadata:
+  id: "PLACEHOLDER"
+  name: "Ensure administrator users are associated with API keys"
+  category: "IAM"
+scope:
+  provider: 'oci'
+definition:
+  or:
+    - and:
+      - resource_types:
+          - oci_identity_group
+        connected_resource_types:
+          - oci_identity_user_group_membership
+        operator: exists
+        cond_type: connection
+      - resource_types:
+          - oci_identity_user_group_membership
+        connected_resource_types:
+          - oci_identity_user
+        operator: exists
+        cond_type: connection
+      - cond_type: attribute
+        attribute: name
+        resource_types:
+          - oci_identity_group
+        operator: equals
+        value: "Administrators"
+      - resource_types:
+          - oci_identity_user
+        connected_resource_types:
+          - oci_identity_api_key
+        operator: exists
+        cond_type: connection
+    - and:
+      - resource_types:
+          - oci_identity_group
+        connected_resource_types:
+          - oci_identity_user_group_membership
+        operator: exists
+        cond_type: connection
+      - resource_types:
+          - oci_identity_user_group_membership
+        connected_resource_types:
+              - oci_identity_user
+        operator: exists
+        cond_type: connection
+      - cond_type: attribute
+        attribute: name
+        resource_types:
+          - oci_identity_group
+        operator: not_equals
+        value: "Administrators"
+    - and:
+      - resource_types:
+          - oci_identity_user_group_membership
+        connected_resource_types:
+          - oci_identity_user
+        operator: not_exists
+        cond_type: connection
+    - cond_type: filter
+      attribute: resource_type
+      value:
+        - oci_identity_user
+      operator: within
+
diff --git a/tests/terraform/graph/checks/resources/AdministratorUserAssociatedWithAPIKey/expected.yaml b/tests/terraform/graph/checks/resources/AdministratorUserAssociatedWithAPIKey/expected.yaml
new file mode 100644
index 0000000000..8619e3e38d
--- /dev/null
+++ b/tests/terraform/graph/checks/resources/AdministratorUserAssociatedWithAPIKey/expected.yaml
@@ -0,0 +1,6 @@
+pass:
+  - 'oci_identity_user.user1'
+  - 'oci_identity_user.user3'
+  - 'oci_identity_user.user4'
+fail:
+  - 'oci_identity_user.user2'
\ No newline at end of file
diff --git a/tests/terraform/graph/checks/resources/AdministratorUserAssociatedWithAPIKey/main.tf b/tests/terraform/graph/checks/resources/AdministratorUserAssociatedWithAPIKey/main.tf
new file mode 100644
index 0000000000..93651cd727
--- /dev/null
+++ b/tests/terraform/graph/checks/resources/AdministratorUserAssociatedWithAPIKey/main.tf
@@ -0,0 +1,111 @@
+# Test Case TLDR:
+# Users in groups:
+#   admin_group:
+#       user1 (has api key)
+#       user2 (no api key)
+#   non_admin_group:
+#       user3 (no api key)
+# Users not in a group:
+#   user4 (no api key)
+
+
+resource "oci_identity_user" "user1" {
+    #Required
+    compartment_id = "var.tenancy_ocid"
+    description = "var.user_description"
+    name = "user1"
+
+    #Optional
+    defined_tags = {"Operations.CostCenter"= "42"}
+    email = "var.user_email"
+    freeform_tags = {"Department"= "Finance"}
+}
+
+resource "oci_identity_user" "user2" {
+    #Required
+    compartment_id = "var.tenancy_ocid"
+    description = "var.user_description"
+    name = "user2"
+
+    #Optional
+    defined_tags = {"Operations.CostCenter"= "42"}
+    email = "var.user_email"
+    freeform_tags = {"Department"= "Finance"}
+}
+
+resource "oci_identity_user" "user3" {
+    #Required
+    compartment_id = "var.tenancy_ocid"
+    description = "var.user_description"
+    name = "user3"
+
+    #Optional
+    defined_tags = {"Operations.CostCenter"= "42"}
+    email = "var.user_email"
+    freeform_tags = {"Department"= "Finance"}
+}
+
+resource "oci_identity_user" "user4" {
+    #Required
+    compartment_id = "var.tenancy_ocid"
+    description = "var.user_description"
+    name = "user3"
+
+    #Optional
+    defined_tags = {"Operations.CostCenter"= "42"}
+    email = "var.user_email"
+    freeform_tags = {"Department"= "Finance"}
+}
+
+
+resource "oci_identity_group" "admin_group" {
+    #Required
+    compartment_id = "var.tenancy_ocid"
+    description = "var.group_description"
+    name = "Administrators"
+
+    #Optional
+    defined_tags = {"Operations.CostCenter"= "42"}
+    freeform_tags = {"Department"= "Finance"}
+}
+
+
+resource "oci_identity_group" "non_admin_group" {
+    #Required
+    compartment_id = "var.tenancy_ocid"
+    description = "var.group_description"
+    name = "NotAdministrators"
+
+    #Optional
+    defined_tags = {"Operations.CostCenter"= "42"}
+    freeform_tags = {"Department"= "Finance"}
+}
+
+
+resource "oci_identity_api_key" "user1_api_key" {
+    #Required
+    key_value = "var.api_key_key_value"
+    user_id = oci_identity_user.user1.id
+}
+
+
+resource "oci_identity_user_group_membership" "user1_in_admin_group" {
+    #Required
+    group_id = oci_identity_group.admin_group.id
+    user_id = oci_identity_user.user1.id
+}
+
+
+resource "oci_identity_user_group_membership" "user2_in_admin_group" {
+    #Required
+    group_id = oci_identity_group.admin_group.id
+    user_id = oci_identity_user.user2.id
+}
+
+
+resource "oci_identity_user_group_membership" "user3_in_non_admin_group" {
+    #Required
+    group_id = oci_identity_group.non_admin_group.id
+    user_id = oci_identity_user.user3.id
+}
+
diff --git a/tests/terraform/graph/checks/test_yaml_policies.py b/tests/terraform/graph/checks/test_yaml_policies.py
index c5aeb1f1b7..2d7349301d 100644
--- a/tests/terraform/graph/checks/test_yaml_policies.py
+++ b/tests/terraform/graph/checks/test_yaml_policies.py
@@ -231,6 +231,9 @@ def test_S3BucketEncryption(self):
     def test_S3BucketLogging(self):
         self.go("S3BucketLogging")
 
+    def test_AdministratorUserAssociatedWithAPIKey(self):
+        self.go("AdministratorUserAssociatedWithAPIKey")
+
     def test_registry_load(self):
         registry = Registry(parser=NXGraphCheckParser(), checks_dir=str(
             Path(__file__).parent.parent.parent.parent.parent / "checkov" / "terraform" / "checks" / "graph_checks"))


Code review comments:
diff_hunk: @@ -0,0 +1,65 @@
+metadata:
+  id: "PLACEHOLDER"
path: checkov/terraform/checks/graph_checks/oci/AdministratorUserAssociatedWithAPIKey.yaml
text: @user2:
@user1-Turgeman doesn't sound like the correct ID to me 😆 

@author:
Changed to CKV2_OCI_1

diff_hunk: @@ -0,0 +1,65 @@
+metadata:
+  id: "PLACEHOLDER"
+  name: "Ensure administrator users are associated with API keys"
+  category: "IAM"
+scope:
+  provider: 'oci'
+definition:
+  or:
+    - and:
+      - resource_types:
+          - oci_identity_group
+        connected_resource_types:
+          - oci_identity_user_group_membership
+        operator: exists
+        cond_type: connection
+      - resource_types:
+          - oci_identity_user_group_membership
+        connected_resource_types:
+          - oci_identity_user
+        operator: exists
+        cond_type: connection
+      - cond_type: attribute
+        attribute: name
+        resource_types:
+          - oci_identity_group
+        operator: equals
+        value: "Administrators"
+      - resource_types:
+          - oci_identity_user
+        connected_resource_types:
+          - oci_identity_api_key
+        operator: exists
+        cond_type: connection
+    - and:
+      - resource_types:
+          - oci_identity_group
+        connected_resource_types:
+          - oci_identity_user_group_membership
+        operator: exists
+        cond_type: connection
+      - resource_types:
+          - oci_identity_user_group_membership
+        connected_resource_types:
+              - oci_identity_user
+        operator: exists
+        cond_type: connection
+      - cond_type: attribute
+        attribute: name
+        resource_types:
+          - oci_identity_group
+        operator: not_equals
+        value: "Administrators"
+    - and:
+      - resource_types:
+          - oci_identity_user_group_membership
+        connected_resource_types:
+          - oci_identity_user
+        operator: not_exists
+        cond_type: connection
path: checkov/terraform/checks/graph_checks/oci/AdministratorUserAssociatedWithAPIKey.yaml
text: @user1:
```suggestion
    - resource_types:
        - oci_identity_user_group_membership
      connected_resource_types:
        - oci_identity_user
      operator: not_exists
      cond_type: connection
```
the `and` is not needed here 🙂 

@user1:
you could also merge the other two `and` blocks to one and then do some differentiation in a nested `or` block, but I think your approach is easier to understand 👍 

@author:
Deleted the unnecessary `and` block

diff_hunk: @@ -0,0 +1,65 @@
+metadata:
+  id: "PLACEHOLDER"
+  name: "Ensure administrator users are associated with API keys"
+  category: "IAM"
+scope:
+  provider: 'oci'
path: checkov/terraform/checks/graph_checks/oci/AdministratorUserAssociatedWithAPIKey.yaml
text: @user2:
@user1-Turgeman this is not necessary, you list the specific resources throughout the definition

@author:
Removed