Fail additive policy load requests that update existing resources

cyberark · Aug 17, 2023 · 211e432 · 211e432
1 parent 38843c8
commit 211e432
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 53 deletions.
diff --git a/app/models/loader/create_policy.rb b/app/models/loader/create_policy.rb
@@ -16,7 +16,7 @@ def call
 
       @loader.delete_shadowed_and_duplicate_rows
 
-      @loader.store_policy_in_db
+      @loader.store_policy_in_db(reject_duplicates: true)
 
       @loader.release_db_connection
     end

diff --git a/app/models/loader/orchestrate.rb b/app/models/loader/orchestrate.rb
@@ -120,8 +120,9 @@ def delete_shadowed_and_duplicate_rows
     end
 
     # TODO: consider renaming this method
-    def store_policy_in_db
-      eliminate_duplicates_pk
+    def store_policy_in_db(reject_duplicates: false)
+      removed_duplicates_count = eliminate_duplicates_pk
+      raise ApplicationController::BedRequest, "Updating existing resource disallowed in additive policy operation" if removed_duplicates_count.positive? && reject_duplicates
 
       insert_new
 
@@ -243,8 +244,9 @@ def eliminate_duplicates_exact
     end
 
     # Delete rows from the new policy which have the same primary keys as existing rows.
+    # Returns the total number of deleted rows.
     def eliminate_duplicates_pk
-      TABLES.each do |table|
+      TABLES.sum do |table|
         eliminate_duplicates(table, Array(model_for_table(table).primary_key) + [ :policy_id ])
       end
     end

diff --git a/cucumber/api/features/policy_load_annotations.feature b/cucumber/api/features/policy_load_annotations.feature
@@ -17,7 +17,7 @@ Feature: Updating Policies with Annotations
   - create / add new         / POST  :   EXPECTED SUCCESS
   - create / add new         / PATCH :   EXPECTED FAIL - 403 on policy load
   - create / update existing / PUT   :   EXPECTED FAIL - 403 on policy load
-  - create / update existing / POST  :   EXPECTED FAIL - 20x on policy load, annot not updated
+  - create / update existing / POST  :   EXPECTED FAIL - 400 on policy load
   - create / update existing / PATCH :   EXPECTED FAIL - 403 on policy load
   - update / add new         / PUT   :   EXPECTED SUCCESS
   - update / add new         / POST  :   EXPECTED FAIL - 403 on policy load
@@ -26,13 +26,6 @@ Feature: Updating Policies with Annotations
   - update / update existing / POST  :   EXPECTED FAIL - 403 on policy load
   - update / update existing / PATCH :   EXPECTED SUCCESS
 
-  All these outcomes align with our expectations, but one may not align with
-  user expectations: ( create / update existing / POST ). A user may expect that
-  a policy load that tries and fails to update the content of a given annotation
-  should either provide a warning or fail outright.
-
-  How can we update how we handle policy to fail in this case?
-
   Background:
     Given I am the super-user
     And I successfully PUT "/policies/cucumber/policy/root" with body:
@@ -183,45 +176,20 @@ Feature: Updating Policies with Annotations
 
   @negative
   @acceptance
-  Scenario: User with create privilege CAN NOT update existing annotations with POST, but policy loads successfully
+  Scenario: User with create privilege CAN NOT update existing annotations with POST
     When I login as "alice"
-    Then I successfully POST "/policies/cucumber/policy/hosts" with body:
+    And I save my place in the log file
+    Then I POST "/policies/cucumber/policy/hosts" with body:
     """
     - !host
       id: annotated
       annotations:
         description: Success
     """
-    And I successfully GET "/resources/cucumber/host/hosts%2Fannotated"
-    Then the JSON should be:
+    Then the HTTP response status code is 403
+    And The following appears in the log after my savepoint:
     """
-    {
-      "annotations": [
-        {
-          "name": "description",
-          "policy": "cucumber:policy:hosts",
-          "value": "Already annotated"
-        }
-      ],
-      "id": "cucumber:host:hosts/annotated",
-      "owner": "cucumber:policy:hosts",
-      "permissions": [
-        {
-          "policy": "cucumber:policy:root",
-          "privilege": "read",
-          "role": "cucumber:user:bob"
-        },
-        {
-          "policy": "cucumber:policy:root",
-          "privilege": "read",
-          "role": "cucumber:user:alice"
-        }
-      ],
-      "policy": "cucumber:policy:hosts",
-      "restricted_to": [
-
-      ]
-    }
+    Updating existing resource disallowed in additive policy operation
     """
 
   @negative

diff --git a/cucumber/api/features/policy_load_modes.feature b/cucumber/api/features/policy_load_modes.feature
@@ -172,22 +172,17 @@ Feature: Updating policies
 
   @acceptance
   Scenario: POST cannot update existing policy records
-    When I successfully POST "/policies/cucumber/policy/dev/db" with body:
+    When I save my place in the log file
+    And I POST "/policies/cucumber/policy/dev/db" with body:
     """
     - !variable
       id: b
       kind: private key
     """
-    When I successfully GET "/resources/cucumber/variable/dev/db/b"
-    Then the JSON at "annotations" should be:
+    Then the HTTP response status code is 400
+    And The following appears in the log after my savepoint:
     """
-    [
-      {
-        "name": "conjur/kind",
-        "policy": "cucumber:policy:dev/db",
-        "value": "password"
-      }
-    ]
+    Updating existing resource disallowed in additive policy operation
     """
 
   @negative @acceptance